Conversation
install.ps1 finds the latest release by requesting the release page without following its redirect and reading the Location header off the exception that raises. Only PowerShell 7 raises there. Windows PowerShell 5.1, the shell every Windows ships with, returns the unfollowed redirect as the response and reports the exceeded redirect count as an error with no response attached, so the lookup always came back empty and every one-line install from the stock shell stopped at "Could not resolve the latest Raven release wheel from GitHub". The request now ignores that error and reads the Location header from the response; PowerShell 7 still raises and still goes through the catch. The installer CI job ran install.ps1 under pwsh alone, which is how this went unnoticed. It now also runs the piped install under Windows PowerShell 5.1, into its own tool, bin and home directories. The short URL raven.evermind.ai/install.ps1 answers with a 308, which Windows PowerShell 5.1 does not follow either. The README, the docs site quick start and the release notes template now name the error it prints, (308) Permanent Redirect, and send readers to the direct URL when they see it. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
Three tests failed when the suite ran on a Chinese-locale Windows checkout. CI runs the unit tests on Linux only, so it never saw them. The workflow scan read each file in the locale encoding, GBK there, and stopped on the UTF-8 in release.yml. It now reads UTF-8, as the rest of the file already does. The font and LibreOffice harnesses run install.sh code under sh. On Windows that sh is Git Bash, whose curl sits outside /usr/bin and whose paths mix separators, so two harness tests failed and others passed for the wrong reason: the digest-mismatch test passed because curl was missing, not because of the digest. They now carry the POSIX-only skip the other sh-driven tests already had, shared as one marker. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed github/main...HEAD at d57ad789ea28. The release lookup now handles Windows PowerShell 5.1's non-terminating redirect error through the returned response while preserving PowerShell 7's exception-response path. The added Windows CI step exercises the stock shell independently from pwsh, and that installer job passed. The POSIX harness skips do not weaken Linux CI coverage; they prevent Git Bash from producing host-inaccurate results.
I covered the repository rules, the complete diff, relevant callers and history, backward compatibility across both PowerShell families, test-strength changes, and architecture constraints (no domain or layer boundary is affected). Verification: uv run --frozen --python 3.12 pytest tests/test_install_script.py tests/test_cli_onboard_commands.py -q passed with 395 tests; the source-language and large-file check scripts both exited 0.
|
Not a blocker -- the change is right and I am not asking for anything to Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. This PR and #861 each merge cleanly onto the tip, but conflict with each The cause is that you are both editing the same decorator lines from opposite Worth knowing beyond the textual conflict: the shared marker exists precisely so The UTF-8 fix addresses the instance, not the class. I am not asking you to touch the other 790; that is a different change and For what it is worth, the part of this PR I found most valuable is the admission |
Python's locale-dependent text I/O uses the system code page when no explicit encoding= is passed. On a GBK-locale Windows checkout, any test that reads or writes a file containing non-ASCII bytes will fail or silently corrupt data. The class of bug was found in this PR when test_no_workflow_step_enters_the_removed_page_directory stopped on the UTF-8 in release.yml; there are ~790 read_text() call sites without encoding in tests/, and no PYTHONUTF8 anywhere in the test infrastructure, so the next GBK-console developer would hit the next one. Set PYTHONUTF8=1 on every CI job that runs pytest: the unit shard matrix (which also runs the coverage steps), the trajectory regression replay, and the Windows self-upgrade job. On Linux this is a no-op. On Windows it forces every open()/read_text()/write_text() to use UTF-8, making the behaviour identical to what Linux CI already sees. tests/conftest.py is not touched: local developers can set the same variable, or add the flag to their pyproject.toml if needed, but that is a follow-up. Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the delta from d57ad789ea28 and rechecked the resulting github/main...HEAD diff. The new PYTHONUTF8=1 configuration covers every CI job that invokes pytest, is inherited by the intended subprocesses, and does not skip or weaken any tests. The Windows installer and self-upgrade jobs pass on this head.
I rechecked the repository rules, affected workflow callers and history, backward compatibility, test integrity, and architecture constraints; this CI-only delta does not cross a domain or layer boundary. Verification: uv run --frozen --python 3.12 pytest tests/test_install_script.py tests/test_cli_onboard_commands.py -q passed with 395 tests, and the source-language gate exited 0.
|
Not a blocker. Round two on Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. Gates on the merged tree (base is an ancestor of head, so head is the merged tree): What I checked and found rightYour "On Linux this is a no-op" is correct, and I verified it rather than taking it: the The class you describe is real and I measured it at this head: 792 Nit 1 -- the measure is in the one place the developer it names never readsYour stated beneficiary is "the next GBK-console developer", and the variable is in The three placements, measured off the parsed workflow:
So nothing it is on can change a decode today, and the only job whose matrix actually Nit 2 -- the in-file comment describes a matrix that does not exist
Smaller, in the same comment: "the stock shell on a GBK-locale Windows machine reads and Stated, not filed
Not coveredThe delta is CI configuration; no runner was exercised. Everything above about what a job |
Summary
Native Windows installs fail from the shell Windows ships with. Both commands the README gives for Windows break under Windows PowerShell 5.1:
irm https://raven.evermind.ai/install.ps1 | iexstops with(308) Permanent Redirect. PowerShell 5.1 follows 301, 302 and 307 redirects but not 308, which is what that URL answers with. This is the error fix: cannot install Raven in Windows via powershell 5.1 #148 reported; the README already offered the direct URL for it.Resolve-RavenLatestVersionrequests the release page without following the redirect and reads the Location header off the exception that raises. Only PowerShell 7 attaches the response to that exception. PowerShell 5.1 returns the unfollowed redirect as the response and reportsMaximumRedirectExceededas an error with no response, so the lookup always came back empty. This came in with fix(*): resolve the latest release without the github api quota #299 and has been on main since.Changes:
install.ps1: the release-page request uses-ErrorAction Ignoreinstead of-ErrorAction Stop, so PowerShell 5.1 reads Location from the returned response. PowerShell 7 still raises regardless of-ErrorActionand goes through the existing catch. The Location is still matched against the same stable-tag pattern.install.ps1underpwshonly, which is how this shipped. It now also runs the piped install under Windows PowerShell 5.1 (shell: powershell), into its own tool, bin and home directories, sharing the uv cache.(308) Permanent Redirectand to switch to the direct URL when that appears. The short URL's redirect itself is unchanged.-ErrorAction Ignore, catch kept) and for the CI gate (both shells, separate tool directories).Second topic, Windows test hygiene: three tests failed when the suite ran on a Chinese-locale Windows checkout. CI runs the unit tests on Linux only, so it never saw them.
test_no_workflow_step_enters_the_removed_page_directoryread workflows in the locale encoding (GBK there) and stopped on the UTF-8 inrelease.yml. It now reads UTF-8.install.shcode undersh, which on Windows is Git Bash: its curl sits outside/usr/binand paths mix separators, so two failed and others passed for the wrong reason (the digest-mismatch test passed because curl was missing). The ten harness tests now carry the POSIX-only skip the other three sh-driven tests already had, shared as onePOSIX_SH_ONLYmarker. Linux runs are unchanged.Overlap with open #861: both PRs touch
tests/test_install_script.py, with one textual conflict where #861 addstest_a_download_that_cannot_be_hashed_is_not_installednext to a test this PR marks. Whichever lands second keeps both, and the sh-harness tests #861 adds should get@POSIX_SH_ONLYtoo.Third topic, CI test infrastructure: Python's locale-dependent text I/O uses the system code page when no
encoding=is passed. On a GBK-locale Windows checkout, any test that reads or writes a file containing non-ASCII bytes will fail or silently corrupt data. The class of bug was found above; there are ~790read_text()call sites withoutencodingintests/, and noPYTHONUTF8anywhere in the test infrastructure, so the next GBK-console developer would hit the next one.PYTHONUTF8=1: the unit shard matrix (which also runs coverage), the trajectory regression replay, and the Windows self-upgrade job. On Linux this is a no-op. On Windows it forces everyopen()/read_text()/write_text()to use UTF-8, identical to what Linux CI already sees.Type
Verification
Run on Windows 11 (zh-CN) with Windows PowerShell 5.1.26100 and PowerShell 7.6.6. Every install below was isolated:
UV_TOOL_DIR,UV_TOOL_BIN_DIRandRAVEN_HOMEin a temp directory,RAVEN_MINIMAL=1,RAVEN_NO_LAUNCH=1.Before the fix, under 5.1:
irm https://raven.evermind.ai/install.ps1fails with(308) Permanent Redirect;irm https://raw.githubusercontent.com/EverMind-AI/Raven/refs/heads/main/install.ps1 | iexstops atCould not resolve the latest Raven release wheel from GitHub.Invoke-WebRequest https://github.com/EverMind-AI/Raven/releases/latest -MaximumRedirection 0 -UseBasicParsing -ErrorAction Stop: 5.1 throwsInvalidOperationException(MaximumRedirectExceeded) with noResponse; 7.6.6 throwsHttpResponseExceptionwhoseResponse.Headers.Locationis the tag URL. With-ErrorAction Ignore, 5.1 returns the 302 with Location set, and 7.6.6 still throws into the catch.Redirects under 5.1, via httpbin
redirect-to: 301, 302 and 307 are followed; 308 is not.After the fix,
Get-Content install.ps1 -Raw | Invoke-Expressionunder 5.1 and under 7.6.6, withuv tool update-shelldisabled in a temp copy of the script to leave the user PATH alone: both resolved v0.2.4, installed raven with everos-memory, design-engine and ppt-engine, andraven.exe --versionprintedRaven v0.2.4.uv run pytest tests/test_install_script.py tests/test_release_plugin_list.py tests/test_constraints_export_contract.pyplus the six installer tests intests/test_cli_onboard_commands.py, on Windows: 48 passed, 13 skipped (before: 55 passed, 3 skipped, 3 failed). The two new tripwires fail against the previousinstall.ps1andci.yml.uv run ruff checkanduv run ruff format --checkon both test files pass;tydoes not covertests/, the only Python this changes.PYTHONPATH=. uv run python scripts/check_source_language.py origin/mainexits 0.Not run locally: the ten newly marked harness tests on Linux, for lack of a Linux environment here. Their bodies are unchanged and the marker does not skip on Linux, so the ubuntu unit job runs them as before.
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
User-visible: the one-line Windows install works again from Windows PowerShell 5.1 through the direct URL. The short URL still answers with a 308 there, and the docs now name that error.
install.ps1is served from main, so the fix reaches users on merge, and reverting the squash commit rolls it back the same way. The resolved Location is still matched against the same stable-tag pattern before anything is downloaded. The installer CI job gains one Windows install, which reuses the uv cache. On Windows, 13 sh-driven tests now report as skipped.Related Issues
#148