diff --git a/skills/unleash/SKILL.md b/skills/unleash/SKILL.md index e144720..2fdba90 100644 --- a/skills/unleash/SKILL.md +++ b/skills/unleash/SKILL.md @@ -75,6 +75,10 @@ An `execute` envelope carries everything you need, so you never fetch the task a - **`bounced`** — `true` when the task is coming *back* to you rather than arriving fresh. The program derives it; you never have to notice it yourself. What to do about it is the next section. +- **`feedback`** — on a bounce, the comments that arrived since the last transition, + already cut for you: **that is the ask**. An empty list means the thread moved but + left nothing past the anchor. The key is *absent* only when the read failed — the one + case where you go read the thread yourself. - **`pr`** — present only when an earlier turn on this task already delivered. It is where that work lives: **continue on that branch and update that PR** rather than opening a second one ([`DELIVERY.md`](DELIVERY.md)). @@ -120,13 +124,13 @@ that leaves the claim ref standing and the task reads as held until the lease ex ## What is yours: the judgment -**On `bounced: true`, read the thread before the Goal** (`gh -R OWNER/tasks issue view - --comments`). The task is coming back: any comment I leave puts a held task in the -queue again — from `awaiting-merge` exactly as from `needs-decision` (PROTOCOL.md §6) — -so **the newest feedback is the ask** and the Goal is only background. If it asks for -nothing actionable ("nice, thanks", "merging tomorrow"), do **not** invent rework — run -`then.escalate` saying the delivery still stands and you cannot tell what I want changed. -A question on the thread is the honest answer to a comment nobody can act on. +**On `bounced: true`, read `feedback` before the Goal.** The task is coming back: any +comment I leave puts a held task in the queue again — from `awaiting-merge` exactly as +from `needs-decision` (PROTOCOL.md §6) — so **the newest feedback is the ask** and the +Goal is only background. If it asks for nothing actionable ("nice, thanks", "merging +tomorrow"), or the list came back empty, do **not** invent rework — run `then.escalate` +saying the delivery still stands and you cannot tell what I want changed. A question on +the thread is the honest answer to a comment nobody can act on. Then, inside an `execute`: diff --git a/skills/unleash/kraken/__init__.py b/skills/unleash/kraken/__init__.py index 6596fb4..1f6191d 100644 --- a/skills/unleash/kraken/__init__.py +++ b/skills/unleash/kraken/__init__.py @@ -102,8 +102,9 @@ ) from .next_action import ( NEXT_ACTIONS, NEXT_ACTION_EXIT, NextAction, NextActionEnvelope, - cmd_next_action, issue_is_finished, lease_block, next_action, - next_action_envelope, resume_verdict, task_brief, then_commands + cmd_next_action, feedback_since, issue_is_finished, lease_block, + next_action, next_action_envelope, resume_verdict, task_brief, + then_commands ) from .watch import ( WATCH_MAX_FAILURES, WATCH_WARN_EVERY, cmd_watch, snapshot_state, diff --git a/skills/unleash/kraken/claim.py b/skills/unleash/kraken/claim.py index 46d0118..8645784 100644 --- a/skills/unleash/kraken/claim.py +++ b/skills/unleash/kraken/claim.py @@ -450,17 +450,26 @@ def acquire_next(api: Api, project: str, worker: Worker, one-task-at-a-time guard, §6 reconcile, then guard + CAS down the candidate list — as DATA rather than as printed output: returns `(exit_code, won)` where `won` is - `{"issue", "title", "body", "bounced", "pr"}` on EXIT_OK, `{"issue"}` naming - the already-held claim on EXIT_NOT_CLEAR, and None on every other outcome. - - `bounced` and `pr` come off the state record this read already carried, and - they are here because the record is READ HERE and nowhere else afterwards: - a first claim writes none (§3.1), so a caller that wanted either would have - to fetch what this function had in hand and threw away. `bounced` is §6's - requeue derivation — two integers, exact — so no reader downstream has to - re-derive "is this task coming back" from the thread, and `pr` is where the - last delivery went, so rework continues on that branch instead of opening a - second PR beside it. + `{"issue", "title", "body", "bounced", "pr", "anchor"}` on EXIT_OK, + `{"issue"}` naming the already-held claim on EXIT_NOT_CLEAR, and None on + every other outcome. + + `bounced`, `pr` and `anchor` come off the state record this read already + carried, and they are here because the record is READ HERE and nowhere else + afterwards: a first claim writes none (§3.1), so a caller that wanted any of + them would have to fetch what this function had in hand and threw away. + `bounced` is §6's requeue derivation — two integers, exact — so no reader + downstream has to re-derive "is this task coming back" from the thread, and + `pr` is where the last delivery went, so rework continues on that branch + instead of opening a second PR beside it. + + `anchor` is the OTHER half of that derivation: the comment total frozen into + the record, which is the index the new comments start at. `bounced` says the + thread moved on; `anchor` says WHERE, and a reader holding both can cut the + feedback out of the thread instead of judging by eye which comments are new. + Carried as the integer rather than as the comments themselves because this + function is the deterministic claim loop and reading a thread is not part of + it — `next-action` pays for that read, and only when `bounced` is true. `claim-next` and `next-action` are both thin renderings of this, which is what keeps the deterministic claim loop from drifting between them: there is @@ -540,7 +549,8 @@ def acquire_next(api: Api, project: str, worker: Worker, return (EXIT_OK, {"issue": cand.number, "title": cand.title, "body": cand.body, "bounced": record.requeued(cand.task.comment_total), - "pr": record.pr}) + "pr": record.pr, + "anchor": record.comments}) if rc == EXIT_TRANSPORT: # State is now ambiguous — do NOT move on to another candidate while # a write of ours may have half-landed. Re-check before any retry. diff --git a/skills/unleash/kraken/next_action.py b/skills/unleash/kraken/next_action.py index 25e6b85..fc61fe9 100644 --- a/skills/unleash/kraken/next_action.py +++ b/skills/unleash/kraken/next_action.py @@ -8,10 +8,11 @@ import os from .contract import ( - ClaimRecord, ENTRYPOINT, EXIT_LOST, EXIT_NONE, EXIT_NOT_CLEAR, EXIT_OK, - EXIT_TRANSPORT, EXIT_UNKNOWN_PROJECT, Envelope, Epoch, Gen, Issue, Json, - Repo, Worker, diag, diagnostics_on_stderr + ClaimRecord, CommentRecord, ENTRYPOINT, EXIT_LOST, EXIT_NONE, + EXIT_NOT_CLEAR, EXIT_OK, EXIT_TRANSPORT, EXIT_UNKNOWN_PROJECT, Envelope, + Epoch, Gen, Issue, Json, Repo, Worker, diag, diagnostics_on_stderr ) +from .comments import parse_marker from .transport import Api, comment_total_of from .lease import ( Lease, UNREADABLE_LEASE, clear_claim_state, format_iso, @@ -107,6 +108,38 @@ def lease_block( } +def feedback_since(api: Api, issue: Issue, anchor: int, + ) -> list[CommentRecord] | None: + """The comments that arrived AFTER the state record was written — §6's + requeue derivation as the words that caused it, not as a boolean the agent + then goes back to the thread to interpret. Returns the records in server + order, or None on transport failure. + + Two steps, and they answer different questions. WHERE the new comments start + is positional: the anchor is a comment TOTAL and `comment_records` returns + the whole thread in that same server order, so the tail past that index is + the range §6 derived its verdict from — every comment counts there (§6), so + no filter may run before the cut without sliding it. + + WHAT of that range is the ask is a different question, and the machine's own + comments are not it. A marker is exactly the structural signal that says + "worker-authored" (§4), and by the time this runs the claim's own comment is + already on the thread — so an unfiltered tail would hand every bounced task + back its own "Claimed this task" line as though the operator had written it. + Filtering AFTER the cut costs the range nothing: the index is already fixed. + + A thread that SHRANK below its anchor yields an empty list rather than a + slice from the end — which is honest, and is the c13 case: the anchor is + stale, §6's re-anchor repair is what fixes it, and a reader must not invent + feedback to fill the gap. Empty is still an answer, and the caller reports + it as one; None is the absence that means "the read did not land".""" + records = api.comment_records(issue) + if records is None: + return None + return [c for c in records[max(0, int(anchor)):] + if parse_marker(c.get("body") or "") is None] + + def task_brief(title: str, body: str) -> Json: """The task as the agent needs it: the issue-form sections split out, plus the raw body for a hand-written issue that carries no headings. An empty or @@ -152,6 +185,7 @@ def build(self, action: str, *, resumed: bool | None = None, bounced: bool | None = None, pr: str | None = None, + feedback: list[CommentRecord] | None = None, brief: Json | None = None, lease: Json | None = None, reason: str | None = None, @@ -159,14 +193,24 @@ def build(self, action: str, *, holding: Json | None = None) -> Envelope: """The one JSON shape next-action emits. - `bounced` and `pr` are what the state record knows about this task's - PAST, carried because the program already computed both and the agent - would otherwise have to rediscover them from the thread — `bounced` by - reading comments and judging whose they are, `pr` by hunting the earlier - delivery in the body. `bounced` rides every execute, false included: "no, - this is a fresh task" is an answer, and an absent key would read as an - older program that could not tell. `pr` is omitted when there is none, - the same rule the markers follow — no delivery is not an empty delivery. + `bounced`, `pr` and `feedback` are what the state record knows about this + task's PAST, carried because the program already computed them and the + agent would otherwise have to rediscover them from the thread — + `bounced` by reading comments and judging whose they are, `pr` by hunting + the earlier delivery in the body. `bounced` rides every execute, false + included: "no, this is a fresh task" is an answer, and an absent key + would read as an older program that could not tell. `pr` is omitted when + there is none, the same rule the markers follow — no delivery is not an + empty delivery. + + `feedback` is what `bounced` is ABOUT: the comments past the record's + anchor, so the ask arrives with the verdict instead of costing a fetch + and an eyeball judgment of which comments are new. It rides a bounced + execute only — a fresh task has no thread to cut — and its two absences + are distinct, which is why it is omitted rather than emitted empty on + failure: `[]` says the read landed and found nothing past the anchor (a + stale anchor, §6's repair case), while an ABSENT key says the read did + not land and the agent owes the thread a look of its own. `holding` is `{"repo", "issue"}` naming a claim this worker must resolve, and it exists because a `blocked` claim may live in a **different repo** @@ -188,6 +232,8 @@ def build(self, action: str, *, env["bounced"] = bool(bounced) if pr: env["pr"] = pr + if feedback is not None: + env["feedback"] = list(feedback) if reason: env["reason"] = reason if detail: @@ -445,13 +491,32 @@ def _resumed(self, issue: Issue, detail: Json, issue_obj: Json, # and still carries the anchor the live total ran past. Every resume of # this task therefore reports the same `bounced` its acquisition did — # which is what the agent needs, since it is rework for its whole turn. + # And because the anchor is the same, so is the cut: a resumed bounce + # carries the same feedback the acquisition did, rather than a shorter + # tail that would silently drop what the agent is meant to answer. + bounced = state.requeued(comment_total_of(issue_obj)) return self.envelope.answer( "execute", issue=issue, resumed=True, - bounced=state.requeued(comment_total_of(issue_obj)), pr=state.pr, + bounced=bounced, pr=state.pr, + feedback=self._feedback(issue, bounced, state.comments), brief=task_brief(issue_obj.get("title") or "", issue_obj.get("body") or ""), lease=lease) + def _feedback(self, issue: Issue, bounced: bool, + anchor: int) -> list[CommentRecord] | None: + """The comments this bounce is about, or None when there is nothing to + cut or the cut could not be read. + + Gated on `bounced` so the common case — a fresh task, no thread — pays + nothing: this is the one read `next-action` adds beyond what the claim + loop already did, and it is owed only when the program has just told the + agent the thread moved on. A failed read degrades to the older behaviour + (the agent goes to the thread itself), never to a wrong cut.""" + if not bounced: + return None + return feedback_since(self.api, issue, anchor) + # --- acquiring the next task --------------------------------------------- def acquire(self) -> tuple[int, Envelope]: @@ -462,6 +527,8 @@ def acquire(self) -> tuple[int, Envelope]: return self.envelope.answer( "execute", issue=won["issue"], resumed=False, bounced=won["bounced"], pr=won["pr"], + feedback=self._feedback(won["issue"], won["bounced"], + won["anchor"]), brief=task_brief(won["title"], won["body"]), lease=lease_block(self.now, self.now, self.ttl, source="estimated")) diff --git a/skills/unleash/kraken/render.py b/skills/unleash/kraken/render.py index 25ec307..20a4709 100644 --- a/skills/unleash/kraken/render.py +++ b/skills/unleash/kraken/render.py @@ -27,6 +27,16 @@ def render_next_action(env: Envelope) -> None: print(f" {env['detail']}") if env.get("pr"): print(f" pr: {env['pr']} (continue on this branch — do not open a second)") + # Only on a bounce, and as a COUNT: the bodies are the JSON's business, and + # a console line that dumped a thread would bury the verdict it sits under. + # An absent key on a bounced envelope is worth saying out loud — it is the + # one case where the agent still owes the thread a read of its own. + if env.get("bounced"): + feedback = env.get("feedback") + if feedback is None: + print(" feedback: unread — check the thread yourself") + else: + print(f" feedback: {len(feedback)} comment(s) past the anchor") brief = env.get("brief") if brief: print(f" title: {brief['title']}") diff --git a/skills/unleash/kraken/workflow.py b/skills/unleash/kraken/workflow.py index d99a0c3..c62d159 100644 --- a/skills/unleash/kraken/workflow.py +++ b/skills/unleash/kraken/workflow.py @@ -16,6 +16,7 @@ from .refs import Refs from .queue import is_empty_section, section_body from .render import render_init +from .state import States # --- subcommand: init -------------------------------------------------------- # The bootstrap `init`, single-sourced here so the skill and the program never @@ -204,7 +205,9 @@ def cmd_validate(args: argparse.Namespace) -> int: a compliant task gets none (no noise on the happy path, and the same exit once the operator fixes what was flagged). Debounced: a re-run whose missing set is unchanged posts no duplicate. Informs only — never holds, closes, or - relabels. Exit 0 on a clean run, 20 on gh/transport failure.""" + relabels; the one ref it writes is the §3.1 anchor refresh its own comment + owes (`_refresh_anchor`), which changes no state. Exit 0 on a clean run, 20 + on gh/transport failure.""" api, issue = args.api, args.issue labels = api.issue_label_names(issue) @@ -248,10 +251,58 @@ def cmd_validate(args: argparse.Namespace) -> int: if not api.post_comment(issue, body_to_post): print(f"validate: gh-failure stage=comment issue={issue}", file=sys.stderr) return EXIT_TRANSPORT + + rc = _refresh_anchor(api, issue) + if rc != EXIT_OK: + return rc print(f"validate: #{issue} flagged (missing: project/Goal/Acceptance as listed)") return EXIT_OK +def _refresh_anchor(api, issue) -> int: + """Move the state record's comment anchor past the comment this pass just + posted — the PROTOCOL.md §3.1 SHOULD that any program-authored comment on a + task's thread owes. + + Without it the validator's own comment is indistinguishable from an + operator's reply: §6 derives a requeue from `total > record.comments` and + counts every comment, so a held task the validator touches reads as bounced. + The next worker then claims rework nobody asked for, finds an automated + nag on the thread, and — correctly, per the skill — escalates it back as a + question. The noise costs the operator a `needs-decision` entry; the fix is + to stop generating it, since the count is arithmetic the program has. + + `re_anchored` is exactly the right write and no more: the anchor moves, the + hold does not lift, the state, worker, expiry count and PR all stand. Only a + task that HAS a record is touched — an absent record reads as queued (§3.1) + and holds nothing, so there is no anchor to move and writing one would + invent state this pass has no business creating. + + A failed read or write is reported as transport rather than swallowed: the + comment has landed by now, so a silent failure leaves precisely the stale + anchor this exists to prevent, and the run should say so. + + The record is read BEFORE the count, which is the cheap order for this + caller: the validator fires on new and edited queue entries, and a task that + has never been put down has no record at all — the overwhelmingly common + case exits here having paid one ref read, never the issue fetch as well.""" + states = States(api) + record = states.of(issue) + if record.unknown: + print(f"validate: gh-failure stage=record issue={issue}", file=sys.stderr) + return EXIT_TRANSPORT + if not record.recorded: + return EXIT_OK + total = api.comment_count(issue) + if total is None: + print(f"validate: gh-failure stage=anchor issue={issue}", file=sys.stderr) + return EXIT_TRANSPORT + if not states.write(issue, record.re_anchored(total)): + print(f"validate: gh-failure stage=record issue={issue}", file=sys.stderr) + return EXIT_TRANSPORT + return EXIT_OK + + def is_identity_label(name: str) -> bool: """A label cleanup MUST preserve on a closed task: the task-type label (kraken-task) and its project routing label (project:). Everything else diff --git a/tests/conformance/test_next_action.py b/tests/conformance/test_next_action.py index ac0aa71..dc8731b 100644 --- a/tests/conformance/test_next_action.py +++ b/tests/conformance/test_next_action.py @@ -122,6 +122,17 @@ def test_a_bounced_task_arrives_flagged_and_carrying_its_pr(self): "the earlier delivery's PR was dropped; a worker " "without it opens a second, orphan PR") + # The bounce arrives with the words that caused it. `bounced` alone still + # cost a `gh issue view --comments` and an eyeball judgment of which + # comments were new — the program holds the anchor, so the cut is a list + # slice and belongs here, not in the agent's head. + self.assertIn("feedback", env, + "a bounce must carry the comments it is about") + self.assertEqual([c["body"].strip() for c in env["feedback"]], + ["almost — the pagination is off by one, please fix"], + "the feedback must be exactly what arrived past the " + "anchor — no earlier comment, nothing missing") + # And it survives the claim: taking the task writes no record (§3.1), so # every resume of it reports the same verdict its acquisition did. A # bounce that evaporated on the first `next-action` after the claim @@ -130,6 +141,22 @@ def test_a_bounced_task_arrives_flagged_and_carrying_its_pr(self): self.assertTrue(again["resumed"], "setup: expected a resume") self.assertTrue(again["bounced"], "the bounce did not survive the claim") self.assertEqual(again["pr"], pr, "the resume dropped the delivery URL") + self.assertEqual([c["body"].strip() for c in again["feedback"]], + ["almost — the pagination is off by one, please fix"], + "the resume must cut from the SAME anchor — a shorter " + "tail would drop what the agent is meant to answer") + + def test_a_fresh_task_carries_no_feedback(self): + """No bounce, no thread to cut, no read paid for. `feedback` is owed by + a bounce and nothing else.""" + self.mk_issue(7, "a fresh task", "kraken-task", "project:app") + self.mk_body(7, "### Goal\nship it") + _r, env = self.envelope("acme/tasks", "app", "w1") + self.assertEqual(env["action"], "execute", "setup claim failed") + self.assertFalse(env["bounced"], "setup: expected a fresh claim") + self.assertNotIn("feedback", env, + "a fresh task must carry no feedback key — an empty " + "one reads as an operator who said nothing") def test_stolen_lease_is_abandoned_without_writing(self): self.mk_issue(7, "task", "kraken-task", "project:app") diff --git a/tests/unit/test_kraken.py b/tests/unit/test_kraken.py index 219b533..ecfb64f 100644 --- a/tests/unit/test_kraken.py +++ b/tests/unit/test_kraken.py @@ -822,7 +822,7 @@ def test_claims_first_startable(self): self.assertEqual(rc, kraken.EXIT_OK) self.assertEqual(self.attempted, [7]) self.assertEqual(won, {"issue": 7, "title": "oldest", "body": "body-7", - "bounced": False, "pr": None}) + "bounced": False, "pr": None, "anchor": 0}) def test_the_won_payload_carries_the_records_bounce_and_pr(self): """A task with no record is a fresh one; a task whose held record the @@ -841,6 +841,9 @@ def test_the_won_payload_carries_the_records_bounce_and_pr(self): "a comment past the record's anchor is a bounce (§6)") self.assertEqual(won["pr"], "https://github.com/o/r/pull/3", "the delivery the task is coming back from was dropped") + self.assertEqual(won["anchor"], 4, + "the anchor is where the new comments start — without " + "it a reader knows the thread moved but not from where") def test_a_task_whose_record_still_holds_is_not_bounced(self): # Same record, nothing said since: the anchor equals the live total, so @@ -2275,6 +2278,61 @@ def test_claim_in_another_repo_blocks(self): "the blocking claim is not named") +class FeedbackSinceTests(unittest.TestCase): + """The cut §6's requeue derivation implies: `bounced` says the thread moved + on, the anchor says from where, and this is the tail between them. Judgment + is the model's — deciding what the feedback ASKS FOR — but finding which + comments are new is arithmetic, and arithmetic is the program's.""" + + THREAD = [{"body": "goal notes", "createdAt": "t0"}, + {"body": "assumptions", "createdAt": "t1"}, + {"body": "delivered", "createdAt": "t2"}, + {"body": "please rename the flag", "createdAt": "t3"}] + + def _api(self, records=None): + thread = self.THREAD if records is None else records + return FakeApi("acme/tasks", comment_records=lambda i: thread) + + def test_the_cut_starts_at_the_anchor(self): + got = kraken.feedback_since(self._api(), 12, 3) + self.assertEqual([c["body"] for c in got], ["please rename the flag"], + "the ask is exactly what arrived past the anchor") + + def test_the_cut_is_positional_and_counts_from_the_total(self): + """WHERE the range starts is the anchor, which is a comment TOTAL: every + comment counts toward it (§6), so nothing may be filtered out BEFORE the + slice or the index slides by one per skipped comment.""" + got = kraken.feedback_since(self._api(), 12, 1) + self.assertEqual([c["body"] for c in got], + ["assumptions", "delivered", "please rename the flag"], + "the cut must start exactly at the anchor index") + + def test_the_machines_own_comments_are_not_the_ask(self): + """By the time this runs the claim's own comment is on the thread. A + marker is what says "worker-authored" (§4), so an unfiltered tail would + hand a bounced task its own "Claimed this task" line as the operator's + ask — and the agent would do rework against a machine's boilerplate.""" + thread = self.THREAD + [ + {"body": kraken.compose_comment("w1", "Claimed this task.", + {"type": "claim", "worker": "w1"}), + "createdAt": "t4"}] + got = kraken.feedback_since(self._api(thread), 12, 3) + self.assertEqual([c["body"] for c in got], ["please rename the flag"], + "a machine comment past the anchor read as the ask") + + def test_an_anchor_past_the_thread_yields_nothing(self): + """The c13 case: the thread SHRANK below its anchor. An empty cut is + honest — §6's re-anchor repair is what fixes the anchor, and a reader + must never invent feedback by slicing from the end.""" + self.assertEqual(kraken.feedback_since(self._api(), 12, 99), [], + "a stale anchor must not produce feedback") + + def test_a_failed_read_is_none_never_an_empty_cut(self): + api = FakeApi("acme/tasks", comment_records=lambda i: None) + self.assertIsNone(kraken.feedback_since(api, 12, 0), + "a failed read must not pose as 'nothing was said'") + + class NextActionEnvelopeTests(unittest.TestCase): """The envelope is the contract the agent reads, so its shape is pinned.""" @@ -2320,6 +2378,24 @@ def test_blocked_names_the_held_claim_and_targets_it(self): "then.%s targets the drained repo, not the held " "claim" % name) + def test_feedback_absent_and_empty_mean_different_things(self): + """The two absences are not interchangeable, so the envelope keeps them + apart: `[]` is "the read landed and the thread has nothing past the + anchor" (the stale-anchor case), while an ABSENT key is "the read did + not land" — the one case where the agent still owes the thread a look. + Collapsing them would tell a worker with a failed read that its + operator asked for nothing.""" + empty = kraken.next_action_envelope( + "execute", "acme/tasks", "env-1", issue=12, bounced=True, + feedback=[]) + self.assertEqual(empty["feedback"], [], + "an empty cut is an answer and must be emitted") + unread = kraken.next_action_envelope( + "execute", "acme/tasks", "env-1", issue=12, bounced=True, + feedback=None) + self.assertNotIn("feedback", unread, + "an unread thread must not pose as an empty one") + def test_every_action_has_an_exit_code(self): self.assertEqual(set(kraken.NEXT_ACTIONS), set(kraken.NEXT_ACTION_EXIT), "an action without an exit code is unreachable from a " diff --git a/tests/unit/test_workflow_commands.py b/tests/unit/test_workflow_commands.py index 75b5865..dc53ab6 100644 --- a/tests/unit/test_workflow_commands.py +++ b/tests/unit/test_workflow_commands.py @@ -480,6 +480,9 @@ def _api(self, **methods): "post_comment": lambda issue, body: ( self.posts.append((issue, body)) or True), "comment_records": lambda issue: [], + # No state ref: the default queue entry has never been put down, so + # the §3.1 anchor refresh finds no record and writes nothing. + "paginated": lambda path, **kw: [], } defaults.update(methods) return FakeApi("acme/tasks", **defaults) @@ -539,6 +542,120 @@ def test_transport_failure_on_labels_is_twenty(self): rc = kraken.cmd_validate(args) self.assertEqual(rc, kraken.EXIT_TRANSPORT) + # --- the §3.1 anchor refresh this pass's own comment owes ---------------- + + def _flagged(self, record=None, total=9, **methods): + """Run a validate pass over a task that WILL be flagged (no project + label), against a state ref that resolves to `record` through the REAL + `States.of` path — one matching-refs read, one batched commit read. + `record=None` is a task that has none. Ref writes land in `self.writes`; + returns the exit code.""" + self.writes = [] + + def request(verb, path, body=None): + self.writes.append((verb, path, body)) + # The commit create is the one write whose RESPONSE is read back + # (Refs.commit wants the sha it then points the ref at). + return (201, json.dumps({"sha": "new-sha"})) + + def paginated(path, **kw): + if "matching-refs" in path and record is not None: + return [{"ref": "refs/kraken/state/1", "object": {"sha": "s1"}}] + return [] + + defaults = { + "issue_label_names": lambda i: ["kraken-task"], + "issue_body": lambda i: self.GOOD, + "paginated": paginated, + "aliased": lambda fields: { + "c0": {"committedDate": "2026-07-01T09:00:00Z", + "message": kraken.make_marker(record.payload())}} + if record is not None else (lambda fields: {}), + "comment_count": lambda issue: total, + "request": request, + } + defaults.update(methods) + args = SimpleNamespace(repo="acme/tasks", issue="1", + api=self._api(**defaults)) + with redirect_stdout(StringIO()), redirect_stderr(StringIO()): + return kraken.cmd_validate(args) + + def _written_record(self): + """The state marker this run committed, decoded — or None if it wrote + no commit at all.""" + for _verb, path, body in self.writes: + if path.endswith("/git/commits"): + return kraken.parse_state_commit((body or {}).get("message", "")) + return None + + def test_its_own_comment_does_not_read_as_an_operator_reply(self): + """THE point of the refresh (PROTOCOL.md §3.1). §6 derives a requeue + from `total > record.comments` and counts EVERY comment, so a validation + comment left un-anchored makes a held task read as bounced: the next + worker claims rework nobody asked for, finds an automated nag, and + escalates it back as a question the operator then has to answer.""" + held = kraken.TaskState(state="awaiting-merge", worker="w0", + comments=4, recorded=True, + pr="https://github.com/o/r/pull/3") + rc = self._flagged(held, total=5) + self.assertEqual(rc, kraken.EXIT_OK) + self.assertEqual(len(self.posts), 1, "the flag itself still posts") + written = self._written_record() + self.assertIsNotNone(written, "the anchor was never refreshed") + self.assertEqual(written.comments, 5, + "the anchor must clear this pass's own comment") + self.assertFalse(written.requeued(5), + "after the refresh the task must NOT read as bounced") + + def test_the_refresh_moves_the_anchor_and_nothing_else(self): + """`re_anchored`, not `moved_to`: validate informs, so the hold, the + worker, the expiry count and the delivery URL all stand. A refresh that + lifted the hold would hand an answered task back to the queue.""" + held = kraken.TaskState(state="needs-decision", worker="w0", comments=4, + expiries=2, recorded=True, + pr="https://github.com/o/r/pull/3") + self._flagged(held, total=5) + written = self._written_record() + self.assertEqual(written.state, "needs-decision", "the hold was lifted") + self.assertEqual(written.worker, "w0") + self.assertEqual(written.expiries, 2, "the expiry count is cumulative") + self.assertEqual(written.pr, "https://github.com/o/r/pull/3") + + def test_a_task_with_no_record_is_not_given_one(self): + """The common case: a new queue entry has never been put down, so there + is no anchor to move and no record this pass may invent.""" + rc = self._flagged(None) + self.assertEqual(rc, kraken.EXIT_OK) + self.assertEqual(len(self.posts), 1, "the flag itself still posts") + self.assertIsNone(self._written_record(), + "a task with no record must not be given one") + + def test_a_compliant_task_pays_for_no_refresh(self): + """No comment, no anchor to clear. The refresh is owed by the POST, so + the happy path must not read the record at all.""" + reads = [] + rc = self._flagged( + None, + issue_label_names=lambda i: ["kraken-task", "project:app"], + paginated=lambda path, **kw: reads.append(path) or []) + self.assertEqual(rc, kraken.EXIT_OK) + self.assertEqual(self.posts, [], "a compliant task is not flagged") + self.assertEqual( + [p for p in reads if "matching-refs" in p], [], + "a compliant task must not pay for a state-ref read") + + def test_an_unreadable_record_is_transport_not_a_silent_stale_anchor(self): + held = kraken.TaskState(state="awaiting-merge", worker="w0", + comments=4, recorded=True) + rc = self._flagged(held, paginated=lambda path, **kw: None) + self.assertEqual(rc, kraken.EXIT_TRANSPORT) + + def test_an_unreadable_count_is_transport(self): + held = kraken.TaskState(state="awaiting-merge", worker="w0", + comments=4, recorded=True) + rc = self._flagged(held, comment_count=lambda issue: None) + self.assertEqual(rc, kraken.EXIT_TRANSPORT) + # --- cleanup-closed: the identity-label rule --------------------------------