fix(compliance): never treat a failed fetch as evidence

[aabusair]

github_api() returns None for every failure mode — a rate-limit 403, a permissions 403, a 404, a network error — so once collapsed into a list those are indistinguishable from "there is nothing here". Four places turned that emptiness into a contributor-visible decision.

The four

Where	Read an unreadable fetch as	Consequence
post_pr_comment dedup	"I haven't commented yet"	Unbounded duplicate comments
check_comments_for_signature	"no sign-off"	failure on someone who signed
check_dco_commits	"not signed off"	failure on signed-off commits
Signature registry read	"user has not signed"	Fails everyone who has signed

The registry one is the worst: it's the primary compliance path, so one failed fetch fails every compliant contributor at once.

post_pr_comment had a second, independent bug: the dedup scan was an unpaginated fetch, returning only GitHub's default first 30 comments (verified independently against a 2495-comment issue). On a thread with 30+ comments older than ours the scan never saw our own comment and posted another. Each post bumps updated_at, keeping the PR inside the sweeper's lookback window, so it posted again every sweep — and each new comment lands later in the listing, so a 30-item window can never catch up. Unbounded, self-sustaining, on a public PR.

Worth stating plainly: naively fixing that pagination alone would have caused the very spam it fixes. A transient error would return [], read as "haven't commented", and post a duplicate. The readability signal isn't optional polish — it's a prerequisite.

The fix

github_api_paginated_checked() returns (items, ok); that signal is threaded through each caller. The plain wrapper stays, so remaining call sites are untouched. process_single_pr writes no status and posts no comment when any input is unreadable — whatever status a PR already has is a better answer than one derived from a failed read.

Also fixed (found by auditing, not by a failing test)

• get_existing_status_state had the same conflation, which quietly disabled the already-resolved short-circuit from fix(compliance): skip re-checking PRs that already have a successful status #67 during API trouble and let the sweeper repaint — and so bump updated_at on — PRs it should have left alone.
• commit.get('sha')[:7] raised TypeError when a commit payload lacked sha. The sweeper's per-PR except swallowed it, silently skipping the PR with no status.
• fetch_shared_config now reports completeness and cla_sweeper aborts the sweep when the licence catalogues can't be loaded. An empty catalogue doesn't fail one PR — it decides CLA-vs-DCO for every PR in the sweep from no data.

Completeness deliberately covers only the two licence catalogues, not the allowlist. An empty allowlist is a valid configuration meaning "no overrides", and fetch_mothership_file can't tell empty from failed — so gating on it would mean emptying cla/allowlist.yml silently aborts every sweep org-wide. Far worse than the skewed decision it would prevent.

New error_log() emits ::error:: for genuine failures. debug_log() emits ::warning:: for everything including success messages, so a real problem was indistinguishable from noise.

Test plan

• 93 tests, identical count via discover and direct run
• 100% of the executable statements this change adds are covered — measured with an AST-filtered line tracer, not estimated (policy_selector 68/68, cla_sweeper 3/3)
• Every guard mutation-proven: removing it makes a specific named test fail
• Behaviour matrix for the previously-shipped fix(compliance): require a deliberate sign-off for the required document #71 fixes still 7/7 — no regression
• Scope: only policy_selector.py, cla_sweeper.py, tests/. Both workflows, both signature registries and cla/allowlist.yml byte-identical to main
• Live workflow_dispatch A/B against main's baseline (running now)

Honest notes

Frequency. None of this is currently firing: zero fetch errors across 214 PR evaluations in 7 sweeps, full rate-limit headroom, and the longest bot-touched thread has 12 comments against the 30 needed. This is prophylactic. The case for it is blast radius per occurrence and the fact that these failures are silent — the sweeper has no alerting that would tell you they happened.

What a live test cannot cover. Every failure path here is stub-only by nature; you can't ask GitHub to 403 you on schedule. The canary can only confirm the happy path is unbroken across 84 repos — which matters, since this change altered the return arity of four functions and added many early returns.

Known gap, deliberately deferred. The sweeper still exits 0 when it aborts, so an org-wide compliance stop looks like a green run. Making a red exit the alerting channel is a follow-up, since turning a 100%-green workflow into a notifier carries its own risk (~79 runs/day).

Pre-existing, not addressed. GitHub caps /pulls/{n}/commits at 250 commits. With per_page=100 the pagination sees 100/100/50, breaks on len(items) < 100, and reports ok=True — so a PR with >250 commits can be painted success on a truncated listing. Its own PR.

Process note. An adversarial audit (7 independent lenses, 51 findings, 79 verdicts) refuted 73 and upheld 6, all at severity low. Its real value was catching two overstatements in an earlier revision of this PR: two guards claimed as mutation-proven weren't, and the test count was inflated by two classes subclassing a TestCase and re-running its tests. Both corrected here.

🤖 Generated with Claude Code

[aabusair]

Closing — not worth merging on current risk.

Every defect here needs a fetch failure to trigger, and the measured rate is zero across 214 PR evaluations in 7 consecutive sweeps, with full rate-limit headroom. The longest bot-touched thread has 12 comments against the 30 needed for the pagination bug. Nothing here is firing.

The work stands verified if it's ever wanted: 93 tests, 100% coverage of the executable statements it adds, every guard mutation-proven, and a same-moment A/B against main across 84 repos showing byte-identical outcomes and zero errors.

One thing worth recording, since it was found here and is independent of this PR. The gate's required context Check CLA/DCO can be satisfied by two things: the commit status the script posts, and the Actions job, which carries the identical name. Verified experimentally (isolated repo-level ruleset on a throwaway branch) that a green check run alone satisfies the requirement with zero commit statuses present.

Today that is near-harmless — it needs the status POST to fail silently while the rest of the run succeeds, since set_commit_status ignores its return value. But it is why this PR was not merged as-is: its six deliberate bail paths would have widened that one narrow path into seven, converting the gate from fail-closed to fail-open. The prerequisite would have been renaming the job so only a real status can satisfy the requirement — measured to affect zero of the 101 currently-passing PRs.

Not doing either for now. Recorded so the coupling is not rediscovered from scratch.