Skip to content

fix: forward extra arguments to devbox run scenarios#47

Merged
pilat merged 2 commits into
mainfrom
pilat/tags-pass-trough
Jul 4, 2026
Merged

fix: forward extra arguments to devbox run scenarios#47
pilat merged 2 commits into
mainfrom
pilat/tags-pass-trough

Conversation

@pilat

@pilat pilat commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Why

devbox run <scenario> [args...] is documented to forward extra arguments to the scenario's command (docs/run.md shows devbox run test --verbose --suite=api), but today any extra argument makes the command fail before the scenario ever runs. Both devbox run e2e --tag foo and devbox run e2e -- --tag foo error out with accepts 1 arg(s), received N.

The cause is two lines that contradict each other: Args: cobra.ExactArgs(1) rejects any argv beyond the scenario name, which silently defeats the SetInterspersed(false) that exists specifically to enable pass-through.

What

  • Relax the validator: cobra.ExactArgs(1)cobra.MinimumNArgs(1). A scenario name is still required; extra args are now accepted.
  • Keep SetInterspersed(false) so the flag-only form (run e2e --tag foo, no --) works and tokens after the scenario name are forwarded rather than swallowed by devbox's own flags.
  • Strip exactly one leading -- from the forwarded args so both run e2e --tag foo and run e2e -- --tag foo forward a clean [--tag foo] — matching the docker/cargo/npm separator convention. A -- appearing later is passed through untouched.
  • Document the explicit-separator form in docs/run.md.

Notes

  • Only the run command changes. shell.go also uses ExactArgs(1), but that is correct — shell takes exactly one service name with no pass-through — so it is left alone.
  • The command construction is extracted into newRunCmd() so the regression tests bind to the real command object (real validator + SetInterspersed(false)) rather than a hand-rebuilt copy that could drift. The contract test is what guards against a future revert to ExactArgs.
  • Tests are Docker-free: a table-driven unit test over the arg-splitting helper, plus a cobra contract test asserting the parse/validate behavior for the flag-only, ---separated, single-arg, and no-scenario cases.

Summary by CodeRabbit

  • New Features
    • devbox run now accepts flexible argument passing, making it easier to forward flags to scenarios.
    • A leading -- is now treated as an explicit separator and won’t be passed through unexpectedly.
  • Documentation
    • Added an example showing the updated devbox run argument format and forwarding behavior.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • cmd/devbox/run_test.go is excluded by !**/*_test.go

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 302201b7-a498-42d7-8afe-019f3ef46c1d

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

The devbox run command now accepts a minimum of one argument instead of exactly one, using a new splitScenarioArgs helper to separate the scenario name from forwarded passthrough arguments, stripping a leading -- separator. Command registration was refactored, and documentation was updated with a corresponding example.

Changes

Run command argument handling

Layer / File(s) Summary
Command registration and argument splitting
cmd/devbox/run.go
newRunCmd() now uses cobra.MinimumNArgs(1), is registered via init(), and returns the configured command; RunE uses a new splitScenarioArgs helper to derive scenario and passthrough args, stripping a leading --.
Documentation of -- separator
docs/run.md
Adds an example showing devbox run test -- --verbose --suite=api where -- is consumed and remaining flags are forwarded to the scenario.

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

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CLI as newRunCmd (Cobra)
  participant Splitter as splitScenarioArgs
  participant Runner as runRun

  User->>CLI: devbox run e2e -- --tag
  CLI->>Splitter: splitScenarioArgs(args)
  Splitter-->>CLI: scenario="e2e", passthrough=["--tag"]
  CLI->>Runner: runRun(ctx, p, scenario, passthrough, noTty)
Loading
🚥 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 accurately summarizes the main change: devbox run now forwards extra scenario arguments.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pilat/tags-pass-trough

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.

@pilat pilat self-assigned this Jul 4, 2026
@pilat pilat merged commit 87d0353 into main Jul 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant