diff --git a/rules/local-agent-tools.md b/rules/local-agent-tools.md index c47127a691..92ed6c2418 100644 --- a/rules/local-agent-tools.md +++ b/rules/local-agent-tools.md @@ -228,7 +228,7 @@ Agent tool definitions live in `src/pro/main/ipc/handlers/local_agent/tools/`. E - When extending `handleLocalAgentStream` retry behavior, do not only match transport errors like `"terminated"`. Providers can emit structured stream errors such as `{ type: "error", error: { type: "server_error", ... } }`, and those transient 5xx / rate-limit failures need explicit retry classification too. - Anthropic rejects any assistant `tool_use` unless the immediately following message contains every matching `tool_result`. When changing local-agent history assembly, retry replay, message injection, or `aiMessagesJson` persistence, run the transcript through the shared tool-call sanitizer at the provider/persistence boundary rather than relying only on the injection site to preserve ordering. - In `prepareStep`-style paths, normalize the step message array even when `prepareStepMessages` returns `undefined`; split parallel tool results can still need merging on no-injection/no-compaction steps. Prefer the shared `sanitizeStepMessages` helper over ad hoc reference comparisons. -- Persisted assistant Git hashes (`sourceCommitHash` / `commitHash`) are database metadata, not part of `content` or `aiMessagesJson`. When local-agent replay needs that provenance, append an in-memory annotation only after `parseAiMessagesJson` has reconstructed the complete database message. Prefer the final `commitHash`; use `sourceCommitHash` only when no final commit exists, and never rewrite the stored transcript or insert the annotation inside a tool-call/tool-result pair. +- Persisted assistant Git hashes (`sourceCommitHash` / `commitHash`) are database metadata, not part of `content` or `aiMessagesJson`. When local-agent replay needs that provenance, append an in-memory annotation only after `parseAiMessagesJson` has reconstructed the complete database message. Prefer the final `commitHash`; use `sourceCommitHash` only when no final commit exists, and never rewrite the stored transcript or insert the annotation inside a tool-call/tool-result pair. Strip model-echoed `` tags from both streamed display text and persisted AI messages so internal provenance cannot leak into the visible transcript or be replayed as model-authored content. - Keep clean no-op turns unversioned: retain their `sourceCommitHash` for provenance but leave `commitHash` null. Do not attach the current `HEAD` merely because it exists; that attributes another turn's checkpoint and exposes unrelated files in version UI and snapshots. ## Metadata-only stop tools diff --git a/rules/typescript-strict-mode.md b/rules/typescript-strict-mode.md index 153da0c107..f133382876 100644 --- a/rules/typescript-strict-mode.md +++ b/rules/typescript-strict-mode.md @@ -59,6 +59,13 @@ When the value must escape a callback (a `withLock` body, for example), return it from that callback and destructure the result rather than closing over a mutable binding. +## `flatMap` over heterogeneous unions may fail inference + +`tsgo` can reject a `flatMap` callback over a discriminated-union array when +different branches return different member arrays. Build an accumulator typed +as `typeof sourceArray` and `push` narrowed members instead; this preserves the +complete union without broad casts. + ## i18next `t()` keys are a literal union, not `string` `useTranslation` returns a `t()` typed against a union of every key in the namespace. Passing a variable whose type widens to `string` (e.g., a `labelKey` field collected into an array) fails with `TS2345: Argument of type '[string]' is not assignable to parameter of type '[key: "added" | "..."]'`. Resolve the label at the call site (`{ label: t("groupToday") }`) instead of storing the key for later lookup (`{ labelKey: "groupToday" }` then `t(group.labelKey)`). If you really need late binding, narrow with `as const` so the literal type survives. diff --git a/src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.test.ts b/src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.test.ts new file mode 100644 index 0000000000..dda52aedfd --- /dev/null +++ b/src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.test.ts @@ -0,0 +1,188 @@ +import { describe, expect, it } from "vitest"; + +import { + GitContextEchoSanitizer, + stripGitContextEchoes, + stripGitContextEchoesFromAssistantMessages, +} from "./git_context_sanitizer"; + +describe("GitContextEchoSanitizer", () => { + it("strips an echoed Git-context tag split across stream chunks", () => { + const sanitizer = new GitContextEchoSanitizer(); + + const output = [ + sanitizer.push("Done.'), + sanitizer.push(""), + sanitizer.finish(), + ].join(""); + + expect(output).toBe("Done."); + }); + + it("preserves ordinary text around internal tag markup", () => { + expect( + stripGitContextEchoes( + 'Before unexpected after', + ), + ).toBe("Before unexpected after"); + }); + + it("does not strip similarly named user text", () => { + expect(stripGitContextEchoes("Use here.")).toBe( + "Use here.", + ); + }); + + it("preserves unterminated prose instead of buffering it without bound", () => { + const sanitizer = new GitContextEchoSanitizer(); + const prose = ` { + expect( + stripGitContextEchoes( + 'İafter', + ), + ).toBe("İafter"); + }); + + it("drops a distinctive internal tag fragment if the stream ends", () => { + const sanitizer = new GitContextEchoSanitizer(); + + expect(sanitizer.push("Done. { + const tag = ''; + const messages = stripGitContextEchoesFromAssistantMessages([ + { role: "user", content: `User literal: ${tag}` }, + { + role: "assistant", + content: [ + { type: "text", text: `Finished.${tag}` }, + { + type: "tool-call", + toolCallId: "call-1", + toolName: "read_file", + input: {}, + }, + ], + }, + ]); + + expect(messages[0]).toEqual({ + role: "user", + content: `User literal: ${tag}`, + }); + expect(messages[1]).toMatchObject({ + role: "assistant", + content: [ + { type: "text", text: "Finished." }, + { type: "tool-call", toolCallId: "call-1" }, + ], + }); + }); + + it("sanitizes tags split across assistant text parts", () => { + const messages = stripGitContextEchoesFromAssistantMessages([ + { + role: "assistant", + content: [ + { type: "text", text: "Before inside after" }, + ], + }, + ]); + + expect(messages).toEqual([ + { + role: "assistant", + content: [ + { type: "text", text: "Before " }, + { type: "text", text: "inside" }, + { type: "text", text: " after" }, + ], + }, + ]); + }); + + it("sanitizes reasoning parts and drops empty assistant content", () => { + const tag = ''; + const messages = stripGitContextEchoesFromAssistantMessages([ + { role: "assistant", content: tag }, + { + role: "assistant", + content: [{ type: "text", text: tag }], + }, + { + role: "assistant", + content: [ + { type: "reasoning", text: `Consider.${tag}` }, + { + type: "tool-call", + toolCallId: "call-1", + toolName: "read_file", + input: {}, + }, + { type: "text", text: tag }, + ], + }, + ]); + + expect(messages).toEqual([ + { + role: "assistant", + content: [ + { type: "reasoning", text: "Consider." }, + { + type: "tool-call", + toolCallId: "call-1", + toolName: "read_file", + input: {}, + }, + ], + }, + ]); + }); + + it("drops modified reasoning parts with provider-bound metadata", () => { + const tag = ''; + + expect( + stripGitContextEchoesFromAssistantMessages([ + { + role: "assistant", + content: [ + { + type: "reasoning", + text: `Signed thought.${tag}`, + providerOptions: { + anthropic: { signature: "signed-original-reasoning" }, + }, + }, + { type: "text", text: "Answer" }, + ], + }, + ]), + ).toEqual([ + { + role: "assistant", + content: [{ type: "text", text: "Answer" }], + }, + ]); + }); +}); diff --git a/src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.ts b/src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.ts new file mode 100644 index 0000000000..1b8c6c1d3a --- /dev/null +++ b/src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.ts @@ -0,0 +1,176 @@ +import type { ModelMessage } from "ai"; + +const GIT_CONTEXT_TAG_MARKERS = [ + " 0) { + const markerIndex = findNextMarkerIndex(this.pending); + if (markerIndex === -1) { + const heldSuffixLength = getPotentialMarkerSuffixLength(this.pending); + const emitLength = this.pending.length - heldSuffixLength; + output += this.pending.slice(0, emitLength); + this.pending = this.pending.slice(emitLength); + break; + } + + output += this.pending.slice(0, markerIndex); + this.pending = this.pending.slice(markerIndex); + + const tagEndIndex = this.pending.indexOf(">"); + if (tagEndIndex === -1) { + if (this.pending.length > MAX_TAG_MARKUP_LENGTH) { + output += this.pending; + this.pending = ""; + } + break; + } + this.pending = this.pending.slice(tagEndIndex + 1); + } + + return output; + } + + finish(): string { + const pending = this.pending; + this.pending = ""; + return startsWithDistinctivePartialMarker(pending) ? "" : pending; + } +} + +export function stripGitContextEchoes(text: string): string { + const sanitizer = new GitContextEchoSanitizer(); + return sanitizer.push(text) + sanitizer.finish(); +} + +export function stripGitContextEchoesFromAssistantMessages( + messages: ModelMessage[], +): ModelMessage[] { + return messages.flatMap((message) => { + if (message.role !== "assistant") { + return [message]; + } + + if (typeof message.content === "string") { + const content = stripGitContextEchoes(message.content); + return content.length > 0 ? [{ ...message, content }] : []; + } + + const sanitizer = new GitContextEchoSanitizer(); + const sanitizedTextByIndex = new Map(); + let lastSanitizedPartIndex = -1; + + message.content.forEach((part, index) => { + if (part.type === "text" || part.type === "reasoning") { + sanitizedTextByIndex.set(index, sanitizer.push(part.text)); + lastSanitizedPartIndex = index; + } + }); + + if (lastSanitizedPartIndex !== -1) { + const trailingText = sanitizer.finish(); + sanitizedTextByIndex.set( + lastSanitizedPartIndex, + (sanitizedTextByIndex.get(lastSanitizedPartIndex) ?? "") + trailingText, + ); + } + + const content: typeof message.content = []; + message.content.forEach((part, index) => { + if (part.type !== "text" && part.type !== "reasoning") { + content.push(part); + return; + } + const text = sanitizedTextByIndex.get(index) ?? ""; + if (text.length > 0) { + if ( + part.type === "reasoning" && + text !== part.text && + part.providerOptions + ) { + return; + } + content.push({ ...part, text }); + } + }); + + return content.length > 0 ? [{ ...message, content } as ModelMessage] : []; + }); +} + +function findNextMarkerIndex(text: string): number { + const normalized = foldAsciiCase(text); + let earliest = -1; + for (const marker of GIT_CONTEXT_TAG_MARKERS) { + let searchFrom = 0; + while (searchFrom < normalized.length) { + const index = normalized.indexOf(marker, searchFrom); + if (index === -1) { + break; + } + if (isTagBoundary(normalized[index + marker.length])) { + if (earliest === -1 || index < earliest) { + earliest = index; + } + break; + } + searchFrom = index + 1; + } + } + return earliest; +} + +function getPotentialMarkerSuffixLength(text: string): number { + const normalized = foldAsciiCase(text); + const maxLength = Math.min( + normalized.length, + Math.max(...GIT_CONTEXT_TAG_MARKERS.map((marker) => marker.length - 1)), + ); + + for (let length = maxLength; length > 0; length -= 1) { + const suffix = normalized.slice(-length); + if (GIT_CONTEXT_TAG_MARKERS.some((marker) => marker.startsWith(suffix))) { + return length; + } + } + return 0; +} + +function startsWithDistinctivePartialMarker(text: string): boolean { + const normalized = foldAsciiCase(text); + const minimumDistinctivePrefix = "= minimumDistinctivePrefix.length && + GIT_CONTEXT_TAG_MARKERS.some( + (marker) => + normalized.length <= marker.length && marker.startsWith(normalized), + ) + ); +} + +function foldAsciiCase(text: string): string { + return text.replace(/[A-Z]/g, (character) => character.toLowerCase()); +} + +function isTagBoundary(character: string | undefined): boolean { + return ( + character === undefined || + character === ">" || + character === "/" || + /\s/.test(character) + ); +} diff --git a/src/pro/main/ipc/handlers/local_agent/local_agent_handler.test.ts b/src/pro/main/ipc/handlers/local_agent/local_agent_handler.test.ts index c52b5e9527..35e2fc5871 100644 --- a/src/pro/main/ipc/handlers/local_agent/local_agent_handler.test.ts +++ b/src/pro/main/ipc/handlers/local_agent/local_agent_handler.test.ts @@ -2382,6 +2382,64 @@ describe("handleLocalAgentStream", () => { expect(lastContentUpdate.data.content).toContain("world!"); }); + it("strips model-echoed Git context from streamed and persisted output", async () => { + const { event, getMessagesByChannel } = createFakeEvent(); + mockSettings = buildTestSettings({ enableDyadPro: true }); + mockChatData = buildTestChat(); + const echoedTag = + ''; + mockStreamResult = { + fullStream: (async function* () { + yield { type: "text-delta", text: "Finished.', + }; + })(), + response: Promise.resolve({ + messages: [ + { + role: "assistant", + content: [{ type: "text", text: `Finished.${echoedTag}` }], + }, + ], + }), + steps: Promise.resolve([]), + }; + + await handleLocalAgentStream( + event, + { chatId: 1, prompt: "test" }, + new AbortController(), + { + placeholderMessageId: 10, + systemPrompt: "You are helpful", + dyadRequestId, + }, + ); + + const sentPayloads = getMessagesByChannel("chat:response:chunk").map( + (message) => JSON.stringify(message.args[0]), + ); + expect(sentPayloads.join("\n")).not.toContain("dyad-git-context"); + expect(sentPayloads.join("\n")).toContain("Finished."); + + const contentUpdates = dbOperations.updates.filter( + (update) => update.data.content !== undefined, + ); + expect(contentUpdates.at(-1)?.data.content).toBe("Finished."); + + const aiMessagesUpdate = dbOperations.updates.find( + (update) => update.data.aiMessagesJson !== undefined, + ); + expect(JSON.stringify(aiMessagesUpdate?.data.aiMessagesJson)).toContain( + "Finished.", + ); + expect( + JSON.stringify(aiMessagesUpdate?.data.aiMessagesJson), + ).not.toContain("dyad-git-context"); + }); + it("should retry and resume when a stream terminates transiently", async () => { // Arrange const { event, getMessagesByChannel } = createFakeEvent(); @@ -2722,6 +2780,75 @@ describe("handleLocalAgentStream", () => { expect(finalContent).toContain("Here is my answer."); }); + it("sanitizes reasoning and flushes buffered text before reasoning starts", async () => { + const { event, getMessagesByChannel } = createFakeEvent(); + mockSettings = buildTestSettings({ enableDyadPro: true }); + mockChatData = buildTestChat(); + const echoedTag = + ''; + mockStreamResult = { + fullStream: (async function* () { + yield { type: "text-delta", text: "Comparison ', + }; + yield { type: "reasoning-end" }; + yield { type: "text-delta", text: "Answer" }; + })(), + response: Promise.resolve({ + messages: [ + { + role: "assistant", + content: [ + { type: "reasoning", text: `Consider.${echoedTag}` }, + { type: "text", text: "Answer" }, + ], + }, + ], + }), + steps: Promise.resolve([]), + }; + + await handleLocalAgentStream( + event, + { chatId: 1, prompt: "test" }, + new AbortController(), + { + placeholderMessageId: 10, + systemPrompt: "You are helpful", + dyadRequestId, + }, + ); + + const contentUpdates = dbOperations.updates.filter( + (update) => update.data.content !== undefined, + ); + expect(contentUpdates.at(-1)?.data.content).toBe( + "Comparison Consider.\nAnswer", + ); + expect( + getMessagesByChannel("chat:response:chunk") + .map((message) => JSON.stringify(message.args[0])) + .join("\n"), + ).not.toContain("dyad-git-context"); + + const aiMessagesUpdate = dbOperations.updates.find( + (update) => update.data.aiMessagesJson !== undefined, + ); + expect(JSON.stringify(aiMessagesUpdate?.data.aiMessagesJson)).toContain( + "Consider.", + ); + expect( + JSON.stringify(aiMessagesUpdate?.data.aiMessagesJson), + ).not.toContain("dyad-git-context"); + }); + it("should close thinking block when transitioning to text", async () => { // Arrange const { event } = createFakeEvent(); diff --git a/src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts b/src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts index 2b8bd710f9..93cf6cbb97 100644 --- a/src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts +++ b/src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts @@ -112,6 +112,10 @@ import { } from "./prepare_step_utils"; import { deleteTodos, loadTodos, saveTodos } from "./todo_persistence"; import { ensureDyadGitignored } from "@/ipc/handlers/gitignoreUtils"; +import { + GitContextEchoSanitizer, + stripGitContextEchoesFromAssistantMessages, +} from "./git_context_sanitizer"; import { TOOL_DEFINITIONS } from "./tool_definitions"; import { parseAiMessagesJson, @@ -1442,6 +1446,8 @@ export async function handleLocalAgentStream( let inThinkingBlock = false; let streamErrorFromIteration: unknown; + const gitContextTextSanitizer = new GitContextEchoSanitizer(); + const gitContextReasoningSanitizer = new GitContextEchoSanitizer(); try { for await (const part of fullStream) { @@ -1456,6 +1462,29 @@ export async function handleLocalAgentStream( let chunk = ""; let clearStreamingPreviewAfterChunk = false; + if (part.type !== "text-delta") { + const bufferedText = gitContextTextSanitizer.finish(); + if (bufferedText.length > 0) { + passProducedChatText = true; + fullResponse += bufferedText; + maybeCaptureRetryReplayText( + activeRetryReplayEvents, + bufferedText, + ); + await updateResponseInDb(placeholderMessageId, fullResponse); + sendChunk(fullResponse); + } + } + + if (part.type !== "reasoning-delta") { + const bufferedReasoning = gitContextReasoningSanitizer.finish(); + if (bufferedReasoning.length > 0) { + fullResponse += bufferedReasoning; + await updateResponseInDb(placeholderMessageId, fullResponse); + sendChunk(fullResponse); + } + } + // Handle thinking block transitions if ( inThinkingBlock && @@ -1483,12 +1512,19 @@ export async function handleLocalAgentStream( break; case "text-delta": - passProducedChatText = true; - chunk += part.text; - maybeCaptureRetryReplayText( - activeRetryReplayEvents, - part.text, - ); + { + const sanitizedText = gitContextTextSanitizer.push( + part.text, + ); + if (sanitizedText.length > 0) { + passProducedChatText = true; + chunk += sanitizedText; + maybeCaptureRetryReplayText( + activeRetryReplayEvents, + sanitizedText, + ); + } + } break; case "reasoning-start": @@ -1503,7 +1539,7 @@ export async function handleLocalAgentStream( chunk = ""; inThinkingBlock = true; } - chunk += part.text; + chunk += gitContextReasoningSanitizer.push(part.text); break; case "reasoning-end": @@ -1647,6 +1683,22 @@ export async function handleLocalAgentStream( } } + const trailingText = gitContextTextSanitizer.finish(); + if (trailingText.length > 0) { + passProducedChatText = true; + fullResponse += trailingText; + maybeCaptureRetryReplayText(activeRetryReplayEvents, trailingText); + await updateResponseInDb(placeholderMessageId, fullResponse); + sendChunk(fullResponse); + } + + const trailingReasoning = gitContextReasoningSanitizer.finish(); + if (trailingReasoning.length > 0) { + fullResponse += trailingReasoning; + await updateResponseInDb(placeholderMessageId, fullResponse); + sendChunk(fullResponse); + } + // Close thinking block if still open if (inThinkingBlock) { const closingThinkBlock = "\n"; @@ -1716,7 +1768,9 @@ export async function handleLocalAgentStream( try { const response = await streamResult.response; steps = (await streamResult.steps) ?? []; - responseMessages = response.messages; + responseMessages = stripGitContextEchoesFromAssistantMessages( + response.messages, + ); } catch (err) { if ( shouldRetryTransientStreamError({