feat(slack): partner follow-ups — per-agent bots, per-channel consent, completion report-back (ent#222/#223/#224)

[dolho]

The OSS half of the three Slack issues from the 2026-07-23 design-partner call. Consolidated into one PR because they are one body of work: they share a migration chain, ent#224 is gated by ent#223's consent flag, and all three touch the same Slack path.

Pairs with trinity-enterprise#232 (the private module: bindings, transport manager, cascade). Two PRs total — one per repo. The repo boundary is the only reason this isn't a single PR.

Supersedes #1761, #1762, #1758 (closed in favour of this).

---

ent#223 — per-channel proactive consent

Consent was per-recipient (agent_sharing.allow_proactive, keyed by verified email). In an open Slack workspace nobody authenticates, so there was no recipient to opt in — and channel posts had no consent gate at all.

• slack_channel_agents.allow_proactive, dual-track (SQLite + Alembic 0030)
• Split default so nothing silently flips: NEW bindings deny; EXISTING bindings backfilled to allow (they worked ungated before). Backfill runs only when this caller added the column, so a later explicit OFF survives re-runs.
• Gate sits between the 404 (not bound) and 429 (rate capped) → three distinguishable reasons; returns named proactive_not_allowed
• Owner-gated toggle endpoint + the switch in SlackChannelPanel.vue
• MCP send_group_message now reports why: consent_required / rate_limited / not_bound

ent#224 — completion reported back to the originating channel/thread

The partner's bug: agent delegates a ~10-min job, it succeeds, nobody tells the user. Two things were missing (the issue called it "a join" — it wasn't):

1. Channel context didn't survive delegation — the A→B path never forwarded source_channel*, so B had no destination. Added parent_execution_id + inheritance, and an optional execution_id on MCP chat_with_agent.
2. Nothing reported at the terminal — new channel_completion_report, hooked at both CAS-won chokepoints beside spawn_task_terminal_event (feat: system-emitted agent.task.completed/failed events at execution terminal (async caller report-back) #1578). Success and failure.

⚠️ The load-bearing safety rule: a direct channel turn is synchronous and already replies inline (triggered_by == the channel name). Reporting those would duplicate every normal Slack reply in a customer workspace, so reporting is restricted to executions that inherited context. Consent-gated, at-most-once via effect_guard keyed on the resolved destination.

ent#222 — per-agent bot identities (OSS half)

• adapters/per_agent_bot.py — inert-by-default hook registry; with no resolver registered every path is a no-op and routing is byte-for-byte unchanged. Both hooks fail open.
• message_router resolves a per-agent bot over the channel binding; slack_adapter stamps the receiving bot identity
• SlackAgentBotPanel.vue — entitlement-gated config UI (resolved identity shown; tokens write-only)

---

Verified on a live PostgreSQL stack (not just unit tests)

25 unit tests, plus a real deployment:

• Migrations applied on real PG; single linear Alembic head (0030 → 0029_product_events)
• ent#223: consent OFF → 403 · unbound → 404 · toggle ON → gate passes · toggle OFF → 403 returns
• ent#224: delegated terminal posted into the originating thread; replay through the real effect_guard → no second send; inline turn → no send
• ent#222: real Slack auth.test reached Slack and mapped to named invalid_bot_token; cascade proven by a real rename (binding followed the agent)

🔴 A boot-breaking bug this deployment caught

0029_slack_channel_allow_proactive collided with 0029_product_events (already on dev) — two Alembic heads → upgrade head fails → backend crash-loops. Reproduced (HTTP 000), then fixed by rebasing to 0030. No unit test could have found this. My eval PR #1752 had the identical defect; it's fixed there too (0031, chained off 0030) — merge #1752 after this.

Honest limits

• No live Slack workspace — sends are mocked. Socket connect, inbound routing, and the actual post are unproven end-to-end; needs a real per-agent Slack app from the partner.
• MCP TypeScript changes couldn't be compiled offline; the UI compiles under Vite but wasn't clicked through in a browser.

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[dolho]

/review report — feat/slack-partner-followups → dev

Files: 25 (+1284/−12) · Merge-base: 49b235c7 · Scope: CLEAN (every file traces to ent#222/#223/#224; no drift)

Self-review caveat: I wrote this code, so I weighted findings toward "assume I got it wrong." Two criticals surfaced — both fixed in this PR, both invisible to the existing tests because those covered the db/service layer, not the principal or the egress.

---

🔴 C1 — An agent could grant ITSELF proactive consent (Confidence 9/10) — FIXED

File: src/backend/routers/slack.py (set_slack_channel_proactive)

if not db.can_user_share_agent(current_user.username, name):
    raise HTTPException(status_code=403, ...)

Issue: dependencies.py:423 states it plainly — "Agent-scoped keys resolve to the owner user on REST, which incidentally made sharing, permission grants, rename, and credential ops reachable by an agent." So this check passes for an agent's own key. An agent could therefore flip on the exact control ent#223 exists to enforce, making consent self-serve and the whole feature decorative.

Why it matters: same class as the retention-acknowledge trap (trinity-ops-agent#232), where require_admin alone was insufficient for the same reason.

Fix applied: reject_agent_principal(current_user) before the owner check. Granting consent is a human decision; sending under it stays agent-callable (unchanged).

---

🔴 C2 — Unsanitised failure text posted into a customer Slack channel (Confidence 9/10) — FIXED

File: src/backend/services/channel_completion_report.py (_summarize)

body = (summary_or_error or "").strip()
if len(body) > _MAX_REPORT_CHARS:
    body = body[:_MAX_REPORT_CHARS].rstrip() + "…"

Issue: the #1578 emit chokepoint credential-sanitises summary_or_error before egress precisely because a failure terminal can carry secrets (event_dispatch_service.py:311). This path truncated but never sanitised — so a token in an error message would be posted verbatim into a persistent, externally-hosted, human-visible surface. Strictly worse than the #1578 case it was modelled on.

Compounding it: the bare slice is the pattern #1578 explicitly warns against — cutting a secret in half can stop the redaction pattern matching, so it survives.

Fix applied: sanitize_text over a 2× window before truncating — same order as #1578. Regression test asserts a bot token in failure text never reaches the posted message.

---

🟡 Informational

• I1 — 0030 backfill is intentional but load-bearing: it flips every existing binding to allow_proactive = 1. Correct (posts were ungated before, so denying would break live integrations) and guarded so a later explicit OFF survives re-runs — but it is a data-touching migration, worth a reviewer's eye.
• I2 — #1649 stub was a false green: test_slack_router_does_not_persist_a_failed_send asserts pytest.raises(HTTPException), and the new consent 403 is one — so it passed without ever reaching the 502 path it exists to cover. CI could not flag this (it stayed green). Fixed by marking the stub consented.

✅ Clean (verified, not assumed)

• SQL safety — no string interpolation introduced; all new queries use text() with bound params
• Credential exposure in logs — reporter logs team_id/execution_id/channel only, never a token
• CAS-won placement — both ent#224 hooks sit inside the CAS-won branches beside spawn_task_terminal_event, so a lost CAS reports nothing
• Idempotency — effect_guard keyed on the resolved destination, never the generated body; verified against the real table (replay → no second post)
• The no-double-post rule — inline channel turns (triggered_by ∈ slack/telegram/whatsapp) never report; proven live and in tests

Summary

Critical: 2 — both fixed in this PR · Informational: 2 · Scope: clean

[dolho]

/review — re-run after fixes

Both criticals are now verified present on the branch (2a0e8fdf), not just claimed:

	Status	Evidence on origin
C1 agent could self-grant consent	✅ Fixed	reject_agent_principal(current_user) runs before the owner check in set_slack_channel_proactive
C2 unsanitised failure text → Slack	✅ Fixed	sanitize_text(window) over a 2× window before truncation

Tests: 14 green, including two new regressions — a static guard that the toggle keeps its human-only check, and one asserting a bot token in failure text never reaches the posted message.

Three things the re-review caught that the first pass did not

1. My first "fixed and pushed" was false. The review worktree was in detached HEAD, so git push origin feat/... pushed a stale local branch while my commit went nowhere — and my success message came from an echo that runs unconditionally. The remote had zero of the fixes when I reported them as landed. Now verified by reading the pushed files back from origin rather than trusting the push command.

2. My own C2 fix had a latent flaw. The first version decided truncation with len(raw) > len(body) — but sanitize_text changes length, so a short redacted message would get a phantom "…" implying content was cut when it wasn't. Truncation is now decided on what was actually cut, with a dedicated test.

3. Push protection rejected the regression test — it hardcoded a realistic xoxb-… literal in a public repo, which is the "hardcoded credential in a test" this very checklist forbids. The token is now assembled at runtime, so no literal is committed while the sanitiser still sees a well-formed token.

Unchanged from the first pass

Informational: the 0030 backfill is intentional but data-touching; the #1649 stub was a false green (fixed). Clean: SQL safety, credential-free logging, CAS-won hook placement, effect_guard idempotency, and the no-double-post rule.

Critical: 0 outstanding · CI re-running on 2a0e8fdf.

[vybe]

Validated via /validate-pr (paired with trinity-enterprise#232). Approving. Since dev's required checks run no unit tests, I ran all four touched test files locally on this branch: 41 passed (test_222_per_agent_bot_seam, test_223_slack_channel_proactive_consent, test_224_channel_completion_report, updated test_1649). Load-bearing integration points verified against the real code, not just the diff:

• effect_guard usage matches the real #1084 API exactly (async CM, g.replay/g.snapshot, EffectInProgressError before yield); db.get_execution returns the ScheduleExecution model (attribute access safe, source_channel* fields present); send_message_detailed kwargs match.
• Dual-track migration complete and correctly chained: schema.py + tables.py + SQLite migrations.py + Alembic 0030 revising 0029_product_events (the real dev head) — the schema-parity blind spot covered.
• Consent posture is the right one and is proven: new-binding-denies + existing-backfilled-to-allow tested against a real legacy table, including idempotency (an operator's explicit OFF survives re-runs).
• The human-only consent toggle (reject_agent_principal before the owner check — access-first, self-uniform per Invariant #8) plus its guard test and the learnings entry: exactly the right response to the self-grant class.
• Sanitize-over-2x-window-before-truncate in the completion reporter matches the #1578 chokepoint discipline, with the phantom-ellipsis edge tested. The seam is inert-by-default and fail-open (failure-injection tested); router early-return conforms to _resolve_agent_and_token's Optional[Tuple[str, str]] contract. Three MCP surfaces updated in sync (Invariant #13).

Non-blocking items:

1. learnings.md has an accidental duplicate line — the ent#183 tar Lesson is repeated verbatim (line ~132). Trivial; fix on the branch or in a follow-up.
2. Manual status bump needed: all three issues are cross-tracker (ent#222/#223/#224), so no auto-promotion — set status-in-dev on them after merge, manual close at release.
3. Known residual to name somewhere visible: with a dedicated bot AND the workspace bot in one channel, an un-@mentioned thread-reply message event is delivered to both apps and can produce two turns (the design's Phase-3 "shared-channel disambiguation tests" are deferred, but Phase 2 + wiring ship now, so the window is live for entitled installs).
4. Behavioral caveat worth documenting: the completion report requires the executing agent (B) to be bound to the originating channel with consent — a non-channel-bound worker's completion still won't report. Defensible consent posture, but it means the classic router-agent topology stays silent.
5. architecture.md's adapters catalog doesn't list the new adapters/per_agent_bot.py seam (one line), and the requirements area file wasn't touched for the new OSS-visible capability. Docs-only; fine as a follow-up given the enterprise-docs privacy split.

Sequencing: either order vs ent#232 is safe (both sides guard); the submodule pointer bump at the next release must carry both.

[dolho]

Enterprise submodule pointer bumped — ent#222 is now active in this PR

trinity-enterprise#232 merged to enterprise main (6f073c7), so this PR now carries the pointer bump that activates it:

-Subproject commit 594b0afd…
+Subproject commit 6f073c73…

Why it had to be in this PR: without it the OSS half here ships inert — adapters/per_agent_bot.py has no resolver registered, so per-agent routing silently doesn't exist and ent#222 would look like it just didn't work (the ent#185 class). This was the third step I flagged in the merge plan; folding it in removes the chance of it being missed.

Verified 6f073c73 carries:

• backend/slack_per_agent_bots/ — module, transport manager, cascade wiring
• backend/migrations/versions/0012_slack_agent_bindings.py — the enterprise Alembic revision

It is a direct child of 594b0afd (which dev already had), so nothing else rides along.

Merge plan is now two steps, not three: merge this PR → then #1752 (its Alembic 0031 chains off 0030 here).