Skip to content

skill: floor 强制澄清(monitor no-gap-only,#14 共识)#18

Merged
loning merged 1 commit into
auto-refact-devfrom
refactor/iter3-skill-concurrency-floor-enforcement
May 25, 2026
Merged

skill: floor 强制澄清(monitor no-gap-only,#14 共识)#18
loning merged 1 commit into
auto-refact-devfrom
refactor/iter3-skill-concurrency-floor-enforcement

Conversation

@loning
Copy link
Copy Markdown
Contributor

@loning loning commented May 25, 2026

实现 #14(self-audit #11)共识(delete):concurrency_monitor 保持 no-gap-only,删 low-threshold/MIN_PARALLEL 死路径;CODEX_FLOOR 补给职责归 controller wakeup step 1.5;SKILL「Concurrency floor」节澄清。验证了 controller 现有 floor 做法。

🤖 Auto-loop · #14

⟦AI:AUTO-LOOP⟧

Phase 9 consensus(delete):删 concurrency_monitor 误导性 low-threshold 路径;
CODEX_FLOOR 补给仅 controller wakeup step 1.5;SKILL 澄清职责。

⟦AI:AUTO-LOOP⟧
@loning loning added auto-loop codex-refactor-loop 管理的 issue/PR 👀 phase:reviewing labels May 25, 2026
@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 25, 2026

🤖 quality review approve

TL;DR

  • 这是什么:PR skill: floor 强制澄清(monitor no-gap-only,#14 共识) #18 的 code quality 独立 review。
  • 结论:approve,没有发现命名、死代码、复杂度、过度抽象或跑偏改动问题。
  • 需要 maintainer 做什么:质量角度无需改动,可继续等其他 reviewer / controller 汇总。

详细说明

这次改动把 concurrency_monitor.py 的职责压回 no-gap sentinel:删掉 low_streak / threshold 分支,没有新增替代抽象,tick() 读起来更短也更直接。SKILL.md 里把 floor 补给唯一归到 controller wakeup step 1.5,和 runtime 脚本边界一致。

自文档注释也到位:skills/codex-refactor-loop/scripts/concurrency_monitor.py:211skills/codex-refactor-loop/SKILL.md:1952 都有 Refactor (iter3/skill-concurrency-floor-enforcement) Old/New 块,不是只写 issue 链接。新增的 ConcurrencyFloorSourceRegressionTests 只 pin 住已删除路径和 controller ownership,没有引入 helper 框架或重复 setup。


📎 完整 codex 原始输出(存档备查)

pr: 18
role: quality
verdict: approve

Verdict

Approve: the PR removes the stale low-threshold path, keeps the monitor no-gap-only, and the added regression tests/readable self-doc comments stay focused on the #14 consensus.

Evidence

  • skills/codex-refactor-loop/scripts/concurrency_monitor.py:210 keeps tick() short and direct after deletion of low_streak / threshold handling; no new abstraction or single-implementer interface was introduced.
  • skills/codex-refactor-loop/scripts/concurrency_monitor.py:211 has the required Refactor (iter3/skill-concurrency-floor-enforcement) Old/New self-doc, and it is understandable without relying on only "see issue [refactor-design] concurrency_monitor 未强制 CODEX_FLOOR 硬下限(self-audit #11) #14".
  • skills/codex-refactor-loop/SKILL.md:1952 has the matching refactor self-doc for the doc behavior change, while skills/codex-refactor-loop/SKILL.md:1961 states the business boundary clearly: controller wakeup step 1.5 owns floor refill, monitor stays no-gap-only.
  • skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:478 adds one focused source-regression test class with two tests; the assertions are specific to deleted dead paths and controller ownership, with no broad helper framework or duplicated setup.
  • The diff is scoped to the skill doc, concurrency_monitor.py, and the source-regression test; I did not find unrelated naming churn, drive-by cleanup, or new unreachable private/public surface.

What would change your verdict (only if comment or reject)

N/A.

REVIEW_DONE:18:quality:approve
⟦AI:AUTO-LOOP⟧

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 25, 2026

🤖 测试评审通过: no-gap 与 floor 归属已有回归覆盖

TL;DR


详细说明

我检查了三点: PR 三点 diff、实际测试文件、以及 host 当前没有配置 polling allowlist / guard 文件的情况。这里没有新增 或 生产文件,所以 映射不适用。

新增的 会同时检查正向业务结果和反向回归 token: 、 必须存在,、、、、 这些旧路径不能回来。新增的 则固定 controller wakeup step 1.5 是 floor 补给唯一位置,并检查 前执行、硬下限表达式、删除 旧脚本、只保留一个 canonical step-1.5 脚本块。

我也跑了 PR head worktree 里的测试: , 30 tests OK。diff scan 没有发现新增 测试节奏、、manual category、polling allowlist 变更、弱化断言或 mock-only pseudo-coverage。


📎 完整 codex 原始输出(存档备查)
---
pr: 18
role: tests
verdict: approve
---

## Verdict
Approve: the PR adds focused source-regression coverage for the no-gap-only monitor contract and controller-owned floor refill wording, with no sleep pacing, skips, weakened assertions, or mock-only pseudo-coverage.

## Evidence
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:482` adds `test_concurrency_monitor_is_no_gap_only`, which asserts the intended monitor contract (`no-gap-violation`, `expected > 0 and actual == 0`) and blocks stale low/floor paths (`MIN_PARALLEL`, `codex-concurrency-low`, `low_streak`, `actual < threshold`, `CODEX_FLOOR`) from returning.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:499` adds `test_skill_assigns_floor_to_controller_step_1_5_only`, which asserts the documented business rule that controller wakeup step 1.5 owns floor refill before `ScheduleWakeup`, keeps the hard floor expression, removes the stale `ACTIVE <= 2` duplicate script, and leaves only one canonical step-1.5 script block.
- Diff scan found no added `sleep` / `delay` test pacing, no polling allowlist change, no `[Skip]`, no `[Trait("Category","Manual")]`, no bump-only assertion, no loosened existing assertion, and no mock-only pseudo-coverage.
- No `src/` or `agents/` production files are touched, so the repository's `test/.../<TypeName>Tests.cs` mapping requirement does not apply to this PR.
- Verified the PR-head test file in the branch worktree: `cd .refactor-loop/worktrees/skill-concurrency-floor-enforcement/skills/codex-refactor-loop/scripts && python3 -m unittest test_ensure_project_rules_fixed_points.py` passed with 30 tests.

REVIEW_DONE:18:tests:approve

⟦AI:AUTO-LOOP

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 25, 2026

🤖 架构审查: comment, SKILL 计数契约还差一行同步

TL;DR

  • 这是什么:PR skill: floor 强制澄清(monitor no-gap-only,#14 共识) #18 的 architect 角度审查,只看 CLAUDE/AGENTS 架构约束。
  • 结论是什么:不阻塞合并,但 SKILL.md 里 monitor 的 actual 计数说明还停在旧规则。
  • 下一步:把那一行改成按 spawn-codex.sh + 绝对 $REPO_ROOT 计数,并排除 -c wrapper;最好加测试锁住这句。

详细说明

这轮 diff 范围是干净的:只动了 skills/codex-refactor-loop/SKILL.mdscripts/concurrency_monitor.pyscripts/test_ensure_project_rules_fixed_points.pySCOPE_EXTEND 覆盖了新增回归测试文件,Old/New refactor 注释也在 floor 文档、monitor 代码和测试类上。

唯一需要评论的是 skills/codex-refactor-loop/SKILL.md:230:它还写 monitor 的 actual 通过 .refactor-loop/logs/.refactor-loop/prompts/ 识别。现在真实代码已经在 scripts/concurrency_monitor.py:144-153 按绝对 REPO_ROOT scope,并排除 -c wrapper。这里不一致会让 skill 契约误导后续 agent,但不构成 reject,因为这次修改本身属于 Phase 9 允许的 host-agnostic policy 注入修正,且已有配套行为测试。

建议补一刀:把 SKILL.md:230 改成“命令行包含 spawn-codex.sh 和绝对 $REPO_ROOT,且不包含 -c wrapper”,再让新增 source-regression test 断言这句,防止 stale logs/prompts 规则回流。


📎 完整 codex 原始输出(存档备查)
---
pr: 18
role: architect
verdict: comment
---

## Verdict
comment-only: the PR is architecturally scoped and mostly matches the Phase 9 decision, but one changed SKILL contract line still documents the old monitor counting rule.

## Evidence
- `skills/codex-refactor-loop/SKILL.md:230` still says monitor `actual` is counted when the command line contains `.refactor-loop/logs/` or `.refactor-loop/prompts/`. The Phase 9 source plan required this section to state the current true rule: "command line contains `spawn-codex.sh` and absolute `$REPO_ROOT`, excluding ` -c ` wrappers." This matters because the live code in `skills/codex-refactor-loop/scripts/concurrency_monitor.py:144-153` now scopes by absolute `REPO_ROOT` and excludes ` -c ` wrappers, so the public skill contract remains stale even though the implementation is corrected. This is a comment, not a reject, because the AGENTS/CLAUDE exception allows this host-agnostic policy fix: "例外:经 Phase 9 deep consensus 明确授权的 host-agnostic bootstrap / policy 注入修正可先落地,但不得引入具体 host 事实,且必须配套行为测试。"
- Scope honesty checks pass: the diff is limited to `skills/codex-refactor-loop/SKILL.md`, `skills/codex-refactor-loop/scripts/concurrency_monitor.py`, and `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py`; the implement summary documents `SCOPE_EXTEND` for the added regression test file.
- Old/New comments are present on the refactored floor section and changed code/test concepts: `skills/codex-refactor-loop/SKILL.md:1952`, `skills/codex-refactor-loop/scripts/concurrency_monitor.py:210`, and `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:478`.
- Anti-pattern grep on added lines found no live additions of actor dispatch bypasses, host `SubscribeAsync<EventEnvelope>`, disabled tests, raw `HttpClient`, production sleep/delay, cross-actor `Dictionary<,>`, or new external repo references. The only matched added strings are forbidden-token assertions in tests and explanatory floor wording.
- No `.proto` files are touched; no version manifests are touched; no compatibility shim, new actor split, or parallel pathway is introduced.

## What would change your verdict
Update `skills/codex-refactor-loop/SKILL.md:230` so the monitor `actual` rule says it counts command lines containing `spawn-codex.sh` and absolute `$REPO_ROOT`, excluding ` -c ` wrappers. Ideally extend the new source-regression test to assert this exact contract so the stale logs/prompts wording cannot return.

REVIEW_DONE:18:architect:comment
⟦AI:AUTO-LOOP

⟦AI:AUTO-LOOP⟧

@loning
Copy link
Copy Markdown
Contributor Author

loning commented May 25, 2026

🤖 测试评审通过: no-gap 与 floor 归属已有回归覆盖

TL;DR


详细说明

我检查了三点: PR 三点 diff、实际测试文件、以及 host 当前没有配置 polling allowlist / guard 文件的情况。这里没有新增 src/agents/ 生产文件,所以 test/.../<TypeName>Tests.cs 映射不适用。

新增的 test_concurrency_monitor_is_no_gap_only 会同时检查正向业务结果和反向回归 token: no-gap-violationexpected > 0 and actual == 0 必须存在,MIN_PARALLELcodex-concurrency-lowlow_streakactual < thresholdCODEX_FLOOR 这些旧路径不能回来。新增的 test_skill_assigns_floor_to_controller_step_1_5_only 则固定 controller wakeup step 1.5 是 floor 补给唯一位置,并检查 ScheduleWakeup 前执行、硬下限表达式、删除 ACTIVE <= 2 旧脚本、只保留一个 canonical step-1.5 脚本块。

我也跑了 PR head worktree 里的测试: python3 -m unittest test_ensure_project_rules_fixed_points.py, 30 tests OK。diff scan 没有发现新增 sleep/delay 测试节奏、[Skip]、manual category、polling allowlist 变更、弱化断言或 mock-only pseudo-coverage。


📎 完整 codex 原始输出(存档备查)
---
pr: 18
role: tests
verdict: approve
---

## Verdict
Approve: the PR adds focused source-regression coverage for the no-gap-only monitor contract and controller-owned floor refill wording, with no sleep pacing, skips, weakened assertions, or mock-only pseudo-coverage.

## Evidence
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:482` adds `test_concurrency_monitor_is_no_gap_only`, which asserts the intended monitor contract (`no-gap-violation`, `expected > 0 and actual == 0`) and blocks stale low/floor paths (`MIN_PARALLEL`, `codex-concurrency-low`, `low_streak`, `actual < threshold`, `CODEX_FLOOR`) from returning.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:499` adds `test_skill_assigns_floor_to_controller_step_1_5_only`, which asserts the documented business rule that controller wakeup step 1.5 owns floor refill before `ScheduleWakeup`, keeps the hard floor expression, removes the stale `ACTIVE <= 2` duplicate script, and leaves only one canonical step-1.5 script block.
- Diff scan found no added `sleep` / `delay` test pacing, no polling allowlist change, no `[Skip]`, no `[Trait("Category","Manual")]`, no bump-only assertion, no loosened existing assertion, and no mock-only pseudo-coverage.
- No `src/` or `agents/` production files are touched, so the repository's `test/.../<TypeName>Tests.cs` mapping requirement does not apply to this PR.
- Verified the PR-head test file in the branch worktree: `cd .refactor-loop/worktrees/skill-concurrency-floor-enforcement/skills/codex-refactor-loop/scripts && python3 -m unittest test_ensure_project_rules_fixed_points.py` passed with 30 tests.

REVIEW_DONE:18:tests:approve

⟦AI:AUTO-LOOP

⟦AI:AUTO-LOOP⟧

@loning loning merged commit 0f151b8 into auto-refact-dev May 25, 2026
@loning loning deleted the refactor/iter3-skill-concurrency-floor-enforcement branch May 25, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-loop codex-refactor-loop 管理的 issue/PR 🎉 phase:merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant