Skip to content

chore: review-ui route never reads the text verdict, so its prescribed clause can assert a PASS that is not there #9310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
usirin merged 3 commits into from
Sep 16, 2026
Merged

chore: review-ui route never reads the text verdict, so its prescribed clause can assert a PASS that is not there #9310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc3656a
feat(review-ui): route reads the text verdict its clause asserts (#9196)
usirin Sep 16, 2026
4e2d309
fix(review-ui): the record's id is free and the contract states the o…
usirin Sep 16, 2026
b33ef70
fix(review-ui): one advisory polarity predicate, so a [FAIL] advisory…
usirin Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions .decisions/0397-a-tuval-route-rests-on-a-standing-text-pass.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
---
id: 0397
title: A routed-elsewhere record rests on the text review it asserts
status: accepted
date: 2026-09-16
tags: [fabrika, review-ui, tuval, pipeline, governance]
---

# 0397 — A routed-elsewhere record rests on the text review it asserts

**What this decides:** the `review-code` verdict is a precondition of a `review-ui route`, not
commentary on it. `review-ui route` reads the verdict in force at `--sha` and refuses on a standing
FAIL; under the interim Tuval exception, where the route rests on a hand-verification, it refuses an
absent verdict too. Founder ruling on
[#9196](https://github.com/kamp-us/phoenix/issues/9196), 2026-09-15:
[the ruling comment](https://github.com/kamp-us/phoenix/issues/9196#issuecomment-5688739893),
recorded by the driver under the standing delegation of founder ruling #8807 R4.1.

## Context

The 2026-09-06 interim exception on [#7306](https://github.com/kamp-us/phoenix/issues/7306) lets a
Tuval PR resolve the `review-ui` namespace with a `routed-elsewhere` record instead of a rendered
verdict, and prescribes the clause that record carries, verbatim: "Tuval surface; no admissible
renderer until #7306 lands (founder ruling 2026-09-06). Text review PASS + builder hand-verification
stand in."

That clause asserts two things, and until now the verb checked neither.
[0391](0391-hand-verification-binds-ui-content.md) mechanized the second half: the
hand-verification's currency became `--verified-at`, landed on
[#9178](https://github.com/kamp-us/phoenix/issues/9178). The first half stayed written text.
`runRoute` in [`route-verb.ts`](../packages/fabrika-cli/src/review-ui/route-verb.ts) read the PR, the
live head, the `ui` class and the `--verified-at` range, and no verdict marker at all — so it would
post the clause at a head where the text gate stood FAIL, and exit 0.

The exception's own text is ambiguous about whether that PASS is required. The bullet after the
clause names exactly one thing as required — the builder's hand-verification — while the clause
asserts the conjunction, and the verb resolved the ambiguity permissively.

The pipeline has read it as a conjunction every time it was used: across #7306's sunset list of
80-plus entries no route was posted over a standing text FAIL, and on
[#8878](https://github.com/kamp-us/phoenix/pull/8878),
[#8804](https://github.com/kamp-us/phoenix/pull/8804) and
[#8802](https://github.com/kamp-us/phoenix/pull/8802) the text gate FAILed, the builder repaired, and
the route landed only at the subsequent PASS head. The gap surfaced on
[#9193](https://github.com/kamp-us/phoenix/pull/9193), where every mechanical precondition held at
the head, `review-code` stood FAIL at that exact head, and the gate ended CANT-SEE rather than post
a clause it could not defend.

The failure is 0391's quiet kind. The `routed-elsewhere` format carries no polarity and no attached
captures, so a record resting on a text PASS and one resting on a standing FAIL read identically, and
`ship gate` resolves both as `routed`. Nothing wrong merges — `ship` gates on `review-code`
independently — so what it costs is a false statement on the permanent record and on the sunset list
the first real Tuval UI review is meant to sweep against.

## Decision

**The text review the clause asserts is read by the verb, at the head the record binds.**

1. **A standing FAIL refuses the route.** The `review-code` verdict in force at `--sha` is resolved
before anything is composed or posted, and a FAIL is exit `20` with the comment named. The route
returns when the text gate passes on a head; nothing about the record changes in the meantime.
2. **Absence refuses exactly where the record claims a PASS.** A route carrying `--verified-at`
stands in for the render under the #7306 exception, whose clause names both halves, so no text
verdict binding `--sha` is the same `20`. A route with no hand-verification — a prose-only diff
under a declared `uiSurfaces` prefix — asserts nothing about the text lane, so an absent verdict
is stated on stderr and in the answer's `textReview` field rather than refused. This is the
narrow arm of the ruling's direction: it makes the clause's assertion unpostable without its
evidence, without ordering the two gates on every PR that has no such clause to make.
3. **The reader is `review verdicts`', and the ordering `ship gate`'s.** The claims are the
`verdict-marker` first line and the §CP advisory carrier, ordered by `inForce` and judged current
by `bindToContent`
([`text-verdict.ts`](../packages/fabrika-cli/src/review-ui/text-verdict.ts)). A condition each
reader derives for itself is a condition each reader derives differently — 0391 §3's rule, applied
to the other half of the same clause. A verdict the head has moved past is not in force, so a
content-bound PASS that survives a rebase survives here too.
4. **GitHub's native review fold stays out.** `ship gate` folds an `APPROVED` or `CHANGES_REQUESTED`
review into `review-code` because it is the merge authority. This verb judges only whether its own
clause states something true, and the fold costs a second API surface for a carrier the text gate
does not emit. The merge gate still reads it.
5. **The exception's text carries the condition.** It lands on #7306 as a dated amendment below the
2026-09-06 ruling, never as an edit to it, so the sunset list stays readable against the text each
entry was posted under — 0391 §5's rule, unchanged. Landed 2026-09-16:
[the amendment comment](https://github.com/kamp-us/phoenix/issues/7306#issuecomment-5701952969),
appended to the issue body in the same shape, stating both halves of the clause as required.

**Rejected: dropping the assertion from the clause instead.** It is the cheapest change and it
weakens what the sunset sweep can rely on: the first real Tuval UI review reads those entries back,
and an entry that claims nothing about the text lane tells it nothing. The founder was asked whether
a ui record may stand over a failed text review and answered no, so the clause stays and the verb
earns it.

**Rejected: refusing an absent verdict on every route.** It orders the two gates on PRs whose record
makes no claim about the text lane at all, and a review-ui lane that runs before the text one parks
on a fact about sequencing rather than about the PR.

**Sunset.** This decision lives exactly as long as the exception it conditions, as 0391 does. When
#7306 lands a trusted evidence path, the clause goes and this record is retired with it.

## Consequences

- A Tuval route over a standing text FAIL is refused at `20` rather than posted, and the reviewer's
move is the one the pipeline already made by hand: let the text lane repair, route at the passing
head.
- A Tuval route now needs the text verdict landed first. The reviewer reads what stands with
`fabrika review verdicts <pr>` instead of deriving it from a comment scan.
- A prose-only route is unchanged in its outcome and louder in its answer: `textReview` says whether
a text verdict backed it, so the sunset sweep can tell the two kinds of entry apart.
- A text verdict carried only by a GitHub native review does not clear the route. The refusal names
the reader, so a lane in that shape sees why.
- #7306's sunset list gains the same fact per entry as 0391 gave it: which text verdict the record
rested on, beside the hand-verification's head.

## Records

no vocabulary impact
10 changes: 10 additions & 0 deletions claude-plugins/fabrika/skills/review-ui/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ the route can post; exit `11` naming the ceiling, or naming the two heads as div
flag where there is no hand-verification, and never derive the range by hand — a condition you check
by eye is one the next gate checks differently.

**The route also rests on the text gate's verdict, and the verb reads that for you.** Exit `20`
means the `review-code` verdict in force at `--sha` is a **FAIL**: the record would assert a text
PASS that is not there, and the polarity-free format leaves no later reader able to falsify it. That
is not yours to route around — the text lane repairs, and you route at the head it passes. The same
`20` covers an **absent** text verdict on a `--verified-at` route, because the exception's clause
names both halves; a prose-only route with no text verdict posts, and the answer's `textReview`
field says which of the two it rested on. The verdict is read before the `--verified-at` range, so a
route that is both spent at `--verified-at` and standing-FAIL at `--sha` meets `20` rather than
`12` — the text lane is the move to make first, and the desk run is re-run after it.

Exit `7` covers four different facts, and only one of them is a clean end — **read the message
before you pick a terminal.** `raises no ui class` means nothing required your namespace and there
is nothing to route: end ROUTED-ELSEWHERE with no write. The other three — the PR proven absent
Expand Down
49 changes: 44 additions & 5 deletions claude-plugins/fabrika/skills/review-ui/contract.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ sibling's numerals is not a goal the doctrine sets.
| `16` | proven: no preview deployment exists for this PR — the announced-preview convention resolves to nothing; the skill's CANT-SEE route |
| `17` | proven: at least one evidence upload or upload-verification failed — **nothing was posted** |
| `18` | refused: the write would retire a standing verdict of the **opposite polarity** at this head and `--supersede` was not passed — nothing posted |
| `20` | refused, proven: the text review a `route` record rests on is not a standing PASS at the head it binds — the `review-code` verdict in force at `--sha` is a FAIL, or a route resting on a hand-verification has no text verdict binding that head — nothing posted |
| `127` | the verb never ran at all (unresolved binary) |

**`7` versus `11`** is the package's spine: a 404 is a fact about the repository, an unreachable
Expand Down Expand Up @@ -758,9 +759,11 @@ The reasoning arrives on **stdin only**, for the same reason as `post` and `note
| stdin | markdown | yes | — | which files changed and why none of them renders anything |

**Output** — machine. One JSON object:
`{"answer":"routed","namespace":"review-ui","sha":"6c6fe226…","uiFiles":2,"verifiedAt":null,"upsert":"created","commentUrl":"…"}`.
`{"answer":"routed","namespace":"review-ui","sha":"6c6fe226…","uiFiles":2,"verifiedAt":null,"textReview":"absent","upsert":"created","commentUrl":"…"}`.
`verifiedAt` is the `--verified-at` head the range was cleared over, and `null` where the route
rested on no hand-verification.
rested on no hand-verification. `textReview` is the `review-code` verdict this record rests on:
`pass` where one stands in force at `--sha`, `absent` where none binds that head — and `absent` is
reachable only on a route carrying no `--verified-at`, because one that does is refused at `20`.

**Why it exists.** `ship scope` raises the `ui` class from a path test that cannot see whether
pixels moved, so a PR whose only change under a declared `uiSurfaces` prefix is prose requires this namespace — and
Expand All @@ -779,7 +782,13 @@ diff was read, and is re-read rather than re-bound. Read the changed-file list;
the gate is meanwhile blocking. Refuse a diff that raises no `ui` class (`7`) — nothing required
this namespace, so there is nothing to route; the predicate is `review/classes.ts`'s own
`isUiSurface`, over the same declared `uiSurfaces` prefixes the gate raised the class from, never a
second copy. With `--verified-at`, compare that head to `--sha` and refuse on `12` when any file in
second copy. Read the PR's comments and resolve the `review-code` verdict in force at `--sha`; a
standing FAIL is `20`, and so is an absent verdict on a route carrying `--verified-at`. That read
runs **before** the `--verified-at` comparison below, so a route that is both spent at
`--verified-at` and standing-FAIL at `--sha` exits `20`, not `12` — a record asserting a text PASS
that is not there is unpostable at any head, while a spent hand-verification is cleared by re-running
it, so the text lane is the move to name first. With `--verified-at`, compare that head to `--sha`
and refuse on `12` when any file in
the range raises the `ui` class — the hand-verification is then spent and a fresh one is owed at
`--sha`; a comparison that came back at GitHub's 300-file ceiling is `11`, because the compare
declares no total and a capped list can only ever hide a `ui`-class file, and so is one whose two
Expand All @@ -799,6 +808,23 @@ head to compare; where a hand-verification exists, naming it is what makes the r
Nothing about the `routed-elsewhere` bytes changes either way: the record stays head-bound to
`--sha` and carries no evidence field.

**The text review the record rests on.** An interim exception that lets a hand-verification stand
in for a render prescribes the clause such a record carries, and that clause asserts a text review
PASS beside the hand-verification. The verb used to post it while reading neither half, so a record
over a standing text FAIL and one over a PASS read identically — and the `routed-elsewhere` format
carries no polarity for a later reader to tell them apart. The text PASS is a precondition, not
commentary, and the verb reads it: it resolves the `review-code` verdict in force at `--sha` through
`review verdicts`' own two carriers — the `verdict-marker` first line and the §CP advisory — ordered
by `ship gate`'s `inForce`, judged current by the same `bindToContent`, and given the advisory's
polarity by the one `advisoryPolarity` its sibling readers call, so no two readers hold different
rules. That last one was a copy before it was shared, and the copy diverged: a `[FAIL]` row inside an
advisory is an invalid emission, and it cleared this route while `ship gate` refused on the same
comment. A standing FAIL refuses on `20`. An **absent** verdict refuses on `20` only where
`--verified-at` is passed: that route asserts the conjunction, while a prose-only route asserts
nothing about the text lane and says so on stderr instead of blocking. The host's native review fold
is `ship gate`'s widening and is not read here — the merge gate still reads it, and this verb only
judges what its own clause claims.

**What this verb does not decide.** Whether the diff renders anything. That is the skill's judgment
over `review diff`'s refusal-guarded bytes. Narrowing the `ui` path class instead was proposed and
rejected: no path test can decide whether pixels moved, so a verb that tried would just
Expand All @@ -817,6 +843,7 @@ relocate the defect. This verb takes the judgment as `--clause` plus a body and
| `10` | `--sha` or `--verified-at` is not a head SHA, or `--clause` is blank |
| `11` | a precondition read failed, the changed-file list came back truncated, or the `--verified-at` comparison came back at the 300-file ceiling or between two diverged heads — nothing was posted |
| `12` | the live head moved past `--sha` — the diff you read is gone; or a `ui`-class file changed between `--verified-at` and `--sha`, so the hand-verification is spent |
| `20` | the `review-code` verdict in force at `--sha` is a FAIL, or a route resting on `--verified-at` has no `review-code` verdict binding that head |

**Errors**

Expand All @@ -839,6 +866,8 @@ relocate the defect. This verb takes the judgment as `--clause` plus a body and
| `review-ui route: cannot read <what> for #<n>: <reason> — nothing was posted.` | 11 | refusal |
| `review-ui route: the live head is <live>, not <sha> — the diff you read is gone; re-read at <live>.` | 12 | refusal |
| `review-ui route: <files> raise the ui class in <verified>..<sha> — the hand-verification at <verified> is spent; re-run it at <sha>.` | 12 | refusal |
| `review-ui route: review-code stands FAIL at <sha> (comment <id>) — this record would assert a text PASS that is not there; repair the finding and route at the head the text gate passes.` | 20 | refusal |
| `review-ui route: no standing review-code verdict binds <sha>, and a route resting on a hand-verification asserts one — land the text verdict first, and read what stands with fabrika review verdicts <n>.` | 20 | refusal |

**Scope** — one PR, one comment write, the caller's stdin.

Expand All @@ -851,7 +880,7 @@ $ fabrika review-ui route 6326 --sha 6c6fe226 \
export or type changed. `design-token-lint.config.json` rewrites two note strings; the guard's
data fields are byte-identical. No component, route, token or style is touched.
EOF
{"answer":"routed","namespace":"review-ui","sha":"6c6fe226","uiFiles":2,"verifiedAt":null,"upsert":"created","commentUrl":"https://github.com/<owner>/<repo>/pull/6326#issuecomment-5123990412"}
{"answer":"routed","namespace":"review-ui","sha":"6c6fe226","uiFiles":2,"verifiedAt":null,"textReview":"absent","upsert":"created","commentUrl":"https://github.com/<owner>/<repo>/pull/6326#issuecomment-5123990412"}
```

A route resting on a hand-verification names the head it ran at, and the range decides whether it
Expand All @@ -863,7 +892,7 @@ $ fabrika review-ui route 4471 --sha fb01065b --verified-at 8efd315a \
The desk run at `8efd315a` drove every readout this diff touches. `8efd315a..fb01065b` is one
commit under `packages/<cli>/`, so the composition is byte-identical.
EOF
{"answer":"routed","namespace":"review-ui","sha":"fb01065b","uiFiles":3,"verifiedAt":"8efd315a","upsert":"created","commentUrl":"https://github.com/<owner>/<repo>/pull/4471#issuecomment-5598041887"}
{"answer":"routed","namespace":"review-ui","sha":"fb01065b","uiFiles":3,"verifiedAt":"8efd315a","textReview":"pass","upsert":"created","commentUrl":"https://github.com/<owner>/<repo>/pull/4471#issuecomment-5598041887"}

$ fabrika review-ui route 4471 --sha fb01065b --verified-at 8efd315a --clause "…" < why.md
review-ui route: scanned 2 files changed in 8efd315a..fb01065b; 2 raise the ui class.
Expand All @@ -875,6 +904,13 @@ $ fabrika review-ui route 4471 --sha 9c40aa71 --verified-at 8efd315a --clause "
review-ui route: 8efd315a is diverged of 9c40aa71, not an ancestor — the comparison answers from
their merge base, so 8efd315a..9c40aa71 was never read. Re-run the hand-verification at 9c40aa71.
# exit 11

$ fabrika review-ui route 4471 --sha fb01065b --verified-at fb01065b --clause "…" < why.md
review-ui route: scanned 6 comments.
review-ui route: scanned 2 review-code claims; FAIL at fb01065b via the marker carrier.
review-ui route: review-code stands FAIL at fb01065b (comment 5598219196) — this record would
assert a text PASS that is not there; repair the finding and route at the head the text gate passes.
# exit 20
```

**Grounding**
Expand All @@ -890,6 +926,9 @@ their merge base, so 8efd315a..9c40aa71 was never read. Re-run the hand-verifica
- **A hand-verification's currency is the verb's judgment, not the gate's**, and it binds the
`ui` files' content rather than the record's own head — a commit that moves no rendered surface
keeps the evidence rather than spending a desk session to re-prove it.
- **The text PASS a hand-verification clause asserts is read, not assumed** — a standing
`review-code` FAIL at the record's head refuses the route, and the reader is `review verdicts`'
own, so the two gates cannot answer one question differently.
- **The head binding**, and why a moved head is re-read rather than re-bound.

---
Expand Down
Loading
Loading