Skip to content

perf(auth): cut round trips in native refresh-token rotation - #8010

Merged
iscekic merged 1 commit into
mainfrom
drain/auth-refresh-rotation-single-lookup
Oct 21, 2026
Merged

iscekic merged 1 commit into
mainfrom
drain/auth-refresh-rotation-single-lookup

Conversation

@iscekic

@iscekic iscekic commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

bot: In apps/web/src/lib/auth/device-sessions.ts (rotateRefreshToken), the three sequential pre-transaction selects — the refresh-token row, the parent device session, and the user's blocked status — are now one joined query that returns the token row, the full session, and the full user. The inner joins keep every refusal outcome and error code exactly as before: a token whose session or user no longer exists refuses the same way as an unknown token, and revoked-session and blocked-user refusals are unchanged.

Token generation inside the transaction now uses the pre-read user row, so the user row is fetched once per rotation instead of twice. The in-transaction FOR UPDATE session lock, the token re-read under it, and the consume and replacement issuance are unchanged.

Happy-path database statements drop from eight to six: one joined lookup, the session lock re-read, the token re-read, the consume, the last_seen_at update, and the replacement-token insert. The unconditional last_seen_at update is kept on purpose so the mobile device list keeps showing a fresh last-seen value on every rotation.

End-to-end verification

  • Existing suite: pnpm --filter web run test -- --testPathPattern device-sessions — 16/16 pass (happy path, unknown token, replay, lost rotation, retired pair, stale request, lock-order recovery, grace expiry, revoked session, concurrent revocation, blocked user, unblock retry, 30-day expiry, issuance rollback).
  • pnpm --filter web run typecheck and pnpm --filter web run lint pass.
  • Live route check on this VM's dev stack: created a device session and refresh token in Postgres, called POST /api/auth/native/refresh, got 200 with a new one-hour pair, confirmed the old token consumed and the new one live, rotated the new token again — 200. Postgres statement logging during one rotation shows exactly six auth statements, the first being the single joined select with both inner joins.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: The finding offers an optional optimization: update last_seen_at only when the stored value is older than a short threshold, so ordinary rotations need only the consume and insert writes. The mobile device-list screen displays last_seen_at as 'Last seen', so skipping fresh writes would stale that display for clients that rotate often. Which way should the PR go?
    Answer: Keep unconditional write (Recommended)
    Confidence: 0.65 · Precedent: Speed and least work: 'do the least amount of work possible, with no drop in quality' (session 7f3470e0ba6f). The step is optional, a stale 'Last seen' would lower quality, and with roughly hourly rotations a short threshold would rarely skip a write.

@iscekic iscekic added drain Work item from the findings drain janitor:performance-reliability Janitor performance-reliability findings labels Oct 19, 2026
@kilo-code-bot

kilo-code-bot Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Reviewed the single joined pre-read in rotateRefreshToken (apps/web/src/lib/auth/device-sessions.ts) that replaces the three sequential selects for the refresh-token row, parent session, and user.

Key checks:

  • Row filtering is equivalent. device_refresh_tokens.device_session_id references device_sessions.id and device_sessions.kilo_user_id references kilocode_users.id, both ON DELETE CASCADE and NOT NULL, so every existing token row has a session and user. The inner joins therefore cannot drop a row the old code would have found, and the refusal/error-code mapping (INVALID_REFRESH_TOKEN for unknown/expired token, SESSION_REVOKED, USER_BLOCKED, reuse revocation) is preserved.
  • Token generation. generateApiToken destructures { id, api_token_pepper } from User; the joined user is a full kilocode_users row, so the contract still holds. The pre-transaction read of the pepper widens the existing (pre-existing) pepper-rotation race only marginally and matches the pattern used elsewhere.
  • Locking / concurrency. The FOR UPDATE session lock still targets session.id, and the in-transaction token re-read, consume, last_seen_at update, and replacement insert are unchanged.
  • No new timers, listeners, caches, or global state — no memory-leak risk.
Files Reviewed (1 file)
  • apps/web/src/lib/auth/device-sessions.ts - 0 issues

Reviewed by deepseek-v4.1-flash · Input: 48.9K · Output: 11.9K · Cached: 435.8K

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added the merge:human Substantive functional risk requires human review. label Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain janitor:performance-reliability Janitor performance-reliability findings merge:human Substantive functional risk requires human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants