fix(surveyor): count unresolved review threads with a fail-closed helper

[devantler]

Why

The surveyor counted a pull request's unresolved review threads by hand, so a failed or incomplete read looked exactly like "no open threads". That count decides whether a PR may be promoted and merged, and it once reported zero while a major finding was still open.

What

The plugin now ships a small tested helper that reports the thread count only when it has read every thread, and says "unknown" otherwise. The surveyor must use it and treat "unknown" as incomplete, never as zero; the read-only guard allows exactly that helper, run on its own. Consumers that install runtime files individually need to add the new helper.

Fixes #231

🤖 Generated with Claude Code

https://claude.ai/code/session_011Aut24ni8XbKPYyY5mYrbb

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Evidence at head 480c6187df7c8dae005ac057b063e569f2c52b03

RED (before the fix, same tests against main's guard and surveyor):

• forge-readonly-guard.test.sh: 394 passed, 11 failed — the bundled counter was refused with exactly the denial the issue quotes (deny: a read must begin with a forge command, not '…/count-unresolved-review-threads.sh'), and none of the counter's shape denials existed.
• surveyor-review-contract.test.sh --check <main's portfolio-surveyor.agent.md>: missing contract: H01, H13, H14, H15, H16, H17 — main still told the surveyor to count threads inline.

GREEN (this branch):

• count-unresolved-review-threads.test.sh: 26 passed, 0 failed — zero, one unresolved, resolved-only, 103 threads over two pages, a later-page failure, an outright failure, an empty read, a missing PR, a node without isResolved, a non-numeric total and totals that change between pages; each non-complete case is UNKNOWN … with exit 2. Ablations: dropping --paginate is caught as UNKNOWN truncated fetched=100 total=103; dropping --paginate and the truncation check reads unresolved=0 total=103 (the original failure); dropping the per-node check reads a malformed node as resolved. 11 usage shapes refused as UNKNOWN usage; a missing gh is UNKNOWN tool-unavailable.
• forge-readonly-guard.test.sh: 405 passed, 0 failed (counter admitted only as the exact sibling in flag form, denied when piped either way, denied for positional/--input/host-qualified repo/zero or non-numeric PR/repeated flag/foreign path; a consumer's own counter script is still refused).
• surveyor-review-contract.test.sh: PASS (72 clauses, each removal rejected); portfolio-surveyor-agent.test.sh: PASS (the prescribed counter form is guard-admitted, the piped form is denied).
• classify-default-branch-ci-runs.test.sh: 27 passed (runtime-asset pin set now includes the counter).

Validation run locally (mirrors CI): validate-manifests.sh, refresh-desired-state-digests.sh --check, check-plugin-version-bump.sh (5.2.0 → 5.2.1), guard-gh-json-fields.sh, every scripts/*.test.sh, every bundled plugins/*/scripts/*.test.sh, and ShellCheck 0.11.0 over all 31 scripts — all exit 0. No skill changed, so the per-skill agentskills.io spec validation was not re-run here; no workflow changed, so actionlint was not needed.

Behaviour exercised: through the real hook adapter and real guard (only the forge response stubbed), a bare count-unresolved-review-threads.sh probe was denied with a classifier-path-json: record naming the relocated install, the decoded and quoted path was admitted, and executing it reported unresolved=1 total=2 with status 1; the same command piped into cat was denied; a missing or non-executable helper gave no path hint. A live read against GitHub was not run from this environment (no gh CLI available here), so the first real survey after this merges is the live observation.

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Lint fix at head 8482baa22267f653387f510af896f85b9648b991

Lint scripts failed at af191ec on SC2015 (A && B || C) in the counter's two repeated-flag guards. My local ShellCheck 0.11.0 did not report it; CI's older ShellCheck does. Reproduced with ShellCheck 0.9.0: the previous counter reports both SC2015 findings (RED); after rewriting them as explicit if statements, ShellCheck 0.9.0 and 0.11.0 are both clean over all 31 scripts (GREEN). Behaviour is unchanged: count-unresolved-review-threads.test.sh 26/0, classify-default-branch-ci-runs.test.sh 27/0 (runtime-asset pin recomputed), surveyor-forge-readonly.test.sh 51/0, validate-manifests.sh, refresh-desired-state-digests.sh --check and the version-bump gate all exit 0. The desired-state digest for the counter was regenerated with the repository's refresh script, not hand-edited.

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer

@coderabbitai review

---

Generated by Claude Code

[coderabbitai]

Rate Limit Exceeded

@devantler have exceeded the limit for the number of chat messages per hour. Please wait 7 minutes and 5 seconds before sending another message.

[devantler]

🤖 Generated by the Agentic Engineer

Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)

Reviewed commit: 85105b3aa691ac87203a66e04495701eea7d5692

• CodeRabbit: requested at this head (comment 5784495227, 21:30Z); replied "Rate Limit Exceeded" at 2026-09-22T21:31:08Z (comment 5784496747) — no review ran at this head
• Codex: usage limit reached (measured 2026-09-22T20:53:06Z on monorepo#3507; maintainer-only, not re-requested)
• Cursor Bugbot: usage limit reached (measured 2026-09-22T20:53:35Z on monorepo#3507; maintainer-only, not re-requested)

What I checked (independent reviewer, not the author):

• Guard allow-rule is exact. The counter is admitted only as the guard's own pwd -P sibling, matched as a quoted (literal) case pattern, at pipeline position 0, with exactly --repo OWNER/REPO and a positive --pr — and only when the whole command is one segment. Probed 24 shapes against the guard at this head: | cat, | jq, a trailing |, || true, && true, >/tmp/x, >&-, --repo=…, --, $PR/${PR}/$R operands, a relative or ../-normalisable path, a bare basename, bash <counter>, and the counter consuming a gh pipeline all deny; only the flag form (optionally with 2>&1, >/dev/null, >&2) allows. --repo ../.. is admitted but is passed as GraphQL variables, never a path. A consumer-declared classifier path equal to the counter still hits the counter branch first, so it cannot be admitted mid-pipeline.
• Fail-closed counter. Every non-complete outcome prints UNKNOWN <reason> and exits 2: usage, missing gh/jq, gh non-zero (including a later-page failure after page 1 printed), empty output, missing/null reviewThreads, a node without a boolean isResolved, a non-integer or negative totalCount, totals that change between pages, and fetched != totalCount. The only zero is a complete parsed read. owner/name/query go through -f (raw, no @file expansion); --pr is validated numeric before -F. The query is a fixed read; GH_TELEMETRY=0 is exported.
• Tests prove it (RED/GREEN + ablations). Independently confirmed RED: main's guard denies the counter (a read must begin with a forge command). My own ablation removing the guard's single-segment check makes exactly the two pipeline tests fail (403/2). The counter suite's ablations (no --paginate, no truncation check, no per-node check) each flip to the dangerous result.
• Surveyor contract. Field (b) forbids inline counting, prescribes the helper by resolved path, maps anything but status 0/1 with the exact line to unresolved=unknown, and routes unknown into the existing fail-closed digest rules (never REVIEW-READY/MERGE-READY). Fixed in 85105b3: the non-negotiable Safety section still called the classifier "the one bundled compound forge read", contradicting field (b) (P1); plus two wording nits (step 4 "recognises only this sibling"; README "nothing else local runs under the guard" ignored consumer-declared classifiers).
• Release hygiene. Version 5.2.0 → 5.2.1 moved in all four places; changelog entry present; desired-state digests (counter, guard, surveyor definition) regenerated with refresh-desired-state-digests.sh and --check passes. No synced skill tree touched (bundled-skill guard: no bundled tree touched). No Python, no workflow change.
• Local validation at 8482baa (full) and 85105b3 (affected suites): ShellCheck 0.11.0 over all 31 scripts, validate-manifests.sh, version-bump gate, digest --check, guard-gh-json-fields.sh, every scripts/*.test.sh and every plugins/*/scripts/*.test.sh — all exit 0 (guard 405/0, counter 26/0, adapter 51/0, review contract 72 clauses).

Verdict: no P0/P1 findings

---

Generated by Claude Code

[devantler]

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

User evaluation at head 85105b3aa691ac87203a66e04495701eea7d5692

85105b3 changes only the surveyor definition text, the README and the regenerated desired-state digest; the guard, counter and adapter scripts are byte-identical to 8482baa. Exercised at this head:

• As the surveyor would run it: the guard admits <installed>/count-unresolved-review-threads.sh --repo devantler-tech/monorepo --pr 2436 (allow). Executing that exact command against a stubbed forge response (only gh stubbed): all-resolved → status 0, unresolved=0 total=2; one open thread → status 1, unresolved=1 total=3; a read that failed after its first page → status 2, UNKNOWN read-failed — the failure the issue describes no longer reads as zero.
• Through the real hook adapter (surveyor-forge-readonly.test.sh, 51/0): bare-name discovery returns the relocated install path, the decoded path is admitted and reports the open thread through status 1, and the piped form is denied.
• RED independently reproduced: main's guard refuses the same counter command (a read must begin with a forge command), matching the issue's quoted denial.
• CI at this head: 41 check runs, 39 success, 2 skipped (bot-only auto-merge jobs); Lint scripts, which runs ShellCheck and every bundled plugin self-test, passed — https://github.com/devantler-tech/agent-plugins/actions/runs/35785973331/job/106942756256

A live read against GitHub itself is not exercisable from this environment (no authenticated gh); the first survey after release is the live observation.

---

Generated by Claude Code