Fix Windows compatibility: UTF-8 stdout, pip self-upgrade, complete UA#36
Open
chewinggum604-art wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes three Windows-specific issues that block first-run setup of the
skill on a fresh install. Tested on Windows 11 with Python 3.12.10 (winget,
user scope) and Chrome 131. No behavior change on macOS/Linux.
What's broken on Windows today
scripts/run.pycrashes immediately on first run.The runner prints a 🔧 emoji during venv bootstrap. On Windows the default
console encoding is
cp1252, which can't encode\U0001f527. Result:UnicodeEncodeErrorbefore any work happens.scripts/setup_environment.pyfails the dependency install.It upgrades pip via
pip.exe install --upgrade pip. Windows can't replacepip.exewhile it's running (FileLock), so the call returns non-zero,check=Trueraises, andensure_venv()returnsFalse. Setup aborts.scripts/config.pyships a truncatedUSER_AGENTmissing the(KHTML, like Gecko) Chrome/<ver> Safari/537.36tail. Truncated UAs are abot-detection signal — minor, but cheap to fix.
Changes
scripts/run.pysys.stdout/sys.stderrto UTF-8 witherrors='replace'at the top of the file (Python 3.7+).
PYTHONIOENCODING=utf-8in the env passed to subprocess calls so thefix propagates to
setup_environment.py,auth_manager.py,notebook_manager.py, andask_question.pywhen launched via the runner.scripts/setup_environment.py[venv_pip, "install", ...]to[venv_python, "-m", "pip", "install", ...]. This is the standardWindows-safe pattern and a no-op cost on POSIX.
scripts/config.pyUSER_AGENTwith the full Chrome 131 string.How to verify
Out of scope (separate PRs if welcome)
A few smaller things I noticed while debugging but kept out of this PR to keep
review focused. Happy to file separately if any are interesting:
auth_manager.is_authenticated()only soft-warns when state is older than7 days — would benefit from a hard cutoff (e.g., 14 days) so callers don't
blunder into expired-cookie territory without a clear error.
notebook_manager.update_notebook()exists on the class but isn't exposedon the CLI; would be nice to have an
updatesubcommand.references/troubleshooting.mdconsistently sayschromiumbut the skillinstalls Chrome (per
setup_environment.pyline ~76); also the Linux-onlypkill -f chromiumdoesn't help Windows users.Notes
non-Windows platforms (
sys.stdout.reconfigureis a no-op when stdout isalready UTF-8;
python -m pipworks the same aspipon POSIX; the longerUSER_AGENT is just more accurate).
the what, so future readers don't have to re-derive the FileLock or
cp1252 reasoning.