Three bugs prevented auto_pr from ever creating a PR: 1. auto_pr was gated on `pushed` flag which requires auto_push to also be true. Changed condition to `!nothingToCommit` so auto_pr works independently. 2. phases.ts called createDraftPR AFTER mergeAndExit (when we're back on main and the milestone branch may not exist on remote). Removed duplicate PR creation from phases.ts — it's already handled inside mergeMilestoneToMain. 3. createDraftPR in git-service.ts lacked --head and --base parameters, so gh would create a PR from whatever branch was current. Added optional opts parameter with head/base support. Fixes #2302 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e39dc7976c
commit
2ddb790141
4 changed files with 101 additions and 57 deletions
|
|
@ -1320,9 +1320,9 @@ export function mergeMilestoneToMain(
|
|||
}
|
||||
}
|
||||
|
||||
// 9b. Auto-create PR if enabled (requires push_branches + push succeeded)
|
||||
// 9b. Auto-create PR if enabled (#2302: no longer gated on pushed/auto_push)
|
||||
let prCreated = false;
|
||||
if (prefs.auto_pr === true && pushed) {
|
||||
if (prefs.auto_pr === true && !nothingToCommit) {
|
||||
const remote = prefs.remote ?? "origin";
|
||||
const prTarget = prefs.pr_target_branch ?? mainBranch;
|
||||
try {
|
||||
|
|
@ -1332,9 +1332,9 @@ export function mergeMilestoneToMain(
|
|||
stdio: ["ignore", "pipe", "pipe"],
|
||||
encoding: "utf-8",
|
||||
});
|
||||
// Create PR via gh CLI
|
||||
// Create PR via gh CLI with explicit --head and --base (#2302)
|
||||
execFileSync("gh", [
|
||||
"pr", "create",
|
||||
"pr", "create", "--draft",
|
||||
"--base", prTarget,
|
||||
"--head", milestoneBranch,
|
||||
"--title", `Milestone ${milestoneId} complete`,
|
||||
|
|
|
|||
|
|
@ -235,23 +235,7 @@ export async function runPreDispatch(
|
|||
// Worktree lifecycle on milestone transition — merge current, enter next
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId!, ctx.ui);
|
||||
|
||||
// Opt-in: create draft PR on milestone completion
|
||||
if (prefs?.git?.auto_pr) {
|
||||
try {
|
||||
const { createDraftPR } = await import("../git-service.js");
|
||||
const prUrl = createDraftPR(
|
||||
s.basePath,
|
||||
s.currentMilestoneId!,
|
||||
`[GSD] ${s.currentMilestoneId} complete`,
|
||||
`Milestone ${s.currentMilestoneId} completed by GSD auto-mode.\n\nSee .gsd/${s.currentMilestoneId}/ for details.`,
|
||||
);
|
||||
if (prUrl) {
|
||||
ctx.ui.notify(`Draft PR created: ${prUrl}`, "info");
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — PR creation is best-effort
|
||||
}
|
||||
}
|
||||
// PR creation (auto_pr) is handled inside mergeMilestoneToMain (#2302)
|
||||
|
||||
deps.invalidateAllCaches();
|
||||
|
||||
|
|
@ -324,23 +308,7 @@ export async function runPreDispatch(
|
|||
if (s.currentMilestoneId) {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId, ctx.ui);
|
||||
|
||||
// Opt-in: create draft PR on milestone completion
|
||||
if (prefs?.git?.auto_pr) {
|
||||
try {
|
||||
const { createDraftPR } = await import("../git-service.js");
|
||||
const prUrl = createDraftPR(
|
||||
s.basePath,
|
||||
s.currentMilestoneId,
|
||||
`[GSD] ${s.currentMilestoneId} complete`,
|
||||
`Milestone ${s.currentMilestoneId} completed by GSD auto-mode.\n\nSee .gsd/${s.currentMilestoneId}/ for details.`,
|
||||
);
|
||||
if (prUrl) {
|
||||
ctx.ui.notify(`Draft PR created: ${prUrl}`, "info");
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — PR creation is best-effort
|
||||
}
|
||||
}
|
||||
// PR creation (auto_pr) is handled inside mergeMilestoneToMain (#2302)
|
||||
}
|
||||
deps.sendDesktopNotification(
|
||||
"GSD",
|
||||
|
|
@ -424,23 +392,7 @@ export async function runPreDispatch(
|
|||
if (s.currentMilestoneId) {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId, ctx.ui);
|
||||
|
||||
// Opt-in: create draft PR on milestone completion
|
||||
if (prefs?.git?.auto_pr) {
|
||||
try {
|
||||
const { createDraftPR } = await import("../git-service.js");
|
||||
const prUrl = createDraftPR(
|
||||
s.basePath,
|
||||
s.currentMilestoneId,
|
||||
`[GSD] ${s.currentMilestoneId} complete`,
|
||||
`Milestone ${s.currentMilestoneId} completed by GSD auto-mode.\n\nSee .gsd/${s.currentMilestoneId}/ for details.`,
|
||||
);
|
||||
if (prUrl) {
|
||||
ctx.ui.notify(`Draft PR created: ${prUrl}`, "info");
|
||||
}
|
||||
} catch {
|
||||
// Non-fatal — PR creation is best-effort
|
||||
}
|
||||
}
|
||||
// PR creation (auto_pr) is handled inside mergeMilestoneToMain (#2302)
|
||||
}
|
||||
deps.sendDesktopNotification(
|
||||
"GSD",
|
||||
|
|
|
|||
|
|
@ -684,13 +684,17 @@ export function createDraftPR(
|
|||
milestoneId: string,
|
||||
title: string,
|
||||
body: string,
|
||||
opts?: { head?: string; base?: string },
|
||||
): string | null {
|
||||
try {
|
||||
const result = execFileSync("gh", [
|
||||
const args = [
|
||||
"pr", "create", "--draft",
|
||||
"--title", title,
|
||||
"--body", body,
|
||||
], { cwd: basePath, encoding: "utf8", timeout: 30000, env: GIT_NO_PROMPT_ENV });
|
||||
];
|
||||
if (opts?.head) args.push("--head", opts.head);
|
||||
if (opts?.base) args.push("--base", opts.base);
|
||||
const result = execFileSync("gh", args, { cwd: basePath, encoding: "utf8", timeout: 30000, env: GIT_NO_PROMPT_ENV });
|
||||
return result.trim();
|
||||
} catch {
|
||||
return null;
|
||||
|
|
|
|||
88
src/resources/extensions/gsd/tests/auto-pr-bugs.test.ts
Normal file
88
src/resources/extensions/gsd/tests/auto-pr-bugs.test.ts
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
/**
|
||||
* auto-pr-bugs.test.ts — Regression tests for #2302.
|
||||
*
|
||||
* Three interacting bugs prevented auto_pr from ever creating a PR:
|
||||
* 1. auto_pr was gated on `pushed` (which requires auto_push)
|
||||
* 2. Milestone branch was not pushed to remote before PR creation
|
||||
* 3. createDraftPR in git-service.ts lacked --head/--base parameters
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
// ─── Bug 1: auto_pr should not depend on auto_push / pushed flag ────────────
|
||||
|
||||
const autoWorktreeSrcPath = join(import.meta.dirname, "..", "auto-worktree.ts");
|
||||
const autoWorktreeSrc = readFileSync(autoWorktreeSrcPath, "utf-8");
|
||||
|
||||
test("#2302 bug 1: auto_pr condition should not require pushed flag", () => {
|
||||
// Find the auto_pr block in mergeMilestoneToMain
|
||||
const autoPrIdx = autoWorktreeSrc.indexOf("auto_pr");
|
||||
assert.ok(autoPrIdx !== -1, "auto_pr reference exists in auto-worktree.ts");
|
||||
|
||||
// Get context around the auto_pr check
|
||||
const lineStart = autoWorktreeSrc.lastIndexOf("\n", autoPrIdx) + 1;
|
||||
const lineEnd = autoWorktreeSrc.indexOf("\n", autoPrIdx);
|
||||
const autoPrLine = autoWorktreeSrc.slice(lineStart, lineEnd);
|
||||
|
||||
// The condition should NOT include `&& pushed`
|
||||
assert.ok(
|
||||
!autoPrLine.includes("&& pushed"),
|
||||
"auto_pr condition should not be gated on pushed flag (auto_push dependency)",
|
||||
);
|
||||
});
|
||||
|
||||
// ─── Bug 2: phases.ts should not duplicate PR creation ──────────────────────
|
||||
|
||||
const phasesSrcPath = join(import.meta.dirname, "..", "auto", "phases.ts");
|
||||
const phasesSrc = readFileSync(phasesSrcPath, "utf-8");
|
||||
|
||||
test("#2302 bug 2: phases.ts should not call createDraftPR (handled by mergeMilestoneToMain)", () => {
|
||||
// After fix, phases.ts should not import or call createDraftPR because
|
||||
// PR creation is handled inside mergeMilestoneToMain in auto-worktree.ts
|
||||
const createDraftPRCalls = phasesSrc.match(/createDraftPR\(/g) || [];
|
||||
|
||||
assert.equal(
|
||||
createDraftPRCalls.length,
|
||||
0,
|
||||
"phases.ts should not call createDraftPR — it's handled by mergeMilestoneToMain",
|
||||
);
|
||||
});
|
||||
|
||||
// ─── Bug 3: createDraftPR should accept head and base branch parameters ─────
|
||||
|
||||
const gitServiceSrcPath = join(import.meta.dirname, "..", "git-service.ts");
|
||||
const gitServiceSrc = readFileSync(gitServiceSrcPath, "utf-8");
|
||||
|
||||
test("#2302 bug 3: createDraftPR should accept head and base branch parameters", () => {
|
||||
// Find the createDraftPR function signature
|
||||
const fnIdx = gitServiceSrc.indexOf("function createDraftPR");
|
||||
assert.ok(fnIdx !== -1, "createDraftPR function exists");
|
||||
|
||||
// Get the function signature (up to the closing paren)
|
||||
const sigEnd = gitServiceSrc.indexOf(")", fnIdx);
|
||||
const signature = gitServiceSrc.slice(fnIdx, sigEnd);
|
||||
|
||||
// Should have head and base parameters
|
||||
assert.ok(
|
||||
signature.includes("head") || signature.includes("branch"),
|
||||
"createDraftPR should accept a head/branch parameter",
|
||||
);
|
||||
});
|
||||
|
||||
test("#2302 bug 3: createDraftPR should pass --head and --base to gh pr create", () => {
|
||||
const fnIdx = gitServiceSrc.indexOf("function createDraftPR");
|
||||
const fnEnd = gitServiceSrc.indexOf("\n}", fnIdx);
|
||||
const fnBody = gitServiceSrc.slice(fnIdx, fnEnd);
|
||||
|
||||
assert.ok(
|
||||
fnBody.includes("--head"),
|
||||
"createDraftPR should pass --head to gh pr create",
|
||||
);
|
||||
assert.ok(
|
||||
fnBody.includes("--base"),
|
||||
"createDraftPR should pass --base to gh pr create",
|
||||
);
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue