refactor(desktop): own Composer readiness, new-task choices and submission below the shell

[chihumyum]

Summary

This is the first of two PRs that finish R2 M3. It covers the submission side. AppShellContent no longer holds the Composer's readiness snapshot, the new-task mode choices, the send-pending flag, the edit-and-resend draft, or the code that assembles the send, Stop, Turn-branch and interaction paths. Each moves to the owner that already holds the related state, and the Host calls go through injected ports and Desktop adapters. The second PR handles the reader side; see "Left for the M3 reader PR" below.

Refs #4582

The four commits can be reviewed one at a time:

1. Readiness. A persistent TaskReadinessProvider sits beside ComposerStagingProvider and is a registered controller owner.
   • Its two Host reads go through TaskReadinessServices and create-task-readiness-services.ts.
   • The notice's only reader is the transcript's notice slot: ChatMessageSurface mounts TaskReadinessNoticeConsumer.
2. New-task choices. The Plan toggle, the orchestration value and the draft's permission choice move into SessionSettingsProvider.
   • Session Settings already decided between the selected Session and the new task for permission writes, and it confirms Bypass for both.
   • The shell reads newTask through the existing useSessionSettingIntent call, so no hook is added.
   • useNewTaskChoice moves to application/contracts, because Session Settings and the Conversation chat-model hook both use it.
   • This deviates from the issue text, which names a Composer sub-owner in Conversation. See "Review focus".
3. Submission and revision. A persistent ComposerSubmissionProvider is a registered controller owner. It owns the send-pending flag, the edit-and-resend draft and its catalog watch, and the retracted workspace references. It assembles createRevisionAwareOnSend (fix(desktop): allow unchanged edit-and-resend #5815), the staged follow-up, and the chat and revision actions.
   • ConversationComposerRegion injects onSend, newTaskSendPending, the interaction answers and the revision notice into the persistent Composer slot.
   • While a draft is open, the region narrows the shell's picker gates.
   • The shell keeps one stable command handle (beginEditUserMessage), following the refactor(desktop): move Composer staging into a persistent owner #5868 staging-commands pattern.
4. Stop, Turn branch and local recovery. The Stop and Turn-branch actions move into the same owner.
   • The Composer slot reads the published Session's Stop claim.
   • handleTurnFooterAction joins the command handle.
   • SessionLocalMessages, the local delivery-recovery reader, now mounts inside the owner for the published Session. Its recovery policy is unchanged.

Which caller loses which capability

Caller	Loses	Keeps / gets instead
AppShellContent	Readiness snapshot and refresh; useNewTaskChoice and the newTaskSendPending, newChatPlanModeActive, newChatOrchestrationMode and revisionDraft state; the refs revisionDraftRef and retractedWorkspaceReferencesRef; construction of the chat, revision, Stop and Turn actions, the follow-up and createRevisionAwareOnSend; the CatalogRowWatch and SessionLocalMessages mounts	It passes these to the owners: a readiness request projection; a newTask projection; a shell port of commands it already owns (surface ownership, navigation, catalog refresh, execution-boundary reload, the Workbar form answer, side chat, the new-task resolver, the model-setup toast, the Turn-action pending registry, the selected Session's orchestration write); and the ComposerSubmissionCommands handle
useAppShellSessionUiState (transitional adapter)	11 fields: stopPendingClaims, addTransientMessage, updateTransientMessage, removeTransientMessage, settleInteraction, markInteractionChanged, clearMessageLoadError, prepareSend, compactSession, readSelectionRevision, captureSelection	Target identity, the selection commands the shell still owns, and readMessages for Copy/Save
Conversation public entry	12 exports: createRevisionAwareOnSend, RevisionSendPorts, createStagedFollowUp, deriveTaskReadinessNotice, isTaskSubmissionHardBlocked, TaskReadinessNotice, useNewTaskChoice, SessionLocalMessages, SessionPendingClaim, toSubmittedAttachments, PendingAttachment, activeHostTurn	8 additions, all owners, readers, service providers or port types; no controller and no state reader
ChatMessageSurface	taskReadinessNotice, onTaskReadinessAction props	It mounts the readiness reader
AppShell-family legacy files	app-shell-chat-actions.ts, app-shell-revision-actions.ts, app-shell-stop-action.ts, app-shell-turn-actions.ts (10 bridge references)	They move into features/conversation/controller behind ComposerSubmissionServices

Other legacy files retired:

• use-task-submission-readiness.ts and the task-readiness-notice.ts re-export;
• skill-invocation-feedback.ts and follow-up-submit-routing.ts, which move into the feature;
• session-copy-attempt.ts, attachment-preflight.ts, side-chat-command.ts and desktop-slash-command.ts, which move to application/contracts because Workbar shares them. Workbar's three budgeted legacy imports become contract imports.

What enforces it

• controllerOwners. useTaskSubmissionReadiness runs only inside TaskReadinessProvider, and useComposerSubmission only inside ComposerSubmissionProvider. The public entry cannot re-export either.
• featurePrivateModules. The submission binding and reader context are private. The readiness context is module-local.
• Zone rules. The feature has no window.maka. Source-scan tests pin one mount for each owner and one composition reference for each adapter, and they require that no renderer file outside platform/desktop calls the moved bridge paths.
• Not enforced statically. Nothing statically forces the owners to stay persistently mounted. That depends on where they are placed beside ComposerStagingProvider; the Conversation README documents it. The owner tests cover it at the feature level only (a reader unmount/remount for readiness, Session switches for the draft).

Behavior preserved

There are no product, copy, IPC, storage or visual changes.

• Readiness: the latest-request fence, the reset on a request or target change, and a re-read on an onboarding refresh.
• New-task choices: keyed by target, reset to Plan off on a new task, and the permission choice cleared only after a created Session consumes it.
• Sends: shared Sessions refused before any Host call; draft-bound staging capture and cleanup (refactor(desktop): move Composer staging into a persistent owner #5868); revision prepare, rollback and abandon.
• Stop: claim and release, and removal of retracted rows. After fix(desktop): offer composer Resume after a manual Stop, drop the turn-footer resume button #5927, which landed while this PR was open, the send slot offers Resume after a manual Stop, so the owner no longer notes the stopped Turn.
• Turn branch: copy-attempt identity.
• Interaction answers and execution-boundary reload.
• workspace_picker routing.

The send-time admission check (checkTaskSubmissionReadiness) moves into the owner unchanged; it never read the readiness snapshot.

Review focus

• New-task choices live in Session Settings, not in a Conversation sub-owner. Holding them in Conversation would force the shell to read them through a new hook, because the Composer mode controls, the command palette's permission check and the execution-boundary surface are all assembled in the shell. Session Settings already owned permission writes for the new task and is already read by the shell. If reviewers prefer Conversation, the choices can move, but a shell reader comes back with them.
• The shell port has 15 named commands. Each is something the shell owns: navigation, the catalog, or another feature's command. None returns Composer state. Moving any of them further down depends on the PR2 reader work and on slices C and D.

Inventory (c7fa6bb6a → this branch)

Rows marked "#4582 measure" are from the issue's completion table. c7fa6bb6a is the issue's baseline column; main at 1a66e4d5e has the same values.

Measure	Source	Before	After
AppShell-family bridge references (#4582 measure)	legacyAppShell.files bridgePaths	43	33
— of which in app-shell.tsx		5	5
AppShell-family action factories (#4582 measure)	actionFactories	6	2 (createAppShellE2eFixtureActions, createAppShellProjectActions for slice C)
Transitional Conversation capabilities (#4582 measure)	README table	4	2 (useAppShellSessionUiReads; Copy/Save message read)
Hook gate: hooks / call sites	check:app-shell-hooks	28 / 42	26 / 33
Retired from the gate			useTaskSubmissionReadiness, useNewTaskChoice; useState 8 → 4; useStableActions 4 → 1
app-shell.tsx lines / imports / nonTriviaTokens		2,019 / 79 / 9,080	1,801 / 71 / 8,225
Registered controllerOwners	ledger	13	15
featurePrivateModules	ledger	13	15
Legacy renderer files	ledger	190	178

scripts/check-app-shell-hooks.mjs is edited by hand. renderer-architecture.json adds two controllerOwners, two private modules and removes two retired ownership entries by hand; the rest is regenerated with --write.

Left for the M3 reader PR

These depend on slices C and D:

• retiring useAppShellSessionUiReads. Its values are read on about 20 lines of app-shell.tsx (turnActive alone on 11), which also feed root chrome (pet state, slash-command availability, permission and mode disabled reasons, the home surface); that needs D's retained-root table.
• the Copy/Save message read, which lives in slice C's app-shell-command-actions.ts.
• placement of useShellResume, useShellChatModel, useTurnActionRegistry, useAppShellTurnPresentation and useActiveExecutionBoundary.

The external PRs #5058, #5274, #5581, #5513, #4138 and #5405 rebase onto these owners.

Verification

At b4a07eed8 (main 8ad836ce1 merged in), Node 24.19.0, fresh npm install plus the dependency patches and Electron install:

• npm --workspace @maka/desktop run clean:main && build:test && test:dist: 3,165 / 3,165 pass, 0 cancelled. fix(desktop): offer composer Resume after a manual Stop, drop the turn-footer resume button #5927 removed some resume tests on main.
• New owner suites: task-readiness-owner.test.ts (12 tests) and composer-submission-owner.test.ts (13 tests), plus new-task tests in session-settings-provider-scope.test.ts (2 tests). They exercise the real Conversation, staging and submission owners in the fake DOM.
• Existing action suites run against the production Desktop adapter over their window.maka doubles: app-shell-*-actions, first-send cleanup, busy race, revision resend and composer staging.
• Tests fail without the change. Each targeted mutation was applied to the built code and the named test failed:
  • readiness: removing the request fence; removing the reset;
  • Session Settings: dropping overlay identity; dropping per-target keying;
  • submission: dropping the edit gate; dropping send-pending; ignoring a shared Session; dropping new-task settings;
  • Stop: not reading the Stop claim;
  • recovery: not mounting it;
  • branch: not binding it.
  • With main's app-shell.tsx and chat-message-surface.tsx restored, the readiness ownership tests fail.
• npm run check:app-shell-hooks: ok, 26 hooks / 33 call sites.
• npm run check:renderer-architecture -- --base 8ad836ce1 --strict-base: passed.
• These all pass:
  • npm run typecheck (includes Storybook);
  • npm run lint, npm run format:check;
  • npx knip --workspace apps/desktop, npx knip --workspace packages/ui;
  • npm run check:asf-headers;
  • npm run windows:inventory (current, 119 declarations);
  • npm run check:e2e-budget (34 tests in 20 files, unchanged).
• npm run astryx:surface-inventory: regenerated; +2 aligned files (the two providers).
• Desktop E2E: npm run build then npx playwright test --config e2e/playwright.config.ts, all 34 Electron tests on b4a07eed8 (after merging fix(desktop): offer composer Resume after a manual Stop, drop the turn-footer resume button #5927, which changed the Stop path this PR moves): 34 passed (3.5 min), 0 failed, no retries. An earlier run before the rebase and merges also passed 34/34. No Electron test was added; per e2e/AGENTS.md the new coverage is in the fake-DOM owner tests. The existing specs already drive first send, local recovery across restart, side chat, /compact and streaming remount through the real app.
• Not run: a packaged build, Storybook smoke.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Opus 5.5 in Claude Code wrote the implementation, the tests, the ledger and inventory regeneration, and this description. The author reviewed them. Each commit carries Generated-by: Claude Opus 5.5.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed e0b1746b41a7e6e2147798647d0dd38cd217bd56. Scope stated up front: this is a large move (+2396/−806 across 72 files), so I verified the shape of the ownership transfer and the gate rather than reading all 23 modified production files line by line.

The transfer is real, and it is a removal from the shell rather than a parallel copy. app-shell.tsx loses 283 lines and gains 65 (net −218). What it stops importing is exactly the ownership this PR claims to move: useTaskSubmissionReadiness, desktopSlashCommandAvailability / parseDesktopSlashCommand, mergeWorkspaceReferences / rebaseWorkspaceFileReferences, createAppShellChatActions / createAppShellTurnActions / createAppShellStopAction, and useNewTaskChoice; along with the inline state that went with them (newTaskSendPending, newChatPlanModeActive, newChatOrchestrationMode). Nothing equivalent is re-imported into the shell, which is what I would want to see — the shell keeps rendering the feature and no longer owns the logic.

The file accounting matches that reading: 3 files added (all tests — composer-submission-owner.test.ts, task-readiness-owner.test.ts, plus composer-submission-fixture.ts), 23 modified, and 4 renamed — the four contract modules that carried the moved responsibilities. The generated architecture ledger is re-baselined with the budget going down (net −208 in that file), which is the expected direction when ownership concentrates rather than spreads.

Gate on this head: test and label are green.

What I did not judge

• The internal correctness of the relocated logic across the 23 modified files. Green tests are the evidence I have, and for a move I read them as weaker than for a behaviour change: many of the modified files are the app-shell-* suites themselves, so their assertions moved with the code. The three added modules do add genuine coverage of the new ownership, which is the stronger half of the signal.
• No Electron run.
• Whether any downstream reader still depends on the four relocated module paths externally.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[chihumyum]

Thanks for the review. Evidence for the three points it left open:

• Electron run. The full suite ran locally (npm run build, then npx playwright test --config e2e/playwright.config.ts): 34 passed, 0 failed, no retries. That run was on the pre-rebase head a9eaaf2ed. Its tree differs from e0b1746b4 only in test(desktop): add storybook pixel-diff capture/compare tool #5847's five CI, Storybook-doc and script files. This is listed under Verification.
• Moved suites. The 10 existing action suites change by +99/−70 lines. Every changed line is one of three things: an import path, services: injected through the production Desktop adapter over the same window.maka doubles, or a factory rename (createAppShellChatActions → createChatActions, and so on). No assertion changed. The new owner suites were checked by mutating the built code; 12 targeted mutations each fail a named test.
• External readers of the relocated paths. No file in the repository imports any retired path, and typecheck and knip are clean.
  • Among the 199 open PRs, five edit the retired files directly: fix(desktop,runtime): interrupt and bound empty assistant loops #4138 (Stop), fix(desktop): clarify delivery states and recover failed messages #5058 and fix(ui): persist AskUserQuestion wizard across session switches #5854 (chat actions), fix(desktop,ui): stage a selected message's quotes on edit #5274 (revision actions) and feat(runtime): replace OS sandboxing with model auto review #5405 (chat and Turn actions).
  • Per the M3 note on refactor(desktop): seal AppShell capability boundaries and move state to regional owners (R2) #4582, they rebase onto the owner. The functions now live at features/conversation/controller/{chat,revision,stop,turn}-actions.ts. Their bodies are unchanged except that window.maka.* calls became services.* calls.
  • fix(desktop): keep sidebar actions anchored to the left #5253 and fix(desktop): preserve model and thinking when adding a project #5581 match only in the generated ledger.

One correction to the summary: the shell still imports desktopSlashCommandAvailability, now from application/contracts/desktop-slash-command.ts, because it builds the slash-command list. Only parseDesktopSlashCommand left the shell.

Posted by Claude Code on behalf of the PR author.

[Astro-Han]

Independent review of e0b1746b41a7e6e2147798647d0dd38cd217bd56 (base 1a66e4d5e; current main 887924e14 is one commit ahead and the PR merges cleanly onto it).

Verdict: the move looks behaviour-preserving. No P0 or P1 found. One cross-PR knip break (P3, coordination) and three further P3s are below.

What I checked line by line against main:

• app-shell-{chat,revision,stop,turn}-actions.ts → features/conversation/controller/*: every window.maka.* call now goes through ComposerSubmissionServices with the same arguments. The Desktop adapter maps them 1:1. The guards, toasts, copy-attempt identity, rollback and unsent-Session cleanup are unchanged.
• useTaskSubmissionReadiness: same sequence fence, same reset on a request, target or refresh change, same dependency list. The services come from composition, so they are stable and checkNow does not churn.
• The picker gates keep their asymmetry: the context picker is blocked only while editing in the draft's Session, and the directory picker is blocked while any draft is open. composer-submission-owner.test.ts pins both, including the switch to another Session.
• SessionLocalMessages receives the same function identities as before: commands.addTransientMessage/removeTransientMessage, toast.error, queue.restoreDraft. Its effect therefore does not re-run more often. activeIdRef is still workspace.publishedSession.
• New-task choices: useNewTaskChoice keeps the same key (currentNewTaskDraftKey). Plan and orchestration stay unkeyed as before, and openNewTaskSurface still resets only Plan. The shell sees the state through the bridge's layout-effect publish, which is the same pattern the overlays already use, so the update lands before paint.
• useStableActions facades keep reader/shellCommands stable. They change only when newTaskSendPending or revisionDraft changes. StrictMode's layout-effect replay re-binds submissionBindings correctly.
• Ledger and gate only tighten: two new controllerOwners, two private modules, four legacy AppShell action files gone, useStableActions 4→1, useState 8→4, and useNewTaskChoice and useTaskSubmissionReadiness removed from the gate.

Local run on this head (Node 22, build:test): composer-submission-owner, task-readiness-owner, session-settings-provider-scope, composer-staging-owner, use-stable-actions and all app-shell-* suites pass (131/131). CI test is green.

P3 (coordination): merging after #5937 breaks knip (CI). (Graded P3: neither PR is wrong on its own; this is a merge-order coordination item, like the protocol-epoch collisions we track across PRs.) This PR copies showSessionWorkspaceUnavailableToast into features/conversation/model/session-workspace-toast.ts. After that, the only importer left for the legacy src/renderer/session-workspace-errors.ts is app-shell-project-actions.ts, and #5937 deletes that file. I merged main + this PR + #5937 locally and ran npx knip --workspace apps/desktop. It reports Unused files (1) apps/desktop/src/renderer/session-workspace-errors.ts, so whichever of the two lands second fails CI's knip step. Suggested fix: move the toast helper into application/contracts/session-workspace-errors.ts, next to isSessionWorkspaceUnavailableError, and import it from there instead of copying it. Then delete the legacy file in whichever PR lands second, including its ledger entry.

P3: test gap, steering Turn id. The owner's getRunningTurnId wiring (use-composer-submission.ts:86) is not covered by any test. I changed it in the built output to always return undefined, which drops hostTurnId from a steering message's transient row, and the full test:dist suite still passed (3,137/3,137). Please add a case to composer-submission-owner.test.ts: with an active Host Turn, a current_turn send publishes the transient row with that hostTurnId.

P3: duplicated helper. session-workspace-toast.ts:27 is a byte-for-byte copy of the legacy helper. The coordination fix above also removes this.

P3: provider value churn. openWorkspacePicker is a new closure on every AppShell render (app-shell.tsx:1241), so TaskReadinessProvider's memoized value, and TaskReadinessNoticeConsumer with it, change on every shell render. This is cheap today. Passing a stable command and the activeSession?.id / canAddProject facts would make the memo meaningful.

Cross-PR / merge order (the four R2 PRs). I ran pairwise git merge-tree against 887924e14:

• After #5936: textual conflicts in app-shell.tsx, renderer-architecture.json, scripts/check-app-shell-hooks.mjs and composer-staging-owner.test.ts.
• After #5937: conflicts in renderer-architecture.json and the Astryx inventory, plus the knip break above.
• After #5934: no textual conflict, but its checker then fails on the merged tree. I ran it there: there is a stale retained-root row for useTaskSubmissionReadiness (README:307), five unrecorded new root symbol uses, and nine stale rootSymbolUses entries. That is the expected --write plus row-deletion follow-up, and all the additions are new exports, so the ratchet admits them.

---

This is an automated review by Claude (Anthropic), run on behalf of the maintainer. It is not an approval. Please verify the findings before acting on them.

[Astro-Han]

Reviewed cd3345ceb4b06f26ba52dcf8206b1d30228f3294 (72 files, +2466/−810, 5 commits).

All three P3s I raised last round are fixed, each verified in the code rather than from the reply.

1. The duplicated toast helper is gone from the Conversation zone. The definition now lives once in the application contract (application/contracts/session-workspace-errors.ts), the Conversation copy is deleted outright, and the callers pass the shell copy in as a parameter, so the contract does not reach back into a feature's copy. One copy of the same helper still exists in the legacy src/renderer/session-workspace-errors.ts, which is exactly the file I flagged as #5937's to remove — and it still has an importer (app-shell-project-actions.ts) only because #5937 has not landed. So this PR did what it could and the residue is correctly attributed to its sibling.
2. The uncovered steering wiring now has a test. composer-submission-owner.test.ts adds "steers into the running Host Turn: the pending row names that Turn before the Host answers", asserting pending?.hostTurnId === 'host-turn-1' for a followUpMode: 'steer' send. That is precisely the assertion my mutation test showed was missing — I had flipped the wiring to return undefined in dist and the whole suite still passed.
3. The per-render closure is gone from the readiness provider. TaskReadinessProvider now takes workspaceRecoverySessionId and the stable openSessionWorkspaceRecovery / addProject commands and memoizes the picker action on those, so its context value no longer changes on every shell render.

Gate on this head: test and label are green; mergeable is true against the base branch, so this PR can still land on its own. Every commit carries a Generated-by: trailer and there is no Grok involvement.

What I could not judge

• I did not run the suites or the mutation check again; I verified the three fixes by reading the code, the deleted file, and the new test's assertion.
• The render-stability test the reply mentions: I confirmed the memoization change on stable inputs but did not execute its new assertion.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed b4a07eed878642a86a082386f0f84aad2f1c4b93.

The rebase is faithful, so my previous review's conclusions carry over unchanged. A git range-diff between the old and new ranges marks every commit of this PR as = — byte-identical — with the only differences being commits that arrived from main itself. The tree difference I first measured against my previously reviewed head was entirely that base advancement, not a change in this PR's own work. The gate is green on the new head.

Merge order, since the dispatch asked whether the #5934 → #5935 → #5936 → #5937 sequence is still required and which pairs still conflict. I measured the file sets at the current heads. All four now sit on the same base, so they cannot land independently; every pair except #5934's overlaps on real source, and most pairs overlap on app-shell.tsx:

• #5934 ∩ #5935 and #5934 ∩ #5936: only the generated renderer-architecture.json.
• #5934 ∩ #5937: that ledger plus features/overlays/index.ts and testing.ts.
• #5935 ∩ #5936: twelve files, including app-shell.tsx, chat-message-surface.tsx, composition/desktop-feature-services.tsx, scripts/check-app-shell-hooks.mjs and features/conversation/testing.ts — the heaviest of the six pairs.
• #5935 ∩ #5937: the ledger, app-shell.tsx, and both inventory documents.
• #5936 ∩ #5937: the ledger, app-shell-effects.ts and app-shell.tsx.

So the prescribed order still holds, and each landing after the first needs a real merge on app-shell.tsx that should be reviewed rather than trusted, since these are ownership moves in the same file rather than adjacent edits.

What I could not judge

• The rebase was verified by range-diff and the overlaps by file set; I did not run the suites or produce the merges themselves.
• I did not re-audit the moved logic, since the rebase left it untouched.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.