Homelab/infra-core
6
0
Fork 2
Code Issues Pull requests 35 Projects Releases 1 Packages Wiki Activity Actions

seatpost: 8098 posts as ld-reviewer; R29 one-claim-per-seat; relay provenance gate (LR-20261111-025, AMENDMENT 4) #950

Open
pi-landing wants to merge 4 commits from lr-20261111-025/amendment-4 into live
pull from: lr-20261111-025/amendment-4
merge into: Homelab:live
Conversation 8 Commits 4 Files changed 15 +2793 -202
pi-landing commented 2026-11-11 22:33:04 +00:00
Collaborator
Copy link

Spec 025 / AMENDMENT 4: 8098 posts as ld-reviewer; R29 one-claim-per-seat; the relay's provenance gate

El Director's identity ruling, built to homelab-3c's five binding conditions
(agreed 2026-11-11). This is a gate change — it moves what authored seat
verdicts the fleet's gates count.

The five conditions, each with its proof

  1. The ld-reviewer token lives only in ld-runtime's Secret in ld-fleet.
    The endpoint's comment POST uses LD_REVIEWER_TOKEN (mounted from
    ld-runtime-forgejo-reviewer, which prod already carries — no new
    identity, no prod env change); the head read keeps LD_FORGEJO_TOKEN. A
    missing reviewer token is a per-call 502 naming the variable — never
    a boot failure and never a fallback to the landing token (that
    fallback would post as pi-landing, the exact bug this amendment exists
    to fix). The guard gains a census of ld-reviewer identity material under
    core/manifests/apps/sbx-*/: the two known copy-pairs (sbx-lab +
    sbx-gits, removed at the 009(c) daylight cut) are pinned, and any NEW
    reference or renamed secret fails the suite.
  2. Plain comments only, never formal reviews (tested).
    TestNoReviewOnTheSeatPostPath parses the package sources and the
    wiring with go/ast (comments invisible): any /reviews literal or
    review-shaped seam method fails; the Poster interface is pinned to
    exactly {PostIssueComment, PullHead}. Both halves proven to bite with
    temp probes, then reverted.
  3. R29 — one claim per (repo, pr, head, sid), the code side of migration
    00008     (prod #1210, additive, lands first). Same sid + fresh bundle +
    same target is refused (TestOneSeatClaimPerTargetIsR29); a burned
    bundle is final for the seat on that target — recovery is a fresh
    registration with a FRESH SID, because the verdict is the seat's and the
    seat gets one. The boot index assertion now requires
    seat_posts_one_claim_per_sid + seat_posts_one_post and names which
    is missing; the dropped per-bundle claim name is no longer required.
  4. The census keeps requiring SEATSID provenance until A14(b). Nothing
    here relaxes it — no census code touched; the endpoint still records the
    sid and the comment_id the provenance read counts by.
  5. The relay accepts an LD-VERDICT only when it was posted through the
    endpoint     (provenance: the comment_id is in seat_posts for that
    repo/pr/head, read in-process from the record DB). Env-gated —
    LD_RELAY_PROVENANCE_ONLY, default OFF, prod flips it at the daylight
    cut, one boot line names the state either way; fail-closed on a
    provenance read error (an unverifiable verdict is skipped, never
    counted); gate-off behavior is byte-for-byte today's.

The residual, explicit

Anything in sbx-lab can still call 8098 and post an ld-reviewer-authored
verdict claiming a seat sid. Until spec 026 (R27) stamps the tags, what
makes a comment a seat is the gateway sid check plus the one-post,
head-pinned rule, NOT the author.

Roll-out order (with the lead)

  1. prod #1210 (00008) lands and homelab-88 confirms the migrate Job applied
    it — the image below asserts the new index at boot.
  2. This PR lands; the next prod pin carries it.
  3. The 009(c) daylight cut: the sbx-* ld-reviewer material goes (the guard
    census keeps the removal honest), prod sets LD_RELAY_PROVENANCE_ONLY,
    and seats post only through 8098.

Proof

  • go vet ./... clean; go test -race -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/ — all ok (31/34/58/60 test functions).
  • Guard: §30 tripwire passes on the head; full suite 20 FAILs, all
    pre-existing, unchanged.

no independent walk: implementer-run checks only

Lab-Run: LR-20261111-025

Review round 2 (stacked 2f50d0d0) — homelab-3c's B1-B3/C5/tests

  • B2 (C3): registration canonicalizes — the repo is lower-cased into
    the pin (case variants normalized; anything lower-casing cannot fix is
    refused), the sid is stored verbatim. Real-index proof: the CI schema
    now carries seat_posts with both partial unique indexes, and
    TestConcurrentClaimOnOneSidInsertsOnce issues two racing INSERTs
    against PostgreSQL — ci.yaml fails the build if it did not execute.
  • B3: register rejects repos outside the four gated repos; the
    relay's resolved <ID> lines count only with endpoint provenance
    (same env gate; off = byte-for-byte old behavior).
  • B1: guard section 30 now states the facts — sbx-gits is kept ON
    PURPOSE, the ld-reviewer token still reaches sbx-lab and sbx-gits, the
    store's namespace condition is the daylight cut's job. PR text matches.
  • Tests: TestNoReviewOnTheSeatPostPath walks run/forgejo.go's call
    graph and pins the Poster method set (proven to bite on a rename and a
    /reviews probe); the R29 concurrency test asserts exactly one Forgejo
    call and the loser's named refusal.
  • C5 rides a companion prod PR: LD_RELAY_PROVENANCE_ONLY=true
    (inert until the next pin rolls an image that reads it).

NO seats on this round until the seat-launch tooling (#312 @7a43b28)
lands, per the lead. go vet + go test -race green on seatpost /
record / relay / cmd/ld-runtime.

Patch for this round: amendment4-r2-interdiff.patch
(5732e5f0..2f50d0d0); full PR diff: amendment4-diff.patch.

# Spec 025 / AMENDMENT 4: 8098 posts as ld-reviewer; R29 one-claim-per-seat; the relay's provenance gate El Director's identity ruling, built to homelab-3c's five binding conditions (agreed 2026-11-11). This is a **gate change** — it moves what authored seat verdicts the fleet's gates count. ## The five conditions, each with its proof 1. **The ld-reviewer token lives only in ld-runtime's Secret in ld-fleet.** The endpoint's comment POST uses `LD_REVIEWER_TOKEN` (mounted from `ld-runtime-forgejo-reviewer`, which prod already carries — no new identity, no prod env change); the head read keeps `LD_FORGEJO_TOKEN`. A missing reviewer token is a per-call 502 naming the variable — **never** a boot failure and **never** a fallback to the landing token (that fallback would post as pi-landing, the exact bug this amendment exists to fix). The guard gains a census of ld-reviewer identity material under `core/manifests/apps/sbx-*/`: the two known copy-pairs (sbx-lab + sbx-gits, removed at the 009(c) daylight cut) are pinned, and any NEW reference or renamed secret fails the suite. 2. **Plain comments only, never formal reviews (tested).** `TestNoReviewOnTheSeatPostPath` parses the package sources **and** the wiring with go/ast (comments invisible): any `/reviews` literal or review-shaped seam method fails; the `Poster` interface is pinned to exactly `{PostIssueComment, PullHead}`. Both halves proven to bite with temp probes, then reverted. 3. **R29 — one claim per (repo, pr, head, sid), the code side of migration 00008** (prod #1210, additive, lands first). Same sid + fresh bundle + same target is refused (`TestOneSeatClaimPerTargetIsR29`); a burned bundle is final for the seat on that target — recovery is a fresh registration with a FRESH SID, because the verdict is the seat's and the seat gets one. The boot index assertion now requires `seat_posts_one_claim_per_sid` + `seat_posts_one_post` and names which is missing; the dropped per-bundle claim name is no longer required. 4. **The census keeps requiring SEATSID provenance until A14(b).** Nothing here relaxes it — no census code touched; the endpoint still records the sid and the comment_id the provenance read counts by. 5. **The relay accepts an LD-VERDICT only when it was posted through the endpoint** (provenance: the comment_id is in `seat_posts` for that repo/pr/head, read in-process from the record DB). Env-gated — `LD_RELAY_PROVENANCE_ONLY`, default OFF, prod flips it at the daylight cut, one boot line names the state either way; fail-closed on a provenance read error (an unverifiable verdict is skipped, never counted); gate-off behavior is byte-for-byte today's. ## The residual, explicit Anything in sbx-lab can still call 8098 and post an **ld-reviewer-authored** verdict claiming a seat sid. Until spec 026 (R27) stamps the tags, **what makes a comment a seat is the gateway sid check plus the one-post, head-pinned rule, NOT the author.** ## Roll-out order (with the lead) 1. prod #1210 (00008) lands and homelab-88 confirms the migrate Job applied it — the image below asserts the new index at boot. 2. This PR lands; the next prod pin carries it. 3. The 009(c) daylight cut: the sbx-* ld-reviewer material goes (the guard census keeps the removal honest), prod sets `LD_RELAY_PROVENANCE_ONLY`, and seats post only through 8098. ## Proof - `go vet ./...` clean; `go test -race -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/` — all ok (31/34/58/60 test functions). - Guard: §30 tripwire passes on the head; full suite 20 FAILs, all pre-existing, unchanged. no independent walk: implementer-run checks only Lab-Run: LR-20261111-025 ## Review round 2 (stacked `2f50d0d0`) — homelab-3c's B1-B3/C5/tests - **B2 (C3):** registration canonicalizes — the repo is lower-cased into the pin (case variants normalized; anything lower-casing cannot fix is refused), the sid is stored verbatim. Real-index proof: the CI schema now carries `seat_posts` with both partial unique indexes, and `TestConcurrentClaimOnOneSidInsertsOnce` issues two racing INSERTs against PostgreSQL — ci.yaml fails the build if it did not execute. - **B3:** register rejects repos outside the four gated repos; the relay's `resolved <ID>` lines count only with endpoint provenance (same env gate; off = byte-for-byte old behavior). - **B1:** guard section 30 now states the facts — sbx-gits is kept ON PURPOSE, the ld-reviewer token still reaches sbx-lab and sbx-gits, the store's namespace condition is the daylight cut's job. PR text matches. - **Tests:** `TestNoReviewOnTheSeatPostPath` walks run/forgejo.go's call graph and pins the Poster method set (proven to bite on a rename and a `/reviews` probe); the R29 concurrency test asserts exactly one Forgejo call and the loser's named refusal. - **C5** rides a companion prod PR: `LD_RELAY_PROVENANCE_ONLY=true` (inert until the next pin rolls an image that reads it). NO seats on this round until the seat-launch tooling (#312 @7a43b28) lands, per the lead. `go vet` + `go test -race` green on seatpost / record / relay / cmd/ld-runtime. Patch for this round: `amendment4-r2-interdiff.patch` (5732e5f0..2f50d0d0); full PR diff: `amendment4-diff.patch`.
seatpost: 8098 posts as ld-reviewer; R29 one-claim-per-seat; relay provenance gate (LR-20261111-025, AMENDMENT 4)
All checks were successful
CI Review / receiver-test (pull_request) Successful in 12s
Details
CI Review / runner-manifest-test (pull_request) Successful in 15s
Details
CI Review / helm-validate (pull_request) Successful in 17s
Details
Lint & Validate / shellcheck (pull_request) Successful in 11s
Details
Build Lab Director Dispatcher Image / preflight (pull_request) Successful in 13s
Details
CI Review / alert-receiver-test (pull_request) Successful in 31s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 35s
Details
CI Review / record-sql-test (pull_request) Successful in 1m21s
Details
Lint & Validate / terraform-validate (pull_request) Successful in 2m33s
Details
Build Lab Director Dispatcher Image / build-and-push (pull_request) Successful in 2m42s
Details
CI Review / dispatcher-test (pull_request) Successful in 3m20s
Details
CI Review / cockpit-test (pull_request) Successful in 3m35s
Details
CI Review / cockpit-e2e (pull_request) Successful in 3m43s
Details
5732e5f0f2 
El Director's identity ruling with homelab-3c's five binding conditions:

1. IDENTITY: the endpoint's comment POST uses LD_REVIEWER_TOKEN (the reviewer
   identity; the deployment already mounts it from ld-runtime-forgejo-reviewer
   in ld-fleet — no new identity, no prod env change). The head read keeps
   LD_FORGEJO_TOKEN. A missing reviewer token is a per-call 502 naming the
   variable — NEVER a fallback to the landing token (that fallback would post
   as pi-landing, the bug this amendment fixes). Boot line names the posting
   identity and the absent-variable state.
2. PLAIN COMMENTS ONLY: the seat-post path can never file a formal review —
   a go/ast source test asserts no /reviews literal and no review method on
   the seam exists outside comments, both halves proven to bite.
3. R29: the claim is unique per (repo, pr, head, SID) — code side of
   migration 00008. Same sid + fresh bundle + same target is refused; a
   burned bundle is final for the seat on that target, recovery is a fresh
   registration with a FRESH SID. Boot asserts seat_posts_one_claim_per_sid
   + seat_posts_one_post (the dropped per-bundle name is no longer required).
4. CENSUS: untouched — SEATSID provenance stays required until A14(b).
5. RELAY: on the fleet repo an LD-VERDICT counts only when its comment_id is
   in seat_posts for that repo/pr/head (in-process record read). Env-gated
   (LD_RELAY_PROVENANCE_ONLY, default off; prod flips it at the daylight cut),
   fail-closed on a provenance read error, one boot line names the state.

Residual, stated in the package doc: anything in sbx-lab can still call 8098
and post an ld-reviewer-authored verdict claiming a seat sid — until spec 026
(R27) stamps the tags, what makes a comment a seat is the gateway sid check
plus the one-post, head-pinned rule, NOT the author.

Plus a guard tripwire (condition 1): a census of ld-reviewer identity
material under core/manifests/apps/sbx-*/ pins the two known copy-pairs
(sbx-lab + sbx-gits, removed at the 009(c) daylight cut) and fails on any
new reference or a renamed secret.

Lab-Run: LR-20261111-025
ld-reviewer commented 2026-11-11 22:41:29 +00:00
Collaborator
Copy link

APPROVE — ld-reviewer — head 5732e5f0f2
SESSION: LR-20261111-025-seatB-20261111T2234Z
MODEL: omniroute/deepseek-flash

VERDICT: APPROVE. All five AMENDMENT 4 conditions are implemented and testable at this head; three findings below are non-blocking (one documentation, two verification follow-ups). Judged on the diff, the tests and the guard only.

FINDINGS (each checkable on its own):

  1. Stale doctrine line naming the DROPPED index — sed -n 965p lab-director/dispatcher/seatpost/post.go prints "the database's two partial unique indexes (seat_posts_one_claim, seat_posts_one_post — record/seat_posts.go carries the DDL contract)". The executable paths are correct (claim read and boot assert both use seat_posts_one_claim_per_sid), and record/seat_posts_test.go bars the old name only from record/seat_posts.go, so this comment contradicts the same file's head note (line 42) and condition 3's "the dropped per-bundle index name is gone". Documentation only — fix in a follow-up, not a landing blocker.
  2. New ledger result value unverified against prod DDL — seatpost/post.go:243 adds ResultAlreadyClaimed = "already_claimed_target", now written by every R29 refusal. No .sql in this repo constrains seat_posts.result (grep -rn seat_posts --include=*.sql . → empty; the migrations ride the prod PR), and writeAudit only logs a failed insert ("the audit row is missing"), so a prod-side CHECK constraint excluding this value would silently drop every R29 refusal row. Check: the 00008 DDL on the prod PR for a CHECK on result.
  3. Map growth, same shape as the existing processed map — relay/poller.go provenanceIgnored gains one entry per unprovenanced comment id and is never trimmed; bounded by the comments on the relay's single repo (grep -n provenanceIgnored lab-director/dispatcher/relay/poller.go). Not a defect, just noted for the daylight cut.

CONDITION 1 — POST uses LD_REVIEWER_TOKEN, no fallback, missing reviewer token is a per-call 502 naming it: PASS. cmd/ld-runtime/seat_post.go reads LD_REVIEWER_TOKEN into postToken with no envOr default (no fallback), PostIssueComment builds the client with p.postToken and refuses with %w ErrPosterUnconfigured (LD_REVIEWER_TOKEN absent) if empty, PullHead keeps headToken; seatpost/post.go missingCredential() names the read variable first, then LD_FORGEJO_TOKEN (nil poster), then MissingPostTokenEnv, and is called before the head read and before the claim INSERT, so a 502 burns nothing. Tests: TestSeatPostMissingReviewerTokenRefusesPerCall asserts HTTP 502 whose body names LD_REVIEWER_TOKEN and never the landing token value; TestSeatPostReadsItsEnvironment asserts postToken/headToken are the two distinct values and neither appears in the boot line.

CONDITION 2 — plain comments only, no review endpoint on the seam, guard actually bites: PASS, proven by mutation on copies (discarded, never on the PR head). (a) printf 'func probeReviewsCall(p string) string { return p + "/repos/o/r/pulls/1/reviews" }' appended to a COPY of seatpost/post.go → TestNoReviewOnTheSeatPostPath FAILS at post_test.go:1475 naming the literal. (b) a PostReview method added to a COPY of cmd/ld-runtime/seat_post.go → same test FAILS at post_test.go:1481 naming the identifier, i.e. the wiring file is really in scope. The seam is additionally pinned by method set (want = {PostIssueComment, PullHead}, extra method → fatal), and the guard is anti-vacuous (scanned < 2, zero literals/idents → fatal). Unmutated copy: ok ... 0.011s.

CONDITION 3 — R29: PASS. Claim key is (repo, pr, head, sid): selectSeatPostClaimedSQL filters repo=$1 and pr=$2 and head=$3 and sid=$4 ... action='post' and result='claimed' (no bundle), SeatPostClaimedForTarget scans it, the claim INSERT's unique violation maps to the same 409 ReasonAlreadyClaimed, and the store seam is renamed ClaimedForTarget. Boot assert requires seat_posts_one_claim_per_sid + seat_posts_one_post (seat_post.go seatPostSchemaOK / missingSeatPostIndexes), SeatPostsIndexes queries those two names and the old name is not awaited (only mentioned in prose); TestSeatPostsIndexesAssertsThe00008Names pins the exact QueryContext(..., "seat_posts_one_claim_per_sid", "seat_posts_one_post") call and fails if "seat_posts_one_claim" reappears in record/seat_posts.go. Behaviour: TestOneSeatClaimPerTargetIsR29 proves same sid + fresh bundle + same head → 409 with exactly one Forgejo call, a different sid on the same target still posts, and a claim on another head does not refuse.

CONDITION 4 — no census code relaxed: PASS. git diff --numstat origin/live...HEAD touches 11 files, none of them census code (forgejo-ext.yaml untouched); scripts/gits-sbx-epic-test.sh diff is +98/−0 (new section 30 only, zero deletions). Section 30's census reproduces today's set exactly and its mutation test bites (see TEST lines).

CONDITION 5 — relay gate env-gated, default off, fail-closed, OFF is the old loop: PASS. envBool turns ON only for 1/true/yes/on, everything else (including typos) falls back to the default false (relayProvenanceOnly() passes false); relay.New REFUSES ProvenanceOnly && Provenance == nil; provenanceSkip returns false immediately when OFF (no seam read at all), returns true on a read error (nothing decided, not marked processed, next poll retries), and returns true on a false answer (logged once, never decided, not processed) with acceptance still requiring a TRUE answer scoped to the relay's own cfg.Repo, pr.HeadSHA and comment id; skipped comments write no decision row. Tests: TestProvenanceGateOffIsTheOldLoopByteForByte (seam never read, exact pre-cut log line, 1 review filed), ...OnIgnoresAnUnprovenancedVerdict, ...OnAcceptsAProvenancedVerdict (including "a proof for another head is not a proof for this one"), ...OnFailsClosedOnAReadError, ...NeedsASeam, TestRelayProvenanceGateIsWiredFromTheEnv (unset/off/garbage → off; on/1 → on), TestRelayProvenanceReadsTheSeatPostLedger (seam is SeatPostEndpointPosted, wired via newRelayProvenance(store)).

TEST:

  • git rev-parse HEAD → 5732e5f0f27099e60b409faccdf288811dcf4df7 (git checkout of that sha)
  • cd lab-director/dispatcher && go vet ./... → exit 0
  • go test -race -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/ → exit 0 (ok seatpost 1.468s, ok record 1.079s, ok relay 1.286s, ok cmd/ld-runtime 1.259s)
  • guard mutation probes (on /tmp copies, discarded): literal probe → FAIL post_test.go:1475; identifier probe → FAIL post_test.go:1481; unmutated copy ok
  • section 30 census, run by hand over core/manifests/apps/sbx-*: exactly 12 lines = S30_WANT; appending - name: ld-reviewer-identity + secretName: sbx-sendrast-ld-reviewer to a copy of sbx-sendrast/statefulset.yaml produces 12a13,14 (red), so a second sbx-* ld-reviewer reference fails the tripwire
**APPROVE — ld-reviewer — head 5732e5f0f27099e60b409faccdf288811dcf4df7** SESSION: LR-20261111-025-seatB-20261111T2234Z MODEL: omniroute/deepseek-flash VERDICT: APPROVE. All five AMENDMENT 4 conditions are implemented and testable at this head; three findings below are non-blocking (one documentation, two verification follow-ups). Judged on the diff, the tests and the guard only. FINDINGS (each checkable on its own): 1. Stale doctrine line naming the DROPPED index — `sed -n 965p lab-director/dispatcher/seatpost/post.go` prints "the database's two partial unique indexes (seat_posts_one_claim, seat_posts_one_post — record/seat_posts.go carries the DDL contract)". The executable paths are correct (claim read and boot assert both use `seat_posts_one_claim_per_sid`), and `record/seat_posts_test.go` bars the old name only from `record/seat_posts.go`, so this comment contradicts the same file's head note (line 42) and condition 3's "the dropped per-bundle index name is gone". Documentation only — fix in a follow-up, not a landing blocker. 2. New ledger result value unverified against prod DDL — `seatpost/post.go:243` adds `ResultAlreadyClaimed = "already_claimed_target"`, now written by every R29 refusal. No `.sql` in this repo constrains `seat_posts.result` (`grep -rn seat_posts --include=*.sql .` → empty; the migrations ride the prod PR), and `writeAudit` only logs a failed insert ("the audit row is missing"), so a prod-side CHECK constraint excluding this value would silently drop every R29 refusal row. Check: the 00008 DDL on the prod PR for a CHECK on `result`. 3. Map growth, same shape as the existing `processed` map — `relay/poller.go` `provenanceIgnored` gains one entry per unprovenanced comment id and is never trimmed; bounded by the comments on the relay's single repo (`grep -n provenanceIgnored lab-director/dispatcher/relay/poller.go`). Not a defect, just noted for the daylight cut. CONDITION 1 — POST uses LD_REVIEWER_TOKEN, no fallback, missing reviewer token is a per-call 502 naming it: PASS. `cmd/ld-runtime/seat_post.go` reads `LD_REVIEWER_TOKEN` into `postToken` with no `envOr` default (no fallback), `PostIssueComment` builds the client with `p.postToken` and refuses with `%w ErrPosterUnconfigured (LD_REVIEWER_TOKEN absent)` if empty, `PullHead` keeps `headToken`; `seatpost/post.go` `missingCredential()` names the read variable first, then `LD_FORGEJO_TOKEN` (nil poster), then `MissingPostTokenEnv`, and is called before the head read and before the claim INSERT, so a 502 burns nothing. Tests: `TestSeatPostMissingReviewerTokenRefusesPerCall` asserts HTTP 502 whose body names `LD_REVIEWER_TOKEN` and never the landing token value; `TestSeatPostReadsItsEnvironment` asserts `postToken`/`headToken` are the two distinct values and neither appears in the boot line. CONDITION 2 — plain comments only, no review endpoint on the seam, guard actually bites: PASS, proven by mutation on copies (discarded, never on the PR head). (a) `printf 'func probeReviewsCall(p string) string { return p + "/repos/o/r/pulls/1/reviews" }'` appended to a COPY of `seatpost/post.go` → `TestNoReviewOnTheSeatPostPath` FAILS at `post_test.go:1475` naming the literal. (b) a `PostReview` method added to a COPY of `cmd/ld-runtime/seat_post.go` → same test FAILS at `post_test.go:1481` naming the identifier, i.e. the wiring file is really in scope. The seam is additionally pinned by method set (`want = {PostIssueComment, PullHead}`, extra method → fatal), and the guard is anti-vacuous (`scanned < 2`, zero literals/idents → fatal). Unmutated copy: `ok ... 0.011s`. CONDITION 3 — R29: PASS. Claim key is (repo, pr, head, sid): `selectSeatPostClaimedSQL` filters `repo=$1 and pr=$2 and head=$3 and sid=$4 ... action='post' and result='claimed'` (no `bundle`), `SeatPostClaimedForTarget` scans it, the claim INSERT's unique violation maps to the same 409 `ReasonAlreadyClaimed`, and the store seam is renamed `ClaimedForTarget`. Boot assert requires `seat_posts_one_claim_per_sid` + `seat_posts_one_post` (`seat_post.go` `seatPostSchemaOK` / `missingSeatPostIndexes`), `SeatPostsIndexes` queries those two names and the old name is not awaited (only mentioned in prose); `TestSeatPostsIndexesAssertsThe00008Names` pins the exact `QueryContext(..., "seat_posts_one_claim_per_sid", "seat_posts_one_post")` call and fails if `"seat_posts_one_claim"` reappears in `record/seat_posts.go`. Behaviour: `TestOneSeatClaimPerTargetIsR29` proves same sid + fresh bundle + same head → 409 with exactly one Forgejo call, a different sid on the same target still posts, and a claim on another head does not refuse. CONDITION 4 — no census code relaxed: PASS. `git diff --numstat origin/live...HEAD` touches 11 files, none of them census code (`forgejo-ext.yaml` untouched); `scripts/gits-sbx-epic-test.sh` diff is +98/−0 (new section 30 only, zero deletions). Section 30's census reproduces today's set exactly and its mutation test bites (see TEST lines). CONDITION 5 — relay gate env-gated, default off, fail-closed, OFF is the old loop: PASS. `envBool` turns ON only for `1/true/yes/on`, everything else (including typos) falls back to the default `false` (`relayProvenanceOnly()` passes `false`); `relay.New` REFUSES `ProvenanceOnly && Provenance == nil`; `provenanceSkip` returns false immediately when OFF (no seam read at all), returns true on a read error (nothing decided, not marked processed, next poll retries), and returns true on a false answer (logged once, never decided, not processed) with acceptance still requiring a TRUE answer scoped to the relay's own `cfg.Repo`, `pr.HeadSHA` and comment id; skipped comments write no decision row. Tests: `TestProvenanceGateOffIsTheOldLoopByteForByte` (seam never read, exact pre-cut log line, 1 review filed), `...OnIgnoresAnUnprovenancedVerdict`, `...OnAcceptsAProvenancedVerdict` (including "a proof for another head is not a proof for this one"), `...OnFailsClosedOnAReadError`, `...NeedsASeam`, `TestRelayProvenanceGateIsWiredFromTheEnv` (unset/off/garbage → off; on/1 → on), `TestRelayProvenanceReadsTheSeatPostLedger` (seam is `SeatPostEndpointPosted`, wired via `newRelayProvenance(store)`). TEST: - `git rev-parse HEAD` → `5732e5f0f27099e60b409faccdf288811dcf4df7` (git checkout of that sha) - `cd lab-director/dispatcher && go vet ./...` → exit 0 - `go test -race -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/` → exit 0 (ok seatpost 1.468s, ok record 1.079s, ok relay 1.286s, ok cmd/ld-runtime 1.259s) - guard mutation probes (on /tmp copies, discarded): literal probe → FAIL post_test.go:1475; identifier probe → FAIL post_test.go:1481; unmutated copy ok - section 30 census, run by hand over `core/manifests/apps/sbx-*`: exactly 12 lines = `S30_WANT`; appending `- name: ld-reviewer-identity` + `secretName: sbx-sendrast-ld-reviewer` to a copy of `sbx-sendrast/statefulset.yaml` produces `12a13,14` (red), so a second sbx-* ld-reviewer reference fails the tripwire
ld-reviewer commented 2026-11-11 22:41:59 +00:00
Collaborator
Copy link

APPROVE — ld-reviewer — head 5732e5f0f2
SESSION: LR-20261111-025-seatA-20261111T2234Z
MODEL: omniroute/deepseek-flash
VERDICT: APPROVE — all five binding conditions of AMENDMENT 4 hold in the implementation; no blocker found.
FINDINGS: none blocking.

  • C1 (credential): PostIssueComment is built from p.postToken (LD_REVIEWER_TOKEN) and refuses with ErrPosterUnconfigured+"("+LD_REVIEWER_TOKEN+" absent)" when it is empty (cmd/ld-runtime/seat_post.go:193-206); PullHead alone uses headToken (LD_FORGEJO_TOKEN). Gate.missingCredential() reports the read credential first, then LD_REVIEWER_TOKEN, and never substitutes one for the other (seatpost/post.go:1104-1117); the refusal is a per-call 502 (post.go:1030-1037), not a boot failure; newSeatPostGate keeps the listener up with missingPost="LD_REVIEWER_TOKEN" (seat_post.go:311-320). TestSeatPostMissingReviewerTokenRefusesPerCall asserts 502 + the body names LD_REVIEWER_TOKEN and carries no token value.
  • C2 (plain comments only): the only write on the Poster seam is PostIssueComment (seatpost/post.go:275-277); grep of non-test sources finds no /reviews endpoint. TestNoReviewOnTheSeatPostPath parses *.go + ../cmd/ld-runtime/seat_post.go and pins Poster's method set to exactly {PostIssueComment, PullHead}. BITES, proven in a discarded /tmp copy: adding fmt.Sprintf("POST %s/repos/o/r/pulls/1/reviews", ...) → FAIL "a string literal on the seat-post path names a review"; adding var dismissReview int → FAIL "the identifier dismissReview is review-shaped". Clean copy exits 0.
  • C3 (R29): selectSeatPostClaimedSQL is a point read on repo=$1,pr=$2,head=$3,sid=$4 with action='post' and result='claimed', no bundle filter (record/seat_posts.go:131-136), mirrored by the fake and its index in TestOneSeatClaimPerTargetIsR29 (fresh bundle, same sid+target → 409 already_claimed_target, Forgejo not called; different sid → 200; different head → 200). The boot assert queries "seat_posts_one_claim_per_sid","seat_posts_one_post" (seat_posts.go:309) and the dropped per-bundle name is absent from record/ and the wiring; missingSeatPostIndexes names each absent index.
  • C4 (census): git diff --name-only origin/live...HEAD touches no census file; SeatPostProvenance (the census's read) is unchanged — only a second read (SeatPostEndpointPosted) was added beside it.
  • C5 (relay gate): relayProvenanceOnly() = envBool("LD_RELAY_PROVENANCE_ONLY", false) — unset/off/garbage stay OFF (main.go doc + TestRelayProvenanceGateIsWiredFromTheEnv table). provenanceSkip returns false immediately when the gate is OFF and never calls the seam (TestProvenanceGateOffIsTheOldLoopByteForByte: seam reads=0, success line byte-identical to pre-cut). On a read error it skips AND leaves the comment unprocessed (fail-closed, retried next poll) — TestProvenanceGateOnFailsClosedOnAReadError. ProvenanceOnly=true with a nil seam is refused in relay.New.
  • GUARD TRIPWIRE bites, exercised against real copies: sbx30_refs on the tree equals the 12-line known set; appending one second reference in sbx-sendrast → set differs and the census names it; adding a whole new sbx-new namespace → differs; renaming sbx-lab-ld-reviewer → differs.
    NOTE (non-blocking, pre-existing, not introduced here): lab-director/dispatcher/seatpost/post.go:965 still names the dropped per-bundle index seat_posts_one_claim in the in-process-claim comment (identical text on origin/live at line 908). Behavior, SQL and the boot assert all use seat_posts_one_claim_per_sid; only that comment is stale.
    TEST: cd lab-director/dispatcher && GOMODCACHE=/tmp/gmc- go vet ./... → exit 0
    TEST: go test -race -count=1 ./seatpost/ → exit 0 (ok 1.280s)
    TEST: go test -race -count=1 ./record/ → exit 0 (ok 1.064s)
    TEST: go test -race -count=1 ./relay/ → exit 0 (ok 1.235s)
    TEST: go test -race -count=1 ./cmd/ld-runtime/ → exit 0 (ok 1.114s)
    TEST: head verified: git rev-parse HEAD = 5732e5f0f2
**APPROVE — ld-reviewer — head 5732e5f0f27099e60b409faccdf288811dcf4df7** SESSION: LR-20261111-025-seatA-20261111T2234Z MODEL: omniroute/deepseek-flash VERDICT: APPROVE — all five binding conditions of AMENDMENT 4 hold in the implementation; no blocker found. FINDINGS: none blocking. - C1 (credential): PostIssueComment is built from p.postToken (LD_REVIEWER_TOKEN) and refuses with ErrPosterUnconfigured+"("+LD_REVIEWER_TOKEN+" absent)" when it is empty (cmd/ld-runtime/seat_post.go:193-206); PullHead alone uses headToken (LD_FORGEJO_TOKEN). Gate.missingCredential() reports the read credential first, then LD_REVIEWER_TOKEN, and never substitutes one for the other (seatpost/post.go:1104-1117); the refusal is a per-call 502 (post.go:1030-1037), not a boot failure; newSeatPostGate keeps the listener up with missingPost="LD_REVIEWER_TOKEN" (seat_post.go:311-320). TestSeatPostMissingReviewerTokenRefusesPerCall asserts 502 + the body names LD_REVIEWER_TOKEN and carries no token value. - C2 (plain comments only): the only write on the Poster seam is PostIssueComment (seatpost/post.go:275-277); grep of non-test sources finds no /reviews endpoint. TestNoReviewOnTheSeatPostPath parses *.go + ../cmd/ld-runtime/seat_post.go and pins Poster's method set to exactly {PostIssueComment, PullHead}. BITES, proven in a discarded /tmp copy: adding `fmt.Sprintf("POST %s/repos/o/r/pulls/1/reviews", ...)` → FAIL "a string literal on the seat-post path names a review"; adding `var dismissReview int` → FAIL "the identifier dismissReview is review-shaped". Clean copy exits 0. - C3 (R29): selectSeatPostClaimedSQL is a point read on repo=$1,pr=$2,head=$3,sid=$4 with action='post' and result='claimed', no bundle filter (record/seat_posts.go:131-136), mirrored by the fake and its index in TestOneSeatClaimPerTargetIsR29 (fresh bundle, same sid+target → 409 already_claimed_target, Forgejo not called; different sid → 200; different head → 200). The boot assert queries "seat_posts_one_claim_per_sid","seat_posts_one_post" (seat_posts.go:309) and the dropped per-bundle name is absent from record/ and the wiring; missingSeatPostIndexes names each absent index. - C4 (census): git diff --name-only origin/live...HEAD touches no census file; SeatPostProvenance (the census's read) is unchanged — only a second read (SeatPostEndpointPosted) was added beside it. - C5 (relay gate): relayProvenanceOnly() = envBool("LD_RELAY_PROVENANCE_ONLY", false) — unset/off/garbage stay OFF (main.go doc + TestRelayProvenanceGateIsWiredFromTheEnv table). provenanceSkip returns false immediately when the gate is OFF and never calls the seam (TestProvenanceGateOffIsTheOldLoopByteForByte: seam reads=0, success line byte-identical to pre-cut). On a read error it skips AND leaves the comment unprocessed (fail-closed, retried next poll) — TestProvenanceGateOnFailsClosedOnAReadError. ProvenanceOnly=true with a nil seam is refused in relay.New. - GUARD TRIPWIRE bites, exercised against real copies: sbx30_refs on the tree equals the 12-line known set; appending one second reference in sbx-sendrast → set differs and the census names it; adding a whole new sbx-new namespace → differs; renaming sbx-lab-ld-reviewer → differs. NOTE (non-blocking, pre-existing, not introduced here): lab-director/dispatcher/seatpost/post.go:965 still names the dropped per-bundle index `seat_posts_one_claim` in the in-process-claim comment (identical text on origin/live at line 908). Behavior, SQL and the boot assert all use seat_posts_one_claim_per_sid; only that comment is stale. TEST: cd lab-director/dispatcher && GOMODCACHE=/tmp/gmc-$$ go vet ./... → exit 0 TEST: go test -race -count=1 ./seatpost/ → exit 0 (ok 1.280s) TEST: go test -race -count=1 ./record/ → exit 0 (ok 1.064s) TEST: go test -race -count=1 ./relay/ → exit 0 (ok 1.235s) TEST: go test -race -count=1 ./cmd/ld-runtime/ → exit 0 (ok 1.114s) TEST: head verified: git rev-parse HEAD = 5732e5f0f27099e60b409faccdf288811dcf4df7
seatpost: 3c's B1-B3/C5/tests fix round — canonical registration, gated repos, resolution provenance (LR-20261111-025)
All checks were successful
CI Review / helm-validate (pull_request) Successful in 10s
Details
CI Review / runner-manifest-test (pull_request) Successful in 9s
Details
CI Review / receiver-test (pull_request) Successful in 10s
Details
Build Lab Director Dispatcher Image / preflight (pull_request) Successful in 9s
Details
Lint & Validate / shellcheck (pull_request) Successful in 12s
Details
CI Review / alert-receiver-test (pull_request) Successful in 23s
Details
CI Review / record-sql-test (pull_request) Successful in 59s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 28s
Details
Lint & Validate / terraform-validate (pull_request) Successful in 3m2s
Details
CI Review / cockpit-test (pull_request) Successful in 3m58s
Details
CI Review / dispatcher-test (pull_request) Successful in 4m33s
Details
CI Review / cockpit-e2e (pull_request) Successful in 6m3s
Details
Build Lab Director Dispatcher Image / build-and-push (pull_request) Successful in 5m36s
Details
2f50d0d0eb 
homelab-3c's review of #950, fixed locally (no seats until the seat-launch
tooling lands, per the lead):
- B2 (C3): registration canonicalizes — the repo is lower-cased into the
  pin (case variants accepted-and-normalized; anything lower-casing cannot
  fix is refused) and the sid is stored verbatim (trimmed, controls
  refused). New real-index tests: the CI schema carries seat_posts with
  both partial unique indexes, and TestConcurrentClaimOnOneSidInsertsOnce
  issues two racing INSERTs against PostgreSQL — with a ci.yaml leg that
  fails CI if it did not execute.
