From 672f2e5ec5c0bc339b97337f8fdffca6381e58fc Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 23 Jul 2026 00:11:20 +0800 Subject: [PATCH] fix(pty): await unpublished spawn teardown --- ...26-07-16-persistent-pty-sessions.i18n.yaml | 4 +- .../2026-07-16-persistent-pty-sessions.md | 4 +- .../2026-07-16-persistent-pty-sessions.zh.md | 4 +- packages/pty/pty/README.md | 1 + packages/pty/pty/src/index.ts | 70 ++++++++++++++---- packages/pty/pty/tests/service.spec.ts | 74 +++++++++++++++++-- 6 files changed, 130 insertions(+), 27 deletions(-) diff --git a/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.i18n.yaml b/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.i18n.yaml index f7998a7615..96d1e06bbe 100644 --- a/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -2026-07-16-persistent-pty-sessions.md: afa6f1771931ed437d8c2c34204ad4a59d02248e -2026-07-16-persistent-pty-sessions.zh.md: 2af97c3d126f8639e6b52685fb7fea1292343c9b +2026-07-16-persistent-pty-sessions.md: a73b8d0247243cc0d9cc93f27d84a8540b3b271d +2026-07-16-persistent-pty-sessions.zh.md: b0dc4010f69206b7aaeab4164199ffd935165955 diff --git a/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.md b/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.md index afa6f17719..a73b8d0247 100644 --- a/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.md +++ b/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.md @@ -34,7 +34,7 @@ Idle detection is backend behavior, not a second public seam. A remote or contai There are no plugin-load auto-start sessions. `terminal_open` creates a session only during an agent tool call, when ownership and the owning event-sourced session are known. A future declarative startup feature must compose through unpublished agent setup rather than create shared global terminals. -Agent-scope disposal closes registrations first, then awaits quiescent teardown of every owned PTY. Backend or tool-plugin reload does not orphan sessions: ownership lives in `PtyService` until the agent ends, following the same service-owned-record pattern as [`ctx.tasks`](../../../../packages/tasks/tasks/README.md). The service reserves the session synchronously for one active send before returning its operation, including before a background task id becomes visible; a second send fails with `SEND_ACTIVE`, so output and cancellation cannot cross operation ownership. +Agent-scope disposal closes registrations first, then awaits quiescent teardown of every owned PTY. Unpublished backend setup is a tracked lifecycle operation: owner or service disposal aborts its service-owned signal, waits for backend settlement and rollback, and only then returns. Caller cancellation retains its exact `AbortSignal.reason` even when the backend rejects in response. Backend or tool-plugin reload does not orphan sessions: ownership lives in `PtyService` until the agent ends, following the same service-owned-record pattern as [`ctx.tasks`](../../../../packages/tasks/tasks/README.md). The service reserves the session synchronously for one active send before returning its operation, including before a background task id becomes visible; a second send fails with `SEND_ACTIVE`, so output and cancellation cannot cross operation ownership. ### Security and process boundary @@ -152,7 +152,7 @@ The package ships concise tool guidance explaining persistent state, owner isola ## Verification -- Per-file coverage pins owner fencing, concurrent reservations, sandbox-mode change rejection, retriable lifecycle cleanup, readiness tiers, sanitizer carry state, complete UTF-8 bounds, task integration, schemas, and exact render intents. +- Per-file coverage pins owner fencing, concurrent reservations, unpublished-spawn cancellation and awaited teardown, sandbox-mode change rejection, retriable lifecycle cleanup, readiness tiers, sanitizer carry state, complete UTF-8 bounds, task integration, schemas, and exact render intents. - Linux process fixtures cover non-leader and non-main-thread stdin waits, zombie quiescence, unreadable process state, supported syscall tables, unsupported architectures, and false-positive rejection; macOS inspector logic is injected into the same unit suite. - Real `node-pty` tests exercise shell state, shared sandbox policy, environment scrubbing, raw-mode foreground `SIGINT`, a TERM-ignoring descendant, and immediate post-disposal quiescence on supported hosts. - A Loader-driven `cordis.yml` test mounts the real three-package composition, while ACP and headless snapshots pin the six schemas, bounded results, error rendering, and terminal/generic cards through opt-in overlays. diff --git a/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.zh.md b/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.zh.md index 2af97c3d12..b0dc4010f6 100644 --- a/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.zh.md +++ b/.agents/notes/implemented/feature/2026-07-16-persistent-pty-sessions.zh.md @@ -34,7 +34,7 @@ idle 检测属于后端行为，不是第二条公共 seam。远程或容器后 实现不提供插件加载期 auto-start 会话。`terminal_open` 只在 agent 工具调用期间创建会话，此时所有权和所属的事件溯源会话都已确定。未来的声明式启动功能必须通过尚未发布的 agent setup 组合，而不能创建全局共享终端。 -agent scope dispose 时先关闭注册，再等待全部所属 PTY 静默退出。后端或工具插件 reload 不会遗留会话：所有权持续存放在 `PtyService` 中，直到 agent 结束，与 [`ctx.tasks`](../../../../packages/tasks/tasks/README.md) 的服务持有记录模式一致。服务会先同步把会话预留给一次活跃发送，再返回该操作；后台发送同样会在 task id 对外可见前完成预留。第二次发送会以 `SEND_ACTIVE` 失败，因此输出与取消无法跨越操作所有权。 +agent scope dispose 时先关闭注册，再等待全部所属 PTY 静默退出。未发布的后端 setup 同样是受追踪的生命周期操作：owner 或服务 dispose 会中止服务自有的 signal，等待后端结算与回滚完成后才返回。即使后端响应取消而 reject，调用方取消仍原样保留其 `AbortSignal.reason`。后端或工具插件 reload 不会遗留会话：所有权持续存放在 `PtyService` 中，直到 agent 结束，与 [`ctx.tasks`](../../../../packages/tasks/tasks/README.md) 的服务持有记录模式一致。服务会先同步把会话预留给一次活跃发送，再返回该操作；后台发送同样会在 task id 对外可见前完成预留。第二次发送会以 `SEND_ACTIVE` 失败，因此输出与取消无法跨越操作所有权。 ### 安全与进程边界 @@ -152,7 +152,7 @@ plugins: ## 验证 -- 每文件覆盖率固定 owner 隔离、并发预留、沙箱模式变更拒绝、可重试的生命周期清理、就绪层级、sanitizer carry state、完整 UTF-8 结果上限、task 集成、schema 和精确 render intent。 +- 每文件覆盖率固定 owner 隔离、并发预留、未发布 spawn 的取消与等待式 teardown、沙箱模式变更拒绝、可重试的生命周期清理、就绪层级、sanitizer carry state、完整 UTF-8 结果上限、task 集成、schema 和精确 render intent。 - Linux 进程 fixture 覆盖非 leader 与非主线程的 stdin 等待、僵尸进程静止性、不可读进程状态、受支持的 syscall 表、不支持的架构和误报拒绝；同一单元测试套件通过注入覆盖 macOS 检查器逻辑。 - 真实 `node-pty` 测试在受支持宿主上覆盖 shell 状态、共享沙箱策略、环境清洗、raw mode 下的前台 `SIGINT`、忽略 `SIGTERM` 的子进程，以及 dispose 返回后立即静默。 - Loader 驱动的 `cordis.yml` 测试挂载真实三包组合；ACP 与 headless 快照通过 opt-in overlay 固定 6 个 schema、有界结果、错误渲染和 terminal/generic card。 diff --git a/packages/pty/pty/README.md b/packages/pty/pty/README.md index 6916a4ef82..624b771164 100644 --- a/packages/pty/pty/README.md +++ b/packages/pty/pty/README.md @@ -6,6 +6,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. - 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. - `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. - One session accepts at most one live send operation. Reads and signals may observe it; another send fails until the operation settles. diff --git a/packages/pty/pty/src/index.ts b/packages/pty/pty/src/index.ts index e809f50e3e..1b937b9dff 100644 --- a/packages/pty/pty/src/index.ts +++ b/packages/pty/pty/src/index.ts @@ -87,12 +87,22 @@ interface SessionRecord { closing: Promise | undefined } +interface PendingSpawn { + readonly controller: AbortController + readonly settled: Promise +} + +interface SpawnReservation { + readonly signal: AbortSignal + release(): void +} + /** In-process registry for replaceable PTY backends and exact-Agent sessions. */ export class PtyService extends Service { private readonly backends = new Map() private readonly sessions = new Map() private readonly reservedNames = new Map>() - private readonly pendingSpawns = new Map() + private readonly pendingSpawns = new Map>() private readonly ownerCleanups = new Map Promise | void>() private readonly disposedOwners = new WeakSet() private nextId = 0 @@ -145,7 +155,10 @@ export class PtyService extends Service { if (backend === undefined) throw new PtyError(`no PTY backend registered for "${request.type}"`, 'NO_BACKEND') if (request.name !== undefined && request.name.length === 0) throw new Error('PTY session name must be non-empty') const releaseName = this.reserveName(owner, request.name) - const releaseSpawn = this.reserveSpawn(owner) + const spawnReservation = this.reserveSpawn(owner) + const backendSignal = signal === undefined + ? spawnReservation.signal + : AbortSignal.any([signal, spawnReservation.signal]) const sessionId = PtySessionId(`pty-${++this.nextId}`) let session: PtyBackendSession | undefined try { @@ -155,7 +168,7 @@ export class PtyService extends Service { type: request.type, ...request.name !== undefined ? { name: request.name } : {}, ...request.cwd !== undefined ? { cwd: request.cwd } : {}, - ...signal !== undefined ? { signal } : {}, + signal: backendSignal, }) signal?.throwIfAborted() if (this.disposing) { @@ -176,16 +189,27 @@ export class PtyService extends Service { this.sessions.set(sessionId, record) return this.snapshot(record, session.motd) } catch (error) { + let rollbackFailure: { error: unknown } | undefined if (session !== undefined && !this.sessions.has(sessionId)) { try { await session.close('PTY spawn rolled back') } catch (closeError: unknown) { - throw new AggregateError([error, closeError], 'PTY spawn and rollback both failed') + rollbackFailure = { error: closeError } } } - throw error + let failure: unknown = error + try { + signal?.throwIfAborted() + spawnReservation.signal.throwIfAborted() + } catch (cancellation: unknown) { + failure = cancellation + } + if (rollbackFailure !== undefined) { + throw new AggregateError([failure, rollbackFailure.error], 'PTY spawn and rollback both failed') + } + throw failure } finally { - releaseSpawn() + spawnReservation.release() releaseName() } } @@ -196,7 +220,7 @@ export class PtyService extends Service { * @returns true across the entire spawn-to-close interval, with no publication gap. */ hasOwnerActivity(owner: Agent): boolean { - return (this.pendingSpawns.get(owner) ?? 0) > 0 + return (this.pendingSpawns.get(owner)?.size ?? 0) > 0 || [...this.sessions.values()].some(record => record.owner === owner) } @@ -314,15 +338,31 @@ export class PtyService extends Service { } } - private reserveSpawn(owner: Agent): () => void { - this.pendingSpawns.set(owner, (this.pendingSpawns.get(owner) ?? 0) + 1) - return () => { - const remaining = (this.pendingSpawns.get(owner) ?? 1) - 1 - if (remaining === 0) this.pendingSpawns.delete(owner) - else this.pendingSpawns.set(owner, remaining) + private reserveSpawn(owner: Agent): SpawnReservation { + const controller = new AbortController() + const settlement = Promise.withResolvers() + const pending: PendingSpawn = { controller, settled: settlement.promise } + const owned = this.pendingSpawns.get(owner) ?? new Set() + owned.add(pending) + this.pendingSpawns.set(owner, owned) + return { + signal: controller.signal, + release: () => { + owned.delete(pending) + if (owned.size === 0) this.pendingSpawns.delete(owner) + settlement.resolve() + }, } } + private async abortPendingSpawns(owner: Agent | undefined, reason: PtyError): Promise { + const pending = owner === undefined + ? [...this.pendingSpawns.values()].flatMap(owned => [...owned]) + : [...(this.pendingSpawns.get(owner) ?? [])] + for (const spawn of pending) spawn.controller.abort(reason) + await Promise.all(pending.map(spawn => spawn.settled)) + } + private expectOwned(owner: Agent, id: PtySessionId): SessionRecord { const record = this.sessions.get(id) if (record === undefined) throw new PtyError(`unknown PTY session ${id}`, 'NO_SESSION') @@ -344,6 +384,7 @@ export class PtyService extends Service { } private async disposeOwned(owner: Agent): Promise { + await this.abortPendingSpawns(owner, new PtyError('PTY owner is no longer live', 'OWNER_NOT_LIVE')) const owned = [...this.sessions.values()].filter(record => record.owner === owner) await this.closeRecords(owned, 'PTY owner disposed') this.reservedNames.delete(owner) @@ -351,11 +392,12 @@ export class PtyService extends Service { private async disposeAll(): Promise { this.disposing = true - const records = [...this.sessions.values()] // Teardown is best-effort: a close failure still clears registries and runs // owner cleanups before the aggregated error propagates, so one stuck // session cannot orphan backends, reservations, or owner detachers. try { + await this.abortPendingSpawns(undefined, new PtyError('PTY service is disposing', 'SERVICE_DISPOSING')) + const records = [...this.sessions.values()] await this.closeRecords(records, 'PTY service disposed') } finally { this.backends.clear() diff --git a/packages/pty/pty/tests/service.spec.ts b/packages/pty/pty/tests/service.spec.ts index 21587163f6..52d22bd4b3 100644 --- a/packages/pty/pty/tests/service.spec.ts +++ b/packages/pty/pty/tests/service.spec.ts @@ -206,9 +206,10 @@ describe('PtyService ownership and lifecycle', () => { ctx.agents.register(owner) const pending = ctx.pty.spawn(owner, { type: 'slow', name: 'main' }) await expect(ctx.pty.spawn(owner, { type: 'slow', name: 'main' })).rejects.toMatchObject({ code: 'DUPLICATE_NAME' }) - await disposeAgentScope(owner) + const disposal = disposeAgentScope(owner) gate.resolve(session) await expect(pending).rejects.toMatchObject({ code: 'OWNER_NOT_LIVE' }) + await disposal expect(session.closed).toEqual(['PTY spawn rolled back']) }) @@ -231,19 +232,70 @@ describe('PtyService ownership and lifecycle', () => { expect(ctx.agents.get(owner.id)).toBe(owner) }) - it('rolls back an unpublished backend session when service disposal wins', async () => { + it('preserves caller cancellation when a backend rejects in response to it', async () => { + const ctx = await harness() + const started = Promise.withResolvers() + const backendFailure = new Error('backend observed cancellation') + ctx.pty.registerBackend({ + type: 'abortable', + spawn: ({ signal }) => new Promise((_resolve, reject) => { + if (signal === undefined) throw new Error('missing spawn signal') + started.resolve(undefined) + signal.addEventListener('abort', () => { reject(backendFailure) }, { once: true }) + }), + }) + 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: 'abortable' }, controller.signal) + await started.promise + controller.abort(reason) + + await expect(pending).rejects.toBe(reason) + }) + + it.each([ + { scope: 'owner', code: 'OWNER_NOT_LIVE' }, + { scope: 'service', code: 'SERVICE_DISPOSING' }, + ] as const)('$scope disposal aborts and awaits unpublished backend setup', async ({ scope, code }) => { const ctx = await harness() const gate = Promise.withResolvers() + const started = Promise.withResolvers() const session = new StubSession() - ctx.pty.registerBackend({ type: 'slow', spawn: () => gate.promise }) + let backendSignal: AbortSignal | undefined + ctx.pty.registerBackend({ + type: 'slow', + spawn: (spec) => { + backendSignal = spec.signal + started.resolve(undefined) + return gate.promise + }, + }) const owner = stubAgent(ctx, 'owner') ctx.agents.register(owner) const pending = ctx.pty.spawn(owner, { type: 'slow' }) - await disposePtyService(ctx) + const pendingFailure = pending.then( + () => { throw new Error('pending spawn unexpectedly succeeded') }, + (error: unknown) => error, + ) + await started.promise + let disposalSettled = false + const disposal = (scope === 'owner' ? disposeAgentScope(owner) : disposePtyService(ctx)) + .then(() => { disposalSettled = true }) + await new Promise(resolve => setTimeout(resolve, 0)) + const signalAbortedBeforeRelease = backendSignal?.aborted ?? false + const signalReasonBeforeRelease = backendSignal?.reason as unknown + const disposalSettledBeforeRelease = disposalSettled gate.resolve(session) - await expect(pending).rejects.toMatchObject({ code: 'SERVICE_DISPOSING' }) + expect(await pendingFailure).toMatchObject({ code }) + await disposal + expect(signalAbortedBeforeRelease).toBe(true) + expect(signalReasonBeforeRelease).toMatchObject({ code }) + expect(disposalSettledBeforeRelease).toBe(false) expect(session.closed).toEqual(['PTY spawn rolled back']) }) @@ -290,14 +342,22 @@ describe('PtyService ownership and lifecycle', () => { ctx.agents.register(owner) const failedSpawn = new StubSession() failedSpawn.rejectClose = true + let ownerDisposal = Promise.resolve() ctx.pty.registerBackend({ type: 'bad-spawn', - async spawn() { - await disposeAgentScope(owner) + async spawn({ signal }) { + if (signal === undefined) throw new Error('missing spawn signal') + ownerDisposal = disposeAgentScope(owner) + if (!signal.aborted) { + await new Promise((resolve) => { + signal.addEventListener('abort', () => { resolve(undefined) }, { once: true }) + }) + } return failedSpawn }, }) await expect(ctx.pty.spawn(owner, { type: 'bad-spawn' })).rejects.toThrow('spawn and rollback both failed') + await ownerDisposal const nextOwner = stubAgent(ctx, 'next') ctx.agents.register(nextOwner)