test(slack-bridge): structural TOFU + load_allowed() regression tests (#899 companion) by bassilkhilo-ag2 · Pull Request #1190 · sonichi/sutando

bassilkhilo-ag2 · 2026-05-26T19:27:09Z

What

11 source-inspection tests for slack-bridge.py's TOFU onboarding + load_allowed() function — ported from tests/telegram-bridge-tofu.test.py (same pattern used by tests/slack-bridge-access.test.py).

No production code changes. Tests use regex over the source file to avoid the slack_bolt import-at-module-load issue.

Why

Issue #899 was diagnosed as intermittent: access.json gets wiped between slack-bridge events. PR #901 (merged) added per-event probes to catch the wipe window. These structural tests guard the TOFU/load_allowed() contract independently of the runtime probe — locking in:

tofu_onboard() payload shape (allowFrom, tofuOwner, tofuOnboardedAt, tofuOnboardedUsername)
Race-safety: ACCESS_FILE.exists() guard prevents clobber of existing config
chmod 0o600 applied after write (same security fix as telegram-bridge: chmod 0o600 on TOFU-written access.json #810 for Slack)
None username stored as JSON null, not the string "None"
from __future__ import annotations before str | None annotation (Python 3.9 compat)
load_allowed() tri-state: None on FileNotFoundError (TOFU signal), set() on other exceptions (fail-closed), set(allowFrom) on success

Tests

All 11 tests passed

🤖 Generated with Claude Code

Ports the guard coverage from tests/telegram-bridge-tofu.test.py to the Slack bridge (source-inspection approach — avoids the slack_bolt dep). 11 tests across 6 categories: (f) load_allowed() tri-state: None/FileNotFoundError, set()/other-exception, set(allowFrom)/success — the load-bearing distinction TOFU relies on. (a) tofu_onboard payload: allowFrom, tofuOwner, tofuOnboardedAt, tofuOnboardedUsername all present. (b) Race-safety: ACCESS_FILE.exists() guard prevents clobbering existing config. (c) chmod 0o600 applied after write — owner's Slack ID must not be world-readable. (d) None username → JSON null, not the literal string "None". (e) py39-compat: str | None annotation present + from __future__ import annotations appears before def tofu_onboard (validates the Pass 41 fix). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

VasiliyRad

All 11 tests pass against current src/slack-bridge.py. Single file addition, no production or unrelated changes. Tests cover the six contracts that matter: tri-state load_allowed() (None/set()/set(allowFrom)), TOFU payload shape, race guard, chmod 0o600, None-as-null username, and py3.9 future import ordering. Source-inspection approach correctly avoids the slack_bolt dep issue. Clean companion to #899.

VasiliyRad

Clean test addition (1 file, +239). Structural TOFU + load_allowed regression coverage for the slack-bridge access path — appropriate companion to #1186 (mtime-cache refactor) which I just approved. Pins the None-vs-empty-set semantics so a future refactor can't quietly collapse them. No source changes, no bundling.

LGTM 👍

One line read `--name mars read '<room>' # publish presence while connected`, which is the exact trap that just had to be fixed in the summon hint: every subcommand except `watch` opens the document, does one thing and closes, so presence published by a `read` is gone before anyone looks at the facepile. An agent following the skill would join invisibly and have no way to tell. The example now shows `watch` with the two flags that make it work, and a note above the flag-ordering rule says why: `watch` publishes presence only when given `--name`, and `--user-id` is what lets a summon card draw the agent's real avatar instead of a placeholder. Pairs with cinny-webclient #1190, which changed the summon hint itself from `read` to `watch --name --user-id`. Without this the skill and the hint disagree, and the skill is what an agent reads first. Added to this PR rather than opened as its own: it edits the same file, so two PRs would conflict, and the owner asked for fewer of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(room-collab): point the skill at the shapes reference before storage decisions The skill tells an agent how to hold a connection and send deltas, then leaves it to decide where anything lives. That decision is where the expensive mistakes are, and the failure has no symptom: a row that was written is not in the list, with no error and no gap. CRDT-SHAPES.md is the measured answer and landed in #4558. This cites it from the one place someone is standing when they need it, rather than restating any of it - a second copy of that page would drift from the measurements. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(room-collab): the skill said a `read` publishes presence; it cannot One line read `--name mars read '<room>' # publish presence while connected`, which is the exact trap that just had to be fixed in the summon hint: every subcommand except `watch` opens the document, does one thing and closes, so presence published by a `read` is gone before anyone looks at the facepile. An agent following the skill would join invisibly and have no way to tell. The example now shows `watch` with the two flags that make it work, and a note above the flag-ordering rule says why: `watch` publishes presence only when given `--name`, and `--user-id` is what lets a summon card draw the agent's real avatar instead of a placeholder. Pairs with cinny-webclient #1190, which changed the summon hint itself from `read` to `watch --name --user-id`. Without this the skill and the hint disagree, and the skill is what an agent reads first. Added to this PR rather than opened as its own: it edits the same file, so two PRs would conflict, and the owner asked for fewer of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…lpart (#4596) * fix(room-collab): watch defaults its presence name to the mxid's localpart `watch` called `set_presence` only when `--name` was given, so omitting it produced a connection that looked joined to the agent and absent to everyone else. A nameless peer is not rendered anywhere: the web client's `peersOf` skips an awareness state without a non-empty name, and the service's presence summary counts such a peer without naming it. That is the same silent-no-op shape as the `read`-instead-of-`watch` bug this follows — the command does not do the thing it appears to do, and nothing says so. An mxid already carries a usable name, and it is the one the summon hint fills in, so default to its localpart rather than requiring a flag nobody can tell they forgot. The derivation moves into `presence_name()` at module scope so it can be tested without a socket, rather than staying an inline expression inside `watch`'s connection loop. Verified: - tests/room-collab-presence-name-default.test.py — 10 cases: explicit name wins and is trimmed, blank falls through to the localpart, `@weird:host:port` takes only the localpart, and a malformed or empty id yields None so the caller skips `set_presence` rather than publishing an empty name the readers would drop anyway. Auto-discovered by scripts/discover-python-tests.sh, and it SKIPs cleanly where the room-collab deps are absent. - Two mutants killed: dropping the localpart branch fails "localpart: got None"; accepting any string as an mxid fails "malformed id: got 'ot-an-mxid'" — which is the exact garbage the `@`/`:` guard exists to reject. - review-checks and lint-workspace-resolution clean on the diff. Pairs with cinny-webclient #1190, whose summon hint passes `--name` explicitly; this is the floor for an agent that omits it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(room-collab): drop the retired ROOM_DOC_DB literal from the new test CI's old-collab-literals ratchet failed: the new test added one retired spelling (0 -> 1), and the ratchet forbids growth. The env var was not needed. It was defensive — room_doc.py makes its store directory at import — but this test loads the CLI module, whose client import sits inside functions, so nothing touches that path. Verified by running the test with and without the room-collab deps: both pass. One root cause, three red checks: the ratchet runs in 'python standalone tests (2/3)' and in typecheck's 'Lint skill manifests' step, and 'tsc + tests (clean install)' is the rollup that requires both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

VasiliyRad approved these changes May 26, 2026

View reviewed changes

VasiliyRad mentioned this pull request May 27, 2026

tests: add Python tests for bridges TOFU, Py3.9 compat, and task-file fields (29 tests) #1226

Closed

3 tasks done

bassilkhilo-ag2 mentioned this pull request May 27, 2026

tests(discord-bridge, telegram-bridge): task-file fields + allowlist coverage #1239

Closed

bassilkhilo-ag2 closed this May 27, 2026

qingyun-wu mentioned this pull request Sep 22, 2026

docs(room-collab): cite the CRDT shapes reference from the skill #4559

Merged

Navigation Menu

Repository navigation

test(slack-bridge): structural TOFU + load_allowed() regression tests (#899 companion) - #1190

test(slack-bridge): structural TOFU + load_allowed() regression tests (#899 companion)#1190
bassilkhilo-ag2 wants to merge 1 commit into
sonichi:mainfrom
bassilkhilo-ag2:test/slack-bridge-tofu-899

bassilkhilo-ag2 commented May 26, 2026

Uh oh!

VasiliyRad left a comment

Uh oh!

VasiliyRad left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bassilkhilo-ag2 commented May 26, 2026

What

Why

Tests

Uh oh!

VasiliyRad left a comment

Choose a reason for hiding this comment

Uh oh!

VasiliyRad left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants