Verify the Resolved Head on handoff.py's Read Path

scripts/README.md

@@ -261,7 +261,7 @@ The chain is machine-readable from one HTML comment on the body's last line, `<!
 
 The invariant is one open handoff per track per repository, read from the metadata block rather than from the label, since an issue carrying the label and no block belongs to no readable track. Zero open is two states rather than one, a track that has never had a handoff and a lane closed out whose newest link is closed, and `new` chains onto the newest link either way: reading the second as the first would file round 1 with `previous=none` beside a chain that already exists, orphaning every link already written. Two or more open on one track is a defect to report rather than one to resolve by picking, so every subcommand that would have to choose refuses and names both. An open handoff carrying no metadata block refuses the same way, since its track is unknown and it could be the one the caller asked about: settling that is a hand edit, adding the block to the issue's body or taking the label off it, since no read can place an issue on a track until its own block says which track that is, and `tracks` surveys without refusing, which is what makes an abandoned lane visible rather than silently current.
 
-`chain --grep` takes a regular expression rather than a literal, and it searches only the links it walks, which is one track's `previous=` chain up to `--limit`. A walk that stopped at the cap names the link it stopped short of, because a capped search reading exactly like an exhaustive one produces the false "not tried yet" this whole mechanism exists to prevent.
+`chain --grep` takes a regular expression rather than a literal, and it searches only the links it walks, which is one track's `previous=` chain up to `--limit`. A walk that stopped at the cap names the link it stopped short of, because a capped search reading exactly like an exhaustive one produces the false "not tried yet" this whole mechanism exists to prevent. A head the chain has moved past is named the same way. Another link may already name it as predecessor, or hold a round at or above its own. `current`, `resume`, and `chain` then print a notice naming that link, and `new` refuses onto it. A walk that ended cleanly also names each link on the track it did not reach.
 
 `new` creates the new issue, comments the forward link on the previous one, and then closes it, in that order, printing each step's own result. Creating first means a failure at any later step leaves a discoverable new issue rather than a closed chain with no successor, and `link` finishes what a failure interrupted without a second issue being filed for it. `link` takes no `--track`, so the two issues' own metadata blocks are the only thing that can say they belong to one lane, and it refuses an issue named as its own predecessor and a pair whose blocks name different tracks rather than closing one lane's handoff into another's. Nothing suppresses a write's output or converts a failure into success, per [GOVERNANCE.md][governance] "Repository Boundaries and Write Safety". Where the caller names an issue, which is `link` alone, that issue is read live before the write and only what the read returned is written back, and every other identifier a write consumes comes from a read in the same run rather than being constructed. A close is confirmed by reading the state back, because a write that appears to have failed may have succeeded on the server.
 

scripts/handoff.py

@@ -22,6 +22,11 @@
 either way. Two or more open on one track is a defect to report, never one to resolve by picking,
 so every command that would have to choose refuses and names both.
 
+A head resolved that way can still be one the chain has moved past, where another link already
+names it as predecessor, holds a higher round, or holds the same round. `new` refuses onto such a
+head, and `current`, `resume`, and `chain` answer from it with a notice naming each such link,
+since answering from it silently is how a search reads "no match" over text in the lane.
+
 Subcommands
   current  The open handoff issue for a track, number and URL. Read-only.
   resume   That issue's body, then a one-line index of the last N closed links. Read-only.
@@ -559,7 +564,8 @@ def on_track(rows: list[dict], track: str) -> dict | None:
     return matches[0] if matches else None
 
 
-def current_or_refuse(repo: str, track: str) -> dict:
+def current_or_refuse(repo: str, track: str) -> tuple[dict, list[str]]:
+    """The open handoff on `track`, with each reason `head_doubts` gives that it is not the head."""
     rows = open_handoffs(repo)
     require_marked(rows)
     found = on_track(rows, track)
@@ -568,7 +574,7 @@ def current_or_refuse(repo: str, track: str) -> dict:
             f"{repo} has no open handoff on track {track!r}. Zero is the state before the first "
             "handoff on a track, so `new` is what follows, not a retry of this."
         )
-    return found
+    return found, head_doubts(found, track_links(repo, track, rows))
 
 
 def walk(repo: str, start: int, limit: int, cap: str) -> tuple[list[dict], str | None]:
@@ -774,16 +780,18 @@ def edit_body(repo: str, number: int, body: str, dry_run: bool) -> None:
 
 
 def cmd_current(a: argparse.Namespace) -> int:
-    found = current_or_refuse(a.repo, a.track)
+    found, doubts = current_or_refuse(a.repo, a.track)
     marker = found["marker"]
     print(f"#{found['number']} {found['url']}")
     print(f"track={marker['track']} round={marker['round']} previous={marker['previous']}")
     print(found["title"])
+    if doubts:
+        print(doubt_notice(found, a.track, doubts))
     return 0
 
 
 def cmd_resume(a: argparse.Namespace) -> int:
-    found = current_or_refuse(a.repo, a.track)
+    found, doubts = current_or_refuse(a.repo, a.track)
     print(f"# {found['title']}")
     print(f"# {found['url']}")
     print()
@@ -794,12 +802,14 @@ def cmd_resume(a: argparse.Namespace) -> int:
     print(f"## Chain history, last {a.history} closed links")
     if previous is None:
         print("(none - this is the first handoff on this track)")
-        return 0
-    links, stopped = walk(a.repo, previous, a.history, "--history")
-    for link in links:
-        print(describe(link))
-    if stopped is not None:
-        print(f"(index stopped at {stopped})")
+    else:
+        links, stopped = walk(a.repo, previous, a.history, "--history")
+        for link in links:
+            print(describe(link))
+        if stopped is not None:
+            print(f"(index stopped at {stopped})")
+    if doubts:
+        print(doubt_notice(found, a.track, doubts))
     return 0
 
 
@@ -816,6 +826,8 @@ def cmd_chain(a: argparse.Namespace) -> int:
             f"{a.repo} has no handoff on track {a.track!r}, open or closed, so there is no chain "
             "to walk."
         )
+    lane = track_links(a.repo, a.track, rows)
+    doubts = head_doubts(head, lane)
     links, stopped = walk(a.repo, head["number"], a.limit, "--limit")
     shown = 0
     for link in links:
@@ -834,6 +846,18 @@ def cmd_chain(a: argparse.Namespace) -> int:
             f"(the walk stopped at {stopped}, so it read {len(links)} link(s) rather than the "
             "chain, and a search over it is not a search over the chain)"
         )
+    if doubts:
+        print(doubt_notice(head, a.track, doubts))
+    walked = {int(link["number"]) for link in links}
+    missed = [row for row in lane.links if int(row["number"]) not in walked]
+    if stopped is None and missed:
+        listed = ", ".join(f"#{row['number']}" for row in missed)
+        print(
+            cut(
+                f"(the walk did not reach {listed}, on track {a.track!r} and outside the chain "
+                "walked, so a search over it is not a search over the track)"
+            )
+        )
     return 0
 
 
@@ -975,13 +999,13 @@ def cmd_new(a: argparse.Namespace) -> int:
     # The check runs before the create rather than after it.
     # A refusal that leaves an issue behind is a refusal that changed something.
     if previous is not None:
-        taken = successor_of(a.repo, a.track, int(previous["number"]), 0)
-        if taken is not None:
+        doubts = head_doubts(previous, track_links(a.repo, a.track, rows))
+        if doubts:
             raise Refusal(
-                f"#{taken['number']} already succeeds #{previous['number']} on track "
-                f"{a.track!r}, so filing another link onto it would fork the chain there and "
-                f"leave #{taken['number']} unreachable. That link is the track's head rather "
-                "than this one."
+                cut(
+                    f"#{previous['number']} may not be the head of track {a.track!r}, so filing "
+                    f"another link onto it could fork the chain there. {' '.join(doubts)}"
+                )
             )
     title = TITLE.format(track=a.track, subject=a.title)
     print(f"1. create the new handoff on track {a.track!r}, round {round_}")
@@ -1005,11 +1029,20 @@ def cmd_new(a: argparse.Namespace) -> int:
     return 0
 
 
