Release: fault-tolerant codemap locator jumps#245
Merged
Merged
Conversation
An ACP `session/request_permission` carries a `ToolCallUpdate` — a DELTA,
not a snapshot. codex leans on this hard: its file-edit approvals arrive as
a bare `{ toolCallId, kind: "edit", status: "pending" }`, with `reason: null`
in `_meta` for an in-workspace edit. The title and the diff were announced
earlier, in the `session/update` that opened the same tool call.
The connector kept no such snapshot, so it built the card from the delta
alone and produced `title/raw_input/locations = null` plus the body
"ACP agent requested permission to continue." — the approver was asked to
allow a write without being told which file. Command approvals were fine
(they populate `rawInput.{command,cwd}`), which is why this only showed up
"sometimes".
Remember each `tool_call` / `tool_call_update` on the run (capped, so a long
turn can't grow it without bound; scoped to the run, so it dies with the
turn) and fold it back in when the permission request lands. Fields on the
request win — it is the newer of the two. Paths are derived into ACP's
`locations`, and the body falls back to them, so an approval always names
its files.
The diff bodies are deliberately NOT forwarded: codex sends the full old+new
text of every changed file, which would bloat the frame and its trace row.
Payloads in the tests are verbatim from a real codex-acp 1.1.2 run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… fallback Field report: a bot wrote loc: cheers:ws/@backend/… — a handle it guessed (the dispatch prompt never tells a bot its own mention name; its only reliable source is the name people @-mention it by). Three layers: - map.yaml header guide: <your-handle> = the EXACT name this channel mentions you by; never guess, never copy from an example; omit loc rather than invent one (plugin INIT_SKELETON + template seed regenerated byte-identical). - resolver fallback: an unmatched @handle in a single-bot channel resolves to that bot — the map is channel-local so the intent is unambiguous, and the existence probe still gates the jump. Existing wrong-handle maps become navigable immediately. - the no-match error now lists the channel's actual bot names, so a human can tell the bot what to fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ections Field report: a bot-written loc (server/src/api/workspace.rs) hit E_NOT_FOUND because the bot's browse root and the repo root disagree — the class of uncertainty every bot-written path carries. The ws jump now mirrors the chat ref-jump philosophy (probe layer by layer, only commit to a jump the server confirms): - exact path first; - strip a redundant leading repo dir (root IS the repo); - prefix each top-level dir of the browse root (root is the repo's PARENT), one tree listing + <=12 probes, never a workspace scan; - only when all layers miss: a clear error telling the human to have the bot refresh its map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n-7f752f fix(codemap): fault-tolerant locator jumps — handle + path uncertainty from bot-written maps
Follow-up to the card-detail fix: naming the files is better than nothing, but an approver should see what the write actually does. codex hands us the full old AND new text of every changed file, not a patch — measured against real codex, a one-line edit to a 49KB file arrives as a 101KB payload (2.1x the file). Forwarding that verbatim would put megabytes through every approval frame and the trace row it is persisted into, which is why it was left out of the first pass. So distil it where the text already is: diff old vs new in the connector and forward only the result. The same 98KB of text becomes a 383-byte patch — a 256x reduction — in `git diff` format, which the card's existing `DiffView` already renders, so no new rendering code. Capped at 16KB with a visible marker for the case that is genuinely large (a new or rewritten file diffs to its whole contents); truncation lands on a line boundary. The card labels the block "Changes" rather than "Command" for an edit, and renders the patch inline — it needs no fetch, and on an edit approval it IS the thing to review. DiffView.test.ts guards the seam with verbatim connector output, including the `a//abs/path` prefix that git's `a/` convention produces for an absolute path (the parser strips it back to the real path). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Checked the fix against the protocol and the other two ACP agents.
The spec backs it: `RequestPermissionRequest.tool_call` is a `ToolCallUpdate`
("All fields except the tool call ID are optional"), and the docs' own example
of a permission request is a bare `{"toolCallId": "call_001"}`. Tracking the
tool call is therefore the client's job, not an agent oversight — codex is
within spec, our client was the one assuming a snapshot.
It is also the canonical client behaviour: Zed upserts the request's
ToolCallUpdate into the tool call it already holds
(`request_tool_call_authorization` -> `upsert_tool_call_inner` ->
`update_fields`), and the SDK ships the same merge as `ToolCall::update`
(only `Some` fields overwrite) — which is what `merge_tool_call_snapshot`
does, just from the other side.
Agents lean on it to wildly different degrees, so pin the other two with
tests taken from their real payload builders:
- opencode 1.18.3 sends a self-contained request (title/kind/locations AND
the diff `content`), so it needs no snapshot at all — and since it uses the
same canonical `content` diff blocks, the patch we now render works for it
for free.
- claude-code-acp 0.16.2 sends `{ toolCallId, title, rawInput }`; no ACP diff
content, so no synthesized patch — it keeps rendering from rawInput exactly
as before.
No behaviour change; comments and cross-agent regression tests only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix(acp): keep tool-call detail on edit approval cards, and show the patch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
develop → main: absorb bot-written locator uncertainty (see #244) —
🤖 Generated with Claude Code