fix(onboard): preserve lifecycle channel selections

[deepujain]

Summary

Preserve explicit messaging lifecycle selections when a reused sandbox or completed registry resume has no matching host credential. This complements the retired-channel filtering merged in #9327 without reintroducing any of that PR's now-upstream implementation or tests.

Related Issue

Fixes #9283

Changes

• Keep a lifecycle-authored selection from channels add, remove, start, or stop authoritative in the reused-sandbox path.
• Apply the same lifecycle exception to completed registry resume reconciliation.
• Add focused regression tests for both lifecycle-owned paths; retired host-backed and QR-channel controls remain supplied by fix(onboard): drop retired channels from reused messaging selections #9327.

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

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 — npx vitest run --project cli src/lib/onboard/machine/handlers/sandbox-messaging.test.ts (30 passed)
• Applicable broad gate passed — npm run typecheck:cli; npm run lint
• 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: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Preserved lifecycle-selected Discord channels when reusing a sandbox, even if the host credential is unavailable.
  • Preserved selected channels when resuming completed sandbox setups.
  • Prevented unnecessary messaging setup during completed resume flows when configuration is unavailable.
  • Improved consistency between sandbox reuse and registry-based resume behavior.

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

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: 3bb0f91f-696c-4d6a-9592-341f49958edf

📥 Commits

Reviewing files that changed from the base of the PR and between acb324e and 57ad709.

📒 Files selected for processing (2)

• src/lib/onboard/machine/handlers/sandbox-messaging.test.ts
• src/lib/onboard/machine/handlers/sandbox-messaging.ts

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

• src/lib/onboard/machine/handlers/sandbox-messaging.ts
• src/lib/onboard/machine/handlers/sandbox-messaging.test.ts

Included review availability: Your plan includes up to 12 reviews per rolling hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The messaging reuse and registry-resume paths now preserve lifecycle-recorded Discord channel selections when host credentials are absent. Tests cover sandbox reuse and completed registry resume, including the skipped messaging setup.

Changes

Sandbox messaging reuse

Layer / File(s)	Summary
Reuse selection filtering src/lib/onboard/machine/handlers/sandbox-messaging.ts	Lifecycle-recorded selections bypass unconfigured-host filtering during reuse and registry-authority resume. Other selections retain the existing filtering behavior.
Ready sandbox regression coverage src/lib/onboard/machine/handlers/sandbox-messaging.test.ts	Tests verify that add-channel Discord selections remain selected without host credentials and that completed resume skips messaging setup.

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

Merge Risk: ⚪ Minimal · up to 57ad7

The change preserves lifecycle channel selections while removing stale host-backed selections during reused-sandbox onboarding. No actionable merge-blocking risk remains after normal checks and review.

🚥 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 describes the primary change: preserving lifecycle-selected messaging channels during onboarding reuse.
Linked Issues check	✅ Passed	The changes remove stale host-backed channels while preserving authoritative lifecycle selections, which satisfies issue #9283.
Out of Scope Changes check	✅ Passed	The tests and implementation changes remain within the linked issue objectives and target the sandbox messaging reuse paths.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches 💡 1

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch fix/9283-prune-unconfigured-channel

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

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

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

1 semantic terminology decision

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

• define — lifecycle-selected at src/lib/onboard/machine/handlers/sandbox-messaging.test.ts:427: Define lifecycle-selected at its first explanatory use, or use a phrase that states that the registry lifecycle workflow selected the channel.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: None

Manual-only E2E: cloud-onboard, onboard-repair, onboard-resume
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

3 optional E2E recommendations

• channels-add-remove
• channels-stop-start
• onboard-policy-preset-sequencing

Blockers

PRA-1 Blocker — Do not exempt every channel in a lifecycle plan from host-configuration filtering

• Location: src/lib/onboard/machine/handlers/sandbox-messaging.ts:443
• Category: correctness
• Problem: The lifecycle-workflow predicate skips host-configuration filtering for every active channel in the plan. A remove-channel plan can retain an unrelated active host-backed channel, so a later absent credential leaves that channel selected and prevents policy pruning.
• Impact: A sandbox can retain the network egress preset for a host-backed messaging channel after the operator removes its credential.
• Fix: Preserve lifecycle authority only for channels selected by the lifecycle operation. Apply the existing host-configuration filter to unrelated and subsequently unconfigured host-backed channels.
• Verification: Inspect a remove-channel plan that retains an active Slack channel, then trace its selected channel through reconcileReusedSandboxMessaging and policies.ts.
• Test coverage: Add reused-plan and completed-registry-resume tests with a lifecycle plan that retains an active host-backed channel whose credential is absent. Assert that the channel is omitted from selectedChannels and reaches policy pruning as unconfigured.
• Evidence: src/lib/onboard/machine/handlers/sandbox-messaging.ts:85-92 classifies all four lifecycle workflow values as lifecycle selections. src/lib/onboard/machine/handlers/sandbox-messaging.ts:443-445 returns every active channel without host-configuration filtering for any such plan. src/lib/messaging/compiler/workflow-planner.test.ts:400-434 shows a remove-channel plan retaining active Slack while recording disabled WeChat. src/lib/onboard/machine/handlers/policies.ts:180-193 prunes egress only after an absent channel is omitted from selectedMessagingChannels.

Workflow run details

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

[wscurran]

✨ Thanks for the fix. This corrects the sandbox reuse path to prune unconfigured host-backed messaging channels before policy reconciliation, preventing stale egress preset re-application. Maintainers will review the onboarding, messaging, and policy changes.

---

Related open issues:

• #9283 [Ubuntu 24.04][Policy&Network] messaging channel preset is still re-applied after the channel is unconfigured by every available means

---

Related open issues:

• #9283 [Ubuntu 24.04][Policy&Network] messaging channel preset is still re-applied after the channel is unconfigured by every available means

[prekshivyas]

Blocking correctness issue: reconcileReusedSandboxMessaging only filters selectedChannels; it leaves the durable plan active and computes changed from that unchanged plan (src/lib/onboard/machine/handlers/sandbox-messaging.ts:428-440). As a result, removing the last host input can return selectedChannels: [] with changed: false, so reuse does not persist an updated session/checkpoint and a later reader can re-enable the channel and its egress preset. Please mark the removed channel inactive/unselected/disabled in the plan, persist that plan, and cover the durable Ready-sandbox path. PR #9296 demonstrates the missing durable behavior and is currently the more complete candidate for #9283.

Cross-issue sweep: no additional candidate issues found.

Security review: secrets/credentials — PASS; input validation/sanitization — PASS; authentication/authorization — PASS; dependencies — PASS; error handling/logging — PASS; cryptography/data protection — PASS; configuration/security headers — PASS; security testing — PASS; system security — PASS.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[deepujain]

Rebased onto current main and narrowed this after #9327 landed. The duplicate retired-channel code and tests are gone; the remaining guards preserve explicit channel lifecycle selections in reused and completed-resume paths. Focused tests (30), typecheck, and lint pass.

[deepujain]

Closing this after #9296 landed. Its durable plan update covers #9283 and confirms that onboarding should still retire host-unconfigured channels after lifecycle commands, so the remaining exception here would conflict with the merged design.