Skip to content

Commit 6a26f09

Browse files
committed
fix(hub): handle PTY restart failures
1 parent 9a3132c commit 6a26f09

5 files changed

Lines changed: 85 additions & 38 deletions

File tree

docs/content/5.add-ons/1.devframes/6.terminals.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Mounted into a hub, the devframe spawns on its own channel (`devframes:plugin:te
6161

6262
`ctx.terminals` is the source of truth; the devframe, the sole PTY provider, duck-types a minimal `register` / `update` / `events` shape to run without `@devframes/hub`.
6363

64-
Both spawned session types carry a `getResult()` accessor. A `startChildProcess()` result is an `await`able `{ stdout, stderr, exitCode }` with live process getters and `kill()`. A `startPtySession()` result captures its merged terminal stream as an `await`able `{ output, exitCode, signal }` with live `pid`, `exitCode`, and `killed` getters.
64+
Both spawned terminal session types carry a `getResult()` accessor. A `startChildProcess()` result is an `await`able `{ stdout, stderr, exitCode }` with live process getters and `kill()`. A `startPtySession()` result captures its merged terminal stream as an `await`able `{ output, exitCode, signal }` with live `pid`, `exitCode`, and `killed` getters. `killed` is the portable termination indicator; `signal` is present when the PTY backend reports one.
6565

6666
## Focusing a session
6767

docs/content/6.errors/DF8203.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,4 @@ directory does not exist, or spawning was denied by the OS.
2121

2222
## Source
2323

24-
- [`packages/hub/src/node/host-terminals.ts`](https://github.com/devframes/devframe/blob/main/packages/hub/src/node/host-terminals.ts)`DevframeTerminalsHost.startPtySession()` throws this when the initial `zigpty` spawn fails.
24+
- [`packages/hub/src/node/host-terminals.ts`](https://github.com/devframes/devframe/blob/main/packages/hub/src/node/host-terminals.ts)`DevframeTerminalsHost.startPtySession()` throws this when an initial or restart `zigpty` spawn fails.

docs/content/8.references/6.hub-api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ What `DevframeHubContext` adds to `DevframeNodeContext` — [Hub](/guide/hub).
1414
| Subsystem | API | Purpose |
1515
|---|---|---|
1616
| `ctx.docks` | `register / update / values / activate` | Dock entries (iframes, launchers, custom-render) and groups; `activate(dockId, params?)` sets the active dock ([Cross-iframe dock activation](/guide/hub#cross-iframe-dock-activation)). |
17-
| `ctx.terminals` | `register / startChildProcess` | Aggregate terminal sessions, streaming output ([Terminals](/add-ons/devframes/terminals#hub-aggregation)). |
17+
| `ctx.terminals` | `register / startChildProcess / startPtySession` | Aggregate terminal sessions, streaming output ([Terminals](/add-ons/devframes/terminals#hub-aggregation)). |
1818
| `ctx.messages` | `add / update / remove / clear` | Server-side toast/notification queue (FIFO, capped at 1000). |
1919
| `ctx.commands` | `register / execute / list` | Hierarchical command palette with keybindings and `when` clauses. |
2020

packages/hub/src/node/__tests__/host-terminals.test.ts

Lines changed: 58 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,19 @@ import { describe, expect, it, vi } from 'vitest'
55
import { hasNative } from 'zigpty'
66
import { DevframeTerminalsHost } from '../host-terminals'
77

8+
const zigptyModuleMock = vi.hoisted(() => ({
9+
spawn: vi.fn(),
10+
}))
11+
12+
vi.mock('zigpty', async (importOriginal) => {
13+
const originalModule = await importOriginal<typeof import('zigpty')>()
14+
zigptyModuleMock.spawn.mockImplementation(originalModule.spawn)
15+
return {
16+
...originalModule,
17+
spawn: zigptyModuleMock.spawn,
18+
}
19+
})
20+
821
const NODE = process.execPath
922
// A real PTY works wherever zigpty's native bindings load (incl. Windows
1023
// ConPTY); skip when they're unavailable.
@@ -419,6 +432,8 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
419432
})
420433

421434
itPty('getResult() resolves merged PTY output after natural exit', async () => {
435+
expect.assertions(9)
436+
422437
const { host } = createTerminalHost()
423438

424439
const session = await host.startPtySession({
@@ -441,6 +456,8 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
441456
})
442457

443458
itPty('getResult() preserves a non-zero PTY exit code', async () => {
459+
expect.assertions(3)
460+
444461
const { host } = createTerminalHost()
445462

446463
const session = await host.startPtySession({
@@ -459,6 +476,8 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
459476
})
460477

461478
itPty('getResult() marks a terminated PTY run as killed', async () => {
479+
expect.assertions(4)
480+
462481
const { host } = createTerminalHost()
463482

464483
const session = await host.startPtySession({
@@ -467,7 +486,8 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
467486
}, { id: 'pty-result-terminate', title: 'PTY result terminate' })
468487
const result = session.getResult()
469488
await waitUntil(() => {
470-
expect(session.buffer?.join('')).toContain('started')
489+
if (!session.buffer?.join('').includes('started'))
490+
throw new Error('PTY output has not started')
471491
})
472492

473493
await session.terminate()
@@ -477,30 +497,16 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
477497
await expect(result).resolves.toMatchObject({
478498
output: expect.stringContaining('started'),
479499
exitCode: undefined,
480-
signal: expect.any(Number),
481500
})
482-
})
483-
484-
itPty('isolates getResult() output between independently spawned PTY sessions', async () => {
485-
const { host } = createTerminalHost()
486-
487-
const firstSession = await host.startPtySession({
488-
command: NODE,
489-
args: ['-e', 'process.stdout.write("first-run")'],
490-
}, { id: 'pty-result-first', title: 'First PTY result' })
491-
const secondSession = await host.startPtySession({
492-
command: NODE,
493-
args: ['-e', 'process.stdout.write("second-run")'],
494-
}, { id: 'pty-result-second', title: 'Second PTY result' })
495-
496-
const [firstOutput, secondOutput] = await Promise.all([firstSession.getResult(), secondSession.getResult()])
497-
expect(firstOutput.output).toContain('first-run')
498-
expect(firstOutput.output).not.toContain('second-run')
499-
expect(secondOutput.output).toContain('second-run')
500-
expect(secondOutput.output).not.toContain('first-run')
501+
if (process.platform === 'win32')
502+
await expect(result).resolves.toHaveProperty('signal', undefined)
503+
else
504+
await expect(result).resolves.toHaveProperty('signal', expect.any(Number))
501505
})
502506

503507
itPty('getResult() isolates the previous PTY run after restart()', async () => {
508+
expect.assertions(8)
509+
504510
const { host } = createTerminalHost()
505511

506512
const session = await host.startPtySession({
@@ -509,15 +515,17 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
509515
}, { id: 'pty-result-restart', title: 'PTY result restart' })
510516
const firstResult = session.getResult()
511517
await waitUntil(() => {
512-
expect(session.buffer?.join('')).toContain(`run:${firstResult.pid}`)
518+
if (!session.buffer?.join('').includes(`run:${firstResult.pid}`))
519+
throw new Error('First PTY run has not started')
513520
})
514521

515522
await session.restart()
516523
const secondResult = session.getResult()
517524
expect(secondResult).not.toBe(firstResult)
518525
expect(secondResult.pid).not.toBe(firstResult.pid)
519526
await waitUntil(() => {
520-
expect(session.buffer?.join('')).toContain(`run:${secondResult.pid}`)
527+
if (!session.buffer?.join('').includes(`run:${secondResult.pid}`))
528+
throw new Error('Second PTY run has not started')
521529
})
522530

523531
await session.terminate()
@@ -530,6 +538,33 @@ describe('devframeTerminalHost interactive PTY sessions', () => {
530538
expect(secondOutput.output).not.toContain(`run:${firstResult.pid}`)
531539
})
532540

541+
itPty('reports a structured error when restart fails to spawn a PTY', async () => {
542+
expect.assertions(5)
543+
544+
const { host } = createTerminalHost()
545+
const session = await host.startPtySession({
546+
command: NODE,
547+
args: ['-e', 'process.stdout.write("started"); setInterval(() => {}, 4000)'],
548+
}, { id: 'pty-result-restart-error', title: 'PTY result restart error' })
549+
const result = session.getResult()
550+
await waitUntil(() => {
551+
if (!session.buffer?.join('').includes('started'))
552+
throw new Error('PTY output has not started')
553+
})
554+
zigptyModuleMock.spawn.mockImplementationOnce(() => {
555+
throw new Error('restart spawn failed')
556+
})
557+
558+
await expect(session.restart()).rejects.toThrow(expect.objectContaining({ code: 'DF8203' }))
559+
expect(session.status).toBe('error')
560+
expect(session.getProcessName()).toBeUndefined()
561+
expect(session.getResult()).toBe(result)
562+
await expect(result).resolves.toMatchObject({
563+
output: expect.stringContaining('started'),
564+
exitCode: undefined,
565+
})
566+
})
567+
533568
itPty('does not accept resize after termination without throwing', async () => {
534569
const { host } = createTerminalHost()
535570

packages/hub/src/node/host-terminals.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType {
421421

422422
const spawnPty = (): IPty => {
423423
const currentRun = ++runId
424-
const proc = spawn(executeOptions.command, executeOptions.args ?? [], {
424+
const ptyProcess = spawn(executeOptions.command, executeOptions.args ?? [], {
425425
name: PTY_TERM_NAME,
426426
cols,
427427
rows,
@@ -456,41 +456,42 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType {
456456
})
457457
}
458458

459-
proc.onData((data) => {
459+
ptyProcess.onData((data) => {
460460
const text = typeof data === 'string' ? data : data.toString('utf8')
461461
outputChunks.push(text)
462462
if (!streamClosed && currentRun === runId)
463463
controller?.enqueue(text)
464464
})
465-
proc.onExit(({ exitCode, signal }) => {
465+
ptyProcess.onExit(({ exitCode, signal }) => {
466466
settle(exitCode, signal)
467467
if (currentRun !== runId)
468468
return
469469
closeStream()
470-
// A signal kill (terminate()/restart()) is a deliberate stop; a clean
471-
// exit is a deliberate stop too. Only an unsignalled non-zero exit
472-
// code is a crash, matching the child-process comment above.
470+
/**
471+
* A signal kill (terminate()/restart()) and a clean exit are deliberate stops.
472+
* Only an unsignalled non-zero exit code is a crash, matching the child-process path.
473+
*/
473474
markStatus(signal === 0 && exitCode !== 0 ? 'error' : 'stopped')
474475
})
475476
currentResult = {
476477
get pid() {
477-
return proc.pid
478+
return ptyProcess.pid
478479
},
479480
get exitCode() {
480-
return killed ? undefined : (proc.exitCode ?? settledExitCode)
481+
return killed ? undefined : (ptyProcess.exitCode ?? settledExitCode)
481482
},
482483
get killed() {
483484
return killed
484485
},
485486
then: (onfulfilled, onrejected) => outputPromise.then(onfulfilled, onrejected),
486487
}
487488
killCurrentRun = () => {
488-
if (proc.exitCode !== null)
489+
if (ptyProcess.exitCode !== null)
489490
return
490491
killed = true
491-
proc.kill()
492+
ptyProcess.kill()
492493
}
493-
return proc
494+
return ptyProcess
494495
}
495496

496497
try {
@@ -547,7 +548,18 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType {
547548
if (streamClosed)
548549
throw diagnostics.DF8206({ id: terminal.id })
549550
killCurrentRun?.()
550-
pty = spawnPty()
551+
pty = undefined
552+
try {
553+
pty = spawnPty()
554+
}
555+
catch (error) {
556+
errorStream(error)
557+
markStatus('error')
558+
throw diagnostics.DF8203({
559+
command: executeOptions.command,
560+
reason: error instanceof Error ? error.message : String(error),
561+
})
562+
}
551563
markStatus('running')
552564
},
553565
}

0 commit comments

Comments
 (0)