refactor(extensions): one owner for the connector token shape - #5361
huangruiteng merged 4 commits into
Conversation
Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
Decided on the folded value with anchors normalized, so an anchored or assembled restatement is reported the same as a literal copy, and a construction whose value cannot be folded has to be declared. The runtime module is the owner because the provider already imports eight other connector contracts from it; a probe asserts that direction, because the whole premise rests on it. The scope and profile shapes stay separate on purpose -- they answer with a different character class, not a different spelling. Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
…premise Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
|
CI triage for exact head All three failing shards point to the same source-fingerprint churn boundary, not to this PRs connector-token changes:
Those failures are in The branch is currently 25 commits behind This explains the red checks only; it is not an approval. The updated exact head still needs review. |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
本次完整评审针对 57e78cc040e7955da00a592bfab41411c5479568,以不可变 merge-base 3b73108e32acfe6657204b902a037171797e3e5c 核对全部三个文件。没有阻塞发现。runtime、generic connector provider 与 Lark comment provider 原有同形 token 校验容易分别演进;复用已经存在的 runtime owner,删除两份重复声明,能降低后续身份校验规则的维护漂移,同时保持普通调用、错误诊断和 constructor 的无副作用行为。
改动思路
最小修复就是让两个现有 consumer import external_connector_runtime.SAFE_TOKEN_PATTERN,无需新的 capability、权限规则、持久化字段或语法框架。Python 在这里是既有 connector transport/lexical adapter,不新增控制面决策 owner,因而无需为两行 import 强制 TS 重写。不同的 scope/profile grammar 继续保留:scope 可含 /,profile 的长度/字符集不同,不能因为长得像 token 就一并收敛。导入依赖方向已有 provider→runtime;Lark provider 原来即依赖这些 provider/runtime 合同,新增常量引用未形成循环。
具体改动
关键代码讲解
loopx/extensions/external_connector_runtime.py:31 — SAFE_TOKEN_PATTERN:既有 owner 未改,仍是 ASCII 字母/数字开头、后续. _ : -等允许字符、fullmatch 总长至多 200;它只验证词法身份,不授予外部 connector 操作权限。loopx/extensions/external_connector_provider.py:51 — _safe_token:仍str(value or "").strip(),大小写不折叠,失败沿原 field-specific ValueError。build_external_connector_permission_requirement:82仍先验证 operation,再验证 provider fields/scopes;共享 pattern 没有把 syntactic acceptance 改成 permission grant。loopx/extensions/lark/document_comment_provider.py:82 — _safe_token和LarkCliDocumentCommentProvider:422:source_ref/provider identity 继续走原校验顺序,URL/profile 的独立规则不变。constructor 不执行 runner;错误 code 仍是 content-free token,避免把外部异常材料纳入诊断文本。tests/architecture/test_connector_token_shape_owner.py:148 — shape_rows:将两 consumer 的符号身份、全树已支持 regex spelling 和 200/201 边界结合;同名SAFE_TOKEN_PATTERNre-export 仍可被原 consumer import,未删除兼容符号。
独立验证:focused pytest 92 passed,包括 architecture owner guard、runtime、generic provider、Lark provider 和 import boundaries;三个 changed paths 的 Ruff、mypy(19 source files)、git diff --check 通过,native public-boundary scan 无错误。相同独立 fixture 在实际 immutable base/head 调用公开 permission builder、实际 Lark constructor 和 content-free error constructor:431 组 omitted/null/空值、数字/容器、大小写/Unicode/控制字符、分隔符和 199/200/201 长度输入,加三组 operation/field 同时非法情况,共 1,296 条完整输出及异常观察逐项相同,constructor runner 调用次数为零。进程内将 generic-provider token 上限缩小一字符后,真实入口独立 oracle 如预期失败;恢复 head 后通过。没有声称跑了 live Lark 或远端写操作。
语义与 CI 对齐
这是一份既有 lexical contract 的 owner 复用,不新增状态分类、authority vocabulary、CLI/config 字段或默认行为。development advisory 对两条 changed production paths 运行成功,发现零 supported vocabulary carriers;它不覆盖 regex grammar、动态构造或全部 test constants,因此仅作 triage,实际语义由同输入 parity 和完整 diff 验证。当前 capability 合同 wait_for_ci=false,本次不获取、轮询或等待 GitHub CI;作者提及的历史 CI 情况不替代本地证据,也未被当作已独立归因的事实。
对主干的风险
最强运行风险是假设“同形”却改变 trim/case/coercion、error priority 或触发外部效果;本次实际入口矩阵覆盖这些观察,两个旧 compiled pattern 与 runtime owner 形状完全一致,调用者行为不变。没有新增输入操作、用户确认、存储写入或 frontend 展示,所以无 frontend companion 要求;没有 feature-off claim,安装/discovery/token 通过也不等于 capability activation 或调用权限。
非阻塞 P2 建议:regex_calls 只识别 literal holder re 等有限拼写;独立 probe 的 import re as rx; PATTERN=rx.compile(...) 被 shape_rows 漏过。解析 regex import alias 并补同形别名负例,或者明确 guard 范围,可避免未来复制规则却绿灯。这与 #5360 的扫描/折叠 helper 相近,集成后在 architecture-test boundary 做一个小型共享 resolver 更容易维护。当前两个真实 consumer 同时有对象 identity 断言和完整 base/head 入口验证,漏检是维护保护的覆盖限制,不是本 head 的输入回归或权限漏洞;延后简化不妨碍这两份生产副本现在退役。
未执行全树 suite、真实远端 Lark、packaged UI 或 deployed promotion;focused Lark tests 的远端 runner 边界使用测试替身,本次 oracle 只验证实际本地 constructor/validation 无远端调用,不声称远端效应已经验收。native scan 的外部 Goal 状态 warnings 与 PR 的公开文件边界结果分开处理。
我的整体评价
APPROVE:既有 runtime owner 足以解决这一实际重复边界,两份生产规则被删除,兼容导出及用户输入/错误体验保留。92 个 focused tests、1,296 条无信息删减的 base/head 观察、边界变异负例和静态检查支撑结论。未来面向 pass 已识别 test parser 的 alias coverage/重复 helper,作为小型非阻塞简化建议延后,没有强加新状态机或框架。批准只针对此 head,不是合并或 CI-ready 结论;批准发布并读回后,按 capability 的 approval-closeout 检查有效旧阻塞评审,未解决/未验证的评审不撤销,历史讨论保留。
English verdict: APPROVE
Goal And Delivered Outcome
Outcome basis / anchor: no pre-existing issue. Measured on the intended base
3b73108e3, the shape[A-Za-z0-9][A-Za-z0-9._:-]{0,199}-- "may this value be carried as a connector token" -- was compiled three times:loopx/extensions/external_connector_runtime.py:31,loopx/extensions/external_connector_provider.py:41andloopx/extensions/lark/document_comment_provider.py:48. All three apply it withfullmatch, so the copies answered the same question with the same text:SAFE_TOKEN_PATTERN.Owner, by the tree's own evidence rather than preference:
external_connector_provider.pyalready imports eleven other names from it (CONNECTOR_SCHEMA_VERSION,ExternalConnectorCapability,ExternalResponsePolicy,build_external_event_response_receipt,decide_external_event_ack,settle_external_connector_eventand others) fromexternal_connector_runtime.py. The runtime module was already the place the connector boundary asked questions; only the token shape had a second local copy.Observable before → after: two local
re.compilecalls deleted, two imports added, and a guard that fails if either file restates the shape or keeps the import while deciding some other way. Every accepted and rejected value is unchanged: the guard measures the bound at its edge (200in,201out) and asserts the class rejects/,@, space, tab and non-ASCII.Intended base:
3b73108e3.Scope And Continuation
SAFE_SCOPE_PATTERN(external_connector_provider.py:42) allows/;SAFE_PROFILE_PATTERN(lark/document_comment_provider.py:48) drops:and bounds at 99. Different character classes are different answers, so a "consolidation" that swallowed them would be a product change wearing this PR's clothes.{0,127},{0,159},{0,255}-- each with its own rejection text at its own surface.^[A-Za-z0-9._:-]{1,200}$-- the opaque-reference shape -- is compiled in five modules on this base (chat_action_store.py:57,chat_actions.py:51,control_plane/goals/deletion_service.py:49,control_plane/goals/botmux_runtime.py:31,control_plane/goals/source_session_registry_state.py:14). It has no single owner yet, and it is the largest same-value family the connector shape sits next to.Validation
57e78cc04(3 commits, 3 files, +404 -2).unitpytest tests/architecture/test_connector_token_shape_owner.py-> 34 passed in 0.55s: value scan overloopx/with anchor normalization and same-file constant folding, per-consumer identity plus a real reference, an assertion that the provider still imports from the runtime (the guard's premise), 7 accept / 10 reject cases including the 200/201 boundary, and 9 spelling probes (5 must be reported, 4 must not).integrationc1cef09c8; the follow-up commit57e78cc04only rewords a docstring inside the new guard file, which was then re-run green (34 passed).regression_paritypytestover the test files that import the three touched modules -> 367 passed, 0 failed in 15s. Head has no failure to attribute, so no base replay was needed; no test file was edited.staticpython -m ruff checkon the three changed paths: clean.python -m mypy(no arguments, as CI runs it):Success: no issues found in 19 source files.staticloopx check --scan-pathfor each changed path through this tree's own entrypoint:ok: true, "public boundary scan clean: 3 files"; both warnings concern the absent local.loopx/registry.json.semantics budgetexamples/semantic-vocabulary-drift-smoke.pyon an unmodified3b73108e3worktree and on this head, same venv, same Node 22.23.2, samenode_modules: output byte-identical,conflicting_definitions=55/55. Removing two duplicate definitions moved no ratchet because are.compilevalue is not one of the inventory's counted kinds, and no new constant name is introduced here.canaryloopx canary premergewith the changed files passed explicitly:selected 13 / executed 13 / failures 0,status: passed, no manual holds.mutationfullmatchmakes them redundant); a new module assembling the shape from two same-file constants (M3); the owner tightening the bound by one character (M4, 4 cases); the owner dropping:from the class (M5, 3 cases); a consumer keeping the import while deciding with a weaker local check (M6); an unfoldable construction in a new module (M7). Survived, and why it is equivalent: M8 addsnormalized.isprintable()alongside the owner check; no value the class accepts is non-printable, so the answer is identical for every input.frontendType Of Change
LoopX Area
loopx/extensions/external_connector_provider.py,loopx/extensions/lark/document_comment_provider.py,tests/architecture/.Technical Direction
Boundary Checklist
none.