diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 0577d29ad9..4e1dcae987 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -803,7 +803,7 @@ Source: [`packages/plan/plan-mode/src/index.ts:57`](../packages/plan/plan-mode/s ## `@deepseek-ai/dsh-pty-local` -Requires: `agents` · `pty` · `sandbox` · `sandboxPolicy` +Requires: `pty` · `sandbox` · `sandboxPolicy` ```ts config-catalog /** Public plugin configuration. */ diff --git a/packages/pty/pty-local/README.md b/packages/pty/pty-local/README.md index 32b2721074..34e3beadec 100644 --- a/packages/pty/pty-local/README.md +++ b/packages/pty/pty-local/README.md @@ -4,11 +4,11 @@ Local `node-pty` backend for `ctx.pty`. It starts an interactive shell under the ## Plugin (`pty-local`) -The plugin injects `agents`, `pty`, `sandbox`, and `sandboxPolicy`, then registers the configured backend type (`shell`). `danger-full-access` starts the shell directly; confined modes wrap the exact shell argv through `ctx.sandbox`. The effective session mode is resolved at spawn. A change to a different effective mode is rejected before its `sandbox/mode` event commits while that owner has an open PTY or a spawn in progress; wait for creation to settle and close the sessions before changing modes, so a terminal opened with wider access cannot survive a downgrade. +The plugin injects `pty`, `sandbox`, and `sandboxPolicy`, then registers the configured backend type (`shell`). `danger-full-access` starts the shell directly; confined modes wrap the exact shell argv through `ctx.sandbox`. The effective session mode is resolved at spawn. A change to a different effective mode is rejected before its `sandbox/mode` event commits while that owner has an open PTY or a spawn in progress; the fence is attached to the exact owner and therefore outlives a local-provider reload that retains existing sessions. Wait for creation to settle and close the sessions before changing modes, so a terminal opened with wider access cannot survive a downgrade. Linux readiness combines a foreground-verified private bash prompt marker, foreground-process-group syscall inspection, silence fallback, and absolute timeout. macOS uses the verified prompt marker plus silence/timeout because it has no `/proc` syscall surface. A marker is not ready until printable prompt text arrives, including when the OSC marker and `PS1` are split across data callbacks. Unrecognized or unreadable process state is never a positive exact-idle signal. During unpublished startup, a fallback requires observed output; zero-output silence cannot publish an empty session, and timeout rejects the spawn. Incomplete terminal-control sequences are bounded by `maxReadBytes` and discarded through their terminator after crossing that limit; a trailing carriage return is carried across callbacks so split CRLF becomes one newline. -Send cancellation resolves the current foreground process group and delivers a real `SIGINT`; it never emulates interruption by writing `\x03`, so raw-mode programs remain cancellable. Close sends `SIGTERM` to descendants, waits, rescans and sends `SIGKILL` to the remaining tree, verifies that descendants left the process table while the shell can still reap them, and only then stops the shell. A survivor failure does not cache a permanently rejected close; a later close retries the teardown. +Send cancellation resolves the current foreground process group and delivers a real `SIGINT`; it never emulates interruption by writing `\x03`, so raw-mode programs remain cancellable. Close sends `SIGTERM` to descendants, waits, then sends `SIGKILL` to the union of captured survivors and newly scanned descendants so reparenting cannot hide a process from teardown. It verifies that every retained identity left the process table while the shell can still reap it and only then stops the shell. A survivor failure does not cache a permanently rejected close; a later close retries the teardown. ## Model Experience diff --git a/packages/pty/pty-local/src/index.ts b/packages/pty/pty-local/src/index.ts index 706a1bba7a..db4de5de28 100644 --- a/packages/pty/pty-local/src/index.ts +++ b/packages/pty/pty-local/src/index.ts @@ -7,6 +7,7 @@ import { Context } from 'cordis' import * as nodePty from 'node-pty' import type { IPtyForkOptions } from 'node-pty' +import type { Agent } from '@deepseek-ai/dsh-agent' import type { Session, SessionEvent } from '@deepseek-ai/dsh-session' import type { PtyBackend, PtyBackendSpawnSpec } from '@deepseek-ai/dsh-pty' import type { SandboxMode } from '@deepseek-ai/dsh-sandbox' @@ -21,10 +22,37 @@ export type { Config as PtyLocalConfig } from './config.ts' /** Cordis plugin name. */ export const name = 'pty-local' -/** Required services: owner/PTY registries plus the one shared confinement policy. */ -export const inject = ['agents', 'pty', 'sandbox', 'sandboxPolicy'] +/** Required services: PTY registry plus the one shared confinement policy. */ +export const inject = ['pty', 'sandbox', 'sandboxPolicy'] const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i +interface SandboxModeFenceState { + pty: Context['pty'] + sandboxPolicy: Context['sandboxPolicy'] +} + +const sandboxModeFences = new WeakMap() + +function ensureSandboxModeFence(ctx: Context, owner: Agent): void { + const existing = sandboxModeFences.get(owner) + if (existing !== undefined) { + existing.pty = ctx.pty + existing.sandboxPolicy = ctx.sandboxPolicy + return + } + const state: SandboxModeFenceState = { pty: ctx.pty, sandboxPolicy: ctx.sandboxPolicy } + sandboxModeFences.set(owner, state) + owner.ctx.on('internal/dispatch', (_mode, eventName, args) => { + if (eventName !== 'session/event') return + const [session, event] = args as [Session, SessionEvent] + if (session !== owner.session || event.type !== 'sandbox/mode') return + const currentMode = effectiveSandboxMode(session.events) ?? state.sandboxPolicy.defaultMode + if (event.data.mode === currentMode || !state.pty.hasOwnerActivity(owner)) return + throw new Error( + `cannot change sandbox mode from "${currentMode}" to "${event.data.mode}" while persistent terminal sessions are open or being created; wait for creation to settle and close them first`, + ) + }, { global: true }) +} function childEnvironment(spec: PtyBackendSpawnSpec): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = {} @@ -75,6 +103,7 @@ export class LocalPtyBackend implements PtyBackend { async spawn(spec: PtyBackendSpawnSpec): Promise { spec.signal?.throwIfAborted() + ensureSandboxModeFence(this.ctx, spec.owner) const argv = spawnArgv(this.ctx, this.config, spec) const file = argv[0] if (file === undefined) throw new Error('pty-local: sandbox returned empty argv') @@ -106,17 +135,4 @@ export function apply(ctx: Context, config: Config): void { validateConfig(config) const inspector = createProcessInspector() ctx.pty.registerBackend(new LocalPtyBackend(ctx, config, inspector)) - ctx.on('internal/dispatch', (_mode, eventName, args) => { - if (eventName !== 'session/event') return - const [session, event] = args as [Session, SessionEvent] - if (event.type !== 'sandbox/mode') return - const currentMode = effectiveSandboxMode(session.events) ?? ctx.sandboxPolicy.defaultMode - if (event.data.mode === currentMode) return - const owner = ctx.agents.get(session.id) - if (owner === undefined) return - if (!ctx.pty.hasOwnerActivity(owner)) return - throw new Error( - `cannot change sandbox mode from "${currentMode}" to "${event.data.mode}" while persistent terminal sessions are open or being created; wait for creation to settle and close them first`, - ) - }, { global: true }) } diff --git a/packages/pty/pty-local/src/session.ts b/packages/pty/pty-local/src/session.ts index a1e638e0d2..99bf142174 100644 --- a/packages/pty/pty-local/src/session.ts +++ b/packages/pty/pty-local/src/session.ts @@ -394,16 +394,31 @@ export class LocalPtySession implements PtyBackendSession { } } + private unionMembers(...groups: ProcessIdentity[][]): ProcessIdentity[] { + const members: ProcessIdentity[] = [] + const seen = new Set() + for (const group of groups) { + for (const member of group) { + const key = JSON.stringify([member.pid, member.started]) + if (seen.has(key)) continue + seen.add(key) + members.push(member) + } + } + return members + } + private async stopDescendants(): Promise { - let members = this.descendants() - this.signalMembers(members, 'SIGTERM') - await this.waitForExit(members) + const captured = this.descendants() + this.signalMembers(captured, 'SIGTERM') + const capturedSurvivors = await this.waitForExit(captured) // A TERM-handling descendant may have forked while winding down. Rescan - // while the shell can still reap every member, then kill the fresh tree. - members = this.descendants() + // while the shell can still reap every member, then kill both the fresh + // tree and captured survivors that were reparented out of that tree. + const members = this.unionMembers(capturedSurvivors, this.descendants()) this.signalMembers(members, 'SIGKILL') - await this.waitForExit(members) - return this.descendants().filter(member => this.inspector.isAlive(member)) + const survivors = await this.waitForExit(members) + return this.survivors(this.unionMembers(survivors, this.descendants())) } private async stopShell(): Promise { diff --git a/packages/pty/pty-local/tests/index.spec.ts b/packages/pty/pty-local/tests/index.spec.ts index 55cc134787..9c28484adf 100644 --- a/packages/pty/pty-local/tests/index.spec.ts +++ b/packages/pty/pty-local/tests/index.spec.ts @@ -9,7 +9,6 @@ import SandboxProvider from '@deepseek-ai/dsh-sandbox' import type { ConfinedArgv, SandboxPolicy } from '@deepseek-ai/dsh-sandbox' import SandboxPolicyService, { setSandboxMode } from '@deepseek-ai/dsh-sandbox-policy' import PtyService, { PtySessionId } from '@deepseek-ai/dsh-pty' -import type { PtyBackendSession } from '@deepseek-ai/dsh-pty' import { LocalPtyBackend } from '@deepseek-ai/dsh-pty-local' import * as ptyLocal from '@deepseek-ai/dsh-pty-local' import type { ResolvedConfig } from '@deepseek-ai/dsh-pty-local/src/config.ts' @@ -64,6 +63,30 @@ function spec(owner: Agent, signal?: AbortSignal) { } } +function stubLocalSession(initialize: () => Promise = () => Promise.resolve()): LocalPtySession { + return { + motd: '', + initialize, + startSend: () => { throw new Error('unused') }, + read: () => { throw new Error('unused') }, + signal: () => Promise.resolve({ delivered: true, targetPgid: 1 }), + status: () => ({ kind: 'running' as const }), + close: () => Promise.resolve(), + } as unknown as LocalPtySession +} + +function registerStubLocalBackend(ctx: Context, createSession: () => LocalPtySession) { + return ctx.inject(['pty', 'sandbox', 'sandboxPolicy'], (providerCtx) => { + providerCtx.pty.registerBackend(new LocalPtyBackend( + providerCtx, + { ...config(), backendType: 'stub' }, + inspector, + (() => ({})) as never, + createSession, + )) + }) +} + describe('LocalPtyBackend startup rollback', () => { it('rejects pre-aborted setup and empty sandbox argv', async () => { const ctx = new Context() @@ -172,7 +195,7 @@ describe('pty-local plugin shape', () => { const loader = Object.create(Loader.prototype) as Loader const unwrapped = loader.unwrapExports(ptyLocal) as Record expect(unwrapped.name).toBe('pty-local') - expect(unwrapped.inject).toEqual(['agents', 'pty', 'sandbox', 'sandboxPolicy']) + expect(unwrapped.inject).toEqual(['pty', 'sandbox', 'sandboxPolicy']) expect(unwrapped.Config).toBeDefined() }) @@ -204,39 +227,45 @@ describe('pty-local plugin shape', () => { expect(() => { setSandboxMode(session, 'read-only') }).not.toThrow() }) - it('rejects an effective sandbox-mode change until the owner closes live terminals', async () => { + it('keeps the owner-lifetime sandbox fence after the local provider unloads', async () => { const ctx = new Context() await ctx.plugin(SessionStore) await ctx.plugin(AgentRegistry) await ctx.plugin(PtyService) - await ctx.plugin(EmptySandbox) + await ctx.plugin(RecordingSandbox) await ctx.plugin(SandboxPolicyService, { mode: 'danger-full-access', workspaceRoot: '/tmp' }) - await ctx.plugin(ptyLocal, config()) const session = ctx.sessions.create(SessionId('mode-owner')) + const ownerFiber = await ctx.plugin(() => {}) const owner: Agent = { - id: session.id, options: {}, session, status: 'idle', ctx, + id: session.id, options: {}, session, status: 'idle', ctx: ownerFiber.ctx, send() {}, steer() {}, inject() {}, cancel() {}, whenIdle: () => Promise.resolve(), } ctx.agents.register(owner) - const backendSession = { - motd: '', - startSend: () => { throw new Error('unused') }, - read: () => { throw new Error('unused') }, - signal: () => Promise.resolve({ delivered: true, targetPgid: 1 }), - status: () => ({ kind: 'running' as const }), - close: () => Promise.resolve(), - } satisfies PtyBackendSession - ctx.pty.registerBackend({ type: 'stub', spawn: () => Promise.resolve(backendSession) }) + const providerFiber = await registerStubLocalBackend(ctx, () => stubLocalSession()) const created = await ctx.pty.spawn(owner, { type: 'stub' }) + const unrelated = ctx.sessions.create(SessionId('unrelated-mode')) + expect(() => { setSandboxMode(unrelated, 'read-only') }).not.toThrow() + expect(() => { + session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) + }).not.toThrow() + expect(() => { setSandboxMode(session, 'danger-full-access') }).not.toThrow() + await providerFiber.dispose() + expect(ctx.pty.listBackends()).toEqual([]) expect(() => { setSandboxMode(session, 'read-only') }).toThrow( 'cannot change sandbox mode from "danger-full-access" to "read-only" while persistent terminal sessions are open or being created; wait for creation to settle and close them first', ) expect(session.events.filter(event => event.type === 'sandbox/mode')).toHaveLength(1) + const replacementFiber = await registerStubLocalBackend(ctx, () => stubLocalSession()) + const second = await ctx.pty.spawn(owner, { type: 'stub' }) + await replacementFiber.dispose() + expect(() => { setSandboxMode(session, 'read-only') }).toThrow('open or being created') + await ctx.pty.kill(owner, created.sessionId) + await ctx.pty.kill(owner, second.sessionId) expect(() => { setSandboxMode(session, 'read-only') }).not.toThrow() expect(session.events.filter(event => event.type === 'sandbox/mode')).toHaveLength(2) }) @@ -246,30 +275,23 @@ describe('pty-local plugin shape', () => { await ctx.plugin(SessionStore) await ctx.plugin(AgentRegistry) await ctx.plugin(PtyService) - await ctx.plugin(EmptySandbox) + await ctx.plugin(RecordingSandbox) await ctx.plugin(SandboxPolicyService, { mode: 'danger-full-access', workspaceRoot: '/tmp' }) - await ctx.plugin(ptyLocal, config()) const session = ctx.sessions.create(SessionId('pending-mode-owner')) + const ownerFiber = await ctx.plugin(() => {}) const owner: Agent = { - id: session.id, options: {}, session, status: 'idle', ctx, + id: session.id, options: {}, session, status: 'idle', ctx: ownerFiber.ctx, send() {}, steer() {}, inject() {}, cancel() {}, whenIdle: () => Promise.resolve(), } ctx.agents.register(owner) - const gate = Promise.withResolvers() - ctx.pty.registerBackend({ type: 'slow', spawn: () => gate.promise }) - const spawning = ctx.pty.spawn(owner, { type: 'slow' }) + const gate = Promise.withResolvers() + await registerStubLocalBackend(ctx, () => stubLocalSession(() => gate.promise)) + const spawning = ctx.pty.spawn(owner, { type: 'stub' }) expect(ctx.pty.hasOwnerActivity(owner)).toBe(true) expect(() => { setSandboxMode(session, 'read-only') }).toThrow('open or being created') - gate.resolve({ - motd: '', - startSend: () => { throw new Error('unused') }, - read: () => { throw new Error('unused') }, - signal: () => Promise.resolve({ delivered: true, targetPgid: 1 }), - status: () => ({ kind: 'running' as const }), - close: () => Promise.resolve(), - }) + gate.resolve(undefined) const created = await spawning await ctx.pty.kill(owner, created.sessionId) expect(ctx.pty.hasOwnerActivity(owner)).toBe(false) diff --git a/packages/pty/pty-local/tests/session.spec.ts b/packages/pty/pty-local/tests/session.spec.ts index 42d634e368..b3a536696d 100644 --- a/packages/pty/pty-local/tests/session.spec.ts +++ b/packages/pty/pty-local/tests/session.spec.ts @@ -415,6 +415,28 @@ describe('LocalPtySession bounds, signals, and teardown', () => { expect(terminal.kills).toEqual(['SIGTERM']) }) + it('retains captured survivors that are reparented out of the teardown rescan', async () => { + vi.useFakeTimers() + const terminal = new FakeTerminal() + const inspector = new FakeInspector() + const captured = { pid: 124, started: 'captured' } + let reads = 0 + inspector.alive.add(captured.pid) + inspector.processTree = () => reads++ === 0 ? [captured] : [] + inspector.signalProcess = (identity, signal) => { + inspector.processes.push([identity.pid, signal]) + if (signal === 'SIGKILL') inspector.alive.delete(identity.pid) + } + const session = new LocalPtySession(terminal.asPty(), inspector, config({ disposeGraceMs: 20 })) + + const closing = session.close('test') + await vi.advanceTimersByTimeAsync(25) + await closing + + expect(inspector.processes).toEqual([[124, 'SIGTERM'], [124, 'SIGKILL']]) + expect(terminal.kills).toEqual(['SIGTERM']) + }) + it('allows teardown to retry after a descendant-survivor failure', async () => { vi.useFakeTimers() const terminal = new FakeTerminal() diff --git a/packages/tasks/tool-tasks/src/index.ts b/packages/tasks/tool-tasks/src/index.ts index cee88db859..01f2092485 100644 --- a/packages/tasks/tool-tasks/src/index.ts +++ b/packages/tasks/tool-tasks/src/index.ts @@ -171,10 +171,10 @@ export function apply(ctx: Context, config: Config): void { }, execute(args, exec) { const id = validateTaskId(args.task_id) + const snapshot = ctx.tasks.get(id, exec.agent) const result = ctx.tasks.kill(id, exec.agent, args.reason) if (result === 'already-finished') { // A snapshot describes terminal state without consuming pending output. - const snapshot = ctx.tasks.get(id, exec.agent) return Promise.resolve([{ type: 'text', text: fitWithSuffix( @@ -185,7 +185,15 @@ export function apply(ctx: Context, config: Config): void { ), }]) } - return Promise.resolve([{ type: 'text', text: `requested cancellation of task ${id}` }]) + return Promise.resolve([{ + type: 'text', + text: fitWithSuffix( + `requested cancellation of task ${id}`, + '', + snapshot.outputLimitBytes, + '\n[notice truncated]', + ), + }]) }, presentCall: args => presentTaskCall(`Kill background task ${args.task_id}`, 'execute', args.task_id), })) diff --git a/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts b/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts index 886a1866fe..0256ac1cf8 100644 --- a/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts +++ b/packages/tasks/tool-tasks/tests/tool-tasks.spec.ts @@ -213,6 +213,16 @@ describe('task_kill', () => { expect(p.cancels).toEqual(['superseded']) }) + it('applies the producer output limit to a cancellation acknowledgement', async () => { + const { ctx } = await setup() + const p = producer({ outputLimitBytes: 8 }) + ctx.tasks.start(p.spec) + + const result = await call(ctx, 'task_kill', { task_id: 'bash-1' }) + expect(Buffer.byteLength(text(result))).toBeLessThanOrEqual(8) + expect(p.cancels).toEqual([undefined]) + }) + it('reports an already-finished task without consuming its pending delta', async () => { const { ctx } = await setup() let delta = 'unread tail'