Feature/discoverability 2#6
Open
rithakith wants to merge 3 commits into
Open
Conversation
…O2 API Manager into the Backstage catalog
📝 WalkthroughWalkthroughAdds self-hosted WSO2 gateway discovery, normalized gateway metadata, API definition fetch endpoints, frontend gateway filtering/operations-only rendering, and related config/types. Also renames several ChangesWSO2 gateway discovery, definition fetch, and UI integration
Backend wiring, package renames, permission cleanup, and packaging scripts
Sequence DiagramsequenceDiagram
participant UI as Frontend UI
participant Router as Backend Router
participant Client as WSO2 API Client
participant GW as Self-Hosted Gateway
participant Provider as Catalog Provider
UI->>Router: GET /apis/:apiId/definition?discoveredFrom=gw
activate Router
Router->>Client: lookup gateway config (wso2PlatformGateway)
Client-->>Router: gateway { discoveryUrl, auth, ... }
Router->>GW: GET {discoveryUrl}/{apiId}
activate GW
GW-->>Router: definition / spec / operations
deactivate GW
Router->>Router: parse/normalize definition
Router-->>UI: JSON definition or operations-only response
deactivate Router
Provider->>Client: read wso2PlatformGateway config
activate Client
Client-->>Provider: selfHostedGateways list
Provider->>GW: fetch APIs and specs (per gateway)
activate GW
GW-->>Provider: API list + swagger/spec
deactivate GW
Provider->>Provider: normalize gatewayType, store fetchedSwagger
Provider-->>Catalog: create/update entities with gateways metadata
deactivate Client
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. 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: 9
🤖 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-wso2-apim/src/module.ts`:
- Around line 30-33: The scheduler frequency used in
scheduler.createScheduledTaskRunner is too long (every 15 minutes) and can
exceed the frontend Wso2ApiManagerPage polling window (catalogSyncTimeoutSeconds
default 60s); change the scheduled frequency to run more frequently than the
frontend timeout (e.g., seconds-level or at least under
catalogSyncTimeoutSeconds), adjust initialDelay/timeout as needed, and ensure
the schedule config (the object passed to createScheduledTaskRunner) will
trigger provider refreshes more often than the frontend polling period so users
won't time out on an empty catalog.
In
`@plugins/catalog-backend-module-wso2-apim/src/providers/Wso2ApiEntityProvider.ts`:
- Line 597: The annotation currently always sets 'wso2.com/api-gateway-vendor'
using normalizeGatewayType(api.gatewayType || api.gatewayVendor), which causes
undefined vendors to be normalized to "apiplatform"; instead, only call
normalizeGatewayType and add the 'wso2.com/api-gateway-vendor' annotation when
api.gatewayType or api.gatewayVendor is present. Update the code around the
annotation in Wso2ApiEntityProvider.ts to guard the call (e.g., check
api.gatewayType || api.gatewayVendor) so unknown/absent vendors are not
synthesized, leaving extractGateways() to only see explicitly provided gateway
metadata.
- Around line 718-730: The code currently serializes the entire
selfHostedGateways array into the 'wso2.com/gateway-endpoints' annotation (the
IIFE that references selfHostedGateways), causing every API to appear deployed
everywhere; change that IIFE to first filter selfHostedGateways to only those
gateways the API is actually deployed to (e.g. match gateway.environmentName or
gateway labels against properties on api such as api.environments /
api.gatewayLabels or whatever deployment indicator your API objects use), then
map only the filtered list to the same JSON shape (keep use of
normalizeGatewayType, api.context handling and URL normalization). Apply the
identical filtering change to the duplicated block used for API Product
generation so both places only attach relevant self-hosted gateways.
In `@plugins/wso2-api-manager-backend/config.d.ts`:
- Around line 22-32: The discoveryAuth field on the wso2PlatformGateway config
object is annotated as frontend-visible but contains credentials; remove the
`@visibility frontend` from discoveryAuth and mark it as backend-only (or simply
omit the frontend visibility tag) so secret credentials are not included in the
browser bundle; update the config schema for the wso2PlatformGateway nested
field `discoveryAuth` (and verify only non-secret fields like `urls` and
`discoveryUrl` retain `@visibility frontend`) to ensure credentials are
backend-only.
In `@plugins/wso2-api-manager-backend/src/service/router.ts`:
- Around line 117-168: The GET /apis/:apiId/definition route is missing
authentication; wrap or apply the existing ensureAuthenticated middleware to
this handler so only authenticated callers can reach discoveryAuth-protected
gateway specs. Update the router.get registration for the handler in router.ts
to include ensureAuthenticated (same function used by other /apis/* routes) as
middleware before the async (req, res) callback, or call ensureAuthenticated at
the start of the handler and abort when it fails; keep the rest of the logic in
the async function (references: router.get('/apis/:apiId/definition',
ensureAuthenticated, ...) or calling ensureAuthenticated within that route).
In
`@plugins/wso2-api-manager/src/components/EntityWso2ApiDefinitionCard/EntityWso2ApiDefinitionCard.tsx`:
- Around line 663-666: When hasOperationsOnly becomes true the "View Source" Tab
is removed but activeTab may still point to that index; add a useEffect in
EntityWso2ApiDefinitionCard that watches hasOperationsOnly (and activeTab) and
when hasOperationsOnly is true and activeTab === 1 call the state updater to
setActiveTab(0) so the component resets to the remaining tab; reference the
activeTab state setter and the hasOperationsOnly boolean used around the <Tab
id="tab-source" .../> and <Tab id="tab-swagger-ui" .../> to locate where to add
the effect.
- Around line 156-205: Wso2OperationsList assumes operations have op.method and
op.path and calls op.method.toUpperCase(), which can throw when operations use
the alternate shape { verb, target }; update the component to normalise each
operation first (e.g., const methodRaw = op.method ?? op.verb ?? ''; const
method = methodRaw.toString().toUpperCase(); const path = op.path ?? op.target
?? '') and then use method for getMethodColor(method) and to render the method
label and use path for the Typography; ensure you guard against empty/null
values before calling toUpperCase so both shapes ({method,path} and
{verb,target}) are handled.
In
`@plugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx`:
- Around line 596-607: The retry button clears isTimedOut but doesn't restart
the timeout window because syncStartTime was captured only once; update the
Button onClick in Wso2ApiManagerPage to also reset the syncStartTime (or its
ref) to the current time before calling apiListState.retry(), e.g., call the
setter used to store syncStartTime (or update the useRef value) along with
setIsTimedOut(false) so the polling timeout calculation uses the new start time
on retry.
- Around line 390-393: filteredApiProducts currently returns the full list and
ignores the global selected gateway; update the useMemo for filteredApiProducts
to mirror the APIs filter: depend on selectedGateway and
apiProductListState.value?.apiProducts, and return
apiProductListState.value?.apiProducts.filter(p => !selectedGateway ||
p.gatewayId === selectedGateway) (or the equivalent gateway identifier field
used by APIs) so the dropdown selection filters API Products as well; keep the
constant name filteredApiProducts and the source
apiProductListState.value?.apiProducts.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc9f7cba-52bf-4017-a060-626bdaf47c1d
📒 Files selected for processing (9)
packages/backend/src/index.tsplugins/catalog-backend-module-wso2-apim/src/module.tsplugins/catalog-backend-module-wso2-apim/src/providers/Wso2ApiEntityProvider.tsplugins/wso2-api-manager-backend/config.d.tsplugins/wso2-api-manager-backend/src/service/router.tsplugins/wso2-api-manager-backend/src/service/wso2Client.tsplugins/wso2-api-manager/src/api/types.tsplugins/wso2-api-manager/src/components/EntityWso2ApiDefinitionCard/EntityWso2ApiDefinitionCard.tsxplugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx
Comment on lines
30
to
33
| const schedule = scheduler.createScheduledTaskRunner({ | ||
| frequency: { minutes: 1 }, // Run every 1 minute for testing/demo purposes; usually { hours: 1 } | ||
| frequency: { minutes: 15 }, // Run every 15 minutes | ||
| timeout: { minutes: 5 }, | ||
| initialDelay: { seconds: 15 }, // Wait a bit before first run |
There was a problem hiding this comment.
This cadence is now out of sync with the frontend timeout.
Wso2ApiManagerPage stops polling after catalogSyncTimeoutSeconds (default 60s), but this scheduler now only refreshes every 15 minutes. On an empty catalog, users can hit the page just after a sync and time out long before the next provider run even though the backend is healthy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-wso2-apim/src/module.ts` around lines 30 - 33,
The scheduler frequency used in scheduler.createScheduledTaskRunner is too long
(every 15 minutes) and can exceed the frontend Wso2ApiManagerPage polling window
(catalogSyncTimeoutSeconds default 60s); change the scheduled frequency to run
more frequently than the frontend timeout (e.g., seconds-level or at least under
catalogSyncTimeoutSeconds), adjust initialDelay/timeout as needed, and ensure
the schedule config (the object passed to createScheduledTaskRunner) will
trigger provider refreshes more often than the frontend polling period so users
won't time out on an empty catalog.
| 'wso2.com/api-type': api.type || '', | ||
| 'wso2.com/api-lifecycle-status': api.lifeCycleStatus || '', | ||
| 'wso2.com/api-gateway-vendor': (api.gatewayType || api.gatewayVendor || 'wso2').toLowerCase(), | ||
| 'wso2.com/api-gateway-vendor': normalizeGatewayType(api.gatewayType || api.gatewayVendor), |
There was a problem hiding this comment.
Don't synthesize apiplatform when the vendor is missing.
normalizeGatewayType(undefined) returns apiplatform, so APIs with no gatewayType/gatewayVendor get annotated as though they belong to ApiPlatform. extractGateways() later treats that as real gateway metadata, which will misclassify unknown APIs in the gateway column and filter.
Proposed fix
- 'wso2.com/api-gateway-vendor': normalizeGatewayType(api.gatewayType || api.gatewayVendor),
+ ...(api.gatewayType || api.gatewayVendor
+ ? {
+ 'wso2.com/api-gateway-vendor': normalizeGatewayType(
+ api.gatewayType || api.gatewayVendor,
+ ),
+ }
+ : {}),📝 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
| 'wso2.com/api-gateway-vendor': normalizeGatewayType(api.gatewayType || api.gatewayVendor), | |
| ...(api.gatewayType || api.gatewayVendor | |
| ? { | |
| 'wso2.com/api-gateway-vendor': normalizeGatewayType( | |
| api.gatewayType || api.gatewayVendor, | |
| ), | |
| } | |
| : {}), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/catalog-backend-module-wso2-apim/src/providers/Wso2ApiEntityProvider.ts`
at line 597, The annotation currently always sets 'wso2.com/api-gateway-vendor'
using normalizeGatewayType(api.gatewayType || api.gatewayVendor), which causes
undefined vendors to be normalized to "apiplatform"; instead, only call
normalizeGatewayType and add the 'wso2.com/api-gateway-vendor' annotation when
api.gatewayType or api.gatewayVendor is present. Update the code around the
annotation in Wso2ApiEntityProvider.ts to guard the call (e.g., check
api.gatewayType || api.gatewayVendor) so unknown/absent vendors are not
synthesized, leaving extractGateways() to only see explicitly provided gateway
metadata.
Comment on lines
+718
to
+730
| 'wso2.com/gateway-endpoints': (() => { | ||
| return JSON.stringify(selfHostedGateways.map(gw => ({ | ||
| environmentName: gw.environmentName, | ||
| environmentType: gw.environmentType || 'PRODUCTION', | ||
| gatewayType: normalizeGatewayType('Self-hosted'), | ||
| displayName: gw.environmentName, | ||
| urls: gw.urls.map(u => { | ||
| const base = u.replace(/\/$/, ''); | ||
| const ctx = (api.context || '/').startsWith('/') ? (api.context || '/') : `/${api.context || '/'}`; | ||
| return `${base}${ctx.replace(/\/$/, '')}`; | ||
| }) | ||
| }))); | ||
| })(), |
There was a problem hiding this comment.
Don't attach every configured self-hosted gateway to every API.
This serializes the full selfHostedGateways list into each publisher API regardless of where the API is actually deployed. Downstream, plugins/wso2-api-manager/src/components/EntityWso2ApiDefinitionCard/EntityWso2ApiDefinitionCard.tsx merges wso2.com/gateway-endpoints into gatewayUrls, so unrelated APIs look deployed on self-hosted gateways and "Try it out" can target the wrong host. The same bug is duplicated in the API Product block at Lines 858-870.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/catalog-backend-module-wso2-apim/src/providers/Wso2ApiEntityProvider.ts`
around lines 718 - 730, The code currently serializes the entire
selfHostedGateways array into the 'wso2.com/gateway-endpoints' annotation (the
IIFE that references selfHostedGateways), causing every API to appear deployed
everywhere; change that IIFE to first filter selfHostedGateways to only those
gateways the API is actually deployed to (e.g. match gateway.environmentName or
gateway labels against properties on api such as api.environments /
api.gatewayLabels or whatever deployment indicator your API objects use), then
map only the filtered list to the same JSON shape (keep use of
normalizeGatewayType, api.context handling and URL normalization). Apply the
identical filtering change to the duplicated block used for API Product
generation so both places only attach relevant self-hosted gateways.
Comment on lines
+22
to
+32
| /** | ||
| * Configuration for self-hosted WSO2 API Platform Gateways. | ||
| * @visibility frontend | ||
| */ | ||
| wso2PlatformGateway?: Array<{ | ||
| name: string; | ||
| urls: string[]; | ||
| /** @visibility frontend */ | ||
| discoveryUrl?: string; | ||
| /** @visibility frontend */ | ||
| discoveryAuth?: string; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Backstage config schema, if a config object or nested field is annotated with @visibility frontend, does that value become available in the frontend app config? What is the recommended way to keep nested secrets like discoveryAuth backend-only while still exposing non-secret gateway metadata?
💡 Result:
Yes, if a config object or nested field is annotated with @visibility frontend (or visibility: "frontend" in JSON schema), that value becomes available in the frontend app config. The @visibility frontend annotation makes the specific field visible in both frontend and backend, while the default is backend-only. For nested objects, use @deepVisibility frontend to propagate visibility recursively downward. Visibility applies only to the direct field; unmarked nested children of a frontend-visible object appear as empty objects in the frontend unless explicitly marked. The recommended way to keep nested secrets like discoveryAuth backend-only while exposing non-secret gateway metadata (e.g., proxy or gateway endpoints) is to use granular visibility annotations in the configuration schema. Mark secret fields or sub-objects with @visibility backend or @deepVisibility secret (e.g., on discovery.auth), and mark non-secret metadata fields with @visibility frontend (e.g., on proxy routes or discovery.endpoints). This filters the config at load time: secrets remain backend-only and may be excluded from logs, while safe metadata reaches the frontend. Use backstage-cli config:print --frontend to verify. Avoid frontend visibility on secret-containing objects; instead, define separate leaf fields for exposure. For discoveryAuth specifically (likely backend.discovery.auth or similar), annotate it secret or backend, and expose only needed non-auth parts like base URLs if required in frontend via explicit frontend marks.
Citations:
- 1: https://backstage.io/docs/next/conf/defining/
- 2: https://backstage.io/docs/conf/defining/
- 3: https://backstage.io/docs/conf/defining
- 4: https://backstage.io/docs/next/conf/defining
- 5: https://github.com/backstage/backstage/blob/aeaed8adb31c22e0204b2ac720174f9b4fa15841/packages/config-loader/src/schema/types.ts
- 6: https://backstage.io/docs/next/plugins/proxying
- 7: https://github.com/backstage/backstage/blob/master/plugins/proxy-backend/config.d.ts
- 8: https://backstage.io/api/next/modules/_backstage_plugin-gateway-backend.html
Remove @visibility frontend from discoveryAuth to prevent secret exposure.
The discoveryAuth field is a credential and must not be frontend-visible. Even though it is a nested field within a frontend-visible parent object, the explicit @visibility frontend annotation on this field makes it available in the browser config bundle, creating a secret leak vulnerability.
Per Backstage config schema best practices, use granular visibility annotations: mark secret fields with @visibility backend while exposing only non-secret metadata (like urls or discoveryUrl endpoints) with @visibility frontend.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/wso2-api-manager-backend/config.d.ts` around lines 22 - 32, The
discoveryAuth field on the wso2PlatformGateway config object is annotated as
frontend-visible but contains credentials; remove the `@visibility frontend`
from discoveryAuth and mark it as backend-only (or simply omit the frontend
visibility tag) so secret credentials are not included in the browser bundle;
update the config schema for the wso2PlatformGateway nested field
`discoveryAuth` (and verify only non-secret fields like `urls` and
`discoveryUrl` retain `@visibility frontend`) to ensure credentials are
backend-only.
Comment on lines
+117
to
+168
| router.get('/apis/:apiId/definition', async (req, res) => { | ||
| const { apiId } = req.params; | ||
| const discoveredFrom = req.query.discoveredFrom as string; | ||
|
|
||
| if (!discoveredFrom) { | ||
| res.status(400).json({ message: 'Missing discoveredFrom parameter' }); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const gw = wso2Config.selfHostedGateways.find(g => g.name === discoveredFrom); | ||
| if (!gw || !gw.discoveryUrl) { | ||
| res.status(404).json({ message: `Discovery URL not found for gateway ${discoveredFrom}` }); | ||
| return; | ||
| } | ||
|
|
||
| logger.info(`[WSO2-Backend] Fetching fallback definition for ${apiId} from ${gw.discoveryUrl}`); | ||
|
|
||
| const data = await client.getApiDefinition(`${gw.discoveryUrl}/${apiId}`, gw.discoveryAuth); | ||
| logger.info(`[WSO2-Backend] Raw gateway response keys: ${Object.keys(data).join(', ')}`); | ||
|
|
||
| const apiData = data.api || data; | ||
| logger.info(`[WSO2-Backend] apiData keys: ${Object.keys(apiData).join(', ')}`); | ||
|
|
||
| let definition = apiData.configuration?.spec?.definition || apiData.spec?.definition || apiData.definition; | ||
|
|
||
| if (!definition) { | ||
| // Return just the spec portion so operations are at the top level | ||
| const gwSpec = apiData.configuration?.spec || apiData.spec || apiData; | ||
| logger.info(`[WSO2-Backend] No raw definition found for ${apiId} — returning gateway spec (has operations: ${Array.isArray(gwSpec?.operations)})`) | ||
| res.json(gwSpec); | ||
| return; | ||
| } | ||
|
|
||
| if (definition && typeof definition === 'string' && definition.trim().startsWith('{')) { | ||
| try { | ||
| definition = JSON.parse(definition); | ||
| } catch (e) { | ||
| logger.warn(`[WSO2-Backend] Failed to parse definition string for ${apiId}`); | ||
| } | ||
| } | ||
|
|
||
| if (!definition) { | ||
| throw new Error('Definition not found in gateway response'); | ||
| } | ||
|
|
||
| res.json(definition); | ||
| } catch (e: any) { | ||
| logger.error(`[WSO2-Backend] Failed to fetch API definition from gateway: ${e.message}`); | ||
| res.status(500).json({ message: e.message }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Require auth on the new definition endpoint.
This handler never calls ensureAuthenticated, unlike the rest of the /apis/* surface, so any caller that can reach the backend can fetch internal gateway specs through the configured discoveryAuth.
Proposed fix
router.get('/apis/:apiId/definition', async (req, res) => {
+ await ensureAuthenticated(req);
const { apiId } = req.params;
const discoveredFrom = req.query.discoveredFrom as string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/wso2-api-manager-backend/src/service/router.ts` around lines 117 -
168, The GET /apis/:apiId/definition route is missing authentication; wrap or
apply the existing ensureAuthenticated middleware to this handler so only
authenticated callers can reach discoveryAuth-protected gateway specs. Update
the router.get registration for the handler in router.ts to include
ensureAuthenticated (same function used by other /apis/* routes) as middleware
before the async (req, res) callback, or call ensureAuthenticated at the start
of the handler and abort when it fails; keep the rest of the logic in the async
function (references: router.get('/apis/:apiId/definition', ensureAuthenticated,
...) or calling ensureAuthenticated within that route).
Comment on lines
+156
to
+205
| const Wso2OperationsList = ({ operations }: { operations: any[] }) => { | ||
| const getMethodColor = (method: string) => { | ||
| const m = method.toUpperCase(); | ||
| if (m === 'GET') return '#61affe'; | ||
| if (m === 'POST') return '#49cc90'; | ||
| if (m === 'PUT') return '#fca130'; | ||
| if (m === 'DELETE') return '#f93e3e'; | ||
| if (m === 'PATCH') return '#50e3c2'; | ||
| return '#9012fe'; | ||
| }; | ||
|
|
||
| return ( | ||
| <Box p={2}> | ||
| <Typography variant="subtitle2" gutterBottom style={{ fontWeight: 'bold', color: '#666', textTransform: 'uppercase', letterSpacing: '0.5px' }}> | ||
| Available Operations | ||
| </Typography> | ||
| <Box mt={2}> | ||
| {operations.map((op, idx) => ( | ||
| <Box | ||
| key={idx} | ||
| display="flex" | ||
| alignItems="center" | ||
| mb={1.5} | ||
| p={1.5} | ||
| border={1} | ||
| borderColor="rgba(0,0,0,0.08)" | ||
| borderRadius={8} | ||
| bgcolor="rgba(0,0,0,0.02)" | ||
| style={{ transition: 'all 0.2s ease' }} | ||
| > | ||
| <Box | ||
| px={1.5} | ||
| py={0.5} | ||
| mr={2} | ||
| borderRadius={4} | ||
| style={{ | ||
| backgroundColor: getMethodColor(op.method), | ||
| color: 'white', | ||
| fontWeight: 'bold', | ||
| minWidth: '80px', | ||
| textAlign: 'center', | ||
| fontSize: '0.75rem', | ||
| boxShadow: '0 2px 4px rgba(0,0,0,0.1)' | ||
| }} | ||
| > | ||
| {op.method.toUpperCase()} | ||
| </Box> | ||
| <Typography variant="body2" style={{ fontFamily: '"Roboto Mono", monospace', fontWeight: 500, color: '#333' }}> | ||
| {op.path} | ||
| </Typography> |
There was a problem hiding this comment.
Handle both operation shapes before calling toUpperCase().
This renderer assumes { method, path }, but elsewhere in this plugin operations are modeled as { verb, target }. If the gateway fallback returns that shape, op.method.toUpperCase() will throw.
Proposed fix
- {operations.map((op, idx) => (
+ {operations.map((op, idx) => {
+ const method = String(op.method ?? op.verb ?? '');
+ const path = String(op.path ?? op.target ?? '');
+ return (
<Box
key={idx}
display="flex"
@@
style={{
- backgroundColor: getMethodColor(op.method),
+ backgroundColor: getMethodColor(method || 'UNKNOWN'),
color: 'white',
@@
>
- {op.method.toUpperCase()}
+ {method.toUpperCase()}
</Box>
<Typography variant="body2" style={{ fontFamily: '"Roboto Mono", monospace', fontWeight: 500, color: '#333' }}>
- {op.path}
+ {path}
</Typography>
</Box>
- ))}
+ )})}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/wso2-api-manager/src/components/EntityWso2ApiDefinitionCard/EntityWso2ApiDefinitionCard.tsx`
around lines 156 - 205, Wso2OperationsList assumes operations have op.method and
op.path and calls op.method.toUpperCase(), which can throw when operations use
the alternate shape { verb, target }; update the component to normalise each
operation first (e.g., const methodRaw = op.method ?? op.verb ?? ''; const
method = methodRaw.toString().toUpperCase(); const path = op.path ?? op.target
?? '') and then use method for getMethodColor(method) and to render the method
label and use path for the Typography; ensure you guard against empty/null
values before calling toUpperCase so both shapes ({method,path} and
{verb,target}) are handled.
Comment on lines
+663
to
+666
| <Tab id="tab-swagger-ui" label={hasOperationsOnly ? "Operations" : "Swagger UI"} className={classes.tabRoot} /> | ||
| )} | ||
| {!hasOperationsOnly && ( | ||
| <Tab id="tab-source" label="View Source" className={classes.tabRoot} /> |
There was a problem hiding this comment.
Reset the selected tab when "View Source" disappears.
When hasOperationsOnly flips true, the Source tab is removed but activeTab can still be 1. That leaves the card blank until the user manually switches back.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/wso2-api-manager/src/components/EntityWso2ApiDefinitionCard/EntityWso2ApiDefinitionCard.tsx`
around lines 663 - 666, When hasOperationsOnly becomes true the "View Source"
Tab is removed but activeTab may still point to that index; add a useEffect in
EntityWso2ApiDefinitionCard that watches hasOperationsOnly (and activeTab) and
when hasOperationsOnly is true and activeTab === 1 call the state updater to
setActiveTab(0) so the component resets to the remaining tab; reference the
activeTab state setter and the hasOperationsOnly boolean used around the <Tab
id="tab-source" .../> and <Tab id="tab-swagger-ui" .../> to locate where to add
the effect.
Comment on lines
+390
to
+393
| // Filter API Products based on selected gateway | ||
| const filteredApiProducts = useMemo(() => { | ||
| return apiProductListState.value?.apiProducts || []; | ||
| }, [apiProductListState.value?.apiProducts]); |
There was a problem hiding this comment.
Apply the selected gateway filter to API products too.
The new selector is global, but filteredApiProducts just returns the full list. On the API Products tab the dropdown currently does nothing, so the feature looks broken there.
Proposed fix
const filteredApiProducts = useMemo(() => {
- return apiProductListState.value?.apiProducts || [];
- }, [apiProductListState.value?.apiProducts]);
+ const products = apiProductListState.value?.apiProducts || [];
+ if (selectedGateway === 'all') return products;
+ return products.filter(product =>
+ product.gateways?.some((gw: any) => gw.gatewayType === selectedGateway),
+ );
+ }, [apiProductListState.value?.apiProducts, selectedGateway]);📝 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
| // Filter API Products based on selected gateway | |
| const filteredApiProducts = useMemo(() => { | |
| return apiProductListState.value?.apiProducts || []; | |
| }, [apiProductListState.value?.apiProducts]); | |
| // Filter API Products based on selected gateway | |
| const filteredApiProducts = useMemo(() => { | |
| const products = apiProductListState.value?.apiProducts || []; | |
| if (selectedGateway === 'all') return products; | |
| return products.filter(product => | |
| product.gateways?.some((gw: any) => gw.gatewayType === selectedGateway), | |
| ); | |
| }, [apiProductListState.value?.apiProducts, selectedGateway]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx`
around lines 390 - 393, filteredApiProducts currently returns the full list and
ignores the global selected gateway; update the useMemo for filteredApiProducts
to mirror the APIs filter: depend on selectedGateway and
apiProductListState.value?.apiProducts, and return
apiProductListState.value?.apiProducts.filter(p => !selectedGateway ||
p.gatewayId === selectedGateway) (or the equivalent gateway identifier field
used by APIs) so the dropdown selection filters API Products as well; keep the
constant name filteredApiProducts and the source
apiProductListState.value?.apiProducts.
Comment on lines
+596
to
+607
| {isTimedOut && !apiListState.error && ( | ||
| <WarningPanel | ||
| title="Sync Timed Out" | ||
| message={`The catalog synchronization took longer than the configured timeout (${syncTimeout}s). We couldn't find any APIs in the catalog. Please check your WSO2 backend logs or verify your provider configuration.`} | ||
| > | ||
| <Button | ||
| variant="contained" | ||
| color="primary" | ||
| onClick={() => { | ||
| setIsTimedOut(false); | ||
| apiListState.retry(); | ||
| }} |
There was a problem hiding this comment.
"Retry Now" doesn't restart the timeout window.
The retry handler clears isTimedOut, but syncStartTime was captured only once on mount. If the catalog is still empty, the next polling cycle compares against the original start time and can time out again almost immediately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx`
around lines 596 - 607, The retry button clears isTimedOut but doesn't restart
the timeout window because syncStartTime was captured only once; update the
Button onClick in Wso2ApiManagerPage to also reset the syncStartTime (or its
ref) to the current time before calling apiListState.retry(), e.g., call the
setter used to store syncStartTime (or update the useRef value) along with
setIsTimedOut(false) so the polling timeout calculation uses the new start time
on retry.
…d and catalog modules
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend-openapi-utils/package.json (1)
18-23:⚠️ Potential issue | 🟠 MajorUpdate
exportsto point todistinstead ofsrcpathsThe
mainfield correctly points todist/index.cjs.js, and thefilesarray only includesdist, but theexportsmap still targets./src/index.tsand./src/testUtils.ts. Since Node.js prioritizesexportsovermainand the published package won't containsrcfiles, these export paths will fail to resolve at runtime. Change them to./dist/index.cjs.jsand./dist/testUtils.cjs.js(or appropriate built output paths).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend-openapi-utils/package.json` around lines 18 - 23, Update the package.json "exports" map so it points to built output in dist rather than source files: replace the "." export value ("./src/index.ts") with the compiled entry (e.g., "./dist/index.cjs.js" or the correct built file), and replace "./testUtils" ("./src/testUtils.ts") with the compiled test util (e.g., "./dist/testUtils.cjs.js"); ensure the "./package.json" export remains if needed and verify the exported paths align with "main" and the "files" array so published packages resolve correctly at runtime.
🧹 Nitpick comments (5)
plugins/wso2-api-manager/src/api/Wso2ApiManagerClient.ts (1)
91-96: 💤 Low valueSilent error recovery may mask infrastructure issues.
The backend endpoint always returns valid JSON (either an array or error object). Silently returning
[]on JSON parse failure could hide network/proxy issues or response corruption. Consider logging the error more prominently or rethrowing after logging.💡 Consider more visible error handling
try { return JSON.parse(text); } catch (e) { - console.warn('[WSO2-APIClient] Invalid JSON from /gateways:', text); - return []; + console.error('[WSO2-APIClient] Invalid JSON from /gateways. Response may be corrupted:', text.substring(0, 200)); + return []; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/wso2-api-manager/src/api/Wso2ApiManagerClient.ts` around lines 91 - 96, The try/catch around JSON.parse(text) currently swallows parse errors via console.warn and returns [], which can hide upstream/network issues; change the catch to log the full error (include the caught error object `e` and the response `text`) using a higher-severity logger (e.g., console.error or your module logger) and then rethrow or propagate the error instead of returning an empty array so callers can handle failures; specifically update the catch that currently logs '[WSO2-APIClient] Invalid JSON from /gateways:' to include `e` and `text` and replace the `return []` with either `throw e` or a new Error with context so the failure is visible to infrastructure/monitoring.plugins/wso2-api-manager/package.json (1)
18-21: 💤 Low valueRepository URL may need updating.
The repository URL still points to
github.com/backstage/backstage, but this appears to be a custom plugin with a non-Backstage namespace (@rk-apim-1). Consider updating this to the actual plugin repository if it's hosted elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/wso2-api-manager/package.json` around lines 18 - 21, The repository.url in package.json still points to https://github.com/backstage/backstage; update the repository.url field to the actual Git URL for this plugin (or remove/set to an appropriate private repo URL) and keep repository.type and repository.directory accurate; modify the repository.url value in the package.json (the "repository.url" property) so it reflects the real source for this plugin instead of the Backstage monorepo URL.bootstrap-errors.js (2)
10-13: ⚡ Quick winMissing
--moduleResolution nodemay cause module resolution failures.The
tscinvocation is missing--moduleResolution nodewhich is present inbootstrap-all.js. Without it, TypeScript may fail to resolve node_modules imports correctly depending on the tsconfig defaults.♻️ Proposed fix to align with bootstrap-all.js
- execSync('npx tsc src/index.ts --outDir dist --module commonjs --target es2019 --esModuleInterop --skipLibCheck', { + execSync('npx tsc src/index.ts --outDir dist --module commonjs --target es2019 --moduleResolution node --esModuleInterop --allowSyntheticDefaultImports --skipLibCheck', { cwd: errorsDir, stdio: 'inherit' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap-errors.js` around lines 10 - 13, The tsc invocation inside the execSync call that builds errors (the execSync(...) block calling 'npx tsc src/index.ts --outDir dist --module commonjs --target es2019 --esModuleInterop --skipLibCheck') is missing the '--moduleResolution node' flag; update the command string passed to execSync to include '--moduleResolution node' so TypeScript resolves node_modules the same way as bootstrap-all.js.
16-24: ⚡ Quick winOutput path assumption may fail if
tscnests output.Depending on the project structure and tsconfig,
tscmay output todist/src/index.jsinstead ofdist/index.js. Thebootstrap-all.jsscript (context snippet 1, lines 83-97) handles both cases. This script only checksdist/index.js.♻️ Proposed fix to handle nested output
// Rename index.js to index.cjs.js to match package.json "main" + const nestedIndex = path.join(errorsDir, 'dist', 'src', 'index.js'); const distIndex = path.join(errorsDir, 'dist', 'index.js'); const targetIndex = path.join(errorsDir, 'dist', 'index.cjs.js'); - if (fs.existsSync(distIndex)) { + if (fs.existsSync(nestedIndex)) { + fs.renameSync(nestedIndex, targetIndex); + // Clean up empty dist/src directory + try { fs.rmdirSync(path.join(errorsDir, 'dist', 'src'), { recursive: true }); } catch {} + console.log('Successfully compiled and mapped `@backstage/errors` to CommonJS!'); + } else if (fs.existsSync(distIndex)) { fs.renameSync(distIndex, targetIndex); console.log('Successfully compiled and mapped `@backstage/errors` to CommonJS!'); } else { console.error('tsc finished but dist/index.js was not found.'); + process.exit(1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap-errors.js` around lines 16 - 24, The code only checks for distIndex (errorsDir + '/dist/index.js') which fails when tsc nested output (e.g., dist/src/index.js); update the logic around distIndex/targetIndex to look for alternative compiled paths (for example errorsDir + '/dist/src/index.js') before erroring: build a small ordered list of candidate paths (dist/index.js, dist/src/index.js, etc.), find the first where fs.existsSync returns true, then call fs.renameSync on that chosen path to targetIndex and adjust the console messages to mention which source path was mapped; make these changes around the variables distIndex, targetIndex and where fs.existsSync/fs.renameSync are used.plugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx (1)
73-77: ⚡ Quick winLocal
Wso2GatewayInfoduplicates the type fromtypes.ts.This interface is already defined in
plugins/wso2-api-manager/src/api/types.ts(lines 1-5). Import the type instead of redeclaring it to maintain a single source of truth.♻️ Proposed fix
import { Wso2ApiSummary, Wso2ApiProductSummary, Wso2McpSummary, wso2AuthApiRef, wso2ApiManagerApiRef, + Wso2GatewayInfo, } from '../../api'; -interface Wso2GatewayInfo { - name: string; - displayName: string; - gatewayType: string; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx` around lines 73 - 77, Remove the duplicated local interface declaration for Wso2GatewayInfo and instead import the Wso2GatewayInfo type that’s defined in types.ts; replace usages in Wso2ApiManagerPage with the imported type so the component uses the single source of truth (Wso2GatewayInfo from types.ts) rather than the redeclared interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bootstrap-all.js`:
- Around line 177-179: Bootstrap print statements still reference the old
workspace scope '@rk-apim'; update the three console.log strings that mention
'@rk-apim/backstage-plugin-wso2-api-manager',
'@rk-apim/backstage-plugin-wso2-api-manager-backend', and
'@rk-apim/backstage-plugin-catalog-backend-module-wso2-apim' to use the new
scope '@rk-apim-1' so the printed yarn workspace commands reflect the renamed
packages.
- Around line 46-52: The current hasCompiledDist function incorrectly treats any
.js in dist as a compiled package; change it to check specifically for the
expected entry file (dist/index.cjs.js) by testing existence of
path.join(pkgDir, 'dist', 'index.cjs.js') (or equivalent direct file check)
instead of scanning for any .js; update the other duplicated detection logic
elsewhere in the file to the same explicit check so packages with unrelated JS
files don’t bypass compilation.
In `@bootstrap-errors.js`:
- Around line 25-27: The catch block that handles tsc compilation errors
currently logs the error via console.error in the catch (e) handler but does not
signal failure to the process; update the error handler (the catch (e) block
around the tsc compile call in bootstrap-errors.js) to set a non-zero exit
status (e.g., set process.exitCode = 1 or call process.exit(1)) after logging
the error so CI and callers observe the failure.
In `@fix-core-packages.js`:
- Around line 81-83: The catch blocks that currently only log errors (e.g., the
catch using "err" that calls console.error(`[FAIL] ${dir}:
${err.message.split('\n')[0]}`) after esbuild.build) must cause a non-zero
process exit so failures propagate to CI; update those catch handlers to either
rethrow the error or call process.exit(1) after logging (and ensure any async
top-level callers await/handle rethrown errors), and apply the same change to
the other catch blocks in this file (the one covering lines 86-93) so any
esbuild.build or related build failures terminate the process with a non-zero
exit code.
- Line 92: The post-run instruction string printed by console.log still
references the old package scope "@rk-apim/backstage-plugin-wso2-api-manager";
update the logged message in the console.log call to use the new scope
"@rk-apim-1/backstage-plugin-wso2-api-manager" so the user sees the correct yarn
workspace command to run after the script completes.
In `@packages/errors/package.json`:
- Around line 25-26: The package.json currently points "main" at
dist/index.cjs.js but the dist output is not produced because
scripts/bootstrap-errors.js is never run; add a lifecycle script (postinstall or
prepare) in package.json to invoke scripts/bootstrap-errors.js (e.g.,
"postinstall": "node scripts/bootstrap-errors.js" or "prepare": "node
scripts/bootstrap-errors.js") so the dist/ is built before package resolution;
ensure the chosen script name is added alongside existing scripts and that it
mirrors how bootstrap-all.js would call bootstrap-errors.js when main does not
start with "src/".
In `@plugins/catalog-backend-module-wso2-apim/package.json`:
- Line 2: The package.json "name" field and the "backstage.pluginPackage" value
are using the nonstandard scope `@rk-apim-1` instead of the repository plugin
convention; update the "name" to follow the `@backstage/plugin-`* pattern (e.g.,
`@backstage/plugin-catalog-backend-module-wso2-apim` or similar canonical plugin
name) and adjust the "backstage.pluginPackage" entry to match that new
`@backstage` scoped package name, ensuring both values are consistent with the
/plugins naming convention and any imports/exports that reference these
identifiers (search for "name" and "backstage.pluginPackage" in the package.json
and update corresponding references).
In `@plugins/wso2-api-manager-backend/package.json`:
- Line 2: The package name in package.json ("name" field) and the corresponding
entries under backstage.pluginPackages use the `@rk-apim-1/`... scope which
conflicts with the required plugin naming convention; update the "name" value to
the official Backstage plugin package format (e.g.,
"@backstage/plugin-wso2-api-manager-backend") and update any matching entries in
backstage.pluginPackages (and the other occurrences noted around lines
referenced) to the same `@backstage/plugin-`* form so the package identity and
pluginPackages entries are consistent with Backstage plugin naming rules.
In `@tsconfig.json`:
- Around line 3-10: The root tsconfig.json was narrowed to only WSO2 plugins by
changing the "include" array; revert the "include" entry in tsconfig.json to the
original broader workspace patterns (all plugins and packages) so the monorepo
type-checking is restored—restore the previous glob entries that cover all
plugins/*/src and packages/*/src (i.e., undo the specific WSO2-only list), save,
and run tsc at the repo root to confirm type-checking succeeds.
---
Outside diff comments:
In `@packages/backend-openapi-utils/package.json`:
- Around line 18-23: Update the package.json "exports" map so it points to built
output in dist rather than source files: replace the "." export value
("./src/index.ts") with the compiled entry (e.g., "./dist/index.cjs.js" or the
correct built file), and replace "./testUtils" ("./src/testUtils.ts") with the
compiled test util (e.g., "./dist/testUtils.cjs.js"); ensure the
"./package.json" export remains if needed and verify the exported paths align
with "main" and the "files" array so published packages resolve correctly at
runtime.
---
Nitpick comments:
In `@bootstrap-errors.js`:
- Around line 10-13: The tsc invocation inside the execSync call that builds
errors (the execSync(...) block calling 'npx tsc src/index.ts --outDir dist
--module commonjs --target es2019 --esModuleInterop --skipLibCheck') is missing
the '--moduleResolution node' flag; update the command string passed to execSync
to include '--moduleResolution node' so TypeScript resolves node_modules the
same way as bootstrap-all.js.
- Around line 16-24: The code only checks for distIndex (errorsDir +
'/dist/index.js') which fails when tsc nested output (e.g., dist/src/index.js);
update the logic around distIndex/targetIndex to look for alternative compiled
paths (for example errorsDir + '/dist/src/index.js') before erroring: build a
small ordered list of candidate paths (dist/index.js, dist/src/index.js, etc.),
find the first where fs.existsSync returns true, then call fs.renameSync on that
chosen path to targetIndex and adjust the console messages to mention which
source path was mapped; make these changes around the variables distIndex,
targetIndex and where fs.existsSync/fs.renameSync are used.
In `@plugins/wso2-api-manager/package.json`:
- Around line 18-21: The repository.url in package.json still points to
https://github.com/backstage/backstage; update the repository.url field to the
actual Git URL for this plugin (or remove/set to an appropriate private repo
URL) and keep repository.type and repository.directory accurate; modify the
repository.url value in the package.json (the "repository.url" property) so it
reflects the real source for this plugin instead of the Backstage monorepo URL.
In `@plugins/wso2-api-manager/src/api/Wso2ApiManagerClient.ts`:
- Around line 91-96: The try/catch around JSON.parse(text) currently swallows
parse errors via console.warn and returns [], which can hide upstream/network
issues; change the catch to log the full error (include the caught error object
`e` and the response `text`) using a higher-severity logger (e.g., console.error
or your module logger) and then rethrow or propagate the error instead of
returning an empty array so callers can handle failures; specifically update the
catch that currently logs '[WSO2-APIClient] Invalid JSON from /gateways:' to
include `e` and `text` and replace the `return []` with either `throw e` or a
new Error with context so the failure is visible to infrastructure/monitoring.
In
`@plugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsx`:
- Around line 73-77: Remove the duplicated local interface declaration for
Wso2GatewayInfo and instead import the Wso2GatewayInfo type that’s defined in
types.ts; replace usages in Wso2ApiManagerPage with the imported type so the
component uses the single source of truth (Wso2GatewayInfo from types.ts) rather
than the redeclared interface.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b2865641-085f-41be-9f26-7497e3af9e6b
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (45)
bootstrap-all.jsbootstrap-errors.jsfix-core-packages.jspackages/app/package.jsonpackages/app/src/App.tsxpackages/app/src/apis.tspackages/app/src/components/PermissionTestPage.tsxpackages/app/src/components/Root/Root.tsxpackages/app/src/components/catalog/EntityPage.tsxpackages/app/src/customPermissions.tspackages/backend-dev-utils/package.jsonpackages/backend-openapi-utils/package.jsonpackages/backend/package.jsonpackages/backend/src/index.tspackages/backend/src/plugins/customPermissionPolicy.tspackages/backend/src/plugins/customPermissions.tspackages/catalog-model/package.jsonpackages/cli-common/package.jsonpackages/cli-node/package.jsonpackages/config-loader/package.jsonpackages/config/package.jsonpackages/e2e-test-utils/package.jsonpackages/errors/package.jsonpackages/errors/src/errors/package.jsonpackages/errors/src/index.tspackages/errors/src/serialization/package.jsonpackages/integration/package.jsonpackages/opaque-internal/package.jsonpackages/release-manifests/package.jsonpackages/types/package.jsonpackages/version-bridge/package.jsonplugins/catalog-backend-module-asgardeo/README.mdplugins/catalog-backend-module-asgardeo/package.jsonplugins/catalog-backend-module-wso2-apim/README.mdplugins/catalog-backend-module-wso2-apim/package.jsonplugins/techdocs-common/package.jsonplugins/user-settings-common/package.jsonplugins/wso2-api-manager-backend/README.mdplugins/wso2-api-manager-backend/package.jsonplugins/wso2-api-manager-backend/src/service/router.tsplugins/wso2-api-manager/README.mdplugins/wso2-api-manager/package.jsonplugins/wso2-api-manager/src/api/Wso2ApiManagerClient.tsplugins/wso2-api-manager/src/components/Wso2ApiManagerPage/Wso2ApiManagerPage.tsxtsconfig.json
💤 Files with no reviewable changes (5)
- packages/app/src/components/PermissionTestPage.tsx
- packages/backend/src/plugins/customPermissions.ts
- packages/app/src/components/Root/Root.tsx
- packages/backend/src/plugins/customPermissionPolicy.ts
- packages/app/src/customPermissions.ts
✅ Files skipped from review due to trivial changes (9)
- packages/errors/src/serialization/package.json
- packages/cli-node/package.json
- packages/types/package.json
- packages/errors/src/errors/package.json
- plugins/catalog-backend-module-asgardeo/README.md
- plugins/catalog-backend-module-asgardeo/package.json
- packages/errors/src/index.ts
- packages/catalog-model/package.json
- packages/config/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/wso2-api-manager-backend/src/service/router.ts
Comment on lines
+46
to
+52
| function hasCompiledDist(pkgDir) { | ||
| const distDir = path.join(pkgDir, 'dist'); | ||
| if (!fs.existsSync(distDir)) return false; | ||
| const files = fs.readdirSync(distDir); | ||
| // Look for any .js file in dist | ||
| return files.some(f => f.endsWith('.js')); | ||
| } |
There was a problem hiding this comment.
Compiled-dist detection can incorrectly skip needed builds.
hasCompiledDist checks for any .js file, but bootstrapping needs dist/index.cjs.js. A package with unrelated dist JS gets counted as “already compiled” and bypasses compile.
Suggested fix
function hasCompiledDist(pkgDir) {
const distDir = path.join(pkgDir, 'dist');
if (!fs.existsSync(distDir)) return false;
- const files = fs.readdirSync(distDir);
- // Look for any .js file in dist
- return files.some(f => f.endsWith('.js'));
+ return (
+ fs.existsSync(path.join(distDir, 'index.cjs.js')) ||
+ fs.existsSync(path.join(distDir, 'index.js')) ||
+ fs.existsSync(path.join(distDir, 'src', 'index.js'))
+ );
}Also applies to: 154-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-all.js` around lines 46 - 52, The current hasCompiledDist function
incorrectly treats any .js in dist as a compiled package; change it to check
specifically for the expected entry file (dist/index.cjs.js) by testing
existence of path.join(pkgDir, 'dist', 'index.cjs.js') (or equivalent direct
file check) instead of scanning for any .js; update the other duplicated
detection logic elsewhere in the file to the same explicit check so packages
with unrelated JS files don’t bypass compilation.
Comment on lines
+177
to
+179
| console.log(' yarn workspace @rk-apim/backstage-plugin-wso2-api-manager build'); | ||
| console.log(' yarn workspace @rk-apim/backstage-plugin-wso2-api-manager-backend build'); | ||
| console.log(' yarn workspace @rk-apim/backstage-plugin-catalog-backend-module-wso2-apim build'); |
There was a problem hiding this comment.
Bootstrap instructions still point to the old workspace scope.
The commands at Lines [177]-[179] still use @rk-apim/...; after this rename they should use @rk-apim-1/....
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-all.js` around lines 177 - 179, Bootstrap print statements still
reference the old workspace scope '@rk-apim'; update the three console.log
strings that mention '@rk-apim/backstage-plugin-wso2-api-manager',
'@rk-apim/backstage-plugin-wso2-api-manager-backend', and
'@rk-apim/backstage-plugin-catalog-backend-module-wso2-apim' to use the new
scope '@rk-apim-1' so the printed yarn workspace commands reflect the renamed
packages.
Comment on lines
+25
to
+27
| } catch (e) { | ||
| console.error('Failed to compile @backstage/errors:', e.message); | ||
| } |
There was a problem hiding this comment.
Script exits successfully even on compilation failure.
When tsc throws an error, the catch block logs the message but doesn't set a non-zero exit code. This could cause CI pipelines to proceed despite the build failing.
🐛 Proposed fix
} catch (e) {
console.error('Failed to compile `@backstage/errors`:', e.message);
+ process.exit(1);
}📝 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
| } catch (e) { | |
| console.error('Failed to compile @backstage/errors:', e.message); | |
| } | |
| } catch (e) { | |
| console.error('Failed to compile `@backstage/errors`:', e.message); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-errors.js` around lines 25 - 27, The catch block that handles tsc
compilation errors currently logs the error via console.error in the catch (e)
handler but does not signal failure to the process; update the error handler
(the catch (e) block around the tsc compile call in bootstrap-errors.js) to set
a non-zero exit status (e.g., set process.exitCode = 1 or call process.exit(1))
after logging the error so CI and callers observe the failure.
Comment on lines
+81
to
+83
| } catch (err) { | ||
| console.error(`[FAIL] ${dir}: ${err.message.split('\n')[0]}`); | ||
| } |
There was a problem hiding this comment.
Propagate build failures to a non-zero process exit.
If esbuild.build fails, the script only logs and continues, so CI/local automation can report success despite failed bootstrapping.
Suggested fix
async function buildPkg({ dir, externals }) {
@@
try {
@@
console.log(`[OK] ${dir}`);
+ return true;
} catch (err) {
console.error(`[FAIL] ${dir}: ${err.message.split('\n')[0]}`);
+ return false;
}
}
(async () => {
console.log('Building core packages with esbuild...\n');
+ let failed = 0;
for (const pkg of PACKAGES) {
- await buildPkg(pkg);
+ const ok = await buildPkg(pkg);
+ if (!ok) failed += 1;
}
+ if (failed > 0) process.exitCode = 1;
console.log('\nDone! Now run:');Also applies to: 86-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fix-core-packages.js` around lines 81 - 83, The catch blocks that currently
only log errors (e.g., the catch using "err" that calls console.error(`[FAIL]
${dir}: ${err.message.split('\n')[0]}`) after esbuild.build) must cause a
non-zero process exit so failures propagate to CI; update those catch handlers
to either rethrow the error or call process.exit(1) after logging (and ensure
any async top-level callers await/handle rethrown errors), and apply the same
change to the other catch blocks in this file (the one covering lines 86-93) so
any esbuild.build or related build failures terminate the process with a
non-zero exit code.
| await buildPkg(pkg); | ||
| } | ||
| console.log('\nDone! Now run:'); | ||
| console.log(' yarn workspace @rk-apim/backstage-plugin-wso2-api-manager build'); |
There was a problem hiding this comment.
Post-run command still uses old package scope.
The workspace name at Line [92] still uses @rk-apim/...; this PR switched to @rk-apim-1/..., so this instruction is now misleading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fix-core-packages.js` at line 92, The post-run instruction string printed by
console.log still references the old package scope
"@rk-apim/backstage-plugin-wso2-api-manager"; update the logged message in the
console.log call to use the new scope
"@rk-apim-1/backstage-plugin-wso2-api-manager" so the user sees the correct yarn
workspace command to run after the script completes.
Comment on lines
+25
to
26
| "main": "dist/index.cjs.js", | ||
| "types": "src/index.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if bootstrap-errors.js is referenced in package.json scripts or CI config
echo "=== Checking for bootstrap-errors.js usage in package.json files ==="
rg -l "bootstrap-errors" --type json
echo ""
echo "=== Checking for bootstrap-errors.js usage in CI/workflow files ==="
rg -l "bootstrap-errors" -g "*.yml" -g "*.yaml" -g "Makefile" -g "*.sh"
echo ""
echo "=== Checking root package.json scripts ==="
cat package.json | jq '.scripts' 2>/dev/null || echo "No root package.json found"Repository: rithakith/backstage
Length of output: 3676
🏁 Script executed:
#!/bin/bash
echo "=== Searching for bootstrap-*.js files ==="
fd "bootstrap.*\.js" -t f
echo ""
echo "=== Checking if bootstrap-errors.js exists ==="
ls -la scripts/bootstrap* 2>/dev/null || echo "No bootstrap files in scripts/"
echo ""
echo "=== Examining packages/errors directory structure ==="
ls -la packages/errors/ | head -20
echo ""
echo "=== Checking packages/errors for build scripts ==="
cat packages/errors/package.json | jq '.scripts' 2>/dev/null || echo "Cannot read scripts"
echo ""
echo "=== Searching for bootstrap-all.js to understand needsBootstrap logic ==="
find . -name "bootstrap-all.js" -o -name "*bootstrap*.js" | head -20Repository: rithakith/backstage
Length of output: 1474
🏁 Script executed:
#!/bin/bash
echo "=== Reading bootstrap-all.js to understand needsBootstrap logic ==="
cat -n bootstrap-all.js | head -80
echo ""
echo "=== Reading bootstrap-errors.js ==="
cat -n bootstrap-errors.js
echo ""
echo "=== Checking for bootstrap invocation in package.json postinstall or prepare ==="
cat package.json | jq '.scripts.postinstall, .scripts.prepare' 2>/dev/nullRepository: rithakith/backstage
Length of output: 4611
🏁 Script executed:
#!/bin/bash
echo "=== Checking if dist/ is tracked in git for packages/errors ==="
cd packages/errors && git ls-files | grep "^dist/" || echo "No dist files tracked"
cd - > /dev/null
echo ""
echo "=== Checking .gitignore for dist pattern ==="
cat .gitignore | grep -E "dist|^dist$" || echo "No dist in root .gitignore"
echo ""
echo "=== Checking if packages/errors has dist directory currently ==="
ls -la packages/errors/dist 2>/dev/null || echo "No dist directory in packages/errors"
echo ""
echo "=== Checking if bootstrap scripts are called in any scripts ==="
rg "bootstrap-all|bootstrap-errors" --type sh --type js -l 2>/dev/null || echo "No references found"
echo ""
echo "=== Checking if build process calls these bootstrap scripts ==="
grep -r "bootstrap" package.json scripts/ 2>/dev/null | head -20 || echo "No matches"Repository: rithakith/backstage
Length of output: 507
Ensure bootstrap-errors.js is invoked before package resolution.
The main field now points to dist/index.cjs.js, which does not currently exist (dist/ is gitignored). The package will fail to resolve without this file. While bootstrap-errors.js exists to compile this package, it is never invoked by the build pipeline—bootstrap-all.js skips it since main no longer starts with "src/", and neither postinstall nor prepare scripts call it.
Add a postinstall or prepack invocation of bootstrap-errors.js to ensure the dist directory is compiled before the package is consumed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/errors/package.json` around lines 25 - 26, The package.json
currently points "main" at dist/index.cjs.js but the dist output is not produced
because scripts/bootstrap-errors.js is never run; add a lifecycle script
(postinstall or prepare) in package.json to invoke scripts/bootstrap-errors.js
(e.g., "postinstall": "node scripts/bootstrap-errors.js" or "prepare": "node
scripts/bootstrap-errors.js") so the dist/ is built before package resolution;
ensure the chosen script name is added alongside existing scripts and that it
mirrors how bootstrap-all.js would call bootstrap-errors.js when main does not
start with "src/".
| "@types/node": "^18.0.0", | ||
| "@types/uuid": "^9.0.0" | ||
| } | ||
| "name": "@rk-apim-1/backstage-plugin-catalog-backend-module-wso2-apim", |
There was a problem hiding this comment.
Plugin package naming does not match repo convention.
Both name and backstage.pluginPackage use @rk-apim-1/..., but this path is under /plugins and the configured convention expects @backstage/plugin-*.
As per coding guidelines, plugins/*/package.json: Organize plugin code in packages prefixed with @backstage/plugin-* in /plugins directory.
Also applies to: 7-7
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/catalog-backend-module-wso2-apim/package.json` at line 2, The
package.json "name" field and the "backstage.pluginPackage" value are using the
nonstandard scope `@rk-apim-1` instead of the repository plugin convention; update
the "name" to follow the `@backstage/plugin-`* pattern (e.g.,
`@backstage/plugin-catalog-backend-module-wso2-apim` or similar canonical plugin
name) and adjust the "backstage.pluginPackage" entry to match that new
`@backstage` scoped package name, ensuring both values are consistent with the
/plugins naming convention and any imports/exports that reference these
identifiers (search for "name" and "backstage.pluginPackage" in the package.json
and update corresponding references).
| { | ||
| "name": "@rk-apim/backstage-plugin-wso2-api-manager-backend", | ||
| "version": "1.0.3", | ||
| "name": "@rk-apim-1/backstage-plugin-wso2-api-manager-backend", |
There was a problem hiding this comment.
Plugin package scope conflicts with plugin naming convention.
The plugin package identity and backstage.pluginPackages entries are under @rk-apim-1/... rather than the required @backstage/plugin-* form for /plugins packages.
As per coding guidelines, plugins/*/package.json: Organize plugin code in packages prefixed with @backstage/plugin-* in /plugins directory.
Also applies to: 8-9
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/wso2-api-manager-backend/package.json` at line 2, The package name in
package.json ("name" field) and the corresponding entries under
backstage.pluginPackages use the `@rk-apim-1/`... scope which conflicts with the
required plugin naming convention; update the "name" value to the official
Backstage plugin package format (e.g.,
"@backstage/plugin-wso2-api-manager-backend") and update any matching entries in
backstage.pluginPackages (and the other occurrences noted around lines
referenced) to the same `@backstage/plugin-`* form so the package identity and
pluginPackages entries are consistent with Backstage plugin naming rules.
Comment on lines
3
to
10
| "include": [ | ||
| "packages/*/src", | ||
| "packages/*/config.d.ts", | ||
| "plugins/*/src", | ||
| "plugins/*/config.d.ts", | ||
| "plugins/*/dev", | ||
| "plugins/*/migrations" | ||
| "plugins/wso2-api-manager/src", | ||
| "plugins/wso2-api-manager-backend/src", | ||
| "plugins/catalog-backend-module-wso2-apim/src", | ||
| "plugins/wso2-api-manager/config.d.ts", | ||
| "plugins/wso2-api-manager-backend/config.d.ts", | ||
| "plugins/catalog-backend-module-wso2-apim/config.d.ts" | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are other plugins/packages that would be excluded by this change
echo "=== Other plugins that are now excluded from tsconfig ==="
fd -t d -d 1 . plugins --exclude wso2-api-manager --exclude wso2-api-manager-backend --exclude catalog-backend-module-wso2-apim | head -20
echo ""
echo "=== Other packages that are now excluded from tsconfig ==="
fd -t d -d 1 . packages | head -20Repository: rithakith/backstage
Length of output: 1385
🏁 Script executed:
#!/bin/bash
# Check the git history to see what the original include pattern was
echo "=== Original tsconfig.json include pattern ==="
git show HEAD~1:tsconfig.json 2>/dev/null | grep -A 20 '"include"' || echo "Could not retrieve previous version"
echo ""
echo "=== Check if other packages/plugins have their own tsconfig.json ==="
fd -t f -name tsconfig.json . | grep -v node_modules | head -20Repository: rithakith/backstage
Length of output: 465
🏁 Script executed:
#!/bin/bash
# Check if this is a workspace configuration and the structure
echo "=== Root tsconfig.json full content ==="
cat -n tsconfig.json | head -30
echo ""
echo "=== Check for workspace or monorepo configuration ==="
if [ -f package.json ]; then
echo "=== package.json workspace/workspaces field ==="
jq -r '.workspaces[]? // .workspace? // "no workspace field"' package.json 2>/dev/null || cat package.json | grep -A 5 -B 5 "workspace"
fiRepository: rithakith/backstage
Length of output: 861
Root tsconfig.json narrowed to only WSO2 plugins - violates coding guidelines and breaks monorepo type-checking.
The include array now only covers 3 WSO2-related plugins, excluding 20+ other plugins and 20+ packages defined in the monorepo workspaces. This breaks TypeScript type-checking for the entire repository when running tsc from the root.
Per the coding guidelines: "Never update ESLint, Prettier, or TypeScript configuration files unless specifically requested." This change was not explicitly requested and violates that directive.
Restore the original broader include patterns to maintain type-checking across the full monorepo workspace.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` around lines 3 - 10, The root tsconfig.json was narrowed to
only WSO2 plugins by changing the "include" array; revert the "include" entry in
tsconfig.json to the original broader workspace patterns (all plugins and
packages) so the monorepo type-checking is restored—restore the previous glob
entries that cover all plugins/*/src and packages/*/src (i.e., undo the specific
WSO2-only list), save, and run tsc at the repo root to confirm type-checking
succeeds.
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
1 participant
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 by CodeRabbit
New Features
Improvements
Removed