fix(e2e): stabilize coverage suite failures - #3291
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors JobContext creation in the agent dispatcher and enhances the reliability of end-to-end tests. It introduces a chat_job_context helper and updates several E2E tests with polling mechanisms and unique data generation to mitigate flakiness in CI environments. The review feedback suggests increasing the event buffer size in the SSE collection loop and expanding the set of monitored event types to ensure the tests remain stable and performant under various execution paths.
| if len(events_received) > 20: | ||
| break |
There was a problem hiding this comment.
The hard limit of 20 events in collect_sse_events is quite low and may lead to flakiness in coverage-instrumented CI environments. Under heavy tool use or verbose logging, the engine can emit many thinking status updates. If the limit is reached before the authentication or approval event is received, the polling loop in the main task will time out and fail the test. Consider increasing this limit to provide more headroom for stabilization.
| if len(events_received) > 20: | |
| break | |
| if len(events_received) > 100: | |
| break |
| while asyncio.get_running_loop().time() < deadline: | ||
| if any( | ||
| e.get("type") == "onboarding_state" and e.get("state") == "auth_required" | ||
| for e in events_received | ||
| ) or "approval_needed" in [e.get("type", "") for e in events_received]: | ||
| break | ||
| await asyncio.sleep(0.5) |
There was a problem hiding this comment.
The polling loop is missing a check for the gate_required event type (specifically for Authentication resume kind), which is the standard auth signal for the engine v2 path. If the engine takes the v2 path, this loop will wait for the full 45-second deadline before proceeding to assertions, unnecessarily slowing down the test suite. Additionally, using a list comprehension inside the loop is less efficient than a generator expression within any().
deadline = asyncio.get_running_loop().time() + 45
while asyncio.get_running_loop().time() < deadline:
if any(
(e.get("type") == "onboarding_state" and e.get("state") == "auth_required") or
(e.get("type") == "gate_required" and isinstance(e.get("resume_kind"), dict) and "Authentication" in e["resume_kind"]) or
(e.get("type") == "approval_needed")
for e in events_received
):
break
await asyncio.sleep(0.5)References
- To improve performance, avoid redundant computations inside loops. For example, pre-calculate values like text.splitlines() before iterating.
fix(e2e): stabilize coverage suite failures
Summary
JobContextsoplan_updateemits current-thread SSE events and plan-mode checklists render in the web UI.Verification
CARGO_TARGET_DIR=/tmp/ironclaw-fix-target CARGO_INCREMENTAL=0 cargo test --lib --no-default-features --features libsql agent::dispatcher::tests::test_chat_job_context_includes_thread_id_for_sse_scoped_tools -- --exactCARGO_TARGET_DIR=/tmp/ironclaw-fix-target CARGO_INCREMENTAL=0 pytest scenarios/test_plan_mode.py scenarios/test_settings_search.py::test_search_filters_user_rows scenarios/test_skill_oauth_flow.py::TestSSEAuthEvents::test_auth_required_sse_event scenarios/test_sse_reconnect.py::test_refresh_skips_readonly_external_active_thread -v --timeout=120