fix(#7231): remint GitHub App token before post-script

[fullsend-ai-coder]

Summary

Re-mint the GitHub App installation token after the sandbox is torn down and before the post-script runs. Installation tokens live 60 minutes, matching the code agent's budget, so a full-budget run previously handed the post-script an expired token and could not push or post a failure comment.

Related Issue

#7231

Changes

• Call mintAgentToken again from the post-script defer in runAgent when the forge is not GitLab and a mint URL is set.
• Overlay the reminted GH_TOKEN / PUSH_TOKEN (and other role token vars) onto h.RunnerEnv so postScriptEnv's last-wins merge does not restore the token snapshotted at the start of the run.
• Treat remint failure as non-fatal: warn and continue with the existing token so a mint outage cannot discard a completed run.

Testing

• gofmt and go vet ./... pass
• go test -race for remint and existing mint tests in internal/cli
• New tests: post-script env carries the reminted token; remint error is non-fatal and the post-script still runs
• make lint (pre-commit could not fetch remote hook repos in this sandbox; equivalent local hooks were run)

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Commits are not signed off (DCO-exempt autonomous agent commit)
• I wrote this contribution myself and can explain all changes in it

---

Closes #7231

Post-script verification

• Branch is not main/master (agent/7231-remint-post-script-token)
• Secret scan passed (gitleaks — 25d08527c7c4dc5cbfea640923be43510d420fa2..HEAD)
• PR body secret scan passed (gitleaks — no-git)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:40 PM UTC · Completed 1:01 PM UTC

Commit: ad1afb5 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $6.73

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/mintclient/mintclient.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Risk Assessment: moderate (2/5)

Details

Re-review anchoring: Tier 1 signals unchanged from the prior assessment at d65b055 (bot author, no protected/security/CI/dependency paths, 50% test ratio, large blast radius driven by size not qualitative risk); Tier 2 again shows run.go/run_test.go as high-churn hub files with the mintclient files low-churn; Tier 3 remains a tight, well-matched fix for the linked production incident with no new risk-relevant issue labels. This diff is a small additive increment (new exported timeout const with doc comment, an Unwrap method, and tests) on top of the already-assessed state rather than new risk surface, so the composite stays at the prior anchor score of 2 (moderate).

Previous run

Risk Assessment: moderate (2/5)

Details

Tier 1 stays low overall (bot author, no protected/security/CI/dependency paths, 50% test ratio) though LINES_CHANGED crossed into the 300-799 band; Tier 2 confirms run.go/run_test.go are high-churn, high-author-contention hub files; Tier 3 shows a tight, well-matched and now further-hardened fix for the linked production incident. The composite matches the prior anchor score of 2 (moderate) since the added lines are additive hardening (race-condition and context-cancellation fixes plus more tests) addressing prior review feedback rather than new risk surface.

Previous run (2)

Risk Assessment: moderate (2/5)

Details

Tier 1 metadata is uniformly low risk (tiny, well-tested, non-protected, bot-authored fix with no CI/dependency changes), and the linked issue shows a tight scope match with the fix directly addressing the reported production incident, but the touched file (internal/cli/run.go) is an extremely high-churn, high-author-contention hub with dense change coupling to many untouched files and no feature flag guarding the new remint behavior, which pulls the composite up from a Tier-1-only baseline of ~1 to a moderate 2.

[fullsend-ai-review]

Looks good to me

Previous run

Looks good to me

Previous run (2)

Review

Findings

Low

• [race-condition] internal/cli/run.go:1734 — The new post-script remint comment claims os.Setenv is safe because "sandbox streaming and OIDC refresh goroutines have already been torn down (LIFO defers)." That holds for OIDC and sandbox streaming, but not for the run-scoped OpenAI credential refresher: refreshCtx/stopRefresh/refreshWg are registered as a defer around lines 1473-1483, before the post-script defer (~1713), so LIFO stops that goroutine only after remintAgentTokenForPostScript has already called mintAgentToken's os.Setenv. While it's alive, refreshOpenAIProvider (WIF path) calls os.Getenv for the credential and for GITHUB_ACTIONS. This contradicts mintAgentToken's own NOTE that minting must complete before goroutines that read env vars are launched. The practical collision window is narrow (the refresher mostly sleeps until near credential expiry).
  Remediation: Either stop the OpenAI refresher (stopRefresh(); refreshWg.Wait()) before the remint call in the post-script defer, the same way OIDC refresh is already stopped earlier in the defer chain, or correct the comment so it doesn't claim env-reading goroutines are torn down when the OpenAI refresher is not.

• [logic-error] internal/cli/run.go:1736 — remintAgentTokenForPostScript is called with the run's ctx directly rather than a cancellation-independent context. The codebase has an established pattern for this exact situation — the completion-notification defer at internal/cli/run.go:1226 uses context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second) so post-run cleanup isn't aborted by parent-context cancellation. The remaining exposure window here is post-agent-success teardown: if a SIGINT/SIGTERM cancels ctx in that window (e.g., a CI job-level timeout close to the agent's own budget), mintAgentTokenWithRetry returns ctx.Err() immediately and remint falls back non-fatally to the existing, possibly-expired token — narrowing but not fully closing the class of failure Code agent's 60-minute budget equals the GitHub App token lifetime; post-script cannot push on full-budget runs #7231 describes.
  Remediation: Mint with a bounded, cancellation-independent context, e.g. remintCtx, remintCancel := context.WithTimeout(context.WithoutCancel(ctx), <a few seconds>); defer remintCancel(), and pass remintCtx into remintAgentTokenForPostScript/mintAgentToken.

• [missing-test] internal/cli/run_test.go:5653 — The two new tests cover the happy-path RunnerEnv overlay and a remint HTTP error, but two contract-relevant branches remain untested: (1) the GitLab / empty-mintURL early return, which must not call mint or syncRunnerEnvTokens; (2) RunnerEnv missing the token keys entirely (the !ok continue branch in syncRunnerEnvTokens), where postScriptEnv must still resolve the reminted process-env value via last-wins.
  Remediation: Add tests for the gitlab/empty-mintURL skip path (assert zero mint calls, RunnerEnv unchanged) and for RunnerEnv missing the token keys (assert postScriptEnv still last-wins the reminted process-env value).

Info

• [missing-test] internal/cli/run.go:5146 — Minor branch-coverage gaps: remintAgentTokenForPostScript's cleanup == nil return, and syncRunnerEnvTokens' h == nil/h.RunnerEnv == nil guards, are not directly exercised by the new tests. Overall line coverage likely still clears the repo's patch-coverage threshold since surrounding statements are covered.

• [scope-authorization-verified] internal/cli/run.go — The PR implements exactly the suggested fix from issue Code agent's 60-minute budget equals the GitHub App token lifetime; post-script cannot push on full-budget runs #7231 (re-mint the token after agent exit/sandbox teardown, before the post-script runs), not the interim mitigation (lowering the budget). No scope creep beyond the linked issue was found.

• [design-coherence] internal/cli/run.go:5145 — remintAgentTokenForPostScript reuses the existing mintAgentToken helper (same role/roleTokenVars/cleanup idiom as the first mint) rather than introducing a parallel mechanism. syncRunnerEnvTokens is narrowly scoped: h.RunnerEnv is only populated from os.Environ() once, at the start of the run, and postScriptEnv's childScriptEnv merge (os.Environ() then h.RunnerEnv, last-wins) would otherwise re-apply that stale snapshot over the freshly reminted process-env token.

• [forge-abstraction] internal/cli/run.go — The new remint call goes through the pre-existing mintAgentToken helper (which calls out to the mint service via internal/mintclient), not a direct GitHub REST/GraphQL call or gh CLI invocation, consistent with the existing precedent set by the first mint call earlier in this same file.

• [commit-classification] — Title fix(#7231): remint GitHub App token before post-script matches the COMMITS.md fix(#<issue>): ... convention. The change is additive/non-fatal (falls back to the existing token on remint failure) and is correctly unmarked as non-breaking; no ! suffix required.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 1:03 PM UTC · Completed 1:27 PM UTC

Commit: ad1afb5 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.40

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (bot-triggered)

Fixed the two Low-severity correctness findings (the OpenAI-refresher/remint os.Setenv race, and the cancellable-context remint that could be aborted near the run's timeout) and the Low-severity missing-test finding (GitLab/empty-mintURL skip path, RunnerEnv-missing-keys path). Added a test for the Info-level syncRunnerEnvTokens nil-guards gap. Disagreed with testing the Info-level remintAgentTokenForPostScript cleanup==nil branch, which is unreachable dead code given mintAgentToken's actual (nil cleanup, nil err) contract. The remaining Info-level items (scope-authorization-verified, design-coherence, forge-abstraction, commit-classification) were pure affirmations with no remediation requested, so no action was needed for them.

Fixed (4):

1. race-condition: os.Setenv in the post-script remint races the OpenAI credential refresher's os.Getenv reads, contrary to the comment's claim that all env-reading goroutines were already torn down (internal/cli/run.go): Verified: the OpenAI refresher's stop defer (stopRefresh/refreshWg.Wait) is registered earlier in runAgent than the post-script defer, so under LIFO it fires after the post-script defer, not before -- unlike OIDC refresh and sandbox streaming, whose stop defers are registered later and do fire first. Took the more thorough of the reviewer's two suggested remediations: collected each refresher's stop func into a new stopOpenAIRefreshers slice and call all of them explicitly at the top of the post-script defer, before the remint call. Also corrected the comment to describe the OpenAI refresher as an exception that is now stopped explicitly rather than implying it was already torn down by LIFO ordering.
2. logic-error: remintAgentTokenForPostScript is called with the run's cancellable ctx directly, so a SIGINT/SIGTERM near the run's own budget can abort the remint via ctx.Err() and fall back to the possibly-expired token (internal/cli/run.go): Verified against mintAgentTokenWithRetry, which returns ctx.Err() immediately once ctx is done. Added a bounded, cancellation-independent context at the post-script defer call site -- context.WithTimeout(context.WithoutCancel(ctx), remintForPostScriptTimeout) with remintForPostScriptTimeout = 30s -- mirroring the existing completion-notification defer's context.WithoutCancel(ctx) pattern, and pass that context into remintAgentTokenForPostScript instead of ctx.
3. missing-test: two contract-relevant branches in the new remint code are untested -- the GitLab/empty-mintURL early return, and RunnerEnv missing the token keys entirely (internal/cli/run_test.go): Added TestRemintAgentTokenForPostScript_SkipsMintOnGitLabOrEmptyMintURL (table test over gitlab platform and empty mintURL, asserting zero mint calls and an untouched RunnerEnv) and TestRemintAgentTokenForPostScript_RunnerEnvMissingTokenKeys (RunnerEnv present but missing PUSH_TOKEN/GH_TOKEN entirely, asserting syncRunnerEnvTokens does not add the missing keys and that postScriptEnv still resolves the reminted token from process env via childScriptEnv's last-wins merge).
4. missing-test (info): syncRunnerEnvTokens' h == nil and h.RunnerEnv == nil guards are not directly exercised (internal/cli/run_test.go): Added TestSyncRunnerEnvTokens_NilGuards, calling syncRunnerEnvTokens directly with a nil harness and with a harness whose RunnerEnv is nil, asserting neither panics and that a nil RunnerEnv is left nil rather than initialized.

Disagreed (1):

1. missing-test (info): remintAgentTokenForPostScript's cleanup == nil return branch is not directly exercised: Traced mintAgentToken's return contract: every path that returns a nil cleanup also returns a non-nil err (resolveMintRepos failure, ValidateRoleName failure, mintAgentTokenWithRetry failure), and every path with err == nil returns a non-nil cleanup (either func(){} or the real cleanup closure). remintAgentTokenForPostScript already returns early on err != nil before reaching the cleanup == nil check, so that branch is unreachable given mintAgentToken's actual contract -- it is defensive dead code, not an untested real path. Writing a test for it would require a test double that violates mintAgentToken's documented (nil cleanup, nil err) never-both-nil-together invariant, which would test an artificial scenario rather than real behavior, so left as-is rather than fabricating one.

Tests: passed

Decision points

• For the OpenAI-refresher race, stopped the refreshers explicitly instead of only correcting the inaccurate comment (alternatives: Only fix the comment to stop claiming the OpenAI refresher is already torn down, Stop the refresher explicitly before the remint, closing the actual race; rationale: The review offered both as acceptable remediations, but the whole point of this PR (Code agent's 60-minute budget equals the GitHub App token lifetime; post-script cannot push on full-budget runs #7231) is correct token handling around goroutine/env-var lifecycle; leaving a known, name-checked data race in place while just editing a comment would be a weaker fix for a PR specifically about token/env correctness. The fix is small (a slice of stop funcs plus a loop) and both stopRefresh and refreshWg.Wait are safe to call more than once, so it composes cleanly with the existing per-provider defer.)
• Chose 30s for remintForPostScriptTimeout on the cancellation-independent remint context (alternatives: A short 'few seconds' timeout as literally suggested in the review remediation text, 30s, matching mintclient's own per-request http.Client timeout; rationale: mintAgentTokenWithRetry's own retry/backoff and mintclient's internal OIDC-fetch/mint-call retries can legitimately take longer than a few seconds under normal transient conditions; a timeout too close to that would defeat the retry logic and make remint fail more often than the fix warrants. 30s bounds worst-case hang time (mint-service outage) without cutting off a normal successful mint, and remint failure is already non-fatal.)

---

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:29 PM UTC · Completed 1:41 PM UTC

Commit: d65b055 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $3.51

Superseded by updated review

[waynesun09]

/fs-fix Do not sign off. No git commit -s, no Signed-off-by trailer. post-fix.sh rejects any commit carrying that trailer and discards the run.

Two findings to fix. Keep the change inside internal/cli/run.go, internal/cli/run_test.go, and internal/mintclient/ if a helper is needed there. Do not touch .github/ or action.yml.

[MEDIUM] — The 30 s remint bound is shorter than the mint client's own retry schedule, so a slow mint service silently reintroduces #7231

File: internal/cli/run.go:108 and :1769
Finding: remintForPostScriptTimeout = 30 * time.Second was chosen without reference to what a mint costs. On this head, internal/mintclient/mintclient.go:18 sets the HTTP client timeout to 30 s; fetchOIDCJWT (:116) retries 3 attempts with 1 s + 2 s backoff; callMint (:188) retries 5 attempts with 1+2+4+8 = 15 s backoff (doWithRetry, :238-249). Under a 30 s ceiling a single hung request consumes the whole bound and gets zero retries, and a mint returning transient 5xx loses its fifth attempt. When the deadline fires, the remint warns and falls through to the expired token, so the post-script fails with exactly the 401 this PR exists to fix. The first mint at run start runs on the unbounded ctx and gets the full schedule; only the remint is cut. The comment's motivating case, a CI job-level timeout near the agent's budget, has no basis: .github/workflows/reusable-code.yml sets no timeout-minutes.
Suggestion: Derive the bound from the client schedule instead of a fixed number — e.g. export a mintclient helper that returns the worst-case attempt duration (sum of backoffs plus attempts × client timeout, or a documented practical ceiling) and set remintForPostScriptTimeout to at least the full callMint schedule (roughly 90–120 s is defensible). When errors.Is(err, context.DeadlineExceeded), emit a distinct warning so a truncated retry is distinguishable from a real mint rejection. Add a unit test that shortens mintclient.retryBaseDelay and proves the remint survives several transient failures inside the bound.

[MEDIUM] — The defer ordering and cancelled-parent behaviour are shipped without a test that exercises them

File: internal/cli/run_test.go:5653-5840 and internal/cli/run.go:1769-1775
Finding: The four new tests call remintAgentTokenForPostScript and postScriptEnv directly on a hand-built RunnerEnv with context.Background(). That does verify the mint → syncRunnerEnvTokens → childScriptEnv last-wins chain, which is the important part. But nothing exercises what the fix depends on inside runAgent: that the remint runs before postCmd.Env = postScriptEnv(...), that the child process sees the second token, or that remintCleanup and the original mintCleanup unwind in the right order. No existing test reaches the post-script defer (grep "Running post-script" internal/cli/run_test.go is empty). The context.WithTimeout(context.WithoutCancel(ctx), …) from the previous round lives in the closure at run.go:1769, not in the helper, so there is no way to pass an already-cancelled parent and assert the mint still fires. The first test also never asserts restoration order after remintCleanup() and after cleanup().
Suggestion: (1) Move the ctx wrapping into remintAgentTokenForPostScript: accept the run ctx, derive WithTimeout(WithoutCancel(ctx), remintForPostScriptTimeout) inside, cancel on return; add a test that passes an already-cancelled parent and asserts the mint is called once and the fresh token is in env. (2) Extend TestRemintAgentTokenForPostScript_PostScriptEnvUsesFreshToken to assert env state after remintCleanup() (first-mint token restored) and after cleanup() (original value). (3) Add a runAgent-level test using the existing fake openshell stub with a harness post_script that writes $PUSH_TOKEN to a file and a mint stub returning a different token per call; assert the file holds the second token. If the stub cannot carry a run to the post-script, say so in the PR description so the gap is a conscious acceptance rather than an assumption.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 3:43 PM UTC · Completed 4:00 PM UTC

Commit: d65b055 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.57

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Fixed both MEDIUM findings: derived remintForPostScriptTimeout from a new, documented mintclient.MaxMintDuration (120s) instead of an arbitrary 30s, made retryableError unwrappable so a timed-out remint gets a distinct warning, and moved the ctx-wrapping into remintAgentTokenForPostScript so cancellation-survival is directly testable. Added four new tests covering the timeout derivation, distinct warning, cancelled-parent-ctx survival, and defer-restoration order, plus extended the mintclient test suite with a transient-failure survival test. Disclosed one sub-item (a full runAgent-level integration test through the post-script) as out of proportion for this fix, per the review's own escape hatch.

Fixed (2):

1. The 30s remint bound is shorter than the mint client's own retry schedule, so a slow mint service silently reintroduces Code agent's 60-minute budget equals the GitHub App token lifetime; post-script cannot push on full-budget runs #7231 (internal/cli/run.go): Added mintclient.MaxMintDuration (120s, documented against fetchOIDCJWT's 3-attempt/3s-backoff and callMint's 5-attempt/15s-backoff schedules, with reasoning for why it is a practical ceiling rather than the literal 258s worst case) and set remintForPostScriptTimeout to that value instead of an arbitrary 30s. Also made retryableError implement Unwrap (internal/mintclient/mintclient.go) so errors.Is(err, context.DeadlineExceeded) works even when the deadline lands mid-HTTP-request, and used that in remintAgentTokenForPostScript to emit a distinct 'timed out' warning instead of the generic mint-failure message. Added a mintclient-level test (TestMintToken_SurvivesTransientFailuresWithinMaxMintDuration) that shortens retryBaseDelay and proves MintToken completes within MaxMintDuration despite several transient 5xx responses across both the OIDC exchange and the mint call.
2. The defer ordering and cancelled-parent behaviour are shipped without a test that exercises them (internal/cli/run.go, internal/cli/run_test.go): Moved the context.WithTimeout(context.WithoutCancel(ctx), remintForPostScriptTimeout) wrapping from the runAgent call site into remintAgentTokenForPostScript itself, so the cancellation-survival behavior is testable without going through the full runAgent closure. Added TestRemintAgentTokenForPostScript_SurvivesCancelledParentCtx, which passes an already-cancelled parent ctx and asserts the mint still fires once and the fresh token lands in postScriptEnv. Added TestRemintAgentTokenForPostScript_DeadlineExceededGetsDistinctWarning covering the new distinct-warning behavior. Extended TestRemintAgentTokenForPostScript_PostScriptEnvUsesFreshToken to call remintCleanup() and the first mint's cleanup() explicitly (instead of only deferring them) and assert env state after each: remintCleanup() restores the first-mint token, and cleanup() then restores the pre-mint value, matching the real LIFO defer order in runAgent. Did not add the suggested runAgent-level test that drives a real post-script process end-to-end through the fake openshell stub to observe the second minted token land in the child process's env: the existing stubs (testdata/openshell, testdata/providers-stub) fail before reaching sandbox streaming/extraction, and no existing test reaches that point either (confirmed via grep for 'Running post-script' in run_test.go, matching the reviewer's own observation). Building fake-openshell/sandbox-exec machinery deep enough to reach the post-script defer with runErr == nil would be a disproportionate addition for a review-feedback fix and risks introducing flaky integration-test infrastructure; disclosing this gap explicitly, as the review's own suggestion allowed.

Tests: passed

Decision points

• Chose 120s as mintclient.MaxMintDuration rather than the literal worst-case 258s (every one of fetchOIDCJWT's 3 + callMint's 5 attempts hanging the full 30s HTTP client timeout) or the raw 18s backoff-only schedule (alternatives: 258s literal worst case (attempts x client timeout + backoff), 18s backoff-only estimate, 120s documented practical ceiling; rationale: 258s is unrealistically long to block teardown on; a real mint-service outage produces fast failing 5xx responses dominated by backoff, not attempts that each hang the full client timeout. 120s sits within the review's own suggested 90-120s range, comfortably covers the backoff schedule plus real per-request latency, and is documented in mintclient.MaxMintDuration's doc comment with the full accounting so a future change to the retry schedule prompts revisiting it.)
• Skipped the runAgent-level end-to-end post-script test suggested as sub-item 3 of the second finding (alternatives: Build new fake-openshell/sandbox-exec stub infrastructure to carry a full run to the post-script defer, Disclose the gap as the review's own suggestion allowed; rationale: No existing test in run_test.go reaches the post-script defer; doing so requires simulating sandbox streaming and SafeDownload extraction, which is a large, separate testing-infrastructure investment out of proportion to this fix's scope. The review finding itself said to disclose this if the stub can't carry a run to the post-script.)

---

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:02 PM UTC · Completed 4:18 PM UTC

Commit: 8881591 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $6.11

Superseded by updated review

[waynesun09]

Approving at 8881591 after three review rounds.

The runtime fix is correct and I verified its premises: the remint runs after sandbox teardown on a cancellation-immune, bounded context; syncRunnerEnvTokens defeats postScriptEnv's last-wins merge, which is what makes the fresh token actually reach the child; cleanup ordering under LIFO restores the env correctly; the cancel func is deferred on every return path; the second token is masked; GitLab is skipped; and fix/review/retro/triage take the same path with consistent role→token mapping. Nothing regresses relative to main, and this closes the failure mode that discarded a complete run on #7218.

Three non-blocking follow-ups, each a one-liner or doc-only, worth a small follow-up PR:

1. TestRemintAgentTokenForPostScript_SurvivesCancelledParentCtx is currently vacuous. Its fake mint discards ctx, and mintAgentTokenWithRetry calls the mint before ever checking ctx.Err(), so the test still passes with context.WithoutCancel removed. Fix: have the fake return ctx.Err() when non-nil before incrementing calls.
2. The "timed out" warning misfires on a final-attempt Client.Timeout. errors.Is(err, context.DeadlineExceeded) now sees through the new retryableError.Unwrap, and net/http's timeout error deliberately matches DeadlineExceeded, so a run that exhausts the client's full schedule is reported as bound truncation. Fix: discriminate on remintCtx.Err() != nil instead; the existing test stays green.
3. MaxMintDuration doc names the wrong caller and states a premise as fact. The bound encloses mintAgentTokenWithRetry (up to 4 MintToken calls, +14 s backoff), not a single MintToken, and "outages fail fast" is an assumption; under a hanging service the bound truncates at mint attempt 4. Either state the assumption explicitly and correct the caller name, or extract the attempt counts as consts and assert the invariant in a test.

Also noted, minor: the transient-failure test cannot fail on the constant it names (40 ms of exercised backoff vs a 120 s bound), and syncRunnerEnvTokens refreshes only same-named keys, which is fine for every current harness but worth a sentence in its doc.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 6:18 PM UTC · Completed 6:28 PM UTC

Commit: 8881591 · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.11

[fullsend-ai-retro]

PR #7234 (fixing #7231: the GitHub App installation token expiring before the post-script ran on full-budget code-agent runs) went through triage → code → 3 review/fix rounds → merge, all within ~6 hours on 2026-09-11, at ~$21 total agent spend. The triage agent correctly diagnosed the root cause but misjudged scope, flagging needs-human because it believed the fix required editing .github/workflows//action.yml; the human (waynesun09) corrected this within ~8 minutes since the token is actually minted by the runner's own Go code, not the workflow. This is the same failure shape already tracked by open issue #3814 ('triage should avoid recommending workflow changes when the fix is app code') — #7231 is a fresh corroborating instance; no new issue filed.

The review agent's round-1 pass found 3 legitimate Low-severity issues (a goroutine race with a newly-added OpenAI credential refresher, a cancellable-context bug, missing tests) that the fix agent addressed correctly — genuine value-add, not noise. Rounds 2 and 3 both returned a bare 'Looks good to me'; this is the harness's mandated zero-findings output format, not evidence of a skipped review — the agents-repo review skill (fullsend-ai/agents@791d281c, skills/pr-review + skills/code-review) keeps the 'correctness' dimension at full scope on every re-review round by design. Despite that, the human reviewer found a MEDIUM issue after round 2 (an unjustified 30s remint timeout, shorter than the mint client's own retry schedule) and, after round 3, three more non-blocking issues: a vacuous cancellation test (matches open issue fullsend-ai/agents#681 on verifying test-assertion effectiveness — new corroborating instance, no new issue filed), a doc comment naming the wrong function, and — most notably — a new errors.Is(err, context.DeadlineExceeded) check that misfires against net/http's own timeout error once the wrapped retryableError gained an Unwrap method.

That last issue is the third dated occurrence of the identical Timeout()/DeadlineExceeded/Unwrap confusion in this repo (after PR #6217 and its follow-ups #6424 and #6425, both closed within the last 3 weeks) — documentation and single-instance hardening haven't stopped it recurring in new code, and it slipped past 3 rounds of automated review whose correctness dimension runs at full scope every round. I'm filing one proposal to consolidate the ~5 independent hand-rolled implementations of this check into a single shared, regression-tested helper, since that closes the gap regardless of whether the review agent's checklist happens to catch the next instance. I did not file additional narrow 'correctness sub-agent should check X' issues for the timeout-derivation or vacuous-test findings: fullsend-ai/agents already carries a very large (30+) backlog of similarly narrow, seemingly-unimplemented correctness-checklist issues, and adding to that pile seemed lower value than the consolidation fix and the evidence notes above.

Proposals filed

• Consolidate hand-rolled context.DeadlineExceeded/Canceled detection into one tested helper (in fullsend-ai/fullsend)