refactor(cli): group sandbox actions#2987
Conversation
This reverts commit 4ebeae4.
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR reorganizes sandbox action modules from a flat file structure to a nested directory layout under ChangesModule Reorganization & Import Path Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/process-recovery.ts (1)
11-24: Run the restart-path E2Es before merge.This module sits directly on the gateway restart/process recovery path, so I'd still validate the refactor with the nightly recovery scenarios; unit and CLI subprocess tests usually won't catch every runtime path-resolution regression in these flows.
As per coding guidelines, "E2E test recommendation:
sandbox-survival-e2e— gateway restart recovery;sandbox-operations-e2e— process recovery after gateway kill".🤖 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 `@src/lib/actions/sandbox/process-recovery.ts` around lines 11 - 24, This change touches the gateway restart/process-recovery path (module process-recovery.ts, symbols like agentRuntime, runOpenshell, getOpenshellBinary and captureOpenshell), so before merging run the recommended E2E suites to validate runtime path resolution: execute sandbox-survival-e2e (gateway restart recovery) and sandbox-operations-e2e (process recovery after gateway kill) against the refactor and confirm the nightly recovery scenarios pass; if failures appear, trace them to the imports/paths and runtime invocation patterns in process-recovery.ts (agentRuntime usage, openshell adapter calls) and fix path/resolution or invocation order until the E2Es pass.
🤖 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 `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 11-24: This change touches the gateway restart/process-recovery
path (module process-recovery.ts, symbols like agentRuntime, runOpenshell,
getOpenshellBinary and captureOpenshell), so before merging run the recommended
E2E suites to validate runtime path resolution: execute sandbox-survival-e2e
(gateway restart recovery) and sandbox-operations-e2e (process recovery after
gateway kill) against the refactor and confirm the nightly recovery scenarios
pass; if failures appear, trace them to the imports/paths and runtime invocation
patterns in process-recovery.ts (agentRuntime usage, openshell adapter calls)
and fix path/resolution or invocation order until the E2Es pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 89b2684c-2eed-4da5-8b92-9be423d956f8
📒 Files selected for processing (29)
src/lib/actions/sandbox/connect.tssrc/lib/actions/sandbox/destroy.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/gateway-state.tssrc/lib/actions/sandbox/logs.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/actions/sandbox/process-recovery.tssrc/lib/actions/sandbox/rebuild.tssrc/lib/actions/sandbox/runtime.tssrc/lib/actions/sandbox/skill-install.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/actions/sandbox/status.tssrc/lib/commands/sandbox/channels/common.tssrc/lib/commands/sandbox/channels/list.tssrc/lib/commands/sandbox/connect.tssrc/lib/commands/sandbox/destroy.tssrc/lib/commands/sandbox/doctor.tssrc/lib/commands/sandbox/logs.tssrc/lib/commands/sandbox/oclif-command-adapters.test.tssrc/lib/commands/sandbox/policy/common.tssrc/lib/commands/sandbox/policy/list.tssrc/lib/commands/sandbox/rebuild.tssrc/lib/commands/sandbox/skill/common.tssrc/lib/commands/sandbox/snapshot/common.tssrc/lib/commands/sandbox/status.tssrc/lib/share-command-deps.tssrc/lib/upgrade-sandboxes-action.tssrc/nemoclaw.tstest/image-cleanup.test.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
## Summary Add a narrow layer-boundary guard for the new CLI directory structure before moving action/state/adapter modules further. ## Stack Navigation - Position: 56 of 60 - Previous PR: [#2985 — refactor(cli): group openshell adapters](#2985) - Next PR: [#2987 — refactor(cli): group sandbox actions](#2987) ## Changes - Added `scripts/check-layer-import-boundaries.ts` to validate initial import-boundary rules. - Added a Vitest smoke test that runs the guard without source-shape assertions. - Enforces domain purity, no oclif imports from actions, no command/action imports from adapters, and one registered oclif command class per production command file. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files --stage pre-push` passes - [x] `npx tsx scripts/check-layer-import-boundaries.ts` - [x] `npx vitest run test/layer-import-boundaries.test.ts` - [x] `npm run source-shape:check` - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated pre-commit hook to improve formatting behavior during commits. * Added automated validation to enforce code architecture layer boundaries and prevent unintended cross-layer imports. * **Tests** * Added an automated test to verify the new layer-boundary validation runs successfully and reports passing status. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <[email protected]>
Signed-off-by: Carlos Villela <[email protected]>
## Summary Move top-level/global workflow action modules into `src/lib/actions/**` so command adapters call a visible action layer instead of flat `src/lib` modules. ## Stack Navigation - Position: 58 of 60 - Previous PR: [#2987 — refactor(cli): group sandbox actions](#2987) - Next PR: [#2989 — refactor(cli): group state modules](#2989) ## Changes - Moved deploy, maintenance, onboarding facade, root help, upgrade-sandboxes, and global action facade modules under `src/lib/actions/`. - Moved the global action facade test with the module. - Updated command adapters, root bootstrap, integration tests, and callers to the new paths. - Kept behavior unchanged; this PR is a structural move plus import updates. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files --stage pre-push` passes - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] `npx tsx scripts/check-layer-import-boundaries.ts` - [x] Global command/action targeted tests pass - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Internal reorganization of CLI action module structure for improved code organization and maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <[email protected]> Co-authored-by: Prekshi Vyas <[email protected]>
Summary
Move sandbox-scoped workflow action modules into
src/lib/actions/sandbox/**so command adapters and sandbox workflow orchestration have separate, visible layers.Stack Navigation
Changes
src/lib/actions/sandbox/.src/lib/actions/sandbox/policy-channel.ts.Type of Change
Verification
npx prek run --all-files --stage pre-pushpassesnpm run build:clinpm run typecheck:clinpx tsx scripts/check-layer-import-boundaries.tsmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela [email protected]
Summary by CodeRabbit