Windows: concurrent OAuth refresh races — loser's revoked tombstone overwrites winner's rotated credentials, forcing spurious re-login

Version

main @ 9a715820c (verified against current source)

Platform

Microsoft Windows NT 10.0.26200.0 x64 (Windows-only by construction)

What issue are you seeing?

On Windows, when two kimi-code processes refresh the same OAuth credential concurrently, the process whose refresh request loses the server-side rotation race can overwrite the winner's freshly-saved credentials with an empty revoked tombstone. The next token read anywhere then throws Stored token for "kimi-code" was rejected; re-login required and the user is forced to log in again, even though a valid rotated refresh token was successfully issued by the server seconds earlier.

Root cause

Two facts compose into the bug:

1. The cross-process refresh mutex is unconditionally disabled on Windows. packages/oauth/src/oauth-manager.ts:183:

   private resolveLockTarget(): string | undefined {
     if (process.platform === 'win32') return undefined;
   

   So two processes enter the refresh critical section at the same time.

2. The stale-token recovery path saves the tombstone unconditionally. packages/oauth/src/oauth-manager.ts:375-389: on a 401 the loser sleeps 100 ms, re-reads the file, and if the file still shows the old refresh token it calls storage.save(name, revokedTombstone(activeToken)) — with no second check and no coordination with a save that may be in flight in the peer process.

Race window timeline (winner = process whose request reaches the server first):

• t=0: both processes load rt1, both call the refresh endpoint.
• t=30ms: server rotates rt1→rt2, returns to winner; winner starts its (slow: disk/AV jitter) FileTokenStorage.save.
• t=50ms: loser's request reaches the server with now-dead rt1 → 401 invalid_grant.
• t=150ms: loser finishes its 100 ms recovery sleep, re-reads the file — winner's save hasn't landed yet, file still shows rt1 → loser concludes "no peer rotated" and starts its own tombstone save.
• t=200ms: winner's rt2 save lands.
• t=300ms: loser's tombstone save lands last → valid credentials are gone.

On Windows, antivirus real-time scanning routinely adds 50–300 ms to the tmp-write/fsync/rename sequence in FileTokenStorage.save, so this window is hit in practice whenever two CLI instances refresh around the same time (e.g. a TUI and a kimi -p background job, or two open terminals).

Steps to reproduce

Deterministic repro against current source (models the server rotation and per-process save latency; ran on Windows, win32 lock path active):

import { mkdtempSync, readFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { OAuthManager } from '@moonshot-ai/kimi-code-oauth/src/oauth-manager.ts';
import { FileTokenStorage } from '@moonshot-ai/kimi-code-oauth/src/storage.ts';
import { OAuthUnauthorizedError } from '@moonshot-ai/kimi-code-oauth/src/errors.ts';

const server = { currentRefreshToken: 'rt1', grantCount: 0 };
const dir = mkdtempSync(join(tmpdir(), 'oauth-repro-'));
const fileStorage = new FileTokenStorage(dir);
await fileStorage.save('kimi-code', {
  accessToken: 'at1', refreshToken: 'rt1',
  expiresAt: Math.floor(Date.now() / 1000) + 30, expiresIn: 3600,
  scope: '', tokenType: 'Bearer',
});
const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));

// Per-process save latency (AV/disk jitter). Loads stay fast.
function wrapStorage(d: number) {
  return {
    load: (n: string) => fileStorage.load(n),
    save: async (n: string, t: never) => { await sleep(d); return fileStorage.save(n, t); },
    remove: (n: string) => fileStorage.remove(n),
    list: () => fileStorage.list(),
  };
}

function makeManager(name: string, storage: ReturnType<typeof wrapStorage>) {
  return new OAuthManager({
    config: { name: 'kimi-code', oauthHost: 'https://example.test', clientId: 'repro' },
    storage, configDir: dir, sleep,
    refreshTokenImpl: async (_c: unknown, rt: string) => {
      if (name === 'winner') {
        await sleep(30); // reaches server first: rotate rt1 -> rt2
        server.grantCount += 1;
        server.currentRefreshToken = 'rt2';
        return { accessToken: 'at-winner', refreshToken: 'rt2',
          expiresAt: Math.floor(Date.now()/1000)+3600, expiresIn: 3600, scope: '', tokenType: 'Bearer' };
      }
      await sleep(50); // arrives after rotation: rejected
      throw new OAuthUnauthorizedError('invalid_grant: refresh token was rotated');
    },
    requestDeviceImpl: async () => { throw new Error('unused'); },
    pollDeviceImpl: async () => { throw new Error('unused'); },
  });
}

const winner = makeManager('winner', wrapStorage(170));
const loser = makeManager('loser', wrapStorage(150));
await Promise.allSettled([
  winner.ensureFresh({ force: true }),
  loser.ensureFresh({ force: true }),
]);

const onDisk = JSON.parse(readFileSync(join(dir, 'kimi-code.json'), 'utf8'));
console.log(onDisk);
// => { access_token: '', refresh_token: '', expires_at: 0, ... }  (tombstone)
// server.grantCount === 1  (a perfectly good rotated token was issued!)
await makeManager('postmortem', wrapStorage(0)).ensureFresh();
// => throws: Stored token for "kimi-code" was rejected; re-login required.

Output on Windows 11, Node 24:

winner: fulfilled
loser : rejected
server grants: 1
FINAL ON-DISK CREDENTIAL: { "access_token": "", "refresh_token": "", ... }
postmortem ensureFresh throws: Stored token for "kimi-code" was rejected; re-login required.

Note the existing cross-process test packages/oauth/test/oauth-manager-multi-process.test.ts is skipIf(win32) precisely because the lock is disabled on Windows, so the race has no test coverage there.

Expected behavior

A lost rotation race must never destroy valid on-disk credentials. Concretely:

1. Enable a cross-process refresh lock on Windows (proper-lockfile works on win32; the existing test's skip comment cites only path quirks), or
2. Make the tombstone save conditional: re-check inside a small critical section that the on-disk refresh token still equals the one we failed with (compare-and-save), so a peer's rt2 is never overwritten, and/or retry the recovery read a few times instead of a single 100 ms sleep.

Additional information

• FileTokenStorage.save itself is tmp+fsync+rename — fine; the problem is purely the unconditional overwrite decision in the recovery path.

• The same unconditional tombstone path exists on POSIX too, but there the proper-lockfile mutex serializes the whole refresh section, so the only remaining window is a peer crash mid-refresh — much narrower.

• I am willing to submit a PR for this bug fix myself (please wait for maintainer approval in this issue first)

Start in packages/oauth/src/oauth-manager.ts, especially resolveLockTarget and the 401 recovery path around lines 375-389, then inspect FileTokenStorage.save in packages/oauth/src/storage.ts. Run the supplied Windows reproduction and review packages/oauth/test/oauth-manager-multi-process.test.ts, which currently skips win32. Done means concurrent Windows refreshes cannot replace valid rotated credentials with a revoked tombstone and the regression has test coverage.