Skip to content

Commit ff87a60

Browse files
authored
Fix runner workspace default branch resolution (#1866)
1 parent 350dfca commit ff87a60

2 files changed

Lines changed: 36 additions & 7 deletions

File tree

.github/scripts/run-agent-task/runner-workspace-publisher.mjs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,21 @@ export async function publishRunnerWorkspace({ request, changedFiles, publicatio
99
const allowed = Array.isArray(record(request.access).allowed_repos) ? request.access.allowed_repos.map((value) => string(value).toLowerCase()) : []
1010
if (!token) throw new Error("No GitHub token is available for runner workspace publication.")
1111
if (!targetRepo || targetRepo !== configuredRepo || !allowed.includes(targetRepo)) throw new Error("Runner workspace publication repository is not authorized.")
12-
const base = string(config.base || config.base_branch || "main")
12+
let base = string(config.base) || string(config.base_branch)
13+
const hasConfiguredBase = Boolean(base)
1314
const prefix = string(config.branch_prefix || "wp-codebox/agent-task/")
1415
const runId = string(config.run_id || request.workload?.id || "agent-task").replace(/[^A-Za-z0-9._/-]+/g, "-")
1516
const head = `${prefix}${runId}`
16-
if (!/^[A-Za-z0-9._/-]+$/.test(prefix) || !head.startsWith(prefix) || head.includes("..") || !/^[A-Za-z0-9._/-]+$/.test(base)) throw new Error("Runner workspace branch configuration is invalid.")
17-
1817
const api = async (method, path, body) => {
1918
const response = await fetchImpl(`https://api.github.com/repos/${targetRepo}${path}`, { method, headers: { Accept: "application/vnd.github+json", Authorization: `Bearer ${token}`, "X-GitHub-Api-Version": "2022-11-28", ...(body ? { "Content-Type": "application/json" } : {}) }, ...(body ? { body: JSON.stringify(body) } : {}) })
2019
const payload = await response.json().catch(() => ({}))
2120
if (!response.ok) throw new Error(`GitHub API ${method} ${path} failed with ${response.status}.`)
2221
return payload
2322
}
23+
if (!base) base = string((await api("GET", "")).default_branch)
24+
if (!/^[A-Za-z0-9._/-]+$/.test(prefix) || !head.startsWith(prefix) || head.includes("..") || !/^[A-Za-z0-9._/-]+$/.test(base)) {
25+
throw new Error(hasConfiguredBase ? "Runner workspace branch configuration is invalid." : "Runner workspace branch configuration is invalid: repository metadata must provide a valid default branch when base is omitted.")
26+
}
2427
let existing = null
2528
try { existing = await api("GET", `/git/ref/heads/${head.split("/").map(encodeURIComponent).join("/")}`) } catch (error) { if (!String(error.message).includes(" 404.")) throw error }
2629
// Existing PR branches are append-only publication targets. Their current tree

tests/runner-workspace-publisher.test.mjs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,51 @@ const request = { target_repo: "owner/repo", workload: { id: "run-1", label: "Up
55
const publicationFiles = [{ path: "README.md", mode: "100644", content: Buffer.from("changed\n").toString("base64"), deleted: false }]
66

77
function response(status, body) { return { ok: status >= 200 && status < 300, status, json: async () => body } }
8-
function fetchMock(existing = false) {
8+
function fetchMock(existing = false, base = "main", defaultBranch) {
99
const calls = []
1010
return { calls, fetch: async (url, init) => {
1111
calls.push([url, init.method, init.body ? JSON.parse(init.body) : undefined])
1212
const parsed = new URL(url)
1313
const path = parsed.pathname.replace("/repos/owner/repo", "")
14-
if (path === "/git/ref/heads/main") return response(200, { object: { sha: "base" } })
14+
if (path === "") return response(200, { default_branch: defaultBranch })
15+
if (path === `/git/ref/heads/${base}`) return response(200, { object: { sha: "base" } })
1516
if (path === "/git/commits/base" || path === "/git/commits/old") return response(200, { tree: { sha: path.endsWith("old") ? "prior-tree" : "tree" } })
1617
if (path.includes("/git/ref/heads/wp-codebox/agent-task/run-1")) return existing ? response(200, { object: { sha: "old" } }) : response(404, {})
1718
if (path === "/git/blobs") return response(201, { sha: "blob" })
1819
if (path === "/git/trees") return response(201, { sha: "next-tree" })
1920
if (path === "/git/commits") return response(201, { sha: "commit" })
2021
if (path === "/git/refs") return response(201, {})
2122
if (path.includes("/git/refs/heads/")) return response(200, {})
22-
if (path === "/pulls" && parsed.search) return response(200, existing ? [{ number: 4, html_url: "https://github.com/owner/repo/pull/4", base: { repo: { full_name: "owner/repo" }, ref: "main" }, head: { ref: "wp-codebox/agent-task/run-1" } }] : [])
23-
if (path === "/pulls") return response(201, { number: 5, html_url: "https://github.com/owner/repo/pull/5", base: { repo: { full_name: "owner/repo" }, ref: "main" }, head: { ref: "wp-codebox/agent-task/run-1" } })
23+
if (path === "/pulls" && parsed.search) return response(200, existing ? [{ number: 4, html_url: "https://github.com/owner/repo/pull/4", base: { repo: { full_name: "owner/repo" }, ref: base }, head: { ref: "wp-codebox/agent-task/run-1" } }] : [])
24+
if (path === "/pulls") return response(201, { number: 5, html_url: "https://github.com/owner/repo/pull/5", base: { repo: { full_name: "owner/repo" }, ref: base }, head: { ref: "wp-codebox/agent-task/run-1" } })
2425
throw new Error(`unexpected ${path}`)
2526
} }
2627
}
28+
{
29+
const nonMainRequest = { ...request, runner_workspace: { ...request.runner_workspace, base: "release", base_branch: "legacy" } }
30+
const mock = fetchMock(false, "release")
31+
const result = await publishRunnerWorkspace({ request: nonMainRequest, changedFiles: ["README.md"], publicationFiles, token: "secret", fetchImpl: mock.fetch })
32+
assert.equal(result.branch.base, "release", "explicit base must remain authoritative")
33+
assert(!mock.calls.some(([url]) => new URL(url).pathname === "/repos/owner/repo"), "explicit base must not look up repository metadata")
34+
}
35+
{
36+
const aliasRequest = { ...request, runner_workspace: { ...request.runner_workspace, base: undefined, base_branch: "stable" } }
37+
const mock = fetchMock(false, "stable")
38+
const result = await publishRunnerWorkspace({ request: aliasRequest, changedFiles: ["README.md"], publicationFiles, token: "secret", fetchImpl: mock.fetch })
39+
assert.equal(result.branch.base, "stable", "base_branch remains a supported base alias")
40+
assert(!mock.calls.some(([url]) => new URL(url).pathname === "/repos/owner/repo"), "base_branch must not look up repository metadata")
41+
}
42+
{
43+
const metadataRequest = { ...request, runner_workspace: { ...request.runner_workspace, base: undefined, base_branch: undefined } }
44+
const mock = fetchMock(false, "trunk", "trunk")
45+
const result = await publishRunnerWorkspace({ request: metadataRequest, changedFiles: ["README.md"], publicationFiles, token: "secret", fetchImpl: mock.fetch })
46+
assert.equal(result.branch.base, "trunk", "omitted base must use the repository default branch")
47+
assert(mock.calls.some(([url]) => new URL(url).pathname === "/repos/owner/repo"), "omitted base must look up repository metadata")
48+
}
49+
for (const defaultBranch of [undefined, "invalid branch"]) {
50+
const metadataRequest = { ...request, runner_workspace: { ...request.runner_workspace, base: undefined, base_branch: undefined } }
51+
await assert.rejects(() => publishRunnerWorkspace({ request: metadataRequest, changedFiles: ["README.md"], publicationFiles, token: "secret", fetchImpl: fetchMock(false, "main", defaultBranch).fetch }), /metadata must provide a valid default branch/)
52+
}
2753

2854
{
2955
const mock = fetchMock(false)

0 commit comments

Comments
 (0)