fix(cron): report success for one-shot jobs removed after completion (#71760)

[JonthanaHanh]

Summary

For finite one-shot jobs (repeat.times=1), mark_job_run removes the job from the store when completed >= times. The subsequent get_job() in _execute_job_now returns None, causing execution_success=False even though the job ran end-to-end and delivered its output.

This produced a false Ran now: failed. message when the operator manually triggered a one-shot job via hermes cron run.

Root Cause

In tools/cronjob_tools.py::_execute_job_now (line 642-643):

refreshed = get_job(job_id) or {}
ok = refreshed.get("last_status") == "ok"

When mark_job_run deletes the completed one-shot job, get_job returns None, so refreshed becomes {}, and last_status is never "ok". The fix handles the None case by treating job removal (after run_one_job returned True) as implicit success.

Changes

• tools/cronjob_tools.py: Handle get_job returning None for completed one-shot jobs in _execute_job_now
• tests/tools/test_cronjob_run_immediate.py: Add test test_execute_job_now_handles_oneshot_removal verifying the fix

Test Plan

• All 8 existing tests pass
• New test test_execute_job_now_handles_oneshot_removal passes
• Syntax check passes

Fixes #71760

[yinkev]

Blocking correctness issue: refreshed is None does not prove the run succeeded.

cron.scheduler.run_one_job() explicitly returns True when the job was processed even if the job itself failed; only an exception makes it return False. Separately, cron.jobs.mark_job_run() removes a finite one-shot once completed >= times regardless of the success argument. Therefore this sequence is valid:

run_job -> success=False
mark_job_run(job_id, False, "provider 500") -> one-shot removed
run_one_job(...) -> True  # processed
get_job(job_id) -> None

With this PR, that failed run is reported as success=True and loses its error.

I reproduced the regression on head f472490ca94f with a focused test: claim_job_for_fire=True, run_one_job=True, get_job=None, expected failure. The branch returns True (assert True is False).

The fix needs a terminal outcome source independent of the deleted job record. The existing execution ledger is the clean seam: create/pass an explicit execution_id into run_one_job, then read that exact execution's completed/failed status and error after it returns. Alternatively, extend the shared run API to return a terminal outcome without changing its existing processed/not-processed boolean contract used by the scheduler. Please add both removed-success and removed-failure one-shot regressions.

[yinkev]

I prepared the narrow correction on top of this PR so you can cherry-pick it rather than re-derive the ledger path:

git cherry-pick 8879aac9bf4711de71053d62272fabe9f8bad9df

Commit: yinkev@8879aac

It keeps the successful self-deleting one-shot behavior, but when the job record is gone it reads the immutable latest_execution(job_id) result written by run_one_job. That distinguishes completed from failed and preserves the execution error. It adds the symmetric removed-failure regression.

Focused result on your head plus this commit: 9 passed, Ruff/py_compile/diff check clean. I did not open a competing PR.

[teknium1]

Thanks for isolating the post-removal status path. The false Ran now: failed premise remains present on current main at tools/cronjob_tools.py:605-611.

Problems

• The changed success=bool(processed) line is not a terminal-success signal. cron/scheduler.py:3890-3891 specifies that run_one_job() returns True even when a job failed, while cron/jobs.py:1739-1743 removes finite jobs for either terminal outcome. A removed failed one-shot would be reported as successful and lose its error.
• tests/tools/test_cronjob_run_immediate.py covers only the successful removed case, so it does not exercise that failure path.

Suggested changes

• Use cron.executions.latest_execution(job_id) when the mutable job record is gone; its terminal completed/failed state and error are available at cron/executions.py:260-262.
• Add the symmetric removed-failure regression. The supplied follow-up in the discussion implements this approach.

Automated hermes-sweeper review.

[teknium1]

processed is not a success result: run_one_job() returns True for a processed failed job too (cron/scheduler.py:3890-3891), and finite jobs are removed for either terminal outcome (cron/jobs.py:1739-1743). Read the execution ledger here so a removed failed one-shot remains failed and retains its error.

[GottZ]

This was generated by AI during triage.

Summary

Four PRs address the same post-run status race: finite jobs delete their mutable job row before _execute_job_now reads last_status. #63570, #71768, and #71981 infer success from disappearance, while #76731 consults the durable execution ledger and also preserves the original response metadata.

Related pull requests

• fix(cron): one-shot jobs report 'failed' after successful completion #63570 [closed] duplicate — (+82/-3) — n/a: Same-root-cause fix, but it can misreport a removed failed run as successful because processed=True is not a terminal-success result; it remains relevant as a closed implementation duplicate, recorded as superseded by fix: report immediate cron run status for depleted repeat jobs #57689.
• fix(cron): report success for one-shot jobs removed after completion (#71760) #71768 best fix — (+28/-1) — recorded best-fix: Correctly isolates the deleted-row cause, but its missing-row branch treats every processed run as successful, contrary to the failed-run path documented at cron/scheduler.py:3890-3891 and cron/jobs.py:1739-1743. The keep-open contributor review should be followed by applying the supplied ledger-based correction and adding the symmetric removed-failure regression.
• fix(cron): report success for one-shot jobs removed after completion (#71760) #71981 duplicate — (+28/-1) — n/a: Its diff is effectively identical to fix(cron): report success for one-shot jobs removed after completion (#71760) #71768 and has the same failed-run misclassification. Despite the maintainer-bot keep-open verdict, the exact duplicate diff and the contributor's duplicate finding support closing it as duplicate of fix(cron): report success for one-shot jobs removed after completion (#71760) #71768 rather than maintaining a second salvage branch.
• fix(cron): report completed one-shot manual runs #76731 best fix — (+46/-4) — recorded best-fix: Uses latest_execution(job_id) to distinguish terminal completed from failed after row deletion and preserves the original job metadata, directly addressing both the false status and fallback metadata. The maintainer-bot keep-open verdict is supported, with author action still needed to add a removed-failure test that verifies error preservation.

Duplicates

#63570, #71768, and #71981 implement the same missing-row-as-success approach; #71981 is an exact practical duplicate of #71768, while #76731 is the ledger-based correction rather than an equivalent duplicate.

Suggested consolidation

Keep #76731 open with a salvage path: add the symmetric removed-failure ledger regression and verify that its terminal error is preserved. Author action on #71768: replace the processed inference with the supplied execution-ledger correction and failure regression, or defer to #76731; close #71981 as duplicate of #71768, and leave #63570 closed as the earlier superseded duplicate.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I71760(["issue #71760 (open)"])
    subgraph Dup63570 ["PRs duplicating each other"]
        P63570["PR #63570 (closed)"]
        P71768["PR #71768 (open)"]
        P71981["PR #71981 (open)"]
    end
    P71768 -->|best fix| I71760
    class I71760 open
    class P63570 closed
    class P71768 open
    class P71981 open
    class P71768 best
    class P71768 target
    click I71760 "https://github.com/NousResearch/hermes-agent/issues/71760"
    click P63570 "https://github.com/NousResearch/hermes-agent/pull/63570"
    click P71768 "https://github.com/NousResearch/hermes-agent/pull/71768"
    click P71981 "https://github.com/NousResearch/hermes-agent/pull/71981"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 4 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 8 kB of issue/PR text, 6 kB of discussion (9 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.