refactor(desktop): move App Update controller below AppShell

[chihumyum]

Summary

Move the renderer-owned App Update lifecycle out of AppShell into a feature slice, with AppUpdateProvider as the sole production owner of useAppUpdateController.

• compose update capabilities through a typed Desktop adapter and feature service port; AppShell now only mounts the Provider
• project update state independently to About and Sidebar Footer, so progress events no longer invalidate navigation or the shell
• keep the shared update contract type-only while preserving subscription-before-snapshot ordering, stale-result fences, and single-flight check/install behavior
• add AST-backed boundary tests that pin the controller, Provider, adapter, and leaf readers, while rejecting raw or computed bridge access, internal imports, aliases, namespaces, re-exports, export-all, and dynamic imports
• keep the About page as fix(desktop): reduce the About page to rows of one shape #4857 left it, moving only update ownership: update state is not the page's to own, so the feature holds the renderer's sole updater subscription above AppShell and publishes About's projection, and the row reads status from AppUpdateAboutProjectionConsumer and issues the feature's guarded check instead of touching the bridge; the row's copy comes from main's aboutUpdateRow

No user-visible copy, update protocol, or install behavior is intended to change.

Refs #4582 (supersedes #3439)

Performance evidence

Measured in Electron 43.4.1 with a production React bundle, the same renderer PID, and alternating legacy/scoped runs:

Controlled downloading-status workload	AppShell owner	Feature owner	Change
React commits per push	1	1	unchanged
Rendered fibers per push	512	1	-99.8%
Pure renderer/controller busy JS	0.1329 ms/push	0.0068 ms/push	-94.9%
Full IPC -> preload -> adapter -> Provider busy JS	0.2111 ms/push	0.0696 ms/push	-67.0%

The structural run used 8 rounds x 40 pushes, the pure renderer run 9 alternating rounds x 1,000 pushes, and the full propagation run 7 alternating rounds x 200 pushes. The full path used a real BrowserWindow, Electron IPC, the production preload bundle, Desktop adapter, and App Update Provider; only the main status producer was a fixture.

These measurements cover controlled renderer update propagation. They do not claim faster updater downloads or installs, lower main-process/network/disk cost, total wall/FPS improvement, or an exact production AppShell fiber count.

Verification

• npm run rebuild, npm run typecheck, npm run lint, and npm run format:check
• App Update focused suites: 22/22
• Desktop and UI full compiled test suites
• renderer architecture fixtures: 61/61; AppShell hook gate: 42 hooks / 77 call sites
• Astryx inventory: 239 files / 1 declared exclusion
• Knip for apps/desktop and packages/ui, ASF headers, stale-dist, and git diff --check
• the default full-workspace sweep's only local non-green signal was Node's experimental SQLite warning from a managed Storage child; with NODE_NO_WARNINGS=1, Storage passed 1,077 tests with 8 skipped
• node scripts/desktop-real-window-smoke.mjs --programmatic-only --diagnostic-wait-ms 20000 passed against the built renderer: visible 1280x840 movable/resizable window, renderer ready, appFrame mounted, Search focused, dock visible, and no ErrorBoundary
• manual real-window inspection after closing Search showed sidebar, tasks, conversation, and composer rendering normally without a crash or ErrorBoundary
• built Settings Storybook play clicked "检查更新", observed "已是最新版本。", and reported no page or console errors

The real-window smoke used a source-built Electron app, not a packaged or signed installer; packaged updater download/install remains outside this PR's evidence.

Review focus

Please review the ownership guard and the two independent projections together: the feature should retain the only update subscription and bridge capability, while About and Sidebar Footer receive only the state needed at their render boundaries.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex contributed substantive architecture design, implementation, tests, performance measurement, and verification. The human contributor reviewed the work and chose to submit it.

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

[chihumyum]

@Astro-Han could you please review exact head d5cd6e9ab2e8b47f1b9e9eddd289c1302b658799?

I rebased it onto main@b714a392192961fef3d89d4358fb0a0858252804. The conflict resolution keeps the new Session Settings / Runtime Host composition alongside the App Update provider and regenerates the renderer architecture and Astryx ledgers from the combined source.

Exact-head local verification passes: rebuild; renderer architecture 71/71 plus the base ratchet; App Update 22/22; Desktop 1,982/1,982; UI 313/313; typecheck, lint, format, Desktop/UI Knip, ASF headers, AppShell hook scope, Astryx inventory; and the real Electron window smoke. The new hosted CI and Windows package checks are currently queued.

[chihumyum]

