Skip to content

fix(goal-start): fence guided bootstrap over orphaned Goal state - #4808

Merged
huangruiteng merged 6 commits into
loopx-project:mainfrom
hahahahahayesyeseys:codex/guard-orphaned-goal-state
Sep 22, 2026
Merged

huangruiteng merged 6 commits into
loopx-project:mainfrom
hahahahahayesyeseys:codex/guard-orphaned-goal-state

Conversation

@hahahahahayesyeseys

@hahahahahayesyeseys hahahahahayesyeseys commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Goal/source and gap: [Bug]: Guard orphaned Goal state before guided bootstrap or host activation #4801. An operator-initiated reset can remove a Goal's project registry entry while its project-local active-state file survives. inspect_bootstrap_connection classified that as the ordinary registry_without_goal absence, so a guided packet kept offering connect_if_needed, fresh-agent registration, Todo writeback, host-loop activation and quota spend for the same human-readable Goal id, and a later diagnose reported a healthy Goal over state an earlier lane left behind. The gap is a missing distinction between "nothing was here" and "state is here but its authority is gone".
  • Observable before -> after, with the validation row that proves it: before, the reproduction project returned connection_state: registry_without_goal together with state_file_exists: true and a bootstrap-oriented continuation (captured by the failing-before run in the regression_parity row). After, the same project returns orphaned_goal_state, ordered_steps collapses to inspect_connection + resolve_orphaned_goal_state, and all five mutation continuation commands are null (real_entrypoint row).
  • Issue/task and intended base: Refs [Bug]: Guard orphaned Goal state before guided bootstrap or host activation #4801; three of its acceptance bullets are deliberately left open and named below. Base: main.

Scope And Continuation

  • Completed scope: the detected fact (candidate routes over the current and legacy project state roots, reported without merging or copying); one builder through which every absence route passes, so the fence cannot be reached or missed branch by branch; a command pack rebuilt from an allowlist rather than a suppression list, with the verb-bearing nested subtrees dropped; an operator-facing gate that keeps only the two routes that actually run today; and regression coverage that walks every consumption surface, including the shipped onboarding entry qualifier.
  • Remaining work: (1) diagnose surfacing of an unresolved orphan; (2) a previewable archive/adopt route for project-local orphan state - measured in this slice, uninstall-project selects goals from the registry and archive-runtime resolves only a goal directory under the shared runtime root, so neither reaches this state, which is recorded here rather than projected as a packet field; (3) an opaque Goal instance/generation id so host bindings cannot reattach across a deletion. Each changes a different command's contract and needs an owner decision on shape.
  • Slice boundary / successor: the fence is independently testable and reversible, and it removes the harmful automatic continuation on its own. Follow-ups are recorded on [Bug]: Guard orphaned Goal state before guided bootstrap or host activation #4801 rather than fabricated as code here.

Validation

  • Tested revision: c787059 (current head). Rows marked "reviewed head" were produced at 144072c, a0ca91d or a472bae, the heads the three review rounds ran against; the exact-head re-run of every row is published in the review replies on this PR.
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
real_entrypoint passed start-goal --guided executed through the shipped CLI against a synthetic project reproducing the issue's shape: two ordered steps, writes_now=false, spends_quota_now=false, goal_start_connect_if_needed / goal_start_refresh_state / goal_start_host_loop_activation / goal_start_quota_should_run / goal_start_plan_prompt all null, recommended_next_step.kind=resolve_orphaned_goal_state
real_entrypoint passed at this head, the review's own counter-example driven through the shipped CLI: a project with a surviving state file and no registry file at all now returns orphaned_goal_state, two ordered steps, registry_exists=false, and all seven continuation command keys null
unit passed new module tests: 16 passed at this head, 9 at the first reviewed head - detection over all four project state routes, the three absence routes separated from ordinary absence and each pinned to the shape it came from, candidate matching scoped to the requested id, blocking transaction and command-pack parity, the shipped onboarding qualifier reporting route=stop with zero actionable commands, and a recursive walk of the guided and standalone packets that fails on any line spelling a mutation command family or carrying --execute
regression_parity passed failing-before: with the module and wiring reverted, the new owning-smoke check asserts connection_state=registry_without_goal with state_file_exists=true, i.e. it reproduces the reported defect; with the change it passes. Negative twins pin that plain absence still keeps connect_if_needed and that a state file for a different Goal id does not block
integration passed this head: tests/control_plane, tests/cli_commands, tests/canary - 4573 passed, 8 skipped, 5 failed. The same selection on a clean worktree at this PR's merge base gives 4557 passed, 5 failed: the failure sets are identical and the passed delta is exactly the cases this PR adds. Four are CLI-subprocess cases that fail on this machine regardless of the change; the fifth is the tests/canary ratchet described in the static row
static passed ruff check clean on all five changed paths; python -m mypy clean; py_compile clean; module ceiling held: the oversized command-pack module is 2236 lines against its reviewed 2240 ceiling, so the canary's unreviewed-finding list is byte-identical to base (loopx/chat_server.py and loopx/extensions/lark/goal_topic_connections.py only, neither is a path this PR touches) and this PR adds no size debt
real_entrypoint passed the third review round's consumption surface, driven through the shipped CLI rather than asserted in-process: start-goal --guided --include-command-pack-detail and bootstrap-command-pack over an orphaned project, both fed to the same recursive guard, so the nested registration field, the slash-command catalog, the onboarding hint and both rendered Markdown bodies are covered, not just the top-level command map
manual passed the two projected preview routes were executed verbatim from the project root: backup-state --project . --current-project-only returned dry_run=true with wrote_local_registry=false, and checksums of the registry and the orphaned state file were identical before and after
integration failed the repository vocabulary-drift check fails in this environment for an unrelated reason: the TypeScript production parser is unavailable until dependencies are installed. It fails identically on a clean base worktree, so it is not attributed to this change
  • Coverage and gaps: all three bypasses found in review are closed, and closed once rather than per branch. Every absence route - a registry that cannot be parsed, a missing or empty one, and a readable registry with no matching entry - now builds its connection through one builder, which owns the orphan projection and records which absence produced the fence in absent_connection_state and absent_reason, so registry_invalid keeps its parse diagnostic instead of being relabelled. The fenced packet is likewise rebuilt from an allowlist that keeps only status plus the two preview routes, with the nested subtrees that carry continuation verbs dropped whole and a fenced standalone pack rendering its own text. The changed paths are the goal-start connection classification, the guided/command-pack projection of a continuation, and their tests, which the rows above execute through the real CLI entry point as well as in-process. Not covered here and not changed: the diagnose projection, host-binding instance identity, and any persisted state or receipt contract - no AuthorityStore, cursor, outbox or provider arm is touched, so no conformance arm applies. The full tests/ sweep is being re-run at this head and its result is recorded in a follow-up comment on this PR.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none
  • Attention review: no user-visible UI changes. Verified rather than assumed: no consumer of guided_transaction or connect_if_needed exists under the dashboard or desktop sources, so the affected surface is the CLI packet a host session executes.

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

Shared-authority RFC fixture impact

  • Production-scale fixture schema: N/A - no persisted schema, receipt or cursor contract changes.
  • Semantic dimensions changed, or reviewed no-impact rationale: N/A - the shared production-scale coordination fixture is not read by this path.
  • Provider conformance arms run: none; no provider or AuthorityStore selection changes.
  • Read-only legacy/file/PostgreSQL three-arm rehearsal (required for promotion, runtime-routing, or compatibility-projection changes): not applicable - no runtime routing or compatibility projection is changed.

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 (including .loopx/, .codex/goals/, and live ACTIVE_GOAL_STATE.md).
  • 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).

A project reset that deletes a Goal's registry entry can leave its
project-local active-state file behind. inspect_bootstrap_connection
classified that as the ordinary registry_without_goal absence, so guided
packets kept offering connect_if_needed, fresh-agent registration, Todo
writeback, host-loop activation and quota spend for the same human-readable
Goal id, and a later diagnose reported a healthy Goal over state an earlier
lane left behind.

Separate the two absences: when no registry entry matches but a state file for
that id still exists under a project goal-state route, the connection is now
orphaned_goal_state and the packet carries no mutation continuation. Only
inspection and the backup-state preview remain, and the gate records that
archiving or adopting project-local orphan state has no shipped command yet.

The whole fence lives in the new goals-domain module so the already oversized
bootstrap_command_pack keeps only its wiring and stays inside its reviewed
module ceiling.

Refs loopx-project#4801

Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>
Cover the detected fact over the current and legacy project state routes, the
blocking guided transaction, the negative twins (plain absence keeps
connect_if_needed, and a state file for another Goal id does not block), and
that the shipped onboarding entry qualifier reports route=stop with zero
actionable commands. Add the same invariant to the owning smoke through the
real CLI so a future packet change cannot silently restore a continuation.

Refs loopx-project#4801

Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>
@hahahahahayesyeseys

Copy link
Copy Markdown
Contributor Author

Full tests/ sweep, as deferred in the validation section.

  • This head (144072c): 91 failed, 10637 passed, 37 skipped in 41m41s.
  • Same sweep on a second worktree at the same base revision whose only difference is an unrelated examples/ smoke: 91 failed, 10628 passed, 37 skipped in 41m35s.
  • The two failure sets are identical: the set difference is empty in both directions. The only movement in the totals is the +9 tests this PR adds.

Attribution of the 91, all pre-existing on main in this environment and none introduced here:

group count reason
tests/architecture/test_semantic_* 88 the TypeScript production parser is unavailable locally, the same root cause as the vocabulary-drift advisory row above
tests/extensions/test_lark_goal_topic_runtime.py 2 date-expired absolute fixture timestamps, reported in #4797 and fixed by #4798
tests/canary/test_maintainability_ratchet.py 1 two unreviewed module findings (loopx/chat_server.py and loopx/extensions/lark/goal_topic_connections.py), byte-identical to base; this change keeps the command-pack module inside its reviewed ceiling (2236 against 2240) rather than raising it

Combined with the earlier domain-scoped run (tests/control_plane, tests/cli_commands, tests/canary: 4570 passed, 8 skipped, 1 failed with that same pre-existing ratchet finding), the changed paths are covered at both the focused and repository-wide level with no new failure.

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

评审 exact head:144072cdbee74cb37b7b91ff8d5b8778f7c86366

动机

这个 PR 解决的是 #4801 中一个真实且高风险的 authority discontinuity:项目注册表不再声明某个 Goal,但同 id 的 ACTIVE_GOAL_STATE.md 仍留在项目目录时,start-goal --guided 不能把它当成全新 Goal,继续提供 bootstrap、注册新 agent、写 Todo、启用 host loop 和消费 quota 的路径。否则同一个可读 Goal id 会同时承载旧状态和新 lane 的写权限。

把这项工作拆成“先阻断危险续跑”的独立 slice 是合理的。archive/adopt、diagnose 告警和 opaque Goal generation identity 可以后续完成,因为单独落地 fence 已经能减少实际危害;但 fence 必须覆盖 issue 定义的全部“没有匹配 registry Goal、却存在 state candidate”情形。

改动思路

实现新增 loopx/control_plane/goals/orphaned_goal_state.py,把四个历史/当前项目状态根目录的候选探测、orphaned_goal_state connection projection、operator gate、command-pack 清理和 guided Markdown 渲染集中到一个 owner。inspect_bootstrap_connection 在“registry 存在且没有匹配 Goal entry”时调用该 classifier;fence_command_pack 将 bootstrap、registration、planning、refresh、host activation、quota 等 continuation 清空;guided_fence 把 transaction 收敛为 inspect_connectionresolve_orphaned_goal_state 两步。

这个 ownership 方向是对的:orphan 是 registry/state 的派生事实,不是新的持久化真相;现有 state-backup 根目录得到复用,也没有偷偷 copy/merge 状态。inspect 与 backup 都保持 preview-only,未把 archive/adopt 伪装成已经可用的命令。

具体改动

  • orphaned_goal_state_routes.loopx/goals.codex/goals.claude/goals.local/goals 中按精确 Goal id 查找候选,只投影项目相对路径。
  • registry_missing_goal_connection 将 truthy registry 中的“无匹配 Goal”分成普通 registry_without_goalorphaned_goal_state
  • fence_command_pack 清空 7 个 mutation continuation,并把 safety contract、recommended next step 和 resolution gate 对齐。
  • guided_fence 替换原 transaction 的 ordered steps,同时移除 identity selection gate;onboarding qualifier 因而返回 route=stop
  • 新增 9 个 focused tests,覆盖四种根目录、普通 absence、connected Goal、command-pack/guided parity 与 qualifier stop;bootstrap smoke 也新增了真实 CLI 形状。

阻塞问题在 loopx/bootstrap_command_pack.py:592if not registry 仍然在新 classifier 之前直接返回 not_connected。我在 exact head 上用 shipped CLI 构造“没有 .loopx/registry.json,但 .codex/goals/reset-goal/ACTIVE_GOAL_STATE.md 存在”的项目,返回结果同时满足:

  • state_file_exists=true
  • connection_state=not_connected
  • transaction 仍包含 connect_if_neededselect_agent_identity、Todo planning/writeback、refresh、activation、quota guard
  • command pack 仍给出 bootstrap、registration、plan 和 refresh continuation

这正是 #4801 要阻断的第二 authority 风险。当前测试 helper 总会写一个 truthy registry(含另一个 live Goal),所以 9 个新测试全部通过也不会触达这个 bypass。请在 ordinary missing/empty-registry return 前复用同一 orphan projection:只有完全没有 candidate 时才返回 not_connected;并补 missing registry、empty registry 与对应 no-state negative twins,最好走真实 CLI。

对主干的风险

已覆盖的 truthy-registry 分支风险控制得不错:普通全新 Goal 与已连接 Goal 的 negative twins 保持旧行为;gate 本身不写状态,所有 projected resolution route 都不含 --execute;当前 head 与最新 origin/main 没有发现文本冲突。

但是未覆盖分支的 blast radius 很大:operator reset 若直接移除整个 registry,host 会拿到普通 onboarding packet,依次获得 bootstrap、fresh-agent、Todo、host-loop 和 quota 的可执行引导。这个漏洞不是 presentation 差异,而是 fence authority 的 false negative,因此不能在当前 head 批准。

本地验证:focused tests 9 passed in 0.94sexamples/bootstrap-command-pack-smoke.py 通过;changed paths Ruff 通过;git diff --check 通过。按 Goal 配置 wait_for_ci=false,没有抓取或等待远端 CI。真实 missing-registry CLI 反例失败,作为本次 REQUEST_CHANGES 的决定性证据。

我的整体评价

这是一个有价值、边界合理且总体架构方向正确的 safety slice;新模块没有制造第二份持久化 authority,未来相关的小型重构也已经体现在集中 classifier/fence owner 上,无需再引入更大的 framework。当前唯一 blocker 很具体:classifier 必须覆盖 registry file 缺失/empty 的同一 orphan invariant。修复后请在新 exact head 重新跑 focused + smoke + real CLI twins;archive/adopt、diagnose 和 generation identity 仍可按 PR 已声明的后续边界继续,不要求在本 PR 扩 scope。

English verdict: REQUEST_CHANGES - head 144072cdbee74cb37b7b91ff8d5b8778f7c86366 correctly fences orphaned state when a truthy registry lacks the Goal, and 9 focused tests, the bootstrap smoke, Ruff, and diff check pass; however, a real exact-head CLI reproduction with a missing registry and surviving state still returns not_connected and exposes bootstrap/registration/Todo/activation/quota continuations, so the same classifier must cover missing and empty registries before approval.

The first version classified orphaned state only inside the readable,
non-empty registry branch, so a reset that removed .loopx/registry.json
outright still reached the early not_connected return and a guided packet
carrying bootstrap, fresh-agent registration, Todo planning and writeback,
host-loop activation and quota guard for a Goal id whose state file survived.

Move the shared classifier above both absence returns: the ordinary case keeps
exactly the fields and connection_state it returned before (registry_exists and
the reason text included), and only a surviving state candidate switches it to
orphaned_goal_state. The command-pack size ceiling still holds (2233 against
2240), because the classifier now replaces two inline literals rather than
adding a third branch.

Refs loopx-project#4801

Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>
Parametrized twins for a missing and an empty registry pin both halves:
surviving state fences the packet with no actionable command, while a genuinely
fresh project keeps its ordinary onboarding continuation and its previous
connection_state. Add the missing-registry case to the owning smoke through the
real CLI so the bypass cannot return silently.

Refs loopx-project#4801

Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>

@hahahahahayesyeseys hahahahahayesyeseys left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker fixed at this head (a0ca91d1). The classifier now runs above both absence returns instead of only inside the readable-registry branch, so a reset that removes the registry file itself can no longer reach a normal onboarding packet.

Your repro, re-run through the shipped CLI at this head (synthetic project with only .codex/goals/reset-goal/ACTIVE_GOAL_STATE.md present and no .loopx/registry.json at all):

  • connection_state: orphaned_goal_state, registry_exists: false
  • guided_transaction.blocked_by: orphaned_goal_state, ordered steps exactly inspect_connection + resolve_orphaned_goal_state
  • bootstrap_dry_run_preview, bootstrap_after_user_confirmation, goal_start_connect_if_needed, goal_start_plan_prompt, goal_start_refresh_state, goal_start_host_loop_activation, goal_start_quota_should_run all null
  • projected orphan route reported relative to the project, nothing written

Re-runs you asked for at the new exact head

check result
focused module tests 13 passed - four new parametrized cases (missing and empty registry, each fenced and each cleared) on top of the previous nine
owning smoke passes, now including the missing-registry case driven through the real CLI entry point
module ceiling the command-pack module is 2233 lines against its reviewed 2240, so the canary's unreviewed-finding list is still exactly the two pre-existing paths
static ruff check and py_compile clean on every changed path

The ordinary-absence half is pinned to be unchanged rather than merely un-broken: a genuinely fresh project still returns not_connected with the same fields it carried before (including registry_exists and the reason text, and no new keys), and its packet still exposes connect_if_needed with a non-null command.

One correction to my own framing, and one case left to you. A readable registry declaring no goals (goals: []) was already covered by the first head, because it falls through to the matching branch; the genuinely open path was the missing-file case your repro found. Both are pinned now, but for different reasons, and the test names say so.

The same false-negative shape still exists one branch earlier: when the registry file exists but cannot be parsed, inspect_bootstrap_connection returns registry_invalid before the classifier, and I measured that a packet over surviving state still carries connect_if_needed, select_agent_identity, Todo planning and writeback, refresh, activation, quota guard and the scheduler ack step, with goal_start_connect_if_needed non-null. I did not fold it in here because overriding connection_state there would bury the parse error that is the operator's actual next action, and that trade-off is yours rather than mine to make. Two shapes I would accept from you: keep registry_invalid as the state and just strip the continuations when a candidate exists, or classify it as its own blocked state that names both facts. Say which and I will land it on this branch.

archive/adopt, diagnose surfacing and opaque Goal instance identity stay as the declared follow-ups.

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

评审 exact head:a0ca91d1d61341df260aaaf49742314ef9bca4dc

动机

这个 PR 处理 #4801 中一个真实且高风险的 authority discontinuity:项目注册表不再声明某个 Goal,但同 id 的 ACTIVE_GOAL_STATE.md 仍留在项目目录时,start-goal --guided 不能把它当成全新 Goal,继续提供 bootstrap、注册新 agent、写 Todo、启用 host loop 和消费 quota 的路径。否则同一个可读 Goal id 会同时承载旧状态和新 lane 的写权限。

本轮复审确认上一轮指出的 missing/empty-registry 漏洞已经修复:当前 head 会在这些分支调用统一的 orphan classifier,并保留普通无状态新项目的旧行为。这是实质性进展。但我继续沿着同一 authority invariant 检查真实输入边界,发现 invalid/unparseable registry 仍从更早的分支绕过 classifier,因此当前 head 仍不能批准。

改动思路

实现把候选状态探测、orphaned_goal_state connection projection、operator gate、command-pack 清理和 guided Markdown 渲染集中在 loopx/control_plane/goals/orphaned_goal_state.pyinspect_bootstrap_connection 现在不仅覆盖“registry 存在但无匹配 Goal entry”,也通过 unregistered_goal_connection 覆盖 missing/empty registry;fence_command_pack 将 bootstrap、registration、planning、refresh、host activation、quota 等 continuation 清空;guided_fence 把 transaction 收敛为只读 inspect 与 resolution preview。

这个 ownership 方向仍然正确:orphan 是 registry/state 的派生事实,不是新的持久化真相;四个 current/legacy state roots 得到复用,也没有偷偷 copy、adopt 或合并状态。此次修复也没有复制第二套 fence。剩余问题应继续复用同一 classifier,而不是新增 registry-error 专用安全模型。

具体改动

  • orphaned_goal_state_routes.loopx/goals.codex/goals.claude/goals.local/goals 中按精确 Goal id 查找候选,只投影项目相对路径。
  • goal_connection_without_matching_entry 现在同时服务 registry-without-goal 与 unregistered project,两类路径都能区分普通 absence 和 orphaned state。
  • fence_command_pack 清空 mutation continuations,并把 safety contract、recommended next step 和 resolution gate 对齐;guided_fence 同步移除 identity selection 与后续 mutation steps。
  • focused tests 增至 13 个,新增 missing registry、empty registry 及 no-state negative twins;bootstrap smoke 也覆盖了真实 CLI 形状。

剩余 blocker 位于 loopx/bootstrap_command_pack.py:580if registry_errorunregistered_goal_connection 和 orphan projection 之前直接返回 registry_invalid。我在当前 exact head 上构造了一个 disposable project:.loopx/registry.json 内容不可解析,同时 .codex/goals/reset-goal/ACTIVE_GOAL_STATE.md 存在。shipped source CLI 的结果同时满足:

  • connection_state=registry_invalid
  • state_file_exists=true
  • safety contract 仍报告 orphaned_goal_state_blocks_continuation=false
  • transaction 仍包含 connect_if_neededselect_agent_identity、Todo planning/writeback、refresh、activation 和 quota guard
  • command pack 仍给出 bootstrap、registration、plan 与 refresh continuation

