diff --git a/CLAUDE.md b/CLAUDE.md index 7d7ca0cc..a5f8bbd4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -573,18 +573,98 @@ start at 1 and has gaps. own runtime DLL would mean `pyproj` is broken for every user, not just under PyInstaller), but the CI artifact captured for this run only covers the test's own app directory, not the shared Miniconda installation tree, so this is inference from convention, not direct confirmation. - **Fix direction (not yet implemented, deliberately deferred to its own loop)**: either (a) call - `:dll_bundle_recover`'s own detection step again after `:hidden_import_recover`'s loop finishes - (a second pass, catching any DLL warning a hidden-import rebuild introduced), or (b) interleave - the two loops so each `:hidden_import_recover` rebuild is itself followed by a DLL-warning - re-scan before the next smoke run. Either needs the same care already documented for this - subsystem's other cross-loop interactions: `HP_PYI_DLLBIND` must keep reaching EVERY later - rebuild command (already true today, re-verify it stays true), the 3-iteration caps on each loop - need to interact sensibly (a DLL fix consuming a hidden-import iteration budget, or vice versa, - would need a considered design rather than an accidental one), and any new call site needs the - same tried-list/re-entry-safety guarantees `:dll_bundle_recover` and `:hidden_import_recover` - each already have on their own. Low urgency: only affects the `cache`-lane, non-gating - `self.layered_e2e.chain` test; does not block any lane that gates PR merges. + **Implemented 2026-08-08, option (a)**: `:run_exe_smokerun`'s flow now calls + `:dll_bundle_recover` a SECOND time immediately after `:hidden_import_recover`'s first call + returns, and, if that second call actually bundled something (a new `HP_DLL_REPAIRED` flag, + reset at `:dll_bundle_recover`'s own entry and set only in its genuine "repaired" branch -- + `HP_DLL_ITER` alone was considered and rejected as the signal, since an early "nothing + detected" return never resets it, so a stale value from an EARLIER call could look like fresh + repair activity), gives `:hidden_import_recover` one more bounded pass too -- needed because a + DLL fix can unblock a package whose OWN hidden-import gap was previously unreachable (exactly + `self.layered_e2e.chain`'s own shape: colorama's gap is only reached once `pyproj`'s DLL is + fixed). Deliberately not chained further than this one extra round each: a third pass risks an + unbounded repair cascade for a pathological dependency tree, and a case ever found needing more + is its own future backlog item, not solved speculatively here. + **Two real, cross-call state-leak bugs found and fixed while implementing this** -- both are the + same class of bug this subsystem has hit before (a fix silently dropped by a LATER rebuild that + doesn't carry it forward), just now occurring ACROSS two calls to the SAME subroutine rather + than between two DIFFERENT ones: + 1. `:dll_bundle_recover` unconditionally reset `HP_PYI_DLLBIND=` at the top of its own + per-call bundling section -- harmless for a single call, but a second call would silently + wipe the FIRST call's own accumulated `--add-binary` flags (e.g. `eccodes.dll`'s binding, + from Item 24) before the second call's rebuild ever runs. Fixed by moving this reset out to + `:run_entry_smoke`'s own fresh-build-attempt initialization (alongside the pre-existing + `HP_NUITKA_FALLBACK_USED`/`HP_DEP_MAYBE_INCOMPLETE` resets there) -- once per fresh build + attempt, not once per call. + 2. `:hidden_import_recover` had the IDENTICAL bug for `HP_PYI_HIDDEN_IMPORTS`/ + `HP_PYI_HID_COLLECT`, at BOTH its entry AND its own exit trailer -- a second call (post-DLL- + fix) would reset these to empty before its own rebuild for the NEXT hidden-import target + (e.g. colorama), silently regressing the FIRST call's own numpy/pyproj fixes back to + `ModuleNotFoundError`. Fixed the same way -- moved to the same once-per-fresh-build-attempt + reset point. + Also threaded `%HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT%` into `:dll_bundle_recover`'s OWN + rebuild command (previously only `%HP_PYI_DLLBIND%` reached `:hidden_import_recover`'s rebuild, + never the reverse) -- the mirror-image of the fix Item 28 already applied in the other + direction. Static regression guard added: `tests/harness.ps1`'s new + `batch.dll_bundle.second_pass` check asserts both new call sites exist, the flag-threading is + present, and -- specifically to catch a REINTRODUCED per-call reset -- the bare + `set "HP_PYI_DLLBIND="` line appears in the file EXACTLY twice (the fresh-build-attempt reset + plus `:run_entry_smoke`'s own pre-existing end-of-pass trailer) and the `HP_PYI_HIDDEN_IMPORTS`/ + `HP_PYI_HID_COLLECT` bare resets appear EXACTLY once each. + **Refined same day per a CodeRabbit review round on PR #421 (both findings addressed, both + genuinely improved the design even though the described failure mode did not reproduce in the + traced real scenario):** (1) the second `:dll_bundle_recover` call is now gated on a new + `HP_HIDDEN_REPAIRED` flag (same reset-at-entry/set-only-on-genuine-rebuild shape as + `HP_DLL_REPAIRED`) -- if `:hidden_import_recover`'s first call did NOT actually rebuild + anything, the build log has not grown since the FIRST `:dll_bundle_recover` call already + scanned it, so the second call is skipped entirely instead of always paying for a pointless + re-scan. (2) `:hidden_import_recover`'s own loop now advances `HP_LOG_SIZE_BEFORE` to right + before EACH of its own rebuilds, mirroring `:dll_bundle_loop`'s identical pattern -- narrows + the second pass's own scan window to just the LAST hidden-import rebuild's output, rather than + everything since the first DLL-bundle pass (which also covered earlier, already-resolved + rebuilds). Traced whether the ORIGINAL wider window could actually cause the failure CodeRabbit + described (re-detecting and re-bundling an already-fixed DLL, spuriously setting + `HP_DLL_REPAIRED`): confirmed it could not, in the observed real scenario -- an already-bundled + DLL's own warning does not reappear in a later rebuild that still carries its `--add-binary` + flag, and `HP_DLL_REPAIRED` is scoped strictly to a genuine "found on disk and rebuilt + successfully" branch, never to a mere re-detection. Implemented both refinements anyway since + they are still objectively more precise and defensive, and directly close a "Major" review + finding cleanly. + **A THIRD CodeRabbit finding on the same PR #421 review round, this one a genuine functional + bug (not a defense-in-depth refinement like the two above): the second `:dll_bundle_recover` + pass could never actually run when the first `:hidden_import_recover` call happened to fix the + smoke run.** The caller had `if "%HP_EXE_EXIT%"=="0" goto :smokerun_ok` immediately after the + first `call :hidden_import_recover` -- so whenever that call's own rebuild made the EXE exit 0, + execution jumped straight to `:smokerun_ok`, skipping the entire Item 29 block (the second + `:dll_bundle_recover` call and its own conditional second `:hidden_import_recover` pass) before + it ever ran. This defeats the whole premise of build-time DLL detection for exactly the + scenario Item 29 exists to catch: a hidden-import rebuild's own `--collect-submodules=X` can + surface a NEW native-DLL warning in the build log even when the CURRENT smoke run's own code + path happens not to load the DLL-needing part of `X` -- the smoke run passing does not mean the + build log has nothing left to flag, and a real user hitting a different code path later could + still crash with a `DLL load failed` error the build already warned about. Fixed by removing + that early goto entirely -- the block is already correctly self-gating (skipped when + `HP_HIDDEN_REPAIRED` is undefined, i.e. the first call did no rebuild at all) and the genuine + final success check already exists right after the whole block (`if "%HP_EXE_EXIT%"=="0" goto + :smokerun_ok`, unchanged) -- so no other logic needed to change. A companion Minor finding on + the same review caught that `tests/harness.ps1`'s own `$hiLogSizeAdvance` check (added for the + second refinement above) was a whole-file `-match`, which stayed true even if the new + `HP_LOG_SIZE_BEFORE` line inside `:hidden_import_recover` were deleted entirely, since the + identical text already exists in `:run_entry_smoke`'s initial snapshot and in + `:dll_bundle_loop` -- fixed by scoping the check to a regex-extracted `:hidden_import_recover` + body (bounded by the next label, `:warn_user_code_launch`), verified locally to flip from + true to false when the new line is removed (confirming the check is no longer vacuous). + **NOT YET CONFIRMED in real CI** -- same status this repo requires before treating a fix like + this as settled (see the Item 24/28 precedent: implemented, then confirmed via a real + `cache`-lane run before being considered closed). The next `self.layered_e2e.chain` run should + show the second `:dll_bundle_recover` pass locating and bundling `proj_9.dll`, then a second + `:hidden_import_recover` pass reaching and fixing colorama's own gap, finally flipping + `chainPass` to `true` for the first time. If it doesn't, capture the same kind of raw + per-attempt artifact this and Item 28's own investigations used + (`dist/~layered_e2e_exe.log` plus `~setup.log`'s own `WARNING: Library not found` lines) before + guessing further -- there is no guarantee a 4th layer doesn't exist beneath this one. Low + urgency either way: only affects the `cache`-lane, non-gating `self.layered_e2e.chain` test; + does not block any lane that gates PR merges. ## Cold Storage (promising ideas, deliberately shelved -- revisit only if a named trigger fires) diff --git a/docs/agent-interconnect.md b/docs/agent-interconnect.md index 45e50fbd..36c2d015 100644 --- a/docs/agent-interconnect.md +++ b/docs/agent-interconnect.md @@ -492,6 +492,149 @@ ever reached) -- see `docs/agent-closed-backlog.md`'s Item 28 entry for the full separately-scoped finding (closed 2026-08-08; its own fix uncovered a further, deeper gap now tracked as CLAUDE.md's Item 29). +**CLAUDE.md Item 29 (implemented 2026-08-08, not yet CI-confirmed): `:dll_bundle_recover` now +runs a SECOND time per fresh build attempt, after `:hidden_import_recover`'s own loop finishes -- +`:run_exe_smokerun`'s flow calls it again right there, and, if that second call actually bundles +something, gives `:hidden_import_recover` one more bounded pass too.** Root cause: a +`--collect-submodules=X` hidden-import fix (Item 28) can pull a package's own compiled extensions +into the bundle for the FIRST time, surfacing a native-DLL warning that never existed in any +EARLIER build -- confirmed via a real `pyproj`/`proj_9.dll` failure (`self.layered_e2e.chain`, +`cache`-lane run `31256064576`): once `--collect-submodules=pyproj` bundled `pyproj`'s own `.pyd` +files, PyInstaller's OWN build log for that rebuild showed 9 fresh `Library not found: could not +resolve 'proj_9.dll'` warnings, one per `.pyd`. `:dll_bundle_recover`'s original, single call site +(before the very first smoke run) had no way to see this, since it only ever scans build output +that existed BEFORE it was called. + +**New `HP_DLL_REPAIRED` flag is the caller's signal for "did this specific call actually bundle +something," deliberately NOT `HP_DLL_ITER`.** `HP_DLL_ITER` is reset to 0 partway through +`:dll_bundle_recover` (after the cheap `--detect` pre-check, before the actual bundling loop) -- +an early "nothing detected" return above that point never touches it, so a SECOND call finding +nothing new would otherwise still read a stale, nonzero `HP_DLL_ITER` left over from the FIRST +call's own successful bundle, making the caller wrongly think fresh repair activity just +happened. `HP_DLL_REPAIRED` is reset to empty at the very top of `:dll_bundle_recover` (before +ANY early-return path, including the pre-`--detect` ones) and set to `1` only in the genuine +"repaired" branch at `:dll_bundle_recover_done` -- a reliable, per-call-scoped signal the caller +can check right after `call :dll_bundle_recover` returns, gating whether the extra +`:hidden_import_recover` pass (and its own EXE re-launch) is actually worth paying for. + +**Two real, cross-call state-leak bugs found and fixed while wiring this up -- the same bug class +this subsystem has hit before (a fix silently dropped by a rebuild that doesn't carry it +forward), just now occurring ACROSS two calls to the SAME subroutine instead of between two +DIFFERENT ones:** +1. `:dll_bundle_recover` unconditionally reset `HP_PYI_DLLBIND=` at the top of its own per-call + bundling section (right after the nuitka/non-conda gates, before `:dll_bundle_loop`). Harmless + for a single call, but a genuine second call would silently wipe the FIRST call's own + accumulated `--add-binary` flags (e.g. `eccodes.dll`'s binding, from Item 24) before the + second call's own rebuild ever ran -- the resulting EXE would have LOST `eccodes.dll`'s + bundling even while gaining `proj_9.dll`'s. Fixed by moving the reset out of + `:dll_bundle_recover` entirely, to `:run_entry_smoke`'s own fresh-build-attempt + initialization block (alongside the pre-existing `HP_NUITKA_FALLBACK_USED`/ + `HP_DEP_MAYBE_INCOMPLETE` resets there, which already follow the identical "once per fresh + build attempt, not once per call" principle for their own reasons) -- so it now happens + exactly once per `:run_entry_smoke` pass, regardless of how many times + `:dll_bundle_recover` itself is called within that pass. +2. `:hidden_import_recover` had the IDENTICAL bug for `HP_PYI_HIDDEN_IMPORTS`/ + `HP_PYI_HID_COLLECT`, and in TWO places: its own entry (reset unconditionally, right after the + Nuitka-skip guard) AND its own exit trailer (reset again right before `exit /b 0`). A second + call (the new post-DLL-fix pass) would have these wiped BOTH on the way in (before doing + anything) AND -- even if that first wipe hadn't existed -- on the way out of the FIRST call, + before the caller's OWN second call to `:dll_bundle_recover` could ever read them to thread + through ITS rebuild command. Either wipe alone would have silently regressed the FIRST call's + own numpy/pyproj hidden-import fixes back to `ModuleNotFoundError` the moment ANY later + rebuild ran without re-specifying them (PyInstaller's `--clean --onefile` invocation does a + fully fresh analysis each time; it does not remember flags from a previous, separate + invocation). Fixed the same way as (1) -- both resets removed, replaced by the single + once-per-fresh-build-attempt reset alongside `HP_PYI_DLLBIND`'s own. + +**`:dll_bundle_recover`'s own rebuild command now also threads +`%HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT%`** -- previously only `%HP_PYI_DLLBIND%` reached +`:hidden_import_recover`'s rebuild (the fix documented earlier in this section, "`HP_PYI_DLLBIND` +must be threaded into `:hidden_import_recover`'s OWN rebuild command"); this closes the reverse +direction, so a DLL-bundle rebuild that happens to run AFTER `:hidden_import_recover` (the new +second-pass case) does not silently drop whatever hidden-import fixes are already accumulated. +Both subroutines' rebuild commands now carry the identical flag ordering: +`%HP_PYI_EXPAT% %HP_PYI_COLLECT% %HP_PYI_DLLBIND% %HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT%`. + +**Deliberately bounded to exactly one extra round of each subroutine, not a further interleave.** +If the post-DLL-fix `:hidden_import_recover` pass itself surfaces ANOTHER native-DLL gap (e.g. a +package it hidden-imports has its own unbundled dependency), this design does NOT call +`:dll_bundle_recover` a third time -- each subroutine call already gets its own fresh, capped +3-iteration budget, and chaining a third round risks an unbounded repair cascade for a +sufficiently pathological dependency tree. A case ever found needing more than this one extra +round is its own future backlog item (following the exact same "each fix reveals the next layer" +precedent that produced Item 28 from Item 24, and Item 29 from Item 28), not solved +speculatively here. + +**Static regression guard**: `tests/harness.ps1`'s `batch.dll_bundle.second_pass` check asserts +both new call sites exist, the flag-threading text is present, `HP_DLL_REPAIRED` is both set and +checked, and -- specifically to catch either state-leak bug above being reintroduced -- the bare +`set "HP_PYI_DLLBIND="` line appears in `run_setup.bat` EXACTLY twice (the fresh-build-attempt +reset plus `:run_entry_smoke`'s own pre-existing end-of-pass trailer reset) and the bare +`HP_PYI_HIDDEN_IMPORTS`/`HP_PYI_HID_COLLECT` resets appear EXACTLY once each. + +**Refined via a CodeRabbit review round on PR #421: `:hidden_import_recover`'s own rebuild also +now advances `HP_LOG_SIZE_BEFORE`, and the second `:dll_bundle_recover` call is gated on a new +`HP_HIDDEN_REPAIRED` flag.** `:hidden_import_recover`'s own loop previously never touched +`HP_LOG_SIZE_BEFORE` at all, so the second `:dll_bundle_recover` pass's scan window stayed +anchored wherever the FIRST `:dll_bundle_recover` call had last left it -- correct (never missed +a genuinely new warning) but wider than necessary (also re-covering the first DLL-bundle pass's +own already-resolved rebuild, and every earlier hidden-import rebuild, not just the LAST one). +Fixed by adding the identical `for %%Z in ("%LOG%") do set "HP_LOG_SIZE_BEFORE=%%~zZ"` line +`:dll_bundle_loop` already uses, right before `:hidden_import_recover`'s own rebuild too -- +narrows the window to just the freshest rebuild's own output. Separately, a new `HP_HIDDEN_ +REPAIRED` flag (identical shape to `HP_DLL_REPAIRED`: reset at `:hidden_import_recover`'s own +entry, before any early-return path; set only where a rebuild is genuinely attempted, alongside +`HP_HIDDEN_ITER`'s own increment) now gates the second `:dll_bundle_recover` call -- if the first +`:hidden_import_recover` call did nothing (e.g. the failure wasn't a fixable `ModuleNotFoundError` +at all), there is nothing new in the log for a second DLL scan to find, so it's skipped entirely +rather than always paying for a pointless re-scan. + +**Traced whether the ORIGINAL wider scan window could actually cause the failure CodeRabbit's +review described** (re-detecting an already-bundled DLL's own warning, re-adding it, and +spuriously setting `HP_DLL_REPAIRED` to trigger an unnecessary extra hidden-import pass) -- +confirmed it could not, for the real observed scenario: once a DLL is bundled via `--add-binary` +(threaded into every later rebuild via `HP_PYI_DLLBIND`), PyInstaller's own build-time analysis +genuinely stops re-emitting that specific "Library not found" warning in subsequent builds (the +dependency really is satisfied), and `HP_DLL_REPAIRED` is scoped strictly to the genuine +"located on disk and rebuild succeeded" branch inside `:dll_bundle_recover_done` -- never to mere +detection. Both refinements were implemented anyway, since narrowing the scan window and skipping +a provably pointless re-scan are still objectively more precise and defensive regardless of +whether the wider window was ever empirically observed to misfire. + +**A third finding on the same PR #421 review round was a genuine functional bug, not a +defense-in-depth refinement: the caller had an early `if "%HP_EXE_EXIT%"=="0" goto :smokerun_ok` +immediately after the FIRST `call :hidden_import_recover`, which could skip this entire second-pass +block outright.** Whenever that first call's own rebuild happened to make the smoke run exit 0, +execution jumped straight to `:smokerun_ok` before the second `:dll_bundle_recover` call (and its +own conditional second `:hidden_import_recover` pass) ever ran. This defeated the whole point of +build-time DLL detection for exactly the case this feature exists to catch: a hidden-import +rebuild's own `--collect-submodules=X` can surface a NEW native-DLL warning in the build log even +when the CURRENT smoke run's own code path does not happen to load the DLL-needing part of `X` -- +a passing smoke run does not mean the build log has nothing left to flag, and a real user later +hitting a different code path could still hit a `DLL load failed` error the build already warned +about but the bootstrapper silently declined to act on. Fixed by removing that early goto +entirely -- the block below it is already correctly self-gating on `HP_HIDDEN_REPAIRED` (skipped +when the first call did no rebuild at all), and the genuine final success check +(`if "%HP_EXE_EXIT%"=="0" goto :smokerun_ok`) already exists unchanged right after the whole +block, so removing the early one needed no other logic change. + +**Companion Minor finding: `tests/harness.ps1`'s own `$hiLogSizeAdvance` check (added for the +`HP_LOG_SIZE_BEFORE` refinement above) was a whole-file `-match`, so it stayed `true` even if the +new line inside `:hidden_import_recover` were deleted** -- the identical +`for %%Z in ("%LOG%") do set "HP_LOG_SIZE_BEFORE=%%~zZ"` text already exists in +`:run_entry_smoke`'s own pre-build snapshot and in `:dll_bundle_loop`, so an unscoped whole-file +match could never actually prove the NEW occurrence exists. Fixed by regex-extracting +`:hidden_import_recover`'s own body (bounded by the next label, `:warn_user_code_launch`) and +matching only within it, immediately preceding its `PyInstaller` rebuild line -- verified locally +(via a simulated removal of the new line) that the scoped check correctly flips from `true` to +`false` when the line is missing, confirming it is no longer vacuous. + +**NOT YET CONFIRMED in real CI** -- needs a fresh `cache`-lane `self.layered_e2e.chain` run +showing the second `:dll_bundle_recover` pass locate and bundle `proj_9.dll`, then a second +`:hidden_import_recover` pass reach and fix colorama's own gap, before `chainPass` can be +considered proven `true` for the first time (see CLAUDE.md's Item 29 entry for the current +status). + --- ## AV-Safe Build Path requirement 9 (`:offer_optimized_build`) -- a strictly safer sibling of Tier A diff --git a/docs/agent-ndjson.md b/docs/agent-ndjson.md index cd6024ef..b4de809e 100644 --- a/docs/agent-ndjson.md +++ b/docs/agent-ndjson.md @@ -195,6 +195,7 @@ batch.failfast.probe, batch.postexec.checkpoint, batch.dll_bundle.ndjson, batch.dll_bundle.pct_sanitizer, +batch.dll_bundle.second_pass, self.dll_bundle.recover, self.bootstrap.state, self.empty_repo.msg, self.empty_repo.no_spurious_warn, self.harness.started, @@ -282,6 +283,30 @@ property) actually works, rather than statically checking `run_setup.bat`'s own implementation attempt used a cmd.exe `%VAR:%%=_%` substitution that CI proved silently produced an empty string) and `docs/agent-lessons-learned.md`'s corresponding entry. +`batch.dll_bundle.second_pass` (`tests/harness.ps1`, CLAUDE.md Item 29) is a THIRD, separate static +check in the same family -- unlike `batch.dll_bundle.ndjson`'s 7-state wiring guard and +`batch.dll_bundle.pct_sanitizer`'s live-executed sanitization fixture, this one guards the +second `:dll_bundle_recover` pass that runs after `:hidden_import_recover` (a hidden-import +rebuild's own `--collect-submodules=X` can surface a native-DLL warning the FIRST +`:dll_bundle_recover` call, which only ever ran before any hidden-import rebuild, could never +have seen -- see `docs/agent-interconnect.md`'s "Conda native-DLL bundling repair loop" section +for the full mechanism and its real `pyproj`/`proj_9.dll` trigger). Asserts: both `call +:dll_bundle_recover` and `call :hidden_import_recover` appear at least twice in the file (the +original call plus the new second-pass call), the DLL rebuild command threads the hidden-import +flags, `HP_DLL_REPAIRED` is both set (on genuine repair) and checked (gating the extra +`:hidden_import_recover` pass), the second `:dll_bundle_recover` call is itself gated on a new +`HP_HIDDEN_REPAIRED` flag, `HP_LOG_SIZE_BEFORE` advances immediately before +`:hidden_import_recover`'s own rebuild (scoped to that subroutine's own body via a regex +extraction bounded by the next label, not a whole-file match -- a whole-file match would stay +true even if this specific line were deleted, since the identical text already exists in +`:run_entry_smoke`'s initial snapshot and in `:dll_bundle_loop`), and -- specifically to catch a +REINTRODUCED per-call reset regressing the two cross-call state-leak bugs this same fix had to +close -- the bare `set "HP_PYI_DLLBIND="` line appears EXACTLY twice in the file (the +fresh-build-attempt reset plus `:run_entry_smoke`'s own pre-existing end-of-pass trailer) and the +bare `HP_PYI_HIDDEN_IMPORTS`/`HP_PYI_HID_COLLECT` resets appear EXACTLY once each. Static wiring +guard only, same as `batch.dll_bundle.ndjson` -- runtime proof is `self.layered_e2e.chain`'s own +`chainPass` (`cache` lane, non-gating), not yet confirmed `true` in real CI as of this writing. + ## selfapps-ux-hardening NDJSON rows (selfapps_ux_hardening.ps1, non-conda-full lanes) ``` diff --git a/run_setup.bat b/run_setup.bat index b400aeb5..27bcd87b 100644 --- a/run_setup.bat +++ b/run_setup.bat @@ -3420,6 +3420,20 @@ if not defined HP_BUILD_OK ( rem a stale flag left by an earlier, cascaded-away provider's failure. Set (if warranted) and rem consumed further down; see :warnfix_cascade_detect and :warn_user_code_launch. set "HP_DEP_MAYBE_INCOMPLETE=" + rem CLAUDE.md Item 29: :dll_bundle_recover can now be called MORE THAN ONCE per fresh build + rem attempt (a second pass after :hidden_import_recover -- see that call site below), so its + rem own accumulated --add-binary flags (HP_PYI_DLLBIND) must survive across those calls + rem rather than being wiped at the start of each one. Reset once here instead, at the start + rem of the fresh build attempt itself -- the same "not just when X happens to run again" + rem reasoning as HP_DEP_MAYBE_INCOMPLETE above, so a cascaded-away provider's own bindings + rem never leak into the next tier's build. HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT need the + rem same reset for the mirror-image reason: :dll_bundle_recover's OWN rebuild command now + rem threads them through too (see its own comment), so a stale value from a PREVIOUS + rem provider's :hidden_import_recover call must not leak into THIS provider's first + rem :dll_bundle_recover call, before THIS provider's own :hidden_import_recover has run. + set "HP_PYI_DLLBIND=" + set "HP_PYI_HIDDEN_IMPORTS=" + set "HP_PYI_HID_COLLECT=" if defined HP_TEST_FORCE_PYINSTALLER_FAIL ( call :log "[TEST] HP_TEST_FORCE_PYINSTALLER_FAIL: simulating PyInstaller build failure." call :try_nuitka_tier_a @@ -3976,6 +3990,15 @@ rem eccodes.dll for a pygrib-only build, ruling out that free-lunch shortcut and rem this loop. goto-based (not a parenthesized block) so each %VAR% reads its runtime rem value, not a parse-time one -- see docs/agent-lessons-learned.md's "Provider-cascade rem dispatch is goto-based on purpose". +rem CLAUDE.md Item 29: this subroutine can now be called a SECOND time per fresh build +rem attempt, after :hidden_import_recover's own loop finishes -- a hidden-import rebuild's +rem own --collect-submodules=X can pull in a package whose compiled extension needs a native +rem DLL never checked before (confirmed via a real pyproj/proj_9.dll failure). Reset this +rem call's own "did I actually bundle something" signal FIRST, before any early-return path, +rem so the caller has a reliable, per-call-scoped flag -- HP_DLL_ITER is NOT reliable for this +rem across two calls (an early "nothing detected" return below never resets it, so a stale +rem value from an EARLIER call could otherwise look like fresh repair activity). +set "HP_DLL_REPAIRED=" if not exist "dist\%ENVNAME%.exe" exit /b 0 if not defined HP_LOG_SIZE_BEFORE exit /b 0 rem Requirement 3: detection is cheap and provider-agnostic -- always checked first, @@ -4053,7 +4076,10 @@ if not "%HP_ENV_MODE%"=="conda" ( ) if exist "~dll_bundle_tried.txt" del "~dll_bundle_tried.txt" >nul 2>&1 set "HP_DLL_ITER=0" -set "HP_PYI_DLLBIND=" +rem CLAUDE.md Item 29: HP_PYI_DLLBIND is deliberately NOT reset here -- it is now reset once +rem per fresh build attempt, in :run_entry_smoke, so a SECOND call to this subroutine (the +rem post-hidden-import-recovery pass) keeps whatever this label's own earlier call already +rem accumulated instead of silently discarding it. set "HP_DLL_FAILED=" set "HP_DLL_EXHAUSTED=" rem derived requirement: real CI evidence (self.layered_e2e.chain, cache lane, 2026-08-07) -- @@ -4140,7 +4166,12 @@ set "HP_PYI_DLLBIND=%HP_PYI_DLLBIND% --add-binary "%HP_NEXT_DLL_PATH%;."" call :log "[REPAIR][DLL_BUNDLE] Bundling native DLL dependency: %HP_NEXT_DLL_SAFE% (found at %HP_NEXT_DLL_PATH_SAFE%); rebuilding EXE (iter %HP_DLL_ITER%/3)." if exist "%ENVNAME%.spec" (set "HP_DLL_SPEC_PRE=1") else (set "HP_DLL_SPEC_PRE=") for %%Z in ("%LOG%") do set "HP_LOG_SIZE_BEFORE=%%~zZ" -"%HP_PY%" -m PyInstaller -y --onefile --clean --log-level WARN %HP_PYI_EXPAT% %HP_PYI_COLLECT% %HP_PYI_DLLBIND% --name "%ENVNAME%" "%HP_ENTRY%" >> "%LOG%" 2>&1 +rem CLAUDE.md Item 29: thread HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT through this rebuild +rem too -- when this is the SECOND call (after :hidden_import_recover), omitting them would +rem silently drop every hidden-import fix already applied, the mirror-image of the bug +rem already fixed for HP_PYI_DLLBIND reaching :hidden_import_recover's own rebuild (see +rem docs/agent-interconnect.md). Empty/undefined on a first call, so no behavior change there. +"%HP_PY%" -m PyInstaller -y --onefile --clean --log-level WARN %HP_PYI_EXPAT% %HP_PYI_COLLECT% %HP_PYI_DLLBIND% %HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT% --name "%ENVNAME%" "%HP_ENTRY%" >> "%LOG%" 2>&1 if errorlevel 1 ( call :log "[ERROR][DLL_BUNDLE] PyInstaller rebuild failed while bundling native DLL dependency: %HP_NEXT_DLL_SAFE%; the previous build may no longer be valid." set "HP_BOOTSTRAP_STATE=error" @@ -4173,6 +4204,7 @@ if defined HP_DLL_EXHAUSTED ( 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 + set "HP_DLL_REPAIRED=1" ) else ( call :log "[INFO][DLL_BUNDLE] Detected native-DLL warning for '%HP_DLL_DETECTED_SAFE%' but could not locate a matching file under the conda env's Library\bin; skipping." call :emit_dll_bundle_row unlocatable @@ -4219,6 +4251,12 @@ rem ImportError / circular-import never triggers a rebuild. Bounded to 3 rebuild rem helper's already-tried list plus the iter cap guarantee the loop cannot run forever. rem Sets HP_EXE_EXIT to the final EXE exit so the caller re-checks success. goto-based rem (not a parenthesized block) so each %VAR% reads its runtime value, not a parse-time one. +rem CLAUDE.md Item 29 (CodeRabbit review finding on PR #421): reset this call's own "did I +rem actually rebuild" signal FIRST, before any early-return path -- mirrors HP_DLL_REPAIRED's +rem identical reasoning in :dll_bundle_recover. Lets the caller (the second :dll_bundle_recover +rem pass) skip its own scan entirely when this call did nothing, instead of always re-scanning +rem an unchanged log for no reason. +set "HP_HIDDEN_REPAIRED=" if not exist "dist\%ENVNAME%.exe" exit /b 0 rem AV-Safe Build Path (requirement 4 follow-up): this loop's ONLY repair mechanism is a rem PyInstaller rebuild with --hidden-import flags -- a PyInstaller-specific mechanism that @@ -4232,7 +4270,6 @@ if defined HP_NUITKA_FALLBACK_USED ( call :log "[INFO][HIDDEN_IMPORT] Skipping --hidden-import auto-recovery: dist\%ENVNAME%.exe was built via the fallback build system (Nuitka), which uses a different missing-import mechanism than PyInstaller's --hidden-import flag." exit /b 0 ) -set "HP_PYI_HIDDEN_IMPORTS=" rem CLAUDE.md Item 28: pair --collect-submodules=X with each --hidden-import=X this loop adds. rem A --hidden-import target only guarantees PyInstaller follows X's own statically-discovered rem imports; it does NOT guarantee every real submodule under X/ is bundled. A compiled C @@ -4244,7 +4281,14 @@ rem Broader than strictly necessary (collects every submodule of X, not just the rem needed) but structurally safe: X is already find_spec-confirmed installed by rem ~hidden_import_scan.py's own gate, so this never targets an unresolvable package name, and it rem never guesses AT a package name the way inferring one from the failure text would. -set "HP_PYI_HID_COLLECT=" +rem CLAUDE.md Item 29: HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT are deliberately NOT reset here +rem (unlike HP_HIDDEN_ITER/HP_HIDDEN_TRIED just below, which DO get a fresh budget every call). +rem This subroutine can now be called a SECOND time per fresh build attempt (after a +rem :dll_bundle_recover repair -- see that call site) -- resetting these here would silently +rem drop every hidden-import flag the FIRST call already accumulated from this SECOND call's +rem own rebuild command, regressing an already-fixed package back to ModuleNotFoundError. Both +rem are reset once per fresh build attempt instead, in :run_entry_smoke, mirroring the identical +rem fix already applied to HP_PYI_DLLBIND in :dll_bundle_recover. set "HP_HIDDEN_ITER=0" set "HP_HIDDEN_TRIED=" rem preserve a user pre-existing spec across recovery rebuilds (the main-build spec-preexist @@ -4271,10 +4315,20 @@ for /f "usebackq delims=" %%M in ("~next_hidden.txt") do set "HP_NEXT_HIDDEN=%%M if exist "~next_hidden.txt" del "~next_hidden.txt" >nul 2>&1 if not defined HP_NEXT_HIDDEN goto :hidden_import_recover_done set /a HP_HIDDEN_ITER+=1 +set "HP_HIDDEN_REPAIRED=1" set "HP_PYI_HIDDEN_IMPORTS=%HP_PYI_HIDDEN_IMPORTS% --hidden-import=%HP_NEXT_HIDDEN%" set "HP_PYI_HID_COLLECT=%HP_PYI_HID_COLLECT% --collect-submodules=%HP_NEXT_HIDDEN%" set "HP_HIDDEN_TRIED=%HP_HIDDEN_TRIED% %HP_NEXT_HIDDEN%" call :log "[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=%HP_NEXT_HIDDEN% --collect-submodules=%HP_NEXT_HIDDEN%; rebuilding EXE (iter %HP_HIDDEN_ITER%/3)." +rem CLAUDE.md Item 29 (CodeRabbit review finding on PR #421): advance HP_LOG_SIZE_BEFORE to +rem right before THIS rebuild, mirroring :dll_bundle_loop's own identical pattern for its own +rem rebuilds. Narrows the SECOND :dll_bundle_recover pass's own scan window to just the LAST +rem hidden-import rebuild's output (rather than everything since the first DLL-bundle pass, +rem which included earlier, already-resolved rebuilds too) -- tighter and more precise, even +rem though the wider window was not observed to cause a false re-detection in practice (an +rem already-bundled DLL's own warning does not reappear in a later rebuild that still includes +rem its --add-binary flag). +for %%Z in ("%LOG%") do set "HP_LOG_SIZE_BEFORE=%%~zZ" "%HP_PY%" -m PyInstaller -y --onefile --clean --log-level WARN %HP_PYI_EXPAT% %HP_PYI_COLLECT% %HP_PYI_DLLBIND% %HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT% --name "%ENVNAME%" "%HP_ENTRY%" >> "%LOG%" 2>&1 if errorlevel 1 ( call :log "[REPAIR][HIDDEN_IMPORT] PyInstaller rebuild failed; stopping recovery." @@ -4298,8 +4352,11 @@ set "HP_HIDDEN_ITER=" set "HP_HIDDEN_TRIED=" set "HP_NEXT_HIDDEN=" set "HP_HID_SPEC_PRE=" -set "HP_PYI_HIDDEN_IMPORTS=" -set "HP_PYI_HID_COLLECT=" +rem CLAUDE.md Item 29: HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT are deliberately NOT reset +rem here either (see this subroutine's own entry comment) -- a later :dll_bundle_recover call +rem in the SAME fresh build attempt needs to read whatever this call accumulated, to thread +rem it through its own rebuild command. Reset once per fresh build attempt instead, in +rem :run_entry_smoke. exit /b 0 :warn_user_code_launch rem REQ-016: tightly-scoped heads-up before a launch that can be force-stopped at ~30s, so the @@ -4400,6 +4457,38 @@ rem Slice 2 (REQ-016): attempt strict --hidden-import auto-recovery before givin rem Skip when HP_EXE_EXIT is -1 (a timeout/hang) -- re-running a hung EXE in the recovery rem loop would hang too; only a real fast non-zero exit (e.g. ModuleNotFoundError) is fixable. if not "%HP_EXE_EXIT%"=="-1" call :hidden_import_recover +rem CLAUDE.md Item 29 (CodeRabbit review finding on PR #421): deliberately no early +rem "if HP_EXE_EXIT==0 goto :smokerun_ok" here, unlike the check at line ~4454 above. A +rem hidden-import rebuild above (--collect-submodules=X) can pull in a package whose OWN +rem compiled extension needs a native DLL that was never checked -- the :dll_bundle_recover +rem call inside :run_entry_smoke only ever saw the ORIGINAL build's own warnings, before any +rem hidden-import rebuild ran. The rebuilt EXE can exit 0 on THIS smoke run's own code path +rem while the build log still shows a fresh, unactioned "Library not found" warning for a DLL a +rem DIFFERENT code path would need -- exactly the class of gap build-time detection exists to +rem catch before it becomes a guaranteed runtime failure (confirmed via a real pyproj/proj_9.dll +rem failure -- see docs/agent-interconnect.md's "Conda native-DLL bundling repair loop" section, +rem "Detects at BUILD time, not runtime"). So the block below must run regardless of whether this +rem rebuild already made the smoke run pass; it is gated on HP_HIDDEN_REPAIRED (below), not on +rem HP_EXE_EXIT -- if the call above did NOT actually rebuild anything, the log has not grown +rem since the FIRST :dll_bundle_recover call already scanned it, so there is nothing new to find, +rem and the gate skips the pointless re-scan in the common case instead of always paying for one. +if not "%HP_EXE_EXIT%"=="-1" if defined HP_HIDDEN_REPAIRED call :dll_bundle_recover +rem Only worth a fresh verification pass if this call actually bundled something -- the +rem common case (nothing new detected) must not pay for an extra EXE launch/wait. +if defined HP_DLL_REPAIRED ( + rem A DLL fix can also unblock a package whose OWN hidden-import gap was previously + rem unreachable (the DLL failure short-circuited the app before it got that far -- e.g. + rem colorama's own gap in self.layered_e2e.chain, only reached once pyproj's DLL is fixed). + rem Give :hidden_import_recover one more bounded pass. Deliberately not chained further: + rem each subroutine call already gets its own fresh, capped 3-iteration budget, and a + rem THIRD round risks an unbounded repair cascade for a pathological dependency tree -- a + rem case ever found needing more than this is its own future backlog item, not solved + rem speculatively here. No -1 guard needed here: HP_DLL_REPAIRED can only be defined if + rem the :dll_bundle_recover call just above actually ran, which itself required HP_EXE_EXIT + rem to already be non--1 (see the guard on that call) -- and dll_bundle_recover never + rem touches HP_EXE_EXIT, so that fact still holds at this point. + call :hidden_import_recover +) if "%HP_EXE_EXIT%"=="0" goto :smokerun_ok rem REQ-016: record that the packaged EXE could not be verified so the post-flight rem briefing can guide the user to run the app directly instead of claiming success. diff --git a/tests/harness.ps1 b/tests/harness.ps1 index c246ea8a..fc4c627d 100644 --- a/tests/harness.ps1 +++ b/tests/harness.ps1 @@ -398,6 +398,53 @@ $hiCollectAppend = $AllText -match [regex]::Escape('set "HP_PYI_HID_COLLECT=%HP_ $hiCollectInject = $AllText -match [regex]::Escape('%HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT% --name') $hasHiddenRecover = $hiCall -and $hiLabel -and $hiPayload -and $hiEmit -and $hiCap -and $hiRepair -and $hiCollectVar -and $hiCollectAppend -and $hiCollectInject Write-Result 'batch.pyi.hidden_import.recover' 'REQ-016: strict --hidden-import auto-recovery wired (recover subroutine called + payload + emit + 3-iter cap + repair log + collect-submodules pairing)' $hasHiddenRecover @{ call=$hiCall; label=$hiLabel; payload=$hiPayload; emit=$hiEmit; cap=$hiCap; repair=$hiRepair; collectVar=$hiCollectVar; collectAppend=$hiCollectAppend; collectInject=$hiCollectInject } +# derived requirement (CLAUDE.md Item 29): a hidden-import rebuild's own --collect-submodules=X +# can surface a NEW native-DLL warning :dll_bundle_recover never saw (it only ran once, before +# any hidden-import rebuild). Static guard against silent deletion/regression of the second-pass +# wiring; runtime proof is self.layered_e2e.chain's own chainPass (cache lane, non-gating). +$dllSecondCall = ([regex]::Matches($AllText, [regex]::Escape('call :dll_bundle_recover'))).Count -ge 2 +$hiSecondCall = ([regex]::Matches($AllText, [regex]::Escape('call :hidden_import_recover'))).Count -ge 2 +$dllRebuildHasHid = $AllText -match [regex]::Escape('%HP_PYI_DLLBIND% %HP_PYI_HIDDEN_IMPORTS% %HP_PYI_HID_COLLECT% --name') +$dllRepairedSet = $AllText -match [regex]::Escape('set "HP_DLL_REPAIRED=1"') +$dllRepairedCheck = $AllText -match [regex]::Escape('if defined HP_DLL_REPAIRED') +# Regression guard for the two state-leak bugs this same fix had to close: HP_PYI_DLLBIND and +# HP_PYI_HIDDEN_IMPORTS/HP_PYI_HID_COLLECT must each be reset exactly once per fresh build +# attempt (in :run_entry_smoke), NOT once per call to :dll_bundle_recover/:hidden_import_recover +# -- a reintroduced per-call reset would silently wipe an earlier call's own accumulated flags +# on the second pass. An exact occurrence count of the bare reset line catches this: DLLBIND is +# reset twice on purpose (the fresh-build-attempt reset plus :run_entry_smoke's own end-of-pass +# trailer, pre-existing); the hidden-import pair is reset once (the fresh-build-attempt reset +# only -- no end-of-pass trailer reset for these two, since HP_PYI_DLLBIND's own precedent never +# needed one either). +$dllbindResetCount = ([regex]::Matches($AllText, [regex]::Escape('set "HP_PYI_DLLBIND="'))).Count +$hiddenImportsResetCount = ([regex]::Matches($AllText, [regex]::Escape('set "HP_PYI_HIDDEN_IMPORTS="'))).Count +$hidCollectResetCount = ([regex]::Matches($AllText, [regex]::Escape('set "HP_PYI_HID_COLLECT="'))).Count +$noPerCallResetLeak = ($dllbindResetCount -eq 2) -and ($hiddenImportsResetCount -eq 1) -and ($hidCollectResetCount -eq 1) +# derived requirement (CodeRabbit review finding on PR #421): the second :dll_bundle_recover +# call must be gated on :hidden_import_recover having actually rebuilt something -- otherwise +# it re-scans a log window that hasn't grown since the FIRST :dll_bundle_recover call already +# covered it, for no purpose. HP_HIDDEN_REPAIRED (reset at :hidden_import_recover's own entry, +# before any early-return path, set only where a rebuild is genuinely attempted) is the +# reliable per-call signal for this, mirroring HP_DLL_REPAIRED's identical shape. Also guards +# that HP_LOG_SIZE_BEFORE advances to right before EACH hidden-import rebuild (mirroring +# :dll_bundle_loop's own identical pattern), tightening the second pass's own scan window to +# just the LAST hidden-import rebuild's output. The advance check is scoped to +# :hidden_import_recover's own body (not a whole-file -match) -- CodeRabbit review finding on +# PR #421: the same "for %%Z in (...) do set HP_LOG_SIZE_BEFORE" text already exists in the +# initial build and in :dll_bundle_loop, so an unscoped -match would stay true even if the new +# occurrence inside :hidden_import_loop were deleted, silently defeating the regression guard. +$hiRepairedSet = $AllText -match [regex]::Escape('set "HP_HIDDEN_REPAIRED=1"') +$hiRepairedReset = $AllText -match [regex]::Escape('set "HP_HIDDEN_REPAIRED="') +$hiddenRecoverMatch = [regex]::Match($AllText, '(?ms)^:hidden_import_recover\r?\n(?.*?)(?=^:warn_user_code_launch)') +$hiLogSizeAdvance = $hiddenRecoverMatch.Success -and ( + $hiddenRecoverMatch.Groups['body'].Value -match ( + [regex]::Escape('for %%Z in ("%LOG%") do set "HP_LOG_SIZE_BEFORE=%%~zZ"') + + '\s*\r?\n\s*"%HP_PY%" -m PyInstaller' + ) +) +$dllSecondCallGated = $AllText -match [regex]::Escape('if not "%HP_EXE_EXIT%"=="-1" if defined HP_HIDDEN_REPAIRED call :dll_bundle_recover') +$hasDllBundleSecondPass = $dllSecondCall -and $hiSecondCall -and $dllRebuildHasHid -and $dllRepairedSet -and $dllRepairedCheck -and $noPerCallResetLeak -and $hiRepairedSet -and $hiRepairedReset -and $hiLogSizeAdvance -and $dllSecondCallGated +Write-Result 'batch.dll_bundle.second_pass' 'CLAUDE.md Item 29: a second :dll_bundle_recover pass runs after :hidden_import_recover (gated on HP_HIDDEN_REPAIRED, so it only fires when a rebuild actually happened), threading its own accumulated hidden-import flags through the DLL rebuild, then giving :hidden_import_recover one more bounded pass if a DLL was actually bundled -- with a regression guard on the exact-once-per-fresh-build-attempt reset counts for the three cross-call accumulator variables' $hasDllBundleSecondPass @{ dllSecondCall=$dllSecondCall; hiSecondCall=$hiSecondCall; dllRebuildHasHid=$dllRebuildHasHid; dllRepairedSet=$dllRepairedSet; dllRepairedCheck=$dllRepairedCheck; dllbindResetCount=$dllbindResetCount; hiddenImportsResetCount=$hiddenImportsResetCount; hidCollectResetCount=$hidCollectResetCount; hiRepairedSet=$hiRepairedSet; hiRepairedReset=$hiRepairedReset; hiLogSizeAdvance=$hiLogSizeAdvance; dllSecondCallGated=$dllSecondCallGated } # derived requirement: tightly-scoped 30s-kill warning must precede launches that force-stop # user code (EXE smoke + hidden-import recovery), so users do not lose work in a verification run. $warnSubCount = ([regex]::Matches($AllText, ':warn_user_code_launch')).Count