mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 08:17:11 +00:00
fix(codex-auth): address review focus areas
This commit is contained in:
@@ -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),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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 ────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user