From 63c9788f502f4d4791f75543f2f528e6ebbf8bb3 Mon Sep 17 00:00:00 2001 From: Jamkris Date: Fri, 15 May 2026 15:06:34 +0900 Subject: [PATCH] fix(hooks): scan full costs.jsonl when locating session row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `readSessionCost` read only the trailing 8 KiB of `~/.claude/metrics/costs.jsonl` to "avoid scanning entire file". That ceiling is the opposite-sign sibling of the double-count bug fixed in the previous commit: once a session's most recent cumulative row gets pushed past the 8 KiB window by newer rows from other sessions, the bridge silently reports `totalCost: 0`, `totalIn: 0`, `totalOut: 0` for that session — same false signal to `ecc-context-monitor.js`, same wrong number injected into the live model turn as `additionalContext`. `cost-tracker.js` has no rotation policy, so on any non-trivial workstation costs.jsonl grows past 8 KiB within minutes of normal use. For users who keep multiple concurrent sessions, this means the second-and-later sessions silently report zero almost immediately. Reproduced before this commit: $ HOME=/tmp/eccc node -e ' const fs = require("fs"); const m = require("./scripts/hooks/ecc-metrics-bridge.js"); // S1 row at file start, then 200 rows of OTHER-session noise (~16 KiB). // S1 is the row we want, but it sits past the 8 KiB tail. const s1 = `{"session_id":"S1","estimated_cost_usd":0.5,"input_tokens":500,"output_tokens":250}`; const other = `{"session_id":"OTHER","estimated_cost_usd":1,"input_tokens":100,"output_tokens":50}`; fs.mkdirSync("/tmp/eccc/.claude/metrics", { recursive: true }); fs.writeFileSync("/tmp/eccc/.claude/metrics/costs.jsonl", [s1, ...Array(200).fill(other)].join("\\n") + "\\n"); console.log(JSON.stringify(m.readSessionCost("S1")));' {"totalCost":0,"totalIn":0,"totalOut":0} Expected: `{"totalCost":0.5, "totalIn":500, "totalOut":250}` (the S1 row that exists in the file). Actual: zero — the row is past the 8 KiB tail. Fix: drop the `fs.openSync` + bounded `fs.readSync` + position arithmetic in favour of `fs.readFileSync(costsPath, 'utf8')` and iterate every line. Each row is ~150 bytes; even 100k rows is ~15 MB and a single sync read on PreToolUse is in the low ms. If file rotation lands in `cost-tracker.js` later, this scan becomes proportionally cheaper. After this commit the reproduction above returns `{"totalCost":0.5, "totalIn":500, "totalOut":250}`. Regression test in `tests/hooks/ecc-metrics-bridge.test.js`: `readSessionCost finds session row beyond the old 8 KiB tail boundary`. The test asserts the costs.jsonl fixture is > 8 KiB before reading so any reintroduction of a bounded tail would re-fail the test (i.e. the assertion is the contract, not the specific number 8192). Together with the previous commit, both directions of the metrics-bridge cost-reporting bug are closed. --- scripts/hooks/ecc-metrics-bridge.js | 62 +++++++++++++------------- tests/hooks/ecc-metrics-bridge.test.js | 41 +++++++++++++++++ 2 files changed, 71 insertions(+), 32 deletions(-) diff --git a/scripts/hooks/ecc-metrics-bridge.js b/scripts/hooks/ecc-metrics-bridge.js index f446dd24..5b2c05e6 100644 --- a/scripts/hooks/ecc-metrics-bridge.js +++ b/scripts/hooks/ecc-metrics-bridge.js @@ -77,46 +77,44 @@ function extractFilePaths(toolName, toolInput) { } /** - * Read cumulative cost for a session from the tail of costs.jsonl. - * Reads last 8KB to avoid scanning entire file. + * Read cumulative cost for a session from costs.jsonl. + * + * Scans the full file because each row is a cumulative session total + * (see cost-tracker.js docblock) and the row we need is the last one + * matching `sessionId`. The previous implementation read only the + * trailing 8 KiB; any session whose latest cumulative row was pushed + * past that window by newer rows from other sessions silently dropped + * to zero — the opposite sign of the double-count bug fixed in the + * previous commit. + * + * costs.jsonl is append-only and unbounded today (no rotation in + * cost-tracker.js). At a typical ~150 bytes per row, even 100k rows + * is ~15 MB and a single sync read on every PreToolUse hook is in + * the low milliseconds. If rotation lands later, this scan becomes + * even cheaper. */ function readSessionCost(sessionId) { try { const costsPath = path.join(getClaudeDir(), 'metrics', 'costs.jsonl'); - const stat = fs.statSync(costsPath); - const readSize = Math.min(stat.size, 8192); - const fd = fs.openSync(costsPath, 'r'); - try { - const buf = Buffer.alloc(readSize); - fs.readSync(fd, buf, 0, readSize, Math.max(0, stat.size - readSize)); - const lines = buf.toString('utf8').split('\n').filter(Boolean); + const content = fs.readFileSync(costsPath, 'utf8'); + const lines = content.split('\n').filter(Boolean); - // Each row in costs.jsonl is *already* a cumulative session total — see - // scripts/hooks/cost-tracker.js: "Each row therefore represents the - // cumulative session total up to that point. To get per-session cost, - // take the last row per session_id." Summing every matching row - // therefore double-counts: for N rows of the same session it over- - // reports by roughly N(N+1)/2 / N = (N+1)/2 ×. Take the last matching - // row instead. - let totalCost = 0; - let totalIn = 0; - let totalOut = 0; - for (const line of lines) { - try { - const row = JSON.parse(line); - if (row.session_id === sessionId) { - totalCost = toNumber(row.estimated_cost_usd); - totalIn = toNumber(row.input_tokens); - totalOut = toNumber(row.output_tokens); - } - } catch { - /* skip malformed lines */ + let totalCost = 0; + let totalIn = 0; + let totalOut = 0; + for (const line of lines) { + try { + const row = JSON.parse(line); + if (row.session_id === sessionId) { + totalCost = toNumber(row.estimated_cost_usd); + totalIn = toNumber(row.input_tokens); + totalOut = toNumber(row.output_tokens); } + } catch { + /* skip malformed lines */ } - return { totalCost, totalIn, totalOut }; - } finally { - fs.closeSync(fd); } + return { totalCost, totalIn, totalOut }; } catch { return { totalCost: 0, totalIn: 0, totalOut: 0 }; } diff --git a/tests/hooks/ecc-metrics-bridge.test.js b/tests/hooks/ecc-metrics-bridge.test.js index bf1cc72c..109e0b6c 100644 --- a/tests/hooks/ecc-metrics-bridge.test.js +++ b/tests/hooks/ecc-metrics-bridge.test.js @@ -184,6 +184,47 @@ function runTests() { passed++; else failed++; + if ( + test('readSessionCost finds session row beyond the old 8 KiB tail boundary', () => { + // The previous implementation read only the trailing 8 KiB of + // costs.jsonl. A long-running deployment where the target session's + // most recent cumulative row sat further back than that — e.g. + // pushed past by many rows from OTHER sessions — silently saw + // cost=0. This test wedges the S1 row at the file start, fills + // ~16 KiB of OTHER-session noise after it, and asserts the S1 row + // is still found. + const tmpHome = makeTempHome(); + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + try { + process.env.HOME = tmpHome; + process.env.USERPROFILE = tmpHome; + const metricsDir = path.join(tmpHome, '.claude', 'metrics'); + fs.mkdirSync(metricsDir, { recursive: true }); + const otherRow = JSON.stringify({ session_id: 'OTHER', estimated_cost_usd: 1, input_tokens: 100, output_tokens: 50 }); + const s1Row = JSON.stringify({ session_id: 'S1', estimated_cost_usd: 0.5, input_tokens: 500, output_tokens: 250 }); + const rows = [s1Row, ...Array(200).fill(otherRow)]; + fs.writeFileSync(path.join(metricsDir, 'costs.jsonl'), rows.join('\n') + '\n', 'utf8'); + // Confirm we're actually past the old 8 KiB ceiling so the test + // would have failed under the previous implementation. + const size = fs.statSync(path.join(metricsDir, 'costs.jsonl')).size; + assert.ok(size > 8192, `setup: expected costs.jsonl > 8 KiB, got ${size} bytes`); + const result = readSessionCost('S1'); + assert.strictEqual(result.totalCost, 0.5); + assert.strictEqual(result.totalIn, 500); + assert.strictEqual(result.totalOut, 250); + } finally { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + fs.rmSync(tmpHome, { recursive: true, force: true }); + } + }) + ) + passed++; + else failed++; + if ( test('readSessionCost does not include unrelated default-session rows', () => { const tmpHome = makeTempHome();