From 7c2829c0e0cb0c7eb073035f5ae26033b2ec8cea Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:50:57 -0500 Subject: [PATCH] fix(copilot): isolate settled turn finalization --- docs/plugins/sdk-agent-harness.md | 25 ++ extensions/copilot/harness.test.ts | 58 ++++ extensions/copilot/harness.ts | 350 ++++++++++---------- extensions/copilot/src/attempt.live.test.ts | 183 +++++++--- extensions/copilot/src/attempt.test.ts | 133 ++++++++ extensions/copilot/src/attempt.ts | 163 ++++++++- extensions/copilot/src/runtime.test.ts | 14 + extensions/copilot/src/runtime.ts | 13 +- 8 files changed, 699 insertions(+), 240 deletions(-) diff --git a/docs/plugins/sdk-agent-harness.md b/docs/plugins/sdk-agent-harness.md index bfc1e9ac9bc2..0e66194b8943 100644 --- a/docs/plugins/sdk-agent-harness.md +++ b/docs/plugins/sdk-agent-harness.md @@ -475,6 +475,31 @@ harnesses. Optional does not mean ignorable for a harness that executes tools: without terminal reports, OpenClaw cannot preserve mutating-tool failure truth across later tool calls, including quiet heartbeat completion. +### Settled tool finalization + +OpenClaw may need one final visible answer after a harness has completed every +tool call but its native turn ended without assistant text. A harness can opt +into that recovery by implementing `finalizeSettledTurn({ attempt, +settledAttempt })`. + +The callback is a separate capability, not another ordinary attempt. It must: + +- continue the exact native transcript that contains the settled tool results; +- expose no tools, permission-grant or user-input capabilities, native execution + hooks, agents, skills, memory, scheduling, extensions, or remote control; +- send only the host-provided finalization prompt; and +- fail closed if the existing native session cannot be resumed with those + restrictions. + +Do not implement this callback by calling `runAttempt` with a best-effort +`disableTools` hint. The harness owner must enforce the complete native +capability boundary. OpenClaw does not provide a generic fallback because it +cannot attest that an arbitrary native runtime honored those restrictions. + +The callback remains optional for experimental third-party harness +compatibility. When the selected harness omits it, OpenClaw preserves the +existing incomplete-turn error instead of risking repeated side effects. + ## Current limitations - The public import path is generic, but some attempt/result type aliases diff --git a/extensions/copilot/harness.test.ts b/extensions/copilot/harness.test.ts index 6004b69f2234..bfc83156a8ca 100644 --- a/extensions/copilot/harness.test.ts +++ b/extensions/copilot/harness.test.ts @@ -376,6 +376,64 @@ describe("createCopilotAgentHarness", () => { ); }); + it("finalizes settled tools by resuming the compatible SDK session in isolated mode", async () => { + const pool = makePoolMock(); + const client = createMockCopilotClient({ deleteSession: vi.fn() }); + const settledResult = asAttemptResult({ assistantTexts: [] }); + const finalResult = asAttemptResult({ assistantTexts: ["final answer"] }); + const params = asAttemptParams({ + ...ATTEMPT_PARAMS, + initialReplayState: { replayInvalid: true }, + sessionId: "openclaw-session-finalize", + }); + mocks.runCopilotAttempt + .mockImplementationOnce(async (_params, deps) => { + deps.onSessionEstablished?.({ + sdkSessionId: "sdk-session-finalize", + pooledClient: { client, key: TEST_POOL_KEY }, + sessionConfig: TEST_SESSION_CONFIG, + }); + return settledResult; + }) + .mockResolvedValueOnce(finalResult); + const harness = createCopilotAgentHarness({ pool }); + + await expect(harness.runAttempt(params)).resolves.toBe(settledResult); + await expect( + harness.finalizeSettledTurn?.({ attempt: params, settledAttempt: settledResult }), + ).resolves.toBe(finalResult); + + expect(mocks.runCopilotAttempt).toHaveBeenCalledTimes(2); + expect(mocks.runCopilotAttempt.mock.calls[1]?.[0]).toMatchObject({ + disableTools: true, + initialReplayState: { sdkSessionId: "sdk-session-finalize" }, + sessionId: "openclaw-session-finalize", + }); + expect(mocks.runCopilotAttempt.mock.calls[1]?.[0]?.initialReplayState).not.toHaveProperty( + "replayInvalid", + ); + expect(mocks.runCopilotAttempt.mock.calls[1]?.[1]).toMatchObject({ + operation: "settled-tool-finalization", + pool, + }); + expect(mocks.runCopilotAttempt.mock.calls[1]?.[1]?.onSessionEstablished).toBeUndefined(); + }); + + it("fails closed when settled finalization has no compatible SDK session", async () => { + const harness = createCopilotAgentHarness({ pool: makePoolMock() }); + const params = asAttemptParams({ + ...ATTEMPT_PARAMS, + sessionId: "openclaw-session-missing", + }); + + await expect( + harness.finalizeSettledTurn?.({ attempt: params, settledAttempt: ATTEMPT_RESULT }), + ).rejects.toThrow( + "cannot safely finalize a settled tool turn without its compatible SDK session", + ); + expect(mocks.runCopilotAttempt).not.toHaveBeenCalled(); + }); + it("multiple harness instances create independent pools", async () => { const poolOne = makePoolMock(); const poolTwo = makePoolMock(); diff --git a/extensions/copilot/harness.ts b/extensions/copilot/harness.ts index d051eb52b09e..cd15611d57ce 100644 --- a/extensions/copilot/harness.ts +++ b/extensions/copilot/harness.ts @@ -654,6 +654,180 @@ export function createCopilotAgentHarness( await Promise.allSettled(pending.map(([cleanup]) => cleanup)); } + async function runHarnessAttempt( + params: AgentHarnessAttemptParams, + operation: "attempt" | "settled-tool-finalization", + ): Promise { + const attemptPromise = (async () => { + if (disposed) { + throw new Error("[copilot] harness has been disposed; cannot start new attempts"); + } + const { resolvePoolAcquire, runCopilotAttempt } = await import("./src/attempt.js"); + if (disposed) { + throw new Error("[copilot] harness was disposed while starting an attempt"); + } + const pool = await getPool(); + if (disposed) { + throw new Error("[copilot] harness was disposed while starting an attempt"); + } + let poolAcquire: ReturnType; + try { + poolAcquire = resolvePoolAcquire(params as never); + } catch (error) { + // Keep invalid forced BYOK model configuration on the normal attempt + // result path so callers receive `model_not_supported` instead of an + // uncaught harness rejection. Finalization cannot safely create a new + // incompatible session and therefore keeps the failure closed. + if (operation === "attempt" && isCopilotByokUnsupportedProviderError(error)) { + return runCopilotAttempt(params, { pool }); + } + throw error; + } + const openclawSessionId = typeof params.sessionId === "string" ? params.sessionId : undefined; + + // Reuse the SDK session across turns within the same OpenClaw session so + // Copilot's prompt cache, tool history, and compaction state survive. + // Compatibility covers provider/model/cwd/auth; incompatible state starts + // a fresh ordinary attempt but cannot be used for settled finalization. + const currentCompatKey = computeSessionCompatKey(params); + const currentCompactKey = computeSessionCompactKey(params); + const compactionCleanupPending = + openclawSessionId !== undefined && hasPendingDeferredCompactionCleanup(openclawSessionId); + const replayBlocked = + openclawSessionId !== undefined && + (compactionCleanupPending || resetBlockedStoredSessions.has(openclawSessionId)); + const tracked = + openclawSessionId && !replayBlocked ? trackedSessions.get(openclawSessionId) : undefined; + const stored = openclawSessionId + ? replayBlocked + ? undefined + : lookupStoredBinding(options?.sessionStore, openclawSessionId) + : undefined; + const resumableSessionId = + tracked && tracked.compatKey === currentCompatKey + ? tracked.sdkSessionId + : !tracked && stored && stored.compatKey === currentCompatKey + ? stored.sdkSessionId + : undefined; + if (operation === "settled-tool-finalization" && !resumableSessionId) { + throw new Error( + "[copilot] cannot safely finalize a settled tool turn without its compatible SDK session", + ); + } + const effectiveParams: AgentHarnessAttemptParams = resumableSessionId + ? ({ + ...params, + ...(operation === "settled-tool-finalization" ? { disableTools: true } : {}), + // Finalization is a new, isolated turn over settled state, not a + // replay of the side-effecting prompt. Ignore replayInvalid while + // still requiring the exact compatible native session above. + initialReplayState: + operation === "settled-tool-finalization" + ? { sdkSessionId: resumableSessionId } + : { + ...params.initialReplayState, + sdkSessionId: resumableSessionId, + }, + } as AgentHarnessAttemptParams) + : params; + + return runCopilotAttempt(effectiveParams, { + pool, + ...(operation === "settled-tool-finalization" ? { operation } : {}), + onSessionEstablished: + operation === "attempt" && openclawSessionId + ? ({ + compactionSessionConfig, + sdkSessionId, + pooledClient, + sessionConfig, + }: { + compactionSessionConfig?: CopilotSessionConfig; + sdkSessionId: string; + pooledClient: PooledClient; + sessionConfig: CopilotSessionConfig; + }) => { + trackedSessions.set(openclawSessionId, { + sdkSessionId, + client: pooledClient.client, + clientOptions: poolAcquire.options, + compatKey: currentCompatKey, + compactKey: currentCompactKey, + poolKey: pooledClient.key, + sessionConfig: compactionSessionConfig ?? sessionConfig, + ...sessionAuthFields(poolAcquire.auth), + }); + registerStoredBinding(options?.sessionStore, openclawSessionId, { + schemaVersion: 2, + sdkSessionId, + compatKey: currentCompatKey, + compactKey: currentCompactKey, + ...sessionAuthFields(poolAcquire.auth), + updatedAt: Date.now(), + }); + resetBlockedStoredSessions.delete(openclawSessionId); + } + : undefined, + onDeferredCompaction: openclawSessionId + ? ({ + abort, + cleanup, + sdkSessionId, + }: { + abort: () => void; + cleanup: Promise; + sdkSessionId: string; + }) => { + const trackedBinding = trackedSessions.get(openclawSessionId); + const storedBinding = lookupStoredBinding(options?.sessionStore, openclawSessionId); + const ownsTrackedSession = trackedBinding?.sdkSessionId === sdkSessionId; + const ownsStoredSession = storedBinding?.sdkSessionId === sdkSessionId; + if (!ownsTrackedSession && !ownsStoredSession) { + return; + } + trackDeferredCompactionCleanup({ + abort, + cleanup, + sessionId: openclawSessionId, + sdkSessionId, + }); + // The attempt retains this SDK session until its background + // compaction resolves. Preserve its binding for a successful + // completion, but do not let a new turn resume it yet. + resetBlockedStoredSessions.add(openclawSessionId); + void cleanup.then((outcome) => { + const currentTracked = trackedSessions.get(openclawSessionId); + const currentStored = lookupStoredBinding(options?.sessionStore, openclawSessionId); + const stillOwnsTrackedSession = currentTracked?.sdkSessionId === sdkSessionId; + const stillOwnsStoredSession = currentStored?.sdkSessionId === sdkSessionId; + if (outcome === "completed") { + if (stillOwnsTrackedSession || stillOwnsStoredSession) { + resetBlockedStoredSessions.delete(openclawSessionId); + } + return; + } + if (stillOwnsTrackedSession) { + trackedSessions.delete(openclawSessionId); + } + if (stillOwnsStoredSession) { + deleteStoredBinding(options?.sessionStore, openclawSessionId); + } + if (stillOwnsTrackedSession || stillOwnsStoredSession) { + resetBlockedStoredSessions.add(openclawSessionId); + } + }); + } + : undefined, + }); + })(); + inFlight.add(attemptPromise); + try { + return await attemptPromise; + } finally { + inFlight.delete(attemptPromise); + } + } + return { id: options?.id ?? "copilot", label: options?.label ?? "GitHub Copilot agent runtime", @@ -702,181 +876,9 @@ export function createCopilotAgentHarness( return { supported: true, priority: 100 }; }, - async runAttempt(params: AgentHarnessAttemptParams): Promise { - const attemptPromise = (async () => { - if (disposed) { - throw new Error("[copilot] harness has been disposed; cannot start new attempts"); - } - const { resolvePoolAcquire, runCopilotAttempt } = await import("./src/attempt.js"); - if (disposed) { - throw new Error("[copilot] harness was disposed while starting an attempt"); - } - const pool = await getPool(); - if (disposed) { - throw new Error("[copilot] harness was disposed while starting an attempt"); - } - let poolAcquire: ReturnType; - try { - poolAcquire = resolvePoolAcquire(params as never); - } catch (error) { - // Keep invalid forced BYOK model configuration on the normal attempt - // result path so callers receive `model_not_supported` instead of an - // uncaught harness rejection. Other auth/pool errors remain fatal. - if (isCopilotByokUnsupportedProviderError(error)) { - return runCopilotAttempt(params, { pool }); - } - throw error; - } - const openclawSessionId = - typeof params.sessionId === "string" ? params.sessionId : undefined; + runAttempt: (params) => runHarnessAttempt(params, "attempt"), - // Dogfood finding #4: reuse the SDK session across turns within - // the same OpenClaw session so that the GitHub Copilot agent runtime's prompt - // cache, tool-call history, and any server-side compaction state - // survive turn boundaries. Without this, every turn called - // `createSession()` and lost cache + thread continuity — the - // smoking gun was distinct `${sdkSessionId}` scopes per turn in - // the playground transcript. - // - // Safety: - // - Only inject when the tracked compatKey still matches the - // current attempt's fingerprint (provider/model/cwd/auth). - // Mismatch falls through to `createSession` and the new SDK - // session replaces the tracked entry below. - // - Preserve any caller-provided `replayInvalid: true` — never - // downgrade an orchestrator-issued safety signal to false. - // `decideReplayAction` treats undefined as resumable already. - // - On resume failure, `attempt.ts` recovers via the - // `replay-shim` (`resumeFailureRecovered:true`) and falls - // back to `createSession`, so a stale-session error never - // surfaces as a prompt error. - const currentCompatKey = computeSessionCompatKey(params); - const currentCompactKey = computeSessionCompactKey(params); - const compactionCleanupPending = - openclawSessionId !== undefined && hasPendingDeferredCompactionCleanup(openclawSessionId); - const replayBlocked = - openclawSessionId !== undefined && - (compactionCleanupPending || resetBlockedStoredSessions.has(openclawSessionId)); - const tracked = - openclawSessionId && !replayBlocked ? trackedSessions.get(openclawSessionId) : undefined; - const stored = openclawSessionId - ? replayBlocked - ? undefined - : lookupStoredBinding(options?.sessionStore, openclawSessionId) - : undefined; - const resumableSessionId = - tracked && tracked.compatKey === currentCompatKey - ? tracked.sdkSessionId - : !tracked && stored && stored.compatKey === currentCompatKey - ? stored.sdkSessionId - : undefined; - const effectiveParams: AgentHarnessAttemptParams = resumableSessionId - ? ({ - ...params, - initialReplayState: { - ...params.initialReplayState, - sdkSessionId: resumableSessionId, - }, - } as AgentHarnessAttemptParams) - : params; - - return runCopilotAttempt(effectiveParams, { - pool, - onSessionEstablished: openclawSessionId - ? ({ - compactionSessionConfig, - sdkSessionId, - pooledClient, - sessionConfig, - }: { - compactionSessionConfig?: CopilotSessionConfig; - sdkSessionId: string; - pooledClient: PooledClient; - sessionConfig: CopilotSessionConfig; - }) => { - trackedSessions.set(openclawSessionId, { - sdkSessionId, - client: pooledClient.client, - clientOptions: poolAcquire.options, - compatKey: currentCompatKey, - compactKey: currentCompactKey, - poolKey: pooledClient.key, - sessionConfig: compactionSessionConfig ?? sessionConfig, - ...sessionAuthFields(poolAcquire.auth), - }); - registerStoredBinding(options?.sessionStore, openclawSessionId, { - schemaVersion: 2, - sdkSessionId, - compatKey: currentCompatKey, - compactKey: currentCompactKey, - ...sessionAuthFields(poolAcquire.auth), - updatedAt: Date.now(), - }); - resetBlockedStoredSessions.delete(openclawSessionId); - } - : undefined, - onDeferredCompaction: openclawSessionId - ? ({ - abort, - cleanup, - sdkSessionId, - }: { - abort: () => void; - cleanup: Promise; - sdkSessionId: string; - }) => { - const trackedBinding = trackedSessions.get(openclawSessionId); - const storedBinding = lookupStoredBinding(options?.sessionStore, openclawSessionId); - const ownsTrackedSession = trackedBinding?.sdkSessionId === sdkSessionId; - const ownsStoredSession = storedBinding?.sdkSessionId === sdkSessionId; - if (!ownsTrackedSession && !ownsStoredSession) { - return; - } - trackDeferredCompactionCleanup({ - abort, - cleanup, - sessionId: openclawSessionId, - sdkSessionId, - }); - // The attempt retains this SDK session until its background - // compaction resolves. Preserve its binding for a successful - // completion, but do not let a new turn resume it yet. - resetBlockedStoredSessions.add(openclawSessionId); - void cleanup.then((outcome) => { - const currentTracked = trackedSessions.get(openclawSessionId); - const currentStored = lookupStoredBinding( - options?.sessionStore, - openclawSessionId, - ); - const stillOwnsTrackedSession = currentTracked?.sdkSessionId === sdkSessionId; - const stillOwnsStoredSession = currentStored?.sdkSessionId === sdkSessionId; - if (outcome === "completed") { - if (stillOwnsTrackedSession || stillOwnsStoredSession) { - resetBlockedStoredSessions.delete(openclawSessionId); - } - return; - } - if (stillOwnsTrackedSession) { - trackedSessions.delete(openclawSessionId); - } - if (stillOwnsStoredSession) { - deleteStoredBinding(options?.sessionStore, openclawSessionId); - } - if (stillOwnsTrackedSession || stillOwnsStoredSession) { - resetBlockedStoredSessions.add(openclawSessionId); - } - }); - } - : undefined, - }); - })(); - inFlight.add(attemptPromise); - try { - return await attemptPromise; - } finally { - inFlight.delete(attemptPromise); - } - }, + finalizeSettledTurn: ({ attempt }) => runHarnessAttempt(attempt, "settled-tool-finalization"), async reset(params: AgentHarnessResetParams): Promise { const openclawSessionId = typeof params.sessionId === "string" ? params.sessionId : undefined; diff --git a/extensions/copilot/src/attempt.live.test.ts b/extensions/copilot/src/attempt.live.test.ts index 1131572e83ab..147f765f2e36 100644 --- a/extensions/copilot/src/attempt.live.test.ts +++ b/extensions/copilot/src/attempt.live.test.ts @@ -2,7 +2,8 @@ import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { CopilotClient, approveAll } from "@github/copilot-sdk"; +import { CopilotClient } from "@github/copilot-sdk"; +import type { SessionConfig } from "@github/copilot-sdk"; import type { AgentHarnessAttemptParams } from "openclaw/plugin-sdk/agent-harness-runtime"; import { isLiveTestEnabled } from "openclaw/plugin-sdk/test-live"; import { describe, expect, it, vi } from "vitest"; @@ -12,10 +13,14 @@ import type { CopilotClientPool } from "./runtime.js"; const liveToolState = vi.hoisted(() => ({ calls: [] as string[], expectedText: "phase-1-green", + permissionRequests: 0, sentinelPrefix: "copilot-live-smoke:", toolName: "live_echo", + userInputRequests: 0, })); +const LIVE_MODEL_PREFERENCES = ["gpt-5.4-mini", "gpt-5.4", "gpt-5.6-luna"] as const; + vi.mock("openclaw/plugin-sdk/agent-harness", async (importOriginal) => { const actual = await importOriginal(); @@ -66,22 +71,47 @@ const TOKEN = ""; const describeLive = LIVE && TOKEN ? describe : describe.skip; -function createApproveAllPool(): CopilotClientPool { +function wrapLiveSessionConfig(config: SessionConfig): SessionConfig { + const onPermissionRequest = config.onPermissionRequest; + const onUserInputRequest = config.onUserInputRequest; + return { + ...config, + ...(onPermissionRequest + ? { + onPermissionRequest: async (...args: Parameters) => { + liveToolState.permissionRequests += 1; + return onPermissionRequest(...args); + }, + } + : {}), + ...(onUserInputRequest + ? { + onUserInputRequest: async (...args: Parameters) => { + liveToolState.userInputRequests += 1; + return onUserInputRequest(...args); + }, + } + : {}), + }; +} + +function createLivePool(): CopilotClientPool { const activeClients = new Set(); return { async acquire(key, options) { - const client = new CopilotClient(options); + const { copilotHome, ...clientOptions } = options; + const client = new CopilotClient({ ...clientOptions, baseDirectory: copilotHome }); activeClients.add(client); return { key, client: { createSession: (config: Parameters[0]) => - client.createSession({ ...config, onPermissionRequest: approveAll }), + client.createSession(wrapLiveSessionConfig(config)), resumeSession: ( sessionId: Parameters[0], config: Parameters[1], - ) => client.resumeSession(sessionId, { ...config, onPermissionRequest: approveAll }), + ) => client.resumeSession(sessionId, wrapLiveSessionConfig(config)), stop: () => client.stop(), } as unknown as CopilotClient, }; @@ -105,8 +135,32 @@ function createApproveAllPool(): CopilotClientPool { }; } +async function resolveLiveModelId(copilotHome: string): Promise { + const client = new CopilotClient({ baseDirectory: copilotHome, gitHubToken: TOKEN }); + try { + await client.start(); + const available = (await client.listModels()).filter( + (model) => model.policy?.state !== "disabled", + ); + for (const preferred of LIVE_MODEL_PREFERENCES) { + if (available.some((model) => model.id === preferred)) { + return preferred; + } + } + const fallback = available[0]?.id; + if (!fallback) { + throw new Error("Copilot live smoke found no enabled models"); + } + return fallback; + } finally { + await client.stop(); + } +} + function createAttemptParams(params: { copilotHome: string; + modelId: string; + onAgentEvent?: (event: unknown) => void | Promise; onAssistantDelta: (payload: { text: string }) => void | Promise; prompt: string; }): AgentHarnessAttemptParams { @@ -128,10 +182,11 @@ function createAttemptParams(params: { messages: [{ content: params.prompt, role: "user", timestamp: now }], model: { api: "openai-responses", - id: "gpt-4.1", + id: params.modelId, provider: "github-copilot", }, - modelId: "gpt-4.1", + modelId: params.modelId, + onAgentEvent: params.onAgentEvent, onAssistantDelta: params.onAssistantDelta, profileVersion, prompt: params.prompt, @@ -145,71 +200,105 @@ function createAttemptParams(params: { } describeLive("copilot agent runtime live smoke", () => { - it("runs one turn on gpt-4.1 with one custom tool", async () => { + it("uses one custom tool, then resumes with an isolated finalization turn", async () => { liveToolState.calls.length = 0; + liveToolState.permissionRequests = 0; + liveToolState.userInputRequests = 0; const streamedTexts: string[] = []; - const prompt = `Use the ${liveToolState.toolName} tool exactly once with text '${liveToolState.expectedText}', then reply with exactly two short sentences totaling at least twelve words.`; + const finalEventTypes: string[] = []; + const prompt = `Use the ${liveToolState.toolName} tool exactly once with text '${liveToolState.expectedText}', then reply with one short sentence.`; const copilotHome = await mkdtemp(join(tmpdir(), "openclaw-copilot-live-")); - const harness = createCopilotAgentHarness({ pool: createApproveAllPool() }); - - expect( - harness.supports({ - provider: "github-copilot", - modelId: "gpt-4.1", - requestedRuntime: "copilot", - }), - ).toEqual({ supported: true, priority: 100 }); + const modelId = await resolveLiveModelId(copilotHome); + const harness = createCopilotAgentHarness({ pool: createLivePool() }); try { - const result = await harness.runAttempt( - createAttemptParams({ - copilotHome, - onAssistantDelta: ({ text }) => { - if (text.trim()) { - streamedTexts.push(text); - } - }, - prompt, + expect( + harness.supports({ + provider: "github-copilot", + modelId, + requestedRuntime: "copilot", }), - ); - const assistantText = result.assistantTexts.join("\n").trim(); - const hasAssistantText = result.assistantTexts.some((text) => text.trim().length > 0); + ).toEqual({ supported: true, priority: 100 }); + + const attempt = createAttemptParams({ + copilotHome, + modelId, + onAssistantDelta: ({ text }) => { + if (text.trim()) { + streamedTexts.push(text); + } + }, + prompt, + }); + const settledResult = await harness.runAttempt(attempt); const matchingCalls = liveToolState.calls.filter( (text) => text === liveToolState.expectedText, ); - const usage = result.attemptUsage; + expect(settledResult.promptError).toBeUndefined(); + expect(settledResult.timedOut).toBe(false); + expect(matchingCalls).toHaveLength(1); + expect( + settledResult.toolMetas.some( + (toolMeta) => + toolMeta.toolName === liveToolState.toolName && + toolMeta.meta?.includes(liveToolState.sentinelPrefix), + ), + ).toBe(true); + + const finalPrompt = "Reply with exactly COPILOT-SETTLED-FINALIZER-OK and nothing else."; + const finalResult = await harness.finalizeSettledTurn?.({ + attempt: { + ...attempt, + onAgentEvent: (event: unknown) => { + const type = (event as { type?: unknown } | undefined)?.type; + if (typeof type === "string") { + finalEventTypes.push(type); + } + }, + prompt: finalPrompt, + runId: `${attempt.runId}-finalize`, + }, + settledAttempt: settledResult, + }); + if (!finalResult) { + throw new Error("Copilot harness did not expose settled tool finalization"); + } + const assistantText = finalResult.assistantTexts.join("\n").trim(); + const finalCapabilityEvents = finalEventTypes.filter((type) => + /(tool|permission|user.?input|subagent)/i.test(type), + ); console.info( "[copilot-live-smoke] summary", JSON.stringify( { assistantText, + finalCapabilityEvents, + finalEventTypes, + modelId, + permissionRequests: liveToolState.permissionRequests, toolCalls: liveToolState.calls, streamedTexts, - toolMetas: result.toolMetas, - usage, + toolMetas: settledResult.toolMetas, + usage: finalResult.attemptUsage, + userInputRequests: liveToolState.userInputRequests, }, null, 2, ), ); - expect(result.promptError).toBeUndefined(); - expect(result.timedOut).toBe(false); - expect(matchingCalls.length).toBeGreaterThanOrEqual(1); - expect(hasAssistantText).toBe(true); - expect(assistantText.length).toBeGreaterThan(0); - expect((usage?.input ?? 0) + (usage?.output ?? 0)).toBeGreaterThan(0); - expect( - result.toolMetas.some( - (toolMeta) => - toolMeta.toolName === liveToolState.toolName && - toolMeta.meta?.includes(liveToolState.sentinelPrefix), - ), - ).toBe(true); + expect(finalResult.promptError).toBeUndefined(); + expect(finalResult.timedOut).toBe(false); + expect(assistantText).toBe("COPILOT-SETTLED-FINALIZER-OK"); + expect(liveToolState.calls).toEqual([liveToolState.expectedText]); + expect(finalResult.toolMetas).toEqual([]); + expect(finalCapabilityEvents).toEqual([]); + expect(liveToolState.permissionRequests).toBe(0); + expect(liveToolState.userInputRequests).toBe(0); } finally { await harness.dispose?.(); await rm(copilotHome, { recursive: true, force: true }); } - }, 90_000); + }, 180_000); }); diff --git a/extensions/copilot/src/attempt.test.ts b/extensions/copilot/src/attempt.test.ts index 4c8426a796f7..0b3b6721995e 100644 --- a/extensions/copilot/src/attempt.test.ts +++ b/extensions/copilot/src/attempt.test.ts @@ -3494,6 +3494,139 @@ describe("runCopilotAttempt", () => { }); }); + describe("settled tool finalization isolation", () => { + it("requires an existing SDK session before constructing any capability surface", async () => { + const sdk = makeFakeSdk(); + const createToolBridge = vi.fn(async () => ({ sdkTools: [], sourceTools: [] })); + + const result = await runCopilotAttempt(makeParams(), { + createToolBridge, + operation: "settled-tool-finalization", + pool: makeFakePool(sdk), + }); + + expect(getPromptErrorCode(result)).toBe("settled_finalization_session_unavailable"); + expect(createToolBridge).not.toHaveBeenCalled(); + expect(sdk.createSession).not.toHaveBeenCalled(); + expect(sdk.resumeSession).not.toHaveBeenCalled(); + }); + + it("resumes with every ambient Copilot capability disabled", async () => { + const sdk = makeFakeSdk({ + onResumeSession: (session) => { + session.sendAndWait.mockResolvedValueOnce(makeAssistantMessageEvent("final answer")); + }, + }); + const permissivePolicy = vi.fn(async () => ({ kind: "approved" }) as never); + const nativeHook = vi.fn(); + const onSessionEstablished = vi.fn(); + const pool = makeFakePool(sdk); + const sdkTool = { + description: "must never be exposed", + handler: async () => ({ resultType: "success", textResultForLlm: "unsafe" }), + name: "unsafe_tool", + parameters: { type: "object" }, + } satisfies SdkTool; + const createToolBridge = vi.fn(async () => ({ sdkTools: [sdkTool], sourceTools: [] })); + + const result = await runCopilotAttempt( + makeParams({ + disableTools: false, + hooksConfig: { onPreToolUse: nativeHook }, + infiniteSessionConfig: { enabled: true }, + initialReplayState: { replayInvalid: true, sdkSessionId: "sdk-settled-session" }, + permissionPolicy: permissivePolicy, + } as never), + { + createToolBridge, + onSessionEstablished, + operation: "settled-tool-finalization", + pool, + }, + ); + + expect(result.promptError).toBeUndefined(); + expect(result.assistantTexts).toEqual(["final answer"]); + expect(result.toolMetas).toEqual([]); + expect(sdk.createSession).not.toHaveBeenCalled(); + expect(sdk.resumeSession).toHaveBeenCalledTimes(1); + expect(pool.acquire).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ mode: "empty" }), + ); + const cfg = requireResumeSessionConfig(sdk); + expect(cfg).toMatchObject({ + availableTools: [], + coauthorEnabled: false, + continuePendingWork: false, + customAgents: [], + customAgentsLocalOnly: true, + embeddingCacheStorage: "in-memory", + enableConfigDiscovery: false, + enableFileHooks: false, + enableHostGitOperations: false, + enableOnDemandInstructionDiscovery: false, + enableSessionStore: false, + enableSkills: false, + excludedTools: ["builtin:*", "mcp:*", "custom:*"], + includeSubAgentStreamingEvents: false, + infiniteSessions: { enabled: false }, + instructionDirectories: [], + manageScheduleEnabled: false, + mcpOAuthTokenStorage: "in-memory", + mcpServers: {}, + memory: { enabled: false }, + pluginDirectories: [], + remoteSession: "off", + requestCanvasRenderer: false, + requestExtensions: false, + skillDirectories: [], + skipCustomInstructions: true, + skipEmbeddingRetrieval: true, + tools: [], + }); + expect(cfg).not.toHaveProperty("hooks"); + expect(cfg).not.toHaveProperty("onUserInputRequest"); + expect(createToolBridge).toHaveBeenCalledWith( + expect.objectContaining({ + attemptParams: expect.objectContaining({ disableTools: true }), + }), + ); + const permissionHandler = cfg.onPermissionRequest as ( + request: unknown, + invocation: unknown, + ) => Promise<{ kind: string }>; + await expect( + permissionHandler({ kind: "shell" }, { sessionId: "sdk-settled-session" }), + ).resolves.toMatchObject({ kind: "reject" }); + expect(permissivePolicy).not.toHaveBeenCalled(); + expect(nativeHook).not.toHaveBeenCalled(); + expect(onSessionEstablished).not.toHaveBeenCalled(); + }); + + it("fails closed instead of creating a fresh session when resume is stale", async () => { + const sdk = makeFakeSdk({ + onResumeSession: () => { + throw new Error("session not found"); + }, + }); + + const result = await runCopilotAttempt( + makeParams({ + initialReplayState: { sdkSessionId: "sdk-stale-session" }, + } as never), + { + operation: "settled-tool-finalization", + pool: makeFakePool(sdk), + }, + ); + + expect(getPromptErrorCode(result)).toBe("settled_finalization_resume_failed"); + expect(sdk.resumeSession).toHaveBeenCalledTimes(1); + expect(sdk.createSession).not.toHaveBeenCalled(); + }); + }); + // ClawSweeper PR #86155 [P1] round-8: the SDK SessionConfig accepts // `availableTools` as a hard catalog allowlist // (`@github/copilot-sdk/dist/types.d.ts:1059-1066`). Without it, the diff --git a/extensions/copilot/src/attempt.ts b/extensions/copilot/src/attempt.ts index 73682505c1f2..0af6c7f835ab 100644 --- a/extensions/copilot/src/attempt.ts +++ b/extensions/copilot/src/attempt.ts @@ -66,6 +66,9 @@ import { resolveCopilotWorkspaceBootstrapContext } from "./workspace-bootstrap.j const BACKGROUND_COMPACTION_CANCEL_TIMEOUT_MS = 5_000; const COPILOT_ASK_USER_AVAILABLE_TOOLS = ["builtin:ask_user"] as const; +const COPILOT_SETTLED_FINALIZATION_EXCLUDED_TOOLS = ["builtin:*", "mcp:*", "custom:*"] as const; + +type CopilotAttemptOperation = "attempt" | "settled-tool-finalization"; type AttemptResultWithSdkSessionId = AgentHarnessAttemptResult & { sdkSessionId?: string }; type PromptErrorWithCode = Error & { code?: string; cause?: unknown }; @@ -73,16 +76,39 @@ type CopilotAgentEndHookParams = Parameters[0]; export type CopilotSessionConfig = Pick< SessionConfig, | "availableTools" + | "coauthorEnabled" + | "customAgents" + | "customAgentsLocalOnly" + | "embeddingCacheStorage" + | "enableConfigDiscovery" + | "enableFileHooks" + | "enableHostGitOperations" + | "enableOnDemandInstructionDiscovery" + | "enableSessionStore" + | "enableSkills" | "enableSessionTelemetry" + | "excludedTools" | "gitHubToken" | "hooks" + | "includeSubAgentStreamingEvents" | "instructionDirectories" | "infiniteSessions" + | "manageScheduleEnabled" + | "mcpOAuthTokenStorage" + | "mcpServers" + | "memory" | "model" | "onPermissionRequest" | "onUserInputRequest" + | "pluginDirectories" | "provider" | "reasoningEffort" + | "remoteSession" + | "requestCanvasRenderer" + | "requestExtensions" + | "skipCustomInstructions" + | "skipEmbeddingRetrieval" + | "skillDirectories" | "systemMessage" | "tools" | "workingDirectory" @@ -159,6 +185,7 @@ type ResolveSandboxContextFn = typeof defaultResolveSandboxContext; interface CopilotAttemptDeps { pool: CopilotClientPool; + operation?: CopilotAttemptOperation; now?: () => number; createToolBridge?: typeof createCopilotToolBridge; /** Host fact resolver; injectable only for focused plugin contract tests. */ @@ -365,7 +392,19 @@ export async function runCopilotAttempt( ): Promise { const now = deps.now ?? Date.now; const attemptStartedAt = now(); - const input = params as AttemptParamsLike; + const settledToolFinalization = deps.operation === "settled-tool-finalization"; + const input = ( + settledToolFinalization + ? { + ...params, + // The finalization operation owns its capability boundary. Never trust + // the caller's ordinary attempt flags to keep the Copilot surface empty. + disableTools: true, + images: [], + imageOrder: [], + } + : params + ) as AttemptParamsLike; const createToolBridge = deps.createToolBridge ?? createCopilotToolBridge; const hostSystemAgentActive = deps.isHostScopedToolActive?.("openclaw") ?? isHostScopedAgentToolActive("openclaw"); @@ -446,6 +485,24 @@ export async function runCopilotAttempt( ); } + const settledFinalizationSessionId = settledToolFinalization + ? readString(input.initialReplayState?.sdkSessionId) + : undefined; + if (settledToolFinalization && !settledFinalizationSessionId) { + return finishAttempt( + createResult(input, { + messagesSnapshot: messages, + now, + promptError: createPromptError( + "settled_finalization_session_unavailable", + "[copilot-attempt] settled tool finalization requires the existing Copilot SDK session", + ), + sdkSessionId: undefined, + sessionIdUsed: input.sessionId, + }), + ); + } + let abortRequested = false; let aborted = false; let externalAbort = false; @@ -606,7 +663,19 @@ export async function runCopilotAttempt( resolvedWorkspace: resolvedWorkspaceForSandbox, }) : undefined; - const poolAcquire = resolvePoolAcquire(input); + const resolvedPoolAcquire = resolvePoolAcquire(input); + const poolAcquire = settledToolFinalization + ? { + ...resolvedPoolAcquire, + options: { + ...resolvedPoolAcquire.options, + // The SDK owns the future-proof baseline for ambient capability + // isolation. A separate pool identity prevents this client-level mode + // from changing ordinary Copilot turns that share the same auth/home. + mode: "empty" as const, + }, + } + : resolvedPoolAcquire; let byokProxy: Awaited>; try { byokProxy = await createCopilotByokProxy(poolAcquire.provider); @@ -683,7 +752,7 @@ export async function runCopilotAttempt( }), }); cleanupToolBridge = toolBridge.cleanup; - sdkTools = toolBridge.sdkTools; + sdkTools = settledToolFinalization ? [] : toolBridge.sdkTools; } catch (error: unknown) { const result = createResult(input, { messagesSnapshot: messages, @@ -760,7 +829,8 @@ export async function runCopilotAttempt( ctx: hookContext, }); }; - const hasNativePromptHook = Boolean(attemptInput.hooksConfig?.onUserPromptSubmitted); + const hasNativePromptHook = + !settledToolFinalization && Boolean(attemptInput.hooksConfig?.onUserPromptSubmitted); const userInputBridge = createCopilotUserInputBridge({ paramsForRun: attemptInput, signal: params.abortSignal, @@ -775,7 +845,7 @@ export async function runCopilotAttempt( promptBuild.developerInstructions || undefined, effectiveWorkspaceDir, effectiveCwd, - userInputBridge.onUserInputRequest, + settledToolFinalization ? undefined : userInputBridge.onUserInputRequest, { hooksBridgeOptions: hasNativePromptHook ? { @@ -784,6 +854,7 @@ export async function runCopilotAttempt( } : undefined, includeAskUser: !ringZeroSystemAgentRun, + operation: deps.operation ?? "attempt", }, ); const compactionSessionConfig = byokProxy @@ -796,7 +867,7 @@ export async function runCopilotAttempt( promptBuild.developerInstructions || undefined, effectiveWorkspaceDir, effectiveCwd, - userInputBridge.onUserInputRequest, + settledToolFinalization ? undefined : userInputBridge.onUserInputRequest, { hooksBridgeOptions: hasNativePromptHook ? { @@ -805,6 +876,7 @@ export async function runCopilotAttempt( } : undefined, includeAskUser: !ringZeroSystemAgentRun, + operation: deps.operation ?? "attempt", }, ) : sessionConfig; @@ -813,8 +885,11 @@ export async function runCopilotAttempt( replayInvalid: input.initialReplayState?.replayInvalid, }); downgradedFromResume = replayDecision.downgradedFromResume; - const resumeSessionId = - replayDecision.action === "resume" ? replayDecision.sdkSessionId : undefined; + const resumeSessionId = settledToolFinalization + ? settledFinalizationSessionId + : replayDecision.action === "resume" + ? replayDecision.sdkSessionId + : undefined; // SAFETY: replay-shim owns the create/resume decision and the // recovery policy when resumeSession fails. See replay-shim.ts. @@ -829,6 +904,13 @@ export async function runCopilotAttempt( continuePendingWork: false, })) as unknown as SessionLike; } catch (error: unknown) { + if (settledToolFinalization) { + throw createPromptError( + "settled_finalization_resume_failed", + `[copilot-attempt] settled tool finalization could not resume the existing Copilot SDK session: ${toError(error).message}`, + error, + ); + } const classification = classifyResumeFailure(error); if (!classification.recoverable) { throw error; @@ -852,7 +934,7 @@ export async function runCopilotAttempt( // session's id is valid. sdkSessionId = readSessionId(session) ?? (resumeFailureRecovered ? undefined : resumeSessionId); sessionIdUsed = sdkSessionId ?? input.sessionId; - if (sdkSessionId && deps.onSessionEstablished) { + if (sdkSessionId && deps.onSessionEstablished && !settledToolFinalization) { try { deps.onSessionEstablished({ compactionSessionConfig, @@ -1353,14 +1435,20 @@ function createSessionConfig( systemMessageContent: string | undefined, effectiveWorkspaceDir: string | undefined, effectiveCwd: string | undefined, - onUserInputRequest: NonNullable, + onUserInputRequest: SessionConfig["onUserInputRequest"] | undefined, options: { hooksBridgeOptions?: Parameters[1]; includeAskUser: boolean; + operation: CopilotAttemptOperation; }, ): CopilotSessionConfig { - const permissionPolicy = params.permissionPolicy ?? rejectAllPolicy; - const hooks = createHooksBridge(params.hooksConfig, options.hooksBridgeOptions); + const settledToolFinalization = options.operation === "settled-tool-finalization"; + const permissionPolicy = settledToolFinalization + ? rejectAllPolicy + : (params.permissionPolicy ?? rejectAllPolicy); + const hooks = settledToolFinalization + ? undefined + : createHooksBridge(params.hooksConfig, options.hooksBridgeOptions); return { model: sdkModelId, // Permission decisions for SDK built-in tool kinds (shell, write, @@ -1385,7 +1473,7 @@ function createSessionConfig( onPermissionRequest: createPermissionBridge(permissionPolicy), // Registers the SDK ask_user bridge. The bridge itself owns pending // reply routing so generic mid-run steering still fails closed. - onUserInputRequest, + ...(onUserInputRequest ? { onUserInputRequest } : {}), // The SDK's ResumeSessionConfig declaration omits ProviderConfig, but its // client forwards config.provider on both session.create and session.resume. // Keep one session config so BYOK resume/compaction stays on the same wire. @@ -1401,9 +1489,13 @@ function createSessionConfig( ? { enableSessionTelemetry: params.enableSessionTelemetry } : {}), // The SDK owns defaulting and validation for this native config block. - ...(params.infiniteSessionConfig ? { infiniteSessions: params.infiniteSessionConfig } : {}), + ...(settledToolFinalization + ? { infiniteSessions: { enabled: false } } + : params.infiniteSessionConfig + ? { infiniteSessions: params.infiniteSessionConfig } + : {}), reasoningEffort: params.reasoningEffort, - tools: sdkTools, + tools: settledToolFinalization ? [] : sdkTools, // Restrict the SDK's tool catalog to the bridged tool names returned // by `createCopilotToolBridge`, plus the built-in `ask_user` tool for // normal runs. Ring-zero OpenClaw runs expose only OpenClaw. Without this, the SDK @@ -1421,14 +1513,51 @@ function createSessionConfig( // `@github/copilot-sdk/dist/types.d.ts:1198` (it picks // `availableTools`, so the spread into `resumeSession` covers // the resume path too). - availableTools: buildCopilotAvailableTools(sdkTools, options.includeAskUser), + availableTools: settledToolFinalization + ? [] + : buildCopilotAvailableTools(sdkTools, options.includeAskUser), + ...(settledToolFinalization + ? { + // Copilot's normal client mode has ambient project, agent, skill, + // memory, scheduling, and extension surfaces. Resume the existing + // transcript with every such surface explicitly disabled so this + // operation can only synthesize the final answer from settled state. + coauthorEnabled: false, + customAgents: [], + customAgentsLocalOnly: true, + embeddingCacheStorage: "in-memory", + enableConfigDiscovery: false, + enableFileHooks: false, + enableHostGitOperations: false, + enableOnDemandInstructionDiscovery: false, + enableSessionStore: false, + enableSkills: false, + excludedTools: [...COPILOT_SETTLED_FINALIZATION_EXCLUDED_TOOLS], + includeSubAgentStreamingEvents: false, + instructionDirectories: [], + manageScheduleEnabled: false, + mcpOAuthTokenStorage: "in-memory", + mcpServers: {}, + memory: { enabled: false }, + pluginDirectories: [], + remoteSession: "off", + requestCanvasRenderer: false, + requestExtensions: false, + skipCustomInstructions: true, + skipEmbeddingRetrieval: true, + skillDirectories: [], + } + : {}), workingDirectory: effectiveCwd ?? effectiveWorkspaceDir ?? readResolvedAttemptPath(params.workspaceDir), // When a task runs from a sub-cwd, keep SDK-native project docs // (AGENTS.md, .github/copilot-instructions.md) visible from the // canonical workspace too; workspace-bootstrap filters AGENTS.md // because the SDK owns those instruction files. - ...(effectiveWorkspaceDir && effectiveCwd && effectiveCwd !== effectiveWorkspaceDir + ...(!settledToolFinalization && + effectiveWorkspaceDir && + effectiveCwd && + effectiveCwd !== effectiveWorkspaceDir ? { instructionDirectories: [effectiveWorkspaceDir] } : {}), // Session-level GitHub token. INDEPENDENT of the client-level diff --git a/extensions/copilot/src/runtime.test.ts b/extensions/copilot/src/runtime.test.ts index 461077439db3..d75cebce0786 100644 --- a/extensions/copilot/src/runtime.test.ts +++ b/extensions/copilot/src/runtime.test.ts @@ -124,6 +124,20 @@ describe("createCopilotClientPool", () => { expect(sdk.ctorCalls.length).toBe(1); }); + it("keeps hardened empty-mode clients separate from normal clients", async () => { + const sdk = makeFake(); + const pool = createCopilotClientPool({ sdkFactory: sdk.fake }); + const key = makeKey(); + + const normal = await pool.acquire(key, makeOptions()); + const isolated = await pool.acquire(key, { ...makeOptions(), mode: "empty" }); + + expect(normal.client).not.toBe(isolated.client); + expect(normal.key.clientMode).toBeUndefined(); + expect(isolated.key.clientMode).toBe("empty"); + expect(sdk.ctorCalls.map((options) => options.mode)).toEqual([undefined, "empty"]); + }); + it("different agentId same copilotHome creates distinct clients", async () => { const sdk = makeFake(); const pool = createCopilotClientPool({ sdkFactory: sdk.fake }); diff --git a/extensions/copilot/src/runtime.ts b/extensions/copilot/src/runtime.ts index cef48ed3834d..de5415cfc4c7 100644 --- a/extensions/copilot/src/runtime.ts +++ b/extensions/copilot/src/runtime.ts @@ -17,6 +17,8 @@ export interface PoolKey { readonly authMode: "useLoggedInUser" | "gitHubToken" | "byok"; readonly authProfileId?: string; readonly authProfileVersion?: string; + /** Distinguishes hardened empty-mode clients from normal Copilot CLI clients. */ + readonly clientMode?: CopilotClientOptions["mode"]; } export interface ClientCreateOptions extends Omit< @@ -198,7 +200,7 @@ export function createCopilotClientPool(options: CopilotClientPoolOptions = {}): inputKey: PoolKey, optionsForCreate: ClientCreateOptions, ): Promise => { - const key = normalizePoolKey(inputKey, optionsForCreate.copilotHome); + const key = normalizePoolKey(inputKey, optionsForCreate.copilotHome, optionsForCreate.mode); const cacheKey = JSON.stringify(key); const clientOptions = normalizeClientCreateOptions(optionsForCreate, key.copilotHome); @@ -348,13 +350,20 @@ export function createCopilotClientPool(options: CopilotClientPoolOptions = {}): }; } -function normalizePoolKey(key: PoolKey, rawCopilotHome: string): PoolKey { +function normalizePoolKey( + key: PoolKey, + rawCopilotHome: string, + clientMode: CopilotClientOptions["mode"], +): PoolKey { return { agentId: key.agentId, copilotHome: normalizeCopilotHome(rawCopilotHome), authMode: key.authMode, authProfileId: key.authProfileId, authProfileVersion: key.authProfileVersion, + // Undefined and `copilot-cli` are equivalent SDK defaults. Preserve the + // existing cache identity while keeping empty-mode finalizers isolated. + ...(clientMode === "empty" ? { clientMode } : {}), }; }