From 7e669bb071b5a56d3cb213d7a5d47c0d837dc74e Mon Sep 17 00:00:00 2001 From: tzhouam Date: Sat, 26 Sep 2026 16:10:33 +0800 Subject: [PATCH 1/2] Load builtin workflow steps when assembling registry --- doc/architecture/CODE_TOUR.md | 4 +-- .../SPEC/engine/steps/__init__.md | 29 +++++++-------- .../engine/steps/__init__.py | 36 +++++++++---------- .../engine/steps/_common.py | 5 +-- test/test_step_registration_boundary.py | 32 +++++++++++++++++ 5 files changed, 69 insertions(+), 37 deletions(-) create mode 100644 test/test_step_registration_boundary.py diff --git a/doc/architecture/CODE_TOUR.md b/doc/architecture/CODE_TOUR.md index 0b80d6fe1..734ab8bfb 100644 --- a/doc/architecture/CODE_TOUR.md +++ b/doc/architecture/CODE_TOUR.md @@ -124,8 +124,8 @@ TaskSpec 遇上仓库能力,解析成一条可执行流水线。 (name/kind/risk/handler)、`StepResult`(ok + 类型化 `FailureKind`,失败是 **值**不是异常)、`StepContext`(handler 能触碰的一切:settings/state/ params/run_dir/trace/llm)。六种失败类型走不同路由。`engine/registry.py` - (39)是"名字→handler"的唯一解析点;`engine/steps/__init__.py`(34)导入 - 即触发 `@step` 自注册、`register_builtin_steps` 刷进 registry; + (39)是"名字→handler"的唯一解析点;`engine/steps/__init__.py` 延迟加载 + 内建模块，`register_builtin_steps` 触发 `@step` 自注册并填充 registry; `engine/__init__.py`(8)导出。 - **`config.py`**(418)——`Settings`(pydantic,从 `.env`/环境,永不提交), 被塞进每个 `StepContext`:LLM 端点、仓库路径、引擎预算 diff --git a/doc/architecture/SPEC/engine/steps/__init__.md b/doc/architecture/SPEC/engine/steps/__init__.md index b4594d1a1..e6d530ada 100644 --- a/doc/architecture/SPEC/engine/steps/__init__.md +++ b/doc/architecture/SPEC/engine/steps/__init__.md @@ -1,39 +1,40 @@ # engine/steps/__init__.py —— 规范 - + `LOC ~34 · step 库聚合 · refactor-status: ok` ## 职责 -为了**注册副作用**而 import 每个 step 模块，并暴露 `register_builtin_steps`。 +暴露 `register_builtin_steps` 作为唯一的内建 step 模块加载与注册入口；单独 +import 本包不会加载写入型 workflow 模块。 ## 功能 -import `_common` + 全部 8 个领域模块（rebase 侧现为 `rebase_v3` + `rebase_knowledge`； -从而运行它们的 `@step`/`register_step` 装饰器， -填充 `_common._COLLECTED`）；`register_builtin_steps` 把这份集合冲刷进一个 +import `_common` 而不加载领域模块。调用 `register_builtin_steps` 时显式加载 +8 个领域模块（rebase 侧为 `rebase_v3` + `rebase_knowledge`），触发各模块的 +`@step`/`register_step` 装饰器填充 `_common._COLLECTED`，然后把集合写入给定 `StepRegistry`。 ## 公开契约 `register_builtin_steps(registry) -> registry`。 ## 不变量 -- import 这个包会把所有 step **恰好注册一次**（靠模块缓存）；对每个全新 registry， - `register_builtin_steps` 是幂等的（**A4**）。 -- **这份 import 列表就是 step 模块的权威集合** —— 新的 step 模块必须加到这里才会被发现。 +- import 这个包不会加载领域模块；首次组装 registry 才加载全部内建模块。 + Python 模块缓存保证装饰器只运行一次；每个全新 registry 都得到同一集合（**A4**）。 +- **`_BUILTIN_MODULES` 就是内建 step 模块的权威集合** —— 新模块必须加入它。 ## 边界 —— 不属于这里 不含 step 逻辑；除冲刷之外不含注册策略。 ## 依赖（允许） -`engine/registry`、`._common`，以及那 8 个 step 模块。 +`engine/registry`、`._common`；只有调用注册函数时才导入那 8 个模块。 ## 扩展点 -新的 step 领域模块 → 加进这份副作用 import 列表。 +新的 step 领域模块 → 加进 `_BUILTIN_MODULES`。 ## 测试 -每次 `register_builtin_steps(StepRegistry())` 调用都会覆盖（冒烟：45 个 step）。 +隔离子进程验证 import 惰性；`register_builtin_steps(StepRegistry())` 的现有 +组装测试覆盖内建 step 集合。 ## 重构备注 -可以考虑自动发现（遍历这个包）以去掉手工 import 列表 —— 但显式 import **可 grep**， -且拼错时会大声失败，所以当前形式是可以接受的。若保持手工，这份列表 + `_COLLECTED` -就是"到底存在哪些 step"的唯一来源。 +保留显式模块列表，避免自动发现把未经审核的 step 带入 registry；模块名拼错在 +组装时大声失败。`_BUILTIN_MODULES` + `_COLLECTED` 是内建 step 集合的来源。 diff --git a/src/infermatrix_copilot/engine/steps/__init__.py b/src/infermatrix_copilot/engine/steps/__init__.py index eba2b2154..9de817061 100644 --- a/src/infermatrix_copilot/engine/steps/__init__.py +++ b/src/infermatrix_copilot/engine/steps/__init__.py @@ -1,34 +1,32 @@ -"""Vetted step library, self-registering via the `@step` decorator. +"""Vetted step library, assembled explicitly when a registry is requested. -Importing this package imports every step module for its registration side -effects (each `@step` / `register_step` records a `StepSpec`); -`register_builtin_steps` flushes the collected specs into a `StepRegistry`. -There is no central `add(StepSpec(...))` block to keep in sync — a step's name, -metadata and handler live together at its definition (see doc/architecture/CODE_TOUR.md §5). +Each module still keeps its step name, metadata and handler together with +`@step` / `register_step`. Importing this package alone is inert; only +`register_builtin_steps` loads the vetted modules and fills a `StepRegistry`. +This keeps application and SDK imports from loading every workflow effect. """ from __future__ import annotations +from importlib import import_module + from ..registry import StepRegistry from . import _common -# side-effect imports: each module registers its steps into the collection. -from . import ( # noqa: F401,E402 - workspace, - review, - report, - pr, - issue, - profile, - rebase_v3, - rebase_knowledge, +_BUILTIN_MODULES = ( + "workspace", "review", "report", "pr", "issue", "profile", + "rebase_v3", "rebase_knowledge", ) def register_builtin_steps(registry: StepRegistry) -> StepRegistry: - """Flush every step collected by import side effects into `registry` and - return it. Called once at startup, after this package's module imports have - populated the collection; the registry then holds the full vetted library.""" + """Load the vetted modules and install their collected specs into `registry`. + + Module imports are cached by Python, so repeated registry assembly uses + the same step definitions without a second registration side effect. + """ + for name in _BUILTIN_MODULES: + import_module(f"{__name__}.{name}") for spec in _common.collected(): registry.register(spec) return registry diff --git a/src/infermatrix_copilot/engine/steps/_common.py b/src/infermatrix_copilot/engine/steps/_common.py index dd41795dc..3163ac1bb 100644 --- a/src/infermatrix_copilot/engine/steps/_common.py +++ b/src/infermatrix_copilot/engine/steps/_common.py @@ -4,8 +4,9 @@ 1. the `@step` / `register_step` **self-registration** surface — decorating a handler (or calling `register_step` for factory-built handlers) records a - `StepSpec` into the package-level collection; `steps.register_builtin_steps` - flushes that collection into a `StepRegistry`. There is no separate + `StepSpec` into the package-level collection when its module is loaded; + `steps.register_builtin_steps` loads the vetted modules and flushes that + collection into a `StepRegistry`. There is no separate `add(StepSpec(...))` block to keep in sync anymore. 2. the cross-module helpers every step file shares (`gh`, `repo_path`, `git`, `task_spec`, `gh_read_tools`, `post_step`) — one home instead of a diff --git a/test/test_step_registration_boundary.py b/test/test_step_registration_boundary.py new file mode 100644 index 000000000..5b8e740e9 --- /dev/null +++ b/test/test_step_registration_boundary.py @@ -0,0 +1,32 @@ +"""Importing the registry facade must not load workflow effect modules.""" + +import os +import subprocess +import sys +from pathlib import Path + + +def test_builtin_steps_load_only_when_registry_is_assembled(): + source = Path(__file__).resolve().parents[1] / "src" + code = """ +import sys +from infermatrix_copilot.engine.steps import register_builtin_steps +from infermatrix_copilot.engine.registry import StepRegistry + +prefix = 'infermatrix_copilot.engine.steps.' +for name in ('workspace', 'review', 'report', 'pr', 'issue', 'profile', + 'rebase_v3', 'rebase_knowledge'): + assert prefix + name not in sys.modules, name + +registry = register_builtin_steps(StepRegistry()) +assert {'workspace.guard_clean', 'review.patch_gate', 'pr.fetch_diff'} <= set(registry.names()) +for name in ('workspace', 'review', 'report', 'pr', 'issue', 'profile', + 'rebase_v3', 'rebase_knowledge'): + assert prefix + name in sys.modules, name +""" + result = subprocess.run( + [sys.executable, "-c", code], + env={**os.environ, "PYTHONPATH": str(source)}, + capture_output=True, text=True, timeout=30, + ) + assert result.returncode == 0, result.stderr From 93e1577e189a21ee4e844e4730f62f6103d4679a Mon Sep 17 00:00:00 2001 From: tzhouam Date: Sat, 26 Sep 2026 16:14:32 +0800 Subject: [PATCH 2/2] docs: align step registration spec with lazy assembly --- doc/architecture/SPEC/engine/steps/_common.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/architecture/SPEC/engine/steps/_common.md b/doc/architecture/SPEC/engine/steps/_common.md index 046645d46..da1776c5e 100644 --- a/doc/architecture/SPEC/engine/steps/_common.md +++ b/doc/architecture/SPEC/engine/steps/_common.md @@ -1,11 +1,13 @@ # engine/steps/_common.py —— 规范 - + `LOC ~300 · step 库基础设施 · refactor-status: ok` ## 职责 -step 的自注册面，以及各 step 文件共享的跨模块 helper。 +step 的自注册面，以及各 step 文件共享的跨模块 helper。内建 step 模块由 +`steps.register_builtin_steps` 显式加载；加载模块时装饰器收集定义，再写入 +`StepRegistry`。 ## 功能 `@step`/`register_step` 装饰器 + `_COLLECTED` + `collected()`； @@ -17,7 +19,7 @@ helper：`repo_path`、`require_repo`、`task_spec`、`from_state`、`published` 以及上述 helper（含 K3/K4/K7 的守卫 helper）。 ## 不变量 -- step 名重复 → **在 import 时抛错**（**A4**）。 +- step 名重复 → **在显式装配期间加载 step 模块时抛错**（**A4**）。 - 这是 step 共享 helper 的**唯一**归处 —— step 模块从这里 import， **绝不互相 import**（**A2**）。 - helper 保持轻薄、对副作用诚实、且仓库中立。