Skip to content

Commit bb2a018

Browse files
dsymeCopilot
andauthored
Fix apply_samples cross-repo subdirectory patch staging (#40086) (#40087)
* fix: stage cross-repo subdirectory sample patches in the right checkout (#40086) apply_samples.cjs always created the sample branch and committed the patch in the main workspace root. For a cross-repo checkout placed in a subdirectory (path: github), the safe-outputs MCP handler resolves the target repo to the subdirectory via the checkout manifest and looks for the branch there, so it failed with 'fatal: Needed a single revision'. Resolve the patch-staging directory using the same manifest-first findRepoCheckout the MCP handler uses, driven by the sample's arguments.repo override or the configured target-repo. Falls back to the workspace root when no target repo is set or the checkout cannot be located. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent b2f42c7 commit bb2a018

3 files changed

Lines changed: 174 additions & 8 deletions

File tree

.changeset/patch-apply-samples-cross-repo-subdir-staging.md

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

actions/setup/js/apply_samples.cjs

Lines changed: 96 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
// GH_AW_SAMPLES — JSON array of replay entries (required)
2424
// GH_AW_AGENT_STDIO_LOG — path where the synthetic stdio log is written
2525
// GH_AW_SAFE_OUTPUTS_CONFIG_PATH — path to the MCP server's config.json
26+
// (also read to resolve the per-tool target-repo so
27+
// cross-repo patches are staged in the right checkout)
2628
// GH_AW_SAFE_OUTPUTS — path to the MCP server's outputs.jsonl
2729
// GITHUB_WORKSPACE — git working directory for pre-staging (optional;
2830
// falls back to cwd)
@@ -35,6 +37,7 @@ const path = require("path");
3537
const os = require("os");
3638
const { getErrorMessage } = require("./error_helpers.cjs");
3739
const { ERR_VALIDATION, ERR_PARSE, ERR_SYSTEM, ERR_API, ERR_CONFIG } = require("./error_codes.cjs");
40+
const { findRepoCheckout } = require("./find_repo_checkout.cjs");
3841

3942
const DEFAULT_BASE_BRANCH = process.env.GH_AW_CUSTOM_BASE_BRANCH || process.env.GITHUB_BASE_REF || process.env.GITHUB_REF_NAME || "main";
4043
const PATCH_SIDECAR_TOOLS = new Set(["create_pull_request", "push_to_pull_request_branch"]);
@@ -243,6 +246,84 @@ async function derivePrHeadRef(entry) {
243246
return null;
244247
}
245248

249+
/**
250+
* Read the configured `target-repo` for a given safe-output tool from the
251+
* safe-outputs config file (GH_AW_SAFE_OUTPUTS_CONFIG_PATH). Returns an empty
252+
* string when no config is available or no target-repo is configured.
253+
* @param {string} tool
254+
* @returns {string}
255+
*/
256+
function readConfiguredTargetRepo(tool) {
257+
const configPath = process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH;
258+
if (!configPath || !configPath.trim()) {
259+
return "";
260+
}
261+
262+
const toolKey = typeof tool === "string" ? tool.replace(/-/g, "_") : "";
263+
264+
try {
265+
const raw = fs.readFileSync(configPath, "utf8");
266+
const parsed = JSON.parse(raw);
267+
268+
// Mirror safe_outputs_config.cjs behavior: normalize top-level keys by replacing '-' with '_'.
269+
const config = parsed && typeof parsed === "object" ? Object.fromEntries(Object.entries(parsed).map(([k, v]) => [String(k).replace(/-/g, "_"), v])) : {};
270+
271+
const toolConfig = toolKey && config && typeof config === "object" ? config[toolKey] : null;
272+
const target = toolConfig && typeof toolConfig === "object" ? toolConfig["target-repo"] || toolConfig["target_repo"] : null;
273+
if (typeof target === "string") {
274+
return target.trim();
275+
}
276+
} catch (err) {
277+
core.debug(`apply_samples: could not read target-repo from ${configPath}: ${getErrorMessage(err)}`);
278+
}
279+
return "";
280+
}
281+
282+
/**
283+
* Resolve the on-disk working directory in which a sample's patch should be
284+
* staged (branch created + patch committed).
285+
*
286+
* For cross-repo checkouts placed in a subdirectory (e.g. `path: github`), the
287+
* branch and commit must be created inside that subdirectory so the safe-outputs
288+
* MCP handler — which resolves the same checkout via the checkout manifest — can
289+
* find the branch. Staging in the main workspace root instead leaves the branch
290+
* invisible to the MCP server, producing "fatal: Needed a single revision" when
291+
* it tries to pin the branch (issue #40086).
292+
*
293+
* Resolution mirrors the MCP handler: the target repo comes from the sample's
294+
* `arguments.repo` override or the configured `target-repo`, and the checkout
295+
* directory is resolved via the manifest-first `findRepoCheckout`. Falls back to
296+
* the workspace root when no target repo is set or the checkout cannot be located.
297+
*
298+
* @param {SampleEntry} entry
299+
* @param {string} workspace
300+
* @returns {string}
301+
*/
302+
function resolvePatchWorkspace(entry, workspace) {
303+
let targetRepo = "";
304+
if (entry.arguments && typeof entry.arguments.repo === "string" && entry.arguments.repo.trim()) {
305+
targetRepo = entry.arguments.repo.trim();
306+
} else {
307+
targetRepo = readConfiguredTargetRepo(entry.tool);
308+
}
309+
if (!targetRepo) {
310+
return workspace;
311+
}
312+
try {
313+
const result = findRepoCheckout(targetRepo, workspace);
314+
if (result && result.success && result.path) {
315+
if (path.resolve(result.path) !== path.resolve(workspace)) {
316+
core.info(`apply_samples: staging patch for ${targetRepo} in checkout subdirectory ${result.path}`);
317+
}
318+
return result.path;
319+
}
320+
core.debug(`apply_samples: findRepoCheckout(${targetRepo}) did not locate a checkout; using workspace root`);
321+
} catch (err) {
322+
core.debug(`apply_samples: findRepoCheckout(${targetRepo}) failed: ${getErrorMessage(err)}; using workspace root`);
323+
}
324+
return workspace;
325+
}
326+
246327
/**
247328
* Pre-stage a branch + patch for samples whose tool reads the workspace diff.
248329
*
@@ -264,6 +345,11 @@ async function preStagePatch(entry, index, workspace) {
264345
return;
265346
}
266347

348+
// Resolve the directory in which to create the branch and commit the patch.
349+
// For cross-repo checkouts in a subdirectory this is the checkout subdir, not
350+
// the main workspace root (issue #40086).
351+
const repoCwd = resolvePatchWorkspace(entry, workspace);
352+
267353
let branch;
268354
if (entry.tool === "push_to_pull_request_branch") {
269355
// Source ref MUST match the PR's head ref so that
@@ -286,36 +372,36 @@ async function preStagePatch(entry, index, workspace) {
286372
entry.arguments.branch = branch;
287373
}
288374

289-
ensureGitIdentity(workspace);
375+
ensureGitIdentity(repoCwd);
290376

291377
// Start from the base branch so the diff is meaningful. Tolerate the case
292378
// where the base ref doesn't exist locally — fall back to HEAD.
293379
try {
294-
runGit(["checkout", DEFAULT_BASE_BRANCH], workspace);
380+
runGit(["checkout", DEFAULT_BASE_BRANCH], repoCwd);
295381
} catch (err) {
296382
core.warning(`apply_samples: could not check out base branch ${DEFAULT_BASE_BRANCH}: ${getErrorMessage(err)}; staying on current HEAD`);
297383
}
298384

299385
// Create the branch (or check it out if it already exists from a previous sample).
300386
try {
301-
runGit(["checkout", "-b", branch], workspace);
387+
runGit(["checkout", "-b", branch], repoCwd);
302388
} catch {
303-
runGit(["checkout", branch], workspace);
389+
runGit(["checkout", branch], repoCwd);
304390
}
305391

306392
// Write patch to a temp file and apply it.
307393
const tmpPatch = path.join(os.tmpdir(), `gh-aw-sample-${index + 1}.patch`);
308394
fs.writeFileSync(tmpPatch, patch.endsWith("\n") ? patch : patch + "\n");
309395
try {
310-
runGit(["apply", "--whitespace=nowarn", tmpPatch], workspace);
396+
runGit(["apply", "--whitespace=nowarn", tmpPatch], repoCwd);
311397
} catch (err) {
312398
// Fall back to --3way for patches that don't apply cleanly on top of an
313399
// empty working tree (uncommon but possible for synthetic samples).
314-
runGit(["apply", "--3way", "--whitespace=nowarn", tmpPatch], workspace);
400+
runGit(["apply", "--3way", "--whitespace=nowarn", tmpPatch], repoCwd);
315401
}
316402

317-
runGit(["add", "-A"], workspace);
318-
runGit(["commit", "-m", `gh-aw sample ${index + 1}: ${entry.tool}`, "--allow-empty"], workspace);
403+
runGit(["add", "-A"], repoCwd);
404+
runGit(["commit", "-m", `gh-aw sample ${index + 1}: ${entry.tool}`, "--allow-empty"], repoCwd);
319405
}
320406

321407
/**
@@ -561,6 +647,8 @@ module.exports = {
561647
main,
562648
loadSamples,
563649
preStagePatch,
650+
resolvePatchWorkspace,
651+
readConfiguredTargetRepo,
564652
resolveMcpServerPath,
565653
selectTokenForRepo,
566654
sendJsonRpc,

actions/setup/js/apply_samples.test.cjs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,79 @@ describe("apply_samples.cjs preStagePatch (create_pull_request / push_to_pull_re
326326
expect(diff).toContain("+hello from a deterministic sample");
327327
});
328328

329+
it("stages the patch in the cross-repo checkout subdirectory (path: github) (issue #40086)", async () => {
330+
// Reproduce the failing layout: a main repo at the workspace root and a
331+
// side-repo checked out into a `github/` subdirectory. The checkout manifest
332+
// maps the target repo to path "github", so preStagePatch must create the
333+
// branch + commit inside ${workspace}/github — not the main repo root —
334+
// otherwise the safe-outputs MCP server cannot pin the branch.
335+
const workspace = makeTempDir("gh-aw-prestage-subdir-");
336+
initRepo(workspace, "main");
337+
338+
const subdir = path.join(workspace, "github");
339+
fs.mkdirSync(subdir);
340+
initRepo(subdir, "main");
341+
342+
const manifestPath = path.join(workspace, "checkout-manifest.json");
343+
fs.writeFileSync(
344+
manifestPath,
345+
JSON.stringify({
346+
"githubnext/gh-aw-side-repo": {
347+
repository: "githubnext/gh-aw-side-repo",
348+
path: "github",
349+
default_branch: "main",
350+
},
351+
})
352+
);
353+
354+
const configPath = path.join(workspace, "config.json");
355+
fs.writeFileSync(
356+
configPath,
357+
JSON.stringify({
358+
create_pull_request: { "target-repo": "githubnext/gh-aw-side-repo" },
359+
})
360+
);
361+
362+
const branchName = "gh-aw-sample-copilot-siderepo-subdir-pr";
363+
const fileToAdd = "subdir-notes.md";
364+
const entry = {
365+
tool: "create_pull_request",
366+
arguments: { title: "Subdir PR", body: "body", branch: branchName },
367+
sidecars: { patch: newFileDiff(fileToAdd, "side repo content\n") },
368+
};
369+
370+
// Reset the checkout-manifest module cache so our manifest is loaded fresh.
371+
require("./checkout_manifest.cjs")._resetCache();
372+
373+
const prevBase = process.env.GH_AW_CUSTOM_BASE_BRANCH;
374+
const prevManifest = process.env.GH_AW_CHECKOUT_MANIFEST;
375+
const prevConfig = process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH;
376+
process.env.GH_AW_CUSTOM_BASE_BRANCH = "main";
377+
process.env.GH_AW_CHECKOUT_MANIFEST = manifestPath;
378+
process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH = configPath;
379+
try {
380+
await preStagePatch(entry, 0, workspace);
381+
} finally {
382+
require("./checkout_manifest.cjs")._resetCache();
383+
if (prevBase === undefined) delete process.env.GH_AW_CUSTOM_BASE_BRANCH;
384+
else process.env.GH_AW_CUSTOM_BASE_BRANCH = prevBase;
385+
if (prevManifest === undefined) delete process.env.GH_AW_CHECKOUT_MANIFEST;
386+
else process.env.GH_AW_CHECKOUT_MANIFEST = prevManifest;
387+
if (prevConfig === undefined) delete process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH;
388+
else process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH = prevConfig;
389+
}
390+
391+
// The branch + commit + file land in the side-repo subdirectory...
392+
expect(git(["rev-parse", "--abbrev-ref", "HEAD"], subdir).trim()).toBe(branchName);
393+
expect(git(["branch", "--list", branchName], subdir)).toContain(branchName);
394+
expect(fs.existsSync(path.join(subdir, fileToAdd))).toBe(true);
395+
396+
// ...and NOT in the main repo root (which stays on its seed branch).
397+
expect(git(["rev-parse", "--abbrev-ref", "HEAD"], workspace).trim()).toBe("main");
398+
expect(git(["branch", "--list", branchName], workspace).trim()).toBe("");
399+
expect(fs.existsSync(path.join(workspace, fileToAdd))).toBe(false);
400+
});
401+
329402
it("derives push_to_pull_request_branch branch from pull_request event payload", async () => {
330403
const workspace = makeTempDir("gh-aw-prestage-push-pr-");
331404
initRepo(workspace, "main");

0 commit comments

Comments
 (0)