Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions scripts/lib/labels.lib.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env bash
# labels.lib.sh — Idempotent label creation for fullsend dispatch labels.
#
# Source from post-scripts:
# source "${SCRIPT_DIR}/lib/labels.lib.sh"

# shellcheck shell=bash

[[ -n "${LABELS_LIB_SH_LOADED:-}" ]] && return 0
LABELS_LIB_SH_LOADED=1

# _label_defaults LABEL — print "description\tcolor" for known labels.
# Returns 1 for unknown labels (caller should handle).
_label_defaults() {
Comment thread
maruiz93 marked this conversation as resolved.
case "$1" in
ready-for-review) printf '%s\t%s' 'Fullsend: triggers review agent dispatch' '0E8A16' ;;
ready-to-code) printf '%s\t%s' 'Fullsend: triggers code agent dispatch' '0e8a16' ;;
Comment thread
maruiz93 marked this conversation as resolved.
ready-for-triage) printf '%s\t%s' 'Fullsend: awaiting triage agent' 'ededed' ;;
ready-for-merge) printf '%s\t%s' 'Fullsend: all reviewers approved' '0E8A16' ;;
requires-manual-review) printf '%s\t%s' 'Fullsend: review requires human judgment' 'FBCA04' ;;
rejected) printf '%s\t%s' 'Fullsend: approach rejected by review' 'B60205' ;;
needs-human) printf '%s\t%s' 'Fullsend: agent loop needs human input' 'D93F0B' ;;
pr-open) printf '%s\t%s' 'Fullsend: open PR addresses this issue' 'D4C5F9' ;;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] premature-decision — incomplete migration leaves two --force writers that clobber these values

The PR summary lists "No --force: gh label create is called without --force, preserving existing label metadata" as one of three headline fixes, and the inline --force create for pr-open was removed from post-triage.sh. But pre-code.sh — which runs on every code-agent dispatch — still does:

# scripts/pre-code.src.sh:112-114  (bundled: scripts/pre-code.sh:139-141)
gh label create "pr-open" --repo "${REPO_FULL_NAME}" \
  --description "An open PR already addresses this issue" --color "D4C5F9" \
  --force 2>/dev/null || true

So the admin-customisation clobber this PR claims to have fixed is still live for pr-open on the highest-frequency path, and the two writers now disagree on the string: triage creates "Fullsend: open PR addresses this issue", pre-code force-rewrites it to "An open PR already addresses this issue". The description now oscillates depending on which agent ran last — a regression relative to the pre-PR state, where post-triage and pre-code used identical text.

The same flip applies to ready-for-triage via post-retro.sh:146, which also uses --force. That label is one of the three designated mandatory and is not wired to this library at all.

Six inline gh label create sites remain unmigrated:

Site Label --force?
pre-code.src.sh:112 pr-open yes
post-retro.sh:146 ready-for-triage yes
post-fix.src.sh:442 needs-human no
post-review.sh:414 ready-for-merge no
post-review.sh:425 requires-manual-review no
post-review.sh:432 rejected no

Note that post-review.sh, post-retro.sh, and post-fix.sh are non-bundled runtime scripts, so per the critical finding on post-triage.sh:24 they cannot migrate to this lib without being converted to .src.sh + BUNDLE_SRCS first — the "migrate incrementally" path does not currently exist.

Suggested fix: Drop --force from pre-code.src.sh:114 and re-run make script-build (a two-line change to an already-bundled script that actually delivers the stated fix), and align the pr-open description between the two writers. If touching pre-code is out of scope, remove the "No --force" bullet from the PR body and open a follow-up, so the claim isn't recorded as delivered.

needs-info) printf '%s\t%s' 'Fullsend: issue needs more information' 'd876e3' ;;
blocked) printf '%s\t%s' 'Fullsend: issue blocked on prerequisites' 'e11d48' ;;
duplicate) printf '%s\t%s' 'Fullsend: duplicate issue' 'cfd3d7' ;;
triaged) printf '%s\t%s' 'Fullsend: triaged, awaiting prioritization' 'c2e0c6' ;;
question) printf '%s\t%s' 'Fullsend: issue is a question' 'd876e3' ;;
bug) printf '%s\t%s' 'Fullsend: bug report' 'd73a4a' ;;
documentation) printf '%s\t%s' 'Fullsend: documentation improvement' '0075ca' ;;
feature) printf '%s\t%s' 'Fullsend: feature request' 'a2eeef' ;;
not-planned) printf '%s\t%s' 'Fullsend: will not be implemented' 'ffffff' ;;
*) return 1 ;;
esac
}

# ensure_label REPO LABEL — create a label if it does not already exist.
# Uses defaults from _label_defaults when available. No-op when the label
# already exists (gh label create returns non-zero for duplicates).
# Always returns 0 so callers don't need error handling.
ensure_label() {
local repo="$1" label="$2"
local defaults desc color
Comment thread
maruiz93 marked this conversation as resolved.
local -a create_args=("$label" --repo "$repo")

if defaults=$(_label_defaults "$label"); then
desc="${defaults%% *}"
color="${defaults##* }"
create_args+=(--description "$desc" --color "$color")
fi

local err
if ! err=$(gh label create "${create_args[@]}" 2>&1); then
case "$err" in
*already\ exists*) ;;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] premature-decision

The duplicate-detection contract is a match on gh's human-readable error prose. I verified the assumption holds today rather than assuming it — cli/cli pkg/cmd/label/create.go (v2.96.0):

func isLabelAlreadyExistsError(err api.HTTPError) bool {
	return err.StatusCode == 422 && len(err.Errors) == 1 &&
	       err.Errors[0].Field == "name" && err.Errors[0].Code == "already_exists"
}
...
if errors.Is(err, errLabelAlreadyExists) {
    return fmt.Errorf("label with name %q already exists; use `--force` to update its color and description", opts.Name)
}

So on a duplicate, gh exits non-zero and prints a message containing already existsthis glob currently matches, and the comment on line 39 is correct. This is not a live bug.

Two things are unverified, though:

  1. The fallback. The mapping is only applied when the 422 body has exactly one error item with field == "name" and code == "already_exists". Any other shape falls through to go-gh's generic HTTPError.Error(), which renders HTTP 422: Validation Failed (https://api.github.com/repos/o/r/labels)no already exists substring. In that case a benign duplicate is reclassified as a real failure and prints Warning: gh label create ... failed on every run, in every customer repo. That is exactly the log-noise-that-reviewers-learn-to-ignore failure mode Port fullsend PR 5657 (ready-for-review label create-on-missing) to the live post-code.sh in this repo #479 is trying to reduce.
  2. Durability. The contract is an English substring in a fmt.Errorf string with no stability guarantee, and there is no test, no minimum-gh-version assertion, and no comment recording which version was checked.

Suggested fix: Either stop parsing prose and use a documented status-code contract —

gh api "repos/${repo}/labels/${label}" --silent >/dev/null 2>&1 || gh label create ...

— or at minimum match the machine-readable forms too (*already\ exists*|*already_exists*|*HTTP\ 422*), record the verification in the comment (# Verified against gh 2.96.0: ...), and cover it in the new scripts/labels-test.sh so a wording drift surfaces as a test failure rather than as production log noise.

This is about how the create result is classified. It is distinct from the settled API-contract-mismatch thread (which was about when to create — preemptive vs. gating on 404/422) and from the error-handling-gap finding resolved in 50c786e (which was about the failure being swallowed entirely; this concerns the classification logic that fix introduced).

*) echo "Warning: gh label create '${label}' failed: ${err}" >&2 ;;
esac
fi
return 0
}
24 changes: 24 additions & 0 deletions scripts/post-code-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,30 @@ run_branch_validation_test "no-agent-target-ignores-allowed-list" \
run_branch_validation_test "substring-not-accepted" \
"release" "main" "release-1,release-2" "reject:release"

