Skip to content

fix(goals): bind deletion to source registry state - #5016

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/fix-goal-delete-source-cas
Sep 25, 2026
Merged

huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/fix-goal-delete-source-cas

Conversation

@Duang777

@Duang777 Duang777 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Goal/source and gap: Goal deletion confirmation was not bound to the authoritative source registry bytes or route mode, and orphan fallback did not keep the declared source path stable through commit.
  • Observable result: deletion previews bind source identity, source content digest, route mode, target identity, and target bytes. Apply rechecks the same basis while holding the source and global locks, immediately before writing, and after write. A restored or changed source now returns stale before mutation or triggers rollback without publishing goal_deleted.
  • Issue/task and intended base: self-contained Goal lifecycle bug fix against main.

Scope And Continuation

  • Completed scope: direct deletion and typed Chat action application, including source-only mutation, orphan source restoration, unsafe lock artifacts, concurrent deletion, resume, rollback, and receipt failure handling.
  • Slice boundary: this PR does not address a later sync operation that starts from an already stale snapshot, or byte-identical delete-and-recreate lineage. Those require separate identity or synchronization contracts.

Validation

  • Tested revision: 7cc208c03a36df9d0f0b8148758f4b2150e74472
  • Run state: finished
  • Input classes: synthetic file-backed registries
Check kind Result Public-safe evidence / limitation
unit passed .venv/bin/python -m pytest tests/control_plane/test_goal_activation.py tests/test_file_lock.py -q; 62 passed.
census passed Manifest current at 232 sites with 0 unclassified; 9 architecture tests passed.
static passed Ruff, Python compilation, and git diff origin/main...HEAD --check passed.
premerge failed on baseline All direct checks, maintainability checks, and 8 risk-profile smokes passed. semantic-vocabulary-drift-smoke.py reports 44 independently maintained py/ts twins; budget is 43 on both this head and a clean origin/main worktree at c36b41678.

The new receipt regression was run red-green: the old catch converted a post-commit receipt write error into failed + retry_safe=True; the new boundary leaves the proposal applying and propagates the error.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none
  • Attention review: N/A because this change has no visual surface.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

Technical Direction

  • Direction / acceptance reference, when applicable: Core control-plane hardening.

Shared-authority RFC fixture impact

N/A. This PR does not claim progress against the TypeScript control-plane migration or shared Goal Authority RFC.

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.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

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

动机

审查对象:PR #5016,精确 head dc3eeeccbb09a511187b6e8b2314ce9eaf420b2c,基线 a6b79e666c25d933dc3febaae282ceba71db7860。Goal 删除是需要操作者确认的破坏性操作;旧版确认只绑定全局 registry 的字节,源项目 registry 单独变化时,旧预览仍可执行。相同合成输入经真实 Chat action 入口比较:基线在源侧改名后仍返回 applied/goal_deleted 并删掉两份记录;此 head 返回 stale,两份记录都保留。这是有价值的安全改进,但 PR 声称的“锁内重验源与路由”尚未覆盖孤儿路由的关键时序。

改动思路

ChatActionService.preview 通过 delete_stopped_goal(execute=False) 取得源身份、源内容摘要、路由模式及合成状态指纹,并存进 proposal;apply 先做预检,再让 deletion service 在 registry 锁内重算并比对,最后依赖删除读回来决定是否写入 goal_deleted receipt。源可用的普通路由会锁源和全局两个 registry,因而源侧单独变化可以正确判 stale。孤儿 fallback 时,实际锁集退化为全局 registry 一处,而所声明的源路径仍是独立的 authority 候选;只在锁内读取一次这个未锁路径,无法保证从比较到删除之间它继续缺席。

具体改动

关键代码讲解

  • deletion_service._source_basis(约 63 行)把声明源路径的原始字节或 unavailable 标记编码为摘要;_state_fingerprint(约 79 行)把它与目标 registry 摘要组合,成为预览和执行的比较依据。
  • deletion_service._resolve_route(约 244 行)保留 fallback 时的 declared_source_registry,_route_snapshot/_load_locked_payloads(约 324–404 行)重算并判 stale;但 _execute_deletion 的锁路径(约 524 行)只有 source_registry 与 target_registry,孤儿 fallback 时两者是同一全局文件,没有锁住 declared_source_registry。
  • chat_goal_lifecycle_actions._goal_lifecycle_preview(约 23 行)写入新 basis;_apply_goal_lifecycle(约 175 行)将预检中的变化标为 stale 或失败;_apply_goal_delete(约 96–155 行)只要 service 报 ok 且 readback.verified 就发布删除 receipt。
  • 新增的 test_goal_activation.py 用例覆盖源在加锁前变化、预览后恢复、并发删除、失败重试;它们没有覆盖“孤儿源在锁内快照之后恢复”的窗口。

对主干的风险

[P1] 孤儿路由可在已确认的删除过程中恢复源,仍被报告为完全删除。 我用临时合成 registry、真实 ChatActionService.preview/apply 路径做了确定性时序探针:在锁内 _route_snapshot 返回“源缺失”后立刻恢复声明源文件,再继续执行。结果 proposal 是 applied、receipt 为 goal_deleted,全局记录已删,但源 registry 仍有该 Goal。触发窗口来自 _execute_deletion 没有保护声明源路径,且 _write_deletion 在 source_available=False 时把 source_missing 直接视为真。操作者会看到错误的成功反馈,后续同步仍可能重新投影该 Goal。最小修复是在 orphan 路由中把声明源路径纳入同一受控锁/交易边界,无法安全锁定时拒绝删除;重验路由并在最终读回检查它仍不存在。请加一个“锁内快照后恢复源”的真实 Chat action 回归用例,断言 stale/无 goal_deleted receipt,而不是仅测试恢复发生在执行前。

语义与 CI 对齐

本 PR 扩展的是现有 Goal 删除确认契约,不需要另造状态词汇;目前实现与“源与路由在锁内保持一致、成功 receipt 代表源和全局均已移除”的现行语义不一致。目标 head 的两组本地测试 44 passed、maintainability ratchet 14 passed、Ruff 与 git diff --check 均通过;新增反例在真实文件后端可复现失败。按当前 Goal 的 review policy,未查询或等待远端 CI;绿色的既有用例不能抵消这个未覆盖窗口。

我的整体评价

REQUEST_CHANGES。 普通源变化的基线/head 对照证明这项改动有实际安全价值,范围也集中在删除 owner、Chat 入口和相应测试;但破坏性删除的长期一致性与用户成功反馈尚未得到证明。建议保留该方向,补齐孤儿源路径的锁定或明确 fail-closed 边界,并让删除读回验证真实声明源,再重跑上述反例与现有 44 项测试。相关的小幅整理可以把路由快照和最终读回放在同一受控边界;不需要为此扩展为新的通用框架。本 head 不应合并。

English verdict: REQUEST_CHANGES - at dc3eeec, normal source-only changes correctly stale the preview, but an orphan source restored after the locked route snapshot is not locked or rechecked: the real Chat action reports applied/goal_deleted while the source Goal remains. Local focused suites: 44 passed; maintainability: 14 passed; Ruff and diff check passed. Hold merge until the orphan race fails closed and has a real-entrypoint regression test.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777
Duang777 force-pushed the codex/fix-goal-delete-source-cas branch from dc3eeec to 7cc208c Compare September 25, 2026 05:04
@Duang777

Copy link
Copy Markdown
Contributor Author

Addressed the orphan-source race from the review in exact head 7cc208c03a36df9d0f0b8148758f4b2150e74472.

The orphan fallback now locks the declared source path before the global registry, rechecks the route and source basis immediately before writing, and verifies that same basis after the write. If the source returns before the write, apply becomes stale with no registry mutation. If it returns after the global write, deletion rolls the global registry back and does not publish a success receipt. Unsafe symlink, hard-link, and non-file lock artifacts fail closed.

I also narrowed Chat error handling. Only preflight or explicit no-write failures become retry-safe failures. Execute or receipt-write uncertainty stays applying and propagates the error, so a committed deletion cannot be labeled safe to retry.

Validation on this head: 62 focused tests passed, 9 registry-census tests passed, the 232-site census has 0 unclassified entries, and Ruff, compilation, diff checks, maintainability, and all 8 risk-profile smokes passed. The only premerge failure is the existing semantic vocabulary budget (44 twins, budget 43), reproduced unchanged on clean origin/main at c36b41678.

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

Reviewed exact head: 7cc208c

动机

停用 Goal 的删除原先只绑定全局 registry 指纹。若项目源 registry 在预览后变化,或原本孤儿化的源路径重新出现,Chat 可能删掉全局投影却留下权威 Goal,并错误返回 goal_deleted。本 PR 要把破坏性删除绑定到源与目标两侧。

改动思路

Chat 预览读取 source_basis 和目标指纹,确认后按这两个基准执行。有源时先取得项目源事务锁,再取全局锁;孤儿 fallback 也先锁声明的源路径。锁内复核路由、内容及停用状态,备份、写入后再读回。写前源变化标 stale;写后源重新出现则撤销全局删除,保持不确定的 applying 供人工检查,不伪造成功收据。共享文件锁拒绝 symlink、非普通文件和多链接 lock inode。

具体改动

关键代码讲解

  • loopx/control_plane/goals/deletion_service.py 的 _source_basis、_state_fingerprint 把源身份/字节、route mode 与目标身份/字节绑定;_validated_locked_route_snapshot 在锁内及写前复核,_write_deletion 在写后重新检查孤儿源并补偿。
  • _execute_deletion 对孤儿 fallback 改锁 declared_source_registry,解决上次评审所指的仅锁全局竞态。新增测试在写前及全局写后恢复源,分别验证 stale 与回滚,而非 goal_deleted。
  • loopx/file_lock.py 的 _open_lock_descriptor 为共享锁入口加 no-follow、inode/nlink 校验;Chat 对确定性的无写入失败给 failed,对不能确定的写入后异常保留 applying。
  • 独立复跑目标测试共 62 passed,使用隔离的真实文件 registry 与锁;git diff --check 通过。未改动活跃 Goal,未查询远端 CI。

对主干的风险

此变更影响控制面删除和共享文件锁,调用者不止 Chat。注入负例支持上次 P1 竞态已关闭,但不合作写者、断电与跨平台锁行为仍有剩余风险;不能把模拟竞态说成全平台证明。现行必跑 semantic-vocabulary-drift-smoke 在不可变 base 与 exact head 均为 44/43;这是仓库原有绝对预算,不是本 PR 新增 twin,但仍是合并阻断。预算所有者需按基线与 head 的同一负载修复或作有证据的决定,然后复跑。此 head 可合并但落后于 main;合并前仍须复验 exact head。

语义与 CI 对齐

源可用、孤儿 fallback、stale 和不确定状态由 route mode、source basis 与状态转移表示,不靠错误文本子串推断成功。删除的默认保护更严格,PR 已披露;未以作者自报检查或未读取的 CI 替代独立证据。

我的整体评价

前次 P1 已有源锁、写前复核及写后补偿,并在真实文件边界通过针对性测试;我没有找到仍能稳定复现的代码阻断。相邻重构局限在快照校验 helper 与共享锁单一打开入口,没有引入第二套 authority。因为必跑预算仍红,本次只能 REQUEST_CHANGES/暂缓合并;预算恢复后对不变 head 复验。此 PR 改控制面删除与锁语义,不能自合并。

English verdict: REQUEST_CHANGES

@huangruiteng
huangruiteng merged commit fbdc969 into loopx-project:main Sep 25, 2026
15 of 22 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