diff --git a/CLAUDE.md b/CLAUDE.md index 67785edb..4551d5cb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -599,11 +599,46 @@ start at 1 and has gaps. `--collect-submodules=X` for that package -- broader than strictly necessary (collects every submodule of `X`, not just the one actually needed) but structurally safe (no name-guessing, no cross-package inference) and directly addresses the mechanism gap this investigation found. - Option (b) is the more promising direction on the evidence gathered so far, but still needs its - own dedicated design-and-test loop (new `tests/test_hidden_import_scan.py` coverage, a real CI - scenario proving it fires, and care that it doesn't change behavior for the numpy/packaging - cases that already work) -- deliberately not attempted in this same investigation pass, same - "don't bolt a guess onto an already-large change" discipline already applied to this item once. + **Implemented 2026-08-08, option (b).** `:hidden_import_recover` now builds a second flag + accumulator, `HP_PYI_HID_COLLECT`, alongside the existing `HP_PYI_HIDDEN_IMPORTS` -- each loop + iteration appends both `--hidden-import=%HP_NEXT_HIDDEN%` AND + `--collect-submodules=%HP_NEXT_HIDDEN%` for the SAME `%HP_NEXT_HIDDEN%`, and both flag lists are + passed to the SAME PyInstaller rebuild call. This is additive to the existing strict detection + gate, not a relaxation of it: `~hidden_import_scan.py` still only ever returns a target from a + genuine `ModuleNotFoundError` for an installed module (see + `docs/agent-lessons-learned.md`'s "--hidden-import auto-recovery must stay STRICT" entry, which + this change does not touch) -- the new flag only changes what happens for a target the strict + gate ALREADY decided to act on, ensuring that package's own submodules are collected in the same + pass rather than needing a separate, undiagnosable failure to surface later. `[REPAIR][HIDDEN_IMPORT] + Adding --hidden-import=X` log lines now read `Adding --hidden-import=X --collect-submodules=X`; + every existing test/doc assertion matching that log line as a PREFIX substring (not full-line) + continues to match unchanged (`self.exe.hidden_import`, `self.exe.hidden_import.exhaust`'s + 3-occurrence count, `self.layered_e2e.chain`'s `mech3Pass` check, `batch.pyi.hidden_import.recover`'s + static wiring guard) -- verified by tracing each one's own match pattern, not just by running the + cross-platform test subset (the real proof needs Windows CI). `tests/selfapps_hidden_import.ps1` + extended with `collectLogged`/`collectPaired` assertions (the one new test this loop adds) -- + both read the SAME `:log` line, so they prove the intended flag text was composed and logged + together, not that the real PyInstaller argv actually received it (no runtime artifact captures + the literal invoked command line; cmd.exe echo is off and PyInstaller does not print its own + argv). `tests/harness.ps1`'s own new `$hiCollectInject` check is the independent SOURCE-LEVEL + proof instead -- it confirms `%HP_PYI_HID_COLLECT%` genuinely sits in `run_setup.bat`'s own + PyInstaller command-line text right after `%HP_PYI_HIDDEN_IMPORTS%`, a static text match, not an + observation of cmd.exe's own runtime expansion or the actual invoked argv (no artifact in this + repo captures that; confirming it would need a real Windows run). Two CodeRabbit review findings + on this fix's own PR caught successive overclaims here: first, that the two + `selfapps_hidden_import.ps1` checks alone were independent proof of the real invocation + (corrected to route that claim through `$hiCollectInject` instead); second, that + `$hiCollectInject` itself proves the flag "reaches the real command line" rather than merely + being present in the source text -- the actual runtime argv is confirmed only by a real Windows + CI run, not by any check in this repo today. + **NOT YET CONFIRMED in real CI** -- same status this repo requires before treating a fix like + this as settled (see the Item 24 precedent: implemented, then confirmed via a real + `cache`-lane run before being considered closed). The next `self.layered_e2e.chain` run should + show iteration 2's `packaging` rebuild also collecting `packaging.version`, letting the EXE get + past pygrib's own import chain far enough to finally reach colorama's own hidden-import gap + (`mech3Pass`) and, if that also succeeds, flip `chainPass` to `true` for the first time. If it + doesn't, capture the SAME kind of raw per-attempt artifact this investigation used + (`dist/~layered_e2e_exe.log`, not the concatenated bootstrap log) before guessing further. 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. diff --git a/docs/agent-interconnect.md b/docs/agent-interconnect.md index 0b8af245..769e9383 100644 --- a/docs/agent-interconnect.md +++ b/docs/agent-interconnect.md @@ -128,6 +128,38 @@ CLAUDE.md as a candidate for a future dedicated test pass if this path's trigger by PyInstaller" signal) -- any subroutine that can produce `dist\.exe` via something other than PyInstaller should set an analogous marker and extend this guard to check it. +**`:hidden_import_recover` now pairs `--collect-submodules=X` with every `--hidden-import=X` it +adds (CLAUDE.md Item 28, implemented 2026-08-08).** A `--hidden-import=X` target alone only +guarantees PyInstaller follows whatever `X`'s own `__init__.py` statically imports -- it does not +guarantee every real submodule under `X/` is bundled. A compiled C extension ELSEWHERE in the app +(invisible to PyInstaller's static scan the same way the original missing import was) can still +need a submodule of `X` that `X`'s own `__init__.py` never references -- confirmed via a real +pygrib 2.1.8 failure (`from packaging import version` inside a Cython extension, needing +`packaging.version` even after `--hidden-import=packaging` alone genuinely worked). New +accumulator `HP_PYI_HID_COLLECT` (distinct from the pre-build `HP_PYI_COLLECT` computed by +`:compute_collect_flags` for packages imported by the USER'S OWN source -- do not confuse the +two, they are gated completely differently) mirrors `HP_PYI_HIDDEN_IMPORTS`'s own +accumulate-then-inject shape: each loop iteration appends +`--collect-submodules=%HP_NEXT_HIDDEN%` for the SAME `%HP_NEXT_HIDDEN%` just added as a hidden +import, and both flag lists are passed to the SAME rebuild call. Broader than strictly necessary +(collects every submodule of `X`, not just the one actually needed) but structurally safe: `X` is +already `find_spec`-confirmed installed by `~hidden_import_scan.py`'s own gate (see +`docs/agent-lessons-learned.md`'s "--hidden-import auto-recovery must stay STRICT" entry, which +this change leaves untouched -- it only changes what happens for a target the strict gate ALREADY +decided to act on, never what triggers the gate itself). + +**The `[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=X` log line now reads +`Adding --hidden-import=X --collect-submodules=X`** -- every existing consumer of that line +matches it as a PREFIX substring (`-match [regex]::Escape('...Adding --hidden-import=colorama')` +or the bare `'...Adding --hidden-import='` prefix), never a full-line or trailing-anchor match, so +each one continues to match unchanged: `self.exe.hidden_import`'s own `addingFired` check, +`self.exe.hidden_import.exhaust`'s `$addingCount -eq 3` occurrence count (the appended text sits +BEFORE the semicolon each line already had, so the per-line occurrence count of the matched +substring is unaffected), `self.layered_e2e.chain`'s `mech3Pass` colorama check, and +`tests/selfapps_nuitka_tiera_hidden_skip.ps1`'s negative "this line must NOT appear" check (still +correctly absent, since that code path returns before ever reaching this line). Re-verify this +holds for any FUTURE change to this log line's own text. + --- ## Conda native-DLL bundling repair loop (`:dll_bundle_recover`, CLAUDE.md Item 24) and its two siblings diff --git a/docs/demo-bootstrapper-output.md b/docs/demo-bootstrapper-output.md index 41708bbf..d21dc35b 100644 --- a/docs/demo-bootstrapper-output.md +++ b/docs/demo-bootstrapper-output.md @@ -1371,16 +1371,26 @@ 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): +itself matches against (`run_setup.bat`'s own `:log` calls at the smokerun/recovery call sites). +`[Extrapolated Branch]` on the one `Adding --hidden-import=...` line specifically: the cited real +capture (run `30328748330`) predates CLAUDE.md Item 28's `--collect-submodules` pairing fix, so +that single line is traced from the CURRENT source instead of copied verbatim from that run; the +other 4 lines in this block are untouched by Item 28 and remain accurate to the original capture: ``` [WARN] EXE smokerun: exited 1 (non-zero) -[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=colorama; rebuilding EXE (iter 1/3). +[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=colorama --collect-submodules=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 ``` +(CLAUDE.md Item 28: every `--hidden-import=X` this loop adds is now paired with +`--collect-submodules=X` on the same rebuild, so a compiled extension needing one of X's own +submodules -- invisible to PyInstaller's static scan the same way the original missing import +was -- gets a chance to be bundled in the same pass, rather than surfacing as a separate, +undiagnosable failure later.) + **Exhaustion** (three DIFFERENT modules missing across three rebuilds, still never fully resolving -- REAL CI CAPTURE): @@ -2771,7 +2781,7 @@ print('colorama via importlib ok:', _mod.__name__) [INFO] EXE smokerun: testing dist\.exe [WARN] Verifying the built standalone EXE (PyInstaller) now: if it stays completely silent for about 30 seconds it will be force-stopped, but any output (including a prompt waiting on your input) keeps it running as long as needed. If your program is interactive, try answering its prompts through to its own quit/exit option now so we can confirm it exits cleanly. Either way, do not start real work in it yet or any unsaved work will be lost. [WARN] EXE smokerun: exited 1 (non-zero) -[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=colorama; rebuilding EXE (iter 1/3) +[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=colorama --collect-submodules=colorama; rebuilding EXE (iter 1/3). [INFO] PyInstaller produced dist\.exe [INFO] EXE smokerun: testing dist\.exe [INFO] EXE smokerun: exited 0 (ok) @@ -3124,8 +3134,8 @@ regex-verified confirmation that these exact lines were present/absent in the re ``` Without this guard, `run_setup.bat` would instead print -`[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=nuitka; rebuilding EXE (iter 1/3)` here and -attempt a PyInstaller rebuild against a Nuitka-built EXE. +`[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=nuitka --collect-submodules=nuitka; rebuilding EXE (iter 1/3).` +here and attempt a PyInstaller rebuild against a Nuitka-built EXE. --- diff --git a/run_setup.bat b/run_setup.bat index 9b7d26fd..b400aeb5 100644 --- a/run_setup.bat +++ b/run_setup.bat @@ -4233,6 +4233,18 @@ if defined HP_NUITKA_FALLBACK_USED ( 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 +rem extension elsewhere in the app (invisible to PyInstaller's source scan the same way the +rem original missing import was) can still need a submodule of X that X's own __init__.py never +rem references -- confirmed via pygrib 2.1.8's real source (`from packaging import version` inside +rem a Cython extension, needing packaging.version even after --hidden-import=packaging alone). +rem Broader than strictly necessary (collects every submodule of X, not just the one actually +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=" set "HP_HIDDEN_ITER=0" set "HP_HIDDEN_TRIED=" rem preserve a user pre-existing spec across recovery rebuilds (the main-build spec-preexist @@ -4260,9 +4272,10 @@ 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_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%; rebuilding EXE (iter %HP_HIDDEN_ITER%/3)." -"%HP_PY%" -m PyInstaller -y --onefile --clean --log-level WARN %HP_PYI_EXPAT% %HP_PYI_COLLECT% %HP_PYI_DLLBIND% %HP_PYI_HIDDEN_IMPORTS% --name "%ENVNAME%" "%HP_ENTRY%" >> "%LOG%" 2>&1 +call :log "[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=%HP_NEXT_HIDDEN% --collect-submodules=%HP_NEXT_HIDDEN%; rebuilding EXE (iter %HP_HIDDEN_ITER%/3)." +"%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." set "HP_EXE_EXIT=1" @@ -4286,6 +4299,7 @@ set "HP_HIDDEN_TRIED=" set "HP_NEXT_HIDDEN=" set "HP_HID_SPEC_PRE=" set "HP_PYI_HIDDEN_IMPORTS=" +set "HP_PYI_HID_COLLECT=" 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 diff --git a/tests/harness.ps1 b/tests/harness.ps1 index a4ceaeac..c246ea8a 100644 --- a/tests/harness.ps1 +++ b/tests/harness.ps1 @@ -390,8 +390,14 @@ $hiPayload = $AllText -match 'set "HP_HIDDEN_IMPORT_SCAN=' $hiEmit = $AllText -match 'emit_from_base64 "~hidden_import_scan.py" HP_HIDDEN_IMPORT_SCAN' $hiCap = $AllText -match [regex]::Escape('if %HP_HIDDEN_ITER% GEQ 3') $hiRepair = $AllText -match [regex]::Escape('[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=') -$hasHiddenRecover = $hiCall -and $hiLabel -and $hiPayload -and $hiEmit -and $hiCap -and $hiRepair -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)' $hasHiddenRecover @{ call=$hiCall; label=$hiLabel; payload=$hiPayload; emit=$hiEmit; cap=$hiCap; repair=$hiRepair } +# derived requirement (CLAUDE.md Item 28): --collect-submodules=X must be composed and injected +# on the same rebuild as --hidden-import=X -- static guard against silent deletion; runtime proof +# is self.exe.hidden_import's own collectLogged/collectInvoked assertions. +$hiCollectVar = $AllText -match 'set "HP_PYI_HID_COLLECT=' +$hiCollectAppend = $AllText -match [regex]::Escape('set "HP_PYI_HID_COLLECT=%HP_PYI_HID_COLLECT% --collect-submodules=%HP_NEXT_HIDDEN%"') +$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: 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 diff --git a/tests/selfapps_hidden_import.ps1 b/tests/selfapps_hidden_import.ps1 index de1805aa..6fd48b87 100644 --- a/tests/selfapps_hidden_import.ps1 +++ b/tests/selfapps_hidden_import.ps1 @@ -13,6 +13,11 @@ # rejection, 3-iter cap) is unit-tested in tests/test_hidden_import_scan.py; # this is the runtime branch-fired proof. # +# CLAUDE.md Item 28: --collect-submodules=colorama must be paired with --hidden-import=colorama +# on the same rebuild (a --hidden-import target alone only guarantees PyInstaller follows the +# package's own statically-discovered imports, not every real submodule under it -- confirmed via +# a real pygrib failure needing packaging.version even after --hidden-import=packaging alone). +# # Emits: self.exe.hidden_import # Lane: real and conda-full (uv + conda provider coverage). param() @@ -92,6 +97,22 @@ $setupText = if (Test-Path $setupLog) { Get-Content -LiteralPath $setupLog -Raw $combined = ($logLines -join "`n") + "`n" + $setupText $addingFired = $combined -match [regex]::Escape('[REPAIR][HIDDEN_IMPORT] Adding --hidden-import=colorama') +# derived requirement (CLAUDE.md Item 28): --collect-submodules=X must be paired with each +# --hidden-import=X this loop adds, on the SAME rebuild -- a --hidden-import target alone only +# guarantees PyInstaller follows X's own statically-discovered imports, not every real submodule +# under X/ (confirmed via a real pygrib failure needing packaging.version even after +# --hidden-import=packaging alone). +# derived requirement (CodeRabbit finding on PR #419): $collectLogged/$collectPaired below both +# read $combined, i.e. the SAME :log line -- they prove the intended flag text was composed and +# logged together, NOT that the real PyInstaller argv actually received --collect-submodules +# (cmd.exe's own echo is off and PyInstaller does not print its own invocation, so no runtime +# artifact captures the literal composed command line for this file to check independently). +# That independent proof lives in tests/harness.ps1's own $hiCollectInject static check instead, +# which confirms %HP_PYI_HID_COLLECT% genuinely sits in the SOURCE's PyInstaller command line +# immediately after %HP_PYI_HIDDEN_IMPORTS% -- a different file checking a different artifact +# (source text, not a runtime log), which is what makes it a real second, independent check. +$collectLogged = $combined -match [regex]::Escape('--collect-submodules=colorama') +$collectPaired = $combined -match [regex]::Escape('--hidden-import=colorama --collect-submodules=colorama') $recoveredFired = $combined -match [regex]::Escape('[REPAIR][HIDDEN_IMPORT] EXE verified after hidden-import recovery') $infraError = $combined -match 'Failed to parse|uv error|pip error' @@ -120,18 +141,20 @@ if ($exeExists) { } } -$hiddenPass = $exeExists -and ($exeExit -eq 0) -and $tokenFound -and $addingFired -and $recoveredFired -and (-not $infraError) +$hiddenPass = $exeExists -and ($exeExit -eq 0) -and $tokenFound -and $addingFired -and $collectLogged -and $collectPaired -and $recoveredFired -and (-not $infraError) Write-NdjsonRow ([ordered]@{ id = 'self.exe.hidden_import' req = 'REQ-016' pass = $hiddenPass - desc = 'EXE failed on a dynamically-imported installed module; strict --hidden-import recovery rebuilt and verified it' + desc = 'EXE failed on a dynamically-imported installed module; strict --hidden-import recovery rebuilt and verified it (paired with --collect-submodules, CLAUDE.md Item 28)' details = [ordered]@{ exitCode = $run1Exit exeExists = $exeExists exeExit = $exeExit tokenFound = $tokenFound addingFired = $addingFired + collectLogged = $collectLogged + collectPaired = $collectPaired recoveredFired = $recoveredFired infraError = $infraError exePath = $exePath