Mike/ralpi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix flaky resume behavior

Browse Source
This commit is contained in:
Michael Freno
2026-08-02 23:36:35 -04:00
parent b0749467c9
commit f9b57ec2ed
8 changed files with 693 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
src/executor.ts
View File

@@ -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,

27
src/progress.ts
View File

@@ -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

28
src/utils.ts
View File

@@ -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.

105
src/worktree.ts
View File

@@ -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;
}