diff --git a/docs/superpowers/plans/2026-07-28-bg-non-git-isolation-split.md b/docs/superpowers/plans/2026-07-28-bg-non-git-isolation-split.md new file mode 100644 index 0000000..7c092aa --- /dev/null +++ b/docs/superpowers/plans/2026-07-28-bg-non-git-isolation-split.md @@ -0,0 +1,841 @@ +# Background dispatch isolation split (v0.11.1) — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Split `runBackground` into a git-agnostic core + a worktree wrapper + an `auto` router so background `subagent` dispatch stops 100%-failing in non-git cwds, and worktree failures surface as synchronous tool errors instead of async toasts. + +**Architecture:** `runBackgroundInPlace` (core: pool + journal + runLifecycle in `ctx.cwd`, no worktree) + `runBackgroundIsolated` (wrapper: sync `isGitRepo` pre-flight → worktree create → core with worktree extras → commit → remove) + `runBackgroundAuto` (router: isolated when git, in-place + per-session warn when not). The `subagent` tool + scheduler gain `isolation: "worktree" | "none" | "auto"` (default `auto`). The sync-fail returns `{ status: "failed", error }` (no runId) → the tool maps to `isError`. + +**Tech Stack:** TypeScript (raw `.ts` via tsx, no build step), node:test, typebox, `@earendil-works/pi-coding-agent` SDK. + +## Global Constraints + +- Raw `.ts` via tsx at runtime — **no build step**. +- `pnpm typecheck` + `pnpm test:run` (`--test-timeout=30000`) green before every commit. +- Commit prefix `fix(bg): …` / `test(bg): …`. No AI attribution. +- Branch: `fix/spec-bg-non-git`. Target release: `v0.11.1` (patch). +- Tests live in `test/` (not `src/`), named `*.test.mts`, run via `node --import tsx --test test/*.test.mts`. +- 2-space indent. Follow existing patterns (`makeRepo()` git fixtures, fake `runLifecycle`, `setTimeout(60)` for pool drain). +- The `runBackground` public name + the `RunBackgroundHandle` success type stay (back-compat for the existing test + any external ref); the return widens to a union `RunBackgroundResult`. + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `src/worktree/worktree-service.ts` | + `isGitRepo()` helper | +| `src/runtime/run-journal.ts` | `RunStartedEvent.worktree?` + `RunCompletedEvent.branch?` optional | +| `src/runtime/async-runner.ts` | the split: `runBackgroundInPlace` (core) + `runBackgroundIsolated` (wrapper) + `runBackgroundAuto` (router) + `runBackground` dispatcher; `RunLifecycleOpts.worktreePath?`/`branch?` optional; `RunBackgroundResult` union | +| `src/runtime/resume.ts` | `scanResumeCandidates` handles in-place interrupted runs (no `worktree` field → abort + notify); `ResumeCandidate.worktreePath?`/`branch?` optional | +| `src/tools/subagent.ts` | + `isolation` param; sync-fail → `isError`; scheduler.register threads `isolation` | +| `src/scheduling/scheduler.ts` | `ScheduleSpec.isolation?` | +| `src/index.ts` | `asyncRunLifecycle` adapter isolation-aware (conditional `artifactDiscovery` + `parentCwd`); scheduler `onFire` threads `isolation` | +| `test/worktree-service.test.mts` | `isGitRepo` true/false | +| `test/run-journal.test.mts` | optional `worktree`/`branch` round-trip | +| `test/async-runner.test.mts` | in-place / isolated / sync-fail / auto routing | +| `test/subagent-tool.test.mts` | `isolation` param routing + sync-fail `isError` | +| `test/scheduler.test.mts` | `isolation` threads through `onFire` | +| `test/resume.test.mts` | in-place interrupted run → abort | + +--- + +### Task 1: Foundation — `isGitRepo()` + optional journal/lifecycle fields + +**Files:** +- Modify: `src/worktree/worktree-service.ts:7,38` (add `isGitRepo`) +- Modify: `src/runtime/run-journal.ts:11,16` (optional fields) +- Modify: `src/runtime/async-runner.ts:28-32` (`RunLifecycleOpts` optional) +- Test: `test/worktree-service.test.mts` (extend) +- Test: `test/run-journal.test.mts` (extend) + +**Interfaces:** +- Produces: `WorktreeService.isGitRepo(dir?: string): boolean`; `RunStartedEvent.worktree?`; `RunCompletedEvent.branch?`; `RunLifecycleOpts.worktreePath?: string; branch?: string` — consumed by Tasks 2 + 5. + +- [ ] **Step 1: Write failing tests** + +Add to `test/worktree-service.test.mts` (after the last test, before EOF): + +```typescript +test("isGitRepo is true in a git repo, false in a plain dir", () => { + const repo = makeRepo(); + const plain = mkdtempSync(join(tmpdir(), "wt-nogit-")); + const svc = new WorktreeService({ rootDir: repo }); + assert.equal(svc.isGitRepo(), true); + const svcPlain = new WorktreeService({ rootDir: plain }); + assert.equal(svcPlain.isGitRepo(), false); + rmSync(repo, { recursive: true, force: true }); + rmSync(plain, { recursive: true, force: true }); +}); +``` + +Add to `test/run-journal.test.mts`: + +```typescript +test("run:started without worktree + run:completed without branch round-trip", () => { + const dir = makeDir(); + const j = new RunJournal(dir); + j.append("fl-ip1", { type: "run:started", runId: "fl-ip1", task: "t", lifecycle: "default", mode: "auto", ts: 1 }); + j.append("fl-ip1", { type: "run:completed", runId: "fl-ip1", ts: 2 }); + const events = j.replay("fl-ip1"); + assert.equal(events.length, 2); + const started = events[0] as any; + assert.equal(started.worktree, undefined); + const completed = events[1] as any; + assert.equal(completed.branch, undefined); + rmSync(dir, { recursive: true, force: true }); +}); + +test("old run:started with worktree still parses after the field becomes optional", () => { + const dir = makeDir(); + const j = new RunJournal(dir); + j.append("fl-old", { type: "run:started", runId: "fl-old", task: "t", lifecycle: "default", worktree: { path: "/x", branch: "fleet/fl-old" }, mode: "auto", ts: 1 }); + const events = j.replay("fl-old"); + assert.equal((events[0] as any).worktree.branch, "fleet/fl-old"); + rmSync(dir, { recursive: true, force: true }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pnpm test:run 2>&1 | grep -E "isGitRepo|without worktree|old run:started" | head` +Expected: 3 failures (`isGitRepo is not a function`; the optional-field tests may pass already since TS structurally allows omission — if they pass, that's fine, the impl change in Step 3 is what makes the *type* legal). Confirm `isGitRepo` test fails. + +- [ ] **Step 3: Implement** + +In `src/worktree/worktree-service.ts`, add after the `exists` method (before `create`): + +```typescript + /** v0.11.1: is `rootDir` (or `dir`) inside a git repo? Cheap sync pre-flight for isolation routing. */ + isGitRepo(dir: string = this.rootDir): boolean { + try { + sh("git rev-parse --show-toplevel", dir); + return true; + } catch { + return false; + } + } +``` + +In `src/runtime/run-journal.ts`, make the two fields optional: + +```typescript +export interface RunStartedEvent { type: "run:started"; runId: string; task: string; lifecycle: string; worktree?: { path: string; branch: string }; mode: "auto" | "checkpointed"; ts: number; } +export interface RunCompletedEvent { type: "run:completed"; runId: string; branch?: string; ts: number; } +``` + +In `src/runtime/async-runner.ts`, make `worktreePath`/`branch` optional: + +```typescript +export interface RunLifecycleOpts { + runId: string; + worktreePath?: string; + branch?: string; + mode: "auto" | "checkpointed"; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pnpm test:run 2>&1 | tail -8` +Expected: PASS — all 438 prior + 3 new = 441. No regressions (the adapter in index.ts still passes `worktreePath: opts.worktreePath` which is now `string | undefined` — assignable to the optional field; `run-lifecycle.ts` already guards `if (deps.artifactDiscovery && opts.worktreePath)`). + +- [ ] **Step 5: Commit** + +```bash +git add src/worktree/worktree-service.ts src/runtime/run-journal.ts src/runtime/async-runner.ts test/worktree-service.test.mts test/run-journal.test.mts +git commit -m "fix(bg): add isGitRepo + optional worktree/branch journal fields + +Foundation for the background isolation split. WorktreeService gains a +cheap isGitRepo() pre-flight. RunStartedEvent.worktree and +RunCompletedEvent.branch become optional (in-place runs have neither). +RunLifecycleOpts.worktreePath/branch become optional. Additive — old +events still parse; no behavior change." +``` + +--- + +### Task 2: The split — core + wrapper + router + adapter + +**Files:** +- Modify: `src/runtime/async-runner.ts` (full rewrite of `runBackground` + new fns) +- Modify: `src/index.ts:203-228` (`asyncRunLifecycle` adapter isolation-aware) +- Test: `test/async-runner.test.mts` (extend) + +**Interfaces:** +- Consumes: `WorktreeService.isGitRepo()` (Task 1), optional journal fields (Task 1) +- Produces: `runBackgroundInPlace(runId, task, opts, isolated?)` (core, fire-and-forget); `runBackgroundIsolated(task, opts): RunBackgroundResult` (sync pre-flight + worktree); `runBackgroundAuto(task, opts): RunBackgroundResult` (router); `runBackground(task, opts): RunBackgroundResult` (dispatcher, default `auto`); `RunBackgroundResult` union; `Isolation` type; `RunBackgroundOpts.isolation?`. + +- [ ] **Step 1: Write failing tests** + +Add to `test/async-runner.test.mts` (after existing tests). Note: the existing two tests use `makeRepo()` (git) and call `runBackground(...)` with no `isolation` → `auto` → isolated → current behavior preserved (regression guard). + +```typescript +test("runBackground isolation:'none' runs in-place in a NON-GIT dir, journals run:started with no worktree, completes, pushes result with no branch", async () => { + const plain = mkdtempSync(join(tmpdir(), "async-nogit-")); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + assert.equal(opts.worktreePath, undefined, "in-place run must not receive a worktreePath"); + writeFileSync(join(plain, "out.txt"), "done\n"); + return { + runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "implement", status: "completed", summary: "did it", paths: ["out.txt"], reviseCount: 0 }], + startedAt: 1, endedAt: 2, todoId: "td-x", + }; + }; + const { deps, journal, inbox, notifications } = makeDeps(plain, fakeLifecycle); + const handle = runBackground("research x", { deps, lifecycle: "default", mode: "auto", isolation: "none" }); + assert.equal(handle.status, "background"); + assert.ok("runId" in handle, "in-place handle has a runId"); + await new Promise((r) => setTimeout(r, 60)); + const events = journal.replay(handle.runId); + const started = events.find((e) => e.type === "run:started") as any; + assert.equal(started.worktree, undefined, "in-place run:started must omit worktree"); + assert.ok(events.some((e) => e.type === "run:completed"), "no run:completed"); + const completed = events.find((e) => e.type === "run:completed") as any; + assert.equal(completed.branch, undefined, "in-place run:completed must omit branch"); + assert.equal(inbox.readyCount(), 1); + assert.equal(inbox.pull()[0]!.branch, undefined, "in-place result has no branch"); + assert.ok(notifications.some((n) => /completed/.test(n)), `notifications: ${notifications.join("|")}`); + rmSync(plain, { recursive: true, force: true }); +}); + +test("runBackground isolation:'worktree' in a NON-GIT dir returns a SYNCHRONOUS failed result (no runId, no async toast, no 90s poll)", () => { + const plain = mkdtempSync(join(tmpdir(), "async-nogit2-")); + const fakeLifecycle: RunLifecycleFn = async () => ({ runId: "x", lifecycleName: "x", task: "x", backend: "pi", mode: "auto", status: "completed", phases: [], startedAt: 1, endedAt: 2, todoId: null }); + const { deps, notifications } = makeDeps(plain, fakeLifecycle); + const handle = runBackground("edit x", { deps, lifecycle: "default", mode: "auto", isolation: "worktree" }); + assert.equal(handle.status, "failed"); + assert.ok(!("runId" in handle), "sync-fail must NOT return a runId"); + assert.ok(/requires a git repo/.test((handle as any).error), `error: ${(handle as any).error}`); + // No run was started → no async toast fires for this runId + assert.equal(notifications.length, 0, "sync-fail must not emit an async notify"); + rmSync(plain, { recursive: true, force: true }); +}); + +test("runBackground default (auto) in a NON-GIT dir falls back to in-place + emits ONE per-session fallback notify", () => { + const plain = mkdtempSync(join(tmpdir(), "async-nogit3-")); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => ({ + runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "implement", status: "completed", summary: "s", paths: [], reviseCount: 0 }], + startedAt: 1, endedAt: 2, todoId: null, + }); + const { deps, notifications } = makeDeps(plain, fakeLifecycle); + // first auto-fallback run → 1 notify + const h1 = runBackground("a", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h1.status, "background"); + // second auto-fallback run → no additional notify (per-session dedup) + const h2 = runBackground("b", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h2.status, "background"); + const fallbackNotes = notifications.filter((n) => /in-place|worktree isolation/.test(n)); + assert.equal(fallbackNotes.length, 1, `expected exactly 1 fallback notify, got: ${notifications.join("|")}`); + rmSync(plain, { recursive: true, force: true }); +}); + +test("runBackground default (auto) in a GIT dir stays isolated (no fallback notify) — regression guard", async () => { + const repo = makeRepo(); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + assert.ok(opts.worktreePath, "git auto run must receive a worktreePath"); + writeFileSync(join(opts.worktreePath!, "d.md"), "# d\n"); + return { runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "brainstorm", status: "completed", summary: "s", paths: ["d.md"], reviseCount: 0 }], startedAt: 1, endedAt: 2, todoId: "t" }; + }; + const { deps, notifications } = makeDeps(repo, fakeLifecycle); + const h = runBackground("x", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h.status, "background"); + assert.ok("runId" in h); + await new Promise((r) => setTimeout(r, 60)); + assert.equal(notifications.filter((n) => /in-place|worktree isolation/.test(n)).length, 0, "git auto must not fallback-notify"); + rmSync(repo, { recursive: true, force: true }); +}); + +test("runBackground isolation:'none' in a GIT dir runs in-place (explicit opt-out, no notify)", async () => { + const repo = makeRepo(); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + assert.equal(opts.worktreePath, undefined, "explicit none must not receive a worktreePath"); + return { runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "implement", status: "completed", summary: "s", paths: [], reviseCount: 0 }], startedAt: 1, endedAt: 2, todoId: null }; + }; + const { deps, journal, notifications } = makeDeps(repo, fakeLifecycle); + const h = runBackground("ro", { deps, lifecycle: "default", mode: "auto", isolation: "none" }); + assert.equal(h.status, "background"); + await new Promise((r) => setTimeout(r, 60)); + const started = journal.replay(h.runId).find((e) => e.type === "run:started") as any; + assert.equal(started.worktree, undefined); + assert.equal(notifications.filter((n) => /in-place|worktree isolation/.test(n)).length, 0, "explicit none must not fallback-notify"); + rmSync(repo, { recursive: true, force: true }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pnpm test:run 2>&1 | grep -E "in-place|SYNCHRONOUS|fallback|regression|explicit none" | head` +Expected: 5 failures (the old `runBackground` always worktrees → the `none`/`auto`-in-non-git tests fail; the sync-fail test fails because the old fn returns `{runId, status:"background"}` not a failed union). + +- [ ] **Step 3: Implement — rewrite `src/runtime/async-runner.ts`** + +Replace the `RunBackgroundOpts`, `RunBackgroundHandle`, and `runBackground` block (from `export interface RunBackgroundOpts {` through the end of `runBackground`) with: + +```typescript +export interface RunBackgroundOpts { + deps: AsyncRunnerDeps; + lifecycle: string; + mode: "auto" | "checkpointed"; + /** v0.11.1: edit isolation for background runs. Default "auto" (worktree when cwd is a git repo, in-place otherwise). */ + isolation?: Isolation; +} + +/** The success shape (back-compat: existing external refs to RunBackgroundHandle still typecheck). */ +export interface RunBackgroundHandle { runId: string; status: "background"; } + +/** v0.11.1: a background dispatch either starts (runId + background) or fails synchronously (error, no runId). */ +export type RunBackgroundResult = + | { runId: string; status: "background" } + | { status: "failed"; error: string }; + +export type Isolation = "worktree" | "none" | "auto"; + +/** Per-session dedup flag for the auto-fallback in-place notify (resets on process restart = new session). */ +let inPlaceFallbackWarned = false; + +/** The git-agnostic core: pool slot → journal → runLifecycle in ctx.cwd (or a worktree, when `isolated` is set) → inbox + notify. + * Fire-and-forget. When `isolated` is present, journals the worktree field, commits on completion, and removes the worktree. */ +function runBackgroundInPlace(runId: string, task: string, opts: RunBackgroundOpts, isolated?: { worktreePath: string; branch: string }): void { + const { deps } = opts; + void deps.pool.withSlot(async () => { + try { + const ev0: JournalEvent = isolated + ? { type: "run:started", runId, task, lifecycle: opts.lifecycle, worktree: { path: isolated.worktreePath, branch: isolated.branch }, mode: opts.mode, ts: Date.now() } + : { type: "run:started", runId, task, lifecycle: opts.lifecycle, mode: opts.mode, ts: Date.now() }; + deps.journal.append(runId, ev0); + emitProgress(deps, runId, { status: "running", phase: "", phaseIndex: 0, phaseTotal: 0, lifecycle: opts.lifecycle, mode: opts.mode, task }); + + const res = await deps.runLifecycle(task, opts.lifecycle, { runId, worktreePath: isolated?.worktreePath, branch: isolated?.branch, mode: opts.mode }); + + if (res.status === "completed") { + if (isolated) { + try { sh("git add -A && git commit -m 'fleet run complete'", isolated.worktreePath); } catch { /* nothing to commit */ } + } + deps.journal.append(runId, isolated + ? { type: "run:completed", runId, branch: isolated.branch, ts: Date.now() } + : { type: "run:completed", runId, ts: Date.now() }); + const total = res.phases.length; + const lastIdx = total; + emitProgress(deps, runId, { status: "completed", phase: res.phases[total - 1]?.name ?? "finish", phaseIndex: lastIdx, phaseTotal: total, lifecycle: opts.lifecycle, mode: opts.mode, task, ...(isolated ? { branch: isolated.branch } : {}) }); + const lastPhase = res.phases[res.phases.length - 1]; + const result: RunResult = { + runId, task, status: "completed", + summary: lastPhase?.summary ?? "", + paths: res.phases.flatMap((p) => p.paths), + completedAt: Date.now(), + ...(isolated ? { branch: isolated.branch } : {}), + }; + deps.inbox.push(result); + deps.notify(`fleet run ${runId} completed`, "info"); + if (isolated) deps.worktree.removeWorktree(runId); + } else { + deps.journal.append(runId, { type: "run:aborted", runId, reason: res.error ?? res.status, ts: Date.now() }); + if (isolated) deps.worktree.remove(runId); + emitProgress(deps, runId, { status: "failed", phase: "", phaseIndex: 0, phaseTotal: res.phases.length, lifecycle: opts.lifecycle, mode: opts.mode, task }); + deps.notify(`fleet run ${runId} ${res.status}: ${res.error ?? ""}`, "warning"); + } + } catch (e) { + const msg = (e as Error).message; + deps.journal.append(runId, { type: "run:aborted", runId, reason: msg, ts: Date.now() }); + if (isolated) deps.worktree.remove(runId); + deps.notify(`fleet run ${runId} failed: ${msg}`, "error"); + emitProgress(deps, runId, { status: "failed", phase: "", phaseIndex: 0, phaseTotal: 0, lifecycle: opts.lifecycle, mode: opts.mode, task }); + } + }); +} + +/** The worktree wrapper: SYNCHRONOUS pre-flight + worktree create, then core-with-isolation. */ +function runBackgroundIsolated(task: string, opts: RunBackgroundOpts): RunBackgroundResult { + const { deps } = opts; + if (!deps.worktree.isGitRepo()) { + return { status: "failed", error: "isolation: 'worktree' requires a git repo; cwd is not one — use isolation: 'none' or run in a git repo" }; + } + const runId = deps.genRunId(); + const baseRef = "HEAD"; + let wt: { path: string; branch: string }; + try { + wt = deps.worktree.create(runId, baseRef); + } catch (e) { + return { status: "failed", error: (e as Error).message }; + } + runBackgroundInPlace(runId, task, opts, { worktreePath: wt.path, branch: wt.branch }); + return { runId, status: "background" }; +} + +/** The auto router: isolated when cwd is a git repo, in-place + one per-session notify when not. */ +function runBackgroundAuto(task: string, opts: RunBackgroundOpts): RunBackgroundResult { + const { deps } = opts; + if (deps.worktree.isGitRepo()) { + return runBackgroundIsolated(task, opts); + } + if (!inPlaceFallbackWarned) { + inPlaceFallbackWarned = true; + deps.notify("background run in-place (no worktree isolation — parallel edits may conflict)", "warning"); + } + const runId = deps.genRunId(); + runBackgroundInPlace(runId, task, opts); + return { runId, status: "background" }; +} + +/** Public dispatcher (keeps the `runBackground` name + RunBackgroundHandle success shape for back-compat). */ +export function runBackground(task: string, opts: RunBackgroundOpts): RunBackgroundResult { + const isolation = opts.isolation ?? "auto"; + if (isolation === "worktree") return runBackgroundIsolated(task, opts); + if (isolation === "none") { + const runId = opts.deps.genRunId(); + runBackgroundInPlace(runId, task, opts); + return { runId, status: "background" }; + } + return runBackgroundAuto(task, opts); +} +``` + +- [ ] **Step 4: Update the index.ts adapter to be isolation-aware** + +In `src/index.ts`, replace the `asyncRunLifecycle` adapter body (the `lifecycleFullDeps` construction + the `runLifecycle` call). Change `artifactDiscovery` to be conditional on `opts.worktreePath`, and `parentCwd` to fall back to `deps.parentCwd`: + +```typescript + const asyncRunLifecycle: AsyncRunnerDeps["runLifecycle"] = async (task, lifecycleName, opts) => { + const { runLifecycle } = await import("./lifecycle/run-lifecycle.ts"); + const { spawnSubagent } = await import("./engine/spawnSubagent.ts"); + const bgLock = createSingleSlotLock(); + // v0.11.1: isolated runs use worktree-diff artifact discovery + the worktree as spawn cwd; + // in-place runs (worktreePath undefined) use the prompt-baked parser + the session cwd. + const isolated = !!opts.worktreePath; + const lifecycleFullDeps: LifecycleRunDeps = { + ...deps.lifecycleDeps, + genRunId: () => opts.runId, + ...(isolated ? { artifactDiscovery: ({ finalText, cwd, baseRef }: { finalText: string; cwd: string; baseRef: string }) => (deps.asyncRunner as AsyncRunnerDeps).diff.diffPhase(cwd, baseRef, finalText) } : {}), + spawn: async (o) => spawnSubagent({ + agent: o.agent, task: o.task, lifecycleTodoId: o.lifecycleTodoId, model: o.model, + skillsOverride: o.skills, backendOverride: o.backend, + registry: deps.registry, todoSync: deps.todoSync, runRegistry: deps.runRegistry, lock: bgLock, + backendRegistry: deps.backendRegistry, parentModel: deps.parentModel, + parentCwd: isolated ? opts.worktreePath! : deps.parentCwd, + runLog: deps.runLog, tierRegistry: deps.tierRegistry, modelRegistry: deps.modelRegistry, + }), + }; + const res = await runLifecycle(task, lifecycleName, { deps: lifecycleFullDeps, mode: opts.mode, worktreePath: opts.worktreePath, baseRef: "HEAD", onCheckpoint: async (p) => p.status === "failed" ? { action: "abort" } : { action: "continue" } }); + return res as unknown as import("./runtime/async-runner.ts").FakeLifecycleResult; + }; +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `pnpm test:run 2>&1 | tail -8` +Expected: PASS — 441 (Task 1) + 5 new = 446. The two existing async-runner tests still pass (git + auto → isolated = current behavior). `pnpm typecheck` clean. + +Run: `pnpm typecheck 2>&1 | tail -3` +Expected: no output (clean). + +- [ ] **Step 6: Commit** + +```bash +git add src/runtime/async-runner.ts src/index.ts test/async-runner.test.mts +git commit -m "fix(bg): split runBackground into git-agnostic core + worktree wrapper + auto router + +runBackgroundInPlace (core) is git-agnostic: pool + journal + runLifecycle +in ctx.cwd, no worktree. runBackgroundIsolated (wrapper) does a SYNCHRONOUS +isGitRepo pre-flight + worktree.create before returning, so worktree failures +surface as { status: 'failed', error } (no runId) — the tool maps to isError, +not an async toast + 90s poll. runBackgroundAuto routes: isolated when cwd is +git (current behavior preserved), in-place + one per-session notify when not. +The index.ts adapter becomes isolation-aware (conditional artifactDiscovery + +parentCwd). Closes the 100% bg failure in non-git cwds." +``` + +--- + +### Task 3: `subagent` tool `isolation` param + sync-fail → `isError` + +**Files:** +- Modify: `src/tools/subagent.ts:24` (add `isolation` param), `:87-90` (background branch handles the union) +- Test: `test/subagent-tool.test.mts` (extend) + +**Interfaces:** +- Consumes: `runBackground` returns `RunBackgroundResult` (Task 2) +- Produces: `subagent` tool accepts `isolation: "worktree" | "none" | "auto"` (default `auto`); background branch returns `isError` on sync-fail. + +- [ ] **Step 1: Write failing tests** + +First inspect the existing `test/subagent-tool.test.mts` harness to match its fake-deps shape: + +```bash +sed -n '1,90p' test/subagent-tool.test.mts +``` + +The existing test file defines a `makeDeps()` helper (lines ~38-56) returning the base deps object. The new tests reuse it via `{ ...makeDeps(), parentCwd: plain, asyncRunner: fakeAsyncRunner }`. Add after the existing tests: + +```typescript +test("subagent background with isolation:'worktree' in a non-git cwd returns isError synchronously", async () => { + const plain = mkdtempSync(join(tmpdir(), "sub-nogit-")); + const fakeAsyncRunner = { + worktree: { isGitRepo: () => false, create: () => { throw new Error("no"); }, removeWorktree: () => {}, remove: () => {}, exists: () => false, branchFor: () => "fleet/x", pathFor: () => plain }, + diff: {}, journal: { append: () => {}, replay: () => [], scanNonTerminal: () => [] }, + pool: { withSlot: async () => {} }, inbox: { push: () => {}, readyCount: () => 0, pull: () => [], renderHint: () => "" }, + runLifecycle: async () => ({ status: "completed", phases: [] } as any), + notify: () => {}, genRunId: () => "fl-x", + } as any; + const tool = createSubagentTool({ ...makeDeps(), parentCwd: plain, asyncRunner: fakeAsyncRunner } as any); + const res = await tool.execute!("id", { agent: "g", task: "x", background: true, isolation: "worktree" } as any, new AbortController().signal, () => {}, {} as any); + ok(res.isError === true, `expected isError, got: ${(res as any).isError}`); + ok(/requires a git repo/.test((res.content as any)[0].text), `text: ${(res.content as any)[0].text}`); + rmSync(plain, { recursive: true, force: true }); +}); + +test("subagent background default (auto) in a non-git cwd returns a background run (in-place)", async () => { + const plain = mkdtempSync(join(tmpdir(), "sub-nogit2-")); + const fakeAsyncRunner = { + worktree: { isGitRepo: () => false, create: () => { throw new Error("no"); }, removeWorktree: () => {}, remove: () => {}, exists: () => false, branchFor: () => "fleet/x", pathFor: () => plain }, + diff: {}, journal: { append: () => {}, replay: () => [], scanNonTerminal: () => [] }, + pool: { withSlot: async () => {} }, inbox: { push: () => {}, readyCount: () => 0, pull: () => [], renderHint: () => "" }, + runLifecycle: async () => ({ status: "completed", phases: [] } as any), + notify: () => {}, genRunId: () => "fl-auto", + } as any; + const tool = createSubagentTool({ ...makeDeps(), parentCwd: plain, asyncRunner: fakeAsyncRunner } as any); + const res = await tool.execute!("id", { agent: "g", task: "x", background: true } as any, new AbortController().signal, () => {}, {} as any); + ok(res.isError === undefined, `expected no isError, got: ${(res as any).isError}`); + ok(/background run:/.test((res.content as any)[0].text), `text: ${(res.content as any)[0].text}`); + rmSync(plain, { recursive: true, force: true }); +}); +``` + +(The `execute!` non-null assertion matches the existing test's pattern at line ~68; the `() => {}` onProgress + `{} as any` ctx match the existing call signature.) + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pnpm test:run 2>&1 | grep -E "isError synchronously|in-place" | head` +Expected: 2 failures (`isolation` not a known param; the tool returns `background run: undefined` on the worktree-in-non-git path because the old code ignored the failed union). + +- [ ] **Step 3: Implement** + +In `src/tools/subagent.ts`, add the `isolation` param to `subagentParams` (after `background`): + +```typescript + isolation: Type.Optional(Type.Union([ + Type.Literal("worktree"), + Type.Literal("none"), + Type.Literal("auto"), + ], { description: "Edit isolation for background runs. 'worktree' = git worktree (requires a git repo; fails sync if not). 'none' = in-place in cwd (no isolation; parallel edits may conflict). 'auto' (default) = worktree when cwd is a git repo, in-place otherwise." })), +``` + +Update the background branch (replace the existing `if (params.background) { ... }` block): + +```typescript + if (params.background) { + if (!deps.asyncRunner) return { isError: true, content: [{ type: "text" as const, text: "background runs not configured (asyncRunner missing)" }] }; + const handle = runBackground(params.task, { deps: deps.asyncRunner, lifecycle: params.lifecycle ?? "default", mode: "auto", isolation: params.isolation }); + if (handle.status === "failed") { + return { isError: true, content: [{ type: "text" as const, text: handle.error }] }; + } + return { content: [{ type: "text" as const, text: `background run: ${handle.runId}` }], details: handle }; + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pnpm test:run 2>&1 | tail -8` +Expected: PASS — 446 + 2 = 448. `pnpm typecheck` clean. + +- [ ] **Step 5: Commit** + +```bash +git add src/tools/subagent.ts test/subagent-tool.test.mts +git commit -m "fix(bg): subagent tool gains isolation param; sync-fail returns isError + +The subagent tool accepts isolation: worktree|none|auto (default auto). +On a synchronous worktree failure (e.g. 'worktree' in a non-git cwd) the +tool now returns isError with the actionable message, instead of +'background run: undefined' + an async toast the model never sees." +``` + +--- + +### Task 4: Scheduler `isolation` plumbing + `onFire` threading + +**Files:** +- Modify: `src/scheduling/scheduler.ts:11-16` (`ScheduleSpec.isolation?`) +- Modify: `src/index.ts:298-300` (`onFire` threads `isolation`) +- Modify: `src/tools/subagent.ts:83-86` (schedule branch threads `isolation`) +- Test: `test/scheduler.test.mts` (extend — check the file exists first; if not, create a minimal harness) + +**Interfaces:** +- Consumes: `Isolation` type (Task 2) +- Produces: `ScheduleSpec.isolation?: Isolation`; `onFire` receives it; `scheduler.register` accepts it. + +- [ ] **Step 1: Inspect the scheduler test harness** + +Run: `ls test/scheduler*.test.mts && sed -n '1,60p' test/scheduler.test.mts` +If `test/scheduler.test.mts` exists, extend it; if not, create it (model on the `resume.test.mts` pattern: a `ScheduleSpec` registered with a fake `onFire` capturing the spec, then assert `spec.isolation` threads through). + +- [ ] **Step 2: Write failing test** + +Add to `test/scheduler.test.mts` (create if absent — use a temp `storePath`/`lockPath`): + +```typescript +test("scheduler.register stores + onFire receives the isolation field", () => { + const tmp = mkdtempSync(join(tmpdir(), "sched-iso-")); + const storePath = join(tmp, "schedules.json"); + const lockPath = join(tmp, "schedules.lock"); + let fired: any = null; + const sch = new Scheduler({ storePath, lockPath, onFire: (spec) => { fired = spec; } }); + const id = sch.register({ task: "t", expression: "5m", lifecycle: "default", auto: true, isolation: "worktree" }); + const stored = sch.list().find((s) => s.id === id); + assert.equal(stored?.isolation, "worktree"); + // onFire fires on the schedule; for the test, call the internal fire directly via the public + // surface by pausing + resuming isn't deterministic — instead assert the stored spec round-trips + // the isolation field through persist+load: + sch.pause(id); + sch.resume(id); + assert.equal(stored?.isolation, "worktree"); + rmSync(tmp, { recursive: true, force: true }); +}); +``` + +(If `Scheduler` lacks `pause`/`resume`/`list` per the earlier read it has `list`; adapt to the actual API surfaced in `scheduler.ts` — read `sed -n '60,120p' src/scheduling/scheduler.ts` for the exact method names before writing the assertion.) + +- [ ] **Step 3: Run test to verify it fails** + +Run: `pnpm test:run 2>&1 | grep -E "isolation field" | head` +Expected: failure (`isolation` not on `ScheduleSpec`; the stored spec has `isolation === undefined`). + +- [ ] **Step 4: Implement** + +In `src/scheduling/scheduler.ts`, add `isolation` to `ScheduleSpec`: + +```typescript +export interface ScheduleSpec { + task: string; + expression: string; + lifecycle?: string; + auto?: boolean; + /** v0.11.1: edit isolation for the background run on fire. Default "auto". */ + isolation?: "worktree" | "none" | "auto"; +} +``` + +(`StoredSchedule extends ScheduleSpec` inherits it; the `persist`/`load` JSON round-trip carries it automatically since it spreads `spec`.) + +In `src/index.ts`, update the scheduler `onFire` (around line 298-300): + +```typescript + onFire: (spec) => { + if (!deps.asyncRunner) return; + runBackground(spec.task, { deps: deps.asyncRunner, lifecycle: spec.lifecycle ?? "default", mode: spec.auto ? "auto" : "checkpointed", isolation: spec.isolation }); + }, +``` + +In `src/tools/subagent.ts`, update the schedule branch (the `if (params.schedule)` block) to thread `isolation`: + +```typescript + const id = deps.scheduler.register({ task: params.task, expression: params.schedule, lifecycle: params.lifecycle ?? "default", auto: params.auto ?? true, isolation: params.isolation }); +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `pnpm test:run 2>&1 | tail -8` +Expected: PASS — 448 + 1 = 449. `pnpm typecheck` clean. + +- [ ] **Step 6: Commit** + +```bash +git add src/scheduling/scheduler.ts src/index.ts src/tools/subagent.ts test/scheduler.test.mts +git commit -m "fix(bg): thread isolation through the scheduler + onFire + +Scheduled runs are background runs; they inherit the isolation opt-in. +ScheduleSpec gains optional isolation (default auto). The onFire callback ++ subagent tool's schedule branch thread it through to runBackground." +``` + +--- + +### Task 5: `scanResumeCandidates` handles in-place interrupted runs + +**Files:** +- Modify: `src/runtime/resume.ts` (full rewrite of `scanResumeCandidates` + `ResumeCandidate` optional fields) +- Test: `test/resume.test.mts` (extend) + +**Interfaces:** +- Consumes: `RunStartedEvent.worktree?` optional (Task 1) +- Produces: `ResumeCandidate.worktreePath?`/`branch?` optional; in-place interrupted runs → `canResume: false` + `run:aborted` written. + +- [ ] **Step 1: Write failing test** + +Add to `test/resume.test.mts`: + +```typescript +test("scanResumeCandidates aborts an interrupted IN-PLACE run (no worktree field) with canResume=false", () => { + const repo = makeRepo(); // repo only for the WorktreeService ctor; the run is in-place + const runsDir = join(repo, ".pi", "fleet", "runs"); + const journal = new RunJournal(runsDir); + const wt = new WorktreeService({ rootDir: repo }); + // an in-place run: run:started with NO worktree field, no terminal event + journal.append("fl-ip-int", { type: "run:started", runId: "fl-ip-int", task: "t", lifecycle: "default", mode: "auto", ts: 1 }); + journal.append("fl-ip-int", { type: "phase:completed", phase: "implement", summary: "s", paths: ["x.ts"], ts: 2 }); + const cands = scanResumeCandidates(repo, { runsDir, worktree: wt }); + assert.equal(cands.length, 1); + assert.equal(cands[0]!.runId, "fl-ip-int"); + assert.equal(cands[0]!.canResume, false); + assert.equal(cands[0]!.worktreePath, undefined); + assert.equal(cands[0]!.branch, undefined); + const events = journal.replay("fl-ip-int"); + assert.equal(events[events.length - 1]!.type, "run:aborted"); + rmSync(repo, { recursive: true, force: true }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test:run 2>&1 | grep -E "IN-PLACE run" | head` +Expected: failure — the current `scanResumeCandidates` reads `started.worktree.path` (undefined for in-place) → throws `Cannot read properties of undefined (reading 'path')`, or pushes a candidate with `worktreePath: undefined` and `canResume` based on `wtExists` (false → writes `run:aborted` but `worktreePath` is undefined, not the new behavior). Confirm the failure mode, then implement. + +- [ ] **Step 3: Implement — rewrite `src/runtime/resume.ts`** + +Replace the `ResumeCandidate` interface + `scanResumeCandidates` body: + +```typescript +export interface ResumeCandidate { + runId: string; + task: string; + lifecycle: string; + /** Present only for isolated (worktree) runs. */ + worktreePath?: string; + /** Present only for isolated (worktree) runs. */ + branch?: string; + lastPhase: string | null; + canResume: boolean; +} + +export interface ScanResumeOpts { + runsDir: string; + worktree: WorktreeService; +} + +export function scanResumeCandidates(_projectDir: string, opts: ScanResumeOpts): ResumeCandidate[] { + const journal = new RunJournal(opts.runsDir); + const ids = journal.scanNonTerminal(); + const cands: ResumeCandidate[] = []; + for (const runId of ids) { + const events = journal.replay(runId); + const started = events.find((e) => e.type === "run:started") as + | (JournalEvent & { type: "run:started"; worktree?: { path: string; branch: string } }) | undefined; + if (!started) continue; + const phaseEvents = events.filter((e) => e.type === "phase:completed" || e.type === "phase:started" || e.type === "phase:failed") as Array<{ phase: string }>; + const lastPhase = phaseEvents.length > 0 ? phaseEvents[phaseEvents.length - 1]!.phase : null; + + if (started.worktree) { + // isolated run: resume iff the worktree still exists + const wtExists = opts.worktree.exists(runId); + if (!wtExists) { + journal.append(runId, { type: "run:aborted", runId, reason: "worktree-missing", ts: Date.now() }); + } + cands.push({ + runId, task: started.task, lifecycle: started.lifecycle, + worktreePath: started.worktree.path, branch: started.worktree.branch, + lastPhase, canResume: wtExists, + }); + } else { + // v0.11.1: in-place interrupted run — no worktree to clean; abort (partial edits may remain in cwd). + journal.append(runId, { type: "run:aborted", runId, reason: "in-place interrupted (partial edits may remain in cwd)", ts: Date.now() }); + cands.push({ runId, task: started.task, lifecycle: started.lifecycle, lastPhase, canResume: false }); + } + } + return cands; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pnpm test:run 2>&1 | tail -8` +Expected: PASS — 449 + 1 = 450. The 3 existing resume tests still pass (they all use `worktree: {path, branch}` on run:started → the isolated branch). `pnpm typecheck` clean. + +- [ ] **Step 5: Commit** + +```bash +git add src/runtime/resume.ts test/resume.test.mts +git commit -m "fix(bg): scanResumeCandidates handles in-place interrupted runs + +An interrupted in-place run (run:started with no worktree field) has no +worktree to clean; abort it + mark canResume=false with an honest reason +(partial edits may remain in cwd). ResumeCandidate.worktreePath/branch +become optional. Isolated-run handling unchanged." +``` + +--- + +### Task 6: Release v0.11.1 + +**Files:** none (release ops) + +- [ ] **Step 1: Full green suite** + +Run: `pnpm typecheck && pnpm test:run 2>&1 | tail -8` +Expected: typecheck clean; 450 pass / 0 fail. + +- [ ] **Step 2: Branch + PR** + +```bash +git checkout -b fix/spec-bg-non-git +# (the spec f2c80ec is on main already; the code commits are on main too per the 6-2 cadence — +# actually: create the branch FROM the last code commit, so the PR contains only the code, not the spec. +# If the code commits landed on main, the branch is just for the PR marker.) +git push -u origin fix/spec-bg-non-git +gh pr create --title "fix(bg): v0.11.1 background dispatch isolation split (non-git cwd fix)" --body "Proper-fix for the background dispatch 100%-failure in non-git cwds. Splits runBackground into a git-agnostic core + worktree wrapper + auto router. Adds isolation: worktree|none|auto (default auto). Synchronous fail-fast on worktree failures. See docs/superpowers/specs/2026-07-28-bg-non-git-isolation-split-design.md." --base main +``` + +- [ ] **Step 3: Merge + tag** + +```bash +gh pr merge --merge --delete-branch +# tag (use update-ref, NOT git tag -a which opens Vim): +git update-ref refs/tags/v0.11.1 refs/heads/main +git push --force origin v0.11.1 +``` + +- [ ] **Step 4: Verify CI publish** + +Run: `gh run list --limit 3` then watch the Release workflow → expect green (~50s) + npm `@getpipher/armory-fleet@0.11.1` published + GitHub Release v0.11.1 created. + +- [ ] **Step 5: Bump settings.json** + +Edit `~/.pi/agent/settings.json` → armory-fleet version `0.11.1`. Sync dotfiles: +```bash +cd ~/dotfiles && git add pi/agent/settings.json && git commit -m "chore: bump armory-fleet to 0.11.1" && git push +``` + +- [ ] **Step 6: Term-smoke on published (the bug repro)** + +Spawn a tmux session, run `pi` in `~/local-dev/bug-bounty` (non-git), fire a background subagent: +``` +subagent({ agent: "general-purpose", task: "list files in this folder", background: true }) +``` +Expected: a working in-place background run (appears in `/fleet`, completes, result in `fleet_results`) + ONE "background run in-place (no worktree isolation…)" notify — NOT the 100%-fail. Then test `isolation: "worktree"` there → expect a synchronous `isError: "…requires a git repo…"` (no 90s poll). + +- [ ] **Step 7: Update handoff pointer** + +Update `~/.pi/agent/memory/-Users-rector-local-dev-getpipher-armory-fleet/handoff-pointer.md` with v0.11.1 state (main ref, 450 tests, the isolation seam, SPEC-6-3 next). + +--- + +## Self-Review + +**1. Spec coverage:** +- §3 split (core+wrapper+router) → Task 2 ✓ +- §3 `isGitRepo()` → Task 1 ✓ +- §3 optional journal fields → Task 1 ✓ +- §3 `scanResumeCandidates` in-place handling → Task 5 ✓ +- §3 `subagent` tool `isolation` param → Task 3 ✓ +- §3 scheduler plumbing → Task 4 ✓ +- §4 edge cases: `auto` in git (Task 2 regression test), `auto` in non-git (Task 2), `worktree` in non-git sync fail (Task 2+3), `none` in git (Task 2), interrupted in-place (Task 5), old journal events (Task 1), scheduled run (Task 4) — all covered ✓ +- §6 release (v0.11.1, branch, tag, smoke) → Task 6 ✓ +- §7 testing (TDD, coverage) → every task TDD ✓ + +**2. Placeholder scan:** The two `` markers in Task 3 are explicitly flagged for the implementer to copy from the existing harness — this is a deliberate pointer to real code (the existing test's deps), not a vague TBD. All other steps have complete code. No "TBD"/"implement later"/"handle edge cases". + +**3. Type consistency:** `Isolation` defined Task 2, used Task 3 (`params.isolation`), Task 4 (`ScheduleSpec.isolation`, `onFire`), consistent. `RunBackgroundResult` union defined Task 2, consumed Task 3 (`handle.status === "failed"`). `ResumeCandidate.worktreePath?`/`branch?` defined Task 5, consistent with `RunStartedEvent.worktree?` Task 1. `runBackgroundInPlace`/`runBackgroundIsolated`/`runBackgroundAuto`/`runBackground` names consistent across Tasks 2-4. + +No issues found inline. Plan complete. \ No newline at end of file diff --git a/docs/superpowers/specs/2026-07-28-bg-non-git-isolation-split-design.md b/docs/superpowers/specs/2026-07-28-bg-non-git-isolation-split-design.md new file mode 100644 index 0000000..ba2d3cb --- /dev/null +++ b/docs/superpowers/specs/2026-07-28-bg-non-git-isolation-split-design.md @@ -0,0 +1,194 @@ +# v0.11.1 — Background dispatch isolation split (non-git cwd fix) + +**Date:** 2026-07-28 +**Type:** Patch / bugfix (not a roadmap SPEC) +**Package:** `@getpipher/armory-fleet` · target release `0.11.1` (patch) +**Predecessor:** v0.11.0 (SPEC-6-2 — quality gates + lifecycle hooks; `main` `cda5e2b`, 438/438 tests) +**Pipeline step:** brainstorm (this doc) → plan → implementation + +## 1. Context — the bug + +### Symptom +Firing background `subagent` runs (`background: true`) in a non-git cwd (e.g. `~/local-dev/bug-bounty`, a bucket folder with no `.git`) produces **100% failure**. Seven parallel research dispatches all die with: + +``` +fleet run fl-ms4tmqq0-… failed: worktree create failed for run fl-ms4tmqq0-… (base HEAD): + fatal: not a git repository (or any of the parent directories): .git +``` + +The model, having received `background run: fl-…` for each (looks successful), then polls `fleet_results` for ~90s waiting for runs that already died. Reproduced in pi session `019fa956-96c9-7bb2-a98a-bccaf2783d67` (`~/local-dev/bug-bounty`). + +### Root cause — two compounding problems + +**1. Worktree isolation is conflated with background execution.** `runBackground()` (`src/runtime/async-runner.ts:84`) **unconditionally** calls `WorktreeService.create()` → `git worktree add`, which requires a git repo. SPEC-5a built `runBackground` as an "isolated-edit-background-runner" (worktree create → `runLifecycle` with `worktreePath` → diff-service artifact discovery → `git commit` on completion → remove worktree). That whole shape assumes an *editing* run on an *isolated branch* — so it breaks in non-git cwds. But background execution and worktree isolation are **orthogonal concerns**: +- **Background execution** = fire-and-forget + pool-gated concurrency + journal + inbox + notify. Git-agnostic. +- **Worktree isolation** = isolated edit surface for parallel edits + auto-commit-to-branch. Requires git. An *editing* concern. + +A read-only research agent in a non-git bucket should background fine; worktree is overhead it doesn't need and a hard dependency it can't satisfy. Foreground `subagent` (no `background`) already runs in-place in `ctx.cwd` with no worktree (`spawnSubagent.ts` has zero worktree refs) — so the asymmetry is **background-only**. + +**2. The failure is async and invisible to the model.** `runBackground` returns `{ runId, status: "background" }` **synchronously**, *before* worktree creation runs (worktree creation is inside `void deps.pool.withSlot(async () => …)` — fire-and-forget). The tool returns `background run: fl-…` (looks successful), then the pool's async block catches the git error and surfaces it only via `ctx.ui.notify(...)` — a **TUI toast the model never sees as a tool result**. From the model's view, all dispatches "succeeded"; it waits for results that will never come. This is a bug **independent of the non-git case** — any worktree-creation failure surfaces as an async toast, not a tool error. + +### What this patch closes +- The non-git 100%-failure (the headline). +- The async-toast-invisible-to-the-model failure pattern (synchronous fail-fast on worktree failures). +- The architectural conflation (worktree stops being a mandatory property of "background"; becomes an opt-in mode) — which is the seam SPEC-6-3's `agent()` `isolation` opt-in will inherit. + +## 2. Design decisions (settled in brainstorm) + +| # | Decision | Choice | +|---|---|---| +| 1 | Fix model | **Proper architectural split** — separate background-execution (git-agnostic) from worktree-isolation (requires git) as two functions, not a flag on one. Same LOC, honest boundaries; the 6-3 `agent()` `isolation` seam falls out for free. | +| 2 | `isolation` default | **`auto`** — isolated when `ctx.cwd` is a git repo (current behavior preserved for editing lifecycles), in-place when not (enables the non-git case). Existing model calls in git repos are unchanged. | +| 3 | Interrupted in-place run on restart | **abort + notify** — consistent with how `reconcile`/`scanResumeCandidates` already aborts interrupted *isolated* runs (worktree-missing → abort). In-place runs have no worktree to clean; partial edits may remain in the cwd (honest). | +| 4 | `auto`-fallback warning cadence | **per-session dedup** — first `auto`→in-place fallback in a session notifies "background run in-place (no worktree isolation — parallel edits may conflict)"; subsequent are silent. 7 parallel runs → 1 toast. | +| 5 | `auto` pre-flight mechanism | **`WorktreeService.isGitRepo()`** helper (`git rev-parse --show-toplevel` succeeds) — one cheap sync call before returning the runId; gates `auto` routing + the `"worktree"` synchronous fail. | +| 6 | Scheduler | scheduled runs gain optional `isolation` (default `auto`); same plumbing as the tool. | +| 7 | Release | patch v0.11.1, branch `fix/spec-bg-non-git`, commit prefix `fix(bg): …`. | + +## 3. Architecture — the split + +### `src/runtime/async-runner.ts` (the bulk of the change) + +Two functions where there was one: + +``` +runBackgroundInPlace(task, opts) # the git-agnostic core + pool.withSlot: + journal run:started (NO worktree field) + runLifecycle(task, lifecycle, { mode, worktreePath: undefined }) # in ctx.cwd + on completed: inbox.push(result with NO branch); notify; journal run:completed (NO branch) + on failed/aborted: journal run:aborted; notify; emitProgress failed + +runBackgroundIsolated(task, opts) # the worktree wrapper + # SYNCHRONOUS pre-flight (before returning any handle): + if !worktree.isGitRepo(cwd): + return { status: "failed", error: "isolation: 'worktree' requires a git repo; cwd '…' is not one — use isolation: 'none' or run in a git repo" } # NO runId — tool maps to isError + runId = deps.genRunId() # generate only after pre-flight passes + wt = worktree.create(runId, baseRef) # synchronous; failure here → same failed shape + void runBackgroundInPlace(runId, task, opts WITH { worktreePath: wt.path, artifactDiscovery: diff-service, onCompleted: commit+remove }) + return { runId, status: "background" } +``` + +**Return contract:** the router returns a union — `{ runId, status: "background" }` (success; the run is now fire-and-forget in the pool) **or** `{ status: "failed", error }` (synchronous pre-flight/worktree failure; **no runId**). The `subagent` tool maps the latter to `{ isError: true, content: [{ text: error }] }` so the model receives an actionable tool result, not an async toast. `runBackgroundInPlace` (the core) always succeeds at dispatch (no git dependency) so it stays fire-and-forget and returns `{ runId, status: "background" }`. + +The wrapper threads three things into the core: `worktreePath` (for `runLifecycle`'s isolated artifact discovery), `artifactDiscovery` (the diff-service), and an `onCompleted` hook (`git commit` the worktree, then `removeWorktree`). The core stays unchanged in shape; the wrapper adds the isolation lifecycle around it. + +**Synchronous fail-fast** is the key correctness property: `runBackgroundIsolated` does its pre-flight + worktree creation **before** returning, so a worktree failure is a tool `isError`, not an async toast. `runBackgroundInPlace` can't fail at dispatch (no git dependency) so it stays fire-and-forget. + +**The `auto` router** (one sync `isGitRepo()` call) picks the function. The `subagent` tool calls the router; the scheduler's `onFire` calls the router. + +### Files touched + +| File | Change | +|---|---| +| `src/runtime/async-runner.ts` | split `runBackground` → `runBackgroundInPlace` (core) + `runBackgroundIsolated` (wrapper) + `runBackgroundAuto` router; add `isolation` to `RunBackgroundOpts`; synchronous pre-flight in the isolated path | +| `src/worktree/worktree-service.ts` | add `isGitRepo(dir?: string): boolean` (`git rev-parse --show-toplevel` succeeds); used by the `auto` router + the `"worktree"` sync-fail | +| `src/runtime/run-journal.ts` | `RunStartedEvent.worktree?` optional; `RunCompletedEvent.branch?` optional (in-place runs have neither). Additive — old events still parse. | +| `src/runtime/resume.ts` | `scanResumeCandidates` handle missing `worktree` field: an interrupted in-place run (no `started.worktree`) → abort + notify (no worktree to clean); `ResumeCandidate.worktreePath?`/`branch?` optional | +| `src/tools/subagent.ts` | add `isolation: "worktree" \| "none" \| "auto"` param (default `auto`); route through `runBackgroundAuto`; `"worktree"`-in-non-git returns synchronous `isError` | +| `src/index.ts` | scheduler `onFire` threads `isolation` (default `auto`); wire `WorktreeService.isGitRepo` into the router; per-session in-place-fallback notify dedup (module-level flag) | +| `src/runtime/async-runner.test.mts` | in-place in non-git (the bug case); isolated in git (regression); `worktree`-in-non-git sync error; `auto` routing; interrupted in-place abort | +| `src/tools/subagent.test.mts` (if present) / `src/runtime/resume.test.mts` | isolation param routing + sync error; in-place interrupted abort | +| `src/runtime/run-journal.test.mts` | optional `worktree`/`branch` round-trip | + +### Type additions + +```ts +// run-journal.ts +interface RunStartedEvent { type: "run:started"; runId: string; task: string; lifecycle: string; + worktree?: { path: string; branch: string }; mode: "auto" | "checkpointed"; ts: number; } // worktree now optional +interface RunCompletedEvent { type: "run:completed"; runId: string; branch?: string; ts: number; } // branch now optional + +// async-runner.ts +type Isolation = "worktree" | "none" | "auto"; +interface RunBackgroundOpts { deps: AsyncRunnerDeps; lifecycle: string; mode: "auto" | "checkpointed"; isolation?: Isolation; } // default "auto" + +// subagent tool +isolation: Type.Optional(Type.Union([ + Type.Literal("worktree"), Type.Literal("none"), Type.Literal("auto"), +], { description: "Edit isolation for background runs. 'worktree' = git worktree (requires a git repo; fails sync if not). 'none' = in-place in cwd (no isolation; parallel edits may conflict). 'auto' (default) = worktree when cwd is a git repo, in-place otherwise." })), + +// worktree-service.ts +isGitRepo(dir?: string): boolean; // git rev-parse --show-toplevel succeeds +``` + +## 4. Edge cases + +| Case | Behavior | +|---|---| +| `auto` in a git cwd | isolated (worktree) — current behavior preserved; no notify | +| `auto` in a non-git cwd | in-place; **one** per-session notify ("background run in-place — no worktree isolation, parallel edits may conflict") | +| `"worktree"` in a non-git cwd | **synchronous `isError`**: "isolation: 'worktree' requires a git repo; cwd '…' is not one — use isolation: 'none' or run in a git repo". Router returns `{ status: "failed", error }` (no runId); tool surfaces `isError`. No async toast, no 90s poll. | +| `"none"` in a git cwd | in-place (skip worktree overhead) — for known read-only runs; no notify (explicit choice) | +| Worktree creation fails for a non-git reason (e.g. disk full) | synchronous `isError` from `runBackgroundIsolated` (worktree.create is now sync, before returning) | +| Interrupted in-place run on restart | abort + notify ("fleet run X interrupted — partial edits may remain in cwd; re-fire to retry"); no worktree to clean | +| Interrupted isolated run on restart | unchanged — worktree-missing → abort (existing `scanResumeCandidates` path) | +| 7 parallel `auto` runs in non-git | 7 in-place runs, 1 notify (per-session dedup) | +| Old journal events (pre-0.11.1, have `worktree`) | parse unchanged (fields are now optional, not removed) | +| Scheduled run (`scheduler.onFire`) | threads `isolation` (default `auto`); scheduled runs usually in git repos → worktree, unchanged | + +## 5. What does NOT change + +- **Foreground `subagent`** (no `background`) — unchanged; already in-place in `ctx.cwd`. +- **`runLifecycle`** — unchanged; it already accepts optional `worktreePath` (undefined → prompt-baked artifact parser; set → diff-service). The core passes `undefined`; the wrapper passes `wt.path`. +- **`WorktreeService.create/remove/removeWorktree`** — unchanged; just gains `isGitRepo()`. +- **`RunLog`** (the per-agent conversation journal) — untouched (this is the `conversations/` journal; the split is in the `runs/` RunJournal). +- **6-2 gate chain / liveness probe / cross-cwd filter** — untouched. +- **`RunResult.branch?`** — already optional in `results-inbox.ts`; in-place runs push results with no `branch`. + +## 6. Scope + release + +### IN scope (→ v0.11.1) +- `runBackground` split (core + wrapper + `auto` router) +- `isolation` param on the `subagent` tool (default `auto`) + scheduler +- `WorktreeService.isGitRepo()` +- Synchronous fail-fast on worktree failures (`"worktree"`-in-non-git + any worktree-create failure) +- `RunStartedEvent.worktree?` / `RunCompletedEvent.branch?` optional +- `scanResumeCandidates` handles in-place interrupted runs (abort + notify) +- Per-session in-place-fallback notify dedup +- Tests (TDD) + +### NOT in scope +- Resume-in-place of interrupted in-place runs (abort is the v0.11.1 choice; resume-in-place is a future refinement if users want it) +- Exposing `isolation` on the `/fleet` panel action submenu (the panel's Run action doesn't currently set isolation; the model-callable tool is the surface — panel exposure is a 6-3 concern when workflows land) +- SPEC-6-3 workflows-as-code (the `agent()` `isolation` opt-in inherits this seam — separate SPEC) + +### Release +| | | +|---|---| +| Target version | `v0.11.1` | +| Branch | `fix/spec-bg-non-git` | +| Commit prefix | `fix(bg): …` | +| Release flow | branch → PR → `gh pr merge --merge --delete-branch` → tag `v0.11.1` → CI publish → bump `settings.json` → term-smoke on published | +| Smoke target | `~/local-dev/bug-bounty` (non-git) — fire a `background: true` subagent there post-patch; expect a working in-place run + 1 notify, not the 100%-fail | +| Compatibility | pi `^0.81.1` (unchanged) | + +## 7. Testing strategy + +TDD, node:test via tsx, `--test-timeout=30000`. Existing patterns. + +### Unit tests (new + extended) + +| Test file | Status | Covers | +|---|---|---| +| `src/runtime/async-runner.test.mts` | EXTENDED | in-place in non-git (the bug case — run completes, no worktree, no branch in result); isolated in git (regression — worktree created, commit, branch in result, worktree removed); `isolation:"worktree"` in non-git → sync `isError` (no async toast, no runId returned); `isolation:"auto"` in non-git → in-place + notify; `isolation:"auto"` in git → isolated, no notify; `isolation:"none"` in git → in-place (no worktree); worktree-create failure (non-git-reason) → sync error | +| `src/runtime/run-journal.test.mts` | EXTENDED | `run:started` without `worktree` round-trips; `run:completed` without `branch` round-trips; old events with the fields still parse | +| `src/runtime/resume.test.mts` | EXTENDED | `scanResumeCandidates` with an interrupted in-place run (no `started.worktree`) → abort + notify, no worktree clean; interrupted isolated run unchanged | +| `src/worktree/worktree-service.test.mts` | EXTENDED | `isGitRepo()` true in a git repo, false in a plain dir | +| `src/tools/subagent.test.mts` | EXTENDED | `isolation` param routing (if a tool-level test harness exists; else covered via async-runner + index wiring) | + +### Integration / term smoke +- **The bug repro** is the integration smoke: in `~/local-dev/bug-bounty` (non-git), fire `subagent({ task: "list files", background: true })` → expect a working in-place background run (appears in `/fleet`, completes, result in inbox), not the 100%-fail. +- `isolation: "worktree"` in non-git → model receives a synchronous `isError` (not `background run: …`). + +### Coverage target +- New code: 80%+ (project standard) +- `runBackgroundIsolated` sync-fail paths: 100% branch (the correctness-critical surface) +- `isGitRepo`: both branches + +## 8. References + +- Bug session: `~/.pi/agent/sessions/--Users-rector-local-dev-bug-bounty--/2026-07-28T15-27-32-041Z_019fa956-96c9-7bb2-a98a-bccaf2783d67.jsonl` +- Predecessor: `docs/superpowers/specs/2026-07-28-spec-6-2-quality-gates-design.md` (v0.11.0) +- Existing split precedent: `src/lifecycle/run-lifecycle.ts` (the phase loop already accepts optional `worktreePath` — the core/wrapper split mirrors this) +- SPEC-6-3 (inherits the `isolation` seam for `agent()`): `PRD.md` §8 \ No newline at end of file diff --git a/package.json b/package.json index ecd15a2..23e2dfa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@getpipher/armory-fleet", - "version": "0.11.0", + "version": "0.11.1", "private": false, "description": "The armory suite's subagent orchestrator for the pi coding agent \u2014 a cross-harness, superpowers-native fleet where every agent is armory-native from birth.", "license": "MIT", diff --git a/src/index.ts b/src/index.ts index adfc5cd..c73c05b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -212,17 +212,20 @@ export default async function (pi: ExtensionAPI): Promise { // foreground subagent holding deps.lock made every bg run's first-phase spawn fail fast // (tryAcquire → "concurrency lock unexpectedly unavailable" → 6ms run:aborted). const bgLock = createSingleSlotLock(); + // v0.11.1: isolated runs use worktree-diff artifact discovery + the worktree as spawn cwd; + // in-place runs (worktreePath undefined) use the prompt-baked parser + the session cwd. + const isolated = !!opts.worktreePath; const lifecycleFullDeps: LifecycleRunDeps = { ...deps.lifecycleDeps, genRunId: () => opts.runId, // override: use the async runner's runId - // SPEC-5a (Q3=A): isolated run — worktree-diff artifact discovery instead of the prompt-baked block. - artifactDiscovery: ({ finalText, cwd, baseRef }) => (deps.asyncRunner as AsyncRunnerDeps).diff.diffPhase(cwd, baseRef, finalText), + ...(isolated ? { artifactDiscovery: ({ finalText, cwd, baseRef }: { finalText: string; cwd: string; baseRef: string }) => (deps.asyncRunner as AsyncRunnerDeps).diff.diffPhase(cwd, baseRef, finalText) } : {}), spawn: async (o) => spawnSubagent({ agent: o.agent, task: o.task, lifecycleTodoId: o.lifecycleTodoId, model: o.model, skillsOverride: o.skills, backendOverride: o.backend, registry: deps.registry, todoSync: deps.todoSync, runRegistry: deps.runRegistry, lock: bgLock, - backendRegistry: deps.backendRegistry, parentModel: deps.parentModel, parentCwd: opts.worktreePath, runLog: deps.runLog, - tierRegistry: deps.tierRegistry, modelRegistry: deps.modelRegistry, // SPEC-6-1 + backendRegistry: deps.backendRegistry, parentModel: deps.parentModel, + parentCwd: isolated ? opts.worktreePath! : deps.parentCwd, + runLog: deps.runLog, tierRegistry: deps.tierRegistry, modelRegistry: deps.modelRegistry, }), }; const res = await runLifecycle(task, lifecycleName, { deps: lifecycleFullDeps, mode: opts.mode, worktreePath: opts.worktreePath, baseRef: "HEAD", onCheckpoint: async (p) => p.status === "failed" ? { action: "abort" } : { action: "continue" } }); @@ -297,7 +300,7 @@ export default async function (pi: ExtensionAPI): Promise { lockPath: join(dir, "schedules.lock"), onFire: (spec) => { if (!deps.asyncRunner) return; - runBackground(spec.task, { deps: deps.asyncRunner, lifecycle: spec.lifecycle ?? "default", mode: spec.auto ? "auto" : "checkpointed" }); + runBackground(spec.task, { deps: deps.asyncRunner, lifecycle: spec.lifecycle ?? "default", mode: spec.auto ? "auto" : "checkpointed", isolation: spec.isolation }); }, }); deps.scheduler.start(); diff --git a/src/runtime/async-runner.ts b/src/runtime/async-runner.ts index f0cd0f8..2962282 100644 --- a/src/runtime/async-runner.ts +++ b/src/runtime/async-runner.ts @@ -23,8 +23,8 @@ export interface FakeLifecycleResult { export interface RunLifecycleOpts { runId: string; - worktreePath: string; - branch: string; + worktreePath?: string; + branch?: string; mode: "auto" | "checkpointed"; } @@ -49,12 +49,22 @@ export interface RunBackgroundOpts { deps: AsyncRunnerDeps; lifecycle: string; mode: "auto" | "checkpointed"; + /** v0.11.1: edit isolation for background runs. Default "auto" (worktree when cwd is a git repo, in-place otherwise). */ + isolation?: Isolation; } -export interface RunBackgroundHandle { - runId: string; - status: "background"; -} +/** The success shape (back-compat: existing external refs to RunBackgroundHandle still typecheck). */ +export interface RunBackgroundHandle { runId: string; status: "background"; } + +/** v0.11.1: a background dispatch either starts (runId + background) or fails synchronously (error, no runId). */ +export type RunBackgroundResult = + | { runId: string; status: "background" } + | { status: "failed"; error: string }; + +export type Isolation = "worktree" | "none" | "auto"; + +/** Per-session dedup flag for the auto-fallback in-place notify (resets on process restart = new session). */ +let inPlaceFallbackWarned = false; function emitProgress(deps: AsyncRunnerDeps, runId: string, partial: Partial & { status: import("../panel/rows.ts").BgStatus; phase: string; phaseIndex: number; phaseTotal: number }): void { if (!deps.onProgress) return; @@ -72,54 +82,98 @@ function sh(cmd: string, cwd: string): void { execSync(cmd, { cwd, encoding: "utf8", stdio: ["pipe", "pipe", "pipe"] }); } -export function runBackground(task: string, opts: RunBackgroundOpts): RunBackgroundHandle { +/** The git-agnostic core: pool slot → journal → runLifecycle in ctx.cwd (or a worktree, when `isolated` is set) → inbox + notify. + * Fire-and-forget. When `isolated` is present, journals the worktree field, commits on completion, and removes the worktree. */ +function runBackgroundInPlace(runId: string, task: string, opts: RunBackgroundOpts, isolated?: { worktreePath: string; branch: string }): void { const { deps } = opts; - const runId = deps.genRunId(); - const baseRef = "HEAD"; - - // Fire-and-forget: the pool gates concurrency; the journal records the run. void deps.pool.withSlot(async () => { - let wt: { path: string; branch: string } | null = null; try { - wt = deps.worktree.create(runId, baseRef); - const ev0: JournalEvent = { type: "run:started", runId, task, lifecycle: opts.lifecycle, worktree: { path: wt.path, branch: wt.branch }, mode: opts.mode, ts: Date.now() }; + const ev0: JournalEvent = isolated + ? { type: "run:started", runId, task, lifecycle: opts.lifecycle, worktree: { path: isolated.worktreePath, branch: isolated.branch }, mode: opts.mode, ts: Date.now() } + : { type: "run:started", runId, task, lifecycle: opts.lifecycle, mode: opts.mode, ts: Date.now() }; deps.journal.append(runId, ev0); emitProgress(deps, runId, { status: "running", phase: "", phaseIndex: 0, phaseTotal: 0, lifecycle: opts.lifecycle, mode: opts.mode, task }); - const res = await deps.runLifecycle(task, opts.lifecycle, { runId, worktreePath: wt.path, branch: wt.branch, mode: opts.mode }); + const res = await deps.runLifecycle(task, opts.lifecycle, { runId, worktreePath: isolated?.worktreePath, branch: isolated?.branch, mode: opts.mode }); if (res.status === "completed") { - // commit the worktree to the branch (lifecycle finish phase or single-delegate completion) - try { sh("git add -A && git commit -m 'fleet run complete'", wt.path); } catch { /* nothing to commit */ } - deps.journal.append(runId, { type: "run:completed", runId, branch: wt.branch, ts: Date.now() }); + if (isolated) { + try { sh("git add -A && git commit -m 'fleet run complete'", isolated.worktreePath); } catch { /* nothing to commit */ } + } + deps.journal.append(runId, isolated + ? { type: "run:completed", runId, branch: isolated.branch, ts: Date.now() } + : { type: "run:completed", runId, ts: Date.now() }); const total = res.phases.length; - const lastIdx = total; // completed = past the last phase - emitProgress(deps, runId, { status: "completed", phase: res.phases[total - 1]?.name ?? "finish", phaseIndex: lastIdx, phaseTotal: total, lifecycle: opts.lifecycle, mode: opts.mode, task, branch: wt.branch }); + const lastIdx = total; + emitProgress(deps, runId, { status: "completed", phase: res.phases[total - 1]?.name ?? "finish", phaseIndex: lastIdx, phaseTotal: total, lifecycle: opts.lifecycle, mode: opts.mode, task, ...(isolated ? { branch: isolated.branch } : {}) }); const lastPhase = res.phases[res.phases.length - 1]; const result: RunResult = { runId, task, status: "completed", summary: lastPhase?.summary ?? "", paths: res.phases.flatMap((p) => p.paths), - branch: wt.branch, completedAt: Date.now(), + completedAt: Date.now(), + ...(isolated ? { branch: isolated.branch } : {}), }; deps.inbox.push(result); deps.notify(`fleet run ${runId} completed`, "info"); - // SPEC-5a: the worktree dir is temporary scaffolding; remove it but keep the branch for merge/inspection. - deps.worktree.removeWorktree(runId); + if (isolated) deps.worktree.removeWorktree(runId); } else { deps.journal.append(runId, { type: "run:aborted", runId, reason: res.error ?? res.status, ts: Date.now() }); - deps.worktree.remove(runId); + if (isolated) deps.worktree.remove(runId); emitProgress(deps, runId, { status: "failed", phase: "", phaseIndex: 0, phaseTotal: res.phases.length, lifecycle: opts.lifecycle, mode: opts.mode, task }); deps.notify(`fleet run ${runId} ${res.status}: ${res.error ?? ""}`, "warning"); } } catch (e) { const msg = (e as Error).message; deps.journal.append(runId, { type: "run:aborted", runId, reason: msg, ts: Date.now() }); - if (wt) deps.worktree.remove(runId); + if (isolated) deps.worktree.remove(runId); deps.notify(`fleet run ${runId} failed: ${msg}`, "error"); emitProgress(deps, runId, { status: "failed", phase: "", phaseIndex: 0, phaseTotal: 0, lifecycle: opts.lifecycle, mode: opts.mode, task }); } }); +} +/** The worktree wrapper: SYNCHRONOUS pre-flight + worktree create, then core-with-isolation. */ +function runBackgroundIsolated(task: string, opts: RunBackgroundOpts): RunBackgroundResult { + const { deps } = opts; + if (!deps.worktree.isGitRepo()) { + return { status: "failed", error: "isolation: 'worktree' requires a git repo; cwd is not one — use isolation: 'none' or run in a git repo" }; + } + const runId = deps.genRunId(); + const baseRef = "HEAD"; + let wt: { path: string; branch: string }; + try { + wt = deps.worktree.create(runId, baseRef); + } catch (e) { + return { status: "failed", error: (e as Error).message }; + } + runBackgroundInPlace(runId, task, opts, { worktreePath: wt.path, branch: wt.branch }); return { runId, status: "background" }; +} + +/** The auto router: isolated when cwd is a git repo, in-place + one per-session notify when not. */ +function runBackgroundAuto(task: string, opts: RunBackgroundOpts): RunBackgroundResult { + const { deps } = opts; + if (deps.worktree.isGitRepo()) { + return runBackgroundIsolated(task, opts); + } + if (!inPlaceFallbackWarned) { + inPlaceFallbackWarned = true; + deps.notify("background run in-place (no worktree isolation — parallel edits may conflict)", "warning"); + } + const runId = deps.genRunId(); + runBackgroundInPlace(runId, task, opts); + return { runId, status: "background" }; +} + +/** Public dispatcher (keeps the `runBackground` name + RunBackgroundHandle success shape for back-compat). */ +export function runBackground(task: string, opts: RunBackgroundOpts): RunBackgroundResult { + const isolation = opts.isolation ?? "auto"; + if (isolation === "worktree") return runBackgroundIsolated(task, opts); + if (isolation === "none") { + const runId = opts.deps.genRunId(); + runBackgroundInPlace(runId, task, opts); + return { runId, status: "background" }; + } + return runBackgroundAuto(task, opts); } \ No newline at end of file diff --git a/src/runtime/resume.ts b/src/runtime/resume.ts index 8322421..bb57ee4 100644 --- a/src/runtime/resume.ts +++ b/src/runtime/resume.ts @@ -8,8 +8,10 @@ export interface ResumeCandidate { runId: string; task: string; lifecycle: string; - worktreePath: string; - branch: string; + /** Present only for isolated (worktree) runs. */ + worktreePath?: string; + /** Present only for isolated (worktree) runs. */ + branch?: string; lastPhase: string | null; canResume: boolean; } @@ -26,23 +28,27 @@ export function scanResumeCandidates(_projectDir: string, opts: ScanResumeOpts): for (const runId of ids) { const events = journal.replay(runId); const started = events.find((e) => e.type === "run:started") as - | (JournalEvent & { type: "run:started" }) | undefined; + | (JournalEvent & { type: "run:started"; worktree?: { path: string; branch: string } }) | undefined; if (!started) continue; const phaseEvents = events.filter((e) => e.type === "phase:completed" || e.type === "phase:started" || e.type === "phase:failed") as Array<{ phase: string }>; const lastPhase = phaseEvents.length > 0 ? phaseEvents[phaseEvents.length - 1]!.phase : null; - const wtExists = opts.worktree.exists(runId); - if (!wtExists) { - journal.append(runId, { type: "run:aborted", runId, reason: "worktree-missing", ts: Date.now() }); + + if (started.worktree) { + // isolated run: resume iff the worktree still exists + const wtExists = opts.worktree.exists(runId); + if (!wtExists) { + journal.append(runId, { type: "run:aborted", runId, reason: "worktree-missing", ts: Date.now() }); + } + cands.push({ + runId, task: started.task, lifecycle: started.lifecycle, + worktreePath: started.worktree.path, branch: started.worktree.branch, + lastPhase, canResume: wtExists, + }); + } else { + // v0.11.1: in-place interrupted run — no worktree to clean; abort (partial edits may remain in cwd). + journal.append(runId, { type: "run:aborted", runId, reason: "in-place interrupted (partial edits may remain in cwd)", ts: Date.now() }); + cands.push({ runId, task: started.task, lifecycle: started.lifecycle, lastPhase, canResume: false }); } - cands.push({ - runId, - task: started.task, - lifecycle: started.lifecycle, - worktreePath: started.worktree.path, - branch: started.worktree.branch, - lastPhase, - canResume: wtExists, - }); } return cands; } \ No newline at end of file diff --git a/src/runtime/run-journal.ts b/src/runtime/run-journal.ts index d31f371..bb9f693 100644 --- a/src/runtime/run-journal.ts +++ b/src/runtime/run-journal.ts @@ -4,12 +4,12 @@ import { appendFileSync, existsSync, mkdirSync, readFileSync, readdirSync } from "node:fs"; import { join } from "node:path"; -export interface RunStartedEvent { type: "run:started"; runId: string; task: string; lifecycle: string; worktree: { path: string; branch: string }; mode: "auto" | "checkpointed"; ts: number; } +export interface RunStartedEvent { type: "run:started"; runId: string; task: string; lifecycle: string; worktree?: { path: string; branch: string }; mode: "auto" | "checkpointed"; ts: number; } export interface PhaseStartedEvent { type: "phase:started"; phase: string; ts: number; } export interface PhaseCompletedEvent { type: "phase:completed"; phase: string; summary: string; paths: string[]; ts: number; } export interface PhaseFailedEvent { type: "phase:failed"; phase: string; error: string; ts: number; } export interface CheckpointEvent { type: "checkpoint"; phase: string; decision: "continue" | "revise" | "abort"; ts: number; } -export interface RunCompletedEvent { type: "run:completed"; runId: string; branch: string; ts: number; } +export interface RunCompletedEvent { type: "run:completed"; runId: string; branch?: string; ts: number; } export interface RunAbortedEvent { type: "run:aborted"; runId: string; reason: string; ts: number; } export type JournalEvent = diff --git a/src/scheduling/scheduler.ts b/src/scheduling/scheduler.ts index 6e03af3..22fca69 100644 --- a/src/scheduling/scheduler.ts +++ b/src/scheduling/scheduler.ts @@ -11,6 +11,8 @@ export interface ScheduleSpec { expression: string; lifecycle?: string; // default "default" auto?: boolean; + /** v0.11.1: edit isolation for the background run on fire. Default "auto". */ + isolation?: "worktree" | "none" | "auto"; } export interface Schedule extends ScheduleSpec { @@ -71,6 +73,7 @@ export class Scheduler { expression: spec.expression, lifecycle: spec.lifecycle ?? "default", auto: spec.auto ?? true, + isolation: spec.isolation, paused: false, }; this.schedules.set(id, { spec: stored, expr, timer: null }); @@ -86,6 +89,7 @@ export class Scheduler { expression: e.spec.expression, lifecycle: e.spec.lifecycle, auto: e.spec.auto, + isolation: e.spec.isolation, paused: e.spec.paused, nextFire: e.spec.paused ? null : e.expr.nextFire(new Date()), })); diff --git a/src/tools/subagent.ts b/src/tools/subagent.ts index 930f43b..a183412 100644 --- a/src/tools/subagent.ts +++ b/src/tools/subagent.ts @@ -22,6 +22,11 @@ export const subagentParams = Type.Object({ lifecycle: Type.Optional(Type.String({ description: "Run a multi-phase superpowers lifecycle by name (e.g. 'default') instead of a single delegate. Tool-driven lifecycles run end-to-end (auto) — checkpoints are a /fleet panel feature." })), auto: Type.Optional(Type.Boolean({ description: "Only relevant with `lifecycle`. Tool-driven is always auto; this flag is forward-compat. Panel-driven uses --auto on /fleet-implement." })), background: Type.Optional(Type.Boolean({ description: "Fire without awaiting. The run goes to the async/bg pool on an isolated git worktree; this returns { runId, status: 'background' } immediately. Foreground (default) awaits the result." })), + isolation: Type.Optional(Type.Union([ + Type.Literal("worktree"), + Type.Literal("none"), + Type.Literal("auto"), + ], { description: "Edit isolation for background runs. 'worktree' = git worktree (requires a git repo; fails sync if not). 'none' = in-place in cwd (no isolation; parallel edits may conflict). 'auto' (default) = worktree when cwd is a git repo, in-place otherwise." })), schedule: Type.Optional(Type.String({ description: 'Schedule the run instead of firing now: a cron string ("0 9 * * 1-5"), an interval ("30m"/"2h"), or a one-shot ISO datetime ("2026-07-25T14:00"). Returns { scheduleId, nextFire }. Session-scoped (fires only while pi is open); no catch-up.' })), maxTurns: Type.Optional(Type.Number({ description: 'Per-run turn budget (default 20). Raise for complex multi-step tasks (e.g. 40) so the subagent doesn\'t hit the budget mid-task; lower for trivial lookups.' })), }); @@ -80,13 +85,14 @@ export function createSubagentTool(deps: SubagentToolDeps) { } if (params.schedule) { if (!deps.scheduler) return { isError: true, content: [{ type: "text" as const, text: "scheduling not configured (scheduler missing)" }] }; - const id = deps.scheduler.register({ task: params.task, expression: params.schedule, lifecycle: params.lifecycle ?? "default", auto: params.auto ?? true }); + const id = deps.scheduler.register({ task: params.task, expression: params.schedule, lifecycle: params.lifecycle ?? "default", auto: params.auto ?? true, isolation: params.isolation }); const entry = deps.scheduler.list().find((s) => s.id === id); return { content: [{ type: "text" as const, text: `scheduled: ${id} · next fire: ${entry?.nextFire?.toISOString() ?? "(paused)"}` }], details: { scheduleId: id, nextFire: entry?.nextFire ?? null } }; } if (params.background) { if (!deps.asyncRunner) return { isError: true, content: [{ type: "text" as const, text: "background runs not configured (asyncRunner missing)" }] }; - const handle = runBackground(params.task, { deps: deps.asyncRunner, lifecycle: params.lifecycle ?? "default", mode: "auto" }); + const handle = runBackground(params.task, { deps: deps.asyncRunner, lifecycle: params.lifecycle ?? "default", mode: "auto", isolation: params.isolation }); + if (handle.status === "failed") return { isError: true, content: [{ type: "text" as const, text: handle.error }] }; return { content: [{ type: "text" as const, text: `background run: ${handle.runId}` }], details: handle }; } if (params.lifecycle) { diff --git a/src/worktree/worktree-service.ts b/src/worktree/worktree-service.ts index 57c8754..bc6ae86 100644 --- a/src/worktree/worktree-service.ts +++ b/src/worktree/worktree-service.ts @@ -40,6 +40,16 @@ export class WorktreeService { return existsSync(this.pathFor(runId)); } + /** v0.11.1: is `rootDir` (or `dir`) inside a git repo? Cheap sync pre-flight for isolation routing. */ + isGitRepo(dir: string = this.rootDir): boolean { + try { + sh("git rev-parse --show-toplevel", dir); + return true; + } catch { + return false; + } + } + create(runId: string, baseRef = "HEAD"): WorktreeRef { if (this.exists(runId)) { throw new Error(`worktree for run ${runId} already exists at ${this.pathFor(runId)}`); diff --git a/test/async-runner.test.mts b/test/async-runner.test.mts index 7d963e9..b040c38 100644 --- a/test/async-runner.test.mts +++ b/test/async-runner.test.mts @@ -44,7 +44,7 @@ function makeDeps(repo: string, runLifecycle: RunLifecycleFn): { deps: AsyncRunn test("runBackground creates a worktree, journals run:started, drives runLifecycle, journals run:completed, pushes to inbox, notifies", async () => { const repo = makeRepo(); const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { - writeFileSync(join(opts.worktreePath, "design.md"), "# design\n"); + writeFileSync(join(opts.worktreePath!, "design.md"), "# design\n"); return { runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", phases: [{ name: "brainstorm", status: "completed", summary: "did it", paths: ["design.md"], reviseCount: 0 }], @@ -52,8 +52,9 @@ test("runBackground creates a worktree, journals run:started, drives runLifecycl }; }; const { deps, journal, inbox, notifications } = makeDeps(repo, fakeLifecycle); - const { runId, status } = runBackground("add hello", { deps, lifecycle: "default", mode: "auto" }); - assert.equal(status, "background"); + const handle = runBackground("add hello", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(handle.status, "background"); + const { runId } = handle; await new Promise((r) => setTimeout(r, 60)); const events = journal.replay(runId); assert.ok(events.some((e) => e.type === "run:started"), "no run:started"); @@ -68,11 +69,107 @@ test("runBackground journals run:aborted + cleans up the worktree when runLifecy const failingLifecycle: RunLifecycleFn = async () => { throw new Error("model blew up"); }; const { deps, journal, notifications } = makeDeps(repo, failingLifecycle); const wt = deps.worktree; - const { runId } = runBackground("bad task", { deps, lifecycle: "default", mode: "auto" }); + const h = runBackground("bad task", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h.status, "background"); + const { runId } = h; await new Promise((r) => setTimeout(r, 60)); const events = journal.replay(runId); assert.ok(events.some((e) => e.type === "run:aborted"), "no run:aborted"); assert.equal(wt.exists(runId), false, "worktree not cleaned up"); assert.ok(notifications.some((n) => /failed|error/i.test(n)), `notifications: ${notifications.join("|")}`); rmSync(repo, { recursive: true, force: true }); +}); + +test("runBackground isolation:'none' runs in-place in a NON-GIT dir, journals run:started with no worktree, completes, pushes result with no branch", async () => { + const plain = mkdtempSync(join(tmpdir(), "async-nogit-")); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + assert.equal(opts.worktreePath, undefined, "in-place run must not receive a worktreePath"); + writeFileSync(join(plain, "out.txt"), "done\n"); + return { + runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "implement", status: "completed", summary: "did it", paths: ["out.txt"], reviseCount: 0 }], + startedAt: 1, endedAt: 2, todoId: "td-x", + }; + }; + const { deps, journal, inbox, notifications } = makeDeps(plain, fakeLifecycle); + const handle = runBackground("research x", { deps, lifecycle: "default", mode: "auto", isolation: "none" }); + assert.equal(handle.status, "background"); + assert.ok("runId" in handle, "in-place handle has a runId"); + await new Promise((r) => setTimeout(r, 60)); + const events = journal.replay(handle.runId); + const started = events.find((e) => e.type === "run:started") as any; + assert.equal(started.worktree, undefined, "in-place run:started must omit worktree"); + assert.ok(events.some((e) => e.type === "run:completed"), "no run:completed"); + const completed = events.find((e) => e.type === "run:completed") as any; + assert.equal(completed.branch, undefined, "in-place run:completed must omit branch"); + assert.equal(inbox.readyCount(), 1); + assert.equal(inbox.pull()[0]!.branch, undefined, "in-place result has no branch"); + assert.ok(notifications.some((n) => /completed/.test(n)), `notifications: ${notifications.join("|")}`); + rmSync(plain, { recursive: true, force: true }); +}); + +test("runBackground isolation:'worktree' in a NON-GIT dir returns a SYNCHRONOUS failed result (no runId, no async toast, no 90s poll)", () => { + const plain = mkdtempSync(join(tmpdir(), "async-nogit2-")); + const fakeLifecycle: RunLifecycleFn = async () => ({ runId: "x", lifecycleName: "x", task: "x", backend: "pi", mode: "auto", status: "completed", phases: [], startedAt: 1, endedAt: 2, todoId: null }); + const { deps, notifications } = makeDeps(plain, fakeLifecycle); + const handle = runBackground("edit x", { deps, lifecycle: "default", mode: "auto", isolation: "worktree" }); + assert.equal(handle.status, "failed"); + assert.ok(!("runId" in handle), "sync-fail must NOT return a runId"); + assert.ok(/requires a git repo/.test((handle as any).error), `error: ${(handle as any).error}`); + // No run was started → no async toast fires for this runId + assert.equal(notifications.length, 0, "sync-fail must not emit an async notify"); + rmSync(plain, { recursive: true, force: true }); +}); + +test("runBackground default (auto) in a NON-GIT dir falls back to in-place + emits ONE per-session fallback notify", () => { + const plain = mkdtempSync(join(tmpdir(), "async-nogit3-")); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => ({ + runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "implement", status: "completed", summary: "s", paths: [], reviseCount: 0 }], + startedAt: 1, endedAt: 2, todoId: null, + }); + const { deps, notifications } = makeDeps(plain, fakeLifecycle); + // first auto-fallback run → 1 notify + const h1 = runBackground("a", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h1.status, "background"); + // second auto-fallback run → no additional notify (per-session dedup) + const h2 = runBackground("b", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h2.status, "background"); + const fallbackNotes = notifications.filter((n) => /in-place|worktree isolation/.test(n)); + assert.equal(fallbackNotes.length, 1, `expected exactly 1 fallback notify, got: ${notifications.join("|")}`); + rmSync(plain, { recursive: true, force: true }); +}); + +test("runBackground default (auto) in a GIT dir stays isolated (no fallback notify) — regression guard", async () => { + const repo = makeRepo(); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + assert.ok(opts.worktreePath, "git auto run must receive a worktreePath"); + writeFileSync(join(opts.worktreePath!, "d.md"), "# d\n"); + return { runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "brainstorm", status: "completed", summary: "s", paths: ["d.md"], reviseCount: 0 }], startedAt: 1, endedAt: 2, todoId: "t" }; + }; + const { deps, notifications } = makeDeps(repo, fakeLifecycle); + const h = runBackground("x", { deps, lifecycle: "default", mode: "auto" }); + assert.equal(h.status, "background"); + assert.ok("runId" in h); + await new Promise((r) => setTimeout(r, 60)); + assert.equal(notifications.filter((n) => /in-place|worktree isolation/.test(n)).length, 0, "git auto must not fallback-notify"); + rmSync(repo, { recursive: true, force: true }); +}); + +test("runBackground isolation:'none' in a GIT dir runs in-place (explicit opt-out, no notify)", async () => { + const repo = makeRepo(); + const fakeLifecycle: RunLifecycleFn = async (task, lifecycleName, opts) => { + assert.equal(opts.worktreePath, undefined, "explicit none must not receive a worktreePath"); + return { runId: opts.runId, lifecycleName, task, backend: "pi", mode: "auto", status: "completed", + phases: [{ name: "implement", status: "completed", summary: "s", paths: [], reviseCount: 0 }], startedAt: 1, endedAt: 2, todoId: null }; + }; + const { deps, journal, notifications } = makeDeps(repo, fakeLifecycle); + const h = runBackground("ro", { deps, lifecycle: "default", mode: "auto", isolation: "none" }); + assert.equal(h.status, "background"); + await new Promise((r) => setTimeout(r, 60)); + const started = journal.replay(h.runId).find((e) => e.type === "run:started") as any; + assert.equal(started.worktree, undefined); + assert.equal(notifications.filter((n) => /in-place|worktree isolation/.test(n)).length, 0, "explicit none must not fallback-notify"); + rmSync(repo, { recursive: true, force: true }); }); \ No newline at end of file diff --git a/test/resume.test.mts b/test/resume.test.mts index 156771a..9faa2a7 100644 --- a/test/resume.test.mts +++ b/test/resume.test.mts @@ -62,4 +62,23 @@ test("scanResumeCandidates skips terminal runs (completed/aborted)", () => { const cands = scanResumeCandidates(repo, { runsDir, worktree: wt }); assert.equal(cands.length, 0); rmSync(repo, { recursive: true, force: true }); +}); + +test("scanResumeCandidates aborts an interrupted IN-PLACE run (no worktree field) with canResume=false", () => { + const repo = makeRepo(); // repo only for the WorktreeService ctor; the run is in-place + const runsDir = join(repo, ".pi", "fleet", "runs"); + const journal = new RunJournal(runsDir); + const wt = new WorktreeService({ rootDir: repo }); + // an in-place run: run:started with NO worktree field, no terminal event + journal.append("fl-ip-int", { type: "run:started", runId: "fl-ip-int", task: "t", lifecycle: "default", mode: "auto", ts: 1 }); + journal.append("fl-ip-int", { type: "phase:completed", phase: "implement", summary: "s", paths: ["x.ts"], ts: 2 }); + const cands = scanResumeCandidates(repo, { runsDir, worktree: wt }); + assert.equal(cands.length, 1); + assert.equal(cands[0]!.runId, "fl-ip-int"); + assert.equal(cands[0]!.canResume, false); + assert.equal(cands[0]!.worktreePath, undefined); + assert.equal(cands[0]!.branch, undefined); + const events = journal.replay("fl-ip-int"); + assert.equal(events[events.length - 1]!.type, "run:aborted"); + rmSync(repo, { recursive: true, force: true }); }); \ No newline at end of file diff --git a/test/run-journal.test.mts b/test/run-journal.test.mts index 8bceba4..84d3790 100644 --- a/test/run-journal.test.mts +++ b/test/run-journal.test.mts @@ -47,4 +47,27 @@ test("scanNonTerminal returns runs whose journal has no terminal event", () => { const nonTerminal = j.scanNonTerminal().sort(); assert.deepEqual(nonTerminal, ["fl-4"]); rmSync(dir, { recursive: true, force: true }); +}); + +test("run:started without worktree + run:completed without branch round-trip", () => { + const dir = makeDir(); + const j = new RunJournal(dir); + j.append("fl-ip1", { type: "run:started", runId: "fl-ip1", task: "t", lifecycle: "default", mode: "auto", ts: 1 }); + j.append("fl-ip1", { type: "run:completed", runId: "fl-ip1", ts: 2 }); + const events = j.replay("fl-ip1"); + assert.equal(events.length, 2); + const started = events[0] as any; + assert.equal(started.worktree, undefined); + const completed = events[1] as any; + assert.equal(completed.branch, undefined); + rmSync(dir, { recursive: true, force: true }); +}); + +test("old run:started with worktree still parses after the field becomes optional", () => { + const dir = makeDir(); + const j = new RunJournal(dir); + j.append("fl-old", { type: "run:started", runId: "fl-old", task: "t", lifecycle: "default", worktree: { path: "/x", branch: "fleet/fl-old" }, mode: "auto", ts: 1 }); + const events = j.replay("fl-old"); + assert.equal((events[0] as any).worktree.branch, "fleet/fl-old"); + rmSync(dir, { recursive: true, force: true }); }); \ No newline at end of file diff --git a/test/scheduler.test.mts b/test/scheduler.test.mts index fe86a9f..ac5164d 100644 --- a/test/scheduler.test.mts +++ b/test/scheduler.test.mts @@ -89,6 +89,22 @@ test("start is idempotent (calling twice is safe)", async () => { rmSync(dir, { recursive: true, force: true }); }); +test("isolation field threads through register + list AND survives persist+load round-trip", () => { + const dir = mkdtempSync(join(tmpdir(), "sched-iso-")); + const storePath = join(dir, "schedules.json"); + const lockPath = join(dir, "schedules.lock"); + const sch = new Scheduler({ storePath, lockPath, onFire: () => {} }); + const id = sch.register({ task: "t", expression: "5m", lifecycle: "default", auto: true, isolation: "worktree" }); + // list() returns it + const stored = sch.list().find((s) => s.id === id); + assert.equal(stored?.isolation, "worktree", "list() should return isolation"); + // persist+load round-trip: construct a NEW Scheduler on the same storePath (constructor calls load()) + const sch2 = new Scheduler({ storePath, lockPath, onFire: () => {} }); + const loaded = sch2.list().find((s) => s.id === id); + assert.equal(loaded?.isolation, "worktree", "isolation must survive persist+load round-trip"); + rmSync(dir, { recursive: true, force: true }); +}); + test("start creates the lock file's parent dir when it doesn't exist (fresh project)", () => { // Regression: in a fresh project `.pi/fleet/` doesn't exist; persist() only runs on // register(), so start() used to ENOENT on writeFileSync(lockPath). start() must diff --git a/test/subagent-tool.test.mts b/test/subagent-tool.test.mts index 5fb7cf7..eab5636 100644 --- a/test/subagent-tool.test.mts +++ b/test/subagent-tool.test.mts @@ -81,4 +81,36 @@ test("tool execute surfaces isError + actionable message on unknown agent", asyn const out = await tool.execute!("c", { agent: "nope", task: "hi" }, new AbortController().signal, () => {}, {} as any); strictEqual(out.isError, true); ok((out.content[0] as any).text.includes("not in registry")); +}); + +test("subagent background with isolation:'worktree' in a non-git cwd returns isError synchronously", async () => { + const plain = mkdtempSync(join(tmpdir(), "sub-nogit-")); + const fakeAsyncRunner = { + worktree: { isGitRepo: () => false, create: () => { throw new Error("no"); }, removeWorktree: () => {}, remove: () => {}, exists: () => false, branchFor: () => "fleet/x", pathFor: () => plain }, + diff: {}, journal: { append: () => {}, replay: () => [], scanNonTerminal: () => [] }, + pool: { withSlot: async () => {} }, inbox: { push: () => {}, readyCount: () => 0, pull: () => [], renderHint: () => "" }, + runLifecycle: async () => ({ status: "completed", phases: [] } as any), + notify: () => {}, genRunId: () => "fl-x", + } as any; + const tool = createSubagentTool({ ...makeDeps(), parentCwd: plain, asyncRunner: fakeAsyncRunner } as any); + const res = await tool.execute!("id", { agent: "g", task: "x", background: true, isolation: "worktree" } as any, new AbortController().signal, () => {}, {} as any); + ok(res.isError === true, `expected isError, got: ${(res as any).isError}`); + ok(/requires a git repo/.test((res.content as any)[0].text), `text: ${(res.content as any)[0].text}`); + rmSync(plain, { recursive: true, force: true }); +}); + +test("subagent background default (auto) in a non-git cwd returns a background run (in-place)", async () => { + const plain = mkdtempSync(join(tmpdir(), "sub-nogit2-")); + const fakeAsyncRunner = { + worktree: { isGitRepo: () => false, create: () => { throw new Error("no"); }, removeWorktree: () => {}, remove: () => {}, exists: () => false, branchFor: () => "fleet/x", pathFor: () => plain }, + diff: {}, journal: { append: () => {}, replay: () => [], scanNonTerminal: () => [] }, + pool: { withSlot: async () => {} }, inbox: { push: () => {}, readyCount: () => 0, pull: () => [], renderHint: () => "" }, + runLifecycle: async () => ({ status: "completed", phases: [] } as any), + notify: () => {}, genRunId: () => "fl-auto", + } as any; + const tool = createSubagentTool({ ...makeDeps(), parentCwd: plain, asyncRunner: fakeAsyncRunner } as any); + const res = await tool.execute!("id", { agent: "g", task: "x", background: true } as any, new AbortController().signal, () => {}, {} as any); + ok(res.isError === undefined, `expected no isError, got: ${(res as any).isError}`); + ok(/background run:/.test((res.content as any)[0].text), `text: ${(res.content as any)[0].text}`); + rmSync(plain, { recursive: true, force: true }); }); \ No newline at end of file diff --git a/test/worktree-service.test.mts b/test/worktree-service.test.mts index c64656f..27b9c9d 100644 --- a/test/worktree-service.test.mts +++ b/test/worktree-service.test.mts @@ -74,3 +74,14 @@ test("removeWorktree removes the worktree dir but KEEPS the branch (SPEC-5a comp assert.ok(branches.includes(branch), `branch ${branch} should be kept, got: ${branches}`); rmSync(repo, { recursive: true, force: true }); }); + +test("isGitRepo is true in a git repo, false in a plain dir", () => { + const repo = makeRepo(); + const plain = mkdtempSync(join(tmpdir(), "wt-nogit-")); + const svc = new WorktreeService({ rootDir: repo }); + assert.equal(svc.isGitRepo(), true); + const svcPlain = new WorktreeService({ rootDir: plain }); + assert.equal(svcPlain.isGitRepo(), false); + rmSync(repo, { recursive: true, force: true }); + rmSync(plain, { recursive: true, force: true }); +});