diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1503fad..149cdd0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -71,7 +71,9 @@ The `model` package is split across files within a single package: | `mode.go` | The `inputMode` enum and a handler per input mode | | `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` — markdown → ANSI via glamour, with a single-entry render cache | +| `readme.go` | `renderReadme` — panel `[3]`'s pipeline: sanitize → preprocess → glamour, with a single-entry render cache | +| `readme_clean.go` | `cleanReadmeMarkdown` — the pure README preprocessor. Fenced blocks and inline spans are segmented out first (code is never rewritten), then badges, hrefs, HTML and emoji are removed from what is left | +| `readme_style.go` | `keepkitStyle(dark)` — panel `[3]`'s glamour theme: the standard config cloned, its accents replaced from `internal/ui`'s palette | | `textutil.go` | Pure text helpers (`wrapText`, `stripANSI`, `colorizeHelp`, `parseHelpEntries`, `markdownToLines` — the card's release-notes markdown → pre-wrapped tagged lines, …) | | `browser.go` | Opening URLs per `GOOS` | @@ -131,8 +133,27 @@ the `keepkit` screen. The protection has two layers: A library that probes the terminal counts as the same hazard: `glamour.WithAutoStyle()` is never used, because its termenv OSC background query reads stdin and races Bubble Tea's input reader. Dark/light is resolved once at construction via lipgloss's cached -`HasDarkBackground()` (`m.darkBG`) and passed to glamour as a fixed `WithStandardStyle`. -The README body itself is bounded (`readmeMaxBytes`) and sanitized before rendering. +`HasDarkBackground()` (`m.darkBG`) and picks one of `keepkitStyle`'s two variants, which +glamour receives as a fixed `WithStyles`. The README body itself is bounded +(`readmeMaxBytes`) and sanitized before rendering. + +Between the sanitizer and glamour sits `cleanReadmeMarkdown`, the house-style +preprocessor: a README is written for a browser, so badges, logos, `` wrappers, +hrefs nobody can click in a TTY and emoji a terminal font renders as tofu are removed +before rendering. It rests on one rule — **code is never rewritten** — because a fenced +block and an inline span are exactly how a README *shows* the markup being deleted, so +fences are segmented out (an unterminated one protects to EOF, since the +`readmeMaxBytes` cut can land mid-fence) and inline spans are masked for the duration. +Autolinks and bare URLs survive: there the URL *is* the content. Every rule that could +swallow ordinary prose is gated — a `[label]` form is unwrapped only for labels a +definition declared (ungated, `arr[i][j]` became `arri`), and a definition line is +deleted only when it cannot be part of a paragraph. Because the pass runs inside +`Update()` on up to `readmeMaxBytes`, no step in it may be superlinear; +`TestCleanReadmeMarkdownPathologicalInputIsFast` is the guard. `keepkitStyle` then +clones the standard config rather than building one, so a glamour upgrade that adds a +field cannot leave the panel with a hole in it — and because the package globals it +clones hold pointers that `styles.DefaultStyles` aliases, every override assigns a fresh +pointer instead of writing through a shared one. ## TUI state machine @@ -429,8 +450,9 @@ packages fails if the isolation is ever dropped. Per-test setup still uses the internal seams: `testConfigDir`, `testCacheDir`, `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). `testAPIBase` is private to `version`, so a `model` +`model`'s `testReadmeStyle` (routes the renderer through a named standard style +instead of `keepkitStyle`; an unknown name forces the glamour construction failure, +which is how the plain-text fallback is covered). `testAPIBase` is private to `version`, so a `model` test cannot redirect a fetch at an httptest server — a network command is executed there only when the cache can answer it (`seedSelfReleaseCache` for the self-check); otherwise `Init` batches are asserted by length, never run. The races are real (mutexes diff --git a/CLAUDE.md b/CLAUDE.md index 72ba48a..7952e1b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,7 +47,9 @@ The `model` package is split by responsibility (one package, several files): | `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 `[u]` and `[U]` — the flag only tags the message), `startUpdateCmd`, `waitForChunkCmd`, `startLaunchCmd`, `execToolCmd` + the pure per-GOOS `shellCommand`) + fetch predicates (`needsInstalled`, `needsRemote`, `needsReadme`, `refreshSelectedCmd`, `autoFetchCmdsForSelected`) | | `render.go` | `View`, panel/card/status-bar/gauge/overlay renderers, scrollbar, mouse handling | -| `readme.go` | `renderReadme` (glamour) + the single-entry `readmeRenderCache`; `readmeStyleName`/`testReadmeStyle` seam | +| `readme.go` | `renderReadme` — panel `[3]`'s pipeline (sanitize → preprocess → glamour) + the single-entry `readmeRenderCache`; `testReadmeStyle` seam | +| `readme_clean.go` | `cleanReadmeMarkdown` — the pure README preprocessor: fenced-block/inline-span segmentation first, then the image/link/HTML/emoji/shortcode removal rules on what is left | +| `readme_style.go` | `keepkitStyle(dark)` — panel `[3]`'s glamour theme, cloned from the standard config and re-accented from `internal/ui`'s palette | | `textutil.go` | Pure text/format helpers (`wrapText`/`wrapLine`, `stripANSI`, `colorizeHelp`, `parseHelpEntries`, `markdownToLines`/`mdInline` — the card's release-notes converter, `findMatches`, `formatStars`, `renderLangBar`, …) | | `browser.go` | `openURLCmd` + per-`GOOS` `browserCommand` | @@ -109,7 +111,12 @@ The model is a three-panel layout with focus cycling via `→/←` between `focu - **Completion**: the self branch of `updateDoneMsg` runs **before** the `toolByName` early return — for an untracked keepkit that lookup gives `!ok`, the message would be dropped, and `[U] restart` would never appear. The discriminator is **`m.isSelfUpdate(msg.tool)`**: an update of keepkit is a self-update whichever key started it, so on a release build `[u]` on a tracked `keepkit` row ends here too and gets the same restart offer (it used to take the tool path, which left the banner announcing an update the card already showed as installed, with `[U] restart` reachable only by running the whole update again — `TestKeepkitUpdateSelfHandlingGatedOnBuild`'s release row), while on a dev build that same keypress falls through to the plain tool path below (`TestKeepkitUpdateSelfHandlingGatedOnBuild`) — the name alone as the discriminator was what let a build with the feature off announce `keepkit updated — [U] restart` and then re-exec its own working copy. Success → `selfState = selfUpdated` + `statusMsg "updated keepkit"`, plus `fetchInstalledCmd` **only when tracked** (nothing else has a card or a `↑` to refresh); note the success write is legitimate even from `selfNone` — a rate-limited startup check plus `[u]` on a tracked row is exactly that, and the new binary really is on disk. Failure → `statusMsg "update failed — see [3]"` + the shared `recordUpdateFailure` and **no `selfState` write at all**: the banner reappears by itself once `updatingFor` clears, so every write here could only walk a state back — over an `[X]` folded mid-update (a deliberate "not now"), over a pending `[U] restart` from an earlier successful update, or over `selfNone`, where there is no `selfLatest` and the forced `selfOffered` rendered `keepkit available` with a hole in it that no later `selfCheckMsg` could fill (that handler writes only from `selfNone`). `TestSelfUpdateDoneFailureKeepsPriorState` pins all five prior states. `updatingFor` is cleared unconditionally in both. - **Restart**: `[U]` at `selfUpdated`/`selfUpdatedLater` sets `m.restartRequested` and returns `tea.Quit` — `selfState` is deliberately not moved (after `tea.Quit` nothing renders, and a failed exec exits anyway, so a sixth enum member would be dead). `RestartRequested()` is the exported accessor `main` reads off the model `p.Run()` returns, **strictly after** it returned: Bubble Tea has restored the terminal by then, whereas exec'ing from inside `Update` would hand the new process an alt screen it never opened. `restart_unix.go`'s `restartSelf()` delegates to the seam-injected `restartSelfWith(resolve, execve, out)` (the `exists`/`lookPath` idiom again, so both failure paths are testable) = `selfPath()` + `syscall.Exec(path, os.Args, os.Environ())` — same pid, same argv, same env, so the tab/tmux pane survives. Only a failure logs `logx.Errorf("restart self: …")` (a genuine anomaly) and prints the shared `restartHint` (`keepkit updated — run keepkit again`) to **stdout** at exit code 0 — the update did succeed, so this is an instruction, not diagnostics; a successful exec never comes back, so the whole tail is unreachable on success and says nothing. `restart_windows.go` prints the same hint and nothing else: there is no image-replacing exec, and spawning a child from a process about to exit hands the new keepkit a console the old one still owns. It logs nothing — planned degradation, not a malfunction. - **Path resolution** (`restart_unix.go`, pure core + thin wrapper): `resolveSelfPath(executable, exists, lookPath, argv0)` mirrors how a shell resolves what the user typed instead of trusting `os.Executable`. An argv0 carrying a path separator (`strings.Contains(argv0, "/")` — the file is `//go:build !windows`, so `/` is the only separator that can reach it; the backslash and the `.exe`-trimming in `sameProgram` that the core carried while it lived in the untagged `restart.go` were dead branches under the tag and are gone, along with the two test rows that exercised them) is already a path the user pointed at and wins when it `exists`, with no `lookPath` call — a same-named binary earlier in `PATH` must not hijack `./keepkit`. A **bare** argv0 goes through `lookPath` **first**, and this order is load-bearing on **Linux**: there `os.Executable()` reads `/proc/self/exe` symlink-*resolved*, so after a keg-style upgrade it can still name the live *old* binary, and exec'ing that would loop the feature (restart → the same banner → restart). Two things disqualify a `PATH` hit: `("", nil)` counts as a miss (an empty string must never reach `syscall.Exec`), and so does a hit whose `filepath.Base` differs from `executable`'s (`sameProgram`) — argv0 comes from the *parent* process, so a wrapper using `exec -a` or a shell that rewrote `argv[0]` would otherwise make the restart exec an unrelated program with keepkit's argv and environment. `executable` is the fallback for both branches and only when it `exists`; nothing left → error → `restartHint`, no exec. **Known asymmetry**: `updater.Detect` resolves the update *target* from `exec.LookPath(t.Name)`, so a release binary launched by path (`./keepkit`) while another keepkit sits on `PATH` updates the `PATH` one and then re-execs the by-path (un-updated) binary — the banner comes back. Fixing it means changing where `Detect` starts, which is shared with `[u]` for every tool and out of this feature's scope; README lists it as a caveat. -- **Panel `[3]` modes (`helpMode`)**: three sources — `helpModeReadme = 2` (the **default** set in `New()`), `helpModeHelp = 0`, `helpModeMan = 1`. `helpMode` is a sticky global field, not per tool: `[r]` (only in `focusHelp` — `case "r"` branches on focus: `focusTools` rename, `focusBrief` refresh), `[h]` and `[m]` (both fire from `focusBrief || focusHelp`) switch it. All three go through the shared **`switchHelpMode(mode)`** (model.go), which sets the mode, dismisses a *completed* update log (a tool's via `updateLogFor`, keepkit's own via `dismissSelfLog()` — the latter ahead of the `selectedMeta` guard), calls `setHelpContent()` + `GotoTop()` and returns the fetch command for the mode's missing source (README via `needsReadme`, `--help`/`man` via the `helpCache` miss + `helpLoadingFor`); focus stays with the caller, because `[h]`/`[m]` also fire from `[2]` and move it while `[r]` is `focusHelp`-only. A **live** update log keeps `[3]` in every path (the log branch sits ahead of the readme branch *and* the `No tool selected` guard in `renderHelpContent`, `setHelpContent` gates on `showsUpdateLog()`, and the readme case in `autoFetchCmdsForSelected` sits after that same case). **`m.helpCache` is a `map[string][2]string` whose values are indexed by `helpMode`, so mode 2 panics on every index site** — README content lives in `m.readmeData` instead and each index site (`rawHelpText`, `renderHelpContent`, `autoFetchCmdsForSelected`) carries a readme early-return/branch *before* the array read; `helpOutputMsg` indexes `msg.mode`, which only ever carries help/man. Rendering: `renderReadme` (readme.go) sanitizes with `cleanTerminalOutput`, then runs glamour with a **fixed** `WithStandardStyle("dark"|"light")` — resolved once at construction into `m.darkBG` via lipgloss's cached `HasDarkBackground()`, because `glamour.WithAutoStyle()` probes the terminal with a termenv OSC query that reads stdin and races Bubble Tea's input reader — plus `WithWordWrap(helpWrapWidth())` and `WithColorProfile(lipgloss.ColorProfile())` (glamour hardcodes TrueColor and would ignore `NO_COLOR`/dumb terms); a glamour failure falls back to the sanitized plain text, never an empty panel. The result is memoized in the single-entry `readmeRenderCache` keyed by `(name, raw, width, dark)` — `setHelpContent` runs on every selection move and width resize and must not re-parse a large README each time. `parseHelpEntries`/`colorizeHelp` are **not** run in readme mode (glamour output is already ANSI): `helpEntries` stays empty so `j`/`k` are plain scroll, and `/` (help search) is a no-op — guarded at the shared `focusBrief || focusHelp` entry point, so the `[1]` tool-list search is unaffected; the `focusHelp` status bar drops the `[/] search` hint in readme mode rather than advertising a dead key. Placeholders are tool-named (`readmeContent` in render.go): no GitHub ref → `No repo for . Press [h] for --help.`, in-flight → `Loading...`, `ErrNoReadme` → `No README in . Press [h] for --help.`, `ErrRateLimited` → `rate limited — press [L]`, and a generic fallback `No README for . Press [h] for --help.` for everything else — a network/5xx failure **and** the case where content exists but glamour rendered it to nothing (a badges/HTML-comment-only README). That last one is why the "real content" test is `data.content != "" && m.helpBase != ""`: returning an empty render as content paints a blank panel with no way out. +- **Panel `[3]` modes (`helpMode`)**: three sources — `helpModeReadme = 2` (the **default** set in `New()`), `helpModeHelp = 0`, `helpModeMan = 1`. `helpMode` is a sticky global field, not per tool: `[r]` (only in `focusHelp` — `case "r"` branches on focus: `focusTools` rename, `focusBrief` refresh), `[h]` and `[m]` (both fire from `focusBrief || focusHelp`) switch it. All three go through the shared **`switchHelpMode(mode)`** (model.go), which sets the mode, dismisses a *completed* update log (a tool's via `updateLogFor`, keepkit's own via `dismissSelfLog()` — the latter ahead of the `selectedMeta` guard), calls `setHelpContent()` + `GotoTop()` and returns the fetch command for the mode's missing source (README via `needsReadme`, `--help`/`man` via the `helpCache` miss + `helpLoadingFor`); focus stays with the caller, because `[h]`/`[m]` also fire from `[2]` and move it while `[r]` is `focusHelp`-only. A **live** update log keeps `[3]` in every path (the log branch sits ahead of the readme branch *and* the `No tool selected` guard in `renderHelpContent`, `setHelpContent` gates on `showsUpdateLog()`, and the readme case in `autoFetchCmdsForSelected` sits after that same case). **`m.helpCache` is a `map[string][2]string` whose values are indexed by `helpMode`, so mode 2 panics on every index site** — README content lives in `m.readmeData` instead and each index site (`rawHelpText`, `renderHelpContent`, `autoFetchCmdsForSelected`) carries a readme early-return/branch *before* the array read; `helpOutputMsg` indexes `msg.mode`, which only ever carries help/man. Rendering: `renderReadme` (readme.go) is a three-stage pipeline — `cleanTerminalOutput` (the same sanitizer every probe capture gets) → **`cleanReadmeMarkdown`** (readme_clean.go, the house-style preprocessor) → glamour with **`WithStyles(keepkitStyle(m.darkBG))`** (readme_style.go), `WithWordWrap(helpWrapWidth())`, `WithColorProfile(lipgloss.ColorProfile())` (glamour hardcodes TrueColor and would ignore `NO_COLOR`/dumb terms) and `WithInlineTableLinks(true)` (a table link in its cell, not a numbered footnote under the table). Dark/light is still resolved **once at construction** into `m.darkBG` via lipgloss's cached `HasDarkBackground()`, because `glamour.WithAutoStyle()` probes the terminal with a termenv OSC query that reads stdin and races Bubble Tea's input reader; what changed is that the answer now picks a *StyleConfig* rather than a style *name*, so `readmeStyleName` is gone and `renderReadme` branches on `testReadmeStyle != ""` (→ `WithStandardStyle`) directly — the seam's only job is to make the constructor fail. A glamour failure falls back to the **preprocessed** text (not the merely sanitized text), or a failed render would be the one path that still shows badge and href noise; never an empty panel. An input that cleans down to whitespace returns `""` early, which is what routes a badge-only README to the placeholder below. + - **The preprocessor (`cleanReadmeMarkdown`)** exists because a README is written for a browser: badges, logos, `` wrappers, hrefs nobody can click in a TTY, emoji a terminal font renders as tofu. It is pure and rests on one inviolable rule — **code is never rewritten**, since a fenced block and an inline span are exactly how a README *shows* the markup these rules delete. So `rcSegments` splits the input into protected fenced blocks (opener = 3+ backticks or tildes at any indent — CommonMark's 3-space limit only ever mis-read a fence nested under a list item; closer = the **same character**, a run **at least as long**, nothing but whitespace after, which keeps a ```` ``` ````-wrapping fence one block; an **unterminated fence protects to EOF**, because `version.getReadme` truncates at `readmeMaxBytes` and the cut can land mid-fence) and the cleanable runs between them, and inline spans are **masked** inside a cleanable segment (`rcMaskSpans`, a NUL-bracketed placeholder — NUL is the sentinel precisely because `cleanTerminalOutput` ran first and drops every control character). Masking rather than segmenting is deliberate: the block-level rules (a multi-line HTML comment, a `` body) need the segment to stay one string, and a span that *contains* `` would gate the unwrapping and eat the very brackets the gate protects. Standalone `[label]: url` definition lines are dropped as the pure metadata they now are, under **two** guards, because deleting a line of someone's README is the most destructive thing this pass does: the destination must be a single token (or ``) with at most a quoted title, and the line **may not interrupt a paragraph** (CommonMark forbids that anyway) — without them `[1]: first item explained` and a `[note]: this matters` sitting mid-paragraph both silently vanished. *HTML*: comments whole; ``/`