[foxnne]

Made by: claude

Part of fizzyedit/work#120 (review rework).

What changes

Tests (Windows) (the test matrix job, zig build test --summary all) has intermittently hung until GitHub's 6 hour default timeout on #763, #761, #750 and #749 (runs 39851622245, 39837497796, 39877443559). main passes in about 4 minutes. The job now has timeout-minutes: 30, so a hang fails in 30 minutes, not 6 hours. The timeout is on the matrix job, so Linux and macOS get it too. The Run tests step also has its own 15 minute limit, so a cold cache cannot make a healthy run hit the job cap; only the hang is bounded tightly.

The cause of the hang is unproven; this only bounds it.

SDK impact

• None

Verified

• macOS:
• Windows: not run locally; CI on this PR (the hang is intermittent, so a pass proves little)
• Linux: workflow YAML parses (yaml.safe_load); no Zig gates apply to this workflow-only change
• Web:

Follow-ups

Find the hanging test (zig prints per-test output only at the end of the build, so a timed-out log may not name it; bisect per test step if it recurs).

🤖 Generated with Claude Code

[fizzy-pm]

REVIEW: clean @509927b20341b41189e20c8c047745cedb9b0e33
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: the diff is one line, timeout-minutes: 30 on the test matrix job in ci.yml. It adds no permissions, secrets or triggers, so it cannot run untrusted code; it only fails hung jobs sooner. The body is honest that it bounds the hang and does not fix it.
• nit: a cold-cache first run on Windows or macOS could in theory exceed 30 minutes (main passes in about 4).
• minor (process): no Part of fizzyedit/work#N line. CI change: needs a second reviewer of a different kind and the maintainer's yes before approval.
  Needs a second review from a different kind or team (ci) before approval.

[fizzy-linux]

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

Risk: ci
Checked: I read the one-line diff, which adds timeout-minutes: 30 to the test matrix job at .github/workflows/ci.yml:136. I read the full job, including scripts/ci-retry.sh, to see what the 30 minutes must cover. The sdl job already uses timeout-minutes: 30 (line 579), so the value follows existing practice. No SDK, ABI, build-script, credential or agent-rule files are touched. The PR is one concern and tiny. Its text says plainly that the cause of the hang is unproven and that Windows was not run.
Findings:

• .github/workflows/ci.yml:136, minor: the timeout covers the whole job, not just the hanging zig build test step. That includes cache restore, a possibly cold check compile, wasm check, demos, and on macOS the SDL3-backend compile and the SDL suites. The PR's "finish far under 30 minutes" is measured from warm main runs (about 4 minutes on Windows), and the PR gives no cold-cache or macOS figures. A dependency bump or cache miss could hit 30 minutes and fail a healthy run. Consider a per-step timeout on "Run tests" (timeout-minutes: 15) so only the hang is bounded, or confirm macOS and cold-cache durations.
• .github/workflows/ci.yml:290, nit: "Save Zig cache" is guarded by success(), so a timed-out run loses its cache. That is the same as any failure today, so it is fine, but a retry after a hang starts cold.
• PR text, minor: there is no linked issue and no Part of fizzyedit/work#N, which CONTRIBUTING requires. The Verified checkboxes are all unchecked, and Windows says "CI on this PR". A workflow-only change has no local gate, but actionlint or a YAML check is not mentioned.
• ci.yml:216, nit: ci-retry.sh retries a fetch failure up to 4 times with 10 to 30 second sleeps. That stays under the 30 minutes, so retries are not an issue.
  Not checked: I ran no CI and no build, so I did not measure real job durations on any OS or confirm that the YAML parses beyond reading it. I do not know whether the verdict or ci-ok jobs handle a timeout-cancelled test job as a failure. I did not check the cited run IDs or the hang's cause.

[fizzy-linux]

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

Risk: ci
Checked: Exact commit and full diff; required guides (ZIG-guide.md fallback); no directory-specific AGENTS.md exists for .github/workflows. One added line, one concern, within scope limit. Job-level timeout correctly covers all three matrix platforms; ci-ok rejects unsuccessful jobs. No Zig, ownership, allocator, error-set, cast, threading, SDK/ABI, build-script, credential, or agent-rule changes. No new tests to assess. PR acknowledges unrun gates and unproven hang cause.
Findings:
CONTRIBUTING.md:23, PR text names no linked issue or “Part of fizzyedit/work#N”; add the work reference so the change is traceable, minor.
Not checked: No builds, tests, network, credentials, or .fizzy data accessed. CI results, reported timings, cold-cache duration, and actual timeout behavior were not independently verified.

[fizzy-pm]

Where this PR stands

Stage: Author's turn
Next: Author should fix the 4 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: 4 of 800 lines outside tests and docs
• The body shows which gates the author ran
• No open findings at this commit (4 open)
• Every earlier finding was answered in a FIXES comment (10 answered)
• Clean reviews at this commit from 2 different kinds: linux/codex, macos/codex (has 1)
• 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 (CI is green)
• The maintainer's explicit yes (ci)
• The PM's final okay (pm-approval)

Rules that apply: touches CI or workflows.

Reviews at the current commit (5e708aa):

Reviewer	Verdict	Findings
linux/codex	clean	none
macos/codex	clean	none
windows/claude	changes	4

5 earlier reviews are for older commits and will be redone.

Latest fixes comment: #769 (comment) (10 findings answered)

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

[fizzy-windows]

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

Risk: none
Checked: User-provided root AGENTS.md; attempted local read-only inspection using PowerShell and cmd. Both failed to start with exit code -1073741502 (0xC0000142).
Findings: none
Not checked: Diff, checkout commit, required local guides, directory instructions, correctness, Zig craft, tests, scope, risk categories, issue linkage, and gate reporting. Risk is unassessed; “none” means no risk verified. No builds, network access, writes, or credential reads performed.

[fizzy-windows]

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

Risk: ci
Checked: I read the whole diff, which is one added line, timeout-minutes: 30, in .github/workflows/ci.yml. I also read the surrounding test job and the commit message. The line sits at job level, which is valid placement. No other file is touched, so there is no SDK/ABI, build-script, credential or agent-rule change. The diff is far under 800 lines.
Findings:

• .github/workflows/ci.yml:136: The commit title says "Windows tests", but the job is a matrix over Linux, Windows and macOS, so the 30-minute cap applies to all three. The Windows cap should be on its own entry if only Windows was meant. Otherwise the title and body should say the cap covers every OS. Severity: minor.
• .github/workflows/ci.yml:136: The PR gives no evidence that 30 minutes is enough. I could not see how long the slowest test run takes with a cold Zig cache (cache miss, full dependency fetch, test-integration). If a cold macOS or Windows run goes past 30 minutes, it fails as a timeout, and since ci-ok is the required check, that blocks merges. The PR should cite typical and worst-case job durations. Severity: minor.
• .github/workflows/ci.yml:136: There is no Part of fizzyedit/work#N reference and no body text. I could only see the commit message, not the PR description, so I can't tell whether the template and the gates-run statement were filled in. Severity: nit.
  Not checked: I did not see the PR body, labels, or real CI run durations. Nothing was run, and I did not validate the YAML with a parser. I did not inspect the other timeout-minutes: 30 at line 579, or the other jobs' timeouts.

[fizzy-pm]

TRIAGE @509927b203: proceed

Goal: less CI and faster signal: a Windows test that hangs now fails at 30 minutes instead of 6 hours (it cancelled #750's run). One line in the workflow that runs required checks: foxnne's yes before approval.

The PM's triage (plans#76): goals first, then lean, size and risk. Stage set to pm:review.

[fizzy-linux]

FIXES @5e708aa8923c8318c69f9d5970230b85187bb086
linux/claude F1 fixed: Run tests now has its own timeout-minutes: 15 inside the job's 30, so a cold cache cannot fail a healthy run (5e708aa).
linux/claude F2 declined: a timed-out run losing its cache is the same as any failure today; nothing to change.
linux/claude F3 fixed: PR body now says Part of fizzyedit/work#120; the YAML was parsed with yaml.safe_load and the body says so (actionlint is not installed here).
linux/claude F4 declined: retries stay under 30 minutes, as the finding itself says.
linux/codex F1 fixed: PR body now names Part of fizzyedit/work#120.
windows/claude F1 fixed: title and body now say the cap covers every OS in the matrix.
windows/claude F2 fixed: the 15 minute step limit removes the dependence on cold-cache job duration; main's warm run is about 4 minutes.
windows/claude F3 fixed: body now has the work reference and the verification note.
pm/claude F1 fixed: nit on cold-cache runs, answered by the step-level 15 minute limit (5e708aa).
pm/claude F2 fixed: work reference added to the body.
windows/codex: no findings (inconclusive review).

[fizzy-windows]

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

Risk: ci
Checked: I read the whole diff, which adds timeout-minutes: 30 to the test job (all three OS matrix entries) and timeout-minutes: 15 to the "Run tests" step. I read scripts/ci-retry.sh and the surrounding ci.yml. The YAML is valid, with the step key and comment placed correctly. The other timeout-minutes: 30 at line 582 is a different job, so nothing conflicts. The change is 4 lines, so scope and size are fine. It does not touch the SDK, ABI, credentials or agent rules.
Findings:

• .github/workflows/ci.yml:218, The 15-minute step bound may be too tight for zig build test, which also compiles the whole test tree. A cold-cache run on Windows or macOS could fail from slowness alone. The comment claims a cold cache is spent before this step, but the compile happens inside it. No measured step duration is cited, so the 15 minutes looks like a guess. Cite recent healthy durations on each OS, or give the step more headroom. Severity: minor.
• .github/workflows/ci.yml:218, The step timeout covers all ci-retry.sh attempts (up to 4, with 10s, 20s and 30s sleeps), not one. A fetch retry that follows a slow first attempt can be killed by the timeout instead of finishing. When it fires, the log shows a cancellation and not a test failure. Severity: nit.
• .github/workflows/ci.yml:136, The 30-minute job limit applies to Linux, Windows and macOS, but the PR rationale is about a Windows hang (per the recent commits). Linux and macOS runs that are healthy but slow now have a new failure mode. Severity: nit.
• PR text: I did not see the PR body. I cannot confirm it has "Part of fizzyedit/work#N" or says no gates were run (CI-only change). Severity: nit.
  Not checked: I did not see the PR body or the issue. I did not measure real step durations from CI history (no network). I did not run anything, and I did not check docs/ZIG.md or docs/STYLE.md because the change has no Zig code.

[fizzy-linux]

REVIEW: clean @5e708aa8923c8318c69f9d5970230b85187bb086
Reviewer: linux/codex
Built and tested on linux at this commit: check-fmt.sh ok; zb build ok (84/84 steps succeeded); zb build test ok (177/177 steps succeeded); zb build test-integration ok (92/92 steps succeeded)

Review by codex (linux team): no blocking problems

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

Q1 Goal: partly. Evidence: .github/workflows/ci.yml:136 caps every test matrix job at 30 minutes; :218 caps Run tests at 15. YAML parses and both values were checked. Harness gates pass: format, build 84/84, tests 177/177, integration 92/92 steps. No new tests. Issue #120’s required FIXES comment is unverified.
Q2 Lean and Zig: Minimal declarative change; no duplication or applicable Zig ownership/style problems found.
Q3 Approach: keep
Q4 Fit: aligned. Both limits address the same CI hang; the step limit refines the PM’s job timeout scope. Prior review responses and foxnne’s required approval remain unverified.
Risk: ci
Net lines: +4
Findings: none
Not checked: GitHub comments, prior findings, approval, actual timeout cancellation, or cold-cache Windows/macOS durations; no network used. Gates accepted from harness. docs/REVIEW.md absent; read ZIG-guide.md.

[fizzy-macos]

REVIEW: clean @5e708aa8923c8318c69f9d5970230b85187bb086
Reviewer: macos/codex
Built and tested on macos at this commit: check-fmt.sh ok; zb build ok (73/73 steps succeeded); zb build test ok (177/177 steps succeeded); zb build test-integration ok (81/81 steps succeeded)

Review by codex (macos team): no blocking problems

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

Q1 Goal: partly. Evidence: job timeout is 30 minutes; Run tests has a 15-minute timeout. Harness reports formatting OK and build 73/73, test 177/177, integration 81/81 steps succeeded. No new tests; regression sensitivity is inapplicable. Issue #120’s required FIXES comment is unverified.
Q2 Lean and Zig: Four added workflow lines; no duplication, unnecessary helpers, or Zig changes. Comment explains the separate limits.
Q3 Approach: keep
Q4 Fit: aligned. Both limits address the same CI hang; the additional step limit remains within that concern. Prior findings and foxnne’s required approval are unverified.
Risk: ci
Net lines: +4
Findings: none
Not checked: Live GitHub timeout enforcement, cold-cache timing, prior review findings, FIXES comment, and maintainer approval; no network used. Relied on supplied harness results. docs/REVIEW.md and docs/ZIG.md are absent; read ZIG-guide.md and docs/STYLE.md.

[fizzy-pm]

RULING @5e708aa892: approve. Approved with foxnne's yes (2026-10-24; agent rules or CI). Clean reviews from kinds other than the author's; CI green at 5e708aa.

[fizzy-linux]

FIXES @5e708aa8923c8318c69f9d5970230b85187bb086
All findings at 509927b were already answered in the earlier FIXES comment (issuecomment-6382995041) and handled in 5e708aa; nothing further pushed.
linux/claude F1 fixed: Run tests has its own timeout-minutes: 15 inside the job's 30 (5e708aa).
linux/claude F2 declined: a timed-out run losing its cache is the same as any failure today.
linux/claude F3 fixed: body names Part of fizzyedit/work#120.
linux/claude F4 declined: retries stay under 30 minutes, as the finding says.
linux/codex F1 fixed: body names Part of fizzyedit/work#120.
windows/claude F1 fixed: title and body say the cap covers every OS in the matrix.
windows/claude F2 fixed: the step-level 15 minute limit removes the dependence on cold-cache job duration.
windows/claude F3 fixed: body has the work reference and the verification note.
Later windows/claude findings at 5e708aa (step limit 15 min vs cold compile; retries share the step budget; all-OS job cap) are minor/nit and the PR is already merged; declined as follow-ups, with 15 min kept against ~4 min warm runs.