HOMELAB-2566: fix(forgejo): submit formal exact-head verdicts #81

Merged

pi-landing merged 3 commits from plane/HOMELAB-2566-formal-verdicts into main 2026-11-03 19:02:45 +00:00

Conversation 8 Commits 3 Files changed 8 +2023 -148

aaron commented 2026-11-03 04:49:50 +00:00

Owner

Copy link

Closes HOMELAB-2566 (epic HOMELAB-2518).

Problem

The verdict helper composed the landing-lock grammar correctly and then wrote it
as a general comment (POST /issues/{n}/comments). The landing lock reads
comment bodies, so the verdict was counted — and branch protection, which reads
formal review state, saw nothing. PR #126 measured exactly that refusal. A
verdict that is not a formal review on the exact head is not an approval, however
well-formed its text.

Contract implemented

--post now submits one formal review:

POST /api/v1/repos/{owner}/{repo}/pulls/{n}/reviews
{ "body": "<composed; grammar first>",
  "event": "APPROVED" | "REQUEST_CHANGES",
  "commit_id": "<the full 40-hex live head this run verified>" }

• Identity before any write. GET /user names the token's own login;
  --author is an expectation that must equal it. Unreadable identity or a
  different login ⇒ zero writes. No proxy identities, no --force.
• Exact head. --head-sha must be the PR's live head (refused before
  writing otherwise), and that head is sent explicitly as commit_id.
• Body. Three contiguous lines from byte zero — ## Verdict: <event>,
  model: <id>, head: <40-hex> — plus tier: premium|worker|cheap on line 4
  when the body supplies a standalone tier line (or --tier); a contradicting
  tier is refused. The resolution sentence and prose still sit after the blank
  line, so the statement block stays exactly the grammar.
• Read-back is the reviews LIST, never the POST echo: same id, the event's
  state, the token's own login as author, commit_id equal to the exact head
  (branch protection would otherwise read a review of another commit), and the
  contiguous prefix (tier: included) as the row's opening bytes. Then the
  lock's parser re-runs on the live stream against the real row
  (asItem — same kind, same id), so COUNTED is about what Forgejo stored.
• Failure discipline. One POST, no retry anywhere. 401/403 stops (exit 2,
  nothing written); another 4xx is a plain refusal (nothing written); a 5xx, an
  in-flight failure or a 2xx without an id is a write that may exist and is
  reported as NOT verified with an explicit do not repost.
• No comment fallback. --compat-general-comment "<reason>" is the only way
  to also write a comment: it requires a written reason (>= 20 chars), it runs
  only after the review is verified, and it never fires on failure.
• Duplicate prevention. A byte-identical verdict that is already the
  reviewer key's current word at this head is refused, not written twice. A later
  hold or a new head changes "current", so a genuine re-verdict still works.

landing.ts and the merge implementation are untouched.

Red first (fresh origin/main 9747346, implementation untouched)

cd forgejo && ./node_modules/.bin/vitest run tests/verdict-formal-review.test.ts
=> Tests  17 failed | 4 passed (21)

The recorded defect: the helper's only write is POST .../issues/{n}/comments,
so event is undefined, no commit_id exists, and no formal review is
created. Already green were the three pre-existing refusals (head mismatch,
--author mismatch, unreadable identity) and a dry run.

The fake is a new route table (tests/formal-review-fake.ts) because the
shared fake.ts cannot answer POST /pulls/N/reviews at all — a fake that
cannot model the contract cannot catch a helper that ignores it. It models a
stored review plus every read-back failure mode: list without the row, unreadable
list, wrong state, wrong commit_id, wrong author, lost prefix, POST-without-id,
4xx and 5xx.

Green

• tests/verdict-formal-review.test.ts: 22 passed (22) — APPROVED,
  REQUEST_CHANGES, tier lift, dry run posts nothing, head mismatch, identity
  mismatch, unreadable identity, duplicate refusal, six read-back failures, POST
  without an id, 5xx, 4xx, and the three compatibility-comment cases.
• full forgejo suite: 15 files / 394 tests passed
• tsc --noEmit: clean; git diff --check: clean
• CI: this run's checks are on the PR; nothing is merged by this job.

Pre-existing harnesses were updated to the new write path — their invariants
(one POST, stored-row-only authorship, NOT verified + do not repost) are
unchanged, only the endpoint moved: verdict-694-replay.test.ts,
comment-identity.test.ts, comment-identity-hardening.test.ts.

Files

forgejo/verdict-cli.ts, forgejo/verdict.ts, docs/forgejo.md,
forgejo/tests/verdict-formal-review.test.ts (new),
forgejo/tests/formal-review-fake.ts (new), the three harness updates above.

Report-only audit (no revert, no history rewrite)

docs/forgejo.md § Pre-amendment landings records:

• PR #74 merged 2026-11-03T03:18:10Z as fe7058b8aaba16c240d38f7421fac4e52b295200
  on head 0f0b980c…, before this contract, on plain-comment verdicts #19875
  (omniroute/deepseek-flash) and #19883 (omniroute/k3.1-pro, tier: premium)
  with zero formal reviews. Overseer ruling: stands; no revert.
• PR #78's premium comment-form hold #19854 (omniroute/k3.1-pro,
  REQUEST_CHANGES at c127f4a3…, naming H2550-K1) was never a formal review. It
  is historical: the fix round moved the head to b2b3d70e…, and both models
  then submitted formal APPROVED reviews there before the merge. Neither comment
  is edited or deleted; under this contract the same hold would be a formal
  REQUEST_CHANGES.

Collision pre-check: #80 touches landing.ts + model-supersede/record tests,
#68 is pi-swarm-suite/**; docs/forgejo.md was held by #78, merged at 9747346.
No overlap with this diff.

No self-review, no verdicts, no merge, no Done: the panel reviews this head
normally.

Closes HOMELAB-2566 (epic HOMELAB-2518). ## Problem The verdict helper composed the landing-lock grammar correctly and then wrote it as a **general comment** (`POST /issues/{n}/comments`). The landing lock reads comment bodies, so the verdict was counted — and branch protection, which reads formal review state, saw nothing. PR #126 measured exactly that refusal. A verdict that is not a formal review on the exact head is not an approval, however well-formed its text. ## Contract implemented `--post` now submits one **formal review**: ``` POST /api/v1/repos/{owner}/{repo}/pulls/{n}/reviews { "body": "<composed; grammar first>", "event": "APPROVED" | "REQUEST_CHANGES", "commit_id": "<the full 40-hex live head this run verified>" } ``` - **Identity before any write.** `GET /user` names the token's own login; `--author` is an expectation that must equal it. Unreadable identity or a different login ⇒ zero writes. No proxy identities, no `--force`. - **Exact head.** `--head-sha` must be the PR's live head (refused before writing otherwise), and that head is sent explicitly as `commit_id`. - **Body.** Three contiguous lines from byte zero — `## Verdict: <event>`, `model: <id>`, `head: <40-hex>` — plus `tier: premium|worker|cheap` on line 4 when the body supplies a standalone tier line (or `--tier`); a contradicting tier is refused. The resolution sentence and prose still sit after the blank line, so the statement block stays exactly the grammar. - **Read-back is the reviews LIST**, never the POST echo: same id, the event's `state`, the token's own login as author, `commit_id` equal to the exact head (branch protection would otherwise read a review of another commit), and the contiguous prefix (`tier:` included) as the row's opening bytes. Then the lock's parser re-runs on the live stream against the **real row** (`asItem` — same kind, same id), so `COUNTED` is about what Forgejo stored. - **Failure discipline.** One POST, no retry anywhere. 401/403 stops (`exit 2`, nothing written); another 4xx is a plain refusal (nothing written); a 5xx, an in-flight failure or a 2xx without an id is a write that **may exist** and is reported as **NOT verified** with an explicit **do not repost**. - **No comment fallback.** `--compat-general-comment "<reason>"` is the only way to also write a comment: it requires a written reason (>= 20 chars), it runs only **after** the review is verified, and it never fires on failure. - **Duplicate prevention.** A byte-identical verdict that is already the reviewer key's current word at this head is refused, not written twice. A later hold or a new head changes "current", so a genuine re-verdict still works. `landing.ts` and the merge implementation are untouched. ## Red first (fresh `origin/main` 9747346, implementation untouched) ``` cd forgejo && ./node_modules/.bin/vitest run tests/verdict-formal-review.test.ts => Tests 17 failed | 4 passed (21) ``` The recorded defect: the helper's only write is `POST .../issues/{n}/comments`, so `event` is `undefined`, no `commit_id` exists, and no formal review is created. Already green were the three pre-existing refusals (head mismatch, `--author` mismatch, unreadable identity) and a dry run. The fake is a **new** route table (`tests/formal-review-fake.ts`) because the shared `fake.ts` cannot answer `POST /pulls/N/reviews` at all — a fake that cannot model the contract cannot catch a helper that ignores it. It models a stored review plus every read-back failure mode: list without the row, unreadable list, wrong state, wrong `commit_id`, wrong author, lost prefix, POST-without-id, 4xx and 5xx. ## Green - `tests/verdict-formal-review.test.ts`: **22 passed (22)** — APPROVED, REQUEST_CHANGES, tier lift, dry run posts nothing, head mismatch, identity mismatch, unreadable identity, duplicate refusal, six read-back failures, POST without an id, 5xx, 4xx, and the three compatibility-comment cases. - full forgejo suite: **15 files / 394 tests passed** - `tsc --noEmit`: clean; `git diff --check`: clean - CI: this run's checks are on the PR; nothing is merged by this job. Pre-existing harnesses were updated to the new write path — their invariants (one POST, stored-row-only authorship, NOT verified + do not repost) are unchanged, only the endpoint moved: `verdict-694-replay.test.ts`, `comment-identity.test.ts`, `comment-identity-hardening.test.ts`. ## Files `forgejo/verdict-cli.ts`, `forgejo/verdict.ts`, `docs/forgejo.md`, `forgejo/tests/verdict-formal-review.test.ts` (new), `forgejo/tests/formal-review-fake.ts` (new), the three harness updates above. ## Report-only audit (no revert, no history rewrite) `docs/forgejo.md` § *Pre-amendment landings* records: - **PR #74** merged `2026-11-03T03:18:10Z` as `fe7058b8aaba16c240d38f7421fac4e52b295200` on head `0f0b980c…`, before this contract, on plain-comment verdicts #19875 (`omniroute/deepseek-flash`) and #19883 (`omniroute/k3.1-pro`, `tier: premium`) with **zero** formal reviews. Overseer ruling: **stands**; no revert. - **PR #78**'s premium **comment-form hold** #19854 (`omniroute/k3.1-pro`, REQUEST_CHANGES at `c127f4a3…`, naming H2550-K1) was never a formal review. It is historical: the fix round moved the head to `b2b3d70e…`, and both models then submitted formal APPROVED reviews there before the merge. Neither comment is edited or deleted; under this contract the same hold would be a formal REQUEST_CHANGES. Collision pre-check: #80 touches `landing.ts` + model-supersede/record tests, #68 is `pi-swarm-suite/**`; `docs/forgejo.md` was held by #78, merged at 9747346. No overlap with this diff. No self-review, no verdicts, no merge, no Done: the panel reviews this head normally.

aaron added 1 commit 2026-11-03 04:49:50 +00:00

HOMELAB-2566: fix(forgejo): submit formal exact-head verdicts ... 9f343ce83e

The verdict helper's only write was a general comment, so a well-formed
verdict was readable by the landing lock and still invisible to branch
protection (PR #126 measured the refusal). The write is now a FORMAL review:

  POST /api/v1/repos/{o}/{r}/pulls/{n}/reviews
  { body, event: APPROVED|REQUEST_CHANGES, commit_id: <full live head> }

Contract enforced by the helper itself:
- GET /user identity asserted before any write; --author must equal it;
  no proxy identities, no --force.
- --head-sha must be the live head; commit_id is that head explicitly.
- the body opens with three contiguous grammar lines (## Verdict:, model:,
  head:), plus a standalone tier: line when the body supplies one (or --tier).
- the reviews LIST is the only read-back evidence: id, state, author,
  commit_id and the byte-exact contiguous prefix are all proved, then the
  lock's parser re-runs on the live stream against the real row.
- one POST, no retry. A 401/403 stops, a 4xx is a plain refusal (nothing
  written), and a 5xx / in-flight failure / 2xx-without-id is reported as
  NOT verified with an explicit do-not-repost.
- no plain-comment fallback; --compat-general-comment '<reason>' is the only
  way to also write a comment (reason required, only after the review is
  verified), and a byte-identical verdict that is already the reviewer key's
  current word is refused as a duplicate.

Red first on 9747346: tests/verdict-formal-review.test.ts => 17 failed |
4 passed (21). Green: 22 passed, full suite 394 passed (15 files),
tsc --noEmit clean, git diff --check clean.

docs/forgejo.md records the contract and, report-only, PR #74's
pre-amendment merge (stands, no revert) and PR #78's old comment-form hold
(historical; head change + formal reviews disposed of it). Nothing was
edited or deleted. landing.ts and the merge path are untouched.

ld-reviewer approved these changes 2026-11-03 05:01:09 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 9f343ce83e

Independent GENERAL review on the exact head (live API head == this sha, base
origin/main 9747346). Read-only worktree
.worktrees/HOMELAB-2566-formal-verdicts; no edits, no merge, no other PR, no
agents. Findings: working-files/homelab-2518/HOMELAB-2566-review-deepseek.md.

What I verified myself

• tests/verdict-formal-review.test.ts → 22 passed; full forgejo suite → 15
  files / 394 passed; tsc --noEmit clean; git diff --check clean.
• Red-first, re-derived independently: head tests against the 9747346
  versions of verdict.ts + verdict-cli.ts → 17 failed | 5 passed. The worker
  log records 17/4 of 21 (one test added after that capture) — same 17 failures.
• landing.ts, api.ts, index.ts untouched: the lock, the policy and the merge
  path are unmodified, and no merge call exists anywhere in the diff. The only
  writes are POST /pulls/{n}/reviews and the opt-in compatibility comment.

The amendment's legs, attacked

• APPROVED / REQUEST_CHANGES both write a review with event and an explicit
  commit_id equal to the verified live head; the tests assert the POST path, the
  event, the commit_id and the three contiguous opening lines, and assert no
  /issues/ write on the same run.
• Head and identity mismatch are refused before any write. GET /user is read
  on every run; tokenIdentity = authenticated ?? author is only reachable after
  the unreadable-identity refusal, so a proxy identity cannot reach a write.
• Read-back is the reviews LIST, not the POST echo, and every leg is checked:
  id, state, author == this token, commit_id == exact head, and the contiguous
  prefix bytes including tier:. Six failure knobs each produce NOT verified +
  do-not-repost with exactly one POST and no comment fallback; a 2xx without
  an id and a 5xx are handled as may-exist; a 4xx is a plain refusal.
• Duplicate prevention: a byte-identical verdict that is already the reviewer
  key's current word at this head is refused with no write, and asItem replaces
  (never appends) the real row in the live re-parse, so the same review id cannot
  be double-counted.
• Tier: a standalone body tier: is lifted to line 4 and dropped from prose;
  --tier contradicting the body, and an unknown tier, are refused; the prefix
  proof covers it. state remains non-evidence for the landing lock — the lock
  still reads body text only.
• PR #74 report-only audit — verified live: merged
  2026-11-03T03:18:10Z as fe7058b8…, head 0f0b980c…, 0 formal reviews,
  verdict comments #19875 (omniroute/deepseek-flash) and #19883
  (omniroute/k3.1-pro, tier: premium) still present. fe7058b8 is still an
  ancestor of this head, so nothing was reverted or rewritten. #19854 (PR #78's
  comment-form hold) is untouched. The record matches the snapshot I re-read.

Observations, not blockers

• --compat-general-comment is the one path that re-creates the duplicate shape
  (comment + review, same reviewer key, same head). It is reason-gated, runs only
  after verification, and the ledger keys by reviewer key with newest-wins, so it
  is benign — but the default must stay the review alone.
• --tier premium is self-declared with no model cross-check; TIER_DECL_RE
  already carried that semantics in landing.ts, so this adds a route, not a hole.
• No pre-POST scan for an orphan row from a crashed run: a reviewer who ignores
  the do-not-repost text can create a second review. The helper itself never
  retries.
• collectState skips bodyless reviews, so such a row fails closed as NOT
  verified — the safe direction.

No blocker. APPROVED on 9f343ce83e92e1d86695068ae9f8f877ade59296.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 9f343ce83e92e1d86695068ae9f8f877ade59296 Independent GENERAL review on the exact head (live API head == this sha, base `origin/main` 9747346). Read-only worktree `.worktrees/HOMELAB-2566-formal-verdicts`; no edits, no merge, no other PR, no agents. Findings: `working-files/homelab-2518/HOMELAB-2566-review-deepseek.md`. ## What I verified myself - `tests/verdict-formal-review.test.ts` → 22 passed; full forgejo suite → 15 files / 394 passed; `tsc --noEmit` clean; `git diff --check` clean. - **Red-first, re-derived independently**: head tests against the `9747346` versions of `verdict.ts` + `verdict-cli.ts` → 17 failed | 5 passed. The worker log records 17/4 of 21 (one test added after that capture) — same 17 failures. - `landing.ts`, `api.ts`, `index.ts` untouched: the lock, the policy and the merge path are unmodified, and no merge call exists anywhere in the diff. The only writes are `POST /pulls/{n}/reviews` and the opt-in compatibility comment. ## The amendment's legs, attacked - **APPROVED / REQUEST_CHANGES** both write a review with `event` and an explicit `commit_id` equal to the verified live head; the tests assert the POST path, the `event`, the `commit_id` and the three contiguous opening lines, and assert no `/issues/` write on the same run. - **Head and identity mismatch** are refused before any write. `GET /user` is read on every run; `tokenIdentity = authenticated ?? author` is only reachable after the unreadable-identity refusal, so a proxy identity cannot reach a write. - **Read-back** is the reviews LIST, not the POST echo, and every leg is checked: id, `state`, author == this token, `commit_id` == exact head, and the contiguous prefix bytes including `tier:`. Six failure knobs each produce NOT verified + do-not-repost with **exactly one** POST and no comment fallback; a 2xx without an id and a 5xx are handled as may-exist; a 4xx is a plain refusal. - **Duplicate prevention**: a byte-identical verdict that is already the reviewer key's current word at this head is refused with no write, and `asItem` replaces (never appends) the real row in the live re-parse, so the same review id cannot be double-counted. - **Tier**: a standalone body `tier:` is lifted to line 4 and dropped from prose; `--tier` contradicting the body, and an unknown tier, are refused; the prefix proof covers it. `state` remains non-evidence for the landing lock — the lock still reads body text only. - **PR #74 report-only audit — verified live**: merged `2026-11-03T03:18:10Z` as `fe7058b8…`, head `0f0b980c…`, **0 formal reviews**, verdict comments #19875 (`omniroute/deepseek-flash`) and #19883 (`omniroute/k3.1-pro`, `tier: premium`) still present. `fe7058b8` is still an ancestor of this head, so nothing was reverted or rewritten. #19854 (PR #78's comment-form hold) is untouched. The record matches the snapshot I re-read. ## Observations, not blockers - `--compat-general-comment` is the one path that re-creates the duplicate shape (comment + review, same reviewer key, same head). It is reason-gated, runs only after verification, and the ledger keys by reviewer key with newest-wins, so it is benign — but the default must stay the review alone. - `--tier premium` is self-declared with no model cross-check; `TIER_DECL_RE` already carried that semantics in `landing.ts`, so this adds a route, not a hole. - No pre-POST scan for an orphan row from a crashed run: a reviewer who ignores the do-not-repost text can create a second review. The helper itself never retries. - `collectState` skips bodyless reviews, so such a row fails closed as NOT verified — the safe direction. No blocker. APPROVED on `9f343ce83e92e1d86695068ae9f8f877ade59296`.

ld-reviewer requested changes 2026-11-03 05:21:01 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: REQUEST_CHANGES

model: omniroute/k3.1-pro
head: 9f343ce83e
tier: premium

Independent premium review of the complete eight-file diff. Targeted 22/22, full 394/394, tsc and CI #189 14/14 are green, but two post-write verification races fail open.

Blocker — H2566-K1: head movement during submission returns successful verification

BLOCKER OPEN: H2566-K1

After the POST, the helper re-reads the PR but does not refuse when its head differs from the pinned head. It also unconditionally returns exit 0 when the stored-row parse is not COUNTED. Reproduced with a fake API: the post-write head changed, the helper printed parse IGNORED and still returned posted plus verified: branch protection reads... with exit 0. Any non-COUNTED verification must use reviewMayExist (NOT verified / do not repost); add APPROVED and REQUEST_CHANGES head-race tests.

Blocker — H2566-K2: stored-row re-parse substitutes verification time for submitted_at

BLOCKER OPEN: H2566-K2

asItem preserves only kind/id; evaluateComposition reconstructs the row with at: nowIso, composed body and author. Reproduced with an APPROVED review stored at 04:00, a later same-key REQUEST_CHANGES at 05:00, and verification at 06:00: the older approval was re-dated to 06:00 and returned COUNTED/current with exit 0. Re-parse the actual stored row, including submitted_at, body and author, and add later-same-key race coverage.

Full findings: working-files/homelab-2518/HOMELAB-2566-review-kimi.md.

## Verdict: REQUEST_CHANGES model: omniroute/k3.1-pro head: 9f343ce83e92e1d86695068ae9f8f877ade59296 tier: premium Independent premium review of the complete eight-file diff. Targeted 22/22, full 394/394, tsc and CI #189 14/14 are green, but two post-write verification races fail open. ### Blocker — H2566-K1: head movement during submission returns successful verification BLOCKER OPEN: H2566-K1 After the POST, the helper re-reads the PR but does not refuse when its head differs from the pinned head. It also unconditionally returns exit 0 when the stored-row parse is not COUNTED. Reproduced with a fake API: the post-write head changed, the helper printed parse IGNORED and still returned `posted` plus `verified: branch protection reads...` with exit 0. Any non-COUNTED verification must use `reviewMayExist` (NOT verified / do not repost); add APPROVED and REQUEST_CHANGES head-race tests. ### Blocker — H2566-K2: stored-row re-parse substitutes verification time for submitted_at BLOCKER OPEN: H2566-K2 `asItem` preserves only kind/id; `evaluateComposition` reconstructs the row with `at: nowIso`, composed body and author. Reproduced with an APPROVED review stored at 04:00, a later same-key REQUEST_CHANGES at 05:00, and verification at 06:00: the older approval was re-dated to 06:00 and returned COUNTED/current with exit 0. Re-parse the actual stored row, including submitted_at, body and author, and add later-same-key race coverage. Full findings: `working-files/homelab-2518/HOMELAB-2566-review-kimi.md`.

aaron force-pushed plane/HOMELAB-2566-formal-verdicts from 9f343ce83e to 84649d440e 2026-11-03 05:36:27 +00:00 Compare

aaron referenced this pull request 2026-11-03 05:36:29 +00:00

HOMELAB-2578: fix(forgejo): resolve landing credential at runtime #82

aaron commented 2026-11-03 05:37:57 +00:00

Author

Owner

Copy link

Round 2 — fix for premium review 1664

Head: 84649d440ee691df302fbdccabf99be3dcc7b444 (single new head; previous 9f343ce, rebased onto origin/main e2b6f6e).

• resolved H2566-K1
• resolved H2566-K2

H2566-K1 — a post-write head move is never a success

The head is now part of the evidence. After the POST the helper re-reads the PR and refuses when the live head is not the pinned head (the PR head moved from <pinned> to <live> …), because the stored review's commit_id is then a commit the PR no longer has. Beyond that specific check, the general rule is enforced: a re-parse that is not COUNTED is not a success. An IGNORED/REFUSED stored-row parse now goes through the existing reviewMayExist path — nonzero exit, NOT verified, explicit do not repost, posted left empty — instead of returning 0 with verified: branch protection reads a formal … review on the exact head this run judged. The separate full-lock preview remains informational (an unrelated leg may legitimately still refuse).

H2566-K2 — the re-parse reads the stored row, never the clock

Composition.asItem carried only kind/id, so evaluateComposition rebuilt the row with at: nowIso, the composed body and the composed author. asItem now carries the stored row's own submitted_at, body and author (all required — a row with no usable timestamp is refused rather than dated by the clock), and the live stream's row for that id must be byte-equal to the row the read-back proved (body, author, submitted_at). An older approval can no longer supersede a later same-key hold.

Red first (rebased, implementation untouched)

cd forgejo && ./node_modules/.bin/vitest run tests/verdict-formal-review.test.ts
=> Test Files  1 failed (1)
=> Tests  7 failed | 26 passed (33)

The 7: head moved after the APPROVED review / after the REQUEST_CHANGES review (exit 0, posted populated, verified: printed over an IGNORED parse); same-key later hold at 05:00Z verified at 06:00Z (counted, current = true, exit 0 with the older approval); the fabricated-timestamp variant; and the changed-row cases (live row body / author / submitted_at differing from the read-back row while the re-parse substituted the composed fields and still exited 0). Two more new cases are green before fix and kept as regression coverage: a row missing from the live stream (existing guard) and the honest success path for both events.

New fake knobs in forgejo/tests/formal-review-fake.ts: headAfterPost, afterPostReviews, hideReviewAfterPost, mutateReviewOnLiveStream. The reviews LIST answers the immediate read-back proof on the first post-POST read and the live stream (with those knobs) on later reads — the exact pair of reads the helper makes.

Green

• tests/verdict-formal-review.test.ts: 33 passed (33)
• full forgejo suite: 15 files / 423 tests passed
• tsc --noEmit: clean; git diff --check: clean
• CI: run #192 on 84649d4 (in progress at comment time; per-job status on the PR)

Files: forgejo/verdict-cli.ts, forgejo/verdict.ts, docs/forgejo.md, forgejo/tests/verdict-formal-review.test.ts, forgejo/tests/formal-review-fake.ts. The formal-review contract, one-write/no-retry discipline, identity assertions, duplicate refusal and the report-only PR #74 audit are unchanged; landing.ts and the merge implementation are untouched.

Evidence log kept in working-files/homelab-2518/HOMELAB-2566-round2-red.log and HOMELAB-2566-worker.md; reproduced on HOMELAB-2566. Fresh formal panel please — no self-review, no verdict, no merge, no Done from this job.

## Round 2 — fix for premium review 1664 Head: `84649d440ee691df302fbdccabf99be3dcc7b444` (single new head; previous `9f343ce`, rebased onto `origin/main e2b6f6e`). - resolved H2566-K1 - resolved H2566-K2 ### H2566-K1 — a post-write head move is never a success The head is now part of the evidence. After the POST the helper re-reads the PR and **refuses** when the live head is not the pinned head (`the PR head moved from <pinned> to <live> …`), because the stored review's `commit_id` is then a commit the PR no longer has. Beyond that specific check, the general rule is enforced: **a re-parse that is not `COUNTED` is not a success**. An `IGNORED`/`REFUSED` stored-row parse now goes through the existing `reviewMayExist` path — nonzero exit, `NOT verified`, explicit `do not repost`, `posted` left empty — instead of returning 0 with `verified: branch protection reads a formal … review on the exact head this run judged`. The separate full-lock preview remains informational (an unrelated leg may legitimately still refuse). ### H2566-K2 — the re-parse reads the stored row, never the clock `Composition.asItem` carried only `kind`/`id`, so `evaluateComposition` rebuilt the row with `at: nowIso`, the composed body and the composed author. `asItem` now carries the stored row's **own** `submitted_at`, body and author (all required — a row with no usable timestamp is refused rather than dated by the clock), and the live stream's row for that id must be byte-equal to the row the read-back proved (body, author, `submitted_at`). An older approval can no longer supersede a later same-key hold. ### Red first (rebased, implementation untouched) ``` cd forgejo && ./node_modules/.bin/vitest run tests/verdict-formal-review.test.ts => Test Files 1 failed (1) => Tests 7 failed | 26 passed (33) ``` The 7: head moved after the APPROVED review / after the REQUEST_CHANGES review (exit 0, `posted` populated, `verified:` printed over an IGNORED parse); same-key later hold at 05:00Z verified at 06:00Z (`counted`, `current = true`, exit 0 with the older approval); the fabricated-timestamp variant; and the changed-row cases (live row `body` / `author` / `submitted_at` differing from the read-back row while the re-parse substituted the composed fields and still exited 0). Two more new cases are green before fix and kept as regression coverage: a row missing from the live stream (existing guard) and the honest success path for both events. New fake knobs in `forgejo/tests/formal-review-fake.ts`: `headAfterPost`, `afterPostReviews`, `hideReviewAfterPost`, `mutateReviewOnLiveStream`. The reviews LIST answers the immediate read-back proof on the first post-POST read and the live stream (with those knobs) on later reads — the exact pair of reads the helper makes. ### Green - `tests/verdict-formal-review.test.ts`: **33 passed (33)** - full forgejo suite: **15 files / 423 tests passed** - `tsc --noEmit`: clean; `git diff --check`: clean - CI: run #192 on `84649d4` (in progress at comment time; per-job status on the PR) Files: `forgejo/verdict-cli.ts`, `forgejo/verdict.ts`, `docs/forgejo.md`, `forgejo/tests/verdict-formal-review.test.ts`, `forgejo/tests/formal-review-fake.ts`. The formal-review contract, one-write/no-retry discipline, identity assertions, duplicate refusal and the report-only PR #74 audit are unchanged; `landing.ts` and the merge implementation are untouched. Evidence log kept in `working-files/homelab-2518/HOMELAB-2566-round2-red.log` and `HOMELAB-2566-worker.md`; reproduced on HOMELAB-2566. Fresh formal panel please — no self-review, no verdict, no merge, no Done from this job.

