Skip to content

Commit 4eb9a55

Browse files
committed
fix(open-pr): anchor PR branches to patch base
1 parent 3a66c1c commit 4eb9a55

5 files changed

Lines changed: 143 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
- Hardened review ingestion so provider findings must cite included files with valid line ranges and matching evidence quotes.
99
- Fixed `clawpatch open-pr` so repositories without default-branch metadata use a dedicated patch branch and let GitHub choose the PR base.
1010
- Fixed `clawpatch open-pr` retries to push the recorded patch commit instead of any later local branch tip.
11+
- Fixed first-time `clawpatch open-pr` branch creation to start from the recorded patch base.
12+
- Fixed command execution so providers that exit before reading stdin do not surface benign `EPIPE` errors.
1113
- Fixed `clawpatch ci --since` empty-review output so it reports `reviewed: 0`.
1214
- Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi.
1315
- Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911.

src/app.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,12 +1119,18 @@ export async function openPrCommand(
11191119
let commitSha = patch.git.commitSha;
11201120
const hadRecordedCommit = commitSha !== null;
11211121
if (commitSha === null) {
1122+
const patchBaseSha = assertDefined(patch.git.baseSha, "missing patch base");
1123+
const targetBranchExists = await localBranchExists(git.root, branch);
1124+
if (targetBranchExists) {
1125+
await assertRefAtPatchBase(git.root, branch, patch);
1126+
}
11221127
if (git.currentBranch !== branch) {
1123-
const switchArgs = (await localBranchExists(git.root, branch))
1128+
const switchArgs = targetBranchExists
11241129
? ["switch", branch]
1125-
: ["switch", "-c", branch];
1130+
: ["switch", "-c", branch, patchBaseSha];
11261131
await checkedRun("git switch", runCommandArgs("git", switchArgs, git.root));
11271132
}
1133+
await assertRefAtPatchBase(git.root, "HEAD", patch);
11281134
const stagePlan = await patchStagePlan(git.root, patchWorktree);
11291135
if (stagePlan.addFiles.length > 0) {
11301136
await checkedRun(
@@ -1528,11 +1534,14 @@ function plannedPrCommands(
15281534
): string[] {
15291535
const commands: string[] = [];
15301536
if (patch.git.commitSha === null) {
1537+
const patchBaseSha = assertDefined(patch.git.baseSha, "missing patch base");
15311538
const commitFiles = stagePlan?.commitFiles ?? gitFiles;
15321539
const addFiles = stagePlan?.addFiles ?? gitFiles;
15331540
const updateFiles = stagePlan?.updateFiles ?? [];
15341541
commands.push(
1535-
branchExists ? `git switch ${shellArg(branch)}` : `git switch -c ${shellArg(branch)}`,
1542+
branchExists
1543+
? `git switch ${shellArg(branch)}`
1544+
: `git switch -c ${shellArg(branch)} ${shellArg(patchBaseSha)}`,
15361545
);
15371546
if (addFiles.length > 0) {
15381547
commands.push(`git add -- ${shellPathspecArgs(addFiles)}`);
@@ -1735,6 +1744,25 @@ async function localBranchExists(gitRoot: string, branch: string): Promise<boole
17351744
return result.exitCode === 0;
17361745
}
17371746

1747+
async function assertRefAtPatchBase(
1748+
gitRoot: string,
1749+
ref: string,
1750+
patch: PatchAttempt,
1751+
): Promise<void> {
1752+
const head = await checkedRun(
1753+
"git rev-parse",
1754+
runCommandArgs("git", ["rev-parse", ref], gitRoot),
1755+
);
1756+
const sha = head.stdout.trim();
1757+
if (sha !== patch.git.baseSha) {
1758+
const message = [
1759+
`patch attempt ${patch.patchAttemptId} was recorded from ${patch.git.baseSha},`,
1760+
`but ${ref} is ${sha}`,
1761+
].join(" ");
1762+
throw new ClawpatchError(message, 2, "invalid-input");
1763+
}
1764+
}
1765+
17381766
function firstUrl(output: string): string | null {
17391767
return /https?:\/\/\S+/u.exec(output)?.[0] ?? null;
17401768
}

src/exec.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ describe("runCommandArgs", () => {
3131
expect(result.stderr).toContain("clawpatch-missing-executable-for-test");
3232
});
3333

34+
it("does not surface EPIPE when a child exits before reading stdin", async () => {
35+
const dir = await mkdtemp(join(tmpdir(), "clawpatch-exec-stdin-"));
36+
const input = "x".repeat(1_000_000);
37+
const result = await runCommandArgs(process.execPath, ["-e", "process.exit(0)"], dir, input);
38+
39+
expect(result.exitCode).toBe(0);
40+
});
41+
3442
it("terminates commands that exceed a timeout", async () => {
3543
const dir = await mkdtemp(join(tmpdir(), "clawpatch-exec-timeout-"));
3644
const script = join(dir, "hang.mjs");

src/exec.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,7 @@ export async function runCommandRaw(
4646
});
4747
child.on("close", resolve);
4848
});
49-
if (input !== undefined) {
50-
child.stdin.end(input);
51-
} else {
52-
child.stdin.end();
53-
}
49+
endChildStdin(child, input);
5450
const exitCode = await exitCodePromise;
5551
if (spawnErrorMessage !== null) {
5652
stderr += stderr.length === 0 ? spawnErrorMessage : `\n${spawnErrorMessage}`;
@@ -137,11 +133,7 @@ export async function runCommandArgs(
137133
});
138134
}, options.timeoutMs);
139135
}
140-
if (input !== undefined) {
141-
child.stdin.end(input);
142-
} else {
143-
child.stdin.end();
144-
}
136+
endChildStdin(child, input);
145137
const exitCode = await exitCodePromise;
146138
if (spawnErrorMessage !== null) {
147139
stderr += stderr.length === 0 ? spawnErrorMessage : `\n${spawnErrorMessage}`;
@@ -229,6 +221,19 @@ function signalExitCode(signal: NodeJS.Signals): number {
229221

230222
function noop(): void {}
231223

224+
function endChildStdin(child: ReturnType<typeof spawn>, input: string | undefined): void {
225+
const stdin = child.stdin;
226+
if (stdin === null) {
227+
return;
228+
}
229+
stdin.on("error", noop);
230+
if (input !== undefined) {
231+
stdin.end(input);
232+
} else {
233+
stdin.end();
234+
}
235+
}
236+
232237
function commandSpawnSpec(
233238
program: string,
234239
args: string[],

src/workflow.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3757,6 +3757,93 @@ describe("workflow", () => {
37573757
}
37583758
});
37593759

3760+
it("creates first PR branches from the recorded patch base", async () => {
3761+
const root = await fixtureRoot("clawpatch-open-pr-recorded-base-");
3762+
await writeFixture(
3763+
root,
3764+
"package.json",
3765+
JSON.stringify({ name: "open-pr-recorded-base", bin: { open: "src/index.ts" } }),
3766+
);
3767+
await writeFixture(root, "src/index.ts", "export const value = 'TODO_BUG';\n");
3768+
await initGit(root);
3769+
await checkCommand(root, "git add package.json src/index.ts");
3770+
await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "base"');
3771+
const baseSha = (await runCommand("git rev-parse HEAD", root)).stdout.trim();
3772+
const origin = await fixtureRoot("clawpatch-open-pr-recorded-base-origin-");
3773+
await checkCommand(root, `git init --bare -q ${origin}`);
3774+
await checkCommand(root, `git remote add origin ${origin}`);
3775+
const context = await makeContext(testOptions(root));
3776+
const paths = statePaths(join(root, ".clawpatch"));
3777+
await initCommand(context, {});
3778+
await writeFixture(root, "src/index.ts", "export const value = 'fixed';\n");
3779+
await writeFixture(root, "src/unrelated.ts", "export const unrelated = true;\n");
3780+
await checkCommand(root, "git add src/unrelated.ts");
3781+
await checkCommand(root, 'git -c commit.gpgsign=false commit -q -m "unrelated"');
3782+
const advancedHead = (await runCommand("git rev-parse HEAD", root)).stdout.trim();
3783+
expect(advancedHead).not.toBe(baseSha);
3784+
const now = new Date().toISOString();
3785+
await writePatchAttempt(paths, {
3786+
schemaVersion: 1,
3787+
patchAttemptId: "pat_open_pr_recorded_base",
3788+
findingIds: [],
3789+
featureIds: [],
3790+
status: "applied",
3791+
plan: "Replace the marker value.",
3792+
filesChanged: ["src/index.ts"],
3793+
commandsRun: [],
3794+
testResults: [
3795+
{
3796+
command: "pnpm test",
3797+
cwd: root,
3798+
exitCode: 0,
3799+
durationMs: 1,
3800+
stdout: "",
3801+
stderr: "",
3802+
},
3803+
],
3804+
provider: null,
3805+
git: {
3806+
baseSha,
3807+
commitSha: null,
3808+
branchName: null,
3809+
prUrl: null,
3810+
},
3811+
createdAt: now,
3812+
updatedAt: now,
3813+
});
3814+
const ghScripts = await fixtureRoot("clawpatch-open-pr-recorded-base-gh-");
3815+
const successGh = join(ghScripts, "success-gh.sh");
3816+
await writeFixture(
3817+
ghScripts,
3818+
"success-gh.sh",
3819+
"#!/bin/sh\necho https://github.com/openclaw/clawpatch/pull/1005\n",
3820+
);
3821+
await chmod(successGh, 0o755);
3822+
const previousGh = process.env["CLAWPATCH_GH"];
3823+
try {
3824+
process.env["CLAWPATCH_GH"] = successGh;
3825+
const opened = (await openPrCommand(context, {
3826+
patch: "pat_open_pr_recorded_base",
3827+
base: "main",
3828+
branch: "clawpatch/pat_open_pr_recorded_base",
3829+
})) as { commit: string; pr: string };
3830+
const parent = (
3831+
await runCommand(`git show -s --format=%P ${opened.commit}`, root)
3832+
).stdout.trim();
3833+
const committed = await runCommand(`git show --name-only --format= ${opened.commit}`, root);
3834+
3835+
expect(opened.pr).toBe("https://github.com/openclaw/clawpatch/pull/1005");
3836+
expect(parent).toBe(baseSha);
3837+
expect(committed.stdout.trim().split("\n")).toEqual(["src/index.ts"]);
3838+
} finally {
3839+
if (previousGh === undefined) {
3840+
delete process.env["CLAWPATCH_GH"];
3841+
} else {
3842+
process.env["CLAWPATCH_GH"] = previousGh;
3843+
}
3844+
}
3845+
});
3846+
37603847
it("switches to an existing patch branch when opening a PR", async () => {
37613848
const root = await fixtureRoot("clawpatch-open-pr-existing-branch-");
37623849
await writeFixture(

0 commit comments

Comments
 (0)