Merged
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. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (27)
📝 WalkthroughWalkthroughMoves various state-related modules into a new ChangesState module relocation and path wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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)
Comment |
wscurran
added
NemoClaw CLI
This was referenced May 4, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/state/sandbox.ts (1)
29-33: Run the targeted sandbox persistence E2E jobs before merge.Given this file’s persistence role, please run:
snapshot-commands-e2erebuild-openclaw-e2eAs per coding guidelines
src/lib/state/sandbox.ts: “Changes affect data persistence across sandbox lifecycle operations” with the listed E2E recommendations.🤖 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/state/sandbox.ts` around lines 29 - 33, This change affects sandbox persistence; before merging, run the targeted E2E jobs snapshot-commands-e2e and rebuild-openclaw-e2e and ensure they pass; if they fail, investigate and fix persistence-related logic in sandbox lifecycle code referenced in this file (look at functions and modules imported here such as loadAgent, resolveOpenshell, captureOpenshellCommand, sanitizeConfigFile, isSensitiveFile, and shellQuote) and re-run the E2E jobs until green.
🤖 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.
Inline comments:
In `@test/gateway-liveness-probe.test.ts`:
- Around line 75-84: The test's description references the old module name
"gateway-state.ts"; update the it(...) title in
test/gateway-liveness-probe.test.ts to reflect the current module path (e.g.,
change "does not modify isGatewayHealthy() in gateway-state.ts" to "does not
modify isGatewayHealthy() in src/lib/state/gateway.ts" or similar) so failure
messages point to the correct file; keep the rest of the test (the
fs.readFileSync and regex against isGatewayHealthy) unchanged.
---
Nitpick comments:
In `@src/lib/state/sandbox.ts`:
- Around line 29-33: This change affects sandbox persistence; before merging,
run the targeted E2E jobs snapshot-commands-e2e and rebuild-openclaw-e2e and
ensure they pass; if they fail, investigate and fix persistence-related logic in
sandbox lifecycle code referenced in this file (look at functions and modules
imported here such as loadAgent, resolveOpenshell, captureOpenshellCommand,
sanitizeConfigFile, isSensitiveFile, and shellQuote) and re-run the E2E jobs
until green.
🪄 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: CHILL
Plan: Enterprise
Run ID: fbe5bb22-a237-4848-9dc3-443c9372d386
📒 Files selected for processing (61)
.coderabbit.yamlscripts/check-legacy-migrated-paths.tsscripts/dev-tier-selector.jsscripts/ts-migration-assist.tssrc/lib/actions/maintenance.tssrc/lib/actions/sandbox/connect.tssrc/lib/actions/sandbox/destroy.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/gateway-state.tssrc/lib/actions/sandbox/policy-channel.tssrc/lib/actions/sandbox/rebuild.tssrc/lib/actions/sandbox/snapshot.tssrc/lib/actions/sandbox/status.tssrc/lib/actions/upgrade-sandboxes.tssrc/lib/agent-runtime.tssrc/lib/commands/debug.tssrc/lib/commands/simple-global-oclif-adapters.test.tssrc/lib/commands/tunnel/common.tssrc/lib/credentials.tssrc/lib/debug.tssrc/lib/http-probe.tssrc/lib/list-command-deps.tssrc/lib/messaging-conflict.test.tssrc/lib/messaging-conflict.tssrc/lib/onboard.tssrc/lib/policies.tssrc/lib/registry-recovery-action.tssrc/lib/sandbox-config.tssrc/lib/sandbox-create-stream.tssrc/lib/sandbox-version.test.tssrc/lib/sandbox-version.tssrc/lib/shields-audit.tssrc/lib/state/config-io.test.tssrc/lib/state/config-io.tssrc/lib/state/gateway.tssrc/lib/state/paths.test.tssrc/lib/state/paths.tssrc/lib/state/registry.tssrc/lib/state/sandbox-session.test.tssrc/lib/state/sandbox-session.tssrc/lib/state/sandbox.tssrc/lib/status-command-deps.tssrc/nemoclaw.tstest/credential-rotation.test.tstest/e2e/brev-e2e.test.tstest/gateway-liveness-probe.test.tstest/gateway-state.test.tstest/image-cleanup.test.tstest/onboard-preset-diff.test.tstest/onboard-resume-provider-recovery.test.tstest/onboard-selection.test.tstest/onboard.test.tstest/policies.test.tstest/policy-tiers-onboard.test.tstest/rebuild-policy-presets.test.tstest/registry.test.tstest/repro-2010.test.tstest/security-sandbox-tar-traversal.test.tstest/shellquote-sandbox.test.tstest/shields.test.tstest/snapshot.test.ts
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
cv
added a commit
that referenced
this pull request
May 6, 2026
## 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 <cvillela@nvidia.com> <!-- 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 <cvillela@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
prekshivyas
approved these changes
May 6, 2026
Contributor
prekshivyas
left a comment
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM. Same #2984/#2985/#2988-pattern PR — pure file-organization, 64 files / +138 / -138.
State modules (gateway, sandbox-session, config-io, paths, registry, sandbox) moved to src/lib/state/. Most consumer files +1/-1 or +2/-2 import-path updates. Largest individual diff is test/onboard.test.ts (+32/-32) — many state imports updated.
Tooling companions: .coderabbit.yaml path instructions, scripts/check-legacy-migrated-paths.ts guard list, scripts/dev-tier-selector.js, scripts/ts-migration-assist.ts — all single-line path migrations required by the move.
Verification checklist explicitly cites running check-layer-import-boundaries.ts from #2986.
CI: pr.yaml mostly green (lint/dco/check-hash/legacy-path-guard/changes PASS); macos-e2e/wsl-e2e/checks + pr-self-hosted builds still in flight at review time. No failures.
cv
added a commit
that referenced
this pull request
May 6, 2026
## Summary Move the Docker adapter package under `src/lib/adapters/docker/**` so Docker process boundaries live with the other adapter modules. ## Stack Navigation - Position: 60 of 60 - Previous PR: [#2989 — refactor(cli): group state modules](#2989) - Next PR: top of stack ## Changes - Moved `src/lib/docker/**` to `src/lib/adapters/docker/**`. - Updated Docker adapter imports across actions, onboarding/runtime helpers, services, shields, and tests. - Updated the Docker abstraction guard to use the new adapter path. - 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] Docker adapter 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 <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Reorganized internal Docker utility modules for improved code organization and maintainability. This structural change enhances the codebase architecture without affecting any user-facing functionality or features. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.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
5 participants
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
Move registry, config, path, gateway, session, and sandbox persistence modules into
src/lib/state/**so persistence boundaries are visible in paths.Stack Navigation
Changes
src/lib/state/.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 cvillela@nvidia.com
Summary by CodeRabbit
Refactor
New Feature