From 21b2b6c7954db99263322d5491bda3a3cb4cb88d Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 09:54:03 -0600 Subject: [PATCH] fix: recursive key sorting in tool-call loop guard hash function (#1962) * Initial plan * fix: use recursive-sort replacer in hashToolCall to preserve nested properties The array replacer in JSON.stringify acted as a property-name whitelist at every nesting level, stripping all nested object properties and causing structurally different tool calls to produce identical hashes. This led to false-positive loop detection for tools with nested/array arguments like ask_user_questions, plan_clarify, browser_batch, etc. Replace with a function replacer that recursively sorts object keys while preserving array order and primitive values. Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com> Agent-Logs-Url: https://github.com/gsd-build/gsd-2/sessions/c10384bc-a2f9-46b8-8380-43ea451ed39d * fix: add missing codeFilesChanged to mergeMilestoneToMain mock in journal-integration test Pre-existing typecheck failure: the mock was missing the codeFilesChanged property added to the mergeMilestoneToMain return type. Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com> Agent-Logs-Url: https://github.com/gsd-build/gsd-2/sessions/debb019f-2fc8-4c76-b809-ecfe48993eff --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: glittercowboy <186001655+glittercowboy@users.noreply.github.com> --- .../gsd/bootstrap/tool-call-loop-guard.ts | 11 ++++- .../gsd/tests/tool-call-loop-guard.test.ts | 45 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/resources/extensions/gsd/bootstrap/tool-call-loop-guard.ts b/src/resources/extensions/gsd/bootstrap/tool-call-loop-guard.ts index 84bc009e3..695c7e746 100644 --- a/src/resources/extensions/gsd/bootstrap/tool-call-loop-guard.ts +++ b/src/resources/extensions/gsd/bootstrap/tool-call-loop-guard.ts @@ -24,8 +24,15 @@ let enabled = true; function hashToolCall(toolName: string, args: Record): string { const h = createHash("sha256"); h.update(toolName); - // Sort keys for deterministic hashing regardless of object key order - h.update(JSON.stringify(args, Object.keys(args).sort())); + // Sort keys recursively for deterministic hashing regardless of object key order + h.update(JSON.stringify(args, (_key, value) => + value && typeof value === "object" && !Array.isArray(value) + ? Object.keys(value).sort().reduce>((o, k) => { + o[k] = value[k]; + return o; + }, {}) + : value + )); return h.digest("hex").slice(0, 16); } diff --git a/src/resources/extensions/gsd/tests/tool-call-loop-guard.test.ts b/src/resources/extensions/gsd/tests/tool-call-loop-guard.test.ts index af5e9001e..fbe3e0670 100644 --- a/src/resources/extensions/gsd/tests/tool-call-loop-guard.test.ts +++ b/src/resources/extensions/gsd/tests/tool-call-loop-guard.test.ts @@ -118,6 +118,51 @@ console.log('\n── Loop guard: arg order is normalized ──'); assertEq(getToolCallLoopCount(), 2, 'Should detect as same call regardless of key order'); } +// ═══════════════════════════════════════════════════════════════════════════ +// Nested/array arguments produce distinct hashes +// ═══════════════════════════════════════════════════════════════════════════ + +console.log('\n── Loop guard: nested args are not stripped ──'); + +{ + resetToolCallLoopGuard(); + + // Simulate ask_user_questions-style calls with different nested content + for (let i = 1; i <= 5; i++) { + const result = checkToolCallLoop('ask_user_questions', { + questions: [{ id: `q${i}`, question: `Question ${i}?` }], + }); + assertTrue(result.block === false, `Nested call ${i} with unique content should be allowed`); + assertEq(getToolCallLoopCount(), 1, `Each unique nested call should reset count to 1`); + } + + // Truly identical nested calls should still be detected + resetToolCallLoopGuard(); + for (let i = 1; i <= 4; i++) { + checkToolCallLoop('ask_user_questions', { + questions: [{ id: 'same', question: 'Same?' }], + }); + } + const blocked = checkToolCallLoop('ask_user_questions', { + questions: [{ id: 'same', question: 'Same?' }], + }); + assertTrue(blocked.block === true, 'Identical nested calls should still be blocked'); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Nested object key order is normalized +// ═══════════════════════════════════════════════════════════════════════════ + +console.log('\n── Loop guard: nested key order is normalized ──'); + +{ + resetToolCallLoopGuard(); + + checkToolCallLoop('tool', { outer: { b: 2, a: 1 } }); + const result = checkToolCallLoop('tool', { outer: { a: 1, b: 2 } }); + assertEq(getToolCallLoopCount(), 2, 'Same nested args in different key order should match'); +} + // ═══════════════════════════════════════════════════════════════════════════ report();