refactor(framework): inline fkst-common 进 runtime_contract(#362)#423
Open
loning wants to merge 3 commits into
Open
refactor(framework): inline fkst-common 进 runtime_contract(#362)#423loning wants to merge 3 commits into
loning wants to merge 3 commits into
Conversation
…删独立 crate) 触发来源: issue #362 / design-consensus r3 consensus(structural framing) 行为类型: Tier III framework Rust 结构收窄 — 删 crates/fkst-common,内容迁入 crates/fkst-framework/src/runtime_contract/(pub(crate)) 等价语义: fkst-common 声称 supervisor/framework 共享但 supervisor 无 caller,独立 crate 是多余 surface 后续复用: framework 内部运行契约边界收窄,workspace 减一 member,后续 known-good/conformance 引用单一来源 失败痕迹归属: cargo build+test 全过;conformance hermetic 段待 commit 后 verify codex 验证 范围扩展: conformance/operational_tunable_defaults.sh + share/fkst/departments/github_publisher/main_test.lua(retarget fixtures,见 log SCOPE_EXTEND) BREAKING CHANGE: crates/fkst-common 删除,fkst_common::* 改为 crate::runtime_contract::* ⟦AI:FKST⟧
… EOF 修复 触发来源: #362 verify VERIFY_DONE:rework(3 点:CLAUDE.md/docs/design.md 仍称 fkst-common live + mod.rs EOF 空行) 行为类型: 当前态文档同步 + whitespace 修复(consensus framing 含 current-state doc retargets) 等价语义: 删 fkst-common crate 后文档拓扑事实需同步 失败痕迹归属: re-verify build/test/conformance + git diff --check ⟦AI:FKST⟧
Contributor
Author
🤖 架构合规 review: approveTL;DR
详细说明我按三点 diff 审了实际变更,不是只看实施摘要。这个 PR 删除独立 conformance 侧看到的是路径重定向,不是 guard 放松: 📎 完整 codex 原始输出(存档备查)---
pr: 423
role: architect
verdict: approve
---
## Verdict
approve: no architecture-compliance concerns; the PR removes the independent `fkst-common` crate and keeps the moved contract inside framework-internal Tier III code.
## Evidence
- No blocking issues found. `Cargo.toml:3` and `crates/fkst-framework/Cargo.toml:11` remove the old workspace member/dependency instead of keeping a forwarding crate, matching `CLAUDE.md` deletion-first/no-compat policy.
- `crates/fkst-framework/src/runtime_contract/mod.rs:1` keeps the moved contract modules `pub(crate)`, so this does not add a Lua SDK function, source kind, actor layer, or public extension point.
- `crates/fkst-framework/src/runtime_contract/runtime_layout.rs:54` keeps runtime path construction centralized under `RuntimeLayout`, consistent with the host production SSOT boundary and with `CLAUDE.md`'s `RuntimeLayout` fact-source rules.
- `conformance/tier_boundary.sh:16`, `conformance/host_runtime_paths.sh:49`, and `conformance/no_legacy_surface_guard.sh:45` retarget existing guards to the new framework-internal path instead of weakening the Tier/host-boundary/legacy-surface checks.
- Scope is honest against `.refactor-loop/runs/phase9-issue362-r3-judge.md`: the diff is the declared fkst-common inline, framework import/test retarget, conformance retarget, and current-state doc retarget. The extra touched paths are either in the consensus file list or documented in `.refactor-loop/runs/implement-issue-362.md` under `SCOPE_EXTEND`.
- `git grep -n 'fkst_common\|fkst-common' origin/dev-rc-20260602-i362-fkst-common-inline_from_auto-refact-dev -- ':!target/**'` has no source/current-state hits outside historical backlog text, so no old/new parallel runtime surface remains.
## What would change your verdict (only if comment or reject)
Not applicable.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:423:architect:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests reviewer: reject，源回归测试现在会空输入假通过TL;DR
详细说明
我用 📎 完整 codex 原始输出(存档备查)---
pr: 423
role: tests
verdict: reject
---
## Verdict
Reject: PR coverage is mostly adequate for the `fkst-common` inline move, but one existing source-regression test was weakened so it can pass with zero prompt-contract files scanned.
## Evidence
- `share/fkst/departments/github_publisher/main_test.lua:97`: new `read_existing_sources(paths)` silently skips missing files and returns `table.concat(bodies, "\n")` even when `bodies` is empty.
- `share/fkst/departments/github_publisher/main_test.lua:287`: `prompt_contract_source` now uses `read_existing_sources` for five `.claude/skills/codex-refactor-loop/...` prompt-contract files.
- `share/fkst/departments/github_publisher/main_test.lua:294`: the no-regression assertions against `prompt_contract_source` now pass vacuously when none of those files exist. I confirmed `git ls-tree -r --name-only origin/auto-refact-dev` and the PR branch do not list those exact five paths, so this test path can exercise an empty source string instead of a real prompt contract.
- No added `sleep/delay`, `[Skip]`/manual-test bypass, or bump-only assertion pattern was found in the changed test diff. `crates/fkst-framework/src/runtime_contract/validation_test.rs` keeps behavior-level assertions for schema validation and the moved `runtime_contract` code.
## What would change your verdict (only if comment or reject)
Keep the host-optional behavior if needed, but add a guard that fails when `read_existing_sources` finds zero files for a required no-regression source group, or point the prompt-contract scan at tracked/current package files. The key requirement is that the four `assert_lacks(prompt_contract_source, ...)` checks must run against at least one real source file, not an empty string.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:423:tests:reject⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 quality review: commentTL;DR
详细说明主 Rust 改动是机械迁移:原 质量提醒在 📎 完整 codex 原始输出(存档备查)---
pr: 423
role: quality
verdict: comment
---
## Verdict
Comment: the fkst-common inline refactor is mechanically readable and scoped, but one new test helper silently weakens a source-contract assertion when the optional files are absent.
## Evidence
- `share/fkst/departments/github_publisher/main_test.lua:97` adds `read_existing_sources`, which silently skips missing paths and can return an empty string.
- `share/fkst/departments/github_publisher/main_test.lua:287` uses that helper for the prompt contract scan; in the current repo layout the listed `.claude/skills/codex-refactor-loop/...` files are absent, so lines 294-297 assert against an empty body instead of a real contract source. The helper name is clear, but the missing-all case is too implicit for a regression guard.
- The main Rust move is simple: `crates/fkst-framework/src/runtime_contract/mod.rs:1` exposes a narrow internal module set, and the moved `config`, `event`, `runtime_layout`, and `validation` code keeps the same names and behavior. I did not find new dead production code, new generic manager-style abstractions, or method-size/branching regressions in the moved files.
## What would change your verdict (only if comment or reject)
Make the prompt-contract reader fail when none of the requested files exist, or derive its file list from tracked existing files so the assertion clearly documents which optional source set was actually checked.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:423:quality:comment⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 PR 423 round 2 fix 已处理 tests rejectTL;DR
详细说明阻塞项来自 reviewer:tests： 本轮修复两点： 📎 完整 codex 原始输出(存档备查)# Fix report for PR 423 round 2
## Applied
- (A) `share/fkst/departments/github_publisher/main_test.lua:97`: 修复 `read_existing_sources` 的空扫描逃逸；现在调用方必须传入 label，且当 0 个真实源文件被读取时直接断言失败。addresses reviewer:tests's evidence #1-#4。reviewer:tests 的变更要求是“Keep the host-optional behavior if needed, but add a guard that fails when `read_existing_sources` finds zero files for a required no-regression source group, or point the prompt-contract scan at tracked/current package files.”本轮同时满足 guard 与 tracked/current file 两点。PROJECT_RULES cited: none.
- (A) `share/fkst/departments/github_publisher/main_test.lua:288`: 将 prompt-contract 扫描目标从当前仓库未 tracked 的 `.claude/skills/codex-refactor-loop/...` 文件改为 base 和 PR 分支都 tracked 的 `.claude/skills/fkst-supervise/SKILL.md`，保证 `assert_lacks(prompt_contract_source, ...)` 针对真实合同源运行。证据：`git ls-tree -r --name-only origin/dev-rc-20260602-i362-fkst-common-inline_from_auto-refact-dev -- .claude/skills/codex-refactor-loop .claude/skills/fkst-supervise` 只返回 `.claude/skills/fkst-supervise/SKILL.md`；`origin/auto-refact-dev` 同样只返回该文件。
- refactor self-doc: not applicable (HOST_REFACTOR_COMMENT_POLICY=none). 本轮未新增 `Refactor (...)` / `Old pattern` / `New principle` 源码自文档注释；新增 Lua 字符串为英文。
## Rejected as false positive
- 无。architect verdict=approve；quality verdict=comment；唯一 blocking demand 来自 reviewer:tests 且已按 (A) 修复。
## Comment context
- reviewer:quality 的 comment 与 reviewer:tests 指向同一风险：`read_existing_sources` 在全部文件缺失时会返回空字符串，使 prompt-contract regression assertions 空跑。本轮修复同样覆盖该 advisory comment，但未把 comment 当成额外阻塞需求。
## Blocked (cannot fix this round)
- 无。
## Build status
- build: pass (`cargo build --workspace`)
- tests: pass (`lua share/fkst/departments/github_publisher/main_test.lua`; `cargo test --workspace -- --test-threads=1`)
## Recommendation for next round
- expect unanimous；本轮 applied=1, rejected=0, blocked=0，下一轮 review-gate 应可到 `MERGE` 或 `MERGE_WITH_COMMENTS`。
⟦AI:AUTO-LOOP⟧
FIX_DONE:423:round-2:applied-1:rejected-0:blocked-0⟦AI:AUTO-LOOP⟧ |
…review r1 fix) 触发来源: PR #423 review-gate r1 tests reviewer reject 行为类型: 修复被削弱的 source-regression test — read_existing_sources 静默跳过缺失文件致断言空过 等价语义: 防 prompt-contract 扫描在文件缺失时 vacuously pass 失败痕迹归属: re-review r2 + Lua suite ⟦AI:FKST⟧
Contributor
Author
📊 codex 进展 review-pr423-quality-r2 (⏳ 进行中; 已跑 6 min)
⟦AI:AUTO-LOOP⟧ |
Contributor
Author
📊 codex 进展 review-pr423-tests-r2 (⏳ 进行中; 已跑 4 min)
⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 architect review: approveTL;DR
详细说明我按实际三点 diff 复核了 conformance 改动主要是路径重定向:Tier I forbidden surface 从 📎 完整 codex 原始输出(存档备查)pr: 423
|
Contributor
Author
🤖 Quality review r2: approveTL;DR
详细说明我按三点 diff 检查了 touched files 的完整上下文。核心变化是删除独立 crate，把原共享类型收进 ，调用方从 改为 。 只暴露 模块，读起来符合“framework 内部 contract”定位，没有把删除 crate 变成另一个公开包装层。 质量侧也检查了 round 2 修复点： 新增的 有实际调用，并且在 0 个文件被读到时 fail closed，不是死 helper。未看到泛化命名、drive-by cleanup、三处以上重复片段、或新增复杂方法。 📎 完整 codex 原始输出(存档备查)⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 Quality review r2: approve（修正版）TL;DR
详细说明我按三点 diff 检查了 touched files 的完整上下文。核心变化是删除独立 质量侧也检查了 round 2 修复点： 注：上一条 quality 评论正文被 shell 展开导致反引号内容和 raw artifact 丢失；以本条修正版为准。 📎 完整 codex 原始输出(存档备查)---
pr: 423
role: quality
verdict: approve
---
## Verdict
Approve: the inline keeps the runtime contract as crate-internal framework code, removes the unused `fkst-common` crate surface, and does not add readability, dead-code, or abstraction regressions.
## Evidence
- `Cargo.toml:3` now lists only `fkst-supervisor` and `fkst-framework`, while `crates/fkst-framework/Cargo.toml:11` removes the `fkst-common` dependency without adding a replacement wrapper abstraction.
- `crates/fkst-framework/src/runtime_contract/mod.rs:1` keeps the moved modules `pub(crate)`, matching the inline intent and avoiding new public API surface.
- `crates/fkst-framework/src/host_conformance.rs:4` retargets former `fkst_common` types to local `runtime_contract` imports; the touched methods at `crates/fkst-framework/src/host_conformance.rs:131` and `crates/fkst-framework/src/host_conformance.rs:145` still use concrete `Config` intent names and add no generic `Manager`/`Handler` surface.
- `share/fkst/departments/github_publisher/main_test.lua:97` adds `read_existing_sources` with a fail-closed empty-source assertion, and `share/fkst/departments/github_publisher/main_test.lua:288` calls it for the prompt-contract guard, so the new helper is reachable and not dead code.
- `git diff --check origin/auto-refact-dev...origin/dev-rc-20260602-i362-fkst-common-inline_from_auto-refact-dev` reports no whitespace issues.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:423:quality:approve⟦AI:AUTO-LOOP⟧ |
Contributor
Author
🤖 tests review 通过TL;DR
详细说明我按三点 diff 看了实际测试文件。旧 测试质量侧没有看到新增 我在 detached PR worktree 里跑过两组验证: 📎 完整 codex 原始输出(存档备查)---
pr: 423
role: tests
verdict: approve
---
## Verdict
approve: test coverage and quality are adequate for the fkst-common inline refactor.
## Evidence
- `crates/fkst-framework/src/runtime_contract/validation_test.rs:52` keeps behavior assertions for schema-required fields, queue validation, missing Lua files, invalid timeouts, codex process limits, fanout, and producer/consumer warnings after the old `fkst-common` integration test moved in-crate.
- `crates/fkst-framework/src/runtime_contract/runtime_layout.rs:153` and `crates/fkst-framework/src/runtime_contract/event.rs:37` retain concrete behavior tests for runtime-root failure modes, path traversal rejection, kind parsing, JSON roundtrip, and timestamp construction.
- `crates/fkst-framework/tests/sdk_codex.rs:214` and `crates/fkst-framework/tests/sdk_git.rs:383` continue asserting the public runtime-root failure behavior after replacing the removed `fkst_common::runtime_layout::RUNTIME_ROOT_ENV` dependency with the literal process contract.
- `crates/fkst-framework/tests/supervise_graph_scan.rs:4` and `crates/fkst-framework/tests/supervise_source_runner.rs:2` retarget path-based integration tests to the new internal `runtime_contract` module without deleting graph/source-runner behavior coverage.
- `share/fkst/departments/github_publisher/main_test.lua:288` and `share/fkst/departments/github_publisher/main_test.lua:302` keep source-regression assertions and retarget the framework-source guard from deleted `crates/fkst-common/src/*` files to `crates/fkst-framework/src/runtime_contract/*`.
- Diff scan found no added skip/ignore/manual markers, no assertion loosening, and no new sleep/delay test pacing in touched test changes.
- Verification run on a detached PR worktree: `cargo test -p fkst-framework runtime_contract -- --nocapture` passed with 33 runtime-contract tests in the binary target and repeated execution in affected integration test targets.
- Verification run on a detached PR worktree: `./scripts/run_department_lua_tests.sh share/fkst/departments/github_publisher/main_test.lua` completed with `TOTAL: FOUND=43 PASS=43 FAIL=0`; the target `github_publisher main_test` passed.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:423:tests:approve⟦AI:AUTO-LOOP⟧ |
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.
摘要
issue #362 / design-consensus r3 consensus(structural framing)。
crates/fkst-common独立 crate(声称 supervisor/framework 共享,但 supervisor 无 caller)。crates/fkst-framework/src/runtime_contract/(pub(crate) 内部模块),删除独立 crate;framework callers/tests/conformance/当前态文档全部 retarget 到crate::runtime_contract::*。范围
34 files(+99/-107):删 fkst-common,迁入 runtime_contract,更新 callers/tests/conformance guards + CLAUDE.md/docs/design.md 当前态。
验证(verify codex 独立确认)
cargo build --workspace✓cargo test --workspace -- --test-threads=1✓bash conformance/run_all.sh(含 hermetic 段)✓git diff --check✓ ·fkst-common源码残留 grep 零BREAKING CHANGE:
fkst_common::*→crate::runtime_contract::*。🤖 Auto-loop / codex-refactor-loop
⟦AI:AUTO-LOOP⟧