fix(cli): stop the packaged gateway service on final sandbox destroy

[laitingsheng]

Summary

Final-sandbox gateway cleanup branched only on whether the gateway is externally supervised, so a gateway owned by the packaged OpenShell gateway service reached the PID-file reaper instead, and that reaper had nothing to stop because a healthy managed-service start clears the PID file. Destroy therefore reported success while the service manager kept the gateway port bound. Cleanup now stops the packaged service before it reaps host processes, and refuses to continue when the service survives.

Related Issue

Fixes #7904

Changes

• src/lib/onboard/docker-driver-gateway-service.ts: stopOpenShellGatewayUserService stops the resolved gateway service through the same manager the start path uses, systemctl --user stop or brew services stop. The command the start path already ran inline moves into a shared runStopService, so start behaviour is unchanged. The stop reports attempted: false when no service is installed or the platform has no service manager, so a caller can fall through to its existing teardown instead of failing. Covered by docker-driver-gateway-service.test.ts.
• src/lib/actions/sandbox/destroy-gateway.ts: cleanupGatewayAfterLastSandbox stops the packaged service when the resolved owner names packaged-service as its source, before the PID-file reaper. The service is stopped, not disabled or removed, because destroy is not uninstall and the next onboarding run starts it again. A stop that was attempted and failed throws with the service status command and aborts before gateway registration and volume removal, matching the existing surviving-listener refusal. A standalone or externally supervised gateway takes the previous paths untouched. Covered by destroy-gateway.test.ts and test/cli/destroy-gateway-cleanup.test.ts.
• docs/reference/commands.mdx, docs/reference/troubleshooting.mdx: document the service stop in the destroy reference, and add the remediation for a gateway port that stays bound after the final destroy.

Type of Change

• 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)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification:
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: blocked
• Evidence: docs/reference/commands.mdx, docs/reference/troubleshooting.mdx; the review is blocked because no documentation writer subagent was available in the authoring session.
• Agent: Claude Code

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit:
• Station profile/scenario:
• Result:
• Supporting evidence:

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/actions/sandbox/destroy-gateway.test.ts src/lib/onboard/docker-driver-gateway-service.test.ts — 40 passed. npx vitest run --project integration test/cli/destroy-gateway-cleanup.test.ts — 12 passed. npx vitest run --project integration test/destroy-wipe-sandbox-state.test.ts test/gateway-final-failure-cleanup.test.ts — 26 passed. npx vitest run --project cli src/lib/actions/sandbox src/lib/onboard --maxWorkers=4 — 545 of 546 files passed; src/lib/onboard/docker-driver-gateway-jwt-bundle.test.ts fails the same way on an unmodified checkout of this base. npm run typecheck:cli, npx biome check, and npm run checks:repository are clean. npm run docs reports 0 errors and 2 warnings, both pre-existing accent-colour contrast warnings from the shared theme configuration rather than the changed pages.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run 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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Improved destroy --cleanup-gateway so that when the packaged OpenShell gateway is systemd-managed, the gateway service is stopped before host-process reaping—freeing the gateway port for later onboarding.
  • If the service cannot be stopped, destroy now exits non-zero after sandbox/registry deletion, prints the service status command, and skips gateway and volume removal.

• Documentation

  • Updated destroy and Troubleshooting guidance for packaged OpenShell gateway cleanup and Linux remediation, including consistent headings and an explicit systemctl --user stop openshell-gateway command for package installs.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8908bc40-70f3-43df-9e32-c1ccf1595573

📥 Commits

Reviewing files that changed from the base of the PR and between 8de7f99 and e73701b.

📒 Files selected for processing (4)

• docs/reference/troubleshooting.mdx
• src/lib/onboard/docker-driver-gateway-service.test.ts
• src/lib/onboard/docker-driver-gateway-service.ts
• test/cli/destroy-gateway-cleanup.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• docs/reference/troubleshooting.mdx
• src/lib/onboard/docker-driver-gateway-service.test.ts

---

📝 Walkthrough

Walkthrough

Final sandbox cleanup now stops packaged OpenShell gateway services before host process reaping. The change adds platform-specific stop handling, failure propagation, tests for packaged and standalone gateways, CLI coverage, and updated operational documentation.

Changes

Gateway cleanup

Layer / File(s)	Summary
Service stop implementation src/lib/onboard/docker-driver-gateway-service.ts, src/lib/onboard/docker-driver-gateway-service.test.ts	Adds structured service-stop results, shared Homebrew/systemd helpers, the packaged gateway stop function, and platform-specific tests.
Final destroy orchestration src/lib/actions/sandbox/destroy-gateway.ts, src/lib/actions/sandbox/destroy-gateway.test.ts	Stops packaged gateway services before host reaping and prevents remaining gateway and volume cleanup when stopping fails.
CLI validation and operational guidance test/cli/destroy-gateway-cleanup.test.ts, docs/reference/commands.mdx, docs/reference/troubleshooting.mdx	Validates Linux service stopping during CLI destroy and documents cleanup behavior, failure handling, and platform-specific commands.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: area: sandbox

Suggested reviewers: cv, brandonpelfrey

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Clearly states the PR stops the packaged gateway service during final sandbox destroy.
Linked Issues check	✅ Passed	For [#7904], final cleanup stops the packaged gateway before host process reaping and fails if the stop does not complete.
Out of Scope Changes check	✅ Passed	The docs, tests, and service-helper refactor all support the gateway-stop cleanup flow and stay within the issue scope.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Fix failing CI checks

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/destroy-packaged-gateway-service

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit e73701b in the fix/destroy-packaged... branch remains at 96%, unchanged from commit 45d7e1a in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit e73701b in the fix/destroy-packaged... branch remains at 81%, unchanged from commit 45d7e1a in the main branch.

Show a code coverage summary of the most impacted files.

File	main 45d7e1a	fix/destroy-packaged... e73701b	+/-
src/lib/onboard/docker-cdi.ts	80%	70%	-10%
src/lib/onboard...host-anchors.ts	94%	90%	-4%
src/lib/actions...eway-restart.ts	95%	94%	-1%
src/lib/onboard/preflight.ts	80%	80%	0%
src/lib/onboard...eway-service.ts	82%	83%	+1%
src/lib/actions...light-guards.ts	86%	90%	+4%
src/lib/onboard...box-gpu-mode.ts	92%	97%	+5%
src/lib/actions...confirmation.ts	69%	79%	+10%
src/lib/actions...ocker-health.ts	65%	82%	+17%
src/lib/onboard...box-prebuild.ts	74%	92%	+18%

---

_{Updated July 30, 2026 14:47 UTC}

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-7907.docs.buildwithfern.com/nemoclaw

[coderabbitai]

Actionable comments posted: 4

🤖 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/troubleshooting.mdx`:
- Line 996: Update the troubleshooting heading “Gateway port stays bound after
destroying the last sandbox” to title case: “Gateway Port Stays Bound After
Destroying the Last Sandbox”.
- Around line 1009-1013: Update the Linux service-stop example in the
troubleshooting documentation to include the package-install command using
openshell-gateway, alongside the existing nemoclaw-openshell-gateway tarball
command.

In `@src/lib/onboard/docker-driver-gateway-service.ts`:
- Around line 688-691: Update the stop flow around runStopService to call
validateSystemdServiceIdentity for systemd services before issuing the stop
command, validating both FragmentPath and ExecStart. If identity validation
fails, return the existing failed-stop result without invoking systemctl stop,
while preserving the homebrew path and successful-stop behavior; add a test
covering a mismatched unit identity.

In `@test/cli/destroy-gateway-cleanup.test.ts`:
- Around line 393-395: Update the systemctlOutput assertion in the cleanup test
to require the exact unit name nemoclaw-openshell-gateway after the --user stop
arguments, while preserving the existing check that disable is not invoked.

🪄 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: e801f7d4-f408-4318-994c-cdd0bbac2299

📥 Commits

Reviewing files that changed from the base of the PR and between deefc61 and 8de7f99.

📒 Files selected for processing (7)

• docs/reference/commands.mdx
• docs/reference/troubleshooting.mdx
• src/lib/actions/sandbox/destroy-gateway.test.ts
• src/lib/actions/sandbox/destroy-gateway.ts
• src/lib/onboard/docker-driver-gateway-service.test.ts
• src/lib/onboard/docker-driver-gateway-service.ts
• test/cli/destroy-gateway-cleanup.test.ts

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 fewer warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume, cloud-onboard

1 optional E2E recommendation

• sandbox-operations

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Cover retry after a packaged-service cleanup partial failure

• Location: src/lib/actions/sandbox/destroy-gateway.test.ts:294
• Category: tests
• Problem: The new packaged-service cleanup path stops the service before host reaping, but no test retries that path after a later cleanup step fails.
• Impact: A failure after the service stops can leave gateway registration or cluster-volume cleanup incomplete. Without path-specific retry coverage, a future change can break convergence and leave stale gateway resources or ports.
• Recommendation: Add a packaged-service-owner test that succeeds at service stop, fails once in host reaping or gateway removal, then retries cleanup and verifies service stop is repeated, gateway removal succeeds, and the cluster volume is removed.
• Verification: Inspect destroy-gateway.test.ts: the existing retry test at line 391 uses the default standalone owner, while packaged-service tests at lines 294-356 cover only immediate success or stop failure.
• Test coverage: Mock a packaged-service owner and a one-time host-reaper or gateway-remove failure. Invoke cleanup twice and assert the second invocation completes service stop, gateway removal, and volume removal.
• Evidence: src/lib/actions/sandbox/destroy-gateway.ts:111-120 adds service stop before host reaping for packaged-service owners. src/lib/actions/sandbox/destroy-gateway.test.ts:294-356 tests packaged-service ordering and immediate stop failure only. src/lib/actions/sandbox/destroy-gateway.test.ts:391-400 tests retry after partial failure with the default standalone owner.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[cjagwani]

Approved on exact head e73701b. The accepted #7904 bug scope is implemented with fail-closed packaged-service identity validation before stop, targeted regression coverage, all 54 current checks green, and the trusted onboard-repair/onboard-resume E2E plan successful. GitHub reports MERGEABLE; the checker’s only failure is base currency, covered by the maintainer stale-base waiver.