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

[JonthanaHanh]

Summary

Fixes #71760.

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

Changes

• tools/cronjob_tools.py: Handle get_job() returning None after run_one_job returns True — treat job removal as implicit success
• tests/tools/test_cronjob_run_immediate.py: Added test test_execute_job_now_handles_oneshot_removal

Testing

• New test verifies that a one-shot job removed by mark_job_run reports success=True

[alt-glitch]

This was generated by AI during triage.

Duplicate of #71768: both treat removal of a successfully processed one-shot job as implicit success in _execute_job_now.

[teknium1]

Thanks for isolating the self-deleting one-shot status path. The false Ran now: failed premise remains on current main at tools/cronjob_tools.py:606-611.

Problems

• The proposed success=bool(processed) branch is not a terminal-success signal. cron/scheduler.py:3890-3891 defines run_one_job() returning True for a processed job even when the job failed, while cron/jobs.py:1740-1744 removes finite jobs for either terminal outcome. A removed failed run would be reported successful and its error lost.
• The added regression covers only the removed-success case; it does not exercise a removed failed one-shot.

Suggested changes

• On a missing post-run job record, use the durable execution-ledger outcome (cron/executions.py:175-196, 260-262) rather than the processed boolean.
• Add the symmetric removed-failure regression, asserting success=False and preservation of the terminal error.

This is an automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address #71760 by changing _execute_job_now after a finite one-shot job has been removed: all three replace the false failure caused by the missing mutable job record with an inferred success, but none of the submitted diffs distinguishes a removed successful run from a removed failed run.

Related pull requests

• #63570 [closed] duplicate — (+82/-3) — closed duplicate: It identifies the correct read-after-removal cause and adds broad tests, but infers success from processed=True plus a missing job record, which can misreport a processed failure as successful. It remains relevant as the same attempted fix, although it was closed as a duplicate of #57689 in favor of that PR's structured outcome approach.
• #71768 best fix — (+28/-1) — keep open with a salvage path: The diff fixes the reported successful-removal case, but run_one_job() can return True for failed executions and finite jobs are removed for either outcome, so the new branch loses failed-run status and error details. Preserve the isolated regression and replace the bool(processed) inference with the durable execution-ledger result, then add the symmetric removed-failure regression, as required by the contributor review.
• #71981 partial — (+28/-1) — close as duplicate of #71768: Its code and test are identical to #71768 and therefore have the same correctness gap: a removed failed one-shot is reported as successful and its error is discarded. Despite the keep-open maintainer-bot verdict on #71981, its salvage path is already represented by the recorded best-fix vehicle #71768, where the ledger-based correction and failure regression can be consolidated without maintaining two identical PRs.

Duplicates

#63570, #71768, and #71981 implement the same missing-record-to-success inference; #71981 is an exact duplicate of #71768, while closed #63570 is the same core approach with additional tests and was itself closed as a duplicate of #57689.

Suggested consolidation

Keep #71768 open with a salvage path: retain its focused successful-removal regression, read the exact terminal outcome and error from the execution ledger when get_job(job_id) returns None, and add a removed-failure regression. Close #71981 as a duplicate of #71768 despite its keep-open maintainer-bot verdict, because the complete diff is identical and offers no separate salvageable change; leave #63570 closed under its existing duplicate chain to #57689.

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
    P71981 -.->|partial| I71760
    class I71760 open
    class P63570 closed
    class P71768 open
    class P71981 open
    class P71768 best
    class P71981 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 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 10 kB of PR diffs, 6 kB of issue/PR text, 5 kB of discussion (7 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.