Repository navigation
Add model selector to scheduled-task frontmatter (#107) - #108
Merged
Merged
Conversation
Scheduled tasks had no way to request a non-default model — every run used whatever the CLI defaults to. Parse an optional `model:` key the same way `tz`/`timeout` are already validated, rejecting unrecognized ids at parse time (so a typo surfaces in the skipped-file warning, not silently at run time). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Plumb ScheduledTask.model through to the Job the scheduler enqueues, so the worker has it available at invocation time. Jobs with no model set keep the existing default of None. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
invoke_claude_job() now accepts an optional model kwarg and, when set, appends ["--model", model] to the argv list passed to create_subprocess_exec — still a list element, never shell-interpolated, so a task-file-controlled value can't reach a shell. When absent, the command is byte-for-byte what it was before (no --model flag at all). process_job() wires job.model through from the queue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The adversarial review of this change flagged that process_job's new model=job.model kwarg had no test exercising it directly — the scheduler-to-Job link and the runner's --model-flag link were each covered, but the middle hop (worker passing job.model through) was not, so a future refactor could silently drop it. Add two tests covering both the set and absent cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Scheduled tasks can now request a specific model via an optional
model:frontmatter key, validated at parse time the same waycron/tz/timeoutalready are:src/clayde/scheduler/tasks.py:ScheduledTaskgains amodel: str | Nonefield. A new_parse_model()returnsNonewhen the key is absent, and raisesValueError(f"bad model {value!r}")for anything not in the newKNOWN_MODELSallowlist (seeded with the current- and prior-generation model ids already referenced in this codebase; carries the same hand-maintained-allowlist caveat as_MODEL_PRICINGinclaude.py).src/clayde/service/queue.py:Jobgainsmodel: str | None = None.src/clayde/scheduler/loop.py:run_tick()passesmodel=task.modelinto the enqueuedJob.src/clayde/service/runner.py:invoke_claude_job()takes an optionalmodelkwarg and, only when set, appends"--model", modelas list elements to thecmdargv passed toasyncio.create_subprocess_exec— never shell-interpolated, so the task-file-controlled value can't reach a shell.src/clayde/service/worker.py:process_job()forwardsmodel=job.model.When
modelis absent, behavior is byte-for-byte unchanged from before this PR (no--modelflag added at all) — existing scheduled tasks are unaffected.Out of scope (per the issue, being handled separately in the
knowledge_basevault repo): thescheduled-taskskill's frontmatter docs, the design doc, andcheck-task.sh. Also untouched:claude.py'sApiBackend/CliBackend— a separate invocation path used by the issue-working flow, not the scheduler.Closes #107
Testing
Added tests mirroring the existing
timeouttest style:tests/scheduler/test_tasks.py: model absent →None, known id accepted, unrecognized id rejected (and skipped bydiscover_tasksthe same way a malformed timeout is).tests/scheduler/test_loop.py:run_tick()carriestask.modelinto the enqueuedJob, and leaves itNonewhen absent.tests/service/test_runner.py:--model <id>appears in the built CLI command whenmodelis set, and is absent entirely when it isn't.tests/service/test_worker.py:process_job()forwardsjob.modelintoinvoke_claude_job(), both when set and when absent.uv run pytest— 428 passed.Review panel
Dispatched one adversarial-reviewer subagent against
git diff origin/main...HEADplus the issue text (not this PR's own commit messages or reasoning), instructed to try to refute the change rather than approve it. It traced every call site ofJob(...),ScheduledTask(...), andinvoke_claude_job(...)across the repo, confirmed the subprocess argv-list safety (no shell interpolation), confirmed backward compatibility (all construction sites use keyword args,modeldefaults toNone), and confirmed the Pebble webhook path can't smuggle an unvalidatedmodelvalue through (nomodelfield onPebblePayload).It raised one non-blocking finding: the worker→runner
modelforwarding (process_jobpassingmodel=job.model) had no direct test — the scheduler→Job and runner→--model-flag links were each covered, but not that middle hop. Fixed by addingtest_process_job_forwards_model_to_invokeandtest_process_job_forwards_none_model_when_absenttotests/service/test_worker.py; re-ranuv run pytestgreen (428 passed) after the fix.No blocking findings.
🤖 Generated with Claude Code