feat(ui): route email chat through the sidecar /v1/email/query loop (V2-10)#2136
Merged
Conversation
Adds failing tests for the not-yet-implemented EmailSidecarProxy.query_stream
(POST /v1/email/query SSE relay) and cancel_query (POST
/v1/email/query/{run_id}/cancel), covering the timeout tuple, non-2xx
SidecarHTTPError boundary, SSE line parsing/ordering, malformed-JSON loud
failure, the cross-thread on_response cancel hook, and 404-is-benign cancel
semantics.
Streams POST /v1/email/query as parsed canonical SSE event dicts (fixed
10s connect timeout, caller-supplied read_timeout spanning the whole
agent-loop run) and adds cancel_query for POST
/v1/email/query/{run_id}/cancel. The on_response hook hands the live
response object to the caller on first iteration so a cancel path on
another thread can force the blocked read to error out by closing it.
Malformed SSE lines raise loudly rather than being swallowed; a 404 on
cancel is benign (the run already finished).
The SSE handler dropped the #1000 fence-injection hack in favor of setting tool_result.result_data directly (#2109) — TestStructuredRenderInjection tested the deleted buffering/prepending behavior, so it's replaced with TestRenderMapResultData, which exercises pretty_print_json's new _render_card_payload-driven result_data path via the public API and asserts print_final_answer no longer touches card content at all.
Adds the red-phase TDD contract for gaia.ui.email_sidecar.relay, which does not exist yet: relay_query's request-body construction, the canonical -> UI event-shape table (status/token/tool_call/tool_result/needs_confirmation /final/error), the mutating-tool visibility status line, the render-card echo-strip, and crash/no-terminal/cancel/timeout control flow — all against hand-rolled fakes. Also adds an integration harness that boots the real gaia_agent_email FastAPI app on a real uvicorn server (build_query_agent seam swapped for scripted fake agents) to prove the pre-scan render card survives pretty_print_json -> CanonicalTranslator -> real HTTP -> proxy end to end, plus a real-socket timing test proving the SSE stream isn't buffered.
…on hack Adds gaia.ui.email_sidecar.relay.relay_query, translating the sidecar's seven-event canonical /query vocabulary (status/token/tool_call/ tool_result/needs_confirmation/final/error) into the UI's own SSE wire, with direct pinned _emit shapes rather than the print_* methods built for the in-process agent loop. Cancellation registers the live HTTP response on handler.active_relay_response so a cross-thread cancel can force a blocked read to error out; every sidecar exception is caught and translated to a terminal agent_error, never left to reach the Lemonade retry classifier. Mutating tools that execute without a confirmation gate (organize/trash/snooze/preference/voice) get an extra visible status line so persistent changes are never buried in the collapsed activity panel. Also fixes sse_handler.py's pretty_print_json to carry a render-map tool's card payload as result_data directly (SSEOutputHandler is shared runtime with the sidecar's own /query route, so this closes the pre_scan data gap for every /query consumer, not just this UI). This replaces the #1000 fence-injection hack entirely: no more buffering pending render payloads or prepending fenced JSON blocks into the final answer text. _RENDER_TOOL_TO_LANG stays as the shared render-map source; _strip_balanced_json_blobs stays for the relay's echo-strip of any card JSON the model still pastes into its prose.
Locks down the already-shipped agent_type=email behavior in _chat_helpers.py/models.py as characterization tests: the non-streaming 400 fail-loud path, the _dispatch_email_query pre-flight gates (sidecar start, contract-version floor, /query readiness, cancellation), the streaming branch's early-return source shape, AgentStepResponse's render/data fields surviving message_to_response's pydantic round-trip, and an end-to-end relay-fidelity test proving a scripted answer and render-map card persist byte-identical through the full SSE -> captured_steps -> DB -> response pipeline.
Retires the in-process agent_type=email tool-calling loop entirely. The streaming branch is now a self-contained early-exit dispatch (_dispatch_email_query): it starts the sidecar, gates on a MAJOR.MINOR contract-version floor (2.4) so a pre-Hub-upgrade binary gets an actionable error instead of 404ing every turn, waits on the sidecar's first-turn readiness check, then hands off to relay_query and returns before the shared agent trunk (which assumes a constructed in-process agent and would crash on the unbound local). Every manager/proxy exception is caught at this boundary and turned into a terminal SSE error, so a sidecar connection problem can never reach the Lemonade retry classifier and trigger a bogus model reload. Non-streaming email now fails loud with an HTTP 400 pointing at stream=true — the sidecar's /query loop has no non-streaming shape, and there were zero verified callers of the old drained path. Deletes EmailProxyAgent, proxy_agent.py, and its test file — the in-process tool registry it depended on is gone, and every tool it exposed is now served by the sidecar's own agent loop through the relay. Cancellation: /api/chat/cancel and the streaming generator's orphan cleanup both now force-close a live email-relay HTTP response via SSEOutputHandler.close_active_relay_response(), so a relay parked in a blocked socket read reacts to cancellation immediately instead of waiting out its full read_timeout. AgentStepResponse gains render/data fields so a render-map card (currently only the pre_scan_inbox -> email_pre_scan card) rehydrates on session reload instead of being silently dropped by pydantic's default extra='ignore' — populated in captured_steps only when the source tool_result actually carried a render kind, keeping today's summary-only retention for everything else.
cancel_query inherited the general 300s sidecar timeout, so a hung sidecar made Cancel exactly as slow as not cancelling. The cancel POST is best-effort cleanup — the forced response close has already unblocked the relay's reader — so it now uses a dedicated 10s timeout and swallows transport failures (timeout/connection) with a warning log instead of letting them escape past relay_query's SidecarError handling into the generic chat exception classifier. Test-first: the three new cancel_query tests (short timeout, timeout swallowed, connection error swallowed) were run red before the fix.
A cancel landing before the relay registered the live response made the router's forced close a no-op (active_relay_response still None), leaving the relay parked in a blocking socket read for the full read_timeout. Two-part fix: query_stream now POSTs and fires on_response eagerly at call time instead of lazily on first next(), and the relay's registration hook closes the just-registered response when the cancelled flag already raced ahead — so every cancel ordering now interrupts the read promptly. Test-first: the raced-cancel relay test and the eager-registration proxy test were run red before the fix; the stale test pinning lazy registration was retired with the behavior it pinned.
…lose The new real-socket integration test (a local threaded HTTP server streaming chunked SSE, then parking silently) proved the cancel design's core assumption wrong: a cross-thread resp.close() does NOT interrupt a blocked read — CPython's buffered reader holds its lock for the whole blocking read(), and close() waits on that same lock, so the forced close itself blocked the full 30s park. In production that meant Cancel against a silently-hung sidecar hung the /api/chat/cancel route too. close_active_relay_response now peels the underlying TCP socket out of the requests response (urllib3 v1/v2 attribute chains) and calls socket.shutdown(SHUT_RDWR), which bypasses the file-object lock and wakes the parked recv immediately; the relay's stream generator still closes the response on its way out. resp.close() remains only as the fallback for mocked/non-socket responses, keeping existing unit tests honest. The test drives the PRODUCTION closer cross-thread and bounds both the closer's own return time (<1s) and the reader's unblock time (<3s).
relay_query called signal_done() itself AND _run_agent's outer finally fired a second one after the email branch returned — two None sentinels per turn, violating the queue's exactly-once contract (masked because the consumer breaks on the first). The relay no longer signals at all: the turn-level sentinel is owned by _run_agent's outer finally, exactly as for every other agent branch (and matching _dispatch_email_query's own pre-flight error paths, which already relied on it). Test-first: a new _stream_chat_impl-level test drives the REAL relay_query and asserts no stray sentinel is left in the queue after the turn (run red before the fix: found 1 stray). The relay-level done_calls assertions flip to the new never-signals contract; the fidelity test's scripted relay stops mimicking the old double-signal behavior.
… noise The streaming consumer's 'answer overrides chunks' rule had an if-truthy guard, so a final answer that legitimately cleans to empty (a card-echo-only email response) kept the raw chunk-accumulated JSON echo as the persisted assistant message. The answer event now assigns full_response unconditionally: empty is a valid answer. Test-first: a _stream_chat_impl-level test streams a render-card echo as tokens plus an echo-only final and asserts the raw JSON never lands in the DB (run red before the fix: the full echoed card was persisted).
The sidecar's /query error events carry str(exc) verbatim, so the most common consumer failure — Lemonade Server down — reached the user as a raw urllib3 repr with no next step, while every other agent branch gets 'is Lemonade Server running?' guidance. The relay now appends (never replaces) a short actionable hint when the detail matches narrow connection/timeout-shaped patterns; anything else passes through untouched. Root fix (actionable copy sidecar-side) ships via the agent release pipeline and is noted for the PR as a follow-up issue. Test-first: hint tests use realistic requests/urllib3 reprs (Max retries exceeded / NewConnectionError / Read timed out), not friendly literals, plus a no-hint passthrough case; run red before the fix.
Covers the seam's three degenerate paths directly: no active response, a mocked/non-socket response (clean close() fallback), and a close() that raises (swallowed, never fatal).
Red-phase specs for Increment 2 (stateless D1): needs_confirmation event wiring, action+summary card rendering, dismiss-is-local-only behavior, unknown-action humanization safety, metacharacter-literal summary rendering (summary embeds email-derived, attacker-influenced content), history persistence/rehydration through the existing message.cards path, and isolation from the pre-existing blocking PermissionPrompt flow. Four files, none of which reference implementation code that exists yet: - components/render/ConfirmationCard.tsx (component-level contract) - ChatView's needs_confirmation dispatch + PermissionPrompt isolation - MessageBubble rehydration of a persisted needs_confirmation card - the SSE-level AGENT_EVENT_TYPES dispatch contract in services/api.ts All four fail today for feature-absence reasons; the 157 pre-existing webui specs are untouched and still pass.
#2109) Email /query confirmation events were invisible to users — the backend relay emitted needs_confirmation but the frontend dropped it as an unknown SSE event type. It now rides the existing agent-event dispatch (AGENT_EVENT_TYPES + StreamEventType) into the generic #2108 card mechanism: ChatView appends a {render: 'needs_confirmation', data: {action, summary}} card (no run_id/confirm_id passthrough), which renders at stream position, persists onto the finalized message via the generic Message.cards path, and rehydrates from history. ConfirmationCard is deliberately non-blocking (stateless D1: the run is already over server-side): no Approve button — the terminal final answer carries the deterministic-path guidance — and Dismiss is local component state only. Rendering safety: the attacker-influenced summary renders as a plain text node in a <pre> (never markdown/innerHTML), the machine action name is shown verbatim and visually separated, and the humanized label derives from action alone. The blocking permission_request / confirm_id / PermissionPrompt flow is untouched (isolation pinned by test). Implements the frozen red-phase contract from the previous commit untouched: 23 files / 180 vitest tests green (was 4 files red).
…#2109) Deletes the last half of the #1000 fence hack: the backend fence injection went earlier on this branch; this removes the frontend parser — promoteStructuredPayloads/STRUCTURED_PAYLOAD_KINDS (bare-JSON promotion), STRUCTURED_PAYLOAD_LANGS and its stripBogusCodeFences bypass, STRUCTURED_FENCE_RE with RenderedContent's ReactMarkdown bypass, and the pre-override EmailPreScanCard mount. Cards now arrive exclusively via tool_result.render into the card registry; the only input the parser still had was pre-cutover session history, which now degrades to a plain fenced-JSON code block (kept legible by retaining the payload tags in KNOWN_CODE_LANGS). The fence characterization tests are rewritten to pin the post-cutover reality per their own header instructions: fenced payloads render as code blocks (never a card, never a crash), bare payload JSON stays text. EmailPreScanCard's stale mounted-via-fence doc comment and the two doc surfaces claiming fence-parsing 'today' (query-sse-contract §4.2, agent-ui.mdx card-rendering section) flip to shipped tense. Gates: EmailProxyAgent/proxy_agent grep across src/+hub/ empty; no new FastAPI routes in the diff; relay logging carries run_id + exception text only; 180 vitest + 856 pytest + lint all green.
This was referenced Jul 16, 2026
Closed
Collaborator
Author
|
Eval control run complete — gate PASS. The bounded control (benchmark leg, committed basis: Gemma-4-E4B-it-GGUF, limit 250, 1 experiment) scored via Within run-to-run noise of the committed baseline, as expected for a diff that touches zero |
itomek-amd
approved these changes
Jul 16, 2026
Contributor
|
Verdict: Approve This PR retires the in-process No blocking issues found. One thing worth a second look before it bites a non-email agent: the persistence tweak that lets an empty final answer overwrite streamed text was broadened from the email path to every agent type, and only the email case is pinned by a test. 🔍 Technical details🟡 ImportantEmpty- The persistence guard changed from 🟢 Minor
Strengths
|
itomek
added a commit
that referenced
this pull request
Jul 17, 2026
…ailbox tag The multi-mailbox consolidation tags items internally, but _run_prescan strips the tag to satisfy the frozen extra="forbid" contract; the route docstring and exported spec claimed the tag survives. Also drop the stale claim that the Agent UI's pre_scan_inbox tool routes through /prescan — post-#2136 the UI chat runs the agent loop in the sidecar.
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
2 participants
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.
Closes #2109 (epic #2014, breakdown item V2-10).
Before: the Agent UI's email chat ran a reduced in-process tool loop (
EmailProxyAgent— prescan/search/calendar-list/archive/undo only), so the agent's advanced tier was unreachable from chat: "which emails am I waiting on a reply for?" improvised a keyword search and answered wrong (the A1 finding from the v2 E2E round), and confirmation requests were invisible. After: email chat relays the sidecar's full/v1/email/querycanonical SSE loop into the V2-9 render-card pipeline — the realcheck_followups(and every other advanced tool) now runs from chat,needs_confirmationrenders as a visible approval card per the stateless D1 model, andEmailProxyAgentplus the #1000 fence-injection hack are fully retired.Evidence (real-world golden path on an AMD Ryzen AI (Strix Halo) machine, Lemonade 11.0.0, Gemma-4-E4B, live Outlook mailbox)
tool_result.renderthrough the relay ("0 surfaced · 25 informational")check_followupstool (visible in agent activity) instead of a keyword-search improvisationneeds_confirmationcard (action name + literal summary, no approve button per stub contract 2.4) → terminal D1 refusal; no send occurred (server log has zero send lines; the stub never executes gated tools)relay.pylog: "stream closed for cancel … Response ended prematurely"), UI returns to readyThe confirmation card also re-renders from persisted history after a full page reload (generic
render/datacard persistence).Tested by driving the built UI in a real browser against the branch running on the AMD machine (frontend dist built at the branch tip, md5-verified transfer); each screenshot was captured at the moment of the step.
Eval (issue AC: "email eval re-run vs baseline")
This diff touches zero
hub/agents/**files (git diff origin/main..HEAD -- hub/is empty) — the sidecar loop, prompts, and tools the email scorecard measures are byte-identical. A bounded control run (benchmark leg, committed basis: Gemma-4-E4B, limit 250) scored viagaia.eval.scorecard_gateagainst the committedSCORECARD.md: result posted in a follow-up comment (run in flight at PR-open time). The committed scorecard is not regenerated.Tests
util/lint.py --allall critical checks passtest_unit.yml(pathssrc/**,tests/**) +test_electron.ymlwebui vitest job (src/gaia/apps/webui/**) both fire on this diffNotes for reviewers
needs_confirmationcarries no structured args and no token (stateless stub, spec §5/Q4); a fresh-/query"approved, do it" would violate D1's approve-what-you-saw invariant. Approve wiring lands with the D1 sign-off / contract 2.5 follow-up.resp.close()cannot interrupt a blocked read (CPython buffered-reader lock); cancel now peels the raw socket and callsshutdown(SHUT_RDWR), proven by a new real-socket integration test and live on hardware.check_followupstrips Microsoft GraphInefficientFilteron Outlook Sent-folder queries (sidecar-side); (3) sidecar-side actionable error copy for Lemonade-down (the relay now appends a hint at the boundary as mitigation).evidence/2109-golden-pathbranch holds the screenshots and should be purged after merge.