Skip to content

fix(update): harden authority upgrade subprocesses - #5107

Merged
huangruiteng merged 6 commits into
loopx-project:mainfrom
Duang777:codex/fix-windows-authority-upgrade-env
Sep 26, 2026
Merged

huangruiteng merged 6 commits into
loopx-project:mainfrom
Duang777:codex/fix-windows-authority-upgrade-env

Conversation

@Duang777

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

  • Goal/source and gap: Recent authority archive changes added two text-mode subprocess calls without the runtime guarantees required by the existing launchers and UTF-8 gate. The Windows installer invoked scripts/loopx_entry.py without LOOPX_RELEASE_ROOT, so pre-activation upgrade always exited with code 2.
  • Observable before -> after, with the validation row that proves it: The Windows pre-activation upgrade now runs against the copied candidate release with the selected Codex home, and failures retain bounded stdout and stderr. Both authority upgrade reads pin UTF-8.
  • Issue/task and intended base: Related to the main regressions exposed by test(control-plane): align timeout-aware runtime doubles #5104; base is main.

Scope And Continuation

  • Completed scope and remaining work: Fixes candidate environment propagation, subprocess decoding, and failure diagnostics for authority upgrades. Native Windows CI remains the final platform check.
  • Slice boundary / successor: Complete within this scope. No UI or persisted authority schema changes.

Validation

  • Tested revision: 64eb757855299f043e55bc247ac47f939a40e583
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
unit passed python -m pytest tests/test_windows_install.py tests/test_runtime_subprocess_utf8.py tests/test_self_update_runtime_activation.py -q: 32 passed, 5 platform skips.
static passed Ruff on all changed and directly affected Python files; git diff --check; the runtime subprocess UTF-8 census reports no offenders.
real_entrypoint passed The copied-release entrypoint completed authority-archive upgrade --all-known --execute in an isolated temporary home using the candidate environment.
integration passed Standard loopx canary premerge --from-git-diff: 5/5 selected canaries passed, including local install, packaged install, and update smokes.
real_backend not_applicable The change only supplies process environment and decoding for existing file-backed upgrade commands; it does not alter a provider or storage transaction.
manual not_run Native Windows execution is unavailable locally and is delegated to the required windows-powershell CI job.
  • Coverage and gaps: Unit tests lock the exact candidate command, environment, and stderr behavior. Existing Windows installer tests cover promotion, external skills, and rollback on native Windows. No frontend entry point changed.

See validation disclosure guidance.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none
  • Attention review: N/A; no visual surface changed.

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

  • Direction / acceptance reference, when applicable: Restore the established candidate-release and UTF-8 subprocess contracts for install/update lifecycle commands.

Shared-authority RFC fixture impact

N/A. This PR does not change authority schemas, provider semantics, or migration behavior.

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

Pass the candidate release environment to the Windows pre-activation upgrade. Pin UTF-8 for authority upgrade output and retain stderr in failures.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>

@Duang777 Duang777 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Exact-head self-review for 64eb757855299f043e55bc247ac47f939a40e583: ready for maintainer review. The Windows installer now passes the copied candidate release and selected Codex home to its pre-activation authority upgrade, while both affected authority upgrade reads pin UTF-8. Failures retain bounded stdout and stderr. Validation passed: 32 focused tests, Ruff, an isolated real candidate-entrypoint upgrade, and the standard premerge gate with 5/5 selected canaries. Native Windows behavior remains covered by the required CI job.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Collaborator Author

Windows CI exposed a stale rollback assertion after the environment fix allowed the native installer suite to reach its late-failure case. PR #5102 intentionally retains an upgraded candidate as recovery material when a post-upgrade user-surface write fails, but the older transactional-install test still expected deletion.

Head ff15b311a updates that regression to prove both halves of the contract: launcher, pointers, and skills roll back to the known-good state, while the upgraded candidate and its entrypoint remain available for recovery. Focused tests pass (28 passed, 5 platform skips), Ruff and diff checks pass, and the standard premerge gate passes 5/5 selected canaries. Native Windows rerun remains the authoritative platform check.

