feat(agent-core-v2): capture real file content for Edit/Write tool calls - #3267
feat(agent-core-v2): capture real file content for Edit/Write tool calls#3267liukx0205 wants to merge 3 commits into
Conversation
Edit and Write already read/write the real file when they execute; surface that before/after content instead of discarding it, so clients can render an accurate per-turn file diff instead of reconstructing one from tool-call args (which breaks once two calls in a turn touch overlapping text). Threaded through a new fileSnapshot field on ExecutableToolResult, a dedicated durable+observable FileEditSnapshot event (kept separate from ToolResultEvent since durable is a static per-event-class flag), the transcript ToolCallFrame (live projection and cold rebuild), and kap-server's own wire schema. Append-mode Write is intentionally excluded to preserve its no-read-before-append behavior. Scoped to only what web/desktop's kap-server path actually needs: packages/protocol and packages/klient are unused by kap-server (only node-sdk/acp-server depend on them), so this deliberately does not touch either of those or node-sdk's event-type surface.
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bbc744375
ℹ️ 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".
…inters
Store real before/after content in the IBlobStore (content-addressed,
deduped by sha256) instead of embedding full file text inline in
wire.jsonl. The durable file.edit_snapshot.recorded record now carries
only {key, bytes} pointers; a separate live-only file.edit_snapshot
event still carries full text for immediate client rendering, so the
event-bus's synchronous publish path never gains an async dependency.
Cold rebuild resolves the blob pointers back to text in a new
kap-server-only module, keeping packages/transcript's fold logic pure
and synchronous.
…ot-v2 # Conflicts: # packages/kap-server/src/services/transcript/transcriptService.ts
|
Closing for now — will revisit later if needed. |
1 participant
Related Issue
No tracked issue — this came out of a cross-repo investigation into why the code-app client's per-turn "files changed" diff shows phantom added-then-removed lines when a turn edits the same file with overlapping regions across multiple calls.
Problem
Edit and Write tool calls already read/write the real file content as part of executing, but that content is discarded — only a plain output string (e.g. "Replaced 1 occurrence in foo.ts") ever reaches clients. Clients are left reconstructing a diff purely from tool-call args (
old_string/new_string), which breaks once two calls in a turn touch overlapping text: a later call'sold_stringcan be a mix of a prior call's own inserted output plus untouched original content, and the client has no way to tell the two apart from args alone.What changed
FileEditService/EditTool/WriteToolnow capture the real before/after file content they already read/write, size-capped at 256 KB combined (over cap →truncated: true, no content). Append-mode Write is intentionally excluded — it never reads before appending, and an existing test pins that as deliberate.fileSnapshotfield onExecutableToolResultcarries this out of tool execution.file.edit_snapshot(FileEditSnapshot) — observable-only, carries the full before/after text, dispatched synchronously alongsideToolResultEventfor immediate live rendering. No async work on this path, so the event bus's synchronouspublish()never gets an ordering gap relative to other same-tick events.file.edit_snapshot.recorded(FileEditSnapshotRecorded) — durable-only, carries only{key, bytes}pointers intoIBlobStore(content-addressed by sha256, deduped automatically). Keeping the two events separate (rather than one durable+observable event) means the durable path can afford an async blob write without ever delaying or reordering the live path.ToolResultEvent:durable/observableare static per-event-class flags, so making the shared result event durable would make every tool's result durable (Bash, Read, Grep, …), not just edit/write; and embedding full file text inline inwire.jsonlwas the thing we were specifically trying to avoid (unbounded per-session journal growth), following the same real-content-plus-blob-store shape asplan.revision.coreEventMap.tspatchesToolCallFrame.editdirectly off the livefile.edit_snapshotevent, unchanged from a plain single-event design.services/transcript/fileEditSnapshot/resolveColdFileEditSnapshots.ts) resolvesfile.edit_snapshot.recorded's blob pointers back to real text and patchesToolCallFrame.editafterfoldWireRecordFactsruns. This keepspackages/transcript(browser-safe, no engine imports, per its own AGENTS.md) free of anyIBlobStore/async dependency —foldFacts.tsstays exactly as it was before this feature.protocol/events-zod.tsand the WS broadcaster's transcript-projection allowlist.Deliberately does not touch
packages/protocol,packages/klient, orpackages/node-sdk— verified none of those are on the kap-server/web-desktop path (onlynode-sdk/acp-serverdepend onklient, andkap-servernever importspackages/protocol), so this stays scoped to the daemon path that actually needs it and never touches the TUI/public-SDK surface.Consumers can now diff a file's real before/after content directly instead of reconstructing it from args. A matching code-app client change (separate PR) prefers this field when present, falling back to the old arg-based reconstruction for daemons that don't ship it yet.
Checklist
gen-docsskill, or this PR needs no doc update.