Skip to content

fix(cron): enforce schedule in required schema properties to prevent validation failure - #37431

Closed
AgatheBauer wants to merge 0 commit into
NousResearch:mainfrom
AgatheBauer:fix/cronjob-schema-validation
Closed

fix(cron): enforce schedule in required schema properties to prevent validation failure#37431
AgatheBauer wants to merge 0 commit into
NousResearch:mainfrom
AgatheBauer:fix/cronjob-schema-validation

Conversation

@AgatheBauer

Copy link
Copy Markdown

This PR promotes schedule to the root-level required array in CRONJOB_SCHEMA in tools/cronjob_tools.py and updates tests/cron/test_cronjob_schema.py accordingly.

This prevents description-driven models (e.g. Grok) from omitting the required schedule argument during action=create calls, fixing the schedule is required for create validation error.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists labels Jun 2, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Adding "schedule" to the top-level required[] array breaks all non-create actions.

On main, the schema has "required": ["action"]. This PR changes it to "required": ["action", "schedule"]. The schedule parameter is only meaningful for action=create — calling cronjob(action="list"), action="remove", action="pause", etc. does not need or use a schedule value. Making it required means JSON schema validation will reject those calls if the caller omits schedule.

The test's own docstring still says: "The schema intentionally does NOT promote schedule/prompt into the top-level required array because they're only mandatory for action=create, not for list/remove/pause/etc." — but the assertion was updated to expect the broken behavior without updating the docstring.

The description-driven approach (flagging schedule as REQUIRED in the action description text) is the correct pattern here — it already exists on main and works for description-driven models like Grok. The required[] array should stay ["action"].

Additionally, the entire diff is CRLF line-ending noise (~88KB for a one-line semantic change). This suggests the file was edited on Windows without .gitattributes normalization. If the intent is to enforce schedule for create only, a targeted description update would be a ~50-byte diff instead.

@AgatheBauer AgatheBauer closed this Jun 2, 2026
@AgatheBauer
AgatheBauer force-pushed the fix/cronjob-schema-validation branch from ef53711 to 04bb74c Compare June 2, 2026 15:17
@AgatheBauer

Copy link
Copy Markdown
Author

Thank you for the feedback! You are completely right. Promoting schedule to the root-level required array would indeed break all non-create actions like list, remove, pause, etc. by rejecting them if the caller omits schedule. We also apologize for the CRLF line-ending noise. Since the description-driven approach on main is the correct design, we will close this PR and handle the model validation issue via system prompt instructions. Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants