diff --git a/.github/workflows/batch-check.yml b/.github/workflows/batch-check.yml index ce074891..fd637cd5 100644 --- a/.github/workflows/batch-check.yml +++ b/.github/workflows/batch-check.yml @@ -368,6 +368,22 @@ jobs: run: | & tests\selfapps_envname.ps1 + - name: "Self-test: env-name sanitization (ampersand readability, CLAUDE.md Item 26)" + if: ${{ !cancelled() && env.HP_CACHE_CORRUPTED != '1' }} + env: + ENVNAME_SCENARIO: 'ampersand' + shell: pwsh + run: | + & tests\selfapps_envname.ps1 + + - name: "Self-test: env-name sanitization (64-char truncation bound, CodeRabbit PR #417)" + if: ${{ !cancelled() && env.HP_CACHE_CORRUPTED != '1' }} + env: + ENVNAME_SCENARIO: 'longname' + shell: pwsh + run: | + & tests\selfapps_envname.ps1 + - name: "Self-test: bootstrapper size tripwire (REQ-017)" # derived requirement (item 7 scoping pass): this step never executes run_setup.bat # (a static byte-size check only), so !cancelled() carries zero duration-inflation risk -- diff --git a/CLAUDE.md b/CLAUDE.md index 7860cda2..d536dabd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -567,26 +567,6 @@ start at 1 and has gaps. discipline as Item 25's own deferral. Low urgency: this only affects the `cache`-lane, non-gating `self.layered_e2e.chain` test; it does not block any lane that gates PR merges. -- **Item 26: `ENVNAME` sanitization collapses `&` (and every other non-word/non-hyphen character) - to a bare underscore in the built EXE's filename, losing readability -- owner-suggested - refinement, deliberately deferred as a far-term nice-to-have, not a defect.** `ENVNAME` (derived - from the project folder name near the top of `run_setup.bat`, right after - `:define_helper_payloads`) is already sanitized via a PowerShell regex (`-replace - '[^A-Za-z0-9_-]', '_'`) before it becomes both the conda env name and the actual built artifact - filename, `dist\%ENVNAME%.exe` -- exactly the kind of file a user might rename and email to - someone. No live bug: `&` is not in the allowed character set, so it already collapses to `_` - today, never reaching the filename raw. The owner's point (unprompted, general guidance for any - future bootstrapper output meant for user consumption, not a report of a broken case): many - tools mishandle a raw `&` in a filename (confuses it for URL query-string syntax), and Outlook - specifically renders `&`-containing filenames oddly in email -- but the CURRENT blanket - `[^A-Za-z0-9_-]` -> `_` substitution already avoids that failure mode categorically, just at the - cost of readability (a folder named `Sales & Marketing` becomes `Sales___Marketing.exe`, not - `Sales_and_Marketing.exe`). A refinement would special-case `&` -> `and` (or `_and_`) BEFORE the - general blanket substitution runs, preserving semantic meaning for that one common case while - leaving every other stripped character's behavior unchanged. Low priority, no reported real-world - friction yet -- filed here rather than implemented immediately since the underlying safety - property is already satisfied. - ## Cold Storage (promising ideas, deliberately shelved -- revisit only if a named trigger fires) Moved to `docs/agent-cold-storage.md` (2026-07-31, to reduce this file's per-session context diff --git a/README.md b/README.md index b7ae6b32..822ac174 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ trend toward this altitude -- keep the requirement crisp and let mechanism detai 2. `pyproject.toml` `requires-python` 3. Otherwise let the **selected provider pick latest** (no hard-coded fallback); then **write back `runtime.txt`**. (When the conda provider is active, conda resolves the latest available Python from conda-forge; when the embedded-Python provider is active, it resolves to the newest entry in its pinned version table.) -- Environment naming: env name equals the **current folder name**, sanitized (characters outside `[A-Za-z0-9_-]` (e.g. spaces) become `_`, a **leading hyphen** is replaced with `_`, and a name that reduces to only separators falls back to `env`; internal hyphens like `my-app` are preserved). When the conda provider is active, this sanitized name is passed to `conda create -n`. The derived name is logged: `[INFO] Environment name: `. +- Environment naming: env name equals the **current folder name**, sanitized (`&` is first special-cased to the bare word `and` for readability, e.g. `Sales & Marketing` -> `Sales_and_Marketing`; then characters outside `[A-Za-z0-9_-]` (e.g. spaces) become `_`; a **leading hyphen** is replaced with `_`; the result is **truncated to 64 characters** whenever it exceeds that length (most commonly caused by the `&`->`and` expansion, which can make the sanitized name longer than the original); and a name that reduces to only separators falls back to `env`; internal hyphens like `my-app` are preserved). When the conda provider is active, this sanitized name is passed to `conda create -n`. The derived name is logged: `[INFO] Environment name: `. - **Provider independence:** The bootstrapper cannot depend exclusively on a single provider. It must be able to function with only any one of the REQ-009 providers available (uv alone, conda alone, embedded Python alone, venv alone, or system Python alone). No bootstrap path may hard-require a specific provider to be present. - UV is the preferred environment provider when available (cached or downloadable), as it is fast and avoids Miniconda download latency. When UV is unavailable or disabled, the bootstrapper falls back to Miniconda (conda provider); if conda also fails, it downloads a checksum-verified embeddable Python build directly from python.org (no admin rights, no pre-existing Python required); if that fails too, it falls back to a local venv built from whatever Python is already on the machine; and as a last resort runs the entry point under any available system Python. Every provider path preserves the Prime Directive -- at least one .py file runs with its imports satisfied. diff --git a/docs/agent-closed-backlog.md b/docs/agent-closed-backlog.md index 1a975ad3..30dc4e90 100644 --- a/docs/agent-closed-backlog.md +++ b/docs/agent-closed-backlog.md @@ -1562,6 +1562,61 @@ this belongs to). `--add-binary`, not yet observed for any real package in this repo's testing -- that made a live CI trigger disproportionate effort to build for this specific gap). +### Item 26 (closed 2026-08-08) + +- **`ENVNAME` sanitization collapsed `&` (and every other non-word/non-hyphen character) to a + bare underscore in the built EXE's filename, losing readability -- owner-suggested refinement, + not a defect.** `ENVNAME` (derived from the project folder name near the top of + `run_setup.bat`, right after `:define_helper_payloads`) is sanitized via a PowerShell regex + (`-replace '[^A-Za-z0-9_-]', '_'`) before it becomes both the conda env name and the actual + built artifact filename, `dist\%ENVNAME%.exe` -- exactly the kind of file a user might rename + and email to someone. No live bug: `&` was never in the allowed character set, so it already + collapsed to `_`, never reaching the filename raw -- many tools mishandle a raw `&` in a + filename (confuses it for URL query-string syntax), and Outlook specifically renders + `&`-containing filenames oddly in email, but the blanket substitution alone already avoided + that failure mode categorically, just at the cost of readability (a folder named + `Sales & Marketing` became `Sales___Marketing.exe`, not `Sales_and_Marketing.exe`). + **Fixed**: `&` is now special-cased to the bare word `and` immediately BEFORE the blanket + substitution runs (`run_setup.bat`, the `ENVNAME_SANITIZED` PowerShell one-liner) -- deliberately + a bare word with no surrounding underscores of its own, since the existing spaces on either + side of `&` are still converted to `_` by the blanket rule right after, so + `"Sales & Marketing"` -> `"Sales and Marketing"` -> `"Sales_and_Marketing"` without this + substitution needing to supply its own separators; matches the backlog's own illustrative + example exactly. Every other stripped character's behavior is unchanged. + **Test coverage**: `tests/selfapps_envname.ps1` gained a second scenario + (`ENVNAME_SCENARIO=ampersand`, `self.envname.ampersand`) alongside its existing leading-hyphen + case (`self.envname.hyphen`) -- both are real, live CI tests (not simulated), each creating a + real folder with the hazard character, running `run_setup.bat` with `HP_CI_SKIP_ENV=1` (no + conda needed, cheap), and asserting the logged `Environment name: ...` line matches the + expected sanitized form with no leading trace of the original hazard character. Wired as a + second CI step in `batch-check.yml` immediately after the existing hyphen step, same + `!cancelled()` gating (any lane, cheap, skip-env). `docs/agent-ndjson.md` updated with the new + row id. + **CodeRabbit review round on PR #417 caught 3 real follow-on gaps in this fix, all closed in the + same PR before merge:** + 1. `&`->`and` is the sanitizer's first 1-to-3-char expansion (every other substitution is + 1-to-1), so an ampersand-heavy folder name could make the sanitized result LONGER than the + original -- fixed with a 64-char post-substitution truncation + (`$san.Substring(0, 64).TrimEnd('_', '-')`), ample for a real project folder and well clear + of Windows/conda length limits for what this value later becomes (`ENV_PATH`, + `dist\.exe`). Third scenario added (`ENVNAME_SCENARIO=longname`, + `self.envname.longname`) with an ampersand-heavy folder name that expands past 64 chars + pre-truncation, asserting both the exact truncated value AND (decoupled from the exact-match + assertion) the logged name's length is `<=64` -- proving the truncation branch actually ran, + not just that some 64-char value happened to match by chance. + 2. The empty-after-truncation fallback check, `$san.Trim('_').Length -eq 0`, only trimmed + underscores -- a separator-only result like `_-` would pass the check (non-zero length after + trimming just underscores) and reach `conda create -n _-`, a name conda would likely also + reject. Fixed: `$san.Trim('_', '-').Length -eq 0`. + 3. If the PowerShell sanitization command itself failed to run (missing `powershell.exe`, + execution-policy lockdown) or emitted no output, `ENVNAME_SANITIZED` stayed undefined and the + caller silently fell through to the RAW, unsanitized folder name -- defeating the whole guard + for exactly the failure mode most likely to coincide with a hostile/malformed name in the + first place. Fixed: fail closed -- `if defined ENVNAME_SANITIZED (...) else (call :log + "[WARN] ...falling back to 'env'..." & set "ENVNAME=env")`, mirroring the existing + empty-name-guard fallback used later in the file (`:env_state_check_done`'s + `if "%ENVNAME%"=="" (...) set "ENVNAME=env"`). + ## Closed Backlog - **Cascade-vs-postexec fix (Active Backlog item 9), 2026-07-25, owner-directed follow-up to a diff --git a/docs/agent-lessons-learned.md b/docs/agent-lessons-learned.md index f3683f9b..e85607e9 100644 --- a/docs/agent-lessons-learned.md +++ b/docs/agent-lessons-learned.md @@ -21,6 +21,77 @@ leaving stale guidance.** --- +## Quote a variable before piping it into `findstr`, or `&` in its value splits the command line + +**Found via a CodeRabbit review finding on PR #417 (an "outside diff range" catch -- pre-existing +code, unrelated to that PR's own change, surfaced incidentally while reviewing nearby lines).** +The system-directory guard (near the top of `run_setup.bat`, right after the UNC-path check) did +`echo %HP_SCRIPT_ROOT%| findstr /I /C:"%WINDIR%\\" >nul` -- `HP_SCRIPT_ROOT` echoed UNQUOTED into +a pipe. cmd.exe's own command-line parser has no notion of "this `&` came from a variable, not +literal text" -- it decides whether `&` is a command separator purely from the CURRENT quote +state as it scans the line left to right, and that scan runs the SAME expansion pass that +substitutes `%HP_SCRIPT_ROOT%`. So a script dropped under a path like +`C:\Users\Sales & Marketing\run_setup.bat` would have this single line silently split into two +commands at the `&` -- `findstr` receives only the truncated prefix, and the guard can miss a +real match (or worse, run a bogus command named after whatever follows the `&`). This is a +DIFFERENT hazard from `:log`'s own "echoes UNQUOTED" entry below (that one is about `:log`'s +`echo %MSG%` misinterpreting `<`/`>`/`|`, here it's `&` splitting the command line the pipe itself +sits on) but the same family: an unquoted `%VAR%` reaching a live cmd.exe operator context. + +**Fix: wrap the variable in quotes** -- `echo "%HP_SCRIPT_ROOT%"| findstr ...`. This works because +cmd.exe tracks quote state THROUGH the expansion, not around it: the literal `"` characters in the +source line toggle quote state before `%HP_SCRIPT_ROOT%` is substituted, so any `&` landing inside +the expanded value is scanned while the parser considers itself "inside quotes" and is never +treated as an operator. The one wrinkle: `echo` is one of the few cmd.exe builtins that does NOT +strip the quote characters from what it prints (unlike normal argument-parsing commands) -- so the +piped text becomes `"C:\Windows\Temp\MyApp\"` (literal quotes at both ends) instead of the bare +path. This is harmless here because `findstr /C:"..."` does a plain substring search, not an +exact-line match -- the real target text still appears in the middle of the quoted output +regardless of the extra leading/trailing `"` characters. Applied to all three system-directory +checks (`WINDIR`/`ProgramFiles`/`HP_PF86`); the existing `self.warn.sysdir` test (a plain, +non-`&` path) continues to prove the base guard still fires correctly post-fix, but does not by +itself exercise the `&`-specific scenario this fix targets -- a dedicated adversarial test +(a folder literally named with `&` under `%WINDIR%\Temp`) is a reasonable future addition, not +built here (small, defensive quoting fix outside this PR's own scope, same "fix now, note as a +candidate for future dedicated coverage" precedent already used elsewhere in this file for +review-caught correctness fixes that reuse an already-tested code shape). + +--- + +## A multi-scenario PowerShell test's NDJSON `id` must stay a literal string at each `Write-NdjsonRow` call site, never a shared variable + +**Found via a real CI failure while adding `tests/selfapps_envname.ps1`'s second scenario +(CLAUDE.md Item 26).** `tools/check_ndjson_registry.py`'s static scan of `tests/*.ps1` matches +one of four fixed textual patterns to discover which NDJSON `id`s a test file emits -- +`CODE_HASHTABLE_ID_RE = re.compile(r"\bid\s*=\s*['\"]([A-Za-z0-9][A-Za-z0-9_.\-]*)['\"]")` is the +one this class of test uses. It is a plain regex over the file's TEXT, not a PowerShell parser -- +it has no notion of variable assignment or control flow, so it can only ever match an `id` key +followed immediately by a quoted literal. + +Refactoring a single-scenario test file into a multi-scenario one (env-var-selected, matching this +repo's own established `PYI_FAIL_SCENARIO`-style convention) naturally tempts consolidating the +per-scenario `id` into one shared variable (`$rowId = 'self.foo.bar'` in a `switch`, then +`Write-NdjsonRow ([ordered]@{ id=$rowId; ... })` at the single call site) -- this is correct, +idiomatic PowerShell and preserves the exact same runtime NDJSON output, but it silently breaks +the regex: `id=$rowId` never matches `\bid\s*=\s*['"]`, so the checker reports the id as +"registered in docs but no matching code emission site found" -- indistinguishable from a genuinely +stale/removed row. Confirmed doubly damaging in practice: not just the NEW scenario's id went +undetected, but the PRE-EXISTING scenario's id did too, even though its own emitted NDJSON content +was completely unchanged -- the regression was in the STATIC TEXT shape, not the runtime behavior. + +**Fix: keep `id='literal.id.here'` as a literal at EVERY `Write-NdjsonRow` call site**, even if that +means branching on the scenario variable a second time right at the call site (`if ($scenario -eq +'x') { Write-NdjsonRow ([ordered]@{ id='self.foo.x'; ... }) } else { Write-NdjsonRow ([ordered]@{ +id='self.foo.y'; ... }) }`) instead of consolidating into one call fed by a shared `$rowId` +variable. This is a small amount of duplication in exchange for staying legible to a scanner that +cannot execute the script. This check is advisory (`continue-on-error: true`, non-gating) so it +never blocks a merge on its own, but the finding is real and worth fixing on sight -- do not treat +it as noise. Verify any new multi-scenario test file against the same regex directly before +pushing: `python3 -c "import re; print(re.findall(r'\bid\s*=\s*[\'\"]([A-Za-z0-9][A-Za-z0-9_.-]*)[\'\"]', open('tests/the_file.ps1').read()))"` +and confirm every scenario's id appears. + +--- + ## Never open a real source file in Python `'w'` mode as part of a "dry run" -- write to a NEW path and diff before overwriting **Genuine near-miss (2026-07-25) while fixing the `HP_PREP_REQUIREMENTS` payload.** A verification diff --git a/docs/agent-ndjson.md b/docs/agent-ndjson.md index 30dcc61c..cd6024ef 100644 --- a/docs/agent-ndjson.md +++ b/docs/agent-ndjson.md @@ -30,7 +30,7 @@ self.entry.helper.invoke.absent, self.entry.results, self.entry.spaced-path, sel self.entry.picker.overflow, self.entry.req011.crossdir, self.entry.req011.sameDir, self.isolation.req010.pythonpath, entry.single.direct, entry.expected, helper.invoke, -self.envname.hyphen, self.size.tripwire, +self.envname.hyphen, self.envname.ampersand, self.envname.longname, self.size.tripwire, reqspec.translate.{gte,eq,compat,gt,neq,lte}, reqspec.conda.dryrun, reqspec.conda.channelpin, reqspec.conda.dryrun.failcase, reqspec.conda.channelpin.req006, reqspec.conda.dryrun.req006, diff --git a/docs/demo-bootstrapper-output.md b/docs/demo-bootstrapper-output.md index dcf1e4cb..41708bbf 100644 --- a/docs/demo-bootstrapper-output.md +++ b/docs/demo-bootstrapper-output.md @@ -186,13 +186,28 @@ every byte of console output via `cmd /c .\run_setup.bat > '~envsmoke_bootstrap. ``` Tue 07/28/2026 4:29:43.96 [INFO] REQ-015: Appending standard ignores to .gitignore. +Tue 07/28/2026 4:29:43.97 [INFO] REQ-015: Appending standard attributes to .gitattributes. ``` -Before that line, nothing prints -- `HP_APP_ARGS` capture (REQ-026, pure variable assignment), the -workspace-path-exists check, `cd /d`, `HP_SCRIPT_ROOT` construction, and the top-of-file UNC-path -check (`if "%HP_SCRIPT_LAUNCH_DIR:~0,2%"=="\\"`, which prints a much louder `*** WARNING: -UNC/network paths detected...` banner when it genuinely fires) are all silent on an ordinary, -non-UNC path. +Before that first line, nothing prints -- `HP_APP_ARGS` capture (REQ-026, pure variable +assignment), the workspace-path-exists check, `cd /d`, `HP_SCRIPT_ROOT` construction, and the +top-of-file UNC-path check (`if "%HP_SCRIPT_LAUNCH_DIR:~0,2%"=="\\"`, which prints a much louder +`*** WARNING: UNC/network paths detected...` banner when it genuinely fires) are all silent on an +ordinary, non-UNC path. Both `.gitignore`/`.gitattributes` lines come from the same +`:merge_git_config` call (the first output-producing call after `run_setup.bat` sets up its own +log file) -- on a fresh scratch directory neither file yet has the bootstrapper's signature +comment, so both append branches fire back to back. + +Immediately after that, the same real capture shows the environment-name and host-diagnostics +lines every run prints (same run, same underlying artifact -- also quoted in Scenario 6 below, +where the exact values are unchanged since they describe the same CI host, not anything specific +to a fresh vs. repeat run): + +``` +[INFO] Environment name: _envsmoke +[INFO] Host OS: Microsoft Windows [Version 10.0.26100.32995] +[INFO] Host PowerShell: 5.1.26100.32995 +``` **The four REQ-025-family pre-flight guards (path-length, OneDrive, system-directory, disk-space -- also part of that unlabeled prologue) are completely silent unless they fire.** Confirmed by both @@ -309,6 +324,12 @@ isolate one mechanism from the other): [INFO] REQ-005.12: autopep723 discovery merge complete. ``` +The "pipreqs (direct) command:" line is a DISPLAY-ONLY string (`HP_PIPREQS_CMD_LOG`, built for +human readability as the CLI-equivalent form) -- the actual invocation, per this repo's own +"never depend on console scripts during bootstrap" rule, is +`"%HP_PY%" -m pipreqs.pipreqs . --force --mode compat --savepath ... --ignore ...`, never the +bare `pipreqs` command shown on screen. + **Not shown above because it doesn't apply to this run, not omitted:** since this app had no pre-existing `requirements.txt` (`docs/agent-closed-backlog.md`'s Item 21), `requirements.txt` was freshly copied from `requirements.auto.txt` a few lines earlier in `:after_pipreqs_run`, so the @@ -975,7 +996,20 @@ positively (not just "dragging works," but that override genuinely beats auto-de win plain auto-detection by name-priority) and `zzz_override.py`, with `zzz_override.py` passed as the override -- the real run confirms the drag message names the override file, the entry-selected log line names `zzz_override.py` (not `main.py`), and the override file's own distinguishing output -is what actually ran. +is what actually ran. The exact console sequence that test asserts against (`tests/ +selfapps_ux_hardening.ps1` lines 1161-1164 -- a scratch directory staged with both `main.py` +containing `print("from-main")` and `zzz_override.py` containing `print("from-override")`, +launched as `run_setup.bat .\zzz_override.py`): + +``` +*** Using drag-and-drop file: .\zzz_override.py +[BOOT] REQ-002: Entry selected: zzz_override.py +from-override +``` + +The third line is the override script's own stdout, captured in `~run.out.txt` -- the test asserts +it contains `from-override` and specifically does NOT contain `from-main`, confirming the override +genuinely ran instead of the higher-name-priority file sitting right next to it. --- @@ -1334,6 +1368,19 @@ the build interpreter, the bootstrapper rebuilds with `--hidden-import=` plain `ImportError` (not a missing module at all) never triggers a rebuild, since the fix is not mechanically derivable. +**Success, one rebuild** (`self.exe.hidden_import`'s own fixture: a dynamic `importlib.import_module` +call on `colorama`, invisible to PyInstaller's static analysis, so the frozen EXE genuinely fails +its first run) -- exact sequence assembled from the log lines `tests/selfapps_hidden_import.ps1` +itself matches against (`run_setup.bat`'s own `:log` calls at the smokerun/recovery call sites): + +``` +[WARN] EXE smokerun: exited 1 (non-zero) +[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=colorama; rebuilding EXE (iter 1/3). +[REPAIR][HIDDEN_IMPORT] EXE verified after hidden-import recovery. +[INFO] EXE smokerun: exited 0 (ok) +[INFO] Entry smoke exit=0 +``` + **Exhaustion** (three DIFFERENT modules missing across three rebuilds, still never fully resolving -- REAL CI CAPTURE): @@ -1500,6 +1547,24 @@ no-op rather than a duplicate append: [INFO] REQ-015: Appending standard attributes to .gitattributes. ``` +This is the first thing `run_setup.bat` prints to the console (`:merge_git_config` runs before the +environment-name/host-diagnostics block Scenario 2 covers, though several earlier steps -- the +UNC-path check among them -- are silent on an ordinary path; see Scenario 2 for the full list) -- +the same real capture continues immediately with: + +``` +[INFO] Environment name: _envsmoke +[INFO] Host OS: Microsoft Windows [Version 10.0.26100.32995] +[INFO] Host PowerShell: 5.1.26100.32995 +``` + +On the idempotent second run, BOTH `.gitignore`/`.gitattributes` lines above are absent entirely -- +`findstr`'s signature check short-circuits straight past each `call :log` line +(`if not errorlevel 1 goto :mgc_gi_done` / `:mgc_ga_done`) before it can fire, so this step +contributes zero console output on a repeat run. `self.ux.gitignore.idem` confirms this by +counting the signature's occurrences in the file (`sigCount:1`, not by scanning for an absent log +line), which is why the idempotent case has no console excerpt of its own here. + The appended `.gitignore` block (verbatim from source): ``` @@ -1651,9 +1716,15 @@ print('hi') After a genuinely fresh, fully-successful `HP_ENV_MODE=uv` dependency install (see Part I, Scenario 3), `:pep723_writeback` promotes the resolved dependency set into the entry file's own PEP 723 header via `uv add --script`, so the pin travels with the user's source file rather than -staying only in `requirements.txt`/the lock file: +staying only in `requirements.txt`/the lock file. The three lines immediately before it are the +same real capture already shown in Scenario 3 (dependency install completing, the pip-freeze +snapshot, the environment-lock snapshot) -- `:pep723_writeback fresh` is called right at +`:lock_done`, immediately after the last of those three: ``` +[INFO] UV_USED=1 +[INFO] DEP_INSTALLED_CAPTURED=1 +[INFO] Environment snapshot written: ~environment.lock.txt [INFO] REQ-005.11: PEP 723 header write-back succeeded via uv add --script. ``` @@ -1701,16 +1772,24 @@ real and passing). This is an opt-in super-user flag (not part of the default happy path -- Part I never sets it): when defined, `run_setup.bat` skips straight to actually RUNNING the entry file live via `uvx autopep723 ` for dependency discovery, before pipreqs or any static analysis even -starts. Real capture: +starts. `:pvw_known_idempotent_run` is called right after entry selection returns (immediately +after the `if defined HP_PVW_KNOWN_IDEMPOTENT ...` gate), so the very next thing on screen after +the entry is chosen (the same "Chosen entry: ..." moment Scenario 2 covers) is this discovery run. +Real capture, `self.pvw_idempotent.discovery`, including the entry script's +own live stdout passed straight through mid-run (real NDJSON detail confirms +`stdoutPassthroughFound:true, appRan:true` -- not captured or suppressed, the exact design point +`tools/pvw_known_idempotent.py` exists to preserve): ``` [INFO] REQ-005.13: HP_PVW_KNOWN_IDEMPOTENT set; running entry via uvx autopep723 for execute-mode discovery. +t2-idempotent-ok [INFO] REQ-005.13: execute-mode discovery run succeeded (RAN:persisted). ``` -The entry script's own stdout is inherited/passed through live during this discovery run (not -captured or suppressed) -- real NDJSON detail confirms `stdoutPassthroughFound:true, appRan:true` --- and whatever dependency it needed (`requests`, in this real capture: `reqsHasRequests:true`) is +(`t2-idempotent-ok` is this specific stub's own `print()` output -- see Scenario 36 for the exact +`app.py` source that produces it, and for the file-content side of this same real test.) + +Whatever dependency the run needed (`requests`, in this real capture: `reqsHasRequests:true`) is persisted back into the PEP 723 header via `uv add --script`, then re-extracted into `requirements.txt` so the rest of the pipeline (pipreqs, Tier 1 autopep723 merge, the actual install) sees it too. Deliberately ADDITIVE, not a replacement for pipreqs -- pipreqs and Tier 1's @@ -1795,11 +1874,20 @@ from `tests/~pandas_excel/`'s own scratch directory: [HEURISTIC] pandas->xlsxwriter ``` -(the console line is a compact tag; the two package names themselves are appended to the conda -install spec list, confirmed by the real conda solve plan later in the same `~pandas_excel` log: -`openpyxl conda-forge/win-64::openpyxl-3.1.5-py314hccc76fc_3` and `xlsxwriter -conda-forge/noarch::xlsxwriter-3.2.9-pyhd8ed1ab_0`, and both packages installed into -`~pandas_excel`'s own `requirements.txt`/`~reqs_conda.txt`/`~reqs_pip.txt`). The claim that +The console line is deliberately a compact tag -- the two package names themselves are appended to +the conda install spec list, then genuinely resolved by conda's own solver a few lines later in the +same run. That solve output reaches `~setup.log` only, never the console (the `conda install` call +this heuristic feeds is redirected via `>> "%LOG%"`, matching this doc's "Console vs. `~setup.log`" +convention noted at the top): + +``` +openpyxl conda-forge/win-64::openpyxl-3.1.5-py314hccc76fc_3 +xlsxwriter conda-forge/noarch::xlsxwriter-3.2.9-pyhd8ed1ab_0 +``` + +Both package names are recorded in `~pandas_excel`'s own `requirements.txt`/`~reqs_conda.txt`/ +`~reqs_pip.txt` (dependency-source files, not installation targets); conda is what actually +installs them into the selected environment, per the solver output quoted above. The claim that `openpyxl` ends up genuinely bundled and importable in a frozen EXE is confirmed by the SIBLING `self.exe.warnfix.real` test's OWN independent scratch directory (`tests/~selftest_warnfix_real/`, a different app that also exercises the pandas heuristic, per its own NDJSON `desc` text quoted @@ -1833,17 +1921,27 @@ correctly skips rather than updating a base that was just installed moments ago: [INFO] Conda base update: skipped (first install). ``` -**`[Extrapolated Branch]`** -- the actual 30-day-elapsed UPDATE-firing branch (`:cbu_run`, -`run_setup.bat`) is not exercised by any current CI run (the only flag that could force it is -deliberately disabled, per the note above). Traced from source: once the timestamp in -`~conda.lastupdate` is more than 30 days old, the subroutine runs `conda update -n base -y` and -would log something in the shape of `[INFO] Conda base update: running (last updated N days -ago)...` followed by conda's own real update-solve output, then rewrites `~conda.lastupdate` to the -current time on completion. This branch realistically only fires for a long-lived, repeatedly-reused -project folder -- not the fresh-checkout scenarios this document otherwise captures -- and is -correctly out of scope for a dedicated CI test: a forced-update test previously broke conda's own -solver in shared CI runners, an accepted, documented tradeoff (`docs/agent-ndjson.md`'s -"conda-full lane rows" section). +**`[Extrapolated Branch]`** -- the actual 30-day-elapsed UPDATE-firing branch (`:cbu_run`) is not +exercised by any current CI run (the only flag that could force it is deliberately disabled, per +the note above). Once the timestamp in `~conda.lastupdate` +is more than 30 days old, the subroutine's two `:log` calls are these exact, deterministic literal +strings -- not an approximation, the source text itself: + +``` +[INFO] Conda base update: running (>=30 days since last update or no record). +[INFO] Conda base update complete. +``` + +Between those two lines, `conda update -n base --all --override-channels -c conda-forge -y` runs +with its output redirected straight to `~setup.log` (`>> "%LOG%" 2>&1`) -- conda's own real +update-solve output (package list, versions, download progress) never reaches the console, matching +this doc's "Console vs. `~setup.log`" convention. If the update itself fails (a nonzero exit from +that command), the second line is `[WARN] Conda base update failed; continuing.` instead -- the +bootstrap is never blocked by a failed base update either way. This branch realistically only fires +for a long-lived, repeatedly-reused project folder -- not the fresh-checkout scenarios this document +otherwise captures -- and is correctly out of scope for a dedicated CI test: a forced-update test +previously broke conda's own solver in shared CI runners, an accepted, documented tradeoff +(`docs/agent-ndjson.md`'s "conda-full lane rows" section). --- @@ -1856,10 +1954,20 @@ the terminal step of a full provider-cascade exhaustion); this scenario complete **Source:** REAL CI CAPTURE, run `30328748330`, job `90179708091` (`real` lane). When every other REQ-009 provider tier has failed or been declined, the system-Python tier is -still reached by any default, no-flag run -- it is gated solely by the REQ-014 consent prompt -(Scenario 5 in Part I already documents the prompt's own framing text in full), never by an -env-var the user would need to set. On ACCEPT, the bootstrapper proceeds to use whatever Python is -already on the machine, unmanaged and unisolated: +still reached by any default, no-flag run -- it is gated solely by the REQ-014 consent prompt, +never by an env-var the user would need to set. The prompt text itself (`:system_python_consent_gate`, +`run_setup.bat`) is echoed unconditionally, even on CI's auto-decline path, so it is exact, literal +source text, not a reconstruction: + +``` +*** WARNING: System Python Execution *** +*** Using global system Python may pollute shared packages. *** + +Proceed with System Python? (Global pollution risk) [y/n]: y to accept, n to decline. +``` + +On ACCEPT, the bootstrapper proceeds to use whatever Python is already on the machine, unmanaged +and unisolated: ``` [INFO] REQ-014: System Python consent: user accepted. @@ -2003,8 +2111,17 @@ Interpreter: [WARN] Interpreter smoke test failed (continuing). ``` -(pipreqs, dependency install, and the pyvisa check all run to completion afterward, effectively as -no-ops against the broken interpreter, before the entry is finally selected and preflight runs) +**`[Extrapolated Branch]`** for what happens between those lines and the misleading block below -- +traced from source, not an independently preserved capture. `run_setup.bat` genuinely keeps +executing -- pipreqs's own install attempt, the dependency-install step, and the pyvisa detection +check are none of them gated on `HP_NO_INTERPRETER` (confirmed by reading each call site directly; +that flag was only ever checked by the fix's own new `:preflight_compile` guard), so each one +genuinely runs against the empty `HP_PY` before the entry is finally selected and preflight fires. +Their own exact console text from this specific historical run was not separately preserved +alongside the two blocks quoted here (only the excerpts a maintainer captured while diagnosing the +bug at the time +survived) -- each would have produced its own `cmd.exe`-level "not recognized" error or install +failure, the same general shape as the two blocks already shown, rather than silently vanishing. ``` *** [ERROR] REQ-021: Your Python program has a syntax error and cannot run. *** @@ -2397,12 +2514,17 @@ already on disk (uv-first runs skip Miniconda entirely until something actually `docs/agent-interconnect.md`'s "uv-First Provider Architecture"), then re-enters the same dependency-install machinery Scenario 3 already documents in full, just under `HP_ENV_MODE=conda` this time (`[Extrapolated Branch]` for this specific re-entry's own console -text, though every individual line reused below is independently real elsewhere in this file): +text, though every individual line reused below is independently real elsewhere in this file -- +including the "pipreqs (direct) command:" line's own display-only caveat, noted in full there): ``` [BOOT] REQ-009: Selected Python provider: Conda (Portable). Creating Python environment '' -- this may take several minutes... [INFO] runtime.txt written: python-3.14.6 +[INFO] pipreqs 0.4.13 installed successfully; using it for dependency discovery. +[INFO] pipreqs (direct) command: pipreqs . --force --mode compat --savepath "...\requirements.auto.txt" --ignore ".git,.github,.venv,venv,env,.uv_env,build,dist,__pycache__,tests" +[INFO] DEP_INSTALLED_CAPTURED=1 +[INFO] Environment snapshot written: ~environment.lock.txt [INFO] Building standalone executable -- this may take a minute or two... [INFO] PyInstaller produced dist\.exe [INFO] EXE smokerun: testing dist\.exe @@ -3103,7 +3225,8 @@ since this scenario has passed on every run so far): ``` *** Your app is ready. *** -*** Want to build an optimized version too? ... *** +*** Want to build an optimized version too? It takes a bit longer to build right now, *** +*** but it starts up more reliably on Windows and runs faster once it is built. *** [INFO] Optimized build: accepted; building now (this may take a minute or two). [WARN] Optimized build verified successfully but could not be swapped into place; your app is still ready to use as-is. ``` @@ -3182,7 +3305,18 @@ The `hidden_import` recovery loop's own separate, narrower verification check (s "Verifying a fresh build is activity-aware and announced" bullet, which calls this exact exception out directly) deliberately keeps the OLDER, unconditional 30-second wording -- it's a bounded repair-verification check on an already-built EXE, not the user's primary run, so it never got the -interactive-friendly rewrite. +interactive-friendly rewrite. Same subroutine (`:warn_user_code_launch`), same PyInstaller-vs-Nuitka +variant split, but a genuinely different, still-unconditional message (the `hidden_import`-site +branch's own literal source text): + +``` +[WARN] Verifying the built standalone EXE (fallback build system) now: it is force-stopped after about 30 seconds even if running perfectly, so do not start real work in it yet or any unsaved work will be lost. +[WARN] Verifying the built standalone EXE (PyInstaller) now: it is force-stopped after about 30 seconds even if running perfectly, so do not start real work in it yet or any unsaved work will be lost. +``` + +Note what's missing compared to the main-run wording above: no mention of output extending the +wait, no guidance toward the program's own quit/exit option -- a real interactive program hitting +this check during hidden-import recovery is still force-stopped at 30 seconds flat, output or not. ### Scenario 42: Argv passthrough (REQ-026) -- launch arguments through the bootstrapper diff --git a/run_setup.bat b/run_setup.bat index d8c959df..9b7d26fd 100644 --- a/run_setup.bat +++ b/run_setup.bat @@ -129,16 +129,27 @@ rem that an ODD number of backslashes right before a closing quote escapes the q rem of closing the string, silently corrupting the whole /C: argument (and swallowing the rem trailing ">nul" into the search pattern) so the match can never succeed. An EVEN count rem (here, two) collapses to a single literal backslash and the quote closes normally. +rem derived requirement: a CodeRabbit review finding (PR #417) -- HP_SCRIPT_ROOT is echoed +rem UNQUOTED into a pipe here; if it contains '&', cmd.exe's own parser (which does not +rem distinguish "this & came from a variable" from "this & was typed") treats it as a command +rem separator, splitting this single line into two commands and feeding findstr only a +rem truncated prefix -- silently defeating the guard for a script dropped under a path like +rem "C:\Users\Sales & Marketing\run_setup.bat". Quoting protects it: cmd.exe tracks quote state +rem left-to-right as it scans (including through %VAR% expansion), so a '&' landing inside the +rem quoted region is never treated as an operator. The literal quote characters `echo` leaves in +rem its own output (echo does not strip them, unlike most commands) don't affect the match -- +rem findstr's /C: pattern is a substring search, so it still finds "%WINDIR%\\" etc. regardless +rem of the extra leading/trailing quote characters surrounding it. if defined WINDIR ( - echo %HP_SCRIPT_ROOT%| findstr /I /C:"%WINDIR%\\" >nul + echo "%HP_SCRIPT_ROOT%"| findstr /I /C:"%WINDIR%\\" >nul if not errorlevel 1 set "HP_SYSDIR_HIT=1" ) if defined ProgramFiles ( - echo %HP_SCRIPT_ROOT%| findstr /I /C:"%ProgramFiles%\\" >nul + echo "%HP_SCRIPT_ROOT%"| findstr /I /C:"%ProgramFiles%\\" >nul if not errorlevel 1 set "HP_SYSDIR_HIT=1" ) if defined HP_PF86 ( - echo %HP_SCRIPT_ROOT%| findstr /I /C:"%HP_PF86%\\" >nul + echo "%HP_SCRIPT_ROOT%"| findstr /I /C:"%HP_PF86%\\" >nul if not errorlevel 1 set "HP_SYSDIR_HIT=1" ) set "HP_PF86=" @@ -391,10 +402,34 @@ call :define_helper_payloads for %%I in ("%CD%") do set "ENVNAME=%%~nI" rem derived requirement: conda env names reject characters like '~'; self env smoke rem scenarios run from tests\~envsmoke so normalize to ASCII word chars/_/-. +rem CLAUDE.md Item 26: '&' is special-cased to the bare word 'and' BEFORE the blanket +rem substitution below -- the blanket rule alone already avoids the real hazard (a raw '&' +rem confuses URL query-string parsing and renders oddly in Outlook), but collapses it to '_' +rem like any other stripped character, losing readability ("Sales & Marketing" -> a folder a +rem user might rename and email becoming "Sales___Marketing.exe" instead of the more legible +rem "Sales_and_Marketing.exe"). Deliberately a bare word (no surrounding underscores): the +rem existing spaces on either side of '&' are still converted to '_' by the blanket rule right +rem after, so "Sales & Marketing" -> "Sales and Marketing" -> "Sales_and_Marketing" without +rem this substitution needing to supply its own separators. +rem derived requirement: a CodeRabbit review finding on this same PR -- '&' -> 'and' is a 1-to-3 +rem character expansion, so a folder name unusually heavy in '&' could make the sanitized name +rem LONGER than the original (every other stripped character before this change was a 1-to-1 +rem substitution, never lengthening the result). Bounded to 64 chars post-substitution -- ample +rem for a real project folder name, well clear of Windows/conda env-name length limits for +rem everything this value later becomes (ENV_PATH, dist\.exe). 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" -if defined ENVNAME_SANITIZED set "ENVNAME=%ENVNAME_SANITIZED%" +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 ($san.Length -gt 64) { $san = $san.Substring(0, 64).TrimEnd('_', '-') }; if ([string]::IsNullOrWhiteSpace($san) -or ($san.Trim('_', '-').Length -eq 0)) { $san = 'env'; } [Console]::Write($san)"` ) do set "ENVNAME_SANITIZED=%%I" +rem derived requirement: fail CLOSED, not open -- if the PowerShell sanitization command itself +rem errored or emitted nothing (missing powershell.exe, execution-policy lockdown, etc.), silently +rem falling through to the raw, UNSANITIZED folder name would defeat this whole guard (a leading +rem hyphen or an embedded '&' would flow straight to `conda create -n` / the exported filename). +if defined ENVNAME_SANITIZED ( + set "ENVNAME=%ENVNAME_SANITIZED%" +) else ( + call :log "[WARN] REQ-004: env-name sanitization command produced no output; falling back to 'env' for safety." + set "ENVNAME=env" +) set "ENVNAME_SANITIZED=" rem derived requirement: a leading hyphen is replaced above because `conda create -n -foo` rem parses the name as a command-line flag (malformed); internal hyphens (my-app) are kept. diff --git a/tests/selfapps_envname.ps1 b/tests/selfapps_envname.ps1 index e135d34e..39669e20 100644 --- a/tests/selfapps_envname.ps1 +++ b/tests/selfapps_envname.ps1 @@ -1,11 +1,26 @@ # ASCII only -# selfapps_envname.ps1 - REQ-004 env-name sanitization edge case. +# selfapps_envname.ps1 - REQ-004 env-name sanitization edge cases. # -# A folder name starting with a hyphen must not flow through to `conda create -n -foo`, -# where argparse would treat "-foo" as a command-line flag (malformed). The sanitizer -# replaces a leading hyphen run with "_" (internal hyphens like my-app are preserved). -# Runs with HP_CI_SKIP_ENV=1 (no conda needed) and asserts the derived env name logged -# by run_setup.bat is conda-safe. +# Three scenarios (ENVNAME_SCENARIO env var; unset defaults to 'hyphen'): +# +# - 'hyphen' (default): a folder name starting with a hyphen must not flow through to +# `conda create -n -foo`, where argparse would treat "-foo" as a command-line flag +# (malformed). The sanitizer replaces a leading hyphen run with "_" (internal hyphens +# like my-app are preserved). +# - 'ampersand' (CLAUDE.md Item 26): '&' is special-cased to the bare word 'and' BEFORE +# the blanket [^A-Za-z0-9_-] -> '_' substitution runs, so a folder like "Sales & Marketing" +# sanitizes to the more legible "Sales_and_Marketing" instead of "Sales___Marketing" -- +# readability only, not a safety fix (the blanket substitution alone already prevented the +# real hazard: a raw '&' in the exported filename confusing URL query-string parsing or +# rendering oddly in Outlook). +# - 'longname' (CodeRabbit finding on PR #417, follow-up to Item 26): '&'->'and' is a 1-to-3 +# char expansion, unlike every other substitution in this sanitizer (all 1-to-1) -- an +# ampersand-heavy folder name can make the sanitized result LONGER than the original, so the +# sanitizer truncates to 64 chars post-substitution. This scenario is the only one that +# actually exercises that truncation branch. +# +# All three scenarios run with HP_CI_SKIP_ENV=1 (no conda needed) and assert the derived env +# name logged by run_setup.bat matches expectations. # # Lane: any (cheap, skip-env). param() @@ -26,18 +41,76 @@ function Write-NdjsonRow { Add-Content -LiteralPath $ciNd -Value $json -Encoding Ascii } -if (-not $IsWindows) { +$scenario = if ($env:ENVNAME_SCENARIO) { $env:ENVNAME_SCENARIO } else { 'hyphen' } +# derived requirement: reject an unrecognized ENVNAME_SCENARIO explicitly instead of silently +# running the hyphen scenario via a catch-all default -- a typo'd CI env var should fail loudly, +# not quietly substitute a different test and still report pass. Still emit a (failed) NDJSON +# row first -- a bare Write-Error/exit with no row at all is invisible to the NDJSON-based +# verdict/registry checks, which would then have no record of why this scenario never ran. +if ($scenario -notin @('hyphen', 'ampersand', 'longname')) { + Write-Error "selfapps_envname.ps1: unknown ENVNAME_SCENARIO value '$scenario' (expected 'hyphen', 'ampersand', or 'longname')." Write-NdjsonRow ([ordered]@{ - id='self.envname.hyphen'; req='REQ-004'; pass=$true - desc='Leading-hyphen folder name sanitized to a conda-safe env name (skipped on non-Windows)' - details=[ordered]@{ skip=$true; reason='non-windows-host' } + id='self.envname.hyphen'; req='REQ-004'; pass=$false + desc="unknown ENVNAME_SCENARIO value '$scenario'" + details=[ordered]@{ error='unrecognized-scenario'; scenario=$scenario } }) + exit 1 +} +switch ($scenario) { + 'ampersand' { + $folderName = 'Sales & Marketing' + $expectedName = 'Sales_and_Marketing' + $desc = "'&' special-cased to 'and' before the blanket sanitizer runs (readability, CLAUDE.md Item 26)" + } + 'longname' { + # derived requirement (CodeRabbit finding on PR #417): the '&'->'and' substitution is the + # first 1-to-3-char expansion in this sanitizer (every other substitution is 1-to-1), so a + # folder name unusually heavy in '&' could make the sanitized name LONGER than the + # original -- this scenario is the only one that actually exercises the resulting 64-char + # truncation bound (`run_setup.bat`'s ENVNAME_SANITIZED computation, CLAUDE.md Item 26). + # 15 repeats of "A & " (4 chars each) = 59 raw chars (short enough to stay well clear of + # Windows MAX_PATH once nested under tests\), expanding to "A and " (6 chars each) = 89 + # sanitized chars pre-truncation -- comfortably over the 64-char cap. + $folderName = ('A & ' * 15).TrimEnd() + $expectedName = ('A_and_' * 11).Substring(0, 64).TrimEnd('_', '-') + $desc = 'Over-64-char, ampersand-heavy folder name exercises the post-substitution truncation bound (CLAUDE.md Item 26)' + } + 'hyphen' { + $folderName = '-hyphen-start' + $expectedName = '_hyphen-start' + $desc = 'Leading-hyphen folder name sanitized to a conda-safe env name (no leading hyphen)' + } +} + +if (-not $IsWindows) { + # derived requirement: tools/check_ndjson_registry.py's static scan matches a literal + # id='...' hashtable-literal pattern (\bid\s*=\s*['"]...['"]), not a variable reference -- + # each scenario's id must appear as a literal string at its own Write-NdjsonRow call site, + # not only assigned to $rowId above, or the row is (falsely) flagged as doc-only/unemitted. + if ($scenario -eq 'ampersand') { + Write-NdjsonRow ([ordered]@{ + id='self.envname.ampersand'; req='REQ-004'; pass=$true + desc="$desc (skipped on non-Windows)" + details=[ordered]@{ skip=$true; reason='non-windows-host' } + }) + } elseif ($scenario -eq 'longname') { + Write-NdjsonRow ([ordered]@{ + id='self.envname.longname'; req='REQ-004'; pass=$true + desc="$desc (skipped on non-Windows)" + details=[ordered]@{ skip=$true; reason='non-windows-host' } + }) + } else { + Write-NdjsonRow ([ordered]@{ + id='self.envname.hyphen'; req='REQ-004'; pass=$true + desc="$desc (skipped on non-Windows)" + details=[ordered]@{ skip=$true; reason='non-windows-host' } + }) + } exit 0 } $batchPath = Join-Path $repo 'run_setup.bat' -# Leaf folder name deliberately starts with a hyphen. -$workDir = Join-Path $here '-hyphen-start' +$workDir = Join-Path $here $folderName if (Test-Path -LiteralPath $workDir) { Remove-Item -LiteralPath $workDir -Recurse -Force } New-Item -ItemType Directory -Force -Path $workDir | Out-Null Copy-Item -LiteralPath $batchPath -Destination $workDir -Force @@ -65,27 +138,95 @@ $logText = if (Test-Path $logPath) { Get-Content -LiteralPath $logPath -Raw - $setupTxt = if (Test-Path $setupLog) { Get-Content -LiteralPath $setupLog -Raw -Encoding ASCII } else { '' } $combined = $logText + "`n" + $setupTxt -# Expected sanitized name: leading "-" -> "_", internal hyphens preserved. -$expectedName = '_hyphen-start' -$sawExpected = $combined -match [regex]::Escape("Environment name: $expectedName") -# Guard: the env name must never be logged with a leading hyphen. -$sawBadLeading = $combined -match 'Environment name:\s+-' +# derived requirement: case-sensitive, line-anchored match -- a loose case-insensitive substring +# search could false-positive on an incorrectly-cased value that happens to still contain the +# right characters. +$expectedPattern = "(?m)^[^\r\n]*Environment name:[ \t]+$([regex]::Escape($expectedName))[ \t]*(?:\r?$)" +$sawExpected = $combined -cmatch $expectedPattern +# Guard, scenario-specific: the hyphen case must never log a leading hyphen; the ampersand case +# must never log a raw, unsubstituted '&' anywhere on the "Environment name:" line (proves the +# special-case actually ran, not just that the blanket rule alone happened to produce a readable +# result by coincidence). Matches anywhere later on the line, not just immediately after the +# colon -- a raw, unsubstituted "Sales & Marketing" has a space (not a non-space run) before the +# '&', which an earlier \S*&-shaped guard would have missed entirely. +$sawBadPattern = switch ($scenario) { + 'ampersand' { $combined -match '(?m)^[^\r\n]*Environment name:[^\r\n]*&' } + 'longname' { $combined -match '(?m)^[^\r\n]*Environment name:[^\r\n]*&' } + default { $combined -match 'Environment name:\s+-' } +} -$pass = ($exit -eq 0) -and $sawExpected -and (-not $sawBadLeading) +# derived requirement (CodeRabbit finding on PR #417): the 'longname' scenario's whole point is +# proving the 64-char truncation branch actually ran, not just that SOME value was logged -- +# extract the logged name via a capture group and assert its length explicitly, decoupled from +# the exact-string match above (which would also pass, coincidentally, if truncation were broken +# in a way that still produced a 64-char string equal to $expectedName by chance). +$loggedNameLen = $null +$sawTruncationBound = $true +if ($scenario -eq 'longname') { + $captureMatch = [regex]::Match($combined, '(?m)^[^\r\n]*Environment name:[ \t]+([^\r\n \t]+)[ \t]*(?:\r?$)') + if ($captureMatch.Success) { $loggedNameLen = $captureMatch.Groups[1].Value.Length } + $sawTruncationBound = ($null -ne $loggedNameLen) -and ($loggedNameLen -le 64) +} -Write-NdjsonRow ([ordered]@{ - id='self.envname.hyphen' - req='REQ-004' - pass=$pass - desc='Leading-hyphen folder name sanitized to a conda-safe env name (no leading hyphen)' - details=[ordered]@{ - exitCode = $exit - expectedName = $expectedName - sawExpected = $sawExpected - sawBadLeading = $sawBadLeading - log = $bootstrapLog - } -}) +# derived requirement: this scenario deliberately does NOT attempt to prove the truncated name +# flows into a real conda ENV_PATH / `conda create -n ` -- like its 'hyphen'/'ampersand' +# siblings, it runs with HP_CI_SKIP_ENV=1 (system Python, no conda) specifically to stay cheap and +# lane-agnostic; ENV_PATH is only ever assigned on the real conda/uv/venv code paths this harness +# intentionally bypasses. What IS proven here: ENVNAME_SANITIZED is computed once, near the very +# top of run_setup.bat, and unconditionally becomes ENVNAME (`set "ENVNAME=%ENVNAME_SANITIZED%"`) +# before ANY provider-specific branch -- so a value confirmed correct here is, by construction, +# the same value every later `%ENVNAME%` substitution (including `ENV_PATH=...\envs\%ENVNAME%` +# and `dist\%ENVNAME%.exe`) will use. Proving the truncated value with a real conda env create is +# already covered by every other test in this file/suite that exercises a full provider build. + +$pass = ($exit -eq 0) -and $sawExpected -and (-not $sawBadPattern) -and $sawTruncationBound + +# derived requirement: same literal-id-per-call-site reasoning as the non-Windows skip block above. +if ($scenario -eq 'ampersand') { + Write-NdjsonRow ([ordered]@{ + id='self.envname.ampersand' + req='REQ-004' + pass=$pass + desc=$desc + details=[ordered]@{ + exitCode = $exit + expectedName = $expectedName + sawExpected = $sawExpected + sawBadPattern = $sawBadPattern + log = $bootstrapLog + } + }) +} elseif ($scenario -eq 'longname') { + Write-NdjsonRow ([ordered]@{ + id='self.envname.longname' + req='REQ-004' + pass=$pass + desc=$desc + details=[ordered]@{ + exitCode = $exit + expectedName = $expectedName + sawExpected = $sawExpected + sawBadPattern = $sawBadPattern + loggedNameLen = $loggedNameLen + sawTruncationBound = $sawTruncationBound + log = $bootstrapLog + } + }) +} else { + Write-NdjsonRow ([ordered]@{ + id='self.envname.hyphen' + req='REQ-004' + pass=$pass + desc=$desc + details=[ordered]@{ + exitCode = $exit + expectedName = $expectedName + sawExpected = $sawExpected + sawBadPattern = $sawBadPattern + log = $bootstrapLog + } + }) +} if (-not $pass) { exit 1 } exit 0