fix: persist manual cron run results

[NocGeek]

Summary

Fixes manual WebUI cron runs so they persist results the same way scheduled cron ticks do.

Manual runs previously called cron.scheduler.run_job(job) and then only cleared the in-memory running flag. That meant output could be dropped and job metadata like last_run_at / last_status was not updated after a manual run.

This PR:

• saves manual-run output with save_job_output
• marks manual runs complete with mark_job_run
• records manual-run failures in job metadata
• keeps _run_cron_tracked self-contained for worker-thread execution
• adds a regression test for manual-run persistence

Cron Jobs Project Note

This does not change project scoping or filtering.

Manual runs still execute through cron.scheduler.run_job, so the v0.50.247 Cron Jobs project/session metadata behavior remains independent of this fix. This patch only persists cron output and job run metadata.

Tests

• python3 -m py_compile api/routes.py tests/test_cron_manual_run_persistence.py
• python -m pytest tests/test_cron_manual_run_persistence.py tests/test_cron_run_job_import.py tests/test_sprint3.py::test_crons_run_nonexistent tests/test_sprint10.py::test_crons_output_limit_param

result: 7 passed

[nesquena]

Review — end-to-end ✅ (clean approve, contributor PR)

External contributor (@NocGeek). Fixes a real defect in the manual cron-run path: clicking "Run Now" in the WebUI executes the cron job's agent run but discards the result — output is never written, last_run_at / last_status never updates. The fix mirrors the scheduled cron path's persistence handling exactly.

What this ships

PR adds 18 lines to _run_cron_tracked at api/routes.py:95-119:

def _run_cron_tracked(job):
    from cron.scheduler import run_job
    from cron.jobs import mark_job_run, save_job_output

    job_id = job.get("id", "")
    try:
        success, output, final_response, error = run_job(job)
        save_job_output(job_id, output)

        if success and not final_response:
            success = False
            error = "Agent completed but produced empty response (model error, timeout, or misconfiguration)"

        mark_job_run(job_id, success, error)
    except Exception as e:
        logger.exception("Manual cron run failed for job %s", job_id)
        try:
            mark_job_run(job_id, False, str(e))
        except Exception:
            logger.debug("Failed to mark manual cron run failure for %s", job_id)
    finally:
        _mark_cron_done(job_id)

Pre-fix behaviour: _run_cron_tracked called run_job(job) and discarded the 4-tuple return value, then cleared the in-memory running flag. No persistence. Output gone. Job metadata stale. Confirmed by tracing.

Traced against upstream hermes-agent

Verified upstream signatures all match:

• cron.scheduler.run_job(job) at /tmp/hermes-agent-fresh/cron/scheduler.py:798 returns tuple[bool, str, str, Optional[str]] = (success, full_output_doc, final_response, error_message). ✅ PR's tuple unpack matches.
• cron.jobs.save_job_output(job_id, output) at /tmp/hermes-agent-fresh/cron/jobs.py:859. Writes timestamped .md file into OUTPUT_DIR/job_id/. ✅
• cron.jobs.mark_job_run(job_id, success, error=None, delivery_error=None) at /tmp/hermes-agent-fresh/cron/jobs.py:678. Updates last_run_at, last_status, last_error. Holds _jobs_file_lock internally. ✅ PR uses positional args (no delivery_error — correct, manual runs don't deliver).

Verified that run_job itself does NOT internally call save_job_output or mark_job_run — those calls live in _process_job (/tmp/hermes-agent-fresh/cron/scheduler.py:1334-1364) which the manual-run path bypasses. So the PR's persistence calls are necessary, not redundant.

End-to-end trace

1. User clicks "Run Now" on a cron job in the UI → POST /api/crons/run with {"job_id": "..."}.
2. _handle_cron_run at api/routes.py:3736-3752:
   • get_job(job_id) from upstream cron.jobs.
   • Prevent double-run via _is_cron_running.
   • _mark_cron_running(job_id) (in-memory flag).
   • threading.Thread(target=_run_cron_tracked, args=(job,), daemon=True).start().
3. _run_cron_tracked runs in worker thread:
   • run_job(job) invokes the agent pipeline; returns (success, full_doc, final_response, error).
   • save_job_output(job_id, full_doc) writes the timestamped .md to OUTPUT_DIR/<job_id>/. The file shows up in the UI's output history.
   • Empty-response demotion (mirrors scheduled path at scheduler.py:1360-1362): if success && !final_response, mark as failure with the same canonical error string. Without this, an agent that ran but produced nothing would falsely show "ok" in the UI.
   • mark_job_run(job_id, success, error) writes to the jobs JSON via _jobs_file_lock — updates last_run_at, last_status, last_error, next_run_at.
4. _mark_cron_done(job_id) in finally clears the in-memory running flag — always fires.

Comparison with scheduled cron path

Step	Scheduled _process_job	Manual _run_cron_tracked (PR)
Run agent	run_job(job)	run_job(job) ✅ same
Save output to disk	save_job_output(...)	save_job_output(...) ✅ same
Deliver final_response to chat	yes	NO (UI is the surface) ✅ correct
Track delivery_error	yes	omitted ✅ correct (no delivery)
Empty-response demotion	yes	yes ✅ matches text + logic
mark_job_run	yes	yes ✅
Exception path	mark_job_run(False, str(e))	mark_job_run(False, str(e)) + nested guard ✅
Cleanup	(n/a)	_mark_cron_done in finally ✅

Cleanly mirrors the scheduled path's persistence semantics while correctly omitting delivery (which only makes sense for scheduled runs that deliver to chat platforms).

Race / lock analysis

• mark_job_run acquires _jobs_file_lock internally (cron/jobs.py:689). The scheduled cron driver and the manual-run worker thread serialize through this lock when updating jobs JSON. ✅
• save_job_output writes a unique timestamped file per call (OUTPUT_DIR/job_id/<YYYY-MM-DD_HH-MM-SS>.md). Two manual runs of the SAME job within the same second could collide on filename — but _handle_cron_run already prevents double-running the same job (the _is_cron_running check at line 3746). ✅
• _run_cron_tracked runs in a daemon thread (line 3751). The HTTP handler returns immediately. The worker has its own stack; no shared mutable state with the handler beyond the in-memory _cron_running dict (which has its own lock).
• run_job may mutate os.environ["TERMINAL_CWD"] for workdir-bearing jobs (scheduler.py:1372-1377 comment). Two concurrent manual runs of DIFFERENT workdir jobs would race on this. Pre-existing concern, not introduced by this PR. The webui also doesn't gate on this — but _is_cron_running only blocks per-job, not globally.

Cross-tool consistency

• ✅ All persistence goes through upstream cron.jobs functions — no webui-internal markers or schemas.
• ✅ The output file produced is the same format that the scheduled path writes; the WebUI's cron output viewer reads the same OUTPUT_DIR/job_id/*.md files (same code path as for scheduled runs).
• ✅ No config.yaml writes.
• ✅ No new endpoints, no new env vars.

Security audit

• ✅ job_id comes from job.get("id", "") where job was fetched via cron.jobs.get_job(job_id) validated at line 3742. Trusted.
• ✅ No string interpolation of user data into shell, SQL, or paths.
• ✅ No new file-serving surface.
• ✅ The error = "Agent completed but produced empty response (...)" literal is hardcoded — no user data in it.
• ✅ logger.exception and logger.debug use %s formatting safely.

Edge-case matrix

Scenario	Pre-fix	Post-fix
Manual run, agent succeeds with text response	Output dropped, status not updated	save + mark(ok) ✅
Manual run, agent succeeds with empty final_response	Output dropped, status not updated	save + mark(error="empty response") ✅
Manual run, agent fails with error	Output dropped, status not updated	save + mark(error) ✅
Manual run, run_job itself raises (e.g. import error)	Worker thread crashes, status never updated, running flag stuck until process restart	except → mark(False, exc); finally clears running flag ✅
save_job_output raises (disk full)	n/a	except → mark(False, exc); slight inaccuracy (run was OK, save failed) but no stuck-running
mark_job_run raises	n/a	nested except logs and falls through to finally ✅
Two manual runs of same job concurrently	_is_cron_running blocks the second	unchanged ✅
SILENT_MARKER in final_response	not detected (no delivery anyway)	not detected — manual runs never deliver, so no behavioral concern. The output is still saved verbatim ✅
Job missing id field	_mark_cron_done("") no-op	mark_job_run("", ...) would try to find job with empty id (pre-existing)

Tests

• PR's tests: 2/2 pass — test_manual_cron_run_saves_output_and_marks_job and test_manual_cron_run_marks_empty_response_as_failure. They use monkeypatch.setitem(sys.modules, "cron", ...) to inject mock cron.jobs and cron.scheduler modules, then assert the exact call sequence and arg shapes. Confirms save → mark order, empty-response demotion, and _is_cron_running cleanup.
• Full suite: 3409 passed, 54 skipped, 3 xpassed, 0 failed in 16.08s on 8d3f005.
• No CI run yet — branch was just pushed and there's no scheduled workflow trigger.

Minor observations (non-blocking)

• Exception path not covered by tests. The except Exception as e: mark_job_run(job_id, False, str(e)) block is a real safety net but isn't exercised. A test that makes run_job raise would lock this behavior. Easy follow-up if the contributor or maintainer chooses.
• The PR uses mark_job_run(job_id, success, error) positionally, not mark_job_run(job_id, success, error=error). Positional binding is fine because the signature mark_job_run(job_id, success, error=None, delivery_error=None) puts error as the 3rd positional arg. Verified.
• save_job_output return value discarded. Scheduled path uses it for if verbose: logger.info("Output saved to: %s", output_file). Manual path could log the same — minor UX nicety, not a defect.
• Manual runs of workdir-bearing jobs racing on os.environ["TERMINAL_CWD"] is pre-existing. Out of scope; mention only because the PR description's "keeps _run_cron_tracked self-contained for worker-thread execution" claim makes me check that the function doesn't introduce new shared state — it doesn't. ✅
• get_job is called BEFORE _is_cron_running — if the user spam-clicks, multiple HTTP handlers could each pass the existence check, then race on _is_cron_running. Only one wins (others see "already_running"). The first to call _mark_cron_running wins. Fine.

Recommendation

✅ Approved. Mirrors the scheduled cron path's persistence pattern exactly, with the correct omissions (no delivery, no delivery_error). All upstream signatures verified. Tests cover happy path + empty-response demotion. Defensive nested try-except around the failure-path mark_job_run prevents stuck-running state on cascading errors.

Thank you @NocGeek for the clean focused PR — straightforward bug fix with solid regression coverage. Parked at approval — ready for the release agent's merge/tag pipeline.

[nesquena-hermes]

Shipped in v0.50.251 (merge 031feda3, tag https://github.com/nesquena/hermes-webui/releases/tag/v0.50.251). Thanks @NocGeek — exactly the focused split-out I asked for in the hold comment on #1352.

Live at https://github.com/nesquena/hermes-webui/releases/tag/v0.50.251.

Your manual-cron persistence fix lands clean — verified the API signatures match upstream cron.scheduler.run_job exactly, the empty-response soft-failure mirrors the scheduled-cron path's error string, and the 2 monkeypatch-based behavioral tests are exactly the right pattern (exercises the real _run_cron_tracked code path).

What's next for #1352

#1352 is now superseded — your #1372 shipped the manual-run-output fix, and #1374 (the resubmit) carries the architectural profile-aware-cron-panel piece. I'm closing #1352 with a pointer to both successors. #1374 is on hold pending the architectural review I outlined in its hold comment.