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>
This commit is contained in:
parent
17a2f55edb
commit
21b2b6c795
2 changed files with 54 additions and 2 deletions
|
|
@ -24,8 +24,15 @@ let enabled = true;
|
|||
function hashToolCall(toolName: string, args: Record<string, unknown>): 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<Record<string, unknown>>((o, k) => {
|
||||
o[k] = value[k];
|
||||
return o;
|
||||
}, {})
|
||||
: value
|
||||
));
|
||||
return h.digest("hex").slice(0, 16);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue