fix(cron): preserve dashboard repeat counts - #68022
Conversation
|
Reviewed against issue #68012. This fully closes the issue as written.
Verifies against all four acceptance criteria (finite preserved, omit → default, invalid → 4xx, gateway API untouched). Note there's a competing PR #68025 that also adds the desktop |
b541549 to
2fd6e37
Compare
|
Rebased onto current upstream main to resolve the merge conflict. The PR now points to |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused regression coverage. The underlying dashboard endpoint defect remains on current main: CronJobCreate lacks repeat at hermes_cli/web_models.py:361, and _create_cron_job_sync() does not forward it at hermes_cli/web_server.py:12462.
Problems
- Current main commit
9179fb72eamovedCronJobCreateout ofhermes_cli/web_server.pyintohermes_cli/web_models.py. The PR’s schema hunk therefore targets the old location and will not change the active request model.
Suggested changes
- Transplant the
Fieldimport andrepeat: Optional[int] = Field(default=None, ge=1, strict=True)declaration intohermes_cli/web_models.py, while retaining the forwarding change inhermes_cli/web_server.pyand the focused endpoint tests.
The core approach matches the existing gateway API behavior in gateway/platforms/api_server.py:5276-5309. This is an automated hermes-sweeper review.
2fd6e37 to
a15b61b
Compare
a15b61b to
eeee395
Compare
SummaryTwo PRs address #68012 by adding the missing dashboard cron Related pull requests
Duplicates#68022 and #68025 substantially duplicate the request-model and Suggested consolidationKeep #68022 open with a salvage path: retain its active-model strict constraint, direct forwarding, and focused endpoint/storage tests, which address the contributor review's stated blocker. Close #68025 as a duplicate of #68022 despite its keep-open review because its visible core diff remains stale and less complete; preserve its one-line desktop TypeScript payload addition by splitting it into a focused follow-up or porting it during #68022 author work. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I68012(["issue #68012 (open)"])
subgraph Dup68022 ["PRs duplicating each other"]
P68022["PR #68022 (open)"]
P68025["PR #68025 (open)"]
end
P68022 -->|best fix| I68012
class I68012 open
class P68022 open
class P68025 open
class P68022 best
class P68022 target
click I68012 "https://github.com/NousResearch/hermes-agent/issues/68012"
click P68022 "https://github.com/NousResearch/hermes-agent/pull/68022"
click P68025 "https://github.com/NousResearch/hermes-agent/pull/68025"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 8 kB of PR diffs, 7 kB of issue/PR text, 7 kB of discussion (11 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
The dashboard
POST /api/cron/jobsendpoint accepted payloads containing a finiterepeatcount but silently stripped the field during Pydantic parsing. The resulting job used the core defaultrepeat=Noneand ran forever.This change declares
repeatas an optional strict positive integer and forwards it tocron.jobs.create_job(). Omitting the field keeps the existing unlimited behavior. Invalid values are rejected before a job is stored.Related Issue
Fixes #68012
Type of Change
Changes Made
hermes_cli/web_server.py: validate and forward finite dashboard cron repeat counts.tests/hermes_cli/test_web_server_cron_repeat.py: cover finite, omitted, non-positive, boolean, fractional, and string values; verify invalid requests create no job.How to Test
repeat: 2; verify the response contains{"times": 2, "completed": 0}.repeat; verifytimesremainsnull.repeatand no job is stored.The exact commit was also reviewed on the contributor fork by both an independent Codex pass and GitHub Codex; neither found an actionable issue.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (focused affected suites pass locally; full CI is left to Actions)Documentation & Housekeeping
cli-config.yaml.exampleupdate: N/A — no config key changedCONTRIBUTING.md/AGENTS.mdupdate: N/A — no architecture or workflow changedScreenshots / Logs
Not applicable; this is an HTTP request-contract fix with regression coverage.