锁定 publish verification quarantine 不写 repo-root marker#1032
Merged
Conversation
ElonSG
added
crnd:lifecycle:managed
Contributor
Author
🤖 tests review: approveTL;DR
测试证据我按 PR head
未发现新增固定 sleep/poll pacing、skip/manual-test marker、断言放宽、或只验证 mock 调用的伪覆盖。 Reasoning-discipline note: reference frame 是文件系统副作用回归测试；known-good shape 是驱动公开行为、验证应写入的 durable state，并在关键边界断言 forbidden side effect 不存在。Candidate reject as bump-only: Ugly defect: 会把具体 forbidden-path assertion 误判成 tautology。Beautiful form: 一个真实状态转换加 forbidden side-effect absence check；当前测试满足。Candidate comment for missing source-regression: Ugly defect: 在没有新增 production no-regression rule 或 forbidden-token surface 的 diff 上强加 source-regression。Beautiful form: 只有规则面新增时才要求 source-regression。Chosen approve: Ugly defect: 这个回归点挂在已有 aggregate retry/quarantine test 上，不是独立单目的测试。Beautiful form: 单个行为测试同时覆盖状态转换和 forbidden root marker，且不复制 setup；当前写法已经达到这个形态。所有判定前提均来自实际 diff、PR-head 测试、生产实现行和本地 focused test run，没有使用 ASSUMED-UNVERIFIED 前提。 📎 Full raw codex output (archival)---
pr: 1032
role: tests
head_sha: aad62da123f8d604c57ddc1efa34ca705bbf9f94
verdict: approve
---
## Verdict
Approve: test coverage and quality are adequate for this test-only diff.
## Evidence
- `skills/consensus-loop/scripts/test_publish_verification.py:385` names the scenario and outcome: failed publish-verification receipts use the per-job retry schedule and then quarantine.
- `skills/consensus-loop/scripts/test_publish_verification.py:388` and `:412` add a concrete side-effect assertion that `${repo_root}/QUARANTINED` is absent before and after quarantine, which directly covers the PR goal instead of only bumping line count.
- `skills/consensus-loop/scripts/test_publish_verification.py:393` through `:410` still asserts the full retry behavior and terminal `QUARANTINED` state, so the new absence check is anchored to the real business transition.
- `skills/consensus-loop/scripts/codex_refactor_loop/publish_verification.py:345` through `:360` shows quarantine is recorded by writing `retry.json` under the job directory, matching the test's expected side-effect boundary.
- Ran `python3 -m unittest skills.consensus-loop.scripts.test_publish_verification.PublishVerificationTests.test_failed_receipts_use_per_job_retry_schedule_then_quarantine`: pass.
- No fixed-duration sleeps, polling/stability allowlist changes, skip/manual-test markers, assertion loosening, or mock-only pseudo-coverage were added in the diff.
Reasoning-discipline note: Reference frame is regression testing for file-system side effects: the known-good shape is to drive the public behavior, assert the intended durable state, and assert the forbidden side effect is absent at the boundary that matters. Candidate reject as bump-only: Ugly defect: it would treat a concrete forbidden-path assertion as a tautology. Beautiful form: require a real transition plus absence check, which this test has. Candidate comment for missing broader source-regression: Ugly defect: it would add a source-regression demand without a new production no-regression rule or forbidden-token surface in this diff. Beautiful form: ask for source-regression only when the work unit adds such a rule. Chosen approve: Ugly defect: the remaining imperfection is that the regression is asserted in an existing aggregate retry/quarantine test rather than a separate single-purpose test. Beautiful form: a single behavior test should cover the state transition and the forbidden root marker together without duplicating setup; the current test does that with minimal churn. Premises were verified against the PR diff, PR-head test contents, implementation lines, and focused test run; no ASSUMED-UNVERIFIED premise is used for the verdict.
## What would change your verdict (only if comment or reject)
N/A
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:1032:tests:approvereview_round: 1 |
Contributor
Author
🤖 质量审查通过TL;DR
审查证据结论：approve。该 diff 是 test-only 变更，目标很窄：锁定 publish verification 的 quarantine 状态只通过 per-job
Reasoning discipline note：参考框架是 regression-test minimalism 和状态 marker 的 single source of truth。已知好形状是一条行为测试同时验证正向 per-job 状态和负向 legacy/global 状态不存在。候选方案：新增独立 root-marker 测试。Ugly defect: setup 重复，并把负向断言从 quarantine 转换点拆开。Beautiful form: 把负向断言放在既有转换序列的边界。采用方案：在既有测试中加入 before/after 断言。Ugly defect: 负向断言若命名不清会像偶然检查。Beautiful form: 用 📎 Full raw codex output (archival)---
pr: 1032
role: quality
head_sha: aad62da123f8d604c57ddc1efa34ca705bbf9f94
verdict: approve
---
## Verdict
Approve: the diff is a focused test-only guard for per-job quarantine state, with readable naming, no new abstraction, no dead code, no comments, and no unrelated cleanup.
## Evidence
- `skills/consensus-loop/scripts/test_publish_verification.py:385` keeps the change inside the existing failed-receipt retry/quarantine behavior test, so the added assertions do not create a separate single-purpose test or expand scope.
- `skills/consensus-loop/scripts/test_publish_verification.py:388` names the checked path `root_quarantine_marker`, which states the business invariant being locked: root-level quarantine marker absence.
- `skills/consensus-loop/scripts/test_publish_verification.py:392` and `skills/consensus-loop/scripts/test_publish_verification.py:412` assert the marker is absent before and after the retry/quarantine sequence, while the existing `statuses` assertion at lines 403-411 still proves per-job retry state reaches `QUARANTINED`.
- `skills/consensus-loop/scripts/codex_refactor_loop/publish_verification.py:345` writes retry/quarantine state through `job_dir / "retry.json"` only, which matches the test's single-source-of-truth shape.
- Fresh verification: `python3 -m unittest skills/consensus-loop/scripts/test_publish_verification.py` ran 17 tests and passed.
- Reasoning discipline note: reference frame is regression-test minimalism plus single source of truth for state markers. Known-good shape is one behavior test that verifies both the positive per-job state and the negative absence of legacy/global state. Candidate weighed: add a new standalone root-marker test. Ugly defect: duplicated setup and a special-case assertion detached from the quarantine transition. Beautiful form: keep the negative assertion beside the existing transition sequence. Candidate chosen: add local before/after assertions in the existing test. Ugly defect: a negative assertion can look incidental if it is not named clearly. Beautiful form: an intent-revealing local `root_quarantine_marker` checked at the transition boundaries. Verified-premise status: diff and full touched file were read; implementation path `record_job_retry` was read; PR head SHA matches `aad62da123f8d604c57ddc1efa34ca705bbf9f94`.
## What would change your verdict (only if comment or reject)
N/A
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:1032:quality:approvereview_round: 1 |
Contributor
Author
🤖 架构审查通过：quarantine 仍是 job-local 状态TL;DR
架构证据本次 diff 只改
验证命令： Reasoning-discipline note: reference frame 是 single source of truth + helper-private runtime side-effect boundary testing。已验证 diff 只触碰一个测试文件，production path 已经写 📎 Full raw codex output (archival)---
pr: 1032
role: architect
head_sha: aad62da123f8d604c57ddc1efa34ca705bbf9f94
verdict: approve
---
## Verdict
approve — no architecture compliance concerns; the diff only adds a behavior assertion that publish-verification quarantine stays job-local and does not create a repo-root marker.
## Evidence
- `skills/consensus-loop/scripts/test_publish_verification.py:388` defines `root_quarantine_marker = self.tmp / "QUARANTINED"`, and `skills/consensus-loop/scripts/test_publish_verification.py:391` plus `skills/consensus-loop/scripts/test_publish_verification.py:408` assert the marker is absent before and after the retry state reaches `QUARANTINED`.
- `skills/consensus-loop/scripts/codex_refactor_loop/publish_verification.py:347` writes retry/quarantine state to `job_dir / "retry.json"`, and `skills/consensus-loop/scripts/codex_refactor_loop/publish_verification.py:351` through `skills/consensus-loop/scripts/codex_refactor_loop/publish_verification.py:360` encode `QUARANTINED` in that per-job file, not in a repo-root marker.
- CLAUDE.md clause alignment: `CLAUDE.md:55` says "行为约束默认由 behavior test 或端到端可观察输入 / 输出 / 副作用验证"; this PR strengthens the existing behavior test with an observable absence-of-side-effect assertion.
- CLAUDE.md clause alignment: `CLAUDE.md:219` says "skill-private runtime directories such as `.refactor-loop/` must not become host production configuration or ledger SSOT"; the diff does not move host production facts into `.refactor-loop/` and instead guards against a repo-root quarantine marker as an alternate fact surface.
- CLAUDE.md clause alignment: `CLAUDE.md:46` says "同一约束禁止在多处平行声明"; this PR does not add a second implementation path or schema, and the checked behavior keeps publish verification state in the existing per-job retry receipt surface.
- Verification run: `python3 -m unittest skills/consensus-loop/scripts/test_publish_verification.py` passed locally with 17 tests.
- Reasoning-discipline note: Reference frame: single source of truth plus side-effect-boundary testing for helper-private runtime state. Known-good shape: one owner writes quarantine as a durable per-job receipt, and tests assert both the intended state and absence of stray repo-root state. Candidate approve: Ugly defect: an absence assertion can become brittle if it checks an unrelated global path; Beautiful form: assert the exact repo root from the fixture that represents the potential accidental marker location. Candidate comment/reject: Ugly defect: treating a narrow test-only guard as architectural drift would invent a rule not present in CLAUDE.md; Beautiful form: reserve reject for verified clause regression and advisory comments for real but non-blocking design debt. Verified premises: the merge-base diff touches only `skills/consensus-loop/scripts/test_publish_verification.py`; the production code path already writes `retry.json` under the job directory; no audit or implement summary env paths were provided; no `ASSUMED-UNVERIFIED` premise is needed for this verdict.review_round: 1 |
ElonSG
added
crnd:phase:merged
This was referenced Jun 21, 2026
Closed
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
2 participants
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.
Changed files
skills/consensus-loop/scripts/test_publish_verification.py: 在既有 failed receipt retry/quarantine 测试中加入 repo-rootQUARANTINED不存在断言,锁定 quarantine 只通过 per-jobretry.json表达。QUARANTINED: 实施前后均 absent,没有 untracked evidence file 需要删除。Test results
python3 -m unittest skills/consensus-loop/scripts/test_publish_verification.py: PASS, 17 tests.PATH="/opt/homebrew/bin:$PATH" /opt/homebrew/bin/python3 skills/consensus-loop/scripts/consensus-rnd-cli check-degradation --static: PASS.PATH="/opt/homebrew/bin:$PATH" /opt/homebrew/bin/python3 -m unittest discover -s skills/consensus-loop/scripts -p "test_*.py": PASS, 2562 tests, skipped=1.Deviations
.gitignore或 root guard。⟦AI:AUTO-LOOP⟧