mosaicstack/stack
1
0
Fork 1
Code Issues 140 Pull Requests 6 Actions Packages Projects Releases 15 Wiki Activity

git wrappers: pr-view.sh cannot witness a PR's review verdicts, and 'tea pr review-comments' returns exit 0 with an empty table on a PR that has them #987

New Issue
Open
opened 2026-07-31 08:06:17 +00:00 by mos-dt-0 · 0 comments
mos-dt-0 commented 2026-07-31 08:06:17 +00:00
Collaborator
Copy Link
Copy Source

Summary

A reviewer who follows hard gate 7 (Mosaic wrappers before any raw provider CLI) cannot read a PR's
review verdicts on Gitea at all. pr-view.sh renders the PR body and the CI status and silently omits
every comment; the one command whose name promises the missing data — tea pr review-comments — returns
exit 0 and an empty table on a PR that carries both a CHANGES-REQUIRED and a CLEAR verdict.

This matters because hard gate 16 makes an independent review mandatory, and the verdict of that review
lives exactly in the surface the mandated tooling cannot witness.

Measured (2026-07-31, sb-it-1-dt, tea login mosaicstack-mos-dt-0)

PR #967 — two issue-level comments exist via the API (19654 "Verdict — CLEAR at ...", 19663
"Re-verdict — CLEAR at ..."):

$ pr-view.sh -n 967 -r mosaicstack/stack     # rc=0, 80 lines, 6623 bytes
$ grep -c 'Re-verdict' <output>              # 0   (rc=1)
$ grep -c 'CLEAR at'   <output>              # 0   (rc=1)

PR #974 — two issue-level comments exist via the API (19713 CHANGES-REQUIRED at exact HEAD by
Mos, 19716 CLEAR at exact HEAD by rev-974):

$ pr-view.sh -n 974 -r mosaicstack/stack     # rc=0, 217 lines, 17321 bytes
$ grep -c 'CHANGES-REQUIRED' <output>        # 0   (rc=1)
$ grep -c 'CLEAR at'         <output>        # 0   (rc=1)
$ grep -c 'rev-974'          <output>        # 0   (rc=1)

So the wrapper renders the CI result but not the human verdict. A seat reading #974 through the
wrapper sees a full body and Pipeline was successful, and gets no indication that a
CHANGES-REQUIRED verdict was ever issued.

The escalation path is worse than the gap, and this is the load-bearing part

tea pr advertises review-comments, rc — List review comments on a pull request. On the specimen above:

$ tea pr rc 974 --repo mosaicstack/stack --login mosaicstack-mos-dt-0
rc=0
┌────┬──────┬──────┬──────┬──────────┬──────────┐
│ ID │ PATH │ LINE │ BODY │ REVIEWER │ RESOLVER │
└────┴──────┴──────┴──────┴──────────┴──────────┘

Exit 0, headers only, zero rows — on a PR carrying two review verdicts. The cause is that tea pr rc
lists inline, path/line-anchored review comments, while fleet verdicts are issue-level comments on
/repos/{owner}/{repo}/issues/{n}/comments. Those are different endpoints, and tea has no
issue-comment lister at all (tea comment only adds one).

The result is a confident negative on a path where the intended measurement never happened: an operator
who escalates from the wrapper to the backend gets a clean empty table that reads as "this PR has no
review comments" and means "I looked somewhere else." Same family as grep -q conflating rc=2 with
rc=1, and as a status API rendering blocked as pending.

Proposed fix (defect 1)

pr-view.sh should render issue-level comments — GET /repos/{owner}/{repo}/issues/{n}/comments, and
ideally GET /repos/{owner}/{repo}/pulls/{n}/reviews for review state — after the body.

If rendering them is out of scope, the minimum acceptable fix is a bound the reader cannot miss: print
review comments not rendered — N exist; read via /issues/{n}/comments with the actual count. Absent and
not-looked-at must not produce the same output. A wrapper that omits a surface silently teaches its
operator that the surface is empty.

Acceptance criterion: pr-view.sh -n 974 -r mosaicstack/stack | grep -c CHANGES-REQUIRED is non-zero,
or the output states the comment count it did not render.

Defect 2 — separable, same file, its own criterion

pr-view.sh rejects a bare positional PR number that both of its backends accept:

$ pr-view.sh 983
Unknown option: 983          # rc=1

pr-view.sh:33-36 has a catch-all *) echo "Unknown option: $1"; exit 1. But the wrapper's own backends
take a positional index — gh pr view <n> and tea pulls [<pull index>] — so the wrapper is stricter
than everything it wraps, and its diagnostic calls a number an "option", which points the reader at the
flag table rather than at the missing -n.

Acceptance criterion: pr-view.sh 983 -r mosaicstack/stack behaves as -n 983, or the error names
-n explicitly.

Not a duplicate

Checked against the full issue corpus (985 issues, #1–#986, paginated — the API caps at 50 per page
regardless of limit). Nearest neighbours are the same family and none covers this:
#867 (pr-metadata/pr-merge lack -r), #954 (issue-view.sh lacks -r), #979 (no wrapper edits a
PR body — a write gap; this is a read gap). pr-view.sh already has -r and it works.

Reported by mos-dt (sb-it-1-dt). Filed through issue-create.sh, so the account on this record is
mos-dt-0 (id 13) — a shared per-host identity, not an author identity. The in-body signature is a
labelled claim, never provenance.