这和上一轮 missing-registry 反例的安全后果相同:registry 已经不再提供可信 Goal authority,但 surviving state 存在时 host 仍拿到了建立第二 authority 的可执行引导。registry_invalid 诊断当然要保留;需要修的是它不应同时放行 mutation continuation。最小修复是在 registry-error 分支探测同一 orphan candidate,保留 parse error/registry_invalid 事实,但投影现有 no-continuation fence。请补 invalid-registry + surviving-state 的真实 CLI regression,以及 invalid-registry + no-state 的 negative twin,确保不是把所有 registry 解析错误都误报成 orphan。

对主干的风险

已覆盖的分支风险控制得不错:普通全新 Goal、missing/empty registry 无 candidate、已连接 Goal都保留既有行为;gate 本身不写状态,resolution routes 也保持 preview-only。当前 head 的 missing/empty 修复没有引入新的持久化状态或权限。

但 invalid registry 并非罕见的理论输入:中断写入、人工编辑或部分损坏都可能触发 parse error。此时 packet 一方面告诉 operator registry 不可信,另一方面仍提供 bootstrap、fresh-agent、Todo、host-loop 和 quota continuation;这是一个相互矛盾的 authority projection。影响范围跨过 CLI presentation,进入后续写路径,因此仍是 P1 blocker。

本地验证:tests/control_plane/test_start_goal_orphaned_goal_state.py13 passed in 2.90sexamples/bootstrap-command-pack-smoke.py 通过;changed paths Ruff 通过;git diff --check 通过。按 Goal 配置 wait_for_ci=false,没有抓取或等待远端 CI。真实 invalid-registry CLI 反例未满足安全 invariant,是本次 REQUEST_CHANGES 的决定性证据。

我的整体评价

当前 head 正确修复了上一轮反馈,而且集中 classifier/fence owner 的方向合理;bounded future-facing pass 不需要再引入新的 framework。剩余修改也很局部:让 parse-error 分支保留诊断但复用既有 orphan fence,并用 state/no-state twins 固定语义。修复后请在新 exact head 重新跑 focused tests、bootstrap smoke、Ruff、diff check 与真实 CLI twins;archive/adopt、diagnose 和 Goal-generation identity 仍可保持本 PR 已声明的后续边界,不要求扩大范围。

English verdict: REQUEST_CHANGES - head a0ca91d1d61341df260aaaf49742314ef9bca4dc fixes the prior missing/empty-registry bypass, and 13 focused tests, the bootstrap smoke, Ruff, and diff check pass; however, an exact-head real-CLI reproduction with an invalid registry and surviving Goal state still reports registry_invalid while exposing bootstrap, registration, Todo, activation, and quota continuations, so the existing orphan fence must also cover that branch while preserving the parse diagnostic.

@hahahahahayesyeseys

Copy link
Copy Markdown
Contributor Author

Recording the CI attribution for this head so nobody has to re-derive it: both red lanes here reproduce outside this PR.

Python Tests - 11 failing tests at this head, and the same workflow on main's own push produced the identical set: the difference in both directions is empty (main run vs this PR run). The files are tests/control_plane/test_settled_replay_construction.py (8), test_blocked_transition_notice.py (2) and test_selection_replan_reentry.py (1). None of them imports or renders the goal-start packet this PR changes.

node-minimum-compatibility - 2 failures, sqlite legacy|native full mixed head reports effective ownership without writes. Both fail closed on the runner's bundled SQLite 3.50.2 against the qualified floor (3.51.3+ / 3.50.7+ / 3.44.6+) and ask for the Node 22.22.3 runtime; the message is a runtime-qualification gate, not an assertion about ownership. An unrelated PR opened the same window fails those same two tests, and this PR touches no TypeScript, provider or Node path.

Local exact-head numbers for this branch remain: 13 focused tests passed, the owning smoke passed, ruff and py_compile clean, and the command-pack module stays at 2233 against its reviewed 2240 ceiling.

@hahahahahayesyeseys

Copy link
Copy Markdown
Contributor Author

Full tests/ sweep at this head (a0ca91d1), completing the row the body deferred: 91 failed, 10641 passed, 37 skipped in 38m59s.

Against the same sweep at the reviewed head 144072cd (91 failed, 10637 passed), the failure sets are identical - the difference in both directions is empty - and the passed count moved by exactly the 4 cases added for the registry-absence branch. The 91 remain the pre-existing set attributed earlier: 88 tests/architecture/test_semantic_* from the unavailable local TypeScript parser, 2 date-expired Lark fixtures (the base predates 277f43ab2, which froze that clock on main), and the tests/canary maintainability ratchet that is red on main.

@Job28703

Copy link
Copy Markdown

Independent review of exact head a0ca91d1d61341df260aaaf49742314ef9bca4dc (adversarial review chain, reviewer PASS at second revision):

Verdict: REQUEST_CHANGES — concurring with the second maintainer round.

The fence correctly covers missing/empty registries (13 focused tests, real-CLI smoke, roots aligned verbatim with state_backup.py:154-157). The remaining P1 blocker, verified by reading the head source directly: bootstrap_command_pack.py:580 returns registry_invalid before any orphan classification, so an unparseable registry with surviving ACTIVE_GOAL_STATE.md still exposes bootstrap/registration/Todo/quota continuations — same second-authority risk the PR exists to close. The disclosed keep-reason (preserving the parse diagnostic) does not conflict with projecting the existing fence alongside it.

CI note: the pytest/test-shard reds at this head come from test_settled_replay_construction.py (8), test_blocked_transition_notice.py (2), test_selection_replan_reentry.py (1) — none import or render the goal-start packet this PR changes; the file does not even exist in the PR tree (CI runs the merge ref). Node-min's two sqlite cases are the SQLite 3.50.2 runtime-qualification gate, fixed for the lease-inspection suite by #4857.

Suggested fix shape is the maintainer's: probe the same orphan candidate in the registry-error branch, keep the registry_invalid diagnostic, project the no-continuation fence, and pin invalid-registry ± surviving-state real-CLI twins.

@Job28703

Copy link
Copy Markdown

Independent Review Comment — PR #4808 (English)

Review of exact head a0ca91d1d61341df260aaaf49742314ef9bca4dc (4 commits, merge-base 4eb6ea9eed3a; branch is 326 commits behind main). Change surface: +821/−23 across 4 files — new module loopx/control_plane/goals/orphaned_goal_state.py (305 lines), rewiring in loopx/bootstrap_command_pack.py, plus 363 lines of new tests and two real-CLI smoke cases.

Verdict: REQUEST_CHANGES — concurring with the second maintainer round, on one specific P1 gap detailed below. Everything else in this slice is solid.

What this PR gets right

  • Single-owner fence. All detection, projection, gate, command-pack scrubbing, and guided-Markdown rendering live in one module; bootstrap_command_pack.py keeps only wiring (:594, :613, :1077, :1711-1713, :1737, :1894, :1924).
  • Detection roots match the backup surface verbatim. GOAL_STATE_ROOTS (.loopx/.codex/.claude/.local × goals, orphaned_goal_state.py:39-44) aligns exactly with the four add(...) calls in state_backup.py:154-157 — so a reset that moved or kept state under any root is caught, not just .codex. This alignment is claimed in the module docstring and holds.
  • No second fence. One classifier (goal_connection_without_matching_entry:104) serves both the registry-without-goal and unregistered-project branches; the two wrappers only differ in absence semantics.
  • Preview-only resolution. Both gate routes carry mutates: false, contain no --execute, and the unavailable routes (archive / adopt) are disclosed as unavailable rather than projected as working commands.
  • Non-widening pinned by tests. Plain-absence and connected-goal negative twins keep old behavior; candidate scoping is bound to the requested goal id; routes stay project-relative (test asserts the absolute path never leaks into host-facing artifacts).
  • Schema hygiene. orphaned_goal_state_blocks_continuation is explicitly false on non-orphan paths (presence-checked via isinstance), not merely absent. Legacy fingerprints are preserved; v0/v1 requests reject the new fields instead of silently ignoring obligations.

The P1 blocker (unchanged from round 2)

loopx/bootstrap_command_pack.py:580 — the if registry_error: branch returns registry_invalid before any orphan classification, so an unparseable registry with a surviving ACTIVE_GOAL_STATE.md still gets the full mutation continuation set (bootstrap, fresh-agent registration, Todo writeback, host-loop activation, quota). I verified the branch order directly in the head source (:580-589 precedes both classifier call sites). This is the same second-authority risk the PR exists to close: the packet simultaneously declares the registry untrusted and hands out the path to write over surviving state under the same goal id.

The PR body's stated reason for not fixing this (preserving the parse diagnostic the operator must see) does not conflict with the fix shape: probe the same orphan candidate in the registry-error branch, keep the registry_invalid state and reason, and project the existing no-continuation fence alongside it. Please add invalid-registry ± surviving-state real-CLI twins (and an invalid-registry no-state negative twin) so the semantics stay pinned.

CI note (attribution, not a defect of this PR)

The six red checks at this head trace to merge-ref conditions outside the branch: test_settled_replay_construction.py does not exist in the PR tree (404 via contents API) — its failures come from main's own 06:24–15:12 red window and were fixed on main after 15:12; the two node-minimum sqlite cases are the runner's SQLite 3.50.2 qualification gate (same on current main, fixed for the lease-inspection suite by #4857). Attribution is clean, but the consequence stands: merge-gate red means this head cannot merge until rebased and re-run green.

Suggested path to approval

  1. Extend the registry-error branch with the same orphan probe; keep the diagnostic; project the fence.
  2. Add the invalid-registry ± state real-CLI regression pair.
  3. Rebase onto current main (picks up test(ci): qualify lease inspection on public Node #4857/test(project-lifecycle): align settlement readback fixture #4859 and clears the merge-ref window), re-run focused tests + smoke + Ruff + diff check on the new exact head.

The maintainer's second-round review already sketches this fix; nothing here requires expanding scope beyond what that round accepted (archive/adopt, diagnose surfacing, and instance/generation identity remain correctly out of scope).

English verdict: REQUEST_CHANGES

The unparseable-registry return built its own connection dict and handed
back bootstrap, registration, Todo, activation and quota continuations
while Goal state for the same id was still on disk, so the fence was
reached only by the absences that happened to call it.

All three absence returns now go through absent_goal_connection, which
owns the orphan projection once and keeps which absence produced the
fence in absent_connection_state and absent_reason. The gate no longer
explains why archive and adopt are not reachable yet; that belongs to
loopx-project#4801's remaining work, not to a packet field.

Refs loopx-project#4801

Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>

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

动机

这个 PR 对准 #4801 的关键安全问题:项目 registry 已经不再声明某个 Goal,但同名 ACTIVE_GOAL_STATE.md 仍存在时,系统不能把它当作普通“尚未连接”,否则会给新 lane 暴露 bootstrap、agent registration、Todo、quota 或 host activation 命令,在旧状态上形成第二个 authority。

改动思路

总体设计方向正确:把“registry absence + 同目标 state file 存在”建模为派生的 orphaned_goal_state,让 missing、empty、invalid 和 readable-without-goal 四类 absence 统一经过一个判断;再把 guided transaction 收敛为只读 inspect + backup preview,并保留原始 absence 原因用于诊断。状态文件根与 backup-state 的 current/legacy 范围对齐,没有引入第二份持久事实。

具体改动

  • 新模块 loopx/control_plane/goals/orphaned_goal_state.py 负责精确目标路径检测、派生 projection、resolution gate、command-pack fence 和 guided rendering。
  • bootstrap_command_pack.py 将各条 no-goal 返回路径接入统一 builder,并在完整 command pack 生成后执行 fence。
  • 测试覆盖四个 state roots、requested-goal 精确匹配、missing/empty/invalid registry、普通 absence parity,以及 guided ordered steps 和主要顶层命令的清空。
  • Focused tests 为 15 passedbootstrap-command-pack-smoke.py 也通过,git diff --check 干净。

但当前 exact head 仍有一个 P1 blocker:fence_command_pack 只把 MUTATION_CONTINUATION_COMMANDS 中七个顶层 key 设为 None。在 orphaned Goal 且没有 agent_id 的真实 builder 输出里,以下路径仍携带可执行注册命令:

  • commands.fresh_agent_registration.execute_command
  • host_loop_activation.identity_selection_gate.fresh_agent_registration.execute_command

而 standalone command-pack 的 Markdown 仍直接渲染:

loopx register-agent --goal-id reset-goal --agent-id '<new-public-safe-agent-id>' --require-new --execute

这与同一 payload 的 forbidden_until_resolved 包含 agent_registration、以及 host_loop_activation_allowed=false 直接矛盾。现有测试只检查了选定的顶层 key 和 guided ordered steps,因此即使全部通过,也没有覆盖 full/detail packet 与 standalone renderer 这个消费面。

对主干的风险

这是 authority 边界风险,不是展示瑕疵:host 若消费完整 command pack 或 standalone 文本,仍可能执行本 PR 明确禁止的 registration,从而产生它要阻止的第二 authority。类似遗漏也容易在未来新增 nested command 时复发,因为当前策略是有限 denylist。

最小修复建议:orphan 分支生成一个显式的安全 projection/allowlist,只保留 status 与 backup-state preview;至少要同步清空 fresh_agent_registration、identity gate、bind/configure 及所有其他 mutation-bearing nested surfaces。新增一个通过真实 guided-detail 与 standalone builder 的递归回归,断言除白名单只读 route 外不存在 --execute、register/bind/configure/Todo/quota/activation 命令,并检查最终渲染文本。

我的整体评价

检测模型、统一 absence 路由和只读 resolution gate 都是有价值且方向正确的,invalid-registry 漏路由也已在当前 head 修复。但核心承诺必须覆盖所有公开消费面;目前机器 safety flag 与同包可执行指导互相冲突,不能批准。修掉 nested/full-renderer 泄漏并补上递归回归后,这个 PR 的边界就能收敛。

English verdict: REQUEST_CHANGES - head a472baedeaa84a004f19ee0bf683d505a977b1e7; the orphan gate still exposes register-agent --execute through nested full/detail command-pack fields and the standalone rendered message despite focused tests passing.

The fence nulled seven top-level command keys, so the same payload still
offered `loopx register-agent --require-new --execute` through nested
registration and identity-gate fields, the issue-fix post-PR templates, the
slash-command catalog and the onboarding hint -- a continuation by
instruction, contradicting this very packet's forbidden_until_resolved.

`commands` is now rebuilt from an allowlist that keeps only `status`, the
verb-bearing subtrees are dropped whole, and a fenced standalone pack renders
its own body instead of the connect/plan/activate template. The guard over
what a packet may spell out lives in control_plane/testing so the focused
test and the shipped CLI smoke enforce one rule across every surface,
including the rendered Markdown.

Refs loopx-project#4801

Signed-off-by: hahahahahayesyeseys <95999512+hahahahahayesyeseys@users.noreply.github.com>
@hahahahahayesyeseys

Copy link
Copy Markdown
Contributor Author

Round 3 addressed at this head (c7870599).

The finding was right, and the surface was wider than the two paths named. Rebuilding the fenced pack from an allowlist instead of a suppression list was your suggestion; taking it removed a class rather than two instances. Measured on the shipped CLI at head a472baede, before this change, an orphaned packet still spelled out:

where what it offered
commands.fresh_agent_registration.execute_command loopx register-agent … --require-new --execute
host_loop_activation.identity_selection_gate.fresh_agent_registration the same, a second copy
commands.issue_fix_pr_lifecycle_template loopx issue-fix pr-lifecycle … --execute
commands.issue_fix_reviewer_request_template loopx issue-fix reviewer-request … --execute
goal_start_contract.domain_route_hints.issue_fix_workflow.post_pr_* the same two verbs again, in the planner hints
available_slash_commands.commands[].cli_reference loopx start-goal --guided …, loopx agent-onboard …
onboarding_hint.suggested_user_note a "useful commands" list including loopx todo add
the standalone rendered message "Write the planned todos with loopx todo add …"

Six of those eight were outside what the previous fence read at all, which is the predictable result of the strategy being a finite denylist over the top-level commands map.

What changed here:

  • commands is now rebuilt from an allowlist: status is the only key a fenced packet keeps. A key any future builder adds to that map is withheld by default instead of needing to be named in a suppression list.
  • Whole verb-bearing subtrees are dropped rather than filtered field by field: host_loop_activation, available_slash_commands, onboarding_hint, and goal_start_contract's activation / domain_route_hints / execution_invariants.
  • A fenced standalone command pack renders its own body (the fact, what is withheld, and its two preview routes) instead of the shared template, whose body is connect / plan-write / activation guidance.
  • The regression you asked for is recursive and shared: loopx/control_plane/testing/continuation_verb_guard.py states the rule once, and both the focused test and the shipped smoke run it over the guided packet with --include-command-pack-detail, the standalone command pack, and both rendered Markdown bodies. Any line that spells a mutation family or carries --execute fails. It also asserts the two read-only routes survive, so it cannot pass on a packet that says nothing.

Validation at this head, local and exact: focused tests 16 passed; examples/bootstrap-command-pack-smoke.py passed including the new recursive case through the real CLI; Ruff clean on all five changed paths; python -m mypy clean; git diff --check clean.

Two mutation runs, so the guard can be checked rather than trusted:

  • Restoring the two subtrees the review did not name (available_slash_commands, onboarding_hint) to not-scrubbed fails the focused test and the shipped smoke, naming packet.command_pack.available_slash_commands.commands[1].cli_reference.
  • Removing the fence entirely fails the whole fenced group at once, because one builder owns every absence route.

loopx/bootstrap_command_pack.py is 2236 lines against its reviewed 2240 ceiling; the canary's unreviewed-finding list stays byte-identical to base.

One judgement call worth your eye: commands.doctor and the issue-fix read-only planning templates are now withheld too, because the allowlist keeps only status. They are non-mutating, but they are planner scaffolding for a Goal this packet says must not be started; if you would rather the fenced packet keep the read-only planner templates, that is a one-line extension of FENCED_COMMAND_KEYS rather than a redesign.

Wide selection at this head, same machine and environment: tests/control_plane, tests/cli_commands, tests/canary -> 4573 passed, 5 failed, 8 skipped in 25m21s. Those five are the pre-existing local set for this checkout, reproduced identically on a second clean worktree at this PR's merge base (4557 passed / 5 failed there, and the passed delta is exactly the cases this PR adds): four are host-loop/CLI-subprocess cases that fail on this machine regardless of the change, and the fifth is tests/canary/test_maintainability_ratchet.py, whose unreviewed-finding list is loopx/chat_server.py and loopx/extensions/lark/goal_topic_connections.py -- byte-identical to base, and it does not name loopx/bootstrap_command_pack.py.

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

动机

我按 exact head c78705998ddf4d15b38e2362bb1a13bd90b4b7f6 重新检查了 #4801 要求的核心结果:当 registry 已无该 Goal、但项目目录仍保留 ACTIVE_GOAL_STATE.md 时,任何公开的 guided/full/standalone packet 都不能再给出 bootstrap、注册 agent、Todo 写入、quota spend 或 host-loop activation 的可执行延续。上一版已能识别 orphan state,但完整 packet 和 Markdown 仍泄漏 register-agent --execute;本次 head 必须同时关闭机器字段与人类可见文案的这条路径。

改动思路

当前实现不再维护“已知危险命令”的窄 denylist,而是从安全面重新构建 orphan packet:FENCED_COMMAND_KEYS = ("status",),其余命令键一律置空;available_slash_commandshost_loop_activationonboarding_hint 等携带延续语义的子树整体移除;goal_start_contract 中 activation/domain route/execution invariants 也被删去。standalone 输出不再复用普通 onboarding renderer,而是走 fenced_standalone_message,只呈现 status 与不带 --execute 的 backup-state preview。

这个边界是合适的:registry/state facts 仍是唯一判定依据,投影不持久化,未知的未来命令键默认被 withholding,而不是等维护者记得补 denylist。

具体改动

  • loopx/control_plane/goals/orphaned_goal_state.pyfence_command_pack 中保留唯一安全命令 status,清空其他 command key,并移除 activation/onboarding continuation subtrees。
  • 同文件新增 dedicated standalone renderer;guided transaction 也只保留 inspect 与 backup preview 两个 read-only resolution routes。
  • loopx/bootstrap_command_pack.py 在 fenced branch 选择专用 renderer,不再让普通注册/激活叙事从共享 renderer 渗出。
  • 新增共享 continuation_verb_guard.py,由 focused tests 与 shipped smoke 同时检查 nested packet 和 rendered message。
  • 独立验证结果:tests/control_plane/test_start_goal_orphaned_goal_state.py 为 16/16 通过;examples/bootstrap-command-pack-smoke.py 通过;五个变更文件 Ruff 通过;Python compile 通过;git diff --check origin/main...HEAD 通过。按 packet 的 wait_for_ci=false,本次没有把远端 CI 当作判定依据。

对主干的风险

先前的 P1 已关闭:当前生产 builder/renderer 的 orphan 分支没有再暴露注册或其他 mutation continuation,ordinary absence 与 connected counterfactual 也由测试保留。

还有一个非阻断 P2:continuation_verb_findings 只扫描包含字面量 loopx 的行,并依赖手工枚举的 command family。CLI 已支持 --cli-bin "python -m loopx.cli",因此测试 helper 会漏掉 python -m loopx.cli register-agent --execute,未来未列入 family 且不带 --execute 的新 mutation verb 也可能漏检。这不会重新打开当前产品漏洞,因为生产 fence 是结构性 allowlist;但会削弱长期回归 oracle。建议让测试直接断言 fenced packet 的精确安全形状,并规范化支持的 cli-bin 前缀,补一个 non-default cli-bin twin。

future-facing pass 已应用在生产边界:denylist 已收敛为 fail-closed safe projection。剩余建议只涉及测试 oracle,不需要为了它阻塞本次权威修复。

我的整体评价

这版完整修复了上一轮指出的 nested packet/standalone Markdown 泄漏,并把“谁拥有安全边界”收敛到 orphan projection 本身;改动和原始风险成比例,验证覆盖了真实入口与负例。我批准这个 exact head,同时建议后续顺手增强 custom cli-bin 下的回归探针。

English verdict: APPROVE - head c78705998ddf4d15b38e2362bb1a13bd90b4b7f6; the orphan-state packet is now structurally fail-closed across guided, full, and rendered surfaces, with only a non-blocking test-oracle hardening suggestion remaining.

@huangruiteng
huangruiteng merged commit ea1d4b7 into loopx-project:main Sep 22, 2026
21 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.

3 participants