Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rules/local-agent-tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<dyad-git-context>` 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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
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.<dyad-git-con"),
sanitizer.push('text commit="fake">'),
sanitizer.push("</dyad-git-context>"),
sanitizer.finish(),
].join("");

expect(output).toBe("Done.");
});

it("preserves ordinary text around internal tag markup", () => {
expect(
stripGitContextEchoes(
'Before <dyad-git-context commit="fake">unexpected</dyad-git-context> after',
),
).toBe("Before unexpected after");
});

it("does not strip similarly named user text", () => {
expect(stripGitContextEchoes("Use <dyad-git-contextual> here.")).toBe(
"Use <dyad-git-contextual> here.",
);
});

it("drops a distinctive internal tag fragment if the stream ends", () => {
const sanitizer = new GitContextEchoSanitizer();

expect(sanitizer.push("Done.<dyad-git-con")).toBe("Done.");
expect(sanitizer.finish()).toBe("");
});

it("removes echoes from assistant message text only", () => {
const tag = '<dyad-git-context commit="fake"></dyad-git-context>';
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" },
],
});
});
});
147 changes: 147 additions & 0 deletions src/pro/main/ipc/handlers/local_agent/git_context_sanitizer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import type { ModelMessage } from "ai";

const GIT_CONTEXT_TAG_MARKERS = [
"<dyad-git-context",
"</dyad-git-context",
] as const;

/**
* Removes internal Git-context tag markup without exposing partial tags while
* model text is streaming. Text between an opening and closing tag is
* preserved; only the internal markup itself is metadata.
*/
export class GitContextEchoSanitizer {
private pending = "";

push(chunk: string): string {
this.pending += chunk;
let output = "";

while (this.pending.length > 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) {
Comment thread
keppo-bot[bot] marked this conversation as resolved.
break;
}
this.pending = this.pending.slice(tagEndIndex + 1);
}

return output;
}

finish(): string {
const pending = this.pending;
this.pending = "";
return startsWithMarker(pending) ||
startsWithDistinctivePartialMarker(pending)
? ""
: pending;
Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
}
}

export function stripGitContextEchoes(text: string): string {
const sanitizer = new GitContextEchoSanitizer();
return sanitizer.push(text) + sanitizer.finish();
}

export function stripGitContextEchoesFromAssistantMessages(
messages: ModelMessage[],
): ModelMessage[] {
return messages.map((message) => {
if (message.role !== "assistant") {
return message;
}

if (typeof message.content === "string") {
return {
...message,
content: stripGitContextEchoes(message.content),
};
}

Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
const content = message.content.map((part) => {
if (part.type !== "text") {
Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
return part;
}
const text = stripGitContextEchoes(part.text);
return { ...part, text };
Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
});

return { ...message, content };
});
Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
}

function findNextMarkerIndex(text: string): number {
const normalized = text.toLowerCase();
Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
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 = text.toLowerCase();
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 startsWithMarker(text: string): boolean {
const normalized = text.toLowerCase();
return GIT_CONTEXT_TAG_MARKERS.some(
(marker) =>
normalized.startsWith(marker) && isTagBoundary(normalized[marker.length]),
);
}

function startsWithDistinctivePartialMarker(text: string): boolean {
const normalized = text.toLowerCase();
const minimumDistinctivePrefix = "<dyad-git-";
return (
normalized.length >= minimumDistinctivePrefix.length &&
GIT_CONTEXT_TAG_MARKERS.some((marker) => marker.startsWith(normalized))
);
}

function isTagBoundary(character: string | undefined): boolean {
return (
character === undefined ||
character === ">" ||
character === "/" ||
/\s/.test(character)
Comment thread
keppo-bot[bot] marked this conversation as resolved.
);
}
58 changes: 58 additions & 0 deletions src/pro/main/ipc/handlers/local_agent/local_agent_handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
'<dyad-git-context commit="hallucinated"></dyad-git-context>';
mockStreamResult = {
fullStream: (async function* () {
yield { type: "text-delta", text: "Finished.<dyad-git-con" };
yield {
type: "text-delta",
text: 'text commit="hallucinated"></dyad-git-context>',
};
})(),
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();
Expand Down
37 changes: 30 additions & 7 deletions src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1442,6 +1446,7 @@ export async function handleLocalAgentStream(

let inThinkingBlock = false;
let streamErrorFromIteration: unknown;
const gitContextEchoSanitizer = new GitContextEchoSanitizer();

try {
for await (const part of fullStream) {
Expand Down Expand Up @@ -1483,12 +1488,19 @@ export async function handleLocalAgentStream(
break;

case "text-delta":
passProducedChatText = true;
chunk += part.text;
maybeCaptureRetryReplayText(
activeRetryReplayEvents,
part.text,
);
{
const sanitizedText = gitContextEchoSanitizer.push(
Comment thread
keppo-bot[bot] marked this conversation as resolved.
Outdated
part.text,
);
if (sanitizedText.length > 0) {
passProducedChatText = true;
chunk += sanitizedText;
maybeCaptureRetryReplayText(
activeRetryReplayEvents,
sanitizedText,
);
}
}
break;

case "reasoning-start":
Expand Down Expand Up @@ -1647,6 +1659,15 @@ export async function handleLocalAgentStream(
}
}

const trailingText = gitContextEchoSanitizer.finish();
if (trailingText.length > 0) {
passProducedChatText = true;
fullResponse += trailingText;
maybeCaptureRetryReplayText(activeRetryReplayEvents, trailingText);
await updateResponseInDb(placeholderMessageId, fullResponse);
sendChunk(fullResponse);
}

// Close thinking block if still open
if (inThinkingBlock) {
const closingThinkBlock = "</think>\n";
Expand Down Expand Up @@ -1716,7 +1737,9 @@ export async function handleLocalAgentStream(
try {
const response = await streamResult.response;
steps = (await streamResult.steps) ?? [];
responseMessages = response.messages;
responseMessages = stripGitContextEchoesFromAssistantMessages(
Comment thread
keppo-bot[bot] marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM

Sanitizing before mid-turn compaction slicing can misalign the offset

responseMessages is now the sanitized array, but the mid-turn compaction path slices it using prevStepMessages.length taken from the unsanitized steps[...].response.messages. stripGitContextEchoesFromAssistantMessages can remove whole messages (an assistant message whose only content was an echoed tag returns an empty array), so the two lengths can disagree and responseMessages.slice(prevStepMessages.length) then drops real post-compaction messages or keeps pre-compaction ones. That silently corrupts the persisted transcript and the history replayed for the rest of the turn.

💡 Suggestion: Compute the slice from the raw response.messages and sanitize afterwards (sanitize messagesToAccumulate), so the offset and the array being sliced come from the same source.

response.messages,
);
} catch (err) {
if (
shouldRetryTransientStreamError({
Expand Down
Loading