test(cz-cli): replay real CLI history against the parser

[suibianwanwank]

No description provided.

[github-actions]

HIGH (confidence: high) — a deleted subcommand under a command group replays as green, so the COMMAND_REMOVED verdict is unreachable for the case it was written for.

const capacity = declared.some((p) => p.variadic) ? rest.length : declared.length
const keptTokens = rest.slice(0, capacity)

Walking it through for task list --like x after someone deletes task list:

1. resolveCommand matches task, leaves rest = ["list"].
2. task is declared as cli.command("task", "Manage Studio tasks", …) (src/commands/task.ts:1219) — the declaration string carries no positionals, so parsePositionals("task") is [] and capacity === 0.
3. list lands in droppedTokens; argv becomes ["task", "--like=x"].
4. commandGroup ends in .demandCommand(1, humanMsg) with humanMsg = "Missing subcommand for 'task'. …" (src/command-group.ts:105), whose fail handler throws SubcommandHelpShown (src/command-group.ts:52).
5. replayArgv maps that to SUBCOMMAND_HELP, which is in ACCEPTED (test/history-argv.test.ts:53). Test passes.

The group already has .strictCommands(), so had list reached argv the parser would have said Unknown commands: list and classifyFailure would have produced COMMAND_REMOVED correctly. The token never gets there. The same applies at depth 3 (analytics-agent domain create → analytics-agent domain bare → pass), and queryUndeclaredSubcommands() in the export script documents this exact collapse for a different purpose, so the mechanism is known.

This matters because it is one of the three regressions the suite advertises: "the suite stays green until someone deletes a flag, narrows a choices list, or renames a subcommand" (known-changes.ts:8-10).

The distinction the harness needs is already available: tokens from entry.sub / entry.sub2 were whitelisted against this tree's own commandTokens at export time (cli-surface.ts:14-17), so they were declared commands when the fixture was written — unlike tokens recovered from _positional, which the dropping logic is legitimately there to handle. Suggest threading that provenance through tokensOf/buildArgv and returning COMMAND_REMOVED when a token of sub/sub2 origin fails to resolve, instead of dropping it.

Separately, droppedTokens and resolvedPath are computed and returned but never asserted on and not printed by describeEntry (history-argv.test.ts:63-75) — so today a token vanishing is invisible even in the failure output.

[github-actions]

MEDIUM (confidence: high) — the value-persistence whitelist is keyed on option name unioned across the whole tree, so free-text values from other commands get written verbatim into the committed fixture.

  and array_contains(${sqlArray(CHOICE_KEYS)}, fk)
  and not array_contains(${sqlArray([...SENSITIVE_KEYS])}, lower(fk))
  and fv is not null and length(fv) <= 40 and fv rlike '^[A-Za-z0-9_.:+-]+$'

CHOICE_KEYS is [...surface.choicesByKey.keys()], and choicesByKey unions choices by option key across every node (cli-surface.ts:198-208). Membership on one command therefore admits the same key everywhere. The docblock above this function and the header's redaction contract both state the stronger property:

Flag values are persisted only for options that declare choices (closed, low-cardinality enums) … a closed enum is low-cardinality by construction and cannot carry customer data.

That does not hold as written. type enters CHOICE_KEYS from analytics-agent knowledge (src/commands/analytics-agent.ts:2796, choices: ["text", "dictionary"]), while --type is a plain unconstrained string on datasource list (src/commands/datasource.ts:354) and on task list / task run-stats (src/commands/task.ts:4218, :4374). Both show up in the fixture as a result: datasource list --type lakehouse (matrix.json:78096) and task list --type 29 (matrix.json:79408-79412).

Nothing leaked here — the surviving values are all enum-shaped, and the ≤40 char + ^[A-Za-z0-9_.:+-]+$ shape check is doing real work. But the shape check is the only thing standing between a free-text option and the committed file, and it would happily pass a table name, a workspace name, or a bare identifier. Given the header calls redaction "a hard requirement," the control should match the claim.

The replay side already has the per-path version of this predicate (isDeclaredOption, history-replay.ts:284), and DeclaredOption.choices is per-node — so scoping the whitelist to (command path, key) pairs is expressible with what's already built. As a bonus, history-argv.test.ts:126-131 then stops skipping the triples it can't use, which is currently dead fixture weight.

[github-actions]

MEDIUM (confidence: high) — nothing automated runs this suite, so the "fails with the usage count of what you broke" guarantee only fires if someone runs it by hand.

"test:history": "bun test test/history-argv.test.ts test/history-effect.test.ts",
"test:all": "bun test test/classify-error.test.ts && bun run test:history && …",

Tracing both routes into it:

