fix(runtime): stop treating route locks as a second authority - #5474
Conversation
Goal configuration locks every candidate runtime registry before writing, and the native effect runtime locks the same registry file. On a fresh machine the untouched legacy root then holds only `registry.global.json.ts-effect.lock`, which `_is_route_observation` did not recognize, so the first configured Goal failed with "Both default LoopX runtime roots contain state". - Recognize the native effect lock as a route observation, like the Python lock it sits beside. - Isolate the default runtime routes per test in `tests/conftest.py`, and fail the session when tests create a real default route that was absent. - Follow the `collect_doctor` signature added in #5457 in two test mocks. - Keep `cancel-in-progress` on pull requests only, so a `main` push run is not cancelled by the next merge. Verified: the added regression test fails on the unpatched `paths.py` and passes with it; the 54 files that failed on CI pass 1149/1152 here, and the 3 remaining failures reproduce unchanged at this branch's base. Signed-off-by: song <liusongstep@gmail.com>
|
This pull request has merge conflicts with Choose the remote for the base repository, not an out-of-date fork. git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEADFor a same-repository clone whose Keep the DCO |
Keep main's explicit collect_doctor stub parameters in tests/test_cli_argument_diagnostics.py; the entrypoint stub remains unrepaired on main. Signed-off-by: song <liusongstep@gmail.com>
Verification on head
|
|
Independent validation on
One adjacent pre-existing recovery gap should stay visible: if a project registry has no The lock-classification repair is independently demonstrated; the remaining review should assess the complete test-isolation and workflow changes at this exact head. No installed runtime or active Goal was modified by these probes. |
Signed-off-by: song <liusongstep@gmail.com>
Signed-off-by: song <liusongstep@gmail.com>
|
修复更新,精确 head:
远端新 CI 已触发,完整 CI、原生 Windows 和 PostgreSQL 尚不在上述本地通过结论内。维护者此前指出的显式双根恢复问题不属于本次 lock 分类回归,仍保留原评论的边界说明。PR 正文已替换为当前实现与验证结果,未自合并。 English: Updated against main and repaired the two stale test fixtures. Focused validation passes (425 + 56 tests, 7 CI helpers; 3 platform skips). Remote CI and maintainer review remain pending; this is a repair readback, not an approval. 补充旧 Windows CI 排查:两个旧失败的修复已经在本次合入的 main 中,分别是 |
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | GPT-5 | OpenAI
动机
结论:APPROVE,评的是 ab4d5d1,没有发现当前切片的阻塞问题。普通 TS effect 锁不是机器状态;原实现把它计入默认路由,首次 configure-goal 在锁前后重读时可能从 fresh 变成双路冲突,后续独立调用也继续失败。真实配置和 registry 仍须保护,不能为了消除误阻塞就忽略整个目录或选择任意一份权威。
改动思路
沿用现有 filesystem/bootstrap adapter:同一个 paths 规则识别既有 producer 的精确锁文件名;route 仍由既有 RuntimeRouteStatus 表达,真实 source registry、写锁及 TS effect authority 不变。测试把各用例的两个默认根和进程 HOME 隔离,并在 session 结束检查新建真实默认根的泄漏。CI 只保留 PR 的抢占,push/手工运行不再取消正在执行的检查;它不保证所有排队提交都会执行。没有第二个 Python domain owner、新配置开关、自动迁移或付费调用。
具体改动
完整阅读八个文件、+97/-5,覆盖一个五行生产修复、五处测试文件和两个 workflow,未略过测试夹具或 doctor 断言。
关键代码讲解
- 「_is_route_observation」(loopx/paths.py:138):先拒绝 redirected path,再仅对 registry.global.json.ts-effect.lock 的普通文件返回 true。文件名相同的目录、符号链接仍是状态;未知文件亦不豁免。这里按 producer 布局分类,不按文本猜 Goal 类型,不取消锁的互斥或安全回收规则。
- 「default_runtime_route」(loopx/paths.py:179):继续无写入地检查两个根和 registry;只有普通诊断锁不再构成第二份状态。configure-goal 实际调用重新进入同一 selector,持久化和全局镜像随后由既有 source-authority flow 完成,独立 readback 验证两者一致。
- 「_isolated_default_runtime_routes」(tests/conftest.py:40):每个测试独立 HOME/USERPROFILE,清除继承的 CODEX_HOME,并更新四个现有模块中的八处默认路径引用。session guard 仅监测开始时不存在的真实默认根,不删用户文件、也不宣称是任意导入/已有目录的安全沙箱。
tests/test_local_state_migration.py 增加 current/legacy 的 lock-only 回归;tests/test_cli_entrypoint.py 跟随 doctor 的 runtime_root_override,但过滤 kwargs 的形式没有断言 registry_path。非阻塞整合建议:与 #5463 相同测试合入时保留它的五键精确断言,不把新增路由参数从回归中筛掉。两个 workflow 只改 cancel-in-progress 表达式;触发器、权限、分组、jobs 和产物原样保留。
新增测试夹具也逐项核验:tests/test_chat_codex_goal.py 先用真正 ChatSessionStore claim 精确 queued Turn,再走原 controller。worker claim 只表示这个 worker 可执行,不是 local-owner 权限;外部 origin 仍进入 failed/local-owner 拒绝,mock host.calls 保持空。生产 queue loop 本来就在 _run_turn 前做同样 claim,权限检查代码未改。tests/test_skill_delivery_parity.py 用 onboarding 所需集合对齐既有 packaged catalog,加上根 skill,取代过时的“七个”数量假设;同文件的实际目录、scope、安装/readback/缺失/stale 测试仍保留。不可变新 base 上前者卡在 starting、后者 8≠7;当前 head 两文件完整集通过,不继承作者的测试数字。
正路:两个根只有普通陈旧孤立锁,实际源 CLI configure-goal 成功写入 max_children=4,source/global 独立读回一致,重复调用 changed=false、sync 回执 not_required/verified=false,再独立检查实际 source/global;已有单一路由与 registry 声明的 custom runtime 也成功。反路:真实双 registry、未知状态、同名目录/符号链接仍拒绝,source 不变;非法 max_children 仍拒写。原来的“锁造成冲突”消失后,重叠非法输入会暴露参数自己的诊断,这是有意的拒绝顺序变化,不伪称逐字等价。
独立规范:spec_ref = docs/product/migrations/local-state-path-migration.md;spec_revision = 447878d。未重写该规范,criterion_id 使用原文片段:
| criterion_id | 判定 |
|---|---|
| An empty directory alone does not select a route | 保留;空根仍 fresh,读路由不创建目录。 |
| a lone global-registry lock | 落实;既有 Python/native 普通锁都不声明机器状态,锁的互斥仍由原 owner 执行。 |
| unknown contents or redirected entries still require an explicit route | 保留;真实 CLI 的未知状态、目录、symlink 控制均拒写。 |
| If both default roots contain state | 保留;两份真实 registry 的冲突和完整诊断 base/head 一致。 |
| common_runtime_root | 保留;声明 custom runtime 的实际写入、镜像和重试验证通过。 |
对主干的风险
当前 head 重新执行完整风险相关 native 集:608 项通过、3 项既有 Win32 junction 条件跳过,覆盖 route/file-lock、source-authority、doctor、Chat Goal 和 skill delivery;CI helper 7 项通过,19 个配置范围源文件的 strict mypy、改动 Python 的 Ruff、diff check、语义 advisory 和两份 workflow 的独立 YAML 对比通过。没有查询、轮询或等待远端 CI。该集合是本切片的风险对应验证,不声称全树 canary、真实 Windows、PostgreSQL server 或 GitHub runner 实验均已通过;PR 没改 authority-store 代码或 UI/Lark 交互,无需新造 PG/前端验收。
额外同一脚本/合成 fixture 在不可变 base 与 head 各运行 11 种情形、每种两个完整源 CLI 调用,真正使用 file backend,不加载 pytest 夹具。旧 fresh 失败、head 成功;五组真实冲突/未知/redirect/显式冲突的完整退出、诊断、source 和镜像逐字段一致。成功 custom-route 的时间戳/探针 PID/摘要不是字节等价项,而是分别验证 actual source、目标摘要匹配及 verified 回执。fixture fingerprint 为 c4316eb9abf361b4cdea29de13bde6eb0ed6aae95a940105950db36a04777f6e;仅归一化自有临时路径和 registry 时间字段,保留原始错误、payload、摘要和重试结果。
隔离夹具另做真实 pytest plugin 探针:连续测试的 HOME 和八个引用均隔离;故意在合成“真实根”注入泄漏,三个测试自身虽通过,session guard 仍使进程 exit=1。探针首次用刚生成的空锁触发了既有十秒安全回收规则,已改用陈旧孤立锁;没有把安全等待改掉。未声明 common_runtime_root 的 source 遇到真实双默认权威时,即使给显式 override 仍可能被现有 discovery 拒绝;base/head 一致,这是独立既有恢复限制,本 PR 不负责迁移或解决实际双权威。
我的整体评价
这是完整、有用、可回滚的 route 修复和测试防污染措施,能让正常配置与后续调用继续推进,又保留反向冲突保护。未来方向检查已考虑 file_lock→paths 循环及生产分类边界:不为一条已归属 filesystem 规则另建 TS 策略或 speculative lock registry;复用既有 owner 足够。#5463 的精确 kwargs 建议属于相关测试整合,不是批准的先决条件。head 更新后重新生成当前 capability plan、读完全部八文件并重跑新 base/head;原 route/lock/source owner 与文档的 blob 身份未变,但没有据此继承旧批准。批准限该不可变 head,不关闭整个本地状态迁移或长期 provider 验收,也不代表已安装生效。批准发布后单独检查过期阻塞评审;无权限或未验证解决的评审必须保留。没有合并。
English verdict: APPROVE - ab4d5d1: no blocking finding in the bounded route/CI-isolation fix; 608 native Python and 7 CI-helper tests passed, three existing Win32 cases skipped; immutable base/head real-CLI negative, readback and retry checks passed. No merge.
Signed-off-by: huangruiteng <huangrt01@163.com>
|
This pull request has merge conflicts with Choose the remote for the base repository, not an out-of-date fork. git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEADFor a same-repository clone whose Keep the DCO |
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | GPT-6 | OpenAI
动机
新安装用户首次配置 Goal 时,需要保存设置并继续工作;维护者需要在测试中验证同一路径而不污染自己的状态目录。
一个原本空白的用户目录在配置时产生两份程序锁文件,旧实现随即误报两份状态并拒绝保存;修复后首次设置成功,重复设置不会再次写入,真实双份状态仍拒绝隐式选择。
实际配置入口、源配置与全局镜像独立读回、重复调用及目录边界反例已验证;共享测试夹具为每个用例建立独立目录。
本 PR 不自动迁移或删除用户状态,不改变账号权限、遥测默认设置、任务调度或前端设置流程,也不宣称整版发布资格已完成。
改动思路
结论为 APPROVE,评审完整 head 23873fe4fddb03dfee35bcdbc1233d307ce42193,不继承旧 head 的批准。先比较不修复、直接忽略目录和当前实现:用户首次保存被自己的锁拒绝,确有修复价值;直接选一个根或忽略整目录会隐藏真实权威。五行改动保留现有 filesystem/bootstrap owner,native TypeScript 写入和 source/global readback 仍是原有路径。
独立规范:spec_ref = docs/product/migrations/local-state-path-migration.md;spec_revision = f6e0409。criterion_id a lone global-registry lock 要求诊断锁不声明第二份机器状态,真实 fresh configure 和文件类型反例验证;criterion_id If both default roots contain state 要求真实双份状态拒绝隐式选择,现有 migration suite 和新真实状态探针验证。规范未为实现改写。
具体改动
八个文件 +94/-5,全部阅读。loopx/paths.py::_is_route_observation 先检查 redirected path,再仅把精确名 registry.global.json.ts-effect.lock 的普通文件列为 observation;同名目录、symlink、未知文件仍是状态。文件名来自现有 producer;因 file_lock 反向导入 paths,保留当前适配层内的精确 spelling,不新增 Python policy owner。
tests/conftest.py 给每项测试独立 HOME/USERPROFILE,清除继承 CODEX_HOME,重绑四个模块的八处默认路径引用。session guard 只检查启动时不存在的真实默认根是否被创建,不删用户数据,也不能证明任意已有目录没有变化。
tests/test_local_state_migration.py 覆盖两类普通锁的双空目录;tests/test_chat_codex_goal.py 先从真实 store claim 精确 queued Turn 再 dispatch,外部 origin 仍拒绝 local-owner continuation,host mock 不被调用;tests/test_skill_delivery_parity.py 对齐既有 canonical packaged catalog,保留实际目录与安装读回测试。集成主干 #5463 的 doctor 五键精确断言,删去本 PR 原先较弱的重复变体。
两个 workflow 仅将 cancel-in-progress 限定为 pull_request;触发器、权限、jobs、分组不变,不宣称保留每一排队提交。project_registry_io_manifest_v1.json 重新生成,唯一变化为 paths.py 的真实代码位置 262→267。
正路:真实 fixture HOME 首次调用 configure_goal_with_global_sync,源配置与全局镜像独立读回 max_children=4,再调用 changed=false。不可变 base 同一入口因自己的锁拒绝且 source 未写。反路:六组真实文件系统比较中,普通锁可 fresh,同名目录、symlink、未知状态仍 conflict;原单路由新增真实内容会 conflict,移除仅本探针自己的内容后恢复,原 registry 字节保留。现有显式路由、迁移和真实双 registry 用例继续通过。
对主干的风险
最强风险是把假冲突修成权威逃逸。此处不采用 substring/prose 猜测;先拒绝 redirect,再检查精确普通文件,目录内其他内容仍参与完整扫描,新增机器状态也不能绕过。对实际配置入口、状态读回和重复保存已验证;无新表单、确认、CLI 参数或前端权限分支,既有 Chat 设置调用同一服务,因此无需前端 companion。没有新 Actor 生命周期、默认 opt-in 能力或遥测策略。
206 项隔离 pytest 通过,3 项 Windows junction 用例在 macOS 条件跳过;另外 59 项升级/安装路由测试通过、2 项平台专属用例跳过;Ruff、scripts/ci 的 3 项 unittest、diff hygiene、8 路径 public-boundary、exact-scope change-quality 和 native premerge 通过。premerge 4 项 selected、5 项 direct,无失败、manual hold;Windows native 和整版 release qualification 尚未由这些检查宣称完成。public-boundary 两项已有 registry projection 警告与 diff 无关。CI 按 wait_for_ci=false 未查询。
初次错误 boundary 参数、不存在的 module main 和零测试的错误 unittest discovery 均保留,不算通过;纠正后独立验证。以前 head 的数目不替代本次结果。
语义与 CI 对齐
RuntimeRouteStatus 的既有 vocabulary 和 native registry-I/O owner 复用,diff advisory 无新增载体,canonical manifest 与全树语义检查一致。实际变化明确为普通 native lock 不再选择第二根、测试隔离和 PR-only cancellation;真实双状态及坏输入继续拒绝。没有把机器义务叫成 guidance,也没有给通用规则加入特定 Goal/benchmark 名字。
我的整体评价
APPROVE,无阻塞发现。修复首次配置与后续重复调用,同时保留真实冲突的显式恢复边界。未来结构评估已应用:保留主干更强 doctor 断言、更新唯一生成清单,复用现有 classifier 与 test fixture;无需新框架或重复 Turn 修复。范围是可独立回滚的升级正确性修复,仍需仓库合并授权与该 head 的原生 readiness,不由本评审代替。
English verdict: APPROVE - exact head 23873fe; verified fresh configuration, independent source/global readback, idempotent replay and authority counterexamples; 206 tests pass with 3 Windows-only skips; exact-scope quality and risk premerge passed. No automatic migration or telemetry default change.
|
Post-merge audit completed at exact head 316 selected tests passed, 5 platform-conditional cases skipped; actual source/global configuration readback and idempotent replay passed. Exact-scope quality and risk premerge passed. This PR was merged by an external operation while the final readiness check was held. The old review body covered an earlier head although GitHub retargeted its commit metadata; the native body gate correctly rejected that stale conclusion. The new post-merge audit repairs the public review record and does not retroactively assert premerge readiness. Release qualification is rerunning on the frozen integrated source. |
Signed-off-by: huangruiteng <huangrt01@163.com>
|
Reviewer: model_agent | GPT-6 | OpenAI 动机这是合入后补充审计。PR 已由外部操作合入;本审计不表示合入前 readiness 曾通过。旧评审正文记录 23873fe,GitHub 元数据却将 commit_id 改记为最终 head;原生评审检查正确拒绝正文中缺少当前 head 的结论。应保留原讨论、补齐当前完整评审,并在后续合入中先通过 unchanged-head readiness。 新安装用户首次配置 Goal 时,需要保存设置并继续工作;维护者需要在测试中验证同一路径而不污染自己的状态目录。 一个原本空白的用户目录在配置时产生两份程序锁文件,旧实现随即误报两份状态并拒绝保存;修复后首次设置成功,重复设置不会再次写入,真实双份状态仍拒绝隐式选择。 实际配置入口、源配置与全局镜像独立读回、重复调用及目录边界反例已验证;共享测试夹具为每个用例建立独立目录。 本 PR 不自动迁移或删除用户状态,不改变账号权限、遥测默认设置、任务调度或前端设置流程,也不宣称整版发布资格已完成。 改动思路结论为 APPROVE,评审完整 head 独立规范:spec_ref = docs/product/migrations/local-state-path-migration.md;spec_revision = 649a54e。criterion_id 具体改动八个文件 +94/-5,全部阅读。
两个 workflow 仅将 cancel-in-progress 限定为 pull_request;触发器、权限、jobs、分组不变,不宣称保留每一排队提交。project_registry_io_manifest_v1.json 重新生成,唯一变化为 paths.py 的真实代码位置 262→267。 正路:真实 fixture HOME 首次调用 configure_goal_with_global_sync,源配置与全局镜像独立读回 max_children=4,再调用 changed=false。不可变 base 同一入口因自己的锁拒绝且 source 未写。反路:六组真实文件系统比较中,普通锁可 fresh,同名目录、symlink、未知状态仍 conflict;原单路由新增真实内容会 conflict,移除仅本探针自己的内容后恢复,原 registry 字节保留。现有显式路由、迁移和真实双 registry 用例继续通过。 对主干的风险最强风险是把假冲突修成权威逃逸。此处不采用 substring/prose 猜测;先拒绝 redirect,再检查精确普通文件,目录内其他内容仍参与完整扫描,新增机器状态也不能绕过。对实际配置入口、状态读回和重复保存已验证;无新表单、确认、CLI 参数或前端权限分支,既有 Chat 设置调用同一服务,因此无需前端 companion。没有新 Actor 生命周期、默认 opt-in 能力或遥测策略。 316 项最终集成 pytest 通过,5 项平台专属用例在 macOS 条件跳过,包含配置、迁移、Chat、安装/升级、source CLI 隐私和源码普查。Ruff、scripts/ci 的 3 项 unittest、diff hygiene、8 路径 public-boundary、exact-scope change-quality 和 native premerge 通过。premerge 4 项 selected、5 项 direct,无失败、manual hold;native Windows 和整版 release qualification 尚未由这些检查宣称完成。public-boundary 两项已有 registry projection 警告与 diff 无关。CI 按 wait_for_ci=false 未查询。 初次错误 boundary 参数、不存在的 module main 和零测试的错误 unittest discovery 均保留,不算通过;纠正后独立验证。以前 head 的证据已归档,不替代本次结果。同步主干后重跑全部 316 项相关集成测试;新增主干改动不扩大本 PR 的八文件 scope。 语义与 CI 对齐RuntimeRouteStatus 的既有 vocabulary 和 native registry-I/O owner 复用,diff advisory 无新增载体,canonical manifest 与全树语义检查一致。实际变化明确为普通 native lock 不再选择第二根、测试隔离和 PR-only cancellation;真实双状态及坏输入继续拒绝。没有把机器义务叫成 guidance,也没有给通用规则加入特定 Goal/benchmark 名字。 我的整体评价APPROVE,无阻塞发现。修复首次配置与后续重复调用,同时保留真实冲突的显式恢复边界。未来结构评估已应用:保留主干更强 doctor 断言、更新唯一生成清单,复用现有 classifier 与 test fixture;无需新框架或重复 Turn 修复。范围是可独立回滚的升级正确性修复,合入前未通过本任务的最终 readiness;本次补充评审仅修复合入后的记录,不能追认此前流程。实际 merge tree 与随后完成集成测试的树一致,整版 release 另从真实主干冻结重新验证。 English verdict: APPROVE - exact head 64fda42; verified fresh configuration, independent source/global readback, idempotent replay and authority counterexamples; 316 tests pass with 5 platform-only skips; exact-scope quality and risk premerge passed. No automatic migration or telemetry default change. |
Result
Fresh Goal configuration no longer treats its own exact regular native registry lock as a second runtime authority. Real dual state and redirected entries still fail closed. Tests use per-case disposable default routes, canonical skill inventory and legitimate queued-Turn claims; CI cancellation is limited to PR events.
Validation
Final head
64fda423223483697bf04b946524110818362b9c: 316 focused integration tests passed; 5 platform-specific cases skipped on macOS. Actual fresh configure independently reads source/global values and a no-change retry. Exact-scope quality and risk premerge passed (4 selected, 5 direct). Full release qualification is separate.Review and process
Post-merge audit completed at exact head
64fda423223483697bf04b946524110818362b9c: #5474 (comment)316 selected tests passed, 5 platform-conditional cases skipped; actual source/global configuration readback and idempotent replay passed. Exact-scope quality and risk premerge passed. This PR was merged by an external operation while the final readiness check was held. The old review body covered an earlier head although GitHub retargeted its commit metadata; the native body gate correctly rejected that stale conclusion. The new post-merge audit repairs the public review record and does not retroactively assert premerge readiness. Release qualification is rerunning on the frozen integrated source.