Skip to content

fix: validate cron no-agent update invariants - #21031

Open
sgtworkman wants to merge 2 commits into
NousResearch:mainfrom
sgtworkman:fix/cron-no-agent-validation
Open

fix: validate cron no-agent update invariants#21031
sgtworkman wants to merge 2 commits into
NousResearch:mainfrom
sgtworkman:fix/cron-no-agent-validation

Conversation

@sgtworkman

@sgtworkman sgtworkman commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add regression coverage for cron no_agent update invariants.
  • Validate script-only cron behavior stays intact across update paths.
  • Refresh cron/kanban documentation for the script-only and no-agent semantics.

Test Plan

  • scripts/run_tests.sh tests/cron/test_cron_no_agent.py tests/cron/test_cron_script.py

Branch State

  • Rebases cleanly onto current origin/main.
  • Local branch verified 1 ahead / 0 behind against origin/main before opening this PR.

@sgtworkman
sgtworkman force-pushed the fix/cron-no-agent-validation branch from 2cdfa3e to d204b2c Compare May 7, 2026 04:53
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management type/docs Documentation improvements P2 Medium — degraded but workaround exists and removed type/docs Documentation improvements labels May 7, 2026
@sgtworkman

Copy link
Copy Markdown
Contributor Author

CI workflows are currently in GitHub action_required state for this fork PR, so no jobs/logs are available yet. Local validation before opening: scripts/run_tests.sh tests/cron -q → 323 passed; npm run lint:diagrams → 0 errors; npm run build completed successfully with existing unrelated broken-link warnings.

@sgtworkman
sgtworkman force-pushed the fix/cron-no-agent-validation branch from d204b2c to 81b72a2 Compare May 10, 2026 02:21
@sgtworkman

Copy link
Copy Markdown
Contributor Author

Validation refresh

Rebased this stale/conflicting PR onto latest origin/main, resolved conflicts, and pushed the refreshed branch.

Head: 81b72a267
Branch divergence after rebase: 0 behind / 1 ahead vs origin/main.

Local validation:

scripts/run_tests.sh tests/cron/test_cron_no_agent.py tests/cron/test_cron_script.py -v

Result: 60 passed.

Additional checks:

  • git diff --check: passed.
  • Static added-line security scan: 0 findings.
  • Independent final diff review: passed, no blocking security or logic issues.

Conflict resolution notes:

  • Kept the upstream cron script-only docs additions, including webhook-related references.
  • Kept/added coverage for empty script output skipping the agent prompt because that behavior now exists on origin/main.
  • Added an extra data-layer regression for whitespace-only script updates while no_agent=True.

GitHub Actions for this fork PR may still require maintainer approval before remote CI runs.

@sgtworkman

Copy link
Copy Markdown
Contributor Author

Validation refresh

Rebased onto latest origin/main after upstream advanced and force-pushed with lease.

Head: 51f138398
Branch divergence vs origin/main: 0 behind / 1 ahead.

Local validation:

scripts/run_tests.sh tests/hermes_cli/test_tools_config.py tests/cron/test_cron_no_agent.py tests/cron/test_cron_script.py -q

Result for shared validation slice: 125 passed.

This PR-specific cron coverage remains covered by that run: tests/cron/test_cron_no_agent.py and tests/cron/test_cron_script.py.

@sgtworkman
sgtworkman force-pushed the fix/cron-no-agent-validation branch from 51f1383 to 29e7891 Compare May 10, 2026 07:18
@sgtworkman

Copy link
Copy Markdown
Contributor Author

Validation refresh

Current head: 29e789178
Branch state before this comment: 1 ahead / 0 behind vs origin/main; 0 ahead / 0 behind vs sgtworkman/fix/cron-no-agent-validation.

Local validation rerun:

scripts/run_tests.sh tests/cron/test_cron_no_agent.py tests/cron/test_cron_script.py

Result: 60 passed in 1.80s.

PR body refreshed to match the current scope. GitHub Actions/check rollup is currently empty for this fork PR; if Actions require maintainer approval, no remote CI logs will be available until that approval happens.

@sgtworkman

Copy link
Copy Markdown
Contributor Author

Local integration validation

