Add coreex-aggregate skill (DDD domain objects)#162
Merged
Conversation
…cing skills coreex-db-migration and coreex-refdata skills had decision tables keyed on Contoso domain names (Products/Shopping/Orders) rather than the consumer's actual database type choice. Replace with generic PostgreSQL vs SQL Server rows and 'check Program.cs' guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the coreex-aggregate skill for DDD domain objects (aggregate
root, entity, value object) built on CoreEx.DomainDriven. Kept
domain-agnostic per request (generic {Aggregate}/{Entity}/{ValueObject}
placeholders) rather than tied to the Contoso.Shopping.Domain sample.
- .github/skills/coreex-aggregate/SKILL.md - entry point: when to use,
quick reference, interview-driven branching (aggregate root vs entity
vs value object)
- .github/skills/coreex-aggregate/references/workflow.md - full workflow:
Phase 0 (confirm Domain layer warranted), Phase 1 interview, Step 1-3
patterns, PersistenceState transition rules, guard helpers, integration
events (not domain events), guardrails
- .github/prompts/coreex-aggregate.prompt.md - prompt bridge
- Wired into CoreEx.Template.csproj
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds a 'Unit Testing Aggregates' section to workflow.md, generalized from samples/tests/Contoso.Shopping.Test.Unit/Domains/BasketTests.cs. Aggregates have no injected dependencies (beyond reference data), making them ideal traditional unit-test targets for both happy-path and guard-rejection logic. Covers: WithGenericTester<EntryPoint> + Test.Scoped(...) pattern, arranging via CreateFrom, invoking mutation methods directly, asserting OnCheckCanMutate guard failures as thrown BusinessException vs other methods returning a failed Result, reference-data test wiring, and how this differs from (and doesn't replace) request-validator tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new coreex-aggregate skill to guide developers in creating/modifying DDD domain objects (aggregate roots, child entities, value objects) using CoreEx.DomainDriven, and wires the skill assets into the template packaging so they ship with generated .github/ AI assets.
Changes:
- Added
.github/skills/coreex-aggregate/skill entrypoint plus a detailed workflow reference (including aggregate unit testing guidance). - Added a
/coreex-aggregateprompt bridge under.github/prompts/. - Updated
src/CoreEx.Template/CoreEx.Template.csprojto copy/pack the new prompt + skill files.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/CoreEx.Template/CoreEx.Template.csproj | Includes the new coreex-aggregate prompt/skill files in the AI content pack and copy steps. |
| .github/skills/coreex-aggregate/SKILL.md | Introduces the coreex-aggregate skill (scope, quick reference, and links to authoritative references). |
| .github/skills/coreex-aggregate/references/workflow.md | Provides the end-to-end workflow for aggregates/entities/value objects and aggregate unit testing guidance. |
| .github/prompts/coreex-aggregate.prompt.md | Adds the agent-mode prompt wrapper to invoke the skill consistently. |
- Introduce {NewIdExpression} placeholder instead of hard-coding
Runtime.NewId() in the aggregate root / child entity examples, since
Runtime.NewId() always returns string and would be wrong/misleading
for a Guid (or other) {IdType}. Rules now explain to pick
Runtime.NewId() vs Runtime.NewGuid() based on the actual id type.
- Corrected the Test.Scoped(...) rationale: Runtime.NewId()/NewGuid()
resolve via IdentifierGenerator.Current and do not need an ambient
ExecutionContext; only Runtime.UtcNow and reference-data validity
checks (ThrowIfInactive) do.
- Replaced quoted "{id}" literals in the unit-test examples with an
unquoted {id} placeholder so the pattern doesn't imply string-only
identifiers.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Summary
Adds the
coreex-aggregateskill for adding/modifying DDD domain objects (aggregate roots, child entities, value objects) built onCoreEx.DomainDriven, per the L1 skills inventory in #151.Intentionally domain-agnostic — unlike sample-anchored skills (
coreex-adapter,coreex-subscriber), this skill uses generic{Aggregate}/{ChildEntity}/{ValueObject}placeholders throughout rather than referencingsamples/src/Contoso.Shopping.Domain/directly, per explicit direction for this skill.What's included
.github/skills/coreex-aggregate/SKILL.md— entry point: when to use / not use, quick reference, key references (instructions files + actualCoreEx.DomainDrivensource, not samples).github/skills/coreex-aggregate/references/workflow.md— full workflow:Aggregate<TId,TSelf>,CreateNew/CreateFrom,OnCheckCanMutate/OnMutate)Entity<TId,TSelf>, internal mutation methods, consumer-authoredClone(PersistenceState)helper)sealed record, invariants ininitaccessors)samples/tests/Contoso.Shopping.Test.Unit/Domains/BasketTests.cs: aggregates have no injected dependencies (beyond reference data), making them the best unit-test target in the codebase for both happy-path and business-rule-rejection coveragePersistenceStateReference (with actual transition nuances —NewstaysNewonModify,Removeis terminal + read-only), Integration Events (not domain events) rationale, Guardrails.github/prompts/coreex-aggregate.prompt.md—/coreex-aggregateprompt bridgesrc/CoreEx.Template/CoreEx.Template.csprojKey rules captured
private;CreateNew(...)/CreateFrom(...)are the only public construction pathsModify(...)/Remove(...)are the only mutation paths —Remove(...)is terminal (setsRemoved+MakeReadOnly(), no restore path)internal, neverpublicOnCheckCanMutate()guard failures throw the exception carried by the failedResult(typicallyBusinessException); other mutation methods may instead return a failedResultdirectly (e.g. not-found) — tests need to assert each correctlyAggregate<TId,TSelf>.Events, forwarded by the Application layerWithGenericTester<EntryPoint>+Test.Scoped(...)) cover mutation logic that request-validator tests structurally cannot reachValidation
rubber-ducksub-agent twice (once for the base skill, once for the added testing section) — each pass caught real accuracy issues before commit (child-entity factory visibility,ThrowOnError()overstatement,PersistenceStatetransition nuances,Clone()mislabeling, a nonexistentRuntime.ClockAPI reference, an overbroad "always throws" claim, an overstated validator claim, and overly narrow reference-data wiring guidance) — all fixeddotnet build src/CoreEx.Template/CoreEx.Template.csproj -c Release— 0 warnings, 0 errorsCo-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com