spec: reconciler token refresh for stale OIDC bug

[bsquizz]

Summary

• Adds specs/platform/reconciler-token-refresh.spec.md — spec for fixing sessions stuck in Creating phase due to stale OIDC tokens
• Registers the new spec in specs/index.spec.md

Problem

execAfterReady receives a *sdkclient.Client with a static bearer token captured at provision time. The sandbox readiness polling loop can block for up to 600s (ndots retries, image pulls, scheduling delays). If the OIDC token expires during this wait, UpdateStatus(phase=Running) fails with HTTP 401 and the session is orphaned in Creating forever.

Observed on 2026-07-31: session dasfadsfdsa hit 3 ndots retries (~26s delay) which pushed the reconciler past the 5-minute token TTL. All subsequent API calls failed with 401.

Fix (specified, not yet implemented)

• Change execAfterReady to accept projectID instead of *sdkclient.Client
• Re-acquire the SDK client via r.factory.ForProject(ctx, projectID) before each API call
• OIDCTokenProvider.Token() already handles lazy refresh — just needs to be called at the right time

Related

• Skip ndots retry loop on ROSA/OpenShift — saves 36s per session boot #421 — ndots retry loop optimization (contributing factor, separate fix)

Test plan

• Spec review: requirements cover the three API call sites in execAfterReady (Running, Failed, Completed)
• Spec review: scenarios cover token-expired, token-valid, and token-refresh-failure cases
• Implementation PR will add unit test for stale-token scenario

🤖 Generated with Claude Code

[jsell-rh]

🤖 Amber Review — spec: reconciler token refresh for stale OIDC bug

This is a well-reasoned spec that correctly diagnoses the root cause (stale OIDC token embedded in a long-lived *sdkclient.Client inside execAfterReady). The proposed fix — calling r.factory.ForProject() before each SDK call rather than passing sdk as a parameter — is the right directional approach. Several spec gaps would lead to an incomplete implementation PR.

Overall: changes-requested — the implementation notes table is missing call sites, and two other issues warrant clarification before this spec guides implementation.

Findings (5)

---

🔴 Critical — Missing call site: sdk.Sessions().Get() at ~line 1082

File: specs/platform/reconciler-token-refresh.spec.md — Implementation Notes table (~line 143)

The table lists three call sites for the SDK client (lines 949, 783 via failSession, and "1059+" for completion marking), but omits the sdk.Sessions().Get(context.Background(), sessionID) call at approximately line 1082 that re-reads stop_on_run_finished after the entrypoint exec stream finishes. An implementer navigating only by this table will leave this call unpatched — producing a compile error once sdk is removed from scope, or a silently stale client if they wire the factory in the wrong spot.

Add this call site to the table.

---

🔴 Critical — Missing call site: sdk.Sessions().Get() at ~line 1057 (exec-retry loop)

File: specs/platform/reconciler-token-refresh.spec.md — Implementation Notes table (~line 143)

The exec-retry loop contains a sdk.Sessions().Get(checkCtx, sessionID) call inside the codes.NotFound branch that checks current phase before exiting the retry loop. This is a distinct call from the "completion marking" entry and is not listed in the table. Same consequence as above.

Add this call site to the table.

---

🟡 Major — failSession path: ForProject failure reproduces the original bug

File: specs/platform/reconciler-token-refresh.spec.md — Scenario: Token refresh fails (~line 124)

The "Token refresh fails" scenario says to "skip with a warning, not crash the reconciler." But failSession calls UpdateStatus(phase=Failed) — if ForProject() fails inside that closure, UpdateStatus is never invoked and the session stays stuck in Creating indefinitely. That's the exact same outcome as the original bug, just triggered by the fix path.

The spec needs to explicitly address what failSession does when ForProject() itself returns an error: retry? use a stale token as fallback? make an out-of-band K8s patch?

---

🟡 Major — Fix altitude: same bug class exists in other long-running functions

File: specs/platform/reconciler-token-refresh.spec.md — Requirement: SDK Client Refresh (~line 60)

Patching 4–5 call sites inside execAfterReady fixes one function. Every other long-running reconciler function that acquires an SDK client and then waits (e.g. deprovisionSessionSandbox at line ~1863, any future function) is subject to the same expiry window. The spec should acknowledge why per-call-site patching is preferred over a client-level fix (e.g. making sdkclient.Client accept a func() (string, error) token provider), or it will invite the same question in the implementation PR review.

---

🟠 Minor — Risk level is inverted in the completion UpdateStatus row

File: specs/platform/reconciler-token-refresh.spec.md — Implementation Notes table, Risk column (~line 147)

The Running-phase UpdateStatus at line 949 (fires after readiness wait) is marked "High" and the Completed-phase UpdateStatus at "1059+" (fires after readiness wait plus the full entrypoint execution — potentially many more minutes) is marked "Medium." Token staleness is a function of total elapsed time, so the completion-phase update is the highest-risk call and should carry the "High" label.

---

🟠 Minor — Incident timeline: token had only ~39s of validity at capture time

File: specs/platform/reconciler-token-refresh.spec.md — Incident timeline (~line 26)

The incident table shows the token acquired at 21:24:48 (expires 21:29:48) and ForProject() called at 21:29:09 — leaving only ~39 seconds of remaining validity at capture time. The scenario framing uses T₀ as both token-acquisition time and polling start, implying a full ~5-minute expiry window from exec start. The real failure mode is narrower: a nearly-expired token can produce a 401 after a very short (30–90 second) sandbox wait. Unit tests simulating 6-minute waits to trigger a 401 won't cover the actual edge case.

Either use the real timing in the scenario or add a "nearly-expired at capture" test variant.

---

— Amber, ACP codebase intelligence