feat: add sandbox subcommand #31568
feat: add sandbox subcommand #31568
Conversation
WalkthroughThe DenoSubcommand::Deploy variant now carries an Option<&'static str> payload indicating deploy or sandbox. deploy_parse and tools::deploy signatures were updated to accept that subcommand payload. A new sandbox_subcommand() Command was added and registered in the root CLI. Argument parsing (flags_from_vec_with_initial_cwd and related logic) now returns Deploy(None) for deploy and Deploy(Some("sandbox")) for sandbox; main.rs forwards the payload to deploy. Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⚙️ CodeRabbit configuration file
Files:
cli/main.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)cli/main.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🔇 Additional comments (2)
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: 0
🧹 Nitpick comments (1)
cli/args/flags.rs (1)
587-603: Consider a more typed representation for the deploy/sandbox modeEncoding the deploy vs sandbox distinction as
Deploy(Option<&'static str>)is a bit stringly-typed and easy to mistype (e.g."sandbox"spelled differently in one place). A small dedicated enum (for exampleenum DeployMode { Default, Sandbox }) or at least a sharedconst SANDBOX_MODE: &str = "sandbox";used everywhere would make this safer and clearer at call-sites and pattern matches.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cli/args/flags.rs(7 hunks)cli/main.rs(2 hunks)cli/tools/deploy.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
cli/tools/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
CLI tools should be implemented in
cli/tools/<tool>orcli/tools/<tool>/mod.rs
Files:
cli/tools/deploy.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or uselldbdirectly
Useeprintln!()ordbg!()macros for debug prints in Rust code
Files:
cli/tools/deploy.rscli/main.rscli/args/flags.rs
⚙️ CodeRabbit configuration file
Don't worry about coverage of Rust docstrings. Don't be nitpicky about it. Leave it to the author's judgement if such a documentation is necessary.
Files:
cli/tools/deploy.rscli/main.rscli/args/flags.rs
cli/main.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Main CLI entry point is in
cli/main.rsand should handle command routing
Files:
cli/main.rs
cli/args/flags.rs
📄 CodeRabbit inference engine (CLAUDE.md)
CLI flag parsing should be defined in
cli/args/flags.rs
Files:
cli/args/flags.rs
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:19:37.808Z
Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/args/flags.rs : CLI flag parsing should be defined in `cli/args/flags.rs`
Applied to files:
cli/tools/deploy.rscli/args/flags.rs
🧬 Code graph analysis (1)
cli/main.rs (1)
cli/tools/deploy.rs (1)
deploy(18-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug windows-x86_64
- GitHub Check: test release linux-x86_64
- GitHub Check: test debug macos-x86_64
- GitHub Check: test debug macos-aarch64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: lint debug macos-x86_64
- GitHub Check: lint debug linux-x86_64
- GitHub Check: build libs
🔇 Additional comments (8)
cli/args/flags.rs (4)
1669-1683: Early routing of deploy/sandbox subcommands looks correctShort‑circuiting deploy and sandbox here to call
deploy_parsebefore the generic help/log‑level handling is a good fit for “pass everything through” semantics, and usingSome("sandbox")vsNonelines up with the newDeploy(Option<&'static str>)variant. No functional issues spotted.
1735-1743: Help handling for deploy/sandbox is consistent with new Deploy payloadSpecial‑casing
deno help deployanddeno help sandboxto setargv = ["--help"],allow_all = true, andDenoSubcommand::Deploy(None|Some("sandbox"))keeps behaviour aligned between the two entry points and with the direct subcommand path. This looks coherent with the rest of the flag plumbing.
2080-2081: sandbox_subcommand wiring mirrors deploy_subcommand correctlyRegistering
sandbox_subcommand()inclap_root()and defining it with the same variadicargs/trailing var‑arg/allow_hyphen_values(true)shape asdeploy_subcommand()should makedeno sandbox …behave identically from the parser’s perspective, differing only in the downstreamDeploy(Some("sandbox"))tagging. This is consistent and minimal.Also applies to: 2858-2866
5914-5927: deploy_parse extension for sandbox tagging is straightforwardAdding the
subcommand: Option<&'static str>parameter and storing it asDenoSubcommand::Deploy(subcommand)cleanly separates plain deploy (None) from sandbox (Some("sandbox")) without changing existing argv handling. The function remains side‑effect‑free beyond populatingflags, and there are no lifetime issues given the use of string literals at call‑sites.cli/tools/deploy.rs (2)
18-21: LGTM!Function signature update correctly adds the subcommand parameter to enable subcommand-aware deployment.
24-26: Remove this comment — the behavior is intentional.The
flags.argvis set during argument parsing incli/args/flags.rs(line 5925), then intentionally narrowed to contain only the subcommand indeploy(). This is correct because@deno/deployonly needs the deployment target (e.g., "production", "sandbox") as its sole argument, not the full CLI arguments.cli/main.rs (2)
150-152: LGTM!Deploy subcommand handling correctly extracts and forwards the subcommand payload to the deploy function, aligning with the updated function signature.
737-737: LGTM!Pattern correctly updated to
Deploy(_)to handle bothDeploy(None)andDeploy(Some("sandbox"))variants, ensuring tunnel initialization is skipped for all deploy subcommands.
3 participants
No description provided.