test(e2e): migrate test-openclaw-slack-pairing.sh to vitest - #5582
Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughMigrates the OpenClaw Slack pairing E2E scenario from a legacy shell script to a typed Vitest test. Shared pairing helpers are extended with Slack channel support, a Socket Mode pairing script, strict proxy validation, and fake Slack API harness. A new CI workflow job and workflow boundary validator are added. ChangesOpenClaw Slack Pairing Vitest E2E Migration
Sequence Diagram(s)sequenceDiagram
participant Test as openclaw-slack-pairing.test.ts
participant Helpers as openclaw-pairing-helpers.ts
participant FakeSlack as Fake Slack API (Docker)
participant Sandbox as OpenClaw Sandbox
participant Script as SLACK_PAIRING_SCRIPT
Test->>Helpers: startFakeSlackApi(botToken, appToken, ...)
Helpers->>FakeSlack: docker run fake-slack-api
FakeSlack-->>Helpers: container + port
Test->>Helpers: issuePairingRequest({channel: "slack", fakeSlackPort})
Helpers->>Script: spawn Node with SLACK_PAIRING_SCRIPT
Script->>FakeSlack: WebSocket connect (Socket Mode)
FakeSlack-->>Script: hello event + pairing challenge payload
Script->>Sandbox: POST chat.postMessage (pairing reply via HTTP proxy)
Sandbox-->>Script: 200 OK
Script-->>Helpers: output with pairing code
Helpers-->>Test: ShellProbeResult
Test->>FakeSlack: read capture log (assertSlackCapture)
FakeSlack-->>Test: captured requests (token rewrites, body content)
Test->>Sandbox: approvePairing(extractedCode)
Sandbox-->>Test: pairing confirmed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
…ase6-openclaw-slack-pairing-linear
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27955653081
|
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27959029453
|
…ing-linear' into e2e-phase6-openclaw-slack-pairing-linear # Conflicts: # test/e2e-scenario/live/openclaw-pairing-helpers.ts
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27963222924
|
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27963225384
|
…iscord-pairing-linear
…iscord-pairing-linear
…ing-linear' into e2e-phase6-openclaw-slack-pairing-linear # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml
…iscord-pairing-linear
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27977501441
|
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27977502816
|
…ing-linear' into e2e-phase6-openclaw-slack-pairing-linear # Conflicts: # test/e2e-scenario/live/openclaw-pairing-helpers.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e-scenario/live/openclaw-pairing-helpers.ts (1)
505-508: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueSlack output omits
replyText, diverging fromextractPairingResultcontract.The Slack script outputs
{ code, senderId, channelId }butextractPairingResult(line 527) expects areplyTextfield. The downstream test correctly usesextractPairingCode, so this isn't a bug now—but if someone later callsextractPairingResultwith Slack output, it will fail with "missing pairing reply text".Consider either:
- Adding
replyTextto the Slack output for parity with Discord, or- Documenting that
extractPairingResultis Discord-only andextractPairingCodeis channel-agnostic.🤖 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/e2e-scenario/live/openclaw-pairing-helpers.ts` around lines 505 - 508, The console.log statement that outputs "PAIRING_E2E_RESULT" is missing the replyText field that the extractPairingResult function expects according to its contract. Add a replyText field to the object being logged in the Slack pairing output to match the structure that extractPairingResult requires, ensuring consistency with the Discord output format and preventing future failures if extractPairingResult is called with this Slack output.
🤖 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/e2e-scenario/live/openclaw-pairing-helpers.ts`:
- Around line 505-508: The console.log statement that outputs
"PAIRING_E2E_RESULT" is missing the replyText field that the
extractPairingResult function expects according to its contract. Add a replyText
field to the object being logged in the Slack pairing output to match the
structure that extractPairingResult requires, ensuring consistency with the
Discord output format and preventing future failures if extractPairingResult is
called with this Slack output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dc70a3d6-ef27-4223-ab95-9402db4ee690
📒 Files selected for processing (4)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/openclaw-pairing-helpers.tstest/e2e-scenario/live/openclaw-slack-pairing.test.tstest/e2e-scenario/support-tests/openclaw-discord-pairing-helpers.test.ts
Vitest E2E Scenario Results —
|
| Job | Result |
|---|---|
| openclaw-discord-pairing-vitest |
Vitest E2E Scenario Results —
|
| Job | Result |
|---|---|
| openclaw-slack-pairing-vitest |
…lack-pairing-linear # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml # test/e2e-scenario/live/openclaw-pairing-helpers.ts # test/e2e-scenario/support-tests/openclaw-discord-pairing-helpers.test.ts
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
Related issue #5098
Changes
test/e2e-scenario/live/openclaw-slack-pairing.test.tsas Vitest coverage fortest/e2e/test-openclaw-slack-pairing.sh.openclaw-slack-pairing-vitestinto.github/workflows/e2e-vitest-scenarios.yaml.Type of Change
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Issue #5098 Acceptance Mapping
Relevant #5098 clauses and this PR evidence:
Goal: "Migrate every direct legacy bash E2E entry point under
test/e2e/test-*.shinto the typed Vitest E2E system" →test/e2e-scenario/live/openclaw-slack-pairing.test.tsreplacestest/e2e/test-openclaw-slack-pairing.shwith typed Vitest coverage.Core Principle: "If a contract needs shell, process, host, platform, installer, or sandbox behavior, preserve that boundary from a typed Vitest test" → the Vitest test keeps the real install/OpenShell sandbox boundary, fake Slack Socket Mode/REST server, sandbox exec probes, and connect-shell approval flow.
Definition of Done 1-2: "Equivalent typed Vitest coverage exists" and "preserves the same user-visible contract" → the live test covers Slack Socket Mode pairing challenge creation, fake REST
chat.postMessage, credential rewrite evidence, pairing approval, and allowFrom persistence.Definition of Done 3: "deterministic artifacts, cleanup, timeout behavior, and secret redaction" → artifacts are written under
e2e-artifacts/vitest/openclaw-slack-pairing/, cleanup is registered through the scenario cleanup registry, and fake Slack capture assertions verify secrets are redacted/re-written.Definition of Done 4: "PR explains the contract mapping" → this section plus the Summary/Changes describe the legacy-to-Vitest mapping.
Definition of Done 5: "replacement test is wired into
.github/workflows/e2e-vitest-scenarios.yaml" →openclaw-slack-pairing-vitestis a free-standing dispatchable workflow job.Shell deletion deferral: "Default conversion PRs should defer legacy shell script deletion unless explicitly scoped to retire the script" → this PR leaves
test/e2e/test-openclaw-slack-pairing.shdeletion to Phase 11 cleanup.Issue comments:
gh issue view 5098 --json commentsreturned 0 comments during PR validation, so there are no separate Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 comment clauses to map beyond the issue body clauses quoted above.Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Release Notes