Repository navigation
Conversation
The coordinator delivered shortcut captures to the thread draft while the composer staged question attachments in a per-question draft, so a capture taken during a question stayed hidden until the answer was sent.
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a localized, tested bug fix that routes snapshot captures into the existing active-question attachment flow while preserving normal snapshot behavior elsewhere. Two unresolved High findings concern capture/question timing races and independently block approval until addressed, although the submitted head includes fixes and regression coverage for those cases. Notes:
You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
…into-open-question
|
Merged Rerun on the merged branch: |
Dismissing prior approval to re-evaluate a614327
…uestion's commit A capture the drain found was pinned only once it was read, so a question opening meanwhile could claim it, and the composer registered the open question in a passive effect, a frame after the question's UI changed.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Problem
While a provider question is open, the composer stages dropped files in a per-question draft, so drag-and-drop lands beside the answer. The SnapShot coordinator still delivered shortcut captures to the thread draft, and the composer keyed the flying-capture animation to that same thread draft. A capture taken during a question therefore landed on the hidden normal composer and only appeared after the answer was sent.
Change
ChatComposerregisters the open question's attachment draft, together with the other question drafts of the same request, while the question can take attachments (the attach button's gate) and its answer is not yet sending. The registration is a layout effect, so it changes in the same commit as the question's UI.SnapShotCoordinatorpins that question to each capture the first time it sees it (at the shortcut's request, or before the read for a capture a drain finds), so the animation and the delivery agree. A capture pinned to a question that has since closed goes to the thread draft, never to a newer question.addImages, becauseaddImagerefuses drafts without a thread session, which a question draft never has, and refuses a capture once the request's questions already hold the shared attachment limit, as drag-and-drop does.Web and desktop share this code. Mobile has no SnapShot path and the browser build has no desktop bridge, so nothing else changes. No contract or server change.
Scope and approval
Closes #12271, accepted by a maintainer with this fix direction: #12271 (comment)
#12293 by @Gigioxx implemented the same routing and was closed only for a missing desktop recording (#12293 (comment)). This PR rebuilds that approach on current
main, keeps its answers to the review findings, adds the ownership recheck before the insert that its final review left open, and supplies the recording.Verification
vp test run --project unit src/components/desktop/SnapShotCoordinator.test.tsinapps/web: 20 passed. Five new cases: a capture during an open question lands in the question draft and leaves the thread draft empty; a capture stays on the question it was pinned to and never moves to a newer one; a capture whose question closes while it is being read falls back to the thread draft; a capture pinned before any question stays off a question that opens while it is being read; a capture is refused once the request's other questions hold the shared limit. With the routing reverted the first three fail, with the recheck moved back before the read the third fails, and without the limit check the fourth fails.vp lintandvp fmt --checkon the four touched files, andvp run --filter @t3tools/web typecheck: clean. The only lint warnings are the ones already onmain.upstream/mainat70d60e03a7was merged in. The conflicts were against perf(web): ChatComposer compiles with no manual memoization #20397 (the composer's image grouping moved intouseComposerImageGroups, which now receives the attachment draft target) and perf(web): snapshot coordinator keeps the last route target in state #20466 (the coordinator keeps its route target in state, so this PR's question pins are the only ref it adds). The tests (25 passed across this file andquestionAttachments.test.ts), lint (no warnings), format and the web typecheck were rerun on the merged branch. The dev desktop app run below is from 2026-10-04 and was not retaken.vp run dev:desktop) on a fresh state with a throwaway project, a Claude thread with anAskUserQuestioncard open, SnapShots on with the default both-Shift shortcut, the dev app in front so it captured itself:mainhid the composer's image strip while a question was open (a regression from the V2 merge). The screenshots and the recording below were taken with fix(web): attachments on an open question are visible again #15537's one-line change applied locally, so the tile is visible. fix(web): attachments on an open question are visible again #15537 merged on 2026-10-10, somainnow shows the strip without it.Before: the shortcut was pressed while the question was open, and nothing landed in the question (its strip holds only the image dropped earlier):
Before, after Submit: the capture surfaces in the normal composer, which is the report's symptom:
After: the capture sits in the question's strip:
Recording of the shortcut capture landing in the open question and the answer being submitted with it:
12271-snapshot-into-open-question.mp4
After, Claude's reply to that answer, describing the captured window:
Claude Fable 5.1 via T3 Code