Fix mutable defaults and caller-side list mutation in task scheduler factories#1516
Open
sena-labs wants to merge 1 commit intoagent0ai:mainfrom
Open
Fix mutable defaults and caller-side list mutation in task scheduler factories#1516sena-labs wants to merge 1 commit intoagent0ai:mainfrom
sena-labs wants to merge 1 commit intoagent0ai:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR eliminates shared mutable state and caller-owned list mutation in the task scheduler model factories by replacing mutable default arguments with None, defensively copying list inputs, and centralizing safe UTC localization for datetime lists.
Changes:
- Added
_normalize_utc_datetimes(...)and used it inTaskPlan.create(...)to avoid in-place normalization of caller lists. - Updated task factory methods to use
attachments: list[str] | None = Noneand to always store a fresh copied list on the model. - Added regression tests validating fresh defaults and defensive copying behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
helpers/task_scheduler.py |
Removes mutable default args, adds safe datetime normalization helper, and ensures attachments are copied on creation. |
tests/test_task_scheduler_mutable_defaults.py |
Adds isolated regression tests to prevent reintroducing shared defaults or caller list mutation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
PR description draft: Fix mutable defaults in
helpers.task_schedulerSuggested title
Fix mutable defaults and caller-side list mutation in task scheduler factories
Summary
This change removes mutable list defaults from scheduler factory methods and prevents
TaskPlan.create(...)from mutating caller-owned datetime lists during UTC normalization.Problem
The scheduler model factories used
list()directly in function signatures:TaskPlan.create(...)AdHocTask.create(...)ScheduledTask.create(...)PlannedTask.create(...)That pattern risks shared state across independent calls.
TaskPlan.create(...)also normalizedtodo/donein place, which mutated lists supplied by the caller.Root cause
Python default arguments are evaluated once, so mutable defaults can be reused across invocations. On top of that, the existing datetime normalization logic modified the original
todo/donearrays instead of working on a copy.What changed
_normalize_utc_datetimes(...)to centralize safe datetime normalization;None;attachmentsbefore storing them on task instances;todo/donebefore constructingTaskPlan.Why this is safe
This is a narrow, low-risk fix:
Tests
Added
tests/test_task_scheduler_mutable_defaults.pycovering:AdHocTask,ScheduledTask, andPlannedTask;attachments;todo/donelists inTaskPlan.create();The tests are intentionally isolated via direct module loading and stubs so they validate the regression without depending on the full Agent Zero runtime bootstrap.
Validation
Run locally in clean worktree:
pytest tests/test_task_scheduler_mutable_defaults.py -q8 passedReviewer notes
The key behavior change to review is that scheduler factories no longer reuse default list objects and no longer mutate external lists during model creation.
This PR does not change task persistence shape, HTTP/API behavior, or scheduler feature scope.