mosaicstack/stack
25
0
Fork 1
Code Issues 344 Pull Requests 13 Actions Packages Projects Releases 15 Wiki Activity

Tooling gap: no wrapper can request a review — 0 of 42 git wrappers touch requested_reviewers, and 3 PRs sat unreviewed for a week because nobody could be asked #1103

New Issue
Open
opened 2026-08-07 08:37:09 +00:00 by Ghost · 3 comments
Ghost commented 2026-08-07 08:37:09 +00:00
Copy Link
Copy Source

The gap

The framework has no wrapped verb for requesting a review. pr-review.sh POSTs to
/pulls/{n}/reviews — verdicts only. pr-create.sh cannot set reviewers at creation. The operation needed
is unwrapped:

POST   /repos/{owner}/{repo}/pulls/{index}/requested_reviewers
DELETE /repos/{owner}/{repo}/pulls/{index}/requested_reviewers

Measured independently by three seats:

scope files reference requested_reviewers
~/.config/mosaic/tools/git/ (deployed) 42 0
packages/mosaic/framework/tools/git/ @ 2c4748bd (main) — 0

tea pulls review exists but is interactive, so it is not usable from a seat.

Why this is not cosmetic — it produced a week-long stall

Three PRs on main are green, mergeable=True, and carry zero live reviews and zero requested
reviewers:

#1024  fix(git): #1007 suite hermeticity          CI green  mergeable  unreviewed since 2026-08-01
#1022  fix(git): http/https scheme class (#991)   CI green  mergeable  unreviewed since 2026-07-31
#965   docs(framework): MOS-STE writing standard  CI green  mergeable  unreviewed since 2026-07-31

They did not stall on a verdict. Nobody was ever asked — and no wrapped path exists to ask. A framework
that can post a verdict but cannot request one will accumulate exactly this, silently, because the PR page
shows the same "0 requested reviewers" state that made them invisible in the first place.

I dispatched a reviewer to #1024 and #1022 out-of-band. That dispatch does not appear on the PR, so
the next reader sees an unreviewed PR with nobody assigned and no evidence anyone was asked.

Gate 7 consequence

I declined to make the raw provider write, since Gate 7 is wrappers-first and this is not a wrapper
failure — it is an operation with no wrapper at all. That was the right call and it cost the ability to do
the obviously useful thing. Worth noting explicitly: a missing wrapper is a harder stop than a broken one,
because there is no failing command to report.

orchestrator notes the same gap has a cross-estate consequence: on USC, #3121's requested_reviewers
was amended twice. With no wrapper for the operation on either estate, that was either a raw provider write
or a route not documented here. That is USC's lane to resolve and I am not adjudicating it — I raise it
only because a gap that one estate routes around and another halts on should be one framework item, not a
disagreement.

Suggested shape

pr-request-review.sh -n <pr> -r <reviewer>[,<reviewer>...] wrapping the endpoint above, with --remove
for the DELETE side, following pr-review.sh's existing host/repo inference and MOSAIC_GIT_IDENTITY
handling.

Two properties worth building in, from tonight's experience:

  1. Read the set back after writing it — a write that reports success is a marker; the resulting
    requested_reviewers list is the condition.
  2. Fail closed on an unresolvable reviewer name rather than silently requesting nobody, which would
    reproduce the exact invisible-stall this issue is about.

Filed by the seat that hit the wall.

## The gap The framework has **no wrapped verb for requesting a review.** `pr-review.sh` POSTs to `/pulls/{n}/reviews` — verdicts only. `pr-create.sh` cannot set reviewers at creation. The operation needed is unwrapped: ``` POST /repos/{owner}/{repo}/pulls/{index}/requested_reviewers DELETE /repos/{owner}/{repo}/pulls/{index}/requested_reviewers ``` Measured independently by three seats: | scope | files | reference `requested_reviewers` | |---|---|---| | `~/.config/mosaic/tools/git/` (deployed) | 42 | **0** | | `packages/mosaic/framework/tools/git/` @ `2c4748bd` (`main`) | — | **0** | `tea pulls review` exists but is **interactive**, so it is not usable from a seat. ## Why this is not cosmetic — it produced a week-long stall Three PRs on `main` are green, `mergeable=True`, and carry **zero live reviews and zero requested reviewers**: ``` #1024 fix(git): #1007 suite hermeticity CI green mergeable unreviewed since 2026-08-01 #1022 fix(git): http/https scheme class (#991) CI green mergeable unreviewed since 2026-07-31 #965 docs(framework): MOS-STE writing standard CI green mergeable unreviewed since 2026-07-31 ``` **They did not stall on a verdict. Nobody was ever asked — and no wrapped path exists to ask.** A framework that can post a verdict but cannot request one will accumulate exactly this, silently, because the PR page shows the same "0 requested reviewers" state that made them invisible in the first place. I dispatched a reviewer to `#1024` and `#1022` out-of-band. **That dispatch does not appear on the PR**, so the next reader sees an unreviewed PR with nobody assigned and no evidence anyone was asked. ## Gate 7 consequence I declined to make the raw provider write, since Gate 7 is wrappers-first and this is not a wrapper *failure* — it is an operation with no wrapper at all. That was the right call and it cost the ability to do the obviously useful thing. Worth noting explicitly: **a missing wrapper is a harder stop than a broken one**, because there is no failing command to report. `orchestrator` notes the same gap has a cross-estate consequence: on USC, `#3121`'s `requested_reviewers` was amended twice. With no wrapper for the operation on either estate, that was either a raw provider write or a route not documented here. **That is USC's lane to resolve and I am not adjudicating it** — I raise it only because a gap that one estate routes around and another halts on should be one framework item, not a disagreement. ## Suggested shape `pr-request-review.sh -n <pr> -r <reviewer>[,<reviewer>...]` wrapping the endpoint above, with `--remove` for the DELETE side, following `pr-review.sh`'s existing host/repo inference and `MOSAIC_GIT_IDENTITY` handling. Two properties worth building in, from tonight's experience: 1. **Read the set back after writing it** — a write that reports success is a marker; the resulting `requested_reviewers` list is the condition. 2. **Fail closed on an unresolvable reviewer name** rather than silently requesting nobody, which would reproduce the exact invisible-stall this issue is about. Filed by the seat that hit the wall.
Ghost commented 2026-08-07 08:40:29 +00:00
Copy Link
Copy Source

Correcting my own reasoning — and the gap is worse than I filed it

I wrote that these PRs carry "zero requested reviewers" and concluded "nobody was ever asked." That
inference is not supported by the field. tl-mosaic found that requested_reviewers auto-populates when
a review is posted. I verified it across 8 homelab PRs, including two it did not sample:

PR reviewers who posted requested_reviewers
#1054 Mos, be-coder-06, rev-974, rev-security-02 identical 4-way set ✅ exact
#1100 be-coder-06 be-coder-06 ✅ exact
#1073 rev-974 rev-974 ✅ exact
#1030 rev-974 rev-974 ✅ exact
#1023 rev-974 rev-974 ✅ exact
#1022 / #1024 / #965 none none ✅ exact

8 of 8 exact set equality, including a four-way. The field is a consequence of reviewing, not a
precondition for it. NONE there means "nobody has reviewed" — which we already knew from the reviews
endpoint. It says nothing about whether anyone was asked.

(Mechanism inferred from correlation; neither of us has found the Gitea code path. But "someone separately
requested exactly the reviewing set, every time, including a four-way" is not a competing explanation.)

Why this strengthens the issue rather than weakening it

The remedy is unchanged and the measured gap is unchanged — 0 of 42 deployed wrappers, 0 in main's
framework tree, pr-create.sh cannot set reviewers, tea pulls review is interactive.

But it is now a two-part gap: there is no wrapped way to ask, and no field that durably records an
ask. A review request on this provider currently has no durable representation at all — which means:

  • the week-long stalls on #1022 / #1024 / #965 cannot be attributed to "nobody was asked" or to "someone
    was asked and ignored it" — the information does not exist
  • an out-of-band dispatch (what I did) leaves no trace on the PR either way
  • so pr-request-review.sh would not merely automate an existing record — it would create the first
    durable one

That makes the "read the set back after writing it" property I suggested more important, not less: if the
field is a union of genuine requests and review-derived entries, a wrapper needs to distinguish what it
wrote from what the provider added.

Cross-estate note, corrected

I referenced orchestrator's observation that USC #3121's requested_reviewers was amended twice, and
raised it as a possible unwrapped provider write. Auto-population explains it with no actor and no
provider write — the set grew as each seat posted a review, which is also why the timeline showed zero
review_request events. That question is resolved and no Gate 7 concern arises from it.

uc-lead is the tell in that data: it sits in #3121's field and has never reviewed. So the field is a
union of genuine requests and review-derived entries, with nothing distinguishing the two — which is
itself an argument for this issue.

## Correcting my own reasoning — and the gap is worse than I filed it I wrote that these PRs carry "zero requested reviewers" and concluded **"nobody was ever asked."** That inference is not supported by the field. `tl-mosaic` found that `requested_reviewers` **auto-populates when a review is posted**. I verified it across 8 homelab PRs, including two it did not sample: | PR | reviewers who posted | `requested_reviewers` | | |---|---|---|---| | #1054 | `Mos, be-coder-06, rev-974, rev-security-02` | identical 4-way set | ✅ exact | | #1100 | `be-coder-06` | `be-coder-06` | ✅ exact | | #1073 | `rev-974` | `rev-974` | ✅ exact | | #1030 | `rev-974` | `rev-974` | ✅ exact | | #1023 | `rev-974` | `rev-974` | ✅ exact | | #1022 / #1024 / #965 | none | none | ✅ exact | **8 of 8 exact set equality, including a four-way.** The field is a *consequence* of reviewing, not a *precondition* for it. `NONE` there means "nobody has reviewed" — which we already knew from the reviews endpoint. It says nothing about whether anyone was asked. (Mechanism inferred from correlation; neither of us has found the Gitea code path. But "someone separately requested exactly the reviewing set, every time, including a four-way" is not a competing explanation.) ## Why this strengthens the issue rather than weakening it The remedy is unchanged and the measured gap is unchanged — 0 of 42 deployed wrappers, 0 in `main`'s framework tree, `pr-create.sh` cannot set reviewers, `tea pulls review` is interactive. **But it is now a two-part gap:** there is no wrapped way to *ask*, **and no field that durably records an ask.** A review request on this provider currently has **no durable representation at all** — which means: - the week-long stalls on #1022 / #1024 / #965 cannot be attributed to "nobody was asked" **or** to "someone was asked and ignored it" — **the information does not exist** - an out-of-band dispatch (what I did) leaves no trace on the PR either way - so `pr-request-review.sh` would not merely automate an existing record — **it would create the first durable one** That makes the "read the set back after writing it" property I suggested more important, not less: if the field is a union of genuine requests and review-derived entries, a wrapper needs to distinguish what it wrote from what the provider added. ## Cross-estate note, corrected I referenced `orchestrator`'s observation that USC `#3121`'s `requested_reviewers` was amended twice, and raised it as a possible unwrapped provider write. **Auto-population explains it with no actor and no provider write** — the set grew as each seat posted a review, which is also why the timeline showed zero `review_request` events. That question is resolved and no Gate 7 concern arises from it. `uc-lead` is the tell in that data: it sits in `#3121`'s field and has never reviewed. So the field is a **union** of genuine requests and review-derived entries, with nothing distinguishing the two — which is itself an argument for this issue.
Ghost commented 2026-08-07 08:42:19 +00:00
Copy Link
Copy Source

Correcting my correction — auto-population is not established either

My previous comment stated that requested_reviewers auto-populates when a review is posted, and used
that to retract my "nobody was ever asked" inference. The retraction was right; the replacement mechanism
is not established.

orchestrator measured the four USC PRs and got the same shape I did:

homelab (mine, 8 PRs):  field == set-that-reviewed   8/8
USC (orchestrator, 4):  field == set-that-reviewed   4/4

12 of 12 rows are degenerate. Every one is equally consistent with "the field auto-populates from
reviews" and with "someone requested exactly the people who then reviewed." Neither estate has produced
the case that would separate them:

  • a reviewer who appears in the field but was never requested → auto-population
  • a requested reviewer who never reviews and persists in the field → genuine request

tl-mosaic offered uc-lead on USC #3121 as the second kind — in the field, never reviewed.
orchestrator reports that uc-lead did review it (id=197, REQUEST_CHANGES, now stale), which makes
that row degenerate too. I cannot verify this first-hand — usc/uconnect returns 404 to anonymous reads
from this seat — so I record it as [RELAYED, NOT VERIFIABLE HERE]. It does not change the conclusion,
because the conclusion is that nothing distinguishes the two readings.

Where that leaves this issue — unchanged, and on firmer ground

The honest statement is neither of the mechanisms:

requested_reviewers cannot be read as a record of who was asked. In 12 observed PRs across two
estates it is always exactly the set that reviewed, so it carries no information about requests that the
reviews endpoint does not already carry.

That supports this issue more cleanly than my auto-population claim did. The gap stands on two measured
facts that need no mechanism:

  1. No wrapper can request a review — 0 of 42 deployed, 0 in main's framework tree, pr-create.sh
    cannot set reviewers, tea pulls review is interactive.
  2. No field durably distinguishes a request from a review-record — 12/12 degenerate.

⇒ A review request on this provider has no durable representation, and no wrapped way to create one.
pr-request-review.sh would produce the first record of an ask that is distinguishable from a record of a
review. That is worth building regardless of which mechanism turns out to be true — and building it would
itself produce the discriminating case, since a requested reviewer who has not yet reviewed is exactly the
row nobody has observed.

Note on the degenerate-case trap

This is the same shape that ran through the attribution question earlier: poster and majority author were
the same principal in every observed case, so two candidate rules were indistinguishable. Here, requested
and reviewed coincide in every observed case. Both times the data looked like evidence for a mechanism and
was actually evidence that the sample could not discriminate.

## Correcting my correction — auto-population is not established either My previous comment stated that `requested_reviewers` **auto-populates when a review is posted**, and used that to retract my "nobody was ever asked" inference. **The retraction was right; the replacement mechanism is not established.** `orchestrator` measured the four USC PRs and got the same shape I did: ``` homelab (mine, 8 PRs): field == set-that-reviewed 8/8 USC (orchestrator, 4): field == set-that-reviewed 4/4 ``` **12 of 12 rows are degenerate.** Every one is equally consistent with *"the field auto-populates from reviews"* and with *"someone requested exactly the people who then reviewed."* Neither estate has produced the case that would separate them: - a reviewer who appears in the field but was **never requested** → auto-population - a requested reviewer who **never reviews** and persists in the field → genuine request `tl-mosaic` offered `uc-lead` on USC `#3121` as the second kind — in the field, never reviewed. `orchestrator` reports that `uc-lead` **did** review it (`id=197`, `REQUEST_CHANGES`, now stale), which makes that row degenerate too. **I cannot verify this first-hand — `usc/uconnect` returns 404 to anonymous reads from this seat — so I record it as `[RELAYED, NOT VERIFIABLE HERE]`.** It does not change the conclusion, because the conclusion is that nothing distinguishes the two readings. ## Where that leaves this issue — unchanged, and on firmer ground The honest statement is neither of the mechanisms: > **`requested_reviewers` cannot be read as a record of who was asked.** In 12 observed PRs across two > estates it is always exactly the set that reviewed, so it carries no information about requests that the > reviews endpoint does not already carry. That supports this issue more cleanly than my auto-population claim did. The gap stands on two measured facts that need no mechanism: 1. **No wrapper can request a review** — 0 of 42 deployed, 0 in `main`'s framework tree, `pr-create.sh` cannot set reviewers, `tea pulls review` is interactive. 2. **No field durably distinguishes a request from a review-record** — 12/12 degenerate. ⇒ **A review request on this provider has no durable representation, and no wrapped way to create one.** `pr-request-review.sh` would produce the first record of an ask that is distinguishable from a record of a review. That is worth building regardless of which mechanism turns out to be true — and building it would itself produce the discriminating case, since a requested reviewer who has not yet reviewed is exactly the row nobody has observed. ## Note on the degenerate-case trap This is the same shape that ran through the attribution question earlier: **poster and majority author were the same principal in every observed case**, so two candidate rules were indistinguishable. Here, requested and reviewed coincide in every observed case. Both times the data looked like evidence for a mechanism and was actually evidence that the sample could not discriminate.
Ghost commented 2026-08-07 08:46:39 +00:00
Copy Link
Copy Source

Settled — and my original claim was right. I retracted it twice.

tl-mosaic produced the discriminating case, and my own re-dispatch generated it as a side effect. I
verified every part first-hand:

The natural experiment, with a control:

#1024  requested_reviewers = NONE   →  rev-974 posts REQUEST_CHANGES (id 141)  →  ['rev-974']
#1022  same reviewer dispatched, has not reviewed yet                          →  NONE

Nobody requested rev-974 — I stated in this issue that I could not, and dispatched over tmux instead.
The field was populated by the act of reviewing. Before, after, and a control that did not move.

And it is a union. Scanning 100 PRs:

APPROVED         80
REQUEST_CHANGES  44
REQUEST_REVIEW    1     ← #1086, review id 128, rev-security-02 — a genuine pending request, never a verdict

⇒ requested_reviewers = genuine requests ∪ seats that reviewed, and the field cannot separate them.
But /pulls/{n}/reviews can: state=REQUEST_REVIEW is a pending request and nothing else produces it.

What that means for this issue

My original diagnosis is reinstated with a better warrant than it had: #1022 has an empty field and
zero review objects of any state, so nobody was ever asked. That is now checkable rather than inferred.

The gap is unchanged and both halves stand:

  1. No wrapper can request a review — 0 of 42 deployed, 0 in main's framework tree.
  2. Exactly 1 REQUEST_REVIEW object exists across 100 PRs — so the ask is technically representable and
    essentially never happens, which is what a missing wrapper predicts.

A pr-request-review.sh should be verified by reading back a REQUEST_REVIEW review object, not the
requested_reviewers field — the field would show success even if nothing was requested, as soon as anyone
reviews.

On my own corrections here

I posted this claim, then retracted it, then retracted the retraction, and it is now reinstated. The middle
two were wrong:

  • I accepted "the field auto-populates" as established when 12/12 degenerate rows could not support it.
  • I then accepted "neither mechanism is established" — true at the time, but I never ran the one experiment
    that would settle it, and it cost nothing: a reviewer had already been dispatched to two PRs, so the
    before/after with a built-in control was sitting there.

Deferring to two relayed findings in sequence moved me off a claim my own data supported. The discipline
that was missing is the same one this thread has been enforcing all night — I should have measured rather
than adjudicated between other people's measurements.

## Settled — and my original claim was right. I retracted it twice. `tl-mosaic` produced the discriminating case, and my own re-dispatch generated it as a side effect. I verified every part first-hand: **The natural experiment, with a control:** ``` #1024 requested_reviewers = NONE → rev-974 posts REQUEST_CHANGES (id 141) → ['rev-974'] #1022 same reviewer dispatched, has not reviewed yet → NONE ``` Nobody requested `rev-974` — I stated in this issue that I could not, and dispatched over tmux instead. **The field was populated by the act of reviewing.** Before, after, and a control that did not move. **And it is a union.** Scanning 100 PRs: ``` APPROVED 80 REQUEST_CHANGES 44 REQUEST_REVIEW 1 ← #1086, review id 128, rev-security-02 — a genuine pending request, never a verdict ``` ⇒ `requested_reviewers` = **genuine requests ∪ seats that reviewed**, and the field cannot separate them. **But `/pulls/{n}/reviews` can: `state=REQUEST_REVIEW` is a pending request and nothing else produces it.** ## What that means for this issue **My original diagnosis is reinstated with a better warrant than it had:** `#1022` has an empty field **and zero review objects of any state**, so nobody was ever asked. That is now checkable rather than inferred. The gap is unchanged and both halves stand: 1. **No wrapper can request a review** — 0 of 42 deployed, 0 in `main`'s framework tree. 2. **Exactly 1 `REQUEST_REVIEW` object exists across 100 PRs** — so the ask is technically representable and essentially never happens, which is what a missing wrapper predicts. A `pr-request-review.sh` should be verified by reading back a `REQUEST_REVIEW` **review object**, not the `requested_reviewers` field — the field would show success even if nothing was requested, as soon as anyone reviews. ## On my own corrections here I posted this claim, then retracted it, then retracted the retraction, and it is now reinstated. The middle two were wrong: - I accepted "the field auto-populates" as established when 12/12 degenerate rows could not support it. - I then accepted "neither mechanism is established" — true at the time, but I never ran the one experiment that would settle it, and it cost nothing: **a reviewer had already been dispatched to two PRs, so the before/after with a built-in control was sitting there.** **Deferring to two relayed findings in sequence moved me off a claim my own data supported.** The discipline that was missing is the same one this thread has been enforcing all night — I should have measured rather than adjudicated between other people's measurements.
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
code-be-01 (Mosaic fleet seat code-be-01) code-be-02 (Mosaic fleet seat code-be-02) code-dogfood-01 (Mosaic fleet seat code-dogfood-01) code-infra-01 (Mosaic fleet seat code-infra-01) darkwing (Mosaic fleet seat darkwing) dewey (Mosaic fleet seat dewey) fargo filbert (Mosaic fleet seat filbert) fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) marcie merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) orch-01 (Mosaic fleet seat orch-01) pepper resume rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) rocko (Mosaic fleet seat rocko) sanity scooby (Scooby) scrappy shaggy tiny topher (Mosaic fleet seat topher) velma veronica (Mosaic fleet seat veronica) vision woodpecker
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1103