Skip to content

refactor(lark): decide the sink visibility pair in one owner - #5597

Merged
loopx-agent merged 3 commits into
loopx-project:mainfrom
Inference1:codex/lark-sink-visibility-owner
Oct 5, 2026
Merged

loopx-agent merged 3 commits into
loopx-project:mainfrom
Inference1:codex/lark-sink-visibility-owner

Conversation

@Inference1

Copy link
Copy Markdown
Contributor

What this decides

owner-only / shared is one decision: whether a Lark projection sink may redact and then write rows that leave the owner's machine. On main it had four independent answers:

  • loopx/extensions/lark/presentation/kanban.py:89-91 and loopx/extensions/lark/presentation/explore_results.py:93-95 each bound all three names with their own module-level assignments.
  • loopx/cli_commands/lark_kanban.py:214-215 and loopx/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 choices list, so the guard reads module-level bindings and string literals as two separate layers.

Change

file what
owner loopx/extensions/lark/presentation/sink_visibility.py (new) the two values plus the accept set; imports nothing but __future__
consumer presentation/kanban.py, presentation/explore_results.py local copies deleted, names imported (each file is now 2 lines shorter)
consumer cli_commands/lark_kanban.py, cli_commands/explore_feishu_commands.py choices and default built from the owner object
guard tests/architecture/test_lark_sink_visibility_owner.py (new, 23 cases) binding scan + literal census + per-consumer wiring + neighbour pins
ratchet loopx/semantics/vocabulary_v0.json, examples/semantic-vocabulary-drift-smoke.py budgets lowered to the measured tree
census loopx/semantics/project_registry_io_manifest_v1.json one site line, 353 -> 354

Behaviour is unchanged: both parsers still build choices=['owner-only', 'shared'] with default='owner-only' (asserted through the real register_*_commands tree), both sinks still refuse anything outside the pair with the same message, and shared still 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_shape is ^[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 word owner-only to answer a different question -- whether an advertised extension surface is public-safe. A test asserts the two sets stay unequal and that public-safe never enters this accept set, so nobody collapses them on the strength of a shared spelling.
  • The bare word shared is used elsewhere for unrelated state. The literal census is therefore gated to files that carry the sink_visibility keyword or the --sink-visibility flag, while the binding scan runs over all of loopx/ -- a mutant that binds SINK_VISIBILITY_SHARED in a file that never mentions the flag is still caught.

Budgets

examples/semantic-vocabulary-drift-smoke.py on the base revision measured same_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_forks 1/1, multi_value_twins 8/8, same_runtime_forks_semantic moved because the exclusion is a full-name anchored regex (^(?:SCHEMA_VERSION|[A-Z][A-Z0-9_]*_SCHEMA_VERSION|...)$) that SINK_VISIBILITY_* does not match. twins_raw=45 and independently_maintained=43/43 do 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.
  • Example smokes, all ok: 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 check over loopx plus 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.py reproduces the committed manifest byte-identically on the clean base tree, which is why the only manifest line in this diff is lark_kanban.py::<module>.handle_lark_kanban_command::codec_read:load_registry#1 moving 353 -> 354. No site changed kind, api or classification, and the totals stay 278 sites / 0 unclassified.

Three checks are already red on main at the merge base and are not caused by this change (verified on a clean ba443e2b9 worktree): test_top_level_module_budget measures 149 top-level modules against the pinned 147, test_goal_instance_binding_inventory_is_complete_and_anchored, and test_current_repository_debt_is_reviewed_without_line_count_pins (its finding is module_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.

probe killed by
either sink restates the accept set (set and frozenset forms) binding scan, census, per-consumer wiring
either CLI parser goes back to inline choices / default literal census, parser wiring
owner renames shared, or drops it from the accept set owner literals, both parsers, both sinks, plus two existing tests in tests/test_explore_presentation_views.py
owner carries the set as a list owner shape case
a seventh owner appears in a file that never mentions the flag binding scan (proves the relevance gate does not blind it)
owner stops being a leaf leaf case
census anchor left stale test_checked_in_project_registry_io_manifest_is_current
registry budget raised without moving the anchor literal drift smoke
explore sink hardcodes public_safe = False existing projection-view tests

One probe survived, by design: rewriting sink_visibility not in SINK_VISIBILITIES as != 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 from core), 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), and LIFECYCLE_PRIORITY (control_plane/status/lifecycle_projection.py:29 + status.py:349, which is a multi_value_twins entry, not a fork). AGENT_LANE_PROGRESS_SCOPE and SKILLS_SUBDIR are left because other pull requests are actively editing their files.

loopx-agent
loopx-agent previously approved these changes Oct 5, 2026

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

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.

Comment thread tests/architecture/test_lark_sink_visibility_owner.py
@mergify

mergify Bot commented Oct 5, 2026

Copy link
Copy Markdown

This pull request has merge conflicts with main and cannot be merged
until they are resolved. Please rebase or merge the base branch, @Inference1.

Choose the remote for the base repository, not an out-of-date fork.
For a fork clone, first inspect git remote -v; upstream must point
to https://github.com/loopx-project/loopx.git. If it is absent, add it
with git remote add upstream https://github.com/loopx-project/loopx.git.
Then run:

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 HEAD

For a same-repository clone whose origin points to
https://github.com/loopx-project/loopx.git, use origin instead of
upstream for fetch/rebase. If you prefer merging the base, use
git merge <base-remote>/main and push normally.

Keep the DCO Signed-off-by trailer on every commit when you rebase.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase Mergify: the pull request has merge conflicts with its base branch label Oct 5, 2026
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]>
@loopx-agent
loopx-agent force-pushed the codex/lark-sink-visibility-owner branch 2 times, most recently from 43f1838 to 455115f Compare October 5, 2026 11:32

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Mergify: the pull request has merge conflicts with its base branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants