From cf9351b2bc25dd0315127d4c2fe665a29225a61a Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:57:07 -0600 Subject: [PATCH 1/4] Controller: a pull request that moved while a pass read it is deferred once with no alert, and the record keeps why (#658) A snapshot that moved while it was read raised Moved, and the entry took it as any read GitHub did not answer: the pass raised facts_unreadable, and the item's writes, never read, left reconciliation short. Seven Factory alerts since 2026-11-08 05:38Z were each one pull request merging, pushing or taking a comment during the read. pass/1 now keeps left_out: each item left out with its code, pull_moved, facts_unanswered, not_facts or listed_twice. A pull request that moved, and that the last pass record did not leave out as moved, is deferred with no alert, and its writes are owed to the next pass, so this reconciliation is whole. A second move in a row, any other code, or a last record that cannot say, is the alert. Whether a move is a first is derived from the last record at each pass (moved_once), never kept. Decision NNNN records it, with two boundary rows naming 11 mutants; two older mutants follow the code. Co-Authored-By: Claude Opus 5.5 --- controller/AGENTS.md | 1 + controller/__main__.py | 25 ++++-- controller/pass_.py | 27 +++++- controller/reads.py | 27 +++++- ...-read-it-is-deferred-once-with-no-alert.md | 24 +++++ schemas/pass.schema.json | 23 +++++ tests/test_controller.py | 88 +++++++++++++++++++ tools/mutants.py | 38 +++++++- 8 files changed, 234 insertions(+), 19 deletions(-) create mode 100644 docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md diff --git a/controller/AGENTS.md b/controller/AGENTS.md index a827453a..e066c6b8 100644 --- a/controller/AGENTS.md +++ b/controller/AGENTS.md @@ -12,6 +12,7 @@ Not built yet, by the stories that hold their rules: relay.py, backpressure.py, | A Run id is minted for an admission, an approval, or a queue act once its workflow is named | `tests/test_controller.py::test_a_queue_act_gets_a_run_id_and_a_dispatch_once_its_workflow_is_named` | | A Run is dispatched by its run-name; a capacity fault defers model Runs for its window | `tests/test_controller.py::test_the_runs_to_dispatch_are_named_by_their_run_names`, `tests/test_controller.py::test_a_capacity_fault_defers_model_runs_for_its_window` | | An item whose facts are not facts, or listed twice, is left out, as one alert; any other is decided as without it | `tests/test_controller.py::test_an_item_whose_facts_are_not_facts_is_left_out_and_raises_one_alert` | +| A pull request that moved while it was read, and not in the last pass, is deferred with no alert and its writes owed; a second move, any other read left out, or a last record that cannot say, is the alert; the record keeps each one's code | `tests/test_controller.py::test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert`, `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once` | | An issue is asked of the issues' Kernel, under its own Lifecycle, and its repo's Pause is read | `tests/test_controller.py::test_an_issue_is_asked_of_the_issues_kernel_with_its_own_lifecycle` | | A record that cannot validate is refused whole, and nothing is dispatched | `tests/test_controller.py::test_a_record_that_cannot_validate_is_refused_and_nothing_is_dispatched` | | A Pause on the org or a repo is a Pause; a value but exact `true`, exact `false` or none is unreadable | `tests/test_controller.py::test_a_pause_on_the_org_or_the_repo_is_a_pause_and_any_other_value_is_unreadable` | diff --git a/controller/__main__.py b/controller/__main__.py index 8081c240..4fd3d8e0 100644 --- a/controller/__main__.py +++ b/controller/__main__.py @@ -13,7 +13,8 @@ effects step holds to it, and the view's three files under `view/`. The Release is the one tag `factory/vX.Y.Z` on the commit the pass checked out (`harness/steps.release`), read before any key. A snapshot that cannot be read leaves its pull -request out of the pass, by its item, as one alert. A list of open pull requests not read whole, a Release, a key or +request out of the pass, by its item and the code of why, as one alert; one that moved while it was read, and had not in +the last pass, is deferred with no alert and its writes are owed to the next pass (#658). A list of open pull requests not read whole, a Release, a key or a run id that cannot be read, or a pass `pass_.py` refuses, writes nothing, so that nothing is dispatched; the step prints one code and exits 1, and nothing it prints is GitHub's text.""" from __future__ import annotations @@ -30,7 +31,7 @@ from adapters.facts import accounts_of as accounts, dispatcher_of as dispatcher, opener_of as opener, queue_of from adapters.github import Live, Unanswered from controller import configuration as configured, pass_, reads as reads_, reconcile, writes as writes_ -from controller.shared import Refused +from controller.shared import Refused, key as page from harness import steps ROOT = pathlib.Path(__file__).resolve().parent.parent @@ -92,11 +93,12 @@ def dated(answer) -> list: return [{"date": at["date"], "age": at["age"]}] if at else [] -def writes_seen(transport, config: dict, numbers: list, kept: dict, before, since: str, clock) -> tuple: +def writes_seen(transport, config: dict, numbers: list, kept: dict, before, since: str, clock, owed=()) -> tuple: """`(seen/1, the pull requests read short)`: what each Factory identity wrote on what this pass served, or on what the pass before served or could not read and has left (decision 0061). What a snapshot could not read is short, and one that is short is read again by the next pass, so that nothing leaves unread. So is a pass with no records before it to say - what has left. Nothing is guessed.""" + what has left. Nothing is guessed. One `owed`, a pull request that moved for the first time, is not read: it is owed + to the next pass, as one read short is, and leaves this reconciliation whole (#658).""" controller = config["controller"] home, accounts_held = controller["home"]["repo"], controller["accounts"] identities, _ = seen_.identities(accounts_held) @@ -105,6 +107,9 @@ def writes_seen(transport, config: dict, numbers: list, kept: dict, before, sinc served = {e["facts"]["item"]["number"] for e in last} | set(reconcile.owed(before)) parts, unread = [], [] for number, held in [(n, kept.get(n)) for n in numbers] + [(n, seen_.LEFT) for n in sorted(served - set(numbers))]: + if number in owed: + unread.append(number) + continue try: part = seen_.read(transport, {"repo": home, "type": "pull_request", "number": number}, held, identities, apps, since) @@ -176,11 +181,17 @@ def read(transport, config: dict, run_id: str, previous, now: str, clock, starte pulls.append({"facts": facts, "title": title, "head": head}) kept[number] = held facts_reads += dated(through.first) - except Unanswered: - pulls.append({"facts": {"item": {"repo": home, "type": "pull_request", "number": number}}, "title": None}) + except Unanswered as err: + # Its code goes in the record: one that moved while it was read is not one GitHub did not answer (#658). + pulls.append({"facts": {"item": {"repo": home, "type": "pull_request", "number": number}}, "title": None, + "unread": reads_.MOVED if isinstance(err, facts_.Moved) else reads_.UNANSWERED}) before = pass_.earlier(reads_.previous_records(previous), now) since = utc(lambda: clock() - WINDOW - controller["not_run_seconds"]) - seen, unread = writes_seen(transport, config, numbers, kept, before, since, clock) + # A pull request that moved for the first time is owed to the next pass's reconciliation, not read short here. + left = [e for e in pulls if "unread" in e] + graced = pass_.moved_once([e["facts"]["item"] for e in left], [e["unread"] for e in left], before) + owed = {e["facts"]["item"]["number"] for e in left if page(e["facts"]["item"]) in graced} + seen, unread = writes_seen(transport, config, numbers, kept, before, since, clock, owed) return {"started": started, "pause": pause_.read(transport, home.split("/")[0], [home]), "records": runs, "pull_requests": pulls, "issues": [], "facts_reads": facts_reads, "usage": {}, "seen": seen, "unread": unread, "effects": recorded_effects(transport, config, runs, int(run_id), before, since)} diff --git a/controller/pass_.py b/controller/pass_.py index 8208a167..0f779926 100644 --- a/controller/pass_.py +++ b/controller/pass_.py @@ -262,6 +262,20 @@ def status_head(entry: dict, kernel: dict) -> dict: return {} if pull_request.left_alone(facts, kernel) else {"head": entry["head"]} +def moved_once(left_out: list, why: list, before) -> set: + """The pages of the items left out because they moved while read that the last pass record read and did not leave + out as moved: each is deferred with no alert, once, and its writes are owed to the next pass (#658). It is derived + from that record, never kept: a last record that cannot be read, or holds no `left_out`, cannot say, so each move + counts as a second.""" + last = before[-1] if isinstance(before, list) and before else None + held = last.get("left_out") if isinstance(last, dict) else None + if not isinstance(held, list): + return set() + again = {key(e["item"]) for e in held if e["code"] == reads_.MOVED and e["item"] is not None} + return {key(i) for i, code in zip(left_out, why) + if code == reads_.MOVED and reads_.item(i) is not None and key(i) not in again} + + def run(reads: dict, config: dict, now: str, previous, mint) -> dict: """One pass: the record, the Runs to dispatch, the acts, the view, the alerts and whether a successor starts.""" rules = reads_.configured(config["controller"]) @@ -270,9 +284,13 @@ def run(reads: dict, config: dict, now: str, previous, mint) -> dict: previous, before = (None, []) if before is None else (before, before) # Facts that are not facts, and an item listed twice, never reach the Kernel and no record could hold them: the # item is left out of this pass, as one alert, and every other item is decided as it would be without it. - pulls, left_out = reads_.items(copy.deepcopy(reads.get("pull_requests")), "facts") - issues, also = reads_.items(copy.deepcopy(reads.get("issues")), "issue-facts") - left_out += also + pulls, left_out, why = reads_.items(copy.deepcopy(reads.get("pull_requests")), "facts") + issues, also, why_also = reads_.items(copy.deepcopy(reads.get("issues")), "issue-facts") + left_out, why = left_out + also, why + why_also + # A pull request that moved while it was read, and had not in the last pass, is deferred with no alert: the pass its + # change starts reads it again. Any other left out, or one that moved twice in a row, is the alert (#658). + graced = moved_once(left_out, why, before) + alarmed = [i for i in left_out if reads_.item(i) is None or key(i) not in graced] repos = {e["facts"]["item"]["repo"] for e in pulls + issues} records = reads.get("records") @@ -304,6 +322,7 @@ def asked(entries: list, decide, lifecycle: dict) -> list: + [dict(r, read="facts") if isinstance(r, dict) else r for r in reads.get("facts_reads") or []], "pull_requests": asked(pulls, pull_request.decide, config["lifecycles"]["pull_request"]), "issues": asked(issues, issue.decide, config["lifecycles"]["issue"]), + "left_out": [{"item": reads_.item(i), "code": code} for i, code in zip(left_out, why)], "reconciliation": reconcile_.entries(found), "reconciled": reconcile_.carried(before, reads.get("effects"), reads.get("seen"), now, reads.get("unread") or ())} checked(record, "pass") @@ -343,7 +362,7 @@ def gap(later: str, earlier_: str) -> bool: due, names_due = credentials_.due(config.get("credentials"), now) causes = automatic_causes(runs, now, rules["automatic_pause"]) if automatic is not None else [] alerts = health_.alerts([e["decision"]["reason"] for e in entries], context, capacity, causes, - bool(left_out), raised, unnamed, reconcile_.alerts(found), due, + bool(alarmed), raised, unnamed, reconcile_.alerts(found), due, whole and never_started(runs, now, rules["automatic_pause"])) decided = [e["facts"]["item"] for e in entries] titles = {key(e["facts"]["item"]): e.get("title") if isinstance(e.get("title"), str) else "" diff --git a/controller/reads.py b/controller/reads.py index cae9cc9d..d27217c7 100644 --- a/controller/reads.py +++ b/controller/reads.py @@ -42,8 +42,10 @@ def _read(entry) -> bool: def items(entries, kind: str) -> tuple: - """`(kept, left_out)`: the entries whose facts are facts of `kind`, and the item of each that is not, as one list. - An item listed twice is left out every time it is listed, so that one pass never decides it twice.""" + """`(kept, left_out, why)`: the entries whose facts are facts of `kind`, the item of each that is not, as one list, + and why each is left out, as `pass/1` keeps it (#658): listed twice; else the facts port's word for a read that + failed, the entry's `unread`; else facts that are not facts. An item listed twice is left out every time it is + listed, so that one pass never decides it twice.""" if not isinstance(entries, list): # A list of items that could not be read is no evidence that there are none: a pass that wrote an empty feed # from it would look healthy. It is refused, and the not-run alert reports it. @@ -52,9 +54,26 @@ def items(entries, kind: str) -> tuple: # An item listed twice is counted wherever it can be named, readable or not, so a bad copy leaves the good one out. seen = [_named(e) for e in entries] kept = [e for e in readable if seen.count(key(e["facts"]["item"])) == 1] + out = [e for e in entries if not any(e is k for k in kept)] left_out = [e.get("facts", {}).get("item") if isinstance(e, dict) and isinstance(e.get("facts"), dict) else None - for e in entries if not any(e is k for k in kept)] - return kept, [i if isinstance(i, dict) else None for i in left_out] + for e in out] + why = [TWICE if _named(e) is not None and seen.count(_named(e)) > 1 + else e["unread"] if isinstance(e, dict) and e.get("unread") in UNREAD else NOT_FACTS for e in out] + return kept, [i if isinstance(i, dict) else None for i in left_out], why + + +# Why an item is left out of a pass (#658): a read that moved while it was read, which earns one pass's grace; one GitHub +# did not answer; facts that are not facts; an item listed twice. +MOVED, UNANSWERED, NOT_FACTS, TWICE = "pull_moved", "facts_unanswered", "not_facts", "listed_twice" +UNREAD = (MOVED, UNANSWERED) + + +def item(found): + """`found` if it is an item as `facts/1` spells one, which a pass record can keep; None otherwise.""" + return found if (isinstance(found, dict) and set(found) == {"repo", "type", "number"} + and isinstance(found["repo"], str) and found["repo"].count("/") == 1 + and "" not in found["repo"].split("/") and found["type"] in ("pull_request", "issue") + and type(found["number"]) is int and found["number"] >= 1) else None def _named(entry): diff --git a/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md b/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md new file mode 100644 index 00000000..5ffebb08 --- /dev/null +++ b/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md @@ -0,0 +1,24 @@ +# NNNN. A pull request that moved while a pass read it is deferred once with no alert + +Status: accepted. The Factory Builder's ruling of 2026-11-08 on issue 658, "Controller: a pull request that changes while a pass reads it raises facts_unreadable and reconciliation_unreadable, though Moved means read it again". From 2026-11-08 05:38Z, Factory alerts were opened seven times and closed within minutes. Each time, one pull request had merged, taken a commit or taken a comment less than a minute before the pass read it. The snapshot raised `Moved`, which means "read it again at the next pass". The entry took it as any read GitHub did not answer, so it raised `facts_unreadable`, and the item's writes, never read, left reconciliation short. It adds a grace pass and amends nothing else: decision 0056's tracking issue still opens on a pass's first cause. + +## Decision + +| What | Decided | Because | +|---|---|---| +| Why an item is left out | `pass/1` keeps `left_out`: each item the pass left out, in the order read, with its code. `pull_moved` is a snapshot that moved while it was read. `facts_unanswered` is any other read the facts port could not make. `not_facts` is facts that do not hold to their schema, and `listed_twice` an item listed twice, whatever its read said. The item is null where none can be named. A record written before it holds none | The cause is seen in the record, not inferred: which read failed, on which item | +| The grace pass | A pull request whose code is `pull_moved`, and which the last pass record read did not leave out as `pull_moved`, is deferred with no alert: no facts, no Decision and no act, as now, but not `facts_unreadable`. Its writes are not read in this pass. They are owed to the next, as a pull request read short is, so this reconciliation stays whole. The pass its change started, through `workflow_run`, reads it again | A merge, a push or a comment during a read is the Factory working, not a fault. A person assigned to an issue that closes itself minutes later learns nothing from it | +| What still alerts at once | A second move in a row. Any other code. A last record that cannot be read, that is not there, or that holds no `left_out`: it cannot say the move is a first, so the move counts as a second | Something that never settles still reaches a person, and only `Moved` earns the grace. A pass fails closed | +| Derived, never kept | Whether a move is a first is read from the last pass record at each pass, in `controller/pass_.py`'s `moved_once`. The entry asks the same of it to leave the owed writes unread | State is derived from facts and run records at every call, never stored | +| Not here | `adapters/checks.py` still raises a plain `Unanswered` when the head moves while the checks are read. That read shows as `facts_unanswered` and alerts at once. Making it a `Moved` would also change how the approving act classes it | The record's codes will show whether it happens. It is a change of its own | + +## Why + +The tracking issue is how the Factory's faults reach the Maintainer. Seven alerts in a day for pull requests that were only changing would teach him to ignore it. Deferring a moved pull request for one pass keeps the Kernel's rule: an unreadable fact defers. Raising it on the second move keeps the alert for what does not settle. + +## Boundaries + +| Boundary | Check | +|---|---| +| A pull request that moved once is deferred with no alert, and its record keeps `pull_moved`. One that moved twice in a row is the alert. One read clean after a move is decided. Any other code is the alert at once, and so is a move after a last record that cannot be read, is not there or holds no `left_out`. An item listed twice is `listed_twice`, and one that names no item is kept as null | `tests/test_controller.py::test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert`, `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once`, `controller/pass_.py`, `controller/reads.py`, `schemas/pass.schema.json`, mutant: "a pull request that moved once is the alert", mutant: "a move in the last pass earns the grace again", mutant: "a last record that cannot say grants the grace", mutant: "any read left out earns the grace", mutant: "the record keeps an item that names none", mutant: "an item listed twice is left out by its read's code", mutant: "a read's code of any word is kept", mutant: "any dict is an item a record keeps" | +| The entry keeps a moved snapshot as `pull_moved` and any other as `facts_unanswered`. A first move's writes are owed and not read, so reconciliation is whole. A second move's, or one after a record that cannot say, are read short | `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once`, `controller/__main__.py`, mutant: "the entry keeps no code of why a snapshot failed", mutant: "a pull request that moved once is read short", mutant: "every pull request left out is owed" | diff --git a/schemas/pass.schema.json b/schemas/pass.schema.json index 60dab9ca..2ddc018b 100644 --- a/schemas/pass.schema.json +++ b/schemas/pass.schema.json @@ -174,6 +174,29 @@ "pattern": "^[A-Z][A-Z0-9_]*$" } }, + "left_out": { + "description": "Each item the pass left out, in the order read, with why: a read that moved while it was read (`pull_moved`), one GitHub did not answer (`facts_unanswered`), facts that are not facts, or an item listed twice. The item is null where none can be named. The next pass reads it to tell a second move from a first (#658). Absent in a record written before it", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["item", "code"], + "properties": { + "item": { + "description": "The item as `facts/1` spells one, or null", + "type": ["object", "null"], + "additionalProperties": false, + "required": ["repo", "type", "number"], + "properties": { + "repo": {"type": "string", "pattern": "^[^/]+/[^/]+$"}, + "type": {"enum": ["pull_request", "issue"]}, + "number": {"type": "integer", "minimum": 1} + } + }, + "code": {"enum": ["pull_moved", "facts_unanswered", "not_facts", "listed_twice"]} + } + } + }, "reconciliation": { "description": "What reconciliation found, as the view says it, each once: each write a Factory identity made that no Effect recorded, and each merge read as an Override, ids and codes alone (decision 0061). Absent in a record written before reconciliation", "type": "array", diff --git a/tests/test_controller.py b/tests/test_controller.py index e0c5c89f..07dc0c94 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -287,6 +287,45 @@ def test_an_item_whose_facts_are_not_facts_is_left_out_and_raises_one_alert(self self.assertEqual(len(twice["dispatch"]), 1) self.assertEqual(twice["left_out"], [dict(support.ITEM, number=42)] * 2 + [dict(test_issue.ITEM, number=7)] * 2) + def test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert(self): # issue 658 + # Moved while it was read, and not in the last pass: deferred, no facts and no act, with no alert, and the record + # keeps why. Moved again in the next pass, it is the alert; read clean after a move, it is decided. + item = dict(support.ITEM, number=43) + + def left(code="pull_moved"): + return {"facts": {"item": dict(item)}, "title": None, "unread": code} + clean = earlier("2026-10-11T12:50:00Z") + once = passed(reads(pulls=[pr(42), left()], runs=[run()]), previous=[clean]) + alone = passed(reads(pulls=[pr(42)], runs=[run()]), previous=[clean]) + self.assertEqual((decisions(once), once["dispatch"]), (decisions(alone), alone["dispatch"])) + self.assertEqual(once["alerts"], []) + self.assertEqual(once["record"]["left_out"], [{"item": item, "code": "pull_moved"}]) + self.assertEqual(shared.errors(once["record"], "pass"), []) + moved = dict(once["record"], now="2026-10-11T12:55:00Z") + twice = passed(reads(pulls=[pr(42), left()], runs=[run()]), previous=[clean, moved]) + self.assertEqual(twice["alerts"], ["facts_unreadable"]) + settled = passed(reads(pulls=[pr(42), pr(43)], runs=[run()]), previous=[clean, moved]) + self.assertEqual(([d["item"]["number"] for d in decisions(settled)], settled["alerts"]), ([42, 43], [])) + # Any other read left out is the alert at once: one GitHub did not answer, or facts that are not facts. + for entry in (left("facts_unanswered"), {"facts": {"item": dict(item)}, "title": None}, left("anything")): + out = passed(reads(pulls=[pr(42), entry], runs=[run()]), previous=[clean]) + self.assertEqual(out["alerts"], ["facts_unreadable"], entry) + self.assertEqual(out["record"]["left_out"], [{"item": item, "code": "not_facts"}]) + # A last record that cannot be read, none at all, or one written before it kept what it left out, cannot say the + # move is a first: it is the alert. + older = dict(clean) + del older["left_out"] + for previous in (None, (), [older], [dict(clean, now="2026-10-11T12:50:00")]): + out = passed(reads(pulls=[pr(42), left()], runs=[run()]), previous=previous) + self.assertEqual(out["alerts"], ["facts_unreadable"], previous) + # An item listed twice is so whatever its read says; an item that names none is kept as null. + twin = passed(reads(pulls=[left(), left()], runs=[run()]), previous=[clean]) + self.assertEqual((twin["alerts"], [e["code"] for e in twin["record"]["left_out"]]), + (["facts_unreadable"], ["listed_twice"] * 2)) + nameless = passed(reads(pulls=[{"facts": {"item": {"number": 0}}, "unread": "pull_moved"}]), previous=[clean]) + self.assertEqual((nameless["alerts"], nameless["record"]["left_out"]), + (["facts_unreadable"], [{"item": None, "code": "pull_moved"}])) + def test_the_pass_asks_the_kernel_through_its_entry_points_alone(self): # issue 193, ruling 8, amended for 270 # The entry, `__main__.py`, is the one place that reads the clock and the environment (Entry, below). for module in sorted(m for m in (ROOT / "controller").glob("*.py") if m.name != "__main__.py"): @@ -1202,6 +1241,55 @@ def whole(transport, item, kept, identities, apps, since): # And it cannot say what has left, so what it saw is short, though each read of it was whole. self.assertIn("reconciliation_unreadable", files["pass.json"]["alerts"]) + def test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once(self): # issue 658 + # A snapshot that moved while it was read is kept in the record as `pull_moved`, one GitHub did not answer as + # `facts_unanswered`. Moved for the first time, its writes are not read: they are owed to the next pass, and this + # reconciliation is whole. Moved again, or after a record that cannot say, it is read short, as any is. + spied = [] + + def snapshot(raised): + def read(transport, repo, number, identities, who, runs_whole, clock, *, kept, with_head, opener, recorded, + opened): + if number == 8: + raise raised + return support.facts(item=dict(support.ITEM, number=number)), "A title", "a" * 40 + return read + + def read(transport, item, kept, identities, apps, since): + spied.append(item["number"]) + return ([], [], kept is not None) + + def entry_with(raised, records): + del spied[:] + with unittest.mock.patch.object(entry.facts_, "snapshot", snapshot(raised)), \ + unittest.mock.patch.object(entry.passes, "records", lambda *a: records), \ + unittest.mock.patch.object(entry.seen_, "read", read), \ + unittest.mock.patch.object(entry.seen_, "pushes", lambda *a, **k: ([], True)), \ + unittest.mock.patch.object(entry.applied, "effects", + lambda *a: {"applied": [], "read": [], "at": {}, "whole": True}), \ + unittest.mock.patch.object(entry.passes, "written", + lambda *a: {"written": [], "read": [], "at": {}, "whole": True}): + code, _, files, _ = self.run_entry(Hub(self.table(numbers=[7, 8]))) + self.assertEqual(code, 0) + return files["pass.json"] + + before = json.loads((ROOT / "examples" / "pass.json").read_text()) + before.update(now="2026-10-12T11:58:00Z", left_out=[]) + item = {"repo": self.HOME, "type": "pull_request", "number": 8} + once = entry_with(entry.facts_.Moved("moved"), [before]) + self.assertEqual(once["left_out"], [{"item": item, "code": "pull_moved"}]) + self.assertEqual((7 in spied, 8 in spied, once["reconciled"]["owed"]), (True, False, [8])) + self.assertFalse({"facts_unreadable", "reconciliation_unreadable"} & set(once["alerts"])) + older = dict(before) + del older["left_out"] + for previous in ([dict(before, left_out=once["left_out"])], [older]): + again = entry_with(entry.facts_.Moved("moved"), previous) + self.assertEqual((8 in spied, again["reconciled"]["owed"]), (True, [8])) + self.assertLessEqual({"facts_unreadable", "reconciliation_unreadable"}, set(again["alerts"])) + unanswered = entry_with(github.Unanswered("no"), [before]) + self.assertEqual(unanswered["left_out"], [{"item": item, "code": "facts_unanswered"}]) + self.assertLessEqual({"facts_unreadable", "reconciliation_unreadable"}, set(unanswered["alerts"])) + def test_each_snapshot_is_read_with_the_configured_accounts_and_kept(self): # decisions 0005, 0060 handed = [] diff --git a/tools/mutants.py b/tools/mutants.py index cb134af6..4737f355 100644 --- a/tools/mutants.py +++ b/tools/mutants.py @@ -7407,8 +7407,8 @@ def moved_before(rel: str, block: str, anchor: str) -> tuple: ' d = decide(facts, runs, context, kernel, now)\n', ' d = decide(facts, [], context, kernel, now)\n', ('tests.test_controller.Record.test_the_pass_record_holds_what_the_kernel_was_given_and_replays_it',)), ("an issue is asked under the pull request's Lifecycle", 'controller/pass_.py', - 'config["lifecycles"]["issue"]),\n "reconciliation"', - 'config["lifecycles"]["pull_request"]),\n "reconciliation"', ('tests.test_controller.Record.test_an_issue_is_asked_of_the_issues_kernel_with_its_own_lifecycle',)), + 'config["lifecycles"]["issue"]),\n "left_out"', + 'config["lifecycles"]["pull_request"]),\n "left_out"', ('tests.test_controller.Record.test_an_issue_is_asked_of_the_issues_kernel_with_its_own_lifecycle',)), ("an issue's repo is not read for a Pause", 'controller/pass_.py', 'for e in pulls + issues}', 'for e in pulls}', ('tests.test_controller.Record.test_an_issue_is_asked_of_the_issues_kernel_with_its_own_lifecycle',)), @@ -10710,8 +10710,38 @@ def moved_before(rel: str, block: str, anchor: str) -> tuple: ' if not whole:\n raise Stop("pull_requests_unreadable")\n', '', ('tests.test_controller.Entry.test_what_the_entry_cannot_read_writes_nothing',)), ('a snapshot that cannot be read stops the pass', 'controller/__main__.py', - ' except Unanswered:\n pulls.append(', - ' except ZeroDivisionError:\n pulls.append(', ('tests.test_controller.Entry.test_the_entry_reads_through_the_ports_and_hands_the_pass_nothing_else',)), + ' except Unanswered as err:\n', + ' except ZeroDivisionError as err:\n', ('tests.test_controller.Entry.test_the_entry_reads_through_the_ports_and_hands_the_pass_nothing_else',)), + # Issue 658: a pull request that moved while it was read is deferred once with no alert, by the code its record keeps. + ("a pull request that moved once is the alert", 'controller/pass_.py', + ' alarmed = [i for i in left_out if reads_.item(i) is None or key(i) not in graced]', + ' alarmed = left_out', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), + ("a move in the last pass earns the grace again", 'controller/pass_.py', + ' again = {key(e["item"]) for e in held if e["code"] == reads_.MOVED and e["item"] is not None}', + ' again = set()', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), + ("a last record that cannot say grants the grace", 'controller/pass_.py', + ' if not isinstance(held, list):\n return set()\n again', + ' if not isinstance(held, list):\n held = []\n again', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), + ("any read left out earns the grace", 'controller/pass_.py', + ' if code == reads_.MOVED and reads_.item(i) is not None and key(i) not in again}', + ' if reads_.item(i) is not None and key(i) not in again}', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), + ("the record keeps an item that names none", 'controller/pass_.py', + '"left_out": [{"item": reads_.item(i), "code": code}', '"left_out": [{"item": i, "code": code}', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("an item listed twice is left out by its read's code", 'controller/reads.py', + ' why = [TWICE if _named(e) is not None and seen.count(_named(e)) > 1', ' why = [TWICE if False', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("a read's code of any word is kept", 'controller/reads.py', + 'else e["unread"] if isinstance(e, dict) and e.get("unread") in UNREAD else NOT_FACTS', + 'else e.get("unread", NOT_FACTS) if isinstance(e, dict) else NOT_FACTS', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("any dict is an item a record keeps", 'controller/reads.py', + ' return found if (isinstance(found, dict) and set(found) == {"repo", "type", "number"}', + ' return found if (isinstance(found, dict)', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("the entry keeps no code of why a snapshot failed", 'controller/__main__.py', + '"unread": reads_.MOVED if isinstance(err, facts_.Moved) else reads_.UNANSWERED', + '"unread": reads_.UNANSWERED', ('tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once',)), + ("a pull request that moved once is read short", 'controller/__main__.py', + ' if number in owed:\n unread.append(number)\n continue\n', '', ('tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once',)), + ("every pull request left out is owed", 'controller/__main__.py', + 'for e in left if page(e["facts"]["item"]) in graced}', 'for e in left}', ('tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once',)), ('a run id of any form is read', 'controller/__main__.py', ' if not (isinstance(run_id, str) and RUN_ID.fullmatch(run_id)):\n raise Stop("run_unreadable")\n', '', ('tests.test_controller.Entry.test_what_the_entry_cannot_read_writes_nothing',)), From 3d8a447c57979610358d0ff85ef3ba8a2079d8a6 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:05:39 -0600 Subject: [PATCH 2/4] Controller: the grace is a pull request's, a read with no time is no move, and the last record's boundaries are tested (#658, after the reads) An issue left out as pull_moved was graced too, though the grace is a pull request's (Muse). A read whose updated_at could not be read made adapters/pull.py's unchanged false, so a malformed answer was a Moved and earned the grace; it is now a read GitHub did not answer (the security pass, the Claude review). The tests now hold what the last record left out for another reason, another item it left out as moved, and one it could not name (the Claude review). The record says the next pass, scheduled or started by a run's end, reads a moved pull request again, since a comment starts none. A counter-example holds the new field's codes; one older mutant follows the code. Co-Authored-By: Claude Opus 5.5 --- adapters/pull.py | 6 +- controller/pass_.py | 4 +- ...-read-it-is-deferred-once-with-no-alert.md | 7 +- .../pass.a-left-out-code-in-words.json | 383 ++++++++++++++++++ tests/test_adapters.py | 17 + tests/test_controller.py | 11 + tools/mutants.py | 20 +- 7 files changed, 437 insertions(+), 11 deletions(-) create mode 100644 examples/invalid/pass.a-left-out-code-in-words.json diff --git a/adapters/pull.py b/adapters/pull.py index 665d5f8d..4f0a4fc7 100644 --- a/adapters/pull.py +++ b/adapters/pull.py @@ -68,8 +68,10 @@ def unchanged(transport, pull: dict) -> bool: which no trial has asked yet. So a snapshot is never of a pull request that changed between its first read and its last; one that did is read again at the next pass.""" again = read(transport, pull["base"]["repo"]["full_name"], pull["number"]) - return (_same(again, pull) and isinstance(pull.get("updated_at"), str) - and again.get("updated_at") == pull["updated_at"]) + if not (isinstance(pull.get("updated_at"), str) and isinstance(again.get("updated_at"), str)): + # A time that cannot be read is no move: the read GitHub did not answer, not one that earns a pass's grace (#658). + raise Unanswered("the pull request's updated_at cannot be read") + return _same(again, pull) and again["updated_at"] == pull["updated_at"] def _account(actor) -> tuple: diff --git a/controller/pass_.py b/controller/pass_.py index 0f779926..83b07793 100644 --- a/controller/pass_.py +++ b/controller/pass_.py @@ -272,8 +272,8 @@ def moved_once(left_out: list, why: list, before) -> set: if not isinstance(held, list): return set() again = {key(e["item"]) for e in held if e["code"] == reads_.MOVED and e["item"] is not None} - return {key(i) for i, code in zip(left_out, why) - if code == reads_.MOVED and reads_.item(i) is not None and key(i) not in again} + return {key(i) for i, code in zip(left_out, why) if code == reads_.MOVED and reads_.item(i) is not None + and i["type"] == "pull_request" and key(i) not in again} def run(reads: dict, config: dict, now: str, previous, mint) -> dict: diff --git a/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md b/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md index 5ffebb08..802a7bb4 100644 --- a/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md +++ b/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md @@ -7,8 +7,8 @@ Status: accepted. The Factory Builder's ruling of 2026-11-08 on issue 658, "Cont | What | Decided | Because | |---|---|---| | Why an item is left out | `pass/1` keeps `left_out`: each item the pass left out, in the order read, with its code. `pull_moved` is a snapshot that moved while it was read. `facts_unanswered` is any other read the facts port could not make. `not_facts` is facts that do not hold to their schema, and `listed_twice` an item listed twice, whatever its read said. The item is null where none can be named. A record written before it holds none | The cause is seen in the record, not inferred: which read failed, on which item | -| The grace pass | A pull request whose code is `pull_moved`, and which the last pass record read did not leave out as `pull_moved`, is deferred with no alert: no facts, no Decision and no act, as now, but not `facts_unreadable`. Its writes are not read in this pass. They are owed to the next, as a pull request read short is, so this reconciliation stays whole. The pass its change started, through `workflow_run`, reads it again | A merge, a push or a comment during a read is the Factory working, not a fault. A person assigned to an issue that closes itself minutes later learns nothing from it | -| What still alerts at once | A second move in a row. Any other code. A last record that cannot be read, that is not there, or that holds no `left_out`: it cannot say the move is a first, so the move counts as a second | Something that never settles still reaches a person, and only `Moved` earns the grace. A pass fails closed | +| The grace pass | A pull request whose code is `pull_moved`, and which the last pass record read did not leave out as `pull_moved`, is deferred with no alert: no facts, no Decision and no act, as now, but not `facts_unreadable`. Its writes are not read in this pass. They are owed to the next, as a pull request read short is, so this reconciliation stays whole. The next pass reads it again: one a run's end starts, or the next scheduled one, at most twenty minutes on | A merge, a push or a comment during a read is the Factory working, not a fault. A person assigned to an issue that closes itself minutes later learns nothing from it | +| What still alerts at once | A second move in a row. Any other code. An issue left out as moved: the grace is a pull request's. A read whose `updated_at` cannot be read is no move: `adapters/pull.py` raises it as a read GitHub did not answer, `facts_unanswered`. A last record that cannot be read, that is not there, or that holds no `left_out`: it cannot say the move is a first, so the move counts as a second | Something that never settles still reaches a person, and only `Moved` earns the grace. A pass fails closed | | Derived, never kept | Whether a move is a first is read from the last pass record at each pass, in `controller/pass_.py`'s `moved_once`. The entry asks the same of it to leave the owed writes unread | State is derived from facts and run records at every call, never stored | | Not here | `adapters/checks.py` still raises a plain `Unanswered` when the head moves while the checks are read. That read shows as `facts_unanswered` and alerts at once. Making it a `Moved` would also change how the approving act classes it | The record's codes will show whether it happens. It is a change of its own | @@ -20,5 +20,6 @@ The tracking issue is how the Factory's faults reach the Maintainer. Seven alert | Boundary | Check | |---|---| -| A pull request that moved once is deferred with no alert, and its record keeps `pull_moved`. One that moved twice in a row is the alert. One read clean after a move is decided. Any other code is the alert at once, and so is a move after a last record that cannot be read, is not there or holds no `left_out`. An item listed twice is `listed_twice`, and one that names no item is kept as null | `tests/test_controller.py::test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert`, `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once`, `controller/pass_.py`, `controller/reads.py`, `schemas/pass.schema.json`, mutant: "a pull request that moved once is the alert", mutant: "a move in the last pass earns the grace again", mutant: "a last record that cannot say grants the grace", mutant: "any read left out earns the grace", mutant: "the record keeps an item that names none", mutant: "an item listed twice is left out by its read's code", mutant: "a read's code of any word is kept", mutant: "any dict is an item a record keeps" | +| A pull request that moved once is deferred with no alert, and its record keeps `pull_moved`. One that moved twice in a row is the alert. A last record that left it out for another reason, left another item out as moved, or left out one it could not name, makes the move a first. An issue left out as moved is the alert. One read clean after a move is decided. Any other code is the alert at once, and so is a move after a last record that cannot be read, is not there or holds no `left_out`. An item listed twice is `listed_twice`, and one that names no item is kept as null | `tests/test_controller.py::test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert`, `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once`, `controller/pass_.py`, `controller/reads.py`, `schemas/pass.schema.json`, mutant: "a pull request that moved once is the alert", mutant: "an issue that moved earns the grace", mutant: "what the last record left out for any reason counts as a move", mutant: "a nameless move in the last record is read as an item", mutant: "a move in the last pass earns the grace again", mutant: "a last record that cannot say grants the grace", mutant: "any read left out earns the grace", mutant: "the record keeps an item that names none", mutant: "an item listed twice is left out by its read's code", mutant: "a read's code of any word is kept", mutant: "any dict is an item a record keeps" | | The entry keeps a moved snapshot as `pull_moved` and any other as `facts_unanswered`. A first move's writes are owed and not read, so reconciliation is whole. A second move's, or one after a record that cannot say, are read short | `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once`, `controller/__main__.py`, mutant: "the entry keeps no code of why a snapshot failed", mutant: "a pull request that moved once is read short", mutant: "every pull request left out is owed" | +| A read whose `updated_at` cannot be read, the first or the last, is a read GitHub did not answer, never a move | `tests/test_adapters.py::test_a_pull_request_whose_updated_at_cannot_be_read_has_not_moved`, `adapters/pull.py`, mutant: "a read with no time is a move" | diff --git a/examples/invalid/pass.a-left-out-code-in-words.json b/examples/invalid/pass.a-left-out-code-in-words.json new file mode 100644 index 00000000..6b45ed7a --- /dev/null +++ b/examples/invalid/pass.a-left-out-code-in-words.json @@ -0,0 +1,383 @@ +{ + "schema": "pass/1", + "pass": "p_01JA7P", + "release": "factory/v0.3.1", + "now": "2026-10-11T13:00:00Z", + "started_by": "own_repo", + "context": { + "paused": false, + "active_runs": 0, + "model_deferred_until": null, + "passes_since": "2026-10-11T12:55:00Z", + "unidentified_runs": [] + }, + "runs": [ + { + "run": "r_01JA6Z", + "item": { + "repo": "combsmith/software-factory", + "type": "pull_request", + "number": 42 + }, + "role": "pr-review", + "intent": "review-change", + "content_id": "sha256:5d0c", + "status": "completed", + "conclusion": "success", + "outcome": null, + "started_at": "2026-10-11T12:05:00Z" + } + ], + "reads": [ + { + "read": "runs", + "date": "2026-10-11T12:59:58Z", + "age": 0 + }, + { + "read": "facts", + "date": "2026-10-11T12:59:59Z", + "age": 0 + } + ], + "pull_requests": [ + { + "facts": { + "schema": "facts/1", + "item": { + "repo": "combsmith/software-factory", + "type": "pull_request", + "number": 42 + }, + "state": "open", + "is_draft": false, + "class": "maintainer", + "author": "don-petry", + "last_pusher": "don-petry", + "targets_protected": true, + "mergeable": true, + "content_id": "sha256:5d0c", + "diff": { + "files": 3, + "lines": 120, + "truncated": false, + "characters": 4800 + }, + "changed_paths": [ + { + "path": "kernel/derive.py", + "previous": "kernel/derive_old.py" + }, + { + "path": "tests/test_decide.py", + "previous": null + }, + { + "path": "docs/glossary.md", + "previous": null + } + ], + "label_events": [ + { + "label": "factory:hold", + "action": "labeled", + "by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "at": "2026-10-11T11:40:00Z" + }, + { + "label": "factory:hold", + "action": "unlabeled", + "by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "at": "2026-10-11T11:45:00Z" + } + ], + "last_push_at": "2026-10-11T12:00:00Z", + "last_reopened_at": null, + "description_edited_at": null, + "expired_reviews": [], + "last_writer_action_at": "2026-10-11T12:00:00Z", + "reviews": [ + { + "by": { + "kind": "machine_user", + "name": "donpetry-bot", + "can_write": true + }, + "state": "dismissed", + "commit": "4b1a9c0", + "content_id": "sha256:4b1a", + "submitted_at": "2026-10-11T11:50:00Z", + "dismissed_by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "dismissed_from": "approved" + }, + { + "by": { + "kind": "human", + "name": "a-colleague", + "can_write": true + }, + "state": "changes_requested", + "commit": "5d0c7e2", + "content_id": "sha256:5d0c", + "submitted_at": "2026-10-11T12:18:00Z", + "dismissed_by": null, + "dismissed_from": null + } + ], + "threads": [ + { + "id": "PRRT_kw7", + "resolved": false, + "opened_by": { + "kind": "lens", + "name": "pr-review", + "can_write": true + }, + "category": "functional", + "severity": "major", + "resolved_by": null, + "opened_at": "2026-10-11T12:15:00Z", + "ruling": null + }, + { + "id": "PRRT_kw1", + "resolved": false, + "opened_by": { + "kind": "lens", + "name": "coderabbit", + "can_write": false + }, + "category": null, + "severity": null, + "resolved_by": null, + "opened_at": "2026-10-11T12:15:00Z", + "ruling": null + }, + { + "id": "PRRT_kw3", + "resolved": true, + "opened_by": { + "kind": "human", + "name": "a-colleague", + "can_write": true + }, + "category": "human", + "severity": null, + "resolved_by": { + "kind": "human", + "name": "a-colleague", + "can_write": true + }, + "opened_at": "2026-10-11T12:16:00Z", + "ruling": null + } + ], + "checks": [ + { + "name": "verify", + "app_id": 15368, + "workflow": ".github/workflows/verify.yml", + "conclusion": "success", + "completed_at": "2026-10-11T12:06:00Z" + }, + { + "name": "Secret scan (gitleaks)", + "app_id": 15368, + "workflow": ".github/workflows/ci.yml", + "conclusion": "success", + "completed_at": "2026-10-11T12:06:00Z" + }, + { + "name": "CodeQL", + "app_id": 57789, + "workflow": null, + "conclusion": "success", + "completed_at": "2026-10-11T12:06:00Z" + } + ], + "queue": false, + "acts_since_writer": 2, + "acceptance": null, + "complete": { + "acceptance": true, + "reviews": true, + "threads": true, + "checks": true, + "diff": true, + "changed_paths": true, + "runs": true, + "timeline": true + }, + "read_at": "2026-10-11T12:20:00Z" + }, + "decision": { + "schema": "decision/1", + "item": { + "repo": "combsmith/software-factory", + "type": "pull_request", + "number": 42 + }, + "state": "addressing", + "reason": "human_finding", + "evidence": [ + "PRRT_kw7", + "a-colleague" + ], + "attention": "needs_you", + "outcome": "admit", + "admit": { + "role": "dev-lead", + "intent": "fix-findings", + "content_id": "sha256:5d0c" + } + }, + "run": "r_01JA7Q", + "head": "fc6f2b03e078f5d8de03dfaa142afbd00dec35ea" + } + ], + "issues": [ + { + "facts": { + "schema": "issue-facts/1", + "item": { + "repo": "combsmith/software-factory", + "type": "issue", + "number": 41 + }, + "state": "open", + "closed_first_at": null, + "author": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "type": "Task", + "label_names": [ + "kernel", + "factory:triage" + ], + "assignees": [ + "don-petry" + ], + "life_started_at": "2026-10-11T09:00:00Z", + "reopened_last_by": "never", + "reopened_last_at": null, + "content_id": "sha256:41c1", + "content_changed_at": "2026-10-11T09:30:00Z", + "label_events": [ + { + "label": "factory:triage", + "action": "labeled", + "by": { + "kind": "app", + "name": "combsmith-sf-controller[bot]", + "can_write": true + }, + "at": "2026-10-11T09:31:00Z" + } + ], + "linked": [ + { + "number": 146, + "class": "agent", + "opener": { + "kind": "app", + "name": "combsmith-sf-dev-lead[bot]", + "can_write": true + }, + "open": true, + "draft": true, + "opened_at": "2026-10-11T09:40:00Z", + "merged_at": null, + "closed_at": null, + "empty": false, + "label_events": [], + "closed_issue_at": null, + "draft_events": [] + }, + { + "number": 140, + "class": "outside", + "opener": { + "kind": "human", + "name": "stranger", + "can_write": false + }, + "open": false, + "draft": false, + "opened_at": "2026-10-11T09:35:00Z", + "merged_at": null, + "closed_at": "2026-10-11T10:00:00Z", + "empty": false, + "label_events": [ + { + "label": "factory:admit", + "action": "labeled", + "by": { + "kind": "human", + "name": "don-petry", + "can_write": true + }, + "at": "2026-10-11T09:45:00Z" + } + ], + "closed_issue_at": null, + "draft_events": [] + } + ], + "role_comments": [ + { + "by": "combsmith-sf-controller[bot]", + "at": "2026-10-11T09:31:00Z" + } + ], + "last_writer_action_at": "2026-10-11T09:30:00Z", + "last_writer_comment_at": null, + "acts_since_writer": 1, + "branch_head": "9f2c41d07a3be58e1c6d0b4a7e25f3c18d90ab6e", + "complete": { + "timeline": true, + "linked": true, + "runs": true + }, + "read_at": "2026-10-11T12:00:00Z" + }, + "decision": { + "schema": "issue-decision/1", + "item": { + "repo": "combsmith/software-factory", + "type": "issue", + "number": 41 + }, + "state": "not_served", + "reason": "left_out", + "evidence": [], + "attention": "none", + "outcome": "nothing" + }, + "run": null + } + ], + "left_out": [ + { + "item": { + "repo": "combsmith/software-factory", + "type": "pull_request", + "number": 43 + }, + "code": "moved" + } + ] +} diff --git a/tests/test_adapters.py b/tests/test_adapters.py index 9349abf4..483728ec 100644 --- a/tests/test_adapters.py +++ b/tests/test_adapters.py @@ -3381,6 +3381,23 @@ def test_a_snapshot_of_a_pull_request_that_changed_while_it_was_read_is_refused( with self.assertRaises(facts.Moved): self.read(6) + def test_a_pull_request_whose_updated_at_cannot_be_read_has_not_moved(self): # issue 658 + # Either read without a time is a read GitHub did not answer, not a move: it earns no pass's grace. + first = body("P-29.base", 1) + + class Answers: + def __init__(self, answer): + self.answer = answer + + def send(self, request): + return {"status": 200, "headers": {}, "body": copy.deepcopy(self.answer)} + for was, again in ((dict(first, updated_at=None), first), (first, dict(first, updated_at=None))): + with self.assertRaises(github.Unanswered) as raised: + pull.unchanged(Answers(again), was) + self.assertNotIsInstance(raised.exception, facts.Moved) + self.assertTrue(pull.unchanged(Answers(first), first)) + self.assertFalse(pull.unchanged(Answers(dict(first, updated_at="2026-10-11T13:00:00Z")), first)) + def test_a_push_that_names_its_pusher_and_no_time_is_refused(self): seq = self.sequence(1) entry = seq[2]["response"]["body"][0] diff --git a/tests/test_controller.py b/tests/test_controller.py index 07dc0c94..16f2bfe6 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -304,8 +304,19 @@ def left(code="pull_moved"): moved = dict(once["record"], now="2026-10-11T12:55:00Z") twice = passed(reads(pulls=[pr(42), left()], runs=[run()]), previous=[clean, moved]) self.assertEqual(twice["alerts"], ["facts_unreadable"]) + # What the last record left out for another reason, or another item it left out as moved, or one it could not + # name, makes this move a first (the Claude review). + for held in ([{"item": item, "code": "facts_unanswered"}], [{"item": dict(item, number=44), "code": "pull_moved"}], + [{"item": None, "code": "pull_moved"}]): + last = dict(clean, now="2026-10-11T12:55:00Z", left_out=held) + out = passed(reads(pulls=[pr(42), left()], runs=[run()]), previous=[clean, last]) + self.assertEqual(out["alerts"], [], held) settled = passed(reads(pulls=[pr(42), pr(43)], runs=[run()]), previous=[clean, moved]) self.assertEqual(([d["item"]["number"] for d in decisions(settled)], settled["alerts"]), ([42, 43], [])) + # An issue left out as moved earns no grace: the grace is a pull request's (Muse). + issue_moved = {"facts": {"item": dict(test_issue.ITEM, number=7)}, "title": None, "unread": "pull_moved"} + out = passed(reads(pulls=[pr(42)], issues=[issue_moved], runs=[run()]), previous=[clean]) + self.assertEqual(out["alerts"], ["facts_unreadable"]) # Any other read left out is the alert at once: one GitHub did not answer, or facts that are not facts. for entry in (left("facts_unanswered"), {"facts": {"item": dict(item)}, "title": None}, left("anything")): out = passed(reads(pulls=[pr(42), entry], runs=[run()]), previous=[clean]) diff --git a/tools/mutants.py b/tools/mutants.py index 4737f355..778345fb 100644 --- a/tools/mutants.py +++ b/tools/mutants.py @@ -3518,8 +3518,8 @@ def moved_before(rel: str, block: str, anchor: str) -> tuple: ' if pusher is not None and pushed_at is None:\n', ' if False:\n', ('tests.test_adapters',)), ('a change that kept the head is not noticed', 'adapters/pull.py', - ' and again.get("updated_at") == pull["updated_at"])\n', - ' )\n', ('tests.test_adapters',)), + ' return _same(again, pull) and again["updated_at"] == pull["updated_at"]\n', + ' return _same(again, pull)\n', ('tests.test_adapters',)), ('a workflow run that is no object is read as none', 'adapters/checks.py', ' if not (run is None or isinstance(run, dict)) or not (file is None or isinstance(file, dict)):\n', ' if False:\n', ('tests.test_adapters',)), @@ -10723,8 +10723,20 @@ def moved_before(rel: str, block: str, anchor: str) -> tuple: ' if not isinstance(held, list):\n return set()\n again', ' if not isinstance(held, list):\n held = []\n again', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), ("any read left out earns the grace", 'controller/pass_.py', - ' if code == reads_.MOVED and reads_.item(i) is not None and key(i) not in again}', - ' if reads_.item(i) is not None and key(i) not in again}', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), + 'if code == reads_.MOVED and reads_.item(i) is not None\n', + 'if reads_.item(i) is not None\n', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert', 'tests.test_controller.Entry.test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once')), + ("an issue that moved earns the grace", 'controller/pass_.py', + ' and i["type"] == "pull_request" and key(i) not in again}', ' and key(i) not in again}', + ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("what the last record left out for any reason counts as a move", 'controller/pass_.py', + 'for e in held if e["code"] == reads_.MOVED and e["item"] is not None}', 'for e in held if e["item"] is not None}', + ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("a nameless move in the last record is read as an item", 'controller/pass_.py', + 'for e in held if e["code"] == reads_.MOVED and e["item"] is not None}', 'for e in held if e["code"] == reads_.MOVED}', + ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), + ("a read with no time is a move", 'adapters/pull.py', + ' if not (isinstance(pull.get("updated_at"), str) and isinstance(again.get("updated_at"), str)):\n', + ' if False:\n', ('tests.test_adapters.Snapshot.test_a_pull_request_whose_updated_at_cannot_be_read_has_not_moved',)), ("the record keeps an item that names none", 'controller/pass_.py', '"left_out": [{"item": reads_.item(i), "code": code}', '"left_out": [{"item": i, "code": code}', ('tests.test_controller.Record.test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert',)), ("an item listed twice is left out by its read's code", 'controller/reads.py', From 268d255914dd69af239c7c3d18ea9b64b78d1799 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:19:24 -0600 Subject: [PATCH 3/4] Controller: the moved pull request's rule joins the left-out row, so controller/AGENTS.md stays at 40 lines (#658) Co-Authored-By: Claude Opus 5.5 --- controller/AGENTS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controller/AGENTS.md b/controller/AGENTS.md index e066c6b8..3b7ec39e 100644 --- a/controller/AGENTS.md +++ b/controller/AGENTS.md @@ -11,8 +11,7 @@ Not built yet, by the stories that hold their rules: relay.py, backpressure.py, | The pass record holds what the Kernel was given, and replays every Decision | `tests/test_controller.py::test_the_pass_record_holds_what_the_kernel_was_given_and_replays_it` | | A Run id is minted for an admission, an approval, or a queue act once its workflow is named | `tests/test_controller.py::test_a_queue_act_gets_a_run_id_and_a_dispatch_once_its_workflow_is_named` | | A Run is dispatched by its run-name; a capacity fault defers model Runs for its window | `tests/test_controller.py::test_the_runs_to_dispatch_are_named_by_their_run_names`, `tests/test_controller.py::test_a_capacity_fault_defers_model_runs_for_its_window` | -| An item whose facts are not facts, or listed twice, is left out, as one alert; any other is decided as without it | `tests/test_controller.py::test_an_item_whose_facts_are_not_facts_is_left_out_and_raises_one_alert` | -| A pull request that moved while it was read, and not in the last pass, is deferred with no alert and its writes owed; a second move, any other read left out, or a last record that cannot say, is the alert; the record keeps each one's code | `tests/test_controller.py::test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert`, `tests/test_controller.py::test_a_snapshot_that_moved_is_kept_by_its_code_and_its_writes_are_owed_once` | +| An item whose facts are not facts, or listed twice, is left out as one alert; a pull request moved once, with none; the rest are decided | `tests/test_controller.py::test_a_pull_request_that_moved_while_read_is_deferred_once_with_no_alert` | | An issue is asked of the issues' Kernel, under its own Lifecycle, and its repo's Pause is read | `tests/test_controller.py::test_an_issue_is_asked_of_the_issues_kernel_with_its_own_lifecycle` | | A record that cannot validate is refused whole, and nothing is dispatched | `tests/test_controller.py::test_a_record_that_cannot_validate_is_refused_and_nothing_is_dispatched` | | A Pause on the org or a repo is a Pause; a value but exact `true`, exact `false` or none is unreadable | `tests/test_controller.py::test_a_pause_on_the_org_or_the_repo_is_a_pause_and_any_other_value_is_unreadable` | From 31aa4a1a41f344b3901013ee4d9cfe33696de0f2 Mon Sep 17 00:00:00 2001 From: Don Petry <36422719+don-petry@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:48:40 -0600 Subject: [PATCH 4/4] Decision 0172: the grace pass's record takes its number (#658) Co-Authored-By: Claude Opus 5.5 --- ...oved-while-a-pass-read-it-is-deferred-once-with-no-alert.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/decisions/{NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md => 0172-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md} (98%) diff --git a/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md b/docs/decisions/0172-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md similarity index 98% rename from docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md rename to docs/decisions/0172-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md index 802a7bb4..77396819 100644 --- a/docs/decisions/NNNN-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md +++ b/docs/decisions/0172-a-pull-request-that-moved-while-a-pass-read-it-is-deferred-once-with-no-alert.md @@ -1,4 +1,4 @@ -# NNNN. A pull request that moved while a pass read it is deferred once with no alert +# 0172. A pull request that moved while a pass read it is deferred once with no alert Status: accepted. The Factory Builder's ruling of 2026-11-08 on issue 658, "Controller: a pull request that changes while a pass reads it raises facts_unreadable and reconciliation_unreadable, though Moved means read it again". From 2026-11-08 05:38Z, Factory alerts were opened seven times and closed within minutes. Each time, one pull request had merged, taken a commit or taken a comment less than a minute before the pass read it. The snapshot raised `Moved`, which means "read it again at the next pass". The entry took it as any read GitHub did not answer, so it raised `facts_unreadable`, and the item's writes, never read, left reconciliation short. It adds a grace pass and amends nothing else: decision 0056's tracking issue still opens on a pass's first cause.