fix: harden quick-task branch lifecycle — disk recovery + integration branch guard (#1342)
This commit is contained in:
deseltrus
GitHub
parent
e6ceb8dfe8
commit
2dc804a485
3 changed files with 340 additions and 3 deletions
|
|
@ -223,9 +223,16 @@ export function readIntegrationBranch(basePath: string, milestoneId: string): st
|
|||
*
|
||||
* The file is committed immediately so the metadata is persisted in git.
|
||||
*/
|
||||
/** Regex matching GSD quick-task branches: gsd/quick/<num>-<slug> */
|
||||
export const QUICK_BRANCH_RE = /^gsd\/quick\//;
|
||||
|
||||
export function writeIntegrationBranch(basePath: string, milestoneId: string, branch: string): void {
|
||||
// Don't record slice branches as the integration target
|
||||
if (SLICE_BRANCH_RE.test(branch)) return;
|
||||
// Don't record quick-task branches — they are ephemeral and merge back
|
||||
// to their origin branch on completion. Recording one as the integration
|
||||
// target causes milestone merges to land on the wrong branch (#1293).
|
||||
if (QUICK_BRANCH_RE.test(branch)) return;
|
||||
// Validate
|
||||
if (!VALID_BRANCH_NAME.test(branch)) return;
|
||||
// Skip if already recorded with the same branch (idempotent across restarts).
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@
|
|||
*/
|
||||
|
||||
import type { ExtensionAPI, ExtensionCommandContext } from "@gsd/pi-coding-agent";
|
||||
import { existsSync, mkdirSync, readdirSync } from "node:fs";
|
||||
import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync, unlinkSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { loadPrompt } from "./prompt-loader.js";
|
||||
import { gsdRoot } from "./paths.js";
|
||||
|
|
@ -168,6 +168,8 @@ export async function handleQuick(
|
|||
slug,
|
||||
description,
|
||||
};
|
||||
// Persist to disk so recovery works across session crashes (#1293).
|
||||
persistPendingReturn(_pendingQuickBranchReturn, basePath);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -181,15 +183,58 @@ let _pendingQuickBranchReturn: {
|
|||
description: string;
|
||||
} | null = null;
|
||||
|
||||
// ─── Disk Persistence ─────────────────────────────────────────────────────
|
||||
|
||||
/** Path to the pending quick-task return file. */
|
||||
function pendingReturnPath(basePath: string): string {
|
||||
return join(gsdRoot(basePath), "runtime", "quick-return.json");
|
||||
}
|
||||
|
||||
/** Write pending return state to disk. */
|
||||
function persistPendingReturn(state: NonNullable<typeof _pendingQuickBranchReturn>, basePath: string): void {
|
||||
const filePath = pendingReturnPath(basePath);
|
||||
mkdirSync(join(gsdRoot(basePath), "runtime"), { recursive: true });
|
||||
writeFileSync(filePath, JSON.stringify(state, null, 2) + "\n", "utf-8");
|
||||
}
|
||||
|
||||
/** Remove pending return file from disk. */
|
||||
function clearPendingReturn(basePath: string): void {
|
||||
try { unlinkSync(pendingReturnPath(basePath)); } catch { /* already gone */ }
|
||||
}
|
||||
|
||||
/** Load pending return from disk (cross-session recovery). */
|
||||
function loadPendingReturn(basePath: string): NonNullable<typeof _pendingQuickBranchReturn> | null {
|
||||
const filePath = pendingReturnPath(basePath);
|
||||
if (!existsSync(filePath)) return null;
|
||||
try {
|
||||
return JSON.parse(readFileSync(filePath, "utf-8"));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge the quick-task branch back to the original branch and switch.
|
||||
* Called from the agent_end handler after a quick task completes.
|
||||
*
|
||||
* Checks both in-memory state (same session) and disk state (cross-session
|
||||
* recovery for crashed/interrupted sessions).
|
||||
*
|
||||
* Returns true if a branch return was performed.
|
||||
*/
|
||||
export function cleanupQuickBranch(): boolean {
|
||||
if (!_pendingQuickBranchReturn) return false;
|
||||
const { basePath, originalBranch, quickBranch, taskNum, slug, description } = _pendingQuickBranchReturn;
|
||||
// Prefer in-memory state; fall back to disk for cross-session recovery
|
||||
let state = _pendingQuickBranchReturn;
|
||||
if (!state) {
|
||||
// Try loading from disk — handles the case where the session that
|
||||
// started the quick task crashed before agent_end could run (#1293).
|
||||
const basePath = process.cwd();
|
||||
state = loadPendingReturn(basePath);
|
||||
}
|
||||
if (!state) return false;
|
||||
|
||||
_pendingQuickBranchReturn = null;
|
||||
const { basePath, originalBranch, quickBranch, taskNum, slug, description } = state;
|
||||
|
||||
try {
|
||||
// Auto-commit any remaining work
|
||||
|
|
@ -205,8 +250,12 @@ export function cleanupQuickBranch(): boolean {
|
|||
|
||||
// Clean up quick branch
|
||||
try { runGit(basePath, ["branch", "-D", quickBranch]); } catch {}
|
||||
|
||||
// Clean up disk state
|
||||
clearPendingReturn(basePath);
|
||||
return true;
|
||||
} catch {
|
||||
// Cleanup failed — leave disk state for next attempt
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,281 @@
|
|||
/**
|
||||
* Tests for quick-task branch lifecycle:
|
||||
* - Branch creation → merge-back → cleanup
|
||||
* - Cross-session recovery via disk-persisted state
|
||||
* - captureIntegrationBranch guard against quick-task branches
|
||||
*
|
||||
* Relates to #1269, #1293.
|
||||
*/
|
||||
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync, readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { execSync } from "node:child_process";
|
||||
|
||||
import { createTestContext } from './test-helpers.ts';
|
||||
import { captureIntegrationBranch, getCurrentBranch } from "../worktree.ts";
|
||||
import { readIntegrationBranch, QUICK_BRANCH_RE } from "../git-service.ts";
|
||||
|
||||
const { assertEq, assertTrue, report } = createTestContext();
|
||||
|
||||
function run(command: string, cwd: string): string {
|
||||
return execSync(command, { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" }).trim();
|
||||
}
|
||||
|
||||
function createTestRepo(): string {
|
||||
const repo = mkdtempSync(join(tmpdir(), "gsd-quick-lifecycle-"));
|
||||
run("git init -b main", repo);
|
||||
run(`git config user.name "GSD Test"`, repo);
|
||||
run(`git config user.email "test@gsd.dev"`, repo);
|
||||
mkdirSync(join(repo, ".gsd", "runtime"), { recursive: true });
|
||||
mkdirSync(join(repo, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
writeFileSync(join(repo, "README.md"), "init\n");
|
||||
run("git add -A", repo);
|
||||
run(`git commit -m "init"`, repo);
|
||||
return repo;
|
||||
}
|
||||
|
||||
async function main(): Promise<void> {
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// QUICK_BRANCH_RE
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log("\n=== QUICK_BRANCH_RE: matches quick-task branches ===");
|
||||
|
||||
assertTrue(QUICK_BRANCH_RE.test("gsd/quick/1-fix-typo"), "matches standard quick branch");
|
||||
assertTrue(QUICK_BRANCH_RE.test("gsd/quick/42-some-long-slug-name"), "matches multi-digit quick branch");
|
||||
assertTrue(!QUICK_BRANCH_RE.test("main"), "rejects main");
|
||||
assertTrue(!QUICK_BRANCH_RE.test("gsd/M001/S01"), "rejects slice branch");
|
||||
assertTrue(!QUICK_BRANCH_RE.test("gsd/quickly-something"), "rejects non-quick prefix");
|
||||
assertTrue(!QUICK_BRANCH_RE.test("feature/gsd/quick/1"), "rejects nested prefix");
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// captureIntegrationBranch: guard against quick-task branches
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log("\n=== captureIntegrationBranch: skips quick-task branches ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
|
||||
// Create and checkout a quick-task branch
|
||||
run("git checkout -b gsd/quick/1-fix-typo", repo);
|
||||
assertEq(getCurrentBranch(repo), "gsd/quick/1-fix-typo", "on quick branch");
|
||||
|
||||
captureIntegrationBranch(repo, "M001");
|
||||
|
||||
assertEq(readIntegrationBranch(repo, "M001"), null,
|
||||
"captureIntegrationBranch is a no-op on quick-task branches");
|
||||
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── Verify main is still recorded correctly ─────────────────────────
|
||||
|
||||
console.log("\n=== captureIntegrationBranch: records main correctly ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
|
||||
// Capture from main — should work normally
|
||||
captureIntegrationBranch(repo, "M001");
|
||||
assertEq(readIntegrationBranch(repo, "M001"), "main",
|
||||
"main is recorded as integration branch");
|
||||
|
||||
// Switch to quick branch — capture should be no-op (doesn't overwrite main)
|
||||
run("git checkout -b gsd/quick/1-fix-typo", repo);
|
||||
captureIntegrationBranch(repo, "M001");
|
||||
assertEq(readIntegrationBranch(repo, "M001"), "main",
|
||||
"quick branch does not overwrite existing integration branch");
|
||||
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── Sequence: main → quick → back to main → capture ────────────────
|
||||
|
||||
console.log("\n=== captureIntegrationBranch: correct after quick branch round-trip ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
|
||||
// Simulate quick-task lifecycle: branch off, do work, return to main
|
||||
run("git checkout -b gsd/quick/1-fix-typo", repo);
|
||||
writeFileSync(join(repo, "fix.txt"), "fixed\n");
|
||||
run("git add -A", repo);
|
||||
run(`git commit -m "quick-fix"`, repo);
|
||||
run("git checkout main", repo);
|
||||
run("git merge --squash gsd/quick/1-fix-typo", repo);
|
||||
run(`git commit -m "quick(Q1): fix-typo"`, repo);
|
||||
run("git branch -D gsd/quick/1-fix-typo", repo);
|
||||
|
||||
// Now capture — should get main, not the deleted quick branch
|
||||
captureIntegrationBranch(repo, "M002");
|
||||
assertEq(readIntegrationBranch(repo, "M002"), "main",
|
||||
"after quick round-trip, main is captured correctly");
|
||||
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// cleanupQuickBranch: in-memory path (same session)
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log("\n=== cleanupQuickBranch: merges back and cleans up (same session) ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
const origCwd = process.cwd();
|
||||
|
||||
// Simulate what handleQuick does: create branch, set pending state
|
||||
run("git checkout -b gsd/quick/1-fix-typo", repo);
|
||||
writeFileSync(join(repo, "fix.txt"), "fixed\n");
|
||||
run("git add -A", repo);
|
||||
run(`git commit -m "quick-fix"`, repo);
|
||||
|
||||
// Write the disk state (simulating handleQuick's persistPendingReturn)
|
||||
const returnState = {
|
||||
basePath: repo,
|
||||
originalBranch: "main",
|
||||
quickBranch: "gsd/quick/1-fix-typo",
|
||||
taskNum: 1,
|
||||
slug: "fix-typo",
|
||||
description: "fix typo",
|
||||
};
|
||||
const runtimeDir = join(repo, ".gsd", "runtime");
|
||||
mkdirSync(runtimeDir, { recursive: true });
|
||||
writeFileSync(join(runtimeDir, "quick-return.json"), JSON.stringify(returnState) + "\n");
|
||||
|
||||
// Switch cwd to repo so cleanupQuickBranch finds the disk state
|
||||
process.chdir(repo);
|
||||
|
||||
// Import and call cleanupQuickBranch
|
||||
// Use dynamic import to get a fresh module scope — the in-memory state
|
||||
// won't be set, so it will fall through to disk recovery
|
||||
const { cleanupQuickBranch } = await import("../quick.ts");
|
||||
const result = cleanupQuickBranch();
|
||||
|
||||
assertTrue(result, "cleanupQuickBranch returns true");
|
||||
assertEq(getCurrentBranch(repo), "main", "back on main after cleanup");
|
||||
|
||||
// Verify merge happened — fix.txt should exist on main
|
||||
assertTrue(existsSync(join(repo, "fix.txt")), "fix.txt merged to main");
|
||||
|
||||
// Verify quick branch deleted
|
||||
const branches = run("git branch", repo);
|
||||
assertTrue(!branches.includes("gsd/quick/1-fix-typo"), "quick branch deleted");
|
||||
|
||||
// Verify disk state cleaned up
|
||||
assertTrue(!existsSync(join(runtimeDir, "quick-return.json")), "quick-return.json removed");
|
||||
|
||||
process.chdir(origCwd);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// cleanupQuickBranch: cross-session recovery from disk
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log("\n=== cleanupQuickBranch: recovers from disk state (cross-session) ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
const origCwd = process.cwd();
|
||||
|
||||
// Simulate a crashed session: branch exists with work, disk state persisted,
|
||||
// but in-memory state is gone (new process)
|
||||
run("git checkout -b gsd/quick/2-add-docs", repo);
|
||||
writeFileSync(join(repo, "docs.md"), "# Docs\n");
|
||||
run("git add -A", repo);
|
||||
run(`git commit -m "add-docs"`, repo);
|
||||
|
||||
// Write disk state manually (simulates what handleQuick would persist)
|
||||
const runtimeDir = join(repo, ".gsd", "runtime");
|
||||
mkdirSync(runtimeDir, { recursive: true });
|
||||
writeFileSync(join(runtimeDir, "quick-return.json"), JSON.stringify({
|
||||
basePath: repo,
|
||||
originalBranch: "main",
|
||||
quickBranch: "gsd/quick/2-add-docs",
|
||||
taskNum: 2,
|
||||
slug: "add-docs",
|
||||
description: "add docs",
|
||||
}) + "\n");
|
||||
|
||||
process.chdir(repo);
|
||||
|
||||
const { cleanupQuickBranch } = await import("../quick.ts");
|
||||
const result = cleanupQuickBranch();
|
||||
|
||||
assertTrue(result, "cross-session recovery returns true");
|
||||
assertEq(getCurrentBranch(repo), "main", "back on main after cross-session recovery");
|
||||
assertTrue(existsSync(join(repo, "docs.md")), "docs.md merged to main");
|
||||
assertTrue(!existsSync(join(runtimeDir, "quick-return.json")), "disk state cleaned up");
|
||||
|
||||
process.chdir(origCwd);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// cleanupQuickBranch: no-op when no pending state
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log("\n=== cleanupQuickBranch: no-op without pending state ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
const origCwd = process.cwd();
|
||||
process.chdir(repo);
|
||||
|
||||
const { cleanupQuickBranch } = await import("../quick.ts");
|
||||
const result = cleanupQuickBranch();
|
||||
|
||||
assertTrue(!result, "returns false when no pending state");
|
||||
assertEq(getCurrentBranch(repo), "main", "stays on main");
|
||||
|
||||
process.chdir(origCwd);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
// End-to-end: quick branch does NOT contaminate integration branch
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
console.log("\n=== E2E: quick branch does not contaminate integration branch ===");
|
||||
|
||||
{
|
||||
const repo = createTestRepo();
|
||||
|
||||
// 1. Record main as integration branch for M001
|
||||
captureIntegrationBranch(repo, "M001");
|
||||
assertEq(readIntegrationBranch(repo, "M001"), "main", "M001 integration = main");
|
||||
|
||||
// 2. Start a quick task (branch off)
|
||||
run("git checkout -b gsd/quick/1-fix-typo", repo);
|
||||
|
||||
// 3. Try to capture integration branch for M002 while on quick branch
|
||||
captureIntegrationBranch(repo, "M002");
|
||||
assertEq(readIntegrationBranch(repo, "M002"), null,
|
||||
"M002 integration NOT recorded from quick branch");
|
||||
|
||||
// 4. Return to main (simulate cleanupQuickBranch)
|
||||
run("git checkout main", repo);
|
||||
|
||||
// 5. Now capture M002 from main — should work
|
||||
captureIntegrationBranch(repo, "M002");
|
||||
assertEq(readIntegrationBranch(repo, "M002"), "main",
|
||||
"M002 integration = main after returning from quick branch");
|
||||
|
||||
// 6. Verify M001 still intact
|
||||
assertEq(readIntegrationBranch(repo, "M001"), "main",
|
||||
"M001 integration unchanged");
|
||||
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
report();
|
||||
}
|
||||
|
||||
main().catch((error) => {
|
||||
console.error(error);
|
||||
process.exit(1);
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue