refactor(desktop): move the AppShell command and project actions' Desktop calls to their owners

[chihumyum]

Summary

Refs #4582 — M5, the AppShell-family action helpers outside M3.

This takes the command-palette and project helpers, plus the legacy diagnostics reports, off the Desktop bridge. Each goes to the owner that already holds that area. The AppShell family drops from 43 → 24 bridge references and 6 → 5 action factories. The 24 that remain are the chat, revision, stop and turn actions (M3), the effects, session-list and app-shell.tsx paths (separate slices), and the exempt E2E fixture. There is one commit per owner.

1. Diagnostics (features/diagnostics)

Three legacy callers still called window.maka.diagnostics.copyReport after #5892. Each now gets one command, with its report surface fixed by the adapter:

Caller	Loses	Gets
Error Boundary (error-boundary.tsx)	window.maka?.diagnostics (4 references): the whole namespace, any surface	copyRendererCrashReport({ title, details }) from an optional RendererCrashReportConsumer
About (settings/about-settings-page.tsx)	window.maka.diagnostics.copyReport	copyManualReport() from ManualDiagnosticReportConsumer
Palette diag:copy-diagnostics and ⌘⇧D	window.maka.diagnostics.copyReport	copyManualDiagnosticReport(target) in the command options; AppShell reads it through the same consumer
AppShell → DiagnosticReportToastProvider	the labels prop and the outer AppShell's shell-copy read	nothing; the provider reads the same strings from locales/shell-copy.ts

• The consumers are render props because both legacy files are hookCalls-ratcheted. About already reads App Update the same way.
• Without Desktop composition (Storybook, renderer tests), the Error Boundary still renders its fallback and copies its own bounded browser report. That optional read is the one addition to the shared factory: createServicesContext gains useOptionalServices (5 lines).
• The toast labels moved to pay for the consumer in app-shell.tsx, which has no token headroom. Reading the feature in app-shell-overlays.tsx would have cost +43 tokens, and a hook in useAppShellCommands would have been new hookCalls debt.

2. Project actions (features/task-entry)

createAppShellProjectActions built 12 actions. Nine had no production caller:

• add, select, select-no-project, prepare and prepare-default were never read;
• relink, rename, archive and restore were destructured at app-shell.tsx:878-881 but never used;
• the rail and new-task flows have gone through taskEntry.commands since fix(desktop): scope project groups by runtime host #5527.

This PR deletes the nine, together with:

• their helpers;
• the project-picker state and refs that only addProject used, including their unmount cleanup in app-shell-effects.ts;
• onProjectSelected, which only those actions fired, so its refreshProjectSkills() call never ran in production.

The two live folder openers move to Task Entry, whose README already titles it "Task Entry / Workspace":

• Port: TaskEntryServices.folders.openProjectFolder(sessionId?) and openWorkspaceFolder().
• Adapter: opens a task's folder through the task and anything else on the default Runtime Host. It returns opened, refused or failed, naming the task or Host profile the result belongs to.
• Controller: exposes openProjectFolder / openWorkspaceFolder commands and reports failures with the same shell copy, diagnostic targets and workspace-unavailable notice as before. TaskEntryError may now carry a sessionId in place of a profileId.
• AppShell: passes taskEntry.commands to the titlebar and the palette.

refreshProjects stays with the workspace projection in use-project-context.ts, inlined on runOnDefaultRuntimeHost. That hook has no new bridge paths, and it loses useStableActions, two useRef calls and one useState.

Also removed: app-shell-project-actions.ts, its workspace-projection ownership entry, and the now unused openPathActionErrorMessage. The open-path classification test now covers Task Entry's folderOpenFailure.

3. Command palette rows (features/overlays)

Five palette rows called the bridge: test a connection, make it the default, test the network proxy, open the local memory file, and save the conversation to a file. They now go through the palette's owner:

• Port: OverlaysServices gains palette: OverlayPaletteActions. The Desktop adapter makes the same bridge calls with the same arguments.
• Connections: the palette names connections by slug, which goes over the connections:testBySlug IPC channel. That is a different channel from Connection Settings' identity-based test/setDefault, so this port does not duplicate that one.
• Delivery: the overlays projection hands the port to AppShell as paletteActions, at a cost of about 6 tokens because AppShell already holds that projection. The rows keep their default-Host resolution, toasts and failure copy.
• Boundary test: the overlays entry-surface pin now lists OverlayPaletteActions for app-shell-command-actions.ts.

Inventory deltas (generated ledger, c7fa6bb6a → this branch)

Measure	Before	After
AppShell-family bridge references (M5 table)	43	24
— command actions	6	0
— project actions	13	file removed
— app-shell.tsx	5	5
Action factories	6	5
AppShell-family files / legacy renderer files	16 / 190	15 / 189
Ownership entries	12	11
Non-trivia tokens: app-shell.tsx / command actions / effects / app-shell-copy.ts	9,080 / 2,188 / 1,361 / 311	9,048 / 2,172 / 1,331 / 249
Closure bridge references (outside the M5 table)	165	160

Unchanged:

• the hook gate (28 hooks / 42 call sites);
• controllerOwners (13; no controller was created or moved);
• featurePrivateModules (13);
• app-shell.tsx imports (79). app-shell.tsx itself goes from 2,019 to 2,005 lines.

What enforces it

• Types: each caller gets named commands. Report surfaces are fixed in the adapters, and the palette and folder ports take only the arguments the rows used to send.
• Ratchet: once this ledger is the base, putting a window.maka.* call back into the command actions, the Error Boundary or About is new bridgePaths debt. For the three diagnostics calls this was checked by re-adding them against this branch's ledger, which produced 7 violations. Recreating a project-action factory in the family is a new actionFactories entry, which the ratchet also rejects.
• Boundary tests: the overlays entry-surface pin and the diagnostics source scan make a new legacy import of these ports a visible test change.
• Not enforced: a new legacy caller can still be handed these commands through a feature's public entry. That changes the feature's index.ts, so it shows up in review.

Overlap

• perf(desktop): bound transcript work on session switches #5712 (open since 09-25, conflicting) rewrites app-shell-command-actions.ts. Its base 1e80e3b88 has the same version of this file, so line numbers compare directly:
  • real overlaps: the overlays import (line 33 against its 32–38) and the save-conversation handler (lines 235 and 256 against its 213–258);
  • adjacent but not overlapping: the memory-file destructure at line 284, next to its 272–282.
  • Its new test builds the options as unknown as AppShellCommandListOptions, so the new required fields do not break that test's types.
• fix(desktop): spell keyboard shortcut labels per platform #3887, fix(desktop): retry update downloads without rechecking feed #5642 and fix(ui): govern leftover icon sizes with ICON_SIZE #3127 touch About or the command actions and already conflict with main.
• Sibling refactor(desktop): seal AppShell capability boundaries and move state to regional owners (R2) #4582 PRs: I simulated each merge with git merge-tree.
  • chore(desktop): check R2 root symbol uses and retained root hooks #5934 merges cleanly. On the merged tree its checker passes --strict-base against its head and admits one new appShell root symbol use, features/diagnostics: ManualDiagnosticReportConsumer, as a new public export. No retained-root hook rows change, because no hook leaves AppShell here.
  • refactor(desktop): own Composer readiness, new-task choices and submission below the shell #5935 conflicts only in the generated ledger and the Astryx inventory.
  • refactor(desktop): retire AppShell's direct Desktop bridge (R2 M5) #5936 conflicts in the ledger and in one spot in app-shell.tsx: both PRs wrap AppShellContent in a render-prop consumer (OnboardingProjectionRoot there, ManualDiagnosticReportConsumer here) and add one prop each. Whichever lands second nests both consumers and keeps both props.
  • This PR also removes two option fields and their cleanup lines from app-shell-effects.ts; refactor(desktop): retire AppShell's direct Desktop bridge (R2 M5) #5936 edits that file in other hunks.
  • Whichever PR lands second regenerates the ledger from main's copy and re-adds any hand-maintained entries.

Verification

• Desktop tests: 3,162/3,162 pass (Node 24, clean:main + build:test + test:dist), against 3,143 on c7fa6bb6a. 22 tests were added and the 3 legacy project-action tests deleted; two of those only covered deleted actions, and the third moved to the Task Entry adapter.
• Fail without the change:
  • diagnostics: with the routing files reverted, 10 of the 19 tests in those two files fail;
  • Task Entry: three targeted mutations are each caught by a new test (the provider's sessionId target, the workspace-unavailable branch, and the adapter's task path);
  • palette: with the five calls pointed back at the bridge, all 3 new palette tests fail.
• Gates, all passing:
  • typecheck (all workspaces), lint, format:check;
  • Knip (apps/desktop, packages/ui), check:asf-headers, windows:inventory;
  • check:locale-hygiene --base c7fa6bb6a, check:app-shell-hooks;
  • check:renderer-architecture --base c7fa6bb6a --strict-base, which also passes against the current main 1a66e4d5e;
  • the Astryx surface inventory (regenerated for one new file).
• Electron, on a local build: rename-focus.spec.ts, settings.spec.ts (2) and zh-tw-locale.spec.ts pass (4/4). Throwaway local scripts, not committed per e2e/AGENTS.md, also checked the real IPC path:
  • 设置 → 关于 → 复制 and ⌘⇧D each wrote Maka Desktop diagnostic report … Surface: manual to the system clipboard;
  • the palette's 测试当前网络代理 came back with the result-branch toast (网络代理测试失败, the e2e environment has no proxy), not the exception toast, so the call reached Desktop through the new port.
• Not run: opening a folder or the save dialog in Electron (both open native UI), Storybook smoke (the stories typecheck; the Settings stories now mount fake diagnostics services), a packaged build, and a real renderer crash in Electron.

AI use

Select exactly one:

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

Tool(s) and scope: Claude Code (Claude Opus 5.5) implemented these ownership moves, their tests and documentation, ran the validation and drafted this description, all under human direction and review. Each commit carries Generated-by: Claude Code.

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 02a1ea720fa1ec82e3e243e2711ed40bfe9469a8 (49 files, +1319/−850, 3 commits).

No P0–P3 findings, at the shape-and-gate depth this card was scoped to.

The ownership move is a removal from the shell. app-shell.tsx goes to +14/−28 (net −14): the command and project actions' Desktop access leaves the shell rather than being copied beside it. The added files are the expected shape for that — owner tests plus an options fixture, with the command-palette Desktop actions getting their own suite.

The ledger only tightens. apps/desktop/renderer-architecture.json moves +11/−61 (net −50), so the AppShell budget shrinks by fifty lines with nothing added back. check-app-shell-hooks.mjs is unchanged, which is consistent rather than a gap: its allowances only need editing when a hook actually leaves the shell, and the absent hook-count change means none of its quotas grew.

Gate on this head: test and label are green; the three commits each carry a Generated-by: trailer and there is no Grok involvement.

What I did not judge

• The internal correctness of the relocated command/project logic beyond the accounting; this card was scoped to shape and gate because a second lineage is doing the per-file pass.
• No Electron run.

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]

Independent review of 02a1ea720fa1ec82e3e243e2711ed40bfe9469a8 (base c7fa6bb6a, two commits behind current main 887924e14; it merges cleanly onto main).

Verdict: behaviour-preserving. No P0 or P1 found. One cross-PR knip break shared with #5935 (P3, coordination) and two further P3s are below.

What I checked against main:

• Palette rows. create-overlays-services.ts makes exactly the old calls: connections.test(slug, undefined, host), connections.setDefault(slug, host), settings.testNetworkProxy(undefined, host), memory.openFile(host), sessions.saveConversationToFile(input). runOnDefaultRuntimeHost, the toasts and the failure copy stay in app-shell-command-actions.ts. paletteActions is services.palette, so its identity never changes and it is read through optionsRef.
• Diagnostics. About, ⌘⇧D/palette and the Error Boundary send the same copyReport payloads: surface: 'manual' with an optional target, and surface: 'renderer_crash' with title and details. The Error Boundary is still a class boundary. The new function wrapper sits outside it and only reads the optional services, so a missing provider still leads to the browser-report fallback. DiagnosticReportToastProvider reads useUiLocale() under the same LocaleProvider that AppShell resolved uiLocale for, so the labels are unchanged.
• Project actions. I confirmed on main that relinkProject, renameProject, archiveProject and restoreProject are only destructured at app-shell.tsx:879-881. The rail uses taskEntry.commands.* (app-shell.tsx:967-970). onProjectSelected was fired only from selectProject, selectNoProject and prepareProject, which are deleted with it. The picker refs were read only by the deleted addProject and the bootstrap cleanup, which is removed together with them. openProjectFolder(ownerActiveId) keeps the old sessionId source (useAppShellProjectContext({ sessionId: ownerActiveId })). The adapter keeps the session-vs-default-Host branching and the diagnostic targets, and folderOpenFailure reproduces the workspace-unavailable branch and the copy, as the open-path.ts helpers resolved it. refreshProjects has the same body, and its only consumer is the bootstrap's effect-event handler, so losing the stable identity is harmless.

Local run on this head (build:test): about-settings-page, command-palette-desktop-actions, diagnostics-owner, expected-error-presentation, overlays-*, task-entry-* and use-project-context pass (100/100). CI is green.

P3 (coordination): knip breaks with #5935. (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 deletes app-shell-project-actions.ts, the last importer of showSessionWorkspaceUnavailableToast from the legacy src/renderer/session-workspace-errors.ts once #5935 has moved the chat, revision and turn actions onto its own copy. On main + #5935 + this PR, npx knip --workspace apps/desktop reports Unused files (1) apps/desktop/src/renderer/session-workspace-errors.ts, so whichever of the two lands second fails CI's knip step. Fix in the second PR to land: delete the legacy file and its ledger entry. Better still, #5935 could move the helper into application/contracts/session-workspace-errors.ts.

P3: unstable handler. openProjectFolder (app-shell.tsx:883) is now a fresh closure on every AppShell render and is passed straight to the titlebar's onOpenFolder. The old useStableActions facade kept it stable. The palette reads it through optionsRef, so only the titlebar sees a new prop each commit. useCallback over [taskEntry.commands, ownerActiveId] would restore the stable identity.

P3: second copy of the open-path helpers. folderOpenFailure re-implements openPathActionLabel and openPathFailureCopy from open-path.ts, including the reason in failures fallback (folder-open-failure.ts:39-46). The output matches today, but there are now two copies to keep in sync. Consider moving those two helpers into an application contract and importing them.

Cross-PR / merge order (pairwise git merge-tree against 887924e14):

• #5936: conflicts in renderer-architecture.json and exactly three app-shell.tsx hunks (@@ -204, -240, -249). Both PRs wrap AppShellContent in a render-prop root and add one prop.
• #5935: conflicts in the ledger and docs/astryx-surface-file-inventory.md, plus the knip break above.
• #5934: no textual conflict. On main + #5934 + this PR, #5934's checker fails with exactly one line: appShell root uses ManualDiagnosticReportConsumer from features/diagnostics/index.ts, which rootSymbolUses does not record. That is a new export, so --write admits it.

---

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 50988296abd130ec8d38914847a154aa204a3d3b (51 files, +1344/−874, 4 commits).

Both P3s from my previous pass are fixed, verified in the code.

1. The titlebar's folder action has its identity back. My finding was that a stable useStableActions facade method had become a fresh closure every render, and that although the command palette reads it through optionsRef (where it is harmless), it also goes straight into the titlebar's onOpenFolder. At this head openProjectFolder is a useCallback over [taskEntry.commands, ownerActiveId] and the titlebar consumes that same value, so the prop's identity is stable between commits again.
2. The duplicated open-path helpers are merged. I flagged that folder-open-failure.ts re-implemented openPathActionLabel and openPathFailureCopy from open-path.ts, fallback included. They now have a single definition in locales/shell-copy.ts, next to the strings they read; folder-open-failure.ts imports and calls them instead of restating the logic, and other consumers still reach them through open-path, which re-exports them. So the output is the same and there is now one place to change.

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

What I could not judge

• I did not run the suites; both fixes are verified by reading the callback and its dependency list, the single helper definition, its consumers and the re-export.
• Whether this PR still conflicts with its sibling on app-shell.tsx — the three overlapping regions I measured last round are in the file this change edits, so the overlap should be re-measured at merge time rather than assumed resolved.

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 eb057b0747c34426e33861f6ec5ec606c52cbec3.

The rebase is faithful, so my previous review's conclusions carry over unchanged. A git range-diff between the old and new ranges marks all five commits of this PR as = — byte-identical — the only differences being commits from main. The tree difference against my previously reviewed head was that base advancement alone. The gate is green on the new head.

Merge order and remaining conflicts (measured): against #5934 the overlap is the ledger plus features/overlays/index.ts and testing.ts; against #5935 it is the ledger, app-shell.tsx and both inventory documents; against #5936 it is the ledger, app-shell-effects.ts and app-shell.tsx. All four PRs share one base, so the #5934 → #5935 → #5936 → #5937 order is still required and this one should land last, with its app-shell.tsx resolution reviewed rather than trusted.

What I could not judge

• Verified by range-diff and file sets; I did not run the suites or perform the merges.
• The moved logic was not re-audited, 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.