structural(#371): 删公共 post-banner CLI 迁 ControllerActions.post_status_banner#379
Merged
Conversation
…_banner + #191 owner gate - 删 cli.py COMMANDS[post-banner] + banners.py public main/parse_args/post_status_banner 等 entry - 新增 controller-internal ControllerActions.post_status_banner(BannerRequest)->str:owner gate→invalid target→body tempfile→gh(self.gh),non-owner/缺context/invalid target fail-closed - 保留 banners.py build_status_banner/gh_comment_command/AUTO_LOOP_SENTINEL - SKILL.md GitHub state surface 改 ControllerActions.post_status_banner、spawn recipe 删 public CLI 示例 - 更新 test_cli_command_router/test_banner_package/test_controller_actions/test_runtime_exception_authorization_sources/test_skill_reference_anchors design-consensus r4 3/3 consensus:structural(issue #371) Closes #371. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
loning
added
crnd:lifecycle:managed
Contributor
Author
🤖 quality review: reject, missing refactor self-docTL;DR
详细说明当前代码本身不复杂: 删除公共 CLI,保留 banner renderer/helper,新增 controller-owned action。问题是仓库当前 具体位置是 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: quality
verdict: reject
---
## Verdict
Reject: the code shape is otherwise small and focused, but the new controller-owned banner write path is a major refactor and lacks the required self-contained Old/New refactor self-doc under the default `self-doc-comment` policy.
## Evidence
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147`: `post_status_banner(...)` is the new owner-gated replacement for the deleted public `consensus-rnd-cli post-banner` path, but the method is added without a `Refactor (...)` comment containing readable `Old pattern` and `New principle` blocks. Because `HOST_REFACTOR_COMMENT_POLICY` is empty and therefore normalizes to `self-doc-comment`, missing self-doc on this major refactor is a must-fix quality issue.
## What would change your verdict (only if comment or reject)
Add a concise self-doc comment immediately above `post_status_banner(...)`, for example documenting that the old public CLI `post-banner` exposed a worker-visible `gh-comment` write surface, and the new principle is to keep banner posting as a controller-internal `ControllerActions` primitive behind the #191 active-controller owner gate. Keep the comment self-contained; do not use a `see issue #371` placeholder.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:quality:reject⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests review: 缺少 post_status_banner 的 gh 失败分支测试TL;DR
详细说明
缺口在 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: tests
verdict: reject
---
## Verdict
Reject: the happy path, non-owner gate, invalid-target gate, CLI removal, and source-regression anchors are covered, but the new controller-owned banner posting method still has an untested `gh` failure branch.
## Evidence
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:172` adds a new branch where `post_status_banner()` raises `RuntimeError("post_status_banner: ...")` when `self.gh(..., check=False)` returns a non-zero status. The added tests in `skills/codex-refactor-loop/scripts/test_controller_actions.py:128`, `:160`, and `:179` cover successful posting, non-owner fail-closed before tempfile/gh, and invalid target fail-closed before tempfile/gh, but no test exercises the post-gate `gh` failure outcome or asserts the tempfile is still removed on that failure path.
- I ran the focused touched-test set against `origin/refactor/iter371-issue371`: `python3 -m unittest skills/codex-refactor-loop/scripts/test_controller_actions.py skills/codex-refactor-loop/scripts/test_banner_package.py skills/codex-refactor-loop/scripts/test_cli_command_router.py skills/codex-refactor-loop/scripts/test_runtime_exception_authorization_sources.py skills/codex-refactor-loop/scripts/test_skill_reference_anchors.py` passed (`Ran 158 tests`). This confirms the current suite is green, but the new failure branch remains uncovered.
## What would change your verdict
Add a behavior test for `ControllerActions.post_status_banner()` where active-controller ownership is allowed, target validation succeeds, `self.gh(...)` returns `returncode != 0`, and the test asserts the raised `RuntimeError` includes the `gh` stderr/stdout while the temporary body file is removed.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:tests:reject⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Architect review approve: post-banner 写面已收窄TL;DR
详细说明这次变更删除 public 从 CLAUDE.md compliance 角度看,它是收窄而不是扩权:没有新增 public lifecycle CLI,没有把 host production SSOT 放进 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: architect
verdict: approve
---
## Verdict
approve: no architectural concerns; the PR removes the public `post-banner` CLI write surface and moves banner posting into the controller-owned, active-controller-gated action without widening lifecycle authority.
## Evidence
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/cli.py:137` removes `post-banner` from `COMMANDS`, matching CLAUDE.md deletion-first: "废弃 skill、deprecated wrapper、`*.bak/*.old/*.deprecated` 直接删除,不保留兼容空壳" and avoiding a public `gh-comment` wrapper.
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147` adds `ControllerActions.post_status_banner`, and `controller_actions.py:148` gates it through `_require_owner_or_raise("post-banner")` before target validation, tempfile creation, or `self.gh(...)`, matching CLAUDE.md active-controller boundary: "跨设备时 GitHub/已 push git 面只允许一个 active-controller lease owner 执行 controller 写路径".
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:149` normalizes issue/PR targets before the GitHub comment call at `controller_actions.py:169`, matching the existing SKILL controller-action contract that lifecycle targets must become canonical positive decimals before side effects.
- `skills/codex-refactor-loop/authorizations/runtime-exceptions.md:169` and `skills/codex-refactor-loop/SKILL.md:320` clarify that observability comments still require #191 `ActiveControllerLease` / `require_active_controller(...)` for issue/PR target writes and are not a cross-device write permit, preserving CLAUDE.md narrow-allowlist/no-generic-lifecycle-actor constraints.
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/banners.py:38` keeps rendering only and `banners.py:66` keeps narrow command construction; there is no new `*WriteActor`, `*ReadActor`, or `*Store` split, no external repo reference, no host production SSOT moved into `.refactor-loop/`, and no compatibility shim introduced.
- Scope matches `.refactor-loop/runs/phase9-issue371-r4-judge.md`: the diff is limited to the declared `skills/codex-refactor-loop` banner/controller-action docs, implementation, and source-regression/behavior tests. No `SCOPE_EXTEND` was needed.
- `$HOST_REFACTOR_COMMENT_POLICY` is missing/empty in host.env and the PR branch documents the default as `none` at `skills/codex-refactor-loop/SKILL.md:115` and `SKILL.md:128`; absence of new Old/New source comments is compliant, and no new refactor-history source comments were added in the changed Python files.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:architect:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 fix-codex round 1: 已补测试覆盖,质量评论按策略判 false-positiveTL;DR
详细说明已在 未按 quality 评论添加 验证: 📎 完整 codex 原始输出(存档备查)# Fix report for PR 379 round 1
## Applied
- (A) skills/codex-refactor-loop/scripts/test_controller_actions.py:160: added `test_post_status_banner_gh_failure_reports_output_and_removes_tempfile`, covering active-controller owner allowed, valid target, `self.gh(..., check=False)` nonzero via stderr and stdout fallback, raised `RuntimeError` content, and temporary body file cleanup (addresses reviewer:tests evidence #1).
## Rejected as false positive
- skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147 cited by reviewer:quality: rejected demand to add `Refactor (...)` / `Old pattern` / `New principle` source comment. Evidence: checked-in `skills/codex-refactor-loop/SKILL.md:115` says missing/empty/default `$HOST_REFACTOR_COMMENT_POLICY` normalizes to `none`; `skills/codex-refactor-loop/SKILL.md:128` says `none` rejects Old/New refactor-history source comments and keeps rationale in external artifacts; `skills/codex-refactor-loop/prompts/review-fix.md:47` says missing/empty/default/`none` must not add those comments and reviewer demands under `none` are host-policy false positives. Architect review independently cited the same policy evidence and approved absence of source comments.
## Blocked (cannot fix this round)
- none
## Build status
- build: pass (`bash -lc "true"`)
- tests: pass (`python3 -m unittest skills/codex-refactor-loop/scripts/test_controller_actions.py` ran 56 tests; `python3 -m unittest discover -s skills/codex-refactor-loop/scripts -p 'test_*.py'` ran 979 tests, 1 skipped)
## Recommendation for next round
- expect merge-gate to clear: tests reject has a focused behavior test now; quality reject is recorded as a host-policy false positive under the branch's checked-in comment policy.
⟦AI:AUTO-LOOP⟧
FIX_DONE:379:round-1:applied-1:rejected-1:blocked-0⟦AI:AUTO-LOOP⟧ |
tests reviewer reject(applied):新增 test_controller_actions 用例 —— owner allowed + target valid + self.gh returncode≠0 → RuntimeError 含 stderr,且 body tempfile 仍删除。 quality reviewer self-doc 注释需求(rejected):加 Refactor-history 源注释会与 test_source_language_policy scanner 冲突(见 #373 同类 finding),full $TEST_CMD 979 OK 证明现状 test-safe;rationale 提交 re-review 裁决。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
🤖 tests reviewer r2：批准TL;DR
详细说明我按三点 diff 查看了实际改动和 touched test files。新增覆盖不是 bump 行数：成功路径断言了 我也检查了测试质量禁项：没有新增 sleep/delay pacing、没有新增 skip/manual trait、没有发现把强断言改弱的模式。针对 touched test modules 的本地验证通过：159 tests OK。 📎 完整 codex 原始输出(存档备查)pr: 379
|
Contributor
Author
🤖 Architect review r2: 通过TL;DR
详细说明我按三点 diff 复核了
📎 完整 codex 原始输出(存档备查)pr: 379
|
Contributor
Author
🤖 quality review: reject, 需补 self-doc Old/NewTL;DR
详细说明
问题只在 refactor self-doc policy:本 reviewer prompt 指定 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: quality
verdict: reject
---
## Verdict
Reject: the code is otherwise focused and readable, but this is a controller-surface refactor under `self-doc-comment` policy and the new key method lacks the required readable Old/New self-doc block.
## Evidence
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147` adds `ControllerActions.post_status_banner(...)`, the new owner-gated replacement for the deleted public `consensus-rnd-cli post-banner` path, but there is no adjacent `Refactor (...)` self-doc comment with Old pattern / New principle. Under the prompt's `self-doc-comment` policy, a major refactor that moves a public GitHub-write surface into a controller action needs that source-level rationale, not just the PR body.
- No naming, dead-code, over-engineering, under-engineering, method-size, filler-comment, or drive-by-change finding found in the touched code. `post_status_banner` is a business-intent name, the method is small, and the retained banner helpers have direct tests/callers.
## What would change your verdict (only if comment or reject)
Add a short source refactor self-doc block immediately above `ControllerActions.post_status_banner(...)` or at the moved banner/controller boundary, with a complete Old/New explanation, for example: old public `consensus-rnd-cli post-banner` could write issue/PR comments without the active-controller owner gate; new controller-internal action runs owner gate, target normalization, tempfile body write, and `self.gh(...)` posting in order.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:quality:reject⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 fix-codex round 2: 已补 self-doc 并通过验证TL;DR
详细说明本轮阻塞项只有 quality reject: 验证中发现两个测试守卫需要跟随这个策略: 📎 完整 fix artifact# Fix report for PR 379 round 2
## Applied
- (A) skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147: added the required English `Refactor (iter371/issue-371)` self-doc block above `ControllerActions.post_status_banner(...)`, documenting the old public `consensus-rnd-cli post-banner` write path and the new owner-gated controller-internal sequence (addresses reviewer:quality evidence #1 and controller override).
- (B) skills/codex-refactor-loop/scripts/test_controller_actions.py:1159: scope-extend reason: the required self-doc block made an existing source-regression test overbroad; narrowed the issue-300 Old/New absence assertion to the merge contract slice it owns.
- (B) skills/codex-refactor-loop/scripts/test_source_language_policy.py:248: scope-extend reason: full validation under `HOST_REFACTOR_COMMENT_POLICY=self-doc-comment` exposed that the negative scanner fixture depended on ambient policy; pinned that fixture to `none` so fail-closed coverage remains explicit.
## Rejected as false positive
- none
## Blocked (cannot fix this round)
- none
## Build status
- build: pass (`bash -lc "true"`)
- tests: pass (`python3 -m unittest skills/codex-refactor-loop/scripts/test_controller_actions.py` ran 56 tests OK; focused touched set ran 159 tests OK; `HOST_REFACTOR_COMMENT_POLICY=self-doc-comment python3 -m unittest discover -s skills/codex-refactor-loop/scripts -p 'test_*.py'` ran 979 tests OK, 1 skipped)
## Recommendation for next round
- expect merge-gate to clear: architect and tests were already approve in round 2, and the only quality reject demand is now applied with policy-matched full-suite verification.
⟦AI:AUTO-LOOP⟧
FIX_DONE:379:round-2:applied-3:rejected-0:blocked-0⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 fix-codex round 3: 已删除违反 none 策略的源码注释TL;DR
详细说明本轮阻塞项来自 controller override: 验证通过: 📎 完整 fix artifact# Fix report for PR 379 round 3
## Applied
- (A) skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147: removed the round-2 `Refactor (iter371/issue-371)` / `Old pattern` / `New principle` source comment block above `ControllerActions.post_status_banner(...)` so the branch complies with `HOST_REFACTOR_COMMENT_POLICY=none` and `test_source_language_policy.test_scan_python_source_language_is_clean`.
## Rejected as false positive
- none
## Blocked (cannot fix this round)
- none
## Build status
- build: pass (`bash -lc "true"`)
- tests: pass (`python3 -m unittest discover -s skills/codex-refactor-loop/scripts -p 'test_*.py'` ran 979 tests in 295.596s, OK, skipped=1)
## Recommendation for next round
- expect merge-gate to clear: the round-1 `post_status_banner` gh-failure behavior test remains present, the round-2 source-regression fixture adjustments remain present, and the only round-3 blocker has been removed under the corrected `none` source-comment policy.
⟦AI:AUTO-LOOP⟧
FIX_DONE:379:round-3:applied-1:rejected-0:blocked-0⟦AI:AUTO-LOOP⟧ |
HOST_REFACTOR_COMMENT_POLICY=none 时本仓库 scanner 禁止新增 refactor-history 源注释。调整 test_controller_actions.py 迁移断言与 test_source_language_policy.py 期望,使其不再要求 post_status_banner 处的 # Refactor 自文档注释;rationale 留在 PR body/commit。979 tests OK (skipped=1) 全套自验通过。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
📊 当前状态 — Consensus-rnd Phase review-gate r4(❌ 不需要人介入)
说明:fix r3 移除了 r2 误加的 refactor-history 源注释期望(本仓库 scanner 在 policy=none 下禁止新增此类注释),rationale 留在 PR body/commit。r4 reviewer 在 policy=none 下审查:新 refactor 源注释缺失即合规,reviewer 不得 demand。 下一步自动会做:3 reviewer 完成 → 真值表(reject=0 → MERGE)→ merge 到 auto-refact-dev(Closes #371)。 🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Quality review: approveTL;DR
详细说明我按三点看: public surface 是否收窄、迁移后的 controller action 是否可读、是否引入死代码或多余抽象。当前 diff 没有发现质量阻塞点。 关键证据: 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: quality
verdict: approve
---
## Verdict
Approve: the PR removes the public banner CLI cleanly, keeps the remaining banner helper surface narrow, and moves posting into a readable controller-owned action without new dead code or unnecessary abstraction.
## Evidence
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/cli.py:48` keeps public command routing in the existing `COMMANDS` table and no longer exposes `post-banner`, so the change does not add a parallel routing mechanism.
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/banners.py:25` leaves `BannerRequest`, `build_status_banner`, and `gh_comment_command` as focused rendering/command-building helpers after deleting the argparse/env/runner posting path.
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147` adds `ControllerActions.post_status_banner` as a compact controller action: owner gate, target normalization, banner rendering, tempfile cleanup, one `gh` comment call.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:128` covers the new public controller surface through owner success, GitHub failure cleanup, non-owner blocking, and invalid target blocking, so the new method is reachable and not speculative.
- `skills/codex-refactor-loop/scripts/test_cli_command_router.py:326` and `skills/codex-refactor-loop/scripts/test_banner_package.py:118` lock the removed public CLI entrypoint and forbid the old posting/environment fallback from creeping back in.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:quality:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests review: approveTL;DR
详细说明我看了 覆盖是够的: 我也跑了相关 unittest 文件, 结果是 164 tests OK。diff 扫描没有看到 PR 新增 sleep/delay pacing、 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: tests
verdict: approve
---
## Verdict
Approve: the PR adds focused behavior and source-regression coverage for moving banner posting from the public CLI into controller-owned actions, with no test-quality blockers found.
## Evidence
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:128` covers the new successful `ControllerActions.post_status_banner` path: active-controller gate, `gh pr comment` command shape, sentinel-bearing body, returned URL, temp-file cleanup, and active-controller status write.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:160` covers `gh` failure handling for both stderr and stdout messages while still asserting the temporary body file is removed.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:195` covers the non-owner path and asserts no tempfile or GitHub call is attempted.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:214` covers invalid GitHub targets and asserts fail-closed behavior before tempfile or GitHub side effects, including the pending-event audit marker.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:1104`, `skills/codex-refactor-loop/scripts/test_banner_package.py:118`, `skills/codex-refactor-loop/scripts/test_cli_command_router.py:326`, `skills/codex-refactor-loop/scripts/test_runtime_exception_authorization_sources.py:702`, and `skills/codex-refactor-loop/scripts/test_skill_reference_anchors.py:588` provide source-regression coverage for the no-public-`post-banner` rule, owner gate, controller action surface, and documentation/authorization anchors.
- Diff scan found no PR-added `sleep` / delay pacing, `[Skip]`, manual-test markers, `Assert.True(true)`-style bump tests, or obvious assertion loosening.
- Verification run from the PR worktree passed: `python3 -m unittest skills/codex-refactor-loop/scripts/test_banner_package.py skills/codex-refactor-loop/scripts/test_cli_command_router.py skills/codex-refactor-loop/scripts/test_controller_actions.py skills/codex-refactor-loop/scripts/test_runtime_exception_authorization_sources.py skills/codex-refactor-loop/scripts/test_skill_reference_anchors.py skills/codex-refactor-loop/scripts/test_source_language_policy.py` -> 164 tests OK.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:tests:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 architect review r4: approveTL;DR
详细说明PR 删除了 public 这符合 CLAUDE.md 的窄授权边界:issue/PR target 写副作用只能来自 #191 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: architect
verdict: approve
---
## Verdict
approve - no architecture compliance concerns; the PR removes the public banner write surface and moves banner posting behind the controller-owned #191 active-controller gate.
## Evidence
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/cli.py:48` keeps the public command catalog without `post-banner`, matching CLAUDE.md's "强类型边界,窄扩展点" and "删除优先" clauses.
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/banners.py:38` and `:66` retain only rendering plus narrow `gh` command construction; the deleted argparse/main/env fallback path does not leave a compat shim or parallel public pathway.
- `skills/codex-refactor-loop/scripts/codex_refactor_loop/controller_actions.py:147` gates `ControllerActions.post_status_banner` with `self._require_owner_or_raise("post-banner")` before target normalization, tempfile creation, and `self.gh(...)`, satisfying the CLAUDE.md clause: "issue/PR target 写副作用的跨设备 permit 只来自 #191 ActiveControllerLease".
- `skills/codex-refactor-loop/authorizations/runtime-exceptions.md:230` narrows `observability-comment-writers-53` by stating issue/PR target writes still require `ActiveControllerLease` / `require_active_controller(...)`, so #53 is not widened into a cross-device write permit.
- `skills/codex-refactor-loop/SKILL.md:21`, `:326`, `:811`, and `:1490` update the skill contract to name `ControllerActions.post_status_banner`, preserve GitHub-state obligations, and document the owner-gated call order.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:147`, `:195`, and `:214` cover owner success, non-owner no-gh/no-tempfile behavior, and invalid target blocking; `test_cli_command_router.py:326` locks removal of public `post-banner`; `test_runtime_exception_authorization_sources.py:702` source-regresses the boundary. This satisfies CLAUDE.md's "变更必须可验证" / behavior + source-regression requirement.
- Scope is honest: changed paths stay under `skills/codex-refactor-loop/`, and the only extra authorization mirror file is documented by `SCOPE_EXTEND:skills/codex-refactor-loop/authorizations/runtime-exceptions.md` in `.refactor-loop/runs/implement-issue-371.md`.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:architect:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 当前状态 — review-gate r4 共识达成,解冲突中(❌ 不需要人介入)
下一步自动会做:resolver 解冲突+stage → controller commit merge + push → 新 head 重派 r5 reviewer(head 变即重审)→ reject=0 + CLEAN → merge 到 auto-refact-dev(Closes #371)。 🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Closed
…ter371-issue371 # Conflicts: # skills/codex-refactor-loop/scripts/test_controller_actions.py
Contributor
Author
📊 当前状态 — review-gate r5(❌ 不需要人介入)
下一步自动会做:3 reviewer 完成 → 真值表(reject=0 → MERGE)→ merge 到 auto-refact-dev(Closes #371)。 🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests 评审通过TL;DR
详细说明我直接看了 PR diff 和实际测试文件。新增测试不是只涨行数:覆盖了 active-controller owner gate、非法 target fail-closed、 未看到新增 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: tests
verdict: approve
---
## Verdict
Approve: the PR adds behavior coverage for the moved banner posting path, preserves source-regression coverage for the removed public CLI surface, and does not add skip/sleep/polling or weakened assertions.
## Evidence
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:139` exercises the new `ControllerActions.post_status_banner` owner path end to end: active-controller gate, `gh pr comment` args, body sentinel, temp-file cleanup, returned URL, and active-controller status artifact.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:171` covers `gh` failure behavior with both stderr and stdout fallback, including temp-file cleanup after failure.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:206` and `:225` cover fail-closed paths before side effects: non-owner blocks before tempfile/`gh`, and invalid targets emit the pending-event audit without creating a tempfile or calling `gh`.
- `skills/codex-refactor-loop/scripts/test_cli_command_router.py:326`, `test_banner_package.py:118`, `test_runtime_exception_authorization_sources.py:702`, and `test_skill_reference_anchors.py:588` provide source-regression coverage that `post-banner` is no longer a public CLI surface and that the controller-owned banner action is documented and owner-gated.
- I found no added sleeps/delays, no added `[Skip]`/manual category bypass, and no assertion weakening in the PR diff. Targeted verification passed: `python3 -m unittest skills/codex-refactor-loop/scripts/test_controller_actions.py skills/codex-refactor-loop/scripts/test_banner_package.py skills/codex-refactor-loop/scripts/test_cli_command_router.py skills/codex-refactor-loop/scripts/test_runtime_exception_authorization_sources.py skills/codex-refactor-loop/scripts/test_skill_reference_anchors.py skills/codex-refactor-loop/scripts/test_source_language_policy.py` ran 165 tests successfully in a detached PR worktree.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:tests:approve⟦AI:AUTO-LOOP⟧ |
1 similar comment
Contributor
Author
🤖 tests 评审通过TL;DR
详细说明我直接看了 PR diff 和实际测试文件。新增测试不是只涨行数:覆盖了 active-controller owner gate、非法 target fail-closed、 未看到新增 📎 完整 codex 原始输出(存档备查)---
pr: 379
role: tests
verdict: approve
---
## Verdict
Approve: the PR adds behavior coverage for the moved banner posting path, preserves source-regression coverage for the removed public CLI surface, and does not add skip/sleep/polling or weakened assertions.
## Evidence
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:139` exercises the new `ControllerActions.post_status_banner` owner path end to end: active-controller gate, `gh pr comment` args, body sentinel, temp-file cleanup, returned URL, and active-controller status artifact.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:171` covers `gh` failure behavior with both stderr and stdout fallback, including temp-file cleanup after failure.
- `skills/codex-refactor-loop/scripts/test_controller_actions.py:206` and `:225` cover fail-closed paths before side effects: non-owner blocks before tempfile/`gh`, and invalid targets emit the pending-event audit without creating a tempfile or calling `gh`.
- `skills/codex-refactor-loop/scripts/test_cli_command_router.py:326`, `test_banner_package.py:118`, `test_runtime_exception_authorization_sources.py:702`, and `test_skill_reference_anchors.py:588` provide source-regression coverage that `post-banner` is no longer a public CLI surface and that the controller-owned banner action is documented and owner-gated.
- I found no added sleeps/delays, no added `[Skip]`/manual category bypass, and no assertion weakening in the PR diff. Targeted verification passed: `python3 -m unittest skills/codex-refactor-loop/scripts/test_controller_actions.py skills/codex-refactor-loop/scripts/test_banner_package.py skills/codex-refactor-loop/scripts/test_cli_command_router.py skills/codex-refactor-loop/scripts/test_runtime_exception_authorization_sources.py skills/codex-refactor-loop/scripts/test_skill_reference_anchors.py skills/codex-refactor-loop/scripts/test_source_language_policy.py` ran 165 tests successfully in a detached PR worktree.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:379:tests:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Quality review r5: approveTL;DR
详细说明我按三点看了 PR head 的完整 touched files: 关键证据: 📎 完整 codex 原始输出(存档备查)pr: 379
|
Contributor
Author
🤖 architect review: comment-only，scope 描述漏列 1 个测试文件TL;DR
详细说明架构主体没有发现阻塞项： 唯一需要 surfacing 的是 scope honesty：PR body 写“5 个 test 文件”，但实际 diff 还改了 📎 完整 codex 原始输出(存档备查)pr: 379
|
Contributor
Author
📊 review-gate 共识 — MERGE_WITH_COMMENTS,合并(❌ 不需要人介入)
Surfaced comment(architect,advisory 不阻塞):PR body 的 scope 行漏列 动作:即刻 ready + squash-merge 到 auto-refact-dev,Closes #371。 🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
This was referenced Jun 1, 2026
Merged
loning
added a commit
that referenced
this pull request
Jun 1, 2026
* fix(#370): closed_label_reconciler 收窄回 phase-only terminal reconciliation (#375) - verify_plan 改 phase-only postcondition(去 exactly-one-human 要求),不再越权 - 单 item apply/verify try/except skip+continue 隔离,单坏 item 不崩整个 daemon loop - run_once 接 beat callback,collect 后及每 item 后续 heartbeat,长 tick 不冻 - SKILL.md release fresh_heartbeats 文档追平实现(every restart-managed daemon fresh within 90s) - 补 4 项 behavior test + release schema source-regression design-consensus r1 3/3 consensus:minimal(issue #370) Closes #370. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#374): 删除 tracked .claude/skills symlink,改为 ignored local setup (#377) - git rm .claude/skills(移除第二 Claude 平台入口,根 .claude-plugin manifest 为唯一入口) - .gitignore 忽略 .claude/skills(本机 setup 不发布) - README 补本地可手动 ln -s ../skills .claude/skills 说明 - test_source_publication_boundary.py 反转断言:git check-ignore .claude/skills 成功 + ls-files 不含 design-consensus r1 3/3 consensus:minimal(issue #374) Closes #374. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#372): wakeup-plan bootstrap host-env 检查下沉 LoopContext,去硬编码 .refactor-loop/host.env (#378) - 删 wakeup_plan.py 内硬编码 .refactor-loop/host.env 存在性检查 - pending_bootstrap_actions 经 LoopContext/host_env_available 判定;无 host env 时 reason 指向 host-owned 注入文件 - 保留 legacy .refactor-loop/host.env 仅 compatibility read - 补 behavior test:CONSENSUS_RND_HOST_ENV 存在 + 无 legacy 时不 emit bootstrap-missing design-consensus r1 3/3 consensus:delete(issue #372) Closes #372. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * #373: branch-topology required-env(删分支硬编码默认) (#381) * #373: branch-topology required-env(删 INTEGRATION_BRANCH/REVIEW_BASE_BRANCH 硬编码默认) r4 consensus:minimal(option A):sync/dev.py、sync/executor.py、controller_actions.py、 release/commits.py 删 auto-refact-dev/dev/origin-dev 硬编码 fallback;SKILL.md host env matrix INTEGRATION_BRANCH/REVIEW_BASE_BRANCH defaulted→required 语义;host.env.example 更新; +5 test(含 release-commits/sync fail-closed-without-branch-env behavior test)。 full $TEST_CMD 981 OK(skipped=1)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#381 review r1): 删 sync/dev.py if-False 死代码,断言改真 branch-topology behavior check quality reviewer reject(applied):#373 implement 为满足 source-regression 塞的 if False: + 裸表达式 Refactor(iter373/issue-373) 是不可达死代码;删除,test_sync_dev source-regression 断言改为锁真实合同(无 DEFAULT_INTEGRATION_BRANCH/DEFAULT_REVIEW_BASE_BRANCH 硬编码默认、 存在 missing required host branch env fail-closed、无 legacy alias 读)。 architect/tests=comment(advisory)。full $TEST_CMD 982 OK(skipped=1)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * structural(#371): 删公共 post-banner CLI 迁 ControllerActions.post_status_banner (#379) * structural(#371): 删公共 post-banner CLI,迁 ControllerActions.post_status_banner + #191 owner gate - 删 cli.py COMMANDS[post-banner] + banners.py public main/parse_args/post_status_banner 等 entry - 新增 controller-internal ControllerActions.post_status_banner(BannerRequest)->str:owner gate→invalid target→body tempfile→gh(self.gh),non-owner/缺context/invalid target fail-closed - 保留 banners.py build_status_banner/gh_comment_command/AUTO_LOOP_SENTINEL - SKILL.md GitHub state surface 改 ControllerActions.post_status_banner、spawn recipe 删 public CLI 示例 - 更新 test_cli_command_router/test_banner_package/test_controller_actions/test_runtime_exception_authorization_sources/test_skill_reference_anchors design-consensus r4 3/3 consensus:structural(issue #371) Closes #371. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#379 review r1): 补 post_status_banner gh-failure 分支 behavior test tests reviewer reject(applied):新增 test_controller_actions 用例 —— owner allowed + target valid + self.gh returncode≠0 → RuntimeError 含 stderr,且 body tempfile 仍删除。 quality reviewer self-doc 注释需求(rejected):加 Refactor-history 源注释会与 test_source_language_policy scanner 冲突(见 #373 同类 finding),full $TEST_CMD 979 OK 证明现状 test-safe;rationale 提交 re-review 裁决。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#379 review r3): policy=none 下对齐 source-language scanner 的注释期望 HOST_REFACTOR_COMMENT_POLICY=none 时本仓库 scanner 禁止新增 refactor-history 源注释。调整 test_controller_actions.py 迁移断言与 test_source_language_policy.py 期望,使其不再要求 post_status_banner 处的 # Refactor 自文档注释;rationale 留在 PR body/commit。979 tests OK (skipped=1) 全套自验通过。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: 删除 2 个违反 HOST_REFACTOR_COMMENTPOLICY=none 的 refactor-history 源注释 review-gate architect(PR #382 rollup)发现 test_source_publication_boundary.py (iter374)与 test_wakeup_plan.py(iter372)各有一个 net-new `# Refactor` Old/New 源注释块,违反本 session 已生效的 HOST_REFACTOR_COMMENT_POLICY=none(test-file scanner scope gap 未机械捕获,architect 按 policy 语义抓到)。仅删这两块注释, rationale 保留在各自 PR body/commit;不改任何 test 逻辑。全套 990 tests OK。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary / 摘要
公共
consensus-rnd-cli post-banner命令暴露gh-comment能力,可在无 #191 active-controller owner gate 下直接写 issue/PR 评论,绕过单写者边界。cli.py COMMANDS["post-banner"]+banners.pypublicmain/parse_args/post_status_banner直接发 banner,无 owner gate。ControllerActions.post_status_banner(BannerRequest)->str,顺序 owner gate → invalid target → body tempfile → gh(self.gh),non-owner / 缺 context / invalid target 均在 tempfile+gh 副作用前 fail-closed。保留build_status_banner/gh_comment_command/AUTO_LOOP_SENTINEL。违反:#191 active-controller lease 单写者边界(banner 发布须经 owner gate)。
Scope / 范围
cli.py、banners.py、controller_actions.py、SKILL.md(GitHub state surface + spawn recipe)、5 个 test 文件(cli-router/banner-package/controller-actions/runtime-exception-sources/skill-reference-anchors)。本地全量:
Ran 978 tests ... OK (skipped=1)。共识来源
design-consensus r4 3/3 consensus:structural(issue #371;r1→r4 binary fork + reflector re-design 后收敛)。
Closes #371.
🤖 Auto-loop / codex-refactor-loop
⟦AI:AUTO-LOOP⟧