From 8d4b1e4b0e240af67c1f4eb6601aee99eac0bc49 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 10 Aug 2026 20:57:43 +0000 Subject: [PATCH] fix(cron): apply create-time origin resolution to the update path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught a real gap: action='update' also accepts deliver, and the tool description explicitly steers agents toward update-over-create — so a cron-context agent updating a job to deliver='origin' would recreate exactly the dangling literal-origin shape the create-path resolution prevents (stored 'origin' on an origin-less job → fire-time home-channel guessing or silent drop). Wrap the update site in the same resolver. Semantics follow the create precedent: in cron context, 'origin' means 'my run's target', resolved concretely at mutation time; outside cron context updates are byte-identical to before. --- tests/cron/test_cron_created_delivery.py | 34 ++++++++++++++++++++++++ tools/cronjob_tools.py | 4 ++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/cron/test_cron_created_delivery.py b/tests/cron/test_cron_created_delivery.py index 61d8c89c6f..8a71abae79 100644 --- a/tests/cron/test_cron_created_delivery.py +++ b/tests/cron/test_cron_created_delivery.py @@ -145,6 +145,40 @@ class TestCronContextDeliveryResolution: assert "origin" not in [p.strip() for p in stored.split(",")] +class TestCronContextUpdatePath: + def test_update_deliver_origin_resolves_in_cron_context(self, temp_cron_home): + """The update action must apply the same resolution as create — a + cron agent updating deliver='origin' would otherwise recreate the + dangling literal-origin shape on an origin-less job.""" + from tools.cronjob_tools import cronjob + from cron.jobs import get_job + + tokens, extra = _enter_cron_context("telegram", "-100123456", "17") + try: + created = _create(deliver="local") + result = json.loads( + cronjob(action="update", job_id=created["job_id"], deliver="origin") + ) + finally: + _exit_cron_context(tokens, extra) + assert result["success"] is True + job = get_job(created["job_id"]) + stored = str(job.get("deliver", "")) + assert "origin" not in [p.strip() for p in stored.split(",")] + assert stored == "telegram:-100123456:17" + + def test_update_deliver_outside_cron_context_unchanged(self, temp_cron_home): + from tools.cronjob_tools import cronjob + from cron.jobs import get_job + + created = _create(deliver="local") + result = json.loads( + cronjob(action="update", job_id=created["job_id"], deliver="origin") + ) + assert result["success"] is True + assert get_job(created["job_id"]).get("deliver") == "origin" + + class TestNonCronContextUnchanged: def test_chat_session_create_keeps_literal_origin(self, temp_cron_home): # No cron_session var — ordinary chat/CLI create. Existing semantics: diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 63ec696b64..19d6503165 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -1358,7 +1358,9 @@ def cronjob( if name is not None: updates["name"] = name if deliver is not None: - updates["deliver"] = _normalize_deliver_param(deliver) + updates["deliver"] = _resolve_cron_context_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