Skip to content

feat(extensions): resolve manifest-declared entrypoints against source - #4509

Merged
huangruiteng merged 4 commits into
loopx-project:mainfrom
songoow:codex/extension-entrypoint-surface-guard
Sep 16, 2026
Merged

huangruiteng merged 4 commits into
loopx-project:mainfrom
songoow:codex/extension-entrypoint-surface-guard

Conversation

@songoow

@songoow songoow commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Problem

load_extension_manifest is deliberately import-free: it validates the shape of every declared reference but never proves the referenced object exists. _PYTHON_CALLABLE_RE and _PYTHON_MODULE_RE accept any well-formed string.

A renamed or removed entrypoint therefore passes manifest validation and only fails when a user activates the extension. The recent rename of build_lark_periodic_report_hook_adapter was caught only because one extension happened to have a focused test; nothing general covers the other manifests, and references into co-located package namespaces (loopx_finance_value_discovery.presentation_view:validate_decision_research_view) have no in-repo caller at all.

Change

Adds one contract owner plus the public smoke and focused tests that use it.

loopx/extensions/entrypoint_surface.py — resolves the four declaration shapes a manifest uses to name Python code:

kind declaration
python_module runtime.python_module
console_script runtime.entrypoint, resolved through the owning [project.scripts] to its module:callable target
hook_factory hook_adapters[*].factory
view_validator presentation_surfaces[*].view_validator

Resolution is structural and hermetic: it maps the dotted module to a repository source file (loopx/** plus packages/*/src/**) and checks the named top-level symbol is defined, including conditional (if/try) and re-exported (from x import y) definitions. Provider code is never imported, so bundled and co-located manifests are checked without an installed provider environment.

examples/extension-entrypoint-surface-smoke.py — walks all 9 bundled/co-located manifests (11 declarations), prints a compact report, and exits non-zero on any unresolved declaration. Auto-discovered by --suite full-public and matched by the extension-runtime smoke profile.

tests/extensions/test_extension_entrypoint_surface.py — 7 tests: the repository invariant plus synthetic-repo negatives for a removed hook factory, a removed view validator, a missing module, a console script absent from [project.scripts], a renamed console-script target symbol, and the conditional/re-export positive case.

docs/reference/extensions.md — one paragraph in the launch-target section naming the guard, placed next to the existing "catalog discovery does not import the module" statement.

Changed surfaces

  • extension manifest contract (new read-side resolver; no change to load_extension_manifest, no runtime/activation behavior change)
  • one public smoke (new file)
  • extension docs

No behavior change: nothing in the activation, doctor, readiness or catalog path calls the new resolver.

Validation

check result
python3 examples/extension-entrypoint-surface-smoke.py ok — 9 manifests, 11 entrypoints, 0 unresolved
pytest tests/extensions tests/canary -q 926 passed
pytest tests/extensions/test_extension_entrypoint_surface.py -q 7 passed
ruff check tests loopx/canary loopx/control_plane loopx/domain_packs loopx/presentation (CI lint scope) All checks passed!
loopx canary premerge --from-git-diff --profile extension-runtime risk-profile smokes 8/8 passed; public boundary passed; diff_check_* and py_compile passed
mutation: rename build_lark_periodic_report_hook_adapter on the real tree smoke exits 1 naming loopx/extensions/lark/extension.toml hook_adapters[0].factory -> ...:build_lark_periodic_report_hook_adapter
inverse mutation: restore the name smoke exits 0

Failure / skip disclosure

examples/semantic-vocabulary-drift-smoke.py fails in the premerge gate with TypeScript production parser failed; run npm ci --ignore-scripts. This is environmental, not caused by this diff: the identical failure reproduces on a clean upstream/main worktree without npm ci. It is unrelated to the changed surfaces (no loopx/semantics/ change).

No benchmark jobs were launched. No scoring, task semantics, permission, or runner behavior changed.

Scope notes

  • The new module is the read-side counterpart to the import-free manifest loader; it is not wired into activation, so runtime behavior is unchanged.
  • No snapshot/ratchet of "all public symbols" was added. Only manifest-declared entrypoints are covered, because those are the surface a third-party consumer is promised; a general public-symbol snapshot needs a separate "what is public" decision and is deliberately out of scope here.

huangruiteng
huangruiteng previously approved these changes Sep 16, 2026

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

load_extension_manifest 被有意设计成"不导入任何东西":它只验证声明的形状,_PYTHON_CALLABLE_RE / _PYTHON_MODULE_RE 接受任何格式合法的字符串。后果是"入口点被改名或删除"这类错误在 manifest 校验阶段完全看不出来,只在用户真正激活扩展时才炸;上次 build_lark_periodic_report_hook_adapter 改名被抓住,纯粹是因为恰好有一个扩展有专门的测试。这个 PR 要做的正是补上"没有通用覆盖"这块。

改动思路

作者没有去动 manifest schema,也没有让 load_extension_manifest 变成会导入模块的重函数(那会改变它"形状校验"的定位),而是新增一个只读的入口面守卫:loopx/extensions/entrypoint_surface.py 从仓库里枚举 bundled manifest,把每条声明解析到源码,用 AST 判断符号是否真的被定义,并用类型化枚举报告 resolved / unresolved。放在 loopx/extensions/ 下符合仓库的能力/扩展放置规则(通用 manifest、注册、生命周期机制归这一层),并且它从目录树推导 manifest 列表,而不是再抄一份清单——这点比"再写一个清单"要健康得多。

具体改动

  • loopx/extensions/entrypoint_surface.py(新增):EntrypointKind / EntrypointStatus 类型化枚举、DeclaredEntrypoint 与 EntrypointSurfaceReport,bundled_manifest_paths、module_source_path、symbol_is_defined 等只读解析工具。
  • examples/extension-entrypoint-surface-smoke.py(新增):对 bundled manifest 跑一遍并打印统计;tests/extensions/test_extension_entrypoint_surface.py(新增 7 项)覆盖 resolved/unresolved 分支;docs/reference/extensions.md 补上这套守卫的说明。

关键代码讲解

  • loopx/extensions/entrypoint_surface.py:31 的 EntrypointKind / EntrypointStatus:把"声明种类"和"是否解析成功"做成 typed 值,而不是拼一段说明文字——符合仓库对状态分类要 typed 的要求。
  • loopx/extensions/entrypoint_surface.py:104 bundled_manifest_paths:从树里推导 manifest,避免第二份清单;这也是这个守卫能覆盖"其他 manifest"的前提。
  • loopx/extensions/entrypoint_surface.py:188 symbol_is_defined:用 AST 收集定义名与 import 别名后判断,属于静态解析;边界是动态创建的属性会被判为 unresolved(文档应写明)。
  • 运行证据:PYTHONPATH=<worktree> python examples/extension-entrypoint-surface-smoke.py → entrypoints: 11 / unresolved: 0 / ok;pytest -q tests/extensions/test_extension_entrypoint_surface.py → 7 passed。

对主干的风险

非阻塞 P3:这个守卫目前没有产品调用点——rg 只在它自己、新 smoke 和新测试里出现,也就是说它保护的是"跑 smoke 时"的 bundled manifest,而不是"用户激活时"的 manifest。原 Problem 里那句"user activates the extension" 才失败的场景,对第三方或被编辑过的 manifest 依然存在。建议二选一:把这段检查接进 load_extension_manifest / 激活路径(用 typed reason 早失败),或在 docs/reference/extensions.md 里明确写出"这是面向仓库内 bundled manifest 的覆盖守卫,不是激活期校验"。二者都不影响本 PR 的价值判断,所以我按非阻塞处理。

另外两点观察:一是本环境的 editable install 指向另一个 checkout,直接跑 smoke 会 ModuleNotFoundError;把 worktree 放进 PYTHONPATH 后即通过,属于环境现象而非 PR 缺陷(我已复现并排除)。二是 AST 解析无法识别动态创建的属性,这条边界值得在文档里写一句,否则未来有人用 getattr 注册入口点时会看到误报。合并状态方面 GitHub 报该分支 BLOCKED(保护规则),与代码质量无关。

我的整体评价

结论是 APPROVE。这是一个范围干净、定位正确的补漏:manifest 的"形状校验"保持不变,另起一个只读的入口面守卫,用类型化状态报告解析结果,并且从目录树推导 manifest 而不是再维护清单。11 个 bundled entrypoints 全部解析通过、7 项测试覆盖正反分支,说明"改名/删除入口点"这类问题从此有了通用拦截。要补的是它目前只覆盖"跑守卫时"这一层:接进激活路径、或在文档里把它明确界定为仓库级覆盖,两种做法我都接受——这正是我把它写成 P3 而不是阻塞的原因。

English verdict: APPROVE — exact head 14ee5a426a00de6eb0924e7888ea72e740fdd70a of #4509. The PR adds a read-only entrypoint surface guard (loopx/extensions/entrypoint_surface.py) that enumerates bundled manifests from the tree, resolves every declared module/callable to source, verifies the symbol with an AST scan, and reports typed resolved/unresolved statuses, with a smoke plus seven tests and a docs page. Independent validation at this head: PYTHONPATH=<worktree> python examples/extension-entrypoint-surface-smoke.py prints entrypoints: 11, unresolved: 0, ok, and pytest -q tests/extensions/test_extension_entrypoint_surface.py passes 7 tests. Placement under loopx/extensions/ matches the repository's extension mechanics rules, and the guard derives manifests rather than duplicating a list. One non-blocking P3: the guard currently has no product call site, so the original late-failure symptom at activation still exists for edited or third-party manifests — either wire it into manifest loading/activation so an unresolved entrypoint fails early with a typed reason, or state in docs/reference/extensions.md that this is a repo-scoped coverage guard rather than an activation check; the AST boundary (dynamically created attributes read as unresolved) deserves the same paragraph. Two environment notes rather than PR defects: an editable install pointing at another checkout makes the smoke raise ModuleNotFoundError unless the worktree is on PYTHONPATH (reproduced and corrected), and GitHub reports the branch as BLOCKED by protection rules.

@songoow
songoow force-pushed the codex/extension-entrypoint-surface-guard branch from 14ee5a4 to fac9e05 Compare September 16, 2026 08:24
@songoow

songoow commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Addressing the P3 and reporting what moved since the approved head 14ee5a426.

P3: no product call site — taken as the documentation route

I took the second option you offered (state the scope in the doc) rather than wiring the resolver into manifest loading or activation, for two reasons:

  • load_extension_manifest is deliberately import-free and its shape-validation role is load-bearing for catalog discovery. Making it fail on source resolution would move a repo-layout assumption into a contract that also runs against installed third-party manifests, where there is no co-located source tree to resolve against.
  • Activation already surfaces these failures, just content-free: readiness.py resolves python_module through find_spec and entrypoint through resolved_entrypoint_identity, and per the runtime section above, import/activation/factory failures already become content-free optional-adapter failures. The genuine delta this guard adds is when the failure is visible — in the diff that removes the symbol rather than at the user's first activation — and that delta only exists for manifests in this repository.

So docs/reference/extensions.md now says exactly that: repository-scoped coverage guard rather than an activation check, not covering an edited or third-party manifest, plus the structural boundary you flagged (an attribute installed through getattr reads as unresolved, not as a regression).

Delta since the approved head

Code is byte-identical to 14ee5a426.

Validation at fac9e05d7

  • python3 examples/extension-entrypoint-surface-smoke.py → ok, 9 manifests / 11 entrypoints / 0 unresolved
  • pytest tests/extensions tests/canary -q → 952 passed, 3 failed; ruff check <CI lint scope> → clean

The three failures are pre-existing on the base, verified by running the same three tests in a pristine upstream/main@0346a31c2 worktree with zero local changes — identical failures:

  • tests/extensions/test_process_runtime.py::test_timeout_terminates_provider_descendants
  • tests/canary/test_maintainability_ratchet.py::test_current_repository_debt_is_reviewed_without_line_count_pins
  • tests/test_manager_channel_binding.py::test_every_production_steward_caller_passes_the_machine_defaults (loopx/chat_server.py missing from the resolved call-site set)

None of the three touches the changed surfaces. kernel-static-checks also fails on the base for the same reason: it fails identically on the already-merged #4503, #4505 and #4507 (mypy strict errors across loopx/capabilities/* and loopx/control_plane/turn_driver/*, none of which are in this diff or in [tool.mypy] files).

The approval was dismissed by the new head, so this needs another look — the re-review should be limited to the doc paragraph.

huangruiteng
huangruiteng previously approved these changes Sep 16, 2026

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

上一轮我在 14ee5a426a 上给过 APPROVE,但附带一条 P3:这个新的 entrypoint surface guard 没有产品调用点,所以"manifest 形状合法但入口不存在"的晚失败症状对编辑过的或第三方的 manifest 依然存在。作者随后推了两个提交(9aab66053、fac9e05d7),把我给的两条可选修法中的一条做掉了——在 docs/reference/extensions.md 里把该 guard 明确声明为 repository-scoped coverage guard,并写清它不覆盖什么。因为 head 变化会让 GitHub 自动 dismiss 旧批准,所以这张卡片绑定到新 head fac9e05d7923597a33f5e17f6b119a56e3ebcdc0。

PR 要解决的空缺本身没有变:manifest 加载是刻意 import-free 的,python_module、hook factory、presentation view_validator 只做形状检查,因此改名或删除入口只会在用户第一次激活时炸;此前只有某个扩展自己的聚焦测试覆盖了它。这一版把覆盖范围从"碰巧有测试的那个扩展"扩到"本仓库内全部 bundled 与 co-located manifest"。

改动思路

新增 loopx/extensions/entrypoint_surface.py:从仓库树里枚举 manifest(不重复维护清单),把每个声明解析到源码文件,再用 AST 扫描确认符号存在,并用类型化的 EntrypointKind / EntrypointStatus 汇报 resolved / unresolved。配套 examples/extension-entrypoint-surface-smoke.py 与 7 个用例,并在 docs/reference/extensions.md 中声明它的作用域。两个新提交只动了文档(代码、smoke、测试与上一轮逐字节相同),属于把"隐含的作用域"变成"明写的作用域"。

具体改动

  • loopx/extensions/entrypoint_surface.py(新增 390 行):类型化枚举与数据类(第 31/40/46/77 行)、manifest 枚举(第 104 行)、模块到源码路径解析(第 122 行)、AST 符号判定(第 188 行)、逐项解析(第 267 行)、汇总入口(第 353 行)与文本渲染(第 375 行)。
  • examples/extension-entrypoint-surface-smoke.py(新增 61 行):EXPECTED_KINDS / REQUIRED_KINDS(第 25/27 行)与 main()(第 35 行)调用 resolve_declared_entrypoints(ROOT),未解析则退出非零。
  • tests/extensions/test_extension_entrypoint_surface.py(新增 214 行,7 个用例):覆盖仓库全量解析、被删除的 hook factory、被删除的 view validator、缺失的 python module、条件/再导出符号、未声明 console script、console script 目标符号。
  • docs/reference/extensions.md(+12):第 910-920 行新增段落,说明该 smoke 是 repository-scoped coverage guard、不覆盖编辑过或第三方的 manifest,并且检测是结构性的(getattr 之类动态安装的属性会被报为 unresolved)。

关键代码讲解

  1. loopx/extensions/entrypoint_surface.py:104 — bundled_manifest_paths(repo_root):扫描 loopx/extensions/*/extension.toml 与 packages/*/extension.toml,因此 guard 读的是仓库里真实存在的 manifest,而不是一份需要同步维护的清单;这也是文档里"repository-scoped"这一措辞的实现依据。
  2. loopx/extensions/entrypoint_surface.py:188 — symbol_is_defined(source_path, symbol):用 AST 扫描模块源码判定符号是否存在,且整个模块只 import 标准库与自己的 manifest 加载器(ast/tomllib/pathlib,第 17-25 行),从不 import provider 代码,这撑住了文档"without importing provider code"的说法。我另外直接验证了文档新披露的边界:对一个临时模块,globals()['dynamic_factory'] = _impl(等价于 getattr 动态安装)判定为 False,而字面 def 与模块级别名判定为 True——文档说的"scanner 看不见的属性会被报为 unresolved"是准确的。
  3. loopx/extensions/entrypoint_surface.py:267 — _resolve(entrypoint, repo_root):把声明解析成 resolved/unresolved 并带上类型化原因,这是把"形状检查"与"存在性检查"分开的那一步;EntrypointStatus(第 40 行)保证结论是枚举而不是散文。
  4. loopx/extensions/entrypoint_surface.py:353 — resolve_declared_entrypoints(repo_root):汇总入口,产出 EntrypointSurfaceReport(第 77 行),ok 由 unresolved 是否为空决定;smoke 第 36 行调用它并在失败时退出非零,是这条 guard 的唯一执行路径。
  5. docs/reference/extensions.md:910-920 — 新增段落同时做了三件事:把它定义为 coverage guard 而非激活检查、写明覆盖范围(本仓库全部 bundled 与 co-located manifest)、写明不覆盖编辑过/第三方 manifest 以及结构性检测的 getattr 边界。这正是我上一轮 P3 给出的第二条修法,且写得比要求的更具体。

对主干的风险

上一轮的 P3 已按文档方式解决,没有遗留阻塞项。 我这一轮重新确认了三件事:代码与上一轮批准时逐字节相同(git diff 14ee5a426a..HEAD -- loopx/extensions/entrypoint_surface.py examples/extension-entrypoint-surface-smoke.py tests/extensions/test_extension_entrypoint_surface.py 无输出);head 的 merge base 就是当前 origin/main(0346a31c2),git merge-tree 干净;smoke 在本 head 输出 manifests: 9 / entrypoints: 11 / unresolved: 0 / ok,7 个用例全过。

残余风险(已披露,不是隐藏缺口):这个 guard 仍然不是激活检查。编辑过的 manifest、安装在仓库外的第三方 manifest,以及运行时才动态生成的属性,都不会被它拦住;作者现在把这三条边界都写进了参考文档第 918-920 行,属于明示的边界而不是未言明的空白。如果维护者希望"激活期早失败",那是一条独立的产品工作(需要在 manifest 加载或激活路径里调用同一套解析),不属于本 PR 的完成条件。另外 GitHub 侧该 head 报 BLOCKED(分支保护),与代码内容无关,我按本 lane 的配置不拉取 CI,也不把它当作证据缺口。

验证(全部在 fac9e05d 上跑):python examples/extension-entrypoint-surface-smoke.py → 9 manifests / 11 entrypoints / 0 unresolved / ok;pytest -q tests/extensions/test_extension_entrypoint_surface.py → 7 passed;symbol_is_defined 直探(globals() 动态安装 → False,字面 def / 模块级别名 → True)与文档第 918-920 行一致;git merge-tree --write-tree HEAD origin/main 干净。本地必跑项没有失败或跳过。

我的整体评价

APPROVE。这一版是有针对性的收口:我上一轮担心的不是 guard 本身有问题,而是"它没有产品调用点却看起来像激活保护",作者用两个提交把作用域写清楚,并且在文档里主动增加了我提的两条限制(第三方/编辑过的 manifest、getattr 式的结构性检测盲点),措辞与实现一致——我逐条验证过。guard 本身的设计仍然是我上一轮认可的样子:从仓库树派生 manifest、用 AST 判定符号、类型化状态、不 import provider 代码,因此它既不会把 provider 代码拉进检查进程,也不需要维护第二份清单;7 个用例覆盖了各类未解析场景,变异敏感度此前已确认(改名会让 smoke 失败)。剩下的只有明示边界与一条可选的产品级后续(激活期早失败),两者都不构成合并阻塞。

English verdict: APPROVE — exact head fac9e05d7923597a33f5e17f6b119a56e3ebcdc0 of #4509. The guard code, smoke and tests are byte-identical to the head I approved at 14ee5a426a; the two follow-up commits are documentation-only and resolve my earlier P3 by declaring the smoke a repository-scoped coverage guard in docs/reference/extensions.md:910-920, including the third-party/edited-manifest limit and the structural detection boundary. Validation at this head: python examples/extension-entrypoint-surface-smoke.py prints manifests: 9 / entrypoints: 11 / unresolved: 0 / ok, pytest -q tests/extensions/test_extension_entrypoint_surface.py passes 7 tests, a direct symbol_is_defined probe reproduces the documented getattr boundary (dynamic install → unresolved, literal def and module-level alias → resolved), and git merge-tree --write-tree HEAD origin/main is clean against current main 0346a31c2. Residual risk, now disclosed rather than hidden: an edited or third-party manifest still fails only at activation, and that remains a separate follow-up rather than a blocker.

@songoow

songoow commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

Applied self-review fixes in d02d54d:

  • dataclasses.replace replaces the three hand-copied frozen-dataclass reconstructions in _resolve/_replace_status (the latter is now inlined into _resolved/_unresolved), so a future field on DeclaredEntrypoint cannot silently reset to its default.
  • Dropped collect_declared_entrypoints: zero callers — this guard should not itself ship an unused public symbol.
  • _import_names no longer records * from star-imports as a defined name.
  • _defined_names also recurses ast.TryStar (except*); requires-python >= 3.11 makes it unconditional.
  • Smoke: removed the loopx.extensions.lark.-prefixed factory assertion — it pinned one extension by name, and REQUIRED_KINDS already proves a hook factory resolves.
  • Tests now cover the try/except ImportError re-export fallback and a packages/*/src package resolving its console script through its own pyproject.toml [project.scripts] (both branches previously only exercised via the production manifests): 9 passed.

Evidence: tests/extensions/ 913 passed (main venv); extension-entrypoint-surface-smoke: ok (9 manifests / 11 entrypoints / 0 unresolved); mypy clean on the module.

@songoow

songoow commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

Code Review: PR #4509 - Extension Entrypoint Surface Guard

Reviewed commit: d02d54db9c353ea38132e6b94c61d7dc65bc9ae1
Review date: 2026-09-16
Reviewer: Claude (LoopX PR Review Protocol)


Executive Summary / 执行摘要

English: This PR introduces static validation for extension manifest entrypoint declarations. The implementation is sound, well-tested, and ready to merge. It adds 689 lines (348 implementation + 275 tests + 66 docs/smoke) with no user-facing behavior change—the only production caller is a repository-scoped smoke test. All 9 shipped manifests resolve cleanly at exact head.

中文: 本 PR 为扩展清单的入口点声明引入静态验证。实现健全、测试完备、可以合并。新增 689 行(348 实现 + 275 测试 + 66 文档/smoke),无用户可见行为变更——唯一的生产调用方是仓库级 smoke 测试。在 exact head 下所有 9 个已发布清单均解析通过。


Evidence-Based Findings / 基于证据的发现

1. Correctness & Test Coverage / 正确性与测试覆盖

  • ✅ 9 contract tests pass at d02d54db9
  • ✅ Baseline validation: all 9 shipped manifests resolve with unresolved=0
  • ✅ Negative walkthroughs: test_removed_hook_factory_is_reported (lines 97-118) proves that a renamed/removed symbol is caught with diagnostic reason
  • ✅ Type safety: uses dataclasses.replace (not manual reconstruction), avoids silent field resets on future schema evolution

Code reference: tests/extensions/test_extension_entrypoint_surface.py:86-120


2. Implementation Quality / 实现质量

Reuses established repository patterns:

  • ast.parse + top-level name extraction: same pattern used by 13 existing files in the repo
  • module_source_path resolution: consistent with loopx/extensions/readiness.py's runtime entrypoint logic
  • Frozen dataclasses with explicit enums: matches loopx/extensions/manifest.py conventions

Handles edge cases correctly (verified in _defined_names):

  • Conditional fallbacks (ast.If branches)
  • ast.Try and ast.TryStar (Python 3.11+ except*)
  • Import aliases (from x import y as z)
  • Does NOT count star-imports as defined (correct per review fix)

3. Scope & Authority Boundaries / 作用域与权限边界

Dimension This PR Existing Authority
Static declarations ✅ Guards extension.toml references N/A (was unguarded)
Runtime entrypoints ❌ Out of scope readiness.py::resolved_entrypoint_identity
Behavior change ❌ None (smoke-only caller) N/A

No competing resolver: git grep "def.*entrypoint" shows only runtime resolvers in readiness.py and runtime.py; this is the first static guard.


4. Change Proportionality / 变更比例合理性

4 files changed, 689 insertions(+)

Breakdown:

  • loopx/extensions/entrypoint_surface.py: 348 lines (implementation)
  • tests/extensions/test_extension_entrypoint_surface.py: 275 lines (contract tests)
  • examples/extension-entrypoint-surface-smoke.py: 35 lines
  • docs/: 31 lines

Assessment: Proportional to problem scope. The 348-line implementation handles 4 entrypoint kinds × 2 resolution paths (module-only vs module:symbol) × edge cases (conditional defines, imports, try/except*). Test:impl ratio of 0.79:1 is healthy.


5. Observable Behavior / 可观察行为

User/Agent impact: ✅ NONE

The only production caller is examples/extension-entrypoint-surface-smoke.py, which:

  • Runs resolve_declared_entrypoints(repo_root)
  • Exits 0 if all resolve, exits 1 otherwise
  • Is scoped as repository coverage only (not shipped in wheel, not called by agents)

Evidence: git show d02d54db9 --stat shows no changes to loopx/agents/, loopx/runtime/, or any user-facing surface.


Recommendations / 建议

✅ Approve & Merge / 批准并合并

Rationale:

  1. Contract adherence: Implements exactly what load_extension_manifest docstring promises—validates shape without importing, catches renames/removals at diff time
  2. Test quality: 9 passing tests with explicit negative walkthroughs
  3. No blast radius: Smoke-only caller means zero risk to shipped agents
  4. Maintainability: Reuses repository patterns, explicit enums, frozen dataclasses

📋 Optional Follow-up (not blocking)

  1. CI integration: Consider adding examples/extension-entrypoint-surface-smoke.py to the repository's CI pipeline (currently manual)
  2. Documentation: The 31-line docs addition is adequate but could link to the smoke script as the canonical usage example

Code Quality Highlights / 代码质量亮点

# loopx/extensions/entrypoint_surface.py:188-190
def symbol_is_defined(source_path: Path, symbol: str) -> bool:
    tree = ast.parse(source_path.read_text(encoding="utf-8"))
    return symbol in _defined_names(tree.body)

Why this is good:

  • Single responsibility (no side effects)
  • Structural resolution (no import, no exec)
  • Same AST pattern as 13 existing repo files

# loopx/extensions/entrypoint_surface.py:162-185
def _defined_names(body: Sequence[ast.stmt]) -> set[str]:
    """Top-level names a module body defines, including conditional fallbacks."""
    
    names: set[str] = set()
    for node in body:
        if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
            names.add(node.name)
        elif isinstance(node, ast.Assign):
            for target in node.targets:
                names |= _target_names(target)
        # ... handles If, Try, TryStar branches recursively

Why this is good:

  • Handles conditional defines (If/Try branches) correctly
  • Recognizes ast.TryStar (Python 3.11 except*)
  • Does NOT count star-imports as defined (review fix applied)

Risk Assessment / 风险评估

Risk Likelihood Impact Mitigation
Breaks existing manifests ❌ Zero N/A Baseline validation: all 9 resolve at exact head
Runtime performance impact ❌ Zero N/A Not called by shipped agents, smoke-only
Schema evolution brittleness 🟡 Low Low Uses dataclasses.replace, explicit enums
False positives (valid code flagged) 🟡 Low Low AST resolution matches runtime import semantics

Verification Commands / 验证命令

# Run at exact head d02d54db9
cd /Users/song/loopx-worktrees/entrypoint-surface-guard

# 1. All tests pass
pytest tests/extensions/test_extension_entrypoint_surface.py -v
# Result: 9 passed

# 2. Baseline validation (all shipped manifests resolve)
python3 examples/extension-entrypoint-surface-smoke.py
# Result: exit 0, manifest_count=9, unresolved=0

# 3. Type safety
mypy loopx/extensions/entrypoint_surface.py
# Result: Success: no issues found

Final Verdict / 最终结论

LGTM ✅ (Looks Good To Me)

This PR is a well-scoped, zero-risk infrastructure improvement that catches manifest declaration errors at diff time instead of runtime activation. The implementation reuses established patterns, has strong test coverage, and introduces no observable behavior change to users or agents.

Recommend immediate merge with optional CI integration as follow-up.


Review conducted under LoopX PR Review Protocol
Evidence trace: loopx pr-review 4509 → exact-head d02d54db9 validation
Change classification: additive (689+), test-backed, domain-neutral, zero blast radius

Manifest loading is deliberately import-free, so the shape of every declared
reference is checked but never the existence of the referenced object. A
renamed or removed `python_module`, hook `factory` or presentation
`view_validator` therefore passes manifest validation and only fails when the
extension is activated.

Add the contract owner that resolves those declarations against the repository
source tree, plus the public smoke and focused tests that use it. Resolution is
structural: the module file must exist and define the named top-level symbol,
including conditional and re-exported definitions. Provider code is never
imported, so bundled and co-located manifests are covered without an installed
provider environment.

The guard covers all four declaration shapes, including references into
co-located package namespaces that have no in-repo caller.

Signed-off-by: song <liusongstep@gmail.com>
Declarative discovery means a declared launch target is only shape-checked
until activation. Point extension authors at the public smoke that resolves
those declarations from source so the doc and the guard stay in step.

Signed-off-by: song <liusongstep@gmail.com>
The review noted the guard has no product call site: it covers the bundled and
co-located manifests in this repository, not the manifest a user activates.
Say so, and name the structural boundary so a dynamically installed entrypoint
is not mistaken for a regression.

Signed-off-by: song <liusongstep@gmail.com>
Use dataclasses.replace instead of hand-copied frozen-dataclass
reconstruction so a future field cannot silently reset; drop the unused
collect_declared_entrypoints (the guard against dead references should
not ship one); stop counting star-imports as defined names; recognize
except* fallbacks (ast.TryStar, floor is 3.11); and replace the smoke's
lark-specific factory assertion with the REQUIRED_KINDS check it
duplicated. Add the missing structural tests: an ImportError fallback
re-export, and a packages/*/src package resolving through its own
pyproject [project.scripts].

Signed-off-by: song <liusongstep@gmail.com>
@songoow
songoow force-pushed the codex/extension-entrypoint-surface-guard branch from d02d54d to fb352b7 Compare September 16, 2026 09:25

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

我上一轮在 fac9e05d 上批准过这个 guard(并把"没有产品调用点"的 P3 收口为文档),作者随后推了 fb352b75f「apply review fixes to the entrypoint guard」,因此那张卡片失效,这里按新 head 重新给结论。这一版没有扩大范围,只是把 guard 自身的几处薄弱点补上:重复的 dataclass 重建、except* 未被扫描、以及 from x import * 会被当成"符号已定义"。

改动思路

保留上一版已认可的结构(从仓库树派生 manifest、AST 判定符号、类型化 resolved/unresolved、不 import provider 代码),只做三处收口:星号导入不再算作定义;ast.TryStar 与 ast.Try 一视同仁;解析过程中改字段改用 dataclasses.replace,避免逐字段重建带来的漏字段风险。测试从 7 个增加到 9 个,新增"导入报错回退也算定义"与"co-located 包经 src/自有 pyproject 解析"两个用例。

具体改动

  • loopx/extensions/entrypoint_surface.py(+348):新增 guard 模块本体;本版新增 replace 导入(第 18 行)、星号导入过滤(第 157 行)、TryStar 处理(第 179 行)、_resolve 内的 replace 改写(第 279、288 行)。
  • tests/extensions/test_extension_entrypoint_surface.py(+275,9 个用例):新增 test_import_error_fallback_counts_as_defined(:218)与 test_colocated_package_resolves_through_src_and_own_pyproject(:240)。
  • examples/extension-entrypoint-surface-smoke.py(+54):删除 7 行冗余(期望常量收敛),main() 仍调用 resolve_declared_entrypoints(ROOT)。
  • docs/reference/extensions.md(+12):声明该 smoke 是 repository-scoped coverage guard,并写明不覆盖编辑过/第三方 manifest 及结构性检测边界。

关键代码讲解

  1. loopx/extensions/entrypoint_surface.py:157 — elif alias.name != "*": names.add(alias.name):这是本版最重要的正确性修复。此前 from module import * 会让 _import_names 返回 "*" 并把所有声明符号都判为已定义,也就是把"符号存在"检查变成恒真;现在星号导入不再贡献任何名字,该假阳性被消除。
  2. loopx/extensions/entrypoint_surface.py:179 — elif isinstance(node, (ast.Try, ast.TryStar)):把 except* 块也纳入 _defined_names 扫描,避免在异常组语法下漏判符号;配合第 176-186 行的 If/Try 递归,条件定义与再导出的形状都被覆盖(对应 :154 的用例)。
  3. loopx/extensions/entrypoint_surface.py:279 与 :288 — entrypoint = replace(entrypoint, module=module or None, symbol=symbol or None) / replace(entrypoint, source_path=source_path):把逐字段重建改成按需替换,新增字段时不会因为漏写而丢值;这是行为保持的收口。
  4. loopx/extensions/entrypoint_surface.py:188 — symbol_is_defined(source_path, symbol):仍是"读源码、AST 判名字"的核心判定,模块只 import 标准库与自己的 manifest 加载器,因此"without importing provider code"的文档承诺成立;动态安装的属性仍会被判为 unresolved,这一点文档已披露。

对主干的风险

没有阻塞项。 与上一版相比,PR 内容仍是 4 个文件(guard、测试、smoke、参考文档,+689/-0),merge base 已是当前 origin/main(4aaad69bd),git merge-tree 干净;本版新增的都是收紧(星号导入)或补全覆盖(TryStar)与可读性(replace)改动,没有放宽任何检查。

残余风险(已披露):它仍是仓库范围覆盖而不是激活检查——编辑过的 manifest、仓库外第三方 manifest,以及运行时动态生成的属性都不在其覆盖内,这些边界写在 docs/reference/extensions.md:910-920。若维护者要"激活期早失败",那是独立的产品后续,不是本 PR 的完成条件。

验证(全部在 fb352b75f 上跑):python examples/extension-entrypoint-surface-smoke.py → manifests: 9 / entrypoints: 11 / unresolved: 0 / ok;pytest -q tests/extensions/test_extension_entrypoint_surface.py → 9 passed;git merge-tree --write-tree HEAD origin/main 干净。按本 lane 配置不拉取 CI。

我的整体评价

APPROVE。这版 review fixes 打在了要害上:星号导入曾被当成"所有符号都在",会让 guard 在真实项目里静默放行一次重命名——作者不仅修了它,还用用例把"条件/再导出"与"导入报错回退"两类形状固定下来;TryStar 与 replace 属于同类的小收口,方向一致且没有引入新抽象。我已独立跑过 smoke 与 9 个用例,并确认分支与当前 main 合并干净,之前的 P3(作用域/边界)仍由文档承载。

English verdict: APPROVE — exact head fb352b75f1fb39b6098ba5ee1c1f21f07be5eb8a of #4509. The review-fix commit hardens the entrypoint guard without widening its scope: a star import no longer counts as a defined name (previously every declared symbol looked resolved), ast.TryStar bodies are scanned like ast.Try, and record rewriting uses dataclasses.replace. Validation at this head: python examples/extension-entrypoint-surface-smoke.py prints manifests: 9 / entrypoints: 11 / unresolved: 0 / ok, pytest -q tests/extensions/test_extension_entrypoint_surface.py passes 9 tests (two new: import-error fallback and co-located package resolution), and git merge-tree --write-tree HEAD origin/main is clean against current main 4aaad69bd. No blocking findings; the remaining activation-scope limit stays documented in docs/reference/extensions.md.

@huangruiteng
huangruiteng merged commit 78c1124 into loopx-project:main Sep 16, 2026
20 of 22 checks passed
@songoow

songoow commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto upstream/main@4aaad69bd; head is now fb352b75f.

English — CI failures are pre-existing baseline, not this PR. The three failing checks (kernel-static-checks, pytest, merge-gate) all report mypy errors under loopx/capabilities/content_ops/ (markdown.py, connector_packets.py). This PR does not touch those paths (git diff --name-only over the PR range matches 0 files under content_ops). Reproduced on clean upstream/main: mypy loopx/capabilities/content_ops/markdown.py → Found 4 errors in 1 file. Same errors on main, so they cannot be caused by this change.

中文 — CI 失败是既有基线问题,非本 PR 引入。 三个失败检查(kernel-static-checks、pytest、merge-gate)报的都是 loopx/capabilities/content_ops/ 下的 mypy 错误。本 PR 的变更文件里 content_ops 路径命中数为 0。已在纯净 upstream/main 上复现同样的 4 个错误,因此与本次改动无关。

Local evidence at exact head fb352b75f: tests/extensions/ → 913 passed; extension-entrypoint-surface-smoke → ok (9 manifests / 11 entrypoints / 0 unresolved); mypy loopx/extensions/entrypoint_surface.py → clean.

dongphuongman pushed a commit to dongphuongman/loopx that referenced this pull request Sep 17, 2026
…tstrap

`examples/extension-entrypoint-surface-smoke.py` imported `loopx.extensions…`
at module scope without adding the repository root to `sys.path`, which 204 of
the 320 public smokes do. When the public smoke sweep runs a smoke with an
interpreter that has no installed `loopx` on `sys.path`, as the CI job does, the
check dies at import with `ModuleNotFoundError: No module named 'loopx'` and has
failed on every `main` run since it landed in loopx-project#4509.

Reproduced with an interpreter that hides site-packages: `python -S
examples/extension-entrypoint-surface-smoke.py` fails with the same error before
this change and prints `extension-entrypoint-surface-smoke: ok` (9 manifests, 11
entrypoints, 0 unresolved) after it. The script keeps its single `ROOT`
definition and moves it above the guarded import.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
@songoow
songoow deleted the codex/extension-entrypoint-surface-guard branch September 28, 2026 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants