From 41d9b2746e1eefc640e86e2f4390ff465b29d772 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 18:27:34 -0700 Subject: [PATCH 1/7] Add deterministic merge conflict actions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ts/packages/agentRpc/src/client.ts | 9 + ts/packages/agentRpc/src/server.ts | 9 + ts/packages/agentRpc/src/types.ts | 3 + ts/packages/agentSdk/src/agentInterface.ts | 6 + .../server/src/clientAgentRegistry.ts | 1 + .../server/test/clientAgentRegistry.spec.ts | 33 + .../github-cli/src/github-cliActionHandler.ts | 217 +++ .../github-cli/src/github-cliSchema.agr | 22 + .../agents/github-cli/src/github-cliSchema.ts | 30 +- .../agents/github-cli/src/mergeConflict.ts | 1312 +++++++++++++++++ .../test/githubCliBuildArgs.spec.ts | 9 + .../test/githubCliReadiness.spec.ts | 16 + .../github-cli/test/mergeConflict.spec.ts | 780 ++++++++++ .../dispatcher/src/execute/actionHandlers.ts | 10 +- .../dispatcher/test/agentReadiness.spec.ts | 15 + 15 files changed, 2470 insertions(+), 2 deletions(-) create mode 100644 ts/packages/agents/github-cli/src/mergeConflict.ts create mode 100644 ts/packages/agents/github-cli/test/mergeConflict.spec.ts diff --git a/ts/packages/agentRpc/src/client.ts b/ts/packages/agentRpc/src/client.ts index 83ab24fa7a..c43b0dc1cf 100644 --- a/ts/packages/agentRpc/src/client.ts +++ b/ts/packages/agentRpc/src/client.ts @@ -829,6 +829,15 @@ export async function createAgentRpcClient( checkReadiness(context: SessionContext) { return rpc.invoke("checkReadiness", getContextParam(context)); }, + getActionReadiness( + action: TypeAgentAction, + context: SessionContext, + ) { + return rpc.invoke("getActionReadiness", { + ...getContextParam(context), + action, + }); + }, setup(context: ActionContext) { return withActionContextAsync(context, (contextParams) => rpc.invoke("setup", { ...contextParams }), diff --git a/ts/packages/agentRpc/src/server.ts b/ts/packages/agentRpc/src/server.ts index 625189c10d..8243a1d6fc 100644 --- a/ts/packages/agentRpc/src/server.ts +++ b/ts/packages/agentRpc/src/server.ts @@ -328,6 +328,15 @@ export function createAgentRpcServer( } return agent.checkReadiness(getSessionContextShim(param)); }, + async getActionReadiness(param) { + if (agent.getActionReadiness === undefined) { + throw new Error("Invalid invocation of getActionReadiness"); + } + return agent.getActionReadiness( + param.action, + getSessionContextShim(param), + ); + }, async setup(param) { if (agent.setup === undefined) { throw new Error("Invalid invocation of setup"); diff --git a/ts/packages/agentRpc/src/types.ts b/ts/packages/agentRpc/src/types.ts index e4632026c6..d9575c68f7 100644 --- a/ts/packages/agentRpc/src/types.ts +++ b/ts/packages/agentRpc/src/types.ts @@ -283,6 +283,9 @@ export type AgentInvokeFunctions = { param: Partial & { schemaName: string }, ): Promise; checkReadiness(param: Partial): Promise; + getActionReadiness( + param: Partial & { action: TypeAgentAction }, + ): Promise; setup( param: Partial, ): Promise; diff --git a/ts/packages/agentSdk/src/agentInterface.ts b/ts/packages/agentSdk/src/agentInterface.ts index 568e40ca8e..9ba4f5b998 100644 --- a/ts/packages/agentSdk/src/agentInterface.ts +++ b/ts/packages/agentSdk/src/agentInterface.ts @@ -154,6 +154,12 @@ export interface AppAgent extends Partial { // checkReadiness should be CHEAP (file-existence / env-var read level). // Expensive probes (network, child processes) belong in `setup`. checkReadiness?(context: SessionContext): Promise; + // Optional per-action override for agents whose actions have different + // runtime dependencies. Return undefined to use the agent-wide readiness. + getActionReadiness?( + action: TypeAgentAction, + context: SessionContext, + ): Promise; // Idempotent setup that brings the agent from `setup-required` to `ready`. // Returns ActionResult so it can use the in-chat yes/no card pattern diff --git a/ts/packages/agentServer/server/src/clientAgentRegistry.ts b/ts/packages/agentServer/server/src/clientAgentRegistry.ts index 9f2a5b26ed..65bb648ee1 100644 --- a/ts/packages/agentServer/server/src/clientAgentRegistry.ts +++ b/ts/packages/agentServer/server/src/clientAgentRegistry.ts @@ -229,6 +229,7 @@ const sessionContextArg: Record = { updateAgentContext: 1, closeAgentContext: 0, checkReadiness: 0, + getActionReadiness: 1, startBackgroundTasks: 0, stopBackgroundTasks: 0, validateWildcardMatch: 1, diff --git a/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts b/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts index 0a56a053c4..8e21237667 100644 --- a/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts +++ b/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts @@ -648,6 +648,39 @@ describe("clientAgentRegistry routing", () => { expect(readinessCalls).toHaveLength(1); }); + test("routes per-action readiness as a read-only session call", async () => { + const registry = createClientAgentRegistry(); + const host = makeHost(); + const calls: string[] = []; + const appAgent: AppAgent = { + async executeAction() { + return undefined; + }, + async getActionReadiness(action) { + calls.push(action.actionName); + return { state: "ready" }; + }, + }; + await register(registry, host, { + instanceId: "a", + connectionId: "conn-a", + appAgent, + }); + + const { context } = makeSessionContext(undefined); + const report = await getMux(registry).getActionReadiness!( + { + schemaName: AGENT_NAME, + actionName: "verifyMergeConflictsResolved", + parameters: {}, + }, + context, + ); + + expect(report).toEqual({ state: "ready" }); + expect(calls).toEqual(["verifyMergeConflictsResolved"]); + }); + // Case 10 test("overlapping requests from different connections route to their own devices", async () => { const registry = createClientAgentRegistry(); diff --git a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts index d8efe97609..c02eea65aa 100644 --- a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts +++ b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts @@ -37,6 +37,14 @@ import { runSetupCommand, whichExists, } from "./setup.js"; +import { + MergePreparationFailure, + MergePreparationSuccess, + MergeVerificationFailure, + MergeVerificationSuccess, + prepareMerge, + verifyMergeConflictsResolved, +} from "./mergeConflict.js"; const execFileAsync = promisify(execFile); @@ -59,6 +67,8 @@ export function instantiate(): AppAgent { initializeAgentContext, executeAction, checkReadiness, + getActionReadiness: async (action) => + getGithubActionReadiness(action.actionName), setup: async (actionContext) => offerInstall( actionContext as ActionContext, @@ -73,6 +83,15 @@ export function instantiate(): AppAgent { }; } +export function getGithubActionReadiness( + actionName: string, +): ReadinessReport | undefined { + return actionName === "resolveMergeConflicts" || + actionName === "verifyMergeConflictsResolved" + ? { state: "ready" } + : undefined; +} + async function initializeAgentContext(): Promise { return { choiceManager: new ChoiceManager(), @@ -425,6 +444,12 @@ export function buildArgs( const p = action.parameters as Record; switch (action.actionName) { + case "resolveMergeConflicts": + case "verifyMergeConflictsResolved": + // This action uses the narrowly scoped local git workflow below, + // never the general-purpose gh argument marshaller. + return undefined; + // ── Auth ── case "authLogin": { const args = ["auth", "login"]; @@ -1907,11 +1932,202 @@ export async function validateAndResolveRepo( }; } +function formatMergeFailure(failure: MergePreparationFailure): string { + const details: string[] = [`**${failure.message}**`]; + if (failure.changedPaths !== undefined) { + details.push( + `Existing paths:\n${failure.changedPaths.map((file) => `- \`${file}\``).join("\n")}`, + ); + } + if (failure.remotes !== undefined) { + details.push( + `Remotes: ${failure.remotes.map((remote) => `\`${remote}\``).join(", ")}`, + ); + } + if (failure.recovery.length > 0) { + details.push( + `Recovery:\n${failure.recovery.map((step) => `- ${step}`).join("\n")}`, + ); + } + return details.join("\n\n"); +} + +export function buildMergeResult( + result: MergePreparationSuccess, +): ActionResultSuccess { + const target = result.target.displayName; + const summary = + result.status === "conflicts" + ? `Merge from ${target} has ${result.conflicts.length} conflict(s).` + : result.status === "ready" + ? `Merge from ${target} is ready for review.` + : `${target} is already incorporated.`; + const conflictLines = result.conflicts.map((conflict) => { + const flags = [ + conflict.kind, + conflict.binary ? "binary" : undefined, + conflict.submodule ? "submodule" : undefined, + ].filter((value): value is string => value !== undefined); + return `- \`${conflict.path}\` (${flags.join(", ")})`; + }); + const blocks: StructuredBlock[] = [ + { kind: "heading", level: 3, text: summary }, + { + kind: "keyValue", + pairs: [ + { label: "Current branch", value: result.currentBranch }, + { label: "Target", value: target }, + { label: "Fetched commit", value: result.target.fetchedCommit }, + { + label: "Merge state", + value: result.mergeInProgress + ? "In progress, not committed" + : "No merge in progress", + }, + ], + }, + ]; + if (conflictLines.length > 0) { + blocks.push({ + kind: "text", + format: "markdown", + text: `**Conflicted files**\n${conflictLines.join("\n")}`, + }); + } + blocks.push({ + kind: "text", + format: "markdown", + text: `**Next steps**\n${result.recovery.map((step) => `- ${step}`).join("\n")}`, + }); + + return { + historyText: JSON.stringify(result), + entities: [], + resultValue: result, + displayContent: createStructuredContent(blocks, { rawData: result }), + }; +} + +async function executeResolveMergeConflicts( + targetBranch: string | undefined, +): Promise { + const result = await prepareMerge(targetBranch); + if (result.status === "blocked") { + return { + error: JSON.stringify(result), + errorCode: result.errorCode, + retryable: result.errorCode !== "mergeFailed", + mayHaveSideEffects: result.mayHaveSideEffects, + errorDisplayContent: { + type: "markdown", + content: formatMergeFailure(result), + }, + }; + } + return buildMergeResult(result); +} + +export function getRequestedMergeTarget(action: { + actionName?: string; + parameters?: { targetBranch?: string }; +}): string | undefined { + return action.parameters?.targetBranch; +} + +function formatVerificationFailure(failure: MergeVerificationFailure): string { + return [ + `**${failure.message}**`, + failure.recovery.length > 0 + ? `Recovery:\n${failure.recovery.map((step) => `- ${step}`).join("\n")}` + : undefined, + ] + .filter((part): part is string => part !== undefined) + .join("\n\n"); +} + +export function buildVerificationResult( + result: MergeVerificationSuccess, +): ActionResultSuccess { + const summary = + result.status === "resolved" + ? "All merge conflicts are resolved and all merge changes are staged." + : result.status === "unresolved" + ? `${result.remainingConflicts.length} merge conflict(s) remain.` + : result.status === "markersRemain" + ? `Conflict markers remain in ${result.markerPaths.length} file(s).` + : `${result.unstagedPaths.length} merge path(s) still have unstaged changes.`; + const details = + result.status === "unresolved" + ? result.remainingConflicts.map( + (conflict) => + `- \`${conflict.path}\` (${conflict.kind}${conflict.binary ? ", binary" : ""}${conflict.submodule ? ", submodule" : ""})`, + ) + : result.status === "markersRemain" + ? result.markerPaths.map((file) => `- \`${file}\``) + : result.unstagedPaths.map((file) => `- \`${file}\``); + const blocks: StructuredBlock[] = [ + { kind: "heading", level: 3, text: summary }, + { + kind: "keyValue", + pairs: [ + { label: "Current branch", value: result.currentBranch }, + { label: "Merge state", value: "In progress, not committed" }, + { + label: "Inspected paths", + value: result.inspectedPaths.length, + }, + ], + }, + ]; + if (details.length > 0) { + blocks.push({ + kind: "text", + format: "markdown", + text: details.join("\n"), + }); + } + blocks.push({ + kind: "text", + format: "markdown", + text: `**Next steps**\n${result.recovery.map((step) => `- ${step}`).join("\n")}`, + }); + return { + historyText: JSON.stringify(result), + entities: [], + resultValue: result, + displayContent: createStructuredContent(blocks, { rawData: result }), + }; +} + +async function executeVerifyMergeConflictsResolved(): Promise { + const result = await verifyMergeConflictsResolved(); + if (result.status === "blocked") { + return { + error: JSON.stringify(result), + errorCode: result.errorCode, + retryable: true, + mayHaveSideEffects: false, + errorDisplayContent: { + type: "markdown", + content: formatVerificationFailure(result), + }, + }; + } + return buildVerificationResult(result); +} + // code-complexity-allow: top-level action dispatch over all github-cli actions async function executeAction( action: TypeAgentAction, context: ActionContext, ): Promise { + if (action.actionName === "resolveMergeConflicts") { + return executeResolveMergeConflicts(getRequestedMergeTarget(action)); + } + if (action.actionName === "verifyMergeConflictsResolved") { + return executeVerifyMergeConflictsResolved(); + } + // Bare-name repo guard — see validateAndResolveRepo. Runs before // buildArgs so we never hand `gh` a malformed --repo value. const validated = await validateAndResolveRepo( @@ -1921,6 +2137,7 @@ async function executeAction( if (validated.kind === "clarify") { return validated.result; } + action = validated.action; const args = buildArgs(action); diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.agr b/ts/packages/agents/github-cli/src/github-cliSchema.agr index c3fbd72312..6808e231ee 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.agr +++ b/ts/packages/agents/github-cli/src/github-cliSchema.agr @@ -479,6 +479,27 @@ } }; + = resolve merge conflicts from $(targetBranch:wildcard) -> { + actionName: "resolveMergeConflicts", + parameters: { + targetBranch + } +} + | bring $(targetBranch:wildcard) into this branch and resolve conflicts -> { + actionName: "resolveMergeConflicts", + parameters: { + targetBranch + } +} + | bring the default branch into this branch and resolve conflicts -> { + actionName: "resolveMergeConflicts", + parameters: {} +} + | merge the default branch into this branch and resolve conflicts -> { + actionName: "resolveMergeConflicts", + parameters: {} +}; + import { GithubCliActions } from "./github-cliSchema.ts"; : GithubCliActions = @@ -515,4 +536,5 @@ import { GithubCliActions } from "./github-cliSchema.ts"; | | | + | | ; diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.ts b/ts/packages/agents/github-cli/src/github-cliSchema.ts index 080dc3a585..bcb16aca49 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.ts +++ b/ts/packages/agents/github-cli/src/github-cliSchema.ts @@ -67,7 +67,9 @@ export type GithubCliActions = | MyPullRequestsAction | IssueAddLabelAction | VariableCreateAction - | DependabotAlertsAction; + | DependabotAlertsAction + | ResolveMergeConflictsAction + | VerifyMergeConflictsResolvedAction; export type AuthLoginAction = { actionName: "authLogin"; @@ -708,3 +710,29 @@ export type DependabotAlertsAction = { state?: string; }; }; + +// Fetch a target branch and prepare a local merge without committing or pushing. +// Use this for requests such as "resolve merge conflicts from main" or "bring +// the default branch into this branch and resolve conflicts". If conflicts +// occur, the result provides the exact unmerged paths for the calling MCP +// client to resolve with its own file tools. +export type ResolveMergeConflictsAction = { + actionName: "resolveMergeConflicts"; + parameters: { + // Branch to merge into the current local branch. A REMOTE/BRANCH value + // disambiguates repositories with multiple remotes. When omitted, use + // the selected remote's configured default branch, then an existing + // main or master branch. + targetBranch?: string; + }; +}; + +// Verify that a merge prepared by resolveMergeConflicts has no unmerged entries +// or conflict markers and that all merge changes are staged. This only inspects +// repository state; it never stages, commits, finalizes, aborts, or pushes. +// +// Example: { actionName: "verifyMergeConflictsResolved", parameters: {} } +export type VerifyMergeConflictsResolvedAction = { + actionName: "verifyMergeConflictsResolved"; + parameters: {}; +}; diff --git a/ts/packages/agents/github-cli/src/mergeConflict.ts b/ts/packages/agents/github-cli/src/mergeConflict.ts new file mode 100644 index 0000000000..88a95b499c --- /dev/null +++ b/ts/packages/agents/github-cli/src/mergeConflict.ts @@ -0,0 +1,1312 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { execFile } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); + +export type GitCommandResult = { + exitCode: number; + stdout: string; + stderr: string; + failureCode?: string | undefined; + timedOut?: boolean | undefined; +}; + +export type GitCommandRunner = ( + args: readonly string[], + cwd?: string, + timeoutMs?: number, +) => Promise; + +export type MergeConflictKind = + | "bothModified" + | "bothAdded" + | "bothDeleted" + | "addedByUs" + | "addedByThem" + | "deletedByUs" + | "deletedByThem" + | "unmerged"; + +export type MergeConflictDetail = { + path: string; + status: string; + kind: MergeConflictKind; + binary: boolean; + submodule: boolean; +}; + +export type MergeTarget = { + remote: string; + branch: string; + displayName: string; + fetchedCommit: string; +}; + +export type MergePreparationSuccess = { + status: "conflicts" | "ready" | "upToDate"; + repositoryRoot: string; + currentBranch: string; + target: MergeTarget; + mergeInProgress: boolean; + conflicts: MergeConflictDetail[]; + recovery: string[]; +}; + +export type MergePreparationFailure = { + status: "blocked"; + errorCode: + | "notRepository" + | "gitUnavailable" + | "detachedHead" + | "branchChanged" + | "operationInProgress" + | "dirtyWorktree" + | "missingRemote" + | "ambiguousRemote" + | "remoteUnavailable" + | "invalidTargetBranch" + | "missingTargetBranch" + | "fetchFailed" + | "mergeFailed"; + message: string; + repositoryRoot?: string; + currentBranch?: string; + changedPaths?: string[]; + operation?: string; + remotes?: string[]; + recovery: string[]; + mayHaveSideEffects: boolean; +}; + +export type MergePreparationResult = + | MergePreparationSuccess + | MergePreparationFailure; + +export type MergeVerificationSuccess = { + status: "resolved" | "unresolved" | "markersRemain" | "unstagedChanges"; + repositoryRoot: string; + currentBranch: string; + mergeInProgress: true; + inspectedPaths: string[]; + remainingConflicts: MergeConflictDetail[]; + markerPaths: string[]; + unstagedPaths: string[]; + recovery: string[]; +}; + +export type MergeVerificationFailure = { + status: "blocked"; + errorCode: + | "notRepository" + | "gitUnavailable" + | "detachedHead" + | "noMergeInProgress" + | "verificationFailed"; + message: string; + repositoryRoot?: string; + currentBranch?: string; + recovery: string[]; + mayHaveSideEffects: false; +}; + +export type MergeVerificationResult = + | MergeVerificationSuccess + | MergeVerificationFailure; + +export type PrepareMergeOptions = { + cwd?: string; + runGit?: GitCommandRunner; + pathExists?: (filePath: string) => boolean; + isBinaryFile?: (filePath: string) => boolean; +}; + +type ResolvedTarget = { + remote: string; + branch: string; + displayName: string; +}; + +const UNMERGED_STATUSES = new Set(["DD", "AU", "UD", "UA", "DU", "AA", "UU"]); +const MUTATING_GIT_TIMEOUT_MS = 10 * 60_000; + +const CONFLICT_KIND_BY_STATUS: Record = { + DD: "bothDeleted", + AU: "addedByUs", + UD: "deletedByThem", + UA: "addedByThem", + DU: "deletedByUs", + AA: "bothAdded", + UU: "bothModified", +}; + +export async function runGitCommand( + args: readonly string[], + cwd = process.cwd(), + timeoutMs = 60_000, +): Promise { + try { + const { stdout, stderr } = await execFileAsync("git", [...args], { + cwd, + encoding: "utf8", + maxBuffer: 4 * 1024 * 1024, + timeout: timeoutMs, + windowsHide: true, + }); + return { + exitCode: 0, + stdout, + stderr, + }; + } catch (error) { + const failure = error as Error & { + code?: number | string; + stdout?: string; + stderr?: string; + }; + return { + exitCode: + typeof failure.code === "number" ? failure.code : Number.NaN, + stdout: String(failure.stdout ?? ""), + stderr: String(failure.stderr ?? failure.message), + failureCode: + typeof failure.code === "string" ? failure.code : undefined, + timedOut: Boolean((failure as Error & { killed?: boolean }).killed), + }; + } +} + +function blocked( + errorCode: MergePreparationFailure["errorCode"], + message: string, + details: Partial = {}, +): MergePreparationFailure { + return { + status: "blocked", + errorCode, + message, + recovery: [], + mayHaveSideEffects: false, + ...details, + }; +} + +function verificationBlocked( + errorCode: MergeVerificationFailure["errorCode"], + message: string, + details: Partial = {}, +): MergeVerificationFailure { + return { + status: "blocked", + errorCode, + message, + recovery: [], + mayHaveSideEffects: false, + ...details, + }; +} + +function splitNullTerminated(output: string): string[] { + return output.split("\0").filter((entry) => entry.length > 0); +} + +function scalarOutput(output: string): string { + return output.trim(); +} + +export function parsePorcelainPaths(output: string): string[] { + const records = splitNullTerminated(output); + const paths: string[] = []; + for (let index = 0; index < records.length; index++) { + const record = records[index]; + if (record.length < 4) { + continue; + } + paths.push(record.slice(3)); + const status = record.slice(0, 2); + if (status.includes("R") || status.includes("C")) { + const originalPath = records[index + 1]; + if (originalPath !== undefined) { + paths.push(originalPath); + index++; + } + } + } + return paths; +} + +export function parseConflictStatuses( + output: string, +): Array<{ path: string; status: string; kind: MergeConflictKind }> { + const records = splitNullTerminated(output); + const conflicts: Array<{ + path: string; + status: string; + kind: MergeConflictKind; + }> = []; + for (let index = 0; index < records.length; index++) { + const record = records[index]; + const status = record.slice(0, 2); + if (UNMERGED_STATUSES.has(status)) { + conflicts.push({ + path: record.slice(3), + status, + kind: CONFLICT_KIND_BY_STATUS[status] ?? "unmerged", + }); + } + if (status.includes("R") || status.includes("C")) { + index++; + } + } + return conflicts; +} + +export function parseSubmodulePaths(output: string): Set { + const submodulePaths = new Set(); + for (const entry of parseConflictIndex(output)) { + if (entry.mode === "160000") { + submodulePaths.add(entry.path); + } + } + return submodulePaths; +} + +type ConflictIndexEntry = { + mode: string; + objectId: string; + path: string; +}; + +function parseConflictIndex(output: string): ConflictIndexEntry[] { + const entries: ConflictIndexEntry[] = []; + for (const record of splitNullTerminated(output)) { + const match = /^(\d{6}) ([0-9a-f]+) [123]\t(.*)$/s.exec(record); + if (match !== null) { + entries.push({ + mode: match[1], + objectId: match[2], + path: match[3], + }); + } + } + return entries; +} + +function parseRemoteDefaultBranch(output: string): string | undefined { + const match = /^ref:\s+refs\/heads\/([^\t\r\n]+)\s+HEAD$/m.exec(output); + return match?.[1]; +} + +type RemoteBranchLookup = + | { status: "found" } + | { status: "missing" } + | { status: "error"; message: string }; + +async function lookupRemoteBranch( + runGit: GitCommandRunner, + root: string, + remote: string, + branch: string, +): Promise { + const result = await runGit( + ["ls-remote", "--exit-code", "--heads", remote, `refs/heads/${branch}`], + root, + ); + if (result.exitCode === 0 && scalarOutput(result.stdout).length > 0) { + return { status: "found" }; + } + if (result.exitCode === 2) { + return { status: "missing" }; + } + return { + status: "error", + message: + result.stderr || + `Unable to inspect branch '${branch}' on remote '${remote}'.`, + }; +} + +async function validateBranchName( + runGit: GitCommandRunner, + root: string, + branch: string, +): Promise { + const result = await runGit(["check-ref-format", "--branch", branch], root); + return result.exitCode === 0; +} + +async function getConfiguredDefaultBranch( + runGit: GitCommandRunner, + root: string, + remote: string, +): Promise< + | { status: "found"; branch: string } + | { status: "missing" } + | { status: "error"; message: string } +> { + const remoteHead = await runGit( + ["ls-remote", "--symref", remote, "HEAD"], + root, + ); + if (remoteHead.exitCode === 0) { + const branch = parseRemoteDefaultBranch(remoteHead.stdout); + if (branch !== undefined) { + return { status: "found", branch }; + } + return { status: "missing" }; + } + return { + status: "error", + message: + remoteHead.stderr || + `Unable to inspect the default branch on remote '${remote}'.`, + }; +} + +async function resolveTarget( + runGit: GitCommandRunner, + root: string, + remotes: string[], + requestedTarget?: string, +): Promise { + const target = requestedTarget?.trim(); + if (target === undefined || target.length === 0) { + if (remotes.length > 1) { + return blocked( + "ambiguousRemote", + "This repository has multiple remotes, so the default target repository is ambiguous.", + { + repositoryRoot: root, + remotes, + recovery: [ + "Retry with an explicit REMOTE/BRANCH target, such as upstream/main.", + "No fetch or merge was attempted.", + ], + }, + ); + } + + const remote = remotes[0]; + const configuredDefault = await getConfiguredDefaultBranch( + runGit, + root, + remote, + ); + if (configuredDefault.status === "error") { + return blocked( + "remoteUnavailable", + `Unable to inspect remote '${remote}' for its default branch.`, + { + repositoryRoot: root, + remotes, + recovery: [ + configuredDefault.message, + "Check network access and remote credentials, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + if (configuredDefault.status === "found") { + return { + remote, + branch: configuredDefault.branch, + displayName: `${remote}/${configuredDefault.branch}`, + }; + } + + for (const fallback of ["main", "master"]) { + const lookup = await lookupRemoteBranch( + runGit, + root, + remote, + fallback, + ); + if (lookup.status === "error") { + return blocked( + "remoteUnavailable", + `Unable to inspect branch '${fallback}' on remote '${remote}'.`, + { + repositoryRoot: root, + remotes, + recovery: [ + lookup.message, + "Check network access and remote credentials, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + if (lookup.status === "found") { + return { + remote, + branch: fallback, + displayName: `${remote}/${fallback}`, + }; + } + } + return blocked( + "missingTargetBranch", + `Remote '${remote}' has no configured default branch and neither main nor master exists.`, + { + repositoryRoot: root, + remotes, + recovery: [ + "Retry with an explicit branch that exists on the remote.", + "No fetch or merge was attempted.", + ], + }, + ); + } + + const explicitRemote = [...remotes] + .sort((left, right) => right.length - left.length) + .find((remote) => target.startsWith(`${remote}/`)); + const branch = + explicitRemote === undefined + ? target + : target.slice(explicitRemote.length + 1); + if (!(await validateBranchName(runGit, root, branch))) { + return blocked( + "invalidTargetBranch", + `'${target}' is not a valid branch name.`, + { + repositoryRoot: root, + recovery: ["Use a valid BRANCH or REMOTE/BRANCH target."], + }, + ); + } + + if (explicitRemote !== undefined) { + const lookup = await lookupRemoteBranch( + runGit, + root, + explicitRemote, + branch, + ); + if (lookup.status === "error") { + return blocked( + "remoteUnavailable", + `Unable to inspect branch '${branch}' on remote '${explicitRemote}'.`, + { + repositoryRoot: root, + remotes, + recovery: [ + lookup.message, + "Check network access and remote credentials, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + if (lookup.status === "missing") { + return blocked( + "missingTargetBranch", + `Branch '${branch}' does not exist on remote '${explicitRemote}'.`, + { + repositoryRoot: root, + remotes, + recovery: [ + "Check the remote and branch names, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + return { + remote: explicitRemote, + branch, + displayName: `${explicitRemote}/${branch}`, + }; + } + + const matchingRemotes: string[] = []; + for (const remote of remotes) { + const lookup = await lookupRemoteBranch(runGit, root, remote, branch); + if (lookup.status === "error") { + return blocked( + "remoteUnavailable", + `Unable to inspect branch '${branch}' on remote '${remote}'.`, + { + repositoryRoot: root, + remotes, + recovery: [ + lookup.message, + "Check network access and remote credentials, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + if (lookup.status === "found") { + matchingRemotes.push(remote); + } + } + if (matchingRemotes.length === 0) { + return blocked( + "missingTargetBranch", + `Branch '${branch}' does not exist on any configured remote.`, + { + repositoryRoot: root, + remotes, + recovery: [ + "Check the branch name or use REMOTE/BRANCH to select a remote.", + "No fetch or merge was attempted.", + ], + }, + ); + } + if (matchingRemotes.length > 1) { + return blocked( + "ambiguousRemote", + `Branch '${branch}' exists on multiple remotes: ${matchingRemotes.join(", ")}.`, + { + repositoryRoot: root, + remotes: matchingRemotes, + recovery: [ + `Retry with one of: ${matchingRemotes.map((remote) => `${remote}/${branch}`).join(", ")}.`, + "No fetch or merge was attempted.", + ], + }, + ); + } + return { + remote: matchingRemotes[0], + branch, + displayName: `${matchingRemotes[0]}/${branch}`, + }; +} + +async function findInProgressOperation( + runGit: GitCommandRunner, + root: string, + pathExists: (filePath: string) => boolean, +): Promise { + const operationPaths: Array<[string, string]> = [ + ["MERGE_HEAD", "merge"], + ["rebase-merge", "rebase"], + ["rebase-apply", "rebase"], + ["CHERRY_PICK_HEAD", "cherry-pick"], + ]; + for (const [gitPath, operation] of operationPaths) { + const result = await runGit(["rev-parse", "--git-path", gitPath], root); + if ( + result.exitCode === 0 && + pathExists(path.resolve(root, scalarOutput(result.stdout))) + ) { + return operation; + } + } + return undefined; +} + +type ConflictReadResult = + | { ok: true; conflicts: MergeConflictDetail[] } + | { ok: false; message: string }; + +function isBinaryFile(filePath: string): boolean { + const stats = fs.lstatSync(filePath); + if (!stats.isFile()) { + return false; + } + const handle = fs.openSync(filePath, "r"); + try { + const prefix = Buffer.alloc(8_000); + const bytesRead = fs.readSync(handle, prefix, 0, prefix.length, 0); + return prefix.subarray(0, bytesRead).includes(0); + } finally { + fs.closeSync(handle); + } +} + +async function readConflicts( + runGit: GitCommandRunner, + root: string, + pathExists: (filePath: string) => boolean, + inspectBinaryFile: (filePath: string) => boolean, +): Promise { + const [status, index] = await Promise.all([ + runGit( + ["status", "--porcelain=v1", "-z", "--untracked-files=no"], + root, + ), + runGit(["ls-files", "-u", "-z"], root), + ]); + if (status.exitCode !== 0 || index.exitCode !== 0) { + return { + ok: false, + message: + status.stderr || + index.stderr || + "Git could not inspect the unmerged index.", + }; + } + + const conflicts = parseConflictStatuses(status.stdout); + const submodulePaths = parseSubmodulePaths(index.stdout); + const binaryPaths = new Set(); + for (const conflict of conflicts) { + if (submodulePaths.has(conflict.path)) { + continue; + } + const absolutePath = path.resolve(root, conflict.path); + if (!pathExists(absolutePath)) { + continue; + } + try { + if (inspectBinaryFile(absolutePath)) { + binaryPaths.add(conflict.path); + } + } catch (error) { + return { + ok: false, + message: + error instanceof Error + ? error.message + : `Unable to inspect conflicted file '${conflict.path}'.`, + }; + } + } + return { + ok: true, + conflicts: conflicts.map((conflict) => ({ + ...conflict, + binary: binaryPaths.has(conflict.path), + submodule: submodulePaths.has(conflict.path), + })), + }; +} + +function reviewRecovery(): string[] { + return [ + "Review the unstaged and staged diffs before finishing the merge.", + "To abandon this merge and restore the pre-merge tree, run: git merge --abort", + "Do not commit or push until the working tree has been reviewed.", + ]; +} + +export async function prepareMerge( + requestedTarget?: string, + options: PrepareMergeOptions = {}, +): Promise { + const runGit = options.runGit ?? runGitCommand; + const cwd = options.cwd ?? process.cwd(); + const pathExists = options.pathExists ?? fs.existsSync; + const inspectBinaryFile = options.isBinaryFile ?? isBinaryFile; + + const rootResult = await runGit(["rev-parse", "--show-toplevel"], cwd); + if (rootResult.failureCode === "ENOENT") { + return blocked( + "gitUnavailable", + "Git is not installed or is not available on PATH.", + { recovery: ["Install Git, then retry."] }, + ); + } + if ( + rootResult.exitCode !== 0 || + scalarOutput(rootResult.stdout).length === 0 + ) { + return blocked( + "notRepository", + "The current working directory is not inside a Git repository.", + { recovery: ["Open a repository working directory and retry."] }, + ); + } + const repositoryRoot = path.resolve(scalarOutput(rootResult.stdout)); + + const branchResult = await runGit( + ["symbolic-ref", "--quiet", "--short", "HEAD"], + repositoryRoot, + ); + if ( + branchResult.exitCode !== 0 || + scalarOutput(branchResult.stdout).length === 0 + ) { + return blocked( + "detachedHead", + "HEAD is detached. A local branch must be checked out before preparing a merge.", + { + repositoryRoot, + recovery: [ + "Check out or create the intended local branch, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + const currentBranch = scalarOutput(branchResult.stdout); + const headResult = await runGit( + ["rev-parse", "--verify", "HEAD^{commit}"], + repositoryRoot, + ); + if ( + headResult.exitCode !== 0 || + scalarOutput(headResult.stdout).length === 0 + ) { + return blocked( + "mergeFailed", + "The current branch does not resolve to a commit.", + { + repositoryRoot, + currentBranch, + recovery: [ + "Create or check out a branch with at least one commit, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + const initialHead = scalarOutput(headResult.stdout); + + const operation = await findInProgressOperation( + runGit, + repositoryRoot, + pathExists, + ); + if (operation !== undefined) { + return blocked( + "operationInProgress", + `A ${operation} operation is already in progress.`, + { + repositoryRoot, + currentBranch, + operation, + recovery: [ + `Continue or abort the existing ${operation} before retrying.`, + "No fetch or new merge was attempted.", + ], + }, + ); + } + + const statusResult = await runGit( + ["status", "--porcelain=v1", "-z", "--untracked-files=all"], + repositoryRoot, + ); + if (statusResult.exitCode !== 0) { + return blocked("mergeFailed", "Unable to inspect the working tree.", { + repositoryRoot, + currentBranch, + recovery: [statusResult.stderr], + }); + } + const changedPaths = parsePorcelainPaths(statusResult.stdout); + if (changedPaths.length > 0) { + return blocked( + "dirtyWorktree", + "The working tree has existing changes. The merge was not started so unrelated edits remain untouched.", + { + repositoryRoot, + currentBranch, + changedPaths, + recovery: [ + "Commit, stash, or otherwise preserve the listed changes, then retry with a clean working tree.", + "No fetch or merge was attempted.", + ], + }, + ); + } + + const remoteResult = await runGit(["remote"], repositoryRoot); + const remotes = scalarOutput(remoteResult.stdout) + .split(/\r?\n/) + .map((remote) => remote.trim()) + .filter((remote) => remote.length > 0); + if (remoteResult.exitCode !== 0 || remotes.length === 0) { + return blocked( + "missingRemote", + "This repository has no configured Git remote.", + { + repositoryRoot, + currentBranch, + recovery: [ + "Configure the intended remote, then retry.", + "No fetch or merge was attempted.", + ], + }, + ); + } + + const resolvedTarget = await resolveTarget( + runGit, + repositoryRoot, + remotes, + requestedTarget, + ); + if ("status" in resolvedTarget) { + return { ...resolvedTarget, currentBranch }; + } + + const temporaryRef = `refs/typeagent/merge-conflict/${randomUUID()}`; + const fetchResult = await runGit( + [ + "fetch", + "--no-tags", + "--no-write-fetch-head", + resolvedTarget.remote, + "--", + `refs/heads/${resolvedTarget.branch}:${temporaryRef}`, + ], + repositoryRoot, + MUTATING_GIT_TIMEOUT_MS, + ); + const cleanupTemporaryRef = async (): Promise => + runGit(["update-ref", "-d", temporaryRef], repositoryRoot); + if (fetchResult.exitCode !== 0) { + const cleanup = await cleanupTemporaryRef(); + return blocked( + "fetchFailed", + `Unable to fetch '${resolvedTarget.displayName}'.`, + { + repositoryRoot, + currentBranch, + recovery: [ + fetchResult.stderr || "Inspect the remote and retry.", + ...(cleanup.exitCode === 0 + ? [] + : [ + `Remove the temporary ref before retrying: git update-ref -d ${temporaryRef}`, + ]), + "No merge was attempted.", + ], + mayHaveSideEffects: true, + }, + ); + } + + const fetchedCommitResult = await runGit( + ["rev-parse", "--verify", `${temporaryRef}^{commit}`], + repositoryRoot, + ); + const cleanup = await cleanupTemporaryRef(); + if ( + fetchedCommitResult.exitCode !== 0 || + scalarOutput(fetchedCommitResult.stdout).length === 0 || + cleanup.exitCode !== 0 + ) { + return blocked( + "fetchFailed", + `Fetch completed but '${resolvedTarget.displayName}' did not resolve to a commit.`, + { + repositoryRoot, + currentBranch, + recovery: [ + fetchedCommitResult.stderr || + "The fetched branch did not resolve to a commit.", + ...(cleanup.exitCode === 0 + ? [] + : [ + `Remove the temporary ref before retrying: git update-ref -d ${temporaryRef}`, + ]), + ], + mayHaveSideEffects: true, + }, + ); + } + const target: MergeTarget = { + ...resolvedTarget, + fetchedCommit: scalarOutput(fetchedCommitResult.stdout), + }; + + const branchBeforeMerge = await runGit( + ["symbolic-ref", "--quiet", "--short", "HEAD"], + repositoryRoot, + ); + const activeBranch = scalarOutput(branchBeforeMerge.stdout); + if (branchBeforeMerge.exitCode !== 0 || activeBranch !== currentBranch) { + return blocked( + "branchChanged", + "The checked-out branch changed while the target was being fetched.", + { + repositoryRoot, + ...(activeBranch.length > 0 + ? { currentBranch: activeBranch } + : {}), + recovery: [ + `Check out '${currentBranch}' with a clean working tree, then retry.`, + "No merge was attempted.", + ], + mayHaveSideEffects: true, + }, + ); + } + const headBeforeMerge = await runGit( + ["rev-parse", "--verify", "HEAD^{commit}"], + repositoryRoot, + ); + if ( + headBeforeMerge.exitCode !== 0 || + scalarOutput(headBeforeMerge.stdout) !== initialHead + ) { + return blocked( + "branchChanged", + "The current branch tip changed while the target was being fetched.", + { + repositoryRoot, + currentBranch, + recovery: [ + "Review the new branch state and retry from a clean working tree.", + "No merge was attempted.", + ], + mayHaveSideEffects: true, + }, + ); + } + const operationBeforeMerge = await findInProgressOperation( + runGit, + repositoryRoot, + pathExists, + ); + if (operationBeforeMerge !== undefined) { + return blocked( + "operationInProgress", + `A ${operationBeforeMerge} operation started while the target was being fetched.`, + { + repositoryRoot, + currentBranch, + operation: operationBeforeMerge, + recovery: [ + `Continue or abort the existing ${operationBeforeMerge} before retrying.`, + "No new merge was attempted.", + ], + mayHaveSideEffects: true, + }, + ); + } + const statusBeforeMerge = await runGit( + ["status", "--porcelain=v1", "-z", "--untracked-files=all"], + repositoryRoot, + ); + if (statusBeforeMerge.exitCode !== 0) { + return blocked("mergeFailed", "Unable to recheck the working tree.", { + repositoryRoot, + currentBranch, + recovery: [statusBeforeMerge.stderr, "No merge was attempted."], + mayHaveSideEffects: true, + }); + } + const newChangedPaths = parsePorcelainPaths(statusBeforeMerge.stdout); + if (newChangedPaths.length > 0) { + return blocked( + "dirtyWorktree", + "The working tree changed while the target was being fetched. The merge was not started.", + { + repositoryRoot, + currentBranch, + changedPaths: newChangedPaths, + recovery: [ + "Preserve the listed changes and retry with a clean working tree.", + "No merge was attempted.", + ], + mayHaveSideEffects: true, + }, + ); + } + + const mergeResult = await runGit( + ["merge", "--no-commit", "--no-ff", "--", target.fetchedCommit], + repositoryRoot, + MUTATING_GIT_TIMEOUT_MS, + ); + const conflictRead = await readConflicts( + runGit, + repositoryRoot, + pathExists, + inspectBinaryFile, + ); + if (!conflictRead.ok) { + const mergeInProgress = await hasMergeHead( + runGit, + repositoryRoot, + pathExists, + ); + return blocked( + "mergeFailed", + "The merge ran, but Git could not inspect its conflict state.", + { + repositoryRoot, + currentBranch, + recovery: [ + conflictRead.message, + ...(mergeInProgress + ? reviewRecovery() + : ["Inspect the repository state before retrying."]), + ], + mayHaveSideEffects: true, + }, + ); + } + if (conflictRead.conflicts.length > 0) { + return { + status: "conflicts", + repositoryRoot, + currentBranch, + target, + mergeInProgress: true, + conflicts: conflictRead.conflicts, + recovery: reviewRecovery(), + }; + } + + const mergeInProgress = await hasMergeHead( + runGit, + repositoryRoot, + pathExists, + ); + if (mergeResult.exitCode !== 0) { + return blocked("mergeFailed", "Git could not prepare the merge.", { + repositoryRoot, + currentBranch, + recovery: [ + mergeResult.stderr || mergeResult.stdout, + ...(mergeResult.timedOut + ? [ + "The merge command timed out. Confirm no Git process is still running; if Git reports an index lock, remove only the repository's stale .git/index.lock before recovery.", + ] + : []), + ...(mergeInProgress + ? reviewRecovery() + : ["Inspect the repository state before retrying."]), + ], + mayHaveSideEffects: true, + }); + } + + return { + status: mergeInProgress ? "ready" : "upToDate", + repositoryRoot, + currentBranch, + target, + mergeInProgress, + conflicts: [], + recovery: mergeInProgress + ? reviewRecovery() + : [ + "The target is already incorporated. No merge commit or push was performed.", + ], + }; +} + +async function hasMergeHead( + runGit: GitCommandRunner, + repositoryRoot: string, + pathExists: (filePath: string) => boolean, +): Promise { + const mergeHeadPath = await runGit( + ["rev-parse", "--git-path", "MERGE_HEAD"], + repositoryRoot, + ); + return ( + mergeHeadPath.exitCode === 0 && + pathExists( + path.resolve(repositoryRoot, scalarOutput(mergeHeadPath.stdout)), + ) + ); +} + +function parseMarkerPaths(output: string): string[] { + const markerPaths = new Set(); + for (const line of output.split(/\r?\n/)) { + const match = /^(.*):\d+: leftover conflict marker$/.exec(line); + if (match !== null) { + markerPaths.add(match[1]); + } + } + return [...markerPaths]; +} + +export async function verifyMergeConflictsResolved( + options: PrepareMergeOptions = {}, +): Promise { + const runGit = options.runGit ?? runGitCommand; + const cwd = options.cwd ?? process.cwd(); + const pathExists = options.pathExists ?? fs.existsSync; + const inspectBinaryFile = options.isBinaryFile ?? isBinaryFile; + + const rootResult = await runGit(["rev-parse", "--show-toplevel"], cwd); + if (rootResult.failureCode === "ENOENT") { + return verificationBlocked( + "gitUnavailable", + "Git is not installed or is not available on PATH.", + { recovery: ["Install Git, then retry."] }, + ); + } + if ( + rootResult.exitCode !== 0 || + scalarOutput(rootResult.stdout).length === 0 + ) { + return verificationBlocked( + "notRepository", + "The current working directory is not inside a Git repository.", + { recovery: ["Open the repository working directory and retry."] }, + ); + } + const repositoryRoot = path.resolve(scalarOutput(rootResult.stdout)); + const branchResult = await runGit( + ["symbolic-ref", "--quiet", "--short", "HEAD"], + repositoryRoot, + ); + if ( + branchResult.exitCode !== 0 || + scalarOutput(branchResult.stdout).length === 0 + ) { + return verificationBlocked( + "detachedHead", + "HEAD is detached, so the prepared merge cannot be verified safely.", + { + repositoryRoot, + recovery: ["Inspect the repository state manually."], + }, + ); + } + const currentBranch = scalarOutput(branchResult.stdout); + + if (!(await hasMergeHead(runGit, repositoryRoot, pathExists))) { + return verificationBlocked( + "noMergeInProgress", + "No merge is in progress. Verification will not guess at a completed or aborted merge.", + { + repositoryRoot, + currentBranch, + recovery: [ + "Run resolveMergeConflicts to prepare a merge, or inspect the repository state manually.", + ], + }, + ); + } + + const conflictRead = await readConflicts( + runGit, + repositoryRoot, + pathExists, + inspectBinaryFile, + ); + if (!conflictRead.ok) { + return verificationBlocked( + "verificationFailed", + "Git could not inspect the merge conflict state.", + { + repositoryRoot, + currentBranch, + recovery: [conflictRead.message, ...reviewRecovery()], + }, + ); + } + + const [changed, unstaged, stagedCheck, unstagedCheck] = await Promise.all([ + runGit(["diff", "--name-only", "-z", "HEAD", "--"], repositoryRoot), + runGit(["diff", "--name-only", "-z", "--"], repositoryRoot), + runGit(["diff", "--cached", "--check", "--"], repositoryRoot), + runGit(["diff", "--check", "--"], repositoryRoot), + ]); + if (changed.exitCode !== 0 || unstaged.exitCode !== 0) { + return verificationBlocked( + "verificationFailed", + "Git could not inspect the prepared merge changes.", + { + repositoryRoot, + currentBranch, + recovery: [ + changed.stderr || + unstaged.stderr || + "Inspect the repository state manually.", + ...reviewRecovery(), + ], + }, + ); + } + if ( + (stagedCheck.exitCode !== 0 && stagedCheck.stderr.length > 0) || + (unstagedCheck.exitCode !== 0 && unstagedCheck.stderr.length > 0) + ) { + return verificationBlocked( + "verificationFailed", + "Git could not check the prepared merge for conflict markers.", + { + repositoryRoot, + currentBranch, + recovery: [ + stagedCheck.stderr || + unstagedCheck.stderr || + "Inspect the repository state manually.", + ...reviewRecovery(), + ], + }, + ); + } + + const inspectedPaths = splitNullTerminated(changed.stdout); + const unstagedPaths = splitNullTerminated(unstaged.stdout); + if (conflictRead.conflicts.length > 0) { + return { + status: "unresolved", + repositoryRoot, + currentBranch, + mergeInProgress: true, + inspectedPaths, + remainingConflicts: conflictRead.conflicts, + markerPaths: [], + unstagedPaths, + recovery: [ + "Resolve and stage every remaining unmerged path, then verify again.", + ...reviewRecovery(), + ], + }; + } + + const markerPaths = [ + ...new Set([ + ...parseMarkerPaths(stagedCheck.stdout), + ...parseMarkerPaths(unstagedCheck.stdout), + ]), + ]; + if (markerPaths.length > 0) { + return { + status: "markersRemain", + repositoryRoot, + currentBranch, + mergeInProgress: true, + inspectedPaths, + remainingConflicts: [], + markerPaths, + unstagedPaths, + recovery: [ + "Remove or intentionally resolve the reported marker lines, stage the affected paths, and verify again.", + ...reviewRecovery(), + ], + }; + } + if (unstagedPaths.length > 0) { + return { + status: "unstagedChanges", + repositoryRoot, + currentBranch, + mergeInProgress: true, + inspectedPaths, + remainingConflicts: [], + markerPaths: [], + unstagedPaths, + recovery: [ + "Review and stage the reported paths before considering the merge resolved.", + ...reviewRecovery(), + ], + }; + } + + return { + status: "resolved", + repositoryRoot, + currentBranch, + mergeInProgress: true, + inspectedPaths, + remainingConflicts: [], + markerPaths: [], + unstagedPaths: [], + recovery: reviewRecovery(), + }; +} diff --git a/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts b/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts index c7d9a801c0..a5e29dadae 100644 --- a/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts +++ b/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts @@ -219,6 +219,15 @@ describe("buildArgs — myPullRequests (cross-repo gh search prs)", () => { expect(joined).toContain("repository"); }); + describe("buildArgs — deterministic local merge actions", () => { + test.each(["resolveMergeConflicts", "verifyMergeConflictsResolved"])( + "keeps %s outside the generic gh execution path", + (actionName) => { + expect(buildArgs(action(actionName, {}))).toBeUndefined(); + }, + ); + }); + test("honors an explicit state and limit", () => { const args = buildArgs( action("myPullRequests", { state: "closed", limit: 5 }), diff --git a/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts b/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts index bddc6d7db4..249333855d 100644 --- a/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts +++ b/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts @@ -17,6 +17,7 @@ import { evaluateGhReadiness, + getGithubActionReadiness, runInstall, validateAndResolveRepo, } from "../src/github-cliActionHandler.js"; @@ -86,6 +87,21 @@ describe("evaluateGhReadiness", () => { }); }); +describe("getGithubActionReadiness", () => { + test.each(["resolveMergeConflicts", "verifyMergeConflictsResolved"])( + "allows the Git-only %s action without gh authentication", + (actionName) => { + expect(getGithubActionReadiness(actionName)).toEqual({ + state: "ready", + }); + }, + ); + + test("uses agent-wide readiness for gh-backed actions", () => { + expect(getGithubActionReadiness("issueList")).toBeUndefined(); + }); +}); + describe("planGhSetupCommand", () => { describe("windows", () => { test("error when winget is missing", () => { diff --git a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts new file mode 100644 index 0000000000..d24c3d573d --- /dev/null +++ b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts @@ -0,0 +1,780 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + parseConflictStatuses, + parsePorcelainPaths, + parseSubmodulePaths, + prepareMerge, + verifyMergeConflictsResolved, +} from "../src/mergeConflict.js"; +import type { + GitCommandResult, + GitCommandRunner, +} from "../src/mergeConflict.js"; +import { + buildMergeResult, + buildVerificationResult, + getRequestedMergeTarget, +} from "../src/github-cliActionHandler.js"; + +const ROOT = process.platform === "win32" ? "C:\\repo" : "/repo"; + +function commandKey(args: readonly string[]): string { + return args.join("\0"); +} + +type RunnerState = { + calls: string[][]; + mergeStarted: boolean; +}; + +type RunnerOverrides = { + branch?: GitCommandResult; + headSequence?: string[]; + initialStatus?: string; + statusSequence?: string[]; + remotes?: string[]; + configuredDefault?: string; + localDefault?: string; + remoteBranches?: Record; + fetch?: GitCommandResult; + fetchedCommit?: GitCommandResult; + merge?: GitCommandResult; + conflictStatus?: string; + conflictInspection?: GitCommandResult; + conflictIndex?: string; + changedPaths?: string; + unstagedPaths?: string; + stagedCheck?: GitCommandResult; + unstagedCheck?: GitCommandResult; +}; + +function ok(stdout = ""): GitCommandResult { + return { exitCode: 0, stdout, stderr: "" }; +} + +function fail(stderr = "failed", exitCode = 1): GitCommandResult { + return { exitCode, stdout: "", stderr }; +} + +function createRunner(overrides: RunnerOverrides = {}): { + runGit: GitCommandRunner; + state: RunnerState; +} { + const state: RunnerState = { calls: [], mergeStarted: false }; + const statusSequence = [...(overrides.statusSequence ?? [])]; + const headSequence = [...(overrides.headSequence ?? [])]; + const remotes = overrides.remotes ?? ["origin"]; + const remoteBranches = overrides.remoteBranches ?? { + origin: ["main", "master", "feature"], + }; + const runGit: GitCommandRunner = async (args) => { + state.calls.push([...args]); + const key = commandKey(args); + if (key === commandKey(["rev-parse", "--show-toplevel"])) { + return ok(ROOT); + } + if ( + key === commandKey(["symbolic-ref", "--quiet", "--short", "HEAD"]) + ) { + return overrides.branch ?? ok("feature/work"); + } + if ( + args[0] === "rev-parse" && + args[1] === "--git-path" && + args[2] !== undefined + ) { + return ok(`.git/${args[2]}`); + } + if (key === commandKey(["rev-parse", "--verify", "HEAD^{commit}"])) { + return ok(headSequence.shift() ?? "fedcba9876543210"); + } + if ( + key === + commandKey([ + "status", + "--porcelain=v1", + "-z", + "--untracked-files=all", + ]) + ) { + return ok(statusSequence.shift() ?? overrides.initialStatus ?? ""); + } + if (key === commandKey(["remote"])) { + return ok(remotes.join("\n")); + } + if ( + args[0] === "symbolic-ref" && + args[1] === "--quiet" && + args[2]?.startsWith("refs/remotes/") + ) { + const remote = args[2].split("/")[2]; + const localDefault = + overrides.localDefault ?? overrides.configuredDefault; + return localDefault === undefined + ? fail() + : ok(`refs/remotes/${remote}/${localDefault}`); + } + if (args[0] === "ls-remote" && args.includes("--symref")) { + return overrides.configuredDefault === undefined + ? ok() + : ok( + `ref: refs/heads/${overrides.configuredDefault}\tHEAD\nabc\tHEAD`, + ); + } + if (args[0] === "check-ref-format") { + const branch = args[2] ?? ""; + return branch.startsWith("-") || branch.includes(" ") + ? fail("invalid branch") + : ok(branch); + } + if (args[0] === "ls-remote" && args.includes("--heads")) { + const remote = args[3]; + const branch = args[4]?.replace("refs/heads/", ""); + return remoteBranches[remote]?.includes(branch) === true + ? ok(`abc\trefs/heads/${branch}`) + : fail("missing"); + } + if (args[0] === "fetch") { + return overrides.fetch ?? ok(); + } + if ( + args[0] === "rev-parse" && + args[1] === "--verify" && + args[2]?.startsWith("refs/typeagent/merge-conflict/") === true && + args[2].endsWith("^{commit}") + ) { + return overrides.fetchedCommit ?? ok("0123456789abcdef"); + } + if (args[0] === "update-ref" && args[1] === "-d") { + return ok(); + } + if (args[0] === "merge") { + state.mergeStarted = true; + return overrides.merge ?? ok(); + } + if ( + key === + commandKey([ + "status", + "--porcelain=v1", + "-z", + "--untracked-files=no", + ]) + ) { + return ( + overrides.conflictInspection ?? + ok(overrides.conflictStatus ?? "") + ); + } + if (key === commandKey(["ls-files", "-u", "-z"])) { + return ok(overrides.conflictIndex ?? ""); + } + if (key === commandKey(["diff", "--name-only", "-z", "HEAD", "--"])) { + return ok(overrides.changedPaths ?? ""); + } + if (key === commandKey(["diff", "--name-only", "-z", "--"])) { + return ok(overrides.unstagedPaths ?? ""); + } + if (key === commandKey(["diff", "--cached", "--check", "--"])) { + return overrides.stagedCheck ?? ok(); + } + if (key === commandKey(["diff", "--check", "--"])) { + return overrides.unstagedCheck ?? ok(); + } + throw new Error(`Unexpected git command: ${args.join(" ")}`); + }; + return { runGit, state }; +} + +function createPathExists( + state: RunnerState, + preExistingOperation?: string, +): (filePath: string) => boolean { + return (filePath) => { + const normalized = filePath.replaceAll("\\", "/"); + if ( + preExistingOperation !== undefined && + normalized.endsWith(`/.git/${preExistingOperation}`) + ) { + return true; + } + return state.mergeStarted && normalized.endsWith("/.git/MERGE_HEAD"); + }; +} + +describe("merge-conflict parsers", () => { + test("parses dirty paths including both sides of a rename", () => { + expect( + parsePorcelainPaths( + " M src/local.ts\0?? notes.txt\0R src/new.ts\0src/old.ts\0", + ), + ).toEqual(["src/local.ts", "notes.txt", "src/new.ts", "src/old.ts"]); + }); + + test("classifies modify, add, and delete conflicts", () => { + expect( + parseConflictStatuses( + "UU src/both.ts\0UD src/theirs-deleted.ts\0DU src/ours-deleted.ts\0AA src/new.ts\0", + ), + ).toEqual([ + { + path: "src/both.ts", + status: "UU", + kind: "bothModified", + }, + { + path: "src/theirs-deleted.ts", + status: "UD", + kind: "deletedByThem", + }, + { + path: "src/ours-deleted.ts", + status: "DU", + kind: "deletedByUs", + }, + { path: "src/new.ts", status: "AA", kind: "bothAdded" }, + ]); + }); + + test("identifies submodule index entries", () => { + expect( + parseSubmodulePaths( + "160000 abcdef 1\tdeps/library\0" + + "100644 abcdef 2\tsrc/file.ts\0", + ).has("deps/library"), + ).toBe(true); + }); +}); + +describe("prepareMerge", () => { + test("prefers the configured remote default branch", async () => { + const { runGit, state } = createRunner({ + configuredDefault: "trunk", + remoteBranches: { origin: ["trunk"] }, + }); + const result = await prepareMerge(undefined, { + cwd: ROOT, + runGit, + pathExists: createPathExists(state), + }); + + expect(result.status).toBe("ready"); + if (result.status !== "blocked") { + expect(result.target.displayName).toBe("origin/trunk"); + } + expect( + state.calls.some( + (args) => + args[0] === "fetch" && + args[1] === "--no-tags" && + args[2] === "--no-write-fetch-head" && + args[3] === "origin" && + args[5]?.startsWith( + "refs/heads/trunk:refs/typeagent/merge-conflict/", + ) === true, + ), + ).toBe(true); + }); + + test("prefers the authoritative remote default over a stale local HEAD", async () => { + const { runGit, state } = createRunner({ + configuredDefault: "main", + localDefault: "master", + remoteBranches: { origin: ["main", "master"] }, + }); + const result = await prepareMerge(undefined, { + cwd: ROOT, + runGit, + pathExists: createPathExists(state), + }); + + expect(result.status).toBe("ready"); + if (result.status !== "blocked") { + expect(result.target.displayName).toBe("origin/main"); + } + expect(state.calls).not.toContainEqual([ + "symbolic-ref", + "--quiet", + "refs/remotes/origin/HEAD", + ]); + }); + + test("falls back to an existing main before master", async () => { + const { runGit, state } = createRunner({ + remoteBranches: { origin: ["main", "master"] }, + }); + const result = await prepareMerge(undefined, { + runGit, + pathExists: createPathExists(state), + }); + + expect(result.status).toBe("ready"); + if (result.status !== "blocked") { + expect(result.target.branch).toBe("main"); + } + expect(state.calls).not.toContainEqual([ + "ls-remote", + "--exit-code", + "--heads", + "origin", + "refs/heads/master", + ]); + }); + + test("does not guess a default when multiple remotes are present", async () => { + const { runGit, state } = createRunner({ + remotes: ["origin", "upstream"], + }); + const result = await prepareMerge(undefined, { + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "ambiguousRemote", + mayHaveSideEffects: false, + remotes: ["origin", "upstream"], + }); + expect(state.calls.some((args) => args[0] === "fetch")).toBe(false); + }); + + test("requires REMOTE/BRANCH when a named branch exists on two remotes", async () => { + const { runGit, state } = createRunner({ + remotes: ["origin", "upstream"], + remoteBranches: { + origin: ["main"], + upstream: ["main"], + }, + }); + const result = await prepareMerge("main", { + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "ambiguousRemote", + remotes: ["origin", "upstream"], + }); + expect(state.calls.some((args) => args[0] === "fetch")).toBe(false); + }); + + test("rejects dirty worktrees before remote inspection or mutation", async () => { + const { runGit, state } = createRunner({ + initialStatus: " M src/local.ts\0?? notes.txt\0", + }); + const result = await prepareMerge("main", { + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "dirtyWorktree", + changedPaths: ["src/local.ts", "notes.txt"], + mayHaveSideEffects: false, + }); + expect(state.calls.some((args) => args[0] === "remote")).toBe(false); + }); + + test("rejects detached HEAD before mutation", async () => { + const detached = createRunner({ branch: fail("detached") }); + await expect( + prepareMerge("main", { + runGit: detached.runGit, + pathExists: createPathExists(detached.state), + }), + ).resolves.toMatchObject({ + status: "blocked", + errorCode: "detachedHead", + }); + }); + + test.each([ + ["MERGE_HEAD", "merge"], + ["rebase-merge", "rebase"], + ["rebase-apply", "rebase"], + ["CHERRY_PICK_HEAD", "cherry-pick"], + ])( + "rejects an existing %s operation before mutation", + async (gitPath, operation) => { + const activeOperation = createRunner(); + await expect( + prepareMerge("main", { + runGit: activeOperation.runGit, + pathExists: createPathExists( + activeOperation.state, + gitPath, + ), + }), + ).resolves.toMatchObject({ + status: "blocked", + errorCode: "operationInProgress", + operation, + }); + expect( + activeOperation.state.calls.some((args) => args[0] === "fetch"), + ).toBe(false); + }, + ); + + test("returns typed conflict details without committing or pushing", async () => { + const { runGit, state } = createRunner({ + merge: fail("Automatic merge failed"), + conflictStatus: + "UU src/text.ts\0UD src/deleted.ts\0UU assets/image.png\0UU deps/lib\0", + conflictIndex: + "100644 aaaaaa 1\tsrc/text.ts\0" + + "100644 bbbbbb 2\tsrc/text.ts\0" + + "100644 cccccc 1\tassets/image.png\0" + + "100644 dddddd 2\tassets/image.png\0" + + "160000 eeeeee 1\tdeps/lib\0", + }); + const result = await prepareMerge("origin/main", { + runGit, + pathExists: (filePath) => + createPathExists(state)(filePath) || + filePath.replaceAll("\\", "/").endsWith("/assets/image.png"), + isBinaryFile: (filePath) => + filePath.replaceAll("\\", "/").endsWith("/assets/image.png"), + }); + + expect(result.status).toBe("conflicts"); + if (result.status === "conflicts") { + expect(result.mergeInProgress).toBe(true); + expect(result.conflicts).toEqual([ + { + path: "src/text.ts", + status: "UU", + kind: "bothModified", + binary: false, + submodule: false, + }, + { + path: "src/deleted.ts", + status: "UD", + kind: "deletedByThem", + binary: false, + submodule: false, + }, + { + path: "assets/image.png", + status: "UU", + kind: "bothModified", + binary: true, + submodule: false, + }, + { + path: "deps/lib", + status: "UU", + kind: "bothModified", + binary: false, + submodule: true, + }, + ]); + } + expect(state.calls).toContainEqual([ + "merge", + "--no-commit", + "--no-ff", + "--", + "0123456789abcdef", + ]); + expect( + state.calls.some((args) => ["commit", "push"].includes(args[0])), + ).toBe(false); + }); + + test("reports fetch failures as retryable pre-merge errors", async () => { + const { runGit, state } = createRunner({ + fetch: fail("authentication failed", 128), + }); + const result = await prepareMerge("main", { + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "fetchFailed", + mayHaveSideEffects: true, + }); + expect(state.calls.some((args) => args[0] === "merge")).toBe(false); + }); + + test("blocks when the worktree changes during fetch", async () => { + const { runGit, state } = createRunner({ + statusSequence: ["", "?? local.txt\0"], + }); + const result = await prepareMerge("main", { + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "dirtyWorktree", + changedPaths: ["local.txt"], + mayHaveSideEffects: true, + }); + expect(state.calls.some((args) => args[0] === "merge")).toBe(false); + }); + + test("blocks when the current branch tip changes during fetch", async () => { + const { runGit, state } = createRunner({ + headSequence: ["aaaaaaaa", "bbbbbbbb"], + }); + const result = await prepareMerge("main", { + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "branchChanged", + mayHaveSideEffects: true, + }); + expect(state.calls.some((args) => args[0] === "merge")).toBe(false); + }); + + test("fails closed when a remote branch cannot be inspected", async () => { + const { runGit, state } = createRunner({ + remotes: ["origin", "upstream"], + remoteBranches: { origin: ["main"] }, + }); + const failingRunner: GitCommandRunner = async ( + args, + cwd, + timeoutMs, + ) => { + if ( + args[0] === "ls-remote" && + args[3] === "upstream" && + args.includes("--heads") + ) { + return fail("authentication failed", 128); + } + return runGit(args, cwd, timeoutMs); + }; + const result = await prepareMerge("main", { + runGit: failingRunner, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "remoteUnavailable", + mayHaveSideEffects: false, + }); + expect(state.calls.some((args) => args[0] === "fetch")).toBe(false); + }); + + test("reports missing Git distinctly from a non-repository", async () => { + const result = await prepareMerge("main", { + runGit: async () => ({ + exitCode: Number.NaN, + stdout: "", + stderr: "spawn git ENOENT", + failureCode: "ENOENT", + }), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "gitUnavailable", + mayHaveSideEffects: false, + }); + }); +}); + +describe("verifyMergeConflictsResolved", () => { + test("reports remaining unmerged paths", async () => { + const { runGit, state } = createRunner({ + conflictStatus: "UU src/file.ts\0", + }); + + state.mergeStarted = true; + const result = await verifyMergeConflictsResolved({ + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "unresolved", + mergeInProgress: true, + remainingConflicts: [ + { + path: "src/file.ts", + status: "UU", + kind: "bothModified", + }, + ], + }); + }); + + test("reports conflict markers after index conflicts are resolved", async () => { + const { runGit, state } = createRunner({ + changedPaths: "src/file.ts\0", + stagedCheck: { + exitCode: 2, + stdout: "src/file.ts:1: leftover conflict marker\n", + stderr: "", + }, + }); + state.mergeStarted = true; + const result = await verifyMergeConflictsResolved({ + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "markersRemain", + markerPaths: ["src/file.ts"], + mergeInProgress: true, + }); + }); + + test("returns resolved while leaving the merge uncommitted", async () => { + const { runGit, state } = createRunner({ + changedPaths: "src/file.ts\0", + }); + state.mergeStarted = true; + const result = await verifyMergeConflictsResolved({ + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "resolved", + mergeInProgress: true, + markerPaths: [], + remainingConflicts: [], + }); + expect( + state.calls.some((args) => + ["add", "commit", "merge --continue", "push"].includes( + args.join(" "), + ), + ), + ).toBe(false); + }); + + test("reports unstaged merge changes instead of claiming resolution", async () => { + const { runGit, state } = createRunner({ + changedPaths: "src/file.ts\0", + unstagedPaths: "src/file.ts\0", + }); + state.mergeStarted = true; + const result = await verifyMergeConflictsResolved({ + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "unstagedChanges", + unstagedPaths: ["src/file.ts"], + }); + }); + + test("fails closed when Git cannot inspect the unmerged index", async () => { + const { runGit, state } = createRunner({ + conflictInspection: fail("index unavailable"), + }); + state.mergeStarted = true; + const result = await verifyMergeConflictsResolved({ + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "blocked", + errorCode: "verificationFailed", + mayHaveSideEffects: false, + }); + }); + + test("accepts a clean prepared merge with no conflict paths", async () => { + const { runGit, state } = createRunner(); + state.mergeStarted = true; + const result = await verifyMergeConflictsResolved({ + runGit, + pathExists: createPathExists(state), + }); + + expect(result).toMatchObject({ + status: "resolved", + inspectedPaths: [], + remainingConflicts: [], + markerPaths: [], + unstagedPaths: [], + }); + }); +}); + +describe("MCP-facing structured results", () => { + test("accepts a grammar action with omitted empty parameters", () => { + expect( + getRequestedMergeTarget({ + actionName: "resolveMergeConflicts", + }), + ).toBeUndefined(); + }); + + test("preparation exposes stable raw data without follow-up actions", () => { + const preparation = { + status: "conflicts" as const, + repositoryRoot: ROOT, + currentBranch: "feature/work", + target: { + remote: "origin", + branch: "main", + displayName: "origin/main", + fetchedCommit: "0123456789abcdef", + }, + mergeInProgress: true, + conflicts: [ + { + path: "src/file.ts", + status: "UU", + kind: "bothModified" as const, + binary: false, + submodule: false, + }, + ], + recovery: ["Review before committing."], + }; + + const result = buildMergeResult(preparation); + expect(result.resultValue).toBe(preparation); + expect(JSON.parse(result.historyText ?? "")).toEqual(preparation); + expect(result.displayContent).toMatchObject({ + rawData: preparation, + }); + expect(result.additionalActions).toBeUndefined(); + }); + + test("verification exposes resolved state without committing or pushing", () => { + const verification = { + status: "resolved" as const, + repositoryRoot: ROOT, + currentBranch: "feature/work", + mergeInProgress: true as const, + inspectedPaths: ["src/file.ts"], + remainingConflicts: [], + markerPaths: [], + unstagedPaths: [], + recovery: ["Review before committing."], + }; + + const result = buildVerificationResult(verification); + expect(result.resultValue).toBe(verification); + expect(JSON.parse(result.historyText ?? "")).toEqual(verification); + expect(result.displayContent).toMatchObject({ + rawData: verification, + }); + expect(result.additionalActions).toBeUndefined(); + }); +}); diff --git a/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts b/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts index b87989eec6..0934c69840 100644 --- a/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts +++ b/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts @@ -23,6 +23,7 @@ import { ParsedCommandParams, ParameterDefinitions, AppAction, + ReadinessReport, } from "@typeagent/agent-sdk"; import type { Span } from "@opentelemetry/api"; import { @@ -116,8 +117,10 @@ export async function checkAgentReady( appAgentName: string, systemContext: CommandHandlerContext, actionContext: ActionContext, + readinessOverride?: ReadinessReport, ): Promise { - const report = systemContext.agents.getReadiness(appAgentName); + const report = + readinessOverride ?? systemContext.agents.getReadiness(appAgentName); if (report.state === "ready") { return undefined; } @@ -291,10 +294,15 @@ async function executeHandlerForActionSpan( let setupResult: ActionResult | undefined; try { + const actionReadiness = await appAgent.getActionReadiness?.( + executableAction.action, + actionContext.sessionContext, + ); setupResult = await checkAgentReady( appAgentName, systemContext, actionContext, + actionReadiness, ); } catch (error) { rethrowIfActionCancelled(error, systemContext); diff --git a/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts b/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts index ed622a878f..8fbffa1881 100644 --- a/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts @@ -378,6 +378,21 @@ describe("checkAgentReady (pre-flight gate)", () => { expect(out).toBeUndefined(); }); + test("honors a ready per-action override", async () => { + const sys = fakeSystemContext({ + readiness: new Map([ + [ + "agentA", + { state: "setup-required", message: "missing tool" }, + ], + ]), + }); + const out = await checkAgentReady("agentA", sys, fakeActionContext(), { + state: "ready", + }); + expect(out).toBeUndefined(); + }); + describe("@config agent setup manual instructions", () => { test("renders setup details as markdown", () => { const display = getManualAgentSetupDisplay("player", { From c6e417e9fea68c057cef64e4cbbec23fc6cef70a Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 18:31:21 -0700 Subject: [PATCH 2/7] Document merge state complexity Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ts/packages/agents/github-cli/src/mergeConflict.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ts/packages/agents/github-cli/src/mergeConflict.ts b/ts/packages/agents/github-cli/src/mergeConflict.ts index 88a95b499c..2649b50383 100644 --- a/ts/packages/agents/github-cli/src/mergeConflict.ts +++ b/ts/packages/agents/github-cli/src/mergeConflict.ts @@ -690,6 +690,7 @@ function reviewRecovery(): string[] { ]; } +// code-complexity-allow: fail-closed Git state machine keeps every mutation guard explicit export async function prepareMerge( requestedTarget?: string, options: PrepareMergeOptions = {}, @@ -1120,6 +1121,7 @@ function parseMarkerPaths(output: string): string[] { return [...markerPaths]; } +// code-complexity-allow: verification reports each distinct unresolved merge state explicitly export async function verifyMergeConflictsResolved( options: PrepareMergeOptions = {}, ): Promise { From f1392981b3ff1d092ef18ecfa43964aa418135c0 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 23:00:44 -0700 Subject: [PATCH 3/7] Simplify merge conflict resolution Use the existing Reasoning action to resolve conflicted files, then verify and create the merge commit without pushing. Remove the per-action readiness framework added by the earlier implementation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ts/packages/agentRpc/src/client.ts | 9 - ts/packages/agentRpc/src/server.ts | 9 - ts/packages/agentRpc/src/types.ts | 3 - ts/packages/agentSdk/src/agentInterface.ts | 6 - .../server/src/clientAgentRegistry.ts | 1 - .../server/test/clientAgentRegistry.spec.ts | 33 - .../github-cli/src/github-cliActionHandler.ts | 251 +-- .../agents/github-cli/src/github-cliSchema.ts | 28 +- .../agents/github-cli/src/mergeConflict.ts | 1551 +++++------------ .../test/githubCliBuildArgs.spec.ts | 2 +- .../test/githubCliReadiness.spec.ts | 16 - .../github-cli/test/mergeConflict.spec.ts | 899 +++------- .../schema/reasoningActionSchema.ts | 3 + .../dispatcher/src/execute/actionHandlers.ts | 10 +- .../dispatcher/src/reasoning/claude.ts | 17 +- .../dispatcher/src/reasoning/copilot.ts | 31 +- .../dispatcher/test/agentReadiness.spec.ts | 15 - 17 files changed, 827 insertions(+), 2057 deletions(-) diff --git a/ts/packages/agentRpc/src/client.ts b/ts/packages/agentRpc/src/client.ts index c43b0dc1cf..83ab24fa7a 100644 --- a/ts/packages/agentRpc/src/client.ts +++ b/ts/packages/agentRpc/src/client.ts @@ -829,15 +829,6 @@ export async function createAgentRpcClient( checkReadiness(context: SessionContext) { return rpc.invoke("checkReadiness", getContextParam(context)); }, - getActionReadiness( - action: TypeAgentAction, - context: SessionContext, - ) { - return rpc.invoke("getActionReadiness", { - ...getContextParam(context), - action, - }); - }, setup(context: ActionContext) { return withActionContextAsync(context, (contextParams) => rpc.invoke("setup", { ...contextParams }), diff --git a/ts/packages/agentRpc/src/server.ts b/ts/packages/agentRpc/src/server.ts index 8243a1d6fc..625189c10d 100644 --- a/ts/packages/agentRpc/src/server.ts +++ b/ts/packages/agentRpc/src/server.ts @@ -328,15 +328,6 @@ export function createAgentRpcServer( } return agent.checkReadiness(getSessionContextShim(param)); }, - async getActionReadiness(param) { - if (agent.getActionReadiness === undefined) { - throw new Error("Invalid invocation of getActionReadiness"); - } - return agent.getActionReadiness( - param.action, - getSessionContextShim(param), - ); - }, async setup(param) { if (agent.setup === undefined) { throw new Error("Invalid invocation of setup"); diff --git a/ts/packages/agentRpc/src/types.ts b/ts/packages/agentRpc/src/types.ts index d9575c68f7..e4632026c6 100644 --- a/ts/packages/agentRpc/src/types.ts +++ b/ts/packages/agentRpc/src/types.ts @@ -283,9 +283,6 @@ export type AgentInvokeFunctions = { param: Partial & { schemaName: string }, ): Promise; checkReadiness(param: Partial): Promise; - getActionReadiness( - param: Partial & { action: TypeAgentAction }, - ): Promise; setup( param: Partial, ): Promise; diff --git a/ts/packages/agentSdk/src/agentInterface.ts b/ts/packages/agentSdk/src/agentInterface.ts index 9ba4f5b998..568e40ca8e 100644 --- a/ts/packages/agentSdk/src/agentInterface.ts +++ b/ts/packages/agentSdk/src/agentInterface.ts @@ -154,12 +154,6 @@ export interface AppAgent extends Partial { // checkReadiness should be CHEAP (file-existence / env-var read level). // Expensive probes (network, child processes) belong in `setup`. checkReadiness?(context: SessionContext): Promise; - // Optional per-action override for agents whose actions have different - // runtime dependencies. Return undefined to use the agent-wide readiness. - getActionReadiness?( - action: TypeAgentAction, - context: SessionContext, - ): Promise; // Idempotent setup that brings the agent from `setup-required` to `ready`. // Returns ActionResult so it can use the in-chat yes/no card pattern diff --git a/ts/packages/agentServer/server/src/clientAgentRegistry.ts b/ts/packages/agentServer/server/src/clientAgentRegistry.ts index 65bb648ee1..9f2a5b26ed 100644 --- a/ts/packages/agentServer/server/src/clientAgentRegistry.ts +++ b/ts/packages/agentServer/server/src/clientAgentRegistry.ts @@ -229,7 +229,6 @@ const sessionContextArg: Record = { updateAgentContext: 1, closeAgentContext: 0, checkReadiness: 0, - getActionReadiness: 1, startBackgroundTasks: 0, stopBackgroundTasks: 0, validateWildcardMatch: 1, diff --git a/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts b/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts index 8e21237667..0a56a053c4 100644 --- a/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts +++ b/ts/packages/agentServer/server/test/clientAgentRegistry.spec.ts @@ -648,39 +648,6 @@ describe("clientAgentRegistry routing", () => { expect(readinessCalls).toHaveLength(1); }); - test("routes per-action readiness as a read-only session call", async () => { - const registry = createClientAgentRegistry(); - const host = makeHost(); - const calls: string[] = []; - const appAgent: AppAgent = { - async executeAction() { - return undefined; - }, - async getActionReadiness(action) { - calls.push(action.actionName); - return { state: "ready" }; - }, - }; - await register(registry, host, { - instanceId: "a", - connectionId: "conn-a", - appAgent, - }); - - const { context } = makeSessionContext(undefined); - const report = await getMux(registry).getActionReadiness!( - { - schemaName: AGENT_NAME, - actionName: "verifyMergeConflictsResolved", - parameters: {}, - }, - context, - ); - - expect(report).toEqual({ state: "ready" }); - expect(calls).toEqual(["verifyMergeConflictsResolved"]); - }); - // Case 10 test("overlapping requests from different connections route to their own devices", async () => { const registry = createClientAgentRegistry(); diff --git a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts index c02eea65aa..fba9872a73 100644 --- a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts +++ b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts @@ -38,12 +38,9 @@ import { whichExists, } from "./setup.js"; import { - MergePreparationFailure, - MergePreparationSuccess, - MergeVerificationFailure, - MergeVerificationSuccess, - prepareMerge, - verifyMergeConflictsResolved, + MergeConflictResult, + completeMergeConflictResolution, + mergeAndCommit, } from "./mergeConflict.js"; const execFileAsync = promisify(execFile); @@ -67,8 +64,6 @@ export function instantiate(): AppAgent { initializeAgentContext, executeAction, checkReadiness, - getActionReadiness: async (action) => - getGithubActionReadiness(action.actionName), setup: async (actionContext) => offerInstall( actionContext as ActionContext, @@ -83,15 +78,6 @@ export function instantiate(): AppAgent { }; } -export function getGithubActionReadiness( - actionName: string, -): ReadinessReport | undefined { - return actionName === "resolveMergeConflicts" || - actionName === "verifyMergeConflictsResolved" - ? { state: "ready" } - : undefined; -} - async function initializeAgentContext(): Promise { return { choiceManager: new ChoiceManager(), @@ -445,7 +431,7 @@ export function buildArgs( switch (action.actionName) { case "resolveMergeConflicts": - case "verifyMergeConflictsResolved": + case "completeMergeConflictResolution": // This action uses the narrowly scoped local git workflow below, // never the general-purpose gh argument marshaller. return undefined; @@ -1932,101 +1918,6 @@ export async function validateAndResolveRepo( }; } -function formatMergeFailure(failure: MergePreparationFailure): string { - const details: string[] = [`**${failure.message}**`]; - if (failure.changedPaths !== undefined) { - details.push( - `Existing paths:\n${failure.changedPaths.map((file) => `- \`${file}\``).join("\n")}`, - ); - } - if (failure.remotes !== undefined) { - details.push( - `Remotes: ${failure.remotes.map((remote) => `\`${remote}\``).join(", ")}`, - ); - } - if (failure.recovery.length > 0) { - details.push( - `Recovery:\n${failure.recovery.map((step) => `- ${step}`).join("\n")}`, - ); - } - return details.join("\n\n"); -} - -export function buildMergeResult( - result: MergePreparationSuccess, -): ActionResultSuccess { - const target = result.target.displayName; - const summary = - result.status === "conflicts" - ? `Merge from ${target} has ${result.conflicts.length} conflict(s).` - : result.status === "ready" - ? `Merge from ${target} is ready for review.` - : `${target} is already incorporated.`; - const conflictLines = result.conflicts.map((conflict) => { - const flags = [ - conflict.kind, - conflict.binary ? "binary" : undefined, - conflict.submodule ? "submodule" : undefined, - ].filter((value): value is string => value !== undefined); - return `- \`${conflict.path}\` (${flags.join(", ")})`; - }); - const blocks: StructuredBlock[] = [ - { kind: "heading", level: 3, text: summary }, - { - kind: "keyValue", - pairs: [ - { label: "Current branch", value: result.currentBranch }, - { label: "Target", value: target }, - { label: "Fetched commit", value: result.target.fetchedCommit }, - { - label: "Merge state", - value: result.mergeInProgress - ? "In progress, not committed" - : "No merge in progress", - }, - ], - }, - ]; - if (conflictLines.length > 0) { - blocks.push({ - kind: "text", - format: "markdown", - text: `**Conflicted files**\n${conflictLines.join("\n")}`, - }); - } - blocks.push({ - kind: "text", - format: "markdown", - text: `**Next steps**\n${result.recovery.map((step) => `- ${step}`).join("\n")}`, - }); - - return { - historyText: JSON.stringify(result), - entities: [], - resultValue: result, - displayContent: createStructuredContent(blocks, { rawData: result }), - }; -} - -async function executeResolveMergeConflicts( - targetBranch: string | undefined, -): Promise { - const result = await prepareMerge(targetBranch); - if (result.status === "blocked") { - return { - error: JSON.stringify(result), - errorCode: result.errorCode, - retryable: result.errorCode !== "mergeFailed", - mayHaveSideEffects: result.mayHaveSideEffects, - errorDisplayContent: { - type: "markdown", - content: formatMergeFailure(result), - }, - }; - } - return buildMergeResult(result); -} - export function getRequestedMergeTarget(action: { actionName?: string; parameters?: { targetBranch?: string }; @@ -2034,86 +1925,90 @@ export function getRequestedMergeTarget(action: { return action.parameters?.targetBranch; } -function formatVerificationFailure(failure: MergeVerificationFailure): string { - return [ - `**${failure.message}**`, - failure.recovery.length > 0 - ? `Recovery:\n${failure.recovery.map((step) => `- ${step}`).join("\n")}` - : undefined, - ] - .filter((part): part is string => part !== undefined) - .join("\n\n"); +function buildMergeFailure( + result: Extract, +): ActionResult { + const recovery = + result.recovery === undefined ? "" : `\n\n${result.recovery}`; + return { + error: JSON.stringify(result), + errorCode: result.errorCode, + retryable: !result.mayHaveSideEffects, + mayHaveSideEffects: result.mayHaveSideEffects, + errorDisplayContent: { + type: "markdown", + content: `**${result.message}**${recovery}`, + }, + }; } -export function buildVerificationResult( - result: MergeVerificationSuccess, -): ActionResultSuccess { +export function buildMergeResult(result: MergeConflictResult): ActionResult { + if (result.status === "blocked") { + return buildMergeFailure(result); + } + + const target = result.target?.displayName; const summary = - result.status === "resolved" - ? "All merge conflicts are resolved and all merge changes are staged." - : result.status === "unresolved" - ? `${result.remainingConflicts.length} merge conflict(s) remain.` - : result.status === "markersRemain" - ? `Conflict markers remain in ${result.markerPaths.length} file(s).` - : `${result.unstagedPaths.length} merge path(s) still have unstaged changes.`; - const details = - result.status === "unresolved" - ? result.remainingConflicts.map( - (conflict) => - `- \`${conflict.path}\` (${conflict.kind}${conflict.binary ? ", binary" : ""}${conflict.submodule ? ", submodule" : ""})`, - ) - : result.status === "markersRemain" - ? result.markerPaths.map((file) => `- \`${file}\``) - : result.unstagedPaths.map((file) => `- \`${file}\``); + result.status === "committed" + ? `Created merge commit ${result.commit.slice(0, 12)}${target ? ` from ${target}` : ""}.` + : result.status === "upToDate" + ? `${target} is already incorporated.` + : `Merge from ${target} has ${result.conflicts.length} conflict(s). Reasoning will resolve them.`; const blocks: StructuredBlock[] = [ { kind: "heading", level: 3, text: summary }, - { - kind: "keyValue", - pairs: [ - { label: "Current branch", value: result.currentBranch }, - { label: "Merge state", value: "In progress, not committed" }, - { - label: "Inspected paths", - value: result.inspectedPaths.length, - }, - ], - }, ]; - if (details.length > 0) { + if (result.status === "conflicts") { + blocks.push({ + kind: "text", + format: "markdown", + text: result.conflicts.map((file) => `- \`${file}\``).join("\n"), + }); blocks.push({ kind: "text", format: "markdown", - text: details.join("\n"), + text: `The merge is in progress in \`${result.repositoryRoot}\`. If Reasoning cannot finish it, run \`git -C "${result.repositoryRoot}" merge --abort\`.`, }); } - blocks.push({ - kind: "text", - format: "markdown", - text: `**Next steps**\n${result.recovery.map((step) => `- ${step}`).join("\n")}`, - }); - return { + const actionResult: ActionResultSuccess = { historyText: JSON.stringify(result), entities: [], resultValue: result, displayContent: createStructuredContent(blocks, { rawData: result }), }; -} - -async function executeVerifyMergeConflictsResolved(): Promise { - const result = await verifyMergeConflictsResolved(); - if (result.status === "blocked") { - return { - error: JSON.stringify(result), - errorCode: result.errorCode, - retryable: true, - mayHaveSideEffects: false, - errorDisplayContent: { - type: "markdown", - content: formatVerificationFailure(result), + if (result.status === "conflicts") { + const files = result.conflicts.map((file) => `- ${file}`).join("\n"); + actionResult.additionalActions = [ + { + schemaName: "dispatcher.reasoning", + actionName: "reasoningAction", + parameters: { + originalRequest: + `Resolve the current Git merge conflicts in the repository at ${result.repositoryRoot}. ` + + `Treat every path below as relative to that root, and run every Git command with that exact repository as its working directory:\n${files}\n\n` + + "Inspect both sides and preserve the intent of each change. Edit and stage only these conflicted paths. Do not edit unrelated files, abort the merge, commit, or push. " + + "Stage each resolved path with git add or git rm. When all conflicts are resolved and staged, execute the " + + `\`github-cli.completeMergeConflictResolution\` action with \`{\"repositoryRoot\":${JSON.stringify(result.repositoryRoot)}}\`; it will verify and create the merge commit.`, + reason: "The merge produced file conflicts that require semantic resolution.", + workingDirectory: result.repositoryRoot, + }, }, - }; + ]; } - return buildVerificationResult(result); + return actionResult; +} + +async function executeResolveMergeConflicts( + targetBranch: string | undefined, +): Promise { + return buildMergeResult(await mergeAndCommit(targetBranch)); +} + +async function executeCompleteMergeConflictResolution( + repositoryRoot: string, +): Promise { + return buildMergeResult( + await completeMergeConflictResolution({ cwd: repositoryRoot }), + ); } // code-complexity-allow: top-level action dispatch over all github-cli actions @@ -2124,8 +2019,10 @@ async function executeAction( if (action.actionName === "resolveMergeConflicts") { return executeResolveMergeConflicts(getRequestedMergeTarget(action)); } - if (action.actionName === "verifyMergeConflictsResolved") { - return executeVerifyMergeConflictsResolved(); + if (action.actionName === "completeMergeConflictResolution") { + return executeCompleteMergeConflictResolution( + action.parameters.repositoryRoot, + ); } // Bare-name repo guard — see validateAndResolveRepo. Runs before diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.ts b/ts/packages/agents/github-cli/src/github-cliSchema.ts index bcb16aca49..4c7beaa2b7 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.ts +++ b/ts/packages/agents/github-cli/src/github-cliSchema.ts @@ -69,7 +69,7 @@ export type GithubCliActions = | VariableCreateAction | DependabotAlertsAction | ResolveMergeConflictsAction - | VerifyMergeConflictsResolvedAction; + | CompleteMergeConflictResolutionAction; export type AuthLoginAction = { actionName: "authLogin"; @@ -711,11 +711,11 @@ export type DependabotAlertsAction = { }; }; -// Fetch a target branch and prepare a local merge without committing or pushing. +// Fetch a target branch, merge it into the current branch, and create the merge +// commit. If Git reports conflicts, hand the conflicted files to Reasoning for +// semantic resolution before the deterministic completion action commits. // Use this for requests such as "resolve merge conflicts from main" or "bring -// the default branch into this branch and resolve conflicts". If conflicts -// occur, the result provides the exact unmerged paths for the calling MCP -// client to resolve with its own file tools. +// the default branch into this branch and resolve conflicts". This never pushes. export type ResolveMergeConflictsAction = { actionName: "resolveMergeConflicts"; parameters: { @@ -727,12 +727,14 @@ export type ResolveMergeConflictsAction = { }; }; -// Verify that a merge prepared by resolveMergeConflicts has no unmerged entries -// or conflict markers and that all merge changes are staged. This only inspects -// repository state; it never stages, commits, finalizes, aborts, or pushes. -// -// Example: { actionName: "verifyMergeConflictsResolved", parameters: {} } -export type VerifyMergeConflictsResolvedAction = { - actionName: "verifyMergeConflictsResolved"; - parameters: {}; +// Complete a conflicted merge after Reasoning has resolved and staged every +// conflicted path. This verifies that no unmerged or unstaged paths remain, +// creates the merge commit, and never pushes. Usually invoked by Reasoning +// rather than selected directly from a user request. +export type CompleteMergeConflictResolutionAction = { + actionName: "completeMergeConflictResolution"; + parameters: { + // Absolute root of the repository whose merge should be completed. + repositoryRoot: string; + }; }; diff --git a/ts/packages/agents/github-cli/src/mergeConflict.ts b/ts/packages/agents/github-cli/src/mergeConflict.ts index 2649b50383..f62dbdb5ce 100644 --- a/ts/packages/agents/github-cli/src/mergeConflict.ts +++ b/ts/packages/agents/github-cli/src/mergeConflict.ts @@ -13,159 +13,90 @@ export type GitCommandResult = { exitCode: number; stdout: string; stderr: string; - failureCode?: string | undefined; - timedOut?: boolean | undefined; }; export type GitCommandRunner = ( args: readonly string[], cwd?: string, - timeoutMs?: number, ) => Promise; -export type MergeConflictKind = - | "bothModified" - | "bothAdded" - | "bothDeleted" - | "addedByUs" - | "addedByThem" - | "deletedByUs" - | "deletedByThem" - | "unmerged"; - -export type MergeConflictDetail = { - path: string; - status: string; - kind: MergeConflictKind; - binary: boolean; - submodule: boolean; -}; - export type MergeTarget = { remote: string; branch: string; displayName: string; - fetchedCommit: string; -}; - -export type MergePreparationSuccess = { - status: "conflicts" | "ready" | "upToDate"; - repositoryRoot: string; - currentBranch: string; - target: MergeTarget; - mergeInProgress: boolean; - conflicts: MergeConflictDetail[]; - recovery: string[]; -}; - -export type MergePreparationFailure = { - status: "blocked"; - errorCode: - | "notRepository" - | "gitUnavailable" - | "detachedHead" - | "branchChanged" - | "operationInProgress" - | "dirtyWorktree" - | "missingRemote" - | "ambiguousRemote" - | "remoteUnavailable" - | "invalidTargetBranch" - | "missingTargetBranch" - | "fetchFailed" - | "mergeFailed"; - message: string; - repositoryRoot?: string; - currentBranch?: string; - changedPaths?: string[]; - operation?: string; - remotes?: string[]; - recovery: string[]; - mayHaveSideEffects: boolean; }; -export type MergePreparationResult = - | MergePreparationSuccess - | MergePreparationFailure; - -export type MergeVerificationSuccess = { - status: "resolved" | "unresolved" | "markersRemain" | "unstagedChanges"; - repositoryRoot: string; - currentBranch: string; - mergeInProgress: true; - inspectedPaths: string[]; - remainingConflicts: MergeConflictDetail[]; - markerPaths: string[]; - unstagedPaths: string[]; - recovery: string[]; -}; - -export type MergeVerificationFailure = { - status: "blocked"; - errorCode: - | "notRepository" - | "gitUnavailable" - | "detachedHead" - | "noMergeInProgress" - | "verificationFailed"; - message: string; - repositoryRoot?: string; - currentBranch?: string; - recovery: string[]; - mayHaveSideEffects: false; -}; - -export type MergeVerificationResult = - | MergeVerificationSuccess - | MergeVerificationFailure; - -export type PrepareMergeOptions = { +export type MergeConflictResult = + | { + status: "committed"; + repositoryRoot: string; + currentBranch: string; + target?: MergeTarget; + commit: string; + } + | { + status: "conflicts"; + repositoryRoot: string; + currentBranch: string; + target: MergeTarget; + conflicts: string[]; + } + | { + status: "upToDate"; + repositoryRoot: string; + currentBranch: string; + target: MergeTarget; + } + | { + status: "blocked"; + errorCode: + | "notRepository" + | "detachedHead" + | "dirtyWorktree" + | "operationInProgress" + | "noMergeInProgress" + | "missingRemote" + | "ambiguousRemote" + | "missingTargetBranch" + | "fetchFailed" + | "mergeFailed" + | "unresolvedConflicts" + | "missingResolutionState" + | "unstagedChanges" + | "unrelatedChanges" + | "conflictMarkers" + | "commitFailed"; + message: string; + recovery?: string; + conflicts?: string[]; + mayHaveSideEffects: boolean; + }; + +export type MergeOptions = { cwd?: string; runGit?: GitCommandRunner; pathExists?: (filePath: string) => boolean; - isBinaryFile?: (filePath: string) => boolean; -}; - -type ResolvedTarget = { - remote: string; - branch: string; - displayName: string; -}; - -const UNMERGED_STATUSES = new Set(["DD", "AU", "UD", "UA", "DU", "AA", "UU"]); -const MUTATING_GIT_TIMEOUT_MS = 10 * 60_000; - -const CONFLICT_KIND_BY_STATUS: Record = { - DD: "bothDeleted", - AU: "addedByUs", - UD: "deletedByThem", - UA: "addedByThem", - DU: "deletedByUs", - AA: "bothAdded", - UU: "bothModified", + readFile?: (filePath: string) => string; + writeFile?: (filePath: string, content: string) => void; + removeFile?: (filePath: string) => void; }; export async function runGitCommand( args: readonly string[], cwd = process.cwd(), - timeoutMs = 60_000, ): Promise { try { const { stdout, stderr } = await execFileAsync("git", [...args], { cwd, encoding: "utf8", maxBuffer: 4 * 1024 * 1024, - timeout: timeoutMs, + timeout: 10 * 60_000, windowsHide: true, }); - return { - exitCode: 0, - stdout, - stderr, - }; + return { exitCode: 0, stdout, stderr }; } catch (error) { const failure = error as Error & { - code?: number | string; + code?: number; stdout?: string; stderr?: string; }; @@ -174,1141 +105,519 @@ export async function runGitCommand( typeof failure.code === "number" ? failure.code : Number.NaN, stdout: String(failure.stdout ?? ""), stderr: String(failure.stderr ?? failure.message), - failureCode: - typeof failure.code === "string" ? failure.code : undefined, - timedOut: Boolean((failure as Error & { killed?: boolean }).killed), }; } } function blocked( - errorCode: MergePreparationFailure["errorCode"], + errorCode: Extract["errorCode"], message: string, - details: Partial = {}, -): MergePreparationFailure { + mayHaveSideEffects = false, + details: Partial> = {}, +): MergeConflictResult { return { status: "blocked", errorCode, message, - recovery: [], - mayHaveSideEffects: false, + mayHaveSideEffects, ...details, }; } -function verificationBlocked( - errorCode: MergeVerificationFailure["errorCode"], - message: string, - details: Partial = {}, -): MergeVerificationFailure { - return { - status: "blocked", - errorCode, - message, - recovery: [], - mayHaveSideEffects: false, - ...details, - }; -} - -function splitNullTerminated(output: string): string[] { - return output.split("\0").filter((entry) => entry.length > 0); -} - -function scalarOutput(output: string): string { - return output.trim(); -} - -export function parsePorcelainPaths(output: string): string[] { - const records = splitNullTerminated(output); - const paths: string[] = []; - for (let index = 0; index < records.length; index++) { - const record = records[index]; - if (record.length < 4) { - continue; - } - paths.push(record.slice(3)); - const status = record.slice(0, 2); - if (status.includes("R") || status.includes("C")) { - const originalPath = records[index + 1]; - if (originalPath !== undefined) { - paths.push(originalPath); - index++; - } - } - } - return paths; -} - -export function parseConflictStatuses( - output: string, -): Array<{ path: string; status: string; kind: MergeConflictKind }> { - const records = splitNullTerminated(output); - const conflicts: Array<{ - path: string; - status: string; - kind: MergeConflictKind; - }> = []; - for (let index = 0; index < records.length; index++) { - const record = records[index]; - const status = record.slice(0, 2); - if (UNMERGED_STATUSES.has(status)) { - conflicts.push({ - path: record.slice(3), - status, - kind: CONFLICT_KIND_BY_STATUS[status] ?? "unmerged", - }); - } - if (status.includes("R") || status.includes("C")) { - index++; - } - } - return conflicts; -} - -export function parseSubmodulePaths(output: string): Set { - const submodulePaths = new Set(); - for (const entry of parseConflictIndex(output)) { - if (entry.mode === "160000") { - submodulePaths.add(entry.path); - } - } - return submodulePaths; -} - -type ConflictIndexEntry = { - mode: string; - objectId: string; - path: string; -}; - -function parseConflictIndex(output: string): ConflictIndexEntry[] { - const entries: ConflictIndexEntry[] = []; - for (const record of splitNullTerminated(output)) { - const match = /^(\d{6}) ([0-9a-f]+) [123]\t(.*)$/s.exec(record); - if (match !== null) { - entries.push({ - mode: match[1], - objectId: match[2], - path: match[3], - }); - } - } - return entries; -} - -function parseRemoteDefaultBranch(output: string): string | undefined { - const match = /^ref:\s+refs\/heads\/([^\t\r\n]+)\s+HEAD$/m.exec(output); - return match?.[1]; -} - -type RemoteBranchLookup = - | { status: "found" } - | { status: "missing" } - | { status: "error"; message: string }; - -async function lookupRemoteBranch( - runGit: GitCommandRunner, - root: string, - remote: string, - branch: string, -): Promise { - const result = await runGit( - ["ls-remote", "--exit-code", "--heads", remote, `refs/heads/${branch}`], - root, - ); - if (result.exitCode === 0 && scalarOutput(result.stdout).length > 0) { - return { status: "found" }; - } - if (result.exitCode === 2) { - return { status: "missing" }; - } - return { - status: "error", - message: - result.stderr || - `Unable to inspect branch '${branch}' on remote '${remote}'.`, - }; +function lines(output: string): string[] { + return output + .split(/\r?\n/) + .map((value) => value.trim()) + .filter(Boolean); } -async function validateBranchName( - runGit: GitCommandRunner, - root: string, - branch: string, -): Promise { - const result = await runGit(["check-ref-format", "--branch", branch], root); - return result.exitCode === 0; +function nullSeparated(output: string): string[] { + return output.split("\0").filter(Boolean); } -async function getConfiguredDefaultBranch( +async function getRepository( + cwd: string, runGit: GitCommandRunner, - root: string, - remote: string, ): Promise< - | { status: "found"; branch: string } - | { status: "missing" } - | { status: "error"; message: string } + { repositoryRoot: string; currentBranch: string } | MergeConflictResult > { - const remoteHead = await runGit( - ["ls-remote", "--symref", remote, "HEAD"], - root, - ); - if (remoteHead.exitCode === 0) { - const branch = parseRemoteDefaultBranch(remoteHead.stdout); - if (branch !== undefined) { - return { status: "found", branch }; - } - return { status: "missing" }; - } - return { - status: "error", - message: - remoteHead.stderr || - `Unable to inspect the default branch on remote '${remote}'.`, - }; -} - -async function resolveTarget( - runGit: GitCommandRunner, - root: string, - remotes: string[], - requestedTarget?: string, -): Promise { - const target = requestedTarget?.trim(); - if (target === undefined || target.length === 0) { - if (remotes.length > 1) { - return blocked( - "ambiguousRemote", - "This repository has multiple remotes, so the default target repository is ambiguous.", - { - repositoryRoot: root, - remotes, - recovery: [ - "Retry with an explicit REMOTE/BRANCH target, such as upstream/main.", - "No fetch or merge was attempted.", - ], - }, - ); - } - - const remote = remotes[0]; - const configuredDefault = await getConfiguredDefaultBranch( - runGit, - root, - remote, - ); - if (configuredDefault.status === "error") { - return blocked( - "remoteUnavailable", - `Unable to inspect remote '${remote}' for its default branch.`, - { - repositoryRoot: root, - remotes, - recovery: [ - configuredDefault.message, - "Check network access and remote credentials, then retry.", - "No fetch or merge was attempted.", - ], - }, - ); - } - if (configuredDefault.status === "found") { - return { - remote, - branch: configuredDefault.branch, - displayName: `${remote}/${configuredDefault.branch}`, - }; - } - - for (const fallback of ["main", "master"]) { - const lookup = await lookupRemoteBranch( - runGit, - root, - remote, - fallback, - ); - if (lookup.status === "error") { - return blocked( - "remoteUnavailable", - `Unable to inspect branch '${fallback}' on remote '${remote}'.`, - { - repositoryRoot: root, - remotes, - recovery: [ - lookup.message, - "Check network access and remote credentials, then retry.", - "No fetch or merge was attempted.", - ], - }, - ); - } - if (lookup.status === "found") { - return { - remote, - branch: fallback, - displayName: `${remote}/${fallback}`, - }; - } - } + const root = await runGit(["rev-parse", "--show-toplevel"], cwd); + if (root.exitCode !== 0) { return blocked( - "missingTargetBranch", - `Remote '${remote}' has no configured default branch and neither main nor master exists.`, - { - repositoryRoot: root, - remotes, - recovery: [ - "Retry with an explicit branch that exists on the remote.", - "No fetch or merge was attempted.", - ], - }, - ); - } - - const explicitRemote = [...remotes] - .sort((left, right) => right.length - left.length) - .find((remote) => target.startsWith(`${remote}/`)); - const branch = - explicitRemote === undefined - ? target - : target.slice(explicitRemote.length + 1); - if (!(await validateBranchName(runGit, root, branch))) { - return blocked( - "invalidTargetBranch", - `'${target}' is not a valid branch name.`, - { - repositoryRoot: root, - recovery: ["Use a valid BRANCH or REMOTE/BRANCH target."], - }, - ); - } - - if (explicitRemote !== undefined) { - const lookup = await lookupRemoteBranch( - runGit, - root, - explicitRemote, - branch, - ); - if (lookup.status === "error") { - return blocked( - "remoteUnavailable", - `Unable to inspect branch '${branch}' on remote '${explicitRemote}'.`, - { - repositoryRoot: root, - remotes, - recovery: [ - lookup.message, - "Check network access and remote credentials, then retry.", - "No fetch or merge was attempted.", - ], - }, - ); - } - if (lookup.status === "missing") { - return blocked( - "missingTargetBranch", - `Branch '${branch}' does not exist on remote '${explicitRemote}'.`, - { - repositoryRoot: root, - remotes, - recovery: [ - "Check the remote and branch names, then retry.", - "No fetch or merge was attempted.", - ], - }, - ); - } - return { - remote: explicitRemote, - branch, - displayName: `${explicitRemote}/${branch}`, - }; - } - - const matchingRemotes: string[] = []; - for (const remote of remotes) { - const lookup = await lookupRemoteBranch(runGit, root, remote, branch); - if (lookup.status === "error") { - return blocked( - "remoteUnavailable", - `Unable to inspect branch '${branch}' on remote '${remote}'.`, - { - repositoryRoot: root, - remotes, - recovery: [ - lookup.message, - "Check network access and remote credentials, then retry.", - "No fetch or merge was attempted.", - ], - }, - ); - } - if (lookup.status === "found") { - matchingRemotes.push(remote); - } - } - if (matchingRemotes.length === 0) { - return blocked( - "missingTargetBranch", - `Branch '${branch}' does not exist on any configured remote.`, - { - repositoryRoot: root, - remotes, - recovery: [ - "Check the branch name or use REMOTE/BRANCH to select a remote.", - "No fetch or merge was attempted.", - ], - }, + "notRepository", + "Run this action from a Git repository.", ); } - if (matchingRemotes.length > 1) { + const repositoryRoot = root.stdout.trim(); + const branch = await runGit( + ["symbolic-ref", "--quiet", "--short", "HEAD"], + repositoryRoot, + ); + if (branch.exitCode !== 0) { return blocked( - "ambiguousRemote", - `Branch '${branch}' exists on multiple remotes: ${matchingRemotes.join(", ")}.`, - { - repositoryRoot: root, - remotes: matchingRemotes, - recovery: [ - `Retry with one of: ${matchingRemotes.map((remote) => `${remote}/${branch}`).join(", ")}.`, - "No fetch or merge was attempted.", - ], - }, + "detachedHead", + "Check out a local branch before merging.", ); } - return { - remote: matchingRemotes[0], - branch, - displayName: `${matchingRemotes[0]}/${branch}`, - }; + return { repositoryRoot, currentBranch: branch.stdout.trim() }; } -async function findInProgressOperation( +async function findOperation( + repositoryRoot: string, runGit: GitCommandRunner, - root: string, pathExists: (filePath: string) => boolean, ): Promise { - const operationPaths: Array<[string, string]> = [ + for (const [gitPath, operation] of [ ["MERGE_HEAD", "merge"], ["rebase-merge", "rebase"], ["rebase-apply", "rebase"], ["CHERRY_PICK_HEAD", "cherry-pick"], - ]; - for (const [gitPath, operation] of operationPaths) { - const result = await runGit(["rev-parse", "--git-path", gitPath], root); - if ( - result.exitCode === 0 && - pathExists(path.resolve(root, scalarOutput(result.stdout))) - ) { - return operation; + ["REVERT_HEAD", "revert"], + ]) { + const result = await runGit( + ["rev-parse", "--git-path", gitPath], + repositoryRoot, + ); + if (result.exitCode === 0) { + const resolved = path.resolve(repositoryRoot, result.stdout.trim()); + if (pathExists(resolved)) { + return operation; + } } } return undefined; } -type ConflictReadResult = - | { ok: true; conflicts: MergeConflictDetail[] } - | { ok: false; message: string }; - -function isBinaryFile(filePath: string): boolean { - const stats = fs.lstatSync(filePath); - if (!stats.isFile()) { - return false; +function selectRemote( + remotes: string[], + explicitRemote: string | undefined, +): string | MergeConflictResult { + if (explicitRemote !== undefined) { + return remotes.includes(explicitRemote) + ? explicitRemote + : blocked( + "missingRemote", + `Remote '${explicitRemote}' does not exist.`, + ); } - const handle = fs.openSync(filePath, "r"); - try { - const prefix = Buffer.alloc(8_000); - const bytesRead = fs.readSync(handle, prefix, 0, prefix.length, 0); - return prefix.subarray(0, bytesRead).includes(0); - } finally { - fs.closeSync(handle); + if (remotes.includes("origin")) { + return "origin"; + } + if (remotes.length === 1) { + return remotes[0]; } + if (remotes.length === 0) { + return blocked("missingRemote", "This repository has no Git remote."); + } + return blocked( + "ambiguousRemote", + `Choose a remote explicitly. Available remotes: ${remotes.join(", ")}.`, + ); } -async function readConflicts( +function parseTarget( + targetBranch: string | undefined, + remotes: string[], +): { + remote?: string; + branch?: string; +} { + if (targetBranch === undefined) { + return {}; + } + const slash = targetBranch.indexOf("/"); + return slash > 0 && remotes.includes(targetBranch.slice(0, slash)) + ? { + remote: targetBranch.slice(0, slash), + branch: targetBranch.slice(slash + 1), + } + : { branch: targetBranch }; +} + +async function getDefaultBranch( + repositoryRoot: string, + remote: string, runGit: GitCommandRunner, - root: string, - pathExists: (filePath: string) => boolean, - inspectBinaryFile: (filePath: string) => boolean, -): Promise { - const [status, index] = await Promise.all([ - runGit( - ["status", "--porcelain=v1", "-z", "--untracked-files=no"], - root, - ), - runGit(["ls-files", "-u", "-z"], root), - ]); - if (status.exitCode !== 0 || index.exitCode !== 0) { - return { - ok: false, - message: - status.stderr || - index.stderr || - "Git could not inspect the unmerged index.", - }; +): Promise { + const head = await runGit( + ["ls-remote", "--symref", remote, "HEAD"], + repositoryRoot, + ); + const match = /^ref:\s+refs\/heads\/(.+)\s+HEAD$/m.exec(head.stdout); + if (head.exitCode === 0 && match?.[1]) { + return match[1]; } - - const conflicts = parseConflictStatuses(status.stdout); - const submodulePaths = parseSubmodulePaths(index.stdout); - const binaryPaths = new Set(); - for (const conflict of conflicts) { - if (submodulePaths.has(conflict.path)) { - continue; - } - const absolutePath = path.resolve(root, conflict.path); - if (!pathExists(absolutePath)) { - continue; - } - try { - if (inspectBinaryFile(absolutePath)) { - binaryPaths.add(conflict.path); - } - } catch (error) { - return { - ok: false, - message: - error instanceof Error - ? error.message - : `Unable to inspect conflicted file '${conflict.path}'.`, - }; + for (const fallback of ["main", "master"]) { + const result = await runGit( + ["ls-remote", "--exit-code", "--heads", remote, fallback], + repositoryRoot, + ); + if (result.exitCode === 0 && result.stdout.trim() !== "") { + return fallback; } } - return { - ok: true, - conflicts: conflicts.map((conflict) => ({ - ...conflict, - binary: binaryPaths.has(conflict.path), - submodule: submodulePaths.has(conflict.path), - })), - }; -} - -function reviewRecovery(): string[] { - return [ - "Review the unstaged and staged diffs before finishing the merge.", - "To abandon this merge and restore the pre-merge tree, run: git merge --abort", - "Do not commit or push until the working tree has been reviewed.", - ]; + return undefined; } -// code-complexity-allow: fail-closed Git state machine keeps every mutation guard explicit -export async function prepareMerge( - requestedTarget?: string, - options: PrepareMergeOptions = {}, -): Promise { - const runGit = options.runGit ?? runGitCommand; - const cwd = options.cwd ?? process.cwd(); - const pathExists = options.pathExists ?? fs.existsSync; - const inspectBinaryFile = options.isBinaryFile ?? isBinaryFile; - - const rootResult = await runGit(["rev-parse", "--show-toplevel"], cwd); - if (rootResult.failureCode === "ENOENT") { - return blocked( - "gitUnavailable", - "Git is not installed or is not available on PATH.", - { recovery: ["Install Git, then retry."] }, - ); +async function resolveTarget( + targetBranch: string | undefined, + repositoryRoot: string, + runGit: GitCommandRunner, +): Promise { + const remoteResult = await runGit(["remote"], repositoryRoot); + const remotes = lines(remoteResult.stdout); + const requested = parseTarget(targetBranch?.trim() || undefined, remotes); + const remote = selectRemote(remotes, requested.remote); + if (typeof remote !== "string") { + return remote; } - if ( - rootResult.exitCode !== 0 || - scalarOutput(rootResult.stdout).length === 0 - ) { + const branch = + requested.branch ?? + (await getDefaultBranch(repositoryRoot, remote, runGit)); + if (branch === undefined || branch === "") { return blocked( - "notRepository", - "The current working directory is not inside a Git repository.", - { recovery: ["Open a repository working directory and retry."] }, + "missingTargetBranch", + `Could not determine the default branch for '${remote}'. Specify a target branch.`, ); } - const repositoryRoot = path.resolve(scalarOutput(rootResult.stdout)); - - const branchResult = await runGit( - ["symbolic-ref", "--quiet", "--short", "HEAD"], + const validBranch = await runGit( + ["check-ref-format", "--branch", branch], repositoryRoot, ); - if ( - branchResult.exitCode !== 0 || - scalarOutput(branchResult.stdout).length === 0 - ) { + if (validBranch.exitCode !== 0) { return blocked( - "detachedHead", - "HEAD is detached. A local branch must be checked out before preparing a merge.", - { - repositoryRoot, - recovery: [ - "Check out or create the intended local branch, then retry.", - "No fetch or merge was attempted.", - ], - }, + "missingTargetBranch", + `'${branch}' is not a valid Git branch name.`, ); } - const currentBranch = scalarOutput(branchResult.stdout); - const headResult = await runGit( - ["rev-parse", "--verify", "HEAD^{commit}"], + return { remote, branch, displayName: `${remote}/${branch}` }; +} + +async function listConflicts( + repositoryRoot: string, + runGit: GitCommandRunner, +): Promise { + const result = await runGit( + ["diff", "--name-only", "--diff-filter=U", "-z"], repositoryRoot, ); - if ( - headResult.exitCode !== 0 || - scalarOutput(headResult.stdout).length === 0 - ) { + return result.exitCode === 0 ? nullSeparated(result.stdout) : []; +} + +async function hasMergeHead( + repositoryRoot: string, + runGit: GitCommandRunner, +): Promise { + const result = await runGit( + ["rev-parse", "--verify", "MERGE_HEAD"], + repositoryRoot, + ); + return result.exitCode === 0; +} + +async function getResolutionStatePath( + repositoryRoot: string, + runGit: GitCommandRunner, +): Promise { + const result = await runGit( + ["rev-parse", "--git-path", "TYPEAGENT_MERGE_CONFLICTS"], + repositoryRoot, + ); + return result.exitCode === 0 + ? path.resolve(repositoryRoot, result.stdout.trim()) + : undefined; +} + +async function commitMerge( + repositoryRoot: string, + runGit: GitCommandRunner, +): Promise { + const commit = await runGit(["commit", "--no-edit"], repositoryRoot); + if (commit.exitCode !== 0) { return blocked( - "mergeFailed", - "The current branch does not resolve to a commit.", + "commitFailed", + commit.stderr.trim() || "Git could not create the merge commit.", + true, { - repositoryRoot, - currentBranch, - recovery: [ - "Create or check out a branch with at least one commit, then retry.", - "No fetch or merge was attempted.", - ], + recovery: + "Resolve the error, then run `git commit` or `git merge --abort`.", }, ); } - const initialHead = scalarOutput(headResult.stdout); + const head = await runGit(["rev-parse", "HEAD"], repositoryRoot); + return head.exitCode === 0 ? head.stdout.trim() : ""; +} - const operation = await findInProgressOperation( - runGit, +async function listChangedPaths( + repositoryRoot: string, + runGit: GitCommandRunner, + args: readonly string[], +): Promise { + const result = await runGit([...args, "-z"], repositoryRoot); + return result.exitCode === 0 ? nullSeparated(result.stdout) : undefined; +} + +export async function mergeAndCommit( + targetBranch?: string, + options: MergeOptions = {}, +): Promise { + const cwd = options.cwd ?? process.cwd(); + const runGit = options.runGit ?? runGitCommand; + const pathExists = options.pathExists ?? fs.existsSync; + const writeFile = + options.writeFile ?? + ((filePath, content) => fs.writeFileSync(filePath, content, "utf8")); + const removeFile = + options.removeFile ?? + ((filePath) => fs.rmSync(filePath, { force: true })); + const repository = await getRepository(cwd, runGit); + if ("status" in repository) { + return repository; + } + const { repositoryRoot, currentBranch } = repository; + const resolutionStatePath = await getResolutionStatePath( repositoryRoot, - pathExists, + runGit, ); + const operation = await findOperation(repositoryRoot, runGit, pathExists); if (operation !== undefined) { return blocked( "operationInProgress", - `A ${operation} operation is already in progress.`, - { - repositoryRoot, - currentBranch, - operation, - recovery: [ - `Continue or abort the existing ${operation} before retrying.`, - "No fetch or new merge was attempted.", - ], - }, + `Finish or abort the current ${operation} before starting another merge.`, ); } - - const statusResult = await runGit( + const status = await runGit( ["status", "--porcelain=v1", "-z", "--untracked-files=all"], repositoryRoot, ); - if (statusResult.exitCode !== 0) { - return blocked("mergeFailed", "Unable to inspect the working tree.", { - repositoryRoot, - currentBranch, - recovery: [statusResult.stderr], - }); - } - const changedPaths = parsePorcelainPaths(statusResult.stdout); - if (changedPaths.length > 0) { + if (status.exitCode !== 0 || status.stdout !== "") { return blocked( "dirtyWorktree", - "The working tree has existing changes. The merge was not started so unrelated edits remain untouched.", - { - repositoryRoot, - currentBranch, - changedPaths, - recovery: [ - "Commit, stash, or otherwise preserve the listed changes, then retry with a clean working tree.", - "No fetch or merge was attempted.", - ], - }, + "Commit or stash local changes before merging.", ); } - - const remoteResult = await runGit(["remote"], repositoryRoot); - const remotes = scalarOutput(remoteResult.stdout) - .split(/\r?\n/) - .map((remote) => remote.trim()) - .filter((remote) => remote.length > 0); - if (remoteResult.exitCode !== 0 || remotes.length === 0) { - return blocked( - "missingRemote", - "This repository has no configured Git remote.", - { - repositoryRoot, - currentBranch, - recovery: [ - "Configure the intended remote, then retry.", - "No fetch or merge was attempted.", - ], - }, - ); + if (resolutionStatePath !== undefined && pathExists(resolutionStatePath)) { + removeFile(resolutionStatePath); } - - const resolvedTarget = await resolveTarget( - runGit, - repositoryRoot, - remotes, - requestedTarget, - ); - if ("status" in resolvedTarget) { - return { ...resolvedTarget, currentBranch }; + const target = await resolveTarget(targetBranch, repositoryRoot, runGit); + if ("status" in target) { + return target; } - - const temporaryRef = `refs/typeagent/merge-conflict/${randomUUID()}`; - const fetchResult = await runGit( + const temporaryRef = `refs/typeagent/merge/${randomUUID()}`; + const fetch = await runGit( [ "fetch", "--no-tags", "--no-write-fetch-head", - resolvedTarget.remote, - "--", - `refs/heads/${resolvedTarget.branch}:${temporaryRef}`, + target.remote, + `refs/heads/${target.branch}:${temporaryRef}`, ], repositoryRoot, - MUTATING_GIT_TIMEOUT_MS, ); - const cleanupTemporaryRef = async (): Promise => - runGit(["update-ref", "-d", temporaryRef], repositoryRoot); - if (fetchResult.exitCode !== 0) { - const cleanup = await cleanupTemporaryRef(); + if (fetch.exitCode !== 0) { return blocked( "fetchFailed", - `Unable to fetch '${resolvedTarget.displayName}'.`, - { - repositoryRoot, - currentBranch, - recovery: [ - fetchResult.stderr || "Inspect the remote and retry.", - ...(cleanup.exitCode === 0 - ? [] - : [ - `Remove the temporary ref before retrying: git update-ref -d ${temporaryRef}`, - ]), - "No merge was attempted.", - ], - mayHaveSideEffects: true, - }, + fetch.stderr.trim() || `Could not fetch ${target.displayName}.`, ); } - - const fetchedCommitResult = await runGit( + const fetchedCommit = await runGit( ["rev-parse", "--verify", `${temporaryRef}^{commit}`], repositoryRoot, ); - const cleanup = await cleanupTemporaryRef(); - if ( - fetchedCommitResult.exitCode !== 0 || - scalarOutput(fetchedCommitResult.stdout).length === 0 || - cleanup.exitCode !== 0 - ) { + if (fetchedCommit.exitCode !== 0) { + await runGit(["update-ref", "-d", temporaryRef], repositoryRoot); return blocked( "fetchFailed", - `Fetch completed but '${resolvedTarget.displayName}' did not resolve to a commit.`, - { - repositoryRoot, - currentBranch, - recovery: [ - fetchedCommitResult.stderr || - "The fetched branch did not resolve to a commit.", - ...(cleanup.exitCode === 0 - ? [] - : [ - `Remove the temporary ref before retrying: git update-ref -d ${temporaryRef}`, - ]), - ], - mayHaveSideEffects: true, - }, + `Could not resolve the fetched commit for ${target.displayName}.`, ); } - const target: MergeTarget = { - ...resolvedTarget, - fetchedCommit: scalarOutput(fetchedCommitResult.stdout), - }; - - const branchBeforeMerge = await runGit( - ["symbolic-ref", "--quiet", "--short", "HEAD"], + const merge = await runGit( + ["merge", "--no-commit", "--no-ff", fetchedCommit.stdout.trim()], repositoryRoot, ); - const activeBranch = scalarOutput(branchBeforeMerge.stdout); - if (branchBeforeMerge.exitCode !== 0 || activeBranch !== currentBranch) { - return blocked( - "branchChanged", - "The checked-out branch changed while the target was being fetched.", - { + await runGit(["update-ref", "-d", temporaryRef], repositoryRoot); + if (merge.exitCode !== 0) { + const conflicts = await listConflicts(repositoryRoot, runGit); + if (conflicts.length > 0) { + if (resolutionStatePath === undefined) { + return blocked( + "mergeFailed", + "Git could not create conflict-resolution state.", + true, + { recovery: "Run `git merge --abort`." }, + ); + } + try { + writeFile(resolutionStatePath, JSON.stringify(conflicts)); + } catch (error) { + return blocked( + "mergeFailed", + `Could not save conflict-resolution state: ${String(error)}`, + true, + { recovery: "Run `git merge --abort`." }, + ); + } + return { + status: "conflicts", repositoryRoot, - ...(activeBranch.length > 0 - ? { currentBranch: activeBranch } - : {}), - recovery: [ - `Check out '${currentBranch}' with a clean working tree, then retry.`, - "No merge was attempted.", - ], - mayHaveSideEffects: true, - }, - ); - } - const headBeforeMerge = await runGit( - ["rev-parse", "--verify", "HEAD^{commit}"], - repositoryRoot, - ); - if ( - headBeforeMerge.exitCode !== 0 || - scalarOutput(headBeforeMerge.stdout) !== initialHead - ) { + currentBranch, + target, + conflicts, + }; + } return blocked( - "branchChanged", - "The current branch tip changed while the target was being fetched.", + "mergeFailed", + merge.stderr.trim() || "Git could not merge the target branch.", + true, { - repositoryRoot, - currentBranch, - recovery: [ - "Review the new branch state and retry from a clean working tree.", - "No merge was attempted.", - ], - mayHaveSideEffects: true, + recovery: + "Inspect `git status`, then run `git merge --abort` if needed.", }, ); } - const operationBeforeMerge = await findInProgressOperation( - runGit, + if (!(await hasMergeHead(repositoryRoot, runGit))) { + return { status: "upToDate", repositoryRoot, currentBranch, target }; + } + const commit = await commitMerge(repositoryRoot, runGit); + return typeof commit === "string" + ? { + status: "committed", + repositoryRoot, + currentBranch, + target, + commit, + } + : commit; +} + +export async function completeMergeConflictResolution( + options: MergeOptions = {}, +): Promise { + const cwd = options.cwd ?? process.cwd(); + const runGit = options.runGit ?? runGitCommand; + const pathExists = options.pathExists ?? fs.existsSync; + const readFile = + options.readFile ?? ((filePath) => fs.readFileSync(filePath, "utf8")); + const removeFile = + options.removeFile ?? + ((filePath) => fs.rmSync(filePath, { force: true })); + const repository = await getRepository(cwd, runGit); + if ("status" in repository) { + return repository; + } + const { repositoryRoot, currentBranch } = repository; + const resolutionStatePath = await getResolutionStatePath( repositoryRoot, - pathExists, + runGit, ); - if (operationBeforeMerge !== undefined) { + if (!(await hasMergeHead(repositoryRoot, runGit))) { return blocked( - "operationInProgress", - `A ${operationBeforeMerge} operation started while the target was being fetched.`, - { - repositoryRoot, - currentBranch, - operation: operationBeforeMerge, - recovery: [ - `Continue or abort the existing ${operationBeforeMerge} before retrying.`, - "No new merge was attempted.", - ], - mayHaveSideEffects: true, - }, + "noMergeInProgress", + "There is no merge in progress to complete.", ); } - const statusBeforeMerge = await runGit( - ["status", "--porcelain=v1", "-z", "--untracked-files=all"], - repositoryRoot, - ); - if (statusBeforeMerge.exitCode !== 0) { - return blocked("mergeFailed", "Unable to recheck the working tree.", { - repositoryRoot, - currentBranch, - recovery: [statusBeforeMerge.stderr, "No merge was attempted."], - mayHaveSideEffects: true, - }); - } - const newChangedPaths = parsePorcelainPaths(statusBeforeMerge.stdout); - if (newChangedPaths.length > 0) { + const conflicts = await listConflicts(repositoryRoot, runGit); + if (conflicts.length > 0) { return blocked( - "dirtyWorktree", - "The working tree changed while the target was being fetched. The merge was not started.", - { - repositoryRoot, - currentBranch, - changedPaths: newChangedPaths, - recovery: [ - "Preserve the listed changes and retry with a clean working tree.", - "No merge was attempted.", - ], - mayHaveSideEffects: true, - }, + "unresolvedConflicts", + "Resolve and stage every conflicted file before completing the merge.", + true, + { conflicts }, ); } - - const mergeResult = await runGit( - ["merge", "--no-commit", "--no-ff", "--", target.fetchedCommit], - repositoryRoot, - MUTATING_GIT_TIMEOUT_MS, - ); - const conflictRead = await readConflicts( - runGit, - repositoryRoot, - pathExists, - inspectBinaryFile, - ); - if (!conflictRead.ok) { - const mergeInProgress = await hasMergeHead( - runGit, - repositoryRoot, - pathExists, - ); + if (resolutionStatePath === undefined || !pathExists(resolutionStatePath)) { return blocked( - "mergeFailed", - "The merge ran, but Git could not inspect its conflict state.", - { - repositoryRoot, - currentBranch, - recovery: [ - conflictRead.message, - ...(mergeInProgress - ? reviewRecovery() - : ["Inspect the repository state before retrying."]), - ], - mayHaveSideEffects: true, - }, + "missingResolutionState", + "Conflict-resolution state is missing. Inspect the merge and commit or abort it manually.", + true, ); } - if (conflictRead.conflicts.length > 0) { - return { - status: "conflicts", - repositoryRoot, - currentBranch, - target, - mergeInProgress: true, - conflicts: conflictRead.conflicts, - recovery: reviewRecovery(), - }; - } - - const mergeInProgress = await hasMergeHead( - runGit, - repositoryRoot, - pathExists, - ); - if (mergeResult.exitCode !== 0) { - return blocked("mergeFailed", "Git could not prepare the merge.", { - repositoryRoot, - currentBranch, - recovery: [ - mergeResult.stderr || mergeResult.stdout, - ...(mergeResult.timedOut - ? [ - "The merge command timed out. Confirm no Git process is still running; if Git reports an index lock, remove only the repository's stale .git/index.lock before recovery.", - ] - : []), - ...(mergeInProgress - ? reviewRecovery() - : ["Inspect the repository state before retrying."]), - ], - mayHaveSideEffects: true, - }); - } - - return { - status: mergeInProgress ? "ready" : "upToDate", - repositoryRoot, - currentBranch, - target, - mergeInProgress, - conflicts: [], - recovery: mergeInProgress - ? reviewRecovery() - : [ - "The target is already incorporated. No merge commit or push was performed.", - ], - }; -} - -async function hasMergeHead( - runGit: GitCommandRunner, - repositoryRoot: string, - pathExists: (filePath: string) => boolean, -): Promise { - const mergeHeadPath = await runGit( - ["rev-parse", "--git-path", "MERGE_HEAD"], - repositoryRoot, - ); - return ( - mergeHeadPath.exitCode === 0 && - pathExists( - path.resolve(repositoryRoot, scalarOutput(mergeHeadPath.stdout)), - ) - ); -} - -function parseMarkerPaths(output: string): string[] { - const markerPaths = new Set(); - for (const line of output.split(/\r?\n/)) { - const match = /^(.*):\d+: leftover conflict marker$/.exec(line); - if (match !== null) { - markerPaths.add(match[1]); + let originalConflicts: string[]; + try { + const parsed: unknown = JSON.parse(readFile(resolutionStatePath)); + if ( + !Array.isArray(parsed) || + !parsed.every((value) => typeof value === "string") + ) { + throw new Error("Invalid conflict path list"); } - } - return [...markerPaths]; -} - -// code-complexity-allow: verification reports each distinct unresolved merge state explicitly -export async function verifyMergeConflictsResolved( - options: PrepareMergeOptions = {}, -): Promise { - const runGit = options.runGit ?? runGitCommand; - const cwd = options.cwd ?? process.cwd(); - const pathExists = options.pathExists ?? fs.existsSync; - const inspectBinaryFile = options.isBinaryFile ?? isBinaryFile; - - const rootResult = await runGit(["rev-parse", "--show-toplevel"], cwd); - if (rootResult.failureCode === "ENOENT") { - return verificationBlocked( - "gitUnavailable", - "Git is not installed or is not available on PATH.", - { recovery: ["Install Git, then retry."] }, + originalConflicts = parsed; + } catch { + return blocked( + "missingResolutionState", + "Conflict-resolution state is invalid. Inspect the merge and commit or abort it manually.", + true, ); } - if ( - rootResult.exitCode !== 0 || - scalarOutput(rootResult.stdout).length === 0 - ) { - return verificationBlocked( - "notRepository", - "The current working directory is not inside a Git repository.", - { recovery: ["Open the repository working directory and retry."] }, + const unstaged = await runGit(["diff", "--quiet"], repositoryRoot); + if (unstaged.exitCode !== 0) { + return blocked( + "unstagedChanges", + "Stage the resolved merge changes before completing the merge.", + true, ); } - const repositoryRoot = path.resolve(scalarOutput(rootResult.stdout)); - const branchResult = await runGit( - ["symbolic-ref", "--quiet", "--short", "HEAD"], + const mergeBase = await runGit( + ["merge-base", "HEAD", "MERGE_HEAD"], repositoryRoot, ); - if ( - branchResult.exitCode !== 0 || - scalarOutput(branchResult.stdout).length === 0 - ) { - return verificationBlocked( - "detachedHead", - "HEAD is detached, so the prepared merge cannot be verified safely.", - { - repositoryRoot, - recovery: ["Inspect the repository state manually."], - }, + const allowedPaths = + mergeBase.exitCode === 0 + ? await listChangedPaths(repositoryRoot, runGit, [ + "diff", + "--name-only", + mergeBase.stdout.trim(), + "MERGE_HEAD", + ]) + : undefined; + const stagedPaths = await listChangedPaths(repositoryRoot, runGit, [ + "diff", + "--cached", + "--name-only", + "HEAD", + ]); + if (allowedPaths === undefined || stagedPaths === undefined) { + return blocked( + "unrelatedChanges", + "Git could not verify the staged merge paths.", + true, ); } - const currentBranch = scalarOutput(branchResult.stdout); - - if (!(await hasMergeHead(runGit, repositoryRoot, pathExists))) { - return verificationBlocked( - "noMergeInProgress", - "No merge is in progress. Verification will not guess at a completed or aborted merge.", - { - repositoryRoot, - currentBranch, - recovery: [ - "Run resolveMergeConflicts to prepare a merge, or inspect the repository state manually.", - ], - }, + const allowed = new Set([...allowedPaths, ...originalConflicts]); + const unrelated = stagedPaths.filter((file) => !allowed.has(file)); + if (unrelated.length > 0) { + return blocked( + "unrelatedChanges", + `Unstage changes unrelated to the merge: ${unrelated.join(", ")}.`, + true, ); } - - const conflictRead = await readConflicts( - runGit, + const markerCheck = await runGit( + ["diff", "--cached", "--check"], repositoryRoot, - pathExists, - inspectBinaryFile, ); - if (!conflictRead.ok) { - return verificationBlocked( - "verificationFailed", - "Git could not inspect the merge conflict state.", - { - repositoryRoot, - currentBranch, - recovery: [conflictRead.message, ...reviewRecovery()], - }, - ); - } - - const [changed, unstaged, stagedCheck, unstagedCheck] = await Promise.all([ - runGit(["diff", "--name-only", "-z", "HEAD", "--"], repositoryRoot), - runGit(["diff", "--name-only", "-z", "--"], repositoryRoot), - runGit(["diff", "--cached", "--check", "--"], repositoryRoot), - runGit(["diff", "--check", "--"], repositoryRoot), - ]); - if (changed.exitCode !== 0 || unstaged.exitCode !== 0) { - return verificationBlocked( - "verificationFailed", - "Git could not inspect the prepared merge changes.", - { - repositoryRoot, - currentBranch, - recovery: [ - changed.stderr || - unstaged.stderr || - "Inspect the repository state manually.", - ...reviewRecovery(), - ], - }, - ); - } if ( - (stagedCheck.exitCode !== 0 && stagedCheck.stderr.length > 0) || - (unstagedCheck.exitCode !== 0 && unstagedCheck.stderr.length > 0) + markerCheck.exitCode !== 0 && + `${markerCheck.stdout}\n${markerCheck.stderr}`.includes( + "leftover conflict marker", + ) ) { - return verificationBlocked( - "verificationFailed", - "Git could not check the prepared merge for conflict markers.", - { - repositoryRoot, - currentBranch, - recovery: [ - stagedCheck.stderr || - unstagedCheck.stderr || - "Inspect the repository state manually.", - ...reviewRecovery(), - ], - }, + return blocked( + "conflictMarkers", + "Remove remaining conflict markers before completing the merge.", + true, ); } - - const inspectedPaths = splitNullTerminated(changed.stdout); - const unstagedPaths = splitNullTerminated(unstaged.stdout); - if (conflictRead.conflicts.length > 0) { - return { - status: "unresolved", - repositoryRoot, - currentBranch, - mergeInProgress: true, - inspectedPaths, - remainingConflicts: conflictRead.conflicts, - markerPaths: [], - unstagedPaths, - recovery: [ - "Resolve and stage every remaining unmerged path, then verify again.", - ...reviewRecovery(), - ], - }; - } - - const markerPaths = [ - ...new Set([ - ...parseMarkerPaths(stagedCheck.stdout), - ...parseMarkerPaths(unstagedCheck.stdout), - ]), - ]; - if (markerPaths.length > 0) { - return { - status: "markersRemain", - repositoryRoot, - currentBranch, - mergeInProgress: true, - inspectedPaths, - remainingConflicts: [], - markerPaths, - unstagedPaths, - recovery: [ - "Remove or intentionally resolve the reported marker lines, stage the affected paths, and verify again.", - ...reviewRecovery(), - ], - }; - } - if (unstagedPaths.length > 0) { - return { - status: "unstagedChanges", - repositoryRoot, - currentBranch, - mergeInProgress: true, - inspectedPaths, - remainingConflicts: [], - markerPaths: [], - unstagedPaths, - recovery: [ - "Review and stage the reported paths before considering the merge resolved.", - ...reviewRecovery(), - ], - }; - } - - return { - status: "resolved", - repositoryRoot, - currentBranch, - mergeInProgress: true, - inspectedPaths, - remainingConflicts: [], - markerPaths: [], - unstagedPaths: [], - recovery: reviewRecovery(), - }; + const commit = await commitMerge(repositoryRoot, runGit); + if (typeof commit === "string") { + removeFile(resolutionStatePath); + } + return typeof commit === "string" + ? { + status: "committed", + repositoryRoot, + currentBranch, + commit, + } + : commit; } diff --git a/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts b/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts index a5e29dadae..b2e46aa26a 100644 --- a/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts +++ b/ts/packages/agents/github-cli/test/githubCliBuildArgs.spec.ts @@ -220,7 +220,7 @@ describe("buildArgs — myPullRequests (cross-repo gh search prs)", () => { }); describe("buildArgs — deterministic local merge actions", () => { - test.each(["resolveMergeConflicts", "verifyMergeConflictsResolved"])( + test.each(["resolveMergeConflicts", "completeMergeConflictResolution"])( "keeps %s outside the generic gh execution path", (actionName) => { expect(buildArgs(action(actionName, {}))).toBeUndefined(); diff --git a/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts b/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts index 249333855d..bddc6d7db4 100644 --- a/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts +++ b/ts/packages/agents/github-cli/test/githubCliReadiness.spec.ts @@ -17,7 +17,6 @@ import { evaluateGhReadiness, - getGithubActionReadiness, runInstall, validateAndResolveRepo, } from "../src/github-cliActionHandler.js"; @@ -87,21 +86,6 @@ describe("evaluateGhReadiness", () => { }); }); -describe("getGithubActionReadiness", () => { - test.each(["resolveMergeConflicts", "verifyMergeConflictsResolved"])( - "allows the Git-only %s action without gh authentication", - (actionName) => { - expect(getGithubActionReadiness(actionName)).toEqual({ - state: "ready", - }); - }, - ); - - test("uses agent-wide readiness for gh-backed actions", () => { - expect(getGithubActionReadiness("issueList")).toBeUndefined(); - }); -}); - describe("planGhSetupCommand", () => { describe("windows", () => { test("error when winget is missing", () => { diff --git a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts index d24c3d573d..ee7c62fa33 100644 --- a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts +++ b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts @@ -2,11 +2,8 @@ // Licensed under the MIT License. import { - parseConflictStatuses, - parsePorcelainPaths, - parseSubmodulePaths, - prepareMerge, - verifyMergeConflictsResolved, + completeMergeConflictResolution, + mergeAndCommit, } from "../src/mergeConflict.js"; import type { GitCommandResult, @@ -14,767 +11,399 @@ import type { } from "../src/mergeConflict.js"; import { buildMergeResult, - buildVerificationResult, getRequestedMergeTarget, } from "../src/github-cliActionHandler.js"; const ROOT = process.platform === "win32" ? "C:\\repo" : "/repo"; -function commandKey(args: readonly string[]): string { - return args.join("\0"); +function ok(stdout = ""): GitCommandResult { + return { exitCode: 0, stdout, stderr: "" }; } -type RunnerState = { - calls: string[][]; - mergeStarted: boolean; -}; +function fail(stderr = "failed"): GitCommandResult { + return { exitCode: 1, stdout: "", stderr }; +} -type RunnerOverrides = { - branch?: GitCommandResult; - headSequence?: string[]; - initialStatus?: string; - statusSequence?: string[]; +function key(args: readonly string[]): string { + return args.join("\0"); +} + +type RunnerOptions = { + dirty?: boolean; remotes?: string[]; - configuredDefault?: string; - localDefault?: string; - remoteBranches?: Record; - fetch?: GitCommandResult; - fetchedCommit?: GitCommandResult; + defaultBranch?: string; + mainExists?: boolean; merge?: GitCommandResult; - conflictStatus?: string; - conflictInspection?: GitCommandResult; - conflictIndex?: string; - changedPaths?: string; - unstagedPaths?: string; - stagedCheck?: GitCommandResult; - unstagedCheck?: GitCommandResult; + conflicts?: string[]; + mergeInProgress?: boolean; + unstaged?: boolean; + stagedPaths?: string[]; + allowedPaths?: string[]; + markers?: boolean; + whitespaceErrors?: boolean; + commit?: GitCommandResult; }; -function ok(stdout = ""): GitCommandResult { - return { exitCode: 0, stdout, stderr: "" }; -} - -function fail(stderr = "failed", exitCode = 1): GitCommandResult { - return { exitCode, stdout: "", stderr }; -} - -function createRunner(overrides: RunnerOverrides = {}): { +function createRunner(options: RunnerOptions = {}): { runGit: GitCommandRunner; - state: RunnerState; + calls: string[][]; } { - const state: RunnerState = { calls: [], mergeStarted: false }; - const statusSequence = [...(overrides.statusSequence ?? [])]; - const headSequence = [...(overrides.headSequence ?? [])]; - const remotes = overrides.remotes ?? ["origin"]; - const remoteBranches = overrides.remoteBranches ?? { - origin: ["main", "master", "feature"], - }; + const calls: string[][] = []; const runGit: GitCommandRunner = async (args) => { - state.calls.push([...args]); - const key = commandKey(args); - if (key === commandKey(["rev-parse", "--show-toplevel"])) { - return ok(ROOT); - } - if ( - key === commandKey(["symbolic-ref", "--quiet", "--short", "HEAD"]) - ) { - return overrides.branch ?? ok("feature/work"); - } - if ( - args[0] === "rev-parse" && - args[1] === "--git-path" && - args[2] !== undefined - ) { - return ok(`.git/${args[2]}`); - } - if (key === commandKey(["rev-parse", "--verify", "HEAD^{commit}"])) { - return ok(headSequence.shift() ?? "fedcba9876543210"); - } - if ( - key === - commandKey([ + calls.push([...args]); + switch (key(args)) { + case key(["rev-parse", "--show-toplevel"]): + return ok(ROOT); + case key(["symbolic-ref", "--quiet", "--short", "HEAD"]): + return ok("feature/work"); + case key([ "status", "--porcelain=v1", "-z", "--untracked-files=all", - ]) - ) { - return ok(statusSequence.shift() ?? overrides.initialStatus ?? ""); - } - if (key === commandKey(["remote"])) { - return ok(remotes.join("\n")); - } - if ( - args[0] === "symbolic-ref" && - args[1] === "--quiet" && - args[2]?.startsWith("refs/remotes/") - ) { - const remote = args[2].split("/")[2]; - const localDefault = - overrides.localDefault ?? overrides.configuredDefault; - return localDefault === undefined - ? fail() - : ok(`refs/remotes/${remote}/${localDefault}`); - } - if (args[0] === "ls-remote" && args.includes("--symref")) { - return overrides.configuredDefault === undefined - ? ok() - : ok( - `ref: refs/heads/${overrides.configuredDefault}\tHEAD\nabc\tHEAD`, - ); + ]): + return ok(options.dirty ? " M local.txt\0" : ""); + case key(["remote"]): + return ok((options.remotes ?? ["origin"]).join("\n")); + case key(["ls-remote", "--symref", "origin", "HEAD"]): + return options.defaultBranch === undefined + ? fail() + : ok( + `ref: refs/heads/${options.defaultBranch}\tHEAD\nabc\tHEAD\n`, + ); + case key(["ls-remote", "--exit-code", "--heads", "origin", "main"]): + return options.mainExists + ? ok("abc\trefs/heads/main\n") + : fail(); + case key(["rev-parse", "--verify", "MERGE_HEAD"]): + return options.mergeInProgress === false ? fail() : ok("abc"); + case key(["diff", "--name-only", "--diff-filter=U", "-z"]): + return ok((options.conflicts ?? []).join("\0")); + case key(["diff", "--quiet"]): + return options.unstaged ? fail() : ok(); + case key(["merge-base", "HEAD", "MERGE_HEAD"]): + return ok("base"); + case key(["diff", "--name-only", "base", "MERGE_HEAD", "-z"]): + return ok((options.allowedPaths ?? ["src/a.ts"]).join("\0")); + case key(["diff", "--cached", "--name-only", "HEAD", "-z"]): + return ok((options.stagedPaths ?? ["src/a.ts"]).join("\0")); + case key(["diff", "--cached", "--check"]): + return options.markers + ? fail("leftover conflict marker") + : options.whitespaceErrors + ? fail("trailing whitespace") + : ok(); + case key(["commit", "--no-edit"]): + return options.commit ?? ok(); + case key(["rev-parse", "HEAD"]): + return ok("0123456789abcdef"); + default: + if (args[0] === "rev-parse" && args[1] === "--git-path") { + return ok(`.git/${args[2]}`); + } + if (args[0] === "check-ref-format" && args[1] === "--branch") { + return args[2]?.startsWith("-") ? fail() : ok(args[2]); + } + if (args[0] === "fetch") { + return ok(); + } + if ( + args[0] === "rev-parse" && + args[1] === "--verify" && + args[2]?.startsWith("refs/typeagent/merge/") + ) { + return ok("fetched-commit"); + } + if ( + key(args) === + key(["merge", "--no-commit", "--no-ff", "fetched-commit"]) + ) { + return options.merge ?? ok(); + } + if (args[0] === "update-ref" && args[1] === "-d") { + return ok(); + } + if ( + args[0] === "ls-remote" && + args[1] === "--symref" && + args[2] === "upstream" + ) { + return fail(); + } + return fail(`Unexpected command: ${args.join(" ")}`); } - if (args[0] === "check-ref-format") { - const branch = args[2] ?? ""; - return branch.startsWith("-") || branch.includes(" ") - ? fail("invalid branch") - : ok(branch); - } - if (args[0] === "ls-remote" && args.includes("--heads")) { - const remote = args[3]; - const branch = args[4]?.replace("refs/heads/", ""); - return remoteBranches[remote]?.includes(branch) === true - ? ok(`abc\trefs/heads/${branch}`) - : fail("missing"); - } - if (args[0] === "fetch") { - return overrides.fetch ?? ok(); - } - if ( - args[0] === "rev-parse" && - args[1] === "--verify" && - args[2]?.startsWith("refs/typeagent/merge-conflict/") === true && - args[2].endsWith("^{commit}") - ) { - return overrides.fetchedCommit ?? ok("0123456789abcdef"); - } - if (args[0] === "update-ref" && args[1] === "-d") { - return ok(); - } - if (args[0] === "merge") { - state.mergeStarted = true; - return overrides.merge ?? ok(); - } - if ( - key === - commandKey([ - "status", - "--porcelain=v1", - "-z", - "--untracked-files=no", - ]) - ) { - return ( - overrides.conflictInspection ?? - ok(overrides.conflictStatus ?? "") - ); - } - if (key === commandKey(["ls-files", "-u", "-z"])) { - return ok(overrides.conflictIndex ?? ""); - } - if (key === commandKey(["diff", "--name-only", "-z", "HEAD", "--"])) { - return ok(overrides.changedPaths ?? ""); - } - if (key === commandKey(["diff", "--name-only", "-z", "--"])) { - return ok(overrides.unstagedPaths ?? ""); - } - if (key === commandKey(["diff", "--cached", "--check", "--"])) { - return overrides.stagedCheck ?? ok(); - } - if (key === commandKey(["diff", "--check", "--"])) { - return overrides.unstagedCheck ?? ok(); - } - throw new Error(`Unexpected git command: ${args.join(" ")}`); - }; - return { runGit, state }; -} - -function createPathExists( - state: RunnerState, - preExistingOperation?: string, -): (filePath: string) => boolean { - return (filePath) => { - const normalized = filePath.replaceAll("\\", "/"); - if ( - preExistingOperation !== undefined && - normalized.endsWith(`/.git/${preExistingOperation}`) - ) { - return true; - } - return state.mergeStarted && normalized.endsWith("/.git/MERGE_HEAD"); }; + return { runGit, calls }; } -describe("merge-conflict parsers", () => { - test("parses dirty paths including both sides of a rename", () => { - expect( - parsePorcelainPaths( - " M src/local.ts\0?? notes.txt\0R src/new.ts\0src/old.ts\0", - ), - ).toEqual(["src/local.ts", "notes.txt", "src/new.ts", "src/old.ts"]); - }); - - test("classifies modify, add, and delete conflicts", () => { - expect( - parseConflictStatuses( - "UU src/both.ts\0UD src/theirs-deleted.ts\0DU src/ours-deleted.ts\0AA src/new.ts\0", - ), - ).toEqual([ - { - path: "src/both.ts", - status: "UU", - kind: "bothModified", - }, - { - path: "src/theirs-deleted.ts", - status: "UD", - kind: "deletedByThem", - }, - { - path: "src/ours-deleted.ts", - status: "DU", - kind: "deletedByUs", - }, - { path: "src/new.ts", status: "AA", kind: "bothAdded" }, - ]); - }); - - test("identifies submodule index entries", () => { - expect( - parseSubmodulePaths( - "160000 abcdef 1\tdeps/library\0" + - "100644 abcdef 2\tsrc/file.ts\0", - ).has("deps/library"), - ).toBe(true); - }); -}); +const resolutionState = { + pathExists: (filePath: string) => + filePath.includes("TYPEAGENT_MERGE_CONFLICTS"), + readFile: () => JSON.stringify(["src/a.ts"]), + removeFile: () => {}, +}; -describe("prepareMerge", () => { - test("prefers the configured remote default branch", async () => { - const { runGit, state } = createRunner({ - configuredDefault: "trunk", - remoteBranches: { origin: ["trunk"] }, - }); - const result = await prepareMerge(undefined, { +describe("mergeAndCommit", () => { + test("uses the remote default branch and creates a merge commit", async () => { + const { runGit, calls } = createRunner({ defaultBranch: "main" }); + const result = await mergeAndCommit(undefined, { cwd: ROOT, runGit, - pathExists: createPathExists(state), + pathExists: () => false, }); - expect(result.status).toBe("ready"); - if (result.status !== "blocked") { - expect(result.target.displayName).toBe("origin/trunk"); - } - expect( - state.calls.some( - (args) => - args[0] === "fetch" && - args[1] === "--no-tags" && - args[2] === "--no-write-fetch-head" && - args[3] === "origin" && - args[5]?.startsWith( - "refs/heads/trunk:refs/typeagent/merge-conflict/", - ) === true, - ), - ).toBe(true); - }); - - test("prefers the authoritative remote default over a stale local HEAD", async () => { - const { runGit, state } = createRunner({ - configuredDefault: "main", - localDefault: "master", - remoteBranches: { origin: ["main", "master"] }, - }); - const result = await prepareMerge(undefined, { - cwd: ROOT, - runGit, - pathExists: createPathExists(state), + expect(result).toMatchObject({ + status: "committed", + currentBranch: "feature/work", + target: { displayName: "origin/main" }, + commit: "0123456789abcdef", }); - - expect(result.status).toBe("ready"); - if (result.status !== "blocked") { - expect(result.target.displayName).toBe("origin/main"); - } - expect(state.calls).not.toContainEqual([ - "symbolic-ref", - "--quiet", - "refs/remotes/origin/HEAD", + expect(calls).toContainEqual([ + "merge", + "--no-commit", + "--no-ff", + "fetched-commit", ]); + expect(calls).toContainEqual(["commit", "--no-edit"]); + expect(calls.some(([command]) => command === "push")).toBe(false); }); - test("falls back to an existing main before master", async () => { - const { runGit, state } = createRunner({ - remoteBranches: { origin: ["main", "master"] }, - }); - const result = await prepareMerge(undefined, { + test("falls back to an existing main branch", async () => { + const { runGit, calls } = createRunner({ mainExists: true }); + const result = await mergeAndCommit(undefined, { + cwd: ROOT, runGit, - pathExists: createPathExists(state), + pathExists: () => false, }); - expect(result.status).toBe("ready"); - if (result.status !== "blocked") { - expect(result.target.branch).toBe("main"); - } - expect(state.calls).not.toContainEqual([ + expect(result).toMatchObject({ + status: "committed", + target: { displayName: "origin/main" }, + }); + expect(calls).toContainEqual([ "ls-remote", "--exit-code", "--heads", "origin", - "refs/heads/master", + "main", ]); }); - test("does not guess a default when multiple remotes are present", async () => { - const { runGit, state } = createRunner({ + test("supports an explicit remote and branch", async () => { + const { runGit, calls } = createRunner({ remotes: ["origin", "upstream"], }); - const result = await prepareMerge(undefined, { - runGit, - pathExists: createPathExists(state), - }); - - expect(result).toMatchObject({ - status: "blocked", - errorCode: "ambiguousRemote", - mayHaveSideEffects: false, - remotes: ["origin", "upstream"], - }); - expect(state.calls.some((args) => args[0] === "fetch")).toBe(false); - }); - - test("requires REMOTE/BRANCH when a named branch exists on two remotes", async () => { - const { runGit, state } = createRunner({ - remotes: ["origin", "upstream"], - remoteBranches: { - origin: ["main"], - upstream: ["main"], - }, - }); - const result = await prepareMerge("main", { + const result = await mergeAndCommit("upstream/release/2.0", { + cwd: ROOT, runGit, - pathExists: createPathExists(state), + pathExists: () => false, }); expect(result).toMatchObject({ - status: "blocked", - errorCode: "ambiguousRemote", - remotes: ["origin", "upstream"], + status: "committed", + target: { displayName: "upstream/release/2.0" }, }); - expect(state.calls.some((args) => args[0] === "fetch")).toBe(false); + expect( + calls.some( + ([command, noTags, noFetchHead, remote, refspec]) => + command === "fetch" && + noTags === "--no-tags" && + noFetchHead === "--no-write-fetch-head" && + remote === "upstream" && + refspec?.startsWith("refs/heads/release/2.0:"), + ), + ).toBe(true); }); - test("rejects dirty worktrees before remote inspection or mutation", async () => { - const { runGit, state } = createRunner({ - initialStatus: " M src/local.ts\0?? notes.txt\0", - }); - const result = await prepareMerge("main", { + test("does not mutate a dirty worktree", async () => { + const { runGit, calls } = createRunner({ dirty: true }); + const result = await mergeAndCommit("main", { + cwd: ROOT, runGit, - pathExists: createPathExists(state), + pathExists: () => false, }); expect(result).toMatchObject({ status: "blocked", errorCode: "dirtyWorktree", - changedPaths: ["src/local.ts", "notes.txt"], mayHaveSideEffects: false, }); - expect(state.calls.some((args) => args[0] === "remote")).toBe(false); - }); - - test("rejects detached HEAD before mutation", async () => { - const detached = createRunner({ branch: fail("detached") }); - await expect( - prepareMerge("main", { - runGit: detached.runGit, - pathExists: createPathExists(detached.state), - }), - ).resolves.toMatchObject({ - status: "blocked", - errorCode: "detachedHead", - }); + expect(calls.some(([command]) => command === "fetch")).toBe(false); }); - test.each([ - ["MERGE_HEAD", "merge"], - ["rebase-merge", "rebase"], - ["rebase-apply", "rebase"], - ["CHERRY_PICK_HEAD", "cherry-pick"], - ])( - "rejects an existing %s operation before mutation", - async (gitPath, operation) => { - const activeOperation = createRunner(); - await expect( - prepareMerge("main", { - runGit: activeOperation.runGit, - pathExists: createPathExists( - activeOperation.state, - gitPath, - ), - }), - ).resolves.toMatchObject({ - status: "blocked", - errorCode: "operationInProgress", - operation, - }); - expect( - activeOperation.state.calls.some((args) => args[0] === "fetch"), - ).toBe(false); - }, - ); - - test("returns typed conflict details without committing or pushing", async () => { - const { runGit, state } = createRunner({ - merge: fail("Automatic merge failed"), - conflictStatus: - "UU src/text.ts\0UD src/deleted.ts\0UU assets/image.png\0UU deps/lib\0", - conflictIndex: - "100644 aaaaaa 1\tsrc/text.ts\0" + - "100644 bbbbbb 2\tsrc/text.ts\0" + - "100644 cccccc 1\tassets/image.png\0" + - "100644 dddddd 2\tassets/image.png\0" + - "160000 eeeeee 1\tdeps/lib\0", - }); - const result = await prepareMerge("origin/main", { - runGit, - pathExists: (filePath) => - createPathExists(state)(filePath) || - filePath.replaceAll("\\", "/").endsWith("/assets/image.png"), - isBinaryFile: (filePath) => - filePath.replaceAll("\\", "/").endsWith("/assets/image.png"), - }); - - expect(result.status).toBe("conflicts"); - if (result.status === "conflicts") { - expect(result.mergeInProgress).toBe(true); - expect(result.conflicts).toEqual([ - { - path: "src/text.ts", - status: "UU", - kind: "bothModified", - binary: false, - submodule: false, - }, - { - path: "src/deleted.ts", - status: "UD", - kind: "deletedByThem", - binary: false, - submodule: false, - }, - { - path: "assets/image.png", - status: "UU", - kind: "bothModified", - binary: true, - submodule: false, - }, - { - path: "deps/lib", - status: "UU", - kind: "bothModified", - binary: false, - submodule: true, - }, - ]); - } - expect(state.calls).toContainEqual([ - "merge", - "--no-commit", - "--no-ff", - "--", - "0123456789abcdef", - ]); - expect( - state.calls.some((args) => ["commit", "push"].includes(args[0])), - ).toBe(false); - }); - - test("reports fetch failures as retryable pre-merge errors", async () => { - const { runGit, state } = createRunner({ - fetch: fail("authentication failed", 128), - }); - const result = await prepareMerge("main", { + test("rejects an option-like target before fetch", async () => { + const { runGit, calls } = createRunner(); + const result = await mergeAndCommit("--upload-pack=bad", { + cwd: ROOT, runGit, - pathExists: createPathExists(state), + pathExists: () => false, }); expect(result).toMatchObject({ status: "blocked", - errorCode: "fetchFailed", - mayHaveSideEffects: true, + errorCode: "missingTargetBranch", }); - expect(state.calls.some((args) => args[0] === "merge")).toBe(false); + expect(calls.some(([command]) => command === "fetch")).toBe(false); }); - test("blocks when the worktree changes during fetch", async () => { - const { runGit, state } = createRunner({ - statusSequence: ["", "?? local.txt\0"], + test("returns conflicted paths for Reasoning instead of committing", async () => { + const { runGit, calls } = createRunner({ + merge: fail("CONFLICT"), + conflicts: ["src/a.ts", "src/b.ts"], }); - const result = await prepareMerge("main", { + const result = await mergeAndCommit("main", { + cwd: ROOT, runGit, - pathExists: createPathExists(state), + pathExists: () => false, + writeFile: () => {}, }); expect(result).toMatchObject({ - status: "blocked", - errorCode: "dirtyWorktree", - changedPaths: ["local.txt"], - mayHaveSideEffects: true, + status: "conflicts", + conflicts: ["src/a.ts", "src/b.ts"], }); - expect(state.calls.some((args) => args[0] === "merge")).toBe(false); + expect(calls.some(([command]) => command === "commit")).toBe(false); + expect(calls.some(([command]) => command === "push")).toBe(false); }); +}); - test("blocks when the current branch tip changes during fetch", async () => { - const { runGit, state } = createRunner({ - headSequence: ["aaaaaaaa", "bbbbbbbb"], +describe("completeMergeConflictResolution", () => { + test("refuses to commit while conflicts remain", async () => { + const { runGit, calls } = createRunner({ + conflicts: ["src/a.ts"], }); - const result = await prepareMerge("main", { + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), - }); - - expect(result).toMatchObject({ - status: "blocked", - errorCode: "branchChanged", - mayHaveSideEffects: true, - }); - expect(state.calls.some((args) => args[0] === "merge")).toBe(false); - }); - - test("fails closed when a remote branch cannot be inspected", async () => { - const { runGit, state } = createRunner({ - remotes: ["origin", "upstream"], - remoteBranches: { origin: ["main"] }, - }); - const failingRunner: GitCommandRunner = async ( - args, - cwd, - timeoutMs, - ) => { - if ( - args[0] === "ls-remote" && - args[3] === "upstream" && - args.includes("--heads") - ) { - return fail("authentication failed", 128); - } - return runGit(args, cwd, timeoutMs); - }; - const result = await prepareMerge("main", { - runGit: failingRunner, - pathExists: createPathExists(state), - }); - - expect(result).toMatchObject({ - status: "blocked", - errorCode: "remoteUnavailable", - mayHaveSideEffects: false, - }); - expect(state.calls.some((args) => args[0] === "fetch")).toBe(false); - }); - - test("reports missing Git distinctly from a non-repository", async () => { - const result = await prepareMerge("main", { - runGit: async () => ({ - exitCode: Number.NaN, - stdout: "", - stderr: "spawn git ENOENT", - failureCode: "ENOENT", - }), + ...resolutionState, }); expect(result).toMatchObject({ status: "blocked", - errorCode: "gitUnavailable", - mayHaveSideEffects: false, + errorCode: "unresolvedConflicts", + conflicts: ["src/a.ts"], }); + expect(calls.some(([command]) => command === "commit")).toBe(false); }); -}); - -describe("verifyMergeConflictsResolved", () => { - test("reports remaining unmerged paths", async () => { - const { runGit, state } = createRunner({ - conflictStatus: "UU src/file.ts\0", - }); - state.mergeStarted = true; - const result = await verifyMergeConflictsResolved({ + test("refuses to commit unstaged merge changes", async () => { + const { runGit } = createRunner({ unstaged: true }); + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), + ...resolutionState, }); - expect(result).toMatchObject({ - status: "unresolved", - mergeInProgress: true, - remainingConflicts: [ - { - path: "src/file.ts", - status: "UU", - kind: "bothModified", - }, - ], + status: "blocked", + errorCode: "unstagedChanges", }); }); - test("reports conflict markers after index conflicts are resolved", async () => { - const { runGit, state } = createRunner({ - changedPaths: "src/file.ts\0", - stagedCheck: { - exitCode: 2, - stdout: "src/file.ts:1: leftover conflict marker\n", - stderr: "", - }, + test("refuses to commit staged changes unrelated to the merge", async () => { + const { runGit } = createRunner({ + allowedPaths: ["src/a.ts"], + stagedPaths: ["src/a.ts", "notes.txt"], }); - state.mergeStarted = true; - const result = await verifyMergeConflictsResolved({ + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), + ...resolutionState, }); - expect(result).toMatchObject({ - status: "markersRemain", - markerPaths: ["src/file.ts"], - mergeInProgress: true, + status: "blocked", + errorCode: "unrelatedChanges", }); }); - test("returns resolved while leaving the merge uncommitted", async () => { - const { runGit, state } = createRunner({ - changedPaths: "src/file.ts\0", - }); - state.mergeStarted = true; - const result = await verifyMergeConflictsResolved({ + test("refuses to commit remaining conflict markers", async () => { + const { runGit } = createRunner({ markers: true }); + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), + ...resolutionState, }); - expect(result).toMatchObject({ - status: "resolved", - mergeInProgress: true, - markerPaths: [], - remainingConflicts: [], + status: "blocked", + errorCode: "conflictMarkers", }); - expect( - state.calls.some((args) => - ["add", "commit", "merge --continue", "push"].includes( - args.join(" "), - ), - ), - ).toBe(false); }); - test("reports unstaged merge changes instead of claiming resolution", async () => { - const { runGit, state } = createRunner({ - changedPaths: "src/file.ts\0", - unstagedPaths: "src/file.ts\0", + test("allows a resolved rename recorded in conflict state", async () => { + const { runGit } = createRunner({ + allowedPaths: ["src/old.ts"], + stagedPaths: ["src/new.ts"], }); - state.mergeStarted = true; - const result = await verifyMergeConflictsResolved({ + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), - }); - - expect(result).toMatchObject({ - status: "unstagedChanges", - unstagedPaths: ["src/file.ts"], + ...resolutionState, + readFile: () => JSON.stringify(["src/new.ts"]), }); + expect(result).toMatchObject({ status: "committed" }); }); - test("fails closed when Git cannot inspect the unmerged index", async () => { - const { runGit, state } = createRunner({ - conflictInspection: fail("index unavailable"), - }); - state.mergeStarted = true; - const result = await verifyMergeConflictsResolved({ + test("does not mistake incoming whitespace errors for conflict markers", async () => { + const { runGit } = createRunner({ whitespaceErrors: true }); + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), - }); - - expect(result).toMatchObject({ - status: "blocked", - errorCode: "verificationFailed", - mayHaveSideEffects: false, + ...resolutionState, }); + expect(result).toMatchObject({ status: "committed" }); }); - test("accepts a clean prepared merge with no conflict paths", async () => { - const { runGit, state } = createRunner(); - state.mergeStarted = true; - const result = await verifyMergeConflictsResolved({ + test("commits a fully resolved and staged merge without pushing", async () => { + const { runGit, calls } = createRunner(); + const result = await completeMergeConflictResolution({ + cwd: ROOT, runGit, - pathExists: createPathExists(state), + ...resolutionState, }); expect(result).toMatchObject({ - status: "resolved", - inspectedPaths: [], - remainingConflicts: [], - markerPaths: [], - unstagedPaths: [], + status: "committed", + commit: "0123456789abcdef", }); + expect(calls).toContainEqual(["commit", "--no-edit"]); + expect(calls.some(([command]) => command === "push")).toBe(false); }); }); -describe("MCP-facing structured results", () => { - test("accepts a grammar action with omitted empty parameters", () => { - expect( - getRequestedMergeTarget({ - actionName: "resolveMergeConflicts", - }), - ).toBeUndefined(); - }); - - test("preparation exposes stable raw data without follow-up actions", () => { - const preparation = { - status: "conflicts" as const, +describe("merge action results", () => { + test("routes conflicted files to the existing Reasoning action", () => { + const result = buildMergeResult({ + status: "conflicts", repositoryRoot: ROOT, currentBranch: "feature/work", target: { remote: "origin", branch: "main", displayName: "origin/main", - fetchedCommit: "0123456789abcdef", }, - mergeInProgress: true, - conflicts: [ - { - path: "src/file.ts", - status: "UU", - kind: "bothModified" as const, - binary: false, - submodule: false, - }, - ], - recovery: ["Review before committing."], - }; - - const result = buildMergeResult(preparation); - expect(result.resultValue).toBe(preparation); - expect(JSON.parse(result.historyText ?? "")).toEqual(preparation); - expect(result.displayContent).toMatchObject({ - rawData: preparation, + conflicts: ["src/a.ts"], }); - expect(result.additionalActions).toBeUndefined(); + + expect(result.error).toBeUndefined(); + if (result.error !== undefined) { + throw new Error(result.error); + } + expect(result.additionalActions).toEqual([ + expect.objectContaining({ + schemaName: "dispatcher.reasoning", + actionName: "reasoningAction", + }), + ]); + expect( + result.additionalActions?.[0].parameters?.originalRequest, + ).toContain(JSON.stringify(ROOT)); + expect(result.additionalActions?.[0].parameters?.workingDirectory).toBe( + ROOT, + ); }); - test("verification exposes resolved state without committing or pushing", () => { - const verification = { - status: "resolved" as const, - repositoryRoot: ROOT, - currentBranch: "feature/work", - mergeInProgress: true as const, - inspectedPaths: ["src/file.ts"], - remainingConflicts: [], - markerPaths: [], - unstagedPaths: [], - recovery: ["Review before committing."], - }; - - const result = buildVerificationResult(verification); - expect(result.resultValue).toBe(verification); - expect(JSON.parse(result.historyText ?? "")).toEqual(verification); - expect(result.displayContent).toMatchObject({ - rawData: verification, - }); - expect(result.additionalActions).toBeUndefined(); + test("handles grammar actions whose empty parameters were omitted", () => { + expect( + getRequestedMergeTarget({ actionName: "resolveMergeConflicts" }), + ).toBeUndefined(); }); }); diff --git a/ts/packages/dispatcher/dispatcher/src/context/dispatcher/schema/reasoningActionSchema.ts b/ts/packages/dispatcher/dispatcher/src/context/dispatcher/schema/reasoningActionSchema.ts index fd12be762e..c8e6bee18c 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/dispatcher/schema/reasoningActionSchema.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/dispatcher/schema/reasoningActionSchema.ts @@ -17,5 +17,8 @@ export interface ReasoningAction { attemptedAction?: string; // JSON-serialized entities on the stack at the time of redirect. contextEntities?: string; + // Override the tool working directory for a task tied to a specific + // local repository. Omit for the TypeAgent repository default. + workingDirectory?: string; }; } diff --git a/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts b/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts index 0934c69840..b87989eec6 100644 --- a/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts +++ b/ts/packages/dispatcher/dispatcher/src/execute/actionHandlers.ts @@ -23,7 +23,6 @@ import { ParsedCommandParams, ParameterDefinitions, AppAction, - ReadinessReport, } from "@typeagent/agent-sdk"; import type { Span } from "@opentelemetry/api"; import { @@ -117,10 +116,8 @@ export async function checkAgentReady( appAgentName: string, systemContext: CommandHandlerContext, actionContext: ActionContext, - readinessOverride?: ReadinessReport, ): Promise { - const report = - readinessOverride ?? systemContext.agents.getReadiness(appAgentName); + const report = systemContext.agents.getReadiness(appAgentName); if (report.state === "ready") { return undefined; } @@ -294,15 +291,10 @@ async function executeHandlerForActionSpan( let setupResult: ActionResult | undefined; try { - const actionReadiness = await appAgent.getActionReadiness?.( - executableAction.action, - actionContext.sessionContext, - ); setupResult = await checkAgentReady( appAgentName, systemContext, actionContext, - actionReadiness, ); } catch (error) { rethrowIfActionCancelled(error, systemContext); diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts index 13e6afb1f5..8aa2a55dbc 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/claude.ts @@ -491,6 +491,7 @@ function createClaudeCanUseTool(context: ActionContext) { function getClaudeOptions( context: ActionContext, + workingDirectory?: string, ): Options { const systemContext = context.sessionContext.agentContext; // Stable clientIO reference for get_user_context (see copilot.ts). @@ -1181,7 +1182,7 @@ function getClaudeOptions( // AVAILABILITY; `canUseTool` decides ALLOW/DENY per call. canUseTool: createClaudeCanUseTool(context), tools: availableBuiltInTools, - cwd: getRepoRoot(), + cwd: workingDirectory ?? getRepoRoot(), settingSources: [], maxTurns: 20, thinking: { type: "adaptive" }, @@ -1951,6 +1952,7 @@ async function executeReasoningWithoutPlanning( fallbackContext?: ReasoningFallbackContext, abortController?: AbortController, requireToolUse: boolean = false, + workingDirectory?: string, ): Promise { const abortSignal = abortController?.signal; // Display initial message @@ -1963,7 +1965,7 @@ async function executeReasoningWithoutPlanning( fallbackContext, ), options: { - ...getClaudeOptions(context), + ...getClaudeOptions(context, workingDirectory), ...claudeExecutableOption(), ...(abortController === undefined ? {} : { abortController }), }, @@ -2002,6 +2004,7 @@ async function executeReasoningWithTracing( fallbackContext?: ReasoningFallbackContext, abortController?: AbortController, requireToolUse: boolean = false, + workingDirectory?: string, ): Promise { const abortSignal = abortController?.signal; const systemContext = context.sessionContext.agentContext; @@ -2016,6 +2019,7 @@ async function executeReasoningWithTracing( undefined, abortController, requireToolUse, + workingDirectory, ); } @@ -2044,7 +2048,7 @@ async function executeReasoningWithTracing( fallbackContext, ), options: { - ...getClaudeOptions(context), + ...getClaudeOptions(context, workingDirectory), ...claudeExecutableOption(), ...(abortController === undefined ? {} : { abortController }), }, @@ -2292,6 +2296,9 @@ export async function executeReasoningAction( planReuseEnabled: planReuseEnabled || scriptReuseEnabled, engine: "claude", requireToolUse: true, + ...(action.parameters.workingDirectory === undefined + ? {} + : { workingDirectory: action.parameters.workingDirectory }), ...(fallbackContext ? { fallbackContext } : {}), }); } @@ -2664,6 +2671,7 @@ export async function executeReasoning( // (reasoningAction). Conversation-answer callers leave it false, since // answering a question with text only is a valid result. requireToolUse?: boolean; + workingDirectory?: string; }, ) { const engine = options?.engine ?? "claude"; @@ -2673,6 +2681,7 @@ export async function executeReasoning( const planReuseEnabled = options?.planReuseEnabled ?? false; const fallbackContext = options?.fallbackContext; const requireToolUse = options?.requireToolUse ?? false; + const workingDirectory = options?.workingDirectory; const controller = new AbortController(); return runInReasoningSpan( context, @@ -2685,6 +2694,7 @@ export async function executeReasoning( fallbackContext, controller, requireToolUse, + workingDirectory, ); } // Trace capture + auto recipe generation @@ -2694,6 +2704,7 @@ export async function executeReasoning( fallbackContext, controller, requireToolUse, + workingDirectory, ); }), { diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts index 333dcbead3..75b638b537 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts @@ -1312,6 +1312,7 @@ export async function executeCodingRequest( */ function getCopilotSessionConfig( context: ActionContext, + workingDirectory?: string, ): SessionConfig { const systemContext = context.sessionContext.agentContext; // Capture the request's clientIO now, before execute_action transiently @@ -2142,7 +2143,7 @@ function getCopilotSessionConfig( "github/search/*", "shell", ], - workingDirectory: getRepoRoot(), + workingDirectory: workingDirectory ?? getRepoRoot(), onPermissionRequest: createCopilotPermissionHandler(context), systemMessage: { mode: "append" as const, @@ -2273,6 +2274,7 @@ async function createCopilotSession( async function executeReasoningWithoutPlanning( originalRequest: string, context: ActionContext, + workingDirectory?: string, ): Promise { debug(`Executing reasoning request: ${originalRequest}`); context.actionIO.appendDisplay("Thinking...", "temporary"); @@ -2299,7 +2301,7 @@ async function executeReasoningWithoutPlanning( let copilotToolLoopIteration = 0; const client = await getCopilotClient(context.sessionContext.agentContext); - const config = getCopilotSessionConfig(context); + const config = getCopilotSessionConfig(context, workingDirectory); // Check for existing session ID to enable multi-turn conversations let sessionId = getSessionId(context); @@ -2605,13 +2607,18 @@ async function executeReasoningWithoutPlanning( async function executeReasoningWithTracing( originalRequest: string, context: ActionContext, + workingDirectory?: string, ): Promise { const systemContext = context.sessionContext.agentContext; const storage = context.sessionContext.sessionStorage; if (!storage) { debug("No sessionStorage available, using standard reasoning"); - return executeReasoningWithoutPlanning(originalRequest, context); + return executeReasoningWithoutPlanning( + originalRequest, + context, + workingDirectory, + ); } const requestId = generateRequestId(); @@ -2654,7 +2661,7 @@ async function executeReasoningWithTracing( const client = await getCopilotClient( context.sessionContext.agentContext, ); - const config = getCopilotSessionConfig(context); + const config = getCopilotSessionConfig(context, workingDirectory); // Check for existing session ID to enable multi-turn conversations let sessionId = getSessionId(context); @@ -3150,6 +3157,9 @@ export async function executeReasoningAction( return executeReasoning(request, context, { planReuseEnabled, engine: "copilot", + ...(action.parameters.workingDirectory === undefined + ? {} + : { workingDirectory: action.parameters.workingDirectory }), }); } @@ -3163,6 +3173,7 @@ export async function executeReasoning( options?: { planReuseEnabled?: boolean; engine?: "copilot"; + workingDirectory?: string; }, ): Promise { const engine = options?.engine ?? "copilot"; @@ -3177,10 +3188,18 @@ export async function executeReasoning( () => { if (!planReuseEnabled) { // Standard reasoning without planning - return executeReasoningWithoutPlanning(request, context); + return executeReasoningWithoutPlanning( + request, + context, + options?.workingDirectory, + ); } // Trace capture + auto recipe generation - return executeReasoningWithTracing(request, context); + return executeReasoningWithTracing( + request, + context, + options?.workingDirectory, + ); }, { genAiSystem: "github_copilot", diff --git a/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts b/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts index 8fbffa1881..ed622a878f 100644 --- a/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/agentReadiness.spec.ts @@ -378,21 +378,6 @@ describe("checkAgentReady (pre-flight gate)", () => { expect(out).toBeUndefined(); }); - test("honors a ready per-action override", async () => { - const sys = fakeSystemContext({ - readiness: new Map([ - [ - "agentA", - { state: "setup-required", message: "missing tool" }, - ], - ]), - }); - const out = await checkAgentReady("agentA", sys, fakeActionContext(), { - state: "ready", - }); - expect(out).toBeUndefined(); - }); - describe("@config agent setup manual instructions", () => { test("renders setup details as markdown", () => { const display = getManualAgentSetupDisplay("player", { From a77c12b52edf63105620e1a3b6e946b2d9e02f81 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 23:20:57 -0700 Subject: [PATCH 4/7] Clarify merge destination branch Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/github-cli/src/github-cliSchema.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.ts b/ts/packages/agents/github-cli/src/github-cliSchema.ts index 4c7beaa2b7..415a61906e 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.ts +++ b/ts/packages/agents/github-cli/src/github-cliSchema.ts @@ -711,18 +711,20 @@ export type DependabotAlertsAction = { }; }; -// Fetch a target branch, merge it into the current branch, and create the merge -// commit. If Git reports conflicts, hand the conflicted files to Reasoning for -// semantic resolution before the deterministic completion action commits. +// Fetch a source branch, merge it into the currently checked-out local branch, +// and create the merge commit. This action never checks out a different +// destination branch. If Git reports conflicts, hand the conflicted files to +// Reasoning for semantic resolution before the deterministic completion action +// commits. // Use this for requests such as "resolve merge conflicts from main" or "bring // the default branch into this branch and resolve conflicts". This never pushes. export type ResolveMergeConflictsAction = { actionName: "resolveMergeConflicts"; parameters: { - // Branch to merge into the current local branch. A REMOTE/BRANCH value - // disambiguates repositories with multiple remotes. When omitted, use - // the selected remote's configured default branch, then an existing - // main or master branch. + // Source branch to merge into the currently checked-out local branch. A + // REMOTE/BRANCH value disambiguates repositories with multiple remotes. + // When omitted, use the selected remote's configured default branch, + // then an existing main or master branch. targetBranch?: string; }; }; From 991243126664f3d79598b4625cb74217b0bb43af Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 22:01:11 -0700 Subject: [PATCH 5/7] Fix native tool access for merge conflict reasoning Use current Copilot built-in tool filters, including model-specific editors and shell lifecycle tools. Cover deterministic merge completion and real Git conflict handling with regression tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb5bc7a3-4119-4f6d-a71b-f928f2490ffa --- .../github-cli/test/mergeConflict.spec.ts | 9 +- .../test/mergeConflictIntegration.spec.ts | 118 ++++++++++++++++++ .../dispatcher/src/reasoning/copilot.ts | 98 ++++++++++----- .../test/copilotAvailableTools.spec.ts | 107 ++++++++++++++++ 4 files changed, 297 insertions(+), 35 deletions(-) create mode 100644 ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts create mode 100644 ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.spec.ts diff --git a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts index ee7c62fa33..32dd0e0f2a 100644 --- a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts +++ b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts @@ -370,7 +370,7 @@ describe("completeMergeConflictResolution", () => { }); describe("merge action results", () => { - test("routes conflicted files to the existing Reasoning action", () => { + test("queues verification after Reasoning instead of relying on model completion", () => { const result = buildMergeResult({ status: "conflicts", repositoryRoot: ROOT, @@ -392,10 +392,15 @@ describe("merge action results", () => { schemaName: "dispatcher.reasoning", actionName: "reasoningAction", }), + { + schemaName: "github-cli", + actionName: "completeMergeConflictResolution", + parameters: { repositoryRoot: ROOT }, + }, ]); expect( result.additionalActions?.[0].parameters?.originalRequest, - ).toContain(JSON.stringify(ROOT)); + ).toContain(ROOT); expect(result.additionalActions?.[0].parameters?.workingDirectory).toBe( ROOT, ); diff --git a/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts b/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts new file mode 100644 index 0000000000..2aafa133f5 --- /dev/null +++ b/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { + completeMergeConflictResolution, + mergeAndCommit, +} from "../src/mergeConflict.js"; + +describe("merge conflict resolution with real Git", () => { + let repository: string; + const file = "source file.ts"; + const git = (...args: string[]) => + execFileSync("git", args, { + cwd: repository, + encoding: "utf8", + windowsHide: true, + stdio: ["ignore", "pipe", "pipe"], + }).trim(); + + beforeEach(() => { + repository = fs.mkdtempSync( + path.join(os.tmpdir(), "typeagent-merge-test-"), + ); + git("init", "-b", "main"); + git("config", "user.name", "Merge test"); + git("config", "user.email", "merge-test@example.invalid"); + git("config", "commit.gpgsign", "false"); + git("config", "core.autocrlf", "false"); + git("config", "core.hooksPath", path.join(repository, "no-hooks")); + fs.writeFileSync( + path.join(repository, file), + "export const base = 1;\n", + ); + git("add", "--", file); + git("commit", "-m", "base"); + git("checkout", "-b", "feature"); + fs.writeFileSync( + path.join(repository, file), + "export const ours = 1;\n", + ); + git("add", "--", file); + git("commit", "-m", "feature change"); + git("checkout", "main"); + fs.writeFileSync( + path.join(repository, file), + "export const theirs = 1;\n", + ); + git("add", "--", file); + git("commit", "-m", "main change"); + git("checkout", "feature"); + git("remote", "add", "origin", repository); + }); + + afterEach(() => { + fs.rmSync(repository, { recursive: true, force: true }); + }); + + test("leaves an incomplete resolution blocked, then verifies and commits both parents", async () => { + const before = git("rev-parse", "HEAD"); + const target = git("rev-parse", "main"); + const merged = await mergeAndCommit("main", { cwd: repository }); + expect(merged).toMatchObject({ + status: "conflicts", + conflicts: [file], + }); + expect(git("rev-parse", "HEAD")).toBe(before); + + // A model returning without editing must not count as completion. + expect( + await completeMergeConflictResolution({ cwd: repository }), + ).toMatchObject({ + status: "blocked", + errorCode: "unresolvedConflicts", + }); + expect(git("rev-parse", "HEAD")).toBe(before); + + fs.writeFileSync( + path.join(repository, file), + "export const ours = 1;\nexport const theirs = 1;\n", + ); + git("add", "--", file); + const completed = await completeMergeConflictResolution({ + cwd: repository, + }); + expect(completed).toMatchObject({ status: "committed" }); + expect(git("show", "-s", "--format=%P", "HEAD").split(" ")).toEqual([ + before, + target, + ]); + expect(git("status", "--porcelain")).toBe(""); + expect(fs.existsSync(path.join(repository, ".git", "MERGE_HEAD"))).toBe( + false, + ); + expect( + fs.existsSync( + path.join(repository, ".git", "TYPEAGENT_MERGE_CONFLICTS"), + ), + ).toBe(false); + }); + + test("rejects staged conflict markers even when Git considers the path resolved", async () => { + const before = git("rev-parse", "HEAD"); + await mergeAndCommit("main", { cwd: repository }); + git("add", "--", file); + expect(git("diff", "--name-only", "--diff-filter=U")).toBe(""); + expect( + await completeMergeConflictResolution({ cwd: repository }), + ).toMatchObject({ + status: "blocked", + errorCode: "conflictMarkers", + }); + expect(git("rev-parse", "HEAD")).toBe(before); + }); +}); diff --git a/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts b/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts index 75b638b537..f24a1e8870 100644 --- a/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts +++ b/ts/packages/dispatcher/dispatcher/src/reasoning/copilot.ts @@ -672,8 +672,8 @@ function formatToolCallDisplay(toolName: string, input: unknown): string { return `**Tool:** \`remember\``; } - // Built-in tools (shell, github/fs/*, github/search/*, ...): show the - // primary argument so parallel or similar calls are distinguishable + // Built-in tools (view, edit, create, glob, grep, powershell, bash, ...): + // show the primary argument so parallel or similar calls are distinguishable // instead of rendering as identical "Tool: " bubbles. The tool name // is rendered as inline code so it reads as a highlighted chip (matching a // single tool call and the folded-batch summary). @@ -1306,9 +1306,63 @@ export async function executeCodingRequest( } } +// These are runtime tool names, not the obsolete github/fs/* or shell aliases. +// Restrict matches to built-ins; onPermissionRequest still authorizes each call. +export const COPILOT_NATIVE_BUILTIN_TOOLS: readonly string[] = [ + "builtin:view", + "builtin:edit", + "builtin:create", + "builtin:apply_patch", + "builtin:str_replace_editor", + "builtin:glob", + "builtin:grep", + "builtin:rg", + "builtin:powershell", + "builtin:read_powershell", + "builtin:stop_powershell", + "builtin:list_powershell", + "builtin:bash", + "builtin:read_bash", + "builtin:stop_bash", + "builtin:list_bash", + "builtin:web_fetch", +]; + +const COPILOT_ALWAYS_ON_CUSTOM_TOOLS: readonly string[] = [ + "discover_actions", + "execute_action", + "search_memory", + "remember", + "get_conversation_info", + "read_conversation", + "list_conversations", + "search_conversations", + "get_user_context", + "find_installable_agent", + "ask_user", + "ask_user_form", +]; + +const COPILOT_SUBAGENT_CUSTOM_TOOLS: readonly string[] = [ + "create_subagent", + "invoke_subagent", + "list_subagents", + "stop_subagent", +]; + +export function buildCopilotAvailableTools(opts: { + subagentsEnabled: boolean; +}): string[] { + return [ + ...COPILOT_ALWAYS_ON_CUSTOM_TOOLS, + ...(opts.subagentsEnabled ? COPILOT_SUBAGENT_CUSTOM_TOOLS : []), + ...COPILOT_NATIVE_BUILTIN_TOOLS, + ]; +} + /** - * Get Copilot SDK session configuration with TypeAgent tools - * (Mirrors getClaudeOptions from claude.ts) + * Get Copilot SDK session configuration with TypeAgent tools. + * Mirrors getClaudeOptions from claude.ts. */ function getCopilotSessionConfig( context: ActionContext, @@ -2118,31 +2172,7 @@ function getCopilotSessionConfig( askUserTool, askUserFormTool, ], - availableTools: [ - "discover_actions", - "execute_action", - "search_memory", - "remember", - "get_conversation_info", - "read_conversation", - "list_conversations", - "search_conversations", - "get_user_context", - ...(subagentsEnabled - ? [ - "create_subagent", - "invoke_subagent", - "list_subagents", - "stop_subagent", - ] - : []), - "find_installable_agent", - "ask_user", - "ask_user_form", - "github/fs/*", - "github/search/*", - "shell", - ], + availableTools: buildCopilotAvailableTools({ subagentsEnabled }), workingDirectory: workingDirectory ?? getRepoRoot(), onPermissionRequest: createCopilotPermissionHandler(context), systemMessage: { @@ -2155,9 +2185,11 @@ function getCopilotSessionConfig( "## Built-in Tools (USE THESE FIRST)", "You have access to powerful built-in capabilities:", "- **Web search**: Use your native web search for looking up information online", - "- **File operations**: `github/fs/*` for reading, writing, editing files", - "- **Code search**: `github/search/*` for searching code patterns", - "- **Shell commands**: `shell` for executing terminal commands", + "- **File reading**: `view` for reading a file (whole file or a line range) or listing a directory", + "- **File search**: `glob` for finding files by name; `grep` or `rg` for searching contents, depending on the model's available tools", + "- **File editing**: Use the available native editor: `edit` / `create`, `apply_patch`, or `str_replace_editor`", + "- **Shell commands**: `powershell` on Windows or `bash` on macOS/Linux; use the matching `read_*`, `stop_*`, and `list_*` tools for commands that continue running", + "- **Web pages**: `web_fetch` for retrieving a URL", "", "## TypeAgent Action Tools (USE WHEN NEEDED)", "For TypeAgent-specific actions like music playback, calendar management, email:", @@ -2202,7 +2234,7 @@ function getCopilotSessionConfig( "- **PREFER built-in tools** for web search, file operations, and code investigation", "- **Use TypeAgent actions** only for domain-specific operations (music, calendar, email, etc.)", "- For web search queries → use your native web search capability", - "- For code operations → use `github/fs/*` and `github/search/*` tools", + "- For code operations → use the available native file, search, and terminal tools directly", "- For TypeAgent capabilities → use `discover_actions` then `execute_action`", ].join("\n"), }, diff --git a/ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.spec.ts b/ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.spec.ts new file mode 100644 index 0000000000..83e4b7e55c --- /dev/null +++ b/ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.spec.ts @@ -0,0 +1,107 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + buildCopilotAvailableTools, + COPILOT_NATIVE_BUILTIN_TOOLS, +} from "../src/reasoning/copilot.js"; + +describe("Copilot availableTools allowlist", () => { + it("uses source-qualified `builtin:` filters so only host-registered built-ins match", () => { + for (const entry of COPILOT_NATIVE_BUILTIN_TOOLS) { + expect(entry.startsWith("builtin:")).toBe(true); + const name = entry.slice("builtin:".length); + expect(name).toMatch(/^[a-zA-Z0-9_-]+$|^\*$/); + } + }); + + it("advertises the native tools the merge-conflict resolution action depends on", () => { + expect(COPILOT_NATIVE_BUILTIN_TOOLS).toEqual( + expect.arrayContaining([ + "builtin:view", + "builtin:edit", + "builtin:create", + "builtin:glob", + "builtin:grep", + ]), + ); + expect(COPILOT_NATIVE_BUILTIN_TOOLS).toEqual( + expect.arrayContaining(["builtin:powershell", "builtin:bash"]), + ); + }); + + it("does not include obsolete `github/fs/*`, `github/search/*`, or bare `shell` patterns", () => { + for (const obsolete of [ + "github/fs/*", + "github/search/*", + "shell", + "builtin:github/fs/*", + "builtin:github/search/*", + "builtin:shell", + ]) { + expect(COPILOT_NATIVE_BUILTIN_TOOLS).not.toContain(obsolete); + } + }); + + it("includes model-specific editors and shell output and cancellation tools", () => { + expect(COPILOT_NATIVE_BUILTIN_TOOLS).toEqual( + expect.arrayContaining([ + "builtin:apply_patch", + "builtin:str_replace_editor", + "builtin:rg", + "builtin:read_powershell", + "builtin:stop_powershell", + "builtin:list_powershell", + "builtin:read_bash", + "builtin:stop_bash", + "builtin:list_bash", + "builtin:web_fetch", + ]), + ); + }); + + it("composes custom tools, subagent tools (when enabled), and native built-ins", () => { + const withoutSubagents = buildCopilotAvailableTools({ + subagentsEnabled: false, + }); + const withSubagents = buildCopilotAvailableTools({ + subagentsEnabled: true, + }); + + for (const custom of [ + "discover_actions", + "execute_action", + "search_memory", + "get_user_context", + "ask_user", + "ask_user_form", + "find_installable_agent", + ]) { + expect(withoutSubagents).toContain(custom); + expect(withSubagents).toContain(custom); + } + + for (const builtin of COPILOT_NATIVE_BUILTIN_TOOLS) { + expect(withoutSubagents).toContain(builtin); + expect(withSubagents).toContain(builtin); + } + + const subagentTools = [ + "create_subagent", + "invoke_subagent", + "list_subagents", + "stop_subagent", + ]; + for (const s of subagentTools) { + expect(withoutSubagents).not.toContain(s); + expect(withSubagents).toContain(s); + } + }); + + it("returns a fresh array each call so callers cannot mutate the source constants", () => { + const first = buildCopilotAvailableTools({ subagentsEnabled: true }); + first.pop(); + const second = buildCopilotAvailableTools({ subagentsEnabled: true }); + expect(second.length).toBeGreaterThan(first.length); + }); +}); From 3afcb23b47ea71d4163a846678d518906b7ee1e2 Mon Sep 17 00:00:00 2001 From: George Ng Date: Fri, 4 Sep 2026 22:41:07 -0700 Subject: [PATCH 6/7] Harden and simplify local merge conflict resolution Use the host-authorized repository and propagate request cancellation. Bind completion to the actual merge state and staged paths, preserving automatic rename resolutions. Consolidate Git probes and regression fixtures, and add an installed SDK tool contract check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb5bc7a3-4119-4f6d-a71b-f928f2490ffa --- ts/packages/agents/github-cli/README.md | 18 +- .../github-cli/src/github-cliActionHandler.ts | 42 +-- .../agents/github-cli/src/github-cliSchema.ts | 2 +- .../agents/github-cli/src/mergeConflict.ts | 328 ++++++++++++------ .../github-cli/test/mergeConflict.spec.ts | 277 +++++++++++++-- .../test/mergeConflictIntegration.spec.ts | 210 ++++++++++- ts/packages/dispatcher/dispatcher/README.md | 16 + .../dispatcher/dispatcher/package.json | 1 + .../test/copilotAvailableTools.test.ts | 75 ++++ 9 files changed, 795 insertions(+), 174 deletions(-) create mode 100644 ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.test.ts diff --git a/ts/packages/agents/github-cli/README.md b/ts/packages/agents/github-cli/README.md index b81fc65457..95909bc5a6 100644 --- a/ts/packages/agents/github-cli/README.md +++ b/ts/packages/agents/github-cli/README.md @@ -51,16 +51,26 @@ the source branch into the currently checked-out local branch. The default source is the remote's default branch, falling back to an existing `main` or `master`. This action does not check out another branch or push. +Both merge actions require the working directory supplied by the host session. +They never use the agent server's working directory as a fallback. + Clean merges are committed locally. For conflicts, Reasoning uses its native file and terminal tools in the repository root to inspect, resolve, and stage only the conflicted paths. No connected editor extension is required. The dispatcher then runs a separate completion action that verifies the index and creates the merge commit; a model's text response alone is not completion. -If resolution fails or is cancelled, the merge remains in progress. Inspect -`git status` before continuing. Resolve and stage the listed files, then run -`completeMergeConflictResolution` with the same `repositoryRoot`, or explicitly -abort with `git merge --abort`. Do not start another merge on top of it. +Completion checks the saved post-merge staged paths and original conflicts, +including changes Git automatically applied through renames. The saved state is +bound to the original `HEAD` and `MERGE_HEAD`; missing, outdated, or mismatched +state requires manual inspection and commit or abort. + +Cancellation stops further Git operations and cleans up the temporary fetch ref. +If a merge has already started, it is left in place rather than automatically +reset or aborted. Inspect `git status` before continuing. Resolve and stage the +listed files, then run `completeMergeConflictResolution` in the same host session +repository, or explicitly abort with `git merge --abort`. Do not start another +merge on top of it. ## Output Formatting diff --git a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts index 7a272beffb..9b963340c6 100644 --- a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts +++ b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts @@ -2038,31 +2038,33 @@ export function buildMergeResult(result: MergeConflictResult): ActionResult { return actionResult; } -async function executeResolveMergeConflicts( - targetBranch: string | undefined, -): Promise { - return buildMergeResult(await mergeAndCommit(targetBranch)); -} - -async function executeCompleteMergeConflictResolution( - repositoryRoot: string, -): Promise { - return buildMergeResult( - await completeMergeConflictResolution({ cwd: repositoryRoot }), - ); -} - // code-complexity-allow: top-level action dispatch over all github-cli actions async function executeAction( action: TypeAgentAction, context: ActionContext, ): Promise { - if (action.actionName === "resolveMergeConflicts") { - return executeResolveMergeConflicts(getRequestedMergeTarget(action)); - } - if (action.actionName === "completeMergeConflictResolution") { - return executeCompleteMergeConflictResolution( - action.parameters.repositoryRoot, + if ( + action.actionName === "resolveMergeConflicts" || + action.actionName === "completeMergeConflictResolution" + ) { + // The server's cwd may belong to an unrelated repository. + if (!context.workingDirectory) { + return buildMergeFailure({ + status: "blocked", + errorCode: "notRepository", + message: + "The host did not provide a working directory for this action. Open a session in a Git repository and retry.", + mayHaveSideEffects: false, + }); + } + const options = { + cwd: context.workingDirectory, + signal: context.abortSignal, + }; + return buildMergeResult( + action.actionName === "resolveMergeConflicts" + ? await mergeAndCommit(getRequestedMergeTarget(action), options) + : await completeMergeConflictResolution(options), ); } diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.ts b/ts/packages/agents/github-cli/src/github-cliSchema.ts index b29bcfd72f..ab616c532e 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.ts +++ b/ts/packages/agents/github-cli/src/github-cliSchema.ts @@ -810,7 +810,7 @@ export type ResolveMergeConflictsAction = { export type CompleteMergeConflictResolutionAction = { actionName: "completeMergeConflictResolution"; parameters: { - // Absolute root of the repository whose merge should be completed. + // Recorded merge root; execution always uses the host-authorized working directory. repositoryRoot: string; }; }; diff --git a/ts/packages/agents/github-cli/src/mergeConflict.ts b/ts/packages/agents/github-cli/src/mergeConflict.ts index f62dbdb5ce..ec4d0e5c65 100644 --- a/ts/packages/agents/github-cli/src/mergeConflict.ts +++ b/ts/packages/agents/github-cli/src/mergeConflict.ts @@ -18,6 +18,7 @@ export type GitCommandResult = { export type GitCommandRunner = ( args: readonly string[], cwd?: string, + signal?: AbortSignal, ) => Promise; export type MergeTarget = { @@ -74,6 +75,7 @@ export type MergeConflictResult = export type MergeOptions = { cwd?: string; + signal?: AbortSignal | undefined; runGit?: GitCommandRunner; pathExists?: (filePath: string) => boolean; readFile?: (filePath: string) => string; @@ -81,9 +83,46 @@ export type MergeOptions = { removeFile?: (filePath: string) => void; }; +// Bind the actual post-merge index paths to this merge, including edits +// Git automatically applied through renames. +type ResolutionState = { + version: 1; + head: string; + mergeHead: string; + conflicts: string[]; + stagedPaths: string[]; +}; + +function abortError(message = "The operation was aborted."): Error { + return new DOMException(message, "AbortError"); +} + +function isAbortError(error: unknown): boolean { + return ( + error instanceof Error && + (error.name === "AbortError" || + ("code" in error && error.code === "ABORT_ERR")) + ); +} + +function throwIfAborted(signal: AbortSignal | undefined): void { + if (signal?.aborted) { + throw abortError(); + } +} + +function cancellableGit(options: MergeOptions): GitCommandRunner { + const runGit = options.runGit ?? runGitCommand; + return (args, cwd) => { + throwIfAborted(options.signal); + return runGit(args, cwd, options.signal); + }; +} + export async function runGitCommand( args: readonly string[], cwd = process.cwd(), + signal?: AbortSignal, ): Promise { try { const { stdout, stderr } = await execFileAsync("git", [...args], { @@ -92,9 +131,14 @@ export async function runGitCommand( maxBuffer: 4 * 1024 * 1024, timeout: 10 * 60_000, windowsHide: true, + signal, }); return { exitCode: 0, stdout, stderr }; } catch (error) { + // Let the dispatcher distinguish cancellation from a failed Git command. + if (isAbortError(error) || signal?.aborted) { + throw abortError(); + } const failure = error as Error & { code?: number; stdout?: string; @@ -304,15 +348,18 @@ async function listConflicts( return result.exitCode === 0 ? nullSeparated(result.stdout) : []; } -async function hasMergeHead( +async function getMergeParents( repositoryRoot: string, runGit: GitCommandRunner, -): Promise { +): Promise<{ head: string; mergeHead: string } | undefined> { const result = await runGit( - ["rev-parse", "--verify", "MERGE_HEAD"], + ["rev-parse", "HEAD", "MERGE_HEAD"], repositoryRoot, ); - return result.exitCode === 0; + const [head, mergeHead] = lines(result.stdout); + return result.exitCode === 0 && head && mergeHead + ? { head, mergeHead } + : undefined; } async function getResolutionStatePath( @@ -357,12 +404,30 @@ async function listChangedPaths( return result.exitCode === 0 ? nullSeparated(result.stdout) : undefined; } +async function deleteTemporaryRef( + repositoryRoot: string, + runGit: GitCommandRunner, + ref: string, +): Promise { + // Cleanup must run even after cancellation, without masking its AbortError. + try { + const result = await runGit(["update-ref", "-d", ref], repositoryRoot); + if (result.exitCode !== 0) { + throw new Error(result.stderr || "git update-ref failed"); + } + } catch (error) { + console.warn( + `Could not remove temporary merge ref ${ref} in ${repositoryRoot}: ${String(error)}`, + ); + } +} + export async function mergeAndCommit( targetBranch?: string, options: MergeOptions = {}, ): Promise { const cwd = options.cwd ?? process.cwd(); - const runGit = options.runGit ?? runGitCommand; + const runGit = cancellableGit(options); const pathExists = options.pathExists ?? fs.existsSync; const writeFile = options.writeFile ?? @@ -370,6 +435,7 @@ export async function mergeAndCommit( const removeFile = options.removeFile ?? ((filePath) => fs.rmSync(filePath, { force: true })); + const repository = await getRepository(cwd, runGit); if ("status" in repository) { return repository; @@ -404,103 +470,161 @@ export async function mergeAndCommit( return target; } const temporaryRef = `refs/typeagent/merge/${randomUUID()}`; - const fetch = await runGit( - [ - "fetch", - "--no-tags", - "--no-write-fetch-head", - target.remote, - `refs/heads/${target.branch}:${temporaryRef}`, - ], - repositoryRoot, - ); - if (fetch.exitCode !== 0) { - return blocked( - "fetchFailed", - fetch.stderr.trim() || `Could not fetch ${target.displayName}.`, + try { + const fetch = await runGit( + [ + "fetch", + "--no-tags", + "--no-write-fetch-head", + target.remote, + `refs/heads/${target.branch}:${temporaryRef}`, + ], + repositoryRoot, ); - } - const fetchedCommit = await runGit( - ["rev-parse", "--verify", `${temporaryRef}^{commit}`], - repositoryRoot, - ); - if (fetchedCommit.exitCode !== 0) { - await runGit(["update-ref", "-d", temporaryRef], repositoryRoot); - return blocked( - "fetchFailed", - `Could not resolve the fetched commit for ${target.displayName}.`, + if (fetch.exitCode !== 0) { + return blocked( + "fetchFailed", + fetch.stderr.trim() || `Could not fetch ${target.displayName}.`, + ); + } + // This invocation's unique ref pins the fetched target until cleanup. + const merge = await runGit( + ["merge", "--no-commit", "--no-ff", temporaryRef], + repositoryRoot, ); - } - const merge = await runGit( - ["merge", "--no-commit", "--no-ff", fetchedCommit.stdout.trim()], - repositoryRoot, - ); - await runGit(["update-ref", "-d", temporaryRef], repositoryRoot); - if (merge.exitCode !== 0) { - const conflicts = await listConflicts(repositoryRoot, runGit); - if (conflicts.length > 0) { - if (resolutionStatePath === undefined) { - return blocked( - "mergeFailed", - "Git could not create conflict-resolution state.", - true, - { recovery: "Run `git merge --abort`." }, - ); - } - try { - writeFile(resolutionStatePath, JSON.stringify(conflicts)); - } catch (error) { - return blocked( - "mergeFailed", - `Could not save conflict-resolution state: ${String(error)}`, - true, - { recovery: "Run `git merge --abort`." }, + if (merge.exitCode !== 0) { + const conflicts = await listConflicts(repositoryRoot, runGit); + if (conflicts.length > 0) { + if (resolutionStatePath === undefined) { + return blocked( + "mergeFailed", + "Git could not create conflict-resolution state.", + true, + { recovery: "Run `git merge --abort`." }, + ); + } + const stagedSnapshot = await listChangedPaths( + repositoryRoot, + runGit, + ["diff", "--cached", "--name-only", "HEAD"], ); + const parents = await getMergeParents(repositoryRoot, runGit); + if (stagedSnapshot === undefined || parents === undefined) { + return blocked( + "mergeFailed", + "Git could not capture post-merge state.", + true, + { recovery: "Run `git merge --abort`." }, + ); + } + const state: ResolutionState = { + version: 1, + ...parents, + conflicts, + stagedPaths: stagedSnapshot, + }; + try { + writeFile(resolutionStatePath, JSON.stringify(state)); + } catch (error) { + return blocked( + "mergeFailed", + `Could not save conflict-resolution state: ${String(error)}`, + true, + { recovery: "Run `git merge --abort`." }, + ); + } + return { + status: "conflicts", + repositoryRoot, + currentBranch, + target, + conflicts, + }; } + return blocked( + "mergeFailed", + merge.stderr.trim() || "Git could not merge the target branch.", + true, + { + recovery: + "Inspect `git status`, then run `git merge --abort` if needed.", + }, + ); + } + if (!(await getMergeParents(repositoryRoot, runGit))) { return { - status: "conflicts", + status: "upToDate", repositoryRoot, currentBranch, target, - conflicts, }; } - return blocked( - "mergeFailed", - merge.stderr.trim() || "Git could not merge the target branch.", - true, - { - recovery: - "Inspect `git status`, then run `git merge --abort` if needed.", - }, + const commit = await commitMerge(repositoryRoot, runGit); + return typeof commit === "string" + ? { + status: "committed", + repositoryRoot, + currentBranch, + target, + commit, + } + : commit; + } finally { + await deleteTemporaryRef( + repositoryRoot, + options.runGit ?? runGitCommand, + temporaryRef, ); } - if (!(await hasMergeHead(repositoryRoot, runGit))) { - return { status: "upToDate", repositoryRoot, currentBranch, target }; +} + +function parseResolutionState(raw: string): ResolutionState | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; } - const commit = await commitMerge(repositoryRoot, runGit); - return typeof commit === "string" - ? { - status: "committed", - repositoryRoot, - currentBranch, - target, - commit, - } - : commit; + if ( + parsed === null || + typeof parsed !== "object" || + Array.isArray(parsed) + ) { + return undefined; + } + const candidate = parsed as Record; + if ( + candidate.version !== 1 || + typeof candidate.head !== "string" || + typeof candidate.mergeHead !== "string" || + !Array.isArray(candidate.conflicts) || + !candidate.conflicts.every((value) => typeof value === "string") || + !Array.isArray(candidate.stagedPaths) || + !candidate.stagedPaths.every((value) => typeof value === "string") + ) { + return undefined; + } + return { + version: 1, + head: candidate.head, + mergeHead: candidate.mergeHead, + conflicts: candidate.conflicts, + stagedPaths: candidate.stagedPaths, + }; } export async function completeMergeConflictResolution( options: MergeOptions = {}, ): Promise { const cwd = options.cwd ?? process.cwd(); - const runGit = options.runGit ?? runGitCommand; + const runGit = cancellableGit(options); const pathExists = options.pathExists ?? fs.existsSync; const readFile = options.readFile ?? ((filePath) => fs.readFileSync(filePath, "utf8")); const removeFile = options.removeFile ?? ((filePath) => fs.rmSync(filePath, { force: true })); + const repository = await getRepository(cwd, runGit); if ("status" in repository) { return repository; @@ -510,7 +634,8 @@ export async function completeMergeConflictResolution( repositoryRoot, runGit, ); - if (!(await hasMergeHead(repositoryRoot, runGit))) { + const parents = await getMergeParents(repositoryRoot, runGit); + if (parents === undefined) { return blocked( "noMergeInProgress", "There is no merge in progress to complete.", @@ -532,20 +657,28 @@ export async function completeMergeConflictResolution( true, ); } - let originalConflicts: string[]; + let raw: string; try { - const parsed: unknown = JSON.parse(readFile(resolutionStatePath)); - if ( - !Array.isArray(parsed) || - !parsed.every((value) => typeof value === "string") - ) { - throw new Error("Invalid conflict path list"); - } - originalConflicts = parsed; - } catch { + raw = readFile(resolutionStatePath); + } catch (error) { + return blocked( + "missingResolutionState", + `Could not read conflict-resolution state: ${String(error)}`, + true, + ); + } + const state = parseResolutionState(raw); + if (state === undefined) { return blocked( "missingResolutionState", - "Conflict-resolution state is invalid. Inspect the merge and commit or abort it manually.", + "Conflict-resolution state is invalid or from an older format. Inspect the merge and commit or abort it manually.", + true, + ); + } + if (parents.head !== state.head || parents.mergeHead !== state.mergeHead) { + return blocked( + "missingResolutionState", + "Conflict-resolution state does not match the current merge. Inspect the merge and commit or abort it manually.", true, ); } @@ -557,33 +690,20 @@ export async function completeMergeConflictResolution( true, ); } - const mergeBase = await runGit( - ["merge-base", "HEAD", "MERGE_HEAD"], - repositoryRoot, - ); - const allowedPaths = - mergeBase.exitCode === 0 - ? await listChangedPaths(repositoryRoot, runGit, [ - "diff", - "--name-only", - mergeBase.stdout.trim(), - "MERGE_HEAD", - ]) - : undefined; const stagedPaths = await listChangedPaths(repositoryRoot, runGit, [ "diff", "--cached", "--name-only", "HEAD", ]); - if (allowedPaths === undefined || stagedPaths === undefined) { + if (stagedPaths === undefined) { return blocked( "unrelatedChanges", "Git could not verify the staged merge paths.", true, ); } - const allowed = new Set([...allowedPaths, ...originalConflicts]); + const allowed = new Set([...state.stagedPaths, ...state.conflicts]); const unrelated = stagedPaths.filter((file) => !allowed.has(file)); if (unrelated.length > 0) { return blocked( diff --git a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts index 32dd0e0f2a..d7fb271860 100644 --- a/ts/packages/agents/github-cli/test/mergeConflict.spec.ts +++ b/ts/packages/agents/github-cli/test/mergeConflict.spec.ts @@ -15,6 +15,8 @@ import { } from "../src/github-cliActionHandler.js"; const ROOT = process.platform === "win32" ? "C:\\repo" : "/repo"; +const HEAD_SHA = "1111111111111111111111111111111111111111"; +const MERGE_HEAD_SHA = "2222222222222222222222222222222222222222"; function ok(stdout = ""): GitCommandResult { return { exitCode: 0, stdout, stderr: "" }; @@ -38,16 +40,19 @@ type RunnerOptions = { mergeInProgress?: boolean; unstaged?: boolean; stagedPaths?: string[]; - allowedPaths?: string[]; markers?: boolean; whitespaceErrors?: boolean; commit?: GitCommandResult; + head?: string; + mergeHead?: string; }; function createRunner(options: RunnerOptions = {}): { runGit: GitCommandRunner; calls: string[][]; } { + const head = options.head ?? HEAD_SHA; + const mergeHead = options.mergeHead ?? MERGE_HEAD_SHA; const calls: string[][] = []; const runGit: GitCommandRunner = async (args) => { calls.push([...args]); @@ -75,16 +80,16 @@ function createRunner(options: RunnerOptions = {}): { return options.mainExists ? ok("abc\trefs/heads/main\n") : fail(); - case key(["rev-parse", "--verify", "MERGE_HEAD"]): - return options.mergeInProgress === false ? fail() : ok("abc"); + case key(["rev-parse", "HEAD", "MERGE_HEAD"]): + return options.mergeInProgress === false + ? fail() + : ok(`${head}\n${mergeHead}`); + case key(["rev-parse", "HEAD"]): + return ok(head); case key(["diff", "--name-only", "--diff-filter=U", "-z"]): return ok((options.conflicts ?? []).join("\0")); case key(["diff", "--quiet"]): return options.unstaged ? fail() : ok(); - case key(["merge-base", "HEAD", "MERGE_HEAD"]): - return ok("base"); - case key(["diff", "--name-only", "base", "MERGE_HEAD", "-z"]): - return ok((options.allowedPaths ?? ["src/a.ts"]).join("\0")); case key(["diff", "--cached", "--name-only", "HEAD", "-z"]): return ok((options.stagedPaths ?? ["src/a.ts"]).join("\0")); case key(["diff", "--cached", "--check"]): @@ -95,8 +100,6 @@ function createRunner(options: RunnerOptions = {}): { : ok(); case key(["commit", "--no-edit"]): return options.commit ?? ok(); - case key(["rev-parse", "HEAD"]): - return ok("0123456789abcdef"); default: if (args[0] === "rev-parse" && args[1] === "--git-path") { return ok(`.git/${args[2]}`); @@ -108,15 +111,10 @@ function createRunner(options: RunnerOptions = {}): { return ok(); } if ( - args[0] === "rev-parse" && - args[1] === "--verify" && - args[2]?.startsWith("refs/typeagent/merge/") - ) { - return ok("fetched-commit"); - } - if ( - key(args) === - key(["merge", "--no-commit", "--no-ff", "fetched-commit"]) + args[0] === "merge" && + args[1] === "--no-commit" && + args[2] === "--no-ff" && + args[3]?.startsWith("refs/typeagent/merge/") ) { return options.merge ?? ok(); } @@ -136,10 +134,27 @@ function createRunner(options: RunnerOptions = {}): { return { runGit, calls }; } +function stateFixture( + overrides: { + head?: string; + mergeHead?: string; + conflicts?: string[]; + stagedPaths?: string[]; + } = {}, +): string { + return JSON.stringify({ + version: 1, + head: overrides.head ?? HEAD_SHA, + mergeHead: overrides.mergeHead ?? MERGE_HEAD_SHA, + conflicts: overrides.conflicts ?? ["src/a.ts"], + stagedPaths: overrides.stagedPaths ?? ["src/a.ts"], + }); +} + const resolutionState = { pathExists: (filePath: string) => filePath.includes("TYPEAGENT_MERGE_CONFLICTS"), - readFile: () => JSON.stringify(["src/a.ts"]), + readFile: () => stateFixture(), removeFile: () => {}, }; @@ -156,13 +171,13 @@ describe("mergeAndCommit", () => { status: "committed", currentBranch: "feature/work", target: { displayName: "origin/main" }, - commit: "0123456789abcdef", + commit: HEAD_SHA, }); expect(calls).toContainEqual([ "merge", "--no-commit", "--no-ff", - "fetched-commit", + expect.stringMatching(/^refs\/typeagent\/merge\//), ]); expect(calls).toContainEqual(["commit", "--no-edit"]); expect(calls.some(([command]) => command === "push")).toBe(false); @@ -246,16 +261,20 @@ describe("mergeAndCommit", () => { expect(calls.some(([command]) => command === "fetch")).toBe(false); }); - test("returns conflicted paths for Reasoning instead of committing", async () => { + test("returns conflicted paths for Reasoning and persists v1 state", async () => { + let stored: { path: string; content: string } | undefined; const { runGit, calls } = createRunner({ merge: fail("CONFLICT"), conflicts: ["src/a.ts", "src/b.ts"], + stagedPaths: ["src/a.ts", "src/b.ts", "src/autoMerged.ts"], }); const result = await mergeAndCommit("main", { cwd: ROOT, runGit, pathExists: () => false, - writeFile: () => {}, + writeFile: (path, content) => { + stored = { path, content }; + }, }); expect(result).toMatchObject({ @@ -264,6 +283,136 @@ describe("mergeAndCommit", () => { }); expect(calls.some(([command]) => command === "commit")).toBe(false); expect(calls.some(([command]) => command === "push")).toBe(false); + + expect(stored).toBeDefined(); + expect(stored!.path).toContain("TYPEAGENT_MERGE_CONFLICTS"); + const parsed = JSON.parse(stored!.content); + expect(parsed).toEqual({ + version: 1, + head: HEAD_SHA, + mergeHead: MERGE_HEAD_SHA, + conflicts: ["src/a.ts", "src/b.ts"], + // Snapshot includes the auto-merged rename target that the + // previous diff-based reconstruction would have missed. + stagedPaths: ["src/a.ts", "src/b.ts", "src/autoMerged.ts"], + }); + }); + + test("cleans up the temporary fetch ref even when the merge fails", async () => { + const { runGit, calls } = createRunner({ + merge: fail("CONFLICT"), + conflicts: ["src/a.ts"], + }); + await mergeAndCommit("main", { + cwd: ROOT, + runGit, + pathExists: () => false, + writeFile: () => {}, + }); + expect( + calls.some( + ([command, flag, ref]) => + command === "update-ref" && + flag === "-d" && + ref?.startsWith("refs/typeagent/merge/"), + ), + ).toBe(true); + }); + + test("propagates AbortError when the signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(); + const { runGit, calls } = createRunner(); + await expect( + mergeAndCommit("main", { + cwd: ROOT, + runGit, + pathExists: () => false, + signal: controller.signal, + }), + ).rejects.toMatchObject({ name: "AbortError" }); + // Never even reached repo detection. + expect(calls.length).toBe(0); + }); + + test.each(["interrupted", "just completed"])( + "cancelling a %s fetch prevents merge and commit and cleans up", + async (fetchState) => { + const controller = new AbortController(); + const { runGit, calls } = createRunner(); + const cancelledRunner: GitCommandRunner = async ( + args, + cwd, + signal, + ) => { + if (args[0] === "fetch") { + expect(signal).toBe(controller.signal); + controller.abort(); + if (fetchState === "interrupted") { + throw new DOMException( + "The operation was aborted.", + "AbortError", + ); + } + return ok(); + } + if (args[0] === "update-ref" && args[1] === "-d") { + expect(signal).toBeUndefined(); + } + return runGit(args, cwd, signal); + }; + await expect( + mergeAndCommit("main", { + cwd: ROOT, + runGit: cancelledRunner, + pathExists: () => false, + signal: controller.signal, + }), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(calls.some(([command]) => command === "merge")).toBe(false); + expect(calls.some(([command]) => command === "commit")).toBe(false); + expect( + calls.some( + ([command, flag]) => + command === "update-ref" && flag === "-d", + ), + ).toBe(true); + }, + ); + + test("reports failed ref cleanup without masking cancellation", async () => { + const controller = new AbortController(); + const { runGit } = createRunner(); + const originalWarn = console.warn; + const warnings: string[] = []; + console.warn = (message: string) => warnings.push(message); + try { + await expect( + mergeAndCommit("main", { + cwd: ROOT, + pathExists: () => false, + signal: controller.signal, + runGit: async (args, cwd, signal) => { + if (args[0] === "fetch") { + controller.abort(); + throw new DOMException("Cancelled", "AbortError"); + } + if (args[0] === "update-ref") { + expect(signal).toBeUndefined(); + return fail("cannot lock ref"); + } + return runGit(args, cwd, signal); + }, + }), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(warnings).toEqual([ + expect.stringMatching( + /Could not remove temporary merge ref .*cannot lock ref/, + ), + ]); + } finally { + console.warn = originalWarn; + } }); }); @@ -301,7 +450,6 @@ describe("completeMergeConflictResolution", () => { test("refuses to commit staged changes unrelated to the merge", async () => { const { runGit } = createRunner({ - allowedPaths: ["src/a.ts"], stagedPaths: ["src/a.ts", "notes.txt"], }); const result = await completeMergeConflictResolution({ @@ -328,44 +476,97 @@ describe("completeMergeConflictResolution", () => { }); }); - test("allows a resolved rename recorded in conflict state", async () => { - const { runGit } = createRunner({ - allowedPaths: ["src/old.ts"], - stagedPaths: ["src/new.ts"], - }); + test("does not mistake incoming whitespace errors for conflict markers", async () => { + const { runGit } = createRunner({ whitespaceErrors: true }); const result = await completeMergeConflictResolution({ cwd: ROOT, runGit, ...resolutionState, - readFile: () => JSON.stringify(["src/new.ts"]), }); expect(result).toMatchObject({ status: "committed" }); }); - test("does not mistake incoming whitespace errors for conflict markers", async () => { - const { runGit } = createRunner({ whitespaceErrors: true }); + test("commits a fully resolved and staged merge without pushing", async () => { + const { runGit, calls } = createRunner(); const result = await completeMergeConflictResolution({ cwd: ROOT, runGit, ...resolutionState, }); - expect(result).toMatchObject({ status: "committed" }); + + expect(result).toMatchObject({ + status: "committed", + commit: HEAD_SHA, + }); + expect(calls).toContainEqual(["commit", "--no-edit"]); + expect(calls.some(([command]) => command === "push")).toBe(false); }); - test("commits a fully resolved and staged merge without pushing", async () => { + test.each([ + ["legacy format", JSON.stringify(["src/a.ts"])], + ["invalid JSON", "not json"], + ["changed HEAD", stateFixture({ head: "different-commit" })], + ["changed MERGE_HEAD", stateFixture({ mergeHead: "different-commit" })], + ])("rejects resolution state with %s", async (_description, raw) => { const { runGit, calls } = createRunner(); const result = await completeMergeConflictResolution({ cwd: ROOT, runGit, ...resolutionState, + readFile: () => raw, }); + expect(result).toMatchObject({ + status: "blocked", + errorCode: "missingResolutionState", + mayHaveSideEffects: true, + }); + expect(calls.some(([command]) => command === "commit")).toBe(false); + }); + test("surfaces a state-read failure instead of silently proceeding", async () => { + const { runGit, calls } = createRunner(); + const result = await completeMergeConflictResolution({ + cwd: ROOT, + runGit, + ...resolutionState, + readFile: () => { + throw new Error("EACCES"); + }, + }); expect(result).toMatchObject({ - status: "committed", - commit: "0123456789abcdef", + status: "blocked", + errorCode: "missingResolutionState", + mayHaveSideEffects: true, }); - expect(calls).toContainEqual(["commit", "--no-edit"]); - expect(calls.some(([command]) => command === "push")).toBe(false); + expect(result).toMatchObject({ + message: expect.stringContaining("EACCES"), + }); + expect(calls.some(([command]) => command === "commit")).toBe(false); + }); + + test("cancellation after index checks prevents committing or removing state", async () => { + const controller = new AbortController(); + const { runGit, calls } = createRunner(); + let removed = false; + await expect( + completeMergeConflictResolution({ + cwd: ROOT, + runGit: async (args, cwd, signal) => { + const result = await runGit(args, cwd, signal); + if (key(args) === key(["diff", "--cached", "--check"])) { + controller.abort(); + } + return result; + }, + ...resolutionState, + removeFile: () => { + removed = true; + }, + signal: controller.signal, + }), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(calls.some(([command]) => command === "commit")).toBe(false); + expect(removed).toBe(false); }); }); diff --git a/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts b/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts index 2aafa133f5..43495a8bbe 100644 --- a/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts +++ b/ts/packages/agents/github-cli/test/mergeConflictIntegration.spec.ts @@ -5,21 +5,58 @@ import { execFileSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import type { + ActionContext, + ActionIO, + SessionContext, + TypeAgentAction, +} from "@typeagent/agent-sdk"; +import type { GithubCliActions } from "../src/github-cliSchema.js"; +import { instantiate } from "../src/github-cliActionHandler.js"; import { completeMergeConflictResolution, mergeAndCommit, } from "../src/mergeConflict.js"; +function gitAt(cwd: string, ...args: string[]): string { + return execFileSync("git", args, { + cwd, + encoding: "utf8", + windowsHide: true, + stdio: ["ignore", "pipe", "pipe"], + }).trim(); +} + +function executeMergeAction( + action: TypeAgentAction, + workingDirectory?: string, + abortSignal?: AbortSignal, +) { + const context: ActionContext = { + get actionIO(): ActionIO { + throw new Error( + "Merge actions should return their display content", + ); + }, + get sessionContext(): SessionContext { + throw new Error( + "Merge actions should not require agent session state", + ); + }, + activityContext: undefined, + isFromReasoningLoop: false, + queueToggleTransientAgent: async () => {}, + streamingContext: undefined, + workingDirectory, + abortSignal, + }; + return instantiate().executeAction!(action, context); +} + describe("merge conflict resolution with real Git", () => { let repository: string; const file = "source file.ts"; - const git = (...args: string[]) => - execFileSync("git", args, { - cwd: repository, - encoding: "utf8", - windowsHide: true, - stdio: ["ignore", "pipe", "pipe"], - }).trim(); + const git = (...args: string[]) => gitAt(repository, ...args); beforeEach(() => { repository = fs.mkdtempSync( @@ -115,4 +152,163 @@ describe("merge conflict resolution with real Git", () => { }); expect(git("rev-parse", "HEAD")).toBe(before); }); + + test("allows an auto-merged rename plus a separate conflict resolution", async () => { + // Git applies main's edit through the feature branch's rename. + const renamed = "src/renamed.ts"; + const original = "src/original.ts"; + const conflictFile = "src/conflict.txt"; + + git("checkout", "main"); + fs.mkdirSync(path.join(repository, "src"), { recursive: true }); + fs.writeFileSync( + path.join(repository, original), + "export const value = 1;\n", + ); + fs.writeFileSync(path.join(repository, conflictFile), "shared line\n"); + git("add", "--", original, conflictFile); + git("commit", "-m", "add rename base"); + + git("checkout", "-b", "rename-feature"); + fs.renameSync( + path.join(repository, original), + path.join(repository, renamed), + ); + fs.writeFileSync(path.join(repository, conflictFile), "feature line\n"); + git("add", "--all"); + git("commit", "-m", "feature rename"); + + git("checkout", "main"); + fs.writeFileSync( + path.join(repository, original), + "export const value = 2;\n", + ); + fs.writeFileSync(path.join(repository, conflictFile), "main line\n"); + git("add", "--", original, conflictFile); + git("commit", "-m", "main change"); + + git("checkout", "rename-feature"); + const before = git("rev-parse", "HEAD"); + + const started = await mergeAndCommit("main", { cwd: repository }); + expect(started).toMatchObject({ + status: "conflicts", + conflicts: [conflictFile], + }); + expect( + git("diff", "--cached", "--name-only", "HEAD").split("\n"), + ).toContain(renamed); + + fs.writeFileSync( + path.join(repository, conflictFile), + "feature line\nmain line\n", + ); + git("add", "--", conflictFile); + + const completed = await completeMergeConflictResolution({ + cwd: repository, + }); + expect(completed).toMatchObject({ status: "committed" }); + expect(git("status", "--porcelain")).toBe(""); + expect(git("rev-parse", "HEAD")).not.toBe(before); + expect(fs.existsSync(path.join(repository, ".git", "MERGE_HEAD"))).toBe( + false, + ); + expect( + fs.existsSync( + path.join(repository, ".git", "TYPEAGENT_MERGE_CONFLICTS"), + ), + ).toBe(false); + }); + + test("both actions honor the request repository rather than the server cwd or action parameter", async () => { + const request = fs.mkdtempSync( + path.join(os.tmpdir(), "typeagent-merge-request-"), + ); + const originalCwd = process.cwd(); + const serverHead = git("rev-parse", "HEAD"); + const requestGit = (...args: string[]) => gitAt(request, ...args); + try { + git("clone", "--quiet", repository, request); + requestGit("config", "user.name", "Merge test"); + requestGit("config", "user.email", "merge-test@example.invalid"); + requestGit("config", "commit.gpgsign", "false"); + requestGit( + "config", + "core.hooksPath", + path.join(request, "no-hooks"), + ); + process.chdir(repository); + expect( + await executeMergeAction( + { + schemaName: "github-cli", + actionName: "resolveMergeConflicts", + parameters: { targetBranch: "main" }, + }, + request, + ), + ).toMatchObject({ resultValue: { status: "conflicts" } }); + + fs.writeFileSync( + path.join(request, file), + "export const resolved = 1;\n", + ); + requestGit("add", "--", file); + expect( + await executeMergeAction( + { + schemaName: "github-cli", + actionName: "completeMergeConflictResolution", + parameters: { repositoryRoot: repository }, + }, + request, + ), + ).toMatchObject({ resultValue: { status: "committed" } }); + expect( + requestGit("show", "-s", "--format=%P", "HEAD").split(" "), + ).toEqual([serverHead, git("rev-parse", "main")]); + expect(requestGit("status", "--porcelain")).toBe(""); + expect(git("rev-parse", "HEAD")).toBe(serverHead); + expect(git("status", "--porcelain")).toBe(""); + expect( + fs.existsSync(path.join(repository, ".git", "MERGE_HEAD")), + ).toBe(false); + } finally { + process.chdir(originalCwd); + fs.rmSync(request, { recursive: true, force: true }); + } + }); + + test.each([ + { + actionName: "resolveMergeConflicts", + parameters: { targetBranch: "main" }, + }, + { + actionName: "completeMergeConflictResolution", + parameters: { repositoryRoot: "not-authorized" }, + }, + ] as const)( + "$actionName rejects missing request context and propagates cancellation", + async (action) => { + const requestAction = { ...action, schemaName: "github-cli" }; + expect(await executeMergeAction(requestAction)).toMatchObject({ + errorCode: "notRepository", + }); + const controller = new AbortController(); + controller.abort(); + await expect( + executeMergeAction( + requestAction, + repository, + controller.signal, + ), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(git("status", "--porcelain")).toBe(""); + expect( + fs.existsSync(path.join(repository, ".git", "MERGE_HEAD")), + ).toBe(false); + }, + ); }); diff --git a/ts/packages/dispatcher/dispatcher/README.md b/ts/packages/dispatcher/dispatcher/README.md index fc0f06c4a2..2b9fab7154 100644 --- a/ts/packages/dispatcher/dispatcher/README.md +++ b/ts/packages/dispatcher/dispatcher/README.md @@ -11,6 +11,22 @@ Dispatcher processes user requests and asks LLM to translate it into an action b See [dispatcher architecture](../../../docs/content/architecture/core/dispatcher.md) doc for more details on the design of the dispatcher component. +## Copilot SDK upgrade check + +After changing the Copilot SDK or native tool allowlist, run these commands +from `ts`: + +```powershell +pnpm --filter agent-dispatcher run tsc +pnpm --filter agent-dispatcher run test:live +``` + +This starts the installed Copilot runtime and checks the initialized session's +tools for Claude and GPT models, including platform-specific shell output and +cancellation tools. It sends no model prompt and executes no tools. Unlike the +unit tests, it detects names that no longer match the actual runtime. It requires +the SDK's bundled CLI to be available and able to start. + ## Usage - Natural Language Requests User can request actions provided by [application agents](../agentSdk/README.md) using natural language. diff --git a/ts/packages/dispatcher/dispatcher/package.json b/ts/packages/dispatcher/dispatcher/package.json index afa3408afd..5626902ffb 100644 --- a/ts/packages/dispatcher/dispatcher/package.json +++ b/ts/packages/dispatcher/dispatcher/package.json @@ -38,6 +38,7 @@ "prettier:fix": "prettier --write . --ignore-path ../../../.prettierignore", "test": "npm run test:local", "test:local": "pnpm run jest-esm --testPathPattern=\".*[.]spec[.]js\"", + "test:live": "pnpm run jest-esm --testPathPattern=\".*[.]test[.]js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" }, diff --git a/ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.test.ts b/ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.test.ts new file mode 100644 index 0000000000..fdeffc74e5 --- /dev/null +++ b/ts/packages/dispatcher/dispatcher/test/copilotAvailableTools.test.ts @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import fs from "node:fs"; +import { createRequire } from "node:module"; +import os from "node:os"; +import path from "node:path"; +import { buildCopilotAvailableTools } from "../src/reasoning/copilot.js"; + +// Jest's ESM VM lacks import.meta.resolve; use the SDK's CommonJS entry so its +// bundled CLI resolver can use Node's native module resolution. +const { CopilotClient }: typeof import("@github/copilot-sdk") = createRequire( + import.meta.url, +)("@github/copilot-sdk"); + +describe("installed Copilot runtime tool contract", () => { + const client = new CopilotClient(); + let workingDirectory: string; + + beforeAll(async () => { + workingDirectory = fs.mkdtempSync( + path.join(os.tmpdir(), "typeagent-tool-contract-"), + ); + await client.start(); + }); + + afterAll(async () => { + try { + expect(await client.stop()).toEqual([]); + } finally { + fs.rmSync(workingDirectory, { recursive: true, force: true }); + } + }); + + test.each([ + ["claude-opus-4.8", ["view", "edit", "create", "grep"]], + ["gpt-5.4", ["view", "apply_patch", "rg"]], + ] as const)("%s exposes usable native tools", async (model, editors) => { + const session = await client.createSession({ + model, + workingDirectory, + availableTools: buildCopilotAvailableTools({ + subagentsEnabled: false, + }), + onPermissionRequest: () => ({ + kind: "denied-no-approval-rule-and-could-not-request-from-user", + }), + }); + try { + // Unlike the static global listing, this applies the host platform, + // model overrides and our allowlist. No model request is sent. + await session.rpc.tools.initializeAndValidate(); + const { tools } = await session.rpc.tools.getCurrentMetadata(); + expect(tools).not.toBeNull(); + const names = tools?.map((tool) => tool.name); + const shell = process.platform === "win32" ? "powershell" : "bash"; + expect(names).toEqual( + expect.arrayContaining([ + ...editors, + "glob", + "web_fetch", + shell, + `read_${shell}`, + `stop_${shell}`, + `list_${shell}`, + ]), + ); + for (const excluded of ["task", "run_factory", "skill"]) { + expect(names).not.toContain(excluded); + } + } finally { + await client.deleteSession(session.sessionId); + } + }); +}); From cca6ab21bd8b93eb598e4a86380d13c1c4bf12b0 Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Sat, 5 Sep 2026 05:43:53 +0000 Subject: [PATCH 7/7] style: apply prettier formatting and policy fixes --- ts/packages/dispatcher/dispatcher/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ts/packages/dispatcher/dispatcher/package.json b/ts/packages/dispatcher/dispatcher/package.json index 5626902ffb..104ed4d827 100644 --- a/ts/packages/dispatcher/dispatcher/package.json +++ b/ts/packages/dispatcher/dispatcher/package.json @@ -37,8 +37,8 @@ "prettier": "prettier --check . --ignore-path ../../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../../.prettierignore", "test": "npm run test:local", - "test:local": "pnpm run jest-esm --testPathPattern=\".*[.]spec[.]js\"", "test:live": "pnpm run jest-esm --testPathPattern=\".*[.]test[.]js\"", + "test:local": "pnpm run jest-esm --testPathPattern=\".*[.]spec[.]js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" },