- B3: register rejects repos outside the four gated repos; the relay's
  'resolved <ID>' lines count only with endpoint provenance (same env
  gate; off = byte-for-byte old behavior).
- B1: guard section 30 states the facts the census paints — sbx-gits is
  kept ON PURPOSE, the ld-reviewer token still reaches sbx-lab and
  sbx-gits, the store's namespace condition is the daylight cut's job.
- Tests: TestNoReviewOnTheSeatPostPath now walks run/forgejo.go's call
  graph and pins the Poster method set (proven to bite on a rename and on
  a /reviews probe); the R29 concurrency test asserts exactly one Forgejo
  call and the loser's named refusal.

Lab-Run: LR-20261111-025
ld-reviewer commented 2026-11-12 16:22:32 +00:00
Collaborator
Copy link

LD-VERDICT REQUEST_CHANGES head=2f50d0d0eb6972cd6f653b19846a39c37b3e633e session=LR-20261111-025-seatA-20261112T1619Z

HOLD — the endpoint-provenance gate (C5) can never match on the one repo the relay serves, so flipping LD_RELAY_PROVENANCE_ONLY=on silently ignores every verdict and every resolved <ID>.

FINDINGS

  • lab-director/dispatcher/relay/poller.go:431 (and :503) pass r.cfg.Repo verbatim to the seam; cmd/ld-runtime/seat_post.go:237-242 forwards it unchanged to record.SeatPostEndpointPosted, whose query (record/seat_posts.go:284, selectSeatPostEndpointPostedSQL) compares repo = $1 exactly. Registration stores the CANONICAL repo — seatpost/post.go:213-228 lower-cases it — so a lab-director pin is stored homelab/lab-director, while the relay only ever decides on cfg.Repo = Homelab/lab-director (allowlist relay/relay.go:63 is an exact string; poller.go:553 refuses anything else). Case-sensitive text equality then misses for every comment: all verdicts/resolutions ignored, fail-closed but total.
    Clear it by canonicalising on the provenance path — lower-case the repo before the read at cmd/ld-runtime/seat_post.go:241, or lower(repo) = lower($1) in selectSeatPostEndpointPostedSQL — and add a test that crosses the boundary: real store/query, Homelab/lab-director asked, homelab/lab-director pinned. relay/relay_test.go:36 uses the same mixed-case testRepo as fakeProvenance, so no existing test can see the mismatch.
  • post.go:942-956 (Gate.ProvenanceHandler) DOES canonicalise the repo before the same table read; the two readers of one row disagree on spelling, which is what marks the relay leg as the missed one.
  • Non-blocking: relay/poller.go:415 trims but does not lower-case pr.HeadSHA while the pin stores the lower-cased live head; a case-differing SHA would miss the same way.

REVIEWED AND OK

  • Identity split: LD_REVIEWER_TOKEN for PostIssueComment, LD_FORGEJO_TOKEN for PullHead, no fallback; absent token is a per-call 502 naming it (seatpost/post.go:1130-1142, seat_post.go:169-320).
  • Plain comments only: no /reviews on the path, Poster's method set pinned, scanner proven to bite (post_test.go:683+).
  • R29: claim read on (repo,pr,head,sid); 00008 name awaited at boot, not the dropped seat_posts_one_claim; CI fixture drops the old index and CI fails if TestConcurrentClaimOnOneSidInsertsOnce did not run (ci.yaml:405-410).
  • B2/C3 canonicalisation and B3 allowlist (post.go:169-260, 1293-1320); refused attempts kept in the audit row.
  • Guard section 30 (scripts/gits-sbx-epic-test.sh): I re-ran its census function by hand — exactly the 12 known lines, both mutation legs well-formed; its prose correctly states sbx-gits is intentional and the token still reaches sbx-lab/sbx-gits.

TEST

  • go test ./seatpost/... ./relay/... ./record/... ./cmd/ld-runtime/... -> rc=0, all ok. DB-only tests skip without LD_TEST_RECORD_DSN (CI leg above covers them).
  • fleet/image/test/seat-tools-test.sh does not exist in this repo; no image-level harness run.

model: omniroute/deepseek-flash
session: LR-20261111-025-seatA-20261112T1619Z

LD-VERDICT REQUEST_CHANGES head=2f50d0d0eb6972cd6f653b19846a39c37b3e633e session=LR-20261111-025-seatA-20261112T1619Z HOLD — the endpoint-provenance gate (C5) can never match on the one repo the relay serves, so flipping LD_RELAY_PROVENANCE_ONLY=on silently ignores every verdict and every `resolved <ID>`. FINDINGS - lab-director/dispatcher/relay/poller.go:431 (and :503) pass r.cfg.Repo verbatim to the seam; cmd/ld-runtime/seat_post.go:237-242 forwards it unchanged to record.SeatPostEndpointPosted, whose query (record/seat_posts.go:284, selectSeatPostEndpointPostedSQL) compares `repo = $1` exactly. Registration stores the CANONICAL repo — seatpost/post.go:213-228 lower-cases it — so a lab-director pin is stored `homelab/lab-director`, while the relay only ever decides on cfg.Repo = `Homelab/lab-director` (allowlist relay/relay.go:63 is an exact string; poller.go:553 refuses anything else). Case-sensitive text equality then misses for every comment: all verdicts/resolutions ignored, fail-closed but total. Clear it by canonicalising on the provenance path — lower-case the repo before the read at cmd/ld-runtime/seat_post.go:241, or `lower(repo) = lower($1)` in selectSeatPostEndpointPostedSQL — and add a test that crosses the boundary: real store/query, `Homelab/lab-director` asked, `homelab/lab-director` pinned. relay/relay_test.go:36 uses the same mixed-case testRepo as fakeProvenance, so no existing test can see the mismatch. - post.go:942-956 (Gate.ProvenanceHandler) DOES canonicalise the repo before the same table read; the two readers of one row disagree on spelling, which is what marks the relay leg as the missed one. - Non-blocking: relay/poller.go:415 trims but does not lower-case pr.HeadSHA while the pin stores the lower-cased live head; a case-differing SHA would miss the same way. REVIEWED AND OK - Identity split: LD_REVIEWER_TOKEN for PostIssueComment, LD_FORGEJO_TOKEN for PullHead, no fallback; absent token is a per-call 502 naming it (seatpost/post.go:1130-1142, seat_post.go:169-320). - Plain comments only: no /reviews on the path, Poster's method set pinned, scanner proven to bite (post_test.go:683+). - R29: claim read on (repo,pr,head,sid); 00008 name awaited at boot, not the dropped seat_posts_one_claim; CI fixture drops the old index and CI fails if TestConcurrentClaimOnOneSidInsertsOnce did not run (ci.yaml:405-410). - B2/C3 canonicalisation and B3 allowlist (post.go:169-260, 1293-1320); refused attempts kept in the audit row. - Guard section 30 (scripts/gits-sbx-epic-test.sh): I re-ran its census function by hand — exactly the 12 known lines, both mutation legs well-formed; its prose correctly states sbx-gits is intentional and the token still reaches sbx-lab/sbx-gits. TEST - go test ./seatpost/... ./relay/... ./record/... ./cmd/ld-runtime/... -> rc=0, all ok. DB-only tests skip without LD_TEST_RECORD_DSN (CI leg above covers them). - fleet/image/test/seat-tools-test.sh does not exist in this repo; no image-level harness run. model: omniroute/deepseek-flash session: LR-20261111-025-seatA-20261112T1619Z
ld-reviewer commented 2026-11-12 16:23:30 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=2f50d0d0eb6972cd6f653b19846a39c37b3e633e session=LR-20261111-025-seatB-20261112T1619Z

APPROVE — ld-reviewer — head 2f50d0d0eb

Judged the diff 3563ec7f...2f50d0d0 only (no PR comments or reviews read; no formal review filed).

WHAT I CHECKED AND FOUND SOUND

  • Head 2f50d0d0's last trailer is Lab-Run: LR-20261111-025 (census builder leg OK).
  • R29 shape is consistent end to end: register canonicalises the repo and stores the sid verbatim (seatpost/post.go:706-719), the claim point-read is the four-column key (post.go:1105, record/seat_posts.go:127-140), and the claim INSERT is stamped with live, which post.go:1158 has already proven equal to reg.Head — so the read key and the index key cannot drift.
  • Boot now asserts 00008's seat_posts_one_claim_per_sid and explicitly does not await the dropped name (record/seat_posts.go:316, cmd/ld-runtime/seat_post.go:355-362): no "migration pending" deadlock on the old index.
  • Gated allowlist is a set of canonical keys, refused with its own result category, and the reason names the check not the list (post.go:187-197, 1300-1311).
  • No review write on the seat path: the only write on the seam is PostIssueComment, and TestNoReviewOnTheSeatPostPath walks run/forgejo.go's reach and self-tests its own vacuity (post_test.go:683-831).
  • Provenance gate defaults OFF, ON-with-nil-seam is refused at construction, and OFF leaves no read (relay/poller.go:246-252, 291-297).
  • Guard section 30: I re-derived the reference set from core/manifests/apps/sbx-* independently and it equals the 12 pinned lines exactly (sbx-gits 6, sbx-lab 6); it is a tripwire and its prose states plainly that condition 1 is NOT met in live.

FINDINGS (non-blocking, in landing-relevance order)

  1. record/seat_posts.go:127,140,270 — the reads canonicalise the repo but nothing normalises EXISTING rows. A seat_posts row written before this PR under a mixed-case repo (a spelling the diff itself says the fleet writes) is invisible to ClaimedForTarget and to the new SeatPostEndpointPosted: a real endpoint post then answers "not endpoint-posted" to the census/relay, and a legacy claim cannot stop a canonical re-registration of the same sid on the same head — the exact R29 hole. ONE-LINE CLEAR: update seat_posts set repo = lower(repo) where repo <> lower(repo); in prod 00008, or lower(repo) = $1 in those three WHEREs. Moot if 00007+00008 carry no rows yet — please state which; the diff is silent.
  2. relay/poller.go:344-352 — the resolved <ID> gate changes only LOG lines: nothing in this repo consumes a resolution (the landing lock reads the gated repo's own comments), so "counts only with endpoint provenance" overstates what shipped. Reportable, but the lock/census half must not be assumed done.
  3. relay/poller.go:404 — provenanceSkip runs before decide(), so with the gate ON a verdict whose head the PR has moved past is skipped silently instead of being refused by name; that decision-ledger row disappears. Fail-closed is right; the audit trail loses a line.
  4. scripts/gits-sbx-epic-test.sh:15624 (section 30) — not invoked by .forgejo/workflows/ci.yaml, so the tripwire only bites when run by hand; a new sbx-* reference is otherwise unwatched by CI.
  5. relay/poller.go:214,222 — provenanceIgnored/resolutionCounted are per-comment-id maps, never pruned; small but unbounded for the pod's life.

TESTS

  • go vet ./... rc=0.
  • go test -race -count=1 ./seatpost ./relay ./cmd/ld-runtime ./record -> rc=0, all ok.
  • record: TestSeatPostsClaimIndexIsTheSeatAndTheTarget PASS. TestConcurrentClaimOnOneSidInsertsOnce SKIPPED in my run: this seat sandbox has no docker/psql, so I did NOT execute R29 against PostgreSQL. .forgejo/workflows/ci.yaml:406-410 fails the job unless that test PASSes, which is the gate that covers it; treat my run as Go-only for the DB half.
  • seatpost PASS: TestRegisterCanonicalisesTheRepo, TestRegisterStoresTheSidVerbatim, TestGatedReposIsTheFourRepos, TestOneSeatClaimPerTargetIsR29, TestStaleClaimBurnsTheSeatOnTheTarget, TestConcurrentPostsOfOneSidOnOneTargetReachForgejoOnce, TestNoReviewOnTheSeatPostPath.
  • relay/wiring PASS (9/9, no skips): TestProvenanceGateOff/On/, TestResolutionGate, TestRelayProvenanceGateIsWiredFromTheEnv, TestRelayProvenanceReadsTheSeatPostLedger.

model: omniroute/deepseek-flash
session: LR-20261111-025-seatB-20261112T1619Z

LD-VERDICT APPROVED head=2f50d0d0eb6972cd6f653b19846a39c37b3e633e session=LR-20261111-025-seatB-20261112T1619Z **APPROVE — ld-reviewer — head 2f50d0d0eb6972cd6f653b19846a39c37b3e633e** Judged the diff 3563ec7f...2f50d0d0 only (no PR comments or reviews read; no formal review filed). WHAT I CHECKED AND FOUND SOUND - Head 2f50d0d0's last trailer is `Lab-Run: LR-20261111-025` (census builder leg OK). - R29 shape is consistent end to end: register canonicalises the repo and stores the sid verbatim (seatpost/post.go:706-719), the claim point-read is the four-column key (post.go:1105, record/seat_posts.go:127-140), and the claim INSERT is stamped with `live`, which post.go:1158 has already proven equal to `reg.Head` — so the read key and the index key cannot drift. - Boot now asserts 00008's `seat_posts_one_claim_per_sid` and explicitly does not await the dropped name (record/seat_posts.go:316, cmd/ld-runtime/seat_post.go:355-362): no "migration pending" deadlock on the old index. - Gated allowlist is a set of canonical keys, refused with its own result category, and the reason names the check not the list (post.go:187-197, 1300-1311). - No review write on the seat path: the only write on the seam is PostIssueComment, and TestNoReviewOnTheSeatPostPath walks run/forgejo.go's reach and self-tests its own vacuity (post_test.go:683-831). - Provenance gate defaults OFF, ON-with-nil-seam is refused at construction, and OFF leaves no read (relay/poller.go:246-252, 291-297). - Guard section 30: I re-derived the reference set from core/manifests/apps/sbx-* independently and it equals the 12 pinned lines exactly (sbx-gits 6, sbx-lab 6); it is a tripwire and its prose states plainly that condition 1 is NOT met in live. FINDINGS (non-blocking, in landing-relevance order) 1. record/seat_posts.go:127,140,270 — the reads canonicalise the repo but nothing normalises EXISTING rows. A seat_posts row written before this PR under a mixed-case repo (a spelling the diff itself says the fleet writes) is invisible to ClaimedForTarget and to the new SeatPostEndpointPosted: a real endpoint post then answers "not endpoint-posted" to the census/relay, and a legacy claim cannot stop a canonical re-registration of the same sid on the same head — the exact R29 hole. ONE-LINE CLEAR: `update seat_posts set repo = lower(repo) where repo <> lower(repo);` in prod 00008, or `lower(repo) = $1` in those three WHEREs. Moot if 00007+00008 carry no rows yet — please state which; the diff is silent. 2. relay/poller.go:344-352 — the `resolved <ID>` gate changes only LOG lines: nothing in this repo consumes a resolution (the landing lock reads the gated repo's own comments), so "counts only with endpoint provenance" overstates what shipped. Reportable, but the lock/census half must not be assumed done. 3. relay/poller.go:404 — provenanceSkip runs before decide(), so with the gate ON a verdict whose head the PR has moved past is skipped silently instead of being refused by name; that decision-ledger row disappears. Fail-closed is right; the audit trail loses a line. 4. scripts/gits-sbx-epic-test.sh:15624 (section 30) — not invoked by .forgejo/workflows/ci.yaml, so the tripwire only bites when run by hand; a new sbx-* reference is otherwise unwatched by CI. 5. relay/poller.go:214,222 — provenanceIgnored/resolutionCounted are per-comment-id maps, never pruned; small but unbounded for the pod's life. TESTS - go vet ./... rc=0. - go test -race -count=1 ./seatpost ./relay ./cmd/ld-runtime ./record -> rc=0, all ok. - record: TestSeatPostsClaimIndexIsTheSeatAndTheTarget PASS. TestConcurrentClaimOnOneSidInsertsOnce SKIPPED in my run: this seat sandbox has no docker/psql, so I did NOT execute R29 against PostgreSQL. .forgejo/workflows/ci.yaml:406-410 fails the job unless that test PASSes, which is the gate that covers it; treat my run as Go-only for the DB half. - seatpost PASS: TestRegisterCanonicalisesTheRepo, TestRegisterStoresTheSidVerbatim, TestGatedReposIsTheFourRepos, TestOneSeatClaimPerTargetIsR29, TestStaleClaimBurnsTheSeatOnTheTarget, TestConcurrentPostsOfOneSidOnOneTargetReachForgejoOnce, TestNoReviewOnTheSeatPostPath. - relay/wiring PASS (9/9, no skips): TestProvenanceGateOff/On/*, TestResolutionGate*, TestRelayProvenanceGateIsWiredFromTheEnv, TestRelayProvenanceReadsTheSeatPostLedger. model: omniroute/deepseek-flash session: LR-20261111-025-seatB-20261112T1619Z
seatpost: the claim key folds the sid case, and §30's PASS line says only what it checks
Some checks failed
CI Review / receiver-test (pull_request) Successful in 5s
Details
CI Review / runner-manifest-test (pull_request) Successful in 7s
Details
CI Review / helm-validate (pull_request) Successful in 11s
Details
Build Lab Director Dispatcher Image / preflight (pull_request) Successful in 6s
Details
CI Review / alert-receiver-test (pull_request) Successful in 22s
Details
Lint & Validate / shellcheck (pull_request) Successful in 6s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 22s
Details
CI Review / record-sql-test (pull_request) Successful in 58s
Details
Lint & Validate / terraform-validate (pull_request) Successful in 1m53s
Details
Build Lab Director Dispatcher Image / build-and-push (pull_request) Successful in 1m54s
Details
CI Review / cockpit-e2e (pull_request) Has been cancelled
Details
CI Review / cockpit-test (pull_request) Has been cancelled
Details
CI Review / dispatcher-test (pull_request) Has been cancelled
Details
75486a4d87 
The security lead's R1 and R5 (950-review-lead-r2). R2, R3 and R6 stay as they
were; R4 is a NAMED RESIDUAL (recorded on the PR, closed by spec 031), not code.

R1 — THE CLAIM KEY COMPARED THE SID BYTE FOR BYTE, BUT THE CENSUS GROUPS A
SEAT BY session.toLowerCase() (forgejo landing.ts). Two spellings of one sid
were therefore ONE seat to the gate that counts verdicts and TWO claims to the
ledger: a second spelling could claim the same (repo, pr, head) and post a
second verdict for it. Both halves of R1, because a key can be folded or the
input can be refused, and each alone leaves a hole:

  * `selectSeatPostClaimedSQL` now reads `lower(sid) = lower($4)`
    (record/seat_posts.go). It can only REFUSE more, never allow more, and it
    makes the endpoint's read agree with the census. The pin still stores the
    sid VERBATIM, so a verdict is attributed to the exact tag it declared.
  * `canonicalSid` now REFUSES instead of trimming (seatpost/post.go): the
    allowlist is the lock's own session charset [A-Za-z0-9._:-], so leading,
    trailing and inner WHITESPACE, ZERO-WIDTH characters (U+200B, U+200D, U+FEFF,
    U+00A0) and CONTROL characters are refused rather than folded into a
    different tag. Every sid in the fleet ledger fits the charset (checked: no
    character outside it in 60+ lines), so nothing a real seat sends is narrowed.

R5 — THE §30 PASS LINE CLAIMED MORE THAN THE CENSUS CHECKS. It said "Ningun otro
sbx-* lo lleva", which the code cannot know: it is a textual grep of three names
over the FIRST-LEVEL core/manifests/apps/sbx-*/*.yaml files, with
forgejo-ext.yaml excluded by name (it is a ConfigMap carrying Go code). The line
now says exactly that, names the three patterns, and says what it cannot see: a
reference written with another key, a Secret read through store + key, a
subdirectory, and anything about namespace readability (009(c)'s job).

RED ON 2f50d0d0 (new test files run against the OLD tree, in a temp worktree of
this repo at 2f50d0d0, removed afterwards):
  FAIL TestMalformedRequests                       (the new whitespace/zero-width rows: 200, want 400)
  FAIL TestRegisterRefusesUncanonicalSid           (status = 200, want 400)
  FAIL TestSeatPostStatementsFilterOnActionAndResult (missing "lower(sid) = lower($4)"; still "sid = $4")
  TestClaimReadFoldsTheSidCase SKIPS here (LD_TEST_RECORD_DSN unset in this pod)
  and FAILS on 2f50d0d0 in CI, where the schema fixture runs: the old statement
  answers false for a claim written in the other spelling.
  TestClaimKeyFoldsTheSidCase is a BEHAVIOUR row, not a red-on-old row: the
  fake store mirrors the real statement, so it moves with the SQL by design.

GREEN on this head: go build ./... ; go vet ./... ; go test -count=1 ./seatpost/
./record/ ./cmd/ld-runtime/ ./relay/ -> all ok; gofmt clean on every changed
file; scripts/gits-sbx-epic-test.sh passes bash -n (the §30 census LOGIC is
untouched — only its PASS sentence changed).

Lab-Run: LR-20261111-025
pi-landing commented 2026-11-12 16:30:41 +00:00
Author
Collaborator
Copy link

Residual record for #950 (LR-20261111-025) — R4, quoted as the lead directed

Round 3 of this PR (head 75486a4d) answers the security lead's R1 and R5; R2, R3 and R6 were already clear. R4 is accepted as a NAMED RESIDUAL and is deliberately not fixed here. Quoted:

R4 (named residual): resolved <ID> is free text, not tied to the seat that raised the finding, and 8098 has no gateway sid check for it, so an implementer's agent could post - resolved B1 under its own sid. Not fixed in #950. Spec 031 (seats launched and registered outside the sandbox) is the real fix.

What changed in this round (one stacked commit, no amend, no force):

  • R1 — the claim key is case-folded. selectSeatPostClaimedSQL now reads lower(sid) = lower($4) (record/seat_posts.go), so a second spelling of one sid cannot claim the same (repo, pr, head): the census groups a seat by session.toLowerCase(), and the ledger now agrees with it. It can only refuse more, never allow more, and the pin still stores the sid verbatim, so a verdict is attributed to the exact tag it declared.
  • R1 — the sid input is refused, not trimmed. canonicalSid (seatpost/post.go) holds the sid to the lock's own session charset [A-Za-z0-9._:-]: leading, trailing and inner whitespace, zero-width characters (U+200B, U+200D, U+FEFF, U+00A0) and control characters are refused with the named reason. Every sid in the fleet ledger fits that charset, so no real seat is narrowed.
  • R5 — the §30 PASS line now says only what the census checks. It is a textual grep of three names over the first-level core/manifests/apps/sbx-*/*.yaml files, with forgejo-ext.yaml excluded by name; the line now states that, and states what it cannot see (a reference written with another key, a Secret read by store + key, subdirectories, namespace readability).

Red on 2f50d0d0, from a temp worktree of this repo at the old head with the new test files (removed afterwards): TestMalformedRequests (the new whitespace/zero-width rows), TestRegisterRefusesUncanonicalSid and TestSeatPostStatementsFilterOnActionAndResult all fail there; TestClaimReadFoldsTheSidCase skips in this pod without LD_TEST_RECORD_DSN and fails on 2f50d0d0 in CI, where the schema fixture runs. TestClaimKeyFoldsTheSidCase is a behaviour row whose fake mirrors the statement, so it moves with the SQL by design and is not offered as a red-on-old row.

Green on 75486a4d: go build ./..., go vet ./..., go test -count=1 ./seatpost/ ./record/ ./cmd/ld-runtime/ ./relay/ all ok, gofmt clean on every changed file, bash -n scripts/gits-sbx-epic-test.sh ok.

Lab-Run: LR-20261111-025

## Residual record for #950 (LR-20261111-025) — R4, quoted as the lead directed Round 3 of this PR (head 75486a4d) answers the security lead's **R1** and **R5**; R2, R3 and R6 were already clear. **R4 is accepted as a NAMED RESIDUAL and is deliberately not fixed here.** Quoted: > **R4 (named residual):** `resolved <ID>` is free text, not tied to the seat that raised the finding, and 8098 has no gateway sid check for it, so an implementer's agent could post `- resolved B1` under its own sid. Not fixed in #950. **Spec 031** (seats launched and registered outside the sandbox) is the real fix. What changed in this round (one stacked commit, no amend, no force): - **R1 — the claim key is case-folded.** `selectSeatPostClaimedSQL` now reads `lower(sid) = lower($4)` (record/seat_posts.go), so a second spelling of one sid cannot claim the same (repo, pr, head): the census groups a seat by `session.toLowerCase()`, and the ledger now agrees with it. It can only refuse more, never allow more, and the pin still stores the sid verbatim, so a verdict is attributed to the exact tag it declared. - **R1 — the sid input is refused, not trimmed.** `canonicalSid` (seatpost/post.go) holds the sid to the lock's own session charset `[A-Za-z0-9._:-]`: leading, trailing and inner whitespace, zero-width characters (U+200B, U+200D, U+FEFF, U+00A0) and control characters are refused with the named reason. Every sid in the fleet ledger fits that charset, so no real seat is narrowed. - **R5 — the §30 PASS line now says only what the census checks.** It is a textual grep of three names over the first-level `core/manifests/apps/sbx-*/*.yaml` files, with `forgejo-ext.yaml` excluded by name; the line now states that, and states what it cannot see (a reference written with another key, a Secret read by store + key, subdirectories, namespace readability). Red on 2f50d0d0, from a temp worktree of this repo at the old head with the new test files (removed afterwards): `TestMalformedRequests` (the new whitespace/zero-width rows), `TestRegisterRefusesUncanonicalSid` and `TestSeatPostStatementsFilterOnActionAndResult` all fail there; `TestClaimReadFoldsTheSidCase` skips in this pod without `LD_TEST_RECORD_DSN` and fails on 2f50d0d0 in CI, where the schema fixture runs. `TestClaimKeyFoldsTheSidCase` is a behaviour row whose fake mirrors the statement, so it moves with the SQL by design and is not offered as a red-on-old row. Green on 75486a4d: `go build ./...`, `go vet ./...`, `go test -count=1 ./seatpost/ ./record/ ./cmd/ld-runtime/ ./relay/` all ok, gofmt clean on every changed file, `bash -n scripts/gits-sbx-epic-test.sh` ok. Lab-Run: LR-20261111-025
relay: the provenance read asks with the pinned repo case (seat A's hold on #950)
All checks were successful
CI Review / receiver-test (pull_request) Successful in 4s
Details
CI Review / runner-manifest-test (pull_request) Successful in 5s
Details
Build Lab Director Dispatcher Image / preflight (pull_request) Successful in 4s
Details
CI Review / helm-validate (pull_request) Successful in 10s
Details
Lint & Validate / shellcheck (pull_request) Successful in 8s
Details
CI Review / alert-receiver-test (pull_request) Successful in 21s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 21s
Details
CI Review / record-sql-test (pull_request) Successful in 50s
Details
Lint & Validate / terraform-validate (pull_request) Successful in 2m10s
Details
CI Review / cockpit-test (pull_request) Successful in 2m27s
Details
CI Review / dispatcher-test (pull_request) Successful in 3m21s
Details
Build Lab Director Dispatcher Image / build-and-push (pull_request) Successful in 3m13s
Details
CI Review / cockpit-e2e (pull_request) Successful in 3m56s
Details
676e0dfc5d 
Seat A's hold #25486 on 2f50d0d0, which R1 does not close: R1 folds the SID,
and this is the REPO column.

THE BUG, traced end to end on the live values. The ledger pins the CANONICAL
lower-cased repo — `canonicalRepo` lower-cases it at register
(seatpost/post.go), so a lab-director row says `homelab/lab-director`. The
relay reads its own repo from `LD_FORGEJO_REPO`, and the live deployment sets
the MIXED-CASE `Homelab/lab-director`
(prod manifests/ld-fleet/ld-runtime-deployment.yaml:577-578). It passed that
string unchanged into `SeatPostEndpointPosted`, whose statement compared
`repo = $1`. So the read missed EVERY row: with `LD_RELAY_PROVENANCE_ONLY=on`
the relay would ignore every LD-VERDICT and every `resolved <ID>` — fail-closed,
silent apart from a pod log line, and it would land exactly when C5 arms the
gate.

THE FOLD, in two places, because the two sides are written by different
components and must not be able to drift apart again:
  * relay/poller.go: both provenance reads go through the new
    `provenanceRepo()`, which lower-cases and trims `cfg.Repo`. The allowlist
    and every decision record keep using cfg.Repo as configured; only the KEY
    the ledger is queried with is folded, and folding can only make a real row
    FOUND.
  * record/seat_posts.go: `selectSeatPostEndpointPostedSQL` is now
    `lower(repo) = lower($1)`, so any other or future caller is covered by the
    statement itself.

TESTS. The existing provenance tests keyed their fake on the SAME string the
relay was configured with — which is precisely why none of them could see this.
The relay fixture now carries both spellings: `testRepo` (what the pod is
configured with) and `pinnedRepo` (what the ledger holds), and every provenance
fake is keyed on the pinned one. The new cross-boundary row asserts the KEY the
gate asked with, not only the outcome.

RED ON 75486a4d (the new test files run against a temp worktree of this repo at
that head, removed afterwards):
  FAIL TestProvenanceReadAsksWithThePinnedRepoCase
       the provenance read asked with ["Homelab/lab-director" ...], want the
       pinned ["homelab/lab-director" ...]
  FAIL TestSeatPostStatementsFilterOnActionAndResult
       the endpoint_posted statement is missing "lower(repo) = lower($1)"
  TestEndpointPostedReadFoldsTheRepoCase SKIPS in this pod (no
       LD_TEST_RECORD_DSN) and fails on 75486a4d in CI, where the schema
       fixture runs: the old statement answers false for a row pinned in the
       canonical spelling.

GREEN on this head: go build ./..., go vet ./..., and
go test -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/ all ok;
gofmt clean on every changed file.

Lab-Run: LR-20261111-025
pi-landing commented 2026-11-12 16:36:42 +00:00
Author
Collaborator
Copy link

Residual (security lead, 2026-11-12): claim uniqueness index is case-exact; closure = migration unique (repo, pr, head, lower(sid)) + boot check + S/s concurrency test, or spec 031.

Residual (security lead, 2026-11-12): claim uniqueness index is case-exact; closure = migration unique (repo, pr, head, lower(sid)) + boot check + S/s concurrency test, or spec 031.
ld-reviewer commented 2026-11-12 16:39:27 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=676e0dfc5da64039662fc22dec2bb9d1fe944a8c session=LR-20261111-025-seatA-20261112T1636Z

APPROVE — ld-reviewer — head 676e0dfc5d

Judged at this head only: diff 3563ec7fad6f...676e0dfc5da6 (15 files, +2793/-202), this PR's own unit tests, and guard section 30. No comments or reviews were read.

WHAT I VERIFIED

  • AMENDMENT 4's two-credential split is real, not a rename: live-head READ keeps LD_FORGEJO_TOKEN, the POST uses LD_REVIEWER_TOKEN, and no fallback path exists to the landing token (seatpost/post.go:1229 missingCredential; cmd/ld-runtime/seat_post.go:197 is the defensive second line). An absent posting credential is a per-call 502 naming its own variable, never a boot failure.
  • Condition 2 holds: the Poster seam has exactly one write, PostIssueComment, and TestNoReviewOnTheSeatPostPath (seatpost/post_test.go:1929) walks the package's syntax tree.
  • R29's key is the seat AND the target on every read (seatpost/post.go:1115 ClaimedForTarget; record/seat_posts.go:139-145), distinct from the bundle-keyed Posted, and the burned-claim sentence now points at a fresh SID rather than a fresh bundle (seatpost/post.go:307-311).
  • The head commit's fix is sound and is the right side of the boundary: the ledger pins the lower-case repo while LD_FORGEJO_REPO carries the mixed-case Homelab/lab-director, so the read folds on both sides (record/seat_posts.go:162-167 lower(repo)=lower($1); relay/poller.go:433 provenanceRepo), and TestEndpointPostedReadFoldsTheRepoCase proves it against real SQL in CI, not against a Go fake that mirrors the query.
  • The gate is OFF by default and byte-for-byte the old loop when off (relay/poller.go:179-191; envBool cmd/ld-runtime/main.go:1140), fails CLOSED on a read error, refuses to boot with the flag on and no seam, and prints its state on the boot line — so the daylight cut is one env change and cannot be silent.
  • B3's other half is covered: a resolved <ID> line needs the same provenance, with its own pinned grammar (relay/poller.go:470, :508).
  • Refusals stay value-free, a non-gated repo gets its own audit category, and the boot index check now names WHICH index is absent instead of waiting for a name 00008 deleted (cmd/ld-runtime/seat_post.go:373-377).

FINDINGS (non-blocking, one per line)

  • record/seat_posts.go:142 folds the sid in the claim READ, but the durable key does not: record/testdata/ci_record_schema.sql:111-113 keys the bare (repo, pr, head, sid), so two case-variant claims that race across replicas both satisfy the index. Clearing it: key prod 00008 and the fixture on (repo, pr, head, lower(sid)) and update record/seat_posts_test.go:267.
  • relay/poller.go:419 runs provenanceSkip's doc comment straight into provenanceRepo's with no blank line, so go/doc gives provenanceRepo no doc; relay/poller.go:221-225 is an orphaned fragment duplicating the field comment at :212-215.
  • No evidence in this diff that prod 00008 (record/seat_posts.go:26-36) is in flight. Rolling this image first makes SeatPostsIndexes (record/seat_posts.go:332) false and refuses every seat post — fail-closed but total; the ordering belongs on the PR.
  • Flipping LD_RELAY_PROVENANCE_ONLY before seats actually post through the endpoint ignores every verdict (same fail-closed shape). The precondition is implied at relay/poller.go:181-190 but is not in the env doc at cmd/ld-runtime/main.go:40-42.
  • relay/poller.go:216 provenanceIgnored is never pruned: one int per unprovenanced comment for the pod's life.

TESTS (this clone, this PR's tests only)

  • go vet ./seatpost/... ./relay/... ./record/... ./cmd/ld-runtime/... → clean; go build ./... → ok
  • go test ./seatpost/... ./relay/... ./record/... ./cmd/ld-runtime/... → rc=0, four packages ok
  • TestConcurrentClaimOnOneSidInsertsOnce, TestClaimReadFoldsTheSidCase, TestEndpointPostedReadFoldsTheRepoCase SKIP here (LD_TEST_RECORD_DSN unset) and CI requires the first to PASS against throwaway PostgreSQL (.forgejo/workflows/ci.yaml:405-412), so the DB half is enforced where it can run
  • guard: recomputed scripts/gits-sbx-epic-test.sh section 30's expected set against core/manifests/apps/sbx-* → exactly the 12 pinned lines; no image-level harness run

model: omniroute/deepseek-flash
session: LR-20261111-025-seatA-20261112T1636Z

LD-VERDICT APPROVED head=676e0dfc5da64039662fc22dec2bb9d1fe944a8c session=LR-20261111-025-seatA-20261112T1636Z **APPROVE — ld-reviewer — head 676e0dfc5da64039662fc22dec2bb9d1fe944a8c** Judged at this head only: diff 3563ec7fad6f...676e0dfc5da6 (15 files, +2793/-202), this PR's own unit tests, and guard section 30. No comments or reviews were read. WHAT I VERIFIED - AMENDMENT 4's two-credential split is real, not a rename: live-head READ keeps LD_FORGEJO_TOKEN, the POST uses LD_REVIEWER_TOKEN, and no fallback path exists to the landing token (seatpost/post.go:1229 missingCredential; cmd/ld-runtime/seat_post.go:197 is the defensive second line). An absent posting credential is a per-call 502 naming its own variable, never a boot failure. - Condition 2 holds: the Poster seam has exactly one write, PostIssueComment, and TestNoReviewOnTheSeatPostPath (seatpost/post_test.go:1929) walks the package's syntax tree. - R29's key is the seat AND the target on every read (seatpost/post.go:1115 ClaimedForTarget; record/seat_posts.go:139-145), distinct from the bundle-keyed Posted, and the burned-claim sentence now points at a fresh SID rather than a fresh bundle (seatpost/post.go:307-311). - The head commit's fix is sound and is the right side of the boundary: the ledger pins the lower-case repo while LD_FORGEJO_REPO carries the mixed-case Homelab/lab-director, so the read folds on both sides (record/seat_posts.go:162-167 lower(repo)=lower($1); relay/poller.go:433 provenanceRepo), and TestEndpointPostedReadFoldsTheRepoCase proves it against real SQL in CI, not against a Go fake that mirrors the query. - The gate is OFF by default and byte-for-byte the old loop when off (relay/poller.go:179-191; envBool cmd/ld-runtime/main.go:1140), fails CLOSED on a read error, refuses to boot with the flag on and no seam, and prints its state on the boot line — so the daylight cut is one env change and cannot be silent. - B3's other half is covered: a `resolved <ID>` line needs the same provenance, with its own pinned grammar (relay/poller.go:470, :508). - Refusals stay value-free, a non-gated repo gets its own audit category, and the boot index check now names WHICH index is absent instead of waiting for a name 00008 deleted (cmd/ld-runtime/seat_post.go:373-377). FINDINGS (non-blocking, one per line) - record/seat_posts.go:142 folds the sid in the claim READ, but the durable key does not: record/testdata/ci_record_schema.sql:111-113 keys the bare (repo, pr, head, sid), so two case-variant claims that race across replicas both satisfy the index. Clearing it: key prod 00008 and the fixture on (repo, pr, head, lower(sid)) and update record/seat_posts_test.go:267. - relay/poller.go:419 runs provenanceSkip's doc comment straight into provenanceRepo's with no blank line, so go/doc gives provenanceRepo no doc; relay/poller.go:221-225 is an orphaned fragment duplicating the field comment at :212-215. - No evidence in this diff that prod 00008 (record/seat_posts.go:26-36) is in flight. Rolling this image first makes SeatPostsIndexes (record/seat_posts.go:332) false and refuses every seat post — fail-closed but total; the ordering belongs on the PR. - Flipping LD_RELAY_PROVENANCE_ONLY before seats actually post through the endpoint ignores every verdict (same fail-closed shape). The precondition is implied at relay/poller.go:181-190 but is not in the env doc at cmd/ld-runtime/main.go:40-42. - relay/poller.go:216 provenanceIgnored is never pruned: one int per unprovenanced comment for the pod's life. TESTS (this clone, this PR's tests only) - go vet ./seatpost/... ./relay/... ./record/... ./cmd/ld-runtime/... → clean; go build ./... → ok - go test ./seatpost/... ./relay/... ./record/... ./cmd/ld-runtime/... → rc=0, four packages ok - TestConcurrentClaimOnOneSidInsertsOnce, TestClaimReadFoldsTheSidCase, TestEndpointPostedReadFoldsTheRepoCase SKIP here (LD_TEST_RECORD_DSN unset) and CI requires the first to PASS against throwaway PostgreSQL (.forgejo/workflows/ci.yaml:405-412), so the DB half is enforced where it can run - guard: recomputed scripts/gits-sbx-epic-test.sh section 30's expected set against core/manifests/apps/sbx-* → exactly the 12 pinned lines; no image-level harness run model: omniroute/deepseek-flash session: LR-20261111-025-seatA-20261112T1636Z
ld-reviewer commented 2026-11-12 16:48:28 +00:00
Collaborator
Copy link

LD-VERDICT APPROVED head=676e0dfc5da64039662fc22dec2bb9d1fe944a8c session=LR-20261111-025-seatB-20261112T1636Z

Approved. R29's claim key (repo, pr, head, sid) is carried by code, by the CI schema fixture and by a real-PostgreSQL racing-insert test that ci.yaml now requires to have executed; the posting credential is a SECOND identity (LD_REVIEWER_TOKEN) with no fallback to the landing token; the registration allowlist and the repo/sid canonicalisation are on the pin, not on the echo; the relay's endpoint-provenance gate ships OFF and is a no-op in that state; and the ld-reviewer material census under sbx-* is a tripwire that bites in both mutated directions. Head carries Lab-Run: LR-20261111-025.

FINDINGS (none blocking; listed so the next commit can take them)

  • relay/poller.go:420 — DOC FUSION introduced by this head commit: the provenanceSkip doc block runs straight into provenanceRepo's, so provenanceRepo (433) carries a doc about provenanceSkip and provenanceSkip (437) carries none. Clear it by inserting a // separator at poller.go:420.
  • relay/poller.go:221-225 — orphan fragment ("// the endpoint's ledger does not account for…") is attached to logf's doc (226); it duplicates provenanceIgnored's doc (212-215). Delete 221-225.
  • relay/poller.go:441 + record/seat_posts.go:166 — the head is compared byte-exact (head = $3) and the relay only TrimSpaces pr.HeadSHA, while the repo half IS case-folded on both sides. Register lowercases the head (seatpost/post.go:722), so a non-lowercase head from the API would make EVERY verdict unprovenanced once the gate is on — the same silent-outage class seat A held on for repo. One-line clear: strings.ToLower at poller.go:441.
  • record/seat_posts.go:119-127 vs the 00008 index — the READ folds the sid (lower(sid) = lower($4)) but the unique index is byte-exact, so two case-variant spellings racing can BOTH insert a claim and both reach Forgejo; the index is therefore not the cross-replica arbiter the doc claims for that race (sequentially the read still refuses, so it can only refuse more). Either index lower(sid), or state this residual beside seatpost/post.go:56.
  • relay/poller.go:508-516 — the resolved <ID> gate is head-pinned: a provenanced withdrawal posted at H1 stops counting as soon as the head moves to H2, and it is then logged as "WITHOUT endpoint provenance" (495). Fail-closed (the hold stays in force), but the withdrawal is silently stranded and misdiagnosed.
  • record/seat_posts.go:333 — the boot assertion on seat_posts_one_claim_per_sid couples the image to prod 00008: an OLD ld-runtime still running after 00008 lands refuses every post, and the only evidence is a boot line printed before the migration applied. The prod PR must roll/restart ld-runtime in the same window.

TESTS (this clone, this head)

  • go build ./... rc=0; go vet ./... rc=0; gofmt -l on every changed .go file: clean.
  • go test -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/ rc=0 — ok seatpost 0.051s, record 0.007s, relay 0.101s, ld-runtime 0.049s.
  • bash scripts/gits-sbx-epic-test.sh rc=1, 24/729 FAIL — but the PR's new section 30 passes all four checks (census, intact-copy control, and both red mutations), and the 24 predate this PR: the base commit already differs from origin/live in the same files (git diff --stat origin/live 3563ec7 -- core/manifests/apps/sbx-gits shows forgejo-ext.yaml + statefulset.yaml), and none of the 24 names a file this diff touches.
  • fleet/image/test/seat-tools-test.sh does NOT exist in this checkout (no fleet/ directory in Homelab/infra-core) — the brief's named command cannot run here; the Go package tests above are this PR's unit tests.
  • Not run, per brief: entrypoint-sim.sh and any image-level harness.

model: omniroute/deepseek-flash
session: LR-20261111-025-seatB-20261112T1636Z

LD-VERDICT APPROVED head=676e0dfc5da64039662fc22dec2bb9d1fe944a8c session=LR-20261111-025-seatB-20261112T1636Z Approved. R29's claim key (repo, pr, head, sid) is carried by code, by the CI schema fixture and by a real-PostgreSQL racing-insert test that ci.yaml now requires to have executed; the posting credential is a SECOND identity (LD_REVIEWER_TOKEN) with no fallback to the landing token; the registration allowlist and the repo/sid canonicalisation are on the pin, not on the echo; the relay's endpoint-provenance gate ships OFF and is a no-op in that state; and the ld-reviewer material census under sbx-* is a tripwire that bites in both mutated directions. Head carries `Lab-Run: LR-20261111-025`. FINDINGS (none blocking; listed so the next commit can take them) - relay/poller.go:420 — DOC FUSION introduced by this head commit: the `provenanceSkip` doc block runs straight into `provenanceRepo`'s, so provenanceRepo (433) carries a doc about provenanceSkip and provenanceSkip (437) carries none. Clear it by inserting a `//` separator at poller.go:420. - relay/poller.go:221-225 — orphan fragment ("// the endpoint's ledger does not account for…") is attached to `logf`'s doc (226); it duplicates provenanceIgnored's doc (212-215). Delete 221-225. - relay/poller.go:441 + record/seat_posts.go:166 — the head is compared byte-exact (`head = $3`) and the relay only TrimSpaces pr.HeadSHA, while the repo half IS case-folded on both sides. Register lowercases the head (seatpost/post.go:722), so a non-lowercase head from the API would make EVERY verdict unprovenanced once the gate is on — the same silent-outage class seat A held on for repo. One-line clear: `strings.ToLower` at poller.go:441. - record/seat_posts.go:119-127 vs the 00008 index — the READ folds the sid (`lower(sid) = lower($4)`) but the unique index is byte-exact, so two case-variant spellings racing can BOTH insert a claim and both reach Forgejo; the index is therefore not the cross-replica arbiter the doc claims for that race (sequentially the read still refuses, so it can only refuse more). Either index `lower(sid)`, or state this residual beside seatpost/post.go:56. - relay/poller.go:508-516 — the `resolved <ID>` gate is head-pinned: a provenanced withdrawal posted at H1 stops counting as soon as the head moves to H2, and it is then logged as "WITHOUT endpoint provenance" (495). Fail-closed (the hold stays in force), but the withdrawal is silently stranded and misdiagnosed. - record/seat_posts.go:333 — the boot assertion on `seat_posts_one_claim_per_sid` couples the image to prod 00008: an OLD ld-runtime still running after 00008 lands refuses every post, and the only evidence is a boot line printed before the migration applied. The prod PR must roll/restart ld-runtime in the same window. TESTS (this clone, this head) - go build ./... rc=0; go vet ./... rc=0; gofmt -l on every changed .go file: clean. - go test -count=1 ./seatpost/ ./record/ ./relay/ ./cmd/ld-runtime/ rc=0 — ok seatpost 0.051s, record 0.007s, relay 0.101s, ld-runtime 0.049s. - bash scripts/gits-sbx-epic-test.sh rc=1, 24/729 FAIL — but the PR's new section 30 passes all four checks (census, intact-copy control, and both red mutations), and the 24 predate this PR: the base commit already differs from origin/live in the same files (`git diff --stat origin/live 3563ec7 -- core/manifests/apps/sbx-gits` shows forgejo-ext.yaml + statefulset.yaml), and none of the 24 names a file this diff touches. - fleet/image/test/seat-tools-test.sh does NOT exist in this checkout (no fleet/ directory in Homelab/infra-core) — the brief's named command cannot run here; the Go package tests above are this PR's unit tests. - Not run, per brief: entrypoint-sim.sh and any image-level harness. model: omniroute/deepseek-flash session: LR-20261111-025-seatB-20261112T1636Z
All checks were successful
CI Review / receiver-test (pull_request) Successful in 4s
Details
CI Review / runner-manifest-test (pull_request) Successful in 5s
Details
Build Lab Director Dispatcher Image / preflight (pull_request) Successful in 4s
Details
CI Review / helm-validate (pull_request) Successful in 10s
Required
Details
Lint & Validate / shellcheck (pull_request) Successful in 8s
Required
Details
CI Review / alert-receiver-test (pull_request) Successful in 21s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 21s
Required
Details
CI Review / record-sql-test (pull_request) Successful in 50s
Details
Lint & Validate / terraform-validate (pull_request) Successful in 2m10s
Required
Details
CI Review / cockpit-test (pull_request) Successful in 2m27s
Details
CI Review / dispatcher-test (pull_request) Successful in 3m21s
Details
Build Lab Director Dispatcher Image / build-and-push (pull_request) Successful in 3m13s
Details
CI Review / cockpit-e2e (pull_request) Successful in 3m56s
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin lr-20261111-025/amendment-4:lr-20261111-025/amendment-4
git switch lr-20261111-025/amendment-4
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
operator-review

Requires human operator review before merge — dangerous class (RBAC, secrets, network policy, Terraform) or explicitly gated. Never auto-merge.

No labels
operator-review
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/infra-core!950
No description provided.