Skip to content

refactor(desktop): move session settings ownership below AppShell - #5509

Open
chihumyum wants to merge 1 commit into
apache:mainfrom
chihumyum:refactor/session-settings-owner
Open

chihumyum wants to merge 1 commit into
apache:mainfrom
chihumyum:refactor/session-settings-owner

Conversation

@chihumyum

@chihumyum chihumyum commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Session-setting write intents currently live in AppShell, so writes for inactive Sessions also re-render the shell. Move their controller into a feature-owned SessionSettingsProvider, registered in controllerOwners, and leave the shell one equality-selected read of the active Session's model/thinking, permission, Plan and orchestration overlays plus stable commands.

Move the Plan transition policy and its authoritative getPlanState read behind feature services. Preserve optimistic rollback, revision-based retirement, captured-owner permission confirmation and the original Session target during Plan confirmation. Shell bridge paths drop from 18 to 17; the existing settings hook remains as a necessary read. Session Collaboration and Plan panel subscriptions remain outside this slice.

Refs #4582.

Verification

Rebased onto main feb9cf22f. Preserve #5532's direct Session catalog subscription inside the feature-owned controller, adapting the provider contract and tests to the current API. Regenerate the architecture ledger and Astryx inventory from the resolved source.

  • Node 24: build:test, Desktop test:dist 2820/2820, Desktop typecheck (including stories), production renderer build.
  • Renderer architecture 112/112 and exact-base check; AppShell hook gate; lint, format, Desktop/UI Knip, ASF headers, Astryx and Windows inventories, git diff --check.
  • New provider tests cover inactive-Session render isolation, active optimistic rollback, navigation during bypass/Plan confirmations, clearing pending intents, StrictMode cleanup and retained commands after unmount. The new catalog regression confirms that only the acknowledged Session overlay retires on catalog observations, without re-rendering the shell or frame.
  • Plan policy tests cover active execution refusal, canceled discard, normal mode changes and Host read errors; existing settings/controller tests retain model/thinking and causal-retirement coverage.
  • Merge-tree against the current base is clean.

No UI layout or intended user-visible behavior change. Interactive Electron/manual UI acceptance was not run; the behavior and render-scope evidence above comes from feature-level React tests.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex implemented the ownership migration, tests and documentation, performed source review and validation, and prepared this PR under the contributor's direction. The commit includes Generated-by: Codex.

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

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 19, 2026
Move setting write intents and Plan transition policy into the feature owner. Keep a selected-session overlay read and stable commands in the shell, with ownership and render-scope coverage.

Generated-by: Codex
@chihumyum
chihumyum force-pushed the refactor/session-settings-owner branch from 56b96ca to f0b0df2 Compare September 20, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant