fix(cron): apply create-time origin resolution to the update path too
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.
This commit is contained in:
committed by
Teknium
parent
68d8b2d4de
commit
8d4b1e4b0e
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user