Cluster of 3 items

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #17456 fix(cron): accept list-form deliver so deliver=['telegram'] works

  merged to main

  files: cron/scheduler.py, tests/cron/test_scheduler.py, tests/tools/test_cronjob_tools.py, tools/cronjob_tools.py

  Show the diff

  • cron/scheduler.py

    @@ -233,12 +233,32 @@ def _resolve_single_delivery_target(job: dict, deliver_value: str) -> Optional[d
         }
     
     
    +def _normalize_deliver_value(deliver) -> str:
    +    """Normalize a stored/submitted ``deliver`` value to its canonical string form.
    +
    +    The contract is that ``deliver`` is a string (``"local"``, ``"origin"``,
    +    ``"telegram"``, ``"telegram:-1001:17"``, or comma-separated combinations).
    +    Historically some callers — MCP clients passing an array, direct edits of
    +    ``jobs.json``, or stale code paths — have stored a list/tuple like
    +    ``["telegram"]``.  ``str(["telegram"])`` would serialize to the literal
    +    string ``"['telegram']"``, which is not a known platform and fails
    +    resolution silently.  Flatten lists/tuples into a comma-separated string
    +    so both forms work.  Returns ``"local"`` for anything falsy.
    +    """
    +    if deliver is None or deliver == "":
    +        return "local"
    +    if isinstance(deliver, (list, tuple)):
    +        parts = [str(p).strip() for p in deliver if str(p).strip()]
    +        return ",".join(parts) if parts else "local"
    +    return str(deliver)
    +
    +
     def _resolve_delivery_targets(job: dict) -> List[dict]:
         """Resolve all concrete auto-delivery targets for a cron job (supports comma-separated deliver)."""
    -    deliver = job.get("deliver", "local")
    +    deliver = _normalize_deliver_value(job.get("deliver", "local"))
         if deliver == "local":
             return []
    -    parts = [p.strip() for p in str(deliver).split(",") if p.strip()]
    +    parts = [p.strip() for p in deliver.split(",") if p.strip()]
         seen = set()
         targets = []
         for part in parts:

  • tests/cron/test_scheduler.py

    @@ -279,6 +279,44 @@ def test_explicit_discord_channel_without_thread(self):
                 "thread_id": None,
             }
     
    +    def test_list_form_deliver_is_normalized(self, monkeypatch):
    +        """deliver=['telegram'] (Python list) should resolve like 'telegram' string.
    +
    +        Regression test for #17139: MCP clients / scripts that pass the deliver
    +        field as an array-shaped value used to fail with "no delivery target
    +        resolved for deliver=['telegram']" because ``str(['telegram'])`` was
    +        passed through to ``split(',')`` verbatim.
    +        """
    +        monkeypatch.setenv("TELEGRAM_HOME_CHANNEL", "-4004")
    +        job = {
    +            "deliver": ["telegram"],
    +            "origin": None,
    +        }
    +
    +        assert _resolve_delivery_target(job) == {
    +            "platform": "telegram",
    +            "chat_id": "-4004",
    +            "thread_id": None,
    +        }
    +
    +    def test_list_form_multiple_platforms_normalized(self, monkeypatch):
    +        """deliver=['telegram', 'discord'] resolves to multiple targets."""
    +        from cron.scheduler import _resolve_delivery_targets
    +
    +        monkeypatch.setenv("TELEGRAM_HOME_CHANNEL", "-111")
    +        monkeypatch.setenv("DISCORD_HOME_CHANNEL", "-222")
    +        job = {"deliver": ["telegram", "discord"], "origin": None}
    +
    +        targets = _resolve_delivery_targets(job)
    +        platforms = sorted(t["platform"] for t in targets)
    +        assert platforms == ["discord", "telegram"]
    +
    +    def test_empty_list_form_deliver_resolves_to_local(self):
    +        """deliver=[] is treated as local (no delivery)."""
    +        from cron.scheduler import _resolve_delivery_targets
    +
    +        assert _resolve_delivery_targets({"deliver": []}) == []
    +
     
     class TestDeliverResultWrapping:
         """Verify that cron deliveries are wrapped with header/footer and no longer mirrored."""

  • tests/tools/test_cronjob_tools.py

    @@ -231,3 +231,60 @@ def test_update_can_clear_skills(self):
             assert updated["success"] is True
             assert updated["job"]["skills"] == []
             assert updated["job"]["skill"] is None
    +
    +    def test_create_normalizes_list_form_deliver(self):
    +        """deliver=['telegram'] (list) is stored as the string 'telegram'.
    +
    +        Regression for #17139: MCP clients / scripts sometimes pass ``deliver``
    +        as an array.  Prior to the fix, ``['telegram']`` was written verbatim
    +        to ``jobs.json`` and the scheduler then tried to resolve the literal
    +        string ``"['telegram']"`` as a platform, failing with
    +        "no delivery target resolved".
    +        """
    +        from cron.jobs import get_job
    +
    +        created = json.loads(
    +            cronjob(
    +                action="create",
    +                prompt="Daily briefing",
    +                schedule="every 1h",
    +                deliver=["telegram"],
    +            )
    +        )
    +        assert created["success"] is True
    +        stored = get_job(created["job_id"])
    +        assert stored["deliver"] == "telegram"
    +
    +    def test_create_normalizes_multi_element_list_deliver(self):
    +        """deliver=['telegram', 'discord'] is stored as 'telegram,discord'."""
    +        from cron.jobs import get_job
    +
    +        created = json.loads(
    +            cronjob(
    +                action="create",
    +                prompt="Daily briefing",
    +                schedule="every 1h",
    +                deliver=["telegram", "discord"],
    +            )
    +        )
    +        assert created["success"] is True
    +        stored = get_job(created["job_id"])
    +        assert stored["deliver"] == "telegram,discord"
    +
    +    def test_update_normalizes_list_form_deliver(self):
    +        """update with deliver=['telegram'] stores the canonical string."""
    +        from cron.jobs import get_job
    +
    +        created = json.loads(
    +            cronjob(action="create", prompt="x", schedule="every 1h")
    +        )
    +        updated = json.loads(
    +            cronjob(
    +                action="update",
    +                job_id=created["job_id"],
    +                deliver=["telegram"],
    +            )
    +        )
    +        assert updated["success"] is True
    +        stored = get_job(created["job_id"])
    +        assert stored["deliver"] == "telegram"

  • tools/cronjob_tools.py

    @@ -150,6 +150,27 @@ def _normalize_optional_job_value(value: Optional[Any], *, strip_trailing_slash:
         return text or None
     
     
    +def _normalize_deliver_param(value: Any) -> Optional[str]:
    +    """Normalize a user-supplied ``deliver`` value to the canonical string form.
    +
    +    The cron schema documents ``deliver`` as a string (``"local"``, ``"origin"``,
    +    ``"telegram"``, ``"telegram:chat_id[:thread_id]"``, or comma-separated combos).
    +    Some callers — MCP clients passing arrays, scripts building the payload as a
    +    list — supply ``["telegram"]``.  ``create_job``/``update_job`` store it as-is,
    +    and the scheduler's ``str(deliver).split(",")`` then serializes the list to
    +    the literal ``"['telegram']"`` which is not a known platform.  Flatten lists
    +    / tuples at the API boundary so storage is always a string.  Returns ``None``
    +    for ``None``/empty so callers can treat it as "not supplied".
    +    """
    +    if value is None:
    +        return None
    +    if isinstance(value, (list, tuple)):
    +        parts = [str(p).strip() for p in value if str(p).strip()]
    +        return ",".join(parts) if parts else None
    +    text = str(value).strip()
    +    return text or None
    +
    +
     def _validate_cron_script_path(script: Optional[str]) -> Optional[str]:
         """Validate a cron job script path at the API boundary.
     
    @@ -283,7 +304,7 @@ def cronjob(
                     schedule=schedule,
                     name=name,
                     repeat=repeat,
    -                deliver=deliver,
    +                deliver=_normalize_deliver_param(deliver),
                     origin=_origin_from_env(),
                     skills=canonical_skills,
                     model=_normalize_optional_job_value(model),
    @@ -364,7 +385,7 @@ def cronjob(
                 if name is not None:
                     updates["name"] = name
                 if deliver is not None:
    -                updates["deliver"] = deliver
    +                updates["deliver"] = _normalize_deliver_param(deliver)
                 if skills is not None or skill is not None:
                     canonical_skills = _canonical_skills(skill, skills)
                     updates["skills"] = canonical_skills