Skip to content

fix(onboard): exclude inactive Discord messaging presets - #10334

Closed
rsliter wants to merge 7 commits into
mainfrom
codex/fix-slack-only-discord-binding
Closed

fix(onboard): exclude inactive Discord messaging presets#10334
rsliter wants to merge 7 commits into
mainfrom
codex/fix-slack-only-discord-binding

Conversation

@rsliter

@rsliter rsliter commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #10314 merged the production fix that makes the explicit messaging selection authoritative for Discord policy and provider construction. After the merge with base commit 687f3e4f0cfedadd4b36deb071f9dd7e100d21a7, this PR's effective production delta only reformats that existing condition.

The remaining semantic diff strengthens the shared onboarding regression for OpenClaw and Hermes. It proves that the Open policy tier omits Discord egress for an empty or Slack-only messaging selection and retains Discord egress when Discord is selected.

Related Issue

Related to #10153 root cause 5.

Changes

E2E root cause: OpenClaw onboarding / policy suggestion / inactive Discord preset gains an unattached provider binding
Source run: https://github.com/NVIDIA/NemoClaw/actions/runs/32909322579 (run 32909322579, attempt 1)
Failed jobs: Messaging: shares OpenClaw Slack pairing approval / NVIDIA inference API key (98000632449, https://github.com/NVIDIA/NemoClaw/actions/runs/32909322579/job/98000632449)
Signature: credential_binding references <sandbox>-discord-bridge, but that provider is not attached
Scope: one root cause

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: Not applicable because this escaped defect required new regression coverage.
  • Tests not applicable, justification: Not applicable.
  • 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 Desktop completed the nine-category review at exact commit 29601364dd756b77d99a5ab25c3cb85163a5dc2f against current main 6c06d0c8a3d935bf6e6c10f1c6e5b1cfbe56eea8; result PASS with no findings. Production code differs only by line wrapping. The semantic delta is test-only and changes no injection, authorization, credential custody, network, filesystem, dependency, logging, or denial-of-service boundary. Independent approval is still required before merge.
  • Non-success, skipped, or missing CI check accepted by maintainer, check name, approval link, and follow-up issue: None accepted.

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Reviewed the complete two-file effective diff at commit 29601364dd756b77d99a5ab25c3cb85163a5dc2f against base 6c06d0c8a3d935bf6e6c10f1c6e5b1cfbe56eea8. Production code changes only line wrapping. The integration test extends the existing inactive Discord regression across Hermes and OpenClaw with empty, Slack-only, and Discord-active matrices. The title is behavior-oriented and uses the issue suffix. The change affects no public behavior, diagnostic, CLI, configuration, documentation claim, or user workflow. CLI build and type-checking, 56 focused integration tests, repository checks, and git diff --check passed.
  • Agent: Codex documentation writer subagent (/root/watch_trigger_merge_blocker)

DGX Station Hardware Evidence

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

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • npm run validate:pr passed at exact commit 29601364dd756b77d99a5ab25c3cb85163a5dc2f against current main 6c06d0c8a3d935bf6e6c10f1c6e5b1cfbe56eea8, covering pre-commit, commitlint, and pre-push checks
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above: npm exec -- vitest run --project integration test/onboarding/onboard-policy-suggestions.test.ts passed 56/56 tests at exact commit 29601364dd756b77d99a5ab25c3cb85163a5dc2f. The CLI build and typecheck, repository checks, npm run validate:pr, and git diff --check passed.
  • Applicable broad gate passed: Not required for this focused onboarding policy-suggestion correction; PR CI owns the broad gate.
  • 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: Rebecca Sliter 571084+rsliter@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved onboarding policy suggestions to correctly account for active communication channels and selected platform preferences.
    • Discord suggestions no longer appear without an active Discord channel or when only Slack is selected.
    • Discord suggestions appear when Discord is explicitly selected.

@rsliter rsliter self-assigned this Aug 26, 2026
@copy-pr-bot

copy-pr-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 26, 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: 0a604623-0a5c-4740-b3f5-bc3c8f657541

📥 Commits

Reviewing files that changed from the base of the PR and between 642b574 and fda2cfb.

📒 Files selected for processing (1)
  • test/onboarding/onboard-policy-suggestions.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/onboarding/onboard-policy-suggestions.test.ts

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


📝 Walkthrough

Walkthrough

The onboarding policy suggestion test now covers Slack-only selections, ambient Discord credentials without an active channel, and explicit Discord activation for Hermes and OpenClaw.

Changes

Onboarding preset filtering

Layer / File(s) Summary
Validate channel-specific suggestions
test/onboarding/onboard-policy-suggestions.test.ts
The parameterized test confirms Slack is included and credential-bound Discord is excluded for Slack-only selections. It retains coverage for explicit Discord activation.

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

Merge Risk: ⚪ Minimal · up to fda2c

This PR expands regression coverage for inactive Discord messaging presets without changing production behavior, so no actionable merge-blocking risk remains after normal checks and review.

🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. 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 The title accurately describes the Discord preset behavior covered by the tests. It does not state that the primary change is regression coverage, but it remains related and sufficiently specific.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-slack-only-discord-binding

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

@github-code-quality

github-code-quality Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 2960136 in the codex/fix-slack-only... branch remains at 96%, unchanged from commit 6c06d0c in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 2960136 in the codex/fix-slack-only... branch remains at 84%, unchanged from commit 6c06d0c in the main branch.

Show a line coverage summary of the most impacted files.
File main 6c06d0c codex/fix-slack-only... 2960136 +/-
src/lib/onboard...cker-journal.ts 75% 73% -2%
src/lib/onboard...er-gpu-patch.ts 81% 79% -2%
src/lib/onboard...trap/adapter.ts 71% 70% -1%
src/lib/onboard...ndbox-create.ts 89% 88% -1%
src/lib/onboard...uild-context.ts 74% 74% 0%
src/lib/onboard...ure-evidence.ts 88% 88% 0%
src/lib/state/gateway.ts 94% 96% +2%
src/lib/onboard...vider/docker.ts 85% 88% +3%
src/lib/onboard...eate-journal.ts 85% 90% +5%
src/lib/onboard...eate-attempt.ts 90% 96% +6%

Updated August 26, 2026 14:10 UTC

@rsliter

rsliter commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Focused validation for #10153 root cause 5 at 432db612d931096007fca41a56a0e7f9cda5162c:

  • Deterministic regression: npm exec -- vitest run --project integration test/onboarding/onboard-policy-suggestions.test.ts passed 56 of 56 tests.
  • The regression proves Slack-only OpenClaw open-tier suggestions retain Slack and exclude the unattached Discord bridge provider. The positive case proves active Discord still retains its preset.
  • Focused live validation against broken PR fix(onboard): require managed images for stock agents #10113 was not dispatched. The current trusted maintainer E2E selector does not authorize an arbitrary target-only manual PR run, and an empty-selector run would not be focused evidence for this root cause.

Live E2E status: blocked by the trusted selector. No live pass is claimed.

@rsliter
rsliter marked this pull request as ready for review August 26, 2026 03:24
@rsliter

rsliter commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Validation refresh for exact head 29601364dd756b77d99a5ab25c3cb85163a5dc2f against current main 6c06d0c8a3d935bf6e6c10f1c6e5b1cfbe56eea8:

  • The effective two-file patch exactly matches the previously reviewed change. Production code changes only line wrapping; the semantic delta is regression coverage.
  • npm exec -- vitest run --project integration test/onboarding/onboard-policy-suggestions.test.ts passed 56/56. CLI build and typecheck, repository checks, npm run validate:pr, and git diff --check passed.
  • Exact security review: PASS across all nine categories with no findings.
  • Documentation review: no-docs-needed with no findings; receipt markers 29601364d and c7de0eb73c13df83dd97dfb6e3f664cac8d75305.
  • Advisor specialists failed on inference transport refusal without producing findings. No candidate-owned CI failure or unresolved review finding is present.
  • No target-only E2E was dispatched for this test-only follow-up. The production fix and affected-target validation remain owned by merged PR fix(onboard): bind Discord policy to create providers #10314.

This comment is validation evidence, not an approval. Outside approval and all required gates remain necessary before merge.

@rsliter rsliter changed the title fix(onboard): exclude inactive OpenClaw messaging presets fix(onboard): exclude inactive Discord messaging presets Aug 26, 2026
@copy-pr-bot

copy-pr-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

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.

@github-actions

Copy link
Copy Markdown
Contributor

PR review advisory complete for commit a60f50a: read the full review. Read it before deciding whether to request changes, approve, or merge this PR.

All previous runs

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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.

@rsliter

rsliter commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this test-only follow-up because PR #10314 merged the production correction at 45a90f0. The previously failing E2E scenario passed on main commit 7409b8f in full-main run 33111284865: both messaging-providers and Messaging: shares OpenClaw Slack pairing approval / NVIDIA inference API key succeeded. PR #10334 is not required to restore this behavior.

@rsliter rsliter closed this Aug 27, 2026
@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow 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 integration: openclaw OpenClaw integration behavior labels Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow 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 integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants