fix(cron): report both streams on bot-chat delivery failure by Halldrix · Pull Request #104068 · NousResearch/hermes-agent

Halldrix · 2026-09-06T05:08:32Z

What does this PR do?

A failed deliver: bot-chat:<profile> turn recorded only (stderr or stdout)[-500:] as last_delivery_error. In -Q mode the resume banner and session_id: go to stderr while the response rides stdout, so half the signal was always discarded — and when stderr was empty and stdout held only the resume banner (the #104056 shape), the recorded "error" carried zero diagnostics.

This PR keeps both tails, labeled and each capped at 500 chars (stderr: … | stdout: …), or no suffix when both streams are empty. No behavior change on success, timeout, or the delivery lane itself.

Related Issue

Refs #104056 (partial — covers problem 2 only, stays open)

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Security fix
Documentation update
Tests (adding or improving test coverage)
Refactor (no behavior change)
New skill (bundled or hub)

Changes Made

cron/scheduler_delivery.py — new _format_failure_streams() helper (None-safe, per-stream 500-char cap); the non-zero-exit branch of _deliver_to_bot_chat() uses it instead of (stderr or stdout)[-500:].
tests/cron/test_cron_bot_chat_delivery.py — _completed() helper gains a stdout param (default "", existing tests unaffected); 3 new tests: both-streams-labeled, stdout-only banner case, empty-streams no-suffix pin.

How to Test

scripts/run_tests.sh tests/cron/test_cron_bot_chat_delivery.py -q
# 20 passed

The two regression tests fail on pre-fix code; the empty-streams test pins the no-suffix contract.

Exclusions

Out of scope (issue stays open): problem 1 (stale-resume fallback in --create-if-missing — it would fight the #86794 intent of creating only on initial miss) and problem 3 (queue/retry on live-owner refusal — covered by open #100319 and #102659 against #99956). Hence Refs, not Fixes.

Checklist

Code

I've read the Contributing Guide
My commit messages follow Conventional Commits (fix(cron): …)
I've searched for duplicates (no open PR covers the error-capture half; fix(cron): queue bot-chat delivery for CLI-only owners #100319/fix(cron): use non-blocking delivery for bot-chat to avoid session lock conflicts #102659 cover only retry)
I've run the tests (scripts/run_tests.sh, 20/20) and added regression tests
No sibling call sites keep the old shape (only remaining match is the new docstring)

Documentation

N/A (no user-facing behavior change beyond a more informative error string)

Platform

Debian GNU/Linux 13 (trixie), x86_64 — Hermes Agent v0.21.0 (v2026.8.31), upstream 245e480

_last_delivery_error_ kept only (stderr or stdout)[-500:], so a failed -Q turn with empty stderr and a banner-only stdout recorded zero diagnostics. Keep both tails, labeled and capped. Refs NousResearch#104056

Enough1122 · 2026-09-06T10:07:09Z

AI code review — automated review for reference; please use your judgment.

Summary

Failed bot-chat deliveries now report labeled, capped tails of both stderr and stdout (cron/scheduler_delivery.py:_format_failure_streams, 500 chars each) instead of stderr or stdout, which discarded half the signal when -Q splits the resume banner/session_id: (stderr) from the response (stdout). Three tests pin both-streams, stdout-only, and empty-streams shapes.

Findings

Non-blocking, note: cron/scheduler_delivery.py:24-26 — negative slicing (err[-500:]) on an already-stripped string is safe for short/empty inputs and the if err / if out guards keep the empty case suffix-free (asserted by test_deliver_failure_empty_streams_have_no_detail_suffix). No edge-case risk.
Diagnostic-only change: the delivery lane, timeout, and success paths are untouched.

Verdict

Looks good. Tiny, precisely tested observability fix. Safe to merge.

Halldrix · 2026-09-08T23:10:24Z

Thanks @Enough1122 — cron/scheduler_delivery.py:_format_failure_streams verified:

err[-500:]/out[-500:] safe on stripped short/empty + if err/if out guards keep empty suffix-free (pinned by test_deliver_failure_empty_streams_have_no_detail_suffix) — no edge risk.
Diagnostic-only, delivery lane/timeout/success untouched.

No changes — tiny observability fix as intended, thanks!

Enough1122 · 2026-09-09T01:59:05Z

Ack — verified at 3c858e5: empty-stream guards test-pinned, diagnostic-only with delivery lane untouched. Closed, no changes needed on my side either.

…ually writes Neither DM delivery retry (NousResearch#93091 item 5) ever fired. Both gates classified the failure with `(proc.stderr or proc.stdout)`, but a failed `hermes ... -Q` turn splits across BOTH streams: the provider prose lands on stdout ("API call failed after 3 retries: HTTP 500: ..."), while stderr carries session bookkeeping ("Session ... Starting fresh.", "session_id: ..."). stderr is non-empty on every run, so it always won, the classifier only ever saw bookkeeping, every transient failure classified as 'unknown', and the re-run was skipped. Every rate limit, 500 and context overflow failed the DM outright instead of retrying once. Two call sites, same mistake: - `tools/bot_mode_dm.py` `_run_local_turn` — same-install `message_agent` deliveries. - `tui_gateway/methods_bot_relay.py` `bot_relay.deliver` — cross-connection deliveries. Its typed `reason` was also always 'unknown', so the sending agent could not tell a rate limit from an auth failure. The existing tests for both hand the error text to stderr and leave stdout empty — a layout no real run produces — so they passed while the feature was inert. `bot_failure_reasons.transport_failure_reason` now owns the decision for both: it classifies each stream and takes the first typed answer, so provider prose on stdout is seen while the deliberate stderr contract (`active_sessions.format_refusal_stderr`, SESSION_NOT_OWNED) still outranks nothing. Both retried attempts also set `HERMES_RESUME_UNANSWERED_TURN`, because the failed attempt already persisted the user row: without it, activating these retries makes the recipient receive the DM twice. Measured against a real CLI turn at a provider that fails the first attempt — one copy delivered, reply returned. The same `(stderr or stdout)` selection in `cron/scheduler_delivery.py` is NousResearch#104068's subject (reporting, not retrying); this PR leaves it alone.

`-Q` writes the resume banner and `session_id:` to one stream while the real reason may sit in the other; `(stderr or stdout)[-500:]` (and the `[-200:]` cut of the recorded detail) discarded half the signal, so a banner-only failure left `last_delivery_error` with no diagnostic at all (#104056). Keep both tails, labeled `stderr:` / `stdout:`, in the log line and the recorded error. Resolved onto the current wording of the delivery failure message; the third test from the PR (empty-streams suffix) was dropped as a change-detector. Part of #104056 Salvages #104068

teknium1 · 2026-09-17T07:31:12Z

Thanks @Halldrix — this landed on main through #113435 (fix(cron): a failed Bot Chat delivery records the exit code and both streams (stdout no lo), merged as 3719e04. Your commits/analysis were carried in with credit (see that PR's body and Co-authored-by trailers); the salvage rebased onto current main, widened to sibling surfaces where the review found gaps, and was live-verified in the real Electron app. Closing this one as superseded — thank you for the fix.

fix(cron): report both streams on bot-chat delivery failure

3c858e5

_last_delivery_error_ kept only (stderr or stdout)[-500:], so a failed -Q turn with empty stderr and a banner-only stdout recorded zero diagnostics. Keep both tails, labeled and capped. Refs NousResearch#104056

Halldrix mentioned this pull request Sep 6, 2026

fix(cron): bot-chat delivery resumes stale canonical session, then dies with no actionable error #104056

Closed

alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels Sep 6, 2026

jonpol01 mentioned this pull request Sep 8, 2026

fix(bot-mode): neither DM delivery retry ever fired — classify from the stream the CLI actually writes #105529

Closed

12 of 13 tasks

teknium1 mentioned this pull request Sep 16, 2026

fix(cron): a failed Bot Chat delivery records the exit code and both streams (stdout no longer dropped when stderr is present) (#104056, salvage #104068) #113435

Merged

teknium1 closed this Sep 17, 2026

Navigation Menu

Repository navigation

fix(cron): report both streams on bot-chat delivery failure - #104068

fix(cron): report both streams on bot-chat delivery failure#104068
Halldrix wants to merge 1 commit into
NousResearch:mainfrom
Halldrix:fix/cron-bot-chat-stale-session-104056

Halldrix commented Sep 6, 2026

Uh oh!

Enough1122 commented Sep 6, 2026

Uh oh!

Halldrix commented Sep 8, 2026

Uh oh!

Enough1122 commented Sep 9, 2026

Uh oh!

teknium1 commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Halldrix commented Sep 6, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Exclusions

Checklist

Code

Documentation

Platform

Uh oh!

Enough1122 commented Sep 6, 2026

Summary

Findings

Verdict

Uh oh!

Halldrix commented Sep 8, 2026

Uh oh!

Enough1122 commented Sep 9, 2026

Uh oh!

teknium1 commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants