diff --git a/shared/src/messages.test.ts b/shared/src/messages.test.ts index b6f4de141a..80df94903c 100644 --- a/shared/src/messages.test.ts +++ b/shared/src/messages.test.ts @@ -1,8 +1,11 @@ import { describe, expect, test } from 'bun:test' import { + collapseRepeats, extractAssistantPlainText, extractNotifySummary, isRedundantGoalStatusEventContent, + matchNotifySummaryLine, + NOTIFY_SUMMARY_TOKEN, type NotifySummary } from './messages' @@ -207,6 +210,60 @@ describe('extractNotifySummary + extractAssistantPlainText (integration)', () => }) }) +describe('collapseRepeats (corruption normalizer)', () => { + test('collapses runs of a repeated character to one', () => { + expect(collapseRepeats('SUMMARY')).toBe('SUMARY') + expect(collapseRepeats('aaa-bb-c')).toBe('a-b-c') + expect(collapseRepeats('')).toBe('') + expect(collapseRepeats('abc')).toBe('abc') + }) + + test('the correct token and the Cursor dup-drop collapse to the same form', () => { + // AGENT_NOTIFY_SUMMARY -> AGENT_NOTIFY_SUMARY (drop one M); + // both must normalize equal so a corrupted line still matches. + expect(collapseRepeats(NOTIFY_SUMMARY_TOKEN)).toBe(collapseRepeats('AGENT_NOTIFY_SUMARY')) + }) +}) + +describe('matchNotifySummaryLine (corruption-tolerant detector)', () => { + test('matches the correct token line and returns the JSON', () => { + expect(matchNotifySummaryLine('AGENT_NOTIFY_SUMMARY {"status":"done"}')).toBe('{"status":"done"}') + }) + + test('matches the Cursor dup-dropped token (SUMARY)', () => { + expect(matchNotifySummaryLine('AGENT_NOTIFY_SUMARY {"status":"done"}')).toBe('{"status":"done"}') + }) + + test('tolerates trailing/leading whitespace on the line', () => { + expect(matchNotifySummaryLine(' AGENT_NOTIFY_SUMMARY {"a":1} ')).toBe('{"a":1}') + }) + + test('rejects a token embedded in prose (not a bare token)', () => { + expect(matchNotifySummaryLine('see the AGENT_NOTIFY_SUMMARY {"x":1}')).toBeNull() + }) + + test('rejects lines with no JSON or no leading token', () => { + expect(matchNotifySummaryLine('AGENT_NOTIFY_SUMMARY not json')).toBeNull() + expect(matchNotifySummaryLine('{"x":1}')).toBeNull() + expect(matchNotifySummaryLine('')).toBeNull() + }) + + test('rejects a non-dup deletion (NOTFY) - acceptable residual loss', () => { + // Non-adjacent-duplicate corruption is not recoverable by collapse-norm; + // observed rate in the wild is ~2 in 5800. Documented as acceptable. + expect(matchNotifySummaryLine('AGENT_NOTFY_SUMMARY {"x":1}')).toBeNull() + }) +}) + +describe('extractNotifySummary (corruption-tolerant end-to-end)', () => { + test('parses a corrupted (SUMARY) trailing line into a summary object', () => { + const text = 'Did the thing.\n\nAGENT_NOTIFY_SUMARY {"version":1,"status":"blocked","summary":"needs key"}' + const r = extractNotifySummary(text) + expect(r?.status).toBe('blocked') + expect(r?.summary).toBe('needs key') + }) +}) + describe('isRedundantGoalStatusEventContent (regression-guard for messages.ts edits)', () => { test('still detects goal-active events', () => { const value = { diff --git a/shared/src/messages.ts b/shared/src/messages.ts index 551b308207..65ddfcd4f5 100644 --- a/shared/src/messages.ts +++ b/shared/src/messages.ts @@ -117,7 +117,7 @@ export function extractAssistantPlainText(content: unknown): string | null { return null } -const NOTIFY_SUMMARY_PREFIX = 'AGENT_NOTIFY_SUMMARY ' +export const NOTIFY_SUMMARY_TOKEN = 'AGENT_NOTIFY_SUMMARY' export type NotifySummary = { version?: number @@ -128,20 +128,57 @@ export type NotifySummary = { summary?: string } +/** + * Collapse every run of a repeated character down to a single instance + * (`"SUMMARY"` -> `"SUMARY"`, `"aaa-bb"` -> `"a-b"`). + * + * This is the corruption-normalizer for the notify contract. Observed in + * the wild (354k stored messages): Cursor drops one of a doubled letter in + * roughly 1 of 7 turns, mangling `AGENT_NOTIFY_SUMMARY` -> `AGENT_NOTIFY_SUMARY` + * (Claude/Codex: 0 occurrences). Matching on the collapse-normalized form + * makes the correct and dup-dropped tokens compare equal, so a corrupted + * line is still detected (and therefore both stripped from the human view + * and parsed into an inbox event) instead of double-failing. + */ +export function collapseRepeats(value: string): string { + return value.replace(/(.)\1+/g, '$1') +} + +const NOTIFY_SUMMARY_TOKEN_NORM = collapseRepeats(NOTIFY_SUMMARY_TOKEN) + +/** + * If `line` is a notify-summary line, return the raw JSON substring `{...}`; + * otherwise `null`. `line` is expected to already be the last non-empty line + * of a message (callers enforce the end-anchor). + * + * Corruption-tolerant: the leading token is matched by collapse-normalized + * equality (see `collapseRepeats`), so Cursor's `SUMMARY`->`SUMARY` dup-drop + * still matches. The JSON must start with `{` and end with `}`; a token that + * is embedded in prose (`"see the AGENT_NOTIFY_SUMMARY {..}"`) fails because + * its collapse-normalized prefix will not equal the bare token. + */ +export function matchNotifySummaryLine(line: string): string | null { + const trimmed = line.trim() + const braceIdx = trimmed.indexOf('{') + if (braceIdx <= 0) return null + const token = trimmed.slice(0, braceIdx).trim() + if (!token || collapseRepeats(token) !== NOTIFY_SUMMARY_TOKEN_NORM) return null + const jsonPart = trimmed.slice(braceIdx).trim() + if (!jsonPart.startsWith('{') || !jsonPart.endsWith('}')) return null + return jsonPart +} + /** * Look for an `AGENT_NOTIFY_SUMMARY {...json...}` line as the **last * non-empty line** of an agent's plain-text message. * - * Strict end-anchor: anything below the JSON line (even whitespace) is - * fine, but if the agent wrote prose AFTER the line we treat it as - * non-compliant and return null. This also makes false positives from + * Strict end-anchor: trailing whitespace-only lines are fine, but prose + * AFTER the JSON line is treated as non-compliant. This also makes a * `AGENT_NOTIFY_SUMMARY` quoted inside an earlier paragraph harmless, * because such a quote is never the last line. * - * Returns the parsed object on success, `null` on any deviation. The - * shape is intentionally loose - we only trust `summary`, `action`, and - * `status` for notification rendering, but the full object is forwarded - * onto the meta-event bus when Phase 2 lands. + * The token match is corruption-tolerant (see `matchNotifySummaryLine`). + * Returns the parsed object on success, `null` on any deviation. */ export function extractNotifySummary(text: unknown): NotifySummary | null { if (typeof text !== 'string' || text.length === 0) return null @@ -151,11 +188,8 @@ export function extractNotifySummary(text: unknown): NotifySummary | null { while (lastIdx >= 0 && lines[lastIdx].trim() === '') lastIdx -= 1 if (lastIdx < 0) return null - const lastLine = lines[lastIdx].trim() - if (!lastLine.startsWith(NOTIFY_SUMMARY_PREFIX)) return null - - const jsonPart = lastLine.slice(NOTIFY_SUMMARY_PREFIX.length).trim() - if (!jsonPart.startsWith('{') || !jsonPart.endsWith('}')) return null + const jsonPart = matchNotifySummaryLine(lines[lastIdx]) + if (!jsonPart) return null try { const parsed: unknown = JSON.parse(jsonPart) diff --git a/shared/src/overseerEvents.test.ts b/shared/src/overseerEvents.test.ts index 94956fdd8d..147eca8621 100644 --- a/shared/src/overseerEvents.test.ts +++ b/shared/src/overseerEvents.test.ts @@ -15,8 +15,10 @@ import { normalizeUrlIdempotencyKey, OVERSEER_EVENT_TYPES, HAPI_EVENTS_BEGIN, - HAPI_EVENTS_END + HAPI_EVENTS_END, + stripAgentContract } from './overseerEvents' +import { extractNotifySummary } from './messages' describe('overseerEvents mapping', () => { test('maps notify status to event_type', () => { @@ -113,3 +115,45 @@ describe('overseerEvents mapping', () => { expect(normalizeUrlIdempotencyKey('https://Example.COM/path/#frag')).toBe('https://example.com/path') }) }) + +describe('stripAgentContract (render-only, human-facing)', () => { + test('strips a correct trailing summary line + the blank line above it', () => { + const text = 'Here is the answer.\n\nAGENT_NOTIFY_SUMMARY {"status":"done","summary":"ok"}' + expect(stripAgentContract(text)).toBe('Here is the answer.') + }) + + test('strips a corrupted (SUMARY) trailing summary line', () => { + const text = 'Here is the answer.\nAGENT_NOTIFY_SUMARY {"status":"done"}' + expect(stripAgentContract(text)).toBe('Here is the answer.') + }) + + test('strips the leading inline-contract prefix block (historical stored msgs)', () => { + const text = `${AGENT_NOTIFY_CONTRACT_INLINE_PREFIX}please do the thing` + expect(stripAgentContract(text)).toBe('please do the thing') + }) + + test('strips both leading prefix and trailing summary in one pass', () => { + const text = `${AGENT_NOTIFY_CONTRACT_INLINE_PREFIX}real content\nAGENT_NOTIFY_SUMMARY {"status":"done"}` + expect(stripAgentContract(text)).toBe('real content') + }) + + test('leaves a quoted-but-not-last token untouched', () => { + const text = 'I emit AGENT_NOTIFY_SUMMARY {json} at the end.\nThen more prose.' + expect(stripAgentContract(text)).toBe(text) + }) + + test('no-ops on clean text and empty input', () => { + expect(stripAgentContract('just a normal reply')).toBe('just a normal reply') + expect(stripAgentContract('')).toBe('') + }) + + test('round-trip invariant: overseer still parses raw, human view has no marker', () => { + const raw = 'Work done.\nAGENT_NOTIFY_SUMARY {"status":"done","summary":"shipped"}' + // overseer reads the RAW text and still gets the event (corruption-tolerant) + expect(extractNotifySummary(raw)?.summary).toBe('shipped') + // the human render is stripped clean and can no longer parse a marker + const human = stripAgentContract(raw) + expect(human).toBe('Work done.') + expect(extractNotifySummary(human)).toBeNull() + }) +}) diff --git a/shared/src/overseerEvents.ts b/shared/src/overseerEvents.ts index b9f44a895f..6c5a8654bd 100644 --- a/shared/src/overseerEvents.ts +++ b/shared/src/overseerEvents.ts @@ -1,4 +1,5 @@ import type { NotifySummary } from './messages' +import { matchNotifySummaryLine } from './messages' export const NOTIFY_SUMMARY_STATUSES = [ 'done', @@ -21,6 +22,41 @@ export const AGENT_NOTIFY_CONTRACT_INLINE_PREFIX = [ '' ].join('\n') +/** + * Strip the machine-only notify contract from text destined for HUMAN eyes. + * + * The `AGENT_NOTIFY_SUMMARY` contract rides fully in-band so it works across + * every agent flavor, but it must never reach the human render. Two removals: + * 1. The trailing `AGENT_NOTIFY_SUMMARY {...}` line (collapse-normalized, so + * Cursor's corrupted `SUMARY` variant strips too) plus any blank lines it + * leaves behind. + * 2. A leading inline-contract prefix block - only present on historical + * operator messages stored before input-side decoupling (the hub now + * injects the prefix into the agent-bound copy only, never the stored one). + * + * Overseer event capture and notification builders MUST read the raw text, not + * this - stripping is render-only so the machine signal survives in the store. + */ +export function stripAgentContract(text: string): string { + if (typeof text !== 'string' || text.length === 0) return text + let out = text + + if (out.startsWith(AGENT_NOTIFY_CONTRACT_INLINE_PREFIX)) { + out = out.slice(AGENT_NOTIFY_CONTRACT_INLINE_PREFIX.length) + } + + const lines = out.split('\n') + let lastIdx = lines.length - 1 + while (lastIdx >= 0 && lines[lastIdx].trim() === '') lastIdx -= 1 + if (lastIdx >= 0 && matchNotifySummaryLine(lines[lastIdx])) { + const kept = lines.slice(0, lastIdx) + while (kept.length > 0 && kept[kept.length - 1].trim() === '') kept.pop() + out = kept.join('\n') + } + + return out +} + export const HAPI_EVENTS_BEGIN = '' export const HAPI_EVENTS_END = '' diff --git a/web/src/lib/assistant-runtime.ts b/web/src/lib/assistant-runtime.ts index 7769ead3c2..8400fe766a 100644 --- a/web/src/lib/assistant-runtime.ts +++ b/web/src/lib/assistant-runtime.ts @@ -4,7 +4,7 @@ import type { AppendMessage, AttachmentAdapter, ThreadMessageLike } from '@assis import { useExternalMessageConverter, useExternalStoreRuntime } from '@assistant-ui/react' import type { PendingSchedule } from '@/components/AssistantChat/ScheduleTimePicker' import { resolvePendingSchedule } from '@/components/AssistantChat/ScheduleTimePicker' -import { safeStringify } from '@hapi/protocol' +import { safeStringify, stripAgentContract } from '@hapi/protocol' import { renderEventLabel } from '@/chat/presentation' import type { ChatBlock, CliOutputBlock, CodexReview, UsageData } from '@/chat/types' import type { AgentEvent, ToolCallBlock } from '@/chat/types' @@ -324,7 +324,11 @@ function toThreadMessageLike(block: VisibleChatBlock, threadMessageId: string): role: 'user', id: threadMessageId, createdAt: new Date(block.createdAt), - content: [{ type: 'text', text: block.text }], + // Strip the machine-only notify contract from the human render. On + // non-Cursor flavors the hub prepends an inline contract prefix to + // the stored operator message (#20); stripAgentContract removes that + // leading block. No-op when absent. + content: [{ type: 'text', text: stripAgentContract(block.text) }], metadata: { custom: { kind: 'user', @@ -343,7 +347,12 @@ function toThreadMessageLike(block: VisibleChatBlock, threadMessageId: string): role: 'assistant', id: threadMessageId, createdAt: new Date(block.createdAt), - content: [{ type: 'text', text: block.text }], + // Strip the trailing AGENT_NOTIFY_SUMMARY line (collapse-normalized, + // so Cursor's corrupted SUMARY variant strips too) so the human never + // sees the machine contract. The raw text stays in the store for the + // overseer event/inbox pipeline. copyText derives from this content, + // so the clipboard is clean too. + content: [{ type: 'text', text: stripAgentContract(block.text) }], metadata: { custom: { kind: 'assistant',