fix(cron): list_cron_job_runs opens job's own profile state.db - #52018
Open
performacia wants to merge 1 commit into
Open
fix(cron): list_cron_job_runs opens job's own profile state.db#52018performacia wants to merge 1 commit into
performacia wants to merge 1 commit into
Conversation
When a cron job's jobs.json lives in one profile (e.g. 'default') but the job carries profile='gerente_agente_de_compras', run sessions are written into the named profile's state.db — not the default one. The endpoint was calling _open_session_db_for_profile(selected) where selected came from _find_cron_job_profile() (the jobs.json owner), so it always opened the wrong database and returned stale/empty run history. Root cause: _annotate_cron_job() overwrote the job's original 'profile' field (which names the profile whose state.db holds the runs) with the name of the profile that owns jobs.json. The original value was lost. Fix: - _annotate_cron_job() now preserves the raw 'profile' field from jobs.json into a new 'scheduler_profile' key before overwriting 'profile' with the jobs.json-owner name. - list_cron_job_runs() reads 'scheduler_profile' and opens that profile's state.db when it differs from the jobs.json owner, ensuring run sessions written by the scheduler are always found. Regression test added: creates a job in 'default' jobs.json tagged with profile='worker_alpha', writes a run session into worker_alpha/state.db, and asserts the endpoint returns that run (not zero).
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes cron run-history retrieval in the dashboard by ensuring /api/cron/jobs/{id}/runs opens the state.db belonging to the profile the scheduler ran the job under (not the profile that owns jobs.json), and adds a regression test for cross-profile cron jobs.
Changes:
- Preserve the scheduler-stamped job execution profile during cron job annotation (
scheduler_profile). - Update
list_cron_job_runs()to select the correct profile database for run-session lookups. - Add a regression test covering cross-profile run history retrieval.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
hermes_cli/web_server.py |
Preserves the job’s scheduler profile during annotation and uses it to open the correct profile’s state.db for run history. |
tests/hermes_cli/test_web_server_cron_profiles.py |
Adds a regression test to ensure run history is read from the job’s execution profile DB. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+202
to
+203
| @pytest.mark.asyncio | ||
| async def test_list_cron_job_runs_reads_job_profile_state_db(isolated_profiles, tmp_path): |
Comment on lines
+213
to
+217
| import json | ||
| import time | ||
|
|
||
| from hermes_state import SessionDB | ||
| from hermes_cli import web_server |
| if j.get("id") == job_id: | ||
| j["profile"] = "worker_alpha" | ||
| break | ||
| jobs_file.write_text(json.dumps(data if isinstance(data, list) else data)) |
Comment on lines
7749
to
+7755
| job = _call_cron_for_profile(selected, "get_job", job_id) | ||
| if job and job.get("id"): | ||
| canonical = str(job["id"]) | ||
| if isinstance(job, dict): | ||
| if job.get("id"): | ||
| canonical = str(job["id"]) | ||
| sched_prof = job.get("scheduler_profile", "") | ||
| if sched_prof and sched_prof != selected: | ||
| db_profile = sched_prof |
alt-glitch
added
type/bug
teknium1
reviewed
Jul 15, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the focused regression test. I found a blocker against current main's cron model.
Problems
- Current cron is intentionally per-profile: a job lives and executes under the same profile HERMES_HOME (
cron/jobs.py:54-64;tests/cron/test_cron_profile_isolation.py:3-17). create_jobhas noprofileargument (cron/jobs.py:1033-1051), and the added test manually injectsjob["profile"]into another profile'sjobs.json. That is not a current supported scheduler record.
Suggested changes
- Please obtain maintainer direction before re-scoping this as a legacy-migration case. A fix should preserve the current profile-isolation boundary rather than route run-history reads from an arbitrary per-job field.
Automated hermes-sweeper review.
| # The scheduler stamps job["profile"] with the profile the job runs under | ||
| # (where its run sessions are written); _annotate_cron_job then overwrites | ||
| # ``profile`` with the jobs.json owner. Keeping the original in | ||
| # ``scheduler_profile`` lets endpoints that need to open the correct |
Contributor
There was a problem hiding this comment.
Current main does not persist a scheduler-stamped per-job profile: cron/jobs.py:create_job has no such parameter, and cron is intentionally per-profile. This new field is therefore populated only by an unsupported/hand-edited record and would reintroduce cross-profile session lookup.
| for j in (data if isinstance(data, list) else data.get("jobs", [])): | ||
| if j.get("id") == job_id: | ||
| j["profile"] = "worker_alpha" | ||
| break |
Contributor
There was a problem hiding this comment.
This manually creates the cross-profile record that current main deliberately avoids: tests/cron/test_cron_profile_isolation.py:3-17 requires jobs to live and execute under one profile. Please replace this with an approved legacy-migration fixture or remove the unsupported scenario.
teknium1
added
sweeper:risk-session-state
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a cron job's
jobs.jsonlives in one profile (e.g.default) but the job carriesprofile='gerente_agente_de_compras', run sessions are written into the named profile'sstate.db— not the default one.The
GET /api/cron/jobs/{id}/runsendpoint was always opening the wrongstate.db, so the UI showed stale or empty run history even though runs existed on disk.Root Cause
_annotate_cron_job()overwrote the job's originalprofilefield (which names the profile whosestate.dbholds the run sessions) with the name of the profile that ownsjobs.json. The original value was permanently lost after annotation.list_cron_job_runs()then called_open_session_db_for_profile(selected)whereselectedwas thejobs.jsonowner — always the wrong DB for cross-profile jobs.Fix
_annotate_cron_job()now copies the rawprofilefield fromjobs.jsoninto a newscheduler_profilekey before overwritingprofilewith thejobs.json-owner name. This preserves the scheduler-stamped value without changing the existingprofilesemantics the UI relies on.list_cron_job_runs()readsscheduler_profilefrom the resolved job and opens that profile'sstate.dbwhen it differs from thejobs.jsonowner, ensuring run sessions written by the scheduler are always found.Test
Regression test added to
tests/hermes_cli/test_web_server_cron_profiles.py:defaultjobs.jsontagged withprofile='worker_alpha'worker_alpha/state.dblist_cron_job_runs()returns that run (previously returned 0)All 8 tests in the file pass;
test_cron.pyandtest_cron_fire_dashboard.py(10 tests) also pass unchanged.