Skip to content

fix(testkit): honor CLI-level --json in argv and run prompt validate - #53

Merged
kjanat merged 1 commit into
masterfrom
fix/testkit-root-flags-and-validate
Jul 1, 2026
Merged

fix(testkit): honor CLI-level --json in argv and run prompt validate#53
kjanat merged 1 commit into
masterfrom
fix/testkit-root-flags-and-validate

Conversation

@kjanat

@kjanat kjanat commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Fixes two testkit gaps where runCommand diverges from the real CLI root.

#33runCommand rejects CLI-level --json in argv

runCommand executed commands in isolation with no root-flag layer, so a
--json in argv was parsed against the command's own schema and rejected as
Unknown flag --json (exit 2). JSON mode was only reachable via the separate
{ jsonMode: true } option, so copying a real mycli --json … invocation into
a test failed verbatim.

runCommand now mirrors CLIBuilder.execute():

  • detect --json before the -- separator (includesBeforeSeparator),
  • enable JSON mode, and
  • strip it before parse so the command schema never sees it.

A literal --json after -- still reaches the command unchanged (#28 parity).
The existing { jsonMode: true } option keeps working — either source enables
JSON mode. The separator-aware strip is extracted as stripBeforeSeparator
(sibling of includesBeforeSeparator) and the planner now reuses it instead of
inlining the same logic.

#36createTestPrompter bypasses prompt validate

createTestPrompter's promptOne dropped the prompt config, so an input
prompt's validate was dead code from the testkit's perspective: an invalid
scripted answer was injected verbatim as the resolved value, and prompt
validation could not be integration-tested.

promptOne now receives config and runs config.validate on string answers
to input prompts, mirroring the terminal prompter. A failing answer is
rejected as a cancellation ({ answered: false }) — exactly as the terminal
prompter behaves when it exhausts its retries — so the resolution pipeline
surfaces it as a required-flag error or default fallback instead of accepting
the invalid value. Non-string answers stay verbatim so downstream coercion
paths remain testable.

Tests / docs

  • testkit-json.test.ts: argv --json enables JSON mode, isn't rejected as
    unknown, redirects log() to stderr, composes with the option, renders
    errors as JSON, and a post--- literal --json reaches the command.
  • testkit-prompt.test.ts: a queued answer passing validate resolves; one
    failing validate is rejected (required → exit 2) or falls back to default.
  • Updated createTestPrompter/TestAnswer TSDoc and prompt/AGENTS.md for the
    new validate contract.

Full suite green: 2557 tests across 76 files; typecheck, lint, and format all
pass.

Closes #33
Closes #36

@kjanat kjanat added kind: bug Broken behavior or regression area: testkit In-process test harness and test utilities cr:review Allow CodeRabbit review labels Jun 30, 2026
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • cr:review
🚫 Excluded labels (none allowed) (2)
  • wip
  • cr:skip

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 232a3bbc-f80f-4840-a6c3-1a67ba45f000

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR fixes two testkit behaviours. First, runCommand() now recognizes a CLI-level --json flag appearing in argv before the -- separator, stripping it via a new shared stripBeforeSeparator parse utility (also adopted by the planner) and enabling JSON mode accordingly, instead of failing with an unknown-flag error. Second, createTestPrompter() now runs a prompt's validate function against queued input answers, treating validation failures as cancellation rather than resolving with the raw invalid value. Documentation, changelog, and tests were updated accordingly.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test as Test code
  participant runCommand
  participant Parse as stripBeforeSeparator
  participant executeCommand

  Test->>runCommand: runCommand(cmd, argv, options)
  runCommand->>Parse: detect & strip pre-separator --json
  Parse-->>runCommand: effectiveArgv, jsonMode
  runCommand->>executeCommand: executeCommand(cmd, effectiveArgv, effectiveOptions)
  executeCommand-->>runCommand: result
  runCommand-->>Test: result
Loading
sequenceDiagram
  participant Test as Test code
  participant TestPrompter as createTestPrompter
  participant Validate as config.validate

  Test->>TestPrompter: promptOne(config)
  TestPrompter->>Validate: validate(queuedAnswer)
  Validate-->>TestPrompter: true / false
  TestPrompter-->>Test: { answered: true, value } or { answered: false }
Loading

Possibly related issues

  • KAJ-256: Fixes runCommand rejecting CLI-level --json in argv, matching this ticket's "Unknown flag --json" bug report.
  • KAJ-259: Fixes createTestPrompter bypassing prompt validate, matching this ticket's request for integration-testable validation.

Possibly related PRs

  • kjanat/dreamcli#31: Both PRs make root --json handling separator-aware, sharing includesBeforeSeparator/stripBeforeSeparator logic across planner and testkit.

Suggested labels: area: cli

Poem

Arrr, the flag --json be tamed at last,
No more "unknown flag" cast!
validate now stands its ground,
No sneaky bad answers slip through, unfound.
Batten the hatches, tests all pass —
A shipshape fix, first class! ⚓🏴‍☠️

🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Semver Version Bump Validation ⚠️ Warning Src code changed, but package.json stayed 3.0.0-rc.1 in base and head, so no SemVer bump was made. Bump the repo version file to the next SemVer patch for this bug-fix PR, e.g. 3.0.1 or the prerelease equivalent.
Agents.Md Documentation Updated ⚠️ Warning PR changed CLI/parse/testkit behaviour, and those dirs each have AGENTS.md, but only src/core/prompt/AGENTS.md was updated. Update AGENTS.md in src/core/cli, src/core/parse, and src/core/testkit to cover the new root-flag and testkit behaviour, or explain an exemption.
✅ Passed checks (9 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects both fixes and includes the required fix prefix.
Description check ✅ Passed The description is directly related to the changeset and matches the implemented fixes.
Linked Issues check ✅ Passed The changes satisfy #33 and #36 by handling CLI-level --json and running prompt validate in the test prompter.
Out of Scope Changes check ✅ Passed The added code, tests, and docs all support the linked fixes; no unrelated scope creep is evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Release Metadata Sync ✅ Passed package.json and deno.json both stay on 3.0.0-rc.1, and CHANGELOG.md’s [Unreleased] / [3.0.0-rc.1] refs point to v3.0.0-rc.1; no partial bump, matey.
Runtime Support Sync ✅ Passed This PR doesn’t change runtime minimum/support claims; the current Node/Bun/Deno versions already match across package.json, src/runtime/support.ts, and docs.
No Generated Docs Artifacts ✅ Passed No edited paths under docs/.vitepress/dist/ or docs/.vitepress/cache/ were found in the commit or tree.
Changelog Update ✅ Passed CHANGELOG.md is updated under [Unreleased] → Fixed with both code changes, and no version bump header is present.
📋 Issue Planner

Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).

View plans for tickets: #33, #36


Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Jun 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

bun add https://pkg.pr.new/@kjanat/dreamcli@53
npm i https://pkg.pr.new/@kjanat/dreamcli@53

commit: 13b75ba

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
dreamcli 374af42 Commit Preview URL

Branch Preview URL
Jun 30 2026, 11:40 AM

@kjanat kjanat changed the title fix(testkit): honor CLI-level --json in argv and run prompt validate fix(testkit): honor CLI-level --json in argv and run prompt validate Jun 30, 2026
@kjanat kjanat self-assigned this Jun 30, 2026
@kjanat

This comment was marked as off-topic.

@coderabbitai

This comment was marked as off-topic.

@coderabbitai coderabbitai Bot added the area: cli CLI dispatch, planning, root UX, or plugins label Jul 1, 2026
@kjanat kjanat removed the cr:review Allow CodeRabbit review label Jul 1, 2026
coderabbitai[bot]

This comment was marked as resolved.

`runCommand` now mirrors the real root: `--json` before `--` enables
JSON mode and is stripped before parse, while a literal `--json` after
`--` still reaches the command. `createTestPrompter` runs the prompt's
`validate` on queued input answers so prompt validation is integration-
testable. Adds the `stripBeforeSeparator` parse helper (used by the
testkit root-flag layer) plus tests, including one locking that a
command's own `--json` flag stays reserved by the root.

The planner-side use of `stripBeforeSeparator` is intentionally dropped:
#54's root-head scan needs the pre-separator `filteredHead` distinct
from the full `filteredArgv`, which the single helper cannot provide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli CLI dispatch, planning, root UX, or plugins area: testkit In-process test harness and test utilities kind: bug Broken behavior or regression

Projects

None yet

1 participant