diff --git a/README.md b/README.md index dc5f91521..07f6a98ee 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,9 @@ orca run implement.sc "add a rate limiter to /login" ``` Useful flags: `--skip-branch` (continue on the current branch instead of -creating one) and `--keep-changes` (leave uncommitted files in place instead of -stashing them). +creating one), `--keep-changes` (leave uncommitted files in place instead of +stashing them) and `--worktree` (run in a git worktree of this repository +instead of the current checkout). In every mode, which agent (and model) handles the planning, coding, and review roles comes from `settings.properties` — written for you by the shell's @@ -337,6 +338,20 @@ Each `flow(...)` run is bound to exactly one feature branch and one progress log stashes. A run that already has a progress log — a resume, or one too broken to read — always stashes and ignores `--keep-changes`, so an interrupted stage's partial work can't leak into the stage that re-runs. + `--worktree` (`OrcaArgs.worktree`) runs the whole flow in + `.orca/worktrees/` of this repository — a second checkout, keyed on the + same prompt hash as the progress log, created on the first run and reused by + every later one for that task. It isolates the run: two tasks can run at once + without sharing a checkout or a branch. Uncommitted work does NOT come along — + a worktree is made from a commit — so `--worktree` is refused with + `--skip-branch` and with `--keep-changes`. The first run in a worktree pays a + cold build (no build outputs, no dependencies, none of the untracked local + config a project may need), an editor or indexer that ignores `.gitignore` + will see the second checkout, and orca never removes it. The run starts on an + `orca-worktree-` branch orca also never deletes, so full cleanup is `git + worktree remove .orca/worktrees/` **and** `git branch -d + orca-worktree-`; a re-run of the task refuses rather than moving that + branch if it has gained commits since. Sharp edge: kept files are unprotected until that first stage commit — a failure before it runs the teardown's `git reset --hard` and destroys kept modifications to tracked files (kept untracked files survive). @@ -951,7 +966,7 @@ action non-interactively and exits. | Command | Key flags | Does | |---|---|---| -| `orca run [task]` | `--verbose` (stack trace on abort), `--skip-branch`, `--keep-changes` (leave uncommitted files in place), `--honor-pin` (use the flow's own pinned orca version) | run a flow, propagating its exit code; task is read from stdin when omitted and piped | +| `orca run [task]` | `--verbose` (stack trace on abort), `--skip-branch`, `--keep-changes` (leave uncommitted files in place), `--worktree` (run in a git worktree of this repository), `--honor-pin` (use the flow's own pinned orca version) | run a flow, propagating its exit code; task is read from stdin when omitted and piped | | `orca view ` | `--plain`, `--color` | print a flow's source (highlighted when stdout is a terminal) | | `orca edit ` | `--to project\|global` | open a flow in `$VISUAL`/`$EDITOR`/vi (`--to` required to customize a built-in) | | `orca create ""` | `--name `, `--global` | author a new flow: the built-in `simple.sc` flow writes it in an isolated sandbox with the configured role agents; `--name` is auto-derived when omitted | diff --git a/adr/0018-stage-bound-flow-runtime.md b/adr/0018-stage-bound-flow-runtime.md index d9ce6dc79..d27a7d0c5 100644 --- a/adr/0018-stage-bound-flow-runtime.md +++ b/adr/0018-stage-bound-flow-runtime.md @@ -438,6 +438,22 @@ the wrong branch. > `git reset --hard` and destroys kept modifications to tracked files. Kept > untracked files survive: R5's clean is skipped for this run too, for the > reason its own text gives. + + > **Amendment (2026-08-27).** `OrcaArgs.worktree` (`--worktree`) runs the flow + > in `.orca/worktrees/` of this repository instead of the current + > checkout. The FIRST run in a freshly created worktree always meets this + > requirement's clean case: a worktree is created from a commit, so its tree + > is clean by construction. A REUSED worktree — every resume, and every re-run + > of the same task text — is subject to the same dirty-tree policy as any + > other directory: a user's edits, or a SIGKILLed run's partial ones, are + > still there, and the prompt fires normally. Uncommitted work in the invoking + > checkout stays there. The run starts on an `orca-worktree-` branch + > that orca creates and never deletes; it refuses to move that branch if it + > has since gained commits. `--worktree` is therefore refused with `--keep-changes` (the + > files it names are in the other checkout and do not come along) and with + > `--skip-branch` (git will not check the current branch out a second time in + > a new worktree). Both refusals happen in `OrcaArgs.parse`, before setup, the + > banner, or any git call. - **R5** — On **successful** exit the progress-log file is removed in a final commit, and a feature branch left with no changes other than the progress log is deleted (throwaway-branch cleanup). That removal commit is also pushed, but only diff --git a/adr/0019-project-stack-settings.md b/adr/0019-project-stack-settings.md index daa73f78f..2bc6dec44 100644 --- a/adr/0019-project-stack-settings.md +++ b/adr/0019-project-stack-settings.md @@ -113,6 +113,9 @@ flips to the safe direction: CACHEDIR.TAG # backup/sync tools skip the dir flow.lock lint-*.txt # spilled lint output + worktrees/ + .gitignore # written by orca, contains "*" (self-ignoring) + / # a --worktree run's own checkout ``` - Ephemeral state moves under `.orca/cache/`, which orca creates with a @@ -126,6 +129,16 @@ flips to the safe direction: `Lint`, and `ProgressStore` each create `.orca` independently, and `Lint` resolves it against `os.pwd` where the others use `workDir` — reconciled while moving). + + > **Amendment (2026-08-27).** `worktrees/` is the exception to this rule: it + > is ignored but not ephemeral, and deliberately sits OUTSIDE `cache/`. A + > `--worktree` run's checkout holds unmerged work, and after a failure the + > only copy of the progress log that resumes it — neither belongs under a + > `CACHEDIR.TAG` announcing the directory as safe to delete, which is why + > `worktrees/` gets the self-ignoring `.gitignore` and no tag. `OrcaDir` is + > the one helper that writes that marker, as for `cache/` above, and it + > writes it before the first worktree is created: without it, `git add -A` in + > the checkout stages the worktree as an embedded git repository. - The progress-log `forceAdd` **stays**. It is load-bearing, not a hack: in a repo that still gitignores `.orca/`, a plain stage `git add -A` would skip the log, and failure teardown's `reset --hard` would then leave an on-disk diff --git a/adr/0021-orca-shell.md b/adr/0021-orca-shell.md index 9c2ad8f38..a1d7da5a9 100644 --- a/adr/0021-orca-shell.md +++ b/adr/0021-orca-shell.md @@ -206,6 +206,35 @@ runtime's own glyph family (`⏺`/`●`/`▶`/`▸`). > fallback. Multiple unfinished logs (different prompts, one branch) offer > only the newest by mtime. +> **Amendment (2026-08-27).** The scan spans the checkout the shell was started +> in plus the worktrees orca made FOR THAT CHECKOUT — the ones under its own +> `.orca/worktrees/`, matching where the data lives, since `.orca/` is +> per-checkout. So the checkout that made the worktrees sees itself and all of +> them, while a worktree, whose own `.orca/worktrees/` is empty, sees only +> itself; surveying every run means running the shell from the checkout the +> worktrees hang off. A `--worktree` run leaves +> its progress log inside its own tree, so a shell scanning only its own +> directory would either offer nothing or offer an older run. The +> newest-by-mtime rule now orders across all of them, and a directory that +> cannot be read costs only its own logs. Discovery is git's own worktree list +> (`Worktrees.list`, resolved at the call site so the scan itself takes plain +> directories and stays testable without a repository), filtered to children of +> `.orca/worktrees/` and capped at the 20 most recently used (orca never removes +> a worktree, and these scans run per redraw) — a worktree checked out to review +> someone else's branch +> carries that branch's committed progress log, and its recorded task text is +> what the offer would hand an agent. The progress-log scan deliberately does +> NOT descend into `.orca/worktrees/`; it stays one level deep. +> +> The offer carries the directory its log was found in, names it in the menu +> label when it is not the shell's own, and the resume RUNS there. No +> shell-voice notice on top: the child recognises an orca worktree from its own +> working directory rather than from the flag, so its closing summary already +> names the tree and offers a `git -C` diff for it. Not `--worktree`: that flag re-derives a path from the task text, which +> is the directory holding the log only when the log was already in an orca +> worktree of that exact prompt — otherwise it would silently start a fresh run +> in a tree with no log, leaving the interrupted one behind. + > **Amendment (2026-08-01).** A `branch: ` line prints directly above > the menu prompt, re-read on every redraw (like Continue's manifest listing > and the resume check) so it stays true after a flow run switches branches — @@ -477,6 +506,21 @@ resume is global, but the resumed context still references that directory): | gemini | `gemini --list-sessions` → match uuid → `gemini --resume ` | medium — `--resume` takes latest/index, not uuid | | pi | `pi --session-dir /.orca/cache/pi-sessions/ --continue` | medium-high — orca writes the dir itself; inferred from pi's `--continue` semantics, not yet live-verified interactively | +> **Amendment (2026-08-27).** The listing spans the checkout the shell was +> started in plus the worktrees orca made for it (`WorktreeScan.dirs`, the same +> set the resume offer scans, §3): a `--worktree` run writes its manifests into +> its own tree, so a shell reading only its own `.orca/cache/runs/` would omit +> the sessions of a run it had just started. The newest-first order now runs +> across all of them. Containment is per directory as well as per file: the +> caller's own directory is read strictly (a symlinked `.orca` there aborts), +> while a failure in any other — unreadable, or removed mid-redraw — becomes one +> warning naming it rather than taking the listing down with it. Lineages are +> keyed on the manifest's `workDir` too, since flow session names are static and +> harness sessions are cwd-scoped: the same name in two worktrees is two +> conversations, and the rows say which tree each is in. Reattaching needs +> nothing further — resume already execs from the manifest's recorded +> `workDir`, which is the worktree. + > **Amendment (2026-07-28).** Pi is now durable (ADR 0018 §2.6); before the argv is > built, resume applies the same resumability predicate as the backend's own probe > (`PiSessionStore.resumable`: a `*.jsonl` whose transcript header matches the @@ -851,6 +895,16 @@ the same thing (`--global`, `--json`, `--yes`). `create`/`fork` run the built-in authoring flow (§9) with the configured role agents — no harness/model/yolo flag exists for either. +> **Amendment (2026-08-27).** `run` gains `--worktree`, passed straight +> through to the flow child like `--verbose`/`--skip-branch`/`--keep-changes`. +> `RunCli` also refuses the two contradictory pairs (`--worktree` with +> `--skip-branch`, and with `--keep-changes`) itself, exiting 2 before any +> `scala-cli` spawn — the child refuses them too and stays the authority, but +> the shell holds the answer already, and spawning only to be told costs a +> dependency resolution. Both sides call the one shared decision +> (`OrcaArgs.worktreeRefusal`), so neither the wording nor the set of refused +> pairs can drift. + Both entry points call a shared `orca.shell.actions` package (`FlowResolution`, `RunAction`, `ViewAction`, `EditAction`, `AuthorAction`, `SessionAction`, `ConfigAction`, `StackAction`): each takes fully-resolved parameters and does diff --git a/docs/plans/worktree-runs/implementation-plan.md b/docs/plans/worktree-runs/implementation-plan.md new file mode 100644 index 000000000..e474fef5e --- /dev/null +++ b/docs/plans/worktree-runs/implementation-plan.md @@ -0,0 +1,335 @@ +# Worktree runs — implementation plan + +Expands [specification.md](specification.md). All paths repo-relative. Planned +against `master` @ `c0dad27a`; every file/line reference was re-read there, and +every git behaviour asserted below was verified against git 2.53 in a scratch +repository (see "Verified git behaviour"). + +## Landing order + +**PR 1 first; PR 2 and PR 3 then in either order.** PR 1 adds the flag and the +resolution, and nothing works without it. PR 2 (closing summary) and PR 3 +(shell) touch disjoint files. + +PR 2 and PR 3 each fix a way the feature is *incomplete* rather than broken — +after PR 1 a worktree run works, but does not say where it went (PR 2) and is +only reachable by typing the flag on a flow script (PR 3). Neither is +optional; both are separable. + +## ADRs + +No new ADR. Three amendments, each because the existing text states something +this plan makes untrue: + +- **ADR 0019 §"`.orca/` flips to committed-by-default"** (`:102–141`) — the + directory listing at `:111` gains `worktrees/`, and the paragraph at `:118` + ("ephemeral state moves under `.orca/cache/`") needs the exception stated: + worktrees are ignored but not ephemeral, and deliberately sit outside + `cache/`. Same amendment names `OrcaDir` as the one helper that writes the + new directory's marker, consistent with `:125`. +- **ADR 0018 §2.5** (`:344`) — documentation only; no code under it changes. + One dated amendment recording that a worktree run always meets the + dirty-tree policy's clean case, and that `--worktree` is refused with + `--skip-branch` and with `--keep-changes` before setup ever runs. +- **ADR 0021 §10** (`:834`) — the `orca run` flag table gains `--worktree`; + §3 (`:160`) and §8 (`:399`) gain the "scan every worktree" rule for the + resume offer and the session listing. + +--- + +# PR 1 — `--worktree`: create or reuse the run's worktree + +## Behavior decisions + +**The git plumbing goes in `tools`, not on `GitTool`.** `GitTool` is the +flow-facing tool trait; worktree operations were deleted from it in the +2026-08 review precisely because no flow can use them, and nothing here +changes that — resolution runs before any tool exists. New object +`orca.tools.Worktrees` (`tools/src/main/scala/orca/tools/Worktrees.scala`), +`private[orca]`, shelling out through `orca.subprocess.QuietProc` the way +`ConfigSummary` already does for `git rev-parse`. Two reads and one write: + +- `mainCheckout(cwd): Option[os.Path]` — `git rev-parse + --path-format=absolute --git-common-dir`, then its parent. The + `--path-format=absolute` is required: the bare form returns a path relative + to the main checkout, which is useless from inside a linked worktree. `None` + outside a git repository. +- `list(cwd): List[os.Path]` — the paths from `git worktree list --porcelain`, + empty outside a git repository. Paths are all any caller needs: PR 1 + distinguishes reuse from refusal by testing the path on disk, and PR 3's + scans read each path's `.orca/`. No branch or `prunable` field — nothing + consumes one. +- `add(cwd, path)` — `git worktree add --detach `, with no commit-ish: + git then detaches at `cwd`'s own HEAD, which is exactly the specified start + point, so there is nothing to resolve first. No `--` separator: the path is + absolute and its last segment is a 12-hex hash, so it cannot be read as a + flag. + +**Run-level policy stays in `runner`.** New object `orca.runner.WorktreeRun` +(`runner/src/main/scala/orca/runner/WorktreeRun.scala`): derive the path, +create-or-reuse, and produce the resolved directory. It owns the only +knowledge of how the path is built, so PR 3's relaunch does not re-derive it. + +**The path.** `OrcaDir.worktreesPath(mainCheckout) / hash`, where `hash` is +`ProgressStore.hashPrompt(args.userPrompt)` — the identical call +`ProgressStore.default` makes at `flow/.../ProgressStore.scala:71`, so the +worktree and the progress log inside it always carry the same hash. +`hashPrompt` is `private[orca]` and `runner` is `package orca`, so no +visibility change is needed. + +**`OrcaDir` gains the directory and its marker.** +`tools/src/main/scala/orca/OrcaDir.scala`: + +- `worktreesPath(workDir)` — passive, `root(workDir) / "worktrees"`, + alongside `runsPath` (`:111`). +- `ensureWorktrees(workDir)` — `abortIfOrcaComponentSymlink`, `makeDir.all`, + then `writeIfAbsent(dir / ".gitignore", gitignoreContents)`, mirroring + `ensureCache` (`:72–79`) minus the `CACHEDIR.TAG`. The tag is deliberately + absent: it tells backup tools to skip the directory, and a worktree holds + unmerged work. Say that in the scaladoc, or someone will add it for + symmetry. + +The marker is not optional. Without it `git add -A` in the main checkout +stages the worktree as an embedded repository — verified, see below — so +`ensureWorktrees` must run before `Worktrees.add`, not after. + +**Resolution happens in `flow()`, not `runFlow`.** This is forced by +`RunManifestWriter.start` at `runner/.../flow.scala:172`, which is in `flow()` +and takes `workDir`: resolving inside `runFlow` would write the run's session +manifests into the *invoking* checkout while the run itself happened in the +worktree, and PR 3's `orca continue` would then find them under the wrong +directory. `runFlow` keeps no worktree logic and its `workDir` parameter is +already-resolved for every caller, tests included. + +Concretely, in `flow()` (`runner/.../flow.scala:144–178`): + +- Resolve after `installUncaughtExceptionHandler()` (`:154`) and before + `RunManifestWriter.start` (`:172`). +- `flowLog.info` at `:150` logs the *resolved* directory, and says when it is + a worktree; move it below the resolution. +- A resolution failure has no dispatcher and no manifest, so it must reach + `flow()`'s stderr backstop and exit 1 — the same treatment the `NonFatal` + catch at `:214–216` gives a pre-dispatcher failure. `RunManifestWriter.start` + is *not* inside that try today, so simply throwing before it escapes + uncaught. Shape: have `WorktreeRun.resolve` return + `Either[String, os.Path]`, and on `Left` print `[orca] `, set + `outcome = FlowOutcome.Failed`, and skip the `supervised:` block — + which requires hoisting `orcaLog.finish()` out of the inner `finally` + (`:223`) so it still runs on that path. The trailing + `if outcome == FlowOutcome.Failed then System.exit(1)` (`:229`) then needs + no change. +- `flow()`'s scaladoc documents `workDir` as "the default is the current + directory"; it gains the `--worktree` exception. +- Pass the resolved directory as `runFlow`'s `workDir`. Inside `runFlow` + nothing changes: every downstream consumer already takes `workDir` + explicitly (`:274` lock, `:297` store, `:304` agent wiring, `:311/:313/:315` + git/gh/fs, `:318` context). + +**The flag.** `runner/src/main/scala/orca/OrcaArgs.scala` gains +`worktree: Flag = Flag()` with doc "run the flow in a git worktree of this +repository instead of the current checkout". + +**The refusals live in `OrcaArgs.parse`** (`:23–26`), not in `WorktreeRun`. +`parse` already returns `Either[String, OrcaArgs]` and `from` already turns a +`Left` into `OrcaFlowException`, so a contradictory flag pair fails at +`OrcaArgs(args)` — before `flow()` runs, before the banner, before anything +touches git. Two messages, each naming both flags and why they conflict: +`--worktree` with `--skip-branch` (git will not check the current branch out +twice), `--worktree` with `--keep-changes` (uncommitted files stay in the +invoking checkout). + +**Not a repository, or no commits yet.** Resolution runs before +`FlowLifecycle.abortIfNoCommits` (`:386`, called at `:331`), so with +`--worktree` those two cases now surface here instead. `resolve` returns a +`Left` for each — `mainCheckout` returning `None` for the first, `add` failing +for the second — reusing `abortIfNoCommits`' wording for the unborn-HEAD case +rather than passing git's raw error through. + +**Reuse and refusal.** `WorktreeRun.resolve` on an existing derived path: +registered worktree of this repository → return it; anything else (a plain +directory, a worktree of a different repository, a file) → `Left` naming the +path and saying orca will not take over a directory it did not create. The +`prunable` case — the directory is gone but the admin entry survives, which +`git clean -xdff` produces — is a `list` entry whose path does not exist: +treat as absent and let `Worktrees.add` run, which prunes and recreates. + +## Verified git behaviour + +Checked against git 2.53 with a nested worktree at +`main/.orca/worktrees/`; these are the facts the code above depends on. + +| Behaviour | Result | +|---|---| +| `git worktree add --detach .orca/worktrees/` inside the tree | allowed, no warning | +| `git worktree add --detach ` with no commit-ish | detaches at the invoking checkout's HEAD | +| `git status` in main, no ignore file | `?? .orca/worktrees/` | +| `git add -A` in main, no ignore file | stages it as a gitlink, `warning: adding embedded git repository` | +| with `.orca/worktrees/.gitignore` = `*` | `git status` empty, `git add -A` stages nothing | +| `git add -A -- . ':(exclude).orca/*'` (orca's own commits) | worktree never staged, ignore file or not | +| `git clean -xdf` in main | `Skipping repository .orca/worktrees/`; removes only the `.gitignore` | +| `git clean -xdff` in main | removes it; admin entry survives, listed `prunable` | +| `git worktree remove .orca/worktrees/` | removes tree and admin entry | +| `git rev-parse --path-format=absolute --git-common-dir` | `
/.git` from main *and* from the worktree | +| `.orca/` inside the worktree | committed content only; `worktrees/` absent | +| checking out the worktree's branch in main | `fatal: '' is already used by worktree at ...` | + +## Tests + +- `tools/src/test/scala/orca/tools/WorktreesTest.scala` (new) — `add` then + `list` round-trip in a temp repo; `mainCheckout` from both the main checkout + and the linked worktree returning the same path; a `--`-requiring path + (leading `-`) not being read as a flag. +- `runner/src/test/scala/orca/runner/WorktreeRunTest.scala` (new) — derives + the same path for the same task text and a different one for different text; + reuses an existing registered worktree; refuses a plain directory at the + path; treats a prunable entry as absent; writes the `.gitignore` before + creating. +- `runner/src/test/scala/orca/OrcaArgsTest.scala` — the two refusals, and + `--worktree` alone parsing. +- `runner/src/test/scala/orca/runner/FlowLifecycleTest.scala` — one end-to-end + run with `--worktree`: work lands on a branch in the worktree, the main + checkout's branch and tree are untouched, the progress log is inside the + worktree, and a second run with the same task text resumes in the same + worktree. It must drive `flow(...)`, not `runFlow(...)` — resolution lives in + `flow()`, so a `runFlow` test would pass an already-resolved directory and + exercise none of it. The file already has both styles (`flow(` at `:76`). + +## Docs + +`README.md:63` (the useful-flags sentence) and `README.md:954` (the `orca run` +flag row) gain `--worktree`. The prose block at `README.md:327–338`, which +explains `--skip-branch`/`--keep-changes` against the progress header, gains a +paragraph: what a worktree run isolates, that uncommitted work does not come +with it, that the first run in a worktree pays a cold build, that an editor or +indexer not honouring `.gitignore` will see the second checkout, and that +cleanup is `git worktree remove`. + +--- + +# PR 2 — Say where the run left the user + +## Behavior decisions + +**`ClosingSummary` learns about the worktree.** +`runner/src/main/scala/orca/runner/ClosingSummary.scala:34`, `lines(branch, +changes)`, gains one parameter: `worktree: Option[os.Path]`, `Some` only for a +worktree run. `None` reproduces today's output byte-for-byte — every existing +case keeps passing unchanged. + +For a worktree run the first line names both the path and the branch inside +it, per the specification, and the `next:` line becomes `git -C diff +` so it works from where the user actually is. `` keeps its +existing two shapes (``, or `..` when HEAD left the +counted branch) — the `-C` is the only change to it. + +**Caller.** `FlowLifecycle.teardownSuccess` (`:1147`) emits the summary at +`:1191`. It has `setup: FlowSetup` and `git`, but not the run directory — +`FlowSetup` (built in `setup`, `:297–376`) does not carry `workDir` today. Add +a single `worktree: Option[os.Path]` field there rather than a directory plus a +Boolean, and rather than a second `teardownSuccess` parameter: `setup` already +receives both `args` (`:298`) and `workDir` (`:303`), so the field is +`Option.when(args.worktree.value)(workDir)` computed at one site. + +**Failure path.** A failed run prints no closing summary today and should not +start; the worktree path reaches the user through the abort message only if the +failure is the resolution itself, which PR 1 already words. No change. + +## Tests + +`runner/src/test/scala/orca/runner/` — a `ClosingSummary` case per shape: +non-worktree unchanged, worktree with changes, worktree with none. The +end-to-end assertion belongs with PR 1's `FlowLifecycleTest` case; extend it +here to assert the emitted `Step` names the worktree. + +--- + +# PR 3 — Shell: the flag, the prompt, and worktree-wide scans + +## Behavior decisions + +**`FlowFlags` gains `worktree`.** +`shell/src/main/scala/orca/shell/run/FlowLauncher.scala:30` — the bundle +exists precisely so a new flag is one field rather than another positional +Boolean. Threaded through `argv` (`:97`): a `worktreeArgs` alongside +`verboseArgs`/`skipBranchArgs`/`keepChangesArgs` (`:108–111`), appended after +`--` at `:117` with the others. Four construction sites update: +`Cli.scala:165`, `Main.scala:365`, `Main.scala:407`, and +`AuthorAction.scala:140` (which passes all-false and stays all-false — +authoring runs in a sandbox, not the user's repo). + +**`orca run --worktree`.** `Cli.scala`'s `run` (`:144`) gains the flag next to +`honorPin` (`:156–160`), passed into `FlowFlags`. + +**The refusals get a shell fast path.** `RunCli.run` already returns +`ExitCodes.UsageError` for bad input. Rejecting `--worktree --skip-branch` and +`--worktree --keep-changes` there saves a `scala-cli` spawn and a dependency +resolution before the child says the same thing. The child's own refusal (PR 1) +stays as the authority; this only makes it fast. Same wording in both places. + +**Interactive prompt.** `Main.runFlow` (`:356`) asks for the flow, the task, +then whether to create a branch (`promptCreateBranch`, `:435`). A worktree +question goes after it, and only when the branch answer was yes — "run in a +worktree" and "run on the current branch" are the incompatible pair, so asking +both independently invites the user to pick a combination orca refuses. + +**The two scans take the directories, they do not discover them.** Both +`ResumeDetector.detect` and `ManifestReader.list` are today driven by tests +that seed a bare temp directory — no git repository in sight. If either starts +calling `git worktree list` internally, those tests break for a reason that has +nothing to do with what they assert. So each takes the directories to scan, and +one small helper (`Worktrees.list(os.pwd)`, falling back to just `os.pwd` when +it comes back empty) resolves them at the two `Main`/`Cli` call sites. This is +the seam convention `Cli`'s own scaladoc already states: explicit `workDir` +params rather than reading `os.pwd` inside. + +**Resume relaunches into the worktree.** +`shell/src/main/scala/orca/shell/resume/ResumeDetector.scala` currently reads +one directory (`detect(workDir)`, `:38`); it takes the list instead, keeping +its newest-by-mtime rule across all of them. `InterruptedRun` (`:8`) gains a +flag saying the log was found in a worktree. `Main.resumeInterruptedRun` +(`:386`) then passes `worktree = true` in the `FlowFlags` it builds at `:407`, +and the derived path takes the relaunch back to the same worktree. The +existing comment at `:378–384` explaining why the other flags are all-false +needs the new flag's *different* reason recorded: it is not inert, it is what +makes the resume land in the right tree. + +**Session listing spans worktrees.** `ManifestReader.list(workDir, pidAlive)` +(`sessions/ManifestReader.scala:30`) reads one `.orca/cache/runs/`; it takes +the list instead, keeping its per-file warning behaviour and its newest-first +sort, which now orders across directories. Its two callers — +`ContinueCli.runContinue` (whose `workDir` is already an explicit test seam) +and `Main.loop` (`:115`) — resolve the list at their call site. +`SessionAction.resume` already uses each manifest's recorded `workDir` +(`:100`), so nothing about reattaching changes. + +**Do not scan by nesting.** `ProgressScan.progressLogPaths` +(`flow/.../ProgressScan.scala:26`) lists `.orca/` one level deep and filters to +files, so it does not descend into `worktrees/` — which is correct and must +stay that way. Discovery goes through `git worktree list`, so it also finds +worktrees outside the repository (someone's pre-existing `../feature-x`), not +only the ones orca made. + +**Not changed: the menu's `branch:` line.** `ConfigSummary.branchLine` +(`actions/ConfigSummary.scala:58`) reads the shell's own checkout, whose branch +a worktree run never touches, so it stays true. + +## Tests + +- `shell/src/test/scala/orca/shell/run/FlowLauncherTest.scala` — `--worktree` + lands after `--`, with the other flow flags. +- `shell/src/test/scala/orca/shell/cli/CliTest.scala` — the flag parses; the + two fast-path refusals return `UsageError`. +- `shell/src/test/scala/orca/shell/resume/ResumeDetectorTest.scala` — a log in + a linked worktree is detected and reported as a worktree run; the existing + main-checkout cases keep passing. +- `shell/src/test/scala/orca/shell/sessions/ManifestReaderTest.scala` — + manifests from two worktrees appear in one listing, newest-first across both. +- `shell/src/test/scala/orca/shell/actions/RunActionTest.scala` — the + interactive path's flag reaches the launcher. + +## Docs + +ADR 0021 §10's flag table, §3's resume-offer description, and §8's session +listing, per the ADRs section above. `README.md:954` is PR 1's edit; no second +README change is needed here beyond the interactive-shell walkthrough if it +enumerates the run prompts. diff --git a/docs/plans/worktree-runs/specification.md b/docs/plans/worktree-runs/specification.md new file mode 100644 index 000000000..289fb0291 --- /dev/null +++ b/docs/plans/worktree-runs/specification.md @@ -0,0 +1,193 @@ +# Worktree runs — specification + +Issue [#150](https://github.com/VirtusLab/orca/issues/150) asks for a way to +run a flow in a git worktree, so several runs can work on one repository at +once without sharing a checkout. A `git.createWorktree` tool inside a flow +cannot deliver that: a flow's working directory is fixed before its body runs, +so a worktree created mid-flow is a directory nothing moves into. Isolation has +to be decided before the run starts. + +This specification adds a run-level `--worktree` flag. Orca creates the +worktree at startup, runs the whole flow inside it, and leaves it in place when +the run ends. + +## Where the run happens + +`--worktree` takes no value. The worktree's location is derived, not chosen: + +
/.orca/worktrees/ + +`promptHash` is the same 12 hex chars that already key the run's progress log +(`.orca/progress-.json`), so one task means one worktree for the lifetime +of that task. Nothing about the path comes from the task text itself — task +text is untrusted input, and a hash is the established safe key. + +The `
` is found from git, not from the current directory: +`git rev-parse --path-format=absolute --git-common-dir` names the main +checkout's `.git` from inside any worktree, so its parent is the main checkout +wherever the flag is given. The path is therefore identical whether the run +starts from the main checkout or from a worktree, and running `--worktree` +with the same task from inside the worktree it already created resolves to +that worktree and reuses it. There is no recursion to guard against. + +Worktrees live inside the repository rather than beside it because the +alternative — a sibling directory — puts a directory orca creates, and never +removes, in a place orca does not own. `.orca/worktrees/` is namespaced, +always writable, and goes away when the clone does. + +They live at `.orca/`'s root, not under `.orca/cache/`, because the cache +carries a `CACHEDIR.TAG` announcing itself as disposable and skippable by +backup tools. A worktree holds committed work that may not be merged anywhere, +and — after a failure — the only copy of the progress log that resumes the +run. Neither belongs anywhere advertised as safe to delete. + +The location is not configurable. A settings key for a worktree root can be +added when someone needs one; until then there is one rule to learn and +nothing to misconfigure. + +## Hiding the worktree from the enclosing repository + +`.orca/worktrees/` gets the self-ignoring `.gitignore` that `.orca/cache/` +already gets, written before the first worktree is created and rewritten +whenever it is missing, exactly as `OrcaDir.ensureCache` does today. + +This is load-bearing, not cosmetic. Without it, `git add -A` in the main +checkout stages the worktree as an embedded git repository — a gitlink to a +commit nothing can resolve — with git's own `warning: adding embedded git +repository` as the only sign. With it, the directory is invisible: `git +status` reports nothing and `git add -A` stages nothing. + +Orca's own stage commits are unaffected either way: they commit through the +`:(exclude).orca/*` pathspec, which already excludes the whole directory. + +## Creating and reusing + +At startup, with `--worktree` given: + +- **Nothing at the derived path** — create it, detached, at the current HEAD + of the invoking checkout, then immediately put it on a branch named + `orca-worktree-`. It is created detached because orca names the + run's *feature* branch with a model call during setup, which happens after + the worktree already exists — but it cannot be left detached: a detached + worktree records the literal `HEAD` as the run's starting branch, which + recovery then refuses as an unsafe ref, so an interrupted worktree run could + never be resumed. The feature branch is created on top of + `orca-worktree-` by the normal branch binding, exactly as it is + for a non-worktree run. Orca will not move that branch once it has gained + commits; a re-run of the task refuses instead. +- **A registered worktree of this repository at that path** — reuse it. This + is what makes a resumed run land back where its progress log is. +- **A directory that is not a registered worktree of this repository** — + refuse, naming the path. Orca does not delete or take over a directory it + did not create. + +Everything downstream follows the resolved directory: git, the filesystem +tool, every agent subprocess, the stack commands, the progress log, the run +lock, and the session manifests. All of them are already given the run's +working directory explicitly, so there is one value to change and no process +working directory to move. + +Uncommitted work in the invoking checkout does **not** come along. A worktree +is created from a commit. That is the point of the isolation, and it is the +first thing the documentation has to say. + +## What the flag refuses + +Two combinations are errors, refused before anything is created: + +- **`--worktree --skip-branch`.** `--skip-branch` binds the run to the branch + that is checked out now; git will not check that branch out a second time in + a new worktree. There is no useful reading of the pair. +- **`--worktree --keep-changes`.** `--keep-changes` asks orca to leave + uncommitted files in place for the flow to work on. Those files are in the + other checkout and are not carried over, so honouring the flag would be a + lie. A fresh worktree is clean by construction, which also means the + dirty-tree prompt never fires for a worktree run. + +## Resuming + +Resume is unchanged in mechanism and unchanged in what the user does: run the +same flow with the same task text, and add `--worktree` again. The task +resolves to the same worktree, the worktree holds the progress log on the +run's branch, and stage replay proceeds as it does for any other run. + +Two shell features currently look only at the directory they were started in, +and would silently stop working for worktree runs: + +- **"Resume interrupted run"** scans for an unfinished progress log. It must + scan every worktree of the repository, and relaunch a run found in one with + `--worktree` set, so the relaunch returns to it. +- **`orca continue`** lists recorded sessions from `.orca/cache/runs/`. Each + worktree has its own, so the listing must span every worktree of the + repository. Resuming a session already uses the working directory recorded + in its manifest, so once a session is *found*, reattaching to it works + without further change. + +Both use git's own worktree list as the source of truth. Neither can rely on +the nesting: the existing progress-log scan lists `.orca/` one level deep and +filters to files, so it does not — and must not start to — descend into +`worktrees/`. + +## Telling the user where the run left them + +A run that ends leaves the user's shell where it started — in the invoking +checkout, not in the worktree. Today's closing summary says "done — you are on +branch X" and offers `git diff `; for a worktree run both are wrong. It +must name the worktree path and the branch inside it, and offer a command the +user can actually run from where they are standing. + +The shell's flow-finished notice needs the same treatment. The main menu's +`branch:` line does not: it reads the checkout the shell is running in, whose +branch a worktree run never changes, so it stays true. What it cannot say is +that work landed elsewhere — the flow-finished notice is where that belongs. + +## Cost of living inside the repository + +A worktree is a full second checkout of the project, including a second copy +of its build definition, sitting inside the project directory. Tools that +honour `.gitignore` — ripgrep, fd, most language servers — skip it. Tools that +do not — some IDE indexers, `find`, broad glob patterns, file watchers — will +see the duplicate. There is no clean mitigation: a `CACHEDIR.TAG` would keep +some of them out, but it also tells backup tools to skip the directory, which +is exactly wrong for one holding unmerged work. + +Repository size grows by a checkout per concurrent worktree, and does so +invisibly from outside the project directory. + +## Cold build trees + +A fresh worktree has no build outputs, no installed dependencies, and none of +the untracked local configuration a project may need. The stack commands +(format, lint, test) run there, so the first worktree run pays a cold build, +and a project that cannot build without untracked local files will fail in a +worktree and succeed outside one. + +This is documented, not solved. A post-creation setup hook can be added later +if it turns out to be needed. + +## Lifetime + +Orca never removes a worktree, and never removes the +`orca-worktree-` branch it puts that worktree on. A failed run's +worktree holds the progress log that resumes it, and a successful run's +worktree holds the branch with the work on it — in both cases removing it +would destroy something the user still wants. Full cleanup is therefore two +commands, `git worktree remove` (which works normally on the nested path) and +`git branch -D orca-worktree-`; the closing summary is where the +user learns the path to hand the first one. + +Nothing else deletes it by accident. `git clean -xdf` in the main checkout +skips it, reporting `Skipping repository .orca/worktrees/` — git treats +it as a repository boundary. Only `git clean -xdff` removes it, and that +leaves the worktree's administrative entry behind as `prunable`. A plain +`git clean -xdf` does delete the `.gitignore` under `.orca/worktrees/`, which +un-hides the directory until the next run rewrites the marker. + +## Concurrency + +Each worktree has its own `.orca/cache/`, and the run lock is keyed on the run +directory, so two runs in two worktrees of one repository do not contend — +which is the isolation the issue asks for. The one shared resource is the +branch namespace: a run cannot create a branch checked out in another +worktree, and git says so plainly (`fatal: '' is already used by +worktree at ...`), which orca surfaces as-is. diff --git a/runner/src/main/scala/orca/OrcaArgs.scala b/runner/src/main/scala/orca/OrcaArgs.scala index d7daa0d69..d7e962a28 100644 --- a/runner/src/main/scala/orca/OrcaArgs.scala +++ b/runner/src/main/scala/orca/OrcaArgs.scala @@ -13,15 +13,60 @@ case class OrcaArgs( @arg(doc = "keep uncommitted/untracked files in the working tree instead of stashing them (fresh runs only)" ) - keepChanges: Flag = Flag() + keepChanges: Flag = Flag(), + @arg(doc = + "run the flow in a git worktree of this repository instead of the current checkout" + ) + worktree: Flag = Flag() ) object OrcaArgs: given ParserForClass[OrcaArgs] = ParserForClass[OrcaArgs] - /** Parse the given argv or return a human-readable error. */ + private val worktreeWithSkipBranchRefusal: String = + "--worktree cannot be combined with --skip-branch: --skip-branch runs on " + + "the branch checked out now, and git will not check that branch out a " + + "second time in a new worktree" + + private val worktreeWithKeepChangesRefusal: String = + "--worktree cannot be combined with --keep-changes: --keep-changes works " + + "on uncommitted files, which stay behind in the invoking checkout — a " + + "worktree is created from a commit and starts clean" + + /** Why these flags cannot be combined, or `None` when they can — asked of a + * whole `OrcaArgs`, so neither [[parse]] nor `flow()` spells out a triple of + * same-typed booleans that a transposition would silently reorder. + */ + private[orca] def worktreeRefusal(args: OrcaArgs): Option[String] = + worktreeRefusal( + worktree = args.worktree.value, + skipBranch = args.skipBranch.value, + keepChanges = args.keepChanges.value + ) + + /** The same question over loose booleans, for the shell — it holds + * `FlowFlags`, not an `OrcaArgs`, and refuses the pair before it spawns a + * flow at all. What must not drift is which pairs are refused, not only how + * each refusal reads. + */ + private[orca] def worktreeRefusal( + worktree: Boolean, + skipBranch: Boolean, + keepChanges: Boolean + ): Option[String] = + if !worktree then None + else if skipBranch then Some(worktreeWithSkipBranchRefusal) + else if keepChanges then Some(worktreeWithKeepChangesRefusal) + else None + + /** Parse the given argv or return a human-readable error — including for a + * contradictory `--worktree` pair, refused here so it fails at parse, before + * the banner and before anything touches git. + */ def parse(args: Seq[String]): Either[String, OrcaArgs] = - summon[ParserForClass[OrcaArgs]].constructEither(args.toList) + summon[ParserForClass[OrcaArgs]] + .constructEither(args.toList) + .flatMap(parsed => worktreeRefusal(parsed).toLeft(parsed)) /** Overload for scala-cli flow scripts, whose top-level `args` is * `Array[String]`. Throws `OrcaFlowException` on a parse failure. diff --git a/runner/src/main/scala/orca/flow.scala b/runner/src/main/scala/orca/flow.scala index 2be4bc18e..69d8106c0 100644 --- a/runner/src/main/scala/orca/flow.scala +++ b/runner/src/main/scala/orca/flow.scala @@ -32,7 +32,8 @@ import orca.runner.{ RoleAgents, RoleOverrides, SurfacedFlowFailure, - WiredAgents + WiredAgents, + WorktreeRun } import orca.runner.manifest.{RunManifestWriter, RunOutcome} import orca.settings.GlobalSettings @@ -113,6 +114,16 @@ private enum FlowOutcome: * passed, the project file's stack keys are ignored and discovery is skipped, * but its agent keys are still honoured (a malformed file still aborts). * + * '''`--worktree`.''' `workDir` is where the run starts looking, not always + * where it happens: with `--worktree` the run moves into + * `.orca/worktrees/` of this repository, created on first use and + * reused after, and everything below it — git, the progress log, the session + * manifest — uses that directory instead. A refusal (no repository, no + * commits, something orca did not create already at the path) ends the run + * before any of that starts. `--worktree` cannot be combined with `skipBranch` + * or `keepChanges`; the pair is refused here as well as in `OrcaArgs.parse`, + * since an `OrcaArgs` built by hand never passed through the parser. + * * Overrides default to `None` so the runtime can build the default lazily — * `TerminalInteraction` in particular takes the resolved `workDir`, which * can't be threaded through a default-arg expression. @@ -147,80 +158,119 @@ def flow( val orcaLog = OrcaLog.start() OrcaBanner.print(System.err, orcaLog.file) val flowLog = LoggerFactory.getLogger("orca.flow") - flowLog.info("orca {} starting (workDir={})", OrcaBanner.version, workDir) flowLog.info("user prompt: {}", args.userPrompt) // A daemon thread or unsupervised fork that throws would otherwise disappear // with no diagnostic; this leaves a trail on the console and in the trace. installUncaughtExceptionHandler() // Tally token usage and print the summary on exit (success or failure). val costTracker = new CostTracker(pricing.lastUpdated) - // `try/finally` so the cost summary always lands — even when a fatal - // throwable (OOM, StackOverflow) escapes the NonFatal catch below. See - // `FlowOutcome`'s scaladoc for why this is one ADT, not two booleans. - var outcome = FlowOutcome.Running - // The manifest writer's actor fork lives in this scope, spanning its - // construction through `finish`; `System.exit` below stays OUTSIDE it. A - // nested `flow()` call gets its own scope and writer. - // Read once and threaded explicitly from here down (RunManifestWriter, - // and the progress header via `runFlow`/`FlowLifecycle.setup`) rather than + // Read once and threaded explicitly from here down (RunManifestWriter, and + // the progress header via `runFlow`/`FlowLifecycle.setup`) rather than // re-read with `sys.env` at each site. val flowName = sys.env.get("ORCA_FLOW_NAME") - supervised: - // Per-run session manifest (ADR 0021 §8), always attached like - // LoggingListener; see RunManifestWriter's scaladoc for `flowName`'s - // ORCA_FLOW_NAME sourcing. - val manifestWriter = RunManifestWriter.start( - workDir, - OrcaBanner.version, - flowName, - () => java.time.Instant.now() - ) - try + + // Where the run happens. This settles before the directory's first consumer, + // the session manifest below; everything downstream is handed `workDir` + // explicitly, so it is the single value to change. + def resolveRunDir(): Either[String, os.Path] = + OrcaArgs + .worktreeRefusal(args) + .toLeft(()) + .flatMap: _ => + if !args.worktree.value then Right(workDir) + else + // Resolution can throw as well as refuse — a symlinked or unwritable + // `.orca`, a git that won't start. One `Left` shape for every outcome + // keeps the reporting below the only way out. + try WorktreeRun.resolve(workDir, args.userPrompt) + catch case NonFatal(e) => Left(TextUtil.throwableMessage(e)) + + // The run proper. Everything under here uses `dir`, never `workDir`. + def runIn(dir: os.Path): FlowOutcome = + supervised: + // Per-run session manifest (ADR 0021 §8), always attached like + // LoggingListener; see RunManifestWriter's scaladoc for `flowName`'s + // ORCA_FLOW_NAME sourcing. Its actor fork lives in this scope, spanning + // construction through `finish`; the `System.exit` at the end of `flow()` + // stays OUTSIDE it, and a nested `flow()` gets its own scope and writer. + val manifestWriter = RunManifestWriter.start( + dir, + OrcaBanner.version, + flowName, + () => java.time.Instant.now() + ) + var outcome = FlowOutcome.Running + // `try/finally` so the cost summary always lands — even when a fatal + // throwable (OOM, StackOverflow) escapes the NonFatal catch below. try - runFlow( - args = args, - workDir = workDir, - interaction = interaction, - extraListeners = extraListeners ++ List(costTracker, manifestWriter), - branchNaming = branchNaming, - stackSettings = stackSettings, - planningAgent = planningAgent, - codingAgent = codingAgent, - reviewAgent = reviewAgent, - returnToStartBranch = returnToStartBranch, - progressStore = progressStore, - flowName = flowName, - pricing = pricing, - wiring = FlowWiring( - claude = claude, - codex = codex, - opencode = opencode, - pi = pi, - gemini = gemini, - git = git, - gh = gh, - fs = fs, - prompts = prompts + try + runFlow( + args = args, + workDir = dir, + interaction = interaction, + extraListeners = + extraListeners ++ List(costTracker, manifestWriter), + branchNaming = branchNaming, + stackSettings = stackSettings, + planningAgent = planningAgent, + codingAgent = codingAgent, + reviewAgent = reviewAgent, + returnToStartBranch = returnToStartBranch, + progressStore = progressStore, + flowName = flowName, + pricing = pricing, + wiring = FlowWiring( + claude = claude, + codex = codex, + opencode = opencode, + pi = pi, + gemini = gemini, + git = git, + gh = gh, + fs = fs, + prompts = prompts + ) + )(body) + outcome = FlowOutcome.Succeeded + catch + // A `SurfacedFlowFailure` marks a failure already reported to the + // user's event surface by the phase that raised it; only the exit + // code remains. + case _: SurfacedFlowFailure => outcome = FlowOutcome.Failed + // Backstop for any other NonFatal — a pre-dispatcher failure (agent + // factory, TerminalInteraction start) has no event surface, so print + // it to stderr rather than exit 1 in silence. + case NonFatal(e) => + outcome = FlowOutcome.Failed + System.err.println(s"[orca] ${TextUtil.throwableMessage(e)}") + outcome + finally + manifestWriter.finish( + if outcome == FlowOutcome.Succeeded then RunOutcome.Succeeded + else RunOutcome.Failed + ) + costTracker.printSummary() + + // Resolution runs inside this bracket, not before it: it can fail, and the + // trace still has to close on a path that never reaches `runIn`. + val outcome = + try + resolveRunDir() match + // A refusal has no dispatcher and no manifest to carry it, so it + // reaches the user the way the NonFatal backstop above does. + case Left(message) => + System.err.println(s"[orca] $message") + FlowOutcome.Failed + case Right(dir) => + val where = + if args.worktree.value then s"$dir (worktree)" else dir.toString + flowLog.info( + "orca {} starting (workDir={})", + OrcaBanner.version, + where ) - )(body) - outcome = FlowOutcome.Succeeded - catch - // A `SurfacedFlowFailure` marks a failure already reported to the user's - // event surface by the phase that raised it; only the exit code remains. - case _: SurfacedFlowFailure => outcome = FlowOutcome.Failed - // Backstop for any other NonFatal — a pre-dispatcher failure (agent - // factory, TerminalInteraction start) has no event surface, so print it - // to stderr rather than exit 1 in silence. - case NonFatal(e) => - outcome = FlowOutcome.Failed - System.err.println(s"[orca] ${TextUtil.throwableMessage(e)}") - finally - manifestWriter.finish( - if outcome == FlowOutcome.Succeeded then RunOutcome.Succeeded - else RunOutcome.Failed - ) - costTracker.printSummary() - orcaLog.finish() + runIn(dir) + finally orcaLog.finish() // Known residual: in a NESTED `flow()` call this `System.exit` tears down the // JVM before the OUTER flow's `finally` (branch restore, lock release) runs, // leaving the outer branch checked out and `.orca/flow.lock` behind (the next diff --git a/runner/src/main/scala/orca/runner/ClosingSummary.scala b/runner/src/main/scala/orca/runner/ClosingSummary.scala index 1e141d1a7..bef789714 100644 --- a/runner/src/main/scala/orca/runner/ClosingSummary.scala +++ b/runner/src/main/scala/orca/runner/ClosingSummary.scala @@ -31,12 +31,30 @@ private[runner] object ClosingSummary: * `returnToStartBranch`), the command is a range diff and names that branch * — a plain `git diff ` would run against the branch the user landed * on and show none of the work. + * + * `worktree` is the run's own checkout when it had one (`--worktree`). The + * user's shell never moved there, so the first line names the directory the + * work is in instead of saying they are on that branch, and the diff is + * scoped with `-C` so it runs from where they are actually standing. */ - def lines(branch: String, changes: Option[RunChanges]): List[String] = - val where = s"done — you are on branch '$branch'" + def lines( + branch: String, + changes: Option[RunChanges], + worktree: Option[os.Path] + ): List[String] = + // Without a worktree the sentence is about HEAD, which is `branch` by + // definition. With one it is about where the work is, so it names the + // branch holding it: the range case passes `countedOn`, because that case + // exists precisely because HEAD has left it. Only that case may name a + // branch other than HEAD's — a run with nothing to show can have had its + // `countedOn` deleted as a throwaway by the handoff just above. + def where(on: String): String = worktree match + case None => s"done — you are on branch '$branch'" + case Some(path) => s"done — the work is in $path on branch '$on'" changes match - case None => List(where) - case Some(RunChanges(_, 0, _)) => List(where, "no files changed") + case None => List(where(branch)) + case Some(RunChanges(_, 0, _)) => + List(where(branch), "no files changed") case Some(RunChanges(base, files, countedOn)) => // `countedOn` still exists whenever the range form is reached: the only // branch teardown deletes is a throwaway one, which by definition @@ -46,8 +64,12 @@ private[runner] object ClosingSummary: val target = if countedOn == branch then base.short else s"${base.short}..$countedOn" + // Quoted: this line is meant to be copied into a shell, and the path + // starts with the user's own repository location, spaces and all. + val diff = + worktree.fold("git diff")(path => s"git -C \"$path\" diff") List( - where, + where(countedOn), s"$files file(s) changed since ${base.short}", - s"next: git diff $target" + s"next: $diff $target" ) diff --git a/runner/src/main/scala/orca/runner/FlowLifecycle.scala b/runner/src/main/scala/orca/runner/FlowLifecycle.scala index 37bb628c2..48d956b83 100644 --- a/runner/src/main/scala/orca/runner/FlowLifecycle.scala +++ b/runner/src/main/scala/orca/runner/FlowLifecycle.scala @@ -229,7 +229,12 @@ object FlowLifecycle: stackSettings: StackSettings, branchMode: BranchMode, untrackedOnFailure: UntrackedFiles, - startingCommit: Option[CommitHash] + startingCommit: Option[CommitHash], + /** The orca worktree the run happened in, when it happened in one — + * `--worktree`, or a resume the shell relaunched into one without the + * flag. The closing summary names it. + */ + worktree: Option[os.Path] ) /** The branch half of [[FlowSetup]] (FP2), resolved by whichever of @@ -375,7 +380,11 @@ object FlowLifecycle: stackSettings, binding.branchMode, untrackedOnFailure, - binding.startingCommit + binding.startingCommit, + // From where the run IS, not from the flag: the shell relaunches a resume + // inside the worktree its log was found in WITHOUT `--worktree` (the flag + // would re-derive a path), and that run has to name the tree too. + Option.when(WorktreeRun.isWorktreeRun(workDir))(workDir) ) /** On an unborn HEAD (`git init`, no commits) every later git call that names @@ -385,11 +394,7 @@ object FlowLifecycle: */ private def abortIfNoCommits(git: GitTool): Unit = if git.headCommit().isEmpty then - throw new OrcaFlowException( - "orca needs a git repository with at least one commit — " + - "initialize one if needed (git init), then make the first commit " + - "(git add -A && git commit -m \"initial commit\")" - ) + throw new OrcaFlowException(GitPreconditions.needsRepoWithCommit) /** Refuse to start a NEW run on a branch that another run's progress log * already claims (ADR 0018 §2.5, R1 amendment). @@ -1189,7 +1194,7 @@ object FlowLifecycle: finishBranch(git, setup, returnToStartBranch) bestEffort("closing summary"): ClosingSummary - .lines(git.currentBranch(), changes) + .lines(git.currentBranch(), changes, setup.worktree) .foreach(line => emit(OrcaEvent.Step(line))) /** Where HEAD ends up after a successful run. A throwaway feature branch diff --git a/runner/src/main/scala/orca/runner/GitPreconditions.scala b/runner/src/main/scala/orca/runner/GitPreconditions.scala new file mode 100644 index 000000000..336335e0d --- /dev/null +++ b/runner/src/main/scala/orca/runner/GitPreconditions.scala @@ -0,0 +1,19 @@ +package orca.runner + +/** What every entry point needs of git before a run can start, worded once. + * + * Its own home rather than [[FlowLifecycle]]'s: [[WorktreeRun]] checks the + * same precondition at startup, before any lifecycle or `GitTool` exists, so + * hanging the wording off the later and much larger of the two would point the + * dependency backwards from the order things run in. + */ +private[runner] object GitPreconditions: + + /** Covers both "not a git repository" and "a repository with no commits" — + * the same next step fixes either, and a caller can rarely tell them apart + * without asking git a second question. + */ + val needsRepoWithCommit: String = + "orca needs a git repository with at least one commit — " + + "initialize one if needed (git init), then make the first commit " + + "(git add -A && git commit -m \"initial commit\")" diff --git a/runner/src/main/scala/orca/runner/WorktreeRun.scala b/runner/src/main/scala/orca/runner/WorktreeRun.scala new file mode 100644 index 000000000..c147fd142 --- /dev/null +++ b/runner/src/main/scala/orca/runner/WorktreeRun.scala @@ -0,0 +1,165 @@ +package orca.runner + +import orca.OrcaDir +import orca.progress.ProgressStore +import orca.tools.{ + MainCheckoutFailure, + StartBranchFailure, + WorktreeAddFailure, + Worktrees +} + +/** Where a `--worktree` run happens: a git worktree of its own, created on + * first use and reused after. + * + * The one place that derives the path. It is keyed on the same prompt hash as + * the run's progress log, so re-running the same task with `--worktree` lands + * back in the worktree that holds that log, without anyone re-deriving where + * that is. (The shell's resume relaunch does not use the flag at all — it runs + * in the directory the log was found in.) + */ +private[orca] object WorktreeRun: + + /** What is at the derived path. A registered worktree whose directory is gone + * — what `git clean -xdff` leaves — counts as [[Absent]]: `Worktrees.add` + * reclaims that entry. + */ + private enum PathState: + case Reusable, Occupied, Absent + + /** The directory a `--worktree` run should use, created if it does not exist + * yet. `invokingDir` is where orca was started, which may itself be a + * worktree of the repository — the answer is the same either way. + * + * `Left` is a message for the user: no repository or no commits to start + * from, something already occupying the path, or git's own refusal. + */ + def resolve( + invokingDir: os.Path, + userPrompt: String + ): Either[String, os.Path] = + Worktrees + .mainCheckoutOrReason(invokingDir) + .left + .map(noMainCheckout) + .flatMap: mainCheckout => + val path = + OrcaDir.worktreesPath(mainCheckout) / + ProgressStore.hashPrompt(userPrompt) + pathState(invokingDir, path) match + case PathState.Reusable => reuse(mainCheckout, path) + case PathState.Occupied => + Left( + s"$path already exists but is not a worktree of this " + + "repository — orca will not take over a directory it did " + + "not create" + ) + case PathState.Absent => create(invokingDir, mainCheckout, path) + + /** What to tell the user when no main checkout could be named. Each case gets + * its own sentence: telling someone whose repository is fine to `git init` + * is advice for a problem they do not have. + */ + private def noMainCheckout(failure: MainCheckoutFailure): String = + failure match + case MainCheckoutFailure.NotARepository => + GitPreconditions.needsRepoWithCommit + case MainCheckoutFailure.MainWorktreeNotACheckout => + "this repository's main worktree is not a checkout (a " + + "--separate-git-dir or submodule layout) — run --worktree from the " + + "main checkout instead" + case MainCheckoutFailure.Unsupported => + "orca could not determine this repository's main checkout (git did " + + "not answer `rev-parse --path-format=absolute`; git 2.31 or newer " + + "is required)" + + /** An existing registered worktree, made fit to run in: ignore marker back in + * place (`git clean -xdf` in the main checkout removes it and leaves the + * worktree it hides), and off a detached HEAD — a create that got half way, + * `add` having succeeded where the branch step did not. Running detached + * records the literal "HEAD" as the run's starting branch, which resume + * refuses as an unsafe ref. + * + * Every path that concludes "reuse the worktree at `path`" comes through + * here, so none of them can skip the repair. + */ + private def reuse( + mainCheckout: os.Path, + path: os.Path + ): Either[String, os.Path] = + val _ = OrcaDir.ensureWorktrees(mainCheckout) + // Anything but a definite branch goes through the repair, unreadable + // included: skipping it for a tree whose state is unknown is how a run ends + // up recording the literal "HEAD" as its starting branch. + if Worktrees.onABranch(path) then Right(path) else bindBranch(path) + + /** Whether `workDir` is a worktree orca made for its own repository — the + * question the closing summary asks, since a run ends up in one either by + * the `--worktree` flag or by the shell relaunching a resume where its + * progress log was found. Reads the layout rather than an input flag, so + * both routes answer the same. + */ + def isWorktreeRun(workDir: os.Path): Boolean = + Worktrees + .mainCheckout(workDir) + .exists(main => workDir / os.up == OrcaDir.worktreesPath(main)) + + private def pathState(invokingDir: os.Path, path: os.Path): PathState = + if !os.exists(path) then PathState.Absent + else if Worktrees.list(invokingDir).contains(path) then PathState.Reusable + else PathState.Occupied + + private def create( + invokingDir: os.Path, + mainCheckout: os.Path, + path: os.Path + ): Either[String, os.Path] = + // The ignore marker goes in before the worktree, never after: until it is + // there, `git add -A` in the main checkout stages the new worktree as an + // embedded git repository. + val _ = OrcaDir.ensureWorktrees(mainCheckout) + Worktrees.add(invokingDir, path) match + case Right(()) => bindBranch(path) + case Left(failure) => + // Another orca may have created it between `pathState` and here: the + // run lock is keyed on the run directory, which does not exist yet, so + // nothing serialises two processes starting the same task at once. If + // the path is a registered worktree now, that is the answer the winner + // got a moment earlier. + if pathState(invokingDir, path) == PathState.Reusable then + reuse(mainCheckout, path) + else + Left(failure match + case WorktreeAddFailure.NoCommitsYet => + GitPreconditions.needsRepoWithCommit + case WorktreeAddFailure.GitFailed(message) => + s"could not create the worktree at $path: $message" + ) + + /** Put the worktree on the branch named after the same task hash, so a re-run + * of the task finds its own branch rather than a stranger's. Its refusals + * say the worktree exists — it does by then, and the next run finds it. + */ + private def bindBranch(path: os.Path): Either[String, os.Path] = + val branch = s"orca-worktree-${path.last}" + Worktrees.startBranch(path, branch) match + case Right(()) => Right(path) + case Left(StartBranchFailure.WouldLoseCommits(name)) => + Left( + s"branch '$name' already has commits this run would not start from " + + s"— orca will not move it; merge it, or delete it with " + + s"git branch -D $name" + ) + case Left(StartBranchFailure.GitFailed(message)) => + // This runs before the run lock, whose directory is the one being + // written to, so another orca binding the same branch in the same + // worktree is a real outcome — and an idempotent one. Proceed only on a + // DEFINITE branch: a worktree that has gone away answers neither, and + // treating that as "already done" would run against a path that is no + // longer a worktree at all. + if Worktrees.onABranch(path) then Right(path) + else + Left( + s"the worktree at $path exists but could not be put on branch " + + s"'$branch': $message" + ) diff --git a/runner/src/test/scala/orca/OrcaArgsTest.scala b/runner/src/test/scala/orca/OrcaArgsTest.scala index 3b59e759a..c07c3f3da 100644 --- a/runner/src/test/scala/orca/OrcaArgsTest.scala +++ b/runner/src/test/scala/orca/OrcaArgsTest.scala @@ -35,6 +35,27 @@ class OrcaArgsTest extends munit.FunSuite: Right(false) ) + test("--worktree flag sets worktree = true"): + val result = OrcaArgs + .parse(Seq("--worktree", "do the thing")) + .toOption + .getOrElse(fail("expected successful parse")) + assertEquals(result.userPrompt, "do the thing") + assertEquals(result.worktree.value, true) + + test("--worktree with --skip-branch is refused, naming both flags"): + assertRefused(Seq("--worktree", "--skip-branch", "x"), "--skip-branch") + + test("--worktree with --keep-changes is refused, naming both flags"): + assertRefused(Seq("--worktree", "--keep-changes", "x"), "--keep-changes") + + private def assertRefused(argv: Seq[String], otherFlag: String): Unit = + OrcaArgs.parse(argv) match + case Left(msg) => + assert(msg.contains("--worktree"), msg) + assert(msg.contains(otherFlag), msg) + case Right(r) => fail(s"expected a refusal, got $r") + test("unknown flags yield a Left with an error message"): OrcaArgs.parse(Seq("--nonexistent")) match case Left(msg) => assert(msg.nonEmpty) diff --git a/runner/src/test/scala/orca/runner/ClosingSummaryTest.scala b/runner/src/test/scala/orca/runner/ClosingSummaryTest.scala new file mode 100644 index 000000000..58108672a --- /dev/null +++ b/runner/src/test/scala/orca/runner/ClosingSummaryTest.scala @@ -0,0 +1,55 @@ +package orca.runner + +import orca.progress.CommitHash + +/** The worktree shapes of [[ClosingSummary.lines]]. The non-worktree ones are + * pinned end-to-end by `FlowLifecycleTest`'s closing-summary cases, which run + * through `teardownSuccess`. + */ +class ClosingSummaryTest extends munit.FunSuite: + + private val base: CommitHash = + CommitHash.from("0123456789abcdef0123456789abcdef01234567").get + + // Never touched: `lines` only formats it into two strings. The space is the + // fixture's job — every worktree case then proves the offered command keeps + // the path as one shell argument. + private val worktree: os.Path = + os.root / "my repo" / ".orca" / "worktrees" / "ab12cd34" + + test("a worktree run names where the work is and scopes the diff to it"): + assertEquals( + ClosingSummary + .lines("work", Some(RunChanges(base, 2, "work")), Some(worktree)), + List( + s"done — the work is in $worktree on branch 'work'", + s"2 file(s) changed since ${base.short}", + s"""next: git -C "$worktree" diff ${base.short}""" + ) + ) + + test("a worktree run that changed nothing offers no diff"): + assertEquals( + ClosingSummary + .lines("work", Some(RunChanges(base, 0, "work")), Some(worktree)), + List( + s"done — the work is in $worktree on branch 'work'", + "no files changed" + ) + ) + + test("HEAD leaving the counted branch still names the branch with the work"): + // What a `returnToStartBranch` flow leaves: HEAD back on the branch the + // worktree was created on, which holds none of the run's commits. + assertEquals( + ClosingSummary.lines( + "orca-worktree-ab12cd34", + Some(RunChanges(base, 2, "work")), + Some(worktree) + ), + List( + s"done — the work is in $worktree on branch 'work'", + s"2 file(s) changed since ${base.short}", + s"""next: git -C "$worktree" diff ${base.short}..work""" + ) + ) diff --git a/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala b/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala index b8a04ab72..f67857388 100644 --- a/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala +++ b/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala @@ -41,7 +41,14 @@ import orca.progress.{ StageEntry } import orca.runner.terminal.TerminalInteraction -import orca.tools.{FsTool, GitHubTool, GitTool, OsGitTool, UntrackedFiles} +import orca.tools.{ + FsTool, + GitHubTool, + GitTool, + OsGitTool, + UntrackedFiles, + Worktrees +} import mainargs.Flag import ox.supervised import ox.either.orThrow @@ -97,6 +104,127 @@ class FlowLifecycleTest extends munit.FunSuite: s"the closing block must reach the terminal: $rendered" ) + test( + "--worktree: the run happens in the worktree, the invoking checkout is untouched, and a re-run returns to it" + ): + val workDir = GitRepo.seeded() + val prompt = "worktree-run" + def branchOf(dir: os.Path): String = + os.proc("git", "rev-parse", "--abbrev-ref", "HEAD") + .call(cwd = dir) + .out + .text() + .trim + // Recorded from inside the body rather than asserted there: a failed + // assertion would escape as a body failure, which `flow()` answers with + // `System.exit(1)`. + val ranIn = new AtomicReference[Option[os.Path]](None) + val logSeen = new AtomicReference(false) + val listener = new RecordingListener + def run(): Unit = + supervised: + val interaction = TerminalInteraction.start( + out = new PrintStream(new ByteArrayOutputStream()), + useColor = false, + animated = false + ) + flow( + args = OrcaArgs(prompt, worktree = Flag(true)), + stackSettings = Some(StackSettings.empty), + claude = Some(_ => StubAgent.claude), + workDir = workDir, + extraListeners = List(listener), + interaction = Some(interaction) + ): + val ctx = summon[FlowContext] + val _ = stage("worktree-stage"): + ranIn.set(Some(ctx.workDir)) + logSeen.set( + os.exists(ProgressStore.default(ctx.workDir, prompt).path) + ) + os.write.over(ctx.workDir / "made-by-the-run.txt", "worktree work") + "ok" + + run() + // Built from the two pieces the derivation is made of, not by calling the + // resolver: that is a write, and the expectation must not come from the + // code under test. + val worktree = + OrcaDir.worktreesPath(workDir) / ProgressStore.hashPrompt(prompt) + assertEquals(ranIn.get(), Some(worktree), "the run must happen there") + assert(logSeen.get(), "the progress log must live inside the worktree") + // The session manifest is the one consumer above `runFlow`, so it is what + // pins resolution to `flow()`: were it to move down into `runFlow`, the + // manifest would land in the invoking checkout while everything else moved. + // `RunManifestWriter.start` creates its runs directory when constructed; + // the manifest file itself waits for a committed session, which a stubbed + // run never has. + assert( + os.exists(OrcaDir.runsPath(worktree)), + "the session manifest must be written inside the worktree" + ) + assert( + !os.exists(OrcaDir.runsPath(workDir)), + "the invoking checkout must get no run manifest" + ) + // The work is on a branch of the worktree's own — neither the detached + // start point nor the invoking checkout's branch. + assertNotEquals(branchOf(worktree), "HEAD") + assertNotEquals(branchOf(worktree), "main") + assertEquals(os.read(worktree / "made-by-the-run.txt"), "worktree work") + // The invoking checkout never moved. + assertEquals(branchOf(workDir), "main") + assertEquals( + os.proc("git", "status", "--porcelain").call(cwd = workDir).out.text(), + "" + ) + assert(!os.exists(workDir / "made-by-the-run.txt")) + // The user's shell never moved, so the closing block has to say where the + // work actually is. + val steps = listener.events.collect: + case OrcaEvent.Step(text) => text + assert( + steps.contains( + s"done — the work is in $worktree on branch '${branchOf(worktree)}'" + ), + steps.mkString("\n") + ) + + ranIn.set(None) + run() + assertEquals(ranIn.get(), Some(worktree), "a re-run returns to it") + assertEquals(Worktrees.list(workDir), List(workDir, worktree)) + + test( + "--worktree: a failed run in the worktree is resumable, not refused" + ): + val invokedIn = GitRepo.seeded() + val prompt = "worktree-resume" + val worktree = WorktreeRun + .resolve(invokedIn, prompt) + .getOrElse(fail("the worktree must resolve")) + val store = ProgressStore.default(worktree, prompt) + val stageOneRuns = new AtomicInteger(0) + + val _ = intercept[SurfacedFlowFailure]: + runFlowForTest(worktree, prompt, store): + val _ = stage("stage-one"): + stageOneRuns.incrementAndGet() + "one-done" + val _ = stage[String]("stage-two"): + throw new RuntimeException("boom") + + // A worktree left detached records `startingBranch` as the literal "HEAD", + // which resume refuses as an unsafe ref — the run would be unresumable and + // unrestartable, its log still on disk. + runFlowForTest(worktree, prompt, store): + val _ = stage("stage-one"): + stageOneRuns.incrementAndGet() + "one-done" + val _ = stage("stage-two"): + "two-done" + assertEquals(stageOneRuns.get(), 1, "stage one must replay, not re-run") + test( "failure teardown: stays on feature branch with clean working tree and earlier commit present" ): @@ -2909,7 +3037,8 @@ class FlowLifecycleTest extends munit.FunSuite: stackSettings = StackSettings.empty, branchMode = BranchMode.Reused, untrackedOnFailure = UntrackedFiles.Remove, - startingCommit = None + startingCommit = None, + worktree = None ) FlowLifecycle.teardownSuccess( git, @@ -2966,7 +3095,8 @@ class FlowLifecycleTest extends munit.FunSuite: stackSettings = StackSettings.empty, branchMode = BranchMode.Reused, untrackedOnFailure = UntrackedFiles.Remove, - startingCommit = None + startingCommit = None, + worktree = None ) TeardownPushRepo(git, remote, setup, store.path.subRelativeTo(workDir)) @@ -3065,7 +3195,8 @@ class FlowLifecycleTest extends munit.FunSuite: stackSettings = StackSettings.empty, branchMode = branchMode, untrackedOnFailure = UntrackedFiles.Remove, - startingCommit = startingCommit + startingCommit = startingCommit, + worktree = None ) test( diff --git a/runner/src/test/scala/orca/runner/WorktreeRunTest.scala b/runner/src/test/scala/orca/runner/WorktreeRunTest.scala new file mode 100644 index 000000000..bfb429a5f --- /dev/null +++ b/runner/src/test/scala/orca/runner/WorktreeRunTest.scala @@ -0,0 +1,155 @@ +package orca.runner + +import orca.OrcaDir +import orca.progress.ProgressStore +import orca.testkit.{GitRepo, TempDirs} +import orca.tools.Worktrees +import ox.discard + +class WorktreeRunTest extends munit.FunSuite: + + test("the worktree lives under .orca/worktrees, keyed by the task text"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + assertEquals(path / os.up, OrcaDir.worktreesPath(repo)) + assertNotEquals(path, resolved(repo, "task B")) + // Same key as the progress log inside it: that coupling is what makes a + // resumed run land where its log is. + assert( + ProgressStore.default(path, "task A").path.last.contains(path.last), + path.last + ) + + test("re-resolving the same task text reuses the worktree already there"): + val repo = GitRepo.seeded() + val first = resolved(repo, "task A") + assertEquals(resolved(repo, "task A"), first) + // The main checkout plus exactly one worktree: no second one was created. + assertEquals(Worktrees.list(repo), List(repo, first)) + + test("a new worktree is put on a branch of its own, not left detached"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + // Detached would read back as the literal "HEAD", which the run would + // record as its starting branch and resume would then refuse. + assertNotEquals( + os.proc("git", "rev-parse", "--abbrev-ref", "HEAD") + .call(cwd = path) + .out + .text() + .trim, + "HEAD" + ) + + test("resolving from inside the worktree lands on that same worktree"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + assertEquals(WorktreeRun.resolve(path, "task A"), Right(path)) + + test("the ignore marker is written before the worktree is created"): + val repo = GitRepo.empty() + // Creation cannot succeed on an unborn HEAD, so the marker is on disk only + // if it was written first. + assert(WorktreeRun.resolve(repo, "task A").isLeft) + assert(os.exists(OrcaDir.worktreesPath(repo) / ".gitignore")) + + test("reuse puts the ignore marker back when it went missing"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + // What `git clean -xdf` in the main checkout removes: the marker only, the + // worktree itself skipped as a repository. + os.remove(OrcaDir.worktreesPath(repo) / ".gitignore").discard + assertEquals(WorktreeRun.resolve(repo, "task A"), Right(path)) + assert(os.exists(OrcaDir.worktreesPath(repo) / ".gitignore")) + + test("a worktree whose directory was deleted is recreated"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + // What `git clean -xdff` leaves: the directory gone, git's administrative + // entry still listing it. + os.remove.all(path) + assertEquals(WorktreeRun.resolve(repo, "task A"), Right(path)) + assert(os.exists(path / ".git")) + + test("recreating one worktree leaves the repository's others registered"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + val unrelated = repo / "unrelated" + assertEquals(Worktrees.add(repo, unrelated), Right(())) + // Both directories unreachable — a stale entry orca does not own must + // survive the reclaim of the one it does. + os.remove.all(path) + os.remove.all(unrelated) + assertEquals(WorktreeRun.resolve(repo, "task A"), Right(path)) + assert(Worktrees.list(repo).contains(unrelated)) + + test("a plain directory at the path is refused, not taken over"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + git(repo, "worktree", "remove", "--force", path.toString) + os.makeDir.all(path) + val refusal = WorktreeRun + .resolve(repo, "task A") + .left + .getOrElse(fail("expected a refusal")) + assert(refusal.contains(path.toString), refusal) + + test("outside a git repository, orca's own wording is used"): + assertEquals( + WorktreeRun.resolve(TempDirs.dir(), "task A"), + Left(GitPreconditions.needsRepoWithCommit) + ) + + test("a repository without commits gets orca's wording, not git's"): + assertEquals( + WorktreeRun.resolve(GitRepo.empty(), "task A"), + Left(GitPreconditions.needsRepoWithCommit) + ) + + test("reuse puts a detached worktree back on its branch"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + // How a half-created worktree looks: `add` succeeded, the branch step did + // not. Running there detached records an unresumable starting branch. + git(path, "checkout", "--detach", "-q") + assertEquals(Worktrees.headBranch(path), Some("HEAD")) + assertEquals(WorktreeRun.resolve(repo, "task A"), Right(path)) + assert(Worktrees.onABranch(path)) + + test("reuse refuses when the run's branch has gained commits"): + val repo = GitRepo.seeded() + val path = resolved(repo, "task A") + val branch = s"orca-worktree-${path.last}" + os.write(path / "work.txt", "work") + git(path, "add", "-A") + git(path, "commit", "-q", "-m", "work in the worktree") + // Detached BEHIND the branch, so putting the run back on it would strand + // that commit — which is what orca refuses to do. + git(path, "checkout", "--detach", "-q", "HEAD~1") + val refusal = WorktreeRun + .resolve(repo, "task A") + .left + .getOrElse(fail("expected a refusal")) + assert(refusal.contains(branch), refusal) + // The remedy names the branch, which is what holds the commits. Removing + // the worktree does not help: the next run recreates it detached and hits + // this same refusal. + assert(refusal.contains(s"git branch -D $branch"), refusal) + + test("isWorktreeRun recognises orca's own worktree and nothing else"): + val repo = GitRepo.seeded() + val ours = resolved(repo, "task A") + val theirs = repo / "review-their-branch" + assertEquals(Worktrees.add(repo, theirs), Right(())) + assert(WorktreeRun.isWorktreeRun(ours), "orca's own worktree") + assert(!WorktreeRun.isWorktreeRun(repo), "the main checkout is not one") + assert(!WorktreeRun.isWorktreeRun(theirs), "a worktree orca did not make") + assert(!WorktreeRun.isWorktreeRun(TempDirs.dir()), "not a repository") + + private def resolved(repo: os.Path, userPrompt: String): os.Path = + WorktreeRun + .resolve(repo, userPrompt) + .getOrElse(fail("expected a resolved worktree")) + + private def git(cwd: os.Path, args: String*): Unit = + os.proc("git" +: args).call(cwd = cwd).discard diff --git a/shell/src/main/scala/orca/shell/Main.scala b/shell/src/main/scala/orca/shell/Main.scala index 1cbec0d43..ec8ed96ee 100644 --- a/shell/src/main/scala/orca/shell/Main.scala +++ b/shell/src/main/scala/orca/shell/Main.scala @@ -99,11 +99,16 @@ object Main: * on every redraw (ADR 0021 §8) — a flow run started from this same menu can * only have just finished, so the freshest listing is worth the re-read. * `ResumeDetector.detect` is likewise re-evaluated every redraw (ADR 0021 §3 - * amendment) — cheap (one dir listing plus one small file read) and - * consistent with Continue's own re-read. The `branch:` line - * ([[ConfigSummary.branchLine]]) is printed here for the same reason: a flow - * run started from this menu can leave HEAD on a new branch, so it is - * re-read per redraw rather than printed once with the startup summary. + * amendment). Both scans share ONE `WorktreeScan.dirs` resolution — the + * discovery is a git subprocess or two, and nothing between them can change + * the answer — over a bounded set of directories, so a redraw stays cheap + * enough to repeat and consistent with Continue's own re-read. + * Re-discovering per redraw is the point: a `--worktree` run started from + * this very menu creates a worktree that was not there when the shell + * started. The `branch:` line ([[ConfigSummary.branchLine]]) is printed here + * for the same reason: a flow run started from this menu can leave HEAD on a + * new branch, so it is re-read per redraw rather than printed once with the + * startup summary. */ @tailrec private def loop( ui: ShellUi, @@ -112,15 +117,23 @@ object Main: terminal: Terminal, tty: Boolean ): Unit = - val (runs, warnings) = ManifestReader.list(os.pwd, ManifestReader.pidAlive) + // Resolved once and shared: both scans want the same answer over the same + // cwd, and the discovery is a git subprocess or two. + val scanDirs = WorktreeScan.dirs(os.pwd) + val (runs, warnings) = + ManifestReader.list( + scanDirs.own, + scanDirs.worktrees, + ManifestReader.pidAlive + ) warnings.foreach(ShellOutput.info) val continueSessionCount = runs.headOption.map(_.manifest.sessions.size) - val resumeOffer = ResumeDetector.detect(os.pwd) + val resumeOffer = ResumeDetector.detect(scanDirs.all) ConfigSummary.branchLine(os.pwd).foreach(ShellOutput.info) ui.select( "orca shell", - MainMenu.choices(continueSessionCount, resumeOffer) + MainMenu.choices(continueSessionCount, resumeOffer, scanDirs.own) ) match case UiOutcome.Cancelled => () case UiOutcome.Selected(MenuItem.Exit) => () @@ -349,27 +362,43 @@ object Main: case UiOutcome.Cancelled => None case UiOutcome.Selected(tier) => Some(tier) - /** Selects a flow, prompts for the task text and whether to create a branch, - * then hands off to [[RunAction.run]]. Verbose is not exposed here in v1 — a - * later task can add a verbose confirm alongside session tracking. + /** Selects a flow, prompts for the task text and for where the run's work + * should go ([[RunTarget]]), then hands off to [[RunAction.run]]. Verbose is + * not exposed here in v1 — a later task can add a verbose confirm alongside + * session tracking. */ - private def runFlow(ui: ShellUi, terminal: Terminal): Unit = + private[shell] def runFlow( + ui: ShellUi, + terminal: Terminal, + workDir: os.Path = os.pwd, + // `runAction` is injectable like `resumeInterruptedRun`'s — see its note + // on why the default is spelled as a lambda; `workDir` is the usual + // explicit-directory test seam. + runAction: ( + DiscoveredFlow, + String, + RunAction.RunOptions, + os.Path, + Terminal + ) => LaunchResult = RunAction.run(_, _, _, _, _) + ): Unit = for - flow <- listFlows().flatMap( + flow <- listFlows(workDir).flatMap( pickFlow(ui, "Run which flow?", _, promoteByName(FlagshipFlow, _)) ) task <- promptTask(ui) - createBranch <- promptCreateBranch(ui) + target <- promptRunTarget(ui) do val opts = RunAction.RunOptions( flags = FlowFlags( verbose = false, - skipBranch = !createBranch, - keepChanges = false + skipBranch = target.skipBranch, + keepChanges = false, + worktree = target.worktree ), fallback = FallbackPolicy.Ask(ui) ) - RunAction.run(flow, task, opts, os.pwd, terminal).discard + runAction(flow, task, opts, workDir, terminal).discard /** Resumes `run` (ADR 0021 §3 amendment): resolves its recorded flow name * against the current catalog and launches it with the recorded task text @@ -380,14 +409,17 @@ object Main: * the resume happens on the current branch by design, and a resumed log's * `bindBranch` (`FlowLifecycle`) ignores `skipBranch` entirely, so the * all-false flags passed here are exactly as correct as any other value - * would be. `runAction` is injectable, [[AuthorAction]]-style, so a test can - * record the call instead of spawning a real subprocess. + * would be — `worktree` included: the run is launched IN `run.dir`, the + * directory its log was found in, which is what makes this a resume. The + * flag would instead re-derive a path from the task text, which is the same + * directory only when the log happened to be in an orca-made worktree of + * that exact prompt. `runAction` is injectable, [[AuthorAction]]-style, so a + * test can record the call instead of spawning a real subprocess. */ private[shell] def resumeInterruptedRun( ui: ShellUi, terminal: Terminal, run: InterruptedRun, - workDir: os.Path = os.pwd, runAction: ( DiscoveredFlow, String, @@ -399,7 +431,7 @@ object Main: // pull into this shape. ) => LaunchResult = RunAction.run(_, _, _, _, _) ): Unit = - FlowResolution.resolve(run.flowName, workDir) match + FlowResolution.resolve(run.flowName, run.dir) match case Left(message) => ShellOutput.error(message) case Right(flow) => val opts = @@ -407,11 +439,12 @@ object Main: flags = FlowFlags( verbose = false, skipBranch = false, - keepChanges = false + keepChanges = false, + worktree = false ), fallback = FallbackPolicy.Ask(ui) ) - runAction(flow, run.userPrompt, opts, workDir, terminal).discard + runAction(flow, run.userPrompt, opts, run.dir, terminal).discard /** Prompts for the flow's task text, re-prompting on blank input — an empty * `userPrompt` reaches the flow's agent directly (branch naming, the coding @@ -426,20 +459,26 @@ object Main: promptTask(ui) case UiOutcome.Selected(text) => Some(text) - /** "Create a new branch for this run?" confirm (default yes — Enter keeps - * today's behavior); declining runs in skip-branch mode instead (ADR 0018 - * amendment), continuing on the current branch — the handoff-from-harness - * case where the user already planned work on a branch carrying plan files. + /** "Where should this run's work go?" — the run's destination as ONE choice + * ([[RunTarget]]). Enter keeps today's behavior because `NewBranch` is the + * FIRST row, which is where both backends start the cursor; `preselect` only + * marks it on [[orca.shell.ui.NumberedUi]] and is a documented no-op on the + * tty backend, so the ordering is what carries the default, not this. + * `CurrentBranch` is skip-branch mode (ADR 0018 amendment): the + * handoff-from-harness case, where the user already planned work on a branch + * carrying plan files. `Worktree` is `--worktree`, which orca refuses + * together with `--skip-branch` (`OrcaArgs.worktreeRefusal`) — one choice + * cannot express that pair, where two independent confirms could. * `private[shell]` so a scripted-UI test can drive it directly. */ - private[shell] def promptCreateBranch(ui: ShellUi): Option[Boolean] = - ui.confirm( - "Create a new branch for this run? (choosing 'no': the flow makes " + - "its changes on the current branch)", - default = true + private[shell] def promptRunTarget(ui: ShellUi): Option[RunTarget] = + ui.select( + "Where should this run's work go?", + MainMenu.runTargetChoices, + preselect = Some(RunTarget.NewBranch) ) match - case UiOutcome.Cancelled => None - case UiOutcome.Selected(v) => Some(v) + case UiOutcome.Cancelled => None + case UiOutcome.Selected(target) => Some(target) /** "How should the changes be made?" (ADR 0021 §6/§9 amendment) — * [[MainMenu.modeChoices]]'s hand-vs-agent prompt, shared by Edit/Create/ @@ -729,8 +768,10 @@ object Main: * needs [[pickFlow]]'s `reorder` (promoting [[FlagshipFlow]]) instead of its * default alphabetical order. */ - private def listFlows(): Option[List[DiscoveredFlow]] = - FlowResolution.list(os.pwd) match + private def listFlows( + workDir: os.Path = os.pwd + ): Option[List[DiscoveredFlow]] = + FlowResolution.list(workDir) match case Left(message) => ShellOutput.error(message) None diff --git a/shell/src/main/scala/orca/shell/MainMenu.scala b/shell/src/main/scala/orca/shell/MainMenu.scala index 5034bf0c3..6bba8be9b 100644 --- a/shell/src/main/scala/orca/shell/MainMenu.scala +++ b/shell/src/main/scala/orca/shell/MainMenu.scala @@ -20,6 +20,32 @@ private[shell] enum MenuItem: private[shell] enum ChangeMode: case Hand, Agent +/** Where a run's work goes, asked via [[MainMenu.runTargetChoices]] once the + * flow and the task text are settled. + * + * One choice on one axis, rather than a branch confirm followed by a worktree + * confirm: the two answers are not independent — orca refuses `--worktree` + * together with `--skip-branch` — and asking them separately makes the illegal + * pair expressible, leaving prompt order to prevent it. A worktree run creates + * a branch of its own, so all three cases here are branch-creating or not in + * exactly one way. + */ +private[shell] enum RunTarget: + /** A branch orca creates in this checkout — the default. */ + case NewBranch + + /** The branch checked out now; the flow commits onto it (`--skip-branch`). */ + case CurrentBranch + + /** A separate checkout under `.orca/worktrees/` (`--worktree`). */ + case Worktree + + /** Runs on the branch already checked out, instead of a new one. */ + def skipBranch: Boolean = this == RunTarget.CurrentBranch + + /** Runs in a worktree rather than the invoking checkout. */ + def worktree: Boolean = this == RunTarget.Worktree + private[shell] object MainMenu: /** Fixed ADR §3 order. Conditional items are ABSENT when inapplicable, never @@ -30,7 +56,10 @@ private[shell] object MainMenu: */ def choices( continueSessionCount: Option[Int], - resumeOffer: Option[InterruptedRun] = None + resumeOffer: Option[InterruptedRun] = None, + // The shell's own directory, to tell a resume that happens here from one + // that happens in a worktree. Defaulted for the tests that don't care. + workDir: os.Path = os.pwd ): List[Choice[MenuItem]] = val continueChoice = continueSessionCount.map(count => Choice( @@ -39,7 +68,9 @@ private[shell] object MainMenu: ) ) val resumeChoice = - resumeOffer.map(run => Choice(MenuItem.ResumeRun, resumeLabel(run))) + resumeOffer.map(run => + Choice(MenuItem.ResumeRun, resumeLabel(run, workDir)) + ) List( Choice(MenuItem.RunFlow, "Run a flow") ) ++ resumeChoice.toList ++ List( @@ -85,11 +116,33 @@ private[shell] object MainMenu: Choice(ChangeMode.Hand, "By hand — open in your editor") ) + /** [[RunTarget]]'s rows, in the order they are offered. `NewBranch` leads + * because it is the default, and the position is load-bearing rather than + * cosmetic: `ConsoleUiShell` cannot honor `preselect`, so the first row is + * what the cursor starts on and what Enter picks. + */ + val runTargetChoices: List[Choice[RunTarget]] = List( + Choice(RunTarget.NewBranch, "A new branch in this checkout"), + Choice( + RunTarget.CurrentBranch, + "The branch checked out now — the flow commits onto it" + ), + Choice( + RunTarget.Worktree, + "A new worktree — a separate checkout under .orca/worktrees/, " + + "leaving this one untouched" + ) + ) + /** `"Resume interrupted run — : "`. The task * comes from a committed header and is often multi-line (`Main.promptTask` * reads multi-line), so it reaches the menu row through * [[TextUtil.onelinePreview]]. */ - private def resumeLabel(run: InterruptedRun): String = + private def resumeLabel(run: InterruptedRun, workDir: os.Path): String = val task = TextUtil.onelinePreview(run.userPrompt, 40) - s"Resume interrupted run — ${run.flowName}: $task" + // The log can be in one of orca's worktrees, and the run resumes THERE — + // an offer that read like any other would send the user's work to a + // directory they were never shown. + val where = if run.dir == workDir then "" else s" (in ${run.dir.last})" + s"Resume interrupted run — ${run.flowName}: $task$where" diff --git a/shell/src/main/scala/orca/shell/WorktreeScan.scala b/shell/src/main/scala/orca/shell/WorktreeScan.scala new file mode 100644 index 000000000..7be0269e4 --- /dev/null +++ b/shell/src/main/scala/orca/shell/WorktreeScan.scala @@ -0,0 +1,81 @@ +package orca.shell + +import orca.OrcaDir +import orca.tools.Worktrees + +/** Where the shell's per-redraw scans look: the checkout it was started in, and + * the worktrees orca itself created for that repository (`--worktree` runs + * live in one, and leave their progress log and session manifests there). + * + * The two are kept apart rather than concatenated: `ManifestReader` reads the + * shell's own directory strictly and the rest guarded, and a plain list would + * leave that difference to list position. + */ +private[shell] case class ScanDirs(own: os.Path, worktrees: List[os.Path]): + /** Every directory to scan, for a consumer that treats them all alike. */ + def all: List[os.Path] = own :: worktrees + +/** Resolution happens here, at the call site, rather than inside the scans + * themselves — the convention `Cli`'s scaladoc states: the scans take explicit + * directories, so their tests can seed bare temp dirs with no git repository + * in sight. + */ +private[shell] object WorktreeScan: + + /** How many worktrees are scanned besides the shell's own directory. Orca + * never removes a worktree and makes one per distinct task, so + * `.orca/worktrees/` grows for the life of the repository — while these + * scans run on every menu redraw. Matches `RunManifestWriter`'s own + * kept-runs budget. + * + * A worktree past the cap is invisible to both scans: its sessions do not + * reach `continue`, and an interrupted run in it is not offered. Ranked by + * when each last recorded a run, so what drops out is what nobody has + * touched in the last twenty tasks. + */ + private[shell] val MaxScannedWorktrees = 20 + + /** `workDir` plus the most recently used worktrees orca made FOR `workDir` — + * the registered worktrees directly under its own `.orca/worktrees/` — + * capped at [[MaxScannedWorktrees]]. + * + * Scoped to one checkout because that is how the data is stored: `.orca/` is + * per-checkout, and a worktree's runs leave their progress log and session + * manifests in the worktree, not in the checkout that created it. So the + * checkout that made the worktrees sees itself and all of them, while a + * worktree — whose own `.orca/worktrees/` is empty — sees only itself. To + * survey every run in the repository, run the shell from the checkout the + * worktrees hang off. + * + * Only `workDir` when git has nothing to say (not a repository, git + * unavailable): the shell's own directory is always worth scanning. + * + * Deliberately NOT every worktree git reports. A progress log is committed + * repo content, so a worktree checked out to review someone else's branch + * carries that branch's log — and its `userPrompt` is the task text the + * resume offer would hand an agent verbatim. + */ + def dirs(workDir: os.Path): ScanDirs = + val root = OrcaDir.worktreesPath(workDir) + ScanDirs( + workDir, + Worktrees + .list(workDir) + .filter(_ / os.up == root) + // Decorate-sort-undecorate: `sortBy` re-evaluates its key on every + // comparison, and this key is a syscall per call. + .map(dir => (lastRunAt(dir), dir)) + .sortBy(-_._1) + .take(MaxScannedWorktrees) + .map(_._2) + ) + + /** When a worktree last recorded a run — the mtime of its + * `.orca/cache/runs/`, which `RunManifestWriter` creates as it starts and + * rewrites per manifest. `0` for a worktree that never ran one, and for one + * that just went away: this runs on every menu redraw, over directories a + * `git worktree remove` in another terminal can delete mid-scan, and every + * step downstream of it survives that. Both answers sort last. + */ + private def lastRunAt(worktree: os.Path): Long = + scala.util.Try(os.mtime(OrcaDir.runsPath(worktree))).getOrElse(0L) diff --git a/shell/src/main/scala/orca/shell/actions/AuthorAction.scala b/shell/src/main/scala/orca/shell/actions/AuthorAction.scala index 815cc58a9..b5823e12e 100644 --- a/shell/src/main/scala/orca/shell/actions/AuthorAction.scala +++ b/shell/src/main/scala/orca/shell/actions/AuthorAction.scala @@ -137,7 +137,12 @@ private[shell] object AuthorAction: flow, prompt, sandbox, - FlowFlags(verbose = false, skipBranch = false, keepChanges = false), + FlowFlags( + verbose = false, + skipBranch = false, + keepChanges = false, + worktree = false + ), terminal ) finishAuthoring(result, sandbox, params) diff --git a/shell/src/main/scala/orca/shell/cli/Cli.scala b/shell/src/main/scala/orca/shell/cli/Cli.scala index 7de3e6d6b..0b1d3805c 100644 --- a/shell/src/main/scala/orca/shell/cli/Cli.scala +++ b/shell/src/main/scala/orca/shell/cli/Cli.scala @@ -3,6 +3,7 @@ package orca.shell.cli import mainargs.{Flag, ParserForMethods, Renderer, Util, arg, main} import org.jline.terminal.Terminal import orca.settings.GlobalSettings +import orca.shell.WorktreeScan import orca.shell.run.{FlowFlags, LaunchResult} import orca.shell.ui.ShellUi import orca.subprocess.TtyProbe @@ -154,6 +155,10 @@ private[shell] object Cli: "keep uncommitted/untracked files in the working tree instead of stashing them (fresh runs only)" ) keepChanges: Flag = Flag(), + @arg(doc = + "run the flow in a git worktree of this repository instead of the current checkout" + ) + worktree: Flag = Flag(), @arg(doc = "run the flow's own pinned orca version instead of forcing this shell's" ) @@ -165,7 +170,8 @@ private[shell] object Cli: flags = FlowFlags( verbose = verbose.value, skipBranch = skipBranch.value, - keepChanges = keepChanges.value + keepChanges = keepChanges.value, + worktree = worktree.value ), honorPin = honorPin.value, workDir = os.pwd, @@ -253,7 +259,13 @@ private[shell] object Cli: @arg(doc = "with --list, emit JSON instead of a table") json: Flag = Flag() ): Int = - ContinueCli.runContinue(os.pwd, selector, list.value, json.value, isTty) + ContinueCli.runContinue( + WorktreeScan.dirs(os.pwd), + selector, + list.value, + json.value, + isTty + ) @main(doc = "Show or set the global role agents (planning/coding/review).\n" + diff --git a/shell/src/main/scala/orca/shell/cli/ContinueCli.scala b/shell/src/main/scala/orca/shell/cli/ContinueCli.scala index 1764f9592..6fd6ebc3e 100644 --- a/shell/src/main/scala/orca/shell/cli/ContinueCli.scala +++ b/shell/src/main/scala/orca/shell/cli/ContinueCli.scala @@ -1,5 +1,6 @@ package orca.shell.cli +import orca.shell.ScanDirs import orca.shell.actions.SessionAction import orca.shell.sessions.{ManifestReader, SessionPicker, SessionSelection} @@ -11,18 +12,21 @@ import Cli.{actionFailure, complete, requireTty, usageFailure, withTerminal} */ private[cli] object ContinueCli: - /** `continue`'s full behavior over an explicit `workDir`/`tty` (test seam) — - * tests seed `workDir` with `.orca/cache/runs/` manifests and simulate - * either a terminal or a pipe via `tty`. + /** `continue`'s full behavior over explicit `dirs`/`tty` (test seam) — tests + * seed each directory with `.orca/cache/runs/` manifests and simulate either + * a terminal or a pipe via `tty`. The directories arrive resolved + * ([[orca.shell.WorktreeScan.dirs]], at the real entry point), so nothing + * here spawns git. */ private[cli] def runContinue( - workDir: os.Path, + dirs: ScanDirs, selector: Option[String], list: Boolean, json: Boolean, tty: Boolean ): Int = - val (runs, warnings) = ManifestReader.list(workDir, ManifestReader.pidAlive) + val (runs, warnings) = + ManifestReader.list(dirs.own, dirs.worktrees, ManifestReader.pidAlive) warnings.foreach(Cli.diagnostic) if list then Tables.printSessionListing(runs, json) diff --git a/shell/src/main/scala/orca/shell/cli/RunCli.scala b/shell/src/main/scala/orca/shell/cli/RunCli.scala index b830f4e7d..22360f06a 100644 --- a/shell/src/main/scala/orca/shell/cli/RunCli.scala +++ b/shell/src/main/scala/orca/shell/cli/RunCli.scala @@ -1,8 +1,11 @@ package orca.shell.cli +import orca.OrcaArgs import orca.shell.actions.{FlowResolution, RunAction} import orca.shell.run.{FallbackPolicy, FlowFlags, FlowLauncher} +import Cli.{actionFailure, complete, usageFailure, withTerminal} + /** `orca run`'s behavior (ADR 0021 §10): resolve the flow, read the task * (argument or piped stdin), then either the forced run ([[RunAction.run]]) or * the pin-honouring one ([[FlowLauncher.runHonoringPin]]), propagating the @@ -10,6 +13,11 @@ import orca.shell.run.{FallbackPolicy, FlowFlags, FlowLauncher} */ private[cli] object RunCli: + /** A contradictory `--worktree` pair is refused first, before anything is + * resolved or spawned, saving a `scala-cli` start and its dependency + * resolution. The flow child refuses it too, on this same shared decision, + * and stays the authority — this only makes the answer immediate. + */ def run( flowRef: String, task: Option[String], @@ -18,42 +26,47 @@ private[cli] object RunCli: workDir: os.Path, tty: Boolean ): Int = - FlowResolution.resolve(flowRef, workDir) match - case Left(message) => - Cli.diagnostic(message) - ExitCodes.ActionFailed - case Right(resolved) => - readTask(task, tty, readAllStdin) match - case Left(message) => - Cli.diagnostic(message) - ExitCodes.UsageError - case Right(taskText) => - Cli.withTerminal: terminal => - val result = - if honorPin then - FlowLauncher.runHonoringPin( - resolved.path, - taskText, - workDir, - flags, - terminal - ) - else - RunAction.run( - resolved, - taskText, - RunAction.RunOptions( - flags = flags, - fallback = - FallbackPolicy.Refuse("re-run with --honor-pin") - ), - workDir, - terminal - ) - // propagates the flow child's raw exit code (LaunchResult.Failed's - // exit, via Cli.exitCodeFor) — run mirrors a wrapped subprocess's - // status rather than the flat 0/1/2 usage-error convention. - Cli.exitCodeFor(result) + complete: + for + _ <- OrcaArgs + .worktreeRefusal( + worktree = flags.worktree, + skipBranch = flags.skipBranch, + keepChanges = flags.keepChanges + ) + .toLeft(()) + .left + .map(usageFailure) + resolved <- FlowResolution + .resolve(flowRef, workDir) + .left + .map(actionFailure) + taskText <- readTask(task, tty, readAllStdin).left.map(usageFailure) + yield withTerminal: terminal => + val result = + if honorPin then + FlowLauncher.runHonoringPin( + resolved.path, + taskText, + workDir, + flags, + terminal + ) + else + RunAction.run( + resolved, + taskText, + RunAction.RunOptions( + flags = flags, + fallback = FallbackPolicy.Refuse("re-run with --honor-pin") + ), + workDir, + terminal + ) + // propagates the flow child's raw exit code (LaunchResult.Failed's + // exit, via Cli.exitCodeFor) — run mirrors a wrapped subprocess's + // status rather than the flat 0/1/2 usage-error convention. + Cli.exitCodeFor(result) private def readAllStdin(): String = String(System.in.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8) diff --git a/shell/src/main/scala/orca/shell/cli/Tables.scala b/shell/src/main/scala/orca/shell/cli/Tables.scala index a5d9240cb..869aa23e4 100644 --- a/shell/src/main/scala/orca/shell/cli/Tables.scala +++ b/shell/src/main/scala/orca/shell/cli/Tables.scala @@ -25,6 +25,11 @@ private[cli] object Tables: private[cli] case class SessionRow( index: Int, sessionName: String, + /** The run's working directory: the listing spans worktrees, so two rows + * can otherwise be identical — and `continue ` then refuses them + * as ambiguous, naming directories the listing never showed. + */ + workDir: String, kind: String, stage: Option[String], harness: String, @@ -60,6 +65,7 @@ private[cli] object Tables: SessionRow( index = i + 1, sessionName = session.sessionName.getOrElse(session.agent), + workDir = selection.manifest.workDir, kind = session.kind.wireName, stage = session.stage, harness = SessionPicker.harnessSettingsName(session.harness), @@ -77,12 +83,15 @@ private[cli] object Tables: if asJson then println(writeToString(rows)) else if rows.isEmpty then println("(no sessions recorded)") else + // The same decision the interactive picker makes, over the same runs. + val tag = SessionPicker.dirTag(runs) val cols = rows.map: r => val status = if r.resumable then "" else s" not resumable: ${r.reason.getOrElse("")}" val sessionName = - r.sessionName + (if r.crashed then " (crashed)" else "") + r.sessionName + + (if r.crashed then " (crashed)" else "") + tag(r.workDir) ( r.index.toString, sessionName, diff --git a/shell/src/main/scala/orca/shell/resume/ResumeDetector.scala b/shell/src/main/scala/orca/shell/resume/ResumeDetector.scala index 5f4f89088..189328b76 100644 --- a/shell/src/main/scala/orca/shell/resume/ResumeDetector.scala +++ b/shell/src/main/scala/orca/shell/resume/ResumeDetector.scala @@ -5,7 +5,16 @@ import orca.progress.{ProgressHeader, ProgressScan, ProgressStore} /** An unfinished flow run, byte-identically relaunchable: the flow script's * filename and the exact task text that started it (ADR 0021 §3 amendment). */ -private[shell] case class InterruptedRun(flowName: String, userPrompt: String) +private[shell] case class InterruptedRun( + flowName: String, + userPrompt: String, + /** The directory the log was found in — the shell's own, or one of the + * worktrees orca made. The relaunch runs THERE, which is what makes it a + * resume rather than a fresh run: the log it resumes from is in that + * directory and nowhere else. + */ + dir: os.Path +) /** Detects an interrupted run for the main menu's "Resume interrupted run" * offer (ADR 0021 §3 amendment): a run that died mid-flight leaves its @@ -26,23 +35,33 @@ private[shell] object ResumeDetector: * (`flowName` unrecorded — the simpler, honest choice over a partial * pick-the-flow-and-prefill-the-task fallback). * + * `dirs` are the directories to scan ([[orca.shell.WorktreeScan.dirs]] picks + * them); the winner reports the one it was found in, so the caller can run + * the resume where its log actually is. Git is never asked here — the + * directories arrive resolved, which is what lets these scans be tested + * against bare temp directories. + * * Any failure here is silent, not surfaced as a menu warning: this runs on * every menu redraw (`Main.loop`), and a warning on each redraw would spam * the terminal for what is, at worst, a missed convenience. */ - def detect(workDir: os.Path): Option[InterruptedRun] = + def detect(dirs: List[os.Path]): Option[InterruptedRun] = try - newestProgressLogPath(workDir).flatMap: path => - ProgressStore.at(workDir, path).loadDetailed() match - case ProgressStore.LoadResult.Loaded(log) => fromHeader(log.header) - case _ => None + newestProgressLog(dirs).flatMap: found => + ProgressStore.at(found.dir, found.path).loadDetailed() match + case ProgressStore.LoadResult.Loaded(log) => + fromHeader(log.header, found.dir) + case _ => None catch case scala.util.control.NonFatal(_) => None - private def fromHeader(header: ProgressHeader): Option[InterruptedRun] = + private def fromHeader( + header: ProgressHeader, + dir: os.Path + ): Option[InterruptedRun] = for flowName <- header.flowName.filter(isBareFlowFilename) userPrompt <- header.userPrompt - yield InterruptedRun(flowName, userPrompt) + yield InterruptedRun(flowName, userPrompt, dir) /** The header is committed repo content, and `flowName` later reaches * `FlowResolution.resolve`, which treats path-like refs as literal paths — a @@ -54,9 +73,32 @@ private[shell] object ResumeDetector: name.endsWith(".sc") && !name.contains('/') && !name.contains('\\') && !name.startsWith("-") && !name.startsWith(".") - /** The newest by mtime of the scanned progress logs: with multiple unfinished - * logs (different prompts, one branch — possible), only the newest is ever - * offered; nothing else about the others is surfaced. + /** A scanned progress log: which directory it was found in, and where. Named + * rather than a pair, since both halves are `os.Path`. + */ + private case class FoundLog(dir: os.Path, path: os.Path, mtime: Long) + + /** The newest by mtime of the scanned progress logs — one ordering across + * every directory, not one winner per directory. With multiple unfinished + * logs (different prompts, one branch, or one per worktree) only the newest + * is ever offered; nothing else about the others is surfaced. */ - private def newestProgressLogPath(workDir: os.Path): Option[os.Path] = - ProgressScan.progressLogPaths(workDir).maxByOption(os.mtime(_)) + private def newestProgressLog(dirs: List[os.Path]): Option[FoundLog] = + dirs.flatMap(logsIn).maxByOption(_.mtime) + + /** One directory's logs, or none — a directory the shell neither created nor + * controls (an unreadable `.orca` in another worktree; a log another orca + * process removed on success between the listing and the read) must cost + * only itself, not the whole offer. Same rule `ProgressScan.progressLogs` + * states for a single bad file. + */ + private def logsIn(dir: os.Path): List[FoundLog] = + try + ProgressScan + .progressLogPaths(dir) + .flatMap: path => + scala.util + .Try(os.mtime(path)) + .toOption + .map(FoundLog(dir, path, _)) + catch case scala.util.control.NonFatal(_) => Nil diff --git a/shell/src/main/scala/orca/shell/run/FlowLauncher.scala b/shell/src/main/scala/orca/shell/run/FlowLauncher.scala index 9bc7f627d..54c0d9cef 100644 --- a/shell/src/main/scala/orca/shell/run/FlowLauncher.scala +++ b/shell/src/main/scala/orca/shell/run/FlowLauncher.scala @@ -26,11 +26,16 @@ private[shell] enum FallbackPolicy: * through: bundled, and built once at the CLI boundary, instead of a run of * adjacent same-typed positional Booleans that a call site could silently * transpose without a compile error. + * + * `worktree` combines with neither `skipBranch` nor `keepChanges`; a builder + * that can produce either pair has to ask `OrcaArgs.worktreeRefusal` (as + * [[orca.shell.cli.RunCli]] does), or the flow child refuses it after a spawn. */ private[shell] case class FlowFlags( verbose: Boolean, skipBranch: Boolean, - keepChanges: Boolean + keepChanges: Boolean, + worktree: Boolean ) /** Runs a selected flow as a `scala-cli run` child inheriting the shell's @@ -79,11 +84,11 @@ private[shell] object FlowLauncher: .getOrElse(Seq.empty) /** `scala-cli run --quiet --verbose [--dep ...] --workspace -- - * [--verbose] [--skip-branch] [--keep-changes]`. The `--verbose` - * before `--` is scala-cli's own ([[loggingArgs]]); the one after is the - * flow's, parsed by its own `OrcaArgs` — which spells its flags - * `--verbose`/`--skip-branch`/`--keep-changes`, so they land after `--` - * alongside the task text, not before it. `--workspace` relocates + * [--verbose] [--skip-branch] [--keep-changes] [--worktree]`. The + * `--verbose` before `--` is scala-cli's own ([[loggingArgs]]); the one + * after is the flow's, parsed by its own `OrcaArgs` — which spells its flags + * `--verbose`/`--skip-branch`/`--keep-changes`/`--worktree`, so they land + * after `--` alongside the task text, not before it. `--workspace` relocates * scala-cli's own `.scala-build`/`.bsp` build metadata to `workspaceDir` * ([[resolveWorkspaceDir]]) instead of next to `flow` — load-bearing for a * Project-tier flow, whose script lives inside the user's own repo @@ -110,11 +115,13 @@ private[shell] object FlowLauncher: if flags.skipBranch then Seq("--skip-branch") else Seq.empty val keepChangesArgs = if flags.keepChanges then Seq("--keep-changes") else Seq.empty + val worktreeArgs = if flags.worktree then Seq("--worktree") else Seq.empty Seq("scala-cli", "run", flow.toString) ++ loggingArgs ++ depArgs(orcaVersion) ++ Seq("--workspace", workspaceDir.toString) ++ - Seq("--", task) ++ verboseArgs ++ skipBranchArgs ++ keepChangesArgs + Seq("--", task) ++ + verboseArgs ++ skipBranchArgs ++ keepChangesArgs ++ worktreeArgs /** The compile probe's argv — same `--workspace` treatment as [[argv]], and * for the same reason: without it, the probe (run whenever the forced diff --git a/shell/src/main/scala/orca/shell/sessions/ManifestReader.scala b/shell/src/main/scala/orca/shell/sessions/ManifestReader.scala index f234f0223..5450cf101 100644 --- a/shell/src/main/scala/orca/shell/sessions/ManifestReader.scala +++ b/shell/src/main/scala/orca/shell/sessions/ManifestReader.scala @@ -17,16 +17,54 @@ private[shell] case class RecordedRun(manifest: RunManifest, crashed: Boolean) */ private[shell] object ManifestReader: - /** Newest-first by `startedAt`. A manifest with [[ManifestOutcome.Running]] - * whose `pid` is no longer alive is a crashed run — its sessions are still - * offered, per ADR 0021 §8. Reads `.orca/cache/runs/` passively - * ([[OrcaDir.runsPath]], not [[OrcaDir.cacheRunsPath]]) — an absent or empty - * directory yields `(Nil, Nil)` without creating anything on disk. A file - * that fails to parse as JSON, or doesn't match the `RunManifest` schema — - * which includes a timestamp that isn't an `Instant` — is skipped with a - * warning naming the file rather than aborting the whole listing. + /** Newest-first by `startedAt` across `own` and every directory in + * `otherWorktrees` — a `--worktree` run keeps its manifests in its own tree, + * so the listing spans the shell's checkout and orca's worktrees of it + * ([[orca.shell.WorktreeScan.dirs]] picks them). Git is never asked here: + * the directories arrive resolved, which is what lets these tests seed bare + * temp directories. + * + * `own` is the directory the caller is standing in and is read strictly: a + * symlinked `.orca` there redirects a read of the user's own tree, and + * aborting is the signal. The others are read guarded — one warning each — + * since the shell neither created nor controls them for the length of a + * redraw (another orca process finishing, a `git worktree remove` in another + * terminal, a tree left unreadable by a run under a different uid). Two + * parameters rather than one list, because that is the whole difference + * between them. + * + * A manifest with [[ManifestOutcome.Running]] whose `pid` is no longer alive + * is a crashed run — its sessions are still offered, per ADR 0021 §8. Each + * directory's `.orca/cache/runs/` is read passively ([[OrcaDir.runsPath]], + * not [[OrcaDir.cacheRunsPath]]) — absent or empty contributes nothing and + * creates nothing on disk. A file that fails to parse as JSON, or doesn't + * match the `RunManifest` schema — which includes a timestamp that isn't an + * `Instant` — is skipped with a warning naming the file rather than aborting + * the whole listing. */ def list( + own: os.Path, + otherWorktrees: List[os.Path], + pidAlive: Long => Boolean + ): (List[RecordedRun], List[String]) = + val perDir = + readRunsDir(own, pidAlive) :: otherWorktrees.map(guarded(_, pidAlive)) + ( + perDir.flatMap(_._1).sortBy(_.manifest.startedAt).reverse, + perDir.flatMap(_._2) + ) + + private def guarded( + workDir: os.Path, + pidAlive: Long => Boolean + ): (List[RecordedRun], List[String]) = + try readRunsDir(workDir, pidAlive) + catch case NonFatal(e) => (Nil, List(s"skipping $workDir: ${firstLine(e)}")) + + /** One directory's manifests, in reverse file order (the caller sorts), and + * its warnings in file order. + */ + private def readRunsDir( workDir: os.Path, pidAlive: Long => Boolean ): (List[RecordedRun], List[String]) = @@ -49,7 +87,7 @@ private[shell] object ManifestReader: RecordedRun(manifest, crashed(manifest, pidAlive)) :: runs, warnings ) - (runs.sortBy(_.manifest.startedAt).reverse, warnings.reverse) + (runs, warnings.reverse) /** An outcome this build doesn't know (a newer build's manifest) is a run * that reached some terminal state, not a crashed one — only `Running` @@ -71,9 +109,9 @@ private[shell] object ManifestReader: * * Only the first line of a decode failure is reported. jsoniter appends a * multi-line hex dump of the buffer to its message, and `Main`'s loop - * reprints every warning on each menu redraw, for up to 20 kept manifests — - * so the untrimmed message paints the menu over. Same trimming, and same - * reason, as `ProgressStore.parseLog`. + * reprints every warning on each menu redraw, for up to 20 kept manifests in + * each scanned directory — so the untrimmed message paints the menu over. + * Same trimming, and same reason, as `ProgressStore.parseLog`. */ private def readManifest(file: os.Path): Either[String, RunManifest] = try diff --git a/shell/src/main/scala/orca/shell/sessions/SessionPicker.scala b/shell/src/main/scala/orca/shell/sessions/SessionPicker.scala index 974604ba3..df1d21c34 100644 --- a/shell/src/main/scala/orca/shell/sessions/SessionPicker.scala +++ b/shell/src/main/scala/orca/shell/sessions/SessionPicker.scala @@ -69,8 +69,14 @@ private[shell] object SessionPicker: yield Occurrence(run, session) val (durable, oneShot) = occurrences.partition(isDurable) + // Keyed on the working directory too: harness sessions are cwd-scoped, and + // flow session names are static ("implementer" in every run), so the same + // name in two worktrees is two different conversations about two different + // tasks — deduping them against each other would hide one behind the other. val lineages = durable - .groupBy(o => (o.session.agent, o.session.sessionName)) + .groupBy(o => + (o.run.manifest.workDir, o.session.agent, o.session.sessionName) + ) .values .map(_.sortBy(recency).reverse) .toList @@ -78,12 +84,17 @@ private[shell] object SessionPicker: val earlier = lineages.flatMap(_.tail).sortBy(recency).reverse val oneShotSorted = oneShot.sortBy(recency).reverse - val primaryRows = primary.map(o => resumeRow(o, primaryLabel(o))) + val tag = dirTag(runs) + val where = (o: Occurrence) => tag(o.run.manifest.workDir) + + val primaryRows = primary.map(o => resumeRow(o, primaryLabel(o) + where(o))) val earlierRows = - if expanded then earlier.map(o => resumeRow(o, earlierLabel(o))) + if expanded then + earlier.map(o => resumeRow(o, earlierLabel(o) + where(o))) else expanderRow(earlier.size, "earlier occurrence") val oneShotRows = - if expanded then oneShotSorted.map(o => resumeRow(o, oneShotLabel(o))) + if expanded then + oneShotSorted.map(o => resumeRow(o, oneShotLabel(o) + where(o))) else expanderRow( oneShotSorted.size, @@ -103,6 +114,24 @@ private[shell] object SessionPicker: private def recency(o: Occurrence): Instant = o.session.lastActiveAt + /** How a row says which tree its session is in, given the runs being + * rendered: a suffix per `workDir`, or nothing at all when they share one — + * then it would tell the user nothing. The interactive picker and `orca + * continue --list` both call this over the same runs, so the two surfaces + * cannot drift on either the rule or the marker's shape. + */ + private[shell] def dirTag(runs: List[RecordedRun]): String => String = + if runs.map(_.manifest.workDir).distinct.sizeIs > 1 then + workDir => s" @${lastSegment(workDir)}" + else _ => "" + + /** A recorded `workDir`'s final segment. String-sliced, not `os.Path`-parsed: + * the value is manifest content, and a hand-edited one need not be an + * absolute path. + */ + private def lastSegment(workDir: String): String = + workDir.split('/').filter(_.nonEmpty).lastOption.getOrElse(workDir) + private def resumeRow(o: Occurrence, label: String): Choice[PickerRow] = Choice( PickerRow.Resume( @@ -251,8 +280,15 @@ private[shell] object SessionPicker: notFound ) case multiple => - val agents = multiple.map(_._2.session.agent).distinct.mkString(", ") - Left(s"'$name' is ambiguous — matches agents: $agents") + // Same name in two worktrees matches on one agent, so naming agents + // alone would read as "ambiguous — matches agents: coder". + val agents = multiple.map(_._2.session.agent).distinct + val detail = + if agents.sizeIs > 1 then s"agents: ${agents.mkString(", ")}" + else + val dirs = multiple.map(_._2.manifest.workDir).distinct + s"working directories: ${dirs.mkString(", ")}" + Left(s"'$name' is ambiguous — matches $detail") /** [[sessionRows]]'s rows, dropping the "show more" expanders — never present * for [[SessionSelection]] callers (`selectByIndex` reads the fully expanded diff --git a/shell/src/test/scala/orca/shell/MainMenuTest.scala b/shell/src/test/scala/orca/shell/MainMenuTest.scala index 54aac7b59..97547b556 100644 --- a/shell/src/test/scala/orca/shell/MainMenuTest.scala +++ b/shell/src/test/scala/orca/shell/MainMenuTest.scala @@ -103,11 +103,13 @@ class MainMenuTest extends munit.FunSuite: ): val run = InterruptedRun( flowName = "implement.sc", - userPrompt = "fix the flaky integration test in the payments module" + userPrompt = "fix the flaky integration test in the payments module", + dir = os.root / "work" ) val choices = MainMenu.choices( continueSessionCount = None, - resumeOffer = Some(run) + resumeOffer = Some(run), + workDir = run.dir ) assertEquals( choices.map(_.value).take(2), @@ -123,11 +125,13 @@ class MainMenuTest extends munit.FunSuite: test("choices(resumeOffer = Some(...)) label drops control characters"): val run = InterruptedRun( flowName = "fix.sc", - userPrompt = "safe\u001b[31m text\u0007" + userPrompt = "safe\u001b[31m text\u0007", + dir = os.root / "work" ) val choices = MainMenu.choices( continueSessionCount = None, - resumeOffer = Some(run) + resumeOffer = Some(run), + workDir = run.dir ) val label = choices.find(_.value == MenuItem.ResumeRun).get.label assert(!label.exists(_.isControl), label) @@ -137,11 +141,13 @@ class MainMenuTest extends munit.FunSuite: ): val run = InterruptedRun( flowName = "fix.sc", - userPrompt = "line one\nline two" + userPrompt = "line one\nline two", + dir = os.root / "work" ) val choices = MainMenu.choices( continueSessionCount = None, - resumeOffer = Some(run) + resumeOffer = Some(run), + workDir = run.dir ) assertEquals( choices.find(_.value == MenuItem.ResumeRun).map(_.label), @@ -153,7 +159,7 @@ class MainMenuTest extends munit.FunSuite: ): val withOffer = MainMenu.choices( continueSessionCount = None, - resumeOffer = Some(InterruptedRun("a.sc", "short task")) + resumeOffer = Some(InterruptedRun("a.sc", "short task", os.root / "work")) ) val without = MainMenu .choices(continueSessionCount = None) @@ -170,3 +176,19 @@ class MainMenuTest extends munit.FunSuite: byValue(MenuItem.EditSettings), "Edit settings — open the project or global settings file in your editor" ) + + test("the resume offer names the worktree when the log is not in this tree"): + val run = InterruptedRun( + flowName = "implement.sc", + userPrompt = "fix the flaky test", + dir = os.root / "repo" / ".orca" / "worktrees" / "ab12cd34" + ) + val label = MainMenu + .choices( + continueSessionCount = None, + resumeOffer = Some(run), + workDir = os.root / "repo" + ) + .find(_.value == MenuItem.ResumeRun) + .map(_.label) + assert(label.exists(_.endsWith("(in ab12cd34)")), label.toString) diff --git a/shell/src/test/scala/orca/shell/MainTest.scala b/shell/src/test/scala/orca/shell/MainTest.scala index fdafd9125..a6cba9cf4 100644 --- a/shell/src/test/scala/orca/shell/MainTest.scala +++ b/shell/src/test/scala/orca/shell/MainTest.scala @@ -13,7 +13,7 @@ import orca.shell.actions.{SettingsEditAction, StackAction} import orca.shell.create.CreateTier import orca.shell.flows.{DiscoveredFlow, FlowOrigin} import orca.shell.resume.InterruptedRun -import orca.shell.run.LaunchResult +import orca.shell.run.{FlowFlags, LaunchResult} import orca.shell.sessions.{RecordedRun, SessionPicker, SessionSelection} import orca.shell.ui.{Choice, ShellUi, UiOutcome} import orca.testkit.TempDirs @@ -21,11 +21,12 @@ import orca.testkit.TempDirs import java.time.Instant /** Answers a single fixed `confirm` outcome, recording the question it was - * asked; every other prompt is unsupported — [[Main.rediscoverStack]] and - * [[Main.promptCreateBranch]] only ever call `confirm`. + * asked and the default offered; every other prompt is unsupported — + * [[Main.rediscoverStack]] only ever calls `confirm`. */ private class ConfirmOnlyUi(outcome: UiOutcome[Boolean]) extends ShellUi: var recordedQuestion: Option[String] = None + var recordedDefault: Option[Boolean] = None def select[A]( title: String, choices: List[Choice[A]], @@ -34,35 +35,40 @@ private class ConfirmOnlyUi(outcome: UiOutcome[Boolean]) extends ShellUi: throw new UnsupportedOperationException("rediscoverStack doesn't select") def confirm(question: String, default: Boolean): UiOutcome[Boolean] = recordedQuestion = Some(question) + recordedDefault = Some(default) outcome def input(prompt: String, default: Option[String] = None): UiOutcome[String] = throw new UnsupportedOperationException("rediscoverStack doesn't input") def inputMultiline(prompt: String): UiOutcome[String] = throw new UnsupportedOperationException("rediscoverStack doesn't input") -/** Records every `select` call's shown choices (in shown order) and always - * answers with the fixed `outcome` — used to verify [[Main.pickFlow]] hands - * `ui.select` the ALREADY-reordered list, not just that the pure - * `promoteByName`/`reorder` helper computes the right order in isolation. - * `confirm`/`input` are unsupported: `pickFlow` never calls them. +/** Records every `select` call's shown choices (in shown order) and the + * preselection offered, and always answers with the fixed `outcome` — used to + * verify [[Main.pickFlow]] hands `ui.select` the ALREADY-reordered list (not + * just that the pure `promoteByName`/`reorder` helper computes the right order + * in isolation), and that [[Main.promptRunTarget]] offers all three + * destinations with the default preselected. `confirm`/`input` are + * unsupported: neither caller uses them. */ -private class RecordingSelectUi(outcome: UiOutcome[DiscoveredFlow]) - extends ShellUi: - private var shown: List[List[Choice[DiscoveredFlow]]] = Nil - def recordedChoices: List[List[Choice[DiscoveredFlow]]] = shown +private class RecordingSelectUi[T](outcome: UiOutcome[T]) extends ShellUi: + private var shown: List[List[Choice[T]]] = Nil + private var preselected: List[Option[T]] = Nil + def recordedChoices: List[List[Choice[T]]] = shown + def recordedPreselect: Option[Option[T]] = preselected.headOption def select[A]( title: String, choices: List[Choice[A]], preselect: Option[A] = None ): UiOutcome[A] = - shown = shown :+ choices.asInstanceOf[List[Choice[DiscoveredFlow]]] + shown = shown :+ choices.asInstanceOf[List[Choice[T]]] + preselected = preselected :+ preselect.asInstanceOf[Option[T]] outcome.asInstanceOf[UiOutcome[A]] def confirm(question: String, default: Boolean): UiOutcome[Boolean] = - throw new UnsupportedOperationException("pickFlow doesn't confirm") + throw new UnsupportedOperationException("neither caller confirms") def input(prompt: String, default: Option[String] = None): UiOutcome[String] = - throw new UnsupportedOperationException("pickFlow doesn't input") + throw new UnsupportedOperationException("neither caller inputs") def inputMultiline(prompt: String): UiOutcome[String] = - throw new UnsupportedOperationException("pickFlow doesn't input") + throw new UnsupportedOperationException("neither caller inputs") /** Counts calls to `select`/`inputMultiline`/`input` and replays queued * outcomes for each — used to verify `Main.createNewFlow`/`createForkFlow` @@ -73,11 +79,13 @@ private class RecordingSelectUi(outcome: UiOutcome[DiscoveredFlow]) private class FlowScriptedUi( selectScript: List[UiOutcome[Any]] = Nil, inputMultilineScript: List[UiOutcome[String]] = Nil, - inputScript: List[UiOutcome[String]] = Nil + inputScript: List[UiOutcome[String]] = Nil, + confirmScript: List[UiOutcome[Boolean]] = Nil ) extends ShellUi: private var pendingSelect = selectScript private var pendingInputMultiline = inputMultilineScript private var pendingInput = inputScript + private var pendingConfirm = confirmScript var selectCount = 0 var inputMultilineCount = 0 var inputCount = 0 @@ -93,9 +101,13 @@ private class FlowScriptedUi( outcome.asInstanceOf[UiOutcome[A]] def confirm(question: String, default: Boolean): UiOutcome[Boolean] = - throw new UnsupportedOperationException( - "createNewFlow/createForkFlow don't confirm" - ) + if pendingConfirm.isEmpty then + throw new UnsupportedOperationException( + "createNewFlow/createForkFlow don't confirm" + ) + val outcome = pendingConfirm.head + pendingConfirm = pendingConfirm.tail + outcome def input(prompt: String, default: Option[String] = None): UiOutcome[String] = inputCount += 1 @@ -530,7 +542,7 @@ class MainTest extends munit.FunSuite: test( "pickFlow: the run picker's reorder promotes implement.sc to the front of what ui.select shows" ): - val ui = new RecordingSelectUi(UiOutcome.Cancelled) + val ui = new RecordingSelectUi[DiscoveredFlow](UiOutcome.Cancelled) val _ = Main.pickFlow( ui, @@ -546,48 +558,90 @@ class MainTest extends munit.FunSuite: test( "pickFlow: view/edit pickers (no reorder given) stay alphabetical" ): - val ui = new RecordingSelectUi(UiOutcome.Cancelled) + val ui = new RecordingSelectUi[DiscoveredFlow](UiOutcome.Cancelled) val _ = Main.pickFlow(ui, "View which flow?", threeFlows) assertEquals( ui.recordedChoices.head.map(_.value.name), List("alpha.sc", "implement.sc", "zeta.sc") ) - // --- promptCreateBranch (the branch-creation confirm before a run) --- + // --- promptRunTarget (where the run's work goes, asked as one choice) --- - test("promptCreateBranch: the question explains what declining does"): - val ui = ConfirmOnlyUi(UiOutcome.Selected(true)) - assertEquals(Main.promptCreateBranch(ui), Some(true)) + test("promptRunTarget: the three destinations are offered, new branch first"): + val ui = RecordingSelectUi(UiOutcome.Selected(RunTarget.NewBranch)) + assertEquals(Main.promptRunTarget(ui), Some(RunTarget.NewBranch)) assertEquals( - ui.recordedQuestion, - Some( - "Create a new branch for this run? (choosing 'no': the flow makes " + - "its changes on the current branch)" - ) + ui.recordedChoices.head.map(_.value), + List(RunTarget.NewBranch, RunTarget.CurrentBranch, RunTarget.Worktree) ) test( - "promptCreateBranch: confirming (Enter's default) keeps normal branch-creating behavior" + "promptRunTarget: a new branch leads the rows and is marked preselected" ): - assertEquals( - Main.promptCreateBranch(ConfirmOnlyUi(UiOutcome.Selected(true))), - Some(true) - ) + val ui = RecordingSelectUi(UiOutcome.Selected(RunTarget.NewBranch)) + assertEquals(Main.promptRunTarget(ui), Some(RunTarget.NewBranch)) + assertEquals(ui.recordedPreselect, Some(Some(RunTarget.NewBranch))) - test( - "promptCreateBranch: declining selects skip-branch mode (caller negates to skipBranch = true)" - ): + test("promptRunTarget: cancelling aborts the run"): assertEquals( - Main.promptCreateBranch(ConfirmOnlyUi(UiOutcome.Selected(false))), - Some(false) + Main.promptRunTarget(RecordingSelectUi[RunTarget](UiOutcome.Cancelled)), + None ) - test("promptCreateBranch: cancelling aborts the run"): + test("RunTarget: each destination maps to one flag pair, never both"): + // The pair orca refuses (`--worktree` with `--skip-branch`) has no case + // that produces it — which is the point of asking once rather than twice. assertEquals( - Main.promptCreateBranch(ConfirmOnlyUi(UiOutcome.Cancelled)), - None + RunTarget.values.toList.map(t => (t.skipBranch, t.worktree)), + List((false, false), (true, false), (false, true)) ) + // --- runFlow (the interactive launch path) --- + + test("runFlow: the chosen destination reaches the launcher's flags"): + withDumbTerminal: terminal => + val workDir = TempDirs.dir() + os.write( + workDir / ".orca" / "flows" / "run-flow.sc", + "// x\n", + createFolders = true + ) + val flow = DiscoveredFlow( + name = "run-flow.sc", + description = None, + origin = FlowOrigin.Project, + path = workDir / ".orca" / "flows" / "run-flow.sc", + shadows = Nil + ) + // Pick the flow, type the task, then pick the worktree destination. + val ui = FlowScriptedUi( + selectScript = List( + UiOutcome.Selected(flow), + UiOutcome.Selected(RunTarget.Worktree) + ), + inputMultilineScript = List(UiOutcome.Selected("do the thing")) + ) + var recorded: Option[FlowFlags] = None + Main.runFlow( + ui, + terminal, + workDir, + runAction = (_, _, opts, _, _) => + recorded = Some(opts.flags) + LaunchResult.Ok + ) + assertEquals( + recorded, + Some( + FlowFlags( + verbose = false, + skipBranch = false, + keepChanges = false, + worktree = true + ) + ) + ) + // --- editFlow / createNewFlow / createForkFlow (ADR 0021 §6/§9 amendment: // hand-vs-agent mode) --- // @@ -931,14 +985,14 @@ class MainTest extends munit.FunSuite: ) val run = InterruptedRun( flowName = "resume-flow.sc", - userPrompt = "fix the flaky test\nwith detail" + userPrompt = "fix the flaky test\nwith detail", + dir = workDir ) var recorded: Option[(String, String)] = None Main.resumeInterruptedRun( FlowScriptedUi(), terminal, run, - workDir, runAction = (flow, task, _, _, _) => recorded = Some(flow.name -> task) LaunchResult.Ok @@ -948,19 +1002,60 @@ class MainTest extends munit.FunSuite: Some("resume-flow.sc" -> "fix the flaky test\nwith detail") ) + test( + "resumeInterruptedRun: the run happens in the directory its log was found in" + ): + withDumbTerminal: terminal => + // A log found in an orca worktree resumes THERE, with no --worktree flag: + // the flag would re-derive a path, this runs where the log actually is. + val worktree = TempDirs.dir() + os.write( + worktree / ".orca" / "flows" / "resume-flow.sc", + "// x\n", + createFolders = true + ) + val run = InterruptedRun( + flowName = "resume-flow.sc", + userPrompt = "fix the flaky test", + dir = worktree + ) + var recorded: Option[(os.Path, FlowFlags)] = None + Main.resumeInterruptedRun( + FlowScriptedUi(), + terminal, + run, + runAction = (_, _, opts, dir, _) => + recorded = Some(dir -> opts.flags) + LaunchResult.Ok + ) + assertEquals( + recorded, + Some( + worktree -> FlowFlags( + verbose = false, + skipBranch = false, + keepChanges = false, + worktree = false + ) + ) + ) + test( "resumeInterruptedRun: an unresolvable flow name reports an error and never launches" ): withDumbTerminal: terminal => val workDir = TempDirs.dir() - val run = InterruptedRun(flowName = "no-such-flow.sc", userPrompt = "x") + val run = InterruptedRun( + flowName = "no-such-flow.sc", + userPrompt = "x", + dir = workDir + ) var launched = false val out = captured( Main.resumeInterruptedRun( FlowScriptedUi(), terminal, run, - workDir, runAction = (_, _, _, _, _) => { launched = true; LaunchResult.Ok } ) ) diff --git a/shell/src/test/scala/orca/shell/WorktreeScanTest.scala b/shell/src/test/scala/orca/shell/WorktreeScanTest.scala new file mode 100644 index 000000000..480323eda --- /dev/null +++ b/shell/src/test/scala/orca/shell/WorktreeScanTest.scala @@ -0,0 +1,77 @@ +package orca.shell + +import orca.OrcaDir +import orca.testkit.{GitRepo, TempDirs} +import orca.tools.Worktrees +import ox.discard + +class WorktreeScanTest extends munit.FunSuite: + + test("outside a git repository the shell's own directory is the whole list"): + val dir = TempDirs.dir() + assertEquals(WorktreeScan.dirs(dir).all, List(dir)) + + test("an orca worktree is scanned alongside the checkout"): + val repo = GitRepo.seeded() + val worktree = OrcaDir.ensureWorktrees(repo) / "0123456789ab" + assertEquals(Worktrees.add(repo, worktree), Right(())) + assertEquals(WorktreeScan.dirs(repo).all, List(repo, worktree)) + + test("a worktree scans only itself, not the checkout that made it"): + val repo = GitRepo.seeded() + val worktree = OrcaDir.ensureWorktrees(repo) / "0123456789ab" + assertEquals(Worktrees.add(repo, worktree), Right(())) + // git lists the checkout and the worktree from either one; the scan is + // scoped to the `.orca/worktrees/` of the directory it is asked about, and + // a worktree's own is empty. + assertEquals(WorktreeScan.dirs(worktree).all, List(worktree)) + + test("a worktree does not scan its siblings"): + val repo = GitRepo.seeded() + val ours = OrcaDir.ensureWorktrees(repo) / "0123456789ab" + val sibling = OrcaDir.ensureWorktrees(repo) / "ba9876543210" + assertEquals(Worktrees.add(repo, ours), Right(())) + assertEquals(Worktrees.add(repo, sibling), Right(())) + assertEquals(WorktreeScan.dirs(ours).all, List(ours)) + // Both are visible from the checkout they hang off. + assertEquals(WorktreeScan.dirs(repo).all.toSet, Set(repo, ours, sibling)) + + test("a worktree orca did not create is left out"): + val repo = GitRepo.seeded() + val theirs = repo / "review-their-branch" + assertEquals(Worktrees.add(repo, theirs), Right(())) + // Its branch's committed progress log would otherwise become a resume + // offer, handing that branch's task text to an agent. + assertEquals(WorktreeScan.dirs(repo).all, List(repo)) + + test("worktrees are ranked by when each last recorded a run"): + val repo = GitRepo.seeded() + val worktrees = List("aaaaaaaaaaaa", "bbbbbbbbbbbb", "cccccccccccc").map: + name => + val path = OrcaDir.ensureWorktrees(repo) / name + assertEquals(Worktrees.add(repo, path), Right(())) + os.makeDir.all(OrcaDir.runsPath(path)) + path + // Newest run last-recorded wins; the shell's own checkout stays first. + os.mtime.set(OrcaDir.runsPath(worktrees(0)), 1000L).discard + os.mtime.set(OrcaDir.runsPath(worktrees(2)), 3000L).discard + os.mtime.set(OrcaDir.runsPath(worktrees(1)), 2000L).discard + assertEquals( + WorktreeScan.dirs(repo).all, + List(repo, worktrees(2), worktrees(1), worktrees(0)) + ) + + test("only the most recently used worktrees are scanned, up to the cap"): + val repo = GitRepo.seeded() + val worktrees = (0 to WorktreeScan.MaxScannedWorktrees).toList.map: i => + val path = OrcaDir.ensureWorktrees(repo) / f"wt$i%012d" + assertEquals(Worktrees.add(repo, path), Right(())) + os.makeDir.all(OrcaDir.runsPath(path)) + // Ascending mtimes, so index 0 is the least recently used. + os.mtime.set(OrcaDir.runsPath(path), 1000L + i * 1000L).discard + path + val scanned = WorktreeScan.dirs(repo) + assertEquals(scanned.worktrees.size, WorktreeScan.MaxScannedWorktrees) + // The one dropped is the oldest, and the newest is kept and comes first. + assert(!scanned.worktrees.contains(worktrees.head), "oldest must drop out") + assertEquals(scanned.worktrees.head, worktrees.last) diff --git a/shell/src/test/scala/orca/shell/actions/AuthorActionTest.scala b/shell/src/test/scala/orca/shell/actions/AuthorActionTest.scala index 10cf78ba4..929566db3 100644 --- a/shell/src/test/scala/orca/shell/actions/AuthorActionTest.scala +++ b/shell/src/test/scala/orca/shell/actions/AuthorActionTest.scala @@ -134,7 +134,12 @@ class AuthorActionTest extends munit.FunSuite: assertEquals(call.flow, builtInFlow) assertEquals( call.flags, - FlowFlags(verbose = false, skipBranch = false, keepChanges = false) + FlowFlags( + verbose = false, + skipBranch = false, + keepChanges = false, + worktree = false + ) ) assertEquals(call.fallback, FallbackPolicy.Ask(NoPromptUi)) assert(call.task.contains("sync issues nightly"), call.task) diff --git a/shell/src/test/scala/orca/shell/actions/RunActionTest.scala b/shell/src/test/scala/orca/shell/actions/RunActionTest.scala index 38c6eaa4d..623facdad 100644 --- a/shell/src/test/scala/orca/shell/actions/RunActionTest.scala +++ b/shell/src/test/scala/orca/shell/actions/RunActionTest.scala @@ -17,7 +17,12 @@ class RunActionTest extends munit.FunSuite: // A non-default combination, so a launcher handed defaults of its own // instead of these fails here. val flags = - FlowFlags(verbose = true, skipBranch = false, keepChanges = true) + FlowFlags( + verbose = true, + skipBranch = false, + keepChanges = true, + worktree = false + ) val recording = RecordingLaunch() val result = RunAction.run( diff --git a/shell/src/test/scala/orca/shell/cli/CliTest.scala b/shell/src/test/scala/orca/shell/cli/CliTest.scala index 2e879dc3d..63f4383de 100644 --- a/shell/src/test/scala/orca/shell/cli/CliTest.scala +++ b/shell/src/test/scala/orca/shell/cli/CliTest.scala @@ -9,6 +9,7 @@ import orca.runner.manifest.{ RunManifest } import orca.settings.{AgentSettings, AgentSpec, SettingsFile} +import orca.shell.ScanDirs import orca.shell.create.CreateTier import orca.shell.flows.{DiscoveredFlow, FlowOrigin} import orca.shell.run.LaunchResult @@ -64,6 +65,50 @@ class CliTest extends munit.FunSuite: Right(1) ) + test( + "run: --worktree parses too (fails later, at flow resolution)" + ): + assertEquals( + invoke("run", "no-such-flow.sc", "a task", "--worktree"), + Right(1) + ) + + test( + "run: --worktree with --skip-branch is refused before the flow is resolved" + ): + // A usage error rather than the flow-not-found 1 the cases above get: the + // refusal runs first, so no scala-cli is ever spawned. + val (_, err) = capturedBoth( + assertEquals( + invoke( + "run", + "no-such-flow.sc", + "a task", + "--worktree", + "--skip-branch" + ), + Right(ExitCodes.UsageError) + ) + ) + assert(err.contains("--worktree") && err.contains("--skip-branch"), err) + + test("run: --worktree with --keep-changes is refused the same way"): + val (_, err) = capturedBoth( + assertEquals( + invoke( + "run", + "no-such-flow.sc", + "a task", + "--worktree", + "--keep-changes" + ), + Right(ExitCodes.UsageError) + ) + ) + assert(err.contains("--worktree") && err.contains("--keep-changes"), err) + // Names the pair the user actually typed, not the other one. + assert(!err.contains("--skip-branch"), err) + test("run: the required flow positional missing is a usage error"): assert(!parses("run")) @@ -923,6 +968,72 @@ class CliTest extends munit.FunSuite: Left("'shared' is ambiguous — matches agents: agentA, agentB") ) + test( + "selectByName: the same name in two worktrees is ambiguous, naming the trees" + ): + // Flow session names are static, so two parallel --worktree runs on + // unrelated tasks both record "shared" under the same agent. They are + // different conversations (harness sessions are cwd-scoped) and must not + // resolve silently to the newer one. + val runs = List( + RecordedRun( + manifest( + workDir = "/repo/.orca/worktrees/aaaaaaaaaaaa", + startedAt = "2026-07-18T09:00:00Z", + sessions = List(durable("shared", "2026-07-18T09:30:00Z")) + ), + crashed = false + ), + RecordedRun( + manifest( + workDir = "/repo/.orca/worktrees/bbbbbbbbbbbb", + startedAt = "2026-07-18T08:00:00Z", + sessions = List(durable("shared", "2026-07-18T08:30:00Z")) + ), + crashed = false + ) + ) + assertEquals( + SessionPicker.selectByName(runs, "shared"), + Left( + "'shared' is ambiguous — matches working directories: " + + "/repo/.orca/worktrees/aaaaaaaaaaaa, /repo/.orca/worktrees/bbbbbbbbbbbb" + ) + ) + // Both are primary rows, and each says which tree it is in. + val labels = SessionPicker + .withoutExpanders(SessionPicker.sessionRows(runs, expanded = false)) + .map(_.label) + assertEquals(labels.count(_.contains("★")), 2) + assert(labels.exists(_.contains("@aaaaaaaaaaaa")), labels.toString) + assert(labels.exists(_.contains("@bbbbbbbbbbbb")), labels.toString) + + test( + "successive runs in ONE directory still collapse into a single lineage" + ): + val runs = List( + RecordedRun( + manifest( + startedAt = "2026-07-18T09:00:00Z", + sessions = List(durable("shared", "2026-07-18T09:30:00Z")) + ), + crashed = false + ), + RecordedRun( + manifest( + startedAt = "2026-07-18T08:00:00Z", + sessions = List(durable("shared", "2026-07-18T08:30:00Z")) + ), + crashed = false + ) + ) + val labels = SessionPicker + .withoutExpanders(SessionPicker.sessionRows(runs, expanded = false)) + .map(_.label) + assertEquals(labels.count(_.contains("★")), 1) + // One directory, so nothing to disambiguate. + assert(!labels.exists(_.contains("@")), labels.toString) + test( "sessionListingRows numbers rows 1-based in the same order continue uses" ): @@ -995,7 +1106,13 @@ class CliTest extends munit.FunSuite: val (out, err) = capturedBoth( assertEquals( ContinueCli - .runContinue(dir, None, list = true, json = true, tty = false), + .runContinue( + ScanDirs(dir, Nil), + None, + list = true, + json = true, + tty = false + ), ExitCodes.Ok ) ) @@ -1039,13 +1156,100 @@ class CliTest extends munit.FunSuite: createFolders = true ) + private def writeSessionManifest(dir: os.Path, workDir: String): Unit = + val json = + s"""{ + | "orcaVersion": "0.0.test", + | "workDir": "$workDir", + | "pid": 1, + | "startedAt": "2026-07-18T09:00:00Z", + | "outcome": "succeeded", + | "sessions": [{ + | "harness": "ClaudeCode", + | "wireId": "uuid", + | "agent": "main", + | "sessionName": "implementer", + | "kind": "durable", + | "firstSeenAt": "2026-07-18T09:00:00Z", + | "lastActiveAt": "2026-07-18T09:00:00Z" + | }] + |}""".stripMargin + os.write( + dir / ".orca" / "cache" / "runs" / "a.json", + json, + createFolders = true + ) + + test("runContinue --list: rows across worktrees say which tree each is in"): + val checkout = TempDirs.dir() + val worktree = TempDirs.dir() + // The same static flow session name in two trees — the suffix is the only + // thing telling the user which one `orca continue ` reattaches to. + writeSessionManifest(checkout, "/repo") + writeSessionManifest(worktree, "/repo/.orca/worktrees/ab12cd34") + val out = captured( + assertEquals( + ContinueCli.runContinue( + ScanDirs(checkout, List(worktree)), + None, + list = true, + json = false, + tty = false + ), + ExitCodes.Ok + ) + ) + assert(out.contains("implementer @repo"), out) + assert(out.contains("implementer @ab12cd34"), out) + + test("runContinue --list: one directory, so no tree suffix on the row"): + val checkout = TempDirs.dir() + writeSessionManifest(checkout, "/repo") + val out = captured( + assertEquals( + ContinueCli.runContinue( + ScanDirs(checkout, Nil), + None, + list = true, + json = false, + tty = false + ), + ExitCodes.Ok + ) + ) + assert(out.contains("implementer"), out) + assert(!out.contains("@"), out) + + test("runContinue --list --json: each row carries its run's workDir"): + val checkout = TempDirs.dir() + writeSessionManifest(checkout, "/repo") + val out = captured( + assertEquals( + ContinueCli.runContinue( + ScanDirs(checkout, Nil), + None, + list = true, + json = true, + tty = false + ), + ExitCodes.Ok + ) + ) + assert(out.contains("\"workDir\":\"/repo\""), out) + test("runContinue --list --json: a crashed run reports crashed=true"): val dir = TempDirs.dir() writeCrashedManifest(dir) val out = captured( assertEquals( ContinueCli - .runContinue(dir, None, list = true, json = true, tty = false), + .runContinue( + ScanDirs(dir, Nil), + None, + list = true, + json = true, + tty = false + ), ExitCodes.Ok ) ) @@ -1057,7 +1261,13 @@ class CliTest extends munit.FunSuite: val out = captured( assertEquals( ContinueCli - .runContinue(dir, None, list = true, json = false, tty = false), + .runContinue( + ScanDirs(dir, Nil), + None, + list = true, + json = false, + tty = false + ), ExitCodes.Ok ) ) @@ -1107,7 +1317,13 @@ class CliTest extends munit.FunSuite: val (out, err) = capturedBoth( assertEquals( ContinueCli - .runContinue(dir, None, list = false, json = false, tty = true), + .runContinue( + ScanDirs(dir, Nil), + None, + list = false, + json = false, + tty = true + ), ExitCodes.ActionFailed ) ) diff --git a/shell/src/test/scala/orca/shell/resume/ResumeDetectorTest.scala b/shell/src/test/scala/orca/shell/resume/ResumeDetectorTest.scala index fc4ef48b0..a7760c29d 100644 --- a/shell/src/test/scala/orca/shell/resume/ResumeDetectorTest.scala +++ b/shell/src/test/scala/orca/shell/resume/ResumeDetectorTest.scala @@ -28,14 +28,14 @@ class ResumeDetectorTest extends munit.FunSuite: test("detect is None when the scan finds no progress log"): val workDir = TempDirs.dir() os.makeDir.all(workDir / ".orca") - assertEquals(ResumeDetector.detect(workDir), None) + assertEquals(ResumeDetector.detect(List(workDir)), None) test("detect finds a fresh log's recorded flow name and task text"): val workDir = TempDirs.dir() ProgressStore.default(workDir, "fix the flaky test").writeHeader(header()) assertEquals( - ResumeDetector.detect(workDir), - Some(InterruptedRun("implement.sc", "fix the flaky test")) + ResumeDetector.detect(List(workDir)), + Some(InterruptedRun("implement.sc", "fix the flaky test", workDir)) ) test("detect is None for a log missing flowName (a run outside the shell)"): @@ -43,7 +43,7 @@ class ResumeDetectorTest extends munit.FunSuite: ProgressStore .default(workDir, "fix the flaky test") .writeHeader(header(flowName = None)) - assertEquals(ResumeDetector.detect(workDir), None) + assertEquals(ResumeDetector.detect(List(workDir)), None) test( "detect drops a flowName that isn't a bare .sc filename (forged header)" @@ -54,21 +54,21 @@ class ResumeDetectorTest extends munit.FunSuite: ProgressStore .default(workDir, "fix the flaky test") .writeHeader(header(flowName = Some(forged))) - assertEquals(ResumeDetector.detect(workDir), None, forged) + assertEquals(ResumeDetector.detect(List(workDir)), None, forged) test("detect is None for an old-format log missing userPrompt"): val workDir = TempDirs.dir() ProgressStore .default(workDir, "fix the flaky test") .writeHeader(header(userPrompt = None)) - assertEquals(ResumeDetector.detect(workDir), None) + assertEquals(ResumeDetector.detect(List(workDir)), None) test("detect is None for a corrupt (unparseable) log, silently"): val workDir = TempDirs.dir() val store = ProgressStore.default(workDir, "fix the flaky test") store.writeHeader(header()) os.write.over(store.path, "not json {{{") - assertEquals(ResumeDetector.detect(workDir), None) + assertEquals(ResumeDetector.detect(List(workDir)), None) test("detect picks the newest of multiple unfinished logs by mtime"): val workDir = TempDirs.dir() @@ -83,6 +83,55 @@ class ResumeDetectorTest extends munit.FunSuite: // Force a distinguishable mtime order regardless of write-speed timing. val _ = os.mtime.set(older.path, System.currentTimeMillis() - 60000) assertEquals( - ResumeDetector.detect(workDir), - Some(InterruptedRun("b.sc", "newer prompt")) + ResumeDetector.detect(List(workDir)), + Some(InterruptedRun("b.sc", "newer prompt", workDir)) ) + + test("detect reports the directory the winning log was found in"): + val shellDir = TempDirs.dir() + val worktree = TempDirs.dir() + ProgressStore + .default(worktree, "fix the flaky test") + .writeHeader(header()) + assertEquals( + ResumeDetector.detect(List(shellDir, worktree)), + Some(InterruptedRun("implement.sc", "fix the flaky test", worktree)) + ) + + test("detect: the newest wins across directories, not within each"): + val shellDir = TempDirs.dir() + val worktree = TempDirs.dir() + val older = ProgressStore.default(worktree, "older prompt") + older.writeHeader( + header(userPrompt = Some("older prompt"), flowName = Some("a.sc")) + ) + ProgressStore + .default(shellDir, "newer prompt") + .writeHeader( + header(userPrompt = Some("newer prompt"), flowName = Some("b.sc")) + ) + val _ = os.mtime.set(older.path, System.currentTimeMillis() - 60000) + // The winner is in the FIRST directory here, the mirror of the case above. + assertEquals( + ResumeDetector.detect(List(shellDir, worktree)), + Some(InterruptedRun("b.sc", "newer prompt", shellDir)) + ) + + test("detect: an unreadable directory costs only its own logs"): + val shellDir = TempDirs.dir() + ProgressStore.default(shellDir, "fix the flaky test").writeHeader(header()) + // `.orca` present but not listable — the scan must still offer the log it + // can read rather than dropping the whole thing. + val unreadable = TempDirs.dir() + os.makeDir.all(unreadable / ".orca") + os.perms.set(unreadable / ".orca", "---------") + assume( + scala.util.Try(os.list(unreadable / ".orca")).isFailure, + "needs a user that file permissions apply to" + ) + try + assertEquals( + ResumeDetector.detect(List(shellDir, unreadable)), + Some(InterruptedRun("implement.sc", "fix the flaky test", shellDir)) + ) + finally os.perms.set(unreadable / ".orca", "rwxr-xr-x") diff --git a/shell/src/test/scala/orca/shell/run/FlowLauncherTest.scala b/shell/src/test/scala/orca/shell/run/FlowLauncherTest.scala index 54e9f9add..b19924d5f 100644 --- a/shell/src/test/scala/orca/shell/run/FlowLauncherTest.scala +++ b/shell/src/test/scala/orca/shell/run/FlowLauncherTest.scala @@ -5,12 +5,23 @@ class FlowLauncherTest extends munit.FunSuite: private val flow = os.root / "home" / "u" / "flow.sc" private val workspaceDir = os.root / "home" / "u" / ".cache" / "workspace" + /** Only the fields under test, defaulted off. Deliberately test-local: + * production `FlowFlags` stays without defaults so every real construction + * site keeps stating each flag. + */ + private def flags( + verbose: Boolean = false, + skipBranch: Boolean = false, + keepChanges: Boolean = false, + worktree: Boolean = false + ): FlowFlags = FlowFlags(verbose, skipBranch, keepChanges, worktree) + test("argv forces --dep with a release version, before --workspace/--"): val result = FlowLauncher.argv( flow, Some("0.0.18"), "do the thing", - FlowFlags(verbose = false, skipBranch = false, keepChanges = false), + flags(), workspaceDir ) assertEquals( @@ -35,7 +46,7 @@ class FlowLauncherTest extends munit.FunSuite: flow, None, "do the thing", - FlowFlags(verbose = false, skipBranch = false, keepChanges = false), + flags(), workspaceDir ) assertEquals( @@ -60,7 +71,7 @@ class FlowLauncherTest extends munit.FunSuite: flow, Some("0.0.18"), "do the thing", - FlowFlags(verbose = true, skipBranch = false, keepChanges = false), + flags(verbose = true), workspaceDir ) assertEquals( @@ -93,7 +104,7 @@ class FlowLauncherTest extends munit.FunSuite: flow, Some("0.0.18"), "do the thing", - FlowFlags(verbose = false, skipBranch = true, keepChanges = false), + flags(skipBranch = true), workspaceDir ) assertEquals( @@ -123,7 +134,7 @@ class FlowLauncherTest extends munit.FunSuite: flow, None, "do the thing", - FlowFlags(verbose = true, skipBranch = true, keepChanges = false), + flags(verbose = true, skipBranch = true), workspaceDir ) assertEquals( @@ -150,7 +161,7 @@ class FlowLauncherTest extends munit.FunSuite: flow, Some("0.0.18"), "do the thing", - FlowFlags(verbose = false, skipBranch = false, keepChanges = true), + flags(keepChanges = true), workspaceDir ) assertEquals( @@ -176,13 +187,44 @@ class FlowLauncherTest extends munit.FunSuite: ) test( - "argv adds every flow flag in a fixed order after --: verbose, skip-branch, keep-changes" + "argv adds --worktree (OrcaArgs's exact flag spelling) after -- when set" ): val result = FlowLauncher.argv( flow, None, "do the thing", - FlowFlags(verbose = true, skipBranch = true, keepChanges = true), + flags(worktree = true), + workspaceDir + ) + assertEquals( + result, + Seq( + "scala-cli", + "run", + flow.toString, + "--quiet", + "--verbose", + "--workspace", + workspaceDir.toString, + "--", + "do the thing", + "--worktree" + ) + ) + + test( + "argv adds every flow flag in a fixed order after --: verbose, skip-branch, keep-changes, worktree" + ): + val result = FlowLauncher.argv( + flow, + None, + "do the thing", + flags( + verbose = true, + skipBranch = true, + keepChanges = true, + worktree = true + ), workspaceDir ) assertEquals( @@ -199,7 +241,8 @@ class FlowLauncherTest extends munit.FunSuite: "do the thing", "--verbose", "--skip-branch", - "--keep-changes" + "--keep-changes", + "--worktree" ) ) @@ -209,7 +252,7 @@ class FlowLauncherTest extends munit.FunSuite: spacedFlow, None, "task", - FlowFlags(verbose = false, skipBranch = false, keepChanges = false), + flags(), workspaceDir ) assertEquals(result(2), spacedFlow.toString) @@ -223,7 +266,7 @@ class FlowLauncherTest extends munit.FunSuite: flow, None, " ", - FlowFlags(verbose = false, skipBranch = false, keepChanges = false), + flags(), workspaceDir ) ) diff --git a/shell/src/test/scala/orca/shell/sessions/ManifestReaderTest.scala b/shell/src/test/scala/orca/shell/sessions/ManifestReaderTest.scala index eaefdd007..da3518806 100644 --- a/shell/src/test/scala/orca/shell/sessions/ManifestReaderTest.scala +++ b/shell/src/test/scala/orca/shell/sessions/ManifestReaderTest.scala @@ -33,20 +33,20 @@ class ManifestReaderTest extends munit.FunSuite: test("list returns (Nil, Nil) for an absent runs dir, creating nothing"): val workDir = TempDirs.dir() - assertEquals(ManifestReader.list(workDir, alwaysDead), (Nil, Nil)) + assertEquals(ManifestReader.list(workDir, Nil, alwaysDead), (Nil, Nil)) assert(!os.exists(workDir / ".orca"), "reading must not create .orca") test("list returns (Nil, Nil) for an empty runs dir"): val workDir = TempDirs.dir() os.makeDir.all(runsDir(workDir)) - assertEquals(ManifestReader.list(workDir, alwaysDead), (Nil, Nil)) + assertEquals(ManifestReader.list(workDir, Nil, alwaysDead), (Nil, Nil)) test("list orders manifests newest-first by startedAt"): val workDir = TempDirs.dir() writeManifest(workDir, "a.json", startedAt = "2026-07-18T10:00:00Z") writeManifest(workDir, "b.json", startedAt = "2026-07-18T12:00:00Z") writeManifest(workDir, "c.json", startedAt = "2026-07-18T11:00:00Z") - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(warnings, Nil) assertEquals( runs.map(_.manifest.startedAt), @@ -77,7 +77,7 @@ class ManifestReaderTest extends munit.FunSuite: |}""".stripMargin, createFolders = true ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals( runs.map(_.manifest.startedAt), List(Instant.parse("2026-07-18T10:00:00Z")) @@ -98,7 +98,7 @@ class ManifestReaderTest extends munit.FunSuite: |}""".stripMargin, createFolders = true ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals( runs.map(_.manifest.startedAt), List(Instant.parse("2026-07-18T11:00:00Z")) @@ -127,7 +127,7 @@ class ManifestReaderTest extends munit.FunSuite: |}""".stripMargin, createFolders = true ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(runs, Nil) assertEquals(warnings.size, 1) assert( @@ -145,7 +145,7 @@ class ManifestReaderTest extends munit.FunSuite: pid = 999999, outcome = "running" ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(warnings, Nil) assertEquals(runs.map(_.crashed), List(true)) @@ -158,7 +158,7 @@ class ManifestReaderTest extends munit.FunSuite: pid = 1, outcome = "running" ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysAlive) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysAlive) assertEquals(warnings, Nil) assertEquals(runs.map(_.crashed), List(false)) @@ -173,7 +173,7 @@ class ManifestReaderTest extends munit.FunSuite: pid = 999999, outcome = "abandoned" ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(warnings, Nil) assertEquals(runs.map(_.crashed), List(false)) @@ -186,7 +186,7 @@ class ManifestReaderTest extends munit.FunSuite: pid = 999999, outcome = "succeeded" ) - val (runs, _) = ManifestReader.list(workDir, alwaysDead) + val (runs, _) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(runs.map(_.crashed), List(false)) test( @@ -194,7 +194,7 @@ class ManifestReaderTest extends munit.FunSuite: ): val workDir = TempDirs.dir() writeManifest(workDir, "badstart.json", startedAt = "not-a-timestamp") - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(runs, Nil) assertEquals(warnings.size, 1) assert( @@ -209,7 +209,7 @@ class ManifestReaderTest extends munit.FunSuite: "{ this is not json", createFolders = true ) - val (runs, warnings) = ManifestReader.list(workDir, alwaysDead) + val (runs, warnings) = ManifestReader.list(workDir, Nil, alwaysDead) assertEquals(runs, Nil) assertEquals(warnings.size, 1) assert( @@ -224,6 +224,80 @@ class ManifestReaderTest extends munit.FunSuite: os.makeDir.all(workDir / ".orca" / "cache") os.symlink(runsDir(workDir), outside) val ex = intercept[OrcaFlowException]( - ManifestReader.list(workDir, alwaysDead) + ManifestReader.list(workDir, Nil, alwaysDead) ) assert(ex.getMessage.contains("symlink"), ex.getMessage) + + test("list spans several worktrees, newest-first across all of them"): + val checkout = TempDirs.dir() + val worktree = TempDirs.dir() + writeManifest(checkout, "a.json", startedAt = "2026-07-18T10:00:00Z") + writeManifest(worktree, "b.json", startedAt = "2026-07-18T12:00:00Z") + writeManifest(checkout, "c.json", startedAt = "2026-07-18T11:00:00Z") + val (runs, warnings) = + ManifestReader.list(checkout, List(worktree), alwaysDead) + assertEquals(warnings, Nil) + assertEquals( + runs.map(_.manifest.startedAt.toString), + List( + "2026-07-18T12:00:00Z", + "2026-07-18T11:00:00Z", + "2026-07-18T10:00:00Z" + ) + ) + + test("list merges warnings from every directory, not just the first"): + val checkout = TempDirs.dir() + val worktree = TempDirs.dir() + os.write( + runsDir(checkout) / "broken-here.json", + "not json {{{", + createFolders = true + ) + os.write( + runsDir(worktree) / "broken-there.json", + "not json {{{", + createFolders = true + ) + writeManifest(worktree, "good.json", startedAt = "2026-07-18T12:00:00Z") + val (runs, warnings) = + ManifestReader.list(checkout, List(worktree), alwaysDead) + assertEquals(runs.size, 1) + assert(warnings.exists(_.contains("broken-here.json")), warnings.toString) + assert(warnings.exists(_.contains("broken-there.json")), warnings.toString) + + test("list: an unreadable worktree is one warning, not a lost listing"): + val checkout = TempDirs.dir() + val worktree = TempDirs.dir() + writeManifest(checkout, "a.json", startedAt = "2026-07-18T10:00:00Z") + // A tree left behind by a run under another uid, or one being removed in + // another terminal: it must not take the shell's own runs down with it. + os.makeDir.all(runsDir(worktree)) + os.perms.set(runsDir(worktree), "---------") + assume( + scala.util.Try(os.list(runsDir(worktree))).isFailure, + "needs a user that file permissions apply to" + ) + try + val (runs, warnings) = + ManifestReader.list(checkout, List(worktree), alwaysDead) + assertEquals(runs.size, 1) + assertEquals(warnings.size, 1) + assert(warnings.head.contains(worktree.toString), warnings.head) + finally os.perms.set(runsDir(worktree), "rwxr-xr-x") + + test("list: a symlinked .orca in another worktree warns, it does not abort"): + val checkout = TempDirs.dir() + val worktree = TempDirs.dir() + writeManifest(checkout, "a.json", startedAt = "2026-07-18T10:00:00Z") + val outside = TempDirs.dir() / "outside-runs" + os.makeDir.all(outside) + os.makeDir.all(worktree / ".orca" / "cache") + os.symlink(runsDir(worktree), outside) + // The hard abort stays for the caller's OWN directory (the case above); + // refusing to read someone else's tree is the whole remedy there. + val (runs, warnings) = + ManifestReader.list(checkout, List(worktree), alwaysDead) + assertEquals(runs.size, 1) + assertEquals(warnings.size, 1) + assert(warnings.head.contains("symlink"), warnings.head) diff --git a/shell/src/test/scala/orca/shell/sessions/ManifestRoundTripTest.scala b/shell/src/test/scala/orca/shell/sessions/ManifestRoundTripTest.scala index 35c1fdc9f..2764d5833 100644 --- a/shell/src/test/scala/orca/shell/sessions/ManifestRoundTripTest.scala +++ b/shell/src/test/scala/orca/shell/sessions/ManifestRoundTripTest.scala @@ -52,7 +52,8 @@ class ManifestRoundTripTest extends munit.FunSuite: ) writer.finish(RunOutcome.Succeeded) - val (runs, warnings) = ManifestReader.list(workDir, pidAlive = _ => true) + val (runs, warnings) = + ManifestReader.list(workDir, Nil, pidAlive = _ => true) assertEquals(warnings, Nil) assertEquals(runs.size, 1) assertEquals(runs.head.crashed, false) diff --git a/tools/src/main/scala/orca/OrcaDir.scala b/tools/src/main/scala/orca/OrcaDir.scala index 055163182..190f0d87e 100644 --- a/tools/src/main/scala/orca/OrcaDir.scala +++ b/tools/src/main/scala/orca/OrcaDir.scala @@ -7,6 +7,9 @@ import ox.tap * via its own `.gitignore` and carries a `CACHEDIR.TAG` so backup tools skip * it. All `.orca` creation routes through this object, so the exclusion * markers are always in place before anything is written into the cache. + * + * `worktrees/` is the one root entry that is not committed: it self-ignores + * like the cache but is not disposable like it — see [[ensureWorktrees]]. */ private[orca] object OrcaDir: private val gitignoreContents = "# Automatically created by orca.\n*\n" @@ -110,6 +113,27 @@ private[orca] object OrcaDir: */ def runsPath(workDir: os.Path): os.Path = cachePath(workDir) / "runs" + /** `/.orca/worktrees`, passively — a run's worktree is derived from + * this path before anything decides whether to create it. + */ + def worktreesPath(workDir: os.Path): os.Path = root(workDir) / "worktrees" + + /** Idempotently ensure `.orca/worktrees/` exists, writing its self-ignoring + * `.gitignore` before returning: without the marker, `git add -A` in this + * checkout stages a worktree under it as an embedded git repository, so the + * marker must be in place before the first worktree is created. + * + * Unlike [[ensureCache]] there is deliberately no `CACHEDIR.TAG` — the tag + * tells backup tools to skip the directory, and a worktree holds unmerged + * work plus the only copy of the progress log that resumes a failed run. + */ + def ensureWorktrees(workDir: os.Path): os.Path = + val worktrees = worktreesPath(workDir) + abortIfOrcaComponentSymlink(workDir, worktrees) + os.makeDir.all(worktrees) + writeIfAbsent(worktrees / ".gitignore", gitignoreContents) + worktrees + /** `/.orca/flows` — project-tier flow scripts (ADR 0021 §5), * committed like the rest of `.orca`'s root. */ diff --git a/tools/src/main/scala/orca/tools/Worktrees.scala b/tools/src/main/scala/orca/tools/Worktrees.scala new file mode 100644 index 000000000..2ed74cab0 --- /dev/null +++ b/tools/src/main/scala/orca/tools/Worktrees.scala @@ -0,0 +1,229 @@ +package orca.tools + +import orca.subprocess.QuietProc + +import scala.util.control.NonFatal + +/** Why [[Worktrees.add]] refused. `NoCommitsYet` is a case of its own because + * the caller reports it in orca's own words; for anything else git's message + * is the best available explanation. + */ +private[orca] enum WorktreeAddFailure: + case NoCommitsYet + case GitFailed(message: String) + +/** Why [[Worktrees.mainCheckoutOrReason]] could not name a main checkout. Kept + * apart because each wants a different thing said to the user, and only the + * command that just asked git knows which happened. + */ +private[orca] enum MainCheckoutFailure: + /** `cwd` is not inside a git working tree — or git could not be run at all. + */ + case NotARepository + + /** git named a main worktree that is not a checkout: it derives one by + * stripping `/.git` from the common dir, which under `--separate-git-dir` or + * in a submodule names the git directory itself. + */ + case MainWorktreeNotACheckout + + /** git did not answer the query as asked — one older than 2.31 echoes the + * unknown `--path-format` and exits 0. + */ + case Unsupported + +/** Why [[Worktrees.startBranch]] refused. */ +private[orca] enum StartBranchFailure: + /** The branch already carries commits the worktree's HEAD cannot reach, so + * moving it there would strand them. + */ + case WouldLoseCommits(branch: String) + case GitFailed(message: String) + +/** Git worktree plumbing for run-level isolation (`--worktree`). + * + * Deliberately not on [[GitTool]]: that trait is flow-facing, and a flow's + * working directory is fixed before its body runs, so no flow can use these. + * They run during startup, before any tool exists. + * + * The two reads answer "not known" (`None` / empty) rather than failing, so a + * caller outside a git repository — or one scanning on every menu redraw — has + * nothing to handle. [[add]] is a write, so its failure is returned. + */ +private[orca] object Worktrees: + + private val WorktreeLine = "worktree (.*)".r + + /** The main checkout of the repository containing `cwd`, identical whether + * `cwd` is that checkout or any linked worktree of it. `None` when `cwd` is + * not inside a git working tree, when the repository's main worktree is not + * a checkout (`--separate-git-dir`, a submodule), and when git is too old to + * answer — [[mainCheckoutOrReason]] tells the three apart, for the caller + * that has to explain the failure. + * + * The checkout is read as `--show-toplevel`, not as the git directory's + * parent: the two differ in a submodule (git dir under the superproject's + * `.git/modules/`) and in a repository created with `--separate-git-dir`, + * where deriving the parent would put orca's worktrees inside someone's + * `.git`. Comparing `--git-dir` with `--git-common-dir` is what tells a + * linked worktree from the main one; only there is git's own worktree list + * consulted, which derives the main worktree the same parent-assuming way. + */ + def mainCheckout(cwd: os.Path): Option[os.Path] = + mainCheckoutOrReason(cwd).toOption + + def mainCheckoutOrReason( + cwd: os.Path + ): Either[MainCheckoutFailure, os.Path] = + mainCheckoutOrReason(cwd, list(cwd)) + + /** [[mainCheckout]] with the reason it could not answer. The three failures + * want three different sentences, and only this command knows which one + * happened — a caller left to re-derive it from a second question gets some + * of them wrong. + */ + def mainCheckoutOrReason( + cwd: os.Path, + worktrees: => List[os.Path] + ): Either[MainCheckoutFailure, os.Path] = + val query = + Seq("rev-parse", "--path-format=absolute") ++ + Seq("--git-dir", "--git-common-dir", "--show-toplevel") + probe(cwd, query*).map(_.linesIterator.toList) match + case None => Left(MainCheckoutFailure.NotARepository) + case Some(List(gitDir, commonDir, topLevel)) => + if gitDir == commonDir then + absolute(topLevel).toRight(MainCheckoutFailure.Unsupported) + // Creating worktrees under a git directory would write a checkout + // inside the repository's own metadata store, so a candidate that is + // not itself a checkout is refused rather than written to. + else + worktrees.headOption + .filter(p => os.exists(p / ".git")) + .toRight(MainCheckoutFailure.MainWorktreeNotACheckout) + case Some(_) => Left(MainCheckoutFailure.Unsupported) + + /** Paths of every worktree of the repository containing `cwd`, the main + * checkout first; empty when `cwd` is not in a git repository. A path may + * not exist on disk — git keeps the administrative entry of a worktree whose + * directory was deleted, and reports it as prunable. + */ + def list(cwd: os.Path): List[os.Path] = + probe(cwd, "worktree", "list", "--porcelain").toList + .flatMap(_.linesIterator) + .flatMap: + case WorktreeLine(path) => absolute(path) + case _ => None + + /** Create a worktree at `path`, detached at `cwd`'s current HEAD. + * + * Detached, then [[startBranch]] — not one `git worktree add -B`: git + * refuses to force-update a branch any worktree entry claims, stale or live + * ("cannot force update the branch ... used by worktree at ..."), so the + * combined form cannot recreate a worktree whose directory was deleted while + * its entry and branch survived — the normal `git clean -xdff` case. + * + * No commit-ish is passed: git then detaches at the invoking checkout's + * HEAD, which is the wanted start point. No `--` separator either: `path` is + * absolute, so git cannot read it as an option. + * + * `--force` reclaims a path whose administrative entry outlived its + * directory, and only that: it still refuses a path that exists, and the + * other safeguard it lifts, a branch already checked out elsewhere, cannot + * apply without a commit-ish. Path-scoped, unlike `git worktree prune`, + * which would drop every stale entry in the repository, including worktrees + * orca never made. + * + * Callers establish the repository with [[mainCheckout]] first, so a HEAD + * that does not resolve here means a repository without commits rather than + * no repository — asked as its own question, so neither side has to + * recognise git's wording for it. + */ + def add(cwd: os.Path, path: os.Path): Either[WorktreeAddFailure, Unit] = + if probe(cwd, "rev-parse", "--verify", "--quiet", "HEAD").isEmpty then + Left(WorktreeAddFailure.NoCommitsYet) + else + val result = + git(cwd, "worktree", "add", "--force", "--detach", path.toString) + if result.exitCode == 0 then Right(()) + else + val stderr = result.err.text().trim + Left( + WorktreeAddFailure.GitFailed( + if stderr.nonEmpty then stderr + else s"git worktree add exited ${result.exitCode}" + ) + ) + + /** Put `worktree` on `name`, at its current HEAD (`checkout -B`). Used on a + * freshly [[add]]ed worktree, which git leaves detached — a state whose + * current branch reads back as the literal "HEAD", which a run records as + * its starting branch and resume then refuses as an unsafe ref — and on a + * reused worktree found detached, which is that same state arrived at later. + * + * `-B` would reset an existing `name`, so a branch already carrying commits + * this HEAD cannot reach is refused instead: every removal route for a + * worktree (`git clean -xdff`, `rm -rf`, `git worktree remove`) leaves its + * branch behind, so a re-run of the same task finds it, possibly with work + * on it. A probe that cannot answer counts as "would lose", so an unreadable + * repository refuses rather than resets. + */ + def startBranch( + worktree: os.Path, + name: String + ): Either[StartBranchFailure, Unit] = + if wouldLoseCommits(worktree, name) then + Left(StartBranchFailure.WouldLoseCommits(name)) + else + val result = git(worktree, "checkout", "-B", name) + if result.exitCode == 0 then Right(()) + else Left(StartBranchFailure.GitFailed(result.err.text().trim)) + + /** The branch `worktree`'s HEAD is on, `Some("HEAD")` when it is detached, + * and `None` when the question could not be answered at all — an unstartable + * git, or a worktree directory that has gone away. + * + * Three-valued on purpose: callers repair a detached worktree, and reading + * "could not answer" as "on a branch" would skip the repair for a tree whose + * state is unknown. Same rule [[startBranch]] states for its own probe. + */ + def headBranch(worktree: os.Path): Option[String] = + probe(worktree, "rev-parse", "--abbrev-ref", "HEAD").map(_.trim) + + /** Whether HEAD is DEFINITELY on a branch — `false` for a detached checkout + * and for a worktree that cannot be read, so a caller acting on it fails + * closed. + */ + def onABranch(worktree: os.Path): Boolean = + headBranch(worktree).exists(_ != "HEAD") + + private def wouldLoseCommits(cwd: os.Path, branch: String): Boolean = + val exists = + probe(cwd, "rev-parse", "--verify", "--quiet", branch).isDefined + exists && git( + cwd, + "merge-base", + "--is-ancestor", + branch, + "HEAD" + ).exitCode != 0 + + /** Git prints absolute paths for both reads above; anything else means the + * command did not do what was asked, which is "not known" rather than a + * failure to raise at a caller that has nowhere to put one — `os.Path` + * throws on a relative or empty string. + */ + private def absolute(path: String): Option[os.Path] = + Option.when(path.startsWith("/"))(os.Path(path)) + + private def git(cwd: os.Path, args: String*): os.CommandResult = + QuietProc.call("git" +: args, cwd = cwd, env = OsGitTool.nonInteractiveEnv) + + /** Stdout of a read-only git command that exited 0; `None` on anything else, + * including a subprocess that could not be started. + */ + private def probe(cwd: os.Path, args: String*): Option[String] = + try + val result = git(cwd, args*) + if result.exitCode == 0 then Some(result.out.text()) else None + catch case NonFatal(_) => None diff --git a/tools/src/test/scala/orca/OrcaDirTest.scala b/tools/src/test/scala/orca/OrcaDirTest.scala index 97c0bb942..46de79008 100644 --- a/tools/src/test/scala/orca/OrcaDirTest.scala +++ b/tools/src/test/scala/orca/OrcaDirTest.scala @@ -37,6 +37,30 @@ class OrcaDirTest extends munit.FunSuite: assert(os.isDir(root)) assert(!os.exists(root / "cache")) + test("worktreesPath points at .orca/worktrees without creating it"): + val wd = TempDirs.dir() + assertEquals(OrcaDir.worktreesPath(wd), wd / ".orca" / "worktrees") + assert(!os.exists(wd / ".orca")) + + test("ensureWorktrees writes the .gitignore but no CACHEDIR.TAG"): + val wd = TempDirs.dir() + val worktrees = OrcaDir.ensureWorktrees(wd) + assertEquals(worktrees, wd / ".orca" / "worktrees") + assertEquals( + os.read(worktrees / ".gitignore"), + "# Automatically created by orca.\n*\n" + ) + assert(!os.exists(worktrees / "CACHEDIR.TAG")) + + test("ensureWorktrees aborts on a symlinked .orca/worktrees"): + val wd = TempDirs.dir() + val outside = TempDirs.dir() / "outside-worktrees" + os.makeDir.all(outside) + os.makeDir.all(OrcaDir.rootPath(wd)) + os.symlink(OrcaDir.worktreesPath(wd), outside) + intercept[OrcaFlowException](OrcaDir.ensureWorktrees(wd)).discard + assert(os.list(outside).isEmpty, "no write must go through the symlink") + test("flowsPath points at .orca/flows without creating it"): val wd = TempDirs.dir() assertEquals(OrcaDir.flowsPath(wd), wd / ".orca" / "flows") diff --git a/tools/src/test/scala/orca/tools/WorktreesTest.scala b/tools/src/test/scala/orca/tools/WorktreesTest.scala new file mode 100644 index 000000000..bb7029e2f --- /dev/null +++ b/tools/src/test/scala/orca/tools/WorktreesTest.scala @@ -0,0 +1,148 @@ +package orca.tools + +import orca.OrcaDir +import orca.testkit.{GitRepo, TempDirs} +import ox.discard + +class WorktreesTest extends munit.FunSuite: + + test("add creates a worktree that list then reports"): + val repo = GitRepo.seeded() + val path = repo / "wt" + assertEquals(Worktrees.add(repo, path), Right(())) + assertEquals(Worktrees.list(repo), List(repo, path)) + + test("add starts the worktree detached at the invoking checkout's HEAD"): + val repo = GitRepo.seeded() + git(repo, "checkout", "-q", "-b", "feature").discard + os.write(repo / "feature.txt", "feature") + git(repo, "add", "-A").discard + git(repo, "commit", "-q", "-m", "feature").discard + val path = repo / "wt" + assertEquals(Worktrees.add(repo, path), Right(())) + // The commit only 'feature' has: a worktree started from the default + // branch, or from anything but cwd's HEAD, would not have it. + assertEquals(head(path), head(repo)) + assertEquals(gitOut(path, "rev-parse", "--abbrev-ref", "HEAD"), "HEAD") + + test("the .orca/worktrees marker keeps a worktree out of git add -A"): + val repo = GitRepo.seeded() + val path = OrcaDir.ensureWorktrees(repo) / "run" + assertEquals(Worktrees.add(repo, path), Right(())) + git(repo, "add", "-A").discard + assertEquals(gitOut(repo, "status", "--porcelain"), "") + + test("mainCheckout answers the same path from the repo and its worktree"): + val repo = GitRepo.seeded() + val path = repo / "wt" + assertEquals(Worktrees.add(repo, path), Right(())) + assertEquals(Worktrees.mainCheckout(repo), Some(repo)) + assertEquals(Worktrees.mainCheckout(path), Some(repo)) + + test("mainCheckout answers the checkout when the git dir lives outside it"): + val root = TempDirs.dir() + val checkout = root / "checkout" + os.makeDir.all(checkout) + git( + checkout, + "init", + "-q", + "-b", + "main", + s"--separate-git-dir=${root / "gitdir"}" + ).discard + assertEquals(Worktrees.mainCheckout(checkout), Some(checkout)) + + test("mainCheckout is empty outside a git repository"): + assertEquals(Worktrees.mainCheckout(TempDirs.dir()), None) + + test("add reports an unborn HEAD as its own failure"): + val repo = GitRepo.empty() + assertEquals( + Worktrees.add(repo, repo / "wt"), + Left(WorktreeAddFailure.NoCommitsYet) + ) + + test("add carries git's message for every other failure"): + val repo = GitRepo.seeded() + val path = repo / "wt" + assertEquals(Worktrees.add(repo, path), Right(())) + Worktrees.add(repo, path) match + case Left(WorktreeAddFailure.GitFailed(message)) => + assert(message.nonEmpty, "git's explanation must survive") + case other => fail(s"expected a git failure, got $other") + + private def git(cwd: os.Path, args: String*): os.CommandResult = + os.proc("git" +: args).call(cwd = cwd) + + private def gitOut(cwd: os.Path, args: String*): String = + git(cwd, args*).out.text().trim + + private def head(cwd: os.Path): String = gitOut(cwd, "rev-parse", "HEAD") + + test("startBranch refuses to move a branch holding commits HEAD lacks"): + val repo = GitRepo.seeded() + val path = repo / "wt" + assertEquals(Worktrees.add(repo, path), Right(())) + assertEquals(Worktrees.startBranch(path, "wt-branch"), Right(())) + os.write(path / "work.txt", "work") + git(path, "add", "-A").discard + git(path, "commit", "-q", "-m", "work in the worktree").discard + // Every removal route leaves the branch behind, so a re-run finds it. + git(repo, "worktree", "remove", "--force", path.toString).discard + assertEquals(Worktrees.add(repo, path), Right(())) + assertEquals( + Worktrees.startBranch(path, "wt-branch"), + Left(StartBranchFailure.WouldLoseCommits("wt-branch")) + ) + // The commit is still reachable from the branch orca refused to move. + assert( + gitOut(repo, "log", "-1", "--format=%s", "wt-branch") + .contains("work in the worktree") + ) + + test("startBranch takes a freshly added worktree off its detached HEAD"): + val repo = GitRepo.seeded() + val path = repo / "wt" + assertEquals(Worktrees.add(repo, path), Right(())) + assertEquals(Worktrees.headBranch(path), Some("HEAD")) + assertEquals(Worktrees.startBranch(path, "wt-branch"), Right(())) + assertEquals(Worktrees.headBranch(path), Some("wt-branch")) + assertEquals(gitOut(path, "rev-parse", "--abbrev-ref", "HEAD"), "wt-branch") + + test("mainCheckout never answers the git dir, even from a linked worktree"): + val root = TempDirs.dir() + val checkout = root / "checkout" + os.makeDir.all(checkout) + git( + checkout, + "init", + "-q", + "-b", + "main", + s"--separate-git-dir=${root / "gitdir"}" + ).discard + git(checkout, "config", "user.email", "t@e.com").discard + git(checkout, "config", "user.name", "T").discard + os.write(checkout / "seed.txt", "seed") + git(checkout, "add", "-A").discard + git(checkout, "commit", "-q", "-m", "seed").discard + val worktree = root / "wt" + assertEquals(Worktrees.add(checkout, worktree), Right(())) + // git names the git dir as the main worktree here, so there is no checkout + // to answer with: refusing beats writing a worktree inside `.git`. + assertEquals(Worktrees.mainCheckout(worktree), None) + // And the reason is the layout, not "you have no repository" — the caller + // words those two very differently. + assertEquals( + Worktrees.mainCheckoutOrReason(worktree), + Left(MainCheckoutFailure.MainWorktreeNotACheckout) + ) + + test("mainCheckoutOrReason separates a non-repository from a bad layout"): + assertEquals( + Worktrees.mainCheckoutOrReason(TempDirs.dir()), + Left(MainCheckoutFailure.NotARepository) + ) + val repo = GitRepo.seeded() + assertEquals(Worktrees.mainCheckoutOrReason(repo), Right(repo))