feat(devtools)!: finish Messages + terminals convergence on Devframe 0.7.7 - #1032
Conversation
…0.7.7 - bump devframe/@devframes/hub to 0.7.7 - route client notifications through the stable hub:messages:add boundary - give each Nuxt-owned process run (analyze-build, install/uninstall, package update) a unique terminal-session id to avoid duplicate-id errors - rewrite the devtools:terminal:* bridge as an output+status-only projection onto the built-in Terminals dock, with pre-ready buffering and per-run generations - derive analyze-build/module-install UI state from awaited RPCs and refreshed info instead of broad onTerminalExit broadcasts - deprecate the ignored TerminalState action fields (v5 removal)
📝 WalkthroughWalkthroughThe change introduces unique terminal session IDs, generation-aware bridging for legacy terminal hooks, and buffered output/status replay across DevTools readiness. Analyze-build and npm RPC flows now expose or accept session IDs and use refreshed state instead of terminal-exit broadcasts for selected UI updates. Module actions guarantee cleanup, notification payload construction is centralized, terminal capability APIs are documented as deprecated, migration guidance is added, and related tests and dependency pins are updated. Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/devtools/test/notify.test.ts (1)
4-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the changed
notify()entry point.These tests validate
buildMessageEntry()but never invokenotify()frompackages/devtools/client/composables/notify.ts, Lines 13-16. Add a mocked RPC test asserting thehub:messages:addtarget and generated payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/devtools/test/notify.test.ts` around lines 4 - 29, Extend the client notify transport tests to invoke notify() from the notify composable rather than only testing buildMessageEntry(). Mock the RPC transport and assert that notify() targets MESSAGES_ADD_RPC ('hub:messages:add') with the generated message payload, while preserving the existing entry-construction tests.packages/devtools/client/composables/notify.ts (1)
16-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAvoid bypassing the RPC contract with
as any.The cast can hide an incorrect RPC name or payload shape. Add
hub:messages:addto the RPC method typing if needed, then call it without the cast.Suggested direction
- await client.call(MESSAGES_ADD_RPC as any, buildMessageEntry(input)) + await client.call(MESSAGES_ADD_RPC, buildMessageEntry(input))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/devtools/client/composables/notify.ts` at line 16, Update the RPC typing used by the notify flow to include the hub:messages:add method and its expected payload, then change the call in the notification function to use MESSAGES_ADD_RPC without an any cast. Preserve the existing buildMessageEntry(input) payload behavior while allowing the typed RPC contract to validate both method name and arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/devtools/src/server-rpc/analyze-build.ts`:
- Around line 32-45: Update the session-start flow around startChildProcess to
reserve the build slot before awaiting terminal creation, and make the existing
request guard reject when either promise or activeSessionId is set. Wrap
terminal creation in catch handling that clears or refreshes activeSessionId
before rethrowing, while preserving the successful session assignment and build
lifecycle behavior.
---
Nitpick comments:
In `@packages/devtools/client/composables/notify.ts`:
- Line 16: Update the RPC typing used by the notify flow to include the
hub:messages:add method and its expected payload, then change the call in the
notification function to use MESSAGES_ADD_RPC without an any cast. Preserve the
existing buildMessageEntry(input) payload behavior while allowing the typed RPC
contract to validate both method name and arguments.
In `@packages/devtools/test/notify.test.ts`:
- Around line 4-29: Extend the client notify transport tests to invoke notify()
from the notify composable rather than only testing buildMessageEntry(). Mock
the RPC transport and assert that notify() targets MESSAGES_ADD_RPC
('hub:messages:add') with the generated message payload, while preserving the
existing entry-construction tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01a39ed7-7987-4e27-ae83-5bcfefacf5f8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
docs/content/2.module/3.migration-v4.mdpackages/devtools-kit/src/_types/analyze-build.tspackages/devtools-kit/src/_types/rpc.tspackages/devtools-kit/src/_types/terminals.tspackages/devtools/client/components/ModuleItemInstall.vuepackages/devtools/client/composables/notify.tspackages/devtools/client/composables/npm.tspackages/devtools/client/composables/state-subprocess.tspackages/devtools/client/pages/modules/analyze-build.vuepackages/devtools/client/setup/client-rpc.tspackages/devtools/src/notify-transport.tspackages/devtools/src/server-rpc/analyze-build.tspackages/devtools/src/server-rpc/npm.tspackages/devtools/src/server-rpc/terminals.tspackages/devtools/src/types/ui-state.tspackages/devtools/src/utils/session-id.tspackages/devtools/test/notify.test.tspackages/devtools/test/session-id.test.tspackages/devtools/test/terminals-bridge.test.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- packages/devtools/src/types/ui-state.ts
| // A fresh id per run — completed sessions linger in the dock, so a fixed id | ||
| // would collide (`DF8200`) on the next build. | ||
| const sessionId = createUniqueSessionId('devtools:analyze-build') | ||
| activeSessionId = sessionId | ||
|
|
||
| const session = await kit.terminals.startChildProcess({ | ||
| command: 'npx', | ||
| args: ['nuxi', 'analyze', '--no-serve', '--name', name], | ||
| cwd: nuxt.options.rootDir, | ||
| }, { | ||
| id: processId, | ||
| id: sessionId, | ||
| title: 'Analyze Build', | ||
| icon: 'logos-nuxt-icon', | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Close the start-up race and clear failed session state.
promise is not assigned until after startChildProcess() resolves. Two requests can both pass Line 25, launch separate builds, and overwrite activeSessionId; a startup rejection also leaves that ID set while isBuilding is false. Reserve the slot before the await (for example, guard on promise || activeSessionId) and clear/refresh activeSessionId in a catch around terminal creation before rethrowing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/devtools/src/server-rpc/analyze-build.ts` around lines 32 - 45,
Update the session-start flow around startChildProcess to reserve the build slot
before awaiting terminal creation, and make the existing request guard reject
when either promise or activeSessionId is set. Wrap terminal creation in catch
handling that clears or refreshes activeSessionId before rethrowing, while
preserving the successful session assignment and build lifecycle behavior.
2 participants
Why
Implements
plans/vite-devtools-integration/02-platform-services-cleanup.mdand moves the platform-service integration onto Devframe 0.7.7. It finishes two partially-landed migrations: client notifications now flow through the stable Messages hub boundary, and all terminal output surfaces in Vite DevTools' built-in Terminals dock rather than a bespoke transport.What
devframe/@devframes/hub(workspace overrides + lockfile).devframes-plugin-messages:add, so client toasts failed best-effort. It now calls the stablehub:messages:add, keepingnotify/leveldefaults and numeric toast lifetimes.devtools:terminal:*hooks are projected ontoctx.terminalsas output + final-status only. Pre-ready registrations, output, and exit are buffered and replayed (a process exiting before connect no longer stays stuckrunning); re-registration mints a fresh session; superseded runs are dropped.onTerminalExitbroadcasts, which are now the minimal completion signal for generic package updates only.TerminalStaterestart/terminate fields are marked deprecated (removal in v5) and the migration guide documents the output-only bridge.Devframe 0.7.7 also exposes a public
terminals.remove(), so the previous private-method shim is gone.Verification
typecheck,lint,test:unit(incl. new coverage for the message transport, unique-id generation, and bridge buffering/re-registration/status mapping), a fullbuild, andtest:e2e:devall pass.This PR was created with the help of an agent.