Skip to content

Add retry-safe live Slack contract tests - #406

Open
alex-clickhouse wants to merge 5 commits into
alex/slack-channel-notificationsfrom
alex/slack-channel-live-tests
Open

Add retry-safe live Slack contract tests#406
alex-clickhouse wants to merge 5 commits into
alex/slack-channel-notificationsfrom
alex/slack-channel-live-tests

Conversation

@alex-clickhouse

@alex-clickhouse alex-clickhouse commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Part 4 of the Slack channel stack. Adds tests that run against a real Slack
workspace, and the harness they need to be trustworthy. Everything here skips
unless the credentials are set, so a fork and the ordinary suite are
unaffected.

Stack

  1. Shared chat-channel foundations #404: shared chat-channel foundations
  2. Slack channel with coherent runtime and access guardrails #403: Slack channel, access guardrails, and runtime ownership
  3. Deliver notifications through Slack #405: Slack notification delivery
  4. Add retry-safe live Slack contract tests #406: real-Slack contract tests and diagnostics (this PR)

What only a real workspace can answer

The unit tests prove the channel is self-consistent against a fake. They
cannot tell you whether Slack accepts a Block Kit payload, whether an emoji
short name exists, whether users.info withholds an email rather than
failing, or whether an event survives the whole trip from a keystroke to an
InboundMessage. That is what these cover.

Two facts about Slack that shape the harness

Slack gives each event to exactly one of an app's open connections. Two
test runs at once therefore steal each other's events rather than both seeing
them. So CI serializes on the workspace, not on the branch: the concurrency
group is a fixed name, and an already-running job is allowed to finish rather
than being cancelled, because cancelling it would leave its events
unacknowledged.

Slack retries anything left unacknowledged, on a schedule: immediately,
then at about +60s and +5min. A run that ends with unacknowledged events
therefore poisons the next run, which sees them arrive as if they were its
own. Two consequences shape the layout:

  • The outbound tests post, edit, upload, and delete. Each of those produces an
    event. They run with a socket open that acknowledges and discards
    everything, so their own traffic never comes back later.
  • The inbound tests are the only thing that interprets events, and they run in
    a separate process so the two roles cannot overlap.

Making the tests mean something

A refusal test has to prove the event arrived. Waiting and finding the
router empty does not distinguish a guardrail that refused the message from an
event Slack never delivered, one it handed to another connection, or one the
harness dropped as stale. Each refusal test now waits for the envelope to
reach the channel first, then asserts nothing came out of it, and says which
of the two failed. Without that, deleting the access policy altogether left
these tests green.

Old events are dropped, in the harness only. A test only ever waits for a
message it just posted, so a retried envelope or one older than the delivery
timeout belongs to an earlier test or an earlier run. Production does the
opposite on purpose: handling a retry is how a message survives a restart.

Event age is measured on Slack's clock. The age of an event is the gap
between a timestamp Slack wrote and a reading of the local clock, which are
two different clocks. A runner drifted ahead of Slack by more than the cutoff
would call every fresh event stale and drop it, and on the refusal tests that
reads as a guardrail working. The offset is measured from a probe's own
timestamp instead of assumed.

Readiness is a fresh event, not a connected socket. While retries are
outstanding a newly opened socket is deaf to new events for 20 to 30 seconds.
Setup posts a probe and waits for that message's own timestamp to arrive
unretried, re-probing until it lands, then waits for its deletion event too.
That makes the round trip a fence: once it returns, it has left nothing of its
own unacknowledged.

Cleaning up after itself

The suite deletes the messages, reactions, and files it creates. Messages go
through chat.delete; a file is not a message, so uploads are tracked by id
and removed with files.delete. Items are recorded before the assertions that
could fail, so a failure part way through does not leave data behind. An
interrupted run can still leave some, which is why the docs call for a
throwaway workspace.

Diagnostics

The harness emits one-line SLACK_LIVE JSON records for connections, probes,
retries, event ages and types, and per-socket summaries. Secrets reach them
through a field allow-list, and CI scopes the tokens to the two steps that
need them so GitHub masks them as well.

docs/testing-slack.md covers the scratch-workspace setup, the required
scopes, and the two-process layout.

@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 8a17b88 to e5400e9 Compare August 24, 2026 07:15
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch 3 times, most recently from 41ea8ab to d05ba74 Compare August 24, 2026 07:43
@alex-clickhouse
alex-clickhouse marked this pull request as ready for review August 24, 2026 09:32
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch 2 times, most recently from 6f2cd50 to eaeddd5 Compare August 24, 2026 11:17
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch 3 times, most recently from cc477da to fb11c30 Compare August 24, 2026 11:53
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from fb11c30 to 54ccbdc Compare August 24, 2026 12:38
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 54ccbdc to 8efe050 Compare August 25, 2026 07:54
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 8efe050 to 3a9f203 Compare August 25, 2026 08:19
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 3a9f203 to 61ab950 Compare August 25, 2026 09:41
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 61ab950 to 3abe891 Compare August 25, 2026 09:44
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 3abe891 to b91d5ca Compare August 25, 2026 10:59
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from b91d5ca to 4b61e4c Compare August 25, 2026 13:48
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from 00e48f9 to cbae554 Compare August 25, 2026 14:43
Exercise the channel against a real workspace in a globally serialized workflow. Keep all outbound mutations acknowledged, fence phase transitions with fresh probes, discard stale test-only callbacks, and emit structured Socket Mode diagnostics without message content or credentials.
The four guardrail tests waited and asserted the router stayed empty. An
event Slack never delivered, one it gave to another connection, and one the
harness dropped as stale all look identical from there, so removing the
policy altogether left them green. Wait for the envelope to reach the
channel first, then assert nothing came out of it, and say which of the two
failed. The settle after arrival covers dispatch rather than delivery, so
it no longer sits at under a third of the delivery timeout.

The staleness cutoff subtracted a stamp Slack wrote from a reading of the
runner's clock. A runner ahead of Slack by more than the cutoff called every
fresh event stale and dropped it, which on those same refusal tests reads as
a guardrail doing its job. Measure the offset from a probe's own ts and
treat an uncalibrated harness as having nothing stale.

An upload is not a message, so chat.delete never removed the file the
send_file test creates and every run added another to the scratch channel.
Record file ids and remove them with files.delete. Track the streaming
placeholder before the edits and the bot's reply before its text is
checked, so a failure part way through leaves nothing behind.

The integration test's probe client returned a counter where Slack returns
epoch seconds, which left the cutoff it exercises uncalibrated. It also now
asserts the fresh probe arrived, not only that the stale one did not.
The reaction test cached a bare conversation target. Nothing in the channel
writes one for a shared channel: a top-level message is the root of its own
thread, so every entry carries a thread ts. Only a notification card sits at
conversation level, and a reaction there now has no session to join.

Cache the anchor against its own thread, which is the target the channel
would have stored for it.
@alex-clickhouse
alex-clickhouse force-pushed the alex/slack-channel-live-tests branch from cbae554 to 2fbf1fa Compare August 26, 2026 11:59
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.

1 participant