Repository navigation
fix(webui): restore smooth streaming and preserve model phases - #6876
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-6876 environment in ironclaw-ci-preview
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR replaces polling-oriented WebUI event delivery with process-local subscriptions where supported, adds SSE keep-alives and recovery, introduces phase-qualified live assistant text, updates chat rendering and history reconciliation, and adjusts integration, browser, provider, and CI tests. ChangesWebUI streaming and live projections
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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 |
🔎 Review · PR #6876
Submitted review →Reviewed the complete trusted base-to-head comparison. The product projection subscription change and WebUI active-stream watchdog are coherent with existing cursor replay, authorization, SSE/WebSocket lifetime, and EventSource recovery contracts. No actionable defects found. Automatic · PR opened · attempt 1 of 3 · completed in 2m 15s Run details
|
There was a problem hiding this comment.
🔍 Review complete · PR #6876
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. The product projection subscription change and WebUI active-stream watchdog are coherent with existing cursor replay, authorization, SSE/WebSocket lifetime, and EventSource recovery contracts. No actionable defects found.
Validation and technical details
- Inspected all 5 changed files and surrounding product projection, SSE/WebSocket handler, EventSource lifecycle, and chat processing-state code.
- Verified trusted comparison refs/ironloop/base..refs/ironloop/head and ran git diff --check successfully.
- Focused frontend SSE suite passed: 16/16 tests.
- Frontend lint, source-convention checks, and TypeScript typecheck passed.
- Rust tests could not be executed because cargo is unavailable in the review environment; Rust behavior was validated through source and contract-test inspection.
- Base:
main - Head:
codex/fix-stalled-webchat-streamata9c545a - Run:
c5e34161-0bda-4bab-80be-37ec22d133f8
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_webui/frontend/src/pages/chat/hooks/useSSE.ts`:
- Around line 35-40: The active-stream watchdog must not fire during healthy
runs with no runtime payloads. Update the SSE projection/keep-alive flow around
push_runtime_cursor_advance to emit application-level KeepAlive frames
periodically whenever any runtime is active, aligned below
ACTIVE_STREAM_STALL_DEADLINE_MS; alternatively increase the JavaScript deadline
in useSSE.ts beyond the guaranteed app-frame interval while preserving half-open
detection.
🪄 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: 4fe2f503-746a-4bcb-b667-c5e8a125aa63
📒 Files selected for processing (5)
crates/ironclaw_product/src/reborn_services.rscrates/ironclaw_product/tests/reborn_services_contract.rscrates/ironclaw_webui/frontend/src/pages/chat/hooks/useChat.tscrates/ironclaw_webui/frontend/src/pages/chat/hooks/useSSE.tscrates/ironclaw_webui/frontend/src/pages/chat/lib/useSSE.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_product/src/projection/tests/live_progress_stream.rs (1)
335-384: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
text_bodies.len() == 64asserts zero drops from a bounded channel and double-counts snapshot items.Two coupling problems in one assertion:
text_bodiesis appended perTextitem per envelope (Line 345-351). BecauseProjectionUpdate.state.itemsis a snapshot, any envelope that re-carries the run's Text item — including the one carrying the capability activity — contributes another body. The count therefore measures snapshot fan-out, not "64 preserved updates".- All 65 milestones are published before the first
subscription.next(), so passing depends onPRODUCT_PROJECTION_SUBSCRIPTION_BUFFER = 128absorbing the whole burst. Change the capacity or the overflow policy and this test fails for reasons unrelated to the provider-rate contract.The contract worth locking is smoothness plus ordering, not an exact integer: the terminal body is already asserted at Line 375-377; add monotonic ordering and a floor instead.
0..70at Line 335 has the same hidden dependency — it only works while the envelope count stays under it.♻️ De-couple the assertion from buffer capacity and snapshot fan-out
- assert_eq!( - text_bodies.len(), - 64, - "the bounded WebUI stream should preserve provider-rate text updates: {text_bodies:#?}" - ); + let mut distinct = text_bodies.clone(); + distinct.dedup(); + assert!( + distinct.len() > 1, + "provider-rate text must stream incrementally rather than arrive as one final blob: {text_bodies:#?}" + ); + let mut ordered = distinct.clone(); + ordered.sort_by_key(|body| { + body.rsplit(' ') + .next() + .and_then(|index| index.parse::<usize>().ok()) + .unwrap_or_default() + }); + assert_eq!( + distinct, ordered, + "live text updates must stay in provider order: {text_bodies:#?}" + );🤖 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_product/src/projection/tests/live_progress_stream.rs` around lines 335 - 384, Update the live projection stream test around the collection loop and final assertions to stop treating text_bodies.len() as a count of preserved provider updates. Track unique or monotonically advancing text bodies, assert their ordering, and retain a lower-bound smoothness check rather than exact equality. Make the receive loop continue until the expected terminal text/tool ordering is observed without relying on a fixed envelope count tied to subscription buffer capacity.
🤖 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_product/src/projection.rs`:
- Around line 458-460: The subscriber capacity in SubscriberCapabilities must
use the manager-owned maximum or a named constant exported by
ironclaw_event_streams, replacing the product-local
PRODUCT_PROJECTION_SUBSCRIPTION_BUFFER, and verify overflow retains the newest
cumulative text item. In crates/ironclaw_reborn_composition/CLAUDE.md lines
399-404, remove the unsupported “maximum bounded subscriber buffer” and “16-item
default” claims or cite the owning constant so the documentation remains
checkable.
In `@crates/ironclaw_reborn_composition/CLAUDE.md`:
- Around line 442-445: Update the documentation bullet for
provider_rate_live_text_stays_smooth_and_precedes_tool_activity to describe that
the latest cumulative text is published before the capability activity, rather
than claiming every text update is ordered before the next milestone.
---
Outside diff comments:
In `@crates/ironclaw_product/src/projection/tests/live_progress_stream.rs`:
- Around line 335-384: Update the live projection stream test around the
collection loop and final assertions to stop treating text_bodies.len() as a
count of preserved provider updates. Track unique or monotonically advancing
text bodies, assert their ordering, and retain a lower-bound smoothness check
rather than exact equality. Make the receive loop continue until the expected
terminal text/tool ordering is observed without relying on a fixed envelope
count tied to subscription buffer capacity.
🪄 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: f321025e-6151-4b0a-9160-f9e0f7e65a4a
📒 Files selected for processing (4)
crates/ironclaw_product/src/projection.rscrates/ironclaw_product/src/projection/live_progress.rscrates/ironclaw_product/src/projection/tests/live_progress_stream.rscrates/ironclaw_reborn_composition/CLAUDE.md
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_llm/src/nearai_chat.rs`:
- Around line 2019-2047: Extend the caller-level streaming tests around
complete_search_tool_streaming to cover parsed tool calls with no finish reason,
asserting FinishReason::ToolUse, and an explicit unrecognized finish reason,
asserting FinishReason::Unknown. Build each case through the existing HTTP/SSE
test server and caller wrapper rather than testing classifiers directly, while
preserving the existing terminal text Stop coverage.
🪄 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: a4ab7340-7770-489b-8d89-6c93a51a6d8d
📒 Files selected for processing (2)
crates/ironclaw_llm/CLAUDE.mdcrates/ironclaw_llm/src/nearai_chat.rs
This reverts commit 5f3ab32.
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_product/src/projection/live_progress.rs`:
- Line 48: Replace the Mutex protecting text_phase_by_run in the live progress
state with an RwLock, using read access when retrieving the existing text_id and
write access when creating or removing phase entries. Preserve the current
phase-state behavior while updating all corresponding lock calls and
initialization.
🪄 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: 97e1d51a-2fca-488d-b809-40c7fc6da0ae
📒 Files selected for processing (10)
crates/ironclaw_product/src/projection/live_progress.rscrates/ironclaw_product/src/projection/tests/live_progress_stream.rscrates/ironclaw_webui/CLAUDE.mdcrates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.tscrates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsxcrates/ironclaw_webui/frontend/src/pages/chat/hooks/useHistory.tscrates/ironclaw_webui/frontend/src/pages/chat/lib/stream-order-memory.tscrates/ironclaw_webui/frontend/src/pages/chat/lib/useChatEvents.test.tscrates/ironclaw_webui/frontend/src/pages/chat/lib/useChatEvents.tscrates/ironclaw_webui/frontend/src/pages/chat/lib/useHistory.test.ts
This reverts commit 3dc16f8.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/ci/ws12_workflow_contracts.py (2)
60-70: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDetect folded
if: ${{ false }}values.The multiline branch only matches bare
false; it misses valid YAML such as:if: >- ${{ false }}A lane can therefore be unconditionally disabled while this guard passes. Parse the workflow or extend the matcher, and add a regression case for folded expressions.
🤖 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 `@scripts/ci/ws12_workflow_contracts.py` around lines 60 - 70, Update the UNCONDITIONAL_SKIP matcher to recognize folded YAML if values containing the expression ${{ false }}, including the shown >- form, or parse the workflow to detect them reliably. Add a regression case covering a folded false expression and preserve detection of the existing bare and block-scalar false forms.
74-87: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftValidate active workflow structure, not arbitrary source substrings.
marker not in textaccepts markers left in comments or dead scalars, so a removed job can still satisfy WS12. It also makes valid folded commands format-sensitive. Tie each marker to parsed jobs, steps, commands, or triggers, with fixtures for comments and inactive steps.🤖 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 `@scripts/ci/ws12_workflow_contracts.py` around lines 74 - 87, Update validate_workflow_texts to parse each workflow’s YAML structure and validate REQUIRED_MARKERS against active jobs, steps, commands, and triggers rather than using raw substring checks. Exclude comments, dead or inactive steps, and other non-executable scalars, while normalizing folded command representations so equivalent valid commands are accepted. Preserve missing-workflow and unconditional-skip reporting, and add fixtures covering commented markers and inactive steps..github/workflows/reborn-e2e.yml (1)
265-272: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid a secondary failure when coverage generation is skipped.
if: always()still runs this upload after the validation step fails, butproduct_surface_coverageis then skipped and its output/path may not exist. Withif-no-files-found: error, the artifact step obscures the original failure with another error. Gate this step onsteps.product_surface_coverage.outcome != 'skipped'.Proposed fix
- if: always() + if: always() && steps.product_surface_coverage.outcome != 'skipped'🤖 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 @.github/workflows/reborn-e2e.yml around lines 265 - 272, Update the “Upload product-surface coverage matrix” step’s condition to require both `always()` and `steps.product_surface_coverage.outcome != 'skipped'`, preventing artifact upload when coverage generation was skipped while preserving uploads after other outcomes.
🤖 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 @.github/workflows/reborn-e2e.yml:
- Around line 265-272: Update the “Upload product-surface coverage matrix”
step’s condition to require both `always()` and
`steps.product_surface_coverage.outcome != 'skipped'`, preventing artifact
upload when coverage generation was skipped while preserving uploads after other
outcomes.
In `@scripts/ci/ws12_workflow_contracts.py`:
- Around line 60-70: Update the UNCONDITIONAL_SKIP matcher to recognize folded
YAML if values containing the expression ${{ false }}, including the shown >-
form, or parse the workflow to detect them reliably. Add a regression case
covering a folded false expression and preserve detection of the existing bare
and block-scalar false forms.
- Around line 74-87: Update validate_workflow_texts to parse each workflow’s
YAML structure and validate REQUIRED_MARKERS against active jobs, steps,
commands, and triggers rather than using raw substring checks. Exclude comments,
dead or inactive steps, and other non-executable scalars, while normalizing
folded command representations so equivalent valid commands are accepted.
Preserve missing-workflow and unconditional-skip reporting, and add fixtures
covering commented markers and inactive steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7e72f906-470a-49b7-8a49-fb62d64e63b5
📒 Files selected for processing (7)
.github/workflows/reborn-e2e.ymlcrates/ironclaw_product/tests/reborn_services_contract.rsscripts/ci/reborn-crate-test-buckets.shscripts/ci/test-reborn-crate-test-buckets.shscripts/ci/ws12_workflow_contracts.pytests/e2e/scenarios/test_reborn_webui_v2_smoke.pytests/integration/webui_v2_product_api.rs
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_product/src/projection/tests/live_progress_stream.rs (1)
404-462: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid wall-clock timing for delivery sequencing.
This test relies on short delays and timed
next()reads to prove provider-cadence updates. Scheduler jitter can make it intermittently time out before the producer publishes. Use synchronization/barrier signals for sequencing, retaining only a generous bounded timeout as a failure watchdog.🤖 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_product/src/projection/tests/live_progress_stream.rs` around lines 404 - 462, Update the live projection sequencing test around the milestone publishing loop and subscription reads to replace the 25ms sleeps and per-read one-second timeouts with explicit synchronization/barrier signals that confirm each provider-cadence update is delivered before publishing the next milestone. Retain only a generous overall bounded timeout as the failure watchdog, while preserving the assertion that text items precede the capability activity.
🤖 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_webui/src/webui_v2/handlers.rs`:
- Around line 1306-1311: Update sse_keep_alive_event to derive its payload from
the typed WebChatV2Event::KeepAlive serialization instead of duplicating a raw
JSON literal, ensuring the event name and wire payload remain aligned. Add or
update serialization, ordering, and transport tests for the keep-alive heartbeat
frame as required by the event contract.
---
Outside diff comments:
In `@crates/ironclaw_product/src/projection/tests/live_progress_stream.rs`:
- Around line 404-462: Update the live projection sequencing test around the
milestone publishing loop and subscription reads to replace the 25ms sleeps and
per-read one-second timeouts with explicit synchronization/barrier signals that
confirm each provider-cadence update is delivered before publishing the next
milestone. Retain only a generous overall bounded timeout as the failure
watchdog, while preserving the assertion that text items precede the capability
activity.
🪄 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: 39254ed5-8070-46bb-9533-500e33a95d2f
📒 Files selected for processing (5)
crates/ironclaw_host_api/src/product_surface.rscrates/ironclaw_product/src/projection/tests/live_progress_stream.rscrates/ironclaw_webui/src/webui_v2/handlers.rsscripts/ci/reborn_changed_coverage.pyscripts/ci/test_reborn_changed_coverage.py
| fn sse_keep_alive_event() -> Event { | ||
| Event::default() | ||
| .event(WebChatV2Event::KeepAlive.event_name()) | ||
| .data(r#"{"type":"keep_alive"}"#) | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep the keep-alive payload tied to the event contract.
The event name is enum-derived, but the payload is now duplicated as a raw JSON literal. A future WebChatV2Event::KeepAlive wire-shape change could make the event name and payload disagree. Reuse typed serialization or add an explicit transport contract test for this exact frame.
As per path instructions, ephemeral heartbeat variants require serialization, ordering, and transport tests.
🤖 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_webui/src/webui_v2/handlers.rs` around lines 1306 - 1311,
Update sse_keep_alive_event to derive its payload from the typed
WebChatV2Event::KeepAlive serialization instead of duplicating a raw JSON
literal, ensuring the event name and wire payload remain aligned. Add or update
serialization, ordering, and transport tests for the keep-alive heartbeat frame
as required by the event contract.
Source: Path instructions
…chat-stream # Conflicts: # .github/workflows/reborn-tests.yml
…retry, stale selectors (nearai#7070) * fix(webui): unblock main E2E coverage — SSE keep_alive cursor, admin pagination retry, stale selectors Main Code Coverage has been red since nearai#6876 (9792a9f, Jul 30) due to five Reborn WebUI v2 E2E tests. This fixes all five in one change; only the SSE keep_alive cursor fix and the admin load-more retry are prod behavior changes (both correctness fixes for bugs shipping to users). The rest are test-only. 1. SSE keep_alive frames must not carry a Last-Event-ID cursor (test_reborn_v2_sse_reconnect_resumes_without_gap_or_duplicate_served) `webchat_sse_event_from_envelope` stamped the envelope's projection cursor into the SSE `id:` field on every frame, including keep-alives. The product seam advances the cursor on `KeepAlive` payloads (projection.rs push_turn with KeepAlive), so when a keep-alive was the last frame before a disconnect, the browser's `EventSource` echoed its inflated cursor back as `Last-Event-ID` on reconnect and the resumed stream skipped real events that preceded it. The reconnect test saw `(12, 1, 0) == (None, 0, 6)` — a keep-alive cursor vs the real terminal cursor. Fix: omit `event.id(...)` when the frame is `KeepAlive`. Keep-alives are liveness pings, not resume positions; the browser keeps the last real event's id as the resume point. Added `stream_events_keep_alive_frame_carries_no_sse_id` pinning the wire contract, and confirmed the preceding real event still carries its id. 2. Tool-gate run artifact 404 after gate resolution (test_reborn_v2_approval_gate_decline_has_no_successful_tool_result, test_reborn_v2_manual_token_auth_gate_resolves_and_resumes) `_wait_for_run_artifact_status` called `_fetch_run_artifact`, which asserted `status_code == 200` on the first call. After a gate resolve resumes a run, the terminal turn record and its projection can lag the artifact read by a short window, so the first 404 was fatal for the whole 60s polling loop. This is a read-after-write projection lag, not a permanent authorization/routing failure. Fix: split `_try_fetch_run_artifact` that raises `_ArtifactNotReady` on 404, and have `_wait_for_run_artifact_status` tolerate transient 404s inside the polling loop (retry until deadline, surface the last 404 in the failure message if it never lands). Non-404 non-200 responses still assert immediately. No prod change. 3. Admin users load-more must retry transient failures (test_admin_users_ui_paginates_retries_and_deduplicates) Since nearai#6908 (fe8f5c2), `loadMore` in `useAdminUsers` uses a manual `fetchAdminUsers(...).catch(...)` with no retry, but the E2E test pins the user-facing "retries safely" contract (expects 2 attempts on a 503 before surfacing the structured load-more error). The stale React Query retry assumption (inherited from the pre-nearai#6908 infinite-query path) dropped that behavior. Fix: restore a single retry on the cursor page load, narrowed to transient errors (429, 5xx, or `payload.retryable === true`); 4xx authorization/validation failures fail fast. Added a VM test pinning two cursor fetches after a transient rejection and one fetch for a non-retryable 403. 4. Extension card state label is "finish setup", not "setup needed" (test_reborn_v2_current_extension_setup_and_delivery_matrix) The i18n key `extensions.state.setup_needed` renders as "finish setup" in en.ts; the E2E still asserted the retired literal "setup needed". Updated two assertions in the legacy extensions matrix to the current label. No prod change. Test Strategy - Unit (Rust): `cargo test -p ironclaw_webui --test webui_v2_handlers_contract` — 125 passed, incl. new `stream_events_keep_alive_frame_carries_no_sse_id`. - Unit (TS): `pnpm vitest run src/pages/admin` — 49 passed, incl. new retry/fail-fast tests. - Clippy: `cargo clippy -p ironclaw_webui --tests -- -D warnings` — clean. - Typecheck: `pnpm typecheck` — clean. - Architecture: no dependency ownership change (test-only + handler-local). - E2E: pending CI on the PR (Playwright scenarios not run locally). Compatibility / rollback - SSE keep_alive: no wire breaking change; clients that ignored keep_alive ids are unaffected. Clients that resumes from keep_alive ids was already broken. Rollback = revert the handler hunk. - Admin retry: one extra fetch on transient cursor failures only; 4xx unchanged. Rollback = revert useAdminUsers.ts hunk. - E2E-only changes: revert freely. Refs: nearai#6876 (trigger), runs 30550182236 → 30816140609 (chronic red). * ci(reborn-pr-plan): skip tests/e2e/ paths as E2E-workflow-owned The Reborn PR test planner raised `unmapped test or CI path` for any `tests/e2e/...` path (including `tests/e2e/scenarios/`), failing the `Detect Reborn test scope` job on PRs that touch E2E scenarios. E2E scenarios are owned by the dedicated `reborn-e2e.yml` workflow, which runs its own changed-path filter and provider/shard matrix; they are not part of the crate-bucket / root-partition / integration-lane plan emitted by `reborn_pr_test_plan.py`. Skip `tests/e2e/` paths with a reason instead of failing closed, mirroring how the planner already defers frontend, stress-tool, and fixture paths to their owning workflows. Added a planner test pinning that a scenario-only change schedules no crates/lanes and records the E2E ownership reason. Test Strategy - Unit: `python3.11 scripts/ci/test_reborn_pr_test_plan.py` — 33 passed. - Reproduced the CI failure locally with the PR's changed files, then confirmed the skip resolves it. * test(review): retain transient 404 detail in artifact polling; cover mixed E2E+crate plan Address CodeRabbit review on nearai#7070: 1. `_wait_for_run_artifact_status` cleared `last_not_ready` on the first non-terminal 200, so a `404 -> non-terminal 200 -> timeout` sequence reported `transient_404=None` and hid the earlier miss. Stop clearing it; the transient 404 detail now survives through the timeout message. Added a regression test `test_wait_for_run_artifact_status_preserves_transient_404_through_timeout` that scripts the 404 -> non-terminal 200 -> timeout sequence with a mock client and asserts the AssertionError includes both the 404 detail and the non-terminal last artifact. 2. The planner test only covered an E2E-scenario-only change. Added `test_reborn_e2e_and_crate_changes_keep_both_owners` proving a mixed PR (E2E scenario + changed crate) skips the E2E path (owned by reborn-e2e.yml) while still scheduling the crate in the affected crate buckets. Test Strategy - Unit (py): `python3.11 scripts/ci/test_reborn_pr_test_plan.py` — 34 passed. - Unit (py logic): verified the artifact-polling regression test's assertion logic with an inline replica of the helper + mock client (3 scripted calls, timeout, message retains `transient_404=status 404` and the non-terminal `Running` artifact). * test(review): drop redundant @pytest.mark.asyncio marker (asyncio_mode=auto) * fix(ci): preserve Reborn E2E harness fail-closed mapping
…i#6876) * fix(webui): keep live projection subscription active * fix(webui): restore provider-rate text streaming * fix(llm): finalize terminal NEAR AI text streams * Revert "fix(llm): finalize terminal NEAR AI text streams" This reverts commit 5f3ab32. * fix(webui): preserve intermediate model phases * fix(webui): render active replies with streamdown * fix(webui): keep one projection subscription per stream * fix(webui): prevent streamed markdown transition starvation * fix(webui): conflate cumulative text at paint cadence * fix(webui): delegate SSE transport lifecycle * fix(webui): compact intermediate activity phases * docs(webui): clarify intermediate phase boundary * test(webui): emulate packaged SSE transport * fix(webui): keep activity runs collapsed * fix(webui): address streaming review feedback * ci: retry Railway preview deployment * test(webui): follow continuous stream in gate refresh * ci: reduce Reborn PR long tails * Revert "ci: reduce Reborn PR long tails" This reverts commit 3dc16f8. * test(ci): satisfy changed-line coverage gate * fix(ci): compare coverage against PR head
…retry, stale selectors (nearai#7070) * fix(webui): unblock main E2E coverage — SSE keep_alive cursor, admin pagination retry, stale selectors Main Code Coverage has been red since nearai#6876 (9792a9f, Jul 30) due to five Reborn WebUI v2 E2E tests. This fixes all five in one change; only the SSE keep_alive cursor fix and the admin load-more retry are prod behavior changes (both correctness fixes for bugs shipping to users). The rest are test-only. 1. SSE keep_alive frames must not carry a Last-Event-ID cursor (test_reborn_v2_sse_reconnect_resumes_without_gap_or_duplicate_served) `webchat_sse_event_from_envelope` stamped the envelope's projection cursor into the SSE `id:` field on every frame, including keep-alives. The product seam advances the cursor on `KeepAlive` payloads (projection.rs push_turn with KeepAlive), so when a keep-alive was the last frame before a disconnect, the browser's `EventSource` echoed its inflated cursor back as `Last-Event-ID` on reconnect and the resumed stream skipped real events that preceded it. The reconnect test saw `(12, 1, 0) == (None, 0, 6)` — a keep-alive cursor vs the real terminal cursor. Fix: omit `event.id(...)` when the frame is `KeepAlive`. Keep-alives are liveness pings, not resume positions; the browser keeps the last real event's id as the resume point. Added `stream_events_keep_alive_frame_carries_no_sse_id` pinning the wire contract, and confirmed the preceding real event still carries its id. 2. Tool-gate run artifact 404 after gate resolution (test_reborn_v2_approval_gate_decline_has_no_successful_tool_result, test_reborn_v2_manual_token_auth_gate_resolves_and_resumes) `_wait_for_run_artifact_status` called `_fetch_run_artifact`, which asserted `status_code == 200` on the first call. After a gate resolve resumes a run, the terminal turn record and its projection can lag the artifact read by a short window, so the first 404 was fatal for the whole 60s polling loop. This is a read-after-write projection lag, not a permanent authorization/routing failure. Fix: split `_try_fetch_run_artifact` that raises `_ArtifactNotReady` on 404, and have `_wait_for_run_artifact_status` tolerate transient 404s inside the polling loop (retry until deadline, surface the last 404 in the failure message if it never lands). Non-404 non-200 responses still assert immediately. No prod change. 3. Admin users load-more must retry transient failures (test_admin_users_ui_paginates_retries_and_deduplicates) Since nearai#6908 (fe8f5c2), `loadMore` in `useAdminUsers` uses a manual `fetchAdminUsers(...).catch(...)` with no retry, but the E2E test pins the user-facing "retries safely" contract (expects 2 attempts on a 503 before surfacing the structured load-more error). The stale React Query retry assumption (inherited from the pre-nearai#6908 infinite-query path) dropped that behavior. Fix: restore a single retry on the cursor page load, narrowed to transient errors (429, 5xx, or `payload.retryable === true`); 4xx authorization/validation failures fail fast. Added a VM test pinning two cursor fetches after a transient rejection and one fetch for a non-retryable 403. 4. Extension card state label is "finish setup", not "setup needed" (test_reborn_v2_current_extension_setup_and_delivery_matrix) The i18n key `extensions.state.setup_needed` renders as "finish setup" in en.ts; the E2E still asserted the retired literal "setup needed". Updated two assertions in the legacy extensions matrix to the current label. No prod change. Test Strategy - Unit (Rust): `cargo test -p ironclaw_webui --test webui_v2_handlers_contract` — 125 passed, incl. new `stream_events_keep_alive_frame_carries_no_sse_id`. - Unit (TS): `pnpm vitest run src/pages/admin` — 49 passed, incl. new retry/fail-fast tests. - Clippy: `cargo clippy -p ironclaw_webui --tests -- -D warnings` — clean. - Typecheck: `pnpm typecheck` — clean. - Architecture: no dependency ownership change (test-only + handler-local). - E2E: pending CI on the PR (Playwright scenarios not run locally). Compatibility / rollback - SSE keep_alive: no wire breaking change; clients that ignored keep_alive ids are unaffected. Clients that resumes from keep_alive ids was already broken. Rollback = revert the handler hunk. - Admin retry: one extra fetch on transient cursor failures only; 4xx unchanged. Rollback = revert useAdminUsers.ts hunk. - E2E-only changes: revert freely. Refs: nearai#6876 (trigger), runs 30550182236 → 30816140609 (chronic red). * ci(reborn-pr-plan): skip tests/e2e/ paths as E2E-workflow-owned The Reborn PR test planner raised `unmapped test or CI path` for any `tests/e2e/...` path (including `tests/e2e/scenarios/`), failing the `Detect Reborn test scope` job on PRs that touch E2E scenarios. E2E scenarios are owned by the dedicated `reborn-e2e.yml` workflow, which runs its own changed-path filter and provider/shard matrix; they are not part of the crate-bucket / root-partition / integration-lane plan emitted by `reborn_pr_test_plan.py`. Skip `tests/e2e/` paths with a reason instead of failing closed, mirroring how the planner already defers frontend, stress-tool, and fixture paths to their owning workflows. Added a planner test pinning that a scenario-only change schedules no crates/lanes and records the E2E ownership reason. Test Strategy - Unit: `python3.11 scripts/ci/test_reborn_pr_test_plan.py` — 33 passed. - Reproduced the CI failure locally with the PR's changed files, then confirmed the skip resolves it. * test(review): retain transient 404 detail in artifact polling; cover mixed E2E+crate plan Address CodeRabbit review on nearai#7070: 1. `_wait_for_run_artifact_status` cleared `last_not_ready` on the first non-terminal 200, so a `404 -> non-terminal 200 -> timeout` sequence reported `transient_404=None` and hid the earlier miss. Stop clearing it; the transient 404 detail now survives through the timeout message. Added a regression test `test_wait_for_run_artifact_status_preserves_transient_404_through_timeout` that scripts the 404 -> non-terminal 200 -> timeout sequence with a mock client and asserts the AssertionError includes both the 404 detail and the non-terminal last artifact. 2. The planner test only covered an E2E-scenario-only change. Added `test_reborn_e2e_and_crate_changes_keep_both_owners` proving a mixed PR (E2E scenario + changed crate) skips the E2E path (owned by reborn-e2e.yml) while still scheduling the crate in the affected crate buckets. Test Strategy - Unit (py): `python3.11 scripts/ci/test_reborn_pr_test_plan.py` — 34 passed. - Unit (py logic): verified the artifact-polling regression test's assertion logic with an inline replica of the helper + mock client (3 scripted calls, timeout, message retains `transient_404=status 404` and the non-terminal `Running` artifact). * test(review): drop redundant @pytest.mark.asyncio marker (asyncio_mode=auto) * fix(ci): preserve Reborn E2E harness fail-closed mapping
Summary
keep_aliveframes every 15 seconds while a subscription is idle so the browser watchdog sees healthy liveness; Axum comments continue protecting proxies.streamdown@2.5.0; completed replies retain marked + DOMPurify.Last-Event-ID, abort, and retry/backoff toevent-source-plus@0.1.15over fetch/ReadableStream. React retains only application policy such as connection status, terminal error classification, visibility, and active-run stall recovery.Authorizationheader rather than the request URL, and incrementconnection_generationfor every package-managed request.Change Type
Linked Issue
None — reported through Railway preview testing.
Validation
cargo fmt --all --checkcargo clippy -p ironclaw_host_api -p ironclaw_product -p ironclaw_webui --all-targets --all-features -- -D warningscargo build— not run separately; test and clippy builds covered affected crates and targets.cargo test -p ironclaw_host_apicargo test -p ironclaw_product --test reborn_services_contract -- --skip trace_reads_are_available_as_product_views(248 passed)cargo test -p ironclaw_webui --all-featurescargo test -p ironclaw_architecturepnpm lintpnpm typecheckpnpm build(/chat208.1 KB gzip, 1.9 KB budget headroom)git diff --checkcargo clippy --all --benches --tests --examples --all-featurescargo test --features integration— not applicable: no database, persistence backend, runtime lane, WASM, or MCP behavior changed.931d6b8e5, synchronized Railway wire/DOM capture observed roughly 690 cumulative text snapshots in about one minute while the browser showed the phase only after terminal timeline refresh. This proved backend delivery remained lossless but left the client reception path unresolved.9761db3ee, a 16 ms producer coalescer still produced zero visible continuation updates for 55 seconds. This ruled out the producer queue/coalescer as the root cause.b498051b0, after replacing the custom native EventSource lifecycle, a fresh Railway run produced 210 visible DOM changes in 55 seconds.c4c7b98d5, the canonical PR-6876 Railway preview rendered five activity groups and five intermediate utterances with a consistent 20 px row gap, zero empty intermediate rows, and zero intermediate timestamp/copy rows. The final reply upgraded todata-final-reply=trueand retained its timestamp/copy controls.a0a97d8c8, a deterministic.invalidHTTP request producedActivity - 1 tool, 1 failedwhile the toggle remainedaria-expanded=falseand zero detail rows were mounted. Manual expansion exposed the failure details, and manual collapse restored zero mounted detail rows.review-prorpr-shepherd --fixwas run before requesting review.Additional dependency evidence:
streamdown@2.5.0is Apache-2.0.event-source-plus@0.1.15is MIT-licensed and passed the repository supply-chain policy during installation.pnpm audit --prodreports existing React Router advisories; the new SSE package is not on those advisory paths.Local environment limitations:
CARGO_PROFILE_TEST_DEBUG=0 CARGO_INCREMENTAL=0 cargo test --workspace --libbuilt the full workspace and ran the library suites, but finished non-green on two untouched Trace Commons enrollment tests inironclaw_host_runtime(543 passed; 2 failed) because this environment returnedNetworkDeniedinstead of the expected unenrolled guidance.bash scripts/pre-commit-safety.shno longer flags files added by the review pass, but remains non-green on five pre-existing findings elsewhere in this PR branch (filesystem/runtime/composition changes outside this streaming fix).trace_reads_are_available_as_product_viewsfails unchanged in isolation because this workstation has a Trace Commons enrollment and the test reaches its external account endpoint instead of the expected unenrolled zero-state. All other 248 product service contracts pass.SecItemCopyMatching) before reaching its streaming assertion. CI and Railway provide production-composition verification.Test Strategy
User behavior:
Given an active chat using a streaming model, when the provider emits cumulative assistant text and tool milestones, the browser renders text continuously, preserves intermediate model phases, and transitions the latest phase in place to sanitized final Markdown. The user must not need to switch threads to reveal buffered output.
Risk areas:
Tests added or updated:
cfg(test)modules while still failing closed for absent production sources.b498051b0streamed a fresh tool-using run through 210 visible DOM changes in 55 seconds. An existing-thread follow-up—the reported zero-update case—streamed through 57 visible changes in 55 seconds. Intermediate phases remained ordered and visible through terminal completion.What the tests prove:
Last-Event-ID, and abort; each reconnect receives a fresh connection generation.Security Impact
The stream remains caller-scoped. The product layer validates visibility before opening the projection subscription and revalidates it every second on an independent control task; revocation closes the continuation with a redacted product error. The continuation field is
serde(skip)and covered by a wire-contract test.The fetch-based SSE client sends the bearer through the
Authorizationheader instead of a URL query parameter. Active model-authored Markdown passes through Streamdown's streaming renderer; completed replies retain marked + DOMPurify.Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests: the process-local continuation isserde(skip)and a contract test proves it never crosses the wire.Database Impact
None. No schema, migration, PostgreSQL, or libSQL behavior changed.
Blast Radius
Touches the host/product event-stream response, product projection bridge, Axum SSE forwarding, packaged browser SSE lifecycle, live assistant-text projection, and active Markdown presentation. A regression could delay delivery, close a stream early, or regress Markdown display. Memory remains bounded, Streamdown remains lazy-loaded, and the production bundle remains within budget.
Rollback Plan
Revert the streaming commits on this branch to restore one-shot product polling, the previous native EventSource lifecycle, the prior producer behavior, and the prior active Markdown renderer. No data or schema rollback is required.
Review Follow-Through\n\n- CI closeout on exact head
7c8024a65: all 59 reported checks pass (9 conditional jobs skipped); Railway and CodeRabbit are green.\n- Changed production-line coverage passes at 90.96% (322/354) against the exact PR head. The first post-merge coverage lane hit a Docker Hub connection reset while pullingpostgres:16-alpine; GitHub’s failed-job-only retry passed without a code change.\nRailway deployed exact head3030f4958with frontend assetassets/app-B3p3T4ll.jsafter one transient 19-second deployment failure was retried with an empty CI-only commit.aria-expanded=falseand no reconnect state appeared..invalidHTTP request producedActivity - 1 tool, 1 failedwhile the run was active; the failed tool and subsequent assistant/reasoning update left the group collapsed with zero mounted detail rows.Review track: C (security-sensitive product/WebUI streaming transport and active Markdown rendering)