feat(state): segregate host state root per gateway port#6711
feat(state): segregate host state root per gateway port#6711laitingsheng wants to merge 36 commits into
Conversation
A non-default NEMOCLAW_GATEWAY_PORT now resolves its own host state root at ~/.nemoclaw/gateways/<port>/, covering the sandbox registry, credentials, snapshots, onboard session, and inference adapter state, so two gateways on one host stay fully segregated. The default port keeps the shared ~/.nemoclaw/ location, so existing single-gateway hosts are byte-identical. Route the registry, credentials, config-io, onboard, inference, share, and diagnostics paths through one nemoclawStateRoot resolver keyed off GATEWAY_PORT. Keep the CLI source clone and the managed_swap marker host-shared. Uninstall targets the port-scoped state root, always preserves the gateways/ subtree so other environments survive even --destroy-user-data, and no longer tears down the host-shared swap file while other gateway-port environments remain. Fixes #3053 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change centralizes gateway-aware state paths, migrates legacy state, scopes uninstall and installer operations, adds registry and symlink-safety validation, and documents non-default gateway lifecycle behavior. ChangesGateway-port state segregation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant migrateLegacyPortState
participant GatewayRegistry
participant SelectedGateway
participant SharedState
CLI->>migrateLegacyPortState: migrate selected port state
migrateLegacyPortState->>GatewayRegistry: inspect registry identities
GatewayRegistry->>SelectedGateway: write owned registry and state
migrateLegacyPortState->>SharedState: retain ambiguous or remaining state
CLI->>SelectedGateway: continue selected-port command
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 79%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-6711.docs.buildwithfern.com/nemoclaw |
laitingsheng
added
area: sandbox
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron is a non-blocking second opinion. Its prose, findings, and E2E guidance do not change the primary assessment above and remain in workflow artifacts only. Since last review: 0 prior items resolved · 0 still apply · 0 new items found E2E guidanceAdvisory only: coverage and selector recommendations are non-authoritative. E2E / PR Gate independently computes and dispatches trusted jobs without consuming this output. Recommended coverage:
This is an automated, non-authoritative review. Findings are inputs to maintainer adjudication. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@docs/manage-sandboxes/lifecycle.mdx`:
- Around line 154-157: Update the agent-specific lifecycle documentation block
to replace the invalid $$nemoclaw uninstall command with the literal Deep Agents
CLI command nemo-deepagents uninstall, while preserving the surrounding
gateway-state behavior and wording.
- Line 150: Update the lifecycle documentation near the NEMOCLAW_GATEWAY_PORT
state-root description to replace “fully segregated” with a narrower claim about
gateway-scoped state. Mention that the CLI source clone and managed_swap marker
remain shared under ~/.nemoclaw/ if describing shared host-level artifacts,
while preserving the separate registry, credentials, snapshots, and uninstall
behavior.
In `@docs/reference/architecture.mdx`:
- Line 311: Update the preceding ~/.nemoclaw/sandboxes.json documentation row to
state explicitly that it is the registry for the default gateway port only;
retain the separate non-default registry location described by the
gateways/<port> entry.
In `@src/lib/actions/uninstall/run-plan-gateway-segregation.test.ts`:
- Around line 70-73: Remove the conditional inside the mock run handler and
record every invoked command unconditionally in swapCommands. Update the related
assertions to verify unwanted commands with not.toContain, preserving the test’s
intent without adding an if statement.
In `@src/lib/onboard/usage-notice.ts`:
- Line 56: Update the usage-notice path construction to pass the active gateway
port to nemoclawStateRoot instead of relying on its default-port behavior.
Preserve the existing HOME fallback and usage-notice.json filename while
ensuring non-default gateways resolve their port-scoped state root.
In `@src/lib/share-command.ts`:
- Line 64: The defaultShareMountDir resolver currently ignores gateway identity,
causing all gateways to use the same mount root. Update defaultShareMountDir to
accept the active gateway port and include that port when deriving the mount
directory, then update every caller to pass the corresponding gateway port so
non-default gateways use isolated mount state.
In `@src/lib/state/onboard-session.ts`:
- Around line 48-50: Update the onboarding session path definitions around
SESSION_DIR, SESSION_FILE, and LOCK_FILE to derive the directory using the
active gateway port, rather than a single module-global nemoclawStateRoot value.
Ensure each non-default gateway receives distinct session and lock paths while
preserving the existing filenames.
In `@src/lib/state/state-root.test.ts`:
- Around line 37-41: Update the getNemoclawStateRoot default-port test to
explicitly control NEMOCLAW_GATEWAY_PORT, clearing it before module
initialization or using the explicit-port helper to assert default behavior.
Ensure the test remains isolated from the host environment and preserves the
expected shared root assertion.
In `@src/lib/state/state-root.ts`:
- Around line 19-24: Update resolveHome to use an empty-value fallback, matching
the credentials store behavior, so empty HOME values fall back to os.homedir()
before getNemoclawStateRoot constructs the state path. Keep the existing
getNemoclawStateRoot and nemoclawStateRoot flow unchanged.
🪄 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: 3fa4a17d-c76f-46ab-82a5-a14de9de2fd0
📒 Files selected for processing (21)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/architecture.mdxsrc/lib/actions/uninstall/run-plan-gateway-segregation.test.tssrc/lib/actions/uninstall/run-plan.tssrc/lib/credentials/store.tssrc/lib/domain/uninstall/paths.tssrc/lib/inference/local-adapter-lifecycle.tssrc/lib/inference/local.tssrc/lib/onboard/docker-gpu-patch.tssrc/lib/onboard/model-router.tssrc/lib/onboard/sandbox-create-failure.tssrc/lib/onboard/usage-notice.tssrc/lib/share-command.tssrc/lib/state/config-io.tssrc/lib/state/onboard-session.tssrc/lib/state/paths.tssrc/lib/state/registry-port-segregation.test.tssrc/lib/state/registry.tssrc/lib/state/sandbox.tssrc/lib/state/state-root.test.tssrc/lib/state/state-root.ts
Fixes a regression the port-keyed state root introduced in a pre-existing CLI status test that temporarily overrides NEMOCLAW_GATEWAY_PORT to probe for a gateway-port conflict, closes the empty-HOME relative-path edge case in the state-root resolver, makes the default-port unit test control the environment variable it claims to test, corrects two doc wording issues (an overstated segregation claim and a Deep Agents command name), and drops a conditional from the new uninstall test to satisfy the codebase-growth guardrail. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/repro-2666-silent-list-status.test.ts`:
- Line 210: Strengthen the port-specific scenario setup around seedRegistry so
the default ~/.nemoclaw registry cannot satisfy the test: clear it before the
scenario or seed it with distinct sentinel data, then assert the selected result
comes only from nemoclawStateRoot(home, port). Apply the same correction to the
corresponding occurrence.
🪄 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: 54a56463-3080-4275-8500-99fb477832c9
📒 Files selected for processing (6)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/architecture.mdxsrc/lib/actions/uninstall/run-plan-gateway-segregation.test.tssrc/lib/state/state-root.test.tssrc/lib/state/state-root.tstest/repro-2666-silent-list-status.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/state/state-root.ts
- src/lib/state/state-root.test.ts
- docs/manage-sandboxes/lifecycle.mdx
- src/lib/actions/uninstall/run-plan-gateway-segregation.test.ts
- docs/reference/architecture.mdx
The port-specific #3271 scenario seeded identical my-assist data under both the default and port-scoped registries, so a regression that kept reading the default root under a non-default NEMOCLAW_GATEWAY_PORT would still pass. Clear the default registry before that scenario so the test only succeeds when the CLI actually reads the port-scoped root. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…ate-root Signed-off-by: Tinson Lai <tinsonl@nvidia.com> # Conflicts: # src/lib/onboard/docker-gpu-patch.ts
Every production call site relied on nemoclawStateRoot's default gatewayPort parameter, which resolves to the correct value only because GATEWAY_PORT happens to be a process-wide constant computed once at module load. That's easy to misread as always resolving the default port. Thread GATEWAY_PORT through all 18 call sites explicitly so each one is self-evidently correct without tracing into core/ports.ts. No behavioural change — GATEWAY_PORT is the exact value the default parameter already supplied. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
E2E Target Results —
|
| Test | Result |
|---|---|
| cloud-onboard | |
| concurrent-gateway-ports | |
| credential-sanitization | |
| inference-routing | ✅ success |
| network-policy | |
| onboard-repair | |
| onboard-resume | |
| security-posture |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
E2E Target Results — ❌ Some tests failedRun: 29265796896
|
# Conflicts: # src/lib/actions/maintenance.ts
E2E Target Results — ❌ Some tests failedRun: 29266737670
|
|
Exact-head live-E2E diagnosis for the failing |
## Summary - validate the pinned Ubuntu `fd-find` binary using its actual `fdfind <version>` identity - keep the trusted workflow-boundary validator aligned with the executable check - make the workflow test fixture reproduce the package output seen on hosted runners ## Why PR review advisors currently fail during SDK bootstrap before analysis. The pinned Ubuntu package reports `fdfind 9.0.0`, while the workflow expects `fd 9.0.0`. This was reproduced on both advisor lanes for #6711. ## Validation - `npx vitest run --project integration test/pr-review-advisor-workflow-boundary.test.ts` (19 passed) - `npm run checks` - pre-commit and pre-push hooks, including CLI typecheck <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected verification of the `fdfind` search tool version during Pi SDK installation. * Improved workflow boundary validation to align with the tool’s actual version output and mismatch messaging. * Ensured YAML is explicitly pinned and validated during Pi SDK installation and related workflow jobs, improving consistency and reliability of the PR review automation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
E2E Target Results —
|
| Test | Result |
|---|---|
| cloud-onboard | |
| concurrent-gateway-ports | |
| credential-sanitization | |
| inference-routing | ✅ success |
| network-policy | |
| onboard-repair | |
| onboard-resume | |
| security-posture |
|
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. |
E2E Target Results — ✅ All requested tests passedRun: 29287712163
|
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
Exact-head maintainer handoff for
The remaining red
@cjagwani please re-review the current head and, if satisfied with the independent E2E evidence, record the exact-diff control-plane exception. No merge action has been taken. |
# Conflicts: # src/lib/onboard/dashboard-port.test.ts # src/lib/onboard/dashboard-port.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/lib/actions/uninstall/run-plan.ts (1)
1170-1205: 📐 Maintainability & Code Quality | 🔵 TrivialRedundant re-computation of gateway-sibling inspection.
removeManagedSwaprecomputesinspectOtherGatewayEnvironments(paths, runtime)from scratch (line 1182), even thoughexecutePlanis already called with the equivalentscopedToSelectedGateway(computed once inrunUninstallPlan, line 1463). This duplicates a non-trivial filesystem walk + registry read and creates two copies of the same "other gateway remains" logic that could silently drift apart.♻️ Thread the already-computed result through instead of recomputing
-function removeManagedSwap(paths: UninstallPaths, runtime: UninstallRuntime): void { +function removeManagedSwap( + paths: UninstallPaths, + runtime: UninstallRuntime, + otherGatewayEnvironmentsRemain: boolean, +): void { ... - if (inspectOtherGatewayEnvironments(paths, runtime).otherGatewayEnvironmentsRemain) { + if (otherGatewayEnvironmentsRemain) { runtime.log( "Other NemoClaw gateway-port environments remain; keeping the host-shared /swapfile.", ); return; }And update the call site in
executePlanto passscopedToSelectedGatewaythrough.🤖 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/uninstall/run-plan.ts` around lines 1170 - 1205, Update removeManagedSwap to accept the already-computed scopedToSelectedGateway result instead of calling inspectOtherGatewayEnvironments. Use that passed result for the other-gateway-environments check, and update executePlan to forward scopedToSelectedGateway when invoking removeManagedSwap; leave the existing cleanup behavior unchanged.src/lib/state/legacy-port-migration.ts (2)
361-367: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify ambiguous-looking precedence in the ownership invariant check.
rawMetadata.rewriteLegacyRegistry !== selectedSandboxNames.length > 0is correct (>binds tighter than!==), but reads as if!==applies toselectedSandboxNames.lengthalone. This guards a resumable-migration consistency check, so misreading it during a future edit is a real risk.✏️ Proposed clarification
- rawMetadata.rewriteLegacyRegistry !== selectedSandboxNames.length > 0 || + rawMetadata.rewriteLegacyRegistry !== (selectedSandboxNames.length > 0) ||🤖 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/legacy-port-migration.ts` around lines 361 - 367, Clarify the first ownership invariant condition in the migration validation check by explicitly grouping the selectedSandboxNames.length > 0 boolean expression before comparing it with rawMetadata.rewriteLegacyRegistry. Preserve the existing logic and leave the other consistency conditions unchanged.
448-470: 🩺 Stability & Availability | 🔵 TrivialCrash windows leave orphaned
.preparing.*/.completed.*directories.If the process crashes between the atomic rename and the subsequent
fs.rmSynccleanup increateMigrationIntent(line 469) orremoveMigrationIntent(line 482), the renamed-away directory is never swept on a later run —readMigrationIntentonly looks for the literalMIGRATION_INTENTname, so these won't corrupt state, but they will accumulate as disk debt over repeated crashes. Consider a startup sweep for stale${MIGRATION_INTENT}.preparing.*/${MIGRATION_INTENT}.completed.*siblings.Also applies to: 477-484
🤖 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/legacy-port-migration.ts` around lines 448 - 470, Add startup cleanup for stale siblings matching the migration intent’s `${MIGRATION_INTENT}.preparing.*` and `${MIGRATION_INTENT}.completed.*` naming patterns, removing them before normal intent processing. Reuse the existing safe-path validation and filesystem conventions used by `createMigrationIntent`, `removeMigrationIntent`, and `readMigrationIntent`; preserve the active literal `MIGRATION_INTENT` directory and avoid deleting unrelated entries.test/install-gateway-state-root.test.ts (1)
40-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSanitize inherited
NEMOCLAW_*env vars for determinism.This spawns with the full parent
process.envand only blanks a fixed allowlist of port vars.test/uninstall.test.tsalready establishes asanitizedParentEnv()pattern that strips allNEMOCLAW_*keys before setting the ones under test; using the same approach here would avoid leaking unrelated inheritedNEMOCLAW_*config (e.g., from CI or a dev shell) into these gateway-state-root assertions.♻️ Proposed sanitization
+function sanitizedParentEnv(): NodeJS.ProcessEnv { + return Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith("NEMOCLAW_")), + ) as NodeJS.ProcessEnv; +} + function runInstallerFunctions( home: string, body: string, ): { output: string; status: number | null } { const result = spawnSync("bash", ["-c", `source "${INSTALLER}" >/dev/null\n${body}`], { encoding: "utf8", env: { - ...process.env, + ...sanitizedParentEnv(), BASH_ENV: "", ENV: "", HOME: home, - NEMOCLAW_BEDROCK_RUNTIME_ADAPTER_PORT: "", - NEMOCLAW_DASHBOARD_PORT: "", NEMOCLAW_GATEWAY_PORT: "9123", - NEMOCLAW_OLLAMA_PORT: "", - NEMOCLAW_OLLAMA_PROXY_PORT: "", - NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT: "", - NEMOCLAW_VLLM_PORT: "", }, });🤖 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/install-gateway-state-root.test.ts` around lines 40 - 61, Update runInstallerFunctions to build its child-process environment from sanitizedParentEnv(), removing all inherited NEMOCLAW_* variables before applying HOME and the specific port values required by these assertions. Reuse the existing sanitization pattern from test/uninstall.test.ts rather than maintaining a fixed list of variables.
🤖 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 `@docs/reference/commands.mdx`:
- Around line 3045-3056: In the “User-data preservation under ~/.nemoclaw/”
section, split each multi-sentence Markdown/MDX line into separate lines so
every period-terminated sentence occupies its own source line, preserving the
existing wording and formatting.
---
Nitpick comments:
In `@src/lib/actions/uninstall/run-plan.ts`:
- Around line 1170-1205: Update removeManagedSwap to accept the already-computed
scopedToSelectedGateway result instead of calling
inspectOtherGatewayEnvironments. Use that passed result for the
other-gateway-environments check, and update executePlan to forward
scopedToSelectedGateway when invoking removeManagedSwap; leave the existing
cleanup behavior unchanged.
In `@src/lib/state/legacy-port-migration.ts`:
- Around line 361-367: Clarify the first ownership invariant condition in the
migration validation check by explicitly grouping the
selectedSandboxNames.length > 0 boolean expression before comparing it with
rawMetadata.rewriteLegacyRegistry. Preserve the existing logic and leave the
other consistency conditions unchanged.
- Around line 448-470: Add startup cleanup for stale siblings matching the
migration intent’s `${MIGRATION_INTENT}.preparing.*` and
`${MIGRATION_INTENT}.completed.*` naming patterns, removing them before normal
intent processing. Reuse the existing safe-path validation and filesystem
conventions used by `createMigrationIntent`, `removeMigrationIntent`, and
`readMigrationIntent`; preserve the active literal `MIGRATION_INTENT` directory
and avoid deleting unrelated entries.
In `@test/install-gateway-state-root.test.ts`:
- Around line 40-61: Update runInstallerFunctions to build its child-process
environment from sanitizedParentEnv(), removing all inherited NEMOCLAW_*
variables before applying HOME and the specific port values required by these
assertions. Reuse the existing sanitization pattern from test/uninstall.test.ts
rather than maintaining a fixed list of variables.
🪄 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: 7c61d79b-093b-4179-83d3-45c0bf3b8e69
📒 Files selected for processing (18)
docs/manage-sandboxes/lifecycle.mdxdocs/reference/architecture.mdxdocs/reference/commands.mdxscripts/install.shsrc/lib/actions/uninstall/run-plan-gateway-segregation.test.tssrc/lib/actions/uninstall/run-plan-preserved-registry.test.tssrc/lib/actions/uninstall/run-plan.test.tssrc/lib/actions/uninstall/run-plan.tssrc/lib/onboard/dashboard-port.test.tssrc/lib/onboard/dashboard-port.tssrc/lib/state/legacy-port-migration.test.tssrc/lib/state/legacy-port-migration.tstest/fixtures/uninstall-prompt-pty-driver.tstest/install-gateway-state-root.test.tstest/install-openshell-upgrade-prompt.test.tstest/install-preexisting-sandbox-recovery.test.tstest/mcp-lifecycle-lock.test.tstest/uninstall.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/actions/uninstall/run-plan.test.ts
- docs/manage-sandboxes/lifecycle.mdx
- src/lib/onboard/dashboard-port.ts
- docs/reference/architecture.mdx
- scripts/install.sh
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
@cjagwani ready for exact-head re-review at Your latest shared-registry uninstall blocker is fixed: every valid legacy row not owned by the selected gateway now keeps cleanup scoped, including two non-default legacy rows after migrating one. The plan-level regression proves no Exact-head regular CI is green, CodeRabbit has no unresolved threads, and primary Advisor is 0 blockers / 0 warnings / 0 suggestions. Trusted E2E dispatch is currently blocked by the base-branch controller crash tracked in #6823; both failing gate jobs execute the broken |
## Summary - replace the unsupported TypeScript constructor parameter property in the PR E2E gate - add a regression that launches the controller with the exact `node --experimental-strip-types` runtime used by Actions ## Why Current `main` crashes every PR E2E gate during initialization with `ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX`, before it can evaluate the PR. This is visible on #6770, #6711, and #6672. ## Validation - `npx vitest run --project integration test/pr-e2e-gate-command.test.ts test/pr-e2e-gate-lifecycle.test.ts test/pr-e2e-gate-exceptions.test.ts test/pr-e2e-gate.test.ts test/pr-e2e-gate-workflow.test.ts` (108 passed) - `npm run build:cli` - pre-commit and pre-push hooks <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved validation for invalid end-to-end gate modes, including clearer error messaging and the correct failure status. * Prevented unsupported TypeScript syntax errors when running the gate command directly in Node’s experimental type-stripping mode. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Exact-head re-review request for The two prior uninstall reviews remain covered: selected-port uninstall preserves all sibling/shared resources, and every valid legacy row not owned by the selected gateway counts as a sibling before global cleanup. The fresh Advisor blocker is also fixed: supported Full The remaining gate is the expected exact-head control-plane exception because this PR changes trusted E2E code. Gate run 29305428800 names the exact head/base and selected jobs. @cjagwani, please re-review the completed fixes. @cv @ericksoa, the environment has no configured protection reviewers, so please use the typed |
|
New exact head after the main-only merge: The current E2E gate now requires a maintainer-authorized |
4 participants
Summary
A non-default
NEMOCLAW_GATEWAY_PORTpreviously shared the single~/.nemoclaw/host state root, so its sandbox registry, legacy credential-migration files, snapshots, and onboarding state collided with the default gateway's. This routes the host state root through onenemoclawStateRootresolver keyed off the existing gateway port, so a non-default port now owns a separate~/.nemoclaw/gateways/<port>/root while the default port stays byte-identical at~/.nemoclaw/.Related Issue
Fixes #3053
Changes
src/lib/state/state-root.ts—nemoclawStateRoot(home, gatewayPort)returns~/.nemoclawfor the default gateway port and~/.nemoclaw/gateways/<port>otherwise. It is keyed off the existingGATEWAY_PORTconstant so the state root and the gateway name (nemoclaw/nemoclaw-<port>) always agree.sandboxes.json), legacy credential-migration path, config-io permission-heal and remediation, onboard session, rebuild backups, blueprint cache, model router, local-inference adapter state and Ollama proxy token, share mounts, usage notice, and onboard-failure diagnostics.sourceclone and themanaged_swapmarker at the shared~/.nemoclaw/root — one CLI binary and one host swap file exist per machine, not per gateway.gateways/subtree so other environments survive even--destroy-user-data, prints the resolved state directory in its plan output, and skips host-swap teardown while other gateway-port environments remain.docs/manage-sandboxes/lifecycle.mdxanddocs/reference/architecture.mdx.This adds one resolver abstraction because the
~/.nemoclawroot was inlined at every state-path site with no single lever; segregation needs a single keyed source of truth. Its current consumer is #3053 (two segregated NemoClaw environments on one host). The default-port byte-identical contract is protected bysrc/lib/state/state-root.test.ts; the end-to-end per-port fork bysrc/lib/state/registry-port-segregation.test.ts; and the uninstall preservation plus host-swap guard bysrc/lib/actions/uninstall/run-plan-gateway-segregation.test.ts.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run test:changed— 154 files and 1,541 tests passed;npm run checks— passed;npm run typecheck:cli— 0 errorsnpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
~/.nemoclaw/gateways/<port>/(default remains in~/.nemoclaw/).uninstalladds--gateway <name>and applies port-scoped targeting/cleanup.--gatewaybehavior.