From 3a32c4924390d30b9c3757d4339183ddc353cb5b Mon Sep 17 00:00:00 2001 From: David Lee <8321301+davidalee@users.noreply.github.com> Date: Sun, 10 May 2026 21:59:30 -0400 Subject: [PATCH 1/2] fix(code-review): resolve PR base host-agnostically in stable skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace hard-coded `github.com:` / `github.com/` substring matching and `https://github.com/...` fallback URL with host+repo matching driven by values derived from the PR URL. On GitHub Enterprise (or any non- github.com host), the previous awk silently failed to identify the upstream remote in fork workflows and fell through to `origin` — which in fork setups points at the user's fork — producing the wrong merge base or no base. The agent now derives `` and `` from the same PR URL it already parses, lowercases both for case-insensitive comparison, and the awk block extracts (host, owner/repo) from each remote URL generically — handling `https://[user@]host[:port]/owner/repo[.git]`, `ssh://[user@]host[:port]/owner/repo[.git]`, and scp-form `user@host:owner/repo[.git]`. Strips userinfo, port, and trailing `.git` before comparison. The HTTPS fallback URL also uses `` instead of a hard-coded `github.com`. No behavior change for `github.com` users; enterprise hosts now resolve correctly in fork workflows. --- .../skills/ce-code-review/SKILL.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/plugins/compound-engineering/skills/ce-code-review/SKILL.md b/plugins/compound-engineering/skills/ce-code-review/SKILL.md index bc7780420..f0e404ce9 100644 --- a/plugins/compound-engineering/skills/ce-code-review/SKILL.md +++ b/plugins/compound-engineering/skills/ce-code-review/SKILL.md @@ -246,12 +246,23 @@ Then fetch PR metadata. Capture the base branch name and the PR base repository gh pr view --json title,body,baseRefName,headRefName,url,reviews,comments --jq '{title, body, baseRefName, headRefName, url, hasPriorComments: ((.reviews | map(select(.state != "APPROVED" or .body != "")) | length) > 0 or (.comments | length) > 0)}' ``` -Use the repository portion of the returned PR URL as `` (for example, `EveryInc/compound-engineering-plugin` from `https://github.com/EveryInc/compound-engineering-plugin/pull/348`). +Derive both the host and the repository portion from the returned PR URL. Both are needed because remote URLs use the same `host` + `owner/repo` pair as the PR URL but in different shapes (HTTPS, SSH, scp-form), and any host other than `github.com` (GitHub Enterprise, self-hosted) must still match correctly. For example, from `https://github.com/EveryInc/compound-engineering-plugin/pull/348` use `github.com` as `` and `EveryInc/compound-engineering-plugin` as ``. From `https://ghe.acme.com/Org/Repo/pull/12` use `ghe.acme.com` as `` and `Org/Repo` as ``. Lowercase both before substituting; host and owner/repo are case-insensitive on GitHub and remote URLs may preserve user-typed casing. -Then compute a local diff against the PR's base branch so re-reviews also include local fix commits and uncommitted edits. Substitute the PR base branch from metadata (shown here as ``) and the PR base repository identity derived from the PR URL (shown here as ``). Resolve the base ref from the PR's actual base repository, not by assuming `origin` points at that repo: +Then compute a local diff against the PR's base branch so re-reviews also include local fix commits and uncommitted edits. Substitute the PR base branch from metadata (shown here as ``), the PR base host (shown here as ``), and the PR base repository (shown here as ``). Resolve the base ref from the PR's actual base repository, not by assuming `origin` points at that repo: ``` -PR_BASE_REMOTE=$(git remote -v | awk 'index($2, "github.com:") || index($2, "github.com/") {print $1; exit}') +PR_BASE_REMOTE=$(git remote -v | awk -v host="" -v repo="" ' + { + url = $2; h = ""; p = "" + if (match(url, /:\/\/[^\/]+\//)) { h = substr(url, RSTART+3, RLENGTH-4); p = substr(url, RSTART+RLENGTH) } + else if (match(url, /@[^:]+:/)) { h = substr(url, RSTART+1, RLENGTH-2); p = substr(url, RSTART+RLENGTH) } + else next + sub(/^[^@]*@/, "", h); sub(/:[0-9]+$/, "", h) + h = tolower(h) + if (!match(p, /^[^\/]+\/[^\/]+/)) next + pr = substr(p, 1, RLENGTH); sub(/\.git$/, "", pr); pr = tolower(pr) + if (h == host && pr == repo) { print $1; exit } + }') if [ -n "$PR_BASE_REMOTE" ]; then PR_BASE_REMOTE_REF="$PR_BASE_REMOTE/"; else PR_BASE_REMOTE_REF=""; fi PR_BASE_REF=$(git rev-parse --verify "$PR_BASE_REMOTE_REF" 2>/dev/null || git rev-parse --verify 2>/dev/null || true) if [ -z "$PR_BASE_REF" ]; then @@ -259,7 +270,7 @@ if [ -z "$PR_BASE_REF" ]; then git fetch --no-tags "$PR_BASE_REMOTE" :refs/remotes/"$PR_BASE_REMOTE"/ 2>/dev/null || git fetch --no-tags "$PR_BASE_REMOTE" 2>/dev/null || true PR_BASE_REF=$(git rev-parse --verify "$PR_BASE_REMOTE_REF" 2>/dev/null || git rev-parse --verify 2>/dev/null || true) else - if git fetch --no-tags https://github.com/.git 2>/dev/null; then + if git fetch --no-tags https:///.git 2>/dev/null; then PR_BASE_REF=$(git rev-parse --verify FETCH_HEAD 2>/dev/null || true) fi if [ -z "$PR_BASE_REF" ]; then PR_BASE_REF=$(git rev-parse --verify 2>/dev/null || true); fi From fb64c71ec3b5bdbf65300f8a31ce4d22389bd92b Mon Sep 17 00:00:00 2001 From: David Lee <8321301+davidalee@users.noreply.github.com> Date: Sun, 10 May 2026 22:30:51 -0400 Subject: [PATCH 2/2] fix(code-review): preserve GHE ports in base remote matching - keep URL-form remote host ports when resolving the PR base remote - allow scp-form SSH remotes to match a ported PR host without the web UI port - add executable regression coverage for HTTPS, ssh URL, and scp-form GHE remotes Note: pre-existing failure in bun test: tests/skills/ce-release-notes-helper.test.ts cannot start Bun.serve on port 0 (EADDRINUSE). --- .../skills/ce-code-review/SKILL.md | 11 +-- ...ce-code-review-base-remote-matcher.test.ts | 85 +++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 tests/skills/ce-code-review-base-remote-matcher.test.ts diff --git a/plugins/compound-engineering/skills/ce-code-review/SKILL.md b/plugins/compound-engineering/skills/ce-code-review/SKILL.md index f0e404ce9..a108d8a7b 100644 --- a/plugins/compound-engineering/skills/ce-code-review/SKILL.md +++ b/plugins/compound-engineering/skills/ce-code-review/SKILL.md @@ -246,22 +246,23 @@ Then fetch PR metadata. Capture the base branch name and the PR base repository gh pr view --json title,body,baseRefName,headRefName,url,reviews,comments --jq '{title, body, baseRefName, headRefName, url, hasPriorComments: ((.reviews | map(select(.state != "APPROVED" or .body != "")) | length) > 0 or (.comments | length) > 0)}' ``` -Derive both the host and the repository portion from the returned PR URL. Both are needed because remote URLs use the same `host` + `owner/repo` pair as the PR URL but in different shapes (HTTPS, SSH, scp-form), and any host other than `github.com` (GitHub Enterprise, self-hosted) must still match correctly. For example, from `https://github.com/EveryInc/compound-engineering-plugin/pull/348` use `github.com` as `` and `EveryInc/compound-engineering-plugin` as ``. From `https://ghe.acme.com/Org/Repo/pull/12` use `ghe.acme.com` as `` and `Org/Repo` as ``. Lowercase both before substituting; host and owner/repo are case-insensitive on GitHub and remote URLs may preserve user-typed casing. +Derive both the host and the repository portion from the returned PR URL. Both are needed because remote URLs use the same `host` + `owner/repo` pair as the PR URL but in different shapes (HTTPS, SSH, scp-form), and any host other than `github.com` (GitHub Enterprise, self-hosted) must still match correctly. For example, from `https://github.com/EveryInc/compound-engineering-plugin/pull/348` use `github.com` as `` and `EveryInc/compound-engineering-plugin` as ``. From `https://ghe.acme.com/Org/Repo/pull/12` use `ghe.acme.com` as `` and `Org/Repo` as ``. From `https://ghe.acme.com:8443/Org/Repo/pull/12` preserve the port and use `ghe.acme.com:8443` as ``. Lowercase both before substituting; host and owner/repo are case-insensitive on GitHub and remote URLs may preserve user-typed casing. For scp-form SSH remotes (`git@host:owner/repo.git`), match the host without the PR URL port as a fallback because scp-form syntax does not carry the web UI port. Then compute a local diff against the PR's base branch so re-reviews also include local fix commits and uncommitted edits. Substitute the PR base branch from metadata (shown here as ``), the PR base host (shown here as ``), and the PR base repository (shown here as ``). Resolve the base ref from the PR's actual base repository, not by assuming `origin` points at that repo: ``` PR_BASE_REMOTE=$(git remote -v | awk -v host="" -v repo="" ' + BEGIN { host_without_port = host; sub(/:[0-9]+$/, "", host_without_port) } { - url = $2; h = ""; p = "" + url = $2; h = ""; p = ""; scp = 0 if (match(url, /:\/\/[^\/]+\//)) { h = substr(url, RSTART+3, RLENGTH-4); p = substr(url, RSTART+RLENGTH) } - else if (match(url, /@[^:]+:/)) { h = substr(url, RSTART+1, RLENGTH-2); p = substr(url, RSTART+RLENGTH) } + else if (match(url, /@[^:]+:/)) { h = substr(url, RSTART+1, RLENGTH-2); p = substr(url, RSTART+RLENGTH); scp = 1 } else next - sub(/^[^@]*@/, "", h); sub(/:[0-9]+$/, "", h) + sub(/^[^@]*@/, "", h) h = tolower(h) if (!match(p, /^[^\/]+\/[^\/]+/)) next pr = substr(p, 1, RLENGTH); sub(/\.git$/, "", pr); pr = tolower(pr) - if (h == host && pr == repo) { print $1; exit } + if ((h == host || (scp && h == host_without_port)) && pr == repo) { print $1; exit } }') if [ -n "$PR_BASE_REMOTE" ]; then PR_BASE_REMOTE_REF="$PR_BASE_REMOTE/"; else PR_BASE_REMOTE_REF=""; fi PR_BASE_REF=$(git rev-parse --verify "$PR_BASE_REMOTE_REF" 2>/dev/null || git rev-parse --verify 2>/dev/null || true) diff --git a/tests/skills/ce-code-review-base-remote-matcher.test.ts b/tests/skills/ce-code-review-base-remote-matcher.test.ts new file mode 100644 index 000000000..381261d72 --- /dev/null +++ b/tests/skills/ce-code-review-base-remote-matcher.test.ts @@ -0,0 +1,85 @@ +import { readFileSync } from "fs" +import path from "path" +import { describe, expect, test } from "bun:test" + +const SKILL_PATH = path.join( + process.cwd(), + "plugins/compound-engineering/skills/ce-code-review/SKILL.md", +) +const SKILL_BODY = readFileSync(SKILL_PATH, "utf8") + +const awkMatch = SKILL_BODY.match( + /awk -v host="" -v repo="" '\n(?[\s\S]*?\n })'\)/, +) +const AWK_PROGRAM = awkMatch?.groups?.program ?? "" + +async function matchRemote( + remotes: string, + host: string, + repo: string, +): Promise { + const proc = Bun.spawn( + ["awk", "-v", `host=${host}`, "-v", `repo=${repo}`, AWK_PROGRAM], + { + stdin: new TextEncoder().encode(remotes), + stdout: "pipe", + stderr: "pipe", + }, + ) + const stdout = await new Response(proc.stdout).text() + const stderr = await new Response(proc.stderr).text() + const exitCode = await proc.exited + + expect(stderr).toBe("") + expect(exitCode).toBe(0) + return stdout.trim() +} + +describe("ce-code-review PR base remote matcher", () => { + test("extracts the embedded AWK matcher from SKILL.md", () => { + expect(AWK_PROGRAM).not.toBe("") + }) + + test("matches HTTPS remotes on a ported GitHub Enterprise host", async () => { + await expect( + matchRemote( + "base https://ghe.acme.com:8443/Org/Repo.git (fetch)\n", + "ghe.acme.com:8443", + "org/repo", + ), + ).resolves.toBe("base") + }) + + test("matches ssh URL remotes on a ported GitHub Enterprise host", async () => { + await expect( + matchRemote( + "base ssh://git@ghe.acme.com:8443/Org/Repo.git (fetch)\n", + "ghe.acme.com:8443", + "org/repo", + ), + ).resolves.toBe("base") + }) + + test("allows scp-form SSH remotes to omit the PR URL port", async () => { + await expect( + matchRemote( + [ + "wrongport https://ghe.acme.com:9443/Org/Repo.git (fetch)", + "base git@ghe.acme.com:Org/Repo.git (fetch)", + ].join("\n") + "\n", + "ghe.acme.com:8443", + "org/repo", + ), + ).resolves.toBe("base") + }) + + test("does not match a different port for URL-form remotes", async () => { + await expect( + matchRemote( + "wrongport https://ghe.acme.com:9443/Org/Repo.git (fetch)\n", + "ghe.acme.com:8443", + "org/repo", + ), + ).resolves.toBe("") + }) +})