Repository navigation
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
Conversation
`-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
૮ >ﻌ< ა ci reviewran on 20d0858 — fix(cron): banner-only bot-chat failure names the exit code; debug infoCI timingsCI timings · View report · View jobWall time 6m18s vs 6m27s (-2.3%). 4 job(s) slower, 6 faster, 2 unchanged.
|
…d stdout tail capped and scrubbed Review follow-up for the both-streams change. - `_format_failure_streams` now always leads with `exit code N`, drops the `Resumed session` / `session_id:` banner lines from the stdout tail, and says `stdout was only the resume banner` when nothing else was printed — so the issue's exact shape (empty stderr, banner-only stdout) records a reason instead of echoing the banner, which is what main already did. - The persisted stdout tail is the model's answer: cap it at 200 chars (stderr keeps 500) and run the whole detail through `agent.redact.redact_sensitive_text(force=True, redact_url_credentials=True)`, the same scrub `cron.incidents` and `cron.delivery_queue` apply to their persisted errors — `last_delivery_error` had none. - Tests: the banner-only test now pins the exit-code/banner-only wording; one new test pins the 200-char cap and the scrub. Both red on the previous head.
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a cron job's
bot-chatdelivery turn fails,last_delivery_error(and the scheduler log line) now records the exit code plus BOTH the child's stderr and stdout tails, labeled and scrubbed — stdout is no longer dropped when stderr is present, and the banner-only shape saysexit code N | stdout was only the resume bannerinstead of echoing the banner as if it were a reason.cron/scheduler_delivery.py::_format_failure_streams(salvaged from fix(cron): report both streams on bot-chat delivery failure #104068, resolved onto the current plain-language failure message):exit code N | stderr: … | stdout: …replaces(stderr or stdout)[-500:]and the extra[-200:]cut. stderr keeps 500 chars; the stdout tail (the model's answer) is capped at 200 chars with theResumed session/session_id:banner lines dropped; the whole detail goes throughagent.redact.redact_sensitive_text(force=True, redact_url_credentials=True)before it is persisted, ascron.incidentsandcron.delivery_queuealready do for their stored errors.Validation
before: with stderr empty and stdout =
Resumed session … "Bot Chat" (1 user message, 1 total messages), the recorded error was… Details: Resumed session …(banner only, no reason; the reporter's case), and when both streams carried text only stderr's last 200 chars survived.after:
… Details: exit code 1 | stderr: <reason> | stdout was only the resume banner; banner-only stdout with empty stderr →exit code 1 | stdout was only the resume banner; a 50 KB stdout tail is persisted as ≤200 chars, secrets scrubbed.Live repro: real
cron.scheduler_delivery._deliver_to_bot_chatagainst a realhermes chat --in ~ -c "Bot Chat" --create-if-missing -Q --query-filechild in a scratchHERMES_HOMEwhose provider is invalid (probe_104056.py). Base:Details: Unknown provider 'openai'. …; head:Details: stderr: Unknown provider 'openai'. …— same child, stream now named; the banner-only shape is pinned by the unit test. Backend-only surface (cron scheduler →hermes cron runs/ the job'slast_delivery_error); no renderer change, so no Electron leg.Tests:
scripts/run_tests.sh tests/cron/test_cron_bot_chat_delivery.py→ 19 passed;scripts/run_tests.sh tests/cron→ 1381 passed, 0 failed.Root cause:
-Qsplits the resume banner/session_id:and the real reason across the two streams, and the delivery lane kept only one of them.Part of #104056 — this is the diagnostics atom (atom 2). Atom 3 is already on main: a Bot Chat held by a Desktop/CLI owner no longer hard-fails the leg (deferred to the owner mailbox / until the CLI owner releases — #111273,
5d8390d1a4b33,3b0fe0cc2b5f9,002ee41cfcc0c), and refusals ride ahermes-refusal-reason:marker on stderr (549e6aab38c49), which this change makes visible in the recorded error. Atom 1 is still open and not touched here:chat -c "Bot Chat" --create-if-missingresolving a stale title→session mapping whose row is gone/unusable still exits 1 instead of falling back to creating a fresh titled session — onorigin/main,hermes_cli/main.py::_resolve_continue_argonly creates when the title lookup misses, andhermes_cli/oneshot.py::_load_resume_targetraisessession not foundfor a resolved id without a row (deliberately, per #105892 for explicit ids). With this PR that failure at least recordsexit code 1 | stderr: session not found: …instead of the bare resume banner; the fail-over-to-create itself belongs to a follow-up on the-c <title> --create-if-missingpath, not the cron delivery code.Salvages #104068 (@Halldrix) — commit authored by the contributor.
Review follow-up
Banner-only case overclaimed (head recorded the same banner as main, only labeled) → fixed in
20d0858061a:_format_failure_streamsalways leads withexit code N, strips the resume banner lines from the stdout tail and writesstdout was only the resume bannerwhen nothing else was printed. Title/body softened accordingly.Persisted
last_delivery_errorgrew to ~1.2 KB with 500 chars of unredacted model output → fixed in20d0858061a: stdout tail capped at 200 chars (stderr keeps 500) and the detail is scrubbed withagent.redact.redact_sensitive_text(force=True, redact_url_credentials=True). No redaction helper existed on thelast_delivery_errorpath (cron/jobs.pystores it verbatim;grep -rn redact cron/shows scrubbing only inincidents.py,delivery_queue.py,scheduler_script.py,scheduler_diagnostics.py), so the same helper those siblings use was applied — none invented.Body said the 'other two halves' were already on main → fixed (body edit): atom 1 (stale title→session mapping → exit 1 instead of the
--create-if-missingfallback) is NOT on main; thePart of #104056paragraph now names it as still open with the main-side evidence (hermes_cli/main.py::_resolve_continue_arg,hermes_cli/oneshot.py::_load_resume_target).Live repro: real
cron.scheduler_delivery._deliver_to_bot_chatwithsubprocess.runreturning realCompletedProcessshapes (probe over the previous head vs this head). Banner-only stdout, empty stderr — before:Details: stdout: Resumed session 20260905_121420_8084c7 "Bot Chat" … session_id: …(304 chars, banner only); after:Details: exit code 1 | stdout was only the resume banner(223 chars). 50 KB stdout + stderr — before: 1195-char record carrying 500 chars of the answer tail; after: 909 chars with a 200-char stdout tail. Backend-only surface; no Electron leg.Infographic