Commit Graph

10 Commits

Author SHA1 Message Date
kshitij 98e96e1a60 refactor(agent): drop the run_agent classify_persistence_error delegating wrapper
Post-merge simplify finding on #81613: the wrapper's docstring claimed it
existed for 'existing callers', but every caller was introduced by the same
PR - there was never a pre-existing import path to preserve. All callers
(conversation_loop, tool_executor, run_agent's own flush handler, tests)
now import the canonical hermes_state.classify_persistence_error directly,
matching how is_disk_full_error is consumed. No behavior change; imports
stay lazy inside the exception handlers.
2026-08-08 14:25:12 +05:30
kshitij 1005a057f0 review follow-ups: canonical classifier in hermes_state, compression-busy=locked, hedged gateway wording, drop dead constant
- Move classify_persistence_error into hermes_state beside is_disk_full_error
  and delegate the disk bucket to it (fixes 'ENOSPC writing state.db' and
  'not enough space' classifying as unknown). run_agent keeps a thin lazy
  delegating wrapper so the documented import path and fast import survive.
- Classify CompressionSessionBusyError (and its RPC-wrapped message forms)
  as 'locked': the motivating #81227 failure mode stringifies to 'is being
  compressed by another writer', which the substring heuristic missed.
- Export PERSISTENCE_ERROR_CAUSES and iterate it in the cron explainer
  suppression instead of a hardcoded tuple, so a future cause bucket cannot
  silently desynchronize cron delivery.
- Hedge the gateway locked/unknown recovery wording ('should already be
  saved' instead of 'was recorded') to match the explainer - the early
  turn-start persist may also have failed.
- Drop STATE_DB_WAL_WARN_BYTES (speculative dead constant with no consumer;
  the pre-existing 50 MB doctor WAL check covers the warning).
- Tests: compression-busy classification, is_disk_full_error delegation,
  causes-tuple coverage; mutation-checked red-green.
2026-08-08 14:18:26 +05:30
Victor Kyriazakos 2a9f5b3476 fix(agent): classify session-persistence failures so lock contention is not misdiagnosed as disk-full
An enterprise deployment hit sustained SQLite write-lock contention on a
shared multi-gigabyte state.db (gateway + CLI processes writing
concurrently). Turns correctly failed closed with
session_persistence_failed, but the only user-facing wording claimed the
disk was full and the gateway rendered a generic failure.

The fast-fail semantics are deliberate and unchanged. This adds a pure
classifier (locked / disk / unknown) applied where the SQLite error is
still visible, threads the cause through the turn-completion explainer,
and stamps a machine-readable failure_reason
(session_persistence_failed:<cause>) plus a guaranteed non-empty error on
the result for downstream surfaces. The cron scheduler's explainer-text
suppression now matches every cause variant so refined wording cannot
leak into scheduled-job deliveries.
2026-08-08 14:18:26 +05:30
Teknium 7c5a98d888
Merge remote-tracking branch 'origin/main' into tests/prune-low-value
# Conflicts:
#	tests/run_agent/test_conversation_fallback_state.py
2026-07-29 15:24:14 -07:00
Soju06 ce9f6712ff refactor(agent): remove the inter-tool delay
The 1.0s sleep between sequential tool calls has been present since the
initial commit with no documented rationale. It sleeps between local
tool executions — the next LLM request goes out only after the whole
batch — so it rate-limits nothing, and the parallel read-only path
already runs with no delay. Every multi-tool turn pays (N-1) seconds
of dead time. Remove the sleep, the internal tool_delay plumbing, and
dead test assignments. AIAgent.__init__ keeps tool_delay as a
deprecated no-op keyword for one release so existing programmatic
callers construct cleanly; passing it emits a DeprecationWarning.
2026-07-29 15:21:26 -07:00
Teknium 6b81590c55
test: prune low-value tests suite-wide (wave 1) — 46,820 → 28,106 test functions
Systematic prune per AGENTS.md test policy, one pass over every major
test tree (gateway, hermes_cli, tools, agent, run_agent, plugins, cli,
cron, tui_gateway, honcho/openviking, root-level):

- DELETE: source-reading tests (read_text/getsource on prod files),
  change-detector tests (exact catalog counts, model-name snapshots,
  config version literals), mock-echo tests (assert a mock returns what
  it was told), assertion-free/trivial tests, near-duplicate
  parametrizations (boundaries + one representative kept), async/sync
  twin duplicates, cosmetic within-file variations.
- KEEP (mandatory): security/redaction/approval guards, message-role
  alternation invariants, prompt-caching/deterministic-call-id
  invariants, issue-number regression tests (deduped), E2E tests.
- 6 test files deleted outright (script-style/no-assert or fully
  redundant); conftest.py, fakes/, fixtures/ untouched.
- tests/acp/conftest.py added: autouse fixture stubs the live
  models.dev/GitHub/Copilot/Anthropic inventory fetches that ACP server
  tests performed on every session create — test_server.py 147s → 3.4s,
  and the tests are now genuinely hermetic.
- Sleep-based slowness shrunk where safe (codex_ttfb_watchdog,
  compression_concurrent_fork, etc.); no wall-clock assertion tightened.

Verification: full hermetic suite via scripts/run_tests.sh —
2439 files, 31,130 tests passed, 0 failed, 0 flaky retries, 315s wall
(baseline: 583s wall, 13,564s subprocess CPU).
2026-07-29 13:10:23 -07:00
kshitijk4poor 8e934e84ac fix: follow-ups for salvaged PR #72425
- codex app-server sibling path: surface a WARNING (was silent debug) when
  the projected-message flush fails — same bug class as the main fix, but
  codex output has already streamed so fail-closed and agent_persisted=False
  are both wrong here (#860/#42039 duplicate-write hazard); loud durability
  gap logging instead.
- map session_persistence_failed in _format_turn_completion_explanation so
  the user sees an actionable reason instead of 'The request failed:
  unknown error' + explainer test.
- contributors/emails mapping for elco@thedaoist.gg (attribution CI).
2026-07-28 00:14:19 +05:30
infinitycrew39 1fa46570fb test(agent,gateway): cover partial-stream recovery and restart helper salvage 2026-06-27 22:03:14 -07:00
teknium1 fb0ab27649 fix(agent): register explainer config key + shorten footer prefix
Follow-up to the salvaged #34452 turn-completion explainer:
- Register display.turn_completion_explainer: True in DEFAULT_CONFIG so the
  setting is discoverable, matching the file_mutation_verifier precedent.
- Shorten the repeated footer prefix from 'Turn ended without a usable
  reply: ' to 'No reply: ' so the 10 reason variants don't all open with
  the same 8-word boilerplate.
- Update the 7 assertions that referenced the old prefix.
2026-05-29 19:23:05 -07:00
Bartok9 59b0ea98c8 fix(agent): explain abnormal turn endings instead of blank/partial reply
When a turn ends abnormally after substantive tool calls (empty content
after retries, a partial/truncated stream, exhausted retries, or an
iteration/budget limit), the CLI/TUI response area was left blank or
showed only a fragment (e.g. "The") with no consolidated reason. The
internal turn_exit_reason values (empty_response_exhausted,
partial_stream_recovery, etc.) were never surfaced to the user.

Add a turn-completion explainer that mirrors the existing file-mutation
verifier footer: at turn end, map an abnormal turn_exit_reason to a
short, actionable message and either replace the bare "(empty)"
sentinel or append the reason after a partial fragment. Normal
text_response exits (e.g. a terse "Done.") stay quiet.

Gated by display.turn_completion_explainer (default on) with
HERMES_TURN_COMPLETION_EXPLAINER env override, matching the
file-mutation verifier seam.

Closes #34452
2026-05-29 19:23:05 -07:00