fix(scheduler): inject capability registry instead of importing catalog from control_plane - #4023
Conversation
…og from control_plane The m6 maintainability ratchet fails on this branch: event_driven_dispatch.py imports loopx.capabilities.catalog directly, violating the control_plane_outward_dependency rule (control_plane/ must not import loopx.capabilities*), and the finding is not covered by a reviewed exception. Fix by dependency injection instead of adding an exception (which would permanently bless the violation) or weakening the ratchet: - capabilities_bridge: add a process-level registry factory hook (set_capability_registry_factory / capability_registry_or_none), following the existing P3 hook pattern; factory errors degrade to token-only matching, never raise out of claim paths - event_driven_dispatch: claim_next_task and build_event_driven_dispatch gain a "registry" kwarg; the capabilities/lease path falls back to the factory hook when the caller passes none; the forbidden catalog import is removed - cli.py: registers build_capability_registry as the factory at import time (the cli layer is outside the ratchet's scope and already imports catalog legally) - starter_scheduler: dispatch call site injects the registry - tests/conftest: register the factory for the test process — tests exercise claim paths without importing loopx.cli, and the fail-closed semantics (binding to an unknown pack is not claimable) structurally require a registry to exist Verified on fork head f6549a9: tests/control_plane/ 1205/1205 passed (was 1204 passed + m6 ratchet red); m6 gates 3/3; cache-free rerun to rule out stale-bytecode false greens.
There was a problem hiding this comment.
🟡 Changes recommended
record_policy_decisions currently defaults to on (via the master switch) despite being described and tested as opt-in/optional, and the PR’s stated scope doesn’t match the breadth of new surfaces introduced.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR significantly expands the Phase 5/6 “new architecture” surfaces in LoopX: it adds a unified PolicyEngine decision contract (and optional decision audit events), wires event-driven scheduler/dispatch advancement into multiple goal-loop runtimes and UI renderers, and extends rollout-event bridging so scheduler/closure logic can rely on the event log rather than markdown-only state.
Changes:
- Introduce
loopx.control_plane.policy(Decision contract, PolicyEngine composition, and optional decision event recording) and wirepolicy_decisioninto live quota decisions and renderers. - Add/extend event-driven control-plane integrations (dispatch probes from PI/OpenCode loops, control-plane observability snapshot, heartbeat-as-event-source, goal closure/acceptance helpers, and additional rollout event kinds).
- Bridge CLI todo mutations into rollout events (
todo_add/todo_complete) to keep event-driven dispatch projections consistent, plus add tests covering these new behaviors.
File summaries
| File | Description |
|---|---|
| tests/test_claude_goal_policy.py | Adds unit tests asserting policy_decision.outcome takes precedence over legacy should_run. |
| tests/pi_goal_loop_runtime.test.mjs | Adds tests for unified policy decision behavior and terminal detection in PI goal loop. |
| tests/opencode_goal_bridge_runtime.test.mjs | Adds tests for unified policy decision behavior and terminal handling in OpenCode bridge. |
| tests/control_plane/test_todo_mutation_authority.py | Adds regression test ensuring todo add appends rollout events visible to event-driven dispatch. |
| tests/control_plane/test_start_goal_compact_projection.py | Adds tests ensuring closed goals require starting a fresh goal id via bootstrap inspection. |
| tests/control_plane/test_rich_decision.py | Tests new rich Decision action vocabulary and normalization maps. |
| tests/control_plane/test_policy_pilot_wiring.py | Tests pilot wiring for attaching/recording policy_decision via live quota decision builder. |
| tests/control_plane/test_policy_integration.py | Adds integration tests verifying PolicyEngine normalization stays consistent with legacy decisions. |
| tests/control_plane/test_policy_engine.py | Adds unit tests for Decision normalization, combination, and engine composition behavior. |
| tests/control_plane/test_policy_decision_events.py | Adds tests for deterministic fingerprints and idempotent/transition-only recording. |
| tests/control_plane/test_heartbeat_prequota.py | Adds tests for new pre-quota hook registry fan-out and isolation behavior. |
| tests/control_plane/test_heartbeat_event_source.py | Adds tests for heartbeat-as-event-source enablement and idempotent observation events. |
| tests/control_plane/test_goal_closure.py | Adds tests for goal closure evaluation and idempotent closure events. |
| tests/control_plane/test_goal_channel_projection_quota.py | Adds tests for compacting/surfacing scheduler hints + policy decisions in projections. |
| tests/control_plane/test_goal_acceptance.py | Adds tests for acceptance criteria/evidence verification and its interaction with closure. |
| tests/control_plane/test_cost_projection.py | Adds tests for quota spend fact projection and usage summaries. |
| tests/control_plane/test_control_plane_observability.py | Adds tests for read-only observability snapshot aggregation over queue/events/decisions. |
| tests/control_plane/test_capabilities_bridge.py | Adds tests around capability token normalization, registrar discovery, and hook/event hub behavior. |
| tests/conftest.py | Registers capability registry factory in pytest so claim-time code paths see the registry. |
| loopx/todos.py | Bridges todo add/todo complete to rollout events for event-driven scheduler projections. |
| loopx/slash_command_install.py | Extends installed slash-command guidance to prefer event-driven scheduler CLI flows and env flags. |
| loopx/rollout_event_log.py | Expands allowed rollout event kinds (goal closure, policy, heartbeat, task lifecycle). |
| loopx/registry.py | Adds helper to sync registry goal status to closed when closure is derived elsewhere. |
| loopx/presentation/renderers/status_markdown.py | Renders policy_decision and control-plane observability sections in status markdown. |
| loopx/pi_goal_mode/pi-goal-loop-runtime.mjs | Adds policy_decision parsing + dispatch probe integration into PI goal loop runtime. |
| loopx/pi_goal_mode/loopx-goal.ts | Implements event-driven dispatch probe invocation for PI extension via CLI. |
| loopx/opencode_goal_mode/goal-bridge-runtime.mjs | Adds policy_decision parsing + dispatch probe integration into OpenCode goal bridge. |
| loopx/heartbeat_prompt.py | Exposes an additional heartbeat rule constant for event-driven execution guidance. |
| loopx/heartbeat_prequota.py | Reworks pre-quota flow into a hook registry model while preserving legacy compatibility keys. |
| loopx/control_plane/testing/cli_output_budget.py | Adjusts output budgets and marks dashboard as an explicit cold-path exception. |
| loopx/control_plane/status/control_plane_observability.py | Adds read-only control-plane observability snapshot builder. |
| loopx/control_plane/runtime/replay.py | Introduces deterministic task state replay utilities (checkpoint + events). |
| loopx/control_plane/runtime/checkpoint.py | Introduces task-level checkpoints and integrity validation helpers. |
| loopx/control_plane/quota/live_decision.py | Wires PolicyEngine into live quota decision building, attaches policy_decision, and optionally records events. |
| loopx/control_plane/policy/engine.py | Adds PolicyEngine composition logic over scheduler validation + quota + optional capability gate. |
| loopx/control_plane/policy/decision.py | Adds Decision dataclass, normalization maps, and strictness/combination semantics. |
| loopx/control_plane/policy/decision_events.py | Adds opt-in decision event recording, deterministic fingerprints, and transition-only dedup state. |
| loopx/control_plane/policy/init.py | Exposes public PolicyEngine/Decision APIs for the control-plane policy package. |
| loopx/control_plane/plan/capabilities-bridge-migration-notes.md | Adds migration notes documenting capability-pack bridge behavior changes and boundaries. |
| loopx/control_plane/new_architecture.py | Introduces master switch semantics for new-architecture feature enablement. |
| loopx/control_plane/heartbeat/task_body.py | Injects event-driven execution guidance into the heartbeat task body prompt. |
| loopx/control_plane/heartbeat/rules.py | Adds an event-driven execution rule string for heartbeat guidance. |
| loopx/control_plane/heartbeat/event_source.py | Adds heartbeat observation event recording as an opt-in event source. |
| loopx/control_plane/goals/goal_channel_projection.py | Adds compaction of scheduler hint + policy decision into frontstage quota projection. |
| loopx/contract.py | Extends scan skip rules and aligns tracked-file scan with skip-dir behavior. |
| loopx/codex_cli_probe.py | Exposes markdown renderer for scheduler dispatch probe output. |
| loopx/codex_cli_probe_markdown.py | Adds markdown rendering for local scheduler dispatch probe payloads. |
| loopx/cli.py | Switches capability pack command registration to registry-driven wiring and registers registry factory at import time. |
| loopx/claude_goal_mode/statusline/goal_status.py | Updates run-state rendering to prefer policy_decision.outcome over legacy should_run. |
| loopx/claude_goal_mode/scripts/goalmode_cmd.py | Updates status command rendering to prefer policy_decision.outcome. |
| loopx/claude_goal_mode/hooks/goal_policy.py | Adds resolve_should_run helper that prefers policy_decision when present. |
| loopx/chat_agent.py | Tightens envelope/proposals requirements for durable todo requests in agent prompt. |
| loopx/canary/module_metric_baseline.json | Updates module metric ceilings to reflect changed file sizes/Any usage. |
| loopx/bootstrap_command_pack.py | Adds detection of already-closed goals from rollout events and recommends fresh goal ids. |
Review details
- Files reviewed: 68/68 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if use_policy_engine is None: | ||
| use_policy_engine = _env_flag("LOOPX_USE_POLICY_ENGINE", default=master_switch_enabled()) | ||
| if record_policy_decisions is None: | ||
| record_policy_decisions = _env_flag( | ||
| "LOOPX_POLICY_DECISION_RECORD", default=master_switch_enabled() | ||
| ) |
| """Unified policy decision layer for the LoopX control plane. | ||
|
|
||
| RFC: Event-Driven Control Plane and Unified Policy Decision Architecture. | ||
|
|
||
| This package is a facade over the existing decision modules | ||
| (``quota/should_run``, ``agents/capability_gate``, ``agents/agent_scope``, | ||
| ``scheduler/execution_context``). It composes and normalizes their results | ||
| into a single stable ``Decision`` contract without reimplementing any domain | ||
| rules and without touching existing execution paths. |
| These tests cover the opt-in wiring added to ``build_live_quota_should_run_decision``: | ||
|
|
||
| * default behavior is unchanged (no ``policy_decision`` key, no env flags); | ||
| * enabling ``use_policy_engine`` attaches the unified decision and verifies | ||
| consistency against the legacy quota payload; | ||
| * enabling ``record_policy_decisions`` writes ``policy_decision`` audit events | ||
| to the goal rollout event log (transition-only, deduplicated); | ||
| * a deliberately diverged unified decision raises ``PolicyIntegrationError``. |
|
The DI approach is the right call versus blessing an ratchet exception, and the commit message's verification story (1205/1205 + m6 green + cache-free rerun) is thorough. Two issues found while tracing the fallback path, one of which contradicts the stated semantics: 1. The registry-missing path is fail-open, not fail-closed. 2. The factory is rebuilt on every claim. Minor: registering the factory as an import-time side effect of |
|
你好,frontend-control-plane-im-prototype-rfc 这个分支已经进主干了; |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
REQUEST_CHANGES,精确 head:451fd72407ce53b82898653c1370eb80fad1f842。
修掉 control_plane -> capabilities.catalog 外向依赖是合理目标,但实际 PR 不只是正文声称的 5 文件 +71/-2。相对当前 PR base 52af33e49e5bd329a69d8bc38c8623b0dd9d55d5,完整范围是 68 文件 +12754/-97,包含父提交 f6549a9e9f1f21cb1600de2765b461840c26a196 的整套事件驱动原型。不能把局部 M6 转绿当成整个迁移已可合并。
评审证据分层说明:本轮完整检查最终五文件 DI 增量、真实 claim/eligibility 与未改的 resident 调用方;继承范围对照父提交的完整评审,并重新验证下面的关键负例。历史全量测试不是本轮重跑结果,未重新完成全部迁移路径的基线 parity;这些证据缺口仍阻止批准。
改动思路
新增链路是 CLI composition root 注册进程级 factory,claim_next_task 未收到 registry 时调用 factory,再把结果传入 claim_next_eligible_task -> eligible_bridged。CLI dispatch 另有一个 lazy builder;测试在全局 conftest 注册 factory。队列仍由既有 JSONL writer 持久化,DI 没有改变队列格式或实现跨进程事务。
正向实测:已注册 issue-fix binding 和普通 shell token 任务在父版、当前版均可 claim;明确关闭 event-driven 时,三类输入均不改队列字节。负向则出现了重构语义差异:父版直接调用真实 claim 会构建 registry 并拒绝未知 pack;当前版若未先 import CLI,或 factory 抛错,会退回 token-only 匹配并持久化错误的 claim。由成功 payload、lease/owner 和独立磁盘读取共同确认,不是仅比较 reason code。
复用方向应是保留 composition-root 注入、复用现有 typed eligibility 所有者,不是让缺失依赖变成另一套宽松授权。resident 的 WorkerPool.acquire/drain 没有传 registry,其行为依赖进程是否曾导入 CLI;这不是稳定的调用合同。
具体改动
完整范围分为:调度/claim/lease/retry/resident/merge;统一 PolicyEngine 与 live quota;acceptance/closure/bootstrap/registry;checkpoint/replay、成本与状态读模型;Todo 事件写入、CLI 与 Claude/OpenCode/Pi host 接入;heartbeat/安装指令;迁移说明、模块指标和测试。它们都属于本 PR 的合并面,不因标题是 DI 修复而消失。
最终增量五文件:cli.py 注册工厂;cli_commands/starter_scheduler.py 构建并传递 registry;capabilities_bridge.py 加可清空的全局工厂及异常降级;scheduler/event_driven_dispatch.py 增加参数并移除直接 import;tests/conftest.py 在整个测试进程注入工厂。没有新增专门验证“无 CLI bootstrap”或“工厂失败”的断言。
关键代码讲解
capabilities_bridge.py::capability_registry_or_none:未注册和任意 factory 异常都返回 None。eligible_bridged仅在 registry 非 None 时检查绑定 pack 是否 ready,因此缺失依赖变成放行,而不是受控错误。event_driven_dispatch.py::claim_next_task:capabilities/lease 分支取上述 fallback,随后真正重写队列;原先总会构建 registry 的前置条件消失。tests/conftest.py的全局 wiring 正好遮蔽这个 fresh-process 场景。resident.py::_command_matches_worker_prefix与原codex_cli_scheduler._command_matches_allowed_prefix:前者只比首 token,后者比完整 token 序列。新 runner 继续以shell=True执行字符串,属于父提交尚未修复的边界问题。slash_command_install.py::_command_prompt_specs:不读取开关便投影 dispatcher 指令,且把“刚写入或完成”的 Todo 都传成--completed-todo-id。本轮实测 master=0/1 两侧都含该指令;运行时关闭不能隔离已经加载给 agent 的行为指导。
对主干的风险
P1:DI 增量破坏未知 capability binding 的 fail-closed 语义。 同一 synthetic queue,父 head 直接调用后仍 pending、字节不变;当前 head 不导入 CLI 时变为 claimed、owner/lease 写盘;导入 CLI 后恢复拒绝;factory 抛出合成 RuntimeError 后再次错误放行。最小修复:在生产 composition root 显式提供 registry,绑定任务缺少可信 registry 时拒绝或返回有上下文的错误;保留合法 plain-token 路径,不要只扩大 catch 或给测试补 bootstrap。增加 fresh subprocess、missing/raising factory、known/unknown pack、关闭模式与磁盘无效果矩阵。
P0:继承的 worker 执行边界仍可绕过。 本轮只用安全命令验证:允许前缀 git status 时,新 matcher 接受 git log、旧 matcher 拒绝;前缀 printf safe 时,新 matcher 接受 printf safe; printf second,真实 runner 输出 safesecond。未执行破坏性命令。需完整 argv 前缀校验并避免隐式 shell;不能把字符串首 token 当执行授权。
P1:default-off 不隔离安装指令。 开关为 0 时仍指示 agent 使用、显式开启新 dispatcher,并将刚添加 Todo 作为 completed input。最小修复是按明确启用的范围生成指导,区分 add 与 complete 意图,并对安装指令、heartbeat、CLI/schema、事件写入和 host consumers 做关闭态对照。可用性不等于激活或完成授权。
其余继承风险仍未获得新的放行证据:JSONL claim 的跨进程原子性、队列 capability 字段传播、局部 closure 与主线 replan/settlement 的权威关系、checkpoint/replay 的真实恢复调用方。父版评审已有具体反例;五文件补丁没有修复这些边界。本轮没有重跑全部并发故障或真实 host 生命周期,因此不把这些维度标成通过。
验证:先前独立 DI 24 个真实队列观察;本次相关 8 个测试文件 127 passed(含 M6),另有上述 matcher/安全 runner/安装指令 paired probes;git diff --check 通过。测试通过与新的 fresh-process 反例同时存在,正说明全局 fixture 不能替代生产入口语义验证。
我的整体评价
不批准当前完整 PR。局部依赖反转有价值,但既引入新的 fail-open,又携带尚未关闭的原型级执行、并发和终局风险。就“修 M6 一项外向依赖”的目标,完整 +12754 行合并面并不相称;最小可审修法应建立在受支持基线上,注入既有 registry 并保留拒绝语义,不夹带未验收的新控制面。
Future-facing pass:建议去掉重复 lazy factory 与隐式全局 bootstrap 依赖,在现有 composition root 明确传递权威输入;不增加通用服务定位框架。通用领域文案不是主要问题,主要问题是名称/说明声称安全与 opt-in,真实生产组合却授予更宽行为。完成上述修复后仍需重新审完整 exact head,不能只验证最后一条 finding。未执行合并。
English verdict: REQUEST_CHANGES at 451fd72407ce53b82898653c1370eb80fad1f842. The DI delta makes unknown capability bindings fail open without CLI bootstrap or when the registry factory fails; real queue readback proves ownership/state mutation despite passing tests. Revalidated inherited command-prefix/shell and disabled-install-guidance defects. 127 focused tests and diff check passed; full migration parity/concurrency/host lifecycle remain unverified. Isolate the dependency repair, preserve fail-closed semantics, and address the inherited full-PR blockers before approval.
问题
refactor/event-driven-control-plane(PR #3200)当前未通过 m6 可维护性棘轮:event_driven_dispatch.py直接from ...capabilities.catalog import build_capability_registry,违反control_plane/内禁止 importloopx.capabilities*的依赖规则,且该 finding 无 reviewed exception。修法:依赖注入,不加豁免
加 exception 会永久固化违规;改 ratchet 是削弱门禁。本 PR 走依赖注入——registry 由 ratchet 作用域外的合法层(cli)注入:
set_capability_registry_factory/capability_registry_or_none),沿用既有 P3 hook 模式;工厂异常降级为 token-only 匹配,绝不从 claim 路径抛出claim_next_task/build_event_driven_dispatch增加registry参数;capabilities/lease 路径在调用方未传时回退到工厂钩子;删除违规的 catalog importbuild_capability_registry为工厂(cli 层在 ratchet 作用域外,本就合法 import catalog)loopx.cliimport,而 fail-closed 语义(绑定 registry 未知的 pack 不可 claim)结构上依赖 registry 存在验证
基于 fork head
f6549a9e:tests/control_plane/全套清
__pycache__+-p no:cacheprovider复跑排除 stale bytecode 假绿;commit 后复验 1205/1205。测试说明
本 PR 不新增也不修改任何断言——修复的验收标准是让既有 gate 转绿,同时不破坏既有语义。具体分三类:
1. 既有 gate 转绿(本 PR 的目的)
test_m6_maintainability_ratchet_has_no_unreviewed_debt:修复前红(unreviewed_count: 1,control_plane outward dependency 违规);修复后绿。ratchet 本身零改动——不是让测试通过,是让代码不再违规。2. 既有语义保护(修复过程中曾红,证明这些测试有效)
test_claim_next_task_binding_fails_closed_for_unknown_pack:纯依赖注入版本(registry 默认 None)曾让此测试红——registry=None时 unknown pack 无从判定,token 匹配放行,fail-closed 语义崩。这个中间态红灯说明:fail-closed 语义结构上依赖 registry 存在,也是引入工厂钩子(而非只加参数)的直接原因。最终版本该测试 PASSED。3. 新增基础设施(非断言)
tests/conftest.py注册工厂一行:测试进程不经过loopx.cliimport,没有这行则 fail-closed 语义在测试环境下测不出来。这是 wiring,不是测试放宽——不改任何既有断言。复现命令(fork head
f6549a9e+ 本 PR):base 分支(
frontend-control-plane-im-prototype-rfc)对照:1299/1299 全绿——即 m6 红灯是 #3200 引入、本 PR 消除,非存量债务。备注
451fd72,5 files +71/−2