feat(devtools)!: reuse the built-in Terminals dock via ctx.terminals#1026
Open
antfubot wants to merge 2 commits into
Open
feat(devtools)!: reuse the built-in Terminals dock via ctx.terminals#1026antfubot wants to merge 2 commits into
antfubot wants to merge 2 commits into
Conversation
antfubot
force-pushed
the
feat/terminals-reuse-devframe
branch
from
July 21, 2026 08:59
badf065 to
4f421ba
Compare
Contributor
📝 WalkthroughWalkthroughThe terminal system now bridges Nuxt terminal hooks to the built-in DevTools Terminals dock through streamed read-only sessions. Terminal RPCs are narrowed to revealing sessions, with exit events replacing terminal-data events. Client terminal pages, routing state, xterm integration, and related dependencies are removed. Build and npm operations broadcast process exit codes, while module UI actions reveal terminals through RPC. Documentation and playground wording now describe the built-in dock. Estimated code review effort: 4 (Complex) | ~45 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/devtools/src/server-rpc/npm.ts (1)
65-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winEnsure the terminal exit is broadcast even if the session rejects.
If
session.getResult()rejects (e.g., due to a failure to spawn the process), the exit is never broadcast. This can leave the client's UI in a perpetual "updating" state. Consider broadcasting a non-zero exit code in the.catchblock.♻️ Proposed fix to broadcast on error
// Surface the exit to the client so the update UI state can settle. void Promise.resolve(session.getResult()).then((result) => { broadcastTerminalExit(ctx, processId, result.exitCode) - }).catch(() => {}) + }).catch(() => { + broadcastTerminalExit(ctx, processId, 1) + }) return {🤖 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/npm.ts` around lines 65 - 78, Update the Promise chain around session.getResult() so its catch handler calls broadcastTerminalExit with processId and a non-zero exit code when the session rejects. Preserve the existing result.exitCode broadcast for successful sessions, ensuring every terminal outcome notifies the client.
🤖 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.
Nitpick comments:
In `@packages/devtools/src/server-rpc/npm.ts`:
- Around line 65-78: Update the Promise chain around session.getResult() so its
catch handler calls broadcastTerminalExit with processId and a non-zero exit
code when the session rejects. Preserve the existing result.exitCode broadcast
for successful sessions, ensuring every terminal outcome notifies the client.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 008b91d4-8ba3-471e-8395-83c3a464aeff
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
docs/content/2.module/1.utils-kit.mdpackages/devtools-kit/src/_types/hooks.tspackages/devtools-kit/src/_types/rpc.tspackages/devtools-kit/src/_types/terminals.tspackages/devtools-kit/src/index.tspackages/devtools/client/components/ModuleItem.vuepackages/devtools/client/components/NpmVersionCheck.vuepackages/devtools/client/components/TerminalPage.vuepackages/devtools/client/components/TerminalView.vuepackages/devtools/client/composables/state-routes.tspackages/devtools/client/composables/state.tspackages/devtools/client/nuxt.config.tspackages/devtools/client/pages/modules/analyze-build.vuepackages/devtools/client/pages/modules/modules.vuepackages/devtools/client/pages/modules/terminals.vuepackages/devtools/client/setup/client-rpc.tspackages/devtools/package.jsonpackages/devtools/src/server-rpc/analyze-build.tspackages/devtools/src/server-rpc/index.tspackages/devtools/src/server-rpc/npm.tspackages/devtools/src/server-rpc/terminals.tsplaygrounds/module-starter/playground/nuxt.config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (9)
- packages/devtools/client/pages/modules/terminals.vue
- packages/devtools/client/components/TerminalPage.vue
- packages/devtools/client/composables/state.ts
- packages/devtools/client/components/TerminalView.vue
- packages/devtools/client/setup/client-rpc.ts
- packages/devtools/package.json
- packages/devtools/client/composables/state-routes.ts
- pnpm-workspace.yaml
- packages/devtools/client/nuxt.config.ts
antfubot
force-pushed
the
feat/terminals-reuse-devframe
branch
from
July 21, 2026 23:06
ad88fe2 to
2bdfdfe
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/terminals.ts`:
- Around line 92-101: Update the existing-terminal branch in the
devtools:terminal:register hook to recreate or replace the bridged session’s
stream/controller when re-registering an exited terminal, rather than only
resetting state and clearing its buffer. Ensure subsequent writes use the fresh
controller and preserve the existing session replacement/reset behavior before
calling host.update(existing.session).
🪄 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: 016f7cca-5c9a-47a4-8aad-5226c840d174
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
docs/content/2.module/1.utils-kit.mdpackages/devtools-kit/src/_types/hooks.tspackages/devtools-kit/src/_types/rpc.tspackages/devtools/client/components/ModuleItem.vuepackages/devtools/client/components/NpmVersionCheck.vuepackages/devtools/client/components/TerminalPage.vuepackages/devtools/client/components/TerminalView.vuepackages/devtools/client/composables/state-routes.tspackages/devtools/client/composables/state.tspackages/devtools/client/nuxt.config.tspackages/devtools/client/pages/modules/analyze-build.vuepackages/devtools/client/pages/modules/modules.vuepackages/devtools/client/pages/modules/terminals.vuepackages/devtools/client/setup/client-rpc.tspackages/devtools/package.jsonpackages/devtools/src/module-main.tspackages/devtools/src/server-rpc/analyze-build.tspackages/devtools/src/server-rpc/index.tspackages/devtools/src/server-rpc/npm.tspackages/devtools/src/server-rpc/terminals.tsplaygrounds/module-starter/playground/nuxt.config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (9)
- packages/devtools/client/components/TerminalView.vue
- packages/devtools/client/components/TerminalPage.vue
- packages/devtools/package.json
- packages/devtools/client/nuxt.config.ts
- packages/devtools/client/composables/state.ts
- packages/devtools/client/pages/modules/terminals.vue
- packages/devtools/client/composables/state-routes.ts
- packages/devtools/client/setup/client-rpc.ts
- pnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (9)
- playgrounds/module-starter/playground/nuxt.config.ts
- packages/devtools/client/components/ModuleItem.vue
- packages/devtools/src/server-rpc/analyze-build.ts
- packages/devtools/src/server-rpc/index.ts
- packages/devtools/src/module-main.ts
- packages/devtools/client/components/NpmVersionCheck.vue
- packages/devtools/client/pages/modules/modules.vue
- packages/devtools/client/pages/modules/analyze-build.vue
- packages/devtools-kit/src/_types/rpc.ts
Comment on lines
92
to
+101
| nuxt.hook('devtools:terminal:register', (terminal) => { | ||
| terminals.set(terminal.id, terminal) | ||
| refresh('getTerminals') | ||
| const existing = terminals.get(terminal.id) | ||
| if (existing) { | ||
| // Re-registration (e.g. `startSubprocess().clear()`/`restart()`): reset | ||
| // the session back to running and clear the display buffer in place. | ||
| existing.state = terminal | ||
| existing.exited = false | ||
| existing.session.status = 'running' | ||
| existing.session.buffer?.splice(0, existing.session.buffer.length) | ||
| host?.update(existing.session) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
git ls-files packages/devtools/src/server-rpc/terminals.ts
wc -l packages/devtools/src/server-rpc/terminals.ts
sed -n '1,240p' packages/devtools/src/server-rpc/terminals.tsRepository: nuxt/devtools
Length of output: 6793
🏁 Script executed:
sed -n '1,240p' packages/devtools/src/server-rpc/terminals.tsRepository: nuxt/devtools
Length of output: 6697
🏁 Script executed:
set -euo pipefail
echo "FILE:"
wc -l packages/devtools/src/server-rpc/terminals.ts
echo "----"
sed -n '1,240p' packages/devtools/src/server-rpc/terminals.tsRepository: nuxt/devtools
Length of output: 6758
🏁 Script executed:
set -euo pipefail
grep -nE 'controller|enqueue|write|re-?registration|restart|clear|buffer|status|devtools:terminal:register' packages/devtools/src/server-rpc/terminals.tsRepository: nuxt/devtools
Length of output: 1496
Recreate the stream/controller on terminal re-registration.
The existing branch only resets state and clears the buffer in place; if the terminal had already exited, the previous controller is closed, so later writes still return success while enqueue() throws and is swallowed. Replace the bridged session or create a fresh stream/controller before calling host.update(...).
🤖 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/terminals.ts` around lines 92 - 101, Update
the existing-terminal branch in the devtools:terminal:register hook to recreate
or replace the bridged session’s stream/controller when re-registering an exited
terminal, rather than only resetting state and clearing its buffer. Ensure
subsequent writes use the fresh controller and preserve the existing session
replacement/reset behavior before calling host.update(existing.session).
Retire Nuxt DevTools' bespoke @xterm terminal UI + RPC and surface terminal
sessions through the Vite DevTools terminals host (ctx.terminals), so they
render in the built-in Terminals dock.
Existing modules keep working: the devtools:terminal:* hooks are bridged onto
ctx.terminals as read-only registered sessions (module still owns the process
and streams output). For the "DevTools owns the process" model (output-only
child processes and interactive PTYs) no new Nuxt-specific API is added —
modules use Vite DevTools' own terminals host directly via the already-exposed
onDevtoolsReady((ctx) => ctx.terminals.startChildProcess(...) / .startPtySession(...)).
Exit-driven cleanup for the internal module-install / analyze-build / npm-update
flows is preserved by broadcasting terminal exits from their startChildProcess
producers.
Adopts the devframe 0.7 docks.activate({ sessionId }) capability: the in-client
"reveal terminal" affordances (module install, build analyze, npm update) focus
the built-in Terminals dock on the relevant session via a revealTerminal RPC,
instead of the removed Terminals tab.
Created with the help of an agent.
…ions
`setupRPC()`'s `ctx.devtoolsKit` is a getter backed by a closure variable set
once the Vite DevTools kit connects. Both `setupRPC`'s own return statement
and `module-main.ts`'s destructuring spread `ctx` (`{ connectDevToolsKit,
...ctx }` / `const { ..., ...ctx } = setupRPC(...)`), and object spread reads
a getter's *current* value into a plain property on the new object — at that
point (synchronous module setup, before the kit has connected) the value is
always `undefined`. This permanently froze `ctx.devtoolsKit` to `undefined`
for every integration wired through `module-main.ts` (vscode, vue-devtools,
vite-inspect, vue-tracer, plugin-metrics, timeline), even long after the kit
actually connected.
Concretely, this made the VS Code Server launcher a silent no-op: its
`ctx.devtoolsKit?.terminals.startChildProcess(...)` call was always skipped
by the optional-chain, so clicking "Launch" never spawned `code-server` and
nothing showed up in the Terminals dock.
Fix: return/destructure `ctx` as a nested property instead of spreading it,
so the getter — and therefore the live connected kit — stays intact.
Verified against a real Nuxt dev server: pre-fix, `ctx.devtoolsKit` is
`undefined` at the VS Code integration's call site and no process spawns;
post-fix it resolves to the connected kit and `startChildProcess` returns a
real, tracked child-process session.
Created with the help of an agent.
antfubot
force-pushed
the
feat/terminals-reuse-devframe
branch
from
July 22, 2026 01:14
2bdfdfe to
04c5d29
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/devtools/client/pages/modules/modules.vue (1)
48-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the nested
role="button"fromNCard.The outer
<button>is now the interactive element, so the childNCardmust not also expose button semantics. Keeping both can produce invalid nested controls and duplicate/confusing announcements for assistive technology.Proposed fix
<NCard border="1.5 dashed" h-full animate-pulse p4 transition hover="border-primary" flex="~ col gap-1 items-center justify-center" - role="button" class="group" >🤖 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/pages/modules/modules.vue` around lines 48 - 71, Remove the redundant role="button" attribute from the NCard inside the processInstallingModules button loop. Keep the outer button as the sole interactive element and leave the card’s styling and content unchanged.
🤖 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.
Outside diff comments:
In `@packages/devtools/client/pages/modules/modules.vue`:
- Around line 48-71: Remove the redundant role="button" attribute from the NCard
inside the processInstallingModules button loop. Keep the outer button as the
sole interactive element and leave the card’s styling and content unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63f2e6f0-cde4-48cc-8fc4-099e502f751d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
docs/content/2.module/1.utils-kit.mdpackages/devtools-kit/src/_types/hooks.tspackages/devtools-kit/src/_types/rpc.tspackages/devtools/client/components/ModuleItem.vuepackages/devtools/client/components/NpmVersionCheck.vuepackages/devtools/client/components/TerminalPage.vuepackages/devtools/client/components/TerminalView.vuepackages/devtools/client/composables/state-routes.tspackages/devtools/client/composables/state.tspackages/devtools/client/nuxt.config.tspackages/devtools/client/pages/modules/analyze-build.vuepackages/devtools/client/pages/modules/modules.vuepackages/devtools/client/pages/modules/terminals.vuepackages/devtools/client/setup/client-rpc.tspackages/devtools/package.jsonpackages/devtools/src/module-main.tspackages/devtools/src/server-rpc/analyze-build.tspackages/devtools/src/server-rpc/index.tspackages/devtools/src/server-rpc/npm.tspackages/devtools/src/server-rpc/terminals.tsplaygrounds/module-starter/playground/nuxt.config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (9)
- packages/devtools/client/setup/client-rpc.ts
- pnpm-workspace.yaml
- packages/devtools/client/composables/state.ts
- packages/devtools/client/components/TerminalPage.vue
- packages/devtools/client/pages/modules/terminals.vue
- packages/devtools/package.json
- packages/devtools/client/components/TerminalView.vue
- packages/devtools/client/composables/state-routes.ts
- packages/devtools/client/nuxt.config.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/devtools/src/server-rpc/analyze-build.ts
- playgrounds/module-starter/playground/nuxt.config.ts
- packages/devtools-kit/src/_types/rpc.ts
- packages/devtools/src/server-rpc/index.ts
- packages/devtools/src/module-main.ts
- packages/devtools/client/components/ModuleItem.vue
- docs/content/2.module/1.utils-kit.md
- packages/devtools-kit/src/_types/hooks.ts
- packages/devtools/client/components/NpmVersionCheck.vue
- packages/devtools/src/server-rpc/npm.ts
- packages/devtools/client/pages/modules/analyze-build.vue
- packages/devtools/src/server-rpc/terminals.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements plan
02-terminals-reusefrom the Vite DevTools integration series. Nuxt DevTools stops shipping its own@xtermterminal UI + RPC and instead surfaces terminal sessions through the devframe terminals host (ctx.terminals), so they render in Vite DevTools' built-in Terminals dock (which auto-hides when empty). The Nuxt "Terminals" tab is removed.Existing modules keep working via a compat shim. For the "DevTools owns the process" model (output-only child processes and interactive PTYs), no new Nuxt-specific API is introduced — modules use Vite DevTools' own terminals host directly via the already-exposed
onDevtoolsReady((ctx) => ctx.terminals.startChildProcess(...) / .startPtySession(...)).Rebased on
mainafter the Vite DevTools Kit0.4.2/ devframe0.7.5upgrade (#1029) and the Messages unification (#1025), and updated to adopt the new devframe capabilities.What changed
Server bridge (
server-rpc/terminals.ts, rewritten)devtools:terminal:*hooks ontoctx.terminalsinstead of maintaining its ownMap+ RPC:register→ a read-onlyDevframeTerminalSessioncarrying aReadableStream(registered viactx.terminals.register). Re-registration resets/clears in place, sostartSubprocess().clear()/restart()still work.write→controller.enqueue(data);exit→ close stream +update(status);remove→ dispose + remove the session.devtools:readycallback.ctx.terminals.startChildProcess(...)/.startPtySession(...)directly throughonDevtoolsReady. (Both surface in the built-in Terminals dock; PTYs arezigpty-powered with a graceful pipe fallback.)devframe 0.7 capability —
docks.activate({ sessionId })revealTerminal(id)RPC that callsctx.docks.activate('devframes_plugin_terminals', { sessionId })to focus the built-in Terminals dock on a specific session.Kit surface (
@nuxt/devtools-kit)revealTerminalRPC type. (No new terminal-spawning API is added — that path is Vite DevTools' ownctx.terminals.)getTerminals/getTerminalDetail/runTerminalAction+onTerminalDatafrom the RPC types (keptonTerminalExit).TerminalState/TerminalInfo/TerminalActionand thedevtools:terminal:*module-facing hooks are preserved.Internal producers — module-install / analyze-build / npm-update already spawn via
startChildProcess; they now broadcastonTerminalExiton process exit so their transient UI state (installing-modules, "Building…", update spinner) still clears. Exit-driven cleanup is not lost.Client — deleted the
terminals.vuetab,TerminalPage.vue,TerminalView.vue; removeduseTerminals,useCurrentTerminalId, and theonTerminalDatahandler. Removed the@xterm/*deps frompackage.json,pnpm-workspace.yaml, and the clientnuxt.config.tsbundling/optimizeDeps; refreshed the lockfile (the built-in@devframes/plugin-terminalsowns xterm now).Docs — documented spawning DevTools-owned terminals via
onDevtoolsReady((ctx) => ctx.terminals.startChildProcess(...) / .startPtySession(...))and refreshed the "Terminals tab" references.Bug fix — VS Code Server launcher was a silent no-op. While validating this
change against a real dev server, found and fixed a pre-existing bug (from
#966, unrelated to this PR's changes) that broke every
module-main.tsintegration relying on
ctx.devtoolsKit(VS Code Server, Vue DevTools, ViteInspect, Vue Tracer, plugin-metrics, timeline):
setupRPC()'s returnstatement and
module-main.ts's destructuring both spreadctx(
{ connectDevToolsKit, ...ctx }), andctx.devtoolsKitis a getter — objectspread reads a getter's current value into a plain property, which at that
point (synchronous module setup, before the kit connects) is always
undefined. This permanently frozectx.devtoolsKittoundefined, so e.g.clicking "Launch" on the VS Code Server tab silently did nothing (its
ctx.devtoolsKit?.terminals.startChildProcess(...)call was always skipped bythe optional chain) — no process spawned, nothing in the Terminals dock.
Fixed by returning/destructuring
ctxas a nested property instead ofspreading it. Verified against a real dev server: pre-fix,
ctx.devtoolsKitis
undefinedat the call site and no process spawns; post-fix it resolvesto the connected kit and a real, tracked
code-serverchild process spawns.Notes / breaking changes
getTerminals/getTerminalDetail/runTerminalAction) and the@xtermUI are removed. Module-facing hooks (devtools:terminal:register/write/remove/exit) andstartSubprocess()continue to work via the shim.startSubprocess/getProcess()deprecation codes (NDT_DEP_0004/0001) already shipped; this PR swaps the underlying implementation rather than adding new codes.pnpm typecheckcurrently reports one pre-existing error in theclient/server/api/echo.post.tsfixture that is present onmain(surfaced by the deps: upgrade Vite DevTools kit to 0.4.2 #1029 dep bump), not introduced here.Verification
pnpm lint,pnpm build, andpnpm test:unitpass;pnpm typecheckis clean apart from the pre-existingecho.post.tsfixture error noted above.This PR was created with the help of an agent.