feat(skills): add experimental mural skill with embedded MCP server, instructions, and agent integrations#1561
Open
WilliamBerryiii wants to merge 23 commits into
Open
feat(skills): add experimental mural skill with embedded MCP server, instructions, and agent integrations#1561WilliamBerryiii wants to merge 23 commits into
WilliamBerryiii wants to merge 23 commits into
Conversation
added 11 commits
May 10, 2026 20:59
…server - ship Python CLI (scripts/mural.py) covering workspaces, rooms, murals, widgets, OAuth, and tagging - expose the same surface as a stdio MCP server for chat-agent integration - include OAuth bootstrap, PKCE, redaction, rate-limit handling, and credential storage - add fuzz harness, ~140 corpus seeds, and pytest suite (~30 modules) - bundle SKILL.md, SECURITY.md, .env.example template, and dt-sections asset 🎨 - Generated by Copilot
- mural-human-record, mural-log-hygiene, mural-writeback-hygiene baseline rules - mural-seeding-patterns and mural-writing-style for cross-agent seeding conventions - mural-destinations registry (yaml + instructions) for adapter-aware writeback 🎨 - Generated by Copilot
…-planner, and ux-ui-designer - reference instructions/experimental/mural/ paths from each agent - align with relocated mural seeding-patterns and writing-style guidance 🎨 - Generated by Copilot
… in hve-core-all and project-planning - include experimental mural skill in both collections - register all six experimental mural instructions for the relevant agents 🎨 - Generated by Copilot
- new docs/agents/mural/credentials.md walks through backend selection, oauth bootstrap, devcontainer setup, troubleshooting, and migration - _category_.json registers a collapsible Mural section in the agents sidebar 📝 - Generated by Copilot
… threats - add Mural Skill Runtime as a fifth component category with token cache and PKCE controls - expand boundary table with the Mural Skill Runtime trust boundary - add OAuth Authentication Threats catalog (OA-1 through OA-16) using the extended STRIDE row format - cross-reference MCP Server Trust Analysis from supply-chain and AI threat sections 🔐 - Generated by Copilot
- Validate-SkillStructure errors when tests/ is present but [tool.ruff.lint].select omits 'I' - new Pester guard exercises real ruff against an I001-violating fixture - prevents silent import-order regressions across all Python skills 🛡️ - Generated by Copilot
…ions 🤖 - Generated by Copilot
- reorder mural instruction entries into alphabetic position - widen tables in powerpoint, tts-voiceover, mural seeding, security-model 🎨 - Generated by Copilot
🤖 - Generated by Copilot
🔒 - Generated by Copilot
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF ScorecardScorecard details
Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
9 tasks
added 4 commits
May 10, 2026 22:48
…n terms - add accessibility, security, and sustainability acronyms (ASVS, CAPEC, COGA, CSRD, ESRS, FAPI, PKCE, SSCM, SSDF, TCFD, VPAT) - add generic technical terms used in mural skill and security docs - ignore gitignored dependency-pinning-artifacts/ outputs 📝 - Generated by Copilot
🎨 - Generated by Copilot
shapely declares 'BSD-3-Clause AND LGPL-2.1-only' (LGPL refers to bundled GEOS headers); compound expressions don't match allow-licenses, so allow shapely as a per-package exception. 🔒 - Generated by Copilot
MDX v3 rejects <https://...> autolinks containing path separators as malformed JSX, breaking the Docusaurus build for credentials.md and security-model.md. 🔒 - Generated by Copilot
added 4 commits
May 10, 2026 22:57
- replace '<43 chars' with 'fewer than 43 chars' in security-model.md to satisfy MDX v3 parser - reformat tables in security-model.md and credentials.md after autolink edits 📝 - Generated by Copilot
📝 - Generated by Copilot
…orkers - update test_mural_commands.py token-store concurrency worker to catch Exception - update test_mural_transport.py reactive and proactive refresh workers to catch Exception - preserves intent: KeyboardInterrupt and SystemExit propagate; clears CodeQL py/catch-base-exception 🛡️ - Generated by Copilot
… dependency-review - add backports-tarfile, cryptography, jaraco-classes, jaraco-context, jaraco-functools, keyrings-alt - explain inline that PyPI metadata reports unknown despite upstream Apache-2.0/MIT/BSD-3-Clause/PSF-2.0 licensing - unblocks GitHub Actions dependency-review for the mural skill keyring stack 📦 - Generated by Copilot
bindsi
requested changes
May 11, 2026
Member
bindsi
left a comment
bindsi
left a comment
There was a problem hiding this comment.
Review Verdict: 🛑 Request Changes
Thanks for the substantial work here — the threat-modelled SECURITY.md, the explicit STRIDE buckets, the fuzz harness, and the loopback / PKCE hardening are all in great shape. Two findings on the in-scope skill code are merge-blocking, and two PR-level concerns need addressing before this can be safely reviewed and merged.
Code-level findings (inline comments)
| ID | Severity | File | Summary |
|---|---|---|---|
| RI-1 | 🔒 HIGH | .github/skills/experimental/mural/scripts/mural.py (_run_login, ~L2305) |
OAuth token-exchange silently follows 30x; defeats the SECURITY.md §B2 guarantee and PKCE protection on the code-for-token exchange |
| RI-2 | 📄 MEDIUM | .github/skills/experimental/mural/scripts/mural.py (_load_env_file, L513) |
Function is dead code; SECURITY.md and SKILL.md describe it as the credential parser, but the real parser is FileBackend._read_all |
| RI-5 | .github/skills/experimental/mural/scripts/mural.py (_validate_hyperlink, L2659) |
Missing URL-scheme allowlist; javascript: / data: accepted (defense-in-depth gap, fuzz seed already exists) |
See inline comments for code, repro reasoning, and concrete patches.
PR-level findings
🟧 RI-3 (HIGH) — PR scope significantly exceeds its stated description
The PR description scopes this as "experimental Mural skill + cross-cutting instructions + agent wiring". The diff also bundles, undocumented in the description:
- A complete model-catalog validation subsystem:
scripts/linting/Test-ModelReferences.ps1,scripts/linting/Update-ModelCatalog.ps1,scripts/linting/model-catalog.json+ schema, two*.Tests.ps1files,.github/workflows/model-validation.yml, relatedpackage.jsonlint:modelsentry .github/workflows/dependency-review.ymlpolicy change (no rationale in description)- Many edits across agent/prompt files unrelated to Mural
prompt-frontmatter.schema.json,copilot-instructions.md,.cspell*changes
This is +30,515/-91 across 217 files spanning ~four distinct subsystems. Bundling them under a Mural-skill PR:
- Hides the model-validation subsystem from reviewers focused on Mural OAuth/MCP security
- Couples roll-back: reverting model-catalog requires reverting Mural changes, and vice versa
- Makes
git bisectand post-incident forensics harder - Violates the project's commit-message-scopes convention (
(skills),(scripts),(workflows),(agents),(prompts)are all touched in one PR)
Requested action (preferred): split into at least three PRs before merge —
- Mural skill + instructions + agent wiring (the actual stated scope)
- Model-catalog validation subsystem (
Test-ModelReferences,Update-ModelCatalog,model-catalog.json,model-validation.yml, related tests,package.jsonlint:modelsentry) - Cross-cutting agent/prompt edits unrelated to Mural
If splitting is infeasible at this point, at minimum: amend the PR description to enumerate every subsystem in scope with rationale for shipping together, and add CODEOWNER reviewers for each subsystem touched.
🟨 RI-4 (LOW) — Documentation path drift in PR description
The PR description references docs/security/mural-credentials.md; the file is actually committed at docs/agents/mural/credentials.md. Update the description, and grep the diff for any other docs/security/mural-credentials strings (in SECURITY.md, SKILL.md, instruction files) that need re-pointing so users following the docs from the PR description don't hit 404s.
Audit caveat
Given the size of this PR (12,705-line mural.py alone), this review is a strategic security sampling — not a line-by-line audit. Functions explicitly verified end-to-end against SECURITY.md claims:
- ✅
_redact/_REDACT_KEYS(line 271, 1352) — keys list looks correct, includes RFC 7521 / 8628 / 6749 §4.3 defense-in-depth entries pluscodeform-param and Azure SAS query strings - ✅
FileBackend._read_all/_write_all/_check_credential_file_perms—O_NOFOLLOW,O_EXCLtemp +os.replace,0o077umask, mode-0600 enforcement all match SECURITY.md §B3 - ✅
_LoopbackHandler— single-shot, GET-only,Host-header check, suppressed access logs all present - ✅
_validate_redirect_uri— http-only, loopback-only (rejects[::1]),/callbackpath, no query/fragment - ✅
_validate_asset_url— https + Azure Blob suffix allowlist, rejects userinfo / fragment / IP literals - ✅
_NoRedirect/_TOKEN_OPENER— correctly blocks 30x… for callers that actually use it (see RI-1) - ✅
_parse_token_response— Content-Type guard, capped body, JSON-object guard - ✅
_read_frame/MURAL_MAX_FRAME_BYTES— chunked read, drains oversized frame to next boundary,FrameTooLargeraised - ✅
_validate_tool_input_schema— coverstype/enum/ length / numeric bounds / array items / required / additionalProperties
Functions not sampled in depth (recommend deeper review before merge): _select_profile, _acquire_cache_lock, _apply_refresh, _authenticated_request, _validate_mural_id, _maybe_elicit, _require_scope, _validate_tool_registry, MCP initialize handshake, elicitation framing, the entire _cmd_* CLI surface, the model-catalog PowerShell scripts, and the Validate-SkillStructure.ps1 isort guard.
I'd recommend at least one more reviewer with explicit OAuth/MCP-protocol expertise on the unsampled surface, and a separate reviewer for the model-catalog subsystem if RI-3 is not split out.
To clear this verdict
- Fix RI-1 (single-line default change + regression test)
- Fix RI-2 (delete the dead function or restore + document its behavior)
- Address RI-5 (one-line scheme allowlist)
- Resolve RI-3 (split or amend description + add reviewers)
- Resolve RI-4 (update PR description path)
Happy to re-review once these are addressed — the underlying engineering quality is high, and the issues above are localized.
- default _run_login _http to _TOKEN_OPENER.open to enforce redirect block - remove dead _load_env_file shim; align docs to FileBackend._read_all - reject non-http/https/mailto hyperlink schemes in _validate_hyperlink 🔒 - Generated by Copilot
added 2 commits
May 12, 2026 21:45
- carve scripts/mural.py into a scripts/mural/ package with focused modules - update SECURITY.md, SKILL.md, and tests to reference the package layout - close review minors RV-01/RV-02/RV-03 in constants and logout transparency test - teach Validate-SkillStructure.ps1 to recurse for python package layouts 📦 - Generated by Copilot
…age layout - update dt-coach, ux-ui-designer, and rai-planner agents to reference scripts/mural/ - refresh mural log-hygiene and seeding-patterns instructions for the package layout - update security-model docs to point at the scripts/mural/ package 🔗 - Generated by Copilot
|
|
||
| _TAG_MERGE_MAX_RETRIES = 3 | ||
| _TAG_MERGE_BACKOFF_MIN_MS = 50 | ||
| _TAG_MERGE_BACKOFF_MAX_MS = 200 |
|
|
||
| # Spatial query feature flags. Both default off until widget rotation and | ||
| # parentId field semantics are verified against the live portal. | ||
| _ROTATION_ENABLED = os.environ.get("MURAL_SPATIAL_ROTATION_ENABLED", "0") == "1" |
| # Spatial query feature flags. Both default off until widget rotation and | ||
| # parentId field semantics are verified against the live portal. | ||
| _ROTATION_ENABLED = os.environ.get("MURAL_SPATIAL_ROTATION_ENABLED", "0") == "1" | ||
| _PARENTID_FILTER_ENABLED = os.environ.get("MURAL_SPATIAL_PARENTID_FILTER", "0") == "1" |
| ) | ||
|
|
||
|
|
||
| _LINE_RE = re.compile( |
| # Profile names: 1-32 chars, leading alphanumeric or underscore, then | ||
| # alphanumeric / underscore / dot / hyphen. Rejects "..", path separators, | ||
| # whitespace, and empty strings. | ||
| _PROFILE_NAME_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.-]{0,31}$") |
| # explicit override. The ``authored-by-ai`` tag is auto-attached to every | ||
| # widget created by AI-driven flows so downstream consumers can distinguish | ||
| # AI-authored objects from human-authored ones. | ||
| _RESERVED_TAGS: frozenset[str] = frozenset({"authored-by-ai"}) |
| # widget created by AI-driven flows so downstream consumers can distinguish | ||
| # AI-authored objects from human-authored ones. | ||
| _RESERVED_TAGS: frozenset[str] = frozenset({"authored-by-ai"}) | ||
| _AUTHORED_BY_AI_TAG_TEXT = "authored-by-ai" |
| # Reserved tag text *prefixes* applied by composite/layout flows. Mutating | ||
| # these via tag tools requires `force_reserved=True` just like literal | ||
| # reserved tags. Kept as a separate registry so prefix membership is cheap. | ||
| _RESERVED_TAG_PREFIXES: tuple[str, ...] = ( |
| "ai-author:", | ||
| ) | ||
|
|
||
| _TAG_MERGE_MAX_RETRIES = 3 |
| ) | ||
|
|
||
| _TAG_MERGE_MAX_RETRIES = 3 | ||
| _TAG_MERGE_BACKOFF_MIN_MS = 50 |
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
4 participants
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.
Pull Request
Description
Adds the experimental Mural skill — a Python-based CLI plus embedded stdio MCP server for the Mural REST API — together with the cross-cutting instruction set, agent integrations, collection registrations, documentation, and supporting validation guards needed to operate it.
The skill enables agents (DT Coach, RAI Planner, UX/UI Designer) to seed and extract Mural workspaces, rooms, murals, and widgets while honoring the Mural-as-human-record contract: AI never silently authors decisions, all writebacks remain inspectable in Mural, and operator logs never echo raw URLs, SAS query strings, OAuth tokens, or
Authorizationheaders.Key changes by area:
.github/skills/experimental/mural/package with bash and PowerShell entry scripts, a Python CLI (scripts/mural.py), an embedded MCP stdio server, polyglot Atheris fuzz harness, full pyproject.toml with ruff/pytest config, SECURITY.md, and.env.example..github/instructions/experimental/mural/covering destinations registry, human-record contract, log hygiene, seeding patterns, writeback hygiene, and asymmetric writing style; plus an open destination registry (destinations/registry.yml).dt-coach.agent.md,rai-planner.agent.md, andux-ui-designer.agent.md.hve-core-allandproject-planningcollections register the skill and all six instructions atexperimentalmaturity; entries alphabetized and tables reformatted viaformat:tables.docs/security/mural-credentials.md) wired into the Docusaurus sidebar; security model (docs/security/security-model.md) extended with sixteen Mural-skill threats (OA-1 .. OA-16) covering runtime, OAuth, and writeback surfaces.validate:skillsnow requires the ruffisort(I) rule when a Python skill ships atests/directory, with matching Pester tests.plugins/**artifacts for both affected collections (autogenerated; do not hand-edit).Related Issue(s)
Closes #1562
Type of Change
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
User Request:
Execution Flow:
.github/skills/experimental/mural/SKILL.mdand the cross-cuttingmural-*.instructions.mdset.scripts/mural.sh/mural.ps1) or the embedded stdio MCP server, authenticating via OAuth credentials sourced from environment / keyring permural-credentials.md.mural-seeding-patterns.instructions.md(duplicate-then-populate, anchor inheritance, probe-before-bulk) andmural-writing-style.instructions.md(sticky-concise outbound).mural-writeback-hygiene.instructions.md._redact()backstop; agents additionally avoid echoing URLs, SAS query strings, orAuthorizationheaders permural-log-hygiene.instructions.md.Output Artifacts:
Success Indicators:
Testing
Automated validation performed during PR preparation (commands run from repo root):
npm run lint:md— passnpm run lint:yaml— passnpm run lint:py— passnpm run lint:ps— passnpm run lint:frontmatter— passnpm run lint:md-links— passnpm run lint:marketplace— passnpm run lint:collections-metadata— passnpm run lint:ai-artifacts— passnpm run lint:version-consistency— passnpm run lint:permissions— passnpm run lint:dependency-pinning— passnpm run lint:links— passnpm run lint:models— passnpm run validate:skills— passnpm run validate:copyright— passnpm run plugin:generate— clean (no diff after regen)npm run test:ps— 2178 passed, 0 failed, 3 skipped, 65 not-runnpm run test:py— mural skill tests pass. Two failures in unrelated skills are pre-existing:tts-voiceover—tests/test_embed_audio.pycollection error:ModuleNotFoundError: No module named 'lxml'powerpoint—tests/test_generate_themes.pycollection error:ModuleNotFoundError: No module named 'ruamel'Diff-based assessments:
mural.py,mural.sh,mural.ps1) follow least-privilege patterns and rely on env/keyring for credentials.validate:skillsguard (isort 'I'requirement) is additive; existing skills already comply.Manual testing not performed in this PR.
Checklist
Required Checks
AI Artifact Contributions
/prompt-analyzeto review contributionprompt-builderreviewRequired Automated Checks
npm run lint:mdnpm run spell-check(N/A — not run during PR prep)npm run lint:frontmatternpm run validate:skillsnpm run lint:md-linksnpm run lint:psnpm run plugin:generatenpm run docs:test(N/A — not run during PR prep)Security Considerations
Highlights:
docs/security/security-model.mdcovering script-runtime, OAuth, and writeback surfaces.mural-log-hygiene.instructions.mdis the operator contract; the skill's_redact()is a defense-in-depth backstop, not a license to log secrets.shapely,scipy,networkx,keyring) are scoped to the skill'spyproject.tomland PEP 723 inline metadata; no global dependency surface change.GHCP Artifact Maturity
Warning
This PR includes experimental GHCP artifacts that may have breaking changes.
.github/skills/experimental/mural/SKILL.md.github/instructions/experimental/mural/mural-destinations.instructions.md.github/instructions/experimental/mural/mural-human-record.instructions.md.github/instructions/experimental/mural/mural-log-hygiene.instructions.md.github/instructions/experimental/mural/mural-seeding-patterns.instructions.md.github/instructions/experimental/mural/mural-writeback-hygiene.instructions.md.github/instructions/experimental/mural/mural-writing-style.instructions.mdexperimental/mural/SKILL.md.github/skills/experimental/experimental/mural/mural-destinations.instructions.mdexperimental/mural/mural-human-record.instructions.mdexperimental/mural/mural-log-hygiene.instructions.mdexperimental/mural/mural-seeding-patterns.instructions.mdexperimental/mural/mural-writeback-hygiene.instructions.mdexperimental/mural/mural-writing-style.instructions.mdGHCP Maturity Acknowledgment
Additional Notes
npm run test:pyreports two pre-existing failures (tts-voiceovermissinglxml,powerpointmissingruamel) in skills not modified by this PR. Tracking separately is recommended.plugins/**is autogenerated bynpm run plugin:generate(post-processed bylint:md:fixandformat:tables); do not hand-edit those files.