porch counts a review lane that never ran as an approval: VERDICT SKIPPED parses to COMMENT, and COMMENT counts as approve

[pseudoseed]

porch prints All reviewers approved! for a project where one reviewer never ran. The summary a human reads when deciding to merge cannot distinguish a lane that approved from a lane that read nothing.

Mechanism

packages/codev/src/commands/porch/verdict.ts

// No valid VERDICT: line found but the consult ran — treat as COMMENT (non-blocking skip)
return "COMMENT";

parseVerdict recognises only APPROVE, REQUEST_CHANGES, COMMENT. Anything else falls through to COMMENT.

// COMMENT counts as approve (non-blocking feedback).
return reviews.every(r => r.verdict === "APPROVE" || r.verdict === "COMMENT");

So VERDICT: SKIPPED — the value our NOT-RUN convention deliberately writes — becomes COMMENT, and COMMENT becomes approval.

Note the default comment says "but the consult ran". That assumption is the bug: it was written for a reviewer that ran and produced unparseable output. A reviewer that never ran cannot be expressed in this vocabulary at all.

Second path to the same place

if (reviews.length === 0) return true; // No verification = auto-approve

Zero reviewers also approves. Two independent routes from "no review happened" to "approved".

Blast radius

Every project today that recorded a codex skip. Codex has been quota-exhausted since before 08:00 UTC and reset is 2026-08-27, so #2, #4, #11 and #12 all wrote a VERDICT: SKIPPED NOT-RUN file, and porch counted each as an approval. All four merged.

The merges themselves were sound: I approved each gate knowing the coverage was two of three, and each PR body states it explicitly. But that was the human channel working despite the tool, not because of it. Anyone reading porch summary got All reviewers approved!.

The NOT-RUN convention did its job. The parser undid it.

Fix

1. Recognise SKIPPED as a first-class verdict, distinct from COMMENT.
2. allApprove must NOT count SKIPPED. A skip is absence of evidence.
3. Change the fall-through. An unrecognised verdict on a lane that produced output is COMMENT; a lane that produced no output, errored, or was never invoked is SKIPPED. Those are different facts.
4. Reconsider reviews.length === 0 -> true. Auto-approving zero reviews is defensible for protocols with no consultation step, but it should be an explicit "consultation not configured", not a silent unanimous pass.
5. The phase summary should state coverage: 2 of 3 lanes ran rather than All reviewers approved!.

Pattern

Sixth arrival today at the rule now in lessons-critical.md: a truncated or absent result must not be emitable as a complete one. Others: the render gate returning CLEAN on an unproven screen (#4), log extraction returning arbitrary lines as a diagnosis (#13), recently-merged timing out into an empty panel (#17), a timed-out check reporting as a test failure (#8), and this.

This one has the largest blast radius because it is the line a human reads before merging.

Found by the #12 builder during its own review phase, and it flagged the finding against its own gate rather than staying quiet.

[pseudoseed]

Correction to the fix, item 2 was wrong

I wrote "allApprove must NOT count SKIPPED". That would make a skipped lane BLOCK the gate, and that is not the behaviour we want. Codex has been quota-exhausted since before 08:00 UTC with reset on 2026-08-27, so blocking on a skip would stop all work in this repo for six days for no safety gain.

The repo owner pushed back on exactly this, correctly.

Blocking and approving are separate questions and I conflated them.

• Should a skipped lane block the gate? No. Non-blocking is right, and is the current behaviour.
• Should a skipped lane be reported as an approval? No. That is the bug.

The defect is not that the gate opened. It is that porch printed All reviewers approved! when one reviewer read nothing, to the human deciding whether to merge.

Revised fix

1. Recognise SKIPPED as a first-class verdict, distinct from COMMENT. Unchanged.
2. allApprove still returns true with a SKIPPED lane. A skip does not block. What changes is that it stops being called an approval.
3. The summary must state coverage rather than unanimity: 2 of 3 lanes ran (codex: SKIPPED) instead of All reviewers approved!. Same gate outcome, accurate sentence.
4. Fall-through unchanged from the original: a lane that produced output but no parseable verdict is COMMENT; a lane that never ran is SKIPPED.
5. reviews.length === 0 -> true unchanged from the original: keep auto-approving, but say "consultation not configured" rather than implying unanimous approval.

The through-line is the same as the rest of today: the system may proceed, it just must not misdescribe what happened while doing it. Non-blocking was never the problem. Claiming an approval that nobody gave was.

[pseudoseed]

A third route, and it shows the guard was always measuring the wrong thing

parseVerdict has a length floor before any parsing:

// Empty or very short output = something went wrong
if (!output || output.trim().length < 50) {
  return "REQUEST_CHANGES";
}

Under 50 characters gets the safe answer. Over 50 with no VERDICT: line falls through to COMMENT, which allApprove counts as approval.

So any lane that prefixes output defeats the safe default. Found live on 2026-08-21 by the new opencode lane reviewing its own PR (#24): a 76-character provenance header pushed short verdict-less output over the floor. Verified directly:

parseVerdict("ok")            -> REQUEST_CHANGES
parseVerdict(header + "ok")   -> COMMENT      (counts as approval)

Why this is the important part

The floor is a proxy. Length stands in for "a review actually happened". It was never measuring that, and it held by luck. A banner, a model id, a timestamp, or a provenance line defeats it — none of which are unusual things for a lane to emit.

So do not fix this by raising 50 to 100. That reintroduces it with the next slightly longer prefix.

What the fix needs

Separate the two questions the parser currently conflates:

• Did the reviewer emit a verdict? A VERDICT: line, present or absent. Nothing to do with length.
• Which verdict was it? Only meaningful once the first answers yes.

#24 is extracting findVerdict() to make exactly that distinction, so the caller can tell "reviewer said COMMENT" from "reviewer said nothing". Whoever picks up this issue should reuse that function rather than write a second one.

Running count of routes from "no review" to "approved"

1. VERDICT: SKIPPED parses to COMMENT (original report)
2. reviews.length === 0 returns true
3. output over the length floor with no verdict line parses to COMMENT

All three end in the same place, and #24 found the third by reviewing itself.