test(cmd): enforce that every PR comment command has a CLI equivalent

[aparajon]

Why this matters

The PR comment surface and the CLI converge on the same service methods and neither is privileged: every command a comment accepts is meant to be runnable from the CLI, which is what makes the CLI a usable path when GitHub is unavailable. That parity has been a convention rather than a guarantee, so a new comment command could ship with no CLI counterpart and nothing would notice. A fallback that covers most of the surface is not a fallback.

What it does

A completeness test compares the registered comment commands against the commands the CLI actually exposes, and fails when one has no counterpart. pkg/cmd is package main, so nothing can import its CLI struct and the test has to live beside it; the comment side gains a small exported CommandNames() accessor over the command registry.

comment command registry ──► webhook.CommandNames()
                               │
                               ├─ name present on the CLI      → parity holds
                               ├─ name in the exemption table  → covered by a differently spelled CLI capability
                               └─ otherwise                    → fails, naming the command with no CLI equivalent

CLI names come from kong's own model rather than a re-derivation from struct tags, so the set under test is exactly the words the binary accepts.

Parity is of capability, not of spelling, so three commands are exempt with their reason recorded beside them:

• help is rendered by kong for the whole CLI, so there is no command field to match.
• apply-confirm exists because a comment has no prompt to answer; on the CLI the same two-step consent is apply's confirmation prompt or its --auto-approve flag.
• rollback-confirm is that same consent on rollback.

Two properties worth calling out:

• The exemption table cannot rot. Every entry must still name a registered comment command, so removing a command cannot leave behind a stale exemption that would silently excuse the name if it were later reused for something else.
• The check is one-directional. The CLI is deliberately a superset (status, logs, storage, serve, and others), and none of that is expected to be reachable from a comment.

How it moves us toward the northstar

Adopters do not all drive SchemaBot from GitHub, and running it entirely from the CLI only works if the CLI covers the whole command surface. This makes that coverage something a contributor cannot regress by accident, rather than something a reviewer has to remember to check.

Opened by Claude (claude-opus-5).

[Copilot]

🟢 Approval recommended

The changes are small, well-scoped, and add a clear structural test that enforces the stated invariant without altering runtime behavior.

Pull request overview

This PR enforces invariant AV-10 (“anything the PR can do, the CLI can do”) by adding a test that ensures every PR comment command has a corresponding top-level CLI command name, or is explicitly exempted with a documented capability-equivalence reason.

Changes:

• Added an exported webhook.CommandNames() accessor to list all registered PR comment command words.
• Added pkg/cmd tests that compare PR comment command names against kong’s modeled CLI command names, with a reasoned exemption table and a “no stale exemptions” guard.

File summaries

File	Description
pkg/webhook/commands.go	Exposes registered PR comment command names via CommandNames() for cross-surface parity testing.
pkg/cmd/command_parity_test.go	Adds tests enforcing PR→CLI command parity (with justified exemptions) and preventing exemption-table drift.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[morgo]

🤖 Approving on Morgan's behalf (automated review).

+115/-0, CI green. Test-only apart from a 10-line exported accessor that copies names out of the existing registry into a fresh slice — no new state, nothing mutable escaping.

I checked the parity it asserts rather than trusting that green CI means the lists agree. Fifteen registered comment commands, three exempt, so twelve need a CLI counterpart: plan, apply, unlock, fix-lint, stop, cancel, start, release, revert, skip-revert, cutover, rollback. All twelve are on the CLI struct.

Taking names from kong's model instead of the struct tags is the detail that makes the test work at all, and it's easy to read past. Two of those twelve — FixLint and SkipRevert — only match because of explicit name:"fix-lint" / name:"skip-revert" tags. A version that derived names from the Go field names would compare FixLint against fix-lint and fail on commands that are in fact at parity, and the natural fix for that false failure would have been an exemption entry, quietly hollowing out the test for the two commands most likely to need it. Asking kong what words the binary accepts avoids that whole class.

The vacuity guards are present, which is the usual way a completeness test rots into a no-op: both sides require.NotEmpty before the comparison, so a registry that stops populating fails loudly instead of trivially passing.

The exemption freshness test is the right companion. An exemption is a name, and names get reused — a stale entry for a removed command would silently excuse a future, unrelated command that happened to reuse the word. Requiring every exemption to still name a registered command closes that, and requiring a non-empty reason keeps the table from degrading into a bare allowlist. The three current entries are each a real spelling difference rather than a missing capability: help is kong's own, and the two -confirm commands exist because a comment has no prompt to answer.

Finding — worth one line in the test comment, not a change: this asserts a name exists, not that it does the same thing. A CLI apply that diverged in behavior from comment apply passes unchanged. That's inherent to a parity-by-name check and the PR is honest that parity is "of capability, not of spelling," but the test can only enforce the spelling half; the capability half stays a convention. Knowing which half is mechanized matters to whoever trusts it later.

The related granularity point resolves fine: Model.Children is top-level only, so a future comment command answered by a CLI subcommand (checks …, storage …) would fail until someone adds an exemption naming it — which is the exemption table doing its job, not a defect.

Low risk, and it converts a convention into a build failure. Not blocking.

[aparajon]

🤖 Took the finding as written — one line in the test comment, no change to what it asserts.

The doc comment on TestEveryPRCommandHasACLIEquivalent now says that only the spelling half of the parity is mechanized: the test asserts a CLI command exists to answer each comment command, not that the two do the same thing, so a CLI apply whose behavior drifted from comment apply still passes and the capability half stays a convention. It goes there rather than on the exemption table because that is where someone reads the test and decides how much to trust it, and a green run does not reveal which half it covers.

Also checked the reading behind your point about kong's model, since it is the load-bearing detail: SkipRevert and FixLint do each carry an explicit name: tag (main.go:44 and :53), so deriving names from the Go field names would have compared FixLint against fix-lint and failed on two commands that are at parity — and the natural fix would have been the exemption entry that hollows the test out. Left as is.

Nothing else changed; both parity tests still pass.