From f9b57ec2edc62a080da6c6ed3bff0caf9d6ddd51 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Sun, 2 Aug 2026 23:36:35 -0400 Subject: [PATCH] fix flaky resume behavior --- index.ts | 206 +++++++++++++++++++++---- package.json | 2 +- src/executor.ts | 33 +++- src/progress.ts | 27 ++++ src/utils.ts | 28 ++++ src/worktree.ts | 105 ++++++++++--- tests/progress-multiprd.test.ts | 82 ++++++++++ tests/worktree-resume.test.ts | 265 ++++++++++++++++++++++++++++++++ 8 files changed, 693 insertions(+), 55 deletions(-) create mode 100644 tests/progress-multiprd.test.ts create mode 100644 tests/worktree-resume.test.ts diff --git a/index.ts b/index.ts index 7c09c0a..70941b2 100644 --- a/index.ts +++ b/index.ts @@ -23,6 +23,7 @@ import { executeBatch, type SendChatMessage } from "./src/executor"; import { cleanupStaleWorktrees, finalizeCommittedWorktrees, + abortMerge, } from "./src/worktree"; import { loadConfig, @@ -585,27 +586,9 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { t.status === "in_progress" ? [id] : [], ); - if (inProgressIds.length === 0) { - // Nothing was mid-flight — loop either finished cleanly between - // the reload landing and this handler running, or was stopped - // between tasks. Clean up the stale marker and bail. - ctx.ui.notify( - "ralpi loop has no in-progress task to resume — marking complete.", - "info", - ); - deleteLoopActive(projectDir); - return; - } - - const taskCount = loopState.taskIds.length; - ctx.ui.notify( - `ralpi loop was interrupted by reload with ${inProgressIds.length} in-progress task(s). ` + - `Resuming execution (${taskCount} tasks, ${loopState.mode} mode)...`, - "info", - ); - // Build the sendProgress wrapper so resumed task messages render the - // same expandable tool-call tree as an interactive run. + // same expandable tool-call tree as an interactive run. Defined before + // the finalize path below so it can report self-healed merges. const sendProgress: SendChatMessage = ( content: string, meta?: { @@ -629,6 +612,121 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { }); }; + if (inProgressIds.length === 0) { + // Nothing was mid-flight — the loop either finished cleanly between + // the reload landing and this handler running, or was stopped + // between tasks. Either way, committed worktree branches from an + // interrupted loop may still be unmerged (e.g. a prior resume + // attempt reset tasks to pending before it was itself interrupted). + // Finalize those first so committed code lands in the workspace, + // persist the state to progress.json, update the PRD file, THEN + // clean up the stale marker. + try { + const config = loadConfig(projectDir); + // Clear any half-done merge left by an interrupted + // conflict-resolution session (it would block every merge below). + abortMerge(projectDir); + const allIds = Object.entries(initialTasks).flatMap(([id, t]) => + t.status !== "failed" && t.status !== "pending" ? [id] : [], + ); + const fin = finalizeCommittedWorktrees( + projectDir, + config.paths.stateDir, + loopState.prdKey, + allIds, + ); + // Persist finalized tasks to progress.json + PRD file so the + // state is correct for subsequent /ralpi resume calls. + const stateDir = config.paths.stateDir; + const progressPath = path.join(projectDir, stateDir, "progress.json"); + // Batch-update progress.json and PRD file for all finalized tasks + if (fin.finalized.length > 0) { + const progressRaw = fs.existsSync(progressPath) + ? JSON.parse(fs.readFileSync(progressPath, "utf-8")) + : null; + for (const id of fin.finalized) { + sendProgress?.( + `✓ ${id} — finalized on resume (committed branch merged into main)`, + ); + if (progressRaw) { + const tasks = + progressRaw.prds?.[loopState.prdKey]?.tasks ?? progressRaw.tasks; + if (tasks && tasks[id]) { + tasks[id].status = "completed"; + tasks[id].completedAt = new Date().toISOString(); + } + } + try { + const prdPath = loopState.taskFile; + if (fs.existsSync(prdPath)) { + updateTaskInFile(prdPath, id, "completed"); + } + } catch { + // Best-effort + } + } + if (progressRaw) { + fs.writeFileSync(progressPath, JSON.stringify(progressRaw, null, 2), "utf-8"); + } + } + // ── Handle conflicted tasks ── + // Same logic as resumeLoop: reset to pending so the DAG can + // re-schedule them, keep the worktree for in-place re-run. + const conflictIds = Object.keys(fin.conflicts); + if (conflictIds.length > 0) { + const detail = conflictIds + .map((id) => `${id}: ${fin.conflicts[id].slice(0, 3).join(", ")}`) + .join("; "); + // Batch-reset all conflicted tasks to pending, then write once + const progressRaw = fs.existsSync(progressPath) + ? JSON.parse(fs.readFileSync(progressPath, "utf-8")) + : null; + for (const id of conflictIds) { + if (progressRaw) { + const tasks = + progressRaw.prds?.[loopState.prdKey]?.tasks ?? progressRaw.tasks; + if (tasks && tasks[id]) { + tasks[id].status = "pending"; + delete tasks[id].startedAt; + delete tasks[id].error; + } + } + try { + const prdPath = loopState.taskFile; + if (fs.existsSync(prdPath)) { + updateTaskInFile(prdPath, id, "pending"); + } + } catch { + // Best-effort + } + } + if (progressRaw) { + fs.writeFileSync(progressPath, JSON.stringify(progressRaw, null, 2), "utf-8"); + } + ctx.ui.notify( + `Reset ${conflictIds.length} conflicted task(s) to pending for re-execution (${detail})`, + "info", + ); + } + } catch { + // Best-effort — the marker is removed either way; the worktrees + // stay on disk for a manual /ralpi-resume. + } + ctx.ui.notify( + "ralpi loop has no in-progress task to resume — marking complete.", + "info", + ); + deleteLoopActive(projectDir); + return; + } + + const taskCount = loopState.taskIds.length; + ctx.ui.notify( + `ralpi loop was interrupted by reload with ${inProgressIds.length} in-progress task(s). ` + + `Resuming execution (${taskCount} tasks, ${loopState.mode} mode)...`, + "info", + ); + // Load config from the project directory so model + thinking level // resolve the same way the interactive command handler does. const config = loadConfig(projectDir); @@ -922,22 +1020,36 @@ async function resumeLoop( // A review-gated task whose agent committed + reviewed successfully still // needs a final merge into main + worktree removal to be "done". If the // loop was interrupted between that commit and the merge, the task is left - // `in_progress` with a clean, committed worktree branch. Resuming without - // finalizing would wastefully re-run finished work. + // with a committed worktree branch. Resuming without finalizing would + // wastefully re-run finished work — or worse, strand the committed code in + // `.ralpi/worktrees/` forever. // - // finalizeCommittedWorktrees merges those branches into main now; the - // rest (dirty trees, nothing committed, conflicts) are left in_progress - // and reset to pending below for a real re-run. + // finalizeCommittedWorktrees runs over EVERY non-failed task, not just + // `in_progress` ones: a prior interrupted resume can reset tasks to + // `pending` while their worktree branch still holds committed work that + // was never merged. Only scanning in_progress tasks would silently leave + // that code out of the workspace on every resume. + // + // `pending` tasks (never started) are excluded — they never had worktrees + // created, so finalize always puts them in `rerun`, which is wasted work. + // Failed tasks keep their worktrees for inspection/re-run and are also + // deliberately excluded. const prdKeyForFinalize = progress.getKey(); - const inProgressIds = Object.entries(progress.getState().tasks) - .filter(([, t]) => t.status === "in_progress") - .map(([id]) => id); - if (inProgressIds.length > 0) { + // Clear any half-done merge left in the main repo by an interrupted + // conflict-resolution session — it would block every merge below + // (`git merge` refuses while a merge is already in progress). No-op when + // the repo isn't mid-merge. + abortMerge(projectDir); + const finalizeCandidateIds = Object.entries(progress.getState().tasks) + .flatMap(([id, t]) => + t.status !== "failed" && t.status !== "pending" ? [id] : [], + ); + if (finalizeCandidateIds.length > 0) { const fin = finalizeCommittedWorktrees( projectDir, config.paths.stateDir, prdKeyForFinalize, - inProgressIds, + finalizeCandidateIds, ); for (const id of fin.finalized) { progress.markCompleted(id, 0); @@ -950,15 +1062,47 @@ async function resumeLoop( `✓ ${id} — finalized on resume (committed branch merged into main)`, ); } + // ── Handle conflicted tasks ── + // + // Tasks whose committed branch could not be auto-merged (git conflicts) + // must be reset to `pending` so the DAG re-schedules them. The worktree + // is preserved — the agent re-runs in-place in the existing worktree via + // createWorktree's reuse logic. If the agent's re-run changes make the + // merge succeed on the next attempt, the loop continues normally. If the + // merge fails again, `executeBatch`'s batch-level conflict resolution + // (`resolveConflictsSession`) handles the conflict markers properly. const conflictIds = Object.keys(fin.conflicts); if (conflictIds.length > 0) { const detail = conflictIds .map((id) => `${id}: ${fin.conflicts[id].slice(0, 3).join(", ")}`) .join("; "); + // Batch-reset all conflicted tasks to pending, then save once. + // Directly mutate the progress state (there's no markPending method + // on ProgressTracker — markFailed would leave it as 'failed' which the + // DAG excludes). The worktree is preserved so createWorktree reuses + // it and the agent re-runs in-place. + const tasks = progress.getState().tasks; + for (const id of conflictIds) { + if (tasks[id]) { + tasks[id].status = "pending"; + delete tasks[id].startedAt; + delete tasks[id].error; + } + try { + updateTaskInFile(taskFile, id, "pending"); + } catch { + // Best-effort + } + } + progress.save(); sendChatMessage?.( `⚠ ${conflictIds.join( ", ", - )} — merge conflict on resume-finalize; re-running (${detail})`, + )} — merge conflict on resume-finalize; reset to pending for re-execution (${detail})`, + ); + ctx.ui.notify( + `Reset ${conflictIds.length} conflicted task(s) to pending for re-execution`, + "info", ); } } diff --git a/package.json b/package.json index c3710f5..0971c19 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mikefreno/ralpi", - "version": "0.4.1", + "version": "0.4.2", "description": "Execute tasks from task files/PRD's using DAG-based dependency resolution with persistent progress tracking", "keywords": [ "pi-package", diff --git a/src/executor.ts b/src/executor.ts index ae9ab28..4202846 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -35,6 +35,7 @@ import { abortMerge, hasMergeConflicts, completeMerge, + worktreeHasPreservableWork, type WorktreeHandle, type MergeResult, } from "./worktree"; @@ -1248,7 +1249,7 @@ async function executeTask( }`, "error", ); - if (wt) removeWorktree(projectDir, wt); + cleanupFailedWorktree(projectDir, wt, task, sendChatMessage); roundRobin?.release(task.id); return; } catch (error) { @@ -1264,7 +1265,7 @@ async function executeTask( } sendChatMessage?.(`✗ ${task.id} · ${task.title} — ${errorMsg}`); ctx.ui.notify(`Task ${task.id} failed: ${errorMsg}`, "error"); - if (wt) removeWorktree(projectDir, wt); + cleanupFailedWorktree(projectDir, wt, task, sendChatMessage); return; } } @@ -1280,11 +1281,37 @@ async function executeTask( `Task ${task.id} failed: all configured models exhausted`, "error", ); - if (wt) removeWorktree(projectDir, wt); + cleanupFailedWorktree(projectDir, wt, task, sendChatMessage); } // ─── Save Reflection to File ──────────────────────────────────────────────── +/** + * Remove a task worktree after a failure UNLESS it still holds recoverable + * work (commits ahead of main, or uncommitted changes). + * + * `removeWorktree` force-deletes the worktree's branch, which makes any + * commits the agent made before failing/timing out unreachable — real code + * loss. A preserved worktree is instead picked up on the next resume: + * resume-finalize merges committed work into main, or the task re-runs in + * place and the agent continues from where it stopped. + */ +function cleanupFailedWorktree( + projectDir: string, + wt: WorktreeHandle | null, + task: Task, + sendChatMessage?: SendChatMessage, +): void { + if (!wt) return; + if (worktreeHasPreservableWork(projectDir, wt)) { + sendChatMessage?.( + `~ ${task.id} · ${task.title} — task failed but worktree preserved (${wt.branch}); committed work will be merged on resume`, + ); + return; + } + removeWorktree(projectDir, wt); +} + function saveReflectionToFile( sourceDir: string, config: RalpiConfig, diff --git a/src/progress.ts b/src/progress.ts index 5f79479..804fc6d 100644 --- a/src/progress.ts +++ b/src/progress.ts @@ -142,6 +142,33 @@ export class ProgressTracker { /** Save current state to disk */ save(): void { + // Merge into the freshest on-disk state instead of writing the + // construction-time snapshot verbatim. Each ProgressTracker instance + // (one per PRD loop) snapshots the WHOLE state at construction; when + // two loops run concurrently in one project, saving a stale snapshot + // would silently revert the OTHER loop's task status changes — tasks + // get wrongly written back to "pending" while their worktrees carry + // real work, stranding it on the next resume. + let disk: ProgressState | null = null; + try { + if (fs.existsSync(this.statePath)) { + const raw = fs.readFileSync(this.statePath, "utf-8"); + disk = JSON.parse(raw) as ProgressState; + } + } catch { + disk = null; + } + if (disk && disk.prds) { + // Keep THIS tracker's in-memory PRD (its own tasks are the source + // of truth — all status mutations happened on it), but adopt the + // on-disk entries for OTHER PRDs instead of writing the stale + // construction-time snapshot over them. + const mine = this.getPRD(); + this.state = disk; + this.state.prds ??= {}; + this.state.prds[this.prdKey] = mine; + } + const prd = this.getPRD(); prd.lastUpdatedAt = new Date().toISOString(); // Sync legacy flat fields with current PRD for backward compat diff --git a/src/utils.ts b/src/utils.ts index bcf10cc..f0e2811 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -673,6 +673,8 @@ function extractAssistantText(content: unknown): string { /** * Check if there are any uncommitted changes in the git repository. + * Includes untracked files — a new file created by a task agent is work + * that still needs committing. */ export function hasUncommittedChanges(projectDir: string): boolean { const { execSync } = require("node:child_process"); @@ -687,6 +689,32 @@ export function hasUncommittedChanges(projectDir: string): boolean { } } +/** + * Check for uncommitted changes to TRACKED files only, ignoring untracked + * (`??`) entries. + * + * Untracked files never block a merge, so a worktree whose task work is + * fully committed is "done" even when it carries stray untracked files + * (scratch files, build artifacts, files created but deliberately left out + * of the commit). Resume-finalize uses this to decide whether a task's + * committed branch should be merged into main: counting `??` entries there + * would strand committed code in `.ralpi/worktrees/` forever. + */ +export function hasTrackedUncommittedChanges(projectDir: string): boolean { + const { execSync } = require("node:child_process"); + try { + const output = execSync("git status --porcelain", { + cwd: projectDir, + encoding: "utf-8", + }).trim(); + return output + .split("\n") + .some((line: string) => line.length > 0 && !line.startsWith("??")); + } catch { + return false; + } +} + /** * Get the current git status in porcelain format. * Includes untracked files, which `git diff` alone would miss. diff --git a/src/worktree.ts b/src/worktree.ts index 3b45b46..d17dd11 100644 --- a/src/worktree.ts +++ b/src/worktree.ts @@ -1,6 +1,10 @@ import * as fs from "node:fs"; import * as path from "node:path"; -import { ensureDir, hasUncommittedChanges } from "./utils"; +import { + ensureDir, + hasUncommittedChanges, + hasTrackedUncommittedChanges, +} from "./utils"; // ─── Types ─────────────────────────────────────────────────────────────────── @@ -83,6 +87,28 @@ export function getCurrentBranch(dir: string): string | null { return git("rev-parse --abbrev-ref HEAD", dir); } +/** + * Canonicalize a directory path, resolving symlinks. + * + * `git worktree list --porcelain` emits REAL paths (symlinks resolved, + * e.g. `/private/tmp/...` for `/tmp/...` on macOS), while `path.join` on a + * caller-supplied path keeps the literal spelling. Comparing the two + * verbatim silently fails — resume then can't see an existing worktree, + * `createWorktree` falls through to a fresh `worktree add` that fails + * because the directory already exists, returns null, and the task agent + * ends up running in the MAIN repo with no worktree merge at all. + * + * All worktree path computation and porcelain comparisons go through this + * so literal vs real paths can never diverge. + */ +function canonicalDir(dir: string): string { + try { + return fs.realpathSync(dir); + } catch { + return path.resolve(dir); + } +} + // ─── Worktree Lifecycle ────────────────────────────────────────────────────── /** @@ -152,6 +178,9 @@ export function createWorktree( baseRef?: string, taskTitle?: string, ): WorktreeHandle | null { + // Canonicalize FIRST: every path below (worktree dir, porcelain + // comparisons, branch refs) must share one spelling of the repo path. + mainDir = canonicalDir(mainDir); if (!isGitRepo(mainDir)) return null; const safeId = safeBranchSuffix(taskId); @@ -357,7 +386,7 @@ export function cleanupStaleWorktrees( // When a prdKey is given, narrow to that PRD's subdir so concurrent // loops (other PRDs) are not disturbed. const managedRoot = path.resolve( - mainDir, + canonicalDir(mainDir), stateDir, "worktrees", ...(prdKey ? [prdKey] : []), @@ -408,20 +437,27 @@ export interface FinalizeResult { } /** - * Finalize in-progress tasks whose worktrees already hold committed, clean - * work that was never merged into main (typically because the loop was - * interrupted between the task commit and the merge/finalize step). + * Finalize worktrees that already hold committed, clean work that was never + * merged into main (typically because the loop was interrupted between the + * task commit and the merge/finalize step). * * For each task ID: * - If no worktree exists / is registered → re-run (fresh worktree later). - * - If the worktree working tree is dirty (uncommitted edits) → re-run, - * preserving the worktree so `createWorktree` reuses it and the agent - * continues where it left off. - * - If the worktree is clean but has no commits ahead of main → re-run. - * - If the worktree is clean AND has ≥1 commit ahead of main → merge the - * branch into main (`--no-ff`), remove the worktree, and report finalized. - * On merge conflict the merge is aborted (main left clean), the worktree - * is preserved, and the task is reported in `conflicts`. + * - If the worktree has uncommitted edits to TRACKED files (e.g. an + * interrupted agent mid-edit) → re-run, preserving the worktree so + * `createWorktree` reuses it and the agent continues where it left off. + * Untracked files are ignored here — they never block a merge, and a + * worktree whose task work is fully committed is "done" even if it + * carries stray untracked files. Counting `??` entries would strand the + * committed branch in `.ralpi/worktrees/` forever on every resume. + * - If the worktree has no commits ahead of main → re-run. + * - If the worktree has ≥1 commit ahead of main → merge the branch into + * main (`--no-ff`) and report finalized. Fully clean worktrees are then + * removed; worktrees that also carry untracked files are kept so that + * (possibly meaningful) uncommitted files aren't destroyed — the next + * fresh-loop sweep cleans them up. On merge conflict the merge is + * aborted (main left clean), the worktree is preserved, and the task is + * reported in `conflicts`. * * This is the self-healing path for an interrupted review-gated loop: * tasks that finished (commit + review already saved) but never got their @@ -434,6 +470,7 @@ export function finalizeCommittedWorktrees( prdKey: string, taskIds: string[], ): FinalizeResult { + mainDir = canonicalDir(mainDir); const result: FinalizeResult = { finalized: [], rerun: [], conflicts: {} }; const mainHead = getGitHead(mainDir); @@ -463,14 +500,15 @@ export function finalizeCommittedWorktrees( continue; } - // Dirty working tree (uncommitted edits, e.g. an interrupted agent) → - // re-run, keeping the worktree so the agent resumes in place. - if (hasUncommittedChanges(wtDir)) { + // Uncommitted edits to TRACKED files (an interrupted agent mid-edit) → + // re-run, keeping the worktree so the agent resumes in place. Untracked + // files alone do NOT count as dirty here (see doc comment above). + if (hasTrackedUncommittedChanges(wtDir)) { result.rerun.push(taskId); continue; } - // Clean tree but nothing committed ahead of main → nothing to merge. + // No commits ahead of main → nothing to merge. const aheadStr = mainHead !== null ? git(`rev-list --count ${mainHead}..HEAD`, wtDir) @@ -481,11 +519,17 @@ export function finalizeCommittedWorktrees( continue; } - // Committed + clean → finalize. mergeWorktree aborts on conflict, - // leaving main's working tree clean. + // Committed + no tracked edits → finalize. mergeWorktree aborts on + // conflict, leaving main's working tree clean. const merge = mergeWorktree(mainDir, branch); if (merge.success) { - removeWorktree(mainDir, { dir: wtDir, branch, mainDir }); + // Remove the worktree only when it's fully clean. If it still carries + // untracked files, keep it so that uncommitted work isn't destroyed + // (the branch is merged; the leftover worktree is swept by the next + // fresh-loop cleanup). + if (!hasUncommittedChanges(wtDir)) { + removeWorktree(mainDir, { dir: wtDir, branch, mainDir }); + } result.finalized.push(taskId); continue; } @@ -497,3 +541,24 @@ export function finalizeCommittedWorktrees( git("worktree prune", mainDir); return result; } + +/** + * Whether a worktree still holds work worth preserving (committed commits + * ahead of main, or uncommitted changes). Used by the task-failure path so a + * failed/timeout agent's partial output isn't force-deleted with the + * worktree. + */ +export function worktreeHasPreservableWork( + mainDir: string, + wt: WorktreeHandle, +): boolean { + mainDir = canonicalDir(mainDir); + // Any uncommitted changes (tracked edits or untracked files) count — the + // agent may have been mid-write when it failed. + if (hasUncommittedChanges(wt.dir)) return true; + const mainHead = getGitHead(mainDir); + if (!mainHead) return true; + const aheadStr = git(`rev-list --count ${mainHead}..${wt.branch}`, mainDir); + const ahead = aheadStr !== null ? parseInt(aheadStr, 10) : 0; + return !Number.isNaN(ahead) && ahead > 0; +} diff --git a/tests/progress-multiprd.test.ts b/tests/progress-multiprd.test.ts new file mode 100644 index 0000000..5c0e529 --- /dev/null +++ b/tests/progress-multiprd.test.ts @@ -0,0 +1,82 @@ +/// +import { describe, it, expect, beforeEach } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { ProgressTracker } from "../src/progress"; + +/** + * Regression test: two concurrent loops (different PRDs) each run their own + * ProgressTracker. Each instance snapshots the whole state at construction; + * a save() that writes that stale snapshot verbatim would revert the OTHER + * loop's task status changes — tasks wrongly back to "pending" while their + * worktrees carry real work, stranding it on the next resume. + */ +let root: string; + +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), "ralpi-prog-test-")); +}); + +function prdA(projectDir: string): ProgressTracker { + return new ProgressTracker( + projectDir, + path.join(projectDir, "tasks/a/README.md"), + ); +} +function prdB(projectDir: string): ProgressTracker { + return new ProgressTracker( + projectDir, + path.join(projectDir, "tasks/b/README.md"), + ); +} + +/** Read the on-disk progress state; the file is written by the tracker, so + * a parse failure is a test bug worth surfacing. */ +function readState(): Record { + const raw = fs.readFileSync( + path.join(root, ".ralpi", "progress.json"), + "utf-8", + ); + try { + return JSON.parse(raw) as Record; + } catch { + throw new Error(`malformed progress.json:\n${raw.slice(0, 200)}`); + } +} + +describe("ProgressTracker multi-PRD save isolation", () => { + it("does not clobber another PRD's task status on save", () => { + const a = prdA(root); + const b = prdB(root); + expect(a.getKey()).not.toBe(b.getKey()); + + // Loop A marks its task in_progress. + a.markInProgress("01"); + expect(a.getTaskStatus("01")).toBe("in_progress"); + + // Loop B (stale snapshot from before A's update) marks ITS task. + b.markInProgress("02"); + + // The on-disk state must show BOTH updates. + const raw = readState(); + expect(raw.prds[a.getKey()].tasks["01"].status).toBe("in_progress"); + expect(raw.prds[b.getKey()].tasks["02"].status).toBe("in_progress"); + }); + + it("preserves other PRD completions when this PRD saves", () => { + const a = prdA(root); + const b = prdB(root); + + a.markCompleted("01", 1000); + b.markInProgress("02"); + + // A completes another task later — A's save must not revert B. + a.markCompleted("03", 500); + + const raw = readState(); + expect(raw.prds[a.getKey()].tasks["01"].status).toBe("completed"); + expect(raw.prds[a.getKey()].tasks["03"].status).toBe("completed"); + expect(raw.prds[b.getKey()].tasks["02"].status).toBe("in_progress"); + }); +}); diff --git a/tests/worktree-resume.test.ts b/tests/worktree-resume.test.ts new file mode 100644 index 0000000..b55d62c --- /dev/null +++ b/tests/worktree-resume.test.ts @@ -0,0 +1,265 @@ +/// +import { describe, it, expect, beforeEach } from "bun:test"; +import { execSync } from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { + createWorktree, + finalizeCommittedWorktrees, + mergeWorktree, + removeWorktree, + worktreeHasPreservableWork, +} from "../src/worktree"; + +/** + * Regression tests for worktree resume/finalize behavior: + * + * 1. finalizeCommittedWorktrees merges a committed worktree branch even + * when the worktree carries UNTRACKED files (previously the dirty check + * counted `??` entries, stranding committed code in .ralpi/worktrees/). + * 2. finalize works for tasks that are NOT in_progress (pending) — the + * stranded-work case after an interrupted resume. + * 3. createWorktree reuses an existing worktree under a symlinked project + * path (git porcelain emits realpaths; literal path.join must not be + * compared verbatim). + * 4. worktreeHasPreservableWork keeps failed-task branches alive so a + * timeout doesn't destroy commits the agent already made. + */ + +const sh = (cmd: string, cwd: string): string => { + try { + return execSync(cmd, { cwd, encoding: "utf-8" }).trim(); + } catch (err) { + throw new Error( + `git cmd failed in ${cwd}: ${cmd}\n${(err as Error).message}`, + ); + } +}; + +const STATE_DIR = ".ralpi"; +const PRD_KEY = "prd"; + +function makeRepo(root: string): void { + sh("git init -q -b master .", root); + sh("git config user.email t@t.co", root); + sh("git config user.name T", root); + sh("echo '# Demo' > README.md", root); + sh("git add -A && git commit -qm init", root); +} + +/** Commit work in a worktree and record the commit message. */ +function commitInWorktree(wt: { dir: string }, filename: string, msg: string) { + sh(`echo '${filename} content' > ${filename}`, wt.dir); + sh(`git add -A && git commit -qm '${msg}'`, wt.dir); +} + +function masterHasFile(root: string, filename: string): boolean { + try { + sh(`git show master:${filename}`, root); + return true; + } catch { + return false; + } +} + +let root: string; + +beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), "ralpi-wt-test-")); + makeRepo(root); +}); + +describe("finalizeCommittedWorktrees", () => { + it("merges a committed worktree branch even when untracked files exist", () => { + const wt = createWorktree( + root, + STATE_DIR, + "01", + PRD_KEY, + undefined, + "task one", + )!; + commitInWorktree(wt, "work.txt", "task 01 work"); + // The task agent left a scratch file untracked (like build artifacts). + sh("mkdir -p scratch && echo junk > scratch/junk.bin", wt.dir); + expect(sh("git status --porcelain", wt.dir)).toContain("??"); + + const fin = finalizeCommittedWorktrees(root, STATE_DIR, PRD_KEY, ["01"]); + + expect(fin.finalized).toEqual(["01"]); + expect(masterHasFile(root, "work.txt")).toBe(true); + }); + + it("finalizes tasks that are pending (not in_progress) with committed work", () => { + const wt = createWorktree( + root, + STATE_DIR, + "02", + PRD_KEY, + undefined, + "task two", + )!; + commitInWorktree(wt, "b.txt", "task 02 work"); + // Simulate a prior interrupted resume: task reset to pending, branch + // never merged. + const fin = finalizeCommittedWorktrees(root, STATE_DIR, PRD_KEY, ["02"]); + expect(fin.finalized).toEqual(["02"]); + expect(masterHasFile(root, "b.txt")).toBe(true); + }); + + it("leaves a worktree with uncommitted TRACKED edits for re-run", () => { + const wt = createWorktree( + root, + STATE_DIR, + "03", + PRD_KEY, + undefined, + "task three", + )!; + commitInWorktree(wt, "c.txt", "task 03 work"); + // Agent was mid-edit when interrupted: a tracked file modified. + sh("echo more >> README.md", wt.dir); + + const fin = finalizeCommittedWorktrees(root, STATE_DIR, PRD_KEY, ["03"]); + expect(fin.finalized).toEqual([]); + expect(fin.rerun).toEqual(["03"]); + expect(masterHasFile(root, "c.txt")).toBe(false); + }); + + it("does not re-merge an already-merged branch", () => { + const wt = createWorktree( + root, + STATE_DIR, + "04", + PRD_KEY, + undefined, + "task four", + )!; + commitInWorktree(wt, "d.txt", "task 04 work"); + expect(mergeWorktree(root, wt.branch).success).toBe(true); + removeWorktree(root, wt); + + // Re-create a worktree on the same (now-merged) branch tip: nothing + // ahead of main → re-run, no spurious merge. + const wt2 = createWorktree( + root, + STATE_DIR, + "04", + PRD_KEY, + undefined, + "task four", + )!; + commitInWorktree(wt2, "e.txt", "task 04 more work"); + const fin = finalizeCommittedWorktrees(root, STATE_DIR, PRD_KEY, ["04"]); + expect(fin.finalized).toEqual(["04"]); + expect(masterHasFile(root, "e.txt")).toBe(true); + }); + + it("reports conflicts and preserves the worktree", () => { + const wt = createWorktree( + root, + STATE_DIR, + "05", + PRD_KEY, + undefined, + "task five", + )!; + // Both sides edit f.txt: master AFTER the worktree exists, so the + // branches genuinely diverge and the merge must conflict. + sh( + "echo master > f.txt && git add -A && git commit -qm 'master f.txt'", + root, + ); + sh("echo worktree > f.txt", wt.dir); + sh("git add -A && git commit -qm 'task 05 work'", wt.dir); + + const fin = finalizeCommittedWorktrees(root, STATE_DIR, PRD_KEY, ["05"]); + expect(fin.finalized).toEqual([]); + expect(fin.conflicts["05"]).toBeTruthy(); + // worktree preserved for manual resolution + expect(fs.existsSync(wt.dir)).toBe(true); + }); +}); + +describe("createWorktree resume reuse under symlinked paths", () => { + it("reuses an existing worktree when the project path contains a symlink", () => { + // macOS /tmp → /private/tmp style symlink: git porcelain reports the + // REAL path, path.join keeps the literal one. Reuse must still match. + const realBase = fs.mkdtempSync(path.join(os.tmpdir(), "ralpi-wt-real-")); + const link = path.join(realBase, "link"); + fs.mkdirSync(path.join(realBase, "repo")); + fs.symlinkSync(path.join(realBase, "repo"), link); + const symRoot = link; + + makeRepo(symRoot); + // Sanity: this is genuinely a symlink situation. + expect(fs.realpathSync(symRoot)).not.toBe(symRoot); + + const wt = createWorktree( + symRoot, + STATE_DIR, + "01", + PRD_KEY, + undefined, + "task one", + )!; + commitInWorktree(wt, "a.txt", "task 01 work"); + + // Resume: createWorktree again must REUSE the registered worktree + // (same dir), not fail and fall through to the main repo. + const reused = createWorktree( + symRoot, + STATE_DIR, + "01", + PRD_KEY, + undefined, + "task one", + )!; + expect(reused.dir).toBe(fs.realpathSync(wt.dir)); + + const fin = finalizeCommittedWorktrees(symRoot, STATE_DIR, PRD_KEY, ["01"]); + expect(fin.finalized).toEqual(["01"]); + expect(masterHasFile(fs.realpathSync(symRoot), "a.txt")).toBe(true); + }); +}); + +describe("worktreeHasPreservableWork", () => { + it("returns true for a worktree with committed work ahead of main", () => { + const wt = createWorktree( + root, + STATE_DIR, + "01", + PRD_KEY, + undefined, + "task one", + )!; + commitInWorktree(wt, "a.txt", "task 01 work"); + expect(worktreeHasPreservableWork(root, wt)).toBe(true); + }); + + it("returns true for a worktree with uncommitted changes", () => { + const wt = createWorktree( + root, + STATE_DIR, + "01", + PRD_KEY, + undefined, + "task one", + )!; + sh("echo x > junk.txt", wt.dir); + expect(worktreeHasPreservableWork(root, wt)).toBe(true); + }); + + it("returns false for an empty fresh worktree", () => { + const wt = createWorktree( + root, + STATE_DIR, + "01", + PRD_KEY, + undefined, + "task one", + )!; + expect(worktreeHasPreservableWork(root, wt)).toBe(false); + }); +});