Skip to content

fix(cron): a restart-safe worker that dies before its ack reports its own stderr instead of a bare exit 1 (#112729) - #113219

Merged
teknium1 merged 2 commits into
mainfrom
fix/b113-cron-delegation-worker-stderr
Sep 17, 2026
Merged

teknium1 merged 2 commits into
mainfrom
fix/b113-cron-delegation-worker-stderr

Conversation

@teknium1

@teknium1 teknium1 commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

When a restart-safe cron worker dies before it acknowledges the handoff, the dispatch error now carries the worker's own stderr tail (e.g. the ModuleNotFoundError traceback) instead of just exit 1.

  • cron/scheduler.py::_launch_external_cron_worker: worker stderr goes to <home>/cron/external-workers/<execution_id>.stderr (0o600) instead of DEVNULL; on a pre-ack exit the redacted tail is appended to the RuntimeError, which already flows into last_error, the executions ledger and the gateway log (Restart-safe cron worker dispatch failed: …). The file rides in handoff_files, so every exit path removes it.
  • cron/scheduler_diagnostics.py::external_worker_stderr_tail: 1500-char tail through redact_sensitive_text(force=True, redact_url_credentials=True).
  • One invariant test in tests/cron/test_restart_safe_worker.py (real subprocess that fails at import; asserts the cause is in the message and no exec-1.* handoff file survives).

Validation

Live probe: real _launch_external_cron_worker under a temp HERMES_HOME, dispatch argv rewritten to an interpreter that fails at import and exits 1 before writing the ack:

before (origin/main 9796235): cron external worker exited before ownership acknowledgement (exit 1)
after: cron external worker exited before ownership acknowledgement (exit 1); worker stderr: Traceback (most recent call last): … ModuleNotFoundError: No module named 'cron_missing_module'
Both: leftover handoff files: [].

scripts/run_tests.sh tests/cron/test_restart_safe_worker.py → 24 passed; the new test is red with the source swapped back to origin/main (23 passed, 1 failed).

Why this and not the proposed interpreter change

The issue's mechanism does not hold: CPython does not resolve symlinks in sys.executable (measured: a venv whose bin/python3 is a symlink reports sys.executable=<venv>/bin/python3, sys.prefix=<venv>, and a child spawned with sys.executable keeps the venv — matching @KeyArgo's analysis and @holny's withdrawal on the thread). The reporter's python -m cron.scheduler --help control is not the worker's launch path either. What IS real is that the worker's actual exit-1 cause was unrecoverable because its stderr went to DEVNULL and its own log handler installs only after cron.scheduler imports — so this PR makes the next such report self-diagnosing. #112731 (@KoNit-K, $VIRTUAL_ENV/bin/python preference) is not salvaged: it is a no-op wherever sys.executable is already the venv interpreter and VIRTUAL_ENV need not name the gateway's venv under systemd.

Part of #112729 — the reporter's install still needs the captured stderr from a re-run to identify the real failure.

Infographic

cron-worker-stderr

Review follow-up

  • MINOR A (orphaned <execution_id>.stderr capture when the gateway dies mid-run in the restart-safe topology) → fixed @44030f683cbda: the acknowledged worker unlinks ack_path.with_suffix(".stderr") in its post-run finally; pre-ack paths are unchanged so a worker dying before its ack still gets its stderr reported. test_external_worker_adopts_execution_and_runs_payload_once now seeds the sibling and asserts it is gone.
  • MINOR B (two comments still said stderr is DEVNULL: systemd-run failure branch, __main__ setup_logging motivation) → fixed @44030f683cbda; git grep DEVNULL -- cron/scheduler.py now matches only the real stdin/stdout DEVNULL args and the reworded comment.

… own stderr

`_launch_external_cron_worker` spawned the worker with `stderr=DEVNULL`, so a worker
that crashed before publishing its acknowledgement — an import error, a module missing
from the interpreter it was handed, a bad payload — surfaced only as
"exited before ownership acknowledgement (exit 1)". #112729 shows the cost: the
reporter, a claimant and a reviewer spent three rounds guessing at the mechanism
(sys.executable symlinks — refuted) because the worker's actual traceback was thrown
away. The worker's own log handler is installed only after `cron.scheduler` imports,
so an import-time failure never reaches agent.log either.

Capture stderr into `<home>/cron/external-workers/<execution_id>.stderr` (0o600), and
on a pre-ack exit append a redacted tail of it to the dispatch error (which lands in
`last_error`, the executions ledger and the gateway log). The file rides in
`handoff_files` so every exit path removes it; the tail helper lives in
`scheduler_diagnostics` next to the run-error redaction.

Part of #112729
@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

ran on 44030f6 — fix(cron): acknowledged worker removes its own stderr captur

⚠️ Warnings

CI timings · View report · View job

Wall time 13m45s vs 6m8s (+124.2%). 6 job(s) slower, 3 faster, 3 unchanged.

  • OS-specific tests / macOS-only tests: +12.0s
  • Python lints / Windows footguns (blocking): +11.0s
  • Python tests / e2e: +6.0s
  • Detect affected areas: -6.0s
  • Python tests / Run tests: -4.0s

…ts match captured stderr

The `<execution_id>.stderr` capture was unlinked only by the launching gateway
(pre-ack exit branch and the `handoff_files` finally). In the restart-safe
topology the worker deliberately outlives a gateway restart, so a gateway that
died mid-run left one orphan file per surviving run under
cron/external-workers/. After the ack the gateway never reads the capture
(it only serves the pre-ack death report), so the worker now unlinks the
sibling derived from its ack path in the post-run finally; the pre-ack paths
are unchanged so a worker dying before its ack still gets its stderr reported.

Two comments still described stderr as DEVNULL (the systemd-run failure branch
and the `__main__` setup_logging motivation); reword them to the captured-stderr
behaviour while keeping the WHY.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant