Repository navigation
Conversation
…ller's jobs The cron runtime keeps every job of a profile in one file, cron/jobs.json, and the distribution payload treated it as a single root replaced wholesale. `hermes profile update` therefore deleted every job the installer had added and re-armed shipped jobs they had paused, and `profile install` left shipped jobs enabled, so a job that was due in the author's profile fired on the first tick, contradicting the "not auto-scheduled" promise in the CLI and docs. Merge the store job by job, keyed on the id the author's store assigned (kept on import so context_from chains still resolve). A shipped job the profile already has gets the author's definition (the create_job fields) and keeps its enabled/paused state and run history. A job new to the profile is imported paused with no next_run_at. The installer's own jobs are left alone. The write goes through the store's lock and save_jobs. Runtime state inside cron/ and skills/ (locks, ticker markers, the WAL-mode ledgers, run output, hub, usage and curator state) is no longer shipped or replaced. The docs' .gitignore guidance now excludes it and keeps cron/jobs.json.
|
Thanks @jonpol01 — for the report in #120823 and for this first fix. The fix has landed in #121264 as a salvage stack built on #120910's per-job merge, and it covers everything this PR set out to do:
You are credited as co-author on 68ca2c8. Merged as |
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.
What does this PR do?
This fixes cron jobs in profile distributions:
profile updateno longer deletes the installer's own jobs. Before, a shippedcron/jobs.jsonreplaced the whole store.profile installnow brings shipped jobs in paused, as the Security section already promised.cron/andskills/.Related Issue
Fixes #120823
This follows up #111344, whose tests modelled user jobs as loose
cron/*.jsonfiles the runtime never reads. #44386 is mostly superseded. #109581 rewrites the copy path without merging cron jobs; this change is local (one table, one merge function, arelargument), so rebasing either one should be simple.Type of Change
Changes Made
hermes_cli/profile_distribution.py:_merge_cron_jobsmergescron/jobs.jsonjob by job, keyed on the job id the author's store assigned, which is kept on import socontext_fromchains resolve. It works under the cron store's own lock, and reads a copy of the shipped file so a local source is never written.create_jobarguments) and keeps its own enabled/paused state, run history and failure streak._MERGED_FILEStable routescron/jobs.jsonto that merge. Every other owned file keeps the existing replace._RUNTIME_STATElists what the runtime itself writes insidecron/andskills/. Owned-entry listing and_merge_dirskip it, so the installer's live copies are never swapped out. Each entry was checked against the code that writes it.website/docs/user-guide/profile-distributions.md:.gitignorenow lists that runtime state (cron/jobs.jsonstays committed);create_jobashermes cron adddoes.test_install_leaves_shipped_cron_jobs_paused_and_not_due: red onmain(is_job_runnableis true, and the job fires).test_update_merges_cron_store_per_job: red onmain("the installer's own cron job was deleted by the update").Trade-offs:
origin,deliverandworkdircount as the author's definition, as they did under the old wholesale copy.How to Test
scripts/run_tests.sh tests/hermes_cli/test_profile_distribution.py tests/cron/test_jobs.py: 162 passed. Both new tests fail onmain.('weekly-digest', 'disabled', 'paused')and nothing is due. Afterprofile update, both the shipped job andmy-own-reminderare still there.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass. I ran only the related files above throughscripts/run_tests.sh, not the full suite.Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A