diff --git a/CLAUDE.md b/CLAUDE.md index 5d605e09..7860cda2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -567,22 +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 25: `:dll_bundle_recover` reports `repaired` instead of a distinct `exhausted` outcome - when a locatable DLL candidate is found after the 3-iteration cap is already hit.** Found via a - CodeRabbit review round on PR #414 (Item 24's own PR), same day as Item 24's real CI diagnosis. - `:dll_bundle_loop` finds the next candidate (`HP_NEXT_DLL`) BEFORE checking - `if %HP_DLL_ITER% GEQ 3` -- so when a 4th locatable DLL exists, that information is silently - discarded (the loop just `goto :dll_bundle_recover_done`), and at `:dll_bundle_recover_done`, the - `if %HP_DLL_ITER% GEQ 1` check reads TRUE (3 DLLs were already genuinely bundled), so it emits - `[REPAIR][DLL_BUNDLE] Native-DLL bundling complete` and the `repaired` NDJSON state, even though - a real candidate was left unbundled. Needs a distinct `exhausted` outcome (mirroring - `:hidden_import_recover`'s own `self.exe.hidden_import.exhaust` precedent) that does NOT claim - completion, plus a matching `docs/agent-ndjson.md` registration and a `tests/harness.ps1` call- - site-count update. Deliberately NOT fixed inside Item 24's already-large PR -- this needs its own - focused loop with its own test coverage. Low real-world trigger rate (needs 4+ conda-forge - packages under ONE PyInstaller build to each separately need `--add-binary`, not yet observed for - any real package in this repo's testing), so not urgent, but a genuine correctness gap. - - **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 diff --git a/docs/agent-closed-backlog.md b/docs/agent-closed-backlog.md index fbaef90f..1a975ad3 100644 --- a/docs/agent-closed-backlog.md +++ b/docs/agent-closed-backlog.md @@ -1525,6 +1525,43 @@ this belongs to). genuinely new, separately-scoped finding, not a continuation of the DLL-bundling work -- filed as its own Active Backlog item rather than reopening this one. See that item for the full trace. +### Item 25 (closed 2026-08-08) + +- **`:dll_bundle_recover` reported `repaired` instead of a distinct `exhausted` outcome when a + locatable DLL candidate was found after the 3-iteration cap was already hit.** Found via a + CodeRabbit review round on PR #414 (Item 24's own PR), same day as Item 24's real CI diagnosis. + `:dll_bundle_loop` found the next candidate (`HP_NEXT_DLL`) BEFORE checking + `if %HP_DLL_ITER% GEQ 3` -- so when a 4th locatable DLL existed, that information was silently + discarded (the loop just `goto :dll_bundle_recover_done`), and at `:dll_bundle_recover_done`, the + `if %HP_DLL_ITER% GEQ 1` check read TRUE (3 DLLs were already genuinely bundled), so it emitted + `[REPAIR][DLL_BUNDLE] Native-DLL bundling complete` and the `repaired` NDJSON state, even though + a real candidate was left unbundled. + **Fixed**, in its own dedicated follow-up loop as originally planned (deliberately not folded + into Item 24's already-large PR): a new `HP_DLL_EXHAUSTED` flag is set in `:dll_bundle_loop`'s + cap-check branch -- reaching that branch at all already means a real candidate was found (the + earlier `if not defined HP_NEXT_DLL` early-return rules out the empty case), so setting the flag + there needs no extra detection logic. `:dll_bundle_recover_done` now checks `HP_DLL_EXHAUSTED` + BEFORE the `HP_DLL_ITER GEQ 1` "repaired" branch: logs a new + `[WARN][DLL_BUNDLE] Native-DLL bundling reached its 3-attempt cap with another native-DLL + dependency still detected...` line (reusing the already-sanitized `HP_DLL_DETECTED_SAFE` from + `:dll_bundle_recover`'s own entry, so no new sanitize pass is needed) and emits the new + `exhausted` NDJSON state instead of `repaired`. `exhausted` is `pass:true` under + `:emit_dll_bundle_row`'s existing `$pass = -not ($state -like 'failed_*')` formula, the same + treatment as `unlocatable` -- both are informational (the bootstrap itself did not fail, only the + repair was incomplete); the EXE smoke run afterward remains the actual arbiter of whether the + still-missing DLL matters at runtime. `HP_DLL_EXHAUSTED` is reset at `:dll_bundle_recover`'s own + top-of-loop reset block (alongside `HP_DLL_FAILED`) and cleared again at + `:dll_bundle_recover_exit`, mirroring `HP_DLL_FAILED`'s own cascade-re-entry-safe reset pattern + so a stale `exhausted` outcome from an earlier `:dll_bundle_recover` call cannot leak into a + later call's outcome during a REQ-009 provider cascade re-entry. + `docs/agent-ndjson.md` and `docs/agent-interconnect.md` updated with the 7th state and the full + mechanism trace; `tests/harness.ps1`'s `batch.dll_bundle.ndjson` extended with the new state + token and 7th call site (the same "static wiring guard, not runtime execution" coverage already + accepted for 4 of this subroutine's other states, per that same low real-world trigger rate -- + needs 4+ conda-forge packages under ONE PyInstaller build to each separately need + `--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). + ## Closed Backlog - **Cascade-vs-postexec fix (Active Backlog item 9), 2026-07-25, owner-directed follow-up to a diff --git a/docs/agent-interconnect.md b/docs/agent-interconnect.md index 0641eded..0b8af245 100644 --- a/docs/agent-interconnect.md +++ b/docs/agent-interconnect.md @@ -352,20 +352,61 @@ condition (alongside embedded `"` characters) for preferring `-File` over `-Comm **A companion CodeRabbit finding on the same review round: the loop's detected/skipped/repaired/ unlocatable/failed outcomes previously reached only `:log`'s console text, with no machine-readable record.** Fixed with a new shared subroutine, `:emit_dll_bundle_row`, called from -all 6 outcome points (`skipped_nuitka`/`skipped_non_conda`/`repaired`/`unlocatable`/ +all 7 outcome points (`skipped_nuitka`/`skipped_non_conda`/`repaired`/`unlocatable`/`exhausted`/ `failed_rebuild`/`failed_missing_exe`) and emitting NDJSON id `self.dll_bundle.recover` -- see `docs/agent-ndjson.md` for the full field list and the `HP_NDJSON`-scoping caveat (this row is not currently observed in `self.layered_e2e.chain`'s own artifact, since that test's isolated sub-bootstrap leaves `HP_NDJSON` unset by the same established convention `selfapps_postexec_ checkpoint.ps1` already uses; `tests/harness.ps1`'s new `batch.dll_bundle.ndjson` static check is the actual coverage for this row's wiring). The state name is passed as a `call` argument (safe -- -always one of the 6 literal tokens above, written directly in `run_setup.bat`, never derived from +always one of the 7 literal tokens above, written directly in `run_setup.bat`, never derived from external content), but the DLL name/provider/iteration are pulled INSIDE the emitting PowerShell command via `[Environment]::GetEnvironmentVariable(...)` rather than `%VAR%` cmd.exe substitution into the `-Command` text -- protecting cmd.exe's OWN command-line parsing (`&`/`|` are metacharacters even inside a quoted `call` argument) the same way the `_SAFE` display variables protect `:log`'s unquoted echo, just at a different vulnerable site. +**A NINTH real bug found via a CodeRabbit review round on PR #414 itself (CLAUDE.md Item 25, +fixed in a dedicated follow-up loop): `:dll_bundle_loop` found the next candidate (`HP_NEXT_DLL`) +BEFORE checking `if %HP_DLL_ITER% GEQ 3`, silently discarding it instead of reporting an +`exhausted` outcome.** When a 4th (or later) locatable DLL exists after the 3-iteration cap is +already hit, the loop just `goto :dll_bundle_recover_done` with the candidate's own name and path +already sitting in `HP_NEXT_DLL`/`HP_NEXT_DLL_PATH` from that iteration's scan, but with nothing +recording that a real candidate was left unbundled. At `:dll_bundle_recover_done`, the +`if %HP_DLL_ITER% GEQ 1` check reads TRUE (3 DLLs were already genuinely bundled in earlier +iterations), so it logged `[REPAIR][DLL_BUNDLE] Native-DLL bundling complete` and emitted the +`repaired` NDJSON state -- indistinguishable from a clean, fully-resolved run, even though a real, +on-disk DLL was never bundled and the EXE could still fail to load it. Fixed with a new +`HP_DLL_EXHAUSTED` flag, set at the cap-check branch in `:dll_bundle_loop` (the early +`if not defined HP_NEXT_DLL` return above it already rules out the empty-candidate case, so +reaching the cap check at all means a real candidate was found and about to be discarded) and +checked at `:dll_bundle_recover_done` BEFORE the `HP_DLL_ITER GEQ 1` "repaired" branch -- emits a +new `[WARN][DLL_BUNDLE]` line and the `exhausted` NDJSON state instead, reusing the already- +sanitized `HP_DLL_DETECTED_SAFE` (the first warning seen this run, computed once at +`:dll_bundle_recover`'s own entry) to safely name at least one detected dependency without a new +sanitize pass. `exhausted` is `pass:true` under `:emit_dll_bundle_row`'s existing +`$pass = -not ($state -like 'failed_*')` formula -- same treatment as `unlocatable`, since neither +state means the bootstrap itself failed, only that the repair was incomplete; the EXE smoke run +afterward is the actual arbiter of whether the missing DLL matters at runtime. Mirrors +`:hidden_import_recover`'s own `[WARN][HIDDEN_IMPORT] Auto-recovery exhausted after 3 attempts` +precedent in spirit (a distinct "gave up, did not fully resolve" signal), though that sibling +loop's exhaustion is detected via a runtime EXE re-run rather than a build-time scan, so the two +mechanisms are not literally shared code. `HP_DLL_EXHAUSTED` is reset at `:dll_bundle_recover`'s +own top-of-loop reset block (alongside `HP_DLL_FAILED`) and cleared again at +`:dll_bundle_recover_exit`, mirroring `HP_DLL_FAILED`'s own cascade-re-entry-safe reset pattern +(see the CodeRabbit finding above about `HP_NEXT_DLL`/`HP_DLL_ITER` leaking across a REQ-009 +cascade re-entry) -- a stale `exhausted` outcome from an EARLIER `:dll_bundle_recover` call could +otherwise leak into a LATER call's own outcome the same way that earlier bug did. Low real-world +trigger rate (needs 4+ conda-forge packages under ONE PyInstaller build to each separately need +`--add-binary`, not yet observed for any real package in this repo's testing) -- covered only by +`tests/harness.ps1`'s `batch.dll_bundle.ndjson` static wiring check (the new 7th call site and +state token), the same "static wiring guard, not runtime execution" precedent already accepted +for 4 of this subroutine's other 5 pre-existing states (see `docs/agent-ndjson.md`'s own note on +this), not a live CI trigger -- building a live fixture would need either 4 genuine native-DLL +dependencies in one build (not yet observed for any real package) or a new test-only injection +hook into the detection step, judged disproportionate effort for a low-trigger-rate correctness +fix relative to the rest of this feature. + **`self.layered_e2e.chain` (`tests/selfapps_layered_e2e.ps1`) is this loop's regression test (Requirement 4)**, extended with a 4th mechanism (`mech4Pass`) alongside the three it already proved for real (REQ-009 cascade, warnfix repair, hidden-import recovery) -- see that file's own diff --git a/docs/agent-ndjson.md b/docs/agent-ndjson.md index 1715c298..30dcc61c 100644 --- a/docs/agent-ndjson.md +++ b/docs/agent-ndjson.md @@ -227,16 +227,21 @@ before the Miniconda install-if-missing block even runs, so the corruption-check in the job -- self-contained by construction, no CI-ordering dependency (unlike `self.conda.bothfail`, above). -`self.dll_bundle.recover` (inline `run_setup.bat`'s `:emit_dll_bundle_row`, called from all 6 +`self.dll_bundle.recover` (inline `run_setup.bat`'s `:emit_dll_bundle_row`, called from all 7 outcome points inside `:dll_bundle_recover` -- CLAUDE.md Item 24 / `docs/prd-conda-native-dll- bundling.md`) is a CodeRabbit review finding on PR #414: the native-DLL bundling repair loop's detected/skipped/repaired/unlocatable/failed outcomes previously only reached `:log`'s console text, with no machine-readable record. `details.state` is one of `skipped_nuitka` (a Nuitka-built EXE, repair not attempted), `skipped_non_conda` (a non-conda provider, repair not attempted), `repaired` (rebuild genuinely succeeded), `unlocatable` (detected but the named DLL was never -found under the conda env's `Library\bin`), `failed_rebuild` (PyInstaller rebuild itself failed), +found under the conda env's `Library\bin`), `exhausted` (CLAUDE.md Item 25 -- the 3-iteration +cap was reached with a real, locatable candidate still pending, so at least one native-DLL +dependency remains unbundled; distinct from `repaired` so a partial fix is never reported as a +clean completion), `failed_rebuild` (PyInstaller rebuild itself failed), or `failed_missing_exe` (rebuild reported success but `dist\.exe` was not produced) -- -`pass` is `false` only for the two `failed_*` states. `details.dll`/`details.provider`/ +`pass` is `false` only for the two `failed_*` states (`exhausted` is `pass:true`, same treatment +as `unlocatable` -- both are informational: the bootstrap itself did not fail, only the repair was +incomplete). `details.dll`/`details.provider`/ `details.iteration` are pulled inside the emitting PowerShell command via `[Environment]::GetEnvironmentVariable(...)` rather than `%VAR%` cmd.exe substitution into the `-Command` text, for the same reason the sibling `HP_DLL_DETECTED_SAFE`/`HP_NEXT_DLL_SAFE` @@ -258,12 +263,14 @@ actually written during that test's run today -- `mech4Pass` continues to rely o `[REPAIR][DLL_BUNDLE]` log-text assertions (`docs/agent-interconnect.md`'s DLL-bundling section), unaffected by this addition. `batch.dll_bundle.ndjson` (`tests/harness.ps1`, static) is the actual coverage for this row: it verifies the `:emit_dll_bundle_row` subroutine exists, the row id string -is present, and all 6 `call :emit_dll_bundle_row ` sites are wired -- the same "static wiring +is present, and all 7 `call :emit_dll_bundle_row ` sites are wired (including the +`exhausted` state added for CLAUDE.md Item 25) -- the same "static wiring guard, not runtime execution" pattern already used for `batch.failfast.probe`/`batch.postexec. checkpoint` above. Per-state runtime coverage (deliberately setting `HP_NDJSON` for a sub-bootstrap and reading the row back) remains a candidate for a future, dedicated test if ever justified -- -not pursued now, since simulating the other 5 states (Nuitka-used, non-conda provider, a genuine -PyInstaller rebuild failure, a missing post-rebuild EXE, a detected-but-unlocatable DLL) each needs +not pursued now, since simulating the other 6 states (Nuitka-used, non-conda provider, a genuine +PyInstaller rebuild failure, a missing post-rebuild EXE, a detected-but-unlocatable DLL, the +3-iteration cap hit with a candidate still pending) each needs its own scaffolding beyond what `self.layered_e2e.chain`'s real `pygrib` trigger already provides for the `repaired` state alone. diff --git a/run_setup.bat b/run_setup.bat index 0e640666..d8c959df 100644 --- a/run_setup.bat +++ b/run_setup.bat @@ -4020,6 +4020,7 @@ if exist "~dll_bundle_tried.txt" del "~dll_bundle_tried.txt" >nul 2>&1 set "HP_DLL_ITER=0" set "HP_PYI_DLLBIND=" set "HP_DLL_FAILED=" +set "HP_DLL_EXHAUSTED=" rem derived requirement: real CI evidence (self.layered_e2e.chain, cache lane, 2026-08-07) -- rem HP_PY_DIR (from %%~dpI) always ends in exactly ONE trailing backslash. Quoted as rem "%HP_PY_DIR%" immediately before another quoted argument, Python's own argv parser @@ -4054,6 +4055,14 @@ if not defined HP_NEXT_DLL ( goto :dll_bundle_recover_done ) if %HP_DLL_ITER% GEQ 3 ( + rem derived requirement: CLAUDE.md Item 25 -- a real candidate was just found (the + rem "if not defined HP_NEXT_DLL" early-return above already ruled out the empty case), but + rem the 3-iteration cap discards it here without a trace. HP_DLL_EXHAUSTED distinguishes + rem this from a clean "repaired" outcome at :dll_bundle_recover_done below -- without it, + rem HP_DLL_ITER GEQ 1 (true: 3 DLLs were genuinely bundled in earlier iterations) alone + rem would claim "Native-DLL bundling complete" even though a real, locatable DLL was left + rem unbundled. + set "HP_DLL_EXHAUSTED=1" if exist "~next_dll.txt" del "~next_dll.txt" >nul 2>&1 goto :dll_bundle_recover_done ) @@ -4121,6 +4130,11 @@ rem increment. HP_DLL_FAILED is the actual success/failure signal; the "complete rem must never fire on a genuine rebuild failure (mirrors the warnfix-rebuild's own rem "never claim success without checking" precedent -- see docs/agent-lessons-learned.md). if defined HP_DLL_FAILED goto :dll_bundle_recover_exit +if defined HP_DLL_EXHAUSTED ( + call :log "[WARN][DLL_BUNDLE] Native-DLL bundling reached its 3-attempt cap with another native-DLL dependency still detected (first seen: '%HP_DLL_DETECTED_SAFE%'); %HP_DLL_ITER% DLL(s) were bundled, but at least one more remains unbundled -- the EXE may still fail to load it." + call :emit_dll_bundle_row exhausted + goto :dll_bundle_recover_exit +) if %HP_DLL_ITER% GEQ 1 ( call :log "[REPAIR][DLL_BUNDLE] Native-DLL bundling complete (%HP_DLL_ITER% DLL(s) added); EXE will be re-verified next." call :emit_dll_bundle_row repaired @@ -4132,13 +4146,16 @@ if %HP_DLL_ITER% GEQ 1 ( if exist "~dll_bundle_tried.txt" del "~dll_bundle_tried.txt" >nul 2>&1 set "HP_DLL_SPEC_PRE=" set "HP_DLL_FAILED=" +set "HP_DLL_EXHAUSTED=" exit /b 0 :emit_dll_bundle_row rem CodeRabbit finding on PR #414: ":dll_bundle_recover"'s detected/skipped/repaired/ rem unlocatable/failed outcomes previously only reached :log's console text, with no rem machine-readable record. %1 is always one of a small set of literal state tokens -rem (skipped_nuitka/skipped_non_conda/repaired/unlocatable/failed_rebuild/ -rem failed_missing_exe) written directly in THIS file, never derived from external +rem (skipped_nuitka/skipped_non_conda/repaired/unlocatable/exhausted/failed_rebuild/ +rem failed_missing_exe -- CLAUDE.md Item 25 added "exhausted": the 3-iteration cap was +rem hit with a real, locatable candidate still pending, distinct from a clean "repaired") +rem written directly in THIS file, never derived from external rem content, so passing it as a call argument is safe. The DLL name/provider/iteration rem are pulled INSIDE PowerShell via [Environment]::GetEnvironmentVariable rather than rem %VAR% cmd.exe substitution into the -Command text -- same reasoning as the diff --git a/tests/harness.ps1 b/tests/harness.ps1 index 590f9818..a4ceaeac 100644 --- a/tests/harness.ps1 +++ b/tests/harness.ps1 @@ -470,18 +470,21 @@ $cpCallCount = ([regex]::Matches($AllText, 'call :run_postexec_checkpoint')). $hasCheckpoint = $cpSub -and $cpTestOverride -and $cpPrompt -and $cpAccept -and $cpDecline -and ($cpCallCount -ge 3) Write-Result 'batch.postexec.checkpoint' 'REQ-018 (2b-C): post-execution checkpoint wired (subroutine + CI-safe test override + unconditional prompt echo + accept/decline log lines + call sites at EXE smoke and both no-EXE interpreter branches)' $hasCheckpoint @{ sub=$cpSub; testOverride=$cpTestOverride; prompt=$cpPrompt; accept=$cpAccept; decline=$cpDecline; callSites=$cpCallCount } # derived requirement: CLAUDE.md Item 24 -- guard against silent removal of the native-DLL -# recovery loop's NDJSON emission subroutine and its 6 call sites (one per outcome state: -# skipped_nuitka, skipped_non_conda, repaired, unlocatable, failed_rebuild, +# recovery loop's NDJSON emission subroutine and its 7 call sites (one per outcome state: +# skipped_nuitka, skipped_non_conda, repaired, unlocatable, exhausted, failed_rebuild, # failed_missing_exe). A CodeRabbit review finding on PR #414 required a dedicated # machine-readable record for these outcomes (previously :log console text only). +# "exhausted" (Item 25) was added when a CodeRabbit review round on PR #414 found that hitting +# the 3-iteration cap with a real, locatable candidate still pending was silently reported as +# "repaired" instead of a distinct outcome. $dbSub = $AllText -match '(?m)^:emit_dll_bundle_row' $dbRowId = $AllText -match [regex]::Escape("id='self.dll_bundle.recover'") $dbCallCount = ([regex]::Matches($AllText, 'call :emit_dll_bundle_row')).Count -$dbStates = @('skipped_nuitka','skipped_non_conda','repaired','unlocatable','failed_rebuild','failed_missing_exe') +$dbStates = @('skipped_nuitka','skipped_non_conda','repaired','unlocatable','exhausted','failed_rebuild','failed_missing_exe') $dbAllStates = $true foreach ($s in $dbStates) { if ($AllText -notmatch [regex]::Escape("call :emit_dll_bundle_row $s")) { $dbAllStates = $false } } -$hasDllBundleRow = $dbSub -and $dbRowId -and $dbAllStates -and ($dbCallCount -ge 6) -Write-Result 'batch.dll_bundle.ndjson' 'CLAUDE.md Item 24: native-DLL bundling recovery loop emits a dedicated NDJSON row (self.dll_bundle.recover) for every outcome state (skipped_nuitka/skipped_non_conda/repaired/unlocatable/failed_rebuild/failed_missing_exe)' $hasDllBundleRow @{ sub=$dbSub; rowId=$dbRowId; allStates=$dbAllStates; callSites=$dbCallCount } +$hasDllBundleRow = $dbSub -and $dbRowId -and $dbAllStates -and ($dbCallCount -ge 7) +Write-Result 'batch.dll_bundle.ndjson' 'CLAUDE.md Item 24/25: native-DLL bundling recovery loop emits a dedicated NDJSON row (self.dll_bundle.recover) for every outcome state (skipped_nuitka/skipped_non_conda/repaired/unlocatable/exhausted/failed_rebuild/failed_missing_exe)' $hasDllBundleRow @{ sub=$dbSub; rowId=$dbRowId; allStates=$dbAllStates; callSites=$dbCallCount } # derived requirement: CLAUDE.md Item 24 -- an earlier version of HP_DLL_DETECTED_SAFE/ # HP_NEXT_DLL_SAFE/HP_NEXT_DLL_PATH_SAFE's display-only sanitization stripped a literal # percent sign via cmd.exe's own %VAR:%%=_% doubled-percent substitution idiom (confirmed