Skip to content

coderouter: wait for sticky credential refreshes on a lease-completion signal - #15087

Merged
lawrencecchen merged 2 commits into
mainfrom
issue-11308-refresh-completion-signal
Sep 28, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
issue-11308-refresh-completion-signal

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Closes #11308

Summary

A sticky Codex session that hits a credential refresh already in flight used to sleep 500 ms and retry, up to 4 times. The delay had no relation to when the refresh finished: a refresh that finished in 50 ms still cost 500 ms, and each retry re-ran the full credential path (envelope read, decrypt, lease claim write). The session now waits on a refresh-completion signal from the lease layer and retries once the lease clears.

The lease lives in Postgres and the web app runs on Vercel serverless, so the refresh winner may be on this instance or on another one. The wait (web/services/coderouter/refreshSignal.ts) therefore races two sources:

  • This instance. completeRefreshLease, releaseRefreshLease, failRefreshLease, and the expired-lease sweep in repository.ts call refreshCompletionRegistry.settled(accountId) after they clear a lease. Local waiters wake with no delay and no extra query.
  • Another instance. A backoff loop re-reads the lease row (refreshLeaseActive: unexpired refresh_lease_id present) after 100, 200, 400, then 500 ms steps. The delays go through an injected RefreshWaitClock, which production backs with timers and tests replace with a fake. The request's AbortSignal cancels the sleep, the query, and the registry waiter.

The whole wait is capped at 2 s and 4 settle-then-busy-again cycles, which matches the old maximum. At the cap, the last CodeRouterRefreshBusy is rethrown and the proxy moves the session exactly as before. A failed lease re-read counts as "still held", so a database error can only end in the old move, never in a new error. Non-sticky requests still fail fast and never wait.

Why not LISTEN/NOTIFY. The app connects through a transaction-mode pooler (postgres(url, { prepare: false })), which does not keep LISTEN registrations across transactions. A dedicated direct listener connection per serverless instance would cost a database connection for each warm instance to save well under a second in a rare race. The trade-off of the chosen design: a refresh on another instance is detected up to one backoff step late (at most 500 ms), and each waiting sticky request issues at most about 6 cheap primary-key reads.

Claude plane. On current main, claudeProxy.ts uses static upstream secrets with no refresh lease and no sleep, so there is nothing to change there. The Claude refresh path with the same fixed delay lives only on the open #11283. createStickyRefreshPatience is plane-agnostic, and that PR should adopt the exported stickyRefreshPatience so both planes behave the same.

No setTimeout remains in codexProxy.ts or claudeProxy.ts for this path. The only timer is the production clock implementation in refreshSignal.ts.

Testing

Commit 1 (00874e3) changes the tests only and fails: bun test tests/coderouter-refresh-signal.test.ts tests/coderouter-responses-proxy.test.ts reports Cannot find module '../services/coderouter/refreshSignal'. Commit 2 (f956975) adds the implementation.

From web/ on f956975:

  • bun test tests/coderouter-*: 461 pass, 41 skip (DB-gated), 0 fail.
  • New sticky-patience tests run on the signal and a fake clock, with no wall time: an in-process settle retries at clock time 0 with no lease re-read; a cross-instance refresh is found by re-reads at 100 and 300 ms; a stuck lease moves the session at exactly 2000 ms; a caller abort during the wait rejects with AbortError with no credential retry, no re-read, and no pending sleep; a non-sticky request never waits.
  • CMUX_DB_TEST=1 bun test tests/coderouter-routing-db-behavior.test.ts against a throwaway local Postgres 17 (schema from drizzle-kit push): the 2 new lease tests pass (clearing a lease through release and fail wakes the registry and flips refreshLeaseActive; an expired lease reads as not active). 3 route-token VM binding tests in that file fail identically on unmodified origin/main, so they are unrelated to this change.
  • bun run typecheck: pass. bun run lint:complexity: pass, 42 findings matched the baseline, none new.

