diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 634ad45..acc4122 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -17,10 +17,12 @@ Both ends of that wiring sit in named functions outside `runTUI` — `newRootMod and `restartIfRequested(final, restart)` — because `runTUI` needs a terminal and cannot be tested, and those two lines are the whole path from the model logic to the user. -Three areas carry more rationale than fits here — updating a tool, self-update, and panel -`[3]`'s README pipeline. Their full text lives under `docs/design/`: -[`updating.md`](docs/design/updating.md), [`self-update.md`](docs/design/self-update.md) -and [`readme-pipeline.md`](docs/design/readme-pipeline.md). +Four areas carry more rationale than fits here — updating a tool, self-update, panel +`[3]`'s README pipeline, and running a tool in the embedded terminal. Their full text +lives under `docs/design/`: [`updating.md`](docs/design/updating.md), +[`self-update.md`](docs/design/self-update.md), +[`readme-pipeline.md`](docs/design/readme-pipeline.md) and +[`tool-overlay.md`](docs/design/tool-overlay.md). ## Package map @@ -34,7 +36,7 @@ graph TD model[internal/model] --> loader[internal/loader] model --> version[internal/version] model --> updater[internal/updater] - model --> launcher[internal/launcher] + model --> term[internal/term] model --> ui[internal/ui] model --> proc[internal/proc] model --> logx @@ -44,6 +46,7 @@ graph TD version --> configdir updater --> loader updater --> proc + term --> proc ui --> loader loader --> logx loader --> configdir @@ -53,16 +56,16 @@ graph TD | Package | Responsibility | |---|---| | `internal/configdir` | Resolve the base user-config dir: `~/.config/keepkit` on macOS/Linux (`$XDG_CONFIG_HOME` else `~/.config`), `%AppData%\keepkit` on Windows. Pure `baseFor(goos, …)` core + `Base()` wrapper; stdlib-only bottom leaf shared by `loader`/`version`/`logx`/`main` | -| `internal/launcher` | Decide how to run a tracked tool in a new terminal tab: pure `planFor(env, command, toolName)` → `Plan{Argv, Fallback, Terminal}`, detection chain tmux → iTerm2 → Terminal.app → kitty → WezTerm → fallback; env-only, no subprocesses | | `internal/loader` | Tracker persistence (`meta.yaml`), status lifecycle (`active → trying → inactive`, legacy values migrated on read), the one-tag-per-tool invariant (a legacy multi-tag list is truncated to its first entry on read), GitHub ref parsing (`NormalizeRepo`, `ParseToolRef`) | | `internal/logx` | Session error journal: errors only, one lazily created file per session, imports only the stdlib-only `configdir` leaf. Package-level state — any package can log without threading a logger through | | `internal/model` | The entire Bubble Tea model: TUI state, key handling, rendering | | `internal/proc` | `DetachTTY` — run probes without a controlling terminal; `KillGroup` — process-group SIGKILL (plain `Kill` on Windows) | +| `internal/term` | Run a command on a pseudo-terminal and stream what it prints: `Start(shell, args, w, h, env)` over `x/xpty` (unix ptys, Windows ConPTY), one reader goroutine, `Events() <-chan Event` ending in a single `Exit{Err, Elapsed, Killed}`. A pty read error is never the verdict — `EIO`/EOF/`os.ErrClosed` are all normal termination and `xpty.WaitProcess` decides. Sets `Setsid`+`Setctty` itself (xpty does not), which is why `proc.DetachTTY` must never touch a pty command. No TUI knowledge, no config, no logging | | `internal/ui` | `Theme` — the app's ten semantic color roles — and `Styles`, the whole style set built from one theme by `NewStyles`; the two non-role palettes (`LanguageColor`, and `HeadingColors`/`ChromaColors` for panel `[3]`); `PlaceOverlay`, `StripANSI` | | `internal/updater` | Detect the package manager that owns an installed binary and produce an update `Plan{Manager, Argv, Display}` | | `internal/version` | Detect the installed version locally — `InstalledVersion(t) (ver, present)`; GitHub API with a 24-hour cache; semver comparison (`IsNewer`) and the card's version spelling (`DisplayVersion`); keepkit's own release check (`SelfRepo`, `SelfLatest`) | -`configdir`, `launcher`, `logx`, `proc`, `ui`, `updater` and `version` sit at the bottom of the import graph: +`configdir`, `logx`, `proc`, `term`, `ui`, `updater` and `version` sit at the bottom of the import graph: they know nothing about the TUI (`ui`, `updater` and `version` reach only into `loader`/`proc`/`logx`). `configdir` is the lowest leaf (stdlib only), shared by `loader`/`version`/`logx`/`main` for one config-dir resolution. GitHub ref parsing @@ -74,6 +77,7 @@ The `model` package is split across files within a single package: |---|---| | `model.go` | The `Model` struct, message types, `New`/`Init`/`Update`, selection and filtering helpers (`selectMeta`, `setFocus`, `searchMatches`, `filteredMeta`, `indexOfMeta`, `setHelpContent`) | | `mode.go` | The `inputMode` enum and a handler per input mode | +| `overlay_term.go` | The embedded tool terminal: the pty session interface, its messages and commands, the input relay, key translation, geometry and the frame renderer | | `commands.go` | All `tea.Cmd` constructors (fetch commands, update streaming) and re-fetch predicates | | `render.go` | `View`, panel/card/status-bar/gauge/overlay renderers, mouse handling. The two list/card builders return their line index alongside the text: `buildCard` → clickable lines, `buildToolRows` → the tool-index ↔ screen-line maps. Carries the single-entry `changelogRenderCache` — the card is rebuilt on every spinner frame, so the release-notes conversion must not repeat | | `readme.go` | `renderReadme` — panel `[3]`'s pipeline: sanitize → preprocess → glamour, with a single-entry render cache; `chromaFormatterFor` maps the color profile onto chroma's formatter so a code fence's plate is not quantized away from the card's | @@ -191,9 +195,10 @@ no focus and fetches nothing (see the layout invariant below). Focus moves with `1`/`2`/`3`, or a mouse click; everything goes through `setFocus(f)`, which repaints the tools list — the only viewport whose content depends on focus. -All modal state is a single field `m.mode inputMode` (12 values: `modeNormal`, `modeSearch`, +All modal state is a single field `m.mode inputMode` (13 values: `modeNormal`, `modeSearch`, `modeEditNote`, `modeEditTags`, `modeTrack`, `modeConfirmUntrack`, `modeRename`, -`modeRunInput`, `modeConfirmUpdate`, `modeAPIStatus`, `modeTokenInput`, `modeHotkeys`). Exactly one mode is active at +`modeRunInput`, `modeConfirmUpdate`, `modeAPIStatus`, `modeTokenInput`, `modeHotkeys`, +`modeToolOverlay`). Exactly one mode is active at a time; `Update()` dispatches via `switch m.mode`, so keys that open other modes structurally cannot fire inside another mode's input. @@ -345,7 +350,7 @@ path). Detection spawns subprocesses, so it runs as a `tea.Cmd`, never inside Five steps are path-convention based (cargo, pipx, uv, pnpm, bun) and take their roots from `managerDirsFrom(getenv, home, goos)` (pure core, `resolveManagerDirs()` -wrapper — the `launcher.planFor` idiom): `$UV_TOOL_DIR`, `$PNPM_HOME` and +wrapper — the `configdir.baseFor` idiom): `$UV_TOOL_DIR`, `$PNPM_HOME` and `$BUN_INSTALL` with per-platform defaults, plus home-derived `~/.cargo/bin` and `~/.local/pipx/venvs` (no `$CARGO_HOME`/`$PIPX_HOME` — unchanged behaviour, and a separate question from path resolution). Carrying all five is what lets @@ -469,26 +474,32 @@ succeed). ## Running a tool (`enter`) `enter` in `[1] Tools` opens a one-line prompt (prefilled with the last command -dispatched for the tool this session, else the tool name) and launches the -command. `launcher.Detect` picks the path from the environment alone — no -subprocesses, so unlike every probe it is safe inside `Update()`. A tab plan -runs its argv as a `tea.Cmd` through `proc.DetachTTY` with a 10-second ceiling -(`proc.KillGroup` on the process group when it fires — mostly for osascript -blocked on the macOS Automation dialog); a `Fallback` plan — terminals with no -scripting API, and native Windows — runs the command in the current window via -`tea.ExecProcess` (`sh -c` / `cmd /c`): keepkit suspends and resumes when the -tool exits. - -An adapter failure **auto-falls back** to `tea.ExecProcess`, so the tool still -launches — but only from `modeNormal`: the result can arrive seconds after -enter, and seizing the terminal under an open editor or overlay would send the -user's keystrokes to the spawned shell. Under any other mode the fallback is -deferred, not dropped: it fires — with a visible status message — on the -keystroke that closes the mode, going straight to the exec fallback (the -failing adapter plan is never re-run). One adapter launch runs at a time (`m.launchingFor`, the -launch twin of `updatingFor`). Working directories differ by path: a tab opens -in the new shell's default cwd, the fallback inherits keepkit's. A non-zero -exit of the tool itself is a status message only — never logged. +dispatched for the tool this session, else the tool name) and runs the command +on a **pseudo-terminal inside keepkit** — a bordered block over the dimmed +layout, with the whole keyboard handed to the tool. One path for everything: a +TUI draws and lives in it, a plain CLI prints and exits and its final screen +stays until `esc`. + +`internal/term` owns the pty and one reader goroutine; the VT emulator lives on +the model, so only `Update` ever touches its screen state. While the tool runs +every key is forwarded — `esc` and `ctrl+c` included, because `esc` is what +makes vim usable and `ctrl+c` is the tool's interrupt. `ctrl+\` is the one +reserved chord: it kills the process group and stays in the mode, since the +outcome line arrives with the exit. Afterwards `esc` closes and nothing else +does anything. + +The block is 70% of the screen with its exit row reserved from the start and +every row padded to the body width, so it never moves when the tool finishes. A +screen too small refuses the keypress and remembers nothing; a terminal shrunk +mid-session clamps to the floor and keeps the tool running. The working +directory is keepkit's. A non-zero exit of the tool itself is shown in the exit +row — never logged. + +This replaced a tab launcher that scripted other terminals (tmux, iTerm2, +Terminal.app, kitty, WezTerm) into opening a tab, plus the `tea.ExecProcess` +fallback and the deferred-fallback machinery that existed to survive an adapter +failing. Full rationale, including the upstream races it works around, in +[`docs/design/tool-overlay.md`](docs/design/tool-overlay.md). ## GitHub API @@ -578,8 +589,8 @@ leaves no file, so the presence of a file is itself the signal. The filename car colon-free zero-padded timestamp: lexicographic order equals chronological order, which is what `Cleanup()` relies on (the 20 most recent are kept). `logx.Recover` is hooked deeper than Bubble Tea's own recover (inside `Update`, `View` and every -command via `safeCmd`; `execToolCmd` is the one unwrapped cmd — `tea.ExecProcess` -only constructs the exec message, nothing there can panic): it records the panic +command via `safeCmd`; the one unwrapped cmd is `handleTermChunk`'s exit +re-emit, a closure returning a prebuilt value that cannot panic): it records the panic with a stack trace and **re-panics** so Bubble Tea restores the terminal correctly. The logger's own failures are swallowed silently. diff --git a/CLAUDE.md b/CLAUDE.md index 42b5fbb..3df0d48 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,32 +19,33 @@ CI (`.github/workflows/ci.yml`) runs build / vet / `test -race` / golangci-lint **`keepkit`** is a terminal TUI tracker for CLI tools built with Bubble Tea. It is a pure TUI app — running `keepkit` launches the interface directly; the only CLI surface is `--version`/`--help`. -**Deep design docs.** Three features carry more rationale than belongs in a file loaded on every session, so their full text lives under `docs/design/` and this file keeps the invariants plus a link. **Read the linked file before changing anything in its area** — each one records what already broke once and why the current shape is the way it is: +**Deep design docs.** Four features carry more rationale than belongs in a file loaded on every session, so their full text lives under `docs/design/` and this file keeps the invariants plus a link. **Read the linked file before changing anything in its area** — each one records what already broke once and why the current shape is the way it is: | File | Covers | |---|---| | [`docs/design/updating.md`](docs/design/updating.md) | `enter` in `[2]`: `updater.Detect`'s manager chain, the confirm dialog, the streaming update log | | [`docs/design/self-update.md`](docs/design/self-update.md) | `U`/`X`: the version gate, `selfState`, the restart and its path resolution | | [`docs/design/readme-pipeline.md`](docs/design/readme-pipeline.md) | panel `[3]`: the three sources, `cleanReadmeMarkdown`, the glamour theme | +| [`docs/design/tool-overlay.md`](docs/design/tool-overlay.md) | `enter` in `[1]`: the embedded pty, the Update-only emulator rule, key translation, `ctrl+\`, the geometry | -Everything else stays here. **Never re-inline these three sections.** When a change makes one of them wrong, fix it in its own file, and touch this one only when the invariant summary itself went wrong — the split is the layout, not drift. +Everything else stays here. **Never re-inline these four sections.** When a change makes one of them wrong, fix it in its own file, and touch this one only when the invariant summary itself went wrong — the split is the layout, not drift. ### Entry point `main.go` is a thin launcher: it loads tracker metadata via `loader.LoadMeta()` and starts the Bubble Tea TUI with `model.New(meta).WithAppVersion(ver)` — the running binary's own version, which is what the self-check compares against keepkit's latest release and the gate that switches that whole feature off on a dev build (see **Self-update** below). Before that, `handleCLI` answers `--version`/`-V`/`-v`/`version` (prints `keepkit ` — dotted-numeric on release/`go install` builds, so `version.InstalledVersion`'s regex parses it and a keepkit tracked inside keepkit shows as installed) and `--help`/`-h`/`help` (static usage text); **any other argument exits 2 with usage on stderr** — falling through to the TUI is what used to make a probed keepkit boot Bubble Tea on a detached TTY, fail with `could not open a new TTY`, and litter `logs/` with junk session files. There are no other subcommands or flags. -`runTUI` keeps the model `p.Run()` returns (it used to discard it) for one reason: `RestartRequested()` is how `[U] restart` after a self-update reaches `restartSelf()`. `runTUI` itself opens a Bubble Tea program on a TTY and cannot be tested, so — the same pure-core-plus-thin-wrapper split as `shellCommand`/`planFor`/`resolveSelfPath` — the two lines that carry the whole self-update feature from the model to the user live outside it: **`newRootModel(meta, ver)`** (the `model.New(meta).WithAppVersion(ver)` wiring, without which every shipped binary would silently have no self-check, no banner and no restart offer) and **`restartIfRequested(final, restart)`** (the post-`p.Run()` decision, with the restart function injected exactly like `restartSelfWith`'s `execve`). Both had zero coverage until a review mutation showed the full suite staying green with each of them removed; `TestNewRootModelInjectsAppVersion` (counts `Init`'s batch — a release version queues one command more than a dev one) and `TestRestartIfRequested` are what kill those mutations now, and deleting the `restartIfRequested` call from `runTUI` is a compile error because `final` would go unused. `restartIfRequested` asserts on a local **`restarter` interface**, not on `model.Model`: the flag is only reachable through unexported state, so a stand-in is the only way to exercise the true branch — the package-level `var _ restarter = model.Model{}` is what keeps the real model bound to it. Still uncovered in `main.go` and known to be so: `buildVersion()` (its pure `resolveVersion` core is tested, the `debug.ReadBuildInfo` wrapper is not), the `logx.SetHeader` format strings, `migrateConfigDir` and `main`'s own `handleCLI` dispatch. The root package carries three more files for the restart, and each one's build tag matches where its code is actually reachable: `restart.go` — **only** the shared `restartHint` const, the one thing both platforms use; `restart_unix.go` (`//go:build !windows`) — `restartSelf`/`restartSelfWith` (`syscall.Exec` over `os.Args`/`os.Environ()`) plus the pure `resolveSelfPath` core and its `selfPath()`/`fileExists` wrappers, all of which the Windows build never calls; `restart_windows.go` — the honest degradation (print the hint, exit). Keeping the path core out of the untagged file is why `unused` no longer depends on a test to see it referenced. +`runTUI` keeps the model `p.Run()` returns (it used to discard it) for one reason: `RestartRequested()` is how `[U] restart` after a self-update reaches `restartSelf()`. `runTUI` itself opens a Bubble Tea program on a TTY and cannot be tested, so — the same pure-core-plus-thin-wrapper split as `shellCommand`/`baseFor`/`resolveSelfPath` — the two lines that carry the whole self-update feature from the model to the user live outside it: **`newRootModel(meta, ver)`** (the `model.New(meta).WithAppVersion(ver)` wiring, without which every shipped binary would silently have no self-check, no banner and no restart offer) and **`restartIfRequested(final, restart)`** (the post-`p.Run()` decision, with the restart function injected exactly like `restartSelfWith`'s `execve`). Both had zero coverage until a review mutation showed the full suite staying green with each of them removed; `TestNewRootModelInjectsAppVersion` (counts `Init`'s batch — a release version queues one command more than a dev one) and `TestRestartIfRequested` are what kill those mutations now, and deleting the `restartIfRequested` call from `runTUI` is a compile error because `final` would go unused. `restartIfRequested` asserts on a local **`restarter` interface**, not on `model.Model`: the flag is only reachable through unexported state, so a stand-in is the only way to exercise the true branch — the package-level `var _ restarter = model.Model{}` is what keeps the real model bound to it. Still uncovered in `main.go` and known to be so: `buildVersion()` (its pure `resolveVersion` core is tested, the `debug.ReadBuildInfo` wrapper is not), the `logx.SetHeader` format strings, `migrateConfigDir` and `main`'s own `handleCLI` dispatch. The root package carries three more files for the restart, and each one's build tag matches where its code is actually reachable: `restart.go` — **only** the shared `restartHint` const, the one thing both platforms use; `restart_unix.go` (`//go:build !windows`) — `restartSelf`/`restartSelfWith` (`syscall.Exec` over `os.Args`/`os.Environ()`) plus the pure `resolveSelfPath` core and its `selfPath()`/`fileExists` wrappers, all of which the Windows build never calls; `restart_windows.go` — the honest degradation (print the hint, exit). Keeping the path core out of the untagged file is why `unused` no longer depends on a test to see it referenced. ### Package overview | Package | Purpose | |---|---| -| `internal/configdir` | Resolve keepkit's base user-config dir (parent of the `keepkit/` subdir holding `meta.yaml`, `cache.json`, `token`, `logs/`). Pure `baseFor(goos, getenv, userConfigDir, userHomeDir)` core + thin `Base()` wrapper (the `shellCommand`/`planFor` idiom). **Windows** → `os.UserConfigDir()` (`%AppData%`); **macOS and Linux** → `$XDG_CONFIG_HOME` else `~/.config` — deliberately *not* `os.UserConfigDir()`, which is `~/Library/Application Support` on macOS. The **bottom leaf of the import graph** (stdlib only), so `loader`/`version`/`logx`/`main` all share one resolution without a cycle | -| `internal/launcher` | Decide how to run a tracked tool in a new terminal tab: pure `planFor(env, command, toolName)` over an injected env lookup → `Plan{Argv, Fallback, Terminal}`, thin `Detect` wrapper over `os.Getenv`. Detection chain (first hit wins): `$TMUX` → iTerm2 → Terminal.app → kitty → WezTerm → `Fallback: true` (no scripting API — run in the current window). `$TMUX` is deliberately first: inside tmux `TERM_PROGRAM` names the *outer* terminal and a tmux window is the correct "tab" there. Terminal.app opens a **window**, not a tab (tabs aren't scriptable without System Events — honest degradation). tmux/kitty/wezterm plans carry command and tool name as argv elements (no escaping; the tmux plan puts `--` before the command so a `-`-leading edit isn't eaten as a flag); the two osascript plans interpolate into script source through `appleScriptQuote` (backslashes, then double quotes, then `\n`/`\r`/`\t` — the control characters an AppleScript literal cannot carry raw), the single escaping point. The kitty plan needs a remote-control socket (`listen_on` → `KITTY_LISTEN_ON`): the adapter runs detached, so tty-transport remote control cannot work — without the socket it degrades to the auto-fallback. Env-only — no subprocesses — so `Detect` is safe inside `Update()`. Bottom of the import graph like `updater`: no TUI knowledge | +| `internal/configdir` | Resolve keepkit's base user-config dir (parent of the `keepkit/` subdir holding `meta.yaml`, `cache.json`, `token`, `logs/`). Pure `baseFor(goos, getenv, userConfigDir, userHomeDir)` core + thin `Base()` wrapper (the `shellCommand` idiom). **Windows** → `os.UserConfigDir()` (`%AppData%`); **macOS and Linux** → `$XDG_CONFIG_HOME` else `~/.config` — deliberately *not* `os.UserConfigDir()`, which is `~/Library/Application Support` on macOS. The **bottom leaf of the import graph** (stdlib only), so `loader`/`version`/`logx`/`main` all share one resolution without a cycle | | `internal/loader` | Persist tracker metadata (`meta.yaml`: name, status, tag, note, github ref, optional `update_cmd` override); own the tool-status lifecycle (`active → trying → inactive` via `NextStatus`; legacy `forgotten`/`archived` values are migrated to `inactive` in `LoadMeta` — in-memory, the file keeps the old value until the next `SaveMeta`); hold the **one tag per tool** invariant (`Tags` stays a `[]string` so the yaml schema is unchanged, but `LoadMeta` truncates a legacy multi-tag list to `Tags[:1]` in the same migration loop — first tag wins, matching what the editor's `parseTag` does to comma-separated input. Unlike the status migration, which swaps a retired value for its successor, this one **discards user-authored data** and the next `SaveMeta` — any note edit or status cycle — makes it permanent, so a load that actually dropped tags stashes the pre-migration file as `meta.yaml.bak` first: best-effort, logged and swallowed on failure, and not rewritten by later already-migrated loads); parse GitHub refs (`NormalizeRepo`, `ParseToolRef` in `github.go`) | | `internal/logx` | Errors-only session logger (imports only the stdlib-only `configdir` leaf); one lazily-created plain-text file per session under `/keepkit/logs`. Package-level state (`mu`/`file`/`path`/`header`), so any package can log without threading a logger through constructors | | `internal/model` | Entire Bubble Tea model — all TUI state, key handling, and rendering | | `internal/proc` | `DetachTTY` — runs tool probe subprocesses without a controlling terminal (`Setsid` on unix, `DETACHED_PROCESS` on Windows); `KillGroup` — process-group SIGKILL (negative pid; plain `Process.Kill` on Windows) for the update streamer's timeout path | +| `internal/term` | Run a command on a pseudo-terminal and stream what it prints — the architectural slot `internal/launcher` vacated: bottom of the import graph, no TUI knowledge. `Start(shell, args, w, h, env)` creates the pty (`x/xpty`: unix ptys via creack/pty, Windows **ConPTY**) and one reader goroutine; everything it observes leaves through `Events() <-chan Event` — zero or more `Data`, then exactly one `Exit{Err, Elapsed, Killed}`, then close — so the model consumes it with the update streamer's `waitForChunkCmd` pattern. The channel is buffered (64) so the reader stays ahead of a once-per-message consumer; past it the child is throttled, which is the correct back-pressure. **A pty read error is never the verdict** (`EIO` on Linux, EOF on macOS, `os.ErrClosed` after a Kill — all normal termination): it comes from `xpty.WaitProcess`, which also synthesises the `*exec.ExitError` `os.Process.Wait` fails to produce on ConPTY. `Elapsed`/`Killed` are stamped in the goroutine, never by the consumer. **xpty sets neither `Setsid` nor `Setctty`** — its `Start` only wires stdio — so this package sets them itself on the unix build, which is also why **`proc.DetachTTY` must never touch the pty command**: it assigns `SysProcAttr` wholesale and would drop them. `Setsid` is what makes `proc.KillGroup`'s negative-pid signal reach an `sh -c` line's grandchildren. The caller builds argv (the model's `shellCommand`), keeping this package goos-agnostic. No config paths, no `logx` — failures ride `Exit.Err` — hence no `TestMain` seam | | `internal/ui` | `Theme` (theme.go) — the app's ten semantic color roles plus the gauge's dim track — and `Styles` (styles.go), every lipgloss style keepkit renders with, built from one theme by the single `NewStyles(Theme)`. `DefaultStyles()` is the fallback for a `Model{}` literal and nothing else. **Two** palettes here are **not** theme roles and must not follow a theme switch, and they are exceptions for different reasons: `LanguageColor` (lang.go) is linguist's per-language brand marks, keyed case-folded, `(color, false)` for a language it does not know — somebody else's colors, so keepkit may not restyle them; `HeadingColors`/`ChromaColors` (readme_palette.go) are keepkit-*invented* shades for panel `[3]`'s heading ladder and code-fence accents, a contained exception to "color is a role, not a shade" taken because a heading *level* is not a meaning `Theme` has a word for and six levels cannot be six meanings. The consequence is stated in all three of theme.go, readme_palette.go and `keepkitStyle`: a theme switch repaints `[3]`'s body, links, quotes and inline code and **not** H1–H5 or the fence accents. `HeadingColors` **descends** — bright H1 to dark H5 — and `TestHeadingColorsDescend` pins that direction, because reordering it is a one-line edit that breaks nothing else. Plus `PlaceOverlay` (which takes the dim style rather than reading a package var) and `StripANSI` | | `internal/updater` | Detect the package manager that owns an installed binary and produce an update `Plan{Manager, Argv, Display}` (brew → go → cargo → pipx → uv → pnpm → bun → npm chain; `update_cmd` override always wins; on a `LookPath` miss **or an exhausted chain**, a brew-by-name fallback before giving up — see [`docs/design/updating.md`](docs/design/updating.md)). Bottom of the import graph like `version`: no TUI knowledge, depends only on `loader` for `Tool`. Pure `detectFromPath`/`brewNamePlanAt`/`managerDirsFrom`/`pnpmShimTarget` cores + OS-facing `Detect`/`brewNamePlan`/`resolveManagerDirs`/`readPnpmShim` wrappers, plus the wrapper-less goos-parameterized `customPlan(goos, cmd)` (`update_cmd` → `sh -c`, `cmd /c` on Windows so a winget/PowerShell command needs no Git Bash) — **a deliberate duplicate of `model.shellCommand`** for `testBrewPrefix`'s reason (`updater` sits below `model` and may not import it); the cross-reference comments on both copies are the only drift guard, and both carry the same accepted caveat — Go's argv quoting is not cmd.exe-aware, so an `update_cmd` embedding double quotes can misparse under `cmd /c`. Seams: `testHomeDir`, `testBrewPrefix` (a deliberate duplicate of `version`'s — two bottom leaves that may not import each other) and `testGOOS`, which exists because **CI runs `go test` on linux only** (the Windows job cross-compiles), so an expectation derived from `runtime.GOOS` asserts nothing about the Windows branch — `TestDetectUpdateCmdOverride` drives the seam per row and is what catches `Detect` passing a literal goos, the regression this package shipped once | | `internal/version` | Detect installed version locally — `InstalledVersion(t) (ver string, present bool)`, the two results independent so the card can tell "installed but won't say its version" from "not installed". Sources in order: `--version`/`-V`, then `brewDirVersion` in `brew.go` (reads the version from the `Caskroom/`/`Cellar/` directory names — no brew subprocess — so casks with no version CLI still resolve), then `cargoListVersion`/`cargoVersionFromList` (same idea one ecosystem over: `cargo install --list` names every cargo-installed crate's version without running its binary; gated on the binary existing, and `LookPath("cargo")` short-circuits before any subprocess). A fallback hit suppresses the anomaly log; `testBrewPrefix` seam. Also: fetch latest release, repo card, changelog and README from the GitHub API with a 24h cache; semver comparison (`IsNewer`) and the card's version spelling (`DisplayVersion`); keepkit's own self-check (`selfcheck.go`: `SelfRepo`, `SelfLatest`) | @@ -54,8 +55,9 @@ The `model` package is split by responsibility (one package, several files): | File | Contents | |---|---| | `model.go` | `Model` struct, msg types, `New`, `Init`, `Update` dispatch, selection/filter helpers | -| `mode.go` | `inputMode` enum + per-mode key handlers (`updateNoteEdit`, `updateTagsEdit`, `updateTrackInput`, `updateUntrackConfirm`, `updateRenameInput`, `updateRunInput`, `updateAPIStatus`, `updateConfirmUpdate`, `updateHotkeys`) and the pure `trackTool`/`renameTool` | -| `commands.go` | Every `tea.Cmd` constructor (`fetchInstalledCmd`, `remoteCmd`, `fetchRateCmd`, `selfCheckCmd`, `changelogCmd`, `fetchHelpCmd`, `readmeCmd`/`fetchReadmeCmd`/`refreshReadmeCmd`, `validateTokenCmd`, `detectUpdateCmd(t, self)` (one command for `enter` and `[U]` — the flag only tags the message), `startUpdateCmd`, `waitForChunkCmd`, `startLaunchCmd`, `execToolCmd` + the pure per-GOOS `shellCommand`) + fetch predicates (`needsInstalled`, `needsRemote`, `needsReadme`, `refreshSelectedCmd`, `autoFetchCmdsForSelected`) | +| `mode.go` | `inputMode` enum + per-mode key handlers (`updateNoteEdit`, `updateTagsEdit`, `updateTrackInput`, `updateUntrackConfirm`, `updateRenameInput`, `updateRunInput`, `updateAPIStatus`, `updateConfirmUpdate`, `updateHotkeys`) and the pure `trackTool`/`renameTool`. `updateToolOverlay` and its key translation live in `overlay_term.go` | +| `overlay_term.go` | The embedded tool terminal (`modeToolOverlay`): the `termSession` narrow interface, the `termStarted`/`termChunk`/`termExit` messages, `startTermCmd`/`waitForTermChunkCmd`, the `termInput` relay and its lifecycle, `updateToolOverlay` + key translation, `termGeometry` and the frame renderer | +| `commands.go` | Every `tea.Cmd` constructor (`fetchInstalledCmd`, `remoteCmd`, `fetchRateCmd`, `selfCheckCmd`, `changelogCmd`, `fetchHelpCmd`, `readmeCmd`/`fetchReadmeCmd`/`refreshReadmeCmd`, `validateTokenCmd`, `detectUpdateCmd(t, self)` (one command for `enter` and `[U]` — the flag only tags the message), `startUpdateCmd`, `waitForChunkCmd` + the pure per-GOOS `shellCommand`, which now builds argv for the tool overlay's pty) + fetch predicates (`needsInstalled`, `needsRemote`, `needsReadme`, `refreshSelectedCmd`, `autoFetchCmdsForSelected`) | | `render.go` | `View`, panel/card/status-bar/gauge/overlay renderers, scrollbar, mouse handling | | `readme.go` | `renderReadme` — panel `[3]`'s pipeline (sanitize → preprocess → glamour) + the single-entry `readmeRenderCache`; `testReadmeStyle` seam | | `readme_clean.go` | `cleanReadmeMarkdown(text, about)` — the pure README preprocessor: fenced-block/inline-span segmentation first, then the image/link/HTML/emoji/shortcode removal rules on what is left, then `rcDropTitleBlock` | @@ -96,7 +98,7 @@ The model is a three-panel layout with focus cycling via `→/←` between `focu **Tag grouping (`space` in `focusTools`)**: the second list view, off by default (`m.groupByTag`). `m.grouped()` — `groupByTag && searchQuery() == "" && hasTaggedTool()` — is the single "tag view is on" predicate, shared by the ordering and the header rows so they can never disagree; an active `/` search suppresses it, so search behaves exactly as before, and so does a tracker with **no** tagged tool (grouping it would draw one `untagged` divider over the unchanged list — a keypress that reads as broken). `toggleGroupByTag` refuses to turn the view *on* in that state with a `no tags to group by — press [#] on a tool` statusMsg (it named `[t]` until the tag editor moved off that key), but never refuses to turn it **off** — otherwise removing the last tag would strand the user in the tag view; a successful toggle reports `grouped by tag` / `flat list`, the only feedback a view switch has. All three exits route through `setStatus`, so `toggleGroupByTag` returns a `tea.Cmd` (the expiry tick) that the `space` case returns — the message auto-clears after `statusMsgTTL` (see the status-message lifecycle below); a view toggle still fetches nothing. Ordering lives in the same single projection point: `searchMatches()` returns early through `groupMatchesByTag(out)`, which puts tools sharing a tag together (groups in first-appearance order, `meta.yaml` order inside a group, untagged last). The grouping identity is **`tagKey(mt)` — the case-folded tag**, because `matchingTag` already compares case-insensitively and two spellings the search treats as one tag must not split into two sections; the header shows the group's first spelling. Grouping and the update partition are **exclusive** — the partition would break the sections' contiguity — so in the tag view an updatable tool stays in its group and only keeps its ` ↑` marker. **`metaSelected` stays a tool index into `filteredMeta()`**, never a screen row: that is what keeps navigation (`j/k/g/G/PgUp/PgDn/ctrl+d/u`), `selectedMeta()`, `indexOfMeta` and every index-writing site untouched by the feature. The header rows exist only in the two translation points, via maps built by `buildToolRows()` alongside the content — `toolLine[i]` (tool index → screen line) and `lineTool[l]` (screen line → tool index, `-1` on a header), both the identity when grouping is off: `syncToolsViewport` scrolls to `selectedLine()` (**both** branches in screen-line units — mixing units breaks scrolling the moment a header sits above the selection; the upward clamp additionally pulls in a header sitting directly above the selection, or the first group would render as if it had none, with no keyboard way to bring it back), `handleMouse` maps a click row through `toolAtLine()` (a header click selects nothing but still focuses the panel), and the **page/half-page keys step screen lines, not tools** — their step is a viewport-row count, so `PgUp/PgDn/ctrl+f/ctrl+b/ctrl+d/ctrl+u` go through `toolNearLine(selectedLine() ± step, dir)`, which lands on the tool at that line or the next one in the direction of travel. Counting rows as tools would jump past the rows the headers occupied. Both helpers fall back to the identity when the maps are empty, so a hand-built model that never painted the list behaves as it did before. **Every repaint of the list content must go through `setToolsContent()`** — it writes the maps for the content it just rendered, so a bare `SetContent(renderLeftContent())` elsewhere would leave them describing the previous list (the `WindowSizeMsg` handler was exactly that and now routes through it; `TestWindowSizeRebuildsLineMaps` guards it). The toggle itself (`toggleGroupByTag`) reorders `filteredMeta()`, so it uses the same capture-name-then-`indexOfMeta` remap as the async handlers — deliberately not via `selectMeta`, since a pure view toggle must fire no auto-fetch. -**Input modes**: all input/modal state lives in one `m.mode inputMode` field (`mode.go`) — `modeNormal` (base), `modeSearch`, `modeEditNote`, `modeEditTags`, `modeTrack`, `modeConfirmUntrack`, `modeRename`, `modeRunInput`, `modeConfirmUpdate`, `modeAPIStatus`, `modeTokenInput`, `modeHotkeys`. Exactly one mode is active at a time; the `tea.KeyMsg` branch in `Update()` dispatches on `switch m.mode`, so a non-normal mode's handler owns the input and other modes' opening keys cannot fire (the old per-flag guard bugs are structurally impossible). `modeTokenInput` is a sub-state of the API-status overlay: entered from `modeAPIStatus` via `[e]`, esc returns to `modeAPIStatus`, and `apiOverlayVisible()` reports "overlay on screen" for both. `overlayVisible()` is the broader predicate — `apiOverlayVisible() || modeHotkeys` — used by `View()` and the mouse gate to mean "any modal on screen". `refreshingFor`, `helpMode`, `focus` and `selfState` (the self-update banner is non-modal by design — the whole app keeps working under it) are deliberately *not* input modes and stay separate fields. +**Input modes**: all input/modal state lives in one `m.mode inputMode` field (`mode.go`) — `modeNormal` (base), `modeSearch`, `modeEditNote`, `modeEditTags`, `modeTrack`, `modeConfirmUntrack`, `modeRename`, `modeRunInput`, `modeConfirmUpdate`, `modeAPIStatus`, `modeTokenInput`, `modeHotkeys`, `modeToolOverlay`. Exactly one mode is active at a time; the `tea.KeyMsg` branch in `Update()` dispatches on `switch m.mode`, so a non-normal mode's handler owns the input and other modes' opening keys cannot fire (the old per-flag guard bugs are structurally impossible). `modeTokenInput` is a sub-state of the API-status overlay: entered from `modeAPIStatus` via `[e]`, esc returns to `modeAPIStatus`, and `apiOverlayVisible()` reports "overlay on screen" for both. `overlayVisible()` is the broader predicate — `apiOverlayVisible() || modeHotkeys || modeToolOverlay` — used by `View()` and the mouse gate to mean "any modal on screen". `refreshingFor`, `helpMode`, `focus` and `selfState` (the self-update banner is non-modal by design — the whole app keeps working under it) are deliberately *not* input modes and stay separate fields. - **Tool-list search (`/` in `focusTools`)** is a commit/rollback transaction over `modeSearch`. `case "/"` captures `m.searchPrevName` (the selected tool's name; empty when the list is empty) before entering the mode, and `filteredMeta()` narrows the list live as the query changes. The predicate is `searchMatches()` (`model.go`): a tool matches when its **name OR its tag** contains the lowercased query (`matchingTag` still iterates the slice — it predates the one-tag invariant and stays correct under it), returning `[]searchMatch{meta, byTagOnly, tag}` so the renderer knows which rows matched only by tag; `filteredMeta()` is a thin projection over it, so all other callers (count, selection, cursor remap) keep seeing plain metas. While searching, the matched name substring renders peach-bold via `highlightNameMatch` (render.go), tag-only rows show the earning tag as a dim `#` suffix when it fits the row budget without wrapping, and the search status bar shows an `N/M` counter (matches / total tracked) between the query and the hints (a keystroke that changes the query text resets `metaSelected` to 0 — first match highlighted, marker visible during search; pure cursor movement like `left`/`right` keeps a user-moved highlight; that reset repaints `[3]` through **`setHelpContent()`**, not a bare `SetContent(renderHelpContent())`: the readme branch serves `m.helpBase`, which nothing else re-renders, so the cheaper call would leave the *previous* tool's README on screen under the new tool's name — no fetch is fired there, one per keystroke would spend the quota on rows merely typed past). Inside the mode: `↑`/`↓` move the highlight through the filtered list via `selectMeta` (modular wrap, full `j`/`k` parity; **never** forwarded to the textinput, so the query text is untouched — with zero matches they are consumed as no-ops); `enter` commits — exits to `modeNormal`, clears the query, remaps the cursor onto the unfiltered (but still update-grouped) list by name via `indexOfMeta(mt.Name)` — the search filter is gone once the mode is normal, but the update grouping is not, so `indexOfMeta` resolves the **displayed** index — and moves focus to `focusBrief` (no matches → no-op, search stays open); `esc` rolls back — unfiltered list with the cursor restored via `indexOfMeta(m.searchPrevName)` (fallback 0 when that tool was untracked mid-search). Both exits clear `searchPrevName` and go through `selectMeta`, so the help panel is re-synced too (an arrow move may have loaded another tool's help mid-search). `indexOfMeta(name)` lives next to `filteredMeta()` in `model.go`; the status bar echoes the live query plus the `N/M` counter and `[enter] open [↑/↓] move [esc] cancel` hints. - **Central panel actions (`focusBrief`)** operate on the data the card already shows: **`enter` installs the release the card is offering** (the panel's primary action, and the mirror of `enter`-runs-a-tool in `[1]` — same key, because in both panels it is the thing the user came to that panel to do; see **Update** below), `o` opens the repo in the browser, `c` opens the changelog/releases page, `r` force-refreshes the tool's data, `s` cycles the status (`loader.NextStatus`: `active → trying → inactive`, unknown values fall back to active), `e` edits the note, `#` edits the tool's single tag. **`#` rather than `t`**: `t` is now the global track verb, and a tag editor reachable only from the card is where the tag is shown. `o`/`c` go through `openURLCmd` (resolved per-`GOOS` by `browserCommand`); a tool with no `GitHub` sets `m.statusMsg` instead of launching. `s`/`e`/`#` mutate `m.meta` via `loader.UpsertMeta`, persist with `loader.SaveMeta`, then refresh the card with `m.briefViewport.SetContent(m.renderCard())`. The tags editor commits through `parseTag` (mode.go): the input is **one** tag — everything past the first comma is dropped, so typing `cli, foo` and loading a legacy `[cli, foo]` list both land on `cli` and the editor can never disagree with `LoadMeta`'s `Tags[:1]` migration about a tag's shape. Spaces inside a tag are kept (`dev tools`); empty input clears it to `nil`, which `omitempty` drops from `meta.yaml`. Everything downstream reads the one tag through `tagOf(mt)` rather than joining the slice. @@ -106,7 +108,7 @@ The model is a three-panel layout with focus cycling via `→/←` between `focu - **Card metrics strip**: `metricsStrip` is what the card's `[info]` section became — installed / latest / maintenance / stars laid out as **captioned columns on the `Theme.Surface` background** instead of six `label: value` lines whose labels ran down the left edge and pushed every value into a column of its own. Captions are uppercase (`INSTALLED`, `LATEST`, `MAINTENANCE`, `STARS`) because a terminal has no smaller type size to demote a label with, and the values are what the eye should land on. **`installed:` still has four states** and the two version-less ones stay distinct: a resolved version in `Text`, `✓ present` in `Ok` (a tool that is installed but won't name its version — a ratatui app that ignores `--version` — is a working install and reads affirmative), `✕ missing` in `Danger` (the one thing on the card that is actually wrong), and `detecting…` in `Dim` while the local probe is in flight. Both version-less values are **one word**, because the caption above them already says INSTALLED and the sentences they used to be were the only values in the strip too wide for a baseline-width column. `latest:` renders in `SignalBold` with a trailing ` ↑` when `hasUpdate`, otherwise `Text`, and the release date is a **second line under it** rather than a suffix on it. **The values sit at `Text`, one step below the tool's name and one above their own captions**: a terminal has a single font size — the grid belongs to the terminal, not to the app — so the three sizes the design draws in the card's head are three steps of weight and brightness here, and there is room for exactly one peak. Spending the brightest role on four measurements left the name nothing to be the peak of. The two exceptions carry meaning rather than rank: a pending release is one of the screen's three "act on this" points, and a broken install is its one alarm. The header block is separated from the strip by **one** plain blank row — the strip's own padding row is filled with the plate colour and already reads as air, so a second plain row on top of it reads as a hole. Both versions go through **`version.DisplayVersion`**, which puts a `v` in front of a bare version number: a tool's `--version` prints `1.10.2` where its release is tagged `v1.10.2`, and the two used to sit one letter apart for the same binary. It edits nothing else — `canonSemver` decides only *whether* the string is a version number (`nightly`, `cli-2.0` pass through untouched), and its own output is deliberately not what is displayed, since it drops zero-padding, a 4th segment and build metadata. The `\uf412` glyph the two version lines used to carry is gone with the labels that needed disambiguating: a caption says what the number is. A metric with nothing to report is **left out entirely**, so a tool with no GitHub ref shows a one-cell strip rather than three empty captions, and an empty strip is no strip at all. The grid **re-flows rather than truncates**, and it is sized by the widest *value* as well as the widest caption (`need`): sizing on captions alone cut `✕ not installed` to `✕ not insta` at the 80×24 baseline — the default terminal, and the exact state a tracker is opened in. The count is solved against the row the strip actually draws — a blank cell at each end plus a rule between every pair — by counting up while `2 + cols*need + (cols-1)*3` still fits, **not** by dividing `inner` by `need`: that division ignores the overhead, so a 40-cell panel was told it had three columns and then handed each of them 10 cells, cutting `MAINTENANCE` to `MAINTENANC` — the caption the floor exists to protect. At the 80-column baseline the card panel is 27 cells and even two columns need 29, so the grid stands on one; a value longer than any caption costs a column rather than its own legibility. Below `metricStripMinWidth` (`metricMinCol` plus the row's two blank edge cells — measuring against `metricMinCol` alone was two cells short, and a 12-cell panel drew a single 10-cell column) the strip stands down completely, which only a hand-built model reaches since the panel has a 30-cell minimum. **Every row is exactly `inner` cells** — a short row would break the fill into a ragged edge — and every segment carries the background itself for the reason the selected list row does. **Each cell is centered in its column** (odd slack to the right, so a caption and its value can differ by at most one cell in where they start): a caption and the value under it are one measurement, and flush-left hangs them off a rule that is nowhere near either. `TestMetricsStripLayout` pins the width, the caption-over-value reading (via `metricValue`, which identifies a column by the `│` rules around it rather than by the caption's start offset — centered caption and value deliberately do not begin at the same cell), the centering and the re-flow; `TestMetricsStripOmitsUnknowns` pins the omission. - **Panel titles**: all three panels inset a title into their top border (`┌─ ▸ [1] tools 27 3↑ ─…─┐`) via the shared `insetPanelTitle` — an ANSI-safe splice over the already-rendered frame (`ui`'s `truncateVisible` is unexported; the helper repaints the border runs from their `stripANSI` text and drops the title in **already styled**). That is what lets one title carry several colors, which the `[1]` counts are the point of: the tracked count in `Dim`, the update count in `Signal` — the tracker's whole reason to exist, so it is the one thing in a panel title that gets the signal color, and it is absent when nothing is behind. A `panelTitle` carries **both** a plain and a styled form: the plain one is what the border arithmetic measures, because escape sequences are not cells, and a title that does not fit is dropped whole (a chopped title reads worse than none). **Focus is marked twice** — by the accent color and by a `▸` prefix — so it survives a monochrome terminal and a reader who cannot separate the two panel colors; `TestPanelTitleFollowsFocus` pins both signals. The titles are lowercase (`[1] tools`, `[2] brief`, `[3] readme` / `[3] help` / `[3] man`, overridden by `[3] update` while a live log shows and by `[3] update finished` / `[3] update failed` once it ended) and double as the documentation for the digit focus hotkeys, so the status bar carries no digit hints. `[3]` additionally names **the two sources it is not showing** (`· h help · m man`) in the border color: those keys switch what the panel *is*, which is a property of the panel rather than an action on its content — and they are dropped while the update log owns it, since none of the three modes is what is on screen. All title characters are single-width and non-East-Asian-Ambiguous except `▸` (U+25B8, Ambiguous like the list markers, and measured with the plain form either way), keeping the border width math stable. - **Panel footers**: each panel reserves the last `panelFooterRows` (2) of its content height for a **blank** spacer plus a footer line. `[1]` carried a border-colored rule there for a while and it only made the footer read as a fourth section of the list rather than as the frame's own caption. Cells are joined by a **dim** ` · ` — the same painted middot the card's language list uses, since an unpainted one renders at the terminal's default brightness, louder than the hint labels it is separating (`TestPanelFooterSeparatorIsDim`). `[1]` carries `/ filter · enter run · space group` — `enter` is a `[1]` action, not a global one (see **Status bar**), and the three are ordered most-important-first because cells drop from the right and on a narrow list "run" outranks "group"; a right cell that is absent now reserves **nothing**, gap included, which is what had been dropping `[1]`'s last cell one step earlier than the width required. `[2]` carries its own actions led by the contextual `enter update to ` when a release is pending (and deliberately **not** `e note` / `# tags`: both are already offered in the meta line beside the values they edit, and a footer repeating them spends the row on saying it twice), `[3]` the source name, the `page/pages` position, the entry-cursor hints while there is an index to walk, the zoom toggle `z zoom` **last** (least actionable of the four — where you are in the text and how to walk it both outrank a width preference — and dropped entirely while an update log owns the panel, exactly as the title drops the source hints), and `ctrl+d/u page` pinned right. Measured shed geometry for that last cell: it survives from ~114 columns with no entry index to walk and from ~150 once there is one, since the index adds a `j/k navigate` cell. **The split is the index, not the source**: `--help` measures at ~150 because it has one, and a `man` page without one sheds at ~105 like the readme — so a per-mode number is the wrong shape to remember, and a test that fixes a width per mode is asserting about its fixture's cache rather than about the mode. Between ~82 (where `z` starts doing something) and whichever of those two widths applies, the key works unadvertised here, which is what the `[?]` overlay, the one surface that never sheds, is for. Cells are dropped from the right until they fit, exactly like the status bar and for the same reason: a footer that wrapped would push the panel one row past its height, and lipgloss answers that by scrolling the top border off the alt screen. **`calcListHeight()` is the single definition** of the viewport height inside a panel (`calcVpHeight() - footerRows()`), used both by `applyLayout` (which sizes the viewports) and by the renderers that stack viewport + footer back to the full height — two copies of that arithmetic would drift and cost a row. On a terminal too short to spare them (`calcVpHeight() < 6`) `footerRows()` is 0 and the panels are content only: two of six rows is a third of the panel, and a footer is a reminder while the content is the point. -- **Panel widths and the zoom toggle (`z`)**: the three panel widths come from **`panelWidthsFor(zoom bool)`** in render.go — 20% tools / 46% brief / remainder to `[3]` normally, 20/30/remainder zoomed, then the 15/30/30 minimum-clamp cascade unchanged. `calcPanelWidths()` is the one-line wrapper reading `m.helpZoom`, the session-only view flag `z` owns (a view flag like `groupByTag`, not an `inputMode` — nothing here owns input — and deliberately not persisted: it is a way to read one README, not a preference). The parameterized core exists so **`toggleZoom()` can compare the two states without copying a `Model`** (the `baseFor`/`planFor`/`shellCommand` idiom), which is exactly what it does: below ~82 columns the clamps produce the **same triple for both variants**, so the keypress reports `too narrow to zoom` and flips nothing rather than setting a flag nothing follows — the honest no-op of `toggleGroupByTag`'s refusal, **one-directional in the same way and for the same reason**: `!m.helpZoom` gates the check, so zoom can always be turned *off*. A symmetric refusal strands the flag on — a session zoomed at 160 columns and resized to 80 could not clear it, and the layout came back zoomed the moment the terminal grew again, which is `toggleGroupByTag`'s stuck-in-the-tag-view failure one feature over (`TestZoomNarrowStillUnzooms`). The refusal is also the reason the status is short (18 cells: `renderStatusBar`'s statusMsg branch does not truncate, so an over-long message is exactly the wrap `TestStatusBarNeverWraps` exists to catch, and this is the one message whose trigger *is* a narrow terminal). `!m.ready` is checked **first and explicitly**, not left to the clamps agreeing by coincidence: before the first `WindowSizeMsg` every width is zero and there is no layout to toggle. **A successful toggle says nothing at all** and therefore returns a nil command — two panels change width over the full height of the screen, so the user is looking straight at the answer and a bar reading `readme zoomed` only restates it one line below where it happened. That is the one place this toggle parts company with `toggleGroupByTag`, which does report: reordering one panel's rows and inserting headers is a subtler change, and its message names which of two orderings you landed in. The refusal is the only exit that speaks, and it is exactly the case where the screen does *not* answer — nothing moved and the reason has nowhere else to live. The nil is a stricter invariant than the expiry tick it replaced: any command returned from the success path is a fetch, and `TestZoomFetchesNothing` says so directly. Two consequences are **stated, tested, and accepted**: the wrap width changes (`helpW` stays ≥ 30, off `helpWrapWidth()`'s floor), so `applyLayout` runs `setHelpContent()` and an active `j/k` spotlight in `[3]` is lost; and the README re-renders through glamour synchronously, since the width is part of `readmeRenderCache`'s key. Both are exactly what a width resize already costs. Zoom is orthogonal to the `[3]` update-log takeover — `showsUpdateLog()` owns *content*, zoom owns *width*. +- **Panel widths and the zoom toggle (`z`)**: the three panel widths come from **`panelWidthsFor(zoom bool)`** in render.go — 20% tools / 46% brief / remainder to `[3]` normally, 20/30/remainder zoomed, then the 15/30/30 minimum-clamp cascade unchanged. `calcPanelWidths()` is the one-line wrapper reading `m.helpZoom`, the session-only view flag `z` owns (a view flag like `groupByTag`, not an `inputMode` — nothing here owns input — and deliberately not persisted: it is a way to read one README, not a preference). The parameterized core exists so **`toggleZoom()` can compare the two states without copying a `Model`** (the `baseFor`/`shellCommand` idiom), which is exactly what it does: below ~82 columns the clamps produce the **same triple for both variants**, so the keypress reports `too narrow to zoom` and flips nothing rather than setting a flag nothing follows — the honest no-op of `toggleGroupByTag`'s refusal, **one-directional in the same way and for the same reason**: `!m.helpZoom` gates the check, so zoom can always be turned *off*. A symmetric refusal strands the flag on — a session zoomed at 160 columns and resized to 80 could not clear it, and the layout came back zoomed the moment the terminal grew again, which is `toggleGroupByTag`'s stuck-in-the-tag-view failure one feature over (`TestZoomNarrowStillUnzooms`). The refusal is also the reason the status is short (18 cells: `renderStatusBar`'s statusMsg branch does not truncate, so an over-long message is exactly the wrap `TestStatusBarNeverWraps` exists to catch, and this is the one message whose trigger *is* a narrow terminal). `!m.ready` is checked **first and explicitly**, not left to the clamps agreeing by coincidence: before the first `WindowSizeMsg` every width is zero and there is no layout to toggle. **A successful toggle says nothing at all** and therefore returns a nil command — two panels change width over the full height of the screen, so the user is looking straight at the answer and a bar reading `readme zoomed` only restates it one line below where it happened. That is the one place this toggle parts company with `toggleGroupByTag`, which does report: reordering one panel's rows and inserting headers is a subtler change, and its message names which of two orderings you landed in. The refusal is the only exit that speaks, and it is exactly the case where the screen does *not* answer — nothing moved and the reason has nowhere else to live. The nil is a stricter invariant than the expiry tick it replaced: any command returned from the success path is a fetch, and `TestZoomFetchesNothing` says so directly. Two consequences are **stated, tested, and accepted**: the wrap width changes (`helpW` stays ≥ 30, off `helpWrapWidth()`'s floor), so `applyLayout` runs `setHelpContent()` and an active `j/k` spotlight in `[3]` is lost; and the README re-renders through glamour synchronously, since the width is part of `readmeRenderCache`'s key. Both are exactly what a width resize already costs. Zoom is orthogonal to the `[3]` update-log takeover — `showsUpdateLog()` owns *content*, zoom owns *width*. - **`applyLayout()` is the single relayout definition** (model.go, beside `helpWrapWidth`): everything the `tea.WindowSizeMsg` handler used to do after storing `m.width`/`m.height` — the width recompute, `calcListHeight`, the first-time `initViewports(vpH)` arm, the wrap-width-changed `setHelpContent` guard, and the `setToolsContent()` + card repaint tail. The handler is now those two assignments plus the call, so the resize path and `z` cannot drift. **The `prevWrapW` capture must stay above the `calcPanelWidths` assignment**: `helpWrapWidth()` reads the **stored** `m.helpW`, so a capture placed below compares the new width against itself, the re-wrap guard is dead, and `[3]` silently keeps its pre-resize wrapping. **Neither of `applyLayout`'s two tests is the sole guard on its mutation, and the comments on them say so** — the capture is also held by `TestHelpNavIdxResetTriggers/resize`, `TestResizeHeightOnlyKeepsCursor` and `TestReadmeResizeRerenders`, and the `setToolsContent` tail by six mouse and line-map tests including `TestWindowSizeRebuildsLineMaps`. What `TestApplyLayoutRewrapsHelpOnWidthChange` and `TestApplyLayoutIdempotent` add is a failure that **names the thing that moved**: the others report a lost spotlight or a click landing on the wrong row, which is three inference steps from the line that actually broke. Claiming sole coverage where there is none is the worse error of the two, because the next reader deletes the redundant-looking test believing the invariant travels with it. - **`panelGutter`**: the blank column a panel keeps between its frame and everything it draws — the `[1]` group headers, every footer, and in `[2]` and `[3]` **all** content, at **both** ends. Content that touches the border it lives in reads as having overflowed it, and on the right the gutter is also what keeps text off the scrollbar thumb. The two wide panels reach it from opposite directions and each through a **single definition**: `[2]` sizes itself to **`cardWidth()`** (`briefW - 1 - 2*panelGutter` — the viewport is one column narrower than the panel, `withScrollbar` keeps that one) and `buildCard` steps the finished card in with **`indentLines`** at the very end; `[3]` wraps to **`helpWrapWidth()`** (the same arithmetic) and `renderHelpContent` is a one-line wrapper applying `indentLines` over `helpContent()`, so the spotlight, the search highlight, the placeholders and the update log all land on the same point and no branch can render flush against the frame. The indent is applied **last, to whole finished lines**: it is plain spaces outside the styling, so it can never split an escape sequence, it lands in front of the metrics plate's own background segments rather than inside them, and it cannot shift a line index — which is why `buildCard`'s clickable-link map, built while writing, stays correct for free. `TestPanelsKeepTheirGutter` pins both panels on rendered output. `[1]` and `[2]` additionally open with a blank **row** for the same reason — their content started against the title spliced into the top border. In `[1]` that row is a real screen line, so it goes into the line maps as a non-selectable one and every tool below it shifts by one (which is what the maps exist for, and `syncToolsViewport` walks *up* over every non-selectable row above the selection so a header and the blank above it are revealed together). Tool rows keep the wider `toolRowIndent` (gutter + marker + blank), which puts the `⏺` in the same column as a section header's label and the names one step in from it. - **Refresh (`r` in `focusBrief`)**: `refreshSelectedCmd(t)` force-refreshes the selected tool bypassing the 24h cache TTL — the repo pass (`refreshRemoteCmd` → `version.RefreshRepoData`) + changelog (`refreshChangelogCmd` → `version.RefreshChangelog`) + README (`refreshReadmeCmd` → `version.RefreshReadme`, preceded by a `delete(m.readmeData, name)` so a session-cached 404/rate-limit negative can recover, then a `markReadmeLoading(name)` — the deletion makes `needsReadme` true again for the whole in-flight window, so without the marker leaving and re-entering the tool would spend a second request; `refreshingFor` does *not* cover it, since `remoteMsg` clears that flag as soon as the repo pass lands, which can be well before the README does) + a local installed re-detect (`fetchInstalledCmd`). It emits the same `remoteMsg`/`changelogMsg` as the startup path, so the merge/re-render logic is reused. While the repo pass is in flight `m.refreshingFor` (the tool name) turns the card title into a status line — `refreshing data ` (`bubbles/spinner`, `MiniDot`; the about is hidden) — with no status-bar takeover; the `remoteMsg` handler clears `refreshingFor` on completion, which reverts the title to name+about and halts the `spinner.TickMsg` loop. **`[r]` answers every press**: on completion the handler returns `setStatus(refreshFailedStatus(msg.err))` whenever **`msg.err != nil`** — every path that actually failed to fetch now carries a named error (see `version.pickFetchErr`), while a pass that fetched something stays silent because the repainted card *is* the answer. The predicate is deliberately **not `!msg.conclusive`**, which is a broader thing: it is also false when the version layer refused the ref outright (an unsupported or spoofed host — a bare `RepoData` with a nil `Err`, no request made), which would report a network failure that never happened, and on a partial pass that fetched a new tag and lost only the repo card, where the bar would contradict a card the user just watched update. Before that, success, a rate limit, a 401, a timeout and a dropped connection were one indistinguishable gesture: the spinner turns, the card does not change. The reason is two-tier — `ErrRateLimited` → `refresh failed: rate limited — press [a]` (the one class with an answer the user can act on), everything else → `refresh failed: network error`. There is deliberately **no token wording**: by the time a fetch fails, `doGH` has already retried a rejected token anonymously, so the refresh did not fail *because of* the token, and the gauge and `[a]` overlay report that state anyway. The write sits inside the `msg.toolName == m.refreshingFor` branch, so the background passes `Init` fires — inconclusive all the time on an offline start — never put a "refresh failed" on the bar for a gesture nobody made. `refreshingFor` doubles as the double-press guard; a tool with no `GitHub` only re-detects the installed version (`m.statusMsg = "no repo to refresh"`, no spinner). Note `case "r"` no longer branches at all: it is refresh in `focusBrief` and unbound everywhere else. Rename went global as `m` and the README source moved to `R`, so the key that once meant three things by focus now means one. @@ -115,12 +117,12 @@ The model is a three-panel layout with focus cycling via `→/←` between `focu - **Panel `[3]` modes (`helpMode`)** — full rationale in **[`docs/design/readme-pipeline.md`](docs/design/readme-pipeline.md)**. Three sources: `helpModeReadme = 2` (the **default** set in `New()`), `helpModeHelp = 0`, `helpModeMan = 1`. `helpMode` is a sticky global field, not per tool; `[R]`/`[H]`/`[M]` switch it from `focusBrief || focusHelp` through the shared **`switchHelpMode(mode)`** (sets the mode, dismisses a *completed* update log, `setHelpContent()` + `GotoTop()`, returns the fetch command for the mode's missing source). The trio is **capitals as a set** so none collides with a lowercase verb (`r` is `[2]`'s refresh, `m` the global rename). **`z` shares their gate and nothing else**: it is the fourth key that changes what `[3]` is, so it fires from the same `focusBrief || focusHelp` pair, but it changes the panel's *width* rather than its source — so it does **not** move focus (a width change is not a change of what you are reading), goes through `toggleZoom()` rather than `switchHelpMode`, and fetches nothing. See **Panel widths and the zoom toggle** below. The invariants most often broken (the full list is in the design file): **`m.helpCache` is a `map[string][2]string` indexed by `helpMode`, so mode 2 panics on every index site** — README content lives in `m.readmeData` and each index site (`rawHelpText`, `renderHelpContent`, `autoFetchCmdsForSelected`) carries a readme early-return *before* the array read; a **live** update log keeps `[3]` in every path (its branch sits ahead of both the readme branch and the `No tool selected` guard). Rendering is `cleanTerminalOutput` → **`cleanReadmeMarkdown`** (readme_clean.go) → glamour with `keepkitStyle`, and a glamour failure falls back to the **preprocessed** text, never to the raw one. In the preprocessor: **code is never rewritten** (fenced blocks segmented, inline spans NUL-masked), CRLF is normalized on entry (a `\r`-suffixed closing fence protects the rest of the file), images run before links, autolinks and bare URLs are left alone, both reference forms are gated on labels collected **document-wide after** the HTML rules, HTML tag names come from the fixed `rcHTMLNames` allowlist whose trailing `\b` is what makes the ~80-branch alternation order-independent, and **`rcLineContent` slices rather than `ReplaceAllString`** — the replace form froze the whole TUI for 8.4 s on a 512 KiB adversarial README. `keepkitStyle` **clones** the glamour globals and assigns a **fresh pointer** per override, because `styles.DefaultStyles` aliases the same structs and writing through a cloned one restyles glamour process-wide. Dark/light is resolved **once at construction** into `m.darkBG` — `glamour.WithAutoStyle()` probes the terminal with an OSC query that races Bubble Tea's input reader. In the theme itself: the heading ladder and the fence accents come from `ui.HeadingColors`/`ui.ChromaColors` and **do not follow a theme switch** (the contract change, stated in three places); **`H1.Prefix = ""`/`H2.Prefix = ""` are load-bearing assignments** — glamour's cascade overrides a child prefix only when it is non-empty, so losing them brings the stock `# `/`## ` markers back rather than falling through to `Heading`; **`H6.Bold` must be set explicitly** (the stock dark H6 is an explicit `Bold: false` that beats inheritance); in the chroma repaint **`Text.BackgroundColor` is what carries the whole plate** — chroma resolves an unset token background up to `Text`, its root, not to `Background`, so the five accents set `Color` only; and `HorizontalRule.Format` is guarded on `width > 0`, since a caller with no layout yet must get the stock rule rather than an empty string. **`chromaFormatterFor`** (readme.go) is what keeps a fence's plate the same color as the card's: glamour's default formatter is `terminal256` for *every* profile, ignoring the `WithColorProfile` beside it, which quantized `Surface` `#343945` to index 237 and put one plate on screen in two colors. Two accepted limits stay: chroma's plate covers only the highlighted **tokens** — glamour paints a code block's left indent with the *enclosing* block's style and never paints the pad out to the wrap width, so a wrapped fence line reads as a short smudge — and `TestFencePlateMatchesTheCard` allows one unit per channel, because **termenv** rounds `#343945` to `52;56;69` while chroma writes the hex verbatim. **Any override new to this file must be confirmed against glamour's renderer once before its struct test is written** — a struct assertion only proves the field changed, and `Item.Color` passes one while doing nothing. - **Help navigation (`j`/`k` in `focusHelp`)**: `[3]` is navigable per *entry* — a flag or subcommand line plus its indented description block. `parseHelpEntries(raw, width)` (textutil.go) detects entries heuristically on the **pre-wrap source lines** (flag start = the `helpTokenRe` flag core at the trimmed line start; subcommand start = `helpEntrySubcmdRe`, an indented non-dash word + 2+ spaces + text — the word class excludes `.` so justified man prose like `tree. See also…` doesn't match; continuation = `continuesEntry`: any deeper-indented non-header line — including deeper lines that *begin* with a flag token (`…overridden with\n --no-ignore.`) — plus blank lines whose next non-blank line still continues, so multi-paragraph descriptions stay one entry; the entry ends at a section header or the next line at the entry's own indent or shallower) and maps the ranges to wrapped display-line indices via `wrapLine` — the same code `wrapText` uses, which is the point: `wrapText` rebuilds wrapped lines from `strings.Fields` (indentation is lost), so parsing wrapped output would break the indent heuristic, and sharing the wrap algorithm plus the single `helpWrapWidth()` (`max(helpW-1-2*panelGutter, 20)` — the viewport is a column narrower than the panel and a gutter is held at each end) keeps entry indices in lockstep with what the viewport shows. `isHelpSectionHeader` is the one definition of a header, used by both `colorizeHelp` and the parser. State is `m.helpEntries []entryRange` + `m.helpNavIdx` (−1 = off) + `m.helpBase` — the wrapped+colorized full-color content, cached because cursor moves repaint per keystroke and must not re-run the colorize regex over a whole man page (`helpContent`'s normal path serves `applySpotlight(helpBase)`, and `renderHelpContent` is the one-line wrapper that steps the result in by `panelGutter`; the base is built directly in `setHelpContent`, not via the renderer, which serves that very base back through `applySpotlight`). **`setHelpContent()` is the single recompute point** — every site where the *visible* text changes (selection via `autoFetchCmdsForSelected`, `[R]`/`[H]`/`[M]`, `helpOutputMsg` — gated on `msg.mode == m.helpMode`, a late fetch for the hidden mode must not reset the cursor, `readmeMsg` for the selected tool while in readme mode, resize — only when `helpWrapWidth()` actually changed, so a height-only resize keeps the cursor and a width change re-renders the README, update-log start) goes through it: recompute entries (empty for the update log, readme mode, `helpLoadingFor` and placeholders — `j`/`k` stay plain scroll there), reset the cursor, repaint, never scroll. Style-only repaints (per-chunk log appends, cursor moves) call `SetContent(renderHelpContent())` directly and must not reset the cursor. Interaction: **only the letter keys navigate — `↑`/`↓` keep their 3-line scroll** so prose between/after entries stays keyboard-reachable; the first `j`/`k` lands via `helpNavStart(delta)` on the first entry intersecting the window, or (none visible) the nearest entry in the movement direction; later presses step clamped without wrap; `applySpotlight` (render.go) dims every line outside the current entry (`Styles.Dim.Render(stripANSI(line))` — the `[a]` overlay's strip-then-repaint trick per whole line) while the entry keeps full `colorizeHelp` color; `scrollToNavEntry` keeps it in view with mutually exclusive branches and a `min(end-Height, start)` clamp so a taller-than-window entry pins its start to the top. `esc` is two-stage: cursor off first (scroll kept), focus walk second. `PgUp`/`PgDn`/`g`/`G`/wheel stay pure scroll and never touch the cursor. Every path that deactivates navigation (esc, any `setFocus` move) goes through `clearHelpNav()`, which pairs the reset with the repaint — clearing the index without repainting leaves stale dimming. The `focusHelp` bar shows `[j/k] navigate` alongside `[↑↓] scroll` when entries exist and prepends `[esc] exit nav` while the cursor is on. - **Tracking verbs are global**: `t` track (add by GitHub URL or plain name → `modeTrack`), `u` untrack (with confirmation → `modeConfirmUntrack`), `m` rename (fix the binary name when the repo name differs → `modeRename`). All three fire in **every focus** and are three of the six keys `globalHints` puts on the status bar. They used to be `[1]`-only because each collided with a `[2]` action (`t` tags, `u` update, `r` refresh); the redesign moved those onto keys of their own (`#` tags, `enter` update) and freed all three to mean one thing everywhere, which is what lets the bar carry a single focus-independent list instead of three per-focus ones. Rename is **`m`, not `R`**: `R` is panel `[3]`'s readme source now, and the tracker's verbs are the lowercase set. That leaves lowercase `r` meaning **refresh in `[2]` and nothing else** — it used to double as `[3]`'s readme switch, so one key meant "spend three requests" or "swap the panel's source" depending on a focus the `[3]` title did not mention. Each mode has a handler in `mode.go` and a matching branch in `renderStatusBar()`, mirroring the `modeEditNote`/`modeEditTags` input pattern. Mutations go through `loader.UpsertMeta`/`RemoveMeta`, persist via `loader.SaveMeta`, then rebuild `m.tools = loader.ToolsFromMeta(m.meta)` and refresh the viewport. -- **Run (`enter` in `focusTools`)**: launches the selected tool without leaving keepkit. `enter` fires only in `modeNormal`+`focusTools` (empty list → no-op; in `modeSearch` enter stays the commit key) and opens `modeRunInput` — a one-line prompt (`m.runInput`, its own textinput like `m.search`, not shared with note/tags) prefilled with `m.lastRun[name]` else the tool name, cursor at end; the status bar echoes `run : [enter] run [esc] cancel`. `m.lastRun map[string]string` is session-only per-tool memory of the last dispatched command — rename's stale-state cleanup deletes the old-name entry alongside `helpCache` et al.; untrack deliberately leaves it (harmless, session-scoped). Enter with empty/whitespace input cancels like `esc`. On dispatch `launcher.Detect(command, name)` picks the path — env-only, so unlike every probe it is safe inside `Update()`: a tab plan runs its `Argv` via `startLaunchCmd` (`exec.Command` + `proc.DetachTTY`, `launchTimeout` — a 10s **var**, shrunk by the timeout/KillGroup test — with `proc.KillGroup` on expiry, `safeCmd`-wrapped) → `launchDoneMsg{toolName, command, err}`, with `m.launchingFor` (the launch twin of `updatingFor`) as the one-adapter-launch-at-a-time guard and a `launching in …` statusMsg as in-flight feedback (this is also `Plan.Terminal`'s consumer); a `Fallback` plan runs `execToolCmd` → `tea.ExecProcess` over `shellCommand(runtime.GOOS, cmd)` (`sh -c` / `cmd /c`; goos-parameterized and spawn-free like `browserCommand`, so both branches are table-testable) — keepkit suspends, Bubble Tea restores the terminal when the tool exits → `execDoneMsg{toolName, err}`. **Auto-fallback**: an adapter failure (kitty remote control off, Automation permission denied) must not strand the launch — the `launchDoneMsg` error handler sets `statusMsg` (`tab open failed — running here`) and returns `execToolCmd(msg.toolName, msg.command)`; `command` rides the msg so the handler never re-reads input state. The auto-fallback is **gated on `modeNormal`**: the result can arrive up to `launchTimeout` after enter (osascript blocked on the macOS Automation dialog), and `tea.ExecProcess` seizing the terminal under an open editor/overlay would route keystrokes to the spawned shell — under any other mode the fallback is **deferred**, not dropped: the gate stores `m.pendingLaunchName`/`Command` and `flushPendingLaunch` (mode.go) dispatches `execToolCmd` with the same statusMsg (single definition: `launchFallbackStatus`, shared with the ungated auto-fallback) on the keystroke that returns the mode to `modeNormal` (every modal return in `Update` funnels through it — the mode-dispatch switch plus the inline `modeSearch` exit; `modeTokenInput`'s esc lands on `modeAPIStatus`, so the flush waits for the overlay to actually close). A statusMsg set at gate time would be dead UI — every open mode's `renderStatusBar` branch outranks the statusMsg branch, and the blanket `statusMsg = ""` reset on `tea.KeyMsg` fires on the very keystroke that closes the mode; setting it in the flush (after both) is what makes the failure visible. The flush goes straight to `execToolCmd` — never back through `launcher.Detect` — so a known-failing adapter plan is never re-run; a new dispatch from `modeRunInput`'s enter drops a pending fallback first (flushing both on one keystroke would run two commands), and confirming untrack of the pending tool itself drops it too — the dialog-closing enter must not exec the now-untracked tool's command (a pending fallback for a *different* tool deliberately survives the untrack and flushes on that keystroke). The handler clears `launchingFor` first in all outcomes. Working directory differs by path: a tab opens in the new shell's default cwd, the ExecProcess fallback inherits keepkit's. The timeout carries one accepted race: an adapter killed after its tab command already executed (osascript stuck post-`write text`) still triggers the fallback, so the command can run twice — narrow, undetectable, and better than stranding genuine failures. This path also serves native Windows: `planFor` is env-only, so WezTerm there yields a doomed `sh -c` plan whose failure lands in the fallback (one noisy attempt accepted; a `GOOS` guard in `planFor` is deliberate YAGNI). Success wording is mode-neutral — `launched ` — because Terminal.app and tmux open a *window*, not a tab. **No `logx` anywhere in the flow**: a non-zero tool exit (`statusMsg " exited: "`) is the tool's business, not a keepkit anomaly, and an adapter error is a degraded path, not a malfunction (the auto-fallback still launches the tool). Launch during a running update is deliberately not blocked — independent concerns; ExecProcess pauses rendering of the live update log and the buffer catches up on resume. A not-installed tool launches anyway (no PATH pre-check): in a tab `sh` reports `command not found` inside that tab, while on the ExecProcess path the shell's not-found exit (`notFoundExit`: 127 sh / 9009 cmd.exe) maps to `statusMsg " not found — is it installed?"` instead of the cryptic raw exit status. The `[?]` overlay's tools group carries `enter — run in tab` (desc kept short — the overlay sits at the 76-col edge of its budget). +- **Run (`enter` in `focusTools`)** — full rationale in **[`docs/design/tool-overlay.md`](docs/design/tool-overlay.md)**. Runs the selected tool on a pseudo-terminal **inside keepkit**: `enter` opens `modeRunInput` (a one-line prompt, `m.runInput`, prefilled with `m.lastRun[name]` else the tool name; blank input or `esc` cancels), and the dispatched command runs in a centred overlay over the dimmed layout. There is **one path for a TUI and a CLI** — vim draws in it, `rg --version` prints and stops and the final screen stays until `esc`. The invariants most often broken (the full list is in the design file): **only `Update` touches the emulator's screen state** — `vt.Emulator` lives on `Model`, the pty is read by one goroutine posting `tea.Msg`s; **input needs a relay goroutine** because x/vt exports no key encoder and its `SendKey` encodes against DECCKM, which no accessor exposes — a hand-rolled encoder would send the wrong arrows to exactly the full-screen tools this exists for; **teardown closes `InputPipe()`'s writer, never `Emulator.Close()`** (upstream race charmbracelet/x#879, reproduced under `-race`), and `closeToolOverlay` is the single teardown, session-first so a parked relay write fails fast; **while the tool runs every key is forwarded, `esc` and `ctrl+c` included** — `ctrl+\` is the one reserved chord and it kills *without* leaving the mode, since the outcome line arrives with `termExitMsg`; after the exit only `esc` acts; **the exit row is reserved from the start and every row is padded to the body width**, the two things that keep `PlaceOverlay` from re-centring the block at the moment the user starts reading the outcome; **`termGeometry` returns the size and `ok` separately** — the keypress refuses on `!ok` (`terminal too small to run a tool`, **no `lastRun` write**), a mid-session shrink clamps to the floor and keeps the tool running, and `!m.ready` is checked first (the `toggleZoom` idiom); **the drain carries the exit rather than swallowing it** (`termChunkMsg.exit`), so a short-lived tool's final screen is painted before the verdict. A not-installed tool needs no special case: `sh` says so and exits 127, which the `✕ exit 127` line shows. `m.lastRun` is session-only per-tool memory; rename deletes the old-name entry alongside `helpCache` et al., untrack deliberately leaves it. Launch during a running update is deliberately not blocked — independent concerns; the log keeps streaming into `[3]` under the dim. **No `logx` anywhere in the flow**: a non-zero tool exit is the tool's business, not a keepkit anomaly. The `[?]` overlay's tools group carries `enter — run in overlay`. - **Status bar** (`renderStatusBar()`) is **global** — the same line in every focus, instead of the three per-focus lists it used to rewrite itself between as the user moved panels. A key belongs on it exactly when it does the same thing in **every** focus, and six do: `t track · u untrack · m rename · a api · ? keys · q quit` (`globalHints`). `enter` and `/` used to lead that list and are precisely the two that failed the definition — `enter` runs a tool in `[1]`, installs a release in `[2]` and is unbound in `[3]`; `/` filters the list in `[1]` and is unbound in `[2]`/`[3]` — so the bar either advertised a key that does nothing in two of the three panels, or (in `enter`'s case) one meaning while being wrong about the others. Both moved to `[1]`'s footer beside `space group`, next to the panel where their meaning is fixed, which is the same rule everything else panel-local follows (see **Panel footers**). Key hints lost their brackets — `m.key(k)` renders the bare key in the accent, and `m.hint(k, label)` pairs it with its word **in `Dim`**, a deliberate two-step contrast (the key is the only part of a hint the eye needs to find, so the word beside it steps back rather than competing at reading brightness), so the key/label contrast is defined once; the accent already says "this is a key", and at the bar's density a pair of brackets per hint cost more columns than the whole `[?]` overlay's third column. Literal brackets survive only where they name a *panel* (`[3] readme`), which is now the only bracketed thing on screen. In the three normal focus states `renderHintsBar` lays the line out as **three zones**: **`appVersionCell()`** pinned to the **left** edge, the hint cells **centered on the bar**, and the **API-usage gauge** pinned to the **right** edge. The two edges are the two facts that hold regardless of what the user is doing — which build this is, and how much quota is left — so they sit where the eye returns rather than inside a key list; centering the keys between them is only honest because those keys no longer change with focus. Centering is measured against the **whole bar**, not against the leftover band between the edges (the block must read as centered on screen, and the two edges are rarely the same width), then clamped into that band, so on a narrow terminal it is pushed up against the version cell rather than overlapping it. The collapsed **self-update cell** rides with the version on the left: it carries its action alone (`U update`) and `appVersionCell` is the subject naming what is being updated — split across the two edges, the verb would lose its noun. `appVersionCell` is keepkit's own identity line — `keepkit v0.1.0` in `Dim`, the one fact about the app that is true on every frame and the answer to "which build is this?" without opening `[?]`; when a newer release is known and still uninstalled (`selfUpdateAvailable()`, the sixth `selfState` site) the version takes `SignalBold` and the same ` ↑` an outdated tool row carries, so the app announces its own update in the vocabulary it announces everyone else's. The version shown stays the **running** one — the offer is that this build is behind, not a preview of what it would become — and once the update is installed the arrow goes (the pending action is a restart, which the self cell says). It **replaced the pending-updates count** that used to sit here: that count was the third surface for the same number (the `[1]` title carries it beside the tracked count, and every outdated row carries its own `↑`), while the running version had none outside an overlay; `updateCount()` still feeds the `[1]` title. Under width pressure the bar sheds in order of how *actionable* the thing dropped is: trailing hint cells first (they are ordered most-important-first, so `? keys` and `q quit` go before `t track`), then the gauge (read-only), then the version cell, then the self cell — and the leading hint cell is never dropped, only truncated. That order is spelled out step by step rather than computed, because "what goes next" is a judgement about meaning, not about width. `TestAppVersionCell` pins the cell across all five self states plus a tagless offer. **The gauge is on screen for the whole session** once the rate is known (`gaugeVisible`): it is also the only visible sign that keepkit has an API surface at all, so hiding it at rest hid the `[a]` overlay along with it — the numbers are the affordance, and `a api` sits in the global key list beside them. Its fill turns `Danger` under `min(gaugeDangerRemaining, limit/4)` remaining, read against the limit as well as absolutely, because an absolute-only bound would paint a token-less user's bar permanently red — the opposite of a signal. It renders as `api ▮▮▮▮░░░░░░░░ 45/60` in the corner, a fixed 12-cell bar of foreground-colored `▮` fill / `░` track glyphs (not painted backgrounds, so it survives a degraded color profile; both glyphs are deliberately non-East-Asian-Ambiguous, where a `█` would measure two cells and break the alignment math), degrading to `api 45/60`, then to `api✕` when there is a rejection to report, and then to nothing. **A rejected token adds a `Danger` `✕` to the label** (`api✕ ▮▮▮…`) in every form — this is the primary place a degraded session is announced, and the compact form is the one a narrow bar keeps, which is exactly where bare numbers explain themselves least. It is a suffix rather than a relabel to `anon` because a user with no token is also anonymous and that is not degradation; the `✕` says a credential was **refused**, and the limit beside it says what is left. `rejectedTokenGlyph` is U+2715, one cell under **both** runewidth conditions (U+00D7 would be two under `RUNEWIDTH_EASTASIAN=1`), which the right-edge arithmetic requires. That one column is also why **`renderRateMarker()`** exists as a third, narrowest form: at the 80×24 baseline the marked gauge no longer fits beside the six hints, so without it arming the degraded state removed the announcement entirely — the session that most needs announcing was the one showing nothing. What survives follows the bar's own shed rule (drop the least actionable thing first): the numbers are a measurement the `[a]` overlay repeats, the `✕` is the only sign anywhere on screen that requests stopped carrying the user's token. It returns `""` when there is no rejection, so a healthy bar gains no form it did not have before. `gaugeFilled` clamps the rounded ratio so any usage shows at least one `▮` and a full bar means exhaustion only. The bar spends **no columns advertising `[a]`** — the `[?]` overlay documents it. Input/modal states show no right group at all. **The bar must stay one line**: `Styles.StatusBar` is width-constrained, so a hint list wider than `m.width-2` wraps to a second row and `View()` returns `m.height+1` lines — one row past the terminal, which scrolls the top border off the alt screen. `renderHintsBar` therefore takes the hints as `[]string` cells ordered most-important-first and drops them from the right until they fit, reserving the two non-gauge right-group members first. The **leading** cell is never dropped, so one cell wider than the whole bar is possible — the fused self banner is 37 cells and overflows below ~39 columns — and that case is **truncated** (ANSI stripped first, since a cut inside an escape sequence would be re-emitted to the terminal verbatim) rather than allowed to wrap. `TestStatusBarNeverWraps` pins the invariant at the 80×24 baseline and, for the banner, down to 24 columns. Below 80 columns only the *bar* is checked: the three panels have their own minimums (15+30+30 plus borders), so the layout overflows there on its own — a separate, pre-existing limit. -- **Status-message lifecycle**: `m.statusMsg` (rendered by `renderStatusBar`, which outranks both the self-update banner and the hints bar whenever non-empty — the banner is not modal, and a transient status covering it for `statusMsgTTL` is intended) has two clears. (1) **Immediate**: the blanket `m.statusMsg = ""` on every `tea.KeyMsg` — any keypress wipes the current status at once. (2) **TTL auto-expiry**: every *transient* status is set via **`setStatus(s) tea.Cmd`** (model.go), which bumps a generation counter `m.statusSeq`, sets the message, and returns `tea.Tick(statusMsgTTL, …)` producing a `statusExpiredMsg{seq}` stamped with the current seq; the `Update` handler clears the message only when `msg.seq == m.statusSeq`, so a stale timer from a message already superseded by a newer one is a harmless no-op. `statusMsgTTL` is a **var** (`1 * time.Second`), shrunk by tests the same way as `launchTimeout` (the tick captures the value at construction, so the shrink must precede the `Update`). The returned `tea.Cmd` must be **batched** into whatever the caller already returns (`tea.Batch`); callers that only set a status return it directly. **In-flight** statuses (`launching in …`, `still launching `) are the deliberate exception and go through **`setStickyStatus(s)`** — no timer, but it **still bumps `statusSeq`**: they report work still in progress and are extinguished by `launchDoneMsg`, not the clock, so letting a timer expire one mid-flight would hide the only sign the adapter is busy for up to `launchTimeout`. The bump is the whole point of the helper rather than a plain assignment — a transient status set within the last `statusMsgTTL` (the group toggle, say) has a tick in flight whose seq would otherwise still match the sticky message and wipe it (`TestInFlightStatusSurvivesStaleExpiry`). `TestStatusExpired*`/`TestSetStatus*` pin the mechanism; `assertOnlyExpiryTick` is the test helper that asserts a site returns *only* the expiry tick (no fetch/exec rode along). +- **Status-message lifecycle**: `m.statusMsg` (rendered by `renderStatusBar`, which outranks both the self-update banner and the hints bar whenever non-empty — the banner is not modal, and a transient status covering it for `statusMsgTTL` is intended) has two clears. (1) **Immediate**: the blanket `m.statusMsg = ""` on every `tea.KeyMsg` — any keypress wipes the current status at once. (2) **TTL auto-expiry**: every *transient* status is set via **`setStatus(s) tea.Cmd`** (model.go), which bumps a generation counter `m.statusSeq`, sets the message, and returns `tea.Tick(statusMsgTTL, …)` producing a `statusExpiredMsg{seq}` stamped with the current seq; the `Update` handler clears the message only when `msg.seq == m.statusSeq`, so a stale timer from a message already superseded by a newer one is a harmless no-op. `statusMsgTTL` is a **var** (`1 * time.Second`), shrunk by tests through `shrinkStatusTTL` — the last shrinkable timeout seam now that `launchTimeout` died with the tab launcher (the tick captures the value at construction, so the shrink must precede the `Update`). The returned `tea.Cmd` must be **batched** into whatever the caller already returns (`tea.Batch`); callers that only set a status return it directly. Every transient status goes through `setStatus`; **work still in progress reports itself on the surface that owns it** — the update log's outcome block, the tool overlay's exit row — rather than on a bar message that would have to outlive its own timer. The `setStickyStatus` helper that existed for the tab launcher's `launching …` feedback died with it. `TestStatusExpired*`/`TestSetStatus*` pin the mechanism; `assertOnlyExpiryTick` is the test helper that asserts a site returns *only* the expiry tick (no fetch/exec rode along). - **API-status overlay (`a`)**: opens a read-only view of the GitHub rate limit and token (source, masked value, used/limit with threshold icon, reset time) with token entry/removal/refresh. When no token is configured it leads with an `add a github token…` nudge; a **rejected** one gets `replace the token to restore the 5000/h limit` instead — "add" reads as advice to someone who has already done it, and the thing to do is swap the credential, not create one. A rejected **env** token gets a third wording, `GITHUB_TOKEN was refused — replace it in your shell`, and offers **no key at all**: `[e]` runs `SetToken`, which writes the config file, and `effectiveToken` reads that only when `GITHUB_TOKEN` is empty — so the key cannot fix this one, and the variable belongs to the shell that launched keepkit. The env arm must stay **first** in the switch or the config wording swallows it; `[d]` two blocks down has gated on the source all along for the same reason. All three are hidden while entering a token, when the open input is already the answer. A rejection also rewrites the token line itself to `token () — rejected (HTTP 401)` in `Danger`, with `requests run unauthenticated` under it: this is the surface that **always** has the answer, since the gauge's `✕` is droppable under width pressure and invisible before the first rate snapshot. **The mask is load-bearing** — it is how the user recognises which credential to replace — and it survives only because `version.Token()` reads the raw `effectiveToken()` core rather than the suppressed `resolveToken()`. It is `modeAPIStatus` (token entry: `modeTokenInput`) with a matching `renderStatusBar()` branch; `a` fires only in `modeNormal`; `esc` **or `q`** closes it. See the GitHub API section for the data flow. -- **Overlay compositing**: two overlays composite over the layout via `ui.PlaceOverlay` (a centered fg-over-bg compositor), gated by the shared `overlayVisible()` predicate in `View()` — the `[a]` API-status overlay (`renderAPIStatus`) and the `[?]` hotkeys overlay (`renderHotkeys`), picked by `m.mode`. `PlaceOverlay` dims the whole visible background — original styling is stripped and repainted with `OverlayDimStyle` (`ColorDim`) — so the modal is the only full-color element. Covered rows get the dim inside `overlayLine` *after* `truncateVisible`/`dropVisible`, because those helpers `StripANSI` the bg and would erase a pre-applied dim from the modal's side margins. -- **Mouse policy** (`handleMouse` in `render.go`, dispatched from `Update()` before the mode switch, gated inside the function): wheel scrolling works in every mode; while any overlay is visible (`overlayVisible()` — `[a]` API status or `[?]` hotkeys) all mouse input is a no-op, and before the first `WindowSizeMsg` (`!m.ready`) too. Clicks that change selection or focus fire only in `modeNormal` — otherwise a click would move `selectedMeta()` under an open note/tags/rename editor and retarget the commit. A click that changes the selected tool goes through the same `selectMeta` helper as the keyboard `j`/`k` path, including the auto-fetch; a click anywhere in the tools panel (row or empty area) focuses it via `setFocus`, matching brief/help — the same helper the keyboard uses, so a click cannot leave the list painted with stale focus styling. Both panels translate the click row the same way — **through `panelRow(msg.Y, vp.Height)`**, then `+ vp.YOffset`. X alone does not mean "inside a panel": the outer `Margin(1,0)` row, the two borders and the status/hints bars all share the panels' columns, and with a scrolled viewport an unbounded `msg.Y - 2` maps that chrome onto real content — a click on the blank top row would open a card link. `panelRow` returns `-1` outside the viewport's rows; inside, `[1]` goes through `toolAtLine()` (a group header maps to `-1` and selects nothing) and `[2]` through `buildCard()`'s link index — a click on the `repo:` line or the changelog release URL returns `openURLCmd(url)`, every other line only moves focus. +- **Overlay compositing**: three overlays composite over the layout via `ui.PlaceOverlay` (a centered fg-over-bg compositor), gated by the shared `overlayVisible()` predicate in `View()` — the `[a]` API-status overlay (`renderAPIStatus`), the `[?]` hotkeys overlay (`renderHotkeys`) and the embedded tool terminal (`renderToolOverlay`), picked by a **`switch m.mode`**. A switch rather than the two-way `if` it grew out of: that form let the API-status default answer for every mode that was not `modeHotkeys`, which with a third overlay would paint the token panel over a running tool. `PlaceOverlay` dims the whole visible background — original styling is stripped and repainted with `OverlayDimStyle` (`ColorDim`) — so the modal is the only full-color element. Covered rows get the dim inside `overlayLine` *after* `truncateVisible`/`dropVisible`, because those helpers `StripANSI` the bg and would erase a pre-applied dim from the modal's side margins. +- **Mouse policy** (`handleMouse` in `render.go`, dispatched from `Update()` before the mode switch, gated inside the function): wheel scrolling works in every mode; while any overlay is visible (`overlayVisible()` — `[a]` API status, `[?]` hotkeys or the tool terminal) all mouse input is a no-op, and before the first `WindowSizeMsg` (`!m.ready`) too. Clicks that change selection or focus fire only in `modeNormal` — otherwise a click would move `selectedMeta()` under an open note/tags/rename editor and retarget the commit. A click that changes the selected tool goes through the same `selectMeta` helper as the keyboard `j`/`k` path, including the auto-fetch; a click anywhere in the tools panel (row or empty area) focuses it via `setFocus`, matching brief/help — the same helper the keyboard uses, so a click cannot leave the list painted with stale focus styling. Both panels translate the click row the same way — **through `panelRow(msg.Y, vp.Height)`**, then `+ vp.YOffset`. X alone does not mean "inside a panel": the outer `Margin(1,0)` row, the two borders and the status/hints bars all share the panels' columns, and with a scrolled viewport an unbounded `msg.Y - 2` maps that chrome onto real content — a click on the blank top row would open a card link. `panelRow` returns `-1` outside the viewport's rows; inside, `[1]` goes through `toolAtLine()` (a group header maps to `-1` and selects nothing) and `[2]` through `buildCard()`'s link index — a click on the `repo:` line or the changelog release URL returns `openURLCmd(url)`, every other line only moves focus. ### File storage @@ -154,7 +156,7 @@ Config files are not the only process-global state a test can leak. `version.rej An errors-only journal so bugs can be researched after the fact instead of reconstructed from memory. One plain-text file per session, created **lazily on the first write** — a session with no errors leaves no file at all, so the presence of a file is itself the signal. The timestamp is colon-free (Windows filenames) and zero-padded, so lexicographic order equals chronological order, which is what `Cleanup()` (keep newest 20) relies on. The header (`keepkit / tools= token=`) is assembled in `main.go` via `logx.SetHeader` and written as the file's first line — it stays in `main` because pulling version/tool-count/token-source into `logx` would invert the import graph (`logx`'s only project import is the stdlib-only `configdir` leaf, keeping it near the bottom of the graph). -**Why our `recover` sits deeper than Bubble Tea's:** `recover()` consumes a panic wherever it fires first, and Bubble Tea's own recover in each `tea.Cmd` goroutine prints the trace *after* `p.cancel()` (async) but *before* the terminal is restored — so the trace lands in the alt-screen buffer and vanishes on exit. `logx.Recover(context)` is therefore deferred *inside* `Update`, `View` and every command (via the `safeCmd` wrapper; `execToolCmd` is the one unwrapped cmd — `tea.ExecProcess` only constructs the exec message, nothing there can panic), records the value plus `debug.Stack()` (which still contains the real panic site from inside the defer), then **re-panics** so Bubble Tea still catches it, restores the terminal and returns `ErrProgramPanic`. `tea.WithoutCatchPanics` is deliberately **not** used — terminal restoration is Bubble Tea's job and it does it correctly. Errors-only by design: there is no debug level, no `KEYS_DEBUG`, no env reading, no JSON, no size-based rotation — without a debug level the file appears only when something went wrong. The logger never breaks the app: its own failures (file won't open, disk full) are swallowed silently. +**Why our `recover` sits deeper than Bubble Tea's:** `recover()` consumes a panic wherever it fires first, and Bubble Tea's own recover in each `tea.Cmd` goroutine prints the trace *after* `p.cancel()` (async) but *before* the terminal is restored — so the trace lands in the alt-screen buffer and vanishes on exit. `logx.Recover(context)` is therefore deferred *inside* `Update`, `View` and every command (via the `safeCmd` wrapper; the one unwrapped cmd is `handleTermChunk`'s exit re-emit — a closure returning a prebuilt value, nothing there can panic, the same safe-by-construction category `execToolCmd` occupied until it died with the tab launcher), records the value plus `debug.Stack()` (which still contains the real panic site from inside the defer), then **re-panics** so Bubble Tea still catches it, restores the terminal and returns `ErrProgramPanic`. `tea.WithoutCatchPanics` is deliberately **not** used — terminal restoration is Bubble Tea's job and it does it correctly. Errors-only by design: there is no debug level, no `KEYS_DEBUG`, no env reading, no JSON, no size-based rotation — without a debug level the file appears only when something went wrong. The logger never breaks the app: its own failures (file won't open, disk full) are swallowed silently. Logging sites are `Errorf`-only: cache read/write failures (`version.LoadCache`/`SaveCache`), GitHub API failures (`doGH`/`classifyStatus` — HTTP code + `X-RateLimit-Remaining`, never the token), installed-version detection give-up (`InstalledVersion`, logged once — but only when a binary that *is* on PATH fails to answer `--version`/`-V`; a plain not-on-PATH miss is the normal "not installed" state and is never logged, so a tracked-but-uninstalled tool does not create a log every startup. A brew/cargo fallback hit and an `isTUITakeover` capture both suppress it too — the first is that path's normal state, the second is a classified kind of tool rather than a malfunction), `meta.yaml` save failures (`loader.SaveMeta`), help-capture failures (`fetchHelpCmd`), update failures (`recordUpdateOutcome`, shared by the tool and self paths — the *on-screen* reason is the outcome block, owner-gated on `updateOutcome.tool == updateLogFor`, while the log line is unconditional; nothing is written into the buffer, so the logged tail is the manager's own output), a failed unix restart (`restartSelf` — a path-resolution miss or an exec refusal; the Windows branch logs nothing, its hint is planned degradation), and a panic-ended session from `main`. `View`/render steady state and keystrokes are deliberately never logged. diff --git a/README.md b/README.md index e515486..b8cfd10 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,9 @@ Pure TUI, no subcommands; the only flags are `--version` and `--help`. draws them as a proportional band in GitHub's own per-language colors - **Tags and grouping** — one tag per tool; `space` regroups the flat list under section headers, led by everything with a pending update, and back -- **Run tools** — launch any tracked tool in a new terminal tab (tmux / iTerm2 / - kitty / WezTerm / Terminal.app) or in the current window, without leaving keepkit +- **Run tools** — `enter` runs any tracked tool on an embedded terminal inside + keepkit: a TUI like vim or yazi draws and lives in it with the full keyboard, + a plain command prints and its final screen stays until you close it - **Search** — `/` filters by name and tag with match highlighting and an `N/M` counter - **GitHub API token** — a quota gauge in the status bar, plus token management in the `a` overlay, lifts the anonymous 60 requests/hour to 5000 @@ -128,7 +129,7 @@ Run `keepkit` — a three-panel interface opens: - **`[1] Tools`** — the tracker list, each row carrying its installed version: search, tag grouping, track / untrack / rename, and `enter` to run the selected - tool. + tool in an embedded terminal. - **`[2] Brief`** — the tool card: the tool's name and repo, its tagline, then a metrics strip (installed / latest / maintenance / stars) and a line carrying languages, status, tag and note. From here `enter` installs a pending release, the @@ -392,6 +393,9 @@ subprocess sandbox — is described in [ARCHITECTURE.md](ARCHITECTURE.md). - [Glamour](https://github.com/charmbracelet/glamour) — markdown rendering for the README panel - [goldmark-emoji](https://github.com/yuin/goldmark-emoji) — GitHub's `:shortcode:` dictionary, used to strip them from a README - [x/ansi](https://github.com/charmbracelet/x) — stripping escape sequences from captured tool output +- [x/xpty](https://github.com/charmbracelet/x) — the pseudo-terminal a tool runs on (unix ptys, Windows ConPTY) +- [x/vt](https://github.com/charmbracelet/x) — the terminal emulator that turns a tool's output back into a screen +- [ultraviolet](https://github.com/charmbracelet/ultraviolet) — x/vt's key and cell types, used to translate keystrokes for the running tool - [termenv](https://github.com/muesli/termenv) — terminal color-profile detection - [go-runewidth](https://github.com/mattn/go-runewidth) — glyph width measurement - [golang.org/x/mod/semver](https://pkg.go.dev/golang.org/x/mod/semver) — version comparison diff --git a/docs/design/tool-overlay.md b/docs/design/tool-overlay.md new file mode 100644 index 0000000..175b7b7 --- /dev/null +++ b/docs/design/tool-overlay.md @@ -0,0 +1,302 @@ +# The tool overlay: running a tracked tool inside keepkit + +`enter` in `[1] tools` opens a one-line prompt, and the command you type runs on +a **pseudo-terminal inside keepkit** — a bordered block over the dimmed layout, +with the whole keyboard handed to the tool. `vim` draws in it and edits; `fzf` +filters in it; `rg --version` prints two lines and stops, and the block stays +put until `esc`. There is one path for all of them. + +This replaced a tab launcher that scripted *someone else's* terminal into +opening a tab (`internal/launcher`: tmux, iTerm2, Terminal.app, kitty, WezTerm, +plus a `tea.ExecProcess` fallback and an auto-fallback for when an adapter +failed). That whole package and its recovery machinery are gone; what follows is +why the current shape is the way it is. + +Read this before changing anything in `internal/term`, `internal/model/overlay_term.go`, +or the `enter` dispatch in `updateRunInput`. + +## The one rule everything else follows + +**Only `Update` touches the emulator's screen state.** + +`vt.Emulator` lives on `Model`, not inside `internal/term`. The pty is read by a +single goroutine that posts bytes as `tea.Msg`s — the same `waitForChunkCmd` +pattern the update streamer uses — and the `termChunkMsg` handler is the only +place that calls `Write`. `Render`, `Resize` and `CursorPosition` are likewise +reached from `Update`/`View` and nowhere else. + +The rule exists because x/vt's screen buffer is not synchronised and the library +carries an open race issue (charmbracelet/x#879). Keeping every screen-touching +call on Bubble Tea's own goroutine sidesteps the whole class instead of guarding +against it call by call. + +The **input** path is the one exception, and it is a deliberate, bounded one — +see below. + +## Why input needs a goroutine after all + +The plan preferred translating a `tea.KeyMsg` into bytes and calling +`Session.Write` directly: no second goroutine, and the rule above becomes +literally true. That turned out to be impossible to do *correctly*. + +`vt.SendKey` encodes against emulator state: + +```go +ack := e.isModeSet(ansi.ModeCursorKeys) // DECCKM: arrows are \x1bOA, not \x1b[A +akk := e.isModeSet(ansi.ModeNumericKeypad) // DECNKM +``` + +`isModeSet` is unexported and nothing surfaces it, and no key encoder is +exported anywhere in the pinned stack (`ultraviolet` has only output-side +`Encode*` helpers). A hand-rolled encoder could not know whether the child had +switched to application cursor keys — the mode `vim` and `less` set on entry. +Arrows in vim are not a corner case for this feature. + +So keys go through the emulator, and `termInput` is the goroutine that reads +them back out of its input pipe and writes them to the pty. It touches no screen +state. + +Translation splits three ways, by what actually depends on emulator state: + +| Keys | How | Why | +|---|---|---| +| runes, space | `SendText` | printable text, no encoding decision | +| arrows, Home/End, PgUp/PgDn, Insert/Delete, shift+tab, F1–F12 | `SendKey` | DECCKM/DECNKM apply | +| the control range (`KeyEnter`, `KeyEsc`, `KeyCtrlA` …) | the byte itself | Bubble Tea's control key *types are* the control bytes, and none of them is mode-dependent — writing the byte is exactly what vt's encoder produces | +| ctrl/shift-modified cursor keys | hand-encoded `CSI 1;` | **x/vt encodes none of them** — its `SendKey` default emits nothing when `Mod != 0`, which would silently swallow `ctrl+left` in every editor. Safe to write out because, unlike the bare arrows, the modified forms do not depend on DECCKM | +| a paste (`KeyRunes` with `Paste: true`) | `Emulator.Paste` | one block of text, not typed keys: `Paste` brackets it with the `?2004` markers exactly when the tool set that mode — without them vim auto-indents every pasted line — and passes it bare to a tool that never asked | + +`Alt` is a Bubble Tea flag rather than a key: it becomes an ESC prefix (a paste takes no prefix — it has no modifier). + +### Teardown: never call `Emulator.Close()` + +This is the sharp edge, and it is load-bearing: + +```go +func (e *Emulator) Read(p []byte) (int, error) { + if e.closed { return 0, io.EOF } // unsynchronised read + return e.pr.Read(p) +} + +func (e *Emulator) Close() error { + e.closed = true // unsynchronised write + return e.pw.CloseWithError(io.EOF) +} +``` + +A relay parked in `Read` plus a `Close()` from `Update` is charmbracelet/x#879, +and `go test -race` reports it immediately — reproduced here, which is why the +workaround is not speculative. + +**`InputPipe()` returns the emulator's `*io.PipeWriter`; closing *that* unblocks +the parked `Read` through `io.Pipe`'s own synchronisation and never touches the +bool.** `termInput.stop` does exactly that, falling back to `Close()` only if a +future x/vt stops handing out an `io.Closer`. +`TestSessionInputPipeCloseIsRaceFree` is what keeps the next reader from +reaching for the obvious teardown. + +`closeToolOverlay` is the single teardown and its order matters: **session first** +(so a relay parked in `Session.Write` fails fast), then stop the relay and +*wait* for it, then drop the emulator. Reversing the first two can park the +relay in a write nobody will drain. + +## esc belongs to the tool + +While the tool runs, **every** key is forwarded — `esc`, `ctrl+c`, `q`, +everything. `esc` is what makes vim usable; `ctrl+c` is the tool's interrupt and +must not be keepkit's quit. That is the whole meaning of "the tool has the +keyboard", and it is why `updateToolOverlay` consumes keys it has no use for +instead of letting them fall through to the normal-mode map. + +**`ctrl+\` is the one reserved chord.** It kills the process group and *stays in +the mode*: the verdict arrives as `termExitMsg`, and that is what turns the +overlay into its outcome line. Leaving on the keypress would throw away the log +the user killed something in order to read. + +Once the tool has exited the overlay is a still image: `esc` closes it, every +other key does nothing, and nothing is forwarded because there is nothing left +to forward to. + +Quitting keepkit while the overlay is open is unreachable by design. The way out +is quitting the tool (or `ctrl+\`), then `esc`. If keepkit itself is killed, the +closing pty master HUPs the child — accepted. + +## Geometry, and the two things that must not move + +The block is 70% of the screen in both directions, centred by `ui.PlaceOverlay`. +`Styles.OverlayBorder` is a rounded border plus `Padding(0, 1)`, so the emulator +body is `outerW - 4` by `outerH - 2 - 2` — the two rows being the title and the +exit row. At the 80×24 baseline that is **52×11**, and the baseline succeeding +is the number that mattered. + +Two invariants keep the block still, and they fail differently: + +- **The exit row is reserved from the start** — blank while the tool runs. Without + it the block would grow by a row at the moment the tool finishes and + `PlaceOverlay` would re-centre it vertically. +- **Every row is padded to the body width.** lipgloss sizes a border to its + widest line, so before this the block measured 54 cells running and 53 exited + (the title loses its `ctrl+\ kill` hint) and jumped sideways at the exact + moment the user starts reading the outcome. `termRow` clamps both directions, + ANSI-safely; the case it really exists for is a tool name longer than the body, + which would otherwise push the frame past the screen edge where `PlaceOverlay` + clips silently. + +`termGeometry` returns the size **and** a separate `ok`, because the two callers +want different things: the keypress refuses on `!ok` (`terminal too small to run +a tool`, no `lastRun` write — a launch that never started is not remembered), +while a terminal shrunk *mid-session* clamps to the floor and keeps the tool +running. Resizing somebody's editor into nothing is bad; killing it outright is +worse. `!m.ready` is checked first and explicitly, the `toggleZoom` idiom: before +the first `WindowSizeMsg` every dimension is zero and the percentages would agree +on a floor-sized overlay by coincidence rather than by measurement. + +Resize propagation hangs off **`applyLayout`**, the single relayout point, so it +cannot drift from it; the emulator is re-laid-out first and the pty told second, +so the child's redraw lands on a screen that is already the right shape. + +## The cursor + +The child's cursor is a reverse-video cell while the tool runs, hidden once it +has exited — a cursor on a dead screen invites typing. The pinned x/vt exposes +`CursorPosition()` but no visibility accessor, so alive/exited is the whole rule. + +The splice goes through `ansi.Truncate`/`ansi.TruncateLeft`, which cut by +**visible column** and keep the SGR runs on both sides intact. A rune-index cut +would land inside an escape sequence, which the terminal then executes. Padding +happens *before* the splice for a related reason: on a short line, a cursor past +its end would otherwise be spliced in right after the last glyph instead of +where the tool actually put it. + +`ui.Styles.TermCursor` is the only style in the set naming no theme colour, and +that is the point — reverse video swaps whatever the *tool* painted into that +cell, which is the only way one style can mark a cursor on a screen keepkit does +not control. A terminal with no SGR support shows no cursor; a terminal with no +SGR support cannot show a cursor block anyway. + +## The pty session + +`internal/term` is the architectural slot `internal/launcher` vacated: bottom of +the import graph, no TUI knowledge, no config paths, no `logx` — a failure rides +`Exit.Err` to the caller, which is the surface that can actually show it. That is +why it needs no `TestMain` seam. + +`Session` owns one pty and one reader goroutine. Events leave through +`Events() <-chan Event`: zero or more `Data`, then exactly one `Exit`, then the +channel closes. The channel is buffered (64) so the reader stays ahead of a +consumer that drains once per Bubble Tea message; past it the reader blocks and +the child is throttled, which is the correct back-pressure. + +Three rules inside it: + +- **A pty read error is never the verdict.** A master whose child has exited + answers `EIO` on Linux and EOF on macOS, and one closed by `Kill`/`Close` + answers `os.ErrClosed`. All three are normal termination; what happened comes + from `xpty.WaitProcess`, which also synthesises the `*exec.ExitError` that + `os.Process.Wait` fails to produce on ConPTY. +- **`Elapsed` and `Killed` are stamped in the goroutine.** `time.Now()` inside + `Update` would make completion non-deterministic in tests, and only the + session knows whether an exit followed a kill — making the model correlate its + own keypress against `signal: killed` would be guesswork. +- **Never call `proc.DetachTTY` on the pty command.** xpty sets neither `Setsid` + nor `Setctty` (its `Start` only wires stdio), so `internal/term` sets them + itself; `DetachTTY` assigns `SysProcAttr` wholesale and would drop them, + leaving the child with no controlling terminal — the exact opposite of the + point. `Setsid` is also what makes the unix child a process-group leader, so + `proc.KillGroup`'s negative-pid signal reaches the tools an `sh -c` line + started. + +The model builds argv with its existing `shellCommand`, which is what keeps +`internal/term` goos-agnostic. + +## Message flow, and the ordering that is easy to break + +``` +enter (prompt) → startTermCmd ──► termStartedMsg{session} + │ Update creates the emulator here + ▼ + waitForTermChunkCmd ──► termChunkMsg{data, exit?} + ▲ │ + └────────────────────┘ + │ exit != nil + ▼ + termExitMsg +``` + +`waitForTermChunkCmd` folds everything already queued into one message — a +full-screen redraw arrives as several reads and repainting once per read would +spend a frame on each. + +**The drain carries the exit rather than swallowing it.** A Go channel cannot be +un-read, so when the drain runs into the final event it has nowhere to put it +back; it rides on `termChunkMsg.exit`, the handler writes the data first and +re-emits the exit as the *next* message. Delivering the exit before the data it +followed would lose a short-lived tool's final screen, which is precisely what +esc-after-exit exists to show. + +Two guards that are structural, not cosmetic: + +- **A chunk whose session is not the current one is dropped without + re-subscribing.** A dead session's channel must not keep a command chain alive. +- **A `termStartedMsg` arriving outside the mode is killed and closed, not + adopted.** A start that lost its race with `esc` would otherwise strand a live + pty nothing can reach. +- **Keys before `termStartedMsg` are dropped.** The window is milliseconds, but a + nil deref there would re-panic through `logx.Recover` and take keepkit down. + +## The outcome line + +The reserved row fills in with one of four verdicts, built from the update +block's own helpers (`formatElapsed`, `fitCells`, `footerSep`): + +``` +✓ exited · 12s · esc close +✕ exit 3 · 4s · esc close +✕ killed · 1m30s · esc close +✕ failed to start · no such file · esc close +``` + +A start failure spends the middle cell on its reason rather than on an elapsed +it does not have. There is **no status message** on exit: the screen the tool +left behind is the answer and this row is the caption on it. The status bar gets +a branch of its own for the same reason the siblings have one — without it the +bar would go on advertising six global keys the running tool has taken over, +including a `q quit` that cannot fire. + +A tool that is not installed needs no special handling: `sh` reports it and exits +127, which the `✕ exit 127` line shows. That replaced the old `notFoundExit` +mapping the tab launcher needed. + +## What was deleted, and what that bought + +`internal/launcher` (the five-adapter detection chain, `Plan`, `appleScriptQuote`) +and, in the model: `startLaunchCmd`, `execToolCmd`, `launchDoneMsg`/`execDoneMsg`, +`m.launchingFor`, `pendingLaunchName`/`Command`, `flushPendingLaunch` and every +modal-return call site it wrapped, `launchTimeout`, `launchFallbackStatus`, +`notFoundExit`, and `setStickyStatus` (whose only callers were the two launch +statuses). + +Most of that existed to survive an adapter failing — a fallback, a deferred +fallback for when the fallback could not seize the terminal safely, a guard so +two of them could not run at once, and a sticky status because the in-flight +message had to outlive its own timer. None of it has anything to answer now: the +overlay is keepkit's own screen, there is no adapter to fail and no terminal to +seize. One mode replaced the lot. + +`shellCommand` survives and now builds argv for the pty. Its deliberate duplicate +in `updater.customPlan` still must not drift. + +## Known limits + +- **Windows works via ConPTY but is untested at runtime** — CI only + cross-compiles, the same level of assurance as `restart_windows`. `ConPty.Start` + populates `cmd.Process`, so `proc.KillGroup`'s Windows branch needs no change. +- **Mouse is not proxied.** `SendMouse` exists; nothing wanted it yet. +- **Child OSC title and bell events are ignored.** +- **Wide graphemes split across two reads can render wrong** (charmbracelet/x#935). + The reader delivers large chunks and the failure is cosmetic and transient. +- **Working directory is inherited** from keepkit, not from anything the tool + chose. + +`docs/research/pty-stack.md` records why this stack was chosen, what was +rejected, and every pinned version's reason. diff --git a/docs/design/updating.md b/docs/design/updating.md index 6775847..67e893c 100644 --- a/docs/design/updating.md +++ b/docs/design/updating.md @@ -8,7 +8,7 @@ Read this before touching `internal/updater`, update rides this same pipeline; its banner, version gate and restart are described in [`self-update.md`](self-update.md). -- **Update (`enter` in `focusBrief`)**: installs a newer release from inside the TUI. `enter` fires the card's primary action in `focusBrief` (in `focusTools` the same key runs the selected tool); it requires `hasUpdate(name)` (else `statusMsg`) and reports the shared `updateBusyStatus` (`another update is running`) while `updatingFor != ""` instead of starting a second one (one update at a time, no queue; the same wording `[U]` uses, since the running update's only other sign is a card spinner invisible unless that tool is selected). The whole guard sequence lives in `startToolUpdate()` — a **pointer receiver**, because it sets a status message, and its caller assigns the command before returning `m`, since Go copies the model into the return before evaluating the second operand. It fires `detectUpdateCmd(t, false)` — detection spawns subprocesses (`go version -m`, `cargo install --list`) so it must never run inside `Update()`, same as every other probe. `updater.Detect` runs the brew → go → cargo → pipx → uv → pnpm → bun → npm chain. **Order is load-bearing twice**: brew before go, so a brew-installed Go binary with buildinfo isn't misrouted, and pnpm/bun before npm, because both layouts carry `node_modules` segments the npm step claims on sight — a bun global (`$BUN_INSTALL/bin/` → `install/global/node_modules//…`) really did resolve to `npm install -g `, the wrong manager, installing a duplicate under npm's prefix that the bun copy keeps shadowing on `PATH`. Five steps are **path-convention** based (cargo, pipx, uv, pnpm, bun), and an empty root switches its own step off — enforced **inside `underDir`/`segmentUnder`**, which answer "no match" for an empty dir, deliberately *not* by a `!= ""` guard repeated at each step. The hazard is real: `filepath.Rel("", "bin/exa")` succeeds and yields `bin/exa`, so a **relative** path reads as living under every disabled root, and one forgotten copy of the guard silently claims it. It lived as six hand-copied copies of that one convention until the review that turned it into one definition — the same lesson as `version.applyReleaseOutcome` (`TestPathHelpersRejectEmptyDir`, plus a per-step `empty leaves a relative path undetected` row): `uv` = `segmentUnder(realPath, uvTools)` → `uv tool upgrade `; `pnpm` = `underDir(realPath, pnpmHome)` plus a name from the shim target or the resolved path → `pnpm add -g `; `bun` = `underDir(realPath, bunInstall)` + `npmPackage(realPath)` → `bun add -g `. **`add -g`, not `update -g`**: `update` honours the semver range saved at install time and can silently refuse a major bump, while keepkit promises the version the card shows as `latest:`. A manager's own binary carries no `node_modules` segment and no shim target, so it falls through — `bun upgrade`/`pnpm self-update` are deliberately out of scope. **All five roots** come from **`managerDirsFrom(getenv, home, goos)`** (pure core, `resolveManagerDirs()` wrapper — the `launcher.planFor` idiom), and carrying cargo/pipx there too is what lets `detectFromPath` stop calling `homeDir()`, making its "no I/O, no environment" contract literally true instead of nearly true. cargo and pipx stay home-derived with **no env var of their own** — `$CARGO_HOME`/`$PIPX_HOME` are not consulted, exactly as before the struct existed, since honouring them changes behaviour for anyone who sets them and belongs in its own commit. `/.cargo/bin` and `/.local/pipx/venvs`; `$UV_TOOL_DIR` else `$XDG_DATA_HOME/uv/tools` else `~/.local/share/uv/tools` (macOS included — uv is XDG there too; **Windows env-only**, its layout is unverified and a wrong guess beats no guess only in the wrong direction), `$PNPM_HOME` else `~/Library/pnpm` (darwin) / `$XDG_DATA_HOME/pnpm` else `~/.local/share/pnpm` (linux) / `%LOCALAPPDATA%\pnpm` (windows), `$BUN_INSTALL` else `~/.bun`. An empty field is a *disabled* check, which is what keeps the zero-value `managerDirs{}` backwards compatible. The wrapper then **expands symlinks in every one of the five roots** (`resolveDir` → `filepath.EvalSymlinks`, falling back to the raw path when it does not resolve — a root that does not exist yet is the normal state for an uninstalled manager and must stay a harmless *non-match* rather than becoming an empty, i.e. disabled, field). That expansion is load-bearing, not hygiene: `Detect` compares these roots against an `EvalSymlinks`-**resolved** binary path, so a root carrying any symlink component — a relocated `~/.bun` → `/mnt/big/bun`, a home on a secondary volume, `/home` under autofs, and on macOS `/var` → `/private/var`, which is what made every fixture test here need `EvalSymlinks(t.TempDir())` — silently fails to match. For uv and pnpm's shim/store layouts that only costs the update offer, but a bun global and a legacy pnpm one still carry a plain `node_modules/` segment, so the npm step claims them and offers `npm install -g ` — the exact duplicate-install misdetection this chain exists to prevent, measured on all five layouts and pinned by `TestDetectSymlinkedManagerRoot`. The expansion is a **loop over pointers to all five fields**, so a sixth field added to `managerDirs` but forgotten there would resolve to a root nothing can ever match; `TestResolveManagerDirsExpandsSymlinks` asserts all five at once as the guard. **pnpm needs a second signal** because its globals are not symlinks at all: `$PNPM_HOME/bin/` is a cmd-shim `/bin/sh` script `EvalSymlinks` resolves to itself, whose last line — `# cmd-shim-target=` — is the only machine-readable link to the owning package. `Detect` reads it (best-effort, capped at `pnpmShimMaxBytes` = 8 KiB against real shims of ~1.5 KiB, and **only** when the found path sits under a non-empty `pnpmHome` — reading every binary on `PATH` to look for a comment would be an open per detection) through `readPnpmShim` → the pure `pnpmShimTarget` (last marker wins: cmd-shim writes it as the final line). A file **over** the cap is rejected whole (`LimitReader(cap+1)` + a length check, `getReadme`'s idiom) rather than parsed truncated, and that is a correctness guard, not tidiness: a cut landing inside the marker line hands the parser a *shortened* path, and `…/node_modules/typescript/…` shortened to `…/node_modules/types` yields `types` — a real package on npm, so the chain would offer a confidently wrong `pnpm add -g types` where everything else here degrades honestly (`TestReadPnpmShim`'s oversized row, whose fixture asserts the truncated parse *would* have said `types`), and passes it into the core as `shimTarget` exactly the way `goBuildinfo` rides in; an unreadable file yields `""`, i.e. the check simply has no signal. Two npm-side consequences of the pnpm layout: `npmPackage` **refuses a bare scope** (`…/node_modules/@angular` with nothing after it is a directory holding packages, not one — returning it offered `pnpm add -g @angular`, and the pnpm step feeds this function a *shim target*, i.e. file content nothing validated) and **skips a post-`node_modules` segment starting with `.`** (`.pnpm` is the virtual store, `.bin` the shim dir — never package names) and keeps scanning, and the **npm step refuses any path through `/node_modules/.pnpm/`** — such a path reaching npm means a pnpm layout the pnpm step failed to attribute, and `npm install -g ` there is the duplicate-install failure again, so the chain falls through to `ErrUnknownManager` + the `update_cmd` hint (honest degradation). Detection is convention-based end to end: a future layout change makes its check miss *silently* and degrade to the hint, never to a wrong command. A `update_cmd` in `meta.yaml` always wins and runs via the platform shell — `sh -c`, or `cmd /c` on Windows, so a `winget`/PowerShell `update_cmd` needs no Git Bash (`customPlan(goos, cmd)`, the pure goos-parameterized core, deliberate sibling of `model`'s `shellCommand`) — and skips detection entirely. The chain starts from `exec.LookPath(t.Name)`, and a **miss there is not the end**: the tracked name can still be a brew formula whose binaries are named differently — `rust` ships `rustc`/`cargo`, so `LookPath("rust")` misses while `brew upgrade rust` is exactly right — so `brewNamePlan` checks for a `Cellar/`/`Caskroom/` directory before `ErrUnknownManager` is returned. **An exhausted chain takes the same fallback**: a binary can be found and still belong to no known manager — a Homebrew cask app keeps its launcher on `PATH` while the executable lives inside the `.app` bundle (`agterm`), matching neither the Cellar regex nor any later step — so `Detect` retries `brewNamePlan` on `ErrUnknownManager` from `detectFromPath` too (`TestDetectBrewByNameChainExhausted`). Both sites are self-validating (they fire only when such a keg actually exists) and share `version.brewDirVersion`'s traversal guard: a name carrying `/` or `\` can't be a formula name and must not turn the `Join` into a traversal. The `updateDetectedMsg` handler drops a stale result through the shared predicate **`acceptsUpdateDetect(msg)`**: both paths refuse while `updatingFor != ""` and while `m.mode != modeNormal` (detection spawns subprocesses and can answer seconds later — a confirm dialog opening under an editor, a search or an overlay steals the keystroke aimed at it, the mirror of `launchDoneMsg`'s mode gate), and beyond that a *tool* result must still match the selection while keepkit's own has no selection to match. It maps `ErrUnknownManager` to a `statusMsg` hint (`no known updater for — set update_cmd or [o] releases` — no dead-end dialog; the wording branches on **`isSelfUpdate(msg.tool)`**, not on `msg.self`, so the identical failure of the identical binary reads the same from `enter` on a tracked `keepkit` row as from `[U]` — `msg.self` keeps the one meaning only it has, "no selection to match", inside `acceptsUpdateDetect`), and on success stores `m.updatePlan` plus **`m.updateTarget = msg.tool`** and enters `modeConfirmUpdate`. The target is resolved *there*, not when enter is pressed: a selection that moved while detection ran can no longer retarget the dialog, and keepkit's own update — which has no row, and no selection at all with an empty tracker — needs no second identity. The confirm status bar shows `update : enter run esc cancel`; `enter` sets `m.updatingFor`/`m.updateLogFor` to that target, resets the log, and fires `startUpdateCmd` (any other key cancels, not just `esc`, and clears `updateTarget` with the plan it named). The `enter update to ` cell leads the `[2]` panel footer only when `hasUpdate(selected)`. +- **Update (`enter` in `focusBrief`)**: installs a newer release from inside the TUI. `enter` fires the card's primary action in `focusBrief` (in `focusTools` the same key runs the selected tool); it requires `hasUpdate(name)` (else `statusMsg`) and reports the shared `updateBusyStatus` (`another update is running`) while `updatingFor != ""` instead of starting a second one (one update at a time, no queue; the same wording `[U]` uses, since the running update's only other sign is a card spinner invisible unless that tool is selected). The whole guard sequence lives in `startToolUpdate()` — a **pointer receiver**, because it sets a status message, and its caller assigns the command before returning `m`, since Go copies the model into the return before evaluating the second operand. It fires `detectUpdateCmd(t, false)` — detection spawns subprocesses (`go version -m`, `cargo install --list`) so it must never run inside `Update()`, same as every other probe. `updater.Detect` runs the brew → go → cargo → pipx → uv → pnpm → bun → npm chain. **Order is load-bearing twice**: brew before go, so a brew-installed Go binary with buildinfo isn't misrouted, and pnpm/bun before npm, because both layouts carry `node_modules` segments the npm step claims on sight — a bun global (`$BUN_INSTALL/bin/` → `install/global/node_modules//…`) really did resolve to `npm install -g `, the wrong manager, installing a duplicate under npm's prefix that the bun copy keeps shadowing on `PATH`. Five steps are **path-convention** based (cargo, pipx, uv, pnpm, bun), and an empty root switches its own step off — enforced **inside `underDir`/`segmentUnder`**, which answer "no match" for an empty dir, deliberately *not* by a `!= ""` guard repeated at each step. The hazard is real: `filepath.Rel("", "bin/exa")` succeeds and yields `bin/exa`, so a **relative** path reads as living under every disabled root, and one forgotten copy of the guard silently claims it. It lived as six hand-copied copies of that one convention until the review that turned it into one definition — the same lesson as `version.applyReleaseOutcome` (`TestPathHelpersRejectEmptyDir`, plus a per-step `empty leaves a relative path undetected` row): `uv` = `segmentUnder(realPath, uvTools)` → `uv tool upgrade `; `pnpm` = `underDir(realPath, pnpmHome)` plus a name from the shim target or the resolved path → `pnpm add -g `; `bun` = `underDir(realPath, bunInstall)` + `npmPackage(realPath)` → `bun add -g `. **`add -g`, not `update -g`**: `update` honours the semver range saved at install time and can silently refuse a major bump, while keepkit promises the version the card shows as `latest:`. A manager's own binary carries no `node_modules` segment and no shim target, so it falls through — `bun upgrade`/`pnpm self-update` are deliberately out of scope. **All five roots** come from **`managerDirsFrom(getenv, home, goos)`** (pure core, `resolveManagerDirs()` wrapper — the `configdir.baseFor` idiom), and carrying cargo/pipx there too is what lets `detectFromPath` stop calling `homeDir()`, making its "no I/O, no environment" contract literally true instead of nearly true. cargo and pipx stay home-derived with **no env var of their own** — `$CARGO_HOME`/`$PIPX_HOME` are not consulted, exactly as before the struct existed, since honouring them changes behaviour for anyone who sets them and belongs in its own commit. `/.cargo/bin` and `/.local/pipx/venvs`; `$UV_TOOL_DIR` else `$XDG_DATA_HOME/uv/tools` else `~/.local/share/uv/tools` (macOS included — uv is XDG there too; **Windows env-only**, its layout is unverified and a wrong guess beats no guess only in the wrong direction), `$PNPM_HOME` else `~/Library/pnpm` (darwin) / `$XDG_DATA_HOME/pnpm` else `~/.local/share/pnpm` (linux) / `%LOCALAPPDATA%\pnpm` (windows), `$BUN_INSTALL` else `~/.bun`. An empty field is a *disabled* check, which is what keeps the zero-value `managerDirs{}` backwards compatible. The wrapper then **expands symlinks in every one of the five roots** (`resolveDir` → `filepath.EvalSymlinks`, falling back to the raw path when it does not resolve — a root that does not exist yet is the normal state for an uninstalled manager and must stay a harmless *non-match* rather than becoming an empty, i.e. disabled, field). That expansion is load-bearing, not hygiene: `Detect` compares these roots against an `EvalSymlinks`-**resolved** binary path, so a root carrying any symlink component — a relocated `~/.bun` → `/mnt/big/bun`, a home on a secondary volume, `/home` under autofs, and on macOS `/var` → `/private/var`, which is what made every fixture test here need `EvalSymlinks(t.TempDir())` — silently fails to match. For uv and pnpm's shim/store layouts that only costs the update offer, but a bun global and a legacy pnpm one still carry a plain `node_modules/` segment, so the npm step claims them and offers `npm install -g ` — the exact duplicate-install misdetection this chain exists to prevent, measured on all five layouts and pinned by `TestDetectSymlinkedManagerRoot`. The expansion is a **loop over pointers to all five fields**, so a sixth field added to `managerDirs` but forgotten there would resolve to a root nothing can ever match; `TestResolveManagerDirsExpandsSymlinks` asserts all five at once as the guard. **pnpm needs a second signal** because its globals are not symlinks at all: `$PNPM_HOME/bin/` is a cmd-shim `/bin/sh` script `EvalSymlinks` resolves to itself, whose last line — `# cmd-shim-target=` — is the only machine-readable link to the owning package. `Detect` reads it (best-effort, capped at `pnpmShimMaxBytes` = 8 KiB against real shims of ~1.5 KiB, and **only** when the found path sits under a non-empty `pnpmHome` — reading every binary on `PATH` to look for a comment would be an open per detection) through `readPnpmShim` → the pure `pnpmShimTarget` (last marker wins: cmd-shim writes it as the final line). A file **over** the cap is rejected whole (`LimitReader(cap+1)` + a length check, `getReadme`'s idiom) rather than parsed truncated, and that is a correctness guard, not tidiness: a cut landing inside the marker line hands the parser a *shortened* path, and `…/node_modules/typescript/…` shortened to `…/node_modules/types` yields `types` — a real package on npm, so the chain would offer a confidently wrong `pnpm add -g types` where everything else here degrades honestly (`TestReadPnpmShim`'s oversized row, whose fixture asserts the truncated parse *would* have said `types`), and passes it into the core as `shimTarget` exactly the way `goBuildinfo` rides in; an unreadable file yields `""`, i.e. the check simply has no signal. Two npm-side consequences of the pnpm layout: `npmPackage` **refuses a bare scope** (`…/node_modules/@angular` with nothing after it is a directory holding packages, not one — returning it offered `pnpm add -g @angular`, and the pnpm step feeds this function a *shim target*, i.e. file content nothing validated) and **skips a post-`node_modules` segment starting with `.`** (`.pnpm` is the virtual store, `.bin` the shim dir — never package names) and keeps scanning, and the **npm step refuses any path through `/node_modules/.pnpm/`** — such a path reaching npm means a pnpm layout the pnpm step failed to attribute, and `npm install -g ` there is the duplicate-install failure again, so the chain falls through to `ErrUnknownManager` + the `update_cmd` hint (honest degradation). Detection is convention-based end to end: a future layout change makes its check miss *silently* and degrade to the hint, never to a wrong command. A `update_cmd` in `meta.yaml` always wins and runs via the platform shell — `sh -c`, or `cmd /c` on Windows, so a `winget`/PowerShell `update_cmd` needs no Git Bash (`customPlan(goos, cmd)`, the pure goos-parameterized core, deliberate sibling of `model`'s `shellCommand`) — and skips detection entirely. The chain starts from `exec.LookPath(t.Name)`, and a **miss there is not the end**: the tracked name can still be a brew formula whose binaries are named differently — `rust` ships `rustc`/`cargo`, so `LookPath("rust")` misses while `brew upgrade rust` is exactly right — so `brewNamePlan` checks for a `Cellar/`/`Caskroom/` directory before `ErrUnknownManager` is returned. **An exhausted chain takes the same fallback**: a binary can be found and still belong to no known manager — a Homebrew cask app keeps its launcher on `PATH` while the executable lives inside the `.app` bundle (`agterm`), matching neither the Cellar regex nor any later step — so `Detect` retries `brewNamePlan` on `ErrUnknownManager` from `detectFromPath` too (`TestDetectBrewByNameChainExhausted`). Both sites are self-validating (they fire only when such a keg actually exists) and share `version.brewDirVersion`'s traversal guard: a name carrying `/` or `\` can't be a formula name and must not turn the `Join` into a traversal. The `updateDetectedMsg` handler drops a stale result through the shared predicate **`acceptsUpdateDetect(msg)`**: both paths refuse while `updatingFor != ""` and while `m.mode != modeNormal` (detection spawns subprocesses and can answer seconds later — a confirm dialog opening under an editor, a search or an overlay steals the keystroke aimed at it, and under the tool overlay it steals one the running tool is waiting for), and beyond that a *tool* result must still match the selection while keepkit's own has no selection to match. It maps `ErrUnknownManager` to a `statusMsg` hint (`no known updater for — set update_cmd or [o] releases` — no dead-end dialog; the wording branches on **`isSelfUpdate(msg.tool)`**, not on `msg.self`, so the identical failure of the identical binary reads the same from `enter` on a tracked `keepkit` row as from `[U]` — `msg.self` keeps the one meaning only it has, "no selection to match", inside `acceptsUpdateDetect`), and on success stores `m.updatePlan` plus **`m.updateTarget = msg.tool`** and enters `modeConfirmUpdate`. The target is resolved *there*, not when enter is pressed: a selection that moved while detection ran can no longer retarget the dialog, and keepkit's own update — which has no row, and no selection at all with an empty tracker — needs no second identity. The confirm status bar shows `update : enter run esc cancel`; `enter` sets `m.updatingFor`/`m.updateLogFor` to that target, resets the log, and fires `startUpdateCmd` (any other key cancels, not just `esc`, and clears `updateTarget` with the plan it named). The `enter update to ` cell leads the `[2]` panel footer only when `hasUpdate(selected)`. - **Streaming** (channel + re-subscribe idiom, no `*tea.Program`): `startUpdateCmd` runs the plan via `exec.Command` + `proc.DetachTTY` (10-min deadline; a sudo prompt fails fast instead of hanging — deliberate), with stdout+stderr merged into one pipe. **Reader ordering is load-bearing** (os/exec forbids `Wait` before pipe reads finish): the goroutine scans the pipe to EOF via `streamLines` → then `cmd.Wait()` → sends the exit error as a final `updateLine{done:true, err}` → then `close(ch)`. `waitForChunkCmd` does one receive → `updateChunkMsg`; a done item or closed channel → `updateDoneMsg`. The channel carries a typed `updateLine{text, replace, done, err, elapsed}` (not `chan string`) so the `replace` flag, the completion error and the duration all ride the same channel — no second error channel threaded through every re-subscribe. **`elapsed` is stamped here, after `Start` returns**, and not in the `updateDoneMsg` handler: `time.Now()` inside `Update()` would make completion non-deterministic in every test that drives it, while this is where the process actually lives. The two early returns (empty argv, a `StdoutPipe`/`Start` error) never reach the stamp and report zero, which the outcome block reads as "it never ran" and prints no duration cell for. Each segment is sanitized through `cleanTerminalOutput` (which already strips ANSI) at the boundary; `streamLines` splits on `\n` **and** `\r`, and a `\r` segment sets `replace` so brew/npm progress bars collapse to one updating line. `m.updateLog` is capped at ~500 lines (tail matters). On deadline, `proc.KillGroup` SIGKILLs the process group (negative pid — `DetachTTY`'s `Setsid` makes the child a session leader, so a plain kill would orphan `sh -c` grandchildren). **That is the unix path only.** On Windows there is no process group to signal: `proc.KillGroup` degrades to `Process.Kill` on the direct child, so a deadline kill ends the `cmd /c` wrapper while the real updater (`winget`, `powershell`) survives — still holding the pipe write handle it inherited. No EOF means `streamLines` never returns, so `cmd.Wait()` is never reached, the `done` item is never sent and `updatingFor` stays set for the rest of the session (every later update answers `another update is running`). `cmd.WaitDelay` does **not** fix it — `StdoutPipe` creates no copier goroutine, so `c.goroutineErr` is nil and os/exec's force-close of `parentIOPipes` only ever runs from inside `Wait()`, which is exactly what the drain is blocking. The fix is a real Windows process-tree kill (a Job Object with `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, or `taskkill /T /F`) in `internal/proc`; it is deferred alongside the `cmd /c` quoting caveat until a real Windows report justifies untestable per-GOOS code (no Windows runner executes this suite — CI cross-compiles only). - **Live log in `[3]`**: `m.updateLog []string` is a single active-session buffer (not a map); `m.updateLogFor` names its tool. Every site that asks "does the log own panel `[3]`?" asks the **single predicate `showsUpdateLog()`** (defined in [`self-update.md`](self-update.md)), never `updateLogFor` directly: `renderHelpContent()` returns the log **ahead of** the `helpLoadingFor`/cache branches, and `autoFetchCmdsForSelected` skips the help fetch (and `helpLoadingFor` set) — otherwise re-selecting the tool paints `Loading...` or a late `helpOutputMsg` clobbers the live log. The panel title comes from **`updateLogTitle()`** (same `insetPanelTitle` path as `[3] help`/`[3] man`, and the source hints are dropped with it): `[3] update` while one runs, then `[3] update finished` / `[3] update failed` once the session has ended. One string feeds both the title and the footer's source cell, so the two can never disagree about whether an update is still running, and a reader who has scrolled away from the end of a long log still learns it ended. **Words, not the block's own `✓`/`✕`**: `insetPanelTitle` measures the title in **runes**, not cells, so an East-Asian-Ambiguous glyph there renders two cells wide under `RUNEWIDTH_EASTASIAN=1` and pushes the top border out by one — inside the viewport the same glyphs are harmless, which is why the block keeps them. The panel autoscrolls to bottom on each chunk, and the buffer persists after completion until the next update. For a *tool* update the claim is per tool: navigating away shows the other tool's normal help; back shows the live log. - **Spinner + completion**: `m.updatingFor` twins `refreshingFor` — card title `updating `; the `spinner.TickMsg` gate is `refreshingFor != "" || updatingFor != ""` (or the spinner freezes after one frame). The `updateDoneMsg` handler clears `updatingFor`; success → `statusMsg "updated "` + `fetchInstalledCmd(t)` (the version merge extinguishes `↑` and the existing by-name cursor remap moves the tool out of the update group); failure → `statusMsg "update failed — see [3]"`. A tool untracked mid-update just clears `updatingFor` (no re-fetch, no crash). diff --git a/docs/plans/completed/20260814-tool-overlay-terminal.md b/docs/plans/completed/20260814-tool-overlay-terminal.md new file mode 100644 index 0000000..046700c --- /dev/null +++ b/docs/plans/completed/20260814-tool-overlay-terminal.md @@ -0,0 +1,267 @@ +# Tool overlay terminal: run tracked tools in an embedded PTY overlay + +## Overview + +- `enter` on the selected tool in `[1] tools` keeps opening the `modeRunInput` prompt (prefill: tool name / `lastRun`, editable), but the dispatched command now runs in an **embedded terminal overlay inside keepkit** (~70% of the screen, centered, `PlaceOverlay`-dimmed background) instead of a new terminal tab. +- One path for everything — **no TUI-vs-CLI distinction**: a TUI (vim/yazi/fzf) draws and lives in the overlay with the full keyboard proxied to it; a plain CLI prints and exits, and the overlay stays with the final screen + exit status until dismissed with `esc`. +- While the process is alive **every key (esc included) goes to the tool**; `ctrl+\` is the one reserved chord (kill). After exit, `esc` closes. This is the biggest feature of the project and **replaces the tab launcher entirely** — `internal/launcher`, the auto-fallback and `tea.ExecProcess` are deleted. + +## Context (from discovery) + +- Stack (researched 2026-08, decided): `github.com/charmbracelet/x/xpty` **v0.1.4** (tagged; unix pty via creack/pty + Windows **ConPTY**) + `github.com/charmbracelet/x/vt` (**untagged — pin the pseudo-version**; VT220+truecolor emulator: `NewEmulator(w, h)`, `Write` pty bytes in, `Render()` ANSI string out, `Resize`, `SendKey`/`SendText`, `InputPipe()`). Rejected: creack/pty direct (no Windows — ConPTY never merged), `taigrr/bubbleterm` (needs Bubble Tea v2; crib its emulator↔Model wiring only). +- x/vt has an open race issue (charmbracelet/x#879, Read/Close) and grapheme-split issue (#935) → **architecture rule: only `Update` touches the emulator's screen state**; the pty is read by one goroutine posting chunks as `tea.Msg`s — the `waitForChunkCmd` pattern from the update streamer. Note the rule sidesteps the *screen-buffer* races; the input path gets its own treatment (Task 1 decides between a key→bytes encoder — preferred, zero extra goroutines — and an `InputPipe` copy goroutine with an explicit lifecycle). +- x/vt master pulls `charmbracelet/ultraviolet` (Bubble Tea v2 rendering core) into the module graph and will likely bump `x/ansi` (pinned v0.11.6 — `ui.StripANSI` delegates to it), `x/cellbuf`, `colorprofile` — all transitive deps of lipgloss/glamour, i.e. **the dependency bump alone can move every render test**. Task 1's gate is therefore the full suite, not the new package. +- Existing anchors (verified by review): `shellCommand` (`internal/model/commands.go:117`) stays — the model builds argv with it and hands it to `term.Start`. `updateRunInput` (`internal/model/mode.go:330`) is the dispatch point to rewire; its refusal path deliberately skips the `lastRun` write (`mode.go:346`). `setStickyStatus`'s only callers are the two launch statuses (`mode.go:353`, `mode.go:371`) — it dies with them, plus `TestInFlightStatusSurvivesStaleExpiry` in `status_test.go`. `mode_test.go`'s `TestRunInputEnterStoresLastRun` asserts the post-enter mode and **will break at the switch** — it is in Task 5's file list. +- keepkit is on bubbletea v1.3.10; x/vt works in a v1 app — `Render()` returns a plain ANSI string. +- Patterns to follow: update streamer (`startUpdateCmd`/`waitForChunkCmd`, elapsed stamped in the cmd, never in `Update`), `modeAPIStatus` modality, `updateOutcomeBlock`'s helpers (`formatElapsed`, `fitCells`, `footerSep` — `render.go:2370`) for the exit line, the `restarter` narrow-interface idiom from main.go for the session seam, `toggleZoom`'s `!m.ready`-first refusal idiom. + +## Development Approach + +- **testing approach**: Regular (code first, then tests in the same task) +- complete each task fully before moving to the next +- make small, focused changes; the old launch path stays intact and green until the single switch-over task (Task 5) +- **CRITICAL: every task MUST include new/updated tests** for code changes in that task + - tests are not optional - they are a required part of the checklist + - unit tests for new and modified functions, covering success and error scenarios +- **CRITICAL: all tests must pass before starting next task** (`go test -race ./...`) - no exceptions +- **CRITICAL: update this plan file when scope changes during implementation** (➕/⚠️ prefixes) +- no meta.yaml schema changes; no config files written by this feature + +## Testing Strategy + +- **unit tests**: required for every task; the per-task gate is `go build ./...` + `go vet ./...` + `go test -race ./...` + `golangci-lint run` (full tree, not just the touched package — see the dependency-bump note above), plus `GOOS=windows go build ./...` wherever a build tag or dependency changes. Note: every symbol added in Tasks 2–4 is unreferenced from non-test code until Task 5 — **its own task's tests are what keep `unused` quiet**. +- **e2e tests**: none in this project (TUI verified by model-level tests, per existing convention); x/vt being pure Go means even alt-screen/truecolor rendering is assertable in model tests +- **real pty**: only in `internal/term`'s own tests (unix `sh -c echo …`, no network), under `-race`. `internal/term` writes no config and does no `logx` logging (failures ride `Exit.Err`), so it needs no `TestMain` seam — stated in the package doc, not left as an omission. +- **model tests never execute the cmd returned by the run-prompt enter** — it would spawn a real pty; assert on the returned cmd/state only (the `assertOnlyExpiryTick` hazard). + +## Progress Tracking + +- mark completed items with `[x]` immediately when done +- add newly discovered tasks with ➕ prefix +- document issues/blockers with ⚠️ prefix +- keep plan in sync with actual work done + +## Solution Overview + +- New package **`internal/term`** — bottom of the import graph, no TUI knowledge (the architectural slot `internal/launcher` vacates): `Session` owns the pty lifecycle. The **model builds argv with its existing `shellCommand`** and passes it to `term.Start` — `internal/term` stays goos-agnostic and needs no duplicate. `Start` creates the `xpty.Pty`, starts **one reader goroutine** pty → chunk channel; the exit (err + elapsed, stamped in the goroutine) lands as the final event on the same channel, then the channel closes. +- **`vt.Emulator` lives on `Model`, not in `internal/term`**: only `Update` writes to its screen state (`termChunkMsg` → `m.termEmu.Write`). Input: ⚠️ *decided in Task 1* — the preferred encoder **does not exist** in the pinned stack and could not be written correctly (DECCKM is unexported), so this is the **copy goroutine**: `Update` calls `emu.SendKey`/`SendText`, a per-session goroutine relays `emu.Read` → `Session.Write`, and teardown closes **`InputPipe()`'s writer** (never `Emulator.Close()`, which is upstream race #879). +- New **`inputMode: modeToolOverlay`** — modal like `modeAPIStatus`: while the process is alive its handler forwards every key to the tool (esc included) and reserves only `ctrl+\` (kill via `Session.Kill`); after `termExitMsg` only `esc` acts (cleanup: close session, `termEmu = nil`, back to `modeNormal`). `overlayVisible()` gains this third member — **mouse gating rides along; `View()`'s fg picker does not** (it is a two-way `if` that must become a `switch m.mode`, done in Task 2 with a placeholder body). One overlay at a time is structural (the mode) — no `launchingFor`-style guard. +- Geometry (owned by `termGeometry`, which must agree with the frame arithmetic, so it lives in Task 4): the **outer block** is 70% of width/height, centered; `Styles.OverlayBorder` is a rounded border **plus `Padding(0, 1)`**, so the emulator body is `emuW = outerW - 4`, `emuH = outerH - 2 - 1 (title row) - 1 (exit row)`. The **exit row is reserved from the start** (blank while running) so the block height never changes and `PlaceOverlay` never re-centers mid-session. Min clamps ≈40×10 on the emulator body; below that — and before the first `WindowSizeMsg` (`!m.ready`, checked first per the `toggleZoom` precedent) — the keypress refuses honestly with a statusMsg. `View()` wraps the composite in `Margin(1, 0)` (the background PlaceOverlay measures is the layout, not the terminal) and PlaceOverlay clips silently past the bottom — the clamps must account for both. + +## Technical Details + +- `internal/term` API sketch (verified against the real packages as Task 1's **first** item, with a stop condition): + - `Start(shell string, args []string, w, h int, env []string) (*Session, error)` — env gets `TERM=xterm-256color`, `COLORTERM=truecolor` appended; cwd inherited + - `Events() <-chan Event` where `Event` is either `Data []byte` or the final `Exit{Err error, Elapsed time.Duration}` + - `Resize(w, h int) error`, `Write(p []byte) (int, error)`, `Kill()` + - reader posts bounded chunks (≈32 KiB buffer). **pty EOF semantics**: reading the master after child exit returns `EIO` on Linux, EOF on macOS — both are normal termination, never `Exit.Err`; the verdict comes from `cmd.Wait()`/xpty's wait alone + - `waitForTermChunkCmd` on the model side drains pending chunks non-blockingly into one msg; **a drain that reaches `Exit` stops there and delivers the accumulated data first** — the exit goes out as the next message (otherwise a short-lived CLI's final screen is lost, defeating the esc-after-exit design) +- Messages: `termStartedMsg{session}` (a start error surfaces as an immediate `termExitMsg`), `termChunkMsg{data []byte}`, `termExitMsg{err, elapsed}`. Elapsed is stamped in the session goroutine, never in `Update`. +- Model state: `m.termSession` (narrow interface, fake-able), `m.termEmu`, `m.termExit *termExitMsg` (nil while running), `m.termW/termH`, `m.termToolName`. **Pre-`termStartedMsg` window**: session and emulator are nil between dispatch and the started msg — keys are dropped, `ctrl+\` is a no-op, the overlay renders a `starting …` body (the `starting update…` idiom); a panic here would re-panic through `logx.Recover` and crash keepkit, so the guard is structural, not cosmetic. +- Rendering (`render.go`): frame `Styles.OverlayBorder`, title = tool name with a dim `ctrl+\ kill` hint on the right while running; body = `m.termEmu.Render()` (exactly w×h); the reserved bottom row carries the outcome line after exit, built from `updateOutcomeBlock`'s helpers: `✓ exited · · esc close` / `✕ exit · · esc close` / `✕ killed · · esc close` / `✕ failed to start · · esc close`. No statusMsg on exit — the screen is the answer. **The child's cursor is rendered**: reverse-video the cell at the emulator's cursor position while the process is alive (hidden after exit; honour the emulator's cursor-visibility state if exposed). +- Status bar: `renderStatusBar` gets a `modeToolOverlay` branch (its siblings all have one; without it the bar advertises six dead global keys incl. a `q quit` that cannot fire): while running ` running ctrl+\ kill`, after exit ` exited esc close` — within the no-truncate budget (`TestStatusBarNeverWraps`). +- Key translation: rune keys → text input, named keys (arrows, enter, backspace, tab, home/end, pgup/pgdn, F-keys, ctrl+letter) → key events; exact mechanism (encoder vs `SendKey`+pipe) fixed in Task 1. Mouse is **not** proxied in v1 (`SendMouse` exists — YAGNI). Child OSC title/bell events ignored in v1. +- Not-installed tool: `sh` prints `command not found` into the overlay and exits 127 — visible with the ✕ line, no special handling (replaces the old `notFoundExit` mapping). +- Kill path: **never call `proc.DetachTTY` on the pty command** — ⚠️ *corrected in Task 1*: xpty sets neither `Setsid` nor `Setctty`, so `internal/term` sets them itself and `DetachTTY` would assign `SysProcAttr` wholesale and drop them. Unix kill goes to the process group (negative pid) off that session, via the existing `proc.KillGroup`; on the ConPTY path `cmd.Process` **is** populated (`os.FindProcess` in `ConPty.Start`), so `KillGroup`'s Windows branch needs no change. +- Quit-while-open is unreachable by design (all keys go to the tool): the way out is quitting the tool (or `ctrl+\`), then `esc`. If keepkit itself dies (SIGTERM/kill), the closing pty master HUPs the child — accepted. +- Windows: works via ConPTY (`xpty`); runtime untested by CI — accepted, the same level as `restart_windows`; the existing `GOOS=windows go build` cross-compile step covers the build. +- Launch during a running update stays allowed (independent concerns; the update log keeps streaming into `[3]` under the dim). +- go.mod: `x/xpty v0.1.4` (tagged) + `x/vt` pinned pseudo-version; upstream issues #879/#935 are tracked limitations. + +## What Goes Where + +- **Implementation Steps** (`[ ]` checkboxes): code, tests, docs — all inside this repo. +- **Post-Completion** (no checkboxes): manual verification in real terminals (real vim/yazi/fzf sessions, Windows smoke test), demo GIF decision — external to unit-testable code. + +## Implementation Steps + +### Task 1: dependencies + `internal/term` package — pty session with reader goroutine + +**Files:** +- Create: `internal/term/session.go` +- Create: `internal/term/session_test.go` +- Create: `docs/research/pty-stack.md` +- Modify: `go.mod`, `go.sum` + +- [x] **first, before writing `Session`**: `go get github.com/charmbracelet/x/xpty@v0.1.4` + `github.com/charmbracelet/x/vt@latest`, pin the vt pseudo-version, and verify the researched API against the real packages (`NewEmulator`/`Render`/`SendKey`/`InputPipe`, cursor accessors, `xpty.NewPty`/`Start`/`Resize`); **check whether x/vt exposes a key→bytes encoder** (`vt.EncodeKey`-style) that would let `Update` translate keys and call `Session.Write` directly — no second goroutine, the Update-only rule becomes literally true. **Stop condition: if `Render() string`, the input mechanism or the cursor API differ materially from the sketch, stop and re-plan Tasks 2–4 before writing code**; record drift with ➕ + - vt pinned at `v0.0.0-20260813141921-f091cedeaf78`. `Render() string`, `NewEmulator`, `Resize`, `CursorPosition()`, `IsAltScreen()`, `xpty.NewPty`/`Start`/`Resize` all match the sketch — **stop condition not tripped**, Tasks 2–4 stand. + - ➕ **drift 1 — there is no key encoder.** `ultraviolet` exports output-side `Encode*` only; `x/ansi` has none. Worse, `vt.SendKey` encodes against `isModeSet(ansi.ModeCursorKeys)` (DECCKM — the mode vim/less set, where arrows are `\x1bOA` not `\x1b[A`) and **no accessor exposes it**, so a hand-rolled encoder would be wrong for exactly the full-screen tools this feature exists for. → **Task 3 takes the plan's fallback: `SendKey` + a copy goroutine.** + - ➕ **drift 2 — the `Emulator.Close()` teardown is the upstream race.** #879 reproduced here under `-race` (`e.closed` written by `Close`, read by every `Read`). **Workaround: never call `Emulator.Close()`** — `InputPipe()` returns the `*io.PipeWriter`; closing *that* unblocks the parked `Read` through `io.Pipe`'s own synchronisation and never touches the bool. Verified race-clean; pinned by `TestSessionInputPipeCloseIsRaceFree`, with a fallback to `Close()` if the assertion ever fails. + - ➕ **drift 3 — `xpty` does *not* make the child a session leader.** The plan assumed "the pty's own `Setsid`+`Setctty`"; `UnixPty.Start` only wires stdio and calls `cmd.Start()`. `internal/term` sets `SysProcAttr{Setsid: true, Setctty: true}` itself (unix build). The **no-`DetachTTY` invariant survives with a sharper reason**: it assigns `SysProcAttr` wholesale and would drop our `Setctty`. + - ➕ **Windows kill needs no detour**: `ConPty.Start` populates `cmd.Process` via `os.FindProcess`, so `proc.KillGroup`'s existing Windows branch works as-is. +- [x] record the transitive bumps the vt/xpty pull causes (`x/ansi`, `x/cellbuf`, `colorprofile`, new `ultraviolet`) with ➕; if any existing render test in `internal/model`/`internal/ui` moves under the bumped deps, fix or pin **before** Task 2 so breakage is attributed to the bump, not to later feature code + - ➕ bumps: `x/ansi` 0.11.6→0.11.7, **`go-runewidth` 0.0.19→0.0.23**, **`displaywidth` 0.9.0→0.11.0**, `uax29/v2` 2.5.0→2.7.0, `go-colorful` 1.3.0→1.4.0, `colorprofile` 0.4.1→0.4.2, `x/sys` 0.38→0.47, `x/sync` 0.17→0.19, new `ultraviolet`. `x/cellbuf` unchanged. **Full suite run on the bump alone before any feature code: all green** — no render test moved. +- [x] drop the 2026-08 stack research (candidates, rejection reasons, pinned versions, upstream issues #879/#935) into `docs/research/pty-stack.md` so the pin has a rationale that outlives this plan +- [x] implement `Session`: `Start(shell, args, w, h, env)` → xpty + command (env: `TERM=xterm-256color`, `COLORTERM=truecolor`; cwd inherited), one reader goroutine → events channel; final `Exit{Err, Elapsed}` (elapsed stamped in the goroutine) then close. **Treat `EIO`/`os.ErrClosed` on the master read as normal EOF** (Linux vs macOS differ) — the verdict comes from the wait, never from the read error + - ➕ `Exit` carries a third field, **`Killed bool`**: the session is the only place that knows both the kill and the status, so it answers rather than making the model correlate its own keypress against `signal: killed`. This is what feeds the `✕ killed` outcome line in Task 4. + - ➕ the events channel is **buffered (64)**, so the reader stays ahead of a consumer that drains once per Bubble Tea message; past it the reader blocks and throttles the child, which is the correct back-pressure. +- [x] implement `Resize`, `Write`, `Kill` — **no `proc.DetachTTY` on the pty command** (it would clobber the pty's `Setctty`); unix kill signals the pty-led process group; verify `cmd.Process` is populated on the ConPTY path, else kill via xpty's own API (record the Windows shape with ➕) + - `Kill` reuses the existing **`proc.KillGroup`** (negative pid on unix, `Process.Kill` on Windows) rather than a duplicate — `proc` is a stdlib-only bottom leaf, so `term` may import it. +- [x] state in the package doc: no config paths, no `logx` — failures ride `Exit.Err`; hence no `TestMain` seam +- [x] write tests (unix): `sh -c 'printf hi'` → data chunk then `Exit{Err: nil}` with elapsed > 0 (this is also the Linux `EIO`-is-not-an-error test); non-zero exit surfaces in `Exit.Err`; `Kill` terminates a `sleep` and the channel closes (no goroutine leak); `Resize` returns no error; all under `-race` + - ➕ four tests beyond the list, each pinning something the plan relies on elsewhere: `TestSessionKillReachesGrandchildren` (the `Setsid` premise — an `sh -c` background job must die too, or the pty never EOFs), `TestSessionEnvOverridesTerm` (os/exec's last-duplicate-wins is why appending TERM is enough), `TestSessionNotInstalledToolExits127` (replaces the deleted `notFoundExit` mapping), `TestSessionOutputRendersInEmulator` (the term↔model contract, so Task 2's fake session is honest). +- [x] write error-case tests: `Start` with a bogus shell → error, no goroutine leak (channel closes) +- [x] run the full gate: `go build ./...` + `go vet ./...` + `go test -race ./...` + `golangci-lint run` + `GOOS=windows go build ./...` - must pass before task 2 + - ⚠️ **local-only**: `go build` fails in a git *worktree* with `error obtaining VCS status: exit status 128` (every git command it shells out to works standalone). CI clones normally and is unaffected; locally the gate runs `go build -buildvcs=false ./...`. + +### Task 2: model plumbing — mode, msgs, cmds, emulator state (no dispatch change yet) + +**Files:** +- Create: `internal/model/overlay_term.go` +- Modify: `internal/model/model.go`, `internal/model/commands.go`, `internal/model/render.go` +- Modify: `internal/model/mouse_test.go`, `internal/model/zoom_test.go` +- Create: `internal/model/overlay_term_test.go` + +- [x] add `modeToolOverlay` to the `inputMode` enum; extend `overlayVisible()` with it — mouse gating rides along, **`View()`'s fg picker does not**: turn the `if m.mode == modeHotkeys` two-way pick into a `switch m.mode` with a placeholder `modeToolOverlay` body here, so Task 4 only fills the renderer in +- [x] add model state: `termSession` behind a narrow local interface (the `restarter` idiom — `var _ termSession = (*term.Session)(nil)`), `termEmu`, `termExit`, `termW/termH`, `termToolName` + - ➕ plus `termInput *termInput`, the relay's handle (Task 1's drift 1 made the goroutine unavoidable). `termExit != nil` is the single "has it finished" discriminator — no separate flag, no "running" enum member, mirroring `selfState`'s derived `selfUpdating()`. +- [x] add msgs `termStartedMsg`/`termChunkMsg`/`termExitMsg` and cmds `startTermCmd(shell, args, w, h)` (safeCmd-wrapped) + `waitForTermChunkCmd(session)` with the non-blocking drain that **stops at `Exit` and delivers accumulated data first**; handlers in `Update`: started → create emulator (Update-only rule) + wire the input path per Task 1's decision, chain the wait cmd; chunk → `termEmu.Write` + chain; exit → store `termExit`, stop chaining + - ➕ **the drain's exit rides `termChunkMsg.exit *termExitMsg`**: a Go channel cannot be un-read, so the exit the drain already consumed had nowhere else to go. The handler writes the data, then re-emits the exit as the *next* message — the plan's stated ordering, literally. + - ➕ `termExitMsg` carries **`startFailed`** so the outcome line can say `✕ failed to start` without reconstructing it from a nil session. + - ➕ **stale-session gating**: a chunk whose session is not `m.termSession` is dropped **without re-subscribing** (a dead session's channel must not keep a command chain alive), and a `termStartedMsg` arriving outside the mode is **killed and closed** rather than adopted — otherwise a start that lost its race with `esc` strands a live pty. +- [x] if Task 1 landed on the `InputPipe` copy goroutine: implement its explicit lifecycle — started exactly once per session when both ends exist; close order on cleanup is `Kill`/wait → close pty → close the emulator's input pipe → goroutine returns; it must never outlive `esc` + - implemented as `termInput.stop(emu)`, called by the single teardown `closeToolOverlay()` in exactly that order, and it **waits** on the goroutine's done channel. +- [x] add `modeToolOverlay` to the modal tables in `mouse_test.go` (mouse no-op set, ~line 201) and `zoom_test.go` (modal `z` guard, ~line 287) — these tables are what "rides along" means + - ⚠️ **plan ordering flaw**: the `zoom_test.go` row cannot be green until the mode-dispatch case exists, which the plan put in Task 3. Pulled the `case modeToolOverlay:` dispatch (and a `updateToolOverlay` that consumes every key) forward into Task 2 — a mode that owns all input is the truthful placeholder, and it is what the table asserts. Task 3 fills in translation, the kill chord and esc. +- [x] write tests: handlers drive a fake session (chunk channel + recorded `Write`/`Kill`/`Resize`); chunk msg reaches a real `vt` emulator and `Render()` shows the bytes (pure Go — no pty); a fake whose channel holds data+data+Exit yields both data chunks before `termExitMsg` and `Render()` shows all of it; exit msg stores status and stops the chain; if the pipe goroutine exists — a `-race` leak test for its close order +- [x] run the full gate (build + vet + `go test -race ./...` + lint) - must pass before task 3 + - ⚠️ **golangci-lint's cache lies across a package's file set**: adding `overlay_term.go` made it report 6 `SA5011` false positives in three *untouched* test files. `golangci-lint cache clean` → 0 issues, before and after. Not a code problem; worth knowing before anyone "fixes" a `t.Fatalf` helper that was never broken. + +### Task 3: input routing — `updateToolOverlay` handler and key translation + +**Files:** +- Modify: `internal/model/mode.go`, `internal/model/overlay_term.go` +- Modify: `internal/model/overlay_term_test.go` + +- [x] implement key translation `tea.KeyMsg` → tool input (runes → text; named keys/ctrl-chords → key events; mechanism per Task 1's decision), confirmed against the pinned vt version; esc translates and is **sent**, not consumed, while the process runs + - three paths, split by what actually depends on emulator state: **runes/space** → `SendText`; **the mode-dependent named keys** (arrows, Home/End, PgUp/PgDn, Insert/Delete, shift+tab, F1–F12) → `SendKey`, so DECCKM/DECNKM are honoured; **the control range** → the byte itself, because Bubble Tea's control key *types are* the control bytes (`KeyEnter` is 0x0d, `KeyEsc` 0x1b, `KeyBackspace` 0x7f) and none of them is mode-dependent — writing the byte is exactly what vt's encoder produces. + - ➕ **`termModifiedKeys`**: the pinned x/vt encodes no modified special key at all — its `SendKey` default emits nothing when `Mod != 0` — so `ctrl+left` and friends would be silently swallowed in every editor. They are written out as xterm's `CSI 1;`, which is safe to hand-encode precisely because, unlike the bare arrows, the modified forms do not depend on DECCKM. + - `Alt` is a Bubble Tea *flag*, not a key: it becomes an ESC prefix (or `ModAlt` on the `SendKey` path). +- [x] add `case modeToolOverlay: return m.updateToolOverlay(msg)` to the mode dispatch **unwrapped** — deliberately *not* through `flushPendingLaunch` like its siblings: a deferred exec fallback must not fire `tea.ExecProcess` on the very keystroke that closes the overlay (the wrapper disappears entirely in Task 5) + - landed in Task 2 (see the ⚠️ ordering note there); `TestToolOverlayDoesNotFlushPendingLaunch` is what pins the unwrapping. +- [x] `updateToolOverlay`: process alive → translate & send everything except `ctrl+\` (→ `Session.Kill`, stays in mode until `termExitMsg`); process exited → `esc` cleans up (session close, `termEmu = nil`, `modeNormal`), everything else no-op +- [x] **pre-`termStartedMsg` nil guard**: keys arriving before the session exists are dropped, `ctrl+\` is a no-op there (a nil deref would re-panic through `logx.Recover` and crash keepkit) +- [x] write tests: keys (incl. esc, ctrl+c) reach the fake's input while alive; `ctrl+\` triggers `Kill` and mode holds; esc before exit does NOT close; esc after `termExitMsg` cleans and returns to `modeNormal`; non-esc after exit is a no-op; a key in the pre-started state neither panics nor reaches anything + - ➕ the forwarding test is a **19-row table asserting the exact bytes** the tool receives, not just that something arrived: the encoding is the feature, and "a key reached the fake" would pass with every sequence wrong. +- [x] run the full gate (build + vet + `go test -race ./...` + lint) - must pass before task 4 + +### Task 4: rendering — geometry, overlay frame, cursor, exit line, status bar, resize + +**Files:** +- Modify: `internal/model/render.go`, `internal/model/overlay_term.go`, `internal/model/model.go` (`applyLayout`/`WindowSizeMsg`) +- Modify: `internal/model/overlay_term_test.go` (and/or `render_test.go`) + +- [x] implement `termGeometry(width, height)` beside the frame arithmetic it must agree with: outer block = 70%×70% centered; body `emuW = outerW - 4` (border + `Padding(0,1)`), `emuH = outerH - 2 - 1 (title) - 1 (reserved exit row)`; account for `View()`'s `Margin(1, 0)` and PlaceOverlay's silent bottom clip; min clamps ≈40×10 body; ok=false below them **and when `!m.ready` (checked first, the `toggleZoom` idiom)** + - ➕ the size and the verdict are **separate answers**: the size is always clamped to the floor and `ok` is returned beside it, because the two callers want different things — the keypress refuses on `!ok`, while a terminal shrunk *mid-session* keeps rendering at the minimum. Killing somebody's editor because they narrowed a window is worse than a cramped overlay. + - at the 80×24 baseline the body is **52×11** — the default terminal succeeds, which is the number that mattered. +- [x] render the overlay in the `View()` switch: `OverlayBorder` frame, title = tool name + dim `ctrl+\ kill` right-hint while running; body = `termEmu.Render()` (pre-start: `starting …`); the **exit row is reserved from the start** (blank while running) so the block height is constant and the overlay never re-centers at exit + - ⚠️ **the visual pass caught what the plan's arithmetic missed**: the reserved row holds the *height*, but nothing held the *width*, and lipgloss sizes a border to its widest line. At 80×24 the block measured 54 cells running and 53 exited — it jumped sideways at the exact moment the user starts reading the outcome. Fixed by padding every row to the body width (`termRow`, ANSI-safe in both directions). +- [x] render the child's cursor: reverse-video the cell at the emulator's cursor position while the process is alive, hidden after exit; honour the emulator's cursor-visibility state if the API exposes it + - the pinned x/vt exposes `CursorPosition()` but **no visibility accessor**, so the alive/exited rule is the whole of it. The splice goes through `ansi.Truncate`/`TruncateLeft` (cut by *visible column*, keeping SGR on both sides) — a rune-index cut would land inside an escape sequence. + - ➕ `ui.Styles.TermCursor` is a new style and the only one naming no theme colour: reverse video swaps whatever the *tool* painted, which is the only way one style can mark a cursor on a screen keepkit does not control. +- [x] fill the reserved row after exit via `updateOutcomeBlock`'s helpers (`formatElapsed`/`fitCells`/`footerSep`): `✓ exited · · esc close` / `✕ exit · · esc close` / `✕ killed · · esc close` / `✕ failed to start · · esc close` (`✓`/`✕` in `Ok`/`Danger`) + - a start failure spends the middle cell on its **reason** rather than on an elapsed it does not have. +- [x] add the `renderStatusBar` branch for `modeToolOverlay` (its siblings all have one — without it the bar advertises six dead global keys): running → ` running ctrl+\ kill`, exited → ` exited esc close`, within the no-truncate budget +- [x] handle `WindowSizeMsg` while open: recompute `termGeometry`, `termEmu.Resize` + `Session.Resize`; shrinking below the minimum keeps the overlay at the clamped floor (no mid-session kill) + - hung off **`applyLayout`**, the single relayout point, so the resize path cannot drift from it; a no-op in every other mode. +- [x] write tests: rendered View contains frame, title, hint, `starting…` body pre-start, cursor cell reverse-video while alive, all four outcome lines after their exits; **block height identical before and after `termExitMsg`**; dimmed background; a body line carrying SGR renders at the right visible width inside the frame and no escape leaks past the frame's right edge (assert on `stripANSI(View())` width and the dim margins); status-bar branch in both states; resize propagates to fake session and emulator; refusal on a tiny terminal and on `!m.ready` + - **every new assertion was mutation-checked** (11 mutations, all killed). Two survived on the first pass and both were real test defects, not code ones: the block-size helper was scanning the *composited* view, where the three panels' own rounded corners are what it found, so it reported a constant size no matter what the overlay did; and the width claim was already satisfied by the body rows, so it was rewritten to assert the case the clamp actually exists for — **a tool name longer than the body must not widen the block**. + - the cursor test needs `forceColor(t)`: reverse video is styling, and the default test profile strips it, so the assertion would have passed against a cursor that was never drawn. +- [x] run the full gate (build + vet + `go test -race ./...` + lint) - must pass before task 5 + +### Task 5: the switch — dispatch to overlay, delete the tab launcher wholesale + +**Files:** +- Modify: `internal/model/mode.go` (`updateRunInput`), `internal/model/model.go`, `internal/model/commands.go` +- Delete: `internal/launcher/` (whole package) +- Modify: `internal/model/mode_test.go`, `internal/model/status_test.go` +- Modify/Delete: `internal/model/launch_test.go` + +- [x] rewire `updateRunInput`'s enter: **refusal first** (`termGeometry` incl. `!m.ready`) → statusMsg (`terminal too small to run a tool`-class wording) with **no `lastRun` write** — a launch that never started is not remembered (today's refusal comment, `mode.go:346`, keeps its meaning); on success: `lastRun` write, `startTermCmd`, `modeToolOverlay`; empty-input-cancels unchanged +- [x] delete `internal/launcher`; in model: `startLaunchCmd`, `execToolCmd`, `launchDoneMsg`/`execDoneMsg`, `m.launchingFor`, `pendingLaunchName`/`Command` + `flushPendingLaunch` and all its modal-return call sites, `launchTimeout`, `launchFallbackStatus`, `notFoundExit`, the `launching…`/`tab open failed…` wordings + - the untrack handler's "drop a pending fallback for this tool" branch goes with them, and the `tea.KeyMsg` dispatch comment now states what the switch actually does rather than what the wrapper used to. +- [x] delete `setStickyStatus` (its only callers were the two launch statuses) and `TestInFlightStatusSurvivesStaleExpiry`; keep `setStatus`/TTL machinery untouched +- [x] keep `shellCommand` (now feeds the overlay dispatch; refresh its comment and the cross-reference on `updater.customPlan`) +- [x] update `mode_test.go`: `TestRunInputEnterStoresLastRun` (post-enter mode → `modeToolOverlay`, `lastRun` still written); audit the other `modeRunInput` tests (`TestRunInputOpensPrefilled`, `TestRunInputEscCancels`, `TestRunInputBlankInputCancels`, `TestRunDuringUpdate`, `TestRunInputKeyGuard`) — prompt-opening ones stay, only dispatch-shape assertions change + - ⚠️ `TestRunInputEnterStoresLastRun` broke for a second reason the plan did not predict: `newTestModel` sets `width`/`height` **without a `WindowSizeMsg`**, so `m.ready` is false and the new refusal fired. It now goes through a real resize — which is the honest fixture, since the dispatch measures the screen. +- [x] rewrite `launch_test.go` into overlay-dispatch tests: enter→prompt→overlay opens with prefill variants; empty list no-op; rename still clears `lastRun`; refusal writes no `lastRun`; **no test executes the returned cmd** (it would spawn a real pty) + - `TestRenameClearsLastRun` already existed in `mode_test.go` and was left there rather than duplicated. +- [x] run the full gate + `GOOS=windows go build ./...` - must pass before task 6 + - ➕ four comments in Go source pointed at now-deleted symbols and were re-anchored here rather than left for Task 8: `updater.go`'s `launcher.planFor` idiom → `configdir.baseFor`, two `launchTimeout` var-seam references → `updateTimeout`, and `acceptsUpdateDetect`'s `launchDoneMsg` mode-gate mirror → the overlay's own reason. + +### Task 6: surfaces — hotkeys overlay sweep + +**Files:** +- Modify: `internal/model/render.go` +- Modify: `internal/model/render_test.go` + +- [x] `[?]` overlay tools group: the row is `{"enter", "run in a tab"}` (`render.go:797`) → `run in overlay`; that is +2 visible cells against the hard ≤76-col framed budget — measure after the change, and if `TestRenderHotkeysSizeBudget` breaks, shorten to `run overlay` rather than dropping a row + - measured after the change: **64×20 in all five self states** against the 76×20 budget. The width worry was unfounded — the overlay is 12 columns inside its ceiling; it is the *height* that sits exactly at 20, which is what the plan's "a new binding needs a row freed" note is about. `run in overlay` stands, no shortening needed. +- [x] verify no status-bar or footer branch references the deleted launch statuses; `[1]` footer `enter run` and the `modeRunInput` bar branch stay as-is +- [x] sweep rendered-surface tests: hotkeys budget across all five self states, footer cells, status bar at the 80×24 baseline stay green with the new wording +- [x] write/adjust tests for the changed `[?]` row +- [x] run the full gate (build + vet + `go test -race ./...` + lint) - must pass before task 7 + +### Task 7: Verify acceptance criteria + +- [x] verify all requirements from Overview: prompt kept, overlay launch, full keyboard proxy while alive, `ctrl+\` kill, esc-after-exit close, reserved exit line, cursor rendered, CLI-and-TUI single path, tab path fully gone + +| Requirement | Pinned by | +|---|---| +| prompt kept, prefilled | `TestRunInputPrefill`, `TestRunInputEnterOpensOverlay` | +| launches into the overlay | `TestRunInputEnterOpensOverlay`, `TestRunInputEnterStoresLastRun` | +| every key reaches the tool | `TestToolOverlayForwardsEveryKeyWhileRunning` (19 rows, exact bytes), `TestToolOverlayEscDoesNotCloseWhileRunning`, `TestToolOverlayCtrlCDoesNotQuit` | +| `ctrl+\` kills | `TestToolOverlayKillChord` | +| esc closes after exit | `TestToolOverlayAfterExit`, `TestToolOverlayCloseDispatchesNothing` | +| exit row reserved | `TestToolOverlayExitRowBlankWhileRunning`, `TestToolOverlayBlockDoesNotMoveAtExit` | +| cursor rendered | `TestToolOverlayCursor` | +| one path for CLI and TUI | `TestToolOverlayEndToEndWithRealPty` (CLI), `TestSessionOutputRendersInEmulator` | +| tab path gone | `internal/launcher` deleted; no reference survives outside prose | + +- [x] verify edge cases: empty list, empty input cancel, launch during a running update, resize during session, tiny-terminal and `!m.ready` refusals, not-installed tool shows 127 in overlay, keys before `termStartedMsg` are safe + - `TestRunInputEmptyList`, `TestRunInputCancels`, `TestRunDuringUpdate`, `TestToolOverlayResize` + `TestToolOverlayResizeBelowMinimumClamps`, `TestRunInputRefusesOnTinyTerminal` + `TestTermGeometryNotReady`, `TestSessionNotInstalledToolExits127`, `TestToolOverlayKeysBeforeStart`. +- [x] run full test suite: `go test -race ./...` + - ➕ **one coverage gap was real and is now closed**: every overlay test drove a *fake* session, so nothing would have noticed if `waitForTermChunkCmd` and `term.Session` disagreed about their own channel. `TestToolOverlayEndToEndWithRealPty` runs start → adopt → drain → render → exit against a real pty (a `printf`, unix-only), pumping the command chain the way the tea runtime does. It deliberately does **not** execute the run-prompt's returned cmd, which stays untested by design. +- [x] run `go vet ./...`, `golangci-lint run`, `GOOS=windows go build ./...`, `GOOS=darwin go build ./...` + - all green, plus `GOOS=windows go vet ./...`. 473 tests pass in `internal/model` + `internal/term`. + +### Task 8: [Final] Update documentation + +**Files:** +- Create: `docs/design/tool-overlay.md` +- Modify: `CLAUDE.md`, `ARCHITECTURE.md`, `README.md`, `docs/design/updating.md`, `internal/updater/updater.go`, `internal/model/model.go` + +- [x] create `docs/design/tool-overlay.md` (fourth deep-design doc: the Update-only emulator rule and why, the input-path decision, esc semantics, the kill chord and the no-`DetachTTY` invariant, geometry incl. the reserved exit row, the pty EOF rule, what was deleted and why) and link it from CLAUDE.md's design-docs table +- [x] CLAUDE.md: replace the `internal/launcher` package row with `internal/term`; rewrite the **Run (`enter` in `focusTools`)** bullet to the overlay invariant summary; input-modes list (`modeToolOverlay`); `overlayVisible()` description; commands.go/mode.go file-table rows; drop `setStickyStatus` from the status-message lifecycle section; **"Three features" → four** in the design-docs preamble (line ~22) and the "never re-inline these three sections" sentence; fix the misquoted hotkeys row (`run in a tab`, not `run in tab`) while touching it +- [x] re-anchor the **`planFor` idiom** onto a surviving example (`baseFor`/`shellCommand`): CLAUDE.md lines ~36/42/109, ARCHITECTURE.md ~348, `docs/design/updating.md` (two sites), `internal/updater/updater.go:93`; rewrite the "`execToolCmd` is the one unwrapped cmd" sentence in CLAUDE.md's logx section and ARCHITECTURE.md ~581 (every cmd is safeCmd-wrapped again); drop `launcher` from ARCHITECTURE.md ~65's bottom-leaf list; replace the "`launchDoneMsg`'s mode gate" mirror in `docs/design/updating.md` and the comment at `internal/model/model.go:839` with the surviving reason +- [x] ARCHITECTURE.md: mermaid edge `model --> term` (drop `--> launcher`), package table row, mode count, rewrite the "Running a tool (`enter`)" section, "Three areas" → four (~20-22) +- [x] README.md: rewrite the Features bullet at ~64-65 (the adapter list is the whole sentence and it all goes), Usage `enter` line at ~129-131, and the `[1]` hint enumeration at ~153 +- [x] move this plan to `docs/plans/completed/` + +## Post-Completion + +**Manual verification** (real terminals, real tools — unit tests must not touch them): +- vim in the overlay: esc reaches vim (mode switch works), cursor visible and tracking, `:q` exits, ✓ line shows, esc closes, keepkit screen intact +- yazi/fzf session end-to-end; a plain `rg --version` run: output + ✓ line stays until esc (verify the final screen survived the drain) +- `ctrl+\` on a hung `sleep 1000` (✕ killed line); terminal resize mid-vim; launch while an update streams in `[3]` +- Windows smoke test via ConPTY when a Windows machine is available (CI only cross-compiles) +- truecolor/alt-screen fidelity spot-check (btop or similar) — known upstream limits (#935 graphemes) noted, not fixed here + +**External follow-ups**: +- demo GIFs (`demo/hero.gif`, `demo/update.gif`) show the old launch flow — decide on regeneration via the demo-gifs skill after merge +- track upstream x/vt issues #879/#935; a future tagged x/vt release replaces the pinned pseudo-version + +## Task 8 notes + +- ➕ **the docs-sync sweep found two real drifts beyond the plan's list**: the mermaid + graph was missing the `term --> proc` edge (`internal/term/session.go` imports it for + `KillGroup`), and README's *Stack* omitted `ultraviolet`, which the key translation + made a **direct** dependency. Both fixed. +- ➕ the `modeRunInput` enum comment in `mode.go` still said *"run the tool in a new + terminal tab"* — corrected while verifying the enum against the docs. +- ⚠️ **CLAUDE.md is 144 641 characters**, over the docs-sync skill's ~140 000 warning + threshold and under the 150 000 harness limit. This change *reduced* it by 1 896 + (146 537 → 144 641): the Run bullet became an invariant summary plus a link, which + more than paid for the new package row and the fourth design-doc row. The threshold + was already crossed before this work, so splitting another section out is left as a + separate decision rather than folded in here. diff --git a/docs/research/pty-stack.md b/docs/research/pty-stack.md new file mode 100644 index 0000000..02cc6a7 --- /dev/null +++ b/docs/research/pty-stack.md @@ -0,0 +1,166 @@ +# The pty stack behind the tool overlay (researched 2026-08) + +Why keepkit runs a tracked tool on an embedded pseudo-terminal the way it does, +which libraries were considered, and what each pinned version is buying. This +outlives the implementation plan on purpose: the pins below are load-bearing and +a future reader upgrading them needs the reasons, not just the numbers. + +## What the feature needs + +Running `vim`, `yazi` or `fzf` *inside* keepkit's own screen means three things +at once, and no single library gives all three: + +1. a **pty** — the tool must believe it owns a terminal, or it will not draw at + all (`isatty` fails, ncurses/crossterm refuse to start); +2. a **terminal emulator** — something has to interpret the escape sequences the + tool writes and hand keepkit a rectangle of styled cells it can paint into a + Bubble Tea `View()`; +3. **Windows support**, because keepkit ships a Windows binary and its release + workflow cross-compiles for it. + +## Chosen: `charmbracelet/x/xpty` + `charmbracelet/x/vt` + +| Module | Version | Why | +|---|---|---| +| `github.com/charmbracelet/x/xpty` | **v0.1.4** (tagged) | one `Pty` interface over unix ptys (via `creack/pty`) and Windows **ConPTY**; `WaitProcess` papers over the Go runtime's inability to `cmd.Wait()` a ConPTY child | +| `github.com/charmbracelet/x/vt` | **pseudo-version `v0.0.0-20260813141921-f091cedeaf78`** — untagged upstream | VT220 + truecolor emulator in pure Go: `NewEmulator(w, h)`, `Write` pty bytes in, `Render() string` out | + +`vt` has **no tagged release**, so the pseudo-version is deliberate rather than +sloppy. Re-pin it only together with a run of the full suite — see *Dependency +blast radius* below. + +### Rejected alternatives + +- **`creack/pty` directly** — no Windows. ConPTY support has been proposed + upstream for years and never merged, so choosing it would mean shipping a + feature that silently does not exist on one of keepkit's three platforms. +- **`taigrr/bubbleterm`** — the closest thing to a drop-in, but it requires + Bubble Tea **v2** and keepkit is on v1.3.10. Upgrading the whole TUI to v2 to + get one feature is the tail wagging the dog. Its emulator↔`Model` wiring was + read for ideas; nothing was vendored. + +`vt` works fine under Bubble Tea v1 because `Render()` returns a plain ANSI +string — it needs no v2 rendering core to hand keepkit its screen. + +## What the API actually looks like (verified, not assumed) + +The implementation plan sketched this API from research; every item below was +re-checked against the pinned modules before a line of `internal/term` was +written, because the plan carried a stop condition for material drift. + +Matches the sketch: + +- `vt.NewEmulator(w, h) *Emulator`, `Write([]byte)`, `Render() string`, + `Resize(w, h)` (no error), `CursorPosition() uv.Position`, `IsAltScreen()`. +- `xpty.NewPty(w, h)`, `Pty.Start(*exec.Cmd)`, `Resize`, `Read`/`Write`/`Close`. +- `xpty.WaitProcess(ctx, cmd)` — falls back to `cmd.Wait()` off Windows and + synthesises the `*exec.ExitError` that `os.Process.Wait` fails to produce on + ConPTY, so the exit code has the same shape on every platform. +- `ConPty.Start` populates `cmd.Process` (via `os.FindProcess`), so the kill + path needs no Windows-specific detour. + +### Drift #1 — there is no key encoder, and the modes it would need are unexported + +The plan preferred translating a `tea.KeyMsg` into bytes and calling +`Session.Write` directly, because that needs no second goroutine and makes +"only `Update` touches the emulator" literally true. **That function does not +exist** in the pinned stack: `ultraviolet` exports only output-side encoders +(`EncodeCursorStyle`, `EncodeMouseMode`, …), and `x/ansi` has none either. + +Writing our own was rejected on correctness, not effort. `vt`'s `SendKey` +encodes **against emulator state**: + +```go +ack := e.isModeSet(ansi.ModeCursorKeys) // DECCKM - arrows are \x1bOA, not \x1b[A +akk := e.isModeSet(ansi.ModeNumericKeypad) // DECNKM +``` + +`isModeSet` is unexported and no accessor surfaces it, so a hand-rolled encoder +could not know whether the child had switched to application cursor keys — the +mode `vim` and `less` set on entry. Arrows in vim are not a corner case for the +project's flagship feature. So keepkit uses `vt`'s own `SendKey`, which means +accepting the copy goroutine the plan listed as the fallback. + +### Drift #2 — `xpty` does **not** make the child a session leader + +The plan assumed "the pty's own `Setsid`+`Setctty`". It has neither: +`UnixPty.Start` wires `Stdin`/`Stdout`/`Stderr` to the slave and calls +`cmd.Start()`, and that is all. Without `Setsid`+`Setctty` the child inherits +keepkit's controlling terminal, job control never engages, and a +`SIGKILL`-to-the-process-group teardown would signal the wrong group. + +`internal/term` therefore sets `SysProcAttr{Setsid: true, Setctty: true}` +itself, on the unix build only. `Ctty` defaults to 0, which is the child's +stdin — the slave that `xpty` just attached, so the default is the correct fd. + +The invariant the plan derived from its wrong premise survives intact, and now +has a sharper reason: **never call `proc.DetachTTY` on the pty command.** It +would overwrite `SysProcAttr` wholesale, dropping `Setctty` and handing the +child a pty with no controlling terminal — the exact opposite of the point. + +## Upstream issues we are living with + +### charmbracelet/x#879 — `Emulator.Read`/`Close` data race + +Real, reproduced here under `-race`, and it dictates the teardown: + +```go +func (e *Emulator) Read(p []byte) (int, error) { + if e.closed { return 0, io.EOF } // unsynchronised read + return e.pr.Read(p) +} + +func (e *Emulator) Close() error { + e.closed = true // unsynchronised write + return e.pw.CloseWithError(io.EOF) +} +``` + +A copy goroutine parked in `Read` plus a `Close()` from `Update` is a textbook +race, and `go test -race` reports it immediately. keepkit's suite runs `-race`, +so this is not a theoretical concern. + +**The workaround: never call `Emulator.Close()`.** `InputPipe()` returns the +emulator's `*io.PipeWriter`; closing *that* unblocks the parked `Read` with +`io.EOF` through `io.Pipe`'s own synchronisation and never touches the +`e.closed` bool. Verified race-clean, and `TestSessionInputPipeCloseIsRaceFree` +in `internal/term` is what keeps the knowledge from being lost the next time +somebody reaches for the obvious `Close()`. + +If a future `vt` fixes #879, the type assertion can go and `Close()` can come +back — the assertion falls back to `Close()` already if `InputPipe()` ever stops +returning an `io.Closer`. + +### charmbracelet/x#935 — grapheme splitting + +Wide graphemes split across two `Write` calls can render wrong. Not worked +around; the pty reader delivers large chunks and the failure is cosmetic and +transient. Tracked, not fixed here. + +## Dependency blast radius + +Pulling `vt` drags in `charmbracelet/ultraviolet` (the Bubble Tea v2 rendering +core) and bumps several modules that lipgloss and glamour already sat on — i.e. +**the dependency bump alone can move keepkit's render tests**, which is why the +gate for the dependency step was the full suite rather than the new package. + +Measured on the bump (`go test ./...` before any feature code): **everything +stayed green.** + +| Module | Before | After | +|---|---|---| +| `x/ansi` | v0.11.6 | v0.11.7 | +| `mattn/go-runewidth` | v0.0.19 | v0.0.23 | +| `clipperhouse/displaywidth` | v0.9.0 | v0.11.0 | +| `clipperhouse/uax29/v2` | v2.5.0 | v2.7.0 | +| `lucasb-eyer/go-colorful` | v1.3.0 | v1.4.0 | +| `charmbracelet/colorprofile` | v0.4.1 | v0.4.2 | +| `golang.org/x/sys` | v0.38.0 | v0.47.0 | +| `golang.org/x/sync` | v0.17.0 | v0.19.0 | +| `charmbracelet/ultraviolet` | — | v0.0.0-20260303162955-0b88c25f3fff (new) | + +`go-runewidth` and `displaywidth` are the two to watch on any future re-pin: +keepkit measures glyph widths in half a dozen places (the gauge, the language +band, the list markers, `insetPanelTitle`'s border arithmetic) and pins several +of them with dedicated tests precisely because a width change is invisible until +a border tears. diff --git a/go.mod b/go.mod index 3afa3ee..f564b83 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,11 @@ require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/glamour v1.0.0 github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834 - github.com/charmbracelet/x/ansi v0.11.6 - github.com/mattn/go-runewidth v0.0.19 + github.com/charmbracelet/ultraviolet v0.0.0-20260303162955-0b88c25f3fff + github.com/charmbracelet/x/ansi v0.11.7 + github.com/charmbracelet/x/vt v0.0.0-20260813141921-f091cedeaf78 + github.com/charmbracelet/x/xpty v0.1.4 + github.com/mattn/go-runewidth v0.0.23 github.com/muesli/termenv v0.16.0 github.com/yuin/goldmark-emoji v1.0.6 golang.org/x/mod v0.37.0 @@ -20,17 +23,21 @@ require ( github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/aymerick/douceur v0.2.0 // indirect - github.com/charmbracelet/colorprofile v0.4.1 // indirect + github.com/charmbracelet/colorprofile v0.4.2 // indirect github.com/charmbracelet/x/cellbuf v0.0.15 // indirect + github.com/charmbracelet/x/conpty v0.2.0 // indirect + github.com/charmbracelet/x/exp/ordered v0.1.0 // indirect github.com/charmbracelet/x/exp/slice v0.0.0-20250327172914-2fdc97757edf // indirect github.com/charmbracelet/x/term v0.2.2 // indirect - github.com/clipperhouse/displaywidth v0.9.0 // indirect - github.com/clipperhouse/stringish v0.1.1 // indirect - github.com/clipperhouse/uax29/v2 v2.5.0 // indirect + github.com/charmbracelet/x/termios v0.1.1 // indirect + github.com/charmbracelet/x/windows v0.2.2 // indirect + github.com/clipperhouse/displaywidth v0.11.0 // indirect + github.com/clipperhouse/uax29/v2 v2.7.0 // indirect + github.com/creack/pty v1.1.24 // indirect github.com/dlclark/regexp2 v1.11.5 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/gorilla/css v1.0.1 // indirect - github.com/lucasb-eyer/go-colorful v1.3.0 // indirect + github.com/lucasb-eyer/go-colorful v1.4.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-localereader v0.0.1 // indirect github.com/microcosm-cc/bluemonday v1.0.27 // indirect @@ -41,7 +48,8 @@ require ( github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/yuin/goldmark v1.7.17 // indirect golang.org/x/net v0.38.0 // indirect - golang.org/x/sys v0.38.0 // indirect + golang.org/x/sync v0.19.0 // indirect + golang.org/x/sys v0.47.0 // indirect golang.org/x/term v0.36.0 // indirect golang.org/x/text v0.30.0 // indirect ) diff --git a/go.sum b/go.sum index d148b56..91b804c 100644 --- a/go.sum +++ b/go.sum @@ -16,28 +16,42 @@ github.com/charmbracelet/bubbles v1.0.0 h1:12J8/ak/uCZEMQ6KU7pcfwceyjLlWsDLAxB5f github.com/charmbracelet/bubbles v1.0.0/go.mod h1:9d/Zd5GdnauMI5ivUIVisuEm3ave1XwXtD1ckyV6r3E= github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= github.com/charmbracelet/bubbletea v1.3.10/go.mod h1:ORQfo0fk8U+po9VaNvnV95UPWA1BitP1E0N6xJPlHr4= -github.com/charmbracelet/colorprofile v0.4.1 h1:a1lO03qTrSIRaK8c3JRxJDZOvhvIeSco3ej+ngLk1kk= -github.com/charmbracelet/colorprofile v0.4.1/go.mod h1:U1d9Dljmdf9DLegaJ0nGZNJvoXAhayhmidOdcBwAvKk= +github.com/charmbracelet/colorprofile v0.4.2 h1:BdSNuMjRbotnxHSfxy+PCSa4xAmz7szw70ktAtWRYrY= +github.com/charmbracelet/colorprofile v0.4.2/go.mod h1:0rTi81QpwDElInthtrQ6Ni7cG0sDtwAd4C4le060fT8= github.com/charmbracelet/glamour v1.0.0 h1:AWMLOVFHTsysl4WV8T8QgkQ0s/ZNZo7CiE4WKhk8l08= github.com/charmbracelet/glamour v1.0.0/go.mod h1:DSdohgOBkMr2ZQNhw4LZxSGpx3SvpeujNoXrQyH2hxo= github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834 h1:ZR7e0ro+SZZiIZD7msJyA+NjkCNNavuiPBLgerbOziE= github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834/go.mod h1:aKC/t2arECF6rNOnaKaVU6y4t4ZeHQzqfxedE/VkVhA= -github.com/charmbracelet/x/ansi v0.11.6 h1:GhV21SiDz/45W9AnV2R61xZMRri5NlLnl6CVF7ihZW8= -github.com/charmbracelet/x/ansi v0.11.6/go.mod h1:2JNYLgQUsyqaiLovhU2Rv/pb8r6ydXKS3NIttu3VGZQ= +github.com/charmbracelet/ultraviolet v0.0.0-20260303162955-0b88c25f3fff h1:uY7A6hTokHPJBHfq7rj9Y/wm+IAjOghZTxKfVW6QLvw= +github.com/charmbracelet/ultraviolet v0.0.0-20260303162955-0b88c25f3fff/go.mod h1:E6/0abq9uG2SnM8IbLB9Y5SW09uIgfaFETk8aRzgXUQ= +github.com/charmbracelet/x/ansi v0.11.7 h1:kzv1kJvjg2S3r9KHo8hDdHFQLEqn4RBCb39dAYC84jI= +github.com/charmbracelet/x/ansi v0.11.7/go.mod h1:9qGpnAVYz+8ACONkZBUWPtL7lulP9No6p1epAihUZwQ= github.com/charmbracelet/x/cellbuf v0.0.15 h1:ur3pZy0o6z/R7EylET877CBxaiE1Sp1GMxoFPAIztPI= github.com/charmbracelet/x/cellbuf v0.0.15/go.mod h1:J1YVbR7MUuEGIFPCaaZ96KDl5NoS0DAWkskup+mOY+Q= +github.com/charmbracelet/x/conpty v0.2.0 h1:eKtA2hm34qNfgJCDp/M6Dc0gLy7e07YEK4qAdNGOvVY= +github.com/charmbracelet/x/conpty v0.2.0/go.mod h1:fexgUnVrZgw8scD49f6VSi0Ggj9GWYIrpedRthAwW/8= github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91 h1:payRxjMjKgx2PaCWLZ4p3ro9y97+TVLZNaRZgJwSVDQ= github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= +github.com/charmbracelet/x/exp/ordered v0.1.0 h1:55/qLwjIh0gL0Vni+QAWk7T/qRVP6sBf+2agPBgnOFE= +github.com/charmbracelet/x/exp/ordered v0.1.0/go.mod h1:5UHwmG+is5THxMyCJHNPCn2/ecI07aKNrW+LcResjJ8= github.com/charmbracelet/x/exp/slice v0.0.0-20250327172914-2fdc97757edf h1:rLG0Yb6MQSDKdB52aGX55JT1oi0P0Kuaj7wi1bLUpnI= github.com/charmbracelet/x/exp/slice v0.0.0-20250327172914-2fdc97757edf/go.mod h1:B3UgsnsBZS/eX42BlaNiJkD1pPOUa+oF1IYC6Yd2CEU= github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI= -github.com/clipperhouse/displaywidth v0.9.0 h1:Qb4KOhYwRiN3viMv1v/3cTBlz3AcAZX3+y9OLhMtAtA= -github.com/clipperhouse/displaywidth v0.9.0/go.mod h1:aCAAqTlh4GIVkhQnJpbL0T/WfcrJXHcj8C0yjYcjOZA= -github.com/clipperhouse/stringish v0.1.1 h1:+NSqMOr3GR6k1FdRhhnXrLfztGzuG+VuFDfatpWHKCs= -github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEXNWYXQgCt4hdOzA= -github.com/clipperhouse/uax29/v2 v2.5.0 h1:x7T0T4eTHDONxFJsL94uKNKPHrclyFI0lm7+w94cO8U= -github.com/clipperhouse/uax29/v2 v2.5.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g= +github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY= +github.com/charmbracelet/x/termios v0.1.1/go.mod h1:rB7fnv1TgOPOyyKRJ9o+AsTU/vK5WHJ2ivHeut/Pcwo= +github.com/charmbracelet/x/vt v0.0.0-20260813141921-f091cedeaf78 h1:CIlgdpAf3PLGkv6Z+tISRNe6cnFoHVnjXtBWzo/NLAU= +github.com/charmbracelet/x/vt v0.0.0-20260813141921-f091cedeaf78/go.mod h1:u1LOIABor9JqY54oZdktK3TCRrgzP6tzHrDYx1nd3wY= +github.com/charmbracelet/x/windows v0.2.2 h1:IofanmuvaxnKHuV04sC0eBy/smG6kIKrWG2/jYn2GuM= +github.com/charmbracelet/x/windows v0.2.2/go.mod h1:/8XtdKZzedat74NQFn0NGlGL4soHB0YQZrETF96h75k= +github.com/charmbracelet/x/xpty v0.1.4 h1:4jaW7u+8AHQMxesiVc+zUMsspu7GyDwtJO+gy/tFtW4= +github.com/charmbracelet/x/xpty v0.1.4/go.mod h1:7t8P7BpPiolHJ1pLzz7/4ujDbD+sUxI9yA3CBOLOIcU= +github.com/clipperhouse/displaywidth v0.11.0 h1:lBc6kY44VFw+TDx4I8opi/EtL9m20WSEFgwIwO+UVM8= +github.com/clipperhouse/displaywidth v0.11.0/go.mod h1:bkrFNkf81G8HyVqmKGxsPufD3JhNl3dSqnGhOoSD/o0= +github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk= +github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= +github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= +github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/dlclark/regexp2 v1.11.5 h1:Q/sSnsKerHeCkc/jSTNq1oCm7KiVgUMZRDUoRu0JQZQ= github.com/dlclark/regexp2 v1.11.5/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= @@ -46,15 +60,15 @@ github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8= github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0= github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= -github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag= -github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= +github.com/lucasb-eyer/go-colorful v1.4.0 h1:UtrWVfLdarDgc44HcS7pYloGHJUjHV/4FwW4TvVgFr4= +github.com/lucasb-eyer/go-colorful v1.4.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4= github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= -github.com/mattn/go-runewidth v0.0.19 h1:v++JhqYnZuu5jSKrk9RbgF5v4CGUjqRfBm05byFGLdw= -github.com/mattn/go-runewidth v0.0.19/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= +github.com/mattn/go-runewidth v0.0.23 h1:7ykA0T0jkPpzSvMS5i9uoNn2Xy3R383f9HDx3RybWcw= +github.com/mattn/go-runewidth v0.0.23/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk= github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA= github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= @@ -81,10 +95,12 @@ golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8= golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8= +golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= +golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= -golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.36.0 h1:zMPR+aF8gfksFprF/Nc/rd1wRS1EI6nDBGyWAvDzx2Q= golang.org/x/term v0.36.0/go.mod h1:Qu394IJq6V6dCBRgwqshf3mPF85AqzYEzofzRdZkWss= golang.org/x/text v0.30.0 h1:yznKA/E9zq54KzlzBEAWn1NXSQ8DIp/NYMy88xJjl4k= diff --git a/internal/launcher/launcher.go b/internal/launcher/launcher.go deleted file mode 100644 index 08d6130..0000000 --- a/internal/launcher/launcher.go +++ /dev/null @@ -1,103 +0,0 @@ -// Package launcher decides how to run a tracked tool in a new terminal tab. -// It sits at the bottom of the import graph like internal/updater: no TUI -// knowledge, a pure core (planFor over an injected env lookup) plus a thin -// os.Getenv-facing wrapper (Detect). -package launcher - -import ( - "fmt" - "os" - "strings" -) - -// Plan describes how to open the user's command in a new terminal tab. -// When no supported terminal is detected, Fallback is true and Argv is empty — -// the caller runs the command in the current window via tea.ExecProcess. -type Plan struct { - Argv []string // adapter command, executed directly (not through a shell) - Fallback bool // no scripting API available; run in the current window - Terminal string // human-readable adapter name ("tmux", "iTerm2", …) -} - -// planFor is the pure detection core. The priority chain is deliberate: -// $TMUX first, because inside tmux TERM_PROGRAM names the *outer* terminal and -// a tmux window is the correct "tab" there; then TERM_PROGRAM/KITTY_WINDOW_ID -// checks; anything else falls back. -// -// The user command always executes as `sh -c ` (tmux runs the string via -// the user's shell itself). For tmux/kitty/wezterm the command and tool name -// travel as argv elements — no escaping. For the two AppleScript paths the -// command is interpolated into the script source, with appleScriptQuote as the -// single escaping point. -func planFor(env func(string) string, command, toolName string) Plan { - switch { - case env("TMUX") != "": - // "--" terminates option parsing: a user command edited to start with - // "-" must reach tmux as the shell command, not be eaten as a flag. - return Plan{ - Terminal: "tmux", - Argv: []string{"tmux", "new-window", "-n", toolName, "--", command}, - } - case env("TERM_PROGRAM") == "iTerm.app": - script := fmt.Sprintf(`tell application "iTerm2" - tell current window - set newTab to (create tab with default profile) - tell current session of newTab - set name to "%s" - write text "%s" - end tell - end tell -end tell`, appleScriptQuote(toolName), appleScriptQuote(command)) - return Plan{ - Terminal: "iTerm2", - Argv: []string{"osascript", "-e", script}, - } - case env("TERM_PROGRAM") == "Apple_Terminal": - // Terminal.app opens a *window*, not a tab: tabs are not scriptable - // without System Events. Honest degradation, documented in the plan. - script := fmt.Sprintf(`tell application "Terminal" to do script "%s"`, appleScriptQuote(command)) - return Plan{ - Terminal: "Terminal.app", - Argv: []string{"osascript", "-e", script}, - } - case env("KITTY_WINDOW_ID") != "": - // kitten @ needs a remote-control socket (`listen_on` in kitty.conf → - // exported KITTY_LISTEN_ON, which the subprocess inherits): the adapter - // runs detached with no controlling terminal, so the tty transport of - // plain `allow_remote_control yes` cannot work. Without the socket the - // run fails and the caller's auto-fallback launches in the current - // window instead. - return Plan{ - Terminal: "kitty", - Argv: []string{"kitten", "@", "launch", "--type=tab", "--tab-title", toolName, "sh", "-c", command}, - } - case env("TERM_PROGRAM") == "WezTerm": - // Tab title left to wezterm defaults; naming needs a second pane-id - // round-trip — deliberately skipped. - return Plan{ - Terminal: "WezTerm", - Argv: []string{"wezterm", "cli", "spawn", "--", "sh", "-c", command}, - } - default: - return Plan{Fallback: true} - } -} - -// appleScriptQuote escapes a string for interpolation inside a double-quoted -// AppleScript string literal: backslashes first, then double quotes, then the -// control characters AppleScript literals cannot carry raw (a pasted newline -// would otherwise split the literal and make osascript fail on user data — -// AppleScript understands \n/\r/\t escapes). -func appleScriptQuote(s string) string { - s = strings.ReplaceAll(s, `\`, `\\`) - s = strings.ReplaceAll(s, `"`, `\"`) - s = strings.ReplaceAll(s, "\n", `\n`) - s = strings.ReplaceAll(s, "\r", `\r`) - return strings.ReplaceAll(s, "\t", `\t`) -} - -// Detect resolves the launch Plan for the current environment. Env-only — no -// subprocesses — so it is safe to call inside Bubble Tea's Update. -func Detect(command, toolName string) Plan { - return planFor(os.Getenv, command, toolName) -} diff --git a/internal/launcher/launcher_test.go b/internal/launcher/launcher_test.go deleted file mode 100644 index 58182a8..0000000 --- a/internal/launcher/launcher_test.go +++ /dev/null @@ -1,191 +0,0 @@ -package launcher - -import ( - "reflect" - "strings" - "testing" -) - -// envFrom builds an env lookup over a fixed map; missing keys return "". -func envFrom(m map[string]string) func(string) string { - return func(k string) string { return m[k] } -} - -func TestPlanFor(t *testing.T) { - tests := []struct { - name string - env map[string]string - command string - toolName string - wantTerminal string - wantFallback bool - wantArgv []string - }{ - { - name: "tmux", - env: map[string]string{"TMUX": "/tmp/tmux-501/default,1234,0"}, - command: "yazi", - toolName: "yazi", - wantTerminal: "tmux", - wantArgv: []string{"tmux", "new-window", "-n", "yazi", "--", "yazi"}, - }, - { - name: "tmux wins over TERM_PROGRAM", - env: map[string]string{ - "TMUX": "/tmp/tmux-501/default,1234,0", - "TERM_PROGRAM": "iTerm.app", - }, - command: "fzf", - toolName: "fzf", - wantTerminal: "tmux", - wantArgv: []string{"tmux", "new-window", "-n", "fzf", "--", "fzf"}, - }, - { - name: "kitty", - env: map[string]string{"KITTY_WINDOW_ID": "3"}, - command: "dive nginx:latest", - toolName: "dive", - wantTerminal: "kitty", - wantArgv: []string{"kitten", "@", "launch", "--type=tab", "--tab-title", "dive", "sh", "-c", "dive nginx:latest"}, - }, - { - name: "wezterm", - env: map[string]string{"TERM_PROGRAM": "WezTerm"}, - command: "btop", - toolName: "btop", - wantTerminal: "WezTerm", - wantArgv: []string{"wezterm", "cli", "spawn", "--", "sh", "-c", "btop"}, - }, - { - name: "empty env falls back", - env: map[string]string{}, - command: "yazi", - toolName: "yazi", - wantFallback: true, - }, - { - name: "unknown TERM_PROGRAM falls back", - env: map[string]string{"TERM_PROGRAM": "ghostty"}, - command: "yazi", - toolName: "yazi", - wantFallback: true, - }, - { - name: "tool name with spaces stays one argv element (tmux)", - env: map[string]string{"TMUX": "x"}, - command: "docker run -it alpine", - toolName: "my tool", - wantTerminal: "tmux", - wantArgv: []string{"tmux", "new-window", "-n", "my tool", "--", "docker run -it alpine"}, - }, - { - name: "dash-leading command survives tmux option parsing", - env: map[string]string{"TMUX": "x"}, - command: "-la", - toolName: "ls", - wantTerminal: "tmux", - wantArgv: []string{"tmux", "new-window", "-n", "ls", "--", "-la"}, - }, - { - name: "unicode tool name stays intact (kitty)", - env: map[string]string{"KITTY_WINDOW_ID": "1"}, - command: "ls", - toolName: "инструмент", - wantTerminal: "kitty", - wantArgv: []string{"kitten", "@", "launch", "--type=tab", "--tab-title", "инструмент", "sh", "-c", "ls"}, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got := planFor(envFrom(tc.env), tc.command, tc.toolName) - if got.Fallback != tc.wantFallback { - t.Fatalf("Fallback = %v, want %v", got.Fallback, tc.wantFallback) - } - if got.Terminal != tc.wantTerminal { - t.Errorf("Terminal = %q, want %q", got.Terminal, tc.wantTerminal) - } - if tc.wantFallback { - if len(got.Argv) != 0 { - t.Errorf("fallback plan carries Argv %v, want empty", got.Argv) - } - return - } - if !reflect.DeepEqual(got.Argv, tc.wantArgv) { - t.Errorf("Argv = %#v, want %#v", got.Argv, tc.wantArgv) - } - }) - } -} - -func TestPlanForITerm(t *testing.T) { - got := planFor(envFrom(map[string]string{"TERM_PROGRAM": "iTerm.app"}), `echo "hi"`, "echo tool") - if got.Terminal != "iTerm2" || got.Fallback { - t.Fatalf("plan = %+v, want iTerm2 non-fallback", got) - } - if len(got.Argv) != 3 || got.Argv[0] != "osascript" || got.Argv[1] != "-e" { - t.Fatalf("Argv = %#v, want [osascript -e