## Summary A reviewer who follows hard gate 7 (Mosaic wrappers before any raw provider CLI) **cannot read a PR's review verdicts on Gitea at all**. `pr-view.sh` renders the PR body and the CI status and silently omits every comment; the one command whose name promises the missing data — `tea pr review-comments` — returns **exit 0 and an empty table** on a PR that carries both a `CHANGES-REQUIRED` and a `CLEAR` verdict. This matters because hard gate 16 makes an independent review mandatory, and the verdict of that review lives exactly in the surface the mandated tooling cannot witness. ## Measured (2026-07-31, sb-it-1-dt, `tea` login `mosaicstack-mos-dt-0`) **PR #967** — two issue-level comments exist via the API (`19654` "Verdict — CLEAR at ...", `19663` "Re-verdict — CLEAR at ..."): ``` $ pr-view.sh -n 967 -r mosaicstack/stack # rc=0, 80 lines, 6623 bytes $ grep -c 'Re-verdict' <output> # 0 (rc=1) $ grep -c 'CLEAR at' <output> # 0 (rc=1) ``` **PR #974** — two issue-level comments exist via the API (`19713` `CHANGES-REQUIRED at exact HEAD` by `Mos`, `19716` `CLEAR at exact HEAD` by `rev-974`): ``` $ pr-view.sh -n 974 -r mosaicstack/stack # rc=0, 217 lines, 17321 bytes $ grep -c 'CHANGES-REQUIRED' <output> # 0 (rc=1) $ grep -c 'CLEAR at' <output> # 0 (rc=1) $ grep -c 'rev-974' <output> # 0 (rc=1) ``` So the wrapper renders **the CI result but not the human verdict**. A seat reading #974 through the wrapper sees a full body and `Pipeline was successful`, and gets **no indication that a CHANGES-REQUIRED verdict was ever issued**. ## The escalation path is worse than the gap, and this is the load-bearing part `tea pr` advertises `review-comments, rc — List review comments on a pull request`. On the specimen above: ``` $ tea pr rc 974 --repo mosaicstack/stack --login mosaicstack-mos-dt-0 rc=0 ┌────┬──────┬──────┬──────┬──────────┬──────────┐ │ ID │ PATH │ LINE │ BODY │ REVIEWER │ RESOLVER │ └────┴──────┴──────┴──────┴──────────┴──────────┘ ``` **Exit 0, headers only, zero rows** — on a PR carrying two review verdicts. The cause is that `tea pr rc` lists *inline, path/line-anchored* review comments, while fleet verdicts are *issue-level* comments on `/repos/{owner}/{repo}/issues/{n}/comments`. Those are different endpoints, and `tea` has **no issue-comment lister at all** (`tea comment` only *adds* one). The result is a confident negative on a path where the intended measurement never happened: an operator who escalates from the wrapper to the backend gets a clean empty table that reads as *"this PR has no review comments"* and means *"I looked somewhere else."* Same family as `grep -q` conflating rc=2 with rc=1, and as a status API rendering `blocked` as `pending`. ## Proposed fix (defect 1) `pr-view.sh` should render issue-level comments — `GET /repos/{owner}/{repo}/issues/{n}/comments`, and ideally `GET /repos/{owner}/{repo}/pulls/{n}/reviews` for review state — after the body. **If rendering them is out of scope, the minimum acceptable fix is a bound the reader cannot miss:** print `review comments not rendered — N exist; read via /issues/{n}/comments` with the actual count. Absent and not-looked-at must not produce the same output. A wrapper that omits a surface silently teaches its operator that the surface is empty. **Acceptance criterion:** `pr-view.sh -n 974 -r mosaicstack/stack | grep -c CHANGES-REQUIRED` is non-zero, **or** the output states the comment count it did not render. ## Defect 2 — separable, same file, its own criterion `pr-view.sh` **rejects a bare positional PR number** that both of its backends accept: ``` $ pr-view.sh 983 Unknown option: 983 # rc=1 ``` `pr-view.sh:33-36` has a catch-all `*) echo "Unknown option: $1"; exit 1`. But the wrapper's own backends take a positional index — `gh pr view <n>` and `tea pulls [<pull index>]` — so the wrapper is **stricter than everything it wraps**, and its diagnostic calls a number an "option", which points the reader at the flag table rather than at the missing `-n`. **Acceptance criterion:** `pr-view.sh 983 -r mosaicstack/stack` behaves as `-n 983`, or the error names `-n` explicitly. ## Not a duplicate Checked against the full issue corpus (985 issues, #1–#986, paginated — the API caps at 50 per page regardless of `limit`). Nearest neighbours are the same *family* and none covers this: `#867` (pr-metadata/pr-merge lack `-r`), `#954` (issue-view.sh lacks `-r`), `#979` (no wrapper *edits* a PR body — a write gap; this is a read gap). `pr-view.sh` already has `-r` and it works. --- Reported by **mos-dt** (sb-it-1-dt). Filed through `issue-create.sh`, so the account on this record is `mos-dt-0` (id 13) — a shared per-host identity, not an author identity. The in-body signature is a labelled claim, never provenance.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
coder-mos1 coder-mos2 f10-coder jason.woltje (Jason Woltje) mos-dt-0 pepper rev-974 (Rev-974 (Mosaic reviewer seat, web1))
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#987