chore: OpenAPI Spec docs for Endpoints: Get Extension & Delete Extension by ID - BED-7412#2376
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughListExtensions now returns a static internal-server-error detail on failure; its test expectation was updated. OpenAPI spec expanded with OpenGraph tag, a GET Changes
Sequence Diagram(s)(Skipped — changes are an error payload tweak, test update, and OpenAPI additions; no new multi-component control flow to visualize.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/go/openapi/src/openapi.yaml (1)
149-195:⚠️ Potential issue | 🟡 Minor
OpenGraphtag is absent fromx-tagGroups— endpoints won't appear in navigation when uncommented.Both new YAML files tag operations with
- OpenGraph, butOpenGraphis not registered in either tag group. The inline comment at line 148 is explicit:
## ALL TAGS MUST BE LISTED HERE TO SHOW UP IN THE NAVIGATION. ##The missing entry needs to be added before these paths are uncommented, otherwise the documentation tooling (ReDoc) will silently omit the entire group.
📝 Proposed fix (add to the appropriate tag group)
- name: Community & Enterprise tags: ... + - OpenGraph - CypherAlso applies to: 391-394
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/openapi/src/openapi.yaml` around lines 149 - 195, The x-tagGroups definition is missing the OpenGraph tag so operations tagged with "OpenGraph" will be omitted from ReDoc navigation; update the x-tagGroups block (the list under the name "Community & Enterprise" in the OpenAPI YAML) to include "OpenGraph" in its tags array (and likewise add it in the second occurrence referenced around lines 391-394) so the OpenGraph-tagged endpoints appear in the documentation navigation; ensure the exact tag string "OpenGraph" matches the operations' tags.
🧹 Nitpick comments (1)
packages/go/openapi/src/paths/opengraph.extension-list.yaml (1)
17-24:descriptionandsummaryare identical — consider providing a more detailed description.description: List OpenGraph Extensions Information # ← same as summary summary: List OpenGraph Extensions InformationThe
descriptionfield can carry richer prose (auth requirements, pagination behaviour, etc.) whilesummarystays concise. At minimum they shouldn't be identical.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/openapi/src/paths/opengraph.extension-list.yaml` around lines 17 - 24, The OpenAPI operation ListExtensions currently has identical summary and description; update the description for the GET operation (operationId: ListExtensions) to include richer details such as authentication requirements, pagination parameters, response shape or filtering behavior, while keeping the summary concise; modify the description field under the get block (not the summary) to describe auth expectations, paging (limit/offset or cursors), and any important response notes so the two fields are no longer identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/go/openapi/src/paths/opengraph.extension-delete.yaml`:
- Around line 30-31: The OpenGraph extension path parameter schema for
extension_id is incorrectly declared as type: string; update the parameter named
extension_id in the OpenGraph DELETE path to use type: integer with format:
int32 so it matches the handler's parsing (which uses
strconv.ParseInt(extensionID, 10, 32)) and prevents misleading consumers about
accepted values.
In `@packages/go/openapi/src/paths/opengraph.extension-list.yaml`:
- Around line 38-39: The OpenAPI spec lists a 404 response for the
ListExtensions operation but the ListExtensions handler never returns 404 (it
has no path parameter lookup and service only surfaces 5xx errors), so remove
the 404 response from the opengraph.extension-list.yaml responses section so the
documented status codes match the actual reachable codes (200, 401, 403, 500);
locate the ListExtensions operation definition in opengraph.extension-list.yaml
and delete the 404 $ref to ./../responses/not-found.yaml (or replace it with
only the actual responses) to keep the spec accurate.
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Line 1: The file
packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml
currently uses Windows CRLF line endings; convert the file to Unix LF endings
(e.g., run dos2unix on that file, set your editor to save with LF, or run git
update-index --renormalize after changing core.autocrlf) so the YAML contains
only \n line endings and linter errors from "wrong new line character" are
resolved.
---
Outside diff comments:
In `@packages/go/openapi/src/openapi.yaml`:
- Around line 149-195: The x-tagGroups definition is missing the OpenGraph tag
so operations tagged with "OpenGraph" will be omitted from ReDoc navigation;
update the x-tagGroups block (the list under the name "Community & Enterprise"
in the OpenAPI YAML) to include "OpenGraph" in its tags array (and likewise add
it in the second occurrence referenced around lines 391-394) so the
OpenGraph-tagged endpoints appear in the documentation navigation; ensure the
exact tag string "OpenGraph" matches the operations' tags.
---
Nitpick comments:
In `@packages/go/openapi/src/paths/opengraph.extension-list.yaml`:
- Around line 17-24: The OpenAPI operation ListExtensions currently has
identical summary and description; update the description for the GET operation
(operationId: ListExtensions) to include richer details such as authentication
requirements, pagination parameters, response shape or filtering behavior, while
keeping the summary concise; modify the description field under the get block
(not the summary) to describe auth expectations, paging (limit/offset or
cursors), and any important response notes so the two fields are no longer
identical.
kpowderly
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/api/src/api/v2/opengraphschema_test.go (1)
508-545:⚠️ Potential issue | 🟡 MinorUpdate this expected response body if the handler's error leak is fixed.
The expected
responseBodyat Line 544 contains the raw internal error ("error listing graph schema extensions: error"). If the handler is updated to use the genericapi.ErrorResponseDetailsInternalServerErrormessage (as recommended in the handler review), this assertion must be updated to match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/api/v2/opengraphschema_test.go` around lines 508 - 545, Update the test's expected response body for the "Error: error retrieving graph schema extensions" case to match the handler's new generic error message; locate the test case in opengraphschema_test.go (the case with name "Error: error retrieving graph schema extensions") and change the expected.responseBody string (currently asserting "error listing graph schema extensions: error") to use the generic api.ErrorResponseDetailsInternalServerError message format used by the handler so the assertion matches the new error payload.
🧹 Nitpick comments (2)
packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml (1)
27-28: Consider addingformat: int32to theidproperty.The Go implementation uses
int32forExtensionInfo.ID, and the delete endpoint's path parameter also specifiesformat: int32(per the PR screenshots). Adding the format here keeps the spec precise and consistent.Suggested fix
id: type: integer + format: int32🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml` around lines 27 - 28, The OpenAPI schema's id property lacks an explicit integer format; update the id property in api.response.graph-extensions-info.yaml to include format: int32 so it matches the Go type ExtensionInfo.ID and the delete endpoint path param (which uses int32); ensure the id node reads type: integer and format: int32 to keep the spec consistent with the Go implementation.cmd/api/src/api/v2/opengraphschema_test.go (1)
491-499: Consider extracting helpers for the repeated context setup patterns, but note the variations.The code contains similar patterns of building
ctx.Contextwith various auth setups across multiple test cases, but they are not uniform:
- Lines 491-499, 689-697: Empty
User{}with no roles- Lines 557-574, 716-733, 752-769, 792-809, 831-848, 871-888: Admin
User{}withRoleAdministratorandAuthManageSelfpermissionWhile
createContextWithAdminOwnerIdexists insaved_queries_test.go, it uses a different structure (includesOwner.ID, full permissions, and callsConstructGoContext()rather thanWithRequestID("id")), so it cannot directly replace the code here. Creating separate helpers for non-admin and admin contexts would reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/api/v2/opengraphschema_test.go` around lines 491 - 499, Tests repeatedly construct ctx.Context with varying auth setups (empty owner vs admin with RoleAdministrator and AuthManageSelf) using inline blocks like ctx.Context{RequestID: "id", AuthCtx: auth.Context{Owner: model.User{}, Session: model.UserSession{}}} and then call WithRequestID("id"); extract two small helpers (e.g., makeTestCtxNoAuth and makeTestCtxAdmin) that return the appropriate ctx.Context (or a helper that returns the context.Context via WithRequestID) so tests can call them instead of repeating the struct literals; align behavior with existing createContextWithAdminOwnerId by keeping the same fields used here (empty Owner or Owner with RoleAdministrator and AuthManageSelf) and have the admin helper accept optional owner ID if needed, and ensure the helpers expose a ConstructGoContext/WithRequestID variant as required by each test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/src/api/v2/opengraphschema.go`:
- Around line 276-283: The 500 path in the ListExtensions handler leaks internal
error text to the client; instead, log the detailed err server-side and return
the opaque api.ErrorResponseDetailsInternalServerError message. Replace the
fmt.Sprintf("error listing graph schema extensions: %v", err) usage passed to
api.BuildErrorResponse with api.ErrorResponseDetailsInternalServerError, and add
a server-side log call (using the existing logger or s.OpenGraphSchemaService’s
logger) that records the detailed err and context (e.g., "error listing graph
schema extensions"). Keep the existing WriteErrorResponse/return flow unchanged.
---
Outside diff comments:
In `@cmd/api/src/api/v2/opengraphschema_test.go`:
- Around line 508-545: Update the test's expected response body for the "Error:
error retrieving graph schema extensions" case to match the handler's new
generic error message; locate the test case in opengraphschema_test.go (the case
with name "Error: error retrieving graph schema extensions") and change the
expected.responseBody string (currently asserting "error listing graph schema
extensions: error") to use the generic
api.ErrorResponseDetailsInternalServerError message format used by the handler
so the assertion matches the new error payload.
---
Duplicate comments:
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Line 1: Convert the CRLF line endings in the file
packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml to
LF-only line endings throughout the file (replace Windows CRLF with Unix LF) so
the file no longer contains CRLF; ensure your editor or git config uses
core.autocrlf=false or run a one-time normalization (e.g., dos2unix) before
committing to avoid reintroducing CRLF.
---
Nitpick comments:
In `@cmd/api/src/api/v2/opengraphschema_test.go`:
- Around line 491-499: Tests repeatedly construct ctx.Context with varying auth
setups (empty owner vs admin with RoleAdministrator and AuthManageSelf) using
inline blocks like ctx.Context{RequestID: "id", AuthCtx: auth.Context{Owner:
model.User{}, Session: model.UserSession{}}} and then call WithRequestID("id");
extract two small helpers (e.g., makeTestCtxNoAuth and makeTestCtxAdmin) that
return the appropriate ctx.Context (or a helper that returns the context.Context
via WithRequestID) so tests can call them instead of repeating the struct
literals; align behavior with existing createContextWithAdminOwnerId by keeping
the same fields used here (empty Owner or Owner with RoleAdministrator and
AuthManageSelf) and have the admin helper accept optional owner ID if needed,
and ensure the helpers expose a ConstructGoContext/WithRequestID variant as
required by each test.
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Around line 27-28: The OpenAPI schema's id property lacks an explicit integer
format; update the id property in api.response.graph-extensions-info.yaml to
include format: int32 so it matches the Go type ExtensionInfo.ID and the delete
endpoint path param (which uses int32); ensure the id node reads type: integer
and format: int32 to keep the spec consistent with the Go implementation.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/go/openapi/src/openapi.yaml (1)
149-196:⚠️ Potential issue | 🟡 Minor
OpenGraphtag must be added tox-tagGroupsbefore uncommenting the extension endpoints.The comment on line 148 explicitly states
## ALL TAGS MUST BE LISTED HERE TO SHOW UP IN THE NAVIGATION. ##. Both extension path files (opengraph.extensions.yamland the existing PUT operation) usetags: [OpenGraph, Community, Enterprise], butOpenGraphis absent from both tag groups. When the paths are uncommented, these endpoints will be invisible in ReDoc's navigation sidebar.♻️ Suggested addition
- name: Community & Enterprise tags: - Auth + - OpenGraph - Roles ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/openapi/src/openapi.yaml` around lines 149 - 196, Add the missing "OpenGraph" tag to the x-tagGroups so ReDoc will show endpoints tagged with OpenGraph (used by opengraph.extensions.yaml and the existing PUT operation that declare tags: [OpenGraph, Community, Enterprise]); specifically, update the x-tagGroups entries (e.g., the Community & Enterprise tag list) to include the exact tag "OpenGraph" so the extension endpoints appear in the navigation and the tag spelling matches across the OpenAPI file and the extension paths.
🧹 Nitpick comments (1)
packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml (1)
22-36: Schema lacksformat: int32onidandrequiredconstraints on nested objects.Two accuracy gaps:
idistype: integerwith noformat. The Go type isint32; withoutformat: int32, generated clients may infer a 64-bit range.- Neither
extensionsinsidedatanor the properties of each extension item (id,name,version,is_builtin) are listed asrequired. This makes the schema technically allow empty/partial extension objects.♻️ Suggested improvement
extensions: type: array + required: + - extensions items: type: object + required: + - id + - name + - version + - is_builtin properties: id: type: integer + format: int32 name: type: string version: type: string is_builtin: type: boolean🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml` around lines 22 - 36, The OpenAPI schema for the response should specify id as a 32-bit integer and mark required fields for nested objects: update the extensions array item schema to set id to type: integer with format: int32, and add required arrays so that the extensions property itself is required under data and each extension item lists required: [id, name, version, is_builtin]; ensure the top-level required still includes data and that the nested required entries reference the same property names (extensions, id, name, version, is_builtin) to enforce non-empty/complete extension objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/go/openapi/src/openapi.yaml`:
- Around line 149-196: Add the missing "OpenGraph" tag to the x-tagGroups so
ReDoc will show endpoints tagged with OpenGraph (used by
opengraph.extensions.yaml and the existing PUT operation that declare tags:
[OpenGraph, Community, Enterprise]); specifically, update the x-tagGroups
entries (e.g., the Community & Enterprise tag list) to include the exact tag
"OpenGraph" so the extension endpoints appear in the navigation and the tag
spelling matches across the OpenAPI file and the extension paths.
---
Duplicate comments:
In `@cmd/api/src/api/v2/opengraphschema_test.go`:
- Around line 537-545: The test is asserting a leaked internal error string in
expected.responseBody; update the assertion to match the new standardized
internal error message constant used by the API
(api.ErrorResponseDetailsInternalServerError) instead of the raw "error listing
graph schema extensions: error" text. Locate the test setup where
mock.mockOpenGraphSchemaService.EXPECT().ListExtensions(...) returns an error
and change expected.responseBody in opengraphschema_test.go to the JSON that
includes the api.ErrorResponseDetailsInternalServerError message (or otherwise
assert the response message field equals
api.ErrorResponseDetailsInternalServerError) so the test aligns with the change
in opengraphschema.go.
In `@cmd/api/src/api/v2/opengraphschema.go`:
- Around line 281-283: The handler currently passes the raw service error into
api.BuildErrorResponse when ListExtensions fails
(s.OpenGraphSchemaService.ListExtensions) causing internal error details to be
returned to clients; instead log the detailed err server-side and call
api.BuildErrorResponse with api.ErrorResponseDetailsInternalServerError (via
api.WriteErrorResponse) so the client only receives the generic internal-server
detail; update the corresponding test in opengraphschema_test.go (around the
failing assertion at line ~544) to expect the generic detail rather than the raw
error string.
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Line 1: Update the new YAML header in
packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml to match
the PR's copyright year by changing the comment to "# Copyright 2026 Specter
Ops, Inc." and ensure the file uses LF (Unix) line endings to fix the YAMLlint
CRLF error; mirror the same header and line-ending style used in
opengraphschema.go, opengraphschema_test.go, and opengraph.extensions.yaml so
the project-wide convention is consistent.
---
Nitpick comments:
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Around line 22-36: The OpenAPI schema for the response should specify id as a
32-bit integer and mark required fields for nested objects: update the
extensions array item schema to set id to type: integer with format: int32, and
add required arrays so that the extensions property itself is required under
data and each extension item lists required: [id, name, version, is_builtin];
ensure the top-level required still includes data and that the nested required
entries reference the same property names (extensions, id, name, version,
is_builtin) to enforce non-empty/complete extension objects.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Around line 23-26: The schema places required: [extensions] alongside type:
array (next to items) where it has no effect; move the required array into the
data object definition (the same level as properties:) so that data.required
includes "extensions", remove the misplaced required from the array schema, and
ensure the data object defines a properties block containing an "extensions"
property (and any needed type) so validators will enforce it.
---
Duplicate comments:
In `@cmd/api/src/api/v2/opengraphschema.go`:
- Around line 281-283: The 500 error path after calling
s.OpenGraphSchemaService.ListExtensions swallows the returned err and lacks
server-side logging; add a slog.WarnContext (or the project's standard logger)
that logs the error and relevant context (e.g., "failed to list OpenGraph
extensions") using ctx and the err before invoking api.WriteErrorResponse/
api.BuildErrorResponse so the internal server error is observable in logs while
keeping the response body as api.ErrorResponseDetailsInternalServerError.
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Line 1: Convert the file to Unix LF line endings (replace CRLF with \n) to
satisfy YAML linting, and update the copyright header string "# Copyright 2024
Specter Ops, Inc." to "# Copyright 2026 Specter Ops, Inc." so it matches the
companion opengraph.extensions.yaml; ensure the file is saved with LF endings
before committing.
sirisjo
reviewed
Feb 20, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml (1)
1-1: Windows-style line endings (CRLF) — reminder from previous review.YAMLlint still flags line 1 with
[error] wrong new line character: expected \n. This was flagged in a prior round but is not yet marked resolved. Rundos2unixor set the file's EOL to LF before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml` at line 1, The file api.response.graph-extensions-info.yaml contains Windows CRLF line endings; convert it to LF before merging (e.g., run dos2unix on that file or set your editor/EOL to LF), re-stage and commit the normalized file, and optionally add/update .gitattributes (e.g., *.yaml text eol=lf) or ensure core.autocrlf is configured so future YAML files are saved with LF.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml`:
- Line 1: The file api.response.graph-extensions-info.yaml contains Windows CRLF
line endings; convert it to LF before merging (e.g., run dos2unix on that file
or set your editor/EOL to LF), re-stage and commit the normalized file, and
optionally add/update .gitattributes (e.g., *.yaml text eol=lf) or ensure
core.autocrlf is configured so future YAML files are saved with LF.
ℹ️ Review info
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/go/openapi/src/openapi.yamlpackages/go/openapi/src/schemas/api.response.graph-extensions-info.yaml
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This ticket is to update our OpenAPI spec to add the following endpoints:
I noticed that both the Upsert Extension & the Delete Extension by ID both sit behind authentication middleware so I added the same for List Extensions Info to stay consistent.
Note: the
openapi.jsonwill not reflect these changes until we uncomment the endpoints prior to release so we don't prematurely expose these endpoints.Motivation and Context
Resolves BED-7412
OpenAPI spec
How Has This Been Tested?
Generated the spec
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
API Changes
Bug Fixes