Skip to content

fix: clarify cron delivery error status - #36731

Open
ctrlBizz wants to merge 1 commit into
NousResearch:mainfrom
ctrlBizz:fix/cron-delivery-status-label
Open

fix: clarify cron delivery error status#36731
ctrlBizz wants to merge 1 commit into
NousResearch:mainfrom
ctrlBizz:fix/cron-delivery-status-label

Conversation

@ctrlBizz

@ctrlBizz ctrlBizz commented Jun 1, 2026

Copy link
Copy Markdown

Summary

  • clarify cron list wording so delivery failures are not mistaken for execution failures
  • add CLI regression coverage for a successful run with a delivery error

Verification

  • ./venv/bin/python3 -m pytest tests/hermes_cli/test_cron.py tests/cron/test_jobs.py::TestMarkJobRun::test_delivery_error_cleared_on_success -q -o 'addopts='
  • hermes cron list --all | grep -A12 'ad510105c212'
  • hermes cron status

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

@mxnstrexgl mxnstrexgl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review

Security Scan

  • ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.

Code Quality

  • ✓ No blocking code-quality issues found by this automated scan.

Summary

Status: APPROVE — security findings: 0, quality suggestions: 0.

Automated review; raw diff content intentionally omitted.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused CLI regression coverage. The premise remains valid on current main: hermes_cli/cron.py:167-179 renders an ok execution status separately from last_delivery_error, so the old Delivery failed: wording can be read as an execution failure.

Problems

  • website/docs/guides/cron-troubleshooting.md:75 says a delivery failure appears in last_error, but cron/jobs.py:1476-1488 records it in last_delivery_error independently of execution status. Please update that troubleshooting guidance alongside the CLI wording.

Suggested changes

  • Document that hermes cron list can show a successful last run plus a separate last-delivery failure.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants