Skip to content

fix: address remaining issues from downstream feedback#140

Merged
antosubash merged 3 commits intomainfrom
claude/fix-remaining-issues-THnFY
Apr 30, 2026
Merged

fix: address remaining issues from downstream feedback#140
antosubash merged 3 commits intomainfrom
claude/fix-remaining-issues-THnFY

Conversation

@antosubash
Copy link
Copy Markdown
Owner

Closes the four issues left open after #127 and #139.

#137 — Document IModulePermissions for downstream module authors

  • sm new module <Name> now scaffolds <Name>Permissions.cs with a starter
    sealed class implementing IModulePermissions plus four conventional
    constants. The pattern is now visible in every freshly-scaffolded module
    instead of read-the-source-only.
  • modules/Permissions/src/SimpleModule.Permissions/README.md gains a
    worked example targeted at consumers writing their own modules
    (MyApp.Customers / CustomersPermissions), points at the framework
    guide, and notes the auto-discovery behavior.
  • docs/CONSTITUTION.md clarifies that ConfigurePermissions is no
    longer required — the source generator picks up IModulePermissions
    implementations automatically.

#135 — Real-consumer smoke test for packed nupkgs

scripts/smoke-test-nupkg-consumer.mjs:

  1. Discovers UI-shipping modules under modules/.
  2. Spins up a temp Microsoft.NET.Sdk.Web consumer at
    mkdtemp()/Consumer.csproj whose nuget.config points at the local
    nupkgs/ feed.
  3. PackageReferences every UI module by id at the requested version.
  4. dotnet restore && dotnet build -c Release.
  5. Asserts the consumer's staticwebassets.build.json references
    _content/{Module}/{Module}.pages.js for each module.

Wired into the existing pack-smoke job in .github/workflows/ci.yml
right after verify-nupkg-static-assets.mjs. Catches the regression class
where 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.Host because it uses ProjectReference.

Verified locally: all 19 UI module packages pass the consumer build check.

#133 — Extract SimpleModule.Testing

Adds a new packable framework project framework/SimpleModule.Testing containing only the truly cross-app integration-test surface:

  • TestAuthDefaultsAuthenticationScheme = "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.Shared now ProjectReferences it and re-exports
TestAuthScheme for backward compatibility. The 20+ module ProjectReferences and module-specific fixture wiring stay in Tests.Shared (still IsPackable=false); the new package is intentionally narrow so it can ship without dragging the world along.

SimpleModule.Testing picks up IsPackable=true from
framework/Directory.Build.props so it'll be packed automatically by the existing release workflow.

#130 — Boot-time peer-module validator

Adds ModuleGraphValidator (IHostedService, opt-in via
SimpleModuleOptions.ValidateModuleGraph, default true outside
Production). Walks AppDomain.CurrentDomain.GetAssemblies() for
SimpleModule.X.Contracts assemblies, enumerates their I*Contracts
interfaces, and logs a LogWarning for every interface with no
registered implementation:

Module graph: contract IFooContracts is referenced but no implementation is registered. The module providing it (SimpleModule.Foo) appears to be missing — add a PackageReference (or call its registration extension) to fix runtime resolution failures.

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 build clean 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.nupkg is produced by dotnet pack.
  • Manual: drop OpenIddict from a downstream app and observe ModuleGraphValidator warning at startup.

Generated by Claude Code

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.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 30, 2026

Deploying simplemodule-website with  Cloudflare Pages  Cloudflare Pages

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

View logs

claude added 2 commits April 30, 2026 09:03
- 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.
@antosubash antosubash marked this pull request as ready for review April 30, 2026 10:05
@antosubash antosubash merged commit 0830600 into main Apr 30, 2026
6 checks passed
@antosubash antosubash deleted the claude/fix-remaining-issues-THnFY branch April 30, 2026 10:25
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.

2 participants