fix(pty): close final review gaps

packages/pty/pty/README.md

@@ -7,7 +7,7 @@ Owner-scoped persistent PTY seam. `PtyService` registers as `ctx.pty`, mints opa
 - Backends register one stable `type` and return an unpublished `PtyBackendSession`; failed or cancelled setup must clean partial resources, and a failed cleanup rejects with `PtyBackendCleanupError` so the registry can retain it across cancellation.
 - Spawn cancellation preserves the caller's exact abort reason. Service disposal and owner loss remain distinct machine-routable failures after backend setup.
 - Owner and service disposal abort unpublished setup through a service-owned signal and await backend settlement plus rollback before returning.
-- A service rollback or backend-reported startup cleanup failure rejects the disposing lifecycle instead of claiming quiescence; the spawn caller still receives its exact cancellation reason.
+- A rollback-close or backend-reported startup cleanup failure rejects the disposing lifecycle instead of claiming quiescence. Caller-triggered cancellation still receives its exact reason; lifecycle-triggered rollback failure also rejects the pending spawn.
 - A backend cleanup failure that follows caller cancellation remains owner activity until owner or service disposal consumes and reports it, so lifecycle policy cannot mistake failed cleanup for quiescence.
 - `hasOwnerActivity(owner)` spans unpublished setup through final close, so lifecycle policy can fence the exact owner without a publication race.
 - A successful spawn publishes one `PtySessionId`. The optional `name` is owner-local display metadata, never authority.

packages/pty/pty/src/index.ts

@@ -213,7 +213,7 @@ export class PtyService extends Service {
       } catch (cancellation: unknown) {
         failure = cancellation
       }
-      if (rollbackFailure !== undefined) {
+      if (rollbackFailure !== undefined && signal?.aborted !== true) {
         throw new AggregateError([failure, rollbackFailure.error], 'PTY spawn and rollback both failed')
       }
       throw failure

packages/pty/pty/tests/service.spec.ts

@@ -233,6 +233,29 @@ describe('PtyService ownership and lifecycle', () => {
     expect(ctx.agents.get(owner.id)).toBe(owner)
   })
 
+  it('preserves caller cancellation when unpublished rollback fails', async () => {
+    const ctx = await harness()
+    const gate = Promise.withResolvers<PtyBackendSession>()
+    const session = new StubSession()
+    session.rejectClose = true
+    ctx.pty.registerBackend({ type: 'slow', spawn: () => gate.promise })
+    const owner = stubAgent(ctx, 'owner')
+    ctx.agents.register(owner)
+    const controller = new AbortController()
+    const reason = new Error('cancelled by caller')
+
+    const pending = ctx.pty.spawn(owner, { type: 'slow' }, controller.signal)
+    controller.abort(reason)
+    gate.resolve(session)
+
+    await expect(pending).rejects.toBe(reason)
+    expect(ctx.pty.hasOwnerActivity(owner)).toBe(true)
+    const internal = ctx.pty as unknown as { disposeAll(): Promise<void> }
+    await expect(internal.disposeAll()).rejects.toThrow('failed to clean up PTY lifecycle')
+    expect(ctx.pty.hasOwnerActivity(owner)).toBe(false)
+    expect(session.closed).toEqual(['PTY spawn rolled back'])
+  })
+
   it('preserves caller cancellation when a backend rejects in response to it', async () => {
     const ctx = await harness()
     const started = Promise.withResolvers<undefined>()

packages/pty/tool-pty/README.md

@@ -11,7 +11,7 @@ Six model-facing tools over `ctx.pty`: `terminal_open`, `terminal_send`, `termin
 | `enableRunInBackground` | `true` | expose and accept `run_in_background`; false omits the schema field and rejects a forced undeclared argument |
 | `maxResultBytes` | `262144` | UTF-8 cap (minimum `64`) for each complete terminal result or PTY task output after wait, session, pagination, truncation, and task-status metadata |
 
-Both values are validated at load. The minimum result cap keeps every registry-issued session or task id visible in its creation acknowledgement. When a result exceeds `maxResultBytes`, rendering reserves space for control metadata and a truncation marker when they fit; cuts preserve UTF-8 boundaries. An outer `tools/post-execute` wrapper applies the same cap after a terminal pre-execute denial or single-text post-execute replacement/block; a structured multi-block policy result retains its shape.
+Both values are validated at load. The minimum result cap keeps every registry-issued session or task id visible in its creation acknowledgement. When a result exceeds `maxResultBytes`, rendering reserves space for control metadata and a truncation marker when they fit; cuts preserve UTF-8 boundaries. Each terminal definition's final-content callback applies the same cap after normalized pre-, around-, and post-execute policy failures, denials, short-circuits, replacements, or blocks; a structured multi-block policy result retains its shape.
 
 ## Model Experience
 
@@ -53,7 +53,7 @@ Prefix-stable while tool visibility and definitions are unchanged.
 
 #### What the model sees
 
-Spawn returns the id and bounded MOTD. Send/read return bounded terminal text plus readiness/history markers. Background mode returns a generic task id. Every terminal-owned or policy-produced single-text result is capped by `maxResultBytes` after normalized errors, denials, replacements, blocks, and generic task status text. Structured multi-block policy results retain their shape. Results remain in session history until compaction; incremental task reads do not repeat consumed output.
+Spawn returns the id and bounded MOTD. Send/read return bounded terminal text plus readiness/history markers. Background mode returns a generic task id. Every terminal-owned or policy-produced single-text result is capped by `maxResultBytes` after normalized tool or pipeline errors, denials, short-circuits, replacements, blocks, and generic task status text. Structured multi-block policy results retain their shape. Results remain in session history until compaction; incremental task reads do not repeat consumed output.
 
 #### Token effect
 

packages/pty/tool-pty/src/index.ts

@@ -12,7 +12,7 @@ import { PtySessionId } from '@deepseek-ai/dsh-pty'
 import type { PtySendResult, PtySessionId as PtySessionIdType, PtySignal } from '@deepseek-ai/dsh-pty'
 import type {} from '@deepseek-ai/dsh-tasks'
 import { defineTool } from '@deepseek-ai/dsh-tools'
-import type { PostToolDecision, ToolExecutionResult } from '@deepseek-ai/dsh-tools'
+import type { ToolDefinition, ToolExecutionResult } from '@deepseek-ai/dsh-tools'
 import { boundTerminalText, renderList, renderRead, renderSend, renderSendRead, renderSpawn } from './render.ts'
 
 declare module '@deepseek-ai/dsh-tasks' {
@@ -31,15 +31,6 @@ export const DEFAULT_MAX_RESULT_BYTES = 256 * 1024
 /** Smallest cap that preserves every counter-backed PTY and task id in its creation acknowledgement. */
 export const MIN_MAX_RESULT_BYTES = 64
 
-const TOOL_NAMES = new Set([
-  'terminal_open',
-  'terminal_send',
-  'terminal_read',
-  'terminal_signal',
-  'terminal_close',
-  'terminal_list',
-])
-
 /** Model-facing terminal tool configuration. */
 export interface Config {
   /** Expose `run_in_background` and accept background sends (default true). */
@@ -114,17 +105,10 @@ export function apply(ctx: Context, config: Config = {}): void {
   if (!Number.isSafeInteger(maxResultBytes) || maxResultBytes < MIN_MAX_RESULT_BYTES) {
     throw new Error(`tool-pty: maxResultBytes must be a safe integer of at least ${MIN_MAX_RESULT_BYTES}`)
   }
-  ctx.on('tools/post-execute', async (exec, result, next): Promise<PostToolDecision> => {
-    const decision = await next()
-    if (!TOOL_NAMES.has(exec.name)) return decision
-    const content = decision.kind === 'block' ? decision.feedback : decision.content ?? result.content
-    const raw = rawContentText(content)
-    if (raw === undefined) return decision
-    const bounded = textResult(raw, maxResultBytes)
-    return decision.kind === 'block'
-      ? { ...decision, feedback: bounded }
-      : { ...decision, content: bounded }
-  }, { prepend: true })
+  const finalizeContent: NonNullable<ToolDefinition['finalizeContent']> = (_exec, result) => {
+    const raw = rawContentText(result.content)
+    return raw === undefined ? undefined : textResult(raw, maxResultBytes)
+  }
   ctx.systemPrompt.section({
     name: 'tool:pty',
     order: 106,
@@ -139,6 +123,7 @@ export function apply(ctx: Context, config: Config = {}): void {
       name: { type: 'string', description: 'Optional owner-local display name such as "main" or "gdb".' },
       cwd: { type: 'string', description: 'Initial working directory. Defaults to the deployment workspace root.' },
     },
+    finalizeContent,
     async execute(args: SpawnArgs, exec) {
       if (args.type.length === 0) throw new Error('type must be a non-empty string')
       const result = await ctx.pty.spawn(requireAgent(exec.agent), {
@@ -166,6 +151,7 @@ export function apply(ctx: Context, config: Config = {}): void {
         ? { run_in_background: { type: 'boolean' as const, description: 'Return a task id immediately; collect with task_output or stop with task_kill.' } }
         : {},
     },
+    finalizeContent,
     async execute(args: SendArgs, exec): Promise<ToolExecutionResult> {
       const owner = requireAgent(exec.agent)
       const id = sessionId(args)
@@ -224,6 +210,7 @@ export function apply(ctx: Context, config: Config = {}): void {
       offset: { type: 'number', description: 'Newest-relative line offset (default 0).' },
       count: { type: 'number', description: 'Requested line count (default 500; backend caps apply).' },
     },
+    finalizeContent,
     execute(args: ReadArgs, exec) {
       const result = ctx.pty.read(requireAgent(exec.agent), sessionId(args), {
         ...args.offset !== undefined ? { offset: args.offset } : {},
@@ -241,6 +228,7 @@ export function apply(ctx: Context, config: Config = {}): void {
       sessionId: { type: 'string', required: true, description: 'Terminal session id.' },
       signal: { type: 'string', required: true, enum: ['SIGINT', 'SIGTERM', 'SIGKILL', 'SIGTSTP', 'SIGHUP'], description: 'Signal to deliver. Shell-targeted SIGKILL is rejected; use terminal_close.' },
     },
+    finalizeContent,
     async execute(args: SignalArgs, exec) {
       const result = await ctx.pty.signal(requireAgent(exec.agent), sessionId(args), args.signal)
       return textResult(`delivered ${args.signal} to foreground process group ${result.targetPgid}`, maxResultBytes)
@@ -254,6 +242,7 @@ export function apply(ctx: Context, config: Config = {}): void {
     parameters: {
       sessionId: { type: 'string', required: true, description: 'Terminal session id.' },
     },
+    finalizeContent,
     async execute(args: SessionArgs, exec) {
       const id = sessionId(args)
       const closed = await ctx.pty.kill(requireAgent(exec.agent), id)
@@ -266,6 +255,7 @@ export function apply(ctx: Context, config: Config = {}): void {
     name: 'terminal_list',
     description: 'List persistent terminal sessions owned by the current agent.',
     parameters: {},
+    finalizeContent,
     execute(_args: Record<string, never>, exec) {
       return Promise.resolve(textResult(renderList(ctx.pty.list(requireAgent(exec.agent)), maxResultBytes), maxResultBytes))
     },

packages/pty/tool-pty/tests/tools.spec.ts

@@ -217,11 +217,17 @@ describe('tool-pty foreground surface', () => {
     expect(Buffer.byteLength(text(background))).toBeLessThanOrEqual(64)
   })
 
-  it('bounds terminal results after pre- and post-execute policy', async () => {
+  it('bounds terminal results after policy decisions and pipeline failures', async () => {
     const { ctx, agent } = await setup(false, { maxResultBytes: 64 })
-    ctx.on('tools/pre-execute', async (exec, next) => exec.name === 'terminal_list'
-      ? { kind: 'deny', reason: 'd'.repeat(1_000) }
-      : next())
+    ctx.on('tools/pre-execute', async (exec, next) => {
+      if (exec.name === 'terminal_list') return { kind: 'deny', reason: 'd'.repeat(1_000) }
+      if (exec.name === 'terminal_signal') throw new Error(`pre failed: ${'p'.repeat(1_000)}`)
+      return next()
+    })
+    ctx.on('tools/execute', async (exec, next) => {
+      if (exec.name === 'terminal_close') throw new Error(`around failed: ${'e'.repeat(1_000)}`)
+      return next()
+    })
     ctx.on('tools/post-execute', async (exec, _result, next) => {
       if (exec.name === 'terminal_open') {
         return { kind: 'accept', content: [{ type: 'text', text: 'a'.repeat(1_000) }] }
@@ -229,6 +235,7 @@ describe('tool-pty foreground surface', () => {
       if (exec.name === 'terminal_read') {
         return { kind: 'block', feedback: [{ type: 'text', text: 'b'.repeat(1_000) }] }
       }
+      if (exec.name === 'terminal_send') throw new Error(`post failed: ${'o'.repeat(1_000)}`)
       return next()
     })
 
@@ -246,6 +253,17 @@ describe('tool-pty foreground surface', () => {
     expect(blocked.isError).toBe(true)
     expect(Buffer.byteLength(text(blocked))).toBeLessThanOrEqual(64)
     expect(text(blocked)).toContain('[output truncated]')
+
+    const failures = [
+      await call(ctx, 'terminal_signal', { sessionId: 'pty-1', signal: 'SIGINT' }, agent),
+      await call(ctx, 'terminal_close', { sessionId: 'pty-1' }, agent),
+      await call(ctx, 'terminal_send', { sessionId: 'pty-1', text: 'work' }, agent),
+    ]
+    for (const failure of failures) {
+      expect(failure.isError).toBe(true)
+      expect(Buffer.byteLength(text(failure))).toBeLessThanOrEqual(64)
+      expect(text(failure)).toContain('[output truncated]')
+    }
   })
 
   it('leaves a structured around-dispatch replacement unchanged', async () => {