Make missing outbound delivery targets recoverable

[serrrfirat]

Summary

• Return a model-visible CapabilityOutcome::Failed when builtin.outbound_delivery_target_set is given a target id that no connected provider can resolve.
• Keep genuine backend/auth/service failures on the existing host-error path.
• Add caller-level local-dev coverage proving the missing-target case is non-terminal and does not persist an outbound preference.

Verification

• cargo fmt --package ironclaw_reborn_composition
• RUSTC=$(rustup which --toolchain 1.96.0-aarch64-apple-darwin rustc) $(rustup which --toolchain 1.96.0-aarch64-apple-darwin cargo) test -p ironclaw_reborn_composition local_dev_yolo_outbound_delivery_target_set_bypasses_approval_gate -- --nocapture

Note: plain cargo test ... initially used Homebrew Rust 1.93 and failed before compilation because the workspace requires Rust 1.96; reran successfully with the installed rustup 1.96 toolchain.

[gemini-code-assist]

Code Review

This pull request updates the outbound delivery target handler to gracefully handle NotFound errors by returning a failed capability outcome with an InvalidInput error kind instead of throwing a host error. Additionally, a test case has been added to verify this new error handling behavior. There are no review comments, so I have no further feedback to provide.

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.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved handling when an outbound delivery target is missing, returning a clearer invalid input failure message (“outbound delivery target is not available”).
  • Ensured failed target-setup attempts do not persist the owner communication preference.
  • Improved reliability of the approval/resume flow so failures are handled consistently.
• Tests
  • Expanded local-dev outbound-delivery “set target” coverage with new missing-target scenarios, validating the expected failure kind and message as well as unchanged communication preferences.

Walkthrough

OutboundDeliveryTargetSetHandler::invoke now maps missing outbound delivery targets to CapabilityOutcome::Failed with InvalidInput, and local-dev tests cover the approval-gated and ungated missing-target cases.

Changes

Outbound Delivery Target NotFound Handling

Layer / File(s)	Summary
Handler: special-case NotFound as InvalidInput crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs	set_outbound_delivery_target_for_model now matches NotFound to CapabilityOutcome::Failed(InvalidInput, "outbound delivery target is not available"); other errors still use outbound_delivery_host_error. Lease consumption now runs after response resolution.
Tests: missing-target failure coverage crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs	Provider-backed and yolo local-dev tests add missing-target invocations, assert InvalidInput with the expected safe summary, confirm the owner preference stays unset, and check the approval lease remains Claimed in the gated flow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• [Reborn] Extract local-dev capability composition out of runtime/composition hot paths #4922: Same local-dev outbound-delivery set-target path and matching test surface.

Possibly related PRs

• nearai/ironclaw#4779: Directly touches OutboundDeliveryTargetSetHandler::invoke and the outbound-delivery target set flow refined here.

Poem

A target vanished, but not a trace,
So InvalidInput took its place.
The lease stood still, the prefs stayed bare,
While tests confirmed the missing-target air.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	Summary and verification are present, but most template sections are missing, including change type, linked issue, security, blast radius, rollback, and review track.	Add the missing template sections and fill in the required repo-specific checklists, linked issue, security impact, blast radius, rollback plan, and review track.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is clearly related to the change, but it does not use the preferred Conventional Commits format.
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.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[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 210-226: The `set_outbound_delivery_target_for_model` flow in
`outbound_delivery.rs` consumes the approval lease before handling the
`NotFound` branch, so the recoverable `InvalidInput` path still burns a one-shot
approval. Move lease consumption out of the early section around
`target_input`/`caller` handling and only consume it after a successful target
set, or add a pre-validation step before the lease is taken; keep the
`Err(error) if error.code == RebornServicesErrorCode::NotFound` handling in
`set_outbound_delivery_target_for_model` so it can return
`CapabilityOutcome::Failed` without spending the lease.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f1e1304f-cd2a-4f9f-9101-bbe414768809

📥 Commits

Reviewing files that changed from the base of the PR and between 04fa25a and 4b798f2.

📒 Files selected for processing (2)

• crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs

[railway-app]

🚅 Deployed to the ironclaw-pr-5445 environment in ironclaw-ci-preview

Service	Status	Web	Updated (UTC)
ironclaw	✅ Success (View Logs)	Web	Jun 30, 2026 at 11:53 am

[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/runtime/local_dev/outbound_delivery.rs (1)

204-225: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

This still is not recoverable in approval-gated runs.

Leaving the lease Claimed here only preserves it on the original approval scope/invocation. A corrected retry creates a fresh invocation and goes back through ApprovalRequired anyway, so the model still cannot recover from a bad target_id without another approval cycle. The new caller-level test in crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs proves that: after the missing-target failure, the next valid set flow still needs a new approval path. You’ve converted NotFound into a model-visible failure, but not into a truly recoverable one.

🤖 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 204 - 225, The approval-gated retry path is still not recoverable
because the approved lease remains Claimed after a NotFound target lookup, so a
fresh invocation still re-enters ApprovalRequired. Update the recovery flow in
outbound_delivery.rs around set_outbound_delivery_target_for_model and the
approved_lease handling so a missing target failure fully releases or
invalidates the current approval lease, allowing the next retry to proceed
without another approval cycle. Use the existing helpers approval_lease_error
and capability_leases.consume as the starting point, and align the behavior with
the caller-level retry test in tests.rs.

Source: Path instructions

🤖 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/runtime/local_dev/outbound_delivery.rs`:
- Around line 204-225: The approval-gated retry path is still not recoverable
because the approved lease remains Claimed after a NotFound target lookup, so a
fresh invocation still re-enters ApprovalRequired. Update the recovery flow in
outbound_delivery.rs around set_outbound_delivery_target_for_model and the
approved_lease handling so a missing target failure fully releases or
invalidates the current approval lease, allowing the next retry to proceed
without another approval cycle. Use the existing helpers approval_lease_error
and capability_leases.consume as the starting point, and align the behavior with
the caller-level retry test in tests.rs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 566279e4-29a1-4b36-884a-2c462369bbcd

📥 Commits

Reviewing files that changed from the base of the PR and between 4b798f2 and 0ea2d49.

📒 Files selected for processing (2)

• crates/ironclaw_reborn_composition/src/runtime/local_dev/outbound_delivery.rs
• crates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs