fix(codex-auth): address review focus areas

This commit is contained in:
Tam Nhu Tran
2026-05-17 17:02:59 -04:00
parent 5c79df4311
commit 211e51b949
4 changed files with 88 additions and 6 deletions
+23 -4
View File
@@ -6,8 +6,9 @@ import { getSharedCodexConfigPath } from './codex-profile-paths';
const logger = createLogger('codex-auth:symlink');
/**
* Ensure <profileDir>/config.toml is a symlink pointing to the shared
* ~/.codex/config.toml. Self-healing: recreates stale or missing symlinks.
* Ensure <profileDir>/config.toml points at the shared ~/.codex/config.toml.
* Self-healing: recreates stale or missing symlinks. If symlink creation is
* unavailable, copies the shared config so the profile still has settings.
*
* @param profileDir - The per-profile directory (will be created if missing).
* @param sharedConfigPath - Override for the shared target path (used in tests
@@ -65,9 +66,27 @@ export function ensureSharedConfigSymlink(profileDir: string, sharedConfigPath?:
}
}
fs.symlinkSync(targetPath, linkPath);
logger.stage('dispatch', 'codex.symlink.created', 'Created shared config symlink', {
try {
fs.symlinkSync(targetPath, linkPath);
logger.stage('dispatch', 'codex.symlink.created', 'Created shared config symlink', {
link: linkPath,
target: targetPath,
});
} catch (err) {
copySharedConfigFallback(targetPath, linkPath, err);
}
}
function copySharedConfigFallback(targetPath: string, linkPath: string, err: unknown): void {
fs.copyFileSync(targetPath, linkPath);
fs.chmodSync(linkPath, 0o600);
process.stderr.write(
`[!] codex-auth: symlink unavailable; copied shared config.toml to ${linkPath}. ` +
`Config edits won't propagate automatically.\n`
);
logger.warn('codex-auth.symlink-copy-fallback', 'Copied shared config after symlink failure', {
link: linkPath,
target: targetPath,
error: err instanceof Error ? err.message : String(err),
});
}
+1 -1
View File
@@ -105,7 +105,7 @@ export function parseArgs(args: string[]): CodexAuthArgs {
export function rejectUnsupportedOptions(parsed: CodexAuthArgs, usage: string): void {
if (parsed.unknownFlags && parsed.unknownFlags.length > 0) {
console.log(`Usage: ${color(usage, 'command')}`);
process.stderr.write(`Usage: ${color(usage, 'command')}\n`);
exitWithError('Unknown options', ExitCode.GENERAL_ERROR);
}
}
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
@@ -19,6 +19,7 @@ beforeEach(async () => {
});
afterEach(() => {
mock.restore();
fs.rmSync(tempDir, { recursive: true, force: true });
});
@@ -104,4 +105,29 @@ describe('ensureSharedConfigSymlink', () => {
expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true);
expect(fs.readlinkSync(linkPath)).toBe(sharedConfigPath);
});
it('copies shared config when symlink creation fails', () => {
fs.writeFileSync(sharedConfigPath, 'model = "gpt-5.5"\n', { mode: 0o600 });
const linkPath = path.join(profileDir, 'config.toml');
const symlinkSpy = spyOn(fs, 'symlinkSync').mockImplementation(() => {
throw Object.assign(new Error('simulated symlink failure'), { code: 'EPERM' });
});
const stderrChunks: string[] = [];
const origWrite = process.stderr.write.bind(process.stderr);
process.stderr.write = (chunk: string | Uint8Array): boolean => {
stderrChunks.push(typeof chunk === 'string' ? chunk : chunk.toString());
return true;
};
try {
ensureSharedConfigSymlink(profileDir, sharedConfigPath);
} finally {
process.stderr.write = origWrite;
symlinkSpy.mockRestore();
}
expect(fs.lstatSync(linkPath).isFile()).toBe(true);
expect(fs.readFileSync(linkPath, 'utf8')).toBe('model = "gpt-5.5"\n');
expect(stderrChunks.join('')).toContain('symlink unavailable');
});
});
@@ -48,6 +48,8 @@ async function captureStreams(
const stderrChunks: string[] = [];
const origOut = process.stdout.write.bind(process.stdout);
const origErr = process.stderr.write.bind(process.stderr);
const origConsoleLog = console.log;
const origConsoleError = console.error;
process.stdout.write = (chunk: string | Uint8Array) => {
stdoutChunks.push(String(chunk));
return true;
@@ -56,11 +58,19 @@ async function captureStreams(
stderrChunks.push(String(chunk));
return true;
};
console.log = (...args: unknown[]) => {
stdoutChunks.push(`${args.map(String).join(' ')}\n`);
};
console.error = (...args: unknown[]) => {
stderrChunks.push(`${args.map(String).join(' ')}\n`);
};
try {
await fn();
} finally {
process.stdout.write = origOut;
process.stderr.write = origErr;
console.log = origConsoleLog;
console.error = origConsoleError;
}
return { stdout: stdoutChunks.join(''), stderr: stderrChunks.join('') };
}
@@ -176,6 +186,33 @@ describe('handleUseCodex — shell syntax', () => {
expect(exitCode).toBeGreaterThan(0);
expect(stderr).toContain('Unsupported');
});
it('unknown option → stderr usage, empty stdout', async () => {
const { handleUseCodex } = await import('../../../../src/codex-auth/commands/use-command');
const ctx = await makeCtxWithProfile('work');
let exitCode = -1;
const origExit = process.exit;
process.exit = (code?: number) => {
exitCode = code ?? 0;
throw new Error('exit');
};
const { stdout, stderr } = await captureStreams(async () => {
try {
await handleUseCodex(ctx, ['work', '--bad-flag']);
} catch {
/* process.exit */
}
}).finally(() => {
process.exit = origExit;
});
expect(stdout).toBe('');
expect(exitCode).toBeGreaterThan(0);
expect(stderr).toContain('Usage:');
expect(stderr).toContain('Unknown options');
});
});
// ── missing profile → stderr only ────────────────────────────────────────────