feat: model select + paywall changes, authoritative frontend models, model router#4077
Open
ehayes2000 wants to merge 1 commit into
Open
feat: model select + paywall changes, authoritative frontend models, model router#4077ehayes2000 wants to merge 1 commit into
ehayes2000 wants to merge 1 commit into
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR removes client-side prechecks for paid chat access, adds frontend model availability fetching and locked-model selection UI, and introduces a backend |
![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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
js/app/packages/service-clients/service-cognition/openapi.json (1)
1276-1321:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign the no-access model status with the backend.
This request now accepts provider IDs, so paywall branching needs a stable status. The backend access check returns
StatusCode::FORBIDDENwhenmodel_access.has_access(&request.model)fails, but this operation documents402 Payment requiredfor that same case. Either return 402 from the handler or document/handle 403 consistently.Also applies to: 2787-2790
🤖 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 `@js/app/packages/service-clients/service-cognition/openapi.json` around lines 1276 - 1321, The OpenAPI specification for the send_chat_message operation documents a 402 status code with description "Payment required — user lacks access to the requested model", but the backend handler returns 403 (FORBIDDEN) when model_access.has_access check fails. Choose one approach: either modify the handler to return 402 to match the documented behavior, or update the OpenAPI documentation to reflect that 403 (Forbidden) is returned for access check failures. Ensure this change is applied consistently at all affected locations in the specification where this mismatch occurs.rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs (1)
1-59:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThis change set modifies a crate marked as non-modifiable by repository policy.
These edits are under
rust/cloud-storage/ai_toolset/**, which is explicitly designated as off-limits in the provided project rules. Please move the implementation to an allowed location or document an explicit exception before merge.As per coding guidelines,
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs` around lines 1 - 59, The changes you've made to the ai_toolset crate (including the Validate trait and validate_tool_schema function) violate repository policy, which explicitly prohibits modifications to rust/cloud-storage/ai_toolset/**. Move the implementation changes to an allowed location within the repository, or obtain and document an explicit exception to the policy before this code can be merged. If these are new additions rather than modifications to existing code, clarify the business justification and get approval from the appropriate stakeholders.Source: Coding guidelines
rust/cloud-storage/ai_toolset/src/schema/error.rs (1)
1-45:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDo not modify
ai_toolsetin this PR.Line 1 is within
rust/cloud-storage/ai_toolset/**, which the project guidelines explicitly mark as immutable. Please move this behavior to an allowed crate or revert these changes.As per coding guidelines:
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/error.rs` around lines 1 - 45, The ValidationError enum and its modifications are located in the immutable ai_toolset crate, which violates project guidelines that explicitly prohibit any changes to rust/cloud-storage/ai_toolset/**. Revert all changes made to this file, or if the functionality is necessary, move the ValidationError enum and related code to an allowed crate outside the ai_toolset directory.Source: Coding guidelines
rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs (1)
1-234:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftBlocked: changes landed in a forbidden crate path.
This file is inside
rust/cloud-storage/ai_toolset/**, which is marked as immutable by project policy. Please move this implementation out ofai_toolsetand revert direct edits in this crate before merge.As per coding guidelines:
rust/cloud-storage/ai_toolset/**: You are not allowed to _EVER_ change this crate.🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs` around lines 1 - 234, The file rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs containing the FrontendSchemas, FrontendSchemasBuilder, and ToolSchemaGenerator implementations is located in the ai_toolset crate, which is marked as immutable by project policy. Move all the code in this file (FrontendToolEntry, ToolSchemaGenerator trait, AsyncToolCollection implementation, FrontendSchemas struct with its methods like mangle_collisions, find_owning_tool, value_contains_ref, rename_refs, and rename_refs_in_value, along with FrontendSchemasBuilder) to a different, mutable crate location outside of ai_toolset, then revert this file to its original state before your changes.Source: Coding guidelines
🧹 Nitpick comments (1)
js/app/packages/core/component/AI/component/input/ChatInput.tsx (1)
63-69: ⚡ Quick winReplace derived-state
createEffectwith a derived signal/setter flow.This effect is deriving and mutating local app state from reactive inputs, which conflicts with the SolidJS guideline for this codebase. Prefer a derived signal/wrapped setter approach here.
As per coding guidelines: “Avoid createEffect in SolidJs… Do not use to derive state or trigger updates—use a derived signal or wrap the setter instead”.
🤖 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 `@js/app/packages/core/component/AI/component/input/ChatInput.tsx` around lines 63 - 69, The createEffect in the ChatInput component is using a side-effect to derive and mutate state by calling input.setModel() based on modelOptions() and model() reactivity, which violates SolidJS coding guidelines. Replace this createEffect with a derived signal approach: create a derived signal that computes the appropriate model ID based on the available options and current model selection logic, then wrap the input.setModel setter with this derived signal to handle the state update in a declarative, non-effect-based manner. The derived signal should encapsulate the logic that checks option availability and determines the first available model ID.Source: Coding guidelines
🤖 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 `@js/app/packages/core/component/AI/component/input/ChatInput.tsx`:
- Around line 54-69: The models query needs to be resolved before allowing
message send to prevent sending a locked model. The createEffect fallback exits
early when modelOptions is empty (before the query finishes loading), but
canSendMessage still allows sending with the current model selection without
validation. Block sending in canSendMessage by checking the models query loading
state (modelsQuery.isPending or isLoading) or ensuring a valid model is
confirmed available before the send is allowed. This prevents sending a model
that hasn't been validated as available to the user yet, avoiding 403 errors and
ensuring the correct locked-model or paywall UX is shown instead.
In `@js/app/packages/service-clients/service-cognition/openapi.json`:
- Around line 373-394: The OpenAPI specification for the `/chats/models`
endpoint is missing a 401 response definition, even though the endpoint is
protected by ensure_user_exists middleware at runtime. Add a 401 response object
to the responses section alongside the existing 200 and 500 responses, with an
appropriate description and schema (typically application/json with an error
message or empty description with text/plain) to accurately document that
unauthenticated requests will receive a 401 Unauthorized response.
- Around line 1445-1454: The enum array in the AgentModel definition contains
duplicate values that violate OpenAPI schema constraints. Remove the duplicate
instances so that `claude-opus-4-8` and `claude-haiku-4-5` each appear only once
in the enum list. Keep the first occurrence of each duplicated value and remove
the subsequent occurrences.
In `@rust/cloud-storage/agent/Cargo.toml`:
- Around line 17-19: The rig-core dependency declaration in Cargo.toml currently
uses branch = "feat/responses-api-non-strict-tools" which creates a hybrid
pinning strategy between the lockfile and manifest. Replace the branch attribute
with rev = "6deadfc6f9b432c849ea79733a549f46407530b7" to make the pinned commit
explicit and single-sourced in the manifest, improving auditability and ensuring
the manifest and lockfile stay consistent without requiring manual
synchronization.
In `@rust/cloud-storage/agent/src/agent_loop.rs`:
- Around line 263-289: The StreamBridge attached at line 263 already emits usage
information via the on_stream_completion_response_finish hook, so the subsequent
emission of StreamPart::Usage in the FinalResponse match arm (lines 284-289)
creates a duplicate. Remove the code block that yields Ok(StreamPart::Usage)
from the FinalResponse match case, as the usage tokens are already being
properly emitted through the StreamBridge hook mechanism.
In `@rust/cloud-storage/agent/src/model/router.rs`:
- Around line 96-106: The route_or_default method currently catches all errors
from the self.route(model) call and silently returns an Anthropic default, which
masks real provider unavailability and configuration issues. Modify the error
handling to distinguish between error types: only return the default Anthropic
model for UnknownModel errors, while allowing ProviderUnavailable errors to
propagate up so they are not hidden by the automatic fallback to the default
model.
In `@rust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rs`:
- Line 6: The std::fs::create_dir call for creating the "schemas" directory
fails when the directory already exists, breaking repeated runs of the schema
generation tool. Replace the create_dir call with create_dir_all, which is
idempotent and will succeed whether the directory exists or not, making the
schema generation process rerunnable without errors.
In `@rust/cloud-storage/ai_toolset/src/schema/generate.rs`:
- Around line 1-91: The changes made to
`rust/cloud-storage/ai_toolset/src/schema/generate.rs` violate the repository
guardrail that forbids modifications to the `ai_toolset` crate. Revert all
changes in the `ai_toolset` directory and refactor the modifications to be made
in an allowed external crate or interface that wraps or extends the
functionality, ensuring the core `ai_toolset` crate remains unmodified as
required by project policy.
In `@rust/cloud-storage/ai_toolset/src/schema/mod.rs`:
- Around line 9-20: The file rust/cloud-storage/ai_toolset/src/schema/mod.rs is
in a crate designated as immutable by project rules and cannot be modified.
Either revert all changes made to files under rust/cloud-storage/ai_toolset/**,
move the implementation to an allowed crate and update the corresponding imports
and exports, or obtain explicit written approval from the project maintainers to
document an exception for this PR with clear justification for why the
immutability rule must be broken.
In `@rust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rs`:
- Around line 70-96: The transform adds "null" to the type for nullable
properties but doesn't handle the case where a property has both type and const
constraints. When const is present, adding "null" to type is insufficient
because const still restricts the value to the original constant. After the type
union logic (around lines 70-95), add a check similar to the enum handling
(lines 97-101) to detect if a const field exists. If it does, convert it to an
enum array containing the original constant value and null, then remove the
const field, ensuring the property truly becomes nullable.
In `@rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs`:
- Around line 1-107: The new implementation of the StripUnsupported struct and
its Transform trait implementation has been added to the ai_toolset crate, which
violates the repository rule that this crate must never be modified. Either
revert all changes in this file
(rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs)
entirely, or move the StripUnsupported struct and its Transform implementation
to an allowed crate outside of ai_toolset, keeping the ai_toolset directory
completely untouched.
In `@rust/cloud-storage/chat/src/outbound/postgres/mod.rs`:
- Line 131: The model field assignment is currently always using FALLBACK_MODEL,
which causes reopened chats to lose their originally selected provider and model
information. Instead of unconditionally setting model to
FALLBACK_MODEL.to_string(), check if a persisted chat model exists in the
fetched data and use that value when available, only falling back to
FALLBACK_MODEL for legacy database rows that do not have a stored model. This
preserves the original model selection for existing chats while providing a
fallback for historical data.
In
`@rust/cloud-storage/document_cognition_service/src/api/stream/chat_message/mod.rs`:
- Around line 182-189: The status code returned in the ChatMessageError when
model access is denied does not match the endpoint's documented contract. In the
model access validation block where has_access is checked (around lines
182-189), the status field is set to StatusCode::FORBIDDEN, but the endpoint
documentation indicates that entitlement/access denial should return 402
(Payment Required) to align with the paywall UX contract. Update the status
field in the ChatMessageError returned from the model access check to use the
correct status code that matches the documented contract at Line 133.
In
`@rust/cloud-storage/document_cognition_service/src/api/structured_completion.rs`:
- Line 23: The structured completion handler accepts a model id in the request
struct but ignores it, always using model_access.model() instead. To fix this,
validate and access-check the request.model field to ensure it is a known and
authorized model id, resolve it to the corresponding runtime model, and use that
resolved model value for both agent phases in the handler. Return a 400 status
code if the model id is unknown or invalid. This ensures the handler honors the
caller's requested model rather than silently substituting a potentially
different model.
In
`@rust/cloud-storage/scheduled_action/src/outbound/inprocess_executor/agent_task.rs`:
- Line 107: The AgentTask.model is not being authorized before being persisted
or executed, allowing users to schedule unauthorized premium provider models.
Add ChatModelAccess authorization checks for action.owner at all affected
locations: (1) before storing messages at the line with model:
agent_task.model.clone() around line 107, (2) at the second affected site around
line 132-133, and (3) before AgentLoop::with_model at the third affected site
around line 193. Either implement access validation checks at each location
before using the model, or enforce authorization at task creation/update time
and validate again at dispatch time to prevent unauthorized model execution.
---
Outside diff comments:
In `@js/app/packages/service-clients/service-cognition/openapi.json`:
- Around line 1276-1321: The OpenAPI specification for the send_chat_message
operation documents a 402 status code with description "Payment required — user
lacks access to the requested model", but the backend handler returns 403
(FORBIDDEN) when model_access.has_access check fails. Choose one approach:
either modify the handler to return 402 to match the documented behavior, or
update the OpenAPI documentation to reflect that 403 (Forbidden) is returned for
access check failures. Ensure this change is applied consistently at all
affected locations in the specification where this mismatch occurs.
In `@rust/cloud-storage/ai_toolset/src/schema/error.rs`:
- Around line 1-45: The ValidationError enum and its modifications are located
in the immutable ai_toolset crate, which violates project guidelines that
explicitly prohibit any changes to rust/cloud-storage/ai_toolset/**. Revert all
changes made to this file, or if the functionality is necessary, move the
ValidationError enum and related code to an allowed crate outside the ai_toolset
directory.
In `@rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs`:
- Around line 1-234: The file
rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs containing the
FrontendSchemas, FrontendSchemasBuilder, and ToolSchemaGenerator implementations
is located in the ai_toolset crate, which is marked as immutable by project
policy. Move all the code in this file (FrontendToolEntry, ToolSchemaGenerator
trait, AsyncToolCollection implementation, FrontendSchemas struct with its
methods like mangle_collisions, find_owning_tool, value_contains_ref,
rename_refs, and rename_refs_in_value, along with FrontendSchemasBuilder) to a
different, mutable crate location outside of ai_toolset, then revert this file
to its original state before your changes.
In `@rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs`:
- Around line 1-59: The changes you've made to the ai_toolset crate (including
the Validate trait and validate_tool_schema function) violate repository policy,
which explicitly prohibits modifications to rust/cloud-storage/ai_toolset/**.
Move the implementation changes to an allowed location within the repository, or
obtain and document an explicit exception to the policy before this code can be
merged. If these are new additions rather than modifications to existing code,
clarify the business justification and get approval from the appropriate
stakeholders.
---
Nitpick comments:
In `@js/app/packages/core/component/AI/component/input/ChatInput.tsx`:
- Around line 63-69: The createEffect in the ChatInput component is using a
side-effect to derive and mutate state by calling input.setModel() based on
modelOptions() and model() reactivity, which violates SolidJS coding guidelines.
Replace this createEffect with a derived signal approach: create a derived
signal that computes the appropriate model ID based on the available options and
current model selection logic, then wrap the input.setModel setter with this
derived signal to handle the state update in a declarative, non-effect-based
manner. The derived signal should encapsulate the logic that checks option
availability and determines the first available model ID.
🪄 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: 1ae0d84a-b9c7-4309-bbfc-066f200a1f51
⛔ Files ignored due to path filters (20)
js/app/packages/core/component/AI/assets/openai.svgis excluded by!**/*.svgjs/app/packages/service-clients/service-cognition/generated/client.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/agentModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/chatResponse.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/chatResponseModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/httpSendChatMessageRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/model.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/modelAccess.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/modelsResponse.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/newChatMessage.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/sendChatMessagePayloadAllOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/streamErrorOneOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/structuredCompletionRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/toolSchema.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/toolSchemas.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/agentModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/agentTask.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/index.tsis excluded by!**/generated/**rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (100)
js/app/packages/app/component/SoupChatInput.tsxjs/app/packages/app/component/home/home.tsxjs/app/packages/core/component/AI/component/input/ChatInput.tsxjs/app/packages/core/component/AI/component/input/ModelSelector.tsxjs/app/packages/core/component/AI/component/input/buildRequest.tsjs/app/packages/core/component/AI/constant/model.tsjs/app/packages/core/util/create.tsjs/app/packages/queries/chat.tsjs/app/packages/service-clients/service-cognition/client.tsjs/app/packages/service-clients/service-cognition/openapi.jsonjs/app/packages/service-clients/service-scheduled-action/openapi.jsonjs/app/scripts/generate-dcs-tools.tsrust/cloud-storage/agent/Cargo.tomlrust/cloud-storage/agent/src/agent_loop.rsrust/cloud-storage/agent/src/anthropic_model.rsrust/cloud-storage/agent/src/anthropic_model/test.rsrust/cloud-storage/agent/src/completion.rsrust/cloud-storage/agent/src/convert.rsrust/cloud-storage/agent/src/convert/test.rsrust/cloud-storage/agent/src/error.rsrust/cloud-storage/agent/src/lib.rsrust/cloud-storage/agent/src/model.rsrust/cloud-storage/agent/src/model/anthropic.rsrust/cloud-storage/agent/src/model/mod.rsrust/cloud-storage/agent/src/model/openai.rsrust/cloud-storage/agent/src/model/router.rsrust/cloud-storage/agent/src/model/router/anthropic.rsrust/cloud-storage/agent/src/model/router/openai.rsrust/cloud-storage/agent/src/model/router/test.rsrust/cloud-storage/agent/src/model/test.rsrust/cloud-storage/agent/src/model/types.rsrust/cloud-storage/agent/src/router.rsrust/cloud-storage/agent/src/structured_output.rsrust/cloud-storage/agent/src/tool_adapter.rsrust/cloud-storage/agent/src/tool_adapter/test.rsrust/cloud-storage/ai_tools/src/bin/gen_ai_request_schemas.rsrust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rsrust/cloud-storage/ai_tools/src/lib.rsrust/cloud-storage/ai_tools/src/search/search_service/content.rsrust/cloud-storage/ai_tools/src/search/search_service/name.rsrust/cloud-storage/ai_toolset/src/schema/error.rsrust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rsrust/cloud-storage/ai_toolset/src/schema/generate.rsrust/cloud-storage/ai_toolset/src/schema/generate/test.rsrust/cloud-storage/ai_toolset/src/schema/mod.rsrust/cloud-storage/ai_toolset/src/schema/phantom_tool.rsrust/cloud-storage/ai_toolset/src/schema/transform/additional_properties.rsrust/cloud-storage/ai_toolset/src/schema/transform/mod.rsrust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rsrust/cloud-storage/ai_toolset/src/schema/transform/ref_siblings.rsrust/cloud-storage/ai_toolset/src/schema/transform/required.rsrust/cloud-storage/ai_toolset/src/schema/transform/rewrite_one_of.rsrust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rsrust/cloud-storage/ai_toolset/src/schema/validate/extract.rsrust/cloud-storage/ai_toolset/src/schema/validate/mod.rsrust/cloud-storage/ai_toolset/src/schema/validate/one_of.rsrust/cloud-storage/ai_toolset/src/schema/validate/recursive.rsrust/cloud-storage/ai_toolset/src/schema/validate/recursive/test.rsrust/cloud-storage/ai_toolset/src/schema/validate/strict.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/mod.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/object.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/tool_async.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/util.rsrust/cloud-storage/ai_toolset/src/toolset/types.rsrust/cloud-storage/call/src/inbound/toolset/test.rsrust/cloud-storage/channels/src/inbound/toolset/test.rsrust/cloud-storage/chat/src/domain/models/chat.rsrust/cloud-storage/chat/src/domain/models/mod.rsrust/cloud-storage/chat/src/domain/models/model_access.rsrust/cloud-storage/chat/src/domain/ports/mod.rsrust/cloud-storage/chat/src/domain/ports/model_access.rsrust/cloud-storage/chat/src/domain/service/mod.rsrust/cloud-storage/chat/src/domain/service/model_access.rsrust/cloud-storage/chat/src/domain/service/model_access/test.rsrust/cloud-storage/chat/src/inbound/http/extractors.rsrust/cloud-storage/chat/src/inbound/http/mod.rsrust/cloud-storage/chat/src/inbound/http/models.rsrust/cloud-storage/chat/src/inbound/toolset/test.rsrust/cloud-storage/chat/src/outbound/postgres/mod.rsrust/cloud-storage/document_cognition_service/src/api/chats/mod.rsrust/cloud-storage/document_cognition_service/src/api/stream/chat_message/mod.rsrust/cloud-storage/document_cognition_service/src/api/stream/util/chat_message/mod.rsrust/cloud-storage/document_cognition_service/src/api/structured_completion.rsrust/cloud-storage/document_cognition_service/src/api/swagger.rsrust/cloud-storage/document_cognition_service/src/api/utils/log.rsrust/cloud-storage/document_cognition_service/src/core/model.rsrust/cloud-storage/document_cognition_service/src/model/chats.rsrust/cloud-storage/document_cognition_service/src/model/stream.rsrust/cloud-storage/document_cognition_service/src/models_bin.rsrust/cloud-storage/document_cognition_service/src/service/get_chat.rsrust/cloud-storage/documents/src/inbound/toolset/test.rsrust/cloud-storage/email/src/inbound/toolset/test.rsrust/cloud-storage/macro_db_client/src/dcs/create_chat.rsrust/cloud-storage/model/src/chat/message.rsrust/cloud-storage/notification/src/inbound/ai_tool/test.rsrust/cloud-storage/properties/src/inbound/toolset/test.rsrust/cloud-storage/scheduled_action/src/domain/models.rsrust/cloud-storage/scheduled_action/src/outbound/inprocess_executor/agent_task.rsrust/cloud-storage/soup/src/inbound/toolset/test.rsrust/cloud-storage/teams/src/inbound/toolset/test.rs
💤 Files with no reviewable changes (7)
- rust/cloud-storage/agent/src/anthropic_model/test.rs
- rust/cloud-storage/agent/src/anthropic_model.rs
- rust/cloud-storage/agent/src/model.rs
- js/app/packages/app/component/home/home.tsx
- rust/cloud-storage/ai_toolset/src/toolset/tool_object/object.rs
- js/app/packages/app/component/SoupChatInput.tsx
- rust/cloud-storage/ai_toolset/src/toolset/tool_object/util.rs
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
js/app/packages/service-clients/service-cognition/openapi.json (1)
1276-1321:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign the no-access model status with the backend.
This request now accepts provider IDs, so paywall branching needs a stable status. The backend access check returns
StatusCode::FORBIDDENwhenmodel_access.has_access(&request.model)fails, but this operation documents402 Payment requiredfor that same case. Either return 402 from the handler or document/handle 403 consistently.Also applies to: 2787-2790
🤖 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 `@js/app/packages/service-clients/service-cognition/openapi.json` around lines 1276 - 1321, The OpenAPI specification for the send_chat_message operation documents a 402 status code with description "Payment required — user lacks access to the requested model", but the backend handler returns 403 (FORBIDDEN) when model_access.has_access check fails. Choose one approach: either modify the handler to return 402 to match the documented behavior, or update the OpenAPI documentation to reflect that 403 (Forbidden) is returned for access check failures. Ensure this change is applied consistently at all affected locations in the specification where this mismatch occurs.rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs (1)
1-59:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThis change set modifies a crate marked as non-modifiable by repository policy.
These edits are under
rust/cloud-storage/ai_toolset/**, which is explicitly designated as off-limits in the provided project rules. Please move the implementation to an allowed location or document an explicit exception before merge.As per coding guidelines,
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs` around lines 1 - 59, The changes you've made to the ai_toolset crate (including the Validate trait and validate_tool_schema function) violate repository policy, which explicitly prohibits modifications to rust/cloud-storage/ai_toolset/**. Move the implementation changes to an allowed location within the repository, or obtain and document an explicit exception to the policy before this code can be merged. If these are new additions rather than modifications to existing code, clarify the business justification and get approval from the appropriate stakeholders.Source: Coding guidelines
rust/cloud-storage/ai_toolset/src/schema/error.rs (1)
1-45:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDo not modify
ai_toolsetin this PR.Line 1 is within
rust/cloud-storage/ai_toolset/**, which the project guidelines explicitly mark as immutable. Please move this behavior to an allowed crate or revert these changes.As per coding guidelines:
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/error.rs` around lines 1 - 45, The ValidationError enum and its modifications are located in the immutable ai_toolset crate, which violates project guidelines that explicitly prohibit any changes to rust/cloud-storage/ai_toolset/**. Revert all changes made to this file, or if the functionality is necessary, move the ValidationError enum and related code to an allowed crate outside the ai_toolset directory.Source: Coding guidelines
rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs (1)
1-234:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftBlocked: changes landed in a forbidden crate path.
This file is inside
rust/cloud-storage/ai_toolset/**, which is marked as immutable by project policy. Please move this implementation out ofai_toolsetand revert direct edits in this crate before merge.As per coding guidelines:
rust/cloud-storage/ai_toolset/**: You are not allowed to _EVER_ change this crate.🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs` around lines 1 - 234, The file rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs containing the FrontendSchemas, FrontendSchemasBuilder, and ToolSchemaGenerator implementations is located in the ai_toolset crate, which is marked as immutable by project policy. Move all the code in this file (FrontendToolEntry, ToolSchemaGenerator trait, AsyncToolCollection implementation, FrontendSchemas struct with its methods like mangle_collisions, find_owning_tool, value_contains_ref, rename_refs, and rename_refs_in_value, along with FrontendSchemasBuilder) to a different, mutable crate location outside of ai_toolset, then revert this file to its original state before your changes.Source: Coding guidelines
🧹 Nitpick comments (1)
js/app/packages/core/component/AI/component/input/ChatInput.tsx (1)
63-69: ⚡ Quick winReplace derived-state
createEffectwith a derived signal/setter flow.This effect is deriving and mutating local app state from reactive inputs, which conflicts with the SolidJS guideline for this codebase. Prefer a derived signal/wrapped setter approach here.
As per coding guidelines: “Avoid createEffect in SolidJs… Do not use to derive state or trigger updates—use a derived signal or wrap the setter instead”.
🤖 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 `@js/app/packages/core/component/AI/component/input/ChatInput.tsx` around lines 63 - 69, The createEffect in the ChatInput component is using a side-effect to derive and mutate state by calling input.setModel() based on modelOptions() and model() reactivity, which violates SolidJS coding guidelines. Replace this createEffect with a derived signal approach: create a derived signal that computes the appropriate model ID based on the available options and current model selection logic, then wrap the input.setModel setter with this derived signal to handle the state update in a declarative, non-effect-based manner. The derived signal should encapsulate the logic that checks option availability and determines the first available model ID.Source: Coding guidelines
🤖 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 `@js/app/packages/core/component/AI/component/input/ChatInput.tsx`:
- Around line 54-69: The models query needs to be resolved before allowing
message send to prevent sending a locked model. The createEffect fallback exits
early when modelOptions is empty (before the query finishes loading), but
canSendMessage still allows sending with the current model selection without
validation. Block sending in canSendMessage by checking the models query loading
state (modelsQuery.isPending or isLoading) or ensuring a valid model is
confirmed available before the send is allowed. This prevents sending a model
that hasn't been validated as available to the user yet, avoiding 403 errors and
ensuring the correct locked-model or paywall UX is shown instead.
In `@js/app/packages/service-clients/service-cognition/openapi.json`:
- Around line 373-394: The OpenAPI specification for the `/chats/models`
endpoint is missing a 401 response definition, even though the endpoint is
protected by ensure_user_exists middleware at runtime. Add a 401 response object
to the responses section alongside the existing 200 and 500 responses, with an
appropriate description and schema (typically application/json with an error
message or empty description with text/plain) to accurately document that
unauthenticated requests will receive a 401 Unauthorized response.
- Around line 1445-1454: The enum array in the AgentModel definition contains
duplicate values that violate OpenAPI schema constraints. Remove the duplicate
instances so that `claude-opus-4-8` and `claude-haiku-4-5` each appear only once
in the enum list. Keep the first occurrence of each duplicated value and remove
the subsequent occurrences.
In `@rust/cloud-storage/agent/Cargo.toml`:
- Around line 17-19: The rig-core dependency declaration in Cargo.toml currently
uses branch = "feat/responses-api-non-strict-tools" which creates a hybrid
pinning strategy between the lockfile and manifest. Replace the branch attribute
with rev = "6deadfc6f9b432c849ea79733a549f46407530b7" to make the pinned commit
explicit and single-sourced in the manifest, improving auditability and ensuring
the manifest and lockfile stay consistent without requiring manual
synchronization.
In `@rust/cloud-storage/agent/src/agent_loop.rs`:
- Around line 263-289: The StreamBridge attached at line 263 already emits usage
information via the on_stream_completion_response_finish hook, so the subsequent
emission of StreamPart::Usage in the FinalResponse match arm (lines 284-289)
creates a duplicate. Remove the code block that yields Ok(StreamPart::Usage)
from the FinalResponse match case, as the usage tokens are already being
properly emitted through the StreamBridge hook mechanism.
In `@rust/cloud-storage/agent/src/model/router.rs`:
- Around line 96-106: The route_or_default method currently catches all errors
from the self.route(model) call and silently returns an Anthropic default, which
masks real provider unavailability and configuration issues. Modify the error
handling to distinguish between error types: only return the default Anthropic
model for UnknownModel errors, while allowing ProviderUnavailable errors to
propagate up so they are not hidden by the automatic fallback to the default
model.
In `@rust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rs`:
- Line 6: The std::fs::create_dir call for creating the "schemas" directory
fails when the directory already exists, breaking repeated runs of the schema
generation tool. Replace the create_dir call with create_dir_all, which is
idempotent and will succeed whether the directory exists or not, making the
schema generation process rerunnable without errors.
In `@rust/cloud-storage/ai_toolset/src/schema/generate.rs`:
- Around line 1-91: The changes made to
`rust/cloud-storage/ai_toolset/src/schema/generate.rs` violate the repository
guardrail that forbids modifications to the `ai_toolset` crate. Revert all
changes in the `ai_toolset` directory and refactor the modifications to be made
in an allowed external crate or interface that wraps or extends the
functionality, ensuring the core `ai_toolset` crate remains unmodified as
required by project policy.
In `@rust/cloud-storage/ai_toolset/src/schema/mod.rs`:
- Around line 9-20: The file rust/cloud-storage/ai_toolset/src/schema/mod.rs is
in a crate designated as immutable by project rules and cannot be modified.
Either revert all changes made to files under rust/cloud-storage/ai_toolset/**,
move the implementation to an allowed crate and update the corresponding imports
and exports, or obtain explicit written approval from the project maintainers to
document an exception for this PR with clear justification for why the
immutability rule must be broken.
In `@rust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rs`:
- Around line 70-96: The transform adds "null" to the type for nullable
properties but doesn't handle the case where a property has both type and const
constraints. When const is present, adding "null" to type is insufficient
because const still restricts the value to the original constant. After the type
union logic (around lines 70-95), add a check similar to the enum handling
(lines 97-101) to detect if a const field exists. If it does, convert it to an
enum array containing the original constant value and null, then remove the
const field, ensuring the property truly becomes nullable.
In `@rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs`:
- Around line 1-107: The new implementation of the StripUnsupported struct and
its Transform trait implementation has been added to the ai_toolset crate, which
violates the repository rule that this crate must never be modified. Either
revert all changes in this file
(rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs)
entirely, or move the StripUnsupported struct and its Transform implementation
to an allowed crate outside of ai_toolset, keeping the ai_toolset directory
completely untouched.
In `@rust/cloud-storage/chat/src/outbound/postgres/mod.rs`:
- Line 131: The model field assignment is currently always using FALLBACK_MODEL,
which causes reopened chats to lose their originally selected provider and model
information. Instead of unconditionally setting model to
FALLBACK_MODEL.to_string(), check if a persisted chat model exists in the
fetched data and use that value when available, only falling back to
FALLBACK_MODEL for legacy database rows that do not have a stored model. This
preserves the original model selection for existing chats while providing a
fallback for historical data.
In
`@rust/cloud-storage/document_cognition_service/src/api/stream/chat_message/mod.rs`:
- Around line 182-189: The status code returned in the ChatMessageError when
model access is denied does not match the endpoint's documented contract. In the
model access validation block where has_access is checked (around lines
182-189), the status field is set to StatusCode::FORBIDDEN, but the endpoint
documentation indicates that entitlement/access denial should return 402
(Payment Required) to align with the paywall UX contract. Update the status
field in the ChatMessageError returned from the model access check to use the
correct status code that matches the documented contract at Line 133.
In
`@rust/cloud-storage/document_cognition_service/src/api/structured_completion.rs`:
- Line 23: The structured completion handler accepts a model id in the request
struct but ignores it, always using model_access.model() instead. To fix this,
validate and access-check the request.model field to ensure it is a known and
authorized model id, resolve it to the corresponding runtime model, and use that
resolved model value for both agent phases in the handler. Return a 400 status
code if the model id is unknown or invalid. This ensures the handler honors the
caller's requested model rather than silently substituting a potentially
different model.
In
`@rust/cloud-storage/scheduled_action/src/outbound/inprocess_executor/agent_task.rs`:
- Line 107: The AgentTask.model is not being authorized before being persisted
or executed, allowing users to schedule unauthorized premium provider models.
Add ChatModelAccess authorization checks for action.owner at all affected
locations: (1) before storing messages at the line with model:
agent_task.model.clone() around line 107, (2) at the second affected site around
line 132-133, and (3) before AgentLoop::with_model at the third affected site
around line 193. Either implement access validation checks at each location
before using the model, or enforce authorization at task creation/update time
and validate again at dispatch time to prevent unauthorized model execution.
---
Outside diff comments:
In `@js/app/packages/service-clients/service-cognition/openapi.json`:
- Around line 1276-1321: The OpenAPI specification for the send_chat_message
operation documents a 402 status code with description "Payment required — user
lacks access to the requested model", but the backend handler returns 403
(FORBIDDEN) when model_access.has_access check fails. Choose one approach:
either modify the handler to return 402 to match the documented behavior, or
update the OpenAPI documentation to reflect that 403 (Forbidden) is returned for
access check failures. Ensure this change is applied consistently at all
affected locations in the specification where this mismatch occurs.
In `@rust/cloud-storage/ai_toolset/src/schema/error.rs`:
- Around line 1-45: The ValidationError enum and its modifications are located
in the immutable ai_toolset crate, which violates project guidelines that
explicitly prohibit any changes to rust/cloud-storage/ai_toolset/**. Revert all
changes made to this file, or if the functionality is necessary, move the
ValidationError enum and related code to an allowed crate outside the ai_toolset
directory.
In `@rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs`:
- Around line 1-234: The file
rust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rs containing the
FrontendSchemas, FrontendSchemasBuilder, and ToolSchemaGenerator implementations
is located in the ai_toolset crate, which is marked as immutable by project
policy. Move all the code in this file (FrontendToolEntry, ToolSchemaGenerator
trait, AsyncToolCollection implementation, FrontendSchemas struct with its
methods like mangle_collisions, find_owning_tool, value_contains_ref,
rename_refs, and rename_refs_in_value, along with FrontendSchemasBuilder) to a
different, mutable crate location outside of ai_toolset, then revert this file
to its original state before your changes.
In `@rust/cloud-storage/ai_toolset/src/schema/validate/mod.rs`:
- Around line 1-59: The changes you've made to the ai_toolset crate (including
the Validate trait and validate_tool_schema function) violate repository policy,
which explicitly prohibits modifications to rust/cloud-storage/ai_toolset/**.
Move the implementation changes to an allowed location within the repository, or
obtain and document an explicit exception to the policy before this code can be
merged. If these are new additions rather than modifications to existing code,
clarify the business justification and get approval from the appropriate
stakeholders.
---
Nitpick comments:
In `@js/app/packages/core/component/AI/component/input/ChatInput.tsx`:
- Around line 63-69: The createEffect in the ChatInput component is using a
side-effect to derive and mutate state by calling input.setModel() based on
modelOptions() and model() reactivity, which violates SolidJS coding guidelines.
Replace this createEffect with a derived signal approach: create a derived
signal that computes the appropriate model ID based on the available options and
current model selection logic, then wrap the input.setModel setter with this
derived signal to handle the state update in a declarative, non-effect-based
manner. The derived signal should encapsulate the logic that checks option
availability and determines the first available model ID.
🪄 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: 1ae0d84a-b9c7-4309-bbfc-066f200a1f51
⛔ Files ignored due to path filters (20)
js/app/packages/core/component/AI/assets/openai.svgis excluded by!**/*.svgjs/app/packages/service-clients/service-cognition/generated/client.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/agentModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/chatResponse.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/chatResponseModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/httpSendChatMessageRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/model.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/modelAccess.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/modelsResponse.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/newChatMessage.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/sendChatMessagePayloadAllOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/streamErrorOneOf.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/structuredCompletionRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/toolSchema.tsis excluded by!**/generated/**js/app/packages/service-clients/service-cognition/generated/schemas/toolSchemas.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/agentModel.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/agentTask.tsis excluded by!**/generated/**js/app/packages/service-clients/service-scheduled-action/generated/schemas/index.tsis excluded by!**/generated/**rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (100)
js/app/packages/app/component/SoupChatInput.tsxjs/app/packages/app/component/home/home.tsxjs/app/packages/core/component/AI/component/input/ChatInput.tsxjs/app/packages/core/component/AI/component/input/ModelSelector.tsxjs/app/packages/core/component/AI/component/input/buildRequest.tsjs/app/packages/core/component/AI/constant/model.tsjs/app/packages/core/util/create.tsjs/app/packages/queries/chat.tsjs/app/packages/service-clients/service-cognition/client.tsjs/app/packages/service-clients/service-cognition/openapi.jsonjs/app/packages/service-clients/service-scheduled-action/openapi.jsonjs/app/scripts/generate-dcs-tools.tsrust/cloud-storage/agent/Cargo.tomlrust/cloud-storage/agent/src/agent_loop.rsrust/cloud-storage/agent/src/anthropic_model.rsrust/cloud-storage/agent/src/anthropic_model/test.rsrust/cloud-storage/agent/src/completion.rsrust/cloud-storage/agent/src/convert.rsrust/cloud-storage/agent/src/convert/test.rsrust/cloud-storage/agent/src/error.rsrust/cloud-storage/agent/src/lib.rsrust/cloud-storage/agent/src/model.rsrust/cloud-storage/agent/src/model/anthropic.rsrust/cloud-storage/agent/src/model/mod.rsrust/cloud-storage/agent/src/model/openai.rsrust/cloud-storage/agent/src/model/router.rsrust/cloud-storage/agent/src/model/router/anthropic.rsrust/cloud-storage/agent/src/model/router/openai.rsrust/cloud-storage/agent/src/model/router/test.rsrust/cloud-storage/agent/src/model/test.rsrust/cloud-storage/agent/src/model/types.rsrust/cloud-storage/agent/src/router.rsrust/cloud-storage/agent/src/structured_output.rsrust/cloud-storage/agent/src/tool_adapter.rsrust/cloud-storage/agent/src/tool_adapter/test.rsrust/cloud-storage/ai_tools/src/bin/gen_ai_request_schemas.rsrust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rsrust/cloud-storage/ai_tools/src/lib.rsrust/cloud-storage/ai_tools/src/search/search_service/content.rsrust/cloud-storage/ai_tools/src/search/search_service/name.rsrust/cloud-storage/ai_toolset/src/schema/error.rsrust/cloud-storage/ai_toolset/src/schema/frontend_typegen.rsrust/cloud-storage/ai_toolset/src/schema/generate.rsrust/cloud-storage/ai_toolset/src/schema/generate/test.rsrust/cloud-storage/ai_toolset/src/schema/mod.rsrust/cloud-storage/ai_toolset/src/schema/phantom_tool.rsrust/cloud-storage/ai_toolset/src/schema/transform/additional_properties.rsrust/cloud-storage/ai_toolset/src/schema/transform/mod.rsrust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rsrust/cloud-storage/ai_toolset/src/schema/transform/ref_siblings.rsrust/cloud-storage/ai_toolset/src/schema/transform/required.rsrust/cloud-storage/ai_toolset/src/schema/transform/rewrite_one_of.rsrust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rsrust/cloud-storage/ai_toolset/src/schema/validate/extract.rsrust/cloud-storage/ai_toolset/src/schema/validate/mod.rsrust/cloud-storage/ai_toolset/src/schema/validate/one_of.rsrust/cloud-storage/ai_toolset/src/schema/validate/recursive.rsrust/cloud-storage/ai_toolset/src/schema/validate/recursive/test.rsrust/cloud-storage/ai_toolset/src/schema/validate/strict.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/mod.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/object.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/tool_async.rsrust/cloud-storage/ai_toolset/src/toolset/tool_object/util.rsrust/cloud-storage/ai_toolset/src/toolset/types.rsrust/cloud-storage/call/src/inbound/toolset/test.rsrust/cloud-storage/channels/src/inbound/toolset/test.rsrust/cloud-storage/chat/src/domain/models/chat.rsrust/cloud-storage/chat/src/domain/models/mod.rsrust/cloud-storage/chat/src/domain/models/model_access.rsrust/cloud-storage/chat/src/domain/ports/mod.rsrust/cloud-storage/chat/src/domain/ports/model_access.rsrust/cloud-storage/chat/src/domain/service/mod.rsrust/cloud-storage/chat/src/domain/service/model_access.rsrust/cloud-storage/chat/src/domain/service/model_access/test.rsrust/cloud-storage/chat/src/inbound/http/extractors.rsrust/cloud-storage/chat/src/inbound/http/mod.rsrust/cloud-storage/chat/src/inbound/http/models.rsrust/cloud-storage/chat/src/inbound/toolset/test.rsrust/cloud-storage/chat/src/outbound/postgres/mod.rsrust/cloud-storage/document_cognition_service/src/api/chats/mod.rsrust/cloud-storage/document_cognition_service/src/api/stream/chat_message/mod.rsrust/cloud-storage/document_cognition_service/src/api/stream/util/chat_message/mod.rsrust/cloud-storage/document_cognition_service/src/api/structured_completion.rsrust/cloud-storage/document_cognition_service/src/api/swagger.rsrust/cloud-storage/document_cognition_service/src/api/utils/log.rsrust/cloud-storage/document_cognition_service/src/core/model.rsrust/cloud-storage/document_cognition_service/src/model/chats.rsrust/cloud-storage/document_cognition_service/src/model/stream.rsrust/cloud-storage/document_cognition_service/src/models_bin.rsrust/cloud-storage/document_cognition_service/src/service/get_chat.rsrust/cloud-storage/documents/src/inbound/toolset/test.rsrust/cloud-storage/email/src/inbound/toolset/test.rsrust/cloud-storage/macro_db_client/src/dcs/create_chat.rsrust/cloud-storage/model/src/chat/message.rsrust/cloud-storage/notification/src/inbound/ai_tool/test.rsrust/cloud-storage/properties/src/inbound/toolset/test.rsrust/cloud-storage/scheduled_action/src/domain/models.rsrust/cloud-storage/scheduled_action/src/outbound/inprocess_executor/agent_task.rsrust/cloud-storage/soup/src/inbound/toolset/test.rsrust/cloud-storage/teams/src/inbound/toolset/test.rs
💤 Files with no reviewable changes (7)
- rust/cloud-storage/agent/src/anthropic_model/test.rs
- rust/cloud-storage/agent/src/anthropic_model.rs
- rust/cloud-storage/agent/src/model.rs
- js/app/packages/app/component/home/home.tsx
- rust/cloud-storage/ai_toolset/src/toolset/tool_object/object.rs
- js/app/packages/app/component/SoupChatInput.tsx
- rust/cloud-storage/ai_toolset/src/toolset/tool_object/util.rs
🛑 Comments failed to post (15)
js/app/packages/core/component/AI/component/input/ChatInput.tsx (1)
54-69:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock sends until model availability is resolved and valid for the selected model.
While
useModelsQuery()is still unresolved,modelOptions()is empty (Line 54-59) and the fallback effect exits early (Line 65), butcanSendMessage()still allows send andsendInput.modeluses current selection (Line 118). This can send a locked model before entitlement data arrives, causing avoidable 403/error UX instead of the locked-model/paywall path.💡 Suggested fix
- const modelOptions = createMemo(() => - (modelsQuery.data?.models ?? []).map((m) => ({ + const modelOptions = createMemo(() => + (modelsQuery.data?.models ?? null)?.map((m) => ({ id: m.id as Model, available: m.available, })) ); + + const selectedModelAvailable = () => { + const options = modelOptions(); + if (!options) return false; // unresolved + return options.some((o) => o.id === model() && o.available); + }; @@ - const canSendMessage = () => - !isEmptyInput() && !generating() && !hasUploadingAttachments(); + const canSendMessage = () => + !!modelOptions() && + selectedModelAvailable() && + !isEmptyInput() && + !generating() && + !hasUploadingAttachments();Also applies to: 94-95, 116-119
🤖 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 `@js/app/packages/core/component/AI/component/input/ChatInput.tsx` around lines 54 - 69, The models query needs to be resolved before allowing message send to prevent sending a locked model. The createEffect fallback exits early when modelOptions is empty (before the query finishes loading), but canSendMessage still allows sending with the current model selection without validation. Block sending in canSendMessage by checking the models query loading state (modelsQuery.isPending or isLoading) or ensuring a valid model is confirmed available before the send is allowed. This prevents sending a model that hasn't been validated as available to the user yet, avoiding 403 errors and ensuring the correct locked-model or paywall UX is shown instead.js/app/packages/service-clients/service-cognition/openapi.json (2)
373-394:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the authenticated error response for
/chats/models.The router wraps this endpoint with
ensure_user_exists, so unauthenticated requests can return 401, but the spec only exposes 200/500. Add the 401 response so generated clients and docs match runtime behavior.Proposed OpenAPI response fix
"200": { "description": "", "content": { "application/json": { "schema": { "$ref": "`#/components/schemas/ModelsResponse`" } } } + }, + "401": { + "description": "", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } }, "500": { "description": "", "content": {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."responses": { "200": { "description": "", "content": { "application/json": { "schema": { "$ref": "`#/components/schemas/ModelsResponse`" } } } }, "401": { "description": "", "content": { "text/plain": { "schema": { "type": "string" } } } }, "500": { "description": "", "content": { "text/plain": { "schema": { "type": "string" } } } } }🤖 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 `@js/app/packages/service-clients/service-cognition/openapi.json` around lines 373 - 394, The OpenAPI specification for the `/chats/models` endpoint is missing a 401 response definition, even though the endpoint is protected by ensure_user_exists middleware at runtime. Add a 401 response object to the responses section alongside the existing 200 and 500 responses, with an appropriate description and schema (typically application/json with an error message or empty description with text/plain) to accurately document that unauthenticated requests will receive a 401 Unauthorized response.
1445-1454:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash python - <<'PY' import collections import json path = "js/app/packages/service-clients/service-cognition/openapi.json" with open(path) as f: spec = json.load(f) enum_values = spec["components"]["schemas"]["AgentModel"]["enum"] dupes = {value: count for value, count in collections.Counter(enum_values).items() if count > 1} print(dupes) raise SystemExit(1 if dupes else 0) PYRepository: macro-inc/macro
Length of output: 105
Remove duplicate enum values from
AgentModelin OpenAPI spec.The enum contains duplicate provider ids that violate OpenAPI schema constraints. Deduplicate by keeping only one instance of
claude-opus-4-8andclaude-haiku-4-5.🤖 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 `@js/app/packages/service-clients/service-cognition/openapi.json` around lines 1445 - 1454, The enum array in the AgentModel definition contains duplicate values that violate OpenAPI schema constraints. Remove the duplicate instances so that `claude-opus-4-8` and `claude-haiku-4-5` each appear only once in the enum list. Keep the first occurrence of each duplicated value and remove the subsequent occurrences.rust/cloud-storage/agent/Cargo.toml (1)
17-19:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "Checking manifest declaration:" rg -n '^\s*rig-core\s*=' rust/cloud-storage/agent/Cargo.toml echo echo "Checking lockfile source pin (if present):" LOCKFILE="$(fd '^Cargo\.lock$' | head -n1 || true)" if [ -n "${LOCKFILE}" ]; then rg -n 'name = "rig-core"|source = "git\+https://github.com/macro-inc/rig' "$LOCKFILE" else echo "Cargo.lock not found in provided checkout." fiRepository: macro-inc/macro
Length of output: 241
🏁 Script executed:
# Check if Cargo.lock exists at workspace root fd -t f '^Cargo\.lock$' rust/cloud-storage/Repository: macro-inc/macro
Length of output: 89
🏁 Script executed:
# If Cargo.lock exists, extract rig-core details if [ -f "rust/cloud-storage/Cargo.lock" ]; then echo "=== Lockfile rig-core entry ==="; rg -A 5 'name = "rig-core"' rust/cloud-storage/Cargo.lock | head -20 else echo "No Cargo.lock at workspace root" fiRepository: macro-inc/macro
Length of output: 307
🏁 Script executed:
# Check agent crate Cargo.toml for other git dependencies or conventions cat rust/cloud-storage/agent/Cargo.toml | head -30Repository: macro-inc/macro
Length of output: 1000
Use
revinstead ofbranchto make the pinned commit explicit in the manifest.The workspace maintains
Cargo.lock, which already resolves this dependency to commit6deadfc6f9b432c849ea79733a549f46407530b7. However, declaringbranch = "feat/responses-api-non-strict-tools"in the manifest creates a hybrid pinning strategy that requires lockfile and manifest to stay in sync. Usingrev = "6deadfc6f9b432c849ea79733a549f46407530b7"in the manifest makes the pinned commit explicit and single-sourced, improving auditability.Suggested manifest change
-rig-core = { git = "https://github.com/macro-inc/rig", branch = "feat/responses-api-non-strict-tools" } +rig-core = { git = "https://github.com/macro-inc/rig", rev = "6deadfc6f9b432c849ea79733a549f46407530b7" }🤖 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 `@rust/cloud-storage/agent/Cargo.toml` around lines 17 - 19, The rig-core dependency declaration in Cargo.toml currently uses branch = "feat/responses-api-non-strict-tools" which creates a hybrid pinning strategy between the lockfile and manifest. Replace the branch attribute with rev = "6deadfc6f9b432c849ea79733a549f46407530b7" to make the pinned commit explicit and single-sourced in the manifest, improving auditability and ensuring the manifest and lockfile stay consistent without requiring manual synchronization.rust/cloud-storage/agent/src/agent_loop.rs (1)
263-289:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winUsage tokens are emitted twice in the streaming path.
Line 263 attaches
StreamBridge, and Lines 284-289 emitStreamPart::Usageagain fromFinalResponse. This can duplicate usage events per request.Minimal fix
- Ok(MultiTurnStreamItem::FinalResponse(final_resp)) => { - let usage = final_resp.usage(); - yield Ok(StreamPart::Usage(crate::stream::Usage { - input_tokens: usage.input_tokens, - output_tokens: usage.output_tokens, - })); - } + Ok(MultiTurnStreamItem::FinalResponse(_)) => {}Based on learnings from the provided context snippet
rust/cloud-storage/agent/src/hook.rs:44-121, usage is already emitted inon_stream_completion_response_finish.🤖 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 `@rust/cloud-storage/agent/src/agent_loop.rs` around lines 263 - 289, The StreamBridge attached at line 263 already emits usage information via the on_stream_completion_response_finish hook, so the subsequent emission of StreamPart::Usage in the FinalResponse match arm (lines 284-289) creates a duplicate. Remove the code block that yields Ok(StreamPart::Usage) from the FinalResponse match case, as the usage tokens are already being properly emitted through the StreamBridge hook mechanism.rust/cloud-storage/agent/src/model/router.rs (1)
96-106:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPropagate routing failures instead of silently downgrading to default.
Line 100 currently catches both
UnknownModelandProviderUnavailableand always returns Anthropic default. That can serve a different model/provider than requested and hide real provider/config outages.Suggested direction
- pub fn route_or_default(&self, model: &str) -> RoutedModel { - self.route(model).unwrap_or_else(|_| { - RoutedModel::Anthropic(AnthropicModel::new( - self.anthropic_client.clone(), - super::AgentModel::default().to_string(), - )) - }) - } + pub fn route_or_default(&self, model: &str) -> crate::error::Result<RoutedModel> { + match self.route(model) { + Ok(routed) => Ok(routed), + Err(AgentError::UnknownModel(_)) => Ok(RoutedModel::Anthropic(AnthropicModel::new( + self.anthropic_client.clone(), + super::AgentModel::default().to_string(), + ))), + Err(e) => Err(e), + } + }🤖 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 `@rust/cloud-storage/agent/src/model/router.rs` around lines 96 - 106, The route_or_default method currently catches all errors from the self.route(model) call and silently returns an Anthropic default, which masks real provider unavailability and configuration issues. Modify the error handling to distinguish between error types: only return the default Anthropic model for UnknownModel errors, while allowing ProviderUnavailable errors to propagate up so they are not hidden by the automatic fallback to the default model.rust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rs (1)
6-6:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake schema directory creation idempotent.
create_dir("schemas")fails on reruns when the directory already exists, which breaks repeated schema-generation runs.Proposed fix
- std::fs::create_dir("schemas").expect("create schemas dir"); + std::fs::create_dir_all("schemas").expect("create schemas dir");🤖 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 `@rust/cloud-storage/ai_tools/src/bin/gen_tool_schemas.rs` at line 6, The std::fs::create_dir call for creating the "schemas" directory fails when the directory already exists, breaking repeated runs of the schema generation tool. Replace the create_dir call with create_dir_all, which is idempotent and will succeed whether the directory exists or not, making the schema generation process rerunnable without errors.rust/cloud-storage/ai_toolset/src/schema/generate.rs (1)
1-91:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
ai_toolsetmodifications violate a repository guardrail.This PR introduces direct changes in
rust/cloud-storage/ai_toolset/**, which is explicitly marked as non-modifiable by project policy. Please move these changes behind allowed crates/interfaces (or get an explicit exception before merge).As per coding guidelines,
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/generate.rs` around lines 1 - 91, The changes made to `rust/cloud-storage/ai_toolset/src/schema/generate.rs` violate the repository guardrail that forbids modifications to the `ai_toolset` crate. Revert all changes in the `ai_toolset` directory and refactor the modifications to be made in an allowed external crate or interface that wraps or extends the functionality, ensuring the core `ai_toolset` crate remains unmodified as required by project policy.Source: Coding guidelines
rust/cloud-storage/ai_toolset/src/schema/mod.rs (1)
9-20:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThis change touches a crate marked as immutable by project rules.
These edits are under
rust/cloud-storage/ai_toolset/**, which the provided repository guidelines designate as do-not-change. Please move this implementation to an allowed crate, revert these edits, or document an explicit exception for this PR.As per coding guidelines,
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/mod.rs` around lines 9 - 20, The file rust/cloud-storage/ai_toolset/src/schema/mod.rs is in a crate designated as immutable by project rules and cannot be modified. Either revert all changes made to files under rust/cloud-storage/ai_toolset/**, move the implementation to an allowed crate and update the corresponding imports and exports, or obtain explicit written approval from the project maintainers to document an exception for this PR with clear justification for why the immutability rule must be broken.Source: Coding guidelines
rust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rs (1)
70-96:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOptional fields with
constcan remain effectively non-nullable.When a property has both
typeandconst, this transform adds"null"totypebut leavesconstunchanged, sonullis still invalid. AfterAddRequired, that field becomes required with a single constant value instead of nullable.Suggested fix
fn make_nullable(property: &mut Value) { let Some(obj) = property.as_object_mut() else { return; }; + // `const` plus type-union does not actually admit null; wrap instead. + if obj.contains_key("const") { + let original = Value::Object(std::mem::take(obj)); + *property = json!({"anyOf": [original, {"type": "null"}]}); + return; + } + // Union node: add a null variant unless one exists. if let Some(Value::Array(variants)) = obj.get_mut("anyOf") { let has_null = variants .iter() .any(|v| v.as_object().is_some_and(has_null_type));🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/transform/nullify_optional.rs` around lines 70 - 96, The transform adds "null" to the type for nullable properties but doesn't handle the case where a property has both type and const constraints. When const is present, adding "null" to type is insufficient because const still restricts the value to the original constant. After the type union logic (around lines 70-95), add a check similar to the enum handling (lines 97-101) to detect if a const field exists. If it does, convert it to an enum array containing the original constant value and null, then remove the const field, ensuring the property truly becomes nullable.rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs (1)
1-107:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDo not modify
ai_toolsetin this PR.This file introduces new code under
rust/cloud-storage/ai_toolset/**, which violates the repository rule that this crate must not be changed. Please revert these edits (or move the implementation to an allowed crate and keepai_toolsetuntouched).As per coding guidelines:
rust/cloud-storage/ai_toolset/**: “You are not allowed to EVER change this crate.”🤖 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 `@rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs` around lines 1 - 107, The new implementation of the StripUnsupported struct and its Transform trait implementation has been added to the ai_toolset crate, which violates the repository rule that this crate must never be modified. Either revert all changes in this file (rust/cloud-storage/ai_toolset/src/schema/transform/strip_unsupported.rs) entirely, or move the StripUnsupported struct and its Transform implementation to an allowed crate outside of ai_toolset, keeping the ai_toolset directory completely untouched.Source: Coding guidelines
rust/cloud-storage/chat/src/outbound/postgres/mod.rs (1)
131-131:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn the stored chat model before falling back.
This still reports every fetched chat as
FALLBACK_MODEL, so reopened chats lose the selected provider id and consumers can render/continue with the wrong model. Use the persisted chat model when present and fall back only for legacy rows.Proposed fix
- model: Some(FALLBACK_MODEL.to_string()), + model: chat.model.or_else(|| Some(FALLBACK_MODEL.to_string())),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.model: chat.model.or_else(|| Some(FALLBACK_MODEL.to_string())),🤖 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 `@rust/cloud-storage/chat/src/outbound/postgres/mod.rs` at line 131, The model field assignment is currently always using FALLBACK_MODEL, which causes reopened chats to lose their originally selected provider and model information. Instead of unconditionally setting model to FALLBACK_MODEL.to_string(), check if a persisted chat model exists in the fetched data and use that value when available, only falling back to FALLBACK_MODEL for legacy database rows that do not have a stored model. This preserves the original model selection for existing chats while providing a fallback for historical data.rust/cloud-storage/document_cognition_service/src/api/stream/chat_message/mod.rs (1)
182-189:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign entitlement denial status with the endpoint’s documented/paywall contract.
Line 184 enforces model access, but Line 188 returns
StatusCode::FORBIDDENwhile this endpoint documents “user lacks access to requested model” as402(Line 133). This contract drift can break clients that map entitlement failures to paywall UX by status code.Suggested fix
- status: Some(StatusCode::FORBIDDEN), + status: Some(StatusCode::PAYMENT_REQUIRED),🤖 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 `@rust/cloud-storage/document_cognition_service/src/api/stream/chat_message/mod.rs` around lines 182 - 189, The status code returned in the ChatMessageError when model access is denied does not match the endpoint's documented contract. In the model access validation block where has_access is checked (around lines 182-189), the status field is set to StatusCode::FORBIDDEN, but the endpoint documentation indicates that entitlement/access denial should return 402 (Payment Required) to align with the paywall UX contract. Update the status field in the ChatMessageError returned from the model access check to use the correct status code that matches the documented contract at Line 133.rust/cloud-storage/document_cognition_service/src/api/structured_completion.rs (1)
23-23:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor the requested model id in structured completion.
Line 23 accepts a model id, but the handler ignores it and always uses
model_access.model()(Line 75). This silently drops caller intent and can run a different (potentially higher-cost) model than requested. Validate/access-checkrequest.model, resolve it to the runtime model, and use that resolved value for both agent phases; return 400 for unknown ids.🤖 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 `@rust/cloud-storage/document_cognition_service/src/api/structured_completion.rs` at line 23, The structured completion handler accepts a model id in the request struct but ignores it, always using model_access.model() instead. To fix this, validate and access-check the request.model field to ensure it is a known and authorized model id, resolve it to the corresponding runtime model, and use that resolved model value for both agent phases in the handler. Return a 400 status code if the model id is unknown or invalid. This ensures the handler honors the caller's requested model rather than silently substituting a potentially different model.rust/cloud-storage/scheduled_action/src/outbound/inprocess_executor/agent_task.rs (1)
107-107:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAuthorize the scheduled task model before persisting or running it.
AgentTask.modelcomes from persisted task JSON, and these usages store/run it directly. Unlike the chat send path, there is noChatModelAccesscheck here, so a user can schedule a premium provider id and have the worker execute it later. Re-check access foraction.ownerbefore storing messages and beforeAgentLoop::with_model, or reject unauthorized models at create/update and again at dispatch.Also applies to: 132-132, 193-193
🤖 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 `@rust/cloud-storage/scheduled_action/src/outbound/inprocess_executor/agent_task.rs` at line 107, The AgentTask.model is not being authorized before being persisted or executed, allowing users to schedule unauthorized premium provider models. Add ChatModelAccess authorization checks for action.owner at all affected locations: (1) before storing messages at the line with model: agent_task.model.clone() around line 107, (2) at the second affected site around line 132-133, and (3) before AgentLoop::with_model at the third affected site around line 193. Either implement access validation checks at each location before using the model, or enforce authorization at task creation/update time and validate again at dispatch time to prevent unauthorized model execution.
ehayes2000
force-pushed
the
model-selector
branch
from
1a43c4d to
a876006
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.