Consolidate sandbox commands under a sandbox parent - #10
Merged
Merged
Conversation
Move sandbox-setup, clean, and shell into commands/sandbox/ and register them through a new sandbox parent command. The flat names are removed: they now fail with Unknown argument and exit 1. The setup module's command literal changes from 'sandbox-setup' to 'setup'; clean and shell keep theirs, since nesting comes from where a module is registered rather than from its own command string. Adds the co-located test sandbox-setup.ts never had. Implements S-1 through S-9 of the change plan.
…bcommand The three user-facing strings naming a CLI command hardcoded the removed flat spelling. After the consolidation they would have directed users to a command that fails with Unknown argument. Implements S-10 of the change plan.
Target name and its build dependency are unchanged; only the command the recipe invokes changes, so make sandbox-setup behaves as before. Implements S-11 of the change plan.
Updates prose, command tables, file-path references, and Mermaid node labels. The two ADRs and the prior planning run's findings keep the old names: they are records of their own moment. Implements Unit 4 of the change plan.
Two gaps the review round surfaced. Nothing exercised index.ts, so a wrong import path there would break the built CLI while every unit test passed. Verified: pointing the sandbox registration at a nonexistent module leaves all 938 unit tests green and fails four of these. The smoke test spawns the entry point and asserts only on rows that short-circuit before any handler, so it needs no sbx. setup.ts was the fourth module declaring --repo-root with a process.cwd() default and the only one not asserting it. Verified: deleting the default now fails. Addresses review findings from test-engineer.
Adds S-13 and D-10 for the registration test, extends S-11 to cover the README the plan promised but never gave a target state, pins S-10's replacement strings, records four deferrals, and fills the review section. Corrects two claims the review falsified: C-5's call graph and the test-organization standard's supposed no-exemptions language.
…ity claim The sbx migration ADR listed command-name stability among its consequences. Its migration decision stands; only that consequence is superseded, so its text is annotated rather than rewritten. Implements S-12 of the change plan.
Prose-only rewrite: shorter sentences, one idea per paragraph, and the two YAGNI gates named so the deferral entries stop citing undefined terms. Code blocks, tables, headings, and all 49 cross-reference anchors are unchanged.
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
This PR nests the sandbox
setup,clean, andshellcommands under a singlesandboxparent, so that they read as one related command group instead of three unrelated top-level names.Behavior changes
The CLI replaces three flat commands with one
sandboxparent and three sub-commands:skillwalker sandbox-setupskillwalker sandbox setupskillwalker cleanskillwalker sandbox cleanskillwalker shellskillwalker sandbox shellThe CLI removes the old flat names outright, with no alias and no deprecation warning. Anyone still typing
sandbox-setup,clean, orshellgets the command list, anUnknown argumenterror, and exit code 1. Scripts or shell aliases that reference the old names will break and need to switch to the new form. A bareskillwalker sandbox(no sub-command) prints the three sub-commands and also exits 1.Three error messages in
sandbox-integrationthat used to point users atsandbox-setupnow point atsandbox setup, including the message shown when a sandbox does not exist yet. Themake sandbox-setuptarget keeps its name and behavior unchanged; only the underlying CLI command it invokes changed.