Expose outbound delivery targets to Reborn model

[serrrfirat]

Summary

• build on [codex] Represent Slack as a product-adapter extension #4778's Slack-as-product-adapter extension surface by adding the runtime/model layer for outbound delivery target selection
• add local-dev model-visible capabilities to list outbound delivery targets and set the caller's default final-reply target, with the setter going through the normal approval path outside local-dev-yolo
• add a mutable Reborn runtime outbound target registry so post-build product adapters can register providers after runtime assembly
• register Slack host-beta DM/shared-channel targets into that registry and use the same caller-scoped provider path from WebUI outbound preferences and model capabilities
• persist triggered-run delivery outcomes through the local-dev outbound store so scheduled runs can deliver final replies to the configured target
• address review feedback around provider error propagation, caller-sensitive target providers, approval consumption before preference persistence, and hiding provider tools when the outbound facade is absent

Stack / Reviewer Notes

• [codex] Represent Slack as a product-adapter extension #4778 is merged and owns the Slack extension manifest/channel UI surface. This PR is the next layer: model/runtime selection of where final replies should be delivered.
• The Slack extension card can still show No capabilities; Slack is a channel/product adapter, while the model-visible outbound target tools are synthetic runtime built-ins.
• local-dev-yolo intentionally bypasses approval gates. Use non-yolo local-dev to exercise the approval prompt for changing the final-reply target.
• Manual local testing exposed a separate local-dev SSO caveat: if a signed SSO session is reused after restart, local trigger access may not be reseeded for that user until re-login. Fresh OAuth login seeds it correctly; follow-up should seed local trigger access on successful session authentication as well.

Manual Slack E2E

• Created a real Slack app from manifest, pointed Events API at local Reborn through ngrok, and paired a personal Slack DM target.
• Created a scheduled automation: every 5 minutes, GET https://api.github.com with a User-Agent header and send the result as the final reply to Slack.
• After seeding the local-dev trigger access row for the active SSO user, the trigger fired and delivered GitHub API check: HTTP 200 OK to the Slack DM.
• Persisted evidence from the local run:
  • trigger_run_history.status = ok
  • triggered-run delivery outcome delivered
  • outbound delivery status delivered
  • delivered candidate kind final_reply

Verification

• cargo check -p ironclaw_reborn_composition --tests
• cargo clippy -p ironclaw_reborn_composition --tests -- -D warnings
• targeted outbound/local-dev regression tests for provider errors, caller-scoped targets, approval/persistence ordering, and facade-absent tool hiding
• bash scripts/pre-commit-safety.sh
• GitHub checks on Expose outbound delivery targets to Reborn model #4779 are green: Code Style, Reborn E2E, Tests (Reborn), Tests (Legacy), and CodeRabbit

[gemini-code-assist]

Code Review

This pull request introduces outbound delivery target capabilities for local development, adding a MutableOutboundDeliveryTargetRegistry to dynamically register target providers and exposing synthetic capabilities to list and set delivery targets. It also refactors Slack integration to register its provider dynamically. The review feedback suggests simplifying mapping closures and match arms in runtime.rs for cleaner code, and correcting the return value of register_provider in outbound_preferences.rs to avoid misleading callers when overwriting an existing provider.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[henrypark133]

Reviewed draft PR #4779 at 038903ba7ea303c77e8014ea0bedf9b7b34b9f58.

Existing live threads are Gemini style/nit comments, so I did not duplicate them. I found four current behavioral issues below.

Verification:

• PASS: cargo test -p ironclaw_reborn_composition local_dev_outbound_delivery_capabilities_use_late_registered_targets -- --nocapture
• PASS: cargo test -p ironclaw_reborn_composition --features slack-v2-host-beta slack_host_beta_targets_wire_through_outbound_preferences_facade -- --nocapture

[henrypark133]

Code Review (re-review)

Re-reviewed draft PR #4779 at 038903ba7ea303c77e8014ea0bedf9b7b34b9f58 against the live unresolved threads.

The four previous Henry threads are still current and not outdated, so I did not duplicate them:

1. model-controlled final_reply_target_id durable mutation needs an approval/ephemeral boundary
2. local-dev exposes outbound delivery tools even when the provider registry is empty
3. runtime-global Slack provider registration leaks targets into WebUI bundles without explicit Slack opt-in
4. Slack mount construction ignores registry registration failure/replacement

This pass found one additional distinct validation issue inline. I also noted the registry/lifecycle/order concerns, but they are mostly consequences of the same runtime-global provider side effect already covered by the existing WebUI/Slack threads.

Verification:

• PASS: cargo test -p ironclaw_reborn_composition local_dev_outbound_delivery_capabilities_use_late_registered_targets -- --nocapture
• PASS: cargo test -p ironclaw_reborn_composition --features slack-v2-host-beta slack_host_beta_targets_wire_through_outbound_preferences_facade -- --nocapture

[henrypark133]

Code Review (multi-agent)

Intent: Expose outbound delivery targets to the Reborn model via local-dev capabilities, runtime registry, Slack registration, and WebUI preference reuse.

Stats: 5 findings (from 11 raw, 5 after filter/dedup) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0.

Bugs

1. Medium List tool accepts malformed JSON as valid input (crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs:243-257, confidence 93) — anchor: crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs:243
parse_optional_channel treats non-object inputs as if no channel was supplied, so malformed list-tool invocations can run despite the declared object schema.

Tests

1. Medium Cover malformed outbound target IDs (crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs:260-272, confidence 83) — anchor: crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs:260
   The invalid-format branch of parse_target_id is not covered, so malformed model input can regress silently.
2. Medium Test explicit-owner precedence in caller selection (crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs:191-213, confidence 79) — anchor: crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs:191
   The capability test covers the actor-only path but not the branch where explicit_owner_user_id must override the actor.

Approach

1. Medium Runtime mutation API cuts across the WebUI bundle boundary (crates/ironclaw_reborn_composition/src/runtime.rs:1030-1054, confidence 80) — anchor: crates/ironclaw_reborn_composition/CLAUDE.md:3-6
   The post-build runtime registration hook makes RebornRuntime a mutable handoff point for a Slack/WebUI dependency that already has an explicit provider-list seam.

Local Patterns

1. Low Registration warning omits the provider key (crates/ironclaw_reborn_composition/src/outbound_preferences.rs:121-124, confidence 74) — anchor: crates/ironclaw_reborn_composition/src/outbound_preferences.rs:121-124
   The poisoned-lock warning does not include the provider key, which weakens diagnostics if registration fails.

[coderabbitai]

📝 Walkthrough

Walkthrough

Threads optional EffectiveRuntimePolicy through local-dev composition structs. Adds local_dev_effects_require_approval and a TOML policy grant. Implements two synthetic local-dev capabilities (builtin.outbound_delivery_targets_list, builtin.outbound_delivery_target_set) with approval-resume orchestration. Conditionally appends a Slack manifest AdminEntry to the first-party trust policy under slack-v2-host-beta. Updates the trigger delivery contract.

Changes

Outbound Delivery Capability and Runtime Policy Wiring

Layer / File(s)	Summary
Runtime policy threading and Slack trust entry src/factory.rs	EffectiveRuntimePolicy added to RebornLocalRuntimeServices and RebornLocalDevStoreGraphInput, threaded through both store-graph builders. builtin_first_party_trust_policy refactored to mutable entries vector; under slack-v2-host-beta, appends Slack manifest AdminEntry at /system/extensions/slack/manifest.toml with digest from slack_manifest_digest(). Test verifies first-party trust only on exact path+digest match.
Approval policy helper and capability policy grant src/local_dev_authorization.rs, src/local_dev_capability_policy.toml	New local_dev_effects_require_approval helper resolves (ApprovalPolicy, RuntimeProfile) from optional runtime policy, builds RuntimeProfileApprovalGatePolicy, and returns .effects_require_approval(...). local_dev_authorizer refactored to share local_dev_approval_policy. TOML grant added for builtin.outbound_delivery_target_set with dispatch_capability+external_write under ambient mounts.
Outbound delivery wiring through local-dev stack src/runtime.rs, src/runtime/local_dev.rs, src/runtime/local_dev/refreshing_capability_port.rs, src/runtime/local_dev/synthetic_capability.rs, src/slack_host_beta.rs	capability_wiring gains optional OutboundPreferencesProductFacade param; clones approval/lease stores, computes approval-required flag, stores all in LocalDevLoopCapabilityPortFactory. New fields propagate through RefreshingLocalDevCapabilityPortConfig. invoke_capability gains resume-mode exclusivity guard and uses approval_resume.input directly when present. runtime.rs call site updated. Slack host-beta hoists provider into local before struct construction.
Outbound delivery list/set capability handlers src/runtime/local_dev/outbound_delivery.rs	New 704-line module implements builtin.outbound_delivery_targets_list (optional channel filter, caller derivation, facade call) and builtin.outbound_delivery_target_set (fingerprint-based approval-request creation on first call; fingerprint+correlation+lease claim on approval resume; facade set_outbound_preferences call). Includes strict JSON validators, scope derivation, resume token helpers, and error mappers.
Integration tests, harness updates, and delivery contract src/runtime/local_dev/tests.rs, src/runtime/local_dev/shell_tests.rs, docs/reborn/contracts/triggers.md	Adds StaticOutboundDeliveryTargetProvider test double; full approval-gate flow test asserting owner-preference persistence, actor non-persistence, lease consumption; LocalYolo bypass; hidden-when-no-facade assertions; harness updates setting outbound_preferences_facade: None. Trigger delivery contract updated to route target selection through outbound delivery track.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(100, 149, 237, 0.5)
    Note over Agent, OutboundPreferencesProductFacade: builtin.outbound_delivery_target_set (approval-gated)
  end
  participant Agent
  participant set_handler as outbound_delivery.set
  participant ApprovalRequestStore
  participant CapabilityLeaseStore
  participant OutboundPreferencesProductFacade

  Agent->>set_handler: invoke(target_id)
  set_handler->>set_handler: compute fingerprint(scope/capability/input)
  set_handler->>ApprovalRequestStore: save pending dispatch approval + resume token
  set_handler-->>Agent: ApprovalRequired(resume_token)

  Agent->>set_handler: invoke(approval_resume=token)
  set_handler->>set_handler: verify replay input equality + recompute fingerprint
  set_handler->>ApprovalRequestStore: load + validate status/action/correlation
  set_handler->>CapabilityLeaseStore: find active lease for requester+fingerprint
  set_handler->>CapabilityLeaseStore: claim lease
  set_handler->>OutboundPreferencesProductFacade: set_outbound_preferences(final_reply_target_id)
  set_handler->>CapabilityLeaseStore: consume claimed lease
  set_handler-->>Agent: Completed

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• [Reborn] Extract local-dev capability composition out of runtime/composition hot paths #4922: This PR places outbound delivery synthetic capabilities inside local_dev/outbound_delivery.rs, which is exactly the architectural placement that issue #4922 proposes to extract into an owner module.
• [Reborn] Add local developer runtime profiles for host workspace and shell #3044: This PR implements the local_dev_effects_require_approval helper and threads EffectiveRuntimePolicy through local-dev construction, directly enabling the approval-preset and capability-authorization infrastructure required by #3044.

Possibly related PRs

• nearai/ironclaw#4778: Directly overlaps — both PRs modify builtin_first_party_trust_policy() in factory.rs and add slack-v2-host-beta-gated trust evaluation tests for Slack manifest identity.
• nearai/ironclaw#4769: Slack channel-delivery E2E tests in that PR exercise the final_reply_target_id outbound preference that this PR's builtin.outbound_delivery_target_set capability writes.

Poem

🦀 Two builtins rise from the local-dev floor,
targets_list to scout, target_set to implore.
A fingerprint guards the approval-gated gate,
Resume with a token — the lease seals your fate.
Slack earns first-party trust by path and by hash,
Runtime policy threaded through factory in a flash. ⚙️

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	Title is vague and doesn't follow Conventional Commits style; lacks type/scope prefix and details what's exposed rather than what changed.	Use Conventional Commits format: feat(local-dev): expose outbound delivery targets or similar, clarifying scope and change type.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	PR description comprehensively covers summary, change type, validation, security impact, trust-boundary checklist, blast radius, and stack notes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[serrrfirat]

CodeRabbit follow-up on the current head (6b4efabe8):

• Addressed the valid duplication nit: caller_for_run and resource_scope_for_run now share effective_user_id(...).
• Verified the registry/provider-registration comments are stale on this branch: MutableOutboundDeliveryTargetRegistry, register_provider(...), and the post-build RebornRuntime provider-registration hook no longer exist. Slack providers are passed as composition-time inputs into build_webui_services_with_connectable_channels(...).
• Verified the product-workflow list-path coverage already exists via list_projects_external_channel_surface_kind_through_extension_info; reran it locally.
• Did not remove builtin__outbound_delivery_target_set: this PR intentionally exposes the setter through the model path with an approval gate for default local-dev, and local-dev-yolo bypasses that gate by runtime policy.

[coderabbitai]

Actionable comments posted: 5

🤖 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_reborn_composition/src/local_dev_capability_policy.toml`:
- Around line 224-228: The grant block authorizing the
builtin.outbound_delivery_target_set capability with external_write and ambient
mounts is stale and should be removed entirely. Since the durable setter was
removed and delivery-target selection is now product-owned via WebUI, this grant
reopens an unauthorized LLM-controlled write path and violates fail-closed
security principles. Delete the entire grants block that contains capability =
"builtin.outbound_delivery_target_set" with its associated effects and mounts
entries.

In
`@crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs`:
- Around line 186-200: The approval lease is being consumed after the durable
outbound preference is set via the facade set_outbound_preferences method,
creating a state consistency issue where the durable preference persists even if
lease consumption fails. Per the coding guidelines to fail closed for approvals,
move the capability_leases.consume operation for the approved_lease BEFORE the
self.facade.set_outbound_preferences call, so that approval validation and lease
consumption happen atomically before committing any durable state changes. This
ensures that if lease consumption fails, the durable preference write never
occurs and state remains consistent.
- Around line 116-121: In
crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs at
lines 116-121 (anchor), 202-207 (sibling), 543-563 (sibling), and 590-599
(sibling), replace all `.map_err(|_| ...)` patterns that discard error
information with patterns that bind the error and preserve it. For each site,
change the closure from ignoring the error (|_|) to capturing it (|e|), then
include the error details in the AgentLoopHostError construction by using
something like error.to_string() or similar to carry the original cause
information into the error message, so that debugging information is not lost
when these serialization and parsing failures occur.

In `@crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs`:
- Around line 177-184: The StaticOutboundDeliveryTargetProvider implementation
currently ignores the _caller parameter in the list_outbound_delivery_targets
method, which means the test cannot verify that the handler is using the correct
caller scope. Modify the StaticOutboundDeliveryTargetProvider to record or
validate the caller parameter instead of discarding it. Store the expected
caller (the owner created in the test) in the provider, and only return the
outbound delivery target entry when the caller matches the expected owner. Then
add an assertion in the test to verify that the discovery operation used the
correct caller scope by checking that targets were retrieved only when queried
by the authorized owner.
- Around line 1641-1648: The test currently only validates that certain
capabilities are absent from the visible_capabilities surface through
descriptor_ids checks, but it should also verify that the corresponding provider
tools are absent from the tool_definitions() to ensure proper fail-closed
security behavior. Add assertions after the existing descriptor_ids checks that
verify builtin__outbound_delivery_targets_list and
builtin__outbound_delivery_target_set tool names are not present in the
surface's tool definitions, following the same pattern as the existing
capability ID assertions to confirm the missing-facade path completely hides
these provider tools from model exposure.

🪄 Autofix (Beta)

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: 7fb62aac-cb98-4542-bc36-0f5acc9cb559

📥 Commits

Reviewing files that changed from the base of the PR and between 6525f1f and c334a1f.

📒 Files selected for processing (12)

• crates/ironclaw_reborn_composition/src/factory.rs
• crates/ironclaw_reborn_composition/src/local_dev_authorization.rs
• crates/ironclaw_reborn_composition/src/local_dev_capability_policy.toml
• crates/ironclaw_reborn_composition/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/synthetic_capability.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs
• crates/ironclaw_reborn_composition/src/slack_host_beta.rs
• docs/reborn/contracts/triggers.md

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ironclaw_reborn_composition/src/factory.rs (1)

4901-4964: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add the missing-digest fail-closed case.

This test covers wrong digest/path, but not a matching Slack path with digest: None. For a digest-bound first-party entry, that identity must stay sandbox.

Proposed test addition

         assert_eq!(
             wrong_digest.provenance,
             ironclaw_trust::TrustProvenance::Default
         );
+
+        let missing_digest = ironclaw_trust::TrustPolicy::evaluate(
+            &policy,
+            &ironclaw_trust::TrustPolicyInput {
+                identity: slack_identity("/system/extensions/slack/manifest.toml", None),
+                requested_trust: ironclaw_host_api::RequestedTrustClass::FirstPartyRequested,
+                requested_authority: Default::default(),
+            },
+        )
+        .expect("missing digest slack identity should evaluate");
+
+        assert_eq!(missing_digest.effective_trust.class(), TrustClass::Sandbox);
+        assert_eq!(
+            missing_digest.provenance,
+            ironclaw_trust::TrustProvenance::Default
+        );
 
         let wrong_path = ironclaw_trust::TrustPolicy::evaluate(
             &policy,

As per coding guidelines, “Fail closed for auth, approvals, trust, filesystem containment, network policy, secret leases, runtime selection, and adapter identity.”

🤖 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_reborn_composition/src/factory.rs` around lines 4901 - 4964,
The test builtin_first_party_trust_policy_includes_slack_local_manifest_entry()
is missing a fail-closed test case for when the Slack manifest path is correct
but the digest is None (missing). Add a fourth test case following the same
pattern as wrong_digest and wrong_path that evaluates the trust policy with the
correct path "/system/extensions/slack/manifest.toml" but with None as the
digest parameter, then assert that it evaluates to TrustClass::Sandbox with
TrustProvenance::Default, ensuring the policy fails closed when digest is
missing.

Source: Coding guidelines

♻️ Duplicate comments (2)

crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs (2)

186-200: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Consume the approval lease before the durable preference write.

set_outbound_preferences persists final_reply_target_id, then consume() can fail and return an error after product state changed. Move lease consumption before Line 186, or add rollback. As per coding guidelines, “Fail closed for auth, approvals, trust, filesystem containment, network policy, secret leases, runtime selection, and adapter identity.”

Fail closed before committing preference

         let target_summary = target_id.as_str().to_string();
         let caller = caller_for_run(&invocation, &self.fallback_user_id);
