Fail-soft git worktree unlock before remove (agent locks worktrees it creates; git refuses
--force remove on locked → orphan accumulation). Dirty-guard still runs first. Codex SAFE,
13 tests pass. Shipped solo (unbundled from #6013 which has a full-suite test-interaction).
WebUI-created worktrees are locked at creation by the agent
(cli._setup_worktree locks with --reason 'hermes pid=...'), but
remove_worktree_for_session never unlocked, so git refused every removal
with "use 'remove -f -f' to override or unlock first" — force=true
included, since a single --force cannot override a lock.
Mirror the agent's own _cleanup_worktree ordering: fail-soft
'git worktree unlock' right before the remove, after all
dirty/untracked/unpushed/stream/terminal guards have passed. Those
guards remain the real safety layer; force keeps meaning 'override
dirty working state', never 'override a foreign lock'.
Tests: locked worktree now removes cleanly without force; unlock is
fail-soft when the worktree was never locked; existing mocked
call-order assertions updated for the leading unlock call.
Fixes#6023
MEDIA tokens (image/video/audio/pdf/diff/csv/excalidraw) render inline in real time
during streaming incl split-across-chunk markers. Codex SAFE r4 (both prose regressions
closed — ownership preserved + fade retained, normal prose byte-identical to master, all
4 original findings fixed, security clean, no ReDoS), stream-gate GREEN all modes (hide_all
RED was load flake, isolated re-run clean). 227 tests + 18 real-SMD subtests pass. 4-round
convergence on the crown-jewel render + security path.
Greptile re-review (3/5) flagged two issues:
1. parserFor fallback ('__default__') string key → WeakMap.set throws
TypeError (WeakMap accepts only object keys). Fixed by introducing a
module-level sentinel object __SMD_PARSER_FALLBACK = {} and using it
in both parserFor definitions instead of the string literal.
2. _smdMediaTailClear was defined but never called despite the comment
claiming it was. Fixed by adding calls in:
- _smdEndParser → clears the ongoing live-stream parser's tail
- _clearAnchorProseIncrementalNode → clears each anchor-scene
parser's tail before the cache is dropped
All 222 tests remain green.
Code-level fixes for the two issues Greptile + owner review surfaced:
1. SAFETY (Greptile #1 / owner security regression): the original
_smdMediaAwareAddText concatenated prose slices + MEDIA HTML into
one string and handed it to DOMParser. Prose containing HTML
metacharacters (e.g. <img onerror=...>) would be parsed as live
elements. Fixed by routing each prose run through baseAddText
(which calls createTextNode) and only feeding each single MEDIA
token's trusted _inlineMediaHtmlForRef output through DOMParser.
2. CROSS-CHUNK SPLITS (Greptile #2): smd may deliver a MEDIA: token
across two flushes (e.g. "MEDIA:C:\...\" then "foo.png"). The
original implementation would treat each half as separate text
because neither half terminated cleanly. Fixed with a per-parser
tail buffer (WeakMap keyed by parser instance) that holds trailing
bytes ending in "MEDIA:" or MEDIA:<partial-ref> until the next call.
The tail-buffer boundary detection was also improved: the previous
`/[\s\)\]]/.test(rest)` incorrectly rejected "before MEDIA:" (the
whitespace is before MEDIA, not after a completed ref). Replaced
with `rest.endsWith('MEDIA:')` and `/MEDIA:...$/.exec(rest)`.
Architecture: _smdAppendMediaNode is a new single-token DOM insertion
helper that creates a DocumentFragment from one _inlineMediaHtmlForRef
output. _smdMediaTailSet/_smdMediaTailClear manage the per-parser
buffer. Both _safeSmdRenderer and _streamFadeRenderer now route through
the updated interceptor with tail map and parser identity.
Coverage: test_smd_media_in_stream.py tests the architectural invariants
(helper exists, both renderers delegate, recursion guard, entity-safety,
cross-chunk tail). 222 tests pass across all renderer/behaviour suites.
The local agent loop drops:
- a `.playwright-mcp/` directory full of intermediate screenshots while
debugging browser sessions
- a `final-check.png` of the page state at the end of a task
Both are local-only debugging artifacts that should never end up in a
commit. Add them next to the existing `screenshot-*.png` / `full-UI.png`
rules so the same dev-loop garbage collection pattern continues to hold
without anyone having to think about it.
The full renderMd() pipeline maps MEDIA:<path-or-url> tokens to inline
images / video / audio / SVG / PDF / etc. via stash + restore passes.
The streaming smd renderer used by the live assistant body never went
through that pass, so any MEDIA token emitted mid-turn showed up as the
raw path string in the chat window and only became a real <img> once
the turn settled and the full render fired.
Fix:
- Extract the URL/media → HTML mapping in renderMd() into a shared
helper _inlineMediaHtmlForRef(ref, sessionId) so the streaming path
can reuse the same DOM the settled message will eventually render.
- Add _smdMediaAwareAddText in messages.js that wraps smd's add_text
to detect MEDIA tokens in any incoming text chunk and splice the
resulting <img>/<video>/<a>/<div data-path=...> node directly into
the smd parent via a DOMParser-created fragment. Plain prose still
delegates to the renderer it intercepted (so the fade renderer keeps
its word-by-word animation, the safe renderer keeps its O(n) plain
text-node insertion).
- Route _safeSmdRenderer.add_text and _streamFadeRenderer.add_text
through the interceptor. Each fast-paths the no-MEDIA case to the
original baseAddText so we can't recurse into our own wrapper.
Coverage:
- New test_smd_media_in_stream.py locks in the architectural invariants
(helper exists, both renderers delegate to it, recursion guard).
- test_renderer_js_behaviour.py driver now provides a minimal
_inlineMediaHtmlForRef stand-in so renderMd can be invoked standalone
under node without the rest of ui.js in scope (matches the patterns
renderMd already relied on for _IMAGE_EXTS / _SVG_EXTS / _AUDIO_EXTS).
- test_svg_audio_video_rendering.py static-count assertions relax from
">=2" to ">=1" since audio/video/SVG markup now lives in a single
shared function rather than being copy-pasted inside renderMd's
stash-restore closure.
Gateway terminal provider errors classified+surfaced (not generic empty turn), session_id-bound,
pending turn persisted through reload incl repeated-prompt (is_exact_checkpoint). Codex SAFE r4 +
stream-gate GREEN all modes. Shipped solo (unbundled from #6002 which has a test-isolation CI fail).
Past committed messages fork during a live response; only the streaming message +
just-sent uncommitted prompt are blocked (toast). Codex SAFE round2 (off-by-one fixed,
_isLastMsg busy-gated, _pending not carried forward), stream-gate GREEN (hide_all RED
was load flake, isolated re-run clean), 99 tests.
Extends the live-message guard to reject both _live assistant rows and
_pending user rows that haven't been committed server-side yet. Adds a
belt-and-suspenders check: when S.busy is true, the last message in
S.messages is also blocked regardless of flags, closing a timing window
where _live might be momentarily absent on the streaming row.
The user's optimistically-pushed message (messages.js:1603) now carries
_pending:true so forkFromMessage can distinguish it from committed
messages.
Removes the blanket S.busy guard from forkFromMessage() that silently
swallowed all fork clicks while the agent was generating a response.
Instead, only blocks fork when the clicked message itself is the one
currently being streamed (msg._live), showing a clear toast.
Skips _ensureAllMessagesLoaded() during streaming to avoid visual
flicker — absoluteKeepCount is captured before any async work so the
fork data is unaffected.
Fable's one recommended fast-follow: the streaming glue that computes
_explicitly_picked from the persisted signature (streaming.py:7905-7931) had no
direct coverage. Adds test_streaming_explicitly_picked_computation_5979 mirroring
that exact comparison across deliberate-pick (honored), stale-after-model-change
(ignored), stale-after-provider-change (ignored), and never-picked (unmarked).
Both advisors passed on the signature mechanism: Codex SAFE TO SHIP (session-update
hole verified closed: update->reload->cold openai/gpt-5.4 strips to gpt-5.4;
b3nw's deliberate pick preserves), Fable 'ship it' (clean data-model invalidation,
correct layering, no #433/#3872/#548 regression). Independent 10-scenario matrix
through the real resolver all correct.
Codex found a verified lifecycle hole: the bare boolean model_explicitly_picked
was not cleared on /api/session/update (api/routes.py:14438-14460 changes+persists
the model/provider without touching it), so after an update+reload the flag stayed
True and a cold #433 stale leftover (openai/gpt-5.4) was wrongly preserved instead
of stripped to gpt-5.4.
Fix (Codex's prescription): replace the unscoped boolean with a SIGNATURE bound to
the routing context.
- api/models.py: new model_explicit_pick_signature(model, provider) ->
'<model>\x1f<provider>'; Session.model_explicit_pick_signature persisted field
(replaces the boolean; in METADATA_FIELDS, restored via kwargs, survives restart).
- api/routes.py /api/chat/start: on a fresh explicit_model_pick, stamp the signature
of the resolved model+provider.
- api/streaming.py: treat the send as a deliberate pick ONLY when the persisted
signature equals the signature recomputed from the session's CURRENT persisted
model+provider. Any model/provider change (chat-start, session-update,
normalization, provider repair) yields a different signature and auto-invalidates
the stale pick — no per-mutation-site clearing needed.
So: deliberate pick on unchanged context -> preserve (b3nw); any switch away ->
signature mismatch -> unmarked -> legacy strip (#433 stays correct even after a
session-update+cold-restart). Adds signature match/invalidate/persist tests.
75 focused tests pass (resolver + sprint40 + the 2 streaming-mock suites).
Two streaming tests monkeypatch resolve_model_provider with a fixed-arg lambda
(_model only). The send path now calls it with explicitly_picked=, so the mocks
raised TypeError. Widened both lambdas to (_model, **_kw). Caught by the full
suite (not the focused resolver tests, which call the real function).
Nathan's call: adopt Codex's complete mechanism instead of the unconditional
cold-preserve. The cold-catalog custom-proxy decision is now gated on whether the
user DELIBERATELY selected the model this session, closing the residual where a
stale #433 leftover would preserve+400 on a cold+no-disk send.
Mechanism:
- resolve_model_provider(model_id, *, explicitly_picked=False): in the custom-arm
cold branch (no warm provenance, not config-declared), explicitly_picked=True
preserves the vendor namespace verbatim (deliberate pick, proxy routes on it);
otherwise the legacy redundant-prefix strip runs (stale cross-provider leftover,
#433). Warm endpoint-advertised provenance still wins over the flag.
- Session.model_explicitly_picked: new persisted field (default False, restored
from metadata via **kwargs, listed in METADATA_FIELDS so it survives a restart
— b3nw's exact cold scenario).
- /api/chat/start stamps it: True on a fresh explicit_model_pick; cleared only
when the resolved model actually CHANGED without a pick (real switch away);
preserved across plain follow-up sends of the same model (the onchange marker
is one-shot, so per-send explicit_model_pick alone would drop the intent).
- streaming.py reads s.model_explicitly_picked and passes explicitly_picked= into
resolve_model_provider (inside the profile scope).
Independent end-to-end matrix (all pass): ben default cold, ben picked non-default
cold+warm, ben stale cold (strips), #433 warm+cold, #3872 bedrock, #548 zai-org.
67 resolver+sprint40 tests pass incl. new explicit-pick + stale-companion +
persistence round-trip.
Codex finding: the warm helper's fast no-op (if _models_cache_provenance is not
None: return) ignored WHICH profile the resident provenance belongs to. The
catalog globals are process-wide, so profile B's resident provenance could block
profile A from loading A's own valid disk cache — A would then resolve against
B's advertised ids (verified: profile A provenance resident + profile B bare-only
disk cache -> A's openai/gpt-5.4 wrongly preserved).
Fix: gate BOTH the fast no-op and the post-lock recheck on a current-profile
fingerprint match (_provenance_is_current: prov[1] == _models_cache_source_fingerprint()).
On mismatch the helper falls through, loads THIS profile's disk snapshot, and
republishes provenance stamped with the current fingerprint.
Tests: rewrote noop-when-warm to use the CURRENT runtime fingerprint (genuinely
hits the fast path) and added ignores-foreign-profile-resident (foreign fp -> falls
through -> loads this profile's disk snapshot). 66 resolver+sprint40 tests pass.
NOTE: Codex also re-raised the cold+no-disk #433 case (a stale openai/gpt-5.4 on a
bare-only relay preserves instead of strips when no catalog exists at all). Fable
weighed this exact residual and judged cold-preserve strictly better than master
(loud-fail-and-heal vs silent-recurring-truncation). The two advisors diverge;
surfacing to Nathan rather than auto-adopting the heavier persisted-explicit-pick
mechanism.
Codex gate found two real issues in the send-path warm:
1. Blocking lock wait (CORE): get_available_models(prefer_cache=True) still
acquires _available_models_cache_lock and can wait up to ~60s (unbounded in
synchronous rebuild mode) on an in-flight rebuild — unacceptable on the send
hot path. Rewrote warm_models_catalog_provenance_if_cold as a genuinely
non-blocking, disk-only publish: try the cache lock NON-BLOCKING (return
immediately if busy — a concurrent build will publish provenance itself),
read ONLY the on-disk cache via _load_models_cache_from_disk (no network, no
rebuild), publish snapshot+fingerprint + _sync_models_cache_provenance().
Verified: returns in ~3.5ms while the lock is held (was a 60s block).
2. Profile scope (CORE): the streaming worker is a separate thread that does NOT
inherit the HTTP handler's request-profile TLS. Both the warm and the resolve
read profile-keyed config (cache path + source fingerprint via
get_active_profile_name), so a cold send from a NAMED profile could resolve
against the DEFAULT profile's config and route to the wrong provider/base_url
(get_available_models reloads config on path mismatch). Wrapped warm + resolve
in profile_scope_for_detached_worker(_resolved_profile_name) so both see the
owning session's profile (no-op for default/root).
Tests: rewrote never-live-rebuilds (now asserts warm NEVER calls
get_available_models — reads disk directly) and added a non-blocking regression
(warm returns <2s while the cache lock is held). 65 resolver+sprint40 tests pass;
b3nw end-to-end + non-blocking + disk-warm all verified.
The earlier #5979 fix (PR #5980, exp-v0.52.53) resolved the WARM-catalog and
config-declared cases, but b3nw confirmed truncation persists on exp-v0.52.55/56.
Root cause (reproduced on master, confirmed by Codex + Fable):
b3nw's config is provider: custom:llm-proxy, model.default: x-ai/grok-4.5, no
models[] allowlist. His DEFAULT (x-ai/grok-4.5) preserves via config-declaration,
but a model he SELECTS in the picker that isn't the default (x-ai/grok-composer-2.5-fast)
truncates on every COLD send. The send path (api/streaming.py:7857) calls
resolve_model_provider without warming the catalog, and the #1855 chat/start fast
path deliberately skips the catalog when a session already has a persisted
model+provider -- so 'cold at send' is the designed state after any restart /
settings-save / cache-expiry, not a rare race. With provenance None and no config
declaration, the #5980 tri-state fell through to the LEGACY family strip
(if prefix in _PROVIDER_MODELS and _is_first_party_model: return bare), and
grok-composer-2.5-fast had graduated into the x-ai first-party catalog -> stripped.
Fix (two changes, both advisor-verified):
1. Flip the custom-arm cold default from family-strip to PRESERVE-verbatim
(api/config.py). For an explicit custom / custom:<slug> proxy the strip now
requires POSITIVE provenance (endpoint advertises only the bare id, #433). A
wrong strip destroys routing info the user can't recover; a wrong preserve
fails loudly and self-heals on warm. This also honours _endpoint_advertised_model_ids's
own documented 'None -> preserve verbatim' contract and removes the data-driven
flaw where a model graduating into the static catalog silently flips routing.
2. Network-free provenance warm on the send path
(warm_models_catalog_provenance_if_cold, called in streaming.py before the
resolve). prefer_cache=True republishes provenance from the durable disk cache
(no live probe, no _cfg_lock held on the worker thread -> no lock-order
deadlock), so #433's bare-only-strip stays exact whenever a disk cache exists.
Residual (accepted, strictly better than master): a #433-shaped wrong-preserve
now requires memory cold AND disk cache absent/invalid (first run, post-config-save
window, schema/version bump). It fails loudly and heals on the next /api/models or
session-visit rebuild. Under master b3nw's failure was the mirror image but silent,
recurring every send, and unfixable short of declaring every model in config.
Supersedes rodboev's #5996, which targeted the providers.<slug> arm and does not
cover b3nw's actual custom:llm-proxy config (confirmed by Codex).
Tests: rewrite the cold-fallback test (strip -> preserve), sharpen the
foreign-profile test so trusted-strip vs ignored-preserve genuinely diverge, add
b3nw's exact cold non-declared custom:slug case, and add three warm-helper tests
(publishes-from-disk, noop-when-warm, never-live-rebuilds). 64 resolver+sprint40
tests pass; b3nw end-to-end verified; #433 positive-provenance strip verified.
Thanks @b3nw for the CLI-vs-WebUI comparison that localized it.
Final-answer detection understands structured content parts (text after tool-use
boundaries); genuine empty completions still show no_response; terminal provider
error still surfaces as apperror (never masked). Codex SAFE round2 (3 precedence
findings fixed), stream-gate GREEN all modes. Maintainer: updated the #5940 source-
assert test for the _captured_terminal_failure rename.
nesquena-hermes