diff --git a/docs/concepts/agent-runtimes.md b/docs/concepts/agent-runtimes.md index 4fe591de50f0..19a446a0190a 100644 --- a/docs/concepts/agent-runtimes.md +++ b/docs/concepts/agent-runtimes.md @@ -230,8 +230,8 @@ canonical subscription `github-copilot` provider and is **never** selected by The harness claims its provider, runtime, CLI session key, and auth profile prefix in `extensions/copilot/doctor-contract-api.ts`, which `openclaw doctor` auto-loads. For configuration, auth, transcript mirroring, -compaction, the doctor probe surface, and the broader PI vs Codex vs Copilot -SDK decision, see [GitHub Copilot agent runtime](/plugins/copilot). +compaction, the declarative doctor contract, and the broader PI vs Codex vs +Copilot SDK decision, see [GitHub Copilot agent runtime](/plugins/copilot). ## Compatibility contract diff --git a/docs/plugins/copilot.md b/docs/plugins/copilot.md index 30914beff089..ae015704486a 100755 --- a/docs/plugins/copilot.md +++ b/docs/plugins/copilot.md @@ -33,15 +33,12 @@ For the broader model/provider/runtime split, start with - A GitHub Copilot subscription that can drive the Copilot CLI (or a `gitHubToken` env / auth-profile entry for headless / cron runs). - A writable `copilotHome` directory. The harness defaults to - `~/.openclaw/agents//copilot` for full per-agent isolation. The - platform default (`%APPDATA%\copilot` on Windows, `$XDG_CONFIG_HOME/copilot` - or `~/.config/copilot` elsewhere) is used as the doctor probe fallback when - no explicit home is set. + `/copilot` when OpenClaw provides an agent directory, otherwise + `~/.openclaw/agents//copilot` for full per-agent isolation. `openclaw doctor` runs the plugin -[doctor contract](#doctor-and-probes) for the extension; failures there are -the canonical way to confirm the environment is ready before opting an agent -in. +[doctor contract](#doctor) for declarative session-state ownership and future +compatibility migrations. It does not run Copilot CLI environment probes. ## Plugin install @@ -153,10 +150,6 @@ the same directory), or `~/.openclaw/agents//copilot` otherwise. Override with `copilotHome: ` on the attempt input when you need a custom location (for example, a shared mount for migration). -`probeCopilotAuthShape` (see [Doctor and probes](#doctor-and-probes)) is the -pure shape check that validates which of the modes above will be used. -It does not perform a live SDK handshake. - ## Configuration surface The harness reads its config from per-attempt input @@ -239,7 +232,7 @@ asserted in [`extensions/copilot/harness.test.ts`](https://github.com/openclaw/openclaw/blob/main/extensions/copilot/harness.test.ts) under `describe("runSideQuestion")`. -## Doctor and probes +## Doctor `extensions/copilot/doctor-contract-api.ts` is auto-loaded by `src/plugins/doctor-contract-registry.ts`. It contributes: @@ -251,18 +244,6 @@ under `describe("runSideQuestion")`. runtime `copilot`; CLI session key `copilot`; auth profile prefix `github-copilot:`. -`extensions/copilot/src/doctor-probes.ts` exports three imperative probes -that hosts (including `openclaw doctor`) can call to verify the environment: - -| Probe | What it checks | Reasons it can fail | -| -------------------------- | --------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- | -| `probeCopilotCliVersion` | `copilot --version` exits 0 with a non-empty version string | `non-zero-exit`, `empty-version`, `spawn-failed`, `spawn-error`, `probe-timeout` | -| `probeCopilotHomeWritable` | `mkdir -p copilotHome` + write + rm a marker file | `copilothome-not-writable` (with the underlying fs error in `details.rawError`) | -| `probeCopilotAuthShape` | At least one of `useLoggedInUser`, `gitHubToken`, or `profileId`+`profileVersion` | `no-auth-source` | - -Each probe accepts a DI seam (`spawnFn`, `fsApi`) so tests do not spawn the -real Copilot CLI or touch the host fs. - ## Limitations - The harness only claims the canonical `github-copilot` provider at MVP. diff --git a/extensions/copilot/README.md b/extensions/copilot/README.md index f625d2a8d1b8..99cb5b13fe4e 100644 --- a/extensions/copilot/README.md +++ b/extensions/copilot/README.md @@ -16,7 +16,7 @@ on a model or provider entry; `auto` never picks it. PI remains the default embedded runtime. See [GitHub Copilot agent runtime](../../docs/plugins/copilot.md) for -configuration, doctor probes, transcript mirroring, compaction, side +configuration, the doctor contract, transcript mirroring, compaction, side questions, replay, and the supported-surface contract. See [qa/copilot-capabilities.md](../../qa/copilot-capabilities.md) for the SDK capability inventory the harness is pinned to. diff --git a/extensions/copilot/doctor-contract-api.ts b/extensions/copilot/doctor-contract-api.ts index ddf6a1d67a71..bc9745584539 100755 --- a/extensions/copilot/doctor-contract-api.ts +++ b/extensions/copilot/doctor-contract-api.ts @@ -11,14 +11,6 @@ * fields exist for copilot yet; the array is empty by design * and normalizeCompatibilityConfig is a structural no-op so * future retirements have a stable in-tree home. - * - * The deeper runtime probes (copilot CLI version, copilot auth, - * copilotHome writability) live in {@link ./src/doctor-probes.ts} - * because they have side effects (subprocess spawn, fs touch) and - * need to be invoked imperatively, not declaratively, from the - * doctor command. They are exported separately so callers can opt - * in. Auto-discovery of doctor-contract-api.ts at the plugin root - * keeps this file purely declarative. */ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; diff --git a/extensions/copilot/src/doctor-probes.test.ts b/extensions/copilot/src/doctor-probes.test.ts deleted file mode 100755 index b6d323a5a98f..000000000000 --- a/extensions/copilot/src/doctor-probes.test.ts +++ /dev/null @@ -1,284 +0,0 @@ -// Copilot tests cover doctor probes plugin behavior. -import { EventEmitter } from "node:events"; -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { - probeCopilotAuthShape, - probeCopilotCliVersion, - probeCopilotHomeWritable, -} from "./doctor-probes.js"; - -type FakeChildOptions = { - exitCode?: number | null; - signal?: NodeJS.Signals | null; - stdout?: string; - stderr?: string; - emitErrorMessage?: string; - /** When true, never emits close; useful for timeout tests. */ - hang?: boolean; -}; - -function makeFakeChild(opts: FakeChildOptions = {}) { - const emitter = new EventEmitter() as EventEmitter & { - stdout: EventEmitter; - stderr: EventEmitter; - kill: () => void; - }; - emitter.stdout = new EventEmitter(); - emitter.stderr = new EventEmitter(); - emitter.kill = vi.fn(); - - queueMicrotask(() => { - if (opts.stdout) { - emitter.stdout.emit("data", Buffer.from(opts.stdout, "utf8")); - } - if (opts.stderr) { - emitter.stderr.emit("data", Buffer.from(opts.stderr, "utf8")); - } - if (opts.emitErrorMessage) { - emitter.emit("error", new Error(opts.emitErrorMessage)); - return; - } - if (!opts.hang) { - emitter.emit("close", opts.exitCode ?? 0, opts.signal ?? null); - } - }); - - return emitter; -} - -const tempDirs: string[] = []; - -afterEach(async () => { - for (const dir of tempDirs.splice(0)) { - await fs.rm(dir, { recursive: true, force: true }); - } -}); - -async function makeTempHome(): Promise { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-copilot-doctor-")); - tempDirs.push(dir); - return dir; -} - -describe("probeCopilotCliVersion", () => { - it("reports ok with trimmed version on exit 0 with stdout", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: () => makeFakeChild({ stdout: " 1.2.3 \n" }) as never, - }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.version).toBe("1.2.3"); - expect(result.command).toBe("copilot"); - } - }); - - it("uses custom command and args when provided", async () => { - const calls: Array<{ cmd: string; args: string[] }> = []; - const result = await probeCopilotCliVersion({ - command: "my-copilot", - args: ["-V"], - spawnFn: ((cmd: string, args: readonly string[]) => { - calls.push({ cmd, args: [...args] }); - return makeFakeChild({ stdout: "9.9.9" }) as never; - }) as never, - }); - expect(calls).toEqual([{ cmd: "my-copilot", args: ["-V"] }]); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.command).toBe("my-copilot"); - } - }); - - it("reports non-zero-exit with stderr details", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: () => makeFakeChild({ exitCode: 2, stderr: "boom: not installed" }) as never, - }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("non-zero-exit"); - expect(result.details?.exitCode).toBe(2); - expect(result.details?.stderr).toBe("boom: not installed"); - } - }); - - it("reports empty-version when exit 0 produces no stdout", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: () => makeFakeChild({ stdout: " \n" }) as never, - }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("empty-version"); - } - }); - - it("reports spawn-failed when spawnFn throws synchronously (e.g. ENOENT)", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: (() => { - throw new Error("ENOENT: copilot not found"); - }) as never, - }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("spawn-failed"); - expect(result.details?.rawError).toContain("ENOENT"); - } - }); - - it("reports spawn-error when child emits 'error'", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: () => makeFakeChild({ emitErrorMessage: "spawn ENOEXEC" }) as never, - }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("spawn-error"); - expect(result.details?.rawError).toBe("spawn ENOEXEC"); - } - }); - - it("reports probe-timeout when child hangs past timeoutMs and kills the child", async () => { - const fakeChild = makeFakeChild({ hang: true }); - const result = await probeCopilotCliVersion({ - timeoutMs: 10, - spawnFn: () => fakeChild as never, - }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("probe-timeout"); - expect(result.details?.timeoutMs).toBe(10); - } - expect(fakeChild.kill).toHaveBeenCalled(); - }); - - it("returns just the first non-empty line as version when stdout has a banner / update hint", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: () => - makeFakeChild({ - stdout: "GitHub Copilot CLI 1.0.48.\nRun 'copilot update' to check for updates.\n", - }) as never, - }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.version).toBe("GitHub Copilot CLI 1.0.48."); - expect(result.rawStdout).toBe( - "GitHub Copilot CLI 1.0.48.\nRun 'copilot update' to check for updates.", - ); - } - }); - - it("does not surface rawStdout when stdout is already single-line", async () => { - const result = await probeCopilotCliVersion({ - spawnFn: () => makeFakeChild({ stdout: "1.2.3\n" }) as never, - }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.version).toBe("1.2.3"); - expect(result.rawStdout).toBeUndefined(); - } - }); -}); - -describe("probeCopilotHomeWritable", () => { - it("reports ok when the directory exists and is writable, cleaning up after itself", async () => { - const home = await makeTempHome(); - const result = await probeCopilotHomeWritable(home); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.copilotHome).toBe(home); - expect(result.probedPath.startsWith(home)).toBe(true); - } - const entries = await fs.readdir(home); - expect(entries).toEqual([]); - }); - - it("creates copilotHome if missing", async () => { - const root = await makeTempHome(); - const home = path.join(root, "nested", "copilot-cfg"); - const result = await probeCopilotHomeWritable(home); - expect(result.ok).toBe(true); - const stat = await fs.stat(home); - expect(stat.isDirectory()).toBe(true); - }); - - it("reports copilothome-not-writable when fs throws on mkdir", async () => { - const result = await probeCopilotHomeWritable("/some/path", { - fsApi: { - mkdir: vi.fn().mockRejectedValueOnce(new Error("EPERM: not permitted")), - writeFile: vi.fn(), - rm: vi.fn(), - } as never, - }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("copilothome-not-writable"); - expect(result.details?.rawError).toContain("EPERM"); - } - }); - - it("falls back to the platform default copilotHome when argument is empty or whitespace", async () => { - const writeFile = vi.fn().mockResolvedValue(undefined); - const result = await probeCopilotHomeWritable(" ", { - fsApi: { - mkdir: vi.fn().mockResolvedValue(undefined), - writeFile, - rm: vi.fn().mockResolvedValue(undefined), - } as never, - }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.copilotHome.length).toBeGreaterThan(0); - expect(result.copilotHome.toLowerCase()).toContain("copilot"); - } - }); -}); - -describe("probeCopilotAuthShape", () => { - it("resolves to useLoggedInUser when the flag is true", () => { - const result = probeCopilotAuthShape({ useLoggedInUser: true }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.resolvedMode).toBe("useLoggedInUser"); - } - }); - - it("resolves to gitHubToken when a non-empty token is supplied", () => { - const result = probeCopilotAuthShape({ gitHubToken: "ghp_xxx" }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.resolvedMode).toBe("gitHubToken"); - } - }); - - it("resolves to profile when both profileId and profileVersion are supplied", () => { - const result = probeCopilotAuthShape({ profileId: "p1", profileVersion: "v1" }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.resolvedMode).toBe("profile"); - } - }); - - it("rejects when no auth source is provided", () => { - const result = probeCopilotAuthShape({}); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.reason).toBe("no-auth-source"); - } - }); - - it("rejects when only one of profileId / profileVersion is provided", () => { - expect(probeCopilotAuthShape({ profileId: "p1" }).ok).toBe(false); - expect(probeCopilotAuthShape({ profileVersion: "v1" }).ok).toBe(false); - }); - - it("rejects useLoggedInUser:false on its own", () => { - const result = probeCopilotAuthShape({ useLoggedInUser: false }); - expect(result.ok).toBe(false); - }); - - it("rejects an empty gitHubToken string", () => { - const result = probeCopilotAuthShape({ gitHubToken: "" }); - expect(result.ok).toBe(false); - }); -}); diff --git a/extensions/copilot/src/doctor-probes.ts b/extensions/copilot/src/doctor-probes.ts deleted file mode 100755 index d18f9a9cbf5e..000000000000 --- a/extensions/copilot/src/doctor-probes.ts +++ /dev/null @@ -1,260 +0,0 @@ -/** - * Runtime doctor probes for the copilot extension. - * - * Imperative side-effecting checks used to diagnose a copilot - * deployment from within `openclaw doctor` (or any equivalent - * harness-side health check). Kept out of doctor-contract-api.ts - * because that contract is declarative and auto-loaded by the - * plugin registry, whereas these probes spawn subprocesses or - * touch the filesystem and must be invoked imperatively. - * - * All probes are pure (no module-level state) and dependency- - * injectable for tests. They never throw on a probe-negative - * result — failure is surfaced via the `ok: false` shape so the - * caller can render a structured doctor report. - */ - -import { spawn } from "node:child_process"; -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; - -export type ProbeResult> = - | ({ ok: true } & TPayload) - | { ok: false; reason: string; details?: Record }; - -export interface ProbeCopilotCliVersionOptions { - /** Command to invoke; defaults to "copilot". */ - command?: string; - /** Argv used to ask for version; defaults to ["--version"]. */ - args?: readonly string[]; - /** Timeout in milliseconds; defaults to 5_000. */ - timeoutMs?: number; - /** Injection seam for testing. Defaults to node:child_process spawn. */ - spawnFn?: typeof spawn; -} - -export interface ProbeCopilotHomeOptions { - /** Injection seam for testing. */ - fsApi?: Pick; - /** Filename used for the writability probe. */ - probeFileName?: string; -} - -const DEFAULT_PROBE_TIMEOUT_MS = 5_000; -const DEFAULT_PROBE_FILENAME = ".copilot-doctor-probe"; - -/** - * Probe that the Copilot CLI is installed and prints a version. - * Treats non-zero exit, missing stdout, and timeout all as failures. - */ -export async function probeCopilotCliVersion( - options: ProbeCopilotCliVersionOptions = {}, -): Promise> { - const command = options.command ?? "copilot"; - const args = options.args ?? ["--version"]; - const timeoutMs = options.timeoutMs ?? DEFAULT_PROBE_TIMEOUT_MS; - const spawnImpl = options.spawnFn ?? spawn; - - return new Promise>( - (resolve) => { - let child: ReturnType | undefined; - let settled = false; - const settle = ( - result: ProbeResult<{ version: string; command: string; rawStdout?: string }>, - ): void => { - if (settled) { - return; - } - settled = true; - if (timer) { - clearTimeout(timer); - } - try { - child?.kill(); - } catch { - // ignore double-kill / already-dead errors - } - resolve(result); - }; - - const timer = setTimeout(() => { - settle({ - ok: false, - reason: "probe-timeout", - details: { command, args: [...args], timeoutMs }, - }); - }, timeoutMs); - - try { - child = spawnImpl(command, [...args], { stdio: ["ignore", "pipe", "pipe"] }); - } catch (error) { - settle({ - ok: false, - reason: "spawn-failed", - details: { command, args: [...args], rawError: formatProbeError(error) }, - }); - return; - } - - let stdout = ""; - let stderr = ""; - child.stdout?.on("data", (chunk: Buffer) => { - stdout += chunk.toString("utf8"); - }); - child.stderr?.on("data", (chunk: Buffer) => { - stderr += chunk.toString("utf8"); - }); - child.on("error", (error: Error) => { - settle({ - ok: false, - reason: "spawn-error", - details: { command, args: [...args], rawError: error.message }, - }); - }); - child.on("close", (code: number | null, signal: NodeJS.Signals | null) => { - if (code !== 0) { - settle({ - ok: false, - reason: "non-zero-exit", - details: { - command, - args: [...args], - exitCode: code, - signal, - stderr: stderr.trim() || undefined, - }, - }); - return; - } - const rawStdout = stdout.trim(); - if (!rawStdout) { - settle({ - ok: false, - reason: "empty-version", - details: { command, args: [...args] }, - }); - return; - } - // Many version commands (notably the GitHub Copilot CLI's `copilot --version`) - // print a banner plus an "update available" hint on subsequent - // lines. Surface only the first non-empty line as `version` so the - // doctor UI gets a clean string; keep the full stdout in - // `rawStdout` for debugging. - const version = firstNonEmptyLine(rawStdout) ?? rawStdout; - const payload: { version: string; command: string; rawStdout?: string } = { - version, - command, - }; - if (rawStdout !== version) { - payload.rawStdout = rawStdout; - } - settle({ ok: true, ...payload }); - }); - }, - ); -} - -function firstNonEmptyLine(value: string): string | undefined { - for (const line of value.split(/\r?\n/)) { - const trimmed = line.trim(); - if (trimmed.length > 0) { - return trimmed; - } - } - return undefined; -} - -/** - * Probe that copilotHome (or default ~/.config/copilot) is writable - * by the running user. Mirrors the existing auth-bridge's expectation - * that the SDK can persist credentials under copilotHome. - */ -export async function probeCopilotHomeWritable( - copilotHome: string | undefined, - options: ProbeCopilotHomeOptions = {}, -): Promise> { - const fsApi = options.fsApi ?? fs; - const probeFileName = options.probeFileName ?? DEFAULT_PROBE_FILENAME; - const resolvedHome = - typeof copilotHome === "string" && copilotHome.trim().length > 0 - ? copilotHome.trim() - : defaultCopilotHome(); - const probedPath = path.join(resolvedHome, probeFileName); - - try { - await fsApi.mkdir(resolvedHome, { recursive: true }); - await fsApi.writeFile(probedPath, "copilot-doctor-probe", "utf8"); - await fsApi.rm(probedPath, { force: true }); - return { ok: true, copilotHome: resolvedHome, probedPath }; - } catch (error) { - return { - ok: false, - reason: "copilothome-not-writable", - details: { - copilotHome: resolvedHome, - probedPath, - rawError: formatProbeError(error), - }, - }; - } -} - -/** - * Probe GitHub Copilot agent runtime auth resolution given a useLoggedInUser hint. - * Validates that at least one of {useLoggedInUser, gitHubToken, - * profileId+profileVersion} is set. This is intentionally a - * shape-only probe: actually performing an SDK auth handshake - * would require a pool and is out of scope for `openclaw doctor`. - */ -export function probeCopilotAuthShape(input: { - useLoggedInUser?: boolean; - gitHubToken?: string; - profileId?: string; - profileVersion?: string; -}): ProbeResult<{ resolvedMode: "useLoggedInUser" | "gitHubToken" | "profile" }> { - if (input.useLoggedInUser === true) { - return { ok: true, resolvedMode: "useLoggedInUser" }; - } - if (typeof input.gitHubToken === "string" && input.gitHubToken.length > 0) { - return { ok: true, resolvedMode: "gitHubToken" }; - } - if ( - typeof input.profileId === "string" && - input.profileId.length > 0 && - typeof input.profileVersion === "string" && - input.profileVersion.length > 0 - ) { - return { ok: true, resolvedMode: "profile" }; - } - return { - ok: false, - reason: "no-auth-source", - details: { - hint: "Set useLoggedInUser:true, or gitHubToken, or both profileId+profileVersion", - }, - }; -} - -function defaultCopilotHome(): string { - // Mirrors the SDK convention; auth-bridge uses the same default. - if (process.platform === "win32") { - return path.join(process.env.APPDATA ?? os.homedir(), "copilot"); - } - const xdg = process.env.XDG_CONFIG_HOME; - if (xdg && xdg.length > 0) { - return path.join(xdg, "copilot"); - } - return path.join(os.homedir(), ".config", "copilot"); -} - -function formatProbeError(error: unknown): string { - if (error instanceof Error) { - return error.message; - } - try { - return JSON.stringify(error); - } catch { - return String(error); - } -} diff --git a/scripts/deadcode-unused-files.allowlist.mjs b/scripts/deadcode-unused-files.allowlist.mjs index c42fd924b521..022eecc5f845 100644 --- a/scripts/deadcode-unused-files.allowlist.mjs +++ b/scripts/deadcode-unused-files.allowlist.mjs @@ -13,7 +13,6 @@ export const KNIP_OPTIONAL_UNUSED_FILE_ALLOWLIST = [ "extensions/acpx/src/runtime-internals/mcp-proxy.mjs", "extensions/canvas/src/host/a2ui-app/bootstrap.js", "extensions/canvas/src/host/a2ui-app/rolldown.config.mjs", - "extensions/copilot/src/doctor-probes.ts", "extensions/diffs/src/viewer-client.ts", "extensions/diffs/src/viewer-payload.ts", "extensions/matrix/src/plugin-entry.runtime.js",