fix(auth): refresh under the lock, finish a started refresh, retry a replaced token [ENG-5309]

docs/decisions.md

@@ -1462,3 +1462,32 @@ behind it, and what a reviewer should check. IDs are stable; append, do not renu
   `account` from the API's userinfo. It is shown, never relied on: a site that
   does not grant the scope, a pre-0.8 login and an API key report `null`, and
   the scope is not part of the granted-scope check.
+
+## D-089 One process's refresh does not break the others
+
+- The API ends the previous access token the moment a refresh issues a new
+  one. With the 30-minute refresh window (D-088), a refresh in one process (a
+  terminal, another agent, the DeepSeek Harness host) broke every other holder
+  of the old token: a running `wait`, or a host-provided credential, until the
+  next read.
+- The refresh now runs entirely under the credentials lock, as lark-cli does:
+  under the lock the CLI re-reads the profile and adopts a token another
+  process already refreshed, so parallel commands spend the refresh token once.
+  The B3 adoption after a failed refresh stays for CLIs before 0.8.1, which
+  refresh outside the lock.
+- From the refresh request to the save, forced exits (a second Ctrl-C, the end
+  of SIGTERM / SIGHUP's grace period) wait (`withExitHeld`). A process killed
+  after the API answered but before the save would leave only a spent refresh
+  token, and the next refresh would end the login.
+- After a 401 with an OAuth token, from the store or from a host, the transport
+  asks for the token stored now for the same login (profile login id; for a
+  host credential the profile whose user id or login id is `DSH_MESHY_SUBJECT`)
+  and retries once when it differs and has not expired. A 401 is answered
+  before the request is acted on, so the retry cannot repeat a create. Bodies
+  that cannot be read twice (none today) are not retried. Another account's
+  token is never used.
+- The Windows key store (DPAPI through Windows PowerShell) waits up to 30 s
+  instead of 5 s. PowerShell took 3–5 s to start on CI runners, so a cold or
+  busy machine could fall back to the key file when sealing, or fail to open a
+  stored login. DPAPI never prompts; the Keychain and the Secret Service keep
+  5 s, since a locked keychain may.

docs/usage.md

@@ -85,6 +85,15 @@ spend the stored login. It uses a still-valid token as it is, and for an expired
 one exits `3` saying where to refresh it; the stored login stays intact.
 `auth login` checks the same thing before it opens the browser.
 
+The refresh runs under the credentials lock, so commands started together
+refresh once and the rest pick up the new token. Once a refresh request is
+sent it is always saved, even if a second Ctrl-C or a terminate signal arrives
+meanwhile; the exit follows the save. The API ends the previous access token as
+soon as it issues a new one, so a command whose token another process replaced
+gets a 401: it then retries once with the token now stored for the same login
+(the same profile, or for `DSH_MESHY_CREDENTIAL` the login named by
+`DSH_MESHY_SUBJECT`), and never with another account's.
+
 **Who is signed in:** `auth status` and `auth list` show the `account` (name and
 email) of a browser login. It is reported by the site at sign-in, so a login
 made before 0.8, an API key, and the China site (which does not report it yet)

src/client/index.ts

@@ -19,7 +19,7 @@ import {
   type TaskResourceDescriptor,
 } from "./resource-registry.js";
 import { createTransport, type Transport } from "./transport.js";
-import { assertCredentialAllowedForOrigin, type MeshyConfig } from "../internal/config.js";
+import { assertCredentialAllowedForOrigin, storedTokenAfterRejection, type MeshyConfig } from "../internal/config.js";
 import { UsageError } from "../internal/errors.js";
 
 
@@ -35,7 +35,12 @@ export class MeshyClient {
 
   constructor(config: MeshyConfig, opts: { fetchImpl?: typeof fetch } = {}) {
     this.config = config;
-    const common = { readTimeoutMs: config.readTimeoutMs, fetchImpl: opts.fetchImpl, credentialKind: config.credentialKind };
+    const common = {
+      readTimeoutMs: config.readTimeoutMs,
+      fetchImpl: opts.fetchImpl,
+      credentialKind: config.credentialKind,
+      renewCredential: (rejected: string) => storedTokenAfterRejection(config, rejected),
+    };
     this.v1 = createTransport({ ...common, baseUrl: config.baseUrlV1, apiKey: config.apiKey });
     this.v2 = createTransport({ ...common, baseUrl: config.baseUrlV2, apiKey: config.apiKey });
     this.balance = new BalanceEndpoint(this.v1);
@@ -67,6 +72,7 @@ export class MeshyClient {
           apiKey: this.config.apiKey,
           credentialKind: this.config.credentialKind,
           readTimeoutMs: this.config.readTimeoutMs,
+          renewCredential: (rejected) => storedTokenAfterRejection(this.config, rejected),
         });
         return this.creativeLabTransport;
       }

src/client/transport.ts

@@ -85,6 +85,11 @@ export interface TransportConfig {
   baseUrl: string;
   apiKey?: string;
   credentialKind?: "oauth" | "api_key";
+  /**
+   * Asked once after a 401: the credential to retry with, or null. Another
+   * process may have refreshed the login while this one held the old token.
+   */
+  renewCredential?: (rejected: string) => string | null;
   readTimeoutMs: number;
   fetchImpl?: typeof fetch;
   userAgent?: string;
@@ -166,10 +171,26 @@ export function createTransport(cfg: TransportConfig): Transport {
   const authenticated = Boolean(cfg.apiKey);
   const userAgent = cfg.userAgent ?? USER_AGENT;
   const maxBody = cfg.defaultMaxBodyBytes ?? DEFAULT_MAX_JSON_BODY_BYTES;
+  let apiKey = cfg.apiKey;
+
+  /**
+   * On a 401, switch to the credential renewCredential offers and report
+   * whether the request should be sent again. A 401 is answered before the
+   * request is acted on, so sending it again cannot repeat a side effect.
+   */
+  function renewAfter(resp: Response, path: string): boolean {
+    if (resp.status !== 401 || !apiKey || !cfg.renewCredential) return false;
+    const next = cfg.renewCredential(apiKey);
+    if (!next || next === apiKey) return false;
+    logger.debug(`401 on ${path}: another process refreshed the login; retrying with the stored token`);
+    apiKey = next;
+    resp.body?.cancel().catch(() => undefined);
+    return true;
+  }
 
   function headersFor(extra?: ConstructorParameters<typeof Headers>[0]): Headers {
     const headers = new Headers(extra);
-    if (cfg.apiKey) headers.set("Authorization", `Bearer ${cfg.apiKey}`);
+    if (apiKey) headers.set("Authorization", `Bearer ${apiKey}`);
     else headers.delete("Authorization");
     headers.delete("Cookie");
     headers.set("User-Agent", userAgent);
@@ -262,15 +283,19 @@ export function createTransport(cfg: TransportConfig): Transport {
           url.searchParams.set(k, String(v));
         }
       }
-      const headers = headersFor(opts.headers);
-      const init: RequestInit = { method };
-      if (opts.body !== undefined) {
-        headers.set("Content-Type", "application/json");
-        init.body = JSON.stringify(opts.body);
-      }
-      init.headers = headers;
+      const body = opts.body === undefined ? undefined : JSON.stringify(opts.body);
+      const send = () => {
+        const headers = headersFor(opts.headers);
+        if (body !== undefined) headers.set("Content-Type", "application/json");
+        return doFetch(url, { method, headers, ...(body === undefined ? {} : { body }) }, timeoutMs, opts.signal, path);
+      };
       const timeoutMs = opts.timeoutMs ?? cfg.readTimeoutMs;
-      const { resp, release, timeoutSignal } = await doFetch(url, init, timeoutMs, opts.signal, path);
+      let sent = await send();
+      if (renewAfter(sent.resp, path)) {
+        sent.release();
+        sent = await send();
+      }
+      const { resp, release, timeoutSignal } = sent;
       try {
         refuseRedirect(resp, path);
         if (!resp.ok) {
@@ -300,9 +325,15 @@ export function createTransport(cfg: TransportConfig): Transport {
 
     async fetchRaw(path, init = {}) {
       const url = resolveApiUrl(baseUrl, path);
-      const headers = headersFor(init.headers);
       const timeoutMs = init.timeoutMs ?? cfg.readTimeoutMs;
-      const { resp, release } = await doFetch(url, { ...init, headers }, timeoutMs, init.signal ?? undefined, path);
+      const send = () => doFetch(url, { ...init, headers: headersFor(init.headers) }, timeoutMs, init.signal ?? undefined, path);
+      let sent = await send();
+      // Only a body that can be read twice is sent twice.
+      if ((init.body == null || typeof init.body === "string") && renewAfter(sent.resp, path)) {
+        sent.release();
+        sent = await send();
+      }
+      const { resp, release } = sent;
       try {
         refuseRedirect(resp, path);
         if (!resp.ok) throw await mapHttpError(resp, path, cfg.credentialKind);
@@ -332,16 +363,20 @@ export function createTransport(cfg: TransportConfig): Transport {
 
     async openStream(path, opts = {}) {
       const url = resolveApiUrl(baseUrl, path);
-      const headers = headersFor({ Accept: "text/event-stream", ...(opts.headers ?? {}) });
       const controller = new AbortController();
       const signal = combineSignals(controller.signal, opts.signal)!;
       const connectTimeoutMs = opts.connectTimeoutMs ?? cfg.readTimeoutMs;
       const connectTimer = setTimeout(() => controller.abort(new Error(`stream connect timed out after ${connectTimeoutMs}ms`)), connectTimeoutMs);
       const fetchImpl = cfg.fetchImpl ?? globalThis.fetch;
+      const open = () => {
+        logger.debug(`HTTP GET (stream) ${url.href}`);
+        const headers = headersFor({ Accept: "text/event-stream", ...(opts.headers ?? {}) });
+        return fetchImpl(url, { method: "GET", headers, signal, redirect: "manual" });
+      };
       let resp: Response;
       try {
-        logger.debug(`HTTP GET (stream) ${url.href}`);
-        resp = await fetchImpl(url, { method: "GET", headers, signal, redirect: "manual" });
+        resp = await open();
+        if (renewAfter(resp, path)) resp = await open();
       } catch (err) {
         clearTimeout(connectTimer);
         throw wrapFetchError(err, "request", path, opts.signal, controller.signal, connectTimeoutMs);

src/index.ts

@@ -13,7 +13,7 @@
 import type { Command } from "commander";
 
 import { buildRootCommand, LOCAL_COMMANDS } from "./root.js";
-import { currentCommand, interruptedBy, markInterrupted, wasInterrupted } from "./internal/context.js";
+import { currentCommand, exitWhenSafe, interruptedBy, markInterrupted, wasInterrupted } from "./internal/context.js";
 import { isCommanderInformational, CliError } from "./internal/errors.js";
 import { painterFor } from "./internal/color.js";
 import { emitEnvelope, printWarnings, type OutputFormat } from "./internal/output.js";
@@ -67,10 +67,10 @@ function installSignalHandlers(): void {
           return;
         }
         process.stderr.write(`\ninterrupted by ${signal} — finishing local bookkeeping\n`);
-        setTimeout(() => process.exit(130), TERMINATE_GRACE_MS).unref();
+        setTimeout(() => exitWhenSafe(130), TERMINATE_GRACE_MS).unref();
         return;
       }
-      process.exit(130);
+      exitWhenSafe(130);
     });
   }
 }

src/internal/config.ts

@@ -28,7 +28,7 @@
  * belongs to) and DSH_MESHY_SUBJECT (the account it binds journal records to).
  */
 
-import { credentialsPath, readCredentials, resolveStoredCredential, type CredentialKind } from "./credentials.js";
+import { credentialsPath, profileSecrets, readCredentials, resolveStoredCredential, type CredentialKind } from "./credentials.js";
 import { loadEnvFile } from "./env-file.js";
 import { authRequiredError, CliError, UsageError } from "./errors.js";
 import { setLogLevel, type LogLevel } from "./logger.js";
@@ -288,6 +288,32 @@ export function loadConfig(overrides: ConfigOverrides = {}): MeshyConfig {
   return cfg;
 }
 
+/**
+ * After a 401, the access token the store holds now for the same login, when
+ * it is a different, unexpired one: another process refreshed while this one
+ * held the old token, and the API ends the old access token the moment it
+ * issues the new one. Null for API keys, another account, or nothing newer.
+ * A host-provided credential matches by its subject (the profile's user id or
+ * login id), a stored one by its login id.
+ */
+export function storedTokenAfterRejection(cfg: MeshyConfig, rejected: string): string | null {
+  if (cfg.credentialKind !== "oauth" || (cfg.credentialSource !== "file" && cfg.credentialSource !== "host")) return null;
+  try {
+    const state = readCredentials(cfg.credentialsFile);
+    const name = cfg.credentialSource === "file" ? cfg.credentialProfile : state?.active_profile;
+    const profile = name ? state?.profiles[name] : undefined;
+    if (!profile || profile.kind !== "oauth" || (profile.expires_at ?? 0) <= Date.now()) return null;
+    const sameLogin = cfg.credentialSource === "host"
+      ? Boolean(cfg.credentialSubject) && (cfg.credentialSubject === profile.user_id || cfg.credentialSubject === profile.login_id)
+      : !cfg.credentialLoginId || cfg.credentialLoginId === profile.login_id;
+    if (!sameLogin) return null;
+    const token = profileSecrets(cfg.credentialsFile, profile).access_token;
+    return token && token !== rejected ? token : null;
+  } catch {
+    return null;
+  }
+}
+
 /**
  * Stored profiles and host-provided credentials were issued for the v1 origin
  * they were resolved against. A different Creative Lab origin only receives an

src/internal/context.ts

@@ -59,6 +59,32 @@ export function interruptedBy(): string {
   return interruptSignal;
 }
 
+let heldExits = 0;
+let pendingExit: number | undefined;
+
+/**
+ * Runs `fn` with forced exits (a second Ctrl-C, the end of a terminate
+ * signal's grace period) held until it settles. For the few steps that must
+ * not stop half-way: once the API has answered an OAuth refresh, the old
+ * refresh token is spent, and a process killed before saving the answer
+ * would leave only that behind.
+ */
+export async function withExitHeld<T>(fn: () => Promise<T>): Promise<T> {
+  heldExits += 1;
+  try {
+    return await fn();
+  } finally {
+    heldExits -= 1;
+    if (heldExits === 0 && pendingExit !== undefined) process.exit(pendingExit);
+  }
+}
+
+/** process.exit, unless withExitHeld is running: then once it settles. */
+export function exitWhenSafe(code: number): void {
+  if (heldExits > 0) pendingExit = code;
+  else process.exit(code);
+}
+
 /** Test hook: reset module state between in-process runs. */
 export function resetCommandContextForTests(): void {
   current = null;

src/internal/credentials.ts

@@ -178,13 +178,15 @@ function sleepSync(ms: number): void {
 }
 
 /**
- * Runs `fn` while holding an exclusive lock keyed to `file`.
+ * Takes the exclusive lock keyed to `file` and returns its release. Callers
+ * that hold it across an await (an OAuth refresh) use this directly; the rest
+ * use withCredentialsLock.
  *
  * O_EXCL creation is the atomic primitive here — `flock` is not exposed by
  * Node's fs module. A lock whose mtime is older than LOCK_STALE_MS is treated
  * as abandoned and broken, so a killed process cannot wedge the CLI forever.
  */
-export function withCredentialsLock<T>(file: string, fn: () => T): T {
+export function acquireCredentialsLock(file: string): () => void {
   const lock = lockPathFor(file);
   mkdirSync(dirname(lock), { recursive: true, mode: 0o700 });
   const deadline = Date.now() + LOCK_TIMEOUT_MS;
@@ -213,15 +215,23 @@ export function withCredentialsLock<T>(file: string, fn: () => T): T {
       sleepSync(25);
     }
   }
-  try {
-    return fn();
-  } finally {
+  return () => {
     closeSync(fd);
     try {
       unlinkSync(lock);
     } catch {
       /* already gone */
     }
+  };
+}
+
+/** Runs `fn` while holding the exclusive lock keyed to `file`. */
+export function withCredentialsLock<T>(file: string, fn: () => T): T {
+  const release = acquireCredentialsLock(file);
+  try {
+    return fn();
+  } finally {
+    release();
   }
 }
 
@@ -347,16 +357,22 @@ export function updateCredentials(
   file: string,
   mutate: (state: CredentialsFile) => CredentialsFile | void,
 ): CredentialsFile {
-  return withCredentialsLock(file, () => {
-    const own = readStateFile(file);
-    const legacy = own ? null : legacyCredentialsPath(file);
-    const seeded = legacy ? readStateFile(legacy) : null;
-    const current = own ?? seeded ?? emptyFile();
-    const next = mutate(current) ?? current;
-    writeCredentials(next, file);
-    if (legacy && seeded) unlinkUnderLock(legacy);
-    return next;
-  });
+  return withCredentialsLock(file, () => updateCredentialsHeld(file, mutate));
+}
+
+/** updateCredentials for a caller that already holds the lock (acquireCredentialsLock). */
+export function updateCredentialsHeld(
+  file: string,
+  mutate: (state: CredentialsFile) => CredentialsFile | void,
+): CredentialsFile {
+  const own = readStateFile(file);
+  const legacy = own ? null : legacyCredentialsPath(file);
+  const seeded = legacy ? readStateFile(legacy) : null;
+  const current = own ?? seeded ?? emptyFile();
+  const next = mutate(current) ?? current;
+  writeCredentials(next, file);
+  if (legacy && seeded) unlinkUnderLock(legacy);
+  return next;
 }
 
 function unlinkUnderLock(file: string): boolean {