Codex re-gate found the single-row resize fast path used parseFloat on
getComputedStyle(el).minHeight, which reads a bogus pixel value for a
non-px unit (parseFloat('50%')===50) and would wrongly enable the fast
path, leaving an oversized composer stuck tall. Parse with a strict
/^<number>px$/ test so %, auto, empty, and calc() fail closed to the
full resize. Adds red-before-green regression coverage across units.
Co-authored-by: sheldon-im <sheldon-im@users.noreply.github.com>
Fable UX polish pass on the gateway approval counter:
- Move the crude inline style (font-size:0.75em;opacity:0.6;margin-top:4px) into
a proper .approval-counter rule: 11px, color:var(--muted) (skin-safe vs opacity),
margin:0 0 12px (fixes the 0px gap that left it flush against the buttons),
tabular-nums for stable digits.
- i18n the '1 of N pending' string (was the only hardcoded-English string in an
otherwise fully-localized approval card): new approval_pending_count key in en
+ 14 locales, wired via t() with an English fallback.
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Codex gate on #6143 found a SILENT regression the reuse-path fix introduced:
_replace_session_db_in_kwargs early-returned the OLD handle when the rebuild
failed, but master assigned None. If the old handle was already CLOSED, both
credential self-heal callers (streaming.py:9032/10253) then built a fresh agent
around a closed SessionDB -> every persist/search fails with 'NoneType has no
attribute execute' while the chat keeps going. Guard on _session_db_is_open:
keep a live handle (subagents hold it by ref), else degrade to None (clean lazy
reinit, master behaviour). +2 regression tests.
Co-authored-by: carlotestor <carlotestor@users.noreply.github.com>
Codex found the body-reparented picker (z-200, later body child) could be revealed
UNDER a full-screen drawer/panel via multiple paths (swipe, breakpoint resize,
profile-switch). Rather than guard each open-site (whack-a-mole + my resize guard
over-fired on keyboard reflows), fix the class once: .model-dropdown--floating gets
z-index:190 — above the composer-mobile-config-panel (z-180) it opens from, below
the mobile sidebar + workspace panel (both z-200) — so any drawer/panel that opens
covers the picker, matching master's stacking intent. Reverted the two boot.js
guards. Co-authored-by webtecnica.
Codex found a sibling of the swipe-stacking edge: a direct landscape/tablet->phone
resize can re-reveal an already-open mobile sidebar or workspace panel under the
body-reparented picker (z-200 siblings, no click to trigger outside-close). The
window resize handler now guardedly closes+restores the picker before re-syncing
drawer state, gated on entering phone width AND a drawer/panel actually being open,
so ordinary keyboard/URL-bar reflows don't dismiss the picker mid-use.
Codex re-gate found the body-reparented picker (z-index:200, body child) would
paint on top of the mobile sidebar drawer when a left-edge swipe opened the
sidebar while the picker was open. _openMobileSidebarFromGesture() now calls
guarded closeModelDropdown() first (which also restores the dropdown to the
footer), so the swipe reveals the session drawer cleanly. Co-authored-by webtecnica.
The composer model dropdown (#composerModelDropdown) is a child of
.composer-footer, which sets container-type:inline-size (and, under the Geist
Contrast skin, a backdrop-filter). Both establish a fixed containing block, so
the phone position:fixed dropdown resolved against the FOOTER (pinned to the
bottom of the screen) instead of the visual viewport — dropping the menu below
the fold on all skins (the "tapping the model picker does nothing / opens
off-screen" symptom in #6080).
Fix (Fable UX gate recommendation on PR #6105): on the phone path
(<=640px) reparent the dropdown to <body> and switch it to position:fixed via a
.model-dropdown--floating class, exactly the working #profileDropdown idiom, so
fixed positioning is viewport-relative on every skin. The visualViewport math
from PR #6105 is preserved (viewport-relative coordinates, above/below flip
preferring the larger space, width = viewport - 16px, titlebar-bottom clamp) and
so are the rAF-coalesced visualViewport resize/scroll listeners.
Desktop (>640px) is unchanged: closeModelDropdown() and the desktop branch of
_positionModelDropdown() restore the element into its original .composer-footer
slot and clear the phone inline styles, so the desktop popover stays an
absolutely positioned footer child, byte-for-byte identical to master. Because
the menu now leaves the footer on phones, no per-skin backdrop-filter override
is needed.
Adds tests/test_issue6080_model_picker_reparent.py, including a node-executed
DOM-parentage guard that runs _positionModelDropdown()/closeModelDropdown()
against a stub DOM and asserts the open phone dropdown has NO .composer-footer
ancestor (escaping the container-type trap) and is restored into the footer on
close and on desktop.
Co-authored-by: webtecnica <75556242+webtecnica@users.noreply.github.com>
Scoped to the two stated fixes only: strips the #composerModelDropdown position:fixed
mobile override that #6104's branch had bundled in (that's the #6080 model-picker
mechanism, being reworked separately to reparent to body level — the fixed override
is trapped by .composer-footer's container-type so it doesn't belong here), and drops
the <=360px tool-name-hiding rules (header overflow-clip prevents the spill without
losing tool identity). Thanks @webtecnica.
Codex re-gate found the same derived-title leak on the sibling /api/sessions/search
endpoint (3 branches redacted only 'title'). Extract _redact_sidebar_title_fields()
shared helper, apply it in both _sidebar_session_response_item and all 3 search
branches, read api_redact_enabled once per search response (mirrors #4662). +helper
test + source-guard that every search branch calls the redactor.
Codex gate found the #6056 derived display_title (from raw user-message content)
bypassed sidebar redaction — the serializer redacted only 'title', so a credential
in a delegated goal leaked to the sidebar even with api_redact_enabled=True.
Redact display_title + _state_db_title + parent_title on the same path; +regression
test (proven to fail on the unfixed serializer).
Codex re-gate asked to distinguish a confirmed-absent sidecar from an
indeterminate/unreadable one before entering the abandoned-shell grace path.
Add _session_sidecar_exists() (True/False/None) and only grace-evict when the
sidecar is confirmed absent (False); a present-or-indeterminate sidecar keeps
the session resident. Age-window bound retained (fully closes the leak, no
data-loss edge); an SSE-mounted-lease refinement is noted as optional follow-up.
Codex gate found the eviction-reorder made never-saved empty shells immortal
(abandoned /api/session/new tabs accumulate unbounded past sessions_cache_max).
Add a grace-window bound: protect a never-persisted empty shell only while fresh
(<30min old) OR while it carries a composer draft; a stale, empty, draftless,
never-saved shell is treated as abandoned and becomes evictable again. A real
compose persists a draft on the first keystroke (disk-backed + reloadable), so
the brick fix is preserved while the leak is closed. +2 regression tests.
Distilled habits that get a PR merged in one review round instead of several:
four root causes + ten rules + show-your-work PR-description contract. Full doc
in docs/GUIDELINES.md, compressed 10-point form in AGENTS.md (agent entry point),
pointer added to CONTRIBUTING.md reading list.
Maintainer revision of #5806: the localization refactor (all profile-concept
help text now goes through t() so it's translatable via the documented
English fallback) is the core value, and the concrete Example row genuinely
clarifies how profiles + workspaces work together. The Binding row restated
the Together row + added scheduled-task edge-case trivia that muddied a concept
explainer, so it's dropped (row + its 2 i18n keys + its test).
Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
- workspace 'scopes file access' -> 'keeps chat, terminal, and file browsing
pointed at that project' (terminal/agent tools can access outside the workspace).
- scheduled tasks 'run under the profile that created them' -> 'run under the
profile selected on the task, or the WebUI server default when none selected'.
Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
The Cmd/Ctrl+/ predicate keyed off e.code==='Slash' + !e.shiftKey, which on
QWERTZ layouts (a) stole Ctrl/Cmd+- browser zoom-out (physical Slash there is
the minus/zoom key) and (b) missed the real shifted-'/' chord. Match e.key==='/'
instead — layout-correct on every keyboard. Tests updated to assert the physical
code isn't used and a layout-shifted '/' still matches.
Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
Measure-only TTFT (ttft_ms) on the metering session + _firstTokenMs diagnostic. Computed once
on first token (no hot-path latency), all mutation under existing GlobalMeter._lock, existing
token/usage/cost metering unchanged. Codex SAFE (16-stream concurrency probe, no leak/None-deref)
+ stream-gate GREEN all modes + 22 tests.
Local (non-gateway) _resolve_approval_legacy collapsed 'once'+'session' into approve_session()
→ 'approve once' silently persisted session-wide. Narrowed to choice=='session' only; 'once'
still resolves the current call but doesn't persist. Codex SAFE (adversarial: once resolves
current+no residue+next re-prompts, session/always/deny unregressed, gateway consistent, tests
non-vacuous + parked-thread probe of all 4 choices).
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).
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.
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.
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.
nesquena-hermes