Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/batch-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ jobs:
run: |
& tests\selfapps_preflight.ps1

- name: "Self-test: line-ending self-check preflight branches (real/conda-full only)"
- name: "Self-test: early preflight branches -- line-ending + writable-CWD (real/conda-full only)"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if: ${{ !cancelled() && (matrix.mode == 'real' || (matrix.mode == 'conda-full' && steps.conda_avail.outputs.available == 'true')) }}
shell: pwsh
run: |
Expand Down Expand Up @@ -2137,6 +2137,10 @@ jobs:
tests\~selftest_lineending_lf_only\~lineending_bootstrap.log
tests/~selftest_lineending_lf_only/~bootstrap.status.json
tests\~selftest_lineending_lf_only\~bootstrap.status.json
tests/~selftest_lineending_cwd_not_writable/~lineending_bootstrap.log
tests\~selftest_lineending_cwd_not_writable\~lineending_bootstrap.log
tests/~selftest_lineending_cwd_not_writable/~bootstrap.status.json
tests\~selftest_lineending_cwd_not_writable\~bootstrap.status.json
tests/~selftest_entrysmoke_no_interpreter/~entrysmoke_no_interpreter_bootstrap.log
tests\~selftest_entrysmoke_no_interpreter\~entrysmoke_no_interpreter_bootstrap.log
tests/~selftest_entrysmoke_no_interpreter/~setup.log
Expand Down
8 changes: 0 additions & 8 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1052,14 +1052,6 @@ way (no live Windows execution available here), that is noted explicitly rather
plain-language message naming Constrained Language Mode specifically if it fails, rather than
letting the failure surface piecemeal as five-plus separate "Could not write ~x" messages later.

- **Item 48: no writable-CWD preflight; `:merge_git_config` writes `.gitignore`/`.gitattributes`
into the app folder before any guard checks the folder is actually writable.** Small, isolated.
`:merge_git_config` (called near the top of the file, before `:acquire_lock`) is the first thing in the file
that writes to the app directory itself, and its own write failures are not checked. Fix: a
cheap `type nul > "~wtest.tmp"` + errorlevel check, with a named message pointing at the folder,
placed before `:merge_git_config`'s own call site (right after Item 44's line-ending check is a
natural spot, since both are "can this even run here at all" preconditions).

- **Item 49: `:lock_is_stale`'s indeterminate PowerShell result is silently treated as "fresh"
(lock held by a live instance), producing a false "another instance of this setup appears to be
running" message instead of a graceful continue.** CONFIRMED directly against current source
Expand Down
80 changes: 80 additions & 0 deletions docs/agent-closed-backlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2231,6 +2231,86 @@ run of the same regex logic before landing, not just reasoned about).
`HP_TEST_SYSCON_ANSWER` is likewise only tested via its deterministic override, not by separately
proving the `HP_CI_LANE` branch resolves without hanging).

### Item 48 (closed 2026-08-18)

- **No writable-CWD preflight; `:merge_git_config` and other early writes could fail silently
before any guard confirmed the app folder was actually writable.** CONFIRMED directly against
source: `type nul > "%HP_CI_MARKER%" 2>nul` and `if not exist "%LOG%" (type nul > "%LOG%")`
both wrote to the app folder with no errorlevel check, ahead of `:merge_git_config`'s own first
write -- none of them would have surfaced a clear message for a genuinely unwritable folder
(a read-only network share, a permissions-locked directory), just a cascade of confusing
downstream failures.

