Skip to content

fix(audit): count the repositories examined, not the ones supplied - #1989

Merged
seonghobae merged 1 commit into
mainfrom
fix/codeql-audit-count-examined-not-supplied
Sep 6, 2026
Merged

seonghobae merged 1 commit into
mainfrom
fix/codeql-audit-count-examined-not-supplied

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Follow-up to #1987 (merged as bf0bf0ab). That change taught the CodeQL coverage
audit to refuse an empty payload. The same defect is still reachable one input
shape sideways
, and this is reproducible on main right now:

$ echo '[{"name":"r","archived":true,"default_setup_state":null,"latest_codeql_analysis":null}]' \
    | python3 scripts/ci/audit_org_codeql_coverage.py
PASS: all 1 repositories have real CodeQL coverage        exit 0

Non-empty, so it clears the new guard — and archived repositories are then
legitimately skipped by the coverage loop, so the subject set is empty again. The
run examined nothing and said PASS.

The fix

Count what was examined, not what was supplied.

auditable_repositories() becomes the single place that decides which
repositories are in scope, and both the coverage loop and the guard use it. The
guard now refuses an empty examined set however it became empty, and the PASS
line reports the examined count.

Sharing the predicate matters for the next case rather than this one: every
exclusion rule added to auditable_repositories() creates one more input shape
that reaches an empty subject set, and a guard with its own copy of the rule
would not follow.

How it was found

#1987 was reviewed by two sessions and each found this shape from a different
direction — one below the fix ([] reaching main), one beside it (a payload
whose rows are all filtered out downstream). My own two-way control found
neither, because it only exercised the branch I had written.

Evidence

gate      2975 passed, 1 skipped, 21 subtests   (predicted main 2973 + 2)
coverage  100%, 0 missing
interrogate 100%
control   `audited = repositories` restored → both new tests fail
restored  30 passed, dirty=0

Reproduced on the merged main tree by another session independently, by running
it rather than by reading for the symbol.

Developer experience: the PASS line's number is the number of repositories the
run actually checked.
User experience: an audit that examined nothing says so instead of passing.

🤖 Generated with Claude Code

#1987 taught the CodeQL coverage audit to refuse an empty payload, because
"PASS: all 0 repositories have real CodeQL coverage" reads as success over a
run that examined nothing. Reviewing that change, host 2 fed it a payload of a
single *archived* repository:

    PASS: all 1 repositories have real CodeQL coverage        exit 0

Non-empty, so it clears the new guard, and archived repositories are then
legitimately skipped by the coverage loop. The subject set is empty again, by a
different route -- the same defect the guard was added to close, one input shape
sideways from the one it checks.

The count that matters is what the audit examined. `auditable_repositories()`
is now the single place that decides which repositories are in scope, shared by
the loop and by the guard, so the two cannot drift apart when the archived rule
changes. The guard refuses an empty examined set however it became empty, and
the PASS line reports the examined count rather than the supplied one, so an
organization of nothing but archived repositories can no longer be reported as
fully covered.

Developer experience: the pass line's number is the number of repositories the
run actually checked.
User experience: an audit that examined nothing says so instead of passing.

Co-Authored-By: Claude Opus 5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 32 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 69316086-63d8-44b0-a0d0-c673e0915ad0

📥 Commits

Reviewing files that changed from the base of the PR and between bf0bf0a and 6e5883c.

📒 Files selected for processing (2)
  • scripts/ci/audit_org_codeql_coverage.py
  • tests/test_audit_org_codeql_coverage.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae merged commit c9052e6 into main Sep 6, 2026
5 of 15 checks passed
@seonghobae
seonghobae deleted the fix/codeql-audit-count-examined-not-supplied branch September 6, 2026 19:20
@seonghobae

Copy link
Copy Markdown
Contributor Author

Verified by running it. Scoped to head 6e5883c4 — this approval does not extend past that sha.

Every input shape that reaches an empty effective set

The rule this change implements is "count what was examined, not what was supplied", so the test is to enumerate the shapes that can empty the effective set, not just the two already known:

input exit output
[] 2 this run audited nothing (0 of 0 repositories were eligible)
archived only 2 this run audited nothing (0 of 1 repositories were eligible)
one covered 0 PASS: all 1 repositories have real CodeQL coverage
configured, empty languages 1 FAIL: 1 repositories have no CodeQL coverage
one archived + one covered 0 PASS: all **1** repositories…

The last row is the one that shows the fix is complete rather than special-cased: a two-element payload reports all 1, because the count comes from the audited set. Previously that line reported the supplied length, which is what made "success over an empty subject set" possible in the first place.

The predicate is shared, not duplicated. auditable_repositories is defined once and called from both the loop and the guard. That is what stops the two drifting apart when a second exclusion rule is added — at which point a new empty-set input shape appears automatically covered rather than needing to be found by hand, which is how this one was found.

Gate and isolated control

check result
pytest 2975 passed, 1 skipped, 21 subtests
coverage (scripts/ci) 100%, 0 missed of 13194
interrogate 100%

Predicted before running: main 2973 plus 2 tests. That is what ran.

Pointing the guard back at the supplied list, changing only that one line, fails exactly the two new tests:

test_main_refuses_a_payload_of_only_archived_repositories
test_pass_line_counts_examined_repositories_not_supplied_ones

So both halves of the change are pinned separately: the refusal and the count.

Note

The behaviour this closes is live on main right now — I ran the archived-only payload against the current tree and it returns PASS: all 1 repositories have real CodeQL coverage with exit 0.

🤖 Addressed by Claude Code

seonghobae added a commit that referenced this pull request Sep 6, 2026
…s four retractions

Today's measurements lived only in private session memory and in scattered pull
request bodies, which the standing conventions say is the wrong home: the
repository and the Project are the source of truth. This consolidates them into
the live gap baseline.

What the measurements say. Fan-out is uniform across the organization at 8-12
workflows per pull request head with no workflow running twice on a head, so the
ceiling is not fed by duplication. It limits concurrent jobs rather than
runner-minutes, which inverts the gate: within strix.yml the scan job is 20% of
the job count and 98.7% of the runner time, while the two jobs that decide
whether to skip that scan must first win a runner slot themselves. Strix
failures hold a runner for a median 74.8 minutes against 12.8 for successes, and
the 119-minute failure examined produced a complete written assessment, retained
as a 33 KB artifact, before failing closed on an exhausted free model pool. The
central CodeQL lane leaves no analysis record in any of eight sampled
repositories; six of those eight have a CodeQL-supported language that appears
in no analysis at all, and life-os has none of any language.

The detector that would have reported life-os had not run since 2026-09-04,
because an owner-configured ruleset drift exits the shared job before it. That
is fixed in #1987 and #1989; the drift itself, the #1929 dispatch actor
variable, and whether an already-generated Strix report can serve as evidence
are owner decisions and are recorded as open rather than resolved.

Four numbers this pass published and withdrew are recorded with their
mechanisms, because three of them were quoted onward by other sessions before
being caught: a duplicate count grouped without the workflow dimension, a
per-language fan-out read as duplication, a creation-rate burst produced by a
window labelled one hour that spanned 1.95, and a repository-scoped claim that
compared June samples against September ones. The shared failure is reading a
value one step removed from the fact as the fact.

The figures for the CodeQL dispatch share use a closed window rather than an
open-ended one, so they are reproducible instead of drifting with the clock --
found while re-checking this record against the same mistake it documents.

It also closes one backlog item rather than leaving it open. GitHub's notice that
App installation tokens move to a stateless format of roughly 520 characters is a
real forward-compatibility risk for code that assumes a token length; it is not
one here. A 520-character `ghs_` token demonstrably round-trips through the
redaction path, both patterns are open-ended, and two independent searches -- for
length assumptions and for length constraints -- found none. Recording that stops
the item reading as an open risk and stops the next pass repeating the work. The
behavioural line is the positive evidence; the two searches are bounded, and the
record says so, because an absence found by grep is the most truncation-vulnerable
claim available and this document already carries two retractions of that kind.

Developer experience: the ceiling investigation, its owner-gated remainders, and
its retracted figures are readable from the repository instead of having to be
reconstructed from pull request comments.

Co-Authored-By: Claude Opus 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant