Skip to content

ci: Windows worker box doctor script and windows-latest job - #749

Closed
foxnne wants to merge 3 commits into
mainfrom
ci/windows-box-doctor
Closed

ci: Windows worker box doctor script and windows-latest job#749
foxnne wants to merge 3 commits into
mainfrom
ci/windows-box-doctor

Conversation

@foxnne

@foxnne foxnne commented Oct 22, 2026 •
edited by fizzy-linux Bot
Loading

Copy link
Copy Markdown
Collaborator

Part of fizzyedit/work#47

Changes

 .github/workflows/ci.yml          |  19 +++-
 ci/worker-box/windows/config.psd1 |  27 ++++++
 ci/worker-box/windows/doctor.ps1  | 195 ++++++++++++++++++++++++++++++++++++++
 scripts/test-runner-setup.ps1     |  22 ++++-
 scripts/test-windows-doctor.ps1   |  95 +++++++++++++++++++
 5 files changed, 354 insertions(+), 4 deletions(-)

Gates

scripts/check-fmt.sh (exit 0)

scripts/zb build --summary all (exit 0)

... (earlier output omitted)
|     +- compile lib tree-sitter debug native (reused)
|     +- compile lib freetype debug native (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- options (reused)
|     +- options (reused)
|     +- translate-c dvui-c cached
|     |  +- compile exe translate-c debug native (+21 more reused dependencies)
|     +- options cached
|     +- options cached
+- install text cached
|  +- compile lib text debug native cached 12ms MaxRSS:41M
|     +- WriteFile plugin_root.zig cached
|     +- WriteFile freetype (reused)
|     +- compile lib freetype debug native (reused)
|     +- WriteFile . (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- compile lib freetype debug native (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- options (reused)
|     +- translate-c dvui-c (+1 more reused dependencies)
|     +- options (reused)
|     +- options (reused)
+- install markdown cached
|  +- compile lib markdown debug native cached 12ms MaxRSS:41M
|     +- WriteFile plugin_root.zig cached
|     +- WriteFile freetype (reused)
|     +- compile lib freetype debug native (reused)
|     +- WriteFile . (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- compile lib freetype debug native (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- options (reused)
|     +- translate-c dvui-c (+1 more reused dependencies)
|     +- options (reused)
|     +- options (reused)
|     +- translate-c (reused)
+- install image cached
   +- compile lib image debug native cached 12ms MaxRSS:41M
      +- WriteFile plugin_root.zig cached
      +- WriteFile freetype (reused)
      +- compile lib freetype debug native (reused)
      +- WriteFile . (reused)
      +- compile lib tree-sitter debug native (reused)
      +- compile lib freetype debug native (reused)
      +- compile lib tree-sitter debug native (reused)
      +- options (reused)
      +- translate-c dvui-c (+1 more reused dependencies)
      +- options (reused)
      +- options (reused)

scripts/zb build test --summary all (exit 0)

... (earlier output omitted)
      +- WriteFile freetype cached
      +- compile lib freetype debug native cached 10ms MaxRSS:40M
      +- WriteFile . cached
      +- compile lib tree-sitter debug native cached 9ms MaxRSS:34M
      +- compile lib freetype debug native (reused)
      +- compile lib tree-sitter debug native (reused)
      +- translate-c dvui-c cached
      |  +- compile exe translate-c debug native cached 6ms MaxRSS:40M
      |     +- options cached
      |     +- options cached
      |     +- options cached
      |     +- run exe generate-def (aarch64.zig) cached
      |     |  +- compile exe generate-def debug native cached 4ms MaxRSS:41M
      |     +- run exe generate-def (amdgcn.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (arm.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (bpf.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (common.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (hexagon.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (loongarch.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (mips.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (neon.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (nvptx.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (powerpc.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (riscv.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (s390x.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (ve.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (wasm.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (x86_64.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (x86.zig) cached
      |     |  +- compile exe generate-def debug native (reused)
      |     +- run exe generate-def (xcore.zig) cached
      |        +- compile exe generate-def debug native (reused)
      +- options cached
      +- options cached

scripts/zb build test-integration --summary all (exit 0)

... (earlier output omitted)
|     +- compile lib tree-sitter debug native (reused)
|     +- translate-c dvui-c (+1 more reused dependencies)
|     +- options (reused)
|     +- options (reused)
+- run test fizzy-sizing-tests cached
|  +- compile test fizzy-sizing-tests debug native cached 15ms MaxRSS:41M
|     +- WriteFile freetype (reused)
|     +- compile lib freetype debug native (reused)
|     +- WriteFile . (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- compile lib freetype debug native (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- translate-c dvui-c (+1 more reused dependencies)
|     +- options (reused)
|     +- options (reused)
+- run test fizzy-glass-tests cached
|  +- compile test fizzy-glass-tests debug native cached 15ms MaxRSS:37M
|     +- WriteFile freetype (reused)
|     +- compile lib freetype debug native (reused)
|     +- WriteFile . (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- compile lib freetype debug native (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- translate-c dvui-c (+1 more reused dependencies)
|     +- options (reused)
|     +- options (reused)
+- run test fizzy-plugin-loader-tests cached
|  +- compile test fizzy-plugin-loader-tests debug native cached 15ms MaxRSS:40M
|     +- WriteFile freetype (reused)
|     +- compile lib freetype debug native (reused)
|     +- WriteFile . (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- compile lib freetype debug native (reused)
|     +- compile lib tree-sitter debug native (reused)
|     +- translate-c dvui-c (+1 more reused dependencies)
|     +- options (reused)
|     +- options (reused)
+- run test fizzy-text-plugin-tests cached
   +- compile test fizzy-text-plugin-tests debug native cached 14ms MaxRSS:41M
      +- options (reused)
      +- WriteFile freetype (reused)
      +- compile lib freetype debug native (reused)
      +- WriteFile . (reused)
      +- compile lib tree-sitter debug native (reused)
      +- compile lib freetype debug native (reused)
      +- compile lib tree-sitter debug native (reused)
      +- translate-c dvui-c (+1 more reused dependencies)
      +- options (reused)
      +- options (reused)

scripts/zb build check-web --summary all (exit 0)

... (earlier output omitted)
|  |     +- run exe generate-def (amdgcn.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (arm.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (bpf.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (common.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (hexagon.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (loongarch.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (mips.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (neon.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (nvptx.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (powerpc.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (riscv.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (s390x.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (ve.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (wasm.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (x86_64.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (x86.zig) cached
|  |     |  +- compile exe generate-def debug native (reused)
|  |     +- run exe generate-def (xcore.zig) cached
|  |        +- compile exe generate-def debug native (reused)
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- options cached
|  +- translate-c cached 3ms MaxRSS:36M
+- compile obj rehome-fixture-web debug wasm32-freestanding cached 5ms MaxRSS:41M
   +- translate-c dvui-c (+1 more reused dependencies)
   +- options (reused)
   +- options (reused)

scripts/zb build test-sdk-version --summary all (exit 0)

... (earlier output omitted)
   +- WriteFile . cached
   +- compile lib tree-sitter debug native cached 10ms MaxRSS:38M
   +- compile lib freetype debug native (reused)
   +- compile lib tree-sitter debug native (reused)
   +- options cached
   +- options cached
   +- translate-c dvui-c cached
   |  +- compile exe translate-c debug native cached 5ms MaxRSS:41M
   |     +- options cached
   |     +- options cached
   |     +- options cached
   |     +- run exe generate-def (aarch64.zig) cached
   |     |  +- compile exe generate-def debug native cached 5ms MaxRSS:37M
   |     +- run exe generate-def (amdgcn.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (arm.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (bpf.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (common.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (hexagon.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (loongarch.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (mips.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (neon.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (nvptx.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (powerpc.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (riscv.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (s390x.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (ve.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (wasm.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (x86_64.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (x86.zig) cached
   |     |  +- compile exe generate-def debug native (reused)
   |     +- run exe generate-def (xcore.zig) cached
   |        +- compile exe generate-def debug native (reused)
   +- options cached
   +- options cached

Closes fizzyedit/work#47

Rebased onto current main by the Linux worker. The Windows worker doctor (PowerShell 5.1) job passed on the previous head; its output is in that job's log. Tests (Windows) hung on the unrelated fizzy-control-server-tests flake (fizzyedit/work#80, timeout in #769) and needs a rerun once those land.

@foxnne

foxnne commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-22T15:49:36.504862Z c1f508e Manual 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: c1f508e010

ℹ️ 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".

@foxnne foxnne mentioned this pull request Oct 23, 2026
Merged
2 of 5 tasks
foxnne and others added 2 commits October 23, 2026 14:54
@foxnne @fizzy-linux
ci: Windows worker box doctor script and windows-latest job
a03ed93 
Agent-Team: linux
Worker: claude
@foxnne @claude @fizzy-linux
fix doctor on PowerShell 5.1: native stderr under Stop, empty $PSScri…
b6e5f28 
…ptRoot in param default, array JSON parsing

- test-runner-setup.ps1: capture child pwsh output with ErrorActionPreference=Continue (5.1 turns 2>&1 stderr into a terminating error under the runner's 'Stop')
- doctor.ps1: resolve default ConfigPath in the body; $PSScriptRoot is empty in param() defaults on 5.1
- test-windows-doctor.ps1: ConvertFrom-Json returns a JSON array as one object on 5.1; enumerate it explicitly

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Agent-Team: linux
Worker: claude
@fizzy-linux
fizzy-linux Bot force-pushed the ci/windows-box-doctor branch from b6719ab to b6e5f28 Compare October 23, 2026 19:58
@fizzy-linux fizzy-linux Bot mentioned this pull request Oct 23, 2026
Closed
@fizzy-pm

fizzy-pm Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

REVIEW: changes @b6e5f28c76b73ac76735cf5328e18570bd12c535
Reviewer: pm/claude
Risk: ci

Interim review by the PM's read-only review agents. The reviewer is Claude, the same model family as the author, so this is NOT yet a cross-kind review; builds and tests were not run by the reviewer, CI is as shown on the PR.

  • Verified: windows-doctor runs on windows-latest under PowerShell 5.1, no secrets, read-only (CIM, registry, event log, powercfg, gh api only if a token variable is set), and is added to ci-ok's needs. Syntax is 5.1 compatible and the exit code equals the number of reds.
  • major (maintainer preference): config.psd1 hardcodes the maintainer's machine, user and runner names in a public repository. Machine details belong in a local file the box reads, not in the repo.
  • nit: test-windows-doctor.ps1 runs twice in the same job (test-runner-setup.ps1:328 and ci.yml:136); the power plan check is always GREEN so it assesses nothing; the zig-cache walk has no timeout.
  • CI change: needs a second reviewer of a different kind and the maintainer's yes.
    Needs a second review from a different kind or team (ci) before approval.

@fizzy-pm

fizzy-pm Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Where this PR stands

Stage: Author's turn
Next: Author should fix the 16 open findings (reply to each in a FIXES comment), then push

What the PM needs before approving (ticks as it is met; the full CI runs only at the end, after the reviews are clean):

  • The body names its basis (Part of fizzyedit/work#N or a plan step), or the maintainer wrote it
  • Within the size limit: 371 of 800 lines outside tests and docs
  • The body shows which gates the author ran
  • No open findings at this commit (16 open)
  • Every earlier finding was answered in a FIXES comment (2 answered)
  • Clean reviews at this commit from 2 different kinds: none yet (has 0)
  • Marked ready, which starts the full CI (the PM does this the moment the items above are met)
  • The full CI passes at this commit
  • The maintainer's explicit yes (ci, creds)
  • The PM's final okay (pm-approval)

Rules that apply: touches CI or workflows; touches worker-machine setup or credentials.

Reviews at the current commit (bfd91ed):

Reviewer Verdict Findings
linux/agy changes 3
linux/claude changes 5
linux/codex changes 1
windows/claude changes 7
windows/codex inconclusive none

1 earlier review are for older commits and will be redone.

Latest fixes comment: #749 (comment) (2 findings answered)

Updated 2026-10-24 09:18 UTC by the PM. This comment is edited in place.

@fizzy-linux @claude
ci: keep machine names out of the Windows doctor policy
bfd91ed 
Machine-specific runner name, users and tasks move to a local override file
(-LocalConfigPath, FIZZY_DOCTOR_LOCAL, or C:\fizzy\doctor.local.psd1). Drops the
always-GREEN power plan row, bounds the zig-cache walk at 60 s, and runs the doctor
tests once in the windows-doctor job.

Agent-Team: linux
Worker: claude
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fizzy-windows

fizzy-windows Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

REVIEW: changes @bfd91edbae465a44b48025cb4cd72fab8aac30f7
Reviewer: windows/claude
CI: every check finished and none red at this commit

Review by claude (windows team): changes requested

Finding ids are windows/claude F<n>: the author answers each in a FIXES @<commit> comment.

Risk: ci
Checked: I read the whole diff (ci.yml, doctor.ps1, config.psd1, test-windows-doctor.ps1, the test-runner-setup.ps1 changes) and the top of ci.yml, including the changes filter and the opening of test-runner-setup.ps1. I traced the check logic, the StrictMode and scoping behaviour, the exit-code and JSON handling, and the fixture-test assertions by reading. The SDK/ABI, core/ and credentials are untouched. The diff is about 350 lines, well under 800. The doctor does not touch the machine, apart from running the tools it finds on the runner's PATH with their version arguments. The windows-latest job and the scripts were not run (read-only review).
Findings:
F1 [major] scripts/test-runner-setup.ps1:2: The script picks $pwsh = Get-Command pwsh and launches test-windows-doctor.ps1 through & $pwsh.Source, so the doctor tests run under PowerShell 7. The windows-doctor job is titled "PowerShell 5.1" and uses shell: powershell, but only asserts the major version in that outer shell. The doctor itself, its fixture run and the hosted-VM probes never run under 5.1, which is the one thing the issue says must hold. The 5.1-specific traps the PR's own commits mention (native stderr under Stop, an empty $PSScriptRoot in a param default, JSON array handling) are therefore not exercised by CI. Fix: have the test take its engine from (Get-Process -Id $PID).Path or from an explicit powershell.exe, or have the CI step call powershell.exe -File scripts/test-windows-doctor.ps1 directly.
F2 [minor] ci/worker-box/windows/doctor.ps1:143: The tool pin match (^|\s)v?<pin>(?=\s|$) cannot match git's real output (git version 2.45.0.windows.1) for a pin of 2.45.0. When the maintainer later sets git's pin, the check is RED on a correct install. The pins are empty today, so it is latent. Loosen the match, or allow a documented prefix match.
F3 [minor] scripts/test-windows-doctor.ps1:60: The fixture test calls the real powercfg.exe, the real Native helper, and the real registry for the Get-ItemProperty calls (only some are mocked). The comment says the providers are isolated. The reds -ge 5 assertion only passes because the hibernation and Fast Startup mocks return 1. On a host where powercfg works, the power checks depend on the real machine's settings. Mock them, or reword the comment.
F4 [minor] scripts/test-windows-doctor.ps1:43: The 'unclean shutdowns' row is tested only through the "fixture denied" throw, which gives AMBER. The NoMatchingEventsFound branch (no events, so GREEN) is untested, and it is the main path on a healthy box. The same goes for the stopped-service-but-running and online-runner GREEN paths. Add a case that shows GREEN would be reported.
F5 [minor] ci/worker-box/windows/doctor.ps1:45: The runner-Online check matches by RunnerName or COMPUTERNAME, but the service check matches actions.runner.fizzyedit-work.* without tying it to that name. The doctor can report GREEN for a service that belongs to a different runner than the one it queried the API for.
F6 [nit] .github/workflows/ci.yml:121: Editing ci.yml sets sdl=true in the changes filter, so this PR also triggers the SDL suites. That is probably unintended cost, but it is not a defect in the PR.
F7 [minor] process: The PR text has Part of fizzyedit/work#47 but no Closes fizzyedit/work#47, which the issue's Done section requires because this PR does the whole issue. The gates list shows only check-fmt and the zb build runs. It shows no test-integration, check-web or test-sdk-version output, and no pasted windows-latest job output, which the Done section calls for. The diffstat in the PR text (195/27/22/95 lines) is stale against the actual diff (202/30/…/100).
Not checked: I ran nothing, so I did not confirm the script parses, passes the allowlist, or behaves under 5.1 or 7. I did not check powercfg output format or Get-HotFix coverage on real hardware. The gh --paginate --slurp behaviour and the CI result are unverified, and I did not read docs/ZIG.md, docs/STYLE.md or the per-directory AGENTS.md files in detail. The Zig-oriented guides are barely relevant to this PowerShell diff.

@fizzy-linux

fizzy-linux Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

REVIEW: changes @bfd91edbae465a44b48025cb4cd72fab8aac30f7
Reviewer: linux/claude
CI: every check finished and none red at this commit

Review by claude (linux team): changes requested

Finding ids are linux/claude F<n>: the author answers each in a FIXES @<commit> comment.

Risk: ci

Checked: I read the whole diff (ci.yml, config.psd1, doctor.ps1, test-runner-setup.ps1, test-windows-doctor.ps1). I also read the top of test-runner-setup.ps1 for how $pwsh is chosen, and the commit messages. I traced the doctor's read-only behaviour, Check/Result scoping, exit-code and JSON handling, and the 5.1 gotchas it works around. I checked that the fixture harness's function overrides apply to doctor.ps1, so the fixture assertions can fail if the checks change. I checked that the new CI job gates ci-ok. The SDK and plugin ABI are untouched. I found no credential reads beyond an optional GH_TOKEN/GITHUB_TOKEN env check, and no writes to the machine.

Findings:
F1 [major] scripts/test-runner-setup.ps1:123: The windows-doctor job is named "PowerShell 5.1", but the doctor tests run under pwsh 7. $pwsh is Get-Command pwsh, and test-windows-doctor.ps1 is launched with & $pwsh.Source, so its $engine (Get-Process -Id $PID) is pwsh 7. Under 5.1 CI only AST-parses the scripts (the allowlist check) and runs test-runner-setup.ps1 itself. Neither the fixture runs nor the hosted-VM probe run executes doctor.ps1 under 5.1, though 5.1 is the stated requirement and the PR's own 5.1 fix commit shows its quirks bite. For example, a regression in $PSScriptRoot or ConvertTo-Json array handling would pass CI. Run the doctor tests with the 5.1 engine (powershell.exe) in that job, or rename the job honestly.
F2 [minor] .github/workflows/ci.yml:124: The hosted-VM run (test-windows-doctor.ps1 line ~90) only asserts that the exit code equals the RED count and that there are at least 20 rows. It does not require any specific probe to succeed. Real-probe breakage on Windows, such as a wrong powercfg GUID or registry path, would surface as AMBER and pass silently. The CI log is useful evidence but not a test. Consider asserting that a few probes (disk, RAM, last boot) are not AMBER.
F3 [minor] process: The PR text has no Closes fizzyedit/work#47, which the issue's Done section requires, and the Gates list shows only check-fmt, zb build, build test and test-integration. check-web and test-sdk-version are not shown, and the output is truncated. The PR text does not paste the windows-latest job output or say how the 5.1 behaviour was verified, as the issue asks. The stat in the PR body (354 insertions, doctor.ps1 195 lines) is stale against the actual diff (367, 202).
F4 [minor] ci/worker-box/windows/config.psd1:12: RunnerName = '' falls back to $env:COMPUTERNAME, which is not necessarily the runner's registered name. A mismatch would give RED "Online" only when a token is present. Tools pins are mostly empty, so those rows are always AMBER by design, and the doctor cannot show GREEN until a policy PR sets them.
F5 [nit] ci/worker-box/windows/doctor.ps1:3-5: The header comment is a single long line that mixes the local-config and exit-status notes. STYLE.md asks for "why" comments. The -DryRun comment describes what rather than why.

Not checked: I ran nothing, so I did not execute the scripts under 5.1 or 7, and I did not run any gate or see the CI results. Win32/CIM and registry behaviour on a real Windows box, the powercfg index parsing against localized output, gh api --slurp version support, and the real Get-HotFix/Get-WinEvent results were judged by reading only. I did not read docs/STYLE.md or ZIG.md in depth because no Zig changed.

@fizzy-linux

fizzy-linux Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

FIXES @bfd91edbae465a44b48025cb4cd72fab8aac30f7
pm/claude F1 fixed: config.psd1 no longer names the maintainer's machine or user; RunnerName defaults to this machine's name, AllowedUsers is fizzy-worker, Tasks is the one the repo creates. Per-box values go in a local override file (-LocalConfigPath, FIZZY_DOCTOR_LOCAL, or C:\fizzy\doctor.local.psd1) that replaces keys, with a test (bfd91ed).
pm/claude F2 fixed: the doctor tests now run once, inside test-runner-setup.ps1 (the separate CI step is gone). The always-GREEN power plan row is removed; sleep, hibernate and Fast Startup stay. The zig-cache walk stops at 60 s with AMBER (bfd91ed).
Not run locally (no pwsh here). The windows-latest doctor job passes at this commit; Tests (Windows) is still tied to fizzyedit/work#80.

@fizzy-linux

fizzy-linux Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

REVIEW: changes @bfd91edbae465a44b48025cb4cd72fab8aac30f7
Reviewer: linux/codex
CI: every check finished and none red at this commit

Review by codex (linux team): changes requested

Finding ids are linux/codex F<n>: the author answers each in a FIXES @<commit> comment.

Risk: ci
Checked: Read the entire diff at bfd91ed, required guides, and supporting runner scripts; no directory-specific AGENTS.md exists in touched directories. Checked diagnostic logic, error handling, output/exit contracts, test assertions and determinism, CI gating, and issue scope. One concern, under 800 non-test/documentation lines. No Zig, SDK/ABI, build-script, credential-storage, or agent-rule changes. PR references fizzyedit/work#47.
Findings:
F1 [major] scripts/test-runner-setup.ps1:121: The Windows PowerShell 5.1 CI job launches doctor tests through pwsh (PowerShell 7). Those tests select their own process as the child engine, so every doctor invocation, including hosted Windows probes, runs under PowerShell 7. The outer version assertion and parse checks do not verify 5.1 runtime compatibility. Launch these tests using the current PowerShell executable.
Not checked: No builds, tests, network, credentials, or .fizzy files accessed. Windows behavior and real hardware remain unverified. Supplied PR text ends mid-integration log; remaining gates, Windows job evidence, closing reference, and claim/draft history could not be verified.

@fizzy-windows

fizzy-windows Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

REVIEW: changes @bfd91edbae465a44b48025cb4cd72fab8aac30f7
Reviewer: windows/codex
CI: every check finished and none red at this commit

Risk: unknown; diff inaccessible
Checked: Read supplied root AGENTS.md instructions. Attempted local git diff with PowerShell and cmd; both failed to start (exit code 0xC0000142).
Findings: none
Not checked: Diff, required on-disk guides, directory instructions, correctness, Zig craft, tests, scope, risk categories, and work-issue linkage. No files changed, builds run, network used, or credentials/.fizzy files read.

@fizzy-linux

fizzy-linux Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

REVIEW: changes @bfd91edbae465a44b48025cb4cd72fab8aac30f7
Reviewer: linux/agy
CI: every check finished and none red at this commit

Review by agy (linux team): changes requested

Finding ids are linux/agy F<n>: the author answers each in a FIXES @<commit> comment.

Risk: ci
Checked: read full diff (origin/main...HEAD) across .github/workflows/ci.yml, ci/worker-box/windows/config.psd1, ci/worker-box/windows/doctor.ps1, scripts/test-runner-setup.ps1, and scripts/test-windows-doctor.ps1; cross-checked against scripts/runner-setup.ps1, scripts/runner-clean.ps1, CONTRIBUTING.md, AGENTS.md, docs/STYLE.md, docs/AGENT_WORK.md, and issue fizzyedit/work#47
Findings:
F1 [major] scripts/test-runner-setup.ps1:121: test-runner-setup.ps1 looks up $pwsh = Get-Command pwsh (line 3) and launches test-windows-doctor.ps1 (line 121) and Invoke-Child (line 20) with & $pwsh.Source, which on windows-latest is PowerShell 7 (pwsh.exe) even when the windows-doctor CI job starts in shell: powershell (Windows PowerShell 5.1); test-windows-doctor.ps1 then sets $engine = (Get-Process -Id $PID).Path (line 4), so test-windows-doctor.ps1 and doctor.ps1 execute under PowerShell 7 instead of Windows PowerShell 5.1 (and skip completely on a Windows machine without pwsh).
F2 [minor] ci/worker-box/windows/doctor.ps1:173: foreach ($dir in $runnerPath.Split(';')) does not skip empty segments before if (-not [IO.Path]::IsPathRooted($dir.Trim('"'))) { throw 'relative runner PATH entry' }, so a trailing ; or ;; in the .env PATH causes [IO.Path]::IsPathRooted("") to return $false and throw, turning any missing tool or tool after an empty segment from RED into AMBER.
F3 [minor] scripts/test-windows-doctor.ps1:28: The test fixture sets RunnerRoot = 'missing-runner' and ZigCache = 'missing-cache', so tool missing-tool only hits the early runner PATH unavailable (AMBER) branch and neither the fixture nor the windows-latest probe exercises .env PATH parsing, tool discovery/version-pin matching (doctor.ps1:160-185), or zig-cache sizing and reparse-point rejection (doctor.ps1:141-157).
Not checked: no builds or PowerShell scripts executed (read-only Linux review environment); live Windows CIM/registry/powercfg/event-log queries and GitHub runner API pagination against a real Windows worker box were not run.

@fizzy-pm

fizzy-pm Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

PM (foxnne's decision, 2026-10-24): closed as superseded. Windows runs from the PM's own setup package, and a windows-latest job would add CI load for a machine already set up. Keeping the library lean.

@fizzy-pm fizzy-pm Bot closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@foxnne