diff --git a/AGENTS.md b/AGENTS.md index b924c5f0..ad2eca86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -266,6 +266,7 @@ fail-closed and only pauses when the agent actually proposed a reviewed output. │ ├── approval-summary/ # Safe-outputs summary renderer (bundled to approval-summary.js; end-of-Agent-job summary tab) │ ├── github-app-token/ # GitHub App token minter (bundled to github-app-token.js; mints installation token in Agent + Detection when engine.github-app-token is set) │ ├── executor-e2e/ # Stage 3 safe-output E2E test harness (not a bundle; runs deterministic scenarios against a real ADO project and files a GitHub issue on failure) +│ ├── prepare-pr-base/ # create-pull-request base-ref preparer (bundled to prepare-pr-base.js; fetches/deepens target branch in the Agent job so mcp.rs finds a diff base on shallow-default pools — issue #1413) │ └── shared/ # Shared modules across bundles (auth, ado-client, env-facts, types.gen.ts) ├── tests/ # Integration tests and fixtures ├── docs/ # Per-concept reference documentation (see index below) @@ -381,8 +382,8 @@ index to jump to the right page. - [`docs/ado-script.md`](docs/ado-script.md) — `ado-script` workspace (`scripts/ado-script/`): the bundled TypeScript runtime helpers (`gate.js`, `import.js`, the execution-context `exec-context-*.js` - bundles, `conclusion.js`, `approval-summary.js`, and - `github-app-token.js`), schemars-driven + bundles, `conclusion.js`, `approval-summary.js`, + `github-app-token.js`, and `prepare-pr-base.js`), schemars-driven type codegen, the A2 design decision, and the bundle env contract modelled in `src/compile/ado_bundle.rs`. - [`docs/local-development.md`](docs/local-development.md) — local development diff --git a/docs/ado-script.md b/docs/ado-script.md index 7ddd7d9f..953f5ea5 100644 --- a/docs/ado-script.md +++ b/docs/ado-script.md @@ -68,6 +68,24 @@ pipeline** as runtime helpers. Today it produces thirteen bundles: env vars, so a pipeline variable can't shadow them (only the private key / minted token ride in masked env). Runs outside AWF. See [`engine.md`](engine.md#github-app-backed-copilot-engine-auth). +- `prepare-pr-base.js` — create-pull-request base-ref preparer that runs in the + **Agent job** before the Copilot invocation when `create-pull-request` is + configured (issue #1413). For each allowed create-PR repo dir (`self` + + every `checkout:` alias, passed as repeated `--repo-dir --target-branch + ` pairs in the same dir form `mcp.rs::resolve_git_dir_for_patch` + resolves), it fetches and progressively deepens THAT repo's resolved target + branch into `refs/remotes/origin/` and points + `refs/remotes/origin/HEAD` at it, so the host-side SafeOutputs MCP server can + compute a diff base on shallow-default agent pools without a full-history + `checkout: self`. In a multi-checkout ("meta repo") setup each dir may carry a + different target (see `create-pull-request`'s `target-branches` / + `infer-target-from-checkout-ref`). Reuses + `shared/merge-base.ts::ensureTargetRefFetched` (the same fetch/deepen logic as + the PR execution-context precompute). Each `--repo-dir` is a double-quoted + ADO-macro path; each `--target-branch` is a single-quoted literal; the ADO + bearer (`SYSTEM_ACCESSTOKEN`) rides in masked env for the authenticated git + fetch. Per-dir failures are isolated (logged + skipped). Runs outside AWF. See + [`safe-outputs.md`](safe-outputs.md#create-pull-request). > **Internal-only.** `ado-script` is not a user-facing front-matter > feature. Authors never write an `ado-script:` block in their agent @@ -520,12 +538,15 @@ scripts/ado-script/ │ ├── exec-context-repo/ # exec-context-repo.js entry point + repository identity context │ │ ├── index.ts # main(): stage branch/SHA/tag/commits-since-tag facts → prompt │ │ └── __tests__/ # unit tests for identity / tag fallback / sanitisation paths -│ └── conclusion/ # conclusion.js entry point + Conclusion-job reporter -│ ├── index.ts # main(): inspect upstream results + safe-outputs manifest → file/append work items -│ └── __tests__/ # unit tests for signal detection and work-item filing behaviour -│ └── github-app-token/ # github-app-token.js entry point + GitHub App token minter -│ ├── index.ts # main(): RS256 JWT → resolve installation → mint installation token → masked GITHUB_APP_TOKEN -│ └── __tests__/ # unit tests for JWT signing / installation resolution / token minting +│ ├── conclusion/ # conclusion.js entry point + Conclusion-job reporter +│ │ ├── index.ts # main(): inspect upstream results + safe-outputs manifest → file/append work items +│ │ └── __tests__/ # unit tests for signal detection and work-item filing behaviour +│ ├── github-app-token/ # github-app-token.js entry point + GitHub App token minter +│ │ ├── index.ts # main(): RS256 JWT → resolve installation → mint installation token → masked GITHUB_APP_TOKEN +│ │ └── __tests__/ # unit tests for JWT signing / installation resolution / token minting +│ └── prepare-pr-base/ # prepare-pr-base.js entry point + create-pull-request base-ref fetch/deepen +│ ├── index.ts # main(): fetch/deepen target branch + set origin/HEAD so mcp.rs finds a diff base +│ └── __tests__/ # unit tests for fetch/deepen + origin/HEAD + benign-failure paths ├── test/ # End-to-end smoke tests (gate, import, exec-context-pr) ├── gate.js # ncc bundle output (gitignored) ├── import.js # ncc bundle output (gitignored) @@ -540,7 +561,8 @@ scripts/ado-script/ ├── exec-context-repo.js # ncc bundle output (gitignored) ├── conclusion.js # ncc bundle output (gitignored) ├── approval-summary.js # ncc bundle output (gitignored) -└── github-app-token.js # ncc bundle output (gitignored) +├── github-app-token.js # ncc bundle output (gitignored) +└── prepare-pr-base.js # ncc bundle output (gitignored) ``` The release workflow (`.github/workflows/release.yml`) runs diff --git a/docs/execution-context.md b/docs/execution-context.md index 1ae0d541..372dc0c3 100644 --- a/docs/execution-context.md +++ b/docs/execution-context.md @@ -41,6 +41,14 @@ The execution-context plugin owns that step centrally — but does The agent does its own diff/show/log/stat work — it has the objects locally and `git` is added to its bash allow-list automatically. +> **Related:** the `create-pull-request` safe output uses the *same* +> credentialed fetch/deepen approach — via the `prepare-pr-base.js` +> bundle (which reuses `shared/merge-base.ts::ensureTargetRefFetched`) — +> so its diff base resolves on shallow-default pools without forcing a +> full-history `checkout: self`. See +> [`safe-outputs.md`](safe-outputs.md#create-pull-request) and +> [`ado-script.md`](ado-script.md). + ## v1 contributors | Contributor | Trigger | Output layout | diff --git a/docs/safe-outputs.md b/docs/safe-outputs.md index 338abe5b..aa83eb79 100644 --- a/docs/safe-outputs.md +++ b/docs/safe-outputs.md @@ -263,6 +263,27 @@ Creates a pull request with code changes made by the agent. When invoked: During Stage 3 execution, the repository is validated against the allowed list (from `checkout:` + "self"), then the patch is applied and a PR is created in Azure DevOps. +**Shallow-clone agent pools (automatic):** The diff base for the patch is +computed at agent time from the checked-out repository. On agent pools whose +default git fetch is shallow (`fetchDepth: 1`), a bare `checkout` leaves no +`origin/` ref, which would otherwise prevent the diff base from +being computed. To handle this transparently, whenever `create-pull-request` is +configured the compiler emits a credentialed **prepare step** in the Agent job +(before the agent runs) that fetches and progressively deepens the configured +`target-branch` and points `origin/HEAD` at it — in the `self` checkout **and in +each additional `checkout:` repo dir**, so a PR to *any* allowed repository works. +This means create-pull-request works on shallow-default pools **without** forcing +a full-history checkout and **without** hand-editing the compiled lock (so the +runtime integrity check keeps passing). No configuration is required. See +[`docs/ado-script.md`](ado-script.md) (`prepare-pr-base.js`). + +> **Branch semantics.** The step deepens each repo's resolved `target-branch` +> (the PR's **destination/base**) — not the per-repo `repos:` checkout `ref` (the +> source side). By default every repo targets the single `target-branch`; enable +> `infer-target-from-checkout-ref` (and/or `target-branches`) to give each repo +> its own base branch in a multi-checkout setup. The deepened branch always +> matches the branch the PR targets (shared resolution). + **Stage 3 Execution Architecture (Hybrid Git + ADO API):** ``` @@ -306,7 +327,39 @@ This hybrid approach combines: Note: The source branch name is auto-generated from a sanitized version of the PR title plus a unique suffix (e.g., `agent/fix-bug-in-parser-a1b2c3`). This format is human-readable while preventing injection attacks. **Configuration options (front matter):** -- `target-branch` - Target branch to merge into (default: "main") +- `target-branch` - Target (base) branch the PR merges into (default: "main"). A + plain literal branch name, applied to every repo unless overridden below. +- `target-branches` - Optional map of per-repository target-branch overrides, + keyed by the repository alias the agent passes to `create-pull-request` (`self` + or a `checkout:` alias). Highest precedence. Lets a multi-checkout ("meta repo") + agent open a PR into a different base branch per repo. +- `infer-target-from-checkout-ref` - Optional bool (default: false). When `true`, + a checkout repo with no explicit `target-branches` entry targets its own + `repos: ref` (the branch it was checked out at). `self` and repos without a + known ref fall back to `target-branch`. It is a separate boolean (not a magic + `target-branch` value) so a real branch name can never be mistaken for a + directive. Only branch refs (`refs/heads/*`) are valid PR targets — if an + inferred repo is checked out at a **tag** (`refs/tags/*`) the compiler warns + and you should give it an explicit `target-branches` entry (a PR cannot target + a tag). + + **Per-repo target resolution precedence** (for a repo `R`): `target-branches[R]` + → (if `infer-target-from-checkout-ref`) `R`'s checkout ref → `target-branch` → + `main`. The same resolution drives both the credentialed base-ref deepening (so + the branch that is fetched/deepened matches the branch the PR targets) and the + Stage 3 PR creation. Example (meta repo): + ```yaml + repos: + - name: my-org/service # checked out at refs/heads/main + - name: my-org/docs + ref: refs/heads/gh-pages + safe-outputs: + create-pull-request: + target-branch: main # self + fallback + infer-target-from-checkout-ref: true # service → main, docs → gh-pages (from their refs) + target-branches: + docs: gh-pages # (redundant here; shown as an explicit override) + ``` - `draft` - Whether to create the PR as a draft (default: **true**). Set to `false` to publish the PR immediately. **Note:** `auto-complete` is silently skipped on draft PRs — set `draft: false` when using `auto-complete: true`. - `auto-complete` - Set auto-complete on the PR (default: false). Requires `draft: false` to take effect. - `delete-source-branch` - Delete source branch after merge (default: true) diff --git a/scripts/ado-script/.gitignore b/scripts/ado-script/.gitignore index 3dc3d4ae..f50cd05e 100644 --- a/scripts/ado-script/.gitignore +++ b/scripts/ado-script/.gitignore @@ -14,6 +14,7 @@ exec-context-repo.js approval-summary.js conclusion.js github-app-token.js +prepare-pr-base.js schema *.tsbuildinfo test-bin diff --git a/scripts/ado-script/package.json b/scripts/ado-script/package.json index 4b518bfe..f2439810 100644 --- a/scripts/ado-script/package.json +++ b/scripts/ado-script/package.json @@ -7,8 +7,8 @@ "node": ">=20.0.0" }, "scripts": { - "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:conclusion && npm run build:approval-summary && npm run build:github-app-token", - "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); for (const n of ['gate','import','exec-context-pr','exec-context-pr-synth','exec-context-manual','exec-context-pipeline','exec-context-ci-push','exec-context-workitem','exec-context-schedule','exec-context-pr-checks','exec-context-repo','conclusion','approval-summary','github-app-token']) fs.rmSync(n+'.js',{force:true});\"", + "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && npm run build:exec-context-manual && npm run build:exec-context-pipeline && npm run build:exec-context-ci-push && npm run build:exec-context-workitem && npm run build:exec-context-schedule && npm run build:exec-context-pr-checks && npm run build:exec-context-repo && npm run build:conclusion && npm run build:approval-summary && npm run build:github-app-token && npm run build:prepare-pr-base", + "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); for (const n of ['gate','import','exec-context-pr','exec-context-pr-synth','exec-context-manual','exec-context-pipeline','exec-context-ci-push','exec-context-workitem','exec-context-schedule','exec-context-pr-checks','exec-context-repo','conclusion','approval-summary','github-app-token','prepare-pr-base']) fs.rmSync(n+'.js',{force:true});\"", "build:gate": "ncc build src/gate/index.ts -o .ado-build/gate -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/gate/index.js','gate.js'); fs.rmSync('.ado-build/gate',{recursive:true,force:true});\"", "build:import": "ncc build src/import/index.ts -o .ado-build/import -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/import/index.js','import.js'); fs.rmSync('.ado-build/import',{recursive:true,force:true});\"", "build:exec-context-pr": "ncc build src/exec-context-pr/index.ts -o .ado-build/exec-context-pr -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-pr/index.js','exec-context-pr.js'); fs.rmSync('.ado-build/exec-context-pr',{recursive:true,force:true});\"", @@ -23,6 +23,7 @@ "build:conclusion": "ncc build src/conclusion/index.ts -o .ado-build/conclusion -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/conclusion/index.js','conclusion.js'); fs.rmSync('.ado-build/conclusion',{recursive:true,force:true});\"", "build:approval-summary": "ncc build src/approval-summary/index.ts -o .ado-build/approval-summary -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/approval-summary/index.js','approval-summary.js'); fs.rmSync('.ado-build/approval-summary',{recursive:true,force:true});\"", "build:github-app-token": "ncc build src/github-app-token/index.ts -o .ado-build/github-app-token -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/github-app-token/index.js','github-app-token.js'); fs.rmSync('.ado-build/github-app-token',{recursive:true,force:true});\"", + "build:prepare-pr-base": "ncc build src/prepare-pr-base/index.ts -o .ado-build/prepare-pr-base -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/prepare-pr-base/index.js','prepare-pr-base.js'); fs.rmSync('.ado-build/prepare-pr-base',{recursive:true,force:true});\"", "build:executor-e2e": "ncc build src/executor-e2e/index.ts -o .ado-build/executor-e2e -m -t && node -e \"const fs=require('node:fs'); fs.mkdirSync('test-bin',{recursive:true}); fs.copyFileSync('.ado-build/executor-e2e/index.js','test-bin/executor-e2e.js'); fs.rmSync('.ado-build/executor-e2e',{recursive:true,force:true});\"", "build:check": "ls -lh gate.js && wc -c gate.js", "codegen": "node -e \"require('node:fs').mkdirSync('schema', { recursive: true })\" && cargo run --quiet --manifest-path ../../Cargo.toml -- export-gate-schema --output schema/gate-spec.schema.json && npx json2ts schema/gate-spec.schema.json -o src/shared/types.gen.ts --bannerComment \"// AUTO-GENERATED from Rust IR via cargo run -- export-gate-schema. Do not edit; run npm run codegen.\"", diff --git a/scripts/ado-script/src/prepare-pr-base/__tests__/index.test.ts b/scripts/ado-script/src/prepare-pr-base/__tests__/index.test.ts new file mode 100644 index 00000000..0615d73a --- /dev/null +++ b/scripts/ado-script/src/prepare-pr-base/__tests__/index.test.ts @@ -0,0 +1,224 @@ +import { describe, expect, it } from "vitest"; + +import type { GitResult } from "../../shared/git.js"; +import type { GitRunners } from "../../shared/merge-base.js"; +import { main, parseArgs } from "../index.js"; + +const SHA_M = "d".repeat(40); + +/** + * Build a `GitRunners` pair that records every `runGit` invocation and lets a + * matcher decide the result. `gitOk` answers `merge-base` queries. + */ +function makeRunners(opts: { + fetchStatus: number; + symbolicStatus?: number; + mergeBase?: string | null; +}): { runners: GitRunners; calls: string[][] } { + const calls: string[][] = []; + const runGit: GitRunners["runGit"] = (args) => { + calls.push(args); + let result: GitResult = { stdout: "", stderr: "", status: 1 }; + if (args[0] === "fetch") { + result = { stdout: "", stderr: "", status: opts.fetchStatus }; + } else if (args[0] === "symbolic-ref") { + result = { stdout: "", stderr: "", status: opts.symbolicStatus ?? 0 }; + } + return result; + }; + const gitOk: GitRunners["gitOk"] = (args) => { + if (args[0] === "merge-base") { + return opts.mergeBase === undefined ? SHA_M : opts.mergeBase; + } + return null; + }; + return { runners: { runGit, gitOk }, calls }; +} + +/** A `chdir` stub that records the dirs it was asked to enter. */ +function recordingChdir(): { chdir: (dir: string) => void; dirs: string[] } { + const dirs: string[] = []; + return { chdir: (dir: string) => void dirs.push(dir), dirs }; +} + +describe("parseArgs", () => { + it("pairs each --repo-dir with the following --target-branch", () => { + const args = parseArgs([ + "--repo-dir", + "/src", + "--target-branch", + "main", + "--repo-dir", + "/src/tools", + "--target-branch", + "release", + ]); + expect(args.repos).toEqual([ + { dir: "/src", target: "main" }, + { dir: "/src/tools", target: "release" }, + ]); + }); + + it("strips a leading refs/heads/ from targets", () => { + const args = parseArgs([ + "--repo-dir", + "/src", + "--target-branch", + "refs/heads/release/2.x", + ]); + expect(args.repos).toEqual([{ dir: "/src", target: "release/2.x" }]); + }); + + it("uses the fallback target for a --repo-dir with no following --target-branch", () => { + // A leading global --target-branch sets the fallback; a bare trailing + // --repo-dir inherits it. + const args = parseArgs(["--target-branch", "develop", "--repo-dir", "/src"]); + expect(args.fallbackTarget).toBe("develop"); + expect(args.repos).toEqual([{ dir: "/src", target: "develop" }]); + }); + + it("defaults the fallback target to main", () => { + expect(parseArgs([]).fallbackTarget).toBe("main"); + expect(parseArgs([]).repos).toEqual([]); + }); +}); + +describe("prepare-pr-base main", () => { + it("fetches origin/ and sets origin/HEAD on success", () => { + const { runners, calls } = makeRunners({ fetchStatus: 0, mergeBase: SHA_M }); + const { chdir, dirs } = recordingChdir(); + const rc = main( + { repos: [{ dir: "/src", target: "main" }], fallbackTarget: "main" }, + {}, + runners, + chdir, + ); + expect(rc).toBe(0); + expect(dirs).toEqual(["/src"]); + + const fetch = calls.find((c) => c[0] === "fetch"); + expect(fetch).toBeDefined(); + expect(fetch).toContain("+refs/heads/main:refs/remotes/origin/main"); + + const sym = calls.find((c) => c[0] === "symbolic-ref"); + expect(sym).toEqual([ + "symbolic-ref", + "refs/remotes/origin/HEAD", + "refs/remotes/origin/main", + ]); + }); + + it("deepens each repo dir with its OWN target branch (meta-repo)", () => { + const { runners, calls } = makeRunners({ fetchStatus: 0, mergeBase: SHA_M }); + const { chdir, dirs } = recordingChdir(); + const rc = main( + { + repos: [ + { dir: "/src", target: "main" }, + { dir: "/src/tools", target: "release" }, + { dir: "/src/docs", target: "gh-pages" }, + ], + fallbackTarget: "main", + }, + {}, + runners, + chdir, + ); + expect(rc).toBe(0); + expect(dirs).toEqual(["/src", "/src/tools", "/src/docs"]); + + // Each dir fetched + set origin/HEAD to ITS target ref. + const fetchRefspecs = calls + .filter((c) => c[0] === "fetch") + .map((c) => c.find((a) => a.startsWith("+refs/heads/"))); + expect(fetchRefspecs).toEqual([ + "+refs/heads/main:refs/remotes/origin/main", + "+refs/heads/release:refs/remotes/origin/release", + "+refs/heads/gh-pages:refs/remotes/origin/gh-pages", + ]); + const symTargets = calls.filter((c) => c[0] === "symbolic-ref").map((c) => c[2]); + expect(symTargets).toEqual([ + "refs/remotes/origin/main", + "refs/remotes/origin/release", + "refs/remotes/origin/gh-pages", + ]); + }); + + it("isolates a per-dir chdir failure — other dirs still processed", () => { + const { runners, calls } = makeRunners({ fetchStatus: 0, mergeBase: SHA_M }); + const dirs: string[] = []; + const chdir = (dir: string) => { + dirs.push(dir); + if (dir === "/src/broken") { + throw new Error("no such directory"); + } + }; + const rc = main( + { + repos: [ + { dir: "/src", target: "main" }, + { dir: "/src/broken", target: "release" }, + { dir: "/src/lib", target: "main" }, + ], + fallbackTarget: "main", + }, + {}, + runners, + chdir, + ); + expect(rc).toBe(0); + expect(dirs).toEqual(["/src", "/src/broken", "/src/lib"]); + // Only the two good dirs fetched + set origin/HEAD (broken skipped). + expect(calls.filter((c) => c[0] === "fetch").length).toBe(2); + expect(calls.filter((c) => c[0] === "symbolic-ref").length).toBe(2); + }); + + it("exits 0 without setting origin/HEAD when every fetch fails (benign)", () => { + const { runners, calls } = makeRunners({ fetchStatus: 1, mergeBase: null }); + const { chdir } = recordingChdir(); + const rc = main( + { repos: [{ dir: "/src", target: "main" }], fallbackTarget: "main" }, + {}, + runners, + chdir, + ); + // Non-fatal: the agent still runs; mcp.rs surfaces its own error if needed. + expect(rc).toBe(0); + expect(calls.some((c) => c[0] === "symbolic-ref")).toBe(false); + }); + + it("falls back to BUILD_SOURCESDIRECTORY + fallback target when no repos given", () => { + const { runners, calls } = makeRunners({ fetchStatus: 0, mergeBase: SHA_M }); + const { chdir, dirs } = recordingChdir(); + main( + { repos: [], fallbackTarget: "develop" }, + { BUILD_SOURCESDIRECTORY: "/agent/src" }, + runners, + chdir, + ); + expect(dirs).toEqual(["/agent/src"]); + const fetch = calls.find((c) => c[0] === "fetch"); + expect(fetch).toContain("+refs/heads/develop:refs/remotes/origin/develop"); + }); + + it("passes the SYSTEM_ACCESSTOKEN bearer into the git fetch env", () => { + const seenEnvs: Array | undefined> = []; + const runners: GitRunners = { + runGit: (args, env) => { + if (args[0] === "fetch") seenEnvs.push(env); + return { stdout: "", stderr: "", status: 0 }; + }, + gitOk: (args) => (args[0] === "merge-base" ? SHA_M : null), + }; + const { chdir } = recordingChdir(); + main( + { repos: [{ dir: "/src", target: "main" }], fallbackTarget: "main" }, + { SYSTEM_ACCESSTOKEN: "tok" }, + runners, + chdir, + ); + expect(seenEnvs[0]).toMatchObject({ + GIT_CONFIG_VALUE_0: "Authorization: bearer tok", + }); + }); +}); diff --git a/scripts/ado-script/src/prepare-pr-base/index.ts b/scripts/ado-script/src/prepare-pr-base/index.ts new file mode 100644 index 00000000..ddc47aff --- /dev/null +++ b/scripts/ado-script/src/prepare-pr-base/index.ts @@ -0,0 +1,216 @@ +/** + * prepare-pr-base — make the create-pull-request diff base available on + * shallow-default Azure DevOps agent pools (issue #1413). + * + * ## Why this exists + * + * The `create-pull-request` safe-output patch is generated at agent time by the + * host-side SafeOutputs MCP server (`src/mcp.rs::find_merge_base`), which only + * inspects **existing local refs** and never fetches. On an agent pool whose + * default git fetch is shallow (`fetchDepth: 1`), `checkout: self` leaves no + * `refs/remotes/origin/` ref and too little history, so the server + * cannot compute a merge base and the PR can't be opened. + * + * This bundle runs as a **credentialed Agent-job prepare step on the host** + * (before the AWF-wrapped Copilot run, using `$(System.AccessToken)`). For each + * allowed create-pull-request repo dir (`self` + every `checkout:` alias, passed + * as repeated `--repo-dir --target-branch ` pairs in the same form + * the MCP server resolves the dirs), it fetches that repo's target branch into + * `refs/remotes/origin/` and progressively deepens local history to the + * merge base — reusing the exact `shared/merge-base.ts::ensureTargetRefFetched` + * logic that the PR execution-context precompute already uses. It also points + * each dir's `refs/remotes/origin/HEAD` at its target so `mcp.rs`'s symbolic-ref + * default-branch detection resolves the right base. Because the MCP server + * operates on those same dirs, the base then resolves with no in-sandbox network. + * + * Each `--target-branch` is the create-pull-request **destination/base** branch + * for its `--repo-dir` (which in a multi-checkout "meta repo" setup may differ + * per repo) — NOT the per-repo `checkout:` ref (the source/HEAD side). + * + * ## Trust boundary + * + * Mirrors the exec-context bundles: the bearer (`SYSTEM_ACCESSTOKEN`) is passed + * to the spawned `git` child via `GIT_CONFIG_*` env vars (see + * `shared/git.ts::bearerEnv`) — never in argv, never written to `.git/config`. + * The compiler-owned, non-secret `--repo-dir` / `--target-branch` are argv flags + * (immune to ADO pipeline-variable shadowing). + * + * ## Posture + * + * Benign fetch failures (e.g. a pool that refuses shallow deepening) are logged + * and the step still exits 0 so the agent runs; the agent then simply hits the + * existing `mcp.rs` diff-base error if truly unrecoverable. Only genuine infra + * errors (an unusable `BUILD_SOURCESDIRECTORY`) hard-fail. + * + * Invocation: node prepare-pr-base.js \ + * --repo-dir --target-branch \ + * [--repo-dir --target-branch ...] + * env: SYSTEM_ACCESSTOKEN (bearer for the git fetch) + */ +import { bearerEnv, gitOk as defaultGitOk, runGit as defaultRunGit } from "../shared/git.js"; +import { ensureTargetRefFetched, type GitRunners } from "../shared/merge-base.js"; + +const defaultRunners: GitRunners = { + runGit: defaultRunGit, + gitOk: defaultGitOk, +}; + +/** A single checkout dir to deepen, with the target branch to deepen there. */ +export interface RepoTarget { + /** Checkout dir (as `mcp.rs::resolve_git_dir_for_patch` resolves it). */ + dir: string; + /** Short target branch to fetch/deepen in that dir (no `refs/heads/`). */ + target: string; +} + +export interface PrepareArgs { + /** + * The checkout dirs to deepen with their per-repo target branch — one per + * allowed create-pull-request repo, in the SAME form the host-side SafeOutputs + * MCP server resolves them (`resolve_git_dir_for_patch`): `working_directory` + * for `self`, and `working_directory/` for each `checkout:` alias. In a + * multi-checkout ("meta repo") setup each dir may carry a different target. + * Empty when none were passed (falls back to `[{ BUILD_SOURCESDIRECTORY, + * fallbackTarget }]`, then the process cwd). + */ + repos: RepoTarget[]; + /** Target for the fallback dir when no `--repo-dir` was passed. */ + fallbackTarget: string; +} + +function shortBranch(name: string): string { + const s = name.replace(/^refs\/heads\//, ""); + // In lock-step with the Rust `short_branch` resolver for every input: a + // normal ref → its short form; a degenerate `refs/heads/` (empty after + // stripping) → the original string (so a malformed ref fails loudly the same + // way on both sides, never silently diverging); and `""` → `""`. `parseArgs` + // supplies its own "main" default for a genuinely absent `--target-branch`. + return s.length > 0 ? s : name; +} + +/** + * Parse repeated `--repo-dir ` / `--target-branch ` flags into + * ordered `{ dir, target }` pairs. Each `--repo-dir` takes the target from the + * `--target-branch` that immediately follows it (compiler emits them adjacent); + * a `--target-branch` with no preceding un-paired `--repo-dir` sets the fallback + * target. Branch names are normalized to short form (`refs/heads/x` → `x`). + */ +export function parseArgs(argv: string[]): PrepareArgs { + const repos: RepoTarget[] = []; + let fallbackTarget = "main"; + let pendingDir: string | null = null; + for (let i = 0; i < argv.length; i++) { + if (argv[i] === "--repo-dir") { + // A previous dir without its own target inherits the fallback. + if (pendingDir !== null && pendingDir.length > 0) { + repos.push({ dir: pendingDir, target: fallbackTarget }); + } + pendingDir = argv[i + 1] ?? ""; + i++; + } else if (argv[i] === "--target-branch") { + const target = shortBranch(argv[i + 1] ?? "main"); + if (pendingDir !== null && pendingDir.length > 0) { + repos.push({ dir: pendingDir, target }); + pendingDir = null; + } else { + fallbackTarget = target; + } + i++; + } + } + // A trailing dir with no following --target-branch inherits the fallback. + if (pendingDir !== null && pendingDir.length > 0) { + repos.push({ dir: pendingDir, target: fallbackTarget }); + } + return { repos, fallbackTarget }; +} + +/** + * Deepen `origin/` in a single checkout dir and point that dir's + * `origin/HEAD` at it. Returns `true` when the base resolved. Never throws — a + * dir that isn't a git repo (a quirky-workspace path the MCP would also fail on) + * or a fetch that can't reach the base is a benign, isolated skip. + */ +function prepareOneRepo( + repoDir: string, + targetShort: string, + fetchEnv: Record, + runners: GitRunners, + chdir: (dir: string) => void, +): boolean { + try { + chdir(repoDir); + } catch (err) { + // Non-fatal: the MCP server would fail on this same path too. Skip and let + // other repos proceed. + process.stdout.write( + `[prepare-pr-base] note: skipping '${repoDir}' (could not chdir: ${(err as Error).message}).\n`, + ); + return false; + } + + const fetched = ensureTargetRefFetched(targetShort, fetchEnv, runners); + if (!fetched.ok) { + process.stdout.write( + `[prepare-pr-base] warning: ${fetched.reason} create-pull-request may fail to compute a diff base for '${repoDir}' on this pool.\n`, + ); + return false; + } + + // Point origin/HEAD at the fetched target so mcp.rs's + // `git symbolic-ref refs/remotes/origin/HEAD` default-branch probe resolves + // to origin/ even when the target is not main/master. + const sym = runners.runGit([ + "symbolic-ref", + "refs/remotes/origin/HEAD", + `refs/remotes/origin/${targetShort}`, + ]); + if (sym.status !== 0) { + process.stdout.write( + `[prepare-pr-base] note: could not set refs/remotes/origin/HEAD -> origin/${targetShort} in '${repoDir}' (${sym.stderr.trim()}); relying on origin/${targetShort} directly.\n`, + ); + } + + process.stdout.write( + `[prepare-pr-base] base ref ready in '${repoDir}': origin/${targetShort} fetched/deepened (merge-base=${fetched.baseSha}).\n`, + ); + return true; +} + +export function main( + args: PrepareArgs, + env: NodeJS.ProcessEnv = process.env, + runners: GitRunners = defaultRunners, + chdir: (dir: string) => void = process.chdir.bind(process), +): number { + // Deepen every checkout dir the MCP server might generate a patch from — one + // per allowed create-pull-request repo (`self` + each `checkout:` alias), each + // with its own target branch. When the compiler passed none, fall back to + // BUILD_SOURCESDIRECTORY then cwd (with the fallback target). + let repos = args.repos; + if (repos.length === 0) { + const fallbackDir = env.BUILD_SOURCESDIRECTORY; + repos = [ + { dir: fallbackDir && fallbackDir.length > 0 ? fallbackDir : ".", target: args.fallbackTarget }, + ]; + } + + const fetchEnv = bearerEnv(env.SYSTEM_ACCESSTOKEN); + + // Per-dir failures are isolated (logged + skipped) so one unreachable repo + // never blocks the others or the agent run. + for (const { dir, target } of repos) { + prepareOneRepo(dir, target, fetchEnv, runners, chdir); + } + return 0; +} + +// CLI entry guard: only run when invoked directly (not when imported by tests). +if ( + typeof process !== "undefined" && + process.argv[1] && + /prepare-pr-base(\/index)?\.js$/.test(process.argv[1]) +) { + const args = parseArgs(process.argv.slice(2)); + process.exit(main(args)); +} diff --git a/scripts/ado-script/src/shared/__tests__/merge-base.test.ts b/scripts/ado-script/src/shared/__tests__/merge-base.test.ts index 6638636c..b121dfc7 100644 --- a/scripts/ado-script/src/shared/__tests__/merge-base.test.ts +++ b/scripts/ado-script/src/shared/__tests__/merge-base.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import type { GitResult } from "../git.js"; -import { resolveMergeBase, type GitRunners } from "../merge-base.js"; +import { ensureTargetRefFetched, resolveMergeBase, type GitRunners } from "../merge-base.js"; // Real-shape SHAs (40-char lowercase hex) so the production // SHA40 guard in resolveMergeBase accepts them. We keep the @@ -276,3 +276,52 @@ describe("resolveMergeBase", () => { } }); }); + +describe("ensureTargetRefFetched", () => { + it("resolves at the first depth and reports the merge base", () => { + const depthArgsSeen: string[] = []; + const runGit: GitRunners["runGit"] = (args) => { + if (args[0] === "fetch") depthArgsSeen.push(args[2] ?? ""); + return { stdout: "", stderr: "", status: 0 }; + }; + const gitOk = makeGitOk([ + { match: (a) => a.join(" ") === "merge-base origin/main HEAD", out: SHA_M }, + ]); + + const result = ensureTargetRefFetched("main", {}, { runGit, gitOk }); + expect(result.ok).toBe(true); + if (result.ok) expect(result.baseSha).toBe(SHA_M); + // Stops after the first depth (does not keep deepening once resolved). + expect(depthArgsSeen).toEqual(["--depth=200"]); + }); + + it("keeps deepening until merge-base resolves, then stops", () => { + const depthArgsSeen: string[] = []; + const runGit: GitRunners["runGit"] = (args) => { + if (args[0] === "fetch") depthArgsSeen.push(args[2] ?? ""); + return { stdout: "", stderr: "", status: 0 }; + }; + let calls = 0; + const gitOk: GitRunners["gitOk"] = (a) => { + if (a.join(" ") === "merge-base origin/main HEAD") { + calls++; + // Resolves only on the 3rd depth (--depth=2000). + return calls >= 3 ? SHA_M : null; + } + return null; + }; + + const result = ensureTargetRefFetched("main", {}, { runGit, gitOk }); + expect(result.ok).toBe(true); + expect(depthArgsSeen).toEqual(["--depth=200", "--depth=500", "--depth=2000"]); + }); + + it("returns ok:false when no depth resolves the merge base", () => { + const runGit: GitRunners["runGit"] = () => ({ stdout: "", stderr: "", status: 0 }); + const gitOk: GitRunners["gitOk"] = () => null; + + const result = ensureTargetRefFetched("main", {}, { runGit, gitOk }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toContain("origin/main"); + }); +}); diff --git a/scripts/ado-script/src/shared/merge-base.ts b/scripts/ado-script/src/shared/merge-base.ts index 2219f99e..e2b1593f 100644 --- a/scripts/ado-script/src/shared/merge-base.ts +++ b/scripts/ado-script/src/shared/merge-base.ts @@ -74,6 +74,54 @@ function fetchTargetAtDepth( return result.status === 0; } +/** Result of [`ensureTargetRefFetched`]. */ +export type FetchDeepenResult = + | { ok: true; baseSha: string } + | { ok: false; reason: string }; + +/** + * Fetch `origin/` into the local clone and progressively + * deepen it (`--depth=200`, `500`, `2000`, then `--unshallow`) until + * `git merge-base origin/ HEAD` resolves — i.e. until the + * clone carries enough history to reach the merge base of HEAD and the + * target branch. + * + * This isolates the *side effect* that both [`resolveMergeBase`] (which + * also returns the SHAs) and the `prepare-pr-base` bundle (which only + * needs the deepened clone + `refs/remotes/origin/` populated so + * the host-side SafeOutputs MCP server can later compute the base) rely + * on. The git call sequence is identical to the loop `resolveMergeBase` + * previously inlined, so injected `GitRunners` stubs see the same order. + * + * `env` is the result of `bearerEnv(token)` — passed to git's fetch + * subprocess so the bearer never leaks into argv or to other tools. + */ +export function ensureTargetRefFetched( + targetShort: string, + env: Record, + runners: GitRunners = defaultRunners, +): FetchDeepenResult { + // Progressive deepening: stop ONLY when merge-base actually resolves + // against the deepened target ref. + const depths = ["--depth=200", "--depth=500", "--depth=2000", "--unshallow"]; + for (const depthArg of depths) { + if (!fetchTargetAtDepth(runners, targetShort, depthArg, env)) { + // Fetch failed at this depth (e.g. --unshallow on an already- + // unshallowed repo). Continue to the next depth or bail out after + // the loop. + continue; + } + const mb = runners.gitOk(["merge-base", `origin/${targetShort}`, "HEAD"]) ?? ""; + if (mb.length > 0) { + return { ok: true, baseSha: mb }; + } + } + return { + ok: false, + reason: `Could not resolve merge-base against 'origin/${targetShort}' after progressive deepening.`, + }; +} + /** * Resolve `BASE_SHA` and `HEAD_SHA` for the PR. * @@ -115,21 +163,12 @@ export function resolveMergeBase( } } else { headTipSha = headSha; - // Progressive deepening: stop ONLY when merge-base actually - // resolves against the deepened target ref. - const depths = ["--depth=200", "--depth=500", "--depth=2000", "--unshallow"]; - for (const depthArg of depths) { - if (!fetchTargetAtDepth(runners, targetShort, depthArg, env)) { - // Fetch failed at this depth (e.g. --unshallow on an - // already-unshallowed repo). Continue to the next depth or - // bail out after the loop. - continue; - } - const mb = runners.gitOk(["merge-base", `origin/${targetShort}`, "HEAD"]) ?? ""; - if (mb.length > 0) { - baseSha = mb; - break; - } + // Progressive deepening (extracted into `ensureTargetRefFetched` so + // the `prepare-pr-base` bundle can reuse the identical fetch/deepen + // sequence for its side effect). + const fetched = ensureTargetRefFetched(targetShort, env, runners); + if (fetched.ok) { + baseSha = fetched.baseSha; } } diff --git a/site/src/content/docs/reference/ado-script.mdx b/site/src/content/docs/reference/ado-script.mdx index e3e2cec9..06a71e88 100644 --- a/site/src/content/docs/reference/ado-script.mdx +++ b/site/src/content/docs/reference/ado-script.mdx @@ -8,7 +8,7 @@ import { Aside } from '@astrojs/starlight/components'; `ado-script` is the umbrella name for the TypeScript workspace at [`scripts/ado-script/`](https://github.com/githubnext/ado-aw/tree/main/scripts/ado-script). It produces small, ncc-bundled Node programs that the **compiler injects into emitted -pipelines** as runtime helpers. Today it produces thirteen bundles: +pipelines** as runtime helpers. Today it produces fifteen bundles: - **`gate.js`** — trigger-filter gate evaluator (Setup job) - **`import.js`** — runtime prompt resolver described in [Runtime Imports](/ado-aw/reference/runtime-imports/) (Agent job) @@ -23,6 +23,8 @@ pipelines** as runtime helpers. Today it produces thirteen bundles: - **`exec-context-repo.js`** — repository identity precompute that stages branch, SHA, last release tag, and commits-since-tag facts (Agent job) - **`conclusion.js`** — Conclusion job reporter that files/comments ADO work items for pipeline failures and diagnostic signals (Conclusion job) - **`approval-summary.js`** — renders a sanitized per-tool summary of proposed safe outputs into the build's `ado-aw-safe-outputs` summary tab (end of Agent job) +- **`github-app-token.js`** — mints (and revokes) a GitHub App installation token for the Copilot engine when `engine.github-app-token` is configured (Agent + Detection jobs) +- **`prepare-pr-base.js`** — fetches/deepens each allowed create-pull-request repo's target branch (self + every `checkout:` alias, each with its own resolved target) so the SafeOutputs MCP server can compute a diff base for a PR to any allowed repo on shallow-default agent pools (Agent job)