Revert "feat(hosted key): Add exa hosted key (#3221)"#3495
Revert "feat(hosted key): Add exa hosted key (#3221)"#3495waleedlatif1 merged 1 commit intostagingfrom
Conversation
This reverts commit 158d523.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Tool execution is simplified to always require an API key when marked required (no hosted-key fallback) and no longer retries on hosted-key 429s or logs fixed tool charges. Provider/execution cost reporting is adjusted to remove Separately, rate-limiter DB token-bucket updates no longer write Written by Cursor Bugbot for commit fe577b0. This will update automatically on new commits. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| )::numeric | ||
| ) - ${requestedTokens}::numeric | ||
| ELSE -1 | ||
| ELSE ${rateLimitBucket.tokens}::numeric |
There was a problem hiding this comment.
Rate limiter allows denied requests due to ELSE clause
High Severity
The ELSE branch in the token consumption SQL was changed from -1 to ${rateLimitBucket.tokens}::numeric. The allowed check on line 85 uses tokens >= 0. When a request is denied (insufficient tokens), the current token count (a non-negative value like 3) is now returned instead of -1, so allowed evaluates to true — effectively disabling rate limiting for all consumers of DbTokenBucket.
There was a problem hiding this comment.
Current behavior. There is an issue, but will be addressed once this is un-reverted
Greptile SummaryThis PR reverts the Exa hosted key feature (#3221) to unblock staging while a required database migration is prepared. The revert removes the hosted key injection pipeline, rate-limiter infrastructure, BYOK Exa support, telemetry events, and Key changes:
Confidence Score: 4/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[executeTool called] --> B{Tool has hosting config?}
B -- "YES (removed)" --> C["injectHostedKeyIfNeeded (deleted)"]
C --> D{BYOK key available?}
D -- Yes --> E[Use user BYOK key]
D -- No --> F[Acquire hosted key via RateLimiter]
F --> G{Rate limited?}
G -- Yes --> H[Throw 429 / fire telemetry]
G -- No --> I[Inject hosted API key]
I --> J["executeWithRetry (deleted)"]
J --> K[executeToolRequest]
K --> L["applyHostedKeyCostToResult (deleted)"]
L --> M["stripInternalFields (deleted)"]
M --> N[Return result]
B -- "NO / after revert" --> O[executeToolRequest directly]
O --> N2[Return result]
style C fill:#f88,stroke:#c00
style D fill:#f88,stroke:#c00
style E fill:#f88,stroke:#c00
style F fill:#f88,stroke:#c00
style G fill:#f88,stroke:#c00
style H fill:#f88,stroke:#c00
style I fill:#f88,stroke:#c00
style J fill:#f88,stroke:#c00
style L fill:#f88,stroke:#c00
style M fill:#f88,stroke:#c00
style O fill:#9f9,stroke:#060
style N2 fill:#9f9,stroke:#060
Last reviewed commit: fe577b0 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| const logger = createLogger('BYOKKeysQueries') | ||
|
|
||
| export type BYOKProviderId = 'openai' | 'anthropic' | 'google' | 'mistral' |
There was a problem hiding this comment.
Duplicated BYOKProviderId type definition. This type is now declared identically in both apps/sim/hooks/queries/byok-keys.ts and apps/sim/lib/api-key/byok.ts. Previously it lived in a single place (tools/types.ts) and was imported where needed. With two independent definitions, a future provider addition would require updating both files — and the two definitions could silently drift apart.
Consider keeping one authoritative declaration (e.g., in lib/api-key/byok.ts) and importing it in this file:
| export type BYOKProviderId = 'openai' | 'anthropic' | 'google' | 'mistral' | |
| import type { BYOKProviderId } from '@/lib/api-key/byok' |
| } | ||
| toolCalls?: FunctionCallResponse[] | ||
| toolResults?: Record<string, unknown>[] | ||
| toolResults?: any[] |
There was a problem hiding this comment.
Type regression: toolResults weakened to any[]. Previously typed as Record<string, unknown>[] — meaningful structure enforced by the type system. Changing it to any[] removes that constraint and can suppress downstream type errors when iterating tool results.
Since ToolResponse.output is Record<string, any>, the array element type could simply match:
| toolResults?: any[] | |
| toolResults?: Record<string, any>[] |
| */ | ||
| export type FixedUsageMetadata = Record<string, unknown> | ||
| export type FixedUsageMetadata = Record<string, never> | ||
|
|
There was a problem hiding this comment.
FixedUsageMetadata is now Record<string, never> — an object type with no valid key-value pairs, effectively making it unusable as a data container. The metadata parameter was also removed from LogFixedUsageParams, so this type is no longer used as a function parameter.
Since metadata is now hardcoded to null in logFixedUsage, consider either removing FixedUsageMetadata from the codebase entirely, or adding a clarifying comment if you want to preserve it as a placeholder for future extensibility:
| export type FixedUsageMetadata = Record<string, never> // Currently unused; reserved for future fixed-charge metadata |
2 participants
Summary
Exa hosted key requires a migration first. To unblock staging, reverting this change for now.
Type of Change
Testing
None. This should revert the state to pre-exa.
Checklist
Screenshots/Videos