fix(chat): steer a draft with image attachments instead of refusing it - #640
Merged
xintaofei merged 5 commits intoSep 4, 2026
Merged
Conversation
The composer's "insert into current turn" entry went dark the moment an image was staged, and a draft with file badges was silently rerouted to the queue. The whole steering chain narrowed the draft to a bare string (ConnectionCommand::Steer carried text, build_steer_params hardcoded a single text block), so those gates were the honest option. The wire never had that limit: the claude adapter's _session/steering handler feeds the prompt array through the same conversion as session/prompt, images included. Steer now carries the draft's PromptInputBlocks end to end. The composer sends the full block list whenever the draft holds more than plain text, with the display text as the recorded note; the backend maps the blocks with map_prompt_blocks, the exact session/prompt encoding, and re-hydrates uploaded file:// markers like /acp_prompt does, so web and remote mode work unchanged. The prompt ledger fingerprints the steered blocks the same way a prompt's are, and a text-only steer stays byte-identical to before. The pull channel still delivers plain text, so a blocks-bearing note on a session that downgraded mid-race is rejected with NoActiveTurn and the composer's existing fallback queues the whole draft, attachments included; an attachment is never silently dropped. Steering while an upload is still settling gets the same toast a plain send does, and the now-unused steerAttachmentsUnsupported string is gone from all ten locales.
`onSteer` widened to `(text, blocks?)` in `MessageInput` and in the panel's handler, but `ConversationShell` and `ChatInput` — the two layers the prop actually travels through — still declared the one-argument form. The optional second parameter keeps that assignable, so tsc reports nothing and the leak would only appear the day either layer wraps the callback instead of forwarding it. Also lock down the two claims the change rests on: the text-only steer's wire shape is now asserted by exact equality rather than field probes, and the enqueue fallback is asserted to carry the image block, not just the prose.
Attachment hydration is the one await `submit_feedback_native` puts between the `turn_in_flight` admission check and the enqueue, and it runs for as long as reading the uploads takes. The loop's idle arm covers "the turn ended" — it replies `NoActiveTurn` and the composer queues the whole draft — but it cannot cover "the next turn started in the meantime": the flag reads true either way, so the loop is in its active arm and injects the note into a turn the user never aimed at, recorded `Delivered` while the composer clears. `turn_in_flight` says only that some turn is running, never which one, so give the state a turn identity: `SessionState.turns_completed`, bumped next to the `turn_in_flight` clear in the `TurnComplete` handler — the single production site that ends a turn. A steer captures it during admission and re-checks it after hydration; a change means the admitted turn is over and the note takes the caller's queue fallback, which re-routes the whole draft, attachment included. A counter rather than the existing `pending_user_message_started_at`: that stamp only exists once a turn has published a user message, and `user_message` is `None` for delegation children and unbound conversations, so those turns would have carried no identity at all. The counter is monotonic rather than an exact turn count — `TurnComplete` has three emitters and a repeat can land on a settled turn — and only inequality is ever read.
main's 830ea83 dropped fork-and-send, taking the `onForkSend` prop and the composer's send split button with it — the same declarations this branch edits to widen `onSteer` with the steered draft's blocks. Three files conflicted: message-input.tsx, chat-input.tsx and conversation-shell.tsx, all on that adjacent prop block. Each resolves the same way: keep main's removal of fork-and-send, keep the widened `onSteer(text, blocks?)`. Nothing else needed hand-resolving; the steering chain, the i18n removals and the backend guard all merged clean, and no fork-send reference survives.
`830ea832` removed fork-and-send and `8eb05f8e` swept its comments, but the steering handler's note still contrasted itself with "the synchronous send/enqueue/fork paths". There is no fork path in the composer any more.
Owner
|
codeg work task |
xintaofei
added a commit
that referenced
this pull request
Sep 5, 2026
main's #640 rewrote the same mid-turn send this branch widens, so both sides of every steer hunk are kept: * `onSteer` takes main's `(text, blocks?)` across all three composer layers, alongside this branch's `steerChannel`. `onForkSend` stays deleted. * `handleSteerClick` keeps main's body — the uploading gate and the block payload — and this branch's channel-keyed label, icon and failure toast. A pull session with an attachment now reaches the backend's blocks rejection as ordinary operation, and lands on the same queue fallback. * `steerAttachmentsUnsupported` goes with main's deletion (attachments are supported now); `steerNoteFailed` stays in all ten locales. * Both test suites keep every case from both sides. `submit_feedback`'s comment on the pull-path blocks rejection said it only arises after a native downgrade; with the affordance on every session it is now the normal path for a pull session, so it says both.
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
2 participants
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.
With the Claude Code agent, "Insert into current turn" refused any draft that had an image staged: the menu entry disabled itself with a text-only tooltip, and a draft holding file badges was quietly rerouted to the queue. The limit was on our side, not the wire's. The steering chain narrowed everything to a bare string (
ConnectionCommand::Steer { text },build_steer_paramshardcoding a single text block), while claude-agent-acp's_session/steeringhandler feeds the prompt array through the exact same conversion assession/prompt, image blocks included.Steer now carries the draft's
PromptInputBlocks end to end. The composer sends the full block list whenever the draft holds more than plain text, with the display text as the recorded note; the backend maps it withmap_prompt_blocks(thesession/promptencoding) and re-hydrates uploadedfile://markers exactly like/acp_prompt, so web and remote mode work unchanged. The prompt ledger fingerprints the steered blocks the way a prompt's are, and a text-only steer stays byte-identical to before.Attachments are still never silently dropped: the pull tool carries plain text only, so a blocks-bearing note that races a mid-session channel downgrade is rejected with
NoActiveTurnand the composer's existing fallback queues the whole draft, image included, with the usual toast. Steering while an upload is still settling gets the same "still uploading" toast a plain send does, and the now-unusedsteerAttachmentsUnsupportedstring is removed from all ten locales.