From 46e310b3521d0b7d10dbf9a14a82697819389d66 Mon Sep 17 00:00:00 2001 From: ferueda Date: Thu, 23 Jul 2026 14:33:05 -0700 Subject: [PATCH 1/4] feat: add idempotent GitHub publication --- .oxlintrc.json | 28 ++ lib/github/client.test.ts | 118 ++++++++ lib/github/client.ts | 163 ++++++++++ lib/github/error.ts | 44 +++ lib/github/git.test.ts | 79 +++++ lib/github/git.ts | 525 +++++++++++++++++++++++++++++++++ lib/github/publication.test.ts | 388 ++++++++++++++++++++++++ lib/github/publication.ts | 260 ++++++++++++++++ lib/github/remote.test.ts | 26 ++ lib/github/remote.ts | 64 ++++ lib/github/types.ts | 82 +++++ test/import-boundaries.test.ts | 18 ++ 12 files changed, 1795 insertions(+) create mode 100644 lib/github/client.test.ts create mode 100644 lib/github/client.ts create mode 100644 lib/github/error.ts create mode 100644 lib/github/git.test.ts create mode 100644 lib/github/git.ts create mode 100644 lib/github/publication.test.ts create mode 100644 lib/github/publication.ts create mode 100644 lib/github/remote.test.ts create mode 100644 lib/github/remote.ts create mode 100644 lib/github/types.ts diff --git a/.oxlintrc.json b/.oxlintrc.json index 2f43bd9..fa58293 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -163,6 +163,34 @@ ] } }, + { + "files": ["lib/github/**/*.ts"], + "rules": { + "no-restricted-imports": [ + "error", + { + "patterns": [ + { + "regex": "^inngest(?:/|$)", + "message": "GitHub publication primitives must not depend on delivery code." + }, + { + "regex": "^@linear/sdk(?:/|$)", + "message": "GitHub publication primitives must not depend on tracker code." + }, + { + "regex": "^(?:@openai/codex(?:-sdk)?|@cursor/sdk)(?:/|$)", + "message": "GitHub publication primitives must not depend on agent providers." + }, + { + "regex": "^\\.\\./(?:\\.\\./)*(?:linear(?:/|-)|inngest(?:/|$)|triage(?:/|$)|spec(?:/|$)|implementation(?:/|$)|providers(?:/|$))", + "message": "GitHub publication owns commit, push, and pull requests, not tracker, delivery, provider, or domain policy." + } + ] + } + ] + } + }, { "files": ["providers/**/*.ts"], "rules": { diff --git a/lib/github/client.test.ts b/lib/github/client.test.ts new file mode 100644 index 0000000..73c3a39 --- /dev/null +++ b/lib/github/client.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it, vi } from "vitest"; +import { createGitHubPullRequestClient } from "./client.ts"; + +const TOKEN = "github-secret+/="; +const LOOKUP = { + repository: { + owner: "ferueda", + repository: "harness", + httpsRemote: "https://github.com/ferueda/harness.git", + }, + baseBranch: "main", + headBranch: "codex/FER-286", +} as const; + +describe("GitHub pull request client", () => { + it("uses exact lookup filters and validates returned pull requests", async () => { + const fetchRequest = vi.fn(async () => + jsonResponse([pullRequestResponse({ state: "closed", mergedAt: "2026-07-23T00:00:00Z" })]), + ); + const client = createGitHubPullRequestClient({ token: TOKEN, fetch: fetchRequest }); + + await expect(client.listPullRequests(LOOKUP)).resolves.toEqual([ + { + url: "https://github.com/ferueda/harness/pull/286", + number: 286, + state: "closed", + merged: true, + owner: "ferueda", + repository: "harness", + baseBranch: "main", + headOwner: "ferueda", + headRepository: "harness", + headBranch: "codex/FER-286", + headSha: "a".repeat(40), + }, + ]); + + const [url, options] = fetchRequest.mock.calls[0] ?? []; + expect(String(url)).toContain("state=all"); + expect(String(url)).toContain("head=ferueda%3Acodex%2FFER-286"); + expect(String(url)).toContain("base=main"); + expect(new Headers(options?.headers).get("authorization")).toBe(`Bearer ${TOKEN}`); + }); + + it("sends the minimal create payload", async () => { + const fetchRequest = vi.fn(async () => jsonResponse(pullRequestResponse())); + const client = createGitHubPullRequestClient({ token: TOKEN, fetch: fetchRequest }); + + await client.createPullRequest({ + ...LOOKUP, + title: "Add FER-286 spec", + body: "Generated by Harness", + }); + + const [, options] = fetchRequest.mock.calls[0] ?? []; + expect(options?.method).toBe("POST"); + expect(JSON.parse(String(options?.body))).toEqual({ + title: "Add FER-286 spec", + body: "Generated by Harness", + head: "codex/FER-286", + base: "main", + }); + }); + + it("rejects malformed responses", async () => { + const client = createGitHubPullRequestClient({ + token: TOKEN, + fetch: async () => jsonResponse([{ number: "not-a-number" }]), + }); + + await expect(client.listPullRequests(LOOKUP)).rejects.toMatchObject({ + code: "invalid-response", + }); + }); + + it("redacts raw and encoded credentials from request failures", async () => { + const encoded = encodeURIComponent(TOKEN); + const client = createGitHubPullRequestClient({ + token: TOKEN, + fetch: async () => { + throw new Error(`failed ${TOKEN} ${encoded}`); + }, + }); + + const error = await client.listPullRequests(LOOKUP).catch((caught: unknown) => caught); + expect(error).toMatchObject({ code: "github-failed" }); + expect(String(error)).not.toContain(TOKEN); + expect(String(error)).not.toContain(encoded); + expect(String(error)).toContain("[REDACTED]"); + }); +}); + +function pullRequestResponse( + overrides: { state?: "open" | "closed"; mergedAt?: string | null } = {}, +) { + return { + html_url: "https://github.com/ferueda/harness/pull/286", + number: 286, + state: overrides.state ?? "open", + merged_at: overrides.mergedAt ?? null, + base: { + ref: "main", + repo: { name: "harness", owner: { login: "ferueda" } }, + }, + head: { + ref: "codex/FER-286", + sha: "a".repeat(40), + repo: { name: "harness", owner: { login: "ferueda" } }, + }, + }; +} + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} diff --git a/lib/github/client.ts b/lib/github/client.ts new file mode 100644 index 0000000..cbcc315 --- /dev/null +++ b/lib/github/client.ts @@ -0,0 +1,163 @@ +import { z } from "zod"; +import { errorMessage, GitHubPublicationError, redactSecrets } from "./error.ts"; +import type { + GitHubPullRequestClient, + GitHubPullRequestLookup, + GitHubPullRequestRecord, +} from "./types.ts"; + +const PullRequestSchema = z + .object({ + html_url: z.url(), + number: z.number().int().positive(), + state: z.enum(["open", "closed"]), + merged_at: z.string().nullable(), + base: z + .object({ + ref: z.string(), + repo: z + .object({ + name: z.string(), + owner: z.object({ login: z.string() }).passthrough(), + }) + .passthrough(), + }) + .passthrough(), + head: z + .object({ + ref: z.string(), + sha: z.string().regex(/^[0-9a-f]{40,64}$/), + repo: z + .object({ + name: z.string(), + owner: z.object({ login: z.string() }).passthrough(), + }) + .passthrough(), + }) + .passthrough(), + }) + .passthrough(); + +const PullRequestListSchema = z.array(PullRequestSchema); + +export function createGitHubPullRequestClient(input: { + token: string; + fetch?: typeof globalThis.fetch; +}): GitHubPullRequestClient { + const token = input.token.trim(); + if (!token) { + throw new GitHubPublicationError( + "invalid-input", + "GitHub publication token must not be empty.", + ); + } + const fetchRequest = input.fetch ?? globalThis.fetch; + + return Object.freeze({ + async listPullRequests( + lookup: GitHubPullRequestLookup, + ): Promise { + const query = new URLSearchParams({ + state: "all", + head: `${lookup.repository.owner}:${lookup.headBranch}`, + base: lookup.baseBranch, + per_page: "100", + }); + const value = await requestJson({ + token, + fetchRequest, + method: "GET", + path: `/repos/${encodeURIComponent(lookup.repository.owner)}/${encodeURIComponent(lookup.repository.repository)}/pulls?${query}`, + }); + const parsed = PullRequestListSchema.safeParse(value); + if (!parsed.success) throw invalidResponse(parsed.error.message); + return Object.freeze(parsed.data.map(toRecord)); + }, + + async createPullRequest( + request: GitHubPullRequestLookup & Readonly<{ title: string; body: string }>, + ): Promise { + const value = await requestJson({ + token, + fetchRequest, + method: "POST", + path: `/repos/${encodeURIComponent(request.repository.owner)}/${encodeURIComponent(request.repository.repository)}/pulls`, + body: { + title: request.title, + body: request.body, + head: request.headBranch, + base: request.baseBranch, + }, + }); + const parsed = PullRequestSchema.safeParse(value); + if (!parsed.success) throw invalidResponse(parsed.error.message); + return toRecord(parsed.data); + }, + }); +} + +async function requestJson(input: { + token: string; + fetchRequest: typeof globalThis.fetch; + method: "GET" | "POST"; + path: string; + body?: Readonly>; +}): Promise { + let response: Response; + try { + response = await input.fetchRequest(`https://api.github.com${input.path}`, { + method: input.method, + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${input.token}`, + "Content-Type": "application/json", + "User-Agent": "harness", + "X-GitHub-Api-Version": "2022-11-28", + }, + ...(input.body ? { body: JSON.stringify(input.body) } : {}), + }); + } catch (error) { + throw new GitHubPublicationError( + "github-failed", + `GitHub request failed: ${redactSecrets(errorMessage(error), [input.token])}`, + ); + } + + const responseBody = await response.text(); + if (!response.ok) { + const diagnostic = redactSecrets(responseBody.slice(-2_000), [input.token]).trim(); + throw new GitHubPublicationError( + "github-failed", + `GitHub request failed with HTTP ${response.status}${diagnostic ? `: ${diagnostic}` : "."}`, + ); + } + + try { + return JSON.parse(responseBody) as unknown; + } catch { + throw invalidResponse("response body was not valid JSON"); + } +} + +function toRecord(value: z.infer): GitHubPullRequestRecord { + return Object.freeze({ + url: value.html_url, + number: value.number, + state: value.state, + merged: value.merged_at !== null, + owner: value.base.repo.owner.login, + repository: value.base.repo.name, + baseBranch: value.base.ref, + headOwner: value.head.repo.owner.login, + headRepository: value.head.repo.name, + headBranch: value.head.ref, + headSha: value.head.sha, + }); +} + +function invalidResponse(detail: string): GitHubPublicationError { + return new GitHubPublicationError( + "invalid-response", + `GitHub returned an invalid pull request response: ${detail}`, + ); +} diff --git a/lib/github/error.ts b/lib/github/error.ts new file mode 100644 index 0000000..9fd7cdc --- /dev/null +++ b/lib/github/error.ts @@ -0,0 +1,44 @@ +export const GITHUB_PUBLICATION_ERROR_CODES = [ + "invalid-input", + "run-conflict", + "changes-mismatch", + "git-failed", + "remote-conflict", + "invalid-response", + "github-failed", + "github-conflict", +] as const; + +export type GitHubPublicationErrorCode = (typeof GITHUB_PUBLICATION_ERROR_CODES)[number]; + +export class GitHubPublicationError extends Error { + readonly code: GitHubPublicationErrorCode; + + constructor(code: GitHubPublicationErrorCode, message: string, options?: ErrorOptions) { + super(message, options); + this.name = "GitHubPublicationError"; + this.code = code; + } +} + +export function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +export function redactSecrets(value: string, secrets: readonly string[]): string { + let redacted = value; + for (const secret of secrets.flatMap(secretForms).filter(Boolean)) { + redacted = redacted.replaceAll(secret, "[REDACTED]"); + } + return redacted; +} + +function secretForms(secret: string): readonly string[] { + if (!secret) return []; + return Object.freeze([ + secret, + encodeURIComponent(secret), + Buffer.from(secret).toString("base64"), + Buffer.from(`x-access-token:${secret}`).toString("base64"), + ]); +} diff --git a/lib/github/git.test.ts b/lib/github/git.test.ts new file mode 100644 index 0000000..70aa293 --- /dev/null +++ b/lib/github/git.test.ts @@ -0,0 +1,79 @@ +import { existsSync } from "node:fs"; +import { readFile, stat } from "node:fs/promises"; +import { describe, expect, it } from "vitest"; +import { createAuthenticatedGitTransport, type AuthenticatedGitExecutor } from "./git.ts"; + +describe("authenticated Git transport", () => { + it("keeps credentials out of arguments and static askpass content", async () => { + const token = "github-secret+/="; + let captured: + | Readonly<{ + args: readonly string[]; + environment: Readonly>; + helper: string; + mode: number; + }> + | undefined; + const executor: AuthenticatedGitExecutor = async (input) => { + const helperPath = input.environment.GIT_ASKPASS ?? ""; + captured = { + args: input.args, + environment: input.environment, + helper: await readFile(helperPath, "utf8"), + mode: (await stat(helperPath)).mode & 0o777, + }; + return ""; + }; + const transport = createAuthenticatedGitTransport({ + executor, + environment: { + PATH: process.env.PATH, + GIT_TRACE: "1", + GIT_CURL_VERBOSE: "1", + }, + }); + + await transport.pushBranch({ + workspace: process.cwd(), + remote: "https://github.com/ferueda/harness.git", + branch: "codex/FER-286", + token, + }); + + expect(captured).toBeDefined(); + expect(captured?.args.join(" ")).not.toContain(token); + expect(captured?.args.join(" ")).toContain("credential.helper="); + expect(captured?.args.join(" ")).toContain("core.hooksPath="); + expect(captured?.helper).not.toContain(token); + expect(captured?.helper).toContain("HARNESS_GITHUB_TOKEN"); + expect(captured?.mode).toBe(0o700); + expect(captured?.environment.HARNESS_GITHUB_TOKEN).toBe(token); + expect(captured?.environment.GIT_TERMINAL_PROMPT).toBe("0"); + expect(captured?.environment.GIT_TRACE).toBeUndefined(); + expect(captured?.environment.GIT_CURL_VERBOSE).toBeUndefined(); + expect(existsSync(captured?.environment.GIT_ASKPASS ?? "")).toBe(false); + }); + + it("redacts credential forms from authenticated Git errors", async () => { + const token = "github-secret+/="; + const encoded = encodeURIComponent(token); + const transport = createAuthenticatedGitTransport({ + executor: async () => { + throw new Error(`push failed ${token} ${encoded}`); + }, + }); + + const error = await transport + .pushBranch({ + workspace: process.cwd(), + remote: "https://github.com/ferueda/harness.git", + branch: "codex/FER-286", + token, + }) + .catch((caught: unknown) => caught); + expect(error).toMatchObject({ code: "git-failed" }); + expect(String(error)).not.toContain(token); + expect(String(error)).not.toContain(encoded); + expect(String(error)).toContain("[REDACTED]"); + }); +}); diff --git a/lib/github/git.ts b/lib/github/git.ts new file mode 100644 index 0000000..f389b2e --- /dev/null +++ b/lib/github/git.ts @@ -0,0 +1,525 @@ +import { execFile } from "node:child_process"; +import { chmod, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { isAbsolute, join, resolve } from "node:path"; +import { promisify } from "node:util"; +import { errorMessage, GitHubPublicationError, redactSecrets } from "./error.ts"; +import type { GitHubPublicationAuthor, GitPushInput, GitPushTransport } from "./types.ts"; +import { inspectGitChanges } from "../repository/git.ts"; +import type { + RepositoryChange, + RepositoryChangeStatus, + RepositoryRun, +} from "../repository/types.ts"; + +const execFileAsync = promisify(execFile); +const FULL_GIT_SHA = /^[0-9a-f]{40,64}$/; +const RUN_ID = /^[A-Za-z0-9][A-Za-z0-9._:/-]{0,199}$/; +const TRAILER = "Harness-Run-ID"; +const NULL_DEVICE = process.platform === "win32" ? "NUL" : "/dev/null"; +const GIT_CONFIG_ARGS = Object.freeze([ + "-c", + "credential.helper=", + "-c", + `core.hooksPath=${NULL_DEVICE}`, + "-c", + "commit.gpgsign=false", +]); +const AUTH_ENVIRONMENT_KEYS = Object.freeze([ + "HOME", + "LANG", + "LC_ALL", + "LC_CTYPE", + "NODE_EXTRA_CA_CERTS", + "PATH", + "SSL_CERT_DIR", + "SSL_CERT_FILE", + "TMPDIR", +] as const); +const ASKPASS_SOURCE = `#!/bin/sh +case "$1" in + *Username*) printf '%s\\n' 'x-access-token' ;; + *Password*) printf '%s\\n' "$HARNESS_GITHUB_TOKEN" ;; + *) exit 1 ;; +esac +`; + +export type AuthenticatedGitExecutor = (input: { + workspace: string; + args: readonly string[]; + environment: Readonly>; +}) => Promise; + +export function createAuthenticatedGitTransport( + options: { + executor?: AuthenticatedGitExecutor; + environment?: NodeJS.ProcessEnv; + } = {}, +): GitPushTransport { + const executor = options.executor ?? executeGit; + const environment = options.environment ?? process.env; + + return Object.freeze({ + async readRemoteBranch(input: GitPushInput): Promise { + const output = await runAuthenticatedGit({ + ...input, + executor, + environment, + args: ["ls-remote", "--heads", "--", input.remote, `refs/heads/${input.branch}`], + }); + return parseRemoteHead(output, input.branch); + }, + + async pushBranch(input: GitPushInput): Promise { + await runAuthenticatedGit({ + ...input, + executor, + environment, + args: ["push", "--porcelain", "--", input.remote, `HEAD:refs/heads/${input.branch}`], + }); + }, + }); +} + +export async function preparePublicationCommit(input: { + run: RepositoryRun; + expectedChanges: readonly RepositoryChange[]; + author: GitHubPublicationAuthor; + commitMessage: string; +}): Promise { + const { run } = input; + assertRunIdentity(run); + await assertWorkspace(run); + await assertBranchName(run.branch, "head branch"); + const fullMessage = publicationCommitMessage(input.commitMessage, run.id); + const headSha = (await runLocalGit(run.workspace, ["rev-parse", "--verify", "HEAD"])).trim(); + + if (headSha === run.baseSha) { + const currentChanges = await inspectGitChanges(run.workspace); + assertChangesEqual(currentChanges, input.expectedChanges, "workspace"); + await stageExpectedChanges(run.workspace, input.expectedChanges); + const stagedChanges = await readStagedChanges(run.workspace); + assertPublishedChangesEqual(stagedChanges, input.expectedChanges, "staged"); + await commitChanges(run.workspace, fullMessage, input.author); + } else { + const currentChanges = await inspectGitChanges(run.workspace); + if (currentChanges.length !== 0) { + throw new GitHubPublicationError( + "run-conflict", + "Repository run contains both a prior commit and uncommitted changes.", + ); + } + } + + const committedSha = (await runLocalGit(run.workspace, ["rev-parse", "--verify", "HEAD"])).trim(); + await assertPublicationCommit({ + workspace: run.workspace, + baseSha: run.baseSha, + commitSha: committedSha, + expectedChanges: input.expectedChanges, + author: input.author, + fullMessage, + }); + return committedSha; +} + +export async function assertBranchName(branch: string, description: string): Promise { + if (!branch.trim() || containsCommandControl(branch)) { + throw new GitHubPublicationError("invalid-input", `${description} must be a Git branch name.`); + } + try { + await runLocalGit(process.cwd(), ["check-ref-format", "--branch", branch]); + } catch { + throw new GitHubPublicationError("invalid-input", `${description} must be a Git branch name.`); + } +} + +async function assertWorkspace(run: RepositoryRun): Promise { + if (!isAbsolute(run.workspace)) { + throw new GitHubPublicationError("invalid-input", "Repository run workspace must be absolute."); + } + const [workspace, root, branch, remote] = await Promise.all([ + realpath(run.workspace), + runLocalGit(run.workspace, ["rev-parse", "--show-toplevel"]).then((value) => + realpath(value.trim()), + ), + runLocalGit(run.workspace, ["branch", "--show-current"]).then((value) => value.trim()), + runLocalGit(run.workspace, ["remote", "get-url", "origin"]).then((value) => value.trim()), + ]); + if (workspace !== root || branch !== run.branch || remote !== run.remote) { + throw new GitHubPublicationError( + "run-conflict", + "Repository run workspace, branch, or origin no longer matches its durable identity.", + ); + } + const pushUrls = ( + await runLocalGit(run.workspace, ["config", "--get-all", "remote.origin.pushurl"], { + acceptedExitCodes: [0, 1], + }) + ).trim(); + if (pushUrls) { + throw new GitHubPublicationError( + "run-conflict", + "Repository run must not override the origin push URL.", + ); + } +} + +function assertRunIdentity(run: RepositoryRun): void { + if ( + run.version !== 1 || + !RUN_ID.test(run.id) || + !FULL_GIT_SHA.test(run.baseSha) || + !run.remote.trim() || + containsCommandControl(run.remote) + ) { + throw new GitHubPublicationError("invalid-input", "Repository run identity is invalid."); + } +} + +function containsCommandControl(value: string): boolean { + return value.includes("\u0000") || value.includes("\r") || value.includes("\n"); +} + +function publicationCommitMessage(message: string, runId: string): string { + const trimmed = message.trim(); + if (!trimmed || trimmed.includes("\u0000") || new RegExp(`^${TRAILER}:`, "im").test(trimmed)) { + throw new GitHubPublicationError( + "invalid-input", + `Commit message must be non-empty and must not contain a ${TRAILER} trailer.`, + ); + } + return `${trimmed}\n\n${TRAILER}: ${runId}`; +} + +async function stageExpectedChanges( + workspace: string, + expectedChanges: readonly RepositoryChange[], +): Promise { + const paths = new Set(); + for (const change of expectedChanges) { + paths.add(change.path); + if (change.previousPath) paths.add(change.previousPath); + } + if (paths.size === 0 || expectedChanges.some((change) => change.status === "conflicted")) { + throw new GitHubPublicationError( + "invalid-input", + "Publication requires at least one non-conflicted expected change.", + ); + } + await runLocalGit(workspace, ["--literal-pathspecs", "add", "-A", "--", ...paths]); +} + +async function commitChanges( + workspace: string, + message: string, + author: GitHubPublicationAuthor, +): Promise { + await runLocalGit(workspace, ["commit", "--no-gpg-sign", "--no-verify", "-m", message], { + environment: { + GIT_AUTHOR_NAME: author.name, + GIT_AUTHOR_EMAIL: author.email, + GIT_COMMITTER_NAME: author.name, + GIT_COMMITTER_EMAIL: author.email, + }, + }); +} + +async function assertPublicationCommit(input: { + workspace: string; + baseSha: string; + commitSha: string; + expectedChanges: readonly RepositoryChange[]; + author: GitHubPublicationAuthor; + fullMessage: string; +}): Promise { + const details = await readCommitDetails(input.workspace, input.commitSha); + if ( + details.parents.length !== 1 || + details.parents[0] !== input.baseSha || + details.authorName !== input.author.name || + details.authorEmail !== input.author.email || + details.message !== input.fullMessage + ) { + throw new GitHubPublicationError( + "run-conflict", + "Existing repository commit does not match this publication run.", + ); + } + const committedChanges = await readCommittedChanges( + input.workspace, + input.baseSha, + input.commitSha, + ); + assertPublishedChangesEqual(committedChanges, input.expectedChanges, "committed"); +} + +async function readCommitDetails( + workspace: string, + commitSha: string, +): Promise< + Readonly<{ + parents: readonly string[]; + authorName: string; + authorEmail: string; + message: string; + }> +> { + const output = await runLocalGit(workspace, [ + "show", + "-s", + "--format=%P%x00%an%x00%ae%x00%B", + commitSha, + ]); + const [parents = "", authorName = "", authorEmail = "", ...messageParts] = output.split("\0"); + return Object.freeze({ + parents: Object.freeze(parents.trim().split(/\s+/).filter(Boolean)), + authorName, + authorEmail, + message: messageParts.join("\0").trimEnd(), + }); +} + +async function readStagedChanges(workspace: string): Promise { + const output = await runLocalGit(workspace, [ + "diff", + "--cached", + "--name-status", + "-z", + "-M", + "-C", + "--no-ext-diff", + "HEAD", + ]); + return parseNameStatus(output); +} + +async function readCommittedChanges( + workspace: string, + baseSha: string, + commitSha: string, +): Promise { + const output = await runLocalGit(workspace, [ + "diff", + "--name-status", + "-z", + "-M", + "-C", + "--no-ext-diff", + baseSha, + commitSha, + ]); + return parseNameStatus(output); +} + +function parseNameStatus(output: string): readonly RepositoryChange[] { + if (!output) return Object.freeze([]); + const fields = output.split("\0"); + const changes: RepositoryChange[] = []; + for (let index = 0; index < fields.length;) { + const code = fields[index++]; + if (!code) continue; + const status = nameStatus(code); + if (status === "renamed" || status === "copied") { + const previousPath = fields[index++]; + const path = fields[index++]; + if (!previousPath || !path) throw invalidNameStatus(); + changes.push(Object.freeze({ path, previousPath, status })); + continue; + } + const path = fields[index++]; + if (!path) throw invalidNameStatus(); + changes.push(Object.freeze({ path, status })); + } + return Object.freeze(changes); +} + +function nameStatus(code: string): RepositoryChangeStatus { + if (code.startsWith("R")) return "renamed"; + if (code.startsWith("C")) return "copied"; + if (code === "A") return "added"; + if (code === "D") return "deleted"; + if (code === "U") return "conflicted"; + if (code === "M" || code === "T") return "modified"; + throw invalidNameStatus(); +} + +function invalidNameStatus(): GitHubPublicationError { + return new GitHubPublicationError("git-failed", "Git returned an invalid change record."); +} + +function assertChangesEqual( + actual: readonly RepositoryChange[], + expected: readonly RepositoryChange[], + stage: string, +): void { + if (canonicalChanges(actual) !== canonicalChanges(expected)) { + throw new GitHubPublicationError( + "changes-mismatch", + `Repository ${stage} changes do not match the approved change set.`, + ); + } +} + +function assertPublishedChangesEqual( + actual: readonly RepositoryChange[], + expected: readonly RepositoryChange[], + stage: string, +): void { + if (canonicalPublishedChanges(actual) !== canonicalPublishedChanges(expected)) { + throw new GitHubPublicationError( + "changes-mismatch", + `Repository ${stage} changes do not match the approved change set.`, + ); + } +} + +function canonicalChanges(changes: readonly RepositoryChange[]): string { + return JSON.stringify( + changes + .map((change) => ({ + path: change.path, + previousPath: change.previousPath ?? "", + status: change.status, + })) + .sort(compareChanges), + ); +} + +function canonicalPublishedChanges(changes: readonly RepositoryChange[]): string { + const normalized: Array<{ path: string; previousPath: string; status: string }> = []; + for (const change of changes) { + if (change.status === "renamed") { + if (change.previousPath) { + normalized.push({ path: change.previousPath, previousPath: "", status: "deleted" }); + } + normalized.push({ path: change.path, previousPath: "", status: "added" }); + continue; + } + if (change.status === "copied" || change.status === "untracked") { + normalized.push({ path: change.path, previousPath: "", status: "added" }); + continue; + } + normalized.push({ path: change.path, previousPath: "", status: change.status }); + } + return JSON.stringify(normalized.sort(compareChanges)); +} + +function compareChanges( + left: { path: string; previousPath: string; status: string }, + right: { path: string; previousPath: string; status: string }, +): number { + return ( + left.path.localeCompare(right.path) || + left.status.localeCompare(right.status) || + left.previousPath.localeCompare(right.previousPath) + ); +} + +function parseRemoteHead(output: string, branch: string): string | null { + const lines = output.trim().split("\n").filter(Boolean); + if (lines.length === 0) return null; + if (lines.length !== 1) { + throw new GitHubPublicationError( + "remote-conflict", + `Git remote returned multiple heads for ${branch}.`, + ); + } + const [sha, ref, ...rest] = (lines[0] ?? "").split(/\s+/); + if (rest.length !== 0 || !sha || !FULL_GIT_SHA.test(sha) || ref !== `refs/heads/${branch}`) { + throw new GitHubPublicationError( + "git-failed", + `Git remote returned an invalid head for ${branch}.`, + ); + } + return sha; +} + +async function runAuthenticatedGit( + input: GitPushInput & { + args: readonly string[]; + executor: AuthenticatedGitExecutor; + environment: NodeJS.ProcessEnv; + }, +): Promise { + const helperDirectory = await mkdtemp(join(tmpdir(), "harness-github-askpass-")); + const helperPath = join(helperDirectory, "askpass.sh"); + try { + await writeFile(helperPath, ASKPASS_SOURCE, { encoding: "utf8", flag: "wx", mode: 0o700 }); + await chmod(helperPath, 0o700); + const environment = authenticatedGitEnvironment(input.environment, helperPath, input.token); + return await input.executor({ + workspace: input.workspace, + args: [...GIT_CONFIG_ARGS, ...input.args], + environment, + }); + } catch (error) { + throw new GitHubPublicationError( + "git-failed", + `Authenticated Git command failed: ${redactSecrets(errorMessage(error), [input.token])}`, + ); + } finally { + await rm(helperDirectory, { force: true, recursive: true }); + } +} + +function authenticatedGitEnvironment( + source: NodeJS.ProcessEnv, + helperPath: string, + token: string, +): Readonly> { + const environment: Record = { + GIT_ASKPASS: helperPath, + GIT_ASKPASS_REQUIRE: "force", + GIT_CONFIG_GLOBAL: NULL_DEVICE, + GIT_CONFIG_NOSYSTEM: "1", + GIT_TERMINAL_PROMPT: "0", + HARNESS_GITHUB_TOKEN: token, + }; + for (const key of AUTH_ENVIRONMENT_KEYS) { + const value = source[key]; + if (value !== undefined) environment[key] = value; + } + return Object.freeze(environment); +} + +async function executeGit(input: { + workspace: string; + args: readonly string[]; + environment: Readonly>; +}): Promise { + const { stdout } = await execFileAsync("git", [...input.args], { + cwd: input.workspace, + encoding: "utf8", + env: { ...input.environment }, + maxBuffer: 8 * 1024 * 1024, + }); + return stdout; +} + +async function runLocalGit( + workspace: string, + args: readonly string[], + options: { + acceptedExitCodes?: readonly number[]; + environment?: Readonly>; + } = {}, +): Promise { + try { + const { stdout } = await execFileAsync("git", [...GIT_CONFIG_ARGS, ...args], { + cwd: resolve(workspace), + encoding: "utf8", + env: { ...process.env, GIT_TERMINAL_PROMPT: "0", ...options.environment }, + maxBuffer: 8 * 1024 * 1024, + }); + return stdout; + } catch (error) { + const exitCode = (error as { code?: unknown }).code; + if (typeof exitCode === "number" && options.acceptedExitCodes?.includes(exitCode)) { + return ""; + } + throw new GitHubPublicationError("git-failed", `Git command failed: ${errorMessage(error)}`, { + cause: error, + }); + } +} diff --git a/lib/github/publication.test.ts b/lib/github/publication.test.ts new file mode 100644 index 0000000..4ab6ef8 --- /dev/null +++ b/lib/github/publication.test.ts @@ -0,0 +1,388 @@ +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { preparePublicationCommit } from "./git.ts"; +import { createGitHubPublicationForClient } from "./publication.ts"; +import type { + GitHubPullRequestClient, + GitHubPullRequestRecord, + GitPushTransport, +} from "./types.ts"; +import type { RepositoryChange, RepositoryRun } from "../repository/types.ts"; +import { inspectGitChanges } from "../repository/git.ts"; + +const AUTHOR = Object.freeze({ name: "Harness", email: "harness@example.com" }); +const TOKEN = "github-secret"; +const BRANCH = "codex/FER-286"; +const EXPECTED_CHANGES = Object.freeze([ + Object.freeze({ path: "dev/plans/FER-286.md", status: "untracked" as const }), +]); +const roots: string[] = []; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { force: true, recursive: true }); +}); + +describe("GitHub publication", () => { + it("commits, pushes, creates one PR, and converges on retry", async () => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + const transport = localTransport(fixture); + const github = fakeGitHub(fixture); + const publication = createPublication(fixture, transport.value, github.client); + + const first = await publication.publishPullRequest(request(fixture)); + const second = await publication.publishPullRequest(request(fixture)); + + expect(second).toEqual(first); + expect(Object.isFrozen(first)).toBe(true); + expect(git(fixture.workspace, ["rev-list", "--count", `${fixture.baseSha}..HEAD`])).toBe("1"); + expect(git(fixture.workspace, ["show", "-s", "--format=%B", "HEAD"])).toContain( + `Harness-Run-ID: ${fixture.run.id}`, + ); + expect(git(fixture.remote, ["rev-parse", `refs/heads/${BRANCH}`])).toBe(first.headSha); + expect(transport.counts()).toEqual({ reads: 2, pushes: 1 }); + expect(github.counts()).toEqual({ lists: 2, creates: 1 }); + }); + + it("rejects changed paths before committing or using remote services", async () => { + const fixture = createFixture(); + writeFileSync(join(fixture.workspace, "unexpected.txt"), "unexpected\n", "utf8"); + const transport = localTransport(fixture); + const github = fakeGitHub(fixture); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).rejects.toMatchObject({ + code: "changes-mismatch", + }); + expect(git(fixture.workspace, ["rev-parse", "HEAD"])).toBe(fixture.baseSha); + expect(transport.counts()).toEqual({ reads: 0, pushes: 0 }); + expect(github.counts()).toEqual({ lists: 0, creates: 0 }); + }); + + it("rejects an unmarked agent-created commit", async () => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + git(fixture.workspace, ["add", "."]); + git(fixture.workspace, ["commit", "-m", "Agent-created commit"]); + const transport = localTransport(fixture); + const github = fakeGitHub(fixture); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).rejects.toMatchObject({ + code: "run-conflict", + }); + expect(transport.counts()).toEqual({ reads: 0, pushes: 0 }); + }); + + it("publishes a renamed file from the caller-approved workspace snapshot", async () => { + const fixture = createFixture(); + renameSync(join(fixture.workspace, "README.md"), join(fixture.workspace, "README-renamed.md")); + const expectedChanges = await inspectGitChanges(fixture.workspace); + const transport = localTransport(fixture); + const github = fakeGitHub(fixture); + const publication = createPublication(fixture, transport.value, github.client); + + await expect( + publication.publishPullRequest(request(fixture, expectedChanges)), + ).resolves.toMatchObject({ number: 286 }); + expect( + git(fixture.workspace, ["diff", "--name-status", "-M", fixture.baseSha, "HEAD"]), + ).toContain("README-renamed.md"); + }); + + it("fails closed when the remote branch points to another commit", async () => { + const fixture = createFixture(); + git(fixture.source, ["checkout", "-b", BRANCH, fixture.baseSha]); + writeFileSync(join(fixture.source, "other.txt"), "other\n", "utf8"); + git(fixture.source, ["add", "other.txt"]); + git(fixture.source, ["commit", "-m", "Conflicting publication"]); + git(fixture.source, ["push", fixture.remote, `HEAD:refs/heads/${BRANCH}`]); + writeSpec(fixture.workspace); + const transport = localTransport(fixture); + const github = fakeGitHub(fixture); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).rejects.toMatchObject({ + code: "remote-conflict", + }); + expect(transport.counts()).toEqual({ reads: 1, pushes: 0 }); + expect(github.counts()).toEqual({ lists: 0, creates: 0 }); + }); + + it("recovers when push succeeds before its response is lost", async () => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + const transport = localTransport(fixture, { loseFirstPushResponse: true }); + const github = fakeGitHub(fixture); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).resolves.toMatchObject({ + number: 286, + }); + expect(transport.counts()).toEqual({ reads: 2, pushes: 1 }); + expect(github.counts()).toEqual({ lists: 1, creates: 1 }); + }); + + it("performs one lookup after an ambiguous PR creation failure", async () => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + const transport = localTransport(fixture); + const github = fakeGitHub(fixture, { loseCreateResponse: true }); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).resolves.toMatchObject({ + number: 286, + }); + expect(github.counts()).toEqual({ lists: 2, creates: 1 }); + }); + + it.each([ + { state: "open" as const, merged: false }, + { state: "closed" as const, merged: false }, + { state: "closed" as const, merged: true }, + ])("returns one existing $state PR without replacing it", async ({ state, merged }) => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + const headSha = await preparePublicationCommit({ + run: fixture.run, + expectedChanges: EXPECTED_CHANGES, + author: AUTHOR, + commitMessage: "Add FER-286 spec", + }); + const transport = localTransport(fixture); + await transport.value.pushBranch(pushInput(fixture)); + const github = fakeGitHub(fixture, { + initialRecords: [pullRequestRecord(headSha, { state, merged })], + }); + const publication = createPublication(fixture, transport.value, github.client); + + const result = await publication.publishPullRequest(request(fixture)); + + expect(result).toMatchObject({ state, merged, headSha }); + expect(github.counts()).toEqual({ lists: 1, creates: 0 }); + }); + + it("rejects multiple matching pull requests", async () => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + const headSha = await preparePublicationCommit({ + run: fixture.run, + expectedChanges: EXPECTED_CHANGES, + author: AUTHOR, + commitMessage: "Add FER-286 spec", + }); + const transport = localTransport(fixture); + await transport.value.pushBranch(pushInput(fixture)); + const github = fakeGitHub(fixture, { + initialRecords: [pullRequestRecord(headSha), { ...pullRequestRecord(headSha), number: 287 }], + }); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).rejects.toMatchObject({ + code: "github-conflict", + }); + }); + + it("rejects a pull request whose head SHA does not match", async () => { + const fixture = createFixture(); + writeSpec(fixture.workspace); + const headSha = await preparePublicationCommit({ + run: fixture.run, + expectedChanges: EXPECTED_CHANGES, + author: AUTHOR, + commitMessage: "Add FER-286 spec", + }); + const transport = localTransport(fixture); + await transport.value.pushBranch(pushInput(fixture)); + const github = fakeGitHub(fixture, { + initialRecords: [ + pullRequestRecord(headSha.replace(/^./, headSha.startsWith("a") ? "b" : "a")), + ], + }); + const publication = createPublication(fixture, transport.value, github.client); + + await expect(publication.publishPullRequest(request(fixture))).rejects.toMatchObject({ + code: "github-conflict", + }); + }); +}); + +type Fixture = Readonly<{ + root: string; + remote: string; + source: string; + workspace: string; + baseSha: string; + run: RepositoryRun; +}>; + +function createFixture(): Fixture { + const root = mkdtempSync(join(tmpdir(), "harness-github-publication-")); + roots.push(root); + const remote = join(root, "remote.git"); + const source = join(root, "source"); + const workspace = join(root, "workspace"); + + git(root, ["init", "--bare", remote]); + git(root, ["clone", remote, source]); + configureAuthor(source); + writeFileSync(join(source, "README.md"), "# Fixture\n", "utf8"); + git(source, ["add", "README.md"]); + git(source, ["commit", "-m", "Initialize fixture"]); + git(source, ["branch", "-M", "main"]); + git(source, ["push", "--set-upstream", "origin", "main"]); + git(remote, ["symbolic-ref", "HEAD", "refs/heads/main"]); + const baseSha = git(source, ["rev-parse", "HEAD"]); + + git(root, ["clone", remote, workspace]); + configureAuthor(workspace); + git(workspace, ["checkout", "-b", BRANCH, baseSha]); + git(workspace, ["remote", "set-url", "origin", "https://github.com/ferueda/harness.git"]); + const run = Object.freeze({ + version: 1 as const, + id: "work-spec-FER-286", + workspace, + remote: "https://github.com/ferueda/harness.git", + baseRef: "main", + baseSha, + branch: BRANCH, + }); + return Object.freeze({ root, remote, source, workspace, baseSha, run }); +} + +function createPublication( + fixture: Fixture, + gitTransport: GitPushTransport, + client: GitHubPullRequestClient, +) { + return createGitHubPublicationForClient({ + token: TOKEN, + author: AUTHOR, + gitTransport, + client, + }); +} + +function request( + fixture: Fixture, + expectedChanges: readonly RepositoryChange[] = EXPECTED_CHANGES, +) { + return { + run: fixture.run, + expectedChanges, + baseBranch: "main", + commitMessage: "Add FER-286 spec", + title: "Add FER-286 spec", + body: "Generated by Harness", + }; +} + +function writeSpec(workspace: string): void { + mkdirSync(join(workspace, "dev", "plans"), { recursive: true }); + writeFileSync(join(workspace, "dev", "plans", "FER-286.md"), "# FER-286\n", "utf8"); +} + +function localTransport(fixture: Fixture, options: { loseFirstPushResponse?: boolean } = {}) { + let reads = 0; + let pushes = 0; + let lost = false; + const value: GitPushTransport = { + async readRemoteBranch(input) { + reads += 1; + const output = git(input.workspace, [ + "ls-remote", + "--heads", + fixture.remote, + `refs/heads/${input.branch}`, + ]); + if (!output) return null; + return output.split(/\s+/)[0] ?? null; + }, + async pushBranch(input) { + pushes += 1; + git(input.workspace, ["push", fixture.remote, `HEAD:refs/heads/${input.branch}`]); + if (options.loseFirstPushResponse && !lost) { + lost = true; + throw new Error("push response lost"); + } + }, + }; + return { + value, + counts: () => ({ reads, pushes }), + }; +} + +function fakeGitHub( + fixture: Fixture, + options: { + loseCreateResponse?: boolean; + initialRecords?: readonly GitHubPullRequestRecord[]; + } = {}, +) { + let lists = 0; + let creates = 0; + let records = [...(options.initialRecords ?? [])]; + const client: GitHubPullRequestClient = { + async listPullRequests() { + lists += 1; + return Object.freeze([...records]); + }, + async createPullRequest() { + creates += 1; + const record = pullRequestRecord(git(fixture.workspace, ["rev-parse", "HEAD"])); + records = [record]; + if (options.loseCreateResponse) throw new Error("PR response lost"); + return record; + }, + }; + return { + client, + counts: () => ({ lists, creates }), + }; +} + +function pullRequestRecord( + headSha: string, + overrides: { state?: "open" | "closed"; merged?: boolean } = {}, +): GitHubPullRequestRecord { + return Object.freeze({ + url: "https://github.com/ferueda/harness/pull/286", + number: 286, + state: overrides.state ?? "open", + merged: overrides.merged ?? false, + owner: "ferueda", + repository: "harness", + baseBranch: "main", + headOwner: "ferueda", + headRepository: "harness", + headBranch: BRANCH, + headSha, + }); +} + +function pushInput(fixture: Fixture) { + return { + workspace: fixture.workspace, + remote: "https://github.com/ferueda/harness.git", + branch: BRANCH, + token: TOKEN, + }; +} + +function configureAuthor(workspace: string): void { + git(workspace, ["config", "user.name", "Fixture"]); + git(workspace, ["config", "user.email", "fixture@example.com"]); +} + +function git(cwd: string, args: readonly string[]): string { + return execFileSync("git", [...args], { + cwd, + encoding: "utf8", + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + stdio: ["ignore", "pipe", "pipe"], + }).trim(); +} diff --git a/lib/github/publication.ts b/lib/github/publication.ts new file mode 100644 index 0000000..bb3c86e --- /dev/null +++ b/lib/github/publication.ts @@ -0,0 +1,260 @@ +import { z } from "zod"; +import { createGitHubPullRequestClient } from "./client.ts"; +import { GitHubPublicationError } from "./error.ts"; +import { + assertBranchName, + createAuthenticatedGitTransport, + preparePublicationCommit, +} from "./git.ts"; +import { parseGitHubRemote } from "./remote.ts"; +import type { + CreateGitHubPublicationOptions, + GitHubPublicationAuthor, + GitHubPublicationService, + GitHubPullRequestClient, + GitHubPullRequestLookup, + GitHubPullRequestRecord, + GitPushTransport, + PublishedPullRequest, + PublishPullRequestInput, +} from "./types.ts"; + +const ChangeSchema = z + .object({ + path: z.string().min(1), + previousPath: z.string().min(1).optional(), + status: z.enum([ + "added", + "modified", + "deleted", + "renamed", + "copied", + "untracked", + "conflicted", + ]), + }) + .strict(); + +const PublishPullRequestInputSchema = z + .object({ + run: z + .object({ + version: z.literal(1), + id: z.string().min(1), + workspace: z.string().min(1), + remote: z.string().min(1), + baseRef: z.string().min(1), + baseSha: z.string().regex(/^[0-9a-f]{40,64}$/), + branch: z.string().min(1), + }) + .strict(), + expectedChanges: z.array(ChangeSchema).min(1), + baseBranch: z.string().min(1), + commitMessage: z.string().min(1), + title: z.string().trim().min(1), + body: z.string(), + }) + .strict(); + +const AuthorSchema = z + .object({ + name: z.string().trim().min(1), + email: z.email(), + }) + .strict(); + +export function createGitHubPublication( + options: CreateGitHubPublicationOptions, +): GitHubPublicationService { + const token = options.token?.trim(); + const author = AuthorSchema.safeParse(options.author); + if (!token || !author.success) { + throw new GitHubPublicationError( + "invalid-input", + "GitHub publication requires a token and an explicit commit author name and email.", + ); + } + return createGitHubPublicationForClient({ + token, + author: Object.freeze(author.data), + client: createGitHubPullRequestClient({ token, fetch: options.fetch }), + gitTransport: createAuthenticatedGitTransport(), + }); +} + +export function createGitHubPublicationForClient(input: { + token: string; + author: GitHubPublicationAuthor; + client: GitHubPullRequestClient; + gitTransport: GitPushTransport; +}): GitHubPublicationService { + const token = input.token.trim(); + const author = AuthorSchema.safeParse(input.author); + if (!token || !author.success) { + throw new GitHubPublicationError( + "invalid-input", + "GitHub publication requires a token and an explicit commit author name and email.", + ); + } + + return Object.freeze({ + async publishPullRequest(request: PublishPullRequestInput): Promise { + const parsed = PublishPullRequestInputSchema.safeParse(request); + if (!parsed.success) { + throw new GitHubPublicationError( + "invalid-input", + `GitHub publication input is invalid: ${parsed.error.issues + .map((issue) => `${issue.path.join(".") || "$"}: ${issue.message}`) + .join("; ")}`, + ); + } + + const repository = parseGitHubRemote(parsed.data.run.remote); + await assertBranchName(parsed.data.baseBranch, "base branch"); + const headSha = await preparePublicationCommit({ + run: parsed.data.run, + expectedChanges: parsed.data.expectedChanges, + author: author.data, + commitMessage: parsed.data.commitMessage, + }); + + await ensureRemoteBranch({ + gitTransport: input.gitTransport, + token, + workspace: parsed.data.run.workspace, + remote: repository.httpsRemote, + branch: parsed.data.run.branch, + headSha, + }); + + const lookup = Object.freeze({ + repository, + baseBranch: parsed.data.baseBranch, + headBranch: parsed.data.run.branch, + }); + const existing = await findExactPullRequest(input.client, lookup, headSha); + if (existing) return toPublishedPullRequest(existing); + + try { + const created = await input.client.createPullRequest({ + ...lookup, + title: parsed.data.title, + body: parsed.data.body, + }); + assertExactPullRequest(created, lookup, headSha); + return toPublishedPullRequest(created); + } catch (creationError) { + try { + const recovered = await findExactPullRequest(input.client, lookup, headSha); + if (recovered) return toPublishedPullRequest(recovered); + } catch (lookupError) { + if ( + lookupError instanceof GitHubPublicationError && + lookupError.code === "github-conflict" + ) { + throw lookupError; + } + } + throw creationError; + } + }, + }); +} + +async function ensureRemoteBranch(input: { + gitTransport: GitPushTransport; + token: string; + workspace: string; + remote: string; + branch: string; + headSha: string; +}): Promise { + const pushInput = { + workspace: input.workspace, + remote: input.remote, + branch: input.branch, + token: input.token, + }; + const remoteSha = await input.gitTransport.readRemoteBranch(pushInput); + if (remoteSha === input.headSha) return; + if (remoteSha !== null) { + throw new GitHubPublicationError( + "remote-conflict", + `Remote branch ${input.branch} does not match this publication commit.`, + ); + } + + try { + await input.gitTransport.pushBranch(pushInput); + } catch (pushError) { + try { + if ((await input.gitTransport.readRemoteBranch(pushInput)) === input.headSha) return; + } catch { + // Preserve the original push failure for the durable caller. + } + throw pushError; + } +} + +async function findExactPullRequest( + client: GitHubPullRequestClient, + lookup: GitHubPullRequestLookup, + headSha: string, +): Promise { + const records = await client.listPullRequests(lookup); + if (records.length === 0) return null; + if (records.length !== 1) { + throw new GitHubPublicationError( + "github-conflict", + "GitHub returned multiple pull requests for the publication branch.", + ); + } + const record = records[0]; + if (!record) return null; + assertExactPullRequest(record, lookup, headSha); + return record; +} + +function assertExactPullRequest( + record: GitHubPullRequestRecord, + lookup: GitHubPullRequestLookup, + headSha: string, +): void { + const fullName = `${lookup.repository.owner}/${lookup.repository.repository}`.toLowerCase(); + if ( + `${record.owner}/${record.repository}`.toLowerCase() !== fullName || + `${record.headOwner}/${record.headRepository}`.toLowerCase() !== fullName || + record.baseBranch !== lookup.baseBranch || + record.headBranch !== lookup.headBranch || + record.headSha !== headSha + ) { + throw new GitHubPublicationError( + "github-conflict", + "GitHub pull request identity does not match this publication.", + ); + } +} + +function toPublishedPullRequest(record: GitHubPullRequestRecord): PublishedPullRequest { + return Object.freeze({ + url: record.url, + number: record.number, + owner: record.owner, + repository: record.repository, + baseBranch: record.baseBranch, + headBranch: record.headBranch, + headSha: record.headSha, + state: record.state, + merged: record.merged, + }); +} + +export { GitHubPublicationError } from "./error.ts"; +export type { GitHubPublicationErrorCode } from "./error.ts"; +export type { + CreateGitHubPublicationOptions, + GitHubPublicationAuthor, + GitHubPublicationService, + PublishedPullRequest, + PublishPullRequestInput, +} from "./types.ts"; diff --git a/lib/github/remote.test.ts b/lib/github/remote.test.ts new file mode 100644 index 0000000..8d9a65c --- /dev/null +++ b/lib/github/remote.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "vitest"; +import { parseGitHubRemote } from "./remote.ts"; + +describe("parseGitHubRemote", () => { + it.each([ + "https://github.com/ferueda/harness.git", + "git@github.com:ferueda/harness.git", + "ssh://git@github.com/ferueda/harness.git", + ])("normalizes supported GitHub remote %s", (remote) => { + expect(parseGitHubRemote(remote)).toEqual({ + owner: "ferueda", + repository: "harness", + httpsRemote: "https://github.com/ferueda/harness.git", + }); + }); + + it.each([ + "https://token@github.com/ferueda/harness.git", + "https://github.com/ferueda/harness/extra.git", + "https://gitlab.com/ferueda/harness.git", + "file:///tmp/harness.git", + "git@example.com:ferueda/harness.git", + ])("rejects unsupported or credential-bearing remote %s", (remote) => { + expect(() => parseGitHubRemote(remote)).toThrow(/credential-free github\.com/); + }); +}); diff --git a/lib/github/remote.ts b/lib/github/remote.ts new file mode 100644 index 0000000..beb9146 --- /dev/null +++ b/lib/github/remote.ts @@ -0,0 +1,64 @@ +import { GitHubPublicationError } from "./error.ts"; +import type { GitHubRepositoryIdentity } from "./types.ts"; + +const SCP_REMOTE = /^git@github\.com:([^/]+)\/([^/]+)$/i; +const REPOSITORY_PART = /^[A-Za-z0-9_.-]+$/; + +export function parseGitHubRemote(remote: string): GitHubRepositoryIdentity { + const trimmed = remote.trim(); + const scp = SCP_REMOTE.exec(trimmed); + if (scp) return repositoryIdentity(scp[1] ?? "", scp[2] ?? ""); + + let url: URL; + try { + url = new URL(trimmed); + } catch { + throw invalidRemote(); + } + + if ( + !["https:", "ssh:"].includes(url.protocol) || + url.hostname.toLowerCase() !== "github.com" || + url.password || + url.search || + url.hash + ) { + throw invalidRemote(); + } + if (url.protocol === "https:" && url.username) throw invalidRemote(); + if (url.protocol === "ssh:" && url.username !== "git") throw invalidRemote(); + + const parts = url.pathname.split("/").filter(Boolean); + if (parts.length !== 2) throw invalidRemote(); + return repositoryIdentity(parts[0] ?? "", parts[1] ?? ""); +} + +function repositoryIdentity(owner: string, repositoryWithSuffix: string): GitHubRepositoryIdentity { + const repository = repositoryWithSuffix.endsWith(".git") + ? repositoryWithSuffix.slice(0, -4) + : repositoryWithSuffix; + if ( + !owner || + !repository || + !REPOSITORY_PART.test(owner) || + !REPOSITORY_PART.test(repository) || + owner === "." || + owner === ".." || + repository === "." || + repository === ".." + ) { + throw invalidRemote(); + } + return Object.freeze({ + owner, + repository, + httpsRemote: `https://github.com/${owner}/${repository}.git`, + }); +} + +function invalidRemote(): GitHubPublicationError { + return new GitHubPublicationError( + "invalid-input", + "GitHub publication requires a credential-free github.com SSH or HTTPS repository remote.", + ); +} diff --git a/lib/github/types.ts b/lib/github/types.ts new file mode 100644 index 0000000..980a905 --- /dev/null +++ b/lib/github/types.ts @@ -0,0 +1,82 @@ +import type { RepositoryChange, RepositoryRun } from "../repository/types.ts"; + +export type GitHubRepositoryIdentity = Readonly<{ + owner: string; + repository: string; + httpsRemote: string; +}>; + +export type PublishedPullRequest = Readonly<{ + url: string; + number: number; + owner: string; + repository: string; + baseBranch: string; + headBranch: string; + headSha: string; + state: "open" | "closed"; + merged: boolean; +}>; + +export type PublishPullRequestInput = Readonly<{ + run: RepositoryRun; + expectedChanges: readonly RepositoryChange[]; + baseBranch: string; + commitMessage: string; + title: string; + body: string; +}>; + +export type GitHubPublicationService = Readonly<{ + publishPullRequest(input: PublishPullRequestInput): Promise; +}>; + +export type GitHubPullRequestRecord = Readonly<{ + url: string; + number: number; + state: "open" | "closed"; + merged: boolean; + owner: string; + repository: string; + baseBranch: string; + headOwner: string; + headRepository: string; + headBranch: string; + headSha: string; +}>; + +export type GitHubPullRequestLookup = Readonly<{ + repository: GitHubRepositoryIdentity; + baseBranch: string; + headBranch: string; +}>; + +export type GitHubPullRequestClient = Readonly<{ + listPullRequests(input: GitHubPullRequestLookup): Promise; + createPullRequest( + input: GitHubPullRequestLookup & Readonly<{ title: string; body: string }>, + ): Promise; +}>; + +export type GitPushInput = Readonly<{ + workspace: string; + remote: string; + branch: string; + token: string; +}>; + +export type GitPushTransport = Readonly<{ + readRemoteBranch(input: GitPushInput): Promise; + pushBranch(input: GitPushInput): Promise; +}>; + +export type GitHubPublicationAuthor = Readonly<{ + name: string; + email: string; +}>; + +export type CreateGitHubPublicationOptions = Readonly<{ + token: string; + author: GitHubPublicationAuthor; + fetch?: typeof globalThis.fetch; +}>; diff --git a/test/import-boundaries.test.ts b/test/import-boundaries.test.ts index f5c3fcf..61137c5 100644 --- a/test/import-boundaries.test.ts +++ b/test/import-boundaries.test.ts @@ -169,6 +169,24 @@ describe("automation import boundaries", () => { ); }); + it("keeps GitHub publication independent of tracker and domain policy", () => { + expect.hasAssertions(); + expectAllowed( + "lib/github/allowed.ts", + 'import type { RepositoryRun } from "../repository/types.ts";\ntype Run = RepositoryRun;\nexport type { Run };', + ); + expectBoundaryViolation( + "lib/github/forbidden.ts", + 'import type { LinearService } from "../linear/client.ts";', + "not tracker, delivery, provider, or domain policy", + ); + expectBoundaryViolation( + "lib/github/forbidden.ts", + 'import { specIssue } from "../spec/spec.ts";', + "not tracker, delivery, provider, or domain policy", + ); + }); + it("keeps provider adapters independent of tracker and domain operations", () => { expect.hasAssertions(); expectAllowed( From 68653a6dd23b3bfc53e5c56b439c301d05e4678a Mon Sep 17 00:00:00 2001 From: ferueda Date: Thu, 23 Jul 2026 14:33:11 -0700 Subject: [PATCH 2/4] docs: define GitHub publication boundary --- docs/contributing/architecture.md | 33 ++++++++++++++++++------ docs/contributing/harness-engineering.md | 10 ++++--- docs/contributing/testing.md | 6 +++++ docs/project-intent.md | 7 ++--- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/docs/contributing/architecture.md b/docs/contributing/architecture.md index d2bc37c..a71f319 100644 --- a/docs/contributing/architecture.md +++ b/docs/contributing/architecture.md @@ -67,14 +67,15 @@ but remain disabled until they have their own consumers. Build a new automation capability from the smallest set of these parts that its first real flow needs: -| Part | Owns | Must not own | -| ---------------------------- | ------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | -| Service primitive | One external system's auth, SDK quirks, pagination, errors, and plain JSON-safe data | Prompts, providers, Inngest, or domain workflow policy | -| Domain operation | Prompt or decision policy, normalized input, strict result validation, and provenance | Tracker lifecycle, delivery retries, or concrete provider wiring | -| Repository/compute primitive | Isolated execution, resumable workspace handles, change inspection, publication, cleanup | Linear, Inngest, Spec, implementation, or other domain policy | -| Projection adapter | Guarded mapping from one domain result to external-system writes | Prompt policy, delivery scheduling, or a shared lifecycle engine | -| Execution consumer | Triggering, fresh reads, claims, durable steps, retries, ordering, coordination, and traces | SDK pagination, prompt rendering, Git commands, or domain choices | -| Event contract | Minimal typed identifiers and stable work identity | A cached replacement for current external truth | +| Part | Owns | Must not own | +| ---------------------------- | ------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| Service primitive | One external system's auth, SDK quirks, pagination, errors, and plain JSON-safe data | Prompts, providers, Inngest, or domain workflow policy | +| Domain operation | Prompt or decision policy, normalized input, strict result validation, and provenance | Tracker lifecycle, delivery retries, or concrete provider wiring | +| Repository/compute primitive | Isolated execution, resumable workspace handles, change inspection, and cleanup | Publication, Linear, Inngest, Spec, implementation, or domain policy | +| Publication primitive | Approved changes, authenticated commit/push, external artifact identity, and retry recovery | Workspace creation, cleanup, tracker lifecycle, or domain policy | +| Projection adapter | Guarded mapping from one domain result to external-system writes | Prompt policy, delivery scheduling, or a shared lifecycle engine | +| Execution consumer | Triggering, fresh reads, claims, durable steps, retries, ordering, coordination, and traces | SDK pagination, prompt rendering, Git commands, or domain choices | +| Event contract | Minimal typed identifiers and stable work identity | A cached replacement for current external truth | Dependencies point inward from the execution consumer: @@ -83,6 +84,7 @@ execution consumer -> domain operation -> provider interface -> projection adapter -> service primitive -> repository/compute primitive + -> publication primitive ``` Concrete provider adapters, delivery hosts, and external SDK objects stay at @@ -139,6 +141,7 @@ real consumers expose the same stable contract. | `lib/triage/` | Triage prompt, structured decision schema, and provider-independent operation | | `lib/spec/` | Spec prompt, structured result schema, issue-key artifact validation, and provider-independent operation | | `lib/repository/` | Grove-backed writable repository leases, safe setup, change inspection, and reset cleanup | +| `lib/github/` | GitHub remote parsing, credential-safe commit and push, exact PR publication, and retry recovery | | `lib/linear-automation/` | Linear readiness policy, application event contracts, Inngest functions, worker config, and process hosting | | `lib/skills/` | Packaged-skill installation support | | `skills/` | Packaged skills installed into target repositories | @@ -200,6 +203,20 @@ removes tracked and ordinary untracked work while retaining ignored dependency caches such as `node_modules`. The module does not commit, push, open pull requests, update Linear, or choose Spec and implementation policy. +### GitHub publication boundary + +The GitHub module accepts a validated `RepositoryRun` and an approved path/status +set. It creates or recognizes one marked commit, pushes one explicit branch +without force, and finds or creates one exact pull request. Recovery observes +the local commit, remote branch SHA, and GitHub PR identity; it does not add a +publication database or retry state machine. + +The token stays inside the service and is exposed only to primitive-owned +authenticated Git and REST calls after local run validation. GitHub publication +does not create or clean workspaces, validate Spec content, update Linear, or +depend on Inngest. The durable consumer owns retries and calls repository +cleanup only after publication succeeds. + ### Delivery boundary Inngest functions reload external truth before deciding or projecting. Event diff --git a/docs/contributing/harness-engineering.md b/docs/contributing/harness-engineering.md index 67dfcbc..976b07b 100644 --- a/docs/contributing/harness-engineering.md +++ b/docs/contributing/harness-engineering.md @@ -61,9 +61,11 @@ Start with one vertical slice and only the primitives it calls. In order: 1. Normalize the external input through a standalone service primitive. 2. Define one operation-specific input, result, policy, and provenance contract. 3. Add an isolated repository or compute primitive only if the operation must - write, execute, or publish an artifact. -4. Keep tracker projection separate from the operation. -5. Compose the flow with a short durable consumer whose steps match meaningful + write or execute an artifact. +4. Add a separate publication primitive only when approved workspace changes + must become a commit, branch, pull request, or another external artifact. +5. Keep tracker projection separate from the operation. +6. Compose the flow with a short durable consumer whose steps match meaningful external side effects. Before handoff, check the dependency direction: @@ -72,6 +74,8 @@ Before handoff, check the dependency direction: - domain operations depend on the provider interface, not a concrete adapter, and do not import Linear, Inngest, Git, or GitHub code; - repository primitives do not import domain or tracker policy; +- publication primitives do not create workspaces or import domain, tracker, or + delivery policy; - projection code does not render prompts or schedule work; - Inngest consumers reload current truth and coordinate the other parts rather than implementing them inline; diff --git a/docs/contributing/testing.md b/docs/contributing/testing.md index 9e73052..9521e5d 100644 --- a/docs/contributing/testing.md +++ b/docs/contributing/testing.md @@ -109,6 +109,12 @@ These smokes clean their disposable state on success. Live protocols require explicit authority, credentials, stop conditions, disposable targets, and cleanup; they are not routine CI coverage. +GitHub publication uses temporary repositories, a local bare remote, and an +injected HTTP transport in Vitest. This proves commit, push, response-loss, and +PR idempotency without live GitHub access. A live push/PR smoke is reserved for +explicitly authorized protocol verification and must clean its disposable +branch and pull request. + ## Verification Commands During iteration, run the narrowest relevant path, for example: diff --git a/docs/project-intent.md b/docs/project-intent.md index a4f37c8..cf144b9 100644 --- a/docs/project-intent.md +++ b/docs/project-intent.md @@ -44,9 +44,10 @@ such as `/path/to/repo`, `harness.json`, and `.harness/runs/reviews//`. workflow policy. - Durable delivery systems may retry, schedule, and observe work, but domain policy belongs in the independent operation that makes the decision. -- Repository and compute primitives own isolated execution, publication, and - cleanup. They return serializable handles and must not own tracker lifecycle - or domain policy. +- Repository and compute primitives own isolated execution and cleanup. + Publication primitives own authenticated materialization into external + source-control systems. Both return serializable handles and must not own + tracker lifecycle or domain policy. - External systems that already own queue or lifecycle state remain the source of truth. Harness must not mirror that state in a second lifecycle store. - Runtime schemas and exported schemas must stay aligned when either side From 3b425d77b7c8af3851a5724dc67f4c85dea1a6e9 Mon Sep 17 00:00:00 2001 From: ferueda Date: Thu, 23 Jul 2026 14:45:25 -0700 Subject: [PATCH 3/4] fix: isolate authenticated Git publication --- lib/github/client.test.ts | 19 ++++++ lib/github/client.ts | 10 ++- lib/github/git.test.ts | 85 +++++++++++++++++++++++++- lib/github/git.ts | 108 ++++++++++++++++++++++++++++----- lib/github/publication.test.ts | 1 + lib/github/publication.ts | 1 + lib/github/types.ts | 6 +- 7 files changed, 210 insertions(+), 20 deletions(-) diff --git a/lib/github/client.test.ts b/lib/github/client.test.ts index 73c3a39..928b6fd 100644 --- a/lib/github/client.test.ts +++ b/lib/github/client.test.ts @@ -88,6 +88,25 @@ describe("GitHub pull request client", () => { expect(String(error)).not.toContain(encoded); expect(String(error)).toContain("[REDACTED]"); }); + + it("normalizes and redacts response body failures", async () => { + const client = createGitHubPullRequestClient({ + token: TOKEN, + fetch: async () => + new Response( + new ReadableStream({ + pull(controller) { + controller.error(new Error(`stream failed ${TOKEN}`)); + }, + }), + ), + }); + + const error = await client.listPullRequests(LOOKUP).catch((caught: unknown) => caught); + expect(error).toMatchObject({ code: "github-failed" }); + expect(String(error)).not.toContain(TOKEN); + expect(String(error)).toContain("[REDACTED]"); + }); }); function pullRequestResponse( diff --git a/lib/github/client.ts b/lib/github/client.ts index cbcc315..b4c2855 100644 --- a/lib/github/client.ts +++ b/lib/github/client.ts @@ -123,7 +123,15 @@ async function requestJson(input: { ); } - const responseBody = await response.text(); + let responseBody: string; + try { + responseBody = await response.text(); + } catch (error) { + throw new GitHubPublicationError( + "github-failed", + `GitHub response body failed: ${redactSecrets(errorMessage(error), [input.token])}`, + ); + } if (!response.ok) { const diagnostic = redactSecrets(responseBody.slice(-2_000), [input.token]).trim(); throw new GitHubPublicationError( diff --git a/lib/github/git.test.ts b/lib/github/git.test.ts index 70aa293..b0ab2ae 100644 --- a/lib/github/git.test.ts +++ b/lib/github/git.test.ts @@ -1,5 +1,8 @@ +import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; -import { readFile, stat } from "node:fs/promises"; +import { chmod, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { createAuthenticatedGitTransport, type AuthenticatedGitExecutor } from "./git.ts"; @@ -8,6 +11,7 @@ describe("authenticated Git transport", () => { const token = "github-secret+/="; let captured: | Readonly<{ + cwd: string; args: readonly string[]; environment: Readonly>; helper: string; @@ -17,6 +21,7 @@ describe("authenticated Git transport", () => { const executor: AuthenticatedGitExecutor = async (input) => { const helperPath = input.environment.GIT_ASKPASS ?? ""; captured = { + cwd: input.cwd, args: input.args, environment: input.environment, helper: await readFile(helperPath, "utf8"), @@ -37,11 +42,14 @@ describe("authenticated Git transport", () => { workspace: process.cwd(), remote: "https://github.com/ferueda/harness.git", branch: "codex/FER-286", + commitSha: "a".repeat(40), token, }); expect(captured).toBeDefined(); + expect(captured?.cwd).not.toBe(process.cwd()); expect(captured?.args.join(" ")).not.toContain(token); + expect(captured?.args.join(" ")).toContain(`${"a".repeat(40)}:refs/heads/codex/FER-286`); expect(captured?.args.join(" ")).toContain("credential.helper="); expect(captured?.args.join(" ")).toContain("core.hooksPath="); expect(captured?.helper).not.toContain(token); @@ -68,6 +76,7 @@ describe("authenticated Git transport", () => { workspace: process.cwd(), remote: "https://github.com/ferueda/harness.git", branch: "codex/FER-286", + commitSha: "a".repeat(40), token, }) .catch((caught: unknown) => caught); @@ -76,4 +85,78 @@ describe("authenticated Git transport", () => { expect(String(error)).not.toContain(encoded); expect(String(error)).toContain("[REDACTED]"); }); + + it("does not load hostile configuration from the worktree", async () => { + const root = await mkdtemp(join(tmpdir(), "harness-github-hostile-config-")); + try { + const workspace = join(root, "workspace"); + const remote = join(root, "remote.git"); + const marker = join(root, "leaked-token"); + git(root, ["init", "--bare", remote]); + git(root, ["clone", remote, workspace]); + git(workspace, ["config", `url.leak::${remote}.insteadOf`, remote]); + await writeFile( + join(root, "git-remote-leak"), + `#!/bin/sh\nprintf '%s' "$HARNESS_GITHUB_TOKEN" > "${marker}"\nexit 1\n`, + { encoding: "utf8", mode: 0o700 }, + ); + await chmod(join(root, "git-remote-leak"), 0o700); + + const transport = createAuthenticatedGitTransport({ + environment: { + ...process.env, + PATH: `${root}:${process.env.PATH ?? ""}`, + }, + }); + await expect( + transport.readRemoteBranch({ + workspace, + remote, + branch: "codex/FER-286", + token: "must-not-leak", + }), + ).resolves.toBeNull(); + expect(existsSync(marker)).toBe(false); + } finally { + await rm(root, { force: true, recursive: true }); + } + }); + + it("pushes the exact commit through isolated object access", async () => { + const root = await mkdtemp(join(tmpdir(), "harness-github-isolated-push-")); + try { + const workspace = join(root, "workspace"); + const remote = join(root, "remote.git"); + git(root, ["init", "--bare", remote]); + git(root, ["clone", remote, workspace]); + git(workspace, ["config", "user.name", "Fixture"]); + git(workspace, ["config", "user.email", "fixture@example.com"]); + await writeFile(join(workspace, "README.md"), "# Fixture\n", "utf8"); + git(workspace, ["add", "README.md"]); + git(workspace, ["commit", "-m", "Initialize fixture"]); + const commitSha = git(workspace, ["rev-parse", "HEAD"]); + + const transport = createAuthenticatedGitTransport(); + await transport.pushBranch({ + workspace, + remote, + branch: "codex/FER-286", + commitSha, + token: "unused-for-local-remote", + }); + + expect(git(remote, ["rev-parse", "refs/heads/codex/FER-286"])).toBe(commitSha); + } finally { + await rm(root, { force: true, recursive: true }); + } + }); }); + +function git(cwd: string, args: readonly string[]): string { + return execFileSync("git", [...args], { + cwd, + encoding: "utf8", + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + stdio: ["ignore", "pipe", "pipe"], + }).trim(); +} diff --git a/lib/github/git.ts b/lib/github/git.ts index f389b2e..b28558a 100644 --- a/lib/github/git.ts +++ b/lib/github/git.ts @@ -1,10 +1,15 @@ import { execFile } from "node:child_process"; -import { chmod, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { isAbsolute, join, resolve } from "node:path"; import { promisify } from "node:util"; import { errorMessage, GitHubPublicationError, redactSecrets } from "./error.ts"; -import type { GitHubPublicationAuthor, GitPushInput, GitPushTransport } from "./types.ts"; +import type { + GitHubPublicationAuthor, + GitPushInput, + GitPushTransport, + GitRemoteBranchInput, +} from "./types.ts"; import { inspectGitChanges } from "../repository/git.ts"; import type { RepositoryChange, @@ -45,7 +50,7 @@ esac `; export type AuthenticatedGitExecutor = (input: { - workspace: string; + cwd: string; args: readonly string[]; environment: Readonly>; }) => Promise; @@ -60,7 +65,7 @@ export function createAuthenticatedGitTransport( const environment = options.environment ?? process.env; return Object.freeze({ - async readRemoteBranch(input: GitPushInput): Promise { + async readRemoteBranch(input: GitRemoteBranchInput): Promise { const output = await runAuthenticatedGit({ ...input, executor, @@ -75,7 +80,14 @@ export function createAuthenticatedGitTransport( ...input, executor, environment, - args: ["push", "--porcelain", "--", input.remote, `HEAD:refs/heads/${input.branch}`], + args: [ + "push", + "--porcelain", + "--", + input.remote, + `${input.commitSha}:refs/heads/${input.branch}`, + ], + includeWorkspaceObjects: true, }); }, }); @@ -436,21 +448,46 @@ function parseRemoteHead(output: string, branch: string): string | null { } async function runAuthenticatedGit( - input: GitPushInput & { + input: GitRemoteBranchInput & { args: readonly string[]; executor: AuthenticatedGitExecutor; environment: NodeJS.ProcessEnv; + commitSha?: string; + includeWorkspaceObjects?: boolean; }, ): Promise { const helperDirectory = await mkdtemp(join(tmpdir(), "harness-github-askpass-")); const helperPath = join(helperDirectory, "askpass.sh"); + const gitDirectory = join(helperDirectory, "repository.git"); try { + if ( + input.includeWorkspaceObjects && + (!input.commitSha || !FULL_GIT_SHA.test(input.commitSha)) + ) { + throw new GitHubPublicationError( + "invalid-input", + "Authenticated Git requires an exact commit SHA.", + ); + } + await initializeIsolatedGitRepository(helperDirectory, gitDirectory, input.environment); + if (input.includeWorkspaceObjects) { + const objectDirectory = await readObjectDirectory(input.workspace); + if (containsCommandControl(objectDirectory)) { + throw new GitHubPublicationError( + "invalid-input", + "Repository object directory contains unsupported control characters.", + ); + } + const alternateFile = join(gitDirectory, "objects", "info", "alternates"); + await mkdir(join(gitDirectory, "objects", "info"), { recursive: true }); + await writeFile(alternateFile, `${objectDirectory}\n`, { encoding: "utf8", flag: "wx" }); + } await writeFile(helperPath, ASKPASS_SOURCE, { encoding: "utf8", flag: "wx", mode: 0o700 }); await chmod(helperPath, 0o700); const environment = authenticatedGitEnvironment(input.environment, helperPath, input.token); return await input.executor({ - workspace: input.workspace, - args: [...GIT_CONFIG_ARGS, ...input.args], + cwd: gitDirectory, + args: [...GIT_CONFIG_ARGS, "--git-dir=.", ...input.args], environment, }); } catch (error) { @@ -463,18 +500,43 @@ async function runAuthenticatedGit( } } -function authenticatedGitEnvironment( +async function initializeIsolatedGitRepository( + cwd: string, + gitDirectory: string, + source: NodeJS.ProcessEnv, +): Promise { + await execFileAsync( + "git", + [...GIT_CONFIG_ARGS, "init", "--bare", "--quiet", "--", gitDirectory], + { + cwd, + encoding: "utf8", + env: { ...unauthenticatedGitEnvironment(source) }, + maxBuffer: 8 * 1024 * 1024, + }, + ); +} + +async function readObjectDirectory(workspace: string): Promise { + const commonDirectory = ( + await runLocalGit(workspace, ["rev-parse", "--path-format=absolute", "--git-common-dir"]) + ).trim(); + if (!isAbsolute(commonDirectory)) { + throw new GitHubPublicationError( + "git-failed", + "Git returned a non-absolute common repository directory.", + ); + } + return realpath(join(commonDirectory, "objects")); +} + +function unauthenticatedGitEnvironment( source: NodeJS.ProcessEnv, - helperPath: string, - token: string, ): Readonly> { const environment: Record = { - GIT_ASKPASS: helperPath, - GIT_ASKPASS_REQUIRE: "force", GIT_CONFIG_GLOBAL: NULL_DEVICE, GIT_CONFIG_NOSYSTEM: "1", GIT_TERMINAL_PROMPT: "0", - HARNESS_GITHUB_TOKEN: token, }; for (const key of AUTH_ENVIRONMENT_KEYS) { const value = source[key]; @@ -483,13 +545,27 @@ function authenticatedGitEnvironment( return Object.freeze(environment); } +function authenticatedGitEnvironment( + source: NodeJS.ProcessEnv, + helperPath: string, + token: string, +): Readonly> { + const environment: Record = { + ...unauthenticatedGitEnvironment(source), + GIT_ASKPASS: helperPath, + GIT_ASKPASS_REQUIRE: "force", + HARNESS_GITHUB_TOKEN: token, + }; + return Object.freeze(environment); +} + async function executeGit(input: { - workspace: string; + cwd: string; args: readonly string[]; environment: Readonly>; }): Promise { const { stdout } = await execFileAsync("git", [...input.args], { - cwd: input.workspace, + cwd: input.cwd, encoding: "utf8", env: { ...input.environment }, maxBuffer: 8 * 1024 * 1024, diff --git a/lib/github/publication.test.ts b/lib/github/publication.test.ts index 4ab6ef8..281ef48 100644 --- a/lib/github/publication.test.ts +++ b/lib/github/publication.test.ts @@ -369,6 +369,7 @@ function pushInput(fixture: Fixture) { workspace: fixture.workspace, remote: "https://github.com/ferueda/harness.git", branch: BRANCH, + commitSha: git(fixture.workspace, ["rev-parse", "HEAD"]), token: TOKEN, }; } diff --git a/lib/github/publication.ts b/lib/github/publication.ts index bb3c86e..6ca7535 100644 --- a/lib/github/publication.ts +++ b/lib/github/publication.ts @@ -173,6 +173,7 @@ async function ensureRemoteBranch(input: { workspace: input.workspace, remote: input.remote, branch: input.branch, + commitSha: input.headSha, token: input.token, }; const remoteSha = await input.gitTransport.readRemoteBranch(pushInput); diff --git a/lib/github/types.ts b/lib/github/types.ts index 980a905..1bfae8d 100644 --- a/lib/github/types.ts +++ b/lib/github/types.ts @@ -58,15 +58,17 @@ export type GitHubPullRequestClient = Readonly<{ ): Promise; }>; -export type GitPushInput = Readonly<{ +export type GitRemoteBranchInput = Readonly<{ workspace: string; remote: string; branch: string; token: string; }>; +export type GitPushInput = GitRemoteBranchInput & Readonly<{ commitSha: string }>; + export type GitPushTransport = Readonly<{ - readRemoteBranch(input: GitPushInput): Promise; + readRemoteBranch(input: GitRemoteBranchInput): Promise; pushBranch(input: GitPushInput): Promise; }>; From 0b4879e9ac544af30061d0adb1c455389b4151f2 Mon Sep 17 00:00:00 2001 From: ferueda Date: Thu, 23 Jul 2026 15:03:32 -0700 Subject: [PATCH 4/4] fix: use current GitHub API version --- lib/github/client.test.ts | 4 +++- lib/github/client.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/github/client.test.ts b/lib/github/client.test.ts index 928b6fd..7b0a518 100644 --- a/lib/github/client.test.ts +++ b/lib/github/client.test.ts @@ -39,7 +39,9 @@ describe("GitHub pull request client", () => { expect(String(url)).toContain("state=all"); expect(String(url)).toContain("head=ferueda%3Acodex%2FFER-286"); expect(String(url)).toContain("base=main"); - expect(new Headers(options?.headers).get("authorization")).toBe(`Bearer ${TOKEN}`); + const headers = new Headers(options?.headers); + expect(headers.get("authorization")).toBe(`Bearer ${TOKEN}`); + expect(headers.get("x-github-api-version")).toBe("2026-03-10"); }); it("sends the minimal create payload", async () => { diff --git a/lib/github/client.ts b/lib/github/client.ts index b4c2855..737b279 100644 --- a/lib/github/client.ts +++ b/lib/github/client.ts @@ -112,7 +112,7 @@ async function requestJson(input: { Authorization: `Bearer ${input.token}`, "Content-Type": "application/json", "User-Agent": "harness", - "X-GitHub-Api-Version": "2022-11-28", + "X-GitHub-Api-Version": "2026-03-10", }, ...(input.body ? { body: JSON.stringify(input.body) } : {}), });