…hority-upgrade-env

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…hority-upgrade-env

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
…hority-upgrade-env

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
kokokoXUY added a commit to kokokoXUY/loopx that referenced this pull request Sep 26, 2026
The GH-C100 row's validation column still names three test files that loopx-project#5105
removed with the Stage 0 Python prototype
(`test_coordination_executor.py`, `test_coordination_file_provider.py`,
`test_coordination_provider_parity.py`), so a reader cannot run it. Point it at
the native-provider ladder and the parity suite that own the row now; both were
checked to exist on `main`.

The CI half of this change is gone: the stale path in
`.github/workflows/python-tests.yml` is removed by loopx-project#5107, which also fixes the
remaining encoding sites, so this PR no longer touches the workflow.

Validation: `grep` on the row confirms no retired path remains, and both
replacement entry points exist on `main`.

Signed-off-by: kokokoXUY <13682395396@163.com>

@kokokoXUY kokokoXUY left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the fix against current main (b15413f); the diagnosis holds and I have
withdrawn my own duplicate so this is the single place the change lives.

Independently reproduced. The windows-powershell job's Run native Windows lifecycle tests step aborts before collecting anything:

ERROR: file or directory not found: tests/control_plane/test_coordination_file_provider.py
no tests ran in 0.00s

pytest resolves its arguments before it collects, so the other 10 files the step names
never execute and the lane reports failure for a reason unrelated to the change under review.
#5105 removed the file; the step kept naming it.

The new guard works on today's formatting. Running its extraction against the step on
main yields 11 paths, all of which exist. I also confirmed the removal is the
right fix rather than a rename: test_coordination_executor.py,
test_coordination_provider_parity.py and test_nokv_shadow_provider_probes.py were retired
in the same change and are not referenced by CI or the task board any more.

One robustness note on the guard. The pattern

test_paths = re.findall(r"^\s+(tests/\S+\.py)\s*$", step, re.MULTILINE)

only matches a path that occupies a whole line by itself. If a later edit puts two paths on one
line, or leaves a trailing comment after one, the guard silently checks fewer paths and still
passes, because assert test_paths only rejects an empty list. The step currently yields
11 line-anchored paths and 11 path-shaped tokens, so nothing is
missed today; asserting that the two counts agree would keep it that way:

assert len(test_paths) == len(set(re.findall(r"tests/[\w./-]+\.py", step)))

Not a blocker for this change — the fix and the guard both do what they claim right now.

Overlap, for the record: my #5120 (adding encoding= at the upgrade call) is closed as
superseded by this PR, and my #5125 no longer touches the workflow for the same reason. Both
say so on their own threads.

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

结论:APPROVE。没有发现本 PR 引入的阻塞问题;这是完整的候选 release 子进程修复,不是放宽 authority 升级或回滚门槛。

Reviewed head: f315c78ad63fc340e7683e5a4193d55d04803722
Comparison base: 03629c69d388a4c53961ef1aaed96f442da20f5c

动机

旧 Windows 安装器虽然调用候选目录的入口,却没有传入入口必需的 release root。用同一组隔离 fixture 执行真实子进程,base 在升级前退出 2,head 则完成实际 authority-admin 命令。这不是偶发测试噪声:反复安装会反复卡在同一位置。另两处 text-mode 读取没有固定 UTF-8,旧 codec census 确实报告这两个调用点。修复后,安装流程能继续,失败时也保留可用于恢复的诊断。

本次限定交付是恢复既有安装/回滚子进程契约及当前原生测试入口,不宣称重做 authority provider 或完整证明所有 Windows 文件系统行为。持续执行能力和用户体验均改善:没有新增设置、手工同步状态或运行权限。

改动思路

沿用现有安装所有权:PowerShell 入口进入 Python 安装器,选定兼容 Python、复制可信 release、验证候选,然后执行已有的 authority 升级,最后才发布 launcher-visible pointer。格式识别、备份和迁移仍由 typed TypeScript authority-archive effect 决策,Python 只补齐调用环境和解码,不建立第二套规则。

最强的反对理由是“mock 返回成功”不能证明这个入口真的可运行,以及不能为了变绿而删掉恢复保护。因此我另外运行了真实复制候选的入口和非零退出的真实子进程;并核对 #5102 已有的候选保留规则。只补 encoding 不能修复缺失环境;引入通用 runner 又没有必要。当前私有 helper 是适当、可单独回退的边界。

具体改动

关键代码讲解

  • _upgrade_authority_archive,162 行:由已选 release 和 skills 参数导出 child 环境,复用 _entry_command 与选定 Python;保持既有 --all-known --execute 和超时。新增 UTF-8 解码及有界 stdout/stderr,非零返回仍中止激活。真实正向入口成功;真实负向入口的非 ASCII stderr 得以保留。
  • install_windows,317 行:用 helper 替换内联调用,升级仍在 skills/launcher/pointer 变更之前。晚期用户面写入失败仍恢复快照、保留已经升级的候选,而不是把 launcher 回滚误当成存储格式回滚。更新的测试明确验证两半契约,没有修改生产回滚规则。
  • execute_rollback_plan,1198 行:只固定 compatibility child 的 UTF-8/errors=replace。目标仍用 --require-current 做只读检查,不会为了回滚迁移数据;不兼容拒绝、doctor 失败恢复原链接的路径保留。
  • workflow guard,242 行:原生 Windows step 删掉 #5105 已退休的测试参数,并检查当前列出的文件存在。完整 diff 是 5 文件、125 增/12 删,新增主要是环境、错误诊断及恢复断言测试,没有新增模块、CLI 或持久化声明。

这是对 whole PR 的判断,不是只接受后续测试修正。初始提交之后增加的晚期恢复断言与 workflow 修复也一起核查了。当前 guard 按“一行一个路径”的格式工作;以后将多个参数放在同一行时,应同步加强 token 完整性检查。这是后续格式修改时的非阻塞维护建议。

对主干的风险

风险集中在错误候选上下文、locale 解码和恢复材料丢失,而不是新 authority 状态机。独立验证使用 source checkout 的 Python 3.13.13;相关 Python suite 为 129 passed、5 skipped,另有 authority archive 集成 4 passed,Ruff、diff hygiene、公私边界扫描通过。5 项原生 Windows 用例在 base/head 都受相同平台条件跳过,不能算通过,原生 pointer/skills 写入仍是明确的本机证据边界。

真实 candidate base/head 对比和真实 UTF-8 非零 child 证明 mock 没有提供待证结论。local-install、准备好 source Chat bundle 后的 packaged-install、update/rollback smoke 均通过;回滚 smoke 使用实际临时 shell/链接,而不是生产回滚。frontend/Lark 无伴随改动需求:没有配置、投影字段或交互入口变化,用户入口就是既有 installer/update。

语义与 CI 对齐

同一不可变 base 的 runtime UTF-8 census 失败在本次两个调用点,head 全部通过。原 workflow 的退休参数导致 pytest 在收集前退出;head 当前列表可收集 158 项,收集不等于在 Windows 执行通过。当前语义 vocabulary 和 maintainability ratchet 在准备 npm dev dependencies 后,base/head 都通过且 inventory 一致。

干净、尚无生成 Chat bundle 的 base/head 上,packaged smoke 都因同一个不完整测试归档失败:缺 manifest 后,fallback 所需 dashboard 目录也未打入 archive。归档清单、POSIX installer 和 Chat builder 不在本 diff,失败发生在改动调用之前;生成 bundle 后 head 重跑通过。保留这条原始失败及归因,不把无关红项转成对此 PR 的 REQUEST_CHANGES,也不冒充全套 CI/merge-readiness 已通过。本评审没有查询或等待远端 CI。

我的整体评价

持续推进与用户体验均为 improved,命名、权限和 typed authority 所有权保持原状;本 PR 复用既有契约,没有创建更宽泛的 actor 协议。未来向的限定重构已经体现在私有 helper:同一安装所有者下集中该操作的环境与错误处理,而没有追加框架。

base/head 的实际退出、诊断、codec census 与收集结果解释了必要差异,普通回滚、格式所有权及恢复规则未漂移。平台跳过和原始 archive fixture 缺口独立披露,不隐藏,也不要求此 PR 修复无关路径。APPROVE 只代表此 exact head 的代码评审结论,不代表合并或本机升级授权。

English verdict: APPROVE - f315c78. The real candidate entry now succeeds where the immutable base exits 2; UTF-8/error handling, recovery semantics and workflow collection are verified. 129 focused tests and 4 archive tests pass; 5 native Windows cases are skipped, not passed. The initial unprepared archive failure is independently base-equivalent; the prepared rerun passes. No blocking PR regression found.

@huangruiteng
huangruiteng merged commit d70b2d1 into loopx-project:main Sep 26, 2026
25 of 31 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