Skip to content

refactor(control-plane): single owner for private-text classification - #5245

Merged
huangruiteng merged 2 commits into
loopx-project:mainfrom
karenchuu:karenchuu/public-safe-text-single-owner
Sep 28, 2026
Merged

huangruiteng merged 2 commits into
loopx-project:mainfrom
karenchuu:karenchuu/public-safe-text-single-owner

Conversation

@karenchuu

@karenchuu karenchuu commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

What this does

Implements direction 1 of #5136: loopx/public_safe_text.py becomes the single home for the decision "does this string look private?", and control_plane/runtime/public_safety.py consumes it instead of owning a competing shape set.

This is the behavior-preserving slice. It relocates ownership, gives the classifier explicit categories and reasons, and closes the path-recognition gap in a way that enforces nothing yet. The behavior changes you flagged are deliberately not here.

1. Shapes have one owner now

SECRET_LIKE_SURFACE_PATTERN, LOCAL_PATH_SURFACE_PATTERN and REMOTE_LOCATION_SURFACE_PATTERN are defined once in public_safe_text.py, byte-identical to what public_safety.py compiled before. public_safety.py imports them in the redundant-alias form (house style under --no-implicit-reexport) so its ~8 direct importers and 30+ recursive-validation callers keep the same objects.

I had to make that re-export explicit because --no-implicit-reexport rejects a plain from … import NAME: bare mypy flagged periodic_report/core.py importing SECRET_LIKE_SURFACE_PATTERN from public_safety.

2. Detection returns a category and a reason

The text-owner rule set is now a list of pattern + category + reason entries, and classify_private_text returns one of four categories (credential, local_path, remote_location, org_marker) with a stable reason. A caller names the policy — a set of categories — that decides what its own surface rejects, which is the separation you asked for in direction 2: recognizing a value never implies publishing it is forbidden.

PRIVATE_TEXT_PATTERNS and find_private_text_match are derived from the categorized list in the same order, so feedback, authority, boundary_authority and the TypeScript Vision checkpoint keep their exact verdicts.

3. artifact_lifecycle makes one policy-aware call

_compact_text used to OR two independent detectors:

if find_private_text_match(value) or SECRET_LIKE_SURFACE_PATTERN.search(value):

Now it names one policy:

if classify_private_text(value, categories=ARTIFACT_LIFECYCLE_CATEGORIES) is not None:

ARTIFACT_LIFECYCLE_CATEGORIES is credential ∪ local_path ∪ org_marker — every category except remote_location, because this projection has always let an ordinary http(s) URL through. Two reasons this stays behavior-identical:

  • No text pattern is categorized remote_location, so the classifier's text pass is exactly find_private_text_match.
  • _compact_text calls validate_public_safe_value first, and that already raises on any LOCAL_PATH_SURFACE_PATTERN or SECRET_LIKE_SURFACE_PATTERN match — so the shape checks in the classifier are provably redundant, not new vetoes.

4. Path-gap recognition is added but enforced nowhere

The classifier can now recognize the two local-path shapes the legacy surface pattern misses: a home-relative ~/… path and a path:-prefixed local reference. This is behind include_path_gaps, which defaults to False, so no existing surface tightens in this PR.

5. ml_experiment keeps its own, stricter rule

Its text.startswith(("/", "~")) check is a per-field alias constraint, not a second copy of the local-path decision, so I left it alone and pinned it with a test: ~username/notes and /just-a-leading-slash are rejected by ml_experiment but are not flagged by the shared classifier even with path gaps on. Folding it into the owner would silently lose that coverage.

Relationship to #5135 and #5196

Both are merged and both are in this branch's base, and this PR deliberately moves the landing spot they chose. #5135 folded the credential shapes into public_safety.py; #5196 put the raw-remote-location shape there for the same reason. Direction 1 then says runtime/public_safety should consume the public_safe_text contract "rather than own a competing set of text shapes", so this moves those definitions one level up into the shared home and leaves public_safety importing them.

That includes editing the two owner guards those PRs added (test_public_safety_credential_shape_owner.py, test_remote_location_shape_owner.py), which pinned the declaring module as public_safety.py. I only repointed them after re-running their own literal scan and confirming the invariant they protect still holds: each relocated shape is declared in exactly one module, now public_safe_text.py, and public_safety.py declares none of them. Nothing from #5135 or #5196 is reverted — the per-site thresholds, the per-site error messages and the anchored whole-value provider-token rule in history_export.py all stay exactly where those PRs left them.

file:// is a local path, and this slice does not act on it yet

I first raised this as an open question and asked the owner. Direction 3 already settles it: the shared classifier should recognize "local paths behind file:// or path: prefixes" and "Public-safe output should reject/redact those local references." So the answer is local path, and a public projection should stop carrying it. I am not waiting on that decision.

It is still not in this PR, because today three things the direction assumes are not true, and changing them is a behavior change rather than a relocation:

  • LOCAL_PATH_SURFACE_PATTERN deliberately excludes a preceding : or /, so file:///Users/… and path:/Users/… are not rejected by public-safe output today — they only failed to surface because the separate raw-location shape rejected any file:// URL as a remote location.
  • artifact_lifecycle has always let an ordinary http(s) URL through, so treating file:// as a local path has to be stated and tested against that surface instead of arriving as a side effect. That is why ARTIFACT_LIFECYCLE_CATEGORIES keeps excluding remote_location here: it reproduces the shipped verdict exactly.
  • The ~/ and path: recognizers added above are behind include_path_gaps, default off, so they enforce nothing yet.

The follow-up lands the tightening with a per-surface newly-rejected list, since it moves four shipped entry points at once.

Evidence that nothing changed in behavior

Check Result
Semantic inventory counts, base vs head identical (same_runtime_forks=12/12, drift smoke ok)
Literal scan: each relocated shape declared in exactly one module (public_safe_text.py)
Bare mypy (repo strict config) Success: no issues found in 19 source files
ruff check on changed files All checks passed!
canary premerge on the changed files self_merge_validation_passed: true, catalog canaries 8/8, risk-profile smokes 8/8, manual_holds: 0
Mutation check on the new logic 7/7 mutants killed
Full tests/architecture + tests/control_plane 6184 passed; the 18 failures are one unrelated file that fails identically on the base commit
Corpus parity + owner guards + projection tests all green (see Test plan)
Newly accepted / newly rejected, per migrated surface none — that is the point of this slice

Mutation results

Because you noted a source-level duplication guard can only supplement, not prove
semantic correctness, I mutated the new logic and checked the tests react:

Mutation Killed by
classify_private_text ignores its categories argument the policy-narrowing test (initially survived — see below)
include_path_gaps defaults to True 2 tests
artifact_lifecycle policy widened to reject ordinary URLs the pass-through test at the real entry point
artifact_lifecycle stops consulting the classifier the pass-through test
public_safety re-grows a competing scheme-list copy the repointed remote-location guard
public_safety re-grows a competing credential-shape copy the repointed credential guard
owner drops the remote_location shape detector 3 tests

The first mutation run left one mutant alive: dropping the category filter on the
text-pattern path kept all tests green, because neither existing policy
excludes a category that a text pattern owns. That is a real hole in the evidence,
so the branch adds a test where each value is matched only by a text pattern, which
makes a policy that drops that category have to accept it and an empty policy have to
recognize nothing. With it, all seven mutants are killed.

Test plan

All runs use Node 22.23.2 on PATH (the effect-runtime tests are otherwise unavailable).

  • pytest tests/control_plane/test_public_safe_text_classifier.py → 25 passed — new: category/reason per shape, the two named policies, the policy filter applied to the text-owner patterns, opt-in path gaps, matches_private_text_policy, the corpus-driven "classifier's first text match is the same object find_private_text_match returns", PRIVATE_TEXT_PATTERNS order/count, re-export identity, and the ml_experiment alias constraint.
  • pytest tests/control_plane/test_public_safe_text_owner_parity.py tests/control_plane/test_public_safety_{credential_shape_owner,path_shapes,field_name_spelling,text_budget}.py tests/control_plane/test_remote_location_shape_owner.py tests/control_plane/test_public_safe_decision_replay.py tests/control_plane/test_goal_{artifact_work,acceptance}_observation.py → 301 passed
  • node --no-warnings --experimental-sqlite --experimental-strip-types --test tests/control_plane_ts/public_safe_text_corpus.test.ts → pass 2 / fail 0 (TypeScript owner unchanged)
  • python examples/semantic-vocabulary-drift-smoke.py → ok, counts identical to base
  • loopx canary premerge → green
  • Wider sweep: pytest tests/capabilities tests/extensions → 2802 passed, 9 failed; those 9 are all in test_repository_change_window.py (git-hook/worktree/ssh fixtures) and fail identically on an unmodified checkout of the same base commit, so they are environmental here, not regressions.
  • Full scope pytest tests/architecture tests/control_plane tests/test_ml_experiment_volc_packet.py → 18 failed, 6184 passed, 5 skipped in 38m53s, and all 18 failures are in one file, tests/control_plane/test_scheduler_compat_state_key.py. That file is unrelated to this change (git-hook/CLI transport fixtures) and fails identically — 18 failed — on an unmodified checkout of the same base commit under the same PATH, which I ran side by side. So this change adds no regression there.

Deliberately not in this PR

  • The bare-word and short-assignment policy you flagged ("the Bearer token expired" as leak evidence, token=x) — a behavior change, so it belongs in its own PR.
  • Enforcing the new path-gap recognition on any surface.
  • The full caller/field → policy review table and the per-surface newly-accepted/newly-rejected report.

Refs #5136

…t owner

Refs loopx-project#5136, direction 1. loopx/public_safe_text.py is now the one home for the
decision "does this string look private?", and
control_plane/runtime/public_safety.py consumes the shapes from it instead of
restating a competing set.

What moved

* SECRET_LIKE_SURFACE_PATTERN, LOCAL_PATH_SURFACE_PATTERN and
  REMOTE_LOCATION_SURFACE_PATTERN are defined once in public_safe_text.py,
  byte-identical to the versions public_safety.py compiled before the move.
  public_safety.py re-exports them (redundant-alias form, the house style under
  --no-implicit-reexport), so its importers and its recursive payload
  validation are unchanged.
* The text-owner rule set is now a list of pattern + category + reason entries.
  PRIVATE_TEXT_PATTERNS and find_private_text_match are derived from it in the
  same order, so the four text owners (feedback, authority,
  boundary_authority, the TypeScript Vision checkpoint) keep their exact
  verdicts.
* classify_private_text / matches_private_text_policy answer detection with an
  explicit category and reason, and a caller names the policy (a set of
  categories) that decides what its own surface rejects. Recognizing a value no
  longer implies rejecting it everywhere.
* artifact_lifecycle._compact_text makes one policy-aware call instead of OR-ing
  find_private_text_match with SECRET_LIKE_SURFACE_PATTERN. The named policy
  reproduces this projection's historical verdict exactly: every category except
  a raw remote location, which it has always let through.

Path-gap recognition, opt-in

The classifier can now recognize the local-path shapes the legacy surface
pattern misses: a home-relative ~/ path and a path:-prefixed local reference.
Recognition is behind include_path_gaps and defaults to False, so this change
tightens no surface. Choosing which surfaces enforce it is the disclosed
behavior change tracked separately.

ml_experiment's leading "/" or "~" rule stays a per-field alias constraint, not
a second copy of the local-path decision: it rejects values the shared
classifier does not flag even with path gaps on, so folding it in would lose
coverage.

Behavior preservation and evidence

* Semantic inventory counts are unchanged (drift smoke reports identical
  same_runtime_forks / conflicting_values / twins on base and head).
* A new test drives the shared corpus and asserts that whenever
  find_private_text_match returns a pattern, classify_private_text's first match
  is that same object.
* The credential-shape and remote-location owner guards now name
  public_safe_text.py as the single owner; the literal scan still finds exactly
  one declaring file for each shape.
* find_private_text_match, the four text owners and the TypeScript owner are
  untouched, so the Python/TypeScript corpus parity test still passes.

No surface newly accepts or newly rejects a value in this commit.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
…s too

A mutation check on the consolidated classifier showed the new tests still
passed when classify_private_text ignored its categories argument on the
text-pattern path: neither existing policy excludes a category that a text
pattern owns, so the filter was untested there.

Add the missing case. Each value is matched only by a text pattern, so a
policy that drops that pattern's category must accept it, and an empty policy
must recognize nothing. With this row the no-op-filter mutation fails.

Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
@karenchuu

Copy link
Copy Markdown
Contributor Author

The two red architecture tests are main-side, not from this head

Head 01c0e3d6c: 24 checks green, 5 skipped, 3 red — and those 3 are one chain (test-shard (4) → pytest → merge-gate) caused by the same two test ids:

  • tests/architecture/test_project_registry_io_census.py::test_checked_in_project_registry_io_manifest_is_current
  • tests/architecture/test_goal_instance_binding_inventory.py::test_goal_instance_inventory_does_not_replace_the_registry_io_census

Both fail on unmodified main at 6643f3670, which is this PR's base (push run 36381588029). "Same test name" is weak evidence on its own, because the CI repr is truncated (assert ['project reg...d_registry#1'] == []), so I reproduced the merge tree GitHub actually runs, locally:

tree scope result
6643f3670 unmodified those two files 2 failed, 7 passed
6643f3670 merged with 01c0e3d6c (git merge --no-edit --no-ff; net diff = exactly this PR's 6 files, +555 −58, no conflict) those two files 2 failed, 7 passed

The full assertion text is the same single item in both trees:

project registry I/O site metadata changed: loopx/contract.py::<module>.check_contract::codec_read:load_registry#1

loopx/contract.py is not touched by this PR. That site's recorded metadata moved when #5239 (perf(contract): overlap bounded full-tree file reads, merged as 6643f3670) rewrote 45 lines of loopx/contract.py without regenerating loopx/semantics/project_registry_io_manifest_v1.json / goal_instance_binding_inventory_v1.json — so the checked-in census is stale on main itself.

Nine open PRs currently modify those two manifest files (#5106 #5130 #5240 #5242 #5247 #5248 #5249 #5250 #5251), so the regeneration looks to be in flight; I am not claiming which one lands it.

No change pushed to this head. Both local runs used the repo's own strict venv, Node 22.23.2 on PATH, and npm ci --ignore-scripts in each worktree.

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

APPROVE — 当前 exact head 未发现阻断回归;这是一份保留现有字段策略的 owner consolidation,不是 #5136 全部隐私政策的完成声明。主干已有的 census 失败另列为合并 hold,不作为本 PR 的 Request Changes 理由。

精确 head:01c0e3d6c217ba039adb69e76a67bb19f25cb9fe。全 PR 三点差异为六文件、+555/-58;实际分叉基线为 f679911568eee2b3c3cfa1a6ade43dcb17cfb06e,当前主干为 6643f367064b9921c864db75a042979fddc4b8c3。本人检查实现、未改动的调用者及真实公开入口,没有查询或等待 GitHub CI。

动机

按 #5136 的维护者验收说明,公共文本检查需要一个分类 owner,同时保留不同字段的明确策略。原来 shape regex 留在 runtime 模块,artifact lifecycle 另行拼接这些检查;下一次规则修复容易遗漏消费方。本次消除这个维护缺口,已有用户的 status 标签、错误结果和后续推进不应改变。更严格的普通单词、相对路径及公开导出政策仍属于既有 issue 的后续验收。

改动思路

扩展既有 public_safe_text,而不是增加能力或第二套权限引擎。shape detector 移到同一 owner,runtime 显式 re-export 原符号,既有文本校验仍使用原顺序。artifact 消费方只选择 credential、local-path、org-marker 类,继续允许它历史上允许的公开 URL;分类器不能反过来替字段决定“一切匹配都拒绝”。Python 放置符合现有文本 owner 的边界,TypeScript 既有 owner 和共享语料保持不变,不以本次重构为由扩大迁移。

具体改动

关键代码讲解

  • public_safe_text.py:265 / classify_private_text 先按显式 categories 筛选既有有序规则,再检查三种 shape。相对/带前缀路径的新识别默认关闭;当前生产调用没有开启它,因此 helper 的可用性不等于改变默认拒绝政策。
  • public_safe_text.py:228 / PrivateTextMatch 是冻结的分类、原因、pattern 结果,不持久化新状态,也不赋予执行权限。find_private_text_match 仍返回旧 regex 对象并保留原匹配次序,原来的调用者无需换接口。
  • artifact_lifecycle.py:57 / _compact_text 从三个独立 shape 检查改为一次显式字段策略调用;先检查完整值,再截断展示文本,避免长字符串尾部的私有形状在截断后消失。调用链仍为 collect_status → attach_goal_artifact_lifecycle_projections → build_goal_artifact_lifecycle_projection,无文件状态写入。
  • runtime/public_safety.py 的显式同名 re-export 保留现有导入面。检查反馈、authority、boundary 与 TS vision 的未改动消费者,未新增一个平行的泛化状态规则 owner。

正例中公开 URL 和普通授权说明保留原输出;负例中本地路径、credential shape、长文本尾部匹配仍被拒绝或替换为原有安全标签。复验也纠正了一个描述细节:包含既有 home-directory 形状的 file URL 在 base 已被过滤,不是本 PR 新增拒绝。已存在的裸敏感单词误报不被本次重构宣称修复。

对主干的风险

326 项目标 Python 测试、TS typecheck、TS 共享语料两项测试、配置指定的十九文件 strict mypy、改动文件 ruff 均通过。风险分层 canary 的五项直接检查和十六项选中检查全部通过。首次 canary 尝试的报告落盘遇到 ENOSPC,未计为通过;环境恢复后的完整同命令重跑有独立结果。

我另写独立 oracle,使用真实文件后端的 collect_status 及四个现有公共文本消费方:十四项 status、七十六项共享语料、七项递归结构,共九十七项 base/head 完整结果一致;boundary builder 的时间由输入显式固定,未删除错误、状态或策略字段。fixture SHA-256 为 1fd288ad28a765fe2a29d3b1c6099fbc9149085358626dcdd87484c36de3e1b4,完整 observation SHA-256 为 b93ed8cc8c84ca0647218591a27a4333018abb9f655f0101efc3e5a1b491886c。强制扩大 URL 拒绝策略、或强制开启相对路径新规则,各使两个真实 status 反例失败,说明 oracle 能捕获这两种回归。测试使用合成状态,未操作活动 Goal。

语义与 CI 对齐

复用已有公共文本/字段策略,不改变 Goal、Todo、lease、quota 或 settlement 语义。没有把目录级展示策略当成授权,也没有把机器拒绝称为“指导”。现有 regex 的字符串启发式仍有误报边界,后续应在 #5136 的既有 owner 中改成明确、经语料验证的政策,而非暗中放宽。

本 PR 的源 head 上,两份 architecture inventory 文件九项测试通过。当前主干和“主干+本 PR”的临时合并树则都出现相同两项 census 失败,细节均为 contract.py 的 codec_read:load_registry#1 元数据不匹配;这条路径不在六文件差异中。比较的是相同命令、失败身份和细节,不仅是数量。该主干整合问题保留在严格质量/合并门槛中,代码 review 仍为 APPROVE。

我的整体评价

这份有限增量有实际维护价值,long_horizon 改善规则定位与后续修复,user_experience 经真实入口保持;全 issue 的更严格隐私验收尚未关闭。默认关闭兼容性、字段间的有意差异以及无副作用读回都有证据,未发现需要本次阻断的语义漂移。

未来向前的 bounded refine 已采用“一个 detector owner+显式字段策略”。非阻断建议:matches_private_text_policy 目前只有测试消费者,可等首个真实调用者再保留这层布尔包装;category 字符串可在同边界进一步收窄为 Literal,而无需新增框架。本轮只 review,不修源码、不合并、不升级;严格质量/整合 hold 不被 review 批准抹除。

English verdict: APPROVE - 01c0e3d. The existing text owner is consolidated with explicit field policies and independently verified public-entrypoint parity. No blocking PR regression was found. Identical current-main/integration census failures remain separate quality and merge holds; this is not full #5136 closeout.

@huangruiteng

Copy link
Copy Markdown
Collaborator

对 01c0e3d6c217ba039adb69e76a67bb19f25cb9fe,按 #5136 维护者验收 的分项结论:

  • 一个既有 Python detector owner、明确的 artifact 字段策略和原 runtime 导入兼容性成立;不新建 capability 或第二套 state authority。
  • 真实 status/四文本消费方九十七项完整 base/head parity,加扩大 URL 策略/强开路径新规则的反例,验证默认行为不变。
  • 这是 owner consolidation 的完整有限增量,不是普通单词误报、严格相对路径或全部公开导出验收的 closeout;既有 [Architecture]: two modules both claim to own "private-looking text" #5136 继续拥有这些缺口。
  • 当前 main 与临时合并树的 census 同身份、同细节失败保留为整合 hold,不据无关红检查请求修改本 PR。
  • bounded future-facing pass 已采用单 owner;未使用的布尔包装可等真实消费者再保留。

结论 APPROVE;当前许可只覆盖审查与公开反馈,不执行 merge、source repair 或 runtime 升级。

本次 exact-head 完整 review

@huangruiteng
huangruiteng merged commit 7fa104d into loopx-project:main Sep 28, 2026
29 of 32 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