fix: address remaining issues from downstream feedback#140
Merged
antosubash merged 3 commits intomainfrom Apr 30, 2026
Merged
Conversation
Closes the four issues left open after #127 and #139: - #137 — `sm new module` now scaffolds a `<Name>Permissions.cs` so the IModulePermissions pattern is discoverable without source-diving. SimpleModule.Permissions README documents the convention with a worked example, and the constitution clarifies that ConfigurePermissions is no longer required (auto-discovered). - #135 — adds `scripts/smoke-test-nupkg-consumer.mjs` plus a CI step that builds a throwaway PackageReference-based consumer against the freshly packed nupkgs and asserts each module's static web assets propagate through the static-asset manifest. Catches the regression class where the package looks fine on disk but the consumer's wwwroot pipeline doesn't pick the assets up — invisible to the in-repo Host because it uses ProjectReference. - #133 — extracts `framework/SimpleModule.Testing` (TestAuthHandler, TestAuthDefaults, AddTestAuthentication, CreateAuthenticatedClient WebApplicationFactory extensions) so downstream apps can pull the helpers from NuGet instead of copy-pasting. Tests.Shared now consumes it; existing 40+28+20 (Users/Admin/OpenIddict) and 209/195 (Core/Generator) tests stay green. - #130 — adds `ModuleGraphValidator` (opt-in via `SimpleModuleOptions.ValidateModuleGraph`, default on outside Production). Walks loaded SimpleModule.X.Contracts assemblies and logs a warning for every contract interface with no registered implementation — the typical signal that the impl module's package is missing.
Deploying simplemodule-website with
|
| Latest commit: |
1a89bee
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://353e9e38.simplemodule-website.pages.dev |
| Branch Preview URL: | https://claude-fix-remaining-issues.simplemodule-website.pages.dev |
- ModuleGraphValidator now uses IServiceProviderIsService.IsService instead of GetService, avoiding eager construction of every contract impl + a redundant scope at startup. - WebApplicationFactoryAuthExtensions uses WellKnownClaims.Permission instead of the bare "permission" string. - Smoke-test consumer puts its NuGet global-packages folder outside the project tree so the SDK's default **/*.cs glob can't reach into it, removing the DefaultItemExcludes workaround. - Drops the now-unused TestAuthScheme const alias on SimpleModuleWebApplicationFactory; remaining call site uses TestAuthDefaults.AuthenticationScheme directly.
This was referenced Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the four issues left open after #127 and #139.
#137 — Document
IModulePermissionsfor downstream module authorssm new module <Name>now scaffolds<Name>Permissions.cswith a startersealed class implementing
IModulePermissionsplus four conventionalconstants. The pattern is now visible in every freshly-scaffolded module
instead of read-the-source-only.
modules/Permissions/src/SimpleModule.Permissions/README.mdgains aworked example targeted at consumers writing their own modules
(
MyApp.Customers/CustomersPermissions), points at the frameworkguide, and notes the auto-discovery behavior.
docs/CONSTITUTION.mdclarifies thatConfigurePermissionsis nolonger required — the source generator picks up
IModulePermissionsimplementations automatically.
#135 — Real-consumer smoke test for packed nupkgs
scripts/smoke-test-nupkg-consumer.mjs:modules/.Microsoft.NET.Sdk.Webconsumer atmkdtemp()/Consumer.csprojwhosenuget.configpoints at the localnupkgs/feed.dotnet restore && dotnet build -c Release.staticwebassets.build.jsonreferences_content/{Module}/{Module}.pages.jsfor each module.Wired into the existing
pack-smokejob in.github/workflows/ci.ymlright after
verify-nupkg-static-assets.mjs. Catches the regression classwhere a package looks fine on disk but its MSBuild props/targets fail to
contribute static assets to a real consumer build — invisible to
template/SimpleModule.Hostbecause it usesProjectReference.Verified locally: all 19 UI module packages pass the consumer build check.
#133 — Extract
SimpleModule.TestingAdds a new packable framework project
framework/SimpleModule.Testingcontaining only the truly cross-app integration-test surface:TestAuthDefaults—AuthenticationScheme = "TestScheme",ClaimsHeader = "X-Test-Claims".TestAuthHandler— header-based test auth.services.AddTestAuthentication()extension.WebApplicationFactory<TEntryPoint>.CreateAuthenticatedClient(params Claim[])and(string[] permissions, params Claim[])extensions.SimpleModule.Tests.Sharednow ProjectReferences it and re-exportsTestAuthSchemefor backward compatibility. The 20+ module ProjectReferences and module-specific fixture wiring stay inTests.Shared(stillIsPackable=false); the new package is intentionally narrow so it can ship without dragging the world along.SimpleModule.Testingpicks upIsPackable=truefromframework/Directory.Build.propsso it'll be packed automatically by the existing release workflow.#130 — Boot-time peer-module validator
Adds
ModuleGraphValidator(IHostedService, opt-in viaSimpleModuleOptions.ValidateModuleGraph, defaulttrueoutsideProduction). Walks
AppDomain.CurrentDomain.GetAssemblies()forSimpleModule.X.Contractsassemblies, enumerates theirI*Contractsinterfaces, and logs a
LogWarningfor every interface with noregistered implementation:
Skipped in Production where downstream apps may legitimately ship a
contracts assembly without the impl. This addresses the generic case from
#130 — the two specific peer-dep crashes (Admin→OpenIddict,
Email→BackgroundJobs) were already caught with directive errors in #127.
Test plan
dotnet buildclean across the full solution.dotnet test modules/Users/tests/...— 40 passed.dotnet test modules/Admin/tests/...— 28 passed, 1 skipped.dotnet test modules/OpenIddict/tests/...— 20 passed.dotnet test tests/SimpleModule.Core.Tests/...— 209 passed.dotnet test tests/SimpleModule.Generator.Tests/...— 195 passed.node scripts/verify-nupkg-static-assets.mjs ./nupkgs— 19 verified.node scripts/smoke-test-nupkg-consumer.mjs ./nupkgs --version 0.0.0-ci— 19 verified end-to-end.SimpleModule.Testing.0.0.0-ci.nupkgis produced bydotnet pack.ModuleGraphValidatorwarning at startup.Generated by Claude Code