fix(#2783): refresh OIDC token before agent start

[waynesun09]

Summary

• Add an immediate refreshOIDCToken() call right before starting the periodic refresh goroutine. The pre-fetched OIDC token from prepare-sandbox-credentials.sh expires after 5 minutes (GHA OIDC token lifetime), but sandbox setup can exceed that window — causing invalid_grant on the agent's first inference call.
• Fix incorrect token lifetime comment in prepare-sandbox-credentials.sh (~10 min → ~5 min).

Test plan

• Existing OIDC unit tests pass (go test ./internal/cli/ -run OIDC)
• Lint passes (make lint)
• Verify in a WIF-enabled org with slow sandbox setup (>5 min) that the agent's first inference call succeeds

Closes #2783

[qodo-code-review]

PR Summary by Qodo

Refresh OIDC token immediately before agent start (WIF mode)

🐞 Bug fix 📝 Documentation 🕐 10-20 Minutes

AI Description

• Refresh the GHA OIDC token once after sandbox setup, before starting the agent.
• Prevent first-call invalid_grant when sandbox provisioning exceeds the 5-minute token lifetime.
• Correct the OIDC token lifetime comment in the sandbox credentials script.

Diagram

graph TD
  A(["fullsend CLI: runAgent"]) --> B(["refreshOIDCToken (initial)"]) --> C[["OIDC auth file"]] --> D(["start refresh goroutine"]) --> E(["refreshOIDCToken (periodic)"]) --> C
  A --> F(["agent inference call"]) --> G{{"GCP STS / WIF"}}

  subgraph Legend
    direction LR
    _svc([Service/Process]) ~~~ _file[[File]] ~~~ _ext{{External}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Refresh-on-failure retry for first inference call

• ➕ Fixes only when needed (stale token actually encountered).
• ➕ Can improve resilience for other transient auth failures.
• ➖ Adds complexity to the inference call path and error handling.
• ➖ First call still fails once, complicating UX and logs.

2. Refresh based on token expiry (decode exp/iat)

• ➕ Avoids an extra network call when token is still fresh.
• ➕ Makes refresh scheduling more precise than a fixed timer.
• ➖ Requires reliably parsing/validating token claims and clock skew handling.
• ➖ More moving parts than a simple unconditional pre-start refresh.

Recommendation: Keep the PR’s approach: an unconditional refresh immediately before starting the agent is the simplest way to guarantee a fresh token and avoid a predictable first-call failure when sandbox setup is slow. The alternatives add complexity or allow an initial failure that this change cleanly prevents.

Files changed (2) +5 / -1

Bug fix (1) +4 / -0

run.goForce an initial OIDC refresh before launching periodic refresh goroutine +4/-0

Force an initial OIDC refresh before launching periodic refresh goroutine

• Adds a one-time 'refreshOIDCToken()' call after sandbox setup and before the existing refresh goroutine starts. This prevents 'invalid_grant' on the agent’s first call when the pre-fetched GHA OIDC token has already expired.

internal/cli/run.go

Documentation (1) +1 / -1

prepare-sandbox-credentials.shCorrect GHA OIDC token lifetime comment +1/-1

Correct GHA OIDC token lifetime comment

• Updates the script comment to reflect the ~5 minute GHA OIDC token lifetime instead of ~10 minutes, aligning documentation with actual behavior.

internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh

[github-actions]

Site preview

Preview: https://554158f0-site.fullsend-ai.workers.dev

Commit: 49edb649020c6d421feb01ffe574be1f6a0bfbb7

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 6:01 PM UTC · Ended 6:07 PM UTC
Commit: f417572 · View workflow run →

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Tickets: 🎫 OIDC token stale before first inference call when sandbox setup exceeds 5 minutes

✅ Compliance rules (platform): 61 rules

✅ Skills: writing-user-docs, writing-adrs

1. Uncancelable startup OIDC refresh ✗ Dismissed 🐞 Bug ☼ Reliability

Description

runAgent now performs an initial refreshOIDCToken() synchronously using an oidcCtx derived from
context.Background(), so cmd.Context() cancellation (Ctrl-C) can’t interrupt the startup refresh and
agent start can stall waiting on network/upload timeouts. refreshOIDCToken() also calls
sandbox.UploadFile(), whose underlying Upload() uses context.Background() with a 5-minute transfer
timeout, further preventing timely cancellation during hangs.

Code

internal/cli/run.go[R924-927]

+			// GHA OIDC tokens expire after 5 min; sandbox setup can exceed that.
+			if err := refreshOIDCToken(oidcCtx, sandboxName, oidcURL, oidcAuth); err != nil {
+				printer.StepWarn("Initial OIDC refresh failed: " + err.Error())
+			}

Relevance

⭐⭐⭐ High

Team has accepted making network flows cancellable via propagated contexts (token-scope ctx;
signal-aware ctx into sandbox exec).

PR-#1333
PR-#2182

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The initial refresh is executed synchronously during startup using a Background-derived context, and
the upload helper used by the refresh is also built on Background with a 5-minute timeout—together
making the startup refresh difficult/impossible to cancel promptly and capable of stalling agent
start under network/upload hangs.

internal/cli/run.go[917-933]
internal/cli/run.go[1764-1807]
internal/sandbox/sandbox.go[21-26]
internal/sandbox/sandbox.go[428-445]

Agent prompt

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

## Issue description
The newly-added synchronous `refreshOIDCToken()` call runs before the agent starts, but it is not cancellable by the CLI’s `cmd.Context()` because `oidcCtx` is derived from `context.Background()`, and the sandbox upload path also uses `context.Background()`.

## Issue Context
- `runAgent` receives `cmd.Context()` and should respect cancellation.
- Initial refresh is now on the critical path; when the OIDC endpoint or sandbox upload is slow/hung, startup can block and ignore Ctrl-C.

## Fix Focus Areas
- Derive OIDC context from the run context (not `Background`).
- Bound the initial refresh with a short timeout.
- Make the sandbox upload path accept a caller context (or at minimum a caller-provided timeout) so cancellation can interrupt the upload.

### Suggested approach
1. Change:
  - `oidcCtx, oidcCancel := context.WithCancel(context.Background())`
  to:
  - `oidcCtx, oidcCancel := context.WithCancel(ctx)`
2. Wrap the initial call in a timeout context:
  - `refreshCtx, cancel := context.WithTimeout(oidcCtx, 30*time.Second)`
  - `defer cancel()`
  - `refreshOIDCToken(refreshCtx, ...)`
3. Update `sandbox.Upload` / `sandbox.UploadFile` (or add a new variant) to accept a `context.Context` so the upload can be canceled promptly.

## Fix Focus Areas (code references)
- internal/cli/run.go[917-933]
- internal/cli/run.go[1764-1807]
- internal/sandbox/sandbox.go[21-26]
- internal/sandbox/sandbox.go[428-445]

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

---

2. runAgent OIDC refresh untested 📘 Rule violation ▣ Testability

Description

New OIDC behavior was added in runAgent (one-shot refreshOIDCToken() before starting the
periodic refresh goroutine) without adding/updating tests to cover this new control flow. This can
regress silently (e.g., the initial refresh call could be removed or reordered) while existing
helper-level tests continue to pass.

Code

internal/cli/run.go[R924-930]

+			// GHA OIDC tokens expire after 5 min; sandbox setup can exceed that.
+			if err := refreshOIDCToken(oidcCtx, sandboxName, oidcURL, oidcAuth); err != nil {
+				printer.StepWarn("Initial OIDC refresh failed: " + err.Error())
+			}
			printer.StepDone("OIDC token refresh enabled (WIF mode)")
			oidcWg.Add(1)
			go func() {

Relevance

⭐ Low

Similar “add integration/unit tests for run.go behavior” suggestions were rejected (e.g., CLAUDE.md
injection, excludeAgentWorkingDirs).

PR-#2428
PR-#1627

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062049 requires tests for new/modified Go logic. The PR adds a new initial refresh
call inside runAgent, but the existing OIDC tests only exercise
refreshOIDCToken/runOIDCRefresh directly and do not validate that runAgent performs the
one-shot refresh before starting the goroutine.

Rule 1062049: Require tests for new or modified Go logic
internal/cli/run.go[924-933]
internal/cli/run_test.go[1209-1300]

Agent prompt

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

## Issue description
`runAgent` now performs an immediate `refreshOIDCToken()` before launching the periodic refresh goroutine, but there is no test asserting this new behavior. Existing tests cover `refreshOIDCToken()` and `runOIDCRefresh()` in isolation, so a regression in `runAgent` wiring/order would not be caught.

## Issue Context
Compliance requires tests for new or modified Go logic. The PR adds new agent-start behavior affecting authentication correctness on the first inference call.

## Fix Focus Areas
- internal/cli/run.go[917-935]
- internal/cli/run_test.go[1187-1300]

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

---

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:10 PM UTC · Completed 6:23 PM UTC
Commit: 49edb64 · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/run.go	0.00%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Looks good to me

[ralphbean]

LGTM. One minor note on the existing context thread.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:10 PM UTC · Completed 7:17 PM UTC
Commit: 49edb64 · View workflow run →

[fullsend-ai-retro]

Retro: PR #2859 — refresh OIDC token before agent start

Timeline: Human-authored PR by waynesun09, created 17:58 UTC, merged 19:06 UTC (~68 min). Single commit, 2 files changed (+7/−2). Fixed a real production bug where stale OIDC tokens caused invalid_grant on first inference call.

What happened:

1. PR opened → first review run dispatched (28537453141)
2. Qodo bot posted review at 18:03, correctly identifying that the new synchronous refreshOIDCToken() call uses oidcCtx (derived from context.Background()), making it uncancellable by Ctrl-C
3. Second review run dispatched (28537898650) at 18:06, cancelling the first run via concurrency group after ~8 min of compute
4. Codecov reported 0% patch coverage at 18:11
5. Fullsend review agent approved at 18:23 with "Looks good to me" — did not flag the context propagation issue or missing test coverage
6. Human reviewer (ralphbean) noted the context issue at 18:43 as non-blocking, then approved at 18:44
7. PR merged at 19:06

Review quality gap: The fullsend review agent missed a real correctness issue that both Qodo and the human caught: the new synchronous blocking call should use the parent ctx (from cmd.Context()) rather than oidcCtx (from context.Background()) so that Ctrl-C works during startup. The human marked it non-blocking since the HTTP client has its own timeout, but it's still a genuine issue.

Filtered proposals (already covered by open issues):

• Missing test coverage detection → Review agent should detect missing test coverage for new code paths #2589
• Duplicate review dispatch / wasted compute → Debounce review dispatch on rapid synchronize events #1014 and Remove stale-head re-dispatch: redundant with concurrency group, causes self-triggering #2399

1 new proposal filed for the context propagation review gap.

Proposals filed

• Review agent should detect Go context propagation violations in new blocking calls (in fullsend-ai/fullsend)