• CI's unit job runs bun turbo test (.github/workflows/test.yml:68). turbo.json declares no generic test task — only opencode#test, @opencode-ai/core#test, @opencode-ai/app#test, @opencode-ai/ui#test, @opencode-ai/session-ui#test. There is no @clickzetta/cli#test, so this package's "test": "bun test --timeout 30000" is not part of that run, and no other workflow invokes cz-cli tests (only release-cos.yml:285 touches the package, and that is a build).
• test:all reaches it, but its only automated caller is test:ci, which starts with build:local — cp ../opencode/dist/cz-cli-darwin-arm64/… plus codesign. That is a macOS-arm64 developer command, not something CI can run.

The no-CI-for-cz-cli situation is pre-existing, not introduced here. It is worth deciding now, though, because this suite's whole value proposition is being a tripwire, and a tripwire nobody walks through catches nothing. Adding a "@clickzetta/cli#test": {} task to turbo.json would put it in the existing linux/windows unit matrix.

Two things to confirm if you do wire it up, neither of which I can measure without running it: the argv layer generates one test per fixture entry (2,000+) plus one per value case, each constructing a fresh yargs tree via registerCommands(createCli(argv)), and Tier B sets setDefaultTimeout(20_000). The unit job's budget is timeout-minutes: 20 for all packages.

[github-actions]

LOW (confidence: high, currently latent) — fillRequired compares alias spellings against the canonical key only, so an aliased mandatory option would be supplied twice.

const supplied = new Set(flags.flatMap((flag) => spellings(flag.key)))
for (const option of new Set(byName.values())) {
  if (!option.demanded || supplied.has(option.key)) continue
  argv.push(`--${option.key}=${synthesizeValue(option.key, option)}`)
}

supplied holds every spelling of the historical key; the guard tests option.key. If a value case names an option by its alias — say -f for a demandOption: true --file — supplied contains f/f but not file, so the loop pushes --file=<synthesized> alongside the --f=<historical value> built above. Both resolve to the same argv key, the later token wins, and the expectation at line 250 then checks for the historical value that got overwritten — reporting FLAG_NOT_IN_ARGV for an option that parsed fine.

Not reachable today: I found no option in src/commands/ combining alias with demandOption, so this only bites when someone adds one. Resolving through the existing lookup(byName, flag.key) and seeding supplied with the canonical key (and its aliases) closes it.

[github-actions]

LOW (confidence: high) — this points at a file that is not in the repo.

"stream, which the fixture deliberately does not persist, so they replay here without it. The highest-" +
"impact divergence found (135 invocations, 4 users) — see .claude/reports/history-regression.md.",

.claude/reports/ does not exist at the repo root or under packages/cz-cli, and this PR does not add it. The reason field is the audit trail that justifies suppressing a failure — the file header calls adding an entry "a deliberate act" — so for the highest-impact suppression in the list the supporting evidence should be reachable by whoever reads this next. Either commit the report or inline the couple of sentences that matter.

[github-actions]

LOW (confidence: high) — the scratch directory is never removed.

const dir = mkdtempSync(join(tmpdir(), "cz-history-replay-"))
scratchFile = join(dir, "input.sql")
writeFileSync(scratchFile, "select 1\n")

One cz-history-replay-* directory accumulates in os.tmpdir() per test process. test/preload.ts already owns this pattern for the test home — it creates the temp dir and removes it in afterAll — so an afterAll(() => rmSync(dir, { recursive: true, force: true })) here would match the convention.

Same shape in the export script: runSql writes each query to $TMPDIR/cz-history-q<N>.sql (script/export-history-matrix.ts:88) and leaves it behind. Lower stakes since that is a manual command and the contents are the generated query rather than customer data.

[github-actions]

LOW (confidence: high) — positionalCountMin is persisted for every entry but never read.

  positionalCountMin: number

It is declared here and written by the export script (querySignatures, npos_min), but no assertion, verdict, or failure message consumes it — nothing outside this interface references the name. Every other field on MatrixEntry has a consumer: usageCount orders the tests, users/firstSeen/lastSeen/versions all land in describeEntry.

Worth deciding rather than leaving ambiguous, because it is 2,000+ extra lines in an 80,000-line committed fixture. It would be genuinely useful if it were wired up: an entry whose positionalCountMin exceeds what resolveCommand can place is precisely the signal that positionals were dropped, which is the gap I flagged in history-replay.ts around capacity. Otherwise drop the field from the export.

[github-actions]

LOW (confidence: medium) — this tier runs real handlers, and the fixture profile points them at what looks like a real internal endpoint.

"service = 'uat-api.clickzetta.com'",

Safe as written: test/preload.ts installs the fetch boundary before any src/ import, and once a test registers any handler an unmatched request throws rather than falling through (test/support/fetch-boundary.ts:17-19). The catch-all match: () => true in beforeEach covers everything, so nothing escapes. I also confirmed telemetry cannot leak — OTEL_DEFAULTS.endpoint is build-time injected and empty in source, so trackCommand short-circuits (src/telemetry.ts:149).

The concern is only that the safety margin is one beforeEach wide, on the one test file in the suite that executes handlers against the network boundary. If a future edit registers handlers per-test instead of in beforeEach, or an early failure path fires a request before registration, these resolve against a host that may actually answer. A deliberately unroutable value (invalid.example, 127.0.0.1:1) costs nothing here — the assertions are all about request shape — and makes the failure mode a connection refusal instead of a live UAT call. Same for analysis_agent_endpoint, which already uses example.clickzetta.com.

[github-actions]

Review summary

Test-only PR: 8 files, all under packages/cz-cli/ (fixture, two test files, two test/support helpers, one script/, plus two package.json script lines). Read the full files plus src/cli.ts, src/execute.ts, src/command-group.ts, src/telemetry.ts, test/preload.ts, test/support/fetch-boundary.ts, turbo.json and .github/workflows/test.yml. I could not run anything, so nothing below is a claim about pass/fail.

A. Upstream invasiveness — no issues found

No file under packages/opencode, packages/tui, or packages/core is touched, so the banner and ledger obligations do not apply and UPSTREAM-PATCHES.md needs no new INTRUSIVE entry. The cz_change: comments in packages/cz-cli/bunfig.toml are pre-existing and correct for that location.

The approach is notably non-invasive where it could easily have gone the other way: the replay harness reaches the parser through createCli/registerCommands and the existing post-validation middleware seam that src/execute.ts:70-80 already uses for its NO_PROFILE gate, rather than adding a test hook to the CLI. Nothing in src/ changed to make the tests possible.

B. Clean fix, or a hole around the problem — one finding

Mostly the right shape. The surface is read out of yargs own command registry instead of scraped from --help, both consumers share one cli-surface.ts rather than duplicating the walk, and the triage list is checked positively (a known change must fail with the verdict it claims) with blamedNames requiring every blamed token to be accounted for — a deliberate guard against exactly the blanket-amnesty failure mode the file warns about.

The one finding is the token-dropping rule in buildArgv (inline, HIGH): dropping recorded tokens that the resolved declaration cannot hold is correct for tokens recovered from _positional, but it is applied to sub/sub2 too. Those were declared commands when the fixture was generated, so after a subcommand deletion the token is dropped, the bare group throws SubcommandHelpShown, and SUBCOMMAND_HELP is in ACCEPTED — the deletion passes. COMMAND_REMOVED is unreachable for group children as a result, and "renames a subcommand" is one of the three regressions the suite advertises.

Two smaller items in this category: the value whitelist claims a per-option property it enforces per-name across the whole tree (inline, MEDIUM), and positionalCountMin is persisted for every entry but never read (inline, LOW). No dead code, commented-out code, leftover debug logging, or unrelated drive-by edits.

C. Regression risk — low, with one caveat

Nothing in src/ changes, so there is no changed default, renamed flag, altered exported API, on-disk path, or command output shape. No test is deleted, skipped, or loosened — the test.skip calls are new and scoped to fixture rows the harness deliberately excludes. --profile/--format reach the CLI only through execute() in the new Tier B cases, which is an existing entry point with existing callers.

What does change is developer and CI surface:

• test:all now runs test:history, adding 2,000+ generated argv tests plus the value-case layer to test:ci. Its only automated caller is test:ci, which begins with the macOS-arm64-only build:local.
• The package test script globs the whole directory, so bun test in packages/cz-cli picks the new files up automatically — but bun turbo test does not run it, because turbo.json declares no generic test task and no @clickzetta/cli#test. Details and the two runtime numbers to watch if you wire it up are inline (MEDIUM). Pre-existing for this package, not introduced here.

On the safety claim the suite rests on: Tier A really is parse-only. Handlers cannot run because the capture middleware throws first, createCli sets exitProcess(false) (src/cli.ts:90) so a --help replay cannot kill the process, and the only .check() in the command tree (src/commands/analytics-agent.ts:3276) is a pure argv predicate. Tier B does run handlers, but test/preload.ts installs the fetch boundary before any src/ import and redirects HOME/CLICKZETTA_TEST_HOME; telemetry cannot escape either, since OTEL_DEFAULTS.endpoint is build-time injected and empty in source. One narrow note on that margin is inline (LOW).

No new cross-package dependency edge: everything imports within packages/cz-cli. script/export-history-matrix.ts importing ../test/support/cli-surface.js is a new script-to-test-code direction, but it is intentional and documented in that file header, the script is manual-only, and script/build.ts does not bundle it — flagging only so it is a conscious choice.

Remaining LOW items: fillRequired alias handling (latent), dangling .claude/reports/ reference, temp dir not cleaned up.

All of these are suggestions — accept or reject as you see fit.