test(onboard): guard Slack channels.* allowlist sync (#4869)

[yimoj]

Summary

Hardens the Slack allowlist regression guard for #4869. The active OpenClaw
config generator (scripts/generate-openclaw-config.mts) already emits
channels["*"] = { enabled: true, requireMention: true, users: [...] }
alongside dmPolicy / allowFrom / groupPolicy whenever SLACK_ALLOWED_USERS
is set, and the #3729 positive guard covers the users-only path. This PR adds
the two regression guards the issue is named after — the backward-compatibility
negative path and the DM/channel scope-synchronization invariant — so the
reported failure mode (empty channels["*"] while allowFrom is populated)
cannot regress unnoticed.

Related Issue

Fixes #4869

Changes

• New test/generate-openclaw-config-slack-allowlist.test.ts (kept separate so
  generate-openclaw-config.test.ts stays within its size budget):
  • Negative path: Slack enabled with no allowlist leaves all four
    allowlist keys (dmPolicy, allowFrom, groupPolicy, channels) absent,
    proving they derive only from SLACK_ALLOWED_USERS and that an unset
    allowlist never silently broadens (listen-everywhere) or narrows (disable)
    channel scope.
  • Scope synchronization: channels["*"].users must equal allowFrom, pinning
    the exact security invariant the reporter relied on (DM and channel scopes
    stay in lockstep, both sourced from the same member IDs).

Type of Change

• Code change (feature, bug fix, or refactor)

Investigation

The reported channels["*"]: {} does not reproduce on current main. The
triage note pointed at the manifest render path ({{allowedIds.slack.channels}}
in src/lib/messaging/channels/slack/manifest.ts), but that render plan is never
applied during onboarding: applyAgentConfigAtOpenShell has no production call
site in any commit, and nothing resolves the {{allowedIds.*}} refs. The sole
writer of /sandbox/.openclaw/openclaw.json is scripts/generate-openclaw-config.mts,
which writes dmPolicy / allowFrom / groupPolicy and the channels["*"]
block in the same branch — so the policies-present-but-channels-empty symptom is
not producible from the generator. Both the env-export and interactive-wizard
input paths populate SLACK_ALLOWED_USERS (the config-prompt hook writes it back
to the environment the generator reads), so neither path can drop the channel
scope. The most likely cause of the reporter's observation is a stale Docker
build layer reusing a pre-#3729 openclaw.json.

Verification

Reporter-workflow E2E for the config-generation step, run against the exact
production code that writes the inspected artifact:

• Ran buildConfig from scripts/generate-openclaw-config.mts (the script the
  sandbox image build executes to write /sandbox/.openclaw/openclaw.json) with
  the reporter's exact inputs (SLACK_ALLOWED_USERS=U_DUMMY_ALLOW1,U_DUMMY_ALLOW2,
  no SLACK_ALLOWED_CHANNELS). Result matches the issue's expected output:

  channels.slack.accounts.default = {
    dmPolicy: "allowlist",
    allowFrom: ["U_DUMMY_ALLOW1","U_DUMMY_ALLOW2"],
    groupPolicy: "allowlist",
    channels: { "*": { enabled: true, requireMention: true,
                       users: ["U_DUMMY_ALLOW1","U_DUMMY_ALLOW2"] } }
  }
  

• Confirmed the same correct output on the reporter's CLI version (v0.0.59 tag).

• Confirmed the host-side collector collectMessagingBuildConfig maps
  SLACK_ALLOWED_USERS → messagingAllowedIds.slack, which feeds
  NEMOCLAW_MESSAGING_ALLOWED_IDS_B64 consumed by the generator.

• vitest run test/generate-openclaw-config-slack-allowlist.test.ts → 2/2 pass.

