Attach Git provenance to user messages

[keppo-bot]

Summary

Move assistant-turn Git provenance out of assistant content and into an in-memory reminder attached to the next user message. This keeps the metadata available to the model without encouraging it to reproduce assistant-only tags, and aligns the affected E2E contracts with the new history shape.

• Use the final commit hash when the assistant turn created a commit.
• When no commit was created, identify the repository commit that preceded that assistant message.
• Leave stored message content and aiMessagesJson unchanged; the reminder exists only in reconstructed model history.
• Preserve reconstructed tool-call/tool-result transcripts without injecting metadata into assistant messages.
• Carry provenance across context-compaction boundaries and target only real user database rows.
• Remove the retired assistant-tag syntax from the Local Agent system prompt so models are no longer taught to reproduce it.
• Keep the curated Build profile aligned with the same user-message provenance format while avoiding references to Git inspection tools that profile cannot call.
• Explain that a created-commit reminder identifies the prior turn's result, while a no-commit reminder identifies its starting state and may differ from uncommitted working-tree changes.
• Treat Anthropic messages that combine tool_result blocks with queued user text as real user prompts while continuing to skip pure tool-result continuations.
• Refresh the affected request snapshots, splitting the first-turn image baseline from later in-chat requests because only the latter carry provenance.
• Wait for the deliberately slow concurrent chat response and for the plan-comment dump itself instead of relying on shorter or already-visible completion signals.
• Normalize reminder hashes in request snapshots while retaining legacy-tag normalization for older fixtures.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ca7790c14

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[dyad-assistant]

Claude review: 3 inline finding(s).

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: 🤔 NOT SURE - Potential issues
Recommendation: ready

The core direction is sound: moving Git provenance out of assistant content and into an in-memory <system-reminder> on the next user message matches the existing injectReferencedAppsReminder pattern, keeps content / aiMessagesJson untouched, and — because it rewrites an existing user message in place rather than appending a synthetic assistant message — it no longer risks disturbing tool_use / tool_result adjacency. The rules doc in rules/local-agent-tools.md was updated to match, and the tests assert non-mutation of aiMessagesJson in two places. No IPC surface, filesystem access, schema, or UI primitive is touched, so the Electron boundary, Drizzle migration, and Base UI concerns do not apply here.

The issues below are all in the new carry-forward logic in buildChatMessageHistory. None of them breaks the common path (assistant turn → user turn, where the reminder is always consumed correctly), which is why they are MEDIUM rather than HIGH — but each is a real behavioral change from the previous implementation, and none is covered by the rewritten tests.

Issues Summary

Severity	File	Issue
🟡 MEDIUM	src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts:355	Stale Git reminder can be attributed to the wrong assistant turn
🟡 MEDIUM	src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts:360	Git provenance is silently dropped when no later user message exists
🟡 MEDIUM	src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts:247	Reminder can attach to a user message nested inside another turn's transcript

🟢 Low Priority Notes (4 items)

• Dead narrowing branch - if (userMessage.role !== "user") return null; can never be true, since findIndex already required message.role === "user". It exists only to re-narrow the type after the index lookup; a typed predicate on the findIndex callback, or reusing the element found by find, would express this without an unreachable branch. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Misleading local name - withReminders (plural) holds the result of applying exactly one reminder; withReminder reads more accurately. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Escaping is now decorative - escapeXmlContent(msg.commitHash) is harmless but a no-op for hex commit hashes. Keeping it is fine as defense-in-depth; just don't read it as validation that the value is a real hash. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Reminder wording assumes adjacency - "Previous assistant message created commit: X" reads as a claim about the immediately preceding message. Compaction summaries are stored with role: "assistant", so a summary can sit between the committing turn and the user message carrying the reminder. Wording like "the previous assistant turn" would stay accurate regardless of what the reconstruction places in between. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)

Confidence notes: the diff is complete (not truncated), but I reviewed the new logic against the pre-merge state of local_agent_handler.ts on disk plus its call sites (lines 1032 and 1247) rather than a built/checked-out PR tree. I was not able to run the test suite in this environment, so the assertions above are from reading the code paths, not from execution.

---

Generated by Dyadbot persona-based code review

[keppo-bot]

🤖 Claude Code Review Summary

PR Confidence: 4/5

All trusted review threads are resolved and focused local checks pass; full CI is still running, and the Codex review job failed before review because its workflow credential could not refresh.

Unresolved Threads

No unresolved threads

Resolved Threads

Issue	Rationale	Link
Preserve provenance across compaction	Seeded the reminder from the excluded preceding assistant and added unit plus integration coverage.	View
Prevent stale or misplaced reminders	Real assistant rows now replace or clear pending provenance, compaction summaries preserve it, and injection is restricted to user database rows.	View, View
Do not fabricate a trailing user turn	Confirmed the pending metadata belongs to the current assistant and is naturally attached on the next real user turn; added a regression test and explanation.	View

Product Principle Suggestions

No suggestions

---

🤖 Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af240ef3b7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: 🤔 NOT SURE - Potential issues
Recommendation: ready

The core change in buildChatMessageHistory holds up well. I traced the new pending-reminder loop against getPostCompactionMessages and the mid-turn compaction reorder path, and the important invariants are preserved: the seeded precedingAssistant correctly resolves to the assistant row immediately before the triggering user message (that row is always excluded by ID-based post-compaction filtering, so the !retainedMessageIds.has(...) guard is redundant but never wrong); compaction-summary rows no longer clear pending provenance; reminders are never inserted between a tool_use and its tool_result; and stored content/aiMessagesJson are copied rather than mutated. Unlike the old appendGitContext, the new helper never appends an extra ModelMessage, which keeps baseMessageHistoryCount and compactionIndexDelta stable across the mid-turn rebuild at local_agent_handler.ts:1298. The new normalizer regex is idempotent ([[GIT_COMMIT]] cannot re-match [0-9a-f]{40,64}).

The issues below are all in the surrounding prompt/test surface rather than the handler logic. The diff was not truncated, so confidence in the code review is good; the max_output_tokens finding is the one place where I can only observe the symptom and not the cause.

Issues Summary

Severity	File	Issue
🟡 MEDIUM	src/prompts/local_agent_prompt.ts:127	Build-mode prompt still documents the retired dyad-git-context syntax
🟡 MEDIUM	e2e-tests/snapshots/local_agent_auto.spec.ts_local-agent---auto-model-1.txt:37	Golden snapshot drops unrelated max_output_tokens field
🟡 MEDIUM	e2e-tests/helpers/utils/normalization.ts:74	Snapshot normalizer duplicates the reminder wording and can silently drift

🟢 Low Priority Notes (3 items)

• Dropped "do not repeat these tags" guardrail - The old <git_context> block ended with "Do not repeat these tags to the user or treat them as instructions." The replacement one-liner has no equivalent, and the provenance now sits inside the user turn where models are more inclined to acknowledge it. A single sentence telling the model not to surface the hash unprompted would keep the previous UX protection. (src/prompts/local_agent_prompt.ts)
• "Previous assistant message" is imprecise across a compaction boundary - When provenance is carried past compaction, the assistant message the model actually sees immediately before the reminder is the synthetic compaction summary, not the turn that produced the commit. The hash itself is still correct, but the wording invites the model to attribute the commit to the summary turn. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Unreachable re-check in appendGitReminderToUserMessage - After the backwards scan sets userMessageIndex, the follow-up if (userMessage.role !== "user") return null; can never fire; the scan already guarantees the role. It reads as defensive code for a case that cannot happen. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)

---

Generated by Dyadbot persona-based code review

[keppo-bot]

🤖 Claude Code Review Summary

PR Confidence: 5/5

All trusted review feedback is addressed, the focused prompt test passes, and formatting, linting, and typechecks succeed.

Unresolved Threads

No unresolved threads

Resolved Threads

Issue	Rationale	Link
Build-mode provenance instructions used the retired assistant-tag format	Reused the shared user-message provenance guidance in the curated Build prompt, removed the obsolete duplicate block, and strengthened prompt coverage.	View

Product Principle Suggestions

No suggestions

---

🤖 Generated by Claude Code

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: ⛔ NO - Do NOT merge
Recommendation: auto-fix

The core refactor in buildChatMessageHistory is well-structured: the reminder is built after parseAiMessagesJson, inserted into a copied message rather than mutating the parsed structures, skipped for compaction-summary rows, and covered by a genuinely thorough set of new unit tests (compaction boundary, nested legacy user rows, transcripts ending in a tool result, aiMessagesJson immutability). No IPC surface, filesystem access, database schema, or UI primitive is touched, and the rules/local-agent-tools.md guidance matches the implementation.

The blocking problem is prompt-side, not in the history builder: collapsing BUILD_GIT_CONTEXT_BLOCK into the shared GIT_CONTEXT_BLOCK silently gives the curated Build profile an instruction to use Git inspection tools that the Build tool profile does not register, and the test that guarded exactly that distinction was replaced in the same commit.

