test(reborn): triggered Slack delivery skips terminal non-Completed runs - #5719
Conversation
RecoveryRequired/Failed/Cancelled fall through the catch-all in triggered_notification_for_state (slack_delivery.rs:2587), which deliver_triggered_run records as Skipped with no Slack egress. Intentional per #5713 (closed not-planned) — this pins the behavior, uncovered until now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted review state before this projection. |
github-actions
Bot
added
size: M
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new unit test in slack_delivery.rs verifying that triggered-run delivery for a terminal RecoveryRequired run state records a Skipped outcome and does not issue any Slack chat.postMessage egress calls. ChangesTerminal State Delivery Test
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
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 |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: e3fa3100b7eefc4e534e28cdd1657b710afedb24
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No blocking issues found. The PR only adds a focused regression test for triggered Slack delivery handling of terminal RecoveryRequired runs, and the expected Skipped/no-egress behavior matches the current delivery path.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloop review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloop reviewwhen the fix may affect multiple areas. - Use
@ironloop statusto check queued/running/completed/stale/stalled state while reviewers run.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new integration test, driver_terminal_recovery_required_run_records_skipped_without_egress, to verify that a terminal RecoveryRequired run is correctly recorded as Skipped and does not trigger any Slack egress messages. I have no feedback to provide as there are no review comments.
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.
Reborn integration-tier coverageLine coverage (Reborn crates): 85.31% — 273178 / 320230 lines Per-crate breakdown (65 crates, lowest-covered first)
This signal is informational: coverage never gates the PR — not the percentage, not the per-crate holes, not the 0-coverage callout. Exemptions (4 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-5719 environment in ironclaw-ci-preview
|
Summary
_ => Ok(None)arm intriggered_notification_for_state(crates/ironclaw_reborn_composition/src/slack_delivery.rs:2467-2587):TurnStatus::RecoveryRequired/Failed/Cancelledare terminal but have no explicit match arm, so they fall through toOk(None).deliver_triggered_runtreats this as a normal skip (TriggeredRunDeliveryOutcomeKind::Skipped, no Slack message ever built). This is the intentional behavior tracked by Triggered/scheduled runs that terminate Failed deliver no Slack notification (silent automation failures) #5713 (closed not-planned) — previously untested.driver_terminal_recovery_required_run_records_skipped_without_egress, added next to its closest sibling (driver_wait_timeout_records_failed_without_egress), reusing the existing driver-test harness. Proved RED via a temporary mutation of the catch-all arm (routed it through aSome(...)delivery), confirmed the test failed for the right reason, then reverted before committing.Why crate-tier, not the integration harness (tier-fallback rationale)
Per the integration-first rule, crate-tier is the fallback only when the integration tier can't reach the path — this is such a case, stated explicitly:
TriggeredRunDeliveryDriver::on_trigger_submitted(slack_delivery.rs:1925) dispatches the delivery via a detachedtokio::spawn(slack_delivery.rs:1970) — fire-and-forget, so a harness driving the public path cannot deterministically capture theTriggeredRunDeliveryOutcomeKind. There is no outcome-capture injection seam on this driver (the delivery/binding sinks are constructed inline as Noop in the production factory), so theSkipped-without-egress result cannot be asserted fromtests/integration/. This is the same C-TRIGGERED-DELIVERY services-shell blocker already tracked on the coverage roadmap;E-OUTBOUND'sRecordingOutboundDeliverySinkcovers the final-reply outbound path, a different code path that does not see this driver.deliver_triggered_run(slack_delivery.rs:2033) directly and inspects its returned outcome — the same tier and pattern as the pre-existing sibling driver tests in this file. An int-tier version becomes possible only once the services-shell delivery-outcome seam exists (the same convergence work tracked for gate-dispatch in Reborn harness: real gate-dispatch (submit_inbound Approval/AuthResolution) unreachable at int tier — interaction services read a disjoint turn-run store #5722).Production call site (for reachability, not observability):
deliver_triggered_run←on_trigger_submitted(slack_delivery.rs:1925) ←trigger_poller.rs:385,407andslack_host_beta/runtime_setup.rs:662— real composition-root wiring, compiled into the int-tier binary since #5656.Test plan
cargo fmt --all --checkcargo clippy --all --benches --tests --examples --all-features— 0 warningscargo test -p ironclaw_reborn_composition --lib --features slack-v2-host-beta slack_delivery::— 77 passed, 0 failed🤖 Generated with Claude Code