From ae26184796db6ee80fb55f23677a49496be4ab3d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Nov 2026 08:53:42 +0000 Subject: [PATCH 1/3] Initial plan From 7f9a1e45288a9b13d7c15d0ef5792fdd744a8cbf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:05:44 +0000 Subject: [PATCH 2/3] Fail loudly when Copilot BYOK drops asset-backed tool images Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/copilot_harness.cjs | 26 ++- actions/setup/js/copilot_image_guard.cjs | 82 ++++++++ actions/setup/js/copilot_image_guard.test.cjs | 180 ++++++++++++++++++ actions/setup/js/copilot_sdk_session.cjs | 24 ++- .../content/docs/reference/engines/copilot.md | 8 + 5 files changed, 312 insertions(+), 8 deletions(-) create mode 100644 actions/setup/js/copilot_image_guard.cjs create mode 100644 actions/setup/js/copilot_image_guard.test.cjs diff --git a/actions/setup/js/copilot_harness.cjs b/actions/setup/js/copilot_harness.cjs index bd9e9e99989..2569bc51aef 100644 --- a/actions/setup/js/copilot_harness.cjs +++ b/actions/setup/js/copilot_harness.cjs @@ -49,6 +49,8 @@ "use strict"; +const { COPILOT_TOOL_IMAGE_EXIT_CODE, isCopilotBYOK, createCopilotImageObserver } = require("./copilot_image_guard.cjs"); + const { iterateJsonl } = require("./jsonl_helpers.cjs"); require("./shim.cjs"); @@ -2085,9 +2087,10 @@ async function main() { // Verified native retries resume the exact UUID; other CLI paths retain --continue. let currentArgs = !copilotSDKMode && attempt > 0 && useContinueOnRetry ? [...resolvedArgs, "--continue"] : resolvedArgs; if (!copilotSDKMode && durableSession?.id && attempt > 0 && !useContinueOnRetry) throw new Error("Durable Copilot continuation cannot restart a recorded session as a fresh conversation"); + const verifyImages = !copilotSDKMode && isCopilotBYOK(); const verifyModel = !copilotSDKMode && (!process.env.GH_AW_PHASE || process.env.GH_AW_PHASE === "agent") && Boolean(process.env.COPILOT_MODEL); let sessionId; - if (!copilotSDKMode && (verifyModel || durableSession)) { + if (!copilotSDKMode && (verifyModel || verifyImages || durableSession)) { const continuedId = attempt === 0 ? durableSession?.id || undefined : useContinueOnRetry ? nativeModelSessionId : undefined; const session = prepareCopilotModelSession(resolvedArgs, continuedId); currentArgs = session.args; @@ -2104,6 +2107,7 @@ async function main() { observeStrategy(strategy); const safeArgs = currentArgs.map((arg, i) => (currentArgs[i - 1] === "--prompt" || currentArgs[i - 1] === "-p" ? "" : arg)); const modelObserver = !verifyModel ? { check: () => "" } : createCopilotModelObserver({ configuredModel: selectionConfiguredModel, resolvedModel: process.env.COPILOT_MODEL || "", sessionId, logger: log }); + const imageObserver = verifyImages ? createCopilotImageObserver({ sessionId }) : { check: () => "" }; // Driver mode: run copilot_sdk_driver.cjs as a normal subprocess. The harness has // already started the sidecar; the driver only opens an SDK client connection. const result = await runProcess({ @@ -2114,7 +2118,12 @@ async function main() { logArgs: safeArgs, env: childEnv, stdin: promptStdin, - runtimeGuard: { shouldTerminate: () => ({ terminate: Boolean(modelObserver.check()), reason: "copilot_model_verification" }) }, + runtimeGuard: { + shouldTerminate: () => { + if (imageObserver.check()) return { terminate: true, reason: "copilot_tool_image_unsupported" }; + return { terminate: Boolean(modelObserver.check()), reason: "copilot_model_verification" }; + }, + }, postResultWatchdog: safeOutputsPath ? { shouldArm: () => hasTerminalSafeOutput(safeOutputsPath), @@ -2124,6 +2133,15 @@ async function main() { }); // The SDK driver observes its main session, records any substitution it refused and // exits with COPILOT_MODEL_VERIFICATION_EXIT_CODE on any verification failure. + const imageDiagnostic = imageObserver.check(true); + if (imageDiagnostic || (copilotSDKMode && result.exitCode === COPILOT_TOOL_IMAGE_EXIT_CODE)) { + result.exitCode = 1; + result.runtimeGuardReason = "copilot_tool_image_unsupported"; + if (imageDiagnostic) { + log(imageDiagnostic); + result.output += `\n${imageDiagnostic}`; + } + } const modelSubstitution = copilotSDKMode ? readCopilotModelSubstitution() : modelObserver.check(true); if (modelSubstitution || (copilotSDKMode && result.exitCode === COPILOT_MODEL_VERIFICATION_EXIT_CODE)) { result.exitCode = 1; @@ -2140,6 +2158,10 @@ async function main() { return { ...result, strategy }; }, handleFailure: async ({ attempt, result }) => { + if (result.runtimeGuardReason === "copilot_tool_image_unsupported") { + result.failureClasses = ["copilot_tool_image_unsupported"]; + return { action: "stop" }; + } if (result.runtimeGuardReason === "copilot_model_verification") { result.failureClasses = ["copilot_model_verification"]; return { action: "stop" }; diff --git a/actions/setup/js/copilot_image_guard.cjs b/actions/setup/js/copilot_image_guard.cjs new file mode 100644 index 00000000000..80a4a239539 --- /dev/null +++ b/actions/setup/js/copilot_image_guard.cjs @@ -0,0 +1,82 @@ +// @ts-check +"use strict"; + +const fs = require("node:fs"); +const path = require("node:path"); +const { iterateJsonl } = require("./jsonl_helpers.cjs"); +const { copilotSessionStateDirectories } = require("./copilot_model_selection.cjs"); +const { copilotSessionContext } = require("./copilot_session.cjs"); + +const COPILOT_TOOL_IMAGE_EXIT_CODE = 80; +const COPILOT_TOOL_IMAGE_DIAGNOSTIC = + "Copilot tool image input is unsupported on this BYOK inference path: an image result has no inline image data and may be silently dropped. Refusing to report success without visual content. See https://github.com/github/copilot-cli/issues/4600."; + +/** @param {NodeJS.ProcessEnv} [env] */ +function isCopilotBYOK(env = process.env) { + return Boolean(env.COPILOT_PROVIDER_BASE_URL || env.COPILOT_API_KEY); +} + +/** @param {any} event */ +function hasUnsupportedCopilotToolImage(event) { + const binaries = event?.data?.result?.binaryResultsForLlm; + return event?.type === "tool.execution_complete" && event.data.success === true && Array.isArray(binaries) && binaries.some(binary => binary?.type === "image" && (typeof binary.data !== "string" || !binary.data)); +} + +/** + * Inspect only newly appended events for this CLI invocation, including subagents. + * @param {{sessionId: string, sessionDirs?: string[]}} options + */ +function createCopilotImageObserver({ sessionId, sessionDirs = copilotSessionStateDirectories() }) { + if (!/^[a-z0-9_-]+$/i.test(sessionId)) throw new Error("Copilot image guard requires an exact invocation session ID"); + const files = sessionDirs.map(directory => path.join(directory, sessionId, "events.jsonl")); + const offsets = new Map(); + const partial = new Map(); + let diagnostic = ""; + const sizeOf = file => { + try { + return fs.statSync(file).size; + } catch (error) { + if (error !== null && typeof error === "object" && "code" in error && error.code === "ENOENT") return 0; + throw error; + } + }; + for (const file of files) offsets.set(file, sizeOf(file)); + + function check(final = false) { + if (diagnostic) return diagnostic; + try { + for (const file of files) { + const size = sizeOf(file); + let offset = offsets.get(file) || 0; + if (size < offset) { + offset = 0; + partial.delete(file); + } + if (size === offset && !partial.has(file)) continue; + const buffer = Buffer.alloc(size - offset); + const fd = fs.openSync(file, "r"); + let bytesRead; + try { + bytesRead = fs.readSync(fd, buffer, 0, buffer.length, offset); + } finally { + fs.closeSync(fd); + } + offsets.set(file, offset + bytesRead); + const text = (partial.get(file) || "") + buffer.subarray(0, bytesRead).toString("utf8"); + const end = final ? text.length : text.lastIndexOf("\n") + 1; + partial.set(file, text.slice(end)); + for (const event of iterateJsonl(text.slice(0, end))) { + const context = copilotSessionContext(event); + if (context.sessionId !== undefined && context.sessionId !== sessionId) continue; + if (hasUnsupportedCopilotToolImage(event)) return (diagnostic = COPILOT_TOOL_IMAGE_DIAGNOSTIC); + } + } + } catch { + diagnostic = "Copilot tool image capability verification failed; refusing to report an unverified run as successful."; + } + return diagnostic; + } + return { check }; +} + +module.exports = { COPILOT_TOOL_IMAGE_EXIT_CODE, COPILOT_TOOL_IMAGE_DIAGNOSTIC, isCopilotBYOK, hasUnsupportedCopilotToolImage, createCopilotImageObserver }; diff --git a/actions/setup/js/copilot_image_guard.test.cjs b/actions/setup/js/copilot_image_guard.test.cjs new file mode 100644 index 00000000000..2ac8506a64a --- /dev/null +++ b/actions/setup/js/copilot_image_guard.test.cjs @@ -0,0 +1,180 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createRequire } from "node:module"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; + +const require = createRequire(import.meta.url); +const { COPILOT_TOOL_IMAGE_DIAGNOSTIC, COPILOT_TOOL_IMAGE_EXIT_CODE, isCopilotBYOK, hasUnsupportedCopilotToolImage, createCopilotImageObserver } = require("./copilot_image_guard.cjs"); +const { runWithCopilotSDK } = require("./copilot_sdk_session.cjs"); +const imageEvent = { + type: "tool.execution_complete", + data: { toolCallId: "view-call", success: true, result: { content: "Viewed image file successfully.", binaryResultsForLlm: [{ type: "image", mimeType: "image/png", byteLength: 21942, assetId: "sha256:test" }] } }, +}; + +describe("Copilot BYOK tool image guard", () => { + let root, file; + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), "copilot-image-")); + file = path.join(root, "session", "events.jsonl"); + fs.mkdirSync(path.dirname(file), { recursive: true }); + vi.stubEnv("GH_AW_COPILOT_CONFIGURED_MODEL", ""); + vi.stubEnv("COPILOT_MODEL", ""); + vi.stubEnv("GH_AW_MODEL_FALLBACK", ""); + vi.stubEnv("GH_AW_COPILOT_SDK_MULTI_PROVIDER_JSON", ""); + vi.stubEnv("COPILOT_PROVIDER_BASE_URL", ""); + vi.stubEnv("COPILOT_API_KEY", ""); + vi.spyOn(process.stderr, "write").mockImplementation(() => true); + }); + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + fs.rmSync(root, { recursive: true, force: true }); + }); + + it("recognizes asset-backed image results without inspecting image bytes or tool names", () => { + expect(hasUnsupportedCopilotToolImage(imageEvent)).toBe(true); + expect(hasUnsupportedCopilotToolImage({ ...imageEvent, data: { ...imageEvent.data, parentToolCallId: "subagent" } })).toBe(true); + }); + + it("recognizes both managed AWF and explicit BYOK without treating native auth as BYOK", () => { + expect(isCopilotBYOK({ COPILOT_API_KEY: "test-sentinel" })).toBe(true); + expect(isCopilotBYOK({ COPILOT_PROVIDER_BASE_URL: "http://api-proxy:10002" })).toBe(true); + expect(isCopilotBYOK({ COPILOT_GITHUB_TOKEN: "test-native-auth" })).toBe(false); + }); + + it.each([ + null, + { type: "assistant.message", data: { content: JSON.stringify(imageEvent) } }, + { ...imageEvent, data: { ...imageEvent.data, success: false } }, + { ...imageEvent, data: { success: true, result: { content: "Viewed image file successfully." } } }, + { ...imageEvent, data: { success: true, result: { binaryResultsForLlm: [{ type: "image", data: "aW1hZ2U=" }] } } }, + { ...imageEvent, data: { success: true, result: { binaryResultsForLlm: [{ type: "audio" }] } } }, + ])("does not mistake text, failed tools, inline images, or non-images for missing image data (%j)", event => { + expect(hasUnsupportedCopilotToolImage(event)).toBe(false); + }); + + it("tails only new records in the exact invocation, including nested tools and split records", () => { + fs.writeFileSync(file, JSON.stringify(imageEvent) + "\n"); + const observer = createCopilotImageObserver({ sessionId: "session", sessionDirs: [root] }); + fs.appendFileSync(file, JSON.stringify({ ...imageEvent, sessionId: "unrelated" }) + "\n"); + expect(observer.check()).toBe(""); + const record = JSON.stringify({ ...imageEvent, parentToolCallId: "subagent" }); + fs.appendFileSync(file, record.slice(0, 40)); + expect(observer.check()).toBe(""); + fs.appendFileSync(file, record.slice(40)); + expect(observer.check()).toBe(""); + expect(observer.check(true)).toBe(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + expect(observer.check()).toBe(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + }); + + it("watches an invocation whose session file does not exist until the process starts", () => { + const observer = createCopilotImageObserver({ sessionId: "session", sessionDirs: [root] }); + expect(observer.check()).toBe(""); + fs.writeFileSync(file, JSON.stringify(imageEvent) + "\n"); + expect(observer.check()).toBe(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + }); + + it("fails closed if session evidence becomes unreadable", () => { + const observer = createCopilotImageObserver({ sessionId: "session", sessionDirs: [root] }); + fs.mkdirSync(file); + expect(observer.check()).toContain("verification failed"); + }); + + it("rejects session path traversal", () => { + expect(() => createCopilotImageObserver({ sessionId: "../other", sessionDirs: [root] })).toThrow("exact invocation session ID"); + }); + + it.each([false, true])("fails a CLI image result without retrying or accepting prior output (long-lived=%s)", longLived => { + const stub = path.join(root, "cli.cjs"); + fs.writeFileSync( + stub, + ` + const fs = require("node:fs"), path = require("node:path"); + const id = process.argv[process.argv.indexOf("--session-id") + 1]; + const file = path.join(process.env.COPILOT_HOME, "session-state", id, "events.jsonl"); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.appendFileSync(file, JSON.stringify(${JSON.stringify(imageEvent)}) + "\\n"); + console.log("CLI says success"); + ${longLived ? "setInterval(() => {}, 1000);" : ""} + ` + ); + const run = spawnSync(process.execPath, [require.resolve("./copilot_harness.cjs"), process.execPath, stub], { + env: { + ...process.env, + HOME: root, + COPILOT_HOME: path.join(root, ".copilot"), + GH_AW_TMP_DIR: root, + GH_AW_AWF_CONFIG_PATH: path.join(root, "awf-config.json"), + COPILOT_API_KEY: "test-sentinel", + COPILOT_SDK_URI: "", + COPILOT_PROVIDER_WIRE_API: "", + GH_AW_PHASE: "agent", + GH_AW_MODEL_ROUTING: "0", + GH_AW_SAFE_OUTPUTS: "", + AWF_REFLECT_ENABLED: "0", + GH_AW_HARNESS_INITIAL_DELAY_MS: "1", + GH_AW_HARNESS_MAX_RETRIES: "1", + }, + encoding: "utf8", + timeout: 10000, + }); + expect(run.error, run.stderr).toBeUndefined(); + expect(run.status, run.stderr).toBe(1); + expect(run.stderr).toContain(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + expect(run.stderr).not.toContain("attempt 2: spawning"); + }); + + it.each(["normal", "partial-output", "idle-timeout", "stalled", "subagent", "native", "inline", "byok-env", "abort-fails", "verification-enabled"])("handles SDK image completion (%s)", async mode => { + const abort = vi.fn().mockResolvedValue(undefined); + if (mode === "abort-fails") abort.mockRejectedValue(new Error("abort failed")); + if (mode === "byok-env") vi.stubEnv("COPILOT_API_KEY", "test-sentinel"); + if (mode === "verification-enabled") vi.stubEnv("GH_AW_COPILOT_CONFIGURED_MODEL", "test"); + const disconnect = vi.fn().mockResolvedValue(undefined); + const logger = vi.fn(); + let handler; + class FakeCopilotClient { + start = async () => {}; + stop = async () => {}; + createSession = async () => ({ + sessionId: "sdk", + on: callback => { + handler = callback; + }, + sendAndWait: async () => { + if (mode === "partial-output") handler({ type: "assistant.message", data: { content: "Starting." } }); + handler( + mode === "inline" ? { ...imageEvent, data: { ...imageEvent.data, result: { binaryResultsForLlm: [{ type: "image", data: "aW1hZ2U=" }] } } } : { ...imageEvent, ...(mode === "subagent" ? { parentToolCallId: "task" } : {}) } + ); + if (mode === "stalled") return new Promise(() => {}); + if (mode === "idle-timeout") throw new Error("Timeout after 100ms waiting for session.idle"); + return { type: "assistant.message", data: { content: "done" } }; + }, + abort, + disconnect, + }); + } + const result = await runWithCopilotSDK({ + sdkUri: "http://127.0.0.1:3002", + prompt: "Describe the PNG.", + logger, + workingDirectory: root, + agentsBaseDir: root, + sessionStateBaseDir: root, + ...(mode === "native" || mode === "byok-env" ? {} : { model: "test", providers: [{ name: "test", type: "openai", baseUrl: "http://api-proxy:10002" }], models: [{ id: "test", provider: "test" }] }), + sdkModule: { CopilotClient: FakeCopilotClient, RuntimeConnection: { forUri: () => ({}) }, approveAll: () => "allow" }, + }); + const guarded = mode !== "native" && mode !== "inline"; + expect(result.exitCode).toBe(guarded ? COPILOT_TOOL_IMAGE_EXIT_CODE : 0); + expect(abort).toHaveBeenCalledTimes(guarded ? 1 : 0); + expect(disconnect).toHaveBeenCalledTimes(1); + if (guarded) { + expect(result.output).toContain(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + expect(logger.mock.calls.flat().some(message => message.includes("treating as completed"))).toBe(false); + const events = fs.readFileSync(path.join(root, "sdk/events.jsonl"), "utf8"); + expect(events).toContain('"code":"copilot_tool_image_unsupported"'); + expect(events).toContain('"errorType":"unsupported_capability"'); + } + }); +}); diff --git a/actions/setup/js/copilot_sdk_session.cjs b/actions/setup/js/copilot_sdk_session.cjs index 5d4d27cface..2831a278f88 100644 --- a/actions/setup/js/copilot_sdk_session.cjs +++ b/actions/setup/js/copilot_sdk_session.cjs @@ -39,6 +39,7 @@ const { copilotSessionContext, getCopilotMessageRefusal } = require("./copilot_s const { loadCopilotSDKCustomAgents } = require("./extract_inline_sub_agents.cjs"); const { qualifyModelForMultiProvider, parseMultiProviderJson } = require("./copilot_sdk_multi_provider.cjs"); const { COPILOT_MODEL_VERIFICATION_EXIT_CODE, createCopilotSDKModelObserver, copilotSessionStateDirectories } = require("./copilot_model_selection.cjs"); +const { COPILOT_TOOL_IMAGE_EXIT_CODE, COPILOT_TOOL_IMAGE_DIAGNOSTIC, isCopilotBYOK, hasUnsupportedCopilotToolImage } = require("./copilot_image_guard.cjs"); const CAPTURED_EPHEMERAL_EVENTS = new Set([...COPILOT_WORKFLOW_EVENT_TYPES, "assistant.usage", "assistant.message_delta", "assistant.reasoning_delta", "model.call_failure", "model.call_final_result", "model.call_finished"]); @@ -178,7 +179,7 @@ async function runWithCopilotSDK(options) { const result = await runCopilotSDKSession({ ...options, verification }); // Native shutdown records arrive while disconnecting, after the session result is known. const verificationFailure = verification.modelObserver && verification.sessionId ? verification.modelObserver.finalize(verification.sessionId) : ""; - if (!verificationFailure) return result; + if (!verificationFailure || result.exitCode === COPILOT_TOOL_IMAGE_EXIT_CODE) return result; // Any verification failure (substitution or unverifiable evidence) is reported with a dedicated exit code so the harness does not retry. return { ...result, exitCode: COPILOT_MODEL_VERIFICATION_EXIT_CODE, output: result.output.includes(verificationFailure) ? result.output : `${result.output}\n${verificationFailure}` }; } @@ -271,6 +272,8 @@ async function runCopilotSDKSession({ let catastrophicToolDenialsTriggered = false; /** @type {Error | null} */ let modelSubstitutionError = null; + /** @type {Error | null} */ + let unsupportedImageError = null; /** * Rejects `denialGuardPromise` once the tool-denial threshold is exceeded and the * bounded force-exit interval elapses. Declared here so `recordToolDenial` (defined @@ -493,6 +496,14 @@ async function runCopilotSDKSession({ } session.on(event => { + if (!unsupportedImageError && (providerConfig || isCopilotBYOK()) && hasUnsupportedCopilotToolImage(event)) { + unsupportedImageError = new Error(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + writeDriverEvent("session.error", { errorType: "unsupported_capability", code: "copilot_tool_image_unsupported", message: unsupportedImageError.message }); + if (denialGuardReject) denialGuardReject(unsupportedImageError); + void session.abort().catch(() => { + log("warning: unable to abort SDK inference after an unsupported tool image; stopping the driver"); + }); + } // Observe the main-session model before capture so records delivered while disconnecting count too. const modelDiagnostic = modelObserver.observe(event); if (modelDiagnostic && !modelSubstitutionError) { @@ -603,6 +614,7 @@ async function runCopilotSDKSession({ // timeout intervenes. denialGuardPromise never settles unless the guard fires. const result = await Promise.race([session.sendAndWait({ prompt }, sendTimeoutMs), denialGuardPromise]); + if (unsupportedImageError) throw unsupportedImageError; if (catastrophicToolDenialsError) { throw catastrophicToolDenialsError; } @@ -635,13 +647,13 @@ async function runCopilotSDKSession({ return { exitCode: 0, output, hasOutput, durationMs }; } catch (err) { const durationMs = Date.now() - startTime; - const failure = catastrophicToolDenialsError ?? modelSubstitutionError ?? (contentFiltered ? contentFilterError() : err instanceof Error ? err : new Error(String(err))); + const failure = unsupportedImageError ?? catastrophicToolDenialsError ?? modelSubstitutionError ?? (contentFiltered ? contentFilterError() : err instanceof Error ? err : new Error(String(err))); log(`error: ${failure.message}`); // When the post-completion idle watchdog force-disconnected the session, the // agent's work is done — the SDK simply failed to resolve sendAndWait after // the final tool result was returned. Treat it as a successful completion. - if (postCompletionWatchdogTriggered && !catastrophicToolDenialsError && !modelSubstitutionError && !contentFiltered && hasOutput && pendingToolCalls.size === 0) { + if (postCompletionWatchdogTriggered && !unsupportedImageError && !catastrophicToolDenialsError && !modelSubstitutionError && !contentFiltered && hasOutput && pendingToolCalls.size === 0) { log(`warning: post-completion watchdog triggered disconnect — treating as completed`); log(`session completed: hasOutput=${hasOutput} assistantTurns=${assistantTurnCount} durationMs=${durationMs}`); return { exitCode: 0, output, hasOutput, durationMs }; @@ -651,7 +663,7 @@ async function runCopilotSDKSession({ // output and all tracked tool calls have already completed, the session work is // done — the SDK simply failed to emit the idle signal. Treat it as a successful // run so the harness does not classify it as a failure or waste retry attempts. - const isIdleTimeout = !catastrophicToolDenialsError && !modelSubstitutionError && !contentFiltered && SDK_IDLE_TIMEOUT_PATTERN.test(failure.message); + const isIdleTimeout = !unsupportedImageError && !catastrophicToolDenialsError && !modelSubstitutionError && !contentFiltered && SDK_IDLE_TIMEOUT_PATTERN.test(failure.message); if (isIdleTimeout && hasOutput && pendingToolCalls.size === 0) { log(`warning: SDK idle-timeout with collected output and no pending tool calls — treating as completed`); log(`session completed: hasOutput=${hasOutput} assistantTurns=${assistantTurnCount} durationMs=${durationMs}`); @@ -663,8 +675,8 @@ async function runCopilotSDKSession({ // Preserve any output collected before the error so the harness can use it // for retry decisions and diagnostics. return { - exitCode: 1, - output: hasOutput ? output : failure.message, + exitCode: unsupportedImageError ? COPILOT_TOOL_IMAGE_EXIT_CODE : 1, + output: unsupportedImageError && hasOutput ? `${output}\n${failure.message}` : hasOutput ? output : failure.message, hasOutput, durationMs, }; diff --git a/docs/src/content/docs/reference/engines/copilot.md b/docs/src/content/docs/reference/engines/copilot.md index 2322be7eb27..e25f9a52cd4 100644 --- a/docs/src/content/docs/reference/engines/copilot.md +++ b/docs/src/content/docs/reference/engines/copilot.md @@ -19,6 +19,14 @@ Sandboxed CLI sessions derive URL approvals from finalized AWF network rules, in Copilot resolves [model aliases](/gh-aw/reference/engines/#model-aliases) before launch. Exact IDs pass through unchanged; unavailable IDs fail the first inference request. +### Tool-returned images + +Copilot CLI's BYOK request assembly can silently omit asset-backed images returned by `view` or MCP tools, even when the tool reports success and records `binaryResultsForLlm`. Sandboxed gh-aw Copilot inference uses BYOK routing through the AWF API proxy, so a vision-capable model alone does not prevent this limitation. No gh-aw capability setting repairs the missing payload. + +The built-in CLI harness and SDK driver fail with `copilot_tool_image_unsupported` when a successful tool result contains an image without inline data. CLI mode checks newly appended session events during execution and once more at exit; SDK mode aborts on the tool-completion event. This guard does not prove that other image payloads reached the provider. Custom harnesses and drivers must implement their own checks. + +Track the request-assembly fix in [Copilot CLI issue #4600](https://github.com/github/copilot-cli/issues/4600). A successful image-view message or an image asset in session logs is not evidence that the model saw the pixels. Do not rely on `@path` in a prompt file as a verified workaround. + ### Dynamic workflows Copilot [dynamic workflows](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/use-dynamic-workflows) are experimental and disabled by default. Current CLI releases require both settings: From 1fb796f1f52e24ad2d7e507a1c34b664078b6d99 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:51:26 +0000 Subject: [PATCH 3/3] Share Copilot event observation and fail on cleanup-time images Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/copilot_image_guard.cjs | 60 +-------- actions/setup/js/copilot_image_guard.test.cjs | 119 ++++++++++++++++- .../setup/js/copilot_invocation_observer.cjs | 70 ++++++++++ actions/setup/js/copilot_model_selection.cjs | 120 ++++++------------ actions/setup/js/copilot_sdk_session.cjs | 27 ++-- .../content/docs/reference/engines/copilot.md | 4 +- 6 files changed, 250 insertions(+), 150 deletions(-) create mode 100644 actions/setup/js/copilot_invocation_observer.cjs diff --git a/actions/setup/js/copilot_image_guard.cjs b/actions/setup/js/copilot_image_guard.cjs index 80a4a239539..2b39f8af6b5 100644 --- a/actions/setup/js/copilot_image_guard.cjs +++ b/actions/setup/js/copilot_image_guard.cjs @@ -1,11 +1,8 @@ // @ts-check "use strict"; -const fs = require("node:fs"); -const path = require("node:path"); -const { iterateJsonl } = require("./jsonl_helpers.cjs"); +const { createCopilotInvocationObserver } = require("./copilot_invocation_observer.cjs"); const { copilotSessionStateDirectories } = require("./copilot_model_selection.cjs"); -const { copilotSessionContext } = require("./copilot_session.cjs"); const COPILOT_TOOL_IMAGE_EXIT_CODE = 80; const COPILOT_TOOL_IMAGE_DIAGNOSTIC = @@ -28,55 +25,12 @@ function hasUnsupportedCopilotToolImage(event) { */ function createCopilotImageObserver({ sessionId, sessionDirs = copilotSessionStateDirectories() }) { if (!/^[a-z0-9_-]+$/i.test(sessionId)) throw new Error("Copilot image guard requires an exact invocation session ID"); - const files = sessionDirs.map(directory => path.join(directory, sessionId, "events.jsonl")); - const offsets = new Map(); - const partial = new Map(); - let diagnostic = ""; - const sizeOf = file => { - try { - return fs.statSync(file).size; - } catch (error) { - if (error !== null && typeof error === "object" && "code" in error && error.code === "ENOENT") return 0; - throw error; - } - }; - for (const file of files) offsets.set(file, sizeOf(file)); - - function check(final = false) { - if (diagnostic) return diagnostic; - try { - for (const file of files) { - const size = sizeOf(file); - let offset = offsets.get(file) || 0; - if (size < offset) { - offset = 0; - partial.delete(file); - } - if (size === offset && !partial.has(file)) continue; - const buffer = Buffer.alloc(size - offset); - const fd = fs.openSync(file, "r"); - let bytesRead; - try { - bytesRead = fs.readSync(fd, buffer, 0, buffer.length, offset); - } finally { - fs.closeSync(fd); - } - offsets.set(file, offset + bytesRead); - const text = (partial.get(file) || "") + buffer.subarray(0, bytesRead).toString("utf8"); - const end = final ? text.length : text.lastIndexOf("\n") + 1; - partial.set(file, text.slice(end)); - for (const event of iterateJsonl(text.slice(0, end))) { - const context = copilotSessionContext(event); - if (context.sessionId !== undefined && context.sessionId !== sessionId) continue; - if (hasUnsupportedCopilotToolImage(event)) return (diagnostic = COPILOT_TOOL_IMAGE_DIAGNOSTIC); - } - } - } catch { - diagnostic = "Copilot tool image capability verification failed; refusing to report an unverified run as successful."; - } - return diagnostic; - } - return { check }; + return createCopilotInvocationObserver({ + sessionId, + sessionDirs, + onEvent: event => (hasUnsupportedCopilotToolImage(event) ? COPILOT_TOOL_IMAGE_DIAGNOSTIC : ""), + onError: () => "Copilot tool image capability verification failed; refusing to report an unverified run as successful.", + }); } module.exports = { COPILOT_TOOL_IMAGE_EXIT_CODE, COPILOT_TOOL_IMAGE_DIAGNOSTIC, isCopilotBYOK, hasUnsupportedCopilotToolImage, createCopilotImageObserver }; diff --git a/actions/setup/js/copilot_image_guard.test.cjs b/actions/setup/js/copilot_image_guard.test.cjs index 2ac8506a64a..e6a69f21dae 100644 --- a/actions/setup/js/copilot_image_guard.test.cjs +++ b/actions/setup/js/copilot_image_guard.test.cjs @@ -76,6 +76,17 @@ describe("Copilot BYOK tool image guard", () => { expect(observer.check()).toBe(COPILOT_TOOL_IMAGE_DIAGNOSTIC); }); + it("discards stale partial records when the invocation log is truncated", () => { + fs.writeFileSync(file, "cached session history\n"); + const observer = createCopilotImageObserver({ sessionId: "session", sessionDirs: [root] }); + fs.appendFileSync(file, '{"type":"unfinished'); + expect(observer.check()).toBe(""); + fs.writeFileSync(file, ""); + expect(observer.check()).toBe(""); + fs.appendFileSync(file, JSON.stringify(imageEvent) + "\n"); + expect(observer.check()).toBe(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + }); + it("fails closed if session evidence becomes unreadable", () => { const observer = createCopilotImageObserver({ sessionId: "session", sessionDirs: [root] }); fs.mkdirSync(file); @@ -86,6 +97,103 @@ describe("Copilot BYOK tool image guard", () => { expect(() => createCopilotImageObserver({ sessionId: "../other", sessionDirs: [root] })).toThrow("exact invocation session ID"); }); + function runCLI(stub, args = [], byok = true) { + return spawnSync(process.execPath, [require.resolve("./copilot_harness.cjs"), process.execPath, stub, ...args], { + env: { + ...process.env, + HOME: root, + COPILOT_HOME: path.join(root, ".copilot"), + GH_AW_TMP_DIR: root, + GH_AW_AWF_CONFIG_PATH: path.join(root, "awf-config.json"), + COPILOT_API_KEY: byok ? "test-sentinel" : "", + COPILOT_SDK_URI: "", + COPILOT_PROVIDER_WIRE_API: "", + GH_AW_COPILOT_SDK: "0", + GH_AW_PHASE: "agent", + GH_AW_MODEL_ROUTING: "0", + GH_AW_SAFE_OUTPUTS: "", + AWF_REFLECT_ENABLED: "0", + GH_AW_HARNESS_INITIAL_DELAY_MS: "1", + GH_AW_HARNESS_MAX_RETRIES: "1", + }, + encoding: "utf8", + timeout: 10000, + }); + } + + it.each(["--resume", "--session-id"])("resumes the exact BYOK invocation on retry and ignores its old image records (%s)", selector => { + const stub = path.join(root, "retry-cli.cjs"); + const attemptsPath = path.join(root, "attempts.json"); + const sessionId = "12345678-1234-1234-1234-123456789abc"; + const sessionFile = path.join(root, ".copilot/session-state", sessionId, "events.jsonl"); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); + fs.writeFileSync(sessionFile, JSON.stringify(imageEvent) + "\n"); + fs.writeFileSync( + stub, + ` + const fs = require("node:fs"); + const file = ${JSON.stringify(attemptsPath)}; + const attempts = fs.existsSync(file) ? JSON.parse(fs.readFileSync(file, "utf8")) : []; + attempts.push(process.argv.slice(2)); + fs.writeFileSync(file, JSON.stringify(attempts)); + if (attempts.length === 1) { + console.log("Partial text output"); + console.error("Execution failed: CAPIError: 400 Bad Request"); + process.exit(1); + } + console.log("Completed"); + ` + ); + const run = runCLI(stub, [selector, sessionId]); + expect(run.error, run.stderr).toBeUndefined(); + expect(run.status, run.stderr).toBe(0); + expect(JSON.parse(fs.readFileSync(attemptsPath, "utf8"))).toEqual([ + [selector, sessionId], + [selector, sessionId], + ]); + }); + + it("assigns an exact BYOK session and reuses only that UUID on retry", () => { + const stub = path.join(root, "retry-cli.cjs"); + const attemptsPath = path.join(root, "attempts.json"); + fs.writeFileSync( + stub, + ` + const fs = require("node:fs"); + const file = ${JSON.stringify(attemptsPath)}; + const attempts = fs.existsSync(file) ? JSON.parse(fs.readFileSync(file, "utf8")) : []; + attempts.push(process.argv.slice(2)); + fs.writeFileSync(file, JSON.stringify(attempts)); + if (attempts.length === 1) { + console.log("Partial text output"); + console.error("Execution failed: CAPIError: 400 Bad Request"); + process.exit(1); + } + console.log("Completed"); + ` + ); + const run = runCLI(stub); + expect(run.error, run.stderr).toBeUndefined(); + expect(run.status, run.stderr).toBe(0); + const attempts = JSON.parse(fs.readFileSync(attemptsPath, "utf8")); + expect(attempts).toHaveLength(2); + expect(attempts[0]).toEqual(["--session-id", expect.stringMatching(/^[0-9a-f-]{36}$/)]); + expect(attempts[1]).toEqual(["--session-id", attempts[0][1]]); + }); + + it.each([["--continue"], ["--connect", "localhost:3000"], ["--resume", "friendly-name"], ["--resume", "12345678"]])("rejects ambiguous BYOK session selectors before spawning (%j)", (...args) => { + const stub = path.join(root, "cli.cjs"); + const marker = path.join(root, "spawned"); + fs.writeFileSync(stub, `require("node:fs").writeFileSync(${JSON.stringify(marker)}, "spawned"); console.log("done");`); + const guarded = runCLI(stub, args); + expect(guarded.status, guarded.stderr).toBe(1); + expect(guarded.stderr).toContain("exact"); + expect(fs.existsSync(marker)).toBe(false); + const native = runCLI(stub, args, false); + expect(native.status, native.stderr).toBe(0); + expect(fs.existsSync(marker)).toBe(true); + }); + it.each([false, true])("fails a CLI image result without retrying or accepting prior output (long-lived=%s)", longLived => { const stub = path.join(root, "cli.cjs"); fs.writeFileSync( @@ -126,23 +234,28 @@ describe("Copilot BYOK tool image guard", () => { expect(run.stderr).not.toContain("attempt 2: spawning"); }); - it.each(["normal", "partial-output", "idle-timeout", "stalled", "subagent", "native", "inline", "byok-env", "abort-fails", "verification-enabled"])("handles SDK image completion (%s)", async mode => { + it.each(["normal", "partial-output", "idle-timeout", "stalled", "subagent", "native", "inline", "byok-env", "abort-fails", "verification-enabled", "disconnect", "stop"])("handles SDK image completion (%s)", async mode => { const abort = vi.fn().mockResolvedValue(undefined); if (mode === "abort-fails") abort.mockRejectedValue(new Error("abort failed")); if (mode === "byok-env") vi.stubEnv("COPILOT_API_KEY", "test-sentinel"); if (mode === "verification-enabled") vi.stubEnv("GH_AW_COPILOT_CONFIGURED_MODEL", "test"); - const disconnect = vi.fn().mockResolvedValue(undefined); + const disconnect = vi.fn(async () => { + if (mode === "disconnect") handler(imageEvent); + }); const logger = vi.fn(); let handler; class FakeCopilotClient { start = async () => {}; - stop = async () => {}; + stop = async () => { + if (mode === "stop") handler(imageEvent); + }; createSession = async () => ({ sessionId: "sdk", on: callback => { handler = callback; }, sendAndWait: async () => { + if (mode === "disconnect" || mode === "stop") return { type: "assistant.message", data: { content: "done" } }; if (mode === "partial-output") handler({ type: "assistant.message", data: { content: "Starting." } }); handler( mode === "inline" ? { ...imageEvent, data: { ...imageEvent.data, result: { binaryResultsForLlm: [{ type: "image", data: "aW1hZ2U=" }] } } } : { ...imageEvent, ...(mode === "subagent" ? { parentToolCallId: "task" } : {}) } diff --git a/actions/setup/js/copilot_invocation_observer.cjs b/actions/setup/js/copilot_invocation_observer.cjs new file mode 100644 index 00000000000..3f796249a78 --- /dev/null +++ b/actions/setup/js/copilot_invocation_observer.cjs @@ -0,0 +1,70 @@ +// @ts-check +"use strict"; + +const fs = require("node:fs"); +const path = require("node:path"); +const { iterateJsonl } = require("./jsonl_helpers.cjs"); +const { copilotSessionContext } = require("./copilot_session.cjs"); + +/** + * Tail newly appended events for one invocation, including its subagents. + * @param {{sessionId: string, sessionDirs: string[], onEvent: (event: any) => string, onError: (error: unknown) => string}} options + */ +function createCopilotInvocationObserver({ sessionId, sessionDirs, onEvent, onError }) { + if (!/^[a-z0-9_-]+$/i.test(sessionId)) throw new Error("Copilot event observation requires an exact invocation session ID"); + const files = sessionDirs.map(directory => path.join(directory, sessionId, "events.jsonl")); + /** @type {Map} */ + const offsets = new Map(); + /** @type {Map} */ + const partial = new Map(); + let diagnostic = ""; + const sizeOf = file => { + try { + return fs.statSync(file).size; + } catch (error) { + if (error !== null && typeof error === "object" && "code" in error && error.code === "ENOENT") return 0; + throw error; + } + }; + for (const file of files) offsets.set(file, sizeOf(file)); + + function check(final = false) { + if (diagnostic) return diagnostic; + try { + for (const file of files) { + const size = sizeOf(file); + let offset = offsets.get(file) || 0; + if (size < offset) { + offset = 0; + offsets.set(file, offset); + partial.delete(file); + } + if (size === offset && !partial.has(file)) continue; + const buffer = Buffer.alloc(size - offset); + const fd = fs.openSync(file, "r"); + let bytesRead; + try { + bytesRead = fs.readSync(fd, buffer, 0, buffer.length, offset); + } finally { + fs.closeSync(fd); + } + offsets.set(file, offset + bytesRead); + const text = (partial.get(file) || "") + buffer.subarray(0, bytesRead).toString("utf8"); + const end = final ? text.length : text.lastIndexOf("\n") + 1; + partial.set(file, text.slice(end)); + for (const event of iterateJsonl(text.slice(0, end))) { + const context = copilotSessionContext(event); + if (context.sessionId !== undefined && context.sessionId !== sessionId) continue; + diagnostic = onEvent(event); + if (diagnostic) return diagnostic; + } + } + } catch (error) { + diagnostic = onError(error); + } + return diagnostic; + } + return { check }; +} + +module.exports = { createCopilotInvocationObserver }; diff --git a/actions/setup/js/copilot_model_selection.cjs b/actions/setup/js/copilot_model_selection.cjs index c015505bdae..c2d90ea2167 100644 --- a/actions/setup/js/copilot_model_selection.cjs +++ b/actions/setup/js/copilot_model_selection.cjs @@ -9,6 +9,7 @@ const { validateModelIdentifier } = require("./model_attribution.cjs"); const { normalizeForCopilotCLI, splitModelIdentifier } = require("./resolve_model_alias.cjs"); const { copilotSessionContext } = require("./copilot_session.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { createCopilotInvocationObserver } = require("./copilot_invocation_observer.cjs"); // Bounds the per-run model history persisted in the selection log (16 KiB) and aw_info.json. const MAX_OBSERVED_MODELS = 32; @@ -124,98 +125,51 @@ function persistCopilotModelSelection(env = process.env) { */ function createCopilotModelObserver({ configuredModel, resolvedModel, sessionId, env = process.env, sessionDirs, logger = () => {} }) { const directories = sessionDirs || copilotSessionStateDirectories(env); - /** @type {Map} */ - const offsets = new Map(); - /** @type {Map} */ - const partial = new Map(); - let diagnostic = ""; let observedModel = ""; const expected = modelID(resolvedModel); if (!expected || (env.GH_AW_PHASE && env.GH_AW_PHASE !== "agent")) return { check: () => "" }; const observedModels = new Set(previouslyObservedModels(env)); if (!sessionId || !/^[a-z0-9_-]+$/i.test(sessionId)) throw new Error("Copilot model verification requires an exact invocation session ID"); - const ownedSessionId = sessionId; - - function eventFiles() { - const files = new Set(); - for (const directory of directories) { - const file = path.join(directory, ownedSessionId, "events.jsonl"); - try { - fs.statSync(file); - } catch (error) { - if (error !== null && typeof error === "object" && "code" in error && error.code === "ENOENT") continue; - throw error; - } - files.add(file); - } - return files; - } - try { - for (const file of eventFiles()) offsets.set(file, fs.statSync(file).size); + return createCopilotInvocationObserver({ + sessionId, + sessionDirs: directories, + onEvent: event => { + const context = copilotSessionContext(event); + if (context.agentId !== undefined || context.parentToolUseId) return ""; + const model = modelID(event.type === "session.model_change" && event.data?.source === "startup" ? event.data.newModel : event.type === "assistant.message" ? event.data?.model : ""); + if (!model || model === observedModel) return ""; + observedModel = model; + if (!observedModels.has(model)) { + const oldest = observedModels.values().next().value; + if (observedModels.size >= MAX_OBSERVED_MODELS && oldest !== undefined) observedModels.delete(oldest); + observedModels.add(model); + } + const substituted = expected.toLowerCase() !== "auto" && model.toLowerCase() !== expected.toLowerCase(); + recordCopilotModelSelection( + { + status: substituted ? "substituted" : "observed", + configured_model: configuredModel, + resolved_model: resolvedModel, + effective_model: model, + ...(observedModels.size > 1 ? { observed_models: [...observedModels] } : {}), + }, + env + ); + if (!substituted) return ""; + const diagnostic = modelSubstitutionDiagnostic(expected, model); + logger(diagnostic); + return diagnostic; + }, + onError: error => { + const diagnostic = verificationFailureDiagnostic(error); + logger(diagnostic); + return diagnostic; + }, + }); } catch (error) { throw new Error(`Cannot initialize Copilot model verification: ${getErrorMessage(error)}`, { cause: error }); } - - function check(final = false) { - try { - if (diagnostic) return diagnostic; - for (const file of eventFiles()) { - const size = fs.statSync(file).size; - let offset = offsets.get(file) || 0; - if (size < offset) { - offset = 0; - partial.delete(file); - } - const content = Buffer.alloc(size - offset); - const fd = fs.openSync(file, "r"); - let bytesRead; - try { - bytesRead = fs.readSync(fd, content, 0, content.length, offset); - } finally { - fs.closeSync(fd); - } - const text = (partial.get(file) || "") + content.subarray(0, bytesRead).toString("utf8"); - offsets.set(file, offset + bytesRead); - const end = final ? text.length : text.lastIndexOf("\n") + 1; - partial.set(file, text.slice(end)); - for (const event of iterateJsonl(text.slice(0, end))) { - const context = copilotSessionContext(event); - if (context.agentId !== undefined || context.parentToolUseId || (context.sessionId !== undefined && context.sessionId !== ownedSessionId)) continue; - const model = modelID(event.type === "session.model_change" && event.data?.source === "startup" ? event.data.newModel : event.type === "assistant.message" ? event.data?.model : ""); - if (!model || model === observedModel) continue; - observedModel = model; - if (!observedModels.has(model)) { - const oldest = observedModels.values().next().value; - if (observedModels.size >= MAX_OBSERVED_MODELS && oldest !== undefined) observedModels.delete(oldest); - observedModels.add(model); - } - const substituted = expected.toLowerCase() !== "auto" && model.toLowerCase() !== expected.toLowerCase(); - recordCopilotModelSelection( - { - status: substituted ? "substituted" : "observed", - configured_model: configuredModel, - resolved_model: resolvedModel, - effective_model: model, - ...(observedModels.size > 1 ? { observed_models: [...observedModels] } : {}), - }, - env - ); - if (substituted) { - diagnostic = modelSubstitutionDiagnostic(expected, model); - logger(diagnostic); - return diagnostic; - } - } - } - return ""; - } catch (error) { - diagnostic = verificationFailureDiagnostic(error); - logger(diagnostic); - return diagnostic; - } - } - return { check }; } /** @param {any} event */ diff --git a/actions/setup/js/copilot_sdk_session.cjs b/actions/setup/js/copilot_sdk_session.cjs index 2831a278f88..f5d9c6e6c59 100644 --- a/actions/setup/js/copilot_sdk_session.cjs +++ b/actions/setup/js/copilot_sdk_session.cjs @@ -174,18 +174,21 @@ function parseSubagentModelsJson(value) { * @returns {Promise<{exitCode: number, output: string, hasOutput: boolean, durationMs: number}>} */ async function runWithCopilotSDK(options) { - /** @type {{ modelObserver: ReturnType | null, sessionId: string }} */ - const verification = { modelObserver: null, sessionId: "" }; + /** @type {{ modelObserver: ReturnType | null, sessionId: string, imageDiagnostic: string }} */ + const verification = { modelObserver: null, sessionId: "", imageDiagnostic: "" }; const result = await runCopilotSDKSession({ ...options, verification }); // Native shutdown records arrive while disconnecting, after the session result is known. const verificationFailure = verification.modelObserver && verification.sessionId ? verification.modelObserver.finalize(verification.sessionId) : ""; + if (verification.imageDiagnostic) { + return { ...result, exitCode: COPILOT_TOOL_IMAGE_EXIT_CODE, output: result.output.includes(verification.imageDiagnostic) ? result.output : `${result.output}\n${verification.imageDiagnostic}` }; + } if (!verificationFailure || result.exitCode === COPILOT_TOOL_IMAGE_EXIT_CODE) return result; // Any verification failure (substitution or unverifiable evidence) is reported with a dedicated exit code so the harness does not retry. return { ...result, exitCode: COPILOT_MODEL_VERIFICATION_EXIT_CODE, output: result.output.includes(verificationFailure) ? result.output : `${result.output}\n${verificationFailure}` }; } /** - * @param {Parameters[0] & { verification: { modelObserver: ReturnType | null, sessionId: string } }} options + * @param {Parameters[0] & { verification: { modelObserver: ReturnType | null, sessionId: string, imageDiagnostic: string } }} options * @returns {Promise<{exitCode: number, output: string, hasOutput: boolean, durationMs: number}>} */ async function runCopilotSDKSession({ @@ -274,6 +277,7 @@ async function runCopilotSDKSession({ let modelSubstitutionError = null; /** @type {Error | null} */ let unsupportedImageError = null; + let sessionClosing = false; /** * Rejects `denialGuardPromise` once the tool-denial threshold is exceeded and the * bounded force-exit interval elapses. Declared here so `recordToolDenial` (defined @@ -496,8 +500,10 @@ async function runCopilotSDKSession({ } session.on(event => { + if (stream.writableEnded || stream.destroyed) return; if (!unsupportedImageError && (providerConfig || isCopilotBYOK()) && hasUnsupportedCopilotToolImage(event)) { unsupportedImageError = new Error(COPILOT_TOOL_IMAGE_DIAGNOSTIC); + verification.imageDiagnostic = unsupportedImageError.message; writeDriverEvent("session.error", { errorType: "unsupported_capability", code: "copilot_tool_image_unsupported", message: unsupportedImageError.message }); if (denialGuardReject) denialGuardReject(unsupportedImageError); void session.abort().catch(() => { @@ -516,7 +522,7 @@ async function runCopilotSDKSession({ } if (denialGuardReject) denialGuardReject(modelSubstitutionError); } - if (stream.writableEnded || stream.destroyed) return; + if (sessionClosing) return; // Defer shell chunks until redaction can protect credentials spanning chunks. // Background output must not extend the post-completion watchdog either. if (event.type === "tool.shell_output") return; @@ -584,7 +590,7 @@ async function runCopilotSDKSession({ // tool call was just started, LLM inference is in progress, or no output yet). // The watchdog fires only if sendAndWait never resolves on its own after // the final tool result is returned — the common SDK post-completion hang. - if (!contentFiltered && hasOutput && pendingToolCalls.size === 0 && !inAssistantTurn) { + if (!unsupportedImageError && !contentFiltered && hasOutput && pendingToolCalls.size === 0 && !inAssistantTurn) { if (postCompletionWatchdog) clearTimeout(postCompletionWatchdog); postCompletionWatchdog = setTimeout(() => { postCompletionWatchdog = null; @@ -681,6 +687,7 @@ async function runCopilotSDKSession({ durationMs, }; } finally { + sessionClosing = true; if (denialGuardTimer) { clearTimeout(denialGuardTimer); denialGuardTimer = null; @@ -690,11 +697,6 @@ async function runCopilotSDKSession({ clearTimeout(postCompletionWatchdog); postCompletionWatchdog = null; } - // Snapshot for null-safe cleanup in this scope. - const stream = eventsStream; - if (stream) { - await new Promise(resolve => stream.end(resolve)); - } // Timeout budget for each cleanup operation. Prevents the driver from // hanging indefinitely when the server is unresponsive after sendAndWait // times out or the watchdog force-disconnects the session. @@ -745,6 +747,11 @@ async function runCopilotSDKSession({ // best-effort cleanup } } + // Keep capturing native events delivered during disconnect and shutdown. + const stream = eventsStream; + if (stream) { + await new Promise(resolve => stream.end(resolve)); + } } } diff --git a/docs/src/content/docs/reference/engines/copilot.md b/docs/src/content/docs/reference/engines/copilot.md index e25f9a52cd4..903c38757cf 100644 --- a/docs/src/content/docs/reference/engines/copilot.md +++ b/docs/src/content/docs/reference/engines/copilot.md @@ -23,7 +23,9 @@ Copilot resolves [model aliases](/gh-aw/reference/engines/#model-aliases) before Copilot CLI's BYOK request assembly can silently omit asset-backed images returned by `view` or MCP tools, even when the tool reports success and records `binaryResultsForLlm`. Sandboxed gh-aw Copilot inference uses BYOK routing through the AWF API proxy, so a vision-capable model alone does not prevent this limitation. No gh-aw capability setting repairs the missing payload. -The built-in CLI harness and SDK driver fail with `copilot_tool_image_unsupported` when a successful tool result contains an image without inline data. CLI mode checks newly appended session events during execution and once more at exit; SDK mode aborts on the tool-completion event. This guard does not prove that other image payloads reached the provider. Custom harnesses and drivers must implement their own checks. +The built-in CLI harness and SDK driver fail with `copilot_tool_image_unsupported` when a successful tool result contains an image without inline data. CLI mode checks newly appended session events during execution and once more at exit; SDK mode aborts on the tool-completion event and checks again after disconnect and shutdown. This guard does not prove that other image payloads reached the provider. Custom harnesses and drivers must implement their own checks. + +BYOK CLI runs require an exact session UUID so the image guard observes only the current invocation. The harness assigns `--session-id ` for a new session and reuses it for continuation retries. Copilot CLI [resumes an existing exact ID with this option](https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-command-reference#command-line-options). User-supplied `--continue`, `--connect`, and resume names or ID prefixes are rejected; use `--resume ` for an existing session. These image-guard restrictions do not apply to native non-BYOK routing. Track the request-assembly fix in [Copilot CLI issue #4600](https://github.com/github/copilot-cli/issues/4600). A successful image-view message or an image asset in session logs is not evidence that the model saw the pixels. Do not rely on `@path` in a prompt file as a verified workaround.