chore(desktop): check R2 root symbol uses and retained root hooks

[chihumyum]

Summary

Refs #4582 (M0 items 1, 2 and 4; M5's completion measure).

This is the R2 enforcement PR. It adds the checker rules the "Completion measure (M3 + M5)" in #4582 needs, so later slices shrink recorded numbers instead of arguing them. It should merge before the remaining M3/M5 slices.

1. Root symbol uses (M0 item 2)

Allowing an import of a feature public entry does not make every export appropriate for the root. renderer-architecture.json now has a generated rootSymbolUses: per feature public entry, the runtime symbols each root zone takes from it.

The root zones are:

• appShell: the 16 AppShell-family files.
• composition: composition/**.
• bootstrap: bootstrap/** plus the guarded main.tsx and app.tsx.

At c7fa6bb6a the record holds 89 symbols across 18 entries: 66 for appShell, 23 for composition and none for bootstrap.

How uses are attributed:

• The rule reuses the checker's module graph. It attributes named and default imports, static namespace members (NS.x, NS.x(), <NS.X>) and named re-exports.

• It follows export { x } from, re-exported imports and export * through any intermediate module until it reaches a feature public entry. Two uses the old analysis could not see are now recorded: NEW_TASK_PENDING_KEY, which app-shell.tsx reads through pending-items.ts, and useAppShellSessionUiReads, read through use-app-shell-session-ui-reads.ts. JSX-only uses are counted too.

• Uses whose symbols cannot be attributed are violations rather than records:

  • a root namespace binding that escapes: passed on, destructured, spread, read by computed member, aliased with import x = NS.y, re-exported or rendered whole;
  • export * or export * as over an entry, directly or through a barrel;
  • a namespace object handed out by another module;
  • a runtime import() or require of an entry.

  There are 0 of these on main today.

• Type positions are not recorded. Deep imports stay with the existing zone rule.

How the record is maintained:

• --write regenerates it, and the tree must match it exactly.
• Against the base it may only shrink, with two exceptions, and the CLI lists each use it admits:
  • New binding. A binding the same change adds to the entry may be taken: an explicit public-contract change. The surface is measured on the materialized base tree, with export * resolved. Exports are compared by resolved binding (declaring module plus local name), so a new alias of an export the base entry already had is not new. Since 2026-09-15, 39 of the root's 40 symbol additions were new exports.
  • Move out of appShell. A use may move one way, from appShell into composition or bootstrap, when appShell gives it up in the same change. This follows the existing one-way move from the AppShell closure to the root closure. A copy or the reverse move fails.
• Taking an export the base entry already had fails, and so does removing the record.

2. Retained-root hook table (M5 completion measure)

apps/desktop/src/renderer/README.md now has a table between retained-root-hooks markers.

• It has one row per call site the AppShell hook gate allows: 42 rows for 28 entries.
• Each row gives the consumer, the owner and the allowed capability. It then gives exactly one of:
  • the root reason: locale, navigation, layout, cross-region command or application lifecycle;
  • the R2 module that removes the call (M0–M5).

How the checker enforces it:

• It reads the gate's ALLOWED literal with Babel. scripts/check-app-shell-hooks.mjs is neither run nor edited.
• It fails when a gate entry has no row, when an entry's row count differs from its call-site count, or when a row names a hook the gate no longer lists.
• Where an entry has several call sites, each row's call site must name, in backticks, an identifier of exactly one of those calls in app-shell.tsx: a binding it declares, or an identifier in its arguments. No two rows may name the same call. A row therefore cannot drift onto another useState or useStableActions call.
• So a slice that moves a hook out of AppShell edits the gate and deletes the matching rows together.

Current classification:

• Retained, 19 rows: navigation 5, layout 4, cross-region command 4, locale 3, application lifecycle 3.
• Scheduled, 23 rows:
  • M5 (6): bootstrap subscriptions, project context, the two useEffects (WorkHub enablement and the onboarding seed), useOnboardingSnapshot and workHubEnabled.
  • M3 (17):
    • every hook refactor(desktop): seal AppShell capability boundaries and move state to regional owners (R2) #4582 retires outright: useAppShellSessionUiReads, useTaskSubmissionReadiness, useNewTaskChoice, and the new-task, send and Plan/orchestration state;
    • revisionDraft;
    • the chat, turn and revision action factories;
    • Composer-facing reads that fit none of the five root reasons: useShellChatModel, useShellResume, useShellLiveTurn, useActiveExecutionBoundary, useSessionSettingIntent, useAppShellTurnPresentation, useTurnActionRegistry.

Calls that review may contest:

• useAppShellSessionWorkspace is kept as navigation, for Session selection and the catalog. Its transient and interaction commands leave with M3.
• The three useShellConnections calls are kept as application lifecycle. Host connection fan-out stays outside R2.
• useAppShellHostEffects is kept as layout. Its app.info read still has to move behind an adapter.

With this PR, the issue's "AppShell hook-gate entries without a retained-root row" goes from 28 to 0.

3. --report

check-renderer-architecture.mjs --report prints the measures #4582 tracks:

• AppShell-family bridge references: 43, of which 5 are in app-shell.tsx, with a per-file breakdown;
• action factories: 6;
• Conversation transitional rows: 4;
• hook-gate entries / call sites / entries without a row: 28 / 42 / 0;
• root symbol uses per zone;
• the AppShell closure's feature-entry uses per file: 66 in 24 files.

A --base run also prints each closure use a change adds. The closure is not a root zone, so this is reported and never ratcheted.

It only reports. These numbers fall over several PRs, and the existing no-growth ratchets already stop them rising.

4. Transitional exports (M0 items 1 and 4, outside Conversation)

• There is no export * outside features/conversation.
• Seven exports only tests or Storybook read leave the production public entries:
  • overlays: createAgentGraphPanelModel, reduceAgentGraphPanelModel and shouldShowAgentGraphPanel move to testing.ts.
  • session-navigation: SessionHistoryNavigation was already in testing.ts; the story now imports it from there.
  • workhub: workHubLinkedWork moves to testing.ts.
  • connection-settings: providerRequestUrlPreview moves to a new testing.ts.
  • startWorkHubCoordinationLifecycle and its host-change type: the test now imports them from their application contract.
• The 39 non-root exports that other production code reads (mostly legacy settings pages) stay public.
• The README lists the 10 non-assembly root exports outside Conversation with their consumer and removal module: render-prop consumers, command ports, the rail layout store and read hooks.
• Conversation's own entry is left to the M3 slice that is rewriting it.

What this does not enforce

These are static rules. They do not prove runtime semantics.

• Wrappers. The symbol rule does not see a legacy helper that wraps a feature export. Example: AppShell calls renderComposerMentionsProvider from composer-mentions.tsx, so the record has no entry for ComposerMentionsProvider.
• Non-root callers. The AppShell closure (176 files), platform adapters and other legacy files are not root zones. The closure's feature-entry uses are reported (--report, and new ones on --base runs), not ratcheted.
• Narrowness. An admitted new export can still hand out a broad object. Review decides whether it is narrow.
• Table accuracy. The checker validates the retained-root table's shape, its counts against the gate, and that each row of a multi-call hook names one real call. Whether each consumer, owner and reason is accurate stays with review. The classification above is a proposal.
• Transitional export list. It is documentation. No rule ties it to rootSymbolUses.
• Cross-check coverage. The base-checker cross-check re-measures the generated record under the base rules. It does not re-run the escape or table checks. Changes to validateMonotonicDebt remain uncovered (existing README caveat).

Verification

Head 4dbfc2c19 on main 1a66e4d5e (it merges cleanly into ab5996bdb), Node 24.19.0, after a clean npm install, the dependency patches, Electron install and build:workspace-deps. The Desktop suite, typecheck and Knip ran on c7fa6bb6a. The rebase over #5847 (Storybook pixel-diff scripts only) and the review follow-up 4dbfc2c19 (checker, its tests and README only) were re-checked with the architecture tests, the strict-base run, lint, format:check, Knip and the hook gate.

• Architecture tests: 165 passed, which is 121 on main plus 44 new.
  • Root symbol rule: one fixture per access path and 13 escape cases.
  • Growth: new-export admission, plus move, copy and reverse-move cases.
  • A git fixture runs the real CLI with --base … --strict-base --report.
  • Retained-root table: accept, inactive without a gate, 12 rejection cases, and a non-literal gate.
  • The report output.
  • Review follow-up:
    • two alias fixtures, plus an alias step in the git fixture;
    • a git fixture for the reported closure use;
    • four fixtures that bind multi-call rows to calls.
  • Two existing fixtures now declare the root use they already had.
• Fail without the change: with main's checker in place (plus empty stubs for the two new exports so the test file loads), 35 of the 37 new tests fail. The two that pass assert the absence of violations: the accepted table, and no rule without a gate. The follow-up's new tests fail on 641982ed9's checker, except the one that accepts a correctly bound table; there the alias step passes where it must fail. The checker was restored and compared byte for byte.
• check:renderer-architecture -- --base 1a66e4d5e --strict-base passed, cross-checked under the base checker. --write leaves the ledger unchanged.
• Desktop: build:test + test:dist gave 3,143/3,143. That includes the five test files whose imports moved (47 tests).
• These also pass:
  • typecheck (root plus Desktop preload/main/renderer/storybook; the story's import changed)
  • lint and format:check
  • Knip for apps/desktop and packages/ui
  • check:asf-headers (the new testing.ts carries the header)
  • windows:inventory and astryx:surface-inventory
  • check:app-shell-hooks (28 hooks / 42 call sites, file untouched)
• Checker runtime went from 5.33 s to 5.53 s.
• Not run: Electron tests. No renderer runtime code changed; only export locations for tests and stories moved.

Merge notes

• fix(desktop,ui): stage a selected message's quotes on edit #5274 and refactor(runtime): extract bots and shared network packages #4293 change the checker's dependency sanction functions and append tests elsewhere. The overlap is textual only.
• After this lands:
  • A slice that removes a root use or a gate hook runs --write and deletes the matching retained-root rows in the same change.
  • A slice whose AppShell newly takes a pre-existing export fails, unless the use moves out of appShell.
• When merging main later, start from main's ledger and regenerate.

AI use

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

Tool(s) and scope: Claude Opus 5.5 (in Claude Code) implemented these checker rules, the retained-root table and transitional export list, the test-only export moves, tests, documentation and validation under human direction. The 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. CI now fails when:

  • a root caller newly takes an export its feature entry already had (other than a move out of appShell);
  • rootSymbolUses and the tree differ;
  • a root namespace, wildcard re-export or runtime load escapes symbol attribution;
  • an AppShell hook-gate entry has no matching retained-root rows.

  Seven test-only exports are no longer on production public entries. No product or runtime behavior changes.

• No

[Astro-Han]

Reviewed 641982ed9eac5792cf0d7065b55dca4b33865919 (16 files, +1455/−39, a single commit).

No P0–P3 findings, with one question I could not settle and am stating rather than assuming.

What this adds. The PR introduces root-symbol-use checks and retained-root-hook checks for the R2 work, and it adds no production behaviour of its own: 16 files, and app-shell.tsx is not touched at all — which also means it cannot conflict with the three sibling PRs on that file.

The one thing I could not determine. apps/desktop/renderer-architecture.json moves +189/−0 — a pure addition, no deletions. Read as new enforcement, that is the tightening direction: new entries that pin root symbol uses and the retained hook set cannot loosen an existing budget, because nothing was removed. Read as new allowances — for instance quotas that permit hooks to remain — it would be the opposite, and the title's phrase "retained root hooks" is ambiguous between pinning a set and granting it. My attempt to classify the added manifest lines did not complete cleanly, so I am flagging it instead of guessing: the distinction is worth one look by whoever owns the manifest semantics, since the acceptance criterion for this series is that the counts only tighten.

Gate on this head: test and label are green; the commit carries a Generated-by: trailer and there is no Grok involvement.

What I did not judge

• The enforcement semantics of the added manifest entries, as above.
• The check implementation's correctness in the two new test suites.
• 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.

[chihumyum]

On the open question in the review of 641982ed9: the +189 lines are enforcement, not allowance.

• What the +189 lines are. All of them are one new generated key, rootSymbolUses: 89 symbols in 18 entries. Every other section of renderer-architecture.json is byte-identical to main (1a66e4d5e), including legacyAppShell, its closure, rootDebt and controllerOwners, so no existing budget moved.

• What the record pins. Before this PR, any export of a feature public entry was available to the root. The record lists the symbols the root uses today, and the checker rejects any root use outside it.

• How it can change. Against the base it may only shrink. Two admissions exist, and the CLI prints each one:

  • an export the same change adds to the entry, measured on the base tree;
  • a one-way move from appShell into composition or bootstrap.

  Taking an export the entry already had fails, and so does deleting the record. The fixtures admits root symbol growth only for an export the head change adds (real CLI, --strict-base) and admits a use moving out of appShell, but not a copy or the reverse move cover these cases.

• Where the "retained root hooks" live. They are not in the ledger. They are a table in apps/desktop/src/renderer/README.md, and it grants nothing. scripts/check-app-shell-hooks.mjs stays the only hook budget, and this PR leaves it untouched. The table must mirror the gate's existing 28 entries, one row per call site (42), so it cannot raise a gate count. When a hook leaves the gate, its rows must go too, or the check fails. Of the 42 rows, 23 already name the R2 module that removes them.

Posted by Claude Code on behalf of the PR author.

[Astro-Han]

Independent review of 641982ed9eac5792cf0d7065b55dca4b33865919 (base 1a66e4d5e; it merges cleanly onto current main 887924e14, and check-renderer-architecture.mjs --base origin/main --strict-base passes there).

Verdict: this is new enforcement, not new allowance. One gap in the admission rule (P3) plus further P3s. The four ratchets themselves look right.

What the +189/-0 ledger change means. rootSymbolUses is a record of 89 symbols across 18 feature entries: appShell 66, composition 23, bootstrap 0. It is not a budget:

• Before this PR, a root caller allowed to import a feature public entry could take any symbol from it.
• After it, the tree must equal the record exactly (validateRootSymbolUses), and against the base the record may only shrink (compareRootSymbolUses). The two exceptions are an export the same change adds, and a one-way move out of appShell. Dropping the record fails.
• The existing 89 uses are grandfathered as a snapshot of what was already unconstrained.
• No existing limit gets looser. The only removed lines in the checker are the readdirSync filter, which is now legacyAppShellFiles() with the same LEGACY_APP_SHELL_FILE regex plus an existsSync guard. scripts/check-app-shell-hooks.mjs is untouched.
• The retained-root table is read-only validation over the gate's ALLOWED literal (row per call-site count, no rows for retired hooks). It grants nothing; the gate is still the budget.

Local run on this head: check-renderer-architecture.test.mjs 158/158, --base origin/main --strict-base passes, and --report matches the PR body: 43 bridge refs, 6 factories, 4 transitional capabilities, 28 gate entries / 42 call sites / 0 without a row. CI is green.

P3: an alias defeats the "existing export" rule. (Graded P3: it is a hole in a new guard, not a runtime defect, and the ratchet is still strictly tighter than before this PR.) Admission keys on the exported name (baseEntrySurfaces.get(dirname(use.entry))?.has(use.symbol)), so re-exporting an existing binding under a new name counts as a "new public export". I checked this on this head:

• Control: composition newly imports the existing AppUpdateAboutProjectionConsumer. The check fails with newly uses existing public export, as it should.
• Alias: I added export { AppUpdateAboutProjectionConsumer as AppUpdateAboutProbe } from './ui/app-update-projection-context.js' to features/app-update/index.ts, imported it in composition, and ran --write. --base HEAD then passes with root symbol use admitted: … composition AppUpdateAboutProbe (new public export).

The index.ts diff and the CLI line do make it visible, but the PR states that "taking an export the base entry already had fails". Suggested fix: measure the base surface by resolved binding (origin module plus original name, which resolveModuleExports already follows) and admit only bindings that the base entry did not expose under any name. A fixture for the alias case would pin it.

P3: the closure gap is already concrete. rootSymbolCallers records only the 16 top-level AppShell-family files, bootstrap/** and composition/**. The body documents this as "Wrappers / Non-root callers". With #5935, chat-message-surface.tsx, which is AppShell closure rather than a root zone, starts importing TaskReadinessNoticeConsumer from the Conversation entry, and nothing records it. Consider recording the AppShell closure's entry uses as a separate zone, or at least listing the closure files that render root regions.

P3: rows are matched by count only. The "Call site" column is free text, so swapping two useState rows' consumers or reasons passes. This is documented under "Table accuracy". A cheap step up would be to require that the call-site text names an identifier that actually appears at a call of that hook in AppShellContent.

Merge-order evidence. I built the merged trees and ran this PR's checker on each:

• main + #5934 + #5935 fails with one stale retained-root row (useTaskSubmissionReadiness, README:307), 5 unrecorded new root uses (ComposerSubmissionProvider, TaskReadinessProvider, createComposerSubmissionCommands, and the two services providers) and 9 stale entries. The gate itself is fine (26/33).
• main + #5934 + #5937 fails with one unrecorded new export, ManualDiagnosticReportConsumer.
• #5936 conflicts textually with this PR in features/workhub/index.ts.

All of these are new exports or shrinkage, so --write plus row deletion clears them. Landing this PR first, as its body asks, puts that small cost on each slice.

---

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]

Thanks @chihumyum — reviews of the four R2 PRs (#5934, #5935, #5936, #5937) are posted. Each merges cleanly onto main on its own, but they conflict pairwise (ledger, app-shell.tsx, check-app-shell-hooks.mjs, features/workhub/index.ts). A suggested landing order is #5934 → #5935 → #5936 → #5937: the enforcement first, then the largest move, then the two mechanical ones adapting to it. After #5934 lands, each later PR needs --write on the ledger and its stale retained-root rows removed, and whichever of #5935/#5937 lands second should delete the now-unused src/renderer/session-workspace-errors.ts so knip stays green.

[chihumyum]

Follow-up 4dbfc2c19 addresses all three P3s from the review of 641982ed9:

• An alias defeated the existing-export rule. Admission now compares resolved bindings (declaring module plus local name) instead of export names. Your AppUpdateAboutProbe alias case now fails with … an alias of existing public export …. Two alias fixtures (re-exported, and imported then re-exported) and an alias step in the --strict-base git fixture cover it.
• The closure gap was already concrete. The closure stays outside the root zones, because legacy-to-entry edges stay free. Its uses are now visible instead:
  • --report lists the AppShell closure's feature-entry uses per file (66 in 24 files today);
  • a --base run prints each one a change adds, marked "reported, not ratcheted", so chat-message-surface.tsx taking TaskReadinessNoticeConsumer in refactor(desktop): own Composer readiness, new-task choices and submission below the shell #5935 will show up in CI output.
• Rows were matched by count only. Where a gate entry has several call sites, each row must now name, in backticks, an identifier of exactly one of those calls in app-shell.tsx, and no two rows may name the same call. The two useEffect rows now name setWorkHubEnabled and defaultHostConnections. Consumer, owner and reason stay a review concern.

Architecture tests: 165/165. --base … --strict-base passes. lint, format:check, Knip and the hook gate pass. The branch merges cleanly into ab5996bdb. I'll keep the landing order you suggested, #5934 → #5935 → #5936 → #5937.

Posted by Claude Code on behalf of the PR author.

[Astro-Han]

Reviewed 4dbfc2c199ef0de45c50d864fe2218140a4951b7 (16 files, +1795/−39, 2 commits).

The P3 I raised last round is fixed, verified in the code rather than from the reply. My finding was that admission keyed on the exported name, so export { ExistingThing as NewName } from './same-module.js' counted as a new public export. This head introduces a resolved-binding model: a bindings map plus bindingOf(file, name, active), documented as "where the runtime binding exported as name is declared … every alias of one binding shares this key", following re-exports and namespace imports with an active cycle guard. An alias of an export the base entry already had therefore resolves to the same binding and is rejected — and there is a fixture that pins exactly that, "take an existing export through a new alias", asserting a non-zero status under --base --strict-base.

My second finding is resolved as report-only, which I am recording as the deliberate partial resolution it is. The gap was that only the sixteen top-level AppShell-family files counted as appShell, so closure files rendering root regions went unrecorded. The reply says this is addressed by reporting rather than gating, and the code matches: the check now reports the AppShell closure's feature-entry uses per file and prints, for a --base run, each one a change adds, while keeping those files out of the root zones so legacy-to-entry edges stay free. The gap is now auditable instead of silent, but it is not enforced — worth knowing when reading the report.

Checked for this card, with the rest of the set's themes deferred to their own PRs. test is green, and mergeable is true against the base branch, so this PR can still land on its own. The other items in the dispatch's list (knip breakage, getRunningTurnId coverage, silent degradation when a provider is missing) do not belong to this checker-only change; I will take them where they apply.

What I could not judge

• I did not execute the checker or its fixtures; I verified the binding-resolution implementation, the fixture's existence and its assertion, and the report path by reading them.
• The 66-in-24-files closure figure the reply quotes is theirs; I did not reproduce it.

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.