Skip to content

Conversation

@jsantell
Copy link
Collaborator

@jsantell jsantell commented Dec 1, 2025

  • Moved cell watching, telemetry into DebuggerController from AppView
  • Moved keyboard listeners into KeyboardController, now a ReactiveController, from AppView
  • AppState now non-optional for AppView
  • AppState configurations now app.config
  • Single command for setting app configurations

Summary by cubic

Reorganized AppView to use dedicated controllers for telemetry and keyboard shortcuts, and consolidated UI flags under app.config with a single set-config command. This simplifies state management and improves cleanup and stability.

  • Refactors

    • Moved cell watching and telemetry into DebuggerController; AppView no longer handles these events.
    • Added KeyboardController as a ReactiveController; global shortcuts registered and cleaned up via the controller.
    • AppState now required in AppView; UI flags moved to app.config.
    • Replaced set-show-* and clear-authentication with set-config and set-identity (can be undefined).
  • Migration

    • Use command { type: "set-config", key, value } for UI toggles.
    • Read flags from app.config.* instead of app.show*.
    • Clear auth with { type: "set-identity", identity: undefined }.
    • Initialize AppState when creating AppView (use createDefaultAppState if needed).

Written for commit 6d033f6. Summary will update automatically on new commits.

* Moved cell watching, telemetry into DebuggerController from AppView
* Moved keyboard listeners into KeyboardController, now a ReactiveController, from AppView
* `AppState` now non-optional for AppView
* `AppState` configurations now `app.config`
* Single command for setting app configurations
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 11 files

Prompt for AI agents (all 3 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/shell/src/lib/app/commands.ts">

<violation number="1" location="packages/shell/src/lib/app/commands.ts:27">
P1: Limit &quot;set-config&quot; to known AppStateConfig keys so untrusted commands cannot set arbitrary or prototype-polluting properties on app.config.</violation>
</file>

<file name="packages/shell/src/lib/app/state.ts">

<violation number="1" location="packages/shell/src/lib/app/state.ts:62">
P1: Writing directly to `next.config.showShellCharmListView` mutates the original state’s config object (clone() is shallow) and even throws when config is absent (e.g., from older serialized state). Reassign a new config object before updating the flag.</violation>

<violation number="2" location="packages/shell/src/lib/app/state.ts:67">
P1: `next.config[command.key] = command.value` mutates the shared config object from the previous state and can crash when `config` is undefined. Create a fresh config object (defaulting to `{}`) before applying the update.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

@jsantell jsantell merged commit 9d3e61b into main Dec 1, 2025
10 checks passed
@jsantell jsantell deleted the shell-app-minimize branch December 1, 2025 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants