Repository navigation
content/ai gets a drift guard against its published master, and a leak guard (#1627) - #2263
Merged
Merged
Conversation
…k guard (#1627) `content/ai` is a MIRROR: the served master for the built-in agents and skills is the private `Systemorph/MeshWeaver.Plugins` pack, and this repo's copy is the in-memory fixture the framework suites run against. Nothing compared the two, so drift accumulated in BOTH directions and was only ever found by measuring by hand. Measured on this branch's base (core d6c198c, pack c29a0de), normalizing line endings: 10 of 14 agent files are byte-identical, and exactly 4 diverge — `Assistant.md` (pack ahead), `LogTriage.md` and `Tutor.md` (core ahead, so the SERVED text is the older one), and `CommentingReference.md`, whose ONLY difference is a sanitised example namespace. Note this supersedes the issue's own table: `ToolsReference.md` has since been reconciled and now matches exactly. Why a guard and not a sync. This repo is PUBLIC and the pack repo is PRIVATE, so "copy the master over" would publish a customer's node path here. That one divergence is deliberate and permanent. The guard therefore pins the reconciliation point rather than demanding equality: - `TestData/AiContentPackSync.json` records, per file, the normalized hash of this repo's copy, the attested hash of the master, a state (InSync / CoreAhead / PackAhead / SanitisedOnly / PackAbsent) and a note. A core edit that does not re-pin its entry fails the build — which is the moment someone has to say what they did about the master. - The sanitised entry must keep its placeholder AND stay recorded as diverged, so a future sync cannot quietly "fix" it back to the customer name. - A leak guard refuses any content carrying a forbidden identifier. The deny-list is stored as HASHES and a hit is reported as `file:line` only — a list that spelled the name out, or a failure message that echoed it, would publish in this public repo (and its public CI log) exactly what it exists to keep out. The guard found three live instances of that name already committed here, all in example text, all fixed in this commit: the `/og-card` skill's example path, an OpenGraph parser test fixture URL, and one migration doc comment. All three now use the `Acme/AIConsulting/FinalReport` placeholder the rest of the section uses. Restoring the pre-fix `og-card.md` makes the guard report exactly `Skill/og-card.md:19, Skill/og-card.md:20` and nothing else. Skills are roster- and leak-checked only, not hash-compared: the master still stores its skills as typed `Skill/*.json` nodes, so there is no byte comparison to make against this repo's `.md`-with-front-matter files. Recorded in the ledger. Content reconciliation itself is deliberately NOT in this commit — changing agent instructions is a behaviour change that needs its own review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CI guards to keep content/ai (the public fixture copy) aligned with the privately-served Systemorph/MeshWeaver.Plugins master, and prevents leaking private/customer identifiers via example text. This strengthens long-term maintainability of built-in agent/skill fixtures by making drift and accidental “unsanitised copy” changes fail fast in tests.
Changes:
- Introduces
AiContentPackDriftTestplus a pinned ledger (AiContentPackSync.json) to enforce a stable reconciliation point and a hashed “leak” deny-list (reported asfile:lineonly). - Sanitises example namespaces/paths in
content/ai/Skill/og-card.mdand in related test/doc text to use the placeholderAcme/AIConsulting/FinalReport. - Updates an existing migration doc comment to remove customer/engagement identifiers from public history.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/OpenGraphHtmlParserTest.cs | Replaces a customer-like example URL/path segment with the standard Acme/AIConsulting/FinalReport placeholder. |
| test/MeshWeaver.AI.Test/TestData/AiContentPackSync.json | Adds the pinned pack-sync ledger (core hash, attested pack hash, state, notes) plus hashed forbidden identifiers. |
| test/MeshWeaver.AI.Test/AiContentPackDriftTest.cs | Adds drift-guard + leak-guard tests enforcing the pinned reconciliation point and scanning content/ai for forbidden identifiers. |
| memex/aspire/Memex.Database.Migration/Migrations/V53_RetypeBuiltinSlideDeckToPublish.cs | Sanitises a doc comment to remove customer/engagement identifiers from public source. |
| content/ai/Skill/og-card.md | Sanitises example OgCard area references to use the Acme/AIConsulting/FinalReport placeholder. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…po (review) The section header claimed repository-wide coverage while Section() enumerates content/ai/**/*.md only. That is the surface this guard is about — the tree that MIRRORS the private pack, where copying the master over is how a customer name gets in — but a future maintainer reading the old header would have believed a repo-wide secret scan existed. Scope is now stated in both the class summary and the section, including what it explicitly does NOT cover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
Test Results (shard 4)2 063 tests - 109 1 763 ✅ - 112 8m 33s ⏱️ - 1m 41s Results for commit 11f9937. ± Comparison against base commit 8906cad. This pull request removes 627 and adds 518 tests. Note that renamed tests count towards both. |
Contributor
Test Results (shard 3) 9 files ± 0 9 suites ±0 5m 21s ⏱️ -6s Results for commit 11f9937. ± Comparison against base commit 8906cad. This pull request removes 512 and adds 642 tests. Note that renamed tests count towards both. |
Contributor
Contributor
Test Results (shard 1)1 877 tests +10 1 877 ✅ +10 8m 4s ⏱️ -21s Results for commit 11f9937. ± Comparison against base commit 8906cad. This pull request removes 97 and adds 107 tests. Note that renamed tests count towards both. |
Contributor
Test Results (shard 2)3 280 tests - 5 3 280 ✅ - 5 9m 24s ⏱️ +41s Results for commit 11f9937. ± Comparison against base commit 8906cad. This pull request removes 107 and adds 102 tests. Note that renamed tests count towards both. |
Contributor
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 guard half of #1627.
The problem
content/aiis a mirror. The served master for the built-in agents and skills is the privateSystemorph/MeshWeaver.Pluginspack (MeshWeaver.AI.csprojsays so in as many words); this repo's copy is the in-memory fixture the framework's delegation / picker / tool-wiring suites run against. Nothing compared the two, so drift accumulated in both directions and was only ever found by measuring by hand.Measured on this branch's base (core
d6c198c0f, packc29a0de), normalizing line endings — this supersedes the issue's own table:Agent/ToolsReference.mdAgent/Assistant.mdAgent/LogTriage.mdAgent/Tutor.mdAgent/CommentingReference.mdWhy a guard and not a sync
This repo is PUBLIC; the pack repo is PRIVATE. "Copy the master over" would publish a customer's node path here — that divergence is deliberate and permanent. So the guard pins the reconciliation point rather than demanding equality:
TestData/AiContentPackSync.jsonrecords per file: the normalized hash of this repo's copy, the attested hash of the master, a state (InSync/CoreAhead/PackAhead/SanitisedOnly/PackAbsent) and a note. A core edit that does not re-pin its entry fails the build — which is the moment someone has to say what they did about the master.🚨 The leak guard reports WHERE, never WHAT — please keep it that way
The deny-list is stored as hashes, and a hit is reported as
file:lineonly. A list that spelled the name out, or a failure message that echoed the token, would publish in this public repo — and in its public CI log — exactly what the guard exists to keep out. Please do not "helpfully" add the value to the error message later; the author already knows what is on the line.It found three live instances, all fixed here
All in example text, none a credential — a company name used where a placeholder belongs:
content/ai/Skill/og-card.md— the/og-cardskill's example pathtest/MeshWeaver.Graph.Test/OpenGraphHtmlParserTest.cs— a fixture URL (assertions do not depend on the segment)All three now use the
Acme/AIConsulting/FinalReportplaceholder the rest of the section uses.Proof the guard fires — restoring the pre-fix
og-card.mdand running only the leak guard:Two sites, named precisely, token withheld. Six in-suite controls drive the same scanner over an invented company name in every spelling it could take (one word, spaced, hyphenated, inside a path segment, embedded in a longer camel-cased identifier) and require a hit; three more require it to stay quiet on the real placeholder and on ordinary prose.
Scope — deliberately not in this PR
Skill/*.jsonnodes, so there is no byte comparison to make against this repo's.md-with-front-matter files. Recorded in the ledger's notes.What's New
Skipped deliberately: this is a test guard plus example-text sanitisation, with no user-visible behaviour change.
Verification
dotnet build -c Release -warnaserror—0 Error(s)forMeshWeaver.AI.Test,MeshWeaver.Graph.Test,Memex.Database.MigrationAiContentPackDriftTest— 15/15 pass (section scan ~70 ms)AiContent*/BuiltInSkillCatalog/SkillMarkdownRoundTrip/SkillNodeType— 50/50 passOpenGraphHtmlParserTest— 18/18 pass🤖 Generated with Claude Code