refactor(skill): host-agnostic skill root 自定位,脱 .claude/skills 硬编码(#19 共识)#33
Merged
loning merged 3 commits intoMay 25, 2026
Merged
Conversation
…共识) Phase 9 r4 structural 共识(4 轮收敛):dev_sync_daemon.py / triage-monitor.sh inline self-location(CODEX_REFACTOR_LOOP_SKILL_ROOT 可选 override,否则 __file__/BASH_SOURCE parent 自定位 + 验证 SKILL.md/spawn-codex.sh/prompts/, invalid override fatal,不 fallback .claude/skills);SKILL.md/REFERENCE.md active launch/dispatch 路径示例改 skill-relative + Skill root contract 段;9 个 prompt + _github-post-rules.md 的 locator 措辞 skill-relative。+ 4 个 self-location 测试。 不引入 SkillRootLocatorV1/skill_root.py/外部 $SKILL_ROOT/host.env 字段。 Closes #19 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
loning
added
auto-loop
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Quality review: comment-only, 1 个 stale commentTL;DR
详细说明
这不是 blocker:没有发现死代码、过度抽象、明显重复抽取需求或越界 cleanup。建议只把那行注释改成类似: 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: quality
verdict: comment
---
## Verdict
Comment-only: the self-location change is simple and focused, but one stale comment now contradicts the new skill-root dispatch path.
## Evidence
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:194` still says `spawn-codex.sh` is "在 main repo", but this PR deliberately changes `SPAWN_CODEX` at line 75 to `SKILL_ROOT / "scripts" / "spawn-codex.sh"`. The comment is now stale and makes the new host-agnostic dispatch path harder to read.
## What would change your verdict (only if comment or reject)
Update or delete the stale line 194 comment, for example: `# spawn-codex.sh is resolved from the installed skill root; --cd points at the conflict worktree.`
REVIEW_DONE:33:quality:comment
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 architect review: comment-only, no blocking architecture regressionTL;DR
详细说明从 CLAUDE/AGENTS 角度看,这次改动把 active 我留下两个 comment-only 点:第一, 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: architect
verdict: comment
---
## Verdict
comment-only: merge is OK from the architect angle, but the PR should tighten the required refactor-comment wording and document/justify the `REFERENCE.md` scope expansion.
## Evidence
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:53` and `skills/codex-refactor-loop/scripts/triage-monitor.sh:16`: the new self-location functions have refactor comments, but they do not match the review checklist's required shape: `// Refactor (iterN/cluster-XXX): Old pattern: … New principle: …`. Current text uses `Old:` / `New:` and is therefore slightly underspecified for the Old/New pattern audit. This is a comment, not a reject, because the intent and cluster id are still clear and no PROJECT_RULES/AGENTS clause is violated.
- `skills/codex-refactor-loop/REFERENCE.md:20`, `skills/codex-refactor-loop/REFERENCE.md:145`, `skills/codex-refactor-loop/REFERENCE.md:219`, `skills/codex-refactor-loop/REFERENCE.md:225`, `skills/codex-refactor-loop/REFERENCE.md:311`, `skills/codex-refactor-loop/REFERENCE.md:319`, `skills/codex-refactor-loop/REFERENCE.md:849`, `skills/codex-refactor-loop/REFERENCE.md:1004`, `skills/codex-refactor-loop/REFERENCE.md:1060`, and `skills/codex-refactor-loop/REFERENCE.md:1269`: the diff updates active command examples in `REFERENCE.md`, but the Phase 9 plan explicitly named runtime scripts, `SKILL.md`, selected prompts, and tests, and did not name `REFERENCE.md` or include a `SCOPE_EXTEND` note. This is a scope-honesty comment rather than a reject because `REFERENCE.md` is linked from `SKILL.md` as the lazy command-body surface and the edits are consistent with AGENTS.md's allowance: `例外：经 Phase 9 deep consensus 明确授权的 host-agnostic bootstrap / policy 注入修正可先落地，但不得引入具体 host 事实，且必须配套行为测试。`
- Architecture compliance checks passed: no new actor/event-envelope/proto/state-version/compat-shim pattern, no new external repo references, no version-manifest drift, `host.env.example` remains free of `CODEX_REFACTOR_LOOP_SKILL_ROOT`, and runtime self-location fails closed instead of falling back to `.claude/skills`.
## What would change your verdict (only if comment or reject)
- Reword the two new refactor comments to the exact Old/New checklist form, preferably `Old pattern:` and `New principle:`.
- Add a short `SCOPE_EXTEND` note in the implement summary or PR body explaining why `REFERENCE.md` command-body examples were included as active skill-root surfaces, or narrow the diff back to the exact Phase 9 file list if the controller wants strict scope.
REVIEW_DONE:33:architect:comment
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests reviewer: 拒绝，缺一个 valid override 分支行为测试TL;DR
详细说明这组测试整体方向是对的:已有 阻塞点很窄: 建议补一个测试:临时创建有效 skill root,含 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: tests
verdict: reject
---
## Verdict
Reject: the suite is mostly solid, but one net-new runtime branch in `dev_sync_daemon.skill_root()` is not covered by behavior tests.
## Evidence
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:52` adds `skill_root()`, including the `CODEX_REFACTOR_LOOP_SKILL_ROOT` override branch and the default `__file__` self-location branch; tests cover default import behavior and invalid override fail-closed, but no test exercises a valid non-default override and proves `SPAWN_CODEX` resolves to that override's `scripts/spawn-codex.sh`.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1188` covers the invalid override outcome for both `triage-monitor.sh` and `dev_sync_daemon.py` by asserting non-zero exit, an invalid-root error, and no triage state mutation. That is good fail-closed coverage, but it does not cover the valid override branch added in `dev_sync_daemon.py`.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:873` exercises `triage-monitor.sh` with a valid `CODEX_REFACTOR_LOOP_SKILL_ROOT` and asserts dispatch state outcomes, so the equivalent triage valid-override path is covered.
- The new source-regression tests at `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1149`, `:1167`, `:1240`, `:1258`, and `:1280` assert the no-regression contract for inline locators, absence of shared locator helpers, and skill-relative prompt/doc paths. These are meaningful source-regression assertions, not bump-only tests.
- Diff scan found no added `sleep` / `delay` test pacing, no polling allowlist changes, no `[Skip]`, no `[Trait("Category","Manual")]`, no weakened assertions, and no mock-only pseudo-coverage.
- Verification run on PR head passed: `python3 -m py_compile skills/codex-refactor-loop/scripts/dev_sync_daemon.py skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py && python3 skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py` => 58 tests OK.
## What would change your verdict (only if comment or reject)
Add a behavior test that imports `dev_sync_daemon.py` with `CODEX_REFACTOR_LOOP_SKILL_ROOT` pointing at a temporary valid skill root and asserts a real outcome, for example `dev_sync_daemon.SPAWN_CODEX == <override>/scripts/spawn-codex.sh` or a stubbed `dispatch_codex_resolve()` invokes that override path. Keep the existing invalid override fail-closed test.
REVIEW_DONE:33:tests:reject
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 状态卡片 — fix 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
tests reject:dev_sync_daemon.skill_root() 一分支缺行为测试。applied-3:补该分支 确定性行为测试 + 修 2 处 Refactor Old/New 注释格式 + 清 1 行与 skill-root 语义 冲突的 stale 注释。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests review: 缺一条 triage 默认自定位行为测试TL;DR
详细说明
现有测试里,实际执行 我没有发现新增 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: tests
verdict: reject
---
## Verdict
Reject: the PR leaves the new `triage-monitor.sh` default self-location branch without behavior coverage.
## Evidence
- `skills/codex-refactor-loop/scripts/triage-monitor.sh:16` adds the default branch that derives the skill root from `${BASH_SOURCE[0]}` when `CODEX_REFACTOR_LOOP_SKILL_ROOT` is unset, but the runtime tests that execute `triage-monitor.sh` set `CODEX_REFACTOR_LOOP_SKILL_ROOT` explicitly.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:975` sets `CODEX_REFACTOR_LOOP_SKILL_ROOT` in `test_triage_monitor_loop_dispatches_state_machine_branches_once`, so the test only covers the override branch.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1251` sets `CODEX_REFACTOR_LOOP_SKILL_ROOT` in `test_invalid_specific_skill_root_fails_closed`, so the fail-closed test also covers only the override path.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1211` has `test_triage_monitor_self_locates_before_state_mutation`, but it is a source-string regression test. It asserts that `${BASH_SOURCE[0]}` appears and ordering is correct, but it does not execute the unset-env branch or prove prompt materialization/spawn works from the script-relative root.
- Positive checks: the PR-head test file passes when executed directly from a temporary archive: `59 tests OK`. I found no added test `sleep`/`delay` pacing, no polling allowlist change, no `[Skip]`, no manual category, no weakened assertion, and no mock-only pseudo-coverage.
## What would change your verdict
Add a behavior test that runs `bash <copied-skill-root>/scripts/triage-monitor.sh` with `CODEX_REFACTOR_LOOP_SKILL_ROOT` unset, fake `gh`, fake `spawn-codex.sh`, `TRIAGE_MONITOR_ONCE=1`, and a temp `REPO_ROOT`; assert that the prompt template is read from the script-relative skill root, the spawn script from that same root is invoked, and state/log output reflects the expected dispatch. This should exercise the actual `${BASH_SOURCE[0]}` path, not just assert the token exists in source.
REVIEW_DONE:33:tests:reject
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 架构审查: comment-only, 建议补 scope 说明TL;DR
详细说明我没有看到新的 PROJECT_RULES/AGENTS 违规:没有新增 shared locator、通用外部 唯一 comment 是 scope honesty: 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: architect
verdict: comment
---
## Verdict
comment-only: merge is OK from the architect angle, but the PR should document the `REFERENCE.md` scope expansion.
## Evidence
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:52` and `skills/codex-refactor-loop/scripts/triage-monitor.sh:15`: the new self-location functions include cluster-specific Old/New refactor comments at lines 53 and 16 respectively, and map to the Phase 9 plan's inline self-location boundary. The relevant AGENTS.md clause allows this category: `例外：经 Phase 9 deep consensus 明确授权的 host-agnostic bootstrap / policy 注入修正可先落地，但不得引入具体 host 事实，且必须配套行为测试。`
- `skills/codex-refactor-loop/SKILL.md:62`: adds a host-agnostic `Skill Root Contract` and keeps `CODEX_REFACTOR_LOOP_SKILL_ROOT` optional for wrappers/nonstandard packaging only, matching the Phase 9 decision at `.refactor-loop/runs/phase9-issue19-r4-judge.md:23` and avoiding a new host fact.
- `skills/codex-refactor-loop/REFERENCE.md:20`, `skills/codex-refactor-loop/REFERENCE.md:145`, `skills/codex-refactor-loop/REFERENCE.md:219`, `skills/codex-refactor-loop/REFERENCE.md:311`, `skills/codex-refactor-loop/REFERENCE.md:849`, `skills/codex-refactor-loop/REFERENCE.md:1004`, `skills/codex-refactor-loop/REFERENCE.md:1060`, and `skills/codex-refactor-loop/REFERENCE.md:1269`: these edits convert active command examples to `<skill-root>`, which is architecturally consistent with the new contract. However, the Phase 9 plan explicitly listed runtime scripts, `SKILL.md`, selected prompts, and tests, while `REFERENCE.md` was not named and `.refactor-loop/runs/implement-skill-skill-root-contract.log` is absent, so there is no documented `SCOPE_EXTEND`. This is a scope-honesty comment, not a reject, because `REFERENCE.md:3` now defines itself as the place for full command bodies and long path examples, and the changed examples are the same active instruction surface that `SKILL.md` links to.
- Architecture grep checks found no new `actor.HandleEventAsync(`, `SubscribeAsync<EventEnvelope>`, JSON serializer for actor state/committed payloads, production `sleep/delay(` addition, sync-over-async wording, raw `HttpClient` constructor, `[Skip]`, actor read/write/store split, external repo reference, proto change, or deletion-first compat shim. `host.env.example` remains free of `CODEX_REFACTOR_LOOP_SKILL_ROOT`, matching `.refactor-loop/runs/phase9-issue19-r4-judge.md:37`.
## What would change your verdict (only if comment or reject)
- Add a short `SCOPE_EXTEND` note in the PR body or implementation summary explaining that `REFERENCE.md` command-body examples are active skill-root surfaces linked from `SKILL.md`, or narrow the diff back to the exact Phase 9 file list if strict scope is desired.
REVIEW_DONE:33:architect:comment
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 quality review: comment, 测试类超过本地拆分阈值TL;DR
详细说明我没有看到 dead code、单实现抽象、命名泛化或明显 unrelated drive-by。 唯一质量问题在 建议把这批新增测试移到独立类,例如 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: quality
verdict: comment
---
## Verdict
Readable and focused overall, but the new tests violate the file's own split threshold and make the regression suite harder to scan.
## Evidence
- skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1129 says `SkillContractSourceRegressionTests` should stay contiguous only until `this class >250 LOC`, but the PR appends the new skill-root locator tests into that same class through line 1520, leaving it at roughly 395 LOC. This is not a functional issue, but it is an objective readability/complexity regression against the local rule.
## What would change your verdict (only if comment or reject)
Move the new skill-root/self-location regression tests into a separate focused class, for example `SkillRootContractSourceRegressionTests`, or split the contract suite according to the documented threshold while keeping the tests in the same file if a new file is still intentionally deferred.
REVIEW_DONE:33:quality:comment
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 状态卡片 — fix 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
…#19) 按 Phase 8 r2 verdict 修 reject + 2 advisory comment: - 给 triage-monitor.sh 加 CODEX_REFACTOR_LOOP_SKILL_ROOT_PRINT short-circuit hook,在自定位之后/任何 mutation 之前打印 resolved skill root 并退出。 补 SkillRootContractSourceRegressionTests::test_triage_monitor_default_self_location 断言 unset override 时该 hook 解析到 tmp 安装目录(default 分支行为测试)。 - 把新增的 skill-root locator 回归测试从 SkillContractSourceRegressionTests (~395 LOC,超本文件 250 LOC 阈值)拆到新 class SkillRootContractSourceRegressionTests,让原 class 回 contiguous-readable。 - SKILL.md 「Skill Root Contract」段末尾加 scope 行,说明详细路径示例和 host 安装变体留 REFERENCE.md,SKILL.md 仅留 controller-level 不变量。 verify: python3 -m unittest discover -s skills/codex-refactor-loop/scripts -p 'test_*.py' → 70 tests passed. ⟦AI:AUTO-LOOP⟧
Contributor
Author
📊 状态卡片 — reviewer 派出
🤖 controller status banner ⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Tests reviewer approve: PR 33 测试覆盖足够TL;DR
详细说明我打开了实际 diff 和测试文件, 重点看 与 这两个运行脚本的新 skill root 自定位逻辑。新增测试不是 bump-line-count: 它们分别验证有效 override 会被真实导入/脚本路径使用、默认路径从脚本位置解析、无效 override 会 fail closed, 以及 triage monitor 在写 状态前已经完成根目录校验。 我也扫了禁区: 没有新增 、manual category、空断言、弱化断言或测试节奏用的 ; 没有改 polling allowlist。用 PR 分支 archive 跑了目标测试类: , 7 tests OK。 📎 完整 codex 原始输出(存档备查)---
pr: 33
role: tests
verdict: approve
---
## Verdict
Test coverage and test quality are adequate for the host-agnostic skill root diff.
## Evidence
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1129` adds source-regression coverage that locks `dev_sync_daemon.py` to inline self-location, validated `CODEX_REFACTOR_LOOP_SKILL_ROOT`, required skill files, and removal of the old `.claude` spawn path.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1147` exercises the real `dev_sync_daemon` import path with a valid override and asserts both `SKILL_ROOT` and `SPAWN_CODEX` resolve to the override-owned files, not just "no exception".
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1191` covers the triage monitor source contract, including resolution before state mutation and use of skill-relative prompt/spawn paths.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1214` exercises default `triage-monitor.sh` self-location from `BASH_SOURCE[0]` in an installed-copy fixture and asserts the print hook exits before creating `.refactor-loop` state.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1246` covers invalid override fail-closed behavior for both `triage-monitor.sh` and `dev_sync_daemon.py`, including no state-file mutation on triage failure.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1297` and `:1314` add negative regression assertions against a shared generic locator/env contract and old host-specific `.claude/skills/...` launch docs.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1363` extends prompt post-rule regression coverage so active prompts reference this skill's relative `prompts/_github-post-rules.md` instead of the old host path.
- Diff scan found no added `[Skip]`, manual-category traits, assertion loosening, no-op asserts, or new sleep/delay test pacing. No polling allowlist file was touched.
- Verification: from an archived copy of `origin/refactor/iter3-skill-skill-root-contract`, `python3 -m unittest test_ensure_project_rules_fixed_points.SkillRootContractSourceRegressionTests` passed: 7 tests, OK.
REVIEW_DONE:33:tests:approve
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 quality review approve: skill-root 自定位可读且聚焦TL;DR
详细说明我按三点 diff 读了 PR,并打开了主要 touched files 的完整内容。新增运行时代码集中在
📎 完整 codex 原始输出(存档备查)---
pr: 18
role: quality
verdict: approve
---
## Verdict
Approve: the PR is readable, focused on host-agnostic skill-root self-location, avoids new shared abstractions, and includes clear refactor self-doc comments.
## Evidence
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:52`: `skill_root()` is a small inline locator with business-specific naming, validates the required skill files, and has a direct production caller through module-level `SKILL_ROOT` / `SPAWN_CODEX`.
- `skills/codex-refactor-loop/scripts/triage-monitor.sh:15`: `resolve_skill_root()` is reachable before repo-state setup, validates `SKILL.md`, `prompts/triage-external-issue.md`, and `scripts/spawn-codex.sh`, and keeps the Bash implementation local instead of adding a single-implementer shared locator.
- `skills/codex-refactor-loop/SKILL.md:62`: the new `Skill Root Contract` section is short and explains the controller-facing invariant without pulling heavy install variants into the entrypoint.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1123`: the added regression class is long but scoped to this contract, uses existing test-file patterns, and does not introduce a new scanner/helper abstraction.
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:53` and `skills/codex-refactor-loop/scripts/triage-monitor.sh:16`: required `Refactor (iter3/skill-skill-root-contract)` Old/New comments are present and understandable without issue-only placeholders.
- Checked active prompt/runtime surfaces for `.claude/skills/codex-refactor-loop/(scripts|prompts)` hardcoded launch paths; none remain in `SKILL.md`, `REFERENCE.md`, active prompts, `dev_sync_daemon.py`, or `triage-monitor.sh`.
## What would change your verdict (only if comment or reject)
N/A
REVIEW_DONE:33:quality:approve
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Tests reviewer approve: PR 33 测试覆盖足够(更正版)TL;DR
详细说明我打开了实际 diff 和测试文件, 重点看 我也扫了禁区: 没有新增 📎 完整 codex 原始输出(存档备查)---
pr: 33
role: tests
verdict: approve
---
## Verdict
Test coverage and test quality are adequate for the host-agnostic skill root diff.
## Evidence
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1129` adds source-regression coverage that locks `dev_sync_daemon.py` to inline self-location, validated `CODEX_REFACTOR_LOOP_SKILL_ROOT`, required skill files, and removal of the old `.claude` spawn path.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1147` exercises the real `dev_sync_daemon` import path with a valid override and asserts both `SKILL_ROOT` and `SPAWN_CODEX` resolve to the override-owned files, not just "no exception".
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1191` covers the triage monitor source contract, including resolution before state mutation and use of skill-relative prompt/spawn paths.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1214` exercises default `triage-monitor.sh` self-location from `BASH_SOURCE[0]` in an installed-copy fixture and asserts the print hook exits before creating `.refactor-loop` state.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1246` covers invalid override fail-closed behavior for both `triage-monitor.sh` and `dev_sync_daemon.py`, including no state-file mutation on triage failure.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1297` and `:1314` add negative regression assertions against a shared generic locator/env contract and old host-specific `.claude/skills/...` launch docs.
- `skills/codex-refactor-loop/scripts/test_ensure_project_rules_fixed_points.py:1363` extends prompt post-rule regression coverage so active prompts reference this skill's relative `prompts/_github-post-rules.md` instead of the old host path.
- Diff scan found no added `[Skip]`, manual-category traits, assertion loosening, no-op asserts, or new sleep/delay test pacing. No polling allowlist file was touched.
- Verification: from an archived copy of `origin/refactor/iter3-skill-skill-root-contract`, `python3 -m unittest test_ensure_project_rules_fixed_points.SkillRootContractSourceRegressionTests` passed: 7 tests, OK.
REVIEW_DONE:33:tests:approve
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 architect review: comment-only，需补一条 REFERENCE.md scope 说明TL;DR
详细说明新增的 唯一需要 surfacing 的是 scope honesty：Phase 9 plan 明列了 runtime scripts、 📎 完整 codex 原始输出(存档备查)---
pr: 18
role: architect
verdict: comment
---
## Verdict
comment-only: merge is OK from the architect angle, but the PR should document the `REFERENCE.md` scope expansion.
## Evidence
- `skills/codex-refactor-loop/scripts/dev_sync_daemon.py:52` and `skills/codex-refactor-loop/scripts/triage-monitor.sh:15`: the new self-location functions include cluster-specific Old/New refactor comments at lines 53 and 16 respectively, and map to the Phase 9 plan's inline self-location boundary. The relevant AGENTS.md clause allows this category: `例外：经 Phase 9 deep consensus 明确授权的 host-agnostic bootstrap / policy 注入修正可先落地，但不得引入具体 host 事实，且必须配套行为测试。`
- `skills/codex-refactor-loop/SKILL.md:62`: adds a host-agnostic `Skill Root Contract` and keeps `CODEX_REFACTOR_LOOP_SKILL_ROOT` optional for wrappers/nonstandard packaging only, matching the Phase 9 decision at `.refactor-loop/runs/phase9-issue19-r4-judge.md:23` and avoiding a new host fact.
- `skills/codex-refactor-loop/REFERENCE.md:20`, `skills/codex-refactor-loop/REFERENCE.md:145`, `skills/codex-refactor-loop/REFERENCE.md:219`, `skills/codex-refactor-loop/REFERENCE.md:311`, `skills/codex-refactor-loop/REFERENCE.md:849`, `skills/codex-refactor-loop/REFERENCE.md:1004`, `skills/codex-refactor-loop/REFERENCE.md:1060`, and `skills/codex-refactor-loop/REFERENCE.md:1269`: these edits convert active command examples to `<skill-root>`, which is architecturally consistent with the new contract. However, the Phase 9 plan explicitly listed runtime scripts, `SKILL.md`, selected prompts, and tests, while `REFERENCE.md` was not named and `.refactor-loop/runs/implement-skill-skill-root-contract.log` is absent, so there is no documented `SCOPE_EXTEND`. This is a scope-honesty comment, not a reject, because `REFERENCE.md:3` now defines itself as the place for full command bodies and long path examples, and the changed examples are the same active instruction surface that `SKILL.md` links to.
- Architecture grep checks found no new `actor.HandleEventAsync(`, `SubscribeAsync<EventEnvelope>`, JSON serializer for actor state/committed payloads, production `sleep/delay(` addition, sync-over-async wording, raw `HttpClient` constructor, `[Skip]`, actor read/write/store split, external repo reference, proto change, or deletion-first compat shim. `host.env.example` remains free of `CODEX_REFACTOR_LOOP_SKILL_ROOT`, matching `.refactor-loop/runs/phase9-issue19-r4-judge.md:37`.
## What would change your verdict (only if comment or reject)
- Add a short `SCOPE_EXTEND` note in the PR body or implementation summary explaining that `REFERENCE.md` command-body examples are active skill-root surfaces linked from `SKILL.md`, or narrow the diff back to the exact Phase 9 file list if strict scope is desired.
REVIEW_DONE:33:architect:comment
⟦AI:AUTO-LOOP⟧⟦AI:AUTO-LOOP⟧ |
This was referenced May 25, 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
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.
🤖 refactor(skill): host-agnostic skill root 自定位(#19 r4 共识)
Phase 9 r4 structural 共识落地(4 轮收敛,直击 README 泛化路线):
dev_sync_daemon.py/triage-monitor.sh:inline self-location ——CODEX_REFACTOR_LOOP_SKILL_ROOT可选 override,否则__file__/BASH_SOURCEparent 自定位(验证 SKILL.md/spawn-codex.sh/prompts/),invalid override fail-closed,不 fallback.claude/skillsSKILL.md/REFERENCE.md:active launch/dispatch 路径示例改 skill-relative + 加Skill root contract段_github-post-rules.md:locator 措辞 skill-relative不引入 SkillRootLocatorV1/skill_root.py/外部
$SKILL_ROOT/host.env 字段。无 SCOPE_EXTEND。Closes #19
⟦AI:AUTO-LOOP⟧