diff --git a/index.ts b/index.ts index 1cb71259..aa6ea961 100644 --- a/index.ts +++ b/index.ts @@ -394,6 +394,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { let startupPreflightShown = false; let beginnerSafeModeEnabled = false; const MIN_BACKOFF_MS = 100; + // An all-accounts rate-limit wait can run for days, and the local accounts + // file is its only wake-up. A quota reset granted server-side leaves that file + // untouched, so such a wait would be slept straight through. Long waits + // therefore re-probe upstream: first after a minute, doubling to a quarter + // hour, so a multi-day sleep costs a handful of usage requests rather than one + // per countdown tick. + const UPSTREAM_REPROBE_MIN_WAIT_MS = 60_000; + const UPSTREAM_REPROBE_FIRST_DELAY_MS = 60_000; + const UPSTREAM_REPROBE_MAX_DELAY_MS = 15 * 60_000; const runtimeMetrics: RuntimeMetrics = { startedAt: Date.now(), @@ -1675,6 +1684,50 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + /** + * `loadAccounts()` reports a read or parse failure the same way it reports + * an absent file - by returning null - and a null load builds a manager + * holding zero accounts. Installing that over a working pool makes this + * process answer "No Codex accounts configured" while the accounts file on + * disk is intact, which cross-process lock contention makes reachable. + * + * Emptying the pool for real always goes through an explicit action + * (`codex-remove`, logout, a storage-mode switch); each installs its own + * manager rather than arriving here, so refusing the shrink costs a genuine + * deletion nothing. + */ + const isUntrustworthyEmptyReload = ( + incumbent: AccountManager | null, + reloaded: AccountManager, + ): boolean => + incumbent !== null && + incumbent !== reloaded && + reloaded.getAccountCount() === 0 && + incumbent.getAccountCount() > 0; + + const EMPTY_RELOAD_RETRY_DELAY_MS = 2000; + const EMPTY_RELOAD_MAX_RETRIES = 3; + let emptyReloadRetries = 0; + let emptyReloadRetryTimer: ReturnType | undefined; + const cancelEmptyReloadRetry = (): void => { + clearTimeout(emptyReloadRetryTimer); + emptyReloadRetryTimer = undefined; + emptyReloadRetries = 0; + }; + const scheduleEmptyReloadRetry = (retry: () => Promise): void => { + if (emptyReloadRetries >= EMPTY_RELOAD_MAX_RETRIES) { + emptyReloadRetries = 0; + return; + } + emptyReloadRetries += 1; + clearTimeout(emptyReloadRetryTimer); + emptyReloadRetryTimer = setTimeout(() => { + emptyReloadRetryTimer = undefined; + void retry(); + }, EMPTY_RELOAD_RETRY_DELAY_MS); + emptyReloadRetryTimer.unref(); + }; + const reloadCachedAccountManager = async (): Promise => { if (!cachedAccountManager) return; const previous = cachedAccountManager; @@ -1693,6 +1746,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } try { const reloadedManager = await AccountManager.loadFromDisk(); + if (isUntrustworthyEmptyReload(previous, reloadedManager)) { + reloadedManager.disposeShutdownHandler(); + logWarn( + `[${PLUGIN_NAME}] Account reload returned no accounts while ${previous.getAccountCount()} are held; keeping the loaded pool and retrying`, + ); + scheduleEmptyReloadRetry(reloadCachedAccountManager); + return; + } + cancelEmptyReloadRetry(); cachedAccountManager = reloadedManager; accountManagerPromise = Promise.resolve(reloadedManager); // Dispose only after the replacement is installed so we never leak @@ -1725,21 +1787,41 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accountsWatcherDisposed = true; unsubscribeAccountsPath?.(); stopAccountsWatcher(); + cancelEmptyReloadRetry(); unregisterCleanup(disposeAccountsWatcher); }; - const readAccountsDigest = async (path: string): Promise => { + const readAccountsFileState = async ( + path: string, + ): Promise<{ digest: string; accountCount: number } | undefined> => { try { const content = await readFile(path, "utf8"); - if (!AnyAccountStorageSchema.safeParse(JSON.parse(content)).success) return; - return createHash("sha256").update(content).digest("hex"); + const data = JSON.parse(content) as unknown; + if (!AnyAccountStorageSchema.safeParse(data).success) return; + // Counted off the raw document rather than the parsed union so the + // count is the same for every storage version. + const accounts = (data as { accounts?: unknown }).accounts; + return { + digest: createHash("sha256").update(content).digest("hex"), + accountCount: Array.isArray(accounts) ? accounts.length : 0, + }; } catch { return; } }; const reloadForExternalAccountsChange = async (path: string, generation: number, attempt = 0, retired?: AccountManager): Promise => { - const digest = await readAccountsDigest(path); - if (generation !== accountsWatchGeneration || !digest || path !== getStoragePath()) return; + const observed = await readAccountsFileState(path); + if (generation !== accountsWatchGeneration || !observed || path !== getStoragePath()) return; + const digest = observed.digest; if (digest === consumeLastWrittenAccountsDigest(path)) return; + const retryLater = (): void => { + if (attempt < 2 && generation === accountsWatchGeneration && !accountsWatcherDisposed) { + accountsReloadTimer = setTimeout(() => { + accountsReloadTimer = undefined; + void reloadForExternalAccountsChange(path, generation, attempt + 1, retired); + }, 1500); + accountsReloadTimer.unref(); + } + }; const previous = cachedAccountManager; try { // A null cache means an invalidation retired the incumbent; the @@ -1760,6 +1842,21 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { reloaded.disposeShutdownHandler(); return; } + // The file this reload observed carried accounts but the load + // produced none, so `loadAccounts()` failed to read it rather than + // the accounts having gone away - a failure it reports as an empty + // result, never as a throw, so the catch below cannot see it. + // Adopting it would answer "No Codex accounts configured" against an + // intact file; the retired incumbent still serves its accounts until + // a retry lands a real one. + if (observed.accountCount > 0 && reloaded.getAccountCount() === 0) { + reloaded.disposeShutdownHandler(); + logWarn( + `[${PLUGIN_NAME}] Externally changed accounts file holds ${observed.accountCount} account(s) but loaded as empty; keeping the current pool and retrying`, + ); + retryLater(); + return; + } const outgoing = cachedAccountManager; if (outgoing && outgoing !== retired) { // Another actor replaced the cached manager while this reload @@ -1775,13 +1872,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { observedAccountsDigest = digest; } catch { logWarn("Could not reload externally updated account storage"); - if (attempt < 2 && generation === accountsWatchGeneration && !accountsWatcherDisposed) { - accountsReloadTimer = setTimeout(() => { - accountsReloadTimer = undefined; - void reloadForExternalAccountsChange(path, generation, attempt + 1, retired); - }, 1500); - accountsReloadTimer.unref(); - } + retryLater(); return; } logDebug("Reloaded cached account manager after external accounts file change"); @@ -1795,8 +1886,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const path = watchedAccountsPath; if (!path) return; const generation = accountsWatchGeneration; - const digest = await readAccountsDigest(path); - if (generation !== accountsWatchGeneration || !digest || digest === observedAccountsDigest) return; + const observed = await readAccountsFileState(path); + if (generation !== accountsWatchGeneration || !observed || observed.digest === observedAccountsDigest) return; + const digest = observed.digest; observedAccountsDigest = digest; clearTimeout(accountsReloadTimer); accountsReloadTimer = undefined; @@ -1821,9 +1913,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }); watchedAccountsPath = path; const generation = accountsWatchGeneration; - const initialDigest = await readAccountsDigest(path); + const initial = await readAccountsFileState(path); if (generation !== accountsWatchGeneration) return; - observedAccountsDigest = initialDigest; + observedAccountsDigest = initial?.digest; // Stat polling follows the path across the storage writer's temp-file rename. watchFile(path, { interval: 1500, persistent: false }, onAccountsStatChanged); unregisterCleanup(disposeAccountsWatcher); @@ -2396,9 +2488,20 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const consumeRetryBudget = ( bucket: RetryBudgetClass, reason: string, + waitMs?: number, ): boolean => { - if (retryBudget.consume(bucket)) { - runtimeMetrics.retryBudgetUsage[bucket] += 1; + // Pass the wait so the charge scales with how long the retry + // blocks. Metrics follow the tracker's own counter rather than + // assuming one unit, or a free sub-second wait would report + // budget it never spent. + const usedBefore = retryBudget.getUsage()[bucket]; + const granted = + waitMs === undefined + ? retryBudget.consume(bucket) + : retryBudget.consumeWait(bucket, waitMs); + if (granted) { + runtimeMetrics.retryBudgetUsage[bucket] += + retryBudget.getUsage()[bucket] - usedBefore; return true; } runtimeMetrics.retryBudgetExhaustions += 1; @@ -2450,16 +2553,35 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { totalMs: number, message: string, intervalMs: number = 5000, + probeUpstream?: () => Promise, ): Promise => { const startTime = Date.now(); const endTime = startTime + totalMs; - + let probeDelayMs = UPSTREAM_REPROBE_FIRST_DELAY_MS; + let nextProbeAt = + probeUpstream && totalMs >= UPSTREAM_REPROBE_MIN_WAIT_MS + ? startTime + probeDelayMs + : Number.POSITIVE_INFINITY; + while (Date.now() < endTime) { if (cachedAccountManager !== accountManager) return; if (abortSignal?.aborted) { throw abortError(); } - + + if (probeUpstream && Date.now() >= nextProbeAt) { + if (await probeUpstream()) return; + if (cachedAccountManager !== accountManager) return; + if (abortSignal?.aborted) { + throw abortError(); + } + probeDelayMs = Math.min(probeDelayMs * 2, UPSTREAM_REPROBE_MAX_DELAY_MS); + // Measured from the end of the probe, so a slow usage + // request cannot schedule the next one in the past and + // collapse the countdown sleep below to zero. + nextProbeAt = Date.now() + probeDelayMs; + } + const remaining = Math.max(0, endTime - Date.now()); const waitLabel = formatWaitTime(remaining); await showToast( @@ -2467,8 +2589,8 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { "warning", { duration: Math.min(intervalMs + 1000, toastDurationMs) }, ); - - const sleepTime = Math.min(intervalMs, remaining); + + const sleepTime = Math.min(intervalMs, remaining, nextProbeAt - Date.now()); if (sleepTime > 0) { await sleep(sleepTime); } else { @@ -2477,6 +2599,32 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + /** + * True when an all-accounts wait can stop early. + * + * `runNow` refreshes `/wham/usage` for every account and persists + * whatever it finds, so a reset that never touched local disk + * becomes visible here. Persisting a recovery also drops the cached + * manager, which is what makes the enclosing retry loop re-resolve + * one that no longer reports a block. + */ + const probeUpstreamBlockLifted = async (): Promise => { + try { + await quotaMonitor.runNow(); + } catch (error) { + logDebug( + `[${PLUGIN_NAME}] Upstream quota re-probe failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return false; + } + if (cachedAccountManager !== accountManager) return true; + const manager = accountManager; + if (!manager) return false; + return manager.getMinWaitTimeForFamily(modelFamily, model) === 0; + }; + let allRateLimitedRetries = 0; let emptyResponseRetries = 0; const attemptedUnsupportedFallbackModels = new Set(); @@ -3795,10 +3943,16 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { consumeRetryBudget( "rateLimitGlobal", `All accounts rate-limited wait ${waitMs}ms`, + waitMs, ) ) { const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; - await sleepWithCountdown(addJitter(waitMs, 0.2), countdownMessage); + await sleepWithCountdown( + addJitter(waitMs, 0.2), + countdownMessage, + undefined, + probeUpstreamBlockLifted, + ); allRateLimitedRetries++; continue; } diff --git a/lib/quota-notifications.ts b/lib/quota-notifications.ts index dc28f5ef..a97810b4 100644 --- a/lib/quota-notifications.ts +++ b/lib/quota-notifications.ts @@ -455,7 +455,11 @@ export function createQuotaMonitor(overrides: Partial = {}) schedule(intervalMs, expectedGeneration); }; - const tick = async (expectedGeneration: number, reschedule: boolean): Promise => { + const tick = async ( + expectedGeneration: number, + reschedule: boolean, + force = false, + ): Promise => { if (disposed || expectedGeneration !== generation) return; if (running) { if (reschedule) scheduleNext(expectedGeneration); @@ -475,7 +479,11 @@ export function createQuotaMonitor(overrides: Partial = {}) // request simply retries on the next interval, so it cannot turn usage // endpoint throttling into a routing block. keepPolling = config.autoProtectCredits !== false || notificationsEnabled; - if (keepPolling) await check(config, expectedGeneration); + // Both switches govern the UNATTENDED poll. A forced check is an + // on-demand request from a caller that is blocked on the answer, so + // honouring them here would let `runNow()` return without asking + // upstream anything at all. + if (force || keepPolling) await check(config, expectedGeneration); } catch (error) { logDebug(`Quota monitor tick failed: ${(error as Error).message}`); } finally { @@ -510,7 +518,7 @@ export function createQuotaMonitor(overrides: Partial = {}) }, dispose: disposeMonitor, async runNow() { - await tick(generation, false); + await tick(generation, false, true); }, }; } @@ -551,7 +559,15 @@ async function fetchUsageForAccount( // only the proactive routing guard is unavailable until the next poll. logWarn(`Failed to persist exhausted usage quota: ${(error as Error).message}`); } - } else if (autoProtectCredits && isUsageQuotaRecovered([usage.primary, usage.secondary])) { + // Clearing a stale block is not part of the credit guard. + // `autoProtectCredits` opts out of BLOCKING rotation, while the request + // path stamps a block from 429 headers regardless of it. Gating the + // clear on it too left those accounts blocked with nothing able to + // clear them, so the long-wait probe could never wake. + } else if ( + quotaExhaustedResetAtMs === undefined && + isUsageQuotaRecovered([usage.primary, usage.secondary]) + ) { try { if (await persistUsageQuotaRecovery(account)) onCredentialsPersisted(); } catch { diff --git a/lib/request/retry-budget.ts b/lib/request/retry-budget.ts index d1ea2beb..a86d01bf 100644 --- a/lib/request/retry-budget.ts +++ b/lib/request/retry-budget.ts @@ -43,6 +43,18 @@ const PROFILE_LIMITS: Record = { }, }; +/** + * How much blocking one budget unit buys when a retry is charged through + * {@link RetryBudgetTracker.consumeWait}. + * + * The budgets are small (1/3/10) and were being charged one unit per wait + * regardless of length, so three consecutive sub-second waits exhausted the + * default and hard-failed a request that one more second would have served. + * Waiting is only expensive in proportion to the time it costs the caller, so + * that is what a unit now measures. + */ +export const RETRY_WAIT_BUDGET_UNIT_MS = 5_000; + const RETRY_BUDGET_CLASSES: RetryBudgetClass[] = [ "authRefresh", "network", @@ -87,12 +99,42 @@ function createUsedCounters(): RetryBudgetLimits { export class RetryBudgetTracker { private readonly used: RetryBudgetLimits = createUsedCounters(); + private readonly waitCarryMs: RetryBudgetLimits = createUsedCounters(); private readonly limits: RetryBudgetLimits; constructor(limits: RetryBudgetLimits) { this.limits = { ...limits }; } + /** + * Charge a retry that blocks for `waitMs` against a bucket, in proportion to + * how long it blocks. + * + * A wait of {@link RETRY_WAIT_BUDGET_UNIT_MS} or longer costs a full unit, + * so a multi-hour block stays governed exactly as before. Shorter waits + * accumulate on a per-bucket carry and only cost a unit once they have added + * up to one, so a burst of sub-second waits is effectively free. + * + * An exhausted bucket refuses even a free wait: the carry bounds how long + * short waits can loop, and without that check they would loop forever once + * the budget ran out. + */ + consumeWait(bucket: RetryBudgetClass, waitMs: number): boolean { + if (this.getRemaining(bucket) <= 0) return false; + + const wait = Number.isFinite(waitMs) && waitMs > 0 ? waitMs : 0; + if (wait >= RETRY_WAIT_BUDGET_UNIT_MS) return this.consume(bucket); + + const carried = this.waitCarryMs[bucket] + wait; + if (carried < RETRY_WAIT_BUDGET_UNIT_MS) { + this.waitCarryMs[bucket] = carried; + return true; + } + + this.waitCarryMs[bucket] = carried - RETRY_WAIT_BUDGET_UNIT_MS; + return this.consume(bucket); + } + consume(bucket: RetryBudgetClass): boolean { const limit = this.limits[bucket]; if (!Number.isFinite(limit)) { diff --git a/lib/storage/load-save.ts b/lib/storage/load-save.ts index fddcbe71..0066c209 100644 --- a/lib/storage/load-save.ts +++ b/lib/storage/load-save.ts @@ -25,7 +25,7 @@ import { AnyAccountStorageSchema, getValidationErrors } from "../schemas.js"; import { renameWithWindowsRetry } from "./atomic-write.js"; import { formatStorageErrorHint, StorageError } from "./errors.js"; import { normalizeAccountStorage } from "./normalize.js"; -import { getConfigDir } from "./paths.js"; +import { getConfigDir, isWithinDirectory } from "./paths.js"; import { getCurrentLegacyProjectStoragePath, getCurrentProjectRoot, @@ -142,6 +142,10 @@ async function checkWorktreeLockForCurrentStorage( }); return; } + // Before the probe, not inside the try: `acquireOrDetectLock` writes a lock + // sidecar next to the accounts file, so a leaked HOME would touch the real + // store here even on a pure read, and this function's catch would hide it. + assertTestRunNeverTouchesRealHome(path); try { const result = await acquireOrDetectLock(path); if (!result.acquired && result.foreign) { @@ -171,6 +175,52 @@ async function checkWorktreeLockForCurrentStorage( } } +/** + * Refuse to touch account storage inside the developer's real home while the + * test suite is running. + * + * `vitest.config.ts` redirects HOME to a sandbox before any module loads, but a + * test that restores the captured real HOME, or a future regression in that + * config, would otherwise read or overwrite live ChatGPT credentials. + * `os.userInfo()` reads the passwd entry instead of `$HOME`, so it still names + * the real home after the redirect and gives the check something the sandbox + * cannot spoof. Inert outside vitest. + * + * The sandbox and the temp directory are exempt, and both exemptions are + * load-bearing rather than convenience: on Windows `os.tmpdir()` normally sits + * inside the user profile, so there every legitimate sandbox path is also a + * real-home path and a bare home-prefix test would reject the entire suite. + * Neither exemption can reach the production store, which lives under the real + * home's `.opencode`. + */ +function assertTestRunNeverTouchesRealHome(path: string): void { + if (!process.env.VITEST) return; + + let realHome: string; + try { + realHome = os.userInfo().homedir; + } catch { + return; + } + if (!realHome || !isWithinDirectory(realHome, path)) return; + + // A root exempts what lies beneath it only while it does not itself swallow + // the real home. Without that clause `OC_CODEX_TEST_HOME=/` or `TMPDIR=$HOME` + // would disarm the guard completely. + const exempts = (root: string | undefined): boolean => + !!root && !isWithinDirectory(root, realHome) && isWithinDirectory(root, path); + + if (exempts(process.env.OC_CODEX_TEST_HOME)) return; + if (exempts(os.tmpdir())) return; + + throw new StorageError( + `Refusing to touch account storage inside the real home directory during a test run: ${path}`, + "TEST_HOME_ESCAPE", + path, + "A test resolved account storage against the developer's real home. Point HOME at a temp directory for the whole vitest process (see vitest.config.ts) instead of overriding it per test.", + ); +} + async function ensureGitignore(storagePath: string): Promise { if (!getCurrentStoragePath()) return; @@ -207,6 +257,10 @@ async function migrateStorageFileIfNeeded( persist: (storage: AccountStorageV3) => Promise, label: string, ): Promise { + // Before the existsSync, and outside the try: this reads the legacy file and + // the catch below swallows everything except a forward-compat reject, so a + // guard placed any later would be silently discarded. + if (legacyPath) assertTestRunNeverTouchesRealHome(legacyPath); if (!legacyPath || legacyPath === nextPath || !existsSync(legacyPath)) { return null; } @@ -303,6 +357,12 @@ async function loadGlobalAccountsFallback(): Promise { return null; } + // The project store is missing, so this reaches for the GLOBAL one, which + // resolves against `homedir()` and is the real pool whenever HOME has been + // restored. Guarded here rather than at the read below, because the catch + // there returns null for everything and would hide the escape. + assertTestRunNeverTouchesRealHome(getGlobalAccountsStoragePath()); + const migrated = await migrateLegacyGlobalStorageIfNeeded(); if (migrated) { return migrated; @@ -537,6 +597,7 @@ async function loadAccountsInternal( * Callers must already be inside withStorageLock when using this helper directly. */ async function writeAccountsToPathUnlocked(path: string, storage: AccountStorageV3): Promise { + assertTestRunNeverTouchesRealHome(path); const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; const tempPath = `${path}.${uniqueSuffix}.tmp`; @@ -759,6 +820,7 @@ export async function clearAccounts(): Promise { let jsonCleared = true; try { const path = getStoragePath(); + assertTestRunNeverTouchesRealHome(path); await fs.unlink(path); } catch (error) { const code = (error as NodeJS.ErrnoException).code; diff --git a/lib/storage/paths.ts b/lib/storage/paths.ts index 1dc45bc6..ad2e11ce 100644 --- a/lib/storage/paths.ts +++ b/lib/storage/paths.ts @@ -81,7 +81,7 @@ function normalizePathForComparison(filePath: string): string { return process.platform === "win32" ? resolvedPath.toLowerCase() : resolvedPath; } -function isWithinDirectory(baseDir: string, targetPath: string): boolean { +export function isWithinDirectory(baseDir: string, targetPath: string): boolean { const normalizedBase = normalizePathForComparison(baseDir); const normalizedTarget = normalizePathForComparison(targetPath); const rel = relative(normalizedBase, normalizedTarget); diff --git a/test/accounts-live-reload.test.ts b/test/accounts-live-reload.test.ts index 1998ded9..3612f87a 100644 --- a/test/accounts-live-reload.test.ts +++ b/test/accounts-live-reload.test.ts @@ -9,6 +9,7 @@ const captured = vi.hoisted((): { context?: ToolContext; listener?: () => void; onWatch?: () => void; + onQuotaProbe?: () => Promise; reads: Promise[]; maxRetries?: number; } => ({ reads: [] })); @@ -32,7 +33,11 @@ vi.mock("../lib/tools/index.js", () => ({ createToolRegistry: (context: ToolContext) => { captured.context = context; return {}; }, })); vi.mock("../lib/quota-notifications.js", () => ({ - createQuotaMonitor: () => ({ start() {}, dispose() {} }), + createQuotaMonitor: () => ({ + start() {}, + dispose() {}, + runNow: async () => { await captured.onQuotaProbe?.(); }, + }), })); vi.mock("../lib/auto-update-checker.js", () => ({ checkAndNotify: vi.fn(async () => {}) })); vi.mock("../lib/config.js", async (original) => ({ @@ -91,6 +96,7 @@ describe("accounts live reload", () => { await fs.writeFile(path, JSON.stringify(storage(true))); captured.listener = undefined; captured.onWatch = undefined; + captured.onQuotaProbe = undefined; captured.reads.length = 0; captured.maxRetries = undefined; plugin = await Reflect.apply(OpenAIOAuthPlugin, undefined, [{ @@ -285,6 +291,106 @@ describe("accounts live reload", () => { await vi.advanceTimersByTimeAsync(5000); expect((await response).status).toBe(200); }); + it("wakes a long wait when an upstream re-probe finds the block lifted", async () => { + const manager = captured.context?.cachedAccountManagerRef.current; + if (!manager) throw new Error("Missing manager"); + const account = manager.getCurrentAccount(); + if (!account) throw new Error("Missing account"); + manager.markQuotaExhausted(account, Date.now() + 86_400_000, "gpt-5.1"); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response("data: [DONE]\n\n", { + status: 200, headers: { "content-type": "text/event-stream" }, + })); + let enteredWait: () => void = () => {}; + const waiting = new Promise((resolve) => { enteredWait = resolve; }); + const minWait = manager.getMinWaitTimeForFamily.bind(manager); + vi.spyOn(manager, "getMinWaitTimeForFamily").mockImplementation((...args) => { + enteredWait(); + return minWait(...args); + }); + let probes = 0; + captured.onQuotaProbe = async () => { + probes += 1; + // What a server-side grant looks like: usage reports the quota back, + // the recovery is persisted, and the cached manager is dropped. The + // accounts file is never written by another process, so the watcher + // has nothing to fire on - this is the wake-up it cannot provide. + captured.context?.invalidateAccountManagerCache(); + }; + const response = request("https://api.openai.com/v1/responses", { + method: "POST", body: JSON.stringify({ model: "gpt-5.1", stream: true, input: [] }), + }); + await waiting; + for (let elapsed = 0; elapsed < 120_000 && probes === 0; elapsed += 5000) { + await vi.advanceTimersByTimeAsync(5000); + } + expect(probes).toBe(1); + await vi.advanceTimersByTimeAsync(5000); + expect((await response).status).toBe(200); + }); + it("wakes a waiting request when a login adds an account mid-sleep", async () => { + const manager = captured.context?.cachedAccountManagerRef.current; + if (!manager) throw new Error("Missing manager"); + const account = manager.getCurrentAccount(); + if (!account) throw new Error("Missing account"); + const blockedUntil = Date.now() + 86_400_000; + manager.markQuotaExhausted(account, blockedUntil, "gpt-5.1"); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response("data: [DONE]\n\n", { + status: 200, headers: { "content-type": "text/event-stream" }, + })); + let enteredWait: () => void = () => {}; + const waiting = new Promise((resolve) => { enteredWait = resolve; }); + const minWait = manager.getMinWaitTimeForFamily.bind(manager); + vi.spyOn(manager, "getMinWaitTimeForFamily").mockImplementation((...args) => { + enteredWait(); + return minWait(...args); + }); + const response = request("https://api.openai.com/v1/responses", { + method: "POST", body: JSON.stringify({ model: "gpt-5.1", stream: true, input: [] }), + }); + await waiting; + const reloaded = nextReload(); + // The incumbent account stays blocked on disk, so the only thing that can + // end this wait is the account the login added. + await fs.writeFile(path, JSON.stringify({ ...storage(true), accounts: [ + { ...storage(true).accounts[0], quotaExhaustedUntil: blockedUntil }, + { accountId: "fresh-login", refreshToken: "fresh-refresh", accessToken: "fresh-access", + expiresAt: Date.now() + 86_400_000, enabled: true, addedAt: 2, lastUsed: 2 }, + ] })); + await tick(); + await settle(); + await reloaded; + await vi.advanceTimersByTimeAsync(5000); + expect((await response).status).toBe(200); + }); + it("keeps the loaded pool when an external change loads as empty", async () => { + const previous = captured.context?.cachedAccountManagerRef.current; + if (!previous) throw new Error("Missing manager"); + expect(previous.getAccountCount()).toBe(1); + const empty = new AccountManager(undefined, { ...storage(true), accounts: [] }); + const load = vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(empty); + await fs.writeFile(path, JSON.stringify(storage(false))); + await tick(); + await settle(); + expect(load).toHaveBeenCalledTimes(1); + expect(captured.context?.cachedAccountManagerRef.current).toBe(previous); + expect(captured.context?.cachedAccountManagerRef.current?.getAccountCount()).toBe(1); + const reloaded = nextReload(); + await vi.advanceTimersByTimeAsync(1500); + await drainReads(); + await reloaded; + expect(load).toHaveBeenCalledTimes(2); + expect(captured.context?.cachedAccountManagerRef.current?.getAccountsSnapshot()[0]?.enabled).toBe(false); + }); + it("adopts an external change that genuinely removes the last account", async () => { + const previous = captured.context?.cachedAccountManagerRef.current; + const reloaded = nextReload(); + await fs.writeFile(path, JSON.stringify({ ...storage(true), accounts: [] })); + await tick(); + await settle(); + await reloaded; + expect(captured.context?.cachedAccountManagerRef.current).not.toBe(previous); + expect(captured.context?.cachedAccountManagerRef.current?.getAccountCount()).toBe(0); + }); it("keeps externally cleared blocks cleared despite queued and late saves from the old manager", async () => { const previous = captured.context?.cachedAccountManagerRef.current; if (!previous) throw new Error("Missing manager"); diff --git a/test/global-setup.ts b/test/global-setup.ts new file mode 100644 index 00000000..2ad8ab64 --- /dev/null +++ b/test/global-setup.ts @@ -0,0 +1,30 @@ +import { rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { resolve } from "node:path"; + +export const MINTED_HOME_PREFIX = "oc-codex-multi-auth-test-home-"; + +/** + * Remove the throwaway home `vitest.config.ts` minted for this run. + * + * Every run otherwise leaves one behind, and on a tmpfs `/tmp` they accumulate + * until a run dies of ENOSPC. + * + * Three conditions gate the delete, because this is an unattended `rm -rf`: + * the config must have minted the directory itself rather than been handed one + * through `OC_CODEX_TEST_HOME`, the resolved path must still sit directly under + * `tmpdir()`, and it must carry the prefix `mkdtempSync` was given. A path that + * fails any of them is left alone rather than guessed at. + */ +export async function teardown(): Promise { + if (process.env.OC_CODEX_TEST_HOME_OWNED !== "1") return; + + const home = process.env.OC_CODEX_TEST_HOME; + if (!home) return; + + const resolved = resolve(home); + const expectedPrefix = resolve(tmpdir(), MINTED_HOME_PREFIX); + if (resolved === expectedPrefix || !resolved.startsWith(expectedPrefix)) return; + + await rm(resolved, { recursive: true, force: true }); +} diff --git a/test/paths.test.ts b/test/paths.test.ts index ca215ec8..c3a88ceb 100644 --- a/test/paths.test.ts +++ b/test/paths.test.ts @@ -2,10 +2,29 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { homedir, tmpdir } from "node:os"; import path from "node:path"; +// `resolvePath` guards against a path that merely shares a string prefix with +// an allowed root, so the lookalike cases below build a sibling of one root and +// require it to be outside all three. Against the real layout that only holds +// by accident: the suite runs with HOME redirected under the temp dir, which +// makes every sibling of home a child of tmpdir() and turns the assertion into +// a no-op. Fixed roots keep these cases meaningful wherever HOME points. +// +// This mock and the isolated HOME in `vitest.config.ts` are load-bearing +// together. Removing the mock as redundant silently turns both lookalike +// assertions into no-ops rather than failing them. +const FAKE_HOME = path.resolve("/fake-storage-paths-home"); +const FAKE_TMPDIR = path.resolve("/fake-storage-paths-tmp"); + vi.mock("node:fs", () => ({ existsSync: vi.fn(), })); +vi.mock("node:os", async (importOriginal) => ({ + ...(await importOriginal()), + homedir: () => FAKE_HOME, + tmpdir: () => FAKE_TMPDIR, +})); + import { existsSync } from "node:fs"; import { getConfigDir, diff --git a/test/quota-notifications-fetch.test.ts b/test/quota-notifications-fetch.test.ts index befad211..47b3e23c 100644 --- a/test/quota-notifications-fetch.test.ts +++ b/test/quota-notifications-fetch.test.ts @@ -243,6 +243,30 @@ describe("default quota fetch path", () => { expect(onCredentialsPersisted).toHaveBeenCalledOnce(); }); + // Setting and clearing a block are not symmetric. `autoProtectCredits` + // opts out of BLOCKING rotation, but the request path still stamps a block + // from 429 headers regardless of it, so gating the clear on it too left + // those accounts blocked with nothing able to release them. + it("clears recovered quota even with credit protection switched off", async () => { + ensureCodexUsageAccessToken.mockResolvedValue({ accessToken: "access-1", persisted: false }); + fetchCodexUsage.mockResolvedValue({ rate_limit: { + primary_window: { used_percent: 10, limit_window_seconds: 18_000 }, + secondary_window: { used_percent: 20, limit_window_seconds: 604_800 }, + } }); + persistUsageQuotaRecovery.mockResolvedValue(true); + await monitorWith({ + loadConfig: () => ({ + enabled: true, + autoProtectCredits: false, + intervalMs: 1_000, + notifyEveryCheck: true, + thresholds: [25, 10, 0], + }), + notify: vi.fn().mockResolvedValue(true), + }).runNow(); + expect(persistUsageQuotaRecovery).toHaveBeenCalledWith(storage.accounts[0]); + }); + it.each([ {}, { rate_limit: { primary_window: { used_percent: 0, limit_window_seconds: 0 } } }, diff --git a/test/quota-notifications.test.ts b/test/quota-notifications.test.ts index a6f1b6be..74e799b5 100644 --- a/test/quota-notifications.test.ts +++ b/test/quota-notifications.test.ts @@ -399,6 +399,7 @@ describe("quota monitor lifecycle", () => { }); it("does not poll a configuration that can never deliver", async () => { + vi.useFakeTimers(); const loadStorage = vi.fn().mockResolvedValue(null); const monitor = createQuotaMonitor({ // Enabled, but with no thresholds and no every-check alert there is @@ -406,11 +407,16 @@ describe("quota monitor lifecycle", () => { loadConfig: () => ({ enabled: true, autoProtectCredits: false, intervalMs: 1_000, notifyEveryCheck: false, thresholds: [] }), loadStorage, notificationsSupported: () => true, + initialDelayMs: 10, }); - await monitor.runNow(); + // Driven through the scheduled poll rather than `runNow()`, which is + // forced by design for the on-demand caller. + monitor.start(); + await vi.advanceTimersByTimeAsync(10); expect(loadStorage).not.toHaveBeenCalled(); + monitor.dispose(); }); it("keeps polling on the configured interval while enabled", async () => { @@ -446,16 +452,36 @@ describe("quota monitor lifecycle", () => { expect(loadStorage).not.toHaveBeenCalled(); }); - it("does not poll accounts when notifications and credit protection are disabled", async () => { - const loadStorage = vi.fn().mockResolvedValue(null); + // Both switches govern the UNATTENDED poll, an invariant the two scheduling + // tests above already hold. `runNow()` is the on-demand path, and its one + // production caller is the all-accounts rate-limit wait, which is blocked + // until it learns whether upstream capacity has returned. Honouring the + // switches here left that wait asleep through a server-side reset, since + // nothing local changes when a reset is granted. + it("polls on demand even when notifications and credit protection are disabled", async () => { + const loadStorage = vi.fn().mockResolvedValue({ + version: 3 as const, + accounts: [{ refreshToken: "token", addedAt: 0, lastUsed: 0 }], + activeIndex: 0, + }); + const fetchSummary = vi.fn().mockResolvedValue(accountUsage({ + fiveHourUsed: 10, + weeklyUsed: 10, + })); + const notify = vi.fn(); const monitor = createQuotaMonitor({ - loadConfig: () => ({ enabled: true, autoProtectCredits: false, intervalMs: 1_000, notifyEveryCheck: false, thresholds: [25, 10, 0] }), + loadConfig: () => ({ enabled: false, autoProtectCredits: false, intervalMs: 1_000, notifyEveryCheck: false, thresholds: [25, 10, 0] }), loadStorage, + fetchSummary, + notify, notificationsSupported: () => false, }); await monitor.runNow(); - expect(loadStorage).not.toHaveBeenCalled(); + + expect(fetchSummary).toHaveBeenCalledOnce(); + // Forcing the probe must not deliver an alert the user switched off. + expect(notify).not.toHaveBeenCalled(); }); it("polls to protect Credits even when desktop notifications are unavailable", async () => { diff --git a/test/retry-budget.test.ts b/test/retry-budget.test.ts index 8f3e62fe..8d67dba7 100644 --- a/test/retry-budget.test.ts +++ b/test/retry-budget.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest"; import { RetryBudgetTracker, resolveRetryBudgetLimits, + RETRY_WAIT_BUDGET_UNIT_MS, type RetryBudgetLimits, } from "../lib/request/retry-budget.js"; @@ -72,6 +73,84 @@ describe("retry-budget", () => { expect(tracker.getUsage().authRefresh).toBe(1); }); + describe("consumeWait", () => { + const balanced = () => new RetryBudgetTracker(resolveRetryBudgetLimits("balanced")); + + it("does not charge the budget for a burst of sub-second waits", () => { + const tracker = balanced(); + + // The production regression: three consecutive 400ms waits spent the + // whole default budget and hard-failed the request. + for (let i = 0; i < 3; i++) { + expect(tracker.consumeWait("rateLimitGlobal", 400)).toBe(true); + } + expect(tracker.getUsage().rateLimitGlobal).toBe(0); + expect(tracker.getRemaining("rateLimitGlobal")).toBe(3); + + for (let i = 0; i < 27; i++) { + expect(tracker.consumeWait("rateLimitGlobal", 400)).toBe(true); + } + }); + + it("charges a full unit for a wait at or above the unit length", () => { + const tracker = balanced(); + + expect(tracker.consumeWait("rateLimitGlobal", RETRY_WAIT_BUDGET_UNIT_MS)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(1); + + expect(tracker.consumeWait("rateLimitGlobal", 6 * 60 * 60 * 1000)).toBe(true); + expect(tracker.consumeWait("rateLimitGlobal", 6 * 60 * 60 * 1000)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(3); + + // A long wait stays governed: the fourth exceeds the balanced budget. + expect(tracker.consumeWait("rateLimitGlobal", 6 * 60 * 60 * 1000)).toBe(false); + }); + + it("accumulates short waits into whole units", () => { + const tracker = balanced(); + const waitMs = RETRY_WAIT_BUDGET_UNIT_MS / 10; + + for (let i = 0; i < 10; i++) { + expect(tracker.consumeWait("rateLimitGlobal", waitMs)).toBe(true); + } + expect(tracker.getUsage().rateLimitGlobal).toBe(1); + }); + + it("refuses free short waits once the bucket is exhausted", () => { + const tracker = balanced(); + + for (let i = 0; i < 3; i++) { + expect(tracker.consumeWait("rateLimitGlobal", RETRY_WAIT_BUDGET_UNIT_MS)).toBe(true); + } + + // Without this the carry would grant sub-unit waits forever and the + // retry loop could never terminate. + expect(tracker.consumeWait("rateLimitGlobal", 1)).toBe(false); + expect(tracker.consumeWait("rateLimitGlobal", 0)).toBe(false); + }); + + it("keeps per-bucket carries independent", () => { + const tracker = balanced(); + const waitMs = RETRY_WAIT_BUDGET_UNIT_MS / 2; + + expect(tracker.consumeWait("rateLimitGlobal", waitMs)).toBe(true); + expect(tracker.consumeWait("rateLimitShort", waitMs)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(0); + expect(tracker.getUsage().rateLimitShort).toBe(0); + + expect(tracker.consumeWait("rateLimitGlobal", waitMs)).toBe(true); + expect(tracker.getUsage().rateLimitGlobal).toBe(1); + expect(tracker.getUsage().rateLimitShort).toBe(0); + }); + + it("treats a zero-limit bucket as immediately exhausted", () => { + const tracker = new RetryBudgetTracker( + resolveRetryBudgetLimits("balanced", { rateLimitGlobal: 0 }), + ); + expect(tracker.consumeWait("rateLimitGlobal", 1)).toBe(false); + }); + }); + it("clones constructor limits to avoid external mutation", () => { const limits: RetryBudgetLimits = { authRefresh: 1, diff --git a/test/test-home-isolation.test.ts b/test/test-home-isolation.test.ts new file mode 100644 index 00000000..4e0c4160 --- /dev/null +++ b/test/test-home-isolation.test.ts @@ -0,0 +1,248 @@ +import { describe, it, expect } from "vitest"; +import { existsSync, mkdtempSync, mkdirSync, rmSync } from "node:fs"; +import { homedir, tmpdir, userInfo } from "node:os"; +import { isAbsolute, join, relative, resolve } from "node:path"; +import { LOG_DIR } from "../lib/logger.js"; +import { ACCOUNTS_FILE_NAME } from "../lib/constants.js"; +import { loadAccounts, saveAccounts } from "../lib/storage/load-save.js"; +import { getConfigDir } from "../lib/storage/paths.js"; +import { + getStoragePath, + setStoragePath, + setStoragePathDirect, +} from "../lib/storage/state.js"; +import { MINTED_HOME_PREFIX, teardown } from "./global-setup.js"; + +function realUserHome(): string { + return userInfo().homedir; +} + +/** + * The real account pool lives in `~/.opencode`, not across the whole home tree. + * + * Isolation cannot be expressed as "outside the real home": on Windows + * `tmpdir()` sits under the user profile, so the sandbox is legitimately a + * descendant of it and that assertion would fail while isolation was working + * perfectly. Containment in the sandbox, and separation from the real store, + * hold on every platform. + */ +function realStoreDir(): string { + return resolve(realUserHome(), ".opencode"); +} + +function sandboxHome(): string { + const home = process.env.OC_CODEX_TEST_HOME; + if (!home) { + throw new Error("OC_CODEX_TEST_HOME is unset; vitest.config.ts must mint a sandbox home"); + } + return home; +} + +function isUnder(baseDir: string, targetPath: string): boolean { + const rel = relative(resolve(baseDir), resolve(targetPath)); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); +} + +function expectSandboxed(path: string): void { + expect(isUnder(sandboxHome(), path)).toBe(true); + expect(isUnder(realStoreDir(), path)).toBe(false); +} + +describe("test home isolation", () => { + it("redirects homedir() into the sandbox", () => { + expect(homedir()).toBe(sandboxHome()); + expect(resolve(homedir())).not.toBe(resolve(realUserHome())); + }); + + it("keeps every resolved account storage path inside the sandbox", () => { + setStoragePath(null); + expectSandboxed(getStoragePath()); + + // Per-project storage is namespaced under `getConfigDir()` rather than + // under the project itself, so this follows the redirected home even + // when the checkout sits inside the real one, as it does on CI. + setStoragePath(process.cwd()); + expectSandboxed(getStoragePath()); + + setStoragePath(null); + expectSandboxed(getConfigDir()); + }); + + // LOG_DIR is captured at module scope from homedir(), so it only lands in + // the sandbox if the override beat the import. That is the property a + // setupFiles entry cannot provide and this whole mechanism exists for. + it("beats module-scope homedir() capture", () => { + expectSandboxed(LOG_DIR); + }); + + // Where the sandbox is a descendant of the real home, a guard keyed on the + // home tree alone would refuse this and redden the whole Windows job. + it("still allows account writes inside the sandbox", async () => { + // A private path, not the sandbox's own global store: every test file + // shares this HOME, so writing the real one would hand another file an + // empty pool mid-run. + const probeDir = join(sandboxHome(), "sandbox-write-probe"); + const probe = join(probeDir, ACCOUNTS_FILE_NAME); + setStoragePathDirect(probe); + try { + expectSandboxed(probe); + await expect( + saveAccounts({ version: 3, accounts: [], activeIndex: 0 }), + ).resolves.toBeUndefined(); + expect(existsSync(probe)).toBe(true); + } finally { + setStoragePathDirect(null); + rmSync(probeDir, { recursive: true, force: true }); + } + }); + + it("refuses to write account storage that escapes into the real home", async () => { + const escaped = resolve(realUserHome(), ".opencode", ACCOUNTS_FILE_NAME); + setStoragePathDirect(escaped); + try { + await expect( + saveAccounts({ version: 3, accounts: [], activeIndex: 0 }), + ).rejects.toMatchObject({ code: "TEST_HOME_ESCAPE" }); + } finally { + setStoragePathDirect(null); + } + }); + + // Guarding writes is not sufficient. A missing project store sends the + // loader to the GLOBAL one, whose path is re-resolved from `homedir()` at + // that moment, so a test that restores the real HOME reads the live pool + // through a path the current-storage check already waved through. + it("refuses to read the global fallback out of the real home", async () => { + const projectRoot = join(sandboxHome(), "fallback-probe-project"); + mkdirSync(join(projectRoot, ".opencode"), { recursive: true }); + setStoragePath(projectRoot); + expectSandboxed(getStoragePath()); + + // A directory that does not exist, never the real store: should this + // guard ever regress, the test has to fail rather than read credentials. + const restoredHome = join(realUserHome(), `.oc-codex-guard-probe-${process.pid}`); + const previousHome = process.env.HOME; + const previousProfile = process.env.USERPROFILE; + process.env.HOME = restoredHome; + process.env.USERPROFILE = restoredHome; + try { + expect(isUnder(realUserHome(), getConfigDir())).toBe(true); + await expect(loadAccounts()).rejects.toMatchObject({ + code: "TEST_HOME_ESCAPE", + }); + } finally { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + if (previousProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = previousProfile; + setStoragePath(null); + rmSync(projectRoot, { recursive: true, force: true }); + } + }); +}); + +describe("test home teardown", () => { + // Every case drives `teardown` against a directory this test made, never + // against the live run's own home, so a broken gate can only destroy scratch. + const runTeardown = async ( + home: string | undefined, + owned: boolean, + ): Promise => { + const previousHome = process.env.OC_CODEX_TEST_HOME; + const previousOwned = process.env.OC_CODEX_TEST_HOME_OWNED; + if (home === undefined) delete process.env.OC_CODEX_TEST_HOME; + else process.env.OC_CODEX_TEST_HOME = home; + if (owned) process.env.OC_CODEX_TEST_HOME_OWNED = "1"; + else delete process.env.OC_CODEX_TEST_HOME_OWNED; + try { + await teardown(); + } finally { + if (previousHome === undefined) delete process.env.OC_CODEX_TEST_HOME; + else process.env.OC_CODEX_TEST_HOME = previousHome; + if (previousOwned === undefined) delete process.env.OC_CODEX_TEST_HOME_OWNED; + else process.env.OC_CODEX_TEST_HOME_OWNED = previousOwned; + } + }; + + const mintedLookalike = (): string => + mkdtempSync(join(tmpdir(), MINTED_HOME_PREFIX)); + + // `vitest.config.ts` spells the prefix as a literal and this module exports + // it as a constant. Were the two to drift apart, teardown would simply stop + // matching the home the config minted and the leak would return silently. + it("agrees with the prefix the config actually minted", () => { + if (process.env.OC_CODEX_TEST_HOME_OWNED !== "1") return; + const home = process.env.OC_CODEX_TEST_HOME as string; + expect(resolve(home).startsWith(resolve(tmpdir(), MINTED_HOME_PREFIX))).toBe(true); + }); + + it("removes a home it minted itself", async () => { + const home = mintedLookalike(); + expect(existsSync(home)).toBe(true); + await runTeardown(home, true); + expect(existsSync(home)).toBe(false); + }); + + // Identical path shape to the case above, so the ownership flag is the only + // thing left deciding it. A home handed in through the environment belongs + // to whoever set it. + it("keeps an inherited home that looks exactly like a minted one", async () => { + const home = mintedLookalike(); + try { + await runTeardown(home, false); + expect(existsSync(home)).toBe(true); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + // The case above only holds while nothing sets the flag for an inherited + // home. A caller exporting both variables would otherwise have its own + // directory recursively deleted, so the config must clear what it inherits. + it("clears an inherited ownership flag rather than trusting it", async () => { + const previousOwned = process.env.OC_CODEX_TEST_HOME_OWNED; + process.env.OC_CODEX_TEST_HOME_OWNED = "1"; + try { + // Re-runs the config's env setup. OC_CODEX_TEST_HOME is already set, + // so it takes the inherited branch and mints no directory. + await import("../vitest.config.js"); + expect(process.env.OC_CODEX_TEST_HOME_OWNED).toBeUndefined(); + } finally { + if (previousOwned === undefined) delete process.env.OC_CODEX_TEST_HOME_OWNED; + else process.env.OC_CODEX_TEST_HOME_OWNED = previousOwned; + } + }); + + it("keeps a path outside the temp directory", async () => { + const scratchRoot = join(process.cwd(), "tmp"); + mkdirSync(scratchRoot, { recursive: true }); + const outside = mkdtempSync(join(scratchRoot, MINTED_HOME_PREFIX)); + try { + await runTeardown(outside, true); + expect(existsSync(outside)).toBe(true); + } finally { + rmSync(outside, { recursive: true, force: true }); + } + }); + + it("keeps a temp path that does not carry the minted prefix", async () => { + const foreign = mkdtempSync(join(tmpdir(), "oc-codex-unrelated-")); + try { + await runTeardown(foreign, true); + expect(existsSync(foreign)).toBe(true); + } finally { + rmSync(foreign, { recursive: true, force: true }); + } + }); + + // An interrupted run can leave the env half-set or the directory already + // gone. Neither may throw, or the failure outlives the run it came from. + it("tolerates a missing home and an already-removed one", async () => { + await expect(runTeardown(undefined, true)).resolves.toBeUndefined(); + + const home = mintedLookalike(); + await runTeardown(home, true); + expect(existsSync(home)).toBe(false); + await expect(runTeardown(home, true)).resolves.toBeUndefined(); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 5499527f..dff3e8e3 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,9 +1,48 @@ import { defineConfig } from 'vitest/config'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** + * Per-run throwaway home. The suite writes real account storage, so without + * this a `npm test` resolves `~/.opencode/oc-codex-multi-auth-accounts.json` + * against the developer's actual home and overwrites live ChatGPT credentials + * with fixtures. + * + * This must be `test.env`, not a `setupFiles` entry: vitest applies `test.env` + * before the worker imports any test module, and `lib/config.ts`, + * `lib/accounts/recovery.ts` and `lib/logger.ts` capture `homedir()` at module + * scope, so anything later than import time is too late for them. + */ +const inheritedHome = process.env.OC_CODEX_TEST_HOME; +const isolatedHome = + inheritedHome ?? mkdtempSync(join(tmpdir(), 'oc-codex-multi-auth-test-home-')); +process.env.OC_CODEX_TEST_HOME = isolatedHome; +// Only a home this config minted may be removed once the run ends. One handed +// in through the environment belongs to whoever set it. +if (!inheritedHome) process.env.OC_CODEX_TEST_HOME_OWNED = '1'; +else delete process.env.OC_CODEX_TEST_HOME_OWNED; export default defineConfig({ test: { globals: true, environment: 'node', + env: { + HOME: isolatedHome, + USERPROFILE: isolatedHome, + OC_CODEX_TEST_HOME: isolatedHome, + // The OS credential store is the one place the HOME redirect cannot + // reach. An inherited opt-in would route fixture writes into the + // developer's real keychain; tests that need the backend opt in per test. + CODEX_KEYCHAIN: '0', + }, + // Four suites import the real `index.ts`, and the first one scheduled pays + // the transform of a 4900-line entry plus its dependency graph: measured at + // 3.2s-6.7s on an idle machine, against a 5s default. Whichever suite loses + // that race times out under full-suite CPU contention, which is flakiness in + // the harness rather than in any assertion (a warm re-import costs ~400ms). + testTimeout: 15_000, + globalSetup: ['./test/global-setup.ts'], include: ['test/**/*.test.ts'], exclude: [ 'node_modules/**',