Skip to content

test(e2e): migrate OpenClaw Slack pairing to Vitest - #5579

Closed
cv wants to merge 1 commit into
e2e-phase6-openclaw-discord-pairingfrom
e2e-phase6-openclaw-slack-pairing
Closed

test(e2e): migrate OpenClaw Slack pairing to Vitest#5579
cv wants to merge 1 commit into
e2e-phase6-openclaw-discord-pairingfrom
e2e-phase6-openclaw-slack-pairing

Conversation

@cv

@cv cv commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Migrates the OpenClaw Slack pairing E2E into live Vitest coverage. The replacement keeps the real install/OpenShell sandbox boundary, hermetic fake Slack Socket Mode/REST token rewrite, runtime pairing request creation, and connect-shell approval flow.

Related Issue

Refs #5098

Changes

  • Add test/e2e-scenario/live/openclaw-slack-pairing.test.ts as Vitest coverage for test/e2e/test-openclaw-slack-pairing.sh.
  • Reuse the OpenClaw pairing helpers introduced by the Discord pairing migration.
  • Wire openclaw-slack-pairing-vitest into .github/workflows/e2e-vitest-scenarios.yaml.

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)

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • 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: Carlos Villela cvillela@nvidia.com

@cv cv self-assigned this Jun 22, 2026
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 533820cc-4a86-4cec-a1aa-eac59a7b48d3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-phase6-openclaw-slack-pairing

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

@github-code-quality

github-code-quality Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the e2e-phase6-openclaw-... branch is 96%. Coverage data for the e2e-phase6-openclaw-... branch is not yet available.

Show a code coverage summary of the most covered files.
File e2e-phase6-openclaw-... e2e-phase6-openclaw-... f510264 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the e2e-phase6-openclaw-... branch is 46%. Coverage data for the e2e-phase6-openclaw-... branch is not yet available.

Show a code coverage summary of the most covered files.
File e2e-phase6-openclaw-... e2e-phase6-openclaw-... f510264 +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 18%

Updated June 22, 2026 01:22 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: openclaw-slack-pairing-vitest
Optional E2E: openclaw-discord-pairing-vitest, messaging-providers-vitest

Dispatch hint: openclaw-slack-pairing-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/e2e-phase6-openclaw-discord-pairing
Head: HEAD
Confidence: high

Required E2E

  • openclaw-slack-pairing-vitest (high): This is the newly added live scenario and workflow job. It should run to validate the workflow selector, job environment, fake Slack Socket Mode/REST token rewrite assertions, sandbox install, pairing request, and connect-shell approval path.

Optional E2E

  • openclaw-discord-pairing-vitest (high): Adjacent pairing coverage using the same helper family and workflow pattern. Useful to catch regressions in shared OpenClaw pairing helpers or report wiring, but not directly changed by this PR.
  • messaging-providers-vitest (medium): Optional broader messaging provider confidence because the new Slack pairing scenario exercises provider creation and messaging integration boundaries.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: openclaw-slack-pairing-vitest

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: openclaw-slack-pairing-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openclaw-slack-pairing-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/e2e-phase6-openclaw-discord-pairing
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • openclaw-slack-pairing-vitest: Focused free-standing Vitest job wired for changed live test test/e2e-scenario/live/openclaw-slack-pairing.test.ts.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=openclaw-slack-pairing-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/live/openclaw-slack-pairing.test.ts

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Review posture: Resolve findings before merge
Action expectation: Address required items before merge. Resolve or explicitly justify warnings. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up.
Findings: 0 required fixes, 3 items to resolve/justify, 0 in-scope improvements
Top item: Slack pairing test mutates the baseline policy instead of using the Slack preset source of truth

Review findings by urgency: 0 required fixes, 3 items to resolve/justify, 0 in-scope improvements

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

  • None.

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

  • Source-of-truth review needed: Slack network policy setup for OpenClaw Slack pairing Vitest: The advisor marked localized patch analysis as needs_followup.
    • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Verification hint: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
    • Missing regression test: Coverage should prove a Slack-enabled install applies the Slack preset-derived REST and websocket credential-rewrite rules without editing the baseline policy file.
    • Evidence: `openclaw-slack-pairing.test.ts` calls `premergeSlackPolicyIfNeeded`; the helper appends Slack endpoint YAML to `openclaw-sandbox.yaml`; the baseline policy comments say messaging endpoints are intentionally not in baseline and presets are the opt-in path.
  • Slack pairing test mutates the baseline policy instead of using the Slack preset source of truth (test/e2e-scenario/live/openclaw-slack-pairing.test.ts:96): The new live test calls `premergeSlackPolicyIfNeeded(cleanup)` before installation. That helper appends Slack endpoints into `nemoclaw-blueprint/policies/openclaw-sandbox.yaml` when the baseline lacks Slack, even though the baseline policy explicitly says Slack must stay out of the baseline and be enabled through `nemoclaw-blueprint/policies/presets/slack.yaml`. This keeps the migrated shell behavior, but it makes the test pass through a modified blueprint rather than proving the opt-in Slack preset/onboarding path that production users rely on.
    • Impact: A regression where the real Slack preset is not applied, is applied too late, or loses its credential-rewrite fields can be masked by the test's temporary baseline edit. Because the touched boundary controls sandbox egress and credential rewriting, masking that source-of-truth path can weaken confidence in policy enforcement without changing production code.
    • Recommendation: Prefer installing or applying Slack through the same preset/onboarding path that production uses, then assert the installed sandbox policy contains the Slack REST and websocket rules from `policies/presets/slack.yaml`. If the pre-merge workaround must remain for this migration, document the exact invalid state it works around, why the preset path cannot be exercised in this PR, and the condition for removing the workaround.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Verification hint: Read `test/e2e-scenario/live/openclaw-slack-pairing.test.ts` around the `premergeSlackPolicyIfNeeded(cleanup)` call, `test/e2e-scenario/live/openclaw-pairing-helpers.ts` for the helper body, `nemoclaw-blueprint/policies/openclaw-sandbox.yaml` around the Slack exclusion comment, and `nemoclaw-blueprint/policies/presets/slack.yaml` for the preset source of truth.
    • Missing regression test: Add or identify coverage that installs the Slack-enabled OpenClaw sandbox without mutating `openclaw-sandbox.yaml` and verifies the resulting sandbox policy has Slack REST request-body credential rewrite plus Socket Mode websocket credential rewrite from `policies/presets/slack.yaml` before the fake Slack pairing flow runs.
    • Evidence: `openclaw-slack-pairing.test.ts` calls `premergeSlackPolicyIfNeeded(cleanup)` before `installSandbox`; the helper appends Slack policy YAML to `openclaw-sandbox.yaml` if `api.slack.com` is absent; the baseline policy comment says messaging endpoints are intentionally not in the baseline and the presets are the opt-in path.
  • New live Slack test adds another execution path for an unpinned Docker image (test/e2e-scenario/live/openclaw-slack-pairing.test.ts:130): The new test starts fake Slack through `startFakeSlackApi`, which delegates to the existing fake-Docker-API helper that runs `node:22-bookworm-slim` by tag. The PR does not introduce that helper, but it does add a new workflow job that will pull and execute the floating image as part of a secret-bearing live E2E workflow.
    • Impact: A floating image tag can change independently of the repository, reducing reproducibility and widening the trusted-code boundary for a workflow that has access to DockerHub credentials and `NVIDIA_INFERENCE_API_KEY`. A malicious or compromised image update would run on the GitHub runner during this scenario.
    • Recommendation: Pin the fake API image to a digest in the shared helper, or provide a repository-level justification for why these E2E fake API containers intentionally remain tag-pinned only. If pinning now is too broad, make the new job use a pinned image path while preserving the fake Slack network and credential-rewrite assertions.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Verification hint: Read `test/e2e-scenario/live/openclaw-slack-pairing.test.ts` at the `startFakeSlackApi(...)` call and `test/e2e-scenario/live/messaging-providers-helpers.ts` in `startFakeDockerApi`, where the Docker args include `node:22-bookworm-slim` without a digest.
    • Missing regression test: Add a workflow-boundary or helper-level assertion that fake API Docker images used by live Vitest jobs are digest-pinned, or an explicit allowlist test documenting any approved floating test image exceptions.
    • Evidence: The changed Slack test invokes `startFakeSlackApi`; nearby helper code builds `docker run ... node:22-bookworm-slim node /opt/nemoclaw-e2e/fake-slack-api.cjs`.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • Runtime validation — Exercise `openclaw-slack-pairing-vitest` or the `openclaw-slack-pairing` scenario selector and verify install, fake Slack REST rewrite, fake Slack websocket rewrite, pending request creation, approval, allowFrom creation, and repeat approval fail-closed behavior.. This PR changes a secret-bearing GitHub Actions workflow and adds a live sandbox/network-policy E2E. Static inspection shows the test is focused and covers the main Slack pairing behavior, but the changed behavior ultimately depends on install.sh, Docker, OpenShell policy updates, fake Slack REST/websocket rewriting, and sandbox state.
  • Runtime validation — Add a support-test assertion that `evaluateE2eVitestWorkflowDispatchSelectors({ scenarios: "openclaw-slack-pairing" })` maps to `openclaw-slack-pairing-vitest` and does not route through the registry scenario runner.. This PR changes a secret-bearing GitHub Actions workflow and adds a live sandbox/network-policy E2E. Static inspection shows the test is focused and covers the main Slack pairing behavior, but the changed behavior ultimately depends on install.sh, Docker, OpenShell policy updates, fake Slack REST/websocket rewriting, and sandbox state.
  • Runtime validation — Add or identify coverage that Slack preset application succeeds without mutating `nemoclaw-blueprint/policies/openclaw-sandbox.yaml` before install.. This PR changes a secret-bearing GitHub Actions workflow and adds a live sandbox/network-policy E2E. Static inspection shows the test is focused and covers the main Slack pairing behavior, but the changed behavior ultimately depends on install.sh, Docker, OpenShell policy updates, fake Slack REST/websocket rewriting, and sandbox state.
  • Slack network policy setup for OpenClaw Slack pairing Vitest — Coverage should prove a Slack-enabled install applies the Slack preset-derived REST and websocket credential-rewrite rules without editing the baseline policy file.. `openclaw-slack-pairing.test.ts` calls `premergeSlackPolicyIfNeeded`; the helper appends Slack endpoint YAML to `openclaw-sandbox.yaml`; the baseline policy comments say messaging endpoints are intentionally not in baseline and presets are the opt-in path.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each finding. A human maintainer must make the final merge decision.

@cv

cv commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #5582, rebased onto the linear Discord pairing branch and updated for the codebase growth guardrail.

@cv cv closed this Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants