[codex] fix exa mcp sse initialize parsing - #5573
Merged
Merged
Conversation
|
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
WalkthroughMCP initialize response validation now accepts SSE-form bodies and raw JSON. Test helpers and coverage were added for single-line and split ChangesSSE MCP Initialize Validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5573
July 2, 2026 21:53
Destroyed
github-actions
Bot
added
size: M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for parsing Server-Sent Events (SSE) formatted responses during MCP initialization. It introduces a helper function mcp_json_value_from_body to extract JSON payloads from lines prefixed with data:, updates is_valid_mcp_initialize_response to use this helper, and adds corresponding unit and integration tests. There are no review comments, and I have no 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.
serrrfirat
marked this pull request as ready for review
July 2, 2026 21:54
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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_first_party_extensions/src/web_access.rs`:
- Around line 567-573: The SSE parsing in `web_access.rs` is treating each
`data:` line as a complete JSON payload, so multi-line events can fail before
`tools/call` proceeds. Update the helper that scans the response body to first
reconstruct a full SSE event by concatenating consecutive `data:` lines before
calling `serde_json::from_str`, then parse the combined payload in the existing
logic around the `body.lines().filter_map(...)` block. Add a regression covering
split `data:` lines so both the helper and the caller behavior stay protected.
🪄 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: 03c0a642-c3c8-4335-890a-c07245a4500e
📒 Files selected for processing (1)
crates/ironclaw_first_party_extensions/src/web_access.rs
Contributor
Reborn integration-tier coverageLine coverage (Reborn crates): 15.13% — 9645 / 63727 lines Per-crate breakdown (11 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. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5573
July 2, 2026 22:05
Destroyed
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5573
July 2, 2026 22:19
Destroyed
|
🚅 Deployed to the ironclaw-pr-5573 environment in ironclaw-ci-preview
|
henrypark133
added a commit
that referenced
this pull request
Jul 3, 2026
…ation, triggered/outbound, budget/comm-context/hooks seams, golden payloads, denied-edge contracts (#5584) * test(reborn): C-TRIGGERED-ORIGIN interactive-origin contrast arm Add the discriminating contrast to the E-TRIGGERED-SUBMIT origin test: a normal interactive submit_turn records TurnOriginKind::Inbound, read through the same coordinator.get_run_state(...).product_context.origin boundary the trigger test uses. Without a contrasting turn on the same wire, the existing ScheduledTrigger assertion could pass on a hardcoded origin; the pair proves the origin is genuinely propagated from the submission path. Mutation-verified: breaking the production TrustedTrigger->ScheduledTrigger mapping turns the trigger test RED while the interactive contrast stays GREEN. Also refresh the reborn_group_triggers TODO to record the current disposition: origin coverage DONE (flat test, not this multi-thread group), mid-fire approval gate STILL OUT (now authorable), and outbound-delivery (C-TRIGGERED-DELIVERY) BLOCKED at int tier (deliver_triggered_run is a private fn reachable only via a detached tokio::spawn, unwired in any harness turn lifecycle; branch logic already densely pinned by slack_delivery.rs's #[cfg(test)] module + product_workflow outbound_delivery_contract.rs). Tests-only; no production behavior change. E-OUTBOUND/C-OUTBOUND/ C-TRIGGERED-DELIVERY deferred: the outbound delivery sink is not wired into any harness-reachable production composition path, so per the no-wire-what-production-doesnt-wire rule it is reported, not wired. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): multi-turn baseline-sliced history asserts + script-discipline doc (Wave-3 infra) Adds the harness-infra baseline items from the reborn coverage roadmap: - assertions.rs (additive impl block): history_len() + fail-loud history_slice, assert_tool_error_since / assert_no_tool_error_since / assert_tool_error_summary_contains_since (baseline-sliced multi-turn variants of the documented single-turn-only tool-error asserts), and assert_conversation_history_contains{,_since} + assert_conversation_history_role_contains (general persisted-transcript containment, role-filterable). - reborn_integration_http_matcher.rs: multi_turn_baseline_sliced_history_assertions demo/regression — two error-raising turns on one thread; positive, slice-exclusion, role-discrimination, and out-of-range-baseline fail-checks. - tests/support/reborn/CLAUDE.md: documents the new asserts and the script-discipline rule that a gated tool-call turn consumes exactly 2 script entries (tool call + one post-resume model call) whether the gate is approved or denied. Existing asserts untouched (strictly additive for parallel-lane union-merge). No new harness wiring; helpers read already-persisted thread history through the existing accessor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): C-MULTIUSER per-actor memory/approval isolation (#5460) Extends tests/reborn_group_multiuser with the C-MULTIUSER matrix: proof that two distinct actors sharing the group's ONE capability backend isolate their capability-side state per owner, the way production does. E-MULTIUSER (#5526, `with_actor_id`) isolates only thread HISTORY (turn store subtrees). Capability-side state — memory, auto-approve, approval settings — was NOT isolated in the harness: the capability port factory hardcoded one fixed execution user for every actor. Production instead derives the capability `(tenant, user)` from the run owner/actor (`local_dev_visible_capability_request`, wired via `LocalDevLoopCapabilityPortFactory` at runtime.rs → capability_wiring), falling back to a fixed id only when no owner is present. Seam (tests/support/reborn, additive, default-off so every existing test is byte-identical): `HostRuntimeCapabilityHarness` gains an opt-in `scope_capability_by_run_owner` flag + `dispatch_user_for_run` that mirrors production's owner→actor→fallback resolution; the harness capability port factory now uses it. Two self-contained group constructors (`multiuser_memory_tools`, `multiuser_approvals`) enable it, plus per-owner auto-approve `enable/disable_auto_approve_for_owner` helpers (same `AutoApproveSettingStore` + `(tenant,user)` key production uses). Scenarios: - memory_isolation_across_actors: A writes memory, distinct actor B cannot search it, A still can (pins the tool-path isolation #5460's reporter confirmed; #5460 itself is the WebUI read surface). - auto_approve_isolation_across_actors: A's always-allow grant lets A's write_file skip the gate while B (own always-allow OFF) still raises a real BlockedApproval on the identical call. Mutation-verified: forcing the fixed user (ignoring the flag) turns BOTH isolation scenarios RED for the right reasons (B reads A's memory; B no longer gates), while two_actors_own_threads stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): C-SYNTH skill_activate over-budget Failed-routing coverage skill_activate ContextBudgetExceeded is the one synthetic-capability failure route coverable with ZERO new harness wiring: an oversized system skill seeded through the existing seed_system_skill_for_test seam drives the real selection path (reserve_skill_budget → ContextBudgetExceeded → CapabilityOutcome::Failed) and surfaces as a model-visible recoverable Failed tool error; the run completes and the failed skill's instructions are proven NOT injected. Remaining C-SYNTH routes (outbound_delivery_* — unwired, needs facade double + constructor enabler; project_create Conflict/Unavailable — needs whitebox service-error injection; skill AmbiguousSkill + source B-routes) are deferred per the spike inventory; project_create Denied/NotFound arms are unreachable from the real create_project path (no require_role call) and stay unit-tier only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): wire budget/comm-context/hooks seams + coverage (C-BUDGET/C-COMMCTX/E-HOOK-INFRA/C-HOOKS) Wave-3 rev-3 + hooks int-tier coverage. All changes are tests-only; the three DefaultPlannedRuntimeParts seams wired here are production-active (verified against build_reborn_runtime: model_budget_accountant, communication_context_provider, hook_dispatcher_builder_factory are all Some in production), so wiring them in the harness closes genuine drift (A1 audit), not new behavior. - C-BUDGET: RebornIntegrationGroupBuilder::budget_accounting() + with_budget_accounting() wire the production build_default_budget_accountant (in-memory governor/gate-store/zero-cost-table + compiled-default seeding) into the group's one planned runtime; assert_budget_user_cap_seeded reads back the seeded daily cap. Wiring-liveness only — semantics stay at crate tier (budget_e2e.rs). tests/reborn_integration_budget.rs (+ negative guard). - C-COMMCTX: RecordingCommunicationContextProvider double + communication_context_provider()/with_communication_context_provider() seams; assert the delivery-target + connected-channel slice renders into the model request. tests/reborn_integration_comm_context.rs (+ negative guard). The provider's facade->context mapping stays crate-tier covered. - E-HOOK-INFRA: recording hook doubles (RecordingHookLog + observer/before-cap hooks) + recording_hook_factory/denying_hook_factory builders, wired via hook_dispatcher_builder_factory()/with_hook_factory(). - C-HOOKS: two scenarios are #[ignore]d RED regressions pinning a genuine cross-crate production bug discovered here: HookedLoopCheckpointPort (crates/ironclaw_hooks/src/middleware/checkpoint_port.rs) overrides only checkpoint() and does NOT forward stage_checkpoint_payload/load_checkpoint_payload to its inner port, so those fall through to the LoopCheckpointPort trait's fail-closed defaults. Any hook-dispatcher-active coordinator turn dies driver_unavailable at checkpoint_before_model. Hooks are off by default in production so this latent wrapper bug was never exercised — this is the first full-turn-with-hooks path. TODO(reborn-hooks-checkpoint-forward). Fix is cross-crate, out of scope for this tests-only lane; un-ignore once forwarded. Shared support-file edits (group.rs/builder.rs/assertions.rs) are strictly additive (new fns/fields appended) for clean union-merge with sibling lanes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): drive triggered runs to completion + mid-fire gates at int tier Extend the E-TRIGGERED-SUBMIT seam with submit_triggered_turn_scripted: mirrors the production trigger poller's materializer (ConversationContentRefMaterializer::materialize_prompt — pub(crate), hence mirrored) instead of the for_fire shortcut: resolve the fire's conversation binding (mints the scope pre-submit), record the prompt as a real inbound thread message (without this the run dies driver_unavailable on "unknown thread" — found empirically), build the real thread-message:<id> content ref, register a scripted gateway for the EXACT resolved scope (no fallback mechanism, no race), then submit through the production TrustedTriggerFireSubmitter. Adds scope-aware wait_for_status_in_scope / approve_gate_in_scope / deny_gate_in_scope / resume_run_in_scope twins (builder.rs twins poll self.turn_scope; additive-only constraint on shared support files defers consolidation). New coverage: - triggered_run_completes_and_persists_reply_in_trigger_thread (reborn_integration_triggered_submit): a triggered run completes and its final reply persists in the trigger's OWN thread — the int-tier-observable half of triggered delivery (the state production's deliver_triggered_run reads before pushing). Probe finding pinned in the test doc: no harness composition routes a completed run through render_outbound/OutboundDeliverySink; the push leg stays with the services-shell spike. - triggered_gate_group (reborn_group_triggers, scenario_triggered_gate::{run_approve,run_deny}): a triggered fire raises a REAL BlockedApproval gate mid-fire; approve re-runs the gated write (file persisted), deny surfaces a non-retryable authorization failure (file absent). Finding: nothing in the scheduled_trigger surface/deny-map (#5505) suppresses approval gates — trigger-origin runs gate exactly like interactive runs. Once{at} post-fire completion derivation deliberately NOT re-authored: already pinned at crate tier (repository_contract.rs clear_active_fire → Completed; trigger_poller_e2e.rs settle). Mutation-verified: (1) skipping the scope-gateway registration turns the completion test RED (model_error via the scope-miss sentinel); (2) resuming in the harness thread scope instead of the fire's scope turns both gate arms RED ("turn run not found"). Both reverted. Tests-only; no production behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): crate-tier trigger→gate→approve Slack loop twin (Wave-3 slackloop) Add a deterministic crate-tier twin of the live Slack canary flow to slack_serve/e2e_tests.rs: a triggered run (personal, foreign thread scope) blocks on approval; the production TriggeredRunDeliveryDriver posts the approval prompt to the creator's Slack DM through a fake protocol egress and auto-records a delivered gate route; an inbound bare `approve` in that DM then resolves the gate on the run's foreign scope via the DRIVER-recorded route (not a hand-seeded one). This welds two production assemblies that were previously pinned only in isolation: the triggered-delivery route recording (slack_delivery cfg(test)) and the inbound delivered-route resolution (bare_approve_in_dm_resolves_gate_recorded_by_observer). Both the driver and the workflow share one DeliveredGateRouteStore; test doubles substitute only at seams the production triggered factory (build_triggered_run_delivery_hook_from_parts) fills (egress real, binding_service Noop). The final-reply tail after approve is pinned separately by slack_approval_reply_resumes_and_delivers_final_reply; stitching it here would race the triggered driver's own delivery loop against the live observer's (two independent active_delivery_run_ids sets), a cross-assembly dedup question outside this test's scope. Test-only; zero production changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): outbound tool-port enabler + coverage (C-SYNTH outbound) Wire the two production local-dev synthetic capabilities builtin.outbound_delivery_targets_list / builtin.outbound_delivery_target_set into the Reborn integration harness at the production-wired OutboundPreferencesProductFacade trait seam: - test_support wrap accessor reusing the REAL outbound_delivery_capabilities + wrap_local_dev_synthetic_capabilities + StoreApprovalSettingsProvider (mirrors the project_create / skill_activate seam precedent; all additions #[cfg(feature = "test-support")]-gated, zero production-behavior change) - FakeOutboundPreferencesFacade double (in-memory targets; unknown id -> NotFound) - outbound_target_tools() harness preset + group constructor; disable_auto_approve() harness method (gate arm); RecordingApprovalRequestStore so port-level synthetic gates record their approval scope for approve/deny_local_dev_gate - New test bin tests/reborn_integration_outbound_target.rs covering: targets_list happy path; target_set happy path with facade read-back; settings-Deny -> Failed/policy_denied; facade NotFound -> Failed/invalid_input; approval gate approve -> resume applies; deny -> facade never reached Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): golden inference-payload exact-match coverage Add exact-match "golden inference payload" assertions to the Reborn integration tier. Where assert_system_prompt_contains proves a substring reached the model, this pins end-to-end prompt construction: the FULL model-visible payload per inference iteration (system prompt, all turns, and tool-call/tool-result messages) plus a compact ordered tool surface, snapshotted with insta (the repo's established snapshot tool), and the exact final user-visible reply. Canonicalization renders the captured request to sorted-key JSON and normalizes exactly one genuinely nondeterministic value — the runtime context's model-visible wall clock — to <TIMESTAMP>. Everything else is byte-exact: tool-call ids (deterministic call-N) and the surface sha256 content hash stay verbatim so real drift is caught. Three representative scenarios: single-turn greeting (base prompt), a tool-call turn (both iterations — pins tool-result feed-back with matching tool_call_id), and a two-user-turn thread (pins history accumulation). Tool JSON schemas are excluded from the golden deliberately (they are pinned by the surface sha256 in the exact-matched system prompt and by each tool's own tests) to avoid coupling these goldens to unrelated builtin-tool edits. Regenerate drift with `cargo insta review`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): C-JOURNEY enabler seams — auth+approval convergence fixture Enablers for multi-turn journey coverage (auth gate -> resolve -> approval gate -> approve -> follow-up on ONE conversation): - HostRuntimeCapabilityHarness::file_and_github_auth_tools(): ONE build_reborn_services local-dev runtime surfacing BOTH file tools at PermissionMode::Ask (BlockedApproval) and an unseeded github.get_repo (BlockedAuth via the REAL ProductAuthRuntimeCredentialResolver). Making github.* genuinely dispatchable needed two test-support-gated composition accessors (publish into the active-extension registry + real asset-dir copy into the harness mount) — no production wiring changes. - RebornIntegrationHarness::resolve_auth_gate(): the 'user submitted credentials' happy arm — seeds a real GitHub credential account WITH secret material through the production manual-token flow, then resumes with BlockedAuthGate precondition so the parked capability re-dispatches. - RebornIntegrationGroup::live_auth_and_approval() preset (group_constructors.rs), subject-user-aligned like live_approvals. - HARNESS BUG FIX: RecordingHostRuntime never forwarded the defaulted auth_resume_capability, so every auth resume died on the trait's fail-loud default ('auth-resume is unsupported by this host runtime') — latent until the first happy-path auth resume exercised it. - HARNESS BUG FIX: resume_run idempotency key was run_id-only, so a run resuming through TWO gates (approval then auth) replayed the first resume's cached response and wedged at BlockedAuth; key is now (run_id, gate_ref)-scoped. - assert_model_request_contains_all(): multi-turn context-carryover assertion (all needles in ONE captured model request). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): C-JOURNEY multi-turn journey scenarios (reborn_group_journeys) Deterministic twins of the live canary multi-turn use cases, chaining gate -> resume -> next turn on ONE conversation over the group's one shared runtime: - interactive_approval_journey: approve turn -> deny turn -> follow-up; asserts per-turn side effects (approved write persists, denied write absent, no cross-turn bleed) and context carryover (turn 3's ONE model request carries turn 1's and turn 3's user text). - auth_then_approval_journey: turn 1 github.get_repo chains BlockedApproval -> approve -> BlockedAuth -> resolve (real manual-token flow) -> the SAME parked capability re-dispatches and its result carries the scripted network fixture body; turn 2 approval gate; turn 3 follow-up sees history across both gate classes. - auth_deny_then_retry_journey (mixed arm): turn 1 auth gate DENIED (run continues, #4944 semantics); turn 2 raises fresh approval+auth gates, resolve succeeds — a denied auth gate does not poison a later resolve. - multi_actor_gate_isolation: RED #[ignore]d — blocked on the unmerged C-MULTIUSER scope_capability_by_run_owner harness seam (distinct actor's gated dispatch is scoped to the canonical user on this base); TODO(reborn-multiuser-gate) pins the coverage for un-ignore. Mutation-verified: seeding disabled -> journeys wedge at BlockedAuth (RED); bogus needle -> context-carryover assert RED. Single-gate mechanics stay pinned by reborn_group_approvals / reborn_integration_auth_gate (docs cross-referenced); journey value is the CHAINING. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): external-wire format-matrix coverage for capability parsers (C-WIREFMT) Pins every legal wire framing at first-party external-response parse sites, per the rule: if a request's Accept header names N content types, cover >= N response-format fixtures. - ironclaw_mcp: crate-tier matrix through the unified parse_mcp_response dispatch (plain JSON, SSE single-event, SSE multi-event w/ keepalive, error-object in both framings, empty-body rejection in both framings). main already parses both framings via one shared entry — pinned so a future JSON-only sibling parse site can't ship untested. - mock_mcp_server + reborn_integration_mcp: additive enable_sse_framing() toggle (default off; existing tests unaffected) and an int-tier case driving the real MCP client over an SSE-framed initialize/tools/list/ tools/call handshake on loopback HTTP. - gsuite_core: empty-body 204 framing through the real google-calendar.delete_event handler (response_body_json Null branch). - oauth_provider_client: malformed 200 token-exchange body surfaces TokenExchangeFailed instead of panicking/succeeding. Fixtures are hand-authored (no live-captured MCP/Google bodies exist under tests/fixtures/); SSE shapes mirror the production unit fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): salvage int-tier SSE web-access regression tests (C-WIREFMT) Int-tier twins of #5573's crate-tier SSE-framing coverage: SSE-framed MCP initialize through the real Reborn web-access handler + a both-legs-SSE sibling-parity case. #5573 (already on base) shipped only the crate-tier matrix; these exercise the full dispatch pipeline. Tests-only; the fix itself rides in via #5573. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): un-ignore multi_actor_gate_isolation via C-MULTIUSER seam The C-MULTIUSER scope_capability_by_run_owner harness seam merged in this fold, so the RED #[ignore]d journey now runs on multiuser_approvals() (per- actor capability dispatch). The scenario disables auto-approve per owner so both actors raise a real BlockedApproval, pinning that gate resolution + resume state stay bound to the raising actor. Green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(reborn): refresh journey deferred-note for landed triggered scripted seam Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn-composition): fully-qualify test-support-only ProductWorkflowError Journey's publish_bundled_extension_for_test is the sole user of ProductWorkflowError in factory.rs and is test-support-gated, so importing it unconditionally warned (unused) under default features. Fully-qualify at the use site to keep the default-features build warning-clean and the addition fully behavior-neutral. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(reborn): rustfmt import ordering in merged slack_serve e2e_tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): strengthen wave-3 assertions per meticulousness audit All 9 HIGH rows plus the mechanical MED/LOW rows from the wave-3 assertion audit: exact-token pins for gate-declined and hook-deny summaries, payload-shape asserts on outbound target set/list, bounded shape-filtered polling for the slack approval prompt (deterministic under the ScriptedTriggerCoordinator auto-advance race), deny-arm reply/egress pins in the auth journey, load-bearing auto-approve grant proof, sliced-history fail-checks, SSE handshake method-order and egress-count pins. Makes FakeOutboundPreferencesFacade stateful and consolidates the duplicated trigger-fire/actor-pairing setup in triggered_submit.rs now that the sibling lanes have landed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(reborn-tests): trim wave-3 comment/doc bloat Prose-only pass over wave-3-added doc-comments and test-body comments: drops lane-history narration, play-by-play that restates adjacent asserts, and cross-file duplicate explanations (one canonical copy + cross-reference kept). Compresses the CLAUDE.md script-discipline and sliced-history sections, corrects the now-stale "must add baseline scoping first" caveats to point at the landed *_since variants, fixes the stale ignored-test module doc on multi_actor_gate_isolation, and scopes the mock MCP server's enable_sse_framing doc to what callers actually exercise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(reborn-tests): suite-wide comment de-bloat across reborn test bins and support Prose-only pass over the whole Reborn suite (integration bins, group scenarios, qa, parity, test-support helpers): deletes play-by-play narration, lane/wave/PR-process history, comments restating adjacent asserts, and cross-file duplicate mechanism explanations (one canonical copy kept with cross-references). Why-pins, seam contracts, mock-fidelity caveats, and C-XXX tags retained. Net ~-200 comment lines; mechanically verified comment/blank-only via -U0 diffs; zero code changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): pin disabled-capability contract for spawn_subagent Two contracts for the global DISABLED_CAPABILITY_IDS deny (CapabilitySurfaceDenyFilter, outermost): the id is stripped from the model-facing tool surface (non-vacuous — the manifest stub and spawn decorator would otherwise surface it; builtin__http as presence control), and a hallucinated call to it anyway is rejected at the model gateway before registration, failing the run terminally with failure category model_error and zero capability dispatch. General-surface sibling of the scheduled-trigger deny coverage (#5515). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): golden context-surfacing payload scenario Adds golden_context_surfacing to the dedicated full-payload monitoring suite: a wired communication-context provider plus the real builtin capability surface on one turn, snapshotting byte-for-byte how the communication section and capability surface render together in the system prompt. Module doc now states the suite's purpose (watching prompt-construction drift on a deliberately small scenario set). Compaction golden is blocked and documented in-file: the byte-cap overflow force-compact flag (CapabilityResultOverflow) is a dead letter under ActiveTaskPreservingCompactionStrategy, which never reads force_compact_on_next_iteration — needs a production decision first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): blocked/denied edge-case batch (C-DENYEDGE) Ranked cheap deny/blocked edge-case coverage over existing bins, all pinned to exact production tokens via persisted-history/typed-error reads: - Row 1 (security): triggered run's approval gate cannot be resumed under a mutated (wrong-tenant) TurnScope — pins the exact TurnError::ScopeNotFound "turn run not found" rejection and proves the gate stays live/resolvable afterward (reborn_group_triggers/scenario_triggered_gate.rs). - Row 2: cross-tenant secret lease fails closed with SecretStoreError::UnknownSecret (same-scope-owner gate), with a same-handle correct-tenant success arm as the non-vacuity proof (reborn_integration_secrets.rs). - Row 4 (int-tier #5515 twin): a scheduled-trigger fire scripting builtin.trigger_create is rejected at the model-gateway seam (CapabilitySurfaceDenyFilter via the scheduled_trigger surface profile) — never dispatched, run Completes, and an interactive trigger_list confirms no trigger was created. Doc comment traces why no ToolResultReference is persisted for this seam (scenario_trigger_self_create_denied.rs). - Row 5 (wedge class): a Failed run releases the thread-busy lock — a second submit on the same thread is Accepted (not RejectedBusy) and reaches its own terminal state (reborn_integration_cancel.rs). - Row 6: double-resolving an already-approved gate fails with ApprovalResolutionError::NotPending{Approved} (scenario_gate_then_approve.rs). - Row 7: local-dev approval resolved with the real gate_ref but resumed with a stale one pins TurnError::InvalidRequest "gate resolution reference mismatch"; the real-ref resume then completes the run (scenario_gate_ref_edge_cases.rs, plus additive builder.rs helpers approve_gate_with_stale_resume_ref / resume_gate). - Row 10: bare resolve of a well-formed never-issued gate ref pins the harness request-not-found rejection (same file). - Row 11: MCP tools/call whose parsed result exceeds McpRuntimeConfig::default() max_output_bytes (1 MiB) surfaces as Failed{output_too_large} and the run recovers (reborn_integration_mcp.rs). Skipped with findings (no seam / by-design): row 3 (scripted calls for unregistered capability ids are silently dropped before dispatch at this tier), row 8 (extension-install store is a runtime-wide singleton, never user-scoped — no owner-isolation seam exists), row 9 (RebornIntegrationHarness has no drain/shutdown seam), row 12 (off-theme for the touched bins). resume_run_in_scope widened to pub(crate) so the row-1 scenario reuses the canonical resume tail instead of hand-building a ResumeTurnRequest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): post-review fixes — resume idempotency parity, scenario error idiom, doc corrections Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): satisfy no-panics lint on slack e2e poll helpers The panic-lint scans files standalone and does not follow the parent's #[cfg(test)] mod attribute, so the three test-only .expect() calls in wait_for_gate_route / wait_for_approval_prompt_messages need explicit // safety: suppression comments, matching sibling handler_tests.rs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): keep no-panics safety comment inline under rustfmt rustfmt moves trailing comments off an if-let scrutinee, which breaks the lint's same-line suppression. Bind the loaded route first so the // safety: comment sits on a plain statement rustfmt leaves alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): review round 2 — typed outbound test parts, discriminating over-budget assert, SSE tools/list contract, atomic fake state Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(reborn): merge origin/main; regenerate golden surface hash for #5574 Merge brings the engine-v2 removal (#5545) and step-efficient tool guidance (#5574). The tool-guidance change shifts the tool-surface content hash inside the golden system prompts — the goldens' only diff is the surface sha256 line, which is exactly the drift they exist to pin. Regenerated both affected snapshots; all wave-3 bins re-verified green on the merged base. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
data:events before parsing the JSON-RPC payloadget_contentcaller-level regression coverageChange Type
Bug fix
Linked Issue
None. Regression from #5534; fix tracked in #5573.
Root Cause
PR #5534 tightened MCP initialize validation to parse only raw JSON. Exa currently returns initialize as Server-Sent Events with the JSON-RPC payload in
data:lines, soexa_websearchandget_contentfailed before reachingtools/callwithOutputDecode.Security Impact
Low. This only broadens MCP initialize parsing to valid SSE framing from the existing Exa endpoint. The initialize response still has to validate as JSON-RPC with a result object, and existing network egress/body limits remain unchanged.
Database Impact
None.
Blast Radius
Limited to first-party
web-access.searchandweb-access.get_contentExa MCP initialize parsing.Rollback Plan
Revert this PR. That restores the previous strict raw-JSON initialize parser, but it also reintroduces the Exa SSE
OutputDecoderegression.Review Follow-Through
data:lines before JSON parsing.data:helper coverage and caller-levelget_contentregression coverage.Review track
Correctness / first-party web-access regression fix.
Validation
cargo fmt --checkCARGO_TARGET_DIR=/Users/firatsertgoz/.codex/worktrees/e801/ironclaw/target cargo test -p ironclaw_first_party_extensions split_data_sse_mcp_initialize_responseCARGO_TARGET_DIR=/Users/firatsertgoz/.codex/worktrees/e801/ironclaw/target cargo test -p ironclaw_first_party_extensions web_access::testsCARGO_TARGET_DIR=/Users/firatsertgoz/.codex/worktrees/e801/ironclaw/target cargo clippy -p ironclaw_first_party_extensions --all-targets --all-features -- -D warningsbash scripts/pre-commit-safety.shgit diff --check