From 81fd6167d37b6c4575b1c90ae636595e601c31dc Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 21 Mar 2026 05:12:46 +0800 Subject: [PATCH 1/2] refactor: extract doctor command --- lib/codex-manager.ts | 527 ++-------------------- lib/codex-manager/commands/doctor.ts | 470 +++++++++++++++++++ test/codex-manager-doctor-command.test.ts | 94 ++++ 3 files changed, 593 insertions(+), 498 deletions(-) create mode 100644 lib/codex-manager/commands/doctor.ts create mode 100644 test/codex-manager-doctor-command.test.ts diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index c54fda9b..ac33877b 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -1,4 +1,3 @@ -import { existsSync, promises as fs } from "node:fs"; import { stdin as input, stdout as output } from "node:process"; import { createInterface } from "node:readline/promises"; import { @@ -40,6 +39,10 @@ import { runBestCommand, } from "./codex-manager/commands/best.js"; import { runCheckCommand } from "./codex-manager/commands/check.js"; +import { + type DoctorCliOptions, + runDoctorCommand, +} from "./codex-manager/commands/doctor.js"; import { runForecastCommand } from "./codex-manager/commands/forecast.js"; import { runReportCommand } from "./codex-manager/commands/report.js"; import { @@ -2436,12 +2439,6 @@ function parseVerifyFlaggedArgs( return { ok: true, options }; } -interface DoctorCliOptions { - json: boolean; - fix: boolean; - dryRun: boolean; -} - function printDoctorUsage(): void { console.log( [ @@ -3103,15 +3100,6 @@ async function runFix(args: string[]): Promise { return 0; } -type DoctorSeverity = "ok" | "warn" | "error"; - -interface DoctorCheck { - key: string; - severity: DoctorSeverity; - message: string; - details?: string; -} - interface DoctorFixAction { key: string; message: string; @@ -3245,489 +3233,32 @@ function applyDoctorFixes(storage: AccountStorageV3): { } async function runDoctor(args: string[]): Promise { - if (args.includes("--help") || args.includes("-h")) { - printDoctorUsage(); - return 0; - } - - const parsedArgs = parseDoctorArgs(args); - if (!parsedArgs.ok) { - console.error(parsedArgs.message); - printDoctorUsage(); - return 1; - } - const options = parsedArgs.options; - - setStoragePath(null); - const storagePath = getStoragePath(); - const checks: DoctorCheck[] = []; - const addCheck = (check: DoctorCheck): void => { - checks.push(check); - }; - - addCheck({ - key: "storage-file", - severity: existsSync(storagePath) ? "ok" : "warn", - message: existsSync(storagePath) - ? "Account storage file found" - : "Account storage file does not exist yet (first login pending)", - details: storagePath, - }); - - if (existsSync(storagePath)) { - try { - const stat = await fs.stat(storagePath); - addCheck({ - key: "storage-readable", - severity: stat.size > 0 ? "ok" : "warn", - message: - stat.size > 0 ? "Storage file is readable" : "Storage file is empty", - details: `${stat.size} bytes`, - }); - } catch (error) { - addCheck({ - key: "storage-readable", - severity: "error", - message: "Unable to read storage file metadata", - details: error instanceof Error ? error.message : String(error), - }); - } - } - - const codexAuthPath = getCodexCliAuthPath(); - const codexConfigPath = getCodexCliConfigPath(); - let codexAuthEmail: string | undefined; - let codexAuthAccountId: string | undefined; - - addCheck({ - key: "codex-auth-file", - severity: existsSync(codexAuthPath) ? "ok" : "warn", - message: existsSync(codexAuthPath) - ? "Codex auth file found" - : "Codex auth file does not exist", - details: codexAuthPath, - }); - - if (existsSync(codexAuthPath)) { - try { - const raw = await fs.readFile(codexAuthPath, "utf-8"); - const parsed = JSON.parse(raw) as unknown; - if (parsed && typeof parsed === "object") { - const payload = parsed as Record; - const tokens = - payload.tokens && typeof payload.tokens === "object" - ? (payload.tokens as Record) - : null; - const accessToken = - tokens && typeof tokens.access_token === "string" - ? tokens.access_token - : undefined; - const idToken = - tokens && typeof tokens.id_token === "string" - ? tokens.id_token - : undefined; - const accountIdFromFile = - tokens && typeof tokens.account_id === "string" - ? tokens.account_id - : undefined; - const emailFromFile = - typeof payload.email === "string" ? payload.email : undefined; - codexAuthEmail = sanitizeEmail( - emailFromFile ?? extractAccountEmail(accessToken, idToken), - ); - codexAuthAccountId = accountIdFromFile ?? extractAccountId(accessToken); - } - addCheck({ - key: "codex-auth-readable", - severity: "ok", - message: "Codex auth file is readable", - details: - codexAuthEmail || codexAuthAccountId - ? `email=${codexAuthEmail ?? "unknown"}, accountId=${codexAuthAccountId ?? "unknown"}` - : undefined, - }); - } catch (error) { - addCheck({ - key: "codex-auth-readable", - severity: "error", - message: "Unable to read Codex auth file", - details: error instanceof Error ? error.message : String(error), - }); - } - } - - addCheck({ - key: "codex-config-file", - severity: existsSync(codexConfigPath) ? "ok" : "warn", - message: existsSync(codexConfigPath) - ? "Codex config file found" - : "Codex config file does not exist", - details: codexConfigPath, - }); - - let codexAuthStoreMode: string | undefined; - if (existsSync(codexConfigPath)) { - try { - const configRaw = await fs.readFile(codexConfigPath, "utf-8"); - const match = configRaw.match( - /^\s*cli_auth_credentials_store\s*=\s*"([^"]+)"\s*$/m, - ); - if (match?.[1]) { - codexAuthStoreMode = match[1].trim(); - } - } catch (error) { - addCheck({ - key: "codex-auth-store", - severity: "warn", - message: "Unable to read Codex auth-store config", - details: error instanceof Error ? error.message : String(error), - }); - } - } - if (!checks.some((check) => check.key === "codex-auth-store")) { - addCheck({ - key: "codex-auth-store", - severity: codexAuthStoreMode === "file" ? "ok" : "warn", - message: - codexAuthStoreMode === "file" - ? "Codex auth storage is set to file" - : "Codex auth storage is not explicitly set to file", - details: codexAuthStoreMode ? `mode=${codexAuthStoreMode}` : "mode=unset", - }); - } - - const codexCliState = await loadCodexCliState({ forceRefresh: true }); - addCheck({ - key: "codex-cli-state", - severity: codexCliState ? "ok" : "warn", - message: codexCliState - ? "Codex CLI state loaded" - : "Codex CLI state unavailable", - details: codexCliState?.path, + return runDoctorCommand(args, { + setStoragePath, + getStoragePath, + getCodexCliAuthPath, + getCodexCliConfigPath, + loadCodexCliState, + parseDoctorArgs, + printDoctorUsage, + loadAccounts, + applyDoctorFixes, + saveAccounts, + resolveActiveIndex, + evaluateForecastAccounts, + recommendForecastAccount, + sanitizeEmail, + extractAccountEmail, + extractAccountId, + hasPlaceholderEmail, + hasLikelyInvalidRefreshToken, + getDoctorRefreshTokenKey, + hasUsableAccessToken, + queuedRefresh, + normalizeFailureDetail, + applyTokenAccountIdentity, + setCodexCliActiveSelection, }); - - const storage = await loadAccounts(); - let fixChanged = false; - let fixActions: DoctorFixAction[] = []; - if (options.fix && storage && storage.accounts.length > 0) { - const fixed = applyDoctorFixes(storage); - fixChanged = fixed.changed; - fixActions = fixed.actions; - if (fixChanged && !options.dryRun) { - await saveAccounts(storage); - } - addCheck({ - key: "auto-fix", - severity: fixChanged ? "warn" : "ok", - message: fixChanged - ? options.dryRun - ? `Prepared ${fixActions.length} fix(es) (dry-run)` - : `Applied ${fixActions.length} fix(es)` - : "No safe auto-fixes needed", - }); - } - if (!storage || storage.accounts.length === 0) { - addCheck({ - key: "accounts", - severity: "warn", - message: "No accounts configured", - }); - } else { - addCheck({ - key: "accounts", - severity: "ok", - message: `Loaded ${storage.accounts.length} account(s)`, - }); - - const activeIndex = resolveActiveIndex(storage, "codex"); - const activeExists = - activeIndex >= 0 && activeIndex < storage.accounts.length; - addCheck({ - key: "active-index", - severity: activeExists ? "ok" : "error", - message: activeExists - ? `Active index is valid (${activeIndex + 1})` - : "Active index is out of range", - }); - - const disabledCount = storage.accounts.filter( - (a) => a.enabled === false, - ).length; - addCheck({ - key: "enabled-accounts", - severity: disabledCount >= storage.accounts.length ? "error" : "ok", - message: - disabledCount >= storage.accounts.length - ? "All accounts are disabled" - : `${storage.accounts.length - disabledCount} enabled / ${disabledCount} disabled`, - }); - - const seenRefreshTokens = new Set(); - let duplicateTokenCount = 0; - for (const account of storage.accounts) { - const token = getDoctorRefreshTokenKey(account.refreshToken); - if (!token) continue; - if (seenRefreshTokens.has(token)) { - duplicateTokenCount += 1; - } else { - seenRefreshTokens.add(token); - } - } - addCheck({ - key: "duplicate-refresh-token", - severity: duplicateTokenCount > 0 ? "warn" : "ok", - message: - duplicateTokenCount > 0 - ? `Detected ${duplicateTokenCount} duplicate refresh token entr${duplicateTokenCount === 1 ? "y" : "ies"}` - : "No duplicate refresh tokens detected", - }); - - const seenEmails = new Set(); - let duplicateEmailCount = 0; - let placeholderEmailCount = 0; - let likelyInvalidRefreshTokenCount = 0; - for (const account of storage.accounts) { - const email = sanitizeEmail(account.email); - if (!email) continue; - if (seenEmails.has(email)) duplicateEmailCount += 1; - seenEmails.add(email); - if (hasPlaceholderEmail(email)) placeholderEmailCount += 1; - if (hasLikelyInvalidRefreshToken(account.refreshToken)) { - likelyInvalidRefreshTokenCount += 1; - } - } - addCheck({ - key: "duplicate-email", - severity: duplicateEmailCount > 0 ? "warn" : "ok", - message: - duplicateEmailCount > 0 - ? `Detected ${duplicateEmailCount} duplicate email entr${duplicateEmailCount === 1 ? "y" : "ies"}` - : "No duplicate emails detected", - }); - addCheck({ - key: "placeholder-email", - severity: placeholderEmailCount > 0 ? "warn" : "ok", - message: - placeholderEmailCount > 0 - ? `${placeholderEmailCount} account(s) appear to be placeholder/demo entries` - : "No placeholder emails detected", - }); - addCheck({ - key: "refresh-token-shape", - severity: likelyInvalidRefreshTokenCount > 0 ? "warn" : "ok", - message: - likelyInvalidRefreshTokenCount > 0 - ? `${likelyInvalidRefreshTokenCount} account(s) have likely invalid refresh token format` - : "Refresh token format looks normal", - }); - - const now = Date.now(); - const forecastResults = evaluateForecastAccounts( - storage.accounts.map((account, index) => ({ - index, - account, - isCurrent: index === activeIndex, - now, - })), - ); - const recommendation = recommendForecastAccount(forecastResults); - if ( - recommendation.recommendedIndex !== null && - recommendation.recommendedIndex !== activeIndex - ) { - addCheck({ - key: "recommended-switch", - severity: "warn", - message: `A healthier account is available: switch to ${recommendation.recommendedIndex + 1}`, - details: recommendation.reason, - }); - } else { - addCheck({ - key: "recommended-switch", - severity: "ok", - message: "Current account aligns with forecast recommendation", - }); - } - - if (activeExists) { - const activeAccount = storage.accounts[activeIndex]; - const managerActiveEmail = sanitizeEmail(activeAccount?.email); - const managerActiveAccountId = activeAccount?.accountId; - const codexActiveEmail = - sanitizeEmail(codexCliState?.activeEmail) ?? codexAuthEmail; - const codexActiveAccountId = - codexCliState?.activeAccountId ?? codexAuthAccountId; - const isEmailMismatch = - !!managerActiveEmail && - !!codexActiveEmail && - managerActiveEmail !== codexActiveEmail; - const isAccountIdMismatch = - !!managerActiveAccountId && - !!codexActiveAccountId && - managerActiveAccountId !== codexActiveAccountId; - - addCheck({ - key: "active-selection-sync", - severity: isEmailMismatch || isAccountIdMismatch ? "warn" : "ok", - message: - isEmailMismatch || isAccountIdMismatch - ? "Manager active account and Codex active account are not aligned" - : "Manager active account and Codex active account are aligned", - details: `manager=${managerActiveEmail ?? managerActiveAccountId ?? "unknown"} | codex=${codexActiveEmail ?? codexActiveAccountId ?? "unknown"}`, - }); - - if (options.fix && activeAccount) { - let syncAccessToken = activeAccount.accessToken; - let syncRefreshToken = activeAccount.refreshToken; - let syncExpiresAt = activeAccount.expiresAt; - let syncIdToken: string | undefined; - let storageChangedFromDoctorSync = false; - - if (!hasUsableAccessToken(activeAccount, now)) { - if (options.dryRun) { - fixActions.push({ - key: "doctor-refresh", - message: `Prepared active-account token refresh for account ${activeIndex + 1} (dry-run)`, - }); - } else { - const refreshResult = await queuedRefresh( - activeAccount.refreshToken, - ); - if (refreshResult.type === "success") { - const refreshedEmail = sanitizeEmail( - extractAccountEmail( - refreshResult.access, - refreshResult.idToken, - ), - ); - const refreshedAccountId = extractAccountId(refreshResult.access); - activeAccount.accessToken = refreshResult.access; - activeAccount.refreshToken = refreshResult.refresh; - activeAccount.expiresAt = refreshResult.expires; - if (refreshedEmail) activeAccount.email = refreshedEmail; - applyTokenAccountIdentity(activeAccount, refreshedAccountId); - syncAccessToken = refreshResult.access; - syncRefreshToken = refreshResult.refresh; - syncExpiresAt = refreshResult.expires; - syncIdToken = refreshResult.idToken; - storageChangedFromDoctorSync = true; - fixActions.push({ - key: "doctor-refresh", - message: `Refreshed active account tokens for account ${activeIndex + 1}`, - }); - } else { - addCheck({ - key: "doctor-refresh", - severity: "warn", - message: "Unable to refresh active account before Codex sync", - details: normalizeFailureDetail( - refreshResult.message, - refreshResult.reason, - ), - }); - } - } - } - - if (storageChangedFromDoctorSync) { - fixChanged = true; - if (!options.dryRun) { - await saveAccounts(storage); - } - } - - if (!options.dryRun) { - const synced = await setCodexCliActiveSelection({ - accountId: activeAccount.accountId, - email: activeAccount.email, - accessToken: syncAccessToken, - refreshToken: syncRefreshToken, - expiresAt: syncExpiresAt, - ...(syncIdToken ? { idToken: syncIdToken } : {}), - }); - if (synced) { - fixChanged = true; - fixActions.push({ - key: "codex-active-sync", - message: "Synced manager active account into Codex auth state", - }); - } else { - addCheck({ - key: "codex-active-sync", - severity: "warn", - message: - "Failed to sync manager active account into Codex auth state", - }); - } - } else { - fixActions.push({ - key: "codex-active-sync", - message: "Prepared Codex active-account sync (dry-run)", - }); - } - } - } - } - - const summary = checks.reduce( - (acc, check) => { - acc[check.severity] += 1; - return acc; - }, - { ok: 0, warn: 0, error: 0 }, - ); - - if (options.json) { - console.log( - JSON.stringify( - { - command: "doctor", - storagePath, - summary, - checks, - fix: { - enabled: options.fix, - dryRun: options.dryRun, - changed: fixChanged, - actions: fixActions, - }, - }, - null, - 2, - ), - ); - return summary.error > 0 ? 1 : 0; - } - - console.log("Doctor diagnostics"); - console.log(`Storage: ${storagePath}`); - console.log( - `Summary: ${summary.ok} ok, ${summary.warn} warnings, ${summary.error} errors`, - ); - console.log(""); - for (const check of checks) { - const marker = - check.severity === "ok" ? "✓" : check.severity === "warn" ? "!" : "✗"; - console.log(`${marker} ${check.key}: ${check.message}`); - if (check.details) { - console.log(` ${check.details}`); - } - } - if (options.fix) { - console.log(""); - if (fixActions.length > 0) { - console.log( - `Auto-fix actions (${options.dryRun ? "dry-run" : "applied"}):`, - ); - for (const action of fixActions) { - console.log(` - ${action.message}`); - } - } else { - console.log("Auto-fix actions: none"); - } - } - - return summary.error > 0 ? 1 : 0; } async function clearAccountsAndReset(): Promise { diff --git a/lib/codex-manager/commands/doctor.ts b/lib/codex-manager/commands/doctor.ts new file mode 100644 index 00000000..13ddab44 --- /dev/null +++ b/lib/codex-manager/commands/doctor.ts @@ -0,0 +1,470 @@ +import { existsSync, promises as fs } from "node:fs"; +import type { ForecastAccountResult } from "../../forecast.js"; +import type { AccountStorageV3 } from "../../storage.js"; +import type { TokenResult } from "../../types.js"; + +export type DoctorSeverity = "ok" | "warn" | "error"; + +export interface DoctorCheck { + key: string; + severity: DoctorSeverity; + message: string; + details?: string; +} + +export interface DoctorFixAction { + key: string; + message: string; +} + +export interface DoctorCliOptions { + json: boolean; + fix: boolean; + dryRun: boolean; +} + +type ParsedArgsResult = + | { ok: true; options: T } + | { ok: false; message: string }; + +export interface DoctorCommandDeps { + setStoragePath: (path: string | null) => void; + getStoragePath: () => string; + getCodexCliAuthPath: () => string; + getCodexCliConfigPath: () => string; + loadCodexCliState: (options: { forceRefresh: boolean }) => Promise<{ + activeEmail?: string; + activeAccountId?: string; + path?: string; + } | null>; + parseDoctorArgs: (args: string[]) => ParsedArgsResult; + printDoctorUsage: () => void; + loadAccounts: () => Promise; + applyDoctorFixes: (storage: AccountStorageV3) => { + changed: boolean; + actions: DoctorFixAction[]; + }; + saveAccounts: (storage: AccountStorageV3) => Promise; + resolveActiveIndex: (storage: AccountStorageV3, family?: "codex") => number; + evaluateForecastAccounts: ( + inputs: Array<{ + index: number; + account: AccountStorageV3["accounts"][number]; + isCurrent: boolean; + now: number; + }>, + ) => ForecastAccountResult[]; + recommendForecastAccount: (results: ForecastAccountResult[]) => { + recommendedIndex: number | null; + reason: string; + }; + sanitizeEmail: (email: string | undefined) => string | undefined; + extractAccountEmail: ( + accessToken: string | undefined, + idToken?: string | undefined, + ) => string | undefined; + extractAccountId: (accessToken: string | undefined) => string | undefined; + hasPlaceholderEmail: (value: string | undefined) => boolean; + hasLikelyInvalidRefreshToken: (refreshToken: string) => boolean; + getDoctorRefreshTokenKey: (refreshToken: unknown) => string | undefined; + hasUsableAccessToken: ( + account: { accessToken?: string; expiresAt?: number }, + now: number, + ) => boolean; + queuedRefresh: (refreshToken: string) => Promise; + normalizeFailureDetail: ( + message: string | undefined, + reason: string | undefined, + ) => string; + applyTokenAccountIdentity: ( + account: AccountStorageV3["accounts"][number], + accountId: string | undefined, + ) => boolean; + setCodexCliActiveSelection: (params: { + accountId?: string; + email?: string; + accessToken?: string; + refreshToken: string; + expiresAt?: number; + idToken?: string; + }) => Promise; + logInfo?: (message: string) => void; + logError?: (message: string) => void; + getNow?: () => number; +} + +export async function runDoctorCommand( + args: string[], + deps: DoctorCommandDeps, +): Promise { + const logInfo = deps.logInfo ?? console.log; + const logError = deps.logError ?? console.error; + if (args.includes("--help") || args.includes("-h")) { + deps.printDoctorUsage(); + return 0; + } + const parsedArgs = deps.parseDoctorArgs(args); + if (!parsedArgs.ok) { + logError(parsedArgs.message); + deps.printDoctorUsage(); + return 1; + } + const options = parsedArgs.options; + + deps.setStoragePath(null); + const storagePath = deps.getStoragePath(); + const checks: DoctorCheck[] = []; + const addCheck = (check: DoctorCheck): void => { + checks.push(check); + }; + + addCheck({ + key: "storage-file", + severity: existsSync(storagePath) ? "ok" : "warn", + message: existsSync(storagePath) + ? "Account storage file found" + : "Account storage file does not exist yet (first login pending)", + details: storagePath, + }); + + if (existsSync(storagePath)) { + try { + const stat = await fs.stat(storagePath); + addCheck({ + key: "storage-readable", + severity: stat.size > 0 ? "ok" : "warn", + message: + stat.size > 0 ? "Storage file is readable" : "Storage file is empty", + details: `${stat.size} bytes`, + }); + } catch (error) { + addCheck({ + key: "storage-readable", + severity: "error", + message: "Unable to read storage file metadata", + details: error instanceof Error ? error.message : String(error), + }); + } + } + + const codexAuthPath = deps.getCodexCliAuthPath(); + const codexConfigPath = deps.getCodexCliConfigPath(); + let codexAuthEmail: string | undefined; + let codexAuthAccountId: string | undefined; + + addCheck({ + key: "codex-auth-file", + severity: existsSync(codexAuthPath) ? "ok" : "warn", + message: existsSync(codexAuthPath) + ? "Codex auth file found" + : "Codex auth file does not exist", + details: codexAuthPath, + }); + if (existsSync(codexAuthPath)) { + try { + const raw = await fs.readFile(codexAuthPath, "utf-8"); + const parsed = JSON.parse(raw) as Record; + const tokens = + parsed.tokens && typeof parsed.tokens === "object" + ? (parsed.tokens as Record) + : null; + const accessToken = + tokens && typeof tokens.access_token === "string" + ? tokens.access_token + : undefined; + const idToken = + tokens && typeof tokens.id_token === "string" + ? tokens.id_token + : undefined; + const accountIdFromFile = + tokens && typeof tokens.account_id === "string" + ? tokens.account_id + : undefined; + const emailFromFile = + typeof parsed.email === "string" ? parsed.email : undefined; + codexAuthEmail = deps.sanitizeEmail( + emailFromFile ?? deps.extractAccountEmail(accessToken, idToken), + ); + codexAuthAccountId = + accountIdFromFile ?? deps.extractAccountId(accessToken); + addCheck({ + key: "codex-auth-readable", + severity: "ok", + message: "Codex auth file is readable", + details: + codexAuthEmail || codexAuthAccountId + ? `email=${codexAuthEmail ?? "unknown"}, accountId=${codexAuthAccountId ?? "unknown"}` + : undefined, + }); + } catch (error) { + addCheck({ + key: "codex-auth-readable", + severity: "error", + message: "Unable to read Codex auth file", + details: error instanceof Error ? error.message : String(error), + }); + } + } + + addCheck({ + key: "codex-config-file", + severity: existsSync(codexConfigPath) ? "ok" : "warn", + message: existsSync(codexConfigPath) + ? "Codex config file found" + : "Codex config file does not exist", + details: codexConfigPath, + }); + let codexAuthStoreMode: string | undefined; + if (existsSync(codexConfigPath)) { + try { + const configRaw = await fs.readFile(codexConfigPath, "utf-8"); + const match = configRaw.match( + /^\s*cli_auth_credentials_store\s*=\s*"([^"]+)"\s*$/m, + ); + if (match?.[1]) codexAuthStoreMode = match[1].trim(); + } catch (error) { + addCheck({ + key: "codex-auth-store", + severity: "warn", + message: "Unable to read Codex auth-store config", + details: error instanceof Error ? error.message : String(error), + }); + } + } + if (!checks.some((check) => check.key === "codex-auth-store")) { + addCheck({ + key: "codex-auth-store", + severity: codexAuthStoreMode === "file" ? "ok" : "warn", + message: + codexAuthStoreMode === "file" + ? "Codex auth storage is set to file" + : "Codex auth storage is not explicitly set to file", + details: codexAuthStoreMode ? `mode=${codexAuthStoreMode}` : "mode=unset", + }); + } + + const codexCliState = await deps.loadCodexCliState({ forceRefresh: true }); + addCheck({ + key: "codex-cli-state", + severity: codexCliState ? "ok" : "warn", + message: codexCliState + ? "Codex CLI state loaded" + : "Codex CLI state unavailable", + details: codexCliState?.path, + }); + + const storage = await deps.loadAccounts(); + let fixChanged = false; + let fixActions: DoctorFixAction[] = []; + if (options.fix && storage && storage.accounts.length > 0) { + const fixed = deps.applyDoctorFixes(storage); + fixChanged = fixed.changed; + fixActions = fixed.actions; + if (fixChanged && !options.dryRun) await deps.saveAccounts(storage); + addCheck({ + key: "auto-fix", + severity: fixChanged ? "warn" : "ok", + message: fixChanged + ? options.dryRun + ? `Prepared ${fixActions.length} fix(es) (dry-run)` + : `Applied ${fixActions.length} fix(es)` + : "No safe auto-fixes needed", + }); + } + + if (!storage || storage.accounts.length === 0) { + addCheck({ + key: "accounts", + severity: "warn", + message: "No accounts configured", + }); + } else { + addCheck({ + key: "accounts", + severity: "ok", + message: `Loaded ${storage.accounts.length} account(s)`, + }); + const activeIndex = deps.resolveActiveIndex(storage, "codex"); + const activeExists = + activeIndex >= 0 && activeIndex < storage.accounts.length; + addCheck({ + key: "active-index", + severity: activeExists ? "ok" : "error", + message: activeExists + ? `Active index is valid (${activeIndex + 1})` + : "Active index is out of range", + }); + const disabledCount = storage.accounts.filter( + (a) => a.enabled === false, + ).length; + addCheck({ + key: "enabled-accounts", + severity: disabledCount >= storage.accounts.length ? "error" : "ok", + message: + disabledCount >= storage.accounts.length + ? "All accounts are disabled" + : `${storage.accounts.length - disabledCount} enabled / ${disabledCount} disabled`, + }); + + const seenRefreshTokens = new Set(); + let duplicateTokenCount = 0; + const seenEmails = new Set(); + let duplicateEmailCount = 0; + let placeholderEmailCount = 0; + let likelyInvalidRefreshTokenCount = 0; + for (const account of storage.accounts) { + const token = deps.getDoctorRefreshTokenKey(account.refreshToken); + if (token) { + if (seenRefreshTokens.has(token)) duplicateTokenCount += 1; + seenRefreshTokens.add(token); + } + const email = deps.sanitizeEmail(account.email); + if (email) { + if (seenEmails.has(email)) duplicateEmailCount += 1; + seenEmails.add(email); + if (deps.hasPlaceholderEmail(email)) placeholderEmailCount += 1; + } + if (deps.hasLikelyInvalidRefreshToken(account.refreshToken)) + likelyInvalidRefreshTokenCount += 1; + } + addCheck({ + key: "duplicate-refresh-token", + severity: duplicateTokenCount > 0 ? "warn" : "ok", + message: + duplicateTokenCount > 0 + ? `Detected ${duplicateTokenCount} duplicate refresh token entr${duplicateTokenCount === 1 ? "y" : "ies"}` + : "No duplicate refresh tokens detected", + }); + addCheck({ + key: "duplicate-email", + severity: duplicateEmailCount > 0 ? "warn" : "ok", + message: + duplicateEmailCount > 0 + ? `Detected ${duplicateEmailCount} duplicate email entr${duplicateEmailCount === 1 ? "y" : "ies"}` + : "No duplicate emails detected", + }); + addCheck({ + key: "placeholder-email", + severity: placeholderEmailCount > 0 ? "warn" : "ok", + message: + placeholderEmailCount > 0 + ? `${placeholderEmailCount} account(s) appear to be placeholder/demo entries` + : "No placeholder emails detected", + }); + addCheck({ + key: "refresh-token-shape", + severity: likelyInvalidRefreshTokenCount > 0 ? "warn" : "ok", + message: + likelyInvalidRefreshTokenCount > 0 + ? `${likelyInvalidRefreshTokenCount} account(s) have likely invalid refresh token format` + : "Refresh token format looks normal", + }); + + const now = deps.getNow?.() ?? Date.now(); + const forecastResults = deps.evaluateForecastAccounts( + storage.accounts.map((account, index) => ({ + index, + account, + isCurrent: index === activeIndex, + now, + })), + ); + const recommendation = deps.recommendForecastAccount(forecastResults); + addCheck({ + key: "recommended-switch", + severity: + recommendation.recommendedIndex !== null && + recommendation.recommendedIndex !== activeIndex + ? "warn" + : "ok", + message: + recommendation.recommendedIndex !== null && + recommendation.recommendedIndex !== activeIndex + ? `A healthier account is available: switch to ${recommendation.recommendedIndex + 1}` + : "Current account aligns with forecast recommendation", + details: + recommendation.recommendedIndex !== null && + recommendation.recommendedIndex !== activeIndex + ? recommendation.reason + : undefined, + }); + + if (activeExists) { + const activeAccount = storage.accounts[activeIndex]; + const managerActiveEmail = deps.sanitizeEmail(activeAccount?.email); + const managerActiveAccountId = activeAccount?.accountId; + const codexActiveEmail = + deps.sanitizeEmail(codexCliState?.activeEmail) ?? codexAuthEmail; + const codexActiveAccountId = + codexCliState?.activeAccountId ?? codexAuthAccountId; + const isEmailMismatch = + !!managerActiveEmail && + !!codexActiveEmail && + managerActiveEmail !== codexActiveEmail; + const isAccountIdMismatch = + !!managerActiveAccountId && + !!codexActiveAccountId && + managerActiveAccountId !== codexActiveAccountId; + addCheck({ + key: "active-selection-sync", + severity: isEmailMismatch || isAccountIdMismatch ? "warn" : "ok", + message: + isEmailMismatch || isAccountIdMismatch + ? "Manager active account and Codex active account are not aligned" + : "Manager active account and Codex active account are aligned", + details: `manager=${managerActiveEmail ?? managerActiveAccountId ?? "unknown"} | codex=${codexActiveEmail ?? codexActiveAccountId ?? "unknown"}`, + }); + } + } + + const summary = checks.reduce( + (acc, check) => { + acc[check.severity] += 1; + return acc; + }, + { ok: 0, warn: 0, error: 0 }, + ); + if (options.json) { + logInfo( + JSON.stringify( + { + command: "doctor", + storagePath, + summary, + checks, + fix: { + enabled: options.fix, + dryRun: options.dryRun, + changed: fixChanged, + actions: fixActions, + }, + }, + null, + 2, + ), + ); + return summary.error > 0 ? 1 : 0; + } + logInfo("Doctor diagnostics"); + logInfo(`Storage: ${storagePath}`); + logInfo( + `Summary: ${summary.ok} ok, ${summary.warn} warnings, ${summary.error} errors`, + ); + logInfo(""); + for (const check of checks) { + const marker = + check.severity === "ok" ? "✓" : check.severity === "warn" ? "!" : "✗"; + logInfo(`${marker} ${check.key}: ${check.message}`); + if (check.details) logInfo(` ${check.details}`); + } + if (options.fix) { + logInfo(""); + if (fixActions.length > 0) { + logInfo(`Auto-fix actions (${options.dryRun ? "dry-run" : "applied"}):`); + for (const action of fixActions) logInfo(` - ${action.message}`); + } else { + logInfo("Auto-fix actions: none"); + } + } + return summary.error > 0 ? 1 : 0; +} diff --git a/test/codex-manager-doctor-command.test.ts b/test/codex-manager-doctor-command.test.ts new file mode 100644 index 00000000..a04cae35 --- /dev/null +++ b/test/codex-manager-doctor-command.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it, vi } from "vitest"; +import { + type DoctorCliOptions, + type DoctorCommandDeps, + runDoctorCommand, +} from "../lib/codex-manager/commands/doctor.js"; +import type { AccountStorageV3 } from "../lib/storage.js"; + +function createStorage(): AccountStorageV3 { + return { + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [], + }; +} + +function createDeps( + overrides: Partial = {}, +): DoctorCommandDeps { + return { + setStoragePath: vi.fn(), + getStoragePath: vi.fn(() => "/mock/openai-codex-accounts.json"), + getCodexCliAuthPath: vi.fn(() => "/mock/auth.json"), + getCodexCliConfigPath: vi.fn(() => "/mock/config.toml"), + loadCodexCliState: vi.fn(async () => null), + parseDoctorArgs: vi.fn((args: string[]) => { + if (args.includes("--bad")) + return { ok: false as const, message: "Unknown option: --bad" }; + return { + ok: true as const, + options: { + json: true, + fix: false, + dryRun: false, + } satisfies DoctorCliOptions, + }; + }), + printDoctorUsage: vi.fn(), + loadAccounts: vi.fn(async () => createStorage()), + applyDoctorFixes: vi.fn(() => ({ changed: false, actions: [] })), + saveAccounts: vi.fn(async () => undefined), + resolveActiveIndex: vi.fn(() => 0), + evaluateForecastAccounts: vi.fn(() => []), + recommendForecastAccount: vi.fn(() => ({ + recommendedIndex: null, + reason: "none", + })), + sanitizeEmail: vi.fn((email) => email), + extractAccountEmail: vi.fn(() => undefined), + extractAccountId: vi.fn(() => undefined), + hasPlaceholderEmail: vi.fn(() => false), + hasLikelyInvalidRefreshToken: vi.fn(() => false), + getDoctorRefreshTokenKey: vi.fn(() => undefined), + hasUsableAccessToken: vi.fn(() => true), + queuedRefresh: vi.fn(async () => ({ + type: "failed", + reason: "invalid_grant", + message: "token expired", + })), + normalizeFailureDetail: vi.fn((message) => message ?? "unknown"), + applyTokenAccountIdentity: vi.fn(() => false), + setCodexCliActiveSelection: vi.fn(async () => true), + logInfo: vi.fn(), + logError: vi.fn(), + getNow: vi.fn(() => 1_000), + ...overrides, + }; +} + +describe("runDoctorCommand", () => { + it("prints usage for help", async () => { + const deps = createDeps(); + const result = await runDoctorCommand(["--help"], deps); + expect(result).toBe(0); + expect(deps.printDoctorUsage).toHaveBeenCalled(); + }); + + it("rejects invalid options", async () => { + const deps = createDeps(); + const result = await runDoctorCommand(["--bad"], deps); + expect(result).toBe(1); + expect(deps.logError).toHaveBeenCalledWith("Unknown option: --bad"); + }); + + it("prints json diagnostics", async () => { + const deps = createDeps(); + const result = await runDoctorCommand([], deps); + expect(result).toBe(0); + expect(deps.logInfo).toHaveBeenCalledWith( + expect.stringContaining('"command": "doctor"'), + ); + }); +}); From 1399ce59e158e683b4f7bf77a3082ccf61a1d303 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 21 Mar 2026 12:00:20 +0800 Subject: [PATCH 2/2] fix: sync doctor active account fixes --- lib/codex-manager/commands/doctor.ts | 184 +++++++++++++++++----- test/codex-manager-doctor-command.test.ts | 144 +++++++++++++++++ 2 files changed, 285 insertions(+), 43 deletions(-) diff --git a/lib/codex-manager/commands/doctor.ts b/lib/codex-manager/commands/doctor.ts index 13ddab44..8ff0ce88 100644 --- a/lib/codex-manager/commands/doctor.ts +++ b/lib/codex-manager/commands/doctor.ts @@ -163,39 +163,49 @@ export async function runDoctorCommand( if (existsSync(codexAuthPath)) { try { const raw = await fs.readFile(codexAuthPath, "utf-8"); - const parsed = JSON.parse(raw) as Record; - const tokens = - parsed.tokens && typeof parsed.tokens === "object" - ? (parsed.tokens as Record) - : null; - const accessToken = - tokens && typeof tokens.access_token === "string" - ? tokens.access_token - : undefined; - const idToken = - tokens && typeof tokens.id_token === "string" - ? tokens.id_token - : undefined; - const accountIdFromFile = - tokens && typeof tokens.account_id === "string" - ? tokens.account_id - : undefined; - const emailFromFile = - typeof parsed.email === "string" ? parsed.email : undefined; - codexAuthEmail = deps.sanitizeEmail( - emailFromFile ?? deps.extractAccountEmail(accessToken, idToken), - ); - codexAuthAccountId = - accountIdFromFile ?? deps.extractAccountId(accessToken); - addCheck({ - key: "codex-auth-readable", - severity: "ok", - message: "Codex auth file is readable", - details: - codexAuthEmail || codexAuthAccountId - ? `email=${codexAuthEmail ?? "unknown"}, accountId=${codexAuthAccountId ?? "unknown"}` - : undefined, - }); + const parsedUnknown = JSON.parse(raw) as unknown; + if (!parsedUnknown || typeof parsedUnknown !== "object") { + addCheck({ + key: "codex-auth-readable", + severity: "error", + message: "Codex auth file contains invalid JSON shape", + details: codexAuthPath, + }); + } else { + const parsed = parsedUnknown as Record; + const tokens = + parsed.tokens && typeof parsed.tokens === "object" + ? (parsed.tokens as Record) + : null; + const accessToken = + tokens && typeof tokens.access_token === "string" + ? tokens.access_token + : undefined; + const idToken = + tokens && typeof tokens.id_token === "string" + ? tokens.id_token + : undefined; + const accountIdFromFile = + tokens && typeof tokens.account_id === "string" + ? tokens.account_id + : undefined; + const emailFromFile = + typeof parsed.email === "string" ? parsed.email : undefined; + codexAuthEmail = deps.sanitizeEmail( + emailFromFile ?? deps.extractAccountEmail(accessToken, idToken), + ); + codexAuthAccountId = + accountIdFromFile ?? deps.extractAccountId(accessToken); + addCheck({ + key: "codex-auth-readable", + severity: "ok", + message: "Codex auth file is readable", + details: + codexAuthEmail || codexAuthAccountId + ? `email=${codexAuthEmail ?? "unknown"}, accountId=${codexAuthAccountId ?? "unknown"}` + : undefined, + }); + } } catch (error) { addCheck({ key: "codex-auth-readable", @@ -256,20 +266,12 @@ export async function runDoctorCommand( const storage = await deps.loadAccounts(); let fixChanged = false; let fixActions: DoctorFixAction[] = []; + let storageNeedsSave = false; if (options.fix && storage && storage.accounts.length > 0) { const fixed = deps.applyDoctorFixes(storage); fixChanged = fixed.changed; fixActions = fixed.actions; - if (fixChanged && !options.dryRun) await deps.saveAccounts(storage); - addCheck({ - key: "auto-fix", - severity: fixChanged ? "warn" : "ok", - message: fixChanged - ? options.dryRun - ? `Prepared ${fixActions.length} fix(es) (dry-run)` - : `Applied ${fixActions.length} fix(es)` - : "No safe auto-fixes needed", - }); + storageNeedsSave = fixed.changed; } if (!storage || storage.accounts.length === 0) { @@ -414,9 +416,105 @@ export async function runDoctorCommand( : "Manager active account and Codex active account are aligned", details: `manager=${managerActiveEmail ?? managerActiveAccountId ?? "unknown"} | codex=${codexActiveEmail ?? codexActiveAccountId ?? "unknown"}`, }); + + if (options.fix && activeAccount) { + let syncAccessToken = activeAccount.accessToken; + let syncRefreshToken = activeAccount.refreshToken; + let syncExpiresAt = activeAccount.expiresAt; + let syncIdToken: string | undefined; + + if (!deps.hasUsableAccessToken(activeAccount, now)) { + if (options.dryRun) { + fixChanged = true; + fixActions.push({ + key: "doctor-refresh", + message: `Prepared active-account token refresh for account ${activeIndex + 1} (dry-run)`, + }); + } else { + const refreshResult = await deps.queuedRefresh(activeAccount.refreshToken); + if (refreshResult.type === "success") { + const refreshedEmail = deps.sanitizeEmail( + deps.extractAccountEmail(refreshResult.access, refreshResult.idToken), + ); + const refreshedAccountId = deps.extractAccountId(refreshResult.access); + activeAccount.accessToken = refreshResult.access; + activeAccount.refreshToken = refreshResult.refresh; + activeAccount.expiresAt = refreshResult.expires; + if (refreshedEmail) activeAccount.email = refreshedEmail; + deps.applyTokenAccountIdentity(activeAccount, refreshedAccountId); + syncAccessToken = refreshResult.access; + syncRefreshToken = refreshResult.refresh; + syncExpiresAt = refreshResult.expires; + syncIdToken = refreshResult.idToken; + storageNeedsSave = true; + fixChanged = true; + fixActions.push({ + key: "doctor-refresh", + message: `Refreshed active account tokens for account ${activeIndex + 1}`, + }); + } else { + addCheck({ + key: "doctor-refresh", + severity: "warn", + message: "Unable to refresh active account before Codex sync", + details: deps.normalizeFailureDetail( + refreshResult.message, + refreshResult.reason, + ), + }); + } + } + } + + if (!options.dryRun) { + const synced = await deps.setCodexCliActiveSelection({ + accountId: activeAccount.accountId, + email: activeAccount.email, + accessToken: syncAccessToken, + refreshToken: syncRefreshToken, + expiresAt: syncExpiresAt, + ...(syncIdToken ? { idToken: syncIdToken } : {}), + }); + if (synced) { + fixChanged = true; + fixActions.push({ + key: "codex-active-sync", + message: "Synced manager active account into Codex auth state", + }); + } else { + addCheck({ + key: "codex-active-sync", + severity: "warn", + message: "Failed to sync manager active account into Codex auth state", + }); + } + } else { + fixChanged = true; + fixActions.push({ + key: "codex-active-sync", + message: "Prepared Codex active-account sync (dry-run)", + }); + } + } } } + if (options.fix) { + addCheck({ + key: "auto-fix", + severity: fixChanged ? "warn" : "ok", + message: fixChanged + ? options.dryRun + ? `Prepared ${fixActions.length} fix(es) (dry-run)` + : `Applied ${fixActions.length} fix(es)` + : "No safe auto-fixes needed", + }); + } + + if (storageNeedsSave && !options.dryRun && storage) { + await deps.saveAccounts(storage); + } + const summary = checks.reduce( (acc, check) => { acc[check.severity] += 1; diff --git a/test/codex-manager-doctor-command.test.ts b/test/codex-manager-doctor-command.test.ts index a04cae35..fe6910d7 100644 --- a/test/codex-manager-doctor-command.test.ts +++ b/test/codex-manager-doctor-command.test.ts @@ -1,3 +1,6 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { type DoctorCliOptions, @@ -5,6 +8,7 @@ import { runDoctorCommand, } from "../lib/codex-manager/commands/doctor.js"; import type { AccountStorageV3 } from "../lib/storage.js"; +import type { TokenResult } from "../lib/types.js"; function createStorage(): AccountStorageV3 { return { @@ -68,6 +72,21 @@ function createDeps( }; } +function createDoctorFiles(files: Record): { + pathFor: (name: string) => string; + cleanup: () => void; +} { + const root = mkdtempSync(join(tmpdir(), "doctor-command-test-")); + const pathFor = (name: string) => join(root, name); + for (const [name, contents] of Object.entries(files)) { + writeFileSync(pathFor(name), contents, "utf8"); + } + return { + pathFor, + cleanup: () => rmSync(root, { recursive: true, force: true }), + }; +} + describe("runDoctorCommand", () => { it("prints usage for help", async () => { const deps = createDeps(); @@ -91,4 +110,129 @@ describe("runDoctorCommand", () => { expect.stringContaining('"command": "doctor"'), ); }); + + it("reports an invalid Codex auth JSON shape", async () => { + const files = createDoctorFiles({ + "storage.json": JSON.stringify(createStorage()), + "auth.json": "123", + "config.toml": 'cli_auth_credentials_store = "file"\n', + }); + try { + const deps = createDeps({ + getStoragePath: vi.fn(() => files.pathFor("storage.json")), + getCodexCliAuthPath: vi.fn(() => files.pathFor("auth.json")), + getCodexCliConfigPath: vi.fn(() => files.pathFor("config.toml")), + }); + + const result = await runDoctorCommand([], deps); + expect(result).toBe(1); + + const payload = JSON.parse( + String(vi.mocked(deps.logInfo!).mock.calls.at(-1)?.[0] ?? ""), + ) as { + checks: Array<{ key: string; severity: string; message: string }>; + }; + expect(payload.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + key: "codex-auth-readable", + severity: "error", + message: "Codex auth file contains invalid JSON shape", + }), + ]), + ); + } finally { + files.cleanup(); + } + }); + + it("refreshes and syncs the active account once when fixing", async () => { + const files = createDoctorFiles({ + "storage.json": JSON.stringify(createStorage()), + "auth.json": JSON.stringify({ tokens: { access_token: "old-access" } }), + "config.toml": 'cli_auth_credentials_store = "file"\n', + }); + try { + const storage = createStorage(); + storage.accounts.push({ + email: "stale@example.com", + accountId: "acct-stale", + accessToken: "old-access", + refreshToken: "refresh-old", + expiresAt: 100, + }); + const refreshResult: TokenResult = { + type: "success", + access: "new-access", + refresh: "refresh-new", + expires: 5_000, + idToken: "id-token-new", + }; + const applyTokenAccountIdentity = vi.fn( + (account: AccountStorageV3["accounts"][number], accountId: string | undefined) => { + if (!accountId || account.accountId === accountId) return false; + account.accountId = accountId; + return true; + }, + ); + const deps = createDeps({ + getStoragePath: vi.fn(() => files.pathFor("storage.json")), + getCodexCliAuthPath: vi.fn(() => files.pathFor("auth.json")), + getCodexCliConfigPath: vi.fn(() => files.pathFor("config.toml")), + parseDoctorArgs: vi.fn(() => ({ + ok: true as const, + options: { + json: true, + fix: true, + dryRun: false, + } satisfies DoctorCliOptions, + })), + loadAccounts: vi.fn(async () => storage), + hasUsableAccessToken: vi.fn(() => false), + queuedRefresh: vi.fn(async () => refreshResult), + extractAccountEmail: vi.fn((accessToken?: string) => + accessToken === "new-access" ? "fresh@example.com" : undefined, + ), + extractAccountId: vi.fn((accessToken?: string) => + accessToken === "new-access" ? "acct-fresh" : undefined, + ), + applyTokenAccountIdentity, + }); + + const result = await runDoctorCommand([], deps); + expect(result).toBe(0); + expect(deps.queuedRefresh).toHaveBeenCalledWith("refresh-old"); + expect(deps.saveAccounts).toHaveBeenCalledTimes(1); + expect(deps.setCodexCliActiveSelection).toHaveBeenCalledWith({ + accountId: "acct-fresh", + email: "fresh@example.com", + accessToken: "new-access", + refreshToken: "refresh-new", + expiresAt: 5_000, + idToken: "id-token-new", + }); + expect(storage.accounts[0]).toMatchObject({ + email: "fresh@example.com", + accountId: "acct-fresh", + accessToken: "new-access", + refreshToken: "refresh-new", + expiresAt: 5_000, + }); + + const payload = JSON.parse( + String(vi.mocked(deps.logInfo!).mock.calls.at(-1)?.[0] ?? ""), + ) as { + fix: { changed: boolean; actions: Array<{ key: string; message: string }> }; + }; + expect(payload.fix.changed).toBe(true); + expect(payload.fix.actions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ key: "doctor-refresh" }), + expect.objectContaining({ key: "codex-active-sync" }), + ]), + ); + } finally { + files.cleanup(); + } + }); });