Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions docs/memory/wt-cli/create-output-phases.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ description: "Phase-separator output contract for `wt create` / `wt init` — Gi
# wt-cli: Create Output Phases Contract

> Post-implementation behavior capture for the `wt create` / `wt init` phase-separator output.
> Source change: `260531-pnmi-add-phase-separators`.
> 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`).

This file documents the phase-separator output contract that `wt create` and `wt init` honor. Future changes touching `src/cmd/wt/create.go`, `src/cmd/wt/init.go`, or `src/internal/worktree/{crud.go,errors.go}` should preserve these invariants unless an explicit spec amendment supersedes them.
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.

## Requirements

Expand Down Expand Up @@ -51,6 +52,15 @@ This file documents the phase-separator output contract that `wt create` and `wt
- `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.

### `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.)
- 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: <msg>\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.
- **`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.

## Design Decisions

### Single helper in `errors.go` as the sole separator producer
Expand All @@ -65,14 +75,21 @@ The runner already prints `Running worktree init...` and already holds the resol

The convention captured by this change: **stdout = machine-readable result; stderr = diagnostics** (progress lines, banners, separators, prompts). `wt create` already followed it (stdout = the path line only). `wt init` used stdout for identical init diagnostics, an asymmetry that made separator placement inconsistent. Since `wt init` has no stdout contract (its tests assert on combined streams and no spec pins it to stdout), realigning it to stderr resolves the inconsistency and makes separator placement uniform across both commands. Keeping each command on its own stream was rejected — it enshrines the inconsistency.

### Single `wt.Warn` helper in `errors.go`, not a `cmd/`-local emitter (`260622-log5`)

**Decision**: a single `Warn(format, args...)` lives in `internal/worktree/errors.go` next to `PhaseSeparator`/`WtError`/`PrintInitFailureBanner`, and every `Warning:` call site in `create.go`/`delete.go` routes through it; the documentation home is this stream-discipline file (a note here), **not** a new dedicated memory file.
**Why**: `errors.go` already centralizes user-facing output formatting and owns the `NO_COLOR`-blanking `init()`, so `Warn` reuses that mechanism and collapses ~10 divergent call sites to one idiom (DRY, and keeps `cmd/` thin per Constitution V) — exactly the rationale that put `PhaseSeparator` here. The warning realignment is the *same* stdout=machine / stderr=human convention this file already canonicalizes (see the `wt init` realignment above), so it belongs as a note here rather than fragmenting the stream-discipline story across two files. A standalone `warning-output-contract.md` would be the smallest file in the bundle for one helper + a two-call-site realignment, against the per-command / per-shared-concern granularity of the sibling files (none of `errors.go`'s other emitters got their own file).
**Rejected**: a `cmd/`-local warning helper (pushes formatting into the thin command layer, can't reuse the package color vars); a dedicated `warning-output-contract.md` memory file (over-granular for a copy-polish change; splits the one stream-discipline contract in two).
*Introduced by*: `260622-log5-dx-copy-polish`.

### Fixed 40-column width, no terminal-size query

Deterministic output for unit tests; no dependency on terminal/ioctl; consistent with the single-binary, no-hidden-state posture (Constitution I). Dynamic width via terminal detection was rejected — non-deterministic in tests and adds a platform-specific code path for a cosmetic gain. (This mirrors `wt list`'s flag-based, no-`isatty` posture; see `list-status-contract.md`.)

## 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`, 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), `src/cmd/wt/init.go` (`runInitScript` Init separator + stderr realignment).
- 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); `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`).
- Constitution: Principle I (Single-Binary CLI, No Hidden State — motivated the fixed width, no terminal query), 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/list-status-contract.md` and `wt-cli/menu-navigation-contract.md` — same post-change invariant-capture pattern for sibling `wt` subcommands.
- 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).
- 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.
Loading
Loading