feat(ai-integrations): add 'mcp-registry-provider' openspec files - #4667
feat(ai-integrations): add 'mcp-registry-provider' openspec files#4667michael-valdron wants to merge 7 commits into
Conversation
Signed-off-by: Michael Valdron <mvaldron@redhat.com> Assisted-by: Claude Opus 4.8
Signed-off-by: Michael Valdron <mvaldron@redhat.com> Assisted-by: Claude Opus 4.8
|
🤖 Review · ❌ Terminated · Started 6:35 PM UTC · Ended 6:37 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4667 +/- ##
=======================================
Coverage 62.50% 62.50%
=======================================
Files 2621 2621
Lines 104902 104902
Branches 29463 29463
=======================================
Hits 65568 65568
Misses 37519 37519
Partials 1815 1815
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 6:35 PM UTC · Completed 6:37 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-5 |
|
/fs-fix Apply the following fix from previous review comment: |
|
🤖 Finished Fix · ❌ Failure (validation failed after 2 iteration(s)) · Started 6:43 PM UTC · Completed 6:46 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-5 |
|
/fs-fix Apply the following fix from previous review comment: |
|
🤖 Finished Fix · ✅ Success · Started 3:38 PM UTC · Completed 3:42 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $0.76 |
Change schema value from rhdh-spec-driven to spec-driven in both .openspec.yaml files to match the established convention used by all other openspec changes in the repository. Addresses review feedback on #4667 Assisted-by: Claude Opus 4.6
|
🤖 Finished Review · ✅ Success · Started 3:44 PM UTC · Completed 3:57 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $5.03 |
ReviewFindingsLow
Previous runReviewVerdict: Approve — no blocking findings. This draft PR adds OpenSpec design documentation for two related changes:
All 15 files are specification/design documents (proposals, design docs, specs, tasks, audits, and journals) under ObservationsWell-designed aspects:
Low-severity notes (non-blocking):
Checklist AssessmentAll PR checklist items are unchecked, which is appropriate for a draft/WIP PR containing only specification documents — no changeset, tests, or screenshots are needed for documentation-only openspec additions. Previous run (2)ReviewFindingsMedium
Low
Previous run (3)Review — commentPR: #4667 — feat(ai-integrations): add 'mcp-registry-provider' openspec files This PR adds 15 new specification/design files across two well-structured openspec change sets ( Three medium-severity findings identified during review, all related to spec accuracy that could affect implementation correctness: Medium1. The spec, design (D10), and proposal describe emitting Remediation: Update the spec, design D10, and proposal to state that the 2. The spec states that when two distinct Remediation: Clarify the hash-suffix application rule. The simplest correct approach: always append a stable hash suffix derived from the pre-sanitization canonical values when sanitization changes any character or the combined length exceeds 63 characters. 3. D9 secret redaction does not prune D9 explicitly lists Remediation: Extend D9 to also prune the Low
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)Review — commentSummaryThis PR contributes OpenSpec design/specification documents for two related features in the
All 15 files are new documentation/specification artifacts (proposals, designs, task lists, specs, audits, journals). No production code is changed. The PR is appropriately marked as a draft with the The specifications are thorough and well-structured, with good separation of concerns between the mapping contract and the provider runtime. Design decisions are well-reasoned with alternatives considered and rejected. Cross-document consistency is strong overall. Several spec underspecifications and convention deviations are noted below for resolution before implementation begins. FindingsMedium1. D10 specifies that Remediation: Specify that the 2. The collision disambiguation scenario states that when two distinct (name, version) pairs sanitize to the same Remediation: Either (a) always append the hash suffix when sanitization modifies the input (preemptive, self-contained), or (b) explicitly state that cross-document collision is delegated to the provider. 3. Empty registry edge case not addressed — The provider spec covers multi-page, single-page, and error scenarios, but not the empty-registry case. An HTTP 200 with Remediation: Add a scenario for the empty-registry case: document it as expected behavior, or add a safeguard (e.g., log a warning, or treat zero-server responses as suspicious). 4. PR title understates scope The title says "add 'mcp-registry-provider' openspec files" but the PR contributes specs for two features: Remediation: Update title to reflect both features, e.g., 5. Missing H1 title headings — Six of seven existing sibling openspec change directories use Remediation: Add H1 headings to match the established pattern (e.g., Low6. The worked example shows 7. Dot-in-key-name ambiguity — JSON keys in 8. Secret redaction depends on upstream No defense-in-depth mechanism for unmarked secrets. A credential-bearing input without 9. Novel file types — Neither file type exists in the 7 existing ai-integrations openspec changes, introducing new conventions without precedent. If intentional, document the expected file set. 10. Six of seven existing siblings include Dimensions reviewed
Labels: PR adds MCP Registry integration specs; documentation label applies to spec-only changes |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-fix
|
|
🤖 Finished Fix · ✅ Success · Started 4:16 PM UTC · Completed 4:23 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $1.96 |
Add status: draft to both .openspec.yaml files to match the convention used by six of seven existing sibling changes. Add H1 headings to proposal.md, design.md, tasks.md, and audit.md in both mcp-registry-provider and mcp-registry-server-mapping to match the established heading pattern (e.g., # Proposal: Title). Addresses review feedback on #4667 Assisted-by: Claude Opus 4.6
|
🤖 Finished Review · ✅ Success · Started 4:25 PM UTC · Completed 4:45 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $6.45 |
fullsend-ai-review
Bot
added
requires-manual-review
|
/fs-fix Restructure each decision body under |
|
🤖 Finished Fix · ✅ Success · Started 7:59 PM UTC · Completed 8:14 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $2.52 |
… pattern Restructure each decision body in mcp-registry-server-mapping/design.md and mcp-registry-provider/design.md to use the established label pattern (Choice/Alternatives considered/Rationale) matching the format used by other design.md files in the openspec/changes directory. Addresses review feedback on #4667 Assisted-by: Claude Opus 4.6
|
🤖 Review · ❌ Terminated · Started 8:15 PM UTC · Ended 8:37 PM UTC Commit: |
775bfaf to
5ffcbcc
Compare
|
🤖 Review · ❌ Terminated · Started 5:30 PM UTC · Ended 5:35 PM UTC Commit: |
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
|
🤖 Finished Review · ✅ Success · Started 5:30 PM UTC · Completed 5:35 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $1.12 |
PR Summary by QodoSpecify MCP registry mapping and catalog provider
AI Description
Diagram
High-Level Assessment
Files changed (15)
|
|
🤖 Review · ❌ Terminated · Started 5:43 PM UTC · Ended 6:02 PM UTC Commit: |
Code Review by Qodo
1. Malformed updates delete valid entries
|
|
Important The |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
workspaces/ai-integrations/openspec/changes/mcp-registry-provider/audit.md(file-level): Line 34 · [low] Internal consistency
The audit.md reports zero findings (0 CRITICAL, 0 WARNING, 0 SUGGESTION), but the sibling journal.jsonl records the audit found 1 WARNING (schedule type name drift) and 1 SUGGESTION (cursor safeguard wording). The mcp-registry-server-mapping audit.md properly documents its resolved findings for comparison.
fullsend-ai-review
Bot
added
ready-for-merge
|
🤖 Finished Review · ✅ Success · Started 5:43 PM UTC · Completed 6:02 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $6.30 |
johnmcollier
left a comment
johnmcollier
left a comment
There was a problem hiding this comment.
Overall, this aligns largely with what I would expect from an entity provider ingesting from an MCP registry. One question that stands out to me:
How do we want to handle collisions between multiple MCP registries? What if multiple MCP registries emit the same catalog entity identity? We might want to consider some kind of namespacing per-mcp registry?
@johnmcollier Good point, supporting multiple MCP Registries would make sense. Though not listed as out of scope, the current design from the feature only considers one MCP Registry at a time:
|
That being said I could revise a bit at least to have the consideration that multiple registries may be supported down the road to avoid future catalog breakages. |
2 participants
Hey, I just made a Pull Request!
Contributes openspecs for MCP Registry Provider plugin implementation.
https://redhat.atlassian.net/browse/RHIDP-15658
✔️ Checklist