capability_fingerprint ignored model.supports_vision and model.context_length,
so an eternal Bot Chat kept its stored prompt after those overrides changed.
A NULL stored prompt already rebuilds and no longer takes the stale probe.
getconf _AVPHYS_PAGES counts page cache as used, so the Desktop statusbar
overreports Linux RAM. Read MemAvailable from /proc/meminfo, fall back to
MemFree, then the existing getconf probe. A reported 0 is a real available
value, not a missing field.
Fixes#102252
The original->norm map is strictly increasing (no UNICODE_MAP entry is empty),
so bisect_right(...)-1 returns the same index as the inverted-dict lookup on
every hit and the expansion-start snap on a miss. One lookup instead of a dict
plus a fallback branch. Found by the /simplify-code reuse+quality reviewers;
verified with a 2000-case randomised probe.
The em-dash case only exercises a 2-char expansion ('--'), where the
mid-expansion equal block can start at exactly one offset. The ellipsis
('...') expansion lets the boundary land two chars deep, so the bisect
snap must walk back more than one norm index. Keeping it inside the
existing test pins the whole boundary class without adding a test
function.
Co-authored-by: Yuan Li <dskwelmcy@163.com>
In _preserve_unicode_in_replacement, SequenceMatcher equal blocks whose
start index lands inside a multi-char Unicode expansion (em-dash -> '--')
have no direct original position; the old fallback mapped them to 0 and
spliced file_region[0:...] into the replacement. A file line
'value = x—y' edited with old 'value = x--y' -> new 'value = x-@-y'
came out as 'value = x—@value = x—y' — the whole region duplicated
after the edit.
Snap such boundaries back to the expansion's first original char via
bisect on the orig->norm map (the whitespace-collapse path already had
the analogous >= norm_start fallback). The em-dash is kept whole and the
insertion lands once: 'value = x—@—y'.
(cherry picked from commit 86dfb1150468c89d8e9889641ce2afa36e92809f)
`HIGH_EFFORT_SILENCE_FLOOR_SECONDS` is documented as the reasoning-effort
>= high floor and gated on effort by `_high_effort_silence_floor()`, yet
the first-progress budget applied it to every large official-Codex
request regardless of effort. Someone tuning the high-effort floor would
silently retune the first-progress fuse. `CODEX_FIRST_PROGRESS_TIMEOUT_SECONDS`
carries the same 300s, so no behaviour changes; it just gets its own knob
and rationale.
The new lifecycle-only test re-implemented `_shorten_implicit_idle_watchdog`
inline. The helper now accepts field overrides, and the test also inherits
its env cleanup so a polluted runner env cannot push the resolver off the
implicit branch.
The notice builder and the kill loop each hand-computed
`retry_started_ts or call_start` and the "stream open, no progress yet"
predicate. Two copies that must agree or the notice countdown and the
actual kill diverge. `_codex_watchdog_snapshot()` now returns the
attempt origin under the same lock, and `_pre_progress()` is the single
phase predicate for both sites.
Also restores the base preference for `retry_started_ts` over
`last_event_ts` as the non-pre-progress activity anchor. On head a
reconnect marker only coexists with events during pre-progress (retries
reset `last_event_ts`; first progress clears the marker), so the
ordering is behaviour-neutral in production and keeps the existing
`(0.0, 0.0, 1.0)` wait-notice case green.
The attempt-local first-progress deadline added `progress_timeout` to
`_NonStreamWatchdogs`, but the SimpleNamespace stubs in
test_nonstream_wait_notice.py and test_wait_notice_cadence.py were not
updated. `_emit_wait_notice` reads `wd.progress_timeout` inside its
blanket `except Exception`, so the AttributeError silently produced no
notice and no liveness touch: 11 tests red.
The new "Codex stream opened" debug line also called `time.time()`
eagerly, consuming a tick of the 3-tick stub in
test_codex_first_event_timing.py and shifting the first-event stamp.
The first-event log a few lines below already carries the attempt's
timeline anchor, so the open marker logs without an epoch.
The per-attempt "first parsed event" / "first substantive progress" /
"physical retry" markers are what an operator needs to see where a
lifecycle-only Codex attempt stalled, so they stay at info. The
"stream opened" line fires on every attempt and carries no diagnostic
value on its own, so it goes to debug to keep the default log quiet.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The previous commit replaced the positional write with _replace_entry, leaving
the enumerate index unused (ruff B007; flagged by the pre-arm simplify gate).
`_credential_token_pair` already maps a non-dict row to (None, None), so the
separate isinstance guard and second early return in
`_merge_pool_row_generation` were the same branch. Adopting a peer generation
now goes through the existing `_replace_entry` swap primitive.
The profile-claims-its-own-credential branch of add_entry wrote the
profile store directly, so the newly owned rows had no recorded token
base until the next pre-refresh sync, and a stale peer could not be told
apart from an unknown one on the first later flush. Pass the current bases
and reseed from the written rows, mirroring _persist().
_persist() may swap a peer's newer token generation into self._entries,
but _adopt() returned the pre-persist object. Callers that rebind the live
client from that return value (try_refresh_matching -> _swap_credential,
auxiliary_client, account_usage) would keep using the stale pair for one
guaranteed 401 while the pool already held the rotated one. Re-look the
entry up by id after persisting so the return value matches the pool.
The "adopt only if the written pair differs from the live pair" guard is
kept: without it every ordinary flush would replace the live object and
pull peer cooldown state merged into the written row into memory. The
kept generation test now asserts an unchanged-pair flush preserves object
identity and that a stale writer's _adopt returns the adopted entry;
reverting either the guard or the re-lookup fails both parametrizations.
Also move the reference-only-rows comment onto the blank-pair guard it
describes instead of the rehydration statement.
The id->token-pair base map was built three ways: _persist dropped
(None, None) pairs, load_pool and _sync_entry_from_pool_store kept them.
_merge_pool_row_generation treats a missing base as "unknown" (plain
recency merge) but a (None, None) base as a known generation, so a
token-less row got the generation override on the first flush after
load_pool and the plain merge on every later one.
Policy chosen: KEEP blank bases everywhere (new auth_mod._token_pairs_by_id,
built on the existing _entry_ids). A blank base means "no pair when we last
looked", which is exactly the CAS witness the boundary needs: when a peer
lands a pair on that row, every flush of ours keeps the peer's generation
instead of writing our blank tokens back. Dropping blank bases would make
the first flush keep the peer's pair and later flushes overwrite it.
_sync_entry_from_pool_store now records the base before its no-token-
material bail-out so all three sites agree. _update_root_pool_rows reuses
_entry_ids for its incoming map as well.
Also collapse the three copy-from-disk-else-pop loops in
_merge_pool_row_generation into a local _take_from_disk helper.
failure_reason is passed alongside _POOL_STATUS_FIELDS rather than added
to the shared constant, which auth_codex/auth_oauth_grants and
_merge_disk_cooldown_state also consume.
The terminal-refresh visibility tests build CredentialPool via __new__ and
hand-set its slots, bypassing __init__. Since _persist() now reads
self._persisted_token_pairs directly (the getattr shim was dropped on
purpose), the four cases that reach _persist raised AttributeError. Seed
the attribute in the fixture like the other slots instead of restoring a
defensive getattr in production code.
After the first stale writer's select() adopted the peer's pair, its next
persist was no longer stale, so the folded 402 assertion passed even on
the bare #120943 change it is meant to guard. Issue the 402 from a third
pool that still holds the pre-rotation pair; the assertion is now red on
#120943 alone ('ok' != 'exhausted') and green with the narrowed merge.
write_credential_pool now takes token_bases, so the in-memory fake standing
in for auth.json in the refresh-race tests must accept it or every persist
in those tests raises TypeError inside the refresh thread. Also carries the
credential-pools doc sentence describing the stale-writer boundary.
Hunks taken verbatim from #120816.
When a stale pool's persist finds the disk pair moved, #120943 copied every
status field from disk, so the stale writer's NEWER account-wide verdict
(402 billing / 429 throttle -> EXHAUSTED) was erased and the rotated pair
re-entered selection immediately. Only a terminal auth death is tied to the
pair it was observed on: discard the stale writer's DEAD onto the peer's
pair, but leave any other status for _merge_disk_cooldown_state's ordinary
recency merge.
Also travel `scope` and `inference_base_url` with the pair (a Nous refresh
rewrites them together with the tokens, so a stale writer must not stamp an
old scope/route onto the newer pair); use the always-initialised
_persisted_token_pairs directly; and re-hydrate an in-memory entry from the
written row only when the store overrode its pair, so unchanged rows keep
runtime-only fields to_dict() omits.
Tests: fold the stale-later-402 witness (rt-1 kept AND last_status ==
exhausted) into the stale-terminal-verdict test and drop the separate 429
rollback test it subsumes.
Fixes#120815
Co-authored-by: John Paul Soliva <soliva.johnpaul@icloud.com>
handle_api_interrupt exits via break into finalize_turn, whose
_close_transcript_tail already closes the tail with the same final_response, so
its own close was a duplicate; the scaffold strip stays (the partial-text row
must follow the tool row). The scaffolding drop no longer returns a flag nobody
reads, and the give-up test relies on the autouse backoff stub in
tests/agent/conftest.py instead of re-stubbing it.
The kept Stop test passed with the `abort_turn_on_interrupt` scaffold strip
reverted (gate mutation: 68/68 green) because it only checked "pair kept,
tail not tool". Assert the closing row carries the owner's own reason so
reverting that hunk goes red.
The `real_loop` fixture let the first empty response sleep its real 5-7.5 s
backoff; the give-up path is reached through the deterministic-empty guard,
not the wait, so stub `jittered_backoff` to 0 in the fixture (the Stop test
still overrides it). Also drop the never-taken `callable(pending[0])`
branch: every scripted response is a SimpleNamespace.
`_persist_session` closed the tool tail it uncovered after popping
empty-response scaffolding with the generic "Operation interrupted." row.
That close was dead on every path whose owner already shapes the tail
(finalizer, abort_turn_on_interrupt) and wrong where it did fire: a
non-interrupt terminal exit (retries exhausted, invalid response, rate
guard) and even the give-up path got an "interrupted" row the finalizer
then followed with a second closing row, and an in-flight Stop during the
nudge request lost its own "Operation interrupted: waiting for model
response (...)" reason because the finalizer's close no-ops on an
assistant tail.
The persist layer now only pops the scaffolding. `handle_api_interrupt`
strips it before appending its partial/placeholder row and, when it has no
partial text, closes the tail with its own reason, mirroring
`abort_turn_on_interrupt`. The give-up path is closed by
`_close_transcript_tail` with the delivered final_response, as before the
stack.
The retry_wait and nudge_request parametrizations exercise the same
invariant (an executed assistant(tool_calls)/tool pair survives a Stop
mid empty-response recovery, and the saved tail is not a bare tool row).
Keep the deterministic retry_wait case and drop the thread-event variant
so the stack adds two invariant tests, not three.
abort_turn_on_interrupt closes an open tool sequence with the caller's
specific interrupt text and then persists. When a Stop lands during
empty-response recovery, the synthetic assistant+nudge pair still sits
after the executed tool result, so the close sees no exposed tool tail
and the generic close in _persist_session wins with "Operation
interrupted.". Strip only the request-local scaffold first, so the exit
owner closes the tail with its own reason.
(taken from 413ad9647b72b27e39c49d8e0daa05bac5411ec1 in #120883,
agent/turn_recovery.py hunk only)
After an empty-response give-up or a Stop mid-recovery,
_drop_trailing_empty_response_scaffolding popped the flagged scaffolding
and then also rewound the trailing tool results and their
assistant(tool_calls) row. Those rows are saved before the tools run, so
the rewind only removed them from the live history: result["messages"]
(which CLI, TUI and ACP reuse as the next turn's history) lost a tool call
that had already executed, and a "continue" made the model run it again.
On a Stop, the finalizer then found no tool tail to close, so the saved
transcript ended on an unanswered tool result.
Drop only the flagged scaffolding rows. _persist_session closes a tool
tail the drop uncovers with close_interrupted_tool_sequence, which covers
the exits that return without the finalizer; _terminal_empty and the
finalizer already close the tail themselves.
(cherry picked from commit 9fe02fc7f27742a28ece5f789be78cfa4b4052ef)
A list or string JSON error body used to raise AttributeError out of
_refresh_access_token; the guard added in the previous commit had no row
exercising it (pre-arm gate: reverting the guard left the suite green). Two
rows: a 400 list body stays non-terminal, a 401 string body is invalid_grant.
Red with the guard reverted (2 failed), green on head.
A JSON body that is a list or string reached `.get()` and raised AttributeError
instead of an AuthError. Treat it like a non-JSON body. Rename the parametrised
test to what it now covers (terminal and non-terminal rows) and state the
401/403 rule in the comment without pointing at a sibling it only resembles.
`format_auth_error` already appends "Please retry in a few seconds." for
`code == "temporarily_unavailable"`, and every CLI surface routes through
it, so the raised message ended up telling the user twice:
"...(HTTP 503); retry shortly. Please retry in a few seconds." Let the
code-keyed formatter own the remediation text, as it does for every
other provider's transient error. No test asserts on the message text.
The stack stopped defaulting an unknown non-200 token-endpoint body to
`invalid_grant` so a 429/404 gateway page no longer wipes credentials.
That also silently demoted 401/403 with a non-OAuth (or non-JSON) body
from terminal to "bench and retry every cooldown", so a dead grant on a
Portal that answers 401 without an `error` key would never prompt a
re-login. A 401/403 from the token endpoint always means the refresh
token itself was rejected, so mirror the Codex sibling
(auth_codex.py `status_code in {401, 403}` -> relogin) and keep the base
grant-dead default for exactly those two statuses; every other status
without an `error` key stays non-terminal.
Folding the non-JSON branch into the same path lets the `code`
coercion collapse to one line (gate quality finding).
Test: extend the kept parametrized row set with 401 (JSON, no `error`)
and 403 (non-JSON) rows asserting terminal; both fail on the previous
commit. Non-5xx rows no longer pin `retryable is None` (an unspecified
"raiser did not say" value), only the 5xx rows assert `retryable is True`.
_refresh_access_token defaulted any non-5xx JSON error body lacking an
`error` key to `invalid_grant`, and flagged a non-JSON body with
relogin_required=True. A 429 quota body or 404 gateway body says nothing
about the refresh token, yet the default made it a terminal grant error
that wipes the OAuth state and quarantines the pool entry (sibling of the
5xx outage path fixed for #120976).
Take the code as the server sent it (None when absent), derive
relogin_required from the shared _OAUTH_GRANT_DEAD_CODES set, and leave
the non-JSON branch non-terminal so the user is not told to re-login for
a transport-shaped failure. Fold the 429/404/non-JSON cases into the
existing outage parametrize so the stack still adds two invariant tests.
Drop test_refresh_token_exchange_terminal_oauth_errors_still_require_relogin:
the terminal invalid_grant / refresh_token_reused paths are unchanged by the
5xx gate and already covered by the existing reuse-detection and invalid_grant
tests, so it only guards behaviour that did not move. Trim the 5xx parametrize
to the range endpoints plus 503 (the real outage status).
`_batch` kept three accumulators holding the same selected entry; keep only
the per-op `matched` list and build `replaced_entries`/`removed_entries` from it
on commit. The background delete gate now asks `destructive_ops` instead of
re-spelling the predicate, `destructive_ops` tolerates a None op like the rest
of the batch path, and the pinned lookup no longer threads a dead `ambiguous`.
