diff --git a/docs/memory/wt-cli/create-output-phases.md b/docs/memory/wt-cli/create-output-phases.md index 9910c80..e6059d6 100644 --- a/docs/memory/wt-cli/create-output-phases.md +++ b/docs/memory/wt-cli/create-output-phases.md @@ -52,13 +52,13 @@ This file documents the phase-separator output contract that `wt create` and `wt - `wt create`'s stdout remains **solely** the final worktree-path line (`fmt.Println(wtPath)`), byte-identical to before this change, **on the success path**. This preserves the launcher-contract guarantee that stdout is the machine-readable result. No separator, summary, banner, prompt, or init output leaks to stdout. - **On any init-failure path the path line is NOT printed** (`260626-n6ma`). The end-of-function guard `if initFailed { os.Exit(wt.ExitInitFailed) }` (`create.go:476–478`) sits **before** `fmt.Println(wtPath)` (`create.go:482`), so a non-zero init exit — including after a successful interactive open-anyway open — exits 7 with **no** stdout path line. This is the correct stdout discipline: stdout is the success result, and an init failure is not a success. Operators detect the kept worktree via exit code 7 (and the stderr banner's absolute `Worktree:` path / `Go:` hint), not via a stdout line. The init-failure banner and the open-anyway prompt are stderr writes, consistent with stdout=machine-result / stderr=human. - `wt init` was **realigned** so ALL its init diagnostics go to stderr: the `Running worktree init...` / `Worktree init complete.` banner (now `fmt.Fprintln(os.Stderr, ...)`), the Init separator, AND the init script's own stdout (the init child is wired with `cmd.Stdout = os.Stderr` at `init.go:83`, joining `cmd.Stderr = os.Stderr`). `wt init` has **no machine-readable stdout result** — it is a side-effecting command whose outcome is its exit code — so all of its output is diagnostic. -- The realignment changed only the stream (stdout → stderr) for `wt init`: exit-code behavior (`ExitInitFailed = 7` on script non-zero exit), graceful-skip behavior, and all output text are unchanged. Existing `wt init` tests assert on the combined `r.Stdout + r.Stderr`, so they stay green. +- The realignment changed only the stream (stdout → stderr) for `wt init`: exit-code behavior, graceful-skip behavior, and all output text were unchanged by *this* change. `wt init` exits `ExitInitFailed = 7` on a script non-zero exit, with one carve-out added later by `260705-irnt`: the **built-in default** `fab sync` exiting fab-kit's `ExitNotManaged = 3` in a non-fab repo is classified (`DefaultNotApplicable`) as "default does not apply" — `wt init` writes the skip warning and exits 0 instead of 7 (an explicit `WORKTREE_INIT_SCRIPT` value, or any other exit code, still exits 7; see `init-failure-contract.md`). Existing `wt init` tests assert on the combined `r.Stdout + r.Stderr`, so they stay green. ### `wt.Warn` is the one-line warning helper; warnings are stderr-only (`260622-log5`) -- `src/internal/worktree/errors.go` exposes `func Warn(format string, args ...any)` alongside the other output helpers (`WtError`, `PrintError`, `PhaseSeparator`, `PrintInitFailureBanner`). It is the **shared one-line** `Warning:`-diagnostic helper used by `create`/`delete` — it writes `"%sWarning:%s %s\n"` (`ColorYellow` prefix, `ColorReset`, then the formatted message) to **stderr**. (Exception: the verbose init not-found warning, `InitNotFound.RenderWarning` in `internal/worktree/init.go`, builds its own multi-line `Warning:` text and does **not** route through `Warn` — so `Warn` is not the CLI's sole `Warning:` producer.) +- `src/internal/worktree/errors.go` exposes `func Warn(format string, args ...any)` alongside the other output helpers (`WtError`, `PrintError`, `PhaseSeparator`, `PrintInitFailureBanner`). It is the **shared one-line** `Warning:`-diagnostic helper used by `create`/`delete` — it writes `"%sWarning:%s %s\n"` (`ColorYellow` prefix, `ColorReset`, then the formatted message) to **stderr**. (Exceptions: the two multi-line `Warning:` renderers in `internal/worktree/init.go` — `InitNotFound.RenderWarning` (verbose not-found copy) and `RenderDefaultSkipWarning` (the `260705-irnt` default-not-applicable skip copy) — build their own multi-line text and do **not** route through `Warn`, so `Warn` is not the CLI's sole `Warning:` producer.) - It is the warnings analogue of `PhaseSeparator`: color detection reuses the **blanked package-level color vars** (the `NO_COLOR`-blanking `init()` zeroes `ColorYellow`/`ColorReset`), so under `NO_COLOR` it emits a plain `Warning: \n` with **no** ANSI escapes. It MUST NOT call `os.Getenv` afresh. -- **All one-line `Warning:` call sites route through `Warn`** — `create.go` (the uncommitted-changes warning and the open-phase "could not open in …" / reused-worktree-init warnings) and `delete.go` (the two `failed to remove …` warnings and the two pre-menu warnings). (The init not-found renderer noted above is the lone multi-line exception that stays out of band.) Call sites pass the **message text only**, never the literal `Warning: ` prefix — the helper owns the prefix, so re-including it would double it. +- **All one-line `Warning:` call sites route through `Warn`** — `create.go` (the uncommitted-changes warning and the open-phase "could not open in …" / reused-worktree-init warnings) and `delete.go` (the two `failed to remove …` warnings and the two pre-menu warnings). The two multi-line renderers noted above (`InitNotFound.RenderWarning` and `RenderDefaultSkipWarning`, both in `init.go`) stay out of band — `Warn` owns the *one-line* `Warning:` idiom, not the multi-line ones. Call sites pass the **message text only**, never the literal `Warning: ` prefix — the helper owns the prefix, so re-including it would double it. - **`wt delete`'s two pre-menu warnings were realigned stdout → stderr.** The uncommitted-changes warning (`handleUncommittedChanges`) and the unpushed-commits warning (`handleUnpushedCommits`) previously printed via `fmt.Printf` (**stdout**); they now route through `wt.Warn` (→ **stderr**), matching the sibling warnings already on stderr in the same file. This is the warnings counterpart of the `wt init` stdout→stderr realignment above — same stream-discipline fix, different command. `wt delete` has no stdout machine contract, but warnings are diagnostics and belong on stderr. - **Menu-layout spacing is preserved at the call site, not in the helper.** `Warn` appends exactly one trailing `\n`. The two pre-menu warnings need a blank line before and after for menu spacing, so each call site frames the `wt.Warn(...)` with a bare `fmt.Fprintln(os.Stderr)` immediately before and after (reproducing the old `\n…\n\n` framing now that the whole block is on stderr). - Each warning's **existing wording is preserved** — the consolidation standardizes only the prefix, stream, and color, never the message text. @@ -91,7 +91,7 @@ Deterministic output for unit tests; no dependency on terminal/ioctl; consistent ## Cross-references - Spec doc: none — phase-separator output is per-command diagnostic structure, not part of `docs/specs/cli-surface.md`'s per-subcommand flag surface. This contract lives in memory only. -- Source: `src/internal/worktree/errors.go` (`PhaseSeparator`, `phaseSeparatorWidth`, `Warn`, the `NO_COLOR`-blanking `init()`), `src/internal/worktree/crud.go` (`RunWorktreeSetupWithObserver` Init separator emission), `src/cmd/wt/create.go` (Git + Open separators, stdout path line gated behind the end-of-function `initFailed` exit-7 guard per `260626-n6ma`, `wt.Warn` call sites), `src/cmd/wt/init.go` (`runInitScript` Init separator + stderr realignment), `src/cmd/wt/delete.go` (`wt.Warn` call sites — incl. the two pre-menu warnings realigned stdout→stderr with `fmt.Fprintln(os.Stderr)` spacing framing). +- Source: `src/internal/worktree/errors.go` (`PhaseSeparator`, `phaseSeparatorWidth`, `Warn`, the `NO_COLOR`-blanking `init()`), `src/internal/worktree/init.go` (`InitNotFound.RenderWarning` / `RenderDefaultSkipWarning` — the two out-of-band multi-line `Warning:` renderers that do not route through `Warn`), `src/internal/worktree/crud.go` (`RunWorktreeSetupWithObserver` Init separator emission), `src/cmd/wt/create.go` (Git + Open separators, stdout path line gated behind the end-of-function `initFailed` exit-7 guard per `260626-n6ma`, `wt.Warn` call sites), `src/cmd/wt/init.go` (`runInitScript` Init separator + stderr realignment; the `260705-irnt` exit-3 default-skip carve-out returns before the `Worktree init complete.` trailer), `src/cmd/wt/delete.go` (`wt.Warn` call sites — incl. the two pre-menu warnings realigned stdout→stderr with `fmt.Fprintln(os.Stderr)` spacing framing). - Tests: `src/internal/worktree/errors_test.go` (`PhaseSeparator` unit test: label presence, colored ANSI + `─`, NO_COLOR ASCII `-` with no `\033[`, 40-column visible width, no trailing newline; `260622-log5`: `Warn` unit test — color-wrapped `Warning:` to stderr when colored, plain `Warning: ` with no ANSI under blanked color vars); `src/cmd/wt/create_test.go` (`Created worktree:` stderr + one-line-stdout guard); `src/cmd/wt/init_test.go` (combined-stream `Running worktree init` / `Worktree init complete`); `src/cmd/wt/delete_test.go` (`260622-log5`: the uncommitted-changes / unpushed-commits warnings appear on stderr, not stdout). - Constitution: Principle I (Single-Binary CLI, No Hidden State — motivated the fixed width, no terminal query), Principle V (Internal Package Boundary — `Warn` lives in `internal/worktree`, keeping `cmd/` thin), Principle VI (Interactive by Default, Scriptable on Demand — stdout=machine-result keeps `wt create`'s path line deterministic for launchers/operators). - Sibling memory: `wt-cli/init-failure-contract.md` — the init runner / resolver / `*InitNotFound` contract this change builds on (the Init separator sits next to the `Running worktree init` line and respects the not-found and reinstall-window invariants); as of `260626-n6ma` it also owns the interactive open-anyway prompt + `Go:` banner hint that make the Open phase reachable after an init failure and keep the stdout path line suppressed on exit 7. `/wt-cli/go-command-contract.md` — `wt go`'s stderr navigation confirmation (`260622-log5`) honors this same stdout=machine / stderr=human stream discipline. `wt-cli/list-status-contract.md` and `wt-cli/menu-navigation-contract.md` — same post-change invariant-capture pattern for sibling `wt` subcommands. diff --git a/docs/memory/wt-cli/init-failure-contract.md b/docs/memory/wt-cli/init-failure-contract.md index 7461ba0..5e937b1 100644 --- a/docs/memory/wt-cli/init-failure-contract.md +++ b/docs/memory/wt-cli/init-failure-contract.md @@ -42,6 +42,16 @@ This file documents the contract that `wt create` and `wt init` honor when the i - `cmd/wt/init.go` (the `wt init` subcommand) also exits via `os.Exit(wt.ExitInitFailed)` on init-script non-zero exit. Returning the error to Cobra would map to `ExitGeneralError = 1`; the explicit `os.Exit` ensures both command paths emit the typed code. (`wt init` has no open-anyway prompt — it does not call `PrintInitFailureBanner` and is unchanged by `260626-n6ma`.) - Operators (shell wrappers, fab-kit, `hop`) can distinguish "worktree exists, init didn't complete" from any other generic failure — including when the user chose to open the kept worktree anyway. +### Default-not-applicable run-time skip bypasses the failure contract (`260705-irnt`) + +> The one carve-out from "`ExitInitFailed = 7` on EVERY path" above. When the **built-in default** init script `fab sync` runs in a repo that is not fab-managed and exits fab-kit's `ExitNotManaged = 3`, the whole failure contract is bypassed: no `ExitInitFailed`, no kept-worktree banner, no open-anyway prompt, no `Worktree init complete.` line, exit 0, and `wt create`'s Open phase proceeds exactly as on init success. + +- **Run-time classification, not resolve-time.** This is a distinct axis from the not-found skip above: resolution *succeeds* and the script genuinely *runs*. Only after `cmd.Run()` returns non-zero is the outcome classified. So it composes with, and is mutually exclusive from, the resolve-time `CommandNotOnPath` / `FileNotFound` skips by construction (post-run vs. pre-run — a script that never ran cannot have exited 3; a script that exited 3 was resolved and ran). `ResolveInitInvocation` and `InitNotFound` are untouched. +- **Single shared helper, keyed on the documented exit code.** `src/internal/worktree/init.go` exposes `func DefaultNotApplicable(err error, isDefault bool) bool` — it returns true only when `isDefault` is true AND `err` unwraps (via `errors.As`) to an `*exec.ExitError` whose code is exactly the documented `exitNotManaged = 3` const. `func RenderDefaultSkipWarning() string` is the single renderer for the two-line skip copy. Both mirror the `RenderWarning()` anti-drift discipline so the two run sites cannot diverge. `wt` embeds the numeric `3` (documented as fab-kit's `ExitNotManaged`) rather than importing from fab-kit — separate repos, no import (Constitution I). +- **Provenance-gated (built-in default only).** The gate is `isDefault`, which `InitScriptPath()` reports as provenance — true only when `WORKTREE_INIT_SCRIPT` is unset/empty, never by string equality. An explicit `WORKTREE_INIT_SCRIPT="fab sync"` (`isDefault=false`) still hard-fails on exit 3 exactly like any other configured script — the user opted into it, so the failure is theirs to see. `DefaultNotApplicable` returns false whenever `isDefault` is false, regardless of exit code. +- **Both run sites route through the helper.** `RunWorktreeSetupWithObserver` (`crud.go:178–181`) and `wt init`'s own `cmd.Run()` failure path (`cmd/wt/init.go:128–131`) apply `DefaultNotApplicable` before the hard-fail path: on the skip they write `RenderDefaultSkipWarning()` to stderr and return `nil` (no `Worktree init complete.`). `wt init` reclaims the terminal foreground **first** (reclaim-before-write ordering preserved, `init.go:119–121`) so the skip-warning write cannot SIGTTOU. `wt create`'s two call sites (normal via `RunWorktreeSetupWithObserver`, `--reuse` via `RunWorktreeSetup`) pass `isDefault` from `InitScriptPath()` and need no other logic — the skip surfaces as a `nil` return, so `initFailed` is never set (no banner, no prompt, exit 0, stdout path line unaffected). +- **Old fab-kit degrades to today's hard-fail.** An installed fab-kit predating PR #471 exits 1 in a non-fab repo, which is not 3, so `DefaultNotApplicable` returns false and behavior is exactly as today (hard fail, exit 7, banner). No version detection, no fallback probe — the skip lights up only when fab-kit updates. Every non-zero exit other than 3 (any provenance) keeps `ExitInitFailed = 7` on every path. + ### Interactive open-anyway prompt (`260626-n6ma`) - On init-script non-zero exit, `wt create` offers to open the kept worktree anyway — but **only when interactive**, gated by `!nonInteractive && reclaimTTY` (`reclaimTTY` is `term.IsTerminal(os.Stdin.Fd())`, already computed earlier in the init block; `create.go:388`). This is the same TTY / `--non-interactive` discipline the rest of `create` uses (Constitution VI). @@ -135,7 +145,7 @@ The interactive *Yes* path does NOT build a second open codepath inside the init ## Cross-references - Spec doc: `docs/specs/init-protocol.md` — wire contract + "Script failure semantics" + "SIGINT during init" sections (rewritten by this change). -- Source: `src/internal/worktree/init.go` (resolver, `InitNotFound`, `RenderWarning`), `src/internal/worktree/errors.go` (`ExitInitFailed`, `PrintInitFailureBanner` + the `bannerLabelGo` `Go:` hint, `260626-n6ma`), `src/internal/worktree/crud.go` (`RunWorktreeSetup`, `RunWorktreeSetupWithObserver`), `cmd/wt/create.go` (rollback disarm + handler swap + unified post-init foreground reclaim/teardown + `initFailed`-flag-driven open-anyway fall-through and end-of-function exit, `260626-n6ma`), `cmd/wt/init.go` (resolver consumer + foreground capture/reclaim around `runInitScript`'s `cmd.Run()`), `src/internal/worktree/menu.go` (`ConfirmYesNo`, the open-anyway prompt), `src/internal/worktree/apps.go` (`shellQuoteSingle`, `OpenInApp` test seam), `src/cmd/wt/tty_unix.go` / `src/cmd/wt/tty_windows.go` (`terminalForeground` / `reclaimTerminalForeground` build-tag split). +- Source: `src/internal/worktree/init.go` (resolver, `InitNotFound`, `RenderWarning`; the `260705-irnt` run-time skip helpers `DefaultNotApplicable` / `RenderDefaultSkipWarning` + the documented `exitNotManaged = 3` const), `src/internal/worktree/context.go` (`InitScriptPath() (script string, isDefault bool)` — provenance report, `260705-irnt`), `src/internal/worktree/errors.go` (`ExitInitFailed`, `PrintInitFailureBanner` + the `bannerLabelGo` `Go:` hint, `260626-n6ma`), `src/internal/worktree/crud.go` (`RunWorktreeSetup`, `RunWorktreeSetupWithObserver` — both gained the `isDefault` param + post-run `DefaultNotApplicable` classification, `260705-irnt`), `cmd/wt/create.go` (rollback disarm + handler swap + unified post-init foreground reclaim/teardown + `initFailed`-flag-driven open-anyway fall-through and end-of-function exit, `260626-n6ma`; both init call sites pass `isDefault`, `260705-irnt`), `cmd/wt/init.go` (resolver consumer + foreground capture/reclaim around `runInitScript`'s `cmd.Run()`; `DefaultNotApplicable` skip before `os.Exit(ExitInitFailed)`, reclaim-before-write preserved, `260705-irnt`), `src/internal/worktree/menu.go` (`ConfirmYesNo`, the open-anyway prompt), `src/internal/worktree/apps.go` (`shellQuoteSingle`, `OpenInApp` test seam), `src/cmd/wt/tty_unix.go` / `src/cmd/wt/tty_windows.go` (`terminalForeground` / `reclaimTerminalForeground` build-tag split). - Tests: `src/cmd/wt/tty_pty_test.go` — `TestIntegration_ReclaimForegroundAfterInit_NotStopped` (Unix-only, `//go:build !windows`): allocates a real PTY via an in-tree `openpty` helper on `x/sys/unix` (`/dev/ptmx` + `TIOCSPTLCK`/`TIOCGPTN` + `/dev/pts/N`, no `creack/pty`), runs `wt create` under it via a session-leader launcher with a foreground-stranding init script, and asserts the `wt` process is not left `WIFSTOPPED`; self-skips when a PTY / process groups can't be set up. Verified non-vacuous (fails `WIFSTOPPED` without the reclaim). - Constitution: Principle III (Typed Exit Codes) — `ExitInitFailed` is the canonical example for "worktree exists, init didn't complete". Principle I (Single-Binary CLI, slim deps) — drove the `x/sys/unix` ioctls (no new module) and the in-tree `openpty` helper (no `creack/pty`). Principle VI (Interactive by Default, Scriptable on Demand) — the reclaim restores the broken interactive Open path; the `term.IsTerminal` gate keeps the scriptable / non-interactive path a no-op. - Sibling memory: `wt-cli/create-output-phases.md` — the Open-phase separator + menu render now sits behind the unconditional pre-Open foreground reclaim. diff --git a/docs/specs/cli-surface.md b/docs/specs/cli-surface.md index 7a34632..93b2a5e 100644 --- a/docs/specs/cli-surface.md +++ b/docs/specs/cli-surface.md @@ -11,12 +11,13 @@ Run `wt --help` for the full inline reference. | Constant | Value | Meaning | |----------|-------|---------| | `ExitSuccess` | 0 | Command completed successfully | -| `ExitGeneralError` | 1 | Non-specific failure (cannot resolve repo context, init failed, no default app, etc.) | +| `ExitGeneralError` | 1 | Non-specific failure (cannot resolve repo context, no default app, unresolved target, etc.) | | `ExitInvalidArgs` | 2 | Caller supplied incompatible flags or invalid input (bad branch name, bad `--base` ref, mutually exclusive flags) | | `ExitGitError` | 3 | A `git` invocation failed or the working dir is not a git repository | | `ExitRetryExhausted` | 4 | Random-name generator could not find a non-colliding name after retries | | `ExitByobuTabError` | 5 | Failed to open the worktree in a byobu tab | | `ExitTmuxWindowError` | 6 | Failed to open the worktree in a tmux window | +| `ExitInitFailed` | 7 | The init script ran but exited non-zero (`wt create` keeps the worktree; `wt init` too). Distinct from `ExitGeneralError` so operators can detect "worktree exists, init didn't complete". See [`init-protocol.md`](init-protocol.md). | Subcommands map domain failures to these codes via `wt.ExitWithError`. SIGINT during `wt create` exits 130 after rolling back partial state (standard Unix @@ -47,7 +48,12 @@ when the chosen app was `open_here` because the wrapper consumed it via Exit codes: `ExitInvalidArgs` for flag misuse or invalid `--base`/branch name; `ExitGitError` for `git worktree add` failures; `ExitRetryExhausted` for name -generation; `ExitGeneralError` for init script failure. +generation; `ExitInitFailed` (7) when the init script runs but exits non-zero +(the worktree is kept; the code holds on every init-failure path, including a +successful interactive open-anyway open). Two init outcomes are **not** failures +and exit 0: a graceful skip when the init command/file is missing, and — for the +built-in default `fab sync` only — the default-not-applicable skip when the repo +is not fab-managed. See [`init-protocol.md`](init-protocol.md). ## `wt list` @@ -174,12 +180,17 @@ lookup contract is documented in [`init-protocol.md`](init-protocol.md). No flags. No positional args. -Exit codes: `ExitGitError` when not in a repo; `ExitGeneralError` (1) when the -init script runs but exits non-zero (the script's exit code is **not** -preserved — `RunE` returns an error, which `main.go` maps to -`ExitGeneralError`). Missing init command/file results in a graceful skip with -guidance — exit 0. See [`init-protocol.md`](init-protocol.md) for full -semantics. +Exit codes: `ExitGitError` when not in a repo; `ExitInitFailed` (7) when the +init script runs but exits non-zero (the script's own exit code is **not** +preserved — `runInitScript` maps every hard init failure to the typed +`ExitInitFailed` via an explicit `os.Exit(wt.ExitInitFailed)`, matching +`wt create`, rather than returning the error to `RunE` — which would map to +`ExitGeneralError`). Three init outcomes are non-failures and exit 0 with +guidance: (1) the init command is not on PATH, (2) the init file path does not +exist, and (3) — for the built-in default `fab sync` only — the repo is not +fab-managed and `fab sync` exits `ExitNotManaged = 3` (run-time skip; +provenance-gated, so an explicit `WORKTREE_INIT_SCRIPT` still exits 7). See +[`init-protocol.md`](init-protocol.md) for full semantics. ## `wt update` diff --git a/docs/specs/init-protocol.md b/docs/specs/init-protocol.md index c90f21e..f3033ac 100644 --- a/docs/specs/init-protocol.md +++ b/docs/specs/init-protocol.md @@ -11,15 +11,25 @@ The init script value is resolved by `worktree.InitScriptPath()`: its value is used verbatim. 2. Otherwise, the default value `"fab sync"` is used. +Alongside the value, `InitScriptPath` reports its **provenance** — whether the +value is the built-in default or an explicit override: + ```go -func InitScriptPath() string { +func InitScriptPath() (script string, isDefault bool) { if v := os.Getenv("WORKTREE_INIT_SCRIPT"); v != "" { - return v + return v, false } - return "fab sync" + return "fab sync", true } ``` +`isDefault` is true **only** when `WORKTREE_INIT_SCRIPT` is unset/empty. It is +**provenance, not string equality**: an explicit `WORKTREE_INIT_SCRIPT="fab sync"` +returns `("fab sync", false)` even though the string matches the default. The +run-time graceful-skip classification (see **Graceful skip behavior** case 3) +keys on this flag, so an explicitly configured script always fails hard while the +built-in default may skip gracefully in a non-fab-managed repo. + The default exists so that users with `fab-kit` installed get a working init flow out of the box. Users who do not run fab-kit override the env var (typically in `.envrc`, `~/.zshrc`, or a project-local rc file) to point at @@ -70,7 +80,10 @@ separator helper (`PhaseSeparator`) lives in `internal/worktree/errors.go`. ## Graceful skip behavior -The init step is non-blocking when the script cannot be located. Two cases: +The init step is non-blocking when the script cannot be located, or when the +built-in default does not apply. Cases 1–2 are **resolve-time** (the script +never runs); case 3 is **run-time** (the default script runs and reports it +does not apply). 1. **Command not on PATH** (e.g., `fab` not installed): ``` @@ -89,9 +102,41 @@ The init step is non-blocking when the script cannot be located. Two cases: ``` Again, exit 0. -This means a freshly-cloned repo without an init script (and without -fab-kit installed) silently no-ops on `wt init`, which is the desired -behavior for the "I just want to use the worktree" path. +3. **Default init not applicable** (repo is not fab-managed): + ``` + Warning: not a fab-managed repo — skipping init (default "fab sync" does not apply) + Set WORKTREE_INIT_SCRIPT to a custom script, or run 'fab init' to make this repo fab-managed. + ``` + When the **built-in default** `fab sync` runs and exits `ExitNotManaged = 3` + (fab-kit ≥ PR #471 checks a config walk-up before any git resolution), `wt` + treats it as "the default does not apply" rather than an init failure. This is + a **run-time classification** — resolution succeeded and the script genuinely + ran; only after `cmd.Run()` returns is the `(isDefault, exit code 3)` pair + inspected (via `errors.As` → `*exec.ExitError`) by the shared helper + `DefaultNotApplicable`. On the skip: the warning above is printed to stderr, + `wt init` exits 0, and `wt create` proceeds to its Open phase exactly as on + init success (no kept-worktree banner, no open-anyway prompt, no + `Worktree init complete.` line, exit 0). + + Two constraints scope this narrowly: + - **Provenance-gated**: only the built-in default skips. An explicit + `WORKTREE_INIT_SCRIPT="fab sync"` (`isDefault=false`) still hard-fails on + exit 3 — the user opted into that script, so the failure is theirs to see + (see **Script failure semantics**). + - **Exit-3-only, no version detection**: only exit code 3 skips. An older + fab-kit predating PR #471 exits 1 in a non-fab repo, which is not 3, so it + degrades to today's hard-fail (`ExitInitFailed = 7` + banner). There is no + fallback probe — the feature lights up when fab-kit updates. + + Because detection is post-run, the Init phase separator, + `Running worktree init...`, and fab's own `ERROR: not in a fab-managed repo…` + stderr line all print before the skip warning; the skip warning is the last + word and the exit code is 0. + +Cases 1–2 mean a freshly-cloned repo without an init script (and without +fab-kit installed) silently no-ops on `wt init`; case 3 extends that "I just +want to use the worktree" ergonomics to a repo where fab-kit **is** installed +but the repo itself is not fab-managed. ## Resolution contract @@ -123,6 +168,17 @@ exits with `ExitInitFailed = 7` — a typed exit code distinct from programmatically detect "worktree exists, init didn't complete" and offer a retry-init affordance. +**Exit-3 carve-out for the built-in default.** The one exception is the +default-not-applicable skip (see **Graceful skip behavior** case 3): when the +**built-in default** `fab sync` (`isDefault=true`) exits `ExitNotManaged = 3`, +`wt` does **not** exit 7 — it prints the skip warning and exits 0, treating the +init step as a no-op. This carve-out is narrow: it applies **only** to the +built-in default and **only** to exit code 3. An explicit `WORKTREE_INIT_SCRIPT` +value (including the literal `"fab sync"`) keeps `ExitInitFailed = 7` on **every** +non-zero exit, and the default itself keeps `ExitInitFailed = 7` on every non-zero +exit **other than 3**. All the failure behavior described below (kept worktree, +banner, open-anyway prompt) applies to those hard-failure paths, unchanged. + For `wt create`, init failure does **not** roll back the just-created worktree. The git operations all succeeded; only the user-supplied script failed. The worktree directory, the branch, and any fetched refs are all diff --git a/fab/changes/260705-irnt-graceful-default-init-skip/.history.jsonl b/fab/changes/260705-irnt-graceful-default-init-skip/.history.jsonl new file mode 100644 index 0000000..6580e4a --- /dev/null +++ b/fab/changes/260705-irnt-graceful-default-init-skip/.history.jsonl @@ -0,0 +1,14 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-07-05T15:43:19Z"} +{"args":"wt should continue to work without errors in non-fab-kit-managed repos: gracefully skip the default 'fab sync' init when the repo is not fab-managed (marker probe mirroring fab ResolveConfig walk-up), instead of exit 7 + failure banner","cmd":"fab-new","event":"command","ts":"2026-07-05T15:43:19Z"} +{"delta":"+4.7","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-07-05T15:44:26Z"} +{"delta":"+0.0","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-07-05T18:13:01Z"} +{"cmd":"fab-clarify","event":"command","ts":"2026-07-05T18:13:01Z"} +{"cmd":"fab-fff","event":"command","ts":"2026-07-05T18:36:42Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-07-05T18:36:50Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-07-05T18:44:41Z"} +{"cmd":"fab-continue","event":"command","ts":"2026-07-05T18:46:15Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-07-05T19:03:43Z"} +{"event":"review","result":"passed","ts":"2026-07-05T19:03:43Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-07-05T19:07:25Z"} +{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-07-05T19:08:48Z"} +{"event":"review","result":"passed","ts":"2026-07-05T19:13:36Z"} diff --git a/fab/changes/260705-irnt-graceful-default-init-skip/.status.yaml b/fab/changes/260705-irnt-graceful-default-init-skip/.status.yaml new file mode 100644 index 0000000..198cad9 --- /dev/null +++ b/fab/changes/260705-irnt-graceful-default-init-skip/.status.yaml @@ -0,0 +1,55 @@ +id: irnt +name: 260705-irnt-graceful-default-init-skip +created: 2026-07-05T15:43:19Z +created_by: sahil-noon +change_type: fix +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: done + review-pr: done +plan: + generated: true + task_count: 9 + acceptance_count: 20 + acceptance_completed: 20 +confidence: + certain: 4 + confident: 4 + tentative: 0 + unresolved: 0 + score: 4.7 + fuzzy: true + dimensions: + signal: 74.4 + reversibility: 83.8 + competence: 85.0 + disambiguation: 76.3 +stage_metrics: + intake: {started_at: "2026-07-05T15:43:19Z", driver: fab-new, iterations: 1, completed_at: "2026-07-05T18:36:50Z"} + apply: {started_at: "2026-07-05T18:36:50Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-05T18:44:41Z"} + review: {started_at: "2026-07-05T18:44:41Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-05T19:03:43Z"} + hydrate: {started_at: "2026-07-05T19:03:43Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-05T19:07:25Z"} + ship: {started_at: "2026-07-05T19:07:25Z", driver: fab-fff, iterations: 1, completed_at: "2026-07-05T19:08:48Z"} + review-pr: {started_at: "2026-07-05T19:08:48Z", driver: git-pr, iterations: 1, completed_at: "2026-07-05T19:13:36Z"} +prs: + - https://github.com/sahil87/wt/pull/29 +true_impact: + added: 812 + deleted: 43 + net: 769 + excluding: + added: 335 + deleted: 23 + net: 312 + tests: + added: 224 + deleted: 5 + net: 219 + computed_at: "2026-07-05T19:08:48Z" + computed_at_stage: ship +# true_impact: lazily created on first stage-finish that computes it (no placeholder here). +last_updated: 2026-07-05T19:13:36Z diff --git a/fab/changes/260705-irnt-graceful-default-init-skip/intake.md b/fab/changes/260705-irnt-graceful-default-init-skip/intake.md new file mode 100644 index 0000000..dba623d --- /dev/null +++ b/fab/changes/260705-irnt-graceful-default-init-skip/intake.md @@ -0,0 +1,119 @@ +# Intake: Graceful Default-Init Skip in Non-Fab-Managed Repos + +**Change**: 260705-irnt-graceful-default-init-skip +**Created**: 2026-07-05 + +## Origin + +Conversational (`/fab-discuss` session, 2026-07-05). The user shared a screenshot of `wt create` in a non-fab-managed repo (`planner`): the git phase succeeded, then the default init (`fab sync`) failed with `ERROR: not in a fab-managed repo. Run 'fab init' to set one up`, producing the init-failure banner, the open-anyway prompt, and exit 7. + +> wt should continue to work without errors even in "non fab-kit managed repo". What changes do we need for this? + +**Design revision (same day)**: the intake originally specified a wt-side marker probe (walk up looking for `fab/project/config.yaml`) because `fab sync` offered no distinguishable outcome — that fab-side contract was backlogged as fab-kit `[52i9]`. The user then implemented and released it (fab-kit PR #471): `fab sync` now exits **`ExitNotManaged = 3`** when not in a fab-managed repo, checked via a config walk-up *before* any git resolution, symmetric with `fab-kit migrations-status`. This intake was revised to key the skip on that exit code; the marker probe is now the rejected alternative. + +## Why + +1. **Pain point**: `wt`'s default init script is `"fab sync"` (`InitScriptPath()`, `src/internal/worktree/context.go:176`). The existing graceful-skip contract only covers `fab` *not being installed* (`CommandNotOnPath`) or a script file not existing (`FileNotFound`). When fab-kit IS installed but the repo is not fab-managed, `fab sync` runs and fails, and `wt` treats that as a real init failure: `ExitInitFailed = 7`, kept-worktree banner, interactive open-anyway prompt. The zero-config convenience default becomes an error in **every** non-fab repo. +2. **Consequence if unfixed**: every `wt create` / `wt init` in a non-fab repo errors and prompts; scripts and operators branching on exit codes see a spurious 7; users must set `WORKTREE_INIT_SCRIPT` per-repo or per-shell just to silence a default they never opted into. +3. **Why this approach**: the built-in default should skip gracefully when it does not apply; an explicitly configured script should keep failing hard (the user opted in, the failure is theirs to see). Detection: run `fab sync` as today and treat **exit code 3** (`ExitNotManaged`, fab-kit ≥ PR #471) as "default does not apply" — fab stays the sole authority on what is fab-managed; wt embeds no knowledge of fab's repo layout. Rejected alternative: the original wt-side marker probe (`fab/project/config.yaml` walk-up mirroring fab's `ResolveConfig`) — it avoided spawning `fab` but duplicated fab's detection logic in wt, with silent-under-sync drift risk if fab ever moved the marker. Superseded by the released exit-code contract. + +## What Changes + +### 1. `InitScriptPath()` signals defaultness (`src/internal/worktree/context.go`) + +The init runner must know whether the value came from the built-in default or the env var: + +```go +// Today: +func InitScriptPath() string { + if v := os.Getenv("WORKTREE_INIT_SCRIPT"); v != "" { + return v + } + return "fab sync" +} +``` + +Change to return `(script string, isDefault bool)` (or an equivalent small struct / second function — exact shape decided at apply). `isDefault` is true **only** when `WORKTREE_INIT_SCRIPT` is unset/empty. An explicit `WORKTREE_INIT_SCRIPT="fab sync"` yields `isDefault=false` even though the string matches — behavior must key on provenance, not string equality. + +Callers to update: `src/cmd/wt/create.go:188` (reuse path), `src/cmd/wt/create.go:279` (normal path), `src/cmd/wt/init.go:63`. + +### 2. Run-time skip classification (exit code 3, default only) + +This is a **run-time** decision, not a resolve-time one — `ResolveInitInvocation` and the `InitNotFound` structure are untouched (resolution still succeeds; the script genuinely runs). After `cmd.Run()` returns a non-zero exit: + +- `isDefault` **and** exit code == **3** (`ExitNotManaged`, extracted via `errors.As` → `*exec.ExitError`) → **not an init failure**: print the skip warning to stderr, treat init as a no-op (`nil` return / exit 0). No kept-worktree banner, no open-anyway prompt, `wt create` proceeds to the Open phase exactly as on init success. +- `isDefault` false → any non-zero exit (including 3) stays a hard failure, exactly as today. The provenance rule from the original design is unchanged. +- Exit codes other than 3 → hard failure regardless of provenance (`ExitInitFailed = 7` on every existing path). + +Put the classification + warning rendering in one shared helper in `src/internal/worktree/init.go` (alongside `InitNotFound`, e.g. `func DefaultNotApplicable(err error, isDefault bool) bool` plus a canonical warning renderer) so the two run sites — `RunWorktreeSetupWithObserver` (`crud.go:166`) and `wt init`'s own `cmd.Run()` (`cmd/wt/init.go:115`) — cannot drift, mirroring how `RenderWarning()` already centralizes the not-found copy. + +Agreed warning copy (final wording may be polished at apply, keeping both halves — the skip statement and the two escape hatches): + +``` +Warning: not a fab-managed repo — skipping init (default "fab sync" does not apply) +Set WORKTREE_INIT_SCRIPT to a custom script, or run 'fab init' to make this repo fab-managed. +``` + +**Accepted output tradeoff**: because the decision is post-run, the Init phase separator, `Running worktree init...`, and fab's own `ERROR: not in a fab-managed repo...` stderr line all print before the skip warning. That is acceptable (arguably more transparent than silently not running); the skip warning is the last word and the exit code is 0. `Worktree init complete.` MUST NOT print on the skip path. + +### 3. Threading `isDefault` to the run sites + +- `RunWorktreeSetup` / `RunWorktreeSetupWithObserver` (`src/internal/worktree/crud.go:134,143`) gain the `isDefault` knowledge (parameter or options struct — apply decides), applying the classification after `cmd.Run()`. +- `wt init` (`src/cmd/wt/init.go:115-127`) applies the same helper before its `os.Exit(wt.ExitInitFailed)`: default + exit 3 → skip warning + `return nil` (exit 0). Terminal-foreground reclaim ordering is preserved (reclaim before the warning write, as the failure path already does). +- `wt create`'s two call paths need no logic beyond passing `isDefault`: the skip surfaces as a `nil` return from the runner, so `initFailed` is never set — no banner, no prompt, exit 0, stdout final-path line unaffected. The `--reuse` path (`create.go:188`) inherits the skip the same way. + +### 4. Old fab-kit degradation (accepted) + +An installed fab-kit predating PR #471 exits 1 in non-fab repos → wt behaves exactly as today (hard fail, exit 7). No version detection, no fallback probe — the feature lights up when fab-kit updates. This is a strict no-regression posture: no existing behavior changes except "default script + exit 3", which old fab-kit never produces. + +### 5. Tests + +Unit (`src/internal/worktree/init_test.go`): +- Classification helper: (exit 3, default) → skip; (exit 3, explicit) → failure; (exit 1, default) → failure; (nil error) → not a skip; (non-`*exec.ExitError` error, default) → failure. +- Warning renderer output for the skip case. +- `InitScriptPath` provenance: env unset → (`"fab sync"`, true); env set to anything incl. `"fab sync"` → (value, false). + +Integration (`src/cmd/integration_test.go`), using a stub `fab` in a `t.TempDir()` prepended to PATH (per `fab/project/code-review.md`, rely on `runWt` env isolation — no host side effects): +- Stub `fab` exits **3** + non-fab repo + default init → `wt create --non-interactive` exits 0, worktree created, skip warning on stderr, no failure banner; `wt init` exits 0. +- Stub `fab` exits **1** (old fab-kit) → `wt create` exits 7 with banner (unchanged). +- `WORKTREE_INIT_SCRIPT="fab sync"` explicit + stub exits 3 → exit 7 (provenance rule). + +### 6. Spec update + +`docs/specs/init-protocol.md`: +- § Lookup contract: `InitScriptPath` now reports defaultness (provenance, not string equality). +- § Graceful skip behavior: add **case 3** — default init not applicable: `fab sync` exited `ExitNotManaged = 3` (fab-kit ≥ PR #471); run-time classification, warning text, exit 0; explicit same-string value is never skipped; older fab-kit degrades to today's hard-fail. +- § Script failure semantics: note the exit-3 carve-out for the default script (all other non-zero exits keep `ExitInitFailed = 7` on every path). + +Memory updates flow through hydrate (see Affected Memory). + +## Affected Memory + +- `wt-cli/init-failure-contract`: (modify) add the default-not-applicable skip as a run-time case that bypasses the failure contract entirely (no `ExitInitFailed`, no banner, no prompt) — provenance-gated (built-in default only) and keyed on fab-kit's documented `ExitNotManaged = 3`. + +## Impact + +- **Code**: `src/internal/worktree/context.go` (InitScriptPath signature), `src/internal/worktree/init.go` (classification helper + warning renderer), `src/internal/worktree/crud.go` (runner threading + post-run classification), `src/cmd/wt/create.go` (two call sites pass provenance), `src/cmd/wt/init.go` (call site + post-run classification before `os.Exit`). +- **Tests**: `init_test.go`, `context_test.go` (if InitScriptPath tests exist), `cmd/integration_test.go`. +- **Docs**: `docs/specs/init-protocol.md`; `docs/memory/wt-cli/init-failure-contract.md` via hydrate. +- **Behavior surface**: exit codes unchanged on all existing paths; the only new behavior is exit 0 + warning where today's outcome is exit 7 + banner (default script, `fab sync` exit 3). No new dependencies. External callers (`hop`, fab-kit operators) keep the `ExitInitFailed` signal for real failures. Cross-repo dependency: fab-kit ≥ the PR #471 release for the skip to activate (older fab-kit: unchanged behavior). +- **CI**: Go — `gofmt` enforced before vet/test (module root `src/`). + +## Open Questions + +*(none — all decision points were resolved in the /fab-discuss session and the post-#471 design revision; see Assumptions)* + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | Skip keys on `fab sync` exit code `ExitNotManaged = 3` (fab-kit PR #471, released) — not a wt-side marker probe | Discussed — user shipped the fab-side contract specifically to supersede the interim probe; fab stays authoritative, no layout coupling | S:95 R:75 A:90 D:90 | +| 2 | Certain | Skip gates on provenance: built-in default only; explicit `WORKTREE_INIT_SCRIPT="fab sync"` still hard-fails on any non-zero exit incl. 3 | Discussed — agreed principle ("the user never opted into the default; an explicit script failure is theirs to see") | S:95 R:80 A:90 D:90 | +| 3 | Certain | Skip semantics: stderr warning, nil/exit 0, no banner, no open-anyway prompt, Open phase proceeds as on success | Matches the existing graceful-skip convention in init-protocol.md; call sites already treat nil as init-ok | S:80 R:90 A:90 D:85 | +| 4 | Certain | Run-time classification in a single shared helper in `init.go` (both run sites use it); `ResolveInitInvocation`/`InitNotFound` untouched | The decision needs the exit code, which only exists post-run; centralizing mirrors the existing `RenderWarning()` anti-drift pattern | S:70 R:85 A:85 D:75 | +| 5 | Confident | Pre-skip output noise accepted (separator, "Running worktree init...", fab's ERROR line print before the skip warning) | Inherent to post-run detection; more transparent than suppressing; skip warning is the last word | S:65 R:85 A:80 D:70 | +| 6 | Confident | Old fab-kit (exit 1) degrades to today's hard-fail — no version detection, no fallback probe | Strict no-regression; feature activates on fab-kit update; avoids reintroducing the coupling the revision removed | S:70 R:80 A:85 D:75 | +| 7 | Confident | Exact seam shape (signatures vs. options struct on `InitScriptPath` / runner) left to apply | Implementation detail; internal package, no external API; easily adjusted in review | S:65 R:85 A:80 D:60 | +| 8 | Confident | Warning copy: "not a fab-managed repo — skipping init …" + hint (set `WORKTREE_INIT_SCRIPT` or run `fab init`) | Reversible user-facing copy; follows stderr conventions; both escape hatches named | S:55 R:90 A:80 D:65 | + +8 assumptions (4 certain, 4 confident, 0 tentative, 0 unresolved). diff --git a/fab/changes/260705-irnt-graceful-default-init-skip/plan.md b/fab/changes/260705-irnt-graceful-default-init-skip/plan.md new file mode 100644 index 0000000..b2f9c37 --- /dev/null +++ b/fab/changes/260705-irnt-graceful-default-init-skip/plan.md @@ -0,0 +1,196 @@ +# Plan: Graceful Default-Init Skip in Non-Fab-Managed Repos + +**Change**: 260705-irnt-graceful-default-init-skip +**Intake**: `intake.md` + +## Requirements + +### Init: Default-Script Provenance + +#### R1: `InitScriptPath` reports whether the value is the built-in default +`InitScriptPath()` (`src/internal/worktree/context.go`) SHALL report the provenance of the resolved init-script value in addition to the value itself, changing its signature to return `(script string, isDefault bool)`. `isDefault` MUST be true **only** when `WORKTREE_INIT_SCRIPT` is unset or empty (the built-in `"fab sync"` default), and false whenever the env var is set to any non-empty value — including the literal `"fab sync"`. Provenance is determined by env-var presence, never by string equality against `"fab sync"`. + +- **GIVEN** `WORKTREE_INIT_SCRIPT` is unset (or empty) +- **WHEN** `InitScriptPath()` is called +- **THEN** it returns `("fab sync", true)` + +- **GIVEN** `WORKTREE_INIT_SCRIPT="fab sync"` (explicit, string matches the default) +- **WHEN** `InitScriptPath()` is called +- **THEN** it returns `("fab sync", false)` — provenance is false because the value came from the env var + +- **GIVEN** `WORKTREE_INIT_SCRIPT="custom/init.sh"` +- **WHEN** `InitScriptPath()` is called +- **THEN** it returns `("custom/init.sh", false)` + +### Init: Run-Time Skip Classification + +#### R2: A shared helper classifies the default-not-applicable outcome +`src/internal/worktree/init.go` SHALL expose a single shared classification helper that decides, from `(err error, isDefault bool)`, whether an init-script non-zero exit is the "default does not apply" skip case. It MUST return true **only** when `isDefault` is true AND `err` unwraps (via `errors.As`) to an `*exec.ExitError` whose exit code is exactly `3` (fab-kit's `ExitNotManaged`). It MUST return false for: a nil `err`; any exit code other than 3; an `err` that does not unwrap to `*exec.ExitError`; and any case where `isDefault` is false (regardless of exit code, including 3). The classification is a run-time decision made after `cmd.Run()`; `ResolveInitInvocation` and `InitNotFound` are untouched. + +- **GIVEN** the default script ran and exited 3, `isDefault=true` +- **WHEN** the helper classifies the outcome +- **THEN** it returns true (skip) + +- **GIVEN** an explicit script (`isDefault=false`) exited 3 +- **WHEN** the helper classifies the outcome +- **THEN** it returns false (hard failure — provenance rule) + +- **GIVEN** the default script exited 1 (`isDefault=true`) +- **WHEN** the helper classifies the outcome +- **THEN** it returns false (hard failure — only exit 3 skips) + +- **GIVEN** a nil error, or a non-`*exec.ExitError` error (`isDefault=true`) +- **WHEN** the helper classifies the outcome +- **THEN** it returns false (not a skip) + +#### R3: A canonical warning renderer produces the skip copy +`src/internal/worktree/init.go` SHALL expose a single renderer producing the canonical two-line skip warning, so both run sites emit byte-identical copy (anti-drift, mirroring `RenderWarning()`). The two lines MUST convey (1) that the repo is not fab-managed and the default `"fab sync"` is being skipped, and (2) the two escape hatches (set `WORKTREE_INIT_SCRIPT`, or run `fab init`). + +- **GIVEN** the skip case is classified +- **WHEN** the warning is rendered and written to stderr +- **THEN** it contains "not a fab-managed repo", "skipping init", `WORKTREE_INIT_SCRIPT`, and `fab init` + +### Init: Skip Semantics at Run Sites + +#### R4: The `wt create` runner treats the skip as an init no-op +`RunWorktreeSetup` / `RunWorktreeSetupWithObserver` (`src/internal/worktree/crud.go`) SHALL accept the `isDefault` provenance and, after `cmd.Run()` returns a non-zero error, apply the shared classification helper. On the skip case it MUST write the canonical skip warning to stderr and return `nil` (init treated as a no-op) WITHOUT printing `Worktree init complete.`. On every non-skip non-zero exit it MUST return the wrapped error exactly as today. Success and not-found paths are unchanged. + +- **GIVEN** `wt create` runs the default init in a non-fab repo where `fab sync` exits 3 +- **WHEN** the runner classifies the outcome +- **THEN** it prints the skip warning, returns nil, and does not print `Worktree init complete.` + +- **GIVEN** `wt create` runs the default init and `fab sync` exits 1 +- **WHEN** the runner classifies the outcome +- **THEN** it returns the wrapped init error (hard failure preserved) + +#### R5: `wt create` proceeds normally on the skip (no banner, no prompt, exit 0) +`src/cmd/wt/create.go` SHALL pass the `isDefault` provenance to the runner at both init call sites (the `--reuse` path and the normal path). Because the skip surfaces as a `nil` return, `initFailed` is never set, so no init-failure banner is printed, no open-anyway prompt is shown, the Open phase proceeds exactly as on init success, and the process exits 0 with the stdout final-path line unaffected. The `--reuse` path inherits the skip identically (its init step is already warn-but-continue). + +- **GIVEN** a non-fab repo where the default `fab sync` exits 3 +- **WHEN** `wt create --non-interactive` runs +- **THEN** the worktree is created, the skip warning is on stderr, no failure banner appears, and the exit code is 0 + +#### R6: `wt init` treats the skip as success (exit 0), preserving reclaim ordering +`src/cmd/wt/init.go` SHALL pass the `isDefault` provenance and apply the shared classification helper on its own `cmd.Run()` failure path, before `os.Exit(wt.ExitInitFailed)`. On the skip case it MUST reclaim the terminal foreground first (preserving the existing reclaim-before-write ordering), then write the skip warning to stderr and `return nil` (exit 0) — NOT printing `Worktree init complete.`. On every non-skip non-zero exit it MUST exit `ExitInitFailed` exactly as today. + +- **GIVEN** a non-fab repo where the default `fab sync` exits 3 +- **WHEN** `wt init` runs +- **THEN** the terminal foreground is reclaimed, the skip warning is on stderr, and the exit code is 0 + +- **GIVEN** an explicit `WORKTREE_INIT_SCRIPT` script exits 3, or the default exits any non-3 code +- **WHEN** `wt init` runs +- **THEN** it exits `ExitInitFailed = 7` (hard failure preserved) + +### Init: No-Regression for Older fab-kit + +#### R7: Older fab-kit (exit 1) degrades to today's hard-fail +The feature SHALL NOT introduce version detection or a fallback probe. An installed fab-kit predating PR #471 exits 1 in non-fab repos, which is not exit 3, so `wt` behaves exactly as today (hard fail, `ExitInitFailed = 7`, banner). The only behavior that changes is "built-in default script + exit 3". + +- **GIVEN** an older fab-kit that exits 1 in a non-fab repo, default init +- **WHEN** `wt create` runs +- **THEN** it exits 7 with the failure banner, unchanged from today + +### Docs: Spec Update + +#### R8: `docs/specs/init-protocol.md` documents the skip contract +`docs/specs/init-protocol.md` SHALL be updated per intake §6: the Lookup contract notes `InitScriptPath` reports provenance (not string equality); the Graceful skip behavior section adds case 3 (default init not applicable — `fab sync` exited `ExitNotManaged = 3`, run-time classification, warning text, exit 0; explicit same-string value never skipped; older fab-kit degrades to hard-fail); and Script failure semantics notes the exit-3 carve-out for the default script (all other non-zero exits keep `ExitInitFailed = 7`). + +- **GIVEN** a reader consulting the init-protocol spec +- **WHEN** they read the Lookup contract, Graceful skip, and Script failure sections +- **THEN** they find the provenance report, the case-3 skip, and the exit-3 carve-out accurately described + +### Design Decisions + +1. **Skip keys on fab-kit's `ExitNotManaged = 3`, not a wt-side marker probe**: `wt` runs `fab sync` as today and treats exit code 3 as "default does not apply". — *Why*: fab stays the sole authority on what is fab-managed; wt embeds no knowledge of fab's repo layout. — *Rejected*: the original wt-side `fab/project/config.yaml` walk-up (duplicated fab's detection in wt, silent-drift risk if fab moved the marker; superseded by the released exit-code contract, fab-kit PR #471). +2. **Provenance-gated skip (built-in default only)**: an explicit `WORKTREE_INIT_SCRIPT="fab sync"` still hard-fails on any non-zero exit including 3. — *Why*: the user never opted into the default; an explicit script's failure is theirs to see. — *Rejected*: string-equality gating (would silence an explicitly-configured script the user chose). +3. **One shared classification helper + warning renderer in `init.go`**: both run sites (`crud.go`, `cmd/wt/init.go`) route through it. — *Why*: the decision needs the post-run exit code and must not drift, mirroring the existing `RenderWarning()` anti-drift pattern. — *Rejected*: inlining the exit-code check at each site (drift risk). +4. **Post-run detection accepts pre-skip output noise**: the Init separator, `Running worktree init...`, and fab's own `ERROR: not in a fab-managed repo...` line print before the skip warning. — *Why*: inherent to run-time detection; more transparent than suppressing; the skip warning is the last word and the exit code is 0. + +### Non-Goals + +- Version-detecting fab-kit or probing for the config marker as a fallback (rejected — reintroduces the coupling the design revision removed). +- Changing any existing exit code or the failure banner/prompt on real failures. +- Suppressing fab's own pre-skip stderr output. + +## Tasks + +### Phase 2: Core Implementation + +- [x] T001 Change `InitScriptPath()` in `src/internal/worktree/context.go` to return `(script string, isDefault bool)` — `isDefault` true only when `WORKTREE_INIT_SCRIPT` is unset/empty. Update its doc comment. +- [x] T002 Add the shared classification helper and canonical skip-warning renderer to `src/internal/worktree/init.go` (e.g. `func DefaultNotApplicable(err error, isDefault bool) bool` using `errors.As` → `*exec.ExitError` with `ExitCode()==3`, plus `func RenderDefaultSkipWarning() string`). Import `errors`. +- [x] T003 Thread `isDefault` through `RunWorktreeSetup` and `RunWorktreeSetupWithObserver` in `src/internal/worktree/crud.go`; after `cmd.Run()` non-zero, apply the classification helper: skip case → print `RenderDefaultSkipWarning()` to stderr, return nil (no `Worktree init complete.`); otherwise return the wrapped error as today. + +### Phase 3: Integration & Edge Cases + +- [x] T004 Update both `wt create` init call sites in `src/cmd/wt/create.go` — the `--reuse` path (`RunWorktreeSetup`) and the normal path (`RunWorktreeSetupWithObserver`) — to capture and pass the `isDefault` provenance from `InitScriptPath()`. No other logic change; the skip surfaces as a nil return so `initFailed` stays false. +- [x] T005 Update `runInitScript()` in `src/cmd/wt/init.go` — capture provenance from `InitScriptPath()`, and on the `cmd.Run()` failure path apply the classification helper: skip case → reclaim terminal foreground first (preserve reclaim-before-write ordering), write `RenderDefaultSkipWarning()` to stderr, `return nil` (exit 0, no `Worktree init complete.`); otherwise exit `ExitInitFailed` as today. + +### Phase 4: Tests & Docs + +- [x] T006 [P] Update `InitScriptPath` provenance unit tests in `src/internal/worktree/context_test.go` (`TestInitScriptPath_Default` → `("fab sync", true)`; `TestInitScriptPath_Custom` → `(value, false)`; add explicit `WORKTREE_INIT_SCRIPT="fab sync"` → `("fab sync", false)`). +- [x] T007 [P] Add classification-helper table test and warning-renderer test to `src/internal/worktree/init_test.go`: (exit 3, default)→skip; (exit 3, explicit)→no-skip; (exit 1, default)→no-skip; (nil err, default)→no-skip; (non-`*exec.ExitError`, default)→no-skip; and assert the rendered warning contains the fab-managed/skip copy plus both escape hatches. +- [x] T008 Add integration tests to `src/cmd/wt/integration_test.go` using a stub `fab` in a `t.TempDir()` prepended to PATH (no host side-effects): stub exits 3 + default init → `wt create --non-interactive` exits 0 (worktree created, skip warning on stderr, no failure banner) and `wt init` exits 0; stub exits 1 → `wt create` exits 7 with banner (unchanged); explicit `WORKTREE_INIT_SCRIPT="fab sync"` + stub exits 3 → exit 7 (provenance rule). +- [x] T009 Update `docs/specs/init-protocol.md` per intake §6: Lookup contract (provenance report), Graceful skip behavior (case 3), Script failure semantics (exit-3 carve-out for the default script). + +## Execution Order + +- T001 and T002 are the foundation; T003 depends on both (needs the new signature + helper). +- T004 and T005 depend on T001–T003 (call-site threading needs the new signatures/helper). +- T006 and T007 [P] depend only on T001/T002 respectively; T008 depends on T003–T005 (exercises the built binary end-to-end). +- T009 (docs) is independent of code and may run any time after the design is fixed. + +## Acceptance + +### Functional Completeness + +- [x] A-001 R1: `InitScriptPath()` returns `(script, isDefault)` with `isDefault` true only when `WORKTREE_INIT_SCRIPT` is unset/empty. +- [x] A-002 R2: A single shared classification helper in `init.go` decides the default-not-applicable skip from `(err, isDefault)`. +- [x] A-003 R3: A single canonical renderer in `init.go` produces the two-line skip warning; both run sites use it. +- [x] A-004 R4: `RunWorktreeSetup`/`RunWorktreeSetupWithObserver` treat the skip as an init no-op (warning + nil, no `Worktree init complete.`). +- [x] A-005 R5: `wt create` passes provenance at both call sites and proceeds normally on the skip (no banner, no prompt, exit 0). +- [x] A-006 R6: `wt init` reclaims the foreground, warns, and exits 0 on the skip; hard-fails (exit 7) otherwise. +- [x] A-007 R8: `docs/specs/init-protocol.md` documents provenance, the case-3 skip, and the exit-3 carve-out. + +### Behavioral Correctness + +- [x] A-008 R1: An explicit `WORKTREE_INIT_SCRIPT="fab sync"` yields `isDefault=false` (provenance, not string equality). +- [x] A-009 R2: Only `isDefault=true` AND exit code exactly 3 classifies as skip; explicit-3, default-non-3, nil, and non-`*exec.ExitError` all classify as not-skip. +- [x] A-010 R4: The skip path does not print `Worktree init complete.`. + +### Scenario Coverage + +- [x] A-011 R5: Integration — stub `fab` exit 3 + default init → `wt create --non-interactive` exits 0, worktree created, skip warning on stderr, no failure banner. +- [x] A-012 R6: Integration — stub `fab` exit 3 + default init → `wt init` exits 0. +- [x] A-013 R6: Integration — explicit `WORKTREE_INIT_SCRIPT="fab sync"` + stub exit 3 → exit 7 (provenance rule). +- [x] A-014 R1/R2/R3: Unit tests cover `InitScriptPath` provenance, the classification helper table, and the warning renderer. + +### Edge Cases & Error Handling + +- [x] A-015 R7: Integration — stub `fab` exit 1 (older fab-kit) + default init → `wt create` exits 7 with banner, unchanged from today. +- [x] A-016 R6: `wt init` skip path reclaims the terminal foreground BEFORE writing the skip warning (reclaim-before-write ordering preserved). + +### Code Quality + +- [x] A-017 Pattern consistency: New code follows the naming and structural patterns of surrounding code (typed helpers on/near `init.go`, `fmt.Fprintln(os.Stderr, ...)` for diagnostics, RFC-style file-per-source tests). +- [x] A-018 No unnecessary duplication: The exit-3 classification and skip-warning copy live in one shared helper each (mirroring `RenderWarning()`); no re-implementation at the two run sites. +- [x] A-019 No magic numbers: The exit code `3` carries a named/commented reference to fab-kit's `ExitNotManaged`; no unexplained literal. +- [x] A-020 Test integrity: Integration tests use a stub `fab` in a `t.TempDir()` PATH with no host side-effects (per code-review.md). + +## Notes + +- Check items as you review: `- [x]` +- All acceptance items must pass before `/fab-continue` (hydrate) +- The intake references `src/cmd/integration_test.go`; the actual path is `src/cmd/wt/integration_test.go` (see Assumptions row 3). + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | `InitScriptPath` returns `(string, bool)` (two return values), not an options struct | Intake left the exact seam to apply (assumption 7); a second return value is the minimal, idiomatic Go shape and matches the sibling resolver's multi-return style; internal package, trivially reversible | S:70 R:90 A:85 D:75 | +| 2 | Certain | Runner threading via an added `isDefault bool` parameter on `RunWorktreeSetup`/`RunWorktreeSetupWithObserver`, not an options struct | Intake left the seam to apply; a bool param matches the existing positional-arg signatures (`wtPath, initScript, repoRoot`); internal API, only two callers, reversible in review | S:70 R:85 A:85 D:75 | +| 3 | Certain | Integration tests go in `src/cmd/wt/integration_test.go` (intake said `src/cmd/integration_test.go`) | The repo has no `src/cmd/integration_test.go`; the actual integration suite and `runWt`/`createTestRepo` helpers live in `src/cmd/wt/integration_test.go` — the intake path was a shorthand; verified by directory listing | S:95 R:90 A:95 D:95 | +| 4 | Confident | Helper names `DefaultNotApplicable(err, isDefault) bool` and `RenderDefaultSkipWarning() string` | Intake suggested `DefaultNotApplicable` verbatim as an example; the renderer name mirrors the existing `RenderWarning()`; both are internal, easily renamed in review | S:75 R:90 A:80 D:70 | +| 5 | Confident | Skip warning copy uses the intake's exact two lines (`Warning: not a fab-managed repo — skipping init (default "fab sync" does not apply)` + the escape-hatch line) | Intake gave the wording and said it MAY be polished at apply; adopting it verbatim keeps both required halves and follows stderr conventions; reversible | S:70 R:90 A:85 D:75 | +| 6 | Confident | Exit code `3` is referenced via a named local const / commented literal citing fab-kit `ExitNotManaged` rather than importing from fab-kit | wt has no dependency on fab-kit (separate repo, no import per context.md); a documented literal is the no-new-dependency choice consistent with Constitution I; the value is fab-kit's documented contract | S:70 R:85 A:85 D:70 | + +6 assumptions (3 certain, 3 confident, 0 tentative). diff --git a/src/cmd/wt/create.go b/src/cmd/wt/create.go index 02bed27..043a0da 100644 --- a/src/cmd/wt/create.go +++ b/src/cmd/wt/create.go @@ -185,8 +185,8 @@ or creates a new branch.`, // Run init script on reuse — ensures skills are current even in existing worktrees. // Non-fatal: reuse proceeds even if init fails (existing worktree may be functional). if worktreeInit == "true" { - initScript := wt.InitScriptPath() - if err := wt.RunWorktreeSetup(existingWtPath, initScript, ctx.RepoRoot); err != nil { + initScript, isDefault := wt.InitScriptPath() + if err := wt.RunWorktreeSetup(existingWtPath, initScript, isDefault, ctx.RepoRoot); err != nil { wt.Warn("worktree init failed for reused worktree %q: %v", finalName, err) } } @@ -276,7 +276,7 @@ or creates a new branch.`, // open must NOT downgrade the exit to 0. var initFailed bool if runInit { - initScript := wt.InitScriptPath() + initScript, isDefault := wt.InitScriptPath() // Terminal-foreground bookkeeping. wt runs the init child in // its own process group (Setpgid: true) while sharing wt's @@ -345,7 +345,7 @@ or creates a new branch.`, }() captureInit := func(c *exec.Cmd) { initCmdPtr.Store(c) } - initErr := wt.RunWorktreeSetupWithObserver(wtPath, initScript, ctx.RepoRoot, captureInit) + initErr := wt.RunWorktreeSetupWithObserver(wtPath, initScript, isDefault, ctx.RepoRoot, captureInit) // SIGINT Option B teardown — tear down the init-child signal // handler before any further TTY work (banner, open-anyway diff --git a/src/cmd/wt/init.go b/src/cmd/wt/init.go index b75e03b..754c07c 100644 --- a/src/cmd/wt/init.go +++ b/src/cmd/wt/init.go @@ -60,7 +60,7 @@ func runInitScript() error { } currentRoot := strings.TrimSpace(string(topOut)) - initScriptRel := wt.InitScriptPath() + initScriptRel, isDefault := wt.InitScriptPath() // Single resolution contract — same helper that wt create's init step uses. cmd, notFound, err := wt.ResolveInitInvocation(initScriptRel, repoRoot) @@ -113,15 +113,27 @@ func runInitScript() error { } if err := cmd.Run(); err != nil { + // Reclaim the terminal foreground BEFORE any further stderr write so it + // cannot SIGTTOU — this ordering is load-bearing for both the skip + // warning and the failure trailer below. + if reclaimTTY { + reclaimTerminalForeground(ttyFd, wtPgid) + } + // Default-not-applicable skip: the built-in "fab sync" default ran in a + // repo that is not fab-managed and exited ExitNotManaged (3). Treat it as + // success — warn on stderr and return nil (exit 0), no ExitInitFailed, no + // "Worktree init complete." trailer. An explicitly configured script + // (isDefault=false) or any other exit code falls through to the hard + // failure below. + if wt.DefaultNotApplicable(err, isDefault) { + fmt.Fprintln(os.Stderr, wt.RenderDefaultSkipWarning()) + return nil + } // Use the typed ExitInitFailed exit code so operators / shell // wrappers can distinguish "init script failed" from generic // errors — matches the contract `wt create` uses. The actual // init-script output streamed to stderr above; we add a one-line // trailer with the underlying error and exit. - if reclaimTTY { - // Reclaim before the failure trailer write so it cannot SIGTTOU. - reclaimTerminalForeground(ttyFd, wtPgid) - } fmt.Fprintf(os.Stderr, "\nInit script failed: %v\n", err) os.Exit(wt.ExitInitFailed) } diff --git a/src/cmd/wt/integration_test.go b/src/cmd/wt/integration_test.go index 700f07a..6980430 100644 --- a/src/cmd/wt/integration_test.go +++ b/src/cmd/wt/integration_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "testing" "time" @@ -265,6 +266,127 @@ func TestIntegration_CreateInitFailure_KeepsWorktreeAndExits7(t *testing.T) { assertContains(t, r.Stderr, "INIT_FAIL_MARKER") } +// stubFabEnv writes a stub `fab` executable that exits with exitCode into a +// fresh t.TempDir(), and returns env overrides that (1) prepend that dir to +// PATH so the stub shadows any real fab and (2) clear WORKTREE_INIT_SCRIPT so +// the default "fab sync" init runs. The stub emits a fab-like error line on +// stderr so the streamed-through-output behavior is exercised. All state lives +// under t.TempDir() with no host side-effects (per code-review.md). +func stubFabEnv(t *testing.T, exitCode int) []string { + t.Helper() + binDir := t.TempDir() + stub := filepath.Join(binDir, "fab") + // The stub echoes a fab-like message and exits with the requested code, + // mirroring `fab sync`'s "not in a fab-managed repo" behavior (exit 3) or an + // older fab-kit's generic failure (exit 1). + content := "#!/usr/bin/env bash\n" + + "echo 'ERROR: not in a fab-managed repo (stub)' >&2\n" + + "exit " + strconv.Itoa(exitCode) + "\n" + if err := os.WriteFile(stub, []byte(content), 0o755); err != nil { + t.Fatalf("WriteFile stub fab: %v", err) + } + return []string{ + // Prepend binDir so the stub shadows any real fab on the dev/CI host. + "PATH=" + binDir + string(os.PathListSeparator) + os.Getenv("PATH"), + // Clear the test-default no-init override so InitScriptPath falls through + // to the built-in default "fab sync" (isDefault=true). + "WORKTREE_INIT_SCRIPT=", + } +} + +// TestIntegration_CreateDefaultInitSkip_NonFabRepo_Exit3 verifies the graceful +// default-init skip: when the built-in default `fab sync` exits ExitNotManaged +// (3) in a non-fab-managed repo, `wt create` treats it as an init no-op — +// worktree kept, skip warning on stderr, NO failure banner, exit 0. +func TestIntegration_CreateDefaultInitSkip_NonFabRepo_Exit3(t *testing.T) { + repo := createTestRepo(t) + env := stubFabEnv(t, 3) + + r := runWt(t, repo, env, "create", "--non-interactive", + "--worktree-name", "skip-default", + "--worktree-open", "skip") + + // Exit 0 — the default does not apply, so it is not a failure. + assertExitCode(t, r, 0) + + // Worktree was created and registered. + assertWorktreeExists(t, repo, "skip-default") + + // The canonical skip warning is on stderr. + assertContains(t, r.Stderr, "not a fab-managed repo") + assertContains(t, r.Stderr, "WORKTREE_INIT_SCRIPT") + + // No init-failure banner / hard-failure artifacts. + assertNotContains(t, r.Stderr, "Worktree init complete.") + assertNotContains(t, r.Stderr, "wt delete") + assertNotContains(t, r.Stderr, "init script exited with status") + + // stdout carries the final worktree path (unaffected by the skip). + if strings.TrimSpace(r.Stdout) != worktreePath(repo, "skip-default") { + t.Errorf("expected stdout final path %q, got %q", + worktreePath(repo, "skip-default"), r.Stdout) + } +} + +// TestIntegration_InitDefaultSkip_NonFabRepo_Exit3 verifies `wt init` exits 0 +// (skip warning, no ExitInitFailed) when the default `fab sync` exits 3. +func TestIntegration_InitDefaultSkip_NonFabRepo_Exit3(t *testing.T) { + repo := createTestRepo(t) + env := stubFabEnv(t, 3) + + r := runWt(t, repo, env, "init") + + assertExitCode(t, r, 0) + assertContains(t, r.Stderr, "not a fab-managed repo") + assertNotContains(t, r.Stderr, "Worktree init complete.") + assertNotContains(t, r.Stderr, "Init script failed") +} + +// TestIntegration_CreateDefaultInit_OldFabKit_Exit1_HardFails verifies the +// no-regression posture: an older fab-kit exits 1 (not 3), so the default init +// hard-fails exactly as today — exit 7 with the failure banner. +func TestIntegration_CreateDefaultInit_OldFabKit_Exit1_HardFails(t *testing.T) { + repo := createTestRepo(t) + env := stubFabEnv(t, 1) + + r := runWt(t, repo, env, "create", "--non-interactive", + "--worktree-name", "oldfab", + "--worktree-open", "skip") + + // Exit 7 (ExitInitFailed), unchanged from today. + assertExitCode(t, r, 7) + + // Worktree is kept, banner is shown. + assertWorktreeExists(t, repo, "oldfab") + assertContains(t, r.Stderr, worktreePath(repo, "oldfab")) + assertContains(t, r.Stderr, "wt delete") + + // The graceful-skip path must NOT engage for a non-3 exit. + assertNotContains(t, r.Stderr, "not a fab-managed repo — skipping init") +} + +// TestIntegration_CreateExplicitFabSync_Exit3_HardFails verifies the provenance +// rule: an EXPLICIT WORKTREE_INIT_SCRIPT="fab sync" still hard-fails on exit 3 +// (the user opted into the script), even though the string matches the default. +func TestIntegration_CreateExplicitFabSync_Exit3_HardFails(t *testing.T) { + repo := createTestRepo(t) + // Start from the exit-3 stub, then override WORKTREE_INIT_SCRIPT to an + // explicit "fab sync" (last-wins) — the stub still shadows fab on PATH. + env := append(stubFabEnv(t, 3), "WORKTREE_INIT_SCRIPT=fab sync") + + r := runWt(t, repo, env, "create", "--non-interactive", + "--worktree-name", "explicit-fabsync", + "--worktree-open", "skip") + + // Exit 7 — explicit script provenance means the failure is the user's to see. + assertExitCode(t, r, 7) + assertWorktreeExists(t, repo, "explicit-fabsync") + assertContains(t, r.Stderr, "wt delete") + + // The graceful-skip path must NOT engage for an explicitly-configured script. + assertNotContains(t, r.Stderr, "not a fab-managed repo — skipping init") +} + // TestIntegration_Go_FromSiblingWorktree exercises the core gap this change // closes: navigating to a sibling worktree from inside another worktree. // `wt go ` run from inside worktree A must resolve sibling B and write diff --git a/src/internal/worktree/context.go b/src/internal/worktree/context.go index 549e05a..0b38002 100644 --- a/src/internal/worktree/context.go +++ b/src/internal/worktree/context.go @@ -172,10 +172,19 @@ func CurrentBranch() (string, error) { return strings.TrimSpace(string(out)), nil } -// InitScriptPath returns the path to the init script, respecting WORKTREE_INIT_SCRIPT env var. -func InitScriptPath() string { +// InitScriptPath returns the init-script value plus its provenance, respecting +// the WORKTREE_INIT_SCRIPT env var. +// +// isDefault is true ONLY when WORKTREE_INIT_SCRIPT is unset/empty and the +// built-in "fab sync" default is used. It is provenance, NOT string equality: +// an explicit WORKTREE_INIT_SCRIPT="fab sync" returns ("fab sync", false), +// because the user opted into that script. The run-time skip classification +// (see DefaultNotApplicable in init.go) keys on this flag so an explicitly +// configured script always fails hard while the built-in default may skip +// gracefully in a non-fab-managed repo. +func InitScriptPath() (script string, isDefault bool) { if v := os.Getenv("WORKTREE_INIT_SCRIPT"); v != "" { - return v + return v, false } - return "fab sync" + return "fab sync", true } diff --git a/src/internal/worktree/context_test.go b/src/internal/worktree/context_test.go index 38542f7..0e85dbd 100644 --- a/src/internal/worktree/context_test.go +++ b/src/internal/worktree/context_test.go @@ -68,18 +68,39 @@ func TestDeriveWorktreeName(t *testing.T) { } func TestInitScriptPath_Default(t *testing.T) { - // When WORKTREE_INIT_SCRIPT is not set, should return default + // When WORKTREE_INIT_SCRIPT is unset/empty, returns the built-in default + // with isDefault=true (provenance). t.Setenv("WORKTREE_INIT_SCRIPT", "") - got := InitScriptPath() + got, isDefault := InitScriptPath() if got != "fab sync" { - t.Errorf("InitScriptPath() = %q, want %q", got, "fab sync") + t.Errorf("InitScriptPath() script = %q, want %q", got, "fab sync") + } + if !isDefault { + t.Errorf("InitScriptPath() isDefault = false, want true (unset env)") } } func TestInitScriptPath_Custom(t *testing.T) { t.Setenv("WORKTREE_INIT_SCRIPT", "custom/init.sh") - got := InitScriptPath() + got, isDefault := InitScriptPath() if got != "custom/init.sh" { - t.Errorf("InitScriptPath() = %q, want %q", got, "custom/init.sh") + t.Errorf("InitScriptPath() script = %q, want %q", got, "custom/init.sh") + } + if isDefault { + t.Errorf("InitScriptPath() isDefault = true, want false (explicit env)") + } +} + +// TestInitScriptPath_ExplicitFabSync verifies provenance keys on env-var +// presence, NOT string equality: an explicit WORKTREE_INIT_SCRIPT="fab sync" +// yields isDefault=false even though the value matches the built-in default. +func TestInitScriptPath_ExplicitFabSync(t *testing.T) { + t.Setenv("WORKTREE_INIT_SCRIPT", "fab sync") + got, isDefault := InitScriptPath() + if got != "fab sync" { + t.Errorf("InitScriptPath() script = %q, want %q", got, "fab sync") + } + if isDefault { + t.Errorf("InitScriptPath() isDefault = true, want false (explicit \"fab sync\" is not the built-in default)") } } diff --git a/src/internal/worktree/crud.go b/src/internal/worktree/crud.go index 17bc704..1e25ff3 100644 --- a/src/internal/worktree/crud.go +++ b/src/internal/worktree/crud.go @@ -123,16 +123,21 @@ func CreateExploratoryWorktree(name string, ctx *RepoContext, rb *Rollback, star // runs it in the worktree directory. // - On structured not-found, prints the unified warning to stderr and // returns nil (init step is treated as a no-op). -// - Returns the exec error verbatim on init-script non-zero exit so callers -// can extract *exec.ExitError via errors.As. +// - On the default-not-applicable skip (isDefault AND exit 3, per +// DefaultNotApplicable), prints the skip warning to stderr and returns nil. +// - Wraps the exec error (via %w) on any other init-script non-zero exit so +// callers can still extract *exec.ExitError via errors.As. +// +// isDefault carries InitScriptPath's provenance (true only for the built-in +// "fab sync" default) so an explicitly configured script always fails hard. // // Callers that want to confirm with the user before running MUST call // ConfirmYesNo themselves first — confirmation is no longer part of the // runner so wt create's SIGINT-during-init handler can be installed AFTER // the prompt completes (installing it before would consume Ctrl-C during // the prompt with no init child to target). -func RunWorktreeSetup(wtPath, initScript, repoRoot string) error { - return RunWorktreeSetupWithObserver(wtPath, initScript, repoRoot, nil) +func RunWorktreeSetup(wtPath, initScript string, isDefault bool, repoRoot string) error { + return RunWorktreeSetupWithObserver(wtPath, initScript, isDefault, repoRoot, nil) } // RunWorktreeSetupWithObserver is like RunWorktreeSetup but invokes observer @@ -140,7 +145,7 @@ func RunWorktreeSetup(wtPath, initScript, repoRoot string) error { // lets wt create's SIGINT-during-init handler capture a reference to the // in-flight init child without growing the public API surface. Pass nil to // behave identically to RunWorktreeSetup. -func RunWorktreeSetupWithObserver(wtPath, initScript, repoRoot string, observer func(cmd *exec.Cmd)) error { +func RunWorktreeSetupWithObserver(wtPath, initScript string, isDefault bool, repoRoot string, observer func(cmd *exec.Cmd)) error { cmd, notFound, err := ResolveInitInvocation(initScript, repoRoot) if err != nil { return err @@ -164,6 +169,16 @@ func RunWorktreeSetupWithObserver(wtPath, initScript, repoRoot string, observer observer(cmd) } if err := cmd.Run(); err != nil { + // Default-not-applicable skip: the built-in "fab sync" default ran in a + // repo that is not fab-managed and exited ExitNotManaged (3). Treat this + // as an init no-op — warn on stderr and return nil so wt create proceeds + // to the Open phase exactly as on success (no banner, no prompt, exit 0). + // An explicitly configured script (isDefault=false) never reaches this + // branch; it falls through to the hard-failure return below. + if DefaultNotApplicable(err, isDefault) { + fmt.Fprintln(os.Stderr, RenderDefaultSkipWarning()) + return nil + } return fmt.Errorf("init script failed: %w", err) } fmt.Fprintln(os.Stderr, "Worktree init complete.") diff --git a/src/internal/worktree/init.go b/src/internal/worktree/init.go index f3a5521..e6cf9cf 100644 --- a/src/internal/worktree/init.go +++ b/src/internal/worktree/init.go @@ -1,6 +1,7 @@ package worktree import ( + "errors" "fmt" "os" "os/exec" @@ -31,6 +32,62 @@ const ( hintFileNotFound = "Create the file or set WORKTREE_INIT_SCRIPT to a custom script." ) +// exitNotManaged is fab-kit's ExitNotManaged code: `fab sync` exits 3 (via a +// config walk-up before any git resolution, symmetric with +// `fab-kit migrations-status`) when it is run outside a fab-managed repo +// (fab-kit >= PR #471). wt embeds the numeric value rather than importing from +// fab-kit — the two are separate repos with no import relationship (see +// docs/memory/wt-cli and the Single-Binary constitution principle). Older +// fab-kit predating PR #471 exits 1, which is not 3, so it degrades to today's +// hard-fail with no version detection. +const exitNotManaged = 3 + +// Skip-warning copy for the default-not-applicable case. File-scoped so the two +// run sites (crud.go's RunWorktreeSetupWithObserver and cmd/wt/init.go) share +// the canonical phrasing without duplicating literals — the same anti-drift +// discipline InitNotFound.RenderWarning applies to the not-found copy. +const ( + skipDefaultLine1 = `Warning: not a fab-managed repo — skipping init (default "fab sync" does not apply)` + skipDefaultLine2 = "Set WORKTREE_INIT_SCRIPT to a custom script, or run 'fab init' to make this repo fab-managed." +) + +// DefaultNotApplicable reports whether an init-script non-zero exit is the +// "built-in default does not apply" skip case rather than a real init failure. +// +// It returns true ONLY when BOTH hold: +// - isDefault is true (the init script is the built-in "fab sync" default, +// not an explicit WORKTREE_INIT_SCRIPT — provenance, not string equality), +// and +// - err unwraps (via errors.As) to an *exec.ExitError whose exit code is +// exactly exitNotManaged (3, fab-kit's ExitNotManaged for a non-fab repo). +// +// It returns false for a nil err, any exit code other than 3, an err that does +// not unwrap to *exec.ExitError, and any case where isDefault is false +// (regardless of exit code — an explicitly configured script always fails +// hard). Callers that get true render RenderDefaultSkipWarning and treat the +// init step as a no-op; all other outcomes stay hard failures exactly as today. +// +// The decision needs the post-run exit code, so it is a run-time +// classification: ResolveInitInvocation and InitNotFound are untouched. +func DefaultNotApplicable(err error, isDefault bool) bool { + if !isDefault || err == nil { + return false + } + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + return false + } + return exitErr.ExitCode() == exitNotManaged +} + +// RenderDefaultSkipWarning returns the canonical two-line warning printed when +// the built-in default init is skipped because the repo is not fab-managed +// (see DefaultNotApplicable). It is the single renderer for this copy so both +// run sites stay byte-identical, mirroring InitNotFound.RenderWarning. +func RenderDefaultSkipWarning() string { + return skipDefaultLine1 + "\n" + skipDefaultLine2 +} + // InitNotFound is the structured "not found" outcome returned by // ResolveInitInvocation. Callers render a verbose warning via RenderWarning // and continue without running an init step (the not-found case is non-fatal diff --git a/src/internal/worktree/init_test.go b/src/internal/worktree/init_test.go index 551f87a..8ddd792 100644 --- a/src/internal/worktree/init_test.go +++ b/src/internal/worktree/init_test.go @@ -3,9 +3,11 @@ package worktree import ( "crypto/rand" "encoding/hex" + "errors" "os" "os/exec" "path/filepath" + "strconv" "strings" "testing" ) @@ -120,6 +122,80 @@ func TestResolveInitInvocation_FileMissing(t *testing.T) { } } +// exitErrorWithCode runs a trivial command that exits with the given code and +// returns the resulting error, which unwraps to an *exec.ExitError carrying +// that code. This produces a real exit error (as cmd.Run does at the run sites) +// rather than a hand-forged one, so DefaultNotApplicable is exercised through +// the same errors.As path it uses in production. +func exitErrorWithCode(t *testing.T, code int) error { + t.Helper() + err := exec.Command("bash", "-c", "exit "+strconv.Itoa(code)).Run() + if err == nil { + t.Fatalf("expected non-nil error for exit %d", code) + } + // Confirm the fixture actually carries the code we asked for. + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *exec.ExitError, got %T", err) + } + if exitErr.ExitCode() != code { + t.Fatalf("fixture exit code = %d, want %d", exitErr.ExitCode(), code) + } + return err +} + +func TestDefaultNotApplicable(t *testing.T) { + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("`bash` not on PATH; skipping") + } + exit3 := exitErrorWithCode(t, 3) + exit1 := exitErrorWithCode(t, 1) + + tests := []struct { + name string + err error + isDefault bool + want bool + }{ + {"default + exit 3 → skip", exit3, true, true}, + {"explicit + exit 3 → no skip (provenance)", exit3, false, false}, + {"default + exit 1 → no skip (only 3)", exit1, true, false}, + {"default + nil error → no skip", nil, true, false}, + {"default + non-ExitError → no skip", errors.New("boom"), true, false}, + {"explicit + exit 1 → no skip", exit1, false, false}, + {"explicit + nil error → no skip", nil, false, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := DefaultNotApplicable(tc.err, tc.isDefault); got != tc.want { + t.Errorf("DefaultNotApplicable(%v, %v) = %v, want %v", + tc.err, tc.isDefault, got, tc.want) + } + }) + } +} + +func TestRenderDefaultSkipWarning(t *testing.T) { + out := RenderDefaultSkipWarning() + if out == "" { + t.Fatal("expected non-empty warning") + } + for _, want := range []string{ + "not a fab-managed repo", + "skipping init", + "WORKTREE_INIT_SCRIPT", + "fab init", + } { + if !strings.Contains(out, want) { + t.Errorf("expected skip warning to contain %q, got:\n%s", want, out) + } + } + // Two lines: the skip statement and the escape-hatch hint. + if got := strings.Count(out, "\n"); got != 1 { + t.Errorf("expected exactly one newline (two lines), got %d:\n%s", got, out) + } +} + func TestInitNotFound_RenderWarning_CommandNotOnPath(t *testing.T) { n := InitNotFound{Kind: CommandNotOnPath, Name: "fab"} out := n.RenderWarning()