fix(ci): the auditor ran the whole suite on a Python it cannot import on - #2183
Conversation
… on (#1822's neighbourhood) `.github/workflows/pr-checks.yml` — the "Misaka Network Agent Auditor", the repository's real merge gate — pinned `python-version: "3.10"` while running the entire test suite. Two test files import `tomllib`, which is stdlib only from 3.11. On 3.10 pytest therefore died during **collection** (`ModuleNotFoundError: No module named 'tomllib'`, exit 2) and the audit reported: ##[error]Test suite failed Audit failed: test suite has issues. which blames the contribution for a runner version. Measured while triaging the open backlog: **3 of the 11 open PRs** (#1999, #2020, #1982) carried exactly this — their red `audit` said nothing about their content, and the reason was several thousand log lines up. (For completeness, the other red audits were different things: #2037 is a genuine DCO failure, and #2090/#2068 never got an audit run at all — neither is touched here.) The required matrix (`ci-cross-platform.yml`) has always been 3.11/3.12/3.13. This job was the outlier. Two changes: 1. **`python-version: "3.11"`** — the version the repository actually tests. 2. **The verdict names its cause.** pytest's exit code is now carried out of the test step (`echo "exit=$PYTEST_EXIT" >> "$GITHUB_OUTPUT"`) and the summary branches on it: exit 2 says the suite could not be COLLECTED and no test ran, and points at the runner; 1 is failing tests, 4 a usage error, 5 nothing collected; the old catch-all remains only as the fallback. A red gate has to name its cause, or the reader has to go and find it — which is exactly how 3 PRs sat blocked on a one-line workflow bug. Gates: * `tests/test_workflow_python_floors.py` (new, 14 tests) derives the suite's floor from the imports in `tests/` — `STDLIB_FLOORS` maps `tomllib` → 3.11 — and fails any job that runs pytest on something lower, or on a version the matrix never tests. The floor is derived rather than declared: a hand-written "the suite needs 3.11" would be stale the moment somebody reaches for `except*` or `datetime.UTC`. Matrix expressions (`${{ matrix.python-version }}`) are resolved, not skipped — a resolver that gave up on that would drop the job that *defines* "tested", and there is a test that it stays visible. Versions are parsed as integers: as strings `"3.10" < "3.9"`, i.e. the exact comparison that would have let this bug through. * `tests/test_audit_report_truth.py` gained 6 tests that execute the real `Post Audit Report` step, with its `_EXPRESSIONS` map extended by the new output. That harness substitutes `${{ … }}` into the step script, so it cannot notice if the *test* step stops publishing the code — hence a separate structural test for the `>> "$GITHUB_OUTPUT"` line. Evidence (2026-09-25): `python3 -m pytest tests/` → 2215 passed, 15 skipped. Mutations, all red: the auditor back to 3.10 · the floor table emptied · a floor entry nothing imports · the matrix dropped below the floor · matrix expressions silently unresolved · versions compared as strings · the case statement replaced by the old catch-all · the exit code no longer published · the "it is the runner" hint dropped · a real test failure relabelled a collection error. Three mutations were *equivalent* and are documented rather than kept: a no-op edit to `_v`, a behaviour-preserving rewrite of the same function, and — in the earlier, separate version of these tests — restoring the old `--kv-only`-style string. Where a mutation proved a test was pinning a shape instead of a behaviour, the test was rewritten, not preserved. Not changed, and worth a decision rather than a silent edit: `pyproject.toml` still declares `requires-python = ">=3.10"`. Only the *test suite* needs 3.11 — the package itself may genuinely run on 3.10 — so lowering that claim is a support-policy change and belongs to the owner. Signed-off-by: misakanet-bot <bot@misakanet.dev>
#1999 已经有落地的工作,这条 PR 不需要继续@Ikalus1988 谢谢你来收 #1999。它不是没人做:
重复投稿不会被合并,也不会重复支付:赏金结在合并了的那一条上。(2026-09-21 的 #1942 就同时收到三条 PR #1959/#1968/#1980。) 回主线的路
|
PR Genius Analysis
Checklist
Anti-Patterns Detected
Suggestions
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
misakanet-web | bb0636b | Commit Preview URL Branch Preview URL |
Sep 25 2026, 02:55 AM |
Standalone PR ReviewPR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
✅ Merged! Thanks again, @Ikalus1988. fix(ci): the auditor ran the whole suite on a Python it cannot import on (+345 lines, 3 files) Quick question — did any MisakaNet lesson help you this time? No need to reply if nothing comes to mind. ⚡ |
🎉 Merged — Thank you!Your contribution has been merged into main. PR: #2183 — fix(ci): the auditor ran the whole suite on a Python it cannot import on What's next:
Welcome to the MisakaNet contributor community! 🧠 |
🧾 Audit Report — PR #2183 (bb0636b)📊 Quality Score🔏 DCO Audit✅ All commits signed-off. 📏 PR Size
🔐 Secret Scan✅ No hardcoded secrets detected. 📦 Dependency Audit⏭️ Skipped; no Python/JS dependency files changed. 🧪 Test Suite✅ PASS — 56% coverage 📋 Lesson Schema✅ All lessons valid. ⚖️ Verdict✅ All gates passed. Ready for merge. Scope: |
PR Code Suggestions ✨Explore these optional code suggestions:
|
…blishing two tokens (#1982) (#2184) While checking whether #1982 was ready to land — docs-only, owner-labelled `auto-merge-eligible`, 3/3 required checks green — its diff turned out to contain two **live-looking node tokens**: docs/field-reports/2026-09-21-kiro-mcp-smoke.md docs/field-reports/2026-09-21-opencode-mcp-smoke.md "Authorization": "Bearer mcp_…" # 32 characters after the prefix That is exactly what `misakanet_register` issues (`workers/register-proxy-sw.js:2510-2515`), and the two bodies carry 25 and 26 distinct characters — every other `mcp_…` string in this repository is a tool name (`mcp__misakanet__search`) or an example and tops out at 15. Nothing objected, and the reasons were structural: the auditor's `Secret Scan` step runs `scripts/check_worker_secrets.py`, which reads `workers/**` only; that step was also gated on `scope == 'full'`, so a docs-only PR skipped it entirely; HOL Guard scans `lessons/`, `scripts/` and `workers/`, not `docs/`; and the audit's test suite was aborting during collection on the pinned Python (fixed earlier today in #2183). Four scanners, none of them looking at the file. **Safety action taken first** (not in this diff): auto-merge turned off on #1982 and the `auto-merge-eligible` label removed, so the next passing check cannot land the tokens; a comment on the PR asks for the redaction, tells the contributor to treat both values as burned, and names the two keys the maintainer can delete to kill them now (`mcp_token:<token>`, `node:MisakaNNNN` — there is no revocation endpoint). Then the gate, so this is not luck next time: * **`scripts/check_published_secrets.py`** (new) scans `docs/**` and `lessons/**` prose plus the root documentation files. A bare `mcp_[A-Za-z0-9_-]{32}` would be useless here — the tree is full of tool names and placeholders — so the rule is shape (exact 32 characters, no leading `_`) plus an entropy gate, with the threshold **measured rather than chosen**: leaked bodies 25/26 distinct, everything legitimate ≤15, `MIN_DISTINCT = 20`. Findings print path, line and kind and never the matched string, since a finding lands in a build log. * **The step now runs for every scope.** `if: steps.scope.outputs.scope == 'full'` is what made the leaking PR class invisible; both scans take seconds. They run in sequence with a shared failure flag, so a finding in the second is not hidden by a non-zero exit from the first. * **`AGENTS.md` §3.3** (where the "token only in the Authorization header" rule lives) now says the rule is enforced, and that a fork PR's diff is public immediately — deleting the line does not un-expose it. * **`docs/maintainer/credentials-and-environments.md` §6** is the incident playbook: redact, re-register with the same `client_id`, delete the stored keys, assume the Bearer-only write tools are reachable until expiry, and — the step that is easy to miss — turn off any auto-merge before fixing the file. Evidence (2026-09-25): - `python3 -m pytest tests/` → 2229 passed, 15 skipped. - `python3 scripts/check_published_secrets.py` → clean over **732** published prose files (the corpus direction matters as much as the catching direction: a gate that reports on main is a muted gate). `tests/test_published_secrets_scan.py` (14 tests) pins both directions and the wiring; 6 mutations all go red: entropy gate removed · tool-name exclusion removed · a finding echoes the matched line · the step goes back to full-scope-only · the prose scanner dropped from the step · `docs/` dropped from the surface. Two of those mutation runs changed the work rather than confirming it, and both are recorded instead of quietly fixed: * A test claiming "without the entropy gate the corpus would be red" **failed, correctly**: measured, the ungated rule finds 0 strings in the tree, because the length and underscore rules already cover everything there. The gate's real job is tomorrow's placeholder (`mcp_` + 32 identical characters), and the test now proves exactly that, synthetically. * The coverage assertion was a file count, so dropping `docs/` from the scan surface left the suite green (lessons/ alone is >200 files). It now names the surface the leak was on — `docs/field-reports/` — because that is what the scanner exists for. Signed-off-by: misakanet-bot <bot@misakanet.dev>
…index than none (#2185) (#2187) `data/sag.db` is gitignored, so every fresh clone builds it — from `data/okf/lessons.jsonl`, a **tracked** file with **no writer in CI**. Measured 2026-09-25: data/okf/lessons.jsonl last written 2026-07-07 (194 rows) a fresh export_okf.py 411 rows the committed export names 177 of 453 lesson files (61% missing) and `misakanet/server/handlers/search.py` prefers SAG over the complete BM25 path (`if HAS_SAG and not explain:`), while the hint it returns when no index exists says only: "Run: python3 scripts/build_sag_index.py to enable BM25/SAG search" `build_sag_index.py` only complains about a missing export (`Run export_okf.py first` is in the *file-not-found* branch), and the tracked file exists — so following the documented remedy produced an index covering 39% of the corpus, preferred over the one that covers all of it. Demonstrated with a probe query taken from a lesson the stale export cannot contain: stale (194 lessons) -> 0 hits fresh (411 lessons) -> 1 hit Two changes, both halves of the trap: * **the hint names both steps, in order** (`export_okf.py && build_sag_index.py`), because the second command reads what the first one writes; * **`build_sag_index.py` measures the export against the corpus** and prints a warning naming the cause and the command when it covers less than `COVERAGE_FLOOR` of it. Threshold measured, not chosen: a fresh export covers 91%, the committed one covered 39%. It warns rather than refuses — an old checkout or a filtered export is legitimate, and a build script that will not run is a worse trap than the one it closes. Evidence (2026-09-25): `python3 -m pytest tests/` → 2241 passed, 15 skipped. Against the committed export the builder now prints the warning; against a fresh one it stays silent. `tests/test_sag_export_freshness.py` (12 tests) pins the measurement, the boundary, the silent case outside a checkout, and both wirings; 4 mutations go red: the call removed from `build_index` · the threshold dropped below the stale population · the denominator counting READMEs · the hint losing the export step. The first of those four was green on the first attempt and is the reason the file now has `test_building_from_a_stale_export_warns_at_the_point_of_use`: every other test called the warning function directly, so deleting its call from `build_index` left the suite passing. The function was tested; the wiring was not — the same shape of hole as the `--kv-only` early return in #1822 and the `exit` output in #2183. What this does NOT fix, and #2185 stays open for it: nothing regenerates the tracked export in CI, so it will drift again. That wants a writer in the daily job (the same treatment `data/lessons.json` gets) or a decision to stop tracking it — an owner call about where the export step belongs, not a code change I should make on my own. Signed-off-by: misakanet-bot <bot@misakanet.dev>
… path answered without them (#2240) `misakanet_search` reads `summary_plain` / `trigger` / `verify` off the *record*, and there are two sources for that record: * the D1 path, where the row's `frontmatter` column is lifted onto it (`:3987`, `:4013`); and * the GitHub/KV fallback (`loadLessons` -> `fetchFromGitHub("lessons.json", "data")`), which applies **no lift at all** — so the only thing the projection can see is a top-level key on the index entry. `scripts/update_lessons_json.py` picks its fields by hand and never wrote one, so on that path every lesson lost all three. Measured before this change: 411 index entries, `summary_plain` 0, `trigger` 0, `verify` 0 — while 15 lessons carry them in frontmatter. Two things made it invisible rather than merely broken: 1. `evidence_level` works on that same path **purely because the generator does emit it** (411/411). The neighbouring `frontmatterField(lesson.frontmatter, ...)` fallback cannot rescue the plain fields, because no index entry has a `frontmatter` key at all (0/411) — so `frontmatterFields()` early-returns `{}`. One line working for a reason nobody had written down, the next one inert for the same reason. 2. The one test that covers the fallback seeds a record that *does* carry a `frontmatter` key (`workers/index-and-trust-shape.test.mjs`), which is a shape production never produces. The fix is the generator, not the projection: `plain_fields()` mirrors the worker's `plainFields()` usable-value rule (a non-empty string, nothing else), and the three values go in top-level. Lessons without the fields gain **no** key — the worker's byte-identity guarantee for them stays structural, and the index stops carrying placeholders that read as content to anything other than the worker. `docs/maintainer/lesson-fields.md` documented this exact gap as "one line, out of scope, can be added later"; that paragraph is now the explanation of why it had to be the generator. The new gate derives its expectations from the **corpus**, not a fixture: it reads each lesson's markdown and requires the index to agree, counts the carriers and fails below a floor (a broken probe is a failure, not a vacuous green), and pins the generator's field table against the worker's `PLAIN_FIELD_KEYS` in both directions. One test drives the real `main()` into a temp path — without it every other assertion stays green when the emission line is deleted, because they all compare the committed file, which is the "function is covered, the wiring into the artifact is not" shape recorded in #1822 and #2183. Verification (this checkout, 2026-09-25): * 7 mutations behave as specified, plus a control that must stay green: deleting the emission line, drifting the generator's field table, drifting the worker's field table, emitting placeholders instead of values, breaking the probe, and letting the committed index go stale. * `python3 -m pytest tests/` -> 2445 passed, 15 skipped (baseline 2437 + the 8 new). * `node --test workers/*.test.mjs` -> 491 tests, 490 passed, 1 skipped (unchanged). * `sync_lesson_count.py --check`, `build_lesson_pages.py --check`, `doctor.py`, `check_published_secrets.py` all green. * `data/lessons.json`: 45 insertions (15 lessons x 3 fields), every pre-existing key byte-identical. Signed-off-by: Ikalus1988 <136884451+Ikalus1988@users.noreply.github.com>
User description
The auditor — the repository's real merge gate — pinned Python 3.10 while running the whole test
suite. Two test files import
tomllib, stdlib only from 3.11, so pytest died during collection:and the verdict blamed the contribution for a runner version. The required matrix has always been
3.11/3.12/3.13 — this job was the outlier.
Measured on the open backlog (2026-09-25)
I checked all 11 open PRs by fetching each one's latest
auditjob log:tomllibcollection error on 3.10So this is 3 PRs, not "all of them" — the number is worth stating precisely, because the point is that
those three reds carried no information about their content.
Two changes
1.
python-version: "3.11"— the version the repository actually tests.2. The red now names its cause. The test step publishes pytest's exit code, and the summary
branches on it:
Audit failed: test suite has issues.was the only message for days while no test ran. A red gatehas to name its cause, or the reader has to go and find it — several thousand log lines up.
Gates
tests/test_workflow_python_floors.py(new) derives the suite's floor from the imports intests/(
STDLIB_FLOORS:tomllib→ 3.11) and fails any job that runs pytest below it, or on a version thematrix never tests. Derived, not declared: a hand-written "the suite needs 3.11" is stale the moment
somebody uses
except*ordatetime.UTC. It also resolves${{ matrix.python-version }}instead ofskipping it — a resolver that gave up there would drop the job that defines "tested" — and has a test
that the matrix job stays visible.
tests/test_audit_report_truth.pygains 6 tests that execute the realPost Audit Reportstep(that file's existing harness) and read its stdout. Because the harness substitutes
${{ … }}into thescript, it cannot notice if the test step stops writing the output, so a structural test pins that
line too.
Evidence
Mutations, all red: auditor back to 3.10 · floor table emptied · floor entry nothing imports ·
matrix below the floor · matrix expressions silently unresolved · versions compared as strings
(
"3.10" < "3.9"— the trap that would have let this through) · case statement replaced by the oldcatch-all · exit code no longer published · "it is the runner" hint dropped · a real failure relabelled
a collection error.
Three mutations were equivalent, and are recorded rather than papered over: a no-op edit to the
version parser, a behaviour-preserving rewrite of it, and (in an earlier separate version of these
tests) restoring a string that a real call site had already made harmless. In the one case where a
mutation showed a test was pinning a shape rather than a behaviour, the test was rewritten.
Left alone on purpose
pyproject.tomlstill saysrequires-python = ">=3.10". Only the test suite needs 3.11 — thepackage itself may genuinely run on 3.10 — so changing that claim is a support-policy decision, and it
is yours rather than mine.
PR Type
Bug fix, Tests
Description
Pin auditor Python to 3.11 (tomllib is stdlib ≥3.11)
Name the cause in audit failure messages via pytest exit code
Add derived floor test preventing any job pinning below it
Add tests verifying collection-error vs test-failure messages
Diagram Walkthrough
File Walkthrough
pr-checks.yml
Bump auditor Python to 3.11 and name failure cause.github/workflows/pr-checks.yml
setup-pythontopython-version: "3.11"(was 3.10) with explanatorycomment
PYTEST_EXITfrom pytest and publish to$GITHUB_OUTPUTcause is named
test_workflow_python_floors.py
Derive and enforce a Python floor for pytest jobstests/test_workflow_python_floors.py
tomllibimports intests/setup-python(lists them)"3.10" < "3.9")test_audit_report_truth.py
Assert audit verdict names its cause by exit codetests/test_audit_report_truth.py
PYTEST_EXITto the substituted expression maprun_reportintorun_report_streamsthat returns stdout too