From d9440e86545dde14af6fa639fb8fdbad7fc00a38 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Sat, 19 Sep 2026 15:27:17 -0300 Subject: [PATCH 1/8] fix(stream): rebuild the assistant body when a replay cursor outlives its live state A transparent-mode SSE drop followed by a run-journal re-attach could settle a turn with its footer (duration / model / TTFT / Done) painted over a blank message body. The cached INFLIGHT entry kept `lastRunJournalSeq` (206 in the report) while its live assistant projection was gone; re-attach passed that cursor to the server as `after_seq`, so every journal event that rebuilds the message body was skipped and only the tail came back. A manual reload - a zero-floor replay - restored the message, which is what proves the server-side journal was complete the whole time. - `_inflightCanSeedJournalReplay()`: only recoverable live output (assistant content/reasoning, `lastAssistantText`/`lastReasoningText`, live DOM, tools, burst anchors, a populated anchor scene) authorises a nonzero replay floor. - `_runJournalReplayFloorForInflight()` / `_runJournalReplayEventIdForInflight()` gate BOTH halves of the cursor. The server reads `after_event_id` before `after_seq`, so zeroing the seq alone would still resume at the stale event id and skip the same range. - `_normalizeInflightReplayCursorForReattach()` re-validates the selected recovery object immediately before reattach (both `loadSession` reattach paths), so a cursor that outlived its projection cannot reach the wire. `_inflightHasVisibleLiveState()` keeps its keep-vs-discard role: a bare user row has to stay visible across a mid-turn reload until pending/live reconciliation runs, so replay authority lives in the floor predicate rather than overloading that contract. Fixes #7640 --- static/messages.js | 15 +- static/sessions.js | 62 +++++++ tests/test_inflight_stream_reuse.py | 248 +++++++++++++++++++++++++++- 3 files changed, 320 insertions(+), 5 deletions(-) diff --git a/static/messages.js b/static/messages.js index 92a10712..488caad0 100644 --- a/static/messages.js +++ b/static/messages.js @@ -2722,11 +2722,18 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ let _currentActivityBurstId=Number((INFLIGHT[activeSid]&&INFLIGHT[activeSid].currentActivityBurstId)||0)||0; let _currentLiveSegmentSeq=Number((INFLIGHT[activeSid]&&INFLIGHT[activeSid].currentLiveSegmentSeq)||0)||0; let _assistantSegmentSeq=Number((INFLIGHT[activeSid]&&INFLIGHT[activeSid].currentLiveSegmentSeq)||0)||0; - let _lastRunJournalSeq=reconnecting - ? Number((INFLIGHT[activeSid]&&INFLIGHT[activeSid].lastRunJournalSeq)||0) + // #7640: the replay floor is only as trustworthy as the recovery state behind + // it. A cache that kept the cursor but lost the live assistant projection must + // not raise `after_seq`: the server would then replay only the tail (often just + // `stream_end`), and the missing journal range never gets a chance to rebuild + // the body — the settled footer paints over a blank message until a reload. + // Fall back to the zero floor whenever the state cannot be validated. + const _replayCursorInflight=reconnecting?INFLIGHT[activeSid]:null; + let _lastRunJournalSeq=(typeof _runJournalReplayFloorForInflight==='function') + ? _runJournalReplayFloorForInflight(_replayCursorInflight) : 0; - let _lastRunJournalEventId=reconnecting - ? String((INFLIGHT[activeSid]&&INFLIGHT[activeSid].lastRunJournalEventId)||'') + let _lastRunJournalEventId=(typeof _runJournalReplayEventIdForInflight==='function') + ? _runJournalReplayEventIdForInflight(_replayCursorInflight) : ''; const _STREAM_FADE_MS=620; const _STREAM_FADE_MAX_MS=900; diff --git a/static/sessions.js b/static/sessions.js index c6a302a4..3b18dc8f 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1017,6 +1017,61 @@ function _inflightHasVisibleLiveState(inflight) { return false; } +// #7640: a run-journal replay cursor is only safe when the recovery object can +// still repaint the live assistant turn. `_inflightHasVisibleLiveState()` above +// deliberately counts a plain user row (the optimistic row has to survive a +// mid-turn reload), but such an object carries no assistant projection at all: +// seeding `after_seq` from it tells the server to skip every journal event that +// would rebuild the body, so the settled footer lands over a blank message until +// a manual reload replays the journal from the zero floor. Only recoverable live +// output may raise the replay floor. +function _inflightCanSeedJournalReplay(inflight){ + if(!inflight||typeof inflight!=='object') return false; + if(String(inflight.lastAssistantText||'').trim()) return true; + if(String(inflight.lastReasoningText||'').trim()) return true; + if(String(inflight.liveTurnHtml||'').trim()) return true; + if(Array.isArray(inflight.toolCalls)&&inflight.toolCalls.length) return true; + if(Array.isArray(inflight.activityBurstAnchors)&&inflight.activityBurstAnchors.length) return true; + const anchorScene=inflight.anchorActivityScene; + if(anchorScene&&Array.isArray(anchorScene.activity_rows)&&anchorScene.activity_rows.length) return true; + if(Array.isArray(inflight.messages)){ + return inflight.messages.some((msg)=>{ + if(!msg||msg.role!=='assistant') return false; + const content=msg.content; + if(typeof content==='string') return Boolean(content.trim()); + if(Array.isArray(content)) return content.length>0; + return Boolean(content); + }); + } + return false; +} + +function _runJournalReplayFloorForInflight(inflight){ + if(!_inflightCanSeedJournalReplay(inflight)) return 0; + return Math.max(0,Number((inflight&&inflight.lastRunJournalSeq)||0)||0); +} + +function _runJournalReplayEventIdForInflight(inflight){ + if(!_runJournalReplayFloorForInflight(inflight)) return ''; + return String((inflight&&inflight.lastRunJournalEventId)||''); +} + +// #7640: re-validate the selected recovery object immediately before reattach +// instead of trusting its existence. A cursor that outlived its assistant +// projection is dropped here so it cannot reach the wire as a replay floor; the +// replay then restarts from zero, which is the path a manual reload already +// proves out. Normalising the object (not just the floor) also keeps a later +// persist from resurrecting the stale cursor on the next reattach. +function _normalizeInflightReplayCursorForReattach(inflight){ + if(!inflight||typeof inflight!=='object') return inflight; + const seq=Math.max(0,Number(inflight.lastRunJournalSeq||0)||0); + if(seq>0&&!_inflightCanSeedJournalReplay(inflight)){ + inflight.lastRunJournalSeq=0; + inflight.lastRunJournalEventId=''; + } + return inflight; +} + function _serverLiveSnapshotToolId(tc){ return String(tc&&(tc.tid||tc.id||tc.tool_call_id||tc.tool_use_id||tc.call_id||'')||'').trim(); } @@ -2184,6 +2239,11 @@ async function loadSession(sid){ if(INFLIGHT[sid].reattach&&activeStreamId&&typeof attachLiveStream==='function'){ INFLIGHT[sid].reattach=false; if (!_isCurrentLoad()) return; + // #7640: validate the selected recovery object at the moment of reattach + // rather than trusting its existence. A cache that kept `lastRunJournalSeq` + // but lost the live assistant projection would otherwise seed the replay + // floor and skip the journal range that rebuilds the message body. + _normalizeInflightReplayCursorForReattach(INFLIGHT[sid]); didReconnect=true; attachLiveStream(sid, activeStreamId, S.session.pending_attachments||[], {reconnecting:true}); } @@ -2313,6 +2373,8 @@ async function loadSession(sid){ if(activeStreamId){ S.busy=true; S.activeStreamId=activeStreamId; + // #7640: same reattach validation as the `reattach` branch above. + if(INFLIGHT[sid]) _normalizeInflightReplayCursorForReattach(INFLIGHT[sid]); if(typeof attachLiveStream==='function') attachLiveStream(sid, activeStreamId, S.session.pending_attachments||[], {reconnecting:true}); else if(typeof watchInflightSession==='function') watchInflightSession(sid, activeStreamId); updateSendBtn(); diff --git a/tests/test_inflight_stream_reuse.py b/tests/test_inflight_stream_reuse.py index 36b215f2..d66d4a40 100644 --- a/tests/test_inflight_stream_reuse.py +++ b/tests/test_inflight_stream_reuse.py @@ -1060,6 +1060,249 @@ def test_load_session_discards_cursor_only_inflight_before_reattach(): assert 0 <= guard_pos < inflight_branch_pos +def test_user_only_inflight_must_not_seed_nonzero_replay_floor(): + """#7640: a plain user row must not authorize a nonzero replay floor. + + After a mid-stream SSE drop in transparent mode the client re-attaches with + `replay=1&after_seq=`. When the cached INFLIGHT entry kept the + optimistic user row plus `lastRunJournalSeq` but lost the live assistant + projection, that cursor told the server to skip every journal event that + rebuilds the message body: only the tail (often just `stream_end`) came back + and the settled footer painted over a blank message until a manual reload. + """ + assert NODE, "node not on PATH" + script = "\n".join( + [ + "const assert=require('assert');", + _function_decl(SESSIONS_JS, "_inflightCanSeedJournalReplay"), + _function_decl(SESSIONS_JS, "_runJournalReplayFloorForInflight"), + _function_decl(SESSIONS_JS, "_runJournalReplayEventIdForInflight"), + """ +const userOnly={ + messages:[{role:'user', content:'a very long ttft prompt'}], + lastRunJournalSeq:206, + lastRunJournalEventId:'stream-e75e:206', +}; +assert.strictEqual(_inflightCanSeedJournalReplay(userOnly), false); +assert.strictEqual(_runJournalReplayFloorForInflight(userOnly), 0); +assert.strictEqual(_runJournalReplayEventIdForInflight(userOnly), ''); +// The floor helper reports, it does not mutate the recovery object. +assert.strictEqual(userOnly.lastRunJournalSeq, 206); + +// A user row with a populated but assistant-less transcript is still user-only. +assert.strictEqual( + _runJournalReplayFloorForInflight({ + messages:[{role:'user', content:'hi'}, {role:'assistant', content:''}], + lastRunJournalSeq:206, + }), + 0 +); +assert.strictEqual( + _runJournalReplayFloorForInflight({ + messages:[{role:'user', content:'hi'}, {role:'assistant', content:' '}], + lastRunJournalSeq:206, + }), + 0 +); + +// Positive inverse: any recoverable live output keeps the cursor. +const recovering=[ + {lastAssistantText:'partial answer'}, + {lastReasoningText:'thinking...'}, + {liveTurnHtml:'
projected turn
'}, + {toolCalls:[{name:'read_file'}]}, + {activityBurstAnchors:[{seq:12}]}, + {anchorActivityScene:{activity_rows:[{kind:'tool'}]}}, + {messages:[{role:'user', content:'p'}, {role:'assistant', content:'answer'}]}, +]; +for (const extra of recovering) { + const state={messages:[{role:'user', content:'p'}], ...extra}; + state.lastRunJournalSeq=206; + state.lastRunJournalEventId='stream-e75e:206'; + assert.strictEqual( + _runJournalReplayFloorForInflight(state), + 206, + 'recoverable live output must keep the cursor: '+JSON.stringify(extra) + ); + assert.strictEqual(_runJournalReplayEventIdForInflight(state), 'stream-e75e:206'); +} + +// Degenerate inputs must stay on the zero floor. +assert.strictEqual(_runJournalReplayFloorForInflight(null), 0); +assert.strictEqual(_runJournalReplayFloorForInflight({}), 0); +assert.strictEqual(_runJournalReplayFloorForInflight({lastRunJournalSeq:206, messages:[]}), 0); +assert.strictEqual( + _runJournalReplayFloorForInflight({lastAssistantText:'partial', lastRunJournalSeq:-4}), + 0 +); +assert.strictEqual(_runJournalReplayEventIdForInflight({}), ''); +""", + ] + ) + result = subprocess.run([NODE, "-e", script], capture_output=True, text=True, check=False) + assert result.returncode == 0, result.stderr + + +def test_reattach_discards_replay_cursor_without_live_assistant_state(): + """#7640: validate the recovery object immediately before reattach. + + Trusting the object's existence (rather than its contents) is what let a + cursor-only cache reach the wire as a replay floor. Reattach now drops the + cursor so the journal replays from zero, which is the path a manual reload + already proves out, while a cache that still holds live assistant output + keeps its cursor and avoids re-rendering the whole journal. + """ + assert NODE, "node not on PATH" + script = "\n".join( + [ + "const assert=require('assert');", + _function_decl(SESSIONS_JS, "_inflightCanSeedJournalReplay"), + _function_decl(SESSIONS_JS, "_runJournalReplayFloorForInflight"), + _function_decl(SESSIONS_JS, "_normalizeInflightReplayCursorForReattach"), + """ +const cursorOnly={ + streamId:'stream-e75e', + reattach:true, + messages:[{role:'user', content:'a very long ttft prompt'}], + lastRunJournalSeq:206, + lastRunJournalEventId:'stream-e75e:206', +}; +assert.strictEqual( + _normalizeInflightReplayCursorForReattach(cursorOnly), + cursorOnly, + 'must return the same object so the caller keeps mutating the live cache' +); +assert.strictEqual(cursorOnly.lastRunJournalSeq, 0); +assert.strictEqual(cursorOnly.lastRunJournalEventId, ''); +assert.strictEqual(_runJournalReplayFloorForInflight(cursorOnly), 0); +assert.strictEqual( + cursorOnly.messages.length, + 1, + 'the optimistic user row must survive so pending dedupe still works' +); + +const recovering={ + streamId:'stream-e75e', + reattach:true, + messages:[{role:'user', content:'p'}], + lastAssistantText:'partial answer', + lastRunJournalSeq:206, + lastRunJournalEventId:'stream-e75e:206', +}; +_normalizeInflightReplayCursorForReattach(recovering); +assert.strictEqual(recovering.lastRunJournalSeq, 206); +assert.strictEqual(recovering.lastRunJournalEventId, 'stream-e75e:206'); + +const noCursor={streamId:'stream-e75e', messages:[{role:'user', content:'p'}]}; +_normalizeInflightReplayCursorForReattach(noCursor); +assert.strictEqual(noCursor.lastRunJournalSeq, undefined); +assert.strictEqual(_runJournalReplayFloorForInflight(noCursor), 0); + +assert.strictEqual(_normalizeInflightReplayCursorForReattach(null), null); +assert.strictEqual(_normalizeInflightReplayCursorForReattach(undefined), undefined); +""", + ] + ) + result = subprocess.run([NODE, "-e", script], capture_output=True, text=True, check=False) + assert result.returncode == 0, result.stderr + + +def test_reattach_replay_params_from_a_cursor_only_cache_replay_from_zero(): + """#7640: end-to-end wire contract for a cursor-only reattach. + + Derives the replay params from the real floor helpers (node) and feeds them + to the real server cursor parser. Tying both halves together matters because + `api.routes._parse_run_journal_after_seq()` reads `after_event_id` BEFORE + `after_seq`: a fix that zeroed only the seq would still resume at the stale + event id and skip every journal event that rebuilds the message body. + """ + assert NODE, "node not on PATH" + script = "\n".join( + [ + "const assert=require('assert');", + _function_decl(SESSIONS_JS, "_inflightCanSeedJournalReplay"), + _function_decl(SESSIONS_JS, "_runJournalReplayFloorForInflight"), + _function_decl(SESSIONS_JS, "_runJournalReplayEventIdForInflight"), + """ +const runId='stream-7640'; +const userOnly={ + messages:[{role:'user', content:'a very long ttft prompt'}], + lastRunJournalSeq:206, + lastRunJournalEventId:runId+':205', +}; +console.log(JSON.stringify({ + after_seq:String(_runJournalReplayFloorForInflight(userOnly)), + after_event_id:String(_runJournalReplayEventIdForInflight(userOnly)||''), +})); +""", + ] + ) + result = subprocess.run([NODE, "-e", script], capture_output=True, text=True, check=False) + assert result.returncode == 0, result.stderr + params = json.loads(result.stdout.strip().splitlines()[-1]) + + # What the fixed client puts on the wire for a cache with no live projection. + assert params == {"after_seq": "0", "after_event_id": ""} + + from api.routes import _parse_run_journal_after_seq + + run_id = "stream-7640" + assert ( + _parse_run_journal_after_seq( + {"after_seq": [params["after_seq"]], "after_event_id": [params["after_event_id"]]}, + run_id, + ) + == 0 + ) + + # Precedence proof: dropping the seq alone would still resume at the stale id. + assert ( + _parse_run_journal_after_seq( + {"after_seq": ["0"], "after_event_id": [f"{run_id}:205"]}, run_id + ) + == 205 + ) + # ...and the pre-fix pair, for the record. + assert ( + _parse_run_journal_after_seq( + {"after_seq": ["206"], "after_event_id": [f"{run_id}:205"]}, run_id + ) + == 205 + ) + + +def test_attach_and_reattach_use_the_validated_replay_floor(): + """#7640 wiring guard: the validated floor is what reaches the wire. + + The behavioral coverage for the floor helpers lives in + `test_user_only_inflight_must_not_seed_nonzero_replay_floor` and + `test_reattach_discards_replay_cursor_without_live_assistant_state`; this + test pins that `attachLiveStream()` and both `loadSession()` reattach paths + actually go through them instead of reading the raw cursor field. + """ + body = _function_body(MESSAGES_JS, "attachLiveStream") + init_pos = body.index("const _replayCursorInflight=") + init_block = body[init_pos : body.index(";", body.index("let _lastRunJournalEventId=", init_pos))] + assert "INFLIGHT[activeSid]" in init_block, "must still read the live cache" + assert "_runJournalReplayFloorForInflight(_replayCursorInflight)" in init_block + assert "_runJournalReplayEventIdForInflight(_replayCursorInflight)" in init_block + assert "INFLIGHT[activeSid].lastRunJournalSeq" not in init_block, ( + "attachLiveStream must not read the raw cursor field anymore" + ) + assert "INFLIGHT[activeSid].lastRunJournalEventId" not in init_block + + load_body = re.sub(r"\s+", "", _function_body(SESSIONS_JS, "loadSession")) + assert load_body.count("_normalizeInflightReplayCursorForReattach(INFLIGHT[sid])") == 2, ( + "both reattach paths must re-validate the recovery object" + ) + first = load_body.index("_normalizeInflightReplayCursorForReattach(INFLIGHT[sid])") + first_attach = load_body.index("attachLiveStream(sid,activeStreamId", first) + assert first < first_attach, "validation must run before the stream is attached" + last = load_body.rindex("_normalizeInflightReplayCursorForReattach(INFLIGHT[sid])") + last_attach = load_body.rindex("attachLiveStream(sid,activeStreamId", 0, last) + assert last_attach < last + + def test_live_recovery_prefers_newer_durable_run_journal_snapshot(): """Recovery chooses by stream identity and durable journal progress.""" assert NODE, "node not on PATH" @@ -1174,7 +1417,10 @@ assert.strictEqual(inflight.lastRunJournalEventId, 'run-a:7'); assert "lastRunJournalEventId:String(stored.lastRunJournalEventId||'')" in load_body assert "lastRunJournalEventId:state.lastRunJournalEventId||''" in compact_body assert "inflight.lastRunJournalEventId||''" in attach_body - assert "INFLIGHT[activeSid]&&INFLIGHT[activeSid].lastRunJournalEventId" in attach_body + # #7640: the cursor's event id is gated by the same validation as the seq — + # reading the raw field here let a cursor outlive its assistant projection. + assert "_runJournalReplayEventIdForInflight(_replayCursorInflight)" in attach_body + assert "INFLIGHT[activeSid]&&INFLIGHT[activeSid].lastRunJournalEventId" not in attach_body assert "inflight.lastRunJournalEventId=raw" in attach_body assert "INFLIGHT[activeSid].streamId=streamId" in attach_body assert "INFLIGHT[activeSid].lastRunJournalEventId=''" in attach_body From a8180bc51f9c2c900c6a570dc7df34193e793295 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Tue, 22 Sep 2026 22:54:12 -0300 Subject: [PATCH 2/8] fix(context-length): guard provider ownership on global model.base_url backfill (#7535) A session on a built-in registry provider (e.g. opencode-go, endpoint in PROVIDER_REGISTRY, empty base_url by design) had its empty base_url slot backfilled with the global model.base_url owned by a *different* provider (custom:ark). The context-length resolver then probed the wrong endpoint's /models catalog and fell back to the generic 128K catalog entry for a 1M-context model, while the on-disk session and SSE stream carried 1,000,000. Only fill the global model.base_url slot when the session provider IS the configured model.provider owner, mirroring the ownership predicate used for model_cfg's API key in _context_length_config_api_key_for_provider. --- api/routes.py | 12 ++- ...e3256_context_length_default_only_guard.py | 73 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/api/routes.py b/api/routes.py index 5f5893ba..407abbac 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7183,7 +7183,17 @@ def _context_length_lookup_inputs_for_model( if not effective_provider: effective_provider = _canonical_context_provider(model_cfg.get("provider")) if not effective_base_url: - effective_base_url = str(model_cfg.get("base_url") or "").strip() + # #7535: the global model.base_url may only fill an empty slot when + # the session provider IS the configured model.provider owner + # (mirror the ownership predicate used for model_cfg's API key in + # _context_length_config_api_key_for_provider). A built-in registry + # provider (empty base_url by design) must keep the slot empty so + # the registry endpoint resolves instead of another provider's URL. + _model_cfg_provider = _canonical_context_provider(model_cfg.get("provider")) + if not effective_provider or _providers_match_for_context( + _model_cfg_provider, effective_provider + ): + effective_base_url = str(model_cfg.get("base_url") or "").strip() custom_providers = cfg.get("custom_providers") if isinstance(cfg, dict) else None if not isinstance(custom_providers, list): diff --git a/tests/test_issue3256_context_length_default_only_guard.py b/tests/test_issue3256_context_length_default_only_guard.py index ea9ac9da..34f4b50f 100644 --- a/tests/test_issue3256_context_length_default_only_guard.py +++ b/tests/test_issue3256_context_length_default_only_guard.py @@ -631,3 +631,76 @@ def test_sse_done_path_broadened_to_any_model_window_mismatch(): assert "_model_matches_configured_default as _mmcd_cc" not in _STREAMING_SRC, ( "the old default-only save matcher (_mmcd_cc) must be removed (#4618)" ) + + +# --- #7535: provider-ownership guard on the global model.base_url backfill --- +# Same family as #3256: a global value scoped to its owner must not leak onto +# sessions owned by a different provider. + + +def test_builtin_provider_lookup_does_not_inherit_other_providers_base_url(): + """#7535: a session on a built-in registry provider (whose base_url is empty + by design because the endpoint lives in PROVIDER_REGISTRY, not config.yaml) + must NOT inherit the global ``model.base_url`` owned by a different (custom) + provider. The unguarded backfill handed the custom endpoint to the + context-length resolver, which probed the wrong ``/models`` catalog and fell + back to the generic 128K catalog entry for a 1M-context model, while the + on-disk session and SSE stream both carried 1,000,000.""" + from api.routes import _context_length_lookup_inputs_for_model + + ark_base = "https://ark.cn-beijing.volces.com/api/coding/v3" + cfg = { + "model": { + "provider": "custom:ark", + "base_url": ark_base, + "default": "ark-code-latest", + "context_length": 1000000, + }, + "custom_providers": [ + { + "name": "ark", + "base_url": ark_base, + "api_key": "ark-key", + "model": "ark-code-latest", + } + ], + } + + lookup = _context_length_lookup_inputs_for_model( + "@opencode-go:deepseek-v4.1-flash", + "opencode-go", + cfg=cfg, + ) + + assert lookup.provider == "opencode-go" + assert lookup.base_url == "", ( + "the global model.base_url belongs to custom:ark; the opencode-go " + "registry slot must stay empty so PROVIDER_REGISTRY's endpoint " + "resolves the context window (#7535)" + ) + + +def test_same_provider_session_keeps_global_base_url_backfill(): + """#7535 control (compat): when the session provider IS the configured + owner of ``model.base_url``, the backfill behavior is preserved.""" + from api.routes import _context_length_lookup_inputs_for_model + + ark_base = "https://ark.cn-beijing.volces.com/api/coding/v3" + cfg = { + "model": { + "provider": "custom:ark", + "base_url": ark_base, + "default": "ark-code-latest", + }, + } + + lookup = _context_length_lookup_inputs_for_model( + "ark-code-latest", + "custom:ark", + cfg=cfg, + ) + + assert lookup.provider == "custom:ark" + assert lookup.base_url == ark_base, ( + "same-owner sessions must keep the model.base_url backfill (#7535 compat)" + ) From 301ed54ab7f176038cc977bb9758a0210a1605e3 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Wed, 23 Sep 2026 09:26:24 -0300 Subject: [PATCH 3/8] fix(context-length): restore base_url for two guarded configs (#7743) The #7535 ownership guard skipped the global model.base_url backfill for two shapes that master handled: - A `model` section with `base_url` but no `provider` declares no owner, so `_providers_match_for_context("", )` was false and the endpoint was dropped, silently falling back to 256,000 instead of querying the configured /v1/models (the profile-setup path writes this shape). - `opencode_go` and `opencode-go` are two spellings of the same built-in provider, but `_canonical_context_provider` does not fold underscores, so the owner session lost its URL. Allow the backfill when no owner is declared, and compare both sides through api.config._canonicalise_provider_id, which folds `_` to `-` while keeping distinct `custom:*` slugs distinct. The #7535 case (a custom:ark owner vs an opencode-go session) still resolves to an empty base_url. --- api/routes.py | 22 ++++- ...e3256_context_length_default_only_guard.py | 89 +++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/api/routes.py b/api/routes.py index 27e3c912..b6c611ed 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7189,9 +7189,27 @@ def _context_length_lookup_inputs_for_model( # _context_length_config_api_key_for_provider). A built-in registry # provider (empty base_url by design) must keep the slot empty so # the registry endpoint resolves instead of another provider's URL. + # + # Two shapes cannot own the slot and therefore cannot conflict, so + # they keep master's backfill: a config that declares no provider + # at all (the profile-setup path writes model.base_url without one) + # and the two spellings of the same built-in id (opencode_go == + # opencode-go), folded through api.config._canonicalise_provider_id + # so distinct custom:* slugs stay distinct. _model_cfg_provider = _canonical_context_provider(model_cfg.get("provider")) - if not effective_provider or _providers_match_for_context( - _model_cfg_provider, effective_provider + _owner_provider = _model_cfg_provider + _session_provider = effective_provider + try: + from api.config import _canonicalise_provider_id as _canon_provider_id + + _owner_provider = _canon_provider_id(_owner_provider) or _owner_provider + _session_provider = _canon_provider_id(_session_provider) or _session_provider + except Exception: + pass + if ( + not effective_provider + or not _model_cfg_provider + or _providers_match_for_context(_owner_provider, _session_provider) ): effective_base_url = str(model_cfg.get("base_url") or "").strip() diff --git a/tests/test_issue3256_context_length_default_only_guard.py b/tests/test_issue3256_context_length_default_only_guard.py index 34f4b50f..b7bb3288 100644 --- a/tests/test_issue3256_context_length_default_only_guard.py +++ b/tests/test_issue3256_context_length_default_only_guard.py @@ -704,3 +704,92 @@ def test_same_provider_session_keeps_global_base_url_backfill(): assert lookup.base_url == ark_base, ( "same-owner sessions must keep the model.base_url backfill (#7535 compat)" ) + + +def test_base_url_only_config_without_provider_keeps_endpoint(): + """Regression (PR #7743 gate): a ``model`` section that declares + ``base_url`` but NO ``provider`` has no owner that could conflict, so any + session must still receive the URL — exactly as master did. The profile + setup path writes this shape (api/profiles.py), and dropping the URL made + the lookup skip the endpoint and silently fall back to 256,000.""" + from api.routes import _context_length_lookup_inputs_for_model + + only_base_url = "http://127.0.0.1:9/v1" + cfg = { + "model": { + "base_url": only_base_url, + "default": "some-model", + }, + } + + for session_provider in ("custom", "openrouter"): + lookup = _context_length_lookup_inputs_for_model( + "some-model", + session_provider, + cfg=cfg, + ) + + assert lookup.provider == session_provider + assert lookup.base_url == only_base_url, ( + f"a config with no declared provider owner cannot conflict with the " + f"{session_provider!r} session, so the base_url must survive the " + f"ownership guard" + ) + + +def test_underscore_provider_owner_matches_hyphen_session(): + """Regression (PR #7743 gate): ``opencode_go`` and ``opencode-go`` are two + spellings of the SAME built-in provider, so the owner session must still + receive ``model.base_url``. Comparing the ids through the repo's + canonicaliser folds the underscore without collapsing distinct + ``custom:*`` slugs.""" + from api.routes import _context_length_lookup_inputs_for_model + + owner_base = "http://127.0.0.1:9/v1" + cfg = { + "model": { + "provider": "opencode_go", + "base_url": owner_base, + "default": "some-model", + }, + } + + lookup = _context_length_lookup_inputs_for_model( + "some-model", + "opencode-go", + cfg=cfg, + ) + + assert lookup.provider == "opencode-go" + assert lookup.base_url == owner_base, ( + "opencode_go and opencode-go are the same built-in provider, so the " + "owner session must keep the base_url backfill" + ) + + +def test_underscore_provider_still_blocks_distinct_custom_owner(): + """Counterpart to test_underscore_provider_owner_matches_hyphen_session: + folding underscores must not open the #7535 guard. ``custom:my_box`` and a + session on the unrelated built-in ``opencode-go`` are still different + providers, so the custom owner's URL stays out of the registry slot.""" + from api.routes import _context_length_lookup_inputs_for_model + + cfg = { + "model": { + "provider": "custom:my_box", + "base_url": "http://127.0.0.1:9/v1", + "default": "some-model", + }, + } + + lookup = _context_length_lookup_inputs_for_model( + "some-model", + "opencode-go", + cfg=cfg, + ) + + assert lookup.provider == "opencode-go" + assert lookup.base_url == "", ( + "underscore folding must not let a distinct custom provider's " + "base_url leak onto another provider's session (#7535 guard)" + ) From 4b791bd82099bca0bddc29004ef519ae0c1a7ca1 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Wed, 23 Sep 2026 09:29:00 -0300 Subject: [PATCH 4/8] fix(stream): require a current live assistant row to seed the replay floor The replay-authority predicate accepted any non-empty assistant message, but a new turn copies the full transcript into INFLIGHT.messages. In an established conversation that content is the previous turn's reply, so `canSeed` came back true with both stale cursor halves preserved and the server kept skipping the range that rebuilds the body. Message-based evidence now counts only from a current `_live` assistant positioned after the latest user boundary; a historical assistant row fails closed to floor 0 and an empty event id. Refs #7640 (blocker on #7651) --- static/sessions.js | 15 +++++++- tests/test_inflight_stream_reuse.py | 58 ++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/static/sessions.js b/static/sessions.js index 73a5bff1..107767b8 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1053,8 +1053,21 @@ function _inflightCanSeedJournalReplay(inflight){ const anchorScene=inflight.anchorActivityScene; if(anchorScene&&Array.isArray(anchorScene.activity_rows)&&anchorScene.activity_rows.length) return true; if(Array.isArray(inflight.messages)){ - return inflight.messages.some((msg)=>{ + // The transcript copy carries every earlier turn (#7651): in an established + // conversation the last assistant row is the PREVIOUS turn's reply, not the + // one the cursor belongs to, so it must never authorize a nonzero floor. + // Message evidence counts only from a current `_live` assistant standing + // after the latest user boundary; anything historical fails closed to zero. + const list=inflight.messages; + let latestUserIdx=-1; + for(let i=list.length-1;i>=0;i--){ + const row=list[i]; + if(row&&row.role==='user'){latestUserIdx=i;break;} + } + return list.some((msg,idx)=>{ if(!msg||msg.role!=='assistant') return false; + if(!msg._live) return false; + if(idx<=latestUserIdx) return false; const content=msg.content; if(typeof content==='string') return Boolean(content.trim()); if(Array.isArray(content)) return content.length>0; diff --git a/tests/test_inflight_stream_reuse.py b/tests/test_inflight_stream_reuse.py index d66d4a40..7f21b434 100644 --- a/tests/test_inflight_stream_reuse.py +++ b/tests/test_inflight_stream_reuse.py @@ -1113,7 +1113,7 @@ const recovering=[ {toolCalls:[{name:'read_file'}]}, {activityBurstAnchors:[{seq:12}]}, {anchorActivityScene:{activity_rows:[{kind:'tool'}]}}, - {messages:[{role:'user', content:'p'}, {role:'assistant', content:'answer'}]}, + {messages:[{role:'user', content:'p'}, {role:'assistant', content:'answer', _live:true}]}, ]; for (const extra of recovering) { const state={messages:[{role:'user', content:'p'}], ...extra}; @@ -1127,6 +1127,39 @@ for (const extra of recovering) { assert.strictEqual(_runJournalReplayEventIdForInflight(state), 'stream-e75e:206'); } +// #7651: an ESTABLISHED conversation copies the whole transcript, so the last +// assistant row is the previous turn's reply. Historical assistant content must +// never authorize the stale cursor that belongs to the still-running turn. +const established={ + messages:[ + {role:'user', content:'first question'}, + {role:'assistant', content:'the previous turn answer'}, + {role:'user', content:'current question'}, + ], + lastRunJournalSeq:206, + lastRunJournalEventId:'stream-e75e:206', +}; +assert.strictEqual( + _inflightCanSeedJournalReplay(established), + false, + 'a historical assistant row must not seed the replay floor' +); +assert.strictEqual(_runJournalReplayFloorForInflight(established), 0); +assert.strictEqual(_runJournalReplayEventIdForInflight(established), ''); + +// Even after the latest user boundary, evidence must be LIVE output: a +// settled/non-live assistant row at the current position still yields zero. +assert.strictEqual( + _runJournalReplayFloorForInflight({ + messages:[ + {role:'user', content:'current question'}, + {role:'assistant', content:'settled reply without _live'}, + ], + lastRunJournalSeq:206, + }), + 0 +); + // Degenerate inputs must stay on the zero floor. assert.strictEqual(_runJournalReplayFloorForInflight(null), 0); assert.strictEqual(_runJournalReplayFloorForInflight({}), 0); @@ -1193,6 +1226,29 @@ _normalizeInflightReplayCursorForReattach(recovering); assert.strictEqual(recovering.lastRunJournalSeq, 206); assert.strictEqual(recovering.lastRunJournalEventId, 'stream-e75e:206'); +// #7651: a historical assistant row must not protect the cursor either, or an +// established conversation settles with a blank body over a painted footer. +const established={ + streamId:'stream-e75e', + reattach:true, + messages:[ + {role:'user', content:'first question'}, + {role:'assistant', content:'the previous turn answer'}, + {role:'user', content:'current question'}, + ], + lastRunJournalSeq:206, + lastRunJournalEventId:'stream-e75e:206', +}; +_normalizeInflightReplayCursorForReattach(established); +assert.strictEqual(established.lastRunJournalSeq, 0); +assert.strictEqual(established.lastRunJournalEventId, ''); +assert.strictEqual(_runJournalReplayFloorForInflight(established), 0); +assert.strictEqual( + established.messages.length, + 3, + 'the transcript must survive; only the cursor halves are dropped' +); + const noCursor={streamId:'stream-e75e', messages:[{role:'user', content:'p'}]}; _normalizeInflightReplayCursorForReattach(noCursor); assert.strictEqual(noCursor.lastRunJournalSeq, undefined); From 5466d09e91548e07b3eac910f99bd09d31fc9c70 Mon Sep 17 00:00:00 2001 From: happy5318 <5318happy@users.noreply.github.com> Date: Wed, 23 Sep 2026 21:23:32 +0800 Subject: [PATCH 5/8] fix(recovery): map vanished-session KeyError to the skip path in WebUI startup recovery (#7753) --- api/background_process.py | 17 ++- ...est_background_process_restart_recovery.py | 102 +++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/api/background_process.py b/api/background_process.py index 6e6323af..eea15ebb 100644 --- a/api/background_process.py +++ b/api/background_process.py @@ -1883,7 +1883,22 @@ def recover_processes_for_webui(process_registry=None, get_session_fn=None) -> i try: proc_session = process_registry.get(process_id) session_key = str(getattr(proc_session, "session_key", "") or "") - if not session_key or get_session_fn(session_key, metadata_only=True) is None: + if not session_key: + continue + # The session resolver (``api.models._resolve_session_once``) + # raises ``KeyError(sid)`` for a missing session — that is its + # documented contract for the "owner gone" outcome, not a + # fault. Treat it identically to a ``None`` return: skip the + # process without logging, and let a neighbouring live + # process still rebind. The narrow ``except KeyError`` lives + # INSIDE the outer try so an unrelated ``KeyError`` from + # ``process_registry.get()`` (or any routing-index work) still + # surfaces in the existing warning path. + try: + resolved = get_session_fn(session_key, metadata_only=True) + except KeyError: + continue + if resolved is None: continue except Exception: logger.warning( diff --git a/tests/test_background_process_restart_recovery.py b/tests/test_background_process_restart_recovery.py index 0ab76e36..45ee82ff 100644 --- a/tests/test_background_process_restart_recovery.py +++ b/tests/test_background_process_restart_recovery.py @@ -1,5 +1,6 @@ """Regression coverage for notify_on_complete across a WebUI restart.""" +import logging import threading import time from types import SimpleNamespace @@ -158,4 +159,103 @@ def test_recovery_is_fail_soft_without_agent(monkeypatch): monkeypatch.setattr("builtins.__import__", fake_import) assert bp.recover_processes_for_webui() == 0 - assert bp._PROCESS_RECOVERY_DONE is False \ No newline at end of file + assert bp._PROCESS_RECOVERY_DONE is False + + +def test_recovery_skips_process_whose_session_is_gone(monkeypatch, caplog): + """A checkpointed process whose owner has been pruned (the resolver's + ``KeyError(sid)`` contract) must be silently skipped, while a neighbouring + live process in the same recovery sweep still rebinds. Regression for + issue #7753: previously every vanished owner logged a full WARNING + traceback, indistinguishable from a real fault. + """ + calls = {"registered": []} + + class FakeRegistry: + def recover_from_checkpoint(self): + return 2 # two checkpointed processes survive restart + + def list_sessions(self): + return [ + {"session_id": "proc_vanished", "detached": True}, + {"session_id": "proc_live", "detached": True}, + ] + + def get(self, process_id): + if process_id == "proc_vanished": + return _FakeProcessSession("vanished-session") + if process_id == "proc_live": + return _FakeProcessSession("live-session") + raise AssertionError(process_id) + + fake_registry = FakeRegistry() + monkeypatch.setattr(bp, "_PROCESS_CHECKPOINT_RECOVERED", False) + monkeypatch.setattr(bp, "_PROCESS_RECOVERY_DONE", False) + monkeypatch.setattr( + bp, + "register_process_session", + lambda key, sid: calls["registered"].append((key, sid)), + ) + + def get_session(sid, metadata_only=False): + # Mirrors api.models._resolve_session_once's contract: missing + # sessions raise KeyError, NOT None. + if sid == "vanished-session": + raise KeyError(sid) + return SimpleNamespace(id=sid) + + with caplog.at_level(logging.WARNING, logger="api.background_process"): + result = bp.recover_processes_for_webui(fake_registry, get_session) + + assert result == 2 + # The live process is rebound; the vanished one is silently skipped. + assert calls["registered"] == [("live-session", "live-session")] + # No WARNING emitted for the vanished-owner case. + warning_records = [ + r for r in caplog.records if r.levelno >= logging.WARNING + ] + assert warning_records == [], ( + "vanished-session KeyError must not produce a WARNING, got: " + f"{[r.getMessage() for r in warning_records]}" + ) + assert "Could not resolve recovered WebUI process" not in caplog.text + + +def test_recovery_still_warns_on_unexpected_resolve_failure(monkeypatch, caplog): + """Non-KeyError failures from the resolver (e.g. ``OSError`` on an + unreadable sidecar) must keep entering the existing WARNING path. This + pins that the fix for #7753 is narrow and does not over-broaden the + silent-skip contract. + """ + class FakeRegistry: + def recover_from_checkpoint(self): + return 1 + + def list_sessions(self): + return [{"session_id": "proc_unexpected", "detached": True}] + + def get(self, process_id): + assert process_id == "proc_unexpected" + return _FakeProcessSession("webui-session") + + fake_registry = FakeRegistry() + monkeypatch.setattr(bp, "_PROCESS_CHECKPOINT_RECOVERED", False) + monkeypatch.setattr(bp, "_PROCESS_RECOVERY_DONE", False) + + def get_session(sid, metadata_only=False): + raise OSError("sidecar unreadable") + + with caplog.at_level(logging.WARNING, logger="api.background_process"): + result = bp.recover_processes_for_webui(fake_registry, get_session) + + assert result == 1 + warning_records = [ + r for r in caplog.records if r.levelno >= logging.WARNING + ] + assert any( + "Could not resolve recovered WebUI process 'proc_unexpected'" in r.getMessage() + for r in warning_records + ), ( + "OSError from the resolver must still emit a WARNING, got: " + f"{[r.getMessage() for r in warning_records]}" + ) \ No newline at end of file From ef7d325a5484d2db32a66ce753c12791cc414eab Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 14:34:41 +0000 Subject: [PATCH 6/8] docs(changelog): #7774 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb2efc3..e0bfaa58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ ### Fixed +- **A session deleted during a restart no longer produces a spurious recovery warning.** When + WebUI startup recovery re-attached background processes, a session that had vanished between + enumeration and rebind raised a `KeyError` that was logged as a warning. It now follows the + existing skip path, confined to the session lookup, so the vanished owner is skipped, live owners + still rebind, and registry errors still warn. (#7753, #7774 by @happy5318) + - **Waiting on the Agent's session lease is shown as a warning instead of looking stuck.** When another Hermes process (gateway, CLI or cron) holds the session's turn lease, the Agent's "another Hermes process is using this session" notices now reach the chat as a warning status From 16bc5ab3edec0df63aea53b130838fede07b7340 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 14:34:41 +0000 Subject: [PATCH 7/8] docs(changelog): #7743 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb2efc3..fbc1014f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ ### Fixed +- **Context-length lookup keeps the configured base URL for ownerless and underscore-named + providers.** The #7535 ownership guard also dropped the global `model.base_url` for a `model:` + section with a `base_url` but no `provider`, and for provider IDs written with an underscore + (`opencode_go` vs `opencode-go`), so those sessions could resolve the wrong context window. Both + now keep the URL, while a different declared owner still does not leak its endpoint. + (#7743 by @webtecnica) + - **Waiting on the Agent's session lease is shown as a warning instead of looking stuck.** When another Hermes process (gateway, CLI or cron) holds the session's turn lease, the Agent's "another Hermes process is using this session" notices now reach the chat as a warning status From a4b2264446de67c5b6dd79c0250c7aa706d46d92 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 14:34:41 +0000 Subject: [PATCH 8/8] docs(changelog): #7651 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb2efc3..4af36054 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ ### Fixed +- **Reconnecting to a new turn no longer resumes from the previous turn's replay cursor.** A new + turn copies the full transcript into the in-flight state, so the previous turn's assistant reply + could seed the replay floor and the reattached stream skipped the current reply's early events. + Replay now seeds only from the current live assistant row, and falls back to a full replay that + rebuilds the assistant body when a cursor outlives its live state. (#7651 by @happy5318) + - **Waiting on the Agent's session lease is shown as a warning instead of looking stuck.** When another Hermes process (gateway, CLI or cron) holds the session's turn lease, the Agent's "another Hermes process is using this session" notices now reach the chat as a warning status