Skip to content

test(cli): add focused dispatch behavior tests - #1457

Merged
Wibias merged 3 commits into
devfrom
codex/cli-tests
Aug 11, 2026
Merged

Wibias merged 3 commits into
devfrom
codex/cli-tests

Conversation

@Wibias

@Wibias Wibias commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Add focused behavior tests for the CLI dispatch module (final phase of the CLI deepening).

  • Adds tests/cli-dispatch.test.ts covering the pure dispatch contract: DISPATCH_COMMANDS/DISPATCH_ALIASES invariants, alias resolution (setup→init, eject→restore, remove→uninstall, model→models), and dispatchCommand exit-code returns for help forms (0) and unknown commands (1).

Verification

  • tests/cli-dispatch.test.ts — 5 pass.
  • bun run typecheck — exit 0.
  • Full-stack CLI suite — 213 pass; the only 4 failures are the known pre-existing cli-restore-back (2) and POST /api/sync ownership (2) environmental cases.

No GUI changes; no screenshot required.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review notes (stacked PR): stacks on #1456 (codex/cli-help). Does not target dev. Diff is only the new dispatch behavior test (e897d990..f1a22339): 1 file, +50. Merge only after the earlier phases (#1444, #1446, #1451, #1455, #1456) land.

Summary by CodeRabbit

  • Bug Fixes

    • Improved CLI command dispatch so aliases and canonical command names resolve consistently.
    • Rejected invalid inherited property names as commands.
    • Preserved expected behavior for help, empty input, and unknown commands.
  • Tests

    • Added coverage for command dispatch through the CLI registry.
    • Verified alias and canonical command resolution.
    • Added checks for expected exit codes across common CLI inputs.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: eaaabe66-5d03-468b-b86b-e4e1db70c816

📥 Commits

Reviewing files that changed from the base of the PR and between 189db28 and cddd375.

📒 Files selected for processing (2)
  • src/cli/dispatch.ts
  • tests/cli-dispatch.test.ts

📝 Walkthrough

Walkthrough

The PR adds resolveDispatchCommand and updates dispatchCommand to use it. Tests cover canonical commands, aliases, unknown and inherited-property inputs, registry dispatchability, and exit codes.

Changes

CLI dispatch resolution

Layer / File(s) Summary
Registry and alias resolution
src/cli/dispatch.ts, tests/cli-dispatch.test.ts
Adds resolveDispatchCommand for canonical and alias resolution. Tests validate registry commands, alias targets, unknown input, undefined input, and inherited property names.
Dispatch exit codes
tests/cli-dispatch.test.ts
Verifies that help and empty inputs return 0, while unknown and inherited-property commands return 1.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding focused CLI dispatch behavior tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/cli-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

Base automatically changed from codex/cli-help to dev August 11, 2026 10:03
Phase 6 (final) of the CLI deepening: add tests/cli-dispatch.test.ts
covering the pure dispatch contract — DISPATCH_COMMANDS/DISPATCH_ALIASES
invariants, alias resolution (setup/init, eject/restore, remove/uninstall,
model/models), and dispatchCommand exit-code returns for help forms (0)
and unknown commands (1).

Full-stack verification: 213 pass / 4 known pre-existing environmental
failures; typecheck green.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/cli-dispatch.test.ts`:
- Around line 31-36: Extend the alias coverage in the test named “canonical
alias pairs resolve to their command” to exercise dispatchCommand rather than
only inspecting DISPATCH_ALIASES. Stub the relevant runners and verify each of
setup, eject, remove, and model invokes its mapped target runner; alternatively
extract a pure resolver used by dispatchCommand and test all four mappings
through it.
- Around line 39-44: Add a `dispatchCommand` assertion in the “returns 0 for
help forms” test for the `-h` command, using the same help request shape and
expected exit code as the existing `help` and `--help` cases.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d61a459a-0474-42cc-9bfc-3416b4a5a5b4

📥 Commits

Reviewing files that changed from the base of the PR and between 3759786 and 7409e83.

📒 Files selected for processing (1)
  • tests/cli-dispatch.test.ts

Comment thread tests/cli-dispatch.test.ts
Comment thread tests/cli-dispatch.test.ts
Addresses two CodeRabbit findings on #1457:
- extract resolveDispatchCommand (the pure resolver dispatchCommand uses for
  runner selection) and test all four aliases (setup/eject/remove/model)
  plus canonical/unknown/undefined cases at the resolution level, so a
  regression in the alias lookup is caught
- cover the -h help form in the dispatchCommand help-forms test

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/dispatch.ts`:
- Around line 513-516: The resolveDispatchCommand lookup currently accepts
inherited Object properties as valid commands. Replace the `command in
commandRunners` check with `Object.prototype.hasOwnProperty.call(commandRunners,
command)`, preserving alias resolution for valid names, and add regression tests
ensuring `dispatchCommand` returns exit code 1 for `__proto__`, `constructor`,
and `toString`.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cbde8509-8b56-4686-8c93-35818f620029

📥 Commits

Reviewing files that changed from the base of the PR and between 7409e83 and 189db28.

📒 Files selected for processing (2)
  • src/cli/dispatch.ts
  • tests/cli-dispatch.test.ts

Comment thread src/cli/dispatch.ts
Addresses CodeRabbit finding on #1457 (stability): commandRunners is a
normal object, so 'in' accepted inherited names (__proto__, constructor,
toString), reaching a non-callable or inherited function. Use
Object.prototype.hasOwnProperty for the lookup and add regression tests
asserting dispatchCommand returns exit code 1 for those names.
@Wibias
Wibias merged commit bf2a4ee into dev Aug 11, 2026
89 of 99 checks passed
@Wibias
Wibias deleted the codex/cli-tests branch August 11, 2026 10:40
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
* test(cli): add focused dispatch behavior tests

Phase 6 (final) of the CLI deepening: add tests/cli-dispatch.test.ts
covering the pure dispatch contract — DISPATCH_COMMANDS/DISPATCH_ALIASES
invariants, alias resolution (setup/init, eject/restore, remove/uninstall,
model/models), and dispatchCommand exit-code returns for help forms (0)
and unknown commands (1).

Full-stack verification: 213 pass / 4 known pre-existing environmental
failures; typecheck green.

* fix(cli): test alias dispatch resolution and -h help form

Addresses two CodeRabbit findings on lidge-jun#1457:
- extract resolveDispatchCommand (the pure resolver dispatchCommand uses for
  runner selection) and test all four aliases (setup/eject/remove/model)
  plus canonical/unknown/undefined cases at the resolution level, so a
  regression in the alias lookup is caught
- cover the -h help form in the dispatchCommand help-forms test

* fix(cli): own-property check for dispatch command lookup

Addresses CodeRabbit finding on lidge-jun#1457 (stability): commandRunners is a
normal object, so 'in' accepted inherited names (__proto__, constructor,
toString), reaching a non-callable or inherited function. Use
Object.prototype.hasOwnProperty for the lookup and add regression tests
asserting dispatchCommand returns exit code 1 for those names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant