diff --git a/claudedocs/handoff-cli-docs-consolidation.md b/claudedocs/handoff-cli-docs-consolidation.md index e400420..cb9d476 100644 --- a/claudedocs/handoff-cli-docs-consolidation.md +++ b/claudedocs/handoff-cli-docs-consolidation.md @@ -14,23 +14,22 @@ table cells measure 60–2,263 chars (median ~400) against `Short` strings of 24 ## State now -- **Branch / PR:** none open from this session. `civitai/cli` @ `24797c7` (main), - `civitai-developer-docs` @ `308b633` (main). Both clean. - Other sessions have two unrelated PRs open in the docs repo: **#45** - (`.md`/LLM channel App Blocks island payloads β€” see the llms.txt note below) and - **#5** (from July). -- πŸ”΄ **Both repos are SHARED checkouts.** `cli`'s tree shows +- **Branch / PR:** none open from this session. `civitai/cli` @ `aeceb6b` (main), + `civitai-developer-docs` @ `9382513` (main). Both clean. +- πŸ”΄ **Both repos are SHARED and BUSY.** `cli`'s tree shows ` M claudedocs/handoff-app-blocks-hardening.md` β€” **another session's edit.** - Other sessions landed #244–#251 and #254 in `cli` during this work. Always - `git branch --show-current` before any write; work in a worktree. + Other sessions have **#267, #268, #269 open in `cli`** and **#5 in docs**, and + landed #244–#254, #266 in `cli` plus #45/#47 in docs during this work. + Always `git branch --show-current` before any write; work in a worktree. -### DONE β€” 12 PRs merged + 1 issue filed +### DONE β€” 14 PRs merged, 1 issue filed and closed | PR | Repo | What | |---|---|---| | #243 | cli | 8 wrong `AGENTS.md` `item N` cross-refs + `agents_xrefs_test.go` | | #247 | cli | `agents_index_test.go` β€” catches an item the file's own index never names | -| #252 | cli | this handoff | +| #252 / #261 | cli | this handoff (r1, r2) | +| #264 | cli | **fixed the `login --help` NUL byte** β€” closes issue #253 | | #37 | docs | CLI snapshot 30 commits stale; `metrics` missing; reverse assertion; `__complete` enumeration | | #38 | docs | `hostHandlerParity` drift was **semantic**, not cosmetic; `replyNote` split | | #39 | docs | 69 example lines rendered; anchor slugs; heading depth | @@ -40,140 +39,152 @@ table cells measure 60–2,263 chars (median ~400) against `Short` strings of 24 | #44 | docs | took the npm pin-freshness fetch out of the required `test-bridge` gate | | #42 | docs | **CLI reference 19 β†’ 52 commands + global flags** | | #46 | docs | retired the hand-maintained flag tables from `site/guide/cli.md` | -| **issue #253** | cli | the `login --help` NUL byte (below) β€” **filed, unfixed** | - -### Verified live -- Published reference documents `generate`, `workflows`, `download`, `models`, - `app metrics`, and `--spend`/`--budget` (the site previously told authors - `dev:live` could not spend Buzz). -- `civitai-developer-docs` branch protection **created this session** β€” it had - **none** before. Requires `test-cli`, `test-messages`, `test-bridge`, - `typecheck-snippets` (`strict=false`, `enforce_admins=false`, no reviews). +| #48 | docs | re-captured the help snapshot at `civitai/cli@9cfe468` | + +### Verified live (not inferred) +- `https://developer.civitai.com/apps/reference/cli` serves the widened reference β€” + `cli-generate` and `cli-global-flags` both present. `/site/guide/cli` serves **zero** + hand-maintained flag rows. +- **`llms-full.txt` now carries the CLI reference** β€” 15 `civitai generate` mentions, + 11 `--layout`. This was **0/0** before docs #45. The llms channel gap is CLOSED. +- Committed snapshot on docs `main`: **0 NUL bytes**, old sentinel gone, + `Binary version: civitai v0.1.90-25-g9cfe468`, 106 `===CMD` blocks. +- `civitai-developer-docs` branch protection **created this session** β€” it had **none** + before. Requires `test-cli`, `test-messages`, `test-bridge`, `typecheck-snippets`. `cli` protection unchanged (5 contexts). -- `appblocks-drift.yml` scheduled cron **green at 2026-08-07T07:52:57Z** β€” first - green schedule since 07-27; #38 fixed the 10-day red streak. -- Forgejo downgrade (`civitai/civitai#3713`) **rolled out** β€” prod on `5.0.2249`, - 6/6 pods. Released by another session. +- `appblocks-drift.yml` scheduled cron **green at 2026-08-07T07:52:57Z** β€” first green + schedule since 07-27; #38 fixed the 10-day red streak. +- Forgejo downgrade (`civitai/civitai#3713`) **rolled out** β€” prod on `5.0.2249`, 6/6 pods. ## Open investigations β€” live diagnosis state -### `civitai login --help` emits a raw NUL byte β€” filed as #253, still ships -- **Symptom + exact repro:** `civitai login --help > out.txt` yields a file git, - grep and `file(1)` classify as binary. -- **Observed (with values), on a CLEAN build from `origin/main`** (not a dirty tree): - - `internal/cmd/login.go:42` β€” `const tokenFlagNoValue = "\x00civitai-token-no-value"` - - `civitai login --help | tr -dc '\000' | wc -c` β†’ **1** - - `file -b` β†’ **`data`**; `grep -c token` β†’ `binary file matches` - - `od -c` at the break: `e n - n o - v a l u e " ] \0 s t` - - Mechanism: pflag's `FlagUsagesWrapped` builds rows as `\x00` and - **splits on the first `\x00`**. `NoOptDefVal` is interpolated as `[="%s"]`, so - the row holds two NULs, pflag aligns on ours, and its own separator reaches - stdout. A parser reading the row sees the flag as `--token string[="`. -- **Ruled out:** terminal/TTY artifact (reproduces piped to a file); a dirty-tree - artifact (clean build from `origin/main` reproduces it). -- **Status:** repaired **downstream only**, at the docs capture seam (#42). CLI-side - unfixed. Suggested fix in #253: any non-NUL sentinel β€” the only requirement is - that `NoOptDefVal` be non-empty and not collide with a real token β€” plus a - regression test asserting `login --help` contains no `\x00`, since the failure is - invisible in a terminal. - -### `scripts/check-appblocks-pins.mjs:35-38` states a trigger that no longer exists -- **Observed, still present on `origin/main`:** *"It ALSO runs on a PR that touches - package.json so a pin bump is verified against `latest` at review time."* +### The snapshot-freshness guard cannot detect between-release drift +- **Symptom:** `scripts/check-appblocks-cli-snapshot.mjs` verdicts `ok` while the + committed snapshot is materially stale. It could not have flagged docs #48. +- **Observed (measured by driving the guard's own exported helpers):** + ``` + OLD snapshot (12 CLI commits stale) -> tag v0.1.90 | ahead 13 | verdict: ok + NEW snapshot -> tag v0.1.90 | ahead 25 | verdict: ok + ``` + It parses `ahead` and `sha` and then **classifies on `tag` alone.** On a repo where + `main` moves daily and tags are rare, that means it misses *most* drift. +- **Ruled out β€” the obvious fix is wrong:** comparing shas goes red on every upstream + commit (8 of those 12 touched no help text at all) β†’ a permanently-red daily gate, + which this repo's own doctrine forbids. +- **Leading option:** content-hash against a freshly built binary. Needs a Go toolchain + in the drift workflow, a decision on which bytes to hash (the header changes every + commit), a SKIP contract for upstream build failures, and a new remedy string. +- **Cheap middle option (~15 lines):** report "N commits behind `main`" informationally + and never fail. Full reasoning is a review comment on docs PR #48. +- **Next probe:** `node -e` against `check-appblocks-cli-snapshot.mjs`'s exports to + re-confirm the `ahead` value is already available at classification time. + +### `scripts/check-appblocks-pins.mjs:36-39` states a trigger that no longer exists +- **Observed, still present on docs `origin/main`:** *"It ALSO runs on a PR that + touches package.json so a pin bump is verified against `latest` at review time."* - **Why false:** #43 removed the `paths:` filter (so it ran on *every* PR, not only - package.json ones), then #44 removed the PR invocation entirely. `check:pins` is - now **schedule-only** via `appblocks-drift.yml:72-77`. -- **Ruled out:** not the comment #44 *did* fix β€” that was the sibling file + package.json ones), then #44 removed the PR invocation entirely. `check:pins` is now + **schedule-only** via `appblocks-drift.yml:72-77`. +- **Ruled out:** not the comment #44 *did* fix β€” that was the sibling `check-design-system-pins.mjs:172-181`. - **Next probe:** one comment edit pointing at `appblocks-drift.yml`. LOW. ### `check-appblocks-pins.mjs` exits 2 on a SemVer build-metadata `latest` -- **Observed (measured, mechanism corrected):** `parseSemver` **returns `null`**, - it does not throw β€” `parseSemver("0.31.0+build.7")` β†’ `null`, - `parseSemver("0.31.0")` β†’ `{major:0,minor:31,patch:0,pre:null}`. The throw is an - explicit guard in `compareSemver`: `if (!pa || !pb) throw new Error(...)`, whose - doc comment says "Unparseable -> throws" β€” intentional, not an oversight. That +- **Observed (mechanism corrected β€” do not restate the old version):** `parseSemver` + **returns `null`**, it does not throw. Regex unchanged on `main` at `:59`: + `/^(\d+)\.(\d+)\.(\d+)(?:-(.+))?$/`. Measured: + `parseSemver("0.31.0+build.7")` β†’ `null`; `parseSemver("0.31.0")` β†’ `{major:0,…}`. + The throw is an explicit guard in `compareSemver` β€” `if (!pa || !pb) throw` β€” whose + doc comment says "Unparseable -> throws". Intentional, not an oversight. It propagates to the catch at ~`:190` β†’ **exit 2**. -- **Why it matters:** exit 2 is triggerable purely by *someone else's* publish, and - the in-file comment lists the red paths as "lagging / 404-410 / unexpected error" - without saying an upstream publish can cause the third. `X.Y.Z+meta` is valid - SemVer 2.0.0. -- **Next probe:** accept build metadata in `parseSemver` (strip `+…` before compare) - or document it. Argues **for** #44 having removed this from the required gate. LOW. - -### IA wart: a public-API reader is now sent into the Apps section +- **Why it matters:** exit 2 is triggerable purely by *someone else's* publish, and the + in-file comment lists the red paths without saying an upstream publish causes that one. + `X.Y.Z+meta` is valid SemVer 2.0.0. +- **Next probe:** strip `+…` before compare, or document it. LOW. + +### IA: the reference page is under `/apps/` and its body still presumes App authoring - **Symptom:** `site/guide/cli.md` (public-API guide β€” searching/downloading models) - now links flag reference to `apps/reference/cli.md`, under `/apps/` (App - *authoring*). That page's intro greets the reader with "the canonical tool for - authoring Civitai Apps" before mentioning downloads. -- **Why it exists:** #46 deleted the guide's hand-maintained flag tables; the - generated reference is the only remaining copy, and it lives under `/apps/`. -- **Options written up, none chosen:** (a) alias or render `` under - `/site/` too; (b) move the generated reference somewhere audience-neutral; - (c) minimum β€” reword the reference page's intro so it does not presume authoring. -- **Note:** docs PR **#45** (another session, open) touches the `.md`/LLM channel and - may interact β€” the generated reference does **not** reach `llms-full.txt` because - `apps/reference/cli.md` is a bare `` tag (INFERRED from how - `vitepress-plugin-llms` operates; confirm by building and reading the artifact). + links flag reference to `apps/reference/cli.md`, under `/apps/` (App *authoring*). +- **Observed β€” narrower than previously recorded.** The **frontmatter** was fixed by + #42 and is now audience-neutral: *"The whole civitai CLI command tree β€” commands, + flags, examples and the global flags…"*. The **body** was not + (`apps/reference/cli.md:23-24`): *"is the canonical tool for **authoring Civitai + Apps**"*, and `:36` *"App authors usually do"*. +- **Ruled out:** the llms-channel concern that used to ride along here β€” docs #45 + closed it (measured above). +- **Options, none chosen:** (a) reword the body's opening two sentences β€” cheapest, and + now the *only* thing wrong with the framing; (b) alias/render `` under + `/site/` too; (c) move the reference somewhere audience-neutral. +- **Next probe:** `git -C show origin/main:apps/reference/cli.md | sed -n '21,40p'` ## Next steps (ranked) -1. **Migrate README command-reference prose into `Long` / `Annotations`** β€” the - actual answer to this session's question, and the only path that converges the - two surfaces without deleting the richer one. gh's - `Annotations["help:json-fields"]` is the working precedent for publishing - `--json` shapes from the command definition. -2. **Decide the IA wart** (above) β€” cheapest is option (c). -3. **Fix #253** in `civitai/cli` β€” one-line sentinel change + a regression test. -4. The two LOW items above (pins header comment; `parseSemver` build metadata). -5. **Prune `claudedocs/`** β€” dated handoffs that belong in neither the repo nor the - site. Coordinate: other sessions actively write here. +1. **Migrate README command-reference prose into `Long` / `Annotations`** β€” the actual + answer to this session's question. + πŸ”΄ **It is not a straight move, and the handoff previously under-stated this.** + `Long` renders in `--help`; README reference cells run to **2,263 chars**. Dumping + them into `Long` fixes the docs surface by wrecking the CLI's own. Decide the split + FIRST β€” terminal-appropriate β†’ `Long`; structured docs-only (the `--json` shapes, + gh's `Annotations["help:json-fields"]` precedent) β†’ `Annotations`, invisible to + `--help`; narrative/tutorials β†’ hosted pages, never generated. Then **pilot one + command group** (`download` or the read commands) and measure `--help` before/after + before touching ~50 files. +2. **Reword `apps/reference/cli.md:23-24`** β€” option (a) above, now the only remaining + IA defect and a two-sentence change. +3. **Decide the freshness-guard gap** β€” the cheap informational option, or commit to + content-hashing. +4. The two LOW items in `check-appblocks-pins.mjs` (stale header; `parseSemver`). +5. **Prune `claudedocs/`** last β€” other sessions actively write here; coordinate. ## Gotchas / decisions / dead-ends -- πŸ”΄ **The `cli.json` export command is DISPROVED β€” do not re-propose it.** The - widening it was meant to enable works with the existing scraper (53 nodes, 0 - enumeration disagreements, 0.79s). `Long` recovers **54/54 byte-exact**; defaults - **0 mismatches over 180 flags** β€” and an export would be *worse* there, because - **24 flags carry their real effective default in authored prose that `DefValue` - does not** (`--dir` β†’ `./` vs `DefValue=""`). Cobra v1.8.1 β†’ v1.10.2 help - output: **0 byte-differences across 53 nodes**. `Annotations`/`GroupID`/ - `Deprecated` counts are **0/0/0**. A `go:generate` program *can* import - `internal/cmd` (negative control: an external module cannot), so the - published-binary-surface question was moot. -- πŸ”΄ **`paths:` filters + required contexts = deadlock.** A required check whose - workflow never triggers blocks a PR forever at `MERGEABLE/BLOCKED`. Self-inflicted - here (protection added before checking the workflows always run); it blocked #42. - Fixed in #43 by removing the filters. **Rejected**: the common "dummy job that - always succeeds with the same name" pattern β€” it makes a required check report - green without running anything. +- πŸ”΄ **The `cli.json` export command is DISPROVED β€” do not re-propose it.** The widening + it was meant to enable works with the existing scraper (53 nodes, 0 enumeration + disagreements, 0.79s). `Long` recovers **54/54 byte-exact**; defaults **0 mismatches + over 180 flags** β€” and an export would be *worse* there, because **24 flags carry + their real effective default in authored prose that `DefValue` does not** (`--dir` β†’ + `./` vs `DefValue=""`). Cobra v1.8.1 β†’ v1.10.2 help output: **0 byte-differences + across 53 nodes**. `Annotations`/`GroupID`/`Deprecated` counts are **0/0/0**. A + `go:generate` program *can* import `internal/cmd` (negative control: an external + module cannot), so the published-binary-surface question was moot. +- πŸ”΄ **`paths:` filters + required contexts = deadlock.** A required check whose workflow + never triggers blocks a PR forever at `MERGEABLE/BLOCKED`. Self-inflicted here + (protection added before checking the workflows always run); it blocked #42. Fixed in + #43. **Rejected**: the common "dummy job that always succeeds with the same name" + pattern β€” it makes a required check report green without running anything. - πŸ”΄ **The generator prefers a live `civitai` on PATH.** `~/.local/bin/civitai` is `v0.1.89-20-g4018e2c` (stale) and silently produced **47 commands instead of 52**, dropping `generate` and `workflows`. #42 made the snapshot the default with - `CIVITAI_CLI_LIVE=1` opt-in **plus** a live-vs-snapshot diff. Also: - `cli/bin/civitai` is rebuilt by other sessions and is often `-dirty` β€” a binary - built from a dirty tree is evidence about that working copy and about no commit. -- **#46's deletion was better-justified than it looked.** The `### Download flags` - table listed **12** flags; `download` has **14**. The two missing were `--version` - and `--yes` β€” *both* about the ambiguous-id safety stop β€” so the rotted rows were - exactly the ones a reader who hits that stop needs. Every one of the 12 was - confirmed present in the generated artifact before deleting. -- **zsh ate a variable:** `"$R:cli"` β†’ the `:c` parsed as a history modifier, - producing `/home/zach/workspace/civit/clili` with no error. Brace it: `${R}`. + `CIVITAI_CLI_LIVE=1` opt-in **plus** a live-vs-snapshot diff. **Always build the + binary from `origin/main` in a detached worktree when re-capturing**, and confirm it + is not `-dirty` β€” a dirty build is evidence about a working copy and about no commit. +- πŸ”΄ **The docs-side NUL repair at the capture seam MUST STAY** even though #264 fixed + the source. The generator can capture from any binary, and `v0.1.90`-era builds still + emit the NUL. Its test now covers a *historical fixture*, not current output β€” the + header in `gen-appblocks-cli.mjs` says so; do not "clean it up". +- **#46's deletion was better-justified than it looked.** The `### Download flags` table + listed **12** flags; `download` has **14**. The two missing were `--version` and + `--yes` β€” *both* about the ambiguous-id safety stop β€” so the rotted rows were exactly + the ones a reader who hits that stop needs. +- **A re-capture is never "just the one row".** Docs #48's diff carried 7 hunks across + three CLI PRs (#242, #251, #264). Attribute every hunk to a commit, and cross-check + the other direction (`git diff .. -- internal/cmd/ ':!*_test.go'`) so a + *missing* change is caught too. +- **zsh ate a variable:** `"$R:cli"` β†’ the `:c` parsed as a history modifier, producing + `/home/zach/workspace/civit/clili` with no error. Brace it: `${R}`. - **Three mutants survived first runs, all the same shape** β€” `assertEnumerationsAgree` - (correct but not wired into `buildArtifact`), the slot assertion (satisfied by a - code *comment*), and "prefer live" (every test drove the pure helper, nothing - reached `resolveBundle`). Component correct, wiring untested. **Mutate the CALL - SITES, not just the function.** -- **A single-line grep over wrapped prose gives a confident wrong answer.** "Item 24 - is unreferenced" was wrong because `and item 24` / `covers the…` wraps across a - newline. `agents_index_test.go` joins the region before matching for this reason. -- **A repeated failure is not a deterministic one.** `api/v1/articles/4797` returned - 500 four times over ~15 min, then recovered; the by-id route 404s correctly for - missing ids and is healthy across the id space. No platform bug β€” do not re-file. -- **Read whole output, not `tail`.** Checking the new flag-table guard with `tail -3` - showed only `apps/reference/cli.md` and read as "it doesn't cover the page it was - built for". Full output shows both pages plus an 11-fixture detector control. + (correct but not wired into `buildArtifact`), the slot assertion (satisfied by a code + *comment*), and "prefer live" (every test drove the pure helper, nothing reached + `resolveBundle`). Component correct, wiring untested. **Mutate the CALL SITES.** +- **A single-line grep over wrapped prose gives a confident wrong answer.** "Item 24 is + unreferenced" was wrong because `and item 24` / `covers the…` wraps across a newline. + `agents_index_test.go` joins the region before matching for this reason. +- **A repeated failure is not a deterministic one.** `api/v1/articles/4797` returned 500 + four times over ~15 min, then recovered; the by-id route 404s correctly for missing ids + and is healthy across the id space. No platform bug β€” do not re-file. +- **Read whole output, not `tail`.** Checking the flag-table guard with `tail -3` showed + only one of the two pages it scans and read as "it doesn't cover the page it was built + for". Full output shows both plus an 11-fixture detector control. ## How to verify @@ -182,16 +193,21 @@ D=/home/zach/workspace/civit/civitai-developer-docs R=/home/zach/workspace/civit/cli git -C ${D} fetch origin -q && git -C ${R} fetch origin -q -# #42 β€” whole tree published, snapshot is the default source -git -C ${D} show origin/main:scripts/gen-appblocks-cli.mjs | command grep -c CIVITAI_CLI_LIVE # expect 3 -git -C ${D} show origin/main:scripts/gen-appblocks-cli.mjs | command grep -c globalOptions # expect >=1 +# #264 β€” the NUL is gone at source, and the guard is in +git -C ${R} show origin/main:internal/cmd/login.go | command grep 'tokenFlagNoValue = ' # "(no value)" +cd ${R} && go test ./internal/cmd/ -run 'TestLoginHelp|TestTokenFlagNoValue' -count=1 + +# #48 β€” the committed snapshot matches, and is plain text +git -C ${D} show origin/main:appblocks-snapshots/civitai-cli-help.txt > /tmp/s.txt +tr -dc '\000' < /tmp/s.txt | wc -c # expect 0 +command grep -c 'civitai-token-no-value' /tmp/s.txt # expect 0 +command grep -c '^===CMD' /tmp/s.txt # expect 106 -# #46 β€” no hand-maintained flag tables anywhere (reads BOTH pages + a detector control) -cd ${D} && node scripts/check-no-hand-flag-tables.mjs # read all output, not the tail +# #46 β€” no hand-maintained flag tables (reads BOTH pages + a detector control) +cd ${D} && node scripts/check-no-hand-flag-tables.mjs # read ALL output, not the tail -# #44 β€” pin freshness is schedule-only -git -C ${D} show origin/main:.github/workflows/appblocks-bridge.yml | command grep -cE '^\s*run:.*check:pins' # expect 0 -git -C ${D} show origin/main:.github/workflows/appblocks-drift.yml | command grep -c 'check:pins' # expect 2 +# #42 / #45 β€” the whole tree is published AND reaches the LLM channel +curl -s https://developer.civitai.com/llms-full.txt | command grep -c 'civitai generate' # expect >0 # #243 / #247 β€” both AGENTS.md guards green cd ${R} && go test ./ -run 'TestEveryAgentsItemIsNamedByTheIndex|TestAgents' @@ -202,7 +218,4 @@ gh api repos/civitai/civitai-developer-docs/branches/main/protection --jq '.requ # the scheduled drift sweep stays green gh run list --repo civitai/civitai-developer-docs --workflow appblocks-drift.yml --limit 3 \ --json conclusion,event,createdAt - -# #253 β€” the NUL bug still ships (expect 1 until it is fixed) -${R}/bin/civitai login --help | tr -dc '\000' | wc -c ```