smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(cli): recognize isolation marker before mount exists

Browse Source 
The setup window between writeIsolationOwner and isoStart left the base
dir holding only the marker file and no `m` mount, so classifyDir
returned null and scanWorktrees classified it as a stray — which a
non-`--all` clear removes, defeating the ownership guard mid-setup.

classifyDir now treats the presence of the ownership marker as a
task-isolation signal (in addition to the mount dir), so an in-progress
sandbox with a live owner is preserved throughout backend setup.

Fixes #6761
This commit is contained in:
roboomp
2026-07-27 03:47:20 +00:00
parent eeca809193
commit 91c0feaa87
2 changed files with 31 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/cli/worktree-cli.ts
+24 -13
View File
@@ -20,7 +20,7 @@ import * as fs from "node:fs/promises";
import * as path from "node:path";
import { getWorktreesDir, isEnoent } from "@oh-my-pi/pi-utils";
import chalk from "chalk";
import { hasLiveIsolationOwner } from "../task/isolation-ownership";
import { hasLiveIsolationOwner, ISOLATION_OWNER_FILE } from "../task/isolation-ownership";
import * as git from "../utils/git";
type WorktreeKind = "pr-checkout" | "task-isolation" | "empty" | "stray";
@@ -214,19 +214,30 @@ async function classifyDir(dir: string): Promise<WorktreeEntry | null> {
if (gitStat?.isFile()) {
return classifyPrCheckout(dir, gitEntry);
}
for (const mountDir of TASK_ISOLATION_MOUNT_DIRS) {
const mountStat = await fs.stat(path.join(dir, mountDir)).catch(() => null);
if (!mountStat?.isDirectory()) continue;
const live = await hasLiveIsolationOwner(dir);
return {
path: dir,
kind: "task-isolation",
// Only after confirming no live owner is the "no live task" claim true.
// A running subagent's sandbox stays live so `clear` won't delete it.
orphanReason: live ? undefined : "task-isolation leftover (no live task owns it)",
};
// A task-isolation sandbox is identified by its ownership marker — written
// before the backend materialises the mount — or by the `m`/`merged` mount
// dir itself (legacy dirs and crashed pre-marker runs). Recognizing the
// marker alone keeps an in-progress sandbox from being mistaken for a stray
// during the window between marker creation and mount materialisation.
let isIsolation = await Bun.file(path.join(dir, ISOLATION_OWNER_FILE)).exists();
if (!isIsolation) {
for (const mountDir of TASK_ISOLATION_MOUNT_DIRS) {
const mountStat = await fs.stat(path.join(dir, mountDir)).catch(() => null);
if (mountStat?.isDirectory()) {
isIsolation = true;
break;
}
}
}
return null;
if (!isIsolation) return null;
const live = await hasLiveIsolationOwner(dir);
return {
path: dir,
kind: "task-isolation",
// Only after confirming no live owner is the "no live task" claim true.
// A running subagent's sandbox stays live so `clear` won't delete it.
orphanReason: live ? undefined : "task-isolation leftover (no live task owns it)",
};
}
async function classifyPrCheckout(dir: string, gitEntry: string): Promise<WorktreeEntry> {
packages/coding-agent/test/cli/worktree-clear-isolation.test.ts
+7
View File
@@ -57,6 +57,12 @@ describe("worktree clear task-isolation ownership", () => {
const corrupt = await makeSandbox("tbad00004");
await Bun.write(path.join(corrupt, ISOLATION_OWNER_FILE), "{ not json");
// Setup race: marker written before the backend materialises `m`. The
// dir holds only the live-owner marker and no mount yet.
const pending = path.join(base, "tpend0005");
await fs.mkdir(pending, { recursive: true });
await writeIsolationOwner(pending, "pend0005");
await clearWorktrees({ all: false, dryRun: false, json: true });
const exists = async (p: string): Promise<boolean> =>
@@ -68,5 +74,6 @@ describe("worktree clear task-isolation ownership", () => {
expect(await exists(dead)).toBe(false);
expect(await exists(orphan)).toBe(false);
expect(await exists(corrupt)).toBe(false);
expect(await exists(pending)).toBe(true);
});
});