Validated the current open PR stack locally while GitHub Actions remain maintainer-gated for fork PRs.

Temporary stack from fresh origin/main (44cdf555a) merged these PR heads cleanly, with no conflicts:

Combined targeted validation:

scripts/run_tests.sh \
  tests/hermes_cli/test_tools_config.py \
  tests/hermes_cli/test_update_check.py \
  tests/hermes_cli/test_web_server.py \
  tests/cron/test_cron_no_agent.py \
  tests/cron/test_cron_script.py -q

Result: 280 passed in 3.04s.

Dashboard build also passed:

cd web && npm run build

Result: passed; only the existing Vite chunk-size warning was emitted.

The temp worktree was removed after validation. No production/deploy action was performed.

@sgtworkman
sgtworkman force-pushed the fix/cron-no-agent-validation branch 2 times, most recently from a6fe297 to 067bd42 Compare May 14, 2026 03:34

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for moving the no-agent script requirement into cron/jobs.py; current main still allows a direct update to clear that script and leave a job that the scheduler rejects at cron/scheduler.py:2523-2528.

Problems

  • cron/jobs.py:702 in this PR rejects a valid agent-mode job when it has a script but no prompt or skills. Current dashboard validation deliberately permits prompt or skills or script (hermes_cli/web_server.py:9998-10002), and the agent path injects non-empty script output into the prompt (cron/scheduler.py:2184-2203). The changed round-trip test at tests/cron/test_cron_no_agent.py:86 masks that supported transition by adding a prompt.

Suggested changes

  • Retain the no_agent=True/script invariant, but remove the no_agent=False prompt-or-skills check and test the script-only agent transition instead.

Automated hermes-sweeper review.

Comment thread cron/jobs.py Outdated
# Switching an existing script-only job back to agent mode must not
# create an empty LLM job. Existing invalid legacy rows are left alone
# unless the update explicitly toggles no_agent off.
if "no_agent" in updates and not updated.get("no_agent"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rejects a supported agent-mode shape: a non-no_agent job may retain its pre-run script as the sole prompt source. Current dashboard validation accepts prompt or skills or script (hermes_cli/web_server.py:9998), and _build_job_prompt() injects non-empty script output before the agent runs (cron/scheduler.py:2184-2203). Remove this branch; keep only the no_agent=True requires-script invariant.

@teknium1 teknium1 added 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 area/install-update Installer, updater, packaging, wheels, doctor labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Two PRs touch this complex: #21031 addresses the reported runtime invariant by normalizing script updates and rejecting script removal from no-agent jobs, while #21594 overlaps only in the cron and Kanban documentation diagrams and does not address the runtime cause. Both diffs currently require the corrections identified by contributor review.

Related pull requests

  • #21031 related — (+137/-31) — keep open, changes required: The data-layer normalization and rejection of empty or whitespace-only scripts correctly prevent updates from leaving no-agent jobs unschedulable. The contributor keep_open review must be addressed before merge: remove the new no_agent=False prompt-or-skills restriction because script-only agent jobs are supported, and restore a regression test for that transition.
  • #21594 related — (+19/-40) — keep open separately, changes required: This is primarily a fallback-provider and documentation-lint cleanup; its cron and Kanban Mermaid conversions overlap with #21031 but do not fix the cron update invariant. Per the contributor keep_open review, route Kanban live updates through the FastAPI WebSocket rather than directly from the DB to the UI, and preserve the legacy fallback_model compatibility note.

Duplicates

#21031 and #21594 contain substantially duplicate cron-script-only and Kanban ASCII-to-Mermaid documentation changes, but their overall scopes are not duplicates.

Suggested consolidation

Merge #21031 after removing the unsupported agent-mode prompt-or-skills validation and testing the valid script-only agent transition. Keep #21594 as the separate documentation PR after its contributor-review corrections, while retaining the overlapping diagram conversions in only one PR to avoid duplicate/conflicting edits; neither full PR should be closed as a duplicate of the other.

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 19 kB of PR diffs, 1 kB of issue/PR text, 5 kB of discussion (7 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@sgtworkman
sgtworkman force-pushed the fix/cron-no-agent-validation branch from 067bd42 to e719db7 Compare July 28, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cron Cron scheduler and job management 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants