fix(command): skip parent run() when a sub command has run

[spokodev]

Fixes #253

Problem

runCommand dispatches an explicit (or default) sub command and then falls through to the "Handle main command" block, so the parent's run() executes right after the sub command's:

const main = defineCommand({
  run() { console.log("main"); },
  subCommands: { test: { run() { console.log("test"); } } },
});
runMain(main); // `cli test` prints "test" then "main"

The documented lifecycle is setup() → resolve subcommand or run() → cleanup() (AGENTS.md), and the E_DEFAULT_CONFLICT guard already shows only one action is meant to execute per invocation. The fall-through has been there since the file was created; the existing sub-command test never caught it because its parent command has no run.

Fix

Track whether a sub command was dispatched and skip the parent run() in that case. Parent setup()/cleanup() still wrap the sub command run, and a parent with run and no matching sub command arg behaves as before.

Tests

does not run the parent's run() after a sub command runs fails on main (parent run was called) and passes with this change; a second case pins that the parent's run() still executes when no sub command is given. Full pnpm test: lint 0 warnings / 0 errors, 111 tests passed, types clean.

Summary by CodeRabbit

• Bug Fixes

  • Fixed command execution flow where the parent command was incorrectly running after a subcommand executed
  • Improved conditional handling of the main command path when subcommands are present

• Tests

  • Added test coverage validating correct command execution behavior when subcommands are and aren't provided

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes a bug where parent and subcommand run() functions both executed when a subcommand was invoked. A ranSubCommand flag now tracks subcommand execution and gates the main command logic, ensuring parent and subcommand runs are mutually exclusive.

Changes

Subcommand Execution Isolation

Layer / File(s)	Summary
Subcommand execution gating in runCommand src/command.ts	runCommand introduces a ranSubCommand flag initialized to false. After dispatching an explicit subcommand or default subcommand, the flag is set to true. The main command's run() is now conditionally executed only if ranSubCommand is false, preventing duplicate invocation of parent and subcommand run functions.
Test coverage for parent/subcommand execution isolation test/main.test.ts	Two new Vitest cases validate the fix: one asserts that the parent run() is not called when a subcommand executes, and the other asserts that the parent run() is called when no subcommand is provided. Both use vi.fn() mocks to track invocation counts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• unjs/citty#231: Modifies src/command.ts subcommand dispatch flow; one fixes subcommand index detection, this one gates parent execution based on subcommand invocation.

Suggested reviewers

• pi0

Poem

🐰 A flag springs forth to mend the day,
When subcommands were running in their way,
Now parent and child shall take their turn,
No double dance—just lessons learn!
Hop hop ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: preventing parent run() execution after a subcommand has run, which directly addresses the bug fix.
Linked Issues check	✅ Passed	The PR directly implements the objective from issue #253: preventing parent command's run() from executing after a subcommand is dispatched, while maintaining proper lifecycle semantics.
Out of Scope Changes check	✅ Passed	All changes are focused on the specific bug fix: tracking subcommand execution with a flag and conditionally skipping parent run(), with corresponding tests validating the new behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

test/main.test.ts (1)

145-182: ⚡ Quick win

Consider adding test coverage for default subcommand path.

The implementation sets ranSubCommand = true for both explicit subcommands (line 65 in src/command.ts) and default subcommands (line 86). The current tests verify the fix for explicit subcommands and the no-subcommand case, but a test confirming that the parent run() is skipped when a default subcommand executes would complete the coverage and guard against regressions in that symmetric code path.

🧪 Suggested test case

+ it("does not run the parent's run() after a default sub command runs", async () => {
+   const mainRunMock = vi.fn();
+   const defaultSubRunMock = vi.fn();
+
+   const command = defineCommand({
+     run: mainRunMock,
+     default: "test",
+     subCommands: {
+       test: {
+         run: defaultSubRunMock,
+       },
+     },
+   });
+
+   await runMain(command, { rawArgs: [] });
+
+   expect(defaultSubRunMock).toHaveBeenCalledOnce();
+   expect(mainRunMock).not.toHaveBeenCalled();
+ });

🤖 Prompt for 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.

In `@test/main.test.ts` around lines 145 - 182, Add a test that verifies the
default subcommand path sets ranSubCommand and prevents the parent run from
executing: create a test similar to the explicit-subcommand one but define
subCommands with a "default" entry (use defineCommand with run: mainRunMock and
subCommands: { default: { run: subRunMock } }), call runMain(command, { rawArgs:
[] }) to trigger the default subcommand, and assert subRunMock was calledOnce
and mainRunMock was not called to cover the code path where ranSubCommand is set
for default subcommands.

🤖 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.

Nitpick comments:
In `@test/main.test.ts`:
- Around line 145-182: Add a test that verifies the default subcommand path sets
ranSubCommand and prevents the parent run from executing: create a test similar
to the explicit-subcommand one but define subCommands with a "default" entry
(use defineCommand with run: mainRunMock and subCommands: { default: { run:
subRunMock } }), call runMain(command, { rawArgs: [] }) to trigger the default
subcommand, and assert subRunMock was calledOnce and mainRunMock was not called
to cover the code path where ranSubCommand is set for default subcommands.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f76b57fe-ade3-48ac-afcc-50eacbecdc8e

📥 Commits

Reviewing files that changed from the base of the PR and between 9cb0edc and 8ec62e4.

📒 Files selected for processing (2)

• src/command.ts
• test/main.test.ts