Add native Windows support: install.ps1, doctor.ps1, and Windows settings - #6
Conversation
…+ README redesign
Windows becomes a first-class install target with the kit's usual
no-silently-inert guarantees:
- install.ps1 / tools/doctor.ps1 — full-parity ports (backups, idempotency,
skill manifests + pruning, -Tier/-StrongModel, same checks and exit codes);
byte-parity with the bash twins is test-enforced, not asserted
- settings-snippet-windows{,-small}.json — hook commands invoke python
(Windows ships no python3); doctor.ps1 flags a merged Unix snippet and a
missing Git Bash (the Windows hook shell) as the inert installs they are
- tests/test_windows_port.py — snippet lockstep guards + pwsh e2e mirroring
test_install_doctor.py; POSIX-installer tests self-skip on Windows
- CI: windows-latest job (compile, install twice, doctor, unit suite) and
ps1 parse + all-snippet JSON checks on the Ubuntu job
- doctrine/skill/workflow text now notes the python-vs-python3 spelling
README restructured around the reader: install (both OSes) first, story
second, component-layer table with the annotated tree collapsible, badges +
nav; all honest-limits content kept, plus one new limit: the Windows port is
CI-verified but not yet live-session-verified.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NDoBxquQ5dysNKZ643mV3k
The parse step interpolated "$f:" (PowerShell reads $f: as a scope
qualifier — ParserError); delimit with ${f}. The Windows idempotency check
piped install.ps1 into Out-String, but the installer reports via Write-Host
(information stream) which bypasses the pipeline, so the capture was always
empty; merge streams with *>&1. Both steps re-verified locally with the
exact Actions invocation (pwsh -command ". step.ps1").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NDoBxquQ5dysNKZ643mV3k
- test_small_tier: guard the five bash-driving tests with the POSIX skip — on Windows bare `bash` is the WSL stub, and the unknown-flag test was even passing by accident off the stub's nonzero exit; their install.ps1 twins (-Tier small print, unknown flag, default-no-pin) now run in test_windows_port.py - test_webdesign_skill: pin encoding="utf-8" for the german-market read — Windows' cp1252 default mojibakes 'Datenschutzerklärung' - test_mem_privacy_guard: skip the case-sensitive-sibling scenario on NT — NTFS realpath case-folds Memory/ -> memory/ before the guard compares, so the distinct-sibling premise cannot be constructed there (and blocking IS correct on that fs) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDoBxquQ5dysNKZ643mV3k
📝 WalkthroughWalkthroughAdds a native Windows port: a PowerShell installer (install.ps1) and doctor script (tools/doctor.ps1) mirroring existing POSIX behavior, new Windows settings snippets, a CI windows job plus PowerShell parsing/settings validation, a Windows parity test suite, Windows skip markers on existing POSIX tests, and README/CHANGELOG/doc updates for ChangesWindows Installer, Doctor, and CI Parity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant InstallPs1 as install.ps1
participant FileSystem
participant DoctorPs1 as doctor.ps1
User->>InstallPs1: run install.ps1 -Tier
InstallPs1->>FileSystem: install agents/workflows/skills/hooks (idempotent)
InstallPs1->>FileSystem: merge doctrine, seed memory/privacy
InstallPs1->>User: print settings.json merge snippet
User->>FileSystem: merge settings.json manually
User->>DoctorPs1: run doctor.ps1
DoctorPs1->>FileSystem: verify installed files, settings wiring, mem CLI health
DoctorPs1->>User: report Ok/Warn/Bad and exit code
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)claude/workflows/memory-gc.jsFile contains syntax errors that prevent linting: Line 15: Illegal return statement outside of a function; Line 54: Illegal return statement outside of a function; Line 153: Illegal return statement outside of a function Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.github/workflows/ci.yml (2)
81-84: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNew
windowsjob checkout persists credentials by default.The new
actions/checkout@v4step in thewindowsjob doesn't setpersist-credentials: false, leaving theGITHUB_TOKENin the local git config for the remainder of the job, exposed to any process/step that runs afterward.🔧 Proposed fix
windows: runs-on: windows-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 81 - 84, The new windows job in the CI workflow uses actions/checkout@v4 without disabling credential persistence, which leaves the GITHUB_TOKEN in git config for later steps. Update the checkout step in the windows job to set persist-credentials to false so the token is not retained after the repository is checked out.Source: Linters/SAST tools
93-108: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winIdempotency check doesn't verify each
install.ps1run's exit code.Both invocations (lines 97 and 100) rely solely on text-matching
'unchanged'in the captured output to detect success/idempotency. Ifinstall.ps1crashes or errors partway (non-zero exit) but its partial/error output happens not to break the regex match, the step could still pass despite a real failure.🔧 Proposed fix
$env:CLAUDE_DIR = Join-Path $env:RUNNER_TEMP 'fake-claude' ./install.ps1 *>&1 | Out-Null + if ($LASTEXITCODE -ne 0) { Write-Host 'first run failed'; exit 1 } # install.ps1 reports via Write-Host (information stream) — merge all # streams or the capture is empty and this check can never pass. $second = ./install.ps1 *>&1 | Out-String + if ($LASTEXITCODE -ne 0) { Write-Host 'second run failed'; exit 1 } if ($second -notmatch 'unchanged') { Write-Host 'second run not idempotent'; exit 1 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 93 - 108, The idempotency step in the workflow only checks for the string "unchanged" and does not verify that each install.ps1 invocation succeeded. Update the "Install script end-to-end (twice, idempotent)" pwsh block to capture and validate the exit status of both install.ps1 runs, alongside the existing output check, so a partial crash or non-zero exit fails the job even if the text match succeeds. Use the install.ps1 invocations and the $second capture as the key spots to adjust the logic.tools/doctor.ps1 (2)
1-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNon-ASCII em-dashes without BOM risk mojibake on legacy Windows PowerShell.
The file uses em-dashes (—) throughout comments and
Ok/Bad/Warnmessages (e.g. lines 3, 9, 63, 66, 74). Static analysis flags the file as missing a BOM for its non-ASCII encoding.pwsh(Core) defaults to UTF-8 and is unaffected, but the legacypowershell.exe5.1 (still the default on many Windows machines) reads scripts using the system codepage unless a BOM is present, which can garble these characters in console output.🔧 Proposed fix
Save the file with a UTF-8 BOM, or replace decorative em-dashes with plain ASCII hyphens to avoid the dependency on encoding detection entirely.
Also applies to: 55-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/doctor.ps1` around lines 1 - 13, The PowerShell doctor script contains non-ASCII em-dashes in comments and status output, so legacy powershell.exe can misread it without a BOM. Update tools/doctor.ps1 to be saved as UTF-8 with BOM, or normalize the affected strings in the script to plain ASCII hyphens; keep the same behavior in the doctor checks and messages while preserving readable output in both powershell.exe and pwsh.Source: Linters/SAST tools
122-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMem CLI check silently skips output when Python is unavailable.
Unlike the hooks loop (lines 79-97), which has a fallback branch (
elseif (-not (Test-Identical...))) for the no-$pycase, this block has noelse— if$pyis$null, noOk/Bad/Warnline is ever printed for the mem CLI. The overall exit code is still correct (the earlier Python check already sets$script:fail = 1), but the doctor output has a silent gap here instead of an explicit status line.🔧 Proposed fix
} elseif ($py) { Invoke-Python $py @('-m', 'py_compile', $mem) | Out-Null ... +} else { + Warn "mem CLI presence unchecked: $mem (no Python found)" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/doctor.ps1` around lines 122 - 149, The mem CLI check in doctor.ps1 skips any status output when $py is unavailable, unlike the hooks validation flow. Add an explicit no-Python fallback in the mem CLI block around the $mem / Invoke-Python logic so it still prints a clear status line (for example, a warning or skip notice) when Python cannot be used, while preserving the existing compile-and-doctor checks when $py is present.tests/test_windows_port.py (1)
212-221: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDoctor "warns on python3 settings" test never checks exit status.
This is described as "the exact real-world failure the doctor exists for" elsewhere in the file (Line 191), yet this test only asserts the warning string appears in stdout — it never asserts
r.returncode != 0. Ifdoctor.ps1only prints the warning but exits 0, hooks would be silently broken and this test would still pass. Consider asserting a non-zero exit alongside the message, mirroring the other doctor failure tests (Lines 190-198, 200-210).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_windows_port.py` around lines 212 - 221, The test_doctor_ps1_warns_on_python3_settings check only verifies the warning text and should also assert the doctor exits non-zero. Update this test in test_doctor_ps1_warns_on_python3_settings to keep the existing stdout assertion and add a return-code assertion for r.returncode != 0, matching the other doctor failure tests around DOCTOR_PS.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@claude/CLAUDE.md`:
- Line 20: The Windows launcher guidance in the cross-project memory note is
incomplete, since it only mentions python and misses the supported py -3
fallback. Update the MEMORY.md guidance around the mem.py search command so the
Windows parenthetical explicitly includes py -3 as the alternative launcher for
the same command, keeping the instruction copy-pasteable across supported
Windows setups.
In `@tests/test_windows_port.py`:
- Around line 27-28: The helper that reads test fixture files via
Path.read_text() should not rely on the platform default encoding, since it can
break on Windows with non-ASCII content. Update the file access in load() and
any related write_text/read_text calls in this module to pass an explicit
encoding="utf-8", so the SETTINGS-based JSON and manifest reads/writes are
consistent across environments.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 81-84: The new windows job in the CI workflow uses
actions/checkout@v4 without disabling credential persistence, which leaves the
GITHUB_TOKEN in git config for later steps. Update the checkout step in the
windows job to set persist-credentials to false so the token is not retained
after the repository is checked out.
- Around line 93-108: The idempotency step in the workflow only checks for the
string "unchanged" and does not verify that each install.ps1 invocation
succeeded. Update the "Install script end-to-end (twice, idempotent)" pwsh block
to capture and validate the exit status of both install.ps1 runs, alongside the
existing output check, so a partial crash or non-zero exit fails the job even if
the text match succeeds. Use the install.ps1 invocations and the $second capture
as the key spots to adjust the logic.
In `@tests/test_windows_port.py`:
- Around line 212-221: The test_doctor_ps1_warns_on_python3_settings check only
verifies the warning text and should also assert the doctor exits non-zero.
Update this test in test_doctor_ps1_warns_on_python3_settings to keep the
existing stdout assertion and add a return-code assertion for r.returncode != 0,
matching the other doctor failure tests around DOCTOR_PS.
In `@tools/doctor.ps1`:
- Around line 1-13: The PowerShell doctor script contains non-ASCII em-dashes in
comments and status output, so legacy powershell.exe can misread it without a
BOM. Update tools/doctor.ps1 to be saved as UTF-8 with BOM, or normalize the
affected strings in the script to plain ASCII hyphens; keep the same behavior in
the doctor checks and messages while preserving readable output in both
powershell.exe and pwsh.
- Around line 122-149: The mem CLI check in doctor.ps1 skips any status output
when $py is unavailable, unlike the hooks validation flow. Add an explicit
no-Python fallback in the mem CLI block around the $mem / Invoke-Python logic so
it still prints a clear status line (for example, a warning or skip notice) when
Python cannot be used, while preserving the existing compile-and-doctor checks
when $py is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d65618e-ad36-43ad-97ba-4754dbc54e45
📒 Files selected for processing (15)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdclaude/CLAUDE.mdclaude/settings/settings-snippet-windows-small.jsonclaude/settings/settings-snippet-windows.jsonclaude/skills/memory-search/SKILL.mdclaude/workflows/memory-gc.jsinstall.ps1tests/test_install_doctor.pytests/test_mem_privacy_guard.pytests/test_small_tier.pytests/test_webdesign_skill.pytests/test_windows_port.pytools/doctor.ps1
| - Version-sensitive, fast-moving, or post-cutoff library/API questions: check live docs or WebSearch instead of trusting training memory. Stable stdlib basics need no lookup. | ||
| - You under-use persistent memory too: before re-deriving a decision about this project, check auto-memory (MEMORY.md); when a saga ends with a non-obvious lesson, bank it (postmortem skill) instead of letting it die with the session. | ||
| - Cross-project memory (fable-mem): native MEMORY.md only covers THIS repo, so before re-deriving a decision you may have made elsewhere, search the machine-wide corpus (`python3 ~/.claude/cli/mem.py search "<terms>"`, or the memory-search skill). Promote a lesson worth other projects to the global corpus via postmortem (the privacy guard blocks work-markers from leaking); run `/memory-gc` when the corpus feels stale. | ||
| - Cross-project memory (fable-mem): native MEMORY.md only covers THIS repo, so before re-deriving a decision you may have made elsewhere, search the machine-wide corpus (`python3 ~/.claude/cli/mem.py search "<terms>"` — `python` on Windows — or the memory-search skill). Promote a lesson worth other projects to the global corpus via postmortem (the privacy guard blocks work-markers from leaking); run `/memory-gc` when the corpus feels stale. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File and line context ==\n'
sed -n '1,80p' claude/CLAUDE.md
printf '\n== Search for Windows launcher guidance ==\n'
rg -n --hidden --glob '!**/.git/**' --glob '!**/node_modules/**' 'py -3|python alias|Windows.*python|python on Windows|launcher' .
printf '\n== Search for mem command references ==\n'
rg -n --hidden --glob '!**/.git/**' --glob '!**/node_modules/**' 'cli/mem.py search|memory-search|Cross-project memory|fable-mem' claude README* .github docs .Repository: blyatiful1/fable-protocol
Length of output: 21621
Make the Windows launcher fallback explicit here.
The Windows parenthetical only says python, but the repo’s Windows notes say to use py -3 when that’s the only available launcher, and that substitution applies to the mem.py commands too. Add that fallback here so the command stays copy-pasteable on the supported Windows setups.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@claude/CLAUDE.md` at line 20, The Windows launcher guidance in the
cross-project memory note is incomplete, since it only mentions python and
misses the supported py -3 fallback. Update the MEMORY.md guidance around the
mem.py search command so the Windows parenthetical explicitly includes py -3 as
the alternative launcher for the same command, keeping the instruction
copy-pasteable across supported Windows setups.
| def load(name): | ||
| return json.loads((SETTINGS / name).read_text()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use explicit UTF-8 for these file reads/writes. Path.read_text()/write_text() can fall back to the Windows locale codepage, so the agent/manifest file access in this module can fail on non-ASCII content; pass encoding="utf-8" to each of them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_windows_port.py` around lines 27 - 28, The helper that reads test
fixture files via Path.read_text() should not rely on the platform default
encoding, since it can break on Windows with non-ASCII content. Update the file
access in load() and any related write_text/read_text calls in this module to
pass an explicit encoding="utf-8", so the SETTINGS-based JSON and manifest
reads/writes are consistent across environments.
2 participants
Summary
This PR brings fable-protocol to Windows as a first-class install target with full parity to the POSIX installers. The kit's discipline layer (hooks, agents, workflows, skills, mem CLI) was already OS-portable, but the delivery layer was POSIX-only. Native Windows users now have deterministic, non-silent installation and verification.
Key Changes
install.ps1— Native Windows installer with full feature parity toinstall.sh:.fable-manifestskill tracking with stale-file pruning on upgrades-Tier smalland-StrongModel <model>flags for small drivers and asymmetric verificationsettings.json; prints the snippet to merge insteadpy -3,python,python3in order; soft-fails gracefully)tools/doctor.ps1— Native Windows verification with same checks and exit codes asdoctor.sh:python3commands in settings (silent failure on Windows)Windows settings snippets (
settings-snippet-windows.json,settings-snippet-windows-small.json):pythoninstead ofpython3(Windows Pythons ship nopython3launcher)Test suite (
tests/test_windows_port.py):CI/CD (
.github/workflows/ci.yml):Documentation (
README.md):pythonvspython3, and WSL notesNotable Implementation Details
install.ps1over a bash-installed tree (or vice versa) reports everything unchanged — dual-boot and WSL+native machines never churn backups.doctor.ps1and the test suite catch the exact failure mode the kit exists to prevent: a botched settings merge that leaves every hook unwired with zero symptoms.doctor.ps1checks for it and fails loudly if absent.https://claude.ai/code/session_01NDoBxquQ5dysNKZ643mV3k
Summary by CodeRabbit
New Features
pythonon Windows.Bug Fixes
Tests
Documentation