Просмотр исходного кода

fix(hooks): count confirmed-identical results, not invocations

The previous counter advanced in tool.execute.before. Overlapping parallel
calls (multiple same-args calls launched before any result returns) could
inflate the count to the block threshold before the after-hook output
reset ran, refusing a legitimate call that was making progress.

The run counter now advances only in tool.execute.after, and only when an
identical-args call returned an output byte-identical to the prior call.
A changed result restarts the run, so it never accumulates toward a block;
tool.execute.before refuses only once a run of BLOCK_AT confirmed
identical results exists, and overlapping calls cannot inflate it.

Adds overlap-specific tests; updates block-threshold tests to the
confirmed-run semantics (5 identical results confirmed -> 6th identical
call refused).
Michael Henke 3 недель назад
Родитель
Сommit
23c6446cd8
3 измененных файлов с 119 добавлено и 75 удалено
  1. 13 8
      docs/tools.md
  2. 53 44
      src/hooks/tool-loop-guard/hook.ts
  3. 53 23
      src/hooks/tool-loop-guard/index.test.ts

+ 13 - 8
docs/tools.md

@@ -82,14 +82,19 @@ A safety net for model-side infinite loops where a sub-agent (e.g. a model
 that can degenerate, such as DeepSeek V4 Flash in Explorer) re-issues the
 exact same tool call with identical arguments and gets identical results,
 making no progress. The plugin watches each session's consecutive identical
-tool calls and responds:
-
-- On the 3rd identical call: appends a corrective notice to the tool output
-  telling the model to stop repeating and change approach. Applies to all
-  tools.
-- On the 5th identical call: refuses execution for the read-only file tools
-  `read`, `grep`, and `glob`, terminating the loop. Other tools stay
-  warn-only.
+tool calls — counting only calls that return results identical to the
+previous call, so a call returning new information (e.g. a file that was
+modified) never counts toward a block — and responds:
+
+- After the 3rd confirmed-identical result: appends a corrective notice to
+  the tool output telling the model to stop repeating and change approach.
+  Applies to all tools.
+- After the 5th confirmed-identical result: refuses the next identical call
+  for the read-only file tools `read`, `grep`, and `glob`, terminating the
+  loop. Other tools stay warn-only.
+
+The count is confirmed in `tool.execute.after`, so overlapping parallel
+calls cannot inflate it before their results are known.
 
 Exempt from the entire guard: the task-control and wait tools (`task`,
 `task_status`, `task_result`, `task_cancel`, `task_message`, `task_revive`,

+ 53 - 44
src/hooks/tool-loop-guard/hook.ts

@@ -9,14 +9,23 @@ import { log } from '../../utils/logger';
  * calls forever).
  *
  * Behavior:
- * - N identical consecutive calls (LOOP_GUARD_WARN_AT): append corrective
- *   text to the tool output telling the model to stop and change approach.
- * - For read-only file tools (READONLY_BLOCK_TOOLS), M identical consecutive
- *   calls (LOOP_GUARD_BLOCK_AT): refuse the call in tool.execute.before by
- *   throwing, so the loop terminates instead of running forever.
+ * - N consecutive calls with identical arguments AND identical results
+ *   (LOOP_GUARD_WARN_AT): append corrective text to the tool output telling
+ *   the model to stop and change approach.
+ * - For read-only file tools (READONLY_BLOCK_TOOLS), M consecutive calls
+ *   with identical arguments AND identical results (LOOP_GUARD_BLOCK_AT):
+ *   refuse the next identical call in tool.execute.before by throwing, so
+ *   the loop terminates instead of running forever.
+ * - The run counter only advances in tool.execute.after, when an identical-
+ *   args call produced an output byte-identical to the prior call. A call
+ *   that returns NEW information resets the run, so it can never accumulate
+ *   toward a block (a legitimate re-read after the file changed).
+ * - tool.execute.before never increments the counter, so overlapping
+ *   parallel calls cannot inflate the count before their results are known.
+ *   A refusal only happens after the run is already confirmed identical.
  *
  * Scope is deliberately narrow to avoid breaking legitimate repeated calls:
- * - All tools warn at N identical consecutive calls.
+ * - All tools warn at N confirmed-identical consecutive calls.
  * - Only the read-only file-analysis tools hard-block: polling tools
  *   (task_*, wait_for_*) legitimately re-issue identical calls waiting on a
  *   long-running background task and must never be refused.
@@ -92,11 +101,11 @@ function stableStringify(value: unknown): string {
 }
 
 interface SessionState {
-  /** Fingerprint of the most recent eligible call in this session. */
+  /** Fingerprint of the most recent completed eligible call (args). */
   last: string;
-  /** How many consecutive identical (args) calls have been observed. */
-  count: number;
-  /** Fingerprint of the most recent eligible call's output. */
+  /** Consecutive completed calls with identical args AND identical output. */
+  runs: number;
+  /** Fingerprint of the most recent completed call's output. */
   lastOutput: string;
 }
 
