diff --git a/CHANGELOG.md b/CHANGELOG.md index 13c35406..282a2c78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,13 +3,7 @@ ## [Unreleased] -### Fixed - -- **The "Clarify endpoint unavailable. Please restart server." toast no longer fires falsely.** `/api/clarify/pending` always returns HTTP 200 when present (it answers `{"pending": null}` for an unknown session — it never 404s), yet the front-end poller warned "restart the server" on *any* caught error whose message merely contained "404" or "not found". An unrelated stale-session 404 (`Session not found`, e.g. an old-profile session still polling briefly after a profile switch) or a transient error therefore surfaced a misleading missing-endpoint toast pointing operators at the wrong layer. Clarify polling now branches on the structured HTTP status (`err.status`) that `api()` attaches: a `404 Session not found` is handled as a stale-session poll (stop + hide the card silently), and the restart-server warning fires only on a genuine route-not-found 404 whose body is not session-scoped. Poll failures are also logged with the request path, HTTP status, polling session id, and current session id for diagnosis. Thanks @claw-io for the report and @ruizanthony for the initial profile-switch fix. (#5345, absorbs #5343) - -- **Stream cancellation now records its provenance.** `cancelStream()` / `cancelSessionStream()` log a `[stream] cancel requested` line with the trigger (`composer-stop`, `slash-stop`, `slash-interrupt`, `busy-interrupt`, `sidebar-stop`) so a backend interrupt (SIGINT / exit code 130) can be attributed to an explicit user action. Passive UI lifecycle events (session switch, tab hide, page unload) continue to tear down only the local SSE transport and never send an interrupt to the running agent or tool — only explicit Stop/interrupt paths do. (#5345) - -_Entries are moved into their version block when a release is tagged._ +_No unreleased changes. Entries are moved into their version block when a release is tagged._ ## [v0.51.792] — 2026-07-01 diff --git a/static/messages.js b/static/messages.js index 5f9d580a..d2fc7309 100644 --- a/static/messages.js +++ b/static/messages.js @@ -7534,25 +7534,23 @@ function _startClarifyFallbackPoll(sid) { // a missing clarify endpoint. (#5345) const status = (e && typeof e.status === "number") ? e.status : null; const currentSid = (S.session && S.session.session_id) || null; - // Structured diagnostics: a clarify poll failure should be inspectable - // without guessing from a toast — log the failed path, HTTP status (if - // any), the session id we were polling, and the current active session - // id. (#5345) - if (typeof console !== "undefined" && console.warn) { - console.warn("[clarify] pending poll failed", { - path: "/api/clarify/pending", - status: status, - pollingSessionId: sid, - currentSessionId: currentSid, - message: msg, - }); - } - // A 404 whose body is "Session not found" is a STALE-SESSION signal — e.g. + const logDetails = { + path: "/api/clarify/pending", + status: status, + pollingSessionId: sid, + currentSessionId: currentSid, + message: msg, + }; + // A 404 from the active session domain is a STALE-SESSION signal — e.g. // the old profile's session still polling briefly after a profile switch, // or a session deleted server-side — NOT a missing clarify endpoint. Stop // this stale poll and hide its card silently instead of telling the user - // to restart the server. (#5343 / #5345) - if (status === 404 && /session\s+not\s+found/i.test(msg)) { + // to restart the server. Keep this branch before any warn-level logging: + // routine profile switches must not fill DevTools with expected warnings. + // (#5343 / #5345) + const isSessionScoped404 = status === 404 + && (/session/i.test(msg) || (currentSid !== null && currentSid !== sid)); + if (isSessionScoped404) { _clearClarifyPendingForSession(sid); _hideClarifyCardIfOwner(sid, true, 'session'); stopClarifyPolling(); @@ -7569,14 +7567,26 @@ function _startClarifyFallbackPoll(sid) { // a session-scoped 404. (#5345) const isMissingEndpoint = status === 404 && /(^|\b)not\s+found(\b|$)/i.test(msg) - && !/session/i.test(msg); - if (!_clarifyMissingEndpointWarned && isMissingEndpoint) { - _clarifyMissingEndpointWarned = true; - setComposerStatus("Clarify unavailable on current server build. Restart server."); - if (typeof showToast === "function") { - showToast("Clarify endpoint unavailable. Please restart server.", 5000); + && !isSessionScoped404; + if (isMissingEndpoint) { + if (!_clarifyMissingEndpointWarned) { + _clarifyMissingEndpointWarned = true; + setComposerStatus("Clarify unavailable on current server build. Restart server."); + if (typeof showToast === "function") { + showToast("Clarify endpoint unavailable. Please restart server.", 5000); + } + if (typeof console !== "undefined" && console.warn) { + console.warn("[clarify] pending poll endpoint unavailable", logDetails); + } } stopClarifyPolling(); + return; + } + // Structured diagnostics: unexpected clarify poll failures should be + // inspectable without guessing from a toast. Expected stale-session 404s + // and the handled missing-endpoint route have already returned above. + if (typeof console !== "undefined" && console.warn) { + console.warn("[clarify] pending poll failed", logDetails); } } finally { _clarifyFallbackPollInFlight = false; diff --git a/tests/test_issue5345_clarify_toast_and_interrupt_provenance.py b/tests/test_issue5345_clarify_toast_and_interrupt_provenance.py index f51c66c6..fe3e714a 100644 --- a/tests/test_issue5345_clarify_toast_and_interrupt_provenance.py +++ b/tests/test_issue5345_clarify_toast_and_interrupt_provenance.py @@ -40,7 +40,7 @@ def _clarify_catch_block(): idx = MESSAGES_JS.find("function _startClarifyFallbackPoll") assert idx != -1, "_startClarifyFallbackPoll not found in messages.js" # Grab a generous window covering the catch/finally of the poll tick. - return MESSAGES_JS[idx: idx + 3500] + return MESSAGES_JS[idx: idx + 5500] # ── Part 1: clarify toast false positive ──────────────────────────────────── @@ -77,20 +77,39 @@ def test_clarify_missing_endpoint_requires_404_and_non_session_body(): assert "status === 404" in guard or "status===404" in guard, ( "missing-endpoint guard must require a 404 status" ) - assert "!/session/i.test(msg)" in guard.replace(" ", "") or "!/session/i.test(msg)" in guard, ( + assert "!isSessionScoped404" in guard.replace(" ", ""), ( "missing-endpoint guard must EXCLUDE session-scoped 404s so a " "'Session not found' error is not mistaken for a missing endpoint." ) +def test_stale_session_guard_is_not_tied_to_one_exact_backend_message(): + """The stale-session branch should not depend only on the exact + 'Session not found' wording: session-like 404 bodies and a current-session + mismatch after a profile switch are expected stale polls.""" + block = _clarify_catch_block() + m = re.search(r"const\s+isSessionScoped404\s*=([^;]+);", block, re.DOTALL) + assert m, "expected an isSessionScoped404 guard expression in the catch block" + guard = m.group(1).replace(" ", "") + assert "status===404" in guard, "stale-session guard must require a 404 status" + assert "/session/i.test(msg)" in guard, ( + "stale-session guard should classify session-scoped 404 bodies without " + "requiring the exact 'Session not found' wording" + ) + assert "currentSid!==null&¤tSid!==sid" in guard, ( + "stale-session guard should also cover the profile-switch race where the " + "active session changed while the old poll request was in flight" + ) + + def test_stale_session_404_handled_before_missing_endpoint(): - """A 404 'Session not found' must be treated as a stale-session poll (stop + + """A session-scoped 404 must be treated as a stale-session poll (stop + hide silently), and that branch must appear BEFORE the missing-endpoint warning branch so the warning can never fire for it.""" block = _clarify_catch_block() - stale_idx = block.find("session\\s+not\\s+found") + stale_idx = block.find("isSessionScoped404") warn_idx = block.find("isMissingEndpoint") - assert stale_idx != -1, "stale-session 404 branch (session not found) not found" + assert stale_idx != -1, "stale-session 404 branch (isSessionScoped404) not found" assert warn_idx != -1, "missing-endpoint guard not found" assert stale_idx < warn_idx, ( "the stale-session 404 branch must be evaluated before the " @@ -98,6 +117,23 @@ def test_stale_session_404_handled_before_missing_endpoint(): ) +def test_expected_handled_clarify_404s_do_not_emit_poll_failed_warn(): + """Routine stale-session 404s and handled missing-endpoint 404s should return + before the generic warn-level 'pending poll failed' diagnostic. Expected + profile-switch teardown must not create warn-level DevTools noise.""" + block = _clarify_catch_block() + stale_idx = block.find("if (isSessionScoped404)") + missing_idx = block.find("if (isMissingEndpoint)") + warn_idx = block.find('console.warn("[clarify] pending poll failed"') + assert stale_idx != -1, "expected stale-session branch" + assert missing_idx != -1, "expected missing-endpoint branch" + assert warn_idx != -1, "expected generic unexpected-failure warning" + assert stale_idx < missing_idx < warn_idx, ( + "generic warn-level clarify poll logging must come only after handled " + "stale-session and missing-endpoint branches return" + ) + + def test_clarify_poll_failure_is_logged_with_context(): """Clarify poll failures must be observable: path, status, polling session id, and current session id (maintainer question #2)."""