mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-03 06:34:16 +08:00
fix(hooks): context-monitor noise — loop-detection false positives + per-call cost-warning spam (#2486)
* fix(hooks): context-monitor noise — loop-detection false positives and per-call cost-warning spam Two independent noise sources in the PostToolUse context monitor injected agent-facing warnings on nearly every tool call: 1. LOOP WARNING false positives. hashToolCall() hashed only the first 160 chars of a Bash command, so distinct long commands sharing a prefix (heredocs, long one-liners) collided and consecutive DIFFERENT calls looked like a stuck loop. Additionally LOOP_THRESHOLD=3 against a 5-entry ring buffer fired on legitimate repetition (retries, polling). Fix: hash the full command (digest truncated, not the input — same treatment the Edit/Write branch already got), and require all 5 of the last 5 calls to be identical before warning. 2. COST NOTICE spam. run() deduped warnings on exact message text, but the cost figure embedded in the text moves on nearly every call, so once a session crossed $5 a 'new' COST NOTICE was injected per tool call for the rest of the session. Context warnings had the same defect via the remaining-% figure. Fix: dedupe on a stable per-tier key (cost:notice/warning/critical, context:warning/critical, scope) so each tier fires exactly once and re-fires only on genuine escalation. The existing ECC_CONTEXT_MONITOR_COST_WARNINGS opt-out is unchanged. Tests: loop threshold updated (5-of-5 fires, 4-of-5 does not), long shared-prefix Bash hash regression, and a run()-level tier-dedupe test (notice fires once, silent on cost tick, re-emits on escalation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: keep context warning state immutable --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com>
This commit is contained in:
@@ -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: {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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++;
|
||||
|
||||
@@ -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 - <<EOF\n' + '# '.repeat(120);
|
||||
const h1 = hashToolCall('Bash', { command: prefix + 'print(1)\nEOF' });
|
||||
const h2 = hashToolCall('Bash', { command: prefix + 'print(2)\nEOF' });
|
||||
assert.notStrictEqual(h1, h2);
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('large edits diverging only after 2048 chars still hash differently', () => {
|
||||
// Shared prefix longer than the old HASH_INPUT_LIMIT (2048) truncation
|
||||
|
||||
Reference in New Issue
Block a user