Repository navigation
test(reborn): W6-API2 — webui_v2 mid-gate approval refresh over real gate dispatch - #5743
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>
…gate dispatch A browser refresh mid-gate must let the user rediscover and resolve the pending gate. Mounts the real webui_v2 router over a hand-built RebornServices facade wired with the harness's own turn-state-converged ApprovalInteractionService (the same seam with_real_gate_dispatch_services uses for DefaultProductWorkflow); a fresh stream_events drain from after_cursor: None (the SSE handler is a polling wrapper over the same drain) surfaces the persisted GatePrompt event, and resolution goes through the real WEBUI_V2_PATTERN_RESOLVE_GATE HTTP route, not a direct-resume test shortcut. Two other legs of this seam (auth-gate mid-gate refresh, and a real AuthFlowRecord-backed submit_auth_resolution proof) turned out to be unreachable with the current int-tier harness: manual-token auth gates (GitHub, the only auth-gated capability any harness profile wires) never create an AuthFlowRecord at all — only Google-OAuth-gated capabilities do (oauth_gate.rs), and no harness profile wires one. Reaching either needs a new OAuth-gated capability profile, tracked as a follow-up rather than fiction-wired around here. 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:
Configuration errorMessage: Trusted agent config is invalid: Invalid type: Expected Object but received true
Available commands
Run metadataOrigin: |
|
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 new integration test that simulates a browser refresh during an active approval gate: it drains the event stream from a fresh cursor, locates the pending GatePrompt, resolves the gate through the real webui_v2 router endpoint, waits for turn completion, and verifies the resulting workspace file. ChangesApproval gate rediscovery and resolution test
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Reviewer note: No sandbox/trust/secrets/egress surface touched here — this is a test-only file wiring against existing RebornServices/router seams. Verify the test actually asserts on 🚥 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 |
github-actions
Bot
added
scope: dependencies
![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: 517733762cc2c121cb66df1d820374114c118ed0
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found. The PR adds a focused integration test for rediscovering and resolving a WebUI v2 approval gate after a cold event-stream replay, and registers it in Cargo.toml.
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.
henrypark133
left a comment
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Add integration coverage for WebUI v2 approval gate rediscovery and resolution after browser refresh using real gate dispatch.
Stats: 1 finding (from 1 raw, 1 after dedup) across 1 file. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, approach. Reviewers failed: none. Body-only: 0
Maintainability
- Low Keep the gate-refresh case in the WebUI product API bin (
Cargo.toml:557-559, confidence 75) — anchor: tests/integration/CLAUDE.md:65
This registers a new one-test integration binary for a WebUI v2 product-API scenario that already has an owning test file. That splits the same hand-built RebornServices/WebUI facade setup across bins and makes future WebUI product API coverage harder to scan or consolidate.
| name = "reborn_integration_web_access" | ||
| path = "tests/integration/web_access.rs" | ||
|
|
||
| [[test]] |
There was a problem hiding this comment.
Low — Keep the gate-refresh case in the WebUI product API bin.
This registers a new one-test integration binary for a WebUI v2 product-API scenario that already has an owning test file. That splits the same hand-built RebornServices/WebUI facade setup across bins and makes future WebUI product API coverage harder to scan or consolidate.
Fix: Move approval_gate_rediscovered_and_resolved_after_refresh into tests/integration/webui_v2_product_api.rs and delete this [[test]] entry. Keep any setup local there unless another scenario needs the exact same bundle.
There was a problem hiding this comment.
Moved approval_gate_rediscovered_and_resolved_after_refresh into tests/integration/webui_v2_product_api.rs (test content unchanged) and dropped the reborn_integration_webui_v2_gate_refresh [[test]] entry. All imports it needed were already present in the product-API bin except TurnStatus. Full binary now runs 11/11 green.
|
🚅 Deployed to the ironclaw-pr-5743 environment in ironclaw-ci-preview
|
🗂️ Archived IronLoop Review: reviewerThis result is from an older PR head and is no longer the active review.
Archived summaryNo concrete blocking issues found. The PR adds a focused integration test for rediscovering and resolving a pending approval gate after a WebUI refresh. |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 83812002a7068507aa31d394ebf26698cb8298c6
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No blocking issues found. The PR adds a focused integration test for rediscovering and resolving a WebUI v2 approval gate after a cold event stream refresh, and registers it in Cargo.toml.
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.
- 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>
…_api bin Move approval_gate_rediscovered_and_resolved_after_refresh into the existing W5-WEBUI-API-1 product-API test binary instead of registering a second one-test bin for the same hand-built RebornServices/WebUI facade setup. Test content is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
risk: low
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 9bf3419d98bbeb2ba1bfe2b9d1441d1e9896479d
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete correctness, security, maintainability, or test-coverage issues found in the PR. The change adds a focused integration test for rediscovering and resolving a pending WebUI approval gate after a refresh.
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.
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: 71e088ccb6e306cd18cd3cbedf96e85ca826a623
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found. The PR adds a focused integration test for rediscovering and resolving a pending approval gate after a WebUI refresh.
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: 5c2bf6603b0a05168e23f4a7b353b1c847d564bb
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Reviewed the test-only change adding WebUI v2 approval gate refresh coverage. I did not find concrete correctness, security, or maintainability issues introduced by the PR.
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.38% — 273556 / 320385 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)
|
Summary
STACKED on open PR #5735 (
w6-gate-dispatch) — merges after it. Builds on that branch'sTurnRunSnapshotSourceseam andsubmit_approval_resolution/submit_auth_resolutionhelpers.T1 (WebUI-API-2, shipped, approval leg only):
tests/integration/webui_v2_gate_refresh.rs— a browser refresh mid-gate must let the user rediscover and resolve the pending gate. Mounts the realwebui_v2router over a hand-builtRebornServicesfacade wired with the harness's own turn-state-convergedApprovalInteractionService. A freshstream_eventsdrain fromafter_cursor: None(the SSE handler is a polling wrapper over the same drain, per W5-WEBUI-SPIKE) surfaces the persistedGatePromptevent for the blocked run/gate; resolution goes through the realWEBUI_V2_PATTERN_RESOLVE_GATEHTTP route (POST .../gates/{gate_ref}/resolve), not a direct-resume test shortcut. Asserts the run completes and the approved write actually re-dispatched and persisted.T1 (auth leg / 5b) and T2(a) (
submit_auth_resolutione2e): NOT shipped — precise blocker found, no fiction-wiring.Investigated deeply (see reasoning below) and confirmed: manual-token auth gates never create an
AuthFlowRecord.DefaultAuthInteractionService::resolve'sfind_gaterequires anAuthFlowRecordwhosecontinuationis exactlyAuthContinuationRef::TurnGateResume{turn_run_ref, gate_ref}. Grepping everyTurnGateResumeconstruction site inironclaw_reborn_composition, the only place that ever creates one isoauth_gate.rs(GoogleOAuthGateProviderRegistry) — i.e. Google OAuth-gated capabilities. GitHub (github.get_repo, the only auth-gated capability any int-tier harness profile wires —live_auth_gate(),live_auth_and_approval()) is manual-token/PAT based; itsBlockedAuthblock never touchesoauth_gate.rsand creates noAuthFlowRecordat all. This is whyresolve_auth_gate's existing test-support shape (seed a scope-visible credential account + directTurnCoordinator::resume_turn) isn't a shortcut — it's the only way, because there's genuinely nothing forAuthFlowRecordSource/find_gateto find. Cross-confirmed independently viaauth_prompt.rs: the Slack-notification auth-prompt rendering explicitly falls back to synthesizing a view straight fromcredential_requirementsprecisely because manual-token gates have no flow/challenge to read.Consequence:
submit_auth_resolution's real dispatch arm is untestable against every existing int-tier harness profile (none wire a Google-OAuth-gated capability). Reaching one needs a new harness profile (OAuth extension asset + provider config + PKCE/state + scripted callback claim) or manually fabricating anAuthFlowRecordvia the publicAuthFlowManager::create_flow/complete_manual_tokenAPI — both a materially separate, bigger seam than "wire a test over existing profiles." Full trace is in the (local-only, not committed) plan doc; happy to paste inline if useful for follow-up planning.T2(b) (triggered run parked on
BlockedAuthrecords the delivery outcome): NOT shipped — same-class blocker.build_triggered_run_delivery_hookneeds&RebornRuntime(SlackHostBetaRuntimeParts::from_runtime), a composition path built only bybuild_reborn_runtime.RebornIntegrationGroup/HostRuntimeCapabilityHarness(this branch's harness) are built viabuild_reborn_servicesdirectly — a separate, parallel composition that never produces aRebornRuntime. No bridge exists between the two object graphs from outside the crate. Even setting that aside, GitHub'sBlockedAuthhas noAuthFlowRecordeither (same finding as above), so aDeliveredoutcome for a personal-scope triggered run parked on auth needs the same OAuth-gate plumbing. Ships no new test beyond the already-mergedtriggered_delivery_outcome.rs(proves theDenied/project-scoped arm through the real public factory).Mutation evidence (T1 approval leg)
gate_refin the HTTP POST →409 conflict(RED), reverted →200 OK(GREEN).GatePromptsearch on a deliberately-wronggate_ref→ assertion panic listing the actual correctly-shapedGatePromptevent present in the drain (RED, confirms the assertion discriminates rather than being vacuously true), reverted → GREEN.Test tails
Regression (unchanged, still green):
cargo fmtclean.cargo clippy --all --benches --tests --examples --all-features: 0 warnings.Test plan
cargo test --features integration --test reborn_integration_webui_v2_gate_refreshcargo test --features integration --test reborn_group_journeys(regression)cargo test --features integration --test reborn_integration_triggered_delivery_outcome(regression)cargo test --features integration --test reborn_group_approvals(regression)cargo fmt/cargo clippy --all --benches --tests --examples --all-features🤖 Generated with Claude Code