fix(telegram): gate the spawn-approval prompt on the channels ceiling

[chenmingwei23]

What is the problem

Channel-side spawn-approval delivery posts the prompt without consulting the
operator's channels governance ceiling. TelegramDispatcher.deliver_spawn_approval
is the registered hook on main, so a host profile that denies the telegram
channel after the transport connected does not stop a spawn-approval prompt,
carrying an agent-authored task preview, from being posted into that conversation.

The startup gate does not cover this: it only stops a transport from
connecting. A profile that denies the channel afterwards leaves the transport up
with its rosters intact, which is exactly the gap channel_inbound_permitted
exists to close on the inbound message path and the callback press path.

Why it matters to the user

The prompt cannot be answered once it is posted. A channel's callback path drops
an inbound press on a governance-denied channel, and only an explicit reject
(a:<rid>:<nonce>:0) is exempt from that drop. So an Approve press on a
denied channel resolves nothing, the deny-by-default wait runs to
APPROVAL_TIMEOUT_S, and the delivery answers False.

That False is not a fall-through. The delivery seam reads it as the user's real
decision, so the host spawn gate refuses the spawn on it instead of re-offering
the prompt on the still-permitted Slack DM or dashboard surface. The user sees a
spawn denied by a prompt they were never able to see, after a full timeout. A
deny is meant to withhold the channel, not to cast a vote in the operator's
name.

How the fix solves it, symptom to root cause

The symptom is "a denied channel denies the spawn". Walking it back: the refusal
came from an elapsed wait; the wait elapsed because the answering press was
dropped; the press was dropped because the channel is denied; and the prompt was
posted anyway because the delivery path never asked. The root cause is a missing
authority, not a wrong one -- the rosters and transport.may_send_to answer "may
this destination receive a send", which is a different question from "does the
operator permit this channel at all".

messaging/spawn_approval_delivery.deliver_spawn_approval now consults
channel_inbound_permitted(channel) and returns None on a deny.

At the seam, once, not inside each hook. Every hook this seam can invoke
posts an interactive prompt whose answering press arrives inbound on the same
channel, so the ceiling is a property of the delivery contract rather than of any
one channel. The seam already resolves the channel to find the hook, so the check
costs nothing extra there, and it gates every present and future hook -- including
a hook written by someone who never reads this PR. A copy inside each dispatcher
would be the same authority duplicated per implementation, which is the shape
this change exists to remove, not to repeat.

Two ordering properties follow structurally rather than by comment:

• No hook runs under a deny, so nothing is armed and nothing is posted. There
  is no window in which a stale press could resolve a later request, and each
  hook's own roster check keeps its suspension-free window before its send
  untouched.
• Hook resolution runs first. A key in a non-channel namespace
  (dashboard:, cron:) or a unified DM bucket names no governed channel, so
  it falls through without asking the profile store about a channel type that
  does not exist, and without emitting a governance decision for a non-channel.

Nothing else moves. No channel gains permission it did not have, no deny is
skipped, no timeout is shortened, and an unreachable delivery is never reported
as an approval: a denied channel answers None (fall through), never True and
never False. A permitted channel behaves exactly as before, and a channel that
registers no hook is untouched.

What tests we did

test/test_spawn_approval_channel_governance_13491.py, 5 tests, run by path with
-n0. Telegram client I/O is faked and the ceiling predicate is substituted, so
nothing touches the network or a real profile store. Four of the five drive the
seam through the real registered Telegram hook rather than a stand-in, so the
pin covers the path the host gate actually takes.

Run alongside the two sibling spawn-approval suites and the Telegram dispatcher
suite to show the permitted path is unchanged: 392 passed for the four files
together.

Red on base first, with the real symptom: with the check absent, four of the five
tests fail, and the one driving the real hook fails assert False is None -- the
delivery returned a refusal produced by the elapsed wait after the prompt was
posted under a deny.

Each property was then verified by an independent mutation:

Mutation	Result
Drop the ceiling check	4 failed, 1 passed -- every deny arm red; the permitted arm already green
Move the check ahead of hook resolution	only the no-hook pin fails: ['', 'unified', 'telegram'] == [] -- three governance questions asked about keys that name no governed channel
Make the check always refuse	only the permitted arm fails -- the guard cannot be satisfied by refusing everything

The third mutation is why the permitted arm is asserted as its own test: a check
that denied unconditionally would pass every other test in the file while making
in-channel spawn approval impossible.

Other suggestions

Design Review (advisory CONCERNS) is what moved the check. The first revision
placed it inside TelegramDispatcher.deliver_spawn_approval, as the issue's own
suggested fix proposed. The review's objection was that a per-dispatcher copy
re-creates the defect class the change fixes, policed only by a lint rule this
PR's own harvest section had proposed. That is correct, and the hoist is strictly
better: messaging/spawn_approval_delivery.py already holds the channel token the
ceiling needs, messaging/identity.py is in the same package so no layering or
import cycle is involved, and the ordering argument becomes structural. The
announced Discord port of this seam no longer needs a copy of its own.

The prose sweep covers two specs. messaging.md documents the check in the
seam's own bullet, next to the None contract it extends. governance.md
enumerated this predicate's call sites as each dispatcher's handle_message; it
now also names this one outbound send that consults the inbound ceiling, why an
outbound prompt depends on an inbound permission, and why the check sits at the
seam.

Not addressed, and deliberately out of scope: the per-agent auto_approve_spawn
allowlist, which remains a separate design.

Pattern harvest

Rule candidate: review-prompt

Pattern: when a cross-channel seam needs an authority, enforce it at the seam,
not once per implementation behind it. The defect class this PR started from is
an authority consulted on the inbound paths of a module but skipped on the
sibling outbound path whose answer depends on that same inbound permission -- the
press that resolves the prompt is inbound, so a channel denied for inbound
traffic cannot answer a prompt sent outbound to it, and the resulting elapsed
wait is read as a decision. A semgrep rule for the narrow shape (a method that
arms an approval nonce must consult the ceiling first) was the first candidate
and is deliberately withdrawn: the check now lives at the single routing layer
every hook passes through, so the recurrence the rule would have policed is
retired by construction. What generalizes is the placement question, which a
reviewer can ask of any seam and a pattern matcher cannot.

Closes #13491

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of efcf30dace6f883fb92b5c16921215f957cd2b7e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: PASS

Verify the red-on-base claim (4 of 5 tests fail without the check) — this run has no shell, and that run is the fix's checkable provenance beyond #13491.

Not justified as shipped

• Item 3 — undeclared: the new logger.info skip line in deliver_spawn_approval is never mentioned by the description; harm-free rider, nothing to remove.

What this change ships

Inventory (5 items) — 4 justified

Intent: FIX — a spawn started from a governance-denied channel should stay answerable on Slack/dashboard instead of being auto-refused by the timeout of a prompt nobody could answer (#13491). The claimed mechanism verifies on base: the callback path drops a non-reject press on a denied channel (src/kiro_crew/telegram/transport_dispatch.py:2941, Discord twin at src/kiro_crew/discord/transport_dispatch.py:1680), the seam has one caller (src/kiro_crew/slack/gateway.py:10381) and one registered hook (src/kiro_crew/telegram/gateway.py:122); Slack's own fallback resolves a denied approve-press promptly (src/kiro_crew/slack/interactions.py:989), so no unfixed sibling of the elapsed-wait shape.

1. A spawn from a denied channel now falls through to Slack/dashboard instead of a timed-out refusal — justified
2. No spawn prompt (carrying the agent-authored task preview) is posted into a denied channel anymore — justified
3. A new INFO log records the skip on a denied channel — undeclared: never mentioned; harm-free rider
4. governance.md, messaging.md and the Telegram hook docstring now document the seam-level check — justified
5. Five new tests pin the deny, ordering and permitted-path behavior — justified

[FIRST-PRINCIPLES-REVIEWED] efcf30d

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of efcf30dace6f883fb92b5c16921215f957cd2b7e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Verify the residual window is acceptable: a deny landing after the prompt posts still drops the press, times out, and returns False as a decision.

Suggestions

• The mid-wait race could be closed within this seam's contract by re-consulting the ceiling on timeout and answering None instead of False when the channel became denied while the prompt was pending — a candidate follow-up, not a change needed here.

[DESIGN-REVIEWED] efcf30d

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed efcf30dace6f883fb92b5c16921215f957cd2b7e — this comment is updated in place on each push.

Review details

Nothing blocks: both survivors are advisory — an audit row named for the wrong caller, and an owning-spec enumeration the change made incomplete.

FINDING — src/kiro_crew/messaging/spawn_approval_delivery.py:159 — await channel_inbound_permitted(channel) reuses the per-message inbound predicate verbatim, so a host-initiated OUTBOUND delivery decision lands in SEL as tool_name=f"inbound:{channel_type}" (messaging/identity.py:190-214) with scope="channels" and outcome="allowed"/"denied"; with a governed channels layer, every channel-parented spawn appends a durable HMAC-chained row asserting an inbound-message decision for a message that never arrived, against the convention the sibling channels chokepoint states explicitly (mcp_core.py:1212 "the persisted audit trail must name the real caller") → Fix: add a tool_name/source parameter to channel_inbound_permitted / _channel_inbound_permitted_sync and pass a caller-accurate name (e.g. spawn_approval:<channel>) from this seam — the remedy edits messaging/identity.py, which this PR does not touch, so it cannot gate the merge.

FINDING — docs/system-specs/modules/subagent.md:406 — the diff adds a third None cause (the channels ceiling deny) and updates messaging.md, governance.md and the function docstring, but the owning spec still enumerates a closed set — `None` (no hook registered for that channel, or the hook could not surface the prompt) at :406-407 and again under Delivery order at :424 — as does the module docstring's bullet list (spawn_approval_delivery.py:18-24, "no hook registered for the session's channel is the same fall-through"), so the spec AGENTS.md requires updating in the same commit now describes a set the code no longer matches → Fix: add the ceiling deny as a third cause in both subagent.md passages and in the module docstring's bullet list.

[OPUS-REVIEWED] efcf30d

Verdict parsed from the review's SHA-scoped output markers for commit efcf30dace6f883fb92b5c16921215f957cd2b7e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable efcf30dace6f883fb92b5c16921215f957cd2b7e: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of efcf30dace6f883fb92b5c16921215f957cd2b7e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] efcf30d

False positive or not applicable? A repository writer can comment:
/ai-review override gpt efcf30dace6f883fb92b5c16921215f957cd2b7e: <one-sentence reason>

[chenmingwei23]

Disposition of the advisory review items on efcf30dace6f883fb92b5c16921215f957cd2b7e

Both whole-review verdicts on this head are PASS (First Principles, Design) and
both AI reviews report no blocking findings (GPT 5.6, Opus 5). Each review still
carries one advisory item; this comment records the disposition of each so the
record is accurate without a description edit re-rolling the edited-triggered
review lane on a settled board.

1. First Principles, item 3 -- the new INFO log is undeclared

Correct, and the classification is accepted: it is a rider, and it stays.

The line is the seam's only observable record of a skip. A denied channel now
answers None, which is the same value the seam returns when no hook is
registered and when a hook could not surface the prompt, so without the log an
operator reading the host log cannot tell "the channel is denied by policy" from
"no channel owns this session". It names the channel and the request id at INFO,
posts nothing, and changes no return value.

Declaring it here rather than in the description: the description is consumed by
the GPT lane, which takes edited, so rewording it re-runs that reviewer on a
head whose board is already green.

2. Design -- the mid-wait residual window

Verified as real, and deferred to its own issue: #13559.

A deny that lands after the prompt has posted is not seen by the one-time check,
so the press is dropped, the deny-by-default wait elapses, and the hook answers
False, which the host gate returns as a decision. That is the same shape as
#13491 in a narrower window.

Confirmed by reading the source rather than inferred: the Telegram hook ends in
return bool(await decider(event)) with the same deny-by-default decider a tool
prompt uses, and the host gate returns any non-None answer directly. The
suggested remedy, re-consulting the ceiling on timeout and answering None
instead of False, is recorded on #13559 together with the scope note that an
elapsed wait on a channel that is still permitted must keep answering
False, since that case is a real operator deny-by-default.

Out of scope here: closing it needs a second read of the ceiling in the timeout
path, which is a behaviour change to the permitted path this PR deliberately
leaves identical.

3. First Principles -- "verify the red-on-base claim, this run has no shell"

Measured, not inherited. The five pins were run by path with -n0 on this exact
head, then again with the ceiling check disabled and nothing else changed:

Tree	Result
This head, unmodified	5 passed
Ceiling check disabled	4 failed, 1 passed
Check restored	5 passed

The arm that drives the real registered Telegram hook fails
assert False is None under the disabled check: the delivery returned a refusal
produced by the elapsed wait after the prompt was posted under a deny. That is
the symptom in #13491, reproduced. The one test that stays green is the permitted
arm, which is expected -- it asserts the unchanged path.

No code, description, or head change accompanies this comment.

[bolichen97]

Approving. All four lanes are green on this head and the author's disposition answers each advisory item: the inbound:<channel> audit label for an outbound decision, the incomplete None-cause enumeration in subagent.md, and the mid-wait deny race (deferred to #13559).

The fix itself is the right shape — checking the operator's channels ceiling at the single delivery entry point and returning None so approval falls back to Slack/dashboard, rather than letting a prompt nobody can answer time out and be read as a denial.