fix: correct env var lookup for Together AI provider in vault middleware#4564
Open
GanJiaKouN16 wants to merge 4 commits into
Open
fix: correct env var lookup for Together AI provider in vault middleware#4564GanJiaKouN16 wants to merge 4 commits into
GanJiaKouN16 wants to merge 4 commits into
Conversation
Wire the existing GenerateResetLinkModal and PasswordResetLinkModal into the Actions dropdown in the workspace members table. - Add 'Reset password' menu item for workspace members (not self) - Add resetPassword API function in profile service - Show confirmation dialog before generating the reset link - Display the generated password reset link with copy functionality Closes Agenta-AI#2572
Several tables with row-level click navigation were missing the shouldIgnoreRowClick guard, causing clicks on interactive elements (checkboxes, dropdowns, buttons) to accidentally trigger row navigation. Changes: - Consolidate shouldIgnoreRowClick with broader selector list (merges EvaluationRunsTablePOC's extra selectors: [role='button'], [role='menuitem'], [role='checkbox'], .ant-btn, etc.) - Export INTERACTIVE_ROW_SELECTORS constant for reuse - Add guard to ObservabilityTable (traces) - Add guard to SessionsTable - Add guard to PromptsPage - Add guard to TestcasesTableShell - Add guard to EntityTable - Replace partial data-ivt-stop-row-click check in ScenarioListView with full shouldIgnoreRowClick - Update useEntityTableState to use consolidated selectors - Remove duplicate shouldIgnoreRowClick from navigationActions.ts - Update EvaluationRunsTablePOC to import from shared utility Closes Agenta-AI#3254
The evaluation table was showing a generic 'too many requests' message instead of the actual provider error because: 1. executeViaFetch never checked for body-level errors on HTTP 200. The Python SDK can return HTTP 200 with a non-200 status.code embedded in the response body (WorkflowBatchResponse.status.code). This path was silently treated as success. 2. Error stacktrace/type/code were not propagated through the pipeline. Even when the HTTP error path was taken, only the message was extracted — the SDK's status.type, status.code, and status.stacktrace were dropped. Changes: - executeViaFetch: detect body-level errors on HTTP 200 by checking responseData.status.code !== 200 and return an error result - executeViaFetch: extract stacktrace (coercing string[] to string), type, and code from both HTTP-error and body-error paths - Add stacktrace and type to ExecutionResult, RunResult, and ExecuteWorkflowRevisionResult error shapes - runInvocationAction: pass stacktrace and type through to upsertStepResultWithInvocation - upsertStepResultWithInvocation: accept type field in error param No UI changes needed — InvocationCell already renders stepError.message and stepError.stacktrace when present; extractStepError already reads error.code, error.type, error.stacktrace from persisted step data. Closes Agenta-AI#3324
…iddleware
The vault middleware built env var names using f'{provider.upper()}_API_KEY'
which produces TOGETHER_AI_API_KEY for the 'together_ai' provider kind.
The actual env var is TOGETHERAI_API_KEY (no underscore), matching the
frontend (llmProviders.ts, transforms.ts), backend (env.py), and the
Daytona sandbox runner (daytona.py).
Add an explicit _PROVIDER_ENV_VAR_MAP dict (mirroring the Daytona runner
pattern) that maps each provider kind to its correct env var name, with
fallback to the original f-string pattern for any future providers.
Closes Agenta-AI#3659
|
Someone is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
|
dosubot
Bot
added
size:L
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR enhances error tracking with richer metadata (type, stacktrace) across the execution pipeline, consolidates table row-click filtering via centralized selectors, fixes Together AI provider API-key lookup with explicit mappings, and adds password reset functionality for workspace members. ChangesError Details Enhancement Across Execution Chain
Row Click Handling Consolidation
Provider API Key Mapping
Reset Password Feature
🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 50d23f9d-8b8b-4e19-b450-ba7c22cba4ce
📒 Files selected for processing (20)
sdks/python/agenta/sdk/middlewares/running/vault.pyweb/oss/src/components/EvalRunDetails/atoms/runInvocationAction.tsweb/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.tsweb/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsxweb/oss/src/components/InfiniteVirtualTable/hooks/useTableManager.tsxweb/oss/src/components/TestcasesTableNew/components/TestcasesTableShell.tsxweb/oss/src/components/pages/observability/components/ObservabilityTable/index.tsxweb/oss/src/components/pages/observability/components/SessionsTable/index.tsxweb/oss/src/components/pages/prompts/PromptsPage.tsxweb/oss/src/components/pages/settings/WorkspaceManage/cellRenderers.tsxweb/oss/src/services/evaluations/invocations/api.tsweb/oss/src/services/profile/index.tsweb/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsxweb/packages/agenta-entities/src/runnable/types.tsweb/packages/agenta-entity-ui/src/shared/EntityTable.tsxweb/packages/agenta-playground/src/executeWorkflowRevision.tsweb/packages/agenta-playground/src/state/execution/executionRunner.tsweb/packages/agenta-playground/src/state/execution/types.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useEntityTableState.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsx
💤 Files with no reviewable changes (1)
- web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
Comment on lines
+40
to
+60
| # Mapping from provider kind to environment variable name. | ||
| # Most providers follow the pattern PROVIDER_API_KEY, but some have | ||
| # underscores in their kind string (e.g. "together_ai") where the env | ||
| # var drops the underscore (TOGETHERAI_API_KEY). This explicit mapping | ||
| # mirrors the one in the Daytona runner and the frontend llmProviders.ts. | ||
| _PROVIDER_ENV_VAR_MAP: Dict[str, str] = { | ||
| "openai": "OPENAI_API_KEY", | ||
| "cohere": "COHERE_API_KEY", | ||
| "anyscale": "ANYSCALE_API_KEY", | ||
| "deepinfra": "DEEPINFRA_API_KEY", | ||
| "alephalpha": "ALEPHALPHA_API_KEY", | ||
| "groq": "GROQ_API_KEY", | ||
| "minimax": "MINIMAX_API_KEY", | ||
| "mistral": "MISTRAL_API_KEY", | ||
| "mistralai": "MISTRAL_API_KEY", | ||
| "anthropic": "ANTHROPIC_API_KEY", | ||
| "perplexityai": "PERPLEXITYAI_API_KEY", | ||
| "together_ai": "TOGETHERAI_API_KEY", | ||
| "openrouter": "OPENROUTER_API_KEY", | ||
| "gemini": "GEMINI_API_KEY", | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Run ruff format and ruff check --fix before committing.
As per coding guidelines, Python changes in {api,sdk,sdks}/**/*.py must be formatted and linted with ruff.
Source: Coding guidelines
Comment on lines
+60
to
+72
| /** | ||
| * Generate a password reset link for a user (admin action). | ||
| * Returns the reset password link string. | ||
| */ | ||
| export const resetPassword = async (userId: string): Promise<string> => { | ||
| const base = getBaseUrl() | ||
| const url = new URL("api/profile/reset-password", base) | ||
| url.searchParams.set("user_id", userId) | ||
| const data = await fetchJson<string>(url, { | ||
| method: "POST", | ||
| }) | ||
| return data | ||
| } |
There was a problem hiding this comment.
Use Fern-generated client instead of raw fetchJson.
As per coding guidelines, all new frontend API code must go through the Fern-generated client, not raw fetch/axios. The Fern-generated client already provides a reset password method that handles this endpoint with proper typing and error handling.
♻️ Recommended refactor to use Fern-generated client
-export const resetPassword = async (userId: string): Promise<string> => {
- const base = getBaseUrl()
- const url = new URL("api/profile/reset-password", base)
- url.searchParams.set("user_id", userId)
- const data = await fetchJson<string>(url, {
- method: "POST",
- })
- return data
-}
+export const resetPassword = async (userId: string): Promise<string> => {
+ const client = getAgentaSdkClient({host: getAgentaApiUrl()})
+ const response = await client.users.resetUserPassword({user_id: userId})
+ // Extract the reset link from the response based on the actual API contract
+ return response as unknown as string
+}Note: You may need to adjust the return type handling based on the actual Fern client response structure.
Based on learnings, the Fern-generated client at web/packages/agenta-api-client/src/generated/api/resources/users/client/Client.ts already implements the resetUserPassword method for the profile/reset-password endpoint with proper query parameter handling.
📝 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.
Suggested change
| /** | |
| * Generate a password reset link for a user (admin action). | |
| * Returns the reset password link string. | |
| */ | |
| export const resetPassword = async (userId: string): Promise<string> => { | |
| const base = getBaseUrl() | |
| const url = new URL("api/profile/reset-password", base) | |
| url.searchParams.set("user_id", userId) | |
| const data = await fetchJson<string>(url, { | |
| method: "POST", | |
| }) | |
| return data | |
| } | |
| /** | |
| * Generate a password reset link for a user (admin action). | |
| * Returns the reset password link string. | |
| */ | |
| export const resetPassword = async (userId: string): Promise<string> => { | |
| const client = getAgentaSdkClient({host: getAgentaApiUrl()}) | |
| const response = await client.users.resetUserPassword({user_id: userId}) | |
| // Extract the reset link from the response based on the actual API contract | |
| return response as unknown as string | |
| } |
Source: Coding guidelines
Comment on lines
+714
to
+742
| // Check for body-level error status (SDK returns HTTP 200 with error in body). | ||
| // The Python SDK's WorkflowBatchResponse may embed a non-200 status.code | ||
| // inside the response body even when the HTTP status is 200. | ||
| const bodyStatus = responseData?.status | ||
| if (bodyStatus && typeof bodyStatus === "object" && bodyStatus.code && bodyStatus.code !== 200) { | ||
| const traceId = extractTraceIdFromPayload(responseData) | ||
| const spanId = extractSpanIdFromPayload(responseData) | ||
| const st = bodyStatus.stacktrace | ||
| return { | ||
| executionId, | ||
| status: "error", | ||
| startedAt, | ||
| completedAt: new Date().toISOString(), | ||
| error: { | ||
| message: bodyStatus.message || "Invocation failed", | ||
| ...(bodyStatus.code ? {code: bodyStatus.code.toString()} : {}), | ||
| ...(bodyStatus.type ? {type: bodyStatus.type} : {}), | ||
| ...(st ? {stacktrace: Array.isArray(st) ? st.join("\n") : st} : {}), | ||
| }, | ||
| ...(traceId | ||
| ? { | ||
| trace: { | ||
| id: traceId, | ||
| ...(spanId ? {spanId} : {}), | ||
| }, | ||
| } | ||
| : {}), | ||
| } | ||
| } |
There was a problem hiding this comment.
Truthy check on code may miss error status 0.
Line 718 checks bodyStatus.code && bodyStatus.code !== 200. If code is 0, the truthy check fails and the error path is skipped. While code 0 may not be common, a null-safe check would be more robust.
🛡️ Suggested fix
- if (bodyStatus && typeof bodyStatus === "object" && bodyStatus.code && bodyStatus.code !== 200) {
+ if (bodyStatus && typeof bodyStatus === "object" && bodyStatus.code != null && bodyStatus.code !== 200) {
GanJiaKouN16
force-pushed
the
fix/together-ai-env-var-lookup
branch
from
6411619 to
c9d112d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c4886e0-04e4-49da-b274-51c061bad81d
📒 Files selected for processing (20)
sdks/python/agenta/sdk/middlewares/running/vault.pyweb/oss/src/components/EvalRunDetails/atoms/runInvocationAction.tsweb/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.tsweb/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsxweb/oss/src/components/InfiniteVirtualTable/hooks/useTableManager.tsxweb/oss/src/components/TestcasesTableNew/components/TestcasesTableShell.tsxweb/oss/src/components/pages/observability/components/ObservabilityTable/index.tsxweb/oss/src/components/pages/observability/components/SessionsTable/index.tsxweb/oss/src/components/pages/prompts/PromptsPage.tsxweb/oss/src/components/pages/settings/WorkspaceManage/cellRenderers.tsxweb/oss/src/services/evaluations/invocations/api.tsweb/oss/src/services/profile/index.tsweb/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsxweb/packages/agenta-entities/src/runnable/types.tsweb/packages/agenta-entity-ui/src/shared/EntityTable.tsxweb/packages/agenta-playground/src/executeWorkflowRevision.tsweb/packages/agenta-playground/src/state/execution/executionRunner.tsweb/packages/agenta-playground/src/state/execution/types.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useEntityTableState.tsweb/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsx
💤 Files with no reviewable changes (1)
- web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
✅ Files skipped from review due to trivial changes (1)
- web/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- web/packages/agenta-playground/src/state/execution/types.ts
- web/packages/agenta-playground/src/executeWorkflowRevision.ts
- web/oss/src/components/pages/observability/components/SessionsTable/index.tsx
- web/packages/agenta-entities/src/runnable/types.ts
- web/oss/src/components/EvalRunDetails/atoms/runInvocationAction.ts
- web/oss/src/components/pages/observability/components/ObservabilityTable/index.tsx
- web/oss/src/components/InfiniteVirtualTable/hooks/useTableManager.tsx
- web/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsx
- sdks/python/agenta/sdk/middlewares/running/vault.py
- web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsx
- web/oss/src/components/pages/prompts/PromptsPage.tsx
- web/packages/agenta-playground/src/state/execution/executionRunner.ts
- web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useEntityTableState.ts
- web/oss/src/services/profile/index.ts
- web/oss/src/components/pages/settings/WorkspaceManage/cellRenderers.tsx
Comment on lines
+550
to
+553
| onClick: (event) => { | ||
| if (shouldIgnoreRowClick(event)) return | ||
| handleRowSelect(record.id, !selectedIdsSet.has(record.id)) | ||
| }, |
There was a problem hiding this comment.
Honor selectionDisabled in row-click toggling.
At Line 550, row clicks still toggle selection even when selectionDisabled is true, so disabled selection can be bypassed via the row background.
Proposed fix
onClick: (event) => {
if (shouldIgnoreRowClick(event)) return
+ if (selectionDisabled) return
handleRowSelect(record.id, !selectedIdsSet.has(record.id))
},📝 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.
Suggested change
| onClick: (event) => { | |
| if (shouldIgnoreRowClick(event)) return | |
| handleRowSelect(record.id, !selectedIdsSet.has(record.id)) | |
| }, | |
| onClick: (event) => { | |
| if (shouldIgnoreRowClick(event)) return | |
| if (selectionDisabled) return | |
| handleRowSelect(record.id, !selectedIdsSet.has(record.id)) | |
| }, |
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3659 — Together AI model runs fail with
InvalidSecretsV0Errorbecause the vault middleware builds the wrong env var name.Root cause
vault.py:309usesf"{provider.upper()}_API_KEY"to build env var names. For"together_ai"this producesTOGETHER_AI_API_KEY, but the actual env var used everywhere else isTOGETHERAI_API_KEY(no underscore).This is the only standard provider with an underscore in its kind string, making it the only one broken by this pattern.
Fix
Add an explicit
_PROVIDER_ENV_VAR_MAPdict that maps each provider kind to its correct env var name, mirroring the pattern already used in:daytona.py:133-149)llmProviders.ts,transforms.ts)env.py)Falls back to the original
f"{provider.upper()}_API_KEY"pattern for any future providers not in the map.Changes
sdks/python/agenta/sdk/middlewares/running/vault.py_PROVIDER_ENV_VAR_MAPwith explicit mappings for all 15 standard providers. Changed lookup fromf"{provider.upper()}_API_KEY"to_PROVIDER_ENV_VAR_MAP.get(provider, fallback).Testing
TOGETHERAI_API_KEYenv var and run a Together AI model (e.g.together_ai/deepseek-ai/DeepSeek-V3) in the PlaygroundInvalidSecretsV0ErrorCloses #3659