From 0df80cdf4127daa2263ed9cf17225d0c3f493a47 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Fri, 4 Sep 2026 08:08:52 +0800 Subject: [PATCH] fix(auto-review): carry lifecycle cancellation to dispatch --- packages/interaction/auto-review/package.json | 2 +- packages/interaction/auto-review/src/index.ts | 46 ++++++++++--- .../auto-review/tests/auto-review.spec.ts | 69 +++++++++++++++++++ .../web/ptc-round/system-prompt.expected.md | 2 +- 4 files changed, 108 insertions(+), 11 deletions(-) diff --git a/packages/interaction/auto-review/package.json b/packages/interaction/auto-review/package.json index 1bdd4b4356..bec0bc70bb 100644 --- a/packages/interaction/auto-review/package.json +++ b/packages/interaction/auto-review/package.json @@ -1,7 +1,7 @@ { "name": "@deepseek-ai/dsh-auto-review", "description": "Per-tool LLM authorization review for the DeepSeek Harness Auto permission preset", - "version": "0.1.2-alpha.5", + "version": "0.1.2-rc.1", "publishConfig": { "access": "public" }, diff --git a/packages/interaction/auto-review/src/index.ts b/packages/interaction/auto-review/src/index.ts index 63251726ef..b3b6ca597e 100644 --- a/packages/interaction/auto-review/src/index.ts +++ b/packages/interaction/auto-review/src/index.ts @@ -103,9 +103,10 @@ interface ReviewSnapshot { readonly action: PendingAction } -/** Runtime facts owned by one in-flight review. */ -interface ActiveReview { +/** Runtime facts owned until one admitted call cannot outlive lifecycle cancellation. */ +interface ActiveCall { readonly done: Promise + readonly complete: () => void } /** Cordis plugin name used by loader diagnostics. */ @@ -587,10 +588,17 @@ export function apply(ctx: Context): void { throw new Error('auto-review: preset "read-only" must resolve to sandbox "read-only" and approval "ask"') } let accepting = true - const active = new Set() + const active = new Map() const retiring = new Set() const lifecycle = new AbortController() + const completeActive = (exec: Pick): void => { + const call = active.get(exec.token) + if (call === undefined) return + active.delete(exec.token) + call.complete() + } + ctx.effect(function* () { const stopListener = ctx.on('tools/pre-execute', async (exec, next): Promise => { const agent = exec.agent @@ -606,8 +614,11 @@ export function apply(ctx: Context): void { } const completed = Promise.withResolvers() - const activeReview: ActiveReview = { done: completed.promise } - active.add(activeReview) + active.set(exec.token, { + done: completed.promise, + complete: () => { completed.resolve(undefined) }, + }) + let admitted = false try { const signal = AbortSignal.any([exec.signal, lifecycle.signal]) const decision = await classifyRisk(ctx, agent, exec, signal).catch(() => undefined) @@ -615,13 +626,30 @@ export function apply(ctx: Context): void { if (decision === undefined) return denied(exec) if (decision.decision === 'deny') return denied(exec, decision.reason) const downstream = await next() - return isAborted(lifecycle.signal) ? { kind: 'cancel' } : downstream + if (isAborted(lifecycle.signal)) return { kind: 'cancel' } + admitted = downstream.kind === 'allow' + return downstream } finally { - active.delete(activeReview) - completed.resolve(undefined) + if (!admitted) completeActive(exec) } }, { prepend: true }) yield stopListener + const stopDispatch = ctx.on('tools/execute', async (exec, next) => { + if (!active.has(exec.token)) return next() + const originalSignal = exec.signal + exec.signal = AbortSignal.any([originalSignal, lifecycle.signal]) + completeActive(exec) + try { + return await next() + } finally { + exec.signal = originalSignal + } + }, { prepend: true }) + yield stopDispatch + const stopResult = ctx.on('tools/result', (exec) => { + completeActive(exec) + }) + yield stopResult const stopContribution = permissionPresets.registerAuto(() => { if (!accepting) throw new Error('auto-review: integration is closing') }) @@ -642,7 +670,7 @@ export function apply(ctx: Context): void { } } lifecycle.abort(new Error('auto-review integration disposed')) - await Promise.allSettled([...active].map(item => item.done)) + await Promise.allSettled([...active.values()].map(item => item.done)) if (errors.length > 0) throw new AggregateError(errors, 'auto-review: failed to migrate live Auto sessions') } }, 'auto-review lifecycle') diff --git a/packages/interaction/auto-review/tests/auto-review.spec.ts b/packages/interaction/auto-review/tests/auto-review.spec.ts index bee7403af2..7ce1223321 100644 --- a/packages/interaction/auto-review/tests/auto-review.spec.ts +++ b/packages/interaction/auto-review/tests/auto-review.spec.ts @@ -1208,6 +1208,75 @@ describe('cancellation and integration teardown', () => { expect(probe.runs()).toBe(0) }) + it('fuses lifecycle cancellation before an execute-stage checkpoint', async () => { + const checkpointEntered = Promise.withResolvers() + const releaseCheckpoint = Promise.withResolvers() + let checkpointSignal: AbortSignal | undefined + let checkpointExecution: { readonly signal: AbortSignal } | undefined + const { ctx, auto } = await harness([ + decisionChunks('{"risk":"medium","decision":"allow"}'), + ]) + const probe = registerProbe(ctx) + ctx.on('tools/execute', async (exec, next) => { + if (exec.name !== 'probe') return next() + checkpointExecution = exec + checkpointSignal = exec.signal + checkpointEntered.resolve(undefined) + await releaseCheckpoint.promise + return next() + }) + const { session, agent } = autoSession(ctx, 'dispose-execute-checkpoint') + appendHeader(session, [{ name: 'probe', description: 'probe', parameters: { type: 'object' } }]) + const callId = ToolCallId('dispose-execute-checkpoint-call') + appendAssistant(session, [{ type: 'tool-call', id: callId, name: 'probe', arguments: '{}' }]) + appendNativeCall(session, callId, 'probe', '{}') + const controller = new AbortController() + const pending = ctx.tools.execute({ + signal: controller.signal, callId, name: 'probe', arguments: {}, agent, + }) + await checkpointEntered.promise + expect(checkpointSignal).not.toBe(controller.signal) + + const disposal = auto.dispose() + await until(() => checkpointSignal?.aborted === true) + releaseCheckpoint.resolve(undefined) + + await expect(pending).resolves.toMatchObject({ + isError: true, + error: { info: { name: 'AbortError', code: TOOL_ABORTED_BEFORE_DISPATCH } }, + }) + await disposal + expect(probe.runs()).toBe(0) + expect(checkpointExecution?.signal).toBe(controller.signal) + }) + + it('cleans an admitted call that caller cancellation finalizes without dispatch', async () => { + const controller = new AbortController() + const { ctx, auto } = await harness([ + decisionChunks('{"risk":"medium","decision":"allow"}'), + ]) + const probe = registerProbe(ctx) + ctx.on('tools/pre-execute', async (exec, next): Promise => { + const decision = await next() + if (exec.name === 'probe') controller.abort(new Error('caller stopped before dispatch')) + return decision + }) + const { session, agent } = autoSession(ctx, 'caller-cancel-no-dispatch') + appendHeader(session, [{ name: 'probe', description: 'probe', parameters: { type: 'object' } }]) + const callId = ToolCallId('caller-cancel-no-dispatch-call') + appendAssistant(session, [{ type: 'tool-call', id: callId, name: 'probe', arguments: '{}' }]) + appendNativeCall(session, callId, 'probe', '{}') + + await expect(ctx.tools.execute({ + signal: controller.signal, callId, name: 'probe', arguments: {}, agent, + })).resolves.toMatchObject({ + isError: true, + error: { info: { name: 'AbortError', code: TOOL_ABORTED_BEFORE_DISPATCH } }, + }) + expect(probe.runs()).toBe(0) + await expect(auto.dispose()).resolves.toBeUndefined() + }) + it('reinstall restores Auto availability without re-enabling a migrated session', async () => { const { ctx, auto } = await harness([]) const { session } = autoSession(ctx, 'reinstall-after-dispose') diff --git a/snapshots/web/ptc-round/system-prompt.expected.md b/snapshots/web/ptc-round/system-prompt.expected.md index 9119e08eb5..fc9cac9176 100644 --- a/snapshots/web/ptc-round/system-prompt.expected.md +++ b/snapshots/web/ptc-round/system-prompt.expected.md @@ -1,6 +1,6 @@ You are an AI agent powered by DeepSeek Harness. -The DeepSeek Harness implementation checkout is at /Users/xht/.codex/goal-worktrees/{{sessionId}}/implementation/. The checkout location and current working directory are separate values and may differ; never infer the working directory from this path. Use pwd to determine the current working directory. Use this checkout only to inspect or extend DSH itself. +The DeepSeek Harness implementation checkout is at {{sourceRoot}}. The checkout location and current working directory are separate values and may differ; never infer the working directory from this path. Use pwd to determine the current working directory. Use this checkout only to inspect or extend DSH itself. You are interacting with the user through the DeepSeek Harness Web GUI at {{webUrl}}. When the user refers to "this page", "this GUI", or "this app" without naming another target, they mean this GUI. The browser provides no implicit DOM, route, or screenshot context. The client-plugin HMR receiver is active, but client-plugin changes reload without a refresh only while `pnpm run dev:web` is also running from this same checkout to rebuild their bundles; verify that watcher before promising automatic updates. Every other change — the apps/web shell and plain packages — requires rebuilding the affected Web artifacts and verifying this existing URL after a page refresh. Starting another server does not update this GUI. The apps/web Vite entry builds the shell but is not a standalone application because only dsh web injects window.__DSH_BOOT__. Do not start a replacement server unless the user asks; if one is needed, use a managed background job and verify its exact URL.