Repository navigation
feat(status-ack): policy and unanswered-tasks probe (#5853 PR 2) - #5862
Conversation
src/status_ack_policy.py decides, for one task, whether its single initial status ack is due, which fixed factual template it uses, and where it goes: - deadline fixed from the earliest intake stamp (timestamp header, id epoch, observed mtime/first-seen), never from activity or start times; - a claimed task still running unanswered past the deadline is covered; - destination is the task's own verified room and thread (in-thread keeps its root, top-level is rooted at the source message); anything unverifiable suppresses, with no fallback; - suppressed on any result, terminal phase, WAITING, cancel, an existing ack claim (ack:<task>:initial), or any unrecognised state. scripts/unanswered-tasks.py --json reports per-task and overall oldest unclaimed age from intake time and whether only a standby holds the inbox. Text mode and exit codes are unchanged. No runner, scheduling or sending: those are PR 3/4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
scripts/unanswered-tasks.pyLines 122-131 122 def _inbox_view(inbox: Path, holders=None) -> dict:
123 """Who holds the inbox, classified by status_ack_policy.inbox_state so probe and policy agree."""
124 import status_ack_policy
125 if holders is None:
! 126 import watcher_identity
! 127 holders = watcher_identity.inbox_holders(str(inbox))
128 observed, pairs, undecided = holders.observed, list(holders.holders), holders.undecided
129 state = status_ack_policy.inbox_state(observed, [role for _, role in pairs], undecided)
130 return {"path": str(inbox), "observed": observed, "undecided": undecided, "state": state,
131 "holders": [{"pid": pid, "role": role} for pid, role in pairs],Lines 145-153 145 out = []
146 for f in sorted(tasks.glob("task-*.txt")) if tasks.is_dir() else []:
147 task_id = task_id_for(f, accept=valid_archive_lookup_id)
148 if task_id is None:
! 149 continue
150 mtime = f.stat().st_mtime
151 headers = parse_task_headers_lenient(f.read_text(errors="replace")).headers
152 intake = status_ack_policy.intake_at(task_id, headers)
153 phase, holder = _phase(workspace, task_id), _holder(workspace, task_id)Lines 189-197 189 print(json.dumps({"error": f"cannot import the probe's policy owner ({exc})"}))
190 return 2
191 except SystemExit as exc:
192 if exc.code != 2:
! 193 raise
194 print(json.dumps({"error": "cannot import src/result_markers.py; refusing to answer"}))
195 return 2
196 except OSError as exc:
197 print(json.dumps({"error": f"cannot read deliveries/ ({exc})"}))src/status_ack_policy.pyLines 68-79 68 if text.endswith("Z"):
69 text = text[:-1] + "+00:00"
70 try:
71 dt = datetime.fromisoformat(text)
! 72 except ValueError:
! 73 return None
74 if dt.tzinfo is None:
! 75 dt = dt.replace(tzinfo=timezone.utc)
76 return _plausible(dt.timestamp())
77
78
79 def _id_epoch(task_id: str) -> Optional[float]: |
…ly, hand-off wording - unanswered-tasks --json resolves each file through task_archive.task_id_for, so a .claimed-core-N or .assigned-<inst> file finds its activity, result and worker under the canonical id and leaves the oldest-unclaimed figure. - intake_at takes only immutable anchors (timestamp header, id epoch, a persisted first-seen); a file mtime is never one. A slug id with neither is reported intake-unknown and the policy suppresses, never treats it as fresh. - RUNNING is hand-off, not engagement: the line now says the task was handed off with no answer yet, and fresh activity changes neither text nor deadline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…arkers is unimportable The JSON path loaded the marker grammar only inside the task loop, so an empty queue with a broken dependency printed count 0 and exited 0. It now resolves the grammar owner before enumerating, as text mode does, and exits 2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
qingyun-wu
left a comment
There was a problem hiding this comment.
Review at d55a22d — agent-assisted review by Mars, Qingyun's Sutando seat. I'll approve once the item below is fixed.
I read status_ack_policy.py in full and snapshot() in the probe, and ran status-ack-policy (82/82), unanswered-tasks-json (40/40) and unanswered-tasks (39/39) at this head. Routing, fail-closed suppression and the factual templates all look right.
Should-fix: _id_epoch reads a timestamp out of random hex task ids. _ID_EPOCH takes any 10- or 13-digit run with no digit on either side. AG2 Space ids are task- plus 18 random hex chars, and a digit run like that turns up in them by chance:
- Across the 23,587 real
task-<18 hex>ids in this host's workspace, 107 produce an epoch, about 0.45%; 200k synthetic ids give 0.44%. - 16 of those epochs land before now, e.g.
task-dd3a13e1514613581a→ 1514613581 (Dec 2017) andtask-e99121be1559237837→ 2019.intake_attakes the min, so that task's intake is 2017.decidethen speaks the moment it is observed, not at intake + 120 s, and the probe reportsage_s/oldest_unclaimed_age_sof about 7 years. That last part is what an alarm built on the probe in PR 3 would trip on. - When there is no
timestampheader, a later spurious epoch moves intake into the future (task-2084056842a242eec2→ 2036), so the task WAITs for years and gets no ack. That breaks the rule that no anchor means unknown and suppress.
Suggested fix: only read an epoch from id shapes that actually carry one, anchored on the whole id (e.g. ^task-(\d{10}|\d{13})(?:[-_].*)?$, or whatever the epoch-bearing sources emit), and never from inside task-<hex>. Please add regression rows using task-dd3a13e1514613581a (with a fresh header, no early speak) and task-2084056842a242eec2 (header-less, intake unknown).
…, never a digit run inside a hex id Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@cla-assistant check |
liususan091219
left a comment
There was a problem hiding this comment.
Code verified at 3bd9003. Not approving yet: there is no approval from another reviewer at this head.
What is missing: Qingyun's re-review at 3bd9003. Her review at d55a22d was a COMMENT that said she would approve once the _id_epoch item was fixed. This commit fixes it. CI was still running when this was written (18 passed, 14 queued or in progress). license/cla is success.
Evidence for the fix (src/status_ack_policy.py:50-51, :80). I called the real intake_at and decide:
d55a22da task-dd3a13e1514613581a (5 s-old timestamp) intake=2017-12-30 -> speak at once
task-2084056842a242eec2 (no timestamp) intake=2036-01-16 -> wait
3bd90039 task-dd3a13e1514613581a (5 s-old timestamp) intake=header -> wait
task-2084056842a242eec2 (no timestamp) intake=None -> suppress
random task-<18 hex>: 983/200,000 -> 0/200,000 yield an epoch; 18-digit ids: 0/200,000
minted shapes still parse: task-<ms>, task-chat-<ms|s>, task-<ms>-<hex>, task-<ms>.claimed-core-N, task-<slug>-<ms>
Mutation: restoring the d55a22d _ID_EPOCH makes the new status-ack-policy row fail (83 passed, 1 failed). At this head: status-ack-policy 84/84, unanswered-tasks-json 40/40, unanswered-tasks 39/39 (env -i).
Non-blocking: the new rows check intake_at only. A decide row for task-dd3a13e1514613581a with a fresh header (it must WAIT, not speak) would pin the user-visible half of the bug.
— Sutando (susans-MacBook-Pro)
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved at 3bd9003 — agent-assisted review by Mars, Qingyun's Sutando seat, on Qingyun's instruction to review and approve Chi's PRs.
My should-fix is resolved. _ID_EPOCH is now anchored to task-(<alpha slug>-)*<10|13 digits> followed by -, . or end, and applied with match. I re-ran my check on this host's ids: 0 of 23,602 real task-<hex> ids yield an epoch (it was 107). Minted task-<ms> / task-<slug>-<ms> names still do. Both regression rows are in. status-ack-policy 84/84, unanswered-tasks-json 40/40 and unanswered-tasks 39/39 pass at this head.
Non-blocking: ids whose slug has digits in it (e.g. task-407fixed-1785827670) now come out intake-unknown and get no ack. That fails closed, which is the right direction; the PR 3 runner's persisted first-seen stamp covers it.
Part of #5853: PR 2 of the plan in the "Implementation path for the status layer" comment, with the later refinements and corrections applied. Stacked on PR 1, #5857, which is merged.
What changed, and why
src/status_ack_policy.py(new, pure, imports onlytask_priority): for one task,decide(observation, now)returnsspeak/wait/suppress, the ack keyack:<task>:initial, the template and text, the destination, anddue_at. The caller supplies everything it observed: headers, activity phase, result state, ack-claim state, worker holder and inbox state. The module never reads the clock, files orps.intake_atis the minimum of the immutable anchors only: thetimestampheader, the epoch in the task id, and a first-seen stamp the caller has persisted. A file mtime is never an anchor. With none of these (for example, a slug id with no timestamp), intake is unknown and the policy suppresses; it never treats the task as fresh.due_at = intake + first_ack_deadline[priority]. Defaults: urgent 30 s, normal 120 s, low 600 s, overridable by the caller (PR 3 puts this in the manifest config).started_atandlast_activity_atare carried but never used for the deadline.thread_root. A top-level request is rooted atsource_message_id. A missing or malformed room, rooms that disagree, a non-$source message, or a malformed or emptythread_rootall suppress. There is no fallback room, DM or top-level post.CANCEL_INSTRUCTIONtask, a cancel request, an existing ack claim (claimed / confirmed / parked unknown), and any unrecognised state. It fails closed.activity_busmarks RUNNING at hand-off. Activity freshness changes neither the wording nor the deadline. The watcher-down template says the owner was told only when the caller reports that it did.inbox_state(observed, roles, undecided)is the one classifier forsession/standby_only/empty/unknown. The probe uses it too.scripts/unanswered-tasks.py --json(with optional--inbox): prints one object. Each file is resolved to its canonical id throughtask_archive.task_id_for, so a.claimed-core-Nor.assigned-<inst>file is looked up (activity, result, worker) under that id. Per task:task_id,file,intake_at,intake_known,age_s(from intake),mtime_age_s,phase,holder,answered,claimed,unclaimed_age_s,unanswered. Overall:oldest_unclaimed_age_sand_task,unclaimed_intake_unknown(unclaimed tasks with no immutable anchor, kept out of the age figure),inbox(holders, state, undecided) andstandby_only(true / false / null when unobservable). Text mode output and exit codes 0 / 1 / 2 are unchanged;--jsonuses the same codes. A failed import of the policy owner exits 2, never 1.docs/src-map.mdregenerated.Look first at
src/status_ack_policy.pydecide()anddestination(), thensnapshot()in the probe.Requirement → test (each with a control)
Every policy row is a predicate that must pass for the real
decideand fail for a deliberately broken variant (theCONTROL:lines). All controls fail as intended.status-ack-policy: a work event every minute still acks at the deadline; one uninterrupted call acks at the deadline; a later mtime, first-seen or forged-latertimestampnever moves intakelast_activity_at; anchored onstarted_at; intake = newest stampstatus-ack-policy: urgent < normal < low; voice → urgent, team channel → low, header wins, config override; no ack before the deadline; ack by deadline + one poll at any phase offsetstatus-ack-policy:claimed_running_long(RUNNING, 900 s, no result → speakshanded_off); worker-held task is coveredstatus-ack-policy: the RUNNING line and deadline are identical with or without fresh activity; the line never claims work is under waystatus-ack-policy: a slug id with no timestamp is unknown, a persisted first-seen anchors it, unknown suppresses.unanswered-tasks-json: atask-plain.txtwith no timestamp isintake_known: falsewith no age, before and after a rewriteunanswered-tasks-json:.claimed-core-2and.assigned-inst-a, each with the id declared and with it parsed from the filename: RUNNING activity, the result and the worker are found under the canonical id, and the task is excluded from oldest-unclaimedPath.stemlookup misses the RUNNING activitystatus-ack-policy: an in-thread request keeps its root; a top-level request is rooted at its source message; a bad or empty root sends nothing; no room, a bad room, disagreeing rooms, or a non-event source sends nothingthread_root; unthreaded top-level; fallback destinationstatus-ack-policy: 13 shapes, plus an answer landing after a WAITstatus-ack-policy: keyack:<task>:initial; one speak over an hour of 15 s polling; claimed / confirmed / unknown never repeat across 12 five-minute ticksstatus-ack-policy: every spoken line equals its template; all templates reachable; no digits or estimate wordsstatus-ack-policy: only a standby →inbox_down+watcher_down; claimed is not reported unread;inbox_statecovers standby, untagged, session, empty, unobserved and undecidedunanswered-tasks-json: a task rewritten 30 s ago still reports 600 s; oldest across tasks; RUNNING and worker-held are claimed, QUEUED is notunanswered-tasks-json: the oldest age is unchanged after another task is answeredunanswered-tasks-json: true for a lone untagged or tagged standby; false with a session watcher or nobody; null when unobserved or undecidedunanswered-tasks-json: an empty queue withresult_markers.pyunimportable exits 2 with an error objectunanswered-tasks(existing, 39/39);unanswered-tasks-json:--jsonrc equals text rc for 1 and 0; rc 2 on unreadable holders and on an unimportable policyorigin/mainon the same fixtureEvidence
Controls at the first head, 3ed6d9e, with the round-2 tests:
status-ack-policy77 passed, 5 failed (the hand-off rows);unanswered-tasks-json24 passed, 14 failed (the 12 canonical-id rows and the 2 slug-intake rows). At f6db41b with the round-3 test:unanswered-tasks-json39 passed, 1 failed (the empty-queue refusal). At the current head both pass in full, 82/82 and 40/40.All three also pass under Python 3.9.6.
check-python39-compat.py,ruff,review-checks.sh(PASS) andlint-workspace-resolution.sh --diffare clean, andactivity-bus,task-priorityandsrc-maptests pass.Before (at
origin/main): neither test file can run, because there is nostatus_ack_policymodule and nosnapshot()or--json. Text-mode output of the old and new script on the same fixture:Left for PR 3 / 4
skills/status-ack/runner: gathering observations,first_ack_deadline_sin the manifest config, the arbitration claim shared with the core's notify, revalidation just before publish, delivery through the proactive outbox lane, the owner alert and remedy when the watcher is down, and thestatus-ack-arbitration,status-ack-suppression(publish-time) andstatus-ack-watcher-downe2e tests.$event id and no Matrix room, so the policy suppresses them as unverifiable. They get no status ack at all, rather than one sent anywhere else. Covering them needs verified per-source routing, which is a later, separate decision.Path.stemlookup (its output and exit codes are pinned unchanged). For a renamed claimed or assigned file it can therefore still disagree with--json; fixing that is a separate change.Other questions
docs/src-map.md.🤖 Generated with Claude Code