diff --git a/scripts/hooks/ecc-context-monitor.js b/scripts/hooks/ecc-context-monitor.js index 62b92287a..84941ea97 100644 --- a/scripts/hooks/ecc-context-monitor.js +++ b/scripts/hooks/ecc-context-monitor.js @@ -21,7 +21,12 @@ const COST_NOTICE_USD = 5; const COST_WARNING_USD = 10; const COST_CRITICAL_USD = 50; const FILES_WARNING_COUNT = 20; -const LOOP_THRESHOLD = 3; +// The recent_tools ring buffer holds 5 entries (RECENT_TOOLS_SIZE in +// ecc-metrics-bridge.js), so 5 means ALL of the last 5 calls must be the +// identical tool+params before a LOOP WARNING fires. At 3, three repeats of +// a legitimate command (retries, polling) among five mixed calls fired a +// false warning. +const LOOP_THRESHOLD = 5; const STALE_SECONDS = 60; function isEnabledEnv(value, defaultValue = true) { @@ -56,7 +61,7 @@ function readWarnState(sessionId) { try { return JSON.parse(fs.readFileSync(getWarnPath(sessionId), 'utf8')); } catch { - return { callsSinceWarn: 0, lastSeverity: null, lastMessage: null }; + return { callsSinceWarn: 0, lastSeverity: null, lastKey: null }; } } @@ -123,6 +128,7 @@ function evaluateConditions(bridge, options = {}) { warnings.push({ severity: 3, type: 'context', + dedupeKey: 'context:critical', message: `CONTEXT CRITICAL: ${remaining}% remaining. Context nearly exhausted. ` + 'Inform the user that context is low and ask how they want to proceed. ' + @@ -132,6 +138,7 @@ function evaluateConditions(bridge, options = {}) { warnings.push({ severity: 2, type: 'context', + dedupeKey: 'context:warning', message: `CONTEXT WARNING: ${remaining}% remaining. ` + 'Be aware that context is getting limited. Avoid starting new complex work.' }); } @@ -144,18 +151,21 @@ function evaluateConditions(bridge, options = {}) { warnings.push({ severity: 3, type: 'cost', + dedupeKey: 'cost:critical', message: `COST CRITICAL: session total ~$${cost.toFixed(2)} (over $${COST_CRITICAL_USD}). Informational only — not an instruction to stop.` }); } else if (cost > COST_WARNING_USD) { warnings.push({ severity: 2, type: 'cost', + dedupeKey: 'cost:warning', message: `COST WARNING: session total ~$${cost.toFixed(2)} (over $${COST_WARNING_USD}). Informational only.` }); } else if (cost > COST_NOTICE_USD) { warnings.push({ severity: 1, type: 'cost', + dedupeKey: 'cost:notice', message: `COST NOTICE: session total ~$${cost.toFixed(2)}. Informational only.` }); } @@ -167,6 +177,7 @@ function evaluateConditions(bridge, options = {}) { warnings.push({ severity: 2, type: 'scope', + dedupeKey: 'scope', message: `SCOPE WARNING: ${fileCount} files modified this session. ` + 'Consider whether changes are too scattered.' }); } @@ -177,6 +188,8 @@ function evaluateConditions(bridge, options = {}) { warnings.push({ severity: 2, type: 'loop', + // The message itself is a stable key: same tool looping again is a + // duplicate; a different tool or count is a new event. message: `LOOP WARNING: Tool '${loop.tool}' called ${loop.count} times ` + 'with same parameters in last 5 calls. This may indicate a stuck loop.' }); } @@ -224,37 +237,38 @@ function run(rawInput) { // duplicate. Only write when there is state to clear — most tool calls // have no warning, and this keeps the common path free of disk writes. const prior = readWarnState(sessionId); - if (prior.lastMessage) { - writeWarnState(sessionId, { callsSinceWarn: 0, lastSeverity: null, lastMessage: null }); + if (prior.lastKey || prior.lastMessage) { + writeWarnState(sessionId, { callsSinceWarn: 0, lastSeverity: null, lastKey: null }); } return rawInput; } // Combine top 2 warnings - const message = warnings - .slice(0, 2) - .map(w => w.message) - .join('\n'); + const top = warnings.slice(0, 2); + const message = top.map(w => w.message).join('\n'); - // Dedupe on message content, not a call counter. The previous logic - // re-emitted the *same* warning every DEBOUNCE_CALLS tool calls, so a - // single unchanged condition (e.g. a cost figure that only refreshes at - // turn boundaries) printed the identical line ~20 times in one turn. Now a - // warning is surfaced only when its text changes (cost moved, a new file - // count, a new loop) or when we newly escalate to critical — genuinely new - // information — and is otherwise suppressed. + // Dedupe on the warning TIER (dedupeKey), not the message text. Message + // text embeds continuously-moving numbers (cost in dollars, context %), + // so text-based dedupe re-emitted the "same" warning on nearly every + // tool call — a COST NOTICE fired once per call for the rest of the + // session once cost passed $5. Each tier now fires once (notice → + // warning → critical each re-fire on escalation), and a genuinely new + // event (different loop, tier change) still surfaces. + const dedupeKey = top.map(w => w.dedupeKey || w.message).join('\n'); const warnState = readWarnState(sessionId); const topSeverity = severityLabel(warnings[0].severity); const escalatedToCritical = topSeverity === 'critical' && warnState.lastSeverity !== 'critical'; - const sameMessage = warnState.lastMessage === message; + const sameKey = warnState.lastKey === dedupeKey; - if (sameMessage && !escalatedToCritical) { + if (sameKey && !escalatedToCritical) { return rawInput; } - warnState.lastSeverity = topSeverity; - warnState.lastMessage = message; - writeWarnState(sessionId, warnState); + writeWarnState(sessionId, { + ...warnState, + lastSeverity: topSeverity, + lastKey: dedupeKey, + }); const output = { hookSpecificOutput: { diff --git a/scripts/hooks/ecc-metrics-bridge.js b/scripts/hooks/ecc-metrics-bridge.js index bd8cb39da..cbecd4536 100644 --- a/scripts/hooks/ecc-metrics-bridge.js +++ b/scripts/hooks/ecc-metrics-bridge.js @@ -47,7 +47,11 @@ function hashToolCall(toolName, toolInput) { const name = String(toolName || ''); let key = ''; if (name === 'Bash') { - key = String(toolInput?.command || '').slice(0, 160); + // Hash the FULL command (digest, not a prefix slice): taking the first + // 160 chars collided distinct long commands that share a common prefix + // (heredocs, long one-liners), so consecutive DIFFERENT Bash calls looked + // like a stuck loop and triggered false LOOP WARNINGs. + key = crypto.createHash('sha256').update(String(toolInput?.command || '')).digest('hex'); } else if (/^(Edit|MultiEdit|Write|NotebookEdit)$/.test(name)) { // Fingerprint the actual change, not just the path. Hashing on file_path // alone made every distinct edit to the same file collide, so a few normal diff --git a/tests/hooks/ecc-context-monitor.test.js b/tests/hooks/ecc-context-monitor.test.js index 38ee8ef33..c62084f51 100644 --- a/tests/hooks/ecc-context-monitor.test.js +++ b/tests/hooks/ecc-context-monitor.test.js @@ -176,6 +176,40 @@ function runTests() { passed++; else failed++; + if ( + test('cost warnings dedupe by tier: notice fires once, re-fires on escalation', () => { + const sessionId = `ctx-monitor-tier-dedupe-${process.pid}-${Date.now()}`; + const warnPath = path.join(os.tmpdir(), `ecc-ctx-warn-${sessionId}.json`); + const input = JSON.stringify({ session_id: sessionId, tool_name: 'Bash' }); + const setCost = cost => + writeBridgeAtomic(sessionId, { total_cost_usd: cost, last_timestamp: new Date().toISOString() }); + try { + setCost(6); + const first = run(input); + assert.ok( + JSON.parse(first).hookSpecificOutput.additionalContext.includes('COST NOTICE'), + 'first crossing of the notice threshold must emit' + ); + + setCost(6.4); // cost ticks up within the same tier — must stay silent + const second = run(input); + assert.strictEqual(second, input, 'same tier must not re-emit on every cost tick'); + + setCost(12); // tier escalation notice → warning must re-emit + const third = run(input); + assert.ok( + JSON.parse(third).hookSpecificOutput.additionalContext.includes('COST WARNING'), + 'tier escalation must re-emit' + ); + } finally { + fs.rmSync(getBridgePath(sessionId), { force: true }); + fs.rmSync(warnPath, { force: true }); + } + }) + ) + passed++; + else failed++; + // evaluateConditions — scope warnings console.log('\nevaluateConditions (scope):'); @@ -205,16 +239,30 @@ function runTests() { console.log('\ndetectLoop:'); if ( - test('3 identical entries returns detected true', () => { - const entries = [ - { tool: 'Bash', hash: 'aabbccdd' }, - { tool: 'Bash', hash: 'aabbccdd' }, - { tool: 'Bash', hash: 'aabbccdd' } - ]; + test('5 identical entries returns detected true', () => { + const entries = Array(5).fill({ tool: 'Bash', hash: 'aabbccdd' }); const result = detectLoop(entries); assert.strictEqual(result.detected, true); assert.strictEqual(result.tool, 'Bash'); - assert.ok(result.count >= 3); + assert.ok(result.count >= 5); + }) + ) + passed++; + else failed++; + + if ( + test('4 identical among 5 entries returns detected false', () => { + // Legitimate repetition (retries, polling) must not fire: only a full + // ring buffer of identical calls counts as a stuck loop. + const entries = [ + { tool: 'Bash', hash: 'aabbccdd' }, + { tool: 'Bash', hash: 'aabbccdd' }, + { tool: 'Bash', hash: 'aabbccdd' }, + { tool: 'Bash', hash: 'aabbccdd' }, + { tool: 'Bash', hash: 'ffffffff' } + ]; + const result = detectLoop(entries); + assert.strictEqual(result.detected, false); }) ) passed++; diff --git a/tests/hooks/ecc-metrics-bridge.test.js b/tests/hooks/ecc-metrics-bridge.test.js index 3046f0405..4bbf3fe35 100644 --- a/tests/hooks/ecc-metrics-bridge.test.js +++ b/tests/hooks/ecc-metrics-bridge.test.js @@ -97,6 +97,21 @@ function runTests() { passed++; else failed++; + if ( + test('long Bash commands diverging only after 160 chars still hash differently', () => { + // Shared prefix longer than the old 160-char command slice; the + // commands differ only afterwards (heredocs, long one-liners). Hashing + // the full command must keep them distinct, otherwise consecutive + // different Bash calls look like a stuck loop. + const prefix = 'python3 - < { // Shared prefix longer than the old HASH_INPUT_LIMIT (2048) truncation