feat: custom passwordless email delivery via shared email service#4570
Open
GanJiaKouN16 wants to merge 10 commits into
Open
feat: custom passwordless email delivery via shared email service#4570GanJiaKouN16 wants to merge 10 commits into
GanJiaKouN16 wants to merge 10 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
The breadcrumb always showed 'Auto Evals' for SDK evaluations because: 1. test.tsx normalized type='custom' to 'auto' before passing to EvalRunPreviewPage, losing the SDK type information 2. Page.tsx typeMap had no 'custom' entry 3. buildBreadcrumbs.ts hardcoded 'auto evaluation' as fallback label Fix: - Remove the custom→auto normalization in test.tsx - Add 'custom' → 'SDK Evals' entry to Page.tsx typeMap (matches the tab label in EvaluationsView.tsx) - Change buildBreadcrumbs.ts fallback from 'auto evaluation' to 'Evaluations' Closes Agenta-AI#4549
Add Playwright acceptance tests for the annotation queue feature, following the existing human-annotation test patterns. Tests cover (WEB-ACC-ANNOTATION-001 through 006): 1. Navigate to annotation queues page and verify empty state or list 2. Create a new annotation queue with testcases kind 3. Create a new annotation queue with traces kind 4. Open a queue detail page by clicking a queue row 5. Search for annotation queues by name 6. Delete an annotation queue via the actions menu Structure mirrors human-annotation/: - annotation-queue.spec.ts — entry point - index.ts — test definitions with tags - tests.ts — fixtures (navigateToAnnotations, createAnnotationQueue) - assets/types.ts — type definitions Closes Agenta-AI#4528
The atom returned when was null. For ephemeral/newly-created entities (no server baseline), always exists, so every new app was falsely marked as dirty — showing an unexpected "draft" badge on the node name tag in SingleLayout. Change the fallback to : an entity with no server baseline is clean until the user makes actual edits. Fixes Agenta-AI#4556
Implement a custom SuperTokens EmailDeliveryOverrideInput for the passwordless recipe that delegates to the shared email_service instead of using SuperTokens' built-in SMTP transporter. This ensures SMTP_USE_TLS is honoured identically for OTP and application emails (invitations, password resets, etc.). Previously, the SuperTokens SMTPSettings only exposed a flag (SSL), and when secure=false it still silently attempted STARTTLS — diverging from the shared email service which only calls starttls() when SMTP_USE_TLS=true. - Add PasswordlessEmailService class implementing the SuperTokens EmailDeliveryInterface for passwordless OTP emails - Add get_passwordless_email_delivery() helper that returns EmailDeliveryConfig when an email provider is configured - Wire email_delivery into passwordless.init() call - OTP emails now go through _send_via_smtp / _send_via_sendgrid with the same TLS/SSL/auth logic as all other emails - Falls back gracefully (logs + skips) when no provider is configured Fixes Agenta-AI#4559
|
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
the
size:XL
|
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR makes SMTP the primary email backend with SendGrid fallback, routes SuperTokens passwordless OTPs through the shared email service, refactors email sending, enriches error payloads, centralizes row-click filtering, adds a workspace password-reset flow, and adds Playwright annotation-queue tests. ChangesSMTP Email Infrastructure and Supporting Features
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related Issues
🚥 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/EvalRunDetails/components/Page.tsx (1)
28-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
customtoEvalRunPreviewPageProps.evaluationTypeto match new navigation flow.Line 28 currently excludes
"custom", but Line 51 explicitly supports it and callers now pass it through. This creates a TypeScript contract break at the component boundary.Proposed fix
interface EvalRunPreviewPageProps { runId: string - evaluationType: "auto" | "human" | "online" + evaluationType: "auto" | "human" | "online" | "custom" projectId?: string | null }Also applies to: 51-51
🧹 Nitpick comments (2)
web/packages/agenta-entities/src/runnable/types.ts (1)
204-209: ⚡ Quick winAlign chain/row error typings with the enriched
ExecutionResult.errorshape.
ExecutionResult.errornow includestypeandstacktrace, butStageExecutionResult.errorandRowExecutionResult.errorstill omit them. That causes type-level loss when stage/row errors are populated from execution results.Suggested follow-up diff
export interface StageExecutionResult { @@ error?: { message: string code?: string + type?: string + stacktrace?: string } @@ export interface RowExecutionResult { @@ error?: { message: string code?: string + type?: string + stacktrace?: string }api/oss/src/core/auth/supertokens/config.py (1)
74-74: ⚡ Quick winMove
email_serviceimports to module scope.Line 74 and Line 137 use local imports for configuration-linked behavior without evidence of a circular dependency. Please prefer a module-level import.
As per coding guidelines, “Avoid local imports inside helper functions for configuration lookup; prefer module-level imports unless there is a proven circular dependency.”
Also applies to: 137-137
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cc3e49bf-1599-4d18-b12d-266d41463a0b
📒 Files selected for processing (43)
api/ee/src/services/db_manager_ee.pyapi/ee/src/services/organization_service.pyapi/oss/src/core/auth/supertokens/config.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/EvalRunDetails/components/Page.tsxweb/oss/src/components/EvalRunDetails/test.tsxweb/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/lib/helpers/buildBreadcrumbs.tsweb/oss/src/services/evaluations/invocations/api.tsweb/oss/src/services/profile/index.tsweb/oss/tests/playwright/acceptance/annotation-queue/annotation-queue.spec.tsweb/oss/tests/playwright/acceptance/annotation-queue/assets/types.tsweb/oss/tests/playwright/acceptance/annotation-queue/index.tsweb/oss/tests/playwright/acceptance/annotation-queue/tests.tsweb/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsxweb/packages/agenta-entities/src/runnable/types.tsweb/packages/agenta-entities/src/workflow/state/store.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)
- api/pyproject.toml
- web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
- api/ee/src/services/db_manager_ee.py
Comment on lines
+119
to
+125
| except Exception: | ||
| log.error( | ||
| "Failed to send passwordless OTP email to %s", | ||
| template_vars.email, | ||
| exc_info=True, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Do not swallow OTP delivery failures.
At Line 119-Line 124, all exceptions are logged and then suppressed. That can make passwordless login appear successful while the OTP email was never delivered. Re-raise after logging so callers can surface a failure state.
Comment on lines
+58
to
+65
| if env.smtp.use_tls: | ||
| 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.
Add implicit SSL SMTP mode support.
Line 58-Line 65 only switches between STARTTLS and plain smtplib.SMTP. There is no implicit SSL path (SMTP_SSL), so SMTPS-only providers (commonly port 465) will fail even when SMTP is otherwise configured.
Comment on lines
+59
to
+71
| server = smtplib.SMTP(smtp_host, smtp_port) | ||
| server.ehlo() | ||
| server.starttls() | ||
| server.ehlo() | ||
| else: | ||
| server = smtplib.SMTP(smtp_host, smtp_port) | ||
|
|
||
| try: | ||
| if env.smtp.username and env.smtp.password: | ||
| server.login(env.smtp.username, env.smtp.password) | ||
| server.sendmail(from_email, [to_email], msg.as_string()) | ||
| finally: | ||
| server.quit() |
There was a problem hiding this comment.
Move SMTP connection setup inside a context-managed try scope.
At Line 59-Line 63, connection/handshake exceptions can occur before the finally block at Line 70-Line 71, which risks leaving sockets unclosed. Wrap client lifecycle with with smtplib.SMTP(...) as server: (or include setup in the try) to guarantee cleanup.
Comment on lines
+929
to
+943
| class SmtpConfig(BaseModel): | ||
| """SMTP Email configuration""" | ||
|
|
||
| host: str | None = os.getenv("SMTP_HOST") | ||
| port: int = int(os.getenv("SMTP_PORT", "587")) | ||
| username: str | None = os.getenv("SMTP_USERNAME") | ||
| password: str | None = os.getenv("SMTP_PASSWORD") | ||
| from_address: str | None = ( | ||
| os.getenv("SMTP_FROM_ADDRESS") | ||
| or os.getenv("SENDGRID_FROM_ADDRESS") | ||
| 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.
Add explicit SMTP_USE_SSL support to the SMTP config surface.
SmtpConfig only exposes SMTP_USE_TLS; SSL-only SMTP setups (implicit TLS, commonly port 465) cannot be configured through env.smtp, which can break OTP/reset email delivery in those deployments. Please add SMTP_USE_SSL in config and wire it through the SMTP transport path.
Suggested config addition
class SmtpConfig(BaseModel):
@@
- use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in ("true", "1", "yes")
+ use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in _TRUTHY
+ use_ssl: bool = os.getenv("SMTP_USE_SSL", "false").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
| class SmtpConfig(BaseModel): | |
| """SMTP Email configuration""" | |
| host: str | None = os.getenv("SMTP_HOST") | |
| port: int = int(os.getenv("SMTP_PORT", "587")) | |
| username: str | None = os.getenv("SMTP_USERNAME") | |
| password: str | None = os.getenv("SMTP_PASSWORD") | |
| from_address: str | None = ( | |
| os.getenv("SMTP_FROM_ADDRESS") | |
| or os.getenv("SENDGRID_FROM_ADDRESS") | |
| 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") | |
| class SmtpConfig(BaseModel): | |
| """SMTP Email configuration""" | |
| host: str | None = os.getenv("SMTP_HOST") | |
| port: int = int(os.getenv("SMTP_PORT", "587")) | |
| username: str | None = os.getenv("SMTP_USERNAME") | |
| password: str | None = os.getenv("SMTP_PASSWORD") | |
| from_address: str | None = ( | |
| os.getenv("SMTP_FROM_ADDRESS") | |
| or os.getenv("SENDGRID_FROM_ADDRESS") | |
| 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") | |
| use_ssl: bool = os.getenv("SMTP_USE_SSL", "false").lower() in ("true", "1", "yes") |
Comment on lines
+64
to
+70
| 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", | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify established OSS service patterns for SDK usage and boundary validation.
rg -n -C3 'getAgentaSdkClient\(|safeParseWithLogging\(|fetchJson\(' web/oss/src/servicesRepository: Agenta-AI/agenta
Length of output: 8411
Switch resetPassword to the Fern SDK client + boundary zod validation
In web/oss/src/services/profile/index.ts (lines 64-70), resetPassword constructs a URL and calls fetchJson(...) directly (setting user_id via url.searchParams), bypassing the Fern-generated client conventions. Update it to use getAgentaSdkClient({ host: getAgentaApiUrl() }) and apply safeParseWithLogging at the API boundary.
Source: Coding guidelines
Comment on lines
+162
to
+168
| const allRows = page.locator("[data-row-key]") | ||
| const rowCount = await allRows.count() | ||
| if (rowCount > 0) { | ||
| // All visible rows should contain the search term | ||
| for (let i = 0; i < rowCount; i++) { | ||
| await expect(allRows.nth(i)).toContainText(queueName) | ||
| } |
There was a problem hiding this comment.
Use visible rows for post-search assertions to avoid flaky failures.
Line 163 currently counts all attached rows, but Line 165 says this should validate visible rows. In virtualized/filtered tables, hidden rows may remain in the DOM and fail toContainText even when search is correct.
Suggested fix
- const allRows = page.locator("[data-row-key]")
- const rowCount = await allRows.count()
+ const visibleRows = page.locator("[data-row-key]:visible")
+ const rowCount = await visibleRows.count()
if (rowCount > 0) {
// All visible rows should contain the search term
for (let i = 0; i < rowCount; i++) {
- await expect(allRows.nth(i)).toContainText(queueName)
+ await expect(visibleRows.nth(i)).toContainText(queueName)
}
}📝 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
| const allRows = page.locator("[data-row-key]") | |
| const rowCount = await allRows.count() | |
| if (rowCount > 0) { | |
| // All visible rows should contain the search term | |
| for (let i = 0; i < rowCount; i++) { | |
| await expect(allRows.nth(i)).toContainText(queueName) | |
| } | |
| const visibleRows = page.locator("[data-row-key]:visible") | |
| const rowCount = await visibleRows.count() | |
| if (rowCount > 0) { | |
| // All visible rows should contain the search term | |
| for (let i = 0; i < rowCount; i++) { | |
| await expect(visibleRows.nth(i)).toContainText(queueName) | |
| } |
Comment on lines
1625
to
1629
| if (!serverData) { | ||
| return !!entityData | ||
| // No server baseline (ephemeral / newly created entity) — | ||
| // treat as clean until the user makes actual edits. | ||
| return false | ||
| } |
There was a problem hiding this comment.
No-baseline branch hides real edits for ephemeral workflows.
At Line 1626, if (!serverData) return false makes every ephemeral/local-without-baseline workflow permanently clean, even after user edits. workflowServerDataSelectorFamily intentionally returns null for ephemeral entities, so this branch suppresses unsaved-change detection in that flow.
💡 Suggested fix
if (!serverData) {
- // No server baseline (ephemeral / newly created entity) —
- // treat as clean until the user makes actual edits.
- return false
+ // No server baseline (ephemeral/new entity):
+ // clean by default, but mark dirty once a local draft exists.
+ if (!isLocal) return false
+ const draft = get(workflowDraftAtomFamily(workflowId))
+ return !!draft
}
Comment on lines
+718
to
+719
| if (bodyStatus && typeof bodyStatus === "object" && bodyStatus.code && bodyStatus.code !== 200) { | ||
| const traceId = extractTraceIdFromPayload(responseData) |
There was a problem hiding this comment.
Normalize bodyStatus.code before the non-200 check.
bodyStatus.code !== 200 misclassifies "200" (string) as an error. Given the upstream status contract can return string codes, this can incorrectly route successful responses into the failure 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 != null
+ ? 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
fix/passwordless-email-delivery-4559
branch
from
dbe0840 to
cdc05d2
Compare
There was a problem hiding this comment.
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/EvalRunDetails/components/Page.tsx (1)
28-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
"custom"to theevaluationTypeunion type.The
typeMapat line 51 now includes acustomentry, but theevaluationTypeparameter type excludes it. This prevents the component from accepting custom evaluations and makes the new breadcrumb mapping unreachable.🔧 Proposed fix
- evaluationType: "auto" | "human" | "online" + evaluationType: "auto" | "human" | "online" | "custom"
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 05947381-54d4-415d-b2f3-f920c5a5c589
📒 Files selected for processing (43)
api/ee/src/services/db_manager_ee.pyapi/ee/src/services/organization_service.pyapi/oss/src/core/auth/supertokens/config.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/EvalRunDetails/components/Page.tsxweb/oss/src/components/EvalRunDetails/test.tsxweb/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/lib/helpers/buildBreadcrumbs.tsweb/oss/src/services/evaluations/invocations/api.tsweb/oss/src/services/profile/index.tsweb/oss/tests/playwright/acceptance/annotation-queue/annotation-queue.spec.tsweb/oss/tests/playwright/acceptance/annotation-queue/assets/types.tsweb/oss/tests/playwright/acceptance/annotation-queue/index.tsweb/oss/tests/playwright/acceptance/annotation-queue/tests.tsweb/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsxweb/packages/agenta-entities/src/runnable/types.tsweb/packages/agenta-entities/src/workflow/state/store.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)
- api/pyproject.toml
- web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
- api/ee/src/services/db_manager_ee.py
✅ Files skipped from review due to trivial changes (4)
- web/oss/tests/playwright/acceptance/annotation-queue/annotation-queue.spec.ts
- hosting/docker-compose/oss/env.oss.dev.example
- docs/docs/self-host/02-configuration.mdx
- web/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsx
🚧 Files skipped from review as they are similar to previous changes (32)
- web/oss/src/components/EvalRunDetails/atoms/runInvocationAction.ts
- web/packages/agenta-entities/src/workflow/state/store.ts
- web/packages/agenta-entities/src/runnable/types.ts
- web/oss/src/components/SharedDrawers/TraceDrawer/components/EvaluatorDetailsPopover.tsx
- web/oss/src/services/profile/index.ts
- web/packages/agenta-playground/src/state/execution/types.ts
- web/oss/src/lib/helpers/buildBreadcrumbs.ts
- web/oss/src/components/TestcasesTableNew/components/TestcasesTableShell.tsx
- web/packages/agenta-entity-ui/src/shared/EntityTable.tsx
- web/oss/src/components/pages/observability/components/ObservabilityTable/index.tsx
- hosting/docker-compose/ee/env.ee.dev.example
- hosting/docker-compose/oss/env.oss.gh.example
- api/oss/src/services/user_service.py
- web/oss/src/components/EvalRunDetails/test.tsx
- web/oss/src/components/pages/observability/components/SessionsTable/index.tsx
- web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useEntityTableState.ts
- web/oss/tests/playwright/acceptance/annotation-queue/assets/types.ts
- sdks/python/agenta/sdk/middlewares/running/vault.py
- web/oss/src/components/pages/prompts/PromptsPage.tsx
- hosting/docker-compose/ee/env.ee.gh.example
- web/packages/agenta-ui/src/InfiniteVirtualTable/hooks/useTableManager.tsx
- web/packages/agenta-annotation-ui/src/components/AnnotationSession/ScenarioListView.tsx
- api/oss/src/services/organization_service.py
- api/oss/src/services/email_service.py
- web/oss/src/components/pages/settings/WorkspaceManage/cellRenderers.tsx
- api/oss/src/core/auth/supertokens/config.py
- web/packages/agenta-playground/src/state/execution/executionRunner.ts
- web/oss/tests/playwright/acceptance/annotation-queue/index.ts
- web/oss/tests/playwright/acceptance/annotation-queue/tests.ts
- api/ee/src/services/organization_service.py
- web/oss/src/components/InfiniteVirtualTable/hooks/useTableManager.tsx
- api/oss/src/utils/env.py
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.
Problem
SuperTokens passwordless OTP emails use SuperTokens' built-in SMTP transporter, which only exposes a
secureflag (mapped fromSMTP_USE_SSL). Whensecure=False, it still silently attempts STARTTLS — diverging from the sharedemail_service._send_via_smtpwhich only callsstarttls()whenSMTP_USE_TLS=true. (#4559)This means OTP and application emails can behave differently for the same SMTP configuration, which is especially problematic for self-hosted users with specific TLS requirements.
Solution
Implement a custom
EmailDeliveryOverrideInputfor the passwordless recipe that delegates to the sharedemail_serviceinstead of using SuperTokens' built-in SMTP transporter.What changed
api/oss/src/core/auth/supertokens/config.py(+104 lines, -11 lines):PasswordlessEmailService— implements SuperTokens'EmailDeliveryInterface[PasswordlessLoginEmailTemplateVars]by delegating toemail_service._send_via_smtp(SMTP) or_send_via_sendgrid(SendGrid fallback)get_passwordless_email_delivery()— returnsEmailDeliveryConfig(service=PasswordlessEmailService())when an email provider is configured, orNoneotherwisepasswordless.init()— now passesemail_delivery=get_passwordless_email_delivery()Benefits
SMTP_USE_TLSis honoured identically for OTP and application emailsTesting
AUTH_METHOD=otp+ SMTP configured withSMTP_USE_TLS=true→ OTP sent via_send_via_smtpwith STARTTLSSMTP_USE_TLS=false→ OTP sent without STARTTLSSENDGRID_API_KEYconfigured → OTP sent via SendGridAUTH_METHOD=password→ no change (passwordless recipe not initialized)Fixes #4559