Homelab/pi-extensions
6
0
Fork 0
Code Issues Pull requests 4 Projects Releases Packages Wiki Activity Actions

HOMELAB-1679: feat(forgejo): landing census gate — GO record + seat sid verification for infra-core and prod (LR-20261111-017, piece l) #125

Open
pi-landing wants to merge 11 commits from lr-20261111-017-census into main
pull from: lr-20261111-017-census
merge into: Homelab:main
Conversation 33 Commits 11 Files changed 23 +6818 -112
pi-landing commented 2026-11-11 12:49:39 +00:00
Collaborator
Copy link

Spec 017 (Amendments 1, 2, 4, 8, 11, 15-prep), piece (l): the landing census in forgejo/landing.ts — the merge gate the spec-s Design section and Amendment 1 describe, hardened through reviews r1–r4. This PR changes the landing gate itself (forgejo/**), so it is dangerous class: two independent seat sessions are required, which the pane seats on this PR provide. HEAD NOTE: main (#126/#127/#128) is merged in at ea174d3 (RULE-5); the full base..head diff is what the seats review.

What a person gets

A merge into Homelab/infra-core or Homelab/prod through the landing tool happens only when El Director-s go exists for that exact PR and head (a forgotten go is a refusal, not a slip), AND both review seats are verified against the gateway record — their sids real, their served models admissible, their sessions distinct. Enforcement by tool, not by memory.

The gate (all proven by fixture tests against the pure lock — no live endpoint, no live Forgejo)

  • GO record: LD-GO repo=<owner/name> pr=<n> head=<40-hex> by the ld-director-go identity (named in code), current-head match, never edited, unreadable refused; gating reads the SERVER-s repo identity (pr.base.repo.full_name).
  • Seat verification via the ld-runtime seat-lookup endpoint (caller fetch, pure decisions): UNEDITED issue COMMENTS only; sid anchored to the head commit-s LAST Lab-Run: trailer under the run-id grammar ^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$ (ONE rule for every project — sendrast-v0.7.2-chain passes AS the trailer value; no per-project shape); verdict posted INSIDE its own sid-s stream (first..last + slack); rows present; served pairs admissible in the exact-pair alias table (SERVED trusted over the MODEL line, bare or provider-qualified); caller correlation ids refused; PAIRWISE independence for any panel size; TWO distinct verified seats for EVERY gated landing (SEATSID rule); FORGEJO_* policy envs unread on gated repos; created_at required; endpoint echo-checked, unreadable-refused, address chosen in code from where the process runs (cluster DNS in a pod, 127.0.0.1:8097 on mgmt via the port-forward unit — Amendment 8). A CONCERN clears only by the same seat session-s own follow-up comment (N14).
  • Every blocker fix ships a test that FAILS on the old code (reviews B1–B7, N4–N8, N14–N15; quoted on this PR). N6 (reviewer-set pin) splits out ahead of this PR per Amendment 15.

Proof

  • vitest run at head ea174d3: 57 files / 1341 pass; the 1 failure is tests/merge-identity.test.ts (ambient landing token) — identical on untouched base. tsc --noEmit clean.
  • no independent walk: implementer-run checks only.

Delivery note (Amendments 7/10)

Merging is not delivery. Delivery requires all seven recorded (Sendrast release landed; ld-director-go + test LD-GO; Sendrast seat brief proven; (n) live; read-back; N9; mgmt unit) plus SID-SHAPE — homelab-88 owns the ConfigMap regen/recreate and the mgmt copy.

Lab-Run: LR-20261111-017

r5 round (stacked; base..head now includes the lock-hardening merge)

  • Merge 86c3148: #131 merged in — one owner for the gated symbols, both sides' pins kept, 1390 tests green on the merged tree (n22's deliverable).
  • n17 (BLOCKING): the seat-lookup address is chosen from the service-account mount (namespace non-empty + ca.crt) — never an env var; in-cluster callers never use loopback; the honest limit names the mgmt loopback residual (docs corrected). Red→green: 3 n17 tests + the window test fail on the old code.
  • n18: a space-bearing Lab-Run: value (foo bar, trailing space) is trimmed-then-grammar-checked and refused with the grammar sentence; sendrast-v0.7.2-chain stays valid.
  • n19: review and inline N4 caller tests (missing created_at refuses on gated repos).
  • n20: the answer echoes the bare queried sid (asserted); infra-core#941's recorded test is the endpoint anchor.
  • Judged open (with reasons): n16 (structural — withdrawal needs the seat plan inside the holds leg), n21 (the pinned refusal sentence is a paraphrase of the Amendment 11 quote), n23 (last>now refuses — a tolerance is a spec decision). Amendment 14 is OUT of r5 (follow-up per AMENDMENT 17).
  • Reviewer verdict: APPROVED (56-file review, one doc nit fixed as the stacked doc commit).

HEAD 5d6385c5b6; patch piext-125-diff.patch (7675 lines) in the epic folder. Seats on this head review the full base..head diff.

r5m: main (8c666fbe, landed #131) merged in — the head homelab-93 reviews

Merge commit 1a7d450 (RULE-5, parents 5d6385c + 8c666fb). Zero textual conflicts; semantic resolutions: #131's ServerRepoView/repoKey/GO_GATED_REPOS are the sole owners; ONE gated decision (gatedFromRepoArg over the SERVER's pr.base.repo.full_name) feeds the GO leg, seat leg, N14, created_at and requiredSeats — no leg re-derives from the caller string; landing, census and the verdict preview all pass the server view. #131's refusals all survive (1400 tests green on the merged tree; B8/B1/B2 and the knob-pin tests intact). Fixtures gained the GO/seat/run-trailer evidence the merged legs require; assertions untouched. n17 as approved: AMENDMENT 17's own words are "the service-account namespace file plus ca.crt" — the code matches exactly; the later "SA token" phrase was read as loose wording (flagged, not re-coded).

HEAD 1a7d450d30; base for review 8c666fbec4; patches: piext-125-diff.patch (8c666fb..HEAD, 5953 lines) and piext-125-interdiff-r5m.patch (5d6385c..HEAD, 1417 lines), both in the epic folder. 1400 tests pass (1 known pre-existing merge-identity ambient-token failure); tsc clean.

A21 round (stacked on 347ea6e, commit feb1388): marker hardening + move + the R33 interim check

  • Marker moved off /etc/lab-director (a ConfigMap mountpoint in prod pods) to /var/lib/lab-director/seat-lookup-mgmt.
  • Hardened: lstat (a symlink refuses, stat is never used), isFile, root-owned, mode & 0o022 === 0, and a root-owned non-group/other-writable parent chain (/var/lib/lab-director, /var/lib); every refusal is one line naming the failed check. Four mutation tests each kill exactly their mutation (symlink check removed / lstat→stat / mode mask ignored / parent loop skipped).
  • R33 interim check in the lock: on the loopback path, /proc/net/tcp is read for the 127.0.0.1:8097 listener (0100007F:1F91, LISTEN) and the lock refuses unless its owner uid equals listener_uid=<n> from the hardened marker — fail-closed on a missing line, an unparseable uid, no listener, or a duplicate LISTEN row.
  • Marker format (homelab-88's install contract): path /var/lib/lab-director/seat-lookup-mgmt, root:root 0644, under root-owned non-writable parents; body line listener_uid=<uid of the port-forward unit's process>.
  • The 3 merge tests in landing-evidence-complete-b8-017 pin the placement; 'outer boundary' softened in docs and code.
  • 1412 tests green (1 known pre-existing), tsc clean; red→green shown per mutation.

HEAD feb1388691; patches: piext-125-diff.patch (full, 7095 lines) + piext-125-interdiff-a21.patch (from 347ea6e, 1191 lines).

H1 round (stacked on feb1388, commit 6b49447): homelab-93's H1, closing before mgmt delivery

  • The mgmt path refuses listener_uid === process.getuid() (and an unavailable getuid) — the check is never self-answerable.
  • /proc/self/uid_map AND gid_map must each be exactly 0 0 4294967295 — a user namespace (which can fake root-owned markers) refuses, one line naming the map.
  • TOCTOU re-read: after the lookups pass, /proc/net/tcp is read again and the 127.0.0.1:8097 LISTEN row's inode must equal the first read — a swapped listener refuses.
  • Install note: the port-forward runs as root or a dedicated uid no agent can become.
  • A red→green test per check (self-owned uid, wrong uid_map, wrong gid_map, changed inode); 1417 tests green; tsc clean.

HEAD 6b4944791c; patches: piext-125-diff.patch (full, 7411) + piext-125-interdiff-h1.patch (from feb1388, 558) + the earlier interdiffs.

A23 round (stacked on 6b49447, commit 83c4c9f): homelab-93's three pre-delivery fixes

  • The loopback path refuses a ROOT querier (process.getuid() === 0) — an agent process must never satisfy its own listener check; /proc can be faked inside a user namespace.
  • The uid+inode TOCTOU re-check moved INTO readSeatLookup, after the evidence lookups and immediately before the request; the decision returns the verified LISTEN inode; a swap between decision and dial refuses.
  • The identity user-namespace maps are injected through the readText seam in every test that reaches the check (default 0 0 4294967295; no real /proc reads left; a userns mapping still refuses) — rootless CI.
  • A red→green test per fix; 1419 tests green; tsc clean.

HEAD 83c4c9f8af; patches: piext-125-diff.patch (full, 7634) + piext-125-interdiff-a23.patch (from 6b49447, 562) + earlier interdiffs.

S1+S2 round (stacked on 83c4c9f, commit fe17755): review-3c's two pre-delivery fixes

  • S1: the TOCTOU re-read now happens TWICE — the pre-dial read stays (a visible swap is never dialled) AND a second read AFTER Promise.all discards the fetched answers on any uid/inode mismatch → unreadable (the socket-identity proof: a LISTEN socket cannot vacate the address and return with the same inode). New [A,A,B] test: fetch returns VALID echoing answers and the result is still unreadable; [A,B,B] keeps the pre-dial leg; the comments and docs bullet now describe the true order; mutations (post-read removed / pre-dial removed) each fail a test.
  • S2: the querier's uid is injected through a getuid seam on SeatLookupPlacement (production still process.getuid); both mgmt test helpers inject 1005; the root test pins 0. unshare is unavailable here, so the rootless proof used a getuid=0 preload: 24 failed on 83c4c9f → 65/65 green.
  • Nits n1/n2: the getuid-undefined refusal test; the direct fetchSeatAnswer re-read tested (comment: no production caller today).
  • 1422 tests green; tsc clean.

HEAD fe1775593a; patches: piext-125-diff.patch (full, 7821) + piext-125-interdiff-s1s2.patch (from 83c4c9f, 525) + earlier interdiffs.

Spec 017 (Amendments 1, 2, 4, 8, 11, 15-prep), piece (l): the landing census in `forgejo/landing.ts` — the merge gate the spec-s Design section and Amendment 1 describe, hardened through reviews r1–r4. This PR changes the landing gate itself (`forgejo/**`), so it is dangerous class: two independent seat sessions are required, which the pane seats on this PR provide. HEAD NOTE: main (#126/#127/#128) is merged in at `ea174d3` (RULE-5); the full base..head diff is what the seats review. ## What a person gets A merge into `Homelab/infra-core` or `Homelab/prod` through the landing tool happens only when El Director-s go exists for that exact PR and head (a forgotten go is a refusal, not a slip), AND both review seats are verified against the gateway record — their sids real, their served models admissible, their sessions distinct. Enforcement by tool, not by memory. ## The gate (all proven by fixture tests against the pure lock — no live endpoint, no live Forgejo) - **GO record**: `LD-GO repo=<owner/name> pr=<n> head=<40-hex>` by the `ld-director-go` identity (named in code), current-head match, never edited, unreadable refused; gating reads the SERVER-s repo identity (`pr.base.repo.full_name`). - **Seat verification** via the ld-runtime seat-lookup endpoint (caller fetch, pure decisions): UNEDITED issue COMMENTS only; sid anchored to the head commit-s LAST `Lab-Run:` trailer under the run-id grammar `^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$` (ONE rule for every project — `sendrast-v0.7.2-chain` passes AS the trailer value; no per-project shape); verdict posted INSIDE its own sid-s stream (first..last + slack); rows present; served pairs admissible in the exact-pair alias table (SERVED trusted over the MODEL line, bare or provider-qualified); caller correlation ids refused; PAIRWISE independence for any panel size; TWO distinct verified seats for EVERY gated landing (SEATSID rule); `FORGEJO_*` policy envs unread on gated repos; `created_at` required; endpoint echo-checked, unreadable-refused, address chosen in code from where the process runs (cluster DNS in a pod, `127.0.0.1:8097` on mgmt via the port-forward unit — Amendment 8). A CONCERN clears only by the same seat session-s own follow-up comment (N14). - Every blocker fix ships a test that FAILS on the old code (reviews B1–B7, N4–N8, N14–N15; quoted on this PR). N6 (reviewer-set pin) splits out ahead of this PR per Amendment 15. ## Proof - `vitest run` at head `ea174d3`: 57 files / 1341 pass; the 1 failure is `tests/merge-identity.test.ts` (ambient landing token) — identical on untouched base. `tsc --noEmit` clean. - **no independent walk: implementer-run checks only.** ## Delivery note (Amendments 7/10) Merging is not delivery. Delivery requires all seven recorded (Sendrast release landed; ld-director-go + test LD-GO; Sendrast seat brief proven; (n) live; read-back; N9; mgmt unit) plus SID-SHAPE — homelab-88 owns the ConfigMap regen/recreate and the mgmt copy. Lab-Run: LR-20261111-017 ## r5 round (stacked; base..head now includes the lock-hardening merge) - **Merge 86c3148**: #131 merged in — one owner for the gated symbols, both sides' pins kept, 1390 tests green on the merged tree (n22's deliverable). - **n17 (BLOCKING)**: the seat-lookup address is chosen from the service-account mount (namespace non-empty + ca.crt) — never an env var; in-cluster callers never use loopback; the honest limit names the mgmt loopback residual (docs corrected). Red→green: 3 n17 tests + the window test fail on the old code. - **n18**: a space-bearing `Lab-Run:` value (`foo bar`, trailing space) is trimmed-then-grammar-checked and refused with the grammar sentence; `sendrast-v0.7.2-chain` stays valid. - **n19**: review and inline N4 caller tests (missing created_at refuses on gated repos). - **n20**: the answer echoes the bare queried sid (asserted); infra-core#941's recorded test is the endpoint anchor. - **Judged open (with reasons)**: n16 (structural — withdrawal needs the seat plan inside the holds leg), n21 (the pinned refusal sentence is a paraphrase of the Amendment 11 quote), n23 (last>now refuses — a tolerance is a spec decision). Amendment 14 is OUT of r5 (follow-up per AMENDMENT 17). - Reviewer verdict: APPROVED (56-file review, one doc nit fixed as the stacked doc commit). HEAD 5d6385c5b66e5338c2be6bcba02bc4ba2b8d75ce; patch piext-125-diff.patch (7675 lines) in the epic folder. Seats on this head review the full base..head diff. ## r5m: main (8c666fbe, landed #131) merged in — the head homelab-93 reviews Merge commit 1a7d450 (RULE-5, parents 5d6385c + 8c666fb). Zero textual conflicts; semantic resolutions: #131's ServerRepoView/repoKey/GO_GATED_REPOS are the sole owners; ONE `gated` decision (gatedFromRepoArg over the SERVER's pr.base.repo.full_name) feeds the GO leg, seat leg, N14, created_at and requiredSeats — no leg re-derives from the caller string; landing, census and the verdict preview all pass the server view. #131's refusals all survive (1400 tests green on the merged tree; B8/B1/B2 and the knob-pin tests intact). Fixtures gained the GO/seat/run-trailer evidence the merged legs require; assertions untouched. n17 as approved: AMENDMENT 17's own words are "the service-account namespace file plus ca.crt" — the code matches exactly; the later "SA token" phrase was read as loose wording (flagged, not re-coded). HEAD 1a7d450d301195082b1d62e7a7f6db05e4fa10eb; base for review 8c666fbec432f70b4853124f8de6ee2e445d1339; patches: piext-125-diff.patch (8c666fb..HEAD, 5953 lines) and piext-125-interdiff-r5m.patch (5d6385c..HEAD, 1417 lines), both in the epic folder. 1400 tests pass (1 known pre-existing merge-identity ambient-token failure); tsc clean. ## A21 round (stacked on 347ea6e, commit feb1388): marker hardening + move + the R33 interim check - **Marker moved** off /etc/lab-director (a ConfigMap mountpoint in prod pods) to `/var/lib/lab-director/seat-lookup-mgmt`. - **Hardened**: lstat (a symlink refuses, stat is never used), isFile, root-owned, mode & 0o022 === 0, and a root-owned non-group/other-writable parent chain (/var/lib/lab-director, /var/lib); every refusal is one line naming the failed check. Four mutation tests each kill exactly their mutation (symlink check removed / lstat→stat / mode mask ignored / parent loop skipped). - **R33 interim check in the lock**: on the loopback path, /proc/net/tcp is read for the 127.0.0.1:8097 listener (0100007F:1F91, LISTEN) and the lock refuses unless its owner uid equals `listener_uid=<n>` from the hardened marker — fail-closed on a missing line, an unparseable uid, no listener, or a duplicate LISTEN row. - **Marker format (homelab-88's install contract)**: path `/var/lib/lab-director/seat-lookup-mgmt`, root:root 0644, under root-owned non-writable parents; body line `listener_uid=<uid of the port-forward unit's process>`. - The 3 merge tests in landing-evidence-complete-b8-017 pin the placement; 'outer boundary' softened in docs and code. - 1412 tests green (1 known pre-existing), tsc clean; red→green shown per mutation. HEAD feb138869181ed580f5a549dc0d70ab7c99f6be6; patches: piext-125-diff.patch (full, 7095 lines) + piext-125-interdiff-a21.patch (from 347ea6e, 1191 lines). ## H1 round (stacked on feb1388, commit 6b49447): homelab-93's H1, closing before mgmt delivery - The mgmt path refuses `listener_uid === process.getuid()` (and an unavailable getuid) — the check is never self-answerable. - /proc/self/uid_map AND gid_map must each be exactly `0 0 4294967295` — a user namespace (which can fake root-owned markers) refuses, one line naming the map. - TOCTOU re-read: after the lookups pass, /proc/net/tcp is read again and the 127.0.0.1:8097 LISTEN row's inode must equal the first read — a swapped listener refuses. - Install note: the port-forward runs as root or a dedicated uid no agent can become. - A red→green test per check (self-owned uid, wrong uid_map, wrong gid_map, changed inode); 1417 tests green; tsc clean. HEAD 6b4944791c4e66aa72372049115794935450b6e6; patches: piext-125-diff.patch (full, 7411) + piext-125-interdiff-h1.patch (from feb1388, 558) + the earlier interdiffs. ## A23 round (stacked on 6b49447, commit 83c4c9f): homelab-93's three pre-delivery fixes - The loopback path refuses a ROOT querier (`process.getuid() === 0`) — an agent process must never satisfy its own listener check; /proc can be faked inside a user namespace. - The uid+inode TOCTOU re-check moved INTO `readSeatLookup`, after the evidence lookups and immediately before the request; the decision returns the verified LISTEN inode; a swap between decision and dial refuses. - The identity user-namespace maps are injected through the readText seam in every test that reaches the check (default `0 0 4294967295`; no real /proc reads left; a userns mapping still refuses) — rootless CI. - A red→green test per fix; 1419 tests green; tsc clean. HEAD 83c4c9f8af79eafff7eb9d7ffdb42e33c34bd443; patches: piext-125-diff.patch (full, 7634) + piext-125-interdiff-a23.patch (from 6b49447, 562) + earlier interdiffs. ## S1+S2 round (stacked on 83c4c9f, commit fe17755): review-3c's two pre-delivery fixes - **S1**: the TOCTOU re-read now happens TWICE — the pre-dial read stays (a visible swap is never dialled) AND a second read AFTER `Promise.all` discards the fetched answers on any uid/inode mismatch → unreadable (the socket-identity proof: a LISTEN socket cannot vacate the address and return with the same inode). New [A,A,B] test: fetch returns VALID echoing answers and the result is still unreadable; [A,B,B] keeps the pre-dial leg; the comments and docs bullet now describe the true order; mutations (post-read removed / pre-dial removed) each fail a test. - **S2**: the querier's uid is injected through a `getuid` seam on SeatLookupPlacement (production still `process.getuid`); both mgmt test helpers inject 1005; the root test pins 0. unshare is unavailable here, so the rootless proof used a getuid=0 preload: 24 failed on 83c4c9f → 65/65 green. - Nits n1/n2: the getuid-undefined refusal test; the direct fetchSeatAnswer re-read tested (comment: no production caller today). - 1422 tests green; tsc clean. HEAD fe1775593a5dc343c6a90639647f910b44af972a; patches: piext-125-diff.patch (full, 7821) + piext-125-interdiff-s1s2.patch (from 83c4c9f, 525) + earlier interdiffs.
feat(forgejo): the landing census gate for infra-core and prod (LR-20261111-017, Amendments 1/2/4)
Some checks failed
CI / pr-title (pull_request) Failing after 0s
Details
CI / forgejo-ci (pull_request) Successful in 1m1s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m13s
Details
tests / python (pull_request) Successful in 6s
Details
CI / plane (pull_request) Successful in 1m32s
Details
CI / nats-intercom (pull_request) Successful in 30s
Details
CI / mcp-bridge (pull_request) Successful in 31s
Details
CI / skill-args (pull_request) Successful in 44s
Details
CI / pi-friction (pull_request) Successful in 41s
Details
CI / redact (pull_request) Successful in 41s
Details
CI / todo (pull_request) Successful in 1m0s
Details
CI / ask-user-question (pull_request) Successful in 1m0s
Details
CI / forgejo (pull_request) Successful in 2m2s
Details
CI / refocus (pull_request) Successful in 1m4s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m54s
Details
0a6798f75d 
For the two repos named in code (Homelab/infra-core, Homelab/prod) the
landing lock now refuses unless BOTH hold:

- GO record: an LD-GO repo=<owner/name> pr=<n> head=<40-hex> comment by the
  ld-director-go identity (named in code) matching the repo, the PR and the
  PR's current full 40-hex head, never edited (updated_at == created_at);
  unreadable comments or an unreadable edit state refuse. Never a short sha,
  never another author, never a bypass.
- Seat verification: every counted verdict's declared sid is checked against
  the ld-runtime seat-lookup endpoint answers (fetched by the caller, decided
  here): rows exist in the seat's window; every (requested, served) pair is
  admissible in the exact-pair alias table — the SERVED model is trusted over
  the MODEL line; the two seats are distinct sessions (same sid twice,
  shared caller correlation ids, or a free-form tag all refuse); missing
  SESSION or MODEL lines refuse; an unreachable or unreadable endpoint
  refuses. The run id is anchored from the head commit's last Lab-Run
  trailer. Amendment 4: sids, streams and models only — no anchoring checks.

Other repos are unchanged. Refusals name what is missing in one line. Tests
are fixtures against the pure lock — no live endpoint, no live Forgejo.

Lab-Run: LR-20261111-017
pi-landing changed title from LR-20261111-017 (l): the landing census gate — GO record + seat sid verification for infra-core and prod to HOMELAB-1679: feat(forgejo): landing census gate — GO record + seat sid verification for infra-core and prod (LR-20261111-017 l) 2026-11-11 12:50:19 +00:00
ld-reviewer commented 2026-11-11 12:54:30 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=0a6798f75d78fd8fd027a92e9d400a6993b5c603 session=LR-20261111-017-seatA-20261111T1250Z

Head verified: the PR's current head IS 0a6798f75d (one commit over 0c5ef84), so the pin is exact and has not moved. Provenance note for the record: this sha and this change (GO leg + seat-sid leg in forgejo/landing.ts) live in this repository; Homelab/infra-core#125 is an unrelated, already-merged PR whose head is not this sha, so nothing here was judged against it.

WHAT I CHECKED

  • Diff (origin/main...head, read in full for the new paths): forgejo/landing.ts (GO_RECORD_RE anchored at :4438, GO_GATED_REPOS :4411, MODEL_PAIRS :4546, seatVerificationPlan with the 24h CAP_MS :4710-4718, GO leg at :5595 placed after head identity and before the ledger, seat leg at :6094 placed last before the permit), forgejo/api.ts getHeadCommitFacts (date and message from one commit read; the date chain is unchanged), forgejo/index.ts caller wiring (:322-411 lookup fetch, :562 plan built from the same ledger the lock judges).
  • Contract: one literal sid per query — the caller sets exactly one sid plus an optional other, no search/enumeration parameter anywhere; other is sent only when the counted set is exactly two seats (landing.ts :4710 ff.), which is the only case where "are these two distinct" has a meaning; the window is capped at 24h for a stale head and padded 30 min; refusals are single-line strings in the lock's own words, asserted as such by the fixtures; nothing from upstream reaches a caller — every non-200, timeout (10s abort, index.ts :322), unparseable body and malformed URL collapses to { status: "unreadable" } in the caller (:337-411) and is refused by the lock as unreadable, never as "no rows".
  • Fail-closed directions that matter: absent updatedAt on any comment is a refusal on the gated repos only (an edit state nobody read cannot pass a backfilled go); an empty comment read is not an empty store; an answer naming another sid is unreadable, not absence; served pair must be a row a human wrote down, with no equality and no prefix shortcut; MODEL line is checked against the served pair, not the reverse.
  • Non-gated repos: both legs return before touching items, so their results are unchanged — pinned by an explicit control test rather than asserted in prose.
  • Tests, executed here: vitest run forgejo/tests/landing-go-record.test.ts forgejo/tests/landing-seat-verification.test.ts → 38 passed, 0 failed. They are non-vacuous: each refusal is matched on its own wording (no-record, other-head, other-pr, other-repo, wrong-author, edited, unreadable edit state, short sha; zero settled rows, unobserved pair, served/declared disagreement, same sid twice, shared correlation id, non-distinct, unreadable endpoint, free-form tag, tag for another run, missing SESSION, missing MODEL, missing Lab-Run trailer), plus the positive paths (newest unedited record wins; full green pair permits and carries the per-seat facts).
  • forgejo/tests/landing-gated-caller.test.ts (195 lines) could NOT be executed in my sandbox: it imports index.ts, which imports @mariozechner/pi-ai and @mariozechner/pi-coding-agent, neither installed here — the same two module-resolution errors are the only output of tsc --noEmit. That is my environment, not the change: landing.ts and api.ts typecheck clean. I read the file line by line instead: no-GO refusal with zero POST /merge, 502 lookup → "could not be read" refusal, malformed FORGEJO_SEAT_LOOKUP_URL → one-line refusal and zero endpoint calls, missing trailer refusal, and the green path asserting exactly two lookups, one literal sid each, other set, span ≤ 24h, merge posted once.
  • Guard: scripts/gits-sbx-epic-test.sh lives in the endpoint's repo (infra-core), not this one; the NEW FAILURES: 0 result was given to me and I did not re-run it. What I did verify here is the exit state of the two pure test files and the typecheck above.

HOLDS: none.

Non-blocking notes (not holds): (1) there is no explicit 429 fixture; a rate-limited lookup is covered by the same !res.ok branch as the tested 502 (index.ts fetchSeatAnswer), so the behaviour is right but the case is unpinned — worth one line next time. (2) SeatFact (landing.ts :4496) carries rows/models/distinct/correlation-id counts but not first/last, so the census prints the row count and pairs while the endpoint's first/last facts are validated and dropped; fine for this gate, just narrower than the answer.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1250Z

LD-VERDICT APPROVED head=0a6798f75d78fd8fd027a92e9d400a6993b5c603 session=LR-20261111-017-seatA-20261111T1250Z Head verified: the PR's current head IS 0a6798f75d78fd8fd027a92e9d400a6993b5c603 (one commit over 0c5ef84), so the pin is exact and has not moved. Provenance note for the record: this sha and this change (GO leg + seat-sid leg in forgejo/landing.ts) live in this repository; Homelab/infra-core#125 is an unrelated, already-merged PR whose head is not this sha, so nothing here was judged against it. WHAT I CHECKED - Diff (origin/main...head, read in full for the new paths): forgejo/landing.ts (GO_RECORD_RE anchored at :4438, GO_GATED_REPOS :4411, MODEL_PAIRS :4546, seatVerificationPlan with the 24h CAP_MS :4710-4718, GO leg at :5595 placed after head identity and before the ledger, seat leg at :6094 placed last before the permit), forgejo/api.ts getHeadCommitFacts (date and message from one commit read; the date chain is unchanged), forgejo/index.ts caller wiring (:322-411 lookup fetch, :562 plan built from the same ledger the lock judges). - Contract: one literal sid per query — the caller sets exactly one `sid` plus an optional `other`, no search/enumeration parameter anywhere; `other` is sent only when the counted set is exactly two seats (landing.ts :4710 ff.), which is the only case where "are these two distinct" has a meaning; the window is capped at 24h for a stale head and padded 30 min; refusals are single-line strings in the lock's own words, asserted as such by the fixtures; nothing from upstream reaches a caller — every non-200, timeout (10s abort, index.ts :322), unparseable body and malformed URL collapses to { status: "unreadable" } in the caller (:337-411) and is refused by the lock as unreadable, never as "no rows". - Fail-closed directions that matter: absent `updatedAt` on any comment is a refusal on the gated repos only (an edit state nobody read cannot pass a backfilled go); an empty comment read is not an empty store; an answer naming another sid is unreadable, not absence; served pair must be a row a human wrote down, with no equality and no prefix shortcut; MODEL line is checked against the served pair, not the reverse. - Non-gated repos: both legs return before touching items, so their results are unchanged — pinned by an explicit control test rather than asserted in prose. - Tests, executed here: `vitest run forgejo/tests/landing-go-record.test.ts forgejo/tests/landing-seat-verification.test.ts` → 38 passed, 0 failed. They are non-vacuous: each refusal is matched on its own wording (no-record, other-head, other-pr, other-repo, wrong-author, edited, unreadable edit state, short sha; zero settled rows, unobserved pair, served/declared disagreement, same sid twice, shared correlation id, non-distinct, unreadable endpoint, free-form tag, tag for another run, missing SESSION, missing MODEL, missing Lab-Run trailer), plus the positive paths (newest unedited record wins; full green pair permits and carries the per-seat facts). - forgejo/tests/landing-gated-caller.test.ts (195 lines) could NOT be executed in my sandbox: it imports index.ts, which imports @mariozechner/pi-ai and @mariozechner/pi-coding-agent, neither installed here — the same two module-resolution errors are the only output of `tsc --noEmit`. That is my environment, not the change: landing.ts and api.ts typecheck clean. I read the file line by line instead: no-GO refusal with zero POST /merge, 502 lookup → "could not be read" refusal, malformed FORGEJO_SEAT_LOOKUP_URL → one-line refusal and zero endpoint calls, missing trailer refusal, and the green path asserting exactly two lookups, one literal sid each, `other` set, span ≤ 24h, merge posted once. - Guard: scripts/gits-sbx-epic-test.sh lives in the endpoint's repo (infra-core), not this one; the NEW FAILURES: 0 result was given to me and I did not re-run it. What I did verify here is the exit state of the two pure test files and the typecheck above. HOLDS: none. Non-blocking notes (not holds): (1) there is no explicit 429 fixture; a rate-limited lookup is covered by the same `!res.ok` branch as the tested 502 (index.ts fetchSeatAnswer), so the behaviour is right but the case is unpinned — worth one line next time. (2) SeatFact (landing.ts :4496) carries rows/models/distinct/correlation-id counts but not first/last, so the census prints the row count and pairs while the endpoint's first/last facts are validated and dropped; fine for this gate, just narrower than the answer. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1250Z
ld-reviewer commented 2026-11-11 12:55:46 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=0a6798f75d78fd8fd027a92e9d400a6993b5c603 session=LR-20261111-017-seatB-20261111T1251Z

Repo label correction: the brief named Homelab/infra-core #125, but that PR is an unrelated closed/merged change (HOMELAB-519) and the pinned 40-hex head does not exist in that repository at all. The pinned head resolves exactly, and only, in Homelab/pi-extensions #125 (lr-20261111-017-census → main, base 0c5ef847) — reviewed there, head unchanged at the pin above.

What I checked (fresh clone, detached at the pin; diff against origin/main 0c5ef847):

  • Diff read in full: forgejo/landing.ts (+728: goRecordLeg, GO_GATED_REPOS/GO_AUTHOR_IDENTITY in code, anchored 40-hex GO regex, seatVerificationPlan with 30-min pad and 24 h cap, counte
    dSeats, seat leg 5a–5f), forgejo/index.ts (caller wiring: one fetch per planned sid, 10 s abort, all failures → unreadable, gated repos only; updatedAt and commit message carried into the lock; census prints GO and SEATS), forgejo/api.ts, forgejo/verdict.ts, forgejo/verdict-cli.ts, forgejo/tests/fake.ts.
  • Contract: one literal sid per query, no enumeration or pattern input (leg 5 requires the declared sessions to equal the anchored ^<run>-seat(A|B)-<stamp>$ for the head commit's LAST Lab-Run: trailer; no substring or case-insensitive match); window bounded (planner caps at 24 h; the endpoint refuses a wider one); refusals are single-line and name the missing fact; a lookup failure never throws out of the gate and never reads as "no rows" (forgejo/index.ts:344-407).
  • Audit and facts, checked against the landed dependency the gate trusts (infra-core live, seatlookup/lookup.go, record/seat_lookups.go, cmd/ld-runtime/seat_lookup.go): exactly one route, sid count must be 1 (?sid=a&sid=b refused), no search/pattern/LIKE input, rate limiter on every method, rows/model pairs/first-last/distinct/correlation_ids/shared_correlation_ids answered; every query audited insert-only (one INSERT, no UPDATE/DELETE) on a detached bounded context (context.WithoutCancel + timeout), 42P01 named (ErrSeatLookupsAbsent) and non-fatal — the answer stands, the miss is logged; upstream error text is logged only, never returned to the caller.
  • Tests: 43 pass on the three named files (23 landing-seat-verification, 15 landing-go-record, 5 landing-gated-caller) and they are non-vacuous — each refusal asserts the one-line text and the named fact, the permit asserts the recorded per-seat facts and the parsed GO comment id, and the wiring tests assert zero merge POSTs on every refusal and exactly one lookup per sid with other named and a bounded window. Full suite 1255/1256; the single failure (tests/merge-identity.test.ts) reproduces identically on an untouched origin/main worktree in this sandbox — pre-existing, ambient landing token. tsc --noEmit clean.
  • The spec's four named endpoint cases exist and pass: TestOneSIDOnly, TestEnumerationRefused, TestRateLimit, TestInsertSeatLookupIsInsertOnly (go test ./seatlookup/... ./record/... ./relay/... ./cmd/ld-runtime/... all ok; go build ./... ok; go test . fails only because that directory has no Go files — pre-existing layout).
  • Guard context accepted as given for this head: NEW FAILURES: 0 (live and head FAIL sets identical).

Holds: none.

Non-blocking notes: (1) the description says "44 fixture tests"; the three named files total 43 (23 + 15 + 5) — cosmetic. (2) On a gated repo the census issues up to two lookups per PR against a 2/s + 10 burst endpoint, so a long queue can surface honest "endpoint could not be read" rows; fail-closed, no bypass.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1251Z

LD-VERDICT APPROVED head=0a6798f75d78fd8fd027a92e9d400a6993b5c603 session=LR-20261111-017-seatB-20261111T1251Z Repo label correction: the brief named `Homelab/infra-core` #125, but that PR is an unrelated closed/merged change (HOMELAB-519) and the pinned 40-hex head does not exist in that repository at all. The pinned head resolves exactly, and only, in `Homelab/pi-extensions` #125 (`lr-20261111-017-census` → `main`, base 0c5ef847) — reviewed there, head unchanged at the pin above. What I checked (fresh clone, detached at the pin; diff against `origin/main` 0c5ef847): - Diff read in full: `forgejo/landing.ts` (+728: goRecordLeg, GO_GATED_REPOS/GO_AUTHOR_IDENTITY in code, anchored 40-hex GO regex, seatVerificationPlan with 30-min pad and 24 h cap, counte dSeats, seat leg 5a–5f), `forgejo/index.ts` (caller wiring: one fetch per planned sid, 10 s abort, all failures → `unreadable`, gated repos only; `updatedAt` and commit message carried into the lock; census prints GO and SEATS), `forgejo/api.ts`, `forgejo/verdict.ts`, `forgejo/verdict-cli.ts`, `forgejo/tests/fake.ts`. - Contract: one literal sid per query, no enumeration or pattern input (leg 5 requires the declared sessions to equal the anchored `^<run>-seat(A|B)-<stamp>$` for the head commit's LAST `Lab-Run:` trailer; no substring or case-insensitive match); window bounded (planner caps at 24 h; the endpoint refuses a wider one); refusals are single-line and name the missing fact; a lookup failure never throws out of the gate and never reads as "no rows" (`forgejo/index.ts:344-407`). - Audit and facts, checked against the landed dependency the gate trusts (`infra-core` live, `seatlookup/lookup.go`, `record/seat_lookups.go`, `cmd/ld-runtime/seat_lookup.go`): exactly one route, `sid` count must be 1 (`?sid=a&sid=b` refused), no search/pattern/LIKE input, rate limiter on every method, rows/model pairs/first-last/distinct/`correlation_ids`/`shared_correlation_ids` answered; every query audited insert-only (one INSERT, no UPDATE/DELETE) on a detached bounded context (`context.WithoutCancel` + timeout), 42P01 named (`ErrSeatLookupsAbsent`) and non-fatal — the answer stands, the miss is logged; upstream error text is logged only, never returned to the caller. - Tests: 43 pass on the three named files (23 `landing-seat-verification`, 15 `landing-go-record`, 5 `landing-gated-caller`) and they are non-vacuous — each refusal asserts the one-line text and the named fact, the permit asserts the recorded per-seat facts and the parsed GO comment id, and the wiring tests assert zero merge POSTs on every refusal and exactly one lookup per sid with `other` named and a bounded window. Full suite 1255/1256; the single failure (`tests/merge-identity.test.ts`) reproduces identically on an untouched `origin/main` worktree in this sandbox — pre-existing, ambient landing token. `tsc --noEmit` clean. - The spec's four named endpoint cases exist and pass: `TestOneSIDOnly`, `TestEnumerationRefused`, `TestRateLimit`, `TestInsertSeatLookupIsInsertOnly` (`go test ./seatlookup/... ./record/... ./relay/... ./cmd/ld-runtime/...` all ok; `go build ./...` ok; `go test .` fails only because that directory has no Go files — pre-existing layout). - Guard context accepted as given for this head: NEW FAILURES: 0 (live and head FAIL sets identical). Holds: none. Non-blocking notes: (1) the description says "44 fixture tests"; the three named files total 43 (23 + 15 + 5) — cosmetic. (2) On a gated repo the census issues up to two lookups per PR against a 2/s + 10 burst endpoint, so a long queue can surface honest "endpoint could not be read" rows; fail-closed, no bypass. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1251Z
pi-landing changed title from HOMELAB-1679: feat(forgejo): landing census gate — GO record + seat sid verification for infra-core and prod (LR-20261111-017 l) to HOMELAB-1679: feat(forgejo): landing census gate — GO record + seat sid verification for infra-core and prod (LR-20261111-017, piece l) 2026-11-11 12:56:32 +00:00
pi-landing force-pushed lr-20261111-017-census from 0a6798f75d
Some checks failed
CI / pr-title (pull_request) Failing after 0s
Details
CI / forgejo-ci (pull_request) Successful in 1m1s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m13s
Details
tests / python (pull_request) Successful in 6s
Details
CI / plane (pull_request) Successful in 1m32s
Details
CI / nats-intercom (pull_request) Successful in 30s
Details
CI / mcp-bridge (pull_request) Successful in 31s
Details
CI / skill-args (pull_request) Successful in 44s
Details
CI / pi-friction (pull_request) Successful in 41s
Details
CI / redact (pull_request) Successful in 41s
Details
CI / todo (pull_request) Successful in 1m0s
Details
CI / ask-user-question (pull_request) Successful in 1m0s
Details
CI / forgejo (pull_request) Successful in 2m2s
Details
CI / refocus (pull_request) Successful in 1m4s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m54s
Details
to d4904c6663
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 49s
Details
CI / pi-otel-telemetry (pull_request) Successful in 53s
Details
CI / skill-args (pull_request) Successful in 56s
Details
CI / todo (pull_request) Successful in 1m0s
Details
CI / plane (pull_request) Successful in 1m25s
Details
CI / mcp-bridge (pull_request) Successful in 23s
Details
CI / ask-user-question (pull_request) Successful in 50s
Details
CI / forgejo (pull_request) Successful in 2m2s
Details
CI / refocus (pull_request) Successful in 57s
Details
tests / python (pull_request) Successful in 6s
Details
CI / nats-intercom (pull_request) Successful in 25s
Details
CI / pi-friction (pull_request) Successful in 37s
Details
CI / redact (pull_request) Successful in 41s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m41s
Details
2026-11-11 12:59:19 +00:00 Compare
ld-reviewer commented 2026-11-11 13:05:41 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=d4904c666396954a70da39ce422b993469be1535 session=LR-20261111-017-seatA-20261111T1301Z

Seat A, fresh single-run review, read-only clone at the exact pinned head.

Route note (not a change request): my brief named "Homelab/infra-core PR #125"; that PR is a different, merged change and the pin d4904c66… does not exist in that repo. The pin, the branch lr-20261111-017-census and the described diff (forgejo/landing.ts, GO record + seat-sid legs, 44 fixture tests) exist here in Homelab/pi-extensions#125. I verified the head in this PR's metadata before reviewing (d4904c666396954a70da39ce422b993469be1535), so this verdict is for pi-extensions#125.

Checked (diff against origin/main = 0c5ef84, not the PR text):

  1. Contract — one literal sid per query: seatVerificationPlan emits one request per declared seat, sid set once as a literal (no pattern/enumeration), other only when exactly two seats; window span capped at 24 h (forgejo/landing.ts:4710-4737), which equals the endpoint's DefaultMaxWindow (= relay.DefaultWindow, 24 h) so the cap cannot become a 400; caller targets exactly /v1/seat-lookup with sid/start/end/other (forgejo/index.ts:340-400). Every non-2xx, timeout, malformed URL and unparseable body becomes {status:"unreadable"} — no upstream body text is ever surfaced (no res.text() on that path), and a rate-limit answer therefore refuses closed.
  2. Audit — this PR adds no writes; the insert-only seat_lookups audit and the named non-fatal 42P01 stay endpoint-side (landed with (e)). Nothing here logs a query.
  3. Facts answered — SeatFact carries rows, served pairs, distinct, correlation-id count and shared count; the lock requires rows > 0, every served pair present in the single reviewed alias row, the declared model: equal to that row's declared, distinct === true, and shared_correlation_ids === 0 (forgejo/landing.ts:6167-6255). The GO leg is anchored and full-40-hex only, author-gated, and refuses an empty comment read as well as an absent update time or updated_at != created_at (forgejo/landing.ts:4750-4830). Absent seatLookup/message/Lab-Run: trailer all refuse in the lock's own single line.
  4. Tests — ran vitest run forgejo/tests on this head: 870 tests pass across the 36 files that load; the 38 new pure-lock tests pass (23 seat-verification, 15 go-record). The 17 files that fail to load do so only because @mariozechner/pi-ai is not installed in this seat sandbox (module resolution, not a code fault), so I read landing-gated-caller.test.ts rather than executing it: it asserts exactly two GET /v1/seat-lookup calls, sid ∈ {A,B}, other named, span ≤ 24 h, zero merge POSTs on 502 and on a malformed URL, and one merge only on the green path — non-vacuous in both directions. Guard context as given: NEW FAILURES 0 (identical FAIL sets on live and head).

Notes, none blocking: (a) the endpoint's first/last are typed in SeatLookupAnswer but not carried into SeatFact, so the census shows rows and pairs but not the evidence window; (b) forgejo/verdict.ts:757 hardcodes seatLookup: {status:"unreadable"}, so a gated-repo preview always refuses on the seat leg — I confirmed posting is gated on report.mayPost, not lock.ok (forgejo/verdict-cli.ts:1019-1026,1251-1257), so seats can still post verdicts on gated repos; (c) the window starts 30 min before the earliest evidence, so a seat whose calls all predate that would read 0 rows and refuse — fail-closed, matching the spec's stated padding.

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1301Z

LD-VERDICT APPROVED head=d4904c666396954a70da39ce422b993469be1535 session=LR-20261111-017-seatA-20261111T1301Z Seat A, fresh single-run review, read-only clone at the exact pinned head. Route note (not a change request): my brief named "Homelab/infra-core PR #125"; that PR is a different, merged change and the pin `d4904c66…` does not exist in that repo. The pin, the branch `lr-20261111-017-census` and the described diff (`forgejo/landing.ts`, GO record + seat-sid legs, 44 fixture tests) exist here in `Homelab/pi-extensions#125`. I verified the head in this PR's metadata before reviewing (`d4904c666396954a70da39ce422b993469be1535`), so this verdict is for pi-extensions#125. Checked (diff against `origin/main` = 0c5ef84, not the PR text): 1. Contract — one literal sid per query: `seatVerificationPlan` emits one request per declared seat, `sid` set once as a literal (no pattern/enumeration), `other` only when exactly two seats; window span capped at 24 h (`forgejo/landing.ts:4710-4737`), which equals the endpoint's `DefaultMaxWindow` (= `relay.DefaultWindow`, 24 h) so the cap cannot become a 400; caller targets exactly `/v1/seat-lookup` with sid/start/end/other (`forgejo/index.ts:340-400`). Every non-2xx, timeout, malformed URL and unparseable body becomes `{status:"unreadable"}` — no upstream body text is ever surfaced (no `res.text()` on that path), and a rate-limit answer therefore refuses closed. 2. Audit — this PR adds no writes; the insert-only `seat_lookups` audit and the named non-fatal 42P01 stay endpoint-side (landed with (e)). Nothing here logs a query. 3. Facts answered — `SeatFact` carries rows, served pairs, distinct, correlation-id count and shared count; the lock requires rows > 0, every served pair present in the single reviewed alias row, the declared `model:` equal to that row's `declared`, `distinct === true`, and `shared_correlation_ids === 0` (`forgejo/landing.ts:6167-6255`). The GO leg is anchored and full-40-hex only, author-gated, and refuses an empty comment read as well as an absent update time or `updated_at != created_at` (`forgejo/landing.ts:4750-4830`). Absent seatLookup/message/`Lab-Run:` trailer all refuse in the lock's own single line. 4. Tests — ran `vitest run forgejo/tests` on this head: 870 tests pass across the 36 files that load; the 38 new pure-lock tests pass (23 seat-verification, 15 go-record). The 17 files that fail to *load* do so only because `@mariozechner/pi-ai` is not installed in this seat sandbox (module resolution, not a code fault), so I read `landing-gated-caller.test.ts` rather than executing it: it asserts exactly two GET `/v1/seat-lookup` calls, sid ∈ {A,B}, `other` named, span ≤ 24 h, zero merge POSTs on 502 and on a malformed URL, and one merge only on the green path — non-vacuous in both directions. Guard context as given: NEW FAILURES 0 (identical FAIL sets on live and head). Notes, none blocking: (a) the endpoint's `first`/`last` are typed in `SeatLookupAnswer` but not carried into `SeatFact`, so the census shows rows and pairs but not the evidence window; (b) `forgejo/verdict.ts:757` hardcodes `seatLookup: {status:"unreadable"}`, so a gated-repo preview always refuses on the seat leg — I confirmed posting is gated on `report.mayPost`, not `lock.ok` (`forgejo/verdict-cli.ts:1019-1026,1251-1257`), so seats can still post verdicts on gated repos; (c) the window starts 30 min before the earliest evidence, so a seat whose calls all predate that would read 0 rows and refuse — fail-closed, matching the spec's stated padding. Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1301Z
ld-reviewer commented 2026-11-11 13:07:03 +00:00
Collaborator
Copy link

LD-VERDICT REQUEST_CHANGES head=d4904c666396954a70da39ce422b993469be1535 session=LR-20261111-017-seatB-20261111T1301Z

Pin: the PR is OPEN, head is exactly d4904c6663, base 0c5ef847 (the pin is on Homelab/pi-extensions #125 on branch lr-20261111-017-census; the merged Homelab/infra-core #125 is an unrelated k3s PR, and its branch for this epic is a different, already-landed change — this review is of the pi-extensions head).

WHAT I CHECKED (diff, tests, guard — read only)

  • Diff vs origin/main: forgejo/landing.ts (GO leg goRecordLeg, seat planner seatVerificationPlan, legs 2b and 5 in evaluateLandingLock), caller wiring in index.ts (10 s abort, any failure ⇒ one unreadable, all-or-nothing over the plan), api.ts getHeadCommitFacts, preview wiring in verdict.ts/verdict-cli.ts, and the accommodated pre-existing suites (landing-corpus, landing-blockers, landing-hold-identity, verdict-694-replay — replayed under a non-gated repo name with their assertions intact, not weakened).
  • Tests on this head: vitest run → 53 files, 1255/1256 pass. The one red (tests/merge-identity.test.ts) is the ambient landing-token path the PR names; it is unrelated to this change.
  • Guard: guard-live.fails and guard-head.fails are the same 16 of 721 (the only difference is a /home/ld/work/tmp path inside one line) ⇒ NEW FAILURES 0, independently confirmed.
  • Contract, as far as this PR owns it: one literal sid per GET, no search=/enumeration/pattern anywhere; window capped at 24 h in the planner (landing.ts:4718); every new refusal is one line; a 502/timeout/malformed body/new URL throw becomes unreadable, never upstream text (index.ts seatAnswerOf/fetchSeatAnswer); missing counts are unreadable, not zero. No write path is added here (the insert-only audit is the endpoint's, piece (e)). The spec's named cases are all covered and non-vacuous: missing rows, unknown served pair, same sid twice, unreadable endpoint, free-form tag, missing SESSION/MODEL, GO absent / other head / other PR / other author / edited / unreadable store, matching GO permitted, ungated repos unchanged.

HOLDS

  1. forgejo/landing.ts:6219 (with MODEL_PAIRS at :4548) — the declared-MODEL check is byte equality against the table's unqualified deepseek-flash, but the seats' own prompt and every recorded real verdict declare the provider-qualified id (model: omniroute/deepseek-flash; recorded live verdicts in forgejo/tests/landing-hold-identity.test.ts:65-75). Reproduced on this head: two otherwise perfect, gateway-verified seats are refused — sid "…" declares model: omniroute/deepseek-flash but the gateway served "deepseek/deepseek-v4-flash" -> "deepseek-v4-flash", whose reviewed pair declares model: deepseek-flash — while the bare deepseek-flash permits. The new fixtures use the bare id (forgejo/tests/landing-seat-verification.test.ts:82), so no test catches it, and the refusal would tell a reviewer to change a line that is already correct. Effect: every real two-seat landing on the two gated repos is refused. Needs normalisation of the declared form (or a reviewed decision to change what seats declare) plus a test with the real shape.
  2. forgejo/landing.ts:6149 — the seat leg demands exactly two approving declared sessions on every gated merge, so a routine infra-core/prod change with one exact-head verdict is refused (… 1 distinct seat session(s) declared … the gate verifies TWO independent seats), which the standing landing rule still permits for a routine change and spec 017's Done-when 1 (the GO) does not require. Either scope the two-seat verification to the dangerous-class/panel case, or land a spec line that says every gated merge needs two verified seats.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1301Z

LD-VERDICT REQUEST_CHANGES head=d4904c666396954a70da39ce422b993469be1535 session=LR-20261111-017-seatB-20261111T1301Z Pin: the PR is OPEN, head is exactly d4904c666396954a70da39ce422b993469be1535, base 0c5ef847 (the pin is on `Homelab/pi-extensions` #125 on branch `lr-20261111-017-census`; the merged `Homelab/infra-core` #125 is an unrelated k3s PR, and its branch for this epic is a different, already-landed change — this review is of the pi-extensions head). WHAT I CHECKED (diff, tests, guard — read only) - Diff vs origin/main: `forgejo/landing.ts` (GO leg `goRecordLeg`, seat planner `seatVerificationPlan`, legs 2b and 5 in `evaluateLandingLock`), caller wiring in `index.ts` (10 s abort, any failure ⇒ one `unreadable`, all-or-nothing over the plan), `api.ts` `getHeadCommitFacts`, preview wiring in `verdict.ts`/`verdict-cli.ts`, and the accommodated pre-existing suites (`landing-corpus`, `landing-blockers`, `landing-hold-identity`, `verdict-694-replay` — replayed under a non-gated repo name with their assertions intact, not weakened). - Tests on this head: `vitest run` → 53 files, 1255/1256 pass. The one red (`tests/merge-identity.test.ts`) is the ambient landing-token path the PR names; it is unrelated to this change. - Guard: `guard-live.fails` and `guard-head.fails` are the same 16 of 721 (the only difference is a `/home/ld/work/tmp` path inside one line) ⇒ NEW FAILURES 0, independently confirmed. - Contract, as far as this PR owns it: one literal sid per GET, no `search=`/enumeration/pattern anywhere; window capped at 24 h in the planner (`landing.ts:4718`); every new refusal is one line; a 502/timeout/malformed body/`new URL` throw becomes `unreadable`, never upstream text (`index.ts` `seatAnswerOf`/`fetchSeatAnswer`); missing counts are `unreadable`, not zero. No write path is added here (the insert-only audit is the endpoint's, piece (e)). The spec's named cases are all covered and non-vacuous: missing rows, unknown served pair, same sid twice, unreadable endpoint, free-form tag, missing SESSION/MODEL, GO absent / other head / other PR / other author / edited / unreadable store, matching GO permitted, ungated repos unchanged. HOLDS 1. `forgejo/landing.ts:6219` (with `MODEL_PAIRS` at `:4548`) — the declared-MODEL check is byte equality against the table's *unqualified* `deepseek-flash`, but the seats' own prompt and every recorded real verdict declare the provider-qualified id (`model: omniroute/deepseek-flash`; recorded live verdicts in `forgejo/tests/landing-hold-identity.test.ts:65-75`). Reproduced on this head: two otherwise perfect, gateway-verified seats are refused — `sid "…" declares model: omniroute/deepseek-flash but the gateway served "deepseek/deepseek-v4-flash" -> "deepseek-v4-flash", whose reviewed pair declares model: deepseek-flash` — while the bare `deepseek-flash` permits. The new fixtures use the bare id (`forgejo/tests/landing-seat-verification.test.ts:82`), so no test catches it, and the refusal would tell a reviewer to change a line that is already correct. Effect: every real two-seat landing on the two gated repos is refused. Needs normalisation of the declared form (or a reviewed decision to change what seats declare) plus a test with the real shape. 2. `forgejo/landing.ts:6149` — the seat leg demands exactly two approving declared sessions on *every* gated merge, so a routine infra-core/prod change with one exact-head verdict is refused (`… 1 distinct seat session(s) declared … the gate verifies TWO independent seats`), which the standing landing rule still permits for a routine change and spec 017's Done-when 1 (the GO) does not require. Either scope the two-seat verification to the dangerous-class/panel case, or land a spec line that says every gated merge needs two verified seats. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1301Z
pi-landing force-pushed lr-20261111-017-census from d4904c6663
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 49s
Details
CI / pi-otel-telemetry (pull_request) Successful in 53s
Details
CI / skill-args (pull_request) Successful in 56s
Details
CI / todo (pull_request) Successful in 1m0s
Details
CI / plane (pull_request) Successful in 1m25s
Details
CI / mcp-bridge (pull_request) Successful in 23s
Details
CI / ask-user-question (pull_request) Successful in 50s
Details
CI / forgejo (pull_request) Successful in 2m2s
Details
CI / refocus (pull_request) Successful in 57s
Details
tests / python (pull_request) Successful in 6s
Details
CI / nats-intercom (pull_request) Successful in 25s
Details
CI / pi-friction (pull_request) Successful in 37s
Details
CI / redact (pull_request) Successful in 41s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m41s
Details
to d1da1dfcb1
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 58s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m5s
Details
CI / plane (pull_request) Successful in 1m9s
Details
CI / mcp-bridge (pull_request) Successful in 25s
Details
tests / python (pull_request) Successful in 5s
Details
CI / skill-args (pull_request) Successful in 46s
Details
CI / nats-intercom (pull_request) Successful in 33s
Details
CI / todo (pull_request) Successful in 50s
Details
CI / ask-user-question (pull_request) Successful in 48s
Details
CI / refocus (pull_request) Successful in 56s
Details
CI / pi-friction (pull_request) Successful in 34s
Details
CI / forgejo (pull_request) Successful in 2m11s
Details
CI / redact (pull_request) Successful in 38s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m54s
Details
2026-11-11 13:32:59 +00:00 Compare
ld-reviewer commented 2026-11-11 13:39:28 +00:00
Collaborator
Copy link

LD-VERDICT REQUEST_CHANGES head=d1da1dfcb12acb3b819e37897217cb8a613c95f3 session=LR-20261111-017-seatB-20261111T1335Z

Checked: pinned head is the branch tip (d1da1dfcb12acb3b819e37897217cb8a613c95f3, one commit on base 0c5ef847), diff vs origin/main read in full (forgejo/landing.ts, forgejo/index.ts, forgejo/api.ts, fixtures), and the tests were run in my own read-only clone: 53 files / 1262 tests, 1261 pass; the only failure is tests/merge-identity.test.ts (ambient landing token), which fails on the untouched base too — pre-existing. No new failures.

What holds up: the GO leg is strict and fail-closed (anchored grammar, ld-director-go only, full 40-hex, short sha and edited/unreadable rows refused, newest unedited match wins, non-gated repos untouched); the caller wiring is real (updatedAt populated for comments and reviews, head message and date from one getHeadCommitFacts read, sid-lookup only for the gated repos and all-or-nothing so a failure lands as unreadable, malformed URL swallowed into the lock's own one line); the seat leg refuses zero rows, absent answers, unlisted served pairs, MODEL lines the served pair contradicts, free-form or other-run tags, missing SESSION/MODEL, unreadable endpoint; and the named cases are non-vacuous — tests/landing-go-record.test.ts (15 tests) and tests/landing-seat-verification.test.ts (23 tests) assert the refusal text, plus tests/landing-gated-caller.test.ts for the wiring.

Holds (numbered, all in the seat leg):

  1. forgejo/landing.ts:6292 (with the plan at forgejo/landing.ts:4762) — independence is fail-OPEN for any panel that is not exactly two. hasCounterpart = seats.length === 2 gates BOTH the distinct check and the shared-correlation-id check, and seatVerificationPlan sends other only when there are two seats, so with three declared seats neither question is asked or answered, and two "verified" sids are enough to permit. Measured in this clone against the branch code: a gated, routine PR with three declared seats where EVERY endpoint answer reports distinct: false, shared_correlation_ids: 4 returns ok: true, with all three seats recorded verified: true. That is one request chain passing as independent seats — the exact fact Amendment 1 exists to establish. Independence must be established pairwise for every verified seat (or a declared shape that is not one or two seats must be refused rather than skipped). No test covers a three-seat panel.

  2. forgejo/landing.ts:6197 (pinned as correct by forgejo/tests/landing-seat-verification.test.ts:317) — requiredSeats is forced to 2 for EVERY GO-gated merge, routine changes included, where this piece requires two distinct verified seats for a dangerous-class change and one verified seat for a routine gated merge. The line also cannot fall through: it sits inside if (isGoGatedRepo(...)), so cls.dangerous ? 2 : 1 there is unreachable code whose only effect is to word the refusal two ways for one threshold. Measured: a gated docs/notes.md change (record.dangerous === false) with a valid GO and one verified seat is refused with "TWO independent seats on a GO-gated repo, a routine landing included". The justification for raising it lives only in a code comment quoting a correction, and it is counted in the suite as a requirement rather than as an open question — so the routine permit path on the two gated repos has no passing test at all.

  3. Minor, forgejo/landing.ts:4793 — a PR with an EMPTY comment store is refused as "the PR's comments could not be read", so the one-line refusal names the wrong missing fact (nothing was unreadable; there is simply no record yet). Fail-closed, so it changes no permit, but the refusal exists to tell a person what to do next.

Not a hold, worth a line: the corpus/replay suites now route the gated repos around the gate (repoRef: "Homelab/pi-extensions", synthetic GO rows, two lock.ok expectations flipped true→false in tests/verdict-694-replay.test.ts). Each is documented and the rules under test are repo-independent, but the end-to-end permit on a gated repo now rests only on the new synthetic fixtures.

For the fleet: NEW FAILURES 0 confirmed in this clone; the guard script belonging to infra-core was not run here (it does not run for this repo).

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1335Z

LD-VERDICT REQUEST_CHANGES head=d1da1dfcb12acb3b819e37897217cb8a613c95f3 session=LR-20261111-017-seatB-20261111T1335Z Checked: pinned head is the branch tip (`d1da1dfcb12acb3b819e37897217cb8a613c95f3`, one commit on base `0c5ef847`), diff vs `origin/main` read in full (`forgejo/landing.ts`, `forgejo/index.ts`, `forgejo/api.ts`, fixtures), and the tests were run in my own read-only clone: **53 files / 1262 tests, 1261 pass**; the only failure is `tests/merge-identity.test.ts` (ambient landing token), which fails on the untouched base too — pre-existing. No new failures. What holds up: the GO leg is strict and fail-closed (anchored grammar, `ld-director-go` only, full 40-hex, short sha and edited/unreadable rows refused, newest unedited match wins, non-gated repos untouched); the caller wiring is real (`updatedAt` populated for comments and reviews, head message and date from one `getHeadCommitFacts` read, sid-lookup only for the gated repos and all-or-nothing so a failure lands as `unreadable`, malformed URL swallowed into the lock's own one line); the seat leg refuses zero rows, absent answers, unlisted served pairs, MODEL lines the served pair contradicts, free-form or other-run tags, missing SESSION/MODEL, unreadable endpoint; and the named cases are non-vacuous — `tests/landing-go-record.test.ts` (15 tests) and `tests/landing-seat-verification.test.ts` (23 tests) assert the refusal text, plus `tests/landing-gated-caller.test.ts` for the wiring. Holds (numbered, all in the seat leg): 1. `forgejo/landing.ts:6292` (with the plan at `forgejo/landing.ts:4762`) — independence is fail-OPEN for any panel that is not exactly two. `hasCounterpart = seats.length === 2` gates BOTH the `distinct` check and the shared-correlation-id check, and `seatVerificationPlan` sends `other` only when there are two seats, so with three declared seats neither question is asked or answered, and two "verified" sids are enough to permit. Measured in this clone against the branch code: a gated, routine PR with three declared seats where EVERY endpoint answer reports `distinct: false, shared_correlation_ids: 4` returns `ok: true`, with all three seats recorded `verified: true`. That is one request chain passing as independent seats — the exact fact Amendment 1 exists to establish. Independence must be established pairwise for every verified seat (or a declared shape that is not one or two seats must be refused rather than skipped). No test covers a three-seat panel. 2. `forgejo/landing.ts:6197` (pinned as correct by `forgejo/tests/landing-seat-verification.test.ts:317`) — `requiredSeats` is forced to 2 for EVERY GO-gated merge, routine changes included, where this piece requires two distinct verified seats for a dangerous-class change and one verified seat for a routine gated merge. The line also cannot fall through: it sits inside `if (isGoGatedRepo(...))`, so `cls.dangerous ? 2 : 1` there is unreachable code whose only effect is to word the refusal two ways for one threshold. Measured: a gated `docs/notes.md` change (`record.dangerous === false`) with a valid GO and one verified seat is refused with "TWO independent seats on a GO-gated repo, a routine landing included". The justification for raising it lives only in a code comment quoting a correction, and it is counted in the suite as a requirement rather than as an open question — so the routine permit path on the two gated repos has no passing test at all. 3. Minor, `forgejo/landing.ts:4793` — a PR with an EMPTY comment store is refused as "the PR's comments could not be read", so the one-line refusal names the wrong missing fact (nothing was unreadable; there is simply no record yet). Fail-closed, so it changes no permit, but the refusal exists to tell a person what to do next. Not a hold, worth a line: the corpus/replay suites now route the gated repos around the gate (`repoRef: "Homelab/pi-extensions"`, synthetic GO rows, two `lock.ok` expectations flipped true→false in `tests/verdict-694-replay.test.ts`). Each is documented and the rules under test are repo-independent, but the end-to-end permit on a gated repo now rests only on the new synthetic fixtures. For the fleet: NEW FAILURES 0 confirmed in this clone; the guard script belonging to infra-core was not run here (it does not run for this repo). model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1335Z
ld-reviewer commented 2026-11-11 13:39:51 +00:00
Collaborator
Copy link

LD-VERDICT REQUEST_CHANGES head=d1da1dfcb12acb3b819e37897217cb8a613c95f3 session=LR-20261111-017-seatA-20261111T1335Z

Reviewed the diff of the pinned head against origin/main: forgejo/landing.ts, api.ts, index.ts, verdict.ts, verdict-cli.ts and the fixture tests. Tests run: tests/landing-seat-verification.test.ts (29) and tests/landing-go-record.test.ts (15) both pass — 44/44; the full suite here gives 876 passing tests, with 17 files unable to collect for a missing @mariozechner/pi-ai dependency — every index.ts-importing file, identical on the untouched base, so tests/landing-gated-caller.test.ts is unverified in this sandbox (not counted against the change).

The GO leg is sound: anchored strict grammar, ld-director-go only, full 40-hex head (short sha refused), other repo / PR / head refused, edited record refused (updated_at != created_at), unreadable comments and unreadable edit state refused, newest unedited match recorded and carried on the result.

Holds:

  1. forgejo/landing.ts:6292 (with :4762) — the independence checks are applied only when exactly two seats are declared: hasCounterpart = seats.length === 2 gates both distinct !== true and shared_correlation_ids > 0, and seatVerificationPlan:4762 sends other only in that same shape. With three declared tags that all satisfy the tag grammar (…-seatA-, …-seatA-, …-seatB-) and every answer reporting distinct=false, shared_correlation_ids=5, the gated lock PERMITS (reproduced against this head). Adding a third verdict therefore removes the "shared caller correlation ids refused" and "distinct sessions" guarantees entirely. Independence has to be checked for every pair / every seat, not only the two-seat shape, and the plan should ask about it whenever more than one seat exists.
  2. forgejo/landing.ts:6197 — requiredSeats = isGoGatedRepo(...) ? 2 : ... makes two verified seats mandatory on every GO-gated landing, so a ROUTINE gated merge that verified one seat is refused (:6315-:6322). The required rule is two distinct verified seats for dangerous class and ONE verified seat for a routine gated landing. tests/landing-seat-verification.test.ts:317 pins the stricter behaviour, so the test has to move with the fix.
  3. forgejo/landing.ts:6131 — the entire seat leg sits inside if (isGoGatedRepo(input.repoRef)), so a dangerous-class change on any other repo lands with two approvals whose declared sessions are never verified: reproduced with session: sess-seat-1 / session: sess-seat-2, no lookup and no Lab-Run trailer, changed path deploy/prod.yaml → permit. "Two distinct verified seats for dangerous class" is unenforced outside the two gated repos.
    Note (not a hold): declaredModelMatches (:4599) matches on the segment after the last / from any provider, so evilprovider/deepseek-flash satisfies the reviewed bare id; the served pair still decides, so it is a loose reading rather than a bypass.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1335Z

LD-VERDICT REQUEST_CHANGES head=d1da1dfcb12acb3b819e37897217cb8a613c95f3 session=LR-20261111-017-seatA-20261111T1335Z Reviewed the diff of the pinned head against origin/main: forgejo/landing.ts, api.ts, index.ts, verdict.ts, verdict-cli.ts and the fixture tests. Tests run: tests/landing-seat-verification.test.ts (29) and tests/landing-go-record.test.ts (15) both pass — 44/44; the full suite here gives 876 passing tests, with 17 files unable to collect for a missing @mariozechner/pi-ai dependency — every index.ts-importing file, identical on the untouched base, so tests/landing-gated-caller.test.ts is unverified in this sandbox (not counted against the change). The GO leg is sound: anchored strict grammar, ld-director-go only, full 40-hex head (short sha refused), other repo / PR / head refused, edited record refused (updated_at != created_at), unreadable comments and unreadable edit state refused, newest unedited match recorded and carried on the result. Holds: 1. forgejo/landing.ts:6292 (with :4762) — the independence checks are applied only when exactly two seats are declared: `hasCounterpart = seats.length === 2` gates both `distinct !== true` and `shared_correlation_ids > 0`, and seatVerificationPlan:4762 sends `other` only in that same shape. With three declared tags that all satisfy the tag grammar (…-seatA-<stamp1>, …-seatA-<stamp2>, …-seatB-<stamp3>) and every answer reporting distinct=false, shared_correlation_ids=5, the gated lock PERMITS (reproduced against this head). Adding a third verdict therefore removes the "shared caller correlation ids refused" and "distinct sessions" guarantees entirely. Independence has to be checked for every pair / every seat, not only the two-seat shape, and the plan should ask about it whenever more than one seat exists. 2. forgejo/landing.ts:6197 — `requiredSeats = isGoGatedRepo(...) ? 2 : ...` makes two verified seats mandatory on every GO-gated landing, so a ROUTINE gated merge that verified one seat is refused (:6315-:6322). The required rule is two distinct verified seats for dangerous class and ONE verified seat for a routine gated landing. tests/landing-seat-verification.test.ts:317 pins the stricter behaviour, so the test has to move with the fix. 3. forgejo/landing.ts:6131 — the entire seat leg sits inside `if (isGoGatedRepo(input.repoRef))`, so a dangerous-class change on any other repo lands with two approvals whose declared sessions are never verified: reproduced with `session: sess-seat-1` / `session: sess-seat-2`, no lookup and no Lab-Run trailer, changed path deploy/prod.yaml → permit. "Two distinct verified seats for dangerous class" is unenforced outside the two gated repos. Note (not a hold): declaredModelMatches (:4599) matches on the segment after the last `/` from any provider, so `evilprovider/deepseek-flash` satisfies the reviewed bare id; the served pair still decides, so it is a loose reading rather than a bypass. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1335Z
pi-landing force-pushed lr-20261111-017-census from d1da1dfcb1
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 58s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m5s
Details
CI / plane (pull_request) Successful in 1m9s
Details
CI / mcp-bridge (pull_request) Successful in 25s
Details
tests / python (pull_request) Successful in 5s
Details
CI / skill-args (pull_request) Successful in 46s
Details
CI / nats-intercom (pull_request) Successful in 33s
Details
CI / todo (pull_request) Successful in 50s
Details
CI / ask-user-question (pull_request) Successful in 48s
Details
CI / refocus (pull_request) Successful in 56s
Details
CI / pi-friction (pull_request) Successful in 34s
Details
CI / forgejo (pull_request) Successful in 2m11s
Details
CI / redact (pull_request) Successful in 38s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m54s
Details
to 3ade6f231a
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / skill-args (pull_request) Successful in 54s
Details
CI / ask-user-question (pull_request) Successful in 56s
Details
CI / forgejo-ci (pull_request) Successful in 58s
Details
CI / todo (pull_request) Successful in 59s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m3s
Details
tests / python (pull_request) Successful in 7s
Details
CI / refocus (pull_request) Successful in 1m6s
Details
CI / mcp-bridge (pull_request) Successful in 20s
Details
CI / nats-intercom (pull_request) Successful in 26s
Details
CI / redact (pull_request) Successful in 31s
Details
CI / plane (pull_request) Successful in 1m28s
Details
CI / pi-friction (pull_request) Successful in 38s
Details
CI / forgejo (pull_request) Successful in 2m5s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m28s
Details
2026-11-11 13:48:48 +00:00 Compare
ld-reviewer commented 2026-11-11 13:51:25 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa session=LR-20261111-017-seatB-20261111T1348Z

Checked the diff of origin/main..3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa (14 files; the PR's current head is that exact 40-hex sha, no drift) by reading forgejo/landing.ts, forgejo/index.ts and forgejo/api.ts and the three fixture suites, then running the suite and the typecheck myself.

GO leg (landing.ts:4411-4850, invoked at landing.ts:5672 after head identity and before the ledger). Gated set is a source constant, owner/name exact and case-insensitive with no suffix match. The grammar is anchored and full-width (GO_RECORD_RE, 40 hex, short sha never accepted, prose around the record rejected, upper hex normalised), the author is fixed to the GO identity and checked before parsing, matching requires repo+PR+current head, updatedAt !== at refuses as EDITED, and an undefined update time refuses on its own sentence rather than assuming "probably not edited". Newest unedited match wins deterministically. Non-gated repos return before touching items.

Seat leg (landing.ts:6152-6470). Run id from the head commit's LAST Lab-Run trailer (missing message / missing trailer each refuse in one line); tag anchored and case-sensitive via seatSIDPattern; zero settled rows, absent answer for a sid, an unobserved served pair, and a MODEL line that disagrees with the SERVED pair all refuse; the served pair is trusted over the line, with the declared id matched bare or provider-qualified through declaredModelMatches (no prefix/family/case folding). Independence is read in a separate loop over EVERY unordered pair (N*(N-1)/2), each pair's facts taken from the endpoint's own answers with the worst fact winning, demoting both members of a failed pair on the record; a pair with no answer refuses as unproven rather than "nothing shared". The planner (seatVerificationPlan) emits one per-seat request plus one request per unordered pair, so a 3+ panel cannot skip comparison. On a GO-gated repo requiredSeats is 2 for routine AND dangerous heads, enforced on the verified panel; unreadable/absent lookup refuses in the lock's own words. Caller wiring (index.ts:301-415, 553-590, 597-613, 1160-1173) keeps all decisions in the pure lock, reads the date and message in one commit pass, computes the plan from the same ledger the lock builds with the same resolved policy, and turns every failure (non-200, timeout, malformed URL, unparseable field) into unreadable instead of throwing.

Tests: 53 files / 1269 tests, 1268 pass. The single failure is tests/merge-identity.test.ts ("merges with the dedicated landing credential"), which is the known pre-existing ambient-landing-token failure in this sandbox, not caused by this change. tsc --noEmit clean. The named cases are present and non-vacuous: no record / other head / other PR / other repo / other author / edited / unreadable edit state / no comments / short sha; rows=0, missing answer, unobserved pair, MODEL-line mismatch, same sid twice, one verified seat on a routine and on a dangerous gated head, shared correlation ids, non-distinct sessions, unreadable endpoint, free-form tag, tag of another run, no SESSION, no MODEL, no message, no trailer; plus a 3-seat panel with all pairs dirty, one dirty pair, a non-distinct pair, a missing pair answer, and an all-clean permit. The caller suite drives the real extension over a mocked fetch and asserts one per-seat request per sid plus exactly one pair request, with a bounded window.

Holds: none.

Non-blocking observations (not holds, no change requested for this landing): (1) landing.ts:6247 the seats.length === 0 sentence speaks of "at least one trusted exact-head approval", while the gated requirement is two — the count check at 5g refuses correctly, so this is wording only. (2) landing.ts:6241 const requiredSeats = isGoGatedRepo(...) ? 2 : ... sits inside a block already gated on isGoGatedRepo, so the first two arms are redundant; harmless and documented. (3) landing.ts:4856 an empty comment read and "nobody commented" share one refusal, so the cause shown for a gated PR with no comments at all reads as an unreadable read; the refusal direction is still correct.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1348Z

LD-VERDICT APPROVED head=3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa session=LR-20261111-017-seatB-20261111T1348Z Checked the diff of origin/main..3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa (14 files; the PR's current head is that exact 40-hex sha, no drift) by reading forgejo/landing.ts, forgejo/index.ts and forgejo/api.ts and the three fixture suites, then running the suite and the typecheck myself. GO leg (landing.ts:4411-4850, invoked at landing.ts:5672 after head identity and before the ledger). Gated set is a source constant, owner/name exact and case-insensitive with no suffix match. The grammar is anchored and full-width (`GO_RECORD_RE`, 40 hex, short sha never accepted, prose around the record rejected, upper hex normalised), the author is fixed to the GO identity and checked before parsing, matching requires repo+PR+current head, `updatedAt !== at` refuses as EDITED, and an undefined update time refuses on its own sentence rather than assuming "probably not edited". Newest unedited match wins deterministically. Non-gated repos return before touching items. Seat leg (landing.ts:6152-6470). Run id from the head commit's LAST Lab-Run trailer (missing message / missing trailer each refuse in one line); tag anchored and case-sensitive via seatSIDPattern; zero settled rows, absent answer for a sid, an unobserved served pair, and a MODEL line that disagrees with the SERVED pair all refuse; the served pair is trusted over the line, with the declared id matched bare or provider-qualified through declaredModelMatches (no prefix/family/case folding). Independence is read in a separate loop over EVERY unordered pair (N*(N-1)/2), each pair's facts taken from the endpoint's own answers with the worst fact winning, demoting both members of a failed pair on the record; a pair with no answer refuses as unproven rather than "nothing shared". The planner (seatVerificationPlan) emits one per-seat request plus one request per unordered pair, so a 3+ panel cannot skip comparison. On a GO-gated repo requiredSeats is 2 for routine AND dangerous heads, enforced on the verified panel; unreadable/absent lookup refuses in the lock's own words. Caller wiring (index.ts:301-415, 553-590, 597-613, 1160-1173) keeps all decisions in the pure lock, reads the date and message in one commit pass, computes the plan from the same ledger the lock builds with the same resolved policy, and turns every failure (non-200, timeout, malformed URL, unparseable field) into unreadable instead of throwing. Tests: 53 files / 1269 tests, 1268 pass. The single failure is tests/merge-identity.test.ts ("merges with the dedicated landing credential"), which is the known pre-existing ambient-landing-token failure in this sandbox, not caused by this change. `tsc --noEmit` clean. The named cases are present and non-vacuous: no record / other head / other PR / other repo / other author / edited / unreadable edit state / no comments / short sha; rows=0, missing answer, unobserved pair, MODEL-line mismatch, same sid twice, one verified seat on a routine and on a dangerous gated head, shared correlation ids, non-distinct sessions, unreadable endpoint, free-form tag, tag of another run, no SESSION, no MODEL, no message, no trailer; plus a 3-seat panel with all pairs dirty, one dirty pair, a non-distinct pair, a missing pair answer, and an all-clean permit. The caller suite drives the real extension over a mocked fetch and asserts one per-seat request per sid plus exactly one pair request, with a bounded window. Holds: none. Non-blocking observations (not holds, no change requested for this landing): (1) landing.ts:6247 the `seats.length === 0` sentence speaks of "at least one trusted exact-head approval", while the gated requirement is two — the count check at 5g refuses correctly, so this is wording only. (2) landing.ts:6241 `const requiredSeats = isGoGatedRepo(...) ? 2 : ...` sits inside a block already gated on isGoGatedRepo, so the first two arms are redundant; harmless and documented. (3) landing.ts:4856 an empty comment read and "nobody commented" share one refusal, so the cause shown for a gated PR with no comments at all reads as an unreadable read; the refusal direction is still correct. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1348Z
ld-reviewer commented 2026-11-11 13:53:15 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa session=LR-20261111-017-seatA-20261111T1348Z

Read the diff at the exact head 3ade6f231a (PR head confirms the pin) against origin/main 0c5ef847: forgejo/landing.ts (+931), forgejo/index.ts + api.ts wiring, and the fixture tests.

GO leg (landing.ts:4395-4920): LD-GO grammar is anchored and strict (^…repo=\S+ pr=\d+ head=[0-9a-fA-F]{40}$), short shas never parse, prose around the record never parses, the whole comment must be the record; author must be the ld-director-go identity (checked before the text); repo/PR/full-40-hex head are all compared, head lowercased on both sides; a record for another head is refused and the refusal names it. Edited comments refused (updatedAt !== at), an unreadable edit state refused before matching, and an empty comment read refused rather than read as "no go". The leg runs before verdict parsing, so a missing go is the first thing a reader sees.

Seat leg (landing.ts:6180-6470): every counted trusted exact-head approval's declared sid is verified — rows > 0, each observed (requested, served) pair must exist in the reviewed alias table, and the MODEL line is measured against the table's declared id with the SERVED pair as the evidence (bare or provider-qualified spelling only). Missing MODEL or SESSION is refused on its own sentence; the tag must be the literal ^<last Lab-Run trailer>-seat(A|B)-<stamp>$ for THIS head's commit message. Endpoint unreadable/absent is refused in the lock's words. On the two gated repos requiredSeats is 2 for every landing, routine included (dangerous class only changes the wording), and 5g counts DISTINCT verified sids, so one sid twice is one seat.

I verified the pairwise independence myself rather than trusting the fixture names: for a 3-seat panel the planner emits 3 per-seat + 3 unordered-pair requests and the lock refuses (a) a pair sharing a caller correlation id, (b) a panel with one pair never asked about, (c) both directions disagreeing (worst fact wins: shared=0 vs shared=3 refuses), (d) distinct=false in either direction, and permits only an all-pairs-clean panel; a 4-seat all-shared panel is refused at its first pair. Pair refusals demote both member seats to verified:false on the record.

Tests: at this head vitest run = 53 files, 1268 passed, 1 failed — tests/merge-identity.test.ts ("dedicated landing credential"), which I confirmed fails identically on an untouched origin/main worktree (1212 passed, same single failure), so NEW FAILURES = 0, matching the PR's claim. tsc --noEmit clean. The new files are non-vacuous: landing-go-record (17 cases: no record / other head / other PR / other repo / other author / edited / no update time / no comments / short sha / newest-unedited-wins / non-gated unchanged), landing-seat-verification (no rows, unknown served pair, MODEL-vs-served, same sid twice, 1 seat routine and dangerous, shared correlation id, not distinct, unreadable and never-asked endpoint, free-form and other-run tags, missing session/model, missing message and missing trailer, non-gated behaviour), landing-gated-caller (merge tool wiring: no-go refusal with zero merge POSTs, 502 refusal, malformed lookup URL refused as one line with no throw, missing trailer, green merge asserting all 3 endpoint calls incl. the pair).

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1348Z

LD-VERDICT APPROVED head=3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa session=LR-20261111-017-seatA-20261111T1348Z Read the diff at the exact head 3ade6f231a9f9dd4bb9b639e73eab96fef5de3fa (PR head confirms the pin) against origin/main 0c5ef847: forgejo/landing.ts (+931), forgejo/index.ts + api.ts wiring, and the fixture tests. GO leg (landing.ts:4395-4920): `LD-GO` grammar is anchored and strict (`^…repo=\S+ pr=\d+ head=[0-9a-fA-F]{40}$`), short shas never parse, prose around the record never parses, the whole comment must be the record; author must be the `ld-director-go` identity (checked before the text); repo/PR/full-40-hex head are all compared, head lowercased on both sides; a record for another head is refused and the refusal names it. Edited comments refused (`updatedAt !== at`), an unreadable edit state refused before matching, and an empty comment read refused rather than read as "no go". The leg runs before verdict parsing, so a missing go is the first thing a reader sees. Seat leg (landing.ts:6180-6470): every counted trusted exact-head approval's declared sid is verified — rows > 0, each observed (requested, served) pair must exist in the reviewed alias table, and the MODEL line is measured against the table's declared id with the SERVED pair as the evidence (bare or provider-qualified spelling only). Missing MODEL or SESSION is refused on its own sentence; the tag must be the literal `^<last Lab-Run trailer>-seat(A|B)-<stamp>$` for THIS head's commit message. Endpoint unreadable/absent is refused in the lock's words. On the two gated repos requiredSeats is 2 for every landing, routine included (dangerous class only changes the wording), and 5g counts DISTINCT verified sids, so one sid twice is one seat. I verified the pairwise independence myself rather than trusting the fixture names: for a 3-seat panel the planner emits 3 per-seat + 3 unordered-pair requests and the lock refuses (a) a pair sharing a caller correlation id, (b) a panel with one pair never asked about, (c) both directions disagreeing (worst fact wins: shared=0 vs shared=3 refuses), (d) `distinct=false` in either direction, and permits only an all-pairs-clean panel; a 4-seat all-shared panel is refused at its first pair. Pair refusals demote both member seats to `verified:false` on the record. Tests: at this head `vitest run` = 53 files, 1268 passed, 1 failed — `tests/merge-identity.test.ts` ("dedicated landing credential"), which I confirmed fails identically on an untouched origin/main worktree (1212 passed, same single failure), so NEW FAILURES = 0, matching the PR's claim. `tsc --noEmit` clean. The new files are non-vacuous: landing-go-record (17 cases: no record / other head / other PR / other repo / other author / edited / no update time / no comments / short sha / newest-unedited-wins / non-gated unchanged), landing-seat-verification (no rows, unknown served pair, MODEL-vs-served, same sid twice, 1 seat routine and dangerous, shared correlation id, not distinct, unreadable and never-asked endpoint, free-form and other-run tags, missing session/model, missing message and missing trailer, non-gated behaviour), landing-gated-caller (merge tool wiring: no-go refusal with zero merge POSTs, 502 refusal, malformed lookup URL refused as one line with no throw, missing trailer, green merge asserting all 3 endpoint calls incl. the pair). Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1348Z
pi-landing force-pushed lr-20261111-017-census from 3ade6f231a
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / skill-args (pull_request) Successful in 54s
Details
CI / ask-user-question (pull_request) Successful in 56s
Details
CI / forgejo-ci (pull_request) Successful in 58s
Details
CI / todo (pull_request) Successful in 59s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m3s
Details
tests / python (pull_request) Successful in 7s
Details
CI / refocus (pull_request) Successful in 1m6s
Details
CI / mcp-bridge (pull_request) Successful in 20s
Details
CI / nats-intercom (pull_request) Successful in 26s
Details
CI / redact (pull_request) Successful in 31s
Details
CI / plane (pull_request) Successful in 1m28s
Details
CI / pi-friction (pull_request) Successful in 38s
Details
CI / forgejo (pull_request) Successful in 2m5s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m28s
Details
to cd85805e5a
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / forgejo-ci (pull_request) Successful in 49s
Details
CI / pi-otel-telemetry (pull_request) Successful in 54s
Details
CI / mcp-bridge (pull_request) Successful in 20s
Details
tests / python (pull_request) Successful in 5s
Details
CI / plane (pull_request) Successful in 1m32s
Details
CI / ask-user-question (pull_request) Successful in 45s
Details
CI / skill-args (pull_request) Successful in 46s
Details
CI / nats-intercom (pull_request) Successful in 20s
Details
CI / redact (pull_request) Successful in 40s
Details
CI / pi-friction (pull_request) Successful in 43s
Details
CI / forgejo (pull_request) Successful in 2m7s
Details
CI / todo (pull_request) Successful in 1m19s
Details
CI / refocus (pull_request) Successful in 1m19s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m38s
Details
2026-11-11 14:30:04 +00:00 Compare
pi-landing commented 2026-11-11 14:30:19 +00:00
Author
Collaborator
Copy link

infra review (homelab-88), file review-88.md — the five blocking findings, quoted verbatim. B1–B4 are fixed at this head (each with a test that fails on the old code); B5 is fixed in its own PR #126.

B1. R6: "comment created_at inside its own sid's stream" is never checked. […] Fix: refuse unless Date.parse(first) ≤ verdict.at ≤ Date.parse(last) + small slack, for each seat's own answer. Refuse when first/last is missing while rows > 0. Add a test where the verdict is outside the stream (refused) and one where it is inside (allowed).

B2. R6 / Research Note: caller-supplied correlation ids are counted but never refused. […] Fix: refuse when correlation_ids > 0 on any seat. Add a test. Also confirm in infra-core (e) that this field really counts caller-supplied ids only.

B3. R8 / NEVER-TRY "no bypass flag": the verifier's address can be set from the environment. […] Fix: hard-code the in-cluster URL. If tests need another address, inject it through a non-env seam […]. At minimum, make an override a refusal on gated repos.

B4. R1: gate membership comes from the caller's string, not the server's identity for the repo. […] Fix: decide gating from pr.base.repo.full_name returned by getPR. Refuse when it differs from the parsed ref, and refuse when it is absent on a merge.

B5. R2 "head == PR's CURRENT head": time-of-check/time-of-use race. […] Fix: send head_commit_id: evidence.headSha (Forgejo returns 409 if the head moved). Add a caller test asserting the merge POST body carries the exact head. This race already existed for every leg; this spec is what makes it a hard requirement.

(Full word-for-word text: review-88.md. Non-blocking findings N1–N7 noted, not addressed in this PR.) Record note only, not a verdict — the pane seats own the verdicts on this head.

infra review (homelab-88), file review-88.md — the five blocking findings, quoted verbatim. B1–B4 are fixed at this head (each with a test that fails on the old code); B5 is fixed in its own PR #126. > **B1. R6: "comment created_at inside its own sid's stream" is never checked.** […] **Fix:** refuse unless `Date.parse(first) ≤ verdict.at ≤ Date.parse(last) + small slack`, for each seat's own answer. Refuse when `first`/`last` is missing while `rows > 0`. Add a test where the verdict is outside the stream (refused) and one where it is inside (allowed). > **B2. R6 / Research Note: caller-supplied correlation ids are counted but never refused.** […] **Fix:** refuse when `correlation_ids > 0` on any seat. Add a test. Also confirm in infra-core (e) that this field really counts caller-supplied ids only. > **B3. R8 / NEVER-TRY "no bypass flag": the verifier's address can be set from the environment.** […] **Fix:** hard-code the in-cluster URL. If tests need another address, inject it through a non-env seam […]. At minimum, make an override a refusal on gated repos. > **B4. R1: gate membership comes from the caller's string, not the server's identity for the repo.** […] **Fix:** decide gating from `pr.base.repo.full_name` returned by `getPR`. Refuse when it differs from the parsed ref, and refuse when it is absent on a merge. > **B5. R2 "head == PR's CURRENT head": time-of-check/time-of-use race.** […] **Fix:** send `head_commit_id: evidence.headSha` (Forgejo returns 409 if the head moved). Add a caller test asserting the merge POST body carries the exact head. This race already existed for every leg; this spec is what makes it a hard requirement. (Full word-for-word text: review-88.md. Non-blocking findings N1–N7 noted, not addressed in this PR.) Record note only, not a verdict — the pane seats own the verdicts on this head.
ld-reviewer commented 2026-11-11 14:34:53 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=cd85805e5a95b81cddebc76849011cb863dab58d session=LR-20261111-017-seatB-20261111T1432Z

Seat B, fresh clone of the branch (origin/lr-20261111-017-census = cd85805e5a, base 0c5ef847a3; the PR's current head equals the pin, so the pin is not stale). Read the diff against origin/main and the three new suites; ran the suite.

GO leg (forgejo/landing.ts:4406-4920): gated list is source, case-insensitive; the record regex is fully anchored with a 40-hex head only (short shas are a named refusal, never a pass); the author check is separate from the text check and both must hold; edited comments are refused and an unreadable edit state (any comment item without an update time) is refused rather than assumed; an empty comment read is a refusal, not "no go"; only the matching rows' edit state is judged, and the newest unedited match is the record, so a re-posted go supersedes. GO runs as leg 2b before the ledger and a permit prints the record's id — no path reaches a permit on the gated repos without it.

Seat leg (landing.ts:6170-6535, caller forgejo/index.ts:290-400, 640-655): per-seat rows>0, a served pair present and present in the reviewed exact-pair table (unobserved pair refused, so no alias is assumed), and the MODEL line measured against the table's declared with the served pair authoritative; the stream window must exist and the verdict must sit inside it plus the documented 5-minute slack. Independence is now pairwise over EVERY pair for any panel size, each pair read from an actual endpoint answer in either direction with the worst fact winning (distinct must be true in every reading, shared ids max), a pair never compared refuses, and a failing pair demotes both of its members. The count is two VERIFIED distinct sessions on a gated repo regardless of dangerous class, checked on the verified set rather than the declared one, so one session twice never lands. Endpoint wiring: fixed in-code base address (no tool parameter can redirect it), 10 s abort, malformed URL inside the try so it lands as unreadable, all-or-nothing read so a partial answer can never look answered; absent/unreadable refuses in the lock's own words. Tag side: run id from the LAST Lab-Run trailer, regex-escaped, anchored, case-sensitive.

Tests: the spec's named cases are all present and non-vacuous — landing-go-record.test.ts (no record, other head/PR/repo, other author, edited, unreadable edit state, no comments, short sha, newest-match permit, non-gated no-op) and landing-seat-verification.test.ts (missing rows, served-model mismatch, same sid twice, routine AND dangerous one-seat refusals, shared caller correlation ids, not-distinct, verdict outside its own stream, unreadable endpoint, three-seat panels: all-pairs-shared / one-pair-shared / one-pair-not-distinct / a pair never compared / all-clean permit, qualified-vs-bare model line both directions). landing-gated-caller.test.ts covers the wiring (no-GO refusal, 502 lookup refusal, missing-trailer refusal, full green merge with both sids queried). I re-ran the suite myself: 53 files / 1278 tests, 1277 pass, and the single failure (tests/merge-identity.test.ts) reproduces identically on the untouched base commit in a clean worktree — pre-existing, not from this change.

Holds: none.

Non-blocking observations (no action required to land): (1) the comment above the verified-panel check says a CONCERN counts as a panel member, but countedSeats keeps approvals only — the effective rule is stricter than the comment, never looser; (2) on gated repos a single comment row missing an update time refuses the whole merge, which is the intended fail-closed reading but worth remembering if a forge response ever omits updated_at; (3) several older suites replay under the non-gated repo name so the repo-independent rules still reach their legs — the gated permit path is now covered only by the new suites, which is where I checked it.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1432Z

LD-VERDICT APPROVED head=cd85805e5a95b81cddebc76849011cb863dab58d session=LR-20261111-017-seatB-20261111T1432Z Seat B, fresh clone of the branch (origin/lr-20261111-017-census = cd85805e5a95b81cddebc76849011cb863dab58d, base 0c5ef847a37e88f1382c9f27c6cd8f384398ae8e; the PR's current head equals the pin, so the pin is not stale). Read the diff against origin/main and the three new suites; ran the suite. GO leg (forgejo/landing.ts:4406-4920): gated list is source, case-insensitive; the record regex is fully anchored with a 40-hex head only (short shas are a named refusal, never a pass); the author check is separate from the text check and both must hold; edited comments are refused and an unreadable edit state (any comment item without an update time) is refused rather than assumed; an empty comment read is a refusal, not "no go"; only the matching rows' edit state is judged, and the newest unedited match is the record, so a re-posted go supersedes. GO runs as leg 2b before the ledger and a permit prints the record's id — no path reaches a permit on the gated repos without it. Seat leg (landing.ts:6170-6535, caller forgejo/index.ts:290-400, 640-655): per-seat rows>0, a served pair present and present in the reviewed exact-pair table (unobserved pair refused, so no alias is assumed), and the MODEL line measured against the table's `declared` with the served pair authoritative; the stream window must exist and the verdict must sit inside it plus the documented 5-minute slack. Independence is now pairwise over EVERY pair for any panel size, each pair read from an actual endpoint answer in either direction with the worst fact winning (distinct must be true in every reading, shared ids max), a pair never compared refuses, and a failing pair demotes both of its members. The count is two VERIFIED distinct sessions on a gated repo regardless of dangerous class, checked on the verified set rather than the declared one, so one session twice never lands. Endpoint wiring: fixed in-code base address (no tool parameter can redirect it), 10 s abort, malformed URL inside the try so it lands as unreadable, all-or-nothing read so a partial answer can never look answered; absent/unreadable refuses in the lock's own words. Tag side: run id from the LAST Lab-Run trailer, regex-escaped, anchored, case-sensitive. Tests: the spec's named cases are all present and non-vacuous — landing-go-record.test.ts (no record, other head/PR/repo, other author, edited, unreadable edit state, no comments, short sha, newest-match permit, non-gated no-op) and landing-seat-verification.test.ts (missing rows, served-model mismatch, same sid twice, routine AND dangerous one-seat refusals, shared caller correlation ids, not-distinct, verdict outside its own stream, unreadable endpoint, three-seat panels: all-pairs-shared / one-pair-shared / one-pair-not-distinct / a pair never compared / all-clean permit, qualified-vs-bare model line both directions). landing-gated-caller.test.ts covers the wiring (no-GO refusal, 502 lookup refusal, missing-trailer refusal, full green merge with both sids queried). I re-ran the suite myself: 53 files / 1278 tests, 1277 pass, and the single failure (tests/merge-identity.test.ts) reproduces identically on the untouched base commit in a clean worktree — pre-existing, not from this change. Holds: none. Non-blocking observations (no action required to land): (1) the comment above the verified-panel check says a CONCERN counts as a panel member, but countedSeats keeps approvals only — the effective rule is stricter than the comment, never looser; (2) on gated repos a single comment row missing an update time refuses the whole merge, which is the intended fail-closed reading but worth remembering if a forge response ever omits `updated_at`; (3) several older suites replay under the non-gated repo name so the repo-independent rules still reach their legs — the gated permit path is now covered only by the new suites, which is where I checked it. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1432Z
ld-reviewer commented 2026-11-11 14:36:11 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=cd85805e5a95b81cddebc76849011cb863dab58d session=LR-20261111-017-seatA-20261111T1432Z

Exact head verified with the forge: PR #125 head is cd85805e5a (base 0c5ef847a3), equal to the pin. Read the diff origin/main..head (14 files: landing.ts +993, index.ts wiring, api.ts, verdict.ts, verdict-cli.ts, 3 new test files) on a read-only clone; did not read any comment or review on this PR.

GO leg: LD-GO grammar is anchored and strict — full 40-hex head only (uppercase normalised, short sha refused), field order and any prose refused, author compared to ld-director-go, repo/PR/current head compared exactly, updated_at != created_at refused, absent update time refused as unreadable, an empty comment read refused as unreadable rather than "no go posted", newest unedited matching comment is the record; zero effect on non-gated repos (record shape and note unchanged).

Seat leg: run id from the head commit's LAST Lab-Run trailer, tag anchored ^-seat(A|B)-$ with the run id regex-escaped; every counted exact-head approval sid is looked up, rows>0 required, every served pair must be in the reviewed exact-pair table, the declared MODEL line is read against that pair's declared id (bare or provider-qualified; the served pair is the evidence), verdict placement inside its own sid's stream, and caller-supplied correlation ids refused. Independence is pairwise over all N*(N-1)/2 declared pairs, both directions read with the worst fact winning, a pair never compared refused, unreadable/absent endpoint refused. On GO-gated repos requiredSeats is 2 regardless of class, so routine landings need two distinct verified sessions, and a permit is returned only when every declared seat and every pair passed. Caller: the lookup is fetched only for gated repos from a source constant (no env seam) with a 10s abort and all-or-nothing failure into unreadable; head message comes from the same commit read; updated_at reaches every item kind; the gate is decided on the forge's base.repo.full_name, so a percent-encoded caller ref cannot skip it; both GO_GATED_REPOS names exist on the forge as written.

Tests: forgejo vitest run → 53 files, 1277/1278 pass; the one failure (tests/merge-identity.test.ts) reproduces identically on the untouched base 0c5ef84 in this sandbox, so it is pre-existing and not a hold. tsc --noEmit clean. landing-go-record (15), landing-seat-verification (40) and landing-gated-caller (10) all pass, with specific one-line refusal assertions plus permit cases (no GO / wrong head / wrong PR / wrong author / edited / unreadable edit state / short sha / missing rows / unserved pair / served-over-declared / same sid twice / one seat on routine and dangerous / free-form and other-run tags / missing session / missing model / shared ids in a pair / correlated 3-panel / missing pair answer / endpoint 502), so the coverage is non-vacuous. Existing suites moved repo-independent rules onto a non-gated name, with the gated permit path covered by the new suites.

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1432Z

LD-VERDICT APPROVED head=cd85805e5a95b81cddebc76849011cb863dab58d session=LR-20261111-017-seatA-20261111T1432Z Exact head verified with the forge: PR #125 head is cd85805e5a95b81cddebc76849011cb863dab58d (base 0c5ef847a37e88f1382c9f27c6cd8f384398ae8e), equal to the pin. Read the diff origin/main..head (14 files: landing.ts +993, index.ts wiring, api.ts, verdict.ts, verdict-cli.ts, 3 new test files) on a read-only clone; did not read any comment or review on this PR. GO leg: LD-GO grammar is anchored and strict — full 40-hex head only (uppercase normalised, short sha refused), field order and any prose refused, author compared to ld-director-go, repo/PR/current head compared exactly, updated_at != created_at refused, absent update time refused as unreadable, an empty comment read refused as unreadable rather than "no go posted", newest unedited matching comment is the record; zero effect on non-gated repos (record shape and note unchanged). Seat leg: run id from the head commit's LAST Lab-Run trailer, tag anchored ^<run>-seat(A|B)-<stamp>$ with the run id regex-escaped; every counted exact-head approval sid is looked up, rows>0 required, every served pair must be in the reviewed exact-pair table, the declared MODEL line is read against that pair's declared id (bare or provider-qualified; the served pair is the evidence), verdict placement inside its own sid's stream, and caller-supplied correlation ids refused. Independence is pairwise over all N*(N-1)/2 declared pairs, both directions read with the worst fact winning, a pair never compared refused, unreadable/absent endpoint refused. On GO-gated repos requiredSeats is 2 regardless of class, so routine landings need two distinct verified sessions, and a permit is returned only when every declared seat and every pair passed. Caller: the lookup is fetched only for gated repos from a source constant (no env seam) with a 10s abort and all-or-nothing failure into unreadable; head message comes from the same commit read; updated_at reaches every item kind; the gate is decided on the forge's base.repo.full_name, so a percent-encoded caller ref cannot skip it; both GO_GATED_REPOS names exist on the forge as written. Tests: forgejo `vitest run` → 53 files, 1277/1278 pass; the one failure (tests/merge-identity.test.ts) reproduces identically on the untouched base 0c5ef84 in this sandbox, so it is pre-existing and not a hold. `tsc --noEmit` clean. landing-go-record (15), landing-seat-verification (40) and landing-gated-caller (10) all pass, with specific one-line refusal assertions plus permit cases (no GO / wrong head / wrong PR / wrong author / edited / unreadable edit state / short sha / missing rows / unserved pair / served-over-declared / same sid twice / one seat on routine and dangerous / free-form and other-run tags / missing session / missing model / shared ids in a pair / correlated 3-panel / missing pair answer / endpoint 502), so the coverage is non-vacuous. Existing suites moved repo-independent rules onto a non-gated name, with the gated permit path covered by the new suites. Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1432Z
pi-landing force-pushed lr-20261111-017-census from cd85805e5a
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / forgejo-ci (pull_request) Successful in 49s
Details
CI / pi-otel-telemetry (pull_request) Successful in 54s
Details
CI / mcp-bridge (pull_request) Successful in 20s
Details
tests / python (pull_request) Successful in 5s
Details
CI / plane (pull_request) Successful in 1m32s
Details
CI / ask-user-question (pull_request) Successful in 45s
Details
CI / skill-args (pull_request) Successful in 46s
Details
CI / nats-intercom (pull_request) Successful in 20s
Details
CI / redact (pull_request) Successful in 40s
Details
CI / pi-friction (pull_request) Successful in 43s
Details
CI / forgejo (pull_request) Successful in 2m7s
Details
CI / todo (pull_request) Successful in 1m19s
Details
CI / refocus (pull_request) Successful in 1m19s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m38s
Details
to 0ab501e51d
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / forgejo-ci (pull_request) Successful in 52s
Details
CI / skill-args (pull_request) Successful in 52s
Details
CI / todo (pull_request) Successful in 57s
Details
tests / python (pull_request) Successful in 6s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m2s
Details
CI / ask-user-question (pull_request) Successful in 1m0s
Details
CI / plane (pull_request) Successful in 1m16s
Details
CI / mcp-bridge (pull_request) Successful in 25s
Details
CI / nats-intercom (pull_request) Successful in 24s
Details
CI / redact (pull_request) Successful in 45s
Details
CI / forgejo (pull_request) Successful in 1m39s
Details
CI / pi-friction (pull_request) Successful in 46s
Details
CI / refocus (pull_request) Successful in 54s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m45s
Details
2026-11-11 15:00:30 +00:00 Compare
pi-landing commented 2026-11-11 15:00:40 +00:00
Author
Collaborator
Copy link

infra review (homelab-88), file review-88-pr125-r2.md — the round-2 blocking finding, quoted verbatim (B1–B4 were confirmed fixed in r2; this head adds the B6 fix, plus N6/N8 and a rebase on 491882a6 per N13):

B6 (blocking): an edited seat verdict defeats B1. In 5e-i, seat.at is the comment's created_at. The seat leg never reads updatedAt; only the GO leg refuses edited rows. […] Failure scenario: seat sid S ran for run X and posted APPROVED head: H1 at T, inside its stream. After a push to H2, someone on the shared ld-reviewer key PATCHes that comment's body to head: H2. created_at is still T […] the seat counts as verified for a head it never reviewed. […] Fix: on gated repos, refuse a counted seat whose item has updatedAt absent or different from at, the same rule the GO leg uses. Formal reviews need care […] either compare against submitted_at with a small tolerance, or count only issue comments as seats on gated repos. Tests: an edited in-stream seat comment is refused; a seat item with no updatedAt is refused.

The fix as landed here: the edited/absent-updatedAt seat refuses (one line naming the seat), and — the stated choice — on gated repos only issue comments count as seats (a formal review's updated_at can move without an edit; the fleet posts seats as comments). Both tests fail on the old code. Record note only, not a verdict — the pane seats own the verdicts on this head.

infra review (homelab-88), file review-88-pr125-r2.md — the round-2 blocking finding, quoted verbatim (B1–B4 were confirmed fixed in r2; this head adds the B6 fix, plus N6/N8 and a rebase on 491882a6 per N13): > **B6 (blocking): an edited seat verdict defeats B1.** In 5e-i, `seat.at` is the comment's `created_at`. The seat leg never reads `updatedAt`; only the GO leg refuses edited rows. […] **Failure scenario:** seat sid S ran for run X and posted `APPROVED head: H1` at T, inside its stream. After a push to H2, someone on the shared `ld-reviewer` key PATCHes that comment's body to `head: H2`. `created_at` is still T […] the seat counts as verified for a head it never reviewed. […] **Fix:** on gated repos, refuse a counted seat whose item has `updatedAt` absent or different from `at`, the same rule the GO leg uses. Formal reviews need care […] either compare against `submitted_at` with a small tolerance, or count only issue comments as seats on gated repos. **Tests:** an edited in-stream seat comment is refused; a seat item with no `updatedAt` is refused. The fix as landed here: the edited/absent-`updatedAt` seat refuses (one line naming the seat), and — the stated choice — on gated repos only issue comments count as seats (a formal review's `updated_at` can move without an edit; the fleet posts seats as comments). Both tests fail on the old code. Record note only, not a verdict — the pane seats own the verdicts on this head.
ld-reviewer commented 2026-11-11 15:06:55 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=0ab501e51d3e96c34c4908987b3613469913ee5f session=LR-20261111-017-seatB-20261111T1502Z

Read a fresh read-only clone of Homelab/pi-extensions, fetched branch lr-20261111-017-census and read the diff against origin/main. PR head reported by the forge is 0ab501e51d — the same 40 hex I reviewed; the clone HEAD and the pin agree.

GO leg (forgejo/landing.ts:4492 GO_GATED_REPOS/two repos, :4567 parseGoRecord, :4720-4820 goRecordLeg, wired at :5800 inside isGoGatedRepo): grammar is anchored whole-comment ^LD-GO repo=(\S+) pr=(\d+) head=([0-9a-fA-F]{40})$ with surrounding whitespace trimmed only; short sha, prose around it, reordered/missing fields all parse to null; author must be the ld-director-go identity (:4801) and is checked before the text decides anything; a full 40-hex equality against the current head plus repo and PR; ALL comments must carry an update time (:4783) and the matching record must have updated_at == created_at (:4810), both refusals one line; zero comment items is refused as an unreadable read, not as "no go" (:4770). Unedited newest matching record wins deterministically. Non-gated repos return before touching items, so their results are unchanged.

Seat leg (:6300-6660): every counted trusted exact-head approval's declared sid is looked up; per-seat answer gives rows>0, served (requested,served) pair must be a reviewed table row (no equality shortcut, no case folding), MODEL line is measured against the pair the gateway served and accepts bare or provider-qualified spelling only; unreadable/absent endpoint refused in the lock's own words; verdict must sit inside its own sid's stream (first..last + 5 min slack, both boundaries pinned) and an EDITED or update-time-less verdict comment is refused; caller-supplied correlation ids refused. Independence is pairwise over ALL N*(N-1)/2 pairs (:6620), read from actual endpoint answers, worst fact wins, a pair with no answer refused rather than assumed clean, and both members of a failing pair demoted to UNVERIFIED. requiredSeats is 2 on GO-gated repos regardless of dangerous class (:6560) and 5g counts distinct verified sids (:6650) — the routine gated single-seat case is the case the tests newly refuse. Seat tag is ^<escaped Lab-Run id>-seat(A|B)-\d{8}T\d{4}Z$ from the head commit's LAST Lab-Run trailer, so another run's tag cannot verify.

Tests: vitest run in forgejo/ = 55 files, 1295 tests, 1294 pass. The single failure is tests/merge-identity.test.ts > "merges with the dedicated landing credential" — I confirmed it fails identically on an untouched origin/main worktree, so it is pre-existing and not a hold. tsc --noEmit clean. Named spec cases are present and non-vacuous (tests/landing-go-record.test.ts:88-260, tests/landing-seat-verification.test.ts:231-880: no-record, other head, other pr, other author, edited, unreadable edit state, no comments, short sha, newest-unedited-wins, non-gated unaffected; missing rows, no answer, unserved pair, MODEL-line mismatch, same sid twice, one-seat routine and dangerous refusals, shared correlation ids, not-distinct, stream edges, unreadable/absent lookup, free-form and other-run tags, missing SESSION, missing MODEL, both spellings, 3-seat pairwise panel in all four shapes and the green pair); tests/landing-gated-caller.test.ts:115-290 covers the caller wiring — no-GO refusal, 502 lookup, env var FORGEJO_SEAT_LOOKUP_URL proven inert, repo identity decided on the server name with percent-encoded ref refused, missing base.repo.full_name refused. The fixture edits that move some older replays to a non-gated repo name are justified in-line (the replayed rules are repo-independent and the gated legs have their own suites); no assertion was silently loosened.

Holds: none.

Notes, not holds: the census row rendering of the new GO/SEATS fields (forgejo/index.ts:1305-1320) has no direct test (the merge-path note is covered); the verdict-cli dry run now always refuses on gated repos by design (forgejo/verdict.ts:760), documented in place.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1502Z

LD-VERDICT APPROVED head=0ab501e51d3e96c34c4908987b3613469913ee5f session=LR-20261111-017-seatB-20261111T1502Z Read a fresh read-only clone of Homelab/pi-extensions, fetched branch lr-20261111-017-census and read the diff against origin/main. PR head reported by the forge is 0ab501e51d3e96c34c4908987b3613469913ee5f — the same 40 hex I reviewed; the clone HEAD and the pin agree. GO leg (forgejo/landing.ts:4492 GO_GATED_REPOS/two repos, :4567 parseGoRecord, :4720-4820 goRecordLeg, wired at :5800 inside isGoGatedRepo): grammar is anchored whole-comment `^LD-GO repo=(\S+) pr=(\d+) head=([0-9a-fA-F]{40})$` with surrounding whitespace trimmed only; short sha, prose around it, reordered/missing fields all parse to null; author must be the ld-director-go identity (:4801) and is checked before the text decides anything; a full 40-hex equality against the current head plus repo and PR; ALL comments must carry an update time (:4783) and the matching record must have updated_at == created_at (:4810), both refusals one line; zero comment items is refused as an unreadable read, not as "no go" (:4770). Unedited newest matching record wins deterministically. Non-gated repos return before touching items, so their results are unchanged. Seat leg (:6300-6660): every counted trusted exact-head approval's declared sid is looked up; per-seat answer gives rows>0, served (requested,served) pair must be a reviewed table row (no equality shortcut, no case folding), MODEL line is measured against the pair the gateway served and accepts bare or provider-qualified spelling only; unreadable/absent endpoint refused in the lock's own words; verdict must sit inside its own sid's stream (first..last + 5 min slack, both boundaries pinned) and an EDITED or update-time-less verdict comment is refused; caller-supplied correlation ids refused. Independence is pairwise over ALL N*(N-1)/2 pairs (:6620), read from actual endpoint answers, worst fact wins, a pair with no answer refused rather than assumed clean, and both members of a failing pair demoted to UNVERIFIED. requiredSeats is 2 on GO-gated repos regardless of dangerous class (:6560) and 5g counts distinct verified sids (:6650) — the routine gated single-seat case is the case the tests newly refuse. Seat tag is `^<escaped Lab-Run id>-seat(A|B)-\d{8}T\d{4}Z$` from the head commit's LAST Lab-Run trailer, so another run's tag cannot verify. Tests: `vitest run` in forgejo/ = 55 files, 1295 tests, 1294 pass. The single failure is tests/merge-identity.test.ts > "merges with the dedicated landing credential" — I confirmed it fails identically on an untouched origin/main worktree, so it is pre-existing and not a hold. `tsc --noEmit` clean. Named spec cases are present and non-vacuous (tests/landing-go-record.test.ts:88-260, tests/landing-seat-verification.test.ts:231-880: no-record, other head, other pr, other author, edited, unreadable edit state, no comments, short sha, newest-unedited-wins, non-gated unaffected; missing rows, no answer, unserved pair, MODEL-line mismatch, same sid twice, one-seat routine and dangerous refusals, shared correlation ids, not-distinct, stream edges, unreadable/absent lookup, free-form and other-run tags, missing SESSION, missing MODEL, both spellings, 3-seat pairwise panel in all four shapes and the green pair); tests/landing-gated-caller.test.ts:115-290 covers the caller wiring — no-GO refusal, 502 lookup, env var FORGEJO_SEAT_LOOKUP_URL proven inert, repo identity decided on the server name with percent-encoded ref refused, missing base.repo.full_name refused. The fixture edits that move some older replays to a non-gated repo name are justified in-line (the replayed rules are repo-independent and the gated legs have their own suites); no assertion was silently loosened. Holds: none. Notes, not holds: the census row rendering of the new GO/SEATS fields (forgejo/index.ts:1305-1320) has no direct test (the merge-path note is covered); the verdict-cli dry run now always refuses on gated repos by design (forgejo/verdict.ts:760), documented in place. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1502Z
ld-reviewer commented 2026-11-11 15:07:03 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=0ab501e51d3e96c34c4908987b3613469913ee5f session=LR-20261111-017-seatA-20261111T1502Z

Head pinned. PR #125 head reads 0ab501e51d — exactly the pin; one commit over base 491882a. Judged the diff, the tests and the guard only.

Diff read (clone of Homelab/pi-extensions, branch lr-20261111-017-census vs origin/main): forgejo/landing.ts (+1140), forgejo/index.ts (+312), forgejo/api.ts, forgejo/verdict.ts, forgejo/verdict-cli.ts, forgejo/tests/fake.ts plus 4 fixture suites adjusted, 4 new suites.

  • GO leg (leg 2b, gated repos only): anchored ^LD-GO repo=\S+ pr=\d+ head=[0-9a-fA-F]{40}$, author must be the named GO identity, newest UNEDITED matching row is the record; an empty comment read, a missing updated_at (unreadable edit state), any non-matching repo/PR/head, a short sha and an edited record each refuse in one line. The gated list is source (GO_GATED_REPOS), case-insensitive; non-gated repos skip the leg entirely.
  • Seat leg (leg 5, 5a–5g): the tag is the anchored ^<run>-seat(A|B)-<stamp>$ with the run id taken from the head commit's LAST Lab-Run: trailer; rows>0, the served pair must be a reviewed exact-pair row and the MODEL line is measured against that row (bare or provider-qualified, served trusted over the declared claim); the verdict must sit inside its own sid's stream (first..last + small posting slack); any caller-supplied correlation id refuses; independence is checked PAIRWISE over every N*(N-1)/2 pair with worst-fact-wins, and a pair that was never asked refuses; endpoint absent/unreadable refuses in the lock's own words; TWO distinct VERIFIED seats are demanded on the gated repos, routine included. There is exactly one permit return and it sits after this leg, so no gated permit can bypass it.
  • Caller wiring: the gate is decided on the forge's own base.repo.full_name (a percent-encoded or redirected ref cannot disable it), the endpoint address is source (no env seam), one commit read supplies date+message, and a failed lookup lands as unreadable rather than throwing. The fixture re-pointings in the corpus/hold/blocker suites are documented and keep the repo-independent rules exercised.

Tests run in my own sandbox at this head: 55 files / 1295 tests → 1294 pass, 1 fail (tests/merge-identity.test.ts, "merges with the dedicated landing credential" — ambient landing token). I reproduced that same single failure on the untouched origin/main (51 files / 1215 tests, same test, same shape), so NEW FAILURES 0 is confirmed by my own base-vs-head comparison (+4 files, +80 tests). tsc --noEmit clean; no .only / .skip / .todo added. The new suites' cases assert the specific one-line reason and the demoted seat record, not just a boolean.

Holds: none.
Non-blocking note: the body's proof line says 53 files / 1256 tests; I measure 55 / 1295 at this head — a stale count in the description, while the behavioural claims are accurate.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1502Z

LD-VERDICT APPROVED head=0ab501e51d3e96c34c4908987b3613469913ee5f session=LR-20261111-017-seatA-20261111T1502Z **Head pinned.** PR #125 head reads 0ab501e51d3e96c34c4908987b3613469913ee5f — exactly the pin; one commit over base 491882a. Judged the diff, the tests and the guard only. **Diff read** (clone of Homelab/pi-extensions, branch lr-20261111-017-census vs origin/main): `forgejo/landing.ts` (+1140), `forgejo/index.ts` (+312), `forgejo/api.ts`, `forgejo/verdict.ts`, `forgejo/verdict-cli.ts`, `forgejo/tests/fake.ts` plus 4 fixture suites adjusted, 4 new suites. - GO leg (leg 2b, gated repos only): anchored `^LD-GO repo=\S+ pr=\d+ head=[0-9a-fA-F]{40}$`, author must be the named GO identity, newest UNEDITED matching row is the record; an empty comment read, a missing `updated_at` (unreadable edit state), any non-matching repo/PR/head, a short sha and an edited record each refuse in one line. The gated list is source (`GO_GATED_REPOS`), case-insensitive; non-gated repos skip the leg entirely. - Seat leg (leg 5, 5a–5g): the tag is the anchored `^<run>-seat(A|B)-<stamp>$` with the run id taken from the head commit's LAST `Lab-Run:` trailer; rows>0, the served pair must be a reviewed exact-pair row and the MODEL line is measured against that row (bare or provider-qualified, served trusted over the declared claim); the verdict must sit inside its own sid's stream (first..last + small posting slack); any caller-supplied correlation id refuses; independence is checked PAIRWISE over every N*(N-1)/2 pair with worst-fact-wins, and a pair that was never asked refuses; endpoint absent/unreadable refuses in the lock's own words; TWO distinct VERIFIED seats are demanded on the gated repos, routine included. There is exactly one permit return and it sits after this leg, so no gated permit can bypass it. - Caller wiring: the gate is decided on the forge's own `base.repo.full_name` (a percent-encoded or redirected ref cannot disable it), the endpoint address is source (no env seam), one commit read supplies date+message, and a failed lookup lands as `unreadable` rather than throwing. The fixture re-pointings in the corpus/hold/blocker suites are documented and keep the repo-independent rules exercised. **Tests run in my own sandbox at this head:** 55 files / 1295 tests → 1294 pass, 1 fail (`tests/merge-identity.test.ts`, "merges with the dedicated landing credential" — ambient landing token). I reproduced that same single failure on the untouched origin/main (51 files / 1215 tests, same test, same shape), so **NEW FAILURES 0** is confirmed by my own base-vs-head comparison (+4 files, +80 tests). `tsc --noEmit` clean; no `.only` / `.skip` / `.todo` added. The new suites' cases assert the specific one-line reason and the demoted seat record, not just a boolean. **Holds: none.** Non-blocking note: the body's proof line says 53 files / 1256 tests; I measure 55 / 1295 at this head — a stale count in the description, while the behavioural claims are accurate. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1502Z
pi-landing force-pushed lr-20261111-017-census from 0ab501e51d
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / forgejo-ci (pull_request) Successful in 52s
Details
CI / skill-args (pull_request) Successful in 52s
Details
CI / todo (pull_request) Successful in 57s
Details
tests / python (pull_request) Successful in 6s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m2s
Details
CI / ask-user-question (pull_request) Successful in 1m0s
Details
CI / plane (pull_request) Successful in 1m16s
Details
CI / mcp-bridge (pull_request) Successful in 25s
Details
CI / nats-intercom (pull_request) Successful in 24s
Details
CI / redact (pull_request) Successful in 45s
Details
CI / forgejo (pull_request) Successful in 1m39s
Details
CI / pi-friction (pull_request) Successful in 46s
Details
CI / refocus (pull_request) Successful in 54s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m45s
Details
to 56db9626f5
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / mcp-bridge (pull_request) Successful in 32s
Details
CI / nats-intercom (pull_request) Successful in 32s
Details
tests / python (pull_request) Successful in 7s
Details
CI / skill-args (pull_request) Successful in 46s
Details
CI / forgejo-ci (pull_request) Successful in 54s
Details
CI / pi-friction (pull_request) Successful in 44s
Details
CI / ask-user-question (pull_request) Successful in 56s
Details
CI / todo (pull_request) Successful in 1m4s
Details
CI / refocus (pull_request) Successful in 1m1s
Details
CI / redact (pull_request) Successful in 35s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m13s
Details
CI / plane (pull_request) Successful in 1m33s
Details
CI / forgejo (pull_request) Successful in 2m2s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m47s
Details
2026-11-11 15:54:55 +00:00 Compare
pi-landing commented 2026-11-11 15:56:26 +00:00
Author
Collaborator
Copy link

infra review (homelab-88), file review-88-pr125-r3.md — the r3 blocking finding and the r4 items it spawned, quoted/paraphrased with the fixes now at head 56db9626:

B7 (blocking on the spec; fails closed): Amendment 8's code is missing — the single cluster URL constant; no KUBERNETES_SERVICE_HOST choice of 127.0.0.1:8097; no N7 echo check; no tests — so every mgmt landing would refuse.

Fixed (r4): two source constants (SEAT_LOOKUP_CLUSTER_URL / SEAT_LOOKUP_MGMT_URL) chosen by KUBERNETES_SERVICE_HOST in code — never a tool parameter or free env value; N7 echo check (sid/other literal + answer timestamps inside the requested window → mismatch is unreadable); tests for both URL branches and the mismatch, all failing on the old code.

N14 (r4, El Director: BLOCKING on gated repos): a formal review must NOT withdraw a seat's CONCERN; only the same seat session's own follow-up comment clears it.

Fixed: the withdrawal match is scoped to counted comment verdicts on gated repos ("only a COMMENT withdraws a hold"), wired at both call sites; test: review-APPROVED session:S does not clear seat S's concern; S's own follow-up comment does. Fails on the old code.

Amendment 11 (also r4): no-trailer = refuse stays THE rule; the run-id grammar ^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$ now has its own refusal sentence; sendrast-v0.7.2-chain passes AS the trailer value; bad-grammar / shared / unstamped sids and no-trailer all fail; no per-project shape exists or was added (Amendment 11, one rule for all).

N15 + N4: FORGEJO_DANGEROUS_PATHS / FORGEJO_PREMIUM_MODELS / FORGEJO_LEGACY_PANEL_OWNERS are ignored on gated repos (N6's class); items without created_at refuse on gated repos instead of falling back to updated_at. Tests fail on the old code.

Record note only, not a verdict — the pane seats own the verdicts on this head.

infra review (homelab-88), file review-88-pr125-r3.md — the r3 blocking finding and the r4 items it spawned, quoted/paraphrased with the fixes now at head `56db9626`: > **B7 (blocking on the spec; fails closed): Amendment 8's code is missing** — the single cluster URL constant; no KUBERNETES_SERVICE_HOST choice of 127.0.0.1:8097; no N7 echo check; no tests — so every mgmt landing would refuse. Fixed (r4): two source constants (`SEAT_LOOKUP_CLUSTER_URL` / `SEAT_LOOKUP_MGMT_URL`) chosen by `KUBERNETES_SERVICE_HOST` in code — never a tool parameter or free env value; N7 echo check (sid/other literal + answer timestamps inside the requested window → mismatch is unreadable); tests for both URL branches and the mismatch, all failing on the old code. > **N14 (r4, El Director: BLOCKING on gated repos):** a formal review must NOT withdraw a seat's CONCERN; only the same seat session's own follow-up comment clears it. Fixed: the withdrawal match is scoped to counted comment verdicts on gated repos ("only a COMMENT withdraws a hold"), wired at both call sites; test: review-APPROVED session:S does not clear seat S's concern; S's own follow-up comment does. Fails on the old code. **Amendment 11** (also r4): no-trailer = refuse stays THE rule; the run-id grammar `^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$` now has its own refusal sentence; `sendrast-v0.7.2-chain` passes AS the trailer value; bad-grammar / shared / unstamped sids and no-trailer all fail; no per-project shape exists or was added (Amendment 11, one rule for all). **N15 + N4**: `FORGEJO_DANGEROUS_PATHS` / `FORGEJO_PREMIUM_MODELS` / `FORGEJO_LEGACY_PANEL_OWNERS` are ignored on gated repos (N6's class); items without `created_at` refuse on gated repos instead of falling back to `updated_at`. Tests fail on the old code. Record note only, not a verdict — the pane seats own the verdicts on this head.
ld-reviewer commented 2026-11-11 16:00:31 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=56db9626f57ac59e477b3911cae287c4d416fb4b session=LR-20261111-017-seatB-20261111T1557Z

Seat B, fresh read-only clone of Homelab/pi-extensions; read the diff of branch lr-20261111-017-census against origin/main, checked out at 56db9626f5 (matches the pin and the PR's current head exactly).

GO leg (forgejo/landing.ts: goRecordLeg + GO_RECORD_RE/parseGoRecord/isGoGatedRepo): anchored ^LD-GO repo=<owner/name> pr=<n> head=<40 hex>$ with whitespace trim only, so prose, reordering and short shas never parse; author must be the ld-director-go identity before any text is considered; repo/PR/current full 40-hex head are compared after parse (wrong repo, wrong PR, other head each refused); a comment with no update time and a matching record with updatedAt != created_at are both refused, and the whole-comment edit-state read plus an empty comment read fail closed. Non-gated repos never enter the leg.

Seat leg (legs 5a–5g): every declared approving sid is looked up (not just the second one) — rows>0, a served (requested, served) pair present in the reviewed exact-pair table, and the declared model line measured against that table's declared id through declaredModelMatches, so the SERVED pair decides and both bare and provider-qualified spellings are the same claim while any other id is refused. The tag is the anchored ^<runId>-seat(A|B)-<stamp>$ built from the head commit's LAST Lab-Run trailer, with one run-id grammar and a malformed/oversized value refused on its own sentence; a verdict must sit inside its own sid's stream (first..last + 5 min slack) and be unedited (updatedAt === at), and only issue comments count as seats on the gated repos. Two DISTINCT verified seats are demanded on GO-gated repos for routine and dangerous heads alike (the generic dangerous?2:1 rule survives only as dead fallback). Independence is pairwise over every unordered pair (N*(N-1)/2), each pair needing its own endpoint answer, with worst-fact-wins across both directions, and either a non-distinct pair or shared caller correlation ids refuses — plus per-seat caller-supplied correlation ids refuse. Missing/absent/unreadable endpoint and an answer that does not echo the query is a one-line refusal in the lock's words, never a throw. Policy: gatedReviewerPolicy pins the default dangerous/premium/legacy/reviewer sets on the two gated repos regardless of how the policy arrived; the caller decides gating on the forge's own base.repo.full_name.

Tests: cd forgejo && vitest run → 55 files / 1325 pass, 1 fail (tests/merge-identity.test.ts "merges with the dedicated landing credential"); that same failure reproduces identically on an untouched origin/main worktree in this sandbox (ambient landing token), so it is pre-existing and NOT a hold. New suites landing-go-record (19 its), landing-seat-verification (57 its), landing-gated-caller, landing-gated-hold-withdrawal and landing-gated-reviewer-set all pass, contain no skips/todos, and drive evaluateLandingLock itself (not a stub); the spec's named cases (no record, other head, other PR, wrong author, edited, unreadable edit state, no comments, short sha; missing rows, served-model mismatch, same sid twice, unreadable endpoint, free-form tag, missing SESSION/MODEL, shared correlation ids, 3-seat correlated panel, green pair) are each present. tsc --noEmit is clean.

Holds: none.

Note (not a hold): the comment above 5g says the seat panel counts a CONCERN-only session, but countedSeats only ever collects approvals, so a head with one approval and one CONCERN-session refuses at 5g instead. The code is stricter than its comment, which is the safe direction; worth rewording later.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1557Z

LD-VERDICT APPROVED head=56db9626f57ac59e477b3911cae287c4d416fb4b session=LR-20261111-017-seatB-20261111T1557Z Seat B, fresh read-only clone of Homelab/pi-extensions; read the diff of branch lr-20261111-017-census against origin/main, checked out at 56db9626f57ac59e477b3911cae287c4d416fb4b (matches the pin and the PR's current head exactly). GO leg (forgejo/landing.ts: goRecordLeg + GO_RECORD_RE/parseGoRecord/isGoGatedRepo): anchored `^LD-GO repo=<owner/name> pr=<n> head=<40 hex>$` with whitespace trim only, so prose, reordering and short shas never parse; author must be the ld-director-go identity before any text is considered; repo/PR/current full 40-hex head are compared after parse (wrong repo, wrong PR, other head each refused); a comment with no update time and a matching record with updatedAt != created_at are both refused, and the whole-comment edit-state read plus an empty comment read fail closed. Non-gated repos never enter the leg. Seat leg (legs 5a–5g): every declared approving sid is looked up (not just the second one) — rows>0, a served (requested, served) pair present in the reviewed exact-pair table, and the declared model line measured against that table's declared id through declaredModelMatches, so the SERVED pair decides and both bare and provider-qualified spellings are the same claim while any other id is refused. The tag is the anchored `^<runId>-seat(A|B)-<stamp>$` built from the head commit's LAST Lab-Run trailer, with one run-id grammar and a malformed/oversized value refused on its own sentence; a verdict must sit inside its own sid's stream (first..last + 5 min slack) and be unedited (updatedAt === at), and only issue comments count as seats on the gated repos. Two DISTINCT verified seats are demanded on GO-gated repos for routine and dangerous heads alike (the generic dangerous?2:1 rule survives only as dead fallback). Independence is pairwise over every unordered pair (N*(N-1)/2), each pair needing its own endpoint answer, with worst-fact-wins across both directions, and either a non-distinct pair or shared caller correlation ids refuses — plus per-seat caller-supplied correlation ids refuse. Missing/absent/unreadable endpoint and an answer that does not echo the query is a one-line refusal in the lock's words, never a throw. Policy: gatedReviewerPolicy pins the default dangerous/premium/legacy/reviewer sets on the two gated repos regardless of how the policy arrived; the caller decides gating on the forge's own base.repo.full_name. Tests: cd forgejo && vitest run → 55 files / 1325 pass, 1 fail (tests/merge-identity.test.ts "merges with the dedicated landing credential"); that same failure reproduces identically on an untouched origin/main worktree in this sandbox (ambient landing token), so it is pre-existing and NOT a hold. New suites landing-go-record (19 its), landing-seat-verification (57 its), landing-gated-caller, landing-gated-hold-withdrawal and landing-gated-reviewer-set all pass, contain no skips/todos, and drive evaluateLandingLock itself (not a stub); the spec's named cases (no record, other head, other PR, wrong author, edited, unreadable edit state, no comments, short sha; missing rows, served-model mismatch, same sid twice, unreadable endpoint, free-form tag, missing SESSION/MODEL, shared correlation ids, 3-seat correlated panel, green pair) are each present. tsc --noEmit is clean. Holds: none. Note (not a hold): the comment above 5g says the seat panel counts a CONCERN-only session, but countedSeats only ever collects approvals, so a head with one approval and one CONCERN-session refuses at 5g instead. The code is stricter than its comment, which is the safe direction; worth rewording later. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1557Z
ld-reviewer commented 2026-11-11 16:03:14 +00:00
Collaborator
Copy link

LD-VERDICT REQUEST_CHANGES head=56db9626f57ac59e477b3911cae287c4d416fb4b session=LR-20261111-017-seatA-20261111T1557Z

Seat A, fresh single-run session. Judged the diff at the pinned head only (own read-only clone, checked out at 56db9626; note the branch was force-updated since an earlier checkout, so the pin was verified against the fetch, not a stale tree). The PR's current head IS 56db9626f5. tsc --noEmit clean at the pin; suite run at the pin.

GO leg (forgejo/landing.ts) — GO_GATED_REPOS:4511, GO_AUTHOR_IDENTITY:4525, GO_RECORD_RE:4538, goRecordLeg:5035-5113. Anchored LD-GO repo=<owner/name> pr=<n> head=<40-hex>, nothing forgiven but surrounding whitespace/one trailing newline; uppercase hex normalised, a short sha and any surrounding prose refused; author compared exactly (case-folded only); repo compared case-insensitively to the two named repos; PR and the PR's CURRENT full head both compared; edited record (updatedAt !== at) refused, and a missing update time is a refusal rather than a pass; an empty comment read is "unreadable", never "nobody posted a go"; the newest unedited matching comment is the record. Non-gated repos skip the leg, and the evaluator's only ok: true is the final permit (:6898) — no path permits before this leg.

Seat leg (same file + caller) — leg body :6425-6800. Plan asks one request per declared sid AND one per unordered pair (:4985-5020), read all-or-nothing with a 10 s ceiling (index.ts:405, :453); every fetched answer must echo its query (sid, other, window) or the whole read is unreadable. Per declared counted approval: rows > 0; every served pair must be in the reviewed exact-pair table (:4682, no served === requested shortcut, no case folding); the MODEL line must name that pair's declared id bare or provider-qualified (:4712) — served wins over the line; the verdict must sit inside its own sid's gateway stream ±5 min slack; rows carrying caller correlation ids refuse. Independence is PAIRWISE (:6711-6755): every unordered pair is compared, worst fact wins, a missing pair answer refuses instead of assuming "nothing shared", and both members of a refused pair are demoted in the record. On a GO-gated repo the demand is TWO verified distinct seats (:6510, :6755+) with the dangerous classification affecting only the wording — a routine gated landing on one seat is refused.

Tests — vitest at the pin: 56 files / 1326 tests, 1325 pass; the single failure is tests/merge-identity.test.ts > merges with the dedicated landing credential, never the operator token, which I reproduced identically on the untouched base b4b4478 → pre-existing in this sandbox (ambient landing token), not this change; NEW FAILURES 0 holds. The named cases are present and non-vacuous: go-record (19 its: no record, other head/PR/repo, wrong author, edited, unreadable edit state, no comments, short sha, newest-match, non-gated no-op, caller at-fallback), seat-verification (56 its: free-form tag, tag for another run, missing SESSION/MODEL, zero rows, absent answer, unserved pair, model disagreement both spellings, same sid twice, unreadable/absent endpoint, caller correlation ids, stream boundaries at ±1 s, edited verdict, 3-seat panels with one dirty pair and with a never-compared pair, green pair and green 3-seat panel), gated-caller (19 its: no GO, 502 lookup, missing trailer, both sids plus the pair actually queried, env cannot move the endpoint).

Holds

  1. forgejo/tests/fake.ts:444-447 — the pinned head does not merge onto main. git merge-tree b4b44788… 56db9626… reports exactly one content conflict, in this file: the branch rewrites the GET /pulls/{n} responder into prFixture() (plus repoFullName?:272, commitMessage?:284, :410) while main's #127 rewrote the same responder for prHeadSequence (main :425-441), and the PR reads Mergeable: false. Both sides are load-bearing for different tests — server-side repo identity for the gated caller, sequenced heads for the head-pin test — so the resolution is not cosmetic and a careless one silently drops one of the two fixtures. Rebase onto b4b4478 and re-review at the new head; that rebase also brings #127's required-pin hardening into the tree this review would cover.

Not holds (context for the next seat/merger)

  • The diff against main displays removals of #127's HEAD_PIN_RE, the required head_commit_id type, the private mergePR, the 409 refusal and two head-pin tests. Those are stale-base artefacts, not this change: the merge-base is 491882a (pre-#127), and a 3-way merge keeps them. A rebase makes the diff show only the real change.
  • The PR body's evidence line (53 files / 1256 tests) does not match the pinned head's tree (56 files / 1326 tests here), so that run was measured on a different tree; my numbers are at the pin.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1557Z

LD-VERDICT REQUEST_CHANGES head=56db9626f57ac59e477b3911cae287c4d416fb4b session=LR-20261111-017-seatA-20261111T1557Z Seat A, fresh single-run session. Judged the diff at the pinned head only (own read-only clone, checked out at 56db9626; note the branch was force-updated since an earlier checkout, so the pin was verified against the fetch, not a stale tree). The PR's current head IS 56db9626f57ac59e477b3911cae287c4d416fb4b. `tsc --noEmit` clean at the pin; suite run at the pin. **GO leg (forgejo/landing.ts)** — `GO_GATED_REPOS`:4511, `GO_AUTHOR_IDENTITY`:4525, `GO_RECORD_RE`:4538, `goRecordLeg`:5035-5113. Anchored `LD-GO repo=<owner/name> pr=<n> head=<40-hex>`, nothing forgiven but surrounding whitespace/one trailing newline; uppercase hex normalised, a short sha and any surrounding prose refused; author compared exactly (case-folded only); repo compared case-insensitively to the two named repos; PR and the PR's CURRENT full head both compared; edited record (`updatedAt !== at`) refused, and a missing update time is a refusal rather than a pass; an empty comment read is "unreadable", never "nobody posted a go"; the newest unedited matching comment is the record. Non-gated repos skip the leg, and the evaluator's only `ok: true` is the final permit (:6898) — no path permits before this leg. **Seat leg (same file + caller)** — leg body :6425-6800. Plan asks one request per declared sid AND one per unordered pair (:4985-5020), read all-or-nothing with a 10 s ceiling (index.ts:405, :453); every fetched answer must echo its query (sid, `other`, window) or the whole read is `unreadable`. Per declared counted approval: rows > 0; every served pair must be in the reviewed exact-pair table (:4682, no `served === requested` shortcut, no case folding); the MODEL line must name that pair's `declared` id bare or provider-qualified (:4712) — served wins over the line; the verdict must sit inside its own sid's gateway stream ±5 min slack; rows carrying caller correlation ids refuse. Independence is PAIRWISE (:6711-6755): every unordered pair is compared, worst fact wins, a missing pair answer refuses instead of assuming "nothing shared", and both members of a refused pair are demoted in the record. On a GO-gated repo the demand is TWO verified distinct seats (:6510, :6755+) with the dangerous classification affecting only the wording — a routine gated landing on one seat is refused. **Tests** — vitest at the pin: 56 files / 1326 tests, 1325 pass; the single failure is `tests/merge-identity.test.ts > merges with the dedicated landing credential, never the operator token`, which I reproduced identically on the untouched base b4b4478 → pre-existing in this sandbox (ambient landing token), not this change; NEW FAILURES 0 holds. The named cases are present and non-vacuous: go-record (19 its: no record, other head/PR/repo, wrong author, edited, unreadable edit state, no comments, short sha, newest-match, non-gated no-op, caller `at`-fallback), seat-verification (56 its: free-form tag, tag for another run, missing SESSION/MODEL, zero rows, absent answer, unserved pair, model disagreement both spellings, same sid twice, unreadable/absent endpoint, caller correlation ids, stream boundaries at ±1 s, edited verdict, 3-seat panels with one dirty pair and with a never-compared pair, green pair and green 3-seat panel), gated-caller (19 its: no GO, 502 lookup, missing trailer, both sids plus the pair actually queried, env cannot move the endpoint). **Holds** 1. `forgejo/tests/fake.ts:444-447` — the pinned head does not merge onto main. `git merge-tree b4b44788… 56db9626…` reports exactly one content conflict, in this file: the branch rewrites the `GET /pulls/{n}` responder into `prFixture()` (plus `repoFullName?`:272, `commitMessage?`:284, `:410`) while main's #127 rewrote the same responder for `prHeadSequence` (main :425-441), and the PR reads `Mergeable: false`. Both sides are load-bearing for different tests — server-side repo identity for the gated caller, sequenced heads for the head-pin test — so the resolution is not cosmetic and a careless one silently drops one of the two fixtures. Rebase onto b4b4478 and re-review at the new head; that rebase also brings #127's required-pin hardening into the tree this review would cover. **Not holds (context for the next seat/merger)** - The diff against main *displays* removals of #127's `HEAD_PIN_RE`, the required `head_commit_id` type, the private `mergePR`, the 409 refusal and two head-pin tests. Those are stale-base artefacts, not this change: the merge-base is 491882a (pre-#127), and a 3-way merge keeps them. A rebase makes the diff show only the real change. - The PR body's evidence line (53 files / 1256 tests) does not match the pinned head's tree (56 files / 1326 tests here), so that run was measured on a different tree; my numbers are at the pin. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1557Z
merge: main (b4b4478, #127 pin hardening) into the census gate branch (LR-20261111-017)
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / pi-otel-telemetry (pull_request) Successful in 56s
Details
CI / forgejo-ci (pull_request) Successful in 1m2s
Details
CI / skill-args (pull_request) Successful in 1m4s
Details
CI / todo (pull_request) Successful in 1m8s
Details
CI / ask-user-question (pull_request) Successful in 1m8s
Details
CI / refocus (pull_request) Successful in 1m17s
Details
tests / python (pull_request) Successful in 9s
Details
CI / plane (pull_request) Successful in 1m22s
Details
CI / mcp-bridge (pull_request) Successful in 32s
Details
CI / nats-intercom (pull_request) Successful in 36s
Details
CI / pi-friction (pull_request) Successful in 47s
Details
CI / redact (pull_request) Successful in 41s
Details
CI / forgejo (pull_request) Successful in 1m56s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m45s
Details
ea174d3d9c 
fake.ts conflict resolved keeping BOTH fixtures: #127's prHeadSequence (sequenced
heads A/B for the head-pin regression guard) and this branch's prFixture /
repoFullName / commitMessage (server-side repo identity for the gated caller).
Both are load-bearing for different tests.

Lab-Run: LR-20261111-017

# Conflicts:
#	forgejo/tests/fake.ts
ld-reviewer commented 2026-11-11 16:14:31 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=ea174d3d9c4926cf9fa35568775543dc60852b6e session=LR-20261111-017-seatA-20261111T1612Z

Seat A review of piece (l). PR head read back from the forge equals the pin ea174d3d9c. Judged the diff base 51daf88 -> head ea174d3 (19 files: forgejo/landing.ts +1285, forgejo/index.ts caller wiring, forgejo/api.ts getHeadCommitFacts, and 6 new/extended fixture suites).

GO leg (forgejo/landing.ts:4870-5010). Anchored record regex ^LD-GO repo=... pr=<n> head=<40 hex>$ with uppercase hex normalised; author compared against the ld-director-go constant only; repo compared against the forge's own base.repo.full_name (the caller's ref is unescaped and refused when it disagrees, so a percent-encoded or redirecting ref cannot switch the legs off); full 40-hex current-head match required and a short sha is refused with its own sentence naming the 40-hex rule; the newest UNEDITED matching record wins; an edited record (updatedAt != at) and an unreadable edit state (no update time on any comment, or no comment items at all) are refusals, not passes. Refusals name what is missing and the exact text to post.

Seat leg (forgejo/landing.ts:6400-6800, index.ts caller). Every counted seat's declared sid is looked up and verified: rows > 0, a readable first/last stream window, the verdict placed inside that sid's own stream with only the 5-minute posting slack, the served (requested, served) pair resolved through the reviewed alias table with SERVED trusted over the MODEL line, both spellings of the reviewed id accepted (bare and provider-qualified) and nothing else, caller-supplied correlation ids refused outright. Independence is PAIRWISE over every unordered pair of declared seats, both request directions read, worst fact wins, and a pair with no endpoint answer is refused rather than read as "nothing shared" - so a panel of 3+ correlated seats cannot pass. Two DISTINCT verified seats are required on GO-gated repos for routine and dangerous changes alike, and one sid declared twice counts as one seat. Endpoint address is chosen in source from where the process runs (no env value can set it), the answer must echo the request (sid, counterpart, window) or it is unreadable, and unreadable/absent lookup refuses in the lock's own words. Non-gated repos are provably untouched: gated policy substitution, comment-only hold withdrawal and the plan reduction are all scoped behind isGoGatedRepo.

Tests. vitest at ea174d3: 57 files, 1341 pass, 1 fail = tests/merge-identity.test.ts (ambient landing token), identical on the untouched base, not a hold. tsc --noEmit clean. The new suites (96 cases, none skipped, no .only/.todo) cover the spec's named cases non-vacuously: missing / other-head / other-PR / other-repo / wrong-author / edited / unreadable / short-sha GO; seat rows, stream boundaries at exactly last+slack and one second past, unobserved pair, declared-vs-served mismatch, three-seat pairwise refusals (all-pairs, one-pair, non-distinct pair, never-compared pair), 24h window cap, run-id grammar including a sendrast value and malformed values, plus non-gated positive controls that pin unchanged behaviour - and each refusal case asserts the refusal text while the green cases assert the recorded seat/pair facts.

holds: none.

Non-blocking notes (not holds): (1) forgejo/landing.ts:6510 - requiredSeats keeps the fallback branches isGoGatedRepo(...) ? 2 : cls.dangerous ? 2 : 1 inside a block that is already gated, so the fallback can never run and reads as if it could; (2) forgejo/landing.ts:6515 - the zero-seat refusal words the bar as "at least one" while the gated rule is two; correct for zero seats, but the sentence understates the requirement the next gate enforces.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1612Z

LD-VERDICT APPROVED head=ea174d3d9c4926cf9fa35568775543dc60852b6e session=LR-20261111-017-seatA-20261111T1612Z Seat A review of piece (l). PR head read back from the forge equals the pin ea174d3d9c4926cf9fa35568775543dc60852b6e. Judged the diff base 51daf88 -> head ea174d3 (19 files: forgejo/landing.ts +1285, forgejo/index.ts caller wiring, forgejo/api.ts getHeadCommitFacts, and 6 new/extended fixture suites). GO leg (forgejo/landing.ts:4870-5010). Anchored record regex `^LD-GO repo=... pr=<n> head=<40 hex>$` with uppercase hex normalised; author compared against the ld-director-go constant only; repo compared against the forge's own base.repo.full_name (the caller's ref is unescaped and refused when it disagrees, so a percent-encoded or redirecting ref cannot switch the legs off); full 40-hex current-head match required and a short sha is refused with its own sentence naming the 40-hex rule; the newest UNEDITED matching record wins; an edited record (updatedAt != at) and an unreadable edit state (no update time on any comment, or no comment items at all) are refusals, not passes. Refusals name what is missing and the exact text to post. Seat leg (forgejo/landing.ts:6400-6800, index.ts caller). Every counted seat's declared sid is looked up and verified: rows > 0, a readable first/last stream window, the verdict placed inside that sid's own stream with only the 5-minute posting slack, the served (requested, served) pair resolved through the reviewed alias table with SERVED trusted over the MODEL line, both spellings of the reviewed id accepted (bare and provider-qualified) and nothing else, caller-supplied correlation ids refused outright. Independence is PAIRWISE over every unordered pair of declared seats, both request directions read, worst fact wins, and a pair with no endpoint answer is refused rather than read as "nothing shared" - so a panel of 3+ correlated seats cannot pass. Two DISTINCT verified seats are required on GO-gated repos for routine and dangerous changes alike, and one sid declared twice counts as one seat. Endpoint address is chosen in source from where the process runs (no env value can set it), the answer must echo the request (sid, counterpart, window) or it is unreadable, and unreadable/absent lookup refuses in the lock's own words. Non-gated repos are provably untouched: gated policy substitution, comment-only hold withdrawal and the plan reduction are all scoped behind isGoGatedRepo. Tests. vitest at ea174d3: 57 files, 1341 pass, 1 fail = tests/merge-identity.test.ts (ambient landing token), identical on the untouched base, not a hold. tsc --noEmit clean. The new suites (96 cases, none skipped, no .only/.todo) cover the spec's named cases non-vacuously: missing / other-head / other-PR / other-repo / wrong-author / edited / unreadable / short-sha GO; seat rows, stream boundaries at exactly last+slack and one second past, unobserved pair, declared-vs-served mismatch, three-seat pairwise refusals (all-pairs, one-pair, non-distinct pair, never-compared pair), 24h window cap, run-id grammar including a sendrast value and malformed values, plus non-gated positive controls that pin unchanged behaviour - and each refusal case asserts the refusal text while the green cases assert the recorded seat/pair facts. holds: none. Non-blocking notes (not holds): (1) forgejo/landing.ts:6510 - `requiredSeats` keeps the fallback branches `isGoGatedRepo(...) ? 2 : cls.dangerous ? 2 : 1` inside a block that is already gated, so the fallback can never run and reads as if it could; (2) forgejo/landing.ts:6515 - the zero-seat refusal words the bar as "at least one" while the gated rule is two; correct for zero seats, but the sentence understates the requirement the next gate enforces. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1612Z
ld-reviewer commented 2026-11-11 16:15:25 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=ea174d3d9c4926cf9fa35568775543dc60852b6e session=LR-20261111-017-seatB-20261111T1612Z

Reviewed the base..head diff at the pinned head (forge's own head read: ea174d3d9c — matches the pin).

GO leg (forgejo/landing.ts:5035 goRecordLeg): grammar anchored and 40-hex only (GO_RECORD_RE, landing.ts:4463), so a short/typed sha and any prose around the record are refused; author must be ld-director-go; repo/PR/head all compared, with the naming mismatch refused in its own sentence; an edited record refused (updatedAt !== at on matching rows only); a missing updated_at on any comment and an empty comment read both refuse rather than pass (absence of edit state is a refusal, not a permit). Gate is decided on the server's pr.base.repo.full_name with an unreadable server identity refused (forgejo/index.ts:596), and it runs before the seat leg.

Seat leg (landing.ts:6425+): head commit message → last Lab-Run: trailer → one run-id grammar, with a bad value refused on its own sentence; seat sid must match the anchored <runId>-seat(A|B)-<stamp> tag; every declared seat is looked up and refused on: no answer, rows <= 0, no served pair, a pair outside the reviewed table, a MODEL line disagreeing with the SERVED pair (served trusted, bare and provider-qualified spellings accepted), no readable stream window, a verdict outside its own sid's first..last(+5 min slack), and caller-supplied correlation ids. Endpoint unreadable → refusal in the lock's words; answer must echo the request (sid, other, window). requiredSeats is 2 on both GO-gated repos for EVERY landing, routine included, and the verified set is a set of lowercased sids, so one session twice counts once. Independence is pairwise: every unordered pair of declared seats must have its own endpoint answer, distinct must be true and shared_correlation_ids 0 with worst-fact-wins across both directions; a missing pair answer refuses (a 3-seat panel with clean per-seat answers only is refused), and a refused pair demotes both members. Endpoint address is source, chosen from where the process runs — no env override; FORGEJO_* policy envs unread for the trusted reviewer set on gated repos.

Tests, at the pinned head (vitest run in forgejo/): 57 files, 1341 passed, 1 failed — only tests/merge-identity.test.ts (ambient landing token); I reproduced the same single failure on untouched base b4b4478, so it is pre-existing and not a hold. The spec's named cases are present and non-vacuous: strict GO grammar/short-sha/wrong-head/wrong-PR/wrong-repo/other-author/edited/unreadable/no-comments, newest-unedited-record wins, per-seat rows and served-pair admissibility, served-over-MODEL disagreement, same-sid-twice is one seat, routine AND dangerous one-seat refusals, stream placement at first, at last+slack and one second past it, endpoint-unreadable and pair-never-compared, 3-seat panels with every pair shared / one pair shared / one pair non-distinct / all clean, edited and no-update-time seat verdicts, formal-review rows not counted as seats on gated repos, and the trailer/run-id grammar cases. Base landing.ts contains no seat lookup or LD-GO path at all, so these cases cannot pass on the old code. Caller wiring in index.ts builds the plan from the same ledger the lock judges and carries an unreadable fetch as unreadable (never throws, never silently omitted).

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1612Z

LD-VERDICT APPROVED head=ea174d3d9c4926cf9fa35568775543dc60852b6e session=LR-20261111-017-seatB-20261111T1612Z Reviewed the base..head diff at the pinned head (forge's own head read: ea174d3d9c4926cf9fa35568775543dc60852b6e — matches the pin). GO leg (`forgejo/landing.ts:5035` `goRecordLeg`): grammar anchored and 40-hex only (`GO_RECORD_RE`, `landing.ts:4463`), so a short/typed sha and any prose around the record are refused; author must be `ld-director-go`; repo/PR/head all compared, with the naming mismatch refused in its own sentence; an edited record refused (`updatedAt !== at` on matching rows only); a missing `updated_at` on any comment and an empty comment read both refuse rather than pass (absence of edit state is a refusal, not a permit). Gate is decided on the server's `pr.base.repo.full_name` with an unreadable server identity refused (`forgejo/index.ts:596`), and it runs before the seat leg. Seat leg (`landing.ts:6425`+): head commit message → last `Lab-Run:` trailer → one run-id grammar, with a bad value refused on its own sentence; seat sid must match the anchored `<runId>-seat(A|B)-<stamp>` tag; every declared seat is looked up and refused on: no answer, `rows <= 0`, no served pair, a pair outside the reviewed table, a MODEL line disagreeing with the SERVED pair (served trusted, bare and provider-qualified spellings accepted), no readable stream window, a verdict outside its own sid's first..last(+5 min slack), and caller-supplied correlation ids. Endpoint unreadable → refusal in the lock's words; answer must echo the request (sid, other, window). `requiredSeats` is 2 on both GO-gated repos for EVERY landing, routine included, and the verified set is a set of lowercased sids, so one session twice counts once. Independence is pairwise: every unordered pair of declared seats must have its own endpoint answer, `distinct` must be true and `shared_correlation_ids` 0 with worst-fact-wins across both directions; a missing pair answer refuses (a 3-seat panel with clean per-seat answers only is refused), and a refused pair demotes both members. Endpoint address is source, chosen from where the process runs — no env override; `FORGEJO_*` policy envs unread for the trusted reviewer set on gated repos. Tests, at the pinned head (`vitest run` in `forgejo/`): 57 files, 1341 passed, 1 failed — only `tests/merge-identity.test.ts` (ambient landing token); I reproduced the same single failure on untouched base b4b4478, so it is pre-existing and not a hold. The spec's named cases are present and non-vacuous: strict GO grammar/short-sha/wrong-head/wrong-PR/wrong-repo/other-author/edited/unreadable/no-comments, newest-unedited-record wins, per-seat rows and served-pair admissibility, served-over-MODEL disagreement, same-sid-twice is one seat, routine AND dangerous one-seat refusals, stream placement at first, at last+slack and one second past it, endpoint-unreadable and pair-never-compared, 3-seat panels with every pair shared / one pair shared / one pair non-distinct / all clean, edited and no-update-time seat verdicts, formal-review rows not counted as seats on gated repos, and the trailer/run-id grammar cases. Base `landing.ts` contains no seat lookup or `LD-GO` path at all, so these cases cannot pass on the old code. Caller wiring in `index.ts` builds the plan from the same ledger the lock judges and carries an unreadable fetch as `unreadable` (never throws, never silently omitted). Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1612Z
Amendment 15 / review N6 says a value a session can set is not a gate. #130
delivered the narrow half (FORGEJO_LANDING_REVIEWERS ignored on the two
GO-gated repos) but decided "gated" from the string the CALLER typed, and left
three quieter knobs live there.

(1) Every gate choice now keys off `pr.base.repo.full_name` — the name the
FORGE reports — threaded out of `getPR` in the merge gate and the census and out
of the fetched PR in the verdict helper. Review-88-pr130 B1 measured the bypass:
`parseRepoRef("Homelab/pr%6Fd")` reached `/repos/Homelab/pr%6Fd`, Forgejo
percent-decodes path params, so the write landed on Homelab/prod while
`repoKey` saw an ungated repo. There is no `%` decoding in the gate's input path
any more: the caller's spelling is irrelevant to the decision. A name that could
not be READ is pinned (`goGatedFromServerName`), never treated as ungated (n1).

(2) On a GO-gated repo ALL FOUR knobs fall back to their defaults —
FORGEJO_LANDING_REVIEWERS, FORGEJO_DANGEROUS_PATHS,
FORGEJO_LEGACY_PANEL_OWNERS, FORGEJO_PREMIUM_MODELS. Pinning reviewers alone
left `FORGEJO_DANGEROUS_PATHS=nothing/**` able to make a `deploy/**` change
routine, a widened legacy-owner list able to swap the panel rule, and a model
added to the premium list able to upgrade a seat. Each ignored override is said
once per call, by NAME only (n3: the line never echoes the value).

The census resolves per row (each row is its own read) behind a de-duplicating
sink, so "once per call" still holds for the operator.

#125's review r4 carries the same N15 pin inside the lock; same names, same
values, same semantics — whichever lands first, the gated policy is the default.

Tests (each fails on #130's code): the pins file's four gated cases and the two
server-name cases in the N6 file; the non-gated controls assert today's
behaviour is untouched. The #694 replay is updated to the truth the pin implies:
a gated repo can no longer be declared legacy through the env, so its panel is
judged by the DEFAULT owners (session rule), and the cured stream is asserted
under an explicitly built era policy.

Lab-Run: LR-20261111-017
Amendment 15's gate was still decided from the string the CALLER typed.
Review-88-pr130 B1 measured the consequence: `parseRepoRef` and `request()`
passed the owner/repo through unchanged, Forgejo runs `url.PathUnescape` over
those path parameters, so `Homelab/pr%6Fd` ADDRESSED `Homelab/prod` while
`repoKey` saw an ungated repo — the trusted set widened, no line printed, and
the merge went out.

Now, at the one place a PR is read (`collectLandingEvidence`):
- the gated decision and every gate choice key off `pr.base.repo.full_name`,
  one URL-decode and case-fold behind it (`serverRepoView`);
- the requested name is used ONLY to refuse: a disagreement is one line naming
  both;
- an unreadable name REFUSES a merge and the census row, and PINS the verdict
  helper — a missing name must never mean "ungated" (n1);
- a `%` anywhere in a repo ref is refused outright, in `parseRepoRef` and in
  the verdict helper's `--repo` parser, naming the character class.

`repoKey` now also decodes one level, so a percent-encoded spelling of a gated
repo matches the gated list even if it reaches a comparison (defence in depth,
not the gate).

n1 delivery: the merge tool, the census and `verdict-cli` all carry the SERVER's
gate decision; undoing any one of them is caught by a test that drives the tool
over a fake forge, not by a unit test handed a ref.
n5: a posting run reads the PR twice, so `runCli` hands `collectState` a
de-duplicating sink — one line per ignored knob, not two.
n6: docs/forgejo.md now says the four overrides are not levers on a GO-gated
repo and that gated-ness comes from the forge's own name.

Tests (12 of 13 are red on the previous commit, and red on #130):
`forgejo_merge_pr refuses "Homelab/pr%6Fd" | "%48omelab/prod" |
"Homelab/infra%2Dcore" and sends no merge` (the homelab-88 reproductions),
`refuses when the request and the forge's own name are different repos`,
`refuses when the forge reports no repo name at all`, `refuses mid-census too`,
`refuses --repo with a percent-escape`, `says the line ONCE across a posting
run`, plus `serverRepoView` unit cases (still pinned + logged for a
percent-encoded request against a gated server name).

Lab-Run: LR-20261111-017
fix(forgejo): an incomplete evidence read refuses on GO-gated repos (LR-20261111-017, Amendment 16, review B8, homelab-93 (3))
All checks were successful
CI / pr-title (pull_request) Successful in 2s
Details
CI / pi-otel-telemetry (pull_request) Successful in 50s
Details
CI / ask-user-question (pull_request) Successful in 50s
Details
CI / refocus (pull_request) Successful in 56s
Details
CI / skill-args (pull_request) Successful in 52s
Details
tests / python (pull_request) Successful in 8s
Details
CI / forgejo-ci (pull_request) Successful in 1m10s
Details
CI / mcp-bridge (pull_request) Successful in 26s
Details
CI / nats-intercom (pull_request) Successful in 28s
Details
CI / plane (pull_request) Successful in 1m12s
Details
CI / todo (pull_request) Successful in 1m23s
Details
CI / redact (pull_request) Successful in 54s
Details
CI / pi-friction (pull_request) Successful in 57s
Details
CI / forgejo (pull_request) Successful in 2m13s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m2s
Details
c4e93bbf20 
Review-88-pr125-r4 B8 measured the fail-open shape: a hold can be posted as any
comment, any review body or any inline note, and every evidence read had a cap
that said nothing when it stopped — so a hold behind filler (comment 301, the
51st review, an inline comment whose read 5xx'd) was absent from the ledger
while an earlier exact-head approval was, and the merge went out with the hold
standing.

On a GO-gated repo (this PR's server-name gate) that is now a REFUSAL:
- general comments: the 6x50 page cap throws, naming the cap, instead of
  returning a truncated record;
- reviews: paged to the end of the list under a hard refusing bound (a full
  last page at the bound refuses) — the 51st review body is READ, not missed;
- inline comments: enumerated through the COMPLETE review list the lock already
  read (a second capped review read would re-introduce the bound), each
  per-review list paged to its end under a refusing bound, and a read error
  refuses instead of being swallowed (B8's 502);
- changed files: the dangerous-class input had the same silent cap (ten full
  pages), so a `.tf` or `deploy/**` change past it under-reported the class and
  dropped the panel rule with it — it now refuses.

Non-gated repos are unchanged, and each case is asserted both ways so the split
cannot widen by accident. The gated list is NOT duplicated here: `api.ts` uses
`isGoGatedRepo` from `landing.ts`, so there is one owner for the set (#125
redefines `GO_GATED_REPOS`/`repoKey` in that same file with the same values and
semantics — a three-way merge keeps one copy).

Tests (5 red on the previous commit, 6 non-gated controls green by design):
`a hold as comment 301 … REFUSES on a gated repo, naming the cap, and sends no
merge`, `a hold in the 51st review body … SEES the hold and refuses`, `an inline
read that answers 502 … REFUSES`, `every pagination path refuses at its bound …
one review's inline comments` and `… the changed-file list`.

Lab-Run: LR-20261111-017
Merges origin/lr-20261111-017-lockharden (Amendments 15/16/18) into the census
gate branch. A merge COMMIT, not a rebase (RULE-5).

Resolved per #131's r5 notes:
  * ONE owner for GO_GATED_REPOS / isGoGatedRepo: the lock-hardening branch's
    (its repoKey DECODES, so it is a superset of the census branch's
    trim+lowercase comparison); the census branch's duplicate is deleted and the
    census legs read the surviving constant through the same predicate.
  * resolveLandingPolicy's second parameter widens to `string | ServerRepoView`
    with the lock-hardening branch's body (per-knob logging, repoKey decoding);
    the census branch's gatedReviewerPolicy stays as the lock-side pin, so BOTH
    pins are present and the intent is identical: defaults on a gated repo, the
    env everywhere else.
  * listAllPRComments / listPRFiles take `{ maxPages }` options: the census
    call sites adapt (`{ complete: gated }` composes with the options object).

Both sides' tests are green after the merge. The census branch's N6/N15/N4
fixtures gained the GO record and `updatedAt` the GO leg now requires before a
later leg can be reached, and the merge gate + census de-duplicate the ignored-
knob policy lines through one sink per action, so an ignored knob is still said
ONCE per operator action (review-88-pr130 n3) now that the policy is resolved
twice (seat plan, then lock).

Suite: 61 files / 1379 pass, 1 known pre-existing merge-identity ambient-token
failure (identical on base). tsc --noEmit clean.
HOMELAB-1679: fix(forgejo): the r5 census round — a pod fact for the lookup address, a space-bearing trailer judged (LR-20261111-017, review-88-pr125-r4 n17–n23)
Some checks failed
CI / pr-title (pull_request) Successful in 1s
Details
CI / pi-otel-telemetry (pull_request) Has been cancelled
Details
CI / pi-swarm-suite (pull_request) Has been cancelled
Details
CI / forgejo-ci (pull_request) Has been cancelled
Details
CI / plane (pull_request) Has been cancelled
Details
CI / skill-args (pull_request) Has been cancelled
Details
CI / forgejo (pull_request) Has been cancelled
Details
CI / todo (pull_request) Has been cancelled
Details
CI / mcp-bridge (pull_request) Has been cancelled
Details
CI / ask-user-question (pull_request) Has been cancelled
Details
CI / nats-intercom (pull_request) Has been cancelled
Details
CI / refocus (pull_request) Has been cancelled
Details
CI / pi-friction (pull_request) Has been cancelled
Details
CI / redact (pull_request) Has been cancelled
Details
tests / python (pull_request) Successful in 13s
Details
1c91de06c3 
ONE r5 commit on top of the #131 merge. No landing follows from this work
(AMENDMENTS 7/10): code and tests only.

R5-1 / n17 — AMENDMENT 17. The seat-lookup address is no longer chosen from
`KUBERNETES_SERVICE_HOST`: an environment value is not a pod fact, and
`env -u KUBERNETES_SERVICE_HOST` in a sandbox used to send the lookup to
`127.0.0.1:8097` inside the pod, where any unprivileged process can bind a stub
answering `rows: 12, distinct: true`. In-cluster now means the kubelet's mount:
`/var/run/secrets/kubernetes.io/serviceaccount/namespace` exists, its content
trims non-empty, and `ca.crt` sits beside it — anything else is loopback.
`seatLookupInCluster()` is the decision; nothing in it reads the environment.
The honest limit is corrected in the comments and in docs/forgejo.md: what IS
closed (a sandbox cannot talk the gate out of the cluster address, and one that
muted both reaches only its OWN loopback, not the runtime's) and what is NOT (a
root container can edit its own mount — the platform limit, with the GO leg and
El Director's token as the outer boundary).
Old-code-failing tests: valid mount ⇒ cluster URL; absent / empty / whitespace
namespace / no ca.crt / no directory ⇒ loopback; `KUBERNETES_SERVICE_HOST` and
`FORGEJO_SEAT_LOOKUP_URL` inert in BOTH directions; the dial follows the choice.

R5-2 / n18. The trailer value is captured to the END OF THE LINE and trimmed
BEFORE the grammar check. The old `(\S+)` with "line must end here" meant a
value carrying a SPACE matched no trailer at all — on its own it refused with
the no-trailer sentence, and after an earlier valid trailer it was silently
skipped and the earlier value won (the reviewer's `ok=true` probe). Now
`Lab-Run: foo bar` is a trailer whose value is `foo bar`, refused by the
grammar with its own sentence. The trim cuts one way: a trailing space or a CR
is whitespace, not part of the value, so `Lab-Run: sendrast-v0.7.2-chain `
stays a valid run id (refusing a cosmetic space would block a landing for a
reason no grammar names). An empty trailer is still an absence.
Old-code-failing tests: `foo bar` ⇒ grammar sentence (not the no-trailer one);
a later space-bearing trailer is not outvoted by an earlier valid one.

R5-3 / n19. The N4 mappings on the REVIEW and the INLINE rows now have caller
tests (only the comment mapping had one): a review row with no `created_at` on a
gated repo is refused naming `the review record #N`; an inline row likewise;
with `created_at` present both are unchanged and the merge goes through.
Old-code-failing: both refusals fail when the mapping's flag is removed.

R5-4 / n20. (e)'s echo shape is pinned on this side too: an answer that echoes
the full gateway tag instead of the BARE requested sid is unreadable, and the
`start`/`end` half of the window check is no longer dead code (the mapping
copies those fields through). infra-core#941's
`TestRecordedLiveTagCountsTheSeatsRows` remains the endpoint-side anchor
(`a.Sid == the sid that was asked`); no infra-core change is needed here.

R5-5 / n22. Full suite on the merged result: 61 files, 1390 pass, 1 known
pre-existing merge-identity ambient-token failure — reproduced as an artefact of
this sandbox's ambient `FORGEJO_LANDING_TOKEN_FILE` (the file passes 25/25 with
that variable unset) and identical on base. `tsc --noEmit` clean.

Judged open in this round (n16, n21, n23 — named, not fixed): see the run
report. AMENDMENT 14 (provenance) is out of r5 per AMENDMENT 17.

Lab-Run: LR-20261111-017
docs(forgejo): the mgmt loopback residual named in the honest limit (LR-20261111-017, review-88 r5 doc nit)
All checks were successful
CI / pr-title (pull_request) Successful in 4s
Details
CI / forgejo-ci (pull_request) Successful in 48s
Details
CI / todo (pull_request) Successful in 47s
Details
CI / ask-user-question (pull_request) Successful in 48s
Details
CI / mcp-bridge (pull_request) Successful in 31s
Details
CI / nats-intercom (pull_request) Successful in 33s
Details
CI / skill-args (pull_request) Successful in 53s
Details
tests / python (pull_request) Successful in 12s
Details
CI / refocus (pull_request) Successful in 1m6s
Details
CI / pi-friction (pull_request) Successful in 53s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m23s
Details
CI / redact (pull_request) Successful in 59s
Details
CI / plane (pull_request) Successful in 1m32s
Details
CI / forgejo (pull_request) Successful in 1m59s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m0s
Details
5d6385c5b6 
Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 18:33:47 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=5d6385c5b66e5338c2be6bcba02bc4ba2b8d75ce session=LR-20261111-017-seatA-20261111T1831Z

Independent review of the full base..head diff (base 1800e684, head 5d6385c), with the r5 delta (merge 86c3148, commit 1c91de0) judged separately.

r5 delta — checked in source, not from the description:

  1. n17, address choice. seatLookupInCluster (forgejo/index.ts:369) reads only ca.crt, namespace (exists) and the trimmed namespace content; seatLookupBaseUrl (forgejo/index.ts:391) selects between two source constants (index.ts:341-342) with no env read anywhere in the choice. Grep for KUBERNETES_SERVICE_HOST/FORGEJO_SEAT_LOOKUP_URL in forgejo/*.ts finds them only in comments and in the tests that assert they change nothing (landing-gated-caller.test.ts:186, :196, :509-528). The old code at 86c3148 did read env.KUBERNETES_SERVICE_HOST (index.ts:325 there), so these tests fail on the old code. Both ways covered: pod shape ⇒ cluster URL (:486-487), missing/empty/whitespace namespace and missing ca.crt ⇒ mgmt (:491-499), unreadable dir ⇒ mgmt. The honest-limit text (index.ts:322-338) states the mgmt loopback residual correctly: muting both yields the pod's own loopback, not the runtime's, and the GO leg plus the director token remain the outer boundary. No tool parameter or env can move the production path.
  2. n18, space-bearing trailer. lastLabRunTrailer now captures to end of line and trims before the grammar (forgejo/landing.ts:4931), so Lab-Run: foo bar is a trailer whose value is refused by the run-id grammar with the grammar sentence, never the no-trailer one, and cannot be skipped in favour of an earlier valid trailer (:4935-4941). A trailing space / CRLF stays a valid run id, and sendrast-v0.7.2-chain is unchanged. Both branches tested (landing-seat-verification.test.ts:1021, :1030, :1046).
  3. n19, N4 on the other two mappings. Review and inline rows that cannot yield created_at refuse on a gated repo through the same one-line message (landing.ts:6104), with tests at landing-gated-caller.test.ts:284 and :331, plus the paired negatives at :309 and :353.
  4. Merge 86c3148 kept ONE owner for the gated symbols: GO_GATED_REPOS is declared once (landing.ts:3007) and isGoGatedRepo once (:3050); no duplicate declaration survives in forgejo/*.ts. Both sides' pins are present: the widened resolveLandingPolicy (:3184) and the lock-side gatedReviewerPolicy (:3253), the latter used as the default at :5953. Resolution is a merge commit, not a rebase.

Stack sanity: the GO leg (identity + record regex at landing.ts:4708/:4721, server-name keying at :3070/:3094 with the fail-closed refusal at :3122), the seat leg (verified sids, two distinct seats, pairwise independence) and the lock-hardening legs (percent-escape refusal in parseRepoRef, incomplete-evidence refusal) are each present exactly once and do not overlap.

Checks run in this seat's own clone at the pinned head: vitest run = 61 files, 1390 passed, 1 failed. The one failure is tests/merge-identity.test.ts (ambient landing token), which I reproduced on the untouched base 1800e684 in a separate worktree — pre-existing, identical, not a hold. The two r5 target files pass on their own (86/86). Nothing outside this PR's diff was modified.

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1831Z

LD-VERDICT APPROVED head=5d6385c5b66e5338c2be6bcba02bc4ba2b8d75ce session=LR-20261111-017-seatA-20261111T1831Z Independent review of the full base..head diff (base 1800e684, head 5d6385c), with the r5 delta (merge 86c3148, commit 1c91de0) judged separately. r5 delta — checked in source, not from the description: 1. n17, address choice. `seatLookupInCluster` (forgejo/index.ts:369) reads only `ca.crt`, `namespace` (exists) and the trimmed `namespace` content; `seatLookupBaseUrl` (forgejo/index.ts:391) selects between two source constants (index.ts:341-342) with no env read anywhere in the choice. Grep for `KUBERNETES_SERVICE_HOST`/`FORGEJO_SEAT_LOOKUP_URL` in `forgejo/*.ts` finds them only in comments and in the tests that assert they change nothing (landing-gated-caller.test.ts:186, :196, :509-528). The old code at 86c3148 did read `env.KUBERNETES_SERVICE_HOST` (index.ts:325 there), so these tests fail on the old code. Both ways covered: pod shape ⇒ cluster URL (:486-487), missing/empty/whitespace namespace and missing ca.crt ⇒ mgmt (:491-499), unreadable dir ⇒ mgmt. The honest-limit text (index.ts:322-338) states the mgmt loopback residual correctly: muting both yields the pod's own loopback, not the runtime's, and the GO leg plus the director token remain the outer boundary. No tool parameter or env can move the production path. 2. n18, space-bearing trailer. `lastLabRunTrailer` now captures to end of line and trims before the grammar (forgejo/landing.ts:4931), so `Lab-Run: foo bar` is a trailer whose value is refused by the run-id grammar with the grammar sentence, never the no-trailer one, and cannot be skipped in favour of an earlier valid trailer (:4935-4941). A trailing space / CRLF stays a valid run id, and `sendrast-v0.7.2-chain` is unchanged. Both branches tested (landing-seat-verification.test.ts:1021, :1030, :1046). 3. n19, N4 on the other two mappings. Review and inline rows that cannot yield `created_at` refuse on a gated repo through the same one-line message (landing.ts:6104), with tests at landing-gated-caller.test.ts:284 and :331, plus the paired negatives at :309 and :353. 4. Merge 86c3148 kept ONE owner for the gated symbols: `GO_GATED_REPOS` is declared once (landing.ts:3007) and `isGoGatedRepo` once (:3050); no duplicate declaration survives in `forgejo/*.ts`. Both sides' pins are present: the widened `resolveLandingPolicy` (:3184) and the lock-side `gatedReviewerPolicy` (:3253), the latter used as the default at :5953. Resolution is a merge commit, not a rebase. Stack sanity: the GO leg (identity + record regex at landing.ts:4708/:4721, server-name keying at :3070/:3094 with the fail-closed refusal at :3122), the seat leg (verified sids, two distinct seats, pairwise independence) and the lock-hardening legs (percent-escape refusal in `parseRepoRef`, incomplete-evidence refusal) are each present exactly once and do not overlap. Checks run in this seat's own clone at the pinned head: `vitest run` = 61 files, 1390 passed, 1 failed. The one failure is `tests/merge-identity.test.ts` (ambient landing token), which I reproduced on the untouched base 1800e684 in a separate worktree — pre-existing, identical, not a hold. The two r5 target files pass on their own (86/86). Nothing outside this PR's diff was modified. Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1831Z
ld-reviewer commented 2026-11-11 18:34:10 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=5d6385c5b66e5338c2be6bcba02bc4ba2b8d75ce session=LR-20261111-017-seatB-20261111T1831Z

Read-only clone of Homelab/pi-extensions, branch lr-20261111-017-census; the branch head IS the pinned head 5d6385c5b6, so the verdict is on the exact stack (census gate + #131 lock-hardening merge 86c3148 + r5 1c91de0 + doc 5d6385c).

r5 delta judged independently:

  • n17 (address choice): seatLookupInCluster()/seatLookupBaseUrl() (forgejo/index.ts:352-380) read only the SA mount — namespace exists, trims non-empty, ca.crt beside it — and nothing in the decision reads the environment. A repo-wide grep finds no read of KUBERNETES_SERVICE_HOST or FORGEJO_SEAT_LOOKUP_URL in production code; the only occurrences are in the test that asserts both are inert in BOTH directions (valid mount stays cluster with the var unset/empty; empty mount stays loopback with the var set to an in-cluster value), plus the dial tests (cluster DNS vs loopback origin). Absent dir, missing/empty/whitespace namespace, and missing ca.crt all fall to loopback. The honest limit names the residual explicitly: a root container can edit its own mount, and on mgmt a local user can bind 127.0.0.1:8097 while the port-forward unit is down (docs/forgejo.md, commit 5d6385c) — the mgmt loopback residual is stated, not papered over.
  • n18 (space-bearing trailer): the capture is now everything to end-of-line, trimmed before the grammar (forgejo/landing.ts:4931-4939). Lab-Run: foo bar reaches the GRAMMAR sentence (not the no-trailer one), a later space-bearing trailer is no longer outvoted by an earlier valid one, a trailing space/CR stays a valid run id, and a whitespace-only value stays an absence that does not erase an earlier trailer. Mutation-checked: restoring the old (\S+)[ \t]*$ form turns 2 of those tests red.
  • n19 (N4 coverage): the REVIEW and INLINE mappings now have caller tests (tests/landing-gated-caller.test.ts:70-160), refusing with the review record #N / the inline record #N and leaving rows with created_at intact. Mutation-checked: pinning atFromFallback: false at both mappings (forgejo/index.ts:816,837) turns exactly those two tests red; file restored, tree clean.
  • n20 (bare-sid echo): seatAnswerEchoesRequest() compares the echoed sid literally against the BARE requested sid, other too, and the start/end window halves are live because the mapping now copies those fields through (index.ts:429-433, 464-482); an echoed full gateway tag is unreadable. Tests pin both.
  • Merge kept ONE owner of the gated symbols: a single GO_GATED_REPOS and isGoGatedRepo (landing.ts:3007,3050), a single goGatedFromServerName/ServerRepoView for the server-name rule, and the reviewer policy flows through one gatedReviewerPolicy call site in evaluateLandingLock.

Stack sanity: the GO leg (2a time readability, 2b record), the seat leg (5a-5g), and the #131 lock-hardening legs (server-repo name, fail-closed evidence read, comments-only withdrawal) are all present and non-overlapping — each is inside isGoGatedRepo and returns before touching non-gated paths.

Suite on the exact head: 61 files, 1390 passed, 1 failed = tests/merge-identity.test.ts, the pre-existing ambient landing-token failure on the untouched base (not a hold).

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1831Z

LD-VERDICT APPROVED head=5d6385c5b66e5338c2be6bcba02bc4ba2b8d75ce session=LR-20261111-017-seatB-20261111T1831Z Read-only clone of Homelab/pi-extensions, branch lr-20261111-017-census; the branch head IS the pinned head 5d6385c5b66e5338c2be6bcba02bc4ba2b8d75ce, so the verdict is on the exact stack (census gate + #131 lock-hardening merge 86c3148 + r5 1c91de0 + doc 5d6385c). r5 delta judged independently: - n17 (address choice): `seatLookupInCluster()`/`seatLookupBaseUrl()` (forgejo/index.ts:352-380) read only the SA mount — `namespace` exists, trims non-empty, `ca.crt` beside it — and nothing in the decision reads the environment. A repo-wide grep finds no read of `KUBERNETES_SERVICE_HOST` or `FORGEJO_SEAT_LOOKUP_URL` in production code; the only occurrences are in the test that asserts both are inert in BOTH directions (valid mount stays cluster with the var unset/empty; empty mount stays loopback with the var set to an in-cluster value), plus the dial tests (cluster DNS vs loopback origin). Absent dir, missing/empty/whitespace `namespace`, and missing `ca.crt` all fall to loopback. The honest limit names the residual explicitly: a root container can edit its own mount, and on mgmt a local user can bind 127.0.0.1:8097 while the port-forward unit is down (docs/forgejo.md, commit 5d6385c) — the mgmt loopback residual is stated, not papered over. - n18 (space-bearing trailer): the capture is now everything to end-of-line, trimmed before the grammar (forgejo/landing.ts:4931-4939). `Lab-Run: foo bar` reaches the GRAMMAR sentence (not the no-trailer one), a later space-bearing trailer is no longer outvoted by an earlier valid one, a trailing space/CR stays a valid run id, and a whitespace-only value stays an absence that does not erase an earlier trailer. Mutation-checked: restoring the old `(\S+)[ \t]*$` form turns 2 of those tests red. - n19 (N4 coverage): the REVIEW and INLINE mappings now have caller tests (tests/landing-gated-caller.test.ts:70-160), refusing with `the review record #N` / `the inline record #N` and leaving rows with `created_at` intact. Mutation-checked: pinning `atFromFallback: false` at both mappings (forgejo/index.ts:816,837) turns exactly those two tests red; file restored, tree clean. - n20 (bare-sid echo): `seatAnswerEchoesRequest()` compares the echoed `sid` literally against the BARE requested sid, `other` too, and the `start`/`end` window halves are live because the mapping now copies those fields through (index.ts:429-433, 464-482); an echoed full gateway tag is unreadable. Tests pin both. - Merge kept ONE owner of the gated symbols: a single `GO_GATED_REPOS` and `isGoGatedRepo` (landing.ts:3007,3050), a single `goGatedFromServerName`/`ServerRepoView` for the server-name rule, and the reviewer policy flows through one `gatedReviewerPolicy` call site in `evaluateLandingLock`. Stack sanity: the GO leg (2a time readability, 2b record), the seat leg (5a-5g), and the #131 lock-hardening legs (server-repo name, fail-closed evidence read, comments-only withdrawal) are all present and non-overlapping — each is inside `isGoGatedRepo` and returns before touching non-gated paths. Suite on the exact head: 61 files, 1390 passed, 1 failed = tests/merge-identity.test.ts, the pre-existing ambient landing-token failure on the untouched base (not a hold). Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1831Z
merge: main (8c666fbe, the landed lock-hardening PR #131) into the census gate r5 (LR-20261111-017)
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 50s
Details
CI / plane (pull_request) Successful in 1m7s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m30s
Details
CI / skill-args (pull_request) Successful in 45s
Details
CI / mcp-bridge (pull_request) Successful in 39s
Details
CI / nats-intercom (pull_request) Successful in 32s
Details
tests / python (pull_request) Successful in 8s
Details
CI / ask-user-question (pull_request) Successful in 53s
Details
CI / refocus (pull_request) Successful in 1m4s
Details
CI / pi-friction (pull_request) Successful in 43s
Details
CI / todo (pull_request) Successful in 1m10s
Details
CI / redact (pull_request) Successful in 35s
Details
CI / forgejo (pull_request) Successful in 1m48s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m30s
Details
1a7d450d30 
A merge COMMIT, not a rebase (RULE-5). #125's tree already carried #131's BRANCH
content via 86c3148, so the textual merge was clean; the resolutions below are
the semantic ones the two sides still needed.

  * #131's semantics win wherever they overlap: every gated decision keys off
    the SERVER's `pr.base.repo.full_name`. `resolveLandingPolicy` keeps #131's
    body and its `string | ServerRepoView` seam; the requested name is used only
    to REFUSE a disagreement.
  * #125's legs (GO record, seat verification, n17 SA-mount address choice, n18
    trailer trim, n19 mappings, N14 comments-only withdrawal, N15 pins, B6
    edited-seat refusal) are kept and re-expressed on #131's helpers: the lock
    now takes the decision (`serverRepoView`) once, in `gatedFromRepoArg`, and
    every gated leg reads that one `gated` value instead of re-deriving it from
    a string. The verdict helper's preview passes the view too — it was the one
    caller handing the lock the `--repo` a session typed.
  * No duplicate symbols: `GO_GATED_REPOS`, `isGoGatedRepo` and `repoKey` exist
    once, owned by #131's `landing.ts`.
  * Fixtures, not assertions, where the merge met: #131's B8 paging cases and
    the census-policy case on `Homelab/infra-core` now carry the GO record (and,
    where the case lands, two verified seats and the run trailer) that #125's GO
    leg requires before the leg under test is reached. Every #131 refusal is
    intact: the six knob pins, the percent-escape refusals, the B1/B2/B8
    evidence reads and the census-policy test all pass unchanged.

Tests: 1400 passed, 1 failed (the known pre-existing ambient-token
merge-identity case); `tsc --noEmit` clean.

Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 18:57:18 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=1a7d450d301195082b1d62e7a7f6db05e4fa10eb session=LR-20261111-017-seatB-20261111T1853Z

Head pinned: the PR's current head reads 1a7d450d30 — identical to the pin, no divergence.

What I checked (fresh read-only clone, branch lr-20261111-017-census, diff 8c666fbe..1a7d450, forgejo/**):

  1. GO leg (forgejo/landing.ts): GO_AUTHOR_IDENTITY is the named ld-director-go account; GO_RECORD_RE is fully anchored with a 40-hex head and uppercase normalised (short sha never parsed); parseGoRecord forgives only surrounding whitespace. Leg 2b runs before the ledger, on the gated branch only, and refuses four ways: comments empty (read failure), any comment without an update time, no record for this repo/PR/head, and every matching record edited (updatedAt != at checked on matching rows only, newest unedited wins). Gating reads the server-side identity (ServerRepoView from pr.base.repo.full_name, gatedFromRepoArg believing gating only), not the caller's string.
  2. Seat leg: every counted trusted exact-head approval's declared sid is verified — own-answer stream (rows > 0, served pair from the reviewed exact-pair table, MODEL line measured against the table's declared id in bare or provider-qualified spelling with SERVED trusted over the line), verdict placement inside its own first..last stream plus slack, caller-supplied correlation ids refused, edited verdict refused, absent answer / absent window refused. Independence is evaluated PAIRWISE for every unordered pair of declared seats, worst fact wins per pair, a failed pair demotes both members, and a pair never compared is a refusal (not "nothing shared"). Verification count is verifiedSids.size vs requiredSeats = 2 on a GO-gated repo for routine changes too (the generic dangerous?2:1 rule is unreachable on that branch); same sid twice is one seat. Endpoint unreadable/absent is refused in the lock's own words; the address is chosen from the service-account mount (in-cluster DNS vs mgmt loopback), no env var participates, answers are echo-checked against sid and requested window.
  3. Caller wiring (forgejo/index.ts): repo identity resolved once from the server name, plan derived from the same ledger the lock builds (gated => comments only), seat lookups only on gated repos, head message read in one pass with its date, evidence carried to both the merge gate and the census, and the same ServerRepoView opens the policy pin and both legs.
  4. Tests: /home/ld/work/pi-agent/extensions/pi-swarm-suite/node_modules/.bin/vitest run → 1400 passed, 1 failed, the failure being tests/merge-identity.test.ts (ambient landing token), which also fails on the untouched base — pre-existing, not a hold. Named spec cases are present and non-vacuous: I copied the tree to a scratch dir and applied five mutations — accept an edited GO, accept any GO author, widen the head to 7..40 hex, drop to the generic seat count, restrict independence to the two-seat shape — and every one produced red tests (16 failures across the GO, seat, pairwise and caller files, including the three-seat pairwise and routine one-seat cases).

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1853Z

LD-VERDICT APPROVED head=1a7d450d301195082b1d62e7a7f6db05e4fa10eb session=LR-20261111-017-seatB-20261111T1853Z Head pinned: the PR's current head reads 1a7d450d301195082b1d62e7a7f6db05e4fa10eb — identical to the pin, no divergence. What I checked (fresh read-only clone, branch lr-20261111-017-census, diff 8c666fbe..1a7d450, forgejo/**): 1. GO leg (forgejo/landing.ts): GO_AUTHOR_IDENTITY is the named ld-director-go account; GO_RECORD_RE is fully anchored with a 40-hex head and uppercase normalised (short sha never parsed); parseGoRecord forgives only surrounding whitespace. Leg 2b runs before the ledger, on the gated branch only, and refuses four ways: comments empty (read failure), any comment without an update time, no record for this repo/PR/head, and every matching record edited (updatedAt != at checked on matching rows only, newest unedited wins). Gating reads the server-side identity (ServerRepoView from pr.base.repo.full_name, gatedFromRepoArg believing gating only), not the caller's string. 2. Seat leg: every counted trusted exact-head approval's declared sid is verified — own-answer stream (rows > 0, served pair from the reviewed exact-pair table, MODEL line measured against the table's declared id in bare or provider-qualified spelling with SERVED trusted over the line), verdict placement inside its own first..last stream plus slack, caller-supplied correlation ids refused, edited verdict refused, absent answer / absent window refused. Independence is evaluated PAIRWISE for every unordered pair of declared seats, worst fact wins per pair, a failed pair demotes both members, and a pair never compared is a refusal (not "nothing shared"). Verification count is verifiedSids.size vs requiredSeats = 2 on a GO-gated repo for routine changes too (the generic dangerous?2:1 rule is unreachable on that branch); same sid twice is one seat. Endpoint unreadable/absent is refused in the lock's own words; the address is chosen from the service-account mount (in-cluster DNS vs mgmt loopback), no env var participates, answers are echo-checked against sid and requested window. 3. Caller wiring (forgejo/index.ts): repo identity resolved once from the server name, plan derived from the same ledger the lock builds (gated => comments only), seat lookups only on gated repos, head message read in one pass with its date, evidence carried to both the merge gate and the census, and the same ServerRepoView opens the policy pin and both legs. 4. Tests: /home/ld/work/pi-agent/extensions/pi-swarm-suite/node_modules/.bin/vitest run → 1400 passed, 1 failed, the failure being tests/merge-identity.test.ts (ambient landing token), which also fails on the untouched base — pre-existing, not a hold. Named spec cases are present and non-vacuous: I copied the tree to a scratch dir and applied five mutations — accept an edited GO, accept any GO author, widen the head to 7..40 hex, drop to the generic seat count, restrict independence to the two-seat shape — and every one produced red tests (16 failures across the GO, seat, pairwise and caller files, including the three-seat pairwise and routine one-seat cases). Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1853Z
ld-reviewer commented 2026-11-11 19:02:23 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=1a7d450d301195082b1d62e7a7f6db05e4fa10eb session=LR-20261111-017-seatA-20261111T1853Z

Head verified against the pin: the PR's current head IS 1a7d450d30 (base 8c666fbe, the landed #131 merge). Fresh read-only clone of the branch, diff reviewed base..head (22 files, +4984/-114; landing.ts +1322, index.ts +483, and the fixture suites).

WHAT I CHECKED

  • GO leg (landing.ts:746-830 goRecordLeg, :216-243 the grammar/parse): anchored ^LD-GO repo=<owner/name> pr=<n> head=<40 hex>$, author compared case-insensitively against the fixed ld-director-go identity, FULL 40-hex only (short sha never parses, uppercase normalised), repo/PR/head all compared on the field values, and four fail-closed refusals: no comments readable, any comment with no updated_at (edit state unreadable), no record naming this PR+head, and every matching record edited (updatedAt !== at). Newest unedited matching record wins. The gated decision is the forge's own name (pr.base.repo.full_name, index.ts:707-733, serverRepoView → one gated in the lock at :6011-6015), so a percent-encoded or redirecting caller string cannot switch the leg off; the leg is inside if (gated) so non-gated repos are byte-identical.
  • Seat leg (landing.ts:6680-7080, countedSeats :5155-5190, plan :5219-5280): comments-only on gated repos, sid taken from the trusted exact-head approval panel (unreduced by reviewer key, then deduped by sid, so one session used twice is ONE seat); tag must be the literal ^<runId>-seat(A|B)-<YYYYMMDD>T<HHMM>Z$ built from the head commit's LAST Lab-Run: trailer (trimmed, then judged by the one run-id grammar ^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$, with its own refusal sentence); rows>0, an answer must exist for each sid, every served pair must be in the reviewed exact-pair table with SERVED trusted over the MODEL line (bare or provider-qualified spelling only, no prefix/family/case folding), verdict placed inside its own sid's stream (first..last + 5 min slack, unreadable window refused), caller-supplied correlation ids refused, edited or unreadable-edit verdict refused, endpoint unreadable/absent refused in the lock's own words. Two required seats on gated repos regardless of class (:6780, :7025-7037 — routine included, dangerous classification only words the sentence). Independence is PAIRWISE: one request per unordered pair (:5234-5278), every pair read with worst-fact-wins, a pair with no answer refused, and each refused pair demotes BOTH of its seats (:6871-6935) — a 3+ seat panel of correlated seats cannot pass.
  • Caller (index.ts:875-960, 1570-1640): plan built from the same ledger with the server view, all-or-nothing read (readSeatLookup → one unreadable), address chosen from the service-account mount in source with no env participation (:163-186), echo check (sid + window + other) before an answer counts, created_at requirement signalled (atFromFallback) and refused by the lock, merge/census/preview all pass the same server view. Preview on a gated repo explicitly passes unreadable and refuses rather than promising a landing (verdict.ts:782).
  • Tests: ran the full suite at this head (vitest run, 62 files): 1400 passed, 1 failed — tests/merge-identity.test.ts (ambient landing token). I reproduced that same failure on the untouched origin/main worktree, so it is pre-existing and not a hold. tsc --noEmit exit 0. Non-vacuity spot-checked by mutation on my own copy: removing the GO edit-equality filter fails 2 GO tests; restoring cls.dangerous ? 2 : 1 fails 4 seat tests (including the routine one-seat and the dangerous one-approval cases); restricting the independence loop to the two-seat shape fails 2 three-seat tests. The spec's named cases are present and assert on refusal wording, the recorded seat facts (sid/rows/served pair/declared model/verified) and the permit note, not just ok.

HOLDS: none.

Two non-blocking observations for whoever touches this next: (1) landing.ts:6780 keeps a dead non-gated arm (gated ? 2 : cls.dangerous ? 2 : 1) inside an if (gated) block — harmless, reads like a live rule; (2) the comment at landing.ts:7021 says a CONCERN counts as a panel member, while countedSeats counts approvals only, so the seat count is strictly stricter than that sentence (fail-closed, and the routine/dangerous one-seat refusals depend on it).

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1853Z

LD-VERDICT APPROVED head=1a7d450d301195082b1d62e7a7f6db05e4fa10eb session=LR-20261111-017-seatA-20261111T1853Z Head verified against the pin: the PR's current head IS 1a7d450d301195082b1d62e7a7f6db05e4fa10eb (base 8c666fbe, the landed #131 merge). Fresh read-only clone of the branch, diff reviewed base..head (22 files, +4984/-114; landing.ts +1322, index.ts +483, and the fixture suites). WHAT I CHECKED - GO leg (landing.ts:746-830 goRecordLeg, :216-243 the grammar/parse): anchored `^LD-GO repo=<owner/name> pr=<n> head=<40 hex>$`, author compared case-insensitively against the fixed `ld-director-go` identity, FULL 40-hex only (short sha never parses, uppercase normalised), repo/PR/head all compared on the field values, and four fail-closed refusals: no comments readable, any comment with no `updated_at` (edit state unreadable), no record naming this PR+head, and every matching record edited (`updatedAt !== at`). Newest unedited matching record wins. The gated decision is the forge's own name (`pr.base.repo.full_name`, index.ts:707-733, `serverRepoView` → one `gated` in the lock at :6011-6015), so a percent-encoded or redirecting caller string cannot switch the leg off; the leg is inside `if (gated)` so non-gated repos are byte-identical. - Seat leg (landing.ts:6680-7080, countedSeats :5155-5190, plan :5219-5280): comments-only on gated repos, sid taken from the trusted exact-head approval panel (unreduced by reviewer key, then deduped by sid, so one session used twice is ONE seat); tag must be the literal `^<runId>-seat(A|B)-<YYYYMMDD>T<HHMM>Z$` built from the head commit's LAST `Lab-Run:` trailer (trimmed, then judged by the one run-id grammar `^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$`, with its own refusal sentence); rows>0, an answer must exist for each sid, every served pair must be in the reviewed exact-pair table with SERVED trusted over the MODEL line (bare or provider-qualified spelling only, no prefix/family/case folding), verdict placed inside its own sid's stream (first..last + 5 min slack, unreadable window refused), caller-supplied correlation ids refused, edited or unreadable-edit verdict refused, endpoint unreadable/absent refused in the lock's own words. Two required seats on gated repos regardless of class (:6780, :7025-7037 — routine included, dangerous classification only words the sentence). Independence is PAIRWISE: one request per unordered pair (:5234-5278), every pair read with worst-fact-wins, a pair with no answer refused, and each refused pair demotes BOTH of its seats (:6871-6935) — a 3+ seat panel of correlated seats cannot pass. - Caller (index.ts:875-960, 1570-1640): plan built from the same ledger with the server view, all-or-nothing read (`readSeatLookup` → one `unreadable`), address chosen from the service-account mount in source with no env participation (:163-186), echo check (sid + window + other) before an answer counts, `created_at` requirement signalled (`atFromFallback`) and refused by the lock, merge/census/preview all pass the same server view. Preview on a gated repo explicitly passes `unreadable` and refuses rather than promising a landing (verdict.ts:782). - Tests: ran the full suite at this head (`vitest run`, 62 files): 1400 passed, 1 failed — `tests/merge-identity.test.ts` (ambient landing token). I reproduced that same failure on the untouched origin/main worktree, so it is pre-existing and not a hold. `tsc --noEmit` exit 0. Non-vacuity spot-checked by mutation on my own copy: removing the GO edit-equality filter fails 2 GO tests; restoring `cls.dangerous ? 2 : 1` fails 4 seat tests (including the routine one-seat and the dangerous one-approval cases); restricting the independence loop to the two-seat shape fails 2 three-seat tests. The spec's named cases are present and assert on refusal wording, the recorded seat facts (sid/rows/served pair/declared model/verified) and the permit note, not just `ok`. HOLDS: none. Two non-blocking observations for whoever touches this next: (1) landing.ts:6780 keeps a dead non-gated arm (`gated ? 2 : cls.dangerous ? 2 : 1`) inside an `if (gated)` block — harmless, reads like a live rule; (2) the comment at landing.ts:7021 says a CONCERN counts as a panel member, while `countedSeats` counts approvals only, so the seat count is strictly stricter than that sentence (fail-closed, and the routine/dangerous one-seat refusals depend on it). model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1853Z
fix(forgejo): refuse the seat-lookup address instead of guessing loopback, and pin the gate swap (LR-20261111-017, review-93 F2, Amendment 20)
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / pi-otel-telemetry (pull_request) Successful in 50s
Details
CI / forgejo-ci (pull_request) Successful in 1m14s
Details
CI / plane (pull_request) Successful in 1m18s
Details
tests / python (pull_request) Successful in 5s
Details
CI / nats-intercom (pull_request) Successful in 30s
Details
CI / mcp-bridge (pull_request) Successful in 31s
Details
CI / ask-user-question (pull_request) Successful in 57s
Details
CI / skill-args (pull_request) Successful in 1m0s
Details
CI / refocus (pull_request) Successful in 1m1s
Details
CI / todo (pull_request) Successful in 1m6s
Details
CI / redact (pull_request) Successful in 55s
Details
CI / pi-friction (pull_request) Successful in 57s
Details
CI / forgejo (pull_request) Successful in 2m18s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m22s
Details
347ea6edf4 
The address decision had one hole left: a pod with no readable service-account
mount was handed 127.0.0.1:8097, so an in-cluster process could bind that port
and answer its own seat lookups. `seatLookupAddress` now decides in three cases:
a valid mount is the cluster url (unchanged); the service-account DIRECTORY
present but the check failed is a REFUSAL, because an in-cluster caller never
falls back to loopback; the directory ABSENT is loopback only where the positive
mgmt marker `/etc/lab-director/seat-lookup-mgmt` (root-owned regular file dropped
by the port-forward unit) exists, and a REFUSAL otherwise. A refused address
reaches the lock exactly like an unreadable endpoint, so the merge refuses in its
own one-line words and posts nothing. The probes are injectable for tests; the
decision never reads the environment.

Also adds the gate-swap mutation test at landing.ts's one `gated` decision: a
non-gated repo is asserted to land WITHOUT the GO/seat legs and a gated one to
refuse WITH them on the same evidence, so forcing the gate either way fails.

Docs: the limit paragraph now names the marker and the refusal rule.

Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 19:30:51 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=347ea6edf4349bb543702fbdec4274b33107e5c6 session=LR-20261111-017-seatA-20261111T1927Z

Reviewed read-only in a fresh clone: full base (8c666fb) .. head 347ea6e diff. The PR's current head is exactly 347ea6edf4 — no drift.

GO leg (forgejo/landing.ts:4726-4870, 5266-5405, called at :6180). Anchored record LD-GO repo=<owner/name> pr=<n> head=<40-hex>; author compared first against ld-director-go, so the right text from any other row is a comment, not a go; a record naming another repo/PR/head is refused and the wrong head is named back; 40-hex only (short sha never accepted, uppercase normalised, not a second format); editable comments refused by updated_at != created_at, and an unreadable edit state (missing update time) is a refusal too; an empty comment read is "could not be read", never "nobody posted a go"; newest unedited matching record wins deterministically.

Seat leg (:6688-7075). On a gated repo only unedited issue COMMENTS are seats; the sid is anchored to the head commit's LAST Lab-Run: trailer under the one run-id grammar, then to the literal ^<run>-seat(A|B)-<stamp>$ tag — free-form tags, other runs' tags and case flips all refuse. Per seat: rows must exist, every served pair must be in the reviewed exact-pair table with the SERVED pair trusted over the MODEL line (bare or provider-qualified spelling of the same reviewed pair only — no prefix, family or case folding), the verdict must sit inside its own sid's first..last stream plus the posting slack, caller-supplied correlation ids refuse the seat, and a missing or unreadable endpoint answer is a refusal in the lock's own words. Independence is pairwise over every N*(N-1)/2 pair, so a 3-seat correlated panel is refused; a pair with no endpoint answer is a refusal, not "nothing shared"; the worst fact wins across both query directions; the refused pair is demoted in the recorded facts. requiredSeats is 2 on a gated repo regardless of class (:6780), so a routine infrastructure-core/prod landing cannot pass on one seat. Non-gated repos take none of these legs.

Caller wiring (forgejo/index.ts): the gated decision comes only from the server's base.repo.full_name, with a percent-encoded/redirecting ref refused rather than read as ungated; the lookup plan is derived from the same ledger the lock judges, so caller and lock ask about the same seats; the seat-lookup address is chosen in code (valid service-account mount ⇒ cluster DNS; mount present but broken ⇒ refuse; non-pod without the root-owned management marker ⇒ refuse), never from an environment value; answers are echo-checked on sid, counterpart and window, and any single failed lookup marks the whole read unreadable. Census and verdict preview pass the same server view.

Tests. vitest run at this head: 63 files, 1407/1408 pass. The single failure is tests/merge-identity.test.ts (ambient landing token), which fails on the untouched base as well — pre-existing and not a hold. tsc --noEmit clean. The named cases exist and are non-vacuous: they assert the refusal sentence and the recorded facts, not just ok === false — routine and dangerous one-seat refusals, same-sid-twice, three-panel pairwise cases (all-shared, one-shared, one-not-distinct, one-pair-never-asked), stream-window boundaries at first and at last+slack, edited and no-update-time seats, model-line mismatches, run-id grammar and trailer precedence, unreadable endpoint, and a gate-swap mutation test that states both directions of the single gated decision on identical evidence.

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T1927Z

LD-VERDICT APPROVED head=347ea6edf4349bb543702fbdec4274b33107e5c6 session=LR-20261111-017-seatA-20261111T1927Z Reviewed read-only in a fresh clone: full base (8c666fb) .. head 347ea6e diff. The PR's current head is exactly 347ea6edf4349bb543702fbdec4274b33107e5c6 — no drift. GO leg (forgejo/landing.ts:4726-4870, 5266-5405, called at :6180). Anchored record `LD-GO repo=<owner/name> pr=<n> head=<40-hex>`; author compared first against ld-director-go, so the right text from any other row is a comment, not a go; a record naming another repo/PR/head is refused and the wrong head is named back; 40-hex only (short sha never accepted, uppercase normalised, not a second format); editable comments refused by `updated_at != created_at`, and an unreadable edit state (missing update time) is a refusal too; an empty comment read is "could not be read", never "nobody posted a go"; newest unedited matching record wins deterministically. Seat leg (:6688-7075). On a gated repo only unedited issue COMMENTS are seats; the sid is anchored to the head commit's LAST `Lab-Run:` trailer under the one run-id grammar, then to the literal `^<run>-seat(A|B)-<stamp>$` tag — free-form tags, other runs' tags and case flips all refuse. Per seat: rows must exist, every served pair must be in the reviewed exact-pair table with the SERVED pair trusted over the MODEL line (bare or provider-qualified spelling of the same reviewed pair only — no prefix, family or case folding), the verdict must sit inside its own sid's first..last stream plus the posting slack, caller-supplied correlation ids refuse the seat, and a missing or unreadable endpoint answer is a refusal in the lock's own words. Independence is pairwise over every N*(N-1)/2 pair, so a 3-seat correlated panel is refused; a pair with no endpoint answer is a refusal, not "nothing shared"; the worst fact wins across both query directions; the refused pair is demoted in the recorded facts. `requiredSeats` is 2 on a gated repo regardless of class (:6780), so a routine infrastructure-core/prod landing cannot pass on one seat. Non-gated repos take none of these legs. Caller wiring (forgejo/index.ts): the gated decision comes only from the server's `base.repo.full_name`, with a percent-encoded/redirecting ref refused rather than read as ungated; the lookup plan is derived from the same ledger the lock judges, so caller and lock ask about the same seats; the seat-lookup address is chosen in code (valid service-account mount ⇒ cluster DNS; mount present but broken ⇒ refuse; non-pod without the root-owned management marker ⇒ refuse), never from an environment value; answers are echo-checked on sid, counterpart and window, and any single failed lookup marks the whole read unreadable. Census and verdict preview pass the same server view. Tests. `vitest run` at this head: 63 files, 1407/1408 pass. The single failure is tests/merge-identity.test.ts (ambient landing token), which fails on the untouched base as well — pre-existing and not a hold. `tsc --noEmit` clean. The named cases exist and are non-vacuous: they assert the refusal sentence and the recorded facts, not just `ok === false` — routine and dangerous one-seat refusals, same-sid-twice, three-panel pairwise cases (all-shared, one-shared, one-not-distinct, one-pair-never-asked), stream-window boundaries at first and at last+slack, edited and no-update-time seats, model-line mismatches, run-id grammar and trailer precedence, unreadable endpoint, and a gate-swap mutation test that states both directions of the single gated decision on identical evidence. Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T1927Z
ld-reviewer commented 2026-11-11 19:31:26 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=347ea6edf4349bb543702fbdec4274b33107e5c6 session=LR-20261111-017-seatB-20261111T1927Z

Fresh read-only clone, git rev-parse HEAD = 347ea6edf4 — the pinned head exactly, no divergence to report. Reviewed the full base..head diff (8c666fbe..347ea6e, 22 files, all under forgejo/, +5358/−113), not a narrowed interdiff.

  1. GO leg (landing.ts:4744, 5266-5345). GO_RECORD_RE is anchored and accepts only 40 hex (uppercase normalised, never shortened); the author is compared to the named ld-director-go identity before any field is read; repo/PR/head are compared against the caller's server-derived ref and the current head; no comments at all and no update time on any item both refuse (an empty read is not an empty store); edited rows (updatedAt !== at) refuse on the matching rows only, so an unrelated edited comment cannot mask a good record; the newest unedited match wins deterministically. Gating is read from one place — gatedFromRepoArg(landing.ts:3284) ORs the server view flag with the name, so a caller cannot spell a gated repo out of the gate; the fallback only ever adds gating.

  2. Seat leg (landing.ts:6760-7055). Tag shape is anchored to the run id taken from the head's LAST Lab-Run trailer (escaped, case-sensitive, no second per-project shape); the run-id grammar is judged on its own sentence, including a space-bearing value. Every counted seat must carry both labels; each declared sid must have a sole-sid answer with rows > 0, an admissible exact (requested, served) pair with SERVED trusted over the MODEL line (bare or last-slash-qualified spelling only), a verdict inside its own stream first..last + slack, and zero caller-supplied correlation ids. An absent answer, an unreadable endpoint and a refused address are all refusals in the lock's own one line, never data. requiredSeats is 2 for every gated landing with the dangerous classification only wording the sentence (landing.ts:6780, 7026); independence is a separate pairwise loop over all N*(N-1)/2 pairs with worst-fact-wins, so a 3+ panel with one correlated pair, one non-distinct pair or one never-compared pair refuses — my own re-read of 5f confirms no seats.length === 2 shortcut survives.

  3. Caller wiring (index.ts:422, 572, 611-700). The seat-lookup address is decided from the service-account mount, never from an env value or tool parameter; an in-pod process never falls back to loopback; a refused address becomes the same unreadable the lock refuses on. Answers are echo-checked (literal bare sid, other, and every carried timestamp inside the requested window); one failed lookup marks the whole read unreadable rather than a partial list. The verdict preview now passes the same serverRepoView, so preview and landing cannot disagree about which rules judge the row.

  4. Tests. vitest at 347ea6e: 63 files, 1408 tests, 1407 pass, 1 fail — tests/merge-identity.test.ts (ambient landing credential). I reproduced that same failure on the untouched base 8c666fbe in a separate worktree (1 failed / 24 passed), so it is pre-existing and not a hold; NEW FAILURES 0 holds from my own run. tsc --noEmit exits 0. The spec's named cases are present and non-vacuous: strict GO grammar incl. a short-sha refusal, wrong author, wrong PR/repo/head, edited and unreadable records; routine gated landing on ONE seat refused with the TWO-seat sentence; distinct-sid-twice counted as one seat; per-seat rows/models/stream/slack boundaries (exactly last+slack permits, +1 s refuses); caller correlation ids; unreadable endpoint; pairwise 3-seat cases that assert the named pair; n18 space-bearing trailer both alone and after a valid trailer; n17 address placements incl. an env attempt that changes nothing; N4 created_at on comment, review and inline rows. Assertions are discriminating (reason text, seat fact rows), not ok === false only.

No holds. One non-blocking observation: landing-corpus.test.ts replays its permit cases under a non-gated repo name (Homelab/pi-extensions) to keep the historical verdict-grammar assertions, with the gated permit path covered in the GO/seat suites — reasonable, and recorded in the fixture comment, but it does mean the corpus no longer exercises a gated permit itself.

Untested in this sandbox by design: the guard script belongs to a different repo and the live seat-lookup endpoint is exercised only through fixtures — I judged the gate, not a live fleet walk.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T1927Z

LD-VERDICT APPROVED head=347ea6edf4349bb543702fbdec4274b33107e5c6 session=LR-20261111-017-seatB-20261111T1927Z Fresh read-only clone, `git rev-parse HEAD` = 347ea6edf4349bb543702fbdec4274b33107e5c6 — the pinned head exactly, no divergence to report. Reviewed the full base..head diff (8c666fbe..347ea6e, 22 files, all under forgejo/, +5358/−113), not a narrowed interdiff. 1. GO leg (landing.ts:4744, 5266-5345). GO_RECORD_RE is anchored and accepts only 40 hex (uppercase normalised, never shortened); the author is compared to the named ld-director-go identity before any field is read; repo/PR/head are compared against the caller's server-derived ref and the current head; no comments at all and no update time on any item both refuse (an empty read is not an empty store); edited rows (updatedAt !== at) refuse on the matching rows only, so an unrelated edited comment cannot mask a good record; the newest unedited match wins deterministically. Gating is read from one place — gatedFromRepoArg(landing.ts:3284) ORs the server view flag with the name, so a caller cannot spell a gated repo out of the gate; the fallback only ever adds gating. 2. Seat leg (landing.ts:6760-7055). Tag shape is anchored to the run id taken from the head's LAST Lab-Run trailer (escaped, case-sensitive, no second per-project shape); the run-id grammar is judged on its own sentence, including a space-bearing value. Every counted seat must carry both labels; each declared sid must have a sole-sid answer with rows > 0, an admissible exact (requested, served) pair with SERVED trusted over the MODEL line (bare or last-slash-qualified spelling only), a verdict inside its own stream first..last + slack, and zero caller-supplied correlation ids. An absent answer, an unreadable endpoint and a refused address are all refusals in the lock's own one line, never data. requiredSeats is 2 for every gated landing with the dangerous classification only wording the sentence (landing.ts:6780, 7026); independence is a separate pairwise loop over all N*(N-1)/2 pairs with worst-fact-wins, so a 3+ panel with one correlated pair, one non-distinct pair or one never-compared pair refuses — my own re-read of 5f confirms no `seats.length === 2` shortcut survives. 3. Caller wiring (index.ts:422, 572, 611-700). The seat-lookup address is decided from the service-account mount, never from an env value or tool parameter; an in-pod process never falls back to loopback; a refused address becomes the same unreadable the lock refuses on. Answers are echo-checked (literal bare sid, other, and every carried timestamp inside the requested window); one failed lookup marks the whole read unreadable rather than a partial list. The verdict preview now passes the same serverRepoView, so preview and landing cannot disagree about which rules judge the row. 4. Tests. vitest at 347ea6e: 63 files, 1408 tests, 1407 pass, 1 fail — tests/merge-identity.test.ts (ambient landing credential). I reproduced that same failure on the untouched base 8c666fbe in a separate worktree (1 failed / 24 passed), so it is pre-existing and not a hold; NEW FAILURES 0 holds from my own run. tsc --noEmit exits 0. The spec's named cases are present and non-vacuous: strict GO grammar incl. a short-sha refusal, wrong author, wrong PR/repo/head, edited and unreadable records; routine gated landing on ONE seat refused with the TWO-seat sentence; distinct-sid-twice counted as one seat; per-seat rows/models/stream/slack boundaries (exactly last+slack permits, +1 s refuses); caller correlation ids; unreadable endpoint; pairwise 3-seat cases that assert the named pair; n18 space-bearing trailer both alone and after a valid trailer; n17 address placements incl. an env attempt that changes nothing; N4 created_at on comment, review and inline rows. Assertions are discriminating (reason text, seat fact rows), not `ok === false` only. No holds. One non-blocking observation: landing-corpus.test.ts replays its permit cases under a non-gated repo name (Homelab/pi-extensions) to keep the historical verdict-grammar assertions, with the gated permit path covered in the GO/seat suites — reasonable, and recorded in the fixture comment, but it does mean the corpus no longer exercises a gated permit itself. Untested in this sandbox by design: the guard script belongs to a different repo and the live seat-lookup endpoint is exercised only through fixtures — I judged the gate, not a live fleet walk. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T1927Z
fix(forgejo): harden the mgmt marker, move it to /var/lib, and verify the loopback listener's uid (LR-20261111-017, review-93 f4, R33, Amendment 21)
All checks were successful
CI / pr-title (pull_request) Successful in 2s
Details
CI / forgejo-ci (pull_request) Successful in 2m14s
Details
CI / pi-otel-telemetry (pull_request) Successful in 2m28s
Details
CI / skill-args (pull_request) Successful in 2m29s
Details
CI / todo (pull_request) Successful in 2m30s
Details
tests / python (pull_request) Successful in 12s
Details
CI / ask-user-question (pull_request) Successful in 2m51s
Details
CI / mcp-bridge (pull_request) Successful in 53s
Details
CI / nats-intercom (pull_request) Successful in 55s
Details
CI / redact (pull_request) Successful in 1m7s
Details
CI / pi-friction (pull_request) Successful in 1m9s
Details
CI / plane (pull_request) Successful in 3m45s
Details
CI / refocus (pull_request) Successful in 1m32s
Details
CI / forgejo (pull_request) Successful in 5m9s
Details
CI / pi-swarm-suite (pull_request) Successful in 7m2s
Details
feb1388691 
The positive management-host marker was checked with `stat` as "a root-owned
regular file", which left three holes review-93 f4 named: a symlink at the path
was followed and judged as its target; a group- or other-writable marker could
have its `listener_uid` rewritten by anyone in that group; and a writable or
non-root PARENT could swap the file wholesale. `checkMgmtMarker` now reads the
path with `lstat` (a symlink is refused outright), requires a regular file owned
by uid 0 whose mode has no group/other write (`mode & 0o022 === 0`), and verifies
its own directory and the one above through the same `lstat`, refusing on any
failure — every refusal is the existing one-line "no positive mgmt marker"
family and names WHICH check failed.

The marker MOVES to `/var/lib/lab-director/seat-lookup-mgmt`: `/etc/lab-director`
is a ConfigMap mountpoint in the production pods, so a root-owned file can never
legitimately exist there, and a path that is a mount for some readers and a
directory for others is exactly the ambiguity F2 exists to end. `/var/lib` is
node-local state; homelab-88's unit owns creating the directory. New marker
format: root:root 0644, body carrying `listener_uid=<uid>`.

R33's INTERIM check lands now: on the loopback path the code reads
`/proc/net/tcp`, finds the LISTEN row on 127.0.0.1:8097 (`0100007F:1FA1`, state
0A) and requires its owning uid (column 7) to EQUAL the marker's `listener_uid`.
Fail closed on a marker without the line, a non-numeric or ambiguous uid, no
listener, an unreadable table, or two LISTEN rows on that address:port (that is
itself a refusal, even with equal uids). One-line refusals name the fact. The
table text is injectable through the same test seam, so the parse is pinned to
the real /proc column layout.

The three merge-path tests in landing-evidence-complete-b8-017 now pin the
PLACEMENT: with the seam on a marked mgmt host the merged evidence read dials
127.0.0.1:8097, and the same fixture with a marker that cannot vouch for the port
refuses as unreadable with no merge sent.

Docs: the marker path, its hardened checks, its format and the listener check;
the "outer boundary" sentence is softened to what is true — the GO record and the
landing identity are controls this layer CHECKS, and the boundary that holds
against host root, server-side rules, reviewer-identity work and F1 is the
platform's, which this code cannot claim.

Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 20:03:50 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=feb138869181ed580f5a549dc0d70ab7c99f6be6 session=LR-20261111-017-seatB-20261111T2000Z

Reviewed the full base..head diff (8c666fbe..feb13886) of forgejo/landing.ts, forgejo/index.ts and the fixture tests at the pinned head; the PR's current head is exactly feb1388691.

  1. GO leg — GO_RECORD_RE is anchored, exactly one line, full 40-hex head (short sha refused, uppercase normalised); author must be the ld-director-go identity; the match is on repo + PR + current head; an unreadable comments read, a missing update time, an edited matching record (updatedAt !== at) and a well-formed record naming another repo/PR/head each refuse with their own one-line sentence; the newest unedited match is the record. Repo comparison is case-insensitive after trim, which cannot loosen the head pin.

  2. Seat leg — every counted trusted exact-head approval's declared sid is verified: rows > 0, served pairs admissible only through the reviewed exact-pair table (served trusted over the MODEL line, bare or provider-qualified spelling of the pair's declared id), first/last window required with verdict inside it plus STREAM_SLACK_MS, caller-supplied correlation ids refused per seat, comments-only on gated repos with the edit test. Tag is the anchored ^<runId>-seat(A|B)-<YYYYMMDD>T<HHMM>Z$ built from the head commit's LAST Lab-Run: trailer, run-id grammar applied to the trailer value itself. Independence is PAIRWISE over every unordered pair (N*(N-1)/2), each pair reading an actual endpoint answer, worst fact winning, a pair that fails un-verifying both members; a never-compared pair refuses. requiredSeats is 2 for every GO-gated landing (routine included) and verifiedSids must reach it. Unreadable/absent endpoint, a refused lookup address and a malformed echo all land as refusal, never as "no rows". Gated decision comes from the server's view (pr.base.repo.full_name) and never from the caller's string, and it can only add gating.

  3. Tests — the spec's named cases are present and non-vacuous: strict GO parse (short sha, wrong author, edited, no update time, unreadable), stream-window boundaries pinned at exactly last+slack and one second past, served-vs-MODEL refusal, same sid twice = one seat, routine and dangerous one-seat refusals, three-seat panels where every pair / one pair / a non-distinct pair / an unasked pair refuse plus the all-clean permit, and the non-gated repos keeping old behaviour. Ran vitest run at the head: 62/63 files pass, 1412/1413 tests pass; the single failure is tests/merge-identity.test.ts (ambient landing token), which is the known pre-existing failure on the untouched base, so NEW FAILURES 0 holds. The guard script belongs to infra-core and does not run for this repo, so nothing here contradicts the claimed guard state.

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T2000Z

LD-VERDICT APPROVED head=feb138869181ed580f5a549dc0d70ab7c99f6be6 session=LR-20261111-017-seatB-20261111T2000Z Reviewed the full base..head diff (8c666fbe..feb13886) of forgejo/landing.ts, forgejo/index.ts and the fixture tests at the pinned head; the PR's current head is exactly feb138869181ed580f5a549dc0d70ab7c99f6be6. 1) GO leg — `GO_RECORD_RE` is anchored, exactly one line, full 40-hex head (short sha refused, uppercase normalised); author must be the `ld-director-go` identity; the match is on repo + PR + current head; an unreadable comments read, a missing update time, an edited matching record (updatedAt !== at) and a well-formed record naming another repo/PR/head each refuse with their own one-line sentence; the newest unedited match is the record. Repo comparison is case-insensitive after trim, which cannot loosen the head pin. 2) Seat leg — every counted trusted exact-head approval's declared sid is verified: rows > 0, served pairs admissible only through the reviewed exact-pair table (served trusted over the MODEL line, bare or provider-qualified spelling of the pair's declared id), `first/last` window required with verdict inside it plus STREAM_SLACK_MS, caller-supplied correlation ids refused per seat, comments-only on gated repos with the edit test. Tag is the anchored `^<runId>-seat(A|B)-<YYYYMMDD>T<HHMM>Z$` built from the head commit's LAST `Lab-Run:` trailer, run-id grammar applied to the trailer value itself. Independence is PAIRWISE over every unordered pair (N*(N-1)/2), each pair reading an actual endpoint answer, worst fact winning, a pair that fails un-verifying both members; a never-compared pair refuses. `requiredSeats` is 2 for every GO-gated landing (routine included) and verifiedSids must reach it. Unreadable/absent endpoint, a refused lookup address and a malformed echo all land as refusal, never as "no rows". Gated decision comes from the server's view (`pr.base.repo.full_name`) and never from the caller's string, and it can only add gating. 3) Tests — the spec's named cases are present and non-vacuous: strict GO parse (short sha, wrong author, edited, no update time, unreadable), stream-window boundaries pinned at exactly last+slack and one second past, served-vs-MODEL refusal, same sid twice = one seat, routine and dangerous one-seat refusals, three-seat panels where every pair / one pair / a non-distinct pair / an unasked pair refuse plus the all-clean permit, and the non-gated repos keeping old behaviour. Ran `vitest run` at the head: 62/63 files pass, 1412/1413 tests pass; the single failure is tests/merge-identity.test.ts (ambient landing token), which is the known pre-existing failure on the untouched base, so NEW FAILURES 0 holds. The guard script belongs to infra-core and does not run for this repo, so nothing here contradicts the claimed guard state. Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T2000Z
ld-reviewer commented 2026-11-11 20:04:32 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=feb138869181ed580f5a549dc0d70ab7c99f6be6 session=LR-20261111-017-seatA-20261111T2000Z

Reviewed the PR's current head against the pin: the forge reports head feb1388691 (base 8c666fbe…), identical to the pinned 40-hex head, so the pin holds. Read the full base..head diff (origin/main...feb1388, 23 files, +6092/−112) in a read-only clone, focusing on forgejo/landing.ts and the caller wiring in forgejo/index.ts.

GO leg (landing.ts:5266 goRecordLeg, grammar at :4744, author at :4731): anchored LD-GO repo=<owner/name> pr=<n> head=<40-hex> only — short sha, extra prose, wrong field order all parse to null; author must be the declared go identity (right text from any other author refuses); record must name this repo, this PR and the full lowercase 40-hex current head; an empty comment read, a missing updated_at, and any matching record whose updatedAt !== at each refuse with their own one-line sentence; newest unedited match wins by at then id; the leg runs only when gated (landing.ts:6180, inside leg 2b) and the gated decision comes from serverRepoView (landing.ts:6009-6010), which gatedFromRepoArg (:3284) treats as a fail-closed OR over the server's pr.base.repo.full_name — the caller's spelling cannot un-gate it (index.ts:1101-1140).

Seat leg (landing.ts:6700-7050): run id from the LAST Lab-Run: trailer (:4930), value trimmed then checked against one grammar (:4998); declared tag must be the anchored ^<runId>-seat(A|B)-<stamp>$ (:4979); rows>0, admissible (requested,served) pair from the reviewed table (:4883, no equality shortcut, no case folding), MODEL line read against the settled pair's declared id with served trusted over the line (:4913); verdict must sit inside its own sid's first..last stream plus bounded slack (:5020); caller-supplied correlation ids refuse per seat; edited or unreadable edit state on a seat verdict refuses (:5131); the endpoint being absent, unreadable or answering another sid refuses in the lock's words. Independence is genuinely pairwise over every i<j pair, worst fact wins across both directions, a missing pair answer refuses rather than assuming "nothing shared", and any shared correlation id in any pair refuses both members (:6966-7013, withPairs demotes them). requiredSeats is 2 for every gated landing, routine included (:6780), and the verified-panel count (:7026) is a second, non-duplicate check since the panel includes concern-only sessions. The unlabelled-seat refusal (:6737) and formal-review rows not counting as seats on gated repos (countedSeats, :5163) are present. Wiring: collectLandingEvidence builds the lookup plan from the SAME ledger (index.ts:1293-1310) and the census feeds the lock the server view and same evidence (index.ts:1975-1998), so plan and lock cannot ask about different seats.

Tests: full vitest run at this head → 63 files, 1412 passed, 1 failed; the failure is tests/merge-identity.test.ts (ambient landing token), the pre-existing base failure, not a hold. Non-vacuity checked by mutation in a throwaway copy inside my own folder (source untouched afterwards): gated requiredSeats 2→1 kills 5 seat tests including the routine-gated single-seat case; restricting the pairwise loop to two-seat panels kills all 5 three-panel independence tests; deleting the GO edited-record filter kills 2 GO tests; trusting the MODEL line over the served pair kills 2 Amendment-6 tests. The spec's named cases (rows, served pair, distinct, shared ids, stream placement and slack boundaries, missing answer, endpoint unreadable, 3+ correlated panel, same sid twice, routine vs dangerous one-seat refusal, both model spellings, malformed run id, non-gated byte-identical behaviour) are each pinned by a test over the real lock — no mocks of the leg.

Holds: none.

Nits (non-blocking, no action required for this landing):

  1. landing.ts:4979 — the anchored pattern's ^/$ are not exercised in the suffix/prefix direction: tests/landing-seat-verification.test.ts:279-293 has no case of a valid tag with trailing junk, so dropping the anchors survives that test. The gate stays closed anyway because the endpoint must return rows for that exact sid, so this is test coverage, not a gate hole.
  2. landing.ts:5271-5280 — a PR that genuinely has no comments at all refuses with "the comments could not be read". That is the intended fail-closed direction and it is documented, but the sentence reads as a transport failure to the person acting on it.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T2000Z

LD-VERDICT APPROVED head=feb138869181ed580f5a549dc0d70ab7c99f6be6 session=LR-20261111-017-seatA-20261111T2000Z Reviewed the PR's current head against the pin: the forge reports head feb138869181ed580f5a549dc0d70ab7c99f6be6 (base 8c666fbe…), identical to the pinned 40-hex head, so the pin holds. Read the full base..head diff (origin/main...feb1388, 23 files, +6092/−112) in a read-only clone, focusing on forgejo/landing.ts and the caller wiring in forgejo/index.ts. GO leg (landing.ts:5266 `goRecordLeg`, grammar at :4744, author at :4731): anchored `LD-GO repo=<owner/name> pr=<n> head=<40-hex>` only — short sha, extra prose, wrong field order all parse to null; author must be the declared go identity (right text from any other author refuses); record must name this repo, this PR and the full lowercase 40-hex current head; an empty comment read, a missing `updated_at`, and any matching record whose `updatedAt !== at` each refuse with their own one-line sentence; newest unedited match wins by at then id; the leg runs only when `gated` (landing.ts:6180, inside leg 2b) and the gated decision comes from `serverRepoView` (landing.ts:6009-6010), which `gatedFromRepoArg` (:3284) treats as a fail-closed OR over the server's `pr.base.repo.full_name` — the caller's spelling cannot un-gate it (index.ts:1101-1140). Seat leg (landing.ts:6700-7050): run id from the LAST `Lab-Run:` trailer (:4930), value trimmed then checked against one grammar (:4998); declared tag must be the anchored `^<runId>-seat(A|B)-<stamp>$` (:4979); rows>0, admissible (requested,served) pair from the reviewed table (:4883, no equality shortcut, no case folding), MODEL line read against the settled pair's declared id with served trusted over the line (:4913); verdict must sit inside its own sid's first..last stream plus bounded slack (:5020); caller-supplied correlation ids refuse per seat; edited or unreadable edit state on a seat verdict refuses (:5131); the endpoint being absent, `unreadable` or answering another sid refuses in the lock's words. Independence is genuinely pairwise over every i<j pair, worst fact wins across both directions, a missing pair answer refuses rather than assuming "nothing shared", and any shared correlation id in any pair refuses both members (:6966-7013, `withPairs` demotes them). `requiredSeats` is 2 for every gated landing, routine included (:6780), and the verified-panel count (:7026) is a second, non-duplicate check since the panel includes concern-only sessions. The unlabelled-seat refusal (:6737) and formal-review rows not counting as seats on gated repos (`countedSeats`, :5163) are present. Wiring: `collectLandingEvidence` builds the lookup plan from the SAME ledger (index.ts:1293-1310) and the census feeds the lock the server view and same evidence (index.ts:1975-1998), so plan and lock cannot ask about different seats. Tests: full `vitest run` at this head → 63 files, 1412 passed, 1 failed; the failure is tests/merge-identity.test.ts (ambient landing token), the pre-existing base failure, not a hold. Non-vacuity checked by mutation in a throwaway copy inside my own folder (source untouched afterwards): gated `requiredSeats` 2→1 kills 5 seat tests including the routine-gated single-seat case; restricting the pairwise loop to two-seat panels kills all 5 three-panel independence tests; deleting the GO edited-record filter kills 2 GO tests; trusting the MODEL line over the served pair kills 2 Amendment-6 tests. The spec's named cases (rows, served pair, distinct, shared ids, stream placement and slack boundaries, missing answer, endpoint unreadable, 3+ correlated panel, same sid twice, routine vs dangerous one-seat refusal, both model spellings, malformed run id, non-gated byte-identical behaviour) are each pinned by a test over the real lock — no mocks of the leg. Holds: none. Nits (non-blocking, no action required for this landing): 1. landing.ts:4979 — the anchored pattern's `^`/`$` are not exercised in the suffix/prefix direction: tests/landing-seat-verification.test.ts:279-293 has no case of a valid tag with trailing junk, so dropping the anchors survives that test. The gate stays closed anyway because the endpoint must return rows for that exact sid, so this is test coverage, not a gate hole. 2. landing.ts:5271-5280 — a PR that genuinely has no comments at all refuses with "the comments could not be read". That is the intended fail-closed direction and it is documented, but the sentence reads as a transport failure to the person acting on it. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T2000Z
fix(forgejo): make the listener-uid check unanswerable by the querier (LR-20261111-017, homelab-93 H1, Amendment 22)
All checks were successful
CI / pr-title (pull_request) Successful in 2s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m22s
Details
CI / plane (pull_request) Successful in 1m21s
Details
CI / forgejo-ci (pull_request) Successful in 1m25s
Details
tests / python (pull_request) Successful in 12s
Details
CI / mcp-bridge (pull_request) Successful in 56s
Details
CI / todo (pull_request) Successful in 1m14s
Details
CI / refocus (pull_request) Successful in 1m24s
Details
CI / skill-args (pull_request) Successful in 1m11s
Details
CI / ask-user-question (pull_request) Successful in 1m8s
Details
CI / nats-intercom (pull_request) Successful in 1m2s
Details
CI / pi-friction (pull_request) Successful in 1m8s
Details
CI / forgejo (pull_request) Successful in 2m38s
Details
CI / redact (pull_request) Successful in 1m9s
Details
CI / pi-swarm-suite (pull_request) Successful in 6m9s
Details
6b4944791c 
The A21 loopback check verified the marker's `listener_uid` against the uid of
the LISTEN row on 127.0.0.1:8097, but three ways remained for the check to be
satisfied by the very process it is meant to police. All three are closed in the
same decision (`seatLookupAddress`), each refusal one line naming the failed fact:

1. REFUSE `listener_uid === process.getuid()`. A marker declaring the querying
   process's own uid is a self-certificate: the caller can drop the marker and
   hold the port. `process.getuid` unavailable is also a refusal — the check can
   then not tell self from foreign.
2. REQUIRE the identity user namespace: `/proc/self/uid_map` and
   `/proc/self/gid_map` must EACH be exactly the single line `0 0 4294967295`.
   Anything else (a range such as `0 1000 1`, extra lines, unreadable) means the
   process is in a user namespace, where userns-root can create files that LOOK
   root-owned, so the marker's owner and mode would be fiction. The refusal names
   WHICH map was wrong.
3. TOCTOU: after the marker, map and owner checks pass, `/proc/net/tcp` is
   re-read and the LISTEN socket's INODE (column 9) must be unchanged. A
   different socket — same address, same uid, different process — is refused,
   because the dial would reach the replacement.

All new reads go through the existing injectable `readText` seam and the
`/proc/net/tcp` parse reuses the A21 parser (now returning uid AND inode, with a
non-numeric or unsafe inode refused). Docs: the three H1 legs, plus the install
note that the port-forward unit must run as root or a dedicated uid no agent can
become — that is what makes `listener_uid` a check and not a self-answering one.

Tests: five new cases — self-uid refusal, uid_map userns, gid_map userns, the
swapped inode, and the control (identity maps + foreign matching uid + stable
inode ⇒ loopback). The four refusals are RED on feb1388 (proved by stashing: 4
failed / 37 passed in that file) and green here; the control holds both sides,
and every A21/F2 case stays green (the one A21 case
that asserted "1000 declaring 1000" now uses a uid that is not the runner's own,
which is exactly the new rule). Full suite: 1417 passed, 1 failed (the known
pre-existing ambient-token merge-identity case); `tsc --noEmit` clean.

Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 20:31:40 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=6b4944791c4e66aa72372049115794935450b6e6 session=LR-20261111-017-seatA-20261111T2029Z

Reviewed the full base..head diff (base 8c666fbec4, merge-base = base) at the pinned head; the PR head is 6b49447, so the pin holds.

Checked, GO leg (forgejo/landing.ts):

  • GO_RECORD_RE is anchored on the whole trimmed body, exactly two spaces of field order, full 40-hex head (uppercase normalised, short sha refused); prose-wrapped text does not parse.
  • author is compared to the ld-director-go identity — right text from any other author is refused.
  • head/repo/PR must all name this landing; a record naming another head is refused and the named head is printed.
  • edited and unreadable are refusals, not passes: updatedAt === undefined over the comment set refuses, updatedAt !== at refuses, updatedAt === at is the only acceptable record, and the newest unedited match wins deterministically.

Checked, seat leg:

  • gate decision taken once from the caller-supplied server view (gatedFromRepoArg on the SERVER's ref) and every gated leg reads that decision, not the caller string.
  • seats come from the unreduced exact-head, trusted, non-hold panel; on gated repos only unedited issue COMMENTS count, and every declared sid is looked up and verified (rows > 0, served (requested,served) pair must exist in the reviewed exact-pair table, the MODEL line is judged against the pair's declared id with served trusted over the line, bare or provider-qualified spelling).
  • the tag is anchored ^<runId>-seat(A|B)-<stamp>$, built by escaping the run id read from the head commit's LAST Lab-Run: trailer; free-form, unstamped and other-run tags are refused, and the trailer value is grammar-checked on its own sentence (space-bearing values reach the grammar, trailing whitespace/CR is trimmed).
  • the verdict must sit inside its own sid's stream (first..last + slack) and carries no caller-supplied correlation ids; a missing readable stream window refuses rather than skipping.
  • requiredSeats is 2 for EVERY landing on a gated repo (routine included), 2 for dangerous elsewhere; the same sid twice is one seat, and the verified-panel count is checked separately from the declaration count.
  • independence is PAIRWISE over all N*(N-1)/2 pairs, for any panel size: every pair needs its own endpoint answer, distinct must be true for every answer to that pair, shared correlation ids are taken as the max across answers in either direction, and a pair that does not pass demotes both members. A 3-seat panel with one correlated pair therefore refuses.
  • unreadable endpoint (absent, non-ok, non-array), a missing per-seat answer, and a missing pair answer all refuse in the lock's own words.

Checked, tests (forgejo/tests): landing-go-record.test.ts (20) and landing-seat-verification.test.ts (72) cover the spec's named cases non-vacuously — short sha, other head/PR/repo, wrong author, edited and edit-state-unreadable records, no-comment read; no rows, unknown served pair, MODEL line contradiction, routine gated one-seat refusal, dangerous two-seat, shared ids, non-distinct pair, out-of-stream verdict on both sides and exactly at first/last+slack, 3-seat panels (all-correlated, one bad pair, one unread pair, all clean), pair-plan shape (N + N*(N-1)/2) and the 24h cap, run-id grammar including the space-bearing and trailing-space cases; plus the gate-swap mutation tests.

Test run at the pinned head (vitest run, forgejo): 63 files, 1418 tests — 1417 pass, 1 fail = tests/merge-identity.test.ts (ambient landing token), the known pre-existing base failure, not a hold.

Holds: none.
Note (non-blocking): the guard script belongs to infra-core and does not run for this repo, so NEW FAILURES 0 is not independently verified here; the in-repo suite above is.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T2029Z

LD-VERDICT APPROVED head=6b4944791c4e66aa72372049115794935450b6e6 session=LR-20261111-017-seatA-20261111T2029Z Reviewed the full base..head diff (base 8c666fbec432f70b4853124f8de6ee2e445d1339, merge-base = base) at the pinned head; the PR head is 6b49447, so the pin holds. Checked, GO leg (forgejo/landing.ts): - GO_RECORD_RE is anchored on the whole trimmed body, exactly two spaces of field order, full 40-hex head (uppercase normalised, short sha refused); prose-wrapped text does not parse. - author is compared to the ld-director-go identity — right text from any other author is refused. - head/repo/PR must all name this landing; a record naming another head is refused and the named head is printed. - edited and unreadable are refusals, not passes: `updatedAt === undefined` over the comment set refuses, `updatedAt !== at` refuses, `updatedAt === at` is the only acceptable record, and the newest unedited match wins deterministically. Checked, seat leg: - gate decision taken once from the caller-supplied server view (gatedFromRepoArg on the SERVER's ref) and every gated leg reads that decision, not the caller string. - seats come from the unreduced exact-head, trusted, non-hold panel; on gated repos only unedited issue COMMENTS count, and every declared sid is looked up and verified (rows > 0, served (requested,served) pair must exist in the reviewed exact-pair table, the MODEL line is judged against the pair's declared id with `served` trusted over the line, bare or provider-qualified spelling). - the tag is anchored `^<runId>-seat(A|B)-<stamp>$`, built by escaping the run id read from the head commit's LAST `Lab-Run:` trailer; free-form, unstamped and other-run tags are refused, and the trailer value is grammar-checked on its own sentence (space-bearing values reach the grammar, trailing whitespace/CR is trimmed). - the verdict must sit inside its own sid's stream (first..last + slack) and carries no caller-supplied correlation ids; a missing readable stream window refuses rather than skipping. - requiredSeats is 2 for EVERY landing on a gated repo (routine included), 2 for dangerous elsewhere; the same sid twice is one seat, and the verified-panel count is checked separately from the declaration count. - independence is PAIRWISE over all N*(N-1)/2 pairs, for any panel size: every pair needs its own endpoint answer, `distinct` must be true for every answer to that pair, shared correlation ids are taken as the max across answers in either direction, and a pair that does not pass demotes both members. A 3-seat panel with one correlated pair therefore refuses. - unreadable endpoint (absent, non-ok, non-array), a missing per-seat answer, and a missing pair answer all refuse in the lock's own words. Checked, tests (forgejo/tests): landing-go-record.test.ts (20) and landing-seat-verification.test.ts (72) cover the spec's named cases non-vacuously — short sha, other head/PR/repo, wrong author, edited and edit-state-unreadable records, no-comment read; no rows, unknown served pair, MODEL line contradiction, routine gated one-seat refusal, dangerous two-seat, shared ids, non-distinct pair, out-of-stream verdict on both sides and exactly at first/last+slack, 3-seat panels (all-correlated, one bad pair, one unread pair, all clean), pair-plan shape (N + N*(N-1)/2) and the 24h cap, run-id grammar including the space-bearing and trailing-space cases; plus the gate-swap mutation tests. Test run at the pinned head (`vitest run`, forgejo): 63 files, 1418 tests — 1417 pass, 1 fail = tests/merge-identity.test.ts (ambient landing token), the known pre-existing base failure, not a hold. Holds: none. Note (non-blocking): the guard script belongs to infra-core and does not run for this repo, so NEW FAILURES 0 is not independently verified here; the in-repo suite above is. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T2029Z
ld-reviewer commented 2026-11-11 20:33:36 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=6b4944791c4e66aa72372049115794935450b6e6 session=LR-20261111-017-seatB-20261111T2029Z

Pinned head verified: origin/lr-20261111-017-census resolved to 6b4944791c, and the reviewed tree is that commit (clean read-only clone, diff vs origin/main).

GO leg (forgejo/landing.ts:4731-4790, 5266-5352, wired at 6178-6184 for gated repos only):

  • strict anchored grammar ^LD-GO repo=<owner/name> pr=<n> head=<40-hex>$; short sha, prose around it, wrong field order and missing fields all parse to null (tests/landing-go-record.test.ts:97).
  • author must be ld-director-go; correct text from any other login is refused (test :152).
  • repo (case-insensitive), PR number and FULL 40-hex current head all compared; a record naming another head/pr/repo is refused and names what it did name (:131-146).
  • edited comments refused (updatedAt !== at), and absence of an update time is a refusal, not a pass; an empty comment read is refused as unreadable (:162-191).
  • newest unedited matching record wins; an older edited record cannot mask it (:238-267).

Seat leg (landing.ts:5043-5310 helpers, 6688-7060 in the lock; caller wiring index.ts:1060-1090, 1200-1460, census rows 2100-2200):

  • every counted seat's declared sid must be a literal ^<Lab-Run id>-seat(A|B)-<stamp>$ tag for the run id taken from the head commit's last Lab-Run trailer (escaped, anchored, case-sensitive; run-id grammar one rule for all projects).
  • rows must be > 0, at least one served pair, and every pair must settle in the reviewed exact-pair table; the served pair is trusted over the MODEL line and a disagreeing line is refused (declaredModelMatches accepts bare or provider-qualified spelling only of the reviewed id).
  • verdict must sit inside the sid's own first..last stream plus bounded posting slack (stream boundaries pinned at both edges), edited verdicts and verdicts with unreadable edit state refused, caller-supplied correlation ids on a seat refused.
  • on a GO-gated repo requiredSeats is 2 unconditionally — the routine path is refused with one verified seat (tests/landing-seat-verification.test.ts:426) as is the dangerous path (:444); the dangerous classification only changes wording.
  • independence is PAIRWISE over every unordered pair: the caller plans N per-seat requests plus N*(N-1)/2 pair requests (index.ts:1423-1440), and the lock requires an actual endpoint answer for every pair, taking the worst reading when both directions exist. A 3-seat panel with any correlated or non-distinct pair is refused, and a missing pair answer is not read as "nothing shared" (:695-792).
  • endpoint unreadable/absent is a one-line lock refusal, never a throw; answer echo (sid, other, window inside the request) is enforced caller-side (index.ts:988-1040).
  • caller and lock derive seats from the same ledger and the same server-side gate decision, and both merge and census pass headCommitMessage/seatLookup/serverRepoView; census rows surface the GO id/author and the per-seat facts (UNVERIFIED included).

Tests: 63 files, 1417 passed / 1 failed. The single failure is tests/merge-identity.test.ts (ambient landing credential in this sandbox); I reproduced the same failure on an untouched origin/main worktree, so it is pre-existing and not a hold. The spec's named cases are present and non-vacuous (GO shape/author/head/edit/unreadable, routine and dangerous one-seat refusals, pairwise 3-seat panels, gate-swap mutation test at tests/landing-gate-swap-mutation-017.test.ts:83). NEW FAILURES 0 is consistent with what this sandbox can run; the guard script belongs to infra-core and does not run here.

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T2029Z

LD-VERDICT APPROVED head=6b4944791c4e66aa72372049115794935450b6e6 session=LR-20261111-017-seatB-20261111T2029Z Pinned head verified: origin/lr-20261111-017-census resolved to 6b4944791c4e66aa72372049115794935450b6e6, and the reviewed tree is that commit (clean read-only clone, diff vs origin/main). GO leg (forgejo/landing.ts:4731-4790, 5266-5352, wired at 6178-6184 for gated repos only): - strict anchored grammar `^LD-GO repo=<owner/name> pr=<n> head=<40-hex>$`; short sha, prose around it, wrong field order and missing fields all parse to null (tests/landing-go-record.test.ts:97). - author must be ld-director-go; correct text from any other login is refused (test :152). - repo (case-insensitive), PR number and FULL 40-hex current head all compared; a record naming another head/pr/repo is refused and names what it did name (:131-146). - edited comments refused (updatedAt !== at), and absence of an update time is a refusal, not a pass; an empty comment read is refused as unreadable (:162-191). - newest unedited matching record wins; an older edited record cannot mask it (:238-267). Seat leg (landing.ts:5043-5310 helpers, 6688-7060 in the lock; caller wiring index.ts:1060-1090, 1200-1460, census rows 2100-2200): - every counted seat's declared sid must be a literal `^<Lab-Run id>-seat(A|B)-<stamp>$` tag for the run id taken from the head commit's last Lab-Run trailer (escaped, anchored, case-sensitive; run-id grammar one rule for all projects). - rows must be > 0, at least one served pair, and every pair must settle in the reviewed exact-pair table; the served pair is trusted over the MODEL line and a disagreeing line is refused (declaredModelMatches accepts bare or provider-qualified spelling only of the reviewed id). - verdict must sit inside the sid's own first..last stream plus bounded posting slack (stream boundaries pinned at both edges), edited verdicts and verdicts with unreadable edit state refused, caller-supplied correlation ids on a seat refused. - on a GO-gated repo requiredSeats is 2 unconditionally — the routine path is refused with one verified seat (tests/landing-seat-verification.test.ts:426) as is the dangerous path (:444); the dangerous classification only changes wording. - independence is PAIRWISE over every unordered pair: the caller plans N per-seat requests plus N*(N-1)/2 pair requests (index.ts:1423-1440), and the lock requires an actual endpoint answer for every pair, taking the worst reading when both directions exist. A 3-seat panel with any correlated or non-distinct pair is refused, and a missing pair answer is not read as "nothing shared" (:695-792). - endpoint unreadable/absent is a one-line lock refusal, never a throw; answer echo (sid, other, window inside the request) is enforced caller-side (index.ts:988-1040). - caller and lock derive seats from the same ledger and the same server-side gate decision, and both merge and census pass headCommitMessage/seatLookup/serverRepoView; census rows surface the GO id/author and the per-seat facts (UNVERIFIED included). Tests: 63 files, 1417 passed / 1 failed. The single failure is tests/merge-identity.test.ts (ambient landing credential in this sandbox); I reproduced the same failure on an untouched origin/main worktree, so it is pre-existing and not a hold. The spec's named cases are present and non-vacuous (GO shape/author/head/edit/unreadable, routine and dangerous one-seat refusals, pairwise 3-seat panels, gate-swap mutation test at tests/landing-gate-swap-mutation-017.test.ts:83). NEW FAILURES 0 is consistent with what this sandbox can run; the guard script belongs to infra-core and does not run here. Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T2029Z
fix(forgejo): refuse a root querier and re-read the listener as the last thing before the dial (LR-20261111-017, homelab-93, Amendment 23)
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 59s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m9s
Details
CI / skill-args (pull_request) Successful in 1m6s
Details
CI / todo (pull_request) Successful in 1m14s
Details
tests / python (pull_request) Successful in 39s
Details
CI / ask-user-question (pull_request) Successful in 1m18s
Details
CI / refocus (pull_request) Successful in 1m29s
Details
CI / nats-intercom (pull_request) Successful in 1m18s
Details
CI / mcp-bridge (pull_request) Successful in 1m39s
Details
CI / redact (pull_request) Successful in 1m29s
Details
CI / pi-friction (pull_request) Successful in 1m36s
Details
CI / plane (pull_request) Successful in 3m1s
Details
CI / forgejo (pull_request) Successful in 3m11s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m44s
Details
83c4c9f8af 
Three defects in the A21/A22 loopback path, each with its own test:

1. A ROOT querier was allowed to satisfy the listener-uid check. Inside a user
   namespace /proc can be faked, and the whole point of the check is that the
   QUERIER cannot answer it; root is exactly the case the install note forbids
   (the port-forward unit runs as root or as a dedicated uid NO agent process can
   become). `seatLookupAddress` now refuses `process.getuid() === 0` in one line,
   before asking whether any other fact could hold. `getuid` unavailable stays a
   refusal.

2. The TOCTOU re-read of /proc/net/tcp ran inside the address decision, which
   returns before the caller parses the plan and builds its URLs — so the window
   it exists to close was open again by the time the bytes left. The decision now
   CARRIES the verified LISTEN socket inode (`seatLookupDecision`, with
   `seatLookupAddress` returning its address half unchanged) and the re-read moved
   into `readSeatLookup`, after the evidence lookups and immediately before the
   first request is sent; `fetchSeatAnswer` does the same on its self-resolved
   path, right before `fetch`. A swap is still a fail-closed refusal and still
   dials nothing. `loopbackSwapReason` holds the one-line reason.

3. The H1 namespace check reads the REAL /proc/self/uid_map and gid_map, which are
   not the identity under rootless CI (a userns maps the runner to ns-root), so
   the mgmt-path tests measured the runner's namespace. Every test that reaches
   the check now INJECTS the identity line through the existing `readText` seam
   (both map paths, identity by default, overridable per world), and the control
   that used the real kernel files injects them too.

Tests: the root refusal (red on the A22 head: the same world answered loopback /
self-uid), the TOCTOU placement (red on the A22 head: the decision still refused
the swap, so the "decision does not re-read" assertion goes red), and the map
injection default with an injected userns mapping (0 1000 1) still refused.

Full suite: 1419 passed, 1 failed (the known pre-existing ambient-token
merge-identity case); `tsc --noEmit` clean.

Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 20:57:55 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=83c4c9f8af79eafff7eb9d7ffdb42e33c34bd443 session=LR-20261111-017-seatB-20261111T2055Z

Fresh read-only clone, branch lr-20261111-017-census at 83c4c9f8af (matches the pin; base 8c666fbec4), full base..head diff reviewed (23 files, +6631/-112): forgejo/landing.ts, forgejo/index.ts, forgejo/api.ts, verdict.ts/verdict-cli.ts, docs, 14 test files.

GO leg (landing.ts:4744-4795 regex/parse, 5266-5347 leg): strict anchored ^LD-GO repo=\S+ pr=\d+ head=[0-9a-fA-F]{40}$ with only outer whitespace trimmed — short sha, extra prose, reordered fields all parse to null and are refused on their own sentence; author must be ld-director-go (case-folded, 5291); repo/PR/head compared against the caller's SERVER-spelling repoRef and the full 40-hex current head (5310-5319), refusal names the head it did name; edited refused via updated_at === created_at on matching rows only (5328-5336) and an item with no updated_at anywhere in the comment set is refused rather than assumed original (5282-5292); an empty comment read is refused as a failed read, never "no go" (5273-5281). Placement is inside if (gated) after head identity (6263-6273), so non-gated results are untouched.

Seat leg (landing.ts:5093-5290 helpers, 6687-7060 leg): head commit message then LAST Lab-Run: trailer (4930-4976, (.*)+trim, empty trailer = absence) then the ONE run-id grammar ^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$ (4998, 6738-6755) — sendrast-v0.7.2-chain passes as the trailer value, foo bar is a trailer whose value the grammar refuses, each with its own sentence. Tag is the literal anchored <runId>-seat(A|B)-<stamp> (4979-4984, 6756); seats are approvals over the unreduced sessionPanel, comments-only on gated repos (5163-5189) and the same set feeds both the caller's plan and the lock. Per seat: unedited (5137-5160), its OWN answer only (6800-6807), rows>0, every served pair must be in the reviewed exact-pair table with SERVED trusted over the MODEL line (6864-6920, both bare and provider-qualified spellings per declaredModelMatches), verdict inside its own first..last stream plus 5 min slack, caller-supplied correlation ids refused (6955-6966). Independence is PAIRWISE over all N*(N-1)/2 pairs (6979-7020): a pair with no endpoint answer refuses (a missing answer is not "nothing shared"), worst fact wins across directions, and either non-distinct or shared>0 un-verifies both members in the record. Required seats: gated ? 2 : dangerous ? 2 : 1 at 6780 — so EVERY gated landing, routine included, needs two distinct verified sessions (5g at 7025-7040), and the dangerous class only words the refusal. Endpoint unreadable/absent and any malformed answer count refused in the lock's own words (6786-6796); caller answers are field-validated, unreadable on any missing count or echo mismatch (index.ts seatAnswerOf/seatAnswerEchoesRequest). Caller wiring is consistent: repo identity resolved once from pr.base.repo.full_name (index.ts:1320), one gated decision (gatedFromRepoArg, landing.ts:3284) feeds GO, seat, N14, created_at and requiredSeats, and the same ServerRepoView opens the plan and the lock.

Tests: vitest run at this head → 1419 passed, 1 failed, the pre-existing tests/merge-identity.test.ts ambient-landing-token failure — reproduced identically on the untouched base 8c666fbec4 (25 tests, same assertion), so not attributable to this change. The spec's named cases are present and non-vacuous: short-sha/al prose/edited/no-update-time/empty-read GO refusals, routine-one-seat and dangerous-one-seat refusals that assert the new refusal wording and record.seats (new API surface, so they cannot pass on the old code), three-seat panels with one dirty pair, an unasked pair, and a fully independent control, edited-seat and no-answer and no-window refusals, stream-boundary pins at exactly slack and slack+1s, and caller-level tests for the percent-encoded ref, the disagreeing server name and the unreadable endpoint (no merge POST).

No holds. Two non-blocking notes, neither affecting the gate's direction: (1) landing.ts:7021-7024 says the verified panel "counts every session that did not hold (a CONCERN is a panel member)", while countedSeats (5163-5189) admits APPROVALS only — the code is the stricter reading and matches the SEATSID intent, so the comment is loose, not the check; (2) landing-gated-caller.test.ts is large (1402 lines) but its caller-side assertions are behavioural (no merge POST on refusal), so the risk is maintenance, not coverage.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T2055Z

LD-VERDICT APPROVED head=83c4c9f8af79eafff7eb9d7ffdb42e33c34bd443 session=LR-20261111-017-seatB-20261111T2055Z Fresh read-only clone, branch lr-20261111-017-census at 83c4c9f8af79eafff7eb9d7ffdb42e33c34bd443 (matches the pin; base 8c666fbec432f70b4853124f8de6ee2e445d1339), full base..head diff reviewed (23 files, +6631/-112): forgejo/landing.ts, forgejo/index.ts, forgejo/api.ts, verdict.ts/verdict-cli.ts, docs, 14 test files. GO leg (landing.ts:4744-4795 regex/parse, 5266-5347 leg): strict anchored `^LD-GO repo=\S+ pr=\d+ head=[0-9a-fA-F]{40}$` with only outer whitespace trimmed — short sha, extra prose, reordered fields all parse to null and are refused on their own sentence; author must be ld-director-go (case-folded, 5291); repo/PR/head compared against the caller's SERVER-spelling repoRef and the full 40-hex current head (5310-5319), refusal names the head it did name; edited refused via updated_at === created_at on matching rows only (5328-5336) and an item with no updated_at anywhere in the comment set is refused rather than assumed original (5282-5292); an empty comment read is refused as a failed read, never "no go" (5273-5281). Placement is inside `if (gated)` after head identity (6263-6273), so non-gated results are untouched. Seat leg (landing.ts:5093-5290 helpers, 6687-7060 leg): head commit message then LAST `Lab-Run:` trailer (4930-4976, `(.*)`+trim, empty trailer = absence) then the ONE run-id grammar `^[A-Za-z0-9][A-Za-z0-9._-]{2,63}$` (4998, 6738-6755) — `sendrast-v0.7.2-chain` passes as the trailer value, `foo bar` is a trailer whose value the grammar refuses, each with its own sentence. Tag is the literal anchored `<runId>-seat(A|B)-<stamp>` (4979-4984, 6756); seats are approvals over the unreduced sessionPanel, comments-only on gated repos (5163-5189) and the same set feeds both the caller's plan and the lock. Per seat: unedited (5137-5160), its OWN answer only (6800-6807), rows>0, every served pair must be in the reviewed exact-pair table with SERVED trusted over the MODEL line (6864-6920, both bare and provider-qualified spellings per declaredModelMatches), verdict inside its own first..last stream plus 5 min slack, caller-supplied correlation ids refused (6955-6966). Independence is PAIRWISE over all N*(N-1)/2 pairs (6979-7020): a pair with no endpoint answer refuses (a missing answer is not "nothing shared"), worst fact wins across directions, and either non-distinct or shared>0 un-verifies both members in the record. Required seats: `gated ? 2 : dangerous ? 2 : 1` at 6780 — so EVERY gated landing, routine included, needs two distinct verified sessions (5g at 7025-7040), and the dangerous class only words the refusal. Endpoint unreadable/absent and any malformed answer count refused in the lock's own words (6786-6796); caller answers are field-validated, unreadable on any missing count or echo mismatch (index.ts seatAnswerOf/seatAnswerEchoesRequest). Caller wiring is consistent: repo identity resolved once from pr.base.repo.full_name (index.ts:1320), one `gated` decision (gatedFromRepoArg, landing.ts:3284) feeds GO, seat, N14, created_at and requiredSeats, and the same ServerRepoView opens the plan and the lock. Tests: `vitest run` at this head → 1419 passed, 1 failed, the pre-existing tests/merge-identity.test.ts ambient-landing-token failure — reproduced identically on the untouched base 8c666fbec432f70b4853124f8de6ee2e445d1339 (25 tests, same assertion), so not attributable to this change. The spec's named cases are present and non-vacuous: short-sha/al prose/edited/no-update-time/empty-read GO refusals, routine-one-seat and dangerous-one-seat refusals that assert the new refusal wording and record.seats (new API surface, so they cannot pass on the old code), three-seat panels with one dirty pair, an unasked pair, and a fully independent control, edited-seat and no-answer and no-window refusals, stream-boundary pins at exactly slack and slack+1s, and caller-level tests for the percent-encoded ref, the disagreeing server name and the unreadable endpoint (no merge POST). No holds. Two non-blocking notes, neither affecting the gate's direction: (1) landing.ts:7021-7024 says the verified panel "counts every session that did not hold (a CONCERN is a panel member)", while countedSeats (5163-5189) admits APPROVALS only — the code is the stricter reading and matches the SEATSID intent, so the comment is loose, not the check; (2) landing-gated-caller.test.ts is large (1402 lines) but its caller-side assertions are behavioural (no merge POST on refusal), so the risk is maintenance, not coverage. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T2055Z
ld-reviewer commented 2026-11-11 21:00:27 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=83c4c9f8af79eafff7eb9d7ffdb42e33c34bd443 session=LR-20261111-017-seatA-20261111T2055Z

Reviewed the full base..head diff against origin/main (8c666fbe, equal to the recorded base) on a fresh read-only clone, pinned head confirmed via the PR record. Scope judged: the GO-record leg and the seat sid-verification leg in forgejo/landing.ts, the caller wiring in forgejo/index.ts, and the fixture tests.

GO leg (forgejo/landing.ts:4731-4795, 5266-5345):

  • Grammar is anchored and strict — ^LD-GO repo=(\S+) pr=(\d+) head=([0-9a-fA-F]{40})$; a short sha, extra prose or a missing field parses to null and is refused with its own sentence naming the 40-hex requirement.
  • Author is ld-director-go by exact identity (case-normalised, trimmed); the right text from any other author, or comments with no record from that identity, are refused.
  • Matching is on repo + PR + full lowercased 40-hex head against the current head, and mismatches name the head the record actually names.
  • Edit state: every comment must carry updated_at (absence is a refusal) and every MATCHING record must have updatedAt === at; an edited GO is refused with "post a new, unedited one".
  • An empty comment read is refused as "could not be read", never as "no go"; the newest unedited matching record wins deterministically.

Seat leg (forgejo/landing.ts:4998-5030, 5163-5265, 5595-5696, 6707-6990):

  • Head message and its LAST Lab-Run: trailer are required; the run id is judged by the one RUN_ID_GRAMMAR, with the offending value named on its own sentence (space-bearing values trimmed then judged, sendrast-v0.7.2-chain passes).
  • Each declared sid must equal the literal anchored tag ^<runId>-seat(A|B)-<YYYYMMDD>T<HHMM>Z$ (run id escaped); same sid twice collapses to one seat.
  • Every counted seat is verified: comments only on gated repos, unedited (seatVerdictEditRefusal), an answer exists, rows > 0, a served pair exists and is in the reviewed exact-pair table, and the MODEL line is read against the pair's declared id with the served pair trusted over the line (provider-qualified spelling accepted, everything else refused).
  • Independence is pairwise over every unordered pair (N*(N-1)/2), each read from its own endpoint answer in either direction, worst fact wins, shared_correlation_ids > 0 and non-distinct both refuse, and a pair never compared refuses rather than assuming zero; per-seat caller correlation ids > 0 also refuse.
  • requiredSeats is 2 on a gated repo regardless of class, and the verified-panel count refuses a routine gated landing with one seat; an unreadable/absent lookup refuses in the lock's own words.
  • Wiring: the gate decision comes once from pr.base.repo.full_name (serverRepoView with requireReported), that same view feeds the GO leg, the seat leg and the plan, and repoRef/updated_at/created_at (with atFromFallback refused by the lock on gated repos) are carried from the API — so no leg re-derives the repo from the caller's string and no lookup is skipped.

Tests: fixture-only against the pure lock. The named cases are present and non-vacuous in tests/landing-go-record.test.ts (shape/anchor/40-hex, other repo/PR/head, edited, unreadable, short sha, newest-record, created_at required, non-gated unchanged), tests/landing-seat-verification.test.ts (alias table, tag grammar, per-seat + every-pair plan, zero rows, missing answer, unobserved pair, MODEL line vs served, same sid twice, routine-one-seat and dangerous-one-seat refusals, shared/non-distinct ids, stream placement and slack boundaries, caller correlation ids, unreadable endpoint, run-id grammar incl. space), tests/landing-gated-caller.test.ts and tests/landing-gate-swap-mutation-017.test.ts (the one gate decides both sides — the same evidence one boolean apart). vitest run in forgejo: 57 files collected, 56 pass, 1278 tests pass; the only failure is tests/merge-identity.test.ts (ambient landing token), which is pre-existing on the untouched base — new failures 0, as claimed.

Holds: none. Two observations, neither a hold: (a) with the sandbox's external vitest the collected test count here is 1279, below the 1419 quoted in the body — an artifact of the harness/vitest version in this sandbox (3 files under tests/ are not collected by its include pattern), not of the code; (b) parseGoRecord accepts a zero-padded pr= (e.g. pr=0125) as the same number, which matches nothing today and refuses on mismatch anyway.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T2055Z

LD-VERDICT APPROVED head=83c4c9f8af79eafff7eb9d7ffdb42e33c34bd443 session=LR-20261111-017-seatA-20261111T2055Z Reviewed the full base..head diff against origin/main (8c666fbe, equal to the recorded base) on a fresh read-only clone, pinned head confirmed via the PR record. Scope judged: the GO-record leg and the seat sid-verification leg in forgejo/landing.ts, the caller wiring in forgejo/index.ts, and the fixture tests. GO leg (forgejo/landing.ts:4731-4795, 5266-5345): - Grammar is anchored and strict — `^LD-GO repo=(\S+) pr=(\d+) head=([0-9a-fA-F]{40})$`; a short sha, extra prose or a missing field parses to null and is refused with its own sentence naming the 40-hex requirement. - Author is `ld-director-go` by exact identity (case-normalised, trimmed); the right text from any other author, or comments with no record from that identity, are refused. - Matching is on repo + PR + full lowercased 40-hex head against the current head, and mismatches name the head the record actually names. - Edit state: every comment must carry `updated_at` (absence is a refusal) and every MATCHING record must have `updatedAt === at`; an edited GO is refused with "post a new, unedited one". - An empty comment read is refused as "could not be read", never as "no go"; the newest unedited matching record wins deterministically. Seat leg (forgejo/landing.ts:4998-5030, 5163-5265, 5595-5696, 6707-6990): - Head message and its LAST `Lab-Run:` trailer are required; the run id is judged by the one `RUN_ID_GRAMMAR`, with the offending value named on its own sentence (space-bearing values trimmed then judged, `sendrast-v0.7.2-chain` passes). - Each declared sid must equal the literal anchored tag `^<runId>-seat(A|B)-<YYYYMMDD>T<HHMM>Z$` (run id escaped); same sid twice collapses to one seat. - Every counted seat is verified: comments only on gated repos, unedited (`seatVerdictEditRefusal`), an answer exists, rows > 0, a served pair exists and is in the reviewed exact-pair table, and the MODEL line is read against the pair's `declared` id with the served pair trusted over the line (provider-qualified spelling accepted, everything else refused). - Independence is pairwise over every unordered pair (N*(N-1)/2), each read from its own endpoint answer in either direction, worst fact wins, `shared_correlation_ids > 0` and non-distinct both refuse, and a pair never compared refuses rather than assuming zero; per-seat caller correlation ids > 0 also refuse. - `requiredSeats` is 2 on a gated repo regardless of class, and the verified-panel count refuses a routine gated landing with one seat; an unreadable/absent lookup refuses in the lock's own words. - Wiring: the gate decision comes once from `pr.base.repo.full_name` (`serverRepoView` with `requireReported`), that same view feeds the GO leg, the seat leg and the plan, and `repoRef`/`updated_at`/`created_at` (with `atFromFallback` refused by the lock on gated repos) are carried from the API — so no leg re-derives the repo from the caller's string and no lookup is skipped. Tests: fixture-only against the pure lock. The named cases are present and non-vacuous in tests/landing-go-record.test.ts (shape/anchor/40-hex, other repo/PR/head, edited, unreadable, short sha, newest-record, created_at required, non-gated unchanged), tests/landing-seat-verification.test.ts (alias table, tag grammar, per-seat + every-pair plan, zero rows, missing answer, unobserved pair, MODEL line vs served, same sid twice, routine-one-seat and dangerous-one-seat refusals, shared/non-distinct ids, stream placement and slack boundaries, caller correlation ids, unreadable endpoint, run-id grammar incl. space), tests/landing-gated-caller.test.ts and tests/landing-gate-swap-mutation-017.test.ts (the one gate decides both sides — the same evidence one boolean apart). `vitest run` in forgejo: 57 files collected, 56 pass, 1278 tests pass; the only failure is tests/merge-identity.test.ts (ambient landing token), which is pre-existing on the untouched base — new failures 0, as claimed. Holds: none. Two observations, neither a hold: (a) with the sandbox's external vitest the collected test count here is 1279, below the 1419 quoted in the body — an artifact of the harness/vitest version in this sandbox (3 files under tests/ are not collected by its include pattern), not of the code; (b) `parseGoRecord` accepts a zero-padded `pr=` (e.g. `pr=0125`) as the same number, which matches nothing today and refuses on mismatch anyway. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T2055Z
fix(forgejo): re-read the listener AFTER the lookups and inject the querier's uid (LR-20261111-017, review-3c S1 S2)
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / pi-otel-telemetry (pull_request) Successful in 58s
Details
tests / python (pull_request) Successful in 17s
Details
CI / forgejo-ci (pull_request) Successful in 1m4s
Details
CI / mcp-bridge (pull_request) Successful in 35s
Details
CI / skill-args (pull_request) Successful in 44s
Details
CI / plane (pull_request) Successful in 1m29s
Details
CI / redact (pull_request) Successful in 55s
Details
CI / pi-friction (pull_request) Successful in 56s
Details
CI / nats-intercom (pull_request) Successful in 1m12s
Details
CI / todo (pull_request) Successful in 1m24s
Details
CI / ask-user-question (pull_request) Successful in 1m35s
Details
CI / refocus (pull_request) Successful in 1m40s
Details
CI / forgejo (pull_request) Successful in 2m5s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m57s
Details
fe1775593a 
S1 (H1-b): the TOCTOU re-read was still pre-dial — two synchronous
/proc/net/tcp reads microseconds apart, with the window from the second read
to each async connect wide open. `readSeatLookup` now performs BOTH reads:
the pre-dial one is kept (a swap already visible is never dialled) and a new
one runs after `Promise.all` resolves. A LISTEN socket cannot vacate the
address and return with the same inode, and no other uid can co-bind it while
it is held, so the same socket at both ends proves every connect in between
reached that socket. On a swap the fetched answers are DISCARDED and the read
is `unreadable`.

S2 (H1-d): the root refusal read the runner's real getuid, so under
`unshare -Ur` (ns-root) 23 of 62 tests failed on it. `SeatLookupPlacement`
now carries an injectable `getuid` (production still reads process.getuid),
both shared mgmt helpers declare a non-root uid (1005), and the root-querier
test pins 0 explicitly.

Tests: [A,A,B] post-lookup swap with VALID echoing answers ⇒ unreadable and
the answers thrown away (red on 83c4c9f); a kept pre-dial read [A,B,B] ⇒
never dialled; the direct `fetchSeatAnswer` self-resolved re-read (n2);
`process.getuid` undefined ⇒ the one-line refusal (n1, was a2-surviving).
Comments at `seatLookupDecision`/`readSeatLookup` and the docs bullet
corrected: the plan is built BEFORE readSeatLookup, the URLs after the
pre-dial read, and the post-lookup re-read is the last line of defense.

Full suite: 1422 passed, 1 failed (the known pre-existing ambient-token
merge-identity case); `tsc --noEmit` clean. `unshare -Ur` is not permitted in
this sandbox; the S2 red→green was proven with a `process.getuid = () => 0`
preload instead (24 failed on 83c4c9f → 0 failed here).

Lab-Run: LR-20261111-017
ld-reviewer commented 2026-11-11 21:26:21 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=fe1775593a5dc343c6a90639647f910b44af972a session=LR-20261111-017-seatB-20261111T2122Z

Reviewed the pinned head fe1775593a (PR base sha 8c666fbec4) on a fresh read-only clone, branch lr-20261111-017-census vs origin/main. Fresh clone, no prior context on this PR; judged the diff, tests and guard only.

GO leg (forgejo/landing.ts goRecordLeg / GO_RECORD_RE): anchored ^LD-GO repo=<owner/name> pr=<n> head=<40-hex>$, full 40-hex required (short sha refuse sentence names NEVER-TRY), author must be ld-director-go (author checked before the parse decides anything), repo/PR/head compared field-wise, edited record refused (updatedAt !== at, checked on the MATCHING rows only), missing update time refused, an empty comment read refused, newest unedited match wins. Gating reads the SERVER view (pr.base.repo.full_name resolved once in index.ts, refusal when the forge's name is absent or disagrees), and the gated legs read one gated decision rather than re-deriving from the caller string.

Seat leg (5a–5g): head commit's LAST Lab-Run trailer, one run-id grammar (space-bearing value trimmed-then-judged, sendrast-v0.7.2-chain valid), anchored tag ^-seat(A|B)-$ with the run id escaped; only issue comments are seats on a gated repo and an edited/unreadable verdict is refused; rows present, served pair must be in the reviewed exact-pair table with the SERVED model trusted over the MODEL line (bare or provider-qualified only — no prefix/family/case folding); verdict must sit inside its own sid's stream [first, last+slack]; per-sid caller correlation ids refused; endpoint absent/unreadable refused in the lock's own words. requiredSeats = 2 for EVERY gated landing (routine included; the dangerous class only picks the sentence) and the whole leg sits inside if (gated). Independence is PAIRWISE over every unordered pair of declared seats (N*(N-1)/2 answers, worst fact wins, both directions read, a missing pair answer is a refusal, a failed pair demotes BOTH seats), so a correlated 3+ panel cannot pass; a shared correlation id in any pair refuses.

Tests: vitest at head — 63 files / 1423 tests, 1422 pass; the single failure is tests/merge-identity.test.ts (ambient landing token), which fails identically on the untouched base, so not a hold. tsc --noEmit clean. The spec's named cases are present and non-vacuous: I killed each rule by mutation in a throwaway copy (outside the PR tree) and every mutation turned tests red — requiredSeats relaxation to routine=1 (4 failures), pairwise relaxation to the two-seat shape (3), GO regex to short sha + edited-allowed (4), GO author unchecked (2), unreadable lookup permitted (1), unknown served pair allowed (1), MODEL-line check removed (2), caller correlation ids permitted (1), zero rows permitted (1), stream-window check removed (3).

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatB-20261111T2122Z

LD-VERDICT APPROVED head=fe1775593a5dc343c6a90639647f910b44af972a session=LR-20261111-017-seatB-20261111T2122Z Reviewed the pinned head fe1775593a5dc343c6a90639647f910b44af972a (PR base sha 8c666fbec432f70b4853124f8de6ee2e445d1339) on a fresh read-only clone, branch lr-20261111-017-census vs origin/main. Fresh clone, no prior context on this PR; judged the diff, tests and guard only. GO leg (forgejo/landing.ts goRecordLeg / GO_RECORD_RE): anchored `^LD-GO repo=<owner/name> pr=<n> head=<40-hex>$`, full 40-hex required (short sha refuse sentence names NEVER-TRY), author must be ld-director-go (author checked before the parse decides anything), repo/PR/head compared field-wise, edited record refused (updatedAt !== at, checked on the MATCHING rows only), missing update time refused, an empty comment read refused, newest unedited match wins. Gating reads the SERVER view (pr.base.repo.full_name resolved once in index.ts, refusal when the forge's name is absent or disagrees), and the gated legs read one `gated` decision rather than re-deriving from the caller string. Seat leg (5a–5g): head commit's LAST Lab-Run trailer, one run-id grammar (space-bearing value trimmed-then-judged, sendrast-v0.7.2-chain valid), anchored tag ^<runId>-seat(A|B)-<stamp>$ with the run id escaped; only issue comments are seats on a gated repo and an edited/unreadable verdict is refused; rows present, served pair must be in the reviewed exact-pair table with the SERVED model trusted over the MODEL line (bare or provider-qualified only — no prefix/family/case folding); verdict must sit inside its own sid's stream [first, last+slack]; per-sid caller correlation ids refused; endpoint absent/unreadable refused in the lock's own words. requiredSeats = 2 for EVERY gated landing (routine included; the dangerous class only picks the sentence) and the whole leg sits inside `if (gated)`. Independence is PAIRWISE over every unordered pair of declared seats (N*(N-1)/2 answers, worst fact wins, both directions read, a missing pair answer is a refusal, a failed pair demotes BOTH seats), so a correlated 3+ panel cannot pass; a shared correlation id in any pair refuses. Tests: vitest at head — 63 files / 1423 tests, 1422 pass; the single failure is tests/merge-identity.test.ts (ambient landing token), which fails identically on the untouched base, so not a hold. tsc --noEmit clean. The spec's named cases are present and non-vacuous: I killed each rule by mutation in a throwaway copy (outside the PR tree) and every mutation turned tests red — requiredSeats relaxation to routine=1 (4 failures), pairwise relaxation to the two-seat shape (3), GO regex to short sha + edited-allowed (4), GO author unchecked (2), unreadable lookup permitted (1), unknown served pair allowed (1), MODEL-line check removed (2), caller correlation ids permitted (1), zero rows permitted (1), stream-window check removed (3). Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatB-20261111T2122Z
ld-reviewer commented 2026-11-11 21:28:20 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=fe1775593a5dc343c6a90639647f910b44af972a session=LR-20261111-017-seatA-20261111T2122Z

Checked at the pin: the PR head reads back as fe1775593a (no drift), base 8c666fbe; full base..head diff read (23 files, +6818), cloned read-only, nothing edited.

GO leg — sound. Anchored grammar ^LD-GO repo=(\S+) pr=(\d+) head=([0-9a-fA-F]{40})$ with no short sha and no prose (landing.ts:4744, parseGoRecord 4784); author must be the named ld-director-go identity (4731, compared case-folded and trimmed at 5291); repo/pr/head compared on the SERVER's view (identity.view.ref, index.ts:1369/1370, passed at 1574/1614) so the caller's typed string cannot name the record; an empty comment read and any comment without updated_at refuse (5276, 5286), an edited record refuses (5331), and the newest unedited match wins (5347).

Seat leg — sound. Every counted seat's declared sid is verified against the endpoint: no answer → refuse, rows must be > 0, served pairs must be in the reviewed exact-pair table, and the served pair beats the model: line via the bare-or-provider-qualified matcher (landing.ts:4913, used at 6907); the tag must be the anchored <Lab-Run>-seat(A|B)-<stamp> for the run id read from the head's LAST trailer under the one run-id grammar (4954, 4979, 4998; the space-bearing value is trimmed then grammar-judged); the verdict must sit inside its own sid's first..last stream within 5 min slack (5020, 6960); any caller-supplied correlation id refuses (6977). Independence is pairwise over every pair for any panel size, worst fact wins in both directions, and a pair the endpoint never compared refuses (6981–7015) — a 3+ panel cannot hide one correlated pair. On a GO-gated repo requiredSeats is 2 for routine changes too (6780), counted against verified distinct sessions in 5g (7026). Endpoint unreadable/absent refuses in the lock's own words (6792), and the caller passes status:"unreadable" rather than throwing (index.ts:1210).

Tests — ran the suite in forgejo/ at the pin: 1422 passed, 1 failed, and the only failure is tests/merge-identity.test.ts (ambient landing token), the pre-existing sandbox failure named in the brief — no new failures. The spec's named cases exist and are non-vacuous: GO grammar/author/head/edited/unreadable in tests/landing-go-record.test.ts; rows, served-pair table, MODEL-vs-served, stream placement and bounds, caller correlation ids, unreadable endpoint, routine one-seat refusal with record assertions (tests/landing-seat-verification.test.ts:426), and pairwise three-seat refusals that blame only the offending pair (:716, :734, :749).

Holds: none.

model: omniroute/deepseek-flash
session: LR-20261111-017-seatA-20261111T2122Z

LD-VERDICT APPROVED head=fe1775593a5dc343c6a90639647f910b44af972a session=LR-20261111-017-seatA-20261111T2122Z Checked at the pin: the PR head reads back as fe1775593a5dc343c6a90639647f910b44af972a (no drift), base 8c666fbe; full base..head diff read (23 files, +6818), cloned read-only, nothing edited. GO leg — sound. Anchored grammar `^LD-GO repo=(\S+) pr=(\d+) head=([0-9a-fA-F]{40})$` with no short sha and no prose (landing.ts:4744, parseGoRecord 4784); author must be the named `ld-director-go` identity (4731, compared case-folded and trimmed at 5291); repo/pr/head compared on the SERVER's view (`identity.view.ref`, index.ts:1369/1370, passed at 1574/1614) so the caller's typed string cannot name the record; an empty comment read and any comment without `updated_at` refuse (5276, 5286), an edited record refuses (5331), and the newest unedited match wins (5347). Seat leg — sound. Every counted seat's declared sid is verified against the endpoint: no answer → refuse, rows must be > 0, served pairs must be in the reviewed exact-pair table, and the served pair beats the `model:` line via the bare-or-provider-qualified matcher (landing.ts:4913, used at 6907); the tag must be the anchored `<Lab-Run>-seat(A|B)-<stamp>` for the run id read from the head's LAST trailer under the one run-id grammar (4954, 4979, 4998; the space-bearing value is trimmed then grammar-judged); the verdict must sit inside its own sid's first..last stream within 5 min slack (5020, 6960); any caller-supplied correlation id refuses (6977). Independence is pairwise over every pair for any panel size, worst fact wins in both directions, and a pair the endpoint never compared refuses (6981–7015) — a 3+ panel cannot hide one correlated pair. On a GO-gated repo requiredSeats is 2 for routine changes too (6780), counted against verified distinct sessions in 5g (7026). Endpoint unreadable/absent refuses in the lock's own words (6792), and the caller passes `status:"unreadable"` rather than throwing (index.ts:1210). Tests — ran the suite in forgejo/ at the pin: 1422 passed, 1 failed, and the only failure is tests/merge-identity.test.ts (ambient landing token), the pre-existing sandbox failure named in the brief — no new failures. The spec's named cases exist and are non-vacuous: GO grammar/author/head/edited/unreadable in tests/landing-go-record.test.ts; rows, served-pair table, MODEL-vs-served, stream placement and bounds, caller correlation ids, unreadable endpoint, routine one-seat refusal with record assertions (tests/landing-seat-verification.test.ts:426), and pairwise three-seat refusals that blame only the offending pair (:716, :734, :749). Holds: none. model: omniroute/deepseek-flash session: LR-20261111-017-seatA-20261111T2122Z
All checks were successful
CI / pr-title (pull_request) Successful in 0s
Details
CI / pi-otel-telemetry (pull_request) Successful in 58s
Details
tests / python (pull_request) Successful in 17s
Details
CI / forgejo-ci (pull_request) Successful in 1m4s
Details
CI / mcp-bridge (pull_request) Successful in 35s
Details
CI / skill-args (pull_request) Successful in 44s
Details
CI / plane (pull_request) Successful in 1m29s
Details
CI / redact (pull_request) Successful in 55s
Details
CI / pi-friction (pull_request) Successful in 56s
Details
CI / nats-intercom (pull_request) Successful in 1m12s
Details
CI / todo (pull_request) Successful in 1m24s
Details
CI / ask-user-question (pull_request) Successful in 1m35s
Details
CI / refocus (pull_request) Successful in 1m40s
Details
CI / forgejo (pull_request) Successful in 2m5s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m57s
Details
This pull request has changes conflicting with the target branch.
  • forgejo/index.ts
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin lr-20261111-017-census:lr-20261111-017-census
git switch lr-20261111-017-census

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff lr-20261111-017-census
git switch lr-20261111-017-census
git rebase main
git switch main
git merge --ff-only lr-20261111-017-census
git switch lr-20261111-017-census
git rebase main
git switch main
git merge --no-ff lr-20261111-017-census
git switch main
git merge --squash lr-20261111-017-census
git switch main
git merge --ff-only lr-20261111-017-census
git switch main
git merge lr-20261111-017-census
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/pi-extensions!125
No description provided.