Fix #20, #25, #35, #43, #44: consult and porch stop approving what nobody read

[pseudoseed]

Closes #20, closes #25, closes #35, closes #43, closes #44.

Five issues filed separately over two days. They are one class: a system that could not tell, and said something else. Fixed together because they compound — #35 feeds an empty diff to a reviewer, #44 feeds it no diff at all, and #20 counts the resulting silence as approval.

#35 — three APPROVEs on a 0-byte diff

On a Forgejo-hosted repo the forge config failed to resolve, gh ran against a host it could not reach, and the diff came back empty with nothing on stderr. The prompt said Changed Files (0). Three lanes returned APPROVE (HIGH).

buildPRQuery now refuses a 0-byte diff and names the three things it could be. There is no legitimate caller: an empty PR is not reviewable, and a genuinely empty diff is indistinguishable from a fetch that failed silently — so both stop.

#44 — the opencode lane reviewing with no diff, and no prompt

opencode auto-rejects reads outside its working directory. The consult sandbox is an mkdtemp dir the agy lane reaches through --add-dir; opencode got no equivalent grant. Observed live:

! permission requested: external_directory (/var/.../codev-consult-XXXX/*); auto-rejecting
✗ Read /var/.../codev-consult-XXXX/pr-42.diff failed

Two artifacts land in that dir. The PR diff — so an opencode PR review silently read the working tree instead of the PR's head→base changes. And, above CLI_PROMPT_INLINE_MAX_CHARS (100k), the entire prompt: the lane then held nothing but an instruction pointing at an unreadable path, and still produced output and a verdict.

That is precisely what this lane's own header says it hard-fails to prevent:

A lane that quietly emits a skip is a lane that quietly lowers the bar. Missing CLI, unknown model, non-zero exit, and empty output all throw.

Every one of those guards catches a process that failed. None catch a process that exits 0 with a confident verdict formed from nothing.

Both artifacts now go through opencode run -f, which attaches content rather than negotiating with the permission system. extractSandboxPaths reads back the paths the prompt actually names, rather than re-deriving them — a second source of truth would drift from the first.

#43 — an error naming a file that has never shipped

A bare --type pr failed with Prompt template not found: codev/consult-types/pr-review.md. That path has never existed in any release; five of the six review types live only under protocols/<name>/consult-types/. The message reads as "create this file". The fix is --protocol, and nothing said so.

It now lists the protocols that actually provide the template, reusing protocolDirs() so it sees the same four tiers the resolver does. When it cannot look, it falls back to the plain not-found error — a guess here would replace one wrong remedy with another.

#25 — the same remedy regardless of cause

The agy skip artifact ended with "install the CLI (https://…) and run agy once to sign in" whatever went wrong. For a quota wall that is two wrong instructions at once: the CLI is installed, and signing in does not refill a quota.

agyRemedy now picks from the failure — missing CLI, quota/rate limit, auth, timeout — and returns nothing for an unrecognised cause, where the caller shows agy's raw output instead. A guessed remedy costs more than no remedy, because the reader acts on it.

#20 — and the mistake I made fixing it

The blocking behaviour is deliberately unchanged. A lane that is unauthenticated or quota-exhausted must not wedge a project; that was the explicit call. What was wrong was the record.

My first attempt used findVerdict — "did the reviewer state a verdict?" — and it was wrong. An existing test caught it by printing 3 of 3 lanes reviewed and approved for a run with a skipped lane. The skip artifact is well-formed: agySkipContent writes a real VERDICT: COMMENT. A missing verdict cannot detect it.

So the lane has to declare it. The artifact now carries LANE_DID_NOT_REVIEW: true and laneReviewed() reads it — a contract between codev's own skip writers and its parser, not an inference from prose.

The gate message changes from:

All reviewers approved!
  gemini: COMMENT
  codex: APPROVE
  claude: APPROVE

to:

2 of 3 lanes actually reviewed. Did not review: gemini — recorded as non-blocking, NOT as approval.
  gemini: COMMENT (LANE DID NOT REVIEW — skipped or produced no verdict)
  codex: APPROVE
  claude: APPROVE

That first line is what a human reads immediately before approving a gate.

stated is optional on ReviewResult: reviews in existing status.yaml files predate the field, and absent provenance must not retroactively accuse them.

Tests

10 for #20, 10 for #25/#43. Two existing tests asserted the old strings and were updated with the reason recorded inline — one of them is the test that caught my wrong first fix, so it now pins the correct behavior instead.

src/commands/porch + new consult tests: 534 passed. src/agent-farm + consult + doctor: 3361 passed, 34 skipped.

One flake seen once under full-suite load: renders a realistic large (~4MB) ring WHOLE within a CI-aware budget. It passes in isolation both on a clean tree and with these changes, so it is load-sensitive rather than a regression here.

🤖 Generated with Claude Code