Not verified: behavior against the live PlanetScale pooler or on Vercel. No production or staging database was touched.

Changelog

none

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes #11308. Sticky Codex sessions that hit an in-flight credential refresh now wait on a refresh-completion signal from the lease layer instead of sleeping a fixed 500 ms and retrying up to 4 times.

Behavior

  • Local lease clears (complete, release, fail, expiry sweep) wake waiters immediately with no extra query; refreshes on another serverless instance are detected by re-reading the lease row at 100–500 ms backoff steps.
  • The wait stays capped at 2 s and 4 settle cycles; at the cap the session moves as before, and a failed lease re-read counts as held so the wait can only end with the previous busy error.
  • Request aborts cancel the sleep, the lease query, and the registry waiter; non-sticky requests still fail fast.
  • The wait uses an injected clock, so tests run deterministically; the only real timer is the production clock in refreshSignal.ts.
  • claudeProxy.ts is unchanged — stickyRefreshPatience is an exported seam that the Claude refresh path should adopt so both planes behave the same.

Written for commit f956975. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Sticky sessions now wait for an in-progress credential refresh to finish before retrying, including refreshes completed by another instance.
    • Waiting stops when the refresh completes, the patience limit is reached, or the request is cancelled. Non-sticky requests continue without waiting for refresh completion.
  • Tests
    • Added coverage for refresh waiting, cancellation, lease changes, and timeout behavior.

lawrencecchen and others added 2 commits September 27, 2026 17:55
…l and fake clock

Sticky sessions that hit an in-flight credential refresh must wake on the
lease layer's refresh-completion signal, fall back to bounded lease re-reads
through an injected clock when another instance holds the lease, give up at
the patience deadline, and stop at once when the caller aborts.

Refs #11308

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replace the codex plane's fixed 4 x 500 ms sleep-and-retry with a lease-layer
wait. The repository wakes in-process waiters whenever it clears a refresh
lease (complete, release, fail, expiry sweep). Refreshes held by another
serverless instance are detected by re-reading the lease row with a 100 ms
to 500 ms backoff through an injected, cancellable clock. The wait stays
bounded at 2 s and 4 settle cycles, and the request's AbortSignal cancels it.
Non-sticky requests still fail fast.

Closes #11308

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b0be7b72-30b6-4d89-a99e-c95661b7d674

📥 Commits

Reviewing files that changed from the base of the PR and between 847c919 and f956975.

📒 Files selected for processing (8)
  • web/services/coderouter/codexProxy.ts
  • web/services/coderouter/refresh.ts
  • web/services/coderouter/refreshSignal.ts
  • web/services/coderouter/repository.ts
  • web/tests/coderouter-refresh-signal.test.ts
  • web/tests/coderouter-responses-proxy.test.ts
  • web/tests/coderouter-routing-db-behavior.test.ts
  • web/tests/refresh-wait-clock-fixture.ts
 ________________________________________________________________
< Just keep coding, just keep coding... I'll handle the reviews. >
 ----------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lawrencecchen
lawrencecchen merged commit 2604935 into main Sep 28, 2026
66 of 68 checks passed
@lawrencecchen
lawrencecchen deleted the issue-11308-refresh-completion-signal branch September 28, 2026 01:06
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for f9569751ae: every check was green at merge (19 verified; 16 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 28, 2026
2604935 coderouter: wait for sticky credential refreshes on a lease-completion signal (manaflow-ai#15087)
369cd16 coderouter: scope org API keys to team-shared accounts (manaflow-ai#15086)
847c919 Keep non-ASCII startup input as UTF-8 (manaflow-ai#15081)
a616a2b Remove obsolete bash PR watcher loops (manaflow-ai#15075)

# Conflicts:
#	.github/workflows/build-ghosttykit.yml
#	.github/workflows/ci-guards.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

coderouter planes: replace fixed-delay sticky-refresh retry with a lease-layer refresh-completion signal

1 participant