Skip to content

fix: Stop docs and test tasks unloading the caller's modules - #226

Open
tablackburn wants to merge 2 commits into
mainfrom
fix/221-222-module-eviction
Open

fix: Stop docs and test tasks unloading the caller's modules#226
tablackburn wants to merge 2 commits into
mainfrom
fix/221-222-module-eviction

Conversation

@tablackburn

Copy link
Copy Markdown
Contributor

Summary

Fixes #221 and #222, which share a root cause: both Build-PSBuildMarkdown and
Test-PSBuildPester treated Remove-Module -Name as an undo for an import. It is not —
it removes every loaded module with that name, including a copy the caller loaded and
the function never imported. Both sit on default task chains (BuildBuildHelp
GenerateMarkdown, and TestPester), so a build silently emptied the session it
ran in. PowerShellOrg/PSDepend dropped the documentation tasks entirely rather than
live with it.

  • Build-PSBuildMarkdown unloads a module the caller had loaded #221Build-PSBuildMarkdown now records what was loaded, removes only the
    instance it imported, and restores what it displaced. That covers the zero-export path
    too, where it warns and returns without generating anything — return inside try
    still runs the finally.
  • Test-PSBuildPester unloads a module it never imported #222Test-PSBuildPester only removes what it actually imported. Its import is
    conditional on -ImportModule (default $false) while its removal was not, so on the
    default Test chain it unloaded a module it had never touched.
  • Build-PSBuildMarkdown also drops -Global from its import.

Three constraints, each measured on this branch:

  • Scoping the removal alone is not sufficient. Import-Module -Force independently
    evicts a same-path copy before the finally is reached — measured
    same-path: instances=1; original instance object still present? False. Hence
    capture-and-restore rather than only changing the removal.
  • -Force must stay. Without it, importing an already-loaded module returns the
    cached PSModuleInfo: measured WITHOUT -Force: Get-Alpha versus
    WITH -Force: Get-Alpha, Get-Beta after a rebuild. $moduleInfo goes straight to
    New-MarkdownCommandHelp -ModuleInfo, so dropping it would silently document a stale
    command surface.
  • -Global on the docs import is unnecessary. PlatyPS 1.0.3 resolves the module
    through the PSModuleInfo object, never by name. Generated markdown compared
    byte-for-byte before and after: Get-Widget.md, Set-Widget.md, and
    PSBuildTestFixture.md all identical=True. Note -Global is kept on the restore
    import — an import issued from inside the module without it reaches only
    PowerShellBuild's own session state, and the caller would still see nothing.

Test Plan

./build.ps1 -Task Test597 passed, 0 failed, 3 skipped.

Six new tests, driven by a new Invoke-PSBuildModuleEvictionProbe fixture helper that
preloads a module in a background job, invokes the command, and reports what is loaded
and callable on both sides of the call:

Red-before-green verified by reverting only the two source files: the four eviction tests
fail with Expected 1, but got 0; the two "nothing loaded" tests pass either way, which
is their job — they guard against over-restoring.

Breaking Changes

None. This is a behavior fix, but it changes what a build does to your session on
upgrade, so it has CHANGELOG.md and docs/migration-v0.8-to-v1.0.md entries.

One residual cost, documented there: the restored module is a fresh import, not literally
the caller's original instance, so a PSModuleInfo reference held across the call goes
stale. That is the price of keeping -Force, and much smaller than the command
disappearing.

tablackburn and others added 2 commits August 28, 2026 18:08
Build-PSBuildMarkdown and Test-PSBuildPester both ended with
Remove-Module -Name $ModuleName, which removes every loaded module of
that name -- including a copy the caller loaded and neither function ever
imported. Both sit on default task chains, so this reached essentially
every consumer.

Test-PSBuildPester was the worse of the two: its import is conditional on
-ImportModule, which defaults to $false, while its removal was not, so on
the default Test chain it removed a module it had never touched.

Both now record what was loaded before they import, remove only the
instance they created, and restore what they displaced. Scoping the
removal alone is not enough -- Import-Module -Force independently evicts a
copy loaded from the same path -- and -Force has to stay, because without
it the PSModuleInfo reports the previous build's exported commands and the
generated markdown would document a stale surface.

Build-PSBuildMarkdown also drops -Global from its import. PlatyPS resolves
the module through the PSModuleInfo it is handed rather than by name, so
there is no session-state lookup for -Global to serve; the generated
markdown is byte-for-byte identical without it.

Resolves #221
Resolves #222

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011GYJrhbrDzqufaeMqD9QjT
Both fixes change observable behavior on upgrade: a build that used to
silently unload a consumer's module stops doing so. The migration guide
gains an entry per task, including how to tell whether you were affected
and how to put back documentation tasks that were dropped to avoid it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011GYJrhbrDzqufaeMqD9QjT
Copilot AI lite review requested due to automatic review settings August 28, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

Test Results

    4 files  ± 0    893 suites  +4   3m 45s ⏱️ + 1m 18s
  602 tests + 6    599 ✅ + 6   3 💤 ±0  0 ❌ ±0 
2 391 runs  +24  2 319 ✅ +24  72 💤 ±0  0 ❌ ±0 

Results for commit c36cc44. ± Comparison against base commit 3ea9e82.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build-PSBuildMarkdown unloads a module the caller had loaded

2 participants