Skip to content

Queue same-process credential writes so each gets its own lock window - #906

Merged
TheGreatAxios merged 4 commits into
mainfrom
cl-7318-createauthstore-has-a-temp-path-collision-and-a-lost-update
Sep 11, 2026
Merged

Queue same-process credential writes so each gets its own lock window#906
TheGreatAxios merged 4 commits into
mainfrom
cl-7318-createauthstore-has-a-temp-path-collision-and-a-lost-update

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Collaborator

Summary

  • Same-process auth-file writers queue per path so each call starts its own lock deadline instead of sharing one that began at invoke time
  • Temp write paths include a per-call counter so two saves in one process never collide on path.pid.tmp
  • Concurrent same-process profile saves and lock-window fairness are covered by tests alongside the existing cross-process suite

Verification

  • bun test ./src/auth/store.test.ts — 7 pass
  • Pre-commit typecheck and build pass on both commits
  • Matches the tmpWriteCounter + updateChains pattern already used in src/mcp/auth-store.ts

Fixes CL-7318

Same-process writers all polled the on-disk lock against a deadline
that started at call time, so one lock held past the timeout failed
the whole burst instead of just the first waiter. Writes now chain
per auth file, and temp paths get a per-call counter so two saves in
one process can never share one.
Replace the fixed 1.4s sleep with await-then-release so the lock-window
case cannot flake if the event loop slips past the foreign lock removal,
and add a same-process dual-profile save that pins the lost-update net
without another timed wait.
@linear-code

linear-code Bot commented Sep 11, 2026

Copy link
Copy Markdown

CL-7318

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critic · Comment

Ports MCP tmpWriteCounter + per-file updateChains into createAuthStore so same-process writers no longer share one lock deadline or one path.pid.tmp.

Findings

No blocking defects.

Nits

  • src/auth/store.test.ts:117queues same-process profile writes so neither save is lost stays green on main for N=2 (file lock alone serializes two saves inside LOCK_TIMEOUT_MS). It documents Done-when #3 but does not red without the queue. The lock-window test at :165 is the real red/green pin. Optional follow-up: a higher-concurrency same-process burst (e.g. 20+ profiles) would fail on main via cascaded timeouts and pass with the queue — reproduced locally: main kept 39/50 under a 50-way Promise.all, branch kept 50/50.
  • src/auth/store.ts:58updateChains never evicts settled entries (same as MCP). Fine for the one-path-per-provider production shape; only matters if a long-lived process injects many distinct home values.

CL-7318 Done-when

  1. Failing concurrency test then passes — met (gives queued same-process writes their own lock window; fails on main with both waiters timing out, passes on tip).
  2. Both fixes ported — met (tmpWriteCounter at store.ts:53/127, updateChains + enqueueAuthFileOp at store.ts:58/170-188).
  3. Two profiles concurrent save without loss — met (test at store.test.ts:117 + suite green).

bun test ./src/auth/store.test.ts — 7 pass.

Replace the dual-save case that stayed green without the queue with a
50-way same-process burst that needs fresh lock deadlines, and document
the queue plus defensive temp-counter in the store header.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critic · Approve

Nit-fix tip replaces the weak N=2 same-process save with a 50-way burst that pins shared-deadline loss, and clarifies queue / temp-counter / lock-window comments.

Findings

No blocking defects.

Notes

  • Burst pin is real: prior local check kept 39/50 on main without the queue; tip keeps 50/50 (~19ms). Lock-window test remains the precise deadline-freshness pin.
  • bun test ./src/auth/store.test.ts — 7 pass.

Ready to merge.

@TheGreatAxios
TheGreatAxios merged commit 9d53234 into main Sep 11, 2026
9 checks passed
@TheGreatAxios
TheGreatAxios deleted the cl-7318-createauthstore-has-a-temp-path-collision-and-a-lost-update branch September 11, 2026 05:00
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.

1 participant