Skip to content

test: make Desktop Nightly validation deterministic - #4282

Merged
Astro-Han merged 3 commits into
mainfrom
fix/nightly-test-rsync-stub
Aug 30, 2026
Merged

test: make Desktop Nightly validation deterministic#4282
Astro-Han merged 3 commits into
mainfrom
fix/nightly-test-rsync-stub

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

The first Desktop Nightly after #4280 reached Windows packaging but failed inside check:release: a workflow-policy test executed the host rsync, which is not installed on Windows. Remove that execution instead of adding an rsync mock or making a Linux publisher dependency part of every platform package.

The exact-head Windows release check then built the installer successfully but exposed a second nondeterministic verifier boundary: the packaged sandbox cancellation probe enumerated every Win32_Process before filtering by image name and exceeded its 10-second probe budget on a busy runner. Filter the WMI query at the provider so the probe reads only the intended Maka sandbox processes.

The final diff also removes a duplicate bootstrap --delete assertion; the shared publisher assertion already covers bootstrap, payload, and feed publication.

No production publisher, package, updater, signing, or upload behavior changes. The real Apache Nightlies run remains the authority for the external rsync boundary.

Verification

  • node --test scripts/desktop-nightly-workflow-policy.test.mjs scripts/desktop-nightly-stage.test.mjs scripts/desktop-nightly.test.mjs (12/12 passed)
  • node --test scripts/verify-windows-harness.test.mjs (42/42 passed)
  • npm run build && npm run check:release (release contracts 135/135 passed before the WMI-only follow-up)
  • npx biome check scripts/desktop-nightly-workflow-policy.test.mjs scripts/verify-windows-sandbox-e2e.mjs
  • git diff --check
  • Original Windows rsync failure: https://github.com/apache/maka/actions/runs/33325772522
  • Exact-head Windows run that built the installer and isolated the WMI timeout: https://github.com/apache/maka/actions/runs/33326849933
  • Required CI and the full Windows release check are running again on final head 374e1b71
  • Apache rsync publication requires a fresh Nightly after merge; failed runs will not be rerun in place.

Root cause

Release validation was coupled to incidental properties of generic runners instead of staying at its owned boundaries: a static workflow-policy test executed the external Linux publisher transport, and a Windows process probe fetched the runner's entire process table before applying its own identity filter. The final state removes the first coupling and scopes the second query at WMI itself.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex diagnosed both exact Windows failures, audited the release-validation ownership boundaries, removed the non-hermetic rsync execution, scoped the WMI process query, simplified duplicate policy coverage, and ran focused validation. The human contributor must review the final diff and own the submission.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/S Under 100 readable lines label Aug 30, 2026
@Astro-Han
Astro-Han marked this pull request as ready for review August 30, 2026 17:48
@Astro-Han
Astro-Han marked this pull request as draft August 30, 2026 17:48

@zhiiw zhiiw 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 at exact head 5788178d (verified unchanged at review time; label completed/success, test still running).

Test-portability fix, verified rather than assumed:

  • The failure mode is exactly what the diff removes: the #4280 policy test executed the host rsync, so any machine without rsync (the Windows packaging runner, and my own Windows machine — I hit the same rsync: command not found locally while reviewing #4280) failed a repository-wide release-contract gate over a Linux-only publish dependency.
  • The stub preserves both invariants: the bash rsync() function shadows the binary, rejects --mkpath with a non-zero exit (mirroring the 3.1.3 receiver), and materializes the source tree with mkdir -p + cp -R "$src". — which mirrors the trailing-slash semantics of the real command. The workflow's actual bootstrap text still runs, so the test keeps pinning the command shape and the destination layout.
  • What the test no longer proves: real-rsync execution now happens only in the live Nightly publish, not in CI. That trade is acceptable here — the invariant that matters (no --mkpath, tree materializes) is pinned by the stub, and the end-to-end proof belongs to the next real publish, which is gated on merge anyway.
  • Scope: test-file only; the workflow and production behavior are unchanged.

Executed on a real Windows machine at this head — the exact environment that failed before: the three nightly suites now pass 12/12 with no rsync installed.


Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.

简体中文

测试可移植性修复,全部核实而非假设:失败模式正是 diff 移除的——#4280 的策略测试执行宿主 rsync,让 Linux 专用的发布依赖变成全仓库 release 门槛(Windows 打包 runner 挂;我审 #4280 时本机也撞过同一个错)。stub 用 bash 函数遮蔽 rsync,--mkpath 返回非零(镜像 3.1.3 接收端),mkdir+cp -R 复刻尾随斜杠语义;workflow 的真实命令文本仍在跑,命令形状和目标布局仍被钉住。代价如实:CI 里不再有真 rsync 执行,端到端证明留给合并后的真实发布(本来就被合并门着)。范围:纯测试文件。本机(正是之前失败的环境)三套件 12/12。批的时候 CI test 还在跑。

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

Approving test: make Nightly bootstrap check portable at head 5788178d.

I reviewed the single-file test change:

  • It replaces the fragile bootstrap.run.replace('--protect-args ', '') string hack with a bash rsync() stub that faithfully drives the bootstrap's rsync invocation against a local workspace: it returns exit 64 when --mkpath is present (emulating an rsync build without that flag) and otherwise emulates the copy from the last two args (source, destination). This makes the bootstrap check deterministic and portable — no reliance on the host's real rsync supporting --mkpath/--protect-args.
  • The stub's argument position assumptions match rsync's CLI shape, and the --mkpath→exit-64 mapping exercises the bootstrap's compatibility fallback as intended.
  • Test-only change; no production reachability impact. 0 unresolved review threads; mergeable MERGEABLE.

No P0–P2.

简体中文

批准 test: make Nightly bootstrap check portable,head 5788178d
审查单文件测试改动:它把脆弱的 bootstrap.run.replace('--protect-args ', '') 字符串 hack 换成 bash rsync() stub,用本地 workspace 忠实驱动 bootstrap 的 rsync 调用——参数含 --mkpath 时返回 exit 64(模拟不支持该 flag 的 rsync 版本),否则从最后两个参数(source、destination)模拟拷贝。这让 bootstrap 检查确定且可移植,不再依赖宿主真实 rsync 是否支持 --mkpath/--protect-args。stub 的参数位次假设与 rsync CLI 形状一致,--mkpath→exit-64 的映射按意图触发 bootstrap 的兼容性回退。纯测试改动,无生产可达性影响;0 未解决线程;mergeable MERGEABLE。无 P0–P2。

Remove the host-rsync execution from the cross-platform release contract. Keep the receiver compatibility and append-only guarantees as workflow policy assertions, while the real Nightlies run verifies the external rsync boundary.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the fix/nightly-test-rsync-stub branch from 5788178 to 36bd55c Compare August 30, 2026 17:50
@Astro-Han Astro-Han changed the title test: make Nightly bootstrap check portable test: keep Nightly workflow policy hermetic Aug 30, 2026

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the latest head 5788178d7. No P0 or P1 — approving. One P3 inline. Still draft, so read this as "content is fine when you are ready". label is green, test was still running.

The portability goal is right and the stub is mostly well built. Rejecting --mkpath with a non-zero status is the correct way to simulate an rsync 3.1 that does not know the flag, and taking the last two positional arguments as source and destination matches the command's shape. The "$source_path". concatenation looks odd at a glance but is the standard cp -R src/. dest idiom for copying contents rather than the directory itself, and it is safe here because the test already pins the trailing slash on the source.

My P3 is that the stub is more permissive than the rsync it stands in for, which costs this test the specific property it was created to prove. Details inline.

One older point, and I will stop repeating it after this: the --delete negative assertion I raised on #4271 and again on #4280 is still not here, and the stub does not reject --delete either — it only rejects --mkpath, so a --delete addition would cp -R happily and pass. I understand if that is a deliberate call; I am noting it once more only because this PR is specifically about strengthening this test, and then leaving it with you.

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

Comment thread scripts/desktop-nightly-workflow-policy.test.mjs Outdated
@Astro-Han
Astro-Han marked this pull request as ready for review August 30, 2026 17:51
Filter the WMI query at its provider so a busy runner does not spend the probe budget enumerating every process before cancellation evidence can be observed.

Generated-by: Codex
Keep the append-only rsync invariant in the shared publisher assertion instead of pinning the bootstrap step twice.

Generated-by: Codex
@Astro-Han Astro-Han changed the title test: keep Nightly workflow policy hermetic test: make Desktop Nightly validation deterministic Aug 30, 2026
@Astro-Han
Astro-Han merged commit 66e6f4e into main Aug 30, 2026
2 checks passed
@Astro-Han
Astro-Han deleted the fix/nightly-test-rsync-stub branch August 30, 2026 18:50
saltand pushed a commit to saltand/maka-agent that referenced this pull request Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants