From 6a6105dc1bcaeed65de283fa8cc6ac094b13c9bf Mon Sep 17 00:00:00 2001 From: creatixchu Date: Thu, 8 Oct 2026 14:59:28 +0800 Subject: [PATCH] fix(ptc): preserve tool execution identity during cancellation --- packages/core/tools/src/ptc.ts | 14 +++++++------- packages/core/tools/tests/ptc-owners.spec.ts | 6 +++++- .../ptc-runtime-node/src/persistent.ts | 2 +- .../tests/persistent-host.spec.ts | 15 ++++++++------- .../tests/persistent-sandbox.spec.ts | 8 ++++---- .../workflow-ptc/tests/workflow-ptc.spec.ts | 2 +- 6 files changed, 26 insertions(+), 21 deletions(-) diff --git a/packages/core/tools/src/ptc.ts b/packages/core/tools/src/ptc.ts index 4a99936a0d..99ece37b5d 100644 --- a/packages/core/tools/src/ptc.ts +++ b/packages/core/tools/src/ptc.ts @@ -369,7 +369,7 @@ export function createRunCodeTool(registry: ToolRuntime, options: RunCodeBridgeO const runtime = requireRuntime() const admission = stateOwners.enter(exec.agent, runtime, exec.signal, args.sandbox_permissions !== undefined, () => runtime.sandboxMode === undefined ? undefined : options.resolveSandboxPolicy(exec)) - exec = { ...exec, signal: admission.signal } + const callSignal = admission.signal try { validateEscalationArgs(args.sandbox_permissions, args.justification) if (args.timeoutMs !== undefined && runtime.timeout === undefined) { @@ -389,20 +389,20 @@ export function createRunCodeTool(registry: ToolRuntime, options: RunCodeBridgeO subject: 'program', }, { approver: options.peekApprover(), agent: exec.agent, callId: exec.callId, - toolName: RUN_CODE_NAME, signal: exec.signal, + toolName: RUN_CODE_NAME, signal: callSignal, }) policy = { ...standingPolicy, mode: approvedMode } } - const cwd = await options.resolveWorkingDirectory(exec) - exec.signal.throwIfAborted() + const cwd = await options.resolveWorkingDirectory({ ...exec, signal: callSignal }) + callSignal.throwIfAborted() // The run-scoped abort: follows the outer signal in, and fires when the // run settles for ANY reason, so an in-flight sub-dispatch is aborted // (its executor kills on this signal) instead of orphaned, and // queued-unstarted dispatches are abandoned. const runController = new AbortController() - const onOuterAbort = (): void => { runController.abort(exec.signal.reason) } - exec.signal.addEventListener('abort', onOuterAbort, { once: true }) + const onOuterAbort = (): void => { runController.abort(callSignal.reason) } + callSignal.addEventListener('abort', onOuterAbort, { once: true }) let dispatches = 0 // The per-run scheduler uses the registry's staged interface and follows @@ -719,7 +719,7 @@ export function createRunCodeTool(registry: ToolRuntime, options: RunCodeBridgeO ...result.value !== undefined ? { result: result.value } : {}, } } finally { - exec.signal.removeEventListener('abort', onOuterAbort) + callSignal.removeEventListener('abort', onOuterAbort) } } finally { await admission.finish() } }, diff --git a/packages/core/tools/tests/ptc-owners.spec.ts b/packages/core/tools/tests/ptc-owners.spec.ts index a2c5dff3e8..c72d9b9ec8 100644 --- a/packages/core/tools/tests/ptc-owners.spec.ts +++ b/packages/core/tools/tests/ptc-owners.spec.ts @@ -26,7 +26,11 @@ async function setup() { await ctx.plugin(SandboxPolicy, { mode: 'workspace-write' }) await ctx.plugin(Approval) ctx.provide('shell', { sandboxMode: 'workspace-write' }) - await ctx.plugin(PermissionPresets, {}) + await ctx.plugin(PermissionPresets, { presets: { + 'read-only': { sandbox: 'read-only', approval: 'ask' }, + 'workspace-write': { sandbox: 'workspace-write', approval: 'ask' }, + 'danger-full-access': { sandbox: 'danger-full-access', approval: 'never' }, + } }) await ctx.plugin(Runtime) const session = ctx.sessions.create(SessionId('ptc-permission-owner')) const agent = { ctx, session } as Agent diff --git a/packages/ptc-runtime/ptc-runtime-node/src/persistent.ts b/packages/ptc-runtime/ptc-runtime-node/src/persistent.ts index 3a20c86c20..b4614fa1b6 100644 --- a/packages/ptc-runtime/ptc-runtime-node/src/persistent.ts +++ b/packages/ptc-runtime/ptc-runtime-node/src/persistent.ts @@ -37,7 +37,7 @@ interface Evaluation { /** One managed process; all mutable program state belongs to this process alone. */ class Kernel { - private confined?: ConfinedArgv + private confined: ConfinedArgv | undefined private handle?: SubprocessHandle private channel?: JsonChannel private active: Evaluation | undefined diff --git a/packages/ptc-runtime/ptc-runtime-node/tests/persistent-host.spec.ts b/packages/ptc-runtime/ptc-runtime-node/tests/persistent-host.spec.ts index 5047bc6817..7a0c315052 100644 --- a/packages/ptc-runtime/ptc-runtime-node/tests/persistent-host.spec.ts +++ b/packages/ptc-runtime/ptc-runtime-node/tests/persistent-host.spec.ts @@ -187,11 +187,11 @@ describe('persistent PTC host protocol', () => { h.stderr.write('stderr') h.emit(frame({ type: 'done' })) }) - expect(await h.start()).toEqual({ logs: ['stdout', 'stderr'] }) + expect(await h.start()).toMatchObject({ logs: ['stdout', 'stderr'] }) h.stdout.write('idle stdout') h.stderr.write('idle stderr') h.onBoot(() => { h.emit(frame({ type: 'done' }, 2)) }) - expect(await h.start(request, NO_INITIAL_FRAME)).toEqual({ logs: [] }) + expect(await h.start(request, NO_INITIAL_FRAME)).toMatchObject({ logs: [] }) expect(h.terminate).not.toHaveBeenCalled() }) @@ -284,7 +284,7 @@ describe('persistent PTC host protocol', () => { ])('bounds cleanup failure after parse diagnostics overflow a $maxOutputBytes-byte budget', async ({ maxOutputBytes, kind }) => { const h = await setup({ maxOutputBytes }) h.onBoot(() => { h.emit(frame({ type: 'done' })) }) - expect(await h.start()).toEqual({ logs: [] }) + expect(await h.start()).toMatchObject({ logs: [] }) h.waitForExit.mockRejectedValue(new Error('x')) const result = await h.runtime.run(h.runtime.resolve({ ...request, program: 'return 1' })) expect(result.error?.kind).toBe(kind) @@ -293,7 +293,7 @@ describe('persistent PTC host protocol', () => { expect(bytes).toBeLessThanOrEqual(maxOutputBytes) expect(h.terminate).toHaveBeenCalledTimes(1) expect(h.waitForExit).toHaveBeenCalledTimes(1) - await h.runtime.release(request.stateOwner!) + await expect(h.runtime.release(request.stateOwner!)).rejects.toThrow('x') }) it('rejects invalid resolved inputs and deployment bounds', async () => { @@ -397,7 +397,7 @@ it('logs an idle process failure with its bounded stderr tail', async () => { } finally { reported.mockRestore() } }) -it('reports a release failure once and removes the failed environment from provider cleanup', async () => { +it('keeps an owner unavailable after release cleanup fails', async () => { const h = await setup() h.onBoot(() => { h.emit(frame({ type: 'done' })) }) await h.start() @@ -406,9 +406,10 @@ it('reports a release failure once and removes the failed environment from provi await expect(h.runtime.release(request.stateOwner!)).rejects.toBe(failure) const reported = vi.spyOn(h.fiber.ctx.logger, 'error').mockImplementation(() => {}) try { - await h.runtime.release(request.stateOwner!) + await expect(h.runtime.release(request.stateOwner!)).rejects.toBe(failure) + await expect(h.runtime.run(h.runtime.resolve(request))).rejects.toThrow('concurrent runs') await h.fiber.dispose() - expect(reported).not.toHaveBeenCalled() + expect(reported).toHaveBeenCalled() expect(h.waitForExit).toHaveBeenCalledOnce() } finally { reported.mockRestore() } }) diff --git a/packages/ptc-runtime/ptc-runtime-node/tests/persistent-sandbox.spec.ts b/packages/ptc-runtime/ptc-runtime-node/tests/persistent-sandbox.spec.ts index b2d0741681..08fc71304c 100644 --- a/packages/ptc-runtime/ptc-runtime-node/tests/persistent-sandbox.spec.ts +++ b/packages/ptc-runtime/ptc-runtime-node/tests/persistent-sandbox.spec.ts @@ -20,9 +20,9 @@ async function setup(mode: 'read-only' | 'workspace-write' = 'read-only') { it('retains variables under read-only confinement and denies writes across calls', async () => { const { cwd, run } = await setup() - expect((await run('var fs = await import("node:fs/promises"); var count = 1;')).error).toBeUndefined() + expect((await run('const fsp = await import("node:fs/promises"); var count = 1;')).error).toBeUndefined() expect((await run('++count')).value).toBe(2) - const result = await run('await fs.writeFile("denied.txt", String(count))') + const result = await run('await fsp.writeFile("denied.txt", String(count))') expect(result.sandbox).toMatchObject({ mode: 'read-only', denied: true }) await expect(readFile(join(cwd, 'denied.txt'))).rejects.toMatchObject({ code: 'ENOENT' }) expect((await run('count')).value).toBe(2) @@ -30,8 +30,8 @@ it('retains variables under read-only confinement and denies writes across calls it('retains workspace write access while refusing a sibling path', async () => { const { root, cwd, run } = await setup('workspace-write') - expect((await run('var fs = await import("node:fs/promises"); await fs.writeFile("inside.txt", "allowed")')).error).toBeUndefined() - const result = await run(`await fs.writeFile(${JSON.stringify(join(root, 'outside.txt'))}, "denied")`) + expect((await run('const fsp = await import("node:fs/promises"); await fsp.writeFile("inside.txt", "allowed")')).error).toBeUndefined() + const result = await run(`await fsp.writeFile(${JSON.stringify(join(root, 'outside.txt'))}, "denied")`) expect(result.sandbox).toMatchObject({ mode: 'workspace-write', denied: true }) expect(await readFile(join(cwd, 'inside.txt'), 'utf8')).toBe('allowed') await expect(readFile(join(root, 'outside.txt'))).rejects.toMatchObject({ code: 'ENOENT' }) diff --git a/packages/workflow/workflow-ptc/tests/workflow-ptc.spec.ts b/packages/workflow/workflow-ptc/tests/workflow-ptc.spec.ts index be7f7f6a2c..f33f331290 100644 --- a/packages/workflow/workflow-ptc/tests/workflow-ptc.spec.ts +++ b/packages/workflow/workflow-ptc/tests/workflow-ptc.spec.ts @@ -181,7 +181,7 @@ describe('dsh-workflow-ptc', { timeout: 120_000 }, () => { expect(result.stopReason).toBe('completed') expect(result.value).toBe('stub reply') expect(execute.mock.calls[0]?.[0].stateOwner).toBeUndefined() - expect(execute.mock.calls[0]?.[0].sandboxPolicy).toBeUndefined() + expect(execute.mock.calls[0]?.[0].sandboxPolicy?.mode).toBe('danger-full-access') }) it('captures args at start and isolates subsequent caller and script mutations', async () => {