Repository navigation
fix(codex-app): refuse automation install without a resolved target thread - #6124
huangruiteng merged 2 commits into
Conversation
…hread _ensure_automation treated an unresolved thread binding as an empty target_thread_id. The install then printed `created missing automation` and `applied ... and ACKed` with exit code 0 while the manifest recorded `target_thread_id = ""`, so an automation that no host-side readback can observe was created without a warning. Host-side readback resolves a goal's threads through the goal's thread_agent_bindings, and the upgrade/adoption reader already fails closed on a thread id the stores disagree about. Resolve the binding in one helper that refuses the install unless the goal holds exactly one usable thread for the agent, and keep that refusal before any manifest or SQLite write. Signed-off-by: GZY-SUPER-HACKER <162807803+GZY-SUPER-HACKER@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=xhigh
Exact reviewed head: 6124@ed78a4a6157041666086fc94fe777030608bc0e1. Immutable PR merge base: e57b49e7b6b485c7f3c364e22b3abe078e693b9b; current target main: 0b78704c6340508cfdfca6ec9b16c4c871840555. 使用完整 PR diff,未把新主干历史作为本 PR 改动。
动机
安装 Codex App 自动化的操作者,需要把定时任务绑定到明确的已有线程。
此前缺失或歧义绑定仍会安装目标线程为空的自动化并报告成功;现在安装前拒绝这两种情况,避免留下无法正确定位的任务。
真实脚本子进程与隔离 TOML/SQLite 文件证明:缺失、空白和歧义绑定在任何文件写入、prompt 获取及 ACK 前拒绝;唯一绑定正常安装,相邻 Agent 和无需 apply 的路径保持原行为。但拒绝后的修复命令无效。
本次不执行真实安装、不激活或改写正在运行的自动化,也不授予新的账户、仓库或合并权限。
拒绝后的绑定指引必须改为实际可调用的根命令,并通过解析及恢复路径验证。
改动思路
先拒绝未解析的目标,并保留现有宿主文件写入器,属于合理的局部修复;当前 PR 内补齐可运行的绑定指引即可,不需要新的状态字段或调度框架。
保留安装前的明确拒绝和原绑定写入路径,修复当前错误指引后再批准;不借此扩大宿主权限或改写多宿主选择规则。
Doing nothing retains the demonstrated defect. A new control-plane or installer framework is unnecessary; the nearest existing owner supports this bounded fix. Ambiguous multi-host selection remains an explicit refusal; this review does not invent a new host-disambiguation policy.
具体改动
独立规格依据 docs/reference/protocols/codex-app-host-command-registry-v0.md,固定改动前 revision e57b49e7b6b485c7f3c364e22b3abe078e693b9b。当前缺陷还按实际绑定写入器和 CLI parser 核验。
Project Root And Agent Identity: implemented — Resolve durable host-thread identity through the registered Goal/Agent binding, not a guessed or empty target.Handoff Packet: not_met — Selected identity is persisted with the existing root loopx bind-agent-thread command; the same binding is reused for host activation.
关键代码讲解
_resolve_bound_thread_id(scripts/codex_app_apply_rrule.py:274):从所选 Agent 的现有绑定派生唯一非空目标,缺失或歧义在写入前拒绝;拒绝消息的命令有误。_ensure_automation(scripts/codex_app_apply_rrule.py:308):保留已有 TOML/SQLite 写入和完全已安装的路径;新安装先解析目标,再获取 prompt、写 manifest 和数据库。
读取完整现有绑定列表后,只筛选明确传入的Agent;缺失、空白和多个可用目标先拒绝。真实隔离子进程证明这时没有manifest、SQLite文件、prompt调用或ACK。唯一绑定随后读取prompt、写TOML/SQLite并ACK;相邻Agent不干扰,apply_needed=false不执行安装。
48项相关Python测试通过,基线46项通过,无skip;十二次真实脚本子进程验证正反路径。当前风险canary4项直接检查和2项选中执行通过。
对主干的风险
[P2] Use the real root binding command in the refusal (scripts/codex_app_apply_rrule.py:301):The emitted loopx registry bind-agent-thread is rejected as unrecognized arguments by this exact source CLI. root loopx bind-agent-thread is the registered command, so the new fail-closed path gives the operator a dead recovery instruction. 最小修复:Use root loopx bind-agent-thread with the required goal, agent, host-surface and thread arguments; test its parser and refusal-to-valid-binding recovery without changing live host state.
语义与验证边界
既有字段和命令owner复用,没有新增共享闭集、并行权威源、授权或机器义务。唯一目标是机器强制条件,错误中的修复文本是操作者指导;必须能通过现有命令恢复。旧base未支持开发期changed-from advisory,已保留该入口限制,并用当前开发工具分析此精确树;没有受支持的新词汇候选,不据此宣称语义等价。完整head维护性/语义canary通过。
Scheduler/prompt/ACK responses were controlled external inputs; actual script subprocess and isolated TOML/SQLite effects were observed. No live Codex App wake-up or store was modified. 未读取、查询、轮询或等待GitHub CI。当前main仅在此脚本的默认registry路由有相关差异;目标解析区域不受它改变。
我的整体评价
REQUEST_CHANGES。保留安装前的明确拒绝和原绑定写入路径,修复当前错误指引后再批准;不借此扩大宿主权限或改写多宿主选择规则。 实际局部修复有价值,但操作者的恢复/执行路径尚未成立;通用canary绿灯和作者验证不能抹去当前反例。
未来相关重构检查:具名本地解析helper已足够;修复错误命令即可,暂不强迫独立的宿主选择策略或TS迁移。 修复后重新核验完整exact head和上述反例,再决定批准。
English verdict: REQUEST_CHANGES — 6124@ed78a4a6157041666086fc94fe777030608bc0e1. The real script now rejects missing, blank and ambiguous targets before any manifest/SQLite/prompt/ACK effect, and unique, neighboring-Agent and no-apply cases preserve their contract.48 tests and4 direct/2 selected canaries pass. However, the new refusal tells users to run loopx registry bind-agent-thread, which this CLI rejects; the existing command is root loopx bind-agent-thread. Fix and validate the actual recovery instruction.
The refusal added for an unresolved thread binding told the operator to run `loopx registry bind-agent-thread`, but bind-agent-thread is registered at the root of the CLI, so that exact command exits 2 with `unrecognized arguments: bind-agent-thread`. The fail-closed path therefore ended in a repair the operator could not execute. Print the registered root command with its required goal, agent, host-surface and thread arguments, and pass --execute: without that flag the command only previews the binding it is asked to write. The ambiguous case advertises unbind-agent-thread in the same shape, and the message now carries the goal id the install was resolving against. The recovery instruction is covered by feeding the advertised command to the registered parser, and the refusal-to-binding recovery re-runs the same install arguments once the binding is present. Signed-off-by: GZY-SUPER-HACKER <162807803+GZY-SUPER-HACKER@users.noreply.github.com>
|
Thanks — reproduced, and the fix is at [P2] Use the registered root binding command in this recovery messageConfirmed as printed. Running the advertised command with the real argument set:
One thing worth adding to the fix. So dropping Verification
I did not rebase onto current main: as your review notes, main's change to this script is the default |
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; xhigh
Exact head: f8f1a7e583ca99dc80eac0b3268b4dfe80d1510f. Immutable pre-change base: e57b49e7b6b485c7f3c364e22b3abe078e693b9b. Whole2-file diff (+248/-11), unchanged installation callers/root binding owner and current main delta reviewed.
动机
给 Codex App 安装定时任务的操作者,需要把任务送到已选 Agent 的明确聊天。
以前没有明确目标聊天也会显示安装成功,操作者只能等待后再排查;现在先拒绝安装并给出可执行的绑定命令,完成绑定后用原安装请求重试即可继续。
真实脚本子进程对缺失、空白、歧义和新 Agent 未绑定场景均在写文件及获取提示词前拒绝;按错误信息执行原生绑定后,原请求安装正确聊天,再次执行数据库仍只有一条记录。
本批修复安装目标不明确时的拒绝与恢复路径;不改多宿主选择策略、已安装任务迁移,也不证明真实自动唤醒或提示词送达。
改动思路
安装前的唯一目标约束和现有绑定写入器解决同一个实际故障,局部 helper 与可运行的修复命令已经足够,不需要新增状态或调度框架。 当前边界是新安装时拒绝不明确目标,并通过原生绑定恢复后完成原安装;多宿主选择与已安装任务迁移保留原边界。
独立规格依据 docs/reference/protocols/codex-app-host-command-registry-v0.md,固定改动前 revision e57b49e7b6b485c7f3c364e22b3abe078e693b9b:accepted contract。Project Root And Agent Identity 已 implemented:只从显式所选 Agent 的原绑定派生唯一非空目标。Handoff Packet 已 implemented:错误消息给出的现有根命令持久化并读回该身份,原安装随后复用它。作者声明不是独立验收依据。
这是宿主专用安装器的局部 invariant,保留现有 scheduler、binding writer 和 TOML/SQLite owner;不新增并行通用控制面或猜测多宿主身份。确实缺失的 thread ID 需要操作者提供;Goal/Agent/host 已在命令里,没有再要求重复填写已知信息或添加冗余确认。
具体改动
_resolve_bound_thread_id:读取完整原绑定,只选显式 Agent 的非空目标,零个/多个匹配先退出;提示根级 bind/unbind-agent-thread,所需 scope 与--execute齐全。_ensure_automation:仅在缺少安装时进入目标约束,拒绝位于 prompt获取、manifest写入、SQLite写入和ACK之前;完全已安装shortcut保留。test_the_refusal_advertises_a_command_this_cli_can_run:覆盖打印命令与现有parser,另有绑定后原安装重试用例。评审独立补了真实原生写入/全局同步/读回,未仅靠parser判断恢复。
完整两个文件及其真实上下游已读。与最新 main 5c62c5c03f13cc84176338100f2bded12801530e 的相关差异仅为默认 registry 选择路由;本修复没有把主干历史算进 diff,解析目标区域未被该变化改写。
对主干的风险
当前51项相关 Python tests、基线同 suites46项通过。风险canary4 direct +2 selected全通过,ruff/diff clean。正确工作树 root 的开发advisory无受支持新词汇,随后full-tree semantic smoke通过;这不自动证明语义相等,实际caller正反对照另行验证。没有查询、轮询或等待GitHub CI。
同一隔离 harness 在 base/head 各跑8个真实脚本子进程场景:missing、blank、ambiguous、unique、neighbor、no_apply、recovery 和已启用Goal中新增但未绑定的Agent。基线对无目标仍exit0、persist空目标;当前先exit1,仅schedulerhint读取,没有manifest/database/prompt/ACK。唯一合法目标和相邻Agent维持正确安装;noapply安静无效果。新增Agent不会借用原Agent的目标。
恢复不是模拟成功payload:捕获当前错误中的bind命令,用实际 source CLI,仅将registry/runtime隔离并填写真正缺失的thread;返回ok、registration_readback verified、global_sync ok。用原安装argv重试exit0,manifest目标thread-one,再执行数据库仍一条记录。此前 review5479603887 的死命令问题已逐项解决:旧 loopx registry bind-agent-thread 不可识别,当前根命令和execute经过实际持久化验证。
首次独立probe误用 python -m loopx(仓库无这个入口),首次开发工具root也误选;保留调用错误记录,由当前 source executable/显式root覆盖原风险,不算作 PR缺陷或暗中省略。当前required验证无失败/skip。真实边界是脚本、隔离TOML/SQLite和原生source/global绑定;scheduler/prompt/ACK是受控外部响应,没有更改任何活跃Goal或真实Apptimer,也未证明实际自动唤醒。
我的整体评价
APPROVE。效果/效率方向为正:避免安装假成功导致后续空等,拒绝后有实际可走通的原身份恢复,重试不重复建任务。这个必要的身份修正需要一次明确绑定,但它补充的是原本缺失的信息;合法已有身份没有增加操作步骤。重复运行保留同一任务记录,未改变原Turn/调度权限。
未来相关重构检查:具名本地helper和原绑定owner已经够用,没有值得在本批引入的额外框架;多宿主disambiguation按既有accepted边界保持拒绝,不制造新policy。当前安装修复验收成立;真实宿主送达和整个 #3927、旧review的GitHub closeout、maintainer merge/安装分别保留其验收和权限。
English verdict: APPROVE — f8f1a7e583ca99dc80eac0b3268b4dfe80d1510f. Missing, blank, ambiguous and newly registered unbound Agent targets fail before prompt/manifest/SQLite/ACK; valid neighboring-Agent and no-apply paths retain their useful contract. The exact printed root command now durably binds and reads back isolated source/global identity; the original installer retry reaches the correct target and replay leaves one row.51 head/46 base tests,8 matched subprocess cases per revision,4 direct/2 selected canaries,ruff and corrected-root advisory followed by full semantic validation pass. The old dead recovery command is resolved. Live App firing/body delivery, multi-host disambiguation and installed-state migration remain outside this bounded repair; maintainer merge authority is separate.
Goal And Delivered Outcome
Outcome basis / optional anchor: Self-contained reproduced defect; no public issue, roadmap card or RFC id is claimed for it.
Goal/source and gap:
_ensure_automationinscripts/codex_app_apply_rrule.pyresolved the automation's target thread withthread_id = ""plusif len(matches) == 1, so zero or more than one matchingthread_agent_bindingsentry both fell through to the empty default. The install then reported success and persistedtarget_thread_id = "". Host-side readback resolves a goal's threads through the goal'sthread_agent_bindings, so an automation installed this way cannot be observed, andloopx/control_plane/heartbeat/automation_upgrade.pyalready fails closed when the TOML and the SQLite row disagree on that field.Observable before → after, with the validation row that proves it: before, a first install against a goal with no usable binding printed
created missing automation: <id>andapplied <rrule> and ACKedwith exit code0, and the manifest heldtarget_thread_id = ""; after, the same install exits non-zero withexpected exactly one bound host thread for agent ..., found 0and writes neither a manifest nor a SQLite row (validation rows 1–2).Issue/task and intended base: Related to [Bug]: Codex App automation integration cannot detect missing scheduled prompt delivery #3927 — this does not close it. Intended base
main.Author Declaration
Implemented against
loopx/control_plane/agents/host_thread_activity.py(readback resolves a goal's threads throughthread_agent_bindings) andloopx/control_plane/heartbeat/automation_upgrade.py(stores that disagree ontarget_thread_idareblocked).scripts/codex_app_apply_rrule.py::_resolve_bound_thread_idtests/test_codex_app_apply_rrule.py::test_apply_refuses_automation_without_a_bound_threadtests/test_codex_app_apply_rrule.py::test_apply_refuses_automation_with_an_ambiguous_bound_thread_ensure_automationhost_surfacevalues such ascodex-appwhile the scheduler constant iscodex_app. Choosing that mapping is a maintainer contract decision, not part of this repair._ensure_automation,_read_automation_prompt,_ensure_sqlite_row,main, the readback paths above, and the existing tests. I verified the refusal is reachable through the script's normalmain()entry with the subprocess boundary patched at the same seam the existing tests already patch. I deliberately left out the live Codex App host and did not read or write any real host store. The head is based one57b49e7brather than currentmainbecause the pushing credential lacks theworkflowscope; the change is one commit, its two files arescripts/codex_app_apply_rrule.pyandtests/test_codex_app_apply_rrule.py, and the edited region is byte-identical across that range.Scope And Continuation
out_of_scoperow). Unrelated:_ensure_sqlite_rowwrites a fixed column set that has notarget_thread_idcolumn, so the empty value lived only in the TOML — out of scope here.#4140'smanifest_thread_mismatchis a different condition — it compares a caller-asserted thread against the manifest and cannot be reached with an empty expectation — so this repair does not overlap that pull request.Validation
ed78a4a6157041666086fc94fe777030608bc0e1unitpassedtests/test_codex_app_apply_rrule.py— 12 passed, including the two refusal casesregression_paritypassedapplied ... and ACKedintegrationpassedtests/control_plane/test_scheduler_fallback_hint.py,tests/control_plane/test_mutation_actor_identity.pyandtests/test_thread_agent_binding.pyalongside the above — 48 passed on this headstaticpassedruff checkclean on both files;git diff --checkcleanreal_entrypointnot_runmain()in-process; tolerated limitation: no live Codex App automation was installed or triggeredreal_backendnot_applicablee57b49e7b..head touches only these two files, but a maintainer may prefer the branch re-based on currentmainbefore merge. Repository-wideruff format --checkreports both files as reformattable on the pristine base as well (CRLF checkout on this host), so that is pre-existing and not addressed here.Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).