Skip to content

refactor(desktop): seal AppShell capability boundaries and move state to regional owners (R2) #4582

Description

@chihumyum

Purpose

Continue R2 by moving state to its actual regional owners and closing the access paths that allowed it to accumulate in AppShell. Each migration must remove an old capability as well as move its implementation: whole-state reads, unscoped subscriptions, arbitrary setters, or lifecycle resources owned by the shell.

This issue continues #3439 after the R1 foundation (#4088, corrected by #4249). This refresh replaces the September 3 task inventory with an audit of main at 53f566b41f8b3c540b2e8717e0c324f46e002368 (2026-09-29; rechecked against GitHub on September 30). The historical discussion below remains useful, but its implementation inventory, WorkHub assumptions, and old performance measurements are not a current baseline. Progress since this refresh is recorded in the last column of the baseline table and in the module checklists.

AppShell should compose regional owners and retain the small layout, locale, navigation, and cross-region projections it actually renders. This issue does not require zero hooks or zero legitimate shell state.

The structural goal is that ordinary callers cannot recreate a migrated dependency using an allowed public interface. Doing so would require an explicit change to the public contract or dependency boundary. This does not claim that types can prove all runtime behavior or that future architecture changes can never introduce defects.

Current baseline

Inventory 2026-09-03 6c8e749d3 2026-09-16 3dcf5a125 2026-09-29 53f566b41 2026-09-30 9b089f58e 2026-10-02 c7fa6bb6a 2026-10-03 229e1b466 2026-10-04 7c90bac2d
app-shell.tsx lines / import statements, including type imports 3,324 / 105 2,829 / 89 2,400 / 84 2,154 / 82 2,019 / 79 1,760 / 73 1,589 / 65
AppShell + AppShellContent hook inventory / call sites 42 / 78 35 / 61 33 / 55 29 / 44 28 / 42 25 / 30 19 / 24
Direct window.maka paths / references in app-shell.tsx 32 / 33 18 / 18 12 / 12 10 / 10 5 / 5 0 / 0 0 / 0
Registered controllerOwners 0 6 9 12 13 15 15
Feature directories 9 17 19 20 21 21 21

These are structural inventories, not performance scores. The independent hook gate and renderer architecture gate both remain in force. References inside legacy helpers remain debt even when the shell has no direct bridge call.

Completed work

The baseline also includes #5815: edit/resend now uses the tested createRevisionAwareOnSend/revisionAwareSend seam in features/conversation/controller/composer-submit.ts. Reuse this seam; its extraction does not move the shell-owned React draft/staging state.

Session controls: the Plan controller moved below PlanProvider in #5851; the shell's remaining useSessionSettingIntent call is the equality-selected read noted above.

What the current code changes about the plan

  • Ordinary text deltas are already isolated from the shell through low-entropy live-turn summaries and reader-local transcript subscriptions. Preserve this boundary.
  • useShellLiveTurn is now a pure derivation. useAppShellTurnPresentation retains a ref-backed cache. Moving these names alone does not remove a render source.
  • The shell still selects whole per-Session maps for errors/restores, retry, stop, interactions, and queue. Most consumers only need the displayed Session. The workspace hook also independently selects the whole queue map. (Resolved by refactor(desktop): scope Session UI reads and seal construction boundaries #5850: fixed-purpose Session-bound readers; both whole-queue subscriptions removed.)
  • The Conversation public entry still exposes the state implementation/controller and general selectors. Narrowing today's call sites without retiring those capabilities leaves a path back to the same coupling. (State constructors, the whole-state getter and arbitrary selectors were removed by refactor(desktop): scope Session UI reads and seal construction boundaries #5850/refactor(desktop): own Conversation publication and observation #5869 and sealed with featurePrivateModules; the entry still exports staging/queue hooks and several export * modules, tracked under M0 and M3.)
  • WorkHub now has a separate renderer and feature-owned controller. Its main-window remainder is enablement/navigation/docking integration, not the original coordination-router block.
  • Feature behavior and scope tests already exist under apps/desktop/src/main/tests. Test location is not evidence of missing feature coverage.
  • Host execution projection is authoritative. See the implemented conversation ownership contract. This refactor must not recreate a renderer execution state machine.

Delivery modules

These are delivery boundaries, not six mandatory new features/providers or six fixed-size PRs. Claim a concrete slice and identify overlapping work before implementation. Move real consumers and retire the corresponding old capability in each slice.

M0. Close public capability and dependency boundaries

Progress: #5850 added the monotonic featurePrivateModules rule (private construction modules cannot be imported or re-exported outside their feature); #5851 and #5869 extended it and the Conversation README lists the remaining transitional capabilities with their removal modules. #5934 added the symbol-level rule (generated rootSymbolUses: a root zone may take an existing export only if it already used it at the base; new exports and one-way moves out of appShell are admitted and listed) and moved seven test-only exports to testing.ts. The first item stays open until the remaining Conversation transitional exports are gone.

Composition receives assembly surfaces and required narrow projections/commands. Regional readers receive read-only ports for their target and purpose. Command callers receive named operations, not setters. Cross-feature contracts belong in existing shared application contracts where genuinely shared and are injected through composition; features must not import each other's implementations.

Completion: the migrated capability is unavailable through permitted root imports. M0's full export cleanup completes incrementally with M1–M5, rather than blocking them on an up-front redesign. Static enforcement covers declared access paths; it is not a proof that an arbitrary returned object is semantically narrow.

M1. Bind Session reads and notifications to their scope

  • Invariant: A reader is notified only when its declared observable projection changes. Relevant values remain current, legitimate aggregate readers still update, and all projections use the same authoritative facts.

  • Current architectural assumptions: The existing Conversation authority contains per-Session state, requested/published/owner identities are distinct, and React readers require coherent stable snapshots. Session-by-update-category routing is a proposed mechanism, not a required data structure.

  • Redesign trigger: An accepted decision changes the state/reactivity model, target identity, or the meaning of a global summary. Re-evaluate the port keys, projections and notification mechanism; reuse an equivalent existing isolation mechanism instead of adding another index.

  • Replace whole-map reads with fixed-purpose queue, interaction, pending and load/restore read ports bound to the appropriate Session. Do not expose a global snapshot or accept arbitrary selectors through those ports.

  • Address both useAppShellSessionUiReads and the queue subscription inside useAppShellSessionUiState. Preserve requested, published and owner Session identities, including shared-session behavior.

  • Let the rail subscribe directly to its legitimate global streaming summary. Keep commands reading current authoritative facts when invoked, independently of React subscriptions.

  • Provide notification isolation for the declared target and projection inside the existing authority. Session/update-category routing is the current proposal; reuse an equivalent existing mechanism where available. Publish coherent snapshots before callbacks; preserve stable snapshot identity, deletion/clearing semantics and aggregate consistency.

The existing state maps may remain internal. Read ports and any notification indexes must not become duplicate business stores. First ship narrow ports with actual consumers, then implement the required notification isolation if separate review is useful; both are required to complete this module. Leaving every UI reader to filter broad notifications is only an intermediate state. No particular index structure or new reactive framework is required.

Status: complete in #5850. The temporary root reader useAppShellSessionUiReads is named in the Conversation README and retires with M3.

Completion: a B queue update does not notify an A queue reader; A text updates do not notify that queue reader; relevant values and rail summaries still publish correctly. The two whole-queue subscriptions disappear. Any temporary narrow root reader is named and retires with M2/M3.

Evidence motivating the work: an audit-only React harness compiled the production hook/selectors at the baseline above. Each of six inactive-Session map update cases produced 20 consumer renders for 20 separately committed updates, versus zero for a scoped selector prototype. Active values remained current. Ordinary text deltas with unchanged summaries reached neither reader. This establishes avoidable subscription scope, not full-AppShell CPU, elapsed latency, or a completed implementation of the proposed notification routing.

M2. Own Conversation publication and observation regionally

  • Invariant: Execution facts have one authority, content retains its original identities, observation resources have a coherent owner, and results from an obsolete scope cannot publish into a new one. Root composition has no arbitrary publication access.

  • Current architectural assumptions: Host execution projections arrive through the existing Main/preload observer; Catalog owns selection; requested selection and published content can differ. Regional ownership reuses that chain and the existing Conversation state.

  • Redesign trigger: An accepted decision relocates transcript publication or observation responsibility across Host/Main/renderer, changes the publication/selection contract, or shares observation lifetimes across surfaces. Re-design the affected lifecycle seam before moving it; do not duplicate the old and new authorities to bridge the change.

  • Move transcript publication, load/recovery/transient state and actual message readers below a Conversation owner, using the existing Catalog and Conversation authorities. Do not duplicate selection state.

  • Keep transcript-open, observation seed/retry, content handoff and disposal as one coherent lifecycle using the existing Main/preload observer. Do not split the lifecycle and add a second coordinator to reconnect it.

  • Make publication pass through an owner-controlled commit boundary bound to its captured identity and scope/version. Remove public message setters, range refs and transient-map mutation access from the shell.

  • Expose only required chrome projections and explicit targeted commands. Move the existing interaction, compact and landmark consumers with their narrow services. — refactor(desktop): own context compaction in Conversation #5893, refactor(desktop): own Composer readiness, new-task choices and submission below the shell #5935, refactor(desktop): retire AppShell's direct Desktop bridge (R2 M5) #5936

Status: complete. #5869 delivered the first three items; #5893 (compaction), #5935 (interaction and transient commands below the Composer submission owner) and #5936 (turn landmarks through ConversationServices.sessions) finished the fourth.

Completion: AppShell owns neither transcript publication nor observation resources and cannot mutate them through its public contracts. Actual readers stay below the owner; a complete model returned through context/props does not qualify. Keep requested/published handoff and per-renderer/scope lifetimes explicit.

Coordinate with #5712/#5627. History budgets, storage, IPC and Host reconstruction redesign are separate work.

M3. Own drafts and submission at the persistent Composer boundary

Completion: Shell no longer owns draft/staging/readiness state or has arbitrary draft mutation access. Navigation does not cancel the identity or recovery responsibility of an already-dispatched command with an unknown outcome. Draft, observation and send lifetimes remain distinct; type branding alone cannot enforce expiration.

Status: complete. #5868 moved staging, #5935 the submission side and #5954 the read side: useAppShellSessionUiReads and the invocation-time message read are retired, the Turn reads live in the regions that render them, the external composerRef is replaced by named Composer edits, and a withdrawn send's staged context returns to the Session it left. useActiveExecutionBoundary, useSessionSettingIntent and useShellChatModel are retained as cross-region rows.

Stage reader/staging migration before submission/recovery migration if useful. #5058, #5274, #5581, #5513, #4138 and #5405 (opened 2026-08-29 to 2026-09-22, all conflicting, none reviewed as of 2026-10-02) do not block M3: the ownership work proceeds on main and those PRs rebase onto the new owner. Their product semantics are not part of an ownership PR.

M4. Own Plan state and readers together

  • Invariant: Plan readers and mutations have explicit ownership; only eligible current-scope results publish, and actions retain their original approval/resume targets. The shell does not receive a full Plan model.

  • Current architectural assumptions: Plan is currently Session-scoped, its existing query gate/scope/retry behavior remains valid, and Session Settings already owns settings writes. Locating the Plan owner inside Conversation is the current implementation preference.

  • Redesign trigger: An accepted decision makes Plan a separate domain, changes its target identity or approval/execution protocol, or reallocates settings-write ownership. Revisit placement and contracts; an independent Plan feature is acceptable if it preserves the ownership and publication constraints.

  • Move usePlanModeState and proposal/execution readers to a Plan sub-owner within Conversation by default. A new feature/package is not required.

  • Inject narrow Plan services and reuse existing Session Settings write intents through contracts/composition. Do not introduce cross-feature implementation imports.

  • Retain latest-read wins, automatic-query eligibility, captured Session scope, confirmation and approval/resume retry inputs at the owner-controlled write boundary.

  • Register the controller owner and remove the shell's full Plan model, controller call and corresponding prop threading.

Status: complete in #5851 (PlanProvider, PlanChatView, PlanExecutionSurface, PlanServicesProvider).

Completion: the real Plan readers and state live below their owner; the shell cannot recover the full controller through an exported hook. Coordinate with #5447's Plan E2E work. This is a bounded ownership improvement, not a claim that Plan is the largest performance hotspot.

M5. Finish root composition and retire legacy access

At 229e1b466 app-shell.tsx has no direct bridge path: transcript attachment bytes and turn landmarks go through Conversation services and WorkHub enablement and onboarding through application authorities (#5936); the Error Boundary, command-palette and About reports go through features/diagnostics (#5937).

Completion measure (M3 + M5). M5 is closed by numbers, not by judgement. The figures come from the generated legacyAppShell.files section of apps/desktop/renderer-architecture.json (the 16 AppShell-family legacy files the architecture checker already measures) and from the AppShell hook gate.

Measure Source 53f566b41 9b089f58e c7fa6bb6a 229e1b466 7c90bac2d Done when
AppShell-family bridge references sum of bridgePaths 55 48 43 1 1 0, apart from the exempt E2E fixture's e2eFixture.getState
— of which in app-shell.tsx app-shell.tsx bridgePaths 12 10 5 0 0 0
AppShell-family action factories actionFactories 8 6 6 1 1 0 business factories; only createAppShellE2eFixtureActions may remain
Transitional Conversation capabilities "Remaining transitional capabilities" table in features/conversation/README.md — 3 4 2 0 0 rows, exports removed
AppShell hook-gate entries without a retained-root row hook gate vs. retained-root table — — 28 (no table yet) 0 0 0

Retired outright (must be absent from the hook gate): useAppShellSessionUiReads, useTaskSubmissionReadiness, useNewTaskChoice, useOnboardingSnapshot, and the new-task/send/WorkHub-enablement state (newTaskSendPending, newChatPlanModeActive, newChatOrchestrationMode, workHubEnabled).

The retained-root table lists every hook still in the gate with its consumer, owner, allowed capability and the reason it stays at the root (locale, navigation, layout, a cross-region command, or an application lifecycle). The table lands together with an architecture-checker rule that fails when a gate entry has no row, so it cannot drift.

At 7c90bac2d every measure is at its target: the only family bridge reference and action factory are the exempt E2E fixture's, the Conversation transitional table is empty, and the 24 retained-root rows (application lifecycle 5, cross-region command 7, layout 4, locale 3, navigation 5) carry a root reason with none scheduled for removal. npm run check:renderer-architecture -- --report prints these figures.

Not part of R2, and not tracked by a follow-up issue: deleting or renaming app-shell.tsx itself, legacy files outside the AppShell family (the ledger's closure list), the rootDebt files (app.tsx, main.tsx), Host connection fan-out, and performance work. The existing no-growth ratchets keep them from regressing, performance has its own issues (#5627, #5712), and the closing comment lists them. A later effort that takes one of them on opens its own scoped issue.

Completion: the root has no direct window.maka access or access to migrated state controllers/full models. Remaining layout, locale, navigation and composition needs have explicit legal homes. This does not require all legacy renderer files or all hooks to disappear.

Dependencies and merge order

Start with a small M0 + M1 consumer migration, then finish M1's scoped notification implementation. M4 can proceed after its M0 contracts are defined. M2 and M3 share the target-identity contracts; their work can be divided, but final Composer integration depends on the Conversation publication/command seam. M5 closes after M1–M4. Serialize shared AppShell/public-entry/ledger integration even where work is otherwise independent.

Boundary protection ships with each slice, not only in M5. The modules are complete only when both their replacement and their old-access removal are complete.

Handling new architecture decisions

Each module records its invariant, current architectural assumptions and redesign trigger above. Before implementing a module, refresh the relevant source/PR heads and compare those assumptions with accepted architecture decisions. Concrete owner names, file locations, notification data structures and the suggested merge order may evolve while preserving the invariant.

When a trigger occurs, record the decision link, affected modules, superseded assumption and replacement boundary in this issue or the implementation PR. Re-plan only the affected work; keep independent completed slices. Ordinary renames or additive fields do not by themselves reopen the whole design. If an accepted product/architecture decision intentionally changes an invariant, explicitly revise the scope and acceptance contract before claiming the migration complete; do not silently restore a broad compatibility interface.

Definition of done for a slice

  1. Confirm the module's invariant, architectural assumptions and redesign triggers against the implementation base. Record the triggering events, authority, write owner, actual readers, allowed public capabilities and mount/resource lifetimes. Demonstrate a concrete improvement in isolation or ownership.
  2. Name which caller loses which capability. Delete the corresponding old exports, setters, subscriptions, threading and helpers as their consumers move. Recreating the migrated coupling must require an explicit public-contract or dependency-rule change.
  3. Move controller call sites with state ownership and use controllerOwners where applicable. Reuse existing authorities for reader-only changes. Keep zone rules and both no-growth ratchets. Do not create a controller merely to register one.
  4. Explain what is enforced by types, module/import restrictions, notification routing and owner-controlled runtime admission. Do not claim that an AST rule or a branded type proves runtime semantics. Do not substitute additional scenario tests for closing the old access path.
  5. Verify the implementation of these boundaries and preserve existing behavior/native/IPC acceptance. Actual consumer values, intended updates, cleanup, captured targets and late-result fencing remain required.
  6. Tie performance claims to the tested SHA, workload and path. Render counts are not elapsed time. Separate selection feedback, transcript-open, IPC delivery, first content frame and settled display; follow same-instance paired measurements. Old perf(desktop): the Session rail re-renders on every AppShell commit #4109 numbers are historical context only.

The hook inventory and architecture ledger remain reviewable evidence of structural progress. They do not require artificial zero counts or justify weakening guards. Pure derivations, real layout/locale state, and explicit cross-region commands may remain at their appropriate composition boundary.

Invariants and validation boundaries

No product, visual, storage-schema, IPC, copy or shortcut changes in an ownership PR. No new global state library, service locator, duplicate Catalog, generic shell context, execution state machine, or duplicate Host observer.

Host execution identity, message delivery, observation availability, and content presentation remain distinct. Preserve unknown delivery outcomes, Turn/Message/Run targets, scope fencing, transcript handoff, draft ownership, and Workbar resource lifecycles.

Use feature/controller tests for pure state, selectors and ownership. Keep Electron coverage for WorkHub native docking/floating windows, WebContentsView continuity, focus/menus, preload/Host observation and reconnect behavior. Do not delete workhub-layout, workhub-reconstruction or streaming-remount wholesale under the old assumption that they are all DOM-only tests.

Separate follow-ups and closure

Keep both hook gates while their convergence is discussed separately; the independent gate's manual-inventory friction was deliberate. Comparator-only ratchet hardening is a separate review scope. Neither change should be bundled into a reader-ownership PR.

Close R2 when M0–M5 are complete: scoped Session notifications and regional owners are in use; migrated broad exports/transitional exceptions are removed; the root has only explicit permitted capabilities; and retained application lifecycles are accounted for — measured by the M5 completion table above. Each original coupling must be unavailable through the allowed interfaces. Do not claim completion while unresolved business ownership merely sits behind a renamed facade.

Host connection fan-out optimization requires separate evidence before consolidation. Preserve target provenance, startup seeds, same-Host stale-while-revalidate and late-response attribution. A new state library, general actor/scope framework, duplicate observer, full history redesign or elimination of every legacy renderer file is not a prerequisite for this issue.


This refresh, module design and bounded selector-scope investigation were prepared with OpenAI Codex. The design is not an implemented migration, and the audit did not perform a new full-application performance benchmark. Implementation, review and merge decisions remain with the human contributors.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions