fix: stabilize logging imports and tests

This commit is contained in:
Peter Steinberger
2026-01-18 19:34:02 +00:00
parent 145b2e5f52
commit bf3021d266
3 changed files with 60 additions and 28 deletions

View File

@@ -1,4 +1,4 @@
import { createSubsystemLogger } from "../../logging.js"; import { createSubsystemLogger } from "../../logging/subsystem.js";
export const AUTH_STORE_VERSION = 1; export const AUTH_STORE_VERSION = 1;
export const AUTH_PROFILE_FILENAME = "auth-profiles.json"; export const AUTH_PROFILE_FILENAME = "auth-profiles.json";

View File

@@ -4,22 +4,19 @@ import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const execSyncMock = vi.hoisted(() => vi.fn()); const execSyncMock = vi.fn();
vi.mock("node:child_process", () => ({
execSync: execSyncMock,
}));
describe("cli credentials", () => { describe("cli credentials", () => {
beforeEach(() => { beforeEach(() => {
vi.useFakeTimers(); vi.useFakeTimers();
}); });
afterEach(() => { afterEach(async () => {
vi.useRealTimers(); vi.useRealTimers();
vi.resetModules();
execSyncMock.mockReset(); execSyncMock.mockReset();
delete process.env.CODEX_HOME; delete process.env.CODEX_HOME;
const { resetCliCredentialCachesForTest } = await import("./cli-credentials.js");
resetCliCredentialCachesForTest();
}); });
it("updates the Claude Code keychain item in place", async () => { it("updates the Claude Code keychain item in place", async () => {
@@ -44,11 +41,14 @@ describe("cli credentials", () => {
const { writeClaudeCliKeychainCredentials } = await import("./cli-credentials.js"); const { writeClaudeCliKeychainCredentials } = await import("./cli-credentials.js");
const ok = writeClaudeCliKeychainCredentials({ const ok = writeClaudeCliKeychainCredentials(
access: "new-access", {
refresh: "new-refresh", access: "new-access",
expires: Date.now() + 60_000, refresh: "new-refresh",
}); expires: Date.now() + 60_000,
},
{ execSync: execSyncMock },
);
expect(ok).toBe(true); expect(ok).toBe(true);
expect(commands.some((cmd) => cmd.includes("delete-generic-password"))).toBe(false); expect(commands.some((cmd) => cmd.includes("delete-generic-password"))).toBe(false);
@@ -130,11 +130,13 @@ describe("cli credentials", () => {
allowKeychainPrompt: true, allowKeychainPrompt: true,
ttlMs: 15 * 60 * 1000, ttlMs: 15 * 60 * 1000,
platform: "darwin", platform: "darwin",
execSync: execSyncMock,
}); });
const second = readClaudeCliCredentialsCached({ const second = readClaudeCliCredentialsCached({
allowKeychainPrompt: false, allowKeychainPrompt: false,
ttlMs: 15 * 60 * 1000, ttlMs: 15 * 60 * 1000,
platform: "darwin", platform: "darwin",
execSync: execSyncMock,
}); });
expect(first).toBeTruthy(); expect(first).toBeTruthy();
@@ -161,6 +163,7 @@ describe("cli credentials", () => {
allowKeychainPrompt: true, allowKeychainPrompt: true,
ttlMs: 15 * 60 * 1000, ttlMs: 15 * 60 * 1000,
platform: "darwin", platform: "darwin",
execSync: execSyncMock,
}); });
vi.advanceTimersByTime(15 * 60 * 1000 + 1); vi.advanceTimersByTime(15 * 60 * 1000 + 1);
@@ -169,6 +172,7 @@ describe("cli credentials", () => {
allowKeychainPrompt: true, allowKeychainPrompt: true,
ttlMs: 15 * 60 * 1000, ttlMs: 15 * 60 * 1000,
platform: "darwin", platform: "darwin",
execSync: execSyncMock,
}); });
expect(first).toBeTruthy(); expect(first).toBeTruthy();
@@ -196,7 +200,7 @@ describe("cli credentials", () => {
}); });
const { readCodexCliCredentials } = await import("./cli-credentials.js"); const { readCodexCliCredentials } = await import("./cli-credentials.js");
const creds = readCodexCliCredentials({ platform: "darwin" }); const creds = readCodexCliCredentials({ platform: "darwin", execSync: execSyncMock });
expect(creds).toMatchObject({ expect(creds).toMatchObject({
access: "keychain-access", access: "keychain-access",
@@ -226,7 +230,7 @@ describe("cli credentials", () => {
); );
const { readCodexCliCredentials } = await import("./cli-credentials.js"); const { readCodexCliCredentials } = await import("./cli-credentials.js");
const creds = readCodexCliCredentials(); const creds = readCodexCliCredentials({ execSync: execSyncMock });
expect(creds).toMatchObject({ expect(creds).toMatchObject({
access: "file-access", access: "file-access",

View File

@@ -6,7 +6,7 @@ import path from "node:path";
import type { OAuthCredentials, OAuthProvider } from "@mariozechner/pi-ai"; import type { OAuthCredentials, OAuthProvider } from "@mariozechner/pi-ai";
import { loadJsonFile, saveJsonFile } from "../infra/json-file.js"; import { loadJsonFile, saveJsonFile } from "../infra/json-file.js";
import { createSubsystemLogger } from "../logging.js"; import { createSubsystemLogger } from "../logging/subsystem.js";
import { resolveUserPath } from "../utils.js"; import { resolveUserPath } from "../utils.js";
const log = createSubsystemLogger("agents/auth-profiles"); const log = createSubsystemLogger("agents/auth-profiles");
@@ -28,6 +28,12 @@ let claudeCliCache: CachedValue<ClaudeCliCredential> | null = null;
let codexCliCache: CachedValue<CodexCliCredential> | null = null; let codexCliCache: CachedValue<CodexCliCredential> | null = null;
let qwenCliCache: CachedValue<QwenCliCredential> | null = null; let qwenCliCache: CachedValue<QwenCliCredential> | null = null;
export function resetCliCredentialCachesForTest(): void {
claudeCliCache = null;
codexCliCache = null;
qwenCliCache = null;
}
export type ClaudeCliCredential = export type ClaudeCliCredential =
| { | {
type: "oauth"; type: "oauth";
@@ -69,6 +75,8 @@ type ClaudeCliWriteOptions = ClaudeCliFileOptions & {
writeFile?: (credentials: OAuthCredentials, options?: ClaudeCliFileOptions) => boolean; writeFile?: (credentials: OAuthCredentials, options?: ClaudeCliFileOptions) => boolean;
}; };
type ExecSyncFn = typeof execSync;
function resolveClaudeCliCredentialsPath(homeDir?: string) { function resolveClaudeCliCredentialsPath(homeDir?: string) {
const baseDir = homeDir ?? resolveUserPath("~"); const baseDir = homeDir ?? resolveUserPath("~");
return path.join(baseDir, CLAUDE_CLI_CREDENTIALS_RELATIVE_PATH); return path.join(baseDir, CLAUDE_CLI_CREDENTIALS_RELATIVE_PATH);
@@ -100,19 +108,24 @@ function computeCodexKeychainAccount(codexHome: string) {
function readCodexKeychainCredentials(options?: { function readCodexKeychainCredentials(options?: {
platform?: NodeJS.Platform; platform?: NodeJS.Platform;
execSync?: ExecSyncFn;
}): CodexCliCredential | null { }): CodexCliCredential | null {
const platform = options?.platform ?? process.platform; const platform = options?.platform ?? process.platform;
if (platform !== "darwin") return null; if (platform !== "darwin") return null;
const execSyncImpl = options?.execSync ?? execSync;
const codexHome = resolveCodexHomePath(); const codexHome = resolveCodexHomePath();
const account = computeCodexKeychainAccount(codexHome); const account = computeCodexKeychainAccount(codexHome);
try { try {
const secret = execSync(`security find-generic-password -s "Codex Auth" -a "${account}" -w`, { const secret = execSyncImpl(
encoding: "utf8", `security find-generic-password -s "Codex Auth" -a "${account}" -w`,
timeout: 5000, {
stdio: ["pipe", "pipe", "pipe"], encoding: "utf8",
}).trim(); timeout: 5000,
stdio: ["pipe", "pipe", "pipe"],
},
).trim();
const parsed = JSON.parse(secret) as Record<string, unknown>; const parsed = JSON.parse(secret) as Record<string, unknown>;
const tokens = parsed.tokens as Record<string, unknown> | undefined; const tokens = parsed.tokens as Record<string, unknown> | undefined;
@@ -170,9 +183,11 @@ function readQwenCliCredentials(options?: { homeDir?: string }): QwenCliCredenti
}; };
} }
function readClaudeCliKeychainCredentials(): ClaudeCliCredential | null { function readClaudeCliKeychainCredentials(
execSyncImpl: ExecSyncFn = execSync,
): ClaudeCliCredential | null {
try { try {
const result = execSync( const result = execSyncImpl(
`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`, `security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`,
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] }, { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
); );
@@ -213,10 +228,11 @@ export function readClaudeCliCredentials(options?: {
allowKeychainPrompt?: boolean; allowKeychainPrompt?: boolean;
platform?: NodeJS.Platform; platform?: NodeJS.Platform;
homeDir?: string; homeDir?: string;
execSync?: ExecSyncFn;
}): ClaudeCliCredential | null { }): ClaudeCliCredential | null {
const platform = options?.platform ?? process.platform; const platform = options?.platform ?? process.platform;
if (platform === "darwin" && options?.allowKeychainPrompt !== false) { if (platform === "darwin" && options?.allowKeychainPrompt !== false) {
const keychainCreds = readClaudeCliKeychainCredentials(); const keychainCreds = readClaudeCliKeychainCredentials(options?.execSync);
if (keychainCreds) { if (keychainCreds) {
log.info("read anthropic credentials from claude cli keychain", { log.info("read anthropic credentials from claude cli keychain", {
type: keychainCreds.type, type: keychainCreds.type,
@@ -263,6 +279,7 @@ export function readClaudeCliCredentialsCached(options?: {
ttlMs?: number; ttlMs?: number;
platform?: NodeJS.Platform; platform?: NodeJS.Platform;
homeDir?: string; homeDir?: string;
execSync?: ExecSyncFn;
}): ClaudeCliCredential | null { }): ClaudeCliCredential | null {
const ttlMs = options?.ttlMs ?? 0; const ttlMs = options?.ttlMs ?? 0;
const now = Date.now(); const now = Date.now();
@@ -279,6 +296,7 @@ export function readClaudeCliCredentialsCached(options?: {
allowKeychainPrompt: options?.allowKeychainPrompt, allowKeychainPrompt: options?.allowKeychainPrompt,
platform: options?.platform, platform: options?.platform,
homeDir: options?.homeDir, homeDir: options?.homeDir,
execSync: options?.execSync,
}); });
if (ttlMs > 0) { if (ttlMs > 0) {
claudeCliCache = { value, readAt: now, cacheKey }; claudeCliCache = { value, readAt: now, cacheKey };
@@ -286,9 +304,13 @@ export function readClaudeCliCredentialsCached(options?: {
return value; return value;
} }
export function writeClaudeCliKeychainCredentials(newCredentials: OAuthCredentials): boolean { export function writeClaudeCliKeychainCredentials(
newCredentials: OAuthCredentials,
options?: { execSync?: ExecSyncFn },
): boolean {
const execSyncImpl = options?.execSync ?? execSync;
try { try {
const existingResult = execSync( const existingResult = execSyncImpl(
`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w 2>/dev/null`, `security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w 2>/dev/null`,
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] }, { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
); );
@@ -308,7 +330,7 @@ export function writeClaudeCliKeychainCredentials(newCredentials: OAuthCredentia
const newValue = JSON.stringify(existingData); const newValue = JSON.stringify(existingData);
execSync( execSyncImpl(
`security add-generic-password -U -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -a "${CLAUDE_CLI_KEYCHAIN_ACCOUNT}" -w '${newValue.replace(/'/g, "'\"'\"'")}'`, `security add-generic-password -U -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -a "${CLAUDE_CLI_KEYCHAIN_ACCOUNT}" -w '${newValue.replace(/'/g, "'\"'\"'")}'`,
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] }, { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
); );
@@ -385,9 +407,11 @@ export function writeClaudeCliCredentials(
export function readCodexCliCredentials(options?: { export function readCodexCliCredentials(options?: {
platform?: NodeJS.Platform; platform?: NodeJS.Platform;
execSync?: ExecSyncFn;
}): CodexCliCredential | null { }): CodexCliCredential | null {
const keychain = readCodexKeychainCredentials({ const keychain = readCodexKeychainCredentials({
platform: options?.platform, platform: options?.platform,
execSync: options?.execSync,
}); });
if (keychain) return keychain; if (keychain) return keychain;
@@ -425,6 +449,7 @@ export function readCodexCliCredentials(options?: {
export function readCodexCliCredentialsCached(options?: { export function readCodexCliCredentialsCached(options?: {
ttlMs?: number; ttlMs?: number;
platform?: NodeJS.Platform; platform?: NodeJS.Platform;
execSync?: ExecSyncFn;
}): CodexCliCredential | null { }): CodexCliCredential | null {
const ttlMs = options?.ttlMs ?? 0; const ttlMs = options?.ttlMs ?? 0;
const now = Date.now(); const now = Date.now();
@@ -437,7 +462,10 @@ export function readCodexCliCredentialsCached(options?: {
) { ) {
return codexCliCache.value; return codexCliCache.value;
} }
const value = readCodexCliCredentials({ platform: options?.platform }); const value = readCodexCliCredentials({
platform: options?.platform,
execSync: options?.execSync,
});
if (ttlMs > 0) { if (ttlMs > 0) {
codexCliCache = { value, readAt: now, cacheKey }; codexCliCache = { value, readAt: now, cacheKey };
} }