Skip to content

fix(plugin): correct runtime lifecycle and package edge cases - #21

Merged
MF-Dust merged 12 commits into
mainfrom
fix/plugin-runtime-review-20260827
Aug 27, 2026
Merged

MF-Dust merged 12 commits into
mainfrom
fix/plugin-runtime-review-20260827

Conversation

@MF-Dust

@MF-Dust MF-Dust commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Summary

This PR follows a focused code review of the new NPS 1.0 plugin infrastructure and fixes four concrete runtime inconsistencies:

  • preserve Disabled after DisableAsync() instead of letting ALC cleanup overwrite it with Installed
  • detect copied Nori contract assemblies correctly by comparing assembly names without .dll
  • allow legitimate native Windows DLL dependencies during plugin pre-scan instead of rejecting every PE without .NET metadata
  • align plugin asset/window handling with the manifest's 128-character plugin ID limit

Details

Lifecycle

DeactivateCoreAsync(..., disabled: true) correctly produced Disabled, but UnloadContext() subsequently forced successful unloads back to Installed. ALC cleanup now preserves the lifecycle result selected by the caller and only changes the state when unload actually requires a restart.

Assembly scanning

ContractAssemblyNames contains assembly names such as Nori.Plugin.Abstractions, while the scanner compared them with Path.GetFileName(...) such as Nori.Plugin.Abstractions.dll. The scanner now compares Path.GetFileNameWithoutExtension(...).

The same scanner also treated native .dll files as malformed managed assemblies. Native PE libraries are valid under plugin runtime layouts, so files without CLR metadata are now skipped by the managed-reference scan while managed assemblies continue to be checked for forbidden host references.

Plugin ID consistency

The manifest accepts plugin IDs up to 128 characters. The AssetServer route still rejected IDs over 64 characters, and PluginWindowHost.GetWindowsForPlugin() used the 64-character window-ID validator. Both now follow the plugin-ID limit.

Regression tests

Added coverage for:

  • disabling a discovered plugin remains Disabled
  • contract DLL copies are rejected before loading
  • native Windows DLLs are not rejected by managed assembly scanning
  • public plugin assets work for a valid plugin ID longer than 64 characters

Scope

This intentionally avoids unrelated refactors and does not alter the public NPS contracts.

@MF-Dust
MF-Dust merged commit 14cc5bb into main Aug 27, 2026
5 checks passed
@MF-Dust
MF-Dust deleted the fix/plugin-runtime-review-20260827 branch August 27, 2026 05:11
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.

1 participant