fix(messaging): preserve forward-list failures during cleanup

[aryansk]

Summary

Before this change, a failed OpenShell forward-list query could be treated as an empty list during messaging recovery. The recovery path could then run a sandbox-scoped stop or accept an unverified live listener. The change preserves unknown ownership, skips the stop, prevents startup verification, and reports accurate recovery guidance.

Related Issue

Fixes #8522

Changes

• Allow the shared forward-list runner to return null and classify that result as list-failed before ownership parsing.
• Preserve the nullable result in the fixed-forward cleanup adapter instead of converting it to an empty list.
• Make the concrete messaging capture nonfatal and return null for a failed OpenShell query.
• Preserve failed ownership queries through startup polling so a live port cannot satisfy verification without ownership evidence.
• Keep successful empty-list cleanup sandbox-scoped.
• Report the preceding OpenShell error or port conflict before directing the operator to reconnect.
• Add focused coverage for the shared helper, concrete messaging adapter, startup verifier, and both rebuild output paths.

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: The command, configuration, recovery sequence, and supported surface are unchanged. Existing documentation already requires ownership reconciliation before accepting an active forward.
• 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: Nine-category maintainer review at db85a5bcd found no remaining issue. The change narrows the OpenShell ownership boundary, adds no credential flow or dependency, and keeps failures fail-closed.
• 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: no-docs-needed
• Evidence: No documentation files changed. The implementation preserves failed OpenShell ownership queries as unverified state, avoids cleanup without ownership evidence, and gives self-contained rebuild remediation. Command syntax, configuration, recovery sequence, agent applicability, and support scope remain unchanged.
• Agent: Pi CLI

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh does not change.
• Station profile/scenario: Not applicable.
• Result: Not applicable.
• Supporting evidence: Not applicable.

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub — contributor action required: add the contributor's DCO declaration. Commit 1e0c4ac453475b09d3b33885e352d3e410482d9f remains unsigned and cannot be replaced because force-push is disabled.
• 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 — npx vitest run --project cli src/lib/actions/sandbox/messaging-host-forward-lifecycle.test.ts src/lib/actions/sandbox/rebuild-finalization.test.ts src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts src/lib/onboard/forward-cleanup.test.ts src/lib/onboard/forward-start.test.ts: 5 files and 67 tests passed.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: not run; focused source tests cover the changed behavior.
• 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)

---

[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]

📝 Walkthrough

Walkthrough

The cleanup contract now preserves null forward-list results. Cleanup returns list-failed and skips stopping when listing fails. The messaging adapter passes nullable capture results directly, with regression coverage for failed and empty listings. Rebuild recovery messages now provide corrective guidance.

Changes

Forward cleanup failure handling

Layer / File(s)	Summary
Nullable cleanup contract src/lib/onboard/forward-cleanup.ts, src/lib/onboard/forward-cleanup.test.ts	ForwardListRunner accepts null. Cleanup classifies thrown errors and null results as list-failed without invoking the stop command. Tests cover successful empty listings and nullable failures.
Messaging adapter propagation and lifecycle coverage src/lib/onboard/agent-fixed-forward.ts, src/lib/actions/sandbox/messaging-host-forward-lifecycle.ts, src/lib/actions/sandbox/messaging-host-forward-lifecycle.test.ts	The cleanup callback passes nullable OpenShell capture results directly. Captured OpenShell commands force ignoreError: true. Lifecycle tests cover failed listing and sandbox-scoped stopping for empty listings.
Rebuild recovery guidance src/lib/actions/sandbox/rebuild-finalization.ts, src/lib/actions/sandbox/rebuild-finalization.test.ts, src/lib/actions/sandbox/rebuild-post-restore-phase.ts, src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts	Incomplete recovery messages now reference preceding OpenShell errors or port conflicts before instructing users to run nemoclaw alpha connect.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• NVIDIA/NemoClaw#8505: Overlaps in rebuild recovery messaging and related tests.
• NVIDIA/NemoClaw#8529: Overlaps in forward cleanup, nullable capture handling, and messaging-host capture behavior.

Suggested labels: area: onboarding, bug-fix

Suggested reviewers: ericksoa, cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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
Linked Issues check	✅ Passed	The changes satisfy issue #8522 by preserving null failures, returning "list-failed", skipping stops on failure, and retaining empty-list cleanup behavior.
Out of Scope Changes check	✅ Passed	The changes remain within messaging forward cleanup and recovery behavior, including focused tests and related corrective error messages.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving forward-list failures during cleanup.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

PR Review Advisor — Informational

Advisor assessment: Informational / low confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: Same-session synthesis validation failed; the advisor result is incomplete.

Model lanes

• GPT-5.6 Terra (primary): Completed · low confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 1 blocker · 1 warning · 0 suggestions

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• established — list-failed at src/lib/onboard/forward-cleanup.ts:68: Keep `list-failed` for the ownership-enumeration failure classification.
• established — port conflict at src/lib/actions/sandbox/rebuild-finalization.ts:146: Keep `port conflict` to distinguish a port-ownership failure from an OpenShell error in rebuild remediation.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: managed-image-multiarch-startup, managed-image-protected-runtime, channels-add-remove, channels-stop-start, onboard-repair, onboard-resume, rebuild-openclaw, state-backup-restore, cloud-onboard

Workflow run details

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

[cv]

Contributor compliance blocks this PR: add the required Signed-off-by: declaration and replace the unsigned commit with GitHub-verified history. The draft also has no required PR workflows. Before marking it ready, cover the concrete messaging adapter that converts an OpenShell command failure into the nullable list result; the current test proves only the shared helper seam. PR #8529 addresses the same issue, so coordinate the intended surviving contribution rather than maintaining two partial implementations.

[cv]

A fresh branch and PR are required because this PR contains unsigned commit 1e0c4ac453475b09d3b33885e352d3e410482d9f and force-push is disabled. The repaired code and validation are complete.

Before a maintainer can transfer your material contribution into the replacement PR, repository attribution policy requires you to add your own DCO declaration to this PR description. Please add this line yourself if it matches your contribution identity:

Signed-off-by: Aryan Singh K. <70511529+aryansk@users.noreply.github.com>

Do not rewrite this branch. After the declaration is present, the replacement PR can preserve your exact source-commit identity and supersede #8526 with verified history.