Skip to content

fix(messaging): defer channel start policy to rebuild - #10026

Merged
prekshivyas merged 4 commits into
mainfrom
codex/fix-hermes-channel-start-policy-order
Aug 24, 2026
Merged

fix(messaging): defer channel start policy to rebuild#10026
prekshivyas merged 4 commits into
mainfrom
codex/fix-hermes-channel-start-policy-order

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Defer a channel-start policy preset to the existing sandbox rebuild instead of applying it to the current sandbox first. This lets the replacement sandbox attach the credential provider before OpenShell accepts the credential-bound Discord policy.

Changes

  • Preserve preset validation, disclosure, and durable messaging-plan updates during channels start.
  • Let the existing rebuild compose the enabled channel preset and provider attachment in the replacement sandbox.
  • Add a deterministic Hermes Discord regression where the provider still exists gateway-side but is detached from the current all-disabled sandbox.
  • Keep the final rebuilt-state observation strict; credential binding, policy content, and retry behavior are unchanged.

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: completed the repository security rubric; credential custody and policy validation are unchanged, and the replacement sandbox must attach the exact provider before policy acceptance.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

Not applicable; this change does not modify scripts/prepare-dgx-station-host.sh.

  • Tested on DGX Station
  • Tested commit: not applicable
  • 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
  • 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: policy-channel conflict suite 34/34; messaging preset, preparation, and sandbox-create-plan suites 56/56; rebuild backup phase 17/17.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: not applicable to this focused two-file lifecycle-order correction.
  • 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: Senthil Ravichandran senthilr@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved channel startup reliability when applying credential-based policy presets.
    • Ensured channel settings are updated and rebuilt before policies take effect.
    • Prevented premature policy changes during interactive and non-interactive starts.
    • Improved policy preservation when restarting Google Chat channels.
    • Updated Teams startup behavior to defer policy changes when required.
  • Documentation
    • Clarified channel re-enabling, policy previews, and rebuild behavior.
  • Tests
    • Added coverage for Discord, Teams, and Google Chat scenarios.

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@github-code-quality

github-code-quality Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit c6b483a in the codex/fix-hermes-cha... branch remains at 96%, unchanged from commit 203a607 in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit c6b483a in the codex/fix-hermes-cha... branch remains at 83%, unchanged from commit 203a607 in the main branch.

Show a line coverage summary of the most impacted files.
File main 203a607 codex/fix-hermes-cha... c6b483a +/-
src/lib/inferen...ed-selection.ts 91% 90% -1%
src/lib/actions...licy-channel.ts 82% 81% -1%
src/lib/onboard...uild-context.ts 74% 74% 0%
src/lib/sandbox...rce-identity.ts 82% 82% 0%

Updated August 24, 2026 00:25 UTC

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: fa3a0c86-5562-4a74-b8f3-c5ab312a17d7

📥 Commits

Reviewing files that changed from the base of the PR and between 1255595 and c6b483a.

📒 Files selected for processing (2)
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/reference/commands.mdx

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


📝 Walkthrough

Walkthrough

channels start now carries the enabled policy plan through sandbox rebuild. The rebuild reattaches providers before credential-bound policy handling. Tests and documentation cover Hermes Discord, Teams, and Google Chat lifecycle behavior.

Changes

Channel policy start flow

Layer / File(s) Summary
Defer policy application until rebuild
src/lib/actions/sandbox/policy-channel.ts
Channel start validates and discloses the policy preset. It no longer applies the preset before rebuild or rolls back to a disabled state on failure.
Verify deferred policy lifecycle
src/lib/actions/sandbox/policy-channel-conflict.test.ts, test/channels-add-bridge-lifecycle.test.ts, docs/manage-sandboxes/manage-messaging-channels.mdx, docs/reference/commands.mdx
Tests verify plan mutation before rebuild, provider binding preservation, rebuild execution, preserved Google Chat policy, and the absence of direct policy application. Documentation describes deferred effects and policy application during rebuild.

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

Merge Risk: ⚪ Minimal · up to c6b48

The change defers channel policy application until the replacement sandbox has the credential provider attached, preserving validation and policy behavior; no actionable merge-blocking risk remains beyond normal checks.

Sequence Diagram(s)

sequenceDiagram
  participant ChannelStart
  participant MessagingPlan
  participant SandboxRebuild
  participant PolicyPreset
  ChannelStart->>PolicyPreset: Validate and disclose preset
  ChannelStart->>MessagingPlan: Record channel as enabled
  ChannelStart->>SandboxRebuild: Rebuild with enabled plan
  SandboxRebuild->>PolicyPreset: Apply preset after provider reattachment
Loading

Suggested reviewers: cv

🚥 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 summarizes the main change: deferring messaging channel policy application until sandbox rebuild.
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 0 files. (2 skipped: 2 unsupported.)
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 PR with unit tests
  • Commit unit tests in branch codex/fix-hermes-channel-start-policy-order

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

@senthilr-nv senthilr-nv added bug-fix PR fixes a bug or regression integration: hermes Hermes integration behavior integration: discord Discord integration or channel behavior area: messaging Messaging channels, bridges, manifests, or channel lifecycle labels Aug 23, 2026
@senthilr-nv senthilr-nv self-assigned this Aug 23, 2026
@senthilr-nv senthilr-nv added area: policy Network policy, egress rules, presets, or sandbox policy security v0.0.114 labels Aug 23, 2026
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Synthesis status: Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions

E2E guidance

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

Recommended E2E: inference-routing

Manual-only E2E: channels-add-remove, channels-stop-start, onboard-repair, onboard-resume, network-policy
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

Workflow run details

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

@github-actions

Copy link
Copy Markdown
Contributor

@prekshivyas
prekshivyas merged commit 711dbdd into main Aug 24, 2026
74 checks passed
@prekshivyas
prekshivyas deleted the codex/fix-hermes-channel-start-policy-order branch August 24, 2026 00:35
jyaunches added a commit that referenced this pull request Aug 24, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Complete the v0.0.114 documentation for user-visible behavior that the
cumulative post-merge workflow missed. The update covers managed-image
onboarding, managed vLLM GPU selection, messaging provider lifecycle,
paused channel status, Deep Agents tool discovery, Portable lifecycle
timing, HTTPS-only updates, and current Hermes runtime architecture.

## Changes

- Complete the v0.0.114 changelog for merged PRs #9323, #9862, #9913,
#9964, #10021, #10025, #10026, #10031, #10047, and #10052.
- Document managed vLLM GPU selection, resume constraints, and
GPU-specific preflight behavior.
- Document exact endpointless messaging-provider validation and stopped
Hermes Discord provider retention across rebuild.
- Document the paused detailed channel-status JSON contract and Portable
lifecycle timing output.
- Correct the Hermes managed-startup architecture description and Deep
Agents loaded MCP tool discovery behavior.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This PR updates public
documentation to match already tested source behavior and adds no
runtime code.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: An independent
documentation review checked credential custody, provider reuse,
stopped-channel effects, pairing claim boundaries, GPU selection,
variant routing, and recovery guidance against current source and tests.
The first review's blockers were corrected, and the final review is
recorded in the authoring evidence.
- [ ] 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: Not applicable
- Station profile/scenario: Not applicable
- Result: Not applicable
- Supporting evidence: This documentation-only change does not modify
`scripts/prepare-dgx-station-host.sh`.

## Verification
- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] 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
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — documentation-only change; targeted
runtime tests are not applicable
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not run; the PR changes documentation only
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [x] `npm run docs` builds without warnings (doc changes only) —
completed with 0 errors and 2 existing Fern warnings hidden by default
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— no new pages

---
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Select managed vLLM GPUs by index or UUID, with selections preserved
when resuming setup.
  - View detailed Portable recovery timing and action results.
  - Discover late-loaded managed tools through progressive tool search.

- **Bug Fixes**
  - Improved sandbox rebuild handling for stopped messaging channels.
- Strengthened provider validation, pairing checks, recovery handoffs,
and duplicate tool detection.
  - Added safer managed-image onboarding and approval-flow handling.
  - Update downloads and redirects now require HTTPS.

- **Documentation**
- Expanded guidance for onboarding, vLLM configuration, messaging
channels, recovery, architecture, and CLI commands.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: messaging Messaging channels, bridges, manifests, or channel lifecycle area: policy Network policy, egress rules, presets, or sandbox policy bug-fix PR fixes a bug or regression integration: discord Discord integration or channel behavior integration: hermes Hermes integration behavior security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants