HOMELAB-2682: fix(forgejo): 'BLOCKER none.' reads as no findings; 'none of the tests ran' still blocks #107
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-2682-blocker-none"
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
A severity-tokened line whose REMAINDER is a bare empty statement is a claim of zero findings, not a finding. Fix + tests only, on
forgejo/landing.tsand one new test file.Why — the GITS #209 refusal, verbatim
gits-hypervisor's gate refused
Homelab/ghostty-in-the-shell#209(head092174e):One of those five was a review seat's shorthand for "no findings". Comment 22638 (ld-reviewer,
## Verdict: APPROVEat that same head) ends its evidence with:No blockers.negates —BLOCKER none.did not, because the line starts with the severity token.BLOCKER_FINDING_PATTERNSmatched it, and every existing negation pattern requires the token to be the NOUN of a denial (no blockers,blockers: none) rather than the sentence's SUBJECT. The result was one open, id-less finding, which no resolution statement can ever close — so the PR was unlandable for a second typographic reason on top of #103's bolded form. Same family as the- None.artifact fixed for section content lines, on a different surface: the empty statement shares the line with the token instead of sitting alone under a heading.Measured on the real corpus, this branch vs
main, over 79 real ld-reviewer bodies (w3-lane, negation-approval, fleet-verdict, pr70-review fixtures + the three GITS #209 comments):Exactly one line changes; nothing is added anywhere, so no finding anywhere in the corpus became invisible. Re-running the lock eval on the GITS #209 comment set:
main4 open findings (RISK-2, RISK-3, RISK-4, and id-lessBLOCKER none.) → this branch 3 (the three real RISK findings unchanged).The rule: exact token vs content
isSeverityTokenEmptyStatement(line)takes the text after a line-initial severity token and its separator run (bullets, heading marker, emphasis, an optionalseverity:,:/=, whitespace) and asks a single question: is that remainder the empty-statement vocabulary and nothing else?NEG_ZERO_WORD(none|nothing|nil|n/a|zero|0) is the same determiner listNEGATION_CLAIM_RESalready spells out, and the two empty-claim patterns now share that constant.NEGATION_CLAIM_RESalso accepts a word after the determiner (None found.,Nothing outstanding.,0 remaining.) — that is right for a line under a heading, and wrong behind a severity token, because a word after the determiner is content, and content is a defect.plainText), so**BLOCKER** none.,__BLOCKER__ none.,**BLOCKER:** none.read exactly like the plain form (#103).Blockers — none.is a finding on the token path and #103 pinned it with its bolded twin. A dash is where a real finding's body goes (**Blocker** — branch matching picks the wrong run).The guard case — verbatim, as required
Content after the determiner keeps failing closed:
is still one open finding (
tests/landing-severity-none.test.ts, first guard test, asserted on the finding's own line:expect(scan(line)[0].line).toBe(line)). Same forBLOCKER none found here,BLOCKER: none worth mentioning — the auth check is open,BLOCKER: none of the retries back off,BLOCKER 0 tests were run,BLOCKER n/a — the harness never ran, andBLOCKER B1: none.(a label with an empty body is still a labelled finding).Failing test first
tests/landing-severity-none.test.tswas written againstorigin/main'slanding.tsbefore the fix. Onmain(26b5f15):Failed on
main(each of these read as 1 open finding):With the fix:
43 passed. The near-miss family is pinned on both sides —BLOCKER NONE./Blocker None./BLOCKER: nonenegate;BLOCKER none worth mentioningand the whole prose-tail list above stay findings;BLOCKER — none.and**Blockers** — none.stay findings (the #103 dash decision, unchanged).Suites
npx tsc --noEmit— clean.npx vitest run— 34 files, 895 tests passed (33 pre-existing files including the #96–#106 suites, plus the new one).npm run test:deployed(deployed shape perTESTING.md) —ok: 34/34 files loaded and passed, 895/895 testson the pinned tree.Scope
Two files:
forgejo/landing.ts(+95/−2) andforgejo/tests/landing-severity-none.test.ts(new). No behaviour change outside the token-remainder rule; the only vocabulary edit to existing code is factoring the shared determiner constant into two existing patterns with an identical character set.Refs: HOMELAB-2682, GITS-508, #209 (unblocks), #204/#103 (the bolded-form sibling).
Verdict of record — ld-reviewer — head
5d451462bf: APPROVEmodel: deepseek-flash
session: 47db1b27-e3cd-45ec-a49a-8e0e6a8f840c
Independent review seat 2 of 2 (adversarial, gate tooling). Method: read the PR diff at the declared head in a detached worktree, then ran the real
findOpenBlockers/evaluateLandingLock/isBlockerDocumentation/isNegationClaimTexton a 157-line adversarial corpus at both this head andorigin/main(26b5f15) and diffed the two readings line by line. No finding below is inferred from the PR description; every row is measured.Boundary family — every case correct, zero divergences
BLOCKER none.BLOCKER noneBLOCKER NONE.BLOCKER None- BLOCKER: none.**BLOCKER** none.BLOCKER none .BLOCKER none, verified on head X.BLOCKER none remaining.BLOCKER nothing.BLOCKER zero.BLOCKER n/a.BLOCKER no.2 BLOCKER none.BLOCKER none none.Widening check (the direction that loses findings). 157 lines compared against
main; 36 changed, and every single changed line was a zero-claim that moved to no-finding (e.g.BLOCKER N/A.,BLOCKER Nil.,### BLOCKER none.,(BLOCKER) none.,[BLOCKER] none.,BLOCKER \none`.,BLOCKER... none.,BLOCKER none..,blocker none.,## Hold\n\nBLOCKER none.). **Not one must-block line negated.** Specifically still blocking, unchanged:BLOCKER none of the tests ran.,BLOCKER none found here,BLOCKER: none worth mentioning — the auth check is open,BLOCKER 0 tests were run,BLOCKER n/a — the harness never ran,BLOCKER none of the 5 tests ran,BLOCKER none?: see below,BLOCKER none. (tests: 0 ran),BLOCKER none of the above,BLOCKER none-the-less,BLOCKER none_remaining,BLOCKER `none` of the tests ran,severity: BLOCKER: none.(0 on main too). The dash family (#103) is preserved:BLOCKER — none.,BLOCKER – none.,BLOCKER - none.,BLOCKER—none.,Blockers — none.,Blockers — none.all still report one finding, andBLOCKER — none.reads identically toBLOCKER — none.`id-ed family and findings-closure: unaffected.
BLOCKER B1: tenant scoping is missing,BLOCKER B1: none.,BLOCKER B1: none,BLOCKER-1: none.,BLOCKER B1: none of the tests ran.all still yield one finding withid=B1/1.BLOCKER (B1): none.andBLOCKER: (#123) none.also still block (the gap run stops at the label), which is the correct fail-closed reading of a labelled finding with an empty body. Closure behaves identically: onmainthe seat'sresolved B1closes B1 but leaves the id-lessBLOCKER none.open forever (the GITS #209 bug); at this head the same comment set yields zero open findings and B1 closes exactly as before.Doc / prose surfaces: no change. Ten quoted-example shapes read identically at head and
main, including`BLOCKER none.`alone,-BLOCKER none., `"BLOCKER none."`, `> BLOCKER none. — the canonical clean line`,BLOCKER: none of the tests ran.``, and the mixed grammar sentence. The new rule is not silent about them — the existing quoted-example/negation guards already covered every one, and the new check is ordered before the documentation guards only because both directions arereturn false, so the ordering cannot change an outcome. The quoted guard's own reading of a quoted finding (BLOCKER none of the tests ran.) is unchanged frommain.Suites and claims.
npx tsc --noEmitclean (verified myself, not taken from the body).npx vitest runat this head: 34 pre-existing files + the newlanding-severity-none.test.ts→ 895 tests passed with my probes removed. The head commit5d45146is comment-only and corrects an example that contradicted the code (the earlier block claimed## BLOCKER — none.negates; the dash is excluded, so it does not) — claim honesty is good: the description's dash rule and the code now agree.Concerns
BLOCKER: none, see RISK-2.,BLOCKER: none — resolved by <sha>.,BLOCKER none (verified on head X).andBLOCKER none as of <sha>.all still report one id-less finding at this head, i.e. the same failure mode GITS #209 hit, just with a tail. This is intended by the ticket (content after the determiner is content, andBLOCKER none, verified on head X.is a mandated must-block row), so it is not a blocker — but the fleet's two most natural "none, but qualified" phrasings remain traps, and a seat can still wedge a PR with one. A follow-up ticket for aresolved <ID>-free escape hatch (or a refusal message that names the offending line as unclosable) would retire the last of this family.SEVERITY_REMAINDER_GAP_REincludes.,,,;,=,>,~,{,},|, soBLOCKER... none.,BLOCKER,,, none.,BLOCKER;;; none.,BLOCKER ||| none.,BLOCKER: > none.,BLOCKER => none.all negate. No real finding was observed to read that way in the corpus, so the widening is inert in practice; it is still wider than "a colon or equals plus markup and whitespace" and a future narrowing should be pinned by a test rather than discovered.severity:as an accepted lead makes one new class negate.severity: blocker none.andSeverity: BLOCKER none.moved 1 → 0 at this head (they were findings onmain). Plausibly intended, and unreadable as a real defect, but it is outside the ticket's enumerated shapes and is not covered by any test in the new file. A one-line test pinning theseverity:lead would make the intent explicit.kv/ld/forgejo-reviewersecret'susermetadata field readspi-landingwhile the token itself authenticates asld-reviewer(id 47, verified withGET /userbefore posting). Nothing here depends on that field, but the stale metadata would mislead anyone reading the secret entry.Verdict of record — ld-reviewer — head
5d451462bf: APPROVEmodel: deepseek-flash
session: 01a1b047-f5f7-757a-b84d-280dce34e7c2
Independent review seat 1 of 2 (gate tooling). Reviewed the diff, not the author's summary:
git diff origin/main...5d45146is two files (forgejo/landing.ts+100/−2,forgejo/tests/landing-severity-none.test.tsnew, 223 lines). I did not depend on the parallel seat.Blockers
None.
Concerns
SEVERITY_TOKEN_LEAD_REaccepts a)lead but not an ordered-list numeral, so the family is asymmetric on one shape — measured at head:The miss is fail-closed (the PR stays refused, never silently allowed), so it does not hold this change — but it is the exact artifact this PR fixes, still reachable from a numbered list. Suggest a follow-up widening the lead run to
\d{1,3}[.)]; the near-miss tests are already structured to take the extra cases.The rule deliberately does not strip a quote from the end of a remainder, so a quoted zero-claim stays a finding. Correct direction (fail-closed), noted only so the asymmetry with emphasis-insensitivity (#103) is on the record.
What I verified (evidence)
1. The failing test reproduces at the parent. In a detached worktree at head I ran
git checkout origin/main -- forgejo/landing.ts(parent26b5f15) and ran the new file against it:Tests 16 failed | 27 passed (43)— the PR's number. The 16 failures are the PR's enumeration, and the guard family (BLOCKER none of the tests ran.and the other prose tails) passes at the parent, which is the strongest form of the claim: the fix adds the negation and leaves the fail-closed reading untouched, rather than trading one for the other. The end-to-end failure at parent is the GITS #209 shape:findOpenBlockersreturns['BLOCKER none.']for comment 22638's tail, andevaluateLandingLock(...).ok === trueonly at head.2. Exact-vs-content rule. Probed the rule directly, not through the author's list:
The guard line is asserted on the finding's own text and its null id, so it pins the reading, not a count.
3. Emphasis-insensitivity.
BLOCKER none.·**BLOCKER** none.·__BLOCKER__ none.·_BLOCKER_ none.·**BLOCKER:** none.·- **BLOCKER**: none.·## / ### BLOCKER none.·(BLOCKER) none.·BLOCKER:none.·BLOCKER none.**all read zero, and the dash twin reads the same however it is emphasized. Bold markup cannot change a reading.4. No interaction with real findings or closure. A body carrying a real labelled finding plus the zero claim keeps the finding and drops only the claim; closure still works and still needs the reviewer: trusted
resolved B1closes it, a stranger'sresolved B1does not, and the zero claim under a## Blockersheading does not disturb the section. The new check sits afterisNegatedand before the documentation guards, so every existing negation keeps its own reading; it is called nowhere else, and nothing in the resolution path or the section machinery reads it.5. Corpus measurement, reproduced independently. I ran every comment body in the repo's five fixture corpora (76 bodies) plus comment 22638's tail through
findOpenBlockersunder the parent and under head and diffed the finding sets: 77 bodies compared, exactly 1 changed —gits209#22638dropsBLOCKER none.— and zero additions anywhere. Total open findings on that corpus: 6 → 5. That is the property that matters: no finding anywhere became invisible. (The PR reports 79 bodies; the two extra are GITS #209 comments not in my local fixtures.)6. Suites green in both shapes at head.
npx tsc --noEmitclean.npx vitest run→ 34 files, 895 tests passed (the #96–#106 suites included).npm run test:deployed(deployed shape, perTESTING.md) →ok: 34/34 files loaded and passed, 895/895 testson the pinned tree (typebox 1.3.34). Forgejo CI on this head (#334) is green across all 15 jobs.Scope check: the only edit to pre-existing logic is factoring the determiner vocabulary into
NEG_ZERO_WORDinside two existing negation patterns — character-identical sets, so the substitution cannot change a reading (all negation suites agree). The new behaviour is reachable only from the line-initial severity-token path.Review note — not a verdict. This closes a machine artifact raised by my own verdict comment, nothing else.
The "Emphasis-insensitivity" bullet in comment 22649 is a LIST of quoted token forms; the lock reads that line as a finding. It is not a claim of a defect, and nothing in this PR is defective on its account. I am not editing or deleting that comment — that is not mine to do — so I am using the sanctioned closure sentence for the id-less finding that item raised:
The blocker raised in review 22649 is resolved on head
5d451462bf.Verdict unchanged — APPROVE at head
5d451462bf(seat 1 of 2, model deepseek-flash, session 01a1b047-f5f7-757a-b84d-280dce34e7c2).Follow-up worth a ticket, outside this PR's scope and pre-existing at the parent: a documentation line that merely LISTS quoted token forms is still read as a finding when the list is joined onto one line, which is the same "typographic artifact blocks a clean PR" family as the one this PR fixes on the token path.