A .md skill imports as a real Skill with its instructions (#1984); banner visible-in-GUI E2E (#1809); EA calendar docs (#1690) by rbuergi · Pull Request #2284 · Systemorph/MeshWeaver

rbuergi · 2026-08-25T21:22:31Z

Clears three issues in this repo. The largest is #1984; the other two are the platform-side halves of
#1809 and #1690.

Fixes #1984 — a `.md` skill imports as a real Skill, carrying its instructions

This was half done, and the symptom had CHANGED rather than gone away. Step 1 (695b72176,
#1998) made MarkdownFileParser bind front matter case-insensitively, so a nodeType: Skill file
finally landed the node type. What that could not do is put the body where a skill's procedure
lives: the catch-all parser produces MarkdownContent, SkillNodeType is
WithContentType<SkillDefinition>(), and ContentAs<T> recovers only a same-short-named type — so
SkillNodeType.cs:132 read Instructions == null and every such skill was silently empty.

🚨 That intermediate state is worse than the bug it replaced, which is why the new guard asserts on
the CONTENT and not the node type. Before, a broken skill was visible in a listing as a Markdown
page. After, the node claims to be a Skill, appears in the slash-command list, and does nothing —
nothing is red, nothing is logged, and it looks completely normal. A guard keyed on NodeType would
have passed the whole time, so the test class leads with the failure stated as a fact:

WithoutTheContributedParser_TheSkillIsTypedButEMPTY

The fix

A contributed SkillFileParser in MeshWeaver.AI, registered by AddAI beside
AgentFileParser and therefore tried before the catch-all — the same shape, and the same reason,
that ContributedParserPriorityTest already pins for agents. It delegates the FORMAT to
SkillMarkdown (the one place a Skill node ↔ its .md is defined, whose round trip
SkillMarkdownRoundTripTest pins) and supplies only what the chain knows and SkillMarkdown cannot:
the id/namespace derived from the file's path. SkillMarkdown hard-codes the platform Skill
partition — correct for content/ai/Skill, wrong for a plugin shipping Hosting/Skill/deployment.

SkillMarkdown's reader becomes case-insensitive too, and that is required, not tidiness:
MarkdownFileParser.Serialize writes any Skill node it still owns with a PascalCase NodeType:, so
a camelCase-only reader would refuse the file it had just written and the skill would degrade a
second time. Case-insensitive matching is strictly wider than the convention it replaces (it
ignores internal capitalisation too, so launchesSubThread still binds); the writer keeps
camelCase, so the emitted bytes are unchanged.

The write side refuses rather than clobbers. SkillDefinition has no required members, so
deserializing a not-yet-retyped node's MarkdownContent into it succeeds and yields an all-null
definition — claiming that write would emit a skill file with front matter and no body. The parser
declines unless the content is a typed SkillDefinition or an untyped payload whose $type says so,
which leaves that node exactly where it is today.

The backfill needs no migration — measured, not assumed

~11 skills are stored in the broken shape, and the obvious reading is that something must rewrite
them: a LogonAction, or (worse) a SQL migration looping partition schemas. Neither is needed.
UpdateAccordingToSourceNode already applies both
NodeType = source.NodeType ?? state.NodeType and Content = source.Content ?? state.Content, so a
re-import through the fixed parser retypes and refills the node as its ordinary behaviour. The only
open question was whether PackageInstaller.IsUnchanged would SKIP that write and leave the node
broken forever. It does not — for either stored shape — and SkillRetypeOnReinstallTest pins
both, plus the already-correct case so the pin cannot be satisfied by an installer that rewrites
everything every time.

The one node this deliberately does not reach is a claimed one (SyncBehavior != Include), which
DecideAndWrite skips before asking whether anything changed. That is correct: claiming is the
deliberate act that decouples a node from its package.

Fixes #1809 — the stale-build banner is asserted VISIBLE in a browser

Its stated blocker (#1807) is closed, so this was unblocked. StaleBuildBannerTest asserts the
rendered control tree through a real mesh, which proves the server writes into $Banner; it does not
prove a user sees anything. StaleBuildBannerE2ETest drives a real portal, in the shape the issue
specified and without weakening it:

a NodeType that genuinely compiles, plus an instance, so a real assembly path is bound;
negative control first — with the page fully rendered on the current build, no offer text
anywhere (an always-on banner would put "newer build available" above every page in the portal);
a different latestAssemblyPath written through /api/mesh/patch (an RFC 7396 merge routed
through GetMeshNodeStream(typePath).Update, so the collection / framework version / Ok status
are preserved and the build stays usable) — no recompile and no sleep, because the state is a
pure function of two strings;
the banner asserted visible, by text on the rendered element inside its own container;
the instance's own content still rendered underneath — the banner is an adornment, and a page that
lost its content (or recycled itself) is the regression this feature replaced;
a full-page screenshot either way. The one on failure is the point: a Playwright timeout says
only "the locator never matched", while the image says whether the page was blank, still spinning,
showing a compile overlay, or rendering fine with no banner.

Verified against a real launched portal, and it discriminated: before the publish the slot held
the empty StackControl the no-offer path writes; after it, the offer, with the content marker still
below. Both screenshots are attached to the run.

🚨 A finding from that run, reported not fixed

Since db552ffbf (merged today) Memex.Portal.Monolith.csproj has an empty
Condition="'$(Configuration)' == 'Debug'" ItemGroup where the Blazor view-pack references used to
be — while the comment above it still says "Debug-conditioned ships-the-bits keeps local dev
whole". It does not: a dev dotnet run now registers no MarkdownControl view, so every control
renders as its model's ToString(). That is what the screenshots show, and it silently degrades
every E2E test driven by E2E_LAUNCH=1 — a test asserting on text still passes, because the dump
contains the text.

The banner test's assertions are on the offer text inside the banner's own container, which holds in
both worlds, and its doc comment records exactly what that does and does not prove so nobody later
reads a text match as evidence of styled markdown. The dev-portal regression itself belongs to the
modularization programme, not to this PR — flagging it rather than papering over it.

#1690 — the documentation half

The code fix is Systemorph/MeshWeaver.Plugins#677 (the plugin moved out in 11454517a, so the
issue is filed on the wrong repo). What stays here is
src/MeshWeaver.Documentation/Data/AI/ExecutiveAssistant.md, which still documented the three-tool
calendar surface. It now documents GetEvent / UpdateEvent and the read-then-patch flow that
replaces cancel-and-recreate — the shape that cost a user an eight-item checklist on 2026-08-16 — and
corrects the stale claim that the plugin lives in Memex.Portal.Shared.

What's New

Two entries, both Category: Fix: the skills fix, and the Executive Assistant calendar fix (whose
code ships from the plugins repo but whose user-visible change is real, and whose docs live here).
The E2E test is internal and gets none.

Verification

dotnet build -c Release -warnaserror: MeshWeaver.AI (full --no-incremental rebuild),
MeshWeaver.GitSync, MeshWeaver.PluginCatalog, and the three touched test projects — all
0 Error(s) / 0 Warning(s).
SkillFileParserTest 10/10, SkillRetypeOnReinstallTest 3/3.
Existing parser suites unaffected: MarkdownFileParserTest + AgentFileParserTest +
ContributedParserPriorityTest 63/63; SkillMarkdownRoundTripTest +
BuiltInSkillCatalogTest 10/10 (the reader change is the one that could have broken these).
DocumentationLinkIntegrityTest green after rebuilding the embedded doc resources.
StaleBuildBannerE2ETest green against a launched portal (skips in CI like the rest of the suite).

🤖 Generated with Claude Code

…banner E2E (#1809) + EA calendar docs (#1690) #1984 — the second half. Step 1 (695b721) made MarkdownFileParser bind front matter case-insensitively, so `nodeType: Skill` finally landed the node TYPE. What it could not do is put the body where a skill's procedure lives: the catch-all parser produces MarkdownContent, SkillNodeType reads SkillDefinition, and ContentAs<T> recovers only a same-short-named type — so every such skill read Instructions == null. That intermediate state is WORSE than the bug it replaced, which is why the new guard asserts on the CONTENT and not the node type. Before, a broken skill was visible in a listing as a Markdown page; after, the node claims to be a Skill, appears in the slash menu, and does nothing. A guard keyed on NodeType would have passed the whole time — so SkillFileParserTest leads with `WithoutTheContributedParser_TheSkillIsTypedButEMPTY`, which states exactly that. The fix is a contributed SkillFileParser in MeshWeaver.AI, registered by AddAI alongside AgentFileParser and therefore tried before the catch-all — the same shape and the same reason ContributedParserPriorityTest already pins for agents. It delegates the FORMAT to SkillMarkdown, the one place a Skill node ↔ its .md is defined, and supplies only what the chain knows and SkillMarkdown cannot: the id/namespace derived from the file's path (SkillMarkdown places every skill in the platform Skill partition, correct for content/ai/Skill, wrong for a plugin shipping Hosting/Skill/deployment). SkillMarkdown's reader becomes case-insensitive too, and that is required rather than tidy: MarkdownFileParser.Serialize writes any Skill node it still owns with a PascalCase `NodeType:`, so a camelCase-only reader would refuse the file it had just written and the skill would degrade a second time. Case-insensitivity is strictly wider than the convention it replaces; the WRITER keeps camelCase, so SkillMarkdownRoundTripTest still pins the emitted bytes. The write side refuses rather than clobbers: SkillDefinition has no required members, so deserializing a not-yet-retyped node's MarkdownContent into it succeeds and yields an all-null definition. Claiming that write would emit a skill file with front matter and NO BODY. The parser declines unless the content is a typed SkillDefinition or an untyped payload whose $type says so, which leaves that node exactly where it is today — with the Markdown parser. ## The backfill needs no migration, and that is measured, not assumed ~11 skills are stored in the broken shape. The obvious reading is that something has to rewrite them — a LogonAction, or a SQL migration looping partition schemas. Neither is needed: CreateOrUpdateNodeRequest's UpdateAccordingToSourceNode already applies `NodeType = source.NodeType ?? state.NodeType` AND `Content = source.Content ?? state.Content`, so a re-import through the fixed parser retypes and refills the node as ordinary behaviour. The only open question was whether PackageInstaller's unchanged-check would SKIP that write; it does not, for either stored shape, and SkillRetypeOnReinstallTest pins both — plus the already-correct case, so the pin cannot be satisfied by an installer that rewrites everything every time. A CLAIMED node (SyncBehavior != Include) is deliberately not reached: claiming is what decouples a node from its package. Also folded in, both platform-side halves of the same cluster: #1809 — a browser-level assertion that the stale-build banner is VISIBLE, not merely written into the $Banner slot. Negative control first (an always-on banner would put a "newer build available" notice above every page), then a different latestAssemblyPath written through /api/mesh/patch — no recompile, no sleep, because the state is a pure function of two strings — then the banner, then the instance's own content still rendered underneath. Verified against a real launched portal: the run discriminated, the slot holding the empty StackControl of the no-offer path before the publish and the offer after it. Screenshots are written either way; the one on FAILURE is the point, since a Playwright timeout says only "the locator never matched" and the image says which of four very different defects it was. 🚨 That run also surfaced something worth knowing: since db552ff the Blazor view packs have no Debug-only reference in Memex.Portal.Monolith, so a dev `dotnet run` registers no MarkdownControl view and every control renders as its model's ToString(). The test's assertions are on the offer TEXT inside the banner's own container, which holds in both worlds; its doc comment records precisely what that does and does not prove, so nobody later reads a text match as evidence of styled markdown. #1690 — the documentation half. The code fix is Systemorph/MeshWeaver.Plugins#677 (the plugin moved out in 1145451); this repo still documented the three-tool calendar surface, and now documents GetEvent / UpdateEvent and the read-then-patch flow that replaces cancel-and-recreate — the shape that cost a user an eight-item checklist on 2026-08-16. Verified: AI, GitSync, PluginCatalog and the three touched test projects build -c Release -warnaserror with 0 Error(s) / 0 Warning(s); SkillFileParserTest 10/10, SkillRetypeOnReinstallTest 3/3, the existing parser suites 63/63, SkillMarkdownRoundTripTest + BuiltInSkillCatalogTest 10/10, DocumentationLinkIntegrityTest green, and the new E2E green against a launched portal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses three tracked items: ensuring .md-authored Skills import as real Skill nodes with usable Instructions (#1984), adding a browser-level E2E assertion that the stale-build banner is actually visible in the portal UI (#1809), and updating Executive Assistant calendar documentation to reflect the non-destructive read-then-patch flow (#1690).

Changes:

Add a contributed SkillFileParser (registered by AddAI) plus shared path-derivation logic so .md skills land as SkillDefinition with body → Instructions.
Add regression coverage for both parsing and reinstall/retype behavior of previously-broken skills.
Add a Playwright E2E test asserting the stale-build banner is visible in a real browser, plus update EA calendar docs and add two “What’s New” entries.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/MeshWeaver.Portal.E2E.Test/StaleBuildBannerE2ETest.cs	New Playwright E2E test asserting stale-build banner visibility in-browser.
test/MeshWeaver.PluginCatalog.Test/SkillRetypeOnReinstallTest.cs	Pins that reinstall detects both broken stored shapes and rewrites as needed (no migration).
test/MeshWeaver.Content.Test/SkillFileParserTest.cs	Parser-chain regression tests: body → `SkillDefinition.Instructions`, casing, priority, and write-side refusal.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-25-skill-markdown-instructions.md	What’s New entry for markdown-authored skills now importing correctly.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-25-executive-assistant-calendar-edits.md	What’s New entry for EA calendar non-destructive edits + improved error behavior.
src/MeshWeaver.Documentation/Data/AI/ExecutiveAssistant.md	Updates EA calendar tool surface and documents read-then-patch edit flow.
src/MeshWeaver.AI/SkillMarkdown.cs	Makes front matter binding case-insensitive and adds `IsSkillMarkdown` gate for contributed parsing.
src/MeshWeaver.AI/Persistence/SkillFileParser.cs	New contributed `.md` Skill parser + safe write-claim rules to avoid clobbering.
src/MeshWeaver.AI/Persistence/MarkdownNodePath.cs	Shared relative-path → (id, namespace) logic for AI markdown parsers.
src/MeshWeaver.AI/Persistence/AgentFileParser.cs	Refactor to use shared `MarkdownNodePath` derivation.
src/MeshWeaver.AI/AIExtensions.cs	Registers `SkillFileParser` in DI alongside `AgentFileParser` so it wins over the catch-all markdown parser.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot review, correctly: `DeclaresSkillDefinition`'s JsonObject arm read the discriminator with `GetValue<string>()`, which THROWS whenever `$type` is present but is not a JSON string. `FileFormatParserRegistry.GetSerializerFor` calls `CanSerialize` with no catch around it, so that throw would take down a sync-back for the whole node rather than handing the file to the next parser — strictly worse than the clobber this method exists to prevent, and reached by exactly the malformed content it is supposed to guard against. Both arms now read the discriminator with a TRY (`JsonValue.TryGetValue<string>` beside the JsonElement arm's existing ValueKind check), and a theory covers the shapes that reach it from a real mesh — a numeric `$type`, an object `$type`, a null `$type`, and no `$type` at all — asserted against BOTH the JsonElement and the JsonObject shapes, since the two arms fail differently. SkillFileParserTest 14/14; Content.Test builds -c Release -warnaserror, 0 Error(s). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…thout it Not part of this PR's subject, and included only because nothing merges until it is fixed: `AiContentPackDriftTest.EveryFile_StillHashesToItsPinnedReconciliationPoint` fails on `origin/main` itself, so every branch cut from it inherits the red and the green-merge gate stops there. `123f6734c` ("Deployment correctness…") edited `content/ai/Skill/group.md` — adding the Email:Enabled-without-the-module caveat to the invitation section — without re-pinning its ledger entry. The drift guard exists for exactly this and printed the replacement hash; verified by measurement, not inference: git show origin/main:content/ai/Skill/group.md is byte-identical to the working tree, and this branch's diff touches neither content/ai/** nor the ledger, so the drift is attributable to main alone. Only the `core` hash moves. `state` stays `PackAbsent`, which was checked rather than assumed: the MeshWeaver.Plugins master pack ships `Skill/group.json`, not a `group.md`, so there is no counterpart to reconcile and nothing to carry over — the entry keeps `pack: null`, which is the coherence rule `EveryEntry_DeclaresACoherentState` enforces for that state. The 22 other PackAbsent entries carry no note, so a bare hash re-pin is the shape the ledger already uses. AiContentPackDriftTest 15/15. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rbuergi · 2026-08-25T21:37:28Z

Added: a red on `main` this branch had to absorb

AiContentPackDriftTest.EveryFile_StillHashesToItsPinnedReconciliationPoint fails on origin/main itself, so every branch cut from it inherits the red and the green-merge gate stops there. Found while running the full MeshWeaver.AI.Test suite locally (1295 passed, this one failed).

123f6734c ("Deployment correctness…") edited content/ai/Skill/group.md — the Email:Enabled-without-the-module caveat in the invitation section — without re-pinning its ledger entry. The guard exists for exactly that and prints the replacement hash.

Attributed by measurement, not inference: git show origin/main:content/ai/Skill/group.md is byte-identical to the working tree, and this branch's diff touches neither content/ai/** nor the ledger.

Only the core hash moves. state stays PackAbsent, checked rather than assumed — the MeshWeaver.Plugins master pack ships Skill/group.json, not a group.md, so there is no counterpart to reconcile and the entry correctly keeps pack: null (which is what EveryEntry_DeclaresACoherentState requires for that state). The 22 other PackAbsent entries carry no note, so a bare hash re-pin is the shape the ledger already uses.

AiContentPackDriftTest 15/15 after the re-pin.

Happy to split this into its own PR if you would rather land it separately — it is one line, and it is here only because nothing merges without it.

rbuergi · 2026-08-25T21:39:23Z

⚠️ Overlap disclosure: #2288 ("Re-pin Skill/group.md — main is red on the drift guard it just gained") was opened at 21:37:07Z with the identical one-line change; my commit landed within about a minute of it, from a concurrent session. We independently reached the same value.

Whichever merges first, the other side of the merge makes the same edit to the same line, so git resolves it without a conflict and the second one simply becomes a no-op for that hunk.

Prefer #2288 — it is the single-purpose PR and the right home for this. I have kept the commit here only because dropping it would leave this PR red on a defect it did not cause, for as long as #2288 takes to land. If you merge #2288 first I am happy to drop mine and re-push; say the word.

github-actions · 2026-08-25T22:06:06Z