refactor(chat): extract composer ownership - #1098
Conversation
|
Codex review: needs maintainer review before merge. Reviewed August 17, 2026, 7:58 PM ET / 23:58 UTC. ClawSweeper reviewWhat this changesThe PR extracts native chat-composer state, workflows, host lifetime, and declarative controls from the chat root into dedicated components. Merge readinessThe change is still necessary because current main retains the original nested composer path. The current head resolves the prior mounted-input lifecycle and accessibility blockers; this collaborator-authored PR should remain open for explicit maintainer landing judgment after pending CI completes. Priority: P1 Review scores
Verification
How this fits togetherThe native chat surface renders composer controls over gateway-backed conversation state. This PR moves per-host draft, attachment, voice, and send handling into a composer session while the root retains provider subscription and timeline ownership. flowchart LR
A[Gateway chat provider] --> B[Chat root]
B --> C[Composer session]
C --> D[Composer state]
C --> E[Composer workflows]
D --> F[Composer controls]
E --> F
F --> G[Chat send and attachments]
Decision needed
Why: Collaborator-authored work requires explicit maintainer judgment even when the code-level blockers are resolved. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the dedicated composer boundary once required CI passes, preserving mounted TextBox lifecycle, UI Automation IDs, and root-only provider ownership. Do we have a high-confidence way to reproduce the issue? Not applicable: this refactor is reviewed against established native composer behavior, and the prior input-lifecycle regressions are source-verified as resolved on the current head. Is this the best way to solve the issue? Yes: separating per-host composer state and workflows while retaining provider and timeline ownership in the root matches the architecture ledger and keeps the previous mounted-control behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f8a8825ccbb4. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (29 earlier review cycles; latest 8 shown)
|
|
Fixed the ClawSweeper P2 on head
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
e666bb8 to
2325e69
Compare
|
Updated exact stacked head @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
2325e69 to
8464a74
Compare
8464a74 to
1c614a2
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Serialize voice and clipboard operation initiation with session disposal, fence callbacks and results to the active operation, and cancel outside the registration gate to avoid callback deadlocks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Port the canonical parent accessibility gating and native context menu into the extracted composer while retaining cancellation-safe paste ownership in ChatComposerController. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02754eab-8008-4859-8a80-9e6462133380
Preserve the pre-extraction composer boundary by normalizing reset-confirmation and lifecycle failures to a rejected send instead of leaving a fire-and-forget task faulted. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02754eab-8008-4859-8a80-9e6462133380
53d100a to
edf0c9e
Compare
Current-head UI proof (PII redacted)Captured from the isolated synthetic chat provider at Composer draft and controls Native composer context menu Model picker Validation on this head: full build passed; Shared 3698 passed / 32 skipped; Tray 2619 passed; ARM64 WinUI 143 passed; live A2UI integration 1 passed. |



Summary
ChatComposerViewModel.ChatComposerController.ChatComposerSessionper mounted ChatPage/ChatWindow host through the existing DI container.ReactorChatComposer.OpenClawReactorChatRootfrom 1,766 to 428 lines while retaining the sole provider subscription, immutable snapshot, selection, timeline, checkpoint, and scrolling ownership.Ownership transfer
ChatComposerViewModelChatComposerControllerChatComposerSession/ factoryReactorChatComposerOpenClawReactorChatRootresidueChatPage and compact ChatWindow share provider state but intentionally retain independent drafts, attachments, popup/focus, voice, and operation lifetime.
Correctness and lifecycle
CancellationTokenSource.Tokengetter.GetBitmapAsyncbegins; if registration wins, initiation occurs before disposal returns and cancellation follows./compact,/reset, and/newrecheck lifetime after each await.Validation
Exact D2 head
53d100a5f431c630bdb4cfb535e983be9b50d3d6, based directly on D159096c38182f07e22bb8980951a9c80828f0c01f:./build.ps1: passedbb6b16e6df147f8b2126eebaeed946c2a8d5164egit diff --check, source-owner guards, conflict markers, UTF-8, and final-newline checks: cleanThe focused barriers cover voice/clipboard dispose-wins and registration-wins ordering, callback reentrancy, cancellation callback waits, supersession, late-result fencing, repeated race stress, idempotent disposal, and real
DataPackageView.GetBitmapAsyncinitiation.Real behavior proof
Exact-head native UI and Axe.Windows suites exercised the real app across compose, queue, stop, lifecycle, model/thinking, slash, attachment/paste, page/window lifetime, provider replacement, focus, keyboard, automation, live regions, theme/high contrast, scrolling, checkpoints, disposal, late completion, dispatcher ordering, and repeated cleanup.
A fresh disposable real-gateway fixture also validated exact stack head
53d100a5:0, and no active turn.app.chat.resetimmediately before the real send and observed reset completion.ConnectingtoReady; isolated profile/distro/tray cleanup completed; shared marker stayed 0 -> 0; redaction and Git-state checks passed.Redacted package:
d2-operation-gate-proof-53d100a5-20260806-190646Manifest SHA-256:
3016f4c5e3c4489e0b89df01eb4967eade125c8e209f1767dfc68a47503109b8Review
Architecture
Adds authoritative and closed ledger rows for composer VM, workflow controller, host lifetime, and Reactor root residue. D1 runtime owners, provider state, queue/history/reset/metadata, timeline rendering, scrolling, and checkpoints remain authoritative.
Stack
Directly based on D1 PR #1096. This is the second/final Chat architecture layer.