Repository navigation
Conversation
Signed-off-by: Jim-jimu <3230103159@zju.edu.cn>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| const binding = await services.store.read(key) | ||
| if (!isCurrent()) return | ||
| if (!binding || binding.terminal || binding.autoResume === false) return |
There was a problem hiding this comment.
Persist input revocation when a resume is still in flight
If interactive/RPC input arrives while /loopx resume is saving a previously paused binding, this read can still observe autoResume: false and return without advancing the generation. The pending resume then successfully persists autoResume: true; its fence check only returns null after that commit. The current instance remains locally blocked, but restarting Pi loses that fence and resumes automatic dispatch despite the newer owner input. This reproduces with a delayed resume write followed by ownerInput, then a new runtime over the same store. Ensure the newer input invalidates the pending durable re-arm even when the binding is already paused.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for catching this. Fixed in e56bb04.
Owner input now revokes dispatch immediately and waits for all already-started resume/activation writes before reading and persisting the pause. A re-arm superseded while its read is pending returns before starting a write. A later explicit resume can still re-arm the session.
The regression tests cover both resume and activation interrupted during a pending read or write, using disposable file-backed storage and a new runtime after restart. They verify that no quota probe or automatic message occurs until a later explicit resume. Additional cases cover overlapping re-arms and a failed resume.
Of the seven new regression/control cases, five fail on the prior runtime and all seven pass with the fix. I also reran the three Pi Node suites at the current head, 9af5d73: all 90 tests pass.
Signed-off-by: Jim-jimu <3230103159@zju.edu.cn>
Signed-off-by: Jim-jimu <3230103159@zju.edu.cn>
Signed-off-by: Jim-jimu <3230103159@zju.edu.cn>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · gpt-6.1-sol · OpenAI · declaration_source=runtime_reported · reasoning_effort=max.
详细中文评审
动机
受影响的是在 Pi 会话中启用 LoopX Goal 自动继续、随后提交自己新指令的用户。 新头已让已观察的 interactive/rpc 输入等待恢复写入后持久暂停;但没有观察到外部 input、只收到非自发 user 启动时,保留的后备暂停仍会在恢复交叠中消失。 实际 adapter 和文件存储验证中,interactive/rpc 均在重启后保持暂停;extension 来源的不同文本经 before_agent_start 后,磁盘仍为 true,原实例和重启实例各会取得配额并派发。 边界只包括既有 Pi 输入暂停和显式恢复,不把本 PR 当作完整自动压缩来源识别、安装后 Pi 或长期效果验收。
用户的新指令应先停止旧任务自动派发,再由 Pi 正常处理这条消息;后台恢复操作不能把已经发生的新输入覆盖掉。REQUEST_CHANGES,精确头 9af5d7366516d486462f613096721aaff6ccc1a4。 旧头的显式输入持久化问题已被本次修复,不把旧结论原样搬过来;剩余发现限于本 PR 声明保留的启动事件后备。
改动思路
原 adapter 观察输入、native runtime 决定继续、file store 保持唯一持久 owner;pendingRearms 是既有写入的本地依赖,不增加权限。 保留已验证的显式输入修复,只在同一个 pause helper 补齐非自发 user-start 后备的交叠撤销,不扩大到完整来源归因。 writeRearm 记录每个会话已经开始的 activation/resume 写入,显式 input 先同步挡派发,等待这些写入后再持久化 false。还在 re-arm 读状态之后加 fence 检查,让已被新输入取代的操作不再启动写入;这是有意义的同 owner 修复。
后备路径需要先区分自己的注入文本与其它 user start,不能把 extension 来源当作自发消息。但当前字符串后备先等待恢复 promise,仍引用等待前的旧 fence;resume 完成后会换掉这个 fence,于是后备在检查文本之前就返回。需要把“非自发启动也要暂停”的既有义务接进同一个撤销顺序,而非增加另一种持久权限或放弃原后备。
具体改动
完整六文件 +734/-66:既有 adapter +19/-3,runtime +121/-54,README +22/-9,新增182行 adapter tests、runtime tests增加383行、Python包装增加7行。新增的是 keyed pendingRearms 本地 promise 链,沿用原 file store、generation/CAS、authority、定时器和显式恢复;无新模块、依赖、CLI 或持久 schema。完整新头和上一个审查头的三个 Pi 文件增量都已重新查看,整份六文件范围重新判定。
规格来自先读的 Pi README@cdbdc14,并对照当前 README 的 Input boundary / Origin limits。external-input-pause、durable-newer-input-revocation、existing-authority-and-recovery 已在普通或显式输入路径实现;retained-nonown-start-fallback not_met。作者也明确保留“starts lacking observed external input”的 text fallback。这里不要求修复完整来源归因,也不要求给已被更早处理器消费且没有任何启动信号的消息造来源。#6107 维护者说明的完整自动压缩 producer 序列仍是独立范围,未据本 PR 宣称关闭。
关键代码讲解
before_agent_start@633:custom 调用显式 ownerInput,user 调用 userPrompt。不同文本的 foreign extension user start 在这里仍承诺走保守暂停;不需要知道该 extension 的完整身份就能确认文本不是本次注入。writeRearm@820:把已开始的原生 store.write 与先前未完成链一起等待,allSettled 让失败的恢复不阻止新输入尝试保存暂停,结束后清理映射。它跟踪 IO,不产生执行许可。pauseForInput@1048:ownerInput 先 revoke,所以恢复写完后它仍持有当前 fence 并写 false;字符串后备在 @1065 等待时没有 revoke,resume 结束把 fence 换成新的 open 对象,@1066 的 isCurrent 便返回,后面的 lastInjectedPrompt 比较和持久暂停都不会执行。
[P2] 让保留的非自发 user-start 后备也能撤销进行中的恢复。 独立程序实际载入 TS adapter 的注册 command/input/start handlers 和原生文件 store/runtime,只把 SDK/UI、CLI admission 回复设为合成值,并延迟正常的 resume rename。先正常注入并识别自己的任务,再以 abort 持久暂停;启动 resume,在 true 落盘前送入 extension-source input 和一条与 lastInjectedPrompt 不同的 user before_agent_start;释放正常提交并等所有 handler 完成。当前文件仍为 true,原 adapter 再取得1次配额并派发1次;替换运行时也再取得1次配额并派发1次,只有 resumed 通知,没有暂停。
同一实际 adapter 程序在不可变 main 基线 92a170f8c7419ac0e32f63a2103aa4bfe464d054 也暴露后备缺口,因此不把它称为新引入回归。相反,interactive 和 rpc 两个当前 adapter 控制都最终保存 false,原实例和重启的配额/派发均为0;同一物理 rename 程序在旧 PR头9e明确失败、当前9af通过,认可已有 finding 的修复。
在原 pauseForInput 内让非自发 user-start 后备也能撤销正在进行的恢复,区分原自发文本且保留后来显式恢复优先;最终持久 false、原实例及重启零配额/派发,再验证新的合法 resume。 修复不能把所有 extension 输入一律当外部,也不能破坏自发注入和后来明确 resume 的优先级。需要在相同 adapter/native 文件入口重跑这组后备、已修复显式输入、自发消息及合法恢复控制。
对主干的风险
新精确头独立90项 Node、97项 Python、root control-plane typecheck、改动 Python Ruff、diff、双语 RFC 检查与 semantic advisory 通过。原生文件的覆盖/同目录其它会话/未来未激活会话/同文逃逸/合法恢复控制在当前头和新 main 基线都重跑;新增未授权会话没有文件或配额。正常检查和作者新增七项用例通过,不覆盖上述实际 adapter 后备交叠反例。
标准门5项直接和15项选择检查全部通过,但必需后备暂停不变量仍失败,当前精确范围质量为 FAIL,aggregate 保留 quality_invalid_receipt。旧头未发布的审查、旧失败回执和新头修复证据都保留;没有复用旧 head 的批准或阻断,也没有改作者源码、删断言、豁免门或查询 hosted CI。
语义与 CI 对齐
复用已有 Pi source、binding、generation/CAS,两个本地 map 分别承接即时拒绝和已开始的写入依赖,不是新的 generic Goal 状态或 actor 权限。保存失败仍明确只保证当前实例停止。本人实际执行的 SDK 依赖被替身隔离;读过 Pi1.4源码,不等于执行安装后 Pi/真实 SDK session、TUI/RPC 或模型。这些、原生 Windows、完整来源问题和全套 CI 未验;作者146/145预算、其它历史失败与CI描述不作为本人的独立归因。关键失败由当前本地 native adapter/store 反例直接证明。
我的整体评价
REQUEST_CHANGES,限 9af5d7366516d486462f613096721aaff6ccc1a4。 当前 problem_context、long_horizon 和 user_experience 保留 not_yet_proven,因为已观察输入的持久修复得到认可,但保留的非自发启动仍可能让旧任务在新工作开始后再次派发。原 adapter 观察输入、native runtime 决定继续、file store 保持唯一持久 owner;pendingRearms 是既有写入的本地依赖,不增加权限。 保留已验证的显式输入修复,只在同一个 pause helper 补齐非自发 user-start 后备的交叠撤销,不扩大到完整来源归因。 增量机制有界,最小修复仍在同一 helper;无需更大重写或额外来源系统。没有渲染页面变化,visual_review 为 not_applicable。保留原讨论,不 dismiss/resolve、不合并;后续新头要执行当前完整计划,不能只看最后一个测试。
英文简短结论
English verdict: REQUEST_CHANGES — 9af5d73. The prior observed interactive/RPC re-arm race is repaired and independently passes real-file and actual-adapter restart controls. The retained non-own user-start fallback still awaits pendingRearms under the old resume fence; resume replaces it, so isCurrent returns before prompt classification and durable pause. Actual registered TS adapter/native-store execution leaves true and dispatches live and after restart. The immutable main base shares this fallback gap; no new-regression claim.90 Node/97 Python and selected checks pass; required fallback validation and exact quality remain failed. Repair the same helper while preserving own injection and later explicit resume.
| try { | ||
| // A paused snapshot is not final while an older resume/activation can | ||
| // still save autoResume=true. Revoke dispatch now, then read after those | ||
| // writes finish so this pause also survives a new runtime instance. |
There was a problem hiding this comment.
[P2] 保留的非自发 user-start 后备也应撤销进行中的恢复。字符串后备进入时没有 revoke,仍捕获 resume 的旧 fence;等待 pendingRearms 后,resume 已把它换成 open 对象,此处直接返回,尚未比较 lastInjectedPrompt 或保存暂停。实际注册 TS adapter + 原生文件存储复现:先暂停、延迟正常 resume rename,送 extension-source input 与不同文本的 user before_agent_start,释放提交后文件仍 true,原实例和新运行时各取得1次配额并派发1次。interactive/rpc 控制已修复并保持 false;main 基线也有后备缺口,不称新回归。请在同一 pause helper 保留后备的撤销/持久顺序,同时保留自己的注入文本及后来明确 resume 的优先权,补实际 adapter/重启回归。
|
按实现基线 Pi README和#6107 的维护者范围说明完成当前 已独立确认:interactive/RPC 的 pending re-arm 持久暂停已修复,旧 finding 的这一部分不再视为未解决。剩余 本结论不要求本 PR 完成未知生产者归因或关闭完整 #6107;90 Node/97 Python 及选择检查通过,必需后备不变量和精确质量仍失败。未合并或处理其它人的 review 状态。 |
Goal And Delivered Outcome
Pi users can steer an active LoopX task or queue a follow-up without surrendering control to the next automatic task. Currently, queued interactive/RPC input bypasses
before_agent_start, and idle input identical to the last injected task is mistaken for LoopX's own message.This change observes Pi's
input.sourceand pauses subsequent LoopX dispatch for interactive/RPC input, including queued, identical-text and image-only input. Pi continues processing the owner's messages, including compaction and recovery. Explicit/loopx resumestill requires fresh quota admission.Related to #6107. Target:
main; implementation baseline:cdbdc14ea3963e6f6aec4d574a1b88baff965132. This does not claim to resolve the original report's automatic-context provenance question.Author Declaration
Implemented against
Specification: Pi Goal mode at the implementation baseline, especially Boundary / Pause and resume boundaries; Pi v1.4.0 input and run-start contracts.
input; runtimeownerInputcontinueSelf-check: reviewed the actual Pi input/queue/compaction paths, replayed the new adapter tests against unchanged baseline source, and exercised Pi's installed loader and runtime. No live provider or active Goal was used.
Scope And Continuation
Complete within the external-input pause scope.
Earlier input handlers can consume events, and
source: extensiondoes not identify an individual producer. Those limits remain documented under #6107; no new follow-up issue is needed. Already-submitted messages and running tools are not withdrawn.Validation
9af5d7366516d486462f613096721aaff6ccc1a4.0336f5d2076a4fd52321b7a2c81130a152cba399exactly matches the locally tested merged tree. Connector publication changed commit metadata only.Review follow-up: persist input pauses across pending re-arms
Fixed the pending-resume finding in
e56bb04073534e61caa4c99878433b4a9abed781. Input now revokes dispatch immediately and waits for the session's already-started resume/activation writes before reading and persisting the pause. A superseded operation that is still reading returns before starting its write. This covers multiple overlapping re-arms while preserving a later explicit resume and existing saved pause reasons.Final published head includes upstream
92a170f8c7419ac0e32f63a2103aa4bfe464d054. Validation on its exact source tree:Validation after the first main synchronization
Merged upstream
15803cd7dcc2fc517049809bbe58998f04f5acacwithout conflicts, retaining the original contribution history. The PR still changes the same six Pi-related files.Rerun against the merged tree: 83 Node tests passed, 97 focused Python tests passed, 4 Pi 1.4.0 SDK queue/compaction cases passed, and the Dashboard model/header smoke passed 66 bilingual assertions. The bilingual RFC diff check and root control-plane typecheck also passed. The top-level module budget still fails locally at 146 / 145, as on the implementation baseline.
New CI for the published revision: Python Tests and Release Artifacts. DCO and dependency review passed. The new kernel-static-checks job again failed the unchanged 146 / 145 top-level module budget, so downstream test shards were skipped. That Release Artifacts run subsequently passed the model/header assertions but failed in the separate browser chat-recovery scenario, timing out while waiting for the global-priorities button. Its relevant files are unchanged by this PR; independent baseline reproduction of that browser timeout is not claimed.
The table below records the original implementation's validation at
bd9c0dcaae096aff6f02fe98834d01429b986b48; checks rerun after synchronization are listed above.pytest -q tests/test_pi_goal_mode.py tests/test_host_loop_runtime_parity.py tests/test_slash_command_install.py tests/test_agent_onboarding_pi_host.py: 97 passed.tests/control_plane/test_pi_adapter_start.py: 4 failed / 8 passed on both base and candidate; bound startup fixtures raiseCanonicalAuthorityRequired.tests/test_chat_conversation_bindings.pyon both arms. Strict adapter checking against Pi 1.4.0 has 6 inherited diagnostics, down from 7 after narrowing the custom/user event union; no full adapter typecheck pass is claimed.benchmark/tests/test_checkpoint_handoff.pyfailed creating a socket withPermissionError; that failure reproduces on the clean baseline. A broader parallel attempt was stopped without claiming full-suite success.Coverage is strongest at the changed adapter/runtime boundary and the actual Pi queue/recovery lifecycle. Baseline failures and the environment-limited full suite remain visible for review and CI; they are not waived by focused passes.
Initial CI failure investigation
The checks for
bd9c0dcaae096aff6f02fe98834d01429b986b48failed on two baseline defects:test_top_level_module_count_stays_at_the_pinned_budgetfinds 146 top-level Python modules against a budget of 145. The same test file produces 1 failed / 5 passed on both the unchanged implementation baseline and this PR. This PR adds no top-level Python module. The reported filename is selected by reverse alphabetical order and does not identify the introducing change.smoke:chat-model-menufails its source-badge / Goal-title assertion. Reproduced withnpm run -s smoke:chat-model-menuon the unchanged implementation baseline. The relevant Dashboard source, smoke and dependency files are unchanged by this PR. Upstream fix(workspace): stop the read-only source badge from repeating the Goal title #7380 subsequently fixed this case; the failed run used the older baseline.checks,pytest,stage2c-correctness-e2eandmerge-gatefail because prerequisites failed or were skipped. They are aggregate failures, not additional failing test cases. The Python test shards, including the Pi wrappers, did not execute in this CI run.Frontend / Visual Evidence
Type of Change / LoopX Area
Technical direction: governed continuation and preservation of owner control. Shared-authority RFC fixture impact: N/A; no authority-store migration or provider routing change.
Boundary Checklist