fix(dedup): hash full question payload, not just IDs
The questionSignature() function only hashed sorted question IDs, meaning calls with the same IDs but different text/options would return stale cached answers. Now hashes the full canonicalized payload (id, header, question, options, allowMultiple). Adds 4 regression tests for signature correctness.
This commit is contained in:
parent
f0759086e4
commit
75a5be9278
2 changed files with 64 additions and 4 deletions
|
|
@ -75,7 +75,9 @@ const AskUserQuestionsParams = Type.Object({
|
|||
// ─── Per-turn deduplication ──────────────────────────────────────────────────
|
||||
// Prevents duplicate question dispatches (especially to remote channels like
|
||||
// Discord) when the LLM calls ask_user_questions multiple times with the same
|
||||
// questions in a single turn. Keyed by sorted question IDs.
|
||||
// questions in a single turn. Keyed by full canonicalized payload (id, header,
|
||||
// question, options, allowMultiple) — not just IDs — so that calls with the
|
||||
// same IDs but different text/options are treated as distinct.
|
||||
|
||||
import { createHash } from "node:crypto";
|
||||
|
||||
|
|
@ -86,9 +88,18 @@ interface CachedResult {
|
|||
|
||||
const turnCache = new Map<string, CachedResult>();
|
||||
|
||||
function questionSignature(questions: Array<{ id: string }>): string {
|
||||
const ids = questions.map((q) => q.id).sort().join("|");
|
||||
return createHash("sha256").update(ids).digest("hex").slice(0, 16);
|
||||
/** @internal Exported for testing only. */
|
||||
export function questionSignature(questions: Question[]): string {
|
||||
const canonical = questions
|
||||
.map((q) => ({
|
||||
id: q.id,
|
||||
header: q.header,
|
||||
question: q.question,
|
||||
options: (q.options || []).map((o) => ({ label: o.label, description: o.description })),
|
||||
allowMultiple: !!q.allowMultiple,
|
||||
}))
|
||||
.sort((a, b) => a.id.localeCompare(b.id));
|
||||
return createHash("sha256").update(JSON.stringify(canonical)).digest("hex").slice(0, 16);
|
||||
}
|
||||
|
||||
/** Reset the dedup cache. Called on session boundaries. */
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import {
|
|||
} from "../bootstrap/tool-call-loop-guard.ts";
|
||||
import {
|
||||
resetAskUserQuestionsCache,
|
||||
questionSignature,
|
||||
} from "../../ask-user-questions.ts";
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
|
@ -68,4 +69,52 @@ describe("ask_user_questions dedup", () => {
|
|||
resetAskUserQuestionsCache();
|
||||
// No error means the cache module is properly exported and functional
|
||||
});
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// questionSignature: full-payload hashing prevents stale cache hits
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
test("same IDs with different question text produce different signatures", () => {
|
||||
const q1 = [{ id: "scope", header: "Scope", question: "Which apps to cover?",
|
||||
options: [{ label: "All", description: "Everything" }] }];
|
||||
const q2 = [{ id: "scope", header: "Scope", question: "Which services to test?",
|
||||
options: [{ label: "All", description: "Everything" }] }];
|
||||
|
||||
assert.notEqual(questionSignature(q1), questionSignature(q2),
|
||||
"Different question text with same ID must produce different signatures");
|
||||
});
|
||||
|
||||
test("same IDs with different options produce different signatures", () => {
|
||||
const q1 = [{ id: "scope", header: "Scope", question: "Pick one",
|
||||
options: [{ label: "A", description: "Option A" }] }];
|
||||
const q2 = [{ id: "scope", header: "Scope", question: "Pick one",
|
||||
options: [{ label: "B", description: "Option B" }] }];
|
||||
|
||||
assert.notEqual(questionSignature(q1), questionSignature(q2),
|
||||
"Different options with same ID must produce different signatures");
|
||||
});
|
||||
|
||||
test("identical payloads in different order produce same signature", () => {
|
||||
const q1 = [
|
||||
{ id: "b", header: "B", question: "Q2", options: [{ label: "X", description: "x" }] },
|
||||
{ id: "a", header: "A", question: "Q1", options: [{ label: "Y", description: "y" }] },
|
||||
];
|
||||
const q2 = [
|
||||
{ id: "a", header: "A", question: "Q1", options: [{ label: "Y", description: "y" }] },
|
||||
{ id: "b", header: "B", question: "Q2", options: [{ label: "X", description: "x" }] },
|
||||
];
|
||||
|
||||
assert.equal(questionSignature(q1), questionSignature(q2),
|
||||
"Same questions in different order must produce the same signature");
|
||||
});
|
||||
|
||||
test("allowMultiple difference produces different signature", () => {
|
||||
const q1 = [{ id: "scope", header: "Scope", question: "Pick",
|
||||
options: [{ label: "A", description: "a" }], allowMultiple: false }];
|
||||
const q2 = [{ id: "scope", header: "Scope", question: "Pick",
|
||||
options: [{ label: "A", description: "a" }], allowMultiple: true }];
|
||||
|
||||
assert.notEqual(questionSignature(q1), questionSignature(q2),
|
||||
"allowMultiple difference must produce different signatures");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue