Conversation
Introduces APME (Ansible Policy & Modernization Engine) integration: - backstage-apme-common: Shared types, ApmeClient, service reference - catalog-backend-module-apme: Backend routes proxying to APME Gateway - backstage-apme: Frontend plugin with health card, violations table - Entity tab showing scan results with live progress polling - Filters by severity level and validator source Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add /apme route with full dashboard view - Stats cards: projects, violations, scans, avg health score - Service health panel showing all APME components - Projects table with sorting, search, and filtering - Sidebar navigation link with Security icon Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Features: - Create projects from Backstage via Add Project dialog - Project detail page with violations, activity history - Trigger scan and remediate operations - Create pull requests from remediation results - RHDH dynamic plugin configuration Tests: - Unit tests for ApmeApiClient - Unit tests for getApmeConfig Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add delete button to remove projects from APME tracking - Handle 204 No Content responses for DELETE operations - Show friendly error page when APME service is unavailable - Add app.support configuration for Backstage support button Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Prompt for GitHub token when creating PR if not configured - Pass inline scm_token through to APME gateway - Show friendly error messages for auth failures - Add loading state during PR creation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 20:28 — with 
GitHub Actions
Error
GitHub Actions
Error
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new APME integration (frontend plugin, API client, UI pages/components, shared common client/types), a catalog backend module exposing /apme/* endpoints, platform-ops AAP job/task support (routes, helpers, types), self-service platform-ops UI and API, app/backend wiring, OpenAPI entries, tests, and example catalog entity/config. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Entity Tab UI
participant FrontendAPI as ApmeApiClient
participant BackendModule as Catalog Backend Router
participant ApmeService as IApmeService
participant APMEBackend as APME Backend
UI->>FrontendAPI: triggerScan(projectId)
FrontendAPI->>BackendModule: POST /apme/projects/:projectId/operation
BackendModule->>ApmeService: triggerScan(projectId, userIdentity)
ApmeService->>APMEBackend: POST /operation (scan)
APMEBackend-->>ApmeService: { operation_id, status }
ApmeService-->>BackendModule: ScanResult
BackendModule-->>FrontendAPI: { scanId, status: "running" }
FrontendAPI-->>UI: ScanResult
Note over UI: Polling every 2s up to limit
loop Polling
UI->>FrontendAPI: getProject(projectId)
FrontendAPI->>BackendModule: GET /apme/projects/:projectId
BackendModule->>ApmeService: getProject(projectId)
ApmeService->>APMEBackend: GET /projects/:projectId
APMEBackend-->>ApmeService: Project (active_operation, last_scanned_at)
ApmeService-->>BackendModule: Project
BackendModule-->>FrontendAPI: Project
FrontendAPI-->>UI: Project
end
sequenceDiagram
participant User as User
participant ProjectPage as ProjectDetailPage
participant FrontendAPI as ApmeApiClient
participant BackendModule as Catalog Backend Router
participant ApmeService as IApmeService
participant APMEBackend as APME Backend
participant SCMDialog as SCM Token Dialog
User->>ProjectPage: Click "Create PR"
ProjectPage->>FrontendAPI: createPullRequest(projectId, activityId)
FrontendAPI->>BackendModule: POST /apme/activity/:activityId/pull-request
BackendModule->>ApmeService: createPullRequest(projectId, activityId, scmToken?)
ApmeService->>APMEBackend: POST /pull-request
alt SCM token required (422)
APMEBackend-->>ApmeService: 422 error (token required)
ApmeService-->>BackendModule: Error
BackendModule-->>FrontendAPI: Error
FrontendAPI-->>ProjectPage: Error
ProjectPage->>SCMDialog: Open dialog
User->>SCMDialog: Enter token
SCMDialog-->>ProjectPage: token
ProjectPage->>FrontendAPI: createPullRequest(..., token)
end
APMEBackend-->>ApmeService: { pr_url }
ApmeService-->>BackendModule: { pr_url }
BackendModule-->>FrontendAPI: { pr_url }
FrontendAPI-->>ProjectPage: { pr_url }
ProjectPage->>ProjectPage: Open PR and refresh
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 38 minutes and 59 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (4)
plugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsx (1)
122-155: LGTM with an optional perf nit.Behaviour looks correct:
validators,filteredViolations, andcolumnsare all recomputed on each render, which is fine for modest result sets. If you ever see large violation payloads, wrapping them (andcolumns) inuseMemokeyed onviolations/filters would avoid re-sorting and re-allocating on unrelated renders. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsx` around lines 122 - 155, The computations for validators, filteredViolations and columns are re-created on every render which can be expensive for large violations arrays; wrap these in useMemo to memoize results using appropriate dependencies: memoize validators using [violations], memoize filteredViolations using [violations, levelFilter, validatorFilter, levelOrder], and memoize columns (if present) using [violations, levelFilter, validatorFilter] or whatever specific props it depends on, so sorting and Set constructions only run when the underlying data or filters change (refer to the variables validators, filteredViolations, columns, levelFilter, validatorFilter, levelOrder, and violations).plugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsx (2)
162-182: Consider breaking down counts per severity.A single “Total Violations” chip colored as
majorwhenever > 0 loses information the backend already provides (blocker/critical/...). Ifprojectexposes per-level counts, rendering a chip per severity (reusing the style classes above) would make the health card much more actionable. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsx` around lines 162 - 182, Replace the single Total Violations chip with per-severity chips when the project exposes counts by iterating over severity keys (e.g., blocker, critical, major, minor) from the project object and render a Chip for each non-zero count inside the existing Box with class violationChips; reuse the Chip props and classes (classes.major, classes.info) mapping severity -> style, keep the existing project.scan_count and project.violation_trend chips as-is, and ensure the label uses the count and severity name (e.g., "2 Blocker") so totalViolations can be removed or optionally kept as an aggregated value.
59-79: Unused severity style classes.
classes.blocker,classes.critical, andclasses.minorare declared but never referenced in the render path (onlymajorandinfoare used). Either wire them to per-severity chips (matchingApmeViolationsTable) or drop them to reduce noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsx` around lines 59 - 79, The style object defines unused classes (classes.blocker, classes.critical, classes.minor) while the render only uses major and info; either remove these unused keys or apply them to per-severity Chips so each severity maps to its corresponding style (match the severity mapping used in ApmeViolationsTable). Locate the makeStyles/useStyles definition that returns blocker/critical/major/minor/info and then update the render path that creates the severity Chip (or the component that selects chip style) to pick classes[severity] (or remove the unused entries if you choose deletion) so there are no dead style definitions.plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx (1)
155-455:useMemoimported but not used;healthtyping narrowed too loosely.
useMemois imported (line 17) but never referenced.- Line 235 destructures
healthas possiblynull, yet downstream accesses usehealth?.status/health.components?.map, which is fine; however, theprojects.reducecalls on line 237–240 rely onprojectsbeing an array —data || { health: null, projects: [] }handles that, good.Drop unused imports or wire
useMemoaround the expensivecolumns/reduce computations if the intent was memoization to avoid recomputing on every render.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx` around lines 155 - 455, The import of useMemo is unused — either remove the import or apply memoization: wrap the expensive computations (columns array and the derived aggregates totalViolations, totalScans, avgScore) in useMemo so they only recompute when projects (and any used helpers/classes) change; specifically memoize columns (referencing columns, getScoreClass, getTrendIcon, classes, navigate) and memoize totalViolations/totalScans/avgScore with [projects] as the dependency, or simply delete the useMemo import if you prefer not to memoize.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app-config.yaml`:
- Around line 144-146: The catalog file location path under the "locations"
entry uses target: ./examples/apme-test-component.yaml which is incorrect at
runtime; update the file location target to
./packages/backend/examples/apme-test-component.yaml (the actual manifest path)
so the "type: file" location resolves from the repository root and Backstage can
load the APME test entity.
In `@plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts`:
- Around line 56-67: The constructor sets this.baseUrl from
getApmeConfig(options.rootConfig) but doesn't normalize trailing slashes,
causing double-slash URLs in executeRequest; update the ApmeClient constructor
to trim any trailing slashes from config.baseUrl (e.g., remove trailing '/'
characters) and leave a single normalized baseUrl, and update executeRequest (or
its concatenation logic) to ensure the endpoint is appended with exactly one
slash (e.g., add a leading '/' to endpoint if missing) so that
`${this.baseUrl}${endpoint}` never produces double slashes; refer to the
ApmeClient constructor and the executeRequest method when making these changes.
- Around line 246-257: The createPullRequest method currently omits projectId
from the request body; update the body construction in
ApmeClient.createPullRequest to include the projectId parameter (initialize body
with projectId) and still conditionally add scm_token if provided, then
JSON.stringify that body when passing to executeRequest so the APME endpoint
receives { projectId, scm_token } as expected.
- Around line 73-77: Add a configurable AbortController timeout around the
outbound fetch in ApmeClient (the code path that calls fetch(url, { ...options,
headers })) so hung APME calls are aborted: create an AbortController, pass its
signal into the fetch options, start a setTimeout that calls controller.abort()
after a configurable timeout (e.g., this.requestTimeoutMs or a timeout param),
clear the timer when fetch resolves/rejects, and ensure abort errors are
handled/propagated appropriately; update any constructor or call sites to
accept/configure the timeout value.
In `@plugins/backstage-apme-common/src/types/index.ts`:
- Around line 17-65: Unify the severity taxonomy: change the Severity union to
the chosen buckets ('blocker' | 'critical' | 'major' | 'minor' | 'info'), make
Violation.level typed as Severity (not string), and update
Project.violationCounts to use the same keys (blocker, critical, major, minor,
info) so all three (Severity, Violation.level, Project.violationCounts) are
consistent; adjust any inline comments referencing old names accordingly and
keep Remediation/other types unchanged.
In `@plugins/backstage-apme/src/api/ApmeApi.ts`:
- Around line 78-83: The code currently treats any non-OK response as "not
found"; update the error handling in getProjectByRepoUrl and getOperationState
(and the other similar blocks around the commented ranges) to only return null
when response.status === 404, and for all other non-OK responses throw an Error
that preserves the HTTP status and response body (e.g., await response.text())
so auth 401s, 5xxs and network errors surface instead of being swallowed as "not
found". Locate the non-OK handling in the functions getProjectByRepoUrl and
getOperationState and change the conditional logic to check for 404 first
(return null) and otherwise read the error body and throw a descriptive Error
including response.status and the error text.
In `@plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx`:
- Around line 123-183: The polling currently relies on project.last_scanned_at
and includes project in the useEffect deps, which can miss fast scans and
recreate the interval mid-scan; change the effect to poll
apmeApi.getOperationState(operationId) (use the operationId returned by
triggerScan) as the authoritative completion signal instead of last_scanned_at,
remove project from the dependency list (keep operationId, scanning, apmeApi,
retry), and on detecting completion or timeout ensure you clear the interval and
call setScanProgress(null) (and setScanning(false)) so the stale "timeout" panel
doesn't persist; also catch and log poll errors instead of silently ignoring
them to aid debugging.
- Around line 108-120: The current lookup passes raw annotation values (repoUrl)
to useAsyncRetry/apmeApi.getProjectByRepoUrl which never matches stored clone
URLs; normalize values first: read
entity.metadata.annotations['backstage.io/source-location'] and strip the "url:"
prefix and any path segments after the repo (remove
"/tree/..."/"/blob..."/subpaths) to yield "https://github.com/org/repo", and if
using entity.metadata.annotations['github.com/project-slug'] convert "org/repo"
into "https://github.com/org/repo"; assign that normalized value (e.g.,
normalizedRepoUrl) and pass it into useAsyncRetry and
apmeApi.getProjectByRepoUrl, preserving null handling if no annotation exists.
In `@plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx`:
- Around line 174-182: handleDelete currently swallows all delete errors; update
the handleDelete callback so that after calling apmeApi.deleteProject(projectId)
it only calls retry() on success, and in the catch block surface the failure to
the user (e.g., via the existing snackbar/banner mechanism or a showError/toast)
with the error message and log the error for diagnostics; also add a
confirmation dialog flow before invoking apmeApi.deleteProject to prevent
accidental deletes (referencing handleDelete, apmeApi.deleteProject, and retry
to locate where to wire the confirmation and user-facing error).
In
`@plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx`:
- Around line 186-225: handleCreatePR currently detects "token required" by
fragile substring checks on err.message and also closes over scmTokenDialog.open
which couples callback identity to dialog state; change createPullRequest to
return a structured error (e.g., { status, code, message }) or have
apmeApi.throw a typed error so handleCreatePR can check result.status === 422 or
error.code === 'SCM_TOKEN_REQUIRED' instead of includes('SCM token'), and add an
explicit flag (e.g., fromDialog: boolean) to handleCreatePR and call it from
handleScmTokenSubmit so the branch that shows dialog errors uses that flag
rather than scmTokenDialog.open; update handleScmTokenSubmit to pass
fromDialog=true when retrying and remove scmTokenDialog.open from
handleCreatePR's dependency array.
- Around line 162-184: handleScan and handleRemediate currently swallow
exceptions; capture errors from apmeApi.triggerScan and triggerRemediate, log
them, and surface them to the UI the same way ApmeEntityTab does (e.g., set the
same scanError state or call the same error handler) instead of silently
resetting the spinner. Specifically, inside both try/catch blocks for handleScan
and handleRemediate catch the error (err), call console.error(err),
setScanning/setRemediating to false and setScanProgress(null), and then set the
shared scanError (or invoke the ApmeEntityTab error handler) so a
toast/ResponseErrorPanel shows the error to the user.
- Around line 145-160: The polling callback in the useEffect (where
apmeApi.getProject(projectId!) is called) lacks error handling and a max-poll
guard; wrap the async call in a try/catch and on error
clearInterval(pollInterval), setScanning(false), setRemediating(false) and
setScanProgress(null) (and call retry() or surface an error state) so the UI
cannot get stuck, and add a simple max-attempts counter or timeout inside the
same useEffect to stop polling after N failures or elapsed time; update
references to pollInterval, apmeApi.getProject, setScanning, setRemediating,
setScanProgress, and retry accordingly.
In `@plugins/catalog-backend-module-apme/app-config.janus-idp.yaml`:
- Around line 4-8: The dynamic plugin entry
dynamicPlugins.backend.ansible.backstage-plugin-catalog-backend-module-apme is
currently parsed as null because it contains only comments; change it to an
explicit object value (e.g., an empty configuration object) so the plugin loader
sees an object instead of null and matches other entries' structure; update the
ansible.backstage-plugin-catalog-backend-module-apme entry to be an object
(preserving any explanatory comment) rather than leaving it comment-only.
In `@plugins/catalog-backend-module-apme/src/router.ts`:
- Around line 134-140: Validate that projectId is present and well-formed before
calling apmeService.createPullRequest: in the router.post handler for
'/apme/activity/:activityId/pull-request' check req.body.projectId (and
optionally its type/format) and return a 400 error response if missing/invalid
instead of calling apmeService.createPullRequest(projectId, activityId,
scm_token); also ensure you never log scm_token in this handler (remove any
logger calls that would include it) and audit ApmeClient/any tracing to avoid
capturing request bodies or bearer tokens downstream.
- Around line 17-29: The import for express-promise-router is using an incorrect
alias (Router__default) causing type mismatches; update the import to use the
default name `Router` instead of `Router__default` at the top of the file so the
`createRouter` function can return `router` without the double cast (`return
router as unknown as Router`), ensuring `router` (created as Router()) matches
the expected `Router` type from the import.
- Around line 27-140: The mutating routes in createRouter (POST /apme/projects,
DELETE /apme/projects/:projectId, POST /apme/projects/:projectId/operation, POST
/apme/projects/:projectId/remediate, POST
/apme/projects/:projectId/operation/approve, POST
/apme/activity/:activityId/pull-request and the handler functions that call
apmeService.createProject, deleteProject, triggerScan, triggerRemediate,
approveProposals, createPullRequest) must require an authenticated user and
perform authorization checks; wire the existing httpAuth middleware (or the
Backstage identity from the request) into the router so handlers extract the
user principal (e.g. from req.user or request auth helper) and pass a
userIdentity object into service calls (use the triggerScan signature that
accepts userIdentity), and enforce permission checks for sensitive actions
(approveProposals, createPullRequest, deleteProject) using the permission
framework before invoking apmeService; additionally validate and deny forwarding
raw scm_token from unauthenticated requests—only accept scm_token when caller is
authenticated and authorized or obtain credentials from the user identity
instead.
---
Nitpick comments:
In `@plugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsx`:
- Around line 162-182: Replace the single Total Violations chip with
per-severity chips when the project exposes counts by iterating over severity
keys (e.g., blocker, critical, major, minor) from the project object and render
a Chip for each non-zero count inside the existing Box with class
violationChips; reuse the Chip props and classes (classes.major, classes.info)
mapping severity -> style, keep the existing project.scan_count and
project.violation_trend chips as-is, and ensure the label uses the count and
severity name (e.g., "2 Blocker") so totalViolations can be removed or
optionally kept as an aggregated value.
- Around line 59-79: The style object defines unused classes (classes.blocker,
classes.critical, classes.minor) while the render only uses major and info;
either remove these unused keys or apply them to per-severity Chips so each
severity maps to its corresponding style (match the severity mapping used in
ApmeViolationsTable). Locate the makeStyles/useStyles definition that returns
blocker/critical/major/minor/info and then update the render path that creates
the severity Chip (or the component that selects chip style) to pick
classes[severity] (or remove the unused entries if you choose deletion) so there
are no dead style definitions.
In `@plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx`:
- Around line 155-455: The import of useMemo is unused — either remove the
import or apply memoization: wrap the expensive computations (columns array and
the derived aggregates totalViolations, totalScans, avgScore) in useMemo so they
only recompute when projects (and any used helpers/classes) change; specifically
memoize columns (referencing columns, getScoreClass, getTrendIcon, classes,
navigate) and memoize totalViolations/totalScans/avgScore with [projects] as the
dependency, or simply delete the useMemo import if you prefer not to memoize.
In
`@plugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsx`:
- Around line 122-155: The computations for validators, filteredViolations and
columns are re-created on every render which can be expensive for large
violations arrays; wrap these in useMemo to memoize results using appropriate
dependencies: memoize validators using [violations], memoize filteredViolations
using [violations, levelFilter, validatorFilter, levelOrder], and memoize
columns (if present) using [violations, levelFilter, validatorFilter] or
whatever specific props it depends on, so sorting and Set constructions only run
when the underlying data or filters change (refer to the variables validators,
filteredViolations, columns, levelFilter, validatorFilter, levelOrder, and
violations).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fd26e3e5-1900-44b6-ba1c-1ba13ba23c63
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (42)
app-config.yamlpackages/app/package.jsonpackages/app/src/App.tsxpackages/app/src/components/Root/Root.tsxpackages/app/src/components/catalog/EntityPage.tsxpackages/backend/examples/apme-test-component.yamlpackages/backend/package.jsonpackages/backend/src/index.tsplugins/backstage-apme-common/package.jsonplugins/backstage-apme-common/src/ApmeService/ApmeClient.test.tsplugins/backstage-apme-common/src/ApmeService/ApmeClient.tsplugins/backstage-apme-common/src/ApmeService/apmeServiceRef.tsplugins/backstage-apme-common/src/ApmeService/index.tsplugins/backstage-apme-common/src/index.tsplugins/backstage-apme-common/src/types/index.tsplugins/backstage-apme-common/tsconfig.jsonplugins/backstage-apme/app-config.janus-idp.yamlplugins/backstage-apme/package.jsonplugins/backstage-apme/src/api/ApmeApi.test.tsplugins/backstage-apme/src/api/ApmeApi.tsplugins/backstage-apme/src/api/index.tsplugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsxplugins/backstage-apme/src/components/ApmeEntityTab/index.tsplugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsxplugins/backstage-apme/src/components/ApmeHealthCard/index.tsplugins/backstage-apme/src/components/ApmePage/ApmePage.tsxplugins/backstage-apme/src/components/ApmePage/index.tsplugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsxplugins/backstage-apme/src/components/ApmeViolationsTable/index.tsplugins/backstage-apme/src/components/CreateProjectDialog/CreateProjectDialog.tsxplugins/backstage-apme/src/components/CreateProjectDialog/index.tsplugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsxplugins/backstage-apme/src/components/ProjectDetailPage/index.tsplugins/backstage-apme/src/index.tsplugins/backstage-apme/src/plugin.tsplugins/backstage-apme/tsconfig.jsonplugins/catalog-backend-module-apme/app-config.janus-idp.yamlplugins/catalog-backend-module-apme/package.jsonplugins/catalog-backend-module-apme/src/index.tsplugins/catalog-backend-module-apme/src/module.tsplugins/catalog-backend-module-apme/src/router.tsplugins/catalog-backend-module-apme/tsconfig.json
Comment on lines
+144
to
+146
| locations: | ||
| - type: file | ||
| target: ./examples/apme-test-component.yaml |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm where apme-test-component.yaml actually lives relative to repo root
fd -a -H 'apme-test-component.yaml'
# Double-check nothing at repo-root ./examples/
ls -la examples 2>/dev/null || echo "no ./examples dir at repo root"Repository: ansible/ansible-backstage-plugins
Length of output: 441
Correct the catalog location path to resolve at runtime.
The manifest file exists at ./packages/backend/examples/apme-test-component.yaml, not ./examples/apme-test-component.yaml. Backstage resolves relative file locations from the repository root when running yarn dev, so the current path will cause a NotFoundError and the APME test entity won't appear in the catalog.
Fix
locations:
- type: file
- target: ./examples/apme-test-component.yaml
+ target: ./packages/backend/examples/apme-test-component.yaml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| locations: | |
| - type: file | |
| target: ./examples/apme-test-component.yaml | |
| locations: | |
| - type: file | |
| target: ./packages/backend/examples/apme-test-component.yaml |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app-config.yaml` around lines 144 - 146, The catalog file location path under
the "locations" entry uses target: ./examples/apme-test-component.yaml which is
incorrect at runtime; update the file location target to
./packages/backend/examples/apme-test-component.yaml (the actual manifest path)
so the "type: file" location resolves from the repository root and Backstage can
load the APME test entity.
Comment on lines
+56
to
+67
| const config = getApmeConfig(options.rootConfig); | ||
| this.baseUrl = config.baseUrl; | ||
| // Note: checkSSL config is available but not used yet (for future TLS verification) | ||
| this.logger = options.logger.child({ service: 'ApmeClient' }); | ||
| this.logger.info(`APME client initialized with baseUrl: ${this.baseUrl}`); | ||
| } | ||
|
|
||
| private async executeRequest<T>( | ||
| endpoint: string, | ||
| options: RequestInit = {}, | ||
| ): Promise<T> { | ||
| const url = `${this.baseUrl}${endpoint}`; |
There was a problem hiding this comment.
Normalize baseUrl before concatenating endpoints.
The provided app-config.yaml example uses a trailing slash, so this builds URLs like https://...com//api/v1/health. Normalize once in the constructor to avoid gateway/proxy routing surprises.
Suggested fix
- this.baseUrl = config.baseUrl;
+ this.baseUrl = config.baseUrl.replace(/\/+$/, '');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const config = getApmeConfig(options.rootConfig); | |
| this.baseUrl = config.baseUrl; | |
| // Note: checkSSL config is available but not used yet (for future TLS verification) | |
| this.logger = options.logger.child({ service: 'ApmeClient' }); | |
| this.logger.info(`APME client initialized with baseUrl: ${this.baseUrl}`); | |
| } | |
| private async executeRequest<T>( | |
| endpoint: string, | |
| options: RequestInit = {}, | |
| ): Promise<T> { | |
| const url = `${this.baseUrl}${endpoint}`; | |
| const config = getApmeConfig(options.rootConfig); | |
| this.baseUrl = config.baseUrl.replace(/\/+$/, ''); | |
| // Note: checkSSL config is available but not used yet (for future TLS verification) | |
| this.logger = options.logger.child({ service: 'ApmeClient' }); | |
| this.logger.info(`APME client initialized with baseUrl: ${this.baseUrl}`); | |
| } | |
| private async executeRequest<T>( | |
| endpoint: string, | |
| options: RequestInit = {}, | |
| ): Promise<T> { | |
| const url = `${this.baseUrl}${endpoint}`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts` around lines 56
- 67, The constructor sets this.baseUrl from getApmeConfig(options.rootConfig)
but doesn't normalize trailing slashes, causing double-slash URLs in
executeRequest; update the ApmeClient constructor to trim any trailing slashes
from config.baseUrl (e.g., remove trailing '/' characters) and leave a single
normalized baseUrl, and update executeRequest (or its concatenation logic) to
ensure the endpoint is appended with exactly one slash (e.g., add a leading '/'
to endpoint if missing) so that `${this.baseUrl}${endpoint}` never produces
double slashes; refer to the ApmeClient constructor and the executeRequest
method when making these changes.
Comment on lines
+73
to
+77
| try { | ||
| const response = await fetch(url, { | ||
| ...options, | ||
| headers, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the APME client request path for timeout or AbortController usage.
rg -n -C3 'fetch\\(|AbortController|signal|setTimeout|timeout' plugins/backstage-apme-common/src/ApmeService/ApmeClient.tsRepository: ansible/ansible-backstage-plugins
Length of output: 185
🏁 Script executed:
cat -n plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts | head -100Repository: ansible/ansible-backstage-plugins
Length of output: 3685
🏁 Script executed:
rg -n 'fetch' plugins/backstage-apme-common/src/ApmeService/ApmeClient.tsRepository: ansible/ansible-backstage-plugins
Length of output: 121
🏁 Script executed:
rg -n 'AbortController\|signal\|setTimeout\|timeout' plugins/backstage-apme-common/src/ApmeService/ApmeClient.tsRepository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
sed -n '100,120p' plugins/backstage-apme-common/src/ApmeService/ApmeClient.tsRepository: ansible/ansible-backstage-plugins
Length of output: 767
Add a timeout to outbound APME requests.
fetch() has no request timeout, so a hung APME gateway can keep backend handlers open indefinitely. Add an AbortController timeout, ideally configurable.
Suggested fix
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), 30_000);
+
try {
const response = await fetch(url, {
...options,
headers,
+ signal: options.signal ?? controller.signal,
});
@@
} catch (error) {
@@
throw new InputError(`Failed to connect to APME: ${(error as Error).message}`);
+ } finally {
+ clearTimeout(timeout);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| const response = await fetch(url, { | |
| ...options, | |
| headers, | |
| }); | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), 30_000); | |
| try { | |
| const response = await fetch(url, { | |
| ...options, | |
| headers, | |
| signal: options.signal ?? controller.signal, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts` around lines 73
- 77, Add a configurable AbortController timeout around the outbound fetch in
ApmeClient (the code path that calls fetch(url, { ...options, headers })) so
hung APME calls are aborted: create an AbortController, pass its signal into the
fetch options, start a setTimeout that calls controller.abort() after a
configurable timeout (e.g., this.requestTimeoutMs or a timeout param), clear the
timer when fetch resolves/rejects, and ensure abort errors are
handled/propagated appropriately; update any constructor or call sites to
accept/configure the timeout value.
Comment on lines
+246
to
+257
| async createPullRequest(projectId: string, activityId: string, scmToken?: string): Promise<{ pr_url: string }> { | ||
| const body: Record<string, string> = {}; | ||
| if (scmToken) { | ||
| body.scm_token = scmToken; | ||
| } | ||
| return this.executeRequest<{ pr_url: string }>( | ||
| `/api/v1/activity/${activityId}/pull-request`, | ||
| { | ||
| method: 'POST', | ||
| body: Object.keys(body).length > 0 ? JSON.stringify(body) : undefined, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find PR creation request handling and tests to verify whether projectId/project_id is expected by the gateway.
rg -n -C4 'createPullRequest|pull-request|projectId|project_id|scm_token' \
plugins/backstage-apme-common \
plugins/catalog-backend-module-apme \
plugins/backstage-apmeRepository: ansible/ansible-backstage-plugins
Length of output: 41296
🏁 Script executed:
#!/bin/bash
# Find test files related to createPullRequest or pull-request endpoint
find plugins -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "createPullRequest\|pull-request" 2>/dev/nullRepository: ansible/ansible-backstage-plugins
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation about the pull-request endpoint
rg -n "pull-request|createPullRequest" plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts -A 15 -B 5Repository: ansible/ansible-backstage-plugins
Length of output: 1034
🏁 Script executed:
#!/bin/bash
# Search for any APME API documentation or schema files
find plugins -type f \( -name "*.md" -o -name "*.yaml" -o -name "*.yml" -o -name "*.json" \) | xargs grep -l "pull-request\|projectId.*activity" 2>/dev/null | head -20Repository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Look for test files that might test the createPullRequest flow
rg -n "createPullRequest" plugins --type ts -B 5 -A 10Repository: ansible/ansible-backstage-plugins
Length of output: 7471
🏁 Script executed:
#!/bin/bash
# Check the complete router endpoint to see what the frontend sends vs what gateway receives
rg -n "pull-request" plugins/catalog-backend-module-apme/src/router.ts -B 10 -A 10Repository: ansible/ansible-backstage-plugins
Length of output: 912
🏁 Script executed:
#!/bin/bash
# Look for any interface or type definitions for PR creation request/response
rg -n "pr_url|pull.*request|createPullRequest" plugins/backstage-apme-common/src/types/ -B 3 -A 3Repository: ansible/ansible-backstage-plugins
Length of output: 580
Include projectId in the request body when calling the APME PR creation endpoint.
The projectId parameter flows through the frontend and backend router but is dropped when constructing the gateway request. The frontend API layer correctly sends { projectId, scm_token: scmToken } to the backend; the ApmeClient.createPullRequest() method should mirror this and include projectId in the body:
const body: Record<string, string> = { projectId };
if (scmToken) {
body.scm_token = scmToken;
}This ensures the APME API receives the project context for validation or activity association checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts` around lines 246
- 257, The createPullRequest method currently omits projectId from the request
body; update the body construction in ApmeClient.createPullRequest to include
the projectId parameter (initialize body with projectId) and still conditionally
add scm_token if provided, then JSON.stringify that body when passing to
executeRequest so the APME endpoint receives { projectId, scm_token } as
expected.
Comment on lines
+17
to
+65
| export type Severity = 'blocker' | 'critical' | 'high' | 'medium' | 'low' | 'info'; | ||
|
|
||
| export type RemediationClass = 1 | 2 | 3 | 9; // 1=auto, 2=assisted, 3=manual, 9=none | ||
|
|
||
| export interface Violation { | ||
| id: number; | ||
| rule_id: string; | ||
| level: string; // 'blocker', 'critical', 'high', 'medium', 'low', 'info' | ||
| message: string; | ||
| file: string; | ||
| line: number; | ||
| path?: string; | ||
| remediation_class: RemediationClass; | ||
| remediation_resolution?: number; | ||
| scope?: number; | ||
| validator_source: string; // 'native', 'opa', 'ansible', 'gitleaks' | ||
| original_yaml?: string; | ||
| fixed_yaml?: string; | ||
| co_fixes?: string[]; | ||
| node_line_start?: number; | ||
| ai_reason?: string; | ||
| ai_suggestion?: string; | ||
| } | ||
|
|
||
| export interface Project { | ||
| id: string; | ||
| name: string; | ||
| repo_url: string; | ||
| branch: string; | ||
| created_at: string; | ||
| health_score: number; | ||
| total_violations: number; | ||
| violation_trend?: string; | ||
| scan_count: number; | ||
| last_scanned_at?: string; | ||
| scm_provider?: string; | ||
| has_scm_token: boolean; | ||
| last_scanned_commit?: string; | ||
| has_new_commits: boolean; | ||
| active_operation?: string | null; | ||
| // Computed on frontend for display | ||
| violationCounts?: { | ||
| blocker: number; | ||
| critical: number; | ||
| major: number; | ||
| minor: number; | ||
| info: number; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Severity taxonomy is inconsistent across Violation.level, Severity, and Project.violationCounts.
Severitydeclares'blocker' | 'critical' | 'high' | 'medium' | 'low' | 'info'.Violation.levelis typed as plainstring(with a comment listing the same six values), giving up compile-time safety and allowing drift.Project.violationCountsusesblocker | critical | major | minor | info, which does not matchSeverity(missinghigh/medium/low, introducingmajor/minor). Any code that aggregates violations bylevelintoviolationCountswill silently bucket nothing or be forced to map names.
Pick one taxonomy and apply it consistently, e.g.:
Proposed type alignment
export interface Violation {
id: number;
rule_id: string;
- level: string; // 'blocker', 'critical', 'high', 'medium', 'low', 'info'
+ level: Severity;
...
}
export interface Project {
...
- violationCounts?: {
- blocker: number;
- critical: number;
- major: number;
- minor: number;
- info: number;
- };
+ violationCounts?: Record<Severity, number>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export type Severity = 'blocker' | 'critical' | 'high' | 'medium' | 'low' | 'info'; | |
| export type RemediationClass = 1 | 2 | 3 | 9; // 1=auto, 2=assisted, 3=manual, 9=none | |
| export interface Violation { | |
| id: number; | |
| rule_id: string; | |
| level: string; // 'blocker', 'critical', 'high', 'medium', 'low', 'info' | |
| message: string; | |
| file: string; | |
| line: number; | |
| path?: string; | |
| remediation_class: RemediationClass; | |
| remediation_resolution?: number; | |
| scope?: number; | |
| validator_source: string; // 'native', 'opa', 'ansible', 'gitleaks' | |
| original_yaml?: string; | |
| fixed_yaml?: string; | |
| co_fixes?: string[]; | |
| node_line_start?: number; | |
| ai_reason?: string; | |
| ai_suggestion?: string; | |
| } | |
| export interface Project { | |
| id: string; | |
| name: string; | |
| repo_url: string; | |
| branch: string; | |
| created_at: string; | |
| health_score: number; | |
| total_violations: number; | |
| violation_trend?: string; | |
| scan_count: number; | |
| last_scanned_at?: string; | |
| scm_provider?: string; | |
| has_scm_token: boolean; | |
| last_scanned_commit?: string; | |
| has_new_commits: boolean; | |
| active_operation?: string | null; | |
| // Computed on frontend for display | |
| violationCounts?: { | |
| blocker: number; | |
| critical: number; | |
| major: number; | |
| minor: number; | |
| info: number; | |
| }; | |
| } | |
| export type Severity = 'blocker' | 'critical' | 'high' | 'medium' | 'low' | 'info'; | |
| export type RemediationClass = 1 | 2 | 3 | 9; // 1=auto, 2=assisted, 3=manual, 9=none | |
| export interface Violation { | |
| id: number; | |
| rule_id: string; | |
| level: Severity; | |
| message: string; | |
| file: string; | |
| line: number; | |
| path?: string; | |
| remediation_class: RemediationClass; | |
| remediation_resolution?: number; | |
| scope?: number; | |
| validator_source: string; // 'native', 'opa', 'ansible', 'gitleaks' | |
| original_yaml?: string; | |
| fixed_yaml?: string; | |
| co_fixes?: string[]; | |
| node_line_start?: number; | |
| ai_reason?: string; | |
| ai_suggestion?: string; | |
| } | |
| export interface Project { | |
| id: string; | |
| name: string; | |
| repo_url: string; | |
| branch: string; | |
| created_at: string; | |
| health_score: number; | |
| total_violations: number; | |
| violation_trend?: string; | |
| scan_count: number; | |
| last_scanned_at?: string; | |
| scm_provider?: string; | |
| has_scm_token: boolean; | |
| last_scanned_commit?: string; | |
| has_new_commits: boolean; | |
| active_operation?: string | null; | |
| // Computed on frontend for display | |
| violationCounts?: Record<Severity, number>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/backstage-apme-common/src/types/index.ts` around lines 17 - 65, Unify
the severity taxonomy: change the Severity union to the chosen buckets
('blocker' | 'critical' | 'major' | 'minor' | 'info'), make Violation.level
typed as Severity (not string), and update Project.violationCounts to use the
same keys (blocker, critical, major, minor, info) so all three (Severity,
Violation.level, Project.violationCounts) are consistent; adjust any inline
comments referencing old names accordingly and keep Remediation/other types
unchanged.
Comment on lines
+186
to
+225
| const handleCreatePR = useCallback(async (activityId: string, token?: string) => { | ||
| setCreatingPR(true); | ||
| setScmTokenDialog(prev => ({ ...prev, error: null })); | ||
| try { | ||
| const result = await apmeApi.createPullRequest(projectId!, activityId, token); | ||
| if (result.pr_url) { | ||
| window.open(result.pr_url, '_blank'); | ||
| } | ||
| setScmTokenDialog({ open: false, activityId: null, error: null }); | ||
| setScmToken(''); | ||
| retry(); | ||
| } catch (err: any) { | ||
| const errorMessage = err?.message || ''; | ||
| if (errorMessage.includes('422') && errorMessage.includes('SCM token')) { | ||
| setScmTokenDialog({ open: true, activityId, error: null }); | ||
| } else if (scmTokenDialog.open) { | ||
| // Show error in the dialog if it's open | ||
| let friendlyError = 'Failed to create pull request.'; | ||
| if (errorMessage.includes('401') || errorMessage.includes('403') || errorMessage.includes('Bad credentials') || errorMessage.includes('Unauthorized')) { | ||
| friendlyError = 'Invalid or expired token. Please check your GitHub token has the correct permissions.'; | ||
| } else if (errorMessage.includes('502')) { | ||
| // APME returns 502 when GitHub rejects the request - usually auth issues | ||
| friendlyError = 'Invalid or expired token. Please check your GitHub token has repo access.'; | ||
| } else if (errorMessage.includes('404')) { | ||
| friendlyError = 'Repository not found. Check the token has access to this repository.'; | ||
| } else if (errorMessage.includes('connect') || errorMessage.includes('ECONNREFUSED')) { | ||
| friendlyError = 'Failed to connect to GitHub. Please try again.'; | ||
| } | ||
| setScmTokenDialog(prev => ({ ...prev, error: friendlyError })); | ||
| } | ||
| } finally { | ||
| setCreatingPR(false); | ||
| } | ||
| }, [projectId, apmeApi, retry, scmTokenDialog.open]); | ||
|
|
||
| const handleScmTokenSubmit = useCallback(() => { | ||
| if (scmTokenDialog.activityId && scmToken) { | ||
| handleCreatePR(scmTokenDialog.activityId, scmToken); | ||
| } | ||
| }, [scmTokenDialog.activityId, scmToken, handleCreatePR]); |
There was a problem hiding this comment.
Fragile SCM error classification via substring matching and brittle closure dep.
- Classifying the "token required" condition via
errorMessage.includes('422') && errorMessage.includes('SCM token')couples the UI to an exact backend error string. If the backend ever reworks error messages (or i18n-izes them), the token dialog will stop opening and users will see generic errors instead. Prefer a structured error (status code + error code) bubbled up fromapmeApi.createPullRequest. handleCreatePRdepends onscmTokenDialog.open, causing the callback's identity to change whenever the dialog opens/closes, and in turn invalidatinghandleScmTokenSubmitat the same time. The branchingelse if (scmTokenDialog.open)is only a proxy for "this attempt came from the dialog retry" — pass that intent explicitly (e.g., a second argfromDialog: boolean) to decouple from state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx`
around lines 186 - 225, handleCreatePR currently detects "token required" by
fragile substring checks on err.message and also closes over scmTokenDialog.open
which couples callback identity to dialog state; change createPullRequest to
return a structured error (e.g., { status, code, message }) or have
apmeApi.throw a typed error so handleCreatePR can check result.status === 422 or
error.code === 'SCM_TOKEN_REQUIRED' instead of includes('SCM token'), and add an
explicit flag (e.g., fromDialog: boolean) to handleCreatePR and call it from
handleScmTokenSubmit so the branch that shows dialog errors uses that flag
rather than scmTokenDialog.open; update handleScmTokenSubmit to pass
fromDialog=true when retrying and remove scmTokenDialog.open from
handleCreatePR's dependency array.
Comment on lines
+4
to
+8
| dynamicPlugins: | ||
| backend: | ||
| ansible.backstage-plugin-catalog-backend-module-apme: | ||
| # No additional configuration needed - the module registers itself | ||
| # with the catalog plugin automatically |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the backend dynamic plugin entry parses as an object, not null.
python - <<'PY'
from pathlib import Path
import subprocess
import sys
try:
import yaml
except ImportError:
subprocess.check_call([sys.executable, "-m", "pip", "install", "--quiet", "PyYAML"])
import yaml
path = Path("plugins/catalog-backend-module-apme/app-config.janus-idp.yaml")
cfg = yaml.safe_load(path.read_text())
entry = cfg["dynamicPlugins"]["backend"]["ansible.backstage-plugin-catalog-backend-module-apme"]
print(f"Parsed entry type: {type(entry).__name__}")
print(f"Parsed entry value: {entry!r}")
if not isinstance(entry, dict):
raise SystemExit("Expected plugin entry to parse as a mapping/object.")
PYRepository: ansible/ansible-backstage-plugins
Length of output: 198
🏁 Script executed:
# Find other dynamic plugin configurations in the codebase to see the pattern
find . -name "*.yaml" -o -name "*.yml" | head -30 | xargs -I {} grep -l "dynamicPlugins:" {} 2>/dev/nullRepository: ansible/ansible-backstage-plugins
Length of output: 372
🏁 Script executed:
# Search for dynamic plugin loader implementation
rg -t js -t ts "dynamicPlugins" --max-count 10Repository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
# Check if there's any configuration validation or schema for dynamic plugins
rg -t js -t ts "backend.*catalog" -A 3 -B 3 --max-count 5Repository: ansible/ansible-backstage-plugins
Length of output: 4281
🏁 Script executed:
# Read the app-config files to compare patterns
for file in ./plugins/backstage-apme/app-config.janus-idp.yaml ./plugins/backstage-rhaap/app-config.janus-idp.yaml ./plugins/scaffolder-backend-module-backstage-rhaap/app-config.janus-idp.yaml ./plugins/self-service/app-config.janus-idp.yaml; do
echo "=== $file ==="
cat "$file"
echo ""
doneRepository: ansible/ansible-backstage-plugins
Length of output: 3210
🏁 Script executed:
# Search for documentation or setup regarding dynamic plugins
rg -t ts -t js "register.*plugin|dynamicPlugins" --max-count 15Repository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
# Search for dynamic plugin loader configuration or schema
rg -t json -t ts "dynamicPlugins|plugin.*config|schema" packages/backend --max-count 20Repository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
# Check if there's any validation or error handling for dynamic plugins
rg "null|undefined" -t ts packages/backend/src --max-count 10 -C 2Repository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
# Look for Janus or dynamic plugin documentation references
grep -r "dynamic.*plugin" packages/backend/package.json 2>/dev/null || echo "Not in package.json"Repository: ansible/ansible-backstage-plugins
Length of output: 96
🏁 Script executed:
# Check the actual backend configuration loading
find packages/backend -name "*.ts" -exec grep -l "dynamicPlugins" {} \;Repository: ansible/ansible-backstage-plugins
Length of output: 59
Give the dynamic plugin entry an object value.
The entry currently parses as null because comments are not YAML values. While the comment suggests no configuration is needed, this is inconsistent with all other dynamic plugin entries in the codebase, which all contain configuration objects. Dynamic plugin loaders typically expect objects, and this null value creates a maintenance risk.
Proposed fix
dynamicPlugins:
backend:
ansible.backstage-plugin-catalog-backend-module-apme:
+ disabled: false
# No additional configuration needed - the module registers itself
# with the catalog plugin automatically📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| dynamicPlugins: | |
| backend: | |
| ansible.backstage-plugin-catalog-backend-module-apme: | |
| # No additional configuration needed - the module registers itself | |
| # with the catalog plugin automatically | |
| dynamicPlugins: | |
| backend: | |
| ansible.backstage-plugin-catalog-backend-module-apme: | |
| disabled: false | |
| # No additional configuration needed - the module registers itself | |
| # with the catalog plugin automatically |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-apme/app-config.janus-idp.yaml` around lines 4
- 8, The dynamic plugin entry
dynamicPlugins.backend.ansible.backstage-plugin-catalog-backend-module-apme is
currently parsed as null because it contains only comments; change it to an
explicit object value (e.g., an empty configuration object) so the plugin loader
sees an object instead of null and matches other entries' structure; update the
ansible.backstage-plugin-catalog-backend-module-apme entry to be an object
(preserving any explanatory comment) rather than leaving it comment-only.
Comment on lines
+17
to
+29
| import { Router, json } from 'express'; | ||
| import Router__default from 'express-promise-router'; | ||
| import { LoggerService } from '@backstage/backend-plugin-api'; | ||
| import { IApmeService } from '@ansible/backstage-apme-common'; | ||
|
|
||
| export interface RouterOptions { | ||
| apmeService: IApmeService; | ||
| logger: LoggerService; | ||
| } | ||
|
|
||
| export async function createRouter(options: RouterOptions): Promise<Router> { | ||
| const { apmeService, logger } = options; | ||
| const router = Router__default(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat plugins/catalog-backend-module-apme/package.json 2>/dev/null | jq '.dependencies, .devDependencies'
rg -nP -C2 "express-promise-router" --type=tsRepository: ansible/ansible-backstage-plugins
Length of output: 1971
🏁 Script executed:
cat -n plugins/catalog-backend-module-apme/src/router.ts | head -150Repository: ansible/ansible-backstage-plugins
Length of output: 6218
🏁 Script executed:
cat tsconfig.json 2>/dev/null || find . -maxdepth 3 -name "tsconfig.json" -type f | head -5
rg -n "esModuleInterop"Repository: ansible/ansible-backstage-plugins
Length of output: 723
🏁 Script executed:
cat plugins/catalog-backend-module-apme/tsconfig.json 2>/dev/nullRepository: ansible/ansible-backstage-plugins
Length of output: 255
🏁 Script executed:
rg -n "express-promise-router" --type=json | head -20
find . -path "*/node_modules/express-promise-router*" -name "package.json" 2>/dev/null | head -1 | xargs cat 2>/dev/null | jq '.main, .exports, .types' 2>/dev/null || echo "Package not in node_modules"Repository: ansible/ansible-backstage-plugins
Length of output: 366
🏁 Script executed:
sed -n '1,50p' plugins/catalog-backend-module-rhaap/src/router.ts
tail -20 plugins/catalog-backend-module-rhaap/src/router.tsRepository: ansible/ansible-backstage-plugins
Length of output: 2456
Fix the express-promise-router import to match the project's established pattern.
Change line 18 from import Router__default from 'express-promise-router' to import Router from 'express-promise-router', consistent with how the RHAAP module imports the same package in this repository. The current __default alias combined with the double cast on line 142 (return router as unknown as Router) indicates a type mismatch stemming from an incorrect import shape. The proper default export should work without casting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-apme/src/router.ts` around lines 17 - 29, The
import for express-promise-router is using an incorrect alias (Router__default)
causing type mismatches; update the import to use the default name `Router`
instead of `Router__default` at the top of the file so the `createRouter`
function can return `router` without the double cast (`return router as unknown
as Router`), ensuring `router` (created as Router()) matches the expected
`Router` type from the import.
Comment on lines
+27
to
+140
| export async function createRouter(options: RouterOptions): Promise<Router> { | ||
| const { apmeService, logger } = options; | ||
| const router = Router__default(); | ||
|
|
||
| // Parse JSON request bodies | ||
| router.use(json()); | ||
|
|
||
| router.get('/apme/health', async (_req, res) => { | ||
| logger.debug('APME health check requested'); | ||
| const health = await apmeService.getHealth(); | ||
| res.json(health); | ||
| }); | ||
|
|
||
| router.get('/apme/projects', async (_req, res) => { | ||
| logger.debug('APME projects list requested'); | ||
| const projects = await apmeService.getProjects(); | ||
| res.json({ items: projects }); | ||
| }); | ||
|
|
||
| router.get('/apme/projects/:projectId', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.debug(`APME project ${projectId} requested`); | ||
| const project = await apmeService.getProject(projectId); | ||
| res.json(project); | ||
| }); | ||
|
|
||
| router.get('/apme/projects/:projectId/violations', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.debug(`APME violations for project ${projectId} requested`); | ||
| const violations = await apmeService.getViolations(projectId); | ||
| res.json(violations); | ||
| }); | ||
|
|
||
| router.post('/apme/projects/:projectId/operation', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.info(`APME operation triggered for project ${projectId}`); | ||
| const result = await apmeService.triggerScan(projectId); | ||
| res.status(201).json(result); | ||
| }); | ||
|
|
||
| router.get('/apme/rules', async (_req, res) => { | ||
| logger.debug('APME rules list requested'); | ||
| const rules = await apmeService.getRules(); | ||
| res.json({ items: rules }); | ||
| }); | ||
|
|
||
| router.get('/apme/lookup', async (req, res) => { | ||
| const repoUrl = req.query.repo_url as string; | ||
| if (!repoUrl) { | ||
| res.status(400).json({ error: 'repo_url query parameter is required' }); | ||
| return; | ||
| } | ||
| logger.debug(`APME project lookup by repo URL: ${repoUrl}`); | ||
| const project = await apmeService.getProjectByRepoUrl(repoUrl); | ||
| if (!project) { | ||
| res.status(404).json({ error: 'Project not found' }); | ||
| return; | ||
| } | ||
| res.json(project); | ||
| }); | ||
|
|
||
| router.post('/apme/projects', async (req, res) => { | ||
| logger.info('APME create project requested'); | ||
| const project = await apmeService.createProject(req.body); | ||
| res.status(201).json(project); | ||
| }); | ||
|
|
||
| router.delete('/apme/projects/:projectId', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.info(`APME delete project ${projectId} requested`); | ||
| await apmeService.deleteProject(projectId); | ||
| res.status(204).send(); | ||
| }); | ||
|
|
||
| router.get('/apme/projects/:projectId/activity', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.debug(`APME activity for project ${projectId} requested`); | ||
| const activity = await apmeService.getActivity(projectId); | ||
| res.json(activity); | ||
| }); | ||
|
|
||
| router.get('/apme/projects/:projectId/operation/state', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.debug(`APME operation state for project ${projectId} requested`); | ||
| const state = await apmeService.getOperationState(projectId); | ||
| if (!state) { | ||
| res.status(404).json({ error: 'No active operation' }); | ||
| return; | ||
| } | ||
| res.json(state); | ||
| }); | ||
|
|
||
| router.post('/apme/projects/:projectId/remediate', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| logger.info(`APME remediate triggered for project ${projectId}`); | ||
| const result = await apmeService.triggerRemediate(projectId); | ||
| res.status(201).json(result); | ||
| }); | ||
|
|
||
| router.post('/apme/projects/:projectId/operation/approve', async (req, res) => { | ||
| const { projectId } = req.params; | ||
| const { approved_ids } = req.body; | ||
| logger.info(`APME approve proposals for project ${projectId}`); | ||
| await apmeService.approveProposals(projectId, approved_ids || []); | ||
| res.status(200).json({ success: true }); | ||
| }); | ||
|
|
||
| router.post('/apme/activity/:activityId/pull-request', async (req, res) => { | ||
| const { activityId } = req.params; | ||
| const { projectId, scm_token } = req.body; | ||
| logger.info(`APME create PR for activity ${activityId}`); | ||
| const result = await apmeService.createPullRequest(projectId, activityId, scm_token); | ||
| res.status(201).json(result); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if auth/permission middleware is applied at the module level.
fd -t f "module.ts" plugins/catalog-backend-module-apme -x cat {}Repository: ansible/ansible-backstage-plugins
Length of output: 1479
🏁 Script executed:
# Find RouterOptions interface definition in router.ts
rg -A 5 "interface RouterOptions" plugins/catalog-backend-module-apme/src/Repository: ansible/ansible-backstage-plugins
Length of output: 544
🏁 Script executed:
# Check if httpAuth is available in the module but not used
rg "httpAuth|HttpAuthService" plugins/catalog-backend-module-apme/Repository: ansible/ansible-backstage-plugins
Length of output: 59
🏁 Script executed:
# Check the imports in router.ts to see what's available
head -n 30 plugins/catalog-backend-module-apme/src/router.tsRepository: ansible/ansible-backstage-plugins
Length of output: 1132
🏁 Script executed:
# Check if IApmeService methods already accept UserIdentity for some endpoints
rg -B 2 -A 5 "triggerScan|createProject|deleteProject|triggerRemediate|approveProposals" plugins/catalog-backend-module-apme/src/ | head -60Repository: ansible/ansible-backstage-plugins
Length of output: 3595
🏁 Script executed:
# Find the IApmeService interface definition to check method signatures
find plugins/catalog-backend-module-apme -name "*.ts" -type f | xargs rg -l "interface IApmeService|class.*ApmeService"Repository: ansible/ansible-backstage-plugins
Length of output: 61
🏁 Script executed:
# Check the backstage-apme-common package for IApmeService definition
find plugins -name "*apme*common*" -o -name "*apme*.ts" | head -20Repository: ansible/ansible-backstage-plugins
Length of output: 170
🏁 Script executed:
# Find interface definitions in the common package
rg "interface IApmeService|triggerScan|createProject" plugins/backstage-apme-common/src/ -A 3Repository: ansible/ansible-backstage-plugins
Length of output: 1376
🏁 Script executed:
# Get full method signatures for all mutating operations in IApmeService
rg -B 1 -A 5 "async (triggerScan|createProject|deleteProject|triggerRemediate|approveProposals)" plugins/backstage-apme-common/src/ApmeService/Repository: ansible/ansible-backstage-plugins
Length of output: 3335
🏁 Script executed:
# Check createPullRequest method signature
rg -A 5 "async createPullRequest" plugins/backstage-apme-common/src/ApmeService/Repository: ansible/ansible-backstage-plugins
Length of output: 707
Add authentication and authorization to all mutating endpoints.
The router exposes POST /apme/projects, DELETE /apme/projects/:projectId, POST /apme/projects/:projectId/operation, POST /apme/projects/:projectId/remediate, POST /apme/projects/:projectId/operation/approve, and POST /apme/activity/:activityId/pull-request with no authentication or permission checks. This allows any caller with access to the catalog backend to register/delete projects, trigger scans and remediation, approve proposals, and create PRs with user-supplied credentials. The scm_token parameter is particularly dangerous—it's accepted directly from the request body and forwarded to an external service without identity verification.
In Backstage, these endpoints should require a user principal via httpAuth (at minimum) and ideally use the permission framework for mutating actions. Additionally, triggerScan already supports a userIdentity parameter for audit logging; extract and propagate user credentials there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-apme/src/router.ts` around lines 27 - 140, The
mutating routes in createRouter (POST /apme/projects, DELETE
/apme/projects/:projectId, POST /apme/projects/:projectId/operation, POST
/apme/projects/:projectId/remediate, POST
/apme/projects/:projectId/operation/approve, POST
/apme/activity/:activityId/pull-request and the handler functions that call
apmeService.createProject, deleteProject, triggerScan, triggerRemediate,
approveProposals, createPullRequest) must require an authenticated user and
perform authorization checks; wire the existing httpAuth middleware (or the
Backstage identity from the request) into the router so handlers extract the
user principal (e.g. from req.user or request auth helper) and pass a
userIdentity object into service calls (use the triggerScan signature that
accepts userIdentity), and enforce permission checks for sensitive actions
(approveProposals, createPullRequest, deleteProject) using the permission
framework before invoking apmeService; additionally validate and deny forwarding
raw scm_token from unauthenticated requests—only accept scm_token when caller is
authenticated and authorized or obtain credentials from the user identity
instead.
Comment on lines
+134
to
+140
| router.post('/apme/activity/:activityId/pull-request', async (req, res) => { | ||
| const { activityId } = req.params; | ||
| const { projectId, scm_token } = req.body; | ||
| logger.info(`APME create PR for activity ${activityId}`); | ||
| const result = await apmeService.createPullRequest(projectId, activityId, scm_token); | ||
| res.status(201).json(result); | ||
| }); |
There was a problem hiding this comment.
projectId sourced from request body is unvalidated and unused by the route path.
The route is /apme/activity/:activityId/pull-request, yet projectId is pulled from req.body and passed to apmeService.createPullRequest(projectId, activityId, scm_token) without any validation. If a client omits it (or sends null), createPullRequest will be called with undefined, producing a confusing downstream error rather than a clean 400. Validate and reject early:
Suggested fix
router.post('/apme/activity/:activityId/pull-request', async (req, res) => {
const { activityId } = req.params;
const { projectId, scm_token } = req.body;
+ if (!projectId || typeof projectId !== 'string') {
+ res.status(400).json({ error: 'projectId is required in request body' });
+ return;
+ }
logger.info(`APME create PR for activity ${activityId}`);
const result = await apmeService.createPullRequest(projectId, activityId, scm_token);
res.status(201).json(result);
});Additionally, avoid logging anything derived from scm_token anywhere in the stack; since it's a bearer credential, double-check that the downstream ApmeClient and any request tracer don't capture the request body.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| router.post('/apme/activity/:activityId/pull-request', async (req, res) => { | |
| const { activityId } = req.params; | |
| const { projectId, scm_token } = req.body; | |
| logger.info(`APME create PR for activity ${activityId}`); | |
| const result = await apmeService.createPullRequest(projectId, activityId, scm_token); | |
| res.status(201).json(result); | |
| }); | |
| router.post('/apme/activity/:activityId/pull-request', async (req, res) => { | |
| const { activityId } = req.params; | |
| const { projectId, scm_token } = req.body; | |
| if (!projectId || typeof projectId !== 'string') { | |
| res.status(400).json({ error: 'projectId is required in request body' }); | |
| return; | |
| } | |
| logger.info(`APME create PR for activity ${activityId}`); | |
| const result = await apmeService.createPullRequest(projectId, activityId, scm_token); | |
| res.status(201).json(result); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-apme/src/router.ts` around lines 134 - 140,
Validate that projectId is present and well-formed before calling
apmeService.createPullRequest: in the router.post handler for
'/apme/activity/:activityId/pull-request' check req.body.projectId (and
optionally its type/format) and return a 400 error response if missing/invalid
instead of calling apmeService.createPullRequest(projectId, activityId,
scm_token); also ensure you never log scm_token in this handler (remove any
logger calls that would include it) and audit ApmeClient/any tracing to avoid
capturing request bodies or bearer tokens downstream.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 20:36 — with
GitHub Actions
Error
GitHub Actions
Error
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 20:37 — with
GitHub Actions
Error
GitHub Actions
Error
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 20:43 — with
GitHub Actions
Error
GitHub Actions
Error
The Spectral OpenAPI linter requires all operations to have a non-empty description field. Added descriptions to all 14 APME endpoints. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 20:50 — with
GitHub Actions
Error
GitHub Actions
Error
The drift checker was doing literal string comparison between Express
:param syntax and OpenAPI {param} syntax. Added normalization to
convert Express-style parameters to OpenAPI-style before comparison.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 20:59 — with
GitHub Actions
Error
GitHub Actions
Error
- Remove unused imports (useMemo, ResponseErrorPanel, CheckCircleIcon)
- Prefix unused params with underscore (projectId, operationId)
- Remove invalid idSynonym option from Table
- Replace gap prop with style={{ gap: 16 }} for MUI v4 compat
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 21:08 — with
GitHub Actions
Error
GitHub Actions
Error
The APME plugins were missing .eslintrc.js files which caused ESLint to fail parsing TypeScript syntax. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 21, 2026 21:12 — with
GitHub Actions
Error
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
plugins/catalog-backend-module-apme/src/router.ts (2)
137-145:⚠️ Potential issue | 🟠 MajorValidate
projectIdbefore creating a PR.
projectIdis required bycreatePullRequestbut is accepted from the body unchecked, so missing or non-string values produce downstream failures instead of a clean 400.Proposed validation
router.post('/apme/activity/:activityId/pull-request', async (req, res) => { const { activityId } = req.params; const { projectId, scm_token } = req.body; + if (!projectId || typeof projectId !== 'string') { + res.status(400).json({ error: 'projectId is required in request body' }); + return; + } logger.info(`APME create PR for activity ${activityId}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/catalog-backend-module-apme/src/router.ts` around lines 137 - 145, The route handler registered with router.post('/apme/activity/:activityId/pull-request') accepts projectId from req.body but does not validate it before calling apmeService.createPullRequest; add an explicit check in that handler to ensure projectId exists and is a non-empty string (e.g., typeof projectId === 'string' && projectId.trim() !== ''), and if validation fails respond with res.status(400).json({ error: 'projectId is required' }) (or similar) without calling apmeService.createPullRequest; keep the rest of the flow unchanged.
60-64:⚠️ Potential issue | 🔴 CriticalRequire auth and authorization before mutating APME state.
These routes can create/delete projects, trigger scans/remediation, approve proposals, and create PRs without any identity or permission check. This is especially risky for
scm_token, which is forwarded as a bearer credential.Verify the router/module wiring has an auth service available before patching:
#!/bin/bash # Inspect APME backend module/router auth wiring and comparable auth usage. rg -n -C4 'httpAuth|HttpAuthService|permissions|Permission|createRouter\\(|RouterOptions' --type tsAlso applies to: 88-98, 119-147
plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx (1)
173-181:⚠️ Potential issue | 🟡 MinorSurface delete failures to the user.
Swallowing all
deleteProjecterrors hides permission, network, and server failures. Show an error snackbar/banner and consider confirming before removal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx` around lines 173 - 181, handleDelete currently swallows all errors from apmeApi.deleteProject; update the handler (the handleDelete function) to 1) prompt for confirmation before calling apmeApi.deleteProject (e.g., open a confirm dialog/modal) and 2) in the catch block surface failures to the user by displaying an error snackbar/banner (use your app's notification hook such as enqueueSnackbar/useSnackbar or the existing alert component) with the error message and optional details, while still calling retry() on success; ensure you stopPropagation remains and include the projectId in logs/messages for context.plugins/backstage-apme/src/api/ApmeApi.ts (1)
76-100:⚠️ Potential issue | 🟠 MajorPreserve HTTP status and only suppress expected 404s.
getProjectByRepoUrl()andgetOperationState()currently convert every failure intonull, hiding auth errors and APME outages. Also keep the 409 scan message local to scan operations instead of applying it to every endpoint.Proposed fix
+class ApmeApiError extends Error { + constructor( + readonly status: number, + message: string, + ) { + super(message); + this.name = 'ApmeApiError'; + } +} + export class ApmeApiClient implements ApmeApi { @@ if (!response.ok) { - if (response.status === 409) { - throw new Error('A scan is already in progress for this project'); - } const error = await response.text(); - throw new Error(`APME API error: ${response.status} - ${error}`); + throw new ApmeApiError( + response.status, + `APME API error: ${response.status} - ${error}`, + ); @@ - } catch { - return null; + } catch (error) { + if (error instanceof ApmeApiError && error.status === 404) { + return null; + } + throw error; @@ - const response = await this.fetch<{ operation_id: string }>( - `/projects/${projectId}/operation`, - { - method: 'POST', - body: JSON.stringify({ action: 'check', options: {} }), - }, - ); - return { - scanId: response.operation_id, - projectId, - status: 'running', - }; + try { + const response = await this.fetch<{ operation_id: string }>( + `/projects/${projectId}/operation`, + { + method: 'POST', + body: JSON.stringify({ action: 'check', options: {} }), + }, + ); + return { + scanId: response.operation_id, + projectId, + status: 'running', + }; + } catch (error) { + if (error instanceof ApmeApiError && error.status === 409) { + throw new Error('A scan is already in progress for this project'); + } + throw error; + } @@ - } catch { - return null; + } catch (error) { + if (error instanceof ApmeApiError && error.status === 404) { + return null; + } + throw error; }Also applies to: 115-122, 137-150, 169-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/api/ApmeApi.ts` around lines 76 - 100, The generic fetch<T> in ApmeApi.ts should not swallow all errors or convert them into generic messages: remove the global 409-to-error mapping and instead let fetch return response status and body, only treating 204 as undefined; change callers like getProjectByRepoUrl and getOperationState to treat a 404 as null but rethrow or propagate other non-2xx statuses (including auth errors and 5xx) so outages and auth failures surface; move the special-case 409 handling into the scan-specific method(s) (e.g., startScan or similar) where a 409 maps to "A scan is already in progress for this project." Ensure fetch still returns response.json() for success and that the callers inspect response.status to implement the selective 404 suppression.plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx (1)
146-149:⚠️ Potential issue | 🟡 MinorCompletion check may miss when
active_operationisundefined.Per
plugins/backstage-apme-common/src/types/index.ts,active_operation?: string | null— the field can be absent from the response. The strict=== nullcomparison will then evaluate false, and the scan will be treated as still running until themaxPollstimeout fires. Use a falsy check (!updatedProject.active_operation) to cover bothnullandundefined. This is related to, but distinct from, the broader polling concerns raised in the previous review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx` around lines 146 - 149, The scanFinished check uses strict === null so it misses when active_operation is undefined; update the logic around the scanFinished variable (where updatedProject.active_operation and initialScanTime are compared) to use a falsy check (e.g., !updatedProject.active_operation) instead of === null so both null and undefined are treated as "no active operation" (ensure you update the scanFinished assignment that references updatedProject.active_operation and initialScanTime and keep the existing last_scanned_at comparison and maxPolls behavior).
🧹 Nitpick comments (2)
plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx (1)
105-105: Dead state:_operationIdis set but never consumed.
setOperationId(result.scanId)stores the scan identifier, but the value is never read (the variable is prefixed with_). If the intent is to drive polling from the operation id (as suggested in the prior review around theuseEffect), wire the read here; otherwise drop this state entirely along with thesetOperationId(null)in the success path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx` at line 105, The _operationId state is dead (declared as const [_operationId, setOperationId]) and never read; either use it where the polling useEffect expects the operation id or remove it entirely. If polling should be driven by the scan id, rename _operationId to operationId and reference operationId inside the useEffect (and other polling logic) so setOperationId(result.scanId) drives retries; otherwise delete the state declaration and remove the setOperationId(null) call in the success path to avoid storing an unused value.plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx (1)
138-145:Promise.allmakes the page fail if any single endpoint is down.If either
getViolationsorgetActivityerrors (backend hiccup, empty permission, partial outage), the whole page renders theResponseErrorPaneleven though the project itself loaded fine. ConsiderPromise.allSettledand degrade gracefully (show the project stats with empty violations/activity sections and a per-section warning). Low priority, but noticeable from a UX perspective.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx` around lines 138 - 145, The current useAsyncRetry call uses Promise.all to fetch apmeApi.getProject, apmeApi.getViolations, and apmeApi.getActivity which fails the whole page if any one request errors; change to Promise.allSettled inside the async callback used by useAsyncRetry and handle each settled result separately: always treat the fulfilled getProject result as required (throw only if it fails), but for getViolations and getActivity map rejected results to sensible defaults (e.g., empty arrays) and attach per-section error flags/messages so the UI can render the Project data while showing degraded warnings for violations/activity; update the returned object shape from the async callback to include project, violations, activity, and per-section error indicators so the component can render sections independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/openapi.yaml`:
- Around line 1380-1384: The requestBody schema for the PR creation endpoint is
too loose — change the application/json schema so it explicitly defines
properties "projectId" (required, string) and "scm_token" (optional, string) and
mark "projectId" in the required array; update the existing schema block under
requestBody -> content -> application/json to use type: object with properties {
projectId: { type: string }, scm_token: { type: string } } and required:
["projectId"] so generated clients always include projectId when calling this
endpoint.
- Around line 1108-1394: The APME endpoints lack security declarations so
mutation and sensitive ops appear unauthenticated; update each relevant
operation (e.g. createApmeProject, deleteApmeProject, triggerApmeOperation,
approveApmeProposals, triggerApmeRemediation, createApmePullRequest and any
other write/protected operation) to include a security requirement referencing
your JWT/bearer scheme (e.g. bearerAuth or jwt) and add standard 401 and 403
responses to their responses block; also ensure a corresponding
components.securitySchemes entry (type: http, scheme: bearer, bearerFormat: JWT
or your scheme) exists so the security reference resolves.
In
`@plugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsx`:
- Around line 118-128: The repoUrl read in ApmeViolationsTable.tsx can contain a
"url:" prefix from the metadata annotation; normalize it before calling the APME
API by stripping any leading "url:" (case-sensitive) and whitespace so
apmeApi.getProjectByRepoUrl receives a plain URL like
"https://github.com/org/repo"; update the repoUrl variable (used inside the
useAsync block and before calling apmeApi.getProjectByRepoUrl and
apmeApi.getViolations) to a sanitizedRepoUrl and use that for the API calls and
the dependency array.
In
`@plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx`:
- Around line 199-201: The code opens external URLs without rel protections:
update the window.open call that uses result.pr_url in ProjectDetailPage (the
branch that calls window.open(result.pr_url, '_blank')) to open links with
noopener and noreferrer protections (i.e., use feature/rel equivalent) to
prevent reverse-tabnabbing, and update the <Link ... target="_blank"> instance
in the component (the table cell link where Link has target="_blank") to include
rel="noopener noreferrer"; ensure you handle potential null/undefined
result.pr_url before opening.
---
Duplicate comments:
In `@plugins/backstage-apme/src/api/ApmeApi.ts`:
- Around line 76-100: The generic fetch<T> in ApmeApi.ts should not swallow all
errors or convert them into generic messages: remove the global 409-to-error
mapping and instead let fetch return response status and body, only treating 204
as undefined; change callers like getProjectByRepoUrl and getOperationState to
treat a 404 as null but rethrow or propagate other non-2xx statuses (including
auth errors and 5xx) so outages and auth failures surface; move the special-case
409 handling into the scan-specific method(s) (e.g., startScan or similar) where
a 409 maps to "A scan is already in progress for this project." Ensure fetch
still returns response.json() for success and that the callers inspect
response.status to implement the selective 404 suppression.
In `@plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx`:
- Around line 146-149: The scanFinished check uses strict === null so it misses
when active_operation is undefined; update the logic around the scanFinished
variable (where updatedProject.active_operation and initialScanTime are
compared) to use a falsy check (e.g., !updatedProject.active_operation) instead
of === null so both null and undefined are treated as "no active operation"
(ensure you update the scanFinished assignment that references
updatedProject.active_operation and initialScanTime and keep the existing
last_scanned_at comparison and maxPolls behavior).
In `@plugins/backstage-apme/src/components/ApmePage/ApmePage.tsx`:
- Around line 173-181: handleDelete currently swallows all errors from
apmeApi.deleteProject; update the handler (the handleDelete function) to 1)
prompt for confirmation before calling apmeApi.deleteProject (e.g., open a
confirm dialog/modal) and 2) in the catch block surface failures to the user by
displaying an error snackbar/banner (use your app's notification hook such as
enqueueSnackbar/useSnackbar or the existing alert component) with the error
message and optional details, while still calling retry() on success; ensure you
stopPropagation remains and include the projectId in logs/messages for context.
In `@plugins/catalog-backend-module-apme/src/router.ts`:
- Around line 137-145: The route handler registered with
router.post('/apme/activity/:activityId/pull-request') accepts projectId from
req.body but does not validate it before calling apmeService.createPullRequest;
add an explicit check in that handler to ensure projectId exists and is a
non-empty string (e.g., typeof projectId === 'string' && projectId.trim() !==
''), and if validation fails respond with res.status(400).json({ error:
'projectId is required' }) (or similar) without calling
apmeService.createPullRequest; keep the rest of the flow unchanged.
---
Nitpick comments:
In `@plugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsx`:
- Line 105: The _operationId state is dead (declared as const [_operationId,
setOperationId]) and never read; either use it where the polling useEffect
expects the operation id or remove it entirely. If polling should be driven by
the scan id, rename _operationId to operationId and reference operationId inside
the useEffect (and other polling logic) so setOperationId(result.scanId) drives
retries; otherwise delete the state declaration and remove the
setOperationId(null) call in the success path to avoid storing an unused value.
In
`@plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx`:
- Around line 138-145: The current useAsyncRetry call uses Promise.all to fetch
apmeApi.getProject, apmeApi.getViolations, and apmeApi.getActivity which fails
the whole page if any one request errors; change to Promise.allSettled inside
the async callback used by useAsyncRetry and handle each settled result
separately: always treat the fulfilled getProject result as required (throw only
if it fails), but for getViolations and getActivity map rejected results to
sensible defaults (e.g., empty arrays) and attach per-section error
flags/messages so the UI can render the Project data while showing degraded
warnings for violations/activity; update the returned object shape from the
async callback to include project, violations, activity, and per-section error
indicators so the component can render sections independently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e11cb228-2bfb-41c9-b2a6-16cf5705ed59
📒 Files selected for processing (17)
api/openapi.yamlapi/scripts/check-drift.mjspackages/app/src/components/Root/Root.tsxplugins/backstage-apme-common/.eslintrc.jsplugins/backstage-apme-common/src/ApmeService/ApmeClient.tsplugins/backstage-apme-common/src/types/index.tsplugins/backstage-apme/.eslintrc.jsplugins/backstage-apme/src/api/ApmeApi.test.tsplugins/backstage-apme/src/api/ApmeApi.tsplugins/backstage-apme/src/components/ApmeEntityTab/ApmeEntityTab.tsxplugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsxplugins/backstage-apme/src/components/ApmePage/ApmePage.tsxplugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsxplugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsxplugins/backstage-apme/src/plugin.tsplugins/catalog-backend-module-apme/.eslintrc.jsplugins/catalog-backend-module-apme/src/router.ts
✅ Files skipped from review due to trivial changes (5)
- packages/app/src/components/Root/Root.tsx
- plugins/backstage-apme-common/.eslintrc.js
- plugins/backstage-apme/.eslintrc.js
- plugins/catalog-backend-module-apme/.eslintrc.js
- plugins/backstage-apme-common/src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/backstage-apme/src/components/ApmeHealthCard/ApmeHealthCard.tsx
- plugins/backstage-apme-common/src/ApmeService/ApmeClient.ts
Comment on lines
+1108
to
+1394
| /apme/health: | ||
| get: | ||
| operationId: getApmeHealth | ||
| summary: Get APME service health status | ||
| description: Returns the health status of the APME backend service including component availability. | ||
| tags: | ||
| - APME | ||
| responses: | ||
| '200': | ||
| description: Health status | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
|
|
||
| /apme/projects: | ||
| get: | ||
| operationId: getApmeProjects | ||
| summary: List all APME projects | ||
| description: Returns all Ansible content projects registered with APME for policy analysis and modernization. | ||
| tags: | ||
| - APME | ||
| responses: | ||
| '200': | ||
| description: List of projects | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| post: | ||
| operationId: createApmeProject | ||
| summary: Create a new APME project | ||
| description: Registers a new Ansible content repository with APME for policy analysis and modernization tracking. | ||
| tags: | ||
| - APME | ||
| requestBody: | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| responses: | ||
| '201': | ||
| description: Project created | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
|
|
||
| /apme/projects/{projectId}: | ||
| get: | ||
| operationId: getApmeProject | ||
| summary: Get a specific APME project | ||
| description: Returns details of a specific APME project including health score, violation counts, and scan history. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '200': | ||
| description: Project details | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| delete: | ||
| operationId: deleteApmeProject | ||
| summary: Delete an APME project | ||
| description: Removes a project from APME tracking. This does not delete the underlying repository. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '204': | ||
| description: Project deleted | ||
|
|
||
| /apme/projects/{projectId}/violations: | ||
| get: | ||
| operationId: getApmeViolations | ||
| summary: Get violations for a project | ||
| description: Returns all policy violations detected in the project from the most recent scan, including severity levels and file locations. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '200': | ||
| description: List of violations | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: array | ||
| items: | ||
| type: object | ||
|
|
||
| /apme/projects/{projectId}/operation: | ||
| post: | ||
| operationId: triggerApmeOperation | ||
| summary: Trigger a scan operation | ||
| description: Initiates a new policy scan on the project. Returns an operation ID for tracking progress. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '201': | ||
| description: Operation started | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
|
|
||
| /apme/projects/{projectId}/operation/state: | ||
| get: | ||
| operationId: getApmeOperationState | ||
| summary: Get current operation state | ||
| description: Returns the current state of an in-progress operation including phase and progress details. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '200': | ||
| description: Operation state | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| '404': | ||
| description: No active operation | ||
|
|
||
| /apme/projects/{projectId}/operation/approve: | ||
| post: | ||
| operationId: approveApmeProposals | ||
| summary: Approve remediation proposals | ||
| description: Approves selected remediation proposals from the current operation to be applied to the codebase. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| requestBody: | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| responses: | ||
| '200': | ||
| description: Proposals approved | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
|
|
||
| /apme/projects/{projectId}/remediate: | ||
| post: | ||
| operationId: triggerApmeRemediation | ||
| summary: Trigger remediation | ||
| description: Initiates automated remediation of detected violations using AI-assisted code fixes. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '201': | ||
| description: Remediation started | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
|
|
||
| /apme/projects/{projectId}/activity: | ||
| get: | ||
| operationId: getApmeActivity | ||
| summary: Get project scan history | ||
| description: Returns the history of scans and remediation operations performed on the project. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: projectId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '200': | ||
| description: Activity history | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: array | ||
| items: | ||
| type: object | ||
|
|
||
| /apme/rules: | ||
| get: | ||
| operationId: getApmeRules | ||
| summary: List all APME rules | ||
| description: Returns the catalog of all policy rules available in APME including rule IDs, descriptions, and severity levels. | ||
| tags: | ||
| - APME | ||
| responses: | ||
| '200': | ||
| description: List of rules | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
|
|
||
| /apme/lookup: | ||
| get: | ||
| operationId: lookupApmeProject | ||
| summary: Lookup project by repo URL | ||
| description: Finds an APME project by its repository URL. Used to check if a repository is already registered. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: repo_url | ||
| in: query | ||
| required: true | ||
| schema: | ||
| type: string | ||
| responses: | ||
| '200': | ||
| description: Project found | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| '404': | ||
| description: Project not found | ||
|
|
||
| /apme/activity/{activityId}/pull-request: | ||
| post: | ||
| operationId: createApmePullRequest | ||
| summary: Create a PR from remediation activity | ||
| description: Creates a GitHub pull request containing the remediation changes from a completed remediation activity. | ||
| tags: | ||
| - APME | ||
| parameters: | ||
| - name: activityId | ||
| in: path | ||
| required: true | ||
| schema: | ||
| type: string | ||
| requestBody: | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| responses: | ||
| '201': | ||
| description: PR created | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| properties: | ||
| pr_url: | ||
| type: string |
There was a problem hiding this comment.
Declare authentication requirements for APME endpoints.
The new APME operations omit security, so the published contract shows project mutation, remediation, approval, and PR creation as unauthenticated. Add the intended JWT/permission requirements and 401/403 responses alongside the backend auth fix.
Example OpenAPI shape
/apme/projects:
get:
+ security:
+ - JWT: []
operationId: getApmeProjects
@@
post:
+ security:
+ - JWT: []
operationId: createApmeProject
@@
responses:
'201':
description: Project created
+ '401':
+ description: Unauthorized
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/ErrorResponse'
+ '403':
+ description: Forbidden
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/ErrorResponse'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/openapi.yaml` around lines 1108 - 1394, The APME endpoints lack security
declarations so mutation and sensitive ops appear unauthenticated; update each
relevant operation (e.g. createApmeProject, deleteApmeProject,
triggerApmeOperation, approveApmeProposals, triggerApmeRemediation,
createApmePullRequest and any other write/protected operation) to include a
security requirement referencing your JWT/bearer scheme (e.g. bearerAuth or jwt)
and add standard 401 and 403 responses to their responses block; also ensure a
corresponding components.securitySchemes entry (type: http, scheme: bearer,
bearerFormat: JWT or your scheme) exists so the security reference resolves.
Comment on lines
+1380
to
+1384
| requestBody: | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object |
There was a problem hiding this comment.
Define the PR creation request body.
The backend consumes projectId and optional scm_token, but the spec allows an arbitrary object and marks nothing required. Generated clients can omit projectId and fail at runtime.
Proposed schema
requestBody:
+ required: true
content:
application/json:
schema:
type: object
+ required:
+ - projectId
+ properties:
+ projectId:
+ type: string
+ scm_token:
+ type: string
+ writeOnly: true
+ additionalProperties: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| requestBody: | |
| content: | |
| application/json: | |
| schema: | |
| type: object | |
| requestBody: | |
| required: true | |
| content: | |
| application/json: | |
| schema: | |
| type: object | |
| required: | |
| - projectId | |
| properties: | |
| projectId: | |
| type: string | |
| scm_token: | |
| type: string | |
| writeOnly: true | |
| additionalProperties: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/openapi.yaml` around lines 1380 - 1384, The requestBody schema for the PR
creation endpoint is too loose — change the application/json schema so it
explicitly defines properties "projectId" (required, string) and "scm_token"
(optional, string) and mark "projectId" in the required array; update the
existing schema block under requestBody -> content -> application/json to use
type: object with properties { projectId: { type: string }, scm_token: { type:
string } } and required: ["projectId"] so generated clients always include
projectId when calling this endpoint.
Comment on lines
+118
to
+128
| const repoUrl = | ||
| entity.metadata.annotations?.['backstage.io/source-location'] || | ||
| entity.metadata.annotations?.['github.com/project-slug']; | ||
|
|
||
| const { value, loading, error } = useAsync(async () => { | ||
| if (!repoUrl) return { project: null, violations: [] }; | ||
| const project = await apmeApi.getProjectByRepoUrl(repoUrl); | ||
| if (!project) return { project: null, violations: [] }; | ||
| const violations = await apmeApi.getViolations(project.id); | ||
| return { project, violations }; | ||
| }, [repoUrl]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect source-location, github project slug, and APME repo_url usage.
rg -n -C2 'backstage\.io/source-location|github\.com/project-slug|repo_url' \
--type-add 'yaml:*.{yaml,yml}' --type yaml --type ts --type tsxRepository: ansible/ansible-backstage-plugins
Length of output: 104
🏁 Script executed:
#!/bin/bash
# Search for annotation patterns - fix the type issue
# Remove tsx from type specification and use general search
rg -n -C2 'backstage\.io/source-location|github\.com/project-slug|repo_url' \
--type yaml --type tsRepository: ansible/ansible-backstage-plugins
Length of output: 50389
🏁 Script executed:
#!/bin/bash
# Find and inspect the ApmeApi implementation
fd -t f 'ApmeApi' --type f -e ts -e tsx | head -20Repository: ansible/ansible-backstage-plugins
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Look at the ApmeViolationsTable file directly to understand context
cat -n plugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsx | head -150Repository: ansible/ansible-backstage-plugins
Length of output: 5023
🏁 Script executed:
#!/bin/bash
# Read the ApmeApi implementation
cat -n plugins/backstage-apme/src/api/ApmeApi.tsRepository: ansible/ansible-backstage-plugins
Length of output: 7560
🏁 Script executed:
#!/bin/bash
# Read the ApmeApi test file to see expected URL formats
cat -n plugins/backstage-apme/src/api/ApmeApi.test.tsRepository: ansible/ansible-backstage-plugins
Length of output: 7952
Strip url: prefix from backstage.io/source-location before passing to APME API.
The API's getProjectByRepoUrl() expects a normalized URL like https://github.com/org/repo, but backstage.io/source-location contains url:https://github.com/org/repo. Without normalization, project lookups will fail and the component will always show "No violations found."
Proposed fix
-import { useState } from 'react';
+import { useMemo, useState } from 'react';
@@
- const repoUrl =
- entity.metadata.annotations?.['backstage.io/source-location'] ||
- entity.metadata.annotations?.['github.com/project-slug'];
+ const repoUrl = useMemo(() => {
+ const sourceLocation =
+ entity.metadata.annotations?.['backstage.io/source-location'];
+ if (sourceLocation) {
+ return sourceLocation.replace(/^url:/, '');
+ }
+ return undefined;
+ }, [entity.metadata.annotations]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/backstage-apme/src/components/ApmeViolationsTable/ApmeViolationsTable.tsx`
around lines 118 - 128, The repoUrl read in ApmeViolationsTable.tsx can contain
a "url:" prefix from the metadata annotation; normalize it before calling the
APME API by stripping any leading "url:" (case-sensitive) and whitespace so
apmeApi.getProjectByRepoUrl receives a plain URL like
"https://github.com/org/repo"; update the repoUrl variable (used inside the
useAsync block and before calling apmeApi.getProjectByRepoUrl and
apmeApi.getViolations) to a sanitizedRepoUrl and use that for the API calls and
the dependency array.
Comment on lines
+199
to
+201
| if (result.pr_url) { | ||
| window.open(result.pr_url, '_blank'); | ||
| } |
There was a problem hiding this comment.
Opening an untrusted URL with window.open('_blank') without noopener.
result.pr_url comes from the APME backend and is opened in a new tab without noopener,noreferrer. The target page receives a non-null window.opener reference (and Referer), enabling reverse-tabnabbing if the URL is ever attacker-controlled. Same concern applies to the <Link ... target="_blank"> at line 335.
🛡️ Proposed fix
- window.open(result.pr_url, '_blank');
+ window.open(result.pr_url, '_blank', 'noopener,noreferrer');And for the table cell link at line 335:
- <Link to={row.pr_url} target="_blank">
+ <Link to={row.pr_url} target="_blank" rel="noopener noreferrer">
<LinkIcon fontSize="small" />
</Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/backstage-apme/src/components/ProjectDetailPage/ProjectDetailPage.tsx`
around lines 199 - 201, The code opens external URLs without rel protections:
update the window.open call that uses result.pr_url in ProjectDetailPage (the
branch that calls window.open(result.pr_url, '_blank')) to open links with
noopener and noreferrer protections (i.e., use feature/rel equivalent) to
prevent reverse-tabnabbing, and update the <Link ... target="_blank"> instance
in the component (the table cell link where Link has target="_blank") to include
rel="noopener noreferrer"; ensure you handle potential null/undefined
result.pr_url before opening.
Add optional SCM token field to the project creation dialog to enable scanning private Git repositories. The token is passed through to the APME gateway, which uses it for cloning and PR creation. - Add scm_token to CreateProjectRequest type - Add password field in CreateProjectDialog for token input - Token is not stored in browser state after form submission Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 22, 2026 08:43 — with
GitHub Actions
Failure
GitHub Actions
Failure
4 tasks
|
Updated CertificateDashboard with modern design inspired by IBM Concert: - Dark gradient header with host info and threshold badges - Summary cards with icons and color-coded status counts - Action Required section with cards for expired/critical/warning certs - Improved table with certificate path, status chips, and color-coded days - Responsive layout and hover effects Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 29, 2026 13:46 — with
GitHub Actions
Error
GitHub Actions
Error
The CERT_REPORT_JSON ends with ]}} (warning array, actionRequired object, main object) not }]} as the previous regex expected. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 29, 2026 13:54 — with
GitHub Actions
Error
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
plugins/self-service/src/apis.ts (2)
314-318: Move imports to the top of the file.Placing imports mid-file (after 300+ lines of code) is unconventional and reduces readability. Consider moving these imports to the top with the other imports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/self-service/src/apis.ts` around lines 314 - 318, The import of PlatformTask, TaskExecution, and CertificateReport is located far down the file; move the line "import type { PlatformTask, TaskExecution, CertificateReport } from '@ansible/backstage-rhaap-common';" up into the main import block at the top of the module alongside the other imports so all type imports are declared together and improve readability and maintainability.
361-385: Remove unusedtokenparameters from API methods.The
_tokenparameters inexecuteTaskandgetJobStatusare never used since the backend uses service tokens. Keeping them in the interface is misleading and could confuse developers about the authentication model.♻️ Proposed fix
Update the interface and implementation:
export interface PlatformOpsApi { getTasks(): Promise<{ tasks: PlatformTask[] }>; executeTask( taskId: string, - token: string, extraVars?: Record<string, unknown>, ): Promise<{ execution: TaskExecution }>; getJobStatus( jobId: number, - token: string, ): Promise<{ status: string; started: string; finished: string }>; }async executeTask( taskId: string, - _token: string, // Token not needed - backend uses service token extraVars?: Record<string, unknown>, ): Promise<{ execution: TaskExecution }> {async getJobStatus( jobId: number, - _token: string, // Token not needed - backend uses service token ): Promise<{ status: string; started: string; finished: string }> {Then update callers in
CertificateDashboard.tsx:- const result = await platformOpsApi.executeTask('cert-check', '', { + const result = await platformOpsApi.executeTask('cert-check', {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/self-service/src/apis.ts` around lines 361 - 385, The _token parameter is unused and misleading in the API methods; remove the `_token` parameter from the executeTask and getJobStatus method signatures in the API interface and their implementations (e.g., the executeTask method shown and the corresponding getJobStatus function), update any method calls (such as those in CertificateDashboard.tsx) to stop passing a token, and adjust any type declarations or overrides that referenced the old signature so all callers and implementations match the new parameter list.plugins/self-service/src/components/PlatformOperations/CertificateDashboard/CertificateDashboard.tsx (1)
318-335: Extract CSS class selection into a helper function.The nested ternaries flagged by ESLint reduce readability. Consider extracting the class determination logic into helper functions.
♻️ Proposed refactor
+const getActionCardClass = ( + type: 'expired' | 'critical' | 'warning', + classes: ReturnType<typeof useStyles>, +) => { + switch (type) { + case 'expired': + return classes.actionCardExpired; + case 'critical': + return classes.actionCardCritical; + default: + return classes.actionCardWarning; + } +}; + +const getDaysClass = ( + type: 'expired' | 'critical' | 'warning', + classes: ReturnType<typeof useStyles>, +) => { + switch (type) { + case 'expired': + return classes.statusExpired; + case 'critical': + return classes.statusCritical; + default: + return classes.statusWarning; + } +}; const ActionRequiredCard: React.FC<{ cert: CertificateInfo; type: 'expired' | 'critical' | 'warning'; }> = ({ cert, type }) => { const classes = useStyles(); - const cardClass = - type === 'expired' - ? classes.actionCardExpired - : type === 'critical' - ? classes.actionCardCritical - : classes.actionCardWarning; - - const daysClass = - type === 'expired' - ? classes.statusExpired - : type === 'critical' - ? classes.statusCritical - : classes.statusWarning; + const cardClass = getActionCardClass(type, classes); + const daysClass = getDaysClass(type, classes);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/self-service/src/components/PlatformOperations/CertificateDashboard/CertificateDashboard.tsx` around lines 318 - 335, The nested ternaries for cardClass, daysClass and daysText hurt readability—extract these into small pure helper functions (e.g., getCardClass(type), getDaysClass(type) and getDaysText(type, cert.daysRemaining)) and replace the inline ternaries with calls to those helpers; each helper should switch on the type ('expired' | 'critical' | otherwise) and return the appropriate class or text (for getDaysText use Math.abs for expired wording), keeping the logic identical but centralized for clarity and easier testing.plugins/self-service/src/components/PlatformOperations/PlatformOpsPage.tsx (1)
119-129: Avoidas anytype assertion.The
as anycast bypasses TypeScript's type checking. Consider defining a proper type or using a more specific assertion to maintain type safety.♻️ Suggested approach
If
HeaderTabsexpects a specific shape, you can cast to that specific type or adjust the mapping:tabs={ - tabs.map(({ label, icon }) => ({ - id: label.toLowerCase(), - label: ( - <Box className={classes.tabWithIcon}> - {icon} - {label} - </Box> - ), - })) as any + tabs.map(({ label, icon }) => ({ + id: label.toLowerCase(), + label: ( + <Box className={classes.tabWithIcon}> + {icon} + {label} + </Box> + ), + })) as { id: string; label: React.ReactNode }[] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/self-service/src/components/PlatformOperations/PlatformOpsPage.tsx` around lines 119 - 129, The mapping that builds the tabs prop uses a blanket "as any" cast which circumvents TypeScript checks; replace this with a proper typed mapping by creating or using the expected tab type (e.g., the HeaderTabs item/interface) and cast the mapped array to that specific type instead of any — update the mapping where tabs.map(({ label, icon }) => ...) and the prop passed to HeaderTabs (tabs={...}) to return objects matching the expected shape (id, label JSX, etc.) and assert that concrete type (or change the HeaderTabs prop type) so the "as any" is removed.plugins/self-service/src/components/SidebarItems/SidebarItems.tsx (1)
85-95: Consider permission gating for consistency.Unlike other sidebar items (
EEBuilderSidebarItem,CollectionsSidebarItem,GitRepositoriesSidebarItem), this component renders unconditionally without checking permissions. While the route page comment notes this is intentional for now, consider adding permission gating whenplatformOpsViewPermissionis implemented to maintain consistency with the pattern used by sibling components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/self-service/src/components/SidebarItems/SidebarItems.tsx` around lines 85 - 95, PlatformOpsSidebarItem currently renders unconditionally; wrap its SidebarItem with the same permission gating pattern used by EEBuilderSidebarItem/CollectionsSidebarItem/GitRepositoriesSidebarItem so it only renders when platformOpsViewPermission is granted—use the same permission hook or Permissioned component your codebase uses (e.g., check platformOpsViewPermission inside PlatformOpsSidebarItem and return null or the gated JSX when the permission is not present).plugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.ts (1)
52-52: Usesubstringinstead of deprecatedsubstr.
substris deprecated. This is a minor issue but worth fixing for consistency.♻️ Suggested fix
- const executionId = `exec-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + const executionId = `exec-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.ts` at line 52, The generation of executionId uses the deprecated String.prototype.substr; update the expression that builds executionId (the constant named executionId) to use substring or slice instead of substr (e.g., replace .substr(2, 9) with .substring(2, 11) or .slice(2, 11)) so the random token extraction is equivalent but avoids the deprecated API.plugins/catalog-backend-module-rhaap/src/platformOps/certificateParser.ts (1)
52-78: Regex may fail on multiline stdout or if JSON contains}]}.The regex
CERT_REPORT_JSON:(\{.*\}\]\})uses greedy matching with.which doesn't match newlines by default. If the playbook JSON spans multiple lines, this will fail silently.Consider using the
s(dotall) flag or a more robust extraction approach:♻️ Suggested fix for multiline support
- const msgJsonMatch = stdout.match(/CERT_REPORT_JSON:(\{.*\}\]\})/); + const msgJsonMatch = stdout.match(/CERT_REPORT_JSON:(\{.*\}\]\})/s);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/catalog-backend-module-rhaap/src/platformOps/certificateParser.ts` around lines 52 - 78, The regex in parseCertificateOutput (msgJsonMatch = stdout.match(/CERT_REPORT_JSON:(\{.*\}\]\})/)) can fail on multiline JSON or when dot is greedy; update the extraction to use a dotall-aware pattern (e.g., enable the s flag or use [\s\S] instead of .) and make the brace-end match non-greedy if needed so the CERT_REPORT_JSON payload is reliably captured, then keep the existing unescape/JSON.parse flow and adjust parseErrors handling in parseCertificateOutput accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.ts`:
- Around line 45-142: executeTask currently awaits aapClient.fetchResult (and
getJobStdout) synchronously which can hit HTTP gateway timeouts for long-running
jobs; change executeTask to launch the job via aapClient.launchJobTemplateById,
immediately return a TaskExecution with status 'running', the generated
executionId, jobId, startedAt and mergedExtraVars (no long blocking awaits), and
move the polling/parse logic (aapClient.fetchResult, aapClient.getJobStdout,
parseCertificateOutput, computeCertificateSummary) into a background worker or a
separate endpoint that clients can poll (e.g., pollJobResult) which will update
the stored TaskExecution to 'completed' or 'failed' and populate output/error
when finished.
- Around line 161-192: Remove the dead exported helpers by deleting the
createPlatformOpsAuthMiddleware and extractBearerToken functions from
platformOpsHelpers.ts and any exports of them; ensure no other modules import
createPlatformOpsAuthMiddleware or extractBearerToken (update or remove those
imports if present) and keep the file compiling (remove unused logger/type
imports if they become unused). Verify the router's existing getAapServiceToken
remains the single source of truth and run a build to confirm no missing
references.
In
`@plugins/self-service/src/components/PlatformOperations/CertificateDashboard/CertificateDashboard.tsx`:
- Around line 627-629: The list rendering uses non-unique keys (e.g.,
expiredCerts.map with <ActionRequiredCard key={cert.name} ... />) which can
break React reconciliation; change the key to a truly unique identifier such as
combining certificate fields (e.g., cert.name + '|' + cert.host or
cert.serialNumber or cert.id), with a deterministic fallback to the index only
if no unique id exists; apply the same key-fix pattern to the other certificate
lists that render ActionRequiredCard (the other map calls in this component) so
every mapped element has a stable, unique key.
---
Nitpick comments:
In `@plugins/catalog-backend-module-rhaap/src/platformOps/certificateParser.ts`:
- Around line 52-78: The regex in parseCertificateOutput (msgJsonMatch =
stdout.match(/CERT_REPORT_JSON:(\{.*\}\]\})/)) can fail on multiline JSON or
when dot is greedy; update the extraction to use a dotall-aware pattern (e.g.,
enable the s flag or use [\s\S] instead of .) and make the brace-end match
non-greedy if needed so the CERT_REPORT_JSON payload is reliably captured, then
keep the existing unescape/JSON.parse flow and adjust parseErrors handling in
parseCertificateOutput accordingly.
In `@plugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.ts`:
- Line 52: The generation of executionId uses the deprecated
String.prototype.substr; update the expression that builds executionId (the
constant named executionId) to use substring or slice instead of substr (e.g.,
replace .substr(2, 9) with .substring(2, 11) or .slice(2, 11)) so the random
token extraction is equivalent but avoids the deprecated API.
In `@plugins/self-service/src/apis.ts`:
- Around line 314-318: The import of PlatformTask, TaskExecution, and
CertificateReport is located far down the file; move the line "import type {
PlatformTask, TaskExecution, CertificateReport } from
'@ansible/backstage-rhaap-common';" up into the main import block at the top of
the module alongside the other imports so all type imports are declared together
and improve readability and maintainability.
- Around line 361-385: The _token parameter is unused and misleading in the API
methods; remove the `_token` parameter from the executeTask and getJobStatus
method signatures in the API interface and their implementations (e.g., the
executeTask method shown and the corresponding getJobStatus function), update
any method calls (such as those in CertificateDashboard.tsx) to stop passing a
token, and adjust any type declarations or overrides that referenced the old
signature so all callers and implementations match the new parameter list.
In
`@plugins/self-service/src/components/PlatformOperations/CertificateDashboard/CertificateDashboard.tsx`:
- Around line 318-335: The nested ternaries for cardClass, daysClass and
daysText hurt readability—extract these into small pure helper functions (e.g.,
getCardClass(type), getDaysClass(type) and getDaysText(type,
cert.daysRemaining)) and replace the inline ternaries with calls to those
helpers; each helper should switch on the type ('expired' | 'critical' |
otherwise) and return the appropriate class or text (for getDaysText use
Math.abs for expired wording), keeping the logic identical but centralized for
clarity and easier testing.
In `@plugins/self-service/src/components/PlatformOperations/PlatformOpsPage.tsx`:
- Around line 119-129: The mapping that builds the tabs prop uses a blanket "as
any" cast which circumvents TypeScript checks; replace this with a proper typed
mapping by creating or using the expected tab type (e.g., the HeaderTabs
item/interface) and cast the mapped array to that specific type instead of any —
update the mapping where tabs.map(({ label, icon }) => ...) and the prop passed
to HeaderTabs (tabs={...}) to return objects matching the expected shape (id,
label JSX, etc.) and assert that concrete type (or change the HeaderTabs prop
type) so the "as any" is removed.
In `@plugins/self-service/src/components/SidebarItems/SidebarItems.tsx`:
- Around line 85-95: PlatformOpsSidebarItem currently renders unconditionally;
wrap its SidebarItem with the same permission gating pattern used by
EEBuilderSidebarItem/CollectionsSidebarItem/GitRepositoriesSidebarItem so it
only renders when platformOpsViewPermission is granted—use the same permission
hook or Permissioned component your codebase uses (e.g., check
platformOpsViewPermission inside PlatformOpsSidebarItem and return null or the
gated JSX when the permission is not present).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: aae8436d-f4cf-42b0-81d8-7403f9254c57
📒 Files selected for processing (24)
app-config.yamlpackages/app/src/components/Root/Root.tsxplugins/backstage-rhaap-common/src/AAPClient/AAPClient.tsplugins/backstage-rhaap-common/src/types/index.tsplugins/backstage-rhaap-common/src/types/platformOps.tsplugins/catalog-backend-module-rhaap/src/mock/mockIAAPService.tsplugins/catalog-backend-module-rhaap/src/module.tsplugins/catalog-backend-module-rhaap/src/platformOps/certificateParser.tsplugins/catalog-backend-module-rhaap/src/platformOps/index.tsplugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.tsplugins/catalog-backend-module-rhaap/src/router.tsplugins/scaffolder-backend-module-backstage-rhaap/src/actions/mockIAAPService.tsplugins/self-service/src/apis.tsplugins/self-service/src/components/PlatformOperations/CertificateDashboard/CertificateDashboard.tsxplugins/self-service/src/components/PlatformOperations/CertificateDashboard/index.tsplugins/self-service/src/components/PlatformOperations/PlatformOpsPage.tsxplugins/self-service/src/components/PlatformOperations/PlatformOpsRoutesPage.tsxplugins/self-service/src/components/PlatformOperations/index.tsplugins/self-service/src/components/RouteView/RouteView.tsxplugins/self-service/src/components/SidebarItems/SidebarItems.tsxplugins/self-service/src/components/SidebarItems/index.tsplugins/self-service/src/index.tsplugins/self-service/src/plugin.tsplugins/self-service/src/routes.ts
✅ Files skipped from review due to trivial changes (9)
- plugins/self-service/src/components/SidebarItems/index.ts
- plugins/self-service/src/components/PlatformOperations/CertificateDashboard/index.ts
- plugins/self-service/src/index.ts
- plugins/self-service/src/routes.ts
- plugins/self-service/src/components/PlatformOperations/index.ts
- plugins/backstage-rhaap-common/src/types/index.ts
- plugins/catalog-backend-module-rhaap/src/platformOps/index.ts
- plugins/scaffolder-backend-module-backstage-rhaap/src/actions/mockIAAPService.ts
- plugins/backstage-rhaap-common/src/types/platformOps.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/app/src/components/Root/Root.tsx
- app-config.yaml
Comment on lines
+45
to
+142
| export async function executeTask( | ||
| context: PlatformOpsContext, | ||
| task: PlatformTask, | ||
| token: string, | ||
| extraVars?: Record<string, unknown>, | ||
| ): Promise<TaskExecution> { | ||
| const { logger, aapClient } = context; | ||
| const executionId = `exec-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; | ||
|
|
||
| if (!task.templateId) { | ||
| throw new Error(`Task "${task.name}" has no associated job template ID`); | ||
| } | ||
|
|
||
| // Merge default extra vars with provided ones (provided takes precedence) | ||
| const mergedExtraVars = { | ||
| ...task.defaultExtraVars, | ||
| ...extraVars, | ||
| }; | ||
|
|
||
| logger.info( | ||
| `[platformOps] Executing task "${task.name}" (template ID: ${task.templateId})`, | ||
| ); | ||
|
|
||
| const startTime = new Date().toISOString(); | ||
|
|
||
| try { | ||
| // Launch the job template with merged extra vars | ||
| const launchResult = await aapClient.launchJobTemplateById( | ||
| task.templateId, | ||
| token, | ||
| Object.keys(mergedExtraVars).length > 0 ? mergedExtraVars : undefined, | ||
| ); | ||
|
|
||
| logger.info( | ||
| `[platformOps] Job launched: ${launchResult.jobId}, polling for completion...`, | ||
| ); | ||
|
|
||
| // Poll for job completion | ||
| const jobResult = await aapClient.fetchResult(launchResult.jobId, token); | ||
|
|
||
| const endTime = new Date().toISOString(); | ||
| const jobStatus = jobResult.jobData?.status || 'unknown'; | ||
|
|
||
| // Get stdout for parsing | ||
| const stdout = await aapClient.getJobStdout(launchResult.jobId, token); | ||
|
|
||
| // Parse output based on parser type | ||
| let parsedOutput: unknown = stdout; | ||
| if (task.parserType === 'certificate') { | ||
| const parsed = parseCertificateOutput(stdout); | ||
| // Use summary from playbook if available, otherwise compute it | ||
| const summary = parsed.summary || computeCertificateSummary(parsed.certificates); | ||
| parsedOutput = { | ||
| certificates: parsed.certificates, | ||
| summary, | ||
| host: parsed.host, | ||
| checkDate: parsed.checkDate, | ||
| thresholds: parsed.thresholds, | ||
| parseErrors: parsed.parseErrors, | ||
| rawStdout: stdout, // Include raw output for debugging | ||
| } as CertificateReport; | ||
| } | ||
|
|
||
| const execution: TaskExecution = { | ||
| id: executionId, | ||
| taskId: task.id, | ||
| status: jobStatus === 'successful' ? 'completed' : 'failed', | ||
| startedAt: startTime, | ||
| completedAt: endTime, | ||
| jobId: launchResult.jobId, | ||
| output: parsedOutput, | ||
| error: | ||
| jobStatus !== 'successful' | ||
| ? `Job finished with status: ${jobStatus}` | ||
| : undefined, | ||
| }; | ||
|
|
||
| logger.info( | ||
| `[platformOps] Task "${task.name}" completed with status: ${execution.status}`, | ||
| ); | ||
|
|
||
| return execution; | ||
| } catch (error) { | ||
| const endTime = new Date().toISOString(); | ||
| const errorMessage = error instanceof Error ? error.message : String(error); | ||
|
|
||
| logger.error(`[platformOps] Task "${task.name}" failed: ${errorMessage}`); | ||
|
|
||
| return { | ||
| id: executionId, | ||
| taskId: task.id, | ||
| status: 'failed', | ||
| startedAt: startTime, | ||
| completedAt: endTime, | ||
| error: errorMessage, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Long-running jobs may cause HTTP request timeouts.
executeTask synchronously polls fetchResult until the job completes. For certificate checks across large infrastructure, this could exceed typical HTTP gateway timeouts (30-60s). Consider returning the job ID immediately and letting clients poll the status endpoint separately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.ts`
around lines 45 - 142, executeTask currently awaits aapClient.fetchResult (and
getJobStdout) synchronously which can hit HTTP gateway timeouts for long-running
jobs; change executeTask to launch the job via aapClient.launchJobTemplateById,
immediately return a TaskExecution with status 'running', the generated
executionId, jobId, startedAt and mergedExtraVars (no long blocking awaits), and
move the polling/parse logic (aapClient.fetchResult, aapClient.getJobStdout,
parseCertificateOutput, computeCertificateSummary) into a background worker or a
separate endpoint that clients can poll (e.g., pollJobResult) which will update
the stored TaskExecution to 'completed' or 'failed' and populate output/error
when finished.
Comment on lines
+161
to
+192
| export function createPlatformOpsAuthMiddleware(options: { | ||
| logger: LoggerService; | ||
| }) { | ||
| return async (req: Request, res: Response, next: NextFunction) => { | ||
| // For now, just check that Authorization header is present | ||
| const authHeader = req.headers.authorization; | ||
| if (!authHeader || !authHeader.startsWith('Bearer ')) { | ||
| options.logger.warn('[platformOps] Missing or invalid Authorization header'); | ||
| res.status(401).json({ error: 'Unauthorized: missing bearer token' }); | ||
| return; | ||
| } | ||
| next(); | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Extracts the AAP token from the X-AAP-Token header or Authorization header. | ||
| */ | ||
| export function extractBearerToken(req: Request): string | undefined { | ||
| // First try X-AAP-Token header (preferred for platform ops) | ||
| const aapToken = req.headers['x-aap-token']; | ||
| if (aapToken && typeof aapToken === 'string') { | ||
| return aapToken; | ||
| } | ||
|
|
||
| // Fall back to Authorization header | ||
| const authHeader = req.headers.authorization; | ||
| if (authHeader && authHeader.startsWith('Bearer ')) { | ||
| return authHeader.substring(7); | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if extractBearerToken or createPlatformOpsAuthMiddleware are used anywhere
echo "=== Searching for extractBearerToken usage ==="
rg -n 'extractBearerToken' --type ts
echo ""
echo "=== Searching for createPlatformOpsAuthMiddleware usage ==="
rg -n 'createPlatformOpsAuthMiddleware' --type tsRepository: ansible/ansible-backstage-plugins
Length of output: 473
🏁 Script executed:
# Search for imports of these functions from platformOpsHelpers
rg -n 'from.*platformOpsHelpers|import.*extractBearerToken|import.*createPlatformOpsAuthMiddleware' --type ts
# Also check if they're re-exported
rg -n 'export.*from.*platformOpsHelpers' --type ts
# Check the actual file to see what's exported from it
fd 'index.ts|index.js' plugins/catalog-backend-module-rhaap/src/platformOps --exec cat {}Repository: ansible/ansible-backstage-plugins
Length of output: 356
🏁 Script executed:
# Search for imports from platformOps module
rg -n "from.*['\"].*platformOps['\"]|import.*platformOps" --type ts
# Look for getAapServiceToken usage
rg -n 'getAapServiceToken' --type ts
# Check the router file to understand the architecture
fd -i 'router\|route' plugins/catalog-backend-module-rhaap/src --type f | head -10Repository: ansible/ansible-backstage-plugins
Length of output: 544
🏁 Script executed:
# Check what's imported from platformOps at line 69 in router.ts
sed -n '60,75p' plugins/catalog-backend-module-rhaap/src/router.tsRepository: ansible/ansible-backstage-plugins
Length of output: 618
🏁 Script executed:
# Comprehensive search for any references to these functions including in comments, type definitions, etc.
rg -n 'extractBearerToken|createPlatformOpsAuthMiddleware' --type ts -A 2 -B 2Repository: ansible/ansible-backstage-plugins
Length of output: 1277
Remove unused exports extractBearerToken and createPlatformOpsAuthMiddleware.
These functions are exported from platformOpsHelpers but never imported or used anywhere in the codebase. The router implements its own local getAapServiceToken() instead. Remove these functions to reduce dead code and technical debt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-rhaap/src/platformOps/platformOpsHelpers.ts`
around lines 161 - 192, Remove the dead exported helpers by deleting the
createPlatformOpsAuthMiddleware and extractBearerToken functions from
platformOpsHelpers.ts and any exports of them; ensure no other modules import
createPlatformOpsAuthMiddleware or extractBearerToken (update or remove those
imports if present) and keep the file compiling (remove unused logger/type
imports if they become unused). Verify the router's existing getAapServiceToken
remains the single source of truth and run a build to confirm no missing
references.
- Click summary cards (Total, OK, Warning, Critical, Expired, Missing) to filter table - Selected card gets highlighted border - "Clear filter" chip appears when filtered - Removed hardcoded Action Required section - table now handles all display - Table header shows active filter and count Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ffirg
had a problem deploying
to
sonarcloud-analysis
April 29, 2026 14:07 — with
GitHub Actions
Failure
GitHub Actions
Failure
8 tasks
craig-br
added a commit
that referenced
this pull request
Jul 2, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends #263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
craig-br
added a commit
that referenced
this pull request
Jul 2, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends #263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
craig-br
added a commit
that referenced
this pull request
Jul 2, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends #263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
djdanielsson
pushed a commit
to djdanielsson/ansible-backstage-plugins
that referenced
this pull request
Jul 28, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends ansible#263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
djdanielsson
pushed a commit
to djdanielsson/ansible-backstage-plugins
that referenced
this pull request
Aug 3, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends ansible#263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
djdanielsson
pushed a commit
to djdanielsson/ansible-backstage-plugins
that referenced
this pull request
Aug 14, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends ansible#263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
djdanielsson
pushed a commit
to djdanielsson/ansible-backstage-plugins
that referenced
this pull request
Aug 18, 2026
Catalog-native APME integration for the Ansible automation portal: entity Quality tab, fleet overview, Git Repos violations column, selective remediation workflow, mock mode, scaffolder template, and portal proxy routes. Extends ansible#263 with ADR-009/010 extension points, Dev Spaces links, and portal-side publish when publishViaGateway is disabled. Includes merge from main (Playwright e2e, sync progress, AAP client updates), OpenAPI route documentation, CI/lint/CodeQL fixes, and review bot suggestions. Co-authored-by: Cursor <cursoragent@cursor.com>
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
3 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.
Summary
Adds a complete APME (Ansible Policy & Modernization Engine) plugin for Backstage/RHDH, enabling static analysis, policy enforcement, and AI-powered remediation of Ansible content.
Features
/apmeNew Packages
@ansible/backstage-apme-common@ansible/backstage-apme@ansible/catalog-backend-module-apmeTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit