Skip to content

fix(messaging): fail closed on channel conflicts - #7811

Merged
jyaunches merged 10 commits into
mainfrom
fix/7808-slack-conflict-abort
Jul 29, 2026
Merged

fix(messaging): fail closed on channel conflicts#7811
jyaunches merged 10 commits into
mainfrom
fix/7808-slack-conflict-abort

Conversation

@sandl99

@sandl99 sandl99 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Messaging credential conflicts now fail closed for every channel during onboarding, rebuild, and channels add. Channel-owned conflicts also honor their declarative onFailure: "abort" policy. The only conflict bypass is the explicit channels add <channel> --force flag.

Related Issue

Fixes #7808.
Concurrency evidence: onboarding and rebuild acquire the same cross-process onboarding lock before messaging preflight and hold it through activation/finalization; channels add remains protected only by its per-sandbox lock.

Changes

  • Abort all generic credential conflicts, legacy entries whose credential hash is unavailable, and incomplete required conflict checks without an interactive override.
  • Convert onboarding and rebuild messaging-registry read failures into deliberate fail-closed aborts with recovery guidance and no override.
  • Propagate the channel ID and hook onFailure policy through MessagingHookConflictError, so abort-policy conflicts stop onboarding, rebuild, and channel add while skip-channel keeps its existing behavior.
  • Preserve channels add <channel> --force as the explicit override for credential, incomplete-check, and abort-policy hook conflicts.
  • Cover generic credentials, Slack shared gateways and cross-gateway credentials, Microsoft Teams shared webhook ports, rebuild preflight, failure-policy propagation, and secret redaction with regression tests.
  • Document fail-closed behavior and the duplicate-consumer or shared-resource risk accepted by --force.

Existing concurrency boundary

Refs #7808 — concurrency evidence: nemoclaw onboard and rebuild acquire the same atomic cross-process onboarding lock before messaging preflight and hold it through finalization or recreation. channels add holds its per-sandbox mutation lock and only stages sandbox-scoped credential providers and the desired messaging plan; it does not attach providers or activate the channel. Activation occurs only during rebuild, whose locked preflight checks the staged registry, so concurrent pending adds cannot create two active consumers. src/lib/state/onboard-session-cross-process-lock.test.ts verifies that a competing CLI process is rejected before gateway creation.

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: Codex security review passed with no findings at d118ba7c5: fix(messaging): fail closed on channel conflicts #7811 (comment)
  • 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: docs-updated
  • Evidence: Reviewed the complete PR behavior and documentation through 98d0cf622. The documentation writer confirmed this test-only fixture commit does not change production behavior or user-facing documentation. The exact onboard messaging suite passed 16 tests; the test-size check, diff check, and normal commit hooks passed.
  • Agent: Codex CLI

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 check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — original focused suite (83 passed), followed after review fixes by npx vitest run src/lib/onboard/messaging-conflict-guard.test.ts src/lib/onboard/sandbox-messaging-preflight.test.ts src/lib/actions/sandbox/rebuild-messaging-conflict-preflight.test.ts (20 passed)
  • 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)

npm run docs passed with zero errors; Fern reported two warnings.


Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Messaging channel conflict handling is now fail-closed: onboarding, rebuilds, and channel additions hard-abort on credential/shared-resource conflicts and related verification failures, removing any interactive “continue” flow.
    • --force is now restricted to channels add and only bypasses the duplicate-consumer/shared-resource risk when explicitly accepted.
    • Conflict/error output is tightened to avoid leaking credential/token details, with consistent abort guidance.
  • Documentation
    • Updated cross-sandbox conflict and --force resolution guidance across messaging channels (Slack/Teams) with clearer abort conditions.
  • Tests
    • Expanded coverage for abort vs --force, prompt absence, registry-read/verification failure paths, and stricter error-message assertions.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 added area: docs Documentation, examples, guides, or docs build area: messaging Messaging channels, bridges, manifests, or channel lifecycle labels Jul 29, 2026
@sandl99 sandl99 self-assigned this Jul 29, 2026
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Messaging conflict handling now aborts by default for detected or unverifiable conflicts, propagates channel failure context through hooks, updates Slack channel-add behavior, and aligns tests and documentation with explicit --force paths.

Changes

Messaging conflict enforcement

Layer / File(s) Summary
Conflict error propagation
src/lib/messaging/hooks/errors.ts, src/lib/messaging/applier/hook-phases.ts, src/lib/messaging/applier/setup-applier.test.ts
MessagingHookConflictError carries channel and failure-mode context, which hook execution preserves and tests validate.
Onboarding conflict guard
src/lib/onboard/messaging-conflict-guard.ts, src/lib/onboard/*test.ts, src/lib/actions/sandbox/rebuild-messaging-conflict-preflight.test.ts
Credential, registry-verification, and abort-on-failure pre-enable conflicts emit errors and abort without interactive continuation.
Sandbox channel policy and guidance
src/lib/actions/sandbox/policy-channel.ts, src/lib/actions/sandbox/policy-channel-conflict.test.ts, docs/manage-sandboxes/*.mdx, docs/reference/*.mdx
Channel addition aborts on detected or incomplete conflict checks unless --force is used; Slack checks, output redaction, and operator guidance are updated.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested labels: area: messaging, bug-fix, area: sandbox, area: integrations, area: security, feature, v0.0.98

Suggested reviewers: cv, prekshivyas

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant channels_add
  participant checkChannelAddConflict
  participant MessagingRegistry
  Operator->>channels_add: add messaging channel
  channels_add->>checkChannelAddConflict: verify cross-sandbox conflicts
  checkChannelAddConflict->>MessagingRegistry: read ownership and credentials
  MessagingRegistry-->>checkChannelAddConflict: return conflict data or verification error
  checkChannelAddConflict-->>channels_add: abort or proceed with --force
  channels_add-->>Operator: report conflict guidance
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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
Linked Issues check ✅ Passed The Slack conflict fix matches #7808 by preventing duplicate active gateways and aborting on detected conflicts.
Out of Scope Changes check ✅ Passed The remaining messaging, docs, and test updates support the same fail-closed conflict policy and are in scope.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: messaging conflict handling now fails closed on channel conflicts.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7808-slack-conflict-abort

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

@github-actions

Copy link
Copy Markdown
Contributor

@github-code-quality

github-code-quality Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 98d0cf6 in the fix/7808-slack-confl... branch remains at 96%, unchanged from commit fa96c91 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 98d0cf6 in the fix/7808-slack-confl... branch remains at 81%, unchanged from commit fa96c91 in the main branch.

Show a code coverage summary of the most impacted files.
File main fa96c91 fix/7808-slack-confl... 98d0cf6 +/-
src/lib/onboard...nflict-guard.ts 100% 88% -12%
src/lib/onboard...ng-preflight.ts 100% 93% -7%
src/lib/actions...licy-channel.ts 80% 80% 0%
src/lib/messagi.../hook-phases.ts 98% 98% 0%
src/lib/messagi...hooks/errors.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 88% 88% 0%
src/lib/onboard...ndbox-create.ts 83% 91% +8%
src/lib/onboard...-create-plan.ts 75% 88% +13%
src/lib/onboard...ndbox-create.ts 33% 83% +50%

Updated July 29, 2026 13:03 UTC

@sandl99

sandl99 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Security review

Verdict

PASS. At exact PR head d118ba7c5, the change strengthens messaging conflict enforcement across channels. Credential conflicts, incomplete required checks, and channel-owned conflicts whose hook declares onFailure: "abort" now fail closed. Onboarding and rebuild also convert messaging-registry read failures into deliberate aborts with no override. The only bypass is the existing explicit channels add <channel> --force operator action. No credential exposure, authorization bypass, unsafe input handling, dependency change, or weakened default security control was found.

Findings

No security findings.

Detailed analysis

  1. Secrets and credentials — PASS: No secrets are added. Conflict detection continues to compare credential hashes, and regression tests assert that raw Slack and Telegram credential material and stored hashes are absent from output.
  2. Input validation and data sanitization — PASS: The change propagates validated built-in channel IDs and declarative hook failure modes. --force remains an explicit parsed CLI boolean. No new command, path, URL, evaluator, or parser surface is introduced.
  3. Authentication and authorization — PASS: No authentication or resource-authorization boundary changes. Only the invoking operator can select the existing add-only --force flag.
  4. Dependencies and third-party libraries — PASS: No dependency or package-lock changes are part of the PR.
  5. Error handling and logging — PASS: Credential conflicts, incomplete required checks, abort-policy hook conflicts, and onboarding/rebuild registry-read failures exit deliberately before provider, policy, rebuild, backup, or sandbox deletion mutation. Diagnostics identify sandbox/channel/resource conflicts without printing credential values or hashes.
  6. Cryptography and data protection — PASS: No cryptographic implementation changes. Existing non-secret credential-hash comparison remains unchanged.
  7. Configuration and security headers — PASS: No runtime permissions, network policy, container, HTTP, CORS, port exposure, or security-header changes.
  8. Security testing — PASS: Regression tests cover default denial for generic credential conflicts, explicit --force, matching Slack credentials across gateways, Slack same-gateway conflicts, Teams shared-port hook conflicts, incomplete registry checks in channel add and onboarding/rebuild, hook-policy propagation, pre-rebuild denial, and secret redaction.
  9. System security — PASS: The default path is stricter and fail-closed. The applier now preserves each conflicting hook's declarative failure policy; abort reaches onboarding/rebuild and channel-add consumers, while existing skip-channel handling remains intact. The deliberate override is limited to channels add <channel> --force.

Files reviewed

  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/actions/sandbox/rebuild-messaging-conflict-preflight.test.ts
  • src/lib/messaging/applier/hook-phases.ts
  • src/lib/messaging/applier/setup-applier.test.ts
  • src/lib/messaging/hooks/errors.ts
  • src/lib/onboard/messaging-conflict-guard.test.ts
  • src/lib/onboard/messaging-conflict-guard.ts
  • src/lib/onboard/sandbox-messaging-preflight.test.ts

@github-actions

github-actions Bot commented Jul 29, 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

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 more 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: channels-add-remove, channels-stop-start, inference-routing, network-policy, onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox, cloud-onboard

Workflow run details

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
docs/manage-sandboxes/manage-messaging-channels.mdx (1)

84-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use second-person wording in the new conflict guidance.

The added text mostly uses third-person/system-subject phrasing (“NemoClaw checks…”, “Slack aborts…”). Rewrite it as direct instructions to the reader, such as “Use different gateways for Slack sandboxes” and “If the check cannot complete, rerun channels add slack with --force only after accepting the risk.”

🤖 Prompt for 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.

In `@docs/manage-sandboxes/manage-messaging-channels.mdx` around lines 84 - 97,
Rewrite the added Slack conflict guidance in
docs/manage-sandboxes/manage-messaging-channels.mdx using second-person, direct
instructions instead of third-person system phrasing. Update statements such as
“NemoClaw checks” and “Slack aborts” to tell the reader what to do, including
using different gateways and rerunning channels add slack with --force only
after accepting the duplicate-session risk; preserve the existing behavior and
guidance for WeChat, Teams, and legacy metadata.

Source: Coding guidelines

src/lib/onboard/messaging-conflict-guard.ts (1)

63-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract a shared requireForceForConflictOverride lookup instead of three independent implementations.

Three sites re-implement the same new manifest-boolean lookup with already-diverging access patterns (named helper vs. inline, fresh registry vs. cached, .get() vs. .list().find()). This is the "duplicate sources of truth" pattern flagged by src/lib/README.md's layer-ownership guidance — a single shared, exported helper (e.g., in the channel manifest module) would let both the onboard and actions/sandbox layers consume one authoritative implementation.

  • src/lib/onboard/messaging-conflict-guard.ts#L63-L68: promote this helper into a shared exported utility (e.g., alongside createBuiltInChannelManifestRegistry) rather than keeping it private to this file.
  • src/lib/actions/sandbox/policy-channel.ts#L553-L553: replace the inline channelManifest.requireForceForConflictOverride === true computation with the shared helper.
  • src/lib/actions/sandbox/policy-channel.ts#L647-L648: replace messagingManifestRegistry.get(channelName)?.requireForceForConflictOverride === true with the same shared helper for consistency with the other two sites.
🤖 Prompt for 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.

In `@src/lib/onboard/messaging-conflict-guard.ts` around lines 63 - 68, Promote
requiresForceForConflictOverride from messaging-conflict-guard.ts into a shared
exported channel-manifest utility, preserving its undefined-safe registry
lookup. In src/lib/onboard/messaging-conflict-guard.ts lines 63-68, import and
use the shared helper instead of defining a private implementation; in
src/lib/actions/sandbox/policy-channel.ts lines 553 and 647-648, replace both
inline manifest checks with that helper so all callers use one authoritative
lookup.

Source: Path instructions

🤖 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/manage-sandboxes/set-up-slack.mdx`:
- Around line 46-54: Update the conflict-resolution guidance in “Avoid Duplicate
Socket Mode Sessions” to scope `--force` exclusively to `channels add slack`.
State that onboarding and rebuild require stopping/removing the conflicting
Slack sandbox or using a different gateway with different credentials, while
only `channels add slack --force` can explicitly accept the conflict risk.

---

Nitpick comments:
In `@docs/manage-sandboxes/manage-messaging-channels.mdx`:
- Around line 84-97: Rewrite the added Slack conflict guidance in
docs/manage-sandboxes/manage-messaging-channels.mdx using second-person, direct
instructions instead of third-person system phrasing. Update statements such as
“NemoClaw checks” and “Slack aborts” to tell the reader what to do, including
using different gateways and rerunning channels add slack with --force only
after accepting the duplicate-session risk; preserve the existing behavior and
guidance for WeChat, Teams, and legacy metadata.

In `@src/lib/onboard/messaging-conflict-guard.ts`:
- Around line 63-68: Promote requiresForceForConflictOverride from
messaging-conflict-guard.ts into a shared exported channel-manifest utility,
preserving its undefined-safe registry lookup. In
src/lib/onboard/messaging-conflict-guard.ts lines 63-68, import and use the
shared helper instead of defining a private implementation; in
src/lib/actions/sandbox/policy-channel.ts lines 553 and 647-648, replace both
inline manifest checks with that helper so all callers use one authoritative
lookup.
🪄 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: 545bc837-426e-4d5c-a9cd-caf12da86a96

📥 Commits

Reviewing files that changed from the base of the PR and between eeab81c and 7b29321.

📒 Files selected for processing (15)
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/messaging/channels/manifests.test.ts
  • src/lib/messaging/channels/slack/hooks/socket-mode-gateway-conflict.test.ts
  • src/lib/messaging/channels/slack/hooks/socket-mode-gateway-conflict.ts
  • src/lib/messaging/channels/slack/manifest.ts
  • src/lib/messaging/hooks/errors.ts
  • src/lib/messaging/manifest/types.ts
  • src/lib/onboard/messaging-conflict-guard.test.ts
  • src/lib/onboard/messaging-conflict-guard.ts
  • src/lib/onboard/sandbox-messaging-preflight.test.ts

Comment thread docs/manage-sandboxes/set-up-slack.mdx Outdated
Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 changed the title fix(messaging): require force for Slack conflicts fix(messaging): fail closed on channel conflicts Jul 29, 2026
@sandl99 sandl99 added bug-fix PR fixes a bug or regression v0.0.98 labels Jul 29, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/onboard/messaging-conflict-guard.ts (1)

90-128: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Registry-read failures in onboarding aren't guarded like the channels-add path.

findChannelConflictsFromPlan(...) at Line 91 and deps.registry.listSandboxes().sandboxes at Line 127 are called with no try/catch. If the registry read throws (a realistic case — the channels-add equivalent in policy-channel.ts explicitly tests and handles this), the exception propagates unhandled out of enforceMessagingChannelConflicts instead of producing the clean "Could not verify … / Aborting: … or re-run with --force"-style guidance that checkChannelAddConflict/checkMessagingPreEnableHooks in policy-channel.ts now provide for the identical failure mode.

This PR's stated goal is to fail closed on "incomplete conflict checks" uniformly, and policy-channel.ts was updated in this same PR to add exactly this guard (Lines 562-580, 621-635) — but the onboarding path was left asymmetric.

As per path instructions, src/lib/messaging/** changes must "Verify persisted-plan hydration and parity across onboard, add/remove, start/stop, rebuild, resume, diagnostics, and build-time application."

🛡️ Proposed fix to mirror policy-channel.ts's guard
   if (currentPlan && hasPlanCredentials) {
-    const conflicts = findChannelConflictsFromPlan(sandboxName, currentPlan, registry);
+    let conflicts: ReturnType<typeof findChannelConflictsFromPlan>;
+    try {
+      conflicts = findChannelConflictsFromPlan(sandboxName, currentPlan, registry);
+    } catch (err) {
+      const message = err instanceof Error ? err.message : String(err);
+      deps.error(`  Could not verify messaging channel conflicts: ${message}`);
+      deps.error(`  Aborting: resolve the messaging channel conflict check or re-run once the registry is readable.`);
+      abort(deps);
+    }
     if (conflicts.length > 0) {
   const hookRegistry = createBuiltInMessagingHookRegistry();
-  const additionalInputs = createMessagingPreEnableHookInputs({
-    currentSandbox: deps.sandboxName,
-    currentGatewayName: deps.gatewayName,
-    registryEntries: deps.registry.listSandboxes().sandboxes,
-  });
+  let registryEntries: ReturnType<typeof deps.registry.listSandboxes>["sandboxes"];
+  try {
+    registryEntries = deps.registry.listSandboxes().sandboxes;
+  } catch (err) {
+    const message = err instanceof Error ? err.message : String(err);
+    deps.error(`  Could not verify messaging pre-enable checks: ${message}`);
+    deps.error(`  Aborting: resolve the messaging pre-enable check or re-run once the registry is readable.`);
+    abort(deps);
+  }
+  const additionalInputs = createMessagingPreEnableHookInputs({
+    currentSandbox: deps.sandboxName,
+    currentGatewayName: deps.gatewayName,
+    registryEntries,
+  });
🤖 Prompt for 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.

In `@src/lib/onboard/messaging-conflict-guard.ts` around lines 90 - 128, Guard
registry reads in enforceMessagingChannelConflicts and
enforceMessagingPreEnableHooks, including findChannelConflictsFromPlan and
deps.registry.listSandboxes, using the same failure handling as
policy-channel.ts. On read failure, emit the established “Could not verify…”
guidance, provide the “Aborting…”/--force recovery instructions, and fail closed
rather than allowing the exception to propagate.

Source: Path instructions

🤖 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.

Outside diff comments:
In `@src/lib/onboard/messaging-conflict-guard.ts`:
- Around line 90-128: Guard registry reads in enforceMessagingChannelConflicts
and enforceMessagingPreEnableHooks, including findChannelConflictsFromPlan and
deps.registry.listSandboxes, using the same failure handling as
policy-channel.ts. On read failure, emit the established “Could not verify…”
guidance, provide the “Aborting…”/--force recovery instructions, and fail closed
rather than allowing the exception to propagate.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0807a78f-2622-4785-8a91-cb74b8d78f4f

📥 Commits

Reviewing files that changed from the base of the PR and between 7b29321 and cb4ef6a.

📒 Files selected for processing (12)
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/actions/sandbox/rebuild-messaging-conflict-preflight.test.ts
  • src/lib/messaging/applier/hook-phases.ts
  • src/lib/messaging/applier/setup-applier.test.ts
  • src/lib/messaging/hooks/errors.ts
  • src/lib/onboard/messaging-conflict-guard.ts
  • src/lib/onboard/sandbox-messaging-preflight.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/lib/actions/sandbox/policy-channel-conflict.test.ts (1)

734-805: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared Slack app-token setup to cut duplication.

The bob entry construction, gatewayName cast, and getCredentialMock wiring in these two tests are nearly identical, differing only by force. A small local helper (e.g. makeSlackGatewayConflictBob()) would remove ~25 duplicated lines and keep future credential/gateway-name tweaks in one place.

🤖 Prompt for 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.

In `@src/lib/actions/sandbox/policy-channel-conflict.test.ts` around lines 734 -
805, Extract the duplicated Slack conflict fixture setup from the tests into a
local helper such as makeSlackGatewayConflictBob(), including the bob
makePlanEntry data, gatewayName assignment, and getCredentialMock wiring as
appropriate. Update both tests to reuse the helper while preserving their
existing force-specific behavior and assertions.
🤖 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.

Nitpick comments:
In `@src/lib/actions/sandbox/policy-channel-conflict.test.ts`:
- Around line 734-805: Extract the duplicated Slack conflict fixture setup from
the tests into a local helper such as makeSlackGatewayConflictBob(), including
the bob makePlanEntry data, gatewayName assignment, and getCredentialMock wiring
as appropriate. Update both tests to reuse the helper while preserving their
existing force-specific behavior and assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 79c009bd-33fe-47b0-a3f5-910741c57016

📥 Commits

Reviewing files that changed from the base of the PR and between eeab81c and cb4ef6a.

📒 Files selected for processing (13)
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx
  • src/lib/actions/sandbox/policy-channel-conflict.test.ts
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/actions/sandbox/rebuild-messaging-conflict-preflight.test.ts
  • src/lib/messaging/applier/hook-phases.ts
  • src/lib/messaging/applier/setup-applier.test.ts
  • src/lib/messaging/hooks/errors.ts
  • src/lib/onboard/messaging-conflict-guard.test.ts
  • src/lib/onboard/messaging-conflict-guard.ts
  • src/lib/onboard/sandbox-messaging-preflight.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
  • docs/manage-sandboxes/set-up-slack.mdx
  • src/lib/messaging/hooks/errors.ts
  • src/lib/messaging/applier/setup-applier.test.ts
  • docs/reference/commands.mdx
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • src/lib/onboard/sandbox-messaging-preflight.test.ts
  • src/lib/onboard/messaging-conflict-guard.test.ts
  • docs/reference/troubleshooting.mdx
  • src/lib/actions/sandbox/policy-channel.ts
  • src/lib/onboard/messaging-conflict-guard.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/manage-sandboxes/manage-messaging-channels.mdx`:
- Around line 87-104: Update the onboarding and rebuild behavior description in
the messaging-channel conflict section to explicitly state that either operation
aborts when any required check cannot be completed, including incomplete or
unavailable checks, and that neither operation supports an override. Keep the
existing conflict behavior and the separate --force exception for channels add
unchanged.
- Around line 82-104: Rewrite the policy section in direct second-person,
active-voice, present-tense language. Update the introductory statements and all
conditions involving NemoClaw so they address the reader directly, including
onboarding, rebuild, channels add, legacy metadata, and registry or
conflict-check failures; preserve the existing policy, channel rules, and
command behavior.

In `@docs/reference/troubleshooting.mdx`:
- Around line 1983-1986: Update the conflict-policy section in troubleshooting
documentation to state that onboarding, rebuild, and channels add also fail
closed when credential hashes are unavailable or required conflict checks cannot
complete, even without a confirmed conflict. Keep the existing credential-reuse,
shared-resource, and --force guidance unchanged.
🪄 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: e752b1d9-5810-4db5-a367-f734e0aed4ab

📥 Commits

Reviewing files that changed from the base of the PR and between d118ba7 and ab150d0.

📒 Files selected for processing (4)
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx

Comment thread docs/manage-sandboxes/manage-messaging-channels.mdx Outdated
Comment thread docs/manage-sandboxes/manage-messaging-channels.mdx Outdated
Comment thread docs/reference/troubleshooting.mdx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/manage-sandboxes/manage-messaging-channels.mdx`:
- Around line 90-95: The `channels add --force` documentation does not state
that it bypasses incomplete required checks. Update the command behavior section
to clarify that `--force` is the only override for `channels add` and bypasses
both duplicate/shared-resource conflicts and incomplete-check aborts, while
onboarding and rebuild remain without an override.
🪄 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: 700e4fb3-14b7-4479-9cf8-17db0e47e683

📥 Commits

Reviewing files that changed from the base of the PR and between ab150d0 and 15cb844.

📒 Files selected for processing (4)
  • docs/manage-sandboxes/manage-messaging-channels.mdx
  • docs/manage-sandboxes/set-up-slack.mdx
  • docs/reference/commands.mdx
  • docs/reference/troubleshooting.mdx

Comment thread docs/manage-sandboxes/manage-messaging-channels.mdx Outdated
sandl99 added 4 commits July 29, 2026 17:05
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@jyaunches
jyaunches merged commit ae71359 into main Jul 29, 2026
181 of 186 checks passed
@jyaunches
jyaunches deleted the fix/7808-slack-conflict-abort branch July 29, 2026 14:40
@sandl99 sandl99 mentioned this pull request Jul 30, 2026
23 tasks
cv pushed a commit that referenced this pull request Jul 30, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical pre-tag release entry for NemoClaw v0.0.98.
The dated entry records the user-visible changes merged after v0.0.97
and links each release theme to its published documentation.

## Changes

- Add `docs/changelog/2026-07-29.mdx` with the exact `## v0.0.98`
release heading.
- Summarize Hermes 0.19, Deep Agents Code automation and skill safety,
readiness diagnostics, lifecycle recovery, uninstall behavior, messaging
conflicts, dependency hardening, and bounded diagnostics.
- Use the parser-safe MDX SPDX comment and root-absolute routes for
published OpenClaw, Hermes, and Deep Agents documentation.

### Source summary

- [#7849](#7849) ->
`docs/changelog/2026-07-29.mdx`: Record the Hermes 0.19 runtime
migration repairs for cron state, dashboard seeding, and MCP naming.
- [#7662](#7662) ->
`docs/changelog/2026-07-29.mdx`: Record bounded gateway and Docker
subprocess diagnostics.
- [#7850](#7850) ->
`docs/changelog/2026-07-29.mdx`: Record verified no-clobber Deep Agents
Code skill installation.
- [#7848](#7848) ->
`docs/changelog/2026-07-29.mdx`: Record post-reboot delivery-chain
recovery for visible OpenClaw sandboxes.
- [#7831](#7831) ->
`docs/changelog/2026-07-29.mdx`: Record OpenShell gateway-state
preservation during uninstall.
- [#7827](#7827) ->
`docs/changelog/2026-07-29.mdx`: Record the removal of upstream test
sources from published Hermes images.
- [#7775](#7775) ->
`docs/changelog/2026-07-29.mdx`: Record the blocking diagnostic for
unsupported `DOCKER_HOST` values.
- [#7833](#7833) ->
`docs/changelog/2026-07-29.mdx`: Record reviewed Python dependency
baselines for Hermes and Deep Agents Code images.
- [#7771](#7771) ->
`docs/changelog/2026-07-29.mdx`: Record the managed Hermes Agent 0.19.0
upgrade.
- [#7811](#7811) ->
`docs/changelog/2026-07-29.mdx`: Record fail-closed messaging channel
conflict handling.
- [#7797](#7797) ->
`docs/changelog/2026-07-29.mdx`: Record the managed non-interactive Deep
Agents Code JSON envelope.
- [#7782](#7782) ->
`docs/changelog/2026-07-29.mdx`: Record the storage-remediation
readiness capability.
- [#7784](#7784) ->
`docs/changelog/2026-07-29.mdx`: Record the 120-second OpenShell
readiness budget for sandbox recreation.
- [#7810](#7810) ->
`docs/changelog/2026-07-29.mdx`: Record rejection of stale Deep Agents
Code security inventories.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the native changelog contract,
including the version heading, MDX SPDX comment, and published routes.
- [ ] Tests not applicable — justification:
- [x] 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

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: `docs/changelog/2026-07-29.mdx` was reviewed against
`docs/CONTRIBUTING.md` and `WRITING.md` for release meaning,
terminology, structure, voice, sentence form, MDX structure, published
routes, and code-sample presentation. The changelog contract passed 6
tests. The docs build completed with 0 errors and 2 existing Fern
warnings.
- Agent: Codex CLI
<!-- docs-review-head-sha: e3221d1 -->
<!-- docs-review-agents-blob-sha: c052d60 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable. `scripts/prepare-dgx-station-host.sh`
is unchanged.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## 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 — `npx vitest run
test/changelog-docs.test.ts` passed 6 tests.
- [ ] 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 for this
documentation-only change.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — The
build completed with 0 errors and 2 existing Fern warnings.
- [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)
— Native changelog entries use the required parser-safe MDX SPDX comment
and do not use frontmatter.

---
Signed-off-by: San Dang <sdang@nvidia.com>


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

## Summary by CodeRabbit

- **New Features**
- Added managed Hermes upgrades with verified releases, version
reporting, and preserved configuration contracts.
- Improved Deep Agents Code JSON output and skill installation behavior.
  - Added clearer Docker host and system readiness reporting.
  - Improved post-reboot delivery recovery and sandbox readiness timing.
- **Bug Fixes**
  - Preserved gateway state when uninstalling with `--keep-openshell`.
- Prevented conflicting messaging credentials from blocking onboarding
and rebuilds.
- Improved gateway diagnostics, dependency security, runtime filesystem
protection, and evidence handling.
- **Documentation**
  - Published the v0.0.98 release notes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build area: messaging Messaging channels, bridges, manifests, or channel lifecycle bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][Agent&Skills] two sandboxes can consume the same Slack app and produce competing replies

2 participants