# ---------------------------------------------------------------------------
# Verify the bundled script uses ensure_label from labels.lib.sh for the
# ready-for-review label, rather than inline create-on-missing fallback.
# ---------------------------------------------------------------------------

# Source script must call ensure_label for ready-for-review
if grep -q 'ensure_label.*ready-for-review' "${POST_SCRIPT}"; then
echo "PASS: script-calls-ensure-label"
else
echo "FAIL: script-calls-ensure-label"
echo " ${POST_SCRIPT} does not call ensure_label for ready-for-review"
FAILURES=$((FAILURES + 1))
fi

# Bundled script must have labels.lib.sh inlined (ensure_label + _label_defaults)
BUNDLED_SCRIPT="${SCRIPT_DIR}/post-code.sh"
if grep -q '_label_defaults' "${BUNDLED_SCRIPT}" && grep -q 'ensure_label' "${BUNDLED_SCRIPT}"; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] no-behavioral-test-coverage

Both new assertions are text greps — grep -q 'ensure_label.*ready-for-review' and grep -q '_label_defaults' && grep -q 'ensure_label'. They assert that certain strings appear in a file. Neither ever executes ensure_label.

They would still pass if _label_defaults returned the wrong colour, if the tab-split produced a description containing the colour, if the *already exists* arm were inverted, or if ensure_label unconditionally exit 1'd. The second assertion is also partly redundant with make check-bundle, which already guarantees the lib is inlined.

This matters because the behaviour change in post-triage.sh's add_label() ships with zero coverage: post-triage-test.sh's mock gh (lines 21-43) always exits 0, so the already exists arm, the Warning: arm, and the new exit 1 branch are unexercised anywhere in the suite. And run_test only grep -qFs a single expected pattern, so the newly-emitted gh label create <control-label> line in ~90 other triage tests is entirely unasserted.

The repo has clear precedent for direct library unit tests — scripts/pr-assignee-test.sh, scripts/gitleaks-install-test.sh, and scripts/post-failure-report-test.sh each source their lib and are wired into make script-test. labels.lib.sh is the only lib in scripts/lib/ with no corresponding *-test.sh.

Suggested fix: Add scripts/labels-test.sh that sources the lib with a stub gh on PATH, and register it in the Makefile script-test list next to pr-assignee-test.sh. Minimum cases:

  • known label emits --description/--color with exactly the expected values
  • unknown label emits neither
  • stub gh exiting 1 with label with name "x" already exists... produces no stderr and returns 0
  • stub gh exiting 1 with HTTP 403: Resource not accessible by integration produces a Warning: and still returns 0
  • (once the mandatory gate exists) ensure_label repo question emits no gh call

Also note BUNDLED_SCRIPT is hardcoded to post-code.sh on line 1394, so under SCRIPT_TEST_TARGET=bundled — CI's second pass — that assertion tests the same file twice rather than the source.

echo "PASS: bundled-has-labels-lib"
else
echo "FAIL: bundled-has-labels-lib"
echo " ${BUNDLED_SCRIPT} missing labels.lib.sh functions"
FAILURES=$((FAILURES + 1))
fi

# --- Summary ---

echo ""
Expand Down
76 changes: 72 additions & 4 deletions scripts/post-code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#
# Exit codes:
# 0 — branch pushed and PR created, OR agent determined nothing to do
# 1 — validation failure or error (nothing pushed)
# 1 — validation failure, error, or post-push label application failure
set -euo pipefail

SCRIPT_DIR_POST="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
Expand Down Expand Up @@ -676,6 +676,68 @@ maybe_assign_pr() {
}
}
# END bundled: lib/pr-assignee.lib.sh
# shellcheck source=lib/labels.lib.sh
# BEGIN bundled: lib/labels.lib.sh
# labels.lib.sh — Idempotent label creation for fullsend dispatch labels.
#
# Source from post-scripts:
# source "${SCRIPT_DIR}/lib/labels.lib.sh"

# shellcheck shell=bash

[[ -n "${LABELS_LIB_SH_LOADED:-}" ]] && return 0
LABELS_LIB_SH_LOADED=1

# _label_defaults LABEL — print "description\tcolor" for known labels.
# Returns 1 for unknown labels (caller should handle).
_label_defaults() {
case "$1" in
ready-for-review) printf '%s\t%s' 'Fullsend: triggers review agent dispatch' '0E8A16' ;;
ready-to-code) printf '%s\t%s' 'Fullsend: triggers code agent dispatch' '0e8a16' ;;
ready-for-triage) printf '%s\t%s' 'Fullsend: awaiting triage agent' 'ededed' ;;
ready-for-merge) printf '%s\t%s' 'Fullsend: all reviewers approved' '0E8A16' ;;
requires-manual-review) printf '%s\t%s' 'Fullsend: review requires human judgment' 'FBCA04' ;;
rejected) printf '%s\t%s' 'Fullsend: approach rejected by review' 'B60205' ;;
needs-human) printf '%s\t%s' 'Fullsend: agent loop needs human input' 'D93F0B' ;;
pr-open) printf '%s\t%s' 'Fullsend: open PR addresses this issue' 'D4C5F9' ;;
needs-info) printf '%s\t%s' 'Fullsend: issue needs more information' 'd876e3' ;;
blocked) printf '%s\t%s' 'Fullsend: issue blocked on prerequisites' 'e11d48' ;;
duplicate) printf '%s\t%s' 'Fullsend: duplicate issue' 'cfd3d7' ;;
triaged) printf '%s\t%s' 'Fullsend: triaged, awaiting prioritization' 'c2e0c6' ;;
question) printf '%s\t%s' 'Fullsend: issue is a question' 'd876e3' ;;
bug) printf '%s\t%s' 'Fullsend: bug report' 'd73a4a' ;;
documentation) printf '%s\t%s' 'Fullsend: documentation improvement' '0075ca' ;;
feature) printf '%s\t%s' 'Fullsend: feature request' 'a2eeef' ;;
not-planned) printf '%s\t%s' 'Fullsend: will not be implemented' 'ffffff' ;;
*) return 1 ;;
esac
}

# ensure_label REPO LABEL — create a label if it does not already exist.
# Uses defaults from _label_defaults when available. No-op when the label
# already exists (gh label create returns non-zero for duplicates).
# Always returns 0 so callers don't need error handling.
ensure_label() {
local repo="$1" label="$2"
local defaults desc color
local -a create_args=("$label" --repo "$repo")

if defaults=$(_label_defaults "$label"); then
desc="${defaults%% *}"
color="${defaults##* }"
create_args+=(--description "$desc" --color "$color")
fi

local err
if ! err=$(gh label create "${create_args[@]}" 2>&1); then
case "$err" in
*already\ exists*) ;;
*) echo "Warning: gh label create '${label}' failed: ${err}" >&2 ;;
esac
fi
return 0
}
# END bundled: lib/labels.lib.sh

# ---------------------------------------------------------------------------
# Setup
Expand Down Expand Up @@ -1332,9 +1394,15 @@ echo "pr_url=${PR_URL}" >> "${GITHUB_OUTPUT:-/dev/null}"
# is used instead (label application requires repo write access). See
# .github/scripts/check-e2e-authorization-test.sh for trusted-actor rules.
PR_NUMBER_FROM_URL="${PR_URL##*/}"
gh issue edit "${PR_NUMBER_FROM_URL}" \
ensure_label "${REPO_FULL_NAME}" "ready-for-review"
label_err=""
if label_err=$(gh issue edit "${PR_NUMBER_FROM_URL}" \
--repo "${REPO_FULL_NAME}" \
--add-label "ready-for-review" 2>/dev/null || \
gha_echo warning "Failed to apply ready-for-review label to PR #${PR_NUMBER_FROM_URL}"
--add-label "ready-for-review" 2>&1); then
echo "Applied ready-for-review label to PR #${PR_NUMBER_FROM_URL}"
else
gha_echo error "Failed to apply ready-for-review label to PR #${PR_NUMBER_FROM_URL} — review agent will NOT be dispatched: ${label_err}"
exit 1
fi

maybe_assign_pr "${PR_NUMBER_FROM_URL}"
16 changes: 12 additions & 4 deletions scripts/post-code.src.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#
# Exit codes:
# 0 — branch pushed and PR created, OR agent determined nothing to do
# 1 — validation failure or error (nothing pushed)
# 1 — validation failure, error, or post-push label application failure
set -euo pipefail

SCRIPT_DIR_POST="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
Expand All @@ -46,6 +46,8 @@ source "${SCRIPT_DIR_POST}/lib/post-failure-report.lib.sh"
source "${SCRIPT_DIR_POST}/lib/gitleaks-install.lib.sh"
# shellcheck source=lib/pr-assignee.lib.sh
source "${SCRIPT_DIR_POST}/lib/pr-assignee.lib.sh"
# shellcheck source=lib/labels.lib.sh
source "${SCRIPT_DIR_POST}/lib/labels.lib.sh"

# ---------------------------------------------------------------------------
# Setup
Expand Down Expand Up @@ -702,9 +704,15 @@ echo "pr_url=${PR_URL}" >> "${GITHUB_OUTPUT:-/dev/null}"
# is used instead (label application requires repo write access). See
# .github/scripts/check-e2e-authorization-test.sh for trusted-actor rules.
Comment thread
maruiz93 marked this conversation as resolved.
PR_NUMBER_FROM_URL="${PR_URL##*/}"
gh issue edit "${PR_NUMBER_FROM_URL}" \
ensure_label "${REPO_FULL_NAME}" "ready-for-review"
label_err=""
if label_err=$(gh issue edit "${PR_NUMBER_FROM_URL}" \
--repo "${REPO_FULL_NAME}" \
--add-label "ready-for-review" 2>/dev/null || \
gha_echo warning "Failed to apply ready-for-review label to PR #${PR_NUMBER_FROM_URL}"
--add-label "ready-for-review" 2>&1); then
echo "Applied ready-for-review label to PR #${PR_NUMBER_FROM_URL}"
else
gha_echo error "Failed to apply ready-for-review label to PR #${PR_NUMBER_FROM_URL} — review agent will NOT be dispatched: ${label_err}"
Comment thread
maruiz93 marked this conversation as resolved.
exit 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] unreachable-assignment-and-silent-failure

exit 1 sits directly above maybe_assign_pr "${PR_NUMBER_FROM_URL}" (line 718), making it unreachable on this path. Previously the || gha_echo warning fallback let execution continue, so a PR whose label application failed still got assigned to a human via the invoker → issue assignee → issue author chain.

After this change, a label failure leaves a pushed branch and an open PR with no ready-for-review label and no assignee — strictly less discoverable than before. Issue #479's motivating incident is a PR sitting unreviewed for 24 days, so this inverts the intent of the fix on exactly the path it was meant to harden.

Second problem: exit 1 does not fire the trap 'report_post_failure_to_issue' ERR installed at line 61. Verified:

$ bash -c 'set -euo pipefail; trap "echo ERR-TRAP-FIRED" ERR; if x=$(false 2>&1); then echo ok; else exit 1; fi'; echo "rc=$?"
rc=1        # no ERR-TRAP-FIRED

Every other fatal path in this file routes through post_fail_to_issue <category> <detail> (15 call sites: :117, :131, :301, :312, :326, :444, :520, :694, ...), which posts a categorized comment on the source issue. This new path posts nothing — the only trace is an ::error:: annotation on a red job. Wiring the ERR trap would not help either: post-failure-report.lib.sh:287 defaults the detail to "Post-code script failed before push or PR creation completed", which is factually wrong here since both the push and the PR succeeded.

Suggested fix: Move the assignment above the label block (it does not depend on the label), and route the failure through the existing machinery:

PR_NUMBER_FROM_URL="${PR_URL##*/}"
maybe_assign_pr "${PR_NUMBER_FROM_URL}"

ensure_label "${REPO_FULL_NAME}" "ready-for-review"
if label_err=$(gh issue edit "${PR_NUMBER_FROM_URL}" --repo "${REPO_FULL_NAME}" \
     --add-label "ready-for-review" 2>&1); then
  echo "Applied ready-for-review label to PR #${PR_NUMBER_FROM_URL}"
else
  post_fail_to_issue label-apply-failed \
    "Failed to apply ready-for-review to PR #${PR_NUMBER_FROM_URL} — review agent will NOT be dispatched: ${label_err}"
fi

Distinct from the settled exit-code-contract-violation thread nearby — that was about updating the exit-code doc comment (addressed in 9f8c520), this is about the assignment being skipped and no issue-side signal being emitted.

fi

maybe_assign_pr "${PR_NUMBER_FROM_URL}"
2 changes: 1 addition & 1 deletion scripts/post-triage-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ run_test "in-progress-multiple-prs-second-linked" \

run_test "in-progress-creates-pr-open-label" \
'{"action":"in-progress","reasoning":"PR #50 fixes the reported bug","pull_requests":[{"url":"https://github.com/test-org/test-repo/pull/50"}],"comment":"An open PR is already addressing this issue."}' \
"gh label create pr-open --repo test-org/test-repo --description An open PR already addresses this issue --color D4C5F9 --force"
"gh label create pr-open --repo test-org/test-repo --description Fullsend: open PR addresses this issue --color D4C5F9"

run_test "in-progress-missing-comment-fails" \
'{"action":"in-progress","reasoning":"PR #50 fixes the reported bug","pull_requests":[{"url":"https://github.com/test-org/test-repo/pull/50"}]}' \
Expand Down
8 changes: 5 additions & 3 deletions scripts/post-triage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@

set -euo pipefail

SCRIPT_DIR_TRIAGE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
Comment thread
maruiz93 marked this conversation as resolved.
# shellcheck source=lib/labels.lib.sh
source "${SCRIPT_DIR_TRIAGE}/lib/labels.lib.sh"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] runtime-source-unavailable

post-triage.sh is a hand-maintained runtime script — it is not in BUNDLE_SRCS (Makefile:4 lists only pre-code, post-code, post-fix, post-prioritize). It now sources a file from scripts/lib/, which does not travel with it at runtime. This aborts triage on every enrolled repo, before the triage result is read, before any label, before the triage comment.

Four independent confirmations:

  1. This repo's own README documents the invariant. README.md:55"Harness fetches each runner script as an isolated blob, so post-scripts cannot source files from scripts/lib/ at runtime." And README.md:63"A custom post-script that tries to source a base lib at runtime will fail because the harness only fetches the single script blob."
  2. The resolver fetches exactly one file. fullsend/internal/harness/compose.go:881 fetchBaseFile fetches <baseURLDir> + <relPath> and caches it content-addressed at <workspace>/.fullsend-cache/<sha256>/content. At exec time dirname "${BASH_SOURCE[0]}" is that hash directory — there is no lib/ sibling. Sibling files are never fetched.
  3. The scaffold ships no lib/. internal/scaffold/fullsend-repo/scripts/ contains 12 files and no lib entry.
  4. Reproduced. Copying only post-triage.sh into an empty directory and running it:
    line 24: /private/tmp/blobsim/lib/labels.lib.sh: No such file or directory
    EXIT=1
    
    Under set -euo pipefail the script dies here.

Why CI is green: make check-bundle only covers BUNDLE_SRCS, so post-triage is skipped; post-triage-test.sh:10 sets POST_SCRIPT="${SCRIPT_DIR}/post-triage.sh" so tests always run with lib/ as a sibling; and shellcheck runs with -e SC1091, suppressing the unresolvable source. All three suites pass on this branch.

Suggested fix: Revert the post-triage.sh changes and confine ensure_label to the bundled post-code path — labels.lib.sh + post-code.src.sh already work correctly because they go through the bundler. The alternative (rename to post-triage.src.sh, add to BUNDLE_SRCS, make script-build, commit the generated post-triage.sh, update post-triage-test.sh to honour SCRIPT_TEST_TARGET) is the real fix but is a much larger change than #479 authorizes.

Independently, add a regression guard in scripts/bundle-sh-test.sh: assert that no scripts/*.sh lacking a matching .src.sh contains a source .*lib/.*\.lib\.sh line.

Distinct from the settled inconsistent-variable-naming thread on this line — that was about the SCRIPT_DIR_TRIAGE name, this is about the source on the next line failing at runtime.


# Find the triage result JSON — prefer the validated iteration when set.
# Trust boundary: FULLSEND_VALIDATED_ITERATION_DIR is set by the fullsend CLI
# on the runner — not by the sandbox or the agent. No containment check
Expand Down Expand Up @@ -74,6 +78,7 @@ echo "Issue: #${ISSUE_NUMBER}"

# add_label uses the labels API to avoid firing issues.edited.
add_label() {
ensure_label "${REPO}" "$1"
Comment thread
maruiz93 marked this conversation as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] mandatory-optional-split-not-implemented

Placing ensure_label at the top of add_label() auto-creates every label the action handlers apply, bypassing the label_exists guard entirely. That guard is pre-existing and only covers the label_actions loop (agent-suggested labels) at line ~505.

Labels now auto-created via direct add_label calls: needs-info (:131), duplicate (:146), blocked (:267), pr-open (:314), triaged (:403/432/436), bug (:409), documentation (:417), feature (:431), question (:449), not-planned (:460), ready-to-code (:539). Eleven labels — only one of which (ready-to-code) is in the declared mandatory set.

Driving the script under its own test mock confirms it:

action=question       -> gh label create question    --description Fullsend: issue is a question
action=not-planned    -> gh label create not-planned --description Fullsend: will not be implemented
action=insufficient   -> gh label create needs-info  --description Fullsend: issue needs more information
action=sufficient/bug -> gh label create bug         --description Fullsend: bug report
                         gh label create ready-to-code --description Fullsend: triggers code agent dispatch

question is the exact label cited in the PR discussion as an example of an optional label that should "fail silently if missing". It is auto-created.

The concrete consequence is the one raised in the open CHANGES_REQUESTED: ready-to-code is a dispatch trigger (labels.lib.sh:17 — "triggers code agent dispatch"). A repo owner who deleted ready-to-code to stop fullsend auto-dispatching the code agent will have it silently recreated and applied on the next triage run at line 539. Absent-label-as-opt-out is bypassed for all eleven.

Separately, ready-for-triage — one of the three labels designated mandatory — never reaches ensure_label at all; post-retro.sh:146 still creates it inline with --force.

Suggested fix: Make the split explicit in the library rather than implicit in add_label:

# Only these labels are auto-created; everything else must pre-exist.
_MANDATORY_LABELS="ready-for-review ready-to-code ready-for-triage"
ensure_label() {
  local repo="$1" label="$2"
  case " ${_MANDATORY_LABELS} " in *" ${label} "*) ;; *) return 0 ;; esac
  ...
}

Then call ensure_label only from the sites that need it rather than from generic add_label, and add a post-triage-test.sh case asserting add_label "question" emits no gh label create.

Distinct from the settled scope-exceeded thread on this line — that was about whether these changes are authorized, this is about whether they do what the PR says they do.

local endpoint="repos/${REPO}/issues/${ISSUE_NUMBER}/labels"
local err_output
if ! err_output=$(gh api "${endpoint}" -f "labels[]=$1" --silent 2>&1); then
Expand Down Expand Up @@ -306,9 +311,6 @@ ${FAILED_CREATES}"
remove_label "blocked"
remove_label "ready-to-code"
remove_label "needs-info"
gh label create "pr-open" --repo "${REPO}" \
--description "An open PR already addresses this issue" --color "D4C5F9" \
--force 2>/dev/null || true
add_label "pr-open"
;;

Expand Down
Loading