Review and fix Instructions/Skills split (#151): version-pinned docs-sync, coreex-solution-scaffolder rename#165
Open
chullybun wants to merge 6 commits into
Open
Review and fix Instructions/Skills split (#151): version-pinned docs-sync, coreex-solution-scaffolder rename#165chullybun wants to merge 6 commits into
chullybun wants to merge 6 commits into
Conversation
…sync, do-not-edit headers, coreex-solution-scaffolder rename Reviews commit 35508da (Instructions/Skills split) and addresses gaps found: - Repurpose coreex-docs-sync from an unpinned live GitHub-main fetch to a version-pinned refresh, consistent with CoreEx.Template/CoreEx sharing one release version. Rewrote SKILL.md, README.md, and the Claude Code command. - Add version-pin discipline guidance (AGENTS.md, AI-WORKFLOWS.md) so dotnet new coreex* installs are pinned to a specific CoreEx.Template release rather than floating to latest. - Document --force overwrite semantics accurately where referenced. - Stamp a version marker (.github/docs/coreex/manifest.txt) into coreex-ai/coreex-bootstrap output recording which CoreEx.Template release populated the AI workflow assets, enabling informed refresh decisions. - Fix a hardcoded 4.0.0-preview-1 version literal in coreex-expert.agent.md. - Add a dual-audience do-not-hand-edit header across 61 template-sourced files (authored here vs. generated in consumer repos). - Fix three instances of a dotnet new template bug: symbol "replaces" values do an unscoped, global literal text-replace across every packed file, not a scoped placeholder substitution. Replaced human-readable literals ("coreex-version", "app-folder") with sentinel tokens (__COREEX_AI_VERSION_TOKEN__, __COREEX_AI_APP_FOLDER_TOKEN__) to avoid corrupting prose that legitimately mentions those names; removed one unused/dead symbol entirely. - Rename solution-scaffolder skill to coreex-solution-scaffolder for coreex- prefix consistency; update all cross-references. - Fix a real CoreEx.Template packaging gap: coreex-solution-scaffolder was documented as shipping to both CoreEx.Bootstrap and CoreEx.Ai, but the csproj Copy list only shipped it to Bootstrap. Added the missing Copy block. - Document the provenance and intentional divergence of the skill's static docker-compose.local.yml/servicebus-config.template.json fallback assets (a retrofit-safe alternative to CoreEx.Core's conditional, options-aware generated equivalents) via a new assets/README.md. - Fix two now-stale assertions in tools/validate-template-pack.ps1 that contradicted the intentional changes above (.github/docs asserted absent; coreex-solution-scaffolder asserted absent from CoreEx.Ai). Verified via tools/validate-template-pack.ps1: 10/10 scenarios pass. Also manually verified via clean build/pack/install/generate cycles for coreex-ai (single-repo and --app-folder monorepo modes) and coreex-bootstrap. host-setup-extract (splitting remaining host-scaffolding procedures out of coreex-host-setup.instructions.md now that coreex-solution-scaffolder exists) is deliberately deferred to a follow-up PR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens the CoreEx AI workflow asset system after the Instructions/Skills split by making the docs refresh mechanism version-pinned (no live main fetch), fixing packaging/copy gaps, and standardizing cross-references and naming so the installed consumer bundle is consistent and self-describing.
Changes:
- Repurposes
/coreex-docs-syncinto a version-pinned “refresh the whole AI asset bundle” workflow and adds a.github/docs/coreex/manifest.txtversion stamp. - Renames
solution-scaffolder→coreex-solution-scaffolderand updates template packing/validation expectations accordingly. - Updates skills/instructions/prompts to be consumer-agnostic (no local
samples/links), adds dual-audience “do not hand-edit” headers, and fixes template symbol replacement hazards via sentinel tokens.
Reviewed changes
Copilot reviewed 76 out of 77 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/validate-template-pack.ps1 | Updates template-pack validation expectations for new docs manifest + renamed skill. |
| src/CoreEx.Template/CoreEx.Template.csproj | Adjusts AI asset packing/copying, adds docs-cache shipping + manifest generation, ships renamed scaffolder. |
| src/CoreEx.Template/content/CoreEx.Core/AGENTS.md | Updates consumer-facing docs-cache wording to match coreex-ai install behavior. |
| src/CoreEx.Template/content/CoreEx.Bootstrap/.template.config/template.json | Removes unused/dead coreex-version symbol from Bootstrap template. |
| src/CoreEx.Template/content/CoreEx.Ai/.template.config/template.json | Switches replaces to sentinel tokens for safer global template replacement. |
| consumer-instructions/README.md | Updates scaffolder skill path reference to new coreex-solution-scaffolder name. |
| consumer-instructions/.github/copilot-instructions.md | Adds dual-audience “do not hand-edit” header for template-sourced assets. |
| AGENTS.md | Documents version-pin discipline for installing CoreEx.Template + expands installed asset catalog. |
| .github/skills/coreex-validator/SKILL.md | Adds dual-audience header; replaces local sample paths with GitHub URLs + related-skill links. |
| .github/skills/coreex-validator/references/workflow.md | Adds dual-audience header and clarifies examples are illustrative. |
| .github/skills/coreex-test-subscribe/SKILL.md | Adds dual-audience header; strengthens “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-test-subscribe/references/workflow.md | Adds dual-audience header. |
| .github/skills/coreex-test-relay/SKILL.md | Adds dual-audience header; replaces sample literals with placeholders; updates references. |
| .github/skills/coreex-test-api/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-test-api/references/workflow.md | Adds dual-audience header and clarifies examples/placeholder usage. |
| .github/skills/coreex-subscriber/SKILL.md | Adds dual-audience header; consumer-agnostic examples + cross-links. |
| .github/skills/coreex-subscriber/references/workflow.md | Adds dual-audience header; replaces sample literals with placeholders; minor wording cleanup. |
| .github/skills/coreex-solution-scaffolder/SKILL.md | Renames skill; adds version-pin discipline + accurate --force semantics note. |
| .github/skills/coreex-solution-scaffolder/references/workflow.md | Adds dual-audience header; updates asset paths to renamed skill. |
| .github/skills/coreex-solution-scaffolder/README.md | Adds dual-audience header; updates #file: reference to renamed skill path. |
| .github/skills/coreex-solution-scaffolder/assets/servicebus-config.template.json | Adds generic placeholder topic/subscription template asset. |
| .github/skills/coreex-solution-scaffolder/assets/README.md | Documents provenance/drift and why retrofit-safe static assets exist. |
| .github/skills/coreex-solution-scaffolder/assets/docker-compose.local.yml | Adds header comment explaining static fallback usage/provenance. |
| .github/skills/coreex-repository/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-repository/references/workflow.md | Adds dual-audience header. |
| .github/skills/coreex-refdata/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-refdata/references/workflow.md | Adds dual-audience header. |
| .github/skills/coreex-policy/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-policy/references/workflow.md | Adds dual-audience header. |
| .github/skills/coreex-docs-sync/SKILL.md | Rewrites skill as version-pinned bundle refresh; updates manifest semantics and guardrails. |
| .github/skills/coreex-docs-sync/README.md | Aligns README with new version-pinned refresh mechanism + manifest.txt. |
| .github/skills/coreex-db-migration/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-db-migration/references/workflow.md | Adds dual-audience header; adds explicit outbox-provisioning workflow. |
| .github/skills/coreex-contract/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-contract/references/workflow.md | Adds dual-audience header and clarifies examples are illustrative. |
| .github/skills/coreex-app-service/SKILL.md | Adds dual-audience header; expands related-skill routing and references. |
| .github/skills/coreex-app-service/references/workflow.md | Adds dual-audience header. |
| .github/skills/coreex-api/SKILL.md | Adds dual-audience header; updates host-setup routing to scaffolder; adds “resolve from AGENTS.md” guidance. |
| .github/skills/coreex-api/references/workflow.md | Adds dual-audience header. |
| .github/skills/coreex-aggregate/SKILL.md | Adds dual-audience header; strengthens gating rules; updates reference strategy. |
| .github/skills/coreex-aggregate/references/workflow.md | Adds dual-audience header; fixes instruction links in narrative. |
| .github/skills/coreex-adapter/SKILL.md | Adds dual-audience header; adds “resolve from AGENTS.md” guidance; updates references. |
| .github/skills/coreex-adapter/references/workflow.md | Adds dual-audience header. |
| .github/SKILL_AUTHORING.md | Formalizes consumer-agnostic cross-referencing rules and quality gates for skills. |
| .github/prompts/coreex-validator.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-test-subscribe.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-test-relay.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-test-api.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-subscriber.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-scaffold.prompt.md | Adds dual-audience header; updates skill path to renamed scaffolder. |
| .github/prompts/coreex-repository.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-refdata.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-policy.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-db-migration.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-contract.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-app-service.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-api.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-aggregate.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/prompts/coreex-adapter.prompt.md | Adds dual-audience header for prompt wrapper. |
| .github/instructions/coreex-validators.instructions.md | Adds dual-audience header; replaces creation-procedure blocks with skill pointers; updates further reading to docs cache. |
| .github/instructions/coreex-tooling.instructions.md | Adds dual-audience header; extracts procedures to skills; clarifies invariants vs workflows. |
| .github/instructions/coreex-tests.instructions.md | Adds dual-audience header; extracts procedures to skills; adds skill pointers. |
| .github/instructions/coreex-repositories.instructions.md | Adds dual-audience header; adds skill pointers; updates further reading links to docs cache. |
| .github/instructions/coreex-host-setup.instructions.md | Adds dual-audience header; adds skill pointer; updates further reading links to docs cache. |
| .github/instructions/coreex-event-subscribers.instructions.md | Adds dual-audience header; adds skill pointer; updates further reading links to docs cache. |
| .github/instructions/coreex-domain.instructions.md | Adds dual-audience header; adds skill pointer; updates further reading links to docs cache. |
| .github/instructions/coreex-conventions.instructions.md | Adds dual-audience header. |
| .github/instructions/coreex-contracts.instructions.md | Adds dual-audience header; extracts ref-data procedure to skill; adds skill pointers. |
| .github/instructions/coreex-application-services.instructions.md | Adds dual-audience header; adds related-skill pointers; updates further reading links to docs cache. |
| .github/instructions/coreex-api-controllers.instructions.md | Adds dual-audience header; adds skill pointer; updates further reading links to docs cache. |
| .github/INSTRUCTION_AUTHORING.md | Codifies “instructions vs skills” split and consumer-resolvable reference rules. |
| .github/copilot-instructions.md | Adds rule to resolve project-wide choices from solution-root AGENTS.md; updates catalog pointers. |
| .github/AI-WORKFLOWS.md | Documents skills/prompts/instructions split and version-pin discipline; updates command catalog. |
| .github/agents/README.md | Updates consumer adoption guidance for agent/asset installation. |
| .github/agents/coreex-expert.agent.md | Aligns agent doc-cache behavior with version-pinned refresh and manifest.txt semantics. |
| .claude/commands/coreex-expert.md | Adds dual-audience header; points to agent file as authoritative. |
| .claude/commands/coreex-docs-sync.md | Rewrites Claude command to match version-pinned “refresh bundle” mechanism. |
- .github/agents/README.md: pin dotnet new install CoreEx.Template to an explicit ::<version> in the adoption instructions (was unpinned, contradicting the version-pin discipline described elsewhere), and stop describing /coreex-docs-sync as a required first-time population step now that dotnet new coreex-ai already installs .github/docs/coreex/ at the pinned version -- docs-sync is a refresh mechanism for later version bumps only. - .github/skills/coreex-docs-sync/SKILL.md and .claude/commands/coreex-docs-sync.md: fix an inaccurate dry-run claim. Empirically verified that omitting --force is NOT always non-mutating: dotnet new coreex-ai without --force only blocks and lists conflicts when target files already exist and differ; if none of the target files exist yet, it silently creates them for real with no confirmation. Switched Step 4 to use --dry-run explicitly, which is verified to never write regardless of target state, and updated the corresponding guardrail text. - src/CoreEx.Template/CoreEx.Template.csproj: AI-WORKFLOWS.md is linked from many shipped skills (/.github/AI-WORKFLOWS.md) but was not actually packaged into either CoreEx.Bootstrap or CoreEx.Ai -- a real broken-link gap. Added it to both the Bootstrap _AiFile list and the Ai Copy block, added the dual-audience do-not-hand-edit header to the file itself (it now ships to consumers), and updated the packaging comment inventory. - tools/validate-template-pack.ps1: added FilesPresent assertions for .github/AI-WORKFLOWS.md in both coreex-ai scenarios to guard against regressing this packaging gap. Verified via tools/validate-template-pack.ps1 (10/10 scenarios pass) and a manual coreex-bootstrap generation confirming AI-WORKFLOWS.md ships there too. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ReferenceDataCollectionCore<TId,TRef> is ConcurrentDictionary-backed with no implied enumeration order (string hash randomization varies per process), so comparing raw serialized JSON strings on round-trip was flaky (~1 in 15 runs failing locally). Assert via GetItems() (stable, sorted List<TRef>) instead of BeEquivalentTo directly on the collection - the latter throws NotSupportedException because AwesomeAssertions/ LINQ ToArray() fast-paths through ICollection<TRef>.CopyTo, which this collection deliberately does not support. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- copilot-instructions.md: pin the CoreEx.Template install instruction and reword the /coreex-docs-sync row to describe its full-bundle refresh scope (matches the wording already used in agents/README.md). - validate-template-pack.ps1: add FilesAbsent regression guards for the legacy .github/skills/solution-scaffolder folder name in both coreex-ai scenarios, so a future packaging change can't reintroduce the old name alongside coreex-solution-scaffolder. - Further Reading sections in 7 instructions files: add GitHub source-link fallbacks (samples/docs/*.md) alongside the docs-sync cache links, so the references resolve both in this repo (where the cache doesn't exist) and in consumer repos (where it does) - matching the pattern already used for the per-package agents/CoreEx.*.md guide links. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… scaffold reference - Use 'dotnet new details CoreEx.Template' to verify installation instead of 'dotnet new list --tag CoreEx' - shows the specific package's installed version and full template list directly, rather than relying on a tag filter match. - Document the CoreEx.Template::<version> pinned-install form alongside the bare install for first-time adopters. - Remove the /coreex-scaffold bullet from the Claude Code section and the 'IDE AI chat' framing on the Alternative: AI-guided scaffold section - /coreex-scaffold is a Copilot-only prompt (no .claude/commands/coreex-scaffold.md exists); added a note pointing Claude Code users at the manual dotnet new coreex* commands or attaching the coreex-solution-scaffolder skill directly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
|
||
| ## Step 6 — Flag possible orphans | ||
|
|
||
| Compare the skill folder names under `.github/skills/coreex-*/` against the current known L1 skill set (listed in [`AI-WORKFLOWS.md`](https://github.com/Avanade/CoreEx/blob/main/.github/AI-WORKFLOWS.md)). Any extra folder is very likely left over from a prior template version and should be flagged for manual review, not assumed safe to delete automatically. |
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
Reviews commit 35508da (the Instructions/Skills split from #151) and fixes the gaps found, per the following review scope: validate the Instructions-vs-Skills content split for an AI consumer ("keep Instructions lean and invariant-only"), validate/create cross-references now that the full skill suite exists, and keep everything domain-agnostic and consistent across AGENTS.md/README.md files.
What changed
coreex-docs-syncrepurposed: was an unpinned live fetch from GitHubmain, inconsistent withCoreEx.Template/CoreExsharing one release version. Now does a version-pinned refresh only. RewroteSKILL.md,README.md, and the Claude Code command (.claude/commands/coreex-docs-sync.md, which was stale and still described the old mechanism).AGENTS.md/AI-WORKFLOWS.mdsodotnet new coreex*installs pin to a specific release instead of floating to latest.--forceoverwrite semantics documented accurately where referenced.dotnet new coreex-ai/coreex-bootstrapnow write.github/docs/coreex/manifest.txtrecording whichCoreEx.Templaterelease populated the AI workflow assets.4.0.0-preview-1literal incoreex-expert.agent.md.dotnet newbug: template symbol"replaces"values do an unscoped, global literal text-replace across every packed file, not a scoped placeholder substitution. Human-readable literals (coreex-version,app-folder) were replaced with sentinel tokens (__COREEX_AI_VERSION_TOKEN__,__COREEX_AI_APP_FOLDER_TOKEN__) to stop them corrupting unrelated prose that legitimately mentions those names. One unused/dead symbol was removed entirely.solution-scaffolder→coreex-solution-scaffolderfor prefix consistency; updated all cross-references.coreex-solution-scaffolderwas documented as shipping to bothCoreEx.BootstrapandCoreEx.Ai, but the csprojCopylist only shipped it to Bootstrap. Added the missingCopyblock.docker-compose.local.yml/servicebus-config.template.jsonfallback assets are a retrofit-safe alternative toCoreEx.Core's conditional, options-aware generated equivalents (used when re-running the root scaffold would unsafely overwrite hand-written code). Addedassets/README.mdexplaining why they exist, where they came from, and that they're manually — not automatically — reconciled.tools/validate-template-pack.ps1that contradicted the changes above (.github/docswas asserted absent;coreex-solution-scaffolderwas asserted absent fromCoreEx.Ai).Verification
tools/validate-template-pack.ps1: 10/10 scenarios pass.coreex-ai(single-repo and--app-foldermonorepo modes) andcoreex-bootstrap, inspecting output byte-for-byte for correct file placement, header survival, and no sentinel-token leakage.Deferred
host-setup-extract(splitting remaining host-scaffolding procedures out ofcoreex-host-setup.instructions.mdnow thatcoreex-solution-scaffolderexists) is deliberately deferred to a follow-up PR — it's a larger, independent change and not required to close out this review.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com