refactor(desktop): extract goals as a feature slice

[chihumyum]

Summary

• Move Goal read state, subscriptions, controls, dialog ownership, and indicator assembly behind a renderer feature boundary.
• Isolate Desktop bridge access behind a narrow GoalServices adapter and provider; keep AppShell limited to Session context plus host, commands, and selectors.
• Add fake services and focused lifecycle, adapter, model, and architecture-boundary coverage.

Refs #3439

Verification

• npm run lint
• npm run format:check
• npm run build
• npm run typecheck
• npm --workspace @maka/desktop run test:dist — 1198 passed
• npx knip --workspace apps/desktop
• npx knip --workspace packages/ui
• npm --workspace @maka/desktop run build-storybook
• npm --workspace @maka/desktop run smoke:storybook — 166 stories passed
• npx playwright test --config e2e/playwright.config.ts from apps/desktop — 50 passed, 1 skipped

No screenshots are included because this changes no Goal copy, CSS, layout, IPC contract, or user-visible behavior.

Review focus

• Session-switch fencing for stale Goal reads.
• Goal-change event filtering and subscription disposal.
• Dialog ownership of the Session captured when it opens.
• Per-Session pause/resume re-entry protection.

AI use

Select exactly one:

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

Tool(s) and scope: Codex assisted with the feature extraction, implementation, tests, and verification.

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 at head b29e3c4e. One blocking item, and it is a generated file rather than the code — the extraction itself came through clean.

[P1] The Astryx surface inventory wasn't regenerated, and that's what's failing test.

test is FAILURE on this exact head (job 97083453961), and I reproduced it locally: npm run astryx:surface-inventory exits 1 on this branch, consistently. Three moved files are on disk but absent from docs/astryx-surface-file-inventory.paths — apps/desktop/src/renderer/features/goals/ui/goal-dialog.tsx, plus services-context.tsx and goal-host.tsx in the same directory — while the pre-move apps/desktop/src/renderer/goal-dialog.tsx is still listed. docs/astryx-surface-file-inventory.md is likewise out of sync with the generator's output.

Fix is mechanical: run npm run astryx:surface-inventory:write and commit both regenerated files. Nothing about the runtime behaviour is at fault here — the gate is doing its job, the branch just moved surfaces without re-running it.

---

On the refactor itself: no P0–P3.

For a pure extraction the question is only ever whether behaviour changed under cover of moving things, so that is where I spent the time. use-goal-controller.ts ends up holding the active goal, session-switch clearing, the late-read sequence and disposed guards, the goal-change subscription, per-session control dedupe, and the dialog's session snapshot — and it stays a projection of Host events rather than becoming a second authority, which is the failure mode this kind of slice usually introduces. create-goal-services.ts is the single Desktop bridge adapter, and AppShell only reaches Goals through GoalHost and the controller's public API.

The deletion side checks out too, which is the part that makes an extraction actually reduce entropy rather than add a layer: use-session-goal.ts is gone, the old dialog's direct window.maka dependency is gone, and AppShell no longer carries goal control state. A repo-wide search finds no useSessionGoal and no surviving production entry to the old dialog path.

Verified: Goals focused tests 12/12; controller, model, adapter, and feature-boundary suites all pass; @maka/desktop builds and typechecks across renderer, preload, main, and Storybook; Biome clean on changed files. The full desktop main suite ran 1190 pass / 0 fail with 8 cancelled — the cancellations sit in pre-existing MCP OAuth pending tests and are not attributable to this branch, so I'm reporting it that way rather than calling the suite green.

Regenerate the inventory and this is ready for another look.

[chihumyum]

Fixed in f9554a9af by regenerating both checked-in Astryx surface inventory files.

Local verification:

• npm run astryx:surface-inventory:write
• npm run astryx:surface-inventory — 203 files, 1 exclusion
• git diff --check

The new CI run is currently action_required with no jobs, so it needs maintainer approval before GitHub will execute the workflow.

[Astro-Han]

Two independent lines on f9554a9af129f6bede8b7975c0a4bb9a2a0cc3be, from different models, neither able to see the other's conclusion. Both came back with nothing at P0–P3, and — more importantly for a refactor — both answered the question that actually matters here.

A refactor's failure mode isn't "didn't fix it", it's "changed behaviour in transit", and in a diff a relocation and a rewrite look identical. So both lines were asked to split -263 into what genuinely disappeared versus what is just move noise.

What disappeared (~150 lines):

• use-session-goal.ts is gone as a file, and its authority — goal state, the change subscription, session-switch semantics — moved wholesale to the controller rather than being copied.
• The indicator construction inlined in app-shell.tsx (~55 lines), plus runGoalControl, pendingGoalControlSessionIdsRef and goalDialogSessionId, all leave the shell. AppShell keeps two lines of wiring.

That is one ownership leaving one place and arriving in another, not a copy. Move noise (~110 lines) is the dialog relocating into features/goals/ui/ and isLiveGoal moving to model/live-goal.ts.

The controller being larger than the hook it replaces is explained rather than hand-waved: it absorbed the behaviour that used to live inline in the shell, so the two are not comparable line-for-line.

Semantic parity was checked point by point rather than asserted: subscription filtering (including the broadcast undefined case), session-switch ordering, the sequence-number race guard, control re-entrancy, dialog keying against the wrong session, and the GoalState type resolving to the same underlying type through a different path. One behavioural difference was found and it is an improvement: the indicator moves from an IIFE rebuilding a closure every render to a memo keyed on the active goal, so the reference is now stable.

No second implementation survives. We verified this at the gate independently: window.maka.goal has zero hits in production code on this head, use-session-goal.ts no longer exists in the tree, and the only remaining references to the old surface are stories pointing at the deliberate testing export.

One thing worth recording without grading it: the adapter's subscribeChanges collapses events down to sessionId | undefined, discarding metadata beyond the reason. That is the right boundary for what goals need today, but if goals ever need another session reason — a removal, say — the adapter is where that has to be reopened. Noting it as a known edge of the seam, not as a defect.

CI is terminal green on this head (test: completed / success), which is the head this approval is bound to.

Approving.