Widen the early issue-fetch gate for worktree mode #323

Closed
opened 2026-11-05 21:43:23 +00:00 by david · 1 comment
Owner

Summary

Make any run that will create a worktree — a bare repo, or a normal clone with worktree.enabled: true — fetch the issue early so the worktree folder can be named with the issue summary. Reuses the existing early-fetch seam; no second fetch path is added.

Background

src/cli/services/runCli.ts contains the private resolveWorkDirForIssue({ originalCwd, runGit, issueRef, config, originHost, resolveWorkDirFn?, fetchIssueFn?, stdout, stderr }). Current flow:

  1. If a mock resolveWorkDirFn is injected (tests), it is called directly with a placeholder summary and { cwd, fetchedContext: undefined } is returned.
  2. isInsideWorktree fast path → return originalCwd, no fetch.
  3. isBareRepository; if (!isBare) → return originalCwd, no early fetch (this is the gate to widen).
  4. Bare → fetch the issue via fetchIssueFn ?? fetchIssue (which dispatches on the provider), wrap any failure in GitWorktreeError (failed to fetch issue <key>: <message>), then call resolveWorkDir with the fetched summary; return { cwd, fetchedContext }.

The caller runSingleIssueMode catches GitWorktreeError → stderr + exit 1, no error.json, and when fetchedContext is set it logs issue context fetched early (bare repo worktree mode) (via setupLogger.info) and the pipeline skips the issue-fetch stage.

Why the early fetch exists (ADR-022 §8): the worktree folder name is buildBranchFolderName(issueKey, summary, provider) (e.g. issue-42-add-login-page) for both repo kinds, and the name needs the issue summary, which only the issue API provides. So any run that will create a worktree must fetch the issue first.

Behavior change to implement and test: for a normal clone with worktree mode enabled, an issue-API error now aborts the run before any stage, wrapped as GitWorktreeError (stderr, exit 1, no error.json). Today a normal clone reaches the issue-fetch stage, which has a bounded-retry policy. This is a deliberate, documented change (called out in the CHANGELOG by a later docs step in this series).

Batch mode reality check: as of this writing, runBatch/runScanMode (src/batch/services/) process each issue in the launch cwd via makeBatchGitSeam and never call resolveWorkDirForIssue — batch mode has no per-issue worktree resolution. The plan this step derives from assumed batch would inherit the behavior for free; verify rather than assume (see step 6).

Depends on: #320 (config schema), #321 (parse-time dir validation), #322 (worktree routing) — the value this step threads is parsed and validated by the first two, and consumed by the third.

Implementation Details

Files: src/cli/services/runCli.ts, src/cli/services/runCli.test.ts.

  1. Derive the effective worktree dir from the parsed config-file result (ConfigFileResult, loaded at startup in runCli as the configFile variable; note --help/--list-models skip loading, so it can be undefined):

    const worktreeDir = (configFile?.worktree?.enabled ?? true) === true
      ? configFile?.worktree?.dir
      : undefined;
    

    (enabled — absent or explicit true — passes dir through; disabled passes undefined; absent section passes undefined.) Thread a worktreeDir: string | undefined field through runSingleIssueMode's input and into resolveWorkDirForIssue (mirror how env is passed).

  2. Add worktreeDir to resolveWorkDirForIssue's input, to the resolveWorkDirFn callable type, and pass it through to the resolveWorkDir call.

  3. Widen the gate — replace

    if (!isBare) {
      return { cwd: originalCwd, fetchedContext: undefined };
    }
    

    with:

    if (!isBare && worktreeDir === undefined) {
      return { cwd: originalCwd, fetchedContext: undefined };
    }
    
  4. Keep the existing GitWorktreeError wrapping and the fetchedContext return untouched, so the issue-fetch stage is skipped exactly as it is for bare repos.

  5. Change the log line 'issue context fetched early (bare repo worktree mode)' → 'issue context fetched early (worktree mode)'.

  6. Batch mode: confirm what runBatch/runScanMode do per issue. If they still route per-issue work through the launch cwd (not through resolveWorkDirForIssue), add a regression test asserting the batch path is unaffected by worktree config (runs in the launch repo, no early fetch) and say so in the PR description — do not claim inheritance. If the batch path has since changed to route per-issue worktree resolution through resolveWorkDirForIssue, confirm it inherits the widened gate and test that instead.

  7. Update the injectable resolveWorkDirFn / fetchIssueFn type declarations (both the top-level CLI deps and the runSingleIssueMode input) and the test doubles in runCli.test.ts.

Acceptance Criteria

  • Bare repo: unchanged — one early fetch, worktree resolved, issue-fetch stage skipped via fetchedContext.
  • Normal clone + worktree.enabled: true (with a valid dir): early fetch happens, fetchedContext is returned, the worktree cwd is used.
  • Normal clone + section absent or enabled: false: no early fetch, no behavior change.
  • Early-fetch failure in worktree mode surfaces as GitWorktreeError (stderr message, exit 1, no error.json) — asserted.
  • --help and --list-models bypass config loading, so a broken worktree.dir never blocks them.
  • Batch-mode behavior is covered per step 6 (regression assertion, or inheritance test if the path changed).
  • npm run lint and npm test pass.

Test Plan

Extend runCli.test.ts (existing resolveWorkDirFn/fetchIssueFn doubles): the three repo-kind/gate cases above, the GitWorktreeError surfacing case, and the batch-mode assertion. Run npm test.

## Summary Make **any** run that will create a worktree — a bare repo, or a normal clone with `worktree.enabled: true` — fetch the issue early so the worktree folder can be named with the issue summary. Reuses the existing early-fetch seam; no second fetch path is added. ## Background `src/cli/services/runCli.ts` contains the private `resolveWorkDirForIssue({ originalCwd, runGit, issueRef, config, originHost, resolveWorkDirFn?, fetchIssueFn?, stdout, stderr })`. Current flow: 1. If a mock `resolveWorkDirFn` is injected (tests), it is called directly with a placeholder summary and `{ cwd, fetchedContext: undefined }` is returned. 2. `isInsideWorktree` fast path → return `originalCwd`, no fetch. 3. `isBareRepository`; `if (!isBare)` → return `originalCwd`, **no early fetch** (this is the gate to widen). 4. Bare → fetch the issue via `fetchIssueFn ?? fetchIssue` (which dispatches on the provider), wrap any failure in `GitWorktreeError` (`failed to fetch issue <key>: <message>`), then call `resolveWorkDir` with the fetched summary; return `{ cwd, fetchedContext }`. The caller `runSingleIssueMode` catches `GitWorktreeError` → stderr + exit 1, **no `error.json`**, and when `fetchedContext` is set it logs `issue context fetched early (bare repo worktree mode)` (via `setupLogger.info`) and the pipeline skips the issue-fetch stage. Why the early fetch exists (ADR-022 §8): the worktree folder name is `buildBranchFolderName(issueKey, summary, provider)` (e.g. `issue-42-add-login-page`) for **both** repo kinds, and the name needs the issue **summary**, which only the issue API provides. So any run that will create a worktree must fetch the issue first. **Behavior change to implement and test:** for a normal clone with worktree mode enabled, an issue-API error now aborts the run **before any stage**, wrapped as `GitWorktreeError` (stderr, exit 1, **no `error.json`**). Today a normal clone reaches the issue-fetch stage, which has a bounded-retry policy. This is a deliberate, documented change (called out in the CHANGELOG by a later docs step in this series). **Batch mode reality check:** as of this writing, `runBatch`/`runScanMode` (`src/batch/services/`) process each issue in the launch `cwd` via `makeBatchGitSeam` and **never call** `resolveWorkDirForIssue` — batch mode has no per-issue worktree resolution. The plan this step derives from assumed batch would inherit the behavior for free; verify rather than assume (see step 6). **Depends on:** #320 (config schema), #321 (parse-time `dir` validation), #322 (worktree routing) — the value this step threads is parsed and validated by the first two, and consumed by the third. ## Implementation Details Files: `src/cli/services/runCli.ts`, `src/cli/services/runCli.test.ts`. 1. Derive the effective worktree dir from the parsed config-file result (`ConfigFileResult`, loaded at startup in `runCli` as the `configFile` variable; note `--help`/`--list-models` skip loading, so it can be `undefined`): ```ts const worktreeDir = (configFile?.worktree?.enabled ?? true) === true ? configFile?.worktree?.dir : undefined; ``` (enabled — absent or explicit `true` — passes `dir` through; disabled passes `undefined`; absent section passes `undefined`.) Thread a `worktreeDir: string | undefined` field through `runSingleIssueMode`'s input and into `resolveWorkDirForIssue` (mirror how `env` is passed). 2. Add `worktreeDir` to `resolveWorkDirForIssue`'s input, to the `resolveWorkDirFn` callable type, and pass it through to the `resolveWorkDir` call. 3. Widen the gate — replace ```ts if (!isBare) { return { cwd: originalCwd, fetchedContext: undefined }; } ``` with: ```ts if (!isBare && worktreeDir === undefined) { return { cwd: originalCwd, fetchedContext: undefined }; } ``` 4. Keep the existing `GitWorktreeError` wrapping and the `fetchedContext` return untouched, so the issue-fetch stage is skipped exactly as it is for bare repos. 5. Change the log line `'issue context fetched early (bare repo worktree mode)'` → `'issue context fetched early (worktree mode)'`. 6. **Batch mode:** confirm what `runBatch`/`runScanMode` do per issue. If they still route per-issue work through the launch `cwd` (not through `resolveWorkDirForIssue`), add a regression test asserting the batch path is unaffected by worktree config (runs in the launch repo, no early fetch) and say so in the PR description — do **not** claim inheritance. If the batch path has since changed to route per-issue worktree resolution through `resolveWorkDirForIssue`, confirm it inherits the widened gate and test that instead. 7. Update the injectable `resolveWorkDirFn` / `fetchIssueFn` type declarations (both the top-level CLI deps and the `runSingleIssueMode` input) and the test doubles in `runCli.test.ts`. ## Acceptance Criteria - [ ] Bare repo: unchanged — one early fetch, worktree resolved, issue-fetch stage skipped via `fetchedContext`. - [ ] Normal clone + `worktree.enabled: true` (with a valid `dir`): early fetch happens, `fetchedContext` is returned, the worktree cwd is used. - [ ] Normal clone + section absent or `enabled: false`: no early fetch, no behavior change. - [ ] Early-fetch failure in worktree mode surfaces as `GitWorktreeError` (stderr message, exit 1, **no `error.json`**) — asserted. - [ ] `--help` and `--list-models` bypass config loading, so a broken `worktree.dir` never blocks them. - [ ] Batch-mode behavior is covered per step 6 (regression assertion, or inheritance test if the path changed). - [ ] `npm run lint` and `npm test` pass. ## Test Plan Extend `runCli.test.ts` (existing `resolveWorkDirFn`/`fetchIssueFn` doubles): the three repo-kind/gate cases above, the `GitWorktreeError` surfacing case, and the batch-mode assertion. Run `npm test`.
david closed this issue 2026-11-06 03:33:13 +00:00
Author
Owner

pi-loop opened and merged a pull request for this issue: #329

pi-loop opened and merged a pull request for this issue: https://git.excelera.net/david/pi-loop/pulls/329
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
2025-12-31
Dependencies

No dependencies set

Reference
david/pi-loop#323
No description provided.