Skip to content

feat(scorecard): Scorecard mcp actions#3332

Merged
yangcao77 merged 13 commits into
redhat-developer:mainfrom
yangcao77:scorecard-mcp
Jun 19, 2026
Merged

feat(scorecard): Scorecard mcp actions#3332
yangcao77 merged 13 commits into
redhat-developer:mainfrom
yangcao77:scorecard-mcp

Conversation

@yangcao77

@yangcao77 yangcao77 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Enable Scorecard MCP actions for listing and fetching entity metrics
✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Walkthroughs

User Description

Hey, I just made a Pull Request!

for issue: https://redhat.atlassian.net/browse/RHIDP-14049
https://redhat.atlassian.net/browse/RHIDP-14050

This PR enables action registry in scorecard plugin, and also introduced 2 actions and their tests are included:

  1. list metrics
  2. get entity metrics

see recording:

scorecard-mcp.mov

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
AI Description
• Enable Scorecard backend as an MCP Actions source via actions registry integration.
• Add list-metrics and get-entity-metrics actions enforcing Scorecard read permissions.
• Add tests and docs, plus sample backend config for MCP authentication.
Diagram
graph TD
  A(("MCP client")) --> B["MCP Actions backend"] --> C["Actions Registry"] --> D["Scorecard actions"]
  D --> E["CatalogMetricService"] --> F[("Scorecard metrics DB")]
  D --> H["MetricProvidersRegistry"]
  E --> G["Catalog Service"]

  subgraph Legend
    direction LR
    _act(("Client")) ~~~ _svc["Service/Module"] ~~~ _db[("Database")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Rely on existing Scorecard REST endpoints (no MCP actions)
  • ➕ No new action schemas to maintain
  • ➕ Reuse existing HTTP router behavior and tooling (OpenAPI, curl, etc.)
  • ➖ MCP clients work best with explicit action contracts
  • ➖ Less discoverable and harder for agents to call safely
2. Generate MCP actions from Scorecard metric/provider registry dynamically
  • ➕ Avoids hand-maintaining action schemas as metrics evolve
  • ➕ Could expose richer discovery (datasources, supported filters) automatically
  • ➖ Harder to keep schemas stable for clients
  • ➖ More abstraction and testing complexity for limited initial action set

Recommendation: Current approach is appropriate for an initial MCP integration: explicit, stable actions (list-metrics, get-entity-metrics) with permission enforcement reuse existing conditional authorization utilities. Consider dynamic generation only if the number of actions/metrics grows and schema maintenance becomes a burden.

Grey Divider

File Changes

Enhancement (5)
App.tsx Register Scorecard plugin feature in the NFS app +2/-1

Register Scorecard plugin feature in the NFS app

• Adds the Scorecard plugin itself to the frontend feature list, alongside the existing Scorecard modules.

workspaces/scorecard/packages/app/src/App.tsx


getEntityMetrics.ts Add get-entity-metrics MCP action +93/-0

Add get-entity-metrics MCP action

• Registers an MCP action that returns latest metric values for a given entity ref and enforces 'scorecardMetricReadPermission' using conditional authorization.

workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts


index.ts Introduce Scorecard actions registration entrypoint +37/-0

Introduce Scorecard actions registration entrypoint

• Adds a 'createScorecardActions' helper to register Scorecard MCP actions in one place and re-exports action factories.

workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts


listMetrics.ts Add list-metrics MCP action +70/-0

Add list-metrics MCP action

• Registers an MCP action that lists available metric definitions and filters them using permission conditions.

workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts


plugin.ts Wire actions registry into Scorecard backend plugin init +13/-0

Wire actions registry into Scorecard backend plugin init

• Adds 'actionsRegistryServiceRef' dependency and registers Scorecard MCP actions during plugin initialization.

workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts


Tests (2)
getEntityMetrics.test.ts Add tests for get-entity-metrics MCP action +155/-0

Add tests for get-entity-metrics MCP action

• Covers successful metric retrieval, empty results, permission denial, and not-found entity behavior for the entity metrics action.

workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts


listMetrics.test.ts Add tests for list-metrics MCP action +90/-0

Add tests for list-metrics MCP action

• Validates metric listing output and permission-denied behavior for the metrics discovery action.

workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts


Documentation (1)
README.md Document Scorecard MCP actions and setup +64/-0

Document Scorecard MCP actions and setup

• Documents available Scorecard MCP actions, how to enable them, required auth config, and a sample Cursor 'mcp.json' configuration.

workspaces/scorecard/plugins/scorecard-backend/README.md


Other (4)
sour-cameras-say.md Add changeset for Scorecard MCP actions release +5/-0

Add changeset for Scorecard MCP actions release

• Introduces a minor-version changeset for the scorecard backend package to ship new MCP actions.

workspaces/scorecard/.changeset/sour-cameras-say.md


app-config.yaml Add sample MCP Actions backend configuration (commented) +11/-0

Add sample MCP Actions backend configuration (commented)

• Adds a commented example showing how to enable 'backend.actions.pluginSources' for Scorecard and configure static token external access for MCP clients.

workspaces/scorecard/app-config.yaml


package.json Add MCP Actions backend dependency and bump backend-defaults +2/-1

Add MCP Actions backend dependency and bump backend-defaults

• Adds '@backstage/plugin-mcp-actions-backend' and bumps '@backstage/backend-defaults' to a newer version to support the integration.

workspaces/scorecard/packages/backend/package.json


index.ts Enable MCP Actions backend plugin in Backstage backend +1/-0

Enable MCP Actions backend plugin in Backstage backend

• Registers '@backstage/plugin-mcp-actions-backend' in the backend startup pipeline.

workspaces/scorecard/packages/backend/src/index.ts


Grey Divider

Qodo Logo

yangcao77 added 3 commits June 1, 2026 18:17
Signed-off-by: Stephanie <yangcao@redhat.com>
Signed-off-by: Stephanie <yangcao@redhat.com>
Signed-off-by: Stephanie <yangcao@redhat.com>
@rhdh-gh-app

rhdh-gh-app Bot commented Jun 8, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
app workspaces/scorecard/packages/app none v0.0.0
backend workspaces/scorecard/packages/backend none v0.0.0
@red-hat-developer-hub/backstage-plugin-scorecard-backend workspaces/scorecard/plugins/scorecard-backend minor v2.7.9

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jun 8, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Tickets: RHIDP-14049

Grey Divider


Action required

1. Entity access check missing ✓ Resolved 🐞 Bug ⛨ Security
Description
createGetEntityMetricsAction returns metrics for any entityRef after checking only
scorecardMetricReadPermission, without enforcing catalogEntityReadPermission for that entity.
Because CatalogMetricService.getLatestEntityMetrics reads the entity using the plugin’s own
service credentials, callers can retrieve metrics for catalog entities they may not be allowed to
read.
Code

workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts[R77-88]

+    action: async ({ input, credentials }) => {
+      const { conditions } = await authorizeConditional(
+        credentials,
+        permissions,
+        scorecardMetricReadPermission,
+      );
+
+      const metrics = await catalogMetricService.getLatestEntityMetrics(
+        input.entityRef,
+        undefined,
+        conditions,
+      );
Relevance

⭐⭐⭐ High

Team previously fixed same exposure by requiring catalog entity read access for metrics (PR #1609).

PR-#1609

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The MCP action only performs scorecard permission authorization and then fetches metrics directly,
while the REST endpoint performs an additional catalog entity access check; the underlying service
uses its own service credentials to resolve entities, so without that check the MCP action can
bypass entity visibility constraints.

workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts[77-90]
workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts[98-110]
workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts[104-133]
workspaces/scorecard/plugins/scorecard-backend/src/permissions/permissionUtils.ts[92-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The `get-entity-metrics` MCP action does not enforce catalog entity read authorization for the requested `entityRef`, which can expose metrics for entities the caller cannot read.

### Issue Context
The REST route `GET /metrics/catalog/:kind/:namespace/:name` explicitly checks entity access via `catalogEntityReadPermission` before calling `CatalogMetricService.getLatestEntityMetrics`, but the MCP action skips that check. Additionally, `CatalogMetricService.getLatestEntityMetrics` fetches the entity using `auth.getOwnServiceCredentials()`, so it will succeed regardless of caller catalog visibility unless the action layer blocks it.

### Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts[77-90]
- workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts[32-155]
- workspaces/scorecard/plugins/scorecard-backend/src/service/router.ts[104-133]
- workspaces/scorecard/plugins/scorecard-backend/src/permissions/permissionUtils.ts[92-106]

### Suggested fix
1. In `getEntityMetrics.ts`, before calling `catalogMetricService.getLatestEntityMetrics(...)`, call `permissions.authorize` for `catalogEntityReadPermission` with `resourceRef: input.entityRef` and the same `credentials`.
2. If the decision is not `ALLOW`, throw `NotAllowedError` (matching the REST path behavior).
3. Add/extend unit tests to verify:
  - when catalog entity permission is denied, the action throws `NotAllowedError`
  - when allowed, it proceeds and calls `getLatestEntityMetrics`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Hard MCP service dependency 🐞 Bug ☼ Reliability
Description
The scorecard backend plugin now declares actionsRegistryServiceRef as a required init dependency,
making the plugin unable to start in backends that include scorecard but do not also provide an
Actions Registry implementation. This couples scorecard’s core runtime to MCP wiring and can break
downstream consumers that haven’t installed @backstage/plugin-mcp-actions-backend (or equivalent
service factory).
Code

workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts[R60-65]

    env.registerInit({
      deps: {
+        actionsRegistry: actionsRegistryServiceRef,
        auth: coreServices.auth,
        catalog: catalogServiceRef,
        config: coreServices.rootConfig,
Relevance

⭐⭐ Medium

No direct history on optional actionsRegistryServiceRef; only general precedent making deps/config
optional to avoid startup failures (PR #1611).

PR-#1611

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The scorecard backend plugin explicitly requires actionsRegistryServiceRef, and repo integration
tests demonstrate that this service ref must be provided via a factory (i.e., it is not inherently
available unless another feature supplies it).

workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts[60-76]
workspaces/mcp-integrations/plugins/scaffolder-mcp-extras/src/plugin.integration.test.ts[42-64]
workspaces/scorecard/packages/backend/src/index.ts[90-96]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The scorecard backend plugin requires `actionsRegistryServiceRef` unconditionally, which can prevent backend startup if the MCP actions backend plugin (or another provider for that service ref) is not installed.

### Issue Context
In this repo’s example backend, `@backstage/plugin-mcp-actions-backend` is added, so it will work here. However, the scorecard backend plugin itself is now tightly coupled to MCP actions infrastructure, which is intended to be an optional feature per the documentation.

### Fix Focus Areas
- workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts[60-76]
- workspaces/scorecard/packages/backend/src/index.ts[90-96]

### Suggested fix
Prefer one of:
1. **Optional dependency**: make the `actionsRegistry` dependency optional (if supported by the Backstage backend system API) and only call `createScorecardActions` when the registry is present.
2. **Separate module/plugin**: move MCP action registration into a dedicated backend plugin/module (similar to other `*-mcp-extras` plugins in this repo) so scorecard core can run without MCP installed.

Ensure docs and sample backend wiring remain consistent with the chosen approach.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Review · Started 9:54 PM UTC
Commit: 2714194 · View workflow run →

…scorecard-mcp

Signed-off-by: Stephanie <yangcao@redhat.com>
@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request Tests labels Jun 8, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [api-contract] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:63actionsRegistryServiceRef is added as a mandatory dependency in the scorecard plugin's registerInit. If this service ref lacks a built-in defaultFactory, the plugin will fail to initialize without @backstage/plugin-mcp-actions-backend installed — a breaking change for existing scorecard users. Every other plugin in this repo that uses actionsRegistryServiceRef does so in a separate dedicated plugin (software-catalog-mcp-extras, scaffolder-mcp-extras, techdocs-mcp-extras, x2a-mcp-extras).
    Remediation: Extract MCP actions into a separate backend module plugin (e.g., scorecard-backend-module-mcp-actions or scorecard-mcp-extras), following the established repo pattern.

Medium

  • [architectural-conflict] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:63 — MCP actions are embedded directly into the core scorecard-backend plugin, violating the established repo pattern where all other MCP integrations are separate -mcp-extras packages. This also introduces an @backstage/backend-plugin-api/alpha (unstable API) dependency directly in the core plugin.
    Remediation: Create a new scorecard-mcp-extras plugin package following the established repo convention.

  • [api-contract] workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts:29createScorecardActions accepts auth (AuthService) and catalog (CatalogService) parameters but neither createGetEntityMetricsAction nor createListMetricsAction uses them. The options object is spread-passed, so TypeScript's structural typing silently ignores extra properties.
    Remediation: Remove auth and catalog from the createScorecardActions options type, or pass only needed subsets to each action creator.

  • [scope-exceeded] workspaces/scorecard/package.json — The PR adds workspace-level resolution overrides pinning @backstage/backend-plugin-api to 1.9.0 and @backstage/plugin-catalog-node to 2.1.0. These force specific versions across the entire scorecard workspace and are not mentioned in the PR description.
    Remediation: Bump the direct dependency in scorecard-backend/package.json or document why workspace-wide resolution overrides are necessary.

Low

  • [pattern-inconsistency] workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts:52 — The router's GET /metrics endpoint does not enforce any permission check before returning listMetrics(). The new listMetrics MCP action enforces authorizeConditional + filterAuthorizedMetrics. While the inconsistency is pre-existing (the REST endpoint existed without permission checks before this PR), it is now more visible.

  • [error-handling-idiom] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts:80 — Entity-access denial logic is duplicated inline instead of reusing the existing checkEntityAccess helper in permissionUtils.ts. The existing helper requires Express Request/HttpAuthService parameters, so the duplication is a reasonable adaptation for the MCP action context.

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts — No test for unexpected error from getLatestEntityMetrics (non-NotFoundError). No test for permission service throwing (network failure).

Previous run

Review

Findings

High

  • [api-contract] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:63 — Adding actionsRegistryServiceRef as a required dependency of the main scorecardPlugin means the scorecard backend plugin will fail to initialize unless @backstage/plugin-mcp-actions-backend is also installed. Every other workspace in this repository (software-catalog-mcp-extras, scaffolder-mcp-extras, x2a-mcp-extras, techdocs-mcp-extras) creates a dedicated -mcp-extras plugin for MCP actions rather than embedding the dependency in the core plugin. This is a breaking change for existing users who upgrade the scorecard plugin without also installing the MCP actions backend.
    Remediation: Extract the MCP actions into a separate backend plugin (e.g., scorecard-backend-mcp-extras) following the established pattern used by all other workspaces in this repository. Alternatively, use Backstage's optional service dependency mechanism so the plugin gracefully skips action registration when the MCP actions backend is absent.

Medium

  • [edge-case] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts:49 — The getEntityMetrics action does not handle NotFoundError thrown by CatalogMetricService.getLatestEntityMetrics when the entity does not exist. CatalogMetricService.getLatestEntityMetrics explicitly throws NotFoundError when the entity reference cannot be found in the catalog. Whether the MCP actions framework catches this gracefully is uncertain.
    Remediation: Consider catching NotFoundError from getLatestEntityMetrics and returning a structured error output. At minimum, verify that the MCP actions framework handles NotFoundError gracefully.

Low

  • [scope-creep] workspaces/scorecard/packages/app-legacy/e2e-tests/pages/CatalogPage.ts — Adding force: true to a Playwright click is an unrelated e2e flakiness fix bundled into a PR whose stated scope is MCP actions.

  • [stale-doc] workspaces/scorecard/README.md:18 — The workspace-level documentation table does not include a row for the new MCP Actions feature documented in plugins/scorecard-backend/README.md.

Info

  • [scope-creep] workspaces/scorecard/packages/app/src/App.tsx — Adding the scorecardPlugin default export to the NFS app's feature list is a pre-existing gap fix unrelated to MCP actions (though it may be required for actions to work in the dev app).

  • [scope-creep] workspaces/scorecard/package.json — Adding yarn resolutions for @backstage/backend-plugin-api and @backstage/plugin-catalog-node at the workspace root affects all packages and should be justified with a comment or changeset note.

Previous run (2)

Review

Reason: stale-head

The review agent reviewed commit a0b92e8dc0132327b0bb2e9d755bb53d7d3b7eb6 but the PR HEAD is now faf5c4ceb050bafdd9dd7cd1a4926fdb5e82552e. This review was discarded to avoid approving unreviewed code.

Previous run (3)

Review

Findings

Medium

  • [unused-parameters] workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts:28 — The createScorecardActions options type declares auth: AuthService and catalog: CatalogService parameters, but neither is consumed by createGetEntityMetricsAction or createListMetricsAction. These parameters are silently accepted and discarded.
    Remediation: Remove auth and catalog from the options type in createScorecardActions, remove the now-unnecessary AuthService and CatalogService imports, and remove them from the call site in plugin.ts.

  • [stale-doc] workspaces/scorecard/plugins/scorecard/README.md:48 — The NFS setup code example in the frontend README (lines 48–67) does not include the scorecardPlugin default import or its addition to the features array. The PR modifies App.tsx to add import scorecardPlugin as a default import and adds it to the features array, but the README example still shows only the named imports (scorecardHomeModule, scorecardTranslationsModule, scorecardCatalogModule).
    Remediation: Update the NFS setup code example in the frontend README to include scorecardPlugin default import and features array entry.

Low

  • [scope-creep] workspaces/scorecard/packages/app-legacy/e2e-tests/pages/CatalogPage.ts:56 — The e2e test change (adding force: true to click and removing the explanatory comment) is unrelated to MCP actions.

  • [scope-creep] workspaces/scorecard/packages/app/src/App.tsx:41 — Adding scorecardPlugin default export to the frontend App.tsx features array is not directly related to the MCP actions backend feature. This is a frontend plugin registration change.

  • [pattern-inconsistency] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts:16 — The import of PermissionsService uses a value import whereas the established pattern in router.ts and permissionUtils.ts uses import type for service interfaces only used as types. Same applies to listMetrics.ts.
    Remediation: Change to import type { PermissionsService } in both files to match the convention.

Info

  • [authorization-traceability] PR references external Jira tickets (RHIDP-14049, RHIDP-14050) not verifiable via GitHub API. Authorization cannot be independently confirmed.

  • [authorization] Security review confirms both actions correctly enforce permissions. The get-entity-metrics action checks catalogEntityReadPermission AND scorecardMetricReadPermission. The list-metrics action uses authorizeConditional + filterAuthorizedMetrics, which is actually stricter than the existing REST endpoint. No security concerns.

Previous run (4)

Review

Findings

High

  • [architectural-conflict] workspaces/scorecard/plugins/scorecard-backend/src/actions/ — The PR embeds MCP actions directly inside the main scorecard-backend plugin. Every other workspace in this repository that exposes MCP actions does so via a dedicated, separately installable -mcp-extras plugin (x2a-mcp-extras, software-catalog-mcp-extras, techdocs-mcp-extras, scaffolder-mcp-extras). Embedding MCP actions in the main backend plugin couples the MCP concern to the core scorecard plugin, meaning users who install scorecard-backend will pull in the actionsRegistryServiceRef dependency even if they do not use MCP.
    Remediation: Create a separate plugin directory (e.g., scorecard-mcp-extras) following the established pattern. Move the MCP action registration into its own createBackendPlugin with a dedicated pluginId, depending on actionsRegistryServiceRef and any scorecard service refs needed. Wire it in packages/backend/src/index.ts as a separate backend.add() call.

Medium

  • [missing-test-coverage] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts:186 — The test for "should throw NotAllowedError when scorecard metric permission is denied" does not verify that getLatestEntityMetrics was NOT called after the metric-level permission denial. Unlike the catalog entity access denial test (which includes expect(mockCatalogMetricService.getLatestEntityMetrics).not.toHaveBeenCalled()), this test omits that assertion. If a future code change accidentally reorders the permission checks or removes the early return, this test would not catch the regression.

Low

  • [code-organization] workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts — The createScorecardActions function declares auth: AuthService and catalog: CatalogService in its options type, but neither parameter is consumed by createGetEntityMetricsAction or createListMetricsAction. These are excess parameters that inflate the API surface.

  • [stale-doc] workspaces/scorecard/plugins/scorecard/README.md:48 — The NFS setup example in the frontend plugin README does not import scorecardPlugin (the default export) or include it in the features array. The PR modifies App.tsx to add this import but does not update this README snippet.

  • [pattern-inconsistency] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts — Test uses jest.resetAllMocks() in beforeEach, while router.test.ts uses jest.clearAllMocks() in afterEach. resetAllMocks also removes mock implementations (not just call history), which is a different teardown strategy.

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts — Missing edge case tests for permissions.authorize returning CONDITIONAL and for unexpected errors from getLatestEntityMetrics.

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts — Missing test for the case where ALL metrics are filtered out by a conditional permission (resulting in an empty array).

Info

  • [api-shape] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts — The REST endpoint at GET /metrics/catalog/:kind/:namespace/:name accepts an optional metricIds query parameter to filter which metrics are returned. The new get-entity-metrics MCP action does not expose an equivalent input field — it always returns all authorized metrics. Consider adding an optional metricIds field for parity.
Previous run (5)

Review

Findings

High

  • [architectural-conflict] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:20 — MCP actions are embedded directly into the main scorecard-backend plugin rather than in a separate -mcp-extras plugin. Every other workspace in this repo that exposes MCP actions does so via a dedicated plugin (e.g., software-catalog-mcp-extras, x2a-mcp-extras, techdocs-mcp-extras, scaffolder-mcp-extras). Embedding actions in the core plugin couples MCP functionality to the scorecard backend, making actionsRegistryServiceRef a required dependency for all scorecard-backend consumers — even those that do not use MCP at all.
    Remediation: Extract the MCP actions into a new scorecard-mcp-extras plugin under workspaces/scorecard/plugins/, following the pattern established by x2a-mcp-extras and software-catalog-mcp-extras. The new plugin should depend on actionsRegistryServiceRef and import shared services from scorecard-node or scorecard-backend.

Medium

  • [error-handling-idiom] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts — Other MCP actions in the repo (e.g., createQueryCatalogEntitiesAction, createFetchTemplateMetadataAction) catch errors and return them in the output object rather than throwing. A comment in createQueryCatalogEntitiesAction.ts explicitly states: "The Backstage MCP server will return a 500 error if we throw a validation error (without saying why), so instead, let's return the error message in the output." The scorecard actions throw NotAllowedError and NotFoundError directly, which will result in opaque 500 errors to MCP clients. See also: listMetrics.ts has the same pattern.
    Remediation: Catch errors in the action callback and return them in the output, matching the pattern used by createQueryCatalogEntitiesAction and createFetchTemplateMetadataAction.

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts:335 — The test "should throw NotAllowedError when scorecard metric permission is denied" does not assert that mockCatalogMetricService.getLatestEntityMetrics was NOT called after the scorecard-metric-level denial. The first denial test (catalog entity access denied) correctly asserts .not.toHaveBeenCalled(), but this second denial test omits the assertion.
    Remediation: Add expect(mockCatalogMetricService.getLatestEntityMetrics).not.toHaveBeenCalled() after the rejects.toThrow(NotAllowedError) assertion.

Low

  • [api-contract] workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts:42 — The action description states "Lists all available scorecard metrics and their datasources" but the output schema does not include any datasource field. The Metric type has no datasource property.

  • [input-validation] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts:76 — The entityRef input is validated only by z.string() with no format constraint. While downstream defenses are adequate, adding parseEntityRef validation at the action boundary would harden the input path.

  • [scope-creep] workspaces/scorecard/packages/app/src/App.tsx:24 — The PR adds scorecardPlugin default import and registers it in the dev app features array. This may be a pre-existing bug fix rather than scope creep, but the PR description should clarify why this frontend change is needed for backend MCP actions.

  • [versioning] workspaces/scorecard/.changeset/sour-cameras-say.md:2 — The changeset marks this as a minor bump, but adding actionsRegistryServiceRef as a required dependency of the core plugin may break existing consumers that don't have @backstage/plugin-mcp-actions-backend installed. Extracting to a separate plugin (per the high finding) would eliminate this concern.

  • [intent-alignment] The PR title references Jira ticket RHIDP-14042 while the body references RHIDP-14049 and RHIDP-14050 — three different ticket numbers with no explanation of their relationship.

  • [missing-doc] workspaces/scorecard/plugins/scorecard/README.md:49 — The NFS setup example omits the scorecardPlugin default export from the features array, but the PR adds it to App.tsx.

  • [missing-doc] workspaces/scorecard/README.md:21 — The workspace-level documentation table describes the backend README as covering "Backend installation and RBAC" but does not mention MCP actions.

  • [naming-convention] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts, listMetrics.ts — The predominant file naming convention for MCP actions in this repo is create<ActionName>Action.ts. These files use shorter names. (Note: listScaffolderTasksAction.ts also deviates, so this is not universal.)

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.test.ts — No test covers the scenario where getLatestEntityMetrics throws a generic error (not NotFoundError).

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts — No test covers the empty metrics array case (no providers registered).

Previous run (6)

Review

Reason: stale-head

The review agent reviewed commit b4af854bb34cbeaea5237891797ad1cd2291ba23 but the PR HEAD is now c2d99bfb81513543262763f275a73a3aa361155b. This review was discarded to avoid approving unreviewed code.

Previous run (7)

Review

Findings

High

  • [api-contract] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:62 — Adding actionsRegistryServiceRef as a required dependency in the main scorecard plugin's deps means the plugin will fail to initialize if @backstage/plugin-mcp-actions-backend is not installed. Every other plugin in this repository that uses actionsRegistryServiceRef does so in a separate dedicated -mcp-extras plugin (e.g., software-catalog-mcp-extras, scaffolder-mcp-extras, techdocs-mcp-extras, x2a-mcp-extras), keeping MCP support opt-in. This change breaks existing deployments of the scorecard plugin that do not have the MCP actions backend installed.
    Remediation: Follow the established pattern — create a separate scorecard-backend-module-mcp-extras (or similar) plugin that depends on actionsRegistryServiceRef and registers the scorecard actions. The main scorecard plugin should not depend on actionsRegistryServiceRef. See also: [architectural-coherence] finding at this location.

Medium

  • [authorization] workspaces/scorecard/plugins/scorecard-backend/src/actions/getEntityMetrics.ts:82 — The get-entity-metrics MCP action checks scorecardMetricReadPermission but does NOT check catalogEntityReadPermission for the requested entity. It delegates to catalogMetricService.getLatestEntityMetrics which calls catalog.getEntityByRef using the plugin's own service credentials (this.auth.getOwnServiceCredentials()), bypassing the caller's catalog-level entity visibility. A caller with scorecard-metric-read permission but no visibility into a particular catalog entity can still retrieve that entity's metrics via this MCP action. The existing HTTP handler separately calls checkEntityAccess before fetching metrics.
    Remediation: Add a catalogEntityReadPermission authorization check using the caller's credentials before calling catalogMetricService.getLatestEntityMetrics, similar to how the existing REST route calls checkEntityAccess.

  • [scope-creep] workspaces/scorecard/packages/app/src/App.tsx:43 — The PR adds scorecardPlugin (the default export from the frontend plugin) to the App.tsx features list. This is a frontend change unrelated to the stated goal of adding MCP backend actions. The PR title, description, and Jira tickets reference MCP actions exclusively. Adding the frontend plugin default export registers scorecardApi and scorecardPage extensions that were previously not loaded.
    Remediation: Either remove this change and submit it as a separate PR with its own tracking ticket, or update the PR description and changeset to explicitly explain the rationale for this frontend change.

  • [test-inadequate] workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.test.ts / getEntityMetrics.test.ts — Both action test suites only cover the ALLOW and DENY permission paths. The CONDITIONAL permission result — where authorizeConditional returns conditions that are used to filter metrics — is the primary behavioral distinction of the permission logic and is entirely untested. For listMetrics, this means filterAuthorizedMetrics filtering is never exercised. For getEntityMetrics, the conditions are never forwarded to getLatestEntityMetrics.
    Remediation: Add test cases where authorizeConditional returns AuthorizeResult.CONDITIONAL with specific conditions, and verify that returned metrics are filtered accordingly.

  • [architectural-coherence] workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts:59 — The established MCP integration pattern in this repo (workspaces/mcp-integrations and workspaces/x2a) uses standalone backend plugins for MCP actions. This PR embeds them directly in the core scorecard-backend plugin, coupling MCP functionality tightly to the core plugin. See also: [api-contract] finding at this location.

Low

  • [import-consolidation] workspaces/scorecard/plugins/scorecard-backend/src/actions/listMetrics.ts:20-21 — Two separate import statements from the same module ../permissions/permissionUtils. The codebase pattern (see router.ts) consolidates imports from the same module into a single statement.

  • [api-shape] workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts:29-30createScorecardActions declares auth: AuthService and catalog: CatalogService parameters that are never forwarded to either action creator, creating a misleading API surface.

  • [scope-alignment] workspaces/scorecard/.changeset/sour-cameras-say.md — The changeset description ('create actions to list metrics and to get entity metrics') is vague and does not mention MCP, which will produce an unclear CHANGELOG entry. Consider: 'Add MCP actions (list-metrics, get-entity-metrics) for AI agent integration'.

  • [formatting-convention] All 5 new files in src/actions/ — Missing blank line between the license header and the first import statement. Every other source file in this package includes a blank line after the closing */ before the first import.

  • [missing-doc-coverage] workspaces/scorecard/README.md — The workspace-level README documentation table does not mention MCP actions as a discoverable topic.

Info

  • [traceability] The PR title references RHIDP-14042 but the body references RHIDP-14049 and RHIDP-14050. No linked GitHub issue exists for external verification.
Previous run (8)

Review

Reason: stale-head

The review agent reviewed commit 7c39aea59270e1dc0e69e84714ccc7a049146540 but the PR HEAD is now 37144ff2cf849458738f8b305b58fb968c030234. This review was discarded to avoid approving unreviewed code.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 9:54 PM UTC · Completed 10:01 PM UTC
Commit: 2714194 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Review · Started 10:03 PM UTC
Commit: 2714194 · View workflow run →

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 53.87%. Comparing base (1912534) to head (359249f).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3332      +/-   ##
==========================================
+ Coverage   50.23%   53.87%   +3.63%     
==========================================
  Files        2252     2398     +146     
  Lines       85416    87157    +1741     
  Branches    24173    24129      -44     
==========================================
+ Hits        42910    46954    +4044     
+ Misses      42328    39990    -2338     
- Partials      178      213      +35     
Flag Coverage Δ *Carryforward flag
adoption-insights 83.58% <ø> (-0.13%) ⬇️ Carriedforward from faf5c4c
ai-integrations 70.03% <ø> (+2.08%) ⬆️ Carriedforward from faf5c4c
app-defaults 69.60% <ø> (-0.20%) ⬇️ Carriedforward from faf5c4c
augment 46.39% <ø> (ø) Carriedforward from faf5c4c
boost ?
bulk-import 72.80% <ø> (+0.34%) ⬆️ Carriedforward from faf5c4c
cost-management 17.48% <ø> (+3.37%) ⬆️ Carriedforward from faf5c4c
dcm 59.17% <ø> (-2.63%) ⬇️ Carriedforward from faf5c4c
extensions 62.24% <ø> (+0.70%) ⬆️ Carriedforward from faf5c4c
global-floating-action-button 74.30% <ø> (+3.12%) ⬆️ Carriedforward from faf5c4c
global-header 61.63% <ø> (+1.92%) ⬆️ Carriedforward from faf5c4c
homepage 51.60% <ø> (+1.75%) ⬆️ Carriedforward from faf5c4c
install-dynamic-plugins 56.23% <ø> (ø) Carriedforward from faf5c4c
konflux 91.01% <ø> (-0.49%) ⬇️ Carriedforward from faf5c4c
lightspeed 67.85% <ø> (-0.73%) ⬇️ Carriedforward from faf5c4c
mcp-integrations 85.46% <ø> (ø) Carriedforward from faf5c4c
orchestrator 37.33% <ø> (-0.42%) ⬇️ Carriedforward from faf5c4c
quickstart 62.09% <ø> (-1.68%) ⬇️ Carriedforward from faf5c4c
sandbox 79.56% <ø> (ø) Carriedforward from faf5c4c
scorecard 83.96% <96.87%> (+0.12%) ⬆️
theme 64.54% <ø> (+3.27%) ⬆️ Carriedforward from faf5c4c
translations 8.49% <ø> (+1.94%) ⬆️ Carriedforward from faf5c4c
x2a 78.79% <ø> (+65.01%) ⬆️ Carriedforward from faf5c4c

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1912534...359249f. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts
Comment thread workspaces/scorecard/packages/app/src/App.tsx
Comment thread workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:03 PM UTC · Completed 10:15 PM UTC
Commit: 2714194 · View workflow run →

@yangcao77

yangcao77 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

The mcp-integrations workspace (with scaffolder-mcp-extras, software-catalog-mcp-extras, techdocs-mcp-extras) exists specifically as a temporary overlay for upstream Backstage plugins. some of the changes are already in, and will be included with the next backstage release we adopt, therefore those specific temporary plugins will be removed.

Scorecard is a RHDH owned plugin so actions belong directly inside scorecard-backend. Creating a separate -mcp-extras package would be unnecessary indirection.

in addition, all the actions registered under actionRegistry are not only for MCP purpose. it is an API can also consumed by backstage-cli execute action without any mcp involvement/dependency. the actions is expected to be defined under & belong to it's specific backend plugin

The fullsend bot is doing pattern-matching without understanding the distinction. dismiss comments from fullsend on the actions location.

yangcao77 added 3 commits June 9, 2026 15:39
Signed-off-by: Stephanie <yangcao@redhat.com>
Signed-off-by: Stephanie <yangcao@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 7:45 PM UTC · Completed 7:55 PM UTC
Commit: 0c4bc08 · View workflow run →

Signed-off-by: Stephanie <yangcao@redhat.com>
@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:57 PM UTC · Completed 8:12 PM UTC
Commit: 0c4bc08 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts
Comment thread workspaces/scorecard/packages/app/src/App.tsx
Comment thread workspaces/scorecard/.changeset/sour-cameras-say.md

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:11 PM UTC · Completed 4:28 PM UTC
Commit: 985b994 · View workflow run →

Signed-off-by: Stephanie <yangcao@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 10, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:15 PM UTC · Completed 7:28 PM UTC
Commit: 3ad2ec3 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 10, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:27 PM UTC · Completed 3:41 PM UTC
Commit: 172d39f · View workflow run →

Signed-off-by: Stephanie <yangcao@redhat.com>
@github-actions

Copy link
Copy Markdown
Contributor

This pull request adds a new top-level directory under workspaces/. Please follow Submitting a Pull Request for a New Workspace in CONTRIBUTING.md.

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:42 PM UTC · Completed 3:56 PM UTC
Commit: 172d39f · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts
@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Jun 11, 2026
@yangcao77

yangcao77 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@its-mitesh-kumar @Eswaraiahsapram @imykhno PTAL on this PR. Thanks!

@yangcao77 yangcao77 changed the title [RHIDP-14042]Scorecard mcp actions feat(scorecard): Scorecard mcp actions Jun 16, 2026
@yangcao77 yangcao77 requested a review from imykhno June 16, 2026 20:56
Signed-off-by: Stephanie <yangcao@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:38 PM UTC · Completed 4:52 PM UTC
Commit: 1912534 · View workflow run →

@imykhno imykhno left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR! I was tested it locally:

Get list of all scorecard metrics

✅ Works as I expect. Example:

Image

Get entity metric

✅ Works as I expect. Example:

Image

Wrong entity metric

✅ Works as I expect. Example:

Image

Base on my local testing:
/lgtm

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts
Comment thread workspaces/scorecard/plugins/scorecard-backend/src/plugin.ts
Comment thread workspaces/scorecard/plugins/scorecard-backend/src/actions/index.ts
@yangcao77 yangcao77 merged commit 0d1ea32 into redhat-developer:main Jun 19, 2026
34 of 35 checks passed
@sonarqubecloud

Copy link
Copy Markdown

Eswaraiahsapram pushed a commit to Eswaraiahsapram/rhdh-plugins that referenced this pull request Jun 22, 2026
* add scorecard actions to get entity metrics and also list metrics

Signed-off-by: Stephanie <yangcao@redhat.com>

* add config

Signed-off-by: Stephanie <yangcao@redhat.com>

* update doc and add changeset

Signed-off-by: Stephanie <yangcao@redhat.com>

* fix ai review comments

Signed-off-by: Stephanie <yangcao@redhat.com>

* fix yarn dedupe

Signed-off-by: Stephanie <yangcao@redhat.com>

* --amend

Signed-off-by: Stephanie <yangcao@redhat.com>

* try fixing ci tests

Signed-off-by: Stephanie <yangcao@redhat.com>

* fix ci

Signed-off-by: Stephanie <yangcao@redhat.com>

* cleanup tests

Signed-off-by: Stephanie <yangcao@redhat.com>

---------

Signed-off-by: Stephanie <yangcao@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request lgtm Tests workspace/scorecard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants