refactor(lark): decide the sink visibility pair in one owner - #5597
Conversation
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; reasoning_effort=xhigh; declaration_source=runtime_reported
动机
需要将工作投影同步到飞书的操作者,以及修改同步规则的维护者,会遇到这四个消费点。
过去调整允许的同步可见性需要同时维护两个 sink 和两个 parser;现在一次修改即可覆盖四处,原有调用参数、默认分支及拒绝行为相同。
已核验的结果是四处消费同一个值集合,10 个完整 CLI/sink 输出与基准完全相等。
本 PR 不改变飞书权限,不证明真实账户远端写入或长期吞吐提升,也不完成任何具体 Goal 的整体业务验收。
本次核验 head 43f18386333b176f7aaf739148cfba5ec5c41325,基准 ba443e2b9d096f5e6ec3c710ef68143ffff73448。两种 Lark 投影同步及其 CLI 曾分别维护 owner-only/shared 接受集合,下一次调整可能只改一个入口。本 PR 将这个已有决定收回一个叶模块。交付目标是四个调用点共用规则,同时保留默认分支、脱敏、拒绝输入与执行授权行为。
不修改也能保持今天的同步,但会继续承担四处同步维护的成本;单改两个 CLI 仍留下两个 sink owner。当前提取范围比新建 capability、改协议或扩大 vocabulary 格式更合适。公开改动不代表任何具体 Goal 的业务验收完成。
改动思路
sink_visibility.py 只导出两个原值和集合;kanban.py、explore_results.py、两个 CLI 引用它。shared 仍只选择既有脱敏路径,owner-only 仍为默认;这不是飞书共享权限/ACL 的创建或授权。默认 execute=False、显式执行及现有 schema preflight 都保留。
与 extension manifest 的 public-safe/owner-only 分离是正确的:相同字符串不证明相同决策。值带连字符,不能借去重顺便放宽全局语义注册表的标识符格式。现有领域 provider 局部提取不需要另造通用 Python 控制面决策。
具体改动
完整核验 9 个文件,+376/-17:新 owner;两个 sink 的常量删除与导入;两个 parser 的 choices/default 接线;338 行架构守卫;语义预算注册表及 drift smoke 锚点下调;生成 IO manifest 的一个行号更新。调用链为真实 CLI → 原 sink → 原投影/脱敏/执行边界,没有新增自动同步动作或用户配置步骤。
独立证据:
- 新 guard 与 kanban preflight 27 passed。
- 同一 workload 的 architecture/canary 与相关 Explore/Lark 测试:base 1474 passed / 3 failed,head 1497 passed / 3 failed;增加的 23 项来自新 guard,三项失败名称和原因相同。
- 在各自源码解释器上比较两种 sink 的默认、显式 owner-only、shared、非法 public-safe,以及两个真实 CLI help,共 10 个完整输出完全相等;Explore 使用非空投影和 dry-run,未连接或写入真实飞书。
lark-kanban-control-plane、explore-feishu-singleflight两个现有 smoke 通过;base/head 完整 semantic drift smoke 均通过,head measured forks 为 7/17/6;Ruff、mypy 19 files、公开边界扫描通过。
对主干的风险
没有复现运行行为回归。独立确认的三个基线失败是 Goal binding inventory、top-level module budget 149/147、既有 goal_topic_runtime.py maintainability debt;没有把它们改称通过或归给本 PR。未实测真实账户的远端写入、分享 ACL 或长期吞吐。
非阻断 P2:新全树 guard 的词法相关性门槛存在误报与漏报。 文件含 sink_visibility 后,任意 "shared" 字面量都会被报成第二 owner,例如 def render(sink_visibility): return {"workspace_mode": "shared"};反过来,SINK_VISIBILITIES: set[str] = {owner.SINK_VISIBILITY_OWNER_ONLY, owner.SINK_VISIBILITY_SHARED} 建立新集合,却因为只扫描 ast.Assign 而两层都漏掉。这两例已实际运行守卫函数验证。建议以相关 AST 参数/绑定收窄扫描并覆盖 AnnAssign,同步压缩守卫,别把全文件字符串禁用当语义一致性证明。它不推翻当前四个真实消费点的独立接线及行为证据。
我的整体评价
APPROVE,附上述非阻断 P2 建议。 长程收益主要是下一次变更只维护一个值 owner,减少 CLI 与 sink 分叉;用户今天的参数和流程保持一致,没有证据支持运行速度提升。小型相邻重构已落实为最近领域叶模块;不建议扩大为全局 vocabulary/权限框架。保留未测远端及长期效果边界,交由维护者合并。
English verdict: APPROVE - head 43f1838. One local visibility owner preserves both real CLI and sink behavior; 10 complete baseline/head outputs match, focused/static/smoke checks pass, and the same three baseline failures are independently attributed. Non-blocking P2: narrow the lexical guard and handle annotated bindings. No live Lark write or sustained throughput claim.
|
This pull request has merge conflicts with Choose the remote for the base repository, not an out-of-date fork. git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEADFor a same-repository clone whose Keep the DCO |
owner-only / shared gate whether a Lark projection sink redacts before it writes. Two sinks bound all three names themselves and the two CLI parsers that feed them spelled the accept set out inline, so the redaction switch had four independent answers. The pair now lives in the presentation package as a leaf owner. The sinks keep exporting the names they used to define, and both parsers build choices and default from the owner object. Signed-off-by: Inference1 <[email protected]>
A name-based scan never sees choices=["owner-only", "shared"], and two of the four owners stated the pair exactly that way. The guard reads module-level bindings and string literals separately, wires each of the four consumers through its own entry point, and pins the manifest surface vocabulary as a different decision so nobody merges it on the strength of the shared word. Signed-off-by: Inference1 <[email protected]>
On the rebased tree the same counters measure same_runtime_forks 3, same_runtime_fork_definitions 9 and the semantic pair 3, down from the 5/13/5 pinned on main, so the registry and its anchor literal in this smoke are pinned to the measured values in one diff instead of leaving the recovered slack as headroom. The census anchor for lark_kanban.py moves one line because the owner import was inserted above it; the manifest is regenerated in place and its diff is that single line. Signed-off-by: Inference1 <[email protected]>
43f1838 to
455115f
Compare
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · xhigh
精确 head:455115f158a337353ac79896d320aa0f69f08279;被 rebase 的前 head:43f18386333b176f7aaf739148cfba5ec5c41325;当前 base:main(be6fc7b60)。
动机
贡献者的 Lark sink 可见性重构此前因 loopx/semantics/vocabulary_v0.json 与 examples/semantic-vocabulary-drift-smoke.py 的 ratchet 预算在前一版主干上取值不同而冲突。本人要求 rebase 到当前主干并合并;被改写过的 head 不继承旧批准,因此重新核验。
改动思路
把三个贡献者提交 rebase 到 origin/main,保留原作者与 DCO sign-off;冲突只出现在 ratchet 预算,不按任一侧硬取,而是在 rebase 后的树上重新测量后把预算钉到实测值:same_runtime_forks 3、same_runtime_fork_definitions 9、same_runtime_forks_semantic 3(主干原为 5/13/5,本次重构把同runtime 分叉从 5 降到 3)。注册表与 smoke 里的锚点字面量必须在同一次改动里一起动,第三个提交的说明也改成 rebase 后的实测数字,避免留下与 diff 不符的历史声明。
具体改动
9 文件、376+/17− 相对当前 main:新增单一 owner loopx/extensions/lark/presentation/sink_visibility.py(23 行),kanban.py 与 explore_results.py 改为消费它,两个 CLI 入口改为显式传参,新增 338 行架构回归 tests/architecture/test_lark_sink_visibility_owner.py,IO manifest 因插入 import 位移一行重新生成,ratchet 预算按实测收紧。验证:tests/architecture/test_lark_sink_visibility_owner.py 与 tests/architecture/test_project_registry_io_census.py 30 项通过;kanban 预检、explore 呈现视图、控制面导入边界、explore 研究证据 77 项通过;examples/semantic-vocabulary-drift-smoke.py 通过且三个键已无 slack(3/3、9/9、3/3);语义 coinage advisory 未检出新载体;loopx check 公共边界扫描 21 文件干净;ruff check 与 git diff --check 通过。
对主干的风险
重构把「同一对可见性常量必须来自同一 owner」从约定变成可执行回归,行为路径未变:四个真实消费方的行为对照在上一轮评审中已通过,本轮只验证 rebase 与预算归属。仍有一条上一轮评审标注为非阻塞的 P2 留在该回归文件:文件级相关性检查会把无关的共享字面量算进去,而 SINK_VISIBILITIES: set[str] = {...} 这类 AnnAssign 不在扫描范围内,存在漏检。本轮按维护者要求只做 rebase 与合并,未改写贡献者的检查逻辑;该项作为已披露的已知限制保留。未测量的部分包括安装版、真实飞书端到端与该守卫的进一步收紧。
我的整体评价
APPROVE:单 owner 的 sink 可见性归属可独立读回,ratchet 预算在 rebase 后按实测收紧而不是遗留回退空间,贡献者署名与 sign-off 保留;唯一的遗留项是评审自己标注为非阻塞、且已明确披露的守卫收紧。
English verdict: APPROVE - 455115f. The contributor's three commits are rebased onto current main with author and DCO sign-off preserved; the only conflict was the inventory ratchet, resolved by re-measuring the rebased tree and pinning registry plus anchor to 3/9/3 instead of taking either stale side. 30 architecture tests, 77 related Lark/explore tests, the semantic drift smoke with no slack, the coinage advisory, the public boundary scan, ruff and the diff check pass. The previously filed non-blocking P2 about the guard's file-level relevance check and AnnAssign coverage is disclosed and intentionally untouched by this rebase.
What this decides
owner-only/sharedis one decision: whether a Lark projection sink may redact and then write rows that leave the owner's machine. Onmainit had four independent answers:loopx/extensions/lark/presentation/kanban.py:89-91andloopx/extensions/lark/presentation/explore_results.py:93-95each bound all three names with their own module-level assignments.loopx/cli_commands/lark_kanban.py:214-215andloopx/cli_commands/explore_feishu_commands.py:152-153-- the two parsers that hand the value to those sinks -- spelled the accept set out a third and fourth time inline:choices=["owner-only", "shared"], default="owner-only".The inline spelling matters for how this is guarded: a scan that groups by constant name never sees an argparse
choiceslist, so the guard reads module-level bindings and string literals as two separate layers.Change
loopx/extensions/lark/presentation/sink_visibility.py(new)__future__presentation/kanban.py,presentation/explore_results.pycli_commands/lark_kanban.py,cli_commands/explore_feishu_commands.pychoicesanddefaultbuilt from the owner objecttests/architecture/test_lark_sink_visibility_owner.py(new, 23 cases)loopx/semantics/vocabulary_v0.json,examples/semantic-vocabulary-drift-smoke.pyloopx/semantics/project_registry_io_manifest_v1.jsonBehaviour is unchanged: both parsers still build
choices=['owner-only', 'shared']withdefault='owner-only'(asserted through the realregister_*_commandstree), both sinks still refuse anything outside the pair with the same message, andsharedstill opens the redacted path.Owner placement. The owner is a leaf inside the presentation package rather than one of its two consumers, so neither sink becomes a de facto authority for the other, and the two CLI parsers import the decision instead of a sibling's copy.
Deliberately not a registered vocabulary. Both values carry a hyphen, and the registry's
value_shapeis^[a-z][a-z0-9_]*$. Widening that shape is a design decision, not a side effect of deduplication, so this PR only single-sources the literals and lowers the counted budgets.Neighbours this guard does not merge (and pins as probes)
loopx/extensions/manifest.py:46_PRESENTATION_SURFACE_VISIBILITIES = {"public-safe", "owner-only"}. It reuses the wordowner-onlyto answer a different question -- whether an advertised extension surface is public-safe. A test asserts the two sets stay unequal and thatpublic-safenever enters this accept set, so nobody collapses them on the strength of a shared spelling.sharedis used elsewhere for unrelated state. The literal census is therefore gated to files that carry thesink_visibilitykeyword or the--sink-visibilityflag, while the binding scan runs over all ofloopx/-- a mutant that bindsSINK_VISIBILITY_SHAREDin a file that never mentions the flag is still caught.Budgets
examples/semantic-vocabulary-drift-smoke.pyon the base revision measuredsame_runtime_forks=9,same_runtime_fork_definitions=21,same_runtime_forks_semantic=8, against budgets of 10 / 23 / 8. After this change the same command measures 7 / 17 / 6 (two groups gone, two modules each), and the registry and its anchor literal are both set to the measured values in this diff. The pre-existing slack (1 and 2) is taken down here rather than left as headroom.Untouched, and measured rather than assumed:
schema_version_same_runtime_forks1/1,multi_value_twins8/8,same_runtime_forks_semanticmoved because the exclusion is a full-name anchored regex (^(?:SCHEMA_VERSION|[A-Z][A-Z0-9_]*_SCHEMA_VERSION|...)$) thatSINK_VISIBILITY_*does not match.twins_raw=45andindependently_maintained=43/43do not move: a package-local Python module creates no cross-runtime pair.Verification
Same interpreter, same Node (22.23.2), base and head measured under identical conditions.
python -m pytest tests/architecture tests/canary: base 1344 passed / 3 failed, head 1367 passed / 3 failed -- the same three names, listed below, so net regressions = 0 and the +23 are this guard.python -m pytest tests/extensions/test_lark_kanban_schema_preflight.py tests/test_explore_presentation_views.py tests/cli_commands/test_project_lifecycle_explore_graph.py tests/capabilities/test_explore_research_evidence.py tests/extensions/test_lark_goal_channel.py: 134 passed on both trees.lark-kanban-control-plane,explore-result-layer,explore-feishu-singleflight,lark-explore-source-runtime-route,lark-capability-layout,lark-projection-source-reconcile,explore-visual-cross-role-integrity.python -m ruff checkoverloopxplus the new test: pass.python -m mypy:Success: no issues found in 19 source files(its file list is untouched here).python -m loopx.cli check --scan-path loopx/extensions/lark/presentation --scan-path loopx/cli_commands --scan-path tests/architecture/test_lark_sink_visibility_owner.py: public boundary scan clean, 140 files, 0 errors.python scripts/generate_project_registry_io_manifest.pyreproduces the committed manifest byte-identically on the clean base tree, which is why the only manifest line in this diff islark_kanban.py::<module>.handle_lark_kanban_command::codec_read:load_registry#1moving 353 -> 354. No site changed kind, api or classification, and the totals stay 278 sites / 0 unclassified.Three checks are already red on
mainat the merge base and are not caused by this change (verified on a cleanba443e2b9worktree):test_top_level_module_budgetmeasures 149 top-level modules against the pinned 147,test_goal_instance_binding_inventory_is_complete_and_anchored, andtest_current_repository_debt_is_reviewed_without_line_count_pins(its finding ismodule_metric_budget:loopx/extensions/lark/goal_topic_runtime.py).Mutation coverage
14 probes, each a single edit, scored as failures the clean tree does not already have: 13 killed.
setandfrozensetforms)choices/defaultshared, or drops it from the accept settests/test_explore_presentation_views.pytest_checked_in_project_registry_io_manifest_is_currentpublic_safe = FalseOne probe survived, by design: rewriting
sink_visibility not in SINK_VISIBILITIESas!= SINK_VISIBILITY_OWNER_ONLY and != SINK_VISIBILITY_SHARED. The values still come from the owner, so it is a spelling of the same decision, not a second owner of it; this guard tracks ownership, not syntax.Not in this change
Counted same-runtime forks this PR leaves alone, measured on the same tree, each with its own owner story:
SNAPSHOT_SCHEMA_VERSION(issue_fix/metrics_projection.py:16+repository_snapshot.py:14, where the two files validate each other's payloads),TRIGGER_DECISION_SCHEMA(periodic_report/core.py:16+triggers.py:22, which already imports nine helpers fromcore),DELIVERY_INTENT_SCHEMA(extensions/lark/miaoda_report.py:23+periodic_report_delivery.py:51),RESPONSE_SCHEMA(semantic_preference/contract.py:27+openviking_semantic_preference/provider.py:17), andLIFECYCLE_PRIORITY(control_plane/status/lifecycle_projection.py:29+status.py:349, which is amulti_value_twinsentry, not a fork).AGENT_LANE_PROGRESS_SCOPEandSKILLS_SUBDIRare left because other pull requests are actively editing their files.