+        if let Some(approved_lease) = approved_lease {
+            self.capability_leases
+                .consume(&approved_lease.scope, approved_lease.lease_id)
+                .await
+                .map_err(|error| approval_lease_error("consume_approval_lease", error))?;
+        }
         let response = self
             .facade
             .set_outbound_preferences(
                 caller,
@@
             .await
             .map_err(|error| outbound_delivery_host_error("set_target", error))?;
-        if let Some(approved_lease) = approved_lease {
-            self.capability_leases
-                .consume(&approved_lease.scope, approved_lease.lease_id)
-                .await
-                .map_err(|error| approval_lease_error("consume_approval_lease", error))?;
-        }

🤖 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_reborn_composition/src/runtime/local_dev/outbound_delivery.rs`
around lines 186 - 200, The approval lease consumption must happen before the
durable state write to satisfy fail-closed security principles. Move the lease
consumption block (the if let Some(approved_lease) conditional that calls
self.capability_leases.consume()) to execute before the set_outbound_preferences
call, so that if lease consumption fails, no durable preference state has been
written. This ensures the system fails closed for approval verification before
committing any preference changes.

Source: Coding guidelines

---

116-121: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Preserve the mapped error cause.

These .map_err(|_| ...) sites still discard the source failure, which violates Fail loud diagnostics for boundary/runtime errors. Bind the error and carry reason: error.to_string() or equivalent typed context into AgentLoopHostError. As per coding guidelines, “Do not use .map_err(|_| OtherError) — a closure that ignores its error binding and substitutes a generic error drops the underlying cause.”

#!/bin/bash
# Verify remaining source-dropping map_err closures in changed Rust files.
rg -n -C2 --type=rust '\.map_err\(\|_\|' crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs

Also applies to: 202-207, 535-540, 549-554, 562-567, 584-589

🤖 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_reborn_composition/src/runtime/local_dev/outbound_delivery.rs`
around lines 116 - 121, The `.map_err(|_| ...)` closure discards the original
error source, violating error diagnostics requirements. Fix this by binding the
error parameter instead of ignoring it with `|_|`, then pass the error details
to the AgentLoopHostError constructor via a reason field using
`error.to_string()` or equivalent context. This issue appears at multiple sites
throughout the file: the serialization failure at lines 116-121, and also at
lines 202-207, 535-540, 549-554, 562-567, and 584-589. Update all of these
`.map_err` closures to capture and preserve the actual error cause rather than
substituting a generic message that hides the underlying failure.

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.

Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/factory.rs`:
- Around line 4901-4964: The test
builtin_first_party_trust_policy_includes_slack_local_manifest_entry() is
missing a fail-closed test case for when the Slack manifest path is correct but
the digest is None (missing). Add a fourth test case following the same pattern
as wrong_digest and wrong_path that evaluates the trust policy with the correct
path "/system/extensions/slack/manifest.toml" but with None as the digest
parameter, then assert that it evaluates to TrustClass::Sandbox with
TrustProvenance::Default, ensuring the policy fails closed when digest is
missing.

---

Duplicate comments:
In
`@crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs`:
- Around line 186-200: The approval lease consumption must happen before the
durable state write to satisfy fail-closed security principles. Move the lease
consumption block (the if let Some(approved_lease) conditional that calls
self.capability_leases.consume()) to execute before the set_outbound_preferences
call, so that if lease consumption fails, no durable preference state has been
written. This ensures the system fails closed for approval verification before
committing any preference changes.
- Around line 116-121: The `.map_err(|_| ...)` closure discards the original
error source, violating error diagnostics requirements. Fix this by binding the
error parameter instead of ignoring it with `|_|`, then pass the error details
to the AgentLoopHostError constructor via a reason field using
`error.to_string()` or equivalent context. This issue appears at multiple sites
throughout the file: the serialization failure at lines 116-121, and also at
lines 202-207, 535-540, 549-554, 562-567, and 584-589. Update all of these
`.map_err` closures to capture and preserve the actual error cause rather than
substituting a generic message that hides the underlying failure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 583db78a-c36e-44a4-9d98-79e00e34a6b1

📥 Commits

Reviewing files that changed from the base of the PR and between c334a1f and 6b4efab.

📒 Files selected for processing (12)

• crates/ironclaw_reborn_composition/src/factory.rs
• crates/ironclaw_reborn_composition/src/local_dev_authorization.rs
• crates/ironclaw_reborn_composition/src/local_dev_capability_policy.toml
• crates/ironclaw_reborn_composition/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/synthetic_capability.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs
• crates/ironclaw_reborn_composition/src/slack_host_beta.rs
• docs/reborn/contracts/triggers.md

[coderabbitai]

Actionable comments posted: 1

🤖 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_reborn_composition/src/runtime/local_dev/outbound_delivery.rs`:
- Around line 333-349: The authorization check in the outbound_delivery.rs file
at lines 333–349 uses CapabilityLeaseStore::leases_for_scope() which silently
converts store errors to empty results, masking transient failures. To fix the
"fail loud" violation, you must either: (1) refactor the CapabilityLeaseStore
trait to return Result<Vec<CapabilityLease>, Error> instead of
Vec<CapabilityLease>, then propagate the error from leases_for_scope() with the
? operator in the approval lease lookup; or (2) keep the trait signature but add
explicit error logging in the trait implementation in
crates/ironclaw_authorization/src/lib.rs and modify the caller to return a
distinct InternalError or Unavailable response (not Unauthorized) when
leases_for_scope() indicates a store failure. Either approach ensures store
failures are surfaced loudly instead of being masked as missing leases.

🪄 Autofix (Beta)

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: 538a61ca-2e84-4c9f-9723-d113d37df7fe

📥 Commits

Reviewing files that changed from the base of the PR and between 6b4efab and 7b2f622.

📒 Files selected for processing (12)

• crates/ironclaw_reborn_composition/src/factory.rs
• crates/ironclaw_reborn_composition/src/local_dev_authorization.rs
• crates/ironclaw_reborn_composition/src/local_dev_capability_policy.toml
• crates/ironclaw_reborn_composition/src/runtime.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/refreshing_capability_port.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/synthetic_capability.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs
• crates/ironclaw_reborn_composition/src/slack_host_beta.rs
• docs/reborn/contracts/triggers.md

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check CapabilityLeaseStore trait signature for leases_for_scope
ast-grep --pattern $'trait CapabilityLeaseStore {
  $$$
  async fn leases_for_scope($$$) -> $RET;
  $$$
}'
# Fallback: grep for return type
rg -n 'fn leases_for_scope' --type rust -A 3

