fix(test): assert the routed teardown that #9098 protects

[VishnuR23]

Outcome

src/lib/actions/sandbox/destroy-flow.test.ts passes again, and the case that #9098 protects is still asserted.

Reason

The test asserted that withGatewayRouteMutationLock was never taken when registry removal did not complete:

expect(harness.withGatewayRouteMutationLockSpy).not.toHaveBeenCalled();

That worked while the routed Model Router teardown was the only holder of that lock. destroySandbox now takes the same lock for its final gateway cleanup, so the assertion fails on main:

FAIL cli src/lib/actions/sandbox/destroy-flow.test.ts
  > does not stop the routed sandbox proxy when registry removal does not complete (#9098)
AssertionError: expected "withGatewayRouteMutationLock" to not be called at all,
                but actually been called 1 times
  ❯ src/lib/actions/sandbox/destroy-flow.test.ts:812:57

The protected behavior is intact. removeSandboxRegistryEntryOutcome maps a removeSandbox result of false to { status: "not-found", removed: false }:

return removeSandbox(sandboxName)
  ? { status: "complete", removed: true }
  : { status: "not-found", removed: false };

removed stays false, so the routed teardown block stays skipped and stopModelRouterForDestroyedSandbox is never called. registryEntryAbsent accepts not-found as well as complete, so the final gateway cleanup still runs and takes the lock once — the retry case its own comment describes:

// A retry after successful registry removal still owns final gateway cleanup.

So the lock acquisition is correct and the assertion on it was the stale part.

Changes

• Assert stopModelRouterForDestroyedSandbox is not called, which is the behavior [Ubuntu 24.04][Inference] Model Router proxy survives sandbox destroy and blocks the next Model Router onboard #9098 protects, and stop asserting on the lock that now guards two paths.
• Record why the lock no longer distinguishes the routed teardown from the final gateway cleanup.

No production change.

Verification

Confirmed the lock acquisition belongs to the final gateway cleanup and not the routed teardown, by instrumenting the spies in the failing case before making the change:

lock calls  = ["nemoclaw-19080"]
routerStops = 0

The router teardown never runs, so the single acquisition is the final cleanup.

• npx vitest run --project cli src/lib/actions/sandbox/destroy-flow.test.ts — 72 passed (1 failed before).
• npx vitest run --project cli destroy-flow.test.ts destroy-podman-cleanup-flow.test.ts destroy.test.ts — 3 files, 87 tests passed. destroy-podman-cleanup-flow.test.ts exercises the same removeSandboxResult: false setup.
• npx oxlint and npx oxfmt --check on the changed file — passed.
• Normal commit hooks passed on the published commit.
• The diff contains no secrets, API keys, or credentials.

Where this reproduces

Not macOS-only. Run 35499788663 on main at f2c0316fc fails the same test with the same message in macOS compatibility (1/4) and WSL compatibility (1/4). The platform-vitest-main.yaml workflow has failed on each of its six most recent main runs.

That run also reports two failures this PR does not address, in case they are not already tracked:

• src/lib/onboard/gateway/automatic-port-completion.test.ts — promotes the trusted pending marker after direct onboarding succeeds (#10824)
• test/e2e/support/openshell-exact-main-driver-config.test.ts — resolves one canonical executable set for CLI, gateway, and sandbox (#11547)

🤖 Generated with Claude Code

https://claude.ai/code/session_01VAoaWK8T98zLhCzNUk6RZF

Summary by CodeRabbit

• Tests
  • Updated routed teardown test coverage to verify that the model-router proxy remains active.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 154307f7-eb20-4514-a0f9-3e0366aaed3b

📥 Commits

Reviewing files that changed from the base of the PR and between f2c0316 and 1c61fc8.

📒 Files selected for processing (1)

• src/lib/actions/sandbox/destroy-flow.test.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.

---

📝 Walkthrough

Walkthrough

The routed teardown test no longer asserts that withGatewayRouteMutationLock is unused. It retains the assertion that the model-router proxy is not stopped and documents possible gateway cleanup locking.

Changes

Sandbox teardown

Layer / File(s)	Summary
Teardown assertion update src/lib/actions/sandbox/destroy-flow.test.ts	The test removes the assertion that withGatewayRouteMutationLock is not called. It retains the model-router proxy assertion and documents final gateway cleanup behavior.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: ericksoa

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies a test fix for the routed teardown behavior protected by #9098. It accurately describes the main change and is concise.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[VishnuR23]

Closing this — the failure it repaired no longer reproduces, so the change would now remove an assertion that holds.

When I opened this, does not stop the routed sandbox proxy when registry removal does not complete (#9098) failed on main at f2c0316fc, locally and in macOS compatibility (1/4) and WSL compatibility (1/4) of run 35499788663.

On main at 41b9d9fa9 the whole file passes, with the original assertion still in place:

npx vitest run --project cli src/lib/actions/sandbox/destroy-flow.test.ts
Test Files  1 passed (1)
     Tests  72 passed (72)

CI agrees. Run 36650369110 reports ✓ cli src/lib/actions/sandbox/destroy-flow.test.ts (72 tests).

No commit changed the test or its harness setup in that range. The gate moved from registry.listSandboxes() to listRegisteredSandboxes() in #11945 and #12336, which changes what the harness observes for the empty-registry condition, so the final gateway cleanup no longer runs in this scenario and the lock is not taken. expect(harness.withGatewayRouteMutationLockSpy).not.toHaveBeenCalled() is therefore accurate again, and merging this PR would delete a passing assertion for no reason.

One correction to carry forward, since the analysis in the PR body still holds and may be useful if this resurfaces: the lock was never exclusive to the routed teardown. I instrumented the spies in the failing case at the time and recorded lock calls = ["nemoclaw-19080"] with routerStops = 0, so the single acquisition belonged to the final gateway cleanup and the #9098 guarantee was intact throughout. If that cleanup path ever becomes reachable again under removeSandbox → false, the assertion will fail again for the same non-defect reason.

platform-vitest-main.yaml is still failing on main — its six most recent runs are red — but with a different set now: src/lib/actions/global.test.ts, src/lib/actions/sandbox/mcp-bridge-runtime-command.test.ts, src/lib/onboard/gateway/automatic-port-completion.test.ts, test/e2e/support/e2e-progress-outcome.test.ts, and test/e2e/support/openshell-exact-main-driver-config.test.ts. None are addressed here.