Repository navigation
fix(solo): keep the composer caret in the prose - #1310
Conversation
Two Solo composer caret bugs: - Picking an agent from the composer's mention menu inserted the tag at the caret, which after typing is the end of the draft (or wherever a restored caret happened to sit). In Solo chats the menu now adds the tag after the draft's leading tags and leaves the caret where it was in the prose. Inline @ completion still inserts at the caret, other conversations are unchanged, and message edits keep the caret path. - Returning to New chat focused the restored draft with the caret at 0, before its default tag. The focus request now puts the caret at the end. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 301e5d77a2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const from = input.current?.selectionStart ?? before.text.length; | ||
| const to = input.current?.selectionEnd ?? from; | ||
| if (!add({ start, end: start, query: "" })) { |
There was a problem hiding this comment.
When a Solo user picks an agent while the caret is in existing prose, this ranged insertion calls EditableInput.insertText with its default focus=true, which first moves the editor selection to the leading-tag insertion point. The later setSelectionRange restores only the live selection, not the selection bookmark stored in ProseMirror's undo history, so undoing the added tag moves the caret to the beginning of the draft; subsequent typing can then land before the prose. Preserve/map the original selection in the insertion transaction, such as by using the input's automatic-edit (focus=false) path, so undo also restores the prose caret.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Fixed in b1040c4: Solo picks now insert through the non-focusing path, so the insertion maps the caret and history keeps the pre-edit prose caret; the editor is focused afterwards. Applies to single and team picks. The "Solo composer caret" browser case now does Undo-then-type (alpha draft| → pick → Undo → caret 11 → alpha draft!); it fails on the previous head with caret 0 in WebKit.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account.
Reviewed head 301e5d77a23a2eae9fa29d837cd98408b7b2966a against base dc305c9e3bff304719cde2d06994a4c163c096db.
One P2 correctness finding; 8/10 against the repository’s review bar. The inline finding concerns Undo’s caret restoration and corroborates the existing Codex review. No additional actionable defects found in the four-file change, supported callers, admission/rejection paths or draft-focus lifecycle. Rocket, Mantis and Groot completed independent source-review lanes; Star Lord reconciled them.
The implementation remains narrowly scoped and reuses the composer/editor owners. Returning to New chat, ordinary successful picker insertion, inline completion and edit-mode scope are consistent in source. The new browser case has a valid native-selection purpose, but does not cover Undo-then-type. The public PR description and commit message were inspected; no attached images or disclosure findings.
Validation: source analysis only, including the pinned prosemirror-history 1.5.0 bookmark implementation; no PR code, tests, builds or app launches executed. One hosted-check snapshot showed WebKit 7/9 still running, Windows skipped and the other listed checks successful. Browser/native caret behavior, IME, screen-reader announcements, rendered themes/viewports/enlarged text and human acceptance remain unverified. This is a non-blocking COMMENT review, not approval.
| } | ||
| const from = input.current?.selectionStart ?? before.text.length; | ||
| const to = input.current?.selectionEnd ?? from; | ||
| if (!add({ start, end: start, query: "" })) { |
There was a problem hiding this comment.
🤖 [P2] Preserve the prose selection in Undo history
With the caret at alpha draft|, picking Honey produces @Honey alpha draft|, but Undo restores |alpha draft: the next typed characters land before the prose. This call supplies a prefix range to EditableInput.insert, whose default focusing path dispatches setRange before the document change (EditableInput.tsx:516,531). That new history event therefore saves the prefix selection. Restoring the live selection below cannot replace its saved bookmark: prosemirror-history 1.5.0 records the pre-edit selection and ignores selection-only transactions for that purpose.
Keep the original selection until the insertion transaction maps it (the adapter already supports non-focusing ranged insertion), then explicitly restore editor focus after a successful pick. Apply this to individual and team picks, and add an Undo-then-type caret regression. This independently corroborates the existing Codex finding; the scenario is established from source, not a runtime reproduction.
There was a problem hiding this comment.
🤖 Fixed in b1040c4: Solo picks now insert through the non-focusing path, so the insertion maps the caret and history keeps the pre-edit prose caret; the editor is focused afterwards. Applies to single and team picks. The "Solo composer caret" browser case now does Undo-then-type (alpha draft| → pick → Undo → caret 11 → alpha draft!); it fails on the previous head with caret 0 in WebKit.
A Solo pick moved the selection to the leading-tag insertion point before inserting, so the history event saved that point: Undo put the caret at the front of the draft and later typing landed before the prose. Insert through the non-focusing path, which maps the caret through the insertion and leaves the pre-edit caret in history, then focus the editor. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account.
Reviewed head b1040c4c597b8e3e0b45aab6e237e6f2438d5d26 against base dc305c9e3bff304719cde2d06994a4c163c096db, focusing on the repair since previously reviewed 301e5d77a23a2eae9fa29d837cd98408b7b2966a.
No actionable findings; 9/10 against the repository’s review bar. The prior Undo-caret P2 is addressed in source. Both Solo picker paths reuse the editor’s non-focusing ranged insertion, which maps the prose selection while preserving the pre-edit selection for Undo; successful picks then restore editor focus. Rejected insertions leave the draft and selection intact. Inline completion, non-Solo composition and message editing retain their existing paths. The added Undo-then-type assertion covers the reported failure and belongs at the browser layer because it exercises native selection and popup focus handoffs.
Rocket, Mantis and Groot completed independent lifecycle, correctness/security and product/accessibility source lanes; Star Lord reconciled the results. Scope was the two-file follow-up and relevant four-file feature/callers, including success, rejection, cancellation and draft-navigation focus. No new lifecycle owner or adapter is introduced. The public PR description and both commit messages were inspected; no attached images or disclosure finding.
Validation: immutable source analysis and diff checks only; no PR code, tests, builds or app launches executed. A single hosted-check snapshot showed browser journeys, browser flake checks, measurements, JavaScript and Rust checks still running; Windows was skipped, and the other listed checks were successful. Full CI, browser/native caret and IME behavior, screen-reader feedback, rendered themes/viewports/enlarged text, and human acceptance remain unverified. This is a non-blocking COMMENT review, not approval.
Fixes two Solo composer caret bugs from the Solo TODO list.
Mention menu put the tag at the end. In a Solo chat, choosing an agent from the composer's bottom-left mention menu inserted the tag at the caret. After typing, the caret is at the end of the draft, so a draft like
alpha draftbecamealpha draft@Owned Honey. In Solo chats the menu now adds the tag after the draft's leading tags (@Owned Honey alpha draft, or@Honey @Brain firstwhen a tag is already there). The caret stays where it was in the prose. A rejected insert restores the caret.Unchanged:
@completion still inserts at the caret.Returning to New chat put the caret at 0. The New chat view's focus request focused the restored draft without setting a selection, so the caret landed before the default tag. It now goes to the end of the draft. Existing chats already did this through the composer's autofocus.
Tests
tests/browser/solo-chats.spec.mjs("Solo composer caret") covers both bugs. It passes on Chromium and WebKit, and fails when either fix is reverted.MessageComposer.test.tsx, the "resolves a sole Solo chat agent" case expected a menu-picked tag at the end ofKeep going. It now expects it at the front.Human test
xyzafter the default tag, then open another Solo chat. Return to New chat: the caret should be afterxyz.@menu. The tag should appear at the start of the draft, and further typing should continue at the end.🤖 Generated with Claude Code