fix: add per-job allow_silent flag for recurring briefing/report jobs - #53248
fix: add per-job allow_silent flag for recurring briefing/report jobs#53248Sahil-SS9 wants to merge 3 commits into
Conversation
Competing with #53252 for the same fix (both implement a per-job |
3d6bfac to
b8d72b1
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused implementation. The issue premise is confirmed on current main: cron/scheduler.py:2263-2276 still injects generic [SILENT] guidance for every cron job, and cron/scheduler.py:3493-3510 suppresses recognized silence responses.
Problems
- The PR changes both contracts without tests. Current tests assert unconditional prompt guidance at
tests/cron/test_scheduler.py:2672-2689and suppression attests/cron/test_run_one_job.py:69-79; add coverage forallow_silent=Falseand legacy/default behavior. - The flag is available only through
cronjob. CLI create/edit has no corresponding option (hermes_cli/subcommands/cron.py:26-136), and blueprint job construction does not carry it (tools/blueprints.py:172-194). This leaves those normal creation paths unable to request always-deliver behavior.
Suggested changes
- Apply the delivery predicate to the current
run_one_job()block atcron/scheduler.py:3493-3510; that block has moved since this branch's scheduler hunk and now preserves deferred-agent teardown. - Add persistence, prompt, and delivery regression tests, then either include the intended creation/edit surfaces or retain this explicitly as the minimal tool-only lane alongside #53252.
Automated hermes-sweeper review.
| @@ -1085,6 +1091,11 @@ def cronjob( | |||
| "type": "boolean", | |||
There was a problem hiding this comment.
Please add regression coverage for this newly exposed parameter: create/update round trips, legacy default suppression, and delivery of a recognized [SILENT] response when allow_silent=False.
There was a problem hiding this comment.
Thanks for the review. I have added the requested regression coverage.
- Create/update round trips —
TestAllowSilentFlagintests/tools/test_cronjob_tools.pycovers creating a job withallow_silent=False/True, the legacy default (True), and updating the flag both ways, including that leaving it unspecified on update does not clear it. - Legacy default suppression — covered by the create-default test.
[SILENT]delivery whenallow_silent=False— addedtest_silent_response_delivered_when_allow_silent_falsetoTestSilentDelivery. Writing that test surfaced a real bug: the authoritative (inner) delivery block inrun_one_jobrecomputedshould_deliverand suppressed[SILENT]unconditionally, ignoringallow_silent=False. I have aligned it with the outer block so it only suppresses whenjob.allow_silentdefaults to True. Full cron scheduler + cronjob tool suites pass (324 tests).
|
Thanks @alt-glitch for the triage note. #53252 is a different fix (briefing delivery requirement) — not a duplicate. This PR and #53252 address different aspects of the same issue (#53230) and can land independently. |
1ce0870 to
0bc861b
Compare
0bc861b to
e2d7f40
Compare
Mirror create_job()'s normalization in the update path: only a real bool
is stored; anything else (e.g. empty string from 'clear this field')
resets to the default True (backward compatible). bool("") wrongly
coerced a reset to False, so test_update_can_clear_allow_silent_to_default
failed and a user clearing the flag got silent suppression re-enabled
instead of restored to the default.
Fixes #53230
Description
Adds a per-job
allow_silentflag (defaultTruefor backward compatibility) that lets recurring briefing/report jobs opt out of the generic[SILENT]suppression behavior.When
allow_silent=False:[SILENT]suppression instruction is omitted from the cron prompt — replaced with guidance that the job must always deliver[SILENT]response from the agent is delivered anyway instead of being suppressedChanges
cron/scheduler.py:_build_job_prompt()conditionally includes the SILENT instruction based onallow_silent.run_one_job()skips silence detection whenallow_silent=False.cron/jobs.py:create_job()accepts and storesallow_silent.tools/cronjob_tools.py:cronjob()tool acceptsallow_silenton create/update, exposes it in_format_job(), and documents it in the schema.Verification
allow_silentdefault toTrue(same behavior as before)