**Fix shipped**: a new writable-CWD preflight right after the `cd /d "%~dp0"` block (before
`HP_SCRIPT_ROOT` is even set, ahead of every other write in the file) attempts `type nul >
"~wtest.tmp" 2>nul` and checks `if not exist` -- on failure, a named `[ERROR]` message points at
`%CD%` and tells the user to move the script to a writable folder, writes
`HP_PREFLIGHT_STATUS` (the same early-preflight status file the line-ending checks already use,
since `:write_status`'s own machinery does not exist yet at this point), pauses for a real
interactive user, and exits 1. On success the probe file is deleted immediately.
`HP_TEST_FORCE_CWD_NOT_WRITABLE` forces the branch deterministically in CI by skipping the real
write attempt entirely (so `~wtest.tmp` is never created) rather than revoking filesystem
permissions on a shared runner -- same forcing technique as `HP_TEST_FORCE_NO_POWERSHELL`.

**Regression coverage folded into the existing line-ending preflight test file** rather than a
new one: `tests/selfapps_lineending_check.ps1`'s `Test-PreflightScenario` helper already covers
the identical shape (env-flag-forced branch, exit code, status.json state/exitCode, expected
log substrings), so a fourth scenario (`self.preflight.cwd_not_writable`) reuses it directly,
with a new `-Req` parameter so its NDJSON row correctly cites `CLAUDE.md-Item-48` instead of the
file's original `CLAUDE.md-Item-44`. The CI step name and file header comment were both updated
to describe the file as covering "early preflight branches" generally, not line-endings alone.

**Refined via CodeRabbit's review on PR #442, three real findings fixed same-day.** (1) The
probe never cleared a pre-existing `~wtest.tmp` before attempting the write -- a crash between
an earlier successful probe and its own cleanup (or any other leftover at that exact path)
would make a genuinely unwritable folder read as writable, since `if not exist` after a no-op
forced branch would find the STALE file still present. Fixed by unconditionally `del /f /q
"~wtest.tmp" >nul 2>&1` immediately before the probe attempt, so the existence check afterward
is always meaningful regardless of prior-run leftovers. (2) `%CD%` was echoed unquoted in the
`[ERROR]` message -- the same `:log`-echoes-UNQUOTED hazard class documented in
`docs/agent-lessons-learned.md` (a folder path containing `&`/`|`/`<`/`>`, all legal in a
Windows folder name, would have been misparsed as a shell metacharacter instead of literal
text). Fixed by wrapping it in quotes at the echo site (`"%CD%"`), the same fix pattern already
established for the `findstr`-piped `HP_SCRIPT_ROOT` case. (3) The NDJSON `req`/`desc` fields on
the non-Windows-skip and missing-`run_setup.bat` early-exit branches still hardcoded
`CLAUDE.md-Item-44`/"Line-ending self-check..." for ALL FOUR row ids, including the new
`cwd_not_writable` row (Item 48) -- those two branches predate the fourth scenario and never
got updated when it was added. Fixed by replacing the flat `$rowIds` array with an ordered
`$rowIdReqs` map (id -> its own correct `req`), consumed by both branches so every row cites
its real backlog item even on paths that never reach `Test-PreflightScenario`'s own `-Req`
parameter. Also added the scenario's own work-directory name to `ExpectedSubstrings`, so a
regression that drops or corrupts `%CD%` from the error message is actually caught (previously
only the fixed prefix/remediation text was asserted). Deliberately NOT implemented: CodeRabbit's
suggestion to treat the FINAL cleanup `del`'s own failure as a hard error -- disproportionate
for a gitignored scratch probe file with no established precedent elsewhere in this file (every
other tilde-prefixed scratch file in `run_setup.bat` uses the same best-effort `>nul 2>&1`
cleanup convention); a `~wtest.tmp` a delete could conceivably strand is harmless clutter, not a
reason to fail the whole bootstrap. Also deliberately NOT adding a `%RANDOM%`/timestamp suffix
to the probe filename (a Blinter SEC017 finding, not a CodeRabbit one): the race/hijack threat
model that rule targets is a SHARED multi-user temp directory (e.g. `/tmp`), not a folder the
calling user already fully owns, which is what `~wtest.tmp` always is here -- matches every
other bare tilde-prefixed scratch filename already used throughout this file.

**A fourth finding on the same thread, caught by CodeRabbit re-inspecting the fix itself
(not the original review pass): `del /f /q` only removes a FILE, never a directory.** If
`~wtest.tmp` happened to already exist as a DIRECTORY (not a file) at the exact probe path,
the pre-probe `del` would be a silent no-op against it, the real `type nul > "~wtest.tmp"`
write attempt would then fail (can't create a file where a directory of the same name already
exists), but `if not exist "~wtest.tmp"` would still find the surviving directory and read the
preflight as SUCCESSFUL -- the exact false-writable outcome the stale-file fix above was meant
to close, just for the other filesystem-entry type. Fixed by adding `if exist "~wtest.tmp" rd
/s /q "~wtest.tmp" >nul 2>&1` right after the existing `del`, so whichever of the two survives
the first attempt is cleared by the second -- `rd` is a no-op against a plain file (by design,
it only removes directories), so ordering `del` then a conditional `rd` handles both shapes of
stale leftover without needing to first detect which one is present. Deliberately NOT adding a
dedicated regression scenario for this exact case: reproducing it needs a directory literally
named `~wtest.tmp` to already exist in a completely fresh app folder before that folder's very
first bootstrap run ever executes -- and proving it would require extending
`Test-PreflightScenario`'s own shape to a new "pre-seed an anomaly, then expect the OVERALL run
to still succeed" mode, distinct from every existing scenario in that file (all four assert a
forced FAILURE), for a real-world trigger rate low enough that the underlying source fix (cheap,
two extra lines) was judged sufficient on its own.

## Known Findings (diagnosed, no action warranted)

- **Backlog item numbering: renumber-on-collision convention dropped, 2026-07-31 owner decision.**
Expand Down
14 changes: 12 additions & 2 deletions docs/agent-ndjson.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ self.collect.submodules,
self.exe.hidden_import, self.exe.hidden_import.exhaust,
self.preflight.syntax,
self.preflight.no_powershell, self.preflight.ps_check_fail, self.preflight.lf_only,
self.preflight.cwd_not_writable,
self.entrysmoke.no_interpreter_guard,
self.cascade.detect, self.cascade.consent, self.cascade.timed,
self.cascade.exec (uv lane only -- selfapps_cascade.ps1; non-gating),
Expand Down Expand Up @@ -936,12 +937,21 @@ PowerShell 7 binary before being wired into `run_setup.bat`, not just reasoned a

Lane: `real` and `conda-full` only, gating from first landing -- matches `self.preflight.syntax`'s
own precedent for a cheap, provider-agnostic, pure-batch preflight check (no environment or
dependency work is ever reached in any of these three scenarios, unlike the Nuitka/MSVC-dependent
dependency work is ever reached in any of these scenarios, unlike the Nuitka/MSVC-dependent
tests elsewhere in this registry that start non-gating specifically because they could not be
verified locally).

**Extended for CLAUDE.md Active Backlog Item 48 (`self.preflight.cwd_not_writable`).** The
writable-CWD preflight sits right after these three checks in `run_setup.bat` (before
`:merge_git_config`'s own first write to the app folder) and is the same class of "can this even
run here at all" precondition, so its regression coverage lives in this same file, reusing the
same `Test-PreflightScenario` helper. Its hook, `HP_TEST_FORCE_CWD_NOT_WRITABLE`, skips the real
`type nul > ~wtest.tmp` write attempt entirely (rather than revoking filesystem permissions on a
shared CI runner), producing the same "not found" signal a genuine write failure would.

```
self.preflight.no_powershell, self.preflight.ps_check_fail, self.preflight.lf_only
self.preflight.no_powershell, self.preflight.ps_check_fail, self.preflight.lf_only,
self.preflight.cwd_not_writable
```

---
Expand Down
36 changes: 36 additions & 0 deletions run_setup.bat
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,42 @@ if errorlevel 1 (
echo [ERROR] Workspace path invalid: %~dp0
exit /b 1
)
rem derived requirement: writable-CWD preflight (CLAUDE.md Item 48) -- this is the first
rem point in the file safe to attempt a real write (CWD is now confirmed to be the app
rem folder itself, via the cd /d above), and it runs before every other write in this file
rem (the CI marker, ~setup.log, and :merge_git_config's own .gitignore/.gitattributes
rem writes) so an unwritable folder fails with one clear, named message instead of a
rem confusing cascade of silently-swallowed write failures later.
rem derived requirement: clear any pre-existing ~wtest.tmp before probing -- a crash between
rem a past successful probe and its own cleanup (or, unlikely given the tilde convention, a
rem coincidentally-named leftover) must not let a stale entry at this exact path masquerade as
rem this run's own successful write. del only removes a FILE at this path; if a directory of
rem the same name exists instead (however unlikely), del is a silent no-op against it, so rd
rem /s /q runs too whenever the entry is still present afterward -- between the two, either
rem shape of stale leftover is cleared before the real probe below ever runs.
del /f /q "~wtest.tmp" >nul 2>&1
if exist "~wtest.tmp" rd /s /q "~wtest.tmp" >nul 2>&1
if defined HP_TEST_FORCE_CWD_NOT_WRITABLE (
rem derived requirement: force the not-writable branch deterministically for CI, without
rem actually revoking filesystem permissions on a shared runner -- skip the real write
rem attempt entirely so ~wtest.tmp is never created, the same signal a genuine write
rem failure would produce. Intentionally empty otherwise: doing nothing IS the forcing
rem mechanism here.
) else (
type nul > "~wtest.tmp" 2>nul
)
if not exist "~wtest.tmp" (
echo ***
echo *** [ERROR] This folder does not appear to be writable: "%CD%"
echo *** This script needs to create files here -- logs, a dependency cache, and
echo *** eventually a standalone program. Move this script and your .py files to
echo *** a folder you have write access to, then run it again.
echo ***
echo {"state":"error","exitCode":1,"pyFiles":0}> "%HP_PREFLIGHT_STATUS%"
if not defined HP_CI_LANE ( pause )
exit /b 1
)
del /f /q "~wtest.tmp" >nul 2>&1
Comment thread
coderabbitai[bot] marked this conversation as resolved.
set "HP_SCRIPT_ROOT=%~dp0"
for %%R in ("%HP_SCRIPT_ROOT%") do set "HP_SCRIPT_ROOT=%%~fR"
if not "%HP_SCRIPT_ROOT:~-1%"=="\" set "HP_SCRIPT_ROOT=%HP_SCRIPT_ROOT%\"
Expand Down
47 changes: 36 additions & 11 deletions tests/selfapps_lineending_check.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,17 @@
# unmodified PowerShell command genuinely throw (FileNotFoundException) and hit its own
# catch{exit 2} branch, exercising the real failure path rather than a simulated one.
#
# Extended for CLAUDE.md Active Backlog Item 48: the writable-CWD preflight (right after
# these three checks, before :merge_git_config's own first write to the app folder) is the
# same class of "can this even run here at all" precondition, so its own regression coverage
# lives here too rather than in a new file -- same Test-PreflightScenario helper, same
# real/conda-full lanes, same status.json/exit-code assertion shape. Its own hook,
# HP_TEST_FORCE_CWD_NOT_WRITABLE, skips the real `type nul > ~wtest.tmp` write attempt
# entirely rather than revoking filesystem permissions on a shared CI runner.
#
# Lane: real and conda-full only (matches self.preflight.syntax's own precedent for a cheap,
# provider-agnostic, pure-batch preflight check -- no environment/dependency work is ever
# reached in any of these three scenarios, so there is no "could not verify locally" risk
# reached in any of these four scenarios, so there is no "could not verify locally" risk
# comparable to a real Nuitka/MSVC build; the command logic itself was verified directly
# against a real PowerShell 7 binary before this file was written).
param()
Expand All @@ -37,20 +45,29 @@ function Write-NdjsonRow {
Add-Content -LiteralPath $ciNd -Value $json -Encoding Ascii
}

$rowIds = @('self.preflight.no_powershell', 'self.preflight.ps_check_fail', 'self.preflight.lf_only')
# derived requirement: a map, not a flat array, so the non-Windows/missing-run_setup.bat
# branches below can still cite each row's OWN correct backlog item -- the cwd_not_writable
# row belongs to Item 48, not the three line-ending rows' Item 44, even on these early-exit
# paths where Test-PreflightScenario itself (which takes -Req per call) never runs.
$rowIdReqs = [ordered]@{
'self.preflight.no_powershell' = 'CLAUDE.md-Item-44'
'self.preflight.ps_check_fail' = 'CLAUDE.md-Item-44'
'self.preflight.lf_only' = 'CLAUDE.md-Item-44'
'self.preflight.cwd_not_writable' = 'CLAUDE.md-Item-48'
}

# Non-Windows skip
# derived requirement: $IsWindows is undefined (reads as $null, so "-not $IsWindows" is
# always true) under Windows PowerShell 5.1 -- it was only introduced in PowerShell 6+.
# [System.Environment]::OSVersion.Platform works identically on 5.1 and 7+.
$platform = [System.Environment]::OSVersion.Platform.ToString()
if ($platform -ne 'Win32NT') {
foreach ($id in $rowIds) {
foreach ($id in $rowIdReqs.Keys) {
Write-NdjsonRow ([ordered]@{
id = $id
req = 'CLAUDE.md-Item-44'
req = $rowIdReqs[$id]
pass = $true
desc = 'Line-ending self-check preflight branch (skipped on non-Windows)'
desc = 'Preflight self-check branch (skipped on non-Windows)'
details = [ordered]@{ skip = $true; platform = $platform; reason = 'non-windows-host' }
})
}
Expand All @@ -59,12 +76,12 @@ if ($platform -ne 'Win32NT') {

$batchPath = Join-Path $repo 'run_setup.bat'
if (-not (Test-Path $batchPath)) {
foreach ($id in $rowIds) {
foreach ($id in $rowIdReqs.Keys) {
Write-NdjsonRow ([ordered]@{
id = $id
req = 'CLAUDE.md-Item-44'
req = $rowIdReqs[$id]
pass = $false
desc = 'Line-ending self-check preflight branch: run_setup.bat not found'
desc = 'Preflight self-check branch: run_setup.bat not found'
details = [ordered]@{ error = 'run_setup.bat not found at ' + $batchPath }
})
}
Expand All @@ -78,7 +95,8 @@ function Test-PreflightScenario {
[string]$EnvFlagName,
[int]$ExpectedExit,
[string[]]$ExpectedSubstrings,
[switch]$WriteLfOnlySentinel
[switch]$WriteLfOnlySentinel,
[string]$Req = 'CLAUDE.md-Item-44'
)

$workDir = Join-Path $here $WorkDirName
Expand Down Expand Up @@ -146,9 +164,9 @@ function Test-PreflightScenario {

Write-NdjsonRow ([ordered]@{
id = $Id
req = 'CLAUDE.md-Item-44'
req = $Req
pass = $pass
desc = "Line-ending self-check preflight branch: $EnvFlagName"
desc = "Preflight self-check branch: $EnvFlagName"
details = [ordered]@{
bootstrapExit = $runExit
expectedExit = $ExpectedExit
Expand Down Expand Up @@ -185,5 +203,12 @@ $results += Test-PreflightScenario -Id 'self.preflight.lf_only' `
-ExpectedSubstrings @('[ERROR] This copy of run_setup.bat has invalid line endings.', 'Easiest fix: delete this copy and download run_setup.bat again') `
-WriteLfOnlySentinel

$results += Test-PreflightScenario -Id 'self.preflight.cwd_not_writable' `
-WorkDirName '~selftest_lineending_cwd_not_writable' `
-EnvFlagName 'HP_TEST_FORCE_CWD_NOT_WRITABLE' `
-ExpectedExit 1 `
-ExpectedSubstrings @('[ERROR] This folder does not appear to be writable:', '~selftest_lineending_cwd_not_writable', 'Move this script and your .py files to') `
-Req 'CLAUDE.md-Item-48'

Comment thread
coderabbitai[bot] marked this conversation as resolved.
if ($results -contains $false) { exit 1 }
exit 0
Loading