feat(ci): surface the advisory quality signals in the PR review UI, for free - #18
Merged
Merged
Conversation
…ore they can be staged The advisory quality jobs produce coverage.xml, a jscpd report dir, a mutmut cache and (with the C901 delta landing next) two ruff JSON files -- none of which were ignored. The ruff JSON is the one that matters. Its `filename` values are ABSOLUTE local paths, so anyone reproducing the delta locally and running `git add -A` stages a home directory path, which is precisely what the leak scanner's home-path detector fires on. Ignoring these first removes the window entirely rather than relying on remembering not to stage them.
…he 122-finding backlog Raw C901 cannot be surfaced on a diff, and measuring it is what settles the question: all 122 findings on this tree are SINGLE-LINE regions anchored on the function-name token of the `def`. GitHub renders a finding inline only when its lines are in the diff, so the raw signal is wrong in both directions -- a PR adding branching inside an existing function moves no anchored line and produces nothing, while a PR reflowing a signature fires an annotation for debt it did not cause. So compare two ruff runs instead and report only what changed: functions this PR introduced over the threshold, or whose complexity it increased. On this tree that turns 122 findings into 0-3. Notes on the implementation: * The complexity number exists ONLY inside ruff's human-readable message, so parsing it is load-bearing. An unrecognised message raises rather than reporting zero findings -- a silent zero is indistinguishable from a clean PR and would leave the report permanently, invisibly green. * Keys are (repo-relative path, function name); ruff emits ABSOLUTE paths, and the "before" run comes from a second working tree at the merge base, so its paths carry an extra directory. If that leaked into the key nothing would ever match and all 122 functions would report as new -- the exact flood this exists to prevent. Verified end to end against two trees rooted in completely different directories, and regression-tested. * Collisions (two same-named methods in one file) take the max complexity, so the delta can under-report an increase but never invent one. Measured today: 122 findings, 122 unique keys.
…ith no write scope
The four advisory quality jobs computed real signals and then printed them to a log nobody opens.
Wire each one to a reviewer, using GitHub workflow-command annotations rather than code scanning.
Why annotations and not SARIF -- measured, and recorded in the workflow header so a future session
does not "helpfully" add an upload back:
1. All 122 C901 findings anchor on a single `def` line, so body edits surface nothing and
signature edits surface pre-existing debt.
2. jscpd emits ONE location per clone pair, chosen by scan order, so ~8 files can never be the
anchor and about half of newly-introduced clones would anchor on the untouched twin.
3. Uploading on pull_request only never builds a default-branch baseline, so every PR forever
would report all ~161 findings as new -- not a one-off first-PR blip.
Workflow commands need no token and no permission grant and behave identically on fork PRs, so
every job keeps `contents: read` and the workflow grants NO write scope anywhere. That, rather than
mere absence from branch protection, is what makes it structurally unable to gate a merge.
Per signal:
* diff-coverage -> INLINE `::notice` annotations on the Files changed tab. Verified against a real
run: adjacent uncovered lines coalesce into ranges (lines 2,3,4 become one annotation), paths are
relative, the console report still prints, exit 0.
* complexity -> the merge-base delta, PR-gated (there is no base ref on the cron).
* clones -> markdown step summary; annotating it would be a coin flip (see 2 above).
* mutation -> step summary + artifact. The job never runs on PRs, so it has no PR surface;
saying so plainly beats inventing one.
Also fixed while here, each a silent failure:
* `.mutmut-cache` is a DOTFILE and upload-artifact skips hidden files by default -- the upload
would have logged "No files were found", uploaded nothing, and reported success.
* diff-cover and pytest-cov were installed under a `--constraint constraints.lock` that contains
neither, so both floated to whatever PyPI served that day. diff-cover is now pinned exactly,
because the annotation surface depends on this version's `--format` semantics.
* the job pinned ruff 0.15.19 while all four locks pin 0.15.22. That drift is now load-bearing
(the delta parses ruff's message text), so it is aligned and asserted against the lock.
Every summary write is truncated: $GITHUB_STEP_SUMMARY is capped at 1 MiB per step and an oversized
write is dropped ENTIRELY, which would silently lose the whole surface.
tests/test_quality_advisory_invariants.py pins all of it -- no write scope, no SARIF, no
pull_request_target, no expression interpolation into a run body, SHA-pinned actions, the tool
pins, and every analysis step non-failing. Each assertion was negative-probed against a mutated
copy of the workflow; all 17 mutations were caught.
…reaches a reviewer
docs/CI.md's workflow table omitted quality-advisory.yml entirely. Add it, plus a table of where
each of the four signals shows up, and record the never-require rule with the real guarantee: the
workflow holds no write permission on any job, so it cannot gate a merge even if branch protection
or a ruleset changes. That is stronger than "branch protection is untouched" and is test-pinned.
Also update the mutation/coverage handoff doc: the jobs now emit annotations and summaries, and
mutation deliberately has no PR surface. Its section 5 ("flip the rubric") is marked UNACTIONABLE
-- docs/Code_Quality_Standards.md is cited by the workflow header and pyproject.toml but has never
existed in this repo's history, so there is no rubric file to flip. Flagged rather than papered
over, and no new reference to it is added.
Left open pending a measurement: whether mutation should run on PRs. Its wall-clock is unknown, so
the honest next step is one workflow_dispatch run and a look at the duration, not a guess.
… silently Steps run under `bash -e`, so an unresolvable merge base aborted the step with no explanation -- and `continue-on-error: true` then swallowed it, leaving a green job with no delta and no reason. Nobody watches an advisory job for the ABSENCE of output, so that is a signal that could stop working indefinitely without anyone noticing. Resolve the merge base explicitly and emit a `::notice` naming the base ref when it cannot be found. Still never fails the job; it just stops being silent about why it did nothing.
…tation wiring A multi-lens review of the previous four commits produced 21 candidate defects; 14 survived independent re-verification. These are the ones that mattered, each reproduced before fixing. 1. THE HEADLINE SURFACE WAS DEFEATED BY A SHALLOW FETCH. The coverage job did `git fetch --depth=1` against the COMPLETE clone that `fetch-depth: 0` had just produced. That writes .git/shallow and grafts away everything behind the base tip, and diff-cover diffs with the three-dot range `origin/<base>...HEAD`, which needs a merge base. Once the base branch advances past the PR's merge ref -- another PR merging, or a re-run of a stale check -- the diff dies with "no merge base". Reproduced in a scratch repo modelling the PR ref. It then failed in the worst available way: diff-cover's markdown reporter runs BEFORE the annotations reporter and truncates its output file on open, so `|| true` swallowed the error, zero annotations were emitted, and the summary showed a bare heading that reads as "nothing uncovered". The `-f` guard passed on the 0-byte file. Now: no `--depth` (the fetch was redundant anyway), `-s` instead of `-f`, and a `::notice` when there is no report or no coverage.xml. 2. THE RUFF PIN TEST COUPLED A REQUIRED CHECK TO THIS WORKFLOW. Asserting the hardcoded pin equals constraints.lock meant a routine Dependabot ruff bump would red `test (ubuntu-latest, py3.14)` -- a BLOCKING context -- over a purely advisory concern. That is a worse failure than the drift it was guarding. The version is now read from the lock at run time, which removes the drift and the coupling together, and the test asserts the derivation instead of the value. 3. THE MUTATION SIGNAL IS DEAD AND WAS REPORTING SUCCESS. Verified against the real scheduled run 30248096425: mutmut 2.5.1 crashes in its pony-ORM cache (`cannot pickle 'itertools.count'`, cache.py:369) after the baseline and BEFORE generating a single mutant. The job went green in 37 seconds because of `|| true`. The previous commit made this worse by adding a summary block that would have presented a traceback as results. Now the exit status is captured, a `::warning` is emitted, and both the summary and the docs say plainly that the signal produces nothing on Python 3.14. Repairing mutmut is separate, unstarted work. Also, in the delta script: * a path outside the scan root discarded a successful repo-root strip and returned the raw absolute path, so the base tree and HEAD keyed differently and every such finding would flood as new; * a file move or function rename reported a whole file's worth of pre-existing complexity as PR-caused (13 annotations reproduced from one move). Vanished-and-reappeared findings with an identical (function, complexity, threshold) are now treated as moved. A move that also changes complexity is still reported; * a PR that TIGHTENS ruff's max-complexity would have flooded, since the threshold was carried but never compared. Mismatched thresholds now report "not comparable" instead of a fake delta; * `--max-annotations` accepted negatives, which sliced from the end of the list. And a claim of mine that was simply wrong, corrected in the workflow header, docs/CI.md and the test docstrings: "the workflow holds no write permission, so it cannot gate a merge" is a non-sequitur. Permissions and branch protection are unrelated mechanisms. What keeps these jobs advisory is that their contexts are not required and every analysis step is non-failing. The no-write-scope assertion stays on its own merits -- least privilege for jobs that run third-party code fetched at run time. docs/CI.md also claimed the jobs create "no new check context"; they do create contexts, just not required ones. And complexity annotations were described as landing inline when most of them do not: they anchor on a `def` line the PR often did not touch, so they appear in the Checks tab and the summary, which is now stated in both places.
…ap, so it runs on PRs Signal 7 had been scored Built since 2026-07-14 while producing nothing. `mutmut<3` resolved to 2.5.1, which crashes on Python 3.14 in its pony-ORM cache (`cannot pickle 'itertools.count'`, cache.py:369) after printing its banner and before generating a single mutant; `|| true` turned that into a green 37-second job. The highest-leverage gate in the rubric was measuring nothing, and nothing noticed because an advisory job that reports success looks exactly like one that passed. Repaired on mutmut==3.6.0, verified on Linux/Python 3.14 in a container rather than reasoned about: 461 mutants, 87 killed, 19 survived, 355 not covered by the scoped test file. Three things in the mutmut 3 config are load-bearing, and each was found by a run that silently produced nothing rather than by reading the docs: * `source_paths` must be the PACKAGE, not the single file being mutated. mutmut 3 copies source_paths into `mutants/` and runs pytest there; with one file copied, tests/conftest.py died on `ModuleNotFoundError: messagefoundry.config` and all 461 mutants came back "not checked" -- a green run, a full results file, and zero information. `only_mutate` supplies the bounded scope instead, so the whole package is copied but one module is mutated. * `pytest-timeout` is REQUIRED. mutmut 3 always passes `--timeout`/`--timeout-method` to pytest; without the plugin every invocation dies with an unrecognised argument, surfacing only as an opaque BadTestExecutionCommandsException. * `paths_to_mutate` and `runner=` are mutmut 2 keys -- deprecated and ignored respectively. MUTATION NOW RUNS ON PULL REQUESTS. The "expensive, never per-PR" note this job carried was a property of mutmut 2 running the suite once per mutant, not of this scope: mutmut 3 runs only the tests that cover each mutant, so the mutating costs ~3 seconds and the job's real cost is its install step, same as its siblings. Survivors are most useful in review, where someone is already looking at the test that failed to kill them. The step summary carries a killed/survived/not-covered table plus the survivor list; a non-zero `mutmut run` emits a ::warning rather than passing quietly. The artifact now uploads the results and run log alongside the cache, since the cache alone is useless to a human.
…laims measurement refuted This rubric is cited by quality-advisory.yml's header and by pyproject.toml:246, but it has never existed anywhere in this repository's git history -- both citations have always dangled. Restored from the maintained copy so they resolve. Restored faithfully at v0.9 content, then bumped to v0.10 with the corrections this cycle produced. The document's own convention is to restatus rows and record why in the version history, so that is what v0.10 does rather than silently rewriting the owner's text: * SIGNAL 7 WAS SCORED "BUILT" IN v0.8 AND v0.9 WHILE PRODUCING NOTHING. The mutation gate ran `mutmut<3` -> 2.5.1, which crashes on Python 3.14 before generating a mutant; `|| true` made it report success in 37 seconds. Now genuinely measured on mutmut 3: 461 mutants, 87 killed, 19 survived, 3 seconds. That also refutes the "Expensive / never per-PR" cost model in section 5. * SIGNAL 11's "85 functions exceed C901>10" is now 122 across 43 files, and the raw list turned out to be unusable as a diff signal -- every finding anchors on a single `def` line -- so a merge-base delta was added that reports only what a PR caused. * SIGNAL 8 now emits inline PR annotations rather than console-only output. The A- verdict stands. Worth stating plainly though: the signal-7 failure is precisely the mode this rubric exists to catch -- an advisory gate reporting success while measuring nothing -- and it was caught by re-verification, not by the gate. Section 4.1's anti-metric rule protects against trusting a number too much; nothing in the rubric protected against trusting a green check that never ran. Note for the public repo: this document links to Secure_Development_Standards.md, which is deliberately gitignored (owner decision -- security-posture docs stay private), so those particular links dangle. Its sibling Secure_AI_Development_Standards.md is tracked and resolves. The leak gate passes on the restored file.
wshallwshall
enabled auto-merge (squash)
July 27, 2026 21:53
wshallwshall
added a commit
that referenced
this pull request
Jul 28, 2026
…e mutmut never prints (#19) The mutation summary table shipped in #18 reports "Killed 0" on a perfectly healthy run. `mutmut results` lists ONLY the mutants worth looking at -- survived, no tests, timeout, suspicious -- and never lists killed ones, so `grep -c ': killed'` can only ever return 0. Caught on the very first real run of the job (30308667584), which printed `killed=0 survived=19 no-tests=355` for a run mutmut's own counter scored at 87 killed. The 87 in the docs was read off that counter and is correct; only the table's arithmetic was wrong. Derive it instead: the run's final progress line carries the total, and every non-killed mutant is exactly one line of the results file, so killed = total - listed. Validated against that run's own uploaded artifact -- 461 total, 374 listed, 87 killed, matching mutmut exactly. If the total cannot be read the table prints "?" rather than a fabricated 0, because a wrong number here is worse than an obviously missing one -- "Killed 0" reads as a catastrophic test suite, and that is precisely the misreading this job existed to avoid. Pinned by a test asserting the grep is gone and the derivation is present.
wshallwshall
added a commit
that referenced
this pull request
Jul 29, 2026
…DR 0034 triage register (#37) * test(cert-cli): assert the exact DN instead of a hostname substring CodeQL py/incomplete-url-substring-sanitization (alerts 119/120/121) flagged three `"<host>" in <str>` assertions in the cert-inventory tests. There is no URL and no sanitization here — `cert inventory` is a read-only report and nothing in the engine makes a trust decision from the rendered subject/issuer — so it is not the vulnerability the query models. The assertions were genuinely weak, though: * `"good.example.org" in g["subject"]` also passes for a lookalike CN, and the test's own SAN list contains `www.good.example.org`; `read_cert_facts` returns `cert.subject.rfc4514_string()`, so the exact expected value is `CN=good.example.org`. * `"human.example.org" in printed` is satisfied by the SAN line alone, so it would keep passing if the human renderer stopped emitting the subject line at all. Tightened to an exact DN comparison and to a subject-line-specific check. No coverage is removed; both tests now fail on a defect they previously accepted. * fix(api): kill the quadratic Content-Disposition scan in the multipart parser CodeQL alert 125 (py/polynomial-redos, high). `(\w+)="([^"]*)"` restarts at every offset inside a word run and walks the rest of that run before failing, so a Content-Disposition line of n word characters costs O(n^2). The header block is attacker-supplied, bounded only by [store].max_upload_bytes (25 MiB default), and parse_single_file_upload runs synchronously on the asyncio event loop -- so one POST /uploads could wedge the entire engine. Measured before: 2k->10ms, 4k->39ms, 8k->156ms, 16k->613ms, 32k->2895ms (clean quadratic); extrapolated to the 25 MiB cap, ~22 days of blocked event loop. After: the same 25 MiB hostile body parses in 353 ms. The fix is a leading (?<!\w) lookbehind, which is O(1) and rejects every offset inside a run immediately, leaving one \w+ walk per run. It removes no match -- `=` is not a word character, so \w+ starting inside a run can only succeed at that run's end, meaning an interior offset matches iff the run's first offset does, and the leftmost scan always reaches the first offset earlier. Pinned by a differential test against the pre-guard pattern plus a growth-ratio test. Refs ADR 0034. * fix(ide): resolve each scanned module once, by descriptor, not twice by path CodeQL js/file-system-race (alert 111). buildSymbolIndex size-checked with statSync(path) and then read with readFileSync(path) — two independent path resolutions with a window between them, so the file that was READ need not be the file that was CHECKED. Exploitability is low (same-privilege, same extension host, over a config dir the extension itself enumerated, and the checked property is a resource guard rather than an authorization decision — an attacker who can swap the file can just write a large .py directly). The reason to fix is non-adversarial correctness: in a live workspace a save, a formatter or a codegen step rewrites a module between the two calls routinely, so the maxBytes guard was unsound as written. The read now goes through readCapped(), which opens once and does fstatSync + readFileSync on that descriptor; a finally closes it on every exit, including the oversize skip (readFileSync does not close a descriptor it is handed). Behaviour is otherwise unchanged: still never throws, still skips an unreadable or oversized file. Pinned by two tests: the size cap still holds, and fs spies assert readFileSync is only ever handed a NUMBER (never a path), that no path-based statSync runs, and that every opened descriptor is closed. Refs ADR 0034. * fix(ci): drop the unpinned pip bootstrap from the SBOM scratch venv Scorecard PinnedDependenciesID (alerts 115 and 118): release.yml:177 and security.yml:140 each ran `/tmp/sbomenv/bin/pip install --upgrade pip` — an unpinned, unverified PyPI install — immediately before the only thing that venv ever installs, a fully ==-pinned, hash-verified lock. --require-hashes performs no dependency resolution at all, so the pip ensurepip provisions is sufficient and the upgrade bought nothing. In release.yml it ran inside the job holding contents/id-token/attestations: write. Deleting the command both removes an unpinned install from the release path and closes the alert, so these two are the only PinnedDependencies findings in the group that resolve without a CI-tool lock (the rest stay open, blocked on DEP-1). Deliberately NOT replaced with `python -m venv --upgrade-deps`, which performs the same unpinned fetch while hiding it from the scanner — ADR 0034 option 3, rejected in favour of a visible dismissal over an invisible filter. The new test fails on either regression, and both halves were mutation-checked by reintroducing the deleted line and the --upgrade-deps variant. Refs ADR 0034. * test(api): make the ReDoS growth-ratio check noise-proof The linear-time guard on the Content-Disposition regex (alert 125) compares a 20k-char scan against an 80k one and fails above 8x. Both samples were single shots of ~0.2ms/0.9ms, so one scheduling slice on a loaded CI runner could inflate the large sample past the threshold and red the build for a timing hiccup rather than a real regression. Take the best of three per size instead: a hiccup can only inflate a sample, never deflate one, so the minimum is the noise-free estimate. Measured here at 219us / 875us (ratio 4.0) with the guard, and 1.0s / 15.7s (ratio 15.8 — the gate fires) against the pre-guard pattern, so the check still detects the very regression it exists for. * fix(ci): the DEP-1 lock-check venv had the same unpinned pip bootstrap The previous pass deleted `<venv>/bin/pip install --upgrade pip` from the two SBOM scratch venvs but missed the third instance of the identical construct, 63 lines above one of them: security.yml's `/tmp/lockcheck`, whose only install is `--require-hashes -r requirements.lock`. Every word of that pass's rationale applies verbatim — --require-hashes rejects any un-hashed requirement and so performs no resolution at all, making the pip ensurepip provisions sufficient — so the bootstrap bought nothing while adding an unpinned, unverified PyPI fetch to the DEP-1 gate itself. It is code-scanning alert #71, currently dismissed "won't fix" with the reason "CI uses editable installs (pip install -e .[extras]) for testing, which cannot use --require-hashes." That is factually wrong for this line: nothing here is an editable install, and the very next line IS a --require-hashes install. ADR 0034 requires a recorded reason; a wrong one is worse than an open finding, so #71 must be closed as fixed rather than renewed. Both earlier edits are also made LINE-NEUTRAL. This scanner re-raises the same expression at a new line as a NEW alert number — dismissed #18 (__main__.py:976) re-fired as open 122 (:1535), dismissed #39 (dependabot-auto-merge.yml:28) as open 87 (:44), both pure line drift. The 11 added comment lines would have shifted dismissed #35/#69 in release.yml and #74/#75/#76 in security.yml onto new lines, re-opening ~5 findings to close 2. Each rationale block is now one line; the argument lives in the test module's docstring, where it cannot move an anchor. The guard is generalized accordingly and renamed: it covers all three lock-only scratch venvs, and matches `<venv>/bin/python -m pip install` as well as `<venv>/bin/pip install` — the former is the spelling used elsewhere in these same workflows and the old guard was blind to it. Verified by mutation: reinstating the bootstrap in either spelling, and hiding it behind `venv --upgrade-deps`, each turn the guard red. * fix(api): bound a multipart part's header block before parsing it The `(?<!\w)` lookbehind made the Content-Disposition scan linear, but left its INPUT sized by the attacker. `max_file_bytes` caps a part's content, and only after its header has already been parsed, so the header block's real bound was the request body cap — `[store].max_upload_bytes`, 25 MiB by default and 512 MiB at the ceiling, raised for the two upload paths by the body middleware. Linear is not free at that size: `parse_single_file_upload` runs synchronously on the asyncio event loop that also drives every listener, router worker, transform worker and delivery worker, so one request blocks the whole engine for ~0.35 s at the default and ~7 s at the ceiling. A stalled loop stops ACKing MLLP senders and stops draining the staged queue. `_MAX_PART_HEADER_BYTES` (16 KiB) is orders of magnitude above anything a real client sends — a Content-Disposition plus a Content-Type is a couple hundred bytes, pinned by a non-vacuity test asserting a realistic header is 50x under the limit. Refuse rather than skip: skipping would surface as the confusing "no file part" error instead of naming the actual problem. It maps to the existing 400. The two controls stay independent on purpose. The cap is a size policy someone could reasonably raise; the linear-time assertion holds at any size. Incidentally the bound also gives the ReDoS analysis a length-bounded source rather than an attacker-sized one, which matters because the query models a lookaround as a zero-width assertion and may not credit the lookbehind. Verified by mutation: deleting the guard block reds test_oversized_part_header_is_refused_not_parsed. * docs(ide): claim only the identity guarantee the fd rewrite actually buys readCapped's docstring implied the fd rewrite restored the maxBytes cap. It does not. `fs.readFileSync(fd)` re-stats the descriptor itself and reads whatever length it then finds, so a file appended to in place between the `fstatSync` and the read is still read in full — the size check is an early-out, not a bound. What the rewrite does buy is real and worth stating exactly: one path resolution instead of two, so `fstatSync(fd)` and `readFileSync(fd)` cannot disagree about WHICH file they touched. That is the identity guarantee, and it is what CodeQL js/file-system-race flagged. The residual is recorded rather than papered over: maxBytes has always been a best-effort resource guard ("a generated blob isn't a feed"), never an authorization decision; the cost of losing it is memory for one oversized regex pass; and scanModuleSymbols returns only {name, kind, file, line}, never file content, so nothing leaks through it. Making the cap sound would take a bounded readSync into a pre-sized buffer — deliberately not done here, since it would also rewrite the fd-spy test and this worktree cannot run the ide mocha suite. Comment-only: the 127 non-comment lines are byte-identical before and after. * docs(adr): record the second static-analysis triage round in ADR 0034 ADR 0034 AC-3 requires the class-level rationale and the accepted-risk register to live in-repo so re-scans converge instead of re-litigating. 32 open findings were triaged and the register had not been touched, so the whole round existed only in per-alert comments. Appended as a dated amendment rather than an edit: the repo's own slug-rot guard treats docs/adr/ as historical by construction — an ADR should describe the topology of ITS day — so the 2026-06-26 text stays intact and this section governs where they disagree. Four things the amendment records that a re-scan would otherwise lose: Topology correction. The original Context and the whole Scorecard register rest on MEFORORG being a read-only mirror fed by force-pushed snapshots. Since the cutover it is the primary development repo; publish.ps1 and the release-sync check are gone. Three dismissals reasoned ENTIRELY on that premise — BranchProtectionID (#33), CodeReviewID (#77), MaintainedID (#78) — now carry a justification that is no longer true and must be re-triaged, not renewed. log-injection, narrowed. The register claims control characters "in every emitted record" are neutralized by ControlCharScrubFilter. Precisely: that filter scrubs the RENDERED MESSAGE (record.getMessage() -> record.msg, so the %-args are covered) and does NOT touch record.exc_text or record.stack_info. RedactionFilter renders and PHI-redacts those but does not escape control characters, and the text formatter then appends exc_text verbatim. Verified by execution: a ValueError carrying a newline, logged with exc_info=True, lands on its own physical line. All four log-injection alerts this round are lazy %-arg sinks with no exc_info so the dismissals stand on their own traces — but the class rationale must not be inherited by a future finding on a log.exception site, and the engine has many. PinnedDependenciesID, corrected. "CI installs editably, which cannot use --require-hashes" is structurally true of the editable installs and was applied too widely: three scratch venvs whose only install is a hash-verified lock carried an unpinned pip bootstrap that bought nothing. Also records the proof, from this repo's own alert data, that an == pin does NOT satisfy the check (bandit==1.9.4 is still flagged) — only --require-hashes does, which couples to DEP-1. Convergence and residuals. The line-drift rule (same expression, new line, new alert number, with both confirmed instances), and a table of five hardening items found while justifying won't-fix dismissals — an unpinned sigstore inside the signing job foremost — which a dismissal would otherwise make invisible. AC-5/6/7 added for the guards this round introduced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the four advisory quality signals in
quality-advisory.ymlvisible to a reviewer for $0,instead of printing them to a log nobody opens — no GitHub Code Quality SKU (GA 2026-07-20, $10 per
active committer/month, a standalone product not bundled with GHAS).
Uses GitHub workflow-command annotations, not code scanning: no token, no permission grant, and
identical behaviour on fork PRs. Every job keeps
contents: readand nothing else.::noticeon the Files changed tab, adjacent uncovered lines coalesced into rangesC901)defline, so mostly renders in the Checks tab + summaryjscpd)mutmut)Why not SARIF / code scanning
Measured, not assumed, and recorded in the workflow header so nobody re-adds it: all 122
C901findings anchor on a single
defline (body edits surface nothing; signature edits surfacepre-existing debt); jscpd emits one location per clone pair chosen by scan order; and a PR-only
upload never builds a baseline, so every PR forever would report all ~161 findings as new.
Two signals were broken and reporting success
git fetch --depth=1against the full clone wrote.git/shallowand killeddiff-cover's merge base whenever
mainadvanced. diff-cover truncates its report before it canfail, so
|| trueswallowed the error and the summary rendered a clean-looking empty section.mutmut<3resolved to 2.5.1, which crashes on Python 3.14 before generating asingle mutant.
|| truemade it green in 37s. It had been scored "Built" in the rubric since2026-07-14 while measuring nothing. Repaired on
mutmut==3.6.0, verified on Linux: 461 mutants,87 killed, 19 survived, 3 seconds — which also refutes the "expensive, never per-PR" cost model,
so it now runs on PRs.
Also here
scripts/quality/c901_delta.py+ 24 tests — reduces 122 pre-existing findings to the 0–3 a PRcaused. Verified end to end against two real trees at different absolute roots.
tests/test_quality_advisory_invariants.py— 23 assertions pinning no-write-scope, no SARIF, nopull_request_target, SHA-pinned actions, and every analysis step non-failing. Each wasnegative-probed against a mutated copy of the workflow; all 20 mutations were caught.
docs/Code_Quality_Standards.mdrestored — cited by this workflow's header andpyproject.tomlbut absent from the repo's entire git history. Bumped to v0.10 with the corrections above.
constraints.lock, which would have turned a required context red on a routine Dependabotbump. The version is now derived from the lock at run time.
Verification
ruff check+format --checkclean ·mypyclean (288 files) · pytest 9092 passed, 797skipped · bandit, zizmor and the leak gate all real exit 0 (not read through a pipe).
Nothing here is or becomes a required check.