From 312d243bdbea750ffb5f72a9c6d9677b0434acb3 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Sun, 30 Aug 2026 23:29:13 -0700 Subject: [PATCH] [eric] agents: a route across pages runs the loop, and a queued send says it is queued Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01U6zrBsUCNzpMBnov3rTVYV --- .../agents/browser/browser_read_script.py | 33 +++++++ backend/apps/agents/manager/Messaging.py | 16 ++++ backend/apps/help/changelog.py | 15 ++++ backend/tests/test_handle_run_error.py | 7 +- backend/tests/test_queued_send_is_visible.py | 41 +++++++++ backend/tests/test_read_script_multi_hop.py | 87 +++++++++++++++++++ electron/voiceHotkey.js | 25 +++++- electron/voiceHotkeyDeaf.test.js | 47 ++++++++++ .../pages/AgentChat/bubbles/MessageBubble.tsx | 18 +++- .../app/pages/Dashboard/cards/AgentCard.tsx | 31 +++++-- .../cards/agentCardStreamSubscription.test.ts | 40 +++++++++ .../Dashboard/desktop/PillArtifactFrame.tsx | 5 +- .../desktop/pillArtifactTheme.test.ts | 31 +++++++ frontend/src/shared/state/agentsSlice.ts | 19 +++- frontend/src/shared/ws/WebSocketManager.ts | 11 +++ 15 files changed, 408 insertions(+), 18 deletions(-) create mode 100644 backend/tests/test_queued_send_is_visible.py create mode 100644 backend/tests/test_read_script_multi_hop.py create mode 100644 electron/voiceHotkeyDeaf.test.js create mode 100644 frontend/src/app/pages/Dashboard/cards/agentCardStreamSubscription.test.ts create mode 100644 frontend/src/app/pages/Dashboard/desktop/pillArtifactTheme.test.ts diff --git a/backend/apps/agents/browser/browser_read_script.py b/backend/apps/agents/browser/browser_read_script.py index 8a1da46c..ff1fe474 100644 --- a/backend/apps/agents/browser/browser_read_script.py +++ b/backend/apps/agents/browser/browser_read_script.py @@ -85,6 +85,34 @@ P_PROSE_DECLINE_RE = re.compile( re.I) +# A task that names a ROUTE, not a page. The read script stages ONE page and asks one aux call over +# its text, so a "go to A, then click through to B, then to C" task is answerable from page 1 only by +# guessing. Measured live 2026-08-30 on the packaged candidate: a 4-hop Wikipedia task ran the child +# with turns=1 and llm=0ms (the loop never started) and came back reporting INSUFFICIENT for page 2, +# which is ENG-355's shape arriving through the child instead of the orchestrator. +# +# Declining here costs a slower full loop; accepting costs a partial answer dressed as a complete one, +# so this fails toward the loop on purpose. It needs a SEQUENCE, never a bare navigation verb, because +# "go to X and read the heading" is exactly what this path is for. +P_MULTI_HOP_RE = re.compile( + r"\bclick(?:ing)?\s+(?:through|into)\b" + r"|\bfrom\s+(?:there|that\s+page)\b" + r"|\bone\s+at\s+a\s+time\b" + r"|\beach\s+of\s+(?:the(?:se|m)?|those)\b" + r"|\b(?:then|next|after\s+that)\b[^.]{0,60}?\b(?:click|navigate|go\s+to|open|visit|follow)\b" + r"|\b(?:click|navigate|go\s+to|open|visit|follow)\b[^.]{0,60}?\b(?:then|next|after\s+that)\b", + re.I, +) + + +def needs_multi_page(task: str) -> bool: + """True when the task describes a ROUTE across pages, which one staged read cannot answer.""" + t = task or "" + if len(set(re.findall(r'https?://[^\s<>"\')\]]+', t))) >= 2: + return True + return bool(P_MULTI_HOP_RE.search(t)) + + def is_answer(reply: str) -> Optional[str]: """The usable answer text, or None. Declines, empties, and hedge-shaped replies all fail closed to the loop, so a thin extraction can never become a wrong answer.""" @@ -105,6 +133,11 @@ async def run_read_script( t0 = time.monotonic() if aux_client is None or not aux_model: return None + # Bail BEFORE the aux call: a route task cannot be answered from one staged page, and paying for + # the call only buys a confident-sounding partial. + if needs_multi_page(task): + logger.info("[browser-read-script] task spans several pages; running the loop instead") + return None try: from backend.apps.agents.core.aux_llm import safe_resp_text diff --git a/backend/apps/agents/manager/Messaging.py b/backend/apps/agents/manager/Messaging.py index 802690c7..27754d0c 100644 --- a/backend/apps/agents/manager/Messaging.py +++ b/backend/apps/agents/manager/Messaging.py @@ -89,6 +89,12 @@ class Messaging(AgentManagerProtocol): existing = self.tasks.get(session_id) if existing and not existing.done(): # A mid-turn message used to be silently dropped here (no bubble, no trace); queue it and the turn task's done callback replays it. + # Queued is NOT the same as sent, and the difference is the whole user experience: a + # person who types "actually stop, just tell me X" watches the agent carry on for as long + # as the current turn lasts. Measured live 2026-08-30: 11 minutes and 119 further tool + # calls, with the message leaving no trace in the transcript or the API, so the only + # honest reading available to the user was "it ignored me". Pressing Stop flushed it in + # 15s. So say it out loud, and name the control that actually works. self.pending_messages.setdefault(session_id, []).append(QueuedMessage( prompt=prompt, mode=mode, model=model, provider=provider, images=images, context_paths=context_paths, forced_tools=forced_tools, @@ -98,6 +104,16 @@ class Messaging(AgentManagerProtocol): selected_setting_ids=selected_setting_ids, client_message_id=client_message_id, )) + if not hidden: + await ws_manager.send_to_session(session_id, "agent:message_queued", { + "session_id": session_id, + "client_message_id": client_message_id, + "queued": len(self.pending_messages.get(session_id) or []), + }) + logger.info( + f"[queued-send] {session_id}: message held until the running turn ends " + f"({len(self.pending_messages.get(session_id) or [])} waiting); Stop sends it now" + ) return session_changed = False diff --git a/backend/apps/help/changelog.py b/backend/apps/help/changelog.py index 993c1fca..eac75fa6 100644 --- a/backend/apps/help/changelog.py +++ b/backend/apps/help/changelog.py @@ -21,6 +21,21 @@ P_RELEASES: List[ReleaseNote] = [ # Only lines that are true of the built app belong here. This one file feeds the in-app card, the # GitHub body AND the Help agent's context, so a line written for a planned feature becomes the # agent confidently describing something that does not exist. + ReleaseNote( + version="1.7.10-exp.1", + headline="Browser research follows links instead of guessing, and long runs finish what they start.", + highlights=[ + "A browsing task that spans several pages now actually walks them. Asking an agent to start somewhere and follow links through to another page no longer answers from the first page alone.", + "Long jobs hold together. A run that reads, edits and re-runs tests over a hundred steps keeps its place and finishes with the detail intact.", + "Panning the canvas no longer snags when the pointer crosses Settings or the Marketplace, and scrolling a settings row scrolls the row instead of dragging the whole board.", + ], + fixes=[ + "The dictation notice no longer claims the fn key is broken when you simply have not pressed a key yet. It only says so once there is real evidence the key is not getting through.", + "Text boxes inside agent cards are readable in light mode again. The free text field on a question could previously render dark on dark.", + "A blocked or refused request no longer leaves the chat unusable, and the provider's wording is never stored as if the agent had said it.", + "If security software quarantines the bundled agent runtime, the app puts it back itself instead of leaving you to find the file.", + ], + ), ReleaseNote( version="1.7.9", headline="Agents keep going when the connection does not, and the fn key finally works.", diff --git a/backend/tests/test_handle_run_error.py b/backend/tests/test_handle_run_error.py index ca659373..7531485c 100644 --- a/backend/tests/test_handle_run_error.py +++ b/backend/tests/test_handle_run_error.py @@ -61,8 +61,11 @@ def test_cli_missing_shows_repair_card_not_dead_path(monkeypatch): sys_msgs = [m for m in session.messages if m.role == "system"] assert sys_msgs, "expected a system card" card = sys_msgs[-1].content - assert "antivirus" in card - assert "reinstall" in card + # Case-folded: the card opens its second sentence with "Antivirus", and a capital letter is not + # a behaviour change. This assertion was checking the copy's punctuation, not its meaning. + low = card.lower() + assert "antivirus" in low + assert "reinstall" in low # The raw path dump is exactly the unactionable card we're replacing. assert "AppData" not in card diff --git a/backend/tests/test_queued_send_is_visible.py b/backend/tests/test_queued_send_is_visible.py new file mode 100644 index 00000000..0f47b358 --- /dev/null +++ b/backend/tests/test_queued_send_is_visible.py @@ -0,0 +1,41 @@ +"""A message typed at a RUNNING agent is queued, not sent, and that has to be said out loud. + +Measured live on the packaged 1.7.10-exp.1 candidate, 2026-08-30: a course-correction ("actually +stop doing that, forget the inventory") was accepted with HTTP 200 at 55 tool calls, the agent ran +on to 174 over 11 minutes, and the message appeared in NO transcript and NO API field the whole +time. The only reading available to the user was "it ignored me". Stop flushed the queue in 15s and +the redirect was answered correctly, so the machinery is right; the silence was the bug. + +Queueing beat the older behaviour (a silent drop), but silent-queued and silent-dropped look +identical from the outside, which is the thing this pins. +""" + +import asyncio +import inspect + +from backend.apps.agents.manager import Messaging + + +def test_a_queued_send_emits_an_event_the_ui_can_show(): + src = inspect.getsource(Messaging.Messaging.send_message) + assert "agent:message_queued" in src, "a queued send must announce itself over the socket" + # It has to fire on the QUEUE path, not somewhere later that a normal send also reaches. + queue_at = src.index("pending_messages.setdefault") + event_at = src.index("agent:message_queued") + ret_at = src.index("return", event_at) + assert queue_at < event_at < ret_at, "the event belongs between queueing and the early return" + + +def test_the_queue_depth_is_reported_not_just_the_fact(): + src = inspect.getsource(Messaging.Messaging.send_message) + seg = src[src.index("agent:message_queued"):src.index("agent:message_queued") + 400] + assert '"queued"' in seg, "how many are waiting is what tells a user this is piling up" + assert "client_message_id" in seg, "the UI needs it to mark the right optimistic bubble" + + +def test_hidden_machine_sends_stay_silent(): + """Nudges, auth heals and watchdog retries queue too. Announcing those would put harness traffic + in front of the user, which is the opposite of the point.""" + src = inspect.getsource(Messaging.Messaging.send_message) + seg = src[src.index("pending_messages.setdefault"):src.index("agent:message_queued")] + assert "if not hidden:" in seg, "only a human's own send may raise the notice" diff --git a/backend/tests/test_read_script_multi_hop.py b/backend/tests/test_read_script_multi_hop.py new file mode 100644 index 00000000..2d8886dd --- /dev/null +++ b/backend/tests/test_read_script_multi_hop.py @@ -0,0 +1,87 @@ +"""ENG-355 through the CHILD, not the orchestrator. + +The browser read script stages ONE page and answers it with a single aux call, so the big-model +loop never starts. That is the right trade for "open X and read the heading" and the wrong one for +"start at X, then click through to Y, then to Z": page 1 cannot answer a route. + +Measured live on the packaged 1.7.10-exp.1 candidate, 2026-08-30: a 4-hop Wikipedia task ran its +browser child with turns=1 and llm=0ms, and the run came back reporting INSUFFICIENT for page 2. +3 of 16 browser agents that session made zero model calls. + +The guard fails toward the LOOP, which is only slower. Accepting instead yields a partial answer +that reads as a complete one, which is further down the ladder. +""" + +from backend.apps.agents.browser.browser_read_script import is_answer, needs_multi_page + + +def test_a_route_across_pages_declines_the_single_page_read(): + assert needs_multi_page( + "Start at https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol . From that page, " + "CLICK through to the article about HTTP/2, then from there to QUIC." + ) + assert needs_multi_page("Visit these six pages one at a time and report the heading of each") + assert needs_multi_page("Go to the dashboard then click the billing tab") + assert needs_multi_page("read each of these pages and summarise them") + + +def test_two_or_more_distinct_urls_is_a_route(): + assert needs_multi_page("compare https://a.example.com and https://b.example.com") + # The same URL repeated is still one page, so it must not read as a route. + assert not needs_multi_page("open https://a.example.com and quote the title of https://a.example.com") + + +def test_an_ordinary_single_page_read_still_takes_the_fast_path(): + """The innocent cases. A bare navigation verb is exactly what this path exists for, so if any + of these start declining, the read script has been throttled into uselessness.""" + for task in ( + "go to https://example.com and tell me the main heading", + "open the pricing page and read the top tier price", + "what does this page say about refunds?", + "visit the careers site and count the open roles", + "navigate to the docs and quote the install command", + ): + assert not needs_multi_page(task), task + + +def test_the_decline_contract_is_unchanged(): + """The guard is additive: INSUFFICIENT and prose declines must still fail closed.""" + assert is_answer("INSUFFICIENT") is None + assert is_answer("I cannot access the individual product page") is None + assert is_answer("The heading is 'Example Domain'") == "The heading is 'Example Domain'" + # A page legitimately lacking a field is still a real answer, not a decline. + assert is_answer("The page does not show a price for that item.") is not None + + +# The pure-function tests above pass even if nothing CALLS needs_multi_page, which is how the first +# version of this file scored green against a deleted call site. These drive run_read_script itself +# and assert it never touched the page, so the wiring is what is under test. +import asyncio + + +class _RecordingTools: + def __init__(self): + self.calls = [] + + async def __call__(self, name, args, browser_id, tab_id): + self.calls.append(name) + return {"text": "x" * 5000} + + +def _run(task): + from backend.apps.agents.browser.browser_read_script import run_read_script + tools = _RecordingTools() + out = asyncio.run(run_read_script( + aux_client=object(), aux_model="cheap-model", task=task, + browser_id="b1", tab_id="t1", execute_tool=tools, current_url="https://example.com", + )) + return out, tools.calls + + +def test_run_read_script_declines_a_route_without_reading_the_page(): + out, calls = _run( + "Start at https://en.wikipedia.org/wiki/HTTP . From that page CLICK through to HTTP/2, " + "then from there to QUIC, and give me the first sentence of each." + ) + assert out is None, "a route task must fall through to the browser loop" + assert calls == [], f"it must bail BEFORE touching the page, but ran {calls}" diff --git a/electron/voiceHotkey.js b/electron/voiceHotkey.js index 7c4720c9..c6aca989 100644 --- a/electron/voiceHotkey.js +++ b/electron/voiceHotkey.js @@ -30,6 +30,8 @@ const LEGACY_COMBO = process.platform === 'darwin' ? 'Meta+Shift+d' : 'Ctrl+Shif const TAP_FRESH_MS = 200; // How long a tap may stay silent before we stop calling it "awaiting proof" and call it broken. const FN_PROOF_GRACE_MS = 60_000; +// How often to re-ask "was the user typing while the watcher stayed silent?". +const FN_DEAF_POLL_MS = 15_000; const FALLBACK_DEFER_MS = 90; // "Meta+Shift+d" (renderer parts format, same as new_agent_shortcut) -> matcher pieces. @@ -106,6 +108,8 @@ function installVoiceHotkey(getMainWindow) { // What the watcher TOLD us, as opposed to what we guessed from it still being alive. let fnPermission = 'unknown'; let fnWireAlive = false; + // Proof the USER was at the keyboard, which is what makes a silent tap mean anything. + let rendererSawKeys = false; let unusableNotified = false; let lastHotkeyIssue = null; let lastTapKeyMs = 0; @@ -271,9 +275,23 @@ function installVoiceHotkey(getMainWindow) { // Armed is not working. If the tap is still deaf after a spell of real use, that is a dead key, // not a shy one, and the user deserves to hear it rather than keep pressing a key that no // longer does anything (it regressed silently once already). - setTimeout(() => { - if (fnProc && !primaryProven() && !fnWireAlive) notifyPrimaryUnusable('tap-deaf'); - }, FN_PROOF_GRACE_MS); + // "No fn events yet" is NOT evidence of a deaf tap. The watcher only taps flagsChanged, so + // wireAlive needs a MODIFIER press; a user who reads the screen, clicks around, or types a + // lowercase prompt produces none. The old fixed timer therefore told anyone who was merely + // quiet for a minute that their fn key was broken, which is a lying status, and it pushed them + // onto the fallback chord for a key that worked fine (observed 2026-08-30 on a packaged build: + // "granted" then "tap-deaf" on a tap that was never touched). + // + // Deafness is only a fact when the user WAS at the keyboard and the watcher still heard nothing, + // so wait for that pairing instead of for the clock, and keep waiting rather than giving up. + const deafPoll = setInterval(() => { + if (unusableNotified || primaryProven() || !fnProc) { clearInterval(deafPoll); return; } + if (fnWireAlive) { clearInterval(deafPoll); return; } + if (!rendererSawKeys) return; + clearInterval(deafPoll); + notifyPrimaryUnusable('tap-deaf'); + }, FN_DEAF_POLL_MS); + if (typeof deafPoll.unref === 'function') deafPoll.unref(); // macOS's own Globe-key action (emoji picker by default) fires on a quick fn tap alongside us; // tell the renderer once so it can point the user at "Press Globe key to: Do Nothing". require('child_process').exec('defaults read com.apple.HIToolbox AppleFnUsageType', (err, out) => { @@ -431,6 +449,7 @@ function installVoiceHotkey(getMainWindow) { if (input.type !== 'keyDown' || input.isAutoRepeat) return; // The relay is the ONLY path a chord has while app-scoped, so knowing it is actually wired // beats inferring it from an absence of complaints. + rendererSawKeys = true; if (!relayProven.has(contents.id)) { relayProven.add(contents.id); console.log(`[voice] relay live on webContents ${contents.id} (${contents.getType()}), first key=${input.key}`); diff --git a/electron/voiceHotkeyDeaf.test.js b/electron/voiceHotkeyDeaf.test.js new file mode 100644 index 00000000..be520d5c --- /dev/null +++ b/electron/voiceHotkeyDeaf.test.js @@ -0,0 +1,47 @@ +const assert = require('node:assert'); +const test = require('node:test'); +const fs = require('node:fs'); +const path = require('node:path'); + +// The fn watcher taps flagsChanged ONLY, so its "wire alive" proof needs a MODIFIER press. The old +// code declared 'tap-deaf' from a bare 60s timer, which meant a user who simply did not press a +// modifier in the first minute was told their fn key was not reaching OpenSwarm and pushed onto the +// fallback chord. Observed on the packaged 1.7.10-exp.1 candidate, 2026-08-30: "Input Monitoring: +// granted" followed by "fn primary unusable (tap-deaf)" on a tap nobody had touched. +// +// Deafness is only a FACT when the user was demonstrably at the keyboard and the watcher still +// heard nothing. These pin that pairing. +const src = fs.readFileSync(path.join(process.cwd(), 'voiceHotkey.js'), 'utf8'); + +test('tap-deaf is never declared from silence alone', () => { + assert.doesNotMatch( + src, + /if \(fnProc && !primaryProven\(\) && !fnWireAlive\) notifyPrimaryUnusable\('tap-deaf'\)/, + 'the bare timer-only check must be gone: it fires on a healthy tap nobody pressed', + ); +}); + +test('tap-deaf requires evidence the user was actually typing', () => { + assert.match(src, /let rendererSawKeys = false;/, 'the keyboard-activity fact must be tracked'); + assert.match(src, /rendererSawKeys = true;/, 'a focused-window key must record that fact'); + const poll = src.slice(src.indexOf('const deafPoll'), src.indexOf('FN_DEAF_POLL_MS);') + 40); + assert.ok(poll.length > 0, 'the evidence poll must exist'); + assert.match(poll, /if \(!rendererSawKeys\) return;/, + 'no keyboard activity means no verdict, ever'); + assert.match(poll, /notifyPrimaryUnusable\('tap-deaf'\)/, + 'once the pairing holds it must still report, or a genuinely deaf key goes unreported'); +}); + +test('the guard still gives up cleanly once the tap proves itself', () => { + const poll = src.slice(src.indexOf('const deafPoll'), src.indexOf('FN_DEAF_POLL_MS);') + 40); + assert.match(poll, /if \(fnWireAlive\) \{ clearInterval\(deafPoll\); return; \}/, + 'a live wire must stop the poll rather than leave a timer running forever'); + assert.match(poll, /unusableNotified \|\| primaryProven\(\) \|\| !fnProc/, + 'notified, proven, or no watcher must all end the poll'); +}); + +test('the real denial path is untouched', () => { + // input-monitoring-denied is a FACT the watcher reports; it must still fire immediately. + assert.match(src, /if \(fnPermission === 'denied'\) notifyPrimaryUnusable\('input-monitoring-denied'\)/); + assert.match(src, /line\.includes\('no-permission'\)/); +}); diff --git a/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx b/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx index 4dd77c61..5b1575fc 100644 --- a/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx +++ b/frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx @@ -1055,8 +1055,12 @@ const ChatMessageBubble: React.FC = ({ message, editing = false, onSaveEd ? content.slice(0, 200) : JSON.stringify(content).slice(0, 200); - const optimisticStatus = (message as any).optimistic_status as 'pending' | 'failed' | undefined; - const isPending = optimisticStatus === 'pending'; + const optimisticStatus = (message as any).optimistic_status as 'pending' | 'failed' | 'queued' | undefined; + // Queued means the backend has it but the agent is mid-turn and will not see it until that turn + // ends. It reads as pending (dimmed) and says so, because "held" and "ignored" look identical + // otherwise, and Stop is the control that delivers it now. + const isQueued = optimisticStatus === 'queued'; + const isPending = optimisticStatus === 'pending' || isQueued; const isFailed = optimisticStatus === 'failed'; return ( @@ -1093,6 +1097,8 @@ const ChatMessageBubble: React.FC = ({ message, editing = false, onSaveEd overflow: 'hidden', opacity: isPending ? 0.7 : 1, transition: 'opacity 0.2s, border-color 0.2s', + // A queued send needs to say why it is dimmed. Without words, held and ignored look the same. + ...(isQueued ? { position: 'relative' } : {}), // User bubbles ease in instead of popping. Assistant bubbles are left alone on purpose: they reveal by typing, and animating them would flash at the streaming -> committed handoff. Transform+opacity only, so it rides the compositor and never shifts layout or the scroll. ...(isUser && !editing ? { animation: 'msgBubbleEnter 160ms ease-out', @@ -1320,6 +1326,14 @@ const ChatMessageBubble: React.FC = ({ message, editing = false, onSaveEd )} + {isQueued && isUser && ( + + Waiting for the current step to finish. Press Stop to send it now. + + )} setPickerOpen(false)} diff --git a/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx b/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx index 8dbd46eb..709107d9 100644 --- a/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx +++ b/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx @@ -48,6 +48,7 @@ import { extractLatestShowUi, extractPendingAskUi, freezeIfDone, artifactName, h import { useDragEndBackstops } from '../hooks/interaction/useDragEndBackstops'; import { useBrowserPillShot } from '../desktop/useBrowserPillShot'; import { subscribeFollowingBrowsers, isSurfaceFollowing } from '../desktop/followingBrowsers'; +import { shallowEqual } from 'react-redux'; import { useAppDispatch, useAppSelector } from '@/shared/hooks'; import AskQuestionCard from '@/app/pages/AgentChat/tool-ui/AskQuestionCard'; import AgentChat from '@/app/pages/AgentChat/AgentChat'; @@ -56,7 +57,6 @@ import { useClaudeTokens, DarkTokensScope } from '@/shared/styles/ThemeContext'; import { GLASS_SURFACE, GLASS_SURFACE_BLUR, GLASS_SURFACE_TEXT } from '@/shared/styles/glassSurface'; import { useDashboardActive } from '@/shared/hooks/useDashboardActive'; import { useOverlayScrollPassthrough } from '../hooks/interaction/useOverlayScrollPassthrough'; -import { useStreamingMessage } from '@/shared/state/streamingSlice'; import { isCanvasInteractionActive, onCanvasInteractionEnd } from '@/shared/canvasInteractionState'; import { setCardSidecar } from '@/shared/state/workflowsSlice'; import { openWorkflowsApp } from '@/shared/state/dashboardLayoutSlice'; @@ -228,6 +228,10 @@ function summarizeToolInput(toolName: string, toolInput: Record): s } } +/** How much of a streaming message the collapsed card paints. Also the subscription boundary: + * past this many characters the head is stable and the card stops re-rendering. */ +const PREVIEW_CHARS = 120; + function getToolDisplayName(toolName: string): string { const mcp = parseMcpToolName(toolName); if (mcp.isMcp) return mcp.displayName; @@ -605,16 +609,25 @@ const AgentCard: React.FC = ({ }; const lastMessage = session.messages[session.messages.length - 1]; - // Subscribe to this card's own streaming entry so per-character mutations don't churn other cards. - const streamingMessage = useStreamingMessage(session.id); - const isStreaming = !!streamingMessage; + // The card shows a 120-CHARACTER preview, so subscribing to the streaming entry itself made every + // token of a long answer re-render all 1,464 lines of this component, on every streaming card at + // once. AgentChat already solved this (it takes the message id and lets a leaf own the text); the + // equivalent here is to project to what is actually painted. Past the first 120 characters the + // head stops changing, so the card goes quiet for the rest of the stream instead of churning. + const stream = useAppSelector((st) => { + const m = st.streaming.bySession[session.id]; + if (!m) return { on: false, head: '' }; + const body = (m.content || '').slice(0, PREVIEW_CHARS); + return { on: true, head: m.role === 'tool_call' ? `[${m.tool_name || ''}] ${body}` : body }; + }, shallowEqual); + const isStreaming = stream.on; const previewContent = isStreaming - ? (streamingMessage!.role === 'tool_call' - ? `[${getToolDisplayName(streamingMessage!.tool_name || '')}] ${streamingMessage!.content}` - : streamingMessage!.content - ).slice(0, 120) + ? (stream.head.startsWith('[') + ? stream.head.replace(/^\[([^\]]*)\]/, (_m, t) => `[${getToolDisplayName(t)}]`) + : stream.head + ).slice(0, PREVIEW_CHARS) : lastMessage && typeof lastMessage.content === 'string' - ? lastMessage.content.slice(0, 120) + ? lastMessage.content.slice(0, PREVIEW_CHARS) : session.last_message_preview ?? ''; const hasPending = session.pending_approvals.length > 0; const pendingReq = session.pending_approvals[0]; diff --git a/frontend/src/app/pages/Dashboard/cards/agentCardStreamSubscription.test.ts b/frontend/src/app/pages/Dashboard/cards/agentCardStreamSubscription.test.ts new file mode 100644 index 00000000..63a0c59c --- /dev/null +++ b/frontend/src/app/pages/Dashboard/cards/agentCardStreamSubscription.test.ts @@ -0,0 +1,40 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; + +// The collapsed card paints a 120-character preview, but it used to subscribe to the whole streaming +// entry, so every token of a long answer re-rendered all 1,464 lines of AgentCard, for every +// streaming card at once. AgentChat had already fixed the same problem one layer down by taking the +// message id and letting a leaf own the text (AgentChat.tsx, "never to the content"); this card +// never got the treatment, which is why a board of streaming agents felt sluggish. +// +// Found by asking an agent the vague question "the canvas feels sluggish when cards are streaming", +// then verifying its answer against the source, 2026-08-30. +const here = path.join(process.cwd(), 'src/app/pages/Dashboard/cards'); +const src = fs.readFileSync(path.join(here, 'AgentCard.tsx'), 'utf8'); + +test('the card never subscribes to the whole streaming entry', () => { + assert.doesNotMatch(src, /useStreamingMessage\(/, + 'useStreamingMessage returns the entry INCLUDING content, so every delta re-renders the card'); + assert.doesNotMatch(src, /from '@\/shared\/state\/streamingSlice'/, + 'the whole-entry hook should no longer be imported here'); +}); + +test('it projects to what is painted, and compares shallowly', () => { + assert.match(src, /const stream = useAppSelector\(/, 'a projected selector must replace it'); + assert.match(src, /shallowEqual\)/, 'the projection needs shallowEqual or it re-renders on every tick anyway'); + assert.match(src, /import \{ shallowEqual \} from 'react-redux';/); + // The projection must slice INSIDE the selector. Slicing after the fact still hands the component + // a new full string every token, which is the bug wearing a hat. + const sel = src.slice(src.indexOf('const stream = useAppSelector('), src.indexOf('const isStreaming = stream.on;')); + assert.match(sel, /slice\(0, PREVIEW_CHARS\)/, 'the slice belongs inside the selector'); + assert.doesNotMatch(sel, /m\.content(?!\s*\|\|)/, 'the raw body must not escape the selector'); +}); + +test('the preview length has ONE definition', () => { + assert.match(src, /const PREVIEW_CHARS = 120;/); + // Two copies of a boundary is how the subscription and the paint drift apart. + assert.equal((src.match(/slice\(0, 120\)/g) || []).length, 0, + 'no bare 120 literals should remain; they must all read PREVIEW_CHARS'); +}); diff --git a/frontend/src/app/pages/Dashboard/desktop/PillArtifactFrame.tsx b/frontend/src/app/pages/Dashboard/desktop/PillArtifactFrame.tsx index 8532b055..c7bd8879 100644 --- a/frontend/src/app/pages/Dashboard/desktop/PillArtifactFrame.tsx +++ b/frontend/src/app/pages/Dashboard/desktop/PillArtifactFrame.tsx @@ -1,5 +1,6 @@ import React, { useCallback, useEffect, useRef, useState } from 'react'; import Box from '@mui/material/Box'; +import { DarkTokensScope } from '@/shared/styles/ThemeContext'; /** Widest and narrowest a collapsed artifact may get dragged to. */ const MIN_W = 240; @@ -84,6 +85,8 @@ function PillArtifactFrame({ name, children }: Props): React.ReactElement { return ( e.stopPropagation()} onClick={(e: React.MouseEvent) => e.stopPropagation()} @@ -92,7 +95,7 @@ function PillArtifactFrame({ name, children }: Props): React.ReactElement { // shrink to must widen the frame, never cut the widget; clipping is not a size option. sx={{ position: 'relative', width, minWidth: 'min-content', maxWidth: '90vw', '&:hover .osw-artifact-grip': { opacity: 1 } }} > - {children} + {children} { + assert.match(src, /className="osw-artifact dark"/, 'the dark class is the premise of this test'); + assert.match(src, /import \{ DarkTokensScope \}/, 'the dark MUI theme must be imported'); + assert.match(src, /\{children\}<\/DarkTokensScope>/, + 'children must render inside DarkTokensScope, or MUI inputs inherit light-mode text on dark glass'); +}); + +test('the dark class never appears without the scope that pairs with it', () => { + const classAt = src.indexOf('className="osw-artifact dark"'); + const scopeAt = src.indexOf(''); + assert.ok(classAt !== -1 && scopeAt !== -1, 'both halves must be present'); + // Ordering matters: the scope has to wrap the content INSIDE the element carrying the class, + // so a later edit that hoists the scope out of the frame fails here rather than shipping. + assert.ok(scopeAt > classAt, 'the scope must sit inside the element that carries the dark class'); +}); diff --git a/frontend/src/shared/state/agentsSlice.ts b/frontend/src/shared/state/agentsSlice.ts index 15146772..4d9d9402 100644 --- a/frontend/src/shared/state/agentsSlice.ts +++ b/frontend/src/shared/state/agentsSlice.ts @@ -27,7 +27,9 @@ export interface AgentMessage { /** Round-tripped optimistic-bubble id; addMessage dedupes the echo against the placeholder. */ client_message_id?: string; /** Frontend-only optimistic lifecycle; dropped on server-echoed messages. */ - optimistic_status?: 'pending' | 'failed'; + // 'queued' = accepted by the backend but held until the running turn ends. Not the same + // as sent, and a user who cannot tell them apart reads the agent as ignoring them. + optimistic_status?: 'pending' | 'failed' | 'queued'; /** Server-stamped duration/token counts; today only thinking messages set these. */ elapsed_ms?: number; tokens?: number; @@ -876,6 +878,20 @@ const agentsSlice = createSlice({ }); }, + // A send that arrived mid-turn is parked server-side and replays when the turn ends, which can + // be many minutes on a long run. Marking the bubble is what separates "held" from "ignored". + markOptimisticQueued( + state, + action: PayloadAction<{ sessionId: string; clientMessageId: string }>, + ) { + const session = state.sessions[action.payload.sessionId]; + if (!session) return; + const msg = session.messages.find( + (m) => m.client_message_id === action.payload.clientMessageId && m.optimistic_status === 'pending', + ); + if (msg) msg.optimistic_status = 'queued'; + }, + markOptimisticFailed( state, action: PayloadAction<{ sessionId: string; clientMessageId: string }>, @@ -1555,6 +1571,7 @@ export const { addMessage, addOptimisticMessage, markOptimisticFailed, + markOptimisticQueued, recordCompaction, setTurnLabel, clearTurnLabel, diff --git a/frontend/src/shared/ws/WebSocketManager.ts b/frontend/src/shared/ws/WebSocketManager.ts index a4cc121e..94f2ce9d 100644 --- a/frontend/src/shared/ws/WebSocketManager.ts +++ b/frontend/src/shared/ws/WebSocketManager.ts @@ -6,6 +6,7 @@ import { updateSessionName, updateGroupMeta, addMessage, + markOptimisticQueued, addApprovalRequest, removeApprovalRequest, updateSessionStatus, @@ -580,6 +581,16 @@ class WebSocketManager { } break; + case 'agent:message_queued': + // Typed at a running agent: the backend holds it until the turn ends. Say so on the bubble, + // or the only reading available is "it ignored me" (measured: 11 minutes of that). + if (session_id && data.client_message_id) { + store.dispatch(markOptimisticQueued({ + sessionId: session_id, clientMessageId: String(data.client_message_id), + })); + } + break; + case 'agent:message': if (session_id && data.message) { store.dispatch(addMessage({ sessionId: session_id, message: data.message }));