diff --git a/README.md b/README.md index 11b4e230..7ee602ef 100644 --- a/README.md +++ b/README.md @@ -265,7 +265,7 @@ Most of these also run as a **direct CLI** with no agent or model involvement, s - unsupported-model handling is strict by default, with opt-in fallback controls - TUI quota status follows the account/workspace used by the latest request - Business workspace memberships and Personal accounts keep separate usage and quota windows. Business members sharing one workspace are distinguished by their member/seat identity, so their usage is not collapsed into one row. -- An account identifies itself by its own ChatGPT email and the last 6 characters of its account id, with the email masked when `maskEmail` is on. The OAuth id_token also lists the API-platform organizations the login belongs to; those are not ChatGPT workspaces and are never used to name an account, so logging in clears a label left behind by one. A label you set with `codex-label` is always kept. +- An account identifies itself by its own ChatGPT email and the last 6 characters of its account id, with the email masked when `maskEmail` is on. An account id names a ChatGPT workspace and every member of a Business workspace shares it, so a record that also carries a member/seat id prints a short excerpt of that as `seat:`. The excerpt is a 6-character tail where that is enough to tell the listed accounts apart. Where it is not, it widens, moves to where those ids first differ, or joins two short excerpts with `..` - real member ids are long, share a leading prefix, and differ in more than one place, so a tail alone often cannot separate them. Where no excerpt that short can separate them, `seat:` is instead an **opaque hash prefix** such as `719f78b5`: it identifies the seat and stays stable, but it is not part of the member id and cannot be matched against anything ChatGPT shows you. Whichever form it takes, two distinct seats never render the same `seat:` and a `seat:` is never longer than 32 characters. A record with no member id renders exactly as before. The OAuth id_token also lists the API-platform organizations the login belongs to; those are not ChatGPT workspaces and are never used to name an account, so logging in clears a label left behind by one. A label you set with `codex-label` is always kept. - The ChatGPT plan (`Free`, `Plus`, `Pro`, `Business`, `Business Premium`, `Enterprise`) is read from the access token, refreshed on every token refresh, and shown by `codex-list` and `codex-status`. `codex-limits` and the TUI read the plan live from the usage endpoint and name it the same way. An unrecognized plan is reported verbatim rather than renamed. --- diff --git a/index.ts b/index.ts index 1cb71259..d5bdaa87 100644 --- a/index.ts +++ b/index.ts @@ -144,7 +144,8 @@ import { matchesModelPoolAccountKey, type ModelPoolAccount, } from "./lib/accounts/pool-identity.js"; -import { resolveDisplayEmail } from "./lib/account-display.js"; +import { formatSeatSuffix, resolveDisplayEmail } from "./lib/account-display.js"; +import { extractAccountUserId } from "./lib/auth/token-utils.js"; import { CodexAuthError } from "./lib/errors.js"; import { getStoragePath, @@ -394,6 +395,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(), @@ -469,11 +479,13 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { account?: { email?: string; accountId?: string; + accountUserId?: string; accountLabel?: string; accountTags?: string[]; accountNote?: string; }; label?: string; + peerAccounts?: readonly ({ accountUserId?: string } | undefined)[]; } = {}, ): Record => ({ index: index + 1, @@ -481,7 +493,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { ...(options.includeSensitive ? { label: - options.label ?? formatCommandAccountLabel(options.account, index), + options.label ?? + formatCommandAccountLabel(options.account, index, { + peerAccounts: options.peerAccounts, + }), email: options.account?.email ?? null, accountId: options.account?.accountId ?? null, } @@ -1264,16 +1279,30 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { account: { email?: string; accountId?: string; + accountUserId?: string; accountLabel?: string; accountTags?: string[]; accountNote?: string; } | undefined, index: number, - options: { maskEmail?: boolean } = {}, + options: { + maskEmail?: boolean; + peerAccounts?: readonly ({ accountUserId?: string } | undefined)[]; + omitSeat?: boolean; + } = {}, ): string => { const email = resolveDisplayEmail(account?.email, options.maskEmail ?? false); const workspace = account?.accountLabel?.trim(); const accountId = formatAccountIdForDisplay(account?.accountId); + // `omitSeat` is for a caller that renders the seat itself in a place + // a long email cannot push it out of - a table column of its own. + // Leaving it in the label too would print the seat twice. + const seat = options.omitSeat + ? undefined + : formatSeatSuffix( + account?.accountUserId, + options.peerAccounts?.map((peer) => peer?.accountUserId), + ); const tags = Array.isArray(account?.accountTags) ? account.accountTags @@ -1285,6 +1314,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (email) details.push(email); if (workspace) details.push(`workspace:${workspace}`); if (accountId) details.push(`id:${accountId}`); + if (seat) details.push(`seat:${seat}`); if (tags.length > 0) details.push(`tags:${tags.join(",")}`); if (details.length === 0) { @@ -1326,7 +1356,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const maskEmail = resolveMaskEmail(); const selected = await select( storage.accounts.map((account, index) => ({ - label: formatCommandAccountLabel(account, index, { maskEmail }), + label: formatCommandAccountLabel(account, index, { + maskEmail, + peerAccounts: storage.accounts, + }), value: index, })), { @@ -1351,7 +1384,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { ): BeginnerAccountSnapshot[] => { return storage.accounts.map((account, index) => ({ index, - label: formatCommandAccountLabel(account, index), + label: formatCommandAccountLabel(account, index, { + peerAccounts: storage.accounts, + }), accountLabel: account.accountLabel, enabled: account.enabled !== false, isActive: index === activeIndex, @@ -1675,6 +1710,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 +1772,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 +1813,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 +1868,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 +1898,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 +1912,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 +1939,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 +2514,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 +2579,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 +2615,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 +2625,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(); @@ -2777,6 +2951,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const failures = await accountManager.incrementAuthFailures(account); const accountLabel = formatAccountLabel(account, account.index, { maskEmail: maskEmailEnabled, + peerAccounts: accountManager.getAccountsSnapshot(), }); if (failures >= ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_REMOVAL) { @@ -2859,6 +3034,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { ) { const accountLabel = formatAccountLabel(account, account.index, { maskEmail: maskEmailEnabled, + peerAccounts: accountManager.getAccountsSnapshot(), }); await showToast( `Using ${accountLabel} (${account.index + 1}/${accountCount})`, @@ -3119,6 +3295,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (workspaceDeactivated) { const accountLabel = formatAccountLabel(account, account.index, { maskEmail: maskEmailEnabled, + peerAccounts: accountManager.getAccountsSnapshot(), }); accountManager.refundToken(account, modelFamily, model); accountManager.recordFailure(account, modelFamily, model); @@ -3434,6 +3611,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (isInvalidatedAuthTokenError(errorBody, response.status)) { const accountLabel = formatAccountLabel(account, account.index, { maskEmail: maskEmailEnabled, + peerAccounts: accountManager.getAccountsSnapshot(), }); accountManager.refundToken(account, modelFamily, model); accountManager.recordFailure(account, modelFamily, model); @@ -3795,10 +3973,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; } @@ -4136,9 +4320,20 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (deepProbe) { ok += 1; + // Both read from the probed token, so the pair is the seat + // the credential actually belongs to. The workspace id + // alone repeats across every member of a Business + // workspace and cannot confirm which seat answered. + const tokenSeat = formatSeatSuffix( + extractAccountUserId(accessToken), + ); + const identity = [ + tokenAccountId ? `id:${tokenAccountId.slice(-6)}` : undefined, + tokenSeat ? `seat:${tokenSeat}` : undefined, + ].filter((part): part is string => part !== undefined); const detail = - tokenAccountId - ? `${authDetail} (id:${tokenAccountId.slice(-6)})` + identity.length > 0 + ? `${authDetail} (${identity.join(", ")})` : authDetail; console.log(`[${i + 1}/${total}] ${label}: ${detail}`); continue; @@ -4490,6 +4685,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } return { accountId: account.accountId, + accountUserId: account.accountUserId, accountLabel: account.accountLabel, email: account.email, index, diff --git a/lib/account-display.ts b/lib/account-display.ts index a9d9f4ef..193afa24 100644 --- a/lib/account-display.ts +++ b/lib/account-display.ts @@ -12,6 +12,8 @@ * user-defined account label when one exists. */ +import { createHash } from "node:crypto"; + /** * Mask an email for display while preserving the domain so collisions between * accounts on the same provider remain distinguishable. @@ -47,3 +49,237 @@ export function resolveDisplayEmail( if (!trimmed) return undefined; return maskEmail ? maskEmailForDisplay(trimmed) : trimmed; } + +const SEAT_SUFFIX_MIN_LENGTH = 6; +/** + * Hard ceiling on a rendered seat, independent of how long the member id is. + * + * A seat sits in a table column beside the account, so its width may not be a + * function of how long the backend's ids happen to be. Member ids measured in + * a real multi-seat Business pool are 67 characters with no shared tail, so a + * search that stops when the ids are separated rather than when it runs out of + * room prints most of the id in every row. + */ +const SEAT_RENDER_MAX_LENGTH = 12; +/** + * Hash prefix lengths for the last-resort renderer. 32 hex characters is 128 + * bits of SHA-256, so the list is exhausted only by a collision that cannot be + * reached with ids a backend hands out. + */ +const SEAT_HASH_LENGTHS: readonly number[] = [8, 12, 16, 24, 32]; +/** Marks the gap between two excerpts, as `accountId` already elides with `...`. */ +const SEAT_WINDOW_SEPARATOR = ".."; + +function normalizeSeatIdentity(accountUserId: string | undefined): string | undefined { + const trimmed = accountUserId?.trim(); + return trimmed && trimmed.length > 0 ? trimmed : undefined; +} + +function sliceSeatSuffix(accountUserId: string, length: number): string { + return accountUserId.length > length ? accountUserId.slice(-length) : accountUserId; +} + +/** + * `length` characters starting at `start`, slid left when the id is too short + * to hold that window whole. Never padded: a short id renders as itself. + */ +function sliceSeatWindow(accountUserId: string, start: number, length: number): string { + if (accountUserId.length <= length) return accountUserId; + const begin = Math.max(0, Math.min(start, accountUserId.length - length)); + return accountUserId.slice(begin, begin + length); +} + +function hashSeatIdentity(accountUserId: string, length: number): string { + return createHash("sha256").update(accountUserId).digest("hex").slice(0, length); +} + +/** Index of the first character at which the given ids are not all equal. */ +function commonPrefixLength(values: readonly string[]): number { + const [first] = values; + if (first === undefined) return 0; + let shared = first.length; + for (const value of values) { + let index = 0; + while (index < shared && index < value.length && first[index] === value[index]) { + index += 1; + } + shared = index; + if (shared === 0) break; + } + return shared; +} + +/** First index at which two ids differ, or their shared length if one prefixes the other. */ +function firstDivergence(left: string, right: string): number { + const limit = Math.min(left.length, right.length); + let index = 0; + while (index < limit && left[index] === right[index]) index += 1; + return index; +} + +/** + * For every pair of ids, the first index at which that pair differs. + * + * Deliberately not "every index where the ids disagree": across a handful of + * random-looking ids that is nearly every index, which localizes nothing. A + * pair is told apart by any excerpt covering its first divergence, so an + * excerpt covering all of these positions tells every pair apart. + */ +function divergenceAnchors(values: readonly string[]): number[] { + const anchors = new Set(); + for (let left = 0; left < values.length; left += 1) { + for (let right = left + 1; right < values.length; right += 1) { + const leftValue = values[left]; + const rightValue = values[right]; + if (leftValue === undefined || rightValue === undefined) continue; + anchors.add(firstDivergence(leftValue, rightValue)); + } + } + return [...anchors].sort((left, right) => left - right); +} + +/** Starts of `width`-wide windows covering `anchors`, dropping those an earlier window already spans. */ +function anchorWindowStarts(anchors: readonly number[], width: number): number[] { + const starts: number[] = []; + for (const anchor of anchors) { + const last = starts[starts.length - 1]; + if (last !== undefined && anchor < last + width) continue; + starts.push(anchor); + } + return starts; +} + +/** + * A renderer that gives every distinct member id in `accountUserIds` a + * different string, short enough to sit in a column beside the account. + * + * Four strategies, each capped, tried in order: + * + * 1. A tail. This is what the surfaces already print for `accountId`, so it + * is preferred wherever it works, which is wherever the ids differ near + * their end. + * 2. One window anchored where the ids first diverge, for ids that share a + * long tail. + * 3. Short windows at each position where some pair first differs, joined by + * `..`. Member ids in the one real multi-seat Business pool this was + * measured against diverge in more than one place - clusters 26 characters + * apart - so no single capped window separates them, and joining excerpts + * is what keeps the seat both bounded and readable off the id. + * 4. A SHA-256 prefix. This is NOT an unreachable branch kept for tidiness: + * it is what remains when the divergences are too many or too spread out + * for (3) to cover inside the cap. What it prints is opaque - it cannot be + * matched against the id by eye - so any surface documenting the seat has + * to say this outcome exists. + * + * Returning the id whole is kept as the final fallback so two distinct ids can + * never render alike; reaching it needs a 128-bit SHA-256 prefix collision. + */ +function resolveSeatRenderer( + accountUserIds: readonly (string | undefined)[], +): (accountUserId: string) => string { + const distinct: string[] = []; + const seen = new Set(); + for (const accountUserId of accountUserIds) { + const normalized = normalizeSeatIdentity(accountUserId); + if (!normalized || seen.has(normalized)) continue; + seen.add(normalized); + distinct.push(normalized); + } + const tailAtMinLength = (accountUserId: string) => + sliceSeatSuffix(accountUserId, SEAT_SUFFIX_MIN_LENGTH); + if (distinct.length <= 1) return tailAtMinLength; + + const separates = (render: (accountUserId: string) => string): boolean => + new Set(distinct.map(render)).size === distinct.length; + + for (let length = SEAT_SUFFIX_MIN_LENGTH; length <= SEAT_RENDER_MAX_LENGTH; length += 1) { + const render = (accountUserId: string) => sliceSeatSuffix(accountUserId, length); + if (separates(render)) return render; + } + + const start = commonPrefixLength(distinct); + for (let length = SEAT_SUFFIX_MIN_LENGTH; length <= SEAT_RENDER_MAX_LENGTH; length += 1) { + const render = (accountUserId: string) => sliceSeatWindow(accountUserId, start, length); + if (separates(render)) return render; + } + + const anchors = divergenceAnchors(distinct); + for (let width = 2; width <= SEAT_RENDER_MAX_LENGTH; width += 1) { + const starts = anchorWindowStarts(anchors, width); + const rendered = + starts.length * width + (starts.length - 1) * SEAT_WINDOW_SEPARATOR.length; + // Skipped, not abandoned: a wider window can span two nearby anchors + // that needed one window each, so the cost falls as the window count + // does. Anchors at {5,6,7,31,32,33} cost 14 at width 2 (four windows) + // and 8 at width 3 (two), so giving up at the first overflow loses a + // rendering that fits comfortably. + if (rendered > SEAT_RENDER_MAX_LENGTH) continue; + const render = (accountUserId: string) => + starts + .map((windowStart) => accountUserId.slice(windowStart, windowStart + width)) + .join(SEAT_WINDOW_SEPARATOR); + if (separates(render)) return render; + } + + for (const length of SEAT_HASH_LENGTHS) { + const render = (accountUserId: string) => hashSeatIdentity(accountUserId, length); + if (separates(render)) return render; + } + + return (accountUserId: string) => accountUserId; +} + +/** + * Render the short seat suffix that separates two accounts sharing one + * workspace `accountId`. + * + * A ChatGPT Business workspace is a single `accountId` shared by every member + * of it; `accountUserId` is that member's own id and the only stored field + * that tells their seats apart. Upstream meters each seat separately - its own + * quota, its own weekly reset - so seats sharing a workspace are distinct + * accounts, not copies of one. + * + * Every display surface used to render `accountId` alone, so four members of + * one Business workspace printed an identical `id:` string and read as the + * same account duplicated four times. Appending this suffix is what makes the + * rendered rows match the accounts they describe. + * + * A six-character tail by default, matching what the surfaces already print + * for `accountId`. Six characters are not an identity on their own - member + * ids sharing a six-character tail were observed, which is the same false + * "these are duplicates" reading this suffix exists to prevent - so pass + * `peerAccountUserIds` (the other accounts rendered alongside this one) and + * {@link resolveSeatRenderer} picks a rendering that separates them inside + * {@link SEAT_RENDER_MAX_LENGTH}: a wider or relocated excerpt of the id + * where one fits, and an opaque hash prefix where none does. + * + * Returns `undefined` when there is no member id, so a token-only record + * renders exactly as it did before. + */ +export function formatSeatSuffix( + accountUserId: string | undefined, + peerAccountUserIds?: readonly (string | undefined)[], +): string | undefined { + const trimmed = normalizeSeatIdentity(accountUserId); + if (!trimmed) return undefined; + if (!peerAccountUserIds) return sliceSeatSuffix(trimmed, SEAT_SUFFIX_MIN_LENGTH); + // This id joins the set the rendering is chosen against, so the guarantee + // holds even for a caller whose peer list is the OTHER accounts rather than + // all of them. Already being there makes it a no-op. + return resolveSeatRenderer([...peerAccountUserIds, trimmed])(trimmed); +} + +/** + * Seat suffixes for a whole rendered set, all built the same way so the rows + * line up and no two distinct member ids share a rendering. Entries without a + * member id come back `undefined`, holding their position. + */ +export function resolveSeatSuffixes( + accountUserIds: readonly (string | undefined)[], +): (string | undefined)[] { + const render = resolveSeatRenderer(accountUserIds); + return accountUserIds.map((accountUserId) => { + const trimmed = normalizeSeatIdentity(accountUserId); + return trimmed ? render(trimmed) : undefined; + }); +} diff --git a/lib/accounts.ts b/lib/accounts.ts index c07c2a0a..70e27b68 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -31,7 +31,7 @@ import { import { formatWaitTime, type RateLimitReason } from "./accounts/rate-limits.js"; import { nowMs } from "./utils.js"; import { logWarn } from "./logger.js"; -import { resolveDisplayEmail } from "./account-display.js"; +import { formatSeatSuffix, resolveDisplayEmail } from "./account-display.js"; export type { AccountSelectionExplainability, ManagedAccount } from "./accounts/state.js"; @@ -424,9 +424,14 @@ export class AccountManager { } export function formatAccountLabel( - account: { email?: string; accountId?: string; accountLabel?: string } | undefined, + account: + | { email?: string; accountId?: string; accountUserId?: string; accountLabel?: string } + | undefined, index: number, - options: { maskEmail?: boolean } = {}, + options: { + maskEmail?: boolean; + peerAccounts?: readonly ({ accountUserId?: string } | undefined)[]; + } = {}, ): string { const accountLabel = account?.accountLabel?.trim(); const email = resolveDisplayEmail(account?.email, options.maskEmail ?? false); @@ -436,17 +441,28 @@ export function formatAccountLabel( ? accountId.slice(-6) : accountId : null; - - if (accountLabel && email && idSuffix) { - return `Account ${index + 1} (${accountLabel}, ${email}, id:${idSuffix})`; - } - if (accountLabel && email) return `Account ${index + 1} (${accountLabel}, ${email})`; - if (accountLabel && idSuffix) return `Account ${index + 1} (${accountLabel}, id:${idSuffix})`; - if (accountLabel) return `Account ${index + 1} (${accountLabel})`; - if (email && idSuffix) return `Account ${index + 1} (${email}, id:${idSuffix})`; - if (email) return `Account ${index + 1} (${email})`; - if (idSuffix) return `Account ${index + 1} (${idSuffix})`; - return `Account ${index + 1}`; + // `accountId` names the workspace, which every member of a Business + // workspace shares. Without the seat, four distinct members printed one + // identical `id:` string (see `formatSeatSuffix`). + const seatSuffix = formatSeatSuffix( + account?.accountUserId, + options.peerAccounts?.map((peer) => peer?.accountUserId), + ); + + const details: string[] = []; + if (accountLabel) details.push(accountLabel); + if (email) details.push(email); + if (idSuffix) { + // The id has always rendered bare when it is the only thing known about + // an account. Once a seat sits beside it, an unprefixed pair of suffixes + // would not say which is which, so the prefix goes back on. + const idIsOnlyDetail = !accountLabel && !email && !seatSuffix; + details.push(idIsOnlyDetail ? idSuffix : `id:${idSuffix}`); + } + if (seatSuffix) details.push(`seat:${seatSuffix}`); + + if (details.length === 0) return `Account ${index + 1}`; + return `Account ${index + 1} (${details.join(", ")})`; } export function formatCooldown( diff --git a/lib/auth/login-runner.ts b/lib/auth/login-runner.ts index 50b5ab3a..7367145e 100644 --- a/lib/auth/login-runner.ts +++ b/lib/auth/login-runner.ts @@ -433,6 +433,12 @@ export async function resolveAndPersistAccountSelection( return persistResolvedAccountSelection(selection, options); } +function formatIdentitySuffix(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + if (!trimmed) return undefined; + return trimmed.length > 6 ? trimmed.slice(-6) : trimmed; +} + /** * Persists login results through the shared storage transaction so overlapping * login retries serialize their read-modify-write cycle instead of racing stale @@ -743,6 +749,13 @@ export async function persistAccountPool( let identityIndexes = buildIdentityIndexes(); + // Whether a login landed on an existing record or appended a new one is + // only knowable here, but the slot it ends up in is only final after the + // prune below, so the decision is recorded now and reported there. Keyed + // by refresh token: the login just wrote it, and a merge keeps the newest + // record's token, so the key still finds the row that survived. + const loginOutcomes: { refreshToken: string; added: boolean }[] = []; + for (const result of results) { const accountId = result.accountIdOverride ?? extractAccountId(result.access); const normalizedAccountId = accountId?.trim() || undefined; @@ -855,6 +868,7 @@ export async function persistAccountPool( addedAt: now, lastUsed: now, }); + loginOutcomes.push({ refreshToken: result.refresh, added: true }); identityIndexes = buildIdentityIndexes(); continue; } @@ -902,6 +916,7 @@ export async function persistAccountPool( oauthScope: normalizedScope ?? existing.oauthScope, lastUsed: now, }; + loginOutcomes.push({ refreshToken: result.refresh, added: false }); identityIndexes = buildIdentityIndexes(); } @@ -923,7 +938,20 @@ export async function persistAccountPool( const accountUserId = account?.accountUserId?.trim() ?? ""; const email = account?.email?.trim().toLowerCase() ?? ""; const refreshToken = account?.refreshToken?.trim() ?? ""; - if (organizationId || accountId || accountUserId) { + // A member id pins one seat of one workspace, so two records + // carrying it are the same seat and the newer one supersedes the + // older. The refresh token is left out because a re-login mints a + // new one: keying on it meant the single case this prune exists + // to collapse was the one case that could never collide. + if (accountUserId) { + return `org:${organizationId}|account:${accountId}|member:${accountUserId}`; + } + // No member id, so the seat is unknown. Two records under one + // workspace id, like two sharing only an email, can be two + // different members whose seat was never recorded - so both keep + // the token that tells them apart rather than risk merging two + // live accounts into one. + if (organizationId || accountId) { return `org:${organizationId}|account:${accountId}|member:${accountUserId}|refresh:${refreshToken}`; } return `email:${email}|refresh:${refreshToken}`; @@ -1001,6 +1029,63 @@ export async function persistAccountPool( if (accounts.length === 0) return; + // A login that lands on a seat the store never held is indistinguishable + // from one that repaired an existing seat unless it says which it did. + // The workspace/email neighbours are named because that is the line that + // distinguishes "this replaced your exhausted account" from "this added a + // ninth account beside it". Slots only - an email is never printed here, + // matching every other identity surface. + const describeSlots = (indexes: number[]): string => + indexes.map((slot) => `Account ${slot + 1}`).join(", "); + + for (const outcome of loginOutcomes) { + const index = accounts.findIndex( + (account) => account?.refreshToken === outcome.refreshToken, + ); + if (index < 0) continue; + const account = accounts[index]; + if (!account) continue; + + const identityParts: string[] = []; + const idSuffix = formatIdentitySuffix(account.accountId); + const seatSuffix = formatIdentitySuffix(account.accountUserId); + if (idSuffix) identityParts.push(`id:${idSuffix}`); + if (seatSuffix) identityParts.push(`seat:${seatSuffix}`); + const identity = identityParts.length > 0 ? ` (${identityParts.join(", ")})` : ""; + + if (!outcome.added) { + logInfo( + `Login updated Account ${index + 1}${identity} in place - an account already in the store.`, + ); + continue; + } + + const workspaceId = account.accountId?.trim(); + const email = sanitizeEmail(account.email); + const sameWorkspace: number[] = []; + const sameEmail: number[] = []; + for (let i = 0; i < accounts.length; i += 1) { + if (i === index) continue; + const other = accounts[i]; + if (!other) continue; + if (workspaceId && other.accountId?.trim() === workspaceId) sameWorkspace.push(i); + if (email && sanitizeEmail(other.email) === email) sameEmail.push(i); + } + + const notes: string[] = []; + if (sameWorkspace.length > 0) { + notes.push(`Same workspace id as ${describeSlots(sameWorkspace)}.`); + } + if (sameEmail.length > 0) { + notes.push(`Same email as ${describeSlots(sameEmail)}.`); + } + logInfo( + `Login added Account ${index + 1}${identity} as a NEW account - it was not in the store, so it repaired no existing account.${ + notes.length > 0 ? ` ${notes.join(" ")}` : "" + }`, + ); + } + const resolveIndexByIdentityKeys = (identityKeys: string[] | undefined): number | undefined => { if (!identityKeys || identityKeys.length === 0) return undefined; for (const identityKey of identityKeys) { diff --git a/lib/cli.ts b/lib/cli.ts index 680ec70c..9b251cce 100644 --- a/lib/cli.ts +++ b/lib/cli.ts @@ -1,7 +1,7 @@ import { createInterface } from "node:readline/promises"; import { stdin as input, stdout as output } from "node:process"; import type { AccountIdSource } from "./types.js"; -import { resolveDisplayEmail } from "./account-display.js"; +import { formatSeatSuffix, resolveDisplayEmail } from "./account-display.js"; import { showAuthMenu, showAccountDetails, @@ -51,6 +51,7 @@ export type LoginMode = export interface ExistingAccountInfo { accountId?: string; + accountUserId?: string; accountLabel?: string; email?: string; index: number; @@ -77,7 +78,10 @@ export interface LoginMenuResult { function formatAccountLabel( account: ExistingAccountInfo, index: number, - options: { maskEmail?: boolean } = {}, + options: { + maskEmail?: boolean; + peerAccounts?: readonly ExistingAccountInfo[]; + } = {}, ): string { const num = index + 1; const label = account.accountLabel?.trim(); @@ -87,10 +91,15 @@ function formatAccountLabel( accountId && accountId.length > 14 ? `${accountId.slice(0, 8)}...${accountId.slice(-6)}` : accountId; + const seatSuffix = formatSeatSuffix( + account.accountUserId, + options.peerAccounts?.map((peer) => peer.accountUserId), + ); const details: string[] = []; if (email) details.push(email); if (label) details.push(`workspace:${label}`); if (accountIdDisplay) details.push(`id:${accountIdDisplay}`); + if (seatSuffix) details.push(`seat:${seatSuffix}`); if (details.length > 0) { return `${num}. ${details.join(" | ")}`; } @@ -116,7 +125,9 @@ async function promptLoginModeFallback( if (existingAccounts.length > 0) { console.log(`\n${existingAccounts.length} account(s) saved:`); for (const account of existingAccounts) { - console.log(` ${formatAccountLabel(account, account.index, { maskEmail })}`); + console.log( + ` ${formatAccountLabel(account, account.index, { maskEmail, peerAccounts: existingAccounts })}`, + ); } console.log(""); } @@ -173,7 +184,10 @@ export async function promptLoginMode( case "verify-flagged": return { mode: "verify-flagged" }; case "select-account": { - const accountAction = await showAccountDetails(action.account, { maskEmail }); + const accountAction = await showAccountDetails(action.account, { + maskEmail, + peerAccounts: existingAccounts, + }); if (accountAction === "delete") { return { mode: "manage", deleteAccountIndex: action.account.index }; } 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..ef20c727 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,36 @@ async function checkWorktreeLockForCurrentStorage( } } +/** + * Refuse to mutate 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 write fixtures straight over 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. + */ +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; + + throw new StorageError( + `Refusing to write 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; @@ -537,6 +571,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 +794,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/lib/tools/codex-dashboard.ts b/lib/tools/codex-dashboard.ts index 71b4db2f..b349d7ee 100644 --- a/lib/tools/codex-dashboard.ts +++ b/lib/tools/codex-dashboard.ts @@ -140,6 +140,7 @@ export function createCodexDashboardTool(ctx: ToolContext): ToolDefinition { ...buildJsonAccountIdentity(entry.index, { includeSensitive: includeSensitiveOutput, account: storage.accounts[entry.index], + peerAccounts: storage.accounts, }), eligible: entry.eligible, healthScore: entry.healthScore, @@ -196,7 +197,7 @@ export function createCodexDashboardTool(ctx: ToolContext): ToolDefinition { const label = formatCommandAccountLabel( storage.accounts[entry.index], entry.index, - { maskEmail }, + { maskEmail, peerAccounts: storage.accounts }, ); const state = entry.eligible ? formatUiBadge(ui, "eligible", "success") @@ -252,7 +253,7 @@ export function createCodexDashboardTool(ctx: ToolContext): ToolDefinition { const label = formatCommandAccountLabel( storage.accounts[entry.index], entry.index, - { maskEmail }, + { maskEmail, peerAccounts: storage.accounts }, ); lines.push( ` - ${label}: ${entry.eligible ? "eligible" : "blocked"} | health=${Math.round(entry.healthScore)} | tokens=${entry.tokensAvailable.toFixed(1)} | reasons=${entry.reasons.join(", ")}`, diff --git a/lib/tools/codex-health.ts b/lib/tools/codex-health.ts index 05820175..c3658bb4 100644 --- a/lib/tools/codex-health.ts +++ b/lib/tools/codex-health.ts @@ -94,8 +94,13 @@ export function createCodexHealthTool(ctx: ToolContext): ToolDefinition { const account = storage.accounts[i]; if (!input || !account) continue; - const label = formatCommandAccountLabel(account, i); - const displayLabel = formatCommandAccountLabel(account, i, { maskEmail }); + const label = formatCommandAccountLabel(account, i, { + peerAccounts: storage.accounts, + }); + const displayLabel = formatCommandAccountLabel(account, i, { + maskEmail, + peerAccounts: storage.accounts, + }); const outcome = await refreshAndPersistAccount(input); if (outcome.status === "refreshed") { diff --git a/lib/tools/codex-label.ts b/lib/tools/codex-label.ts index 588f199e..38dad374 100644 --- a/lib/tools/codex-label.ts +++ b/lib/tools/codex-label.ts @@ -163,6 +163,7 @@ export function createCodexLabelTool(ctx: ToolContext): ToolDefinition { const accountLabel = formatCommandAccountLabel(account, targetIndex, { maskEmail, + peerAccounts: accounts, }); return { kind: "ok", accountLabel, previousLabel }; }, diff --git a/lib/tools/codex-limits.ts b/lib/tools/codex-limits.ts index 62daa5e0..bb61f803 100644 --- a/lib/tools/codex-limits.ts +++ b/lib/tools/codex-limits.ts @@ -174,11 +174,12 @@ export function createCodexLimitsTool(ctx: ToolContext): ToolDefinition { const label = formatCommandAccountLabel( effectiveDisplayAccount, displayIndex, + { peerAccounts: storage.accounts }, ); const displayLabel = formatCommandAccountLabel( effectiveDisplayAccount, displayIndex, - { maskEmail }, + { maskEmail, peerAccounts: storage.accounts }, ); const isActive = i === activeIndex || sharesActiveCredential; const activeSuffix = isActive diff --git a/lib/tools/codex-list.ts b/lib/tools/codex-list.ts index a8922be0..65a485ca 100644 --- a/lib/tools/codex-list.ts +++ b/lib/tools/codex-list.ts @@ -6,6 +6,7 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"; import { getStoragePath, loadAccounts } from "../storage.js"; import { formatCooldown } from "../accounts.js"; +import { resolveSeatSuffixes } from "../account-display.js"; import { buildTableHeader, buildTableRow, type TableOptions } from "../table-formatter.js"; import { formatUiBadge, @@ -179,6 +180,7 @@ export function createCodexListTool(ctx: ToolContext): ToolDefinition { ...buildJsonAccountIdentity(index, { includeSensitive: includeSensitiveOutput, account, + peerAccounts: storage.accounts, }), enabled: account.enabled !== false, isActive: index === activeIndex, @@ -210,7 +212,10 @@ export function createCodexListTool(ctx: ToolContext): ToolDefinition { ]; filteredEntries.forEach(({ account, index }) => { - const label = formatCommandAccountLabel(account, index, { maskEmail }); + const label = formatCommandAccountLabel(account, index, { + maskEmail, + peerAccounts: storage.accounts, + }); const badges: string[] = []; if (index === activeIndex) badges.push(formatUiBadge(ui, "current", "accent")); @@ -286,10 +291,31 @@ export function createCodexListTool(ctx: ToolContext): ToolDefinition { return lines.join("\n"); } + // The seat gets a column of its own, sized to the widest seat actually + // rendered. Kept inside the label it sat behind an email and a + // workspace label, neither of which has a length bound, so any long + // one pushed it past the cell's right edge and two members of one + // workspace went back to rendering as the same truncated string. A + // column cannot be pushed out of by its neighbours, and one sized to + // its own contents never truncates what it holds. + const seatSuffixes = resolveSeatSuffixes( + storage.accounts.map((entry) => entry.accountUserId), + ); + const seatHeader = "Seat"; + const seatWidth = filteredEntries.reduce( + (widest, { index }) => + Math.max(widest, seatSuffixes[index]?.length ?? 0), + seatHeader.length, + ); const listTableOptions: TableOptions = { columns: [ { header: "#", width: 3 }, - { header: "Label", width: 42 }, + // Wide enough for "Account 10 (name@example.com, + // id:05cd9f04...989a40)" at 57 characters. Longer emails and + // labels still truncate here, which is why the seat is no longer + // one of them. + { header: "Label", width: 68 }, + { header: seatHeader, width: seatWidth }, { header: "Plan", width: 18 }, { header: "Status", width: 20 }, ], @@ -302,7 +328,11 @@ export function createCodexListTool(ctx: ToolContext): ToolDefinition { ]; filteredEntries.forEach(({ account, index }) => { - const label = formatCommandAccountLabel(account, index, { maskEmail }); + const label = formatCommandAccountLabel(account, index, { + maskEmail, + peerAccounts: storage.accounts, + omitSeat: true, + }); const statuses: string[] = []; const rateLimit = formatRateLimitEntry(account, now); const quotaExhausted = formatQuotaExhaustionEntry(account, now); @@ -322,6 +352,7 @@ export function createCodexListTool(ctx: ToolContext): ToolDefinition { [ String(index + 1), label, + seatSuffixes[index] ?? "-", formatPlanType(account.planType) ?? "unknown", statusText, ], diff --git a/lib/tools/codex-note.ts b/lib/tools/codex-note.ts index 80e10826..173e02e3 100644 --- a/lib/tools/codex-note.ts +++ b/lib/tools/codex-note.ts @@ -104,7 +104,10 @@ export function createCodexNoteTool(ctx: ToolContext): ToolDefinition { accountManagerPromiseRef.current = Promise.resolve(reloadedManager); } - const accountLabel = formatCommandAccountLabel(persistedAccount, targetIndex, { maskEmail }); + const accountLabel = formatCommandAccountLabel(persistedAccount, targetIndex, { + maskEmail, + peerAccounts: storage.accounts, + }); if (normalizedNote.length === 0) { return `Cleared note for ${accountLabel}`; } diff --git a/lib/tools/codex-pool.ts b/lib/tools/codex-pool.ts index 2b49f686..1e01297b 100644 --- a/lib/tools/codex-pool.ts +++ b/lib/tools/codex-pool.ts @@ -164,7 +164,10 @@ function buildPoolSnapshot( ...ctx.buildJsonAccountIdentity(index, { includeSensitive, account, - label: ctx.formatCommandAccountLabel(account, index, { maskEmail }), + label: ctx.formatCommandAccountLabel(account, index, { + maskEmail, + peerAccounts: storedAccounts, + }), }), enabled: account.enabled !== false, }); @@ -197,7 +200,7 @@ function renderPoolStatusText( const account = storage?.accounts[index]; if (account) { lines.push( - ` ${ctx.formatCommandAccountLabel(account, index, { maskEmail })}${ + ` ${ctx.formatCommandAccountLabel(account, index, { maskEmail, peerAccounts: storage?.accounts })}${ account.enabled === false ? " [disabled]" : "" }`, ); diff --git a/lib/tools/codex-refresh.ts b/lib/tools/codex-refresh.ts index a9872809..541f7502 100644 --- a/lib/tools/codex-refresh.ts +++ b/lib/tools/codex-refresh.ts @@ -53,7 +53,10 @@ export function createCodexRefreshTool(ctx: ToolContext): ToolDefinition { const input = inputs[i]; const account = storage.accounts[i]; if (!input || !account) continue; - const label = formatCommandAccountLabel(account, i, { maskEmail }); + const label = formatCommandAccountLabel(account, i, { + maskEmail, + peerAccounts: storage.accounts, + }); const outcome = await refreshAndPersistAccount(input); if (outcome.status === "refreshed") { diff --git a/lib/tools/codex-remove.ts b/lib/tools/codex-remove.ts index 38e06855..d8ac1572 100644 --- a/lib/tools/codex-remove.ts +++ b/lib/tools/codex-remove.ts @@ -152,6 +152,7 @@ export function createCodexRemoveTool(ctx: ToolContext): ToolDefinition { const label = formatCommandAccountLabel(account, targetIndex, { maskEmail, + peerAccounts: accounts, }); storage.accounts.splice(targetIndex, 1); diff --git a/lib/tools/codex-reset.ts b/lib/tools/codex-reset.ts index 7c235fa1..a5497fdc 100644 --- a/lib/tools/codex-reset.ts +++ b/lib/tools/codex-reset.ts @@ -211,9 +211,12 @@ export function createCodexResetTool(ctx: ToolContext): ToolDefinition { if (!target) { throw new Error(`No account at position ${index + 1}.`); } - const label = formatCommandAccountLabel(target, index); + const label = formatCommandAccountLabel(target, index, { + peerAccounts: storage.accounts, + }); const displayLabel = formatCommandAccountLabel(target, index, { maskEmail, + peerAccounts: storage.accounts, }); const identity = buildJsonAccountIdentity(index, { includeSensitive: includeSensitiveOutput, diff --git a/lib/tools/codex-status.ts b/lib/tools/codex-status.ts index e2b680c7..5567140f 100644 --- a/lib/tools/codex-status.ts +++ b/lib/tools/codex-status.ts @@ -6,6 +6,7 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"; import { loadAccounts } from "../storage.js"; import { AccountManager, formatCooldown, formatWaitTime } from "../accounts.js"; +import { resolveSeatSuffixes } from "../account-display.js"; import { MODEL_FAMILIES } from "../prompts/codex.js"; import { recommendBeginnerNextAction } from "../ui/beginner.js"; import { @@ -141,6 +142,7 @@ export function createCodexStatusTool(ctx: ToolContext): ToolDefinition { ...buildJsonAccountIdentity(index, { includeSensitive: includeSensitiveOutput, account, + peerAccounts: storage.accounts, }), enabled: account.enabled !== false, isActive: index === activeIndex, @@ -166,6 +168,7 @@ export function createCodexStatusTool(ctx: ToolContext): ToolDefinition { ...buildJsonAccountIdentity(index, { includeSensitive: includeSensitiveOutput, account, + peerAccounts: storage.accounts, }), families: Object.fromEntries( MODEL_FAMILIES.map((family) => { @@ -207,7 +210,10 @@ export function createCodexStatusTool(ctx: ToolContext): ToolDefinition { ]; storage.accounts.forEach((account, index) => { - const label = formatCommandAccountLabel(account, index, { maskEmail }); + const label = formatCommandAccountLabel(account, index, { + maskEmail, + peerAccounts: storage.accounts, + }); const badges: string[] = []; if (index === activeIndex) badges.push(formatUiBadge(ui, "active", "accent")); @@ -300,10 +306,23 @@ export function createCodexStatusTool(ctx: ToolContext): ToolDefinition { return lines.join("\n"); } + // A column of its own, sized to what it holds, for the same reason as + // in `codex-list`: behind an unbounded email this 42-wide Label + // truncates, and a seat that does not reach the screen cannot tell + // two members of one workspace apart. + const seatSuffixes = resolveSeatSuffixes( + storage.accounts.map((entry) => entry.accountUserId), + ); + const seatHeader = "Seat"; + const seatWidth = seatSuffixes.reduce( + (widest, seat) => Math.max(widest, seat?.length ?? 0), + seatHeader.length, + ); const statusTableOptions: TableOptions = { columns: [ { header: "#", width: 3 }, { header: "Label", width: 42 }, + { header: seatHeader, width: seatWidth }, { header: "Plan", width: 18 }, { header: "Active", width: 6 }, { header: "Rate Limit", width: 16 }, @@ -319,7 +338,11 @@ export function createCodexStatusTool(ctx: ToolContext): ToolDefinition { ]; storage.accounts.forEach((account, index) => { - const label = formatCommandAccountLabel(account, index, { maskEmail }); + const label = formatCommandAccountLabel(account, index, { + maskEmail, + peerAccounts: storage.accounts, + omitSeat: true, + }); const active = index === activeIndex ? "Yes" : "No"; const rateLimit = formatRateLimitEntry(account, now) ?? "None"; const cooldown = formatCooldown(account, now) ?? "No"; @@ -333,6 +356,7 @@ export function createCodexStatusTool(ctx: ToolContext): ToolDefinition { [ String(index + 1), label, + seatSuffixes[index] ?? "-", formatPlanType(account.planType) ?? "unknown", active, rateLimit, diff --git a/lib/tools/codex-switch.ts b/lib/tools/codex-switch.ts index 54c805df..71d74c45 100644 --- a/lib/tools/codex-switch.ts +++ b/lib/tools/codex-switch.ts @@ -130,6 +130,7 @@ export function createCodexSwitchTool(ctx: ToolContext): ToolDefinition { const label = formatCommandAccountLabel(account, targetIndex, { maskEmail, + peerAccounts: accounts, }); try { await persist(storage); diff --git a/lib/tools/codex-tag.ts b/lib/tools/codex-tag.ts index efd7db34..9cf5819d 100644 --- a/lib/tools/codex-tag.ts +++ b/lib/tools/codex-tag.ts @@ -126,7 +126,10 @@ export function createCodexTagTool(ctx: ToolContext): ToolDefinition { accountManagerPromiseRef.current = Promise.resolve(reloadedManager); } - const accountLabel = formatCommandAccountLabel(persistedAccount, targetIndex, { maskEmail }); + const accountLabel = formatCommandAccountLabel(persistedAccount, targetIndex, { + maskEmail, + peerAccounts: storage.accounts, + }); const previousText = previousTags.length > 0 ? previousTags.join(", ") : "none"; const nextText = diff --git a/lib/tools/codex-warm.ts b/lib/tools/codex-warm.ts index d2cf632e..05dcb84c 100644 --- a/lib/tools/codex-warm.ts +++ b/lib/tools/codex-warm.ts @@ -144,6 +144,7 @@ export function createCodexWarmTool(ctx: ToolContext): ToolDefinition { const account = storage.accounts[result.index]; const label = formatCommandAccountLabel(account, result.index, { maskEmail, + peerAccounts: storage.accounts, }); if (result.status === "warmed") { lines.push(` ${getStatusMarker(ui, "ok")} ${label}: Window started`); diff --git a/lib/tools/index.ts b/lib/tools/index.ts index 6fa83d8e..1f7198d7 100644 --- a/lib/tools/index.ts +++ b/lib/tools/index.ts @@ -109,13 +109,18 @@ export interface ToolContext { | { email?: string; accountId?: string; + accountUserId?: string; accountLabel?: string; accountTags?: string[]; accountNote?: string; } | undefined, index: number, - options?: { maskEmail?: boolean }, + options?: { + maskEmail?: boolean; + peerAccounts?: readonly ({ accountUserId?: string } | undefined)[]; + omitSeat?: boolean; + }, ) => string; resolveMaskEmail: () => boolean; normalizeAccountTags: (raw: string) => string[]; @@ -154,11 +159,13 @@ export interface ToolContext { account?: { email?: string; accountId?: string; + accountUserId?: string; accountLabel?: string; accountTags?: string[]; accountNote?: string; }; label?: string; + peerAccounts?: readonly ({ accountUserId?: string } | undefined)[]; }, ) => Record; buildRoutingVisibilitySnapshot: (overrides?: { diff --git a/lib/ui/auth-menu.ts b/lib/ui/auth-menu.ts index f463f2b6..b1b17457 100644 --- a/lib/ui/auth-menu.ts +++ b/lib/ui/auth-menu.ts @@ -3,7 +3,7 @@ import { confirm } from "./confirm.js"; import { getUiRuntimeOptions } from "./runtime.js"; import { select, type MenuItem } from "./select.js"; import { paintUiText, formatUiBadge } from "./format.js"; -import { resolveDisplayEmail } from "../account-display.js"; +import { formatSeatSuffix, resolveDisplayEmail } from "../account-display.js"; export type AccountStatus = | "active" @@ -18,6 +18,7 @@ export type AccountStatus = export interface AccountInfo { index: number; accountId?: string; + accountUserId?: string; accountLabel?: string; email?: string; addedAt?: number; @@ -110,7 +111,11 @@ function formatAccountIdSuffix(accountId: string | undefined): string | undefine : trimmed; } -function accountTitle(account: AccountInfo, maskEmail = false): string { +function accountTitle( + account: AccountInfo, + maskEmail = false, + peerAccounts?: readonly AccountInfo[], +): string { const email = resolveDisplayEmail(account.email, maskEmail); const label = account.accountLabel?.trim(); const accountIdSuffix = formatAccountIdSuffix(account.accountId); @@ -121,6 +126,11 @@ function accountTitle(account: AccountInfo, maskEmail = false): string { if (accountIdSuffix && (!label || !label.includes(accountIdSuffix))) { details.push(`id:${accountIdSuffix}`); } + const seatSuffix = formatSeatSuffix( + account.accountUserId, + peerAccounts?.map((peer) => peer.accountUserId), + ); + if (seatSuffix) details.push(`seat:${seatSuffix}`); if (details.length === 0) { return `${account.index + 1}. Account`; @@ -159,7 +169,7 @@ export async function showAuthMenu( ? (ui.v2Enabled ? ` ${formatUiBadge(ui, "disabled", "danger")}` : ` ${ANSI.red}[disabled]${ANSI.reset}`) : ""; const statusSuffix = badge ? ` ${badge}` : ""; - const label = `${accountTitle(account, maskEmail)}${currentBadge}${statusSuffix}${disabledBadge}`; + const label = `${accountTitle(account, maskEmail, accounts)}${currentBadge}${statusSuffix}${disabledBadge}`; return { label: ui.v2Enabled ? paintUiText(ui, label, "heading") : label, hint: `used ${formatRelativeTime(account.lastUsed)}`, @@ -191,12 +201,13 @@ export async function showAuthMenu( export async function showAccountDetails( account: AccountInfo, - options: { maskEmail?: boolean } = {}, + options: { maskEmail?: boolean; peerAccounts?: readonly AccountInfo[] } = {}, ): Promise { const ui = getUiRuntimeOptions(); const maskEmail = options.maskEmail ?? false; + const peerAccounts = options.peerAccounts; const header = - `${accountTitle(account, maskEmail)} ${statusBadge(account.status)}` + + `${accountTitle(account, maskEmail, peerAccounts)} ${statusBadge(account.status)}` + (account.enabled === false ? (ui.v2Enabled ? ` ${formatUiBadge(ui, "disabled", "danger")}` @@ -227,11 +238,11 @@ export async function showAccountDetails( if (!action) return "cancel"; if (action === "delete") { - const confirmed = await confirm(`Delete ${accountTitle(account, maskEmail)}?`); + const confirmed = await confirm(`Delete ${accountTitle(account, maskEmail, peerAccounts)}?`); if (!confirmed) continue; } if (action === "refresh") { - const confirmed = await confirm(`Re-authenticate ${accountTitle(account, maskEmail)}?`); + const confirmed = await confirm(`Re-authenticate ${accountTitle(account, maskEmail, peerAccounts)}?`); if (!confirmed) continue; } return action; diff --git a/scripts/install-oc-codex-multi-auth-core.js b/scripts/install-oc-codex-multi-auth-core.js index ff5fb911..1dcf5252 100644 --- a/scripts/install-oc-codex-multi-auth-core.js +++ b/scripts/install-oc-codex-multi-auth-core.js @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { existsSync, realpathSync } from "node:fs"; import { copyFile, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; @@ -410,24 +411,167 @@ function accountIdSuffix(accountId, includeSensitive) { return accountId.slice(-4); } +// A member id is what tells two seats of one Business workspace apart, and no +// fixed-length tail always does it: member ids sharing a six-character tail +// were observed, and in a real nine-seat pool the ids are 67 characters with +// no shared tail at all, so growing a tail until it separates them prints most +// of the id in every row. The renderer below mirrors `resolveSeatRenderer` in +// lib/account-display.ts - a tail, else one window anchored where the ids first +// diverge, else short windows at each position where a pair first differs +// joined by `..`, else a hash prefix, each capped - and the id whole only if +// none of those separate them, which needs a 128-bit SHA-256 collision. The +// hash outcome is reachable, and it prints a value that cannot be matched +// against the id by eye. +const STANDALONE_SEAT_MAX_LENGTH = 12; +const STANDALONE_SEAT_HASH_LENGTHS = [8, 12, 16, 24, 32]; +const STANDALONE_SEAT_WINDOW_SEPARATOR = ".."; + +function seatIsDisclosable(accountUserId, includeSensitive) { + if (!accountUserId) return false; + return includeSensitive || accountUserId.length >= MASK_MIN_LENGTH; +} + +function seatTail(accountUserId, length) { + return accountUserId.length > length ? accountUserId.slice(-length) : accountUserId; +} + +function seatWindow(accountUserId, start, length) { + if (accountUserId.length <= length) return accountUserId; + const begin = Math.max(0, Math.min(start, accountUserId.length - length)); + return accountUserId.slice(begin, begin + length); +} + +function seatCommonPrefixLength(values) { + const [first] = values; + if (first === undefined) return 0; + let shared = first.length; + for (const value of values) { + let index = 0; + while (index < shared && index < value.length && first[index] === value[index]) { + index += 1; + } + shared = index; + if (shared === 0) break; + } + return shared; +} + +function seatFirstDivergence(left, right) { + const limit = Math.min(left.length, right.length); + let index = 0; + while (index < limit && left[index] === right[index]) index += 1; + return index; +} + +// For every pair, the first index at which that pair differs - not every index +// where the ids disagree, which across a handful of random-looking ids is +// nearly all of them and localizes nothing. +function seatDivergenceAnchors(values) { + const anchors = new Set(); + for (let left = 0; left < values.length; left += 1) { + for (let right = left + 1; right < values.length; right += 1) { + anchors.add(seatFirstDivergence(values[left], values[right])); + } + } + return [...anchors].sort((left, right) => left - right); +} + +function seatAnchorWindowStarts(anchors, width) { + const starts = []; + for (const anchor of anchors) { + const last = starts[starts.length - 1]; + if (last !== undefined && anchor < last + width) continue; + starts.push(anchor); + } + return starts; +} + +function resolveStandaloneSeatRenderer(accountUserIds, includeSensitive) { + // Starts at the length the mask above allows, so masked output widens only + // when leaving it short would print a lie. + const base = includeSensitive ? 6 : 4; + const distinct = []; + const seen = new Set(); + for (const accountUserId of accountUserIds) { + if (!seatIsDisclosable(accountUserId, includeSensitive)) continue; + if (seen.has(accountUserId)) continue; + seen.add(accountUserId); + distinct.push(accountUserId); + } + const atBase = (accountUserId) => seatTail(accountUserId, base); + if (distinct.length <= 1) return atBase; + + const separates = (render) => new Set(distinct.map(render)).size === distinct.length; + + for (let length = base; length <= STANDALONE_SEAT_MAX_LENGTH; length += 1) { + const render = (accountUserId) => seatTail(accountUserId, length); + if (separates(render)) return render; + } + const start = seatCommonPrefixLength(distinct); + for (let length = base; length <= STANDALONE_SEAT_MAX_LENGTH; length += 1) { + const render = (accountUserId) => seatWindow(accountUserId, start, length); + if (separates(render)) return render; + } + const anchors = seatDivergenceAnchors(distinct); + for (let width = 2; width <= STANDALONE_SEAT_MAX_LENGTH; width += 1) { + const starts = seatAnchorWindowStarts(anchors, width); + const rendered = + starts.length * width + (starts.length - 1) * STANDALONE_SEAT_WINDOW_SEPARATOR.length; + // Skipped, not abandoned: a wider window can span two nearby anchors + // that needed one window each, so the cost falls as the window count + // does. Mirrors `resolveSeatRenderer` in lib/account-display.ts, where + // the measured counter-example is written out. + if (rendered > STANDALONE_SEAT_MAX_LENGTH) continue; + const render = (accountUserId) => + starts + .map((windowStart) => accountUserId.slice(windowStart, windowStart + width)) + .join(STANDALONE_SEAT_WINDOW_SEPARATOR); + if (separates(render)) return render; + } + for (const length of STANDALONE_SEAT_HASH_LENGTHS) { + const render = (accountUserId) => createHash("sha256").update(accountUserId).digest("hex").slice(0, length); + if (separates(render)) return render; + } + return (accountUserId) => accountUserId; +} + function summarizeStandaloneAccounts(storage, includeSensitive, tag) { const accounts = Array.isArray(storage?.accounts) ? storage.accounts : []; const normalizedTag = typeof tag === "string" ? tag.trim().toLowerCase() : ""; - return accounts + const entries = accounts .map((account, index) => ({ account, index })) .filter(({ account }) => !normalizedTag || (Array.isArray(account?.accountTags) && - account.accountTags.some((entry) => String(entry).toLowerCase() === normalizedTag))) + account.accountTags.some((entry) => String(entry).toLowerCase() === normalizedTag))); + const renderSeat = resolveStandaloneSeatRenderer( + entries.map(({ account }) => + (typeof account?.accountUserId === "string" ? account.accountUserId.trim() : "") || undefined, + ), + includeSensitive, + ); + return entries .map(({ account, index }) => { const trimmedId = typeof account?.accountId === "string" ? account.accountId.trim() : ""; const accountId = trimmedId || undefined; + // Members of one Business workspace share `accountId`, so the seat is + // what tells them apart. It is carried masked next to its suffix for + // the same reason `accountId` is: so the printed `seat:` discloses no + // more of an id than the field beside it unless telling two seats + // apart requires it. + const trimmedUserId = + typeof account?.accountUserId === "string" ? account.accountUserId.trim() : ""; + const accountUserId = trimmedUserId || undefined; return { index, label: account?.accountLabel ?? `Account ${index + 1}`, email: maskValue(account?.email, includeSensitive), accountId: maskValue(accountId, includeSensitive), idSuffix: accountIdSuffix(accountId, includeSensitive), + accountUserId: maskValue(accountUserId, includeSensitive), + seatSuffix: seatIsDisclosable(accountUserId, includeSensitive) + ? renderSeat(accountUserId) + : undefined, accountIdSource: account?.accountIdSource, enabled: account?.enabled !== false, hasRefreshToken: typeof account?.refreshToken === "string" && account.refreshToken.length > 0, @@ -453,7 +597,11 @@ function printStandaloneResult(command, payload, json) { console.log(`Accounts: ${payload.totalAccounts}`); if (Array.isArray(payload.accounts)) { for (const account of payload.accounts) { - const identity = [account.email, account.idSuffix ? `id:${account.idSuffix}` : undefined] + const identity = [ + account.email, + account.idSuffix ? `id:${account.idSuffix}` : undefined, + account.seatSuffix ? `seat:${account.seatSuffix}` : undefined, + ] .filter(Boolean) .join(", "); const name = identity ? `${account.label} (${identity})` : account.label; diff --git a/test/account-display.test.ts b/test/account-display.test.ts index 20173383..89454729 100644 --- a/test/account-display.test.ts +++ b/test/account-display.test.ts @@ -1,6 +1,8 @@ import { + formatSeatSuffix, maskEmailForDisplay, resolveDisplayEmail, + resolveSeatSuffixes, } from "../lib/account-display.js"; describe("account-display", () => { @@ -80,3 +82,305 @@ describe("email masking edge cases", () => { } }); }); + +describe("seat suffix", () => { + it("uses six characters when nothing else needs telling apart", () => { + expect(formatSeatSuffix("user_aaaaaa111111")).toBe("111111"); + expect(formatSeatSuffix("abc")).toBe("abc"); + expect(formatSeatSuffix(undefined)).toBeUndefined(); + expect(formatSeatSuffix(" ")).toBeUndefined(); + }); + + // Six characters is a tail, not an identity. These two member ids are + // different seats - different quota, different weekly reset - that happen + // to end the same way, so a fixed six-character suffix renders both as + // `000001` and reports two accounts as one. + it("grows past six characters when distinct ids share a six-character tail", () => { + const ids = ["member-000001", "other-000001"]; + + expect(resolveSeatSuffixes(ids)).toEqual(["ber-000001", "her-000001"]); + expect(formatSeatSuffix("member-000001", ids)).toBe("ber-000001"); + expect(formatSeatSuffix("other-000001", ids)).toBe("her-000001"); + }); + + it("never renders two distinct member ids the same way", () => { + const cases: string[][] = [ + ["member-000001", "other-000001"], + ["aaaaaa", "bbbbbb"], + ["prefix-a-xxxxxx", "prefix-b-xxxxxx", "prefix-c-xxxxxx"], + ["short", "a-very-long-member-identifier-short"], + ["x".repeat(40), `y${"x".repeat(39)}`], + ]; + + for (const ids of cases) { + const rendered = resolveSeatSuffixes(ids).filter( + (seat): seat is string => seat !== undefined, + ); + expect(new Set(rendered).size, `rendering ${ids.join(" / ")}`).toBe( + new Set(ids).size, + ); + } + }); + + it("holds the position of entries that have no member id", () => { + expect(resolveSeatSuffixes(["member-000001", undefined, "other-000001"])).toEqual([ + "ber-000001", + undefined, + "her-000001", + ]); + }); + + // Repeats of one id are one seat listed twice, not a collision to resolve, + // so they must not push every row into a longer rendering. + it("keeps six characters when the only repeats are the same id", () => { + expect(resolveSeatSuffixes(["user_aaaaaa111111", "user_aaaaaa111111"])).toEqual([ + "111111", + "111111", + ]); + }); + + // A caller passing the OTHER accounts rather than all of them still gets a + // suffix that separates this id from them. + it("counts the rendered id itself even when the peer list omits it", () => { + expect(formatSeatSuffix("member-000001", ["other-000001"])).toBe("ber-000001"); + }); + + // A SYNTHETIC single-divergence shape - one distinguishing character, then + // 38 identical ones - not what this backend issues; see the measured + // profile below. No tail reaches a difference at the head, so a tail search + // returns all 39 characters for every account. This is the case the single + // anchored window exists for. + it("stays short for ids that differ only at their first character", () => { + const workspace = "05cd9f04-d56a-4256-9934-9cb827989a40"; + const ids = ["9", "X", "E", "W"].map((seat) => `${seat}__${workspace}`); + + expect(resolveSeatSuffixes(ids)).toEqual([ + "9__05c", + "X__05c", + "E__05c", + "W__05c", + ]); + expect(formatSeatSuffix(`9__${workspace}`, ids)).toBe("9__05c"); + }); + + /** + * The profile measured structurally against a real nine-seat Business pool: + * 67 characters, five shared leading characters, no shared tail, and + * pairwise first divergences at three positions 26 apart. Reproduced rather + * than described, because the fixture it replaces encoded a description of + * that data that turned out to be wrong. + */ + const realPoolProfileIds = (): string[] => { + const member = (head: string) => `${head}0123456789abcdefghijklmno`; + const seat = (head: string, workspace: string) => `user_${member(head)}${workspace}`; + const w1 = "05cd9f04-d56a-4256-9934-9cb827989a40"; + const w2 = "0ce0db3a-1111-2222-3333-444444ff8839"; + const w3 = "15aaaaaa-2222-3333-4444-555555aa1111"; + const w4 = "25bbbbbb-3333-4444-5555-666666bb2222"; + const w5 = "35cccccc-4444-5555-6666-777777cc3333"; + return [ + seat("A", w1), + seat("B", w1), + seat("C", w1), + seat("D", w1), + seat("E", w2), + seat("A", w2), + seat("B", w3), + seat("F", w4), + seat("C", w5), + ]; + }; + + const firstDivergence = (left: string, right: string): number => { + let index = 0; + while (index < left.length && left[index] === right[index]) index += 1; + return index; + }; + + it("carries the structure measured on the real pool, not a paraphrase of it", () => { + const ids = realPoolProfileIds(); + const divergences = new Set(); + for (let left = 0; left < ids.length; left += 1) { + for (let right = left + 1; right < ids.length; right += 1) { + divergences.add(firstDivergence(ids[left]!, ids[right]!)); + } + } + + expect([...new Set(ids.map((id) => id.length))]).toEqual([67]); + expect(new Set(ids).size).toBe(9); + expect([...divergences].sort((left, right) => left - right)).toEqual([5, 31, 32]); + // Neither of the first two strategies can separate these inside the cap, + // which is what makes this shape worth a fixture at all. + expect(new Set(ids.map((id) => id.slice(-32))).size).toBe(5); + expect(new Set(ids.map((id) => id.slice(5, 5 + 12))).size).toBe(6); + }); + + // Distinct and bounded is the entire contract on this shape. Deliberately + // not a specific string: which strategy reaches it is an implementation + // detail, and pinning one is how the previous fixture came to assert a + // rendering the real data never produced. + it("renders the real-pool profile distinct and bounded", () => { + const rendered = resolveSeatSuffixes(realPoolProfileIds()).filter( + (seat): seat is string => seat !== undefined, + ); + + expect(rendered).toHaveLength(9); + expect(new Set(rendered).size).toBe(9); + for (const seat of rendered) { + expect(seat.length, seat).toBeLessThanOrEqual(32); + } + }); + + // Distinct and bounded is satisfied by a hash too, so on its own it leaves + // the joined-excerpt strategy unpinned - delete that strategy and this + // profile silently falls through to opaque output. This asserts the part + // that is worth having: every piece of the rendered seat is lifted from the + // id, so a human can find it there. Still no literal window. + it("keeps the real-pool seat derived from the id rather than hashed", () => { + const ids = realPoolProfileIds(); + const rendered = resolveSeatSuffixes(ids); + + ids.forEach((id, index) => { + const seat = rendered[index]; + expect(seat, id).toBeDefined(); + for (const piece of String(seat).split("..")) { + expect(piece.length, `"${piece}" of "${seat}"`).toBeGreaterThan(0); + expect(id, `"${piece}" of "${seat}"`).toContain(piece); + } + }); + }); + + /** + * Two clusters of three adjacent divergences, 26 apart - the real pool's + * clustering with one more member per cluster. Every id differs from every + * other at exactly one marker position, so an excerpt separates a pair + * only by covering a marker, which is what makes the window arithmetic + * below decide the outcome rather than merely describe it. + */ + const clusteredAnchorIds = (): string[] => { + const base = "user_0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnop"; + const flip = (at: number) => `${base.slice(0, at)}Z${base.slice(at + 1)}`; + return [base, ...[5, 6, 7, 31, 32, 33].map(flip)]; + }; + + /** The renderer's own window placement, to measure the fixture rather than trust it. */ + const windowStarts = (anchors: readonly number[], width: number): number[] => { + const starts: number[] = []; + for (const anchor of anchors) { + const last = starts[starts.length - 1]; + if (last !== undefined && anchor < last + width) continue; + starts.push(anchor); + } + return starts; + }; + + const joinedCost = (windows: number, width: number): number => + windows * width + (windows - 1) * "..".length; + + it("carries a divergence structure whose window cost falls as the window widens", () => { + const ids = clusteredAnchorIds(); + const divergences = new Set(); + for (let left = 0; left < ids.length; left += 1) { + for (let right = left + 1; right < ids.length; right += 1) { + divergences.add(firstDivergence(ids[left]!, ids[right]!)); + } + } + const anchors = [...divergences].sort((left, right) => left - right); + + expect([...new Set(ids.map((id) => id.length))]).toEqual([67]); + expect(new Set(ids).size).toBe(7); + expect(anchors).toEqual([5, 6, 7, 31, 32, 33]); + // Neither earlier strategy can separate these inside the cap. + expect(new Set(ids.map((id) => id.slice(-32))).size).toBe(1); + expect(new Set(ids.map((id) => id.slice(5, 5 + 12))).size).toBe(4); + + // The non-monotonicity itself: at width 2 the adjacent anchors need a + // window each and the join costs 14, over the 12-character cap. At + // width 3 each cluster collapses into ONE window and the same join + // costs 8. A search that stops at the first overflow never sees it. + expect(windowStarts(anchors, 2)).toEqual([5, 7, 31, 33]); + expect(joinedCost(4, 2)).toBeGreaterThan(12); + expect(windowStarts(anchors, 3)).toEqual([5, 31]); + expect(joinedCost(2, 3)).toBeLessThanOrEqual(12); + }); + + // Distinct and bounded would pass on a hash, which is exactly what this + // shape used to produce, so the assertion that matters is DERIVED: every + // piece lifted from the id it names. + it("excerpts clustered divergences instead of giving up at the first overflow", () => { + const ids = clusteredAnchorIds(); + const rendered = resolveSeatSuffixes(ids); + + expect(new Set(rendered).size).toBe(ids.length); + ids.forEach((id, index) => { + const seat = rendered[index]; + expect(seat, id).toBeDefined(); + expect(String(seat).length, String(seat)).toBeLessThanOrEqual(32); + for (const piece of String(seat).split("..")) { + expect(piece.length, `"${piece}" of "${seat}"`).toBeGreaterThan(0); + expect(id, `"${piece}" of "${seat}"`).toContain(piece); + } + }); + }); + + // The hash is not a branch kept for tidiness. Divergences too many or too + // far apart for joined excerpts to cover inside the cap land here, and what + // it prints cannot be matched against the id by eye - which is why the + // surfaces that print it have to say so. + it("falls back to a bounded hash when the divergences are too scattered to excerpt", () => { + const ids = [ + "A".repeat(60), + ...[0, 3, 6, 9, 12, 15, 18, 21].map( + (at) => `${"A".repeat(at)}B${"A".repeat(59 - at)}`, + ), + ]; + + const rendered = resolveSeatSuffixes(ids).filter( + (seat): seat is string => seat !== undefined, + ); + + expect(new Set(rendered).size).toBe(ids.length); + for (const seat of rendered) { + expect(seat).toMatch(/^[0-9a-f]{8}$/); + } + }); + + // A rendered seat sits in a table column, so its width may not be a + // function of how long the member id happens to be. 32 is the contract: + // the longest hash prefix the last-resort renderer can reach. + it("bounds every rendering regardless of id length", () => { + const workspace = "05cd9f04-d56a-4256-9934-9cb827989a40"; + const otherWorkspace = "0ce0db3a-1111-2222-3333-444444ff8839"; + const cases: string[][] = [ + ["9", "X", "E", "W"].map((seat) => `${seat}__${workspace}`), + [ + ...["9", "X"].map((seat) => `${seat}__${workspace}`), + ...["Q", "R"].map((seat) => `${seat}__${otherWorkspace}`), + ], + ["member-000001", "other-000001"], + ["a".repeat(200), `b${"a".repeat(199)}`], + // Long enough that returning any of them whole would breach the bound. + [`${"A".repeat(50)}X`, `${"A".repeat(50)}Y`, `B${"A".repeat(50)}X`], + realPoolProfileIds(), + clusteredAnchorIds(), + [ + "A".repeat(60), + ...[0, 3, 6, 9, 12, 15, 18, 21].map( + (at) => `${"A".repeat(at)}B${"A".repeat(59 - at)}`, + ), + ], + ]; + + for (const ids of cases) { + const rendered = resolveSeatSuffixes(ids).filter( + (seat): seat is string => seat !== undefined, + ); + expect(new Set(rendered).size, `rendering ${ids.join(" / ")}`).toBe( + new Set(ids).size, + ); + for (const seat of rendered) { + expect(seat.length, `"${seat}" from ${ids.join(" / ")}`).toBeLessThanOrEqual(32); + } + } + }); +}); 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/accounts.test.ts b/test/accounts.test.ts index 24a44be2..98368ad5 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -820,6 +820,64 @@ describe("AccountManager", () => { expect(formatAccountLabel({ accountId: "123456" }, 0)).toBe("Account 1 (123456)"); }); + // Same index on both sides: the rendered label may only differ by seat, so + // the assertion cannot pass on "Account 7" vs "Account 8" alone. + it("renders distinct labels for two seats sharing one workspace accountId", () => { + const workspace = { + email: "shared@example.com", + accountId: "05cd9f040000000000989a40", + }; + + const first = formatAccountLabel( + { ...workspace, accountUserId: "user_aaaaaa111111" }, + 6, + ); + const second = formatAccountLabel( + { ...workspace, accountUserId: "user_bbbbbb222222" }, + 6, + ); + + expect(first).not.toBe(second); + expect(first).toBe("Account 7 (shared@example.com, id:989a40, seat:111111)"); + expect(second).toBe("Account 7 (shared@example.com, id:989a40, seat:222222)"); + }); + + // Same index on both sides again, and now the member ids end identically: + // six characters renders both seats `000001`, so the label is only distinct + // if the suffix grows. `peerAccounts` is what tells the formatter which + // other accounts it has to stay distinguishable from. + it("renders distinct labels for two seats whose member ids share a six-character tail", () => { + const workspace = { + email: "shared@example.com", + accountId: "05cd9f040000000000989a40", + }; + const peerAccounts = [ + { ...workspace, accountUserId: "member-000001" }, + { ...workspace, accountUserId: "other-000001" }, + ]; + + const first = formatAccountLabel(peerAccounts[0], 6, { peerAccounts }); + const second = formatAccountLabel(peerAccounts[1], 6, { peerAccounts }); + + expect(first).not.toBe(second); + expect(first).toBe("Account 7 (shared@example.com, id:989a40, seat:ber-000001)"); + expect(second).toBe("Account 7 (shared@example.com, id:989a40, seat:her-000001)"); + }); + + it("renders an account with no accountUserId exactly as before", () => { + expect( + formatAccountLabel({ email: "user@example.com", accountId: "abcdef123456" }, 0), + ).toBe("Account 1 (user@example.com, id:123456)"); + expect(formatAccountLabel({ accountId: "abcdef123456" }, 2)).toBe("Account 3 (123456)"); + expect( + formatAccountLabel( + { accountLabel: "Work", email: "work@co.com", accountId: "abcdef123456" }, + 0, + ), + ).toBe("Account 1 (Work, work@co.com, id:123456)"); + expect(formatAccountLabel({ accountUserId: "" }, 3)).toBe("Account 4"); + }); + it("performs true round-robin rotation across multiple requests", () => { const now = Date.now(); const stored = { diff --git a/test/auth-menu.test.ts b/test/auth-menu.test.ts index 6449803f..57554973 100644 --- a/test/auth-menu.test.ts +++ b/test/auth-menu.test.ts @@ -56,6 +56,61 @@ describe("auth-menu", () => { expect(accountRows[1]?.label).toContain("id:org-cccc...dd3333"); }); + it("renders distinct rows for two seats sharing one workspace accountId", async () => { + vi.mocked(select).mockResolvedValueOnce({ type: "cancel" }); + + const workspaceId = "org-aaaa1111bbbb2222"; + const accounts: AccountInfo[] = [ + { + index: 0, + email: "shared@example.com", + accountId: workspaceId, + accountUserId: "user_aaaaaa111111", + }, + { + index: 1, + email: "shared@example.com", + accountId: workspaceId, + accountUserId: "user_bbbbbb222222", + }, + ]; + + await showAuthMenu(accounts); + + const items = vi.mocked(select).mock.calls[0]?.[0] as Array<{ + label: string; + value?: { type?: string }; + }>; + const accountRows = items.filter((item) => item.value?.type === "select-account"); + expect(accountRows).toHaveLength(2); + expect(accountRows[0]?.label).toContain("seat:111111"); + expect(accountRows[1]?.label).toContain("seat:222222"); + // The rows carry the same email and the same workspace id, so dropping + // the seat collapses them into one indistinguishable string. + expect(accountRows[0]?.label.replace(/^1\. /, "")).not.toBe( + accountRows[1]?.label.replace(/^2\. /, ""), + ); + }); + + it("omits the seat for an account with no accountUserId", async () => { + vi.mocked(select).mockResolvedValueOnce({ type: "cancel" }); + + await showAuthMenu([ + { + index: 0, + email: "solo@example.com", + accountId: "org-aaaa1111bbbb2222", + }, + ]); + + const items = vi.mocked(select).mock.calls[0]?.[0] as Array<{ + label: string; + value?: { type?: string }; + }>; + const row = items.find((item) => item.value?.type === "select-account"); + expect(row?.label).toBe("1. solo@example.com | id:org-aaaa...bb2222"); + }); + it("uses detailed account title in delete confirmation", async () => { vi.mocked(select).mockResolvedValueOnce("delete"); vi.mocked(confirm).mockResolvedValueOnce(true); diff --git a/test/cli.test.ts b/test/cli.test.ts index 971902ed..92171a04 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -189,6 +189,24 @@ describe("CLI Module", () => { expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("1. id:acc_1234567890")); }); + it("displays a seat suffix that separates two members of one workspace", async () => { + mockRl.question.mockResolvedValueOnce("a"); + const consoleSpy = vi.spyOn(console, "log"); + + const { promptLoginMode } = await import("../lib/cli.js"); + await promptLoginMode([ + { index: 0, email: "shared@example.com", accountId: "acc_1234567890", accountUserId: "user_aaaaaa111111" }, + { index: 1, email: "shared@example.com", accountId: "acc_1234567890", accountUserId: "user_bbbbbb222222" }, + ]); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("1. shared@example.com | id:acc_1234567890 | seat:111111"), + ); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("2. shared@example.com | id:acc_1234567890 | seat:222222"), + ); + }); + it("displays plain Account N when no email or accountId", async () => { mockRl.question.mockResolvedValueOnce("f"); const consoleSpy = vi.spyOn(console, "log"); 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/index.test.ts b/test/index.test.ts index 454cab49..b9d995b3 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3916,6 +3916,234 @@ describe("OpenAIOAuthPlugin", () => { } }); }); + + // Every member of a ChatGPT Business workspace shares its `accountId`, so + // rendering that alone gave distinct members one identical `id:` and read as + // a single account duplicated. These drive the REAL `codex-list`, and so the + // real `formatCommandAccountLabel` closure behind every `codex-*` tool. + describe("seat identity across account-display surfaces", () => { + const setMaskEmail = async (value: boolean) => { + const configModule = await import("../lib/config.js"); + vi.mocked(configModule.getCodexTuiMaskEmail).mockReturnValue(value); + }; + + const WORKSPACE_ID = "05cd9f040000000000989a40"; + + // The rendered account rows, stripped of their leading number. The + // number alone always differs, so comparing whole rows would pass even + // when every identity on them is identical - which is the bug. + const identityRows = (output: string): string[] => + output + .split("\n") + .filter((line) => /^\d+ /.test(line)) + .map((line) => line.replace(/^\d+ +/, "").trim()); + + it("codex-list: distinguishes two seats sharing one workspace account id", async () => { + await setMaskEmail(false); + // Same email AND same workspace id on both rows, so the seat is the + // only thing that can tell them apart. + mockStorage.accounts = [ + { + refreshToken: "r1", + email: "shared@example.com", + accountId: WORKSPACE_ID, + accountUserId: "user_aaaaaa111111", + }, + { + refreshToken: "r2", + email: "shared@example.com", + accountId: WORKSPACE_ID, + accountUserId: "user_bbbbbb222222", + }, + ]; + + const output = (await plugin.tool["codex-list"].execute()) as string; + + expect(output).toContain("111111"); + expect(output).toContain("222222"); + const [first, second] = identityRows(output); + expect(first).not.toBe(second); + }); + + // The reported case, with the reviewer's own example ids. Six characters + // is a tail, not an identity: `member-000001` and `other-000001` are + // different seats that end the same way. A fixed six-character seat + // renders both as `000001` and puts the display back to claiming two + // accounts are one - the exact false reading this suffix exists to stop. + it("codex-list: distinguishes member ids that share a six-character tail", async () => { + await setMaskEmail(false); + mockStorage.accounts = [ + { + refreshToken: "r1", + email: "shared@example.com", + accountId: WORKSPACE_ID, + accountUserId: "member-000001", + }, + { + refreshToken: "r2", + email: "shared@example.com", + accountId: WORKSPACE_ID, + accountUserId: "other-000001", + }, + ]; + + const output = (await plugin.tool["codex-list"].execute()) as string; + + const [first, second] = identityRows(output); + expect(first).not.toBe(second); + // Grown past six to the shortest tail that separates them. + expect(output).toContain("ber-000001"); + expect(output).toContain("her-000001"); + }); + + // Email and workspace label have no length bound, so anything that + // shares a fixed-width cell with them can be pushed off its right edge. + // The seat must survive an email long enough to truncate the label. + it("codex-list: keeps both seats legible when a long email truncates the label", async () => { + await setMaskEmail(false); + const longEmail = + "extremely.long.account.display.name@very-long-corporate-subdomain.example.com"; + mockStorage.accounts = [ + { + refreshToken: "r1", + email: longEmail, + accountId: WORKSPACE_ID, + accountUserId: "user_aaaaaa111111", + }, + { + refreshToken: "r2", + email: longEmail, + accountId: WORKSPACE_ID, + accountUserId: "user_bbbbbb222222", + }, + ]; + + const output = (await plugin.tool["codex-list"].execute()) as string; + + // The label really is truncated here, so the assertions below are + // exercising the overflow case rather than a comfortable fit. + expect(output).toContain("…"); + expect(output).toContain("111111"); + expect(output).toContain("222222"); + const [first, second] = identityRows(output); + expect(first).not.toBe(second); + }); + + // The seat column is sized to the seats it holds rather than to a fixed + // number. These two seats are the same length and differ only in their + // last character, so any column narrower than they are truncates both to + // the same string - a seat that is present but no longer distinguishing. + it("codex-list: sizes the seat column so it never truncates a seat", async () => { + await setMaskEmail(false); + mockStorage.accounts = [ + { + refreshToken: "r1", + email: "shared@example.com", + accountId: WORKSPACE_ID, + accountUserId: "xAAAAAB", + }, + { + refreshToken: "r2", + email: "shared@example.com", + accountId: WORKSPACE_ID, + accountUserId: "yAAAAAC", + }, + ]; + + const output = (await plugin.tool["codex-list"].execute()) as string; + + expect(output).toContain("AAAAAB"); + expect(output).toContain("AAAAAC"); + const [first, second] = identityRows(output); + expect(first).not.toBe(second); + }); + + it("codex-list: renders no seat for an account with no member id", async () => { + await setMaskEmail(false); + mockStorage.accounts = [ + { + refreshToken: "r1", + email: "solo@example.com", + accountId: WORKSPACE_ID, + }, + ]; + + const output = (await plugin.tool["codex-list"].execute()) as string; + + expect(output).toContain("solo@example.com"); + expect(output).not.toContain("seat:"); + }); + + // A SYNTHETIC single-divergence shape - see the measured profile below + // for what the backend actually issues. Four seats differing only at + // index 0 across a 38-character shared tail: a tail-based seat renders + // all 39 of those characters, and the column is sized to what it holds, + // so the row balloons past 150 characters and spends 38 of them + // repeating the workspace id already in the Label cell. + it("codex-list: keeps the seat column narrow for ids that differ only at the head", async () => { + await setMaskEmail(false); + const workspaceUuid = "05cd9f04-d56a-4256-9934-9cb827989a40"; + mockStorage.accounts = ["9", "X", "E", "W"].map((seat, position) => ({ + refreshToken: `r${position}`, + email: "shared@example.com", + accountId: workspaceUuid, + accountUserId: `${seat}__${workspaceUuid}`, + })); + + const output = (await plugin.tool["codex-list"].execute()) as string; + + const rows = identityRows(output); + expect(rows).toHaveLength(4); + expect(new Set(rows).size).toBe(4); + // The seat is an excerpt, not the id repeated into a second column. + expect(output).not.toContain(`9__${workspaceUuid}`); + // 119 characters with a 6-wide seat column, 152 with a 39-wide one. + for (const line of output.split("\n").filter((line) => /^\d+ /.test(line))) { + expect(line.length, line).toBeLessThan(130); + } + }); + + // The profile measured structurally against a real nine-seat Business + // pool: 67-character ids, five shared leading characters, no shared + // tail, and pairwise first divergences in clusters 26 characters apart. + // Asserted as distinct and narrow rather than as a particular excerpt - + // which strategy reaches it is an implementation detail, and pinning one + // is how the fixture above came to encode a wrong reading of the data. + it("codex-list: keeps rows distinct and narrow on the measured real-pool id shape", async () => { + await setMaskEmail(false); + const w1 = "05cd9f04-d56a-4256-9934-9cb827989a40"; + const w2 = "0ce0db3a-1111-2222-3333-444444ff8839"; + const w3 = "15aaaaaa-2222-3333-4444-555555aa1111"; + const w4 = "25bbbbbb-3333-4444-5555-666666bb2222"; + const w5 = "35cccccc-4444-5555-6666-777777cc3333"; + const pool: Array<[string, string]> = [ + ["A", w1], + ["B", w1], + ["C", w1], + ["D", w1], + ["E", w2], + ["A", w2], + ["B", w3], + ["F", w4], + ["C", w5], + ]; + mockStorage.accounts = pool.map(([head, workspace], position) => ({ + refreshToken: `r${position}`, + email: "shared@example.com", + accountId: workspace, + accountUserId: `user_${head}0123456789abcdefghijklmno${workspace}`, + })); + + const output = (await plugin.tool["codex-list"].execute()) as string; + + const rows = identityRows(output); + expect(rows).toHaveLength(9); + expect(new Set(rows).size).toBe(9); + for (const line of output.split("\n").filter((line) => /^\d+ /.test(line))) { + expect(line.length, line).toBeLessThan(130); + } + }); + }); }); describe("OpenAIOAuthPlugin edge cases", () => { @@ -5271,10 +5499,12 @@ describe("OpenAIOAuthPlugin fetch handler", () => { // The runtime label must be built with masking enabled. If the // `{ maskEmail }` option is dropped from this call site, this fails. + // Matched by containment so a later option added alongside it - the + // seat-disambiguating `peerAccounts` - does not read as a regression. expect(vi.mocked(accountsModule.formatAccountLabel)).toHaveBeenCalledWith( expect.anything(), expect.any(Number), - { maskEmail: true }, + expect.objectContaining({ maskEmail: true }), ); }); diff --git a/test/login-runner.test.ts b/test/login-runner.test.ts index b305d025..91ec2dec 100644 --- a/test/login-runner.test.ts +++ b/test/login-runner.test.ts @@ -14,6 +14,9 @@ import { } from "../lib/auth/login-runner.js"; import { JWT_CLAIM_PATH } from "../lib/constants.js"; import { loadAccounts, setStoragePathDirect } from "../lib/storage.js"; +import type { AccountMetadataV3, AccountStorageV3 } from "../lib/storage.js"; +import * as loadSaveModule from "../lib/storage/load-save.js"; +import * as loggerModule from "../lib/logger.js"; import { JWT_CLAIM_PATH } from "../lib/constants.js"; function createTokenResult( @@ -605,6 +608,175 @@ describe("login-runner account and quota identities", () => { ]); }); + const loginAs = async ( + workspaceId: string, + memberId: string, + email: string, + refresh: string, + ): Promise => { + await persistAccountPool( + [ + { + type: "success", + access: businessAccessTokenFor(workspaceId, memberId, email), + refresh, + expires: Date.now() + 60_000, + }, + ], + false, + ); + }; + + it("reports a re-login of a stored seat as an in-place update", async () => { + const infoSpy = vi.spyOn(loggerModule, "logInfo"); + await loginAs("workspace-a", "member-a", "a@example.com", "refresh-a"); + infoSpy.mockClear(); + + await loginAs("workspace-a", "member-a", "a@example.com", "refresh-a-new"); + + expect(await loadAccounts().then((stored) => stored?.accounts)).toHaveLength(1); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("Login updated Account 1"), + ); + expect(infoSpy).toHaveBeenCalledWith(expect.stringContaining("in place")); + expect(infoSpy).not.toHaveBeenCalledWith( + expect.stringContaining("as a NEW account"), + ); + }); + + it("reports a new seat in a stored workspace as an addition, naming the slot it did not repair", async () => { + const infoSpy = vi.spyOn(loggerModule, "logInfo"); + await loginAs("workspace-a", "member-a", "first@example.com", "refresh-a"); + infoSpy.mockClear(); + + await loginAs("workspace-a", "member-b", "second@example.com", "refresh-b"); + + expect(await loadAccounts().then((stored) => stored?.accounts)).toHaveLength(2); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("Login added Account 2"), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("as a NEW account"), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("Same workspace id as Account 1."), + ); + // Only slots are named. An email on this line would be the one identity + // surface that ignores `maskEmail`. + const messages = infoSpy.mock.calls.map(([message]) => String(message)); + expect(messages.join("\n")).not.toContain("example.com"); + }); + + it("names the slot sharing an email when a new seat is added under a different workspace", async () => { + const infoSpy = vi.spyOn(loggerModule, "logInfo"); + await loginAs("workspace-a", "member-a", "shared@example.com", "refresh-a"); + infoSpy.mockClear(); + + await loginAs("workspace-b", "member-b", "shared@example.com", "refresh-b"); + + expect(await loadAccounts().then((stored) => stored?.accounts)).toHaveLength(2); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("Same email as Account 1."), + ); + expect(infoSpy).not.toHaveBeenCalledWith( + expect.stringContaining("Same workspace id as"), + ); + }); + + // Reads the array the runner hands to `persist`, because reading it back + // through the storage layer cannot see this: `saveAccounts` normalizes on + // write using the same org|account|member seat key, so it merges same-seat + // records itself and hides whether the prune did anything. + const prunedAccountsFor = async ( + stored: AccountMetadataV3[], + result: TokenSuccessWithAccount, + ): Promise => { + let persisted: AccountStorageV3 | undefined; + const transaction = vi + .spyOn(loadSaveModule, "withAccountStorageTransaction") + .mockImplementation(( + handler: ( + current: AccountStorageV3 | null, + persist: (storage: AccountStorageV3) => Promise, + ) => Promise, + ): Promise => + handler( + { version: 3, accounts: stored, activeIndex: 0, activeIndexByFamily: {} }, + async (storage) => { + persisted = storage; + }, + )); + try { + await persistAccountPool([result], false); + } finally { + transaction.mockRestore(); + } + return persisted?.accounts ?? []; + }; + + /** A login for a seat none of the seeded records hold, so only the prune acts on them. */ + const unrelatedSeatLogin = (): TokenSuccessWithAccount => ({ + type: "success", + access: businessAccessTokenFor("workspace-z", "member-z", "z@example.com"), + refresh: "refresh-z", + expires: Date.now() + 60_000, + }); + + it("merges two records of one seat that carry different refresh tokens", async () => { + const accounts = await prunedAccountsFor( + [ + { + accountId: "workspace-a", + accountUserId: "member-a", + email: "a@example.com", + refreshToken: "refresh-stale", + addedAt: 1_000, + lastUsed: 1_000, + }, + { + accountId: "workspace-a", + accountUserId: "member-a", + email: "a@example.com", + refreshToken: "refresh-current", + addedAt: 2_000, + lastUsed: 2_000, + }, + ], + unrelatedSeatLogin(), + ); + + const seat = accounts.filter((account) => account.accountUserId === "member-a"); + expect(seat).toHaveLength(1); + expect(seat[0]?.refreshToken).toBe("refresh-current"); + expect(accounts).toHaveLength(2); + }); + + it("keeps two email-only records with different refresh tokens apart", async () => { + const accounts = await prunedAccountsFor( + [ + { + email: "shared@example.com", + refreshToken: "refresh-1", + addedAt: 1_000, + lastUsed: 1_000, + }, + { + email: "shared@example.com", + refreshToken: "refresh-2", + addedAt: 2_000, + lastUsed: 2_000, + }, + ], + unrelatedSeatLogin(), + ); + + expect(accounts.map((account) => account.refreshToken)).toEqual([ + "refresh-1", + "refresh-2", + "refresh-z", + ]); + }); + /** An access token that names one ChatGPT account, the unit the backend meters. */ const accessTokenFor = (chatgptAccountId: string): string => encodeJwt({ [JWT_CLAIM_PATH]: { chatgpt_account_id: chatgptAccountId } }); 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/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/standalone-cli.test.ts b/test/standalone-cli.test.ts index 562faa46..0e835640 100644 --- a/test/standalone-cli.test.ts +++ b/test/standalone-cli.test.ts @@ -199,6 +199,165 @@ describe("standalone oc-codex-multi-auth CLI commands", () => { expect(identities[1]).toBe("(dup@....com, id:bbbb)"); }); + // This CLI keeps its own copy of the seat renderer, so it can drift from + // `lib/account-display.ts` silently. The ids here are a SYNTHETIC + // single-divergence shape - one distinguishing character, then the + // workspace uuid - not what the backend issues; see the measured profile + // below. No tail reaches a difference at the head, so a tail-based renderer + // cannot separate them with fewer than all 39 characters. + it("list: keeps the seat short for member ids that differ only at the head", async () => { + vi.resetModules(); + tempHome = await createTempHome(); + const workspaceUuid = "05cd9f04-d56a-4256-9934-9cb827989a40"; + await seedPool( + tempHome, + ["9", "X"].map((seat, position) => ({ + email: "shared@example.com", + accountId: workspaceUuid, + accountUserId: `${seat}__${workspaceUuid}`, + accountIdSource: "token", + refreshToken: `refresh-${position}`, + addedAt: 1000, + lastUsed: 2000, + })), + ); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runInstaller } = await import("../scripts/install-oc-codex-multi-auth-core.js"); + + await expect(runInstaller(["list", "--include-sensitive"], { + env: { ...process.env, HOME: tempHome, USERPROFILE: tempHome }, + })).resolves.toMatchObject({ exitCode: 0 }); + + const identities = logSpy.mock.calls + .map((call) => String(call[0])) + .filter((line) => line.startsWith("- [")) + .map(extractIdentity); + + expect(identities).toHaveLength(2); + // Matched exactly, not by prefix: the whole 39-character id starts with + // the short rendering, so `toContain` would pass on the defect. + const seats = identities.map((identity) => identity.match(/seat:([^,)]+)/)?.[1]); + expect(seats).toEqual(["9__05c", "X__05c"]); + }); + + // The profile measured structurally against a real nine-seat Business pool: + // 67-character ids, five shared leading characters, no shared tail, and + // pairwise first divergences in clusters 26 characters apart. Asserted as + // distinct and bounded rather than as a particular excerpt - which strategy + // reaches it is an implementation detail, and pinning one is how the + // fixture above came to encode a wrong reading of the data. + it("list: keeps the seat distinct and bounded on the measured real-pool id shape", async () => { + vi.resetModules(); + tempHome = await createTempHome(); + const workspaces = [ + "05cd9f04-d56a-4256-9934-9cb827989a40", + "0ce0db3a-1111-2222-3333-444444ff8839", + "15aaaaaa-2222-3333-4444-555555aa1111", + "25bbbbbb-3333-4444-5555-666666bb2222", + "35cccccc-4444-5555-6666-777777cc3333", + ]; + const pool: Array<[string, number]> = [ + ["A", 0], + ["B", 0], + ["C", 0], + ["D", 0], + ["E", 1], + ["A", 1], + ["B", 2], + ["F", 3], + ["C", 4], + ]; + await seedPool( + tempHome, + pool.map(([head, workspaceIndex], position) => ({ + email: "shared@example.com", + accountId: workspaces[workspaceIndex], + accountUserId: `user_${head}0123456789abcdefghijklmno${workspaces[workspaceIndex]}`, + accountIdSource: "token", + refreshToken: `refresh-${position}`, + addedAt: 1000, + lastUsed: 2000, + })), + ); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runInstaller } = await import("../scripts/install-oc-codex-multi-auth-core.js"); + + await expect(runInstaller(["list", "--include-sensitive"], { + env: { ...process.env, HOME: tempHome, USERPROFILE: tempHome }, + })).resolves.toMatchObject({ exitCode: 0 }); + + const seats = logSpy.mock.calls + .map((call) => String(call[0])) + .filter((line) => line.startsWith("- [")) + .map((line) => extractIdentity(line).match(/seat:([^,)]+)/)?.[1]); + + expect(seats).toHaveLength(9); + expect(new Set(seats).size).toBe(9); + for (const seat of seats) { + expect(seat, String(seat)).toBeDefined(); + expect(String(seat).length, String(seat)).toBeLessThanOrEqual(32); + } + // Distinct and bounded is satisfied by a hash too, so it alone would not + // notice this copy losing the joined-excerpt strategy the lib has. Every + // piece has to be lifted from the id it names. + pool.forEach(([head, workspaceIndex], position) => { + const id = `user_${head}0123456789abcdefghijklmno${workspaces[workspaceIndex]}`; + for (const piece of String(seats[position]).split("..")) { + expect(piece.length, `"${piece}" of "${seats[position]}"`).toBeGreaterThan(0); + expect(id, `"${piece}" of "${seats[position]}"`).toContain(piece); + } + }); + }); + + // Two clusters of three adjacent divergences, 26 apart. At a two-character + // window each cluster needs a window of its own and the join overflows the + // cap; at three characters each cluster collapses into one window and the + // join fits. A search that abandons the widths after the first overflow + // prints a hash here, so this is where this copy would drift from the lib. + it("list: excerpts clustered divergences rather than giving up on them", async () => { + vi.resetModules(); + tempHome = await createTempHome(); + const base = "user_0123456789abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnop"; + const memberIds = [ + base, + ...[5, 6, 7, 31, 32, 33].map((at) => `${base.slice(0, at)}Z${base.slice(at + 1)}`), + ]; + await seedPool( + tempHome, + memberIds.map((accountUserId, position) => ({ + email: "shared@example.com", + accountId: "05cd9f04-d56a-4256-9934-9cb827989a40", + accountUserId, + accountIdSource: "token", + refreshToken: `refresh-${position}`, + addedAt: 1000, + lastUsed: 2000, + })), + ); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runInstaller } = await import("../scripts/install-oc-codex-multi-auth-core.js"); + + await expect(runInstaller(["list", "--include-sensitive"], { + env: { ...process.env, HOME: tempHome, USERPROFILE: tempHome }, + })).resolves.toMatchObject({ exitCode: 0 }); + + const seats = logSpy.mock.calls + .map((call) => String(call[0])) + .filter((line) => line.startsWith("- [")) + .map((line) => extractIdentity(line).match(/seat:([^,)]+)/)?.[1]); + + expect(seats).toHaveLength(memberIds.length); + expect(new Set(seats).size).toBe(memberIds.length); + memberIds.forEach((id, position) => { + const seat = String(seats[position]); + expect(seat.length, seat).toBeLessThanOrEqual(32); + for (const piece of seat.split("..")) { + expect(piece.length, `"${piece}" of "${seat}"`).toBeGreaterThan(0); + expect(id, `"${piece}" of "${seat}"`).toContain(piece); + } + }); + }); + it("list: drops the org-derived label the plugin no longer generates", async () => { // The standalone CLI reads the pool through its own normalizer, so // without a mirror of the drop it keeps printing the wrong diff --git a/test/test-home-isolation.test.ts b/test/test-home-isolation.test.ts new file mode 100644 index 00000000..3f2618d8 --- /dev/null +++ b/test/test-home-isolation.test.ts @@ -0,0 +1,153 @@ +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 { 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; +} + +function isUnder(baseDir: string, targetPath: string): boolean { + const rel = relative(resolve(baseDir), resolve(targetPath)); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); +} + +describe("test home isolation", () => { + it("redirects homedir() away from the real user home", () => { + expect(process.env.OC_CODEX_TEST_HOME).toBeTruthy(); + expect(homedir()).toBe(process.env.OC_CODEX_TEST_HOME); + expect(isUnder(realUserHome(), homedir())).toBe(false); + }); + + it("keeps every resolved account storage path out of the real user home", () => { + const real = realUserHome(); + + setStoragePath(null); + expect(isUnder(real, getStoragePath())).toBe(false); + + setStoragePath(process.cwd()); + expect(isUnder(real, getStoragePath())).toBe(false); + + setStoragePath(null); + expect(isUnder(real, getConfigDir())).toBe(false); + }); + + // 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", () => { + expect(isUnder(realUserHome(), LOG_DIR)).toBe(false); + expect(isUnder(process.env.OC_CODEX_TEST_HOME as string, LOG_DIR)).toBe(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); + } + }); +}); + +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 }); + } + }); + + 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..d28c7049 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,9 +1,43 @@ 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'; export default defineConfig({ test: { globals: true, environment: 'node', + env: { + HOME: isolatedHome, + USERPROFILE: isolatedHome, + OC_CODEX_TEST_HOME: isolatedHome, + }, + // 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/**',