diff --git a/scripts/local-host-e2e.mjs b/scripts/local-host-e2e.mjs index cd64adb4..d0445c85 100644 --- a/scripts/local-host-e2e.mjs +++ b/scripts/local-host-e2e.mjs @@ -20,10 +20,6 @@ function assert(condition, message) { } } -function compactableOutput(prefix, count) { - return Array.from({ length: count }, (_, index) => `${prefix}/example-${index + 1}.json`).join("\n"); -} - function postToolUsePayload(command, toolResponse) { return `${JSON.stringify({ hook_event_name: "PostToolUse", @@ -126,13 +122,28 @@ async function runCodexE2E() { }); const report = JSON.parse(doctor.stdout); assert(report.status === "ok", `expected Codex doctor status ok, got ${doctor.stdout}`); + const hookEnv = { + CODEX_HOME: codexHome, + // Keep the authoritative-vs-normalized assertions independent of a developer's shell env. + TOKENJUICE_NO_OMISSION: "", + }; const payload = postToolUsePayload( - "find src/rules -maxdepth 2 -type f | head -n 40", - compactableOutput("src/rules", 40), + "git status", + [ + "On branch pr-65478-security-fix", + "Your branch and 'origin/pr-65478-security-fix' have diverged,", + "and have 8 and 642 different commits each, respectively.", + "", + "Changes not staged for commit:", + "\tmodified: src/agents/pi-embedded-runner/run/attempt.prompt-helpers.ts", + "\tmodified: src/agents/pi-embedded-runner/run/attempt.test.ts", + "", + "no changes added to commit", + ].join("\n"), ); const hook = await run(process.execPath, [distCliPath, "codex-post-tool-use"], { - env: { CODEX_HOME: codexHome }, + env: hookEnv, input: payload, }); @@ -141,11 +152,35 @@ async function runCodexE2E() { const additionalContext = output.hookSpecificOutput?.additionalContext; assert(output.hookSpecificOutput?.hookEventName === "PostToolUse", "expected Codex PostToolUse output"); assert(typeof additionalContext === "string", "expected Codex additionalContext"); - assert(additionalContext.includes("40 matches"), "expected Codex hook output to contain compacted match count"); - assert(additionalContext.includes("src/rules/example-1.json"), "expected Codex hook output to include compacted paths"); - assert(additionalContext.includes("tokenjuice wrap --raw -- "), "expected Codex hook output to include raw rerun hint"); + assert(additionalContext.includes("Changes not staged:"), "expected Codex hook output to retain status context"); + assert( + additionalContext.includes("M: src/agents/pi-embedded-runner/run/attempt.prompt-helpers.ts"), + "expected Codex hook output to include compacted status paths", + ); + assert(!additionalContext.includes("and have 8 and 642"), "expected Codex hook output to omit noisy branch details"); + assert( + !additionalContext.includes("tokenjuice wrap --raw -- "), + "expected non-authoritative Codex rewrites to avoid a raw rerun hint", + ); assert(!hook.stdout.includes("\"decision\""), "Codex hook feedback must not emit JSON decision:block output"); + const authoritativeHook = await run(process.execPath, [distCliPath, "codex-post-tool-use"], { + env: hookEnv, + input: postToolUsePayload( + "git log --oneline", + Array.from( + { length: 40 }, + (_, index) => `${(index + 1).toString(16).padStart(7, "a")} feat: commit ${index}`, + ).join("\n"), + ), + }); + const authoritativeOutput = JSON.parse(authoritativeHook.stdout); + const authoritativeContext = authoritativeOutput.hookSpecificOutput?.additionalContext; + assert( + authoritativeContext?.includes("tokenjuice wrap --raw -- "), + "expected authoritative Codex omissions to retain a raw recovery hint", + ); + return { version: version.stdout.trim(), doctor: report.status, diff --git a/src/cli/main.ts b/src/cli/main.ts index d2ca6d2c..1a9b1c8a 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -1298,7 +1298,10 @@ async function runInstall(args: ParsedArgs): Promise { } if (target === "codex") { - const result = await installCodexHook(undefined, { local: args.local }); + const result = await installCodexHook(undefined, { + local: args.local, + ...(args.noOmit ? { noOmit: true } : {}), + }); if (args.format === "json") { process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); return 0; @@ -5056,7 +5059,10 @@ async function runDoctor(args: ParsedArgs): Promise { } if (args.positionals[0] === "codex") { - const report = await doctorCodexHook(undefined, { local: args.local }); + const report = await doctorCodexHook(undefined, { + local: args.local, + ...(args.noOmit ? { noOmit: true } : {}), + }); if (args.format === "json") { process.stdout.write(`${JSON.stringify(report, null, 2)}\n`); @@ -7372,7 +7378,10 @@ async function main(argv = process.argv.slice(2)): Promise { case "stats": return await runStats(args); case "codex-post-tool-use": - return await runCodexPostToolUseHook(await readStdin(args.maxInputBytes)); + return await runCodexPostToolUseHook( + await readStdin(args.maxInputBytes), + { noOmit: args.noOmit }, + ); case "claude-code-pre-tool-use": return await runClaudeCodePreToolUseHook(await readStdin(args.maxInputBytes), args.wrapLauncher); case "claude-code-post-tool-use": diff --git a/src/hosts/codex/index.ts b/src/hosts/codex/index.ts index 77c4abc6..cc0b0239 100644 --- a/src/hosts/codex/index.ts +++ b/src/hosts/codex/index.ts @@ -6,6 +6,8 @@ import packageJson from "../../../package.json" with { type: "json" }; import { stripLeadingCdPrefix } from "../../core/command.js"; import { storeArtifactMetadata } from "../../core/artifacts.js"; +import type { CompactionMetadata } from "../../core/compaction-metadata.js"; +import { readNoOmissionFromEnv } from "../../core/env.js"; import { compactBashResult, getOutputAwareInspectionSkipReason } from "../../core/integrations/compact-bash-result.js"; import { classifyOnly } from "../../core/reduce.js"; import { countTextChars, stripAnsi } from "../../core/text.js"; @@ -100,6 +102,7 @@ export type CodexHookCommandOptions = { local?: boolean; binaryPath?: string; nodePath?: string; + noOmit?: boolean; /** * Override for the config.toml consulted when reporting the * `codex_hooks` feature-flag state. Defaults to `~/.codex/config.toml`. @@ -370,22 +373,30 @@ async function buildCodexHookCommand(options: CodexHookCommandOptions = {}): Pro throw new Error("unable to resolve tokenjuice binary path for codex install"); } + let command: string | undefined; if (!options.local) { const installedBinaryPath = await resolveInstalledTokenjuicePath(); if (installedBinaryPath) { - return `${shellQuote(installedBinaryPath)} codex-post-tool-use`; + command = `${shellQuote(installedBinaryPath)} codex-post-tool-use`; } } - if (binaryPath.endsWith(".js")) { - return `${shellQuote(nodePath)} ${shellQuote(binaryPath)} codex-post-tool-use`; + if (!command) { + command = binaryPath.endsWith(".js") + ? `${shellQuote(nodePath)} ${shellQuote(binaryPath)} codex-post-tool-use` + : `${shellQuote(binaryPath)} codex-post-tool-use`; } - return `${shellQuote(binaryPath)} codex-post-tool-use`; + // Codex launches hooks from its own process, which may not inherit environment variables + // loaded by the Bash tool's login shell. Snapshot no-omit into the command at install time. + return options.noOmit || readNoOmissionFromEnv() ? `${command} --no-omit` : command; } -function getCodexFixCommand(local = false): string { - return local ? "tokenjuice install codex --local" : TOKENJUICE_CODEX_FIX_COMMAND; +function getCodexFixCommand(local = false, noOmit = false): string { + return [ + local ? "tokenjuice install codex --local" : TOKENJUICE_CODEX_FIX_COMMAND, + ...(noOmit ? ["--no-omit"] : []), + ].join(" "); } async function pathExists(path: string): Promise { @@ -731,12 +742,27 @@ function commandRequestsTokenjuiceRawBypass(command: string): boolean { return optionArgs.includes("--raw") || optionArgs.includes("--full"); } -function buildCodexFeedback(inlineText: string, rawRefId?: string): string { +function buildCodexFeedback( + inlineText: string, + rawRefId?: string, + compaction?: CompactionMetadata, +): string { + if (compaction?.authoritative !== true) { + // A rewrite without an authoritative omission marker is already usable as-is. Advertising a + // raw rerun for every normalized result trains the agent to discard compacted context even + // when the reducer did not flag recoverable detail. + return inlineText; + } + return `${inlineText}\n\n${buildCompactionHint(rawRefId)}`; } -function buildCodexReplacementOutput(inlineText: string, rawRefId?: string): Record { - const feedback = buildCodexFeedback(inlineText, rawRefId); +function buildCodexReplacementOutput( + inlineText: string, + rawRefId?: string, + compaction?: CompactionMetadata, +): Record { + const feedback = buildCodexFeedback(inlineText, rawRefId, compaction); return { hookSpecificOutput: { hookEventName: "PostToolUse", @@ -831,8 +857,9 @@ export async function doctorCodexHook( hooksPath = getDefaultHooksPath(), options: CodexHookCommandOptions = {}, ): Promise { + const noOmit = options.noOmit || readNoOmissionFromEnv(); const expectedCommand = await buildCodexHookCommand(options); - const installFixCommand = getCodexFixCommand(options.local); + const installFixCommand = getCodexFixCommand(options.local, noOmit); let fixCommand = installFixCommand; const { config, exists } = await readHooksConfig(hooksPath); const detectedCommand = findTokenjuiceCodexHookCommand(config); @@ -895,7 +922,7 @@ export async function doctorCodexHook( } if (options.local && await detectStaleLocalBuild(checkedPaths)) { issues.push("local Codex hook target is older than the source tree"); - fixCommand = "pnpm build && tokenjuice install codex --local"; + fixCommand = `pnpm build && ${getCodexFixCommand(true, noOmit)}`; } if (!featureFlag.enabled) { issues.push( @@ -1078,7 +1105,10 @@ async function recordImmediateHookStats( ); } -export async function runCodexPostToolUseHook(rawText: string): Promise { +export async function runCodexPostToolUseHook( + rawText: string, + options: { noOmit?: boolean } = {}, +): Promise { let payload: CodexPostToolUsePayload; try { payload = JSON.parse(rawText) as CodexPostToolUsePayload; @@ -1087,10 +1117,12 @@ export async function runCodexPostToolUseHook(rawText: string): Promise } const command = payload.tool_input?.command; + const noOmit = options.noOmit || readNoOmissionFromEnv(); const debug: Record = { hookEvent: payload.hook_event_name, toolName: payload.tool_name, command, + noOmit, rewrote: false, }; @@ -1159,6 +1191,7 @@ export async function runCodexPostToolUseHook(rawText: string): Promise ...(typeof payload.cwd === "string" && payload.cwd.trim() ? { cwd: payload.cwd } : {}), ...(typeof exitCode === "number" ? { exitCode } : {}), ...(typeof maxInlineChars === "number" ? { maxInlineChars } : {}), + ...(noOmit ? { noOmit: true } : {}), storeRaw, metadata: { source: "codex-post-tool-use", @@ -1179,6 +1212,7 @@ export async function runCodexPostToolUseHook(rawText: string): Promise debug.savedChars = savedChars; debug.ratio = result.stats.ratio; debug.matchedReducer = result.classification.matchedReducer; + debug.compaction = result.compaction; } if (outcome.action === "keep") { @@ -1186,7 +1220,11 @@ export async function runCodexPostToolUseHook(rawText: string): Promise return 0; } - process.stdout.write(`${JSON.stringify(buildCodexReplacementOutput(outcome.result.inlineText, outcome.result.rawRef?.id))}\n`); + process.stdout.write(`${JSON.stringify(buildCodexReplacementOutput( + outcome.result.inlineText, + outcome.result.rawRef?.id, + outcome.result.compaction, + ))}\n`); await writeHookDebug({ ...debug, rewrote: true }); return 0; } catch (error) { diff --git a/test/cli/main.test.ts b/test/cli/main.test.ts index 580a4fe3..f8073256 100644 --- a/test/cli/main.test.ts +++ b/test/cli/main.test.ts @@ -25,6 +25,7 @@ describe("parseArgs", () => { it("parses --no-omit for reduce and wrap", () => { expect(parseArgs(["reduce", "--no-omit"]).noOmit).toBe(true); expect(parseArgs(["wrap", "--no-omit", "--", "echo", "hi"]).noOmit).toBe(true); + expect(parseArgs(["codex-post-tool-use", "--no-omit"]).noOmit).toBe(true); }); }); diff --git a/test/hosts/codex.test.ts b/test/hosts/codex.test.ts index d6a3643a..3608f7af 100644 --- a/test/hosts/codex.test.ts +++ b/test/hosts/codex.test.ts @@ -3,7 +3,7 @@ import { mkdir, mkdtemp, readFile, rm, symlink, utimes, writeFile } from "node:f import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { doctorCodexHook, installCodexHook, listArtifactMetadata, runCodexPostToolUseHook, uninstallCodexHook } from "../../src/index.js"; @@ -11,11 +11,22 @@ const tempDirs: string[] = []; const PACKAGE_VERSION = JSON.parse(readFileSync(new URL("../../package.json", import.meta.url), "utf8")).version as string; const originalHome = process.env.HOME; const originalPath = process.env.PATH; +const originalNoOmission = process.env.TOKENJUICE_NO_OMISSION; + +beforeEach(() => { + // These assertions describe the default reducer policy, not a caller's opt-out environment. + delete process.env.TOKENJUICE_NO_OMISSION; +}); afterEach(async () => { delete process.env.CODEX_HOME; process.env.HOME = originalHome; process.env.PATH = originalPath; + if (originalNoOmission === undefined) { + delete process.env.TOKENJUICE_NO_OMISSION; + } else { + process.env.TOKENJUICE_NO_OMISSION = originalNoOmission; + } await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); }); @@ -169,6 +180,28 @@ describe("installCodexHook", () => { expect(parsed.hooks.PostToolUse?.[0]?.hooks[0]?.command).toBe(`${launcherPath} codex-post-tool-use`); }); + it("captures no-omit mode in the installed hook command", async () => { + const home = await createTempDir(); + const hooksPath = join(home, "hooks.json"); + const binDir = join(home, "bin"); + const launcherPath = join(binDir, "tokenjuice"); + + process.env.PATH = binDir; + process.env.TOKENJUICE_NO_OMISSION = "1"; + await mkdir(binDir, { recursive: true }); + await writeFile(launcherPath, "#!/usr/bin/env bash\nexit 0\n", { encoding: "utf8", mode: 0o755 }); + + const result = await installCodexHook(hooksPath); + const parsed = JSON.parse(await readFile(hooksPath, "utf8")) as { + hooks: Record }>>; + }; + + expect(result.command).toBe(`${launcherPath} codex-post-tool-use --no-omit`); + expect(parsed.hooks.PostToolUse?.[0]?.hooks[0]?.command).toBe( + `${launcherPath} codex-post-tool-use --no-omit`, + ); + }); + it("can install a local codex hook without preferring PATH", async () => { const home = await createTempDir(); const hooksPath = join(home, "hooks.json"); @@ -261,6 +294,29 @@ describe("doctorCodexHook", () => { expect(report.featureFlag.enabled).toBe(true); }); + it("reports a stale hook when no-omit mode is not captured", async () => { + const home = await createTempDir(); + const hooksPath = join(home, "hooks.json"); + const binDir = join(home, "bin"); + const launcherPath = join(binDir, "tokenjuice"); + + process.env.PATH = binDir; + await mkdir(binDir, { recursive: true }); + await writeFile(launcherPath, "#!/usr/bin/env bash\nexit 0\n", { encoding: "utf8", mode: 0o755 }); + await installCodexHook(hooksPath); + + process.env.TOKENJUICE_NO_OMISSION = "1"; + const report = await doctorCodexHook(hooksPath); + + expect(report.status).toBe("warn"); + expect(report.expectedCommand).toBe(`${launcherPath} codex-post-tool-use --no-omit`); + expect(report.detectedCommand).toBe(`${launcherPath} codex-post-tool-use`); + expect(report.fixCommand).toBe("tokenjuice install codex --no-omit"); + expect(report.issues).toContain( + "configured Codex hook command does not match the current recommended command", + ); + }); + it("warns when the stable launcher resolves to an older Homebrew tokenjuice version", async () => { const home = await createTempDir(); const hooksPath = join(home, "hooks.json"); @@ -612,6 +668,10 @@ describe("runCodexPostToolUseHook", () => { const debug = JSON.parse(await readFile(join(home, "tokenjuice-hook.last.json"), "utf8")) as { rewrote: boolean; matchedReducer?: string; + compaction?: { + authoritative?: boolean; + kinds?: string[]; + }; }; const response = parseCodexReplacementOutput(stdout); @@ -622,10 +682,120 @@ describe("runCodexPostToolUseHook", () => { expect(response.hookSpecificOutput?.additionalContext).toContain("Changes not staged:"); expect(response.hookSpecificOutput?.additionalContext).toContain("M: src/agents/pi-embedded-runner/run/attempt.prompt-helpers.ts"); expect(response.hookSpecificOutput?.additionalContext).not.toContain("and have 8 and 642"); - expect(response.hookSpecificOutput?.additionalContext).toContain("tokenjuice wrap --raw -- "); + expect(response.hookSpecificOutput?.additionalContext).not.toContain("tokenjuice wrap --raw -- "); expect(response.hookSpecificOutput?.additionalContext).not.toContain("tokenjuice wrap --full -- "); expect(debug.rewrote).toBe(true); expect(debug.matchedReducer).toBe("git/status"); + expect(debug.compaction?.authoritative).toBe(false); + }); + + it("does not suggest a raw rerun for a formatting-only rewrite", async () => { + const home = await createTempDir(); + process.env.CODEX_HOME = home; + + const payload = JSON.stringify({ + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { + command: "custom-tool --emit-json", + }, + tool_response: JSON.stringify({ + status: "ok", + files: Array.from({ length: 18 }, (_, index) => ({ + path: `src/file-${index}.ts`, + changed: true, + })), + }, null, 2), + }); + + const { code, stdout, stderr } = await captureStdio(() => runCodexPostToolUseHook(payload)); + const debug = JSON.parse(await readFile(join(home, "tokenjuice-hook.last.json"), "utf8")) as { + rewrote: boolean; + compaction?: { + authoritative?: boolean; + kinds?: string[]; + }; + }; + const response = parseCodexReplacementOutput(stdout); + + expect(code).toBe(0); + expect(stderr).toBe(""); + expect(debug.rewrote).toBe(true); + expect(debug.compaction?.authoritative).not.toBe(true); + expect(response.hookSpecificOutput?.additionalContext).not.toContain("tokenjuice wrap --raw -- "); + expect(response.hookSpecificOutput?.additionalContext).toContain('"status":"ok"'); + }); + + it("keeps a raw recovery hint for authoritative omissions", async () => { + const home = await createTempDir(); + process.env.CODEX_HOME = home; + + const payload = JSON.stringify({ + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { + command: "git log --oneline", + }, + tool_response: Array.from( + { length: 40 }, + (_, index) => `${(index + 1).toString(16).padStart(7, "a")} feat: commit ${index}`, + ).join("\n"), + }); + + const { code, stdout, stderr } = await captureStdio(() => runCodexPostToolUseHook(payload)); + const debug = JSON.parse(await readFile(join(home, "tokenjuice-hook.last.json"), "utf8")) as { + rewrote: boolean; + compaction?: { + authoritative?: boolean; + kinds?: string[]; + }; + }; + const response = parseCodexReplacementOutput(stdout); + + expect(code).toBe(0); + expect(stderr).toBe(""); + expect(debug.rewrote).toBe(true); + expect(debug.compaction?.authoritative).toBe(true); + expect(response.hookSpecificOutput?.additionalContext).toContain("tokenjuice wrap --raw -- "); + }); + + it("keeps the original output when the installed hook explicitly enables no-omit", async () => { + const home = await createTempDir(); + process.env.CODEX_HOME = home; + + const payload = JSON.stringify({ + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { + command: "git log --oneline", + }, + tool_response: Array.from( + { length: 40 }, + (_, index) => `${(index + 1).toString(16).padStart(7, "a")} feat: commit ${index}`, + ).join("\n"), + }); + + const { code, stdout, stderr } = await captureStdio( + () => runCodexPostToolUseHook(payload, { noOmit: true }), + ); + const debug = JSON.parse(await readFile(join(home, "tokenjuice-hook.last.json"), "utf8")) as { + noOmit?: boolean; + rewrote: boolean; + skipped?: string; + compaction?: { + authoritative?: boolean; + kinds?: string[]; + }; + }; + + expect(code).toBe(0); + expect(stdout).toBe(""); + expect(stderr).toBe(""); + expect(debug.noOmit).toBe(true); + expect(debug.rewrote).toBe(false); + expect(debug.skipped).toBe("no-compaction"); + expect(debug.compaction?.authoritative).toBe(false); + expect(debug.compaction?.kinds).toContain("no-omit-head-tail-passthrough"); }); it("skips rewriting generic fallback output for compound shell diagnostics", async () => { @@ -750,6 +920,10 @@ describe("runCodexPostToolUseHook", () => { reducedChars?: number; savedChars?: number; ratio?: number; + compaction?: { + authoritative?: boolean; + kinds?: string[]; + }; }; const response = parseCodexReplacementOutput(stdout); @@ -762,10 +936,12 @@ describe("runCodexPostToolUseHook", () => { expect(debug.rewrote).toBe(true); expect(debug.skipped).toBeUndefined(); expect(debug.matchedReducer).toBe("filesystem/find"); + expect(debug.compaction?.authoritative).toBe(true); expect(debug.rawChars).toBeGreaterThan(0); expect(debug.reducedChars).toBeLessThan(debug.rawChars!); expect(debug.savedChars).toBeGreaterThan(0); expect(debug.ratio).toBeLessThan(1); + expect(response.hookSpecificOutput?.additionalContext).toContain("tokenjuice wrap --raw -- "); }); it("skips auto-rewrite for file-content inspection commands", async () => {