@@ -138,27 +147,24 @@ export function createToolLoopGuardHook(): ToolLoopGuardHook {
       if (LOOP_GUARD_EXEMPT[tool]) return;
 
       const key = fingerprint(tool, output.args);
-
       const existing = sessions.get(sessionID);
-      const sameArgs = existing !== undefined && existing.last === key;
-      const count = sameArgs ? existing.count + 1 : 1;
-      sessions.set(sessionID, {
-        last: key,
-        count,
-        // New args start a fresh run; identical args carry the prior output
-        // forward so the after-hook can detect when the result changed.
-        lastOutput: sameArgs ? existing.lastOutput : '',
-      });
-      keepSessionsBounded();
 
-      if (count >= LOOP_GUARD_BLOCK_AT && LOOP_GUARD_BLOCK_TOOLS[tool]) {
+      // Refuse only on a CONFIRMED identical run: the previous BLOCK_AT
+      // calls all had identical args AND identical results. The current
+      // call's result is not yet known, but the run is already degenerate.
+      if (
+        existing &&
+        existing.last === key &&
+        existing.runs >= LOOP_GUARD_BLOCK_AT &&
+        LOOP_GUARD_BLOCK_TOOLS[tool]
+      ) {
         log('[tool-loop-guard] blocked repeated tool call', {
           sessionID,
           tool,
-          count,
+          runs: existing.runs,
         });
         throw new Error(
-          `Refusing to execute "${tool}": this exact call (same tool, same arguments) has been issued ${count} times in a row with identical results and constitutes an infinite loop. Stop repeating it. Reassess your goal, make a different call, or produce your final answer.`,
+          `Refusing to execute "${tool}": this exact call (same tool, same arguments) has returned identical results ${existing.runs} times in a row and constitutes an infinite loop. Stop repeating it. Reassess your goal, make a different call, or produce your final answer.`,
         );
       }
 
@@ -173,36 +179,39 @@ export function createToolLoopGuardHook(): ToolLoopGuardHook {
       if (!sessionID) return;
       const tool = input.tool.toLowerCase();
       if (LOOP_GUARD_EXEMPT[tool]) return;
-      const state = sessions.get(sessionID);
-      if (!state) return;
 
       const key = input.callID ? callKeys.get(input.callID) : undefined;
       if (input.callID) callKeys.delete(input.callID);
+      const outputHash = fingerprint(tool, output.output);
 
-      if (key === state.last) {
-        // Identical args. If the result differs from the prior call's result,
-        // this is progress, not a loop — reset the run so it cannot block.
-        const outputHash = fingerprint(tool, output.output);
-        if (state.lastOutput !== '' && outputHash !== state.lastOutput) {
-          state.count = 1;
-        }
-        state.lastOutput = outputHash;
-      }
-
-      if (
-        key !== undefined &&
-        (key !== state.last || state.count < LOOP_GUARD_WARN_AT)
-      ) {
-        return;
+      const existing = sessions.get(sessionID);
+      let state: SessionState;
+      if (existing && key !== undefined && key === existing.last) {
+        // Identical args. Advance the run only when the result is also
+        // identical; a changed result is progress and restarts the run.
+        state = {
+          last: key,
+          runs: outputHash === existing.lastOutput ? existing.runs + 1 : 1,
+          lastOutput: outputHash,
+        };
+      } else {
+        // Different args or untracked call: start a fresh run.
+        state = {
+          last: key ?? `${tool}:<untracked>`,
+          runs: 1,
+          lastOutput: outputHash,
+        };
       }
-      if (key === undefined && state.count < LOOP_GUARD_WARN_AT) return;
+      sessions.set(sessionID, state);
+      keepSessionsBounded();
 
+      if (state.runs < LOOP_GUARD_WARN_AT) return;
       if (typeof output.output !== 'string') return;
       if (output.output.includes(LOOP_GUARD_MARKER)) return;
       log('[tool-loop-guard] warned repeated tool call', {
         sessionID,
-        tool: input.tool.toLowerCase(),
-        count: state.count,
+        tool,
+        runs: state.runs,
       });
       output.output += `\n${LOOP_GUARD_WARNING}`;
     },

+ 53 - 23
src/hooks/tool-loop-guard/index.test.ts

@@ -62,28 +62,28 @@ describe('tool-loop-guard', () => {
     expect(o3.output).toContain(LOOP_GUARD_WARNING);
   });
 
-  test('fifth identical call is refused in tool.execute.before', async () => {
-    for (let i = 1; i <= 4; i++) {
+  test('sixth identical call with stable identical results is refused', async () => {
+    for (let i = 1; i <= 5; i++) {
       await runIdenticalCall(`c${i}`, { filePath: 'a.ts' });
     }
     await expect(
-      hook['tool.execute.before'](beforeInput({ callID: 'c5' }), {
+      hook['tool.execute.before'](beforeInput({ callID: 'c6' }), {
         args: { filePath: 'a.ts' },
       }),
     ).rejects.toThrow('infinite loop');
   });
 
   test('blocked fingerprint stays blocked', async () => {
-    for (let i = 1; i <= 4; i++) {
+    for (let i = 1; i <= 5; i++) {
       await runIdenticalCall(`c${i}`, { filePath: 'a.ts' });
     }
     await expect(
-      hook['tool.execute.before'](beforeInput({ callID: 'c5' }), {
+      hook['tool.execute.before'](beforeInput({ callID: 'c6' }), {
         args: { filePath: 'a.ts' },
       }),
     ).rejects.toThrow();
     await expect(
-      hook['tool.execute.before'](beforeInput({ callID: 'c6' }), {
+      hook['tool.execute.before'](beforeInput({ callID: 'c7' }), {
         args: { filePath: 'a.ts' },
       }),
     ).rejects.toThrow();
@@ -115,19 +115,21 @@ describe('tool-loop-guard', () => {
   });
 
   test('non-readonly tools warn but are never hard-blocked', async () => {
-    // bash repeats warn at 3 but must not throw at 5+
+    // bash is not in BLOCK_TOOLS: confirmed identical runs warn at 3 but
+    // even a long identical run never throws.
     for (let i = 1; i <= 6; i++) {
       await hook['tool.execute.before'](
         beforeInput({ tool: 'bash', callID: `b${i}` }),
         { args: { command: 'uname' } },
       );
+      const output = { output: 'Linux', metadata: {} };
+      await hook['tool.execute.after'](
+        afterInput({ tool: 'bash', callID: `b${i}` }),
+        output,
+      );
+      // never throws (bash is not in BLOCK_TOOLS)
+      expect(output.output).toContain(i >= 3 ? LOOP_GUARD_WARNING : 'Linux');
     }
-    const output = { output: 'Linux', metadata: {} };
-    await hook['tool.execute.after'](
-      afterInput({ tool: 'bash', callID: 'b6' }),
-      output,
-    );
-    expect(output.output).toContain(LOOP_GUARD_WARNING);
   });
 
   test('identical args returning changing results never block', async () => {
@@ -150,21 +152,49 @@ describe('tool-loop-guard', () => {
         beforeInput({ tool: 'bash', callID: `e${i}` }),
         { args: { command: 'uname' } },
       );
+      await hook['tool.execute.after'](
+        afterInput({ tool: 'bash', callID: `e${i}` }),
+        { output: 'Linux', metadata: {} },
+      );
     }
-    const output = { output: 'Linux', metadata: {} };
-    await hook['tool.execute.after'](
-      afterInput({ tool: 'bash', callID: 'e6' }),
-      output,
-    );
-    expect(output.output).toContain(LOOP_GUARD_WARNING);
+    // never throws (bash is not in BLOCK_TOOLS)
   });
 
-  test('identical args with stable identical output still block at 5', async () => {
-    for (let i = 1; i <= 4; i++) {
-      await runIdenticalCall(`f${i}`, { filePath: 'a.ts' }); // output identical each time
+  test('overlapping same-args calls with changing results never block', async () => {
+    // All befores fire before any after — simulates parallel execution
+    // where the counter previously inflated past the threshold.
+    for (let i = 1; i <= 6; i++) {
+      await hook['tool.execute.before'](beforeInput({ callID: `o${i}` }), {
+        args: { filePath: 'a.ts' },
+      });
+    }
+    // then results arrive, each different (file was being modified)
+    for (let i = 1; i <= 6; i++) {
+      const output = { output: `revision ${i}`, metadata: {} };
+      await hook['tool.execute.after'](afterInput({ callID: `o${i}` }), output);
+      expect(output.output).toBe(`revision ${i}`); // never warned
+    }
+    // next same-args call is still allowed
+    await hook['tool.execute.before'](beforeInput({ callID: 'o7' }), {
+      args: { filePath: 'a.ts' },
+    });
+  });
+
+  test('overlapping same-args calls with identical results block the next call', async () => {
+    for (let i = 1; i <= 6; i++) {
+      await hook['tool.execute.before'](beforeInput({ callID: `p${i}` }), {
+        args: { filePath: 'a.ts' },
+      });
+    }
+    for (let i = 1; i <= 5; i++) {
+      await hook['tool.execute.after'](afterInput({ callID: `p${i}` }), {
+        output: 'same',
+        metadata: {},
+      });
     }
+    // 5 confirmed identical results: the next same-args call is refused
     await expect(
-      hook['tool.execute.before'](beforeInput({ callID: 'f5' }), {
+      hook['tool.execute.before'](beforeInput({ callID: 'p7' }), {
         args: { filePath: 'a.ts' },
       }),
     ).rejects.toThrow('infinite loop');