fix(qa): restore Slack transport under isolated installs - #137283
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Dependency GuardThis PR changes dependency-related files. Maintainers should confirm these changes are intentional. Changed files:
Maintainer follow-up:
|
Dependency graph changes notedThis PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin, a member of
Security review is still recommended before merge when the dependency graph change is intentional. |
|
Codex review: needs maintainer review before merge. Reviewed September 3, 2026, 8:15 AM ET / 12:15 UTC. ClawSweeper reviewWhat this changesThe PR adds the existing pinned Slack Web API package to the root dependency graph and tests that private QA’s root distribution can resolve it after an isolated pnpm install. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readiness✅ Ready for maintainer review No blocking correctness findings. This active member-authored PR repairs a current-main isolated-install failure at the root build boundary; current main still lacks the root dependency declaration, so the PR remains necessary. Priority: P2 Review scores
Verification
How this fits togetherPrivate QA compiles its QA Lab transport code into the root distribution, where it imports the Slack plugin’s public client API. That client retains the Slack SDK as an external Node dependency, which must resolve from the root distribution before credential acquisition and the QA scenario begin. flowchart LR
A[Isolated pnpm install] --> B[Private QA build]
B --> C[Root distribution]
C --> D[QA Lab Slack adapter]
D --> E[Slack plugin client API]
E --> F[External Slack SDK]
F --> G[Credential and transport scenario]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceTests +23, Config +1, Other +3. Total +27 across 3 files. View PR surface stats
Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Retain the root declaration at the plugin’s existing pinned version and the root-dist resolver regression test, while keeping the Slack plugin as the owner of its SDK usage. Do we have a high-confidence way to reproduce the issue? Yes. The current source proves the root-built QA adapter reaches an externalized Slack SDK, and the linked issue supplies a concrete isolated-install build/run failure before any credential or Slack API call. Is this the best way to solve the issue? Yes. Declaring the existing exact plugin SDK version at the root is the narrow owner-boundary repair; bundling the SDK would contradict the explicit externalization contract and moving ownership from the Slack plugin would be broader and less maintainable. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e0dad1fc1333. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
7f0aeaf to
14d08f7
Compare
…36761-session-mode-local-20260903 * commit '58187248c8cafd7b8cfbba3de8d001762fb08b48': improve(testing): add watchOS Gateway upgrade survivor (#137203) improve(ui): structure Devices rows with an actions menu and a facts panel (#137084) test(cli): reuse canonical pairing call guard (#137210) fix(sessions): prevent gateway stalls during large session cleanup (#126035) fix(qa): resolve Slack runtime dependency (#137283) fix: fence Session Observer catalog requests (#137243) fix(memory): make watcher pressure guidance actionable (#137181) test(entry): type the respawn exit mock directly (#137258) fix(release): install first-hop fixtures through the candidate registry (#137286) # Conflicts: # scripts/e2e/lib/upgrade-survivor/assertions.mjs
Fixes #137245
What Problem This Solves
Fixes an issue where maintainers running the source-only private QA Slack transport would see it fail before credential acquisition after an isolated pnpm install.
Why This Change Was Made
The private QA build emits its Slack consumer under the root
distgraph while deliberately externalizing@slack/web-api. The root package now declares the same pinned SDK version as the Slack plugin, so Node can resolve that external dependency under pnpm's isolated linker without changing plugin ownership or bundling topology.User Impact
Slack QA and RTT qualification can start the live transport again after an isolated frozen install.
Evidence
Cannot find package '@slack/web-api'.check:changedpassed, including typecheck, lint, dependency, SDK, boundary, and import-cycle gatesSLACK_WEB_API_RESOLVED=8.0.0SLACK_WEB_API_RESOLVED=8.0.0git diff --checkpassed.