test(slack): delivery trace goldens for buffered native streaming and block finals#104583
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 11, 2026, 1:26 PM ET / 17:26 UTC. Summary PR surface: Source +45, Tests +637. Total +682 across 6 files. Reproducibility: not applicable. This PR characterizes existing Slack delivery behavior rather than reporting a failing user-visible bug. The scenarios are source-verifiable against current production branches and the pinned Slack SDK. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Land the deterministic Slack traces as the Phase 0 conformance oracle after the branch receives the repository’s normal maintainer refresh and exact-head gates, preserving the documented intentional sensitivity to the pinned Slack SDK buffering contract. Do we have a high-confidence way to reproduce the issue? Not applicable; this PR characterizes existing Slack delivery behavior rather than reporting a failing user-visible bug. The scenarios are source-verifiable against current production branches and the pinned Slack SDK. Is this the best way to solve the issue? Yes; extending the existing shared delivery-trace harness at the real Slack dispatch seam is the narrowest maintainable way to lock wire behavior before the channel refactor, without adding a parallel runtime path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 702ba19e3aba. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +45, Tests +637. Total +682 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Merged via squash.
|
Related: #104318
What Problem This Solves
Extends the delivery trace harness (#104478) to Slack — the buffered-ack native-streaming channel whose accepted-vs-visible semantics and stop-as-first-network-call fallback are core inputs to v2's tri-state write-ack design.
Why This Change Was Made
Five golden scenarios drive real
dispatchPreparedSlackMessagewiring with a recording WebClient and the real @slack/web-api ChatStreamer: native streaming (tool payload accepted with zero wire traffic under the 256-char SDK buffer; visibility landing at stream start); stop-as-first-network-call (benign rejection → durable full-text postMessage fallback, proving the SDK private-buffer reach-in); blocks+fallback-text final duality; cancel (draft deleted); and documentation that the custom-identity preview-edit tier is structurally unreachable (draft streaming disabled when identity is set because chat.update cannot preserve authorship) — direct evidence for v2's contextual capability predicates. All ts values normalized.User Impact
None at runtime — test infrastructure.
Evidence
tbx_01kx92cngs8qpp2gm79gdrrtam: full slack suite (118 files / 1822 tests) +pnpm check:changedgreen; trace-onlytbx_01kx92s0dfvmwtg89scw3hhhkg5/5; byte-stable across macOS record + double verify + Linux Node 24 verify.