-
Notifications
You must be signed in to change notification settings - Fork 55
Regenerated OpenAPI spec + doc #451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughIntroduces new authorization schemas (Action, AccessRule, AuthorizationConfiguration), JWT role extraction (JsonPathOperator, JwtRoleRule) and wires them into JwtConfiguration/JwkConfiguration. Adds optional authorization to top-level Configuration. Refactors ConversationDetails to last_used_model/provider. Updates examples. Adds QueryRequest.media_type in output doc. Minor wording fixes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client
participant Service
participant JWT_Verifier as JWT Verifier
participant Role_Engine as Role Rule Engine
participant Authorizer
User->>Client: Send request with JWT
Client->>Service: API call + Authorization header
Service->>JWT_Verifier: Validate JWT (signature, expiry)
JWT_Verifier-->>Service: Claims
rect rgba(200,230,255,0.3)
note over Service,Role_Engine: Extract roles via JSONPath rules
Service->>Role_Engine: role_rules + claims
Role_Engine-->>Service: roles[]
end
rect rgba(220,255,220,0.3)
note over Service,Authorizer: Check ACL against action
Service->>Authorizer: roles[] + action
Authorizer-->>Service: allow/deny
end
alt allowed
Service-->>Client: Proceed with requested action
else denied
Service-->>Client: 403 Forbidden
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (2)
docs/openapi.json (2)
55-67: Response examples are attached at invalid locations under responses; move them under content…example(s).In OpenAPI 3.1, example payloads must live inside responses.content[""].example or .examples. The fields currently placed directly under the HTTP status (e.g., "name"/"version" at Lines 65–67; "models" at Lines 89–110; "conversation_id"/"response" at Lines 146–148; and custom "detail"/"cause" blocks at 171–174, 247–286, 467–484, 539–542, 579–597) make the spec invalid for generators/validators.
Apply this pattern and replicate across all affected responses:
@@ "responses": { "200": { "description": "Successful Response", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/InfoResponse" }, + "example": { + "name": "Lightspeed Stack", + "version": "1.0.0" + } } } - , - "name": "Service name", - "version": "Service version" } }For error responses that currently lack content but show example fields (e.g., 503 blocks), either:
- Add the appropriate content/schema (e.g., ErrorResponse) and place the example there, or
- Remove the inline example properties from the response object if the response has no body.
This change will make the spec consumable by tooling and prevent CI schema-validation failures.
Also applies to: 79-112, 137-148, 169-175, 237-287, 457-484, 529-542, 577-599
189-215: Streaming API media type should be text/event-stream, not application/json.The streaming endpoint returns SSE. The current 200 response advertises application/json with an empty schema (Lines 211–214), which misleads clients.
Proposed fix:
"responses": { "200": { "description": "Successful Response", "content": { - "application/json": { - "schema": {} - } + "text/event-stream": { + "schema": { "type": "string", "description": "SSE stream" }, + "examples": { + "sse": { + "summary": "SSE example", + "value": "event: token\ndata: {\"text\":\"Hello\"}\n\n" + } + } + } } },This aligns the spec with actual behavior and improves client compatibility.
🧹 Nitpick comments (21)
docs/openapi.json (4)
701-744: Consider bounding untrusted arrays with maxItems where reasonable (CKV_OPENAPI_21).To reduce the risk of pathological inputs and align with static analysis (CKV_OPENAPI_21), consider adding maxItems to:
- AccessRule.actions (Lines 707–713),
- AuthorizationConfiguration.access_rules (Lines 852–858),
- JwtConfiguration.role_rules (Lines 1591–1597),
- JwtRoleRule.roles (Lines 1621–1626).
Avoid capping listing endpoints such as ModelsResponse.models (Lines 1735–1761) and ConversationsListResponse.conversations (Lines 1160–1165) unless you intend to enforce pagination.
Example:
"actions": { "items": { "$ref": "#/components/schemas/Action" }, "type": "array", "title": "Actions", + "maxItems": 64 },Pick limits that match your operational expectations.
Also applies to: 850-863, 1591-1597, 1603-1637, 1158-1173, 1733-1769
777-797: Minor grammar fix: “can be send” → “can be sent” in Attachment description.Tweak the description to read naturally.
- "description": "Model representing an attachment that can be send from the UI as part of query. + "description": "Model representing an attachment that can be sent from the UI as part of a query.
1107-1112: Fix Python example: add missing comma after conversation_id and ensure consistent quoting.Current example under ConversationDetails is not valid Python.
- conversation = ConversationDetails( - conversation_id="123e4567-e89b-12d3-a456-426614174000" + conversation = ConversationDetails( + conversation_id="123e4567-e89b-12d3-a456-426614174000", created_at="2024-01-01T00:00:00Z", last_message_at="2024-01-01T00:05:00Z", message_count=5, last_used_model="gemini/gemini-2.0-flash", last_used_provider="gemini", )
2003-2017: Tighten QueryRequest.media_type to known values or document extensibility.If the only supported value is application/json (as examples suggest), consider constraining with enum and documenting defaults; otherwise list accepted values.
"media_type": { "anyOf": [ - { "type": "string" }, + { "type": "string", "enum": ["application/json"] }, { "type": "null" } ], "title": "Media Type", - "description": "Media type (used just to enable compatibility)" + "description": "Optional media type for compatibility. Currently only application/json is accepted." }If you intend to support more, expand the enum accordingly.
docs/output.md (9)
407-417: AccessRule section lacks field details/examples; add brief definitions or a sample.Consider adding a short example and clarifying both fields:
| Field | Type | Description | |-------|------|-------------| -| role | string | | -| actions | array | | +| role | string | Role name (e.g., "admin", "viewer") | +| actions | array(Action) | Allowed actions for this role | + +Example: +```json +{ "role": "viewer", "actions": ["list_conversations", "get_conversation"] } +```
419-426: Document the Action enum values for users.Right now the Action section is empty of values. List them so integrators don’t need to dig into JSON.
-## Action - - -Available actions in the system. +## Action + +Available actions in the system: + +- admin +- list_other_conversations +- read_other_conversations +- query_other_conversations +- delete_other_conversations +- query +- streaming_query +- get_conversation +- list_conversations +- delete_conversation +- feedback +- get_models +- get_metrics +- get_config +- info
430-441: Grammar/clarity: “can be send” → “can be sent”; “as part of query” → “as part of a query”.Also align phrasing with openapi.json to keep docs consistent.
-Model representing an attachment that can be send from the UI as part of query. +Model representing an attachment that can be sent from the UI as part of a query. -A list of attachments can be an optional part of 'query' request. +A list of attachments can be an optional part of a 'query' request. YAML attachments with **kind** and **metadata/name** attributes will -be handled as resources with the specified name: +be handled as resources with the specified name:
470-479: AuthorizationConfiguration is underspecified; add a sentence on its purpose and constraints.Briefly explain that it is a list of AccessRule entries, and note any system-level reserved roles if applicable. This aids operators configuring RBAC.
526-527: Surface that Configuration.authorization is optional and defaults to null/empty; mention effect when omitted.Add one sentence clarifying that without authorization, all authenticated users may be permitted (or that endpoints enforce defaults). Helps set expectations.
568-580: Fix Python examples: missing comma after conversation_id.Same issue appears here; add the comma so the examples can be copied verbatim.
- conversation = ConversationDetails( - conversation_id="123e4567-e89b-12d3-a456-426614174000" + conversation = ConversationDetails( + conversation_id="123e4567-e89b-12d3-a456-426614174000",Also applies to: 573-581
802-815: InfoResponse section improved; consider adding a compact JSON example block.For consistency with other sections, include a JSON example for quick copy/paste.
Example: ```python info_response = InfoResponse( name="Lightspeed Stack", version="1.0.0", )+JSON:
+json +{ "name": "Lightspeed Stack", "version": "1.0.0" } +--- `823-830`: **JsonPathOperator: list allowed values inline to avoid lookup.** Add enum values next to the description. ```diff -## JsonPathOperator - -Supported operators for JSONPath evaluation. +## JsonPathOperator + +Supported operators for JSONPath evaluation: equals, contains, in.
850-854: JwtConfiguration/JwtRoleRule docs: clarify expected value types per operator and add a short example.Suggest adding a note:
- equals/contains expect scalar value.
- in expects array of scalars.
- roles should be non-empty.
Optionally show a concise example mapping a claim to roles.
Also applies to: 856-869
docs/openapi.md (8)
419-426: Document the Action enum values for parity with openapi.json.Replicate the enum list so readers of this page see the same allowed actions without cross-referencing.
-## Action - - -Available actions in the system. +## Action + +Available actions in the system: + +- admin +- list_other_conversations +- read_other_conversations +- query_other_conversations +- delete_other_conversations +- query +- streaming_query +- get_conversation +- list_conversations +- delete_conversation +- feedback +- get_models +- get_metrics +- get_config +- info
430-441: Polish Attachment wording for clarity and consistency.-Model representing an attachment that can be send from UI as part of query. -List of attachments can be optional part of 'query' request. +Model representing an attachment that can be sent from the UI as part of a query. +A list of attachments can be an optional part of a 'query' request. YAML attachments with **kind** and **metadata/name** attributes will -be handled as resources with specified name: +be handled as resources with the specified name:
471-479: AuthorizationConfiguration section: add 1–2 lines on structure and intent.E.g., “A list of AccessRule items mapping roles to allowed actions. If omitted, authorization is disabled/enforced by defaults.”
526-527: Configuration.authorization: note default and behavior when absent.Mirror the note suggested for output.md for symmetry across docs.
568-580: Fix Python examples: missing comma after conversation_id.Same missing comma here; update both examples.
Also applies to: 573-581
812-825: InfoResponse section: add a compact JSON example for quick reference.Same suggestion as output.md to improve developer ergonomics.
833-840: JsonPathOperator: inline the allowed values (equals, contains, in).Keeps docs self-contained.
861-864: JwtConfiguration/JwtRoleRule docs: clarify “value” typing and add an example rule.Briefly document that “in” expects an array and show a JSON snippet mapping a JSONPath to roles. This will reduce configuration mistakes.
Also applies to: 866-879
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/openapi.json(11 hunks)docs/openapi.md(8 hunks)docs/output.md(8 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
docs/openapi.json
[MEDIUM] 707-713: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
| "components": { | ||
| "schemas": { | ||
| "AccessRule": { | ||
| "properties": { | ||
| "role": { | ||
| "type": "string", | ||
| "title": "Role" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spec declares 401/403 but defines no securitySchemes or per-operation security; add bearer JWT scheme and apply to protected endpoints.
Given the presence of JWK/JWT configuration and 401/403 responses, the spec should declare an auth mechanism to be usable by code generators and API portals.
- Add a bearer security scheme under components.
- Attach security requirements to operations that require auth (e.g., POST /v1/query, POST /v1/streaming_query, GET/DELETE conversation endpoints, POST /v1/feedback, GET /v1/conversations, POST /authorized if applicable).
Minimal addition:
@@
"components": {
"schemas": {
...
}
+ ,
+ "securitySchemes": {
+ "bearerAuth": {
+ "type": "http",
+ "scheme": "bearer",
+ "bearerFormat": "JWT",
+ "description": "JWT Bearer token obtained via your identity provider"
+ }
+ }
}Then, per operation (example for POST /v1/query):
"post": {
"tags": ["query"],
"summary": "Query Endpoint Handler",
+ "security": [ { "bearerAuth": [] } ],
...Without these, clients won’t know how to authenticate.
Also applies to: 940-1012, 1556-1570, 1579-1602
🤖 Prompt for AI Agents
In docs/openapi.json around lines 699-706 (and also review ranges ~940-1012,
1556-1570, 1579-1602), the OpenAPI spec lists 401/403 responses but defines no
security schemes or per-operation security; add a bearer JWT security scheme
under components.securitySchemes (type: http, scheme: bearer, bearerFormat: JWT)
and then attach security requirements to each protected operation (e.g., POST
/v1/query, POST /v1/streaming_query, GET/DELETE conversation endpoints, POST
/v1/feedback, GET /v1/conversations, POST /authorized if applicable) by adding a
security array referencing the bearer scheme so clients and code generators know
how to authenticate.
| "type": "string", | ||
| "enum": [ | ||
| "equals", | ||
| "contains", | ||
| "in" | ||
| ], | ||
| "title": "JsonPathOperator", | ||
| "description": "Supported operators for JSONPath evaluation." | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
JwtRoleRule.value is required but has no type; define explicit schema and document operator-dependent expectations.
Right now, "value" (Lines 1618–1619) is title-only with no type, yet it is required (Lines 1630–1634). This is too permissive and will likely fail client generation or allow invalid configs at runtime.
Recommend constraining "value" and clarifying semantics:
"value": {
- "title": "Value"
+ "title": "Value",
+ "description": "Literal to compare for 'equals'/'contains', or array of candidates for 'in'.",
+ "anyOf": [
+ { "type": "string" },
+ { "type": "number" },
+ { "type": "boolean" },
+ { "type": "array", "items": { "type": "string" }, "minItems": 1 },
+ { "type": "array", "items": { "type": "number" }, "minItems": 1 }
+ ]
},Optionally, add a note to "operator" clarifying that "in" expects an array, while "equals"/"contains" expect scalars. Without this, validators and UIs can’t guide users correctly.
Also applies to: 1603-1637
🤖 Prompt for AI Agents
In docs/openapi.json around lines 1546 to 1554, JwtRoleRule.value is currently
only a title and left untyped while marked required elsewhere; update the schema
to explicitly declare allowed types and document operator-dependent
expectations: set "value" to allow either a string (for "equals" and "contains")
or an array of strings (for "in") — preferably via a oneOf that includes
{"type":"string"} and {"type":"array","items":{"type":"string"}} — and add a
clear description noting that "in" expects an array and "equals"/"contains"
expect a scalar; ensure the required list still includes "value" and adjust any
examples or validators to match the new schema.
Description
Regenerated OpenAPI spec + doc
Type of change
Summary by CodeRabbit
New Features
Documentation