Dual-write Compass includes and legacy skills on registered plugins. by RedTanny · Pull Request #167 · RHEcosystemAppEng/agentic-plugins

RedTanny · 2026-11-08T15:04:44Z

The live processor requires spec.skills, while spec.includes is the ADR membership list. Keep the old dependsOn/dependencyOf graph so the pipeline stays green, and teach the same shape in the validator and authoring skills.

Summary

Dual-write Compass plugin membership on the four registered plugins: structured spec.includes (skills + owned MCP) and legacy spec.skills (skill refs only). The live ModelProcessor still requires skills; includes is the ADR membership list. Functional dependsOn / dependencyOf are unchanged so the publication pipeline needs no engine change.

GitLab ingest YAML is already merged. This is the public mirror, plus GitHub-only validator and authoring templates. Lightspeed stays mcpserver:redhat/red-hat-lightspeed-mcp-server on this tree (no mcps/lightspeed-mcp.yaml); it is not in includes. Canonical redhat/security-mcp-server on rh-basic stays dependsOn only.

Also updates validate_compass_manifests.py (reject empty/duplicate/self/plugin/string includes), tests, compass-manifest-maintenance, agentic-contribution-skill, CLAUDE.md, and CONTRIBUTING.md so the next skill is not authored against dependencyOf alone.

Plugin(s) affected

Change type

Contribution method

Created/imported with /agentic-contribution-skill
Manual contribution (validated with make validate + make validate-skill-design-changed)

Plugin-persona alignment (new skills only)

N/A — no new skills.

AGENTS.md compliance

N/A — Compass descriptor / tooling only; no skill or agent behavior change.

Validation

make validate-compass-manifests and scripts/test_validate_compass_layout.py pass locally
make validate full suite (run before merge if not already)
New/changed skills have valid YAML frontmatter (name, description) — N/A
New/changed agents have valid YAML frontmatter (name, description) — N/A

The live processor requires spec.skills, while spec.includes is the ADR membership list. Keep the old dependsOn/dependencyOf graph so the pipeline stays green, and teach the same shape in the validator and authoring skills. Co-authored-by: Cursor <cursoragent@cursor.com>

r2dedios

A couple of structural suggestions on the new _validate_plugin_dual_write / includes validation, from a review pass on this PR.

r2dedios · 2026-11-10T09:17:59Z

+        if member_type is not None and member_type not in _ALLOWED_INCLUDE_TYPES:
+            errors.append(f"{loc} type must be skill or mcp-server, got {member_type!r}")
+        if member_type == "plugin":
+            errors.append(f"{loc} must not include a plugin member")


Missing continue after rejecting a plugin member

if member_type == "plugin": errors.append(f"{loc} must not include a plugin member")

This branch doesn't continue, so a rejected entry still falls through to the entityRef prefix classification a few lines below (entity_ref.startswith("airesource:ai5-marketplace/")) and gets added to include_skills. If that entityRef isn't also in spec.skills, the same bad line produces a second, unrelated-looking error (spec.includes lists unknown skills [...]) on top of "must not include a plugin member".

Example: {entityRef: airesource:ai5-marketplace/ocp-admin, type: plugin} raises both messages today for one bad line. Adding continue right after errors.append(...) here would report the one real problem once; the surrounding for loop still validates every other spec.includes entry — this wouldn't abort the check early.

r2dedios · 2026-11-10T09:17:59Z

+        if entry.get("gitRef") is not None:
+            errors.append(f"{loc} must not set gitRef")
+        member_type = entry.get("type")
+        if member_type is not None and member_type not in _ALLOWED_INCLUDE_TYPES:


Request: tighten includes validation to match what this PR documents

Compass's own schema doesn't catch either of these for us — I validated a few spec.includes payloads directly against the live Compass validate-entity endpoint, and it accepts entries with no type at all, and even accepts a type that contradicts the entityRef prefix (e.g. {entityRef: mcpserver:..., type: skill} validates cleanly). So our own validator is the only thing that can enforce this:

type is currently optional (member_type = entry.get("type"), only checked if member_type is not None) and never cross-checked against the entityRef prefix classification below. Since {entityRef, type} is the whole point of the new structured format, could type be made required and verified against the ref (airesource: → skill, mcpserver: → mcp-server)?

_check_skill_conventions doesn't reject spec.includes / spec.skills / spec.system on skill manifests, even though relationship-rules.md (added in this PR) documents them as plugin-only: "includes / spec.skills / spec.system on skills — plugins only for the first two". A copy-pasted plugin block landing on a skill's catalog-info.yaml would currently pass make validate-compass-manifests.

Happy to help with either if useful.

r2dedios · 2026-11-10T09:18:14Z

Add a written reference for "Phase C H.5" / "H.4"

This PR introduces "Phase C H.5" and "H.4" as the trigger for retiring the skill→plugin dependsOn edge and the legacy spec.skills dual-write, across 6 places:

CLAUDE.md#L90
.claude/skills/compass-manifest-maintenance/SKILL.md#L128
.claude/skills/compass-manifest-maintenance/references/relationship-rules.md (L8, L19, L32)
.claude/skills/compass-manifest-maintenance/assets/plugin-catalog-info.yaml#L72
scripts/validate_compass_manifests.py#L8

We checked the repo and its git history and couldn't find this defined anywhere in writing — it's unrelated to the older "Phase A/B/C" rollout in specs/001-collection-consistency-audit/plan.md (a different, now-deleted feature doc from an earlier PR).

Could you add a short written reference (a ticket link, or even just one sentence) for what "Phase C H.5"/"H.4" means and when it's considered reached? Right now the only record of it seems to be a verbal agreement from a meeting, so there's no way for anyone outside that conversation — or for CI — to know when the dual-write and the extra dependsOn edge are supposed to come out.

r2dedios

A few more findings from a deeper pass on the includes validation logic and the retired-dependencyOf doc wording.

r2dedios · 2026-11-10T12:06:44Z

+        if entity_ref in seen:
+            errors.append(f"{rel}: duplicate includes entityRef {entity_ref}")
+        seen.add(entity_ref)
+        if entity_ref == plugin_ref:


Same missing-continue bug as the type: plugin check, here too

if entity_ref == plugin_ref: errors.append(f"{rel}: includes must not self-reference the plugin")

No continue here either, so a self-referencing entry falls through to the entityRef prefix classification just below and gets added to include_skills (the plugin's own airesource: ref matches that prefix). On rh-basic, a self-referencing includes entry produces three errors for one bad line: the correct self-reference message, a bogus spec.includes lists unknown skills ['rh-basic'], and a cascading roster-mismatch error. Same fix as the type: plugin case: add continue right after errors.append(...) here.

r2dedios · 2026-11-10T12:06:44Z

+            name = entity_ref.split("/", 1)[1]
+            include_skills.add(name)
+        elif entity_ref.startswith("mcpserver:"):
+            if entity_ref.startswith("mcpserver:redhat/"):


Canonical-MCP check misses mcpserver:default/*

if entity_ref.startswith("mcpserver:redhat/"): errors.append(f"{loc} canonical MCP {entity_ref} must not be in includes")

This hardcodes "mcpserver:redhat/", but the file already has a shared _CANONICAL_MCP_PREFIXES = ("mcpserver:redhat/", "mcpserver:default/") tuple used elsewhere (e.g. _is_allowed_dangling_mcp). A canonical ref under mcpserver:default/* placed in includes doesn't get this specific error — it only hits the generic spec.includes lists unexpected MCPs fallback, and if that ref also happens to be in owned_mcps, nothing would catch it at all. Worth using _CANONICAL_MCP_PREFIXES here instead of the literal string.

r2dedios · 2026-11-10T12:06:44Z

+        for ref in skill_refs
+        if ref.startswith("airesource:ai5-marketplace/")
+    }
+    if skill_names_legacy and skill_names_legacy != disk:


Roster-mismatch checks silently no-op when every spec.skills entry is malformed

skill_names_legacy = { ref.split("/", 1)[1] for ref in skill_refs if ref.startswith("airesource:ai5-marketplace/") } if skill_names_legacy and skill_names_legacy != disk:

skill_names_legacy silently drops entries that fail the airesource: prefix check instead of counting them as wrong. If every spec.skills entry is malformed, skill_names_legacy ends up empty, and this if skill_names_legacy and ... guard — plus the parallel spec.skills vs spec.includes check further down — both get skipped entirely, even though the roster genuinely diverges from disk. The equivalent spec.includes check (include_skills != disk) isn't guarded this way, so the two checks aren't actually at parity today.

r2dedios · 2026-11-10T12:06:44Z

 #### Entity Relationships

-Use `dependsOn` / `dependencyOf` for custom kinds (`AiResource`, `MCPServer`); do not use `partOf` / `hasPart` on those kinds. Compass does not auto-generate inverse relations (COMPASS-1288) — declare both sides on every edge.
+Plugin **membership** is `spec.includes` (skills + owned MCP) **and** legacy `spec.skills` (skill refs only, required by the live processor). Functional deps stay on `dependsOn` (skill→MCP, skill→skill, plugin MCP union, skill→plugin until Phase C H.5). Do **not** list skills on plugin `dependencyOf` — that roster is retired.


This sentence contradicts itself for the skill→plugin edge

Functional deps stay on dependsOn (skill→MCP, skill→skill, plugin MCP union, skill→plugin until Phase C H.5). ... Do not list skills on plugin dependencyOf — that roster is retired.

This classifies skill→plugin dependsOn as a functional edge, and the sentence right before this paragraph (in the surrounding section) states Compass doesn't auto-generate inverse relations for custom kinds (COMPASS-1288) and that we "dual-write dependsOn/dependencyOf on every functional edge." But this same sentence then forbids the plugin-side dependencyOf inverse for that identical edge. relationship-rules.md#L15 repeats the same pattern — "Every functional relationship must be declared on both sides" immediately followed by a table row exempting Skill→Plugin from it.

Since compass-manifest-maintenance reads relationship-rules.md as its own reference when authoring manifests, this reads as a genuine contradiction rather than a documented exception — could the skill→plugin edge be called out explicitly as the exception to the bidirectional rule, rather than appearing to violate the rule stated one sentence earlier?

r2dedios · 2026-11-10T12:06:50Z

Two more issues from a deeper pass on this PR

1. validate_plugin() can crash instead of reporting a clean error

plugin_spec = plugin_data.get("spec", {}) only defaults to {} when the spec key is absent. A plugin YAML with spec: present but empty (valid YAML, parses to None) makes plugin_spec itself None, and _validate_plugin_dual_write then calls plugin_spec.get("skills"), raising an uncaught AttributeError that crashes make validate-compass-manifests for every plugin in that run, instead of reporting a clean per-file error for the one that's actually broken. Suggest plugin_data.get("spec") or {}.

2. support_level disagrees between the two sources for the same plugin

All 4 registered plugins now carry two different values for the same field:

Plugin	`<plugin>-plugin.yaml` (this PR)	`.catalog/collection.yaml` (unchanged)
ocp-admin	`developer-preview`	`Unknown`
rh-basic	`developer-preview`	`Unknown`
rh-sre	`developer-preview`	`Unknown`
rh-virt	`developer-preview`	`Unknown`

Same field name, two unreconciled values per plugin, and nothing validates that they match. Any downstream consumer reading one source vs. the other (catalog pipeline vs. Compass) would report a different support status for the same plugin. Worth picking one source of truth, or syncing the two.

RedTanny · 2026-11-10T13:58:47Z

Hi @r2dedios
fixed issues
commits -
d66752a

bb8695b

for following issues

Missing continue after type: plugin — fixed
Rejected plugin includes still fell through into skill classification and produced a second “unknown skills” error. Skip that entry after the plugin-member error.
Tighten includes validation — fixed
Compass does not enforce {entityRef, type}. type is now required and must match the ref (airesource: → skill, mcpserver: → mcp-server). Skill manifests now reject plugin-only spec.includes / spec.skills / spec.system.
Document “Phase C H.5” / “H.4” — deferred
Those names are the later drop of spec.skills and skill→plugin dependsOn. No ticket yet, so no in-repo pointer.
Missing continue on self-include — fixed
Same fallthrough as (1). Self-reference no longer cascades into unknown-skill / roster errors.
Canonical MCP check missed mcpserver:default/* — fixed
Includes now use _CANONICAL_MCP_PREFIXES instead of only mcpserver:redhat/.
Malformed spec.skills skipped roster checks — fixed
If every spec.skills entry failed the prefix filter, disk and skills vs includes checks were skipped. Those compares always run now.
Skill→plugin contradicted the bidirectional rule — fixed
Called out as the exception: skill dependsOn plugin until H.5, no plugin dependencyOf inverse. CLAUDE.md, relationship-rules.md, maintenance skill.
Empty YAML spec: crashed the validator — fixed
plugin_data.get("spec") or {} so spec: reports missing fields instead of AttributeError.
support_level mismatch — fixed
Schema is a free label string; we still picked one value. Compass already had developer-preview. Synced .catalog on the four registered plugins (ocp-admin, rh-basic, rh-sre, rh-virt) and regenerated collection.json. Unregistered packs still Unknown.

RedTanny requested review from dmartinol, ikrispin, nemerna and r2dedios as code owners November 8, 2026 15:04

RedTanny requested a review from rhartuv November 8, 2026 15:05

RedTanny added 2 commits November 9, 2026 14:22

retire old schemas

a7e5490

publication completeness on GitHub

dbfc971

r2dedios reviewed Nov 10, 2026

View reviewed changes

RedTanny added 2 commits November 10, 2026 15:15

validator fixes

d66752a

code review fixes

bb8695b

RedTanny requested a review from r2dedios November 10, 2026 13:59

Navigation Menu

Repository navigation

Dual-write Compass includes and legacy skills on registered plugins. - #167

Dual-write Compass includes and legacy skills on registered plugins.#167
RedTanny wants to merge 5 commits into
RHEcosystemAppEng:mainfrom
RedTanny:align-to-compass-schema

RedTanny commented Nov 8, 2026

Uh oh!

r2dedios left a comment

Uh oh!

r2dedios Nov 10, 2026

Uh oh!

r2dedios Nov 10, 2026

Uh oh!

r2dedios commented Nov 10, 2026

Uh oh!

r2dedios left a comment

Uh oh!

r2dedios Nov 10, 2026

Uh oh!

r2dedios Nov 10, 2026

Uh oh!

r2dedios Nov 10, 2026

Uh oh!

r2dedios Nov 10, 2026

Uh oh!

r2dedios commented Nov 10, 2026

Uh oh!

RedTanny commented Nov 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

RedTanny commented Nov 8, 2026

Summary

Plugin(s) affected

Change type

Contribution method

Plugin-persona alignment (new skills only)

AGENTS.md compliance

Validation

Uh oh!

r2dedios left a comment

Choose a reason for hiding this comment

Uh oh!

r2dedios Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

r2dedios Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

r2dedios commented Nov 10, 2026

Uh oh!

r2dedios left a comment

Choose a reason for hiding this comment

Uh oh!

r2dedios Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

r2dedios Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

r2dedios Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

r2dedios Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

r2dedios commented Nov 10, 2026

Uh oh!

RedTanny commented Nov 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants