Skip to content

refactor(extensions): one owner for the connector token shape - #5361

Merged
huangruiteng merged 4 commits into
loopx-project:mainfrom
karenchuu:codex/connector-token-shape-owner
Oct 1, 2026
Merged

huangruiteng merged 4 commits into
loopx-project:mainfrom
karenchuu:codex/connector-token-shape-owner

Conversation

@karenchuu

Copy link
Copy Markdown
Contributor

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:41 and loopx/extensions/lark/document_comment_provider.py:48. All three apply it with fullmatch, 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.py already imports eleven other names from it (CONNECTOR_SCHEMA_VERSION, ExternalConnectorCapability, ExternalResponsePolicy, build_external_event_response_receipt, decide_external_event_ack, settle_external_connector_event and others) from external_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.compile calls 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 (200 in, 201 out) and asserts the class rejects /, @, space, tab and non-ASCII.

  • Intended base: 3b73108e3.

Scope And Continuation

  • Done: the convergence plus a 34-case guard, in three files.
  • Not merged, and pinned as probe cases that must not be reported:
    • 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.
    • The same character class carries other bounds across the tree -- {0,127}, {0,159}, {0,255} -- each with its own rejection text at its own surface.
  • Named successor, sized by the same value scan: ^[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.
  • Slice boundary / successor: complete within scope; reverting is two constant definitions restored.

Validation

  • Tested revision: 57e78cc04 (3 commits, 3 files, +404 -2).
  • Run state: finished.
  • Input classes: synthetic fixtures; no live connector or Lark calls.
Check kind Result Public-safe evidence / limitation
unit passed pytest tests/architecture/test_connector_token_shape_owner.py -> 34 passed in 0.55s: value scan over loopx/ 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).
integration passed 1122 passed, 0 failed in 1m55s at c1cef09c8; the follow-up commit 57e78cc04 only rewords a docstring inside the new guard file, which was then re-run green (34 passed).
regression_parity passed pytest over 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.
static passed python -m ruff check on the three changed paths: clean. python -m mypy (no arguments, as CI runs it): Success: no issues found in 19 source files.
static passed loopx check --scan-path for 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 budget passed examples/semantic-vocabulary-drift-smoke.py on an unmodified 3b73108e3 worktree and on this head, same venv, same Node 22.23.2, same node_modules: output byte-identical, conflicting_definitions=55/55. Removing two duplicate definitions moved no ratchet because a re.compile value is not one of the inventory's counted kinds, and no new constant name is introduced here.
canary passed loopx canary premerge with the changed files passed explicitly: selected 13 / executed 13 / failures 0, status: passed, no manual holds.
mutation passed 8 mutations, one at a time in a separate worktree at this head, all files restored before every round, control round green (34 passed) before and after: 7 caught / 1 survived. Caught: the provider restating its own copy (M1, 2 cases); the document consumer restating it anchored (M2, 2 cases -- anchors are normalized because fullmatch makes 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 adds normalized.isprintable() alongside the owner check; no value the class accepts is non-printable, so the answer is identical for every input.
frontend none No UI, projection or rendered surface changes.

Type Of Change

  • Internal refactor / single-owner convergence with an anti-regression guard. No behaviour change.

LoopX Area

  • loopx/extensions/external_connector_provider.py, loopx/extensions/lark/document_comment_provider.py, tests/architecture/.

Technical Direction

  • The owner is chosen from the direction the code already imports in, and that direction is itself asserted, so a future re-parenting has to say so out loud.
  • Adjacent shapes that differ by one character or one bound are declared as not this decision and carried as negative probes. A consolidation guard that over-reports gets switched off; one that states its boundary gets extended.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.

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>
@karenchuu

Copy link
Copy Markdown
Contributor Author

CI triage for exact head 57e78cc040e7955da00a592bfab41411c5479568:

All three failing shards point to the same source-fingerprint churn boundary, not to this PRs connector-token changes:

  • test-shard (2): test_runtime_source_churn_has_a_stable_readiness_diagnostic
  • test-shard (3): test_runtime_fingerprint_rescans_when_a_snapshotted_file_disappears_while_reading
  • test-shard (4): test_runtime_request_source_churn_raises_a_stable_startup_diagnostic

Those failures are in loopx/control_plane/effect_runtime.py and tests/control_plane/test_turn_journal_runtime_readiness.py; neither path is changed by this PR. This PR only changes two connector providers and adds the connector-token single-owner architecture guard. Its TypeScript shards and the other required checks passed.

The branch is currently 25 commits behind main. Current main includes 647756d21 / #5367, which specifically updates the source-fingerprint snapshot implementation and these readiness tests. Please update the branch from current main and rerun CI rather than adding an unrelated runtime fix here.

This explains the red checks only; it is not an approval. The updated exact head still needs review.

@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.

动机

本次完整评审针对 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_PATTERN re-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

@huangruiteng
huangruiteng merged commit 208e690 into loopx-project:main Oct 1, 2026
4 of 5 checks passed
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