diff --git a/src/core/commands/clients.js b/src/core/commands/clients.js index b99677123..f836d9387 100644 --- a/src/core/commands/clients.js +++ b/src/core/commands/clients.js @@ -206,12 +206,13 @@ async function runClientLifecycle(action, argv, ctx) { // A refused client is still attached and may still route through the // proxy, so the CA and its trust stay with it. if (parsed.purge && !parsed.dryRun && refused) { - if (!parsed.json) ctx.stderr.write('Skipped --purge: a client that could not be detached may still use the local CA.\n') + ctx.stderr.write('Skipped --purge: a client that could not be detached may still use the local CA.\n') } else if (parsed.purge && !parsed.dryRun) { const purged = await purgeProxyTrustResidue({ ctx }) - if (!parsed.json) { - for (const line of purged.lines) ctx.stdout.write(`${line}\n`) - } + // Under --json, stdout is the per-client NDJSON, so the purge lines + // (diagnostics, not data) go to stderr rather than being dropped. + const out = parsed.json ? ctx.stderr : ctx.stdout + for (const line of purged.lines) out.write(`${line}\n`) } return exitCode } diff --git a/test/core/detach-stops-recording.test.js b/test/core/detach-stops-recording.test.js index 267805a84..6a838232f 100644 --- a/test/core/detach-stops-recording.test.js +++ b/test/core/detach-stops-recording.test.js @@ -315,6 +315,117 @@ test('a refused detach skips --purge and keeps the local CA', async () => { } }) +/** + * Run `hyp client detach --json` against a fresh stage with + * `process.platform` pinned to linux, so the purge never reaches the macOS + * keychain or launchd. `seedCa` shapes the local CA before the run. + * stdout comes back with the stage's temp root replaced by ``. + * + * @param {string[]} argv + * @param {(caDir: string) => Promise} seedCa + */ +async function detachJson(argv, seedCa) { + const s = await stage() + const platform = /** @type {PropertyDescriptor} */ (Object.getOwnPropertyDescriptor(process, 'platform')) + Object.defineProperty(process, 'platform', { value: 'linux' }) + try { + const caDir = path.join(s.env.HYP_HOME, 'hypaware', 'tls') + await fsp.mkdir(caDir, { recursive: true }) + await seedCa(caDir) + const cli = cliCtx(s) + const code = await runDetach([...argv, '--json'], cli.ctx) + const leftover = (await fsp.readdir(caDir).catch(() => [])).sort() + // Each stage has its own temp root, which the records name; strip it so + // two runs' stdout compare byte for byte. + const out = cli.out().split(s.root).join('') + return { code, out, err: cli.err(), leftover } + } finally { + Object.defineProperty(process, 'platform', platform) + await s.cleanup() + } +} + +/** @param {string} caDir */ +async function seedWholeCa(caDir) { + await fsp.writeFile(path.join(caDir, 'ca-key.pem'), 'key') + await fsp.writeFile(path.join(caDir, 'ca-cert.pem'), 'cert') +} + +/** + * The stdout contract a scripted caller parses: one JSON object per line. + * The stderr side is the reverse: a consumer that merges the two streams + * must not be able to read a diagnostic as one more record. + * + * @param {string} out + * @param {string} err + */ +function assertStreamsSplit(out, err) { + for (const line of out.split('\n').filter(Boolean)) JSON.parse(line) + for (const line of err.split('\n').filter(Boolean)) { + assert.throws(() => JSON.parse(line), `stderr line parses as JSON: ${line}`) + } +} + +test('detach --purge --json reports the purge on stderr and leaves the NDJSON untouched', async () => { + const plain = await detachJson(['codex'], seedWholeCa) + const purged = await detachJson(['codex', '--purge'], seedWholeCa) + assert.equal(purged.code, 0) + assert.equal(purged.out, plain.out, 'the purge adds nothing to stdout') + assert.match(purged.err, /removed the local interception CA/) + assert.deepEqual(purged.leftover, []) + assertStreamsSplit(purged.out, purged.err) +}) + +test('detach --purge --json surfaces a cleanup failure on stderr without changing the exit', async () => { + // A directory where the key belongs: unlink fails, so the CA stays. + /** @param {string} caDir */ + const stuck = async (caDir) => { + await fsp.mkdir(path.join(caDir, 'ca-key.pem')) + await fsp.writeFile(path.join(caDir, 'ca-cert.pem'), 'cert') + } + const plain = await detachJson(['codex'], stuck) + const purged = await detachJson(['codex', '--purge'], stuck) + assert.equal(purged.code, plain.code) + assert.equal(purged.out, plain.out, 'the failure adds nothing to stdout') + assert.match(purged.err, /! the local CA could not be removed \(.+\); delete it by hand/) + assertStreamsSplit(purged.out, purged.err) +}) + +test('detach all --purge --json writes every client record and purges once', async () => { + const purged = await detachJson(['all', '--purge'], seedWholeCa) + const records = purged.out.split('\n').filter(Boolean) + assert.ok(records.length > 1, 'one record per client') + assert.equal((purged.err.match(/removed the local interception CA/g) ?? []).length, 1) + assert.deepEqual(purged.leftover, []) + assertStreamsSplit(purged.out, purged.err) +}) + +test('detach --purge --json --dry-run leaves the CA and says nothing about a purge', async () => { + const dry = await detachJson(['codex', '--purge', '--dry-run'], seedWholeCa) + assert.equal(dry.code, 0) + assert.doesNotMatch(dry.err, /local CA|interception CA/) + assert.deepEqual(dry.leftover, ['ca-cert.pem', 'ca-key.pem']) + assertStreamsSplit(dry.out, dry.err) +}) + +test('a refused detach with --json still says on stderr that the purge was skipped', async () => { + const s = await stage() + const platform = /** @type {PropertyDescriptor} */ (Object.getOwnPropertyDescriptor(process, 'platform')) + Object.defineProperty(process, 'platform', { value: 'linux' }) + try { + const controlDir = path.join(s.env.HYP_HOME, 'hypaware', 'config-control') + await fsp.mkdir(controlDir, { recursive: true }) + await fsp.writeFile(path.join(controlDir, 'seed.json'), JSON.stringify({ version: 2, plugins: [{ name: '@hypaware/codex' }] })) + const cli = cliCtx(s) + assert.equal(await runDetach(['codex', '--purge', '--json'], cli.ctx), 1) + assert.match(cli.err(), /Skipped --purge/) + assertStreamsSplit(cli.out(), cli.err()) + } finally { + Object.defineProperty(process, 'platform', platform) + await s.cleanup() + } +}) + test('attach all resumes Claude Desktop, which registers no adapter', async () => { const s = await stage() try {