fix(tests): name the Windows home and wait on pipes without select (#692, classes 6-7) - #913

Claude finished @cdeust's task in 1m 39s —— View job

PR #913 review (1 of 3)

• Read the PR diff and the code it touches
• Ran five reviews: code quality, test coverage, security, performance, documentation accuracy
• Checked the reviewers' findings against the code
• Posted the review as a PR comment: fix(tests): name the Windows home and wait on pipes without select (#692, classes 6-7) #913 (comment)

Result: no Blockers, 3 Notes, Verdict: approve.

The Note that matters most: the consolidate-background test copies the real HOME/USERPROFILE. The hook then writes the developer's real ~/.claude/methodology/.last_consolidate, because that path doesn't use CORTEX_CLAUDE_DIR. This already happens on POSIX; the PR now does the same on Windows.

I did not run the tests here; the review comes from reading the code.

Review 1 of 3: PR #913 (#692 classes 6-7)

Five reviews ran: code quality, test coverage, security, performance and documentation accuracy. None found a Blocker. I checked the security reviewer's one finding against the code myself and rated it a Note below.

What I traced:

• _bounded (tests_py/infrastructure/test_shared_host_store.py:29-50): each operation still has its own PROTOCOL_TIMEOUT and still fails with AssertionError. Errors from the worker are re-raised in the caller. With bufsize=0, readline on the raw pipe stops at the newline or at EOF. After a timeout, the stuck daemon thread is released when _stop waits for or kills the worker, which ends the pipe.
• The skip on is_supported() in test_capture_worker_logging.py still runs the class on Linux and macOS.
• The USERPROFILE additions only add environment variables and change nothing on POSIX. In test_response_budget_measurements.py, the sandbox home root/home is not root, so validate_environment still accepts the root.

Notes, no action before merge

• test_consolidate_background_wiki_composition_root.py:32 copies the real HOME/USERPROFILE, and consolidate_background.py:37,145 writes Path.home()/.claude/methodology/.last_consolidate without reading CORTEX_CLAUDE_DIR. A green run therefore overwrites the developer's real consolidation stamp. This already happens on POSIX; the PR now does the same on Windows. Pointing both variables at the sandbox, as _hermetic_hook_env.py does, would fix it.
• test_response_budget_measurements.py:105: "Path.home() needs HOME on POSIX" overstates it: without HOME, posixpath.expanduser falls back to pwd. "Reads HOME" would be accurate.
• _concurrent_writes waits at most 30 s on future.result(timeout=PROTOCOL_TIMEOUT), but _exchange can take up to 60 s (send plus read). A stalled worker therefore shows up as a bare TimeoutError rather than the "timed out" assertion message.

Verdict: approve

merge is red because environment (macos-latest) was cancelled (run 42243547825, job 127987153121). The job ran from 15:43:25 to 15:58:27 and never got a runner: runner_name is empty, no step started, and the log does not exist (HTTP 404 BlobNotFound). The gate then read environment: abandoned and install: skipped.

This is not caused by the PR. The diff touches neither .github/, pyproject.toml nor uv.lock. Every test job is green on b394376: contract, unit, regression, e2e, plus environment on ubuntu and windows.

I re-ran the failed jobs once (rerun-failed-jobs). If macOS stays without a runner, the problem is runner capacity, not this code.

Generated by Claude Code

CI autopilot : branche en retard de 18 commits sur main (52697b2e), rebase non poussé.

J'ai rebasé la branche sur origin/main avec autosquash, sans conflit de fond (pour #878 : conflits mécaniques du CHANGELOG, les deux entrées gardées, contenu final identique à la branche). Les tests des fichiers touchés et ruff passent sur le résultat. Mais le push --force-with-lease=<branche>:<sha distant> est refusé par la politique de permissions de cette session de routine, et je ne le contourne pas.

Il faut qu'un humain rebase la branche sur main (ou autorise le push avec lease pour cette routine).

Generated by Claude Code

Claude finished @cdeust's task in 49s —— View job

PR #913 review (2 of 3)

• Checked the earlier Blockers: review 1 had none
• Found the commits pushed after b394376: the old commit was rebased onto main as c7e57d3, and no new commit was added
• Reviewed only that delta
• Posted the result

What changed since review 1. The PR is still one commit. c7e57d3 sits on 52697b2 (main). The diff is still +54/−19 across the same 7 files. Each hunk matches the code review 1 traced, down to the line numbers: _bounded in test_shared_host_store.py:29-50, the is_supported() skip, and the four USERPROFILE additions. The only place the rebase could have gone wrong is the CHANGELOG.md conflict. The entry sits once, under ## [Unreleased] → ### Fixed (around line 182), and the neighbouring entries are intact. Because the code itself did not change, I did not audit it again or re-run the five reviewers, as the review contract asks.

Not verified here:

• I could not compare the old and new commits directly. Fetching b394376 and calling gh api both need approval in this session, so I checked the new commit against the hunks review 1 quoted.
• I could not run the tests locally (pytest also needs approval). On c7e57d3, the PR #913 check is green. Pipeline was still running when I posted.

Notes, no action before merge (carried over from review 1, still open)

• test_consolidate_background_wiki_composition_root.py:32 passes the real HOME/USERPROFILE to the hook. A green run therefore still writes the developer's real ~/.claude/methodology/.last_consolidate (consolidate_background.py:37,145). With this PR that now also happens on Windows. Pointing both variables at the sandbox would fix it.
• test_response_budget_measurements.py:105: the comment "Path.home() needs HOME on POSIX" is too strong. Without HOME, posixpath.expanduser falls back to pwd. "Reads HOME" would be accurate.
• _concurrent_writes waits at most 30 s on future.result, but _exchange can take up to 60 s (send plus read). A stuck worker therefore shows up as a bare TimeoutError instead of the "timed out" assertion message.

Verdict: approve