feat: migrate email from SendGrid to SMTP with fallback#4566
Open
GanJiaKouN16 wants to merge 6 commits into
Open
feat: migrate email from SendGrid to SMTP with fallback#4566GanJiaKouN16 wants to merge 6 commits into
GanJiaKouN16 wants to merge 6 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
… drawer The 'Open evaluator registry' button in the Trace Drawer's EvaluatorDetailsPopover navigated human evaluators to the registry page (/evaluators?tab=human&openEvaluator=...) instead of the evaluator playground. Automatic evaluators already linked correctly. Unify both evaluator types to navigate to the playground (/evaluators/playground?revisions=...), consistent with how other parts of the codebase link to evaluators (EvaluatorSection, ConfigurationView). Update button text to 'Open evaluator playground'. Closes Agenta-AI#4535
Replace the SendGrid-only email backend with SMTP support, keeping SendGrid as a legacy fallback for existing deployments. Changes: - email_service.py: use smtplib for SMTP (priority), SendGrid as fallback, no-op when neither is configured - env.py: add SmtpConfig (SMTP_HOST, SMTP_PORT, SMTP_USERNAME, SMTP_PASSWORD, SMTP_FROM_ADDRESS, SMTP_USE_TLS), keep SendgridConfig for backwards compatibility; update AuthFacade.email_method to check both - OSS/EE organization_service.py: use env.smtp.enabled || env.sendgrid .enabled; use configured from_address instead of hardcoded email - user_service.py: same email-enabled check update - db_manager_ee.py: remove dead sendgrid import and unused sg client - pyproject.toml: remove sendgrid dependency (imported lazily only when SENDGRID_API_KEY is set) - env example files: add SMTP vars, mark SendGrid as legacy - docs: add SMTP config table, mark SendGrid as legacy Closes Agenta-AI#4536
|
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 implements SMTP email support as the primary backend with SendGrid fallback, refactors table row-click handling to use centralized selectors, enriches error diagnostics with type and stacktrace fields, adds password reset for workspace members, updates evaluator navigation to use a unified playground route, and introduces provider-to-environment-variable mapping for SDK API key resolution. ChangesSMTP Email Backend Migration
Row-Click Interactive Element Handling Refactoring
Error Type Enrichment and Execution Flow
Workspace Member Password Reset Feature
Evaluator Navigation Routing Update
LLM Provider API Key Environment Variable Mapping
🎯 4 (Complex) | ⏱️ ~75 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/oss/src/components/SharedDrawers/TraceDrawer/hooks/useEvaluatorNavigation.ts (1)
19-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign evaluator type detection with the canonical URI-first rule.
Line 37 currently depends on a local
isHumanEvaluatorthat only checks flags/meta, but the canonical logic also checks URI category first. This can misclassify some human evaluators as"auto".Suggested patch
const isHumanEvaluator = (evaluator: any) => { - return Boolean(evaluator?.flags?.is_feedback || evaluator?.meta?.is_feedback) + const uri = evaluator?.data?.uri + if (typeof uri === "string") { + return uri.split(":")[2] === "feedback" + } + return Boolean(evaluator?.flags?.is_feedback || evaluator?.meta?.is_feedback) }Also applies to: 37-37
🧹 Nitpick comments (1)
api/oss/src/services/email_service.py (1)
47-48: ⚡ Quick winApply keyword-only params for new API service function signatures.
New helper signatures are positional; this diverges from the API Python guideline and makes callsites easier to misuse.
As per coding guidelines, "Prefer keyword-only parameters using
*in function signatures." and "Use grouped sections in function signatures/calls with#separators for readability."Also applies to: 74-75, 87-89
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca0382dc-7e01-463b-a6ce-91fa346f98b5
📒 Files selected for processing (34)
api/ee/src/services/db_manager_ee.pyapi/ee/src/services/organization_service.pyapi/oss/src/services/email_service.pyapi/oss/src/services/organization_service.pyapi/oss/src/services/user_service.pyapi/oss/src/utils/env.pyapi/pyproject.tomldocs/docs/self-host/02-configuration.mdxhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.examplesdks/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/SharedDrawers/TraceDrawer/components/EvaluatorDetailsPopover.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/hooks/useEvaluatorNavigation.tsweb/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 (3)
- web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
- api/pyproject.toml
- api/ee/src/services/db_manager_ee.py
Comment on lines
+22
to
+25
| import sendgrid | ||
|
|
||
| _sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key) | ||
| log.info("✓ Email enabled via SendGrid (legacy)") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether sendgrid remains declared in dependency manifests.
fd -i 'pyproject.toml|requirements*.txt|poetry.lock|uv.lock|Pipfile'
rg -n --iglob 'pyproject.toml' 'sendgrid'
rg -n --iglob 'requirements*.txt' 'sendgrid'Repository: Agenta-AI/agenta
Length of output: 1458
Guard legacy SendGrid import to prevent startup crash when SendGrid env is enabled but sendgrid isn’t installed.
rg found no sendgrid declarations in any **/pyproject.toml or **/requirements*.txt, so the module-scope import sendgrid can raise ImportError during startup.
Proposed fix
elif _USE_SENDGRID:
- import sendgrid
-
- _sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key)
- log.info("✓ Email enabled via SendGrid (legacy)")
+ try:
+ import sendgrid
+ _sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key)
+ log.info("✓ Email enabled via SendGrid (legacy)")
+ except ImportError:
+ _sg = None
+ log.warning("✗ SendGrid configured but package is not installed; email disabled")
+ _USE_SENDGRID = False📝 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
| import sendgrid | |
| _sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key) | |
| log.info("✓ Email enabled via SendGrid (legacy)") | |
| try: | |
| import sendgrid | |
| _sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key) | |
| log.info("✓ Email enabled via SendGrid (legacy)") | |
| except ImportError: | |
| _sg = None | |
| log.warning("✗ SendGrid configured but package is not installed; email disabled") | |
| _USE_SENDGRID = False |
Comment on lines
+59
to
+65
| server = smtplib.SMTP(smtp_host, smtp_port) | ||
| server.ehlo() | ||
| server.starttls() | ||
| server.ehlo() | ||
| else: | ||
| server = smtplib.SMTP(smtp_host, smtp_port) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find SMTP client constructions and check whether timeout is passed.
rg -nP --type=py -C2 'smtplib\.SMTP\(' apiRepository: Agenta-AI/agenta
Length of output: 690
Add SMTP connection timeout to prevent request hangs on stalled mail servers
api/oss/src/services/email_service.py creates smtplib.SMTP(smtp_host, smtp_port) in both the TLS and non-TLS branches without a timeout, so a slow/unreachable SMTP server can block the request. Add a timeout= (e.g., from existing env.smtp config) to both SMTP connection sites.
Comment on lines
+106
to
108
| if not _USE_SMTP and not _USE_SENDGRID: | ||
| log.info(f"[EMAIL] Email disabled - would send '{subject}' to {to_email}") | ||
| return True |
There was a problem hiding this comment.
Avoid logging recipient email and subject in disabled-email mode.
Line 107 logs direct recipient and subject values, which can expose PII in normal operational logs.
Proposed fix
- log.info(f"[EMAIL] Email disabled - would send '{subject}' to {to_email}")
+ log.info("[EMAIL] Email disabled - skipped outbound message")| or os.getenv("AGENTA_AUTHN_EMAIL_FROM") | ||
| or os.getenv("AGENTA_SEND_EMAIL_FROM_ADDRESS") | ||
| ) | ||
| use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in ("true", "1", "yes") |
There was a problem hiding this comment.
Use _TRUTHY for SMTP_USE_TLS parsing to avoid silent misconfiguration.
Line 942 currently accepts only true/1/yes; values already accepted elsewhere (for example on, enabled) will evaluate to false and unexpectedly disable TLS.
Proposed fix
- use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in ("true", "1", "yes")
+ use_tls: bool = (os.getenv("SMTP_USE_TLS") or "true").lower() in _TRUTHY📝 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
| use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in ("true", "1", "yes") | |
| use_tls: bool = (os.getenv("SMTP_USE_TLS") or "true").lower() in _TRUTHY |
Comment on lines
+64
to
+71
| 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.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the Fern client for this new profile endpoint.
New frontend API wrappers in web are supposed to go through the shared SDK client, not fetchJson. Adding resetPassword() here creates a parallel API path and skips the repo-standard queryParams boundary. If the generated client does not expose this route yet, please add/regenerate that binding first and call it from here.
As per coding guidelines, "All new frontend API code must go through the Fern-generated client, not raw axios", "Use getAgentaSdkClient({host: getAgentaApiUrl()}) — it is a lazy singleton, share it across calls", and "Pass query params via {queryParams: {...}}, NOT axios's {params: {...}}".
Source: Coding guidelines
Comment on lines
1677
to
1679
| const handleRowClick = useCallback((_event: React.MouseEvent, record: ScenarioTableRow) => { | ||
| const target = _event.target as HTMLElement | ||
| if (target?.closest("[data-ivt-stop-row-click]")) return | ||
| if (shouldIgnoreRowClick(_event)) return | ||
| setDrawerScenarioId(record.scenarioId) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether legacy marker usage still exists and whether shared selectors account for it.
echo "=== Shared interactive selector contract ==="
rg -n -C3 'INTERACTIVE_ROW_SELECTORS|data-ivt-stop-row-click|data-interactive' web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsx
echo
echo "=== Legacy marker usages in repo ==="
rg -n -C2 'data-ivt-stop-row-click'
echo
echo "=== New marker usages in repo ==="
rg -n -C2 'data-interactive'Repository: Agenta-AI/agenta
Length of output: 5344
Fix row-click ignore contract: data-ivt-stop-row-click is not covered by shouldIgnoreRowClick
shouldIgnoreRowClick(viaINTERACTIVE_ROW_SELECTORS) ignores[data-interactive]but does not include"[data-ivt-stop-row-click]"(web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsx).- The legacy attribute is still emitted in the repo (e.g.
web/packages/agenta-entity-ui/src/shared/SharedGenerationResultUtils.tsxand multipleweb/osstable/action components). - Update remaining row-action elements to use
[data-interactive](or extendINTERACTIVE_ROW_SELECTORSto temporarily include"[data-ivt-stop-row-click]") to prevent ScenarioListView-style row navigation regressions.
Comment on lines
+718
to
+731
| 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} : {}), |
There was a problem hiding this comment.
Normalize status.code before comparing to 200.
At Line 718, "200" (string) is treated as a failure because it is compared to numeric 200. That can incorrectly route successful responses into the error path.
Proposed fix
- const bodyStatus = responseData?.status
- if (bodyStatus && typeof bodyStatus === "object" && bodyStatus.code && bodyStatus.code !== 200) {
+ const bodyStatus = responseData?.status
+ const bodyStatusCode =
+ bodyStatus && typeof bodyStatus === "object" && bodyStatus.code !== undefined
+ ? String(bodyStatus.code).trim()
+ : undefined
+ if (bodyStatus && typeof bodyStatus === "object" && bodyStatusCode && bodyStatusCode !== "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()} : {}),
+ ...(bodyStatusCode ? {code: bodyStatusCode} : {}),
...(bodyStatus.type ? {type: bodyStatus.type} : {}),
...(st ? {stacktrace: Array.isArray(st) ? st.join("\n") : st} : {}),
},
GanJiaKouN16
force-pushed
the
feat/smtp-email-migration
branch
from
7b29544 to
1706e43
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/oss/src/components/pages/settings/WorkspaceManage/cellRenderers.tsx (1)
26-27: ⚡ Quick winUse
EnhancedModalfor this new reset-password modal flow.These new modal imports are part of newly added UX; please align the underlying modal components to the project modal standard (
@agenta/uiEnhancedModal) instead of raw antdModal.As per coding guidelines: “Use
EnhancedModalfrom@agenta/uiinstead of raw antdModalwhen building new modals.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a98f1230-8685-4cbd-985d-29783ca9e0a1
📒 Files selected for processing (34)
api/ee/src/services/db_manager_ee.pyapi/ee/src/services/organization_service.pyapi/oss/src/services/email_service.pyapi/oss/src/services/organization_service.pyapi/oss/src/services/user_service.pyapi/oss/src/utils/env.pyapi/pyproject.tomldocs/docs/self-host/02-configuration.mdxhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.examplesdks/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/SharedDrawers/TraceDrawer/components/EvaluatorDetailsPopover.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/hooks/useEvaluatorNavigation.tsweb/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 (3)
- web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
- api/pyproject.toml
- api/ee/src/services/db_manager_ee.py
✅ Files skipped from review due to trivial changes (7)
- web/oss/src/components/SharedDrawers/TraceDrawer/components/EvaluatorDetailsPopover.tsx
- web/packages/agenta-playground/src/executeWorkflowRevision.ts
- hosting/docker-compose/ee/env.ee.gh.example
- hosting/docker-compose/oss/env.oss.dev.example
- docs/docs/self-host/02-configuration.mdx
- hosting/docker-compose/oss/env.oss.gh.example
- web/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsx
🚧 Files skipped from review as they are similar to previous changes (20)
- web/packages/agenta-playground/src/state/execution/types.ts
- web/oss/src/services/evaluations/invocations/api.ts
- web/packages/agenta-entities/src/runnable/types.ts
- web/oss/src/components/pages/prompts/PromptsPage.tsx
- web/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsx
- web/oss/src/components/SharedDrawers/TraceDrawer/hooks/useEvaluatorNavigation.ts
- web/oss/src/components/pages/observability/components/SessionsTable/index.tsx
- web/packages/agenta-entity-ui/src/shared/EntityTable.tsx
- web/oss/src/components/TestcasesTableNew/components/TestcasesTableShell.tsx
- api/oss/src/services/organization_service.py
- 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
- sdks/python/agenta/sdk/middlewares/running/vault.py
- web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useEntityTableState.ts
- web/packages/agenta-playground/src/state/execution/executionRunner.ts
- web/oss/src/services/profile/index.ts
- hosting/docker-compose/ee/env.ee.dev.example
- api/oss/src/services/email_service.py
- api/ee/src/services/organization_service.py
Comment on lines
57
to
61
| export const shouldIgnoreRowClick = (event: MouseEvent<HTMLElement>): boolean => { | ||
| const target = event.target as HTMLElement | ||
|
|
||
| // Check if clicking on interactive elements | ||
| if ( | ||
| target.closest("button") || | ||
| target.closest("a") || | ||
| target.closest(".ant-dropdown-trigger") || | ||
| target.closest(".ant-checkbox-wrapper") || | ||
| target.closest(".ant-select") || | ||
| target.closest("input") || | ||
| target.closest("textarea") | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| if (!target) return false | ||
| return Boolean(target.closest(INTERACTIVE_ROW_SELECTORS)) | ||
| } |
There was a problem hiding this comment.
Use instanceof type guard for safer type narrowing.
The as HTMLElement cast bypasses TypeScript's type checking. While this works at runtime (mouse events on table rows always have HTMLElement targets), TypeScript cannot verify that event.target has the closest() method. Using an instanceof guard provides proper type narrowing.
🔍 Proposed fix using instanceof
export const shouldIgnoreRowClick = (event: MouseEvent<HTMLElement>): boolean => {
- const target = event.target as HTMLElement
- if (!target) return false
+ const target = event.target
+ if (!(target instanceof HTMLElement)) return false
return Boolean(target.closest(INTERACTIVE_ROW_SELECTORS))
}📝 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
| export const shouldIgnoreRowClick = (event: MouseEvent<HTMLElement>): boolean => { | |
| const target = event.target as HTMLElement | |
| // Check if clicking on interactive elements | |
| if ( | |
| target.closest("button") || | |
| target.closest("a") || | |
| target.closest(".ant-dropdown-trigger") || | |
| target.closest(".ant-checkbox-wrapper") || | |
| target.closest(".ant-select") || | |
| target.closest("input") || | |
| target.closest("textarea") | |
| ) { | |
| return true | |
| } | |
| return false | |
| if (!target) return false | |
| return Boolean(target.closest(INTERACTIVE_ROW_SELECTORS)) | |
| } | |
| export const shouldIgnoreRowClick = (event: MouseEvent<HTMLElement>): boolean => { | |
| const target = event.target | |
| if (!(target instanceof HTMLElement)) return false | |
| return Boolean(target.closest(INTERACTIVE_ROW_SELECTORS)) | |
| } |
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 #4536 — migrate the email backend from SendGrid to SMTP, allowing self-hosting users to use their own SMTP servers while keeping SendGrid as a legacy fallback.
Priority order
SMTP > SendGrid > no-op — the email service checks SMTP first, falls back to SendGrid if configured, and silently skips email if neither is set.
Changes
api/oss/src/services/email_service.pysmtplibfor SMTP (priority), SendGrid as lazy fallback. Removed top-level sendgrid import.api/oss/src/utils/env.pySmtpConfigclass withSMTP_HOST,SMTP_PORT,SMTP_USERNAME,SMTP_PASSWORD,SMTP_FROM_ADDRESS,SMTP_USE_TLS. AddedsmtptoEnvironSettings. UpdatedAuthFacade.email_methodto check both SMTP and SendGrid.api/oss/src/services/organization_service.pyenv.smtp.enabled || env.sendgrid.enabled; use configuredfrom_addressapi/oss/src/services/user_service.pyapi/ee/src/services/organization_service.pyaccount@hello.agenta.aiwith configured from_addressapi/ee/src/services/db_manager_ee.pyimport sendgridand unusedsgclientapi/pyproject.tomlsendgrid>=6,<7dependency (imported lazily only when needed)hosting/docker-compose/*/env.*.example(×4)docs/docs/self-host/02-configuration.mdxNew environment variables
Backwards compatibility
SENDGRID_FROM_ADDRESSis still read as a fallback forSMTP_FROM_ADDRESSAGENTA_AUTHN_EMAIL_FROM,AGENTA_SEND_EMAIL_FROM_ADDRESS) still workTesting
SMTP_HOST,SMTP_PORT,SMTP_FROM_ADDRESSand verify invitation/reset emails are sent via SMTPSENDGRID_API_KEY+SENDGRID_FROM_ADDRESSand verify fallback worksCloses #4536