Skip to content

fix(cron): let briefing jobs require delivery - #53252

Open
harjothkhara wants to merge 1 commit into
NousResearch:mainfrom
harjothkhara:fix/cron-allow-silent
Open

fix(cron): let briefing jobs require delivery#53252
harjothkhara wants to merge 1 commit into
NousResearch:mainfrom
harjothkhara:fix/cron-allow-silent

Conversation

@harjothkhara

@harjothkhara harjothkhara commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes cron jobs that are supposed to always deliver a report, such as daily briefings, inheriting the generic [SILENT] suppression instruction.

This adds an explicit per-job allow_silent flag. Existing jobs keep the current behavior by default. Jobs with allow_silent=False get an always-deliver prompt hint instead of the silent-suppression rule, and the delivery gate will not drop a final [SILENT] response for that job.

Related Issue

Fixes #53230

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Tests (adding or improving test coverage)

Changes Made

  • Added strict allow_silent storage/update support for cron jobs.
  • Made cron prompt construction and delivery suppression respect allow_silent=False.
  • Exposed the flag through the cronjob tool, CLI create/edit, skill-backed blueprints, dashboard cron API, and API-server cron jobs API.
  • Marked built-in daily/morning briefing, weekly review, and workday-start catalog paths as always-deliver.
  • Added regression coverage for scheduler behavior, job persistence, tool/API round trips, and blueprint/suggestion catalog paths.

How to Test

  1. scripts/run_tests.sh tests/cron/test_scheduler.py tests/cron/test_jobs.py tests/tools/test_cronjob_tools.py tests/cron/test_blueprint_catalog.py tests/cron/test_suggestions.py tests/tools/test_blueprints.py tests/gateway/test_api_server_jobs.py tests/hermes_cli/test_cron_parser_builder.py tests/hermes_cli/test_web_server_skill_editor.py

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run the repo test wrapper and all focused tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — no platform-specific primitives added
  • I've updated tool descriptions/schemas if I changed tool behavior

Screenshots / Logs

Duplicate-work checks performed before implementation:

Proof:

=== Summary: 9 files, 504 tests passed, 0 failed (100% complete) in 11.2s (16 workers) ===

Private pre-push review note: the normal push hook launched the configured background review. The Claude/Grogu side failed local auth (401 Invalid authentication credentials), but the Codex/Mario report completed and flagged strict boolean/API/CLI/catalog/internal-silence gaps; the latest commit addresses those findings.

2026-07-13 shepherding refresh

The issue premise still holds on current main: cron/scheduler.py::_build_job_prompt() still injects the generic [SILENT] instruction for every agent-driven cron job, and the delivery path still suppresses a recognized silence response without a per-job opt-out.

The open implementation cluster is now:

This comparison is review context rather than a claim that later-opening alternatives should be closed automatically. A maintainer still needs to choose the intended API shape and whether to land the minimal core surface or the end-to-end wiring.

@harjothkhara
harjothkhara force-pushed the fix/cron-allow-silent branch from 944b00f to 5f01a50 Compare June 26, 2026 20:22
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/tools Tool registry, model_tools, toolsets comp/gateway Gateway runner, session dispatch, delivery labels Jun 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Competing with #53248 for the same fix (both implement a per-job allow_silent flag fixing #53230). #53248 is the minimal 3-file core variant (cron/scheduler.py, cron/jobs.py, tools/cronjob_tools.py); this PR is the broader end-to-end variant wiring the flag through the cronjob tool, blueprints, the dashboard cron-create API, and the api_server cron jobs API. Not a duplicate -- flagging the cluster so a maintainer can choose minimal-fix vs full-surface.

@harjothkhara

Copy link
Copy Markdown
Contributor Author

Heads up on the overlap with #53248 — both add the per-job allow_silent flag for #53230. That one is the small core-only change (3 files, no tests). This one hooks the flag up everywhere jobs get created or edited (catalogs, API, CLI, web UI) and has tests. If the small one lands first, I can cut this down to just the extra wiring + tests on top of it.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the end-to-end cron delivery fix. The premise remains present on current main: cron/scheduler.py:2337-2340 injects [SILENT] guidance for every agent-driven job, and cron/scheduler.py:3631-3633 suppresses every recognized successful silence response.

The PR adds a persisted per-job opt-out, changes both scheduler decisions, and covers the relevant creation/update paths (tool, CLI, dashboard, API server, blueprints, and catalog entries). Its internal-silence exception preserves existing no-output/wake-gate behavior. GitHub currently reports the branch MERGEABLE/CLEAN with successful checks.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added type/feature New feature or request sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages and removed type/bug Something isn't working labels Jul 15, 2026
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@harjothkhara
harjothkhara force-pushed the fix/cron-allow-silent branch from a04bdca to 5f281f9 Compare July 24, 2026 05:35
@harjothkhara

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Rebased onto current main: _create_cron_job_sync gained an Optional[str] profile signature on main, so the allow_silent validation now sits on the new signature. Full tests/cron/ plus the touched gateway/tools/CLI suites pass locally (925 tests).

@alt-glitch alt-glitch added comp/cli CLI entry point, hermes_cli/, setup wizard and removed sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 24, 2026
@Fferreira58

Copy link
Copy Markdown

We hit exactly the prompt-conflict described in #53230 with briefing-style jobs, and have been carrying a small local patch since early June that does the prompt-hint half of this — a per-job boolean choosing between the [SILENT] rule and an always-deliver instruction. We converged independently on the same per-job-flag interface, which hopefully reads as validation that the design is right.

One design note from having lived with the prompt-only variant: it is not enough on its own. The generic hint is only an instruction — if the model emits [SILENT] anyway, the delivery gate still suppresses the run, so an always-deliver contract is not actually enforceable from the prompt side alone. Gating _cron_silence_allowed() inside run_one_job() as this PR does is what makes the flag trustworthy, and is the reason we are dropping our local patch in favor of this.

+1 to landing this (or #53248 first with this cut down to the extra wiring + tests on top, per the earlier comment) — the need is real for recurring briefings/reports.

@harjothkhara
harjothkhara force-pushed the fix/cron-allow-silent branch from ce40a2e to 9a80125 Compare August 7, 2026 18:42
@alt-glitch alt-glitch removed the needs-decision Awaiting maintainer decision before any implementation label Aug 7, 2026
@harjothkhara
harjothkhara force-pushed the fix/cron-allow-silent branch from 9a80125 to 68fa302 Compare August 19, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Generic cron [SILENT] prompt policy suppresses recurring briefing/report jobs that should always send an all-clear

4 participants