diff --git a/docs/superpowers/plans/2026-08-22-dcp-reliability-troubleshooting/phase-04-message-id-sanitization.md b/docs/superpowers/plans/2026-08-22-dcp-reliability-troubleshooting/phase-04-message-id-sanitization.md index 32fcfec..ed47873 100644 --- a/docs/superpowers/plans/2026-08-22-dcp-reliability-troubleshooting/phase-04-message-id-sanitization.md +++ b/docs/superpowers/plans/2026-08-22-dcp-reliability-troubleshooting/phase-04-message-id-sanitization.md @@ -2,229 +2,262 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Remove bounded orphan DCP message IDs without deleting legitimate prose and eliminate one redundant context-wide strip pass. +**Goal:** Remove bounded orphan, suffix-only, and transposed DCP message-ID fragments without deleting legitimate prose. -**Architecture:** Extend the ordered sanitizer with a narrow orphan-message-ID rule before lone-tag removal. Keep `message_end` as the persistence boundary and `injectMessageIds` as the model-context boundary; remove only the earlier pipeline-wide assistant cleanup. +**Architecture:** Extend the ordered string sanitizer with two narrow message-ID rules before lone-tag removal. Keep all existing cleanup boundaries: `message_end` protects persistence, the early pipeline pass cleans restored assistant content before priority calculation, and `injectMessageIds` cleans injectable user/assistant content before adding one canonical tag. -**Tech Stack:** TypeScript regular expressions, Pi `message_end` and `context` extension semantics, Vitest. +**Tech Stack:** TypeScript regular expressions, Pi 0.84.2 `message_end` and `context` extension semantics, Vitest. -**Spec:** `docs/superpowers/specs/2026-08-22-dcp-troubleshooting-design.md` +**Spec:** `docs/superpowers/specs/2026-08-22-dcp-troubleshooting-design.md`. This plan supersedes only the spec's instruction to remove the pipeline-wide assistant pass; the verified current pipeline reads that cleaned content in `buildPriorityMap()` before injection cleanup runs. ## Global Constraints -- Preserve ambiguous prose instead of consuming the remainder of a line or string. -- Consume only IDs matching `m\d{4,}` after an orphan `` opening tag. +- Use Node.js 24.15.0 or newer, as required by `package.json`. +- Add no dependency or new sanitizer abstraction. +- Remove only message references matching `m\d{4,}` when followed by a recognized closing tag or by a character outside a Unicode identifier. +- For suffix-only matches, require the `m` token not to be preceded by a Unicode identifier character so payloads such as `claim0001` and `文m0001` remain intact. +- Recognize only `dcp-message-id` and the observed `dpc-message-id` transposition; do not broaden every DCP tag rule to arbitrary `dpc-*` markup. +- Preserve ambiguous prose and identifier-like payloads such as `m0001abc`. - Keep complete-pair, truncated-pair, lone-tag, and partial-tag behavior. - Keep stripping idempotent. -- Keep `message_end` and `injectMessageIds` sanitization boundaries. +- Keep `message_end`, the early pipeline assistant pass, and `injectMessageIds` sanitization boundaries. --- -### Task 1: Specify orphan-ID behavior +## Verified Root Cause and Pi Lifecycle + +In session `2026-08-23T01-28-17-585Z_01a02c3b-95b1-7c96-8d29-33bcf0999465.jsonl`, DCP persisted `nextRefIndex: 112` immediately before MiniMax-M3 produced: + +```text +**Creating the GitHub PR**m0112 +``` + +The model predicted DCP's next sequential reference and transposed `dcp` to `dpc`. The current sanitizer recognizes only `dcp-*`, so the `message_end` handler returned no replacement and Pi persisted the malformed text unchanged. + +Pi 0.84.2 establishes the boundary ordering: + +- `packages/agent/src/agent-loop.ts` applies `transformContext` only to the messages sent to the model. +- `packages/coding-agent/src/core/extensions/runner.ts` chains `message_end` replacements. +- `packages/coding-agent/src/core/agent-session.ts` installs the replacement into agent state before appending the authoritative message to session history. + +Within DCP, the early `stripHallucinations(messages)` pass is not redundant: it runs before `buildPriorityMap(state, messages)`, while `injectMessageIds()` performs its cleanup afterward. Removing the early pass can change message-mode token counts and priority assignments, so this phase leaves it intact. + +--- + +### Task 1: Sanitize bounded malformed message-ID fragments **Files:** + - Modify: `tests/strip.test.ts` -- Modify: `tests/message-end.test.ts` +- Modify: `tests/index.test.ts` +- Modify: `tests/pipeline.test.ts` +- Modify: `src/messages/strip.ts` +- Verify unchanged: `src/index.ts` +- Verify unchanged: `src/messages/inject.ts` +- Verify unchanged: `src/pipeline.ts` +- Verify unchanged: `tests/message-end.test.ts` +- Verify unchanged: `tests/inject.test.ts` **Interfaces:** -- Consumes: `stripHallucinationsFromString()` and `mapText()`. -- Produces: bounded orphan-ID behavior shared by persistence and context sanitization. -- [ ] **Step 1: Add focused string cases** +- Consumes: `stripHallucinationsFromString(text: string): string`, the registered `message_end` handler, and `runPipeline()`. +- Produces: bounded cleanup shared by persistence and model-context sanitization without changing handler or pipeline interfaces. + +- [ ] **Step 1: Add focused string regressions** Add inside `describe("stripHallucinationsFromString")` in `tests/strip.test.ts`: ```typescript -it("removes an orphan message-id tag and its bounded reference", () => { - expect(stripHallucinationsFromString("hello m0001")).toBe("hello "); +it("removes the observed transposed message-id suffix", () => { + expect( + stripHallucinationsFromString( + "**Creating the GitHub PR**m0112", + ), + ).toBe("**Creating the GitHub PR**"); }); -it("preserves prose after an orphan message reference", () => { +it("removes bounded message-id suffixes and transposed pairs", () => { + expect(stripHallucinationsFromString("hello m0001")).toBe( + "hello ", + ); expect( - stripHallucinationsFromString("hello m0001 continued prose"), - ).toBe("hello continued prose"); + stripHallucinationsFromString( + "hello m0002", + ), + ).toBe("hello "); }); -it("preserves non-reference payload after an orphan opening tag", () => { - expect(stripHallucinationsFromString("hello discussion")) - .toBe("hello discussion"); +it("removes an orphan message-id opening tag and its bounded reference", () => { + expect(stripHallucinationsFromString("hello m0001")).toBe( + "hello ", + ); + expect(stripHallucinationsFromString("hello m0002")).toBe( + "hello ", + ); }); -it("removes adjacent orphan message references", () => { +it("preserves prose after an orphan message reference", () => { expect( stripHallucinationsFromString( - "m0001m0002", + "hello m0001 continued prose", ), - ).toBe(""); + ).toBe("hello continued prose"); }); -it("is idempotent for orphan message references", () => { - const once = stripHallucinationsFromString("hello m0001 prose"); - expect(stripHallucinationsFromString(once)).toBe(once); +it("preserves ambiguous message-like payloads", () => { + expect( + stripHallucinationsFromString("hello discussion"), + ).toBe("hello discussion"); + expect(stripHallucinationsFromString("hello m0001abc")).toBe( + "hello m0001abc", + ); }); -``` - -- [ ] **Step 2: Add a `message_end` persistence-boundary case** - -Add to `tests/message-end.test.ts`: -```typescript -it("removes an orphan message reference before persistence", () => { - const msg = makeAssistantMessage("Result m0093 followed by prose"); - - const stripped = mapText(msg, stripHallucinationsFromString); - const textPart = (stripped as unknown as { content: Array<{ text: string }> }).content[0]; - - expect(textPart.text).toBe("Result followed by prose"); +it("is idempotent for malformed message references", () => { + const once = stripHallucinationsFromString( + "hello m0001 prose m0002", + ); + expect(stripHallucinationsFromString(once)).toBe(once); }); ``` -- [ ] **Step 3: Run the focused tests and verify failure** - -Run: - -```bash -pnpm vitest run tests/strip.test.ts tests/message-end.test.ts -``` - -Expected: FAIL because `m0001`/`m0093` survive after the opening tag is removed. - -### Task 2: Add the bounded orphan rule - -**Files:** -- Modify: `src/messages/strip.ts` - -**Interfaces:** -- Consumes: text containing DCP markup. -- Produces: the existing `stripHallucinationsFromString(text: string): string` contract with bounded orphan-ID removal. - -- [ ] **Step 1: Add the ordered expression** +- [ ] **Step 2: Add the registered `message_end` boundary regression** -After `DCP_TRUNCATED_PAIR`, add: +Add this import to `tests/index.test.ts`: ```typescript -// 3. Orphan message-ID opening tag followed by a valid bounded reference. -const DCP_ORPHANED_MESSAGE_ID = /]*)?>m\d{4,}/gi; +import { makeAssistantMessage } from "./helpers.ts"; ``` -Renumber the comments for `DCP_UNPAIRED_TAG` and `DCP_PARTIAL_TAG`. - -- [ ] **Step 2: Apply it before lone-tag removal** - -Update the replacement chain: +Add inside `describe("dcp extension")`: ```typescript -return text - .replace(DCP_COMPLETE_PAIR, "") - .replace(DCP_TRUNCATED_PAIR, "") - .replace(DCP_ORPHANED_MESSAGE_ID, "") - .replace(DCP_UNPAIRED_TAG, "") - .replace(DCP_PARTIAL_TAG, ""); -``` - -Update the function comment to name orphan message references in the order description. - -- [ ] **Step 3: Run sanitizer tests** - -Run: - -```bash -pnpm vitest run tests/strip.test.ts tests/message-end.test.ts tests/inject.test.ts +it("message_end strips the observed transposed message-id suffix", async () => { + const { api, handlers } = createMockApi(); + createExtension(api); + + const handler = handlers.get("message_end")?.[0]; + expect(handler).toBeDefined(); + + const result = await (handler as (...args: unknown[]) => Promise)( + { + type: "message_end", + message: makeAssistantMessage( + "**Creating the GitHub PR**m0112", + ), + }, + {}, + ); + + expect(result).toBeDefined(); + const message = ( + result as { message: { content: Array<{ type: string; text?: string }> } } + ).message; + expect(message.content[0]?.text).toBe("**Creating the GitHub PR**"); +}); ``` -Expected: PASS. +This exercises the actual handler registered in `src/index.ts`; do not add another manual `mapText()` case to `tests/message-end.test.ts`. -### Task 3: Remove only the redundant pipeline pass +- [ ] **Step 3: Add the restored-context pipeline regression** -**Files:** -- Modify: `src/pipeline.ts` -- Modify: `tests/pipeline.test.ts` -- Verify: `tests/inject.test.ts` - -**Interfaces:** -- Consumes: `injectMessageIds()`, which still cleans existing tags before canonical injection. -- Produces: `runPipeline()` without a separate pre-injection assistant strip. - -- [ ] **Step 1: Add a pipeline boundary regression** - -Replace the current pipeline hallucination test input with an orphan reference and strengthen the assertion: +Replace the existing pipeline hallucination test in `tests/pipeline.test.ts` with: ```typescript -it("sanitizes orphan DCP refs at the injection boundary", () => { +it("sanitizes a persisted transposed message-id suffix before canonical injection", () => { const state = createSessionState(); const config = makeDefaultConfig(); - const messages = [ + const messages: AgentMessage[] = [ makeUserMessage("Hello"), - makeAssistantMessage("Response m0099 followed by prose"), + makeAssistantMessage("**Creating the GitHub PR**m0112"), ]; const result = runPipeline(state, config, messages, undefined); const text = extractMessageText(result.messages[1]); - expect(text).toContain("Response followed by prose"); - expect(text).not.toContain("m0099"); + expect(text).toContain("**Creating the GitHub PR**"); + expect(text).not.toContain("m0112"); + expect(text).not.toContain("dpc-message-id"); expect(text.match(/]*)?>)?(?/giu; +// 4. Orphan message-ID opening tag followed by a valid bounded reference. +const DCP_ORPHANED_MESSAGE_ID = + /<(?:dcp|dpc)-message-id(?:\s[^>]*)?>m\d{4,}(?!\p{ID_Continue})/giu; ``` -Replace: +Renumber the comments for `DCP_UNPAIRED_TAG` and `DCP_PARTIAL_TAG`, then update the replacement chain to: ```typescript -let result = stripHallucinations(messages); +return text + .replace(DCP_COMPLETE_PAIR, "") + .replace(DCP_TRUNCATED_PAIR, "") + .replace(DCP_MESSAGE_ID_SUFFIX_OR_PAIR, "") + .replace(DCP_ORPHANED_MESSAGE_ID, "") + .replace(DCP_UNPAIRED_TAG, "") + .replace(DCP_PARTIAL_TAG, ""); ``` -with: +Update the function comment to describe this exact order. Do not change the generic DCP expressions to match arbitrary `dpc-*` tags. -```typescript -let result = messages; -``` +- [ ] **Step 6: Run the sanitizer and boundary tests** -Update the Step 0 comment to: +Run: -```typescript -// Step 0: Rebuild stable refs before state rehydration. +```bash +pnpm vitest run tests/strip.test.ts tests/message-end.test.ts tests/index.test.ts tests/inject.test.ts tests/pipeline.test.ts ``` -Do not remove `stripHallucinationsFromString` from `src/messages/inject.ts` or `src/index.ts`. +Expected: PASS. -- [ ] **Step 4: Run all sanitization and pipeline tests** +- [ ] **Step 7: Verify the supported runtime and full repository** Run: ```bash -pnpm vitest run tests/strip.test.ts tests/message-end.test.ts tests/inject.test.ts tests/pipeline.test.ts -pnpm typecheck +node -e 'const [major, minor] = process.versions.node.split(".").map(Number); if (major < 24 || (major === 24 && minor < 15)) { console.error(`Node >=24.15.0 required, found ${process.versions.node}`); process.exit(1); }' +pnpm check git diff --check ``` -Expected: all PASS. +Expected: the runtime guard exits successfully, formatting/lint/typecheck/full tests pass, and `git diff --check` reports no whitespace errors. + +- [ ] **Step 8: Review the final scope** -- [ ] **Step 5: Commit Phase 4** +Run: ```bash -git add src/messages/strip.ts src/pipeline.ts tests/strip.test.ts tests/message-end.test.ts tests/pipeline.test.ts -git commit -m "fix: strip orphan dcp message references safely" +git diff --stat +git diff -- src/messages/strip.ts tests/strip.test.ts tests/index.test.ts tests/pipeline.test.ts ``` -`tests/inject.test.ts` is verification-only unless implementation requires a justified assertion update. +Expected: production changes are limited to `src/messages/strip.ts`; `src/index.ts`, `src/messages/inject.ts`, and `src/pipeline.ts` remain unchanged. + +- [ ] **Step 9: Commit Phase 4** + +```bash +git add src/messages/strip.ts tests/strip.test.ts tests/index.test.ts tests/pipeline.test.ts +git commit -m "fix: sanitize malformed dcp message references" +``` diff --git a/src/messages/strip.ts b/src/messages/strip.ts index cbedfed..e2f931a 100644 --- a/src/messages/strip.ts +++ b/src/messages/strip.ts @@ -5,22 +5,29 @@ import { mapText } from "../utils/message-content.ts"; const DCP_COMPLETE_PAIR = /]*)?>[\s\S]*?<\/dcp[-\w]*>/gi; // 2. Truncated pair (no final > on close): content]*)?>[\s\S]*?<\/dcp[-\w]*/gi; -// 3. Lone unpaired tags: or +// 3. Bounded message-ID suffixes or pairs, including the observed dpc transposition. +const DCP_MESSAGE_ID_SUFFIX_OR_PAIR = + /(?:<(?:dcp|dpc)-message-id(?:\s[^>]*)?>)?(?/giu; +// 4. Orphan message-ID opening tag followed by a valid bounded reference. +const DCP_ORPHANED_MESSAGE_ID = /<(?:dcp|dpc)-message-id(?:\s[^>]*)?>m\d{4,}(?!\p{ID_Continue})/giu; +// 5. Lone unpaired tags: or const DCP_UNPAIRED_TAG = /<\/?dcp[-\w]*(?:\s[^>]*)?>/gi; -// 4. Partial tag at end of line/string: \n]*)?$/gim; /** * Strip hallucinated DCP tags from a string. - * Handles complete paired tags, truncated pairs, lone unpaired tags, and - * partial tags at end of string. Order matters: complete pairs first (they - * consume the closing >), then truncated pairs, then lone tags, then partials. + * Handles complete pairs, truncated pairs, bounded message-ID suffixes or + * pairs, orphan message-ID openings, lone unpaired tags, and partial tags. + * Order matters: each more-specific pattern runs before its broader fallback. */ export function stripHallucinationsFromString(text: string): string { return text .replace(DCP_COMPLETE_PAIR, "") .replace(DCP_TRUNCATED_PAIR, "") + .replace(DCP_MESSAGE_ID_SUFFIX_OR_PAIR, "") + .replace(DCP_ORPHANED_MESSAGE_ID, "") .replace(DCP_UNPAIRED_TAG, "") .replace(DCP_PARTIAL_TAG, ""); } diff --git a/tests/index.test.ts b/tests/index.test.ts index 8e6811b..6ba6fbb 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -6,6 +6,7 @@ import createExtension from "../src/index.ts"; import * as subagentResults from "../src/subagents/subagent-results.ts"; import { createSessionState } from "../src/state/state.ts"; import { serializeDcpSnapshot } from "../src/state/persistence.ts"; +import { makeAssistantMessage } from "./helpers.ts"; const agentDir = vi.hoisted(() => `/tmp/dcp-index-test-${Date.now()}-${Math.random()}`); @@ -128,6 +129,27 @@ describe("dcp extension", () => { ).resolves.toBeUndefined(); }); + it("message_end strips the observed transposed message-id suffix", async () => { + const { api, handlers } = createMockApi(); + createExtension(api); + + const handler = handlers.get("message_end")?.[0]; + expect(handler).toBeDefined(); + + const result = await (handler as (...args: unknown[]) => Promise)( + { + type: "message_end", + message: makeAssistantMessage("**Creating the GitHub PR**m0112"), + }, + {}, + ); + + expect(result).toBeDefined(); + const message = (result as { message: { content: Array<{ type: string; text?: string }> } }) + .message; + expect(message.content[0]?.text).toBe("**Creating the GitHub PR**"); + }); + it("context handler tags messages with dcp-message-id", async () => { const { api, handlers } = createMockApi(); createExtension(api); diff --git a/tests/pipeline.test.ts b/tests/pipeline.test.ts index 34932cd..3dea6ec 100644 --- a/tests/pipeline.test.ts +++ b/tests/pipeline.test.ts @@ -35,25 +35,22 @@ describe("runPipeline", () => { expect(state.nudges.iterationAnchors).toEqual(new Set()); }); - it("strips hallucinated DCP tags from assistant messages", () => { + it("sanitizes a persisted transposed message-id suffix before canonical injection", () => { const state = createSessionState(); const config = makeDefaultConfig(); const messages: AgentMessage[] = [ makeUserMessage("Hello"), - makeAssistantMessage('Response with hallucination'), + makeAssistantMessage("**Creating the GitHub PR**m0112"), ]; const result = runPipeline(state, config, messages, undefined); + const text = extractMessageText(result.messages[1]); - const assistantContent = (result.messages[1] as any).content as Array<{ - type: string; - text: string; - }>; - const text = assistantContent[0].text; - // The hallucinated ref should have been stripped and replaced with the correct sequential ref - expect(text).not.toContain('ref="m0001"'); - // A legitimate message ID was injected (not the hallucinated one) - expect(text).toContain(" { diff --git a/tests/strip.test.ts b/tests/strip.test.ts index a5790e9..da5942d 100644 --- a/tests/strip.test.ts +++ b/tests/strip.test.ts @@ -4,6 +4,58 @@ import type { AgentMessage } from "@earendil-works/pi-agent-core"; describe("strip", () => { describe("stripHallucinationsFromString", () => { + it("removes the observed transposed message-id suffix", () => { + expect( + stripHallucinationsFromString("**Creating the GitHub PR**m0112"), + ).toBe("**Creating the GitHub PR**"); + }); + + it("removes bounded message-id suffixes and transposed pairs", () => { + expect(stripHallucinationsFromString("hello m0001")).toBe("hello "); + expect(stripHallucinationsFromString("hello m0002")).toBe( + "hello ", + ); + }); + + it("preserves identifier-like text before a message-id suffix", () => { + expect(stripHallucinationsFromString("claim0001")).toBe("claim0001"); + expect(stripHallucinationsFromString("room0001")).toBe( + "room0001", + ); + expect(stripHallucinationsFromString("文m0001")).toBe("文m0001"); + expect(stripHallucinationsFromString("ém0001")).toBe( + "ém0001", + ); + }); + + it("removes an orphan message-id opening tag and its bounded reference", () => { + expect(stripHallucinationsFromString("hello m0001")).toBe("hello "); + expect(stripHallucinationsFromString("hello m0002")).toBe("hello "); + }); + + it("preserves prose after an orphan message reference", () => { + expect(stripHallucinationsFromString("hello m0001 continued prose")).toBe( + "hello continued prose", + ); + }); + + it("preserves ambiguous message-like payloads", () => { + expect(stripHallucinationsFromString("hello discussion")).toBe( + "hello discussion", + ); + expect(stripHallucinationsFromString("hello m0001abc")).toBe( + "hello m0001abc", + ); + expect(stripHallucinationsFromString("hello m0001文")).toBe("hello m0001文"); + }); + + it("is idempotent for malformed message references", () => { + const once = stripHallucinationsFromString( + "hello m0001 prose m0002", + ); + expect(stripHallucinationsFromString(once)).toBe(once); + }); + it("removes paired dcp tags", () => { const result = stripHallucinationsFromString( "hello m0001 world",