Rebased onto current main (c86da40d7, which includes #4571's About page rebuild). Exact head: 076fb612f579e0ed890b082b8e488bf134a9a793.

How the two real conflicts were resolved:

• about-settings-page.tsx is refactor(desktop): rebuild the About page as labeled settings groups #4571's page verbatim; only update ownership changed. The status row is now AboutUpdateRow, rendered through AppUpdateAboutProjectionConsumer, and the page no longer calls window.maka.app.*. The dev-build decision (no status line) stays on the page as refactor(desktop): rebuild the About page as labeled settings groups #4571 put it.
• aboutUpdateStatusDetail follows main's two-argument signature (no isDevBuild) and lives in the feature model; aboutChannelFacts stays in settings/about-update-status.ts because it is About copy, not update state. about-update-status.test.ts imports each from its owner.
• Stories: About is back to main's dev-checkout form (its e2e asserts no update button); the previous play moved to a new AboutCheckForUpdates story on a packaged release. The story-side services read window.maka.app per call so the channel stories' updateStatus overrides reach the provider.
• renderer-architecture.json and the Astryx inventory were regenerated.

Local verification on this head with Node 24: desktop test:dist 1988/1988, @maka/ui 313/313, typecheck (four projects), lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks (42 hooks / 75 call sites), Astryx inventory, Knip (desktop, ui), ASF headers, git diff --check, and build:renderer — all green.

Automated update from Claude Code on behalf of the PR author.

[chihumyum]

Rebased again onto current main (ad18da42c, #4455). One real conflict, in packages/ui/stories/session-rail-harness.tsx: #4455 added the railSelection prop and forwards it to SessionRailProvider, this PR wraps that provider in SidebarUpdateProjectionProvider; the merge keeps both. The Astryx inventory was regenerated. Exact head: 750ee054c66babfe2107279a9bb3d979bbbaaf31.

Same local verification on this head with Node 24: desktop test:dist, @maka/ui test:dist, typecheck (four projects), lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks, Astryx inventory, Knip (desktop, ui), ASF headers, git diff --check, build:renderer — all green.

Automated update from Claude Code on behalf of the PR author.

[chihumyum]

Rebased onto main 68cda0b54. The only non-generated part of the resolution is the ledger's hand-authored ownership policy: regeneration keeps policy fields from the base, so the app-lifecycle → app-shell-app-update.ts entry (this PR deletes that file) was removed again by hand, as in the original commit. Exact head: 8cc5de4fbd82d89aadddda60984c0a5989cdc27b; the full local verification is green on it.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

Rebased once more onto main b9748a773; the ledger was the only conflict, regenerated with the same hand-authored ownership edit replayed. Exact head: 8a30f7511ddf834be2526e6bca63a82a56e975da; the full local verification (desktop and ui suites) is green on it.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

Rebased onto main 2c948e4e3 (after #4315) and folded in the three follow-ups this PR was waiting on:

• Registered with the controllerOwners guard: useAppUpdateController → AppUpdateProvider (AppShell already mounts the provider directly, so no structural change was needed). The retired app-lifecycle → app-shell-app-update.ts ownership entry is removed again by hand, as before.
• Adopted refactor(desktop): share the feature services context and pass bridge namespaces through #4590's shared services context: features/app-update/services-context.tsx is now the four-line createServicesContext form; exports and the error message are unchanged.
• Absorbed feat(i18n): add Traditional Chinese locale #3853 (Traditional Chinese): the feature copy catalog is keyed zh-CN / zh-TW / en (the zh-TW strings are the ones feat(i18n): add Traditional Chinese locale #3853 had added to the shell catalog for these keys, which this PR removes from there along with zh-CN and en), localizedAppUpdateErrorMessage delegates to generalizedErrorMessageForLocale like the shell copy does, and aboutUpdateStatusDetail takes feat(i18n): add Traditional Chinese locale #3853's { isDevBuild, errorDetail } options so the About row localizes a raw updater error the same way main does. The AppShell conflict was the Session rail mount: main's Module Hub boundary form, minus the two update props this PR retires.

Exact head: a54ef209dfb5e7a568a10b0463e2a5b32f585492. Full local verification on it with Node 24: desktop test:dist, @maka/ui test:dist, typecheck (four projects), lint, format, check:renderer-architecture --base upstream/main (validating both registered owners), check:app-shell-hooks, Astryx inventory, Knip (desktop, ui), ASF headers, git diff --check, build:renderer — all green.

[chihumyum]

Rebased onto main cbeb1a93c; the Astryx inventory was the only conflict and was regenerated (the ledger merged cleanly, with both registered owners and the retired ownership entry as before). Exact head: 08c27efda2d3960fc6ab37026158b17a565dd40c; the full local verification is green on it.

[chihumyum]

The run on 08c27efda failed only in Desktop e2e, on session-workbar.spec.ts:54 ("the composer usage action opens Task trace in the right workbar"), with everything else green. That spec was rewritten by cbeb1a93c (the workbar shell rebuild this head sits on) and exercises the right workbar, which this PR does not touch; on this head it passes locally three times in a row (24/24 across the file). main's own run for cbeb1a93c is red in the same e2e job on a different spec (transcript-scroll-cost.spec.ts:239). Re-pushed with identical content to re-run; exact head 96a84dc5d884d71a238d0320e6b5e3c62ea08fcd.

[Astro-Han]

Reviewed at 96a84dc. All four done-conditions from #4582 hold: the two effects, one state and two refs leave the shell render body (check-app-shell-hooks.mjs 14→12 / 15→14), the four window.maka.* update paths leave app-shell.tsx and the three in about-settings-page.tsx go with them, useAppUpdateController → AppUpdateProvider is in controllerOwners, and the same PR deletes app-shell-app-update.ts, the nine shell-copy keys, the updateReminder / onOpenUpdate threading through SessionRailChrome, and the second copy of AppUpdateStatus in preload and main. The hand-removed ownership entry is the one the checker would otherwise reject (check-renderer-architecture.mjs:2346), so the ledger edit is consistent with regeneration.

The case for the move is stronger than the earlier slices: main had two subscribers to the same status (AppShellContent and About), and the shell one fed download progress into a useState on the renderer root, which is what the 512 → 1 fibers number is measuring. It also fixes a real ordering bug on the About page: the old effect read before it subscribed with no fence, so a slow updateStatus() could overwrite a newer push. The revision counter and the subscribe-then-read test pin that. I compared the nine copy keys across zh-CN / zh-TW / en against the deleted shell-copy.ts entries: identical.

No P0–P2. Five small things, none blocking, take what you want in one pass:

• app-update-projection-context.ts gives the About projection a silent default (inactiveAboutProjection), where module-hub-provider.tsx:118 throws Provider is missing. A stray About mount outside the Provider then sits on "not checked yet" forever and the button toasts. Make the desktop context nullable and throw on read; the packages/ui one can keep its default since it explicitly serves other hosts.
• Three cases in app-update-boundary.test.ts add no constraint: exclusive owner and index/testing-only entry are what check-renderer-architecture.mjs:1865 and :2087 already enforce, and the "recognizes alias / namespace / re-export / dynamic import" case tests the helper against inline strings. Keep the cross-package sidebar binding, the raw-capability ban and the mount-point checks; drop those three (~70 lines). Same point we settled on #4315.
• isDevBuild is dead in about-status-detail.ts (its own comment says there is no dev-build branch) and this PR threads it through a new AboutUpdateRow prop. Delete the option, the prop and the call site.
• localizedAppUpdateErrorMessage in app-update-copy.ts only forwards to generalizedErrorMessageForLocale, the same shell it copies from shell-copy.ts. Call the original.
• create-app-update-services.ts hands the feature the whole bridge.app namespace (icons, sessionProjectInfo, info included) and the adapter test asserts identity on it. create-goal-services.ts closes over the methods it needs; do the same here and drop the === assertion.

Two things before merge. The branch went CONFLICTING a few minutes ago because #4640 and #4823 both landed on renderer-architecture.json; please regenerate rather than hand-merge, and replay the ownership deletion. And test / package were still pending on this head when I read it; the approval stands across the rebase per CONTRIBUTING, but the exact head that merges needs green CI, including the session-workbar.spec.ts line that was red on main.

#4491 shares the ledger, app-shell.tsx, the hook gate and the Astryx inventory with this PR, so whichever lands second regenerates once; I would land this one first since it deletes more.

Manual acceptance still owed on a packaged build, both themes: About page across the eight update states with the percentage moving during downloading; the check button disabling and recovering, including closing and reopening Settings mid-check (the one semantic shift, since checking moved from the page to the Provider); the dev checkout still rendering no status row; the sidebar footer button appearing only for downloaded and versioned error; the install-failure toast and the running-tasks confirm, three locales.

Evidence boundary: static read at 96a84dc against main; no tests, build, Storybook or app run, so the fiber counts and suite results are the body's, not mine.

AI-assisted review: drafted with Maka; I verified the four done-conditions, the copy parity, the ownership edit and the checker overlap myself.

[chihumyum]

Rebased onto main efbcc6c08 (ledger regenerated, the owner registration and the ownership deletion replayed) and took all five items in one follow-up commit, 17ed533d2:

• The About projection context is nullable and AppUpdateAboutProjectionConsumer throws AppUpdateProvider is missing on a read outside the Provider; the packages/ui sidebar context keeps its default since it serves other hosts.
• The three boundary cases the checker already proves are gone (escape-hatch helper, exclusive owner, index/testing-only entry); the sidebar binding, the raw-capability ban, and the mount-point checks stay.
• isDevBuild is deleted from aboutUpdateStatusDetail, AboutUpdateRow, and the call site; the options object is now just { errorDetail? }.
• localizedAppUpdateErrorMessage is gone; the Provider calls generalizedErrorMessageForLocale directly.
• create-app-update-services.ts closes over the five capabilities the port names instead of handing the feature bridge.app; the adapter test now asserts each call reaches the bridge and the exact key set, instead of identity.

Exact head: 17ed533d2db16108ff85aae8d31ca46eb3b095b2. Full local verification with Node 24: typecheck (four projects), lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks, Astryx inventory, Knip (desktop, ui), ASF headers, git diff --check, build:renderer, @maka/ui test:dist — green; desktop test:dist 2181/2182, where the one failure is shell-env.test "kills login-shell descendants when capture times out", a main-process case this PR does not touch that fails identically on plain main in my environment today after passing earlier in the day, so CI arbitrates.

The packaged-build manual acceptance you listed is still owed and is on the author.

[chihumyum]

@Astro-Han this needed a real re-merge, so please re-look at the About page before merging: your #4857 rebuilt about-settings-page.tsx and moved the update row's copy into aboutUpdateRow in settings/about-update-status.ts. The branch now sits on main c0229b0c6 with:

• About page: your rows-of-one-shape page verbatim, with only update ownership changed. The update row is an AboutUpdateStatusRow component rendered through AppUpdateAboutProjectionConsumer; it feeds update.status into your aboutUpdateRow (same errorDetail localization) and issues update.checkForUpdates() under the action guard with your toast semantics (copy.updateFailed[status.operation] / .check). isLoading is update.checking || row.action === 'checking'; the dev-checkout branch, the button-only-where-a-check-is-honoured rule, and app.info stay exactly as on main. No window.maka.app.* update path remains in the page.
• Feature model: model/about-status-detail.ts is deleted — your aboutUpdateRow supersedes it, and it is About copy, so it stays in settings next to aboutChannelSummary. about-update-status.test.ts is main's version unchanged.
• Stories: main's file plus the provider wrapping inside SettingsStory's ToastProvider, the per-call settingsAppUpdateServices, and the AboutCheckForUpdates play (now asserts the row label 已是最新版本). Your About / AboutNightly / AboutRelease stories are untouched.
• The two review-follow-up commits are squashed into the one PR commit; the five items are still in.
• Ledger regenerated; useAppUpdateController → AppUpdateProvider and the app-lifecycle ownership deletion replayed.

Exact head: bb04383c9c1cd2875b13a3fbd915bb51b67a571e. Local verification with Node 24: desktop test:dist (all green, including the About page, app-update, and boundary suites), @maka/ui test:dist, lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks, Astryx inventory, Knip (desktop, ui), ASF headers, git diff --check, build:renderer — green. typecheck fails only in stories/app-shell.stories.tsx on rightCollapsed, which is main's breakage after #4694 that your #4895 fixes; I will rebase once more when it lands so the merging head has green CI.

Posted by Claude Code on behalf of the PR author.

[chihumyum]

#4895 landed, so this is rebased once more onto main e226ac532. No conflicts; the ledger and the inventory are byte-identical to the previous head. Exact head: 1ea93eaf4. Local verification with Node 24 is now fully green, typecheck included (desktop test:dist 2300/2300, lint, format, check:renderer-architecture --base upstream/main, check:app-shell-hooks, Astryx inventory, Knip, ASF headers, git diff --check, preload/overlay builds).

Posted by Claude Code on behalf of the PR author.

[chihumyum]

@Astro-Han #4491 landing first put this branch into a real conflict, so one more look is needed. Rebased onto main 083027afa; exact head: 4a7b99e6b. What was resolved by hand:

• app-shell.tsx: both wrappers are now mounted, <AppUpdateProvider> outside <TaskEntry.TaskEntryRoot>, so TaskEntryRoot's render prop stays the innermost frame around AppShellContent exactly as on main, and AppUpdateProvider's children is still an element the shell built (the render-scope bail-out this PR relies on is unchanged). Neither provider reads the other's context.
• scripts/check-app-shell-hooks.mjs: useState: 14 (this PR removes one), and useTaskEntryController stays gone as refactor(desktop): move Task Entry controller below AppShell #4491 left it. The gate passes.
• Ledger: regenerated from main's side, then the useAppUpdateController → AppUpdateProvider owner registration and the app-lifecycle ownership retirement replayed; controllerOwners now lists App Update, Module Hub and Task Entry. check:renderer-architecture --base upstream/main passes.

Everything else merged cleanly and is byte-identical to the head you approved plus the #4857 re-merge described above. Local verification with Node 24 is fully green (desktop test:dist 2305/2305, typecheck, lint, format, both gates, Astryx inventory, Knip, ASF headers, git diff --check, preload/overlay builds).

Posted by Claude Code on behalf of the PR author.

[Astro-Han]

Re-reviewed at 1ea93ea against main (90014db94), clean merge, CI green. Thanks for calling out the About re-merge instead of burying it.

Interdiff from the head I approved (96a84dc): the five follow-ups are all in (ui/app-update-projection-context.ts:31-45 nullable context and the throwing Consumer; the three boundary cases dropped; model/about-status-detail.ts deleted; localizedAppUpdateErrorMessage gone in favour of generalizedErrorMessageForLocale at app-update-provider.tsx:111/129; create-app-update-services.ts:35-41 closing over the five capabilities), plus the About re-merge. Everything else is byte-identical.

About page: #4857's page verbatim with only update ownership moved. aboutUpdateRow is fed the same errorDetail localisation (:78), toast semantics unchanged (:86-95), isLoading keeps the same meaning with checking now held by the controller (:110), the dev-checkout branch and the button-only-where-a-check-is-honoured rule are copied from main (:211, :103), and the page has no window.maka.app.* update path left. The three deleted boundary tests are covered by the ledger registration (renderer-architecture.json:294-297 with check-renderer-architecture.mjs:1851-1885) and the feature-entry rule at check-renderer-architecture.mjs:2102-2112, so nothing is lost. Stories: the Provider sits inside SettingsStory's ToastProvider, settingsAppUpdateServices reads window.maka.app per call for the reason its comment gives, and the three About stories are untouched.

Two P3s, not blocking:

• about-settings-page.tsx:65-71: the first five lines of the AboutUpdateStatusRow doc comment argue for the change (who owns the subscription, why not the bridge); that belongs in the PR body. The last sentence, that a component is used because the action guard is a hook, is the one the next editor needs.
• AppUpdateAboutProjectionConsumer's throw when the Provider is missing has no test; app-update-provider-scope.test.ts only covers the mounted path. One assert.throws render case.

Manual acceptance before merge, as in the earlier approval: packaged build, the eight update states on the About row, the check button disabling and recovering, dev checkout showing no status row, the sidebar reminder only in downloaded / versioned-error, install-failure toast and the running-task confirmation, in all three locales. I will run that on this head.

AI-assisted review: drafted with Maka; I verified the interdiff, the About page against main and the replacement coverage for the deleted tests myself.

[chihumyum]

Both P3s taken in one follow-up commit on top of 4a7b99e6b, so the interdiff is the whole commit. Exact head: 7aeaca3ec.

• about-settings-page.tsx: the AboutUpdateStatusRow doc comment is now two lines, keeping only the sentence about the action guard being a hook; the ownership argument moved into the PR body's Summary.
• app-update-provider-scope.test.ts: throws for an About reader mounted outside AppUpdateProvider renders AppUpdateAboutProjectionConsumer with no Provider under act and asserts the AppUpdateProvider is missing throw with assert.throws, plus that the reader's render callback never ran.

No runtime code changed, so a packaged build from 4a7b99e6b is still representative for the manual acceptance. Local verification on Node 24 is fully green (desktop test:dist 2306/2306, typecheck, lint, format, both gates, Astryx inventory, Knip, ASF headers, git diff --check, preload/overlay builds).

Posted by Claude Code on behalf of the PR author.

[chihumyum]

Correction to the comment above: the merge went in at 4a7b99e6b a few minutes before the follow-up commit reached the branch, so 7aeaca3ec is not in main. The two P3s now land through #4905 (the same commit cherry-picked onto 00cada01c; the new case was checked to fail with the Consumer's throw removed).

Posted by Claude Code on behalf of the PR author.