fix(agent-team): handle real slack_bolt event envelope + map thread replies; bind start invoker #26

Merged

amoussa1229 merged 2 commits from fix/agent-team-slack-inbound-envelope into main

2026-06-22 19:30:48 +00:00

amoussa1229 commented

2026-06-22 19:08:31 +00:00

(Migrated from github.com)

Summary

Live R720 P1 bring-up exposed that the Socket Mode inbound listener was unit-tested against a synthetic payload shape that slack_bolt never sends, so the suite was green while a real free-text Slack reply was silently dropped (clarifier question stayed open). Plus a CLI fix needed for the demo.

Commit 1 — slack envelope + thread-reply mapping (`0f3c0fa`)

Real slack_bolt delivers Events API as {"type":"event_callback","event":{"type":"message",...}}, and a free-text thread reply carries no callback_id/question_id/metadata. Three breaks fixed:

Type gate gated on the outer type (event_callback) → dropped. Now collapses to the inner event.type (_discriminating_type/_inner_event).
question_id recovery — when explicit-id recovery fails, resolve by the inner event.thread_ts against the OPEN ledger row whose channel_ref equals it (new find_open_question_by_channel_ref, constrained to status='open' as anti-replay). Explicit id still wins.
answer taken from event.text (stripped), stored as opaque data.

AUTHZ-01 unchanged and still runs FIRST (gates event.user, fails closed). Mapping only resolves which question, never who may answer.

Commit 2 — bind subscription invoker in `start` CLI (`ae5aa02`)

run-team.py start runs the clarifier graph (which calls Claude) in-process but never bound the invoker (only serve did) → "claude_invoke has no invoker bound". Now binds it, mirroring serve.

Verification

/sh-security-review: PASS — detector fan-out (authz/injection/logic) returned zero findings; verifier had no candidates. Auth-first preserved, parameterized SQL, status='open' anti-replay, opaque-data answer, malformed-envelope guards.
ruff check . + ruff format --check . clean; pytest (agent-team) 960 passed (+4 real-envelope regression tests replacing the synthetic fixtures).
Deploy + live end-to-end Slack verification: PENDING — box currently runs the PR #25 matcher fix; this will be rsync'd to sh-secrev and a real Slack thread reply verified BEFORE merge (deploy-then-merge).

Non-blocking follow-ups (noted by the review)

Pre-existing narrow TOCTOU KeyError in _question_turn/_question_thread not caught by the ValueError handler (not attacker-reachable).
No DB-level partial-UNIQUE on channel_ref WHERE status='open' (collision prevented emergently by Slack ts uniqueness).

## Summary Live R720 P1 bring-up exposed that the Socket Mode inbound listener was unit-tested against a **synthetic payload shape that slack_bolt never sends**, so the suite was green while a real free-text Slack reply was silently dropped (clarifier question stayed `open`). Plus a CLI fix needed for the demo. ### Commit 1 — slack envelope + thread-reply mapping (`0f3c0fa`) Real slack_bolt delivers Events API as `{"type":"event_callback","event":{"type":"message",...}}`, and a free-text thread reply carries no `callback_id`/`question_id`/metadata. Three breaks fixed: 1. **Type gate** gated on the outer `type` (`event_callback`) → dropped. Now collapses to the inner `event.type` (`_discriminating_type`/`_inner_event`). 2. **question_id recovery** — when explicit-id recovery fails, resolve by the inner `event.thread_ts` against the OPEN ledger row whose `channel_ref` equals it (new `find_open_question_by_channel_ref`, constrained to `status='open'` as anti-replay). Explicit id still wins. 3. **answer** taken from `event.text` (stripped), stored as opaque data. AUTHZ-01 unchanged and still runs FIRST (gates `event.user`, fails closed). Mapping only resolves *which* question, never *who* may answer. ### Commit 2 — bind subscription invoker in `start` CLI (`ae5aa02`) `run-team.py start` runs the clarifier graph (which calls Claude) in-process but never bound the invoker (only `serve` did) → "claude_invoke has no invoker bound". Now binds it, mirroring `serve`. ## Verification - `/sh-security-review`: **PASS** — detector fan-out (authz/injection/logic) returned zero findings; verifier had no candidates. Auth-first preserved, parameterized SQL, `status='open'` anti-replay, opaque-data answer, malformed-envelope guards. - `ruff check .` + `ruff format --check .` clean; `pytest` (agent-team) **960 passed** (+4 real-envelope regression tests replacing the synthetic fixtures). - **Deploy + live end-to-end Slack verification: PENDING** — box currently runs the PR #25 matcher fix; this will be rsync'd to sh-secrev and a real Slack thread reply verified BEFORE merge (deploy-then-merge). ## Non-blocking follow-ups (noted by the review) - Pre-existing narrow TOCTOU `KeyError` in `_question_turn`/`_question_thread` not caught by the `ValueError` handler (not attacker-reachable). - No DB-level partial-UNIQUE on `channel_ref WHERE status='open'` (collision prevented emergently by Slack ts uniqueness).

This repo is archived. You cannot comment on pull requests.

No reviewers