Release exp-v0.52.45: errored-turn response visible (#5941) + recovery honors model pick (#5924) (#5964)

* fix(streaming): keep errored-turn assistant response visible (#5941)

An errored turn that produced assistant content (tool calls + reasoning)
folded that content into a collapsed worklog above the error card, so the
user read a lone error bubble as "nothing came back". The settled-scene
renderer collapsed the worklog unconditionally, never consulting the
turn terminal_state. Now an errored/failure terminal_state keeps the
worklog expanded by default (unless the user explicitly collapsed it),
while completed turns and genuinely-empty errored turns are unchanged.

Reported by @b3nw.

* fix(review): update #4970 brittle collapsed-expr assertion for #5941

test_issue4970_stream_done_shrink_regression asserted the literal
`collapsed:!keepSettledWorklogOpen` in _renderSettledAnchorSceneForMessage.
#5941 OR'd an errored-turn keep-open term into that expression
(`collapsed:!(keepSettledWorklogOpen||erroredWorklogKeepOpen)`), so the
substring no longer matched and the test failed in the full suite (the PR
updated its own touched tests but missed this pre-existing one).

Re-anchor to `collapsed:!(keepSettledWorklogOpen` — still guards #4970's
invariant (keepSettledWorklogOpen negates into the settled-render collapsed
flag) without pinning the exact term list. #4970 + #5941 suites green.

* fix(chat): honor explicit model pick on post-failure recovery send (#5924)

The onchange explicit-pick marker is single-shot: send() consumes it once.
submitEdit() and cmdRetry() truncated and called send() directly without
re-arming it, so the recovery send went out with explicit_model_pick=false and
the server's compatible-model resolution re-reverted a freshly-picked
cross-family model back to the failed/stale value (Facet 1). Facet 4 is the
same loop keeping the persisted model_provider pinned across fork/refresh.

Re-arm the pending explicit-pick marker from the current selector state
immediately before await send() in both recovery paths. Survives a second
consecutive recovery send. Normal send path and the #3737/#5731 server repair
guard are unchanged.

Reported by @b3nw.

* fix(#5924): gate recovery re-arm on genuine pick + session-race guards

Codex gate found 4 defects on the recovery send path; fixed all:
- /retry + edit-resubmit re-armed the explicit-pick marker UNCONDITIONALLY, forcing
  explicit_model_pick even with no fresh pick (suppressed server compatible-model
  resolution). Now gated on _recoveryPick (selector model differs from session's own
  stored model), captured pre-await.
- session-switch races: added active-session guards after the retry GET await and the
  edit truncate await so session A's recovery intent can't apply to session B.
- 3 new regression tests (gated re-arm, pre-await capture, post-await re-guard).

* fix(#5924) round-2: derive recovery pick from non-default session model, not inference

Codex round-2 CORE: the state-comparison predicate false-negatived an already-applied
pick (consumed marker → looks unchanged) and false-positived on provider inference
(@removed:mistral-large + null stored provider → inferred 'removed' → fake change).
Replace with _deliberateSessionModelPick(sid): reports {model,provider} only when the
session's OWN model is genuinely non-default vs window._defaultModel/_activeProvider —
inference-free + survives marker consumption. Both recovery paths + tests updated.

* fix(#5924) round-3: require known-default+owned-provider evidence + fire-time re-arm guard

Codex round-3: (1) _deliberateSessionModelPick still false-positived when the
profile default was unknown or the provider was only inferred — now requires a
session-OWNED provider AND a known window._defaultModel/_activeProvider, else
fails closed. (2) new same-session race: a model change DURING the recovery awaits
made the pre-await pick stale and clobbered a newer marker — new _reArmRecoveryPick
helper re-arms only if the current session model/provider still equals the captured
pick AND no different pending marker exists. Both recovery paths route through it.

* Release exp-v0.52.45: errored-turn response stays visible (#5950/#5941) + recovery honors model pick (#5949/#5924)

* fix: keep absoluteKeepCount capture before _recoveryPick in submitEdit

The #5924 _recoveryPick comment contained the word 'await', tripping the
pre-existing test_issue_edit_regenerate_absolute_keep_count regex (first \bawait\b
must come after the absoluteKeepCount capture). Reorder both synchronous pre-network
captures (absoluteKeepCount first) + reword the comment. No behavior change.

---------

Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
This commit is contained in:
authored and GitHub committed 2026-07-11 18:07:28 -07:00
1 parent 3a8f980e2b
commit 0037061cfd
6 files changed
+579 -3

No files matched your search

+4
View File
@@ -13,6 +13,10 @@
### Fixed
- **An errored turn no longer hides the response the assistant already produced.** When a turn ended in an error, the tool calls and reasoning the assistant had already generated were auto-collapsed behind a single header above the error card, so it looked like nothing came back. That produced content now stays visible by default on an errored turn (the error card still shows); successful turns collapse their worklog exactly as before. Thanks @b3nw for the report. (#5950, #5941)
- **After a provider failure, retrying or edit-resubmitting now honors your newly-picked model.** If a turn failed and you switched the model/provider in the selector, `/retry` and edit-resubmit re-sent the *failed* model, forcing you to fork the conversation. A genuine non-default model pick is now carried into the recovery send, while an unchanged model still lets the server pick a compatible one — and the recovery can't leak one session's model into another when you switch sessions mid-retry. Thanks @b3nw for the report. (#5949, #5924)
- **Russian localization refreshed and onboarding notes localized.** The `ru` bundle was refreshed and brought to full key parity with English (identical key sets across all locales), new update/workspace UI strings were propagated to every locale, and onboarding provider notes now resolve through a localized key (with the English text preserved as fallback). Thanks @DrMaks22. (#5778)
- **A pending prompt no longer renders twice across a context-compaction boundary.** On reload / reattach to an active turn, a synthetic `[CONTEXT COMPACTION]` marker (a user-role row that isn't a real submitted turn) placed after your prompt was treated as the latest user message, so the tail scan missed the actual prompt right before it and rendered the same pending prompt twice. The session-load and refresh/reconnect tail scans now skip compaction markers when locating the current user message, while completed assistant rows stay hard boundaries so genuinely-repeated prompts still render. Thanks @starship-s. (#5920)
+20 -1
View File
@@ -1680,13 +1680,32 @@ async function cmdRetry(){
if(!S.session){showToast(t('no_active_session'));return;}
if(S.session.is_cli_session){showToast(t('cmd_webui_only_session'));return;}
const activeSid=S.session.session_id;
// #5924: honor a genuine deliberate model pick across a recovery /retry without
// forcing explicit_model_pick when there is no real pick. The single-shot marker
// is already consumed by the failed send (messages.js clears it before
// /api/chat/start regardless of outcome), so we can't read it back. Instead
// derive the deliberate-pick signal the SAME way send()'s persistent path does:
// the session's own model is a non-default pick vs the profile default. This is
// NOT provider inference (no false positive) and survives marker consumption (no
// false negative for an already-applied pick). Captured pre-await, scoped to
// activeSid. No non-default pick → no re-arm → server compatible-model resolution runs.
const _recoveryPick=_deliberateSessionModelPick(activeSid);
try{
const r=await api('/api/session/retry',{method:'POST',body:JSON.stringify({session_id:activeSid})});
if(r&&r.error){showToast(r.error);return;}
if(!S.session||S.session.session_id!==activeSid)return;
const data=await api('/api/session?session_id='+encodeURIComponent(activeSid));
// #5924 SILENT-race guard: a session switch during the GET await must not let
// this recovery apply session A's intent to whatever session is now visible.
if(!S.session||S.session.session_id!==activeSid)return;
if(data&&data.session){S.messages=data.session.messages||[];S.toolCalls=[];if(typeof clearLiveToolCards==='function')clearLiveToolCards();if(typeof _messagesTruncated!=='undefined')_messagesTruncated=false;renderMessages();}
$('msg').value=r.last_user_text||'';if(typeof autoResize==='function')autoResize();await send();
$('msg').value=r.last_user_text||'';if(typeof autoResize==='function')autoResize();
// Re-arm the single-shot explicit-pick marker from the captured non-default
// pick — but only if it's still safe at fire time (session unchanged, current
// model still matches the pick, and no newer onchange marker to clobber). See
// _reArmRecoveryPick. Scoped to activeSid so it can't leak to another session.
_reArmRecoveryPick(activeSid, _recoveryPick);
await send();
}catch(e){showToast(t('retry_failed')+e.message);}
}
async function cmdUndo(){
+104 -1
View File
@@ -2848,6 +2848,65 @@ function _clearPendingSessionModel(sessionId){
if(!sid) return;
try{sessionStorage.removeItem(_pendingSessionModelKey(sid));}catch(_){}
}
// #5924: the recovery-send deliberate-pick signal. Returns {model, model_provider}
// ONLY when the active session's own model is a genuine non-default pick vs the
// profile default — the same signal send()'s persistent-pick path (_isCrossProviderPick)
// uses, generalized to same-provider non-default picks too. Used by the recovery
// paths (cmdRetry / submitEdit) to decide whether to re-arm the single-shot
// explicit-pick marker: the marker is consumed by the failed send before we reach
// recovery, so we can't read it back, and comparing _chatPayloadModel() to itself
// either false-negatives (an already-applied pick looks unchanged) or false-positives
// (provider inference manufactures a "change"). A non-default session model is the
// durable, inference-free evidence of a real pick. Returns null (no re-arm → the
// server's compatible-model resolution runs) when the session is on the default.
function _deliberateSessionModelPick(sessionId){
if(!S.session||S.session.session_id!==sessionId) return null;
const model=String(S.session.model||'').trim();
if(!model) return null;
// Require SESSION-OWNED provider evidence — a stored model_provider on the
// session itself. Do NOT infer a provider from the model string: an
// unreachable/renamed model like "@removed:mistral-large" with no stored
// provider must NOT count as a deliberate pick (round-2/3 false-positive).
const provider=S.session.model_provider?String(S.session.model_provider).trim():'';
if(!provider) return null;
// Require a KNOWN profile default to compare against. If we don't know the
// default (empty window._defaultModel), we can't prove this is a non-default
// pick, so fail closed → no re-arm (server compatible-model resolution runs).
const defaultModel=(typeof window!=='undefined'&&window._defaultModel)?String(window._defaultModel):'';
const activeProvider=(typeof window!=='undefined'&&window._activeProvider)?String(window._activeProvider):'';
if(!defaultModel||!activeProvider) return null;
// Non-default = a different model OR a different provider than the profile
// default. A session sitting exactly on the profile default is NOT a pick.
const isDefault=(model===defaultModel)&&(provider===activeProvider);
if(isDefault) return null;
return {model, model_provider:provider};
}
// #5924: re-arm the single-shot explicit-pick marker from a recovery pick, but
// ONLY if it's still safe at fire time. Guards the SILENT same-session race where
// the user changes the model DURING the recovery's awaits: (1) the session must
// still be the captured one; (2) the session's CURRENT model/provider must still
// equal the captured pick (a mid-flight change means the pick is stale — skip);
// (3) never clobber a NEWER pending marker (an onchange during the await already
// wrote the authoritative one). Returns true if it re-armed.
function _reArmRecoveryPick(sessionId, pick){
if(!pick||!pick.model) return false;
if(!S.session||S.session.session_id!==sessionId) return false;
// Current session state must still match the captured pick (no mid-flight change).
if(String(S.session.model||'')!==String(pick.model||'')
||String(S.session.model_provider||'')!==String(pick.model_provider||'')) return false;
// Do not overwrite a newer marker written by an onchange during the await.
if(typeof _readPendingSessionModel==='function'){
const existing=_readPendingSessionModel(sessionId);
if(existing&&existing.model
&&(String(existing.model)!==String(pick.model)
||String(existing.model_provider||'')!==String(pick.model_provider||''))) return false;
}
if(typeof _rememberPendingSessionModel==='function'){
_rememberPendingSessionModel(sessionId, pick.model, pick.model_provider);
return true;
}
return false;
}
function _applyPendingSessionModelForSession(sessionId){
if(!S.session||S.session.session_id!==sessionId) return false;
const pending=_readPendingSessionModel(sessionId);
@@ -12512,6 +12571,23 @@ function _anchorSceneSceneHasWorklogWorthyRows(scene){
}
return false;
}
// #5941: an errored/failed turn's terminal_state. A turn that ended in a
// provider/agent failure but which DID produce assistant content (tool calls,
// reasoning) still folds that content into a collapsed worklog above the error
// card — so the user reads a lone error bubble as "nothing came back", even
// though the real response is one click away. These are the terminal states
// that must keep the produced content VISIBLE by default. `completed` (normal
// turn) and null are deliberately excluded, and `cancelled`/`interrupted`
// (user-initiated stops with their own dedicated cards + #5224 transcript
// preservation) are left to their existing behavior — this is scoped to the
// error/failure family the report is about.
const _ANCHOR_SCENE_ERRORED_TERMINAL_STATES=new Set([
'error','no_response','degraded','connection_lost','tool_limit_reached','compression_exhausted',
]);
function _anchorSceneHasErroredTerminalState(scene){
const state=String(scene&&scene.terminal_state||'').trim().toLowerCase();
return _ANCHOR_SCENE_ERRORED_TERMINAL_STATES.has(state);
}
function _renderSettledAnchorSceneTransparentForMessage(message, segment, rawIdx){
if(!message||!message._anchor_activity_scene||!segment) return false;
if(!_anchorSceneSceneHasWorklogWorthyRows(message._anchor_activity_scene)) return false;
@@ -12637,6 +12713,19 @@ function _renderSettledAnchorSceneForMessage(message, segment, rawIdx){
if(streamId&&!_readActivityDisclosureState(activityKey)){
_copyActivityDisclosureState(`live:${streamId}`, activityKey);
}
// #5941: an errored turn that produced assistant content (tool calls /
// reasoning) must not hide that content behind a collapsed header — the user
// reads a lone error card as "nothing came back". When the settled scene's
// terminal_state is an error/failure (NOT a normal completion) keep the
// worklog EXPANDED by default so the produced response stays visible. This
// path is only reached for worklog-worthy scenes (the guard at the top
// requires >=1 tool/thinking/compression row), so a genuinely-empty errored
// turn — a real no_response with zero produced content — never gets here and
// still shows only its error card, no phantom empty body. A user who has
// explicitly collapsed THIS turn's worklog (saved 'closed' disclosure state)
// is still respected, so the default-open never fights an intentional collapse.
const erroredWorklogKeepOpen=_anchorSceneHasErroredTerminalState(scene)
&& _readActivityDisclosureState(activityKey)!=='closed';
// keepSettledWorklogOpen forces collapsed:false for the ONE height-stable settle
// render of the just-settled turn (no STREAM_DONE shrink jump) for both pinned
// followers AND unpinned mid-turn readers. The keep-open is made genuinely
@@ -12648,7 +12737,7 @@ function _renderSettledAnchorSceneForMessage(message, segment, rawIdx){
// (_isKeepSettledWorklogOpenArmed), so it never persists across restores.
const group=_anchorSceneWorklogGroup(blocks,{
live:false,
collapsed:!keepSettledWorklogOpen,
collapsed:!(keepSettledWorklogOpen||erroredWorklogKeepOpen),
beforeAnchor:true,
anchor:segment,
activityKey,
@@ -17082,6 +17171,11 @@ async function submitEdit(msgIdx, newText) {
if(!S.session || S.busy) return;
const initialSid = S.session.session_id;
const absoluteKeepCount = _oldestIdx + msgIdx;
// #5924: capture the deliberate-pick signal up front (pre-network), scoped to
// initialSid — a non-default session model (vs profile default), which is
// inference-free and survives the failed send's marker consumption. See
// _deliberateSessionModelPick. null → no re-arm → server resolution runs.
const _recoveryPick=_deliberateSessionModelPick(initialSid);
if(typeof _ensureAllMessagesLoaded==='function'){
await _ensureAllMessagesLoaded();
}
@@ -17091,9 +17185,18 @@ async function submitEdit(msgIdx, newText) {
session_id: initialSid,
keep_count: absoluteKeepCount
})});
// #5924 SILENT-race guard: a session switch during the truncate await must not
// let this recovery apply session A's intent (truncate/re-arm/send) to the
// newly-visible session.
if(!S.session || S.session.session_id !== initialSid) return;
S.messages = S.messages.slice(0, absoluteKeepCount);
renderMessages();
$('msg').value = newText;
// #5924 (Facet 1 + Facet 4): edit-resubmit is a recovery send. Re-arm the
// Re-arm the single-shot explicit-pick marker from the captured non-default
// pick — only if still safe at fire time (session unchanged, current model
// still matches, no newer onchange marker to clobber). See _reArmRecoveryPick.
_reArmRecoveryPick(initialSid, _recoveryPick);
await send();
} catch(e) { setStatus(t('edit_failed') + e.message); }
}
@@ -75,7 +75,12 @@ def test_helper_and_token_threaded_through_render():
)
render_fn = _function_body(UI_JS, "_renderSettledAnchorSceneForMessage")
assert "_shouldKeepSettledWorklogOpenForStreamSettle(streamId)" in render_fn
assert "collapsed:!keepSettledWorklogOpen" in render_fn
# keepSettledWorklogOpen must still drive the settled-render collapsed flag.
# #5941 OR'd in an errored-turn keep-open term, so the expression is now
# `collapsed:!(keepSettledWorklogOpen||erroredWorklogKeepOpen)` — assert the
# invariant (keepSettledWorklogOpen negates into `collapsed`) without pinning
# the exact term list.
assert "collapsed:!(keepSettledWorklogOpen" in render_fn
group_fn = _function_body(UI_JS, "_anchorSceneWorklogGroup")
assert "collapsed:(opts&&opts.collapsed!==undefined)?opts.collapsed:!live" in group_fn
# The STREAM_DONE handler arms one-shot then disarms around the render.
@@ -0,0 +1,256 @@
"""Regression coverage for #5924 — post-failure recovery must honor a fresh pick.
After a provider failure the reporter (@b3nw) could not switch models: changing
the model in the selector and then **edit-resubmit** or **/retry** re-sent the
*failed* model, forcing a session fork to escape.
Root cause (Facet 1 + Facet 4): the onchange explicit-pick marker
(``_rememberPendingSessionModel``) is single-shot — ``send()`` consumes it once
(``messages.js``). The two recovery paths (``submitEdit`` in ``ui.js`` and
``cmdRetry`` in ``commands.js``) truncate and call ``send()`` directly WITHOUT
re-arming the marker, so ``explicit_model_pick`` goes out ``false`` and the
server's ``_resolve_compatible_session_model_state`` re-reverts a freshly-picked
cross-family model back to the profile default.
Two-layer invariant pinned here:
* WebUI: both recovery paths re-arm the pending explicit-pick marker from the
CURRENT selector state *before* ``await send()`` (so a recovery send —
including a SECOND consecutive one — carries ``explicit_model_pick:true``).
* Server: with ``explicit_model_pick=True`` the fresh cross-family pick is
honored (NOT reverted), and without it the stale value is still normalized
(the #3737/#5731 repair path must not regress).
"""
from pathlib import Path
import api.routes as routes
ROOT = Path(__file__).resolve().parents[1]
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
COMMANDS_JS = (ROOT / "static" / "commands.js").read_text(encoding="utf-8")
def _function_body(src: str, name: str) -> str:
# Match either "async function NAME" or plain "function NAME".
start = src.find(f"async function {name}")
if start == -1:
start = src.find(f"function {name}")
if start == -1:
raise AssertionError(f"function {name!r} not found")
brace = src.index("{", start)
depth = 0
for i in range(brace, len(src)):
if src[i] == "{":
depth += 1
elif src[i] == "}":
depth -= 1
if depth == 0:
return src[start : i + 1]
raise AssertionError(f"function {name!r} body not found")
# ── WebUI layer: recovery paths re-arm the marker before send() ──────────────
def test_submit_edit_rearms_pending_pick_before_send():
"""Edit-resubmit must re-arm the explicit-pick marker before ``await send()``."""
body = _function_body(UI_JS, "submitEdit")
assert "_reArmRecoveryPick(" in body, (
"submitEdit must re-arm via _reArmRecoveryPick (#5924); otherwise the "
"recovery send loses explicit_model_pick and the server re-reverts the model"
)
rearm_idx = body.index("_reArmRecoveryPick(")
send_idx = body.rindex("await send()")
assert rearm_idx < send_idx, "the re-arm must happen BEFORE await send()"
def test_cmd_retry_rearms_pending_pick_before_send():
"""/retry must re-arm the explicit-pick marker before ``await send()``."""
body = _function_body(COMMANDS_JS, "cmdRetry")
assert "_reArmRecoveryPick(" in body, (
"cmdRetry must re-arm via _reArmRecoveryPick (#5924); otherwise /retry "
"re-sends the failed model instead of the freshly-picked one"
)
rearm_idx = body.index("_reArmRecoveryPick(")
send_idx = body.rindex("await send()")
assert rearm_idx < send_idx, "the re-arm must happen BEFORE await send()"
def test_recovery_rearm_sources_deliberate_pick_helper():
"""The re-arm must source the deliberate-pick signal from the shared helper.
Both recovery paths capture ``_deliberateSessionModelPick(<sid>)`` (a non-default
session-model signal that is inference-free and survives the failed send's marker
consumption) BEFORE any await, rather than comparing ``_chatPayloadModel()`` to
itself (which false-negatives an already-applied pick and false-positives on
provider inference — the round-2 gate finding).
"""
for body in (_function_body(UI_JS, "submitEdit"), _function_body(COMMANDS_JS, "cmdRetry")):
assert "_deliberateSessionModelPick(" in body, (
"recovery paths must derive the pick via _deliberateSessionModelPick"
)
def test_deliberate_pick_helper_ignores_default_and_inference():
"""The helper only reports a pick for a genuine NON-DEFAULT session model.
It must key off the session's own model vs the profile default (window._defaultModel
/ _activeProvider) — NOT provider inference on an unchanged model, and NOT a
self-comparison. A session on the profile default returns null (no re-arm), so the
server's compatible-model resolution runs for a no-real-pick recovery.
"""
body = _function_body(UI_JS, "_deliberateSessionModelPick")
assert "window._defaultModel" in body and "window._activeProvider" in body, (
"the pick signal must compare against the profile default, not infer a provider"
)
assert "return null" in body, "a default-model session must return null (no re-arm)"
# must NOT resurrect the round-2 false-positive: no provider inference in the signal
assert "_providerFromModelValue" not in body and "_chatPayloadModelProvider" not in body, (
"the deliberate-pick signal must not use provider inference (round-2 false-positive)"
)
# ── Server layer: explicit pick is honored; repair path preserved (#3737) ────
def test_explicit_pick_honors_fresh_cross_family_model_on_recovery():
"""The freshly-picked cross-family model survives when explicit_model_pick=True.
This is the value the re-armed marker carries into /api/chat/start on the
recovery send. It must NOT be reverted to the failed/profile-default model.
"""
effective, provider, changed = routes._resolve_compatible_session_model_state(
"gpt-5.4-mini", # freshly picked, cross-family vs anthropic profile
None,
profile_provider="anthropic",
profile_default_model="claude-sonnet-4",
explicit_model_pick=True,
)
assert changed is False, "an explicit recovery pick must not be reverted"
assert effective == "gpt-5.4-mini", "the freshly-picked model must survive"
assert provider == "anthropic"
def test_second_consecutive_recovery_send_still_honors_pick():
"""A SECOND consecutive recovery send re-arms the marker, so it stays explicit.
send() consumes the marker each time, but both recovery paths re-arm it from
the current selector state on every invocation — so two retries/edits in a
row both carry explicit_model_pick=True and both honor the pick.
"""
for _ in range(2):
effective, provider, changed = routes._resolve_compatible_session_model_state(
"gpt-5.4-mini",
None,
profile_provider="anthropic",
profile_default_model="claude-sonnet-4",
explicit_model_pick=True,
)
assert changed is False
assert effective == "gpt-5.4-mini"
assert provider == "anthropic"
def test_non_explicit_send_still_normalizes_stale_model():
"""Guard against regressing the #3737/#5731 repair path.
Without an explicit pick (the normal 2nd+-turn continuation), a stale
cross-family model is still normalized to the profile default. The #5924 fix
only re-arms on the recovery entry points, so this path is unchanged.
"""
effective, provider, changed = routes._resolve_compatible_session_model_state(
"gpt-5.4-mini",
None,
profile_provider="anthropic",
profile_default_model="claude-sonnet-4",
explicit_model_pick=False,
)
assert changed is True, "stale model must still be normalized on a plain send"
assert effective == "claude-sonnet-4"
assert provider == "anthropic"
# ── #5924 gate re-fixes: gated re-arm + session-race guards ──────────────────
def test_recovery_rearm_is_gated_on_a_genuine_deliberate_pick():
"""The re-arm must be CONDITIONAL on a real pick, not unconditional.
Codex round-1 CORE: re-arming on EVERY recovery send (even with no fresh pick)
forced explicit_model_pick=true and suppressed the server's compatible-model
resolution. Both recovery paths now derive `_recoveryPick` from the shared
_deliberateSessionModelPick helper and re-arm only via _reArmRecoveryPick,
which no-ops on a null/absent pick.
"""
for body in (_function_body(UI_JS, "submitEdit"), _function_body(COMMANDS_JS, "cmdRetry")):
assert "_recoveryPick" in body, "recovery re-arm must be gated on _recoveryPick"
assert "_deliberateSessionModelPick(" in body, (
"the pick must come from _deliberateSessionModelPick, not an inline comparison"
)
assert "_reArmRecoveryPick(" in body, (
"the re-arm must go through _reArmRecoveryPick (fire-time safety guards)"
)
def test_rearm_helper_guards_stale_pick_and_newer_marker():
"""_reArmRecoveryPick must not re-arm a stale pick or clobber a newer marker.
Codex round-3 SILENT: a same-session model change DURING the recovery awaits
made the pre-await captured pick stale; re-arming it overwrote the newer
pending marker and restored the old model on a later session load. The helper
must (1) require the current session model/provider to still equal the pick,
and (2) not overwrite a different existing pending marker.
"""
body = _function_body(UI_JS, "_reArmRecoveryPick")
# still-matches-current-state guard
assert "S.session.model" in body and "pick.model" in body, (
"must confirm the current session model still equals the captured pick"
)
# newer-marker guard
assert "_readPendingSessionModel(" in body, (
"must read the existing pending marker and refuse to clobber a newer one"
)
# session-scope guard
assert "S.session.session_id!==sessionId" in body.replace(" ", ""), (
"must confirm the session is still the captured one before re-arming"
)
def test_recovery_pick_is_captured_before_the_first_await():
"""_recoveryPick must be captured BEFORE any awaited network call so a session
switch during the recovery's round-trips can't make it read the wrong
session's selector state. The `_recoveryPick` assignment must appear before
the first awaited call expression (``await api(`` / ``await _ensure`` / ``await send``).
"""
import re as _re
for body in (_function_body(UI_JS, "submitEdit"), _function_body(COMMANDS_JS, "cmdRetry")):
pick_idx = body.index("const _recoveryPick")
# match a real awaited call, not the word "await" inside a comment
m = _re.search(r"await\s+\w", body)
assert m is not None, "expected an awaited call in the recovery path"
assert pick_idx < m.start(), (
"_recoveryPick must be captured before the first awaited call (pre-await snapshot)"
)
def test_recovery_reguards_active_session_after_each_await():
"""After each await, a session-switch guard must re-check the captured sid.
Codex gate SILENT findings: switching sessions during the retry GET await (or
the edit truncate await) let session A's recovery intent apply to session B —
in /retry it wrote B's model into A's pending marker. Each recovery path must
re-assert its captured session id AFTER its post-network await, before
mutating messages / re-arming / calling send().
"""
retry = _function_body(COMMANDS_JS, "cmdRetry")
# cmdRetry: guard after the session GET await, before the render/re-arm/send
assert retry.count("S.session.session_id!==activeSid") >= 2, (
"cmdRetry must re-guard activeSid after the session GET await (>=2 guards total)"
)
edit = _function_body(UI_JS, "submitEdit")
# submitEdit: guard after the truncate await, before slice/re-arm/send
assert edit.count("S.session.session_id !== initialSid") >= 2, (
"submitEdit must re-guard initialSid after the truncate await (>=2 guards total)"
)
@@ -0,0 +1,189 @@
"""Regression tests for #5941: an errored turn's produced response stays VISIBLE.
Reported by b3nw (Discord #report-bugs). When a turn ends in a provider/agent
error but the assistant HAD already produced content (tool calls + reasoning),
the settled-scene render folded that whole turn into a *collapsed* worklog above
the error card. The user saw only the error bubble and reasonably concluded that
nothing came back — even though the real response was one click away behind the
collapsed header.
Root cause: `_renderSettledAnchorSceneForMessage` (static/ui.js) built the
settled worklog with `collapsed: !keepSettledWorklogOpen` UNCONDITIONALLY — the
errored turn's `terminal_state` (error / no_response / tool_limit_reached / ...)
was never consulted, so an errored-but-content-bearing turn collapsed exactly
like a normal completed turn.
Fix: classify the scene's `terminal_state`. When it is an error/failure state
(NOT a normal `completed`), keep the settled worklog EXPANDED by default so the
produced response stays visible, unless the user has explicitly collapsed THIS
turn's worklog (saved 'closed' disclosure state). The genuinely-empty errored
turn is unaffected: the render gate at the top of the function requires a
worklog-worthy scene (>=1 tool/thinking/compression row), so a real no_response
with zero produced content never reaches the collapse decision and still shows
only its error card.
Behavioral tests extract the real predicate from static/ui.js and execute it in
Node; structural tests lock the wiring into the render gate.
"""
import json
import shutil
import subprocess
import textwrap
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parent.parent
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
NODE = shutil.which("node")
def _function_body(src: str, name: str) -> str:
marker = f"function {name}"
start = src.index(marker)
brace = src.index("{", start)
depth = 0
for idx in range(brace, len(src)):
ch = src[idx]
if ch == "{":
depth += 1
elif ch == "}":
depth -= 1
if depth == 0:
return src[brace + 1 : idx]
raise AssertionError(f"function {name} body not found")
def _extract(src: str, name: str) -> str:
marker = f"function {name}"
start = src.index(marker)
body = _function_body(src, name)
sig = src[start : src.index("{", start)]
return f"{sig}{{{body}}}"
def test_render_gate_wires_errored_terminal_state_into_collapse_decision():
"""Structural lock: the settled-scene collapse decision must honor the
errored-terminal-state predicate, not just keepSettledWorklogOpen."""
body = _function_body(UI_JS, "_renderSettledAnchorSceneForMessage")
# The predicate is computed for this turn's scene...
assert "_anchorSceneHasErroredTerminalState(scene)" in body, (
"render gate must classify the scene's terminal_state"
)
# ...an explicit user-collapsed worklog is still respected...
assert "_readActivityDisclosureState(activityKey)!=='closed'" in body, (
"default-open must not override an explicit user collapse"
)
# ...and it feeds the collapsed:! decision alongside keepSettledWorklogOpen.
assert "collapsed:!(keepSettledWorklogOpen||erroredWorklogKeepOpen)" in body, (
"errored-turn keep-open must flow into the worklog collapsed flag"
)
# The worklog-worthiness guard is still the entry gate (empty errored turns
# never reach the collapse decision → they keep their bare error card).
assert "_anchorSceneSceneHasWorklogWorthyRows(message._anchor_activity_scene)" in body
# Predicate definition exists.
assert "function _anchorSceneHasErroredTerminalState" in UI_JS
@pytest.mark.skipif(NODE is None, reason="node required for behavioral test")
def test_errored_terminal_states_keep_content_visible_completed_does_not():
"""Errored/failure terminal states → true (keep visible); completion / null
/ user-stop states → false (unchanged behavior)."""
predicate = _extract(UI_JS, "_anchorSceneHasErroredTerminalState")
# Pull the shared Set constant the predicate closes over.
set_line_start = UI_JS.index("const _ANCHOR_SCENE_ERRORED_TERMINAL_STATES=new Set([")
set_line_end = UI_JS.index("]);", set_line_start) + len("]);")
set_decl = UI_JS[set_line_start:set_line_end]
harness = textwrap.dedent(f"""
{set_decl}
{predicate}
const out = {{}};
// (1) Provider/agent errors that produced content must keep it visible.
out.error = _anchorSceneHasErroredTerminalState({{ terminal_state: 'error' }});
out.no_response = _anchorSceneHasErroredTerminalState({{ terminal_state: 'no_response' }});
out.degraded = _anchorSceneHasErroredTerminalState({{ terminal_state: 'degraded' }});
out.connection_lost = _anchorSceneHasErroredTerminalState({{ terminal_state: 'connection_lost' }});
out.tool_limit_reached = _anchorSceneHasErroredTerminalState({{ terminal_state: 'tool_limit_reached' }});
out.compression_exhausted = _anchorSceneHasErroredTerminalState({{ terminal_state: 'compression_exhausted' }});
out.error_upper = _anchorSceneHasErroredTerminalState({{ terminal_state: 'ERROR' }});
// (2) A normal completed turn must NOT be force-opened (default collapse stays).
out.completed = _anchorSceneHasErroredTerminalState({{ terminal_state: 'completed' }});
// (3) User-initiated stops keep their existing behavior (own dedicated cards).
out.cancelled = _anchorSceneHasErroredTerminalState({{ terminal_state: 'cancelled' }});
out.interrupted = _anchorSceneHasErroredTerminalState({{ terminal_state: 'interrupted' }});
// (4) Missing / null terminal_state → false.
out.null_state = _anchorSceneHasErroredTerminalState({{ terminal_state: null }});
out.no_field = _anchorSceneHasErroredTerminalState({{}});
out.no_scene = _anchorSceneHasErroredTerminalState(null);
console.log(JSON.stringify(out));
""")
res = subprocess.run([NODE, "-e", harness], capture_output=True, text=True, timeout=30)
assert res.returncode == 0, res.stderr
out = json.loads(res.stdout.strip())
# Errored/failure states keep produced content visible.
for state in (
"error",
"no_response",
"degraded",
"connection_lost",
"tool_limit_reached",
"compression_exhausted",
"error_upper",
):
assert out[state] is True, f"errored terminal_state '{state}' must keep response visible"
# Normal completion + user stops + absent state are unchanged (not force-open).
for state in ("completed", "cancelled", "interrupted", "null_state", "no_field", "no_scene"):
assert out[state] is False, f"terminal_state '{state}' must NOT be treated as an errored keep-open"
@pytest.mark.skipif(NODE is None, reason="node required for behavioral test")
def test_errored_worklog_keep_open_decision_matrix():
"""End-to-end of the exact collapse expression the render gate evaluates:
erroredWorklogKeepOpen = hasErroredTerminalState(scene)
&& savedDisclosure !== 'closed'
collapsed = !(keepSettledWorklogOpen || erroredWorklogKeepOpen)
Encodes the #5941 invariant plus its guards:
* errored + content-bearing (default disclosure) -> visible (not collapsed)
* errored + content-bearing, user explicitly collapsed -> collapsed (respected)
* completed + content-bearing -> collapsed (unchanged)
"""
predicate = _extract(UI_JS, "_anchorSceneHasErroredTerminalState")
set_line_start = UI_JS.index("const _ANCHOR_SCENE_ERRORED_TERMINAL_STATES=new Set([")
set_line_end = UI_JS.index("]);", set_line_start) + len("]);")
set_decl = UI_JS[set_line_start:set_line_end]
harness = textwrap.dedent(f"""
{set_decl}
{predicate}
function decide(scene, savedDisclosure, keepSettledWorklogOpen) {{
const erroredWorklogKeepOpen =
_anchorSceneHasErroredTerminalState(scene) && savedDisclosure !== 'closed';
const collapsed = !(keepSettledWorklogOpen || erroredWorklogKeepOpen);
return collapsed;
}}
const out = {{}};
// Errored turn that produced content, no explicit user disclosure → VISIBLE.
out.errored_default = decide({{ terminal_state: 'error' }}, null, false);
// Same errored turn, but the user explicitly collapsed it → stays collapsed.
out.errored_user_collapsed = decide({{ terminal_state: 'error' }}, 'closed', false);
// Errored turn the user explicitly opened → visible.
out.errored_user_open = decide({{ terminal_state: 'no_response' }}, 'open', false);
// A normal completed turn collapses as before.
out.completed_default = decide({{ terminal_state: 'completed' }}, null, false);
console.log(JSON.stringify(out));
""")
res = subprocess.run([NODE, "-e", harness], capture_output=True, text=True, timeout=30)
assert res.returncode == 0, res.stderr
out = json.loads(res.stdout.strip())
assert out["errored_default"] is False, (
"an errored turn that produced content must render its worklog EXPANDED (visible) by default"
)
assert out["errored_user_collapsed"] is True, (
"a worklog the user explicitly collapsed must stay collapsed even on an errored turn"
)
assert out["errored_user_open"] is False, "a user-opened errored worklog stays visible"
assert out["completed_default"] is True, (
"a normal completed turn must keep its default-collapsed worklog (no behavior change)"
)