Fix EventMarkdownGenerator: multiline descriptions, generic partition-key discovery, split topic fields#284
Merged
Merged
Conversation
…-key discovery, split topic fields (#283) - Collapse whitespace (not just strip newlines) in property/partition-key descriptions so multi-line XML doc summaries render on a single table row instead of breaking the markdown table. - Discover partition-key attributes generically by name (configurable via --partition-key-attribute), mirroring event/topic discovery, instead of a hardcoded PartitionKeyAttribute type. Order is resolved via reflection. - Split EventMetadata/EventViewModel.TopicName into Topic (plain topic/hub name) and FullyQualifiedTopicName (the {env}.domain.visibility.topic.version convention). TopicName is removed (breaking change, no back-compat shim). - Support an optional EventName property on the event attribute to override the documented event name. - Add CHANGELOG.md and a pre-PR hook (scripts/check-changelog-before-pr.sh) that blocks `gh pr create` until CHANGELOG.md has an entry for the branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
…Api, Refit) - Enable CentralPackageTransitivePinningEnabled so transitive dependency versions can be overridden from Directory.Packages.props. - Pin MessagePack to 2.5.301, patching GHSA-hv8m-jj95-wg3x and related LZ4/MessagePack CVEs pulled in transitively by Aspire.Hosting.AppHost (and Orleans, when included). Verified via GlobalPackageReference first, which compiled fine but forces an explicit reference into every project; switched to a plain PackageVersion pin relying on transitive pinning instead, and moved it out of the INCLUDE_ORLEANS conditional after confirming via `dotnet new mmt` (with and without --orleans) that the vulnerable dependency comes from AppHost regardless of Orleans. - Pin Microsoft.OpenApi to 2.7.5, patching GHSA-v5pm-xwqc-g5wc. A prior attempt via GlobalPackageReference broke the build because it forced an explicit project reference that conflicted with how the Microsoft.AspNetCore.OpenApi source generator resolves its dependency; plain transitive pinning avoids that. - Bump Refit 10.1.6 -> 11.0.1: CentralPackageTransitivePinningEnabled's stricter validation surfaced a pre-existing floor violation (Refit.HttpClientFactory 11.0.1 requires Refit >= 11.0.1). - Bump MicrosoftExtensionsVersion 10.0.9 -> 10.0.10 (latest .NET 10 patch, verified available across the whole Microsoft.Extensions.* group). - Suppress NU5104 in libs/Momentum/Directory.Build.props (library-authoring build only, excluded from template output): OpenTelemetry.Instrumentation.GrpcNetClient has never shipped a stable release, so packing Momentum.ServiceDefaults.Api / Momentum.Extensions.Messaging.Kafka in Debug always trips "stable package depends on prerelease" once the security-audit gate above it is cleared. Verified: AppDomain.slnx builds and restores clean (0 errors). Confirmed the fix propagates into `dotnet new mmt` output by actually generating projects with and without --orleans and building them (0 errors both). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
vgmello
commented
Jul 15, 2026
Owner
Author
Review summaryReviewed for correctness, security, performance, test coverage, and repo guideline compliance (no 1 correctness bug found, left as inline comments:
Everything else looked solid: the generic attribute-by-name discovery refactor, Not approving — leaving this for a human to confirm the fix. |
Supersedes Dependabot #280, #279 (partial), #278, #273, #271, #276 (partial), #281 (Refit only), #263, #264: - Aspire 13.4.3 -> 13.4.6 - Microsoft.Orleans* 10.1.0 -> 10.2.1 (+ VersionOverride for Microsoft.Orleans.Analyzers in AppDomain.Api/AppDomain, matching how the source-generator reference is pinned independently of the main Orleans package group) - Google.Protobuf 3.35.0 -> 3.35.1, Grpc.Tools 2.80.0 -> 2.82.0 - OpenTelemetry.Instrumentation.AspNetCore 1.15.2 -> 1.16.0, OpenTelemetry.Instrumentation.GrpcCore beta.11 -> beta.13 - Scalar.AspNetCore 2.16.3 -> 2.16.11 - Testcontainers/.Kafka/.PostgreSql 4.12.0 -> 4.13.0 - Refit and Refit.HttpClientFactory 11.0.1 -> 13.1.0 (bumped together; HttpClientFactory 13.1.0 has an exact floor on Refit 13.1.0) - MessagePack 2.5.301 -> 2.5.302 (Aspire 13.4.6's StreamJsonRpc raised its own floor) - @types/node 25.x -> ^26.0.0 in both docs sites, lockfiles regenerated Intentionally NOT applied, each broke the build in isolation: - Microsoft.CodeAnalysis.Analyzers/CSharp/Common 5.0.0 -> 5.6.0 (PR #279, partial): JasperFx.RuntimeCompiler 4.5.0, pulled in by WolverineFx 5.39.2, hard-pins Microsoft.CodeAnalysis(.CSharp/.Scripting) to exactly 5.0.0. This bump is only safe alongside the WolverineFx major upgrade (PRs #282/#277), which is being deferred separately. - NSubstitute 5.3.0 -> 6.0.0 (PR #276, partial): 6.0.0 added proper nullable annotations to its public API (e.g. the CallInfo indexer), which surfaces CS8600 in ~38 existing call sites across the unit test suite that cast `x[0]` and read members into non-nullable properties. Needs a real test-code pass, not a drop-in version bump. - Refitter.MSBuild 2.0.0 -> 2.1.0 (PR #281, partial): fails code generation at build time with `MissingMethodException: System.Text.ValueStringBuilder.AsSpan()`. Unrelated to the Refit runtime library bump above, which is unaffected and kept. CentralPackageTransitivePinningEnabled (enabled in the prior commit) caught two additional floor violations these bumps triggered transitively, both fixed here: MessagePack needed 2.5.302 for Aspire 13.4.6's StreamJsonRpc, matching the earlier fix for a similar Refit floor issue. Verified: AppDomain.slnx and libs/Momentum/Momentum.slnx both build with 0 errors. Full test run: 175 passed / 34 failed (all Integration tests failing solely on missing local Docker, same pre-existing baseline as before this change) + 604 passed / 0 failed across all libs test projects. GitHub Actions bumps (#270 actions/cache, #262 actions/checkout) are gated behind a PreToolUse hook requiring separate confirmation before editing .github/workflows/*, not yet applied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
Supersedes Dependabot #262 (actions/checkout v5 -> v7) and #270 (actions/cache v4 -> v6). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
This was referenced Jul 15, 2026
CreateEventMetadata set Topic = eventDomain instead of Topic = topicName, so the new Topic field always rendered the same value as Domain instead of the plain topic/hub name used to build FullyQualifiedTopicName. Also fixes the GenericAttributeDiscoveryTests assertion that encoded the bug, and regenerates the affected scenario baselines. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VJZH8avHkNqGcDUw5ZX2L1
Field names are self-explanatory; the comments didn't add anything beyond what the code already says. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
The VersionOverride reference added while replicating Dependabot #271 was unconditional, so every generated project got it -- not just Orleans-enabled ones. Without the rest of Orleans present, the analyzer throws NullReferenceException ('Object reference not set to an instance of an object') and fails the build. The root repo's own AppDomain.slnx build didn't catch this because its sample app already pulls in Orleans transitively via AppDomain.BackOffice.Orleans. Reproduced via `dotnet new mmt` (matching CI's "Template Tests (Real-World Patterns)" failures on TestDefault, TestApiNoBackOffice, TestDefaultNoSample) and confirmed the fix: default, --orleans, --backoffice false, and --no-sample all now build with 0 errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
Fixes SonarCloud findings on EventViewModelFactory.cs:210 -- regex timeout vulnerability and maintainability suggestion to use GeneratedRegexAttribute. Source-generated regex removes both concerns: no runtime Regex instance to time-bound, and no ReDoS surface on this trivial \s+ pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B
|
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
1 participant
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.
Summary
Fixes #283.
--partition-key-attributeoption added, mirroring--event-attribute. Discovery resolves the attribute by name (or prefix) and readsOrdervia reflection, instead of a hardcodedPartitionKeyAttributetype — so custom partition-key attributes now work.EventMetadata/EventViewModel.TopicNameis replaced byTopic(plain topic/hub name) andFullyQualifiedTopicName(the{env}.domain.visibility.topic.versionconvention string). This is a breaking rename with no back-compat shim (intentional, per maintainer). The defaultevent.liquidtemplate now renders both.EventNameoverride: event attributes may expose an optionalEventNameproperty to override the documented event name.CHANGELOG.mdand aPreToolUsehook (scripts/check-changelog-before-pr.sh) that blocksgh pr createuntilCHANGELOG.mdhas a dated entry for the branch — documented inCLAUDE.md.Test plan
dotnet test tests/Momentum.Extensions.EventMarkdownGenerator.Tests— 143 passed, 3 skipped, 0 failedGenericAttributeDiscoveryTestscovers custom-named event + partition-key attributes, ordering, andEventNameoverrideFluidMarkdownGeneratorTestscover multi-line description collapsing and the split topic fieldsgh pr createcorrectly (manual exit-code test)Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01DqMbT47Qhz4ZjjccAEQL6B