fix(#5188): re-trigger review via label after fix-agent push

[ggallen]

What this fixes

After the fix agent pushes a commit to a PR, the review agent is never re-dispatched (fullsend-ai/fullsend#5188). pull_request_target.synchronize fires on that push, but the dispatch routing's actor-identity check is gated on the PR's original author, which for agent-authored PRs is always the code agent's bot account, regardless of who actually triggered this fix run. GitHub App bots have no collaborator role, so that check always fails closed.

The fix

post-fix.src.sh now calls a new shared library, scripts/lib/relabel-retrigger.lib.sh (retrigger_via_label), which removes then re-adds the ready-for-review label after a successful push, forcing a fresh labeled webhook event. That path has no actor-authorization gate at all — applying a label already requires write access, so no separate identity check is needed — mirroring post-code.src.sh's existing handling of the PR-open case. GitHub does not fire a new labeled event when a label already present is simply re-added, hence the remove-then-add sequence.

Why a library, not inline logic

This was originally implemented inline in fullsend-ai/fullsend#5551. Review feedback there (from @ifireball) pointed out this kind of dispatch-retrigger functionality should be reusable across agent post-scripts, not duplicated per-agent — so it's extracted here as retrigger_via_label, following this repo's existing scripts/lib/*.lib.sh convention (e.g. pr-assignee.lib.sh).

The library also closes a real bug found during that PR's review: the GitHub token is taken as an explicit argument and scoped to each gh invocation individually (GH_TOKEN="${token}" gh ...), rather than relying on the caller having exported GH_TOKEN into the shell environment at the right point in script execution. The original inline version broke exactly that way — GH_TOKEN was exported after these calls ran, so they silently authenticated with whatever token was ambient. Taking the token as a parameter makes that entire class of ordering bug structurally impossible here, rather than merely caught by a test.

Why this repo, not fullsend-ai/fullsend

fullsend-ai/fullsend#5551 attempted this same fix against internal/scaffold/fullsend-repo/scripts/post-fix.sh in that repo. That copy is being deleted entirely by fullsend-ai/fullsend#5588, since agent scripts are now served from this repo at runtime via resolveAgentSource(). This repo's post-fix.sh is the actual production script, and it still has the original #5188 bug — #5551 would have fixed a copy that's already effectively dead. Closed in favor of this PR.

Scope

This PR only covers the fix-agent-push synchronize path described in fullsend-ai/fullsend#5188 — i.e. retrigger_via_label fires from post-fix.src.sh, gated on a successful push by the fix agent itself. It does not cover:

• GitHub's native ready_for_review event (draft-to-ready transition), a different thing from the same-named ready-for-review label this PR toggles.
• synchronize events from any pusher other than the fix agent's own post-script (e.g. a human or other automation pushing directly to a bot-owned PR branch).

Those remaining gaps stay open against fullsend-ai/fullsend#5188 — see the scoping comment there.

Test plan

• bash scripts/relabel-retrigger-test.sh — new tests cover both gh calls receiving the correct token, the remove/add sequence tolerating either call failing without hard-failing the script, and sanitization of injected ::, percent-encoded %0A/%0D, and bare CR sequences in error output.
• Verified the token-scoping test actually catches a regression: removed the explicit GH_TOKEN= scoping from one call and confirmed the test fails; restored and confirmed it passes.
• bash scripts/post-fix-test.sh — no regressions.
• make check-bundle — post-fix.sh is in sync with post-fix.src.sh.
• make script-test — all relevant suites pass (pre-existing gitlint-forbidden-type-scope-test.sh failures in this environment are due to gitlint not being installed locally, unrelated to this change).
• Pre-commit hooks pass, including shellcheck and gitlint.

References fullsend-ai/fullsend#5188 (partial — see Scope above). Supersedes fullsend-ai/fullsend#5551.

[qodo-code-review]

PR Summary by Qodo

Fix: retrigger review by relabeling after fix-agent push

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Re-apply ready-for-review after fix-agent pushes to re-dispatch the review agent.
• Introduce shared retrigger_via_label library with explicit per-call token scoping.
• Add a focused Bash test and update fix-agent docs to match new behavior.

Diagram

graph TD
  A["post-fix(.src).sh"] --> B["retrigger_via_label()"] --> C{{"gh pr edit"}} --> D{{"ready-for-review label"}} --> E{{"labeled webhook"}} --> F["Review agent dispatch"]
  G["post-code(.src).sh"] --> D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fix synchronize-path authorization (use triggering actor, not PR author)

• ➕ Avoids label churn and the small remove/add concurrency window
• ➕ Keeps dispatch semantics tied to pushes rather than label events
• ➖ Requires careful security design: must reliably identify the human/actor behind agent runs
• ➖ May be harder to implement safely across GitHub App/bot identities and events

2. Dispatch review via explicit workflow_dispatch / repository_dispatch

• ➕ Removes reliance on GitHub label event behavior
• ➕ Can include explicit payload (PR number, run context) and clearer auditability
• ➖ More moving parts (additional workflow endpoints, permissions, and request signing concerns)
• ➖ Bypasses the existing label-gated routing conventions already in use

3. Use a status/check-run based trigger (commit status / check_suite)

• ➕ Naturally tied to pushes and commit SHAs
• ➕ Often easier to reason about concurrency than label toggling
• ➖ Would require redesigning the current label-gated routing and filters
• ➖ More work to plumb through existing agent orchestration

Recommendation: Keep the label-based retrigger. It aligns with the existing post-code strategy (label application implies write access, so no extra actor gate is needed), and extracting it into a shared library reduces duplication while eliminating a common ordering bug by passing/scoping the token per gh call. If the remove/add concurrency window becomes a real issue, revisit the synchronize-path authorization fix as the next best option.

Files changed (6) +371 / -13

Bug fix (3) +220 / -12

relabel-retrigger.lib.shAdd shared retrigger_via_label helper (remove-then-add label) +78/-0

Add shared retrigger_via_label helper (remove-then-add label)

• Introduces 'retrigger_via_label' to force a fresh 'labeled' webhook by removing then re-adding a PR label. Scopes the GitHub token per 'gh' invocation and emits notice/warning output via 'gha_echo' when available, while never hard-failing the caller.

scripts/lib/relabel-retrigger.lib.sh

post-fix.shBundle relabel-retrigger lib and retrigger review after successful push +110/-6

Bundle relabel-retrigger lib and retrigger review after successful push

• Bundles the new 'relabel-retrigger' library into the generated post-fix script and adds a new step to relabel 'ready-for-review' after a successful push. Renumbers subsequent steps to account for the new behavior.

scripts/post-fix.sh

post-fix.src.shCall retrigger_via_label after fix-agent push +32/-6

Call retrigger_via_label after fix-agent push

• Sources the new 'relabel-retrigger' library and invokes it after pushing, to re-dispatch review via the authorization-free label path. Adds documentation about the underlying synchronize authorization failure and a narrow concurrency caveat.

scripts/post-fix.src.sh

Tests (2) +150 / -0

MakefileRun relabel-retrigger library tests in script-test target +1/-0

Run relabel-retrigger library tests in script-test target

• Adds 'scripts/relabel-retrigger-test.sh' to the 'script-test' Makefile target so the new library is exercised in CI/local test runs.

Makefile

relabel-retrigger-test.shAdd unit-style Bash tests for relabel-retrigger library +149/-0

Add unit-style Bash tests for relabel-retrigger library

• Adds tests that stub 'gh' to validate remove/add sequencing, GH_TOKEN scoping, non-fatal failure behavior, and sanitization/flattening of error output to avoid unsafe '::' and newline leakage in GitHub Actions logs.

scripts/relabel-retrigger-test.sh

Documentation (1) +1 / -1

fix.mdDocument review retrigger after fix-agent push +1/-1

Document review retrigger after fix-agent push

• Updates the fix-agent pipeline description to include re-applying the 'ready-for-review' label to re-trigger the review agent after pushing fixes.

docs/fix.md

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:07 PM UTC · Completed 3:23 PM UTC
Commit: 037fcf7 · View workflow run →

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

Context used

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

1. Protected scripts/ files modified ✗ Dismissed 📜 Skill insight § Compliance

Description

This PR changes protected governance/infrastructure paths under scripts/, which must not be
auto-approved and requires explicit human review. Protected-path changes are high risk because they
affect CI/runtime automation behavior.

Code

scripts/post-fix.src.sh[R59-60]

+# shellcheck source=lib/relabel-retrigger.lib.sh
+source "${SCRIPT_DIR_POST}/lib/relabel-retrigger.lib.sh"

Relevance

●●● Strong

Protected-path enforcement is explicit; PR #303 added schema rules rejecting approval when
protected-path findings exist.

PR-#303

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The compliance rule explicitly lists scripts/ as a protected path that always requires a finding
when modified. The diff shows new sourcing of a library and new runtime behavior in
scripts/post-fix.src.sh, which is within the protected directory.

scripts/post-fix.src.sh[59-60]
scripts/lib/relabel-retrigger.lib.sh[1-78]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Protected governance/infrastructure paths were modified (under `scripts/`). Policy requires that such PRs are not auto-approved and receive explicit human review.

## Issue Context
This PR introduces/updates post-script behavior and shared libraries under `scripts/`, which affects automation and dispatch behavior.

## Fix Focus Areas
- scripts/post-fix.src.sh[59-60]
- scripts/lib/relabel-retrigger.lib.sh[1-78]
- scripts/post-fix.sh[506-900]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Retrigger step can abort ✓ Resolved 🐞 Bug ☼ Reliability

Description

retrigger_via_label is documented as best-effort (“Never fails the caller's script”) but uses
unguarded mktemp and an unguarded pipefail-sensitive pipeline; under post-fix.src.sh's `set -euo
pipefail` this can terminate the post-fix script after a successful push. That defeats the intended
non-fatal behavior and can skip structured-output processing and summary posting.

Code

scripts/lib/relabel-retrigger.lib.sh[R30-55]

+# Remove then re-add `label` on `repo_full_name`'s pull request `target_pr`,
+# to force a fresh `labeled` webhook event for label-gated dispatch to
+# re-trigger on. Never fails the caller's script — a missed re-dispatch is
+# not worth aborting an otherwise-successful run over.
+# Note: parameter is target_pr (not pr_number) to avoid SC2153 against
+# PR_NUMBER once this lib is bundled into post-fix.sh alongside it — same
+# reasoning as maybe_assign_pr's target_pr in pr-assignee.lib.sh.
+# Args: repo_full_name target_pr label token
+retrigger_via_label() {
+  local repo_full_name="$1" target_pr="$2" label="$3" token="$4"
+
+  local remove_err
+  remove_err="$(mktemp)"
+  if ! GH_TOKEN="${token}" gh pr edit "${target_pr}" --repo "${repo_full_name}" \
+       --remove-label "${label}" 2>"${remove_err}"; then
+    # Expected when the label wasn't present (e.g. a human-authored PR that
+    # never went through the code agent's PR-open labeling); still worth a
+    # breadcrumb since an unexpected API/permission failure looks identical
+    # from here. Flatten embedded newlines/CRs first — sanitize_gha_log_output
+    # strips "::" and percent-encoded %0A/%0D but not literal line breaks,
+    # which would otherwise split this across multiple raw output lines.
+    local flattened_err
+    flattened_err="$(tr -d '\r' < "${remove_err}" | tr '\n' ' ')"
+    _relabel_retrigger_notice "Could not remove ${label} label from ${repo_full_name}#${target_pr} (may not have been present): ${flattened_err}"
+  fi
+  rm -f "${remove_err}"

Relevance

●●● Strong

Team accepted prior “best-effort, don’t abort under set -e/pipefail” hardening (added guards/||
true) in PR #284.

PR-#284

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The function claims it never fails callers but calls mktemp and a tr | tr pipeline without any
|| true/conditional guard; because post-fix.src.sh enables set -euo pipefail and calls this
function after push, any failure in those unguarded commands will abort the script despite the
intended best-effort behavior.

scripts/lib/relabel-retrigger.lib.sh[30-60]
scripts/post-fix.src.sh[52-61]
scripts/post-fix.src.sh[354-381]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`retrigger_via_label()` promises to never fail its caller, but it can still hard-exit under `set -euo pipefail` because `mktemp` and the `tr | tr` pipeline are not protected. This can cause `post-fix.src.sh` / `post-fix.sh` to fail after a successful push.

### Issue Context
The caller (`scripts/post-fix.src.sh`) runs with `set -euo pipefail` and invokes `retrigger_via_label` right after pushing.

### Fix Focus Areas
- scripts/lib/relabel-retrigger.lib.sh[30-60]

### Suggested fix approach
- Avoid `mktemp` entirely and capture stderr safely inside the existing `if ! ...; then` conditional, e.g.:
 - `local remove_err=""`
 - `if ! remove_err=$(GH_TOKEN="${token}" gh pr edit ... --remove-label ... 2>&1); then ... fi`
 - Flatten with bash parameter expansion (`${remove_err//$'\r'/}` and `${remove_err//$'\n'/ }`) instead of a `pipefail` pipeline.
- Ensure any remaining non-critical commands are guarded so the function always returns 0 (best-effort), even if helpers like `mktemp`, `tr`, or cleanup fail.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. gha_echo lacks ANSI/control sanitization 📜 Skill insight ⛨ Security

Description

The new retrigger_via_label path emits GitHub Actions workflow commands via gha_echo, but
sanitize_gha_log_output only strips :: and %0A/%0D sequences and does not remove ANSI escape
sequences or other control characters. This violates the workflow-command sanitization requirement
and could allow log injection or confusing/malicious runner output when error text contains
ANSI/control bytes.

Code

scripts/lib/relabel-retrigger.lib.sh[R64-76]

+_relabel_retrigger_notice() {
+  if declare -F gha_echo >/dev/null 2>&1; then
+    gha_echo notice "$*"
+  else
+    echo "notice: $*"
+  fi
+}
+
+_relabel_retrigger_warn() {
+  if declare -F gha_echo >/dev/null 2>&1; then
+    gha_echo warning "$*"
+  else
+    echo "warning: $*" >&2

Relevance

●● Moderate

Similar workflow-command sanitization (incl. ANSI/control) was only partially adopted in PR #284 and
related hardening was rejected in PR #38.

PR-#284
PR-#38

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The compliance rule requires sanitizing workflow-command interpolations for ::, %0A/%0D, ANSI
escapes, and control characters. The new library uses gha_echo for notice/warning, but
sanitize_gha_log_output only removes :: and %0A/%0D (no ANSI/control stripping), so emitted
workflow commands are not fully sanitized per the rule.

scripts/lib/relabel-retrigger.lib.sh[64-76]
scripts/lib/post-failure-report.lib.sh[25-33]
scripts/lib/post-failure-report.lib.sh[65-70]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`retrigger_via_label` emits `::notice::` / `::warning::` via `gha_echo`, but the underlying sanitizer (`sanitize_gha_log_output` -> `_sanitize_workflow_value`) does not strip ANSI escape sequences or other control characters, which is required for safe GHA workflow-command output.

## Issue Context
The new library includes externally-sourced error text (`flattened_err`) in a `gha_echo notice ...` message. Even though `::` and `%0A/%0D` are stripped, ANSI/control bytes can still manipulate logs.

## Fix Focus Areas
- scripts/lib/post-failure-report.lib.sh[25-33]
- scripts/lib/post-failure-report.lib.sh[49-70]
- scripts/lib/relabel-retrigger.lib.sh[41-54]
- scripts/lib/relabel-retrigger.lib.sh[64-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [test-integrity] scripts/relabel-retrigger-test.sh:159 — The run_retrigger_test function checks gh call log contents using string equality against a newline-separated expected sequence. If the library changes call order, the test breaks on ordering rather than on argument correctness. The call ordering is semantically significant (the pr view precheck must precede --remove-label to determine ground truth), so this is an accepted tradeoff for test simplicity.

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Currently unreachable from post-fix.src.sh (gha_echo is always available via post-failure-report.lib.sh). Practical risk is low.

• [missing-authorization] — This PR references Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 (in a different repository) rather than a local issue. The PR body provides clear problem statement, scope, and rationale. AGENTS.md §6 documents the lockstep versioning relationship between these repos, but does not explicitly define cross-repo references as an authorization path.

• [comment-convention] scripts/lib/relabel-retrigger.lib.sh:124 — The comment describing the gha_echo fallback structure uses a comparative tone ("but that function's own fallback path doesn't sanitize; this one does"). The comparison is factually accurate and serves as an explanatory note for why this library's fallback differs from the convention it follows.

• [missing-library-documentation] scripts/lib/relabel-retrigger.lib.sh — No repository-level documentation catalogs this new library. The library is well-documented internally with comprehensive header comments. Consistent with other libraries in scripts/lib/ which also lack central documentation.

Previous run

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [test-integrity] scripts/relabel-retrigger-test.sh:159 — The run_retrigger_test function checks gh call log contents using string equality against a newline-separated expected sequence. If the library changes call order, the test breaks on ordering rather than on argument correctness. The call ordering is semantically significant (the pr view precheck must precede --remove-label to determine ground truth), so this is an accepted tradeoff for test simplicity.

• [test-adequacy] scripts/relabel-retrigger-test.sh:237 — The run_retrigger_does_not_depend_on_mktemp_test overrides mktemp as a shell function, but relabel-retrigger.lib.sh never calls mktemp. The test exercises the gh-failure-plus-fallback path (which is valid), but the test name and the mktemp override are misleading — the override has no effect on the code under test. The test passes because the library is resilient by design (pure bash builtins), not because it handles mktemp absence.

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Currently unreachable from post-fix.src.sh (gha_echo is always available via post-failure-report.lib.sh). Practical risk is low.

• [pattern-inconsistency] scripts/lib/relabel-retrigger.lib.sh — The _relabel_retrigger_warn and _relabel_retrigger_notice helpers call _relabel_retrigger_sanitize_fallback in the no-gha_echo fallback path, but the comment claims this "matches pr-assignee.lib.sh's _pr_assignee_warn convention." In pr-assignee.lib.sh, _pr_assignee_warn does NOT sanitize in the fallback path. The comment is factually inaccurate — this library adds sanitization that pr-assignee.lib.sh lacks. The extra sanitization is strictly better; only the comment is wrong.

Previous run (2)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [test-integrity] scripts/relabel-retrigger-test.sh:159 — The run_retrigger_test function checks gh call log contents using string equality against a newline-separated expected sequence. If the library changes call order, the test breaks on ordering rather than on argument correctness. The call ordering is semantically significant (the pr view precheck must precede --remove-label to determine ground truth), so this is an accepted tradeoff for test simplicity.

• [test-adequacy] scripts/relabel-retrigger-test.sh:237 — The run_retrigger_survives_broken_mktemp_test overrides mktemp as a shell function, but relabel-retrigger.lib.sh never calls mktemp. The test exercises the gh-failure-plus-fallback path (which is valid), but the test name and the mktemp override are misleading — the override has no effect on the code under test. The test passes because the library is resilient by design (pure bash builtins), not because it handles mktemp absence.

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Currently unreachable from post-fix.src.sh (gha_echo is always available via post-failure-report.lib.sh). Practical risk is low.

Previous run (3)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [test-integrity] scripts/relabel-retrigger-test.sh:159 — The run_retrigger_test function checks gh call log contents using string equality against a newline-separated expected sequence. If the library changes call order, the test breaks on ordering rather than on argument correctness. The call ordering is semantically significant (the pr view precheck must precede --remove-label to determine ground truth), so this is an accepted tradeoff for test simplicity.

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Currently unreachable from post-fix.src.sh (gha_echo is always available via post-failure-report.lib.sh). Practical risk is low.

• [architectural-deviation] scripts/lib/relabel-retrigger.lib.sh — The PR implements a label-based retriggering approach rather than the routing-logic modification proposed in Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188. The PR body explicitly justifies this: the synchronize event has the same actor-identity problem, and label-based retriggering bypasses the authorization check entirely since label application already requires write access. Approach validated through prior review on fix(#5188): re-trigger review via label after fix-agent push fullsend#5551.

Previous run (4)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [test-integrity] scripts/relabel-retrigger-test.sh:159 — The run_retrigger_test function checks gh call log contents using string equality against a newline-separated expected sequence. If the library changes call order, the test breaks on ordering rather than on argument correctness. The call ordering is semantically significant (the pr view precheck must precede --remove-label to determine ground truth), so this is an accepted tradeoff for test simplicity.

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh:115 — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Currently unreachable from post-fix.src.sh (gha_echo is always available via post-failure-report.lib.sh). Practical risk is low.

• [architectural-deviation] scripts/lib/relabel-retrigger.lib.sh — The PR implements a label-based retriggering approach rather than the routing-logic modification proposed in Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188. The PR body explicitly justifies this: the synchronize event has the same actor-identity problem, and label-based retriggering bypasses the authorization check entirely since label application already requires write access. Approach validated through prior review on fix(#5188): re-trigger review via label after fix-agent push fullsend#5551.

Previous run (5)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [test-integrity] scripts/relabel-retrigger-test.sh — The fallback sanitization test's re-sourcing of post-failure-report.lib.sh (to restore gha_echo after unset -f gha_echo) is a no-op due to the file's include guard (POST_FAILURE_REPORT_SH_LOADED is already set). The comment claims restoration occurs but it doesn't. No impact today since the fallback test is last in the file, but a future test appended after this one would silently take the fallback path.

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh:115 — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Practical risk is low.

Previous run (6)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh:115 — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Practical risk is low.

Previous run (7)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

• [incomplete-doc] docs/review.md:17 — The "Triggers" section lists "New commits are pushed to a PR (synchronized)" without qualifying that this path fails for agent-authored PRs due to the actor-identity authorization check in dispatch routing. After this PR, the fix agent uses the ready-for-review label retrigger as the actual review dispatch mechanism for agent PRs, but the Triggers section does not distinguish between synchronize-based triggering (human PRs) and label-based triggering (agent PRs).

Low

• [GHA-sanitization-gap] scripts/lib/relabel-retrigger.lib.sh:113 — The _relabel_retrigger_sanitize_fallback function strips :: and %0A/%0D but does not strip ANSI escape sequences. Consistent with the existing sanitization pattern in _sanitize_workflow_value (post-failure-report.lib.sh) — not a regression. Practical risk is low.

• [scope-creep] scripts/lib/relabel-retrigger.lib.sh — The extraction into a reusable library is currently single-use (only post-fix.src.sh calls retrigger_via_label). The library's error-handling complexity (ground-truth label precheck, distinct failure modes, sanitization fallback) justifies the code volume, but if no other agent post-scripts are planned to use this, an inline implementation would be simpler per AGENTS.md §2.

Previous run (8)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [stale-section-reference] scripts/post-fix-test.sh:325 — The integration test comment says "skips sections 0-4 (secret scan, pre-commit, push)" but the PR inserted a new section 5 (retrigger via label) that is also guarded by NO_PUSH=false and therefore also skipped when NO_PUSH=true. The parenthetical should be "sections 0-5" to accurately describe which sections are bypassed. The target section reference ("section 6") was correctly updated, but the skip-range was not.

Previous run (9)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/post-fix-test.sh, scripts/relabel-retrigger-test.sh — Five files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [stale-section-reference] scripts/post-fix-test.sh:326 — The comment at line 326 references "section 5" for the FULLSEND_VALIDATED_ITERATION_DIR selection logic. This PR renumbered sections by inserting a new section 5 (retrigger), so the structured-output processing that FULLSEND_VALIDATED_ITERATION_DIR belongs to is now section 6, making this cross-reference stale. The separate reference at line 45 ("push retry logic from post-fix.sh section 5") was already incorrect before this PR — the push retry logic has always lived in section 4.

Previous run (10)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/relabel-retrigger-test.sh — Four files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [extra-review-cycle] scripts/post-fix.src.sh — The retrigger_via_label call fires unconditionally when NO_PUSH is false, without checking the iteration count. At the final-cap iteration, this still triggers a review agent run. The review is arguably desirable (the final fix should be reviewed), and any subsequent fix dispatch is blocked by the pre-script's iteration cap check. Minor efficiency concern, not a correctness or security issue.

Previous run (11)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/relabel-retrigger-test.sh — Four files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [extra-review-cycle] scripts/post-fix.src.sh — The retrigger_via_label call fires unconditionally when NO_PUSH is false, without checking the iteration count. At the final-cap iteration, this still triggers a review agent run. The review is arguably desirable (the final fix should be reviewed), and any subsequent fix dispatch is blocked by the pre-script's iteration cap check. Minor efficiency concern, not a correctness or security issue.

• [gha-injection-fallback] scripts/lib/relabel-retrigger.lib.sh — The _relabel_retrigger_notice and _relabel_retrigger_warn fallback paths (when gha_echo is not declared) do not sanitize GHA workflow command sequences. Currently unreachable in post-fix.src.sh (gha_echo is always available via post-failure-report.lib.sh), but a defense-in-depth concern for future callers of this reusable library.

• [scope-creep] .gitignore — Python cache entries (__pycache__/, *.pyc) are unrelated to the linked issue Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188. Infrastructure hygiene bundled with the fix.

Previous run (12)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/relabel-retrigger-test.sh — Four files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes (label-based retrigger to work around actor-identity authorization check failure for bot-authored PRs). Human approval is always required for protected-path changes regardless of context.

Low

• [error-handling-idiom] scripts/lib/relabel-retrigger.lib.sh:64 — The --add-label call suppresses stderr (2>/dev/null), losing diagnostic output (permission errors, rate-limit details, network errors) on failure. The --remove-label path captures stderr and logs it via _relabel_retrigger_notice — consider matching that pattern for the add call.

• [stale-doc] docs/review.md:42 — The ready-for-review label description says "Applied by the code agent after pushing." After this PR, the fix agent also re-applies this label to re-trigger the review agent, making the description incomplete.

• [stale-doc] docs/fix.md:104 — The fix agent's Control labels table lists only fullsend-no-fix and needs-human. After this PR, the fix agent also interacts with ready-for-review (removing and re-adding it). The pipeline description (step 4) was updated in this PR, but the Control labels table was not.

• [stale-doc] docs/code.md:33 — The ready-for-review label description says "Applied by the code agent after pushing a PR." After this PR, the fix agent also re-applies this label, making the description incomplete.

---

Labels: PR modifies fix agent post-script and impacts review agent dispatch via label-based retrigger

Previous run (13)

Review

Findings

Medium

• [protected-path] scripts/lib/relabel-retrigger.lib.sh, scripts/post-fix.sh, scripts/post-fix.src.sh, scripts/relabel-retrigger-test.sh — Four files under the scripts/ protected path are modified. The PR links to Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts fullsend#5188 and the description explains the rationale for the changes. Human approval is always required for protected-path changes regardless of context.

Low

• [race condition / label state inconsistency] scripts/lib/relabel-retrigger.lib.sh:43 — The remove-then-add sequence is not atomic. If the script is killed after the remove succeeds but before the add executes, the ready-for-review label is left removed with no automatic recovery until the next successful fix push. The PR author explicitly acknowledges this race window in a detailed inline comment and documents that it self-heals. Consider adding a trap within retrigger_via_label that re-adds the label on EXIT/TERM if the remove succeeded but the add has not yet run.

• [error-handling-idiom] scripts/lib/relabel-retrigger.lib.sh:52 — The --add-label call suppresses stderr (2>/dev/null), losing diagnostic output (permission errors, rate-limit details, network errors) on failure. The --remove-label path captures stderr to a temp file and logs it via _relabel_retrigger_notice — consider matching that pattern for the add call.

• [stale-doc] docs/review.md:42 — The ready-for-review label description says "Applied by the code agent after pushing." After this PR, the fix agent also re-applies this label to re-trigger the review agent, making the description incomplete.

• [stale-doc] docs/fix.md:104 — The fix agent's Control labels table lists only fullsend-no-fix and needs-human. After this PR, the fix agent also interacts with ready-for-review (removing and re-adding it). The pipeline description (step 4) was updated in this PR, but the Control labels table was not.

• [commit-message-compliance] — The commit includes a Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com> trailer. The codebase explicitly rejects agent-attributed Signed-off-by trailers (post-fix.src.sh section 1b), noting that DCO is a human attestation and the bot noreply email causes gitlint body-max-line-length failures.

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 3:31 PM UTC · Ended 3:52 PM UTC
Commit: 4550e46 · View workflow run →

[ggallen]

cc @rh-hemartin @ifireball — this is the replacement PR for fullsend-ai/fullsend#5551, which I closed after your feedback there that the fix belongs in this repo (the actual production home for agent scripts) rather than fullsend-ai/fullsend's scaffold copy, which is being deleted by fullsend-ai/fullsend#5588.

Same fix, re-implemented against the real post-fix.sh here, and extracted as a shared library (retrigger_via_label in scripts/lib/relabel-retrigger.lib.sh) per @ifireball's suggestion rather than fix-specific inline logic.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:31 PM UTC · Completed 3:52 PM UTC
Commit: 4550e46 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:04 PM UTC · Completed 4:36 PM UTC
Commit: 9ee4cfd · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:41 PM UTC · Completed 4:58 PM UTC
Commit: c8a1b27 · View workflow run →

[waynesun09]

Review-only pass — 4 findings verified against current HEAD (c8a1b27), deduplicated against existing bot/human comments on this thread. No approval or change request; a human reviewer should still weigh in given the protected-path requirement already flagged above.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:32 PM UTC · Completed 7:46 PM UTC
Commit: 7f9976c · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:25 AM UTC · Completed 10:38 AM UTC
Commit: a7ed33c · View workflow run →

[waynesun09]

Review-only pass — findings verified against current HEAD (a7ed33c), deduplicated against existing comments on this thread. No approval or change request.

Severity: HIGH — "Fixes" keyword will auto-close #5188 while two of its three named trigger paths remain broken

Verified via gh pr view 469 --json closingIssuesReferences: GitHub itself has parsed this PR as closing fullsend-ai/fullsend#5188 on merge (via the PR description's closing line and the fix(#5188): ... title). Fetching that issue directly confirms it is still OPEN, labeled priority/high, titled "Review agent skipped on bot-authored PRs due to collaborator permission check failing for GitHub App accounts," and its triage comment states the bug is is_event_actor_authorized(PR_USER_LOGIN) failing closed in reusable-dispatch.yml for pull_request_target opened/synchronize/ready_for_review events on ANY bot-authored PR — with the recommended fix being a routing-logic change to reusable-dispatch.yml/dispatch.yml in fullsend-ai/fullsend.

This PR lives entirely in fullsend-ai/agents and never touches that check. Its actual mechanism, retrigger_via_label, is called from post-fix.src.sh gated on NO_PUSH=false (post-fix.src.sh:373-374) — i.e. it fires only after the fix agent's own successful push. It does not address: (1) GitHub's native ready_for_review event (draft-to-ready transition) that the issue explicitly names — a different thing from the same-named ready-for-review label this PR toggles; (2) synchronize events from any push that isn't the fix agent's own post-script (e.g. a human or other automation pushing directly to a bot-owned PR branch), which is the scenario the issue's own originating evidence describes generically. No existing comment/thread on this PR raises this scope/auto-close concern.

Suggestion: Change "Fixes fullsend-ai/fullsend#5188" to "References fullsend-ai/fullsend#5188" (or similar non-closing phrasing) so the issue doesn't auto-close on merge, and post a comment on #5188 scoping exactly what this PR covers (only the fix-agent-push synchronize path) versus what remains open (the ready_for_review event, and synchronize events from any other pusher), so the remaining gaps stay tracked.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:34 PM UTC · Completed 3:51 PM UTC
Commit: 70202b0 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:59 PM UTC · Completed 4:14 PM UTC
Commit: de5681e · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:22 PM UTC · Completed 4:37 PM UTC
Commit: 833d10d · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[waynesun09]

Review-only pass — 3 findings verified against current HEAD (833d10d), deduplicated against existing comments on this thread. No approval or change request.

[waynesun09]

Severity: HIGH

Unconditional label retrigger double-dispatches review on human-authored PRs.

retrigger_via_label is called unconditionally here on every fix-agent push, with no check of who originally opened the PR. Cross-referencing the live routing logic this interacts with (fullsend-ai/.fullsend/.github/workflows/dispatch.yml, whose "Determine stage" step is unchanged in this regard from fullsend-ai/fullsend's internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml and .github/workflows/reusable-dispatch.yml): under pull_request_target, opened|synchronize|ready_for_review dispatches review via is_event_actor_authorized("${PR_USER_LOGIN}", triage) — keyed on the PR's original author, not who pushed — while labeled dispatches review whenever TRIGGERING_LABEL == "ready-for-review" with no actor-authorization check at all. has_repo_permission()'s role hierarchy means admin/maintain/write roles pass regardless of the requested minimum, so any human PR author with write+ permission — the exact prerequisite for this repo's own pull_request_review.changes_requested auto-fix-loop branch (which requires has_label("fullsend-fix") && has_repo_permission(PR_USER_LOGIN, write) for human-authored PRs) — automatically also passes the triage-level check synchronize uses.

That makes the double-dispatch not merely possible but guaranteed whenever that loop runs: synchronize already dispatches review successfully on the fix agent's push, and this unconditional call fires a second, independent review dispatch via the labeled path for the same push. The review workflow's concurrency group is fullsend-review-${source_repo}-${pr} with cancel-in-progress: true, and pre-review.sh has no head-SHA/already-reviewed dedup (it only skips merged/closed PRs) — so both runs actually start before one is cancelled, non-deterministically discarding whichever run's partial work loses.

TRIGGER_SOURCE can't gate this either: the pull_request_review.changes_requested branch sets TRIGGER_SOURCE="${REVIEW_USER_LOGIN}" (the review bot's own login) in both the bot-authored-PR and human-authored-PR cases, so is_bot_user("${TRIGGER_SOURCE}") is true either way and would get this exact case backwards. Neither the PR description's Scope section (which names two different, unrelated known gaps) nor docs/review.md's new paragraph (which describes the label workaround only in terms of agent-authored PRs) raise or accept this trade-off.

Suggestion: Gate retrigger_via_label on the PR's original author, not on TRIGGER_SOURCE: thread the PR opener's login into post-fix.src.sh as a new input from the calling workflow, and only call retrigger_via_label when that login would fail the same check synchronize uses (e.g. matches a bot-login pattern, or lacks qualifying collaborator permission) — mirroring dispatch.yml's own is_event_actor_authorized. Until that's threaded through, at minimum update the PR description and docs/review.md to explicitly document the double-dispatch trade-off for human-authored PRs undergoing the auto-fix loop, rather than presenting this fix as scoped purely to bot-authored PRs.

[waynesun09]

Severity: MEDIUM

Forced unlabeled event's side effects elsewhere are unverified, not just undocumented.

Every retrigger_via_label call removes ready-for-review then re-adds it, necessarily firing a genuine pull_request_target.unlabeled webhook in addition to the intended labeled one. fullsend-ai/agents/.github/workflows/fullsend.yaml itself subscribes to pull_request_target: types: [..., labeled, unlabeled] and forwards every such event, unfiltered by action, to fullsend-ai/.fullsend's dispatch.yml. That file's pull_request_target case statement has no unlabeled) branch today, so the event is currently a no-op there.

But this codebase also ships a documented, general-purpose CEL-trigger system for exactly this class of event: internal/harnessdispatch/input/ghaevent.go maps raw unlabeled actions to normalized transition.kind == "label_changed" with transition.label.action == "removed", and docs/normative/normalized-event/v1/README.md's own worked CEL example is event.transition.kind == "label_changed" && event.transition.label.name == "ready-to-code" && event.transition.label.action == "added". The symmetric trigger for action == "removed" on ready-for-review — exactly the event this library forces on every fix push — is already fully expressible with today's documented vocabulary, requiring only a per-repo config addition, no dispatch.yml code change.

So "no consumer reacts to this today" is a narrower and more fragile guarantee than the library implies: its doc comment documents live verification that the labeled re-fire works (lines 16-21) but says nothing about the unlabeled transition's consequences, and no test in this PR exercises real webhook/CEL-trigger delivery for it. Because this library is explicitly written to be reused by other agent post-scripts, this unverified assumption propagates to every future adopter.

Suggestion: Add a note to the library's doc comment (next to the existing "Verified live" note) stating plainly that the interim unlabeled event is a no-op only because no current dispatch.yml case or configured CEL trigger reacts to ready-for-review removal, with a pointer to that case statement and to the label_changed/removed CEL vocabulary, so that wiring up such a trigger anywhere becomes a deliberate, reviewed decision rather than a silent regression against this library's callers.

[waynesun09]

Severity: MEDIUM

Durable --add-label failure is silent beyond the Actions log, with no retry.

The only failure handling for the final --add-label call (lines 96-102) is _relabel_retrigger_warn, which emits only a gha_echo warning / echo ... >&2 — a GitHub Actions run-log annotation, never a PR comment — and there is no retry. This call can fail for reasons entirely independent of the concurrency scenario the adjacent "Concurrency note" comment (post-fix.src.sh:367-371) describes: a transient GitHub API error, secondary rate limit, or momentary incident hitting this call specifically, in an otherwise fully sequential, non-concurrent run, immediately after --remove-label succeeded.

In that case there is no "next successful fix push" to self-heal if this was the run's last iteration — the iteration-cap mechanism (post-fix.src.sh:456-473) applies needs-human at the point documented as where "the autonomous review→fix loop needs human direction," i.e. the loop is expected to stop. The PR is then left with ready-for-review genuinely absent (not merely "not re-triggered"), which per docs/code.md ("Also marks workflow state for humans and the retro agent") is itself a human-visible signal, with zero trace on the PR.

For comparison, this same script already has a more visible escalation path for a comparable failure: push failures use post_fail_to_pr, which (via report_post_failure_to_pr in post-failure-report.lib.sh) actually calls gh pr comment to post a real, PR-visible comment — retrigger_via_label's add-label failure has no equivalent. Note: pr-assignee.lib.sh's maybe_assign_pr/_pr_assignee_warn is not a counter-example of visible reporting — its failure path is the same log-only-warning pattern as this library's, not a PR comment.

Suggestion: Add a single bounded retry around the --add-label call specifically (this script already has this convention elsewhere, e.g. its push --force-with-lease retry), since an add-label failure is uniquely consequential compared to a remove-label failure. Independently, surface a durable add-label failure on the PR itself rather than only in the runner log — e.g. fold a note into process-fix-result.py's step-6 summary comment, which runs immediately after this call and already posts a PR-visible comment — so a missed re-dispatch is visible to whoever reads the PR, not only to someone checking the Actions log.

[ggallen]

Closing in favor of a more direct upstream fix, not merging this workaround.

While this PR was in review, @waynesun09 found that its retrigger_via_label mechanism, called unconditionally on every fix-agent push, double-dispatches the review agent on human-authored PRs going through the fix agent (via the automatic changes_requested auto-fix loop or manual /fs-fix) — pull_request_target.synchronize already dispatches review successfully for those (a human author with write+ access passes the same check that only fails for bot accounts), so the label-toggle fires a redundant second dispatch on top of it.

Investigating that led to fullsend-ai/fullsend#5706, which was actually the correct root-cause fix all along: dispatch.yml's pull_request_target → opened|synchronize|ready_for_review routing was missing the [bot]$ carve-out that the code and fix stages already have. That's now fixed in fullsend-ai/fullsend#5782 (merged), which resolves fullsend-ai/fullsend#5188 directly — synchronize now dispatches review correctly for bot-authored PRs with no workaround needed, and with no double-dispatch risk for human-authored PRs, since the carve-out only activates for bot logins.

That makes this PR's scripts/lib/relabel-retrigger.lib.sh/post-fix.src.sh changes unnecessary. Thanks @rh-hemartin for the review and approval, and @waynesun09 for the thorough review throughout — the double-dispatch finding is what led to finding the better fix.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 10:25 AM UTC · Completed 10:38 AM UTC
Commit: 833d10d · View workflow run →

[fullsend-ai-retro]

Retro: PR #469 — re-trigger review via label after fix-agent push

What happened

PR #469 was a human-authored workaround for fullsend-ai/fullsend#5188 (review agent skipped on bot-authored PRs because is_event_actor_authorized() fails for GitHub App accounts). The code agent had previously failed 3 times trying to fix dispatch.yml directly in the fullsend repo — twice due to lint/shellcheck failures, and once due to the GitHub App lacking workflows write permission (a structural limitation). The human author (ggallen) created a workaround in this repo: a shared library (retrigger_via_label) that removes then re-adds the ready-for-review label after fix-agent push to force a fresh dispatch.

The PR went through 14 review runs across 13 commits over 4 days (July 27-31). During review, an external reviewer (waynesun09) found that the workaround fires unconditionally on every fix-agent push, causing double-dispatch on human-authored PRs where the existing synchronize mechanism already works. That insight led to discovering the actual root cause fix: dispatch.yml was missing a [bot]$ carve-out for the review stage (fullsend-ai/fullsend#5782, merged). PR #469 was closed without merge.

What went well

• Review caught a fundamental approach problem. The most impactful outcome was that thorough review revealed the workaround had unintended side effects (double-dispatch), which surfaced the proper root cause fix. The review process worked as designed for catching this class of issue.
• qodo-code-review[bot] found a real bug early. It identified that retrigger_via_label could abort the caller under set -euo pipefail due to unguarded mktemp and pipe usage — a bug the fullsend review agent missed.
• Author responded thoroughly. Every review finding was addressed with a specific commit, clear explanation, and in several cases mutation testing to verify the fix.

Review quality delta

The fullsend-ai-review[bot] found only LOW-severity findings across all 14 runs (GHA sanitization gap, stale section references, test integrity observations, misleading test name). All MEDIUM and HIGH findings came from other reviewers:

• HIGH: Indistinguishable failure modes in error handling (waynesun09)
• HIGH: Fixes #5188 keyword would auto-close a partially-fixed issue (waynesun09)
• MEDIUM: Zero test coverage of the call site in post-fix.src.sh (waynesun09)
• MEDIUM: Unverified claim about GitHub's labeled-webhook dedup behavior (waynesun09)
• MEDIUM: Missing gh pr view precheck failure test coverage (waynesun09)
• Bug: mktemp/pipefail abort under set -euo pipefail (qodo-code-review)

The review agent's correctness sub-agent produced zero findings on an 854-line diff touching shell scripts with complex error handling logic — a notable gap given the density of real bugs found by other reviewers.

Existing issues receiving new evidence

All improvement opportunities from this workflow are covered by existing open issues. New evidence from this retro:

• agents#106 (re-raises findings after author declines scope): The review agent re-posted the identical "GHA-sanitization-gap" finding 3 times across review runs after the author explicitly declined to fix it locally, citing tracking issue gha_echo/sanitize_gha_log_output doesn't strip ANSI escape or other control characters #472. The "test-integrity" finding about call-ordering string equality was also re-posted 3 times after the author confirmed the ordering is semantically significant. Each duplicate required a fresh author response.
• agents#167 (flag PRs that auto-close with partial scope): waynesun09's HIGH finding is a concrete real-world example — the PR's Fixes #5188 title/description would have auto-closed an issue where two of three named trigger paths remained broken.
• agents#490 (verify error-path completeness in shell scripts): The review agent missed a HIGH bug where --remove-label failure for any reason was indistinguishable from the benign "label already absent" case, and the idempotent --add-label call silently swallowed the real failure. This is exactly the class of error-path completeness issue Review agent should verify error-path completeness in shell scripts #490 targets.
• agents#343 (scope re-review to finding verification): When the author pushed commits addressing specific findings, the review agent re-ran full analysis and re-produced the same unaddressed findings rather than focusing on verifying the addressed ones.
• fullsend#3627 and fullsend#3814 (code agent and triage should handle workflow permission limitations): The code agent's 3 failures on the original issue — including the final workflows permission rejection — directly caused a human to create this workaround PR. Days of review effort on the workaround were rendered moot when the proper fix was discovered. Earlier detection that the code agent cannot push to .github/workflows/ would have redirected effort to the correct fix path sooner.
• fullsend#5265 (re-review anchoring should incorporate author response context): The review agent's repeated GHA-sanitization-gap findings persisted because the file was modified in each commit (triggering fresh evaluation), but the sanitization code itself was unchanged. The author's response context ("tracked centrally in gha_echo/sanitize_gha_log_output doesn't strip ANSI escape or other control characters #472") was not incorporated into subsequent runs.