Skip to content

fix: persist rotated refresh tokens, close silent-empty auth gaps - #79

Open
ryantlee25-droid wants to merge 1 commit into
mainfrom
fix/persist-rotated-refresh-token
Open

fix: persist rotated refresh tokens, close silent-empty auth gaps#79
ryantlee25-droid wants to merge 1 commit into
mainfrom
fix/persist-rotated-refresh-token

Conversation

@ryantlee25-droid

Copy link
Copy Markdown
Owner

Summary

Rotated Supabase refresh tokens were only ever kept in memory by the long-lived MCP server, so the server was silently spending the CLI's on-disk refresh token (~/.config/tages/auth.json, shared by both) on its 30s auto-refresh tick without ever writing the rotation back. The on-disk credential eventually hit refresh_token_already_used and stayed dead. Confirmed empirically: auth.json's mtime was still the original login while a direct token-endpoint call returned refresh_token_already_used. rlee@mersive.com's session died about an hour after login and stayed dead for four days; both pending phoenix teammates would have hit the same wall within an hour of joining.

  • packages/shared/src/auth-store.ts (new): the single auth.json reader/writer, moved out of the CLI so the server uses the same code path. Atomic writes (temp file + rename), unconditional 0600, cleans up the temp file on any write failure.
  • packages/shared/src/auth-persist.ts (new): persistSessionOnRefresh registers an onAuthStateChange listener that writes TOKEN_REFRESHED sessions back to disk. persistRotatedTokens refuses to write when the token already on disk has a later expiry than the incoming one, so a stale long-lived process can't clobber a fresh tages login run elsewhere. Registered before setSession() at every long-lived call site — server index.ts/config.ts, CLI auth/session.ts, both backfill-*.ts scripts — because an expired access token makes setSession refresh immediately, and that first rotation is the one that was spending the disk token.
  • init.ts, link.ts, migrate.ts had their own bare writeFileSync on auth.json; now route through the shared writer. link is the command a teammate runs to join — a truncate-then-write racing the server's rename could drop a freshly minted OAuth token into an orphaned inode while still reporting success.
  • New requireLiveSession guard on team (all 4 subcommands), status, and onboard. On a dead session these previously fell back to an anonymous client, read zero rows through RLS, and exited 0 with "No team members" / "Memories: 0" / an empty briefing — tages team list reported no members for a project with two pending invites. The guard covers anonymous sessions as well as expired (tages logout && tages team list reproduces it in one step).
  • packages/shared/tsconfig.build.json (new) keeps tests out of the published dist while tsconfig.json keeps them under tsc --noEmit; shared also gained @types/node (TS 6.0.2 stopped auto-discovering them).
  • Versions: @tages/shared 0.2.3→0.2.4, @tages/server 0.3.4→0.3.5, @tages/cli 0.5.5→0.5.6.
  • Docs: docs/quickstart.md and docs/team-onboarding.md version numbers refreshed, a stale claim about MCP remember silently reporting success corrected, and a new trap section documenting this bug.

Not fixed

  • persistRotatedTokens does read-compare-write with no lock. Two processes refreshing at the same instant can still lose one rotation — strictly better than always losing it, not zero. An O_EXCL lockfile would close it.
  • A hard kill between write and rename can leave an auth.json.tmp.<pid> holding a live refresh token at 0600 inside a 0700 directory. Not a disclosure, but nothing reaps it.
  • No fsync before rename: atomic against concurrent readers, not against power loss.
  • 29 other CLI commands still use createAuthenticatedClient and keep the same empty-vs-expired ambiguity this PR closed for team/status/onboard. recall-context, pending, brief, and query are next.

Test plan

  • pnpm build — 0 errors
  • pnpm typecheck — 0 errors
  • pnpm -r test — 1554 passing
  • pnpm install --frozen-lockfile — clean
  • 12 new tests, each mutation-tested (verified to fail when the behavior under test was removed, then restored): staleness guard, TOKEN_REFRESHED event filter, team list guard, anonymous branch, missing-token guard, listener write-failure catch, readAuthFile null returns, temp-file cleanup on failure

Note: the 6 pre-existing eslint errors in packages/server/src/index.ts and packages/cli/src/__tests__/commands-smoke.test.ts are outside these hunks and unrelated to this change; lint is not a required CI check.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_012j48bNYeNWSu4dorw5wG6V

Supabase rotates the refresh token on every refresh and invalidates the
previous one immediately. createSupabaseClient used client defaults
(autoRefreshToken: true) with Node's in-memory session store, so the
long-lived MCP server kept spending the CLI's on-disk refresh token on
its 30s auto-refresh tick without ever writing the rotation back to the
~/.config/tages/auth.json the CLI and server share. The on-disk
credential eventually hit refresh_token_already_used and stayed dead.
rlee@mersive.com's session died about an hour after login and stayed
dead for four days; both pending phoenix teammates would have hit the
same wall within an hour of joining.

- packages/shared/src/auth-store.ts (new): single auth.json
  reader/writer shared by CLI and server. Atomic writes (temp file +
  rename), unconditional 0600, cleans up the temp file on failure.
- packages/shared/src/auth-persist.ts (new): persistSessionOnRefresh
  listens for TOKEN_REFRESHED and writes it to disk;
  persistRotatedTokens refuses to overwrite a disk token with a later
  expiry than the incoming one, so a stale background process can't
  clobber a fresh `tages login` from another terminal. Registered
  before setSession() at every long-lived call site (server
  index.ts/config.ts, CLI auth/session.ts, both backfill-*.ts
  scripts), since an expired access token makes setSession refresh
  immediately and that first rotation was the one spending the disk
  token.
- init.ts, link.ts, migrate.ts now route their auth.json writes
  through the shared writer instead of a bare writeFileSync. link is
  the command a teammate runs to join a project; the old
  truncate-then-write could race the server's rename and drop a
  freshly minted OAuth token into an orphaned inode while still
  reporting success.
- New requireLiveSession guard on team (all 4 subcommands), status,
  and onboard. A dead session previously fell back silently to an
  anonymous client, read zero rows through RLS, and exited 0 with
  "No team members" / "Memories: 0" / an empty briefing. Covers
  anonymous as well as expired (`tages logout && tages team list`
  reproduces it in one step).
- Versions: @tages/shared 0.2.3->0.2.4, @tages/server 0.3.4->0.3.5,
  @tages/cli 0.5.5->0.5.6. Docs refreshed to match, plus a new trap
  section on this bug.

Not fixed, noted in the PR: persistRotatedTokens has no lock, so two
simultaneous refreshes can still lose one rotation; no fsync before
rename; 29 other CLI commands still share the old empty-vs-expired
ambiguity (recall-context, pending, brief, query next).

Tests: +12, each mutation-tested against its own removal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012j48bNYeNWSu4dorw5wG6V
@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
dashboard Ready Ready Preview Aug 30, 2026 4:19pm

Request Review

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