fix(#1541): re-dispatch review after stale-head discard

[fullsend-ai-coder]

When the review agent detects a stale HEAD (e.g. after a force-push during review), it posts a discard notice but no follow-up review is triggered. This leaves the PR with zero agent review.

Three changes fix this:

1. Add a staleHeadError type in postreview.go that carries exit code
   10 (StaleHeadExitCode), distinguishing stale-head from other
   errors.

2. Update cmd/fullsend/main.go to detect errors implementing an
   ExitCode() method and exit with that code instead of the
   default 1.

3. Update post-review.sh to catch exit code 10 and re-dispatch a
   fresh review by posting a /fs-review comment on the PR. A loop
   guard checks for recent re-dispatch comments (within 5 minutes)
   to prevent cascading dispatches from rapid force-pushes.

Note: pre-commit could not run due to infrastructure error (Go toolchain download permission denied in sandbox). The post-script runs an authoritative pre-commit check on the runner.

---

Closes #1541

Post-script verification

• Branch is not main/master (agent/1541-stale-head-redispatch)
• Secret scan passed (gitleaks — 972ce7f6654f0b7edaf699298901b10db69e4e4c..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

Site preview

Preview: https://7ef046c8-site.fullsend-ai.workers.dev

Commit: aaf3d9d002d162f00165e31c55dc4071817ec181

[fullsend-ai-review]

Review

Findings

Medium

• [edge-case] internal/scaffold/fullsend-repo/scripts/post-review.sh:155 — The gh api --paginate --jq combination applies the jq expression independently to each page of results. When there are multiple pages (PRs with >100 issue comments), the | length at the end of the jq filter produces one number per page (e.g. 0\n1). The subsequent [ "${RECENT_REDISPATCH}" -gt 0 ] bash test fails on multi-line input. Since this test is inside an if statement, set -e does not terminate the script — the condition silently evaluates as false, causing the loop guard to fail open and an unnecessary re-dispatch to proceed. Impact is limited: this only affects PRs with >100 comments and the failure mode is an extra re-dispatch (not a missed review).
  Remediation: Either drop --paginate (a single page of 100 comments is sufficient for a 5-minute lookback) or aggregate the per-page counts, e.g.: ... | jq -s 'add'.

• [api-contract] internal/cli/postreview.go:27 — StaleHeadExitCode = 10 is defined in Go but consumed as a hardcoded literal 10 in post-review.sh. This creates a silent coupling between the two files with no enforcement mechanism to keep them synchronized. If the Go constant changes, the shell script will silently break.
  Remediation: Add a test in postreview_test.go that reads post-review.sh and verifies the hardcoded 10 matches StaleHeadExitCode.

Low

• [error-handling] internal/scaffold/fullsend-repo/scripts/post-review.sh:150 — The jq filter compares .created_at against now - 300 | strftime("%Y-%m-%dT%H:%M:%SZ") using string-based lexicographic ordering of ISO 8601 timestamps. This works correctly since both sides produce Zulu time, but the correctness depends on that invariant holding.

• [scope-expansion] internal/scaffold/fullsend-repo/scripts/post-review.sh:146 — Issue Re-trigger review after stale-head discard #1541 noted an alternative approach (investigating synchronize event routing) alongside the implemented re-dispatch approach. A brief comment explaining why re-dispatch was chosen over fixing event routing would help future maintainers understand the trade-off.

• [exit-code-architecture] cmd/fullsend/main.go:10 — The exitCoder interface establishes a new pattern for process exit codes. A brief comment noting when to implement this interface (workflow-level state signaling vs. error reporting) would help future maintainers.

• [naming-convention] cmd/fullsend/main.go:14 — The name exitCoder follows Go conventions (ends in -er, correctly unexported) but Coder is slightly unusual as a suffix. Minor observation.

• [code-organization] internal/scaffold/fullsend-repo/scripts/post-review.sh:145 — The hardcoded exit code 10 and time window 300 are magic numbers embedded in conditionals. The established pattern in this repo's shell scripts is to define named constants near the top of the file for discoverability.

• [test-naming] internal/cli/postreview_test.go:149 — TestStaleHeadError_ExitCode uses an underscore separator. The naming convention in this test file is mixed, but other tests like TestPostStaleHeadNotice use no underscore.

Previous run

Review

Findings

Medium

• [edge-case] internal/scaffold/fullsend-repo/scripts/post-review.sh:141 — The gh api --paginate --jq combination applies the jq expression independently to each page of results. When there are multiple pages (PRs with >100 issue comments), the | length at the end of the jq filter produces one number per page (e.g. 0\n0). The subsequent [ "${RECENT_REDISPATCH}" -gt 0 ] bash test fails on multi-line input (bash integer comparison does not accept newlines). Since this test is inside an if statement, set -e does not terminate the script — the condition silently evaluates as false, causing the loop guard to fail and an unnecessary re-dispatch to proceed. Impact is limited: this only affects PRs with >100 comments and the failure mode is an extra re-dispatch (not a missed review).
  Remediation: Either drop --paginate (a single page of 100 comments is sufficient for a 5-minute lookback) or aggregate the per-page counts, e.g.: ... | awk '{s+=$1} END{print s+0}'.

Low

• [error-handling] internal/scaffold/fullsend-repo/scripts/post-review.sh:155 — The gh pr comment failure path uses || echo "::warning::..." which logs a GitHub Actions warning but exits 0. If the re-dispatch comment fails to post, the stale-head condition is silently swallowed. The synchronize event from the original force-push is the backup mechanism, but if the dispatcher did not route it (which is what caused the original issue Re-trigger review after stale-head discard #1541), the PR could be left without review.

• [api-contract] internal/cli/postreview.go:25 — StaleHeadExitCode = 10 is a Go constant consumed by the shell script as a hardcoded literal 10. No enforcement mechanism ensures they stay in sync. Consider adding a test that reads post-review.sh and verifies the hardcoded value matches the constant.

• [code-organization] internal/scaffold/fullsend-repo/scripts/post-review.sh:145 — The time window 300 (5 minutes) is a magic number embedded in the jq filter. The established pattern in this repo's shell scripts is to define named constants near the top of the script for discoverability.

[ralphbean]

LGTM. One minor note inline.

[ralphbean]

[moderate] The marker value is hardcoded so this is safe today, but jq's --arg would make it defensive by default:

gh api ... --arg marker "${REDISPATCH_MARKER}" --jq \
  '[.[] | select(.body | contains($marker))
        | select(.created_at > (now - 300 | strftime("%Y-%m-%dT%H:%M:%SZ")))] | length'

Non-blocking — just a pattern worth adopting if we touch this again.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:40 PM UTC · Completed 6:52 PM UTC
Commit: ba204cb · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:41 PM UTC · Completed 7:48 PM UTC
Commit: 27b1874 · View workflow run →

[fullsend-ai-retro]

Retro: PR #1834 — Re-dispatch review after stale-head discard

Overall assessment: This was a clean, efficient workflow. The code agent produced a working solution in a single commit (~14 min), the review agent caught two real bugs, and the human reviewer approved the same day. No fix agent cycles were needed. Total agent compute was ~38 minutes; the 6-day wall-clock time to merge was entirely human latency.

What went well:

• One-shot code generation with no rework cycles
• Review agent identified a genuine pagination edge case (loop guard fails open on PRs with >100 comments) and a cross-language constant coupling issue
• Human reviewer (ralphbean) provided a constructive inline suggestion about defensive jq --arg usage

Proposals filed: 1 proposal targeting a real bug the code agent introduced that the review agent caught. The pagination pitfall (gh api --paginate applies jq per-page, so | length produces multiple lines) is a common shell scripting mistake that could be prevented with agent guidance.

Skipped proposals:

• Cross-language exit code sync (Go StaleHeadExitCode=10 vs shell literal 10): Review agent caught this, and it's a minor coupling issue. Code agent should discover and use existing constants instead of hardcoded strings #1810 (code agent should discover existing constants) partially overlaps.
• Unnecessary second review after force-push: Skip re-review on merge-only commits with no code changes #1282 and Reduce unnecessary re-review runs on large PRs #1390 already cover reducing unnecessary re-reviews.
• Human review latency (6 days between approval and merge): Priority-aware escalation for agent-approved PRs awaiting human review #1974 already covers priority-aware escalation for approved PRs.

Proposals filed

• Code agent should understand gh api --paginate + jq per-page semantics (in fullsend-ai/fullsend)