diff --git a/CHANGELOG.md b/CHANGELOG.md index dc6c8a5..eac7bfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [Unreleased] + +### Added + +- Strip residual `` and `` fragments and bare `m####` refs that some models (notably `MiniMax-M3`) leak into assistant output. Notify the user when a turn produces no tool call but still contains residual metadata, so silent stops become visible. + ## [0.5.0] - 2026-08-01 ### Added diff --git a/docs/superpowers/plans/2026-08-16-strip-residual-metadata.md b/docs/superpowers/plans/2026-08-16-strip-residual-metadata.md index 9df767f..c7da012 100644 --- a/docs/superpowers/plans/2026-08-16-strip-residual-metadata.md +++ b/docs/superpowers/plans/2026-08-16-strip-residual-metadata.md @@ -4,7 +4,7 @@ **Goal:** Strip `MiniMax-M3`'s leaked `` residual fragments and bare `m####` refs from assistant output, and notify the user when the sanitizer runs but leaves a residual shape. -**Architecture:** Extend the existing `stripHallucinationsFromString` pipeline in `src/messages/strip.ts` with two new regex constants (inline residual, end-of-line residual) and a state-aware `stripKnownRefsFromString` helper. Reuse the existing `state.messageIds.byRawId` as the source of truth for which `m####` refs are eligible to strip. Wire the known-refs set into the `message_end` handler in `src/index.ts`, and add a heuristic `looksLikeUnproductiveTurn` check that notifies the user when stripping is a no-op but residual metadata is still present. +**Architecture:** Extend the existing `stripHallucinationsFromString` pipeline in `src/messages/strip.ts` with one new regex constant (inline residual) and a state-aware `stripKnownRefsFromString` helper. Reuse the existing `state.messageIds.byRawId` as the source of truth for which `m####` refs are eligible to strip. Wire the known-refs set into the `message_end` handler in `src/index.ts`, and add a heuristic `looksLikeUnproductiveTurn` check that notifies the user when stripping is a no-op but residual metadata is still present. (Earlier versions of this plan also added an end-of-line residual regex; that regex was dropped after review confirmed it cannot distinguish truncated residuals from prose that merely mentions the namespace phrase — see `docs/07-addendum-residual-regex.md` "What this regex does NOT fix" §3.) **Tech Stack:** TypeScript, Vitest, regex (no new dependencies). @@ -16,7 +16,7 @@ - TypeScript strict mode. No `any` in new code. - Existing 9 cases in `tests/strip.test.ts` must continue to pass after every change. -- The sanitizer must never match the namespace phrase `dcp-message-id` / `dcp-system-reminder` when followed by a word character and a `>` somewhere further on the line — one documented false positive exists (`dcp-message-id foo>bar` → `bar`), see spec "Component 1" rationale. +- The sanitizer must never match the namespace phrase `dcp-message-id` / `dcp-system-reminder` in prose that merely mentions it (e.g. `dcp-message-id is generally safe` must be preserved). One documented false positive exists (`dcp-message-id foo>bar` → `bar`), see spec "Component 1" rationale. The end-of-line residual case (e.g. a line containing just `-dcp-message-id` with no closing `>`) is intentionally NOT addressed by a regex; it surfaces via the `looksLikeUnproductiveTurn` warning in Task 3. - `state.messageIds.byRawId` is owned by `src/state/state.ts:resetSessionState`. Do not introduce a new state field. - No new dependencies. - Notifications use `ctx.ui.notify(message, level)` where level is `"info"` (sanitizer changed text) or `"warning"` (sanitizer unchanged but heuristic fired). Both calls are guarded by `ctx.hasUI`. @@ -25,10 +25,10 @@ | file | responsibility | | -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| `src/messages/strip.ts` | Strip pipeline. Adds 2 regex constants, extends `stripHallucinationsFromString` signature, adds `stripKnownRefsFromString` helper. | +| `src/messages/strip.ts` | Strip pipeline. Adds 1 regex constant, extends `stripHallucinationsFromString` signature, adds `stripKnownRefsFromString` helper. | | `src/index.ts` | `message_end` handler. Wires known-refs snapshot, adds `collectText` and `looksLikeUnproductiveTurn` helpers, sends info/warning notifies. | -| `tests/strip.test.ts` | Appends regression cases for the new residual regexes and the known-refs helper. Existing cases unchanged. | -| `tests/message-end-sanitize-failure.test.ts` | New file. Mocks `createMockPi` / `createMockContext` (pattern from `tests/index.test.ts`) and exercises the three notify branches. | +| `tests/strip.test.ts` | Appends regression cases for the new residual regex and the known-refs helper. Existing cases unchanged. | +| `tests/message-end-sanitize-failure.test.ts` | New file. Uses the existing `createMockApi` pattern from `tests/index.test.ts` to exercise the three notify branches. | --- @@ -72,7 +72,11 @@ it("strips inline residual on its own line", () => { }); it("strips inline system-reminder residual", () => { - expect(stripHallucinationsFromString("-dcp-system-reminder>\n")).toBe(""); + // The inline regex matches `-dcp-system-reminder>` but stops at the + // newline (the body class `[^<>\n]*` excludes newlines). The trailing + // `\n` remains. This is fine — the message_end handler treats whitespace + // as harmless and downstream code joins text parts with `\n` anyway. + expect(stripHallucinationsFromString("-dcp-system-reminder>\n")).toBe("\n"); }); it("does not match prose that mentions the namespace without a >", () => { @@ -147,101 +151,7 @@ git commit -m "feat(strip): add inline residual regex for prefix-less dcp-* frag --- -### Task 2: Add the end-of-line residual regex and its tests - -**Files:** - -- Modify: `src/messages/strip.ts` (add `DCP_RESIDUAL_EOL` constant; add it as the 6th step) -- Modify: `tests/strip.test.ts` (append tests) - -**Interfaces:** - -- Consumes: text where stripping so far left a residual on its own line, e.g. after `DCP_PARTIAL_TAG` consumed a partial opener at end of line. -- Produces: text with line-end residuals removed. The `(^|\s)` capture is consumed, so the leading whitespace on the line is also removed. - -- [ ] **Step 1: Append the failing tests** - -Append to `tests/strip.test.ts`, inside `describe("stripHallucinationsFromString")`, after the inline-residual tests: - -```ts -it("strips end-of-line residual after a partial opener was consumed", () => { - expect( - stripHallucinationsFromString( - 'line1\n { - expect(stripHallucinationsFromString("hello\n-dcp-message-id>\nworld")).toBe( - "hello\nworld", - ); - // (duplicate of inline test for the EOL case; both patterns cover it.) -}); - -it("does not match prose that mentions the namespace without > on its own line", () => { - // The EOL pattern requires no `>` on the line. Prose like - // "dcp-message-id is generally safe" has no `>`, so it WILL match the - // EOL pattern. This test documents the consequence: the entire line - // gets consumed. We accept this because (a) the namespace phrase is - // rare on its own line, (b) when it appears on its own line the model - // emission almost certainly leaked it, (c) the user-visible result is - // a shorter document, not data loss. - expect( - stripHallucinationsFromString( - "intro paragraph\ndcp-message-id is generally safe\noutro", - ), - ).toBe("intro paragraph\n\noutro"); -}); -``` - -- [ ] **Step 2: Run the new tests and verify they fail** - -Run: `pnpm vitest run tests/strip.test.ts` -Expected: 3 new tests fail; the existing 17 pass. - -- [ ] **Step 3: Add the EOL residual constant and step** - -Edit `src/messages/strip.ts`. Add below `DCP_RESIDUAL_INLINE`: - -```ts -// 6. End-of-line residual: same shape on its own line, no closing `>`. -// Covers the line-174 mechanism (partial opener truncated mid-attribute, -// newline, residual opener on next line). Anchored on (^|\s) so the -// leading whitespace on the line is consumed with the match. -const DCP_RESIDUAL_EOL = - /(^|\s)-?dcp-(?:message-id|system-reminder)\b[^\n]*$/gim; -``` - -Add the step to the pipeline: - -```ts -export function stripHallucinationsFromString(text: string): string { - return text - .replace(DCP_COMPLETE_PAIR, "") - .replace(DCP_TRUNCATED_PAIR, "") - .replace(DCP_UNPAIRED_TAG, "") - .replace(DCP_PARTIAL_TAG, "") - .replace(DCP_RESIDUAL_INLINE, "") - .replace(DCP_RESIDUAL_EOL, ""); -} -``` - -- [ ] **Step 4: Run all strip tests and verify they pass** - -Run: `pnpm vitest run tests/strip.test.ts` -Expected: all 20 cases pass (9 existing + 8 inline + 3 EOL). - -- [ ] **Step 5: Commit** - -```bash -git add src/messages/strip.ts tests/strip.test.ts -git commit -m "feat(strip): add end-of-line residual regex for dcp-* fragments" -``` - ---- - -### Task 3: Add the state-aware known-refs strip helper +### Task 2: Add the state-aware known-refs strip helper **Files:** @@ -363,13 +273,15 @@ export function stripHallucinationsFromString( - [ ] **Step 4: Run all strip tests and verify they pass** Run: `pnpm vitest run tests/strip.test.ts` -Expected: all 28 cases pass (9 existing + 8 inline + 3 EOL + 8 known-refs). +Expected: all 25 cases pass (9 existing + 8 inline + 8 known-refs). - [ ] **Step 5: Run the full check to catch downstream regressions** Run: `pnpm check` Expected: lint, typecheck, and the full vitest suite pass. The signature change is backward compatible (optional second argument) so no other call sites should break. +Note: the higher-level `stripHallucinations(messages)` function is intentionally NOT extended with a known-refs argument. Pipeline messages (`src/pipeline.ts`) are pre-sanitization and the bare-`m####` leak surfaces only on the assistant's emitted text at `message_end`. Keeping `stripHallucinations` signature-free avoids threading known-refs into the pipeline. + - [ ] **Step 6: Commit** ```bash @@ -379,7 +291,7 @@ git commit -m "feat(strip): add state-aware known-refs strip helper" --- -### Task 4: Wire known-refs and the heuristic notify into message_end +### Task 3: Wire known-refs and the heuristic notify into message_end **Files:** @@ -498,23 +410,71 @@ Expected: passes with no errors. If `AgentMessage` is not imported, add it to th - [ ] **Step 5: Write the integration tests** -Create `tests/message-end-sanitize-failure.test.ts`. The file must import `createMockPi` and `createMockContext` from `./helpers.ts` (same pattern as `tests/index.test.ts`). Read `tests/helpers.ts` first to confirm the exact helper signatures before writing the tests. +Create `tests/message-end-sanitize-failure.test.ts`. Reuse the `createMockApi` helper from `tests/index.test.ts` (same file is fine, copy verbatim — it's a self-contained local helper, ~30 lines). Pattern: build a mock api with `createMockApi()`, call `createExtension(api)`, fetch the handler via `handlers.get("message_end")?.[0]`, and call it with a real-shape event payload + mock context. + +To test the bare-ref branch, the test must inject a known ref into `state.messageIds.byRawId` before firing `message_end`. Since `state` is a closure inside `createExtension`, the cleanest way is to drive a real `context` pass first (which fills `byRawId`) and then fire `message_end`. That mirrors what would happen in production. Alternatively, expose `state` for test purposes via a test-only seam — but adding production surface for tests is not worth it; prefer the realistic flow. ```ts -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import createExtension from "../src/index.ts"; -import { createMockContext, createMockPi } from "./helpers.ts"; -describe("message_end sanitizer failure handling", () => { - it("emits info notify when sanitizer strips residual metadata", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); +const agentDir = vi.hoisted( + () => `/tmp/dcp-message-end-test-${Date.now()}-${Math.random()}`, +); + +vi.mock("@earendil-works/pi-coding-agent", () => ({ + getAgentDir: () => agentDir, +})); + +type Handler = (...args: never[]) => unknown; + +function createMockApi() { + const handlers = new Map(); + const entries: Array<{ customType: string; data: unknown }> = []; + const commands = new Map(); + const tools = new Map(); + const api = { + on(event: string, handler: Handler) { + const list = handlers.get(event) ?? []; + list.push(handler); + handlers.set(event, list); + }, + registerTool(tool: { name: string }) { + tools.set(tool.name, tool); + }, + registerCommand(name: string, command: unknown) { + commands.set(name, command); + }, + appendEntry(customType: string, data: unknown) { + entries.push({ customType, data }); + }, + } as unknown as import("@earendil-works/pi-coding-agent").ExtensionAPI; + return { api, handlers, entries, commands, tools }; +} + +function makeSessionStartCtx() { + return { + sessionManager: { + getSessionDir: () => "/tmp/dcp-test-session", + getSessionId: () => "test-session", + getBranch: () => [] as unknown[], + }, + getContextUsage: () => undefined, + }; +} - await mock.fireEvent( - "message_end", +describe("message_end sanitizer failure handling", () => { + it("emits info notify when sanitizer strips inline residual metadata", async () => { + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0]; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0]; + const result = await messageEnd( { - type: "message_end", message: { role: "assistant", content: [ @@ -523,61 +483,94 @@ describe("message_end sanitizer failure handling", () => { stopReason: "stop", timestamp: Date.now(), }, - } as never, + }, ctx, ); - const infos = ctx.notifications.filter((n) => n.type === "info"); - expect(infos.some((n) => n.message.includes("stripped residual"))).toBe( - true, + expect(notify).toHaveBeenCalledWith( + expect.stringContaining("stripped residual"), + "info", ); + // Handler must return the stripped message so the agent sees the + // sanitized text on its next pass. + expect(result).toHaveProperty("message"); + const strippedContent = ( + result as { message: { content: Array<{ text: string }> } } + ).message.content; + expect(strippedContent[0].text).not.toContain("dcp-message-id"); }); it("emits info notify when sanitizer strips a bare known ref", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0]; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + // Drive a context pass so byRawId is populated with m0001. + const context = handlers.get("context")?.[0]; + await context( + { + messages: [ + { + role: "user", + content: [{ type: "text", text: "hi" }], + timestamp: 1, + }, + ], + }, + { + ...makeSessionStartCtx(), + getContextUsage: () => undefined, + hasUI: false, + }, + ); - await mock.fireEvent( - "message_end", + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0]; + const result = await messageEnd( { - type: "message_end", message: { role: "assistant", content: [ { type: "text", - text: "Sort the selected names alphabetically on enter:\n\n\n\nm0103", + text: "Sort the selected names alphabetically on enter:\n\n\n\nm0001", }, ], stopReason: "stop", timestamp: Date.now(), }, - } as never, + }, ctx, ); - const infos = ctx.notifications.filter((n) => n.type === "info"); - expect(infos.some((n) => n.message.includes("stripped residual"))).toBe( - true, + expect(notify).toHaveBeenCalledWith( + expect.stringContaining("stripped residual"), + "info", ); + const strippedText = ( + result as { message: { content: Array<{ text: string }> } } + ).message.content[0].text; + expect(strippedText).not.toContain("m0001"); }); it("emits warning notify when sanitizer is a no-op but residual pattern remains", async () => { - // Construct a shape the residual regexes can't catch: dcp-message-id - // embedded in a larger identifier. The boundary check in the residual - // regexes (^|[^\w-]) prevents matching inside `xdcp-message-idy`, so - // the strip pipeline returns the text unchanged. The heuristic still - // detects the substring and fires the warning. This is the - // defense-in-depth branch. - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - - await mock.fireEvent( - "message_end", + // Shape the residual regex can't catch: dcp-message-id embedded in a + // larger identifier. The boundary check `(^|[^\w-])` prevents matching + // inside `xdcp-message-idy`, so the strip pipeline returns the text + // unchanged. The heuristic still detects the substring and fires the + // warning. Defense-in-depth branch. + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0]; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0]; + const result = await messageEnd( { - type: "message_end", message: { role: "assistant", content: [ @@ -586,29 +579,33 @@ describe("message_end sanitizer failure handling", () => { stopReason: "stop", timestamp: Date.now(), }, - } as never, + }, ctx, ); - const warnings = ctx.notifications.filter((n) => n.type === "warning"); - expect( - warnings.some((n) => n.message.includes("model output looked malformed")), - ).toBe(true); - const infos = ctx.notifications.filter((n) => n.type === "info"); - expect(infos.some((n) => n.message.includes("stripped residual"))).toBe( - false, + expect(notify).toHaveBeenCalledWith( + expect.stringContaining("model output looked malformed"), + "warning", + ); + expect(notify).not.toHaveBeenCalledWith( + expect.stringContaining("stripped residual"), + "info", ); + // No stripping happened, so no message replacement. + expect(result).toBeUndefined(); }); it("does not notify on a clean message with a tool call", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - - await mock.fireEvent( - "message_end", + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0]; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0]; + const result = await messageEnd( { - type: "message_end", message: { role: "assistant", content: [ @@ -623,11 +620,12 @@ describe("message_end sanitizer failure handling", () => { stopReason: "stop", timestamp: Date.now(), }, - } as never, + }, ctx, ); - expect(ctx.notifications).toHaveLength(0); + expect(notify).not.toHaveBeenCalled(); + expect(result).toBeUndefined(); }); }); ``` @@ -635,12 +633,12 @@ describe("message_end sanitizer failure handling", () => { - [ ] **Step 6: Run the new tests and verify they pass** Run: `pnpm vitest run tests/message-end-sanitize-failure.test.ts` -Expected: 4 tests pass. If the `as never` casts on the event payload cause type errors, run `pnpm exec tsc --noEmit` first to see the exact shape `tests/helpers.ts` expects for `fireEvent`'s event argument, and adjust the cast. The mock's `fireEvent` likely takes `unknown` for the event, so the cast is correct as written. +Expected: 4 tests pass. If a test errors on `getContextUsage` being undefined or on missing `sessionManager` keys, add them to the mock context — the `context` handler is invoked during the bare-ref test to populate `state.messageIds.byRawId`. - [ ] **Step 7: Run the full check suite** Run: `pnpm check` -Expected: biome lint clean, tsc clean, all vitest tests pass (existing 28 strip tests + 4 new message-end tests + everything else). +Expected: biome lint clean, tsc clean, all vitest tests pass (existing 25 strip tests + 4 new message-end tests + everything else). - [ ] **Step 8: Commit** @@ -651,7 +649,7 @@ git commit -m "feat(message-end): notify on sanitizer strip or unproductive turn --- -### Task 5: Final verification and documentation +### Task 4: Final verification and documentation **Files:** @@ -723,18 +721,19 @@ git commit -m "docs: changelog entry for residual metadata stripping" **1. Spec coverage:** -| spec requirement | task | -| ----------------------------------------------------------------- | -------------------------------------------- | -| Component 1 — residual regex additions | Tasks 1, 2 | -| Component 2 — state-aware bare-ref stripping | Task 3 | -| Component 3 — known-refs wiring in `message_end` | Task 4 | -| Component 3 — `collectText` + `looksLikeUnproductiveTurn` helpers | Task 4 | -| Existing 9 strip.test.ts cases continue to pass | Tasks 1, 2, 3 (each runs full strip.test.ts) | -| New tests for residual regexes | Tasks 1, 2 | -| New tests for known-refs stripping | Task 3 | -| New tests for `message_end` notify behavior | Task 4 | -| `pnpm check` clean | Tasks 3, 4, 5 | -| Changelog entry | Task 5 | +| spec requirement | task | +| ----------------------------------------------------------------- | ----------------------------------------- | +| Component 1 — inline residual regex addition | Task 1 | +| Component 2 — state-aware bare-ref stripping | Task 2 | +| Component 3 — known-refs wiring in `message_end` | Task 3 | +| Component 3 — `collectText` + `looksLikeUnproductiveTurn` helpers | Task 3 | +| Existing 9 strip.test.ts cases continue to pass | Tasks 1, 2 (each runs full strip.test.ts) | +| New tests for inline residual regex | Task 1 | +| New tests for known-refs stripping | Task 2 | +| New tests for `message_end` notify behavior | Task 3 | +| `pnpm check` clean | Tasks 2, 3, 4 | +| Changelog entry | Task 4 | +| EOL residual NOT addressed by regex (per `docs/07`) | covered by Task 3 warning notify | No gaps. @@ -742,9 +741,11 @@ No gaps. **3. Type consistency:** -- `stripHallucinationsFromString` signature: `text: string, knownRefs?: ReadonlySet)` — consistent across Tasks 3, 4. Caller in `message_end` builds the set as `new Set(state.messageIds.byRawId.values())`. State field name verified against `src/state/state.ts:50`. -- `stripKnownRefsFromString`: defined in Task 3, only used inside `stripHallucinationsFromString` in the same file. No cross-task name drift. -- `collectText(msg: AgentMessage): string` and `looksLikeUnproductiveTurn(text: string, msg: AgentMessage): boolean` — defined once in Task 4, used once in Task 4's handler. No cross-task drift. -- `mapText(msg, (t) => stripHallucinationsFromString(t, knownRefs))` — `mapText` already imported in `src/index.ts` (line ~14 of the existing handler context). Reused unchanged. +- `stripHallucinationsFromString` signature: `(text: string, knownRefs?: ReadonlySet) => string` — defined once in Task 1 (extended in Task 2), used in Tasks 2 and 3. Caller in `message_end` (Task 3) builds the set as `new Set(state.messageIds.byRawId.values())`. State field name verified against `src/state/state.ts:50`. +- `stripKnownRefsFromString`: defined in Task 2, only used inside `stripHallucinationsFromString` in the same file. No cross-task name drift. +- `collectText(msg: AgentMessage): string` and `looksLikeUnproductiveTurn(text: string, msg: AgentMessage): boolean` — defined once in Task 3, used once in Task 3's handler. No cross-task drift. +- `mapText(msg, (t) => stripHallucinationsFromString(t, knownRefs))` — `mapText` already imported in `src/index.ts`. Reused unchanged. + +`stripHallucinations(messages)` deliberately keeps its single-arg signature: the bare-`m####` leak is an end-of-turn phenomenon, not a pipeline-pruning one. Pipeline messages pass through `stripHallucinations` unchanged (no known-refs available there), and the `message_end` handler re-strips with known-refs as the final sanitization pass. All consistent. diff --git a/package.json b/package.json index ce35940..156a85c 100644 --- a/package.json +++ b/package.json @@ -55,8 +55,8 @@ }, "devDependencies": { "@biomejs/biome": "^2.5.6", - "@earendil-works/pi-agent-core": "^0.83.0", - "@earendil-works/pi-coding-agent": "^0.83.0", + "@earendil-works/pi-agent-core": "^0.84.2", + "@earendil-works/pi-coding-agent": "^0.84.2", "@types/node": "^26.1.1", "tsx": "^4.23.1", "typescript": "^6.0.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9286765..75e35e0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -16,11 +16,11 @@ importers: specifier: ^2.5.6 version: 2.5.6 '@earendil-works/pi-agent-core': - specifier: ^0.83.0 - version: 0.83.0(ws@8.21.0)(zod@4.4.3) + specifier: ^0.84.2 + version: 0.84.2(ws@8.21.0)(zod@4.4.3) '@earendil-works/pi-coding-agent': - specifier: ^0.83.0 - version: 0.83.0(ws@8.21.0)(zod@4.4.3) + specifier: ^0.84.2 + version: 0.84.2(ws@8.21.0)(zod@4.4.3) '@types/node': specifier: ^26.1.1 version: 26.1.2 @@ -203,22 +203,34 @@ packages: cpu: [x64] os: [win32] - '@earendil-works/pi-agent-core@0.83.0': - resolution: {integrity: sha512-RorGp9OH5l3ElpuC5a5ZQ2eWcchZGXflXRzVGkV99y3y6tT+LLNyxoYIdVKvTKWEObwhExeQbTH0fI2tE4iX4g==} + '@earendil-works/pi-agent-core@0.84.2': + resolution: {integrity: sha512-8Pn3wSCxj0cfo5I6jxQYVB/3uuQRmHhAlEclyjqpOuMEdQMIODHizRogv56FLdbU+dTiGnybeHQ2N+sV1/L2YA==} engines: {node: '>=22.19.0'} - '@earendil-works/pi-ai@0.83.0': - resolution: {integrity: sha512-m3IZD4g3er0V8TC9+Vpgw/sjTKqcJlkcIBy/JvsgRubuuik3tAVzyugUg4rVrShIkkOT69mEd34NEqKUIsl6JQ==} + '@earendil-works/pi-ai@0.84.2': + resolution: {integrity: sha512-6MzsrYIYNVlE7SfpbL2yYb67Qo58p/7Q+xWG1RZvoX1P80aRCHSod2/13aFpxkow1lPO2LEh3c495J0Gwmyjig==} engines: {node: '>=22.19.0'} hasBin: true - '@earendil-works/pi-coding-agent@0.83.0': - resolution: {integrity: sha512-uYhF+FsZxogoSX/AxBcUdiY+ZklubwaXyAoEGA2eQwsHcyEAhUYIKh/WLXe/a8+k8eTCmxb+ZN2Zo9mzQtzbWw==} + '@earendil-works/pi-client@0.84.2': + resolution: {integrity: sha512-/RFSPhD/bZbpOp1oJj+UneSUFSgZhWxzcSENUY+8+8xhoBrWXMYI2t77XNx4Yf+c8YK2qTHquForhNcelYpXvg==} + engines: {node: '>=22.19.0'} + + '@earendil-works/pi-coding-agent@0.84.2': + resolution: {integrity: sha512-l4E+B7hgXKWddRo8bC/eSue2aWZjEgJ9xIpf5p0Og+lq8a2TArCwJ0HCoCPCgaBP/tN4zbYH/wOwvx9pJpeLCA==} engines: {node: '>=22.19.0'} hasBin: true - '@earendil-works/pi-tui@0.83.0': - resolution: {integrity: sha512-IoYrb0rORjELmEpNtoCA/U8je3KopMkRAVJRdSzvXRvgb+Huo1gNh8Q5CSZvNOiYtDxJdj2tYZZHZ4B3+IN3hA==} + '@earendil-works/pi-protocol@0.84.2': + resolution: {integrity: sha512-jbBh03fkeckWEroHpcZBr4w5/Ibat8WwdXFlXHivYQImrQNFtLpDeL0t1cku4hmK0q3pceIRQHkw4fwbM4YILQ==} + engines: {node: '>=22.19.0'} + + '@earendil-works/pi-telemetry@0.84.2': + resolution: {integrity: sha512-wg5caea7uIv1BHRBm2Y116RvFG4oSAiP5qk9tA2463PDGIr4K8M1Ceyyg5DOpF/shUUl0gk826yQJAeAcHYB9g==} + engines: {node: '>=22.19.0'} + + '@earendil-works/pi-tui@0.84.2': + resolution: {integrity: sha512-ds2TLihOnM5sLJB3VpXV6y0uR5efVuHf4MN7yDpsty6hA2DUO/EDVzjp/0od0G2JslzVLMjT8T8zavtxVb+qbg==} engines: {node: '>=22.19.0'} '@emnapi/core@1.11.1': @@ -466,14 +478,6 @@ packages: resolution: {integrity: sha512-ABnA53mdfkGZwOFUdZNv2S0CWGO/EIuPj8Vv9xmBFmSYg/qFc7ihO6q5FcQjvoE67kZpWkEc4AhD6B/os04yuA==} engines: {node: '>= 10'} - '@mistralai/mistralai@2.2.6': - resolution: {integrity: sha512-W8pX7zHxjJvMIpw8JMxeJEleapXX0Q9NPszdNzqkM3MIEoIGPObdodujj+WHteXEvGfaP/AMwlNyRfEzSY6dQQ==} - peerDependencies: - '@opentelemetry/api': ^1.9.0 - peerDependenciesMeta: - '@opentelemetry/api': - optional: true - '@napi-rs/wasm-runtime@1.1.6': resolution: {integrity: sha512-ZLv/JdUfkvOy9eCnnBaGfiO+XimbjebAeO+MRQqD/B+FR1tnRN0tpKSJHRbE8sFfS6aqsXZ67TQjfwfsxULVbg==} peerDependencies: @@ -484,10 +488,6 @@ packages: resolution: {integrity: sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==} engines: {node: '>=8.0.0'} - '@opentelemetry/semantic-conventions@1.42.0': - resolution: {integrity: sha512-icc5xCzndZfhuJMy5oqk5AvloWquR7jtae74qzpkKkhGp8BivK+oCcEXgGnjCdTfp8hA44l+w8gE8yYJbocJJw==} - engines: {node: '>=14'} - '@oxc-project/types@0.138.0': resolution: {integrity: sha512-1a7ZKmrRTCoN1XMZ4L0PyyqrMnrNlLyPuOkdSX2MZg7IiIGRUyurNhAm73ptDOraoBcIordsIGKNPKUzy3ZmfA==} @@ -674,9 +674,6 @@ packages: '@types/estree@1.0.9': resolution: {integrity: sha512-GhdPgy1el4/ImP05X05Uw4cw2/M93BCUmnEvWZNStlCzEKME4Fkk+YpoA5OiHNQmoS7Cafb8Xa3Pya8m1Qrzeg==} - '@types/node@26.1.0': - resolution: {integrity: sha512-O0A1G3xPGy4w7AgQdAQYUlQ+BKk2Oovw8eRpofyp5KdBZULnbe+WqaOVNrm705SHphCiG4XHsACrSmPu1f+Kgw==} - '@types/node@26.1.2': resolution: {integrity: sha512-Vu4a5UFA9rIIFJ7rB/Vaafh9lrCQszopTCx6KjFboXTGQbPNasehVR5TEiithSDGyd1DEiUByggTZsg8jukeIg==} @@ -846,6 +843,10 @@ packages: graceful-fs@4.2.11: resolution: {integrity: sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==} + grok-mermaid@0.2.2: + resolution: {integrity: sha512-XcJEP5dDC8liHBh52mlLjU18fNvu1ckFsu0QpIG3+APZ270fsj9wxpiA6cOURmbUEuoMVgjbC2+UYgTdCqqgzA==} + engines: {node: '>=18'} + highlight.js@10.7.3: resolution: {integrity: sha512-tzcUFauisWKNHaRkN4Wjl/ZA07gENAjFl3J/c480dprkGTg5EQstgaNFqBfUqCq54kZRIEcreTsAgF/m2quD7A==} @@ -1003,9 +1004,8 @@ packages: resolution: {integrity: sha512-9miFgM2OFba7hB+pRgvtV84pYTBaoTHohvmIgiRt6dRIzbwEOIaNaP+dIlGs2fNFoB0SeISs0Jz5WFVRid6Xyg==} engines: {node: '>=12.20.0'} - openai@6.26.0: - resolution: {integrity: sha512-zd23dbWTjiJ6sSAX6s0HrCZi41JwTA1bQVs0wLQPZ2/5o2gxOJA5wh7yOAUgwYybfhDXyhwlpeQf7Mlgx8EOCA==} - hasBin: true + openai@6.40.0: + resolution: {integrity: sha512-MWtTjd/gQt4jpbji61NTgFWJLoY/PdRJ6wG9/ZDRMYNMlBKrCrSlkLI+KgHP1vR1qT6LKSAyAqIxno6lcK9JiA==} peerDependencies: ws: ^8.18.0 zod: ^3.25 || ^4.0 @@ -1136,8 +1136,8 @@ packages: undici-types@8.3.0: resolution: {integrity: sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ==} - undici@8.5.0: - resolution: {integrity: sha512-xamtWoB1EshgjpmlXd7GGm2VfdDtw1+rD8uhry8pSNW3If6S8E0m2T2+orSKeZXEn/aPJMviCpDBA65WJt8zhg==} + undici@8.9.0: + resolution: {integrity: sha512-aWZpUj7XoGonMClx4gdDRfgBjqeA+F473aDmROQQbM9n6PRfK/u1q/a0X4wMTgcHfT8H6fpbt98PFuDUwFg2YA==} engines: {node: '>=22.19.0'} vite@8.1.3: @@ -1255,11 +1255,6 @@ packages: engines: {node: '>= 14.6'} hasBin: true - zod-to-json-schema@3.25.2: - resolution: {integrity: sha512-O/PgfnpT1xKSDeQYSCfRI5Gy3hPf91mKVDuYLUHZJMiDFptvP41MSnWofm8dnCm0256ZNfZIM7DSzuSMAFnjHA==} - peerDependencies: - zod: ^3.25.28 || ^4 - zod@4.4.3: resolution: {integrity: sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==} @@ -1522,9 +1517,10 @@ snapshots: '@biomejs/cli-win32-x64@2.5.6': optional: true - '@earendil-works/pi-agent-core@0.83.0(ws@8.21.0)(zod@4.4.3)': + '@earendil-works/pi-agent-core@0.84.2(ws@8.21.0)(zod@4.4.3)': dependencies: - '@earendil-works/pi-ai': 0.83.0(ws@8.21.0)(zod@4.4.3) + '@earendil-works/pi-ai': 0.84.2(ws@8.21.0)(zod@4.4.3) + '@earendil-works/pi-telemetry': 0.84.2 diff: 8.0.4 ignore: 7.0.5 typebox: 1.3.7 @@ -1537,17 +1533,17 @@ snapshots: - ws - zod - '@earendil-works/pi-ai@0.83.0(ws@8.21.0)(zod@4.4.3)': + '@earendil-works/pi-ai@0.84.2(ws@8.21.0)(zod@4.4.3)': dependencies: '@anthropic-ai/sdk': 0.91.1(zod@4.4.3) '@aws-sdk/client-bedrock-runtime': 3.1048.0 + '@earendil-works/pi-telemetry': 0.84.2 '@google/genai': 1.52.0 - '@mistralai/mistralai': 2.2.6(@opentelemetry/api@1.9.0) '@opentelemetry/api': 1.9.0 '@smithy/node-http-handler': 4.7.3 http-proxy-agent: 7.0.2 https-proxy-agent: 7.0.6 - openai: 6.26.0(ws@8.21.0)(zod@4.4.3) + openai: 6.40.0(ws@8.21.0)(zod@4.4.3) partial-json: 0.1.7 typebox: 1.3.7 transitivePeerDependencies: @@ -1558,16 +1554,23 @@ snapshots: - ws - zod - '@earendil-works/pi-coding-agent@0.83.0(ws@8.21.0)(zod@4.4.3)': + '@earendil-works/pi-client@0.84.2': dependencies: - '@earendil-works/pi-agent-core': 0.83.0(ws@8.21.0)(zod@4.4.3) - '@earendil-works/pi-ai': 0.83.0(ws@8.21.0)(zod@4.4.3) - '@earendil-works/pi-tui': 0.83.0 + '@earendil-works/pi-protocol': 0.84.2 + + '@earendil-works/pi-coding-agent@0.84.2(ws@8.21.0)(zod@4.4.3)': + dependencies: + '@earendil-works/pi-agent-core': 0.84.2(ws@8.21.0)(zod@4.4.3) + '@earendil-works/pi-ai': 0.84.2(ws@8.21.0)(zod@4.4.3) + '@earendil-works/pi-client': 0.84.2 + '@earendil-works/pi-protocol': 0.84.2 + '@earendil-works/pi-tui': 0.84.2 '@silvia-odwyer/photon-node': 0.3.4 chalk: 5.6.2 cross-spawn: 7.0.6 diff: 8.0.4 glob: 13.0.6 + grok-mermaid: 0.2.2 highlight.js: 10.7.3 hosted-git-info: 9.0.3 ignore: 7.0.5 @@ -1576,7 +1579,7 @@ snapshots: proper-lockfile: 4.1.2 semver: 7.8.0 typebox: 1.3.7 - undici: 8.5.0 + undici: 8.9.0 yaml: 2.9.0 optionalDependencies: '@mariozechner/clipboard': 0.3.9 @@ -1588,7 +1591,13 @@ snapshots: - ws - zod - '@earendil-works/pi-tui@0.83.0': + '@earendil-works/pi-protocol@0.84.2': + dependencies: + typebox: 1.3.7 + + '@earendil-works/pi-telemetry@0.84.2': {} + + '@earendil-works/pi-tui@0.84.2': dependencies: get-east-asian-width: 1.6.0 marked: 18.0.5 @@ -1744,18 +1753,6 @@ snapshots: '@mariozechner/clipboard-win32-x64-msvc': 0.3.9 optional: true - '@mistralai/mistralai@2.2.6(@opentelemetry/api@1.9.0)': - dependencies: - '@opentelemetry/semantic-conventions': 1.42.0 - ws: 8.21.0 - zod: 4.4.3 - zod-to-json-schema: 3.25.2(zod@4.4.3) - optionalDependencies: - '@opentelemetry/api': 1.9.0 - transitivePeerDependencies: - - bufferutil - - utf-8-validate - '@napi-rs/wasm-runtime@1.1.6(@emnapi/core@1.11.1)(@emnapi/runtime@1.11.1)': dependencies: '@emnapi/core': 1.11.1 @@ -1765,8 +1762,6 @@ snapshots: '@opentelemetry/api@1.9.0': {} - '@opentelemetry/semantic-conventions@1.42.0': {} - '@oxc-project/types@0.138.0': {} '@protobufjs/aspromise@1.1.2': {} @@ -1911,10 +1906,6 @@ snapshots: '@types/estree@1.0.9': {} - '@types/node@26.1.0': - dependencies: - undici-types: 8.3.0 - '@types/node@26.1.2': dependencies: undici-types: 8.3.0 @@ -2100,6 +2091,8 @@ snapshots: graceful-fs@4.2.11: {} + grok-mermaid@0.2.2: {} + highlight.js@10.7.3: {} hosted-git-info@9.0.3: @@ -2225,7 +2218,7 @@ snapshots: obug@2.1.3: {} - openai@6.26.0(ws@8.21.0)(zod@4.4.3): + openai@6.40.0(ws@8.21.0)(zod@4.4.3): optionalDependencies: ws: 8.21.0 zod: 4.4.3 @@ -2273,7 +2266,7 @@ snapshots: '@protobufjs/path': 1.1.2 '@protobufjs/pool': 1.1.0 '@protobufjs/utf8': 1.1.2 - '@types/node': 26.1.0 + '@types/node': 26.1.2 long: 5.3.2 retry@0.12.0: {} @@ -2350,7 +2343,7 @@ snapshots: undici-types@8.3.0: {} - undici@8.5.0: {} + undici@8.9.0: {} vite@8.1.3(@types/node@26.1.2)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.23.1)(yaml@2.9.0): dependencies: @@ -2410,8 +2403,5 @@ snapshots: yaml@2.9.0: {} - zod-to-json-schema@3.25.2(zod@4.4.3): - dependencies: - zod: 4.4.3 - - zod@4.4.3: {} + zod@4.4.3: + optional: true diff --git a/src/index.ts b/src/index.ts index 7ef483d..8390640 100644 --- a/src/index.ts +++ b/src/index.ts @@ -342,14 +342,49 @@ export default function createExtension(pi: ExtensionAPI): void { logger.info("dcp", "session shutdown"); }); - pi.on("message_end", async (event, _ctx) => { + pi.on("message_end", async (event, ctx) => { if (!config.enabled) return; if (event.message.role !== "assistant") return; - const stripped = mapText(event.message, stripHallucinationsFromString); + const knownRefs = new Set(state.messageIds.byRawId.values()); + const stripped = mapText(event.message, (t) => stripHallucinationsFromString(t, knownRefs)); + if (stripped !== event.message) { + if (ctx.hasUI) { + ctx.ui.notify("dcp: stripped residual metadata from model output", "info"); + } return { message: stripped }; } + + // Defense-in-depth: if stripping returned the message unchanged but the + // visible text still carries a dcp-* shape AND the turn produced no + // tool call, the regex set missed a case. Notify so silent stops become + // visible. Per docs/06: notify-only, not provider fail-closed. + const msg = event.message; + if (msg.role === "assistant" && msg.stopReason === "stop" && ctx.hasUI) { + const content = msg.content; + const hasToolCall = + Array.isArray(content) && + content.some( + (p) => + typeof p === "object" && p !== null && (p as { type?: string }).type === "toolCall", + ); + const text = Array.isArray(content) + ? content + .filter( + (p): p is { type: "text"; text: string } => + typeof p === "object" && p !== null && (p as { type?: unknown }).type === "text", + ) + .map((p) => p.text) + .join("\n") + : ""; + if (!hasToolCall && /-?dcp-(message-id|system-reminder)/.test(text)) { + ctx.ui.notify( + "dcp: model output looked malformed (no tool call, residual metadata present). Try re-prompting.", + "warning", + ); + } + } }); pi.on("tool_call", async (event, _ctx) => { diff --git a/src/messages/strip.ts b/src/messages/strip.ts index cbedfed..6fc6df6 100644 --- a/src/messages/strip.ts +++ b/src/messages/strip.ts @@ -10,6 +10,26 @@ const DCP_UNPAIRED_TAG = /<\/?dcp[-\w]*(?:\s[^>]*)?>/gi; // 4. Partial tag at end of line/string: ". Requires `>` so prose that merely mentions +// the namespace is not swallowed. +const DCP_RESIDUAL_INLINE = /(^|[^\w-])-?dcp-(?:message-id|system-reminder)\b[^<>\n]*>/gi; + +/** + * Strip bare m#### message-id refs that were injected this session. + * Anchored on word boundaries so legitimate numeric tokens (model numbers, + * file sizes, hex suffixes) are not affected. + * + * The alternation source is sorted longest-first so a short ref like "m01" + * doesn't match inside a longer injected ref like "m0103". + */ +function stripKnownRefsFromString(text: string, knownRefs: ReadonlySet): string { + if (knownRefs.size === 0) return text; + const alts = [...knownRefs].sort((a, b) => b.length - a.length).join("|"); + const re = new RegExp(`(?\n]*)?$/gim; * partial tags at end of string. Order matters: complete pairs first (they * consume the closing >), then truncated pairs, then lone tags, then partials. */ -export function stripHallucinationsFromString(text: string): string { - return text +export function stripHallucinationsFromString( + text: string, + knownRefs?: ReadonlySet, +): string { + const stripped = text .replace(DCP_COMPLETE_PAIR, "") .replace(DCP_TRUNCATED_PAIR, "") .replace(DCP_UNPAIRED_TAG, "") - .replace(DCP_PARTIAL_TAG, ""); + .replace(DCP_PARTIAL_TAG, "") + .replace(DCP_RESIDUAL_INLINE, ""); + return knownRefs ? stripKnownRefsFromString(stripped, knownRefs) : stripped; } /** diff --git a/tests/message-end-sanitize-failure.test.ts b/tests/message-end-sanitize-failure.test.ts new file mode 100644 index 0000000..e2bd7a1 --- /dev/null +++ b/tests/message-end-sanitize-failure.test.ts @@ -0,0 +1,194 @@ +import { describe, expect, it, vi } from "vitest"; +import createExtension from "../src/index.ts"; + +const agentDir = vi.hoisted(() => `/tmp/dcp-message-end-test-${Date.now()}-${Math.random()}`); + +vi.mock("@earendil-works/pi-coding-agent", () => ({ + getAgentDir: () => agentDir, +})); + +type Handler = (...args: never[]) => unknown; + +function createMockApi() { + const handlers = new Map(); + const entries: Array<{ customType: string; data: unknown }> = []; + const commands = new Map(); + const tools = new Map(); + const api = { + on(event: string, handler: Handler) { + const list = handlers.get(event) ?? []; + list.push(handler); + handlers.set(event, list); + }, + registerTool(tool: { name: string }) { + tools.set(tool.name, tool); + }, + registerCommand(name: string, command: unknown) { + commands.set(name, command); + }, + appendEntry(customType: string, data: unknown) { + entries.push({ customType, data }); + }, + } as unknown as import("@earendil-works/pi-coding-agent").ExtensionAPI; + return { api, handlers, entries, commands, tools }; +} + +function makeSessionStartCtx() { + return { + sessionManager: { + getSessionDir: () => "/tmp/dcp-test-session", + getSessionId: () => "test-session", + getBranch: () => [] as unknown[], + }, + getContextUsage: () => undefined, + }; +} + +describe("message_end sanitizer failure handling", () => { + it("emits info notify when sanitizer strips inline residual metadata", async () => { + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0] as ( + ...args: unknown[] + ) => Promise; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0] as (...args: unknown[]) => Promise; + const result = await messageEnd( + { + message: { + role: "assistant", + content: [{ type: "text", text: "Let me cast:\n\n\n\n-dcp-message-id>" }], + stopReason: "stop", + timestamp: Date.now(), + }, + }, + ctx, + ); + + expect(notify).toHaveBeenCalledWith(expect.stringContaining("stripped residual"), "info"); + // Handler must return the stripped message so the agent sees the + // sanitized text on its next pass. + expect(result).toHaveProperty("message"); + const strippedContent = (result as { message: { content: Array<{ text: string }> } }).message + .content; + expect(strippedContent[0].text).not.toContain("dcp-message-id"); + }); + + it("emits info notify when sanitizer strips a bare known ref", async () => { + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0] as ( + ...args: unknown[] + ) => Promise; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + // Drive a context pass so byRawId is populated with m0001. + const context = handlers.get("context")?.[0] as (...args: unknown[]) => Promise; + await context( + { + messages: [{ role: "user", content: [{ type: "text", text: "hi" }], timestamp: 1 }], + }, + { ...makeSessionStartCtx(), getContextUsage: () => undefined, hasUI: false }, + ); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0] as (...args: unknown[]) => Promise; + const result = await messageEnd( + { + message: { + role: "assistant", + content: [ + { + type: "text", + text: "Sort the selected names alphabetically on enter:\n\n\n\nm0001", + }, + ], + stopReason: "stop", + timestamp: Date.now(), + }, + }, + ctx, + ); + + expect(notify).toHaveBeenCalledWith(expect.stringContaining("stripped residual"), "info"); + const strippedText = (result as { message: { content: Array<{ text: string }> } }).message + .content[0].text; + expect(strippedText).not.toContain("m0001"); + }); + + it("emits warning notify when sanitizer is a no-op but residual pattern remains", async () => { + // Shape the residual regex can't catch: dcp-message-id embedded in a + // larger identifier. The boundary check `(^|[^\w-])` prevents matching + // inside `xdcp-message-idy`, so the strip pipeline returns the text + // unchanged. The heuristic still detects the substring and fires the + // warning. Defense-in-depth branch. + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0] as ( + ...args: unknown[] + ) => Promise; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0] as (...args: unknown[]) => Promise; + const result = await messageEnd( + { + message: { + role: "assistant", + content: [{ type: "text", text: "all clear xdcp-message-idy still here" }], + stopReason: "stop", + timestamp: Date.now(), + }, + }, + ctx, + ); + + expect(notify).toHaveBeenCalledWith( + expect.stringContaining("model output looked malformed"), + "warning", + ); + expect(notify).not.toHaveBeenCalledWith(expect.stringContaining("stripped residual"), "info"); + // No stripping happened, so no message replacement. + expect(result).toBeUndefined(); + }); + + it("does not notify on a clean message with a tool call", async () => { + const { api, handlers } = createMockApi(); + createExtension(api); + const sessionStart = handlers.get("session_start")?.[0] as ( + ...args: unknown[] + ) => Promise; + await sessionStart({ reason: "new" }, makeSessionStartCtx()); + + const notify = vi.fn(); + const ctx = { hasUI: true, ui: { setStatus: vi.fn(), notify } }; + const messageEnd = handlers.get("message_end")?.[0] as (...args: unknown[]) => Promise; + const result = await messageEnd( + { + message: { + role: "assistant", + content: [ + { type: "text", text: "All done." }, + { + type: "toolCall", + id: "tc-1", + name: "read", + arguments: { path: "/tmp/x" }, + }, + ], + stopReason: "stop", + timestamp: Date.now(), + }, + }, + ctx, + ); + + expect(notify).not.toHaveBeenCalled(); + expect(result).toBeUndefined(); + }); +}); diff --git a/tests/strip.test.ts b/tests/strip.test.ts index a5790e9..fa0e625 100644 --- a/tests/strip.test.ts +++ b/tests/strip.test.ts @@ -50,6 +50,58 @@ describe("strip", () => { const input = "line1\n { + expect(stripHallucinationsFromString("-dcp-message-id>")).toBe(""); + }); + + it("strips inline prefix-less residual without leading hyphen", () => { + expect(stripHallucinationsFromString("dcp-message-id>")).toBe(""); + }); + + it("strips inline residual after a complete pair", () => { + expect( + stripHallucinationsFromString( + '-dcp-message-id>', + ), + ).toBe(""); + }); + + it("strips inline residual on its own line", () => { + expect(stripHallucinationsFromString("hello\n-dcp-message-id>\nworld")).toBe("hello\nworld"); + }); + + it("strips inline system-reminder residual", () => { + // The inline regex matches `-dcp-system-reminder>` but stops at the + // newline (the body class `[^<>\n]*` excludes newlines). The trailing + // `\n` remains. This is fine — the message_end handler treats whitespace + // as harmless and downstream code joins text parts with `\n` anyway. + expect(stripHallucinationsFromString("-dcp-system-reminder>\n")).toBe("\n"); + }); + + it("does not match prose that mentions the namespace without a >", () => { + expect(stripHallucinationsFromString("dcp-message-id is generally safe")).toBe( + "dcp-message-id is generally safe", + ); + expect(stripHallucinationsFromString("dcp-system-reminder is active")).toBe( + "dcp-system-reminder is active", + ); + }); + + it("does not match inside identifiers (boundary check)", () => { + expect(stripHallucinationsFromString("m0103-dcp-message-id>")).toBe("m0103-dcp-message-id>"); + }); + + it("documents the dcp-message-id foo>bar false positive", () => { + // Documented false positive — see docs/07 in the investigation chain. + // The inline residual requires `>` to be the terminator of the residual + // itself; any prose between the tag-name and `>` is consumed because + // attribute-bearing canonical tags may contain a space. False positive + // is bounded: namespace phrase is rare in English prose, trailing `>` + // is unusual, and the user-visible result is a slightly shorter + // sentence rather than data loss. + expect(stripHallucinationsFromString("dcp-message-id foo>bar")).toBe("bar"); + }); }); describe("stripHallucinations", () => { @@ -123,4 +175,52 @@ describe("strip", () => { expect(result[0]).toBe(messages[0]); }); }); + + describe("known-refs stripping", () => { + it("is a no-op when no refs are known", () => { + expect(stripHallucinationsFromString("m0103", new Set())).toBe("m0103"); + expect(stripHallucinationsFromString("m0103")).toBe("m0103"); + }); + + it("strips a single bare known ref", () => { + expect(stripHallucinationsFromString("m0103", new Set(["m0103"]))).toBe(""); + }); + + it("strips a known ref embedded in prose (line-152 case)", () => { + expect( + stripHallucinationsFromString( + "Sort the selected names alphabetically on enter:\n\n\n\nm0103", + new Set(["m0103"]), + ), + ).toBe("Sort the selected names alphabetically on enter:\n\n\n\n"); + }); + + it("does not strip a numeric token that looks like an m-id but isn't in the set", () => { + expect(stripHallucinationsFromString("the m1024 model", new Set(["m0103"]))).toBe( + "the m1024 model", + ); + }); + + it("does not match inside identifiers (boundary check)", () => { + expect(stripHallucinationsFromString("xem0103y", new Set(["m0103"]))).toBe("xem0103y"); + }); + + it("does not match when the ref is a prefix of a longer token", () => { + expect(stripHallucinationsFromString("m01034", new Set(["m0103"]))).toBe("m01034"); + }); + + it("matches longest-first when multiple refs share a prefix", () => { + // m0103 must not be stripped from inside m01034 even if m0103 is in + // the set. The sort-longest-first logic on the alternation source + // keeps m01034 from being shadowed. + const refs = new Set(["m01", "m0103"]); + expect(stripHallucinationsFromString("m01034", refs)).toBe("m01034"); + expect(stripHallucinationsFromString("m01", refs)).toBe(""); + }); + + it("strips multiple distinct known refs", () => { + const refs = new Set(["m0103", "m0117"]); + expect(stripHallucinationsFromString("m0103 and m0117", refs)).toBe(" and "); + }); + }); });