feat(core,cli): add kidd run command and configurable strict mode#118
Merged
zrosenbauer merged 7 commits intomainfrom Mar 27, 2026
Merged
feat(core,cli): add kidd run command and configurable strict mode#118zrosenbauer merged 7 commits intomainfrom
zrosenbauer merged 7 commits intomainfrom
Conversation
Add `strict` option to CliOptions, CommandDef, and ScreenDef to control whether yargs rejects unknown flags. Defaults to true (preserving existing behavior). Per-command `strict: false` overrides the CLI-level setting. Add `kidd run` command with three engine modes: - node (default): builds then runs node dist/index.mjs - tsx: runs source entry directly via tsx - binary: builds, compiles, then executes the compiled binary Supports --inspect, --inspect-brk, --inspect-wait, --inspect-port for debugging (node and tsx only). Unknown flags are forwarded to the CLI. Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 6fe8c11 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…n-cmd # Conflicts: # packages/core/src/cli.ts
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 342-352: extractPassthroughArgs currently filters passthrough
tokens by value, which wrongly drops legitimate command arguments (e.g., the
value following a known flag); instead, rewrite the function to iterate argv
starting at the 'run' token index and parse sequentially: when you encounter a
known flag (use isKnownFlag with params.knownArgs), append the flag to the
passthrough list and if that flag accepts a value, consume and append the next
argv token (do not drop it by value-based filtering); otherwise append
standalone tokens as command args. Apply the same sequential-consume fix to the
other passthrough parsing block later in the file that uses similar value-based
filtering.
- Around line 257-263: The code currently returns the first item from
params.compileOutput.binaries (const [firstBinary] =
params.compileOutput.binaries) which may be for a different OS/arch; change the
selection logic to resolve the host target and pick the binary whose target
matches the host before falling back. Specifically, determine the current host
target (e.g., via existing host target resolver or platform detection), iterate
params.compileOutput.binaries to find a binary whose target/metadata matches
that host, and return that binary; if none match, keep the current
params.ctx.fail or fallback behavior but do not execute binaries[0] blindly.
- Around line 435-456: hasInspectFlag and buildInspectFlags ignore a port-only
invocation of --inspect-port, letting port-only runs bypass checks and emit no
flags; update hasInspectFlag to also return true when args['inspect-port'] is
set, and change buildInspectFlags to, after calling resolveInspectMode(args), if
no mode flag is returned but args['inspect-port'] exists then emit a port-only
Node flag (e.g. `--inspect-port=${args['inspect-port']}`) instead of returning
[]; validate that args['inspect-port'] is a valid number/string port before
emitting; touch the functions hasInspectFlag, buildInspectFlags and referenced
helpers resolveInspectMode/formatInspectFlag and the RunArgs handling of
'inspect-port'.
🪄 Autofix (Beta)
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
Run ID: 767e3541-d56d-4e3f-be5d-18ef440ee150
⛔ Files ignored due to path filters (1)
.changeset/feat-run-command.mdis excluded by!.changeset/**
📒 Files selected for processing (6)
packages/cli/src/commands/run.tspackages/core/src/cli.tspackages/core/src/runtime/register.tspackages/core/src/types/cli.tspackages/core/src/types/command.tspackages/core/src/ui/screen.tsx
- Detect host platform/arch in resolveBinary instead of blindly picking first binary when --target is not provided - Rewrite extractPassthroughArgs with sequential token parsing to avoid false positives from value-based filtering - Include --inspect-port in hasInspectFlag so binary engine guard catches it, and default to --inspect when only --inspect-port is provided Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 137-139: The failure path after checking
hasCompileTargets(configWithTarget) calls params.ctx.fail(...) but does not stop
execution; update the conditional to return immediately after calling
params.ctx.fail so the function exits and does not proceed to buildProject or
later logic (refer to hasCompileTargets, configWithTarget, params.ctx.fail and
buildProject to locate the flow).
- Around line 57-58: The code currently ignores the error element from
loadConfig by destructuring as [, configResult]; update the run command to
capture both elements (e.g., const [configErr, configResult] = await
loadConfig({ cwd })) and check configErr before calling extractConfig; if
configErr is present, log or throw the error (using the project error handling
conventions) and return/exit early to avoid using a null/undefined configResult.
Ensure references to loadConfig, extractConfig, configResult (and cwd) are
updated so the error path is handled per
contributing/standards/typescript/errors.md.
- Around line 545-559: The spawnProcess promise doesn't handle the child's
'error' event so failures (e.g., missing binary) leave the promise pending;
update spawnProcess to listen for child.on('error', ...) and in that handler
either call the promise resolver with a non-zero code (e.g., _resolve(1)) or
reject the promise so callers get notified of the failure; ensure you reference
the same child object created inside spawnProcess and cleanly resolve/reject the
promise in both 'close' and 'error' handlers to avoid leaks or double
resolution.
- Around line 330-334: The code casts params.target directly to CompileTarget
(e.g., when building compile: { ...existingCompile, targets: [params.target as
CompileTarget] }) without validation; define a Zod TargetSchema
(z.enum([...allowed strings...])) and in applyTargetOverride validate
params.target with TargetSchema.safeParse, returning ctx.fail with a clear
message listing TargetSchema.enum values if parsing fails, and only then assign
the validated value into compile.targets (use the parsed value rather than
casting).
🪄 Autofix (Beta)
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
Run ID: 4f47e83e-f9b4-4326-b02b-3eb3ab721d8f
📒 Files selected for processing (1)
packages/cli/src/commands/run.ts
- Check configError from loadConfig instead of silently discarding it - Add missing return before ctx.fail in runWithBinary compile targets check - Validate --target against CompileTarget enum with TargetSchema instead of unsafe `as CompileTarget` cast - Handle spawn error event to prevent promise from hanging indefinitely Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cli/src/commands/run.ts (1)
419-430:⚠️ Potential issue | 🟠 MajorPassthrough parsing still loses valid child arguments.
The sequential skip logic fixes the old value-based filtering bug, but this still anchors on the first literal
run, sokidd --cwd run run --foowill treat the--cwdvalue as the command boundary and forward an extrarun.collectPassthroughTokens()also keeps stripping wrapper flags after--, so there is still no escape hatch for child args like--targetor--inspect. Anchor parsing to the matched command position and stop consuming wrapper flags once--is seen.Also applies to: 461-483
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/run.ts` around lines 419 - 430, The current extractPassthroughArgs implementation anchors on the first literal 'run' and then calls collectPassthroughTokens which still strips wrapper flags after encountering '--', causing valid child args (e.g. --target, --inspect or a later 'run' value) to be lost; change extractPassthroughArgs to locate the exact matched command position in process.argv (use the index of the actual matched 'run' token rather than the first literal) and pass the slice from that exact position into collectPassthroughTokens, and modify collectPassthroughTokens so that once it encounters a '--' token it stops consuming or stripping wrapper flags and instead forwards all subsequent tokens verbatim; apply the same anchored parsing and '--' stop logic to the other duplicated section (the block around lines 461-483 / the other extract code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 261-267: resolveBinary's auto-target logic uses
resolveHostTarget() but doesn't account for musl variants, so on Alpine-like
systems it can miss or choose the wrong binary; update resolveBinary (and the
similar logic around lines 305-308) to: derive hostTarget = resolveHostTarget(),
then attempt to find a binary by exact match and also try the musl variant (e.g.
`${hostTarget}-musl`) and the inverse (strip `-musl`) so both `linux-x64` and
`linux-x64-musl` are considered; if the runtime is musl-detected (or a musl
artifact exists while the non-musl also exists), prefer the musl artifact. Use
the existing params.compileOutput.binaries.find logic but check both variants
and prefer the one matching the actual host libc variant.
---
Duplicate comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 419-430: The current extractPassthroughArgs implementation anchors
on the first literal 'run' and then calls collectPassthroughTokens which still
strips wrapper flags after encountering '--', causing valid child args (e.g.
--target, --inspect or a later 'run' value) to be lost; change
extractPassthroughArgs to locate the exact matched command position in
process.argv (use the index of the actual matched 'run' token rather than the
first literal) and pass the slice from that exact position into
collectPassthroughTokens, and modify collectPassthroughTokens so that once it
encounters a '--' token it stops consuming or stripping wrapper flags and
instead forwards all subsequent tokens verbatim; apply the same anchored parsing
and '--' stop logic to the other duplicated section (the block around lines
461-483 / the other extract code).
🪄 Autofix (Beta)
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
Run ID: 52b04105-3ca8-4356-b6da-efec90756ddb
📒 Files selected for processing (1)
packages/cli/src/commands/run.ts
Prefer linux-x64-musl over linux-x64 on x64 Linux hosts so Alpine-like systems find the correct binary when --target is not provided. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 642-661: The spawnProcess function swallows spawn errors in the
child.on('error') handler and only resolves with 1, losing diagnostic context;
modify spawnProcess to accept an optional onError callback (e.g., add to params:
onError?: (err: Error) => void) and invoke it with the error inside the
child.on('error') handler before resolving, or alternatively include the error
message when resolving/rejecting so callers using spawnProcess (refer to
function spawnProcess, params, and child.on('error')) can log or surface the
underlying spawn error (like ENOENT) for better diagnostics.
🪄 Autofix (Beta)
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
Run ID: a77ecdee-99b8-4390-8e84-9cd32cbfecd2
📒 Files selected for processing (1)
packages/cli/src/commands/run.ts
Log the error message to stderr when spawn fails (e.g. ENOENT for missing tsx binary) instead of silently resolving with exit code 1. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/cli/src/commands/run.ts (1)
467-478:⚠️ Potential issue | 🟠 MajorPassthrough parsing can still lock onto the wrong
runtoken.Line 470 uses the first raw
runtoken fromprocess.argvas the command boundary.kidd --cwd run run --foowill treat the--cwdvalue as the boundary and forward an extrarunto the child process. Use the parser’s matched command position instead ofargv.indexOf('run').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/run.ts` around lines 467 - 478, The current extractPassthroughArgs uses argv.indexOf('run') which can pick the wrong literal token; instead derive the run command boundary from the parser's matched command position available on params.knownArgs and slice process.argv using that position (so use the parser's matched command index from params.knownArgs to compute runIndex, fallback to -1 if not present), then call collectPassthroughTokens on the tokens after that position; update extractPassthroughArgs to stop referencing argv.indexOf('run') and use the parser-provided index from params.knownArgs (keeping collectPassthroughTokens and the rest of the logic intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/run.ts`:
- Line 15: DEFAULT_ENTRY is set to './src/index.ts' but KiddConfig.entry
defaults to './index.ts', causing tsx runs to pick the wrong file; update
DEFAULT_ENTRY to './index.ts' (or derive it from KiddConfig.entry) and ensure
the code path that selects entry (where config.entry is read and fallback to
DEFAULT_ENTRY in the run command) uses this updated value so tsx falls back to
the same default as node/binary.
- Around line 154-158: The hasCompileTargets helper currently only returns true
for a compile config with an explicit non-empty targets array, which rejects
valid configs like compile: true or object configs that omit targets (both
should default to current platform); update hasCompileTargets to return true
when config.compile === true, or when config.compile is an object and either
targets is undefined (meaning use defaults) or targets is a non-empty array, and
return false only when compile is an object with an explicitly empty targets
array; reference the hasCompileTargets function used in run.ts and adjust its
conditional logic accordingly.
---
Duplicate comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 467-478: The current extractPassthroughArgs uses
argv.indexOf('run') which can pick the wrong literal token; instead derive the
run command boundary from the parser's matched command position available on
params.knownArgs and slice process.argv using that position (so use the parser's
matched command index from params.knownArgs to compute runIndex, fallback to -1
if not present), then call collectPassthroughTokens on the tokens after that
position; update extractPassthroughArgs to stop referencing argv.indexOf('run')
and use the parser-provided index from params.knownArgs (keeping
collectPassthroughTokens and the rest of the logic intact).
🪄 Autofix (Beta)
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
Run ID: 86fd926f-9377-4838-8520-f991fe318f58
📒 Files selected for processing (1)
packages/cli/src/commands/run.ts
Update hasCompileTargets to accept compile: true (boolean shorthand) and object configs that omit targets (bundler fills in defaults). Co-Authored-By: Claude <noreply@anthropic.com>
zrosenbauer
added a commit
that referenced
this pull request
Mar 27, 2026
The `kidd run` command (PR #118) merged after the DisplayConfig PR and still referenced `ctx.spinner` instead of `ctx.status.spinner`. Also adds an empty changeset for the refactor-only change. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
strictmode toCliOptions,CommandDef, andScreenDef— controls whether yargs rejects unknown flags. Defaults totrue(existing behavior), per-commandstrict: falseoverrides the CLI-level setting.kidd runcommand with three engine modes:node(default) — builds first, then runsnode dist/index.mjstsx— runs the source entry file directly viatsxbinary— builds and compiles, then executes the compiled binary--inspect,--inspect-brk,--inspect-wait, and--inspect-portfor debugging (node and tsx engines only — errors if used with binary)--separator needed)Test plan
kidd run greet --name=worldbuilds and runs with node enginekidd run --engine=tsx greet --name=worldruns from source via tsxkidd run --engine=binary --target=darwin-arm64 greetcompiles and runs binarykidd run --engine=binary --inspect greetfails with clear error messagekidd run --inspect-brk greetstarts with debugger breakpointkidd run --engine=binary greetwithout targets configured fails with clear errorstrict: falseaccepts unknown flagspnpm test)