Repository: nearai/ironclaw

Length of output: 8028

---

Authorization gate silently masks store failures — fail-loud violation.

CapabilityLeaseStore::leases_for_scope() returns Vec<CapabilityLease> (not Result). The trait implementation in crates/ironclaw_authorization/src/lib.rs:931–934 explicitly catches store errors and returns Vec::new() instead of propagating them. This means a transient DB failure silently converts to "no leases found," causing the code at lines 333–349 to return Unauthorized instead of surfacing the actual Unavailable/InternalError. Violates "Fail loud" invariant.

The approval gate should either:

1. Change the trait to return Result<Vec<…>, …> and propagate errors with ?, or
2. Log and return a distinct error for store unavailability (not "lease is unavailable").

🤖 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_reborn_composition/src/runtime/local_dev/outbound_delivery.rs`
around lines 333 - 349, The authorization check in the outbound_delivery.rs file
at lines 333–349 uses CapabilityLeaseStore::leases_for_scope() which silently
converts store errors to empty results, masking transient failures. To fix the
"fail loud" violation, you must either: (1) refactor the CapabilityLeaseStore
trait to return Result<Vec<CapabilityLease>, Error> instead of
Vec<CapabilityLease>, then propagate the error from leases_for_scope() with the
? operator in the approval lease lookup; or (2) keep the trait signature but add
explicit error logging in the trait implementation in
crates/ironclaw_authorization/src/lib.rs and modify the caller to return a
distinct InternalError or Unavailable response (not Unauthorized) when
leases_for_scope() indicates a store failure. Either approach ensures store
failures are surfaced loudly instead of being masked as missing leases.