ci: point the windows test job's temp at a short-alias-free path - #10034
Merged
Conversation
Collaborator
Collaborator
|
Thanks for the PR! Re-running triage on Template looks good ✓ Problem: observed, not theoretical. The before-state is pinned in real logs: the Direction: aligned — CI-only change, hardens the Windows gate, no product surface touched. Size: not applicable — no core paths. 2 files, +150/−0: 25 lines of workflow, 125 lines of test-script wiring. Approach: minimal and self-checking. Both notes from the previous pass are resolved on this head: the Risk: no elevated risk signals (no high-risk-path matches). Moving on to code review. 🔍 中文说明感谢贡献！本次在 模板完整 ✓ 问题：已观测到，不是理论问题。修复前状态有真实日志佐证：#10021 的 方向：对齐——纯 CI 变更，加固 Windows 门禁，不触及产品代码。 规模：不适用——未触及核心路径。2 个文件，+150/−0：25 行工作流，125 行测试脚本。 方案：最小化且可自检。上次审查的两个问题在本 head 上均已解决： 风险：无升级风险信号（未命中高风险路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewRe-reviewed at On the new code:
Test evidence — the PR's own CI, read via API (no PR code built or executed)Before-state (real logs): #10021's Windows run (run 32858173547, 2026-08-25) — 28 failed files, 588 After-state on the Windows lane (real logs, with one commit caveat): workflow-dispatch run 32955788152 ran This head's CI: green everywhere it runs.
One row per check name (latest run); failures sort first. / 每个检查名一行（取最新一次运行），失败项排在最前。 Not verified: the exact head on the Windows lane — the two commits after the dispatch run (case-insensitive comparison in the guard, Linux-only gate on the symlink simulation) have no Windows-lane execution yet. Both deltas are low-risk in one direction: the comparison change only relaxes case-only differences from fail to warning (genuine aliases still fail), and the test gating doesn't touch workflow behavior. The merge-queue lane exercises the final code on Windows before anything lands, and that is the oracle for these commits — no 中文说明代码审查在 新增代码方面：
测试证据——本 PR 自己的 CI，经 API 读取（未构建或执行任何 PR 代码）修复前（真实日志）： #10021 的 Windows 运行（run 32858173547，2026-08-25）——28 个文件失败，588 处 修复后（Windows 通道，真实日志，有一个提交差距需说明）：workflow-dispatch run 32955788152 于 **本 head 的 CI：**所到之处全绿。 **未验证：**最终 head 在 Windows 通道上的运行——dispatch 运行之后的两个提交（守卫的大小写不敏感比较、符号链接模拟限定 Linux）尚无 Windows 通道执行记录。两处差异的风险都朝单一方向：比较改动只把仅大小写差异从失败放宽为告警（真实别名仍失败），测试平台限定不触及工作流行为。merge-queue 通道会在任何代码合入前以最终代码运行 Windows 通道，那就是这两个提交的验证基准—— — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — both prior blockers verified fixed in real lane logs, the guard is tested by execution rather than text-pinning, and this head's CI is green; the last two commits still owe a Windows-lane run, which the merge queue supplies before anything lands. Stepping back: this is still the right shape of fix, and it matured well across the re-runs. I didn't take the PR's word for the problem — the before-state (28 failed files, 588 What keeps this at 4 rather than 5: the final head has not been exercised on the Windows lane — the dispatch evidence stops at Am I approving because it's genuinely good, or because I ran out of objections? Genuinely good — a focused CI fix with execution-level tests, real-lane evidence for every claim it makes, and a revert story that is one step deletion. 中文说明退一步看整体：这仍然是正确形态的修复，并且在多轮迭代中成熟得很好。我没有只听 PR 的描述——修复前（28 个文件失败、#10021 通道上 588 处 没给到 5 的原因：最终 head 尚未在 Windows 通道上运行——dispatch 证据止于 是真心认可才批准，还是因为找不出反对理由？是真心认可——一个聚焦的 CI 修复，带执行级测试，每项声明都有真实通道证据，回滚方式就是删除那个步骤。 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
yiliang114
commented
Aug 26, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
One blocking Windows runner issue remains at the current head.
chiga0
reviewed
Aug 26, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Blocker found — not approving at this head.
Checked: placement of the new step in Test (windows-latest, Node 22.x) (after checkout, before setup-node — correct); the if: condition matches sibling steps; RUNNER_WORKSPACE as a short-alias-free anchor; cross-file read of .github/actions/configure-windows-runner/action.yml to verify the TEMP/TMP override order and the UTF-8 pattern established there.
CI disclosure: Test (windows-latest, Node 22.x) was SKIPPED in the run against this head (the job's if: excludes pull_request events, so the step has not been exercised in any check visible here). The remaining checks are automation bots — no build/test check ran over the changed file.
Cross-check: the two findings below were previously filed by another reviewer; I independently identified both and am confirming them as holding at the current head.
Reviewed with AI assistance.
yiliang114
enabled auto-merge
August 26, 2026 09:12
yiliang114
commented
Aug 26, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
The redirect itself is taking effect, but the real self-hosted Windows run exposed one blocking path-length regression. Details inline.
chiga0
reviewed
Aug 26, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
One blocker at this head — not approving.
Round 2 ledger (base: round-1 head 297da27)
| id | prior verdict | status at 0cca0ce |
|---|---|---|
| R1-1 | blocker: Out-File missing -Encoding utf8 | Fixed — both Out-File calls now carry -Encoding utf8 -Append |
| R1-2 | major-question: configure-windows-runner re-appends TEMP/TMP, overriding redirect | Fixed — redirect step is now placed after configure-windows-runner in the YAML; the in-code comment reads "after self-hosted tuning so this value wins"; the ordering test confirms (redirectTempIndex > configureUseIndex) |
R2-1 — Blocker: new temp path exceeds socket-path length limit on self-hosted lane
Inline comment on the $temp assignment below.
Checked at this head
- Placement: redirect step comes after
configure-windows-runner(correct; GITHUB_ENV last-write wins) and before both node-setup paths (correct). - Encoding:
-Encoding utf8 -Appendpresent on bothOut-Filecalls — matches the patternconfigure-windows-runnerdocuments. - Verify step:
process.exitCode=1guard present; runs after both node-setup paths and before Install Dependencies — correct placement. - Ordering tests: cover
redirectTempIndex > configureUseIndex, both< hostedNodeIndexand< selfHostedNodeIndex,verifyTempIndexbetween both node setups and install.
CI at head 0cca0ce
Test (windows-latest, Node 22.x)— failure (run 32951775140 / job 98124581703). The two new steps both pass; the suite fails at "Run tests and generate reports". Failure includessupervisor-process.test.tssocket-path mismatch (see R2-1) and adisk fullerror on the self-hosted runner.Test (ubuntu-latest, Node 22.x)— success.Integration Tests (CLI, No Sandbox)— skipped (both runs); pre-existing skip condition, not introduced by this PR.Verify temp paths carry no short aliasstep — success — confirms the GITHUB_ENV export lands correctly for the verify step itself.
Reviewed with AI assistance.
Collaborator
Author
|
Follow-up verification on commit 90d1626:
Local workflow wiring tests (9 passed, 1 skipped), Prettier, and actionlint 1.7.12 also pass. |
The short-alias guard compared TEMP/TMP to fs.realpathSync with a strict !==. On Windows realpathSync returns the on-disk casing, so a drive-letter or directory-case difference — the same directory under one spelling — failed the job for a reason unrelated to the 8.3 alias. The guard runs on both lanes, so that false positive could red-wall the self-hosted lane against its own pre-existing RUNNER_TEMP. Compare case-insensitively instead: RUNNER~1 -> runneradmin differs by more than casing and still fails, while a casing-only difference warns and passes. An unset TEMP/TMP now reports itself instead of surfacing as realpathSync(undefined)'s TypeError. The test now extracts the script from the workflow and runs it against symlinked temp dirs, covering all three outcomes, rather than pinning the JS text — a substring pin cannot tell a working comparison from a reverted one.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— qwen3.8-max via Qwen Code /review (v0.22.0)
yiliang114
added
the
autofix/takeover
Collaborator
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Collaborator
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no actionNo code changes this round. Triage of the new feedback: Prior blockers are fixed and verified on the real laneAll earlier blockers — the UTF-16 The two new suggestions stay deferred to follow-up, as already recorded on their threadsNeither new suggestion claims current behavior is wrong; both are Suggestion-severity, and each thread already carries a recorded reply deferring it out of this PR. Both claims were re-verified against the exact code at the current head before affirming the deferral:
The PR has been through roughly five review rounds, after which repo policy lands only Critical fixes and defers the remaining suggestions; both threads keep their recorded reason, and both findings are also recorded in this PR's deferred review findings tracker, so nothing is silently dropped at merge. Other new feedback
Files changedNone — no commit this round. No verification commands were run, since nothing was changed or committed; the latest CI snapshot at this head shows 中文说明Autofix 本轮结果:无操作本轮未做任何代码修改。新反馈的分类处理如下: 此前的阻塞问题已修复,并已在真实通道上验证所有早先的阻塞问题——UTF-16 编码写入 两条新建议维持"推迟到后续跟进"的处理,其线程上已有记录两条新建议均未声称当前行为有错;两者都是 Suggestion 级别,且各自的线程上已有一条记录在案的回复,将其推迟到本 PR 之外。在确认该推迟处理之前,两条建议均已对照当前 head 的确切代码重新核实:
本 PR 已经历约五轮评审,按仓库政策,此后只落地 Critical 级别的修复,其余建议推迟处理;两条线程都保留了记录在案的理由,且两条建议均已记录在本 PR 的“推迟处理的评审发现”跟踪 issue 中,不会在合并时被静默丢弃。 其他新反馈
变更文件无——本轮没有提交。由于未做任何修改、也没有提交,本轮未运行任何验证命令;当前 head 的最新 CI 快照显示 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 83 passed · 0 failed · 83 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：83 通过 · 0 失败 · 83 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 10034 deep verification —
|
| build | exact match | casing-only | genuine alias (RUNNER~1) |
unset |
|---|---|---|---|---|
base fc0e827 |
guard step absent | guard step absent | guard step absent | guard step absent |
strict !== (reconstructed intermediate) |
PASS | FAIL (false positive) | FAIL (alias) | FAIL (unset) |
head fc77adb (case-insensitive) |
PASS, no warning | WARN + PASS | FAIL (alias) | FAIL (names both keys, no TypeError) |
The strict variant was reconstructed by removing exactly the casing branch (the hunk the PR's final commit introduced; intermediate commits are unreachable in the depth-2 checkout). It false-positives the casing-only cell with TEMP carries a short alias: …/RUNNERADMIN -> …/runneradmin — the regression the commit exists to stop. The head build warns (::warning:: … only by casing) and exits 0. The case-insensitive comparison is load-bearing. Witness: 01-ab-guard-builds.png.
Full independent matrix (9 env/symlink cells incl. independent-TEMP/TMP and unset permutations, plus fixture sanity controls and both A/B control cells): 40/40 scripted checks passed — 03-guard-symlink-cells.png, log guard-harness.log.
Wiring, asserted from the parsed YAML step array (independent of the PR's indexOf helpers): redirect is hosted-only (runner.environment != 'self-hosted'), runs under powershell, creates the directory with New-Item -Force, writes both TEMP= and TMP= to $GITHUB_ENV, and sits after configure-windows-runner but before both node-setup steps; the guard's if is exactly skip_ci != 'true' (both lanes), it runs after both node setups and before the npm-cache/install steps, and inherits the workflow-level bash default shell.
Targeted gates
| gate | head fc77adb |
base fc0e827 |
|---|---|---|
scripts/tests/no-ak-integration-ci.test.js (vitest) |
13/13 pass | 10/10 pass (+3 tests, +0 failures) |
| actionlint 1.7.12 (repo flags) | clean (exit 0) | — |
| actionlint liveness (planted broken expression) | caught at ci.yml:1083, restored after |
— |
bash -n on extracted guard run block |
OK | — |
| shellcheck 0.11.0 on extracted guard run block | clean (exit 0) | — |
| mutation matrix (below) | 3 killed / 2 survivors, both classified | — |
Pre-existing-failure attribution: the base run of the same file is green with byte-identical test names for the 10 shared tests; the head delta is exactly the 3 new tests, no failures either side.
Mutation matrix (vacuity + completeness)
Each mutant alters only ci.yml in a scratch worktree; the PR's own 13-test suite is the oracle. M0 is the unmutated control (green — kills mean nothing without it).
| mutant | change | suite result | classification |
|---|---|---|---|
| M0 | control, unmutated | green (13/13) | baseline |
| M1 | remove if (real === value) continue; |
survived | test gap (see Finding 1) |
| M2 | revert casing branch (strict !==) |
killed | positive control — red at the intended assertion |
| M3 | loop over ['TEMP'] only |
survived | test gap (see Finding 2) |
| M4 | redirect moved after setup-node | killed | wiring assertions live |
| M5 | redirect step deleted | killed | wiring assertions live |
Witness: 02-mutation-matrix.png, log mutation-matrix.log.
The central test is not vacuous: M2 fails accepts, warns, or fails on the three ways an env path can meet its realpath at its not.toThrow() with the behavioral mismatch quoted in the error — TEMP carries a short alias: /tmp/temp-guard-…/RUNNERADMIN -> /tmp/temp-guard-…/runneradmin (1 failed | 12 passed). The failure names expected-vs-actual, and it lands in the file that reads the mutated workflow. The two survivors were each probed behaviorally to prove they are coverage gaps, not dead code (Findings 1–2).
Secondary claim — supervisor socket-path budget
Executed against the real head function getAgentViewSupervisorSocketPath from packages/cli/dist/ (UNIX_SOCKET_PATH_LIMIT = 100), coupling globalDir and runtimeDir to the temp dir exactly as the supervisor tests do (04-socket-path-probe.png, log socket-probe.log, 8/8):
| temp length | primary socket path | outcome |
|---|---|---|
38 ch (PR-stated ECS RUNNER_TEMP) |
83 bytes (matches PR) | primary kept |
| 54 ch | 99 bytes | primary kept |
| 55 ch | 100 bytes | flips to fallback tier |
| 57 ch (38 + 19, the workspace-adjacent replacement) | 102 bytes (matches PR) | lands on compact qav-* tier, < 100 |
The bisect through the real code path switches at exactly the 100-byte limit, and the 57-char case lands on the qav-<digest-or-uid> compact tier — precisely the "new qav-* fallback mismatch" the PR describes. The self-hosted carve-out (keep the shorter configured RUNNER_TEMP) is therefore justified by measurement, not assertion. The hosted replacement is safe: a D:\a\qwen-code-style workspace-adjacent path (~30 ch) yields a ~75-byte primary, far under the limit.
Findings
Ordered by severity. None are blocking; the guard as shipped is correct.
1. Suggestion — suite does not pin "exact match emits no warning" (mutant M1 survives)
Removing if (real === value) continue; from the guard leaves all 13 tests green, yet changes behavior: an exact-match path then falls into the casing branch and prints a spurious warning. Measured: M1 on exact-match paths: code=0 stdout="::warning::TEMP differs from its real path only by casing: /tmp/mutprobe-…/runneradmin -> /tmp/mutprobe-…/runneradmin" (twice — once per key). Severity is low (warning only, exit code unchanged), but the suite's stated design ("The guard's decisions are asserted by executing it") does not actually distinguish the working fast path from the reverted one.
Reproduce:
git worktree add tmp/mx HEAD && cd tmp/mx
# delete "if (real === value) continue; " from the guard run line in .github/workflows/ci.yml
npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/no-ak-integration-ci.test.js # 13/13 greenSuggested fixture (in the existing case-1 cell): assert runGuard({ TEMP: canonical, TMP: canonical }) returns stdout not containing ::warning::.
2. Suggestion — suite does not pin that TMP is checked independently (mutant M3 survives)
Narrowing the loop to for (const key of ['TEMP']) leaves all 13 tests green — every behavioral cell sets TEMP and TMP to the same value, and the unset cell matches on TEMP is not set — while an alias carried by TMP alone now slips through. Measured: M3 on {TEMP clean, TMP aliased}: code=0 (the 8.3-alias shape this guard exists to catch, undetected). Real-world risk is low because both producers (configure-windows-runner, the hosted redirect) always write TEMP and TMP together, but the guard's contract covers both keys and nothing asserts it.
Suggested fixture: runGuard({ TEMP: canonical, TMP: alias }) → expect throw matching TMP carries a short alias (the independent harness cell C6 does exactly this and fails mutant M3).
3. Nit — nonexistent temp path crashes with a stack trace instead of the clean-message style
realpathSync throws ENOENT when TEMP/TMP points at a missing directory: measured C8 nonexistent path: code=1 stderr="node:fs:2791". The step still fails loud (nonzero exit), and both producers guarantee the directory exists (New-Item -Force / pre-existing RUNNER_TEMP), so this is unreachable in the wired lanes — noted only because the unset branch went to the trouble of replacing a TypeError with a clean message while this adjacent shape still throws.
4. Nit — a trailing-separator path would be labeled "carries a short alias"
Any spelling difference that is not pure casing gets the alias message: measured C9 trailing separator: code=1 … "TEMP carries a short alias: /…/runneradmin/ -> /…/runneradmin". Neither writer emits a trailing separator (both write $temp/$env:RUNNER_TEMP verbatim), so this shape cannot reach the guard today; the message would simply mislabel it if it ever did.
Not covered
- Real Windows execution. No Windows host and no
pwshin this container, so neither lane ran end-to-end; thetest_windowsjob triggers only on merge_group/schedule/workflow_dispatch, so the PR's own CI never ran it either. The PR body cites run 32955788152 (self-hosted lane: redirect skipped, guard passed, supervisor tests green) — not verifiable here (no GitHub token); notably it contains no hosted-lane run, so the redirect's first live execution will be post-merge. The guard's decision logic was instead exercised through the symlink-equivalent harness above, which reproduces each comparison the guard makes. - Redirect PowerShell step, static review only.
Join-Path $env:RUNNER_WORKSPACE 'qwen-code-temp'+New-Item -Force+ twoOut-File -Encoding utf8 -Appendwrites is the byte-for-byte idiom already running in production viaconfigure-windows-runner/action.yml(and pinned by the suite), so the GITHUB_ENV propagation risk is inherited from proven code, but I did not execute the step itself. The replay is therefore uncalibrated — a real hosted-lane log would have been the calibrating artifact and none is retrievable here. - Per-commit attribution. Metadata lists 6 commits; the depth-2 checkout makes only the aggregate
HEAD^1..HEADreachable (git rev-list HEAD^1..HEAD^2= 1, shallow = true), so the intermediate strict-guard commit's evolution was reconstructed from its message rather than checked out. - yamllint could not run (
pip3 install --user→ Permission denied in-container); actionlint, shellcheck, andbash -ncovered the workflow instead. - Repo-wide gates (
npm run lint, full test suite) — not run; the affected surface is workflow YAML + one root-level test file, both gated directly above.
Methodology
Environment: the CI verify container (node:22-bookworm), tree at merge commit faadf45 with npm ci + npm run build already complete. All harnesses live in tmp/pr10034-verify-20260827-024413/ as rerunnable .mjs files with raw logs beside them (guard-harness.log, mutation-matrix.log, socket-probe.log, actionlint-clean.log, shellcheck-guard.log). The guard harness parses ci.yml with js-yaml, extracts the guard's node -e argument by delimiter (asserting no embedded double quotes), and execFileSynces it with an env containing only TEMP/TMP against symlinked dirs (runneradmin target, RUNNERADMIN casing alias, RUNNER~1 genuine alias) — the same mechanism the PR's test uses, derived independently; fixture sanity cells assert the symlinks really realpath to the target before any verdict cell runs. The A/B control variant is the head script with exactly the casing branch removed; the base cell asserts step absence from the base worktree's YAML. Mutants were applied to a git worktree copy of ci.yml (main tree never mutated), run through the PR's vitest suite, and restored; survivors were behaviorally probed by executing the mutant scripts directly. The socket probe imports the built dist/ function and bisects temp-dir length across the tier switch. The actionlint gate was proven live by planting a broken ${{ }} expression in the scratch worktree (caught at ci.yml:1083) and restoring. Scratch worktrees were removed after use; git status is clean. One probe iteration was discarded and rerun: the first socket-probe P2 cell forgot to couple runtimeDir to the temp dir the way the supervisor tests do — a harness setup bug, not a PR signal.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/no-ak-integration-ci.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/no-ak-integration-ci.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/no-ak-integration-ci.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/no-ak-integration-ci.test.js: P (exit 0)
round 2 · scripts/tests/no-ak-integration-ci.test.js: P (exit 0)
round 3 · scripts/tests/no-ak-integration-ci.test.js: P (exit 0)
round 4 · scripts/tests/no-ak-integration-ci.test.js: P (exit 0)
round 5 · scripts/tests/no-ak-integration-ci.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
approved these changes
Aug 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
chiga0
approved these changes
Aug 27, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
All prior blockers resolved — no new blockers.
Prior findings at this head:
| ID | Prior verdict | Status |
|---|---|---|
| R1-1 (round 1) | Blocker: Out-File missing -Encoding utf8; Win PS 5.1 writes UTF-16LE by default, breaking GITHUB_ENV |
Fixed — -Encoding utf8 -Append present on both TEMP and TMP lines |
| R2-1 (round 2) | Blocker: workspace-adjacent path 19 chars longer than RUNNER_TEMP; socket-path limit crossed on self-hosted ECS lane |
Fixed — redirect step now guarded with runner.environment != 'self-hosted'; self-hosted keeps its configured shorter path |
Checked at this head:
- Redirect step: guard condition
!= 'self-hosted'correct;shell: 'powershell'explicit;-Encoding utf8 -Appendon bothOut-Filecalls. ✓ - Guard step: runs after both
actions/setup-nodeandself-hosted-node(ordering assertions in test confirm this);node -e "..."is shell-agnostic. ✓ - Test suite: symlink-based guard test (
it.runIf(platform === 'linux')) covers all three cases (alias-free, casing-only, genuine alias); ordering assertions inno-ak-integration-ci.test.jspin redirect-before-Node and verify-before-install. - CI: Test (ubuntu-latest, Node 22.x) ✅ · Desktop Shell (ubuntu-22.04) ✅ · Desktop Shell (windows-2022) ✅ · web-shell E2E ✅ · CVE audit ✅ · Secret scan ✅.
Not reviewed: 8.3-alias behavior on a real Windows host — by design, not reproducible off Windows. Run 32955788152 is the production witness.
Reviewed with AI assistance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
For the hosted Windows test lane, points
TEMP/TMPat an alias-free directory next to the workspace before Node setup and dependency installation. Self-hosted Windows runners keep their configuredRUNNER_TEMP, which is already alias-free and shorter. A realpath guard runs on both lanes, accepts case-only path differences, and still rejects genuine alternate spellings such as 8.3 aliases.Why it's needed
GitHub-hosted Windows runners can expose the profile temp directory through the 8.3 alias
C:\Users\RUNNER~1whilefs.realpathreturnsC:\Users\runneradmin. Tests comparing path strings then see the same directory under two spellings and fail.The hosted-only condition is important: on the ECS Windows runner, replacing the 38-character
RUNNER_TEMPwith the longer workspace-adjacent path pushed the supervisor socket path from 83 to 102 bytes, crossing its production limit and causing a newqav-*fallback mismatch.Reviewer Test Plan
How to verify
TEMPandTMPuse the workspace-adjacent path.RUNNER_TEMPremains active, and the realpath guard passes.Evidence (Before & After)
In workflow run 32955788152, the self-hosted redirect was skipped and the realpath guard passed.
supervisor-process.test.tspassed all 86 tests; it was the only failed test file removed compared with the prior run. The remaining seven failed files are unchanged base-branch Windows standing failures and are outside this PR.Tested on
Risk & Scope
Linked Issues
N/A