diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e462df1..ea9e67b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -45,7 +45,7 @@ graph TD | `internal/proc` | `DetachTTY` — run probes without a controlling terminal; `KillGroup` — process-group SIGKILL (plain `Kill` on Windows) | | `internal/ui` | Lip Gloss styles, `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; GitHub API with a 24-hour cache; semver comparison (`IsNewer`) | +| `internal/version` | Detect the installed version locally — `InstalledVersion(t) (ver, present)`; GitHub API with a 24-hour cache; semver comparison (`IsNewer`) | `configdir`, `launcher`, `logx`, `proc`, `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 @@ -74,7 +74,10 @@ The `model` package is split across files within a single package: Each tool has five data sources, split so local detection never waits on the network: -- **installed** — `fetchInstalledCmd`: a local subprocess (`--version`/`-V`), always fired; +- **installed** — `fetchInstalledCmd`: a local subprocess (`--version`/`-V`, with brew-directory + and `cargo install --list` fallbacks that need no subprocess of the tool itself), always + fired. It reports a version *and* whether the tool is present at all, so the card can tell + "installed, version unknown" from "not installed"; - **remote** — `fetchRemoteCmd`: a single network pass via `version.GetRepoData` (release + repo card + languages), only when `github` is set; - **changelog** — `fetchChangelogCmd`; @@ -176,6 +179,11 @@ Go binary is not misrouted to `go install`). `update_cmd` from `meta.yaml` alway wins and runs via `sh -c`. Detection spawns subprocesses, so it runs as a `tea.Cmd`, never inside `Update()`. +When the tool has no binary of its own on PATH, one fallback runs before +`ErrUnknownManager`: a `Cellar/`/`Caskroom/` directory means brew owns +the name, so the plan is `brew upgrade `. That is the `rust` case — the +formula ships `rustc` and `cargo`, so `LookPath("rust")` can only miss. + Output streaming uses the "channel + re-subscribe" idiom, with no `*tea.Program`: a goroutine reads the merged stdout+stderr to EOF (`streamLines`, splitting on `\n` **and** `\r` — brew/npm progress bars collapse into one updating line), then @@ -280,7 +288,8 @@ rewrites the tracker wholesale. A `TestConfigDirIsolated` test in each of those packages fails if the isolation is ever dropped. Per-test setup still uses the internal seams: `testConfigDir`, `testCacheDir`, -`testTokenDir`, `testAPIBase`, `testBrewPrefix`, `updater`'s `testHomeDir`, and +`testTokenDir`, `testAPIBase`, `testBrewPrefix` (one copy in `version`, a second in +`updater` — each private to its package), `updater`'s `testHomeDir`, and `model`'s `testReadmeStyle` (forces the glamour construction failure so the plain-text fallback is covered). The races are real (mutexes in `version`, `logx`), so tests always run with `-race`: diff --git a/CLAUDE.md b/CLAUDE.md index a73cc67..4430701 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,8 +34,8 @@ CI (`.github/workflows/ci.yml`) runs build / vet / `test -race` / golangci-lint | `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/ui` | Lip Gloss styles and `PlaceOverlay` helper | -| `internal/updater` | Detect the package manager that owns an installed binary and produce an update `Plan{Manager, Argv, Display}` (brew → go → cargo → pipx → npm chain; `update_cmd` override always wins). Bottom of the import graph like `version`: no TUI knowledge, depends only on `loader` for `Tool`. Pure `detectFromPath` core + OS-facing `Detect` wrapper; `testHomeDir` seam | -| `internal/version` | Detect installed version locally (`--version`/`-V`, then a brew-directory fallback: `brewDirVersion` in `brew.go` reads the version from the `Caskroom/`/`Cellar/` directory names — no brew subprocess — so casks with no version CLI still resolve; a fallback hit suppresses the anomaly log; `testBrewPrefix` seam); fetch latest release, repo card, changelog and README from the GitHub API with a 24h cache; semver comparison (`IsNewer`) | +| `internal/updater` | Detect the package manager that owns an installed binary and produce an update `Plan{Manager, Argv, Display}` (brew → go → cargo → pipx → npm chain; `update_cmd` override always wins; on a `LookPath` miss, a brew-by-name fallback before giving up — see the `[u]` section). Bottom of the import graph like `version`: no TUI knowledge, depends only on `loader` for `Tool`. Pure `detectFromPath`/`brewNamePlanAt` cores + OS-facing `Detect`/`brewNamePlan` wrappers; `testHomeDir` and `testBrewPrefix` seams (the latter a deliberate duplicate of `version`'s — two bottom leaves that may not import each other) | +| `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`) | The `model` package is split by responsibility (one package, several files): @@ -57,12 +57,12 @@ The `model` package is split by responsibility (one package, several files): **Async fetch responsibility split**: two paths must stay symmetric. Per tool there are five sources, split so local detection never waits on the network: -- `fetchInstalledCmd(t)` — always fired; runs `version.InstalledVersion` locally (subprocess) and emits `installedMsg{toolName, installed}`. The handler merges `Installed` into `m.versions[toolName]` without clobbering `Latest`, so the installed version renders immediately regardless of network state. +- `fetchInstalledCmd(t)` — always fired; runs `version.InstalledVersion` locally (subprocess) and emits `installedMsg{toolName, installed, present}`. The handler merges `Installed` and `InstalledPresent` into `m.versions[toolName]` without clobbering `Latest`, so the installed version renders immediately regardless of network state. `present` is the second result of `InstalledVersion` — a version-less install is still an install, and the card's `installed:` line renders the two differently. - `fetchRemoteCmd(t)` — fired only when `t.GitHub != ""`; makes a single network pass via `version.GetRepoData` (release + repo info + languages in one shot, no duplicate `fetchRepoInfo`) and emits `remoteMsg{toolName, latest, repoStatus, card, rate, err}` (`rate` is the post-request `version.Rate()` snapshot for the gauge). The handler merges `Latest` into `m.versions[toolName]` and writes `m.repoStatus` / `m.repoCards`. - changelog (`fetchChangelogCmd`), `--help` (`fetchHelpCmd`) and the README (`fetchReadmeCmd`) round out the five. `fetchHelpCmd` has one source per mode with no cross-fallback: `[h]` runs the tool's own `--help`/`-h`/`help`, `[m]` runs `man `. When a source is empty the `helpOutputMsg` handler caches an explicit, tool-named message (`No man page for . Press [h] for --help.` / `No --help output for . Press [m]…`) so a missing man page or `--help` is surfaced instead of silently showing the other mode's output. - `fetchReadmeCmd(githubField, name)` — the only *remote* source of panel `[3]`; requires `t.GitHub != ""`, calls `version.GetReadme` and emits `readmeMsg{toolName, content, err}`. Unlike the other four it is **lazy**: `Init()` seeds only the initially selected tool and `autoFetchCmdsForSelected` fires it on a selection move **only while `helpMode == helpModeReadme`** (plus the `[r]` key, which covers a selection moved under `[h]`/`[m]`), so the extra request is spent per *visited* tool, not per tracked tool. The whole msg is stored in `m.readmeData map[string]readmeMsg`, so an error (404, rate limit) is a session-cached negative — `needsReadme(t)` treats any present entry as answered and never refetches. The entry only lands when the response arrives, so the in-flight window has its own marker: every site that fires the command calls `markReadmeLoading(name)` (the set `m.readmeLoading`, cleared by the `readmeMsg` handler) and `needsReadme` returns false while a name is in it — otherwise a `j`/`k` bounce back onto the same tool would spend a second request. `[r]` in `focusBrief` (`refreshSelectedCmd`) deletes the entry so a negative can recover, and the `tokenValidatedMsg` handler drops the **rate-limited** entries (content-less, `ErrRateLimited`) before its backfill — otherwise the `rate limited — press [L]` placeholder would be a dead end, since doing exactly what it says would not retry. A 404 is conclusive and survives the token add. The handler is a known-content-wins merge: a later error keeps the previously fetched README. `readmeCmd` logs only *unexpected* failures: `ErrNoReadme` (a repo simply has no README) and `ErrRateLimited` (already logged by `doGH`) are skipped, or every launch would re-create a session log and destroy the "a log file means something went wrong" signal. -**Tool probes are sandboxed on two layers** (a tracked tool that ignores its args and boots its own TUI — e.g. a ratatui app answering `--help` with the app itself — must not shred keys' screen): (1) every probe subprocess (`fetchHelpCmd`, `version.InstalledVersion`, `man`) runs through `proc.DetachTTY`, i.e. in its own session with no controlling terminal, so the child's attempt to open `/dev/tty` fails (ENXIO) instead of toggling raw mode/alt-screen on the live terminal; (2) captured output is sanitized before it can reach a viewport — `ui.StripANSI` delegates to `x/ansi.Strip` (full escape grammar: private-mode CSI like `ESC[?1049l`, OSC, DCS — not just SGR) and `cleanTerminalOutput` additionally drops stray control characters (keeps `\n`/`\t`), because anything left in viewport content is re-emitted verbatim by the renderer and flips the real terminal's state. On top of that, `fetchHelpCmd` discards a capture whose RAW bytes carry the alt-screen signature (`isTUITakeover`, `ESC[?1049`): that's a TUI boot plus crash trace, not help text, so the panel falls through to the friendly `No --help output for …` message. +**Tool probes are sandboxed on two layers** (a tracked tool that ignores its args and boots its own TUI — e.g. a ratatui app answering `--help` with the app itself — must not shred keys' screen): (1) every probe subprocess (`fetchHelpCmd`, `version.InstalledVersion`, `man`) runs through `proc.DetachTTY`, i.e. in its own session with no controlling terminal, so the child's attempt to open `/dev/tty` fails (ENXIO) instead of toggling raw mode/alt-screen on the live terminal; (2) captured output is sanitized before it can reach a viewport — `ui.StripANSI` delegates to `x/ansi.Strip` (full escape grammar: private-mode CSI like `ESC[?1049l`, OSC, DCS — not just SGR) and `cleanTerminalOutput` additionally drops stray control characters (keeps `\n`/`\t`), because anything left in viewport content is re-emitted verbatim by the renderer and flips the real terminal's state. On top of that, `fetchHelpCmd` discards a capture whose RAW bytes carry the alt-screen signature (`isTUITakeover`, `ESC[?1049`): that's a TUI boot plus crash trace, not help text, so the panel falls through to the friendly `No --help output for …` message. `version.InstalledVersion` applies the same guard to its own `--version`/`-V` captures — with its own copy of `isTUITakeover`, since `version` sits below `model` in the import graph — and there it also stops the probe loop and drops the accumulated failure reasons: parsing that capture would mis-read a dependency's version out of the panic trace (`ratatui-0.30.2`), and logging it would re-create a session log on every startup for a tool that is merely a TUI app. `Init()` fires `fetchInstalledCmd` + (conditionally) `fetchRemoteCmd` for every tool, plus changelog/README for the selected one (the README is what panel `[3]` shows on the first screen, so without that seed the default mode would sit on a placeholder until the selection moved). The `--help` seed is gated on `helpMode == helpModeHelp` — the probe spawns a subprocess, and in the default readme mode its capture would never be rendered (same reasoning as the readme case in `autoFetchCmdsForSelected`). `autoFetchCmdsForSelected()` runs after track/untrack/rename and selection changes (every selection move — `j`/`k`, arrows, pgup/pgdown, search commit/rollback, mouse click — funnels through the shared `selectMeta(idx)` helper in `model.go`, which refreshes the tools list, brief card and help viewport and fires the auto-fetch); it re-fetches the same sources for the selected tool, guarded by the pure predicates `needsInstalled(t)` / `needsRemote(t)` (skip if already cached; `needsRemote` also requires `t.GitHub != ""` and a missing `Latest` or card). If a tool is added or renamed mid-session, this path populates its card without a restart. Rename also deletes the stale old-name entries from `m.repoCards` / `m.versions` / `m.repoStatus` / `m.changelogData` / `m.helpCache` / `m.readmeData` so the tool re-fetches under its new name. @@ -87,10 +87,10 @@ The model is a three-panel layout with focus cycling via `→/←` between `focu - **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 — distinct from `highlightMatch` in textutil.go, which belongs to the help search), 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: `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, `t` edits the tool's single tag. `o`/`c` go through `openURLCmd` (resolved per-`GOOS` by `browserCommand`); a tool with no `GitHub` sets `m.statusMsg` instead of launching. `s`/`e`/`t` 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. - **Clickable card lines**: `renderCard()` is a thin wrapper over **`buildCard() (string, map[int]string)`**, which returns the card text plus the index of its clickable lines (0-based content line → URL) — the `repo:` line (`https://` + `t.GitHub`, exactly what `[o]` opens) and the changelog block's leading release URL (`msg.htmlUrl` **verbatim** — note this is the release's own page, unlike `[c]`'s `/releases`), registered only when the block actually starts with it (a loading/failed/URL-less changelog emits no such line, and registering it unconditionally would map onto body text). Indices are recorded *while writing* (`strings.Count(sb.String(), "\n")` immediately before the line), because line heights vary — dividers are 3 lines and about/note/changelog bodies wrap. The wrapper keeps the ~30 `SetContent(renderCard())` call sites unchanged; `handleMouse` is the only consumer of the map and recomputes it per click, so it can never be stale. A **content line is a screen row** here: the viewport *truncates* a line wider than the panel (a release URL regularly is one) rather than soft-wrapping it, which is what lets a click row map straight onto a content-line index — `TestBriefContentLineIsScreenRow` pins that, because a bubbles that wrapped instead would shift every link below an overlong line. No visual styling marks the links (no underline/bold) — deliberate. -- **Card versions**: `renderCard`'s `[info]` section shows `installed:` from `m.versions[name].Installed` — all three states (`installed: \uf412 `, `installed: detecting…`, `installed: ✕ not found`) in the section's muted `InfoStyle`, with the `✕` (U+2715) in `DangerStyle` — the same red glyph/style pair the `[L]` overlay uses for exhaustion, reused as the "not installed" marker; the section's gate includes `installed != ""`, so a locally installed tool with no `GitHub` ref still renders it. When `hasUpdate(name)` (installed older than latest per `version.IsNewer`) the `latest:` value renders in `UpdateAvailableStyle` with a ` ↑` suffix — the same glyph the tools list appends to the row. The `latest:` line is gated on `card.Latest != ""` (a repo with no release shows no line) and appends the release date as a ` (YYYY-MM-DD)` suffix derived from `card.PublishedAt`. **Both version lines carry the `\uf412` glyph** (nf-oct-tag) before the version — its meaning is "a version", not specifically "a release tag", which is why it widened from `latest:` to `installed:`; the two states with no version to tag (`detecting…`, `✕ not found`) stay bare. Written as a `\u` escape in source and prose alike: the raw glyph is invisible in most editors and diffs and gets silently lost. Versions live in the card only; the 30-column list never shows version strings. +- **Card versions**: `renderCard`'s `[info]` section shows `installed:` from `m.versions[name].Installed` — all four states (`installed: \uf412 `, `installed: detecting…`, `installed: ✓ no version`, `installed: ✕ not installed`) in the section's muted `InfoStyle`. The two version-less states split what `InstalledKnown` alone used to collapse, on `InstalledPresent`: a tool that is installed but won't name its version (a ratatui app that ignores `--version`) is a working install and reads `✓` (U+2713) in `ui.OkStyle` — green, deliberately not `UpdateAvailableStyle`, whose orange means "act on this" — while a genuine absence keeps the `✕` (U+2715) in `DangerStyle`, the same red glyph/style pair the `[L]` overlay uses for exhaustion. The section's gate includes `installed != ""`, so a locally installed tool with no `GitHub` ref still renders it. When `hasUpdate(name)` (installed older than latest per `version.IsNewer`) the `latest:` value renders in `UpdateAvailableStyle` with a ` ↑` suffix — the same glyph the tools list appends to the row. The `latest:` line is gated on `card.Latest != ""` (a repo with no release shows no line) and appends the release date as a ` (YYYY-MM-DD)` suffix derived from `card.PublishedAt`. **Both version lines carry the `\uf412` glyph** (nf-oct-tag) before the version — its meaning is "a version", not specifically "a release tag", which is why it widened from `latest:` to `installed:`; the three states with no version to tag (`detecting…`, `✓ no version`, `✕ not installed`) stay bare. Written as a `\u` escape in source and prose alike: the raw glyph is invisible in most editors and diffs and gets silently lost. Versions live in the card only; the 30-column list never shows version strings. - **Panel titles**: all three panels inset a title into their top border (`┌─ [1] Tools ─…─┐`) via the shared `insetPanelTitle` (render.go) — an ANSI-safe splice over the already-rendered frame (`ui`'s `truncateVisible` is unexported; the helper remeasures with `stripANSI` and rebuilds the top line), colored like the border (focus-aware) and dropped whole when the panel is too narrow. The titles are `[1] Tools`, `[2] Brief` and `[3] Readme` / `[3] Help` / `[3] Man` (a `switch m.helpMode`, overridden by `[3] Update` while a live log shows) — they double as the documentation for the digit focus hotkeys, so the status bar carries no digit hints. All title characters are single-width and non-East-Asian-Ambiguous, keeping the border width math stable. - **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. `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 the same `case "r"` branches on focus three ways: rename in `focusTools`, refresh in `focusBrief`, README mode in `focusHelp`. -- **Update (`u` in `focusBrief`)**: installs a newer release from inside the TUI. `[u]` fires only in `focusBrief` (in `focusTools` `u` stays untrack — same `case "u"`, branched on focus); it requires `hasUpdate(name)` (else `statusMsg`) and is a no-op while `updatingFor != ""` (one update at a time, no queue). The key fires `detectUpdateCmd(t)` — 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 → npm chain (order matters: brew before go, so a brew-installed Go binary with buildinfo isn't misrouted); a `update_cmd` in `meta.yaml` always wins and runs via `sh -c` (skips detection entirely). The `updateDetectedMsg` handler drops a stale result (tool no longer selected / update already running), maps `ErrUnknownManager` to a `statusMsg` hint (`no known updater for — set update_cmd or [o] releases` — no dead-end dialog), and on success stores `m.updatePlan` and enters `modeConfirmUpdate`. The confirm status bar shows `update : [enter] run [esc] cancel`; `enter` sets `m.updatingFor`, resets the log, and fires `startUpdateCmd` (any other key cancels, not just `esc`). The `[u] update` hint is prepended to the `focusBrief` bar only when `hasUpdate(selected)`. +- **Update (`u` in `focusBrief`)**: installs a newer release from inside the TUI. `[u]` fires only in `focusBrief` (in `focusTools` `u` stays untrack — same `case "u"`, branched on focus); it requires `hasUpdate(name)` (else `statusMsg`) and is a no-op while `updatingFor != ""` (one update at a time, no queue). The key fires `detectUpdateCmd(t)` — 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 → npm chain (order matters: brew before go, so a brew-installed Go binary with buildinfo isn't misrouted); a `update_cmd` in `meta.yaml` always wins and runs via `sh -c` (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. The branch is self-validating (it fires only when such a keg actually exists) and shares `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 (tool no longer selected / update already running), maps `ErrUnknownManager` to a `statusMsg` hint (`no known updater for — set update_cmd or [o] releases` — no dead-end dialog), and on success stores `m.updatePlan` and enters `modeConfirmUpdate`. The confirm status bar shows `update : [enter] run [esc] cancel`; `enter` sets `m.updatingFor`, resets the log, and fires `startUpdateCmd` (any other key cancels, not just `esc`). The `[u] update` hint is prepended to the `focusBrief` bar 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}` (not `chan string`) so the `replace` flag and completion error ride the same channel — no second error channel threaded through every re-subscribe. 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). - **Live log in `[3]`**: `m.updateLog []string` is a single active-session buffer (not a map); `m.updateLogFor` names its tool. `renderHelpContent()` returns the log **ahead of** the `helpLoadingFor`/cache branches when `updateLogFor == selected`, and `autoFetchCmdsForSelected` skips the help fetch (and `helpLoadingFor` set) for that tool — otherwise re-selecting it paints `Loading...` or a late `helpOutputMsg` clobbers the live log. The panel title reads `[3] Update` while the log shows (same `insetPanelTitle` path as `[3] Help`/`[3] Man`), autoscrolls to bottom on each chunk, and the buffer persists after completion until the next update. 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]"` + `logx.Errorf` (manager, exit code, last log lines — never the token). A tool untracked mid-update just clears `updatingFor` (no re-fetch, no crash). @@ -126,7 +126,7 @@ The base dir is resolved by `configdir.Base()`: `~/.config/keeptui` on **macOS a | `loader.SetConfigDirForTesting` | `meta.yaml` | root, `loader`, `model` | | `version.SetConfigDirForTesting` | `cache.json`, `token` | `model`, `version` | -The internal `testConfigDir`/`testCacheDir`/`testTokenDir`/`testAPIBase`/`testBrewPrefix`/`testHomeDir` vars still exist for per-test setup; the exported seams exist because the package that can *reach* a file is usually not the one that owns it — a `model` test that drives the tags/track/rename/untrack/status handlers lands in `loader.SaveMeta`, which rewrites `meta.yaml` **wholesale**, and one that drives the `[L]` overlay lands in `version.SetToken`. Leaving that to each test to remember (with its own temp `HOME`) is not a safeguard: an ad-hoc probe test that forgot it destroyed a real user's tracker, which is why the isolation moved to `TestMain`. Each of those packages carries a `TestConfigDirIsolated` test whose only job is to fail loudly if the isolation is ever removed. Never write a test that reaches a config path without one of these seams active. +The internal `testConfigDir`/`testCacheDir`/`testTokenDir`/`testAPIBase`/`testBrewPrefix`/`testHomeDir` vars still exist for per-test setup (`testBrewPrefix` exists **twice** — once in `version`, once in `updater` — each private to its package, so an override must name the right one; `version`'s `TestMain` additionally pins its copy to an empty throwaway prefix package-wide, which is why a test there sees no brew layout unless it sets one up); the exported seams exist because the package that can *reach* a file is usually not the one that owns it — a `model` test that drives the tags/track/rename/untrack/status handlers lands in `loader.SaveMeta`, which rewrites `meta.yaml` **wholesale**, and one that drives the `[L]` overlay lands in `version.SetToken`. Leaving that to each test to remember (with its own temp `HOME`) is not a safeguard: an ad-hoc probe test that forgot it destroyed a real user's tracker, which is why the isolation moved to `TestMain`. Each of those packages carries a `TestConfigDirIsolated` test whose only job is to fail loudly if the isolation is ever removed. Never write a test that reaches a config path without one of these seams active. ### Session error log (`internal/logx`) @@ -134,7 +134,7 @@ An errors-only journal so bugs can be researched after the fact instead of recon **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. -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), `meta.yaml` save failures (`loader.SaveMeta`), help-capture failures (`fetchHelpCmd`), and a panic-ended session from `main`. `View`/render steady state and keystrokes are deliberately never logged. +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`), and a panic-ended session from `main`. `View`/render steady state and keystrokes are deliberately never logged. ### GitHub API diff --git a/README.md b/README.md index 99cdc99..038be30 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,12 @@ Statuses: `active` (●) · `trying` (○) · `inactive` (✕) — shown on the Legacy `forgotten` / `archived` values from `meta.yaml` are automatically read as `inactive`; a legacy list of several tags is read as its first tag. +The `installed:` line has four states: the detected version, `detecting…` while the +local probe runs, `✓ no version` for a tool that is installed but won't report one +(a TUI app that ignores `--version`), and `✕ not installed`. The version is read from +`--version` / `-V`, and failing that from Homebrew's directory layout or +`cargo install --list` — so tools with no version CLI still resolve. + The card's links are clickable: a click on the `repo:` line opens the repository in the browser, a click on the release URL under `[changelog]` opens that release page — the same thing `o` and `c` do from the keyboard. @@ -170,6 +176,11 @@ in the brief panel. `keeptui` detects the package manager the binary was install - `pipx` — a venv in `~/.local/pipx/venvs//` → `pipx upgrade `; - `npm` — a global `node_modules/` → `npm install -g `. +If the tool has no binary of its own on `PATH`, one more check runs before giving up: +a Homebrew keg or cask named after the tool → `brew upgrade `. That covers +formulae whose binaries are named differently — `rust` installs `rustc` and `cargo`, +so there is no `rust` binary to detect from. + The command is shown in the status bar for confirmation (`enter` runs it, any other key cancels); its output streams into panel `[3] Update` in real time and the TUI stays responsive. After a successful update the version is re-detected, the `↑` diff --git a/docs/plans/completed/20260724-rust-version-update-detect.md b/docs/plans/completed/20260724-rust-version-update-detect.md new file mode 100644 index 0000000..d71c89a --- /dev/null +++ b/docs/plans/completed/20260724-rust-version-update-detect.md @@ -0,0 +1,155 @@ +# Rust: installed-detect + updater fallbacks (Вариант A) + +## Overview + +Чинит две проблемы с Rust-инструментами в keeptui и заодно делает строку `installed:` честной: + +1. **`rust` не обновляется.** Трекается как `github.com/rust-lang/rust`, но бинарника `rust` нет — `rustc`/`cargo` стоят из Homebrew (`/opt/homebrew/Cellar/rust/1.96.1`). `updater.Detect` стартует с `exec.LookPath("rust")`, промахивается и сразу отдаёт `ErrUnknownManager` → «no known updater». Правильная команда очевидна: `brew upgrade rust`. +2. **Rust-TUI (пример `inertia`, крейт `inertia-tui`, `~/.cargo/bin/inertia`) показываются `installed: ✕ not found`,** хотя установлены и запускаются по `enter`. Это ratatui-приложение: игнорирует `--version`/`-V`, грузит свой TUI, а запущенное детачнуто (`proc.DetachTTY`) паникует с exit 101. Вывод содержит alt-screen сигнатуру `\x1b[?1049l` и строку `ratatui-0.30.2`. Итог: (а) версия не парсится → not found; (б) anomaly-лог пишется при **каждом** старте; (в) латентный мис-парс — `versionRe` матчит `0.30.2` из `ratatui-0.30.2`, сейчас маскируется ненулевым exit. Версия доступна без запуска бинарника: `cargo install --list` → `inertia-tui v0.1.0:`. +3. **Строка `installed:` схлопывает два разных состояния в «✕ not found»:** (а) установлена, но версию не определить; (б) не установлена вообще. Различаем. + +**Решение — Вариант A:** автоматические fallback'и, симметричные существующему `brewDirVersion` (никакой ручной настройки), плюс различение present/not-installed. `version` и `updater` остаются нижними листами графа, друг друга и `model` не импортируют — маленькие куски дублируем, а не абстрагируем (философия кодовой базы: per-package чистые ядра + свои тест-сиды, ср. `shellCommand`/`browserCommand`/`planFor`). + +## Context (from discovery) + +- **Проект:** Go TUI (Bubble Tea), трекер CLI-инструментов. Тесты — table-driven, `go test -race ./...`, golangci-lint v2. +- **Файлы/области:** + - `internal/version/detect.go` — `InstalledVersion`, `versionRe`; образцы `brewDirVersion`/`brewPrefix`/`testBrewPrefix` в `internal/version/brew.go`. + - `internal/updater/updater.go` — `Detect`, `detectFromPath`, `cargoCrateFromList`, `autoPlan`, `runProbe`, сид `testHomeDir`. + - `internal/model/model.go` — `VersionInfo`, `installedMsg`, `case installedMsg` (~427). + - `internal/model/commands.go:159` — `fetchInstalledCmd` (единственный продакшн-вызов `InstalledVersion`). + - `internal/model/render.go` (~1095) — switch рендера `installed:`. + - `internal/model/textutil.go:196` — `isTUITakeover` (образец для дубля). + - `internal/ui/styles.go` — стили, `ColorGreen` (#6AAF6A), `DangerStyle`. +- **Ripple от смены сигнатуры `InstalledVersion`:** продакшн — `commands.go:159`; тесты — `detect_test.go` (строки 87, 104, 123, 145, 162), `brew_test.go` (150, 154, 157). +- **Паттерны:** «узнать версию не запуская бинарник» уже есть ровно в одном месте — `brewDirVersion`. Добавляем симметрично: cargo-list в `version`, brew-by-name в `updater`. + +## Development Approach + +- **testing approach:** Regular (реализация, затем тесты в той же задаче) — под table-driven стиль пакетов. +- каждую задачу доводить до конца перед следующей; маленькие сфокусированные изменения. +- **CRITICAL: каждая задача включает новые/обновлённые тесты** для своего кода (успех + ошибки/крайние случаи). +- **CRITICAL: все тесты зелёные перед следующей задачей.** +- **CRITICAL: держать `go build ./...` зелёным на каждой задаче** — смена сигнатуры `InstalledVersion` компилируется только вместе со всеми потребителями, поэтому сигнатура и её plumbing в `model` объединены в одну задачу (Task 1). +- **CRITICAL: обновлять этот файл при изменении скоупа.** +- прогонять `go test -race ./...` после каждого изменения; сохранять обратную совместимость `meta.yaml`. + +## Testing Strategy + +- **unit tests:** обязательны в каждой задаче. + - чистые ядра (`cargoVersionFromList`, `brewNamePlanAt`) — table-driven. + - сиды: `testBrewPrefix` (`updater`, дубль), `testHomeDir` (уже есть). + - рендер — прямая проверка строки карточки по 4 состояниям. +- **e2e:** в проекте нет UI-e2e (только `demo/*.tape` для GIF-ов, вне области). Ручной smoke — в Post-Completion. + +## Progress Tracking + +- `[x]` сразу по завершении; ➕ для новых задач; ⚠️ для блокеров; синхронизировать план с фактом. + +## Solution Overview + +- **`version`:** `InstalledVersion` возвращает `(ver, present)`. Порядок источников версии: `--version`/`-V` → `brewDirVersion` → **новый** `cargoListVersion` (только когда бинарник есть, но версии нет). Гард `isTUITakeover` отсекает alt-screen-вывод до `versionRe` — убирает мис-парс и anomaly-лог для TUI-приложений. `present` = бинарник в PATH (`LookPath`) или версия найдена fallback'ом. +- **`updater`:** на промахе `LookPath` в `Detect`, до `ErrUnknownManager`, пробуем brew-by-name (`Cellar/`/`Caskroom/` существует → `brew upgrade `). Самовалидирующаяся ветка (срабатывает только при name==формула). +- **`model`/`ui`:** `installed:` — 4 состояния; `✓ no version` (`ui.OkStyle`, зелёный) для present-без-версии, `✕ not installed` (`DangerStyle`) для отсутствия. + +## Technical Details + +- `isTUITakeover(out []byte) bool` = `bytes.Contains(out, []byte("\x1b[?1049"))` — дубль из `model/textutil.go` (не импортим `model` в `version`). +- `cargoVersionFromList(list, binName string) string`: идём по строкам; header (не начинается с пробела/таба) вида ` vX.Y.Z:` — запоминаем версию через **`versionRe.FindStringSubmatch(line)[1]`** (группа `(\d+\.\d+[\d.]*)` **без** ведущего `v` — `FindString` вернул бы `v0.1.0`, а installed-строки в проекте без `v`: brew-dir → `1.96.1`, `--version` → `26.5.6`); indented-строка = имя бинарника; `trimmed == binName` → версия запомненного header'а. `inertia` в `inertia-tui v0.1.0:` → `0.1.0`. +- `cargoListVersion(binName string) string`: **у `version` нет `runProbe`** (он в `updater`), поэтому инлайним свой прогон с собственным таймаутом: `LookPath("cargo")` (при промахе `""` мгновенно) → `context.WithTimeout` (мирроринг 2s из `InstalledVersion`, чтобы большой список крейтов не стопорил горутину `fetchInstalledCmd`) + `exec.CommandContext("cargo","install","--list")` + `proc.DetachTTY` → `cargoVersionFromList`. +- `brewPrefix()` в `updater` (дубль ~12 строк из `version/brew.go`): `HOMEBREW_PREFIX` env, иначе первый существующий из `/opt/homebrew`, `/usr/local`, `/home/linuxbrew/.linuxbrew`; сид `var testBrewPrefix string`. +- `brewNamePlanAt(name, prefix string) (Plan, bool)`: `prefix==""` || `name==""` || `name` содержит `/`/`\` → `false` (traversal-гард, зеркало `brewDirVersion`); `os.Stat`+`IsDir` для `/Cellar/` и `/Caskroom/` → `autoPlan("brew", []string{"brew","upgrade",name})`. +- Рендер (switch в порядке): `installed!=""` → `installed:  `; `InstalledKnown && "" && InstalledPresent` → `installed: ` + `OkStyle.Render("✓")` + ` no version`; `InstalledKnown && "" && !InstalledPresent` → `installed: ` + `DangerStyle.Render("✕")` + ` not installed`; default → `installed: detecting…`. **Правим только две средние ветви и default** — ветку `installed != ""` (сохраняет glyph U+F412 + версию, render.go:1097) не трогаем. +- **Не трогаем:** `hasUpdate`/`↑` (нужны обе версии), `needsInstalled` (завязан на `InstalledKnown`). + +## What Goes Where + +- **Implementation Steps** (`[ ]`): код в `internal/version`, `internal/updater`, `internal/model`, `internal/ui`, их тесты, обновление CLAUDE.md. +- **Post-Completion** (без чекбоксов): ручной smoke в живом TUI; caveat про источник данных GitHub Releases для rust. + +## Implementation Steps + +### Task 1: `version.InstalledVersion` — presence + cargo/TUI fallbacks (+ model plumbing) + +Смена сигнатуры компилируется только вместе с потребителем, поэтому `version/detect.go` и минимальный plumbing в `model` — одна задача. Рендер (4 состояния) вынесен в Task 2 — он на компиляцию не влияет. + +**Files:** +- Modify: `internal/version/detect.go` +- Modify: `internal/version/detect_test.go` +- Modify: `internal/version/brew_test.go` +- Modify: `internal/model/model.go` +- Modify: `internal/model/commands.go` + +- [x] `detect.go`: сменить сигнатуру `InstalledVersion(t loader.Tool) string` → `(ver string, present bool)`; завести `binaryExists bool`, ставить `true` на успешном `exec.LookPath(args[0])` (на промахе — не трогать) +- [x] `detect.go`: добавить дубль `isTUITakeover(out []byte) bool` (`bytes.Contains(out, []byte("\x1b[?1049"))`, import `"bytes"`); сразу после `cmd.CombinedOutput()` — если `isTUITakeover(out)`, сбросить capture и `break` из цикла, **не** добавляя reason + - ➕ уточнение скоупа: на takeover сбрасываем **и накопленные** `reasons` (`reasons = nil`), а не только текущий capture. Иначе тул, у которого `--version` падает не-нулём, а `-V` уходит в TUI, всё равно писал бы anomaly-лог на каждом старте — ровно тот сигнал logx, который задача чинит. Takeover — это классифицированное поведение, а не аномалия +- [x] `detect.go`: добавить `cargoVersionFromList(list, binName string) string` — чистый парсер header/indented, версию берёт через `versionRe.FindStringSubmatch(line)[1]` (**без** ведущего `v`, чтобы `installed:` совпадал с brew/`--version` формой) +- [x] `detect.go`: добавить `cargoListVersion(binName string) string` — `LookPath("cargo")` (промах → `""`) → **свой** `context.WithTimeout` (2s, как в `InstalledVersion`; `version` не имеет `runProbe`) + `exec.CommandContext` + `proc.DetachTTY` → `cargoVersionFromList` +- [x] `detect.go`: порядок fallback `--version`/`-V` → `brewDirVersion(t.Name)` → `cargoListVersion(t.Name)` (вызывать **только** при `binaryExists`); вернуть `(ver, present)`, где `present = binaryExists || ver!=""` +- [x] `model/model.go`: добавить `present bool` в `installedMsg`; `InstalledPresent bool` в `VersionInfo` (коммент почему); в `case installedMsg` — `info.InstalledPresent = msg.present` +- [x] `model/commands.go`: `fetchInstalledCmd` → `ver, present := version.InstalledVersion(t)`; прокинуть оба в `installedMsg` +- [x] обновить существующие тесты под 2-значную сигнатуру: `detect_test.go` (строки ~87/104/123/145/162), `brew_test.go` (~150/154/157) +- [x] написать тесты: `cargoVersionFromList` (несколько крейтов, бинарник во втором блоке, `inertia-tui v0.1.0:` → `0.1.0`, промах → `""`); дубль `isTUITakeover` (сырой `\x1b[?1049` → true) +- [x] написать тесты: present-но-без-версии (LookPath есть, версии нет → `present==true, ver==""`) и not-present (`missingtool` → `present==false`); проверить, что TUI-takeover-вывод **не** пишет anomaly-лог и не парсит `0.30.2` +- [x] `go build ./...` + `go test -race ./internal/version/... ./internal/model/...` — зелёные перед Task 2 + +### Task 2: Рендер `installed:` — 4 состояния + `ui.OkStyle` + +**Files:** +- Modify: `internal/ui/styles.go` +- Modify: `internal/model/render.go` +- Modify: `internal/model/render_test.go` + +- [x] `ui/styles.go`: добавить `OkStyle = lipgloss.NewStyle().Foreground(ColorGreen)` (не переиспользовать `UpdateAvailableStyle`) +- [x] `render.go` (~1095): расширить switch до 4 ветвей — версия / `✓ no version` (`OkStyle`) / `✕ not installed` (`DangerStyle`) / `detecting…` +- [x] **обновить существующий подтест** `render_test.go:2558-2566` «detection reported empty: not found»: `VersionInfo{Latest:"v2.0.0", InstalledKnown:true}` теперь рендерит `✕ not installed` — поменять assert `installed: ✕ not found` → `installed: ✕ not installed`; поправить doc-коммент `TestRenderCardInstalledLatest` (строка ~2517, `"✕ not found" once it reported empty`) +- [x] добавить подтест `present-but-no-version`: `VersionInfo{Latest:"v2.0.0", InstalledKnown:true, InstalledPresent:true}` → assert `installed: ✓ no version` +- [x] (подтест `detecting…` на строке ~2568 остаётся как есть — `InstalledKnown:false`); его негативный assert перевели с мёртвой строки `"not found"` на актуальную `"not installed"` +- [x] `go test -race ./internal/model/...` — зелёные перед Task 3 + +### Task 3: `updater` — brew-by-name для бинарников не в PATH + +**Files:** +- Modify: `internal/updater/updater.go` +- Modify: `internal/updater/updater_test.go` + +- [x] `updater.go`: добавить `brewPrefix()` + сид `var testBrewPrefix string` (дубль из `version/brew.go`) +- [x] `updater.go`: добавить чистое ядро `brewNamePlanAt(name, prefix string) (Plan, bool)` (traversal-гард на `/`/`\`; `Cellar/` и `Caskroom/` через `os.Stat`+`IsDir`; хит → `autoPlan("brew", ["brew","upgrade",name])`) + обёртку `brewNamePlan(name) (Plan, bool)` +- [x] `updater.go`: в `Detect`, в ветке промаха `exec.LookPath(t.Name)` — до возврата `ErrUnknownManager` вставить `if plan, ok := brewNamePlan(t.Name); ok { return plan, nil }` +- [x] написать тесты: `brewNamePlanAt` table (temp-prefix с `Cellar/rust/1.96.1` → `brew upgrade rust`; нет каталога → `ok=false`; имя с `/`/`\` → `false`) +- [x] написать тест `Detect` через `testBrewPrefix` (name без бинарника, но с `Cellar/` → brew-план); заодно закрепили, что `update_cmd` по-прежнему выигрывает у новой ветки +- [x] ➕ `TestDetectMissingBinary` изолирован пустым `testBrewPrefix` — иначе исход зависел бы от того, что стоит на машине через brew +- [x] `go test -race ./internal/updater/...` — зелёные перед Task 4 + +### Task 4: Verify acceptance criteria + +- [x] сверить, что все пункты Overview реализованы (rust → `brew upgrade rust`; inertia → `installed: 0.1.0`; не-установленный тул → `✕ not installed`; present-без-версии → `✓ no version`) + - проверено временными probe-тестами против **живой машины** (созданы, прогнаны, удалены). `version.InstalledVersion`: `inertia` → `("0.1.0", true)`, `rust` → `("1.96.1", true)`, `definitely-missing-xyz` → `("", false)`, `git` → `("2.50.1", true)`; лог по итогам всех четырёх — **пустой**. `updater.Detect`: `rust` → `brew upgrade rust`, `inertia` → `cargo install inertia-tui`, отсутствующий тул → `ErrUnknownManager` + - ⚠️ нюанс харнесса: `TestMain` пакета `version` пинит `testBrewPrefix` в пустой temp-каталог, поэтому probe изнутри пакета видит `rust` как not-present, пока префикс не вернуть на `/opt/homebrew`. На продакшн-путь не влияет, но любой будущий probe об это споткнётся +- [x] сверить крайние случаи (нет cargo → мгновенный short-circuit; TUI-takeover → нет anomaly-лога и мис-парса; traversal-гард) +- [x] прогнать полный набор: `go build ./... && go vet ./... && go test -race ./...` +- [x] прогнать lint: `golangci-lint run` (проверить отсутствие `unused` на новых хелперах) — `0 issues` +- [x] (skill `preflight` покрывает build/vet/test-race/lint одним прогоном) + +### Task 5: Update documentation + +- [x] обновить `CLAUDE.md` (skill `docs-sync`): в описании version-fallback'ов добавить cargo-list рядом с brew; в цепочке `updater` — brew-by-name на промахе `LookPath`; отметить 4-е состояние `installed:` и новую сигнатуру `InstalledVersion(t) (string, bool)` + - плюс: `installedMsg{…, present}` в async-split, `isTUITakeover` в секции sandboxing, оговорка про `testBrewPrefix` в двух пакетах, logx-сайт `InstalledVersion` +- [x] ➕ обновить `ARCHITECTURE.md` (skill `docs-sync` требует все три документа): таблица пакетов, «пять источников данных» (installed), секция `Updating a tool (u)`, список тест-сидов. Граф импортов не менялся — новых внутренних зависимостей нет, mermaid трогать не нужно +- [x] обновить README.md, если затронуты видимые фичи (оказалось — нужно): 4 состояния `installed:` в разделе `Panel [2] Brief` + brew-by-name в `Updating tools` +- [x] переместить этот план в `docs/plans/completed/` + +## Post-Completion + +*Только ручные/внешние действия — без чекбоксов.* + +**Ручной smoke в живом TUI** (`go run .`): +- `rust`: карточка `[2]`, `[u]` — теперь либо запускает `brew upgrade rust`, либо честно «no update available» (см. caveat ниже), но **не** «no known updater». +- `inertia`: `installed:  0.1.0` без «not found», в `logs/` нет нового файла после старта. +- любой не-установленный трекнутый тул: `installed: ✕ not installed`. +- проверить, что `logs/` не пополняется на чистом старте (сигнал logx «лог = что-то сломалось» цел). + +**Caveat (скоуп не расширять):** +- Фикс `updater` убирает тупик «no known updater», но *предложит* ли keeptui апдейт rust (маркер `↑` + активный `[u]`) зависит от того, публикует ли `github.com/rust-lang/rust` GitHub **Releases** и есть ли тег новее установленного `1.96.1`. Если релизов нет или версия совпадает — `[u]` скажет «no update available». Это про источник данных, Вариантом A не покрывается. +- Присутствие определяется по `LookPath`, а не по наличию man/help (они в `model`; тянуть в `version` — инверсия графа). Кейс «man есть, бинарника в PATH нет» не покрываем (YAGNI). +- Максимум один лишний `cargo install --list` для редкого «установлен, непробуем, не brew и не cargo» тула; при отсутствии cargo `LookPath` закорачивает мгновенно. diff --git a/internal/model/commands.go b/internal/model/commands.go index c21fe32..d43229f 100644 --- a/internal/model/commands.go +++ b/internal/model/commands.go @@ -156,10 +156,11 @@ func notFoundExit(err error) bool { // so the installed version can render before any GitHub fetch completes. func fetchInstalledCmd(t loader.Tool) tea.Cmd { return safeCmd("fetchInstalledCmd", func() tea.Msg { - installed := version.InstalledVersion(t) + installed, present := version.InstalledVersion(t) return installedMsg{ toolName: t.Name, installed: installed, + present: present, } }) } diff --git a/internal/model/model.go b/internal/model/model.go index e0c0188..dfd286b 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -32,14 +32,23 @@ type VersionInfo struct { // flight" — only the installedMsg handler sets it, so the card can show // a pending state instead of a premature "not found". InstalledKnown bool + // InstalledPresent separates the two states InstalledKnown collapses when + // Installed == "": the tool is installed but won't name its version (a TUI + // app that ignores --version) versus not installed at all. Only the + // installedMsg handler sets it, from version.InstalledVersion's second + // result; the card renders the two differently. + InstalledPresent bool } -// installedMsg carries the locally detected installed version for a tool. -// It is emitted by fetchInstalledCmd independently of any network activity so -// the installed version renders immediately, without waiting on GitHub. +// installedMsg carries the locally detected installed version for a tool, plus +// whether the tool is present at all (a version-less install is still an +// install). It is emitted by fetchInstalledCmd independently of any network +// activity so the installed version renders immediately, without waiting on +// GitHub. type installedMsg struct { toolName string installed string + present bool } // remoteMsg carries the result of a single network pass (release + repo info + @@ -433,6 +442,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { info := m.versions[msg.toolName] info.Installed = msg.installed info.InstalledKnown = true + info.InstalledPresent = msg.present m.versions[msg.toolName] = info if hasSel { m.metaSelected = m.indexOfMeta(prev.Name) diff --git a/internal/model/render.go b/internal/model/render.go index 7a1b5a3..d296f9c 100644 --- a/internal/model/render.go +++ b/internal/model/render.go @@ -1084,21 +1084,27 @@ func (m Model) buildCard() (string, map[int]string) { if hasCard && card.Stars > 0 { sb.WriteString(ui.InfoStyle.Render(fmt.Sprintf("stars: %s", formatStars(card.Stars))) + "\n") } - // "not found" only once the local probe reported back — before that - // an empty Installed just means the detection is still in flight. + // The version-less states resolve only once the local probe reported + // back (InstalledKnown) — before that an empty Installed just means the + // detection is still in flight. Present-but-version-less is a working + // install (a TUI app that ignores --version), so it reads green and + // affirmative; only a genuine absence gets the red ✕. // A resolved version carries the same U+F412 (nf-oct-tag) glyph the // latest: line uses — here it marks a version, not specifically a // release tag; the two version lines read as a pair. The states with no - // version to tag (not found / detecting) stay bare. Written as a \u - // escape on purpose — the raw glyph is invisible in most editors and - // diffs and gets silently lost. + // version to tag stay bare. Written as a \u escape on purpose — the raw + // glyph is invisible in most editors and diffs and gets silently lost. switch { case installed != "": sb.WriteString(ui.InfoStyle.Render("installed: \uf412 "+installed) + "\n") + case vinfo.InstalledKnown && vinfo.InstalledPresent: + sb.WriteString(ui.InfoStyle.Render("installed: ") + + ui.OkStyle.Render("✓") + + ui.InfoStyle.Render(" no version") + "\n") case vinfo.InstalledKnown: sb.WriteString(ui.InfoStyle.Render("installed: ") + ui.DangerStyle.Render("✕") + - ui.InfoStyle.Render(" not found") + "\n") + ui.InfoStyle.Render(" not installed") + "\n") default: sb.WriteString(ui.InfoStyle.Render("installed: detecting…") + "\n") } diff --git a/internal/model/render_test.go b/internal/model/render_test.go index 7b195fc..7a85998 100644 --- a/internal/model/render_test.go +++ b/internal/model/render_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "os/exec" + "path/filepath" "regexp" "slices" "strings" @@ -1717,6 +1718,77 @@ func TestFetchInstalledCmd(t *testing.T) { if msg.toolName != "nonexistent-tool-xyz" { t.Errorf("toolName = %q, want %q", msg.toolName, "nonexistent-tool-xyz") } + // The command must carry InstalledVersion's second result, not just the + // version: a tool that is not on PATH is the "not installed" state the card + // renders differently from a version-less install. + if msg.present { + t.Error("present = true for a tool that does not exist") + } + + // false is the zero value, so the assertion above cannot catch a dropped + // present on its own — pin the true case against a binary that really is on + // PATH but answers no version, the state the two differ in. + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "presenttool"), []byte("#!/bin/sh\nexit 1\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + hit, ok := fetchInstalledCmd(loader.Tool{Name: "presenttool"})().(installedMsg) + if !ok { + t.Fatalf("expected installedMsg, got %T", hit) + } + if hit.installed != "" { + t.Errorf("installed = %q, want empty — the fake answers no version", hit.installed) + } + if !hit.present { + t.Error("present = false for a binary that is on PATH") + } +} + +// TestInstalledMsgCarriesPresence pins the plumbing between the probe and the +// card: fetchInstalledCmd's present flag must survive the handler into +// VersionInfo and reach the rendered installed: line. Without this the +// assignment could be dropped and every other test would stay green — the +// render tests build VersionInfo directly. +func TestInstalledMsgCarriesPresence(t *testing.T) { + newModel := func() Model { + m := New([]loader.ToolMeta{{Name: "gh", GitHub: "cli/cli"}}) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 24}) + return updated.(Model) + } + + tests := []struct { + name string + present bool + want string + notWant string + }{ + {"installed but version-less", true, "installed: ✓ no version", "not installed"}, + {"absent", false, "installed: ✕ not installed", "no version"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := newModel() + updated, _ := m.Update(installedMsg{toolName: "gh", installed: "", present: tt.present}) + nm := updated.(Model) + + info := nm.versions["gh"] + if info.InstalledPresent != tt.present { + t.Errorf("InstalledPresent = %v, want %v", info.InstalledPresent, tt.present) + } + if !info.InstalledKnown { + t.Error("InstalledKnown = false, want true — the probe reported back") + } + card := stripANSI(nm.renderCard()) + if !strings.Contains(card, tt.want) { + t.Errorf("card missing %q; got:\n%s", tt.want, card) + } + if strings.Contains(card, tt.notWant) { + t.Errorf("card shows the other presence state (%q); got:\n%s", tt.notWant, card) + } + }) + } } func TestNeedsInstalled(t *testing.T) { @@ -2514,7 +2586,8 @@ func TestRenderCardSpinner(t *testing.T) { // TestRenderCardInstalledLatest covers the [info] version lines: installed: // renders whenever the section is open (muted "detecting…" while the local -// probe is in flight, "✕ not found" once it reported empty), the section +// probe is in flight, "✓ no version" / "✕ not installed" once it reported +// empty, split by presence), the section // opens for a GitHub-less tool once an installed version is known, and // latest: gains the update highlight + ↑ only when the installed version is // older. The model goes through New + WindowSizeMsg so renderCard sees the @@ -2555,16 +2628,31 @@ func TestRenderCardInstalledLatest(t *testing.T) { } }) - t.Run("detection reported empty: not found", func(t *testing.T) { + t.Run("detection reported empty and absent: not installed", func(t *testing.T) { m := newCardModel("cli/cli") m.versions["gh"] = VersionInfo{Latest: "v2.0.0", InstalledKnown: true} m.repoCards["gh"] = version.RepoCard{Latest: "v2.0.0"} card := stripANSI(m.renderCard()) - if !strings.Contains(card, "installed: ✕ not found") { + if !strings.Contains(card, "installed: ✕ not installed") { t.Errorf("card missing installed fallback with ✕ marker; got:\n%s", card) } }) + // A tool that is installed but won't name its version (a TUI app ignoring + // --version) must not read as missing: same empty Installed, different line. + t.Run("detection reported empty but present: no version", func(t *testing.T) { + m := newCardModel("cli/cli") + m.versions["gh"] = VersionInfo{Latest: "v2.0.0", InstalledKnown: true, InstalledPresent: true} + m.repoCards["gh"] = version.RepoCard{Latest: "v2.0.0"} + card := stripANSI(m.renderCard()) + if !strings.Contains(card, "installed: ✓ no version") { + t.Errorf("card missing present-but-version-less line; got:\n%s", card) + } + if strings.Contains(card, "not installed") { + t.Errorf("an installed tool must not read as missing; got:\n%s", card) + } + }) + t.Run("detection pending: detecting, not \"not found\"", func(t *testing.T) { m := newCardModel("cli/cli") m.versions["gh"] = VersionInfo{Latest: "v2.0.0"} @@ -2573,8 +2661,8 @@ func TestRenderCardInstalledLatest(t *testing.T) { if !strings.Contains(card, "installed: detecting…") { t.Errorf("card missing pending installed line; got:\n%s", card) } - if strings.Contains(card, "not found") || strings.Contains(card, "✕") { - t.Errorf("card claims not found before detection finished; got:\n%s", card) + if strings.Contains(card, "not installed") || strings.Contains(card, "✕") { + t.Errorf("card claims not installed before detection finished; got:\n%s", card) } }) diff --git a/internal/ui/styles.go b/internal/ui/styles.go index 2eea271..6d63c4a 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -84,6 +84,13 @@ var ( Foreground(ColorOrange). Bold(true) + // OkStyle marks a benign affirmative state — the card's "installed but + // version unknown". Deliberately not UpdateAvailableStyle: orange there + // means "act on this", and a working install needs no action. + OkStyle = lipgloss.NewStyle(). + Foreground(ColorGreen). + Bold(true) + // My Tools status colors StatusColorActive = ColorGreen StatusColorTrying = ColorOrange diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 1a8b488..171f847 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -47,6 +47,55 @@ func homeDir() string { return h } +// testBrewPrefix overrides the Homebrew prefix in tests. +var testBrewPrefix string + +// brewPrefix resolves the Homebrew installation prefix without spawning brew +// (a ruby process that costs ~1.5s per invocation): the HOMEBREW_PREFIX env +// var brew's shellenv exports, else the first standard location that exists. +// Empty when brew is not installed (including Windows, where none exist). +// A deliberate duplicate of version.brewPrefix — both packages are bottom +// leaves of the import graph and neither may depend on the other; twelve +// duplicated lines cost less than a shared package they would both import. +func brewPrefix() string { + if testBrewPrefix != "" { + return testBrewPrefix + } + if p := os.Getenv("HOMEBREW_PREFIX"); p != "" { + return p + } + for _, p := range []string{"/opt/homebrew", "/usr/local", "/home/linuxbrew/.linuxbrew"} { + if fi, err := os.Stat(p); err == nil && fi.IsDir() { + return p + } + } + return "" +} + +// brewNamePlan reports the brew upgrade plan for a tracked name that is itself +// a formula/cask, for binaries LookPath cannot find. Thin OS-facing wrapper +// over brewNamePlanAt. +func brewNamePlan(name string) (Plan, bool) { + return brewNamePlanAt(name, brewPrefix()) +} + +// brewNamePlanAt is the pure core: a keg or cask directory named after the tool +// means brew owns it, and `brew upgrade ` is the update command. Mirrors +// version.brewDirVersion's traversal guard — a brew formula/cask name is a bare +// identifier, so a name carrying a path separator can't be one and must not +// turn the Join into a traversal. +func brewNamePlanAt(name, prefix string) (Plan, bool) { + if prefix == "" || name == "" || strings.ContainsAny(name, `/\`) { + return Plan{}, false + } + for _, room := range []string{"Cellar", "Caskroom"} { + if fi, err := os.Stat(filepath.Join(prefix, room, name)); err == nil && fi.IsDir() { + return autoPlan("brew", []string{"brew", "upgrade", name}), true + } + } + return Plan{}, false +} + // Detect resolves the update Plan for a tool. It is the OS-facing wrapper over // the pure detectFromPath core: it spawns subprocesses (go version -m, cargo // install --list) and must therefore never run on a latency-sensitive path such @@ -74,6 +123,14 @@ func Detect(t loader.Tool) (Plan, error) { found, err := exec.LookPath(t.Name) if err != nil { + // No binary by that name, but the tracker name can still be a brew + // formula whose binaries are named differently — "rust" ships rustc and + // cargo, so LookPath("rust") misses while `brew upgrade rust` is exactly + // the right command. The branch is self-validating: it fires only when a + // keg/cask directory of that name actually exists. + if plan, ok := brewNamePlan(t.Name); ok { + return plan, nil + } return Plan{}, fmt.Errorf("%s not installed: %w", t.Name, ErrUnknownManager) } realPath, err := filepath.EvalSymlinks(found) diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go index 33fa770..e9b0ad0 100644 --- a/internal/updater/updater_test.go +++ b/internal/updater/updater_test.go @@ -126,6 +126,10 @@ func TestDetectUpdateCmdOverride(t *testing.T) { } func TestDetectMissingBinary(t *testing.T) { + // Empty brew prefix: no keg can be found by name either, so the miss is + // conclusive regardless of what the host machine has installed. + setTestBrewPrefix(t, t.TempDir()) + tool := loader.Tool{Name: "definitely-not-a-real-binary-xyz"} _, err := Detect(tool) if !errors.Is(err, ErrUnknownManager) { @@ -133,6 +137,105 @@ func TestDetectMissingBinary(t *testing.T) { } } +// setTestBrewPrefix points brewPrefix at dir for the duration of the test, +// restoring the previous override afterwards. +func setTestBrewPrefix(t *testing.T, dir string) { + t.Helper() + orig := testBrewPrefix + testBrewPrefix = dir + t.Cleanup(func() { testBrewPrefix = orig }) +} + +func TestBrewNamePlanAt(t *testing.T) { + prefix := t.TempDir() + // rust: the motivating case — a formula whose binaries (rustc, cargo) are + // named differently, so LookPath("rust") can never find it. + mustMkdirAll(t, filepath.Join(prefix, "Cellar", "rust", "1.96.1")) + mustMkdirAll(t, filepath.Join(prefix, "Caskroom", "someapp", "2.0.0")) + // A plain file, not a directory — must not be mistaken for a keg. + mustMkdirAll(t, filepath.Join(prefix, "Cellar")) + if err := os.WriteFile(filepath.Join(prefix, "Cellar", "notadir"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + tool string + prefix string + wantOK bool + wantArgv []string + }{ + {"cellar formula", "rust", prefix, true, []string{"brew", "upgrade", "rust"}}, + {"caskroom cask", "someapp", prefix, true, []string{"brew", "upgrade", "someapp"}}, + {"no such keg", "nothere", prefix, false, nil}, + {"keg path is a file", "notadir", prefix, false, nil}, + {"no brew prefix", "rust", "", false, nil}, + {"empty name", "", prefix, false, nil}, + // Traversal guard: a brew formula name is a bare identifier. + {"name with slash", "../../etc", prefix, false, nil}, + {"name with backslash", `..\..\etc`, prefix, false, nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + plan, ok := brewNamePlanAt(tt.tool, tt.prefix) + if ok != tt.wantOK { + t.Fatalf("brewNamePlanAt(%q, %q) ok = %v, want %v", tt.tool, tt.prefix, ok, tt.wantOK) + } + if !ok { + return + } + if plan.Manager != "brew" { + t.Errorf("Manager = %q, want %q", plan.Manager, "brew") + } + if !equalStrings(plan.Argv, tt.wantArgv) { + t.Errorf("Argv = %v, want %v", plan.Argv, tt.wantArgv) + } + if want := strings.Join(tt.wantArgv, " "); plan.Display != want { + t.Errorf("Display = %q, want %q", plan.Display, want) + } + }) + } +} + +// TestDetectBrewByName pins the Detect branch: a tracked name with no binary of +// its own still resolves to brew when a keg of that name exists — the "rust is +// tracked, rustc is installed" case that used to dead-end in ErrUnknownManager. +func TestDetectBrewByName(t *testing.T) { + prefix := t.TempDir() + mustMkdirAll(t, filepath.Join(prefix, "Cellar", "rust", "1.96.1")) + setTestBrewPrefix(t, prefix) + // An empty PATH guarantees the LookPath miss this branch hangs off. + t.Setenv("PATH", t.TempDir()) + + plan, err := Detect(loader.Tool{Name: "rust"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if plan.Manager != "brew" { + t.Errorf("Manager = %q, want %q", plan.Manager, "brew") + } + wantArgv := []string{"brew", "upgrade", "rust"} + if !equalStrings(plan.Argv, wantArgv) { + t.Errorf("Argv = %v, want %v", plan.Argv, wantArgv) + } + + // An explicit UpdateCmd still wins over the brew branch. + custom, err := Detect(loader.Tool{Name: "rust", UpdateCmd: "rustup update"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if custom.Manager != "custom" || custom.Display != "rustup update" { + t.Errorf("UpdateCmd override lost to the brew branch: %+v", custom) + } +} + +func mustMkdirAll(t *testing.T, dir string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } +} + func TestDetectResolvesSymlink(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink/PATH fixture is unix-only") diff --git a/internal/version/brew_test.go b/internal/version/brew_test.go index fab3ffd..45eb089 100644 --- a/internal/version/brew_test.go +++ b/internal/version/brew_test.go @@ -147,15 +147,16 @@ func TestInstalledVersionBrewFallback(t *testing.T) { restore := logx.SetDirForTesting(logDir) defer restore() - if got := InstalledVersion(loader.Tool{Name: "agterm"}); got != "0.15.1" { - t.Fatalf("expected brew fallback to find 0.15.1, got %q", got) + if got, present := InstalledVersion(loader.Tool{Name: "agterm"}); got != "0.15.1" || !present { + t.Fatalf("expected brew fallback to find 0.15.1 (present), got %q (present=%v)", got, present) } - // A cask with no PATH binary at all resolves purely from the brew layout. - if got := InstalledVersion(loader.Tool{Name: "noclitool"}); got != "14.1.0" { - t.Fatalf("expected brew fallback to find 14.1.0, got %q", got) + // A cask with no PATH binary at all resolves purely from the brew layout — + // and a version found there is itself proof the tool is installed. + if got, present := InstalledVersion(loader.Tool{Name: "noclitool"}); got != "14.1.0" || !present { + t.Fatalf("expected brew fallback to find 14.1.0 (present), got %q (present=%v)", got, present) } - if got := InstalledVersion(loader.Tool{Name: "clitool"}); got != "5.5.5" { - t.Fatalf("expected CLI answer to win over brew dir, got %q", got) + if got, present := InstalledVersion(loader.Tool{Name: "clitool"}); got != "5.5.5" || !present { + t.Fatalf("expected CLI answer to win over brew dir, got %q (present=%v)", got, present) } // A fallback hit is this path's normal state, not a malfunction — the // failed --version/-V attempts must not create a session log. diff --git a/internal/version/detect.go b/internal/version/detect.go index 8217f86..4bf4052 100644 --- a/internal/version/detect.go +++ b/internal/version/detect.go @@ -1,6 +1,7 @@ package version import ( + "bytes" "context" "os/exec" "regexp" @@ -16,7 +17,14 @@ import ( var versionRe = regexp.MustCompile(`v?(\d+\.\d+[\d.]*)`) -func InstalledVersion(t loader.Tool) string { +// InstalledVersion detects the locally installed version of t and reports +// whether the tool is present at all. The two results are independent: a tool +// can be installed yet refuse to name its version (a ratatui app that ignores +// --version and boots its own TUI), which the card must not render the same +// way as "not installed". Sources in order: the binary's own --version/-V, +// then Homebrew's directory layout, then `cargo install --list` — the last two +// answer without running the tool. +func InstalledVersion(t loader.Tool) (string, bool) { var candidates [][]string if t.VersionCmd != "" { @@ -38,10 +46,14 @@ func InstalledVersion(t loader.Tool) string { // "a log file means something went wrong" signal. We also do not log inside // the loop — a --version miss followed by a -V success must stay silent. var reasons []string + // binaryExists is the presence signal: set on a LookPath hit, i.e. the tool + // is installed regardless of whether any candidate names its version. + var binaryExists bool for _, args := range candidates { if _, err := exec.LookPath(args[0]); err != nil { continue // not installed — benign, never logged } + binaryExists = true ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) cmd := exec.CommandContext(ctx, args[0], args[1:]...) // Detached from the controlling terminal: a tool that ignores @@ -49,12 +61,25 @@ func InstalledVersion(t loader.Tool) string { proc.DetachTTY(cmd) out, err := cmd.CombinedOutput() cancel() + // The tool ignored the flag and booted its own TUI (the same signature + // fetchHelpCmd discards): the capture is app frames plus the crash + // trace DetachTTY provokes, not a version string. Parsing it would + // mis-read a dependency's version out of the panic ("ratatui-0.30.2"), + // so the capture is dropped whole and the loop stops — the remaining + // candidate would only boot the app again. Accumulated reasons are + // dropped with it: this is now a classified kind of tool, not an + // anomaly, and logging it would re-create a session log on every + // startup, defeating logx's "a log file means something went wrong". + if isTUITakeover(out) { + reasons = nil + break + } if err != nil { reasons = append(reasons, strings.Join(args, " ")+": "+err.Error()) continue } if m := versionRe.FindString(string(out)); m != "" { - return m + return m, true } reasons = append(reasons, strings.Join(args, " ")+": no version string in output") } @@ -64,13 +89,91 @@ func InstalledVersion(t loader.Tool) string { // startup is this path's normal state, not a malfunction, and logx's // signal is "a log file means something went wrong". if v := brewDirVersion(t.Name); v != "" { - return v + return v, true + } + // Cargo fallback, the same idea one ecosystem over: a Rust TUI records its + // version in the crate registry even when the binary won't say it. Gated on + // binaryExists — the list is keyed by the binary name, so for a tool that + // isn't installed it can only miss, and this spends a subprocess. + if binaryExists { + if v := cargoListVersion(t.Name); v != "" { + return v, true + } } // Only an installed-but-unresponsive binary reaches here with reasons; a // tool simply absent from PATH leaves reasons empty and logs nothing. if len(reasons) > 0 { logx.Errorf("version.InstalledVersion: %s: %s", t.Name, strings.Join(reasons, "; ")) } + return "", binaryExists +} + +// isTUITakeover reports whether captured output carries the alt-screen +// signature, i.e. the probed tool ignored its arguments and booted its own TUI. +// Deliberately a duplicate of model.isTUITakeover: version sits at the bottom +// of the import graph and importing model would invert it. +func isTUITakeover(out []byte) bool { + return bytes.Contains(out, []byte("\x1b[?1049")) +} + +// cargoListVersion reads binName's version out of `cargo install --list`, +// which knows every cargo-installed crate's version without running its binary. +// Returns "" when cargo is absent (the common case, short-circuited by +// LookPath before any subprocess) or the binary isn't cargo-installed. The +// timeout mirrors InstalledVersion's own: a large crate list must not stall +// fetchInstalledCmd's goroutine. +func cargoListVersion(binName string) string { + if binName == "" { + return "" + } + if _, err := exec.LookPath("cargo"); err != nil { + return "" + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "cargo", "install", "--list") + proc.DetachTTY(cmd) + out, err := cmd.Output() + if err != nil { + return "" + } + return cargoVersionFromList(string(out), binName) +} + +// cargoVersionFromList extracts binName's version from `cargo install --list` +// output, whose shape is a crate header at column 0 followed by its indented +// binaries: +// +// inertia-tui v0.1.0: +// inertia +// ripgrep v14.1.0: +// rg +// +// The version comes from the regex's capture group, i.e. without the leading +// "v": every other source of installed: is bare (brew dirs "1.96.1", --version +// "26.5.6") and the card must not show one shape for cargo tools and another +// for the rest. A binary under a header with no parseable version yields "". +func cargoVersionFromList(list, binName string) string { + if binName == "" { + return "" + } + var current string + for line := range strings.SplitSeq(list, "\n") { + line = strings.TrimRight(line, "\r") + if line == "" { + continue + } + if line[0] != ' ' && line[0] != '\t' { + current = "" + if m := versionRe.FindStringSubmatch(line); m != nil { + current = m[1] + } + continue + } + if strings.TrimSpace(line) == binName { + return current + } + } return "" } diff --git a/internal/version/detect_test.go b/internal/version/detect_test.go index 476c055..8413c71 100644 --- a/internal/version/detect_test.go +++ b/internal/version/detect_test.go @@ -70,27 +70,200 @@ func TestInstalledVersion(t *testing.T) { t.Setenv("PATH", dir) tests := []struct { - name string - tool loader.Tool - want string + name string + tool loader.Tool + want string + wantPresent bool }{ - {"--version output", loader.Tool{Name: "goodtool"}, "1.2.3"}, - {"-V fallback when --version fails", loader.Tool{Name: "flagvtool"}, "2.0.1"}, - {"tool not on PATH", loader.Tool{Name: "missingtool"}, ""}, - {"tool exits non-zero on all candidates", loader.Tool{Name: "brokentool"}, ""}, + {"--version output", loader.Tool{Name: "goodtool"}, "1.2.3", true}, + {"-V fallback when --version fails", loader.Tool{Name: "flagvtool"}, "2.0.1", true}, + {"tool not on PATH", loader.Tool{Name: "missingtool"}, "", false}, + // Installed yet unresponsive: no version, but present — the card must + // not call this "not installed". + {"tool exits non-zero on all candidates", loader.Tool{Name: "brokentool"}, "", true}, // VersionCmd is never populated from ToolMeta today; this pins the // unit contract of the override path, not a production flow. - {"VersionCmd override", loader.Tool{Name: "customtool", VersionCmd: "customtool version"}, "v3.4.5"}, + {"VersionCmd override", loader.Tool{Name: "customtool", VersionCmd: "customtool version"}, "v3.4.5", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := InstalledVersion(tt.tool); got != tt.want { + got, present := InstalledVersion(tt.tool) + if got != tt.want { t.Errorf("InstalledVersion(%+v) = %q, want %q", tt.tool, got, tt.want) } + if present != tt.wantPresent { + t.Errorf("InstalledVersion(%+v) present = %v, want %v", tt.tool, present, tt.wantPresent) + } + }) + } +} + +func TestCargoVersionFromList(t *testing.T) { + // The real shape of `cargo install --list`: crate header at column 0, + // its binaries indented below it. + const list = `ripgrep v14.1.0: + rg +inertia-tui v0.1.0: + inertia +multi-bin v2.3.4: + alpha + beta +pathinstall v0.9.0 (/Users/me/src/pathinstall): + pathy +noversion: + orphan +` + + tests := []struct { + name string + list string + binName string + want string + }{ + {"first crate", list, "rg", "14.1.0"}, + // The motivating case: the binary name differs from the crate name and + // sits in a later block. + {"binary in a later block, name differs from crate", list, "inertia", "0.1.0"}, + {"second binary of a multi-binary crate", list, "beta", "2.3.4"}, + // A path/git install carries a source suffix after the version. + {"header with a source suffix", list, "pathy", "0.9.0"}, + {"header with no parseable version", list, "orphan", ""}, + {"binary not in the list", list, "missing", ""}, + {"crate name is not a binary name", list, "inertia-tui", ""}, + {"empty list", "", "rg", ""}, + {"empty binary name", list, "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := cargoVersionFromList(tt.list, tt.binName); got != tt.want { + t.Errorf("cargoVersionFromList(_, %q) = %q, want %q", tt.binName, got, tt.want) + } + }) + } +} + +func TestIsTUITakeover(t *testing.T) { + tests := []struct { + name string + out string + want bool + }{ + {"alt-screen enter", "\x1b[?1049h frames", true}, + {"alt-screen leave", "boom\x1b[?1049l", true}, + {"plain help text", "usage: tool [--version]", false}, + {"plain SGR color is not a takeover", "\x1b[31mred\x1b[0m", false}, + {"empty", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isTUITakeover([]byte(tt.out)); got != tt.want { + t.Errorf("isTUITakeover(%q) = %v, want %v", tt.out, got, tt.want) + } }) } } +func TestInstalledVersionTUITakeover(t *testing.T) { + dir := t.TempDir() + // A ratatui app probed with --version: it ignores the flag, boots its TUI + // (alt-screen), then panics because DetachTTY left it no terminal. The + // capture carries a dependency version (ratatui-0.30.2) that must never be + // mistaken for the tool's own. + writeFakeTool(t, dir, "tuitool", `printf '\033[?1049h\033[2J'; echo "panicked at ratatui-0.30.2/src/lib.rs"; exit 101`) + t.Setenv("PATH", dir) + + logDir := t.TempDir() + restore := logx.SetDirForTesting(logDir) + defer restore() + + got, present := InstalledVersion(loader.Tool{Name: "tuitool"}) + if got != "" { + t.Errorf("a TUI takeover must not yield a version, got %q", got) + } + if !present { + t.Error("a TUI app that boots is installed: present should be true") + } + // A TUI app answering this way is classified behavior, not an anomaly — + // logging it would re-create a session log on every startup. + if out := logx.ReadAllForTesting(logDir); out != "" { + t.Errorf("a TUI takeover must not log, got:\n%s", out) + } +} + +// TestInstalledVersionCargoFallback covers the wiring, not the parser +// (TestCargoVersionFromList owns that): a binary that won't name its version +// still resolves through `cargo install --list`, under the binary name rather +// than the crate name. TestMain's brew prefix is an empty directory, so the +// brew fallback misses and the cargo one is what answers. +func TestInstalledVersionCargoFallback(t *testing.T) { + dir := t.TempDir() + // The motivating tool: a ratatui app that ignores --version, boots its TUI + // and dies without a terminal. + writeFakeTool(t, dir, "inertia", `printf '\033[?1049h'; echo "panicked at ratatui-0.30.2"; exit 101`) + // cargo knows the version without running the binary at all. Note the crate + // name differs from the binary name — the whole point of the list lookup. + writeFakeTool(t, dir, "cargo", `echo "inertia-tui v0.1.0:"; echo " inertia"`) + t.Setenv("PATH", dir) + + logDir := t.TempDir() + restore := logx.SetDirForTesting(logDir) + defer restore() + + got, present := InstalledVersion(loader.Tool{Name: "inertia"}) + if got != "0.1.0" { + t.Errorf("InstalledVersion via cargo = %q, want %q", got, "0.1.0") + } + if !present { + t.Error("a cargo-installed binary on PATH must report present") + } + // A fallback hit is this path's normal state, like the brew one. + if out := logx.ReadAllForTesting(logDir); out != "" { + t.Errorf("a successful cargo fallback must not log, got:\n%s", out) + } +} + +// TestInstalledVersionBrewBeatsCargo pins the source order. Both fallbacks can +// answer for this tool and they disagree; brew is consulted first, so a tool +// installed by brew is never reported with a stale crate version left behind by +// an older cargo install. +func TestInstalledVersionBrewBeatsCargo(t *testing.T) { + binDir := t.TempDir() + writeFakeTool(t, binDir, "dualtool", `exit 1`) + writeFakeTool(t, binDir, "cargo", `echo "dualtool v9.9.9:"; echo " dualtool"`) + t.Setenv("PATH", binDir) + makeBrewLayout(t, map[string][]string{"Cellar/dualtool": {"1.2.3"}}) + + if got, present := InstalledVersion(loader.Tool{Name: "dualtool"}); got != "1.2.3" || !present { + t.Errorf("InstalledVersion = %q (present=%v), want %q from the brew layout", got, present, "1.2.3") + } +} + +// TestInstalledVersionCargoGatedOnPresence pins the binaryExists gate: the +// cargo list is keyed by binary name, so for a tool that isn't installed it can +// only miss — and spawning a subprocess per absent tool on every startup is the +// cost the gate exists to avoid. The fake cargo would happily answer, so a +// leaked gate fails twice: the marker appears and a version comes back. +func TestInstalledVersionCargoGatedOnPresence(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(t.TempDir(), "cargo-ran") + // The marker is written with a shell redirection, not `touch`: PATH is the + // fixture directory alone, so any external command the fake calls would + // silently fail to resolve and leave this assertion dead. + writeFakeTool(t, dir, "cargo", `: > "`+marker+`"; echo "ghost-crate v1.0.0:"; echo " ghosttool"`) + t.Setenv("PATH", dir) + + got, present := InstalledVersion(loader.Tool{Name: "ghosttool"}) + if got != "" { + t.Errorf("InstalledVersion = %q, want empty for a tool that is not installed", got) + } + if present { + t.Error("a tool absent from PATH must not report present") + } + if _, err := os.Stat(marker); err == nil { + t.Error("cargo install --list ran for a tool that is not installed — the binaryExists gate leaked") + } +} + func TestInstalledVersionMissingBinaryNoLog(t *testing.T) { dir := t.TempDir() t.Setenv("PATH", dir) @@ -101,9 +274,13 @@ func TestInstalledVersionMissingBinaryNoLog(t *testing.T) { // A tool that is simply not on PATH is the normal "not installed" state, // not a malfunction — it must not create a session log. - if got := InstalledVersion(loader.Tool{Name: "missingtool"}); got != "" { + got, present := InstalledVersion(loader.Tool{Name: "missingtool"}) + if got != "" { t.Fatalf("expected empty version, got %q", got) } + if present { + t.Error("a not-on-PATH tool must not report present") + } if out := logx.ReadAllForTesting(logDir); out != "" { t.Errorf("a not-on-PATH tool must not log, got:\n%s", out) } @@ -120,9 +297,13 @@ func TestInstalledVersionPresentButBrokenLogs(t *testing.T) { restore := logx.SetDirForTesting(logDir) defer restore() - if got := InstalledVersion(loader.Tool{Name: "brokentool"}); got != "" { + got, present := InstalledVersion(loader.Tool{Name: "brokentool"}) + if got != "" { t.Fatalf("expected empty version, got %q", got) } + if !present { + t.Error("a binary on PATH is installed even when it won't name its version") + } out := logx.ReadAllForTesting(logDir) // Exactly one log line despite two candidates (--version and -V). if lines := strings.Count(out, "version.InstalledVersion"); lines != 1 { @@ -142,7 +323,7 @@ func TestInstalledVersionLoggingFallbackNoLog(t *testing.T) { restore := logx.SetDirForTesting(logDir) defer restore() - if got := InstalledVersion(loader.Tool{Name: "flagvtool"}); got != "2.0.1" { + if got, _ := InstalledVersion(loader.Tool{Name: "flagvtool"}); got != "2.0.1" { t.Fatalf("expected 2.0.1 via -V fallback, got %q", got) } if out := logx.ReadAllForTesting(logDir); out != "" { @@ -159,7 +340,7 @@ func TestInstalledVersionLoggingSuccessNoLog(t *testing.T) { restore := logx.SetDirForTesting(logDir) defer restore() - if got := InstalledVersion(loader.Tool{Name: "goodtool"}); got != "1.2.3" { + if got, _ := InstalledVersion(loader.Tool{Name: "goodtool"}); got != "1.2.3" { t.Fatalf("expected 1.2.3, got %q", got) } if out := logx.ReadAllForTesting(logDir); out != "" {