[plonkus]

While a Mac's screen is locked or its display is off, AppKit runs no animation to its end. MainWindowTests.openingTheInspectorAtTheNarrowestGrowsTheWindow opens the inspector through the animated toggle, so on such a Mac the inspector stays 0 wide, the window never grows, and the test fails after 60 s, looking like a code failure.

Evidence:

• On a locked Mac with its display off, a plain NSAnimationContext animation never completes. A bare NSSplitViewController doesn't grow the window when the inspector opens through the animated toggle, but grows it as expected when the inspector is opened without animation.
• The test failed on this Mac at the commit that added it, and passed in CI on macOS 26.

Now scripts/test.py refuses the installer suite, which holds the Swift app tests, on such a Mac before anything runs, and says why:

test.py: installer: the Mac app's Swift tests need the screen unlocked and the display on so AppKit animations can finish; this Mac's screen is locked and display is off: wake and unlock it, then run again

• The state is screen is locked, display is off, or both: the lock state from ioreg, the display state from CoreGraphics' CGDisplayIsAsleep.
• macOS only. Other suites, Linux, --linux, and build-only runs are unaffected.
• scripts/test_test.py: the plan helper assumes an awake screen, as it already assumes every tool is installed. A new test covers the refusal for the gate and for installer alone, with core browser and Linux's installer still planned. 18/18 pass.

A mac-tests run on this branch checks that CI's macOS runners pass the check.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RFixJSUTTb3WjpFumMdmNi

[srosro]

📋 First review of this PR — reviewed 61dedbf. 🧪 Tests not run. ✅ Prior-art (KID) checked. ⚡ Small-diff lane (44 lines): skipped architecture-refined, consumers — screened by aggregator.

It appears @plonkus is working towards giving developers a clear, actionable explanation instead of misleading Mac app test failures when the screen is locked or the display is off by adding a preflight refusal for the macOS installer suite in scripts/test.py, with instructions to wake and unlock the Mac before retrying — reviewing against that goal.

Overview — The PR replaces misleading AppKit animation failures with an actionable preflight refusal. The implementation fits the existing prerequisite checks, but it also refuses a shard that never runs the Swift app tests.

Strengths — The refusal explains both the cause and recovery action without weakening assertions or skipping tests. The check respects Linux, build-only runs, and --only filtering, and the regression test follows the existing planner-test convention.

Probes

1. [medium] [from: data-integrity] [bug] On a locked or sleeping Mac, just test --shard 2/2 --ci now aborts with the Swift-test screen requirement even though that shard runs no Swift app tests. plan() checks screen state before parsing the shard, while steps() schedules the installer and Swift app checks only for unsharded runs or shard 1. The existing macOS workflow invokes both shards, so its second shard unnecessarily inherits the screen prerequisite. Files: scripts/test.py:223, scripts/test.py:280, scripts/test.py:330, .github/workflows/mac-tests.yml:80. Edit: In scripts/test.py, move the screen check after shard parsing and restrict it to unsharded runs or shard 1, matching steps(); approximately +1 LOC.

Security — None.

Test coverage — The added planner test exercises refusal and unaffected suite selection; staged just test stopped before running tests because cargo-nextest was missing (exit 127), an environment prerequisite failure, so the gate remains unverified.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo's operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is "no, this complexity isn't needed" should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[plonkus]

1. Fixed in d0e3cf5: the screen check now runs after the shard is parsed, and only for an unsharded run or shard 1, the same runs steps() gives the installer suite. just test-plan --shard 2/2 now plans on a locked Mac, and --shard 1/2 still refuses. test_test.py covers shard 2/2 planning installer on a locked Mac.

[plonkus]

/srosro-update-review

[srosro]

📋 Re-review of changes from 61dedbf to d0e3cf5 (git diff 61dedbf..d0e3cf5). 🧪 Tests not run. ✅ Prior-art (KID) checked. ⚡ Small-diff lane (20 lines): skipped architecture-refined, consumers — screened by aggregator.

It appears @plonkus is working towards preventing misleading Mac app test failures when the screen is locked or the display is off, while keeping unaffected test runs available, by limiting the screen-state refusal in scripts/test.py to unsharded runs or the first shard that actually runs the installer suite — reviewing against that goal.

Overview — The revision resolves the prior shard-selection bug: screen readiness is checked after shard parsing and only for runs that schedule the installer suite. Unsharded runs and shard 1 retain the actionable refusal; shard 2 remains available on a locked or sleeping Mac.

Strengths — The correction stays within the existing planner and extends the existing regression test. The shard-2 case keeps the unusable-screen mock active, directly exercising the previously failing path without weakening the refusal assertions.

Probes

None.

Probe resolved: the unconditional screen check is gone. scripts/test.py:245–247 now matches installer scheduling at scripts/test.py:332; the regression case at scripts/test_test.py:204 covers shard 2 with an unusable screen. No additional cross-angle findings survived consolidation.

Security — None.

Test coverage — Planner coverage includes refusal, unaffected suites, Linux, and shard 2; staged just test stopped because cargo-nextest was missing (exit 127), a reviewer-environment prerequisite failure, so the gate remains unverified.

For AI authors — (Codex, Claude Code, Cursor, etc. reading this PR): The Probes above are load-bearing. Treat each [open] probe as a hard requirement — answer it directly in your reply or revisit the structural decision. Do NOT silently absorb a probe by adding more code; that path inverts the cost stance the probe is encoding. This repo's operating point (see its REVIEW.md) prefers cutting LOC over adding it; an [open] [simplification] probe whose answer is "no, this complexity isn't needed" should land as a deletion in your next push, not a new defensive guard.

---

How to use: auto-reviews every new PR and re-reviews after a period of inactivity. Trigger an incremental re-review with /srosro-update-review, or a whole-PR re-review with /srosro-review.

For humans only: push-access collaborators can post:

• /srosro-approve — APPROVE the PR.
• /srosro-props [from: <specialist>] — give props to a specialist's contribution.
• /srosro-critique [from: <specialist>] — flag a specialist's contribution as a misread.
• /srosro-memorize <feedback> — teach a calibration lesson (learn-from-replies updates COMMENT_REVIEW_MISTAKES.md from your body, sentiment-aware via LLM).

Props: /srosro-props [from: shape] caught a real layering bug we'd have shipped.
Critique: /srosro-critique [from: architecture-refined] DRY suggestion misread distinct seams.
Calibration: /srosro-memorize the architecture-refined DRY finding was a misread; those helpers serve different contracts.

AI agents must not use /srosro-memorize, /srosro-props, or /srosro-critique — those signals tune shared global state.

Generated by sam's ai review bot.

[srosro]

Approving per automated review above.