[Devportal] Add integration tests for devportal#2529
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds Developer Portal integration testing infrastructure, REST API and Cypress test suites, and CI wiring. It also updates API type handling, key manager settings, organization deletion, actor resolution, webhook delivery, and OAuth token error handling. ChangesIntegration Test Infrastructure and Suites
Backend Behavior Fixes and Key Manager Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (6)
portals/developer-portal/it/backend/apis/rest-apis.spec.js (1)
51-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude
idin update PUT bodies for robustness.This update test omits
idfrom theapiMetadataJSON. As documented in the label-updateNOTEat lines 233–240,apiDao.update()recomputes the handle fromname+versionwhenidis absent, silently changing the resource's identifier. The label tests correctly includeid: api.idbecause they re-GET by the original id. While this test doesn't re-GET, addingidwould prevent a subtle break if a follow-up GET is added later.The same pattern affects
graphql-apis.spec.js(lines 63–78),soap-apis.spec.js(lines 39–54),websocket-apis.spec.js(lines 39–54), andwebsub-apis.spec.js(lines 38–53).♻️ Proposed fix
.field('apiMetadata', JSON.stringify({ + id: api.id, name: 'Updated Name', version: 'v1.0', type: 'REST', status: 'PUBLISHED', endPoints: { productionURL: 'https://updated.example.invalid', sandboxURL: 'https://updated-sandbox.example.invalid' }, }))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/backend/apis/rest-apis.spec.js` around lines 51 - 66, The REST API update test’s apiMetadata payload is missing the existing id, which can cause apiDao.update() to recompute the handle from name and version instead of preserving the original resource identity. Update the putMultipart() setup in this test to include api.id in the apiMetadata JSON, and apply the same fix to the analogous update tests in graphql-apis.spec.js, soap-apis.spec.js, websocket-apis.spec.js, and websub-apis.spec.js so the update flow stays robust if later assertions re-fetch by id.portals/developer-portal/it/backend/api-workflows/api-workflows.spec.js (1)
34-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
createViewhelper across test files.The same
createViewfunction appears verbatim ingenerate-prompt.spec.js(lines 28-39) and in a slightly different form inview-label-visibility.spec.js(lines 35-42). Consider extracting it into the sharedsupport/fixtures.jsmodule to reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/backend/api-workflows/api-workflows.spec.js` around lines 34 - 45, The createView helper is duplicated across multiple spec files, so move the shared view-seeding logic into the common support/fixtures.js module and have api-workflows.spec.js (and the other specs using createView) import it instead. Keep the behavior in the createView helper the same, including label creation, POST /views seeding, and the non-201 error handling, so the tests continue to use one maintained implementation..github/workflows/devportal-integration-test.yml (1)
28-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout steps.Both
actions/checkout@v4steps persist the GitHub token in.git/configby default. Since this workflow only needs the source tree (no git push, tag, or history operations), disabling credential persistence reduces the token's exposure surface on the runner.🔒️ Proposed fix
- name: Checkout code uses: actions/checkout@v4 + with: + persist-credentials: falseApply the same change to the
ui-testjob's checkout step (line 63-64).Also applies to: 63-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/devportal-integration-test.yml around lines 28 - 29, The checkout steps in the workflow currently persist the GitHub token by default, which is unnecessary for this read-only job. Update both actions/checkout@v4 uses in the devportal-integration-test workflow by setting persist-credentials to false on each Checkout code step, including the one in the ui-test job, so the token is not written to .git/config.Source: Linters/SAST tools
portals/developer-portal/it/docker-compose.test.postgres.yaml (1)
114-146: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider skipping
better-sqlite3compilation in the PostgreSQL variant.The comment on line 143-145 acknowledges that
better-sqlite3is compiled from source even though the postgres variant never uses it, because it's an unconditionaldevDependency. Movingbetter-sqlite3tooptionalDependenciesinpackage.jsonand usingnpm install --omit=optionalfor this compose file would eliminate theapk add python3 make g++step and speed up container startup.♻️ Proposed changes
In
portals/developer-portal/it/backend/package.json:"devDependencies": { "jest": "^29.7.0", "jest-junit": "^16.0.0", "supertest": "^7.0.0", "cookiejar": "^2.1.4", "nock": "^13.5.5", - "better-sqlite3": "^11.3.0", "pg": "^8.14.0" + }, + "optionalDependencies": { + "better-sqlite3": "^11.3.0" }In
docker-compose.test.postgres.yaml:- command: ["sh", "-c", "apk add --no-cache python3 make g++ >/dev/null && npm install --no-audit --no-fund && npm test"] + command: ["sh", "-c", "npm install --no-audit --no-fund --omit=optional && npm test"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/docker-compose.test.postgres.yaml` around lines 114 - 146, The backend-tests service still installs build tools just to compile better-sqlite3, even though the PostgreSQL path never uses it. Move better-sqlite3 out of the unconditional devDependencies in backend/package.json (or make it optional) and update the backend-tests command in docker-compose.test.postgres.yaml to install dependencies without optional/native sqlite compilation, so the container can skip apk add python3 make g++ and start faster.portals/developer-portal/it/backend/support/envelopeCrypto.js (1)
27-39: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAnnotate RSA-OAEP usage with
// TODO(pqc): migrateper coding guidelines.This file introduces RSA-OAEP, a quantum-vulnerable algorithm, in new JavaScript code under
portals/developer-portal. The coding guidelines require that quantum-vulnerable cryptography be annotated with// TODO(pqc): migrateand tracked — "do not leave undocumented vulnerable crypto." While this test helper must mirror the productionencryptToSubscriberimplementation and cannot migrate independently, the annotation signals the dependency and ensures it's tracked for the eventual PQC transition.🔒️ Proposed annotation
// Test-side mirror of src/services/webhooks/envelopeCrypto.js's decryptFromEnvelope. // Can't require the app source directly: the backend-tests container only has // `it/backend` mounted (docker-compose.test*.yaml), not the rest of the repo. Kept in // lockstep with the app's encryptToSubscriber — RSA-OAEP(SHA-256)-wrapped AES-256-GCM // key, base64-encoded fields. +// TODO(pqc): migrate — mirrors src/services/webhooks/envelopeCrypto.js; migrate +// in lockstep when production encryption switches to ML-KEM-based key wrapping. const crypto = require('crypto');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/backend/support/envelopeCrypto.js` around lines 27 - 39, The RSA-OAEP usage in decryptFromEnvelope is missing the required PQC migration annotation. Add a // TODO(pqc): migrate comment directly above the crypto.privateDecrypt call (and keep the existing encryptToSubscriber/decryptFromEnvelope behavior unchanged) so the quantum-vulnerable dependency is explicitly tracked per the coding guidelines.Source: Coding guidelines
portals/developer-portal/it/backend/support/fixtures.js (1)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
crypto.randomUUID()overDate.now()+Math.random()foruniqueHandle.Jest runs spec files in parallel workers. While collision probability is low,
Date.now()+Math.random()is not guaranteed unique across workers starting at the same millisecond.crypto.randomUUID()provides 122 bits of entropy and eliminates flaky test failures from handle collisions entirely.♻️ Proposed refactor
+const crypto = require('crypto'); + function uniqueHandle(prefix) { - return `${prefix}-${Date.now()}-${Math.floor(Math.random() * 1e6)}`; + return `${prefix}-${crypto.randomUUID()}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/backend/support/fixtures.js` around lines 28 - 30, The uniqueHandle helper currently builds IDs with Date.now() and Math.random(), which can still collide across parallel Jest workers. Update uniqueHandle to generate the suffix with crypto.randomUUID() instead, keeping the existing prefix format intact so all call sites in fixtures.js continue to work without change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/developer-portal/it/backend/ai-discovery/llms-txt.spec.js`:
- Around line 63-75: The save assertion in the llms-txt spec is outside the
cleanup path, so a failed PUT can leave aiEnabled disabled for later tests.
Update the llms-txt test around the configPath/save flow so the put call and the
status check are both covered by the same try/finally, ensuring the restore
request always runs even if client.page('publisher').put fails or returns a
non-200 response.
In `@portals/developer-portal/it/backend/api-keys/webhook-events.spec.js`:
- Around line 175-179: The RSA key generation in the webhook event test uses
quantum-vulnerable crypto and needs the required PQC migration marker. Add the
coding-guidelines annotation directly above the crypto.generateKeyPairSync call
in webhooks-events.spec.js, keeping the existing test behavior intact while
clearly marking this RSA usage with the TODO(pqc): migrate note.
In `@portals/developer-portal/it/backend/api-workflows/api-workflows.spec.js`:
- Around line 120-135: The test name in api-workflows.spec.js does not match
what the body verifies: the current `respects agent_visibility` case only
exercises `GET /views/${view.id}/api-workflows` and confirms the HIDDEN workflow
is still present in the publisher listing. Either rename this test to describe
publisher-side visibility, or extend the existing `it('respects
agent_visibility...')` case to also call the agent-facing discovery endpoint
(for example via the same client helper used for `llms.txt` or equivalent) and
assert that `hiddenId` is excluded there while `visibleId` remains available.
In `@portals/developer-portal/it/backend/apis/rest-apis.spec.js`:
- Around line 156-159: The unauthenticated-session test in rest-apis.spec.js is
too permissive because it accepts both 401 and 403. Tighten the assertion in the
`rejects requests without an authenticated session` case to require `401` only,
so it matches the auth contract enforced by the `/apis` endpoint and the
surrounding auth middleware behavior.
In `@portals/developer-portal/it/backend/key-managers/token-generation.spec.js`:
- Around line 127-135: The token generation spec is too permissive because it
allows 500 as an acceptable response for invalid credentials. Update the
assertion in token-generation.spec.js within the "rejects generation with an
incorrect consumer secret" test so it only accepts the expected client-error
status from the generate-token flow, and do not treat 500 as valid. Use the
existing setupAppWithKeyMapping and client.as('developer').post call as the
anchor for the change.
In `@portals/developer-portal/it/backend/subscriptions/webhook-events.spec.js`:
- Around line 167-171: The RSA key generation in the webhook events spec needs
the required PQC migration marker. Update the crypto setup in the relevant test
block that calls crypto.generateKeyPairSync in the webhook-events spec to
include the guideline-mandated comment, matching the pattern used in
api-keys/webhook-events.spec.js and keeping the annotation directly above the
quantum-vulnerable RSA usage.
- Around line 79-85: The webhook payload assertion is using the wrong field name
for the plan data, so update the `webhook-events.spec.js` expectation around
`received.body.data` to use `subscription_plan.plan_name` instead of
`subscription_plan.name`. Make sure the Developer Portal webhook payload matches
what `platform-api` unmarshals, and verify the relevant subscription webhook
builder/serializer and the `webhook-events.spec.js` test stay aligned.
In `@portals/developer-portal/it/backend/support/global-setup.js`:
- Around line 32-48: The health-check loop in the global setup can hang forever
because `retry()` is only reached from `client.get(...)` error or non-200
responses, so add a per-request timeout to the request made in the `attempt`
function. Update the polling logic in `global-setup.js` to ensure each
`client.get(`${BASE_URL}/health`, ...)` call fails or retries when no HTTP
response arrives, and make sure the timeout path still uses the existing
`retry()` deadline handling and `TIMEOUT_MS` rejection message.
In `@portals/developer-portal/it/backend/support/webhook-sink.js`:
- Line 62: The webhook sink’s req.on('end') parsing path currently calls
JSON.parse on rawBody without protection, so non-JSON payloads can throw and
crash the Jest worker. Update the logic in webhook-sink.js around the body
assignment to guard the parse in a try/catch or validate the content before
parsing, and surface a clear failure instead of letting the exception escape
from the callback.
In
`@portals/developer-portal/it/backend/webhook-subscribers/webhook-delivery.spec.js`:
- Around line 131-132: The test is selecting the first `application.created`
event by org and type only, which can pick up an earlier event from this suite
instead of the one created in the current flow. Update the `findEvents` call in
`webhook-delivery.spec.js` to also filter by the `event.uuid` captured from
`waitForEvent`, so `finalEvent` always refers to the matching event before
asserting its `status`. Use the existing `event` variable and the
`findEvents`/`waitForEvent` helpers to locate the correct event reliably.
In `@portals/developer-portal/it/docker-compose.test.postgres.yaml`:
- Around line 44-59: The platform-api healthcheck is using curl, which is not
present in the ghcr.io/wso2/api-platform/platform-api:0.11.0 image and keeps the
service unhealthy. Update the healthcheck in docker-compose.test.postgres.yaml
to use a probe command that is bundled with the platform-api image, or modify
the image so the command exists. Keep the fix localized to the platform-api
service definition and its healthcheck settings.
In `@portals/developer-portal/it/Makefile`:
- Around line 46-48: The test targets in the Makefile stop before cleanup
because the `docker compose up` command in `test` (and the other affected
targets) fails the recipe, preventing `_collect-exit` from running. Update the
`test` target and the related test targets to prefix the `docker compose up`
invocation with `-` so Make continues to `$(MAKE) _collect-exit` even when
Cypress exits non-zero. Keep the cleanup flow centered around `_collect-exit`,
and use the existing target names like `test` and `_collect-exit` to apply the
same fix consistently across all four targets.
In `@portals/developer-portal/src/dao/auditDao.js`:
- Around line 30-38: The `record` function in `auditDao` is calling
`sequelize.withWriteLock`, but the shared Sequelize instance does not provide
that helper, so this will fail at runtime. Fix it by either adding the
`withWriteLock` helper to the shared sequelize setup used by
`sequelizeConfig.js`, or by updating `record` to use `Audit.create()` directly
without the nonexistent wrapper. Keep the change localized around `record` and
the shared sequelize initialization so the call chain remains consistent.
---
Nitpick comments:
In @.github/workflows/devportal-integration-test.yml:
- Around line 28-29: The checkout steps in the workflow currently persist the
GitHub token by default, which is unnecessary for this read-only job. Update
both actions/checkout@v4 uses in the devportal-integration-test workflow by
setting persist-credentials to false on each Checkout code step, including the
one in the ui-test job, so the token is not written to .git/config.
In `@portals/developer-portal/it/backend/api-workflows/api-workflows.spec.js`:
- Around line 34-45: The createView helper is duplicated across multiple spec
files, so move the shared view-seeding logic into the common support/fixtures.js
module and have api-workflows.spec.js (and the other specs using createView)
import it instead. Keep the behavior in the createView helper the same,
including label creation, POST /views seeding, and the non-201 error handling,
so the tests continue to use one maintained implementation.
In `@portals/developer-portal/it/backend/apis/rest-apis.spec.js`:
- Around line 51-66: The REST API update test’s apiMetadata payload is missing
the existing id, which can cause apiDao.update() to recompute the handle from
name and version instead of preserving the original resource identity. Update
the putMultipart() setup in this test to include api.id in the apiMetadata JSON,
and apply the same fix to the analogous update tests in graphql-apis.spec.js,
soap-apis.spec.js, websocket-apis.spec.js, and websub-apis.spec.js so the update
flow stays robust if later assertions re-fetch by id.
In `@portals/developer-portal/it/backend/support/envelopeCrypto.js`:
- Around line 27-39: The RSA-OAEP usage in decryptFromEnvelope is missing the
required PQC migration annotation. Add a // TODO(pqc): migrate comment directly
above the crypto.privateDecrypt call (and keep the existing
encryptToSubscriber/decryptFromEnvelope behavior unchanged) so the
quantum-vulnerable dependency is explicitly tracked per the coding guidelines.
In `@portals/developer-portal/it/backend/support/fixtures.js`:
- Around line 28-30: The uniqueHandle helper currently builds IDs with
Date.now() and Math.random(), which can still collide across parallel Jest
workers. Update uniqueHandle to generate the suffix with crypto.randomUUID()
instead, keeping the existing prefix format intact so all call sites in
fixtures.js continue to work without change.
In `@portals/developer-portal/it/docker-compose.test.postgres.yaml`:
- Around line 114-146: The backend-tests service still installs build tools just
to compile better-sqlite3, even though the PostgreSQL path never uses it. Move
better-sqlite3 out of the unconditional devDependencies in backend/package.json
(or make it optional) and update the backend-tests command in
docker-compose.test.postgres.yaml to install dependencies without
optional/native sqlite compilation, so the container can skip apk add python3
make g++ and start faster.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0aec9a30-f43f-4bda-9430-c3e466d85a22
⛔ Files ignored due to path filters (2)
portals/developer-portal/it/backend/package-lock.jsonis excluded by!**/package-lock.jsonportals/developer-portal/it/ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (81)
.github/workflows/devportal-integration-test.ymlportals/developer-portal/.gitignoreportals/developer-portal/README.mdportals/developer-portal/database/queries/search-apis.postgres.sqlportals/developer-portal/docs/devportal-openapi-spec-v0.9.yamlportals/developer-portal/it/Makefileportals/developer-portal/it/README.mdportals/developer-portal/it/backend/ai-discovery/llms-txt.spec.jsportals/developer-portal/it/backend/api-keys/api-keys.spec.jsportals/developer-portal/it/backend/api-keys/webhook-events.spec.jsportals/developer-portal/it/backend/api-workflows/api-workflows.spec.jsportals/developer-portal/it/backend/api-workflows/generate-prompt.spec.jsportals/developer-portal/it/backend/apis/artifact-zip-apis.spec.jsportals/developer-portal/it/backend/apis/graphql-apis.spec.jsportals/developer-portal/it/backend/apis/rest-apis.spec.jsportals/developer-portal/it/backend/apis/soap-apis.spec.jsportals/developer-portal/it/backend/apis/websocket-apis.spec.jsportals/developer-portal/it/backend/apis/websub-apis.spec.jsportals/developer-portal/it/backend/applications/applications.spec.jsportals/developer-portal/it/backend/applications/webhook-events.spec.jsportals/developer-portal/it/backend/auth/file-based-login.spec.jsportals/developer-portal/it/backend/jest.config.jsportals/developer-portal/it/backend/key-managers/key-managers.spec.jsportals/developer-portal/it/backend/key-managers/token-generation.spec.jsportals/developer-portal/it/backend/mcp-servers/mcp-servers.spec.jsportals/developer-portal/it/backend/organizations/organizations.spec.jsportals/developer-portal/it/backend/package.jsonportals/developer-portal/it/backend/subscriptions/subscriptions.spec.jsportals/developer-portal/it/backend/subscriptions/webhook-events.spec.jsportals/developer-portal/it/backend/support/client.jsportals/developer-portal/it/backend/support/db.jsportals/developer-portal/it/backend/support/envelopeCrypto.jsportals/developer-portal/it/backend/support/fixtures.jsportals/developer-portal/it/backend/support/global-setup.jsportals/developer-portal/it/backend/support/global-teardown.jsportals/developer-portal/it/backend/support/wait-for.jsportals/developer-portal/it/backend/support/webhook-sink.jsportals/developer-portal/it/backend/support/zipBuilder.jsportals/developer-portal/it/backend/views-and-labels/labels.spec.jsportals/developer-portal/it/backend/views-and-labels/view-label-visibility.spec.jsportals/developer-portal/it/backend/views-and-labels/views.spec.jsportals/developer-portal/it/backend/webhook-subscribers/webhook-delivery.spec.jsportals/developer-portal/it/backend/webhook-subscribers/webhook-subscribers.spec.jsportals/developer-portal/it/configs/config-platform-api-it.tomlportals/developer-portal/it/docker-compose.test.postgres.yamlportals/developer-portal/it/docker-compose.test.yamlportals/developer-portal/it/ui/cypress.config.jsportals/developer-portal/it/ui/cypress/e2e/000-smoke/001-smoke.cy.jsportals/developer-portal/it/ui/cypress/e2e/000-smoke/002-api-listing.cy.jsportals/developer-portal/it/ui/cypress/e2e/001-basic/001-portal-access.cy.jsportals/developer-portal/it/ui/cypress/e2e/auth/asgardeo-login.cy.jsportals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.jsportals/developer-portal/it/ui/cypress/e2e/auth/thunderid-login.cy.jsportals/developer-portal/it/ui/cypress/e2e/design-mode/api-content-override.cy.jsportals/developer-portal/it/ui/cypress/e2e/design-mode/org-theme-override.cy.jsportals/developer-portal/it/ui/cypress/e2e/graphql-apis/tryout.cy.jsportals/developer-portal/it/ui/cypress/e2e/mcp-servers/tryout.cy.jsportals/developer-portal/it/ui/cypress/e2e/rest-apis/documents.cy.jsportals/developer-portal/it/ui/cypress/e2e/rest-apis/theming.cy.jsportals/developer-portal/it/ui/cypress/e2e/rest-apis/tryout.cy.jsportals/developer-portal/it/ui/cypress/e2e/search/api-search.cy.jsportals/developer-portal/it/ui/cypress/e2e/websocket-apis/tryout.cy.jsportals/developer-portal/it/ui/cypress/fixtures/org.jsonportals/developer-portal/it/ui/cypress/fixtures/users.jsonportals/developer-portal/it/ui/cypress/support/commands.jsportals/developer-portal/it/ui/cypress/support/commands/applications.jsportals/developer-portal/it/ui/cypress/support/commands/auth.jsportals/developer-portal/it/ui/cypress/support/e2e.jsportals/developer-portal/it/ui/package.jsonportals/developer-portal/scripts/drift_check.jsportals/developer-portal/src/controllers/apiContentController.jsportals/developer-portal/src/controllers/devportalController.jsportals/developer-portal/src/controllers/viewConfigureController.jsportals/developer-portal/src/dao/apiDao.jsportals/developer-portal/src/dao/auditDao.jsportals/developer-portal/src/dao/organizationDao.jsportals/developer-portal/src/dao/viewDao.jsportals/developer-portal/src/pages/settings/page.hbsportals/developer-portal/src/services/apiMetadataService.jsportals/developer-portal/src/services/mcpServerService.jsportals/developer-portal/src/services/webhooks/eventPublisher.js
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
portals/developer-portal/it/rest-api/support/envelopeCrypto.js (1)
27-39: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAnalyze RSA usage with
// TODO(pqc): migrateper coding guidelines.This test helper introduces RSA-OAEP (a quantum-vulnerable algorithm) in a JavaScript file under
portals/developer-portal. While this code must mirror the production envelope crypto for interop and cannot be changed independently, the coding guidelines require that remaining quantum-vulnerable cryptography be annotated with// TODO(pqc): migrate.📝 Proposed annotation
+// TODO(pqc): migrate — RSA-OAEP key transport is quantum-vulnerable; mirrors +// src/services/webhooks/envelopeCrypto.js and must migrate in lockstep. function decryptFromEnvelope(privateKeyPem, envelope) { const aesKey = crypto.privateDecrypt(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/rest-api/support/envelopeCrypto.js` around lines 27 - 39, The decryptFromEnvelope helper uses RSA-OAEP via crypto.privateDecrypt and must be annotated per the PQC coding guideline. Add a clear // TODO(pqc): migrate comment in decryptFromEnvelope near the RSA-based key unwrap so the remaining quantum-vulnerable crypto is flagged, while leaving the existing envelope interoperability logic unchanged.Source: Coding guidelines
docs/rest-apis/devportal/apis.md (1)
62-89: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale
"type": "REST"examples don't match the new canonical constant.The 201 (line 71) and 200 (line 619) response examples still show
"type": "REST", but per the newly documented type-resolution behavior (line 318) the stored/returned value should be"RestApi".📝 Proposed fix
- "type": "REST", + "type": "RestApi",(apply at both the create and update response examples)
Also applies to: 605-638
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/rest-apis/devportal/apis.md` around lines 62 - 89, Update the API response examples in the devportal docs so the documented type value matches the new canonical constant: replace the stale "type": "REST" examples in the create/update response sections with "RestApi". Make the change in both example blocks associated with the API response payloads, keeping the rest of the JSON structure unchanged and consistent with the type-resolution behavior already documented.docs/rest-apis/devportal/mcp-servers.md (1)
61-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale
"type": "MCP"/ "Always typedMCP" text should beMcp.The create/update response examples (lines 70, 614) and the accompanying prose ("Always typed
MCP" at lines 156 and 697) still use the pre-canonicalization keywordMCP, but per the type-resolution mapping (line 317) the stored/returned value isMcp.📝 Proposed fix
- "type": "MCP", + "type": "Mcp",-Created MCP server metadata payload returned by the service. Always typed `MCP`. +Created MCP server metadata payload returned by the service. Always typed `Mcp`.Also applies to: 152-156, 602-629, 693-697
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/rest-apis/devportal/mcp-servers.md` around lines 61 - 88, Update the MCP server docs to use the canonical type value everywhere: change the create/update response examples in the MCP server examples and the surrounding prose from `MCP` to `Mcp`. Keep the wording aligned with the type-resolution mapping referenced in the document, and make the same correction in the sections that describe the stored/returned type and the “Always typed” text so the examples and narrative match consistently.
♻️ Duplicate comments (2)
portals/developer-portal/it/docker-compose.test.postgres.yaml (1)
52-57: 🩺 Stability & Availability | 🔴 Critical
curlis not available in the platform-api image — healthcheck will never pass.This was previously flagged and remains unresolved. The
platform-api:0.11.0image does not includecurl, so this healthcheck will perpetually fail. Sincedevportalandrest-api-testsboth depend onplatform-apibeingservice_healthy, the entire test suite is blocked.Replace with a command bundled in the image (e.g.,
wget,node, or a Go binary health probe), or installcurlin the image.🔧 Proposed fix
healthcheck: - test: ["CMD", "curl", "-fk", "https://localhost:9243/health"] + test: ["CMD-SHELL", "node -e \"require('https').get('https://localhost:9243/health', {rejectUnauthorized: false}, r => process.exit(r.statusCode === 200 ? 0 : 1)).on('error', () => process.exit(1))\""] interval: 5s timeout: 5s retries: 20 start_period: 10s🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/docker-compose.test.postgres.yaml` around lines 52 - 57, The healthcheck for the platform-api service is using curl, but that binary is not present in the platform-api:0.11.0 image, so the service can never become healthy. Update the healthcheck in the docker-compose test setup to use a command that is already available inside the platform-api image (or add the missing tool to the image), keeping the existing healthcheck structure and service dependency flow for devportal and rest-api-tests intact.docs/rest-apis/devportal/mcp-servers.md (1)
317-317: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSame "returned unchanged" inaccuracy for
MCP.Duplicate of the issue in
apis.md—MCPis remapped toMcpperconstants.js, so it should not be grouped with the values "returned unchanged."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/rest-apis/devportal/mcp-servers.md` at line 317, Update the `type` description in the MCP servers docs to match the actual `API_TYPE` mapping in `constants.js`: `REST` maps to `RestApi`, `WEBSUB` maps to `WebSubApi`, and `MCP` must be listed as remapped to `Mcp` rather than grouped with values returned unchanged. Keep the wording aligned with the other API type docs and use the `type` row in `ApiMetadataMultipartBody` as the reference point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/rest-apis/devportal/apis.md`:
- Line 318: The API type description is inaccurate for MCP in the docs table.
Update the wording around the stored/returned type in the
ApiMetadataMultipartBody section to match resolveApiType and API_TYPE constants:
REST maps to RestApi, MCP maps to Mcp, and WEBSUB maps to WebSubApi, while the
other request keywords (SOAP, WS, GRAPHQL) are returned unchanged.
In `@docs/rest-apis/devportal/schemas.md`:
- Line 590: The ApiInfoResponse type description in the shared schema is
inaccurate because “the rest are returned unchanged” excludes MCP, which is
remapped. Update the canonical field text for the type property in the
ApiInfoResponse/allOf source so it explicitly reflects the actual mappings from
resolveApiType and constants.js: REST to RestApi, WEBSUB to WebSubApi, MCP to
Mcp, and the remaining values unchanged. This will correct the duplicated
wording that propagates into apis.md and mcp-servers.md.
---
Outside diff comments:
In `@docs/rest-apis/devportal/apis.md`:
- Around line 62-89: Update the API response examples in the devportal docs so
the documented type value matches the new canonical constant: replace the stale
"type": "REST" examples in the create/update response sections with "RestApi".
Make the change in both example blocks associated with the API response
payloads, keeping the rest of the JSON structure unchanged and consistent with
the type-resolution behavior already documented.
In `@docs/rest-apis/devportal/mcp-servers.md`:
- Around line 61-88: Update the MCP server docs to use the canonical type value
everywhere: change the create/update response examples in the MCP server
examples and the surrounding prose from `MCP` to `Mcp`. Keep the wording aligned
with the type-resolution mapping referenced in the document, and make the same
correction in the sections that describe the stored/returned type and the
“Always typed” text so the examples and narrative match consistently.
In `@portals/developer-portal/it/rest-api/support/envelopeCrypto.js`:
- Around line 27-39: The decryptFromEnvelope helper uses RSA-OAEP via
crypto.privateDecrypt and must be annotated per the PQC coding guideline. Add a
clear // TODO(pqc): migrate comment in decryptFromEnvelope near the RSA-based
key unwrap so the remaining quantum-vulnerable crypto is flagged, while leaving
the existing envelope interoperability logic unchanged.
---
Duplicate comments:
In `@docs/rest-apis/devportal/mcp-servers.md`:
- Line 317: Update the `type` description in the MCP servers docs to match the
actual `API_TYPE` mapping in `constants.js`: `REST` maps to `RestApi`, `WEBSUB`
maps to `WebSubApi`, and `MCP` must be listed as remapped to `Mcp` rather than
grouped with values returned unchanged. Keep the wording aligned with the other
API type docs and use the `type` row in `ApiMetadataMultipartBody` as the
reference point.
In `@portals/developer-portal/it/docker-compose.test.postgres.yaml`:
- Around line 52-57: The healthcheck for the platform-api service is using curl,
but that binary is not present in the platform-api:0.11.0 image, so the service
can never become healthy. Update the healthcheck in the docker-compose test
setup to use a command that is already available inside the platform-api image
(or add the missing tool to the image), keeping the existing healthcheck
structure and service dependency flow for devportal and rest-api-tests intact.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33f03d4d-0e5e-4089-9cf9-6faf5a3f0238
⛔ Files ignored due to path filters (1)
portals/developer-portal/it/rest-api/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (49)
.github/workflows/devportal-integration-test.ymldocs/rest-apis/devportal/api-workflows.mddocs/rest-apis/devportal/apis.mddocs/rest-apis/devportal/mcp-servers.mddocs/rest-apis/devportal/organizations.mddocs/rest-apis/devportal/schemas.mdportals/developer-portal/README.mdportals/developer-portal/it/Makefileportals/developer-portal/it/README.mdportals/developer-portal/it/docker-compose.test.postgres.yamlportals/developer-portal/it/docker-compose.test.yamlportals/developer-portal/it/rest-api/ai-discovery/llms-txt.spec.jsportals/developer-portal/it/rest-api/api-keys/api-keys.spec.jsportals/developer-portal/it/rest-api/api-keys/webhook-events.spec.jsportals/developer-portal/it/rest-api/api-workflows/api-workflows.spec.jsportals/developer-portal/it/rest-api/api-workflows/generate-prompt.spec.jsportals/developer-portal/it/rest-api/apis/artifact-zip-apis.spec.jsportals/developer-portal/it/rest-api/apis/graphql-apis.spec.jsportals/developer-portal/it/rest-api/apis/rest-apis.spec.jsportals/developer-portal/it/rest-api/apis/soap-apis.spec.jsportals/developer-portal/it/rest-api/apis/websocket-apis.spec.jsportals/developer-portal/it/rest-api/apis/websub-apis.spec.jsportals/developer-portal/it/rest-api/applications/applications.spec.jsportals/developer-portal/it/rest-api/applications/webhook-events.spec.jsportals/developer-portal/it/rest-api/auth/file-based-login.spec.jsportals/developer-portal/it/rest-api/jest.config.jsportals/developer-portal/it/rest-api/key-managers/key-managers.spec.jsportals/developer-portal/it/rest-api/key-managers/token-generation.spec.jsportals/developer-portal/it/rest-api/mcp-servers/mcp-servers.spec.jsportals/developer-portal/it/rest-api/organizations/organizations.spec.jsportals/developer-portal/it/rest-api/package.jsonportals/developer-portal/it/rest-api/subscriptions/subscriptions.spec.jsportals/developer-portal/it/rest-api/subscriptions/webhook-events.spec.jsportals/developer-portal/it/rest-api/support/client.jsportals/developer-portal/it/rest-api/support/db.jsportals/developer-portal/it/rest-api/support/envelopeCrypto.jsportals/developer-portal/it/rest-api/support/fixtures.jsportals/developer-portal/it/rest-api/support/global-setup.jsportals/developer-portal/it/rest-api/support/global-teardown.jsportals/developer-portal/it/rest-api/support/wait-for.jsportals/developer-portal/it/rest-api/support/webhook-sink.jsportals/developer-portal/it/rest-api/support/zipBuilder.jsportals/developer-portal/it/rest-api/views-and-labels/labels.spec.jsportals/developer-portal/it/rest-api/views-and-labels/view-label-visibility.spec.jsportals/developer-portal/it/rest-api/views-and-labels/views.spec.jsportals/developer-portal/it/rest-api/webhook-subscribers/webhook-delivery.spec.jsportals/developer-portal/it/rest-api/webhook-subscribers/webhook-subscribers.spec.jsportals/developer-portal/src/dao/auditDao.jsportals/developer-portal/src/services/oauthTokenService.js
💤 Files with no reviewable changes (26)
- portals/developer-portal/it/rest-api/apis/websocket-apis.spec.js
- portals/developer-portal/it/rest-api/views-and-labels/view-label-visibility.spec.js
- portals/developer-portal/it/rest-api/support/global-teardown.js
- portals/developer-portal/it/rest-api/api-workflows/generate-prompt.spec.js
- portals/developer-portal/it/rest-api/apis/websub-apis.spec.js
- portals/developer-portal/it/rest-api/key-managers/key-managers.spec.js
- portals/developer-portal/it/rest-api/auth/file-based-login.spec.js
- portals/developer-portal/it/rest-api/support/fixtures.js
- portals/developer-portal/it/rest-api/subscriptions/subscriptions.spec.js
- portals/developer-portal/it/rest-api/webhook-subscribers/webhook-subscribers.spec.js
- portals/developer-portal/it/rest-api/views-and-labels/views.spec.js
- portals/developer-portal/it/rest-api/views-and-labels/labels.spec.js
- portals/developer-portal/it/rest-api/organizations/organizations.spec.js
- portals/developer-portal/it/rest-api/apis/graphql-apis.spec.js
- portals/developer-portal/it/rest-api/support/client.js
- portals/developer-portal/it/rest-api/support/wait-for.js
- portals/developer-portal/it/rest-api/applications/applications.spec.js
- portals/developer-portal/it/rest-api/api-keys/api-keys.spec.js
- portals/developer-portal/it/rest-api/applications/webhook-events.spec.js
- portals/developer-portal/it/rest-api/api-keys/webhook-events.spec.js
- portals/developer-portal/it/rest-api/apis/rest-apis.spec.js
- portals/developer-portal/it/rest-api/apis/soap-apis.spec.js
- portals/developer-portal/it/rest-api/apis/artifact-zip-apis.spec.js
- portals/developer-portal/it/rest-api/support/db.js
- portals/developer-portal/it/rest-api/mcp-servers/mcp-servers.spec.js
- portals/developer-portal/it/rest-api/subscriptions/webhook-events.spec.js
✅ Files skipped from review due to trivial changes (6)
- portals/developer-portal/it/rest-api/support/zipBuilder.js
- portals/developer-portal/it/rest-api/support/webhook-sink.js
- portals/developer-portal/it/rest-api/jest.config.js
- portals/developer-portal/src/dao/auditDao.js
- docs/rest-apis/devportal/organizations.md
- portals/developer-portal/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- portals/developer-portal/it/docker-compose.test.yaml
Purpose
$subject
Fix #2528
Approach
Added jest and supertest dependencies
Added tests for the existing flows
Added a github action workflow