From d2cf7c8904f3767d4c341ffb22aae5950a442969 Mon Sep 17 00:00:00 2001 From: masonxhuang Date: Fri, 31 Jul 2026 15:32:26 +0800 Subject: [PATCH] fix(artifacts): fail open when optional stats cannot be written --- src/core/artifacts.ts | 13 +++++++++++++ src/core/reduce.ts | 14 +++++++------- src/hosts/codex/index.ts | 4 ++-- test/core/wrap.test.ts | 39 ++++++++++++++++++++++++++++++++++++++- test/hosts/codex.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/src/core/artifacts.ts b/src/core/artifacts.ts index d246f944..8f262b97 100644 --- a/src/core/artifacts.ts +++ b/src/core/artifacts.ts @@ -153,6 +153,19 @@ export async function storeArtifactMetadata(input: StoredArtifactInput, storeDir }; } +export async function tryStoreArtifactMetadata( + input: StoredArtifactInput, + storeDir?: string, +): Promise { + try { + return await storeArtifactMetadata(input, storeDir); + } catch { + // Stats are ancillary: a read-only or unavailable artifact directory must not discard the + // command result. Explicit artifact storage remains strict through storeArtifact(). + return undefined; + } +} + export async function getArtifact(id: string, storeDir?: string): Promise { if (!isValidArtifactId(id)) { return null; diff --git a/src/core/reduce.ts b/src/core/reduce.ts index 46d8f6a8..c08f2df5 100644 --- a/src/core/reduce.ts +++ b/src/core/reduce.ts @@ -4,7 +4,7 @@ import { classifyExecution, resolveRuleMatch } from "./classify.js"; import { isFileContentInspectionCommand, isVerbatimConfigInspectionCommand } from "./command-identity.js"; import { normalizeExecutionInput } from "./execution-input.js"; import { clampTextMiddleWithMetadata, clampTextWithMetadata, countTextChars, dedupeAdjacent, headTail, normalizeLines, pluralize, stripAnsi, trimEmptyEdges } from "./text.js"; -import { storeArtifact, storeArtifactMetadata } from "./artifacts.js"; +import { storeArtifact, tryStoreArtifactMetadata } from "./artifacts.js"; import { NO_COMPACTION_METADATA, mergeCompactionMetadata, type CompactionMetadata } from "./compaction-metadata.js"; import { buildGithubActionsFailureSummary } from "./github-actions-summary.js"; import { rewriteGhLines, rewriteGitDiffLines, rewriteGitStatusLines, rewriteSearchLines } from "./reduce-formatters.js"; @@ -373,7 +373,7 @@ export async function reduceExecutionWithRules( ) : undefined; if (!opts.store && opts.recordStats) { - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input: normalizedInput, rawText, @@ -432,7 +432,7 @@ export async function reduceExecutionWithRules( : undefined; if (!opts.store && opts.recordStats) { - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input: normalizedInput, rawText, @@ -456,7 +456,7 @@ export async function reduceExecutionWithRules( if (classification.matchedReducer === "generic/fallback" && isFileContentInspectionCommand(normalizedInput)) { if (!opts.store && opts.recordStats) { - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input: normalizedInput, rawText, @@ -515,7 +515,7 @@ export async function reduceExecutionWithRules( : undefined; if (!opts.store && opts.recordStats) { - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input: normalizedInput, rawText, @@ -565,7 +565,7 @@ export async function reduceExecutionWithRules( : undefined; if (!opts.store && opts.recordStats) { - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input: normalizedInput, rawText, @@ -611,7 +611,7 @@ export async function reduceExecutionWithRules( : undefined; if (!opts.store && opts.recordStats) { - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input: normalizedInput, rawText, diff --git a/src/hosts/codex/index.ts b/src/hosts/codex/index.ts index 77c4abc6..e230dd12 100644 --- a/src/hosts/codex/index.ts +++ b/src/hosts/codex/index.ts @@ -5,7 +5,7 @@ import { homedir } from "node:os"; import packageJson from "../../../package.json" with { type: "json" }; import { stripLeadingCdPrefix } from "../../core/command.js"; -import { storeArtifactMetadata } from "../../core/artifacts.js"; +import { tryStoreArtifactMetadata } from "../../core/artifacts.js"; import { compactBashResult, getOutputAwareInspectionSkipReason } from "../../core/integrations/compact-bash-result.js"; import { classifyOnly } from "../../core/reduce.js"; import { countTextChars, stripAnsi } from "../../core/text.js"; @@ -1064,7 +1064,7 @@ async function recordImmediateHookStats( const stats = buildImmediateSkipStats(rawText); const classification = await classifyOnly(input); - await storeArtifactMetadata( + await tryStoreArtifactMetadata( { input, rawText, diff --git a/test/core/wrap.test.ts b/test/core/wrap.test.ts index 07069553..cc10d682 100644 --- a/test/core/wrap.test.ts +++ b/test/core/wrap.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, rm } from "node:fs/promises"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -93,6 +93,43 @@ describe("runWrappedCommand", () => { expect(wrapped.result.stats.ratio).toBe(1); }); + it("preserves raw output when optional stats cannot be written", async () => { + const tempDir = await createTempDir(); + const blockedStoreDir = join(tempDir, "artifact-dir-is-a-file"); + await writeFile(blockedStoreDir, "not a directory", "utf8"); + + const wrapped = await runWrappedCommand([ + process.execPath, + "-e", + "require('node:fs').writeSync(1, 'raw marker\\n'); process.exit(7);", + ], { + raw: true, + recordStats: true, + storeDir: blockedStoreDir, + }); + + expect(wrapped.exitCode).toBe(7); + expect(wrapped.stdout).toBe("raw marker\n"); + expect(wrapped.result.inlineText).toBe("raw marker\n"); + expect(wrapped.result.stats.ratio).toBe(1); + }); + + it("keeps explicit artifact storage failures strict", async () => { + const tempDir = await createTempDir(); + const blockedStoreDir = join(tempDir, "artifact-dir-is-a-file"); + await writeFile(blockedStoreDir, "not a directory", "utf8"); + + await expect(runWrappedCommand([ + process.execPath, + "-e", + "require('node:fs').writeSync(1, 'raw marker\\n');", + ], { + raw: true, + store: true, + storeDir: blockedStoreDir, + })).rejects.toThrow(/EEXIST|ENOTDIR/u); + }); + it("records the requested source for wrapper stats", async () => { const storeDir = await createTempDir(); diff --git a/test/hosts/codex.test.ts b/test/hosts/codex.test.ts index d6a3643a..dd54a644 100644 --- a/test/hosts/codex.test.ts +++ b/test/hosts/codex.test.ts @@ -944,6 +944,44 @@ describe("runCodexPostToolUseHook", () => { expect(debug.ratio).toBe(1); }); + it("keeps raw bypasses fail-open when optional stats cannot be written", async () => { + const home = await createTempDir(); + const blockedArtifactDir = join(home, "artifact-dir-is-a-file"); + const originalArtifactDir = process.env.TOKENJUICE_ARTIFACT_DIR; + process.env.CODEX_HOME = home; + process.env.TOKENJUICE_ARTIFACT_DIR = blockedArtifactDir; + await writeFile(blockedArtifactDir, "not a directory", "utf8"); + + try { + const payload = JSON.stringify({ + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { + command: "tokenjuice wrap --raw -- bash -lc 'git show HEAD --stat'", + }, + tool_response: "commit abcdef\n README.md | 1 +\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; + skipped?: string; + }; + + expect(code).toBe(0); + expect(stdout).toBe(""); + expect(stderr).toBe(""); + expect(debug.rewrote).toBe(false); + expect(debug.skipped).toBe("explicit-raw-bypass"); + } finally { + if (originalArtifactDir === undefined) { + delete process.env.TOKENJUICE_ARTIFACT_DIR; + } else { + process.env.TOKENJUICE_ARTIFACT_DIR = originalArtifactDir; + } + } + }); + it("honors absolute tokenjuice raw bypass commands without re-compacting them", async () => { const home = await createTempDir(); process.env.CODEX_HOME = home;