Skip to content

Fix mutable defaults and caller-side list mutation in task scheduler factories#1516

Open
sena-labs wants to merge 1 commit intoagent0ai:mainfrom
sena-labs:audit/mutable-defaults
Open

Fix mutable defaults and caller-side list mutation in task scheduler factories#1516
sena-labs wants to merge 1 commit intoagent0ai:mainfrom
sena-labs:audit/mutable-defaults

Conversation

@sena-labs
Copy link
Copy Markdown
Contributor

PR description draft: Fix mutable defaults in helpers.task_scheduler

Suggested 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 normalized todo / done in 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 / done arrays instead of working on a copy.

What changed

  • added _normalize_utc_datetimes(...) to centralize safe datetime normalization;
  • changed factory signatures from mutable list defaults to None;
  • allocated fresh lists inside task factory methods;
  • copied attachments before storing them on task instances;
  • copied and normalized todo / done before constructing TaskPlan.

Why this is safe

This is a narrow, low-risk fix:

  • no public feature expansion;
  • no persistence or schema changes;
  • no persistence format change;
  • no scheduler semantics change beyond eliminating unintended aliasing / mutation.

Tests

Added tests/test_task_scheduler_mutable_defaults.py covering:

  • fresh attachment lists for AdHocTask, ScheduledTask, and PlannedTask;
  • defensive copying of caller-provided attachments;
  • fresh default todo / done lists in TaskPlan.create();
  • defensive copying and UTC localization of caller-provided datetime lists.

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 -q
  • Result: 8 passed

Reviewer 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.

Copilot AI review requested due to automatic review settings April 14, 2026 13:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 in TaskPlan.create(...) to avoid in-place normalization of caller lists.
  • Updated task factory methods to use attachments: list[str] | None = None and 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants