[josephschorr]

Summary

When the operator restarted, running sessions lost access to their memory until the operator got around to each one individually. Sessions were recovered one at a time, while a fixed one-minute window counted down from the moment the process started; any running session not reached before that window ran out was failed permanently, even though nothing was actually wrong with it.

Now every running session, and every running workshop's sidecar, is recovered in a single pass as soon as the operator is ready, before it starts doing any other session work. Until that pass completes, unrecognized credentials are told to retry rather than refused; if part of the pass fails, it is retried rather than finished without the missing credentials. Once it completes, an unrecognized credential is refused exactly as before.

Clients still retry within their existing retry budget, so a request that arrives very early in a slow restart can still time out, as before — but the pass now finishes far sooner than sessions used to be reached one by one.

Test plan

• Unit tests for the recovery pass: running sessions and ready workshops recovered with the same access they had; deleting ones and ones without stored credentials skipped; a failed read reported and retried without blocking the rest
• Unit tests that recovery is only declared complete once every part succeeds, and never when cut short by shutdown
• Unit tests for the retry-versus-refuse behavior before, during, and after recovery, for both affected services
• Unit, integration and e2e suites

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
openagentprimitives	Ignored	Preview	Oct 30, 2026 11:08pm UTC

[samkim]

Review notes (code review plus a data-security pass, each finding re-checked against the head commit and against the PR description).

Should fix before merge

1. Workshop sidecar bearers lose the removed 60s grace and get a terminal 401 after a restart (pkg/controllers/agentsession/tokenrestore.go, pkg/memory/httpsrv/httpsrv.go)
The old cold-start grace also covered Workshop sidecar tokens. Those are only re-registered by the Workshop reconciler (pkg/controllers/workshop/controller.go:496-497), which is leader-only, and the new restore pass doesn't include them. The pass ends the restoring state about a second after cache sync, but with --leader-elect and no LeaderElectionReleaseOnCancel, the new pod can wait up to the lease duration before the Workshop reconciler runs. In that window the sidecar (internal/cmd/workshop/main.go hitting /memory/...) gets a 401 instead of the retryable 503 it got before. This contradicts "sessions are told to retry rather than refused" and "refused exactly as before"; the description doesn't mention Workshops.
Suggested fix: restore Workshop tokens in the same pass (list Workshops, read their bearer Secrets), or keep the registry restoring until they are registered too.

2. A failed Secret read during the pass is swallowed and the restore still finishes (tokenrestore.go:117-126, memorytoken.go:74-85)
One transient Get failure is logged, FinishRestore runs anyway, and that session gets terminal 401s until its next reconcile. That contradicts "every running session is recovered". Suggested fix: collect failures and retry them before FinishRestore, or have reregisterMemoryToken return an error the runnable retries on.

3. "However long the pass takes" overstates the client side. The 503 is only retried within the memory httpclient's ~17s budget (pkg/memory/httpclient/httpclient.go:411-420, 8 attempts), so a request that arrives early and outlasts cache sync plus the pass still fails. Not a regression (the old code had the same limit and this PR shortens time-to-restore), but either reword the description or have the client honor Retry-After and keep retrying on this specific 503.

Minor / follow-up

• Multi-replica (latent): the restore runs on every replica (NeedLeaderElection() false), but Revoke only runs in the leader's finalize. A standby now holds tokens for sessions that are later deleted and keeps accepting them until it restarts; before this PR its registry was empty. The default install is 1 replica with Recreate, so this isn't reachable today. If multi-replica serving is intended, serve the memory API only on the leader or propagate revocations.
• If ctx is cancelled mid-pass, every Get fails, the errgroup returns nil, and FinishRestore runs on a partial registry. Check ctx.Err() first.
• The sessions count in "memory token registry restored" counts sessions examined, not tokens registered.
• The "unknown bearer while restoring → 503" branch is duplicated in httpsrv and goals; a shared helper would keep them in step.

Checked and clean

No revoked token is resurrected (deleting sessions are skipped, and Reconcile blocks on WaitRestored, so no finalize runs between the List and the Set). No wrong-session binding (reuses reregisterMemoryToken, never mints). While restoring, an unknown bearer gets 503, never access. No token material in logs, no RBAC change.

[josephschorr]

Thanks — addressed in 6a417e4:

1. Workshop sidecar bearers are now restored in the same startup pass (the restore is a shared runnable with one restorer per owner), and the Workshop reconciler waits on it like AgentSession does.
2. A failed Secret read is now returned instead of swallowed; the pass is retried and the registry stays restoring until every token is back.
3. Reworded the description rather than changing the client — an unbounded retry on this 503 could hang runners if a restore never finishes.

Minor: cancellation mid-pass no longer finishes the restore, the log counts tokens registered, and the 503 branch is a shared tokens.RetryWhileRestoring. Multi-replica revocation left as a follow-up since it isn't reachable with the single-replica install.

[samkim]

The fixes in 6a417e4 cover everything from the first review. A re-review turned up one new blocker and a few related problems, so I pushed two commits on top: b2767e0 and c9e753f.

Blocker: one unadopted Secret crashed the operator on every boot (b2767e0)

The restore reads each session's memory-token Secret through the adoptguard reader. With the default --secret-guard-mode=panic, that reader panics when the Secret exists but lacks the adopted label (adoptguard/guard.go:69). Inside Reconcile, controller-runtime recovers the panic and only that session fails. The restore runs that same read on errgroup goroutines under a manager Runnable, where nothing recovers it. So a single unadopted <session>-memory-token Secret crash-looped the operator before any reconcile ran. That Secret could have been pre-created under that name, or had its label removed.

• Each restore goroutine (agentsession.restoreOne) and each restorer call (tokenrestore.restore) now recovers the panic.
• A panic is logged at Error and that session is skipped, not returned as an error. The guard panics on the same Secret every time, so retrying would only hold every AgentSession and Workshop Reconcile behind WaitRestored. A skipped session is handled by its own Reconcile, as it was on main.
• A regression test uses the real guard in panic mode. With the recovery removed, it crashes the test binary with the same adoptguard panic.

Retries that block reconciles too long (b2767e0, c9e753f)

Every Reconcile waits on the restore, so a pass that never succeeds needs a bound:

• Give-up deadline: after 5 minutes (defaultGiveUpAfter), the registry is declared restored and the failure is logged at Error. Each owner's Reconcile still re-registers any bearer the pass missed. Change the value if you want a different bound.
• Each pass is bounded too: the deadline was only checked after a pass returned, so a Secret GET hung on a stalled apiserver connection kept the pass from returning and reconciles waited forever. Each pass now runs under a context bounded by what's left of the give-up budget.
• A Forbidden read is skipped like a panic. It won't clear on retry, so it shouldn't hold reconciles for the whole window. Both the AgentSession and Workshop restorers do this.

Token lookup race in the memory and goals APIs (b2767e0)

httpsrv and goals checked Restoring() only after a token lookup missed. If the restore registered that token and finished between those two steps, a now-valid token got a permanent 401, and the session failed with MemoryUnavailable. The goals comment said its single snapshot prevented this, but Restoring() was a separate read.

Both now read Restoring() before the lookup. tokens.RetryWhileRestoring takes that snapshot as a bool, and Registry.Restoring() is nil-safe. There's no dedicated test for this timing, because forcing it would need a test-only hook in the handler. The existing 503/401 tests pass.

Tests

• go test -race passes twice in a row on controllers/{tokenrestore,agentsession,workshop}, memory/{tokens,httpsrv}, web/goals and internal/cmd/operator.
• go vet is clean on those packages with no tags, with -tags=integration and with -tags=e2e.
• -tags=integration (envtest 1.30.3) passes on the six pkg/ packages.
• I haven't run the full mage test:integration / mage test:e2e locally; CI should cover them now.

Follow-ups I left for you (not blocking)

• Workshop routes still return 401 during the restore. workshopthreadsrv, workshoptranscriptsrv, workshopdraftsrv, workshopprojectsrv and secretoutsrv still answer an unknown bearer with a plain 401 while restoring. That was true on main too, but it means Workshop bearers are restored while the routes sidecars actually call still refuse them in that window. Putting the "retry while restoring" check inside the shared registry lookup, rather than in each handler, would cover them all. Otherwise, scope the description's "unrecognized credentials are told to retry" to /memory/ and goals. This came up in all three reviews.
• Multi-replica. A non-leader replica restores once and never sees later Revokes, which are leader-only. It's latent at replicas: 1, but the Runnable comment ("every replica needs it restored") reads as if multi-replica already works.
• Reconcile workers. AgentSession and Workshop Reconcile hold their worker while blocked in WaitRestored. Requeuing with a delay while Restoring() would free the workers.
• Workshop restore duplicates step 8. It re-implements step 8's key, Secret name and Set (controller.go:503-506), and the two already differ: step 8 registers an empty token, the restore skips it. A shared helper, like reregisterMemoryToken on the AgentSession side, would keep them in sync.
• The Workshop restore reads Secrets one at a time, while the AgentSession restore reads 16 at once. On a large install this lengthens the restoring window.
• Pass cost on large installs. The AgentSession pass GETs one Secret per AgentSession ever created, terminal ones included, and every Reconcile waits for the whole pass.
• No test that AgentSession Reconcile waits on WaitRestored. Workshop has TestReconcile_WaitsForTokenRestore.