From 3a8dd2fcaf64797b6652ed9eb5c129eaa5993f54 Mon Sep 17 00:00:00 2001 From: mingchuno Date: Wed, 23 Sep 2026 16:46:35 +0100 Subject: [PATCH 1/2] feat(copilot): add bounded evidence tools Let read-only publication and review sessions inspect their captured change without requesting shell access. Restrict listing, chunk reads, and literal search to the verified invocation manifest, with bounded responses and explicit failures for absent or modified evidence. Closes #16 Co-authored-by: Codex Agent-Workflows-Run: 905089e8-f83b-4b66-919a-71daf7030455 --- docs/configuration.md | 9 + docs/providers.md | 6 +- src/adapters/agents.ts | 1 + src/adapters/evidence-tools.ts | 100 ++++++++++ src/adapters/sdk-protocol.ts | 7 + src/domain.ts | 1 + src/evidence-query.ts | 327 +++++++++++++++++++++++++++++++++ src/evidence.ts | 18 +- src/invocation.ts | 16 +- tests/evidence-query.test.ts | 109 +++++++++++ tests/invocation.test.ts | 53 ++++++ tests/sdk-contract.test.ts | 175 ++++++++++++++++++ 12 files changed, 812 insertions(+), 10 deletions(-) create mode 100644 src/adapters/evidence-tools.ts create mode 100644 src/evidence-query.ts create mode 100644 tests/evidence-query.test.ts diff --git a/docs/configuration.md b/docs/configuration.md index e58a145..c8a3938 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -250,6 +250,15 @@ Text chunks are at most 64 KiB; total text evidence, including indexes, is at mo bounded pages; binary changes contain metadata instead of encoded content. These are internal limits, not configurable model context limits. +Copilot publication and review sessions receive runner-owned tools automatically +when captured evidence is present. The tools list changed paths in bounded pages, +read one manifest-backed patch or untracked-content chunk, and search captured +text for a case-sensitive literal with bounded matches. References come only +from the current invocation's verified manifest; arbitrary filesystem paths, +missing pages or chunks, and modified artifacts fail explicitly. List and read +responses report unread chunks so the agent can disclose incomplete inspection. +Binary entries are returned as metadata. No configuration or opt-in is required. + Review output includes `complete` and `limitations`. Incomplete reviews preserve partial findings locally and block normal review publication. Prompt content, output contract and evidence identities are retained with each response attempt. diff --git a/docs/providers.md b/docs/providers.md index 4353c1a..2a53bae 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -95,7 +95,11 @@ providers. Stage task text cannot remove those checks. Both retain their existin inspection permissions: Codex's read-only sandbox and Copilot's read-only permission handler (`approve-once` for reads, `reject` for non-read requests). Managed human-approval requirements remain denied. No shell permission is added -for Copilot. +for Copilot. Copilot publication and review additionally receive three +runner-owned tools for listing, chunk-reading and literal-searching only their +current verified evidence manifest. Tool calls and explicit failures flow into +the invocation event log. The tools do not grant shell, write or arbitrary path +access. Codex tool access and evidence presentation are unchanged. Evidence indexes use absolute paths outside the checkout. Controlled tests check schema mapping, outside-directory reads and denied write/shell requests. The diff --git a/src/adapters/agents.ts b/src/adapters/agents.ts index d540040..4ff19dc 100644 --- a/src/adapters/agents.ts +++ b/src/adapters/agents.ts @@ -120,6 +120,7 @@ export class SDKAgent implements AgentAdapter { outputSchema: invocation.outputSchema, readOnly: invocation.readOnly, timeoutMs: invocation.timeoutMs ?? defaultStageTimeoutMs, + evidence: invocation.evidence, }, invocation, ); diff --git a/src/adapters/evidence-tools.ts b/src/adapters/evidence-tools.ts new file mode 100644 index 0000000..1e9dab9 --- /dev/null +++ b/src/adapters/evidence-tools.ts @@ -0,0 +1,100 @@ +import type { Tool, ToolInvocation } from "@github/copilot-sdk"; +import type { ChangeEvidence } from "../evidence.js"; +import { EvidenceQuery } from "../evidence-query.js"; +import type { Emit } from "./sdk-protocol.js"; + +type ToolOperation = ( + input: Record, + signal?: AbortSignal, +) => Promise; + +export function createEvidenceTools( + evidence: ChangeEvidence, + emit: Emit, +): Tool[] { + const query = new EvidenceQuery(evidence); + const handler = + (toolName: string, operation: ToolOperation) => + async (input: unknown, invocation: ToolInvocation) => { + emit("event", { + type: "evidence.tool.started", + data: { toolName, arguments: input }, + }); + try { + const result = await operation( + input as Record, + invocation.signal, + ); + emit("event", { + type: "evidence.tool.completed", + data: { toolName }, + }); + return result; + } catch (error) { + emit("event", { + type: "evidence.tool.failed", + data: { toolName, error: String(error) }, + }); + throw error; + } + }; + + return [ + { + name: "evidence_list_changes", + description: + "List only this invocation's captured changes in bounded pages. Returns manifest-backed references, change kinds, binary metadata, available chunks, and unread chunks. Start here and paginate until nextPage is null.", + parameters: { + type: "object", + properties: { + page: { type: "integer", minimum: 1, default: 1 }, + pageSize: { type: "integer", minimum: 1, maximum: 50, default: 25 }, + }, + additionalProperties: false, + }, + handler: handler("evidence_list_changes", (args, signal) => + query.list(args as { page?: number; pageSize?: number }, signal), + ), + skipPermission: true, + defer: "never", + }, + { + name: "evidence_read_change", + description: + "Read one bounded patch or untracked-content chunk selected by a reference and chunk ordinal returned by evidence_list_changes. Never accepts filesystem paths. Continue until remainingUnreadChunks is zero or report the inspection limit.", + parameters: { + type: "object", + properties: { + reference: { type: "string", pattern: "^change-[0-9]+$" }, + chunk: { type: "integer", minimum: 0 }, + }, + required: ["reference", "chunk"], + additionalProperties: false, + }, + handler: handler("evidence_read_change", (args, signal) => + query.read(args as { reference: string; chunk: number }, signal), + ), + skipPermission: true, + defer: "never", + }, + { + name: "evidence_search", + description: + "Search only this invocation's captured text for a case-sensitive literal term. Returns bounded manifest references, chunk ordinals, line and column locations, previews, and whether more matches exist. Binary changes are counted but not searched.", + parameters: { + type: "object", + properties: { + term: { type: "string", minLength: 1, maxLength: 256 }, + limit: { type: "integer", minimum: 1, maximum: 50, default: 20 }, + }, + required: ["term"], + additionalProperties: false, + }, + handler: handler("evidence_search", (args, signal) => + query.search(args as { term: string; limit?: number }, signal), + ), + skipPermission: true, + defer: "never", + }, + ]; +} diff --git a/src/adapters/sdk-protocol.ts b/src/adapters/sdk-protocol.ts index 61462fc..3781619 100644 --- a/src/adapters/sdk-protocol.ts +++ b/src/adapters/sdk-protocol.ts @@ -6,6 +6,8 @@ import type { } from "@openai/codex-sdk"; import type { AgentProfile } from "../config.js"; import { defaultStageTimeoutMs } from "../defaults.js"; +import type { ChangeEvidence } from "../evidence.js"; +import { createEvidenceTools } from "./evidence-tools.js"; export interface WorkerInput { provider: "codex" | "copilot"; @@ -18,6 +20,7 @@ export interface WorkerInput { readOnly: boolean; processFile?: string; timeoutMs?: number; + evidence?: ChangeEvidence; } export type Emit = (type: string, value: unknown) => void; export interface CodexClient { @@ -104,6 +107,10 @@ export async function runCopilot( infiniteSessions: input.profile.context ? { enabled: true, ...input.profile.context } : undefined, + tools: + input.readOnly && input.evidence + ? createEvidenceTools(input.evidence, emit) + : undefined, onPermissionRequest: async (request) => request.managedApprovalRequired || (input.readOnly && request.kind !== "read") diff --git a/src/domain.ts b/src/domain.ts index 169f952..e8d27a7 100644 --- a/src/domain.ts +++ b/src/domain.ts @@ -119,6 +119,7 @@ export interface AgentInvocation { readOnly: boolean; signal: AbortSignal; timeoutMs?: number; + evidence?: import("./evidence.js").ChangeEvidence; session: (id: string) => Promise; event: (event: unknown) => Promise; } diff --git a/src/evidence-query.ts b/src/evidence-query.ts new file mode 100644 index 0000000..4b0dbe2 --- /dev/null +++ b/src/evidence-query.ts @@ -0,0 +1,327 @@ +import { readFile } from "node:fs/promises"; +import { z } from "zod"; +import { + type ChangeEvidence, + type EvidenceArtifact, + verifyEvidence, +} from "./evidence.js"; +import { sha256 } from "./prompts.js"; + +export const evidenceQueryLimits = { + defaultPageSize: 25, + maxPageSize: 50, + defaultSearchResults: 20, + maxSearchResults: 50, + maxSearchTermBytes: 256, + previewCharacters: 512, + maxResponseBytes: 512 * 1024, +} as const; + +const artifactSchema = z.object({ + path: z.string().min(1), + sha256: z.string().min(1), + bytes: z.number().int().nonnegative(), +}); +const chunkSchema = artifactSchema.extend({ + ordinal: z.number().int().nonnegative(), + byteOffset: z.number().int().nonnegative(), + startLine: z.number().int().positive(), +}); +const changeSchema = z.object({ + reference: z.string().regex(/^change-[0-9]+$/), + path: z.string().min(1), + kind: z.string().min(1), + change: z.enum(["added", "modified", "deleted"]), + sha256: z.string().min(1), + binary: z.boolean(), + reason: z.string().optional(), + chunks: z.array(chunkSchema), +}); +type EvidenceChange = z.infer; + +const listArguments = z.strictObject({ + page: z.number().int().positive().default(1), + pageSize: z + .number() + .int() + .positive() + .max(evidenceQueryLimits.maxPageSize) + .default(evidenceQueryLimits.defaultPageSize), +}); +const readArguments = z.strictObject({ + reference: z.string().min(1), + chunk: z.number().int().nonnegative(), +}); +const searchArguments = z.strictObject({ + term: z + .string() + .min(1) + .refine( + (term) => + Buffer.byteLength(term) <= evidenceQueryLimits.maxSearchTermBytes, + `Search term exceeds ${evidenceQueryLimits.maxSearchTermBytes} bytes`, + ), + limit: z + .number() + .int() + .positive() + .max(evidenceQueryLimits.maxSearchResults) + .default(evidenceQueryLimits.defaultSearchResults), +}); + +export type ListEvidenceArguments = z.input; +export type ReadEvidenceArguments = z.input; +export type SearchEvidenceArguments = z.input; + +/** A stateful, read-only view of one invocation's verified evidence manifest. */ +export class EvidenceQuery { + private changes?: EvidenceChange[]; + private readonly readChunks = new Set(); + + constructor(private readonly evidence: ChangeEvidence) {} + + async list(input: ListEvidenceArguments, signal?: AbortSignal) { + const { page, pageSize } = listArguments.parse(input); + const changes = await this.load(signal); + const totalPages = Math.max(1, Math.ceil(changes.length / pageSize)); + if (page > totalPages) + throw new Error( + `Evidence page ${page} is absent; available pages are 1-${totalPages}`, + ); + const pageChanges = changes.slice((page - 1) * pageSize, page * pageSize); + return this.bounded({ + page, + pageSize, + totalPages, + totalChanges: changes.length, + changedPaths: this.evidence.changedPaths, + nextPage: page < totalPages ? page + 1 : null, + unreadChunks: this.countUnread(changes), + changes: pageChanges.map((change) => ({ + reference: change.reference, + path: change.path, + kind: change.kind, + change: change.change, + binary: change.binary, + reason: change.reason, + chunks: change.chunks.map(({ ordinal, bytes, startLine }) => ({ + ordinal, + bytes, + startLine, + })), + unreadChunks: change.chunks + .filter((chunk) => !this.readChunks.has(this.chunkKey(change, chunk))) + .map((chunk) => chunk.ordinal), + })), + }); + } + + async read(input: ReadEvidenceArguments, signal?: AbortSignal) { + const { reference, chunk: ordinal } = readArguments.parse(input); + const changes = await this.load(signal); + const change = changes.find( + (candidate) => candidate.reference === reference, + ); + if (!change) throw new Error(`Evidence reference ${reference} is absent`); + const chunk = change.chunks.find( + (candidate) => candidate.ordinal === ordinal, + ); + if (!chunk) + throw new Error( + `Evidence chunk ${ordinal} is absent from reference ${reference}`, + ); + const text = await this.readManifestArtifact(chunk, signal); + this.readChunks.add(this.chunkKey(change, chunk)); + return this.bounded({ + reference, + path: change.path, + kind: change.kind, + change: change.change, + binary: change.binary, + chunk: ordinal, + startLine: chunk.startLine, + byteOffset: chunk.byteOffset, + bytes: chunk.bytes, + text, + remainingUnreadChunks: change.chunks.filter( + (candidate) => !this.readChunks.has(this.chunkKey(change, candidate)), + ).length, + }); + } + + async search(input: SearchEvidenceArguments, signal?: AbortSignal) { + const { term, limit } = searchArguments.parse(input); + const changes = await this.load(signal); + const matches: Array<{ + reference: string; + path: string; + kind: string; + chunk: number; + line: number; + column: number; + previewStartColumn: number; + preview: string; + }> = []; + let hasMore = false; + for (const change of changes) { + signal?.throwIfAborted(); + if (change.binary || change.chunks.length === 0) continue; + const parts = await Promise.all( + change.chunks.map((chunk) => this.readManifestArtifact(chunk, signal)), + ); + const content = parts.join(""); + const characterOffsets: number[] = []; + let characterOffset = 0; + for (const part of parts) { + characterOffsets.push(characterOffset); + characterOffset += part.length; + } + for (let offset = content.indexOf(term); offset >= 0; ) { + if (matches.length === limit) { + hasMore = true; + break; + } + const before = content.slice(0, offset); + const lineStart = before.lastIndexOf("\n") + 1; + const lineEnd = content.indexOf("\n", offset); + const previewStart = Math.max(lineStart, offset - 80); + const previewEnd = Math.min( + lineEnd < 0 ? content.length : lineEnd, + previewStart + evidenceQueryLimits.previewCharacters, + ); + const chunkIndex = Math.max( + 0, + characterOffsets.findLastIndex((start) => start <= offset), + ); + matches.push({ + reference: change.reference, + path: change.path, + kind: change.kind, + chunk: change.chunks[chunkIndex]!.ordinal, + line: before.split("\n").length, + column: offset - lineStart + 1, + previewStartColumn: previewStart - lineStart + 1, + preview: content.slice(previewStart, previewEnd), + }); + offset = content.indexOf(term, offset + Math.max(1, term.length)); + } + if (hasMore) break; + } + return this.bounded({ + term, + limit, + searchedChanges: changes.filter((change) => !change.binary).length, + binaryChanges: changes.filter((change) => change.binary).length, + matches, + truncated: hasMore, + unreadChunks: this.countUnread(changes), + }); + } + + private async load(signal?: AbortSignal): Promise { + signal?.throwIfAborted(); + await verifyEvidence(this.evidence); + if (this.changes) return this.changes; + const indexArtifact = this.manifestArtifact({ + path: this.evidence.index, + sha256: this.evidence.identity, + }); + const root = z + .object({ + version: z.literal(1), + indexDepth: z.number().int().nonnegative(), + pages: z.array(artifactSchema), + }) + .parse( + JSON.parse(await this.readManifestArtifact(indexArtifact, signal)), + ); + let pages = root.pages; + for (let depth = root.indexDepth; depth > 0; depth--) { + const catalog = await this.readPages(pages, signal); + pages = z.array(artifactSchema).parse(JSON.parse(catalog)); + } + const jsonl = await this.readPages(pages, signal); + this.changes = jsonl + .split("\n") + .filter(Boolean) + .map((line) => changeSchema.parse(JSON.parse(line))); + if ( + new Set(this.changes.map((change) => change.reference)).size !== + this.changes.length + ) + throw new Error("Evidence manifest contains duplicate change references"); + return this.changes; + } + + private async readPages( + pages: EvidenceArtifact[], + signal?: AbortSignal, + ): Promise { + const parts = []; + for (const page of pages) { + signal?.throwIfAborted(); + parts.push(await this.readManifestArtifact(page, signal)); + } + return parts.join(""); + } + + private manifestArtifact( + reference: Pick & + Partial>, + ): EvidenceArtifact { + const artifact = this.evidence.files.find( + (candidate) => candidate.path === reference.path, + ); + if ( + !artifact || + artifact.sha256 !== reference.sha256 || + (reference.bytes !== undefined && artifact.bytes !== reference.bytes) + ) + throw new Error( + `Evidence artifact is absent from manifest: ${reference.path}`, + ); + return artifact; + } + + private async readManifestArtifact( + reference: EvidenceArtifact, + signal?: AbortSignal, + ): Promise { + signal?.throwIfAborted(); + const artifact = this.manifestArtifact(reference); + const content = await readFile(artifact.path); + if ( + content.length !== artifact.bytes || + sha256(content) !== artifact.sha256 + ) + throw new Error(`Change evidence changed: ${artifact.path}`); + return content.toString("utf8"); + } + + private chunkKey( + change: EvidenceChange, + chunk: EvidenceArtifact & { ordinal: number }, + ) { + return `${change.reference}:${chunk.ordinal}`; + } + + private countUnread(changes: EvidenceChange[]): number { + return changes.reduce( + (total, change) => + total + + change.chunks.filter( + (chunk) => !this.readChunks.has(this.chunkKey(change, chunk)), + ).length, + 0, + ); + } + + private bounded(response: T): T { + const bytes = Buffer.byteLength(JSON.stringify(response)); + if (bytes > evidenceQueryLimits.maxResponseBytes) + throw new Error( + `Evidence tool response ${bytes} bytes exceeds limit ${evidenceQueryLimits.maxResponseBytes} bytes; request a smaller page or search limit`, + ); + return response; + } +} diff --git a/src/evidence.ts b/src/evidence.ts index ae15959..abd7511 100644 --- a/src/evidence.ts +++ b/src/evidence.ts @@ -9,7 +9,7 @@ export const evidenceLimits = { chunkBytes: 64 * 1024, totalBytes: maxCapturedOutputBytes, } as const; -interface Artifact { +export interface EvidenceArtifact { path: string; sha256: string; bytes: number; @@ -17,7 +17,7 @@ interface Artifact { export interface ChangeEvidence { index: string; identity: string; - files: Artifact[]; + files: EvidenceArtifact[]; changedPaths: number; base: string; head?: string; @@ -36,10 +36,10 @@ export function chunkText(text: string): string[] { return chunks; } export class EvidenceWriter { - readonly files: Artifact[] = []; + readonly files: EvidenceArtifact[] = []; private bytes = 0; constructor(readonly directory: string) {} - async write(name: string, content: string): Promise { + async write(name: string, content: string): Promise { const bytes = Buffer.byteLength(content); const total = this.bytes + bytes; if (total > evidenceLimits.totalBytes) @@ -63,7 +63,7 @@ export class EvidenceWriter { async index( contents: string, metadata: Record, - ): Promise { + ): Promise { let pages = await this.chunks("index", contents); let depth = 0; const serialize = () => @@ -140,7 +140,9 @@ export async function captureEvidence( ...untracked, ]), ].sort(); - const entries = []; + const entries: Record[] = []; + const addEntry = (entry: Record) => + entries.push({ reference: `change-${entries.length}`, ...entry }); for (const [number, path] of paths.entries()) { signal?.throwIfAborted(); if (untracked.includes(path)) { @@ -155,7 +157,7 @@ export async function captureEvidence( } catch { /* Binary metadata is intentional. */ } - entries.push({ + addEntry({ path, kind: "untracked", change: "added", @@ -180,7 +182,7 @@ export async function captureEvidence( const patch = await diff(...range.args, "--", path); if (!patch) continue; const blobs = /^index ([0-9a-f]+)\.\.([0-9a-f]+)/m.exec(patch); - entries.push({ + addEntry({ path, kind: range.kind, change: /^new file mode /m.test(patch) diff --git a/src/invocation.ts b/src/invocation.ts index a725ada..93d1e06 100644 --- a/src/invocation.ts +++ b/src/invocation.ts @@ -128,6 +128,7 @@ function prepareRequest(execution: StageExecution) { const outputSchema = task.outputContract ? z.toJSONSchema(task.outputContract) : undefined; + const profile = resolveProfile(project.agent, stage.profile); const contract = outputSchema ? `Return ONLY JSON satisfying this application-owned schema:\n${JSON.stringify(outputSchema)}` : ""; @@ -137,9 +138,14 @@ function prepareRequest(execution: StageExecution) { task.readOnly ? "Inspection only. Do not modify files, commit, push, or publish." : "Do not commit, push, or publish.", + task.evidence && + profile.provider === "copilot" && + task.readOnly && + (execution.name === "publication" || execution.name === "review") + ? "Use the runner-owned evidence_list_changes, evidence_read_change, and evidence_search tools to inspect the captured change. These tools are the authority for this stage's change evidence. Inspect all material chunks; if any remain unread or unavailable, state that limitation honestly. Do not request shell or write permission." + : "", contract, ].join("\n\n"); - const profile = resolveProfile(project.agent, stage.profile); return { resolved, outputSchema, fullPrompt, profile }; } @@ -290,6 +296,7 @@ async function invokeProvider( const { run, name, + task, dependencies: { project, store, redact }, } = execution; const { adapter, profile, outputSchema, invocationSignal, deadline } = @@ -307,6 +314,13 @@ async function invokeProvider( readOnly, signal: invocationSignal, timeoutMs: Math.max(1, deadline - Date.now()), + evidence: + profile.provider === "copilot" && + readOnly && + task.evidence && + (name === "publication" || name === "review") + ? task.evidence + : undefined, session: async (sessionId) => { record.sessionId = sessionId; record.sessionState = "available"; diff --git a/tests/evidence-query.test.ts b/tests/evidence-query.test.ts new file mode 100644 index 0000000..c3711d0 --- /dev/null +++ b/tests/evidence-query.test.ts @@ -0,0 +1,109 @@ +import assert from "node:assert/strict"; +import { mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import { + type ChangeEvidence, + captureEvidence, + evidenceLimits, +} from "../src/evidence.js"; +import { EvidenceQuery } from "../src/evidence-query.js"; +import { ExistingCheckout } from "../src/workspace.js"; +import { repository } from "./fixtures.js"; + +async function capturedEvidence(): Promise { + const { root, project } = await repository(); + await writeFile( + join(root, "large.txt"), + `${"first line\n".repeat(7_000)}${"x".repeat(1_000)}needle at the end\n`, + ); + await writeFile(join(root, "other.txt"), "literal needle twice: needle\n"); + await writeFile(join(root, "binary.bin"), Buffer.from([0, 1, 2, 3])); + return captureEvidence({ + project, + directory: await mkdtemp(join(tmpdir(), "query-evidence-")), + snapshot: await new ExistingCheckout().inspect(project), + }); +} + +test("evidence queries paginate changes and report unread chunks and binary metadata", async () => { + const query = new EvidenceQuery(await capturedEvidence()); + const first = await query.list({ page: 1, pageSize: 2 }); + assert.equal(first.changes.length, 2); + assert.equal(first.page, 1); + assert.equal(first.nextPage, 2); + assert.ok(first.unreadChunks > 1); + + const large = first.changes.find((change) => change.path === "large.txt"); + assert.ok(large); + assert.ok(large.chunks.length > 1); + assert.deepEqual( + large.unreadChunks, + large.chunks.map((chunk) => chunk.ordinal), + ); + + const read = await query.read({ + reference: large.reference, + chunk: large.chunks[0]!.ordinal, + }); + assert.ok(Buffer.byteLength(read.text) <= evidenceLimits.chunkBytes); + assert.equal(read.remainingUnreadChunks, large.chunks.length - 1); + + const after = await query.list({ page: 1, pageSize: 2 }); + assert.equal(after.unreadChunks, first.unreadChunks - 1); + assert.deepEqual( + after.changes.find((change) => change.reference === large.reference) + ?.unreadChunks, + large.chunks.slice(1).map((chunk) => chunk.ordinal), + ); + + const second = await query.list({ page: 2, pageSize: 2 }); + const binary = [...first.changes, ...second.changes].find( + (change) => change.path === "binary.bin", + ); + assert.equal(binary?.binary, true); + assert.equal(binary?.chunks.length, 0); + assert.match(binary?.reason ?? "", /metadata only/i); +}); + +test("literal search returns bounded manifest locations and distinguishes no matches", async () => { + const query = new EvidenceQuery(await capturedEvidence()); + const result = await query.search({ term: "needle", limit: 2 }); + assert.equal(result.matches.length, 2); + assert.equal(result.limit, 2); + assert.equal(result.truncated, true); + assert.ok(result.matches.every((match) => match.preview.includes("needle"))); + assert.ok( + result.matches.every((match) => match.line > 0 && match.column > 0), + ); + assert.ok( + result.matches.every((match) => match.reference.startsWith("change-")), + ); + + const absent = await query.search({ term: "not present" }); + assert.deepEqual(absent.matches, []); + assert.equal(absent.truncated, false); + assert.ok(absent.searchedChanges > 0); + assert.ok(absent.binaryChanges > 0); +}); + +test("evidence queries reject absent pages, references, chunks, and modified artifacts", async () => { + const evidence = await capturedEvidence(); + const query = new EvidenceQuery(evidence); + await assert.rejects(query.list({ page: 99 }), /page 99 is absent/i); + await assert.rejects( + query.read({ reference: "change-999", chunk: 0 }), + /reference.*absent/i, + ); + const listed = await query.list({ page: 1, pageSize: 10 }); + await assert.rejects( + query.read({ reference: listed.changes[0]!.reference, chunk: 999 }), + /chunk 999 is absent/i, + ); + await assert.rejects(query.search({ term: "" }), /term/i); + + const artifact = evidence.files.find((file) => file.path !== evidence.index)!; + await writeFile(artifact.path, `${await readFile(artifact.path, "utf8")}x`); + await assert.rejects(query.list({}), /evidence changed/i); +}); diff --git a/tests/invocation.test.ts b/tests/invocation.test.ts index d41bf26..87ea334 100644 --- a/tests/invocation.test.ts +++ b/tests/invocation.test.ts @@ -157,6 +157,59 @@ test("custom text stages return literal output without format correction", async assert.equal(calls, 1); }); +test("only Copilot publication and review receive captured evidence tools", async () => { + const { EvidenceWriter } = await import("../src/evidence.js"); + const directory = await mkdtemp(join(tmpdir(), "invocation-evidence-")); + const writer = new EvidenceWriter(directory); + const index = await writer.index("", { changedPaths: 0 }); + const evidence = { + index: index.path, + identity: index.sha256, + files: writer.files, + changedPaths: 0, + base: "base", + }; + const calls: AgentInvocation[] = []; + const setup = await fixture(async (invocation) => { + calls.push(invocation); + return '{"result":"ok"}'; + }); + setup.input.name = "publication"; + setup.input.task.evidence = evidence; + setup.input.dependencies.project.agent.provider = "copilot"; + setup.input.dependencies.agents.copilot = + setup.input.dependencies.agents.codex!; + assert.equal(await invokeStage(setup.input), '{"result":"ok"}'); + assert.equal(calls[0]!.evidence, evidence); + assert.match(calls[0]!.prompt, /evidence_list_changes/); + assert.match(calls[0]!.prompt, /Do not request shell or write permission/); + + const codexCalls: AgentInvocation[] = []; + const codex = await fixture(async (invocation) => { + codexCalls.push(invocation); + return '{"result":"ok"}'; + }); + codex.input.name = "publication"; + codex.input.task.evidence = evidence; + assert.equal(await invokeStage(codex.input), '{"result":"ok"}'); + assert.equal(codexCalls[0]!.evidence, undefined); + assert.doesNotMatch(codexCalls[0]!.prompt, /evidence_list_changes/); + + const implementationCalls: AgentInvocation[] = []; + const implementation = await fixture(async (invocation) => { + implementationCalls.push(invocation); + return '{"result":"ok"}'; + }); + implementation.input.name = "implementation"; + implementation.input.task.evidence = evidence; + implementation.input.dependencies.project.agent.provider = "copilot"; + implementation.input.dependencies.agents.copilot = + implementation.input.dependencies.agents.codex!; + assert.equal(await invokeStage(implementation.input), '{"result":"ok"}'); + assert.equal(implementationCalls[0]!.evidence, undefined); + assert.doesNotMatch(implementationCalls[0]!.prompt, /evidence_list_changes/); +}); + for (const response of ["valid", "invalid", "throws"] as const) test(`final persistence failure takes precedence over ${response} output and stops correction`, async () => { let calls = 0; diff --git a/tests/sdk-contract.test.ts b/tests/sdk-contract.test.ts index c9dbf50..be09f28 100644 --- a/tests/sdk-contract.test.ts +++ b/tests/sdk-contract.test.ts @@ -308,3 +308,178 @@ test("Copilot permits outside-checkout file reads while denying shell and writes ); assert.ok(messages.includes("captured evidence")); }); + +test("Copilot read-only stages expose bounded evidence tools and retain tool failures", async () => { + const { mkdtemp } = await import("node:fs/promises"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + const { EvidenceWriter } = await import("../src/evidence.js"); + const directory = await mkdtemp(join(tmpdir(), "copilot-tools-")); + const writer = new EvidenceWriter(directory); + const chunks = await writer.chunks("patch-0", "patch needle\n"); + const index = await writer.index( + `${JSON.stringify({ + reference: "change-0", + path: "file.txt", + kind: "published", + change: "modified", + sha256: "captured-change", + binary: false, + chunks, + })}\n`, + { base: "base", head: "head", changedPaths: 1 }, + ); + const evidence = { + index: index.path, + identity: index.sha256, + files: writer.files, + changedPaths: 1, + base: "base", + head: "head", + }; + const events: unknown[] = []; + await runCopilot( + { + async start() {}, + async listModels() { + return []; + }, + async createSession(options) { + assert.deepEqual( + options.tools?.map((tool) => tool.name), + ["evidence_list_changes", "evidence_read_change", "evidence_search"], + ); + assert.ok(options.tools?.every((tool) => tool.skipPermission)); + const listed = (await options.tools![0]!.handler!( + { page: 1 }, + { + sessionId: "test", + toolCallId: "list", + toolName: "evidence_list_changes", + arguments: { page: 1 }, + }, + )) as { changes: Array<{ reference: string }> }; + assert.equal(listed.changes[0]!.reference, "change-0"); + const read = (await options.tools![1]!.handler!( + { reference: "change-0", chunk: 0 }, + { + sessionId: "test", + toolCallId: "read", + toolName: "evidence_read_change", + arguments: { reference: "change-0", chunk: 0 }, + }, + )) as { text: string }; + assert.equal(read.text, "patch needle\n"); + const searched = (await options.tools![2]!.handler!( + { term: "needle" }, + { + sessionId: "test", + toolCallId: "search", + toolName: "evidence_search", + arguments: { term: "needle" }, + }, + )) as { matches: unknown[] }; + assert.equal(searched.matches.length, 1); + await assert.rejects( + async () => + options.tools![1]!.handler!( + { reference: "/tmp/arbitrary", chunk: 0 }, + { + sessionId: "test", + toolCallId: "bad-read", + toolName: "evidence_read_change", + arguments: { reference: "/tmp/arbitrary", chunk: 0 }, + }, + ), + /reference.*absent/i, + ); + const permission = options.onPermissionRequest!; + assert.equal( + ( + await permission( + { + kind: "shell", + fullCommandText: "git diff", + commands: [], + possiblePaths: [], + possibleUrls: [], + hasWriteFileRedirection: false, + intention: "inspect", + canOfferSessionApproval: false, + }, + { sessionId: "test" }, + ) + ).kind, + "reject", + ); + return { + sessionId: "test", + on() {}, + async sendAndWait() { + return { data: { content: "finished" } }; + }, + async disconnect() {}, + }; + }, + async stop() { + return []; + }, + async forceStop() {}, + }, + { + ...input, + provider: "copilot", + operation: "invoke", + evidence, + }, + (type, value) => { + if (type === "event") events.push(value); + }, + ); + assert.ok( + events.some( + (event) => (event as { type?: string }).type === "evidence.tool.failed", + ), + ); + assert.ok( + events.some( + (event) => (event as { type?: string }).type === "evidence.tool.started", + ), + ); + assert.ok( + events.some( + (event) => + (event as { type?: string }).type === "evidence.tool.completed", + ), + ); +}); + +test("Copilot does not register evidence tools without invocation evidence", async () => { + let tools: SessionConfig["tools"]; + await runCopilot( + { + async start() {}, + async listModels() { + return []; + }, + async createSession(options) { + tools = options.tools; + return { + sessionId: "none", + on() {}, + async sendAndWait() { + return { data: { content: "finished" } }; + }, + async disconnect() {}, + }; + }, + async stop() { + return []; + }, + async forceStop() {}, + }, + { ...input, provider: "copilot", operation: "invoke" }, + () => {}, + ); + assert.equal(tools, undefined); +}); From eaa423c7efa6044f8ce61717ab45c27d13b2f570 Mon Sep 17 00:00:00 2001 From: mingchuno Date: Wed, 23 Sep 2026 21:11:13 +0100 Subject: [PATCH 2/2] fix(copilot): Bound evidence search and cover stage tools --- src/adapters/evidence-tools.ts | 2 +- src/evidence-query.ts | 110 +++++++++++------ tests/evidence-query.test.ts | 45 +++++++ tests/invocation.test.ts | 211 ++++++++++++++++++++++++++++++--- 4 files changed, 315 insertions(+), 53 deletions(-) diff --git a/src/adapters/evidence-tools.ts b/src/adapters/evidence-tools.ts index 1e9dab9..12472c2 100644 --- a/src/adapters/evidence-tools.ts +++ b/src/adapters/evidence-tools.ts @@ -80,7 +80,7 @@ export function createEvidenceTools( { name: "evidence_search", description: - "Search only this invocation's captured text for a case-sensitive literal term. Returns bounded manifest references, chunk ordinals, line and column locations, previews, and whether more matches exist. Binary changes are counted but not searched.", + "Search only this invocation's captured text for a case-sensitive literal term. Returns bounded manifest references, chunk ordinals, line and column locations, previews, and fully searched versus unsearched change and chunk counts. Binary changes are counted but not searched.", parameters: { type: "object", properties: { diff --git a/src/evidence-query.ts b/src/evidence-query.ts index 4b0dbe2..79dd907 100644 --- a/src/evidence-query.ts +++ b/src/evidence-query.ts @@ -163,54 +163,88 @@ export class EvidenceQuery { preview: string; }> = []; let hasMore = false; + let searchedChanges = 0; + let searchedChunks = 0; for (const change of changes) { signal?.throwIfAborted(); if (change.binary || change.chunks.length === 0) continue; - const parts = await Promise.all( - change.chunks.map((chunk) => this.readManifestArtifact(chunk, signal)), - ); - const content = parts.join(""); - const characterOffsets: number[] = []; - let characterOffset = 0; - for (const part of parts) { - characterOffsets.push(characterOffset); - characterOffset += part.length; - } - for (let offset = content.indexOf(term); offset >= 0; ) { - if (matches.length === limit) { - hasMore = true; - break; + let current = await this.readManifestArtifact(change.chunks[0]!, signal); + let previousTail = ""; + let line = 1; + let lineStart = 0; + let chunkStart = 0; + for (const [index, chunk] of change.chunks.entries()) { + signal?.throwIfAborted(); + const next = change.chunks[index + 1]; + const following = next + ? await this.readManifestArtifact(next, signal) + : ""; + const content = previousTail + current + following; + const currentStart = previousTail.length; + let position = 0; + const advance = (end: number) => { + for (let index = position; index < end; index++) { + if (current.charCodeAt(index) === 10) { + line++; + lineStart = chunkStart + index + 1; + } + } + position = end; + }; + for ( + let offset = content.indexOf(term, currentStart); + offset >= currentStart && offset < currentStart + current.length; + offset = content.indexOf(term, offset + term.length) + ) { + if (matches.length === limit) { + hasMore = true; + break; + } + const localOffset = offset - currentStart; + advance(localOffset); + const absoluteOffset = chunkStart + localOffset; + const previewStart = Math.max(lineStart, absoluteOffset - 80); + const previewOffset = currentStart + previewStart - chunkStart; + const lineEnd = content.indexOf("\n", offset); + const previewEnd = Math.min( + lineEnd < 0 ? content.length : lineEnd, + previewOffset + evidenceQueryLimits.previewCharacters, + ); + matches.push({ + reference: change.reference, + path: change.path, + kind: change.kind, + chunk: chunk.ordinal, + line, + column: absoluteOffset - lineStart + 1, + previewStartColumn: previewStart - lineStart + 1, + preview: content.slice(previewOffset, previewEnd), + }); } - const before = content.slice(0, offset); - const lineStart = before.lastIndexOf("\n") + 1; - const lineEnd = content.indexOf("\n", offset); - const previewStart = Math.max(lineStart, offset - 80); - const previewEnd = Math.min( - lineEnd < 0 ? content.length : lineEnd, - previewStart + evidenceQueryLimits.previewCharacters, - ); - const chunkIndex = Math.max( - 0, - characterOffsets.findLastIndex((start) => start <= offset), - ); - matches.push({ - reference: change.reference, - path: change.path, - kind: change.kind, - chunk: change.chunks[chunkIndex]!.ordinal, - line: before.split("\n").length, - column: offset - lineStart + 1, - previewStartColumn: previewStart - lineStart + 1, - preview: content.slice(previewStart, previewEnd), - }); - offset = content.indexOf(term, offset + Math.max(1, term.length)); + if (hasMore) break; + advance(current.length); + searchedChunks++; + chunkStart += current.length; + previousTail = current.slice(-80); + current = following; } if (hasMore) break; + searchedChanges++; } + const searchableChanges = changes.filter( + (change) => !change.binary && change.chunks.length > 0, + ); return this.bounded({ term, limit, - searchedChanges: changes.filter((change) => !change.binary).length, + searchedChanges, + unsearchedChanges: searchableChanges.length - searchedChanges, + searchedChunks, + unsearchedChunks: + searchableChanges.reduce( + (total, change) => total + change.chunks.length, + 0, + ) - searchedChunks, binaryChanges: changes.filter((change) => change.binary).length, matches, truncated: hasMore, diff --git a/tests/evidence-query.test.ts b/tests/evidence-query.test.ts index c3711d0..fa40c50 100644 --- a/tests/evidence-query.test.ts +++ b/tests/evidence-query.test.ts @@ -73,6 +73,9 @@ test("literal search returns bounded manifest locations and distinguishes no mat assert.equal(result.matches.length, 2); assert.equal(result.limit, 2); assert.equal(result.truncated, true); + assert.ok(result.unsearchedChanges > 0); + assert.ok(result.unsearchedChunks > 0); + assert.equal(result.searchedChanges, 1); assert.ok(result.matches.every((match) => match.preview.includes("needle"))); assert.ok( result.matches.every((match) => match.line > 0 && match.column > 0), @@ -86,6 +89,48 @@ test("literal search returns bounded manifest locations and distinguishes no mat assert.equal(absent.truncated, false); assert.ok(absent.searchedChanges > 0); assert.ok(absent.binaryChanges > 0); + assert.equal(absent.unsearchedChanges, 0); + assert.equal(absent.unsearchedChunks, 0); +}); + +test("search reports partial coverage when the first change exceeds the limit", async () => { + const { root, project } = await repository(); + await writeFile(join(root, "a.txt"), "needle needle needle\n"); + await writeFile(join(root, "b.txt"), "needle\n"); + const evidence = await captureEvidence({ + project, + directory: await mkdtemp(join(tmpdir(), "query-coverage-")), + snapshot: await new ExistingCheckout().inspect(project), + }); + const result = await new EvidenceQuery(evidence).search({ + term: "needle", + limit: 1, + }); + assert.equal(result.truncated, true); + assert.equal(result.searchedChanges, 0); + assert.equal(result.unsearchedChanges, 2); + assert.equal(result.searchedChunks, 0); + assert.equal(result.unsearchedChunks, 2); +}); + +test("search finds a literal across chunks with its starting location", async () => { + const { root, project } = await repository(); + await writeFile( + join(root, "split.txt"), + `${"x".repeat(evidenceLimits.chunkBytes - 3)}needle\n`, + ); + const evidence = await captureEvidence({ + project, + directory: await mkdtemp(join(tmpdir(), "query-boundary-")), + snapshot: await new ExistingCheckout().inspect(project), + }); + const result = await new EvidenceQuery(evidence).search({ term: "needle" }); + assert.equal(result.matches.length, 1); + assert.equal(result.matches[0]?.chunk, 0); + assert.equal(result.matches[0]?.line, 1); + assert.equal(result.matches[0]?.column, evidenceLimits.chunkBytes - 2); + assert.match(result.matches[0]!.preview, /needle/); + assert.equal(result.unsearchedChunks, 0); }); test("evidence queries reject absent pages, references, chunks, and modified artifacts", async () => { diff --git a/tests/invocation.test.ts b/tests/invocation.test.ts index 87ea334..31960bf 100644 --- a/tests/invocation.test.ts +++ b/tests/invocation.test.ts @@ -3,13 +3,17 @@ import { mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; +import type { SessionConfig } from "@github/copilot-sdk"; import { z } from "zod"; +import { runCopilot } from "../src/adapters/sdk-protocol.js"; import { stageSchema } from "../src/config.js"; import type { AgentAdapter, AgentInvocation, RunRecord, } from "../src/domain.js"; +import { publicationSchema } from "../src/domain.js"; +import { captureEvidence } from "../src/evidence.js"; import { invokeStage } from "../src/invocation.js"; import type { InvocationRecord, Store } from "../src/store.js"; import { ExistingCheckout } from "../src/workspace.js"; @@ -169,20 +173,64 @@ test("only Copilot publication and review receive captured evidence tools", asyn changedPaths: 0, base: "base", }; - const calls: AgentInvocation[] = []; - const setup = await fixture(async (invocation) => { - calls.push(invocation); - return '{"result":"ok"}'; - }); - setup.input.name = "publication"; - setup.input.task.evidence = evidence; - setup.input.dependencies.project.agent.provider = "copilot"; - setup.input.dependencies.agents.copilot = - setup.input.dependencies.agents.codex!; - assert.equal(await invokeStage(setup.input), '{"result":"ok"}'); - assert.equal(calls[0]!.evidence, evidence); - assert.match(calls[0]!.prompt, /evidence_list_changes/); - assert.match(calls[0]!.prompt, /Do not request shell or write permission/); + for (const name of ["publication", "review"] as const) { + const calls: AgentInvocation[] = []; + const setup = await fixture(async (invocation) => { + calls.push(invocation); + return '{"result":"ok"}'; + }); + setup.input.name = name; + setup.input.task.evidence = evidence; + setup.input.dependencies.project.agent.provider = "copilot"; + setup.input.dependencies.agents.copilot = + setup.input.dependencies.agents.codex!; + assert.equal(await invokeStage(setup.input), '{"result":"ok"}'); + assert.equal(calls[0]!.evidence, evidence); + assert.match(calls[0]!.prompt, /evidence_list_changes/); + assert.match(calls[0]!.prompt, /Do not request shell or write permission/); + if (name === "review") { + let registeredTools: string[] | undefined; + await runCopilot( + { + async start() {}, + async listModels() { + return []; + }, + async createSession(options) { + registeredTools = options.tools?.map((tool) => tool.name); + return { + sessionId: "review-session", + on() {}, + async sendAndWait() { + return { data: { content: "{}" } }; + }, + async disconnect() {}, + }; + }, + async stop() { + return []; + }, + async forceStop() {}, + }, + { + provider: "copilot", + operation: "invoke", + id: calls[0]!.id, + cwd: calls[0]!.cwd, + prompt: calls[0]!.prompt, + profile: calls[0]!.profile, + readOnly: calls[0]!.readOnly, + evidence: calls[0]!.evidence, + }, + () => {}, + ); + assert.deepEqual(registeredTools, [ + "evidence_list_changes", + "evidence_read_change", + "evidence_search", + ]); + } + } const codexCalls: AgentInvocation[] = []; const codex = await fixture(async (invocation) => { @@ -210,6 +258,141 @@ test("only Copilot publication and review receive captured evidence tools", asyn assert.doesNotMatch(implementationCalls[0]!.prompt, /evidence_list_changes/); }); +test("a Copilot publication invocation navigates multi-chunk evidence and returns valid JSON without shell approval", async () => { + const { root, project } = await repository(); + await writeFile( + join(root, "large.txt"), + `${"context line\n".repeat(7_000)}publication needle\n`, + ); + const evidence = await captureEvidence({ + project, + directory: await mkdtemp(join(tmpdir(), "publication-evidence-")), + snapshot: await new ExistingCheckout().inspect(project), + }); + let sessions = 0; + let inspectedChunks = 0; + const output = JSON.stringify({ + commitMessage: "feat: describe captured change", + title: "Describe captured change", + description: + "Inspected all captured chunks and found the publication needle.", + }); + const setup = await fixture(async (invocation) => { + const pending: Promise[] = []; + let result = ""; + await runCopilot( + { + async start() {}, + async listModels() { + return []; + }, + async createSession(options: SessionConfig) { + sessions++; + assert.deepEqual( + options.tools?.map((tool) => tool.name), + [ + "evidence_list_changes", + "evidence_read_change", + "evidence_search", + ], + ); + assert.ok(options.tools?.every((tool) => tool.skipPermission)); + const tools = options.tools!; + const call = async (index: number, args: Record) => + tools[index]!.handler!(args, { + sessionId: "publication-session", + toolCallId: `call-${index}-${inspectedChunks}`, + toolName: tools[index]!.name, + arguments: args, + }); + return { + sessionId: "publication-session", + on() {}, + async sendAndWait() { + const listed = (await call(0, { page: 1 })) as { + changes: Array<{ + reference: string; + chunks: Array<{ ordinal: number }>; + }>; + }; + assert.equal(listed.changes.length, 1); + assert.ok(listed.changes[0]!.chunks.length > 1); + const found = (await call(2, { term: "publication needle" })) as { + matches: Array<{ reference: string }>; + }; + assert.equal( + found.matches[0]?.reference, + listed.changes[0]!.reference, + ); + for (const chunk of listed.changes[0]!.chunks) { + const read = (await call(1, { + reference: listed.changes[0]!.reference, + chunk: chunk.ordinal, + })) as { text: string; remainingUnreadChunks: number }; + inspectedChunks++; + if (inspectedChunks === listed.changes[0]!.chunks.length) + assert.equal(read.remainingUnreadChunks, 0); + } + const permission = await options.onPermissionRequest!( + { + kind: "shell", + fullCommandText: "git diff", + commands: [], + possiblePaths: [], + possibleUrls: [], + hasWriteFileRedirection: false, + intention: "inspect", + canOfferSessionApproval: false, + }, + { sessionId: "publication-session" }, + ); + assert.equal(permission.kind, "reject"); + return { data: { content: output } }; + }, + async disconnect() {}, + }; + }, + async stop() { + return []; + }, + async forceStop() {}, + }, + { + provider: "copilot", + operation: "invoke", + id: invocation.id, + cwd: invocation.cwd, + prompt: invocation.prompt, + profile: invocation.profile, + readOnly: invocation.readOnly, + evidence: invocation.evidence, + outputSchema: invocation.outputSchema, + }, + (type, value) => { + if (type === "result") result = value as string; + if (type === "session") + pending.push(invocation.session(value as string)); + if (type === "event") pending.push(invocation.event(value)); + }, + ); + await Promise.all(pending); + return result; + }); + setup.input.name = "publication"; + setup.input.task.evidence = evidence; + setup.input.task.outputContract = publicationSchema; + setup.input.dependencies.project.agent.provider = "copilot"; + setup.input.dependencies.agents.copilot = + setup.input.dependencies.agents.codex!; + const result = await invokeStage(setup.input); + assert.deepEqual( + publicationSchema.parse(JSON.parse(result)), + JSON.parse(output), + ); + assert.equal(sessions, 1); + assert.ok(inspectedChunks > 1); +}); + for (const response of ["valid", "invalid", "throws"] as const) test(`final persistence failure takes precedence over ${response} output and stops correction`, async () => { let calls = 0;