A fresh process with an unchanged store takes zero auth-store locks, and a
root store that gains a forked grant invalidates the persisted mark so the
heal re-runs. The corrupt-mark fallback, same-mtime size change and
no-secrets checks were pinning implementation details of the same cache.
`_heal_forked_single_use_oauth_grants()` runs on every `load_pool()`, and its
clean mark lived only in memory. Every fresh `hermes` invocation and every new
worker therefore re-took the heal's two nested EXCLUSIVE auth-store locks just
to rediscover a store it had already cleared — 4 acquisitions per process on a
two-provider profile, every one of them finding nothing to consolidate. Behind
a sibling process holding those locks that costs a full
`AUTH_LOCK_TIMEOUT_SECONDS` per provider before the process can do anything at
all: measured 30.1s for two providers.
Persist the mark next to the store it describes (`<profile>/cache/
oauth_heal_clean.json`, 0600, paths and stat data only — no credential
material) and consult it BEFORE taking any lock.
Outliving the process means the mark needs a stronger key than the in-memory
one did:
- The ROOT store joins the fingerprint. This heal consolidates root → profile,
so root acquiring a counterpart turns a row the heal deliberately KEPT into a
fork it must strip. The in-memory mark could ignore root because it died with
the process; a persisted mark would keep skipping a heal that has become
necessary.
- File sizes join it too, so a metadata-preserving rewrite (`rsync -t`,
`tar -p`, a restore) cannot leave a stale mark looking current indefinitely
rather than for one process.
Measured on an isolated HERMES_HOME with two OAuth providers:
lock acquisitions per fresh process 4 -> 0
contended load_pool() x2 30.1s -> 0.00s
mark-file reads per 100 load_pool() - -> 2 (one per provider)
The mark stays a cache: absent, unreadable, corrupt or wrong-shaped content all
mean "unknown" and fall through to the locked heal, and a failed write only
means the next process re-runs it — the behaviour before this cache existed.
An HTTPError means the host answered — a 401 from a wrong API key must not
be remembered as "unreachable" for the next 60s, or a user who fixes the key
gets a cached empty catalog on the immediate re-probe. Connection-level
failures (timeouts, refused, DNS) are the only thing the cache records.
_probe_neg_key hand-rolled scheme/port defaulting that utils.base_url_origin
already provides; use it.
The contributor's 15-test module pinned encoding details (frozen-arg shapes,
postponed-annotation resolution, monkeypatch seams). The two behaviour
contracts that matter survive: a 12-request /api/profiles burst admits ONE
worker and leaves /api/status responsive on a 2-token pool (this file), and
the kanban board read shares one worker per key (test_kanban_read_admission).
Both go red when the coalescing wrapper is removed from the route.
_nous_picker_model_ids only uses the ids the Portal unions append — both
unions discard the pricing map (`model_ids, _ = union_with_portal_*`) — yet
it called get_pricing_for_provider("nous") without cached_only, so a cold
pricing cache paid a full /v1/models round-trip (network timeout on a slow
Portal) on the picker-open path for nothing. Pass cached_only=True; the
background pricing prewarm (#101685) fills the same cache for later opens.
Re-derived from #102099 by @finn763: the original patched
hermes_cli/model_switch.py, which 3b1ecfc0a1 decomposed; the live call site is
hermes_cli/model_switch_providers.py.
Based on #102099 by @finn763.
Replace the attribute-presence check (asserts a cache_clear attribute the decorator does not
expose) with one invariant: eight concurrent projects/tree requests run the profile scan once
and each caller gets its own copy of the payload. The scope-suite fixture now disables the TTL
instead of calling a non-existent clear hook, which silently no-op'd.
The all-profiles sidebar polls GET /api/profiles/projects/tree, the one
heavy sidebar endpoint that was not wrapped in @_sidebar_singleflight_cache.
Every poll fanned out list_profiles() + _build_project_tree() over every
profile (51 on this box, ~17k SKILL.md files walked), and each profile
resolution re-parsed its config.yaml because read_user_config_raw() ran
uncached. On a 2-vCore VPS running 'hermes serve' for the Desktop remote
backend this pinned both cores (py-spy: 110-116% sustained, HostHighCPU).
- Wrap get_profiles_projects_tree in the existing single-flight cache,
matching get_profiles_sessions_sidebar (5s TTL, errors[] not cached).
- Memoize read_user_config_raw() on a (st_dev, st_ino, st_size,
st_mtime_ns) fingerprint under _CONFIG_LOCK, same strategy as
read_raw_config(). Hits return a deepcopy so write-back round-trips keep
their fresh-dict semantics; parse errors are never cached; the docstring
no longer claims 'no caching'.
- Tests: memo semantics (deepcopy isolation, inode-replace reparse, error
non-caching, parse-count), a wrapper-presence pin for both heavy sidebar
endpoints, and a cold-cache autouse fixture in the scope tests (the 5s
TTL otherwise leaks one test's payload into the next).
Measured on the affected box: list_profiles 1.9s -> 0.16s warm, serve CPU
112% -> 22-34%, host CPU 50-70% -> ~26%.
- Pass 1 no longer materialises every classified record (full
`messages.content` included) until pass 2; `LayoutEvidence` keeps only the
capped per-position value sets (+ sessions rows for the one cross-column
invariant) and pass 2 re-streams the lost_and_found tables. A 276 MB
corrupted store no longer has to fit in memory.
- `_sentinel_holds` / `_text_shape_holds` if-ladders become rule tables.
- Tests trimmed to the three that bind behaviour (upgraded store maps by
name; verifier refuses when rows matched no layout; replayed history ends
at the current schema — the drift guard). No behaviour change; reverting
inference to "no layout" still fails the name-mapping test.
A store's physical column order depends on which schema it was created at and
which ALTER TABLE ADD COLUMNs it lived through; one hardcoded "upgraded"
layout cannot cover them. `session_schema_history` records the declared
schema of the salvaged tables over time; `reachable_physical_layouts`
replays it to enumerate every physical order a store can have. The mapper
infers the layout once per kind from the whole recovered population (one
store wrote all of them), maps cells by name, and counts records whose width
matched no layout; the recovery verifier refuses to report such a salvage as
healthy.
Rebased onto the simplified session modules (c88d60551e, b9b4600cb2,
7e5a1a11d9, 1915a0d27e); no behaviour change from the pre-rebase branch.
Upgraded state.db files gain columns via ALTER TABLE ADD COLUMN, so
physical order diverges from SCHEMA_SQL. Prefix-mapping onto the fresh
template put started_at at 0 and shifted titles/models. Insert by name
using known physical layouts; keep the plausibility gate.
Fixes#101409
Follow-up to the probe-unknown guard:
- `require_wal=True` now raises WalUnsupportedError when the on-disk mode
cannot be read instead of reporting an unverified "wal" (the function's
contract is "the mode actually set").
- Drop `_apply_wal_companions` from the branch: journal_size_limit /
synchronous pragmas were being applied to a file whose mode is unknown,
contradicting the "touch nothing" rule the branch exists for.
- Cut the retracted #104596 mechanism narrative from comments, log text and
docstrings; the guard is hardening (same rule the DELETE branch already
applied), not a root-cause fix.
- Tests: one binding test per file (no pragma of any kind reaches the
connection; require_wal raises); drop the log-dedupe test (`_log_once`
is already covered). test_captures_cause_on_failed_init's double now
fails only set-pragmas, matching the read-only-mount it simulates.
Startup only quarantined a 0-byte / all-NUL state.db. A file whose first page
was clobbered with record bytes (#102198) went straight to sqlite3.connect,
which raised "file is not a database" and deleted the -wal sidecar — the one
piece of evidence that could have been recovered.
`has_invalid_sqlite_header_preopen` generalises the zeroed probe (zeroed is a
subset of "no SQLite header"; same live-connection contract, never raises).
`quarantine_invalid_state_db` moves the file AND its -wal/-shm aside as
`state.db.<zeroed|notadb>-<ts>-<pid>.bak` before anything opens it; a fresh
DB is created as before.
Re-authored on current main (the quarantine helpers moved to
hermes_state_dbfile.py in d15c61b5dc); one regression test proves the
notadb case is quarantined with its sidecars and the new DB passes
integrity_check.
Refs #102198 (the write-after-SIGTERM that clobbers page 0 is not addressed
here; this preserves the evidence instead of destroying it).
The overflow-terminal path ends the turn without reaching finalize_turn, so
a transcript that overflowed right after a tool batch ended on a raw tool
result; strict providers reject the next user turn (tool -> user). Close it
with the same final text, mirroring the truncated-tool-call terminal above.
Also: classify once before either log so an overflow no longer emits a
"so the loop can continue" WARNING followed by the contradicting "NOT
seeding" one; reset the stale-streak breaker once for both branches; drop
the "compression could not recover it" wording (this path never reached
compression); trim the test file to the three tests that bind behaviour
(stream -> terminal stub; 413 stays non-terminal; the terminal ends the
turn, closes the tool tail, carries compression_exhausted).
Address review P1s (andrexibiza) on #106266:
1. The overflow-terminal exit in recover_from_truncation now forwards the
#98722 typed compression_exhausted bit (partial_result/end_turn gained the
flag) so the gateway resets/moves future input to a clean session instead
of leaving the bloated durable session authoritative for the next turn.
2. _overflow_terminal is scoped to FailoverReason.context_overflow ONLY.
payload_too_large (413) has its own byte-scored recovery owner
(turn_overflow._recover_payload_too_large, #88960/#47339) that must not be
bypassed; a post-delta 413 keeps its normal continuation stub. Regression
covers both lanes.
Tests: unit asserts result compression_exhausted=True on the marker; a
real streamed partial hitting a 413 payload-too-large error keeps content and
is not terminal. 50 streaming/continuation/gateway regressions pass.
When a stream delivered text and then died on a context-overflow /
payload-too-large error, the partial content (often tens of KB) was seeded as
a length-continuation stub, growing the transcript monotonically. In a
session whose transcript cannot be compressed back under budget
(protect_last_n covers everything -> no_progress, or the summary would
itself be larger -> would_grow), every later request is larger than the one
that just failed — an unrecoverable loop where the user sees a 30+ minute
fake hang and the only remedy is killing the session (#106260).
classify_api_error already labels these errors context_overflow /
payload_too_large (should_compress=True). _partial_stream_stub now returns
an EMPTY stub marked _overflow_terminal for that class instead of seeding
the recovered text, and recover_from_truncation treats the marker as
terminal: the turn ends via the recovery contract with a clear message
(start /new) and the transcript is not polluted with the partial.
Normal partials (network stall, output-cap truncation, tool-call drops) are
unchanged — only the overflow error class changes behavior.
Tests: stub marker + empty content; a real streamed partial hitting a
'maximum context length' error returns the terminal stub; recover_from_
truncation ends the turn (no fragment/nudge appended) on the marker while a
normal stub still runs the continuation path. 67 streaming/continuation
regressions pass.
_maybe_inject_run_budget_wrapup() appends its wrap-up notice to the newest
role:"tool" message in place, with no _DB_PERSISTED_MARKER check. Its sibling,
_maybe_inject_iteration_budget_warning(), got exactly this guard added in the
same recent saga (turn_iteration_prep.py), with the comment "an older turn may
already be cached."
The reachability is structural, not an edge case: _maybe_inject_run_budget_wrapup
is only ever called from prepare_iteration(), at the START of the next iteration
-- strictly after tool_executor.py's _flush_session_db_after_tool_progress has
already flushed and marked the previous iteration's tool row persisted. So every
successful injection was mutating an already-persisted row: the wire request for
that turn carried the notice, but the durable transcript never did, diverging
replay from the live bytes and invalidating the provider's prompt-cache prefix
from that row onward.
Fix:
- Add the same _DB_PERSISTED_MARKER guard to _maybe_inject_run_budget_wrapup,
scoped to the specific tool row the reversed scan lands on (not just
messages[-1], since this function -- unlike its sibling -- scans backward for
the newest tool row rather than only checking the tail).
- Wire _maybe_inject_run_budget_wrapup into _flush_session_db_after_tool_progress
(pre-flush), mirroring exactly how _maybe_inject_iteration_budget_warning is
wired in both places. Without this, the guard alone would make the notice stop
firing in the common case, since prepare_iteration's call site almost always
hits an already-persisted row -- the pre-flush call site is what actually lets
it land in durable bytes.
Verified empirically: read the real call graph (tool_executor.py's three
_flush_session_db_after_tool_progress call sites cover every tool-completion
path) to confirm the guard's premise, then added an end-to-end test using a real
AIAgent + SessionDB that flushes and checks the persisted row for the notice
text. Mutation-verified: reverting the two production files drops exactly the 2
new/updated assertions (28 pass, 2 fail); reapplying restores green (30 passed).
Also ran the sibling iteration-budget-warning and /steer suites (71 passed) to
check for interaction regressions -- none.
The old tests encoded the skip (agent never constructed, [drift_skip] text,
alert-once bit). New invariants, proven red on origin/main: an unpinned job
with model_snapshot=A / provider_snapshot=P runs with AIAgent(model=A) and
resolve_runtime_provider(requested=P) after the global default moved to B/Q;
an explicit job pin still beats the snapshot; cron.model / cron.model_provider
still beat the snapshot; a legacy job without a snapshot follows the global
default. test_cron_drift_alert_once.py is deleted with the bit it tested; the
config-notice and impact-summary tests drop guard_enabled / model_drift_guard;
the desktop toast test asserts the informational copy.
Cron jobs are scheduled work bound to the source profile and its origin
channel. A clone that inherited cron/jobs.json fired every job twice: two
gateways with identical job ids running the same weekly jobs in parallel
(double spend, duplicate deliveries) until one gateway died.
Root cause: `cron` was not in _CLONE_ALL_HISTORY_EXCLUDE_ROOT, so the
copytree in _clone_all_into carried jobs.json along. Add it to the
per-profile history exclude set (applies to any source, CLI, dashboard
and TUI/desktop RPC all funnel through create_profile), recreate the
_PROFILE_DIRS skeleton after the copy so the clone still has an empty
cron/ (and sessions/), and say so in the CLI summary line and docs.
--clone (config-only) never copied cron; export/backup keep cron as
before (an archive is a portable snapshot, not a second live profile).
Drop the bare "tool.content" pattern (any 400 mentioning tool.content in a
non-list context would be sent through the image-strip path) and the profile
flag snapshot test; the behaviour tests (classifier verdict + proactive
downgrade) already pin the contract.
- Declare `supports_vision_tool_messages=False` and `supports_vision=True` on `opencode_go` provider profile in `plugins/model-providers/opencode-zen/__init__.py`
- Route HTTP 422 errors through `_IMAGE_TOOL_RULES` and add `tool.content.str`, `tool.content`, and `input should be a valid string` patterns to `_MULTIMODAL_TOOL_CONTENT_PATTERNS` in `agent/error_classifier.py`
- Add unit tests for OpenCode Go proactive tool result downgrade, HTTP 422 Console Go classification, and profile capability contract in `tests/run_agent/test_multimodal_tool_content_recovery.py` and `tests/plugins/model_providers/test_opencode_go_profile.py`
run_backup() previously wrote "hermes-backup-<timestamp>.zip" on every
invocation without deleting old ones. Hourly callers accumulated 157 zips
(14 GiB). Add _prune_run_backups() to keep the newest N (default 3,
configurable via backup.run_backup_keep or --keep CLI flag).
Two picker-freshness defects in cached_fetch_api_models():
1. The disk cache row was keyed on base_url only, with the credential
fingerprint stored inside the row. N custom_providers entries sharing
one proxy URL with different keys (#106184) took turns overwriting the
single slot; every sibling then failed the fingerprint check, got an
empty catalog, and disappeared from the Desktop pickers (which hide
zero-model rows). Key on url#fingerprint so each credential owns a row.
2. cache_only opens (Desktop model.options without refresh) served a
past-TTL row for up to 7 days without ever revalidating, so a model
loaded on a non-current local endpoint stayed invisible until the user
found "Refresh Models". Serve the stale row AND spawn the same
off-thread SWR refresh the blocking path uses; the caller still never
waits on the network.
Live repro (two rows, one URL, keys A/B; real loopback /v1/models):
GUI no-probe open before {'proxy-a': ['model-A1'], 'proxy-b': ['model-B1']}
after {'proxy-a': ['model-A1','model-A2'], 'proxy-b': ['model-B1']}
`hermes setup --reset` calls `save_config(copy.deepcopy(DEFAULT_CONFIG))`,
which writes `get_hermes_home()/config.yaml` — the exact file the backup
block a few lines below copies to `config.yaml.bak.<timestamp>`. Because the
copy ran after the reset, the backup captured the defaults that had just been
written, not the user's config. The one invocation where a backup matters most
produced a worthless one, and the original was unrecoverable.
The block's own comment already claimed it runs "before setup modifies it";
on the --reset path that was false. Move it above the --reset branch so it
captures the true pre-setup state on every path.
Also report the backup location on the --reset path. --reset is destructive
and can leave the wizard early (the non-interactive return exits before the
end-of-setup notice), so a user who just lost their config was never told
where the copy is. The end-of-setup notice is unchanged for the normal path
and is suppressed only when it has already been shown, so no run prints it
twice; the shared wording now lives in one helper.
Behaviour otherwise preserved: `copy2` (config.yaml holds secrets, so mode is
preserved), the try/except fallback to `_backup_path = None`, and the existing
notice for the full-setup path.
Follow-ups deliberately out of scope: pruning accumulated `.bak.*` files, and
printing the notice on the other early-return paths (--portal, section runs).
Refs #3522
Four writers each dropped their own uniquely-named copy of config.yaml next to
the real file and none of them ever deleted anything: hermes setup
(config.yaml.bak.YYYYMMDD_HHMMSS, one per run even with no change), the
corrupt-YAML snapshot (config.yaml.corrupt.<ts>.bak), hermes migrate xai
(config.yaml.bak-pre-migrate-xai-<ts>) and the Docker boot migration
(config.yaml.bak-<ts>, .env.bak-<ts>). A home dir accumulated a dozen variants
with no way to tell which mattered.
hermes_cli/config_backups.py::backup_config is now the single writer:
backups/config/config.yaml.<reason>.<YYYYMMDD-HHMMSS>, skipped when the newest
copy for that reason is byte-identical, rotated to the newest five per reason.
backups/ is already excluded from full backups so nothing nests. Legacy
siblings written by the old schemes are moved into the dir on first use;
hand-named copies (config.yaml.bak-my-note) are left alone.
Live: three `hermes setup --non-interactive` runs against an unchanged config
went from three .bak files in HERMES_HOME to one pre-setup copy under
backups/config/; repeated loads of broken YAML produce one corrupt copy
instead of one per process (deduped by content).
The ledger in verification_evidence.db exists only to feed the verify-on-stop
guard, but the recorder kept running on every foreground terminal command and
every file edit after #53552 turned the guard off by default. Users who never
opted in still accumulated a multi-MB database (7 MB / 4.6k rows on one install).
Every ledger entry point (record_terminal_result, record_verify_run,
mark_workspace_edited, verification_status) now checks verify_on_stop_enabled()
first and returns without opening or creating the database when the guard is
off. verification_status reports {"status": "disabled"} in that case; no client
consumes the verification.status RPC yet, so nothing downstream changes.
Existing ledger tests pin HERMES_VERIFY_ON_STOP=1 since they exercise the ledger
itself; the new test proves the off path never creates the file (red on base).
A Desktop-owned `hermes serve --isolated --ssh-session-token-file ...` child
is spawned with an explicit `--profile <name>` when the connection names a
remote profile, and with no flag for the remote root home. Without the flag,
`_apply_profile_override` read the remote host's sticky `active_profile`
file and re-homed the backend into whatever profile the user last selected
on that machine's CLI. Settings then read one config.yaml while the remote
gateway wrote another, so model picks and toggles "didn't stick".
Treat the SSH token flag as a fixed-identity marker, the same way
supervisor-launched gateway children are (#74872): a Desktop backend's
profile is chosen by the client, never by the host.
Live repro (before/after, temp HERMES_HOME with active_profile=foo):
serve --isolated --ssh-session-token-file ... hermes_home=<root>/profiles/foo -> <root>
same + --profile foo hermes_home=<root>/profiles/foo (unchanged)
serve (no token file, user CLI) hermes_home=<root>/profiles/foo (unchanged)
`slash.exec` runs on the RPC pool, outside any turn, so `/review` dispatched the
reviewer with no HERMES_UI_SESSION_ID and no steer authority bound. delegate_task
registered the child with `owner_session_id=None`, `subagent.list` (owner-scoped)
returned nothing for the parent session, and the Desktop status stack's 5s
snapshot poll reconciled the live `subagent.start` row away — the user saw only
"Review started. Results will return here." with no subagent card.
Bind the same session identity a turn binds (`_set_session_context(...,
ui_session_id=sid)` + `_current_runtime_session_record`) around `start_review`,
and clear it after. The reviewer now registers under the parent sid with the
request transport as authority, so `subagent.list`, steer, stop and the Desktop
roster all see it.
Live repro (tui_gateway stdio, real OpenRouter reviewer):
before: registry owner_session_id=None, owner_transport=NoneType;
subagent.list -> {"subagents": []}
after: owner_session_id=<parent sid>, owner_transport=StdioTransport;
subagent.list -> [{"goal": "Review recent work", "status": "running", ...}]
With account-identity matching gone, the providers.<id> block consolidation
only fired on shared token material. A historical fork (same copied pool-row
id, profile rotated, both pairs diverged) then healed the pool row into root
but left root's providers.openai-codex block on the spent pair; root's next
load_pool() re-seeds its device_code row FROM that block and undid the heal.
_HealPass now records that a profile pool row matched root by copied id or
shared tokens and passes that verdict to _heal_forked_provider_block, which
accepts it as lineage proof. No account-identity guessing is restored; an
independent same-account grant (no id/token match) is still left alone.
Follow-up to simpolism's #106177.
A quarantined/replaced/split-generation handle must never run a full-file rewrite or an FTS5
'optimize': both read damaged or foreign pages and commit the result back, turning contained,
diagnosable corruption into an amplified one. Same guard _execute_write applies to every write.
Salvaged from #102092 onto current main: the _try_wal_checkpoint half landed via #106315's
_quarantine_reason(), so only the two rewrite sites remain.
Follow-up to #106317. Typing the steer row (display_kind="steer") for the renderer and the
alternation-repair guard collided with the convention that any display_kind on a user row means
scaffolding: is_user_originated_turn / _is_actionable_user_turn / split_user_originated_turn
returned False for it (tail anchoring, auto-focus, dispatcher views, resume counts) while
_is_real_user_message returned True (anchor restoration) — the two predicate families disagreed
on the same row, and list_recent_user_messages (/undo, /rewind) skipped it in SQL. A steer
carries full user authority; the steer kind is now whitelisted in all four.
Also: the pre-API drain's requeue tail reuses _requeue_pending_steer instead of a copy; the TUI
history projection compares against STEER_DISPLAY_KIND; the steer() docstring describes the row.
Follow-up to #106315. vacuum() ran PRAGMA wal_checkpoint + VACUUM + wal_checkpoint(TRUNCATE) on
self._conn with no quarantine check; the only guard it inherited (optimize_fts raising
DeletedWalGenerationError) was swallowed by its own try/except and the rewrite proceeded on the
split-brain handle. Mutation on main: vacuum() returned 2 and rewrote pages after the write stop.
Follow-up to #106316. send_final_ledgered resolved the live adapter internally and
_send_final_text resolved it a second time for _schedule_ephemeral_delete; a reconnect between
the two sent the delete to a transport that never owned result.message_id (the ownership rule
_final_delivery_adapter documents). The bracket now returns (result, adapter).
The queued lane carried the ledger identity through MessageEvent.message_id while the PR added
ledger_message_id for exactly that; it now uses the typed field, and the ledger read is
getattr-tolerant of duck-typed events (a missing attribute was swallowed as "ledger skipped").
Follow-up to #106312. _preflight_timeout_result carries the prior history without this turn's
user row (#7100); with a repeated prompt ("continue") the verbatim scan resolved to the
historical copy and exported it as this turn's proven boundary — the exact relabeling the export
exists to prevent. Nothing is exported for that envelope now.
The trailing `agent._persist_user_message_idx = idx` ran after finalize_turn had already flushed
the transcript, so it never influenced a persist and the next turn reset it: dead state, removed.
`kwargs.get("target") is sched._run_callback` is always False (a bound method is a fresh object
per access), so the fake never returned Boom and the _dispatch failure branch went untested;
the test passed on the normal worker. Compare with == and assert the interception happened
(mutation: retiring the handle on start failure now fails the test).
The thread-count assertions sampled while per-fire workers were still live; quiesce every
handle with cancel(wait=) before sampling so the count is deterministic (AGENTS.md: timing tests
must not assume a quiet runner).
Follow-up to #106308.
_apply_stored_session_runtime was a line-for-line copy of the first half of
_restore_session_model (stored-model guard, session_gateway_runtime, bare-custom heal,
model/provider-changed check). Extract that pure decision into
cli_model_switch_mixin.stored_session_route and have both resume paths call it; the
one-shot keeps only the _ModelChoice mapping and the drop-ambient-key rule.
main.py stops re-normalising `resume` — _resolve_chat_session_args already did.
Tests trimmed from 20 to 13: near-duplicate unit tests of the private helpers go, the
end-to-end _run_agent contracts (stored runtime + reopen; explicit --model wins) and the
empty-session-keeps-id case stay.
Review fixes (#105957):
- A resumed one-shot ignored the session's stored model/provider runtime:
_resolve_model_and_provider()/resolve_runtime_provider() ran before
_load_resume_target(), which only loaded the session id + transcript, so an
ambient config (e.g. openrouter/ambient-model) served the resumed transcript
instead of the stored route (custom:stored/stored-model). The stored runtime
is now applied before runtime resolution, with the same contract as the
interactive _restore_session_model(): stored model/provider/base_url/api_mode
replace the ambient choice unless --model was passed explicitly, and a
changed provider drops the ambient api_key so resolution re-fetches
credentials for the restored endpoint.
- Passing the resumed id to AIAgent did not reopen the already-ended session
row: end_session() only writes rows whose ended_at is null and the
existing-row upsert never clears the end fields, so the resumed turn was
recorded under a session that stayed closed and its new lifecycle boundary
was lost. _load_resume_target() now reopens the row (best effort), same as
the interactive resume does before continuing.
Review finding on #105957: `_load_resume_target` returned None for a
resolved session with no stored messages, so `hermes -z "hello" -c <title>
--create-if-missing` recorded the turn under a freshly minted session id and
the just-created titled session stayed empty. Preserve `resolved` unconditionally — the interactive /resume path keeps the selected id for an
empty session too; only the history replay is empty. Regression tests pin the
durable id for both a plain empty session and an empty compression-chain head.
The -z exit path accepted --resume/-c in the parser but never forwarded
args.resume: every resumed one-shot turn silently started a fresh session,
so each wire request carried only [system, current user] and the model
lost all prior context (reported against Ollama/custom OpenAI-compatible
endpoints, but provider-independent).
Normalize session args (latest/title/--continue/--in + cwd restore) via
the chat path's _resolve_chat_session_args before the oneshot exit path
takes over, then load the resumed transcript in _run_agent through the
same contract the interactive CLI uses (compression-chain redirect,
safe-resume guard, session_meta filtering) and continue the existing
session id instead of creating a new one. An explicit --resume of an
unknown session now fails loudly instead of starting fresh.
"hermes" matches the profile-id regex, so canonicalising it unconditionally at the RPC
boundary misrouted a genuine <root>/profiles/hermes to the default profile. Alias only
when no such named profile exists; ".hermes" can never be a real id and stays aliased.
Also: profile_name_for_home collapses its duplicated pre/post-resolve block into one
loop over (path, resolved path) and drops the bare "parent named profiles" fallback
that bypassed named_profile_home's root check; _profile_home goes back to main's
single resolve() comparison; the symlink-loop assertion in the target-unavailable
test is no longer wrapped in a try/except that could silently skip it.
Both steer sites now build the row through one helper, prompt_builder.steer_user_row:
a role:user row with display_kind="steer" and no leading blank lines. The alternation
repair (_merge_consecutive_users) skips a steer-typed prev row, so a run that ended
right after a steered batch (Ctrl-C, interrupt) does not get the next real prompt
merged INTO the already-persisted steer row — which would have rewritten it in place
and re-broken live≠replay parity, the exact class this PR fixes.
TUI/desktop history projects the steer row as the user's own words instead of the
model-facing marker wrapper; 'steer' joins the display_kind union. The compression
anchor scan keeps its tool-row branch for transcripts persisted before this change and
its docstring says so.
Second site of the same bug class #104444 fixes in apply_pending_steer_to_tool_results:
_inject_steer_into_newest_tool_result (the drain that runs when a /steer lands during an
API call) mutated the newest role:tool row in place. That row was already flushed
append-only, so the replayed history diverged from the live request bytes at the
injection point and broke the prompt cache exactly like the post-batch path.
Deliver it the same way: a standalone user row inserted right after the newest tool
result (not yet persisted, so the next flush writes it to the transcript). Restash when
there is no tool row yet, unchanged. Stale comments claiming steer lands "in the newest
tool result" and agent/AGENTS.md's alternation rule now describe the real shape.
kshitijk4poor