Skip to content

refactor(ui-react): stabilize app shell effect callbacks - #512

Merged
Astro-Han merged 5 commits into
mainfrom
codex/app-shell-effect-stability
Jul 4, 2026
Merged

refactor(ui-react): stabilize app shell effect callbacks#512
Astro-Han merged 5 commits into
mainfrom
codex/app-shell-effect-stability

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace the local latest-ref pattern in AppShell effect hooks with React useEffectEvent.
  • Keep bootstrap subscriptions mounted once and active-session subscriptions keyed by activeId, while long-lived callbacks invoke the latest handlers.
  • Keep the plan reminder toast action as a plain UI callback while preserving latest navigation behavior.
  • Replace source-shape stability checks with behavior tests that rerender handler props, trigger captured old subscription callbacks, and assert no resubscribe plus latest-handler dispatch.
  • Keep startup fail-soft, permission boundary, and active-session lifecycle contracts aligned with the effect-event shape.

Why

Refs #477

AppShell subscription effects intentionally avoid resubscribing on every render, but they previously closed over many callback props. React 19's useEffectEvent is the smaller boundary for subscription, timer, and listener callbacks: subscription lifetime stays stable, callback bodies stay fresh, and ordinary UI callbacks stay outside the effect-event boundary.

Scope

Changed:

  • useAppShellBootstrapSubscriptions() now wraps subscription, timer, keydown, and cleanup callbacks with useEffectEvent while preserving the mount-once effect boundary.
  • useActiveSessionEvents() still resubscribes by activeId, and its async read / session-event callbacks now use effect events for latest setters and handlers.
  • Plan reminder due notifications still refresh reminders and show the same toast action, but the action click itself is a normal inline callback.
  • AppShell effect stability coverage now proves old captured callbacks call the latest handler without a resubscribe, including the plan reminder toast action path.
  • Existing startup fail-soft, permission boundary, and active-session lifecycle contracts were updated to match the effect-event owner path.

Not included:

  • No visual redesign.
  • No targeted memo work.
  • No ChatView or AppShell component-size split.
  • No change to subscription timing or user-visible copy.

Verification

  • GREEN: npm run -w @maka/desktop build:main && (cd apps/desktop && node --test dist/main/__tests__/app-shell-effect-stability-contract.test.js dist/main/__tests__/session-message-lifecycle-contract.test.js dist/main/__tests__/permission-response-ipc-boundary.test.js dist/main/__tests__/renderer-startup-fail-soft-contract.test.js) (26 passed)
  • GREEN: npm run -w @maka/desktop build:renderer (passes; existing Vite large chunk warning remains)
  • GREEN: npm run -w @maka/desktop test (1863 passed)

User-facing impact

None expected. This is an AppShell effect-boundary refactor; UI layout, copy, and interaction timing should stay the same.

Reviewer notes

The PR topic commits are rollback-sized:

  • refactor(ui-react): stabilize app shell effect callbacks
  • test(ui-react): lock app shell effect stability contracts
  • refactor(ui-react): use effect events for app shell subscriptions
  • refactor(ui-react): keep plan toast actions outside effect events

A final merge commit syncs origin/main so the PR is mergeable after the Windows titlebar change landed.

@Astro-Han
Astro-Han merged commit 2791ea4 into mainJul 4, 2026
@Astro-Han
Astro-Han deleted the codex/app-shell-effect-stability branch July 4, 2026 13:56
Sign up for freeto 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.

1 participant

@Astro-Han