aaron referenced this pull request 2026-11-03 05:38:02 +00:00

HOMELAB-2567: test(forgejo): make deployed vitest load typebox #83

ld-reviewer approved these changes 2026-11-03 06:00:37 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 84649d440e

Independent general review at round-2 head 84649d4 — APPROVED.

Both prior blockers are closed, and I reproduced each independently against a fake Forgejo
(no repo edits, my own harness).

H2566-K1 (head move reported as success): the helper now re-reads the live stream after the single
POST, refuses on any head change ("the PR head moved … it is not a verdict on the head this run
judged"), and requires parse COUNTED before it may say verified. My reproduction (pinned head read,
POST stored, post-write read returns another head) exits 1 with posted=undefined, NOT verified, do
not repost, and no success line.

H2566-K2 (substituted clock): Composition.asItem.at is now required and carries the row's own
submitted_at; evaluateComposition rebuilds the row from the STORED body/author/time and replaces
rather than appends it. My reproduction — same-key REQUEST_CHANGES stored at 05:00 while the
approval under verification is stored at 04:00 and verification runs at 06:00 — now exits 1 with
"IGNORED: a later verdict of the same reviewer key replaced this one". An older approval can no
longer outrank a later hold.

Changed and missing rows fail closed: row absent from the read-back list, row absent from the live
stream, live body/author/submitted_at diverging from the proved row, and a stored row with no usable
timestamp each exit 1 through reviewMayExist. 4xx is a plain refusal, 5xx and a 2xx-without-id are
"may exist", and only one POST is ever made.

Successful path verified: one write to /pulls/81/reviews with event and commit_id equal to the exact
pinned head, body opening with the three contiguous grammar lines from byte zero, tier lifting to
line 4 when declared, --compat-general-comment writing only after the review is verified.

Local reruns: verdict-formal-review 33/33, full forgejo suite 15 files 423/423, tsc --noEmit clean,
git diff --check clean. Round-2 red log shows 7 failed / 26 passed with the implementation reverted.
CI #192 on this exact head: 14/14 jobs successful. landing.ts untouched; the amendment contract and
the PR #74 report-only audit (no revert, no rewrite) are preserved.

Two non-blocking findings, recorded in the review document and raised for the panel rather than as
holds:

H2566-R2-1: the duplicate guard matches ledger.current without regard to itemKind, so a
byte-identical verdict whose current record is a plain COMMENT (the pre-2566 helper produced the
same bytes) is refused instead of converted into the formal review that branch protection reads —
the one upgrade this ticket exists to enable, at an unchanged head. A one-line narrowing to
itemKind === "review" would fix it; a prose change is a workaround, so this costs rather than blocks.

H2566-R2-2: --tier premium is not checked against the model's own class, so premium evidence can be
declared for a model the policy does not class as premium. Not a new class of spoofing (a prose
line always worked), but the flag makes it one keystroke.

Nothing was merged, edited or deleted; no prior verdict was touched.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 84649d440ee691df302fbdccabf99be3dcc7b444 Independent general review at round-2 head 84649d4 — APPROVED. Both prior blockers are closed, and I reproduced each independently against a fake Forgejo (no repo edits, my own harness). H2566-K1 (head move reported as success): the helper now re-reads the live stream after the single POST, refuses on any head change ("the PR head moved … it is not a verdict on the head this run judged"), and requires parse COUNTED before it may say verified. My reproduction (pinned head read, POST stored, post-write read returns another head) exits 1 with posted=undefined, NOT verified, do not repost, and no success line. H2566-K2 (substituted clock): Composition.asItem.at is now required and carries the row's own submitted_at; evaluateComposition rebuilds the row from the STORED body/author/time and replaces rather than appends it. My reproduction — same-key REQUEST_CHANGES stored at 05:00 while the approval under verification is stored at 04:00 and verification runs at 06:00 — now exits 1 with "IGNORED: a later verdict of the same reviewer key replaced this one". An older approval can no longer outrank a later hold. Changed and missing rows fail closed: row absent from the read-back list, row absent from the live stream, live body/author/submitted_at diverging from the proved row, and a stored row with no usable timestamp each exit 1 through reviewMayExist. 4xx is a plain refusal, 5xx and a 2xx-without-id are "may exist", and only one POST is ever made. Successful path verified: one write to /pulls/81/reviews with event and commit_id equal to the exact pinned head, body opening with the three contiguous grammar lines from byte zero, tier lifting to line 4 when declared, --compat-general-comment writing only after the review is verified. Local reruns: verdict-formal-review 33/33, full forgejo suite 15 files 423/423, tsc --noEmit clean, git diff --check clean. Round-2 red log shows 7 failed / 26 passed with the implementation reverted. CI #192 on this exact head: 14/14 jobs successful. landing.ts untouched; the amendment contract and the PR #74 report-only audit (no revert, no rewrite) are preserved. Two non-blocking findings, recorded in the review document and raised for the panel rather than as holds: H2566-R2-1: the duplicate guard matches ledger.current without regard to itemKind, so a byte-identical verdict whose current record is a plain COMMENT (the pre-2566 helper produced the same bytes) is refused instead of converted into the formal review that branch protection reads — the one upgrade this ticket exists to enable, at an unchanged head. A one-line narrowing to itemKind === "review" would fix it; a prose change is a workaround, so this costs rather than blocks. H2566-R2-2: --tier premium is not checked against the model's own class, so premium evidence can be declared for a model the policy does not class as premium. Not a new class of spoofing (a prose line always worked), but the flag makes it one keystroke. Nothing was merged, edited or deleted; no prior verdict was touched.

ld-reviewer requested changes 2026-11-03 06:30:43 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: REQUEST_CHANGES

model: omniroute/k3.1-pro
head: 84649d440e

tier: premium

Independent premium final review of exact head 84649d4. The K1/K2 fixes and 423-test suite are green, but one post-write verification gap remains.

Blocker — H2566-K3: read-back accepts reviews branch protection does not count

BLOCKER OPEN: H2566-K3

formalReviewProblems ignores the stored official, dismissed, and stale flags. Forgejo preserves APPROVED/REQUEST_CHANGES state when dismissed and serializes these flags separately; branch protection requires official, non-dismissed reviews and can exclude stale approvals. My independent no-network probe changed only one flag per run: dismissed=true, official=false, and stale=true each returned exit 0 with postedId=1601 and the false verified: branch protection reads a formal APPROVED review success line. Require the validity legs at read-back and in the live-row comparison, with refusal tests.

R2-1 duplicate itemKind narrowing and R2-2 premium model-class validation remain non-blocking follow-ups. Evidence: working-files/homelab-2518/HOMELAB-2566-review-kimi-final.md.

## Verdict: REQUEST_CHANGES model: omniroute/k3.1-pro head: 84649d440ee691df302fbdccabf99be3dcc7b444 tier: premium Independent premium final review of exact head 84649d4. The K1/K2 fixes and 423-test suite are green, but one post-write verification gap remains. ### Blocker — H2566-K3: read-back accepts reviews branch protection does not count BLOCKER OPEN: H2566-K3 `formalReviewProblems` ignores the stored `official`, `dismissed`, and `stale` flags. Forgejo preserves APPROVED/REQUEST_CHANGES state when dismissed and serializes these flags separately; branch protection requires official, non-dismissed reviews and can exclude stale approvals. My independent no-network probe changed only one flag per run: `dismissed=true`, `official=false`, and `stale=true` each returned exit 0 with `postedId=1601` and the false `verified: branch protection reads a formal APPROVED review` success line. Require the validity legs at read-back and in the live-row comparison, with refusal tests. R2-1 duplicate itemKind narrowing and R2-2 premium model-class validation remain non-blocking follow-ups. Evidence: `working-files/homelab-2518/HOMELAB-2566-review-kimi-final.md`.

aaron added 1 commit 2026-11-03 06:47:04 +00:00

HOMELAB-2566: round 3 — only a review branch protection COUNTED is verified ... 0af8dfed89

H2566-K3. Forgejo converts a review's `state` from its review TYPE alone and
copies `official`, `dismissed` and `stale` alongside, so a dismissed, unofficial
or stale review still reads back as APPROVED / REQUEST_CHANGES. Branch
protection requires `official = true AND dismissed = false` and drops stale
approvals where the branch opts in, so the read-back proved a formal state about
a row it does not count and the helper printed

  verified: branch protection reads a formal APPROVED review on the exact head

for a row branch protection ignores.

- `reviewValidityProblems()` is now part of the read-back proof
  (`formalReviewProblems`) and of the post-write live-row comparison: an
  `official: false`, `dismissed: true` or `stale: true` row refuses through
  `reviewMayExist` (nonzero, NOT verified, do-not-repost, exactly one POST, no
  success line). An absent `official` is not a refusal — Forgejo always
  serializes it (measured: formal review `official: true`, comment review
  `official: false`).
- `collectState` returns the raw review rows it read, because `LockState.items`
  drops the flags; the live leg needs them to re-check the row the landing lock
  reads.
- Fake knobs `flags`, `flagsAtReadBack` and the three `mutateReviewOnLiveStream`
  values keep the two legs independently testable.
- 13 new tests: 6 stored-row cases (each flag, APPROVED and REQUEST_CHANGES),
  3 read-back-only cases, 3 live-row-degradation cases, 1 counted-row control.

Red first: 12 failed | 34 passed (46) against the pre-fix helper. Green: 46/46
focused, 436/436 full suite, tsc clean, diff check clean. Each leg disabled
alone fails exactly its own 3 cases. K1/K2 behaviour and the exact
id/state/head/body/author/submitted_at legs are unchanged; landing.ts and the
merge path are untouched. R2-1/R2-2 remain follow-ups.

aaron commented 2026-11-03 06:53:16 +00:00

Author

Owner

Copy link

Round 3 — fix for premium review #1671 (H2566-K3)

Head: 0af8dfed899f716fc85cb834ee20a8f2e271d078 (single new head; previous 84649d4, based on current origin/main e2b6f6e).

Blocker H2566-K3 resolved — formalReviewProblems and the post-write live-row comparison now reject a stored review whose official is false, dismissed is true, or stale is true, so the helper can no longer print verified: branch protection reads a formal APPROVED review for a row branch protection does not count.

K1/K2 remain resolved (post-write head move and stored-row re-parse), unchanged by this round.

H2566-K3 — the stored row must be one branch protection COUNTS

Forgejo maps a review's state from its review TYPE alone and serializes official, dismissed, stale separately, so a dismissed/unofficial/stale review keeps reading APPROVED or REQUEST_CHANGES; branch protection requires official = true AND dismissed = false and drops stale approvals where the branch opts in.

• New reviewValidityProblems() is applied in the read-back proof (formalReviewProblems) AND on the raw live row. LockState.items drops the flags, so collectState now also returns the raw reviewRows it read; the live leg finds that id there and re-checks it, refusing when the row degraded after a proved read-back.
• Every refusal goes through the existing reviewMayExist path: nonzero exit, NOT verified, explicit do not repost, posted left empty, exactly one POST, no success line, no comment fallback.
• An ABSENT official is not a refusal: Forgejo always serializes the flag (measured on this instance — a formal review reads official: true, a general COMMENT review reads official: false), so absence only appears in fixtures. What is refused is an explicit false.
• Fake knobs flags, flagsAtReadBack and mutateReviewOnLiveStream: official|dismissed|stale keep the two legs independently testable.

Red first (pre-fix helper, new tests kept)

cd forgejo && ./node_modules/.bin/vitest run tests/verdict-formal-review.test.ts
=> Test Files  1 failed (1)
=> Tests  12 failed | 34 passed (46)

The 12: each of official=false / dismissed=true / stale=true on APPROVED and on REQUEST_CHANGES (6), the three read-back-only cases, and the three live-row-degradation cases — all exiting 0 with posted id 1601 and the false verified: branch protection … line. The 13th new test is the positive control (a counted row still verifies for both events) and passes before and after.

Leg independence, each leg disabled alone: read-back leg off ⇒ 3 failed (its own cases only); live-row leg off ⇒ 3 failed (its own cases only); restored ⇒ 46/46.

Green

• tests/verdict-formal-review.test.ts: 46 passed (46) (43 round-2 + 13 K3)
• full forgejo suite: 15 files / 436 tests passed
• tsc --noEmit: clean; git diff --check: clean
• CI run #197 on 0af8dfe: 14/14 jobs successful

Files: forgejo/verdict-cli.ts, forgejo/tests/verdict-formal-review.test.ts, forgejo/tests/formal-review-fake.ts, docs/forgejo.md. The K1/K2 behaviour, exact id/state/head/body/author/submitted_at legs, duplicate refusal and compatibility-comment discipline are unchanged; landing.ts, the merge implementation, PR #82/#83 files and ~/.pi/agent/extensions are untouched. R2-1 (duplicate-guard itemKind narrowing) and R2-2 (premium tier vs model class) remain follow-ups.

Evidence: working-files/homelab-2518/HOMELAB-2566-round3-worker.md and HOMELAB-2566-round3-red.log.

This new head invalidates every prior review — fresh DeepSeek + premium formal panel please. No self-review, no verdict, no merge, no Done from this job.

## Round 3 — fix for premium review #1671 (H2566-K3) Head: `0af8dfed899f716fc85cb834ee20a8f2e271d078` (single new head; previous `84649d4`, based on current `origin/main` e2b6f6e). Blocker H2566-K3 resolved — `formalReviewProblems` and the post-write live-row comparison now reject a stored review whose `official` is false, `dismissed` is true, or `stale` is true, so the helper can no longer print `verified: branch protection reads a formal APPROVED review` for a row branch protection does not count. K1/K2 remain resolved (post-write head move and stored-row re-parse), unchanged by this round. ### H2566-K3 — the stored row must be one branch protection COUNTS Forgejo maps a review's `state` from its review TYPE alone and serializes `official`, `dismissed`, `stale` separately, so a dismissed/unofficial/stale review keeps reading APPROVED or REQUEST_CHANGES; branch protection requires `official = true AND dismissed = false` and drops stale approvals where the branch opts in. - New `reviewValidityProblems()` is applied in the read-back proof (`formalReviewProblems`) AND on the raw live row. `LockState.items` drops the flags, so `collectState` now also returns the raw `reviewRows` it read; the live leg finds that id there and re-checks it, refusing when the row degraded after a proved read-back. - Every refusal goes through the existing `reviewMayExist` path: nonzero exit, `NOT verified`, explicit `do not repost`, `posted` left empty, exactly one POST, no success line, no comment fallback. - An ABSENT `official` is not a refusal: Forgejo always serializes the flag (measured on this instance — a formal review reads `official: true`, a general COMMENT review reads `official: false`), so absence only appears in fixtures. What is refused is an explicit `false`. - Fake knobs `flags`, `flagsAtReadBack` and `mutateReviewOnLiveStream: official|dismissed|stale` keep the two legs independently testable. ### Red first (pre-fix helper, new tests kept) ``` cd forgejo && ./node_modules/.bin/vitest run tests/verdict-formal-review.test.ts => Test Files 1 failed (1) => Tests 12 failed | 34 passed (46) ``` The 12: each of `official=false` / `dismissed=true` / `stale=true` on APPROVED and on REQUEST_CHANGES (6), the three read-back-only cases, and the three live-row-degradation cases — all exiting 0 with `posted` id 1601 and the false `verified: branch protection …` line. The 13th new test is the positive control (a counted row still verifies for both events) and passes before and after. Leg independence, each leg disabled alone: read-back leg off ⇒ 3 failed (its own cases only); live-row leg off ⇒ 3 failed (its own cases only); restored ⇒ 46/46. ### Green - `tests/verdict-formal-review.test.ts`: **46 passed (46)** (43 round-2 + 13 K3) - full forgejo suite: **15 files / 436 tests passed** - `tsc --noEmit`: clean; `git diff --check`: clean - CI run #197 on `0af8dfe`: **14/14 jobs successful** Files: `forgejo/verdict-cli.ts`, `forgejo/tests/verdict-formal-review.test.ts`, `forgejo/tests/formal-review-fake.ts`, `docs/forgejo.md`. The K1/K2 behaviour, exact id/state/head/body/author/`submitted_at` legs, duplicate refusal and compatibility-comment discipline are unchanged; `landing.ts`, the merge implementation, PR #82/#83 files and `~/.pi/agent/extensions` are untouched. R2-1 (duplicate-guard `itemKind` narrowing) and R2-2 (premium tier vs model class) remain follow-ups. Evidence: `working-files/homelab-2518/HOMELAB-2566-round3-worker.md` and `HOMELAB-2566-round3-red.log`. This new head invalidates every prior review — fresh DeepSeek + premium formal panel please. No self-review, no verdict, no merge, no Done from this job.

ld-reviewer approved these changes 2026-11-03 07:09:17 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 0af8dfed89

Independent GENERAL review, round 3, exact head. Read-only worktree
/home/aaron/repos/pi-extensions/.worktrees/HOMELAB-2566-formal-verdicts; no edits,
no merge, no deployment, no other PR, no ~/.pi/agent/extensions change. Findings:
working-files/homelab-2518/HOMELAB-2566-review-deepseek-round3.md.

H2566-K3 — reproduced independently, both legs, not taken from the worker report

• Both legs exist and each one alone stops the success line. reviewValidityProblems()
  rejects official === false, dismissed === true, stale === true; it is called from
  formalReviewProblems() (the read-back of the reviews LIST) and again on the raw live
  row through collectState().reviewRows — needed because LockState.items drops the
  flags. Every refusal routes through reviewMayExist: exit 1 (nonzero), posted
  undefined, NOT verified, do not repost, no verified: branch protection line,
  exactly one POST, no comment fallback.
• Red-first re-derived in a throwaway copy (0700 temp dir, copy mutated, deleted): read-back
  leg off ⇒ 3 failed | 43 passed (its own read-back-only cases); live-row leg off ⇒ 3 failed
  | 43 passed (its own live-degradation cases); both off ⇒ 12 failed | 34 passed (46). Each
  leg fails only its own cases, so neither leg is covered by the other, and the 12 reproduce
  the recorded red numbers.
• Positive control holds: a counted row (official=true, dismissed=false, stale=false)
  still verifies for both events, with the success line and exactly one POST.

The blocker was concrete on this instance

Read-only GET /pulls/81/reviews: review #1666 reads APPROVED with
official=false, dismissed=true, stale=true once #1671 was submitted — a reviewer's
superseded review keeps its formal state while losing exactly the legs branch protection
filters on. That is the row the pre-fix helper would have announced as "branch protection
reads a formal APPROVED review".

K1 / K2 / exact-leg regressions

• K1: a post-write head move refuses (the PR head moved from …), including the variant
  where the lock preview still passes; one POST, no retry.
• K2: the re-parse uses the stored submitted_at, body and author; the live row must equal
  the proved row (body/author/submitted_at); a row with no usable timestamp refuses; a
  later same-key hold makes the older row IGNORED and therefore a refusal.
• Exact legs unchanged: id, formal state equal to the event, commit_id equal to the pinned
  head, author equal to the token's own login, contiguous opening bytes including the
  optional tier line. Six read-back failure knobs, POST-without-id, 5xx and 4xx all keep
  NOT verified / do-not-repost (or a plain 4xx refusal), with exactly one POST.

Checks run on this head

Focused verdict-formal-review 46/46; full forgejo suite 15 files 436/436; tsc --noEmit
exit 0; git diff --check clean; worktree clean. CI run #197 on
0af8dfed899f716fc85cb834ee20a8f2e271d078: 14/14 jobs success. Diff is 8 files;
landing.ts and the merge path are untouched.

Non-blocking observations

• An ABSENT official is treated as counted (only an explicit false refuses). All four rows
  on this instance serialize all three flags, so today this is fixture-only — but it is the
  one direction that would fail open if a Forgejo upgrade stopped serializing the field.
• The live-row comparison re-checks the three validity legs and body/author/submitted_at but
  not commit_id, which LockState.items does not carry; a re-pin after the read-back is
  therefore not re-proven. Immutable in practice, and outside K3's scope.
• H2566-R2-1 (duplicate guard does not narrow to itemKind === "review") and H2566-R2-2
  (self-declared premium tier is not cross-checked against model class) remain non-blocking
  follow-ups.

No blocker open. APPROVED on 0af8dfed899f716fc85cb834ee20a8f2e271d078.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 0af8dfed899f716fc85cb834ee20a8f2e271d078 Independent GENERAL review, round 3, exact head. Read-only worktree `/home/aaron/repos/pi-extensions/.worktrees/HOMELAB-2566-formal-verdicts`; no edits, no merge, no deployment, no other PR, no `~/.pi/agent/extensions` change. Findings: `working-files/homelab-2518/HOMELAB-2566-review-deepseek-round3.md`. ### H2566-K3 — reproduced independently, both legs, not taken from the worker report - Both legs exist and each one alone stops the success line. `reviewValidityProblems()` rejects `official === false`, `dismissed === true`, `stale === true`; it is called from `formalReviewProblems()` (the read-back of the reviews LIST) and again on the raw live row through `collectState().reviewRows` — needed because `LockState.items` drops the flags. Every refusal routes through `reviewMayExist`: exit 1 (nonzero), `posted` undefined, `NOT verified`, `do not repost`, no `verified: branch protection` line, exactly one POST, no comment fallback. - Red-first re-derived in a throwaway copy (0700 temp dir, copy mutated, deleted): read-back leg off ⇒ 3 failed | 43 passed (its own read-back-only cases); live-row leg off ⇒ 3 failed | 43 passed (its own live-degradation cases); both off ⇒ 12 failed | 34 passed (46). Each leg fails only its own cases, so neither leg is covered by the other, and the 12 reproduce the recorded red numbers. - Positive control holds: a counted row (`official=true, dismissed=false, stale=false`) still verifies for both events, with the success line and exactly one POST. ### The blocker was concrete on this instance Read-only `GET /pulls/81/reviews`: review #1666 reads APPROVED with `official=false, dismissed=true, stale=true` once #1671 was submitted — a reviewer's superseded review keeps its formal state while losing exactly the legs branch protection filters on. That is the row the pre-fix helper would have announced as "branch protection reads a formal APPROVED review". ### K1 / K2 / exact-leg regressions - K1: a post-write head move refuses (`the PR head moved from …`), including the variant where the lock preview still passes; one POST, no retry. - K2: the re-parse uses the stored `submitted_at`, body and author; the live row must equal the proved row (body/author/`submitted_at`); a row with no usable timestamp refuses; a later same-key hold makes the older row IGNORED and therefore a refusal. - Exact legs unchanged: id, formal state equal to the event, `commit_id` equal to the pinned head, author equal to the token's own login, contiguous opening bytes including the optional tier line. Six read-back failure knobs, POST-without-id, 5xx and 4xx all keep NOT verified / do-not-repost (or a plain 4xx refusal), with exactly one POST. ### Checks run on this head Focused `verdict-formal-review` 46/46; full forgejo suite 15 files 436/436; `tsc --noEmit` exit 0; `git diff --check` clean; worktree clean. CI run #197 on `0af8dfed899f716fc85cb834ee20a8f2e271d078`: 14/14 jobs success. Diff is 8 files; `landing.ts` and the merge path are untouched. ### Non-blocking observations - An ABSENT `official` is treated as counted (only an explicit `false` refuses). All four rows on this instance serialize all three flags, so today this is fixture-only — but it is the one direction that would fail open if a Forgejo upgrade stopped serializing the field. - The live-row comparison re-checks the three validity legs and body/author/`submitted_at` but not `commit_id`, which `LockState.items` does not carry; a re-pin after the read-back is therefore not re-proven. Immutable in practice, and outside K3's scope. - H2566-R2-1 (duplicate guard does not narrow to `itemKind === "review"`) and H2566-R2-2 (self-declared premium tier is not cross-checked against model class) remain non-blocking follow-ups. No blocker open. APPROVED on `0af8dfed899f716fc85cb834ee20a8f2e271d078`.

ld-reviewer approved these changes 2026-11-03 07:33:17 +00:00

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/k3.1-pro
head: 0af8dfed89

tier: premium

Independent PREMIUM round-3 review of the exact head. Evidence: working-files/homelab-2518/HOMELAB-2566-review-kimi-round3.md.

H2566-K3 verified independently

reviewValidityProblems() rejects explicit official=false, dismissed=true, and stale=true in the immediate reviews-LIST read-back and independently on the raw live row returned by collectState().reviewRows; the fake isolates those legs so neither can cover for the other.
The focused suite passed 46/46, including every wrong flag on both formal events, three read-back-only failures, three live-row degradations after a valid read-back, and valid-row success for both events.
Every invalid-row case asserts nonzero refusal, NOT verified, do not repost, no branch-protection success line, exactly one POST, one stored review, and no comment fallback; the valid control exits 0 with COUNTED and the success line.
Live API evidence confirms the defect class: superseded rows retain APPROVED or REQUEST_CHANGES while reading official=false, dismissed=true, and stale=true; current general review #1675 reads official, non-dismissed, and non-stale on the exact head.

K1/K2 and checks

K1 post-write head movement, including the lock-still-passes variant, and K2 stored submitted_at, later-same-key-hold, and live body/author/timestamp equality regressions remain green with one-POST refusal semantics.
Full Forgejo suite passed 15 files and 436/436 tests; tsc --noEmit exited 0; git diff --check was clean; CI #197 has 14/14 successful jobs on this head; landing.ts and merge code are outside the diff.

Non-blocking observations

R2-1 duplicate-guard narrowing, R2-2 premium-tier/model cross-checking, absent-official fail-open hardening, and live-row commit_id re-proof are explicitly owned by follow-up HOMELAB-2592 and are not blockers on the current Forgejo behavior and threat model.

No blocker open.

## Verdict: APPROVED model: omniroute/k3.1-pro head: 0af8dfed899f716fc85cb834ee20a8f2e271d078 tier: premium Independent PREMIUM round-3 review of the exact head. Evidence: `working-files/homelab-2518/HOMELAB-2566-review-kimi-round3.md`. ### H2566-K3 verified independently `reviewValidityProblems()` rejects explicit `official=false`, `dismissed=true`, and `stale=true` in the immediate reviews-LIST read-back and independently on the raw live row returned by `collectState().reviewRows`; the fake isolates those legs so neither can cover for the other. The focused suite passed 46/46, including every wrong flag on both formal events, three read-back-only failures, three live-row degradations after a valid read-back, and valid-row success for both events. Every invalid-row case asserts nonzero refusal, `NOT verified`, `do not repost`, no branch-protection success line, exactly one POST, one stored review, and no comment fallback; the valid control exits 0 with `COUNTED` and the success line. Live API evidence confirms the defect class: superseded rows retain APPROVED or REQUEST_CHANGES while reading `official=false`, `dismissed=true`, and `stale=true`; current general review #1675 reads official, non-dismissed, and non-stale on the exact head. ### K1/K2 and checks K1 post-write head movement, including the lock-still-passes variant, and K2 stored `submitted_at`, later-same-key-hold, and live body/author/timestamp equality regressions remain green with one-POST refusal semantics. Full Forgejo suite passed 15 files and 436/436 tests; `tsc --noEmit` exited 0; `git diff --check` was clean; CI #197 has 14/14 successful jobs on this head; `landing.ts` and merge code are outside the diff. ### Non-blocking observations R2-1 duplicate-guard narrowing, R2-2 premium-tier/model cross-checking, absent-`official` fail-open hardening, and live-row `commit_id` re-proof are explicitly owned by follow-up HOMELAB-2592 and are not blockers on the current Forgejo behavior and threat model. No blocker open.

aaron referenced this pull request 2026-11-03 07:37:13 +00:00

HOMELAB-2586: fix(forgejo): honor wrapped blocker negations #84

pi-landing merged commit 195fbb692c into main 2026-11-03 19:02:45 +00:00

pi-landing referenced this pull request from a commit 2026-11-03 19:02:46 +00:00

Merge pull request 'HOMELAB-2566: fix(forgejo): submit formal exact-head verdicts' (#81) from plane/HOMELAB-2566-formal-verdicts into main

Sign in to join this conversation.

Reviewers

No reviewers

ld-reviewer

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

Subscribe

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!81

No description provided.