diff --git a/docs/memory/wt-cli/create-output-phases.md b/docs/memory/wt-cli/create-output-phases.md index aa8d8fd..9910c80 100644 --- a/docs/memory/wt-cli/create-output-phases.md +++ b/docs/memory/wt-cli/create-output-phases.md @@ -6,7 +6,8 @@ description: "Phase-separator output contract for `wt create` / `wt init` — Gi > Post-implementation behavior capture for the `wt create` / `wt init` phase-separator output. > Source change: `260531-pnmi-add-phase-separators` -> (single `wt.Warn` warning emitter + `wt delete` stdout→stderr warning realignment added by `260622-log5-dx-copy-polish`). +> (single `wt.Warn` warning emitter + `wt delete` stdout→stderr warning realignment added by `260622-log5-dx-copy-polish`; +> Open phase now reachable after init failure on the open-anyway *Yes* path, and the stdout path line suppressed on init-failure paths, by `260626-n6ma-create-init-failure-open-anyway`). This file documents the phase-separator output contract that `wt create` and `wt init` honor, and — as the canonical stdout/stderr stream-discipline file — the single `wt.Warn` warning emitter that all `Warning:` call sites route through. Future changes touching `src/cmd/wt/create.go`, `src/cmd/wt/init.go`, `src/cmd/wt/delete.go`, or `src/internal/worktree/{crud.go,errors.go}` should preserve these invariants unless an explicit spec amendment supersedes them. @@ -32,7 +33,7 @@ This file documents the phase-separator output contract that `wt create` and `wt - A separator is emitted **only when its phase produces output** — a separator never precedes a phase that emits nothing: - **Git** is emitted on every successful create, because the deferred summary block always prints (it is the Git phase's output). - **Init** is emitted only when the init phase actually runs an init command — NOT when init is disabled and NOT on the `*InitNotFound` path. - - **Open** is emitted only when the open phase runs, gated on `worktreeOpen != "skip"`. `--worktree-open=skip` (and `--non-interactive`, which defaults open to `skip`) suppress it. + - **Open** is emitted only when the open phase runs, gated on `worktreeOpen != "skip"`. `--worktree-open=skip` (and `--non-interactive`, which defaults open to `skip`) suppress it. As of `260626-n6ma`, the Open phase is **also reachable after an init failure** on the interactive open-anyway *Yes* path — previously the Open separator + menu were reached only on init **success**. On the open-anyway *No* path the failure branch sets `worktreeOpen = "skip"`, so the separator + menu are suppressed there. The stdout discipline is unaffected (see below): on any init-failure path the function exits `ExitInitFailed = 7` via the end-of-function `initFailed` guard, which sits **before** the stdout path-line print, so the path line is NOT emitted on an init-failure path (even after a successful open-anyway open). - The Git separator MUST stay inside the existing deferred-summary emission, **before** the init-phase `signal.Reset(syscall.SIGINT, syscall.SIGTERM)` call. It MUST NOT introduce new I/O or a prompt inside the tight reinstall window between `git worktree add` returning and that `signal.Reset` (see `init-failure-contract.md` "SIGINT during init"). - Existing summary/warning/prompt substrings are preserved verbatim — in particular `Created worktree:` and `Open in:`. @@ -48,7 +49,8 @@ This file documents the phase-separator output contract that `wt create` and `wt ### STDOUT discipline: separators are stderr-only - All phase separators are written to **stderr**. None ever appears on stdout. -- `wt create`'s stdout remains **solely** the final worktree-path line (`fmt.Println(wtPath)`), byte-identical to before this change. This preserves the launcher-contract guarantee that stdout is the machine-readable result. No separator, summary, or init output leaks to stdout. +- `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. @@ -89,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, `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/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). - 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). `/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. +- 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/index.md b/docs/memory/wt-cli/index.md index 7f8c7e1..4d3a442 100644 --- a/docs/memory/wt-cli/index.md +++ b/docs/memory/wt-cli/index.md @@ -11,7 +11,7 @@ description: "Behavior contracts for the `wt` CLI binary — commands, exit code | [go-command-contract](go-command-contract.md) | `wt go` worktree-selection contract — selection-only navigation via `WT_CD_FILE`/stdout (no launch), exit codes, the current-worktree-included menu, and the `wt open --go` composition. | | [help-dump-contract](help-dump-contract.md) | Contract for the hidden `wt help-dump` command — the JSON envelope shll.ai's scheduled puller consumes. | | [idle-staleness-contract](idle-staleness-contract.md) | The shared idle predicate, the `wt delete --stale` selector, and the safety invariant that idleness never gates a deletion on its own. | -| [init-failure-contract](init-failure-contract.md) | Init-failure behavior of `wt create` / `wt init` — kept-worktree contract, `ExitInitFailed`, SIGINT handling, and terminal-foreground reclaim. | +| [init-failure-contract](init-failure-contract.md) | Init-failure behavior of `wt create` / `wt init` — kept-worktree contract, `ExitInitFailed` on every path, the interactive open-anyway prompt, the `wt go` banner hint, SIGINT handling, and terminal-foreground reclaim. | | [list-status-contract](list-status-contract.md) | `wt list` output contract — enrichment-free default, `--status` opt-in dashboard, `--sort` ordering, and pointer-field JSON shape. | | [menu-navigation-contract](menu-navigation-contract.md) | Arrow-key navigation contract for the shared `ShowMenu` — TTY gating, keybindings, and a byte-identical non-TTY fallback. | | [recency-ordering-contract](recency-ordering-contract.md) | The single recency definition (`RecencyOf`/`RecencyLess`/`SortByRecency`) and newest-first ordering across `wt list`, `wt open`, and `wt delete`. | diff --git a/docs/memory/wt-cli/init-failure-contract.md b/docs/memory/wt-cli/init-failure-contract.md index a7b9d00..7461ba0 100644 --- a/docs/memory/wt-cli/init-failure-contract.md +++ b/docs/memory/wt-cli/init-failure-contract.md @@ -1,11 +1,12 @@ --- type: memory -description: "Init-failure behavior of `wt create` / `wt init` — kept-worktree contract, `ExitInitFailed`, SIGINT handling, and terminal-foreground reclaim." +description: "Init-failure behavior of `wt create` / `wt init` — kept-worktree contract, `ExitInitFailed` on every path, the interactive open-anyway prompt, the `wt go` banner hint, SIGINT handling, and terminal-foreground reclaim." --- # wt-cli: Init Failure Contract > Post-implementation behavior capture for the `wt create` init-failure DX overhaul. -> Source change: `260516-g5e7-wt-create-init-failure-dx`. +> Source change: `260516-g5e7-wt-create-init-failure-dx` +> (interactive open-anyway prompt + `wt go` banner hint + unified post-init reclaim/teardown + flag-based exit added by `260626-n6ma-create-init-failure-open-anyway`). This file documents the contract that `wt create` and `wt init` honor when the init script is missing or fails. Future changes touching `cmd/wt/create.go`, `cmd/wt/init.go`, or `src/internal/worktree/{init.go,errors.go,crud.go,apps.go}` should preserve these invariants unless an explicit spec amendment supersedes them. @@ -34,12 +35,20 @@ This file documents the contract that `wt create` and `wt init` honor when the i - The `defer rb.Execute()` still fires on **any other** failure between worktree creation and the init step — only init-script non-zero exit is exempt from rollback. - Not-found (handled above) is distinct from non-zero exit. Only "resolver succeeded, process executed, exited non-zero" triggers this requirement. -### Init failure exits with `ExitInitFailed = 7` +### Init failure exits with `ExitInitFailed = 7` on EVERY path - `ExitInitFailed = 7` is declared in `src/internal/worktree/errors.go`, appended after `ExitTmuxWindowError = 6`. Existing exit codes (0–6) MUST NOT be renumbered. -- `cmd/wt/create.go` exits via `os.Exit(wt.ExitInitFailed)` on init-script non-zero exit — NOT via `wt.ExitWithError(...)` with a generic code. -- `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. -- Operators (shell wrappers, fab-kit, `hop`) can distinguish "worktree exists, init didn't complete" from any other generic failure. +- **`cmd/wt/create.go` exits 7 on ALL init-failure paths** — non-interactive, interactive open-anyway *Yes* (even when the open succeeds), and interactive *No*. As of `260626-n6ma`, exit 7 is NOT issued inline at the banner. Instead an `initFailed bool` flag is set in the init-failure branch (`create.go:376`), and a single end-of-function guard `if initFailed { os.Exit(wt.ExitInitFailed) }` (`create.go:476–478`) fires after the Open phase. That guard is placed **before** `fmt.Println(wtPath)` / `return nil`, so a *successful* open-anyway open can never downgrade the exit to 0 and erase the init-failure signal. The lone inline `os.Exit(wt.ExitInitFailed)` that remains is the non-interactive branch (`create.go:400`), reached only when no prompt is offered. +- `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. + +### 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). +- **Interactive**: after the banner, prompt `wt.ConfirmYesNo("Continue and open the worktree anyway?")` (`ConfirmYesNo` in `src/internal/worktree/menu.go`). + - **Yes** → fall through into the EXISTING Open phase (no new open codepath) so the user can open the kept worktree. The function still exits 7 at the end via the `initFailed` guard. + - **No** → set `worktreeOpen = "skip"` so the Open separator + app menu are suppressed; the banner's `Go:` line already shows how to reach the kept worktree. Exit 7 at the end. +- **Non-interactive / piped / CI**: the prior behavior is preserved exactly — banner (now including the `Go:` line) + inline `os.Exit(wt.ExitInitFailed)`, **NO prompt** (`create.go:398–401`). ### `--reuse` is exempt from the new failure contract @@ -49,7 +58,9 @@ This file documents the contract that `wt create` and `wt init` honor when the i ### Init-failure banner - `internal/worktree/errors.go` exposes `PrintInitFailureBanner(wtPath, name string, err error)`. -- Banner contents, in order: status line (with `*exec.ExitError` numeric code when available, generic phrasing otherwise) + reminder that init output streamed above; kept-worktree marker with absolute `wtPath`; retry hint `cd '' && wt init` (using `&&` so it parses in bash/zsh/fish); remove hint `wt delete ''` (using the worktree name, not the absolute path, so it composes with `wt delete`'s name resolution). Path and name are both single-quoted via `shellQuoteSingle` so paths with spaces / shell metacharacters stay copy-paste-safe. +- Banner contents, in order: status line (with `*exec.ExitError` numeric code when available, generic phrasing otherwise) + reminder that init output streamed above; kept-worktree marker with absolute `wtPath`; **go hint `wt go ''`** (`260626-n6ma`); retry hint `cd '' && wt init` (using `&&` so it parses in bash/zsh/fish); remove hint `wt delete ''` (using the worktree name, not the absolute path, so it composes with `wt delete`'s name resolution). Path and name are all single-quoted via `shellQuoteSingle` so paths with spaces / shell metacharacters stay copy-paste-safe. +- The **`Go:` hint** (`260626-n6ma`) points the user at the selection-free navigation command (`wt go`) for the kept worktree. It is grouped with the other action hints, placed **after** the `Worktree:` line and **before** `Retry:` (banner order: status; `Worktree:`; `Go:`; `Retry:`; `Remove:`). It lives in the shared banner helper, so EVERY caller path gains the same discoverability — interactive Yes, interactive No, AND non-interactive/CI. +- Labels are named constants in one `const (...)` block — `bannerLabelWorktree` / `bannerLabelGo` (`"Go: "`) / `bannerLabelRetry` / `bannerLabelRemove`, column-aligned — so the canonical wording lives in one place (no duplication; `bannerLabelGo` was added alongside its siblings by `260626-n6ma`). - Uses existing `ColorRed` / `ColorBold` / `ColorReset` helpers. Labels are confined to this one helper — no duplication elsewhere. ### SIGINT during init: Option B (handler swap + Setpgid) @@ -58,7 +69,7 @@ This file documents the contract that `wt create` and `wt init` honor when the i 1. Calls `signal.Reset(syscall.SIGINT, syscall.SIGTERM)` to drop the rollback-based handler. 2. Installs a new handler that signals the init `*exec.Cmd`'s process group. - The init `*exec.Cmd` is constructed with `SysProcAttr.Setpgid = true` so the script's own children also receive the signal. -- After the child exits, control falls through to the natural init-failure path: banner + `ExitInitFailed`. The worktree is kept. +- After the child exits, control falls through to the unified post-init teardown/reclaim block (below), then the init-failure path: banner + (interactive) open-anyway prompt or (non-interactive) `os.Exit(ExitInitFailed)`. Either way the worktree is kept and the process exits 7 (the end-of-function `initFailed` guard handles the interactive paths). The init-child signal handler is torn down (`signal.Stop`/`close(initSigCh)`) before any of this, on every init outcome (`260626-n6ma`). - Before worktree creation succeeds, the original SIGINT semantics apply: rollback executes and the process exits 130. - The reinstallation window (between `git worktree add` returning and the new handler being installed) MUST stay tight — no I/O, prompts, or non-trivial work. If a panic occurs here, the original `defer rb.Execute()` still fires (losing the worktree is the correct fallback when the handler is in an inconsistent state). - `RunWorktreeSetupWithObserver` is the variant exposed for SIGINT integration; `RunWorktreeSetup` is the thin wrapper used elsewhere. @@ -69,15 +80,14 @@ This file documents the contract that `wt create` and `wt init` honor when the i - `wt create` AND `wt init` capture the controlling terminal's foreground process group (`tcgetpgrp` on `os.Stdin.Fd()`) **immediately before** running the init child, and reclaim it (`tcsetpgrp` back to that captured pgrp — `wt`'s own group, since `wt` is the foreground at capture time) **after** the child returns, on ALL exit paths: - **`wt create`** (`cmd/wt/create.go`): - - Capture is adjacent to the SIGINT-Option-B `signal.Reset` (`create.go:292–304`), gated on `term.IsTerminal(os.Stdin.Fd())`. If `tcgetpgrp` itself errors on a TTY, the bookkeeping is disabled (no reclaim to a bogus pgrp) rather than guessed. - - **Success path**: explicit reclaim after `signal.Stop`/`close`, **before** the Open phase separator + menu render (`create.go:361–363`) — the load-bearing reclaim that prevents the Open-phase TTY write from SIGTTOU-suspending `wt`. - - **Init-failure path**: explicit reclaim **before** `PrintInitFailureBanner` + `os.Exit(ExitInitFailed)` (`create.go:349–351`) — the banner is itself a TTY write and would SIGTTOU if foreground were lost. - - **Panic / non-`os.Exit` early-return**: a best-effort `defer reclaimTerminalForeground(...)` installed immediately after capture (`create.go:314–316`). Because both load-bearing paths exit via `os.Exit` (which skips deferred funcs) or fall through to Open, this defer only actually fires on a panic or a non-`os.Exit` return. (The pre-init SIGINT exit-130 path runs before this defer is installed and before any foreground was captured, so it has nothing to reclaim.) - - **`wt init`** (`cmd/wt/init.go::runInitScript`): same capture before `cmd.Run()` (`init.go:96–113`), explicit reclaim before the failure trailer + `os.Exit(ExitInitFailed)` (`init.go:121–124`) and before the `Worktree init complete.` trailer on success (`init.go:129–132`), plus the same best-effort `defer` safety net. A standalone interactive `wt init` (no Open phase) would otherwise strand the user's shell at a suspended prompt after returning. + - Capture is adjacent to the SIGINT-Option-B `signal.Reset` (`create.go:298–310`), gated on `term.IsTerminal(os.Stdin.Fd())`. If `tcgetpgrp` itself errors on a TTY, the bookkeeping is disabled (no reclaim to a bogus pgrp) rather than guessed. + - **Unified post-init reclaim/teardown** (`260626-n6ma`): as of the open-anyway change, the previously-duplicated teardown + reclaim now run **ONCE, unconditionally**, in a single post-init block immediately after `RunWorktreeSetupWithObserver` returns — `signal.Stop(initSigCh)` + `close(initSigCh)` (`create.go:355–356`) then `reclaimTerminalForeground(ttyFd, wtPgid)` (gated on `reclaimTTY`, `create.go:363–365`) — **before** the init-failure banner, the open-anyway prompt, AND the Open phase separator + menu render. This single reclaim is the load-bearing one for every downstream TTY write: it covers the success-path Open menu, the init-failure banner (itself a TTY write), the open-anyway prompt, and the Open menu reached via the open-anyway *Yes* fall-through. It replaced the prior two separate reclaim sites (a success-path reclaim before Open and an init-failure-path reclaim before the banner), removing a double `close(initSigCh)` and a duplicate reclaim. + - **Panic / non-`os.Exit` early-return**: a best-effort `defer reclaimTerminalForeground(...)` installed immediately after capture (`create.go:322–324`). The unconditional post-init reclaim above is the load-bearing one; because every init-failure path either `os.Exit`s (non-interactive) or falls through to Open, this defer only actually fires on a panic or a non-`os.Exit` return. (The pre-init SIGINT exit-130 path runs before this defer is installed and before any foreground was captured, so it has nothing to reclaim.) + - **`wt init`** (`cmd/wt/init.go::runInitScript`): same capture before `cmd.Run()` (`init.go:96–113`), explicit reclaim before the failure trailer + `os.Exit(ExitInitFailed)` (`init.go:121–124`) and before the `Worktree init complete.` trailer on success (`init.go:129–132`), plus the same best-effort `defer` safety net. A standalone interactive `wt init` (no Open phase, no open-anyway prompt) would otherwise strand the user's shell at a suspended prompt after returning. (`wt init` was unchanged by `260626-n6ma`.) - The reclaim wraps the `tcsetpgrp` ioctl in `signal.Ignore(syscall.SIGTTOU)` with `defer signal.Reset(syscall.SIGTTOU)` (`tty_unix.go`). Without this, a `tcsetpgrp` issued from a possibly-background process is itself a foreground-mutating operation the kernel answers with SIGTTOU — the reclaim that is supposed to un-stick `wt` would stick it instead. `signal.Reset` is the correct restore because `wt` installs no other SIGTTOU handler. - All bookkeeping is **gated on `term.IsTerminal(os.Stdin.Fd())`** — when stdin is not a TTY (`--non-interactive`, piped, CI), capture and reclaim are complete no-ops: no syscall, no error, no warning. This mirrors the existing `isInteractiveTTY()` gate in `menu.go` and Constitution VI. - Reclaim errors are intentionally ignored (`_ = unix.IoctlSetPointerInt(...)`): the bookkeeping is best-effort cleanup that MUST never block `rb.Execute()` rollback or alter exit codes. -- **Preserved SIGINT Option B invariants**: `SysProcAttr{Setpgid: true}` on the init child is unchanged (the reclaim never touches the child's pgrp); the tight reinstall window is NOT widened (`tcgetpgrp` is a single cheap syscall with no I/O or prompt, placed adjacent to `signal.Reset`); `defer rb.Execute()` remains the panic fallback and the foreground-restore defer is independent of it. +- **Preserved SIGINT Option B invariants**: `SysProcAttr{Setpgid: true}` on the init child is unchanged (the reclaim never touches the child's pgrp); the tight reinstall window (worktree-add → `signal.Reset`) is NOT widened (`tcgetpgrp` is a single cheap syscall with no I/O or prompt, placed adjacent to `signal.Reset`); `defer rb.Execute()` remains the panic fallback and the foreground-restore defer is independent of it. **These invariants hold on the open-anyway fall-through path too** (`260626-n6ma`): the SIGINT teardown (`signal.Stop`/`close(initSigCh)`) and the foreground reclaim both run in the unified post-init block — once, before the banner, the open-anyway prompt, and the Open menu — so the *Yes* fall-through reaches the Open menu with the init handler torn down and foreground already reclaimed (no SIGTTOU), and `rb.Disarm()` (not `rb.Execute()`) still keeps the worktree on the new path. The end-of-function `if initFailed { os.Exit(ExitInitFailed) }` is the single exit point for the open-anyway path. - Helpers `terminalForeground(ttyFd int) (int, error)` (tcgetpgrp via `unix.IoctlGetInt(fd, unix.TIOCGPGRP)`) and `reclaimTerminalForeground(ttyFd int, pgrp int)` (tcsetpgrp via `unix.IoctlSetPointerInt(fd, unix.TIOCSPGRP, pgrp)`) live in `src/cmd/wt/tty_unix.go` (`//go:build !windows`) with no-op stubs in `src/cmd/wt/tty_windows.go` (`//go:build windows`) — mirroring the `init_unix.go`/`init_windows.go` and `signal_unix.go`/`signal_windows.go` build-tag split. Windows has no `tcsetpgrp` / process-group terminal model, so the stubs do nothing (`terminalForeground` returns `(0, nil)`). The ioctls use `golang.org/x/sys/unix`, promoted from indirect to direct in `src/go.mod` with no new module (already present transitively via `golang.org/x/term`, used by `ShowMenu`). ### `RunWorktreeSetup` is thin (<50 lines) @@ -116,12 +126,16 @@ The reuse path's init step is a refresh, not a creation gate. Operators do not n ### Banner text is not byte-pinned -Tests assert presence of the worktree path, the `wt init` retry hint, and the `wt delete ` remove hint — NOT byte-equality of the banner template. Wording can evolve without test churn; the contract is the information surface, not the prose. +Tests assert presence of the worktree path, the `wt go ''` go hint (`260626-n6ma`, including the single-quote-escaping case for a name with an embedded quote), the `wt init` retry hint, and the `wt delete ` remove hint — NOT byte-equality of the banner template. Wording can evolve without test churn; the contract is the information surface, not the prose. + +### Open-anyway via flag + Open-phase fall-through, not a new open codepath (`260626-n6ma`) + +The interactive *Yes* path does NOT build a second open codepath inside the init-failure branch — it sets `worktreeOpen` appropriately and falls through to the EXISTING Open phase. *Why*: the load-bearing pre-Open terminal-foreground reclaim already guards the Open menu against SIGTTOU, so reusing the Open phase inherits that guarantee for free and avoids duplicating the open logic. The exit code is driven by an `initFailed` bool checked once at the function's end (before the stdout path line), so a successful open never downgrades to 0. *Rejected*: a separate inline open call in the failure block (would duplicate the Open phase and bypass its reclaim); a new exit code 8 or exit 0 on successful open (would erase the operator-depended-on `ExitInitFailed = 7` signal — Constitution III). (Source: change `260626-n6ma`, plan Design Decisions 1/3.) ## 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`), `src/internal/worktree/crud.go` (`RunWorktreeSetup`, `RunWorktreeSetupWithObserver`), `cmd/wt/create.go` (rollback disarm + handler swap + foreground capture/reclaim around the init block), `cmd/wt/init.go` (resolver consumer + foreground capture/reclaim around `runInitScript`'s `cmd.Run()`), `src/internal/worktree/apps.go` (`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`), `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). - 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/init-protocol.md b/docs/specs/init-protocol.md index 22e4738..c90f21e 100644 --- a/docs/specs/init-protocol.md +++ b/docs/specs/init-protocol.md @@ -131,14 +131,31 @@ kept. The user sees a structured banner containing: - A status line with the extracted exit code (or a generic phrase if the underlying error does not unwrap to `*exec.ExitError`). - The absolute worktree path that was kept. +- A navigation hint: `wt go ''` — the selection-free command to reach + the kept worktree. This hint lives in the banner itself, so it appears on + every path (interactive and non-interactive). - A copy-paste-ready retry hint: `cd && wt init` (uses `&&`, never `;`, so it parses identically in bash/zsh/fish). - A remove hint: `wt delete `. +**Interactive open-anyway prompt.** When `wt create` is interactive (stdin is +a TTY and `--non-interactive` was not passed), after printing the banner it +prompts `Continue and open the worktree anyway?`. Answering **yes** opens the +kept worktree via the normal Open phase (app menu); answering **no** skips the +Open phase. **In both cases `wt create` still exits `ExitInitFailed = 7`** — a +successful open never downgrades the exit code, so operators always see the +init-failure signal. When `wt create` is **non-interactive** (piped, CI, or +`--non-interactive`), no prompt is shown: it prints the banner and exits 7 +immediately, exactly as before. + +`ExitInitFailed = 7` thus holds on **every** init-failure path: non-interactive, +interactive open-anyway yes (even after a successful open), and interactive no. + The `wt create --reuse` path is exempt from `ExitInitFailed`: a reused worktree is presumed functional pre-existing, so its init step is a refresh (warn-but-continue) rather than a gate. `wt init` also exits with -`ExitInitFailed` when the init script it located exits non-zero. +`ExitInitFailed` when the init script it located exits non-zero (it has no +open-anyway prompt — it is not a worktree-creating command). ### SIGINT during init diff --git a/fab/changes/260626-n6ma-create-init-failure-open-anyway/.history.jsonl b/fab/changes/260626-n6ma-create-init-failure-open-anyway/.history.jsonl new file mode 100644 index 0000000..6b94950 --- /dev/null +++ b/fab/changes/260626-n6ma-create-init-failure-open-anyway/.history.jsonl @@ -0,0 +1,17 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-06-26T10:06:14Z"} +{"args":"wt create should degrade gracefully on init-script failure: print init-failure banner, then (interactive only) prompt to open the worktree anyway; either way add a 'Go: wt go \u003cname\u003e' hint to the banner; always exit ExitInitFailed=7 regardless of open choice/outcome; non-interactive keeps banner+exit7 with no prompt","cmd":"fab-new","event":"command","ts":"2026-06-26T10:06:14Z"} +{"delta":"+4.7","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-26T10:07:40Z"} +{"delta":"+0.0","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-26T10:07:47Z"} +{"delta":"+0.0","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-26T10:07:51Z"} +{"delta":"+0.0","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-26T10:08:13Z"} +{"delta":"+0.0","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-26T10:08:19Z"} +{"delta":"+0.0","event":"confidence","score":4.7,"trigger":"calc-score","ts":"2026-06-26T10:08:27Z"} +{"cmd":"fab-switch","event":"command","ts":"2026-06-26T10:09:22Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-06-26T10:11:51Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-06-26T10:22:40Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-06-26T10:28:38Z"} +{"event":"review","result":"passed","ts":"2026-06-26T10:28:38Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-26T10:33:33Z"} +{"cmd":"git-pr","event":"command","ts":"2026-06-26T10:34:13Z"} +{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-06-26T10:35:13Z"} +{"event":"review","result":"passed","ts":"2026-06-26T10:57:50Z"} diff --git a/fab/changes/260626-n6ma-create-init-failure-open-anyway/.status.yaml b/fab/changes/260626-n6ma-create-init-failure-open-anyway/.status.yaml new file mode 100644 index 0000000..0086ec0 --- /dev/null +++ b/fab/changes/260626-n6ma-create-init-failure-open-anyway/.status.yaml @@ -0,0 +1,52 @@ +id: n6ma +name: 260626-n6ma-create-init-failure-open-anyway +created: 2026-06-26T10:06:14Z +created_by: sahil-noon +change_type: feat +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: done + review-pr: done +plan: + generated: true + task_count: 7 + acceptance_count: 15 + acceptance_completed: 0 +confidence: + certain: 5 + confident: 2 + tentative: 0 + unresolved: 0 + score: 4.7 + fuzzy: true + dimensions: + signal: 87.9 + reversibility: 83.7 + competence: 87.9 + disambiguation: 82.1 +stage_metrics: + intake: {started_at: "2026-06-26T10:06:14Z", driver: fab-new, iterations: 1, completed_at: "2026-06-26T10:11:51Z"} + apply: {started_at: "2026-06-26T10:11:51Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T10:22:40Z"} + review: {started_at: "2026-06-26T10:22:40Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T10:28:38Z"} + hydrate: {started_at: "2026-06-26T10:28:38Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T10:33:33Z"} + ship: {started_at: "2026-06-26T10:33:33Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-26T10:35:13Z"} + review-pr: {started_at: "2026-06-26T10:35:13Z", driver: git-pr, iterations: 1, completed_at: "2026-06-26T10:57:50Z"} +prs: + - https://github.com/sahil87/wt/pull/27 +change_type_source: explicit +true_impact: + added: 0 + deleted: 0 + net: 0 + excluding: + added: 0 + deleted: 0 + net: 0 + computed_at: "2026-06-26T10:33:33Z" + computed_at_stage: hydrate +# true_impact: lazily created on first apply-finish (no placeholder here). +last_updated: 2026-06-26T10:57:50Z diff --git a/fab/changes/260626-n6ma-create-init-failure-open-anyway/intake.md b/fab/changes/260626-n6ma-create-init-failure-open-anyway/intake.md new file mode 100644 index 0000000..50e5513 --- /dev/null +++ b/fab/changes/260626-n6ma-create-init-failure-open-anyway/intake.md @@ -0,0 +1,196 @@ +# Intake: wt create — graceful degrade on init-script failure (open-anyway prompt + wt go hint) + +**Change**: 260626-n6ma-create-init-failure-open-anyway +**Created**: 2026-06-26 + +## Origin + +> `wt create` should degrade gracefully on init-script failure instead of hard-exiting — prompt the +> user to open the worktree anyway, and always point them at `wt go` to reach the kept worktree. + +Mode: one-shot synthesized dispatch (promptless-defer) from a live design conversation. The +conversation settled several load-bearing decisions, which are recorded as Certain/Confident +assumptions below: + +- **Exit code stays `ExitInitFailed = 7` on every path** — explicitly decided with the user. The + user rejected both "exit 0 if open succeeds" and "introduce a new exit code (8)" in favor of + *always exit 7*. +- **The `wt go` hint goes in the banner itself** (not only the interactive branch), because it + benefits every path including non-interactive. +- **The open-anyway prompt is interactive-only**, gated by the same TTY / `--non-interactive` + discipline the rest of `create` uses. + +## Why + +Today `wt create`'s init phase hard-exits the moment the init script returns non-zero. In +`src/cmd/wt/create.go` (the init-failure block, currently lines 339–354) the code does: + +```go +if err := wt.RunWorktreeSetupWithObserver(wtPath, initScript, ctx.RepoRoot, captureInit); err != nil { + signal.Stop(initSigCh) + close(initSigCh) + if reclaimTTY { + reclaimTerminalForeground(ttyFd, wtPgid) + } + rb.Disarm() + wt.PrintInitFailureBanner(wtPath, finalName, err) + os.Exit(wt.ExitInitFailed) // <-- hard exit; phase 5 (Open) is never reached +} +``` + +1. **Problem (pain point).** The worktree itself exists and is git-valid — only a user-supplied + script failed (the worktree is kept via `rb.Disarm()`). But `os.Exit(7)` fires inline at the + banner, so control never reaches the Open phase (phase 5). An interactive user who would happily + open the just-created worktree and fix init by hand is instead dropped back to their shell. This + is heavier-handed than necessary for a recoverable, kept-worktree situation. + +2. **Consequence if unfixed.** The interactive user must manually `cd` into the worktree (or run + `wt go`) after every init failure, even though `wt create` already had the Open menu one step + away. There is also no on-screen pointer to `wt go` in the banner today (only `cd … && wt init` + retry and `wt delete` hints), so the fastest path to *reach* the kept worktree is undiscoverable + from the failure output. + +3. **Why this approach over alternatives.** Falling through to the existing Open phase (rather than + building a new open path) reuses the load-bearing pre-Open terminal-foreground reclaim that is + already in place, so the open-anyway menu render cannot SIGTTOU. Adding the `wt go` hint to the + *banner* (rather than only to the interactive branch) means the non-interactive/CI path also + gains the discoverability benefit at zero behavior cost. Keeping the exit code at 7 on all paths + preserves the documented init-failure contract that operators depend on (see Design Decisions). + +## What Changes + +Two files change: a behavior change in `src/cmd/wt/create.go`, and a one-line additive hint in +`PrintInitFailureBanner` in `src/internal/worktree/errors.go`. + +### 1. `PrintInitFailureBanner` gains a `Go:` hint (`src/internal/worktree/errors.go`) + +The banner currently emits (in order): status line + `(init output is above)`; `Worktree:` +(absolute path, kept-note); `Retry: cd '' && wt init`; `Remove: wt delete ''`. + +Add a `Go: wt go ''` hint line. It points the user at the selection-free navigation command +for the kept worktree. + +- Single-quote the name via the existing `shellQuoteSingle` helper (`src/internal/worktree/apps.go`), + consistent with the sibling `Retry`/`Remove` hints — the banner already single-quotes both path + and name. +- Use a named banner label constant in the same `const (...)` block as `bannerLabelWorktree` / + `bannerLabelRetry` / `bannerLabelRemove` (e.g. `bannerLabelGo = "Go: "`), aligned to the + existing label column width, so the canonical wording stays confined to this one helper (no + duplication — anti-pattern per code-quality). +- This hint benefits EVERY caller path (interactive yes, interactive no, and non-interactive), which + is why it lives in the banner itself, not in the interactive branch. + +Placement relative to the existing hints (Retry/Remove) is a presentation detail; placing `Go:` +after `Worktree:` and alongside the other action hints is the natural grouping. + +### 2. `wt create` init-failure block degrades gracefully (`src/cmd/wt/create.go`) + +Restructure the init-failure block (currently lines 339–354) so it no longer `os.Exit(7)`s inline. + +**Non-interactive / piped / CI — EXACT current behavior preserved (no prompt):** +- Print the banner (now including the new `Go:` line) and exit `ExitInitFailed = 7`. +- The new prompt sits behind the same gating the rest of `create` uses: the `nonInteractive` flag + and the TTY check. Note `reclaimTTY` (already computed as `term.IsTerminal(ttyFd)` at + `create.go:292`) is the existing interactivity signal in this block; the prompt is shown only when + interactive (`!nonInteractive` AND stdin is a TTY). + +**Interactive — new open-anyway flow:** +1. Acknowledge the error exactly as today: `wt.PrintInitFailureBanner(wtPath, finalName, err)` (init + output already streamed above). +2. Prompt: `wt.ConfirmYesNo("Continue and open the worktree anyway?")` (`ConfirmYesNo` lives in + `src/internal/worktree/menu.go`). + + - **Yes** → fall through into the existing Open phase (phase 5) so the user can open the kept + worktree. The pre-Open terminal-foreground reclaim already runs before the Open separator/menu + render, so the open-anyway menu render will NOT SIGTTOU. + - **No** → do not open; show the user how to reach the already-created worktree (the banner's new + `Go:` line already provides this; the No branch shows no app menu). +3. **Either way the process MUST still exit `ExitInitFailed = 7`** at the end — regardless of the + open-anyway choice and regardless of whether the open succeeded. + +**Implementation note (load-bearing):** do NOT `os.Exit(7)` inline at the banner. Instead set an +"init failed" flag (or otherwise restructure) so the open-anyway *Yes* path falls through to the +existing Open phase, and the function exits 7 at the end on all init-failure paths. The Open phase's +own normal-success exit (which today prints the path line and returns 0) must be overridden to 7 when +the init-failure flag is set, so a *successful open* never downgrades the exit to 0. + +### Invariants that MUST be preserved on the new open-anyway branch + +These already hold on today's single exit path and must continue to hold on the *Yes* fall-through: + +- **Worktree is KEPT**: `rb.Disarm()` (NOT `rb.Execute()`) on init-script non-zero exit. The + `defer rb.Execute()` still fires on any other failure. +- **SIGINT Option B teardown**: `signal.Stop(initSigCh)` + `close(initSigCh)` must run before the + open phase on the open-anyway path too (the init-child signal handler must be torn down before the + Open menu). +- **Terminal-foreground reclaim**: the load-bearing pre-Open reclaim (`reclaimTerminalForeground`, + gated on `reclaimTTY`) must run before the open phase on the open-anyway path — the banner is a TTY + write and the menu render is a TTY write; both would SIGTTOU if foreground were stranded by a + shared-TTY init child. + +### Out of scope / unchanged + +- **`--reuse` path is exempt.** Its init step is a refresh (warn-but-continue), not a creation gate, + and does not adopt `ExitInitFailed`. Unchanged. +- **Not-found path is distinct and unchanged.** Init-script-missing (`*InitNotFound`) is non-fatal: + warning + exit 0. Only "resolver succeeded, process executed, exited non-zero" triggers this flow. +- **`wt init` (standalone)** is not in scope — only `wt create`'s init step changes behavior. (The + banner hint change is shared via `PrintInitFailureBanner`, but `wt init` does not call it.) + +## Affected Memory + +- `wt-cli/init-failure-contract.md`: (modify) The kept-worktree + `ExitInitFailed` contract gains + the interactive open-anyway prompt and the new `wt go` banner hint. Specifically: the + "`wt create` exits with `ExitInitFailed`" requirement is amended so that exit-7 holds on ALL + init-failure paths including a successful open-anyway open; the banner requirement gains the `Go:` + hint line; the SIGINT-teardown + terminal-foreground-reclaim invariants are documented as also + holding on the open-anyway fall-through. +- `wt-cli/create-output-phases.md`: (modify) The Open phase can now run after an init failure on the + interactive *Yes* path — previously the Open separator/menu were reachable only on init success. + +## Impact + +- **Code**: + - `src/cmd/wt/create.go` — restructure the init-failure block (lines ~339–354): replace inline + `os.Exit(ExitInitFailed)` with a flag-based fall-through to Open + interactive prompt; ensure the + function exits 7 on all init-failure paths (incl. successful open). + - `src/internal/worktree/errors.go` — add the `Go:` hint line + label constant to + `PrintInitFailureBanner`. +- **APIs / contracts**: + - `ExitInitFailed = 7` exit-code contract — preserved (the whole point). No new exit code. + - `PrintInitFailureBanner` signature unchanged (still `(wtPath, name string, err error)`); only its + output gains a line. +- **Consumers**: operators (fab-kit, `hop`, shell wrappers) that branch on `$?` to detect "worktree + exists, init didn't complete" — behavior preserved (still exit 7). +- **Tests** (`src/cmd/wt/create_test.go`, and any banner test in + `src/internal/worktree/errors_test.go`): + - Banner test gains a `wt go` / `Go:` substring assertion (the contract tests assert on information + surface, not byte equality). + - Non-interactive init-failure path: still banner + exit 7, NO prompt. + - Interactive init-failure path: new prompt; *Yes* falls through to Open; *No* skips open; both exit + 7. Per `code-review.md`, any test exercising the open path MUST be side-effect-free (use a + non-side-effecting `--worktree-open` target or rely on `runWt`'s env isolation / the + `WT_TEST_NO_LAUNCH=1` seam — actual window creation is not exercised in the unit suite). +- **Spec**: `docs/specs/init-protocol.md` "Script failure semantics" section is updated to describe + the interactive open-anyway prompt and the `wt go` hint (and to reaffirm exit 7 on all paths). + Memory hydrate / spec hydrate handle the doc edits downstream. + +## Open Questions + +- None blocking. The exact prompt string (`"Continue and open the worktree anyway?"`) and whether the + *No* branch needs any message beyond the banner's `Go:` line are minor copy decisions resolved + inline (see Assumptions) and easily tuned via `/fab-clarify`. + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | Process exits `ExitInitFailed = 7` on every init-failure path, including a successful open-anyway open (no exit 0, no new code 8). | Explicitly decided with the user; exit 7 is a documented, depended-on contract (Constitution III; `init-failure-contract.md`). Letting a successful open downgrade to 0 would overload `0` and erase the init-failure signal — a contract break. | S:98 R:88 A:95 D:95 | +| 2 | Certain | The `wt go ''` hint goes in `PrintInitFailureBanner` itself (not only the interactive branch), single-quoted via `shellQuoteSingle`, with a named label constant alongside the existing banner labels. | Explicitly stated: benefits every path incl. non-interactive; mirrors the existing `Retry`/`Remove` hints' single-quoting + label-constant pattern; banner already centralizes wording (no duplication). | S:95 R:90 A:95 D:90 | +| 3 | Certain | The open-anyway prompt is interactive-only, gated by `!nonInteractive` AND the existing TTY check (`reclaimTTY`/`term.IsTerminal`); non-interactive/piped/CI keeps today's exact banner + exit 7 with NO prompt. | Constitution VI (Interactive by Default, Scriptable on Demand); the description pins the gate to the same TTY/`--non-interactive` discipline the rest of `create` uses; `reclaimTTY` is already computed in this block. | S:95 R:85 A:95 D:90 | +| 4 | Certain | On *Yes*, control falls through to the EXISTING Open phase (phase 5) rather than a new open codepath; SIGINT-teardown (`signal.Stop`/`close(initSigCh)`) and the pre-Open terminal-foreground reclaim run before the Open menu on this path too. | Reuses the load-bearing reclaim that already guards the Open menu against SIGTTOU; the description names these invariants as MUST-preserve; `init-failure-contract.md` documents them for the existing success path. | S:92 R:80 A:90 D:90 | +| 5 | Certain | The worktree is KEPT (`rb.Disarm()`, not `rb.Execute()`) on init-script non-zero exit — unchanged; `--reuse` and the not-found path remain exempt/unchanged. | Existing contract restated verbatim in the description; no change requested to rollback, reuse, or not-found semantics. | S:95 R:88 A:95 D:95 | +| 6 | Confident | Implementation uses an "init failed" flag (set in the failure block) rather than inline `os.Exit`, so the *Yes* path falls through to Open and the function exits 7 at the end; the Open phase's normal exit-0/return is overridden to 7 when the flag is set. | The description gives this as the implementation note; it is the natural Go structuring, but the exact mechanism (flag vs. labeled restructure vs. early helper) is an implementation choice the apply agent finalizes. Reversible, low blast radius. | S:80 R:75 A:80 D:70 | +| 7 | Confident | Exact prompt wording is `"Continue and open the worktree anyway?"`, and the *No* branch shows no additional message beyond the banner's `Go:` line (which already shows how to reach the worktree). | The description suggests this exact string ("e.g. …") and says "show how to reach the worktree" — the banner's new `Go:` line satisfies that, so no extra No-branch copy is assumed. Reversible copy decision via `/fab-clarify`; a clear front-runner default exists. | S:60 R:80 A:65 D:45 | + +7 assumptions (5 certain, 2 confident, 0 tentative, 0 unresolved). diff --git a/fab/changes/260626-n6ma-create-init-failure-open-anyway/plan.md b/fab/changes/260626-n6ma-create-init-failure-open-anyway/plan.md new file mode 100644 index 0000000..c972d41 --- /dev/null +++ b/fab/changes/260626-n6ma-create-init-failure-open-anyway/plan.md @@ -0,0 +1,180 @@ +# Plan: wt create — graceful degrade on init-script failure (open-anyway prompt + wt go hint) + +**Change**: 260626-n6ma-create-init-failure-open-anyway +**Intake**: `intake.md` + +## Requirements + +### Init-Failure Banner: `wt go` hint + +#### R1: Banner emits a `Go:` navigation hint +`PrintInitFailureBanner` in `src/internal/worktree/errors.go` SHALL emit a `Go: wt go ''` +hint line on every invocation, single-quoting the name via the existing `shellQuoteSingle` helper, +keyed off a named label constant (`bannerLabelGo`) declared in the SAME `const (...)` block as +`bannerLabelWorktree` / `bannerLabelRetry` / `bannerLabelRemove` and aligned to the existing label +column width. The line SHALL be grouped with the other action hints, placed after the `Worktree:` +line. + +- **GIVEN** an init-failure banner is rendered for a kept worktree named `abc` +- **WHEN** `PrintInitFailureBanner(wtPath, "abc", err)` runs +- **THEN** stderr contains a line with `wt go 'abc'` (the name single-quoted) +- **AND** the canonical wording lives only in the `bannerLabelGo` constant (no duplication) +- **AND** the hint appears on every caller path (interactive yes, interactive no, non-interactive), + because it lives in the shared banner helper. + +#### R2: Banner hint single-quotes shell-special names +The `Go:` hint SHALL stay copy-paste-safe when the worktree name contains shell-special characters, +using `shellQuoteSingle` exactly as the sibling `Remove:` hint does. + +- **GIVEN** a worktree name containing a single quote (e.g. `my'name`) +- **WHEN** the banner is rendered +- **THEN** the `Go:` hint escapes the embedded quote as `'\''` (matching the `Remove:` hint behavior) + +### `wt create` Init-Failure Degradation + +#### R3: Non-interactive init failure preserves today's exact behavior +On init-script non-zero exit when NOT interactive (`nonInteractive` set OR stdin is not a TTY), +`wt create` SHALL print the init-failure banner (now including the `Go:` line) and exit +`ExitInitFailed = 7` with NO prompt — byte-for-byte the same flow as today minus the inline exit +location. The interactivity gate is `!nonInteractive` AND `reclaimTTY` (`term.IsTerminal(ttyFd)`, +already computed earlier in the block). + +- **GIVEN** `wt create --non-interactive` with an init script that exits non-zero +- **WHEN** init fails +- **THEN** the banner is printed, no prompt is shown, and the process exits 7 +- **AND** the worktree directory and branch survive (kept). + +#### R4: Interactive init failure offers open-anyway, exits 7 either way +On init-script non-zero exit when interactive (`!nonInteractive` AND stdin is a TTY), `wt create` +SHALL: (1) print the banner; (2) prompt `wt.ConfirmYesNo("Continue and open the worktree anyway?")`; +on **Yes** fall through into the EXISTING Open phase so the user can open the kept worktree; on +**No** skip the Open phase (no app menu). In BOTH cases the function SHALL exit `ExitInitFailed = 7`. +A successful open MUST NEVER downgrade the exit code to 0. + +- **GIVEN** an interactive `wt create` with an init script that exits non-zero +- **WHEN** the user answers Yes to the open-anyway prompt +- **THEN** control falls through to the existing Open phase +- **AND** the process exits 7 regardless of whether the open succeeded. +- **GIVEN** the same failure +- **WHEN** the user answers No +- **THEN** the Open phase does not run and the process exits 7. + +#### R5: Open-anyway path preserves the kept-worktree + signal/TTY invariants +The restructured block SHALL preserve, on the open-anyway fall-through path, every invariant the +current single-exit path holds: the worktree is KEPT via `rb.Disarm()` (NOT `rb.Execute()`) on +init-script non-zero exit; SIGINT Option B teardown (`signal.Stop(initSigCh)` + `close(initSigCh)`) +runs before the Open phase; the load-bearing terminal-foreground reclaim +(`reclaimTerminalForeground`, gated on `reclaimTTY`) runs before the banner AND before the Open menu. + +- **GIVEN** an init failure on a shared-TTY init child +- **WHEN** the user falls through to Open on the Yes path +- **THEN** `rb.Disarm()` has run (worktree kept), the init signal channel is stopped and closed + before the Open menu, and terminal foreground is reclaimed before both the banner write and the + Open menu write (no SIGTTOU). + +#### R6: Implementation uses an init-failed flag, not inline `os.Exit` +The init-failure block SHALL NOT call `os.Exit(7)` inline at the banner. Instead it SHALL set an +"init failed" flag so the Yes path falls through to the existing Open phase, and the function SHALL +exit 7 at the end on all init-failure paths. The Open phase's normal success exit (which prints the +path line and returns 0) SHALL be overridden to exit 7 when the flag is set. + +- **GIVEN** the init-failure flag is set after a Yes-and-successful-open +- **WHEN** the function reaches its normal return/exit +- **THEN** it exits 7 (the success-path `return nil` / path print does not run, or is overridden). + +### Non-Goals + +- `--reuse` init path — exempt (warn-but-continue refresh), unchanged. +- `*InitNotFound` (init-script-missing) path — non-fatal, exit 0, unchanged. +- Standalone `wt init` — out of scope (does not call `PrintInitFailureBanner`). +- No new exit code: `ExitInitFailed = 7` is reused on every path. + +### Design Decisions + +1. **Flag-based fall-through over a new open codepath**: set an `initFailed` bool in the failure + block and let the Yes path fall through to the existing Open phase — *Why*: reuses the + load-bearing pre-Open terminal-foreground reclaim that already guards the Open menu against + SIGTTOU; avoids duplicating the open logic — *Rejected*: a separate inline open call in the + failure block (would duplicate the Open phase and bypass its reclaim). +2. **`Go:` hint in the banner, not the interactive branch**: *Why*: benefits every path including + non-interactive/CI at zero behavior cost; centralizes the wording in one helper — *Rejected*: + printing the hint only on the interactive No branch (loses discoverability for non-interactive). +3. **Exit 7 on all paths, including successful open**: *Why*: `ExitInitFailed = 7` is a documented, + operator-depended-on contract (Constitution III); a successful open downgrading to 0 would erase + the init-failure signal — *Rejected*: exit 0 on successful open; a new exit code 8. + +## Tasks + +### Phase 1: Banner hint (errors.go) + +- [x] T001 Add `bannerLabelGo = "Go: "` to the existing banner-label `const (...)` block in `src/internal/worktree/errors.go`, aligned to the same label column width as `bannerLabelWorktree`/`bannerLabelRetry`/`bannerLabelRemove`. +- [x] T002 In `PrintInitFailureBanner` (`src/internal/worktree/errors.go`), emit the `Go:` hint line after the `Worktree:` line and before the `Retry:` line, single-quoting `name` via `shellQuoteSingle`, mirroring the `Remove:` hint's `Fprintf` shape; update the banner-order doc comment to list the new line. + +### Phase 2: Init-failure degradation (create.go) + +- [x] T003 Restructure the init-failure block (`src/cmd/wt/create.go` ~339-354): replace the inline `os.Exit(wt.ExitInitFailed)` with setting an `initFailed` flag; keep `signal.Stop(initSigCh)`+`close(initSigCh)`, the pre-banner `reclaimTerminalForeground` (gated on `reclaimTTY`), `rb.Disarm()`, and `wt.PrintInitFailureBanner(...)`; after the banner, when interactive (`!nonInteractive && reclaimTTY`) prompt `wt.ConfirmYesNo("Continue and open the worktree anyway?")` — on No, set `worktreeOpen = "skip"` (or otherwise suppress the Open menu) and fall through; on the non-interactive branch, exit 7 immediately (no prompt). The Yes path falls through to the existing Open phase. +- [x] T004 Override the success exit when `initFailed` is set: at the function's normal end (`src/cmd/wt/create.go`, after the Open phase / `rb.Disarm()`), when `initFailed` is true, exit `wt.ExitInitFailed` instead of printing the path line and returning 0, so a successful open never downgrades to 0. Preserve all kept-worktree and signal/TTY invariants on this path. + +### Phase 3: Tests + +- [x] T005 [P] Extend the banner test in `src/internal/worktree/errors_test.go` to assert the `wt go ''` / `Go:` substring is present (information-surface assertion, not byte equality), including the single-quote-escaping case for a name with an embedded quote. +- [x] T006 Add a non-interactive init-failure test in `src/cmd/wt/create_test.go` asserting the banner now contains the `wt go` hint and the prompt string is NOT present (still exit 7, no prompt) — reuse `createFailingInitScript` + `--non-interactive --worktree-open skip`. +- [x] T007 Add interactive init-failure tests in `src/cmd/wt/create_test.go` driving stdin: Yes -> prompt shown, falls through to Open, exit 7, worktree kept; No -> prompt shown, no app menu, exit 7, worktree kept. Use a non-side-effecting open target / `runWt`'s env isolation / the `WT_TEST_NO_LAUNCH=1` seam so no real editor/tmux window is created. + +## Execution Order + +- T001 blocks T002 (constant before use). +- T003 blocks T004 (flag must be set before the override reads it). +- T005 is independent ([P]); T006/T007 depend on Phase 1 + Phase 2 being complete. + +## Acceptance + +### Functional Completeness + +- [ ] A-001 R1: `PrintInitFailureBanner` emits a `Go: wt go ''` line via a named `bannerLabelGo` constant in the shared label block, on every caller path. +- [ ] A-002 R2: The `Go:` hint single-quotes the name via `shellQuoteSingle`, escaping embedded quotes as `'\''`. +- [ ] A-003 R3: Non-interactive (`--non-interactive` / piped / CI) init failure prints the banner and exits 7 with NO prompt; worktree + branch kept. +- [ ] A-004 R4: Interactive init failure prompts `Continue and open the worktree anyway?`; Yes falls through to Open, No skips Open; both exit 7. +- [ ] A-005 R6: No inline `os.Exit(7)` remains in the failure block; an `initFailed` flag drives fall-through and the end-of-function exits 7 when set. + +### Behavioral Correctness + +- [ ] A-006 R4: A successful open on the Yes path still exits 7 — the success path's path-print/return-0 is overridden when `initFailed` is set. +- [ ] A-007 R5: On the open-anyway path the worktree is kept (`rb.Disarm()`, not `rb.Execute()`), `signal.Stop(initSigCh)`+`close(initSigCh)` run before Open, and `reclaimTerminalForeground` (gated on `reclaimTTY`) runs before both the banner and the Open menu. + +### Scenario Coverage + +- [ ] A-008 R3: A test asserts non-interactive init failure -> banner (with `wt go`) + exit 7, no prompt string. +- [ ] A-009 R4: Tests assert interactive Yes (falls through to Open, exit 7) and No (no app menu, exit 7), both side-effect-free. +- [ ] A-010 R1: The banner unit test asserts the `wt go` substring on the information surface. + +### Edge Cases & Error Handling + +- [ ] A-011 R5: Open-anyway tests confirm the worktree directory and branch survive on both Yes and No. +- [ ] A-012 R4: No test creates a real editor/tmux/byobu window (uses a non-side-effecting open target or `WT_TEST_NO_LAUNCH` / env isolation). + +### Code Quality + +- [ ] A-013 Pattern consistency: New code follows naming and structural patterns of surrounding code (label-constant pattern, `Fprintf` banner shape, the create.go signal/TTY comment density). +- [ ] A-014 No unnecessary duplication: The `Go:` wording lives only in `bannerLabelGo`; the Yes path reuses the existing Open phase rather than a new open codepath. +- [ ] A-015 No magic strings: The prompt string and banner label are the only new literals; the label is a named constant (banner) consistent with siblings. + +## Notes + +- Check items as you review: `- [x]` +- All acceptance items must pass before `/fab-continue` (hydrate) +- gofmt is CI-enforced from `src/` — run `gofmt -l .` after edits. + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | Process exits `ExitInitFailed = 7` on every init-failure path, including a successful open-anyway open (no exit 0, no new code 8). | Explicitly decided with the user; exit 7 is a documented, depended-on contract (Constitution III; init-failure-contract). A successful-open downgrade to 0 would erase the init-failure signal. | S:98 R:88 A:95 D:95 | +| 2 | Certain | The `wt go ''` hint goes in `PrintInitFailureBanner` itself (not only the interactive branch), single-quoted via `shellQuoteSingle`, with a named `bannerLabelGo` constant alongside the existing banner labels. | Benefits every path incl. non-interactive; mirrors the existing Retry/Remove single-quoting + label-constant pattern; centralizes wording (no duplication). | S:95 R:90 A:95 D:90 | +| 3 | Certain | The open-anyway prompt is interactive-only, gated by `!nonInteractive` AND the existing TTY check (`reclaimTTY`/`term.IsTerminal`); non-interactive/piped/CI keeps today's banner + exit 7 with NO prompt. | Constitution VI; the intake pins the gate to the same TTY/--non-interactive discipline the rest of create uses; `reclaimTTY` is already computed in this block. | S:95 R:85 A:95 D:90 | +| 4 | Certain | On Yes, control falls through to the EXISTING Open phase rather than a new open codepath; SIGINT-teardown and the pre-Open terminal-foreground reclaim run before the Open menu on this path too. | Reuses the load-bearing reclaim guarding the Open menu against SIGTTOU; the intake names these invariants as MUST-preserve. | S:92 R:80 A:90 D:90 | +| 5 | Certain | The worktree is KEPT (`rb.Disarm()`, not `rb.Execute()`) on init-script non-zero exit; `--reuse` and the not-found path remain exempt/unchanged. | Existing contract restated verbatim in the intake; no change requested to rollback, reuse, or not-found semantics. | S:95 R:88 A:95 D:95 | +| 6 | Confident | Implementation uses an `initFailed` flag (set in the failure block) rather than inline `os.Exit`; the Yes path falls through to Open and the function exits 7 at the end; the success path's exit-0/return is overridden to 7 when the flag is set. The No branch suppresses the Open menu by setting `worktreeOpen = "skip"`. | The intake gives this as the implementation note; flag + worktree-open=skip is the minimal, low-blast-radius structuring that reuses the existing Open phase without a new codepath. | S:80 R:75 A:82 D:72 | +| 7 | Confident | Exact prompt wording is `"Continue and open the worktree anyway?"`; the No branch shows no message beyond the banner's `Go:` line. | The intake suggests this exact string and says "show how to reach the worktree" — the banner's `Go:` line satisfies that. Reversible copy decision; a clear front-runner default exists. | S:60 R:80 A:65 D:45 | + +7 assumptions (5 certain, 2 confident, 0 tentative). diff --git a/src/cmd/wt/create.go b/src/cmd/wt/create.go index 79d342c..02bed27 100644 --- a/src/cmd/wt/create.go +++ b/src/cmd/wt/create.go @@ -268,6 +268,13 @@ or creates a new branch.`, fmt.Fprintf(os.Stderr, "Created worktree: %s\nPath: %s\nBranch: %s\n", finalName, wtPath, createdSummaryBranch) + // initFailed records that the init script exited non-zero. It is + // set in the failure branch below instead of an inline os.Exit so + // the interactive "open anyway" path can fall through to the Open + // phase (phase 5). It is read at the very end of the function to + // force ExitInitFailed on ALL init-failure paths — a successful + // open must NOT downgrade the exit to 0. + var initFailed bool if runInit { initScript := wt.InitScriptPath() @@ -301,15 +308,17 @@ or creates a new branch.`, wtPgid = fg } } - // Best-effort restore: a panic/early-return safety net. The two - // explicit reclaims below (pre-Open on success, pre-banner on - // init failure) are the load-bearing ones — both terminate via - // os.Exit or fall through to Open, and os.Exit skips deferred - // funcs, so this defer only actually fires on a non-os.Exit - // return or a panic in this block. (The pre-init SIGINT handler's - // exit-130 path runs before this defer is installed and before - // any foreground was captured, so it has nothing to reclaim.) It - // never blocks rollback or changes exit codes. + // Best-effort restore: a panic/early-return safety net. The + // single explicit reclaim below (run unconditionally after the + // init call, before the banner / open-anyway prompt / Open menu) + // is the load-bearing one — every init-failure path either + // os.Exits (non-interactive) or falls through to Open, and + // os.Exit skips deferred funcs, so this defer only actually + // fires on a non-os.Exit return or a panic in this block. (The + // pre-init SIGINT handler's exit-130 path runs before this defer + // is installed and before any foreground was captured, so it has + // nothing to reclaim.) It never blocks rollback or changes exit + // codes. if reclaimTTY { defer reclaimTerminalForeground(ttyFd, wtPgid) } @@ -336,30 +345,61 @@ or creates a new branch.`, }() captureInit := func(c *exec.Cmd) { initCmdPtr.Store(c) } - if err := wt.RunWorktreeSetupWithObserver(wtPath, initScript, ctx.RepoRoot, captureInit); err != nil { - // Init-script non-zero exit: keep the worktree, print the - // structured banner, exit with ExitInitFailed so operators - // can distinguish "worktree exists, init didn't complete" - // from other failures. - signal.Stop(initSigCh) - close(initSigCh) - // Reclaim foreground BEFORE the banner — the banner is a TTY - // write too, and would itself SIGTTOU if foreground was lost. - if reclaimTTY { - reclaimTerminalForeground(ttyFd, wtPgid) - } - rb.Disarm() - wt.PrintInitFailureBanner(wtPath, finalName, err) - os.Exit(wt.ExitInitFailed) - } + initErr := wt.RunWorktreeSetupWithObserver(wtPath, initScript, ctx.RepoRoot, captureInit) + + // SIGINT Option B teardown — tear down the init-child signal + // handler before any further TTY work (banner, open-anyway + // prompt, or the Open menu). This runs on EVERY init outcome + // (success and the open-anyway fall-through), so the init + // handler is never left armed once the init child has exited. signal.Stop(initSigCh) close(initSigCh) - // Reclaim foreground before the Open phase so the menu render - // can never SIGTTOU on a shared-TTY init child. This is the - // load-bearing reclaim. + // Reclaim foreground before any further TTY write. This is the + // load-bearing reclaim: it precedes the init-failure banner AND + // the open-anyway prompt AND the Open menu (the menu is reached + // either on init success or via the open-anyway Yes fall-through + // below) — all are TTY writes that would SIGTTOU if foreground + // were stranded by a shared-TTY init child. if reclaimTTY { reclaimTerminalForeground(ttyFd, wtPgid) } + + if initErr != nil { + // Init-script non-zero exit: keep the worktree and print the + // structured banner. We do NOT os.Exit inline — instead we + // set initFailed and (when interactive) offer to open the + // kept worktree anyway by falling through to the Open phase. + // The function exits ExitInitFailed at the end on every + // init-failure path (incl. a successful open-anyway open), so + // operators can still distinguish "worktree exists, init + // didn't complete" from other failures. + initFailed = true + // Worktree is KEPT on init-script non-zero exit: disarm the + // rollback so the deferred rb.Execute() (which still fires on + // any OTHER failure) does not remove the just-created worktree. + rb.Disarm() + wt.PrintInitFailureBanner(wtPath, finalName, initErr) + + // Open-anyway prompt is interactive-only, gated by the same + // TTY / --non-interactive discipline the rest of create uses + // (!nonInteractive AND stdin is a TTY — reclaimTTY is that + // signal, already computed above). Non-interactive / piped / + // CI keeps today's exact behavior: banner + exit 7, NO prompt. + if !nonInteractive && reclaimTTY { + if !wt.ConfirmYesNo("Continue and open the worktree anyway?") { + // No: do not open. The banner's Go: line already shows + // how to reach the worktree, so no app menu is shown. + worktreeOpen = "skip" + } + // Yes: fall through into the existing Open phase (foreground + // already reclaimed above) so the user can open the kept + // worktree. Either way the function exits 7 at the end via + // the initFailed check. + } else { + // Non-interactive: exit 7 immediately, no prompt, no Open. + os.Exit(wt.ExitInitFailed) + } + } } // Open @@ -422,9 +462,21 @@ or creates a new branch.`, } } - // Success — disarm rollback + // Success — disarm rollback (a no-op on the open-anyway path, + // where rb was already disarmed in the init-failure branch — the + // worktree was always kept). rb.Disarm() + // Init-failure override: when the interactive open-anyway path + // fell through to Open, the process MUST still exit + // ExitInitFailed — a successful open must NOT downgrade the exit + // to 0 and erase the init-failure signal operators depend on. + // This is the single exit point for the open-anyway path; it runs + // regardless of whether the open succeeded or the user chose No. + if initFailed { + os.Exit(wt.ExitInitFailed) + } + // Output the worktree path as the last line if !suppressPath { fmt.Println(wtPath) diff --git a/src/cmd/wt/create_init_failure_pty_test.go b/src/cmd/wt/create_init_failure_pty_test.go new file mode 100644 index 0000000..7486f6a --- /dev/null +++ b/src/cmd/wt/create_init_failure_pty_test.go @@ -0,0 +1,296 @@ +//go:build !windows + +package main + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// The interactive open-anyway prompt on init failure is gated on stdin being a +// real TTY (term.IsTerminal) AND wt being able to read the terminal's +// foreground process group (tcgetpgrp succeeding). A plain exec with a pipe +// stdin fails the TTY check, so these tests drive wt under a real PTY via a +// session-leader launcher — the same harness shape as +// TestIntegration_ReclaimForegroundAfterInit_NotStopped, but here the launcher +// feeds a "y\n" or "n\n" answer to the open-anyway prompt and reports wt's exit +// code back through its own exit code. +// +// Host-side-effect safety (code-review.md): wt is invoked with +// --worktree-open=open_here, the cooperative app that only prints a `cd` line +// (and never under WT_TEST_NO_LAUNCH short-circuits it anyway). No real +// editor/tmux/byobu window is ever created. The PTY env also clears +// TMUX/BYOBU_*/TERM_PROGRAM and sets WT_TEST_NO_LAUNCH=1. + +// setupFailureExit is the launcher's "could not set up PTY / process groups" +// signal (e.g. sandboxed CI). wt itself never exits with this code, so the test +// treats it as a skip rather than a failure. +const setupFailureExit = 30 + +// runInteractiveInitFailure builds the launcher, wires up a failing init script, +// and runs `wt create` under a PTY with the given answer ("y" or "n") fed to the +// open-anyway prompt. It returns wt's combined PTY output and exit code, or +// skips the test when the PTY/process-group harness is unavailable. +func runInteractiveInitFailure(t *testing.T, repo, wtName, answer string) (string, int) { + t.Helper() + + helperDir := t.TempDir() + launcherBin := buildHelper(t, helperDir, "answer_launcher", ptyAnswerLauncherSrc) + + // Failing init script: stream a marker, exit non-zero. This drives the + // init-failure branch without grabbing terminal foreground (the foreground + // concern is covered by the dedicated reclaim test). + scriptDir := filepath.Join(repo, "scripts") + if err := os.MkdirAll(scriptDir, 0o755); err != nil { + t.Fatalf("MkdirAll script dir: %v", err) + } + script := filepath.Join(scriptDir, "init-fail.sh") + content := "#!/usr/bin/env bash\necho 'INIT_FAIL_MARKER' >&2\nexit 1\n" + if err := os.WriteFile(script, []byte(content), 0o755); err != nil { + t.Fatalf("write init script: %v", err) + } + gitRun(t, repo, "add", "scripts/init-fail.sh") + gitRun(t, repo, "commit", "-q", "-m", "Add failing init script") + // Keep the repo clean so wt does not stop at the interactive dirty-state + // prompt (which would block on PTY input before the init-failure prompt). + gitRun(t, repo, "add", "-A") + if status := gitRun(t, repo, "status", "--porcelain"); status != "" { + gitRun(t, repo, "commit", "-q", "-m", "clean") + } + + // argv: launcher + launch := exec.Command(launcherBin, answer, wtBinary, repo, wtName) + launch.Env = append(os.Environ(), + "NO_COLOR=1", + "WORKTREE_INIT_SCRIPT=scripts/init-fail.sh", + "WT_TEST_NO_LAUNCH=1", + "TMUX=", "BYOBU_BACKEND=", "BYOBU_TTY=", "BYOBU_SESSION=", + "BYOBU_CONFIG_DIR=", "TERM_PROGRAM=", + ) + combined, runErr := launch.CombinedOutput() + + exitCode := 0 + if runErr != nil { + ee, ok := runErr.(*exec.ExitError) + if !ok { + t.Skipf("launcher did not run (sandboxed?): %v\n%s", runErr, combined) + } + exitCode = ee.ExitCode() + } + if exitCode == setupFailureExit { + t.Skipf("PTY/process-group harness unavailable (launcher exit %d) — skipping\n%s", exitCode, combined) + } + return string(combined), exitCode +} + +// TestCreate_InitFailureInteractive_OpenAnyway exercises the interactive Yes +// path: the open-anyway prompt is shown, the user answers Yes, control falls +// through to the existing Open phase (open_here prints a cd line), and the +// process STILL exits ExitInitFailed (7) — a successful open must not downgrade +// the exit to 0. The worktree and branch are kept. +func TestCreate_InitFailureInteractive_OpenAnyway(t *testing.T) { + repo := createTestRepo(t) + + out, exitCode := runInteractiveInitFailure(t, repo, "open-anyway-yes", "y") + + if exitCode != 7 { + t.Fatalf("expected exit 7 on interactive open-anyway (Yes), got %d\noutput:\n%s", exitCode, out) + } + // The banner (now with the wt go hint) and the open-anyway prompt are shown. + assertContains(t, out, "wt go 'open-anyway-yes'") + assertContains(t, out, "Continue and open the worktree anyway?") + // Yes fell through to the Open phase: open_here prints a cd line. + assertContains(t, out, "cd -- '") + // Worktree + branch are kept. + assertWorktreeExists(t, repo, "open-anyway-yes") + assertBranchExists(t, repo, "open-anyway-yes") +} + +// TestCreate_InitFailureInteractive_DeclineOpen exercises the interactive No +// path: the prompt is shown, the user answers No, the Open phase is skipped (no +// cd line, no app menu), and the process exits ExitInitFailed (7). The worktree +// and branch are kept. +func TestCreate_InitFailureInteractive_DeclineOpen(t *testing.T) { + repo := createTestRepo(t) + + out, exitCode := runInteractiveInitFailure(t, repo, "open-anyway-no", "n") + + if exitCode != 7 { + t.Fatalf("expected exit 7 on interactive open-anyway (No), got %d\noutput:\n%s", exitCode, out) + } + assertContains(t, out, "wt go 'open-anyway-no'") + assertContains(t, out, "Continue and open the worktree anyway?") + // No: the Open phase did not run — no open_here cd line and no "Open in:" menu. + assertNotContains(t, out, "cd -- '") + assertNotContains(t, out, "Open in:") + // Worktree + branch are still kept. + assertWorktreeExists(t, repo, "open-anyway-no") + assertBranchExists(t, repo, "open-anyway-no") +} + +// ptyAnswerLauncherSrc is the session-leader harness for the interactive +// open-anyway prompt. It allocates a PTY, becomes a session leader owning the +// slave as its controlling terminal, runs wt as a foreground child in wt's own +// (non-orphaned) process group with a failing init script wired in, feeds the +// requested answer to the open-anyway prompt, copies wt's PTY output to its own +// stdout (for the test to assert on), and exits with wt's exit code (or 30 on +// setup failure). +// +// argv: launcher +// env: WORKTREE_INIT_SCRIPT, WT_TEST_NO_LAUNCH, NO_COLOR, TMUX=, BYOBU_*, etc. +const ptyAnswerLauncherSrc = `package main + +import ( + "bytes" + "os" + "os/exec" + "os/signal" + "sync" + "syscall" + "time" + + "golang.org/x/sys/unix" +) + +func fail() { os.Exit(30) } + +func itoa(n int) string { + if n == 0 { + return "0" + } + var b [20]byte + i := len(b) + for n > 0 { + i-- + b[i] = byte('0' + n%10) + n /= 10 + } + return string(b[i:]) +} + +func main() { + if len(os.Args) < 5 { + fail() + } + answer, wtBin, repo, wtName := os.Args[1], os.Args[2], os.Args[3], os.Args[4] + + // Allocate a PTY. + mfd, err := unix.Open("/dev/ptmx", unix.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + fail() + } + if err := unix.IoctlSetPointerInt(mfd, unix.TIOCSPTLCK, 0); err != nil { + fail() + } + n, err := unix.IoctlGetInt(mfd, unix.TIOCGPTN) + if err != nil { + fail() + } + sfd, err := unix.Open("/dev/pts/"+itoa(n), unix.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + fail() + } + + master := os.NewFile(uintptr(mfd), "ptmx") + + // Drain the master into a buffer and mirror it to our stdout so the test can + // assert on wt's banner / prompt / cd-line output. + var mu sync.Mutex + var buf bytes.Buffer + go func() { + b := make([]byte, 4096) + for { + m, e := master.Read(b) + if m > 0 { + mu.Lock() + buf.Write(b[:m]) + mu.Unlock() + _, _ = os.Stdout.Write(b[:m]) + } + if e != nil { + return + } + } + }() + + // Become a session leader and acquire the slave as controlling terminal. + if _, err := unix.Setsid(); err != nil { + fail() + } + if err := unix.IoctlSetPointerInt(sfd, unix.TIOCSCTTY, 0); err != nil { + fail() + } + + slave := os.NewFile(uintptr(sfd), "pts") + + // Run wt as a child in its OWN process group (non-orphaned: this launcher is + // its live parent in the same session). NOT --non-interactive, so the + // open-anyway prompt is reachable; --worktree-name skips the name prompt; + // exploratory (no branch arg) skips the "Initialize worktree?" prompt; + // --worktree-open=open_here is the side-effect-free Open target. + cmd := exec.Command(wtBin, "create", "--worktree-name", wtName, "--worktree-open", "open_here") + cmd.Dir = repo + cmd.Stdin = slave + cmd.Stdout = slave + cmd.Stderr = slave + cmd.Env = os.Environ() + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + fail() + } + wpgid, err := unix.Getpgid(cmd.Process.Pid) + if err != nil { + _ = cmd.Process.Kill() + fail() + } + // Put wt's process group in the terminal foreground so wt's tcgetpgrp + // (terminalForeground) succeeds and reclaimTTY stays true — that is what + // makes the interactive open-anyway prompt reachable. The launcher's own + // group is the current foreground (it just did setsid + TIOCSCTTY), so this + // handoff does not SIGTTOU; ignore it defensively anyway. + signal.Ignore(syscall.SIGTTOU) + _ = unix.IoctlSetPointerInt(sfd, unix.TIOCSPGRP, wpgid) + + // Feed the answer to the open-anyway prompt. The prompt uses a line-buffered + // ReadString('\n') in cooked mode, so the bytes can be written ahead of the + // prompt being rendered — they wait in the PTY line buffer until wt reads + // them. Give wt a brief head start so the answer lands after the banner. + go func() { + time.Sleep(500 * time.Millisecond) + _, _ = master.Write([]byte(answer + "\n")) + }() + + // Wait for wt to exit, bounded by a deadline so a hang cannot wedge the test. + type waitResult struct { + ws unix.WaitStatus + err error + } + done := make(chan waitResult, 1) + go func() { + var ws unix.WaitStatus + _, e := unix.Wait4(cmd.Process.Pid, &ws, 0, nil) + done <- waitResult{ws: ws, err: e} + }() + + select { + case r := <-done: + if r.err != nil { + _ = syscall.Kill(-wpgid, syscall.SIGKILL) + fail() + } + // Let the drain goroutine flush any tail output. + time.Sleep(100 * time.Millisecond) + _ = master.Close() + if r.ws.Exited() { + os.Exit(r.ws.ExitStatus()) + } + // Signaled or stopped — not an expected wt outcome here. + os.Exit(30) + case <-time.After(20 * time.Second): + _ = syscall.Kill(-wpgid, syscall.SIGKILL) + fail() + } +} +` diff --git a/src/cmd/wt/create_test.go b/src/cmd/wt/create_test.go index fc000c4..397e942 100644 --- a/src/cmd/wt/create_test.go +++ b/src/cmd/wt/create_test.go @@ -627,6 +627,44 @@ func TestCreate_InitFailureBannerHasRetryHint(t *testing.T) { assertContains(t, r.Stderr, "&&") } +// TestCreate_InitFailureBannerHasGoHint asserts the banner now points the user +// at `wt go ''` for the kept worktree, on the non-interactive path. The +// hint lives in PrintInitFailureBanner so it appears on every caller path. +func TestCreate_InitFailureBannerHasGoHint(t *testing.T) { + repo := createTestRepo(t) + env := createFailingInitScript(t, repo) + + r := runWt(t, repo, env, "create", "--non-interactive", + "--worktree-name", "go-hint-test", + "--worktree-open", "skip") + + assertExitCode(t, r, 7) + assertContains(t, r.Stderr, "wt go 'go-hint-test'") +} + +// TestCreate_InitFailureNonInteractive_NoPrompt asserts the non-interactive +// init-failure path preserves today's exact behavior: banner + exit 7 with NO +// open-anyway prompt. The interactivity gate is !nonInteractive AND a TTY, so +// --non-interactive must never reach ConfirmYesNo. +func TestCreate_InitFailureNonInteractive_NoPrompt(t *testing.T) { + repo := createTestRepo(t) + env := createFailingInitScript(t, repo) + + r := runWt(t, repo, env, "create", "--non-interactive", + "--worktree-name", "noprompt-test", + "--worktree-open", "skip") + + assertExitCode(t, r, 7) + // The banner is shown (with the go hint)... + assertContains(t, r.Stderr, "wt go 'noprompt-test'") + // ...but the open-anyway prompt is NOT, and the Open phase did not run. + assertNotContains(t, r.Stdout, "Continue and open the worktree anyway?") + assertNotContains(t, r.Stderr, "Continue and open the worktree anyway?") + assertNotContains(t, r.Stdout, "cd -- '") + // Worktree survives. + assertWorktreeExists(t, repo, "noprompt-test") +} + func TestCreate_OpenHereSuppressesPath(t *testing.T) { repo := createTestRepo(t) diff --git a/src/internal/worktree/errors.go b/src/internal/worktree/errors.go index 4ababa8..5d8e360 100644 --- a/src/internal/worktree/errors.go +++ b/src/internal/worktree/errors.go @@ -130,6 +130,7 @@ func ExitWithError(code int, what, why, fix string) { // lives in one place and tests can assert against shape, not byte equality. const ( bannerLabelWorktree = "Worktree:" + bannerLabelGo = "Go: " bannerLabelRetry = "Retry: " bannerLabelRemove = "Remove: " bannerKeptNote = "(kept — git operations succeeded)" @@ -141,12 +142,19 @@ const ( // *exec.ExitError, otherwise a generic phrasing), the absolute worktree path // that was kept, a copy-paste-ready retry command, and a remove hint. // -// Banner order: +// Banner order (name/path values are single-quoted for copy/paste safety — +// see shellQuoteSingle and the assertions in errors_test.go): // 1. Status line ("init script exited with status N" / "did not complete") // + reminder that the init output streamed above. // 2. Worktree: (kept — git operations succeeded) -// 3. Retry: cd && wt init -// 4. Remove: wt delete +// 3. Go: wt go '' +// 4. Retry: cd '' && wt init +// 5. Remove: wt delete '' +// +// The Go/Retry/Remove hints are grouped after Worktree as the action hints. +// The Go hint points at the selection-free navigation command for the kept +// worktree; it lives in this shared helper so EVERY caller path (interactive +// yes/no AND non-interactive) gains the same discoverability. // // `&&` (never `;`) is used in the retry hint so it parses identically in // bash, zsh, and fish. @@ -164,8 +172,10 @@ func PrintInitFailureBanner(wtPath, name string, err error) { fmt.Fprintln(os.Stderr, statusLine) fmt.Fprintf(os.Stderr, " %s%s%s %s %s\n", ColorBold, bannerLabelWorktree, ColorReset, wtPath, bannerKeptNote) - // Single-quote both path and name so the hints stay correct when they - // contain spaces or shell metacharacters (e.g. macOS dirs with spaces). + // Single-quote path and name so the hints stay correct when they contain + // spaces or shell metacharacters (e.g. macOS dirs with spaces). + fmt.Fprintf(os.Stderr, " %s%s%s wt go '%s'\n", + ColorBold, bannerLabelGo, ColorReset, shellQuoteSingle(name)) fmt.Fprintf(os.Stderr, " %s%s%s cd '%s' && wt init\n", ColorBold, bannerLabelRetry, ColorReset, shellQuoteSingle(wtPath)) fmt.Fprintf(os.Stderr, " %s%s%s wt delete '%s'\n", diff --git a/src/internal/worktree/errors_test.go b/src/internal/worktree/errors_test.go index 2fc8397..7b94310 100644 --- a/src/internal/worktree/errors_test.go +++ b/src/internal/worktree/errors_test.go @@ -218,6 +218,9 @@ func TestPrintInitFailureBanner_ExitError(t *testing.T) { if !strings.Contains(out, "status 2") { t.Errorf("banner missing 'status 2' for *exec.ExitError:\n%s", out) } + if !strings.Contains(out, "wt go '"+name+"'") { + t.Errorf("banner missing 'wt go %s' navigation hint (single-quoted):\n%s", name, out) + } if !strings.Contains(out, "wt init") { t.Errorf("banner missing 'wt init' retry hint:\n%s", out) } @@ -249,6 +252,9 @@ func TestPrintInitFailureBanner_NonExitError(t *testing.T) { if !strings.Contains(out, wtPath) { t.Errorf("banner missing worktree path %q:\n%s", wtPath, out) } + if !strings.Contains(out, "wt go '"+name+"'") { + t.Errorf("banner missing 'wt go %s' navigation hint (single-quoted):\n%s", name, out) + } if !strings.Contains(out, "wt init") { t.Errorf("banner missing 'wt init' retry hint:\n%s", out) } @@ -285,6 +291,10 @@ func TestPrintInitFailureBanner_PathWithSpaces(t *testing.T) { if !strings.Contains(out, `wt delete 'my'\''name'`) { t.Errorf("remove hint must escape embedded single quotes in name:\n%s", out) } + // The Go: navigation hint single-quotes the name the same way. + if !strings.Contains(out, `wt go 'my'\''name'`) { + t.Errorf("go hint must escape embedded single quotes in name:\n%s", out) + } } // stripANSI removes CSI escape sequences (ESC [ ... letter) so the visible diff --git a/src/internal/worktree/menu.go b/src/internal/worktree/menu.go index d79fc60..8148baf 100644 --- a/src/internal/worktree/menu.go +++ b/src/internal/worktree/menu.go @@ -207,8 +207,11 @@ func runFallbackMenu(prompt string, options []string, defaultIdx int) (int, erro } // ConfirmYesNo prompts for a Y/n confirmation. Returns true if yes (default). +// The prompt is written to stderr (human-facing copy), keeping stdout reserved +// for machine-readable results even when the caller's stdout is redirected +// while stdin remains a TTY. func ConfirmYesNo(prompt string) bool { - fmt.Printf("%s [Y/n] ", prompt) + fmt.Fprintf(os.Stderr, "%s [Y/n] ", prompt) reader := bufio.NewReader(os.Stdin) line, err := reader.ReadString('\n') if err != nil {