Commit Graph

33032 Commits

Author SHA1 Message Date
nftpoetrist
9e0dc4319a fix(state): guard vacuum() and optimize_fts() against quarantined SessionDB handles
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.
2026-09-09 13:17:19 +05:30
kshitijk4poor
26f4a674e0 fix(agent): a /steer row is human input for every user-turn predicate
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.
2026-09-09 13:08:25 +05:30
kshitijk4poor
bb2c961e9a fix(state): VACUUM is gated by the same quarantine rule as the checkpoints
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.
2026-09-09 13:06:34 +05:30
kshitijk4poor
b530a482d8 fix(gateway): the ephemeral delete goes to the adapter that sent the final
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").
2026-09-09 13:06:23 +05:30
kshitijk4poor
91433c8466 fix(loop): the turn-boundary export skips preflight-timeout envelopes and stops re-anchoring the persist index
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.
2026-09-09 12:55:43 +05:30
kshitijk4poor
7f3e0bb59a test(agent): the worker-start-failure test intercepts the callback worker again
`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.
2026-09-09 12:42:02 +05:30
kshitijk4poor
8077206073 fix(cli): keep the no-stored-model early return ahead of the route read
CI: tests/cli/test_cli_resume_command.py builds bare HermesCLI objects without .model; the
refactor read self.model before the stored-model check the contributor's code made first.
2026-09-09 12:41:07 +05:30
kshitijk4poor
32273b8118 refactor(cli): one stored_session_route for interactive and one-shot resume
_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.
2026-09-09 12:41:07 +05:30
liuhao1024
8aa773af89 fix(cli): restore stored session runtime and reopen ended rows on oneshot resume
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.
2026-09-09 12:41:07 +05:30
liuhao1024
5ff6cb0edb fix(cli): keep the resolved session id when a resumed oneshot session is empty
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.
2026-09-09 12:41:07 +05:30
liuhao1024
86d606ca34 fix(cli): honor --resume in one-shot mode (#105892)
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.
2026-09-09 12:41:07 +05:30
kshitijk4poor
c402bcf1d8 fix(profiles): a stored <root>/profiles/<name> home names its profile even when the root carries no markers
CI: tests/test_tui_gateway_server.py::test_ensure_session_db_row_stamps_profile_name used a bare tmp
root; profile_name_for_home fell through to None and the row was stamped default. The stored home
is authoritative (its owner resolved it), so the profiles/<name> shape is sufficient.
2026-09-09 12:39:39 +05:30
kshitijk4poor
a39fedff34 fix(tui): a real named profile "hermes" is not swallowed by the legacy-basename alias
"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.
2026-09-09 12:39:39 +05:30
joaomarcos
81ff1a4022 test(tui): add coverage for custom default roots, real session db stamping, and sibling isolation 2026-09-09 12:39:39 +05:30
joaomarcos
e1b0ee6f03 fix(tui): fail closed on unavailable profile targets matching custom root basenames 2026-09-09 12:39:39 +05:30
joaomarcos
96128b0673 fix(tui): preserve names for custom profile homes 2026-09-09 12:39:39 +05:30
joaomarcos
298893df66 fix(tui): resolve default profile session names 2026-09-09 12:39:39 +05:30
kshitijk4poor
7dc796463d fix(agent): a persisted /steer row survives the next prompt's alternation repair; typed for history
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.
2026-09-09 12:21:28 +05:30
kshitijk4poor
4d0cec9a7d fix(agent): the pre-API-call /steer drain also stops smearing the persisted tool row
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.
2026-09-09 12:21:28 +05:30
kshitijk4poor
0d6e3637ef test: keep the steer suite on the canonical patch targets, not PLUGIN-COMPAT pointers
The cherry-picked commit carried an unrelated hunk repointing three patch()
targets back to run_agent.* — those are PLUGIN-COMPAT re-exports, off limits
in-tree (scripts/check_compat_pointers.py; removed 2026-09-14). Keep main's
model_tools.* / agent.process_bootstrap.OpenAI targets.
2026-09-09 12:21:28 +05:30
Albert.Zhou
d24810483d fix(agent): persist /steer as a standalone user message
`apply_pending_steer_to_tool_results` used to smear the steer text onto
the last `role:tool` message's content. That tool row had already been
flushed to the session store and carries `_DB_PERSISTED_MARKER`; the
append-only persistence never rewrites it, so the replayable transcript
diverged from the live request bytes at the injection point — resumed
sessions (surface switch / process restart / background-review close)
missed the provider prompt cache (75-85% hit) and the user's mid-run
instructions were never part of the durable history.

The steer is now emitted as a standalone `role:user` message (marker
text preserved):
- role alternation stays legal: assistant(tool_calls) -> tool -> user is
  the documented 'user jumped in mid-run' pattern that
  `repair_message_sequence` deliberately keeps;
- the appended dict carries no `_DB_PERSISTED_MARKER`, so the next
  `_flush_messages_to_session_db` writes it to the session store —
  transcript bytes and replayed history finally agree, and the steer
  becomes searchable/retrievable like any other user message;
- the no-tool-result fallback (interrupt) still requeues the steer, which
  the caller then delivers as a normal next-turn user message.

Tests: TestSteerInjection updated for the new shape plus a persistability
assertion (no marker => flushable); tool-batch-segmentation malformed
scenario updated. steer + segmentation suites: 67 passed, 1 skipped.
2026-09-09 12:21:28 +05:30
kshitijk4poor
146afcc5ba chore: map albert748's contributor email (#104444 salvage) 2026-09-09 12:21:28 +05:30
kshitijk4poor
f290946e4e fix(state): the deferred FTS rebuild retry is quarantined by the same rule as the checkpoints
retry_deferred_fts_recovery gated only on _db_corrupt ("mirrors _try_wal_checkpoint /
close") — after this PR it no longer mirrored them: on a replaced/lost-generation handle
the periodic housekeeping tick still ran FTS DDL/DML + commit, the same split-brain write
class as the #105670 checkpoint. One SessionDB._quarantine_reason() now decides for the
periodic checkpoint, close(), and the FTS retry, with the halt path's precedence
(replaced before generation loss) and the operator wording in one place.

Test: the periodic-checkpoint case folds into the close test (same setup), which now
also proves the FTS retry returns False without touching the file; the mutation with
main's schema sibling swapped in returns True (a rebuild ran).
2026-09-09 12:21:19 +05:30
kshitijk4poor
ae94e349ac test(state): mark the checkpoint-guard tests linux_only instead of a bare skipif
AGENTS.md: a bare skipif(sys.platform != linux) is never listed by
scripts/ci/list_os_marked_tests.py, so the tests would run nowhere on the
OS lanes. The marker is the contract.
2026-09-09 12:21:19 +05:30
kshitijk4poor
7a4985a814 refactor(state): drop the ruff-format reflow from the checkpoint guard, keep the ~20 semantic lines
The cherry-picked commit re-wrapped hermes_state.py wholesale (+527/-131 for a
fix of about twenty lines). Restore main's layout and re-apply only the fix:
disable the close-time checkpoint on both generation-loss halts, gate the
periodic checkpoint on the sticky generation flags, and name the quarantine
reason at close.
2026-09-09 12:21:19 +05:30
Konstantin Khlopkov
68d9365aa7 fix(state): guard close-time checkpoint for replaced/deleted-generation handles (#105670)
- close() and _try_wal_checkpoint() now skip when _db_replaced or _db_wal_generation_lost
  (previously only _db_corrupt was checked) — prevents checkpointing stale-generation frames
  into the main DB, which is the shutdown-time damage reported in #105670
- _halt_if_db_generation_changed() calls _disable_close_time_checkpoint() alongside the flag
  set (3.12+: disables SQLite internal last-connection checkpoint too)
- Regression tests: halted handle must not run explicit PRAGMA checkpoint on close(),
  halt must call setconfig(NO_CKPT_ON_CLOSE), periodic _try_wal_checkpoint() must skip
2026-09-09 12:21:19 +05:30
kshitijk4poor
b567261731 refactor(gateway): one send_final_ledgered bracket for the normal and queued final lanes
The queued lane re-implemented _send_final_text's record / send-with-retry / finalize
sequence by duck-typing four private adapter members from gateway/. Lift the bracket
into a public BasePlatformAdapter.send_final_ledgered(event, session_key, text,
metadata, *, reply_to, is_ephemeral_response); _send_final_text keeps only the
ephemeral-delete tail on top, the queued lane calls it with the inbound-id ledger event.
ledger_message_id is a real dataclass field now, read directly.

Tests trimmed from 18 to 8 invariants (bracket recorded+delivered; flood refusal stays a
failed ledger row; forum-topic identity; plain adapter keeps plain send; normal-lane
parity; chained/terminal/deeper-chain inbound ids). Still 6 red with main's
run_notifications.py swapped in.
2026-09-09 12:20:51 +05:30
Alexander Russell
6532d9d8da fix(gateway): ledger a queued chain's terminal reply under its own inbound id
The outer final send is bracketed by the adapter against the event that OPENED
the chain, so a terminal reply was recorded under the first message's id. When
two turns of one chain answered with the same text, the terminal reply computed
the earlier row's obligation id, replaced its outstanding row and marked it
delivered, so a first reply the platform had refused was never redelivered.

MessageEvent gains a documented ledger_message_id that the obligation hash
prefers, the queued follow-up returns the terminal turn's inbound id (innermost
wins on nested chains), and the handler sets it before the adapter brackets the
send. Reply routing is untouched: the anchor still comes from the event.

Three of the four new tests fail without this change; the whole tests/gateway
suite shows the same failure set before and after.
2026-09-09 12:20:51 +05:30
Alexander Russell
b2ed699427 test(gateway): let the queued-native-image fake accept the persist kwargs
Now that a queued follow-up carries its raw inbound id, the gateway passes
persist_user_platform_id to run_conversation for that turn (run_turn_runner.py
only adds it when inbound_message_id is set). The real AIAgent accepts it; the
test's fake did not. Accept **kwargs, matching the real signature.
2026-09-09 12:20:51 +05:30
Alexander Russell
ec4eecd864 fix(gateway): carry the raw inbound id through a chained queued turn
Round-2 review. _run_agent_deliver_first_response passed the turn's inbound id to
the queued lane, but the recursive _run_agent for a chained follow-up did not, so
the chained turn ran with inbound_message_id=None. In a Telegram forum topic (no
reply anchor) two chained follow-ups answering with the same text would then key
their queued-final obligations on None and collide. The recursive call now carries
pending_event's raw message id, with a test on the chained path.
2026-09-09 12:20:51 +05:30
Alexander Russell
af09c4033e fix(gateway): key the queued-lane obligation on the raw inbound id
The queued lane's ledger bracket used the reply anchor as the obligation's
message reference. The anchor is None wherever replies are not used (Telegram
forum topics, Slack reaction handoffs), so two turns in one topic answering
with the same text shared an obligation id and the second record overwrote
the first's outstanding row; the id also differed from the normal lane's.

The lane now runs the adapter's record / send-with-retry / finalize sequence
itself, keyed on turn_ctx.inbound_message_id like the normal lane, while the
anchor stays the reply target. Tests cover the forum-topic identity, the row
being `attempting` while the send is in flight, and the call site passing
both ids.
2026-09-09 12:20:51 +05:30
Alexander Russell
c237c492cd fix(gateway): ledger-bracket the queued-lane final send
When a follow-up is queued behind a turn, the first response is delivered by
the queued lane before the follow-up runs. That lane called adapter.send bare
and discarded the result: no delivery-ledger obligation was recorded, so a
final refused there (flood control, a transport that had just died) was lost
for good. Neither the boot sweep nor the runtime redelivery could see it and
the follow-up ran as if the answer had landed.

Route the queued lane's text send through the adapter's _send_final_text, the
same ledger-bracketed method the normal lane uses: the obligation is recorded
before the send under the id the normal lane would compute for the same turn,
the result finalizes it, and the reply is marked notify-worthy like every
other final. The reconcile-by-edit path is unchanged; adapters without the
base contract and sends without a session key keep the plain send.
2026-09-09 12:20:51 +05:30
kshitijk4poor
4ddbcbd35e fix(mcp): a profile only adopts a shared connection whose credentials match its own config
_same_server_route compared config_fingerprint alone, which by design excludes
env/headers/auth (so the schema cache survives a token rotation). Profile B with the
same URL but different headers/env therefore adopted profile A's live connection and
called tools as A. _connection_identity = route fingerprint + env + headers + auth mode,
used by both the adopt and the stale-removal checks.

Also collapses the three writers of _server_tool_scopes to two: the adoption loop
re-implemented in mcp_tool_discovery._select_new_servers is dropped —
register_connected_into_current_scope (which runs first in register_mcp_servers) is
the single adopter, and _register_candidates records scope for freshly registered tools.
2026-09-09 12:20:21 +05:30
joaomarcos
d02edc2cbc fix(gateway): register shared MCP tools per profile 2026-09-09 12:20:21 +05:30
joaomarcos
7107185f58 fix(gateway): preserve shared MCP visibility across profile reloads 2026-09-09 12:20:21 +05:30
kshitijk4poor
1f6718b0ff test(agent): prove the re-anchor through prepare_iteration; reuse the compaction _reanchor
The salvaged regression test exercised only repair_message_sequence and
reanchor_current_turn_user_idx — pre-existing helpers — so reverting the fix left it
green. It now drives prepare_iteration on a real AIAgent with adjacent user rows and
asserts the returned index addresses this turn's row and mirrors into
_persist_user_message_idx (red without the re-anchor: IndexError).

Both re-anchor sites (repair and compression restart) call
turn_context_compaction._reanchor instead of inlining "reanchor + mirror", so they
cannot drift. The export tests fold into one parametrized invariant plus the
run_conversation envelope test; the WHAT-restating comment shrinks to the WHY.
2026-09-09 12:20:04 +05:30
Felipe Portavales
37f42713ef feat(loop): export {turn_id, current_turn_user_idx} on every result envelope
Hosts that settle their own transcript by index (hermes-webui) cannot prove which
row of result["messages"] is the current user turn once this loop rewrote history
(alternation repair, compaction, post-turn micro-compaction): the instance-side
_persist_user_message_idx predates those rewrites, and a text match relabels an
identical historical prompt and claims its old answer. Only the producer can
assert the coordinate against the exact list it returns.

run_conversation now wraps the turn (_run_conversation_turn) and stamps the pair
through export_current_turn_boundary on every envelope that leaves the loop
(success, partial/error, interrupt, retry-exhausted, tool-limit, preflight
timeout, codex runtime), computed on the final messages after finalize_turn and
micro-compaction. The pair is exported only when the addressed row is this turn's
user message verbatim (reanchor's last-match rule); a rewritten row exports
nothing so hosts fail closed. The final index is mirrored into
_persist_user_message_idx for the persist override.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013gp366ijf39n4UUtJhZuMh
2026-09-09 12:20:04 +05:30
Felipe Portavales
fe21a4d2f0 fix(loop): re-anchor current_turn_user_idx after the alternation repair merges rows
prepare_iteration() runs repair_message_sequence_with_cursor() before each API
call; the repair merges adjacent user rows in place (after a compaction, the
role=user summary sits next to the protected first user message). The loop's
current_turn_user_idx was recorded at turn start, so after a merge it points
past the current user row: the per-turn context injection (prefetch/plugin
context) silently misses it, and hosts that settle the transcript by this index
(hermes-webui) write the current user turn to the FRONT of the context —
rewriting the prompt's leading messages every turn (0% prefix-cache hits at
200K+ tokens, ~100 s re-prefill per turn) and duplicating the user's question.

The in-loop compression restart path already re-anchors; do the same after a
repair that changed the list: reanchor_current_turn_user_idx (last user row
carrying this turn's text), return the index through the IterationPrep verdict
so the loop state picks it up, and mirror it into agent._persist_user_message_idx,
which hosts read when the result carries no index. The new phase parameters
default to None so direct callers keep their signature.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013gp366ijf39n4UUtJhZuMh
2026-09-09 12:20:04 +05:30
kshitijk4poor
83d68203c5 chore: map portavales's contributor email (#105694 salvage) 2026-09-09 12:20:04 +05:30
kshitijk4poor
19c9734021 refactor(state): drop the table-exists probe made dead by the early return above it 2026-09-09 12:19:58 +05:30
fangliquan
5ca028d16a fix(sessions): restore trigram after deferred bootstrap 2026-09-09 12:19:58 +05:30
fangliquan
8b5123e238 fix(sessions): serialize fresh FTS bootstrap 2026-09-09 12:19:58 +05:30
kshitijk4poor
e333113871 fix(review): keep /refine under the background_review origin; attendedness is its own flag
The salvaged commit forked an explicit /refine under a new "refine_review" origin so
the memory delete gate would not treat it as unattended. But is_background_review()
is the key for every other review guard — skill_manager_guards (curator-owned-only,
read-before-write), skill_manager_tool (archive instead of rmtree), skill_ledger
actor, write_approval staging, the [auto] tag — so a /refine fork silently escaped
all of them.

Carry attendedness separately: the fork keeps origin "background_review" and sets
_review_attended; turn_context binds it beside the origin ContextVar; the memory
gate keys on the new is_unattended_review(). Also run the gate AFTER
_validate_single_op / the operations list check, as memory_tool's own docstring
requires, so an invalid replace is rejected now rather than staged and failed at
approve time.
2026-09-09 12:19:13 +05:30
liuhao1024
c0714575c3 fix(review): distinguish explicit /refine from unattended reviews and surface staged consolidations
Review follow-up on #105944 (#105921):

- explicit /refine forks now run under the refine_review write origin
  (explicit flows from the CLI/gateway handlers through
  _spawn_background_review_now and spawn_background_review_thread down
  to build_cache_parity_fork), so a user-requested review keeps the
  full memory operation set; only automatic reviews stay behind the
  unattended delete gate.
- the unattended delete gate now stages the denied replace/remove (or
  whole batch) into the pending store instead of dropping it: the
  fork's own review summary is never published, so a plain denial lost
  the consolidation request with no surfacing path. The staged proposal
  carries a proposal_staged marker that summarize surfaces as an action
  line, and a staging failure still fails closed to a plain denial.
- regression tests: explicit-path origin pass-through, refine_review
  keeping replace working, near-limit denial end to end (add rejected
  by budget -> replace staged -> proposal surfaces, store unchanged).
2026-09-09 12:19:13 +05:30
liuhao1024
1571f502a9 fix(agent): scope background review memory access to its trigger (#105921)
The review fork's tool whitelist granted the whole memory toolset
whenever the profile had memory enabled, regardless of which nudge
fired, so a skill-nudge fork held remove/replace on MEMORY.md it was
never asked to use; combined with the memory tool's near-limit
'consolidate now' hint, an unattended fork deleted standing rules with
no user in the loop.

- Pass review_memory from spawn_background_review_thread through
  _run_review_in_thread/_run_review_fork into _review_tool_whitelist;
  a skill-only review no longer gets the memory tool at all.
- Fail-closed operation gate in memory_tool: a background-review fork
  may add, never replace/remove (single or in a batch) — consolidation
  decisions reach a human via the review summary instead.
- Keep the deny/prompt wording in sync with the whitelist so a
  memory-less review doesn't advertise memory.
2026-09-09 12:19:13 +05:30
kshitijk4poor
2369606f98 refactor(agent): one _requeue for the three heappush sites; timing test asserts ordering, not a 0.3 s bound
Fold the identical heappush(...) into PeriodicScheduler._requeue; notify() instead of
notify_all() now that the scheduler thread is the only condition waiter; drop the
PR-history paragraph from the module docstring (the commit carries it).

Tests: the blocked-sibling test asserted `sibling_ran.wait(0.30)` — a wall-clock bound
under the repo's ≥2 s flake floor; it now asserts the sibling fired while the blocker
still held its worker. The worker-start-failure fake keys on this scheduler's own
_run_callback rather than the global thread-name prefix so a leaked handle on _DEFAULT
cannot consume the single injected failure. The base-green no-overlap test is dropped
(it does not prove the fix).
2026-09-09 12:17:57 +05:30
finn763
1562b87d5d fix(agent): isolate periodic scheduler callbacks from blocking siblings (#102574) 2026-09-09 12:17:57 +05:30
kshitijk4poor
e669800e2a test(agent): fold the five memory-ceiling cases into one parametrized invariant
Same coverage (5 red on main, 1 guard green), one contract test instead of five.
2026-09-09 12:17:29 +05:30
Tim Kaufmann
b7e4712029 fix(agent): classify local-inference memory-ceiling rejections as overloaded
oMLX/MLX prefill memory-guard rejections name an allocation peak in BYTES but
close with "Reduce context length", so _CONTEXT_OVERFLOW_PATTERNS claims them
and the turn enters the compress-and-shrink loop. Compression cannot lower a
prefill peak — the prompt is usually far below the window — so it burns the
compression budget, re-hits the wedged server on every attempt and ends in
"Cannot compress further" plus a destructive session reset.

Classify them as `overloaded` instead: retry with backoff, no compression, no
session reset (mirrors 503/529 recovery).

The guard runs before the overflow check AND before the usage-limit
disambiguation. The second ordering matters more than it looks: "memory limit
exceeded" contains "limit exceeded", so a status-less rejection — a proxy that
flattened the body — is currently classified as `billing` and rotates a
healthy credential.

Sites covered:
  - _OVERFLOW_AS_5XX_RULES, which _400_TAIL_RULES extends → 400, 500, 502,
    503, 529
  - _MESSAGE_HEAD_RULES for the status-less path (ahead of usage-limit)
  - _ERROR_CODE_VERDICTS for the structured oMLX codes
  - _classify_400, because _by_status runs before _by_error_code, so a body
    whose wording a proxy stripped would otherwise fall through to
    format_error

Every pattern names memory/allocation in bytes, never a token or window count,
so the list stays disjoint from _CONTEXT_OVERFLOW_PATTERNS. Both oMLX wordings
are kept: 0.5.6 says "Prefill would require ~13.87 GB peak", 0.5.7 reworded it
to "predicted peak would require/exceed" and both are in the field. A test
pins that a genuine window overflow still compresses.

Refs #52261. Supersedes #52289, which predates the classifier rewrite and can
no longer be merged.
2026-09-09 12:17:29 +05:30
kshitijk4poor
2b0b03ec58 chore: map tkaufmann's contributor email (#105463 salvage) 2026-09-09 12:17:29 +05:30