-def successor_of(repo: str, track: str, number: int, ignore: int) -> dict | None:
-    """The link on `track` that already names `number` as its predecessor, or None.
+class Lane(typing.NamedTuple):
+    """Every readable link on one track, and what kept that read from being whole."""
 
-    Read from the open and the closed side together, since a predecessor's successor is closed as
-    soon as the round after it is filed, so the closed side is where it usually sits.
+    links: list[dict]
+    unreadable: list[str]
+    full: bool
+
+
+def track_links(repo: str, track: str, rows: list[dict]) -> Lane:
+    """The links on `track`, the open ones taken from `rows` and the closed ones read here.
+
+    A row whose block is absent or cannot be read could be on this track, so it is named in
+    `unreadable` rather than skipped, and a closed read that fills its window sets `full`, since a
+    link past it is unseen. Each caller decides whether those refuse or are reported.
     """
     closed = gh_json(
         [
@@ -1028,17 +1061,9 @@ def successor_of(repo: str, track: str, number: int, ignore: int) -> dict | None
         ]
     )
     closed = rows_of(closed, f"closed handoff list for {repo}", "number")
-    if len(closed) >= CLOSED_WINDOW:
-        raise Refusal(
-            f"{repo} has at least {CLOSED_WINDOW} closed `{LABEL}` issues, which fills the read "
-            f"window, so a link already succeeding #{number} could sit past it. Pointing a second "
-            "link at it would fork the chain there unseen."
-        )
+    links: list[dict] = []
     unreadable: list[str] = []
-    bare: list[str] = []
-    for row in [*open_handoffs(repo), *closed]:
-        if row["number"] == ignore:
-            continue
+    for row in [*rows, *closed]:
         marker = row.get("marker")
         if marker is None:
             try:
@@ -1047,20 +1072,75 @@ def successor_of(repo: str, track: str, number: int, ignore: int) -> dict | None
                 unreadable.append(str(exc))
                 continue
         if marker is None:
-            # `newest_closed` holds these two to be the same hazard, and so does this.
-            # An absent block leaves what the link succeeds unknown, as an unreadable one does.
-            # Reading either as "not the successor" is a guess.
-            bare.append(
-                f"#{row['number']} carries the `{LABEL}` label and no metadata block, so what it "
-                "succeeds cannot be read. Add one to its body by hand, or take the label off it."
+            unreadable.append(
+                f"#{row['number']} carries the `{LABEL}` label and no metadata block, so its "
+                "track cannot be read. Add one to its body by hand, or take the label off it."
             )
             continue
-        if marker["track"] == track and marker["previous"] == str(number):
+        if marker["track"] == track:
             row["marker"] = marker
+            links.append(row)
+    return Lane(links, unreadable, len(closed) >= CLOSED_WINDOW)
+
+
+def head_doubts(head: dict, lane: Lane) -> list[str]:
+    """Each reason `head` may not be its track's head, one sentence apiece, empty where none.
+
+    `on_track` takes an open link whatever round it holds, and `newest_closed` breaks a tie on one
+    round by issue number, so either can resolve a link the chain has already moved past. Three
+    states show it: a link already naming it as predecessor, a link at a higher round, and another
+    link at its own round. Answering from such a head is how a search reads "no match" over text
+    in the lane, so the reads print these and `new` refuses on them.
+    """
+    number = int(head["number"])
+    round_ = int(head["marker"]["round"])
+    doubts: list[str] = []
+    for row in lane.links:
+        if int(row["number"]) == number:
+            continue
+        other = int(row["marker"]["round"])
+        if row["marker"]["previous"] == str(number):
+            doubts.append(f"#{row['number']} already succeeds #{number}.")
+        elif other > round_:
+            doubts.append(f"#{row['number']} holds round {other}, above #{number}'s {round_}.")
+        elif other == round_:
+            doubts.append(f"#{row['number']} also holds round {round_}.")
+    if lane.full:
+        doubts.append(
+            f"The closed `{LABEL}` issues fill the read window of {CLOSED_WINDOW}, so a link past "
+            "it went unchecked."
+        )
+    doubts.extend(lane.unreadable)
+    return doubts
+
+
+def doubt_notice(head: dict, track: str, doubts: list[str]) -> str:
+    """The line a read prints over a head `head_doubts` questioned, bounded like a refusal."""
+    return cut(
+        f"(#{head['number']} may not be the head of track {track!r}, so this read may miss the "
+        f"chain's newest links. {' '.join(doubts)})"
+    )
+
+
+def successor_of(repo: str, track: str, number: int, ignore: int) -> dict | None:
+    """The link on `track` that already names `number` as its predecessor, or None.
+
+    Read from the open and the closed side together, since a predecessor's successor is closed as
+    soon as the round after it is filed, so the closed side is where it usually sits.
+    """
+    lane = track_links(repo, track, open_handoffs(repo))
+    if lane.full:
+        raise Refusal(
+            f"{repo} has at least {CLOSED_WINDOW} closed `{LABEL}` issues, which fills the read "
+            f"window, so a link already succeeding #{number} could sit past it. Pointing a second "
+            "link at it would fork the chain there unseen."
+        )
+    for row in lane.links:
+        if row["number"] != ignore and row["marker"]["previous"] == str(number):
             return row
-    if unreadable or bare:
+    if lane.unreadable:
         raise Refusal(
-            cut(" ".join([*unreadable, *bare]))
+            cut(" ".join(lane.unreadable))
             + f" One of those could already succeed #{number}, so whether pointing a second link "
             "at it forks the chain cannot be read."
         )