Proof: a dispatch admits the run's origin, not only the artifact's name - #552
Merged
Conversation
added 7 commits
August 6, 2026 18:48
Наблюдаемый дефект: coordinator проверял, что --evidence-run-id положительное целое, но не то, что такой прогон существует и несёт названный артефакт. Положительность — не свидетельство существования. Я запустил диспатч против run 99999999 и получил 133 обречённых прогона, каждый падал за 9-11 секунд на скачивании несуществующего артефакта. Одна ошибка координатора превращается в 256 обречённых задач, и узнаёшь ты об этом 256 раз подряд вместо одного. Закон: перед первым диспатчем координатор спрашивает, несёт ли названный прогон названный артефакт, и отказывает закрыто. Наблюдение GitHub — единственная нечистая граница — инъектируется, поэтому само решение проверяется без сети. Любой сбой наблюдения (нет прогона, нет токена, битый ответ) — тоже отказ, а не падение и не молчаливый проход. Dry run остаётся офлайновым по контракту, гейт живёт только на боевом пути. Доказательства: четыре RED-теста (прогон без артефакта, недостижимый прогон, успешный допуск, anti-vacuity — наблюдателя спрашивают именно про названный прогон) плюс проверка реальностью: admit_evidence_artifact_v1(99999999, ...) отвергает тот самый вход, что стоил 133 прогонов, а 31089986150 допускается для обоих движков. Локально: 14 тестов координатора, OK.
…n provably Две независимые линзы ревью дали FAIL. Находки закрыты; две из них означали, что гейт лишь ВЫГЛЯДЕЛ рабочим. 1. Истёкшие артефакты. Листинг GitHub включает артефакты с expired:true, скачать которые нельзя. Гейт брал имя не глядя, поэтому старый evidence-run проходил бы допуск и снова давал 256 обречённых полос — ровно тот класс, который срез объявляет закрытым, только через соседний путь. Теперь jq отбирает `select(.expired | not)`. 2. Шов не был покрыт. Четыре теста доказывали РЕШЕНИЕ допуска, но ни один не доказывал, что main() спрашивает. Удаление прошивки оставляло все 14 тестов зелёными. Добавлены четыре теста шва: отказ не диспатчит ничего, допуск диспатчит все 256 полос, dry run никого не спрашивает и ничего не запускает, отвергнутый набор команд уходит через exit 64. Саботаж подтверждает чувствительность: удаление прошивки красит test_a_refused_evidence_run_ dispatches_nothing. 3. Точка инъекции была декларативной. Значение по умолчанию связывало gh_run_artifact_names_v1 при определении функции, поэтому подмена наблюдателя не действовала: шов выглядел инъектируемым и не был им. Дефект вскрыл сам тест шва — резолвинг перенесён в тело. 4. Отвергнутый набор команд попадал в цикл и давал нетипизированный TypeError: 'ShardCorpusRejectedV1' object is not iterable. Дефект предсуществующий, но мой guard `type(commands) is tuple` был написан ВОКРУГ отказа, не обработав его, и нарушал тот самый инвариант, который срез заявляет. Теперь типизированный exit 64. 5. Пагинация: gh api без --paginate отдаёт одну страницу; имя на второй странице читалось бы как отсутствующее. Добавлен --paginate. Направление и без того было fail-closed, но ложный отказ на валидном прогоне — тоже дефект. 6. Причина отказа теперь едет с отказом: оператор обязан отличать «нет такого прогона» от «нет токена», иначе диагностика инцидента требует ручного повтора вызова. Проверка реальностью: 99999999 отвергается, 31089986150 и 31022375756 (оба движка, живые артефакты) допускаются. Локально: 18 тестов координатора, OK.
…rule where a test can reach it Пере-ревью финального состояния. Два дефекта, оба доказаны мутацией, и оба означали, что защита выглядит рабочей и не является ею. 1. Тесты шва не закрепляли КООРДИНАТУ RUN_ID. Оба подменённых наблюдателя игнорировали аргумент, поэтому мутант, зашивающий чужой id в вызов допуска (admit_evidence_artifact_v1(1, args.evidence_artifact)), выживал при всех 18 зелёных тестах. Координата артефакта была закреплена, а run_id — тот самый, на котором и произошёл инцидент с 99999999, — нет. Живой сценарий: гейт наблюдает прогон A и допускает, а 256 полос диспатчатся связанными с прогоном B. Наблюдатели шва сделаны чувствительными к run_id; мутант гибнет. 2. Правило истёкших артефактов жило в jq-выражении, то есть за сетью, и не было покрыто ничем: удаление select(.expired | not) оставляло все 18 тестов зелёными. Наблюдение теперь только НАБЛЮДАЕТ и возвращает пары (имя, expired), а решение, что считается свидетельством, применяется в чистом ядре, где до него дотягивается тест. Два теста: истёкший артефакт — отказ; живой рядом с истёкшим — допуск (иначе правило выродилось бы в сплошной отказ). Мутант «убрать фильтр» гибнет. 3. Причина отказа несла repr(error), а CalledProcessError.__repr__ отбрасывает stderr — ровно тот текст, который отличает «нет такого прогона» от «нет токена». Комментарий обещал различимость, код её не давал. Теперь detail несёт stderr: живая проверка даёт «cannot observe run 99999999: gh: Not Found (HTTP 404)». Проверка реальностью: 99999999 отвергается с причиной, 31089986150 допускается, наблюдение возвращает обе пары с expired=False. Локально: 20 тестов координатора, OK.
…ly claimed Two independent reviews of the final state — one of them running actual mutants — showed this slice narrowed the incident class without closing it, and that its own commit message overstated what was pinned. THE POLARITY. The incident was a forgotten `--dry-run`, and the fix left the dangerous run as the default: safety still meant remembering a flag. Printing is now what happens, `--execute` is the opt-in, and it has to name the campaign's size with `--expect-lanes`. Swapping the two widths is a realistic slip — `--lane-width 256 --shard-width 4` is 65536 runs — and the only cheap moment to notice is before the first invocation. THE ARTIFACT COORDINATE. A mutant hardcoding `EVIDENCE_ARTIFACTS_V1[0]` at the admission call site survived all twenty tests: both seam tests always passed the arb name, so pinning one engine was indistinguishable. The producer runs its two engines as independent jobs, so a run really can carry one artifact and not the other, and admitting the wrong one buys 256 doomed lanes — the incident down a second axis. The previous message claimed this coordinate was pinned; it was pinned where commands are built, not where the admission asks. THE EXPIRY RULE. Moving the filter out of the query put the decision where a test could reach it and left the decoding that produces it behind the network. Two mutants survived: one that never marks anything expired, one that drops the field from the query. `parse_artifact_listing_v1` is pure now and refuses a listing that is not the two-column form it asked for — the old parser read an unrecognised line as "live", which admits exactly the stale evidence run this gate exists to refuse. The repository currently holds 127 expired artifacts, so the path is not hypothetical. A campaign that dies mid-flight now reports how many lanes it launched and returns the typed exit instead of a traceback, so the retry can resume rather than duplicate. Every repair is proven by the mutant it kills, each failing exactly one test: the hardcoded artifact, the never-expired listing, the field dropped from the query, and `repr(error)` in place of the stderr that tells "no such run" from "no token". Verified: 276 local tests, 7 errors — the exact Windows baseline. Left out deliberately, and not hidden: the admission still checks that the named artifact exists, not that it carries `job.bin` and the comparator bundle, and not which workflow or commit produced it. Those are a different class — supply-chain provenance rather than operator error — and want their own slice.
… it was pinned A third review ran fifteen mutants against the previous commit; nine survived, and all but one lived in the code that commit added. The polarity change and the partial-failure report were shipped without a single test that could hold them. THE RUN ID. All three seam tests take the run id from one helper that hardcodes 424242, so a call site hardcoding that same number was indistinguishable from one that reads the argument — while the test's own comment claimed the opposite in as many words. The mirror axis was fixed last round and this one left standing. A seam test now names a different run and records what the observer was actually asked. THE DECLARED SCALE. Making `--expect-lanes` optional survived every test, because falling through to the mismatch comparison refuses too — with a message that tells the operator nothing. It has its own refusal now, and the test reads the message rather than only the exit code: the two differ exactly where the operator does. THE PARTIAL FAILURE. Deleting the whole handler, returning 0 mid-campaign, or moving the counter before the call all stayed green. The count is the point — a retry that cannot tell three from zero duplicates everything already dispatched — so the test now reads "after 3 of 256" out of the report. The handler also catches OSError: a missing `gh` abandoned a campaign with a traceback and no count. ORDER. The authenticated observation ran before the free local check, so a mistyped width spent an API call and answered with a diagnosis about the evidence run. The cheap refusals come first now, which is what the comment claimed all along. `--mode plan --execute` was a silent no-op: a flag with live semantics that is quietly ignored is worse than one that refuses, because the operator believes a campaign started. `--paginate` was checked by reading the function's source, which a mutant that drops the flag from argv and leaves the word in a comment passes untouched. The check is behavioural now — it asserts the argv actually built, plus `check` and `capture_output`, which the source read never saw. Names that referred to the removed flag are gone; only the comment recounting the incident still says `--dry-run`, which is history and stays. Every repair is proven by the mutant it kills, each failing exactly one test: the hardcoded run id, the optional scale, the vanished counter, `return 0` mid-campaign, the ignored `--execute` in plan mode, and `--paginate` demoted to a comment. Verified: 282 local tests, 7 errors — the exact Windows baseline.
The evidence admission checked that a live artifact of the named name existed inside the named run. Three runs pass that and must not: one produced by a different workflow, one a fork's pull request produced — carrying a comparator bundle nobody reviewed — and one that never finished successfully. Each buys 256 lanes replaying something the operator did not intend, and the reproduction is exit 0 with 256 dispatches while nothing was ever asked about the run. The run is now observed as a projection of five named fields (never the run object: an unknown reply is how this project has leaked), decoded by a rule that refuses shape drift instead of defaulting, and admitted by a pure rule covering the workflow path, the workflow_dispatch trigger, completion with conclusion success, and a readable head_sha. Any failure to observe is a typed refusal, as with the artifact. Staleness cannot be decided here — last week's green producer run satisfies every rule above — so the one coordinate that settles it is put in front of the operator at the moment of admission, before the lanes start. Twelve mutants of the new rules were run against the suite and all twelve were killed, including a dropped seam call in `main`, a fail-open observation, a decode that invents missing fields, and a query that fetches the whole reply. The artifact's *contents* are still unchecked: proving job.bin and the comparator bundle exist requires downloading it, and that is its own slice.
Independent verification ran mutants against the previous commit and found three rules that lived in the code and in comments beside it, with nothing to hold them. `text=True` was the dangerous one. Removing it makes `gh` return bytes, the decode raises, and every run is refused — a gate that looks alive and admits nobody, fail-closed and silent. No test saw it, and the same gap existed for the artifact observation, so both are pinned now. The comment claimed origin is asked before contents because a run of the wrong workflow lists an artifact of exactly the right name. True, and unproven: swapping the two blocks left everything green. The order is the claim, so the order is what the test reads — the artifact observer must never be asked when the origin is refused. `RunProvenanceV1.__post_init__` was unreachable — the parser only ever feeds it string slices — and raised a bare `TypeError` in a module whose whole thesis is typed refusals. Deleting it left the suite green, which is the definition of speculative defence. Gone. Proven by the mutant each kills, one test apiece: the missing `text=True`, and the swapped gate order. Verified: 54 dispatch tests pass; 452 on Linux with the three pre-existing `test_build` failures that also fail on an untouched tree. This commit was authored on the #548 branch by mistake and moved to its own before anything was pushed — #548 has been through three review rounds and does not need a new feature landing inside it.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
Conflicts were real and resolved by intent, not by side: - the observation stubs take main's shape — kwargs recorded and asserted outside the stub, where PYTHONOPTIMIZE=2 cannot delete them — and keep this branch's `text=True` pin on top, so both lessons survive; - the dispatch loop takes main's single handler for every way a campaign dies mid-flight; - the origin gate stays ahead of the artifact gate, and both stay behind the free coordinate checks; - main's overdeclared-scale test and this branch's provenance suite coexist. Per the merge gate's recommendation, the provenance observation now carries the same deadline as the artifact listing: it is the first call of the whole campaign, and a hung `gh` there is just as indistinguishable from work in progress. Verified: 55 dispatch tests pass, and pass again under PYTHONOPTIMIZE=2.
lemone112
pushed a commit
that referenced
this pull request
Aug 7, 2026
One conflict, the module docstring: both paragraphs are true — the origin admission from #552 and the collect mode from this branch — so both stand. Everything else, including the provenance chain and its tests, merged clean. Verified: 95 dispatch tests pass, and again under PYTHONOPTIMIZE=2.
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.
Что это
Третий путь инцидента со 133 мусорными прогонами. Допуск evidence-прогона проверял имя артефакта, но никогда не спрашивал, какой прогон его произвёл.
Дефект, воспроизведённый на исходном коде
Прогон от
pull_requestиз форка, выгрузивший артефакт с честным именемverification-evidence-arb:256 полос реплеят под отравленным comparator-bundle, и никто не спросил, какой воркфлоу, какое событие и какой коммит это породили.
Тот же путь открыт для стейл-прогона старого коммита: имя честное, содержимое устарело.
Что закрыто
Наблюдение прогона белым списком из пяти полей (не сырой объект — печатать ответы API неизвестной формы в этом проекте запрещено), и чистое правило допуска поверх него:
path, а не basename:vendor/.github/workflows/full-domain-run.ymlне пройдёт;workflow_dispatch, неpull_request;completedиsuccess— оба, потому чтоin_progressс пустым заключением иначе проходит;head_sha, и он печатается оператору в момент допуска.Порядок: origin раньше contents, до первого диспатча.
Что нашла независимая проверка и что исправлено
Проверяющий прогнал 23 мутации: 15 убиты, 8 выжили. Три починены:
text=Trueне был закреплён ничем. Убрать одну строку — иghотдаёт байты, декод падает, каждый прогон отвергается. Гейт выглядит живым и не пускает никого. Fail-closed, то есть не дыра, но фича мертва, и ни один тест этого не видел. Тот же пробел был и у наблюдения артефактов — закреплены оба.Порядок гейтов заявлялся комментарием, а не проверялся. Перестановка блоков оставляла набор зелёным. Теперь тест читает именно порядок: при отвергнутом origin наблюдатель артефактов не должен быть спрошен ни разу.
__post_init__был недостижим — парсер всегда подаёт строки — и бросал голыйTypeErrorв модуле, весь тезис которого типизированные отказы. Удаление оставляло набор зелёным, что и есть определение спекулятивной защиты. Убран.Каждая починка доказана убитым мутантом, по одному тесту на каждый.
Проверки
test_build, воспроизведены на нетронутом дереве.test_corpus_dispatch.pyне входит в закрепляемый набор.--jqпо пяти полям, в отказ цитируется только stderr.Происхождение ветки
Коммит был по ошибке создан на ветке PR #548 и перенесён на свою до любого пуша: #548 прошёл три круга ревью, и новая функциональность не должна приземляться внутрь него.
Оставлено открытым и названо
Ещё пять мутантов выжили:
.strip()полей, фильтр пустых строк,!= 5против< 5в проверке ширины записи, лишний флаг в argv. Все они не меняют поведение на реалистичном входе, но и не различаются тестами. Это долг, а не дефект, и он назван здесь, а не спрятан.Откат
Ревертом одного коммита. Внешних контрактов не меняет; единственный видимый эффект — диспатч отказывает там, где раньше молча соглашался.