Repository navigation
[PRODUCTION CRATE — trait-object seam, zero behavior change] Real gate-dispatch harness convergence + triggered-delivery outcome proof - #5735
Conversation
…table Issue #5722: RebornIntegrationGroup's real runs live in a turn-state store disjoint from the harness's own local-dev composition, so the interaction services built from local_dev_{approval,auth}_interaction_service_for_test could never find the group's runs (WorkflowRejected{ScopeNotFound}). Adds a TurnRunSnapshotSource seam so the turn-run locator can read from a caller-supplied store instead of always deriving it from local_runtime.turn_state; production callers are unaffected (same value, now behind a trait object). Wires this into RebornIntegrationGroup via a new opt-in with_real_gate_dispatch_services() flag, and adds submit_approval_resolution/submit_auth_resolution so tests can drive the literal submit_inbound dispatch arm instead of the harness's direct TurnCoordinator::resume_turn shortcut. Also adds an integration-tier proof that build_triggered_run_delivery_hook assembles a working driver over a real local-dev RebornRuntime and records outcomes through the caller-supplied TriggeredRunDeliveryStore — the factory-construction path crate-tier tests don't cover (they construct the driver directly). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
✅ 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
scope: dependencies
|
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
WalkthroughAdds a shared turn-run snapshot seam, rewires approval/auth and active-run lookup consumers to use it, extends integration harness plumbing for real gate dispatch, and adds integration coverage for approval resolution and triggered-delivery outcome recording. ChangesTurn-run snapshot seam and integration dispatch coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DefaultProductWorkflow
participant ApprovalInteractionService
participant LocalDevApprovalTurnRunLocator
participant TurnRunSnapshotSource
Client->>DefaultProductWorkflow: submit_inbound(ApprovalResolution)
DefaultProductWorkflow->>ApprovalInteractionService: dispatch resolution
ApprovalInteractionService->>LocalDevApprovalTurnRunLocator: find active run for gate
LocalDevApprovalTurnRunLocator->>TurnRunSnapshotSource: turn_run_snapshot()
TurnRunSnapshotSource-->>LocalDevApprovalTurnRunLocator: TurnPersistenceSnapshot
LocalDevApprovalTurnRunLocator-->>ApprovalInteractionService: run reference
ApprovalInteractionService-->>DefaultProductWorkflow: resume/reject
DefaultProductWorkflow-->>Client: ProductInboundAck
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 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 |
![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: 6b55d78360b52b223ccf11e9dae31f23d89cfc20
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete correctness, security, or test-coverage issues found in the reviewed diff. The change adds test-support seams for gate-resolution integration coverage and new integration tests without changing production wiring paths beyond the trait abstraction wrappers.
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.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 6b55d78360b52b223ccf11e9dae31f23d89cfc20
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete correctness, security, or test-coverage issues found in the reviewed diff. The change adds test-support seams for gate-resolution integration coverage and new integration tests without changing production wiring paths beyond the trait abstraction wrappers.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@tests/integration/support/builder.rs`:
- Around line 744-762: `submit_auth_resolution` is still taking a raw `&str` for
the auth request reference even though the auth-flow already works with
`GateRef`; update the `submit_auth_resolution` method signature in `builder.rs`
to accept `&GateRef` and pass that through directly from
`submit_turn_until_auth_blocked`. Keep the string conversion only at the
`verified_auth_resolution_envelope` boundary, and adjust any call sites in this
harness to pass the `GateRef` value instead of a string.
In `@tests/integration/support/test_adapter.rs`:
- Around line 326-381: The two envelope builders duplicate the same
`ProtocolAuthEvidence::test_verified` and
`TrustedInboundContext::from_verified_evidence` setup; extract that shared
construction into a private helper near `verified_approval_resolution_envelope`
and `verified_auth_resolution_envelope` that returns the trusted envelope from a
provided payload. Then have both public methods only build their specific
`ProductInboundPayload` variant (`ApprovalResolution` vs `AuthResolution`) and
delegate to the helper.
🪄 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: cc83f3bc-57fe-460a-b638-122c09f63ad9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (18)
Cargo.tomlcrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/auth_interaction.rscrates/ironclaw_reborn_composition/src/runtime/test_support.rscrates/ironclaw_reborn_composition/src/runtime/turn_run_snapshot.rstests/integration/group_approvals/main.rstests/integration/group_approvals/scenario_submit_inbound_approval_resolution.rstests/integration/support/builder.rstests/integration/support/group.rstests/integration/support/group_options.rstests/integration/support/harness/mod.rstests/integration/support/harness/profiles/core_builtin.rstests/integration/support/harness/profiles/github.rstests/integration/support/harness/profiles/mock_mcp.rstests/integration/support/harness/profiles/qa_smoke.rstests/integration/support/harness/profiles/web_access.rstests/integration/support/test_adapter.rstests/integration/triggered_delivery_outcome.rs
|
🚅 Deployed to the ironclaw-pr-5735 environment in ironclaw-ci-preview
|
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add harness enablers for gate-dispatch coverage while preserving production behavior, plus a triggered-delivery outcome integration proof.
Stats: 3 findings (from 5 raw, 3 after dedup) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0
Tests
- Medium Auth gate-dispatch seam lacks caller-level coverage (
tests/integration/support/group.rs:1159-1169, confidence 100) - anchor: AGENTS.md:101
The opt-in workflow now wires both the approval and auth interaction services, and the PR addssubmit_auth_resolutionplus the verifiedAuthResolutionenvelope builder. The approvalsubmit_inboundpath has a real scenario, but the auth side is explicitly left unexercised, so a regression whereAuthResolutioncannot find the auth gate throughLocalDevAuthInteractionReadModelwould still pass.
Maintainability
- Medium Collapse the duplicate turn-snapshot source traits (
crates/ironclaw_reborn_composition/src/runtime/turn_run_snapshot.rs:23-24, confidence 75) - anchor: crates/ironclaw_reborn_composition/src/trigger_poller/active_run_lookup.rs:113
This addsTurnRunSnapshotSourcefor the approval/auth locators even thoughtrigger_polleralready hasTriggerTurnSnapshotSourceover the sameTurnPersistenceSnapshotoperation. Keeping two private traits plus two store impl sets means future store-shape or snapshot error changes must be mirrored in both places.
Local Patterns
- Low Module doc now misdescribes the approval scenarios (
tests/integration/group_approvals/main.rs:99-105, confidence 75) - anchor: tests/integration/group_approvals/main.rs:99
The file-level comment still says one sequential test drives every real-gate scenario throughapprove_gate/deny_gate, with onlyfailure_category_demaskedas an exception. This PR adds a secondreal_gate_dispatchtest whose point is resolving throughsubmit_inbound(ApprovalResolution), so the navigation docs now describe the file incorrectly.
| .ok_or( | ||
| "local-dev approval interaction service unavailable (harness has no local runtime)", | ||
| )?; | ||
| let auth_interaction_service = reborn_services |
There was a problem hiding this comment.
Medium - Auth gate-dispatch seam lacks caller-level coverage.
The opt-in workflow now wires both the approval and auth interaction services, and the PR adds submit_auth_resolution plus the verified AuthResolution envelope builder. The approval submit_inbound path has a real scenario, but the auth side is explicitly left unexercised, so a regression where AuthResolution cannot find the auth gate through LocalDevAuthInteractionReadModel would still pass.
Fix: Add a live_auth_and_approval-style integration scenario built with with_real_gate_dispatch_services that drives RebornIntegrationHarness::submit_auth_resolution through DefaultProductWorkflow, or leave the auth helper/wiring out until that fixture lands.
Also flagged by: conventions/Medium, maintainability/Low
There was a problem hiding this comment.
Confirmed and tracked in #5744: manual-token auth gates never create an AuthFlowRecord, so submit_inbound(AuthResolution) is structurally unreachable against every int-tier harness profile today (only OAuth-gated capabilities create one). Deferring the real-dispatch auth arm to #5744's OAuth-gated-capability profile enabler rather than building a scenario that fails for the wrong reason.
| use ironclaw_turns::{TurnError, TurnPersistenceSnapshot}; | ||
|
|
||
| #[async_trait] | ||
| pub(crate) trait TurnRunSnapshotSource: Send + Sync { |
There was a problem hiding this comment.
Medium - Collapse the duplicate turn-snapshot source traits.
This adds TurnRunSnapshotSource for the approval/auth locators even though trigger_poller already has TriggerTurnSnapshotSource over the same TurnPersistenceSnapshot operation. Keeping two private traits plus two store impl sets means future store-shape or snapshot error changes must be mirrored in both places.
Fix: Move one generic turn snapshot source abstraction to a shared composition module and have trigger poller, approval, and auth call it, mapping domain-specific errors at their boundaries.
There was a problem hiding this comment.
Collapsed: moved TurnRunSnapshotSource to a crate-root turn_run_snapshot module (sibling of runtime/trigger_poller, not nested under runtime) and switched trigger_poller's SnapshotActiveRunLookup to consume it directly, mapping TurnError -> TriggerError at its own boundary the same way the approval/auth locators already map to ProductWorkflowError. This also deleted the now-unnecessary LocalTriggerTurnSnapshotSource<S> wrapper struct and its two feature-gated blanket impls entirely — confirmed the cfg-gating on those was incidental copy-drift, not load-bearing (FilesystemTurnStateStore/InMemoryTurnStateStore are defined unconditionally in ironclaw_turns, same as this trait's own unconditional impls already proved). Net: one duplicate trait + one duplicate blanket-impl pair + one wrapper struct removed, single crate, all 6 trigger_poller unit tests plus the full composition-crate suite (908 tests) still pass.
| report.assert_all_passed(); | ||
| } | ||
|
|
||
| /// Proof-of-seam group — the harness mid-stack bypass that resolved |
There was a problem hiding this comment.
Low - Module doc now misdescribes the approval scenarios.
The file-level comment still says one sequential test drives every real-gate scenario through approve_gate/deny_gate, with only failure_category_demasked as an exception. This PR adds a second real_gate_dispatch test whose point is resolving through submit_inbound(ApprovalResolution), so the navigation docs now describe the file incorrectly.
Fix: Update the module doc to scope the approve_gate/deny_gate description to approvals_group_e2e/libSQL and mention approvals_group_real_gate_dispatch_e2e as the submit_inbound proof.
There was a problem hiding this comment.
Fixed — module doc now scopes the approve_gate/deny_gate description to approvals_group_e2e and documents approvals_group_real_gate_dispatch_e2e as the separate submit_inbound proof.
🗂️ Archived IronLoop Review: reviewerThis result is from an older PR head and is no longer the active review.
Archived summaryNo concrete blocking issues found in the reviewed diff. The production changes are narrowly scoped to sharing a turn-run snapshot abstraction across approval/auth interaction lookup and trigger active-run lookup, with added integration coverage for submit_inbound gate resolution and triggered delivery hook factory wiring. |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: af256fb08b2175562309d14a33d5a3899d425a4b
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete correctness, security, maintainability, or test-coverage issues found in the reviewed diff. The changes are scoped to test-support seams and integration coverage, with production behavior preserved through wrapper paths.
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.
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.34% — 273355 / 320308 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (4 entry/entries excluded from the accounting above)
|
- Collapse TurnRunSnapshotSource/TriggerTurnSnapshotSource into one shared trait at crate-root turn_run_snapshot module; trigger_poller's SnapshotActiveRunLookup now maps TurnError -> TriggerError at its own boundary instead of duplicating the trait + blanket impls + a wrapper struct that turned out unnecessary once both consumers share the type. - submit_auth_resolution now takes &GateRef (matching submit_approval_resolution), converting to &str only at the envelope boundary. - Extract a shared verified_resolution_envelope helper for the approval/auth submit_inbound envelope builders. - Fix group_approvals module doc to describe both tests now present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 36b8db02d03e14c5cc3433c280bb0fe47a50c945
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the PR diff. The changes keep production wiring behavior scoped while adding test-support seams and integration coverage for real gate-resolution and triggered-delivery factory paths.
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.
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_reborn_composition/src/trigger_poller/active_run_lookup.rs (1)
46-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBatch path double-wraps the snapshot error.
trigger_backend_error(error)already yields aTriggerError::Backend;.to_string()on it then feeds that rendered backend string back into anotherTriggerError::Backend, so the reason carries the backend prefix twice. The single-item path (Line 34) wraps once via.map_err(trigger_backend_error)?. Use the raw error string here so both paths produce identical reasons.The
reason.contains("snapshot failed")assertion at Line 376 still passes, which is why this divergence isn't caught.🐛 Wrap once
- let reason = trigger_backend_error(error).to_string(); + let reason = error.to_string();🤖 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/trigger_poller/active_run_lookup.rs` around lines 46 - 59, The batch error handling in active_run_lookup::lookup_snapshot is double-wrapping the snapshot failure by calling trigger_backend_error(error).to_string() and then embedding that rendered backend message inside another TriggerError::Backend. Update the Err(error) branch to use the raw snapshot error text directly, matching the single-item path that uses .map_err(trigger_backend_error) so both paths produce the same reason string.
🤖 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.rs`:
- Line 117: The `TurnRunSnapshotSource` re-export in `runtime.rs` is
unnecessarily public within the crate and creates an extra import path with no
callers. Change the `pub(crate) use` for `TurnRunSnapshotSource` to a private
`use` in `runtime.rs`, keeping the symbol available internally while removing
the unused re-export path.
---
Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/trigger_poller/active_run_lookup.rs`:
- Around line 46-59: The batch error handling in
active_run_lookup::lookup_snapshot is double-wrapping the snapshot failure by
calling trigger_backend_error(error).to_string() and then embedding that
rendered backend message inside another TriggerError::Backend. Update the
Err(error) branch to use the raw snapshot error text directly, matching the
single-item path that uses .map_err(trigger_backend_error) so both paths produce
the same reason string.
🪄 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: 8b82956b-c8cc-48c8-986d-fc746e11d8ac
📒 Files selected for processing (9)
crates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/auth_interaction.rscrates/ironclaw_reborn_composition/src/trigger_poller.rscrates/ironclaw_reborn_composition/src/trigger_poller/active_run_lookup.rscrates/ironclaw_reborn_composition/src/turn_run_snapshot.rstests/integration/group_approvals/main.rstests/integration/support/builder.rstests/integration/support/test_adapter.rs
No in-crate consumer imports it via the runtime module path; all of them (trigger_poller, auth_interaction, test_support via super::*) resolve it through crate::turn_run_snapshot directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: df2fdc7db71911f9229d2ffa0155bf2db2c7772f
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the reviewed diff. The production changes are narrowly scoped to sharing a turn-run snapshot abstraction across approval/auth interaction lookup and trigger active-run lookup, with added integration coverage for submit_inbound gate resolution and triggered delivery hook factory wiring.
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.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: c6c92c9b5d8f3bfddbc49bf78519a9fd303b27eb
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the PR. The changes are scoped to a shared turn-run snapshot abstraction plus test-support seams and integration coverage for real gate-dispatch and triggered-delivery outcome wiring.
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.
Summary
Two harness enablers for the gate-dispatch coverage lane.
E1 — turn-state convergence (production crate change).
crates/ironclaw_reborn_composition'sLocalDevApprovalTurnRunLocatorandLocalDevAuthInteractionReadModelhardcoded their turn-run locator to a concreteArc<LocalDevTurnStateStore>field.tests/integration'sRebornIntegrationGroupruns real turns against its ownshared.turn_store(built via a separatebuild_default_planned_runtimecomposition) — a store those locators had zero visibility into. Driving a realsubmit_inbound(ApprovalResolution)through the harness's interaction-service test accessors therefore always failed withWorkflowRejected { kind: ScopeNotFound }, even though the approval request was found (the request store is correctly shared; only the turn run state locator was not).Fix: added
runtime/turn_run_snapshot.rs, a small private trait (TurnRunSnapshotSource) abstractingpersistence_snapshot()over any turn-state store shape (blanket impl forFilesystemTurnStateStore<F>, one forInMemoryTurnStateStore). Both locators now holdArc<dyn TurnRunSnapshotSource>instead of the concrete type.build_local_dev_approval_interaction_service/build_webui_auth_interaction_servicebecame thin wrappers over new_with_turn_run_sourcevariants that accept the store explicitly — production callers (build_reborn_runtime) still passArc::clone(&local_runtime.turn_state), unchanged behavior, now going through a trait-object cast instead of a concrete type. This also deleted two copies of duplicated#[cfg(...)]-gated snapshot-reading logic (one per locator), collapsing them into the one shared trait.Two new
test-support-gated methods onRebornServices(local_dev_{approval,auth}_interaction_service_with_turn_state_for_test) let a caller supply an alternate store.RebornIntegrationGroupgained an opt-in.with_real_gate_dispatch_services()flag; when set,RebornThreadBuilder::build()wires the harness's real interaction services over the group's ownshared.turn_storeinto the thread'sDefaultProductWorkflow— every other group's workflow keeps the defaultRejecting*InteractionServicestubs, byte-identical to today.RebornTestIngressgainedverified_approval_resolution_envelope/verified_auth_resolution_envelope(built directly —ApprovalResolution/AuthResolutionhave no wire format in the test adapter's JSON enum).RebornIntegrationHarnessgainedsubmit_approval_resolution/submit_auth_resolution, which build an envelope and callself.workflow.submit_inbound(envelope)— the literal dispatch arm a real adapter's "approve"/"deny" reply hits, as opposed toapprove_gate/deny_gate's directTurnCoordinator::resume_turnshortcut.Proof test:
group_approvals/scenario_submit_inbound_approval_resolution.rs(approve + deny arms), driven from a newapprovals_group_real_gate_dispatch_e2etest ingroup_approvals/main.rs. Asserts at a seam beyondwait_for_status(Completed):assert_workspace_file_contains/assert_workspace_file_absenton the real filesystem effect of the (re-)dispatched capability.Mutation-verified: I temporarily re-pointed the wiring at the pre-fix zero-arg accessors (which read the harness's own disjoint
local_runtime.turn_state), re-ran the test, and confirmed it fails with the exactScopeNotFoundfrom the root-cause investigation, then reverted. Tail:After reverting to the fix, both scenarios pass.
Auth-side (
submit_auth_resolution) plumbing is included and symmetric, but is not exercised end-to-end in this PR: driving a realAuthResolutionneeds a gate backed by a genuineAuthFlowRecord(an OAuth-style flow), whichlive_approvals's file-tool fixture doesn't raise — that's a materially different fixture than this PR's scope. Flagging as a natural follow-up once alive_auth_and_approval-shaped.with_real_gate_dispatch_services()scenario is written.E2 decision — triggered-delivery outcome seam.
Per the roadmap's C-TRIGGERED-DELIVERY row, and per the standing instruction that a user-flow coverage gap at int tier is a real signal to close, not wave off: investigated whether a triggered-run Slack-delivery outcome (
TriggeredRunDeliveryOutcomeKind) can be made observable at integration tier, today only covered at crate tier (ironclaw_reborn_composition::slack_delivery's#[cfg(test)]module).Evidence:
build_triggered_run_delivery_hook(&runtime, &config, delivery_store: Arc<dyn TriggeredRunDeliveryStore>)(slack_host_beta.rs:518) takes the delivery store as a public, caller-supplied parameter.TriggeredRunDeliveryStore+ publicInMemoryTriggeredRunDeliveryStorelive inironclaw_outbound.Skippedvia pending-queue exhaustion,Deniedvia project scope) by constructingTriggeredRunDeliveryDriverdirectly (::new) — never through the composition factory a real host binds.tests/integration'sRebornIntegrationGroupbuilds its ownbuild_default_planned_runtimecomposition, not theRebornRuntimethe Slack delivery hook needs (SlackHostBetaRuntimeParts::from_runtime) — the same "two disjoint runtime compositions" shape E1 above fixes for gate dispatch. A sibling test (slack_pairing_actor_resolution.rs) documents this exact gap for a related Slack feature and works around it by driving the real component directly rather than the full group harness.Verdict: implemented, small and non-duplicate.
tests/integration/triggered_delivery_outcome.rsbuilds a real local-devRebornRuntime(the same recipewiring_parity.rs's smoke test already uses), calls the unmodified publicbuild_triggered_run_delivery_hookfactory with an injectedInMemoryTriggeredRunDeliveryStore, drives a project-scopedTriggerFirethrough the realPostSubmitDeliveryHook::on_trigger_submitted, and assertsDeniedis recorded through the exact store this test supplied — proving the factory construction path over a realRebornRuntime, which crate tier's driver-direct tests do not cover. Needed one new dev-dependency (ironclaw_outbound, for the public in-memory store) and one new[[test]]entry, matching the project's existing one-file-per-concern integration test convention (trace_capture.rs,budget.rs, etc.).Does not drive a live trigger-poller fire end-to-end (pairing + seeding a due
TriggerRecord+ polling for the poller to claim it) — that full path is already proven at crate tier (build_slack_host_beta_mounts_wires_trigger_delivery_hook_writes_record) and would require materially more int-tier harness build-out (real Slack egress fakes, poller wiring) for marginal additional value over this PR's factory-construction proof.Mutation-verified: swapped in a fresh, unconnected
InMemoryTriggeredRunDeliveryStorefor the read (instead of the one actually injected into the factory) and confirmed the assertion fails to find a record, then reverted.Quality gates
cargo fmt— cleancargo clippy --all --benches --tests --examples --all-features— clean (0 warnings beyond a pre-existing, unrelatednet.retriesconfig-key notice)cargo test -p ironclaw_reborn_composition --lib/--test runtime— all pass in isolation; a subset of unrelated pre-existing tests intermittently hitRunTimeoutonly under heavy parallel load (defaultcargo testthread-per-test concurrency against 1396+ tests each spinning up a real tokio runtime) — every such test passes cleanly alone or at--test-threads=1/4, confirming this is pre-existing environment flakiness, not a regression from this diff.cargo test --test reborn_group_approvals --test reborn_integration_triggered_delivery_outcome --test reborn_group_extensions --test reborn_group_multiuser --test reborn_group_skills --features integration— all green, repeated runs.budget,trace_capture,tool_disclosure). One optional, non-blocking finding:build_local_dev_approval_interaction_service/build_webui_auth_interaction_servicecould be deleted entirely in favor of calling their_with_turn_run_sourcetwins directly frombuild_reborn_runtime's two call sites — left as-is since it's a pure style choice with no behavior or maintainability cost either way.Closes #5722.
🤖 Generated with Claude Code