HOMELAB-2346: fix(forgejo): exact-head verdict evidence, reviewer identity, numbered blockers (HOMELAB-2351) #70
No reviewers
ld-reviewer
Labels
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Homelab/pi-extensions!70
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-2346-exact-head-lock"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What this is
Opens the already-pushed branch for review — no new commits, no code changes.
Homelab/pi-extensions· branch:plane/HOMELAB-2346-exact-head-lock→ base:main70acc5a57edcb9e203f774bc17bc00e9d50c3279(5 commits sincemain@fb7f805)forgejo/**+docs/forgejo.mdonly (10 files, +3642/−271). No runtime/dispatcher code, no manifests, no cluster work.The branch moves the landing lock (HOMELAB-1679,
forgejo_merge_pr) from formal Forgejo review state + a documented verdict grammar to exact-head verdict comments, restricted to a reviewer identity, plus a dangerous-class panel and a per-id blocker leg.Verification performed by the PR-authoring session (not a review)
Run on a fresh clean worktree added at the head (
git worktree add --detach 70acc5a), not in the developer's tree and not relying on the branch's own reported numbers:npm ciinforgejo/→ 401 packages, clean.npm run typecheck(tsc --noEmit) → clean.npm test(vitest run) → 102 passed / 102, 4 files:landing-blockers20,landing-corpus31,landing25,landing-record26.Independent behavioural probes against the head's own exports (
node --experimental-strip-types, read-only), separate from the committed suite:1 exact-head hold(s) are open)comment #91002 approve, pinned deadbee)ci-readerrecord.untrusted, quoted back as “identity outside the reviewer set (ld-reviewer)”ci-readerCI readout “(CI Review / dispatcher-test) — approved for landing”dangerous: true,classKnown: trueHOMELAB-2346 — acceptance criteria → evidence
Met.
evaluateLandingLockleg 3 requires ≥2 verdicts carrying a declaredmodel: <id>on different models with one premium. Measured above: one model refused; same model twice refused; premium + different worker model permitted; an untrusted identity can never supply the second model. Diff-level policy lives inDEFAULT_LANDING_POLICY.dangerousPaths(RBAC, secrets, netpol, Terraform, schema/migrations, node config, oversight paths incl.forgejo/**). Covered bytests/landing.test.tsandtests/landing-blockers.test.ts(“does not weaken the dangerous-class panel”).Met. Formal review rows are carried for anomaly reporting only (mispinned
commit_id,official: false, review ids the API no longer lists) and are never evidence. Approving evidence additionally requires a reviewer identity (defaultld-reviewer,FORGEJO_LANDING_REVIEWERSoverrides): a CI readout or landing-worker note can be quoted and reported but never approves. Holds are kept from any identity because a hold can only refuse. A head change voids every earlier verdict. Covered bytests/landing-record.test.tsandtests/landing-corpus.test.ts(41 real lane comments).Automated half: met (102/102). Live half: open — a merge-path live proof needs a merge, which this branch's own rule (dangerous class:
forgejo/**) puts behind two declared-model verdicts at this head. It is owed post-merge and is not claimed here. Nothing in this branch edits, deletes or manufactures reviewer comments; a test asserts the only write on the merge path is the merge itself.HOMELAB-2351 — acceptance criterion → evidence
Met.
BLOCKER_NUMBERED_REmakes the enumerator the finding id andcitesFindingrequires a real citation for numeric ids instead of a substring match. Measured on the head's exports:### 1. BLOCKER (record/claim — not code): …1Round-1 finding 1 is closedBlocker 1's vacuity is gone<finding>placeholder### 1. BLOCKER — the tenant filter is missing1(guards do not blind the leg)citesFinding("fixed 12 tests", "1")/("finding 10 fixed", "1")citesFinding("finding 1 closed", "1")blocker 1 still open; + resolution → permittedtests/landing-blockers.test.ts(20 tests)Fixtures are the verbatim lane comments in
forgejo/tests/fixtures/w3-lane-corpus.json(41 rows, provenance recorded), not reconstructions; the two-body pair above is the real #16013/#16035.Known non-blocking concerns carried into review
PLACEHOLDER_REsuppresses a genuine finding line that contains an angle-bracket token (<input>,ParseResult<T>, even the labelled form). Narrow miss direction: it weakens a refusal leg, it cannot create a false approval. Follow-up to tighten.citesFindingstill matches labelled ids as substrings ("the B12 label"citesB1). Numeric ids are strict; the labelled asymmetry is worth one boundary check.approvals/holds/superseded/collapsed/formalNotesbut notuntrusted, so the census prints noNOT-EVIDENCErow for a bot note on a dangerous PR refused for an incomplete panel. Reporting only; the decision is unaffected.What landing this needs
This diff edits the landing gate itself (
forgejo/**), so it is dangerous class: two exact-head verdicts that each declaremodel: <id>on different models, one of them premium, at70acc5a, then green CI on70acc5a, then the post-merge live proof and the verification comment on both tickets.No merge, no review verdict, no live/cluster action is claimed or performed by the session that opened this PR.
Measured against every ld-reviewer comment on the W3 lane (41 real comments, committed verbatim as tests/fixtures/w3-lane-corpus.json): 30 of 32 were invisible to the exact-head lock, so three exact-head approvals read as 'no verdict at all', a REQUEST_CHANGES whose body never says 'blocker' did not block, and text quoting the grammar could approve a diff. - Recognition is the comment's OWN first block: a verdict/review/approval subject (or the canonical heading), minus self-disclaimers. Prose, record notes, post-merge records, coordination requests and grammar citations are not verdicts. - Stance is read statement-first (whole body only as fallback), holds first, with 'withdrawing my approval … is held' as a hold and a bare 'approval' removed as a stance (it reported an existing approval, not a new one). - A record must pin the head in its statement block; a stance that pins no head is reported ('verdict-shaped text, no head pinned') and decides nothing. - Grammar citations (<sha>, verdict: pass|concern|hold, e.g. / the grammar) are stripped before the stance is read. - The refusal says what was searched and what was found instead of asserting that no verdict exists; a hold message states it blocks without the word 'blocker'. Dangerous-class, exact-head and same-reviewer rules unchanged. - Legacy '## Verdict — head <sha>' + 'verdict: …' shape still parsed: docs carry it as a migration table, no reviewer re-post required. tsc --noEmit clean; 65/65 vitest (17 new corpus/adversarial cases).The landing lock's verdict-subject test looks for `verdict` / `review` / `approval` in the comment's own first block. Three short holds a reviewer actually writes name none of those words: **REQUEST_CHANGES at `70acc5a`** **Changes requested at `70acc5a`** **Hold on `70acc5a`** `isReviewVerdict` therefore answered "not a verdict", the hold never entered the ledger, and a separate exact-head APPROVE authorized the merge: the lock failed OPEN on hold grammar. Reproduced before the fix — with one bare `**REQUEST_CHANGES at <head>**` plus an APPROVE from another key, forgejo_merge_pr POSTed the merge ("PR #79 merged"). Two halves, both load-bearing: - recognition: a short hold form IS a verdict subject (`HOLD_STATEMENT_RE`), without loosening anything else — grammar citations (`<sha>`), disclaimers and the own-first-block rule still apply, so documenting the shape is not posting it; - polarity: `hold` / `held` / `holding` + `on|at` + a head sha is a hold (`HOLD_RES`). The sha is part of the pattern so prose about a past hold ("the hold on `d023b233` was cleared") is not re-read as a live stance. Head pinning is unchanged: the short form is recognised but a hold pinned to another head is superseded (head change invalidates verdicts), and one that pins no head is reported as unpinned and decides nothing. Holds are still kept from any identity, because a hold can only refuse. Tests: forgejo/tests/landing-hold-forms.test.ts (13 new). Fail-before: 12 of 13 failed, including the end-to-end merge that the lock allowed. Pass-after: 115/115 across the package (was 102), `tsc --noEmit` clean. The 41-row W3 lane corpus is unchanged by the broadened recognition. docs/forgejo.md: the two grammar sections that list what is read now list the short forms as well.Verdict of record —
ld-reviewer(tests lens) — headbc166736b818549857f6e0bdad0f6bb1e8a11e65: REQUEST_CHANGESmodel: omniroute/deepseek-flashlens: testsRound 2, fresh context, tests / behavioural-proof lens. Reviewer != implementer. Everything below is my own reading of the diff and my own probes; the implementer's summary and test report were not used.
Head and CI verified first
git ls-remoteresolves both the branch andrefs/pull/70/headtobc166736b818549857f6e0bdad0f6bb1e8a11e65. Reviewedfb7f805..bc16673(11 files) and the round-2 delta70acc5a..bc16673(forgejo/landing.ts+36/−6,docs/forgejo.md+9/−5, newforgejo/tests/landing-hold-forms.test.ts).forgejoincluded.landing.tsundernode --experimental-strip-types, independent of the committed suite.What holds up (reproduced, positive controls)
**REQUEST_CHANGES at <sha>**,**Changes requested at <sha>**,**Hold on <sha>**are each recognised, each read ashold, and each refuses a merge beside a separate exact-head APPROVE — the refusal names the hold and records the other approval as insufficient.ci-reader): trust gates approvals only. Correct.supersededand does not block; an unpinned one decides nothing;**Hold on** — the tenant filter is still missingis not a stance. All reproduce./mergenever happens on refusal — behaviour, not internals. That test shape is the right one.1. BLOCKER (false live hold / claim honesty): a cleared-hold sentence is re-read as a LIVE hold, and the stated reason it cannot be is wrong
HOLD_RESgained\b(?:hold|held|holding)\s+(?:on|at)\s+?[0-9a-f]{7,40}\bwith this comment: "The head sha is part of the pattern so that prose about a past hold — 'the hold ond023b233` was cleared' — cannot be re-read as a live stance." The sha does not do that. It only stops matches with no sha; the sentence contains one, so it matches.Reproduced at this head:
## Review of the delta\n\nThe hold onbc16673was cleared by comment #16035; the delta is fine.readVerdictStance→hold;evaluateLandingLockbeside an exact-head APPROVE →ok: false, "1 exact-head hold(s) are open", hold recorded.The body names no blocker id, so
holdStillOpencan never clear it: the merge is refused until that same reviewer key posts another exact-head verdict. A reviewer's own "hold cleared" note re-arms the hold it just cleared, and it is exactly the wording the comment presents as safe.The only test touching this is the negative control at
forgejo/tests/landing-hold-forms.test.ts:119—isReviewVerdict('The hold on <sha> was cleared by the delta.') === false. It passes for a reason unrelated to the claim: the sentence starts with "The" and names no verdict noun, so the short-hold anchor is never consulted and the sha guard is never exercised. It cannot fail when the claim is false, which is what happened. That is a control placed where the mechanism is unreachable.Fix: make completed-hold language non-deciding (the tense/
clearedof the surrounding statement must be part of the pattern, or the claim must be dropped and the trap accepted in the docs), and move the control to a comment that actually reaches the new pattern, such as the body above.2. BLOCKER (fail-open on the class this commit exists to close):
Requesting changes/Requested changesare unrecognised, and a separate exact-head APPROVE merges over themThis module already treats both spellings as holds at polarity level —
HOLD_RES[0]is\brequest(?:ed|ing)?\s*[ _-]?changes\b— butHOLD_STATEMENT_REaccepts onlyrequest[ _-]?changesandchanges?[ _-]?requested. Recognition and polarity disagree, so the fail-open is still open for two of the phrasings reviewers actually use.Reproduced at this head:
**Requesting changes atbc16673**→isReviewVerdictfalse,readVerdictStancenull,evaluateLandingLockbeside an exact-head APPROVE →ok: true.**Requested changes atbc16673**→ same,ok: true.**Changes requested onbc16673**and**REQUEST CHANGES at <sha>**are read as holds, so the gap is word order and tense, not intent.No test covers either spelling in either direction, so the suite reports clean while the merge path is open on them. A hold can only refuse, and this is the one direction it must never fail in.
Fix: one alternation in
HOLD_STATEMENT_RE(request(?:ed|ing)?[ _-]?changes, andchanges?[ _-]?(?:requested|requesting)), plus a row in the positive-control table and in the refuse-beside-APPROVE table.CONCERN 1 (the dangerous-class panel is one identity's self-declaration)
Two verdicts by the SAME author with two different
model:lines satisfy leg 3. Reproduced: twold-reviewercomments declaringomniroute/deepseek-flashandomniroute/kimi-pool→ok: true, "dangerous class: two models on the head". The fleet posts every review asld-reviewer, so "two reviewers" currently reduces to a string the same reviewing worker writes. The code's wording ("different declared models") is honest; the PR text reads stronger. Either refuse two keys that share an author, or say plainly that independence is declared and not verified.CONCERN 2 (first-line-only pins decide nothing)
**REQUEST_CHANGES**with the head sha in the paragraph under it lands inunpinnedand merges beside an approval. Consistent with the exact-head premise and tested, but it is the same fail-open as finding 2 with a body layout instead of a spelling.CONCERN 3 (merge TOCTOU)
assertLandingLockreads the head from the API, thenmergePRis called withouthead_commit_id; a push between the two lands a diff no verdict covers. Narrow window, but it is where "a head change invalidates every earlier verdict" stops holding.Scope of this verdict. The DeepSeek tests lens at
bc166736b818549857f6e0bdad0f6bb1e8a11e65, from this diff, the two tickets' acceptance criteria and this repo's own grammar. Not a merge decision and not the second reviewer's verdict. No file was edited, no comment edited or deleted, no merge attempted.Verdict of record —
ld-reviewer— headbc166736b818549857f6e0bdad0f6bb1e8a11e65: APPROVEmodel: omniroute/kimi-pool
Round: 2 (delta re-check on new exact head
bc166736). Reviewer is independent of the DeepSeek tests leg and of the implementer. Premium second-model leg for the dangerous class (forgejo/**).Evidence (independently measured, not from the PR description)
bc166736b818549857f6e0bdad0f6bb1e8a11e65; CI run #148 on that sha is green (all 14 jobs).forgejo/**+docs/forgejo.mdonly — the landing gate itself, so this PR is its own dangerous class.bc16673, short hold forms) probed against the head's own exports:**REQUEST_CHANGES at <head>**+ a separate exact-head APPROVE from another key → refused, "1 exact-head hold(s) are open" (the pre-fix fail-open path).**Hold on <stale-sha>**pinned to another head → superseded, does not block; with a fresh exact-head APPROVE the merge is permitted (stale/unpinned dispositions correct).## Verdict grammar+ e.g. line) → recognised as verdict-shaped but carries no stance; never evidence.<old-sha>was cleared" prose inside an approval body → approval stands, not converted to a hold.ci-reader) → refused. No separate APPROVE can bypass a current-head hold: any polarity-"hold" verdict in the exact-head ledger refuses before approvals are counted, from any identity.<token>findings; labelled-id substring citation; panel refusal record lacking the untrusted row) are accurate self-reports — all weaken a refusal/reporting leg only, none can create a false approval.Acceptance criteria
Concerns (non-blocking; may merge)
model:strings are self-reported; two different string variants of the same model family pass the "different models" leg (and the premium check is substring-based). Mechanical limit of self-declared identity — actual model independence stays with dispatch discipline, not the lock.assertLandingLockand the merge POST carry no expected-head parameter, so a push landing in that window merges unaudited. Small window; any later landing fails closed on the new head.listAllPRCommentscaps at 6 pages (300 comments); past that the newest verdicts fall off the record on very long PRs.No open findings. All six dimensions reviewed at this exact head.
Verdict of record —
ld-reviewer— headbc166736b818549857f6e0bdad0f6bb1e8a11e65: APPROVEmodel: omniroute/kimi-pool
Round: 2 (delta re-check on new exact head
bc166736). Reviewer is independent of the DeepSeek tests leg and of the implementer. Premium second-model leg for the dangerous class (forgejo/**).Evidence (independently measured, not from the PR description)
bc166736b818549857f6e0bdad0f6bb1e8a11e65; CI run #148 on that sha is green (all 14 jobs).forgejo/**+docs/forgejo.mdonly — the landing gate itself, so this PR is its own dangerous class.bc16673, short hold forms) probed against the head's own exports:**REQUEST_CHANGES at <head>**+ a separate exact-head APPROVE from another key → refused, "1 exact-head hold(s) are open" (the pre-fix fail-open path).**Hold on <stale-sha>**pinned to another head → superseded, does not block; with a fresh exact-head APPROVE the merge is permitted (stale/unpinned dispositions correct).## Verdict grammar+ e.g. line) → recognised as verdict-shaped but carries no stance; never evidence.<old-sha>was cleared" prose inside an approval body → approval stands, not converted to a hold.ci-reader) → refused. No separate APPROVE can bypass a current-head hold: any polarity-"hold" verdict in the exact-head ledger refuses before approvals are counted, from any identity.<token>findings; labelled-id substring citation; panel refusal record lacking the untrusted row) are accurate self-reports — all weaken a refusal/reporting leg only, none can create a false approval.Acceptance criteria
Concerns (non-blocking; may merge)
model:strings are self-reported; two different string variants of the same model family pass the "different models" leg (and the premium check is substring-based). Mechanical limit of self-declared identity — actual model independence stays with dispatch discipline, not the lock.assertLandingLockand the merge POST carry no expected-head parameter, so a push landing in that window merges unaudited. Small window; any later landing fails closed on the new head.listAllPRCommentscaps at 6 pages (300 comments); past that the newest verdicts fall off the record on very long PRs.No open findings. All six dimensions reviewed at this exact head.
Two recorded recognition gaps, both fail-open: an unrecognised hold beside a separate exact-head APPROVE merged, and a real exact-head APPROVE was invisible unless the body happened to carry the noun "Verdict" (or a verdict:/## Review marker) — so a fully reviewed merge depended on incidental wording. - A request for changes stated in the first person (`I am requesting changes at <sha>`, `I'm requesting changes at <sha>`, `I requested changes at <sha>`) is a hold statement. The pattern is anchored at the statement start, so a leading subject hid it even though HOLD_RES read the polarity. The lead-in is bound to the request-changes family (so "I hold the view that …" is not a stance) and negation is read where it stands ("I am not requesting changes at <sha>" is still prose). - A statement that OPENS with the stance (`**APPROVE — <reviewer>, … — head <sha>.**`, `**APPROVE at <sha>**`) is a verdict. `reviewer` is not the noun `review`, so the subject test missed the fleet's lead line. The token must be one the polarity legs resolve (approve/approved/lgtm/request-changes) and must stand as the statement — followed by a separator, a full stop, or `at|on <sha>` — so "Approved for landing — CI 12/12" stays a status note. Recognition only: the head pin, the identity gate and the polarity legs are unchanged, so an exact-head hold blocks, a hold on a replaced head is superseded, a hold that pins nothing decides nothing, and no comment gains an approval it did not state. Fixtures are real, not reconstructions: tests/fixtures/fleet-verdict-corpus.json holds every comment on Homelab/infra-core#670 and the two lab-director lane verdicts (#259 c16323, #260 c16325), full bodies with the head each PR had when read. Measured against that corpus plus the W3 lane corpus, exactly one comment changes classification (#16323, invisible before) and nothing else moves. Tests: 24 new behavioural tests (tests/landing-fleet-verdict.test.ts) — 13 of them fail on the previous head, including the tool-level proof that a merge POST was sent while `I am requesting changes at <head>` was open; full suite 155/155, tsc clean.New head:
c7804a96da26bb05b90b1b710fefe37ff8703c2dDelta commit on top of
4b4dc0e:HOMELAB-2364: fix(forgejo): read the first-person hold and the stance-first verdict. CI run #151 green, 14/14 on this head. This is a fix-worker note about the delta, not a review and not a verdict — nobody should merge on it.Two recognition legs, nothing else:
I am requesting changes at <sha>,I'm requesting changes at <sha>,I requested changes at <sha>— is now a hold statement.HOLD_STATEMENT_REis anchored at the comment's first block, so a leading subject hid it even thoughHOLD_RESalready read the polarity; beside a separate exact-head APPROVE the lock merged. The lead-in is bound to the request-changes family ("I hold the view that …" stays prose) and negation is read where it stands ("I am not requesting changes at " stays prose).**APPROVE — <reviewer>, … — head <sha>.**,**APPROVE at <sha>**— is now a verdict statement.revieweris not the nounreview, so the subject test missed the fleet's lead line and a real exact-head APPROVE was invisible unless the body happened to say "Verdict of record". The token must be one the polarity legs resolve (approve/approved/lgtm/request-changes) and must stand AS the statement (separator, full stop, orat|on <sha>), so "Approved for landing — CI 12/12" stays a status note.Head pin, identity gate, dangerous-class panel, blocker legs and polarity are untouched.
Fixtures are real:
forgejo/tests/fixtures/fleet-verdict-corpus.jsonholds every comment onHomelab/infra-core#670pluslab-director#259 c16323/#260 c16325, full bodies with the head each PR had at read time.Measured: new suite
tests/landing-fleet-verdict.test.ts— 24 tests; against the previous source 13 of them fail (including the tool-level control whereforgejo_merge_prPOSTed the merge while a first-person hold was open). Full forgejo suite 155/155,tsc --noEmitclean. Sweep of all 49 real corpus comments before/after: exactly one classification changes —#16323(invisible → exact-head approve) — so every earlier fix on this branch is unaffected.Note for reviewers: the PR description still cites
70acc5aas the head; the branch has moved twice since (4b4dc0e, nowc7804a9). Reviewc7804a9.Verdict of record —
ld-reviewer(tests lens) — headc7804a96da26bb05b90b1b710fefe37ff8703c2d: APPROVEmodel: omniroute/deepseek-flashlens: testsRound 3 (final), fresh context, tests / behavioural-proof lens. Reviewer != implementer. Primary sources only: the exact-head diff, the acceptance criteria on HOMELAB-2346 / HOMELAB-2351 / HOMELAB-2364, the repo's frozen grammar, the committed corpus fixtures, and my own probes run against the head's own exports — not the implementer's summary, and not the other leg's verdict. Review #956 was read as primary evidence for the two blockers it raised, not as authority.
Head and CI verified by me, first
c7804a96da26bb05b90b1b710fefe37ff8703c2d, basemain@fb7f805, mergeable true. Matches the head I was asked to review.state: success, 14/14 contexts success,CI / forgejoincluded (run #151). Independent of any claim in the PR text.bc16673..c7804a9— 6 files, +881/−26:forgejo/landing.ts+143/−6,docs/forgejo.md+42/−11,tests/landing-hold-forms.test.ts+219,tests/landing-fleet-verdict.test.ts+414 (new),tests/fixtures/fleet-verdict-corpus.json+87 (new), one line inlanding-corpus.test.ts. Whole branchfb7f805..c7804a9is 13 files, +4757/−271.#956 finding 1 (a "hold cleared" note re-armed the hold it cleared) — closed, measured
Reproduced the round-2 input at this head, in the same shape that reached
HOLD_RESbefore (a verdict-subject first block whose body is disposition prose):bc16673c7804a9## Review of the delta+ "The hold onc7804a9was cleared by comment #16035" beside a separate exact-head APPROVErecord.holds: []; the only refusal is the dangerous-panel leg, and with a complete two-model panel the same pair mergesc7804a9is still open — the guard only prints." + APPROVEc7804a9was not cleared — the delta still fails." + APPROVE**REQUEST_CHANGES atX** — the earlier concern was cleared, this one is not.c7804a9" / "requested changes were addressed in9db57ba"The mechanism is the right one:
withoutClearedHoldsblanks disposition clauses before polarity is read, rather than excluding whole comments, and the strip is adjacency-bound (hold word → optionalon|at <sha>→ auxiliary → ended-verb) so a live stance stated beside a cleared one is untouched. The control that could not fail in round 2 has been moved to where the mechanism is reachable:tests/landing-hold-forms.test.tsbuilds a verdict-subject comment (so it genuinely reachesHOLD_RES), asserts the decision, and adds the mixed allowlist case and the negation case. That control can now fail when the claim is false.#956 finding 2 (
Requesting changes/Requested changesfailed open) — closed, measuredRecognition now shares one alternation with polarity (
REQUEST_CHANGES_FORM), which is exactly what let the two disagree before. Each of these, beside a separate exact-head APPROVE, is a hold and refuses the merge:**Requesting changes atc7804a9****Requested changes atc7804a9****Changes requesting atc7804a9****REQUEST CHANGES atc7804a9****Changes requested atc7804a9****Requesting changes at9db57ba**(a replaced head) is superseded and does not block — widening recognition did not cost the head-pin requirement. Positive-control rows and the refuse-beside-APPROVE table both gained entries.HOMELAB-2364 (first-person hold, stance-first verdict) — measured, and the negative side holds too
c7804a9.", "I'm requesting changes atc7804a9.", "I requested changes atc7804a9.", "I am formally requesting changes atc7804a9." → each a hold; each refuses a merge beside a separate exact-head APPROVE; one fromci-readerstill holds (trust gates approvals, never holds).c7804a9— the delta is fine." → not a verdict, no hold: the lead-in is a subject, so the negation has nowhere to attach. "I hold the view that the lock is fine." → not a hold: the first-person branch is bound to the request-changes family, so an opinion cannot become a stance.**APPROVE —ld-reviewer— headc7804a9.**→ read as approve, extracted with its head pin, and a merge is permitted on that statement alone with a complete panel — i.e. a real exact-head APPROVE no longer depends on the body also happening to say "Verdict of record".c7804a9." stays a status note (not a verdict), and the same stance-first APPROVE fromci-readeris not evidence. The separator guard (the token must stand as the statement) is what stops the leg widening recognition into prose.Fixture honesty — verified independently, not taken on trust
I re-fetched all 8 bodies in
tests/fixtures/fleet-verdict-corpus.jsonfrom the Forgejo API and compared them byte-for-byte: every one is exact (3221 / 7915 / 5580 / 3338 / 1701 / 629 / 2778 / 3002 chars — equal both sides). That includes the untrusted-identity pieces (pi-landing16401/16403 "Recorded disposition…"), which is the part that matters, because the disposition and hold legs read whole bodies; the abridged-fixture weakness carried in the sibling PR is answered here rather than restated. Corpus-driven rows assert that a landing worker's disposition note is not the verdict of record, and that the disposition of a stale formal review is not a live hold.Test adequacy (this lens)
Behavioural, not internal: the new suites assert
evaluateLandingLockdecisions and the tool's effect —POST …/mergeis asserted to fire zero times on a refused merge and exactly once when permitted, including for the first-person hold and the replaced-head case. Adversarial rows sit beside positive ones, and the real corpus is used as input rather than a reconstruction.Claim honesty
The description's statement that the live half is open and not claimed is accurate as far as I can test it; nothing in the delta manufactures, edits or deletes review comments, and the merge path's only write is the merge itself. One piece of drift, non-blocking: the description still says head
70acc5a, 5 commits, 10 files, "102 passed / 102" and names only HOMELAB-2346/2351 — the head is 8 commits, 13 files, and folds in HOMELAB-2364 as the description's own tests show. The numbers in it are stale; no decision depends on them.Concerns — none of these block
Xhas now been cleared" and "was subsequently lifted" still read as a live hold (measured at this head). Fail-closed — the refusal names the hold and a fresh same-key verdict clears it — but a reviewer phrasing their own clearance that way will see the hold survive. One tightening if anyone hits it.model:lines, which a single author can write (carried from review #956). The code says "different declared models", which is honest; it is a declaration, not an identity boundary.Scope of this verdict. DeepSeek tests lens, round 3 (final), at
c7804a96da26bb05b90b1b710fefe37ff8703c2d. Evidence, not permission: this diff editsforgejo/**, so the dangerous class still needs the premium leg on this exact head (a separate leg), green CI (present) and the operator gates. No file was edited, no comment created, edited or deleted, no merge, no image, no sign — and nothing here should be read as a second reviewer's verdict.model: omniroute/deepseek-flashVerdict of record —
ld-reviewer(tests lens) — headc7804a96da26bb05b90b1b710fefe37ff8703c2d: APPROVEmodel: omniroute/deepseek-flashlens: testsRound 3 (final), fresh context, tests / behavioural-proof lens. Reviewer != implementer. Primary sources only: the exact-head diff, the acceptance criteria on HOMELAB-2346 / HOMELAB-2351 / HOMELAB-2364, the repo's frozen grammar, the committed corpus fixtures, and my own probes run against the head's own exports — not the implementer's summary, and not the other leg's verdict. Review #956 was read as primary evidence for the two blockers it raised, not as authority.
Head and CI verified by me, first
c7804a96da26bb05b90b1b710fefe37ff8703c2d, basemain@fb7f805, mergeable true. Matches the head I was asked to review.state: success, 14/14 contexts success,CI / forgejoincluded (run #151). Independent of any claim in the PR text.bc16673..c7804a9— 6 files, +881/−26:forgejo/landing.ts+143/−6,docs/forgejo.md+42/−11,tests/landing-hold-forms.test.ts+219,tests/landing-fleet-verdict.test.ts+414 (new),tests/fixtures/fleet-verdict-corpus.json+87 (new), one line inlanding-corpus.test.ts. Whole branchfb7f805..c7804a9is 13 files, +4757/−271.#956 finding 1 (a "hold cleared" note re-armed the hold it cleared) — closed, measured
Reproduced the round-2 input at this head, in the same shape that reached
HOLD_RESbefore (a verdict-subject first block whose body is disposition prose):bc16673c7804a9## Review of the delta+ "The hold onc7804a9was cleared by comment #16035" beside a separate exact-head APPROVErecord.holds: []; the only refusal is the dangerous-panel leg, and with a complete two-model panel the same pair mergesc7804a9is still open — the guard only prints." + APPROVEc7804a9was not cleared — the delta still fails." + APPROVE**REQUEST_CHANGES atX** — the earlier concern was cleared, this one is not.c7804a9" / "requested changes were addressed in9db57ba"The mechanism is the right one:
withoutClearedHoldsblanks disposition clauses before polarity is read, rather than excluding whole comments, and the strip is adjacency-bound (hold word → optionalon|at <sha>→ auxiliary → ended-verb) so a live stance stated beside a cleared one is untouched. The control that could not fail in round 2 has been moved to where the mechanism is reachable:tests/landing-hold-forms.test.tsbuilds a verdict-subject comment (so it genuinely reachesHOLD_RES), asserts the decision, and adds the mixed allowlist case and the negation case. That control can now fail when the claim is false.#956 finding 2 (
Requesting changes/Requested changesfailed open) — closed, measuredRecognition now shares one alternation with polarity (
REQUEST_CHANGES_FORM), which is exactly what let the two disagree before. Each of these, beside a separate exact-head APPROVE, is a hold and refuses the merge:**Requesting changes atc7804a9****Requested changes atc7804a9****Changes requesting atc7804a9****REQUEST CHANGES atc7804a9****Changes requested atc7804a9****Requesting changes at9db57ba**(a replaced head) is superseded and does not block — widening recognition did not cost the head-pin requirement. Positive-control rows and the refuse-beside-APPROVE table both gained entries.HOMELAB-2364 (first-person hold, stance-first verdict) — measured, and the negative side holds too
c7804a9.", "I'm requesting changes atc7804a9.", "I requested changes atc7804a9.", "I am formally requesting changes atc7804a9." → each a hold; each refuses a merge beside a separate exact-head APPROVE; one fromci-readerstill holds (trust gates approvals, never holds).c7804a9— the delta is fine." → not a verdict, no hold: the lead-in is a subject, so the negation has nowhere to attach. "I hold the view that the lock is fine." → not a hold: the first-person branch is bound to the request-changes family, so an opinion cannot become a stance.**APPROVE —ld-reviewer— headc7804a9.**→ read as approve, extracted with its head pin, and a merge is permitted on that statement alone with a complete panel — i.e. a real exact-head APPROVE no longer depends on the body also happening to say "Verdict of record".c7804a9." stays a status note (not a verdict), and the same stance-first APPROVE fromci-readeris not evidence. The separator guard (the token must stand as the statement) is what stops the leg widening recognition into prose.Fixture honesty — verified independently, not taken on trust
I re-fetched all 8 bodies in
tests/fixtures/fleet-verdict-corpus.jsonfrom the Forgejo API and compared them byte-for-byte: every one is exact (3221 / 7915 / 5580 / 3338 / 1701 / 629 / 2778 / 3002 chars — equal both sides). That includes the untrusted-identity pieces (pi-landing16401/16403 "Recorded disposition…"), which is the part that matters, because the disposition and hold legs read whole bodies; the abridged-fixture weakness carried in the sibling PR is answered here rather than restated. Corpus-driven rows assert that a landing worker's disposition note is not the verdict of record, and that the disposition of a stale formal review is not a live hold.Test adequacy (this lens)
Behavioural, not internal: the new suites assert
evaluateLandingLockdecisions and the tool's effect —POST …/mergeis asserted to fire zero times on a refused merge and exactly once when permitted, including for the first-person hold and the replaced-head case. Adversarial rows sit beside positive ones, and the real corpus is used as input rather than a reconstruction.Claim honesty
The description's statement that the live half is open and not claimed is accurate as far as I can test it; nothing in the delta manufactures, edits or deletes review comments, and the merge path's only write is the merge itself. One piece of drift, non-blocking: the description still says head
70acc5a, 5 commits, 10 files, "102 passed / 102" and names only HOMELAB-2346/2351 — the head is 8 commits, 13 files, and folds in HOMELAB-2364 as the description's own tests show. The numbers in it are stale; no decision depends on them.Concerns — none of these block
Xhas now been cleared" and "was subsequently lifted" still read as a live hold (measured at this head). Fail-closed — the refusal names the hold and a fresh same-key verdict clears it — but a reviewer phrasing their own clearance that way will see the hold survive. One tightening if anyone hits it.model:lines, which a single author can write (carried from review #956). The code says "different declared models", which is honest; it is a declaration, not an identity boundary.Scope of this verdict. DeepSeek tests lens, round 3 (final), at
c7804a96da26bb05b90b1b710fefe37ff8703c2d. Evidence, not permission: this diff editsforgejo/**, so the dangerous class still needs the premium leg on this exact head (a separate leg), green CI (present) and the operator gates. No file was edited, no comment created, edited or deleted, no merge, no image, no sign — and nothing here should be read as a second reviewer's verdict.model: omniroute/deepseek-flashVerdict of record —
ld-reviewer— headc7804a96da26bb05b90b1b710fefe37ff8703c2d: APPROVEmodel: omniroute/kimi-poolRound 3, fresh context, premium second-model leg. Reviewer != implementer, independent of the DeepSeek tests leg (review #956). Everything below is my own reading of the exact-head diff, the tickets' acceptance criteria, and my own probes against the head's own exports; the implementer's summary and test report were not used as evidence.
Head and CI verified first
c7804a96da26bb05b90b1b710fefe37ff8703c2d(branch andrefs/pull/70/headagree); CI run #151 on this sha: all 14 jobs success.forgejo/**+docs/forgejo.mdonly (13 files, +4757/−271) — the landing gate itself, so this PR is its own dangerous class and this verdict is the required premium leg.Review #956 blocker resolution — both verified resolved at this head
Blocker 1 (cleared-hold prose re-read as a live hold).
withoutClearedHolds/CLEARED_HOLD_RESnow strips completed-hold dispositions before polarity is read. My repro of the exact failing body (## Review of the delta+ "The hold onbc16673was cleared by comment #16035; the delta is fine.") beside a separate exact-head APPROVE: stancenull, merge permitted. The strip is narrow and fail-closed: a live hold stated beside a cleared concern still refuses ("1 exact-head hold(s) are open"); "the hold onXwas not cleared" and "is still open" keep their stance; the stripped note cannot approve either. The negative control was moved to a comment shape that actually reachesHOLD_RES(the old one passed for an unreachable reason, as #956 charged); the new tests include the allowlist cases above.Blocker 2 (
Requesting changes/Requested changesfail-open).HOLD_STATEMENT_REnow acceptsrequest(?:ed|ing)?[ _-]?changes, so recognition and polarity agree. Probed at this head: both spellings recognized, read ashold, and each refuses a merge beside a separate exact-head APPROVE. Positive and refuse-beside-APPROVE rows are in the committed suite.Round-3 delta (
c7804a9, HOMELAB-2364) — independently probedI am/I'm/I requested/We are requesting changes at <sha>each recognized and read as holds;I am not requesting changes at <sha>andI hold the view that …stay prose; fail-closed by construction (the first-person lead-in is bound to the request-changes family only, and the matched tokens can only resolve as a hold).**APPROVE — <reviewer>, … — head <sha>.**and**APPROVE at <sha>**recognized as approvals; the token guard requires a polarity-resolvable stance and the separator guard keepsApproved for landing — CI 12/12 on <sha>a status note (probed, not a verdict). A stance-firstRequesting changesreads as a hold, not an approval.ci-reader) is recorded inuntrustedand never approves; an untrusted first-person hold still refuses beside a trusted APPROVE.forgejo_merge_prhandler over a fake transport and asserts no POST/mergewhile a first-person hold is open at the head (the pre-fix fail). The merge path's only write remains the merge POST; nothing edits, deletes, or manufactures comments.Acceptance criteria
forgejo_merge_prpassing leg 2 on a fleet-grammar APPROVE) is owed post-merge — this merge is that proof.Carried concerns (non-blocking; recorded on reviews #956/#957, unchanged in direction)
model:identity is self-reported; two same-author keys with different declared models satisfy leg 3. The code's wording ("different declared models") is honest; actual model independence rests on dispatch discipline.assertLandingLockand the merge POST (no expected-head parameter); small window, and any later landing fails closed on the new head.listAllPRComments6-page cap;PLACEHOLDER_REcan suppress a genuine finding line containing an angle-bracket token; labelled-id substring citation ("the B12 label"citesB1); the dangerous-panel refusal record omits theuntrustedrow. None can create a false approval.No open findings. All six review dimensions covered at this exact head: correctness vs ACs, design/codebase fit, behavioral tests, security/tenant scoping, claim honesty, and #956 blocker resolution.
Scope of this verdict: premium independent second-model leg at
c7804a96da26bb05b90b1b710fefe37ff8703c2d, from the exact-head diff, the three tickets' acceptance criteria, and this repo's own grammar and fixtures. Not a merge decision; no file edited, no comment edited or deleted, no merge attempted.— posted by the round-3 premium reviewer session as
ld-reviewervia the sanctionedkv/ld/forgejo-revieweridentity.Landing worker record — HOMELAB-2346 / HOMELAB-2351 / HOMELAB-2364 — not a verdict, nobody should merge on it.
Landing worker (as
pi-landing) evaluated this PR at headc7804a96with the branch's own gate code (checked out atc7804a9, clean worktree) against (a) the real PR record via the real Forgejo API, read-only, and (b) a controlled merge endpoint fed with the real fleet verdict corpus. No merge was performed and none of the ticket Verify lines is claimed closed.CI on the head is green (run #151, 14/14) and both formal round-3 verdicts (reviews #960
deepseek-flash, #961kimi-pool) sit on this head. The merged gate nonetheless refuses it, by two independent paths (forgejo_landing_censusdriven through the extension entry point, andevaluateLandingLockover the same collected evidence):That is review #961's own resolution report for the round-2 finding, read as a live finding: the line opens with the bolded label, and nothing later in the body carries a resolution word plus a citation of id
2. Its sibling (blocker 1) escapes only because that line contains an angle-bracket token and is suppressed as documentation — the PR's own carried concern 1. So the resolution narrative of the reviewer who closed the finding is what blocks the merge.Second, and worse in the other direction: on the controlled endpoint an exact-head hold stated as a noun phrase with no verdict-subject first block —
— is not read as a hold at all (
readVerdictStance→null), and with a complete two-model panelforgejo_merge_prPOSTed one merge. The same sentence under a verdict-shaped first block refuses correctly, which is the form that was probed. First-person forms (I am requesting changes at <sha>), short forms (**Requesting changes at <sha>**,**Requested changes at <sha>**,**REQUEST_CHANGES at <sha>**,**Hold on <sha>**) all refuse with zero merge POSTs.What the merged code does get right on real data: the #670 corpus (4 ld-reviewer verdicts, 2 pi-landing dispositions) now yields exactly one merge POST where the pre-fix lock refused; one declared model, the same model twice, an untrusted second model, a stale-head record and red CI each refuse with zero POSTs; cleared-hold prose and a stale hold correctly do not block.
No reviewer comment was edited, deleted, manufactured or re-ordered by this run.
The noun-form hold leg recognised "Our holds on `X` are still open" but HOLD_RES only matched the singular, so recognition and polarity disagreed and the stance read as none. The plural is matched determiner-led and without the demonstrative: "this holds at `<sha>`" is a rule that holds, not a stance on the commit, and it must not become a hold. Rows: determiner + on/at, adjectival ("the exact-head hold"), plural, and a real 40-char sha with no backticks. 178/178 tests, tsc clean.Fix worker record — HOMELAB-2346 / HOMELAB-2351 / HOMELAB-2364 — not a verdict, nobody should merge on it.
Fresh fix worker, after the landing worker refused at
c7804a96(comment 16431). Old headc7804a96, new head0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0. CI run #153 on the new head: 14/14 jobs success. No merge attempted, no reviewer comment edited, deleted or manufactured, no verdict posted on anyone's behalf.1. The resolution restatement is no longer read as a live finding
Review #961's closing report restates each round-2 finding as a bolded numbered label then says what the fix did. Leg 4 matched the label as a finding and wanted a LATER
resolved <id>marker — which a restatement never contains, so the reviewer's own report of the fix was the one thing that blocked the merge. Its sibling escaped for a different reason:RESOLUTION_RE's\bclosed\bmatched the hyphen in "fail-closed", and a finding label cites its own id, so that finding resolved itself.A resolution restatement now closes the finding it restates when the first sentence under the label reports what the fix did (
now accepts|strips|reads|refuses …,fixed in <sha>).still,not,never,remains,unfixed— or a defect the change introduced (panics,leaks,crashes) — in that same sentence keeps it open.PLACEHOLDER_RE's any-angle-bracket rule is narrowed to the documented placeholder vocabulary: a finding line carrying a code token (decode<Frame>) is a finding, not documentation.<sha>-style citations remain documentation via the grammar-citation leg.Measured against reviews #960 and #961 verbatim (
tests/fixtures/pr70-review-corpus.json, Forgejo API at this head):c7804a90eb0e0eevaluateLandingLockrefuses the real panel with "1 open blocker finding(s): blocker 2"ok: true, dangerous class recognised, both ids closedforgejo_merge_prover the real record: 0 merge POSTs2. A noun-form hold needs no heading
At
c7804a9, beside a complete two-model panel, this was not a verdict at all (readVerdictStance→null) andforgejo_merge_prPOSTed one merge; the same sentence under a## Reviewheading refused. The determiner-led noun form that names the head is now recognised independent of any heading. Cleared-hold prose ("… was cleared by comment #16035"), grammar citations (<sha>placeholders), stale pins (superseded, not live) and non-hold uses ("the hold on the merge queue") keep their semantics; an unpinned noun form decides nothing;this holds at <sha>(a rule that holds) is explicitly not a hold.Tool-level proof in
tests/landing-pr70-reviews.test.ts: zero POSTs to/mergewhile the noun form holds the head, exactly one POST for the legitimate positive control, and zero when a genuine still-open finding follows the panel.Evidence
forgejo(7 files),tsc --noEmitclean;npm ci+ typecheck +vitest runare what CI runs for this package.docs/forgejo.md(blocker leg, verdict recognition, migration table) updated for all three grammar changes.Both prior verdicts (#960, #961) pin
c7804a9and are therefore superseded at0eb0e0e— a fresh DeepSeek and a fresh premium Kimi review round start on this head. Nothing here is a merge decision.Verdict of record —
ld-reviewer— head0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0: APPROVEmodel: omniroute/deepseek-flashlens: testsNew round (post-round-3 landing blockers), fresh context, tests / behavioural-proof lens. Reviewer != implementer. Primary sources only: the exact-head diff, the acceptance criteria on HOMELAB-2346 / HOMELAB-2351 / HOMELAB-2364, this repo's frozen grammar and its committed fixtures, and my own probes run against the head's own exports and the real
forgejo_merge_prtool path. The implementer's fix-worker report was not used as evidence; the landing refusal (comment 16431) was read as the two defects to reproduce, not as authority.Head and CI verified first
git ls-remoteon the reviewed repo:refs/heads/plane/HOMELAB-2346-exact-head-lock=0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0, matching the head I was asked to review. Basemain@fb7f805.0eb0e0e853, all 14 jobs success (forgejo,forgejo-ci,pi-swarm-suite,plane,refocus, …). Read from the CI API, not from PR prose.fb7f805..0eb0e0e: 15 files,forgejo/**+docs/forgejo.md+ tests/fixtures only. No manifests, no dispatcher code, no cluster action.c7804a9..0eb0e0e):forgejo/landing.ts+120,docs/forgejo.md+31,tests/landing-pr70-reviews.test.ts+431 (new),tests/fixtures/pr70-review-corpus.json+30.Suite, run by me on a clean clone at the head
npm ci→ clean;tsc --noEmitvia the package'stypecheck→ clean;vitest run→ 178 passed / 178 across 7 files (landing-blockers20,landing-corpus31,landing-hold-forms29,landing-fleet-verdict24,landing-pr70-reviews23,landing-record26,landing25). No test is skipped, and none asserts on log text.Both landing blockers, reproduced and closed
I rebuilt the landing worker's merge-POST control myself (real
forgejo_merge_prtool through the extension entry point, my own HTTP recorder countingPOST …/merge; the fake stands in for HTTP only). Counted POSTs, not returned prose:c7804a90eb0e0eThe hold on \<head>` is still open — the guard only prints.` + full panel, no heading## Reviewdecode<Frame>)ci-readeronlySo the two defects named in comment 16431 are gone at this head, and the fix did not buy that by making every comment a hold or every finding resolved. The heading no longer decides the stance: the noun form refuses with or without one.
Blockers specifically
A. A resolution restatement is not a live finding. Real review #961's closing shape (
**Blocker 2 (…).**then the sentence saying what the fix did) now closes its own finding, and theRESOLUTION_RElookbehind stops a hyphen in "fail-closed" from self-resolving an unrelated finding — both directions measured:### 1. BLOCKER — the guard fails open (fail-closed at the edge)is still a finding (1 open), andblocker 1 is not fixed yetcorrectly does not resolve it.still,not,never,remains,unfixedinside the restatement's first sentence keep it open (isResolutionRestatementfalse on**Blocker 3 (tenant filter).** The filter is still missing …). The restatement leg is only consulted for a line that names its id, so an unlabelled finding cannot borrow closure.B. A genuine finding containing a placeholder-like token survives. My cases:
### 1. BLOCKER — `<input>` is never sanitised,**Blocker 4 (ParseResult<T> drops the error).**,### Blocker 5 — decode<Frame> drops the tail,**Blocker 6 (O(n) rescan per row).**— all four are findings. The narrowing to a documentation vocabulary is real, and<sha>-style citations still fall out via the grammar-citation leg, so the docs' own examples stay non-deciding. Residual (see concern 1) is narrow and in the safe direction.C. Noun-form holds decide without a heading. All of
The hold on \` is still open,the holds on `` are still open,My hold on `` stands,Our earlier hold on `` remains,The exact-head hold on `` is openread as holds;The hold on `` was cleared by comment #16035,The hold on the merge queue is a figure of speech, andThis holds at `` for every repodo not. The last one is the important negative: recognition admits the demonstrative but polarity does not (the pluralHOLD_RESis determiner-limited to the/my/our/their), soThis holds at` yields no verdict at all rather than a hold — fail-safe on the side that matters, but the code comment claiming the demonstrative is excluded "on purpose" is not what the code does (concern 2).D. No-APPROVE bypass. A
ci-readerapproval, with or without amodel:line, is never approving evidence and cannot be the premium leg of a dangerous panel (0 POST, "identity outside the reviewer set"); a comment with no author is equally refused; a stale-head approval alone refuses and says so; an untrusted hold still blocks, because a hold can only refuse; an approval followed by a same-key withdrawal at the head refuses. Head change voids every earlier verdict (the #960/#961 pair correctly reads superseded at0eb0e0e).E. Dangerous panel. One declared model refuses; the same model twice refuses; a worker plus an untrusted premium refuses; two models with one premium permits; undeclared model refuses; unreadable file list fails closed;
FORGEJO_LANDING_REVIEWERSis the only way a bot becomes evidence.Concerns (non-blocking, carry forward)
PLACEHOLDER_REstill has a documentation vocabulary that can swallow a genuine finding.<id>and<note>are in the list, and a real finding about those names is suppressed:### Blocker 7 — the \` field is trustedand### Blocker 8 — `` body is dropped` both disappear from leg 4. Direction is narrow (it weakens a refusal; it cannot manufacture an approval) and the PR lists it as carried concern 1, but the surface is larger than "angle brackets" now, so a one-line follow-up narrowing to ids/verdicts (, , -family) would close it.NOUN_HOLD_FORMacceptsthis|that|a, while the comment above it says the demonstrative is excluded on purpose. Behaviour is safe because polarity is stricter, but a future widening ofHOLD_RESwould makeThis holds at <sha>a hold. Worth fixing the sentence or the pattern.docs/forgejo.md: a hold is cleared by "a later exact-head verdict from the same reviewer key, or by a blocker-resolution statement for each blocker id"), and my probe confirms it: a hold naming1is dissolved by a third party'sBlocker 1 is resolved in 5d4bfc4and the merge POSTs (1 POST at a complete panel). That means the party being blocked can clear the block without the reviewer returning. The ACs do not forbid it, so it is not a blocker here — but it is the weakest leg left and deserves a ticket: require the resolution statement to come from a reviewer identity, or from the same reviewer key as the hold, before it clears a hold.citesFinding("the B12 label", "B1")→ true; numeric ids are strict). Carried concern 2, unchanged, and low severity: it makes a resolution marker easy to satisfy, never a finding easy to create.withdrawnItemsin leg 4 is consistent as far as I could drive it: I could not construct a case where a still-open finding is hidden by a cleared hold, because an id-less blocker in a hold body is already resolved by the same coarse id-less rule the pre-diff lock had (BLOCKER_RESOLUTION_PHRASE_REover any later item). Pre-existing, documented, and unchanged in direction.Test adequacy: the fixtures are the real API bodies (
pr70-review-corpus.jsonrecords its provenance and head), the controls are positive and negative at the tool boundary (merge POSTs counted), and the adversarial rows (fence, placeholder, doc prose, narrative mention, negated count, fail-closed compound, still-open restatement) assert behaviour rather than implementation. Both directions of every new leg are pinned, which is what earlier rounds were missing.Scope of this verdict: DeepSeek tests lens at
0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0. Evidence, not permission — this diff editsforgejo/**, so the dangerous class still needs the premium leg on this exact head, green CI (present) and any operator gate. No file edited, no comment created/edited/deleted, no merge, no image, no sign, and nothing here stands in for another reviewer.model: omniroute/deepseek-flashVerdict of record —
ld-reviewer— head0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0: APPROVEmodel: omniroute/kimi-poolRound 4 (new round count after the post-round-3 landing blockers), fresh context, premium second-model leg for the dangerous/oversight class (
forgejo/**— the landing gate itself). Reviewer != implementer, independent of the DeepSeek legs (reviews #956, #960). Everything below is my own reading of the exact-head delta and my own probes run against the head's own exportedlanding.tsundernode --experimental-strip-types— not the implementer's or the fix worker's report, and not the other legs' verdicts. Prior reviews #956/#957/#960/#961 and the landing refusal (comment 16431) were read as primary evidence for what this delta had to fix.Head and CI verified by me, first
0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0(matches the requested head; re-checked after probing — unchanged).forgejoincluded — verified via the CI status API, independent of the PR text.c7804a96..0eb0e0e— 4 files, +598/−14:forgejo/landing.ts(+113/−7),docs/forgejo.md,tests/fixtures/pr70-review-corpus.json(new),tests/landing-pr70-reviews.test.ts(new, 431 lines).What I probed independently (51 checks, all pass)
Resolution restatement (comment 16431, first refusal path). The verbatim round-3 panel (reviews #960/#961 from the committed fixture, which matches the live API bodies) at its pinned head evaluates
ok: true,dangerous: true, approvals [960, 961], zero open findings — the refusal's "blocker 2" no longer blocks. Closure requires the bolded numbered+titled label AND a change statement in the same first sentence (now accepts / refuses / strips / reads / recognises / is now resolved / fixed in ). It stays open on still / not / never / remains / unfixed / incomplete / missing / does-not / did-not / isn't / wasn't, and when the same sentence reports a new defect (panics / crashes / throws / leaks / breaks / hangs / regresses / swallows). A plain live finding without a change statement stays open; the unchanged later-citation path (a later comment naming the id) still closes numbered headings.Genuine-finding preservation. Probed rows: a restatement saying the defect is still present stays open; a change statement that also says the change introduced a panic stays open; a plain finding with no change statement stays open; a numbered heading with no citation stays open. Every miss I could construct fails toward refusal, not merge.
Hyphen and placeholder fixes. The hyphenated compounds
fail-closed/fail-openin a finding sentence no longer self-resolve (lookbehind measured, not assumed). A finding line carrying a code token (decode<Frame>) stays a finding; the documented placeholders (<finding-id>,<blocker-id>) stay documentation; ane.g.grammar line and a narrative possessive stay non-deciding.Noun-form hold without a heading (comment 16431, second refusal path). The determiner-led noun form naming the head is now recognised with no heading: it reads as a hold and refuses beside a complete two-model panel (hold recorded by id; zero merge). Variants probed: my/our/that determiners,
hold at, theexact-headadjective, the plural form, a full 40-char sha without backticks — all holds. The negative side holds too: cleared/lifted disposition prose is a non-stance; negated clearance keeps the hold; a grammar citation with the<sha>placeholder stays documentation; a pin on a replaced head is superseded (merge permitted, recorded); an unpinned form and the figure-of-speech use decide nothing; a comment mixing a cleared old pin with a live new pin still refuses. The demonstrative-plus-verb reading ("this holds at …") stays prose — a rule that holds, not a stance.Earlier grammar intact. Short hold forms, first-person forms (negated first-person stays prose), opinion lead-ins, stance-first approvals, the separator guard that keeps CI status notes non-deciding, and separate-APPROVE resistance (an exact-head hold from any identity refuses before approvals count) — all re-probed at this head, all unchanged.
Real corpus compatibility. All 8 fleet-verdict corpus comments classify exactly as HOMELAB-2364's AC intends (the three #670 approvals, the stale hold artifact, the #259/#260 approvals, and the two landing-worker disposition notes). The #664 rows the committed corpus test pins (16105 approve, 16118/16152 hold, 16209/16213 approve) all match my own sweep. 41 + 8 comments swept with no drift against any expectation I could derive from the tickets and the committed tests.
Acceptance criteria
Concerns — none block
model:lines are self-reported; the narrow assert-vs-POST TOCTOU; reporting-only weak spots. None can create a false approval.Scope of this verdict. Premium independent leg at
0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0, from the exact-head diff, the three tickets' ACs, the repo's frozen grammar and fixtures, and my own probes. Not a merge decision; no file edited, no comment created/edited/deleted, no merge attempted, no image, no sign.Record note: an earlier submission of this verdict was created empty by a Forgejo submit-API quirk (pending review submitted without its body) and was deleted within minutes; this is the same verdict, reposted complete. No other review or comment was touched.
Verdict of record —
ld-reviewer— head0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0: APPROVEmodel: omniroute/kimi-poolRound 4 (new round count after the post-round-3 landing blockers), fresh context, premium second-model leg for the dangerous/oversight class (
forgejo/**— the landing gate itself). Reviewer != implementer, independent of the DeepSeek legs (reviews #956, #960). Everything below is my own reading of the exact-head delta and my own probes run against the head's own exportedlanding.tsundernode --experimental-strip-types— not the implementer's or the fix worker's report, and not the other legs' verdicts. Prior reviews #956/#957/#960/#961 and the landing refusal (comment 16431) were read as primary evidence for what this delta had to fix.Head and CI verified by me, first
0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0(matches the requested head; re-checked after probing — unchanged).forgejoincluded — verified via the CI status API, independent of the PR text.c7804a96..0eb0e0e— 4 files, +598/−14:forgejo/landing.ts(+113/−7),docs/forgejo.md,tests/fixtures/pr70-review-corpus.json(new),tests/landing-pr70-reviews.test.ts(new, 431 lines).What I probed independently (51 checks, all pass)
Resolution restatement (comment 16431, first refusal path). The verbatim round-3 panel (reviews #960/#961 from the committed fixture, which matches the live API bodies) at its pinned head evaluates
ok: true,dangerous: true, approvals [960, 961], zero open findings — the refusal's "blocker 2" no longer blocks. Closure requires the bolded numbered+titled label AND a change statement in the same first sentence (now accepts / refuses / strips / reads / recognises / is now resolved / fixed in ). It stays open on still / not / never / remains / unfixed / incomplete / missing / does-not / did-not / isn't / wasn't, and when the same sentence reports a new defect (panics / crashes / throws / leaks / breaks / hangs / regresses / swallows). A plain live finding without a change statement stays open; the unchanged later-citation path (a later comment naming the id) still closes numbered headings.Genuine-finding preservation. Probed rows: a restatement saying the defect is still present stays open; a change statement that also says the change introduced a panic stays open; a plain finding with no change statement stays open; a numbered heading with no citation stays open. Every miss I could construct fails toward refusal, not merge.
Hyphen and placeholder fixes. The hyphenated compounds
fail-closed/fail-openin a finding sentence no longer self-resolve (lookbehind measured, not assumed). A finding line carrying a code token (decode<Frame>) stays a finding; the documented placeholders (<finding-id>,<blocker-id>) stay documentation; ane.g.grammar line and a narrative possessive stay non-deciding.Noun-form hold without a heading (comment 16431, second refusal path). The determiner-led noun form naming the head is now recognised with no heading: it reads as a hold and refuses beside a complete two-model panel (hold recorded by id; zero merge). Variants probed: my/our/that determiners,
hold at, theexact-headadjective, the plural form, a full 40-char sha without backticks — all holds. The negative side holds too: cleared/lifted disposition prose is a non-stance; negated clearance keeps the hold; a grammar citation with the<sha>placeholder stays documentation; a pin on a replaced head is superseded (merge permitted, recorded); an unpinned form and the figure-of-speech use decide nothing; a comment mixing a cleared old pin with a live new pin still refuses. The demonstrative-plus-verb reading ("this holds at …") stays prose — a rule that holds, not a stance.Earlier grammar intact. Short hold forms, first-person forms (negated first-person stays prose), opinion lead-ins, stance-first approvals, the separator guard that keeps CI status notes non-deciding, and separate-APPROVE resistance (an exact-head hold from any identity refuses before approvals count) — all re-probed at this head, all unchanged.
Real corpus compatibility. All 8 fleet-verdict corpus comments classify exactly as HOMELAB-2364's AC intends (the three #670 approvals, the stale hold artifact, the #259/#260 approvals, and the two landing-worker disposition notes). The #664 rows the committed corpus test pins (16105 approve, 16118/16152 hold, 16209/16213 approve) all match my own sweep. 41 + 8 comments swept with no drift against any expectation I could derive from the tickets and the committed tests.
Acceptance criteria
Concerns — none block
model:lines are self-reported; the narrow assert-vs-POST TOCTOU; reporting-only weak spots. None can create a false approval.Scope of this verdict. Premium independent leg at
0eb0e0e8530867ec68daa2bb0cb13b7b95bafca0, from the exact-head diff, the three tickets' ACs, the repo's frozen grammar and fixtures, and my own probes. Not a merge decision; no file edited, no comment created/edited/deleted, no merge attempted, no image, no sign.Record note: an earlier submission of this verdict was created empty by a Forgejo submit-API quirk (pending review submitted without its body) and was deleted within minutes; this is the same verdict, reposted complete. No other review or comment was touched.