Skip to content

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

Open
hahahahahayesyeseys wants to merge 2 commits into
loopx-project:mainfrom
hahahahahayesyeseys:codex/guard-orphaned-goal-state
Open

hahahahahayesyeseys wants to merge 2 commits into
loopx-project:mainfrom
hahahahahayesyeseys:codex/guard-orphaned-goal-state

Conversation

@hahahahahayesyeseys

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), the blocking guided transaction and command pack, an operator-facing gate that keeps only the two routes that actually run today, and regression coverage 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, and the gate says so instead of projecting a command that fails; (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: 144072c
  • 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
unit passed new module tests: 9 passed - detection over all four project state routes, the two absences separated, candidate matching scoped to the requested id, blocking transaction and command-pack parity, and the shipped onboarding qualifier reporting route=stop with zero actionable commands
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 tests/control_plane, tests/cli_commands, tests/canary: 4570 passed, 8 skipped, 1 failed - the failure is pre-existing, see the static and not_applicable rows
static passed ruff check clean on all four changed paths; 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 (two findings, same paths) and this PR adds no size debt
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: 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 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.

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