refactor: simplify Responses lifecycle permissions to require explicit per-verb flags and expose them in UI - #4880
Conversation
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughBackend permission checks for Responses API lifecycle operations (retrieve, delete, cancel, input items) are simplified to evaluate their own granular flags directly, removing fallback logic and a helper method. Frontend types, schemas, constants, and provider forms are extended to support these four new request types, and a related test file is removed. ChangesBackend permission logic
Estimated code review effort: 3 (Moderate) | ~20 minutes Frontend granular request type support
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant AllowedRequests
Client->>Server: request Responses lifecycle operation
Server->>AllowedRequests: IsOperationAllowed(operation)
AllowedRequests-->>Server: granular flag value
Server-->>Client: allow or deny
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Confidence Score: 4/5Safe to merge with one gap: the old test file was deleted without replacement, so the new explicit-flag semantics have no automated regression coverage. The Go logic change is minimal and correct — each lifecycle verb now returns its own dedicated field with no branching. The UI changes are consistent across all touched surfaces (types, schema, constants, form defaults). The only gap is the deleted test file: the new behavior (responses: true alone must deny lifecycle operations) is entirely untested, which could allow a quiet regression in a later refactor. core/schemas/allowedrequestsresponseslifecycle_test.go — deleted without a replacement that covers the new strict-flag semantics. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[IsOperationAllowed called] --> B{ar == nil?}
B -- yes --> C[return true]
B -- no --> D{operation type}
D -- ResponsesRequest --> E[return ar.Responses]
D -- ResponsesStreamRequest --> F[return ar.ResponsesStream]
D -- ResponsesRetrieveRequest --> G[return ar.ResponsesRetrieve]
D -- ResponsesDeleteRequest --> H[return ar.ResponsesDelete]
D -- ResponsesCancelRequest --> I[return ar.ResponsesCancel]
D -- ResponsesInputItemsRequest --> J[return ar.ResponsesInputItems]
style G fill:#d4edda,stroke:#28a745
style H fill:#d4edda,stroke:#28a745
style I fill:#d4edda,stroke:#28a745
style J fill:#d4edda,stroke:#28a745
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[IsOperationAllowed called] --> B{ar == nil?}
B -- yes --> C[return true]
B -- no --> D{operation type}
D -- ResponsesRequest --> E[return ar.Responses]
D -- ResponsesStreamRequest --> F[return ar.ResponsesStream]
D -- ResponsesRetrieveRequest --> G[return ar.ResponsesRetrieve]
D -- ResponsesDeleteRequest --> H[return ar.ResponsesDelete]
D -- ResponsesCancelRequest --> I[return ar.ResponsesCancel]
D -- ResponsesInputItemsRequest --> J[return ar.ResponsesInputItems]
style G fill:#d4edda,stroke:#28a745
style H fill:#d4edda,stroke:#28a745
style I fill:#d4edda,stroke:#28a745
style J fill:#d4edda,stroke:#28a745
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ui/lib/constants/logs.ts (1)
273-276: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAll four new lifecycle badges share the same color.
responses_retrieve,responses_delete,responses_cancel, andresponses_input_itemsall map to identical teal classes, unlike other sibling groups (e.g.,batch_*,video_*) which each get a distinct color for visual differentiation in the logs table.🤖 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 `@ui/lib/constants/logs.ts` around lines 273 - 276, The new lifecycle badge entries all use the same teal styling, so update the mappings in logs constants to give responses_retrieve, responses_delete, responses_cancel, and responses_input_items distinct badge colors similar to other grouped keys like batch_* and video_*. Adjust the color values in the logs mapping so each response lifecycle status is visually differentiated in the logs table while keeping the rest of the constants structure unchanged.ui/lib/types/schemas.ts (1)
587-621: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider matching required-ness of sibling fields.
The new
responses_retrieve/delete/cancel/input_itemsfields are.optional(), while siblingresponses/responses_streamare required. Since every current consumer (addNewCustomProviderSheet.tsx,apiStructureFormFragment.tsx) already supplies explicit defaults, making these required would tighten the contract and catch future callers that forget to set a default.🤖 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 `@ui/lib/types/schemas.ts` around lines 587 - 621, The new responses_retrieve, responses_delete, responses_cancel, and responses_input_items fields in allowedRequestsSchema are currently optional while sibling response fields are required. Update allowedRequestsSchema to make these fields required like responses and responses_stream, and ensure any callers such as addNewCustomProviderSheet.tsx and apiStructureFormFragment.tsx continue to pass explicit boolean defaults through the schema contract.
🤖 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.
Nitpick comments:
In `@ui/lib/constants/logs.ts`:
- Around line 273-276: The new lifecycle badge entries all use the same teal
styling, so update the mappings in logs constants to give responses_retrieve,
responses_delete, responses_cancel, and responses_input_items distinct badge
colors similar to other grouped keys like batch_* and video_*. Adjust the color
values in the logs mapping so each response lifecycle status is visually
differentiated in the logs table while keeping the rest of the constants
structure unchanged.
In `@ui/lib/types/schemas.ts`:
- Around line 587-621: The new responses_retrieve, responses_delete,
responses_cancel, and responses_input_items fields in allowedRequestsSchema are
currently optional while sibling response fields are required. Update
allowedRequestsSchema to make these fields required like responses and
responses_stream, and ensure any callers such as addNewCustomProviderSheet.tsx
and apiStructureFormFragment.tsx continue to pass explicit boolean defaults
through the schema contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c6c76ee0-21a0-447e-aa66-cf6c73ed7da1
📒 Files selected for processing (9)
core/schemas/allowedrequestsresponseslifecycle_test.gocore/schemas/provider.goui/app/workspace/providers/dialogs/addNewCustomProviderSheet.tsxui/app/workspace/providers/fragments/allowedRequestsFields.tsxui/app/workspace/providers/fragments/apiStructureFormFragment.tsxui/lib/constants/config.tsui/lib/constants/logs.tsui/lib/types/config.tsui/lib/types/schemas.ts
💤 Files with no reviewable changes (1)
- core/schemas/allowedrequestsresponseslifecycle_test.go
Merge activity
|
* 'dev' of https://github.com/maximhq/bifrost: ipv6 support (maximhq#4895) docs: add virtual key expiry support docs (maximhq#4889) test: add Postman e2e collection and runner for virtual key expiry validation and enforcement (maximhq#4888) feat: add expiry field to virtual keys (maximhq#4887) fix: converts thinking to disabled if tool choice is required for deepseek (maximhq#4861) chore: adds docs for deepseek provider (maximhq#4854) chore: adds tests for deepseek provider (maximhq#4853) feat: adds deepseek provider (maximhq#4852) fix: cost for image generation or image edit streaming (maximhq#4802) feat: add `BedrockMantleKeyConfig` support to key hashing, schema/table mapping, and sensitive field clearing (maximhq#4886) fix: skip O(N) reference refresh on request-time rate-limit/budget reset (maximhq#4883) refactor: simplify Responses lifecycle permissions to require explicit per-verb flags and expose them in UI (maximhq#4880) fix: append datasheet models for incomplete list models call (maximhq#4879) # Conflicts: # ui/app/workspace/providers/fragments/allowedRequestsFields.tsx # ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx # ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx # ui/app/workspace/virtual-keys/views/virtualKeysTable.tsx # ui/components/ui/datePickerWithRange.tsx
…t per-verb flags and expose them in UI (maximhq#4880) ## Summary Responses API lifecycle operations (`retrieve`, `delete`, `cancel`, `input_items`) are now independently gated by their own `AllowedRequests` flags rather than falling back to the `responses` (create) flag. Previously, a granular mode was inferred when any lifecycle flag was set, with unset verbs falling back to the `responses` flag. This change removes that fallback logic and makes each lifecycle verb require its own explicit flag. ## Changes - Removed the `granularResponsesLifecycleUsed()` helper and the associated fallback logic in `IsOperationAllowed`. Each of `ResponsesRetrieveRequest`, `ResponsesDeleteRequest`, `ResponsesCancelRequest`, and `ResponsesInputItemsRequest` now maps directly to its own boolean flag with no inheritance from `Responses`. - Removed the test file that covered the now-deleted granular/legacy fallback behavior. - Added `responses_retrieve`, `responses_delete`, `responses_cancel`, and `responses_input_items` to the UI's `RequestType` union, `AllowedRequests` interface, Zod schema, request type lists, labels, and color mappings. - Exposed the four new flags in the allowed requests form, defaulting to `false` for existing providers and `true` when creating a new custom provider. - Added a `PathOverrideUnsupported` set in the allowed requests UI to suppress the path-override control for lifecycle request types, since those paths embed a response ID and a static override cannot produce a valid URL. - Added the four lifecycle request types to the OpenAI entry in `PROVIDER_SUPPORTED_REQUESTS`. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./core/schemas/... # UI cd ui pnpm i pnpm build ``` Verify that a provider with only `responses: true` (and no lifecycle flags) now denies retrieve/delete/cancel/input_items requests. Verify that explicitly setting `responses_retrieve: true` allows retrieve while leaving delete/cancel/input_items denied. ## Breaking changes - [x] Yes - [ ] No Any existing configuration that relied on `responses: true` implicitly allowing lifecycle verbs (retrieve, delete, cancel, input_items) will now deny those operations. Operators must explicitly add `responses_retrieve`, `responses_delete`, `responses_cancel`, and/or `responses_input_items` to their `AllowedRequests` configuration to restore access. ## Security considerations The change is strictly more restrictive by default. Lifecycle operations that were previously allowed implicitly through the `responses` flag now require explicit opt-in, reducing the risk of unintended access to stored response data. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
…t per-verb flags and expose them in UI (maximhq#4880) ## Summary Responses API lifecycle operations (`retrieve`, `delete`, `cancel`, `input_items`) are now independently gated by their own `AllowedRequests` flags rather than falling back to the `responses` (create) flag. Previously, a granular mode was inferred when any lifecycle flag was set, with unset verbs falling back to the `responses` flag. This change removes that fallback logic and makes each lifecycle verb require its own explicit flag. ## Changes - Removed the `granularResponsesLifecycleUsed()` helper and the associated fallback logic in `IsOperationAllowed`. Each of `ResponsesRetrieveRequest`, `ResponsesDeleteRequest`, `ResponsesCancelRequest`, and `ResponsesInputItemsRequest` now maps directly to its own boolean flag with no inheritance from `Responses`. - Removed the test file that covered the now-deleted granular/legacy fallback behavior. - Added `responses_retrieve`, `responses_delete`, `responses_cancel`, and `responses_input_items` to the UI's `RequestType` union, `AllowedRequests` interface, Zod schema, request type lists, labels, and color mappings. - Exposed the four new flags in the allowed requests form, defaulting to `false` for existing providers and `true` when creating a new custom provider. - Added a `PathOverrideUnsupported` set in the allowed requests UI to suppress the path-override control for lifecycle request types, since those paths embed a response ID and a static override cannot produce a valid URL. - Added the four lifecycle request types to the OpenAI entry in `PROVIDER_SUPPORTED_REQUESTS`. ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./core/schemas/... # UI cd ui pnpm i pnpm build ``` Verify that a provider with only `responses: true` (and no lifecycle flags) now denies retrieve/delete/cancel/input_items requests. Verify that explicitly setting `responses_retrieve: true` allows retrieve while leaving delete/cancel/input_items denied. ## Breaking changes - [x] Yes - [ ] No Any existing configuration that relied on `responses: true` implicitly allowing lifecycle verbs (retrieve, delete, cancel, input_items) will now deny those operations. Operators must explicitly add `responses_retrieve`, `responses_delete`, `responses_cancel`, and/or `responses_input_items` to their `AllowedRequests` configuration to restore access. ## Security considerations The change is strictly more restrictive by default. Lifecycle operations that were previously allowed implicitly through the `responses` flag now require explicit opt-in, reducing the risk of unintended access to stored response data. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Responses API lifecycle operations (
retrieve,delete,cancel,input_items) are now independently gated by their ownAllowedRequestsflags rather than falling back to theresponses(create) flag. Previously, a granular mode was inferred when any lifecycle flag was set, with unset verbs falling back to theresponsesflag. This change removes that fallback logic and makes each lifecycle verb require its own explicit flag.Changes
granularResponsesLifecycleUsed()helper and the associated fallback logic inIsOperationAllowed. Each ofResponsesRetrieveRequest,ResponsesDeleteRequest,ResponsesCancelRequest, andResponsesInputItemsRequestnow maps directly to its own boolean flag with no inheritance fromResponses.responses_retrieve,responses_delete,responses_cancel, andresponses_input_itemsto the UI'sRequestTypeunion,AllowedRequestsinterface, Zod schema, request type lists, labels, and color mappings.falsefor existing providers andtruewhen creating a new custom provider.PathOverrideUnsupportedset in the allowed requests UI to suppress the path-override control for lifecycle request types, since those paths embed a response ID and a static override cannot produce a valid URL.PROVIDER_SUPPORTED_REQUESTS.Type of change
Affected areas
How to test
Verify that a provider with only
responses: true(and no lifecycle flags) now denies retrieve/delete/cancel/input_items requests. Verify that explicitly settingresponses_retrieve: trueallows retrieve while leaving delete/cancel/input_items denied.Breaking changes
Any existing configuration that relied on
responses: trueimplicitly allowing lifecycle verbs (retrieve, delete, cancel, input_items) will now deny those operations. Operators must explicitly addresponses_retrieve,responses_delete,responses_cancel, and/orresponses_input_itemsto theirAllowedRequestsconfiguration to restore access.Security considerations
The change is strictly more restrictive by default. Lifecycle operations that were previously allowed implicitly through the
responsesflag now require explicit opt-in, reducing the risk of unintended access to stored response data.Checklist
docs/contributing/README.mdand followed the guidelines