Benjamin Kurrek 90f5532fcb feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted (#7157) ...

* feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted

Re-landed PR #7157 on current main (aa8b748c3 → 967f33aa4, across the WS2
composition inversion, the WS6 crate renames, and the WS7 family moves), with
all 44 review comments dispositioned.

Two-lane delivery model: a run's final reply always lands in its own
conversation (lane 1); reaching any other surface is the model's explicit
`builtin.outbound_deliver` call (lane 2 — bot identity, one catalog target
per call, synchronous through the DeliveryCoordinator, provider-issued
message refs as evidence). Background-run notices fan out to a user-configured
notification-channel set (new record field + read-side legacy migration,
`builtin.notification_channels_set`, first-approve-wins, WebUI multi-select).
The stored delivery heuristics are deleted (route_current, builtin:web_app,
outbound_delivery_target_set, per-trigger delivery_target_id + precedence
chains + four-slot preference fallback), with an idempotent boot migration of
stored trigger targets into explicit prompt steps and the retired vocabulary
pinned in reborn_retired_taxonomy.rs.

Port notes (old → new homes): ironclaw_reborn_composition→app/ironclaw_composition,
ironclaw_product→product/ironclaw_assistant, first_party_extensions→extensions/packages,
run-profile vocabulary→ironclaw_loop_contracts, PreferenceTargetCodec→
ironclaw_extension_contracts, wire DTOs→ironclaw_product_contracts::product_wire.
The model-delivery implementation moved extension_host→assistant
(CoordinatedModelChannelDelivery) — the WS2 port inversion forbids extension_host
naming product types; the deferred-slot registration and post-coordinator bind now
live in composition's production assembly, mirroring TriggeredRunDeliveryDriver.

Re-folded 2026-08-05 onto b2023bc8f (#7258): channel-adapter vocabulary
re-imported from ironclaw_extension_contracts, product-adapter/inbound
vocabulary from host_api/product_contracts, module-charter map's outbound
row renamed to the two-lane vocabulary. Post-branch CI gates adapted in
the same change: skills/ classified in the PR test planner (test-first,
sabotage-verified), panic baseline ratcheted down, nested test fixtures
renamed to the scanner-sanctioned support_tests.rs shape, composition's
inline trigger-migration tests split to tests.rs (mass budget green with
no ceiling raise), extension_contracts size ceiling 7727 -> 7748 (+21:
the ActivePreferenceTargetCodecs port), loop_contracts ceiling
re-captured down 14479 -> 13850 after the delivery-vocabulary deletion.

Third fold 2026-08-05 onto b72d7da66 (#6831, standardized messaging
framework): the two-lane guidance moved into the canonical messaging core
prompt (host_api prompts/messaging/send_message.core.md), now naming
builtin__outbound_deliver with the arrive-twice and trigger caveats for
every messaging extension; slack vendor addendum/manifest taken as #6831
shipped them; ceiling-table union (host_api 18570 beside this PR's two
re-captures); retired slack schema embed and deleted preferences
capability stay deleted; golden context-surfacing snapshot regenerated
(one surface-hash line).

Fourth fold 2026-08-06 onto c69ed2d70 (#7263 program-closure batch +
sibling fixes): ceiling-table union (product_contracts 15685 from #7230
beside this PR's re-captures) and main's tracing-target syntax sweep
(target = -> target:, gate-enforced) applied over this PR's kept lines;
deleted delivery-heuristic code stays deleted.

Fifth fold 2026-08-06 onto 0c297cb24 (#7264 guidance-layer sweep):
zero conflicts; guidance/doc-pointer changes auto-merged over this delta.

Routing-UX slice 2026-08-06 (product thread + follow-ups): result routing
is prompt-owned with a pinned source-surface default (bare "send me" =
the surface you asked from; web app = no delivery step; explicit
destinations override, one delivery step each) — iterated against live
recordings until a real model followed it, with two live-recorded QA
fixtures (bare-webui, multi-channel) plus contracts and replays. The
automations-page panel is retained as the notification-channel selector
(notices only); the conversational notification_channels_set tool writes
the same validated set.

Delivery-evidence fix (theredspoon's flag; #7029 fixes the same swallow
on main): mark_terminal reports whether the durable write committed and
a confirmed send whose Delivered row failed to commit returns
DeliveredUnconfirmed (refs retained, durably_recorded: false), never a
fabricated Delivered — regression-tested and sabotage-verified. Plus a
CodeRabbit triage batch: correctable coordinator errors stay
model-visible, omitted target_ids no longer clears the set, the success
schema requires evidence, the composition outbound facade is dissolved,
and guidance/contract docs are aligned.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: harden channel delivery routines and replay

* fix: preserve automation loading and identity freshness

* fix: close channel delivery review defects

* fix: skip paused routine catch-up slots

* ci: record channel delivery composition budget

* test: align composition baseline with channel delivery

* fix(auth): survive interrupted OAuth callbacks

* fix(auth): keep callback coordination panic-free

* fix(ci): reconcile channel delivery merge seams

* fix(ci): recapture merged contracts ceiling

* fix(delivery): close review findings across delivery, migration, and guidance

Fixes the findings from the multi-agent review of this PR. Every behavioral
fix ships with a regression test that fails before it.

CI (red on this head)
- `standalone_yolo_notification_channels_set_bypasses_approval_gate`
  expected the shared "invalid outbound delivery request" summary for a
  `builtin__notification_channels_set` call. Production deliberately
  specializes that message per operation and pins it with
  `notification_channel_failure_names_the_operation_the_model_can_correct`;
  the assertion was the stale side.

Delivery evidence (kernel + assistant + outbound)
- `AlreadyDelivered` replays reported `delivered: false` "unverified"
  because the ledger row retains no provider refs, inviting the duplicate
  resend the at-most-once claim exists to prevent. Evidence gained
  `already_delivered`; a replay now reads as delivered with an honest
  "not resent" summary. The classification suite had no `AlreadyDelivered`
  case at all, which is why this shipped.
- `DeliveredUnconfirmed` is the one non-`Delivered` outcome that actually
  sent something, but `delivered_messages_from_outcome` dropped its refs,
  so gate reply-routes went unrecorded and a live OAuth prompt could never
  be retracted on that path.
- `content` is now rejected when empty: the input schema advertised
  minLength 1 and nothing enforced it, so empty content reached the channel
  as an empty part and returned an opaque provider error.

Background-run notifier (assistant)
- One run legitimately emits several `RunBlocked` notices (re-auth
  stand-in, unserviceable-auth cancellation, run failure), but all three
  derived the same projection ref, and the delivery id hashes it. The
  second notice to a target came back `AlreadyDelivered`, was treated as
  success, and was never sent — a user could be told a routine needed
  re-authorization and never told it then failed. Notices carry a
  discriminator; once-per-run kinds keep their historical id shape, so
  existing delivery identities are unchanged.
- When every catalog lookup failed, the empty result was recorded as
  `NoDefaultConfigured`, reporting a backend outage as the benign "user
  configured nothing" state. It now records `Failed`.

Boot migration (composition)
- The retired `builtin:web_app` target meant "no external delivery". It was
  being rewritten into a delivery step to an id nothing can resolve,
  inverting the stored intent on every later fire. It now clears without
  adding a step.
- One unmigratable row aborted the entire composition boot, with the error
  telling the operator to shorten a prompt through the UI that no longer
  starts. It now pauses its own routine — a paused trigger cannot fire, so
  "never fire unrouted" still holds per record — and boot continues. Only a
  systemic store failure stays boot-fatal. A row deleted during the CAS
  retry ends that record instead of failing boot.
- The CAS retry loop, its bounded exhaustion, and the vanished-row arm had
  no caller-level coverage; adds a delegating repository double that forces
  CAS misses. The prior fail-closed test is rewritten to pin the invariant
  it documented (route survives, record not half-migrated) under the new
  per-record mechanism.

Model-visible messages (composition)
- The targets-list denial said "not permitted to change the outbound
  delivery target" for a read-only call, and the lease denial named the
  retired delivery-target concept on the notification-channel path that is
  its only production caller. Both are now operation-specific and pinned.

WebUI (frontend)
- `setNotificationChannels()` with no argument posted `target_ids: []`,
  turning an omitted argument into a destructive clear-all and defeating
  the backend contract that deliberately rejects an omitted field.
- The notification-channels panel stayed editable after a failed read, so
  toggling one row full-replaced the stored set from an empty baseline and
  silently dropped every channel the user never saw. Editing is now locked
  on a failed read, with a rendered explanation.
- Adds the missing `tools.description.builtin.notification_channels_set`
  key to all 11 locales, plus save-failure coverage for the hook (which was
  correct, but untested) and locale-parity tests.

Guidance
- The new `.claude/rules/tools.md` was ported from a pre-restructure branch:
  it named `ironclaw_dispatcher` (deleted) and `ironclaw_extensions` (never
  existed), and its review command grepped three paths removed by WS6/WS7.
  Its `paths:` frontmatter also never matched the product/composition
  callers its rules govern, so the rule never loaded for them.
- `ironclaw_loop_contracts` now records both embedded prompt assets; this
  PR added a second one while the crate's Known-debt entry still said one.
- Bumps `skills/delegation` (rewritten guidance, unlike its two siblings in
  this PR which both bumped) and fixes a pre-rename path in the
  extension-runtime checklist.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(delivery): unify the DM-rule enforcement point and re-ratchet composition

CI (composition mass budget, red on the previous head): the per-record
migration quarantine pushed composition 6 LOC over its absolute ceiling.
Resolved by the reduction the budget file itself blesses rather than a
raise — `runtime/approval.rs`'s 417-line inline `#[cfg(test)]` module split
verbatim into `runtime/approval/tests.rs` (the gate excludes test-only
files but counts inline test modules). Composition is now 40,432 LOC,
smaller than before these fixes, and `loc_ceiling`/`loc_observed` plus the
arch-test record are re-captured together at the measured value per the
gate's one-directional ratchet rule.

The codec-scan that decodes a binding and enforces "an OAuth authorization
URL only ever lands in a personal DM" existed twice — once in
`TriggeredReplyTargetAuthority`, once as `CodecChannelTargetResolver` —
with both copies commented as "the single enforcement point". They are now
one implementation, shared by the notifier and `builtin.outbound_deliver`,
with a context label so each path keeps its own diagnostic.

That rule turned out to be UNGUARDED: sabotaging it (`if false && ...`)
failed no test in the crate. The vendor codecs pin the predicate in
isolation and the coordinator test pins rejection handling with a double
that decides the verdict itself, so nothing covered the wiring that joins
them. Adds a contract test driving the real resolver through
`DeliveryCoordinator::deliver` for both verdicts, asserting a non-DM target
never reaches the vendor adapter. Sabotage-verified: the test fails with
the rule disabled and passes with it restored.

Smaller findings: the notification-channel schema cap now derives from
`ironclaw_outbound::NOTIFICATION_TARGETS_CAP` instead of hand-mirroring
`8`; `triggered_run_delivery`'s module and trait docs described the retired
result-push model this PR deletes; the two new notification strings used a
different brand spelling and dash style from the nine siblings in their own
module; and several new comments navigated by pre-rename paths
(`ironclaw_product::`, `local_dev::`, `crates/ironclaw_webui/`) plus a
citation of a test symbol that does not exist.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(outbound): scope the delivery catalog to the authenticated actor

`builtin.outbound_deliver` resolved its destination catalog under
`ResourceScope.user_id` while performing the send as
`authenticated_actor_user_id`. Those are the same user on a personal thread
and on an automation fire, but they diverge on a shared-route channel
conversation: the scope user is the route's SUBJECT
(`TurnScope::explicit_owner_user_id`) and the actor is whoever sent the
message. Any participant of such a channel could therefore name the
subject's target ids and push bot-identity content into the subject's own
destinations — their personal DM included — from a conversation the subject
may never read.

The catalog now follows the actor, so a caller stays inside their own
connected surfaces on every path and an unfamiliar target simply does not
resolve. Behavior is unchanged wherever owner and actor already agree,
which is every non-shared-route path.

Regression test drives the divergent case through the port (participant
denied with `TargetUnavailable`, nothing reaching a vendor adapter) plus a
control proving the owner's own delivery still works. Sabotage-verified:
restoring owner-scoping fails it.

NOT changed here, and flagged for a product decision: the sibling
`builtin.outbound_delivery_targets_list` and
`builtin.notification_channels_set` derive their caller from the same
owner-preferring `effective_user_id`, so on a shared route a participant
can still enumerate — and, with the approval gate auto-approved, rewrite —
the subject's notification channels. That helper also scopes approval
gates and capability leases, so flipping its precedence risks breaking
approval raise/resume matching in a path no test covers; it needs its own
change with that coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(delivery): stop rewriting the DM-target row on every message

The post-admission backfill calls `FilesystemChannelDmTargetStore::upsert`
for every admitted inbound direct message, and the store unconditionally
wrote a fresh row. After the first message the stored record is already
correct, so the steady state was one durable backend write per DM message,
forever, whose only effect was a new `updated_at` — and each message's
reply-delivery observation was serialized behind it. An unchanged record
now short-circuits; the existing row is loaded here anyway to preserve
`created_at`, so the comparison costs nothing.

Also adds the regression test the `NoDefaultConfigured` -> `Failed`
classification fix landed without: the notifier's `SkipEntry` lookup lane
had no coverage at all (no test ever made a catalog lookup error), so
neither the skip nor the all-failed arm was exercised. The triggered
harness gains an injectable catalog provider for it. Sabotage-verified.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(loop): bound the connected-channels line so the runtime slice fits

Confirmed live, not theoretical: a worst-case runtime context renders 4,391
bytes against the 4,096-byte `PromptTextSurface::SafeSummary` cap that
`instruction_bundle::push_runtime_context` validates the whole slice on —
and exceeding it is a run-ending error on EVERY prompt build for that user,
not a one-off.

This PR's fixed ~1.1 KiB delivery-guidance block is what pushes a
previously-fitting context over. The individual parts are each bounded
(location 200 chars at its producer, locale 35, per-label safe-text
validation), but nothing bounded their SUM, and the connected-channels line
is the one part that grows without limit: up to 20 entries whose names and
presentation hints are only individually capped.

That line now renders as many channels as fit a 1 KiB budget and folds the
rest into the "+N more" counter it already carried, so the fixed guidance
can never be squeezed out by variable content. The worst-case test that
found this stays as the pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(outbound): resolve outbound capabilities as the acting user

`builtin.outbound_delivery_targets_list` and
`builtin.notification_channels_set` derived their caller from
`effective_user_id`, which prefers the thread owner over the actor. Those
agree on a direct message and on an automation fire, but diverge on a
shared-route channel conversation, where the owner is the route's
configured subject — the deployment operator by default
(`channel_workflow.rs`) — and the actor is whoever posted.

So any participant of a shared channel could enumerate the operator's
connected destinations and rewrite the operator's notification-channel set,
which is where approval prompts, re-auth prompts and failure notices are
delivered. The caller now follows the acting user, matching the fix already
applied to `builtin.outbound_deliver`.

This deliberately REVERSES a previously pinned preference. Two tests
asserted the owner won when the two differ; that pin predates shared-route
subjects defaulting to the operator, and it contradicts the rule that a run
acts as whoever invoked it. Both are updated to pin the actor, with the
reversal recorded at each site rather than silently relaxed, and the
notification-channel write is now asserted to land under the acting user
with the thread owner's own set left untouched.

INTERIM, by design: `resource_scope_for_run` and `settings_scope_for_run`
still follow the owner, because they scope the approval-gate raise and the
capability lease and those must stay matched between raise and resume.
Unifying them belongs with the follow-up that removes shared-route subject
binding entirely so a shared channel runs wholly as its invoker; that needs
approval raise/resume coverage which does not exist yet. A new test pins
the split so the interim state is explicit rather than accidental.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): keep loop_contracts under its size ceiling and ratchet down

The runtime-context byte-budget fix and its worst-case pin pushed
`ironclaw_loop_contracts` to 14,032 production lines against a 13,949
ceiling. Resolved by the reduction the gate prefers over a raise:
`runtime_context.rs`'s 919-line inline `#[cfg(test)]` module split verbatim
into a `runtime_context/tests.rs` sibling, which `production_rust_files`
excludes (an inline test module inside a production file is counted; a
test-only file is not).

The crate now measures 13,115 — 834 lines below the previous ceiling and
smaller than before this review round — so the ceiling is re-captured
downward at the measured value rather than raised, per the gate's
one-directional ratchet. Count read from the gate's own failure message,
not by eye.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): chart the notification-channel handlers in the WebUI charter map

`handlers_module_charter` failed: `get_notification_channels` and
`set_notification_channels` — handlers this PR adds — had no sub-owner row
in `CONTRACT.md`'s enforced charter map, and the row they belong to still
named `get_outbound_preferences`, `set_outbound_preferences` and
`outbound_preferences_activity_id`, all deleted by this PR.

Both halves are fixed together because the gate checks both in one test:
unclaimed items first, then entries naming items that no longer exist. Only
the first had fired, so the stale half was still latent behind it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(ci): re-capture the loop_contracts ceiling after main's merge

Main's #7361/#7363 landed 66 lines in
`ironclaw_loop_contracts/src/instruction_bundle.rs`, which this branch picked
up when folding onto main. That put the crate at 13,181 against the 13,115
ceiling re-captured earlier in this PR.

Not growth from this PR. The gate's upward check is a hard `lines > ceiling`
with no headroom — `TOLERANCE` (400) governs only the downward
ratchet-nudge — so a ceiling captured at the exact observed value reddens
every open branch the moment anyone adds a line to that crate, including
from main. Re-captured at the measured value; count read from the gate's own
failure message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

2026-08-07 21:39:07 +00:00

..

SKILL.md

feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted (#7157)

2026-08-07 21:39:07 +00:00