refactor(cli): organize migrated workflow modules#3163
Merged
Conversation
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 (3)
📝 WalkthroughWalkthroughThis PR restructures module imports across the codebase to nested paths (e.g., action/domain nesting), adds READMEs for Actions/Domain/Adapters, updates CLI/action/test imports to the new layout, and improves tests with firmware mocking and dynamic router port allocation. ChangesModule Restructuring and Architectural Documentation
Test Infrastructure Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
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)
Comment |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/onboard.test.ts (1)
2481-2488: ⚡ Quick winFail fast if blueprint patch replacement stops matching
Line 2485/Line 2486 use exact string replacements; if blueprint formatting changes, this silently no-ops and can regress to the default router port path. Add a guard so the test fails explicitly when patching does not occur.
Suggested patch
fs.readFileSync = (filePath, ...args) => { const raw = originalReadFileSync(filePath, ...args); if (filePath === blueprintPath || String(filePath) === blueprintPath) { - return String(raw) - .replace('endpoint: "http://localhost:4000/v1"', 'endpoint: "http://localhost:' + routerPort + '/v1"') - .replace("port: 4000", "port: " + routerPort); + const next = String(raw) + .replace( + /endpoint:\s*"http:\/\/localhost:4000\/v1"/, + `endpoint: "http://localhost:${routerPort}/v1"`, + ) + .replace(/port:\s*4000\b/, `port: ${routerPort}`); + if (next === String(raw)) { + throw new Error("Failed to patch blueprint router endpoint/port in test fixture"); + } + return next; } return raw; };🤖 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 `@test/onboard.test.ts` around lines 2481 - 2488, The fs.readFileSync test shim that patches the blueprint (overriding fs.readFileSync and using blueprintPath, originalReadFileSync, and routerPort) should fail the test if the expected string replacements did not occur; after performing the two .replace calls, detect whether the returned string actually changed (or whether it contains the expected patched substrings like the new endpoint or port) and throw or assert (fail fast) when the patch hasn't been applied so the test will break if blueprint formatting changes.
🤖 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 `@test/onboard.test.ts`:
- Around line 2481-2488: The fs.readFileSync test shim that patches the
blueprint (overriding fs.readFileSync and using blueprintPath,
originalReadFileSync, and routerPort) should fail the test if the expected
string replacements did not occur; after performing the two .replace calls,
detect whether the returned string actually changed (or whether it contains the
expected patched substrings like the new endpoint or port) and throw or assert
(fail fast) when the patch hasn't been applied so the test will break if
blueprint formatting changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2c9c1b48-d4d2-4642-ae6f-4387063a8a3b
📒 Files selected for processing (25)
src/commands/internal/README.mdsrc/commands/internal/dev/npm-link-or-shim.tssrc/commands/internal/installer/normalize-env.tssrc/commands/internal/installer/plan.tssrc/commands/internal/uninstall/classify-shim.tssrc/commands/internal/uninstall/plan.tssrc/commands/internal/uninstall/run-plan.tssrc/lib/actions/README.mdsrc/lib/actions/dev/npm-link-or-shim.test.tssrc/lib/actions/dev/npm-link-or-shim.tssrc/lib/actions/dns/index.test.tssrc/lib/actions/dns/index.tssrc/lib/actions/installer/plan.test.tssrc/lib/actions/installer/plan.tssrc/lib/actions/uninstall/plan.test.tssrc/lib/actions/uninstall/plan.tssrc/lib/actions/uninstall/run-plan.test.tssrc/lib/actions/uninstall/run-plan.tssrc/lib/adapters/README.mdsrc/lib/domain/README.mdsrc/lib/domain/dev/npm-link-or-shim.test.tssrc/lib/domain/dev/npm-link-or-shim.tssrc/lib/nim.test.tstest/npm-link-or-shim.test.tstest/onboard.test.ts
jyaunches
pushed a commit
that referenced
this pull request
May 8, 2026
## Summary - Bump the docs release metadata to `0.0.37`. - Document release-prep updates for messaging policy presets, sandbox runtime utilities, and the GPU CDI troubleshooting path. - Refresh generated `nemoclaw-user-*` skills from the updated docs. ## Source summary - #3159 -> `docs/reference/troubleshooting.md`: Documents the GPU CDI preflight warning and remediation for `nvidia.com/gpu=all` gateway start failures. - #2415 -> `docs/reference/network-policies.md`, `docs/manage-sandboxes/messaging-channels.md`, `docs/network-policy/customize-network-policy.md`: Clarifies that Telegram, Discord, and Slack egress comes from opt-in messaging presets, not the baseline policy. - #3091 -> `docs/deployment/sandbox-hardening.md`, `docs/network-policy/customize-network-policy.md`: Documents the retained sandbox utilities `vi`, `jq`, and `dos2unix` while keeping host-side policy files as the durable source of truth. ## Test plan - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user` - `make docs` - `npm run build:cli` - `npm run typecheck:cli` - Commit and pre-push hooks: markdownlint, docs-to-skills verification, gitleaks, commitlint, CLI typecheck ## Skipped - #3193 and #3191 matched `docs/.docs-skip` entries for experimental shields/config paths. - #3200 and #3183 were test-only fixes. - #3189 and #3163 were internal documentation/refactor changes with no public docs impact. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified which utilities remain in the sandbox runtime for lightweight inspection and cleanup * Noted that messaging endpoints (Discord, Slack, Telegram) are not in the baseline policy and that channel presets are applied during onboarding * Added GPU passthrough troubleshooting for gateway startup * Updated release/version bump and release-prep workflow guidance, including Discord preset description updates <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced May 8, 2026
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
3 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
Organizes the recently migrated internal workflows so command, action, domain, and adapter directories explain the intended layering. The PR also makes two environment-sensitive tests hermetic so local pre-push checks pass on Spark/router developer machines.
Changes
src/lib/actions/<area>/directories.src/lib/domain/dev/npm-link-or-shim.ts.src/commands/internal,src/lib/actions,src/lib/domain, andsrc/lib/adapters.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Documentation
Bug Fixes