From 54418363aa5ed0d5ea3da33d82eb1aa10a5bc5e0 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Mon, 5 Oct 2026 22:30:44 -0500 Subject: [PATCH] fix(pm-agent): take the newest comment by list order, not by comparing timestamp strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit last_activity() is the whole trigger: the PM acts iff the newest thing in a thread was not written by it. It found that newest comment by comparing Gitea's RFC 3339 created_at STRINGS, which is only correct while every offset is identical — true today, silently wrong after a server timezone change, and the failure would be the PM answering its own last comment forever. The comments endpoint returns them in creation order, so the last element IS the newest. Using that removes the dependency entirely. Found by self-review, before anything ran. Co-Authored-By: Claude Opus 5 (1M context) --- decisions/2026-10-05-pm-agent-issue-to-pr.md | 6 ++++++ infra/agent/pm-dispatch.py | 21 +++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/decisions/2026-10-05-pm-agent-issue-to-pr.md b/decisions/2026-10-05-pm-agent-issue-to-pr.md index 18a673d..f07d9e0 100644 --- a/decisions/2026-10-05-pm-agent-issue-to-pr.md +++ b/decisions/2026-10-05-pm-agent-issue-to-pr.md @@ -90,6 +90,12 @@ All offline, in the pinned toolbox (`./infra/devcontainer/dev.sh run`), never on kustomization, because the kustomization's comment named it by filename. Correctly refused: the distance between a filename in a comment and a line under `resources:` is one careless edit. The comment now points at it in prose. +- **A self-review of the trigger found and fixed one real defect** before any of this ran: + `last_activity()` compared RFC 3339 timestamp STRINGS to find the newest comment, which is + only correct while every offset Gitea returns is identical — and the failure mode is the + worst available one, the PM answering its own last comment in a loop. It now takes the last + element of the comments list, which is the order the endpoint returns, so there is nothing + about timezones to be right about. - **`check-jsonschema --schemafile schemas/servicedefinition.schema.json catalog/gitea.yaml` → ok.** - **The golden reproduces:** `python3 catalog/render.py --all catalog -o apps --wave-layout diff --git a/infra/agent/pm-dispatch.py b/infra/agent/pm-dispatch.py index 5ef7e77..df16e6d 100755 --- a/infra/agent/pm-dispatch.py +++ b/infra/agent/pm-dispatch.py @@ -599,19 +599,22 @@ def render_thread(cfg: Config, issue: dict, comments, pr: dict, status: dict, def last_activity(issue: dict, comments) -> tuple: - """Who wrote the newest thing in this thread, and when. + """Who wrote the newest thing in this thread, and when. THE TRIGGER DEPENDS ON THIS. Deliberately NOT `issue.updated_at`: that moves when a label is added or the title is edited, which would re-trigger the PM for a change nobody asked it about. + + And deliberately NOT a comparison of the timestamp STRINGS. Gitea returns RFC 3339 with + a timezone offset, and lexicographic order over such strings is only correct while every + offset is identical — true today, silently wrong after a server timezone change, and the + failure would be the PM answering its own last comment in a loop. The comments endpoint + returns them in creation order, so the last element IS the newest; that is one less thing + to be right about. """ - actor = (issue.get("user") or {}).get("login") - when = issue.get("created_at") or "" - for c in comments: - ts = c.get("created_at") or "" - if ts >= when: - when = ts - actor = (c.get("user") or {}).get("login") - return actor, when + if comments: + newest = comments[-1] + return (newest.get("user") or {}).get("login"), newest.get("created_at") or "" + return (issue.get("user") or {}).get("login"), issue.get("created_at") or "" # --------------------------------------------------------------------------- job