feat(inspector): add bounded diagnostic session storage - #7230
Merged
Merged
Conversation
italic-jinxin
added
size: XL
|
🚅 Deployed to the ironclaw-pr-7230 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 5, 2026 11:05
Destroyed
github-actions
Bot
added
risk: low
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds public inspector diagnostic contracts and a bounded in-memory diagnostic store. It supports scoped records, bounded payloads, snapshots, ordered retained updates, deterministic eviction, live subscriptions, cursor rebasing, and explicit state errors. ChangesDiagnostic inspection
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant InspectorClient
participant InMemoryDiagnosticStore
participant DiagnosticSnapshot
participant DiagnosticSubscription
InspectorClient->>InMemoryDiagnosticStore: record scoped diagnostic
InMemoryDiagnosticStore->>DiagnosticSnapshot: update bounded state
InMemoryDiagnosticStore->>DiagnosticSubscription: publish ordered update
DiagnosticSubscription-->>InspectorClient: deliver retained live update
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🔎 Review · PR #7230
2 actionable findings →The new diagnostic contracts consistently bound and redact retained content, but the store has two replay/live-stream correctness issues that should be fixed before consumers are wired. Automatic · PR opened · attempt 1 of 3 · completed in 1m 33s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_product_contracts/src/inspector.rs`:
- Around line 600-604: Change the component_count field in the PromptUpdated
wire-contract variant from usize to u32. Preserve the existing
MAX_PROMPT_COMPONENTS limit and update any construction or serialization paths
for PromptUpdated so the value is represented consistently as u32.
- Around line 677-709: Add a focused test alongside
prompt_constructor_applies_component_and_skill_caps named
prompt_constructor_applies_the_total_byte_budget, using a small number of
components with PROMPT_COMPONENT_CONTENT_MAX_BYTES-sized contents so the total
reaches and exceeds PROMPT_COMPONENT_TOTAL_MAX_BYTES. Construct the prompt with
no skills, then assert retained content bytes stay within the total budget,
fewer components are retained than provided, and any_content_truncated() is
true, covering partial content rebound, the zero-remaining break, and truncation
state.
- Line 664: Remove the redundant UTF-8 assertion in the inspector test because
bounded.content() already returns valid UTF-8 by type. If validation is needed,
replace it with an assertion that the retained text ends at the expected
character count.
- Around line 107-120: Make the diagnostic cursor contract round-trip capable by
deriving Deserialize alongside Serialize on DiagnosticCursor,
DiagnosticStreamId, and DiagnosticSequence. Add a test covering serialization
followed by deserialization and equality for DiagnosticCursor, including its
stream ID and sequence values; do not add unrelated changes.
- Around line 313-324: Update the truncation handling in inspector.rs so
`components_truncated` only reflects whether the component list was shortened by
`take(MAX_PROMPT_COMPONENTS)` or by `remaining == 0`, and stop OR-ing in each
component’s `content.truncated()` in that loop. Keep per-component content
truncation detection separate via `any_content_truncated`, and preserve the
existing `PromptComponentDiagnostic::new` and `bounded_components` flow so
retained components still get rebound correctly.
- Around line 471-475: Update the test covering the result/output_bytes handling
near the existing truncation test to pass distinct values for the result’s
original byte length and caller-supplied output_bytes, then assert that the
result-derived length takes precedence when result is Some. Preserve coverage
for the fallback behavior when result is None if already present.
In `@crates/ironclaw_product/src/inspector_store.rs`:
- Around line 436-441: Update the diagnostic subscription flow around subscribe
and recv so the process-wide self.updates broadcast capacity is sized for
expected concurrent scopes, not merely DEFAULT_MAX_RETAINED_UPDATES_PER_RUN.
Document that a SubscriberLagged result terminates the subscription and requires
callers to re-subscribe and rebase through updates_after, and add or update the
relevant API documentation to make this recovery contract explicit.
- Around line 480-490: Add a test near the diagnostic store limits tests that
calls `DiagnosticStoreLimits::default().validate()` and asserts success,
ensuring the defaults remain compatible with `InMemoryDiagnosticStore::new()`
and cannot silently diverge from its validation path.
- Around line 325-364: Refactor the existing record gateway/closure used by
record so it accepts an optional session-state mutation or equivalent callback,
allowing record_stats to update session.stats within the same locked operation.
Route record_stats through that gateway and remove its duplicated lock, run_mut,
sequencing, envelope, retention, and broadcast logic; preserve the existing
stats assignment and single-lock atomicity.
- Around line 806-819: Strengthen
subscription_filters_scope_and_preserves_sequence by retaining the allowed scope
before record_activity consumes it, then assert that the received update’s scope
equals the allowed scope in addition to its sequence. This must verify filtering
through subscription.recv(), not merely rely on identical per-run sequence
values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: df734d2d-82b3-4fb6-815b-c1dfa35c93ff
📒 Files selected for processing (4)
crates/ironclaw_product/src/inspector_store.rscrates/ironclaw_product/src/lib.rscrates/ironclaw_product_contracts/src/inspector.rscrates/ironclaw_product_contracts/src/lib.rs
Contributor
Author
|
@claude review |
This comment was marked as resolved.
This comment was marked as resolved.
29 tasks
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 5, 2026 11:57
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_product/src/inspector_store.rs (1)
543-555: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the lag and close recovery contract on
recv.
SubscriberLaggedandSubscriptionClosedhave different recovery semantics, and neither is documented on this public API. AfterLagged, the underlyingbroadcast::Receiverstays usable and the caller may callrecvagain, but it must rebase throughupdates_afterbecause updates were dropped. AfterClosed, the subscription is terminal and the caller must callsubscribeagain. Callers cannot infer either rule from the type.Add doc comments on
recvandsubscribestating both rules.This repeats an earlier request on the same recovery contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_product/src/inspector_store.rs` around lines 543 - 555, Document the recovery contract on the public DiagnosticSubscription::recv and subscribe APIs: explain that SubscriberLagged leaves the receiver usable but requires rebasing through updates_after before calling recv again, while SubscriptionClosed is terminal and requires creating a new subscription via subscribe. Keep the existing behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_product/src/inspector_store.rs`:
- Around line 250-261: Update live_update_sender and the live-update publication
flow so recording no longer calls prune_inactive_live_updates or scans the
entire live_updates map. Move pruning into subscribe, and when send reports that
no receivers remain, remove the corresponding dead entry opportunistically;
preserve delivery for active subscribers across record_prompt,
record_model_call, record_tool_execution, record_activity, and record_stats.
- Around line 496-506: Keep the state lock held through the live update
broadcast in the recording path: call sender.send with the envelope before
dropping the guard, then release the lock and return the cursor. Add a
concurrent-writer test covering multiple recorders on the same scope and assert
subscribers receive sequence updates in order, while preserving the existing
single-writer subscription test.
- Around line 921-943: Update
unrelated_scope_saturation_does_not_lag_scoped_subscription to create and retain
a live subscription for the other scope before recording its updates, ensuring
that scope’s channel is populated and saturated. Continue recording more than
live_update_capacity updates for other, then verify the allowed subscription
still receives its sequence-1 update.
---
Outside diff comments:
In `@crates/ironclaw_product/src/inspector_store.rs`:
- Around line 543-555: Document the recovery contract on the public
DiagnosticSubscription::recv and subscribe APIs: explain that SubscriberLagged
leaves the receiver usable but requires rebasing through updates_after before
calling recv again, while SubscriptionClosed is terminal and requires creating a
new subscription via subscribe. Keep the existing behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b5ad2abf-65c7-4f74-a042-73c0d71df618
📒 Files selected for processing (2)
crates/ironclaw_product/src/inspector_store.rscrates/ironclaw_product_contracts/src/inspector.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 5, 2026 12:15
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 5, 2026 12:16
Destroyed
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_product/src/lib.rs (1)
122-148: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftRemove the path-preservation re-export shim.
These
pub usestatements retain the oldironclaw_productpaths after the binding contracts moved toironclaw_product_contracts::binding. Update consumers to import the contract owner directly, then remove this re-export.As per coding guidelines, “When relocating a shared module, update every consumer's imports and do not leave a public re-export shim preserving the old path.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_product/src/lib.rs` around lines 122 - 148, Remove the ironclaw_product re-export of ProductConversationRouteKind, ResolveBindingRequest, ResolvedBinding, and route_kind_for_inbound_payload. Update every consumer to import these symbols directly from ironclaw_product_contracts::binding, preserving existing usage and eliminating the old public paths.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_product/src/inspector_store.rs`:
- Around line 250-258: Bound the live_updates scope map independently of
session/run limits by adding a configured maximum and deterministic eviction
when subscribe creates a new scope; evict the defined oldest or
least-recently-used scope, close its sender so retained receivers observe
closure, and preserve existing send_live_update cleanup for disconnected
receivers. Update the relevant configuration and subscription logic around
subscribe and send_live_update, and add a test that retains subscriptions across
more scopes than the limit and verifies eviction and closed-subscription
behavior.
---
Outside diff comments:
In `@crates/ironclaw_product/src/lib.rs`:
- Around line 122-148: Remove the ironclaw_product re-export of
ProductConversationRouteKind, ResolveBindingRequest, ResolvedBinding, and
route_kind_for_inbound_payload. Update every consumer to import these symbols
directly from ironclaw_product_contracts::binding, preserving existing usage and
eliminating the old public paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d2b8e1f8-eb99-42e4-9805-edd42aee79ee
📒 Files selected for processing (3)
crates/ironclaw_product/src/inspector_store.rscrates/ironclaw_product/src/lib.rscrates/ironclaw_product_contracts/src/lib.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 5, 2026 12:58
Destroyed
…diagnostics-store # Conflicts: # crates/ironclaw_assistant/src/inspector_store.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 5, 2026 13:24
Destroyed
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_assistant/src/inspector_store.rs`:
- Around line 441-448: Update the absent-run early return in the
state.run(scope) handling to set rebase_required based on whether after is
present, while keeping the empty updates and no latest_cursor behavior
unchanged. Add a test covering run recording, eviction via max_runs_per_session,
and polling with a cursor, asserting the returned batch requires a rebase.
- Around line 391-403: The record_stats method currently stores stats on the
session while diagnostic sequencing, batches, and subscriptions are run-scoped.
Either document and preserve this as an intentional run-independent API, or move
stats into DiagnosticRunState and propagate an explicit run ID through the stats
update so each run’s DiagnosticSnapshot observes only its own stats.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0732c1ec-c5a9-4b30-bdb3-f2c34118f734
📒 Files selected for processing (4)
crates/ironclaw_assistant/src/inspector_store.rscrates/ironclaw_assistant/src/lib.rscrates/ironclaw_product_contracts/src/inspector.rscrates/ironclaw_product_contracts/src/lib.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-7230
August 6, 2026 09:16
Destroyed
think-in-universe
approved these changes
Aug 6, 2026
BenKurrek
added a commit
that referenced
this pull request
Aug 6, 2026
…ls, delivery heuristics deleted Re-landed PR #7157 on current main (aa8b748 → 967f33a, 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 b2023bc (#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 b72d7da (#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 c69ed2d (#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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7 tasks
BenKurrek
added a commit
that referenced
this pull request
Aug 6, 2026
…ls, delivery heuristics deleted Re-landed PR #7157 on current main (aa8b748 → 967f33a, 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 b2023bc (#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 b72d7da (#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 c69ed2d (#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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek
added a commit
that referenced
this pull request
Aug 6, 2026
…ls, delivery heuristics deleted Re-landed PR #7157 on current main (aa8b748 → 967f33a, 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 b2023bc (#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 b72d7da (#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 c69ed2d (#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 0c297cb (#7264 guidance-layer sweep): zero conflicts; guidance/doc-pointer changes auto-merged over this delta. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek
added a commit
that referenced
this pull request
Aug 6, 2026
…ls, delivery heuristics deleted Re-landed PR #7157 on current main (aa8b748 → 967f33a, 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 b2023bc (#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 b72d7da (#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 c69ed2d (#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 0c297cb (#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>
23 tasks
BenKurrek
added a commit
that referenced
this pull request
Aug 7, 2026
…ls, delivery heuristics deleted Re-landed PR #7157 on current main (aa8b748 → 967f33a, 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 b2023bc (#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 b72d7da (#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 c69ed2d (#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 0c297cb (#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>
This was referenced Aug 7, 2026
personal-upstream-sync Bot
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Aug 7, 2026
…ls, delivery heuristics deleted (nearai#7157) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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 nearai#7361/nearai#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>
This was referenced Aug 7, 2026
pull Bot
pushed a commit
to Stars1233/ironclaw
that referenced
this pull request
Aug 8, 2026
nearai#7157 follow-ups) (nearai#7377) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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(delivery): key observer gate notices by their gate ref One run can park on several approval/auth gates in sequence (the observer's blocked-state loop re-announces whenever the (status, gate) marker changes), but the live observer derived every gate notice's projection id with no discriminator, so all ApprovalNeeded notices in one run collapsed to a single durable delivery identity. The second gate's prompt came back AlreadyDelivered from the coordinator, was treated as success, and was never sent — the user was never told about the gate their run was parked on, and no reply route was recorded for it, so a bare `approve` could not resolve it either. Key the projection id by the notification's gate ref (the mechanism nearai#7157 added for the triggered notifier's RunBlocked notices). A repeat announcement of the SAME gate still dedupes; kinds that carry no gate ref (FinalReplyReady) keep the historical undiscriminated id shape so existing delivery identities are not re-keyed. Regression: observer_delivers_a_prompt_for_each_distinct_approval_gate drives the real DeliveryCoordinator over the real outbound store through two distinct scripted gates and asserts two delivered prompts plus a recorded reply route for each. Sabotage-verified: reverting the discriminator to None fails exactly this test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(composition): pin the notification-channels gate dance when owner ≠ actor The full builtin.notification_channels_set approval dance — raise, replay payload, user approve (store + lease mint from the stored row), approved resume, lease claim, dispatch, lease consume — driven through the real capability port on a run whose thread owner differs from its acting user. Pins two properties ahead of unifying the scope derivation onto the actor: raise and resume must derive the same scope (every store in the dance is scope-keyed, so a half-unified derivation strands the approved capability), and whose identity that scope carries (the thread owner, under the interim split nearai#7157 shipped). The approve step mints the lease from the stored request's own scope, grantee, and fingerprint — the same material the production click-approval resolution uses — never a re-derivation. Capability-host tier rather than tests/integration because the product rule "a run acts as its invoker" makes owner ≠ actor unconstructible through every product front door; the run-context shape remains legal kernel state (runs parked across the deploy boundary carry it). The owner == actor dance stays covered end-to-end at the integration tier (outbound_target.rs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(outbound): scope the whole notification-channels gate dance as the acting user Unify the interim nearai#7157 split: resource_scope_for_run and settings_scope_for_run now derive from the acting user like caller_for_run already did, and effective_user_id (the owner-first ladder) is deleted. The approval-gate raise, the replay payload, the durable gate record, the lease, and the approval-settings read all follow the user who invoked the run — so the invoker sees and approves the gate, and their settings govern it. The raise/resume coverage added one commit earlier ran before and after this change and caught a real half-unification in between: the resume-side replay load lives in ironclaw_loop_host's synthetic-capability wrap (a different crate from the raise-side save in notification_channels_set) and still derived owner-first, stranding an approved resume with "replay payload is unavailable". The acting-identity ladder now has exactly one definition — LoopRunContext::acting_user_id in ironclaw_loop_contracts — and both sides delegate to it, so the hand-synced-copy class is closed rather than re-synced. notification_channels_set's replay/gate-record writes move from the capability_host-wide owner-first helper onto the outbound module's base_resource_scope_for_run so every store in one dance derives one user; the capability_host-wide helper itself is unchanged (thread/durable-result scoping legitimately follows thread ownership, and owner == actor on every binding created under the run-acts-as-invoker rule). Loop-contracts size ceiling: +16 lines for the shared ladder, paid for by splitting host/run_context.rs's 104-line inline #[cfg(test)] module into its run_context/tests.rs sibling; ceiling re-captured DOWN 13_115 -> 13_028 from the gate's own failure message. Runs raised before this change with owner != actor and resumed after it will miss their replay payload once and fail closed; re-requesting approval recovers. Documented in the PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(conversations): key shared-route bindings per (conversation, actor) A run acts as the user who invoked it, so a shared conversation binds one thread per paired actor — each owned by that actor — instead of one conversation-wide thread owned by a configured subject. BindingKey gains a serde-defaulted shared_actor_user_id component (None for Direct routes, whose identity stays the conversation alone); the trusted-owner parameter is deliberately ignored on Shared creates (it remains the trigger lane's way to bind Direct conversations for their creator), and the legacy shared-owner backfill is removed with it. Migration is ignore-but-retain, pinned with a restart-path test: legacy Direct keys deserialize byte-identically (continuity), while legacy conversation-keyed shared rows deserialize to a key no per-actor lookup builds — retained in durable state untouched, and every participant (including the old subject) starts a fresh thread they own. Morphed legacy pins record what became structural: a shared probe/lookup can no longer address (or widen) a Direct binding at all; stored reply targets are isolated per actor; an actor's unpair cannot take the conversation away from other participants' own threads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(product)!: remove shared-route subject binding; scope = invoker Owner ruling: a run acts as the user who invoked it, in a DM and in a shared channel alike, with one thread per (conversation, user). This removes the subject half of shared-route configuration end to end and keeps the admission half, fail-closed: - ironclaw_product_contracts: subject_route becomes shared_admission — the SharedConversationAdmission port answers only "is this shared conversation connected"; ProductConversationRouteKey survives as the admission key. ResolvedBinding loses subject_user_id (retired-field JSON still deserializes; persisted-shape test updated); the actor is the one identity. - ironclaw_assistant: ProductInstallationScope drops the default-subject, static-route, and subject-resolver knobs for one shared_conversation_admission port; resolve/lookup/reset check admission fail-closed (no port wired, or an unlisted conversation, rejects with a not-connected BindingRequired); resolve passes no trusted owner — the conversations domain keys and owns shared bindings by the paired actor. Thread and turn scopes derive their owner from the binding's actor on every route kind. - ironclaw_extension_host: channel_subject_routes.rs becomes channel_shared_admission.rs; ChannelConfigSharedAdmission admits by membership in the operator-saved *_allowed_channels JSON array; the managed derived subject (user:{ext}-channel:{sha16}) is deleted; legacy *_subject_routes values are inert (pinned by test). Shared conversations are no longer offered as per-user notification delivery targets — their ownership came from the retired subject map — and stored channel-target preferences fail closed at resolution; DM targets are unchanged. - slack manifest: slack_shared_subject_user_id and slack_subject_routes are retired with a gravestone comment; slack_allowed_channels is the admission surface (saves to the retired handles already fail closed as unknown fields — the extension-config analog of the config.toml retired-section gravestone). - architecture tests: the INVERTED_PORTS row moves with the port rename. User-visible consequences (also in the PR body): each shared-channel participant now gets their own persistent thread and must be paired; no cross-user shared context; the operator's identity is never a fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(reborn): align guidance, specs, and live-QA scripts with invoker scope Guidance rows and the moved-ports list rename the inverted port (SharedConversationAdmission, ex ProductConversationSubjectRouteResolver); the assistant boundary prose states the new rule (one thread per (conversation, actor), admission is the only shared-conversation configuration, fail-closed on resolve/lookup/reset). The composition CONTRACT.md's never-shipped per-channel subject admin API section is excised with a dated correction; CHECKLIST/PROPOSAL get dated amendments beside the historical text. Operator docs teach slack_allowed_channels + per-user pairing. CHANGELOG records the behavior change and the retired config fields. The live-QA scripts drop subject handling for allowed-channels admission (200 script tests green), and the orphaned canary env var is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(telegram): connect group chats via telegram_allowed_channels Fail-closed shared-conversation admission left Telegram groups with no operator affordance to connect one — the manifest declared no *_allowed_channels handle, so every group/supergroup @-mention was unadmittable. Declare the handle (the same generic [channel.config] convention Slack uses): listed chats are served with each participant running as themselves once paired; unlisted groups stay fail-closed. Previously any group the bot was added to ran as the deployment operator, which is the exposure this branch removes. Surfaced by the integration scenario telegram_update_becomes_a_turn_and_a_coordinated_reply failing closed after the admission change — kept red until this ruling rather than narrowed to a private chat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(reborn): morph the test tier to invoker scope Every fixture and pin that carried the retired subject model moves to the per-actor rule, with a recorded rationale at each semantic morph: - extension-host channel e2e: an admitted (allowed-channels) shared channel runs as the paired actor; unlisted conversations stay rejected; stored shared-channel outbound targets and binding refs fail closed; the telegram supergroup journey admits its chat via the new telegram_allowed_channels handle and proves the reply as the invoker. - assistant contract suites: admission replaces subject-route coverage (recording/failing/admit-all doubles; not-connected rejections on resolve/lookup/reset including existing bindings — a deliberate flip from the old existing-binding exemption; admission precedes actor-pairing side effects; direct routes never consult admission; per-actor threads for two participants; lookups never surface another actor's thread). - root integration harness + journeys: the binding fake, thread/turn scopes, and the group canonical user derive from the actor; multi-actor isolation pins unchanged and strictly stronger. - parity QA binary harness: subject resolution returns the actor. - webui product API redaction pin: the new telegram admission handle joins the admin-metadata forbidden list. Suites: extension_host 390/0; assistant 1084/0; conversations 105/0; architecture suite full pass; integration bins: extension_delivery 21/0 (Postgres legs under colima), delivery_user_journeys 22/0, mcp 22/0, trace_capture 14/0, generated_gate_sequences 29/0, group_journeys 16/0, group_multiuser 14/0. Workspace cargo fmt applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(changelog): record the telegram_allowed_channels admission field Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(merge): reconcile composition ceilings and capability_wiring test arity Post-merge fixups after folding main (nearai#7157 squashed + nearai#7214 + the inspector prompt-diagnostic work) into run-acts-as-invoker: - Re-capture the composition absolute-mass ceiling 40_747 -> 40_811 in both the budget manifest and reborn_restructure_baselines.rs: the acting-user scope helper and shared-admission wiring add +64 production LOC on the merged tree. Recorded rather than parked in the 150-line tolerance. - Add the 10th `tool_diagnostic_sink` argument (None) to the invoker's capability_wiring test call — main grew the signature after this branch wrote that call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(conversations): refuse legacy-row route-kind mismatches; drop unreachable widen A Direct request is the one key shape a retained legacy conversation-scoped shared row can collide with. Resolve, lookup, reset, and link now refuse the mismatch outright (BindingRequired) instead of trusting adapters never to re-classify a conversation's route kind — pinned by a Direct-probe leg on the legacy restart-path test. The forward half of the migration contract is pinned too: per_actor_shared_bindings_keep_their_threads_across_reopen proves a new per-actor shared binding survives a restart (a deserialize-side regression would previously have orphaned every group thread silently). widen_binding_route_access and ReplyRouteAccess::allow_shared are deleted: every Shared-keyed row is born shared under per-actor keying, so both widen call sites were unreachable. The persisted flag stays for legacy reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(delivery): key gate notices by gate ref on the triggered lane too The gate-collapse fix shipped on the observer lane only; the background lane still minted undiscriminated projection ids for ApprovalNeeded/AuthRequired, so an automation run parking on a SECOND gate deduped to AlreadyDelivered, recorded the whole delivery Failed, and the gate was never announced or reply-routable (AGENTS.md: fix the sibling when a pattern bug is fixed). TriggeredNotification's discriminator now carries the gate ref for gate prompts (RunBlocked stand-ins compose their label with it), matching the observer keying, with a triggered two-gate regression pinning outcome, prompts, and both reply routes. Also pinned: same-gate re-announcement dedupe (g1->g2->g1), two distinct AUTH gates, and the refless id shapes incl. FinalReplyReady. Over-long discriminators are bounded with a stable FNV-1a suffix so a maximal legal TurnGateRef can never overflow ProjectionUpdateRef and silently lose a notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(identity): one contract derivation for every acting-identity scope LoopRunContext::acting_resource_scope joins acting_user_id on the contract type: the raise/resume scope recipe both gate-dance crates hand-synced is now declared once, and every surviving ladder delegates — composition's owner-first resource_scope_for_run (workspace/skill mounts) and the inline grant-minting copy, loop_host's synthetic resume load, and project_create_capability's effective_user_id (deleted; its doc claimed a mirror that no longer existed). On the only run shape where owner and actor differ — legacy runs parked across the deploy — mounts and grants now follow the ACTOR like the rest of the dance; the pin flip is recorded in visible_capability_request_uses_acting_user_for_runtime_scope. The ladder is unit-pinned in its owning crate (all three rungs) and the accepted deploy-boundary resume-miss is pinned on the synthetic port with an acting-scope positive control. loop_contracts ceiling re-captured 13094 -> 13107 with provenance (the +13-line contract method). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(extension-host): collapse admission handles; operator-identity channels never admit ChannelConfigSharedAdmission now holds the one declared *_allowed_channels handle as a plain String (the scan returns Option<String>): 'installed but handle-less' is no longer representable and the per-request Option branch is gone. The root pub use of the admission items is removed — consumers are crate-local and use the module path. Structural closure of the no-auth-vendor residual: a channel whose actor identity is not per-user (no OAuth vendor, no pairing strategy) never receives an admission resolver at all — an operator-identity channel that admitted a group would run every participant as the operator, the exact exposure run-acts-as-invoker removed. Previously this was unreachable only by manifest inventory. The extension_manager wire-shape pin gains the telegram_allowed_channels row (production projection was already correct), and extension_delivery gains the caller-path rejection leg: a correctly-signed webhook for an UNLISTED supergroup is acknowledged but produces no turn and no reply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test+docs(reborn): re-pin parity to per-actor scopes; align contract, docs, vocabulary The identity-parity bin now pins the run-acts-as-invoker property its fixture can actually express: the shared-room support binding keeps ONE thread (the per-actor thread model is locked at the integration tier by scenario_two_actors_own_threads), and inside that shared thread each RUN's scope is owned by its own invoking actor with identity context never crossing. The shared-admission suite gains the reset checkpoint leg (deny before rotation, thread survives), and the connect-nudge suite's shared leg is documented as the deliberate unpaired-participant silence contract. docs/reborn/contracts/conversation-binding.md (the owning contract) is amended: per-actor key in rule 8, participant widening retired in rule 14, subject ownership struck in rule 24, and the admission/retention semantics recorded. Operator docs and CHANGELOG state the real unpaired-shared behavior (silence; pairing via Extensions; DMs still nudge), the CHANGELOG gains the both-lanes gate-announcement entry and Added-first ordering, CHECKLIST's contradictory open-status is reconciled with a dated note, the new REBORN_WEBUI_V2_LIVE_QA_SLACK_ALLOWED_CHANNELS is threaded through live-canary.yml, observer.rs carries its arch-exempt annotation, and retired 'subject' vocabulary is renamed out of live test support and doc comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
…ls, delivery heuristics deleted (nearai#7157) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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 nearai#7361/nearai#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>
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
nearai#7157 follow-ups) (nearai#7377) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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(delivery): key observer gate notices by their gate ref One run can park on several approval/auth gates in sequence (the observer's blocked-state loop re-announces whenever the (status, gate) marker changes), but the live observer derived every gate notice's projection id with no discriminator, so all ApprovalNeeded notices in one run collapsed to a single durable delivery identity. The second gate's prompt came back AlreadyDelivered from the coordinator, was treated as success, and was never sent — the user was never told about the gate their run was parked on, and no reply route was recorded for it, so a bare `approve` could not resolve it either. Key the projection id by the notification's gate ref (the mechanism nearai#7157 added for the triggered notifier's RunBlocked notices). A repeat announcement of the SAME gate still dedupes; kinds that carry no gate ref (FinalReplyReady) keep the historical undiscriminated id shape so existing delivery identities are not re-keyed. Regression: observer_delivers_a_prompt_for_each_distinct_approval_gate drives the real DeliveryCoordinator over the real outbound store through two distinct scripted gates and asserts two delivered prompts plus a recorded reply route for each. Sabotage-verified: reverting the discriminator to None fails exactly this test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(composition): pin the notification-channels gate dance when owner ≠ actor The full builtin.notification_channels_set approval dance — raise, replay payload, user approve (store + lease mint from the stored row), approved resume, lease claim, dispatch, lease consume — driven through the real capability port on a run whose thread owner differs from its acting user. Pins two properties ahead of unifying the scope derivation onto the actor: raise and resume must derive the same scope (every store in the dance is scope-keyed, so a half-unified derivation strands the approved capability), and whose identity that scope carries (the thread owner, under the interim split nearai#7157 shipped). The approve step mints the lease from the stored request's own scope, grantee, and fingerprint — the same material the production click-approval resolution uses — never a re-derivation. Capability-host tier rather than tests/integration because the product rule "a run acts as its invoker" makes owner ≠ actor unconstructible through every product front door; the run-context shape remains legal kernel state (runs parked across the deploy boundary carry it). The owner == actor dance stays covered end-to-end at the integration tier (outbound_target.rs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(outbound): scope the whole notification-channels gate dance as the acting user Unify the interim nearai#7157 split: resource_scope_for_run and settings_scope_for_run now derive from the acting user like caller_for_run already did, and effective_user_id (the owner-first ladder) is deleted. The approval-gate raise, the replay payload, the durable gate record, the lease, and the approval-settings read all follow the user who invoked the run — so the invoker sees and approves the gate, and their settings govern it. The raise/resume coverage added one commit earlier ran before and after this change and caught a real half-unification in between: the resume-side replay load lives in ironclaw_loop_host's synthetic-capability wrap (a different crate from the raise-side save in notification_channels_set) and still derived owner-first, stranding an approved resume with "replay payload is unavailable". The acting-identity ladder now has exactly one definition — LoopRunContext::acting_user_id in ironclaw_loop_contracts — and both sides delegate to it, so the hand-synced-copy class is closed rather than re-synced. notification_channels_set's replay/gate-record writes move from the capability_host-wide owner-first helper onto the outbound module's base_resource_scope_for_run so every store in one dance derives one user; the capability_host-wide helper itself is unchanged (thread/durable-result scoping legitimately follows thread ownership, and owner == actor on every binding created under the run-acts-as-invoker rule). Loop-contracts size ceiling: +16 lines for the shared ladder, paid for by splitting host/run_context.rs's 104-line inline #[cfg(test)] module into its run_context/tests.rs sibling; ceiling re-captured DOWN 13_115 -> 13_028 from the gate's own failure message. Runs raised before this change with owner != actor and resumed after it will miss their replay payload once and fail closed; re-requesting approval recovers. Documented in the PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(conversations): key shared-route bindings per (conversation, actor) A run acts as the user who invoked it, so a shared conversation binds one thread per paired actor — each owned by that actor — instead of one conversation-wide thread owned by a configured subject. BindingKey gains a serde-defaulted shared_actor_user_id component (None for Direct routes, whose identity stays the conversation alone); the trusted-owner parameter is deliberately ignored on Shared creates (it remains the trigger lane's way to bind Direct conversations for their creator), and the legacy shared-owner backfill is removed with it. Migration is ignore-but-retain, pinned with a restart-path test: legacy Direct keys deserialize byte-identically (continuity), while legacy conversation-keyed shared rows deserialize to a key no per-actor lookup builds — retained in durable state untouched, and every participant (including the old subject) starts a fresh thread they own. Morphed legacy pins record what became structural: a shared probe/lookup can no longer address (or widen) a Direct binding at all; stored reply targets are isolated per actor; an actor's unpair cannot take the conversation away from other participants' own threads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(product)!: remove shared-route subject binding; scope = invoker Owner ruling: a run acts as the user who invoked it, in a DM and in a shared channel alike, with one thread per (conversation, user). This removes the subject half of shared-route configuration end to end and keeps the admission half, fail-closed: - ironclaw_product_contracts: subject_route becomes shared_admission — the SharedConversationAdmission port answers only "is this shared conversation connected"; ProductConversationRouteKey survives as the admission key. ResolvedBinding loses subject_user_id (retired-field JSON still deserializes; persisted-shape test updated); the actor is the one identity. - ironclaw_assistant: ProductInstallationScope drops the default-subject, static-route, and subject-resolver knobs for one shared_conversation_admission port; resolve/lookup/reset check admission fail-closed (no port wired, or an unlisted conversation, rejects with a not-connected BindingRequired); resolve passes no trusted owner — the conversations domain keys and owns shared bindings by the paired actor. Thread and turn scopes derive their owner from the binding's actor on every route kind. - ironclaw_extension_host: channel_subject_routes.rs becomes channel_shared_admission.rs; ChannelConfigSharedAdmission admits by membership in the operator-saved *_allowed_channels JSON array; the managed derived subject (user:{ext}-channel:{sha16}) is deleted; legacy *_subject_routes values are inert (pinned by test). Shared conversations are no longer offered as per-user notification delivery targets — their ownership came from the retired subject map — and stored channel-target preferences fail closed at resolution; DM targets are unchanged. - slack manifest: slack_shared_subject_user_id and slack_subject_routes are retired with a gravestone comment; slack_allowed_channels is the admission surface (saves to the retired handles already fail closed as unknown fields — the extension-config analog of the config.toml retired-section gravestone). - architecture tests: the INVERTED_PORTS row moves with the port rename. User-visible consequences (also in the PR body): each shared-channel participant now gets their own persistent thread and must be paired; no cross-user shared context; the operator's identity is never a fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(reborn): align guidance, specs, and live-QA scripts with invoker scope Guidance rows and the moved-ports list rename the inverted port (SharedConversationAdmission, ex ProductConversationSubjectRouteResolver); the assistant boundary prose states the new rule (one thread per (conversation, actor), admission is the only shared-conversation configuration, fail-closed on resolve/lookup/reset). The composition CONTRACT.md's never-shipped per-channel subject admin API section is excised with a dated correction; CHECKLIST/PROPOSAL get dated amendments beside the historical text. Operator docs teach slack_allowed_channels + per-user pairing. CHANGELOG records the behavior change and the retired config fields. The live-QA scripts drop subject handling for allowed-channels admission (200 script tests green), and the orphaned canary env var is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(telegram): connect group chats via telegram_allowed_channels Fail-closed shared-conversation admission left Telegram groups with no operator affordance to connect one — the manifest declared no *_allowed_channels handle, so every group/supergroup @-mention was unadmittable. Declare the handle (the same generic [channel.config] convention Slack uses): listed chats are served with each participant running as themselves once paired; unlisted groups stay fail-closed. Previously any group the bot was added to ran as the deployment operator, which is the exposure this branch removes. Surfaced by the integration scenario telegram_update_becomes_a_turn_and_a_coordinated_reply failing closed after the admission change — kept red until this ruling rather than narrowed to a private chat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(reborn): morph the test tier to invoker scope Every fixture and pin that carried the retired subject model moves to the per-actor rule, with a recorded rationale at each semantic morph: - extension-host channel e2e: an admitted (allowed-channels) shared channel runs as the paired actor; unlisted conversations stay rejected; stored shared-channel outbound targets and binding refs fail closed; the telegram supergroup journey admits its chat via the new telegram_allowed_channels handle and proves the reply as the invoker. - assistant contract suites: admission replaces subject-route coverage (recording/failing/admit-all doubles; not-connected rejections on resolve/lookup/reset including existing bindings — a deliberate flip from the old existing-binding exemption; admission precedes actor-pairing side effects; direct routes never consult admission; per-actor threads for two participants; lookups never surface another actor's thread). - root integration harness + journeys: the binding fake, thread/turn scopes, and the group canonical user derive from the actor; multi-actor isolation pins unchanged and strictly stronger. - parity QA binary harness: subject resolution returns the actor. - webui product API redaction pin: the new telegram admission handle joins the admin-metadata forbidden list. Suites: extension_host 390/0; assistant 1084/0; conversations 105/0; architecture suite full pass; integration bins: extension_delivery 21/0 (Postgres legs under colima), delivery_user_journeys 22/0, mcp 22/0, trace_capture 14/0, generated_gate_sequences 29/0, group_journeys 16/0, group_multiuser 14/0. Workspace cargo fmt applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(changelog): record the telegram_allowed_channels admission field Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(merge): reconcile composition ceilings and capability_wiring test arity Post-merge fixups after folding main (nearai#7157 squashed + nearai#7214 + the inspector prompt-diagnostic work) into run-acts-as-invoker: - Re-capture the composition absolute-mass ceiling 40_747 -> 40_811 in both the budget manifest and reborn_restructure_baselines.rs: the acting-user scope helper and shared-admission wiring add +64 production LOC on the merged tree. Recorded rather than parked in the 150-line tolerance. - Add the 10th `tool_diagnostic_sink` argument (None) to the invoker's capability_wiring test call — main grew the signature after this branch wrote that call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(conversations): refuse legacy-row route-kind mismatches; drop unreachable widen A Direct request is the one key shape a retained legacy conversation-scoped shared row can collide with. Resolve, lookup, reset, and link now refuse the mismatch outright (BindingRequired) instead of trusting adapters never to re-classify a conversation's route kind — pinned by a Direct-probe leg on the legacy restart-path test. The forward half of the migration contract is pinned too: per_actor_shared_bindings_keep_their_threads_across_reopen proves a new per-actor shared binding survives a restart (a deserialize-side regression would previously have orphaned every group thread silently). widen_binding_route_access and ReplyRouteAccess::allow_shared are deleted: every Shared-keyed row is born shared under per-actor keying, so both widen call sites were unreachable. The persisted flag stays for legacy reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(delivery): key gate notices by gate ref on the triggered lane too The gate-collapse fix shipped on the observer lane only; the background lane still minted undiscriminated projection ids for ApprovalNeeded/AuthRequired, so an automation run parking on a SECOND gate deduped to AlreadyDelivered, recorded the whole delivery Failed, and the gate was never announced or reply-routable (AGENTS.md: fix the sibling when a pattern bug is fixed). TriggeredNotification's discriminator now carries the gate ref for gate prompts (RunBlocked stand-ins compose their label with it), matching the observer keying, with a triggered two-gate regression pinning outcome, prompts, and both reply routes. Also pinned: same-gate re-announcement dedupe (g1->g2->g1), two distinct AUTH gates, and the refless id shapes incl. FinalReplyReady. Over-long discriminators are bounded with a stable FNV-1a suffix so a maximal legal TurnGateRef can never overflow ProjectionUpdateRef and silently lose a notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(identity): one contract derivation for every acting-identity scope LoopRunContext::acting_resource_scope joins acting_user_id on the contract type: the raise/resume scope recipe both gate-dance crates hand-synced is now declared once, and every surviving ladder delegates — composition's owner-first resource_scope_for_run (workspace/skill mounts) and the inline grant-minting copy, loop_host's synthetic resume load, and project_create_capability's effective_user_id (deleted; its doc claimed a mirror that no longer existed). On the only run shape where owner and actor differ — legacy runs parked across the deploy — mounts and grants now follow the ACTOR like the rest of the dance; the pin flip is recorded in visible_capability_request_uses_acting_user_for_runtime_scope. The ladder is unit-pinned in its owning crate (all three rungs) and the accepted deploy-boundary resume-miss is pinned on the synthetic port with an acting-scope positive control. loop_contracts ceiling re-captured 13094 -> 13107 with provenance (the +13-line contract method). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(extension-host): collapse admission handles; operator-identity channels never admit ChannelConfigSharedAdmission now holds the one declared *_allowed_channels handle as a plain String (the scan returns Option<String>): 'installed but handle-less' is no longer representable and the per-request Option branch is gone. The root pub use of the admission items is removed — consumers are crate-local and use the module path. Structural closure of the no-auth-vendor residual: a channel whose actor identity is not per-user (no OAuth vendor, no pairing strategy) never receives an admission resolver at all — an operator-identity channel that admitted a group would run every participant as the operator, the exact exposure run-acts-as-invoker removed. Previously this was unreachable only by manifest inventory. The extension_manager wire-shape pin gains the telegram_allowed_channels row (production projection was already correct), and extension_delivery gains the caller-path rejection leg: a correctly-signed webhook for an UNLISTED supergroup is acknowledged but produces no turn and no reply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test+docs(reborn): re-pin parity to per-actor scopes; align contract, docs, vocabulary The identity-parity bin now pins the run-acts-as-invoker property its fixture can actually express: the shared-room support binding keeps ONE thread (the per-actor thread model is locked at the integration tier by scenario_two_actors_own_threads), and inside that shared thread each RUN's scope is owned by its own invoking actor with identity context never crossing. The shared-admission suite gains the reset checkpoint leg (deny before rotation, thread survives), and the connect-nudge suite's shared leg is documented as the deliberate unpaired-participant silence contract. docs/reborn/contracts/conversation-binding.md (the owning contract) is amended: per-actor key in rule 8, participant widening retired in rule 14, subject ownership struck in rule 24, and the admission/retention semantics recorded. Operator docs and CHANGELOG state the real unpaired-shared behavior (silence; pairing via Extensions; DMs still nudge), the CHANGELOG gains the both-lanes gate-announcement entry and Added-first ordering, CHECKLIST's contradictory open-status is reconciled with a dated note, the new REBORN_WEBUI_V2_LIVE_QA_SLACK_ALLOWED_CHANNELS is threaded through live-canary.yml, observer.rs carries its arch-exempt annotation, and retired 'subject' vocabulary is renamed out of live test support and doc comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
…ls, delivery heuristics deleted (nearai#7157) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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 nearai#7361/nearai#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>
Kampouse
pushed a commit
to Kampouse/ironclaw
that referenced
this pull request
Aug 13, 2026
nearai#7157 follow-ups) (nearai#7377) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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(delivery): key observer gate notices by their gate ref One run can park on several approval/auth gates in sequence (the observer's blocked-state loop re-announces whenever the (status, gate) marker changes), but the live observer derived every gate notice's projection id with no discriminator, so all ApprovalNeeded notices in one run collapsed to a single durable delivery identity. The second gate's prompt came back AlreadyDelivered from the coordinator, was treated as success, and was never sent — the user was never told about the gate their run was parked on, and no reply route was recorded for it, so a bare `approve` could not resolve it either. Key the projection id by the notification's gate ref (the mechanism nearai#7157 added for the triggered notifier's RunBlocked notices). A repeat announcement of the SAME gate still dedupes; kinds that carry no gate ref (FinalReplyReady) keep the historical undiscriminated id shape so existing delivery identities are not re-keyed. Regression: observer_delivers_a_prompt_for_each_distinct_approval_gate drives the real DeliveryCoordinator over the real outbound store through two distinct scripted gates and asserts two delivered prompts plus a recorded reply route for each. Sabotage-verified: reverting the discriminator to None fails exactly this test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(composition): pin the notification-channels gate dance when owner ≠ actor The full builtin.notification_channels_set approval dance — raise, replay payload, user approve (store + lease mint from the stored row), approved resume, lease claim, dispatch, lease consume — driven through the real capability port on a run whose thread owner differs from its acting user. Pins two properties ahead of unifying the scope derivation onto the actor: raise and resume must derive the same scope (every store in the dance is scope-keyed, so a half-unified derivation strands the approved capability), and whose identity that scope carries (the thread owner, under the interim split nearai#7157 shipped). The approve step mints the lease from the stored request's own scope, grantee, and fingerprint — the same material the production click-approval resolution uses — never a re-derivation. Capability-host tier rather than tests/integration because the product rule "a run acts as its invoker" makes owner ≠ actor unconstructible through every product front door; the run-context shape remains legal kernel state (runs parked across the deploy boundary carry it). The owner == actor dance stays covered end-to-end at the integration tier (outbound_target.rs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(outbound): scope the whole notification-channels gate dance as the acting user Unify the interim nearai#7157 split: resource_scope_for_run and settings_scope_for_run now derive from the acting user like caller_for_run already did, and effective_user_id (the owner-first ladder) is deleted. The approval-gate raise, the replay payload, the durable gate record, the lease, and the approval-settings read all follow the user who invoked the run — so the invoker sees and approves the gate, and their settings govern it. The raise/resume coverage added one commit earlier ran before and after this change and caught a real half-unification in between: the resume-side replay load lives in ironclaw_loop_host's synthetic-capability wrap (a different crate from the raise-side save in notification_channels_set) and still derived owner-first, stranding an approved resume with "replay payload is unavailable". The acting-identity ladder now has exactly one definition — LoopRunContext::acting_user_id in ironclaw_loop_contracts — and both sides delegate to it, so the hand-synced-copy class is closed rather than re-synced. notification_channels_set's replay/gate-record writes move from the capability_host-wide owner-first helper onto the outbound module's base_resource_scope_for_run so every store in one dance derives one user; the capability_host-wide helper itself is unchanged (thread/durable-result scoping legitimately follows thread ownership, and owner == actor on every binding created under the run-acts-as-invoker rule). Loop-contracts size ceiling: +16 lines for the shared ladder, paid for by splitting host/run_context.rs's 104-line inline #[cfg(test)] module into its run_context/tests.rs sibling; ceiling re-captured DOWN 13_115 -> 13_028 from the gate's own failure message. Runs raised before this change with owner != actor and resumed after it will miss their replay payload once and fail closed; re-requesting approval recovers. Documented in the PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(conversations): key shared-route bindings per (conversation, actor) A run acts as the user who invoked it, so a shared conversation binds one thread per paired actor — each owned by that actor — instead of one conversation-wide thread owned by a configured subject. BindingKey gains a serde-defaulted shared_actor_user_id component (None for Direct routes, whose identity stays the conversation alone); the trusted-owner parameter is deliberately ignored on Shared creates (it remains the trigger lane's way to bind Direct conversations for their creator), and the legacy shared-owner backfill is removed with it. Migration is ignore-but-retain, pinned with a restart-path test: legacy Direct keys deserialize byte-identically (continuity), while legacy conversation-keyed shared rows deserialize to a key no per-actor lookup builds — retained in durable state untouched, and every participant (including the old subject) starts a fresh thread they own. Morphed legacy pins record what became structural: a shared probe/lookup can no longer address (or widen) a Direct binding at all; stored reply targets are isolated per actor; an actor's unpair cannot take the conversation away from other participants' own threads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(product)!: remove shared-route subject binding; scope = invoker Owner ruling: a run acts as the user who invoked it, in a DM and in a shared channel alike, with one thread per (conversation, user). This removes the subject half of shared-route configuration end to end and keeps the admission half, fail-closed: - ironclaw_product_contracts: subject_route becomes shared_admission — the SharedConversationAdmission port answers only "is this shared conversation connected"; ProductConversationRouteKey survives as the admission key. ResolvedBinding loses subject_user_id (retired-field JSON still deserializes; persisted-shape test updated); the actor is the one identity. - ironclaw_assistant: ProductInstallationScope drops the default-subject, static-route, and subject-resolver knobs for one shared_conversation_admission port; resolve/lookup/reset check admission fail-closed (no port wired, or an unlisted conversation, rejects with a not-connected BindingRequired); resolve passes no trusted owner — the conversations domain keys and owns shared bindings by the paired actor. Thread and turn scopes derive their owner from the binding's actor on every route kind. - ironclaw_extension_host: channel_subject_routes.rs becomes channel_shared_admission.rs; ChannelConfigSharedAdmission admits by membership in the operator-saved *_allowed_channels JSON array; the managed derived subject (user:{ext}-channel:{sha16}) is deleted; legacy *_subject_routes values are inert (pinned by test). Shared conversations are no longer offered as per-user notification delivery targets — their ownership came from the retired subject map — and stored channel-target preferences fail closed at resolution; DM targets are unchanged. - slack manifest: slack_shared_subject_user_id and slack_subject_routes are retired with a gravestone comment; slack_allowed_channels is the admission surface (saves to the retired handles already fail closed as unknown fields — the extension-config analog of the config.toml retired-section gravestone). - architecture tests: the INVERTED_PORTS row moves with the port rename. User-visible consequences (also in the PR body): each shared-channel participant now gets their own persistent thread and must be paired; no cross-user shared context; the operator's identity is never a fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(reborn): align guidance, specs, and live-QA scripts with invoker scope Guidance rows and the moved-ports list rename the inverted port (SharedConversationAdmission, ex ProductConversationSubjectRouteResolver); the assistant boundary prose states the new rule (one thread per (conversation, actor), admission is the only shared-conversation configuration, fail-closed on resolve/lookup/reset). The composition CONTRACT.md's never-shipped per-channel subject admin API section is excised with a dated correction; CHECKLIST/PROPOSAL get dated amendments beside the historical text. Operator docs teach slack_allowed_channels + per-user pairing. CHANGELOG records the behavior change and the retired config fields. The live-QA scripts drop subject handling for allowed-channels admission (200 script tests green), and the orphaned canary env var is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(telegram): connect group chats via telegram_allowed_channels Fail-closed shared-conversation admission left Telegram groups with no operator affordance to connect one — the manifest declared no *_allowed_channels handle, so every group/supergroup @-mention was unadmittable. Declare the handle (the same generic [channel.config] convention Slack uses): listed chats are served with each participant running as themselves once paired; unlisted groups stay fail-closed. Previously any group the bot was added to ran as the deployment operator, which is the exposure this branch removes. Surfaced by the integration scenario telegram_update_becomes_a_turn_and_a_coordinated_reply failing closed after the admission change — kept red until this ruling rather than narrowed to a private chat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(reborn): morph the test tier to invoker scope Every fixture and pin that carried the retired subject model moves to the per-actor rule, with a recorded rationale at each semantic morph: - extension-host channel e2e: an admitted (allowed-channels) shared channel runs as the paired actor; unlisted conversations stay rejected; stored shared-channel outbound targets and binding refs fail closed; the telegram supergroup journey admits its chat via the new telegram_allowed_channels handle and proves the reply as the invoker. - assistant contract suites: admission replaces subject-route coverage (recording/failing/admit-all doubles; not-connected rejections on resolve/lookup/reset including existing bindings — a deliberate flip from the old existing-binding exemption; admission precedes actor-pairing side effects; direct routes never consult admission; per-actor threads for two participants; lookups never surface another actor's thread). - root integration harness + journeys: the binding fake, thread/turn scopes, and the group canonical user derive from the actor; multi-actor isolation pins unchanged and strictly stronger. - parity QA binary harness: subject resolution returns the actor. - webui product API redaction pin: the new telegram admission handle joins the admin-metadata forbidden list. Suites: extension_host 390/0; assistant 1084/0; conversations 105/0; architecture suite full pass; integration bins: extension_delivery 21/0 (Postgres legs under colima), delivery_user_journeys 22/0, mcp 22/0, trace_capture 14/0, generated_gate_sequences 29/0, group_journeys 16/0, group_multiuser 14/0. Workspace cargo fmt applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(changelog): record the telegram_allowed_channels admission field Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(merge): reconcile composition ceilings and capability_wiring test arity Post-merge fixups after folding main (nearai#7157 squashed + nearai#7214 + the inspector prompt-diagnostic work) into run-acts-as-invoker: - Re-capture the composition absolute-mass ceiling 40_747 -> 40_811 in both the budget manifest and reborn_restructure_baselines.rs: the acting-user scope helper and shared-admission wiring add +64 production LOC on the merged tree. Recorded rather than parked in the 150-line tolerance. - Add the 10th `tool_diagnostic_sink` argument (None) to the invoker's capability_wiring test call — main grew the signature after this branch wrote that call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(conversations): refuse legacy-row route-kind mismatches; drop unreachable widen A Direct request is the one key shape a retained legacy conversation-scoped shared row can collide with. Resolve, lookup, reset, and link now refuse the mismatch outright (BindingRequired) instead of trusting adapters never to re-classify a conversation's route kind — pinned by a Direct-probe leg on the legacy restart-path test. The forward half of the migration contract is pinned too: per_actor_shared_bindings_keep_their_threads_across_reopen proves a new per-actor shared binding survives a restart (a deserialize-side regression would previously have orphaned every group thread silently). widen_binding_route_access and ReplyRouteAccess::allow_shared are deleted: every Shared-keyed row is born shared under per-actor keying, so both widen call sites were unreachable. The persisted flag stays for legacy reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(delivery): key gate notices by gate ref on the triggered lane too The gate-collapse fix shipped on the observer lane only; the background lane still minted undiscriminated projection ids for ApprovalNeeded/AuthRequired, so an automation run parking on a SECOND gate deduped to AlreadyDelivered, recorded the whole delivery Failed, and the gate was never announced or reply-routable (AGENTS.md: fix the sibling when a pattern bug is fixed). TriggeredNotification's discriminator now carries the gate ref for gate prompts (RunBlocked stand-ins compose their label with it), matching the observer keying, with a triggered two-gate regression pinning outcome, prompts, and both reply routes. Also pinned: same-gate re-announcement dedupe (g1->g2->g1), two distinct AUTH gates, and the refless id shapes incl. FinalReplyReady. Over-long discriminators are bounded with a stable FNV-1a suffix so a maximal legal TurnGateRef can never overflow ProjectionUpdateRef and silently lose a notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(identity): one contract derivation for every acting-identity scope LoopRunContext::acting_resource_scope joins acting_user_id on the contract type: the raise/resume scope recipe both gate-dance crates hand-synced is now declared once, and every surviving ladder delegates — composition's owner-first resource_scope_for_run (workspace/skill mounts) and the inline grant-minting copy, loop_host's synthetic resume load, and project_create_capability's effective_user_id (deleted; its doc claimed a mirror that no longer existed). On the only run shape where owner and actor differ — legacy runs parked across the deploy — mounts and grants now follow the ACTOR like the rest of the dance; the pin flip is recorded in visible_capability_request_uses_acting_user_for_runtime_scope. The ladder is unit-pinned in its owning crate (all three rungs) and the accepted deploy-boundary resume-miss is pinned on the synthetic port with an acting-scope positive control. loop_contracts ceiling re-captured 13094 -> 13107 with provenance (the +13-line contract method). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(extension-host): collapse admission handles; operator-identity channels never admit ChannelConfigSharedAdmission now holds the one declared *_allowed_channels handle as a plain String (the scan returns Option<String>): 'installed but handle-less' is no longer representable and the per-request Option branch is gone. The root pub use of the admission items is removed — consumers are crate-local and use the module path. Structural closure of the no-auth-vendor residual: a channel whose actor identity is not per-user (no OAuth vendor, no pairing strategy) never receives an admission resolver at all — an operator-identity channel that admitted a group would run every participant as the operator, the exact exposure run-acts-as-invoker removed. Previously this was unreachable only by manifest inventory. The extension_manager wire-shape pin gains the telegram_allowed_channels row (production projection was already correct), and extension_delivery gains the caller-path rejection leg: a correctly-signed webhook for an UNLISTED supergroup is acknowledged but produces no turn and no reply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test+docs(reborn): re-pin parity to per-actor scopes; align contract, docs, vocabulary The identity-parity bin now pins the run-acts-as-invoker property its fixture can actually express: the shared-room support binding keeps ONE thread (the per-actor thread model is locked at the integration tier by scenario_two_actors_own_threads), and inside that shared thread each RUN's scope is owned by its own invoking actor with identity context never crossing. The shared-admission suite gains the reset checkpoint leg (deny before rotation, thread survives), and the connect-nudge suite's shared leg is documented as the deliberate unpaired-participant silence contract. docs/reborn/contracts/conversation-binding.md (the owning contract) is amended: per-actor key in rule 8, participant widening retired in rule 14, subject ownership struck in rule 24, and the admission/retention semantics recorded. Operator docs and CHANGELOG state the real unpaired-shared behavior (silence; pairing via Extensions; DMs still nudge), the CHANGELOG gains the both-lanes gate-announcement entry and Added-first ordering, CHECKLIST's contradictory open-status is reconciled with a dated note, the new REBORN_WEBUI_V2_LIVE_QA_SLACK_ALLOWED_CHANNELS is threaded through live-canary.yml, observer.rs carries its arch-exempt annotation, and retired 'subject' vocabulary is renamed out of live test support and doc comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
9 tasks
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* feat(inspector): add bounded diagnostic session store * fix(inspector): address diagnostics store review feedback * fix(inspector): preserve ordered scoped live updates * fix(inspector): bound retained live update scopes * fix(inspector): keep diagnostics aligned to run scope * fix(inspector): deserialize diagnostic wire enums * fix(inspector): validate diagnostic text decoding * fix(inspector): enforce owning diagnostic decode limits * fix(inspector): enforce prompt retention boundaries * fix(inspector): bound all retained diagnostics * test(architecture): recapture product contracts ceiling
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…ls, delivery heuristics deleted (nearai#7157) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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 nearai#7361/nearai#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>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
nearai#7157 follow-ups) (nearai#7377) * feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR nearai#7157 on current main (aa8b748 → 967f33a, 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 b2023bc (nearai#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 b72d7da (nearai#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 nearai#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 c69ed2d (nearai#7263 program-closure batch + sibling fixes): ceiling-table union (product_contracts 15685 from nearai#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 0c297cb (nearai#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; nearai#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(delivery): key observer gate notices by their gate ref One run can park on several approval/auth gates in sequence (the observer's blocked-state loop re-announces whenever the (status, gate) marker changes), but the live observer derived every gate notice's projection id with no discriminator, so all ApprovalNeeded notices in one run collapsed to a single durable delivery identity. The second gate's prompt came back AlreadyDelivered from the coordinator, was treated as success, and was never sent — the user was never told about the gate their run was parked on, and no reply route was recorded for it, so a bare `approve` could not resolve it either. Key the projection id by the notification's gate ref (the mechanism nearai#7157 added for the triggered notifier's RunBlocked notices). A repeat announcement of the SAME gate still dedupes; kinds that carry no gate ref (FinalReplyReady) keep the historical undiscriminated id shape so existing delivery identities are not re-keyed. Regression: observer_delivers_a_prompt_for_each_distinct_approval_gate drives the real DeliveryCoordinator over the real outbound store through two distinct scripted gates and asserts two delivered prompts plus a recorded reply route for each. Sabotage-verified: reverting the discriminator to None fails exactly this test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(composition): pin the notification-channels gate dance when owner ≠ actor The full builtin.notification_channels_set approval dance — raise, replay payload, user approve (store + lease mint from the stored row), approved resume, lease claim, dispatch, lease consume — driven through the real capability port on a run whose thread owner differs from its acting user. Pins two properties ahead of unifying the scope derivation onto the actor: raise and resume must derive the same scope (every store in the dance is scope-keyed, so a half-unified derivation strands the approved capability), and whose identity that scope carries (the thread owner, under the interim split nearai#7157 shipped). The approve step mints the lease from the stored request's own scope, grantee, and fingerprint — the same material the production click-approval resolution uses — never a re-derivation. Capability-host tier rather than tests/integration because the product rule "a run acts as its invoker" makes owner ≠ actor unconstructible through every product front door; the run-context shape remains legal kernel state (runs parked across the deploy boundary carry it). The owner == actor dance stays covered end-to-end at the integration tier (outbound_target.rs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(outbound): scope the whole notification-channels gate dance as the acting user Unify the interim nearai#7157 split: resource_scope_for_run and settings_scope_for_run now derive from the acting user like caller_for_run already did, and effective_user_id (the owner-first ladder) is deleted. The approval-gate raise, the replay payload, the durable gate record, the lease, and the approval-settings read all follow the user who invoked the run — so the invoker sees and approves the gate, and their settings govern it. The raise/resume coverage added one commit earlier ran before and after this change and caught a real half-unification in between: the resume-side replay load lives in ironclaw_loop_host's synthetic-capability wrap (a different crate from the raise-side save in notification_channels_set) and still derived owner-first, stranding an approved resume with "replay payload is unavailable". The acting-identity ladder now has exactly one definition — LoopRunContext::acting_user_id in ironclaw_loop_contracts — and both sides delegate to it, so the hand-synced-copy class is closed rather than re-synced. notification_channels_set's replay/gate-record writes move from the capability_host-wide owner-first helper onto the outbound module's base_resource_scope_for_run so every store in one dance derives one user; the capability_host-wide helper itself is unchanged (thread/durable-result scoping legitimately follows thread ownership, and owner == actor on every binding created under the run-acts-as-invoker rule). Loop-contracts size ceiling: +16 lines for the shared ladder, paid for by splitting host/run_context.rs's 104-line inline #[cfg(test)] module into its run_context/tests.rs sibling; ceiling re-captured DOWN 13_115 -> 13_028 from the gate's own failure message. Runs raised before this change with owner != actor and resumed after it will miss their replay payload once and fail closed; re-requesting approval recovers. Documented in the PR body. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(conversations): key shared-route bindings per (conversation, actor) A run acts as the user who invoked it, so a shared conversation binds one thread per paired actor — each owned by that actor — instead of one conversation-wide thread owned by a configured subject. BindingKey gains a serde-defaulted shared_actor_user_id component (None for Direct routes, whose identity stays the conversation alone); the trusted-owner parameter is deliberately ignored on Shared creates (it remains the trigger lane's way to bind Direct conversations for their creator), and the legacy shared-owner backfill is removed with it. Migration is ignore-but-retain, pinned with a restart-path test: legacy Direct keys deserialize byte-identically (continuity), while legacy conversation-keyed shared rows deserialize to a key no per-actor lookup builds — retained in durable state untouched, and every participant (including the old subject) starts a fresh thread they own. Morphed legacy pins record what became structural: a shared probe/lookup can no longer address (or widen) a Direct binding at all; stored reply targets are isolated per actor; an actor's unpair cannot take the conversation away from other participants' own threads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(product)!: remove shared-route subject binding; scope = invoker Owner ruling: a run acts as the user who invoked it, in a DM and in a shared channel alike, with one thread per (conversation, user). This removes the subject half of shared-route configuration end to end and keeps the admission half, fail-closed: - ironclaw_product_contracts: subject_route becomes shared_admission — the SharedConversationAdmission port answers only "is this shared conversation connected"; ProductConversationRouteKey survives as the admission key. ResolvedBinding loses subject_user_id (retired-field JSON still deserializes; persisted-shape test updated); the actor is the one identity. - ironclaw_assistant: ProductInstallationScope drops the default-subject, static-route, and subject-resolver knobs for one shared_conversation_admission port; resolve/lookup/reset check admission fail-closed (no port wired, or an unlisted conversation, rejects with a not-connected BindingRequired); resolve passes no trusted owner — the conversations domain keys and owns shared bindings by the paired actor. Thread and turn scopes derive their owner from the binding's actor on every route kind. - ironclaw_extension_host: channel_subject_routes.rs becomes channel_shared_admission.rs; ChannelConfigSharedAdmission admits by membership in the operator-saved *_allowed_channels JSON array; the managed derived subject (user:{ext}-channel:{sha16}) is deleted; legacy *_subject_routes values are inert (pinned by test). Shared conversations are no longer offered as per-user notification delivery targets — their ownership came from the retired subject map — and stored channel-target preferences fail closed at resolution; DM targets are unchanged. - slack manifest: slack_shared_subject_user_id and slack_subject_routes are retired with a gravestone comment; slack_allowed_channels is the admission surface (saves to the retired handles already fail closed as unknown fields — the extension-config analog of the config.toml retired-section gravestone). - architecture tests: the INVERTED_PORTS row moves with the port rename. User-visible consequences (also in the PR body): each shared-channel participant now gets their own persistent thread and must be paired; no cross-user shared context; the operator's identity is never a fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(reborn): align guidance, specs, and live-QA scripts with invoker scope Guidance rows and the moved-ports list rename the inverted port (SharedConversationAdmission, ex ProductConversationSubjectRouteResolver); the assistant boundary prose states the new rule (one thread per (conversation, actor), admission is the only shared-conversation configuration, fail-closed on resolve/lookup/reset). The composition CONTRACT.md's never-shipped per-channel subject admin API section is excised with a dated correction; CHECKLIST/PROPOSAL get dated amendments beside the historical text. Operator docs teach slack_allowed_channels + per-user pairing. CHANGELOG records the behavior change and the retired config fields. The live-QA scripts drop subject handling for allowed-channels admission (200 script tests green), and the orphaned canary env var is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * feat(telegram): connect group chats via telegram_allowed_channels Fail-closed shared-conversation admission left Telegram groups with no operator affordance to connect one — the manifest declared no *_allowed_channels handle, so every group/supergroup @-mention was unadmittable. Declare the handle (the same generic [channel.config] convention Slack uses): listed chats are served with each participant running as themselves once paired; unlisted groups stay fail-closed. Previously any group the bot was added to ran as the deployment operator, which is the exposure this branch removes. Surfaced by the integration scenario telegram_update_becomes_a_turn_and_a_coordinated_reply failing closed after the admission change — kept red until this ruling rather than narrowed to a private chat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * test(reborn): morph the test tier to invoker scope Every fixture and pin that carried the retired subject model moves to the per-actor rule, with a recorded rationale at each semantic morph: - extension-host channel e2e: an admitted (allowed-channels) shared channel runs as the paired actor; unlisted conversations stay rejected; stored shared-channel outbound targets and binding refs fail closed; the telegram supergroup journey admits its chat via the new telegram_allowed_channels handle and proves the reply as the invoker. - assistant contract suites: admission replaces subject-route coverage (recording/failing/admit-all doubles; not-connected rejections on resolve/lookup/reset including existing bindings — a deliberate flip from the old existing-binding exemption; admission precedes actor-pairing side effects; direct routes never consult admission; per-actor threads for two participants; lookups never surface another actor's thread). - root integration harness + journeys: the binding fake, thread/turn scopes, and the group canonical user derive from the actor; multi-actor isolation pins unchanged and strictly stronger. - parity QA binary harness: subject resolution returns the actor. - webui product API redaction pin: the new telegram admission handle joins the admin-metadata forbidden list. Suites: extension_host 390/0; assistant 1084/0; conversations 105/0; architecture suite full pass; integration bins: extension_delivery 21/0 (Postgres legs under colima), delivery_user_journeys 22/0, mcp 22/0, trace_capture 14/0, generated_gate_sequences 29/0, group_journeys 16/0, group_multiuser 14/0. Workspace cargo fmt applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * docs(changelog): record the telegram_allowed_channels admission field Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7eyxusG6UGBXBfJ8BnQDy * fix(merge): reconcile composition ceilings and capability_wiring test arity Post-merge fixups after folding main (nearai#7157 squashed + nearai#7214 + the inspector prompt-diagnostic work) into run-acts-as-invoker: - Re-capture the composition absolute-mass ceiling 40_747 -> 40_811 in both the budget manifest and reborn_restructure_baselines.rs: the acting-user scope helper and shared-admission wiring add +64 production LOC on the merged tree. Recorded rather than parked in the 150-line tolerance. - Add the 10th `tool_diagnostic_sink` argument (None) to the invoker's capability_wiring test call — main grew the signature after this branch wrote that call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(conversations): refuse legacy-row route-kind mismatches; drop unreachable widen A Direct request is the one key shape a retained legacy conversation-scoped shared row can collide with. Resolve, lookup, reset, and link now refuse the mismatch outright (BindingRequired) instead of trusting adapters never to re-classify a conversation's route kind — pinned by a Direct-probe leg on the legacy restart-path test. The forward half of the migration contract is pinned too: per_actor_shared_bindings_keep_their_threads_across_reopen proves a new per-actor shared binding survives a restart (a deserialize-side regression would previously have orphaned every group thread silently). widen_binding_route_access and ReplyRouteAccess::allow_shared are deleted: every Shared-keyed row is born shared under per-actor keying, so both widen call sites were unreachable. The persisted flag stays for legacy reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(delivery): key gate notices by gate ref on the triggered lane too The gate-collapse fix shipped on the observer lane only; the background lane still minted undiscriminated projection ids for ApprovalNeeded/AuthRequired, so an automation run parking on a SECOND gate deduped to AlreadyDelivered, recorded the whole delivery Failed, and the gate was never announced or reply-routable (AGENTS.md: fix the sibling when a pattern bug is fixed). TriggeredNotification's discriminator now carries the gate ref for gate prompts (RunBlocked stand-ins compose their label with it), matching the observer keying, with a triggered two-gate regression pinning outcome, prompts, and both reply routes. Also pinned: same-gate re-announcement dedupe (g1->g2->g1), two distinct AUTH gates, and the refless id shapes incl. FinalReplyReady. Over-long discriminators are bounded with a stable FNV-1a suffix so a maximal legal TurnGateRef can never overflow ProjectionUpdateRef and silently lose a notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(identity): one contract derivation for every acting-identity scope LoopRunContext::acting_resource_scope joins acting_user_id on the contract type: the raise/resume scope recipe both gate-dance crates hand-synced is now declared once, and every surviving ladder delegates — composition's owner-first resource_scope_for_run (workspace/skill mounts) and the inline grant-minting copy, loop_host's synthetic resume load, and project_create_capability's effective_user_id (deleted; its doc claimed a mirror that no longer existed). On the only run shape where owner and actor differ — legacy runs parked across the deploy — mounts and grants now follow the ACTOR like the rest of the dance; the pin flip is recorded in visible_capability_request_uses_acting_user_for_runtime_scope. The ladder is unit-pinned in its owning crate (all three rungs) and the accepted deploy-boundary resume-miss is pinned on the synthetic port with an acting-scope positive control. loop_contracts ceiling re-captured 13094 -> 13107 with provenance (the +13-line contract method). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(extension-host): collapse admission handles; operator-identity channels never admit ChannelConfigSharedAdmission now holds the one declared *_allowed_channels handle as a plain String (the scan returns Option<String>): 'installed but handle-less' is no longer representable and the per-request Option branch is gone. The root pub use of the admission items is removed — consumers are crate-local and use the module path. Structural closure of the no-auth-vendor residual: a channel whose actor identity is not per-user (no OAuth vendor, no pairing strategy) never receives an admission resolver at all — an operator-identity channel that admitted a group would run every participant as the operator, the exact exposure run-acts-as-invoker removed. Previously this was unreachable only by manifest inventory. The extension_manager wire-shape pin gains the telegram_allowed_channels row (production projection was already correct), and extension_delivery gains the caller-path rejection leg: a correctly-signed webhook for an UNLISTED supergroup is acknowledged but produces no turn and no reply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test+docs(reborn): re-pin parity to per-actor scopes; align contract, docs, vocabulary The identity-parity bin now pins the run-acts-as-invoker property its fixture can actually express: the shared-room support binding keeps ONE thread (the per-actor thread model is locked at the integration tier by scenario_two_actors_own_threads), and inside that shared thread each RUN's scope is owned by its own invoking actor with identity context never crossing. The shared-admission suite gains the reset checkpoint leg (deny before rotation, thread survives), and the connect-nudge suite's shared leg is documented as the deliberate unpaired-participant silence contract. docs/reborn/contracts/conversation-binding.md (the owning contract) is amended: per-actor key in rule 8, participant widening retired in rule 14, subject ownership struck in rule 24, and the admission/retention semantics recorded. Operator docs and CHANGELOG state the real unpaired-shared behavior (silence; pairing via Extensions; DMs still nudge), the CHANGELOG gains the both-lanes gate-announcement entry and Added-first ordering, CHECKLIST's contradictory open-status is reconciled with a dated note, the new REBORN_WEBUI_V2_LIVE_QA_SLACK_ALLOWED_CHANNELS is threaded through live-canary.yml, observer.rs carries its arch-exempt annotation, and retired 'subject' vocabulary is renamed out of live test support and doc comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Linked Issue
Closes #7219
Part of #7218
Validation
cargo fmt --all -- --checkcargo test -p ironclaw_product_contractscargo test -p ironclaw_product inspector_store --libcargo clippy -p ironclaw_product_contracts --all-targets --all-features -- -D warningscargo clippy -p ironclaw_product --all-targets --all-features -- -D warningscargo test -p ironclaw_architecturebash scripts/pre-commit-safety.shSecurity Impact
Diagnostic content remains process-local, scope-isolated, size-bounded, and redacted from debug output. No API exposure, persistence, authorization changes, or execution-path wiring are introduced.
Database Impact
None.
Blast Radius
Limited to new diagnostic contracts and the in-memory store. Existing chat, event, execution, transport, and frontend behavior is unchanged.
Rollback Plan
Revert commit
8b4e40a59. No migration or persistent state cleanup is required.Review track: C