diff --git a/AGENTS.md b/AGENTS.md index 761b664..1a1cf3d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -304,6 +304,9 @@ Read the module header for the incident and for what is not covered. identity for untouched values so this is cheap to detect. A handler that always returned a copy would rewrite every tool result and every request context, for nothing. +- **Hits are counted per field, then the highest count per name is reported.** + pi's bash returns its output twice (content text and `structuredContent.output`), + so a sum would report one `echo` as "×2". - **The `context` pass walks the whole conversation on every request.** Measured: 36 ms on a 53 MB session, under 1 ms on a typical one. - **End-to-end check** (2026-10-12, real pi on erdc/deepseek-v4.1-flash; the secret diff --git a/extensions/secret-guard.ts b/extensions/secret-guard.ts index 22b0ea9..11de21b 100644 --- a/extensions/secret-guard.ts +++ b/extensions/secret-guard.ts @@ -126,6 +126,13 @@ function addHits(into: Hits, from: Hits) { for (const [k, n] of from) into.set(k, (into.get(k) ?? 0) + n); } +// One tool result can carry the same output in several fields: pi's bash returns +// it as content text AND structuredContent.output. Count a name by the field that +// holds it most often, not by the sum, so a single echo is reported once. +function maxHits(into: Hits, from: Hits) { + for (const [k, n] of from) into.set(k, Math.max(into.get(k) ?? 0, n)); +} + /** Replace every known value. Text with no secret comes back as the same string. */ export function redactText(text: string, known: Known): { text: string; hits: Hits } { const hits: Hits = new Map(); @@ -185,6 +192,7 @@ interface ToolResultLike { export function toolResultChange(event: ToolResultLike, known: Known) { if (known.length === 0) return undefined; const hits: Hits = new Map(); + const contentHits: Hits = new Map(); const result: Record = {}; let contentChanged = false; @@ -193,14 +201,15 @@ export function toolResultChange(event: ToolResultLike, known: Known) { const r = redactText(part.text, known); if (r.hits.size === 0) return part; contentChanged = true; - addHits(hits, r.hits); + addHits(contentHits, r.hits); return { ...part, text: r.text }; }); if (contentChanged) result.content = content; + maxHits(hits, contentHits); if (event.structuredContent !== undefined) { const r = redactDeep(event.structuredContent, known); - addHits(hits, r.hits); + maxHits(hits, r.hits); // Replacing content without returning structuredContent DROPS it (types.d.ts), // so return it whenever content changed, redacted or not. if (r.value !== event.structuredContent || contentChanged) result.structuredContent = r.value; @@ -208,7 +217,7 @@ export function toolResultChange(event: ToolResultLike, known: Known) { if (event.details !== undefined && event.details !== null) { const r = redactDeep(event.details, known); if (r.value !== event.details) { - addHits(hits, r.hits); + maxHits(hits, r.hits); result.details = r.value; } } diff --git a/test/secret-guard.test.mjs b/test/secret-guard.test.mjs index bdde7d4..9257d06 100644 --- a/test/secret-guard.test.mjs +++ b/test/secret-guard.test.mjs @@ -191,6 +191,15 @@ test("toolResultChange: secrets in details or structuredContent alone are redact assert.ok(!("content" in ch.result), "unchanged content is not returned"); }); +test("toolResultChange: the same output in content AND structuredContent (bash) counts once", () => { + // pi's bash returns its output twice: content text and structuredContent.output (measured 2026-10-12). + const known = envSecrets(ENV); + const out = `val=${V.a}`; + const ch = toolResultChange(bashEvent(out, { structuredContent: { output: out, exit_code: 0 } }), known); + assert.equal(ch.result.structuredContent.output, "val="); + assert.equal(ch.hits.get("MEMPALACE_REMOTE_TOKEN"), 1); +}); + test("toolResultChange: no secret anywhere -> undefined (handler returns nothing)", () => { assert.equal(toolResultChange(bashEvent("total 12\ndrwx------ 2 developer"), envSecrets(ENV)), undefined); assert.equal(toolResultChange(bashEvent(`x ${V.a}`), []), undefined, "no known secrets -> nothing to do");