PRDCT-556: CLI command-reference sync workflow + docs freshness gate#1037
Conversation
Docs-repo side of the CLI<->docs zero-drift loop (option b per connection-docs#1015 discussion): commit a synced copy of the CLI's generated command reference, gate PR builds on it. - scripts/check-cli-reference.mjs: validates every backticked/fenced `kbagent ...` invocation repo-wide (command exists, flags exist, required flags present in fenced recipes) plus the curated cli/commands.md group bullets and global-flags table; skip-next escape hatch; nearest-match suggestions; exit 2 on unparseable reference so a format change can't pass an empty gate. - _data/cli/command-reference.md: bootstrap copy (v0.72.0, verbatim). - sync-cli-reference.yml: daily fetch of the latest release asset, auto-PR via create-pull-request with the freshness report in the body (bot PRs don't trigger branch.yml with the default token). - branch.yml: npm run check:cli before the build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
padak
left a comment
There was a problem hiding this comment.
Review — the loop closes correctly; one demonstrated latent false-positive, two minor notes
This is the right shape for option (b), and I verified it end-to-end rather than trusting the description. Independently reproduced:
- Gate clean on the real subject. Ran
check-cli-reference.mjsagainst #1015's 7 pages with the bundled v0.72.0 reference → 0 findings, exactly as claimed (238 commands). - Bootstrap is genuinely byte-identical to
releases/latest/download/command-reference.md(diff empty) — good discipline. - Real drift is caught. Negative probes: bogus flag → named
file:line+ suggestion; bogus command → caught;config detailwith no--component-id→missing required(and correctly doesn't flag--project, which the reference marks optional). - exit-2 guard works — a truncated reference exits 2 with the "refusing to run an empty gate" message.
- Nice side-validation of the whole loop: the reference already carries the
--allow-env-manage-token"since 0.29.0" fix I made in keboola/cli#500 — a correction propagated CLI → asset → docs with no hand-editing, which is the entire point.
🟡 1 — takesValue silently depends on the <…> metavar form; a CLI Typer bump would turn valid docs red
parseOptionRow decides an option takes a value via takesValue = spans.some(s => /^<.*>$/.test(s)) — i.e. it only recognizes <str> / <int>. That holds for v0.72.0, but it's an undocumented coupling to a format that already drifted once: at v0.70.1 the generator emitted bare TEXT / INTEGER (Click's native make_metavar), and #500's own local output in the PR thread showed `TEXT`. A Typer/Click version bump on the CLI side reverting to that form would silently break takesValue — and the exit-2 guard would not catch it, because commands still parse fine.
Demonstrated, not theoretical — I rewrote <str>→TEXT in the reference and reran the same valid invocation:
# docs: kbagent --config-dir /home/me/.kbagent project list (valid)
fp.md:8 kbagent --config-dir /home/me/.kbagent project list
✗ unknown command `kbagent /home/me/.kbagent`
The unconsumed value gets misparsed as the command path → false-positive build failure on correct docs. Two-sided fix:
- Docs side (cheap, mergeable now): accept any trailing metavar span, not just angle-bracketed —
const takesValue = spans.some(s => /^<.*>$/.test(s) || /^[A-Z][A-Z0-9_]*$/.test(s) || s.startsWith('['));
(covers<str>,TEXT, and choice[a|b]forms). - CLI side (root cause — I own the generator): I'll pin the metavar format as a stated contract with a test in keboola/cli so it can't drift under a dependency bump. Filing that now.
Not blocking merge — the current asset works — but I'd take the one-line checker hardening here so a future release can't red-fail valid content.
🟢 2 — chained-command gap is real in practice, not just theoretical
Confirmed the documented "one invocation per line" limit: kbagent A && kbagent B scans only A, so a bogus flag on B slips through. Docs do chain kbagent commands occasionally, so this is a live (small) hole, not a corner case. If you want to close it later: split each fence line on top-level && / ; / | and scan each kbagent-leading segment. Optional — the header already discloses it honestly.
🟢 3 — sync workflow conflates exit-2 with exit-1 in the PR body
Freshness report treats any non-zero as status = "FINDINGS — docs mention commands/flags the CLI no longer has". But exit 2 = the reference format broke and the gate couldn't run — a different failure with a different fix. On a format regression the auto-PR would mislabel it as stale-docs. One-line triage improvement: branch on $? (1 vs 2) and set a distinct status for 2.
Verdict
Approving on the strength of the verification — the gate is accurate (0 FP on valid input, catches genuine drift) and the bootstrap/asset chain is sound. #1 is a latent-not-live false-positive I'd still fix here with the one-liner; I'm taking the durable metavar-contract fix on the CLI side. #2/#3 are optional follow-ups.
|
Filed the CLI-side durable fix as keboola/cli#513 (pin the metavar format as a stated contract + a test that fails on a Typer/Click bump), so the coupling gets hardened from the source. The one-line checker mitigation in finding #1 is still worth taking here — it makes this repo robust regardless of when #513 lands. |
padak
left a comment
There was a problem hiding this comment.
Approving — verified end-to-end (details in my review comment above): gate is clean on #1015's 7 pages, catches real drift, and the bootstrap copy is byte-identical to the v0.72.0 release asset. The one 🟡 (metavar-format coupling) is a latent-not-live false-positive; I'd still take the one-line takesValue hardening here, and the durable source-side fix is tracked in keboola/cli#513. 🟢 #2/#3 are optional follow-ups. Nice execution — this closes the CLI↔docs zero-drift loop.
…rkflow (review findings) - accept bare TEXT/INTEGER and [a|b] choice metavar forms so a Typer/Click bump on the CLI side can't turn valid docs red (padak's finding #1; durable generator-side contract tracked in keboola/cli#513) - sync workflow: exit 2 (gate could not run) now reports a distinct GATE ERROR status instead of being mislabeled as stale-docs FINDINGS Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 4/5) · profile docs
PR exceeds size cap (3336 source LOC across 6 source files; 0 test/fixture LOC across 0 files excluded from the cap).
Docs-repo side of the CLI↔docs zero-drift loop (PRDCT-556, child of PRDCT-555/470) — option (b) from the #1015 discussion, per @padak's recommendation: the curated CLI pages stay hand-written; the CLI's generated command reference becomes a CI gate.
What's in here
scripts/check-cli-reference.mjs— the freshness gate. Validates, repo-wide, every backticked/fencedkbagent …invocation against_data/cli/command-reference.md: the command must exist, every flag must exist for that command (or be a global option), and fenced recipes must include allRequired=yesoptions and positionals — the automated version of the source-level review Padak did by hand on PRDCT-490: new top-level CLI section — kbagent #1015. Pluscli/commands.md-specific checks: the curated group bullets (with compressed tokens likedescription-get/setexpanded) and the global-flags table. Findings name file, line, and the offending token, with a nearest-match suggestion.<!-- kbagent-check: skip-next -->suppresses a deliberate exception. Exits 2 (not 0!) if the reference is missing or unparseable, so an upstream format change can't silently pass an empty gate._data/cli/command-reference.md— bootstrap copy of the current latest asset (v0.72.0, byte-identical to the release asset; verified). Auto-synced from here on — don't hand-edit..github/workflows/sync-cli-reference.yml— daily (05:30 UTC) + manual: fetchesreleases/latest/download/command-reference.md, and when it changed opens an auto-PR (title carries the CLI version; body embeds the freshness report). The PR diff reads as the CLI changelog between releases. Known limitation, documented in the header: bot PRs made with the defaultGITHUB_TOKENdon't triggerbranch.yml, so the workflow runs the checker itself and puts the report in the PR body; adding a PAT later upgrades bot PRs to full CI.branch.yml—npm run check:clion every PR, before the build (fail-fast).Sequencing with #1015
maincurrently has nokbagentinvocations, so the gate passes trivially here and this PR is mergeable independently. The real subject — the CLI section from #1015 — was verified against this gate locally: 0 findings across all 7 pages (238 commands, v0.72.0). Once both are merged, any CLI rename/removal turns the docs PR build red within a day of the release.Verified
...) and placeholder (<group>) invocations correctly skipped; env-prefixes,\continuations,$console prompts, quoted args handlednpm run buildclean (258 pages)metadata-*wildcard check, Node pinned in the sync workflow)🤖 Generated with Claude Code