The diff was not truncated; all 10 changed files were reviewed.

Issues Summary

Severity	File	Issue
🔴 HIGH	src/prompts/local_agent_prompt.ts:590	Build profile prompt now points at Git inspection tools it does not have
🟡 MEDIUM	src/prompts/local_agent_prompt.test.ts:590	Build prompt regression guard for Git inspection tools was removed
🟡 MEDIUM	src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts:371	Post-compaction reminder references an assistant message the model cannot see
🟡 MEDIUM	e2e-tests/snapshots/local_agent_auto.spec.ts_local-agent---auto-model-1.txt:37	Unrelated max_output_tokens removal in the e2e request snapshot

🟢 Low Priority Notes (4 items)

• Dead narrowing branch - appendGitReminderToUserMessage re-checks userMessage.role !== "user" after the backward scan has already established it; the branch is unreachable and only exists for TypeScript narrowing. A find-style helper or an index-typed local would read more cleanly. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Pending reminder can leak forward - when appendGitReminderToUserMessage returns null (a user database row whose parsed messages contain no usable user message), pendingReminder stays set and will attach to whatever user row comes next, which may be several turns later. That path is not covered by the new tests. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Seeding block density - the retainedMessageIds / firstRetainedUserId / precedingAssistant computation is a fairly intricate block inlined into an already long buildChatMessageHistory. Extracting it as a named helper (e.g. findAssistantExcludedByCompaction) would make the compaction-boundary intent self-evident. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Provenance dropped on trailing assistant turns - when the history ends on an assistant row, its commit hash is discarded entirely rather than annotated, which is a deliberate tradeoff of the new format and is asserted by a test. Worth confirming no consumer relied on the old always-attached annotation. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)

---

Generated by Dyadbot persona-based code review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7c9592b19

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[dyad-assistant]

Claude review: 2 inline finding(s).

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: 🤔 NOT SURE - Potential issues
Recommendation: ready

The core change is sound. buildChatMessageHistory now carries assistant-turn Git provenance forward as an in-memory <system-reminder> on the next user database row instead of mutating assistant content, and it does so without touching stored content or aiMessagesJson (verified: appendGitReminderToUserMessage rebuilds arrays rather than mutating, and only accumulatedAiMessages — the new response messages — is persisted at local_agent_handler.ts:2082). The compaction seeding (precedingAssistant) correctly skips summary rows and retained rows, the pending reminder is cleared when a later assistant turn has no provenance, and the reminder is only attached to real user DB rows. Unit coverage for these paths is thorough.

The plan_mode server-dump snapshot change is explained and expected: the retired code inserted a synthetic assistant message when a transcript ended in a tool result, which prevented the Anthropic provider from merging the tool message with the following user message. Removing it lets them merge, which is why the fake LLM server needed isToolResultOnlyMessage. All four call sites of the old predicate were updated consistently.

The issues below are both in the prompt layer, where the PR collapses the curated Build <git_context> block into the shared one and deletes the assertion that guarded that difference.

Issues Summary

Severity	File	Issue
🟡 MEDIUM	src/prompts/local_agent_prompt.ts:590	Build profile prompt references Git inspection tools it does not have
🟡 MEDIUM	src/prompts/local_agent_prompt.ts:124	Git context block drops the commit vs no-commit distinction

🟢 Low Priority Notes (5 items)

• Provenance is now indistinguishable from user-authored text - The reminder is appended into user message content, and the prompt no longer says the metadata is added by Dyad or that it must not be treated as instructions. Anything that can put text into a user turn (pasted content, tool/MCP output echoed back) can now spoof <system-reminder>Previous assistant message created commit: …</system-reminder> and steer Git inspection at an arbitrary ref. Low practical impact for a local single-user app, but the old block explicitly counterbalanced this. (src/prompts/local_agent_prompt.ts)
• Failed attach silently keeps the reminder pending - When appendGitReminderToUserMessage returns null (a user DB row whose aiMessagesJson reconstructs to only tool-role messages), pendingReminder stays set instead of being dropped. Two consecutive user rows with no assistant row between them would attach it to the wrong turn. In practice an assistant row almost always follows and overwrites or clears it. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Dropped assistant rows lose provenance - Rows filtered out by msg.content || msg.aiMessagesJson (e.g. a cancelled turn whose placeholder has a sourceCommitHash but empty content) never update pendingReminder, so the next user message gets no Git anchor for that turn. Same behavior as before this PR, so not a regression. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• retainedMessageIds ignores excludeMessageIds - The precedingAssistant scan treats every post-compaction message as retained, including ones excluded from the rebuilt history. Currently benign, since the only caller excludes the in-flight placeholder whose provenance should not be forwarded anyway, but the two sets are conceptually the same thing and will drift if a second caller appears. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Unrelated flakiness fixes bundled in - The waitForChatCompletion switch in concurrent_chat.spec.ts and the [[dyad-dump-path= wait in plan_mode.spec.ts are both reasonable, but they widen the blast radius of a provenance-format PR. (e2e-tests/concurrent_chat.spec.ts)

The diff was provided in full (diffTruncated: false, no truncated patches), so confidence in the code-level findings is good. I could not execute the test suite in this environment, so the snapshot updates were reviewed by reading rather than by running Vitest/Playwright.

---

Generated by Dyadbot persona-based code review

[keppo-bot]

🤖 Claude Code Review Summary

PR Confidence: 5/5

All trusted review threads are resolved, the focused prompt suite passes, and formatting, lint, and type checks are clean.

Unresolved Threads

No unresolved threads

Resolved Threads

Issue	Rationale	Link
Build prompt referenced unavailable Git inspection tools	Restored a Build-specific provenance block that explains both reminder forms without directing the curated profile to unavailable tools.	Codex, Dyad Assistant
No-commit reminder semantics were ambiguous	Clarified that a created-commit hash represents the prior turn's result, while a no-commit hash represents its starting state and may differ from uncommitted working-tree changes.	View
Local Agent request snapshots still used the retired assistant tag	Regenerated every affected request baseline in the prior E2E fix commit and verified the affected E2E specs without update mode.	View

Product Principle Suggestions

No suggestions; the review findings were concrete capability and correctness issues rather than ambiguous product decisions.

---

🤖 Generated by Claude Code

[dyad-assistant]

🔍 Dyadbot Code Review Summary

Verdict: ⛔ NO - Do NOT merge
Recommendation: auto-fix

The core refactor in buildChatMessageHistory is sound: provenance is derived from DB metadata only, aiMessagesJson is never mutated (asserted by the new tests), compaction summaries neither consume nor clear the pending reminder, reminders are only attached to real user DB rows, and no synthetic user message is invented when history ends on an assistant turn. Unit and integration snapshots line up with the new format.

The blocker is the local-agent - auto model E2E baseline: its <git_context> block was replaced with a condensed one-line paraphrase that does not match the GIT_CONTEXT_BLOCK this PR actually ships, and the same baseline silently drops "max_output_tokens": 32000. Both look like a hand-edited (or stale-build) baseline rather than a regenerated one, and both will fail the dump comparison. The unit snapshot src/prompts/__snapshots__/local_agent_prompt.test.ts.snap has the correct bulleted text in all ten places, which confirms the E2E baseline is the odd one out.

The diff was provided in full (no truncation), so confidence in the code findings is high; the E2E finding is based on comparing the new baseline against the new prompt source rather than on an actual test run.

Issues Summary

Severity	File	Issue
🔴 HIGH	e2e-tests/snapshots/local_agent_auto.spec.ts_local-agent---auto-model-1.txt:7	Auto-model baseline git_context text does not match the new prompt
🟡 MEDIUM	e2e-tests/snapshots/local_agent_auto.spec.ts_local-agent---auto-model-1.txt:37	Auto-model baseline drops max_output_tokens with no source change
🟡 MEDIUM	src/prompts/local_agent_prompt.ts:128	Removed guard against echoing or obeying provenance reminders

🟢 Low Priority Notes (4 items)

• Compaction-seeded reminder describes an invisible turn - After compaction the first retained user message gets "Previous assistant message created …" seeded from an assistant row that was dropped from model-visible history, so "previous assistant message" refers to something the model cannot see. Harmless today, but worth a clarifying wording if it ever confuses a model. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• Duplicated "append a system-reminder to the last user message" logic - appendGitReminderToUserMessage and the existing injectReferencedAppsReminder now implement the same backwards scan and string/array content append with different return conventions (new array vs. in-place index replacement). A shared helper would keep the two reminder injectors from drifting. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)
• isToolResultOnlyMessage can be flipped by the injected reminder - The fake server now classifies any tool_result message that carries text as a real user prompt. If a persisted user row ever parses to tool-result-only content, appending the Git reminder gives it text and the message stops qualifying for findOriginalLocalAgentFixture, falling back to the canned response. Reachability is unclear from the diff; a guard that ignores <system-reminder> text when deciding "is this a real prompt" would remove the doubt. (testing/fake-llm-server/anthropicMessagesHandler.ts)
• Redundant role re-check - appendGitReminderToUserMessage re-tests userMessage.role !== "user" immediately after the loop already matched on that role; it only exists to satisfy narrowing and reads as dead defensive code. (src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts)

---

Generated by Dyadbot persona-based code review

[github-actions]

🎭 Playwright Test Results

❌ Some tests failed

OS	Passed	Failed	Flaky	Skipped
🍎 macOS	408	1	5	151
🪟 Windows	405	22	2	151

Summary: 813 passed, 23 failed, 7 flaky, 302 skipped

Failed Tests

🍎 macOS

• local_agent_auto.spec.ts > local-agent - auto model
  • Error: expect(string).toMatchSnapshot(expected) failed

🪟 Windows

Show all 22 failures

• app_search.spec.ts > app search - basic search dialog functionality
  • Error: expect(locator).toBeVisible() failed
• app_search.spec.ts > app search - search functionality with different terms
  • Error: expect(locator).toBeVisible() failed
• app_search.spec.ts > app search - navigation and selection
  • Error: expect(locator).toBeVisible() failed
• app_search.spec.ts > app search - empty search shows all apps
  • Error: expect(locator).toBeVisible() failed
• app_search.spec.ts > app search - search by app name
  • Error: expect(locator).toBeVisible() failed
• attach_image.spec.ts > attach image - chat
  • Error: expect(locator).toBeVisible() failed
• attach_image.spec.ts > attach image via drag - chat
  • Error: expect(locator).toBeVisible() failed
• chat_history.spec.ts > should open, navigate, and select from history menu
  • Error: expect(locator).toBeVisible() failed
• chat_history.spec.ts > should handle edge cases: guards, escape, and sending after cancel
  • Error: expect(locator).toBeVisible() failed
• concurrent_chat.spec.ts > concurrent chat
  • Error: expect(locator).toBeVisible() failed
• edit_code.spec.ts > edit code
  • Error: expect(locator).toBeHidden() failed
• edit_code.spec.ts > edit code edits the right file during rapid switches
  • Error: expect(locator).toBeHidden() failed
• editor_commit_menu.spec.ts > editor commit menu commits multiple staged files at once
  • Error: expect(locator).toBeHidden() failed
• fix_error.spec.ts > fix all errors button
  • Error: expect(locator).toBeVisible() failed
• home_chat_existing_app.spec.ts > home chat - start new chat in existing app
  • Error: expect(locator).toBeVisible() failed
• mention_app.spec.ts > mention app (without pro)
  • Error: expect(locator).toBeVisible() failed
• mention_app.spec.ts > mention app (with pro)
  • Error: expect(locator).toBeVisible() failed
• mention_files.spec.ts > mention file
  • Error: expect(locator).toBeVisible() failed
• per_chat_input.spec.ts > chat input is preserved when switching between chats
  • Error: expect(locator).toBeVisible() failed
• per_chat_input.spec.ts > closing a chat tab clears its stored input
  • Error: expect(locator).toBeVisible() failed
• per_chat_input.spec.ts > input preserved when switching back and forth multiple times
  • Error: expect(locator).toBeVisible() failed
• queued_message.spec.ts > queued messages > fires queued message while on another page
  • Error: expect(locator).not.toBeVisible() failed

📋 Re-run Failing Tests (macOS)

Copy and paste to re-run all failing spec files locally:

npm run e2e \
  e2e-tests/local_agent_auto.spec.ts

⚠️ Flaky Tests

🍎 macOS

• chat_completion_notifications.spec.ts > notification click navigates to the chat (passed after 1 retry)
• engine.spec.ts > send message to engine (passed after 1 retry)
• mcp.spec.ts > mcp - call calculator (passed after 1 retry)
• mention_files.spec.ts > mention file (passed after 1 retry)
• refresh.spec.ts > reload shortcuts stay scoped to the focused preview (passed after 1 retry)

🪟 Windows

• app_screenshot.spec.ts > captures an app screenshot after the first generated commit (passed after 1 retry)
• screenshot_prompt.spec.ts > upload a chat session and report it with a screenshot (passed after 1 retry)

---

📊 View full report