• vitest run test/generate-openclaw-config.test.ts → 135/135 pass (existing
  #3729 positive guards included).

A full live nemoclaw onboard to inspect the in-sandbox config was not run to
completion: the shared OpenShell gateway on this host is continuously in use by
concurrent worktree onboards, and a fresh onboard switches the gateway driver and
disrupts those active runs. The buildConfig execution above exercises the exact
code path that produces the file the reporter inspects.

• npx prek run --all-files (lint/format on changed files)
• Targeted tests pass; existing Slack suite unchanged and green
• Tests added for the guarded behavior
• No secrets, API keys, or credentials committed

---

Signed-off-by: Yimo Jiang yimoj@nvidia.com

[coderabbitai]

📝 Walkthrough

Walkthrough

A new Vitest test file validates Slack allowlist handling in buildConfig. Tests verify that allowlist fields remain absent when unconfigured, and that when enabled via environment variables, channel policies synchronize with DM policies and correctly populate the wildcard channel entry with matching user IDs.

Changes

Slack allowlist regression tests

Layer / File(s)	Summary
Slack allowlist regression test suite test/generate-openclaw-config-slack-allowlist.test.ts	Helper functions slackEnv() and slackAccount() support two test cases: absence of allowlist fields when no IDs are set, and synchronized user IDs across dmPolicy, allowFrom, groupPolicy, and channels["*"] when configured via NEMOCLAW_MESSAGING_ALLOWED_IDS_B64.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

integration: slack, area: messaging

Suggested reviewers

• cv

Poem

🐰 A test hops in to catch the bug,
Where Slack allowlists need a hug.
Channels now sync with DM scope,
Regression guards restore our hope! 🎯

🚥 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
Title check	✅ Passed	The title clearly and specifically describes the main change: adding regression guards for Slack channels.* allowlist synchronization, matching the core purpose of the PR.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #4869: tests verify allowlist keys absent when unconfigured, confirm channels[*].users matches allowFrom for sync, and guard against regressions.
Out of Scope Changes check	✅ Passed	All changes are within scope: the new test file directly addresses issue #4869 requirements without introducing unrelated modifications or dependencies.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

test/generate-openclaw-config-slack-allowlist.test.ts (1)

1-1: ⚡ Quick win

Consider removing @ts-nocheck and adding proper type annotations.

The @ts-nocheck directive disables all TypeScript type checking, reducing type safety. Consider:

• Properly typing the buildConfig return value
• Adding a return type annotation to slackAccount instead of any
• Removing the @ts-nocheck directive

This would catch type errors at compile time and make refactoring safer.

♻️ Suggested improvement

-// `@ts-nocheck`
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0

And update the slackAccount helper:

function slackAccount(env: Record<string, string>) {
  return buildConfig(env as any).channels.slack.accounts.default;
}

If buildConfig has proper type exports, you could also type the return value more precisely.

🤖 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 `@test/generate-openclaw-config-slack-allowlist.test.ts` at line 1, Remove the
file-level "`@ts-nocheck`", add proper types for buildConfig's return and for the
slackAccount helper: give buildConfig an explicit exported return type (so
callers can import it) and change slackAccount from "any" to a typed signature
(e.g. accept env: Record<string,string> and return the precise type of
channels.slack.accounts.default derived from buildConfig's return type); update
the helper to call buildConfig(env as any) only if necessary and prefer using
the exported buildConfig return type instead of any so TypeScript can validate
accesses to channels.slack.accounts.default.

🤖 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 `@test/generate-openclaw-config-slack-allowlist.test.ts`:
- Line 1: Remove the file-level "`@ts-nocheck`", add proper types for
buildConfig's return and for the slackAccount helper: give buildConfig an
explicit exported return type (so callers can import it) and change slackAccount
from "any" to a typed signature (e.g. accept env: Record<string,string> and
return the precise type of channels.slack.accounts.default derived from
buildConfig's return type); update the helper to call buildConfig(env as any)
only if necessary and prefer using the exported buildConfig return type instead
of any so TypeScript can validate accesses to channels.slack.accounts.default.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4f6c3931-09cf-4bc8-a954-f973f7f37628

📥 Commits

Reviewing files that changed from the base of the PR and between 784f573 and 7cd1f27.

📒 Files selected for processing (1)

• test/generate-openclaw-config-slack-allowlist.test.ts

[wscurran]

✨
Related open issues:

• #4869 [Ubuntu 24.04][Policy&Network] Slack allowlist only applies dm/group policies; channels.* block is empty (regression guard fails)
• #3729 [macOS][Agent&Skills] Slack channel @mention not covered by DM allowFrom; no allowlist path

[prekshivyas]

LGTM. Test-only regression guard for #4869, and it pins the right behavior: the scope-sync test asserts slack.channels["*"] is fully populated ({enabled, requireMention, users}) and that its users stays identical to allowFrom — directly guarding the 'channels.* comes back empty' regression the issue names. Verified main's generate-openclaw-config.mts populates channels.* from the allowlist, so the behavior is correct and the test passes (CI green). The negative path (no allowlist → keys absent) is a good backward-compat guard too. Low-risk.