[FE chore] Move secrets and tools to monorepo packages#4338
Merged
ardaerzin merged 23 commits intoMay 19, 2026
Merged
Conversation
…shared - Move User, LlmProvider, llmAvailableProviders, removeEmptyFromObjects to @agenta/shared so package code can consume them without reaching back into @/oss/*. - Add a primitive userAtom + setUserAtom in @agenta/shared/state, matching the projectIdAtom / sessionAtom pattern: package-readable primitive, populated by app code. - Add OSS UserListener mirroring SessionListener to keep the shared atom synced with the rich profile state. Mounted in Providers.tsx alongside the existing listeners. - Sweep OSS consumers to import directly from @agenta/shared/types (no thin re-exports through @/oss/lib/Types or @/oss/lib/helpers/*), consistent with the no-restricted-syntax lint rule for tree-shaking.
Vault-backed secret storage for LLM provider keys, scoped per project and gated by user authentication. Reference implementation of the entities/molecule pattern at its smallest viable shape — query and mutation only: - No draft semantics - No imperative get/set surface - No isDirty tracking Vault has no artifact-revision concept, so adding those slots would only contribute dead methods. The hook name (useVaultSecret) and return shape are preserved verbatim from the legacy OSS implementation so consumer migration is an import-path refactor rather than a rewrite. The query key (["vault", "secrets", user?.id, projectId]) and migration atom semantics (idempotency, logout-reset) are byte-identical with the legacy source. Includes a README documenting the folder shape, the explicit "no full molecule API" decision, and the canonical homes for cross-cutting types and atoms the package depends on.
Sweep the 9 OSS consumer files (ModelRegistry modals/drawer, settings/Secrets, app management, custom workflow modal, evaluations, prompts) to import useVaultSecret from @agenta/entities/secret. Same hook name, same return shape — purely an import-path refactor. Update ConfigureProviderDrawer to import SecretDTOProvider, PROVIDER_KINDS, and PROVIDER_LABELS from the new secret package directly, consistent with the no-restricted-syntax rule. Drop the legacy barrel re-exports (./atoms/vault, ./hooks/useVaultSecret) from web/oss/src/state/app/index.ts and update the SelectLLMProvider example to point at the new path. Remove the legacy OSS source: - web/oss/src/services/vault/ - web/oss/src/state/app/atoms/vault.ts - web/oss/src/state/app/hooks/useVaultSecret.ts - web/oss/src/hooks/useVaultSecret.ts - web/oss/src/lib/helpers/llmProviders.ts
…end-chore/move-secrets-and-tools-to-entities
… hook to @agenta/shared
…gatewayTool and @agenta/entity-ui/gatewayTool
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughExtracts gateway tools and vault secrets into shared packages with Fern-backed APIs, updates OSS and entity-UI components to new types/hooks, adds shared user/types/utils, and removes legacy OSS-specific types/hooks. ChangesGateway Tool entity extraction and OSS/UI rewiring
Secret/Vault entity extraction and OSS migration
Shared types/state/utils and OSS user sync
Sequence Diagram(s)sequenceDiagram
participant OSS_UI as OSS UI
participant Entities_Hooks as Entities Hooks
participant Entities_API as Entities API
participant SDK as Agenta SDK
OSS_UI->>Entities_Hooks: useConnectionsQuery/useCatalogActions
Entities_Hooks->>Entities_API: queryConnections/fetchActions
Entities_API->>SDK: tools client (projectScopedRequest)
SDK-->>Entities_API: responses
Entities_API-->>Entities_Hooks: Tool* typed data
Entities_Hooks-->>OSS_UI: connections/actions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
dosubot
Bot
added
the
size:XL
dosubot
Bot
added
Frontend
refactoring
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web/packages/agenta-entity-ui/src/gatewayTool/drawers/ConnectDrawer.tsx (1)
92-104:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard
redirect_urltype before opening or assigning.At Line 94, truthiness alone is not enough; Lines 97 and 104 cast unknown data to
string. If backend payload drifts, this can navigate to an invalid URL stringification.Suggested fix
- const redirectUrl = (result.connection?.data as Record<string, unknown> | undefined) - ?.redirect_url - if (redirectUrl) { + const redirectUrl = (result.connection?.data as Record<string, unknown> | undefined) + ?.redirect_url + if (typeof redirectUrl === "string" && redirectUrl) { // Composio handles all auth (OAuth and API key) via their redirect UI const popup = window.open( - redirectUrl as string, + redirectUrl, "tools_oauth", "width=600,height=700,popup=yes", ) if (!popup) { setLoading(false) message.warning("Popup blocked. Redirecting in this tab.") - window.location.assign(redirectUrl as string) + window.location.assign(redirectUrl) return }web/oss/src/components/pages/settings/Tools/components/GatewayToolsSection.tsx (1)
74-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard popup-blocked flow and make refresh finalization single-shot.
Line 74 can return
nullwhen popups are blocked; the current flow still registers long-lived listeners/timers. Also, cleanup can run twice (message + polling), causing duplicate invalidation/toasts.Suggested fix
- const popup = window.open( + const popup = window.open( redirectUrl, "tools_oauth", "width=600,height=700,popup=yes", ) + if (!popup) { + message.warning("Popup was blocked. Please allow popups and try again.") + return + } const cleanup = async () => { window.focus() // Poll the individual connection endpoint which checks // Composio for status and updates is_valid in the DB. @@ - const handler = (event: MessageEvent) => { + let finalized = false + let pollTimer: ReturnType<typeof setInterval> | undefined + const finalize = () => { + if (finalized) return + finalized = true + if (pollTimer) clearInterval(pollTimer) + window.removeEventListener("message", handler) + void cleanup() + } + + const handler = (event: MessageEvent) => { if ( event.data?.type === "tools:oauth:complete" && trustedOrigins.has(event.origin) ) { - window.removeEventListener("message", handler) - void cleanup() + finalize() } } window.addEventListener("message", handler) // Fallback: detect popup closed - const pollTimer = setInterval(() => { - if (popup && popup.closed) { - clearInterval(pollTimer) - window.removeEventListener("message", handler) - void cleanup() - } - }, 1000) + pollTimer = setInterval(() => { + if (popup.closed) finalize() + }, 1000)web/packages/agenta-entities/src/secret/state/atoms.ts (1)
210-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the existing secret id when deciding update vs create.
Line 212 currently requires
provider.idto update. IffindSecretexists butprovider.idis missing, this falls into create and can duplicate/fail instead of updating the existing vault record.💡 Suggested fix
- if (findSecret && provider.id) { - await updateMutation.mutateAsync({secret_id: provider.id, payload}) + const secretId = findSecret?.id ?? provider.id + if (secretId) { + await updateMutation.mutateAsync({secret_id: secretId, payload}) } else { await createMutation.mutateAsync(payload) }
🧹 Nitpick comments (4)
web/packages/agenta-shared/src/utils/llmProviders.ts (1)
20-34: ⚡ Quick winRemove the empty
keyfield fromllmAvailableProvidersentries.The
key: ""field is defined in every provider entry but is never used. Search shows no code accesses this field from the array; only thenamefield is used for provider matching. Either remove it to reduce clutter or document its intended purpose if it's reserved for future use.web/packages/agenta-entities/src/gatewayTool/api/api.ts (1)
31-34: ⚡ Quick winPrefer named
interfacedeclarations for exported parameter object shapes.These inline object types work, but using named interfaces here improves reuse/readability and aligns with repo standards.
As per coding guidelines, "Prefer
interfacefor defining object shapes in TypeScript".Also applies to: 60-66, 98-101
web/packages/agenta-entities/src/gatewayTool/hooks/useActionDetail.ts (1)
11-12: ⚡ Quick winUse a named
interfacefor the atom-family argument shape.Inline object-shape typing here can be replaced with a small interface to match project conventions and improve reuse.
As per coding guidelines, "Prefer
interfacefor defining object shapes in TypeScript".web/packages/agenta-entities/src/gatewayTool/prompt/atoms.ts (1)
34-43: ⚡ Quick winUse
interfacefor these local object-shape declarations.
EnhancedPromptandEnhancedToolare object-shape definitions; convert them tointerfaceto match repo TypeScript conventions.♻️ Suggested change
- type EnhancedPrompt = { + interface EnhancedPrompt extends Record<string, unknown> { __id?: string __name?: string llm_config?: Record<string, unknown> llmConfig?: Record<string, unknown> - } & Record<string, unknown> - type EnhancedTool = { + } + interface EnhancedTool extends Record<string, unknown> { value?: {function?: {name?: string}} __tool?: string - } & Record<string, unknown> + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 254519d3-15de-484b-839d-49640463d9c4
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (98)
web/oss/src/components/DrillInView/OSSdrillInUIProvider.tsxweb/oss/src/components/ModelRegistry/Drawers/ConfigureProviderDrawer/assets/ConfigureProviderDrawerContent.tsxweb/oss/src/components/ModelRegistry/Drawers/ConfigureProviderDrawer/assets/constants.tsweb/oss/src/components/ModelRegistry/Drawers/ConfigureProviderDrawer/assets/types.d.tsweb/oss/src/components/ModelRegistry/Modals/ConfigureProviderModal/assets/types.d.tsweb/oss/src/components/ModelRegistry/Modals/ConfigureProviderModal/index.tsxweb/oss/src/components/ModelRegistry/Modals/DeleteProviderModal/assets/types.d.tsweb/oss/src/components/ModelRegistry/Modals/DeleteProviderModal/index.tsxweb/oss/src/components/Playground/Components/PlaygroundGenerations/assets/GatewayToolExecuteButton.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfigPrompt/assets/GatewayToolsPanel.tsxweb/oss/src/components/Playground/Playground.tsxweb/oss/src/components/Sidebar/hooks/useDropdownItems/types.d.tsweb/oss/src/components/pages/app-management/modals/CustomWorkflowModal/hooks/useCustomWorkflowConfig.tsxweb/oss/src/components/pages/evaluations/NewEvaluation/Components/NewEvaluationModalInner.tsxweb/oss/src/components/pages/settings/Secrets/SecretProviderTable/index.tsxweb/oss/src/components/pages/settings/Tools/components/ActionsList.tsxweb/oss/src/components/pages/settings/Tools/components/ConnectionsList.tsxweb/oss/src/components/pages/settings/Tools/components/GatewayToolsSection.tsxweb/oss/src/components/pages/settings/Tools/components/IntegrationGrid.tsxweb/oss/src/components/pages/settings/Tools/hooks/useIntegrationDetail.tsweb/oss/src/components/pages/settings/Tools/hooks/useToolsConnections.tsweb/oss/src/components/pages/settings/Tools/hooks/useToolsIntegrations.tsweb/oss/src/components/pages/settings/WorkspaceManage/cellRenderers.tsxweb/oss/src/features/gateway-tools/components/ScrollSentinel.tsxweb/oss/src/features/gateway-tools/components/ScrollToTopButton.tsxweb/oss/src/features/gateway-tools/index.tsweb/oss/src/features/gateway-tools/utils/slugify.tsweb/oss/src/hooks/useLLMProviderConfig.tsxweb/oss/src/hooks/useVaultSecret.tsweb/oss/src/lib/Types.tsweb/oss/src/lib/helpers/llmProviders.tsweb/oss/src/lib/helpers/utils.tsweb/oss/src/services/app-selector/api/index.tsweb/oss/src/services/tools/api/index.tsweb/oss/src/services/tools/api/types.tsweb/oss/src/services/vault/api/index.tsweb/oss/src/state/Providers.tsxweb/oss/src/state/app/index.tsweb/oss/src/state/org/selectors/org.tsweb/oss/src/state/profile/UserListener.tsxweb/oss/src/state/profile/selectors/user.tsweb/oss/src/state/project/selectors/project.tsweb/packages/agenta-entities/package.jsonweb/packages/agenta-entities/src/gatewayTool/api/api.tsweb/packages/agenta-entities/src/gatewayTool/api/client.tsweb/packages/agenta-entities/src/gatewayTool/api/index.tsweb/packages/agenta-entities/src/gatewayTool/core/index.tsweb/packages/agenta-entities/src/gatewayTool/core/types.tsweb/packages/agenta-entities/src/gatewayTool/hooks/index.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useActionDetail.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useCatalogActions.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useCatalogIntegrations.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useConnectionActions.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useConnectionQuery.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useConnectionsQuery.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useIntegrationConnections.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useIntegrationDetail.tsweb/packages/agenta-entities/src/gatewayTool/hooks/useToolExecution.tsweb/packages/agenta-entities/src/gatewayTool/index.tsweb/packages/agenta-entities/src/gatewayTool/prompt/atoms.tsweb/packages/agenta-entities/src/gatewayTool/prompt/index.tsweb/packages/agenta-entities/src/gatewayTool/state/atoms.tsweb/packages/agenta-entities/src/gatewayTool/state/index.tsweb/packages/agenta-entities/src/index.tsweb/packages/agenta-entities/src/secret/README.mdweb/packages/agenta-entities/src/secret/api/api.tsweb/packages/agenta-entities/src/secret/api/client.tsweb/packages/agenta-entities/src/secret/api/index.tsweb/packages/agenta-entities/src/secret/core/index.tsweb/packages/agenta-entities/src/secret/core/transforms.tsweb/packages/agenta-entities/src/secret/core/types.tsweb/packages/agenta-entities/src/secret/index.tsweb/packages/agenta-entities/src/secret/state/atoms.tsweb/packages/agenta-entities/src/secret/state/index.tsweb/packages/agenta-entities/src/secret/state/useVaultSecret.tsweb/packages/agenta-entity-ui/package.jsonweb/packages/agenta-entity-ui/src/gatewayTool/components/ConnectionStatusBadge.tsxweb/packages/agenta-entity-ui/src/gatewayTool/components/ResultViewer.tsxweb/packages/agenta-entity-ui/src/gatewayTool/components/SchemaForm.tsxweb/packages/agenta-entity-ui/src/gatewayTool/drawers/CatalogDrawer.tsxweb/packages/agenta-entity-ui/src/gatewayTool/drawers/ConnectDrawer.tsxweb/packages/agenta-entity-ui/src/gatewayTool/drawers/ConnectionManagerDrawer.tsxweb/packages/agenta-entity-ui/src/gatewayTool/drawers/ToolExecutionDrawer.tsxweb/packages/agenta-entity-ui/src/gatewayTool/index.tsweb/packages/agenta-shared/src/api/env.tsweb/packages/agenta-shared/src/api/index.tsweb/packages/agenta-shared/src/hooks/index.tsweb/packages/agenta-shared/src/hooks/useDebouncedAtomSearch.tsweb/packages/agenta-shared/src/state/index.tsweb/packages/agenta-shared/src/state/user.tsweb/packages/agenta-shared/src/types/index.tsweb/packages/agenta-shared/src/types/llmProvider.tsweb/packages/agenta-shared/src/types/user.tsweb/packages/agenta-shared/src/utils/gatewayToolSchema.tsweb/packages/agenta-shared/src/utils/index.tsweb/packages/agenta-shared/src/utils/llmProviders.tsweb/packages/agenta-shared/src/utils/objectUtils.tsweb/packages/agenta-ui/src/SelectLLMProvider/README.md
💤 Files with no reviewable changes (10)
- web/oss/src/services/tools/api/types.ts
- web/oss/src/hooks/useVaultSecret.ts
- web/oss/src/lib/helpers/llmProviders.ts
- web/oss/src/features/gateway-tools/utils/slugify.ts
- web/oss/src/features/gateway-tools/components/ScrollToTopButton.tsx
- web/oss/src/features/gateway-tools/components/ScrollSentinel.tsx
- web/oss/src/state/app/index.ts
- web/oss/src/services/tools/api/index.ts
- web/oss/src/services/vault/api/index.ts
- web/oss/src/features/gateway-tools/index.ts
Comment on lines
+113
to
+117
| // Secret-domain types (HeaderDTO, StandardSecretDTO, CustomSecretDTO, | ||
| // VaultData, SecretDTOKind, SecretDTOProvider, PROVIDER_KINDS, | ||
| // PROVIDER_LABELS, etc.) live in @agenta/entities/secret. Import from | ||
| // there directly — re-exporting from @agenta packages is disallowed | ||
| // (see eslint no-restricted-syntax rule for tree-shaking). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any imports of the removed types from `@/oss/lib/Types`
echo "=== Checking for imports of removed secret types from Types.ts ==="
rg -n --type=ts --type=tsx "from ['\"]`@/oss/lib/Types`['\"]" -A5 | rg -i "(HeaderDTO|StandardSecret|SecretDTO|SecretDTOKind|SecretDTOProvider|PROVIDER_LABELS|PROVIDER_KINDS|CustomSecretDTO)" || echo "No secret type imports found"
echo ""
echo "=== Checking for User imports from Types.ts ==="
rg -n --type-add 'ts:*.{ts,tsx}' --type=ts "import.*User.*from ['\"]`@/oss/lib/Types`['\"]" || echo "No User imports from Types.ts found"
echo ""
echo "=== Checking for any remaining usage of removed constants ==="
rg -n --type-add 'ts:*.{ts,tsx}' --type=ts "\b(PROVIDER_LABELS|PROVIDER_KINDS)\b" web/oss/src/ || echo "No PROVIDER_LABELS/PROVIDER_KINDS usage found"Repository: Agenta-AI/agenta
Length of output: 1424
🏁 Script executed:
#!/bin/bash
# Get the actual import line from the billing atoms file
echo "=== Actual import statement in billing atoms.ts ==="
sed -n '1,20p' web/ee/src/state/billing/atoms.ts | cat -n
echo ""
echo "=== Check if User exists in `@agenta/shared/types` ==="
# Find all files in shared/types
find . -path "./web/*/node_modules" -prune -o -type f -name "*types*" -path "*/shared/*" -print | head -20
echo ""
echo "=== Search for User export in shared package ==="
rg "export.*User" --type=ts --type=tsx | grep -i "shared\|types" | head -20
echo ""
echo "=== Check actual imports in ConfigureProviderDrawerContent.tsx ==="
head -20 web/oss/src/components/ModelRegistry/Drawers/ConfigureProviderDrawer/assets/ConfigureProviderDrawerContent.tsx | cat -nRepository: Agenta-AI/agenta
Length of output: 2388
🏁 Script executed:
#!/bin/bash
echo "=== Search for all User type definitions/exports ==="
rg "export.*type.*User\b|export.*interface.*User\b" --type=ts --type=tsx | head -30
echo ""
echo "=== Check `@agenta/shared/types` exports ==="
find . -path "*/node_modules" -prune -o -path "*/web/packages/agenta-shared/*" -type f -name "*.ts" -print | xargs grep -l "export.*User" 2>/dev/null | head -10
echo ""
echo "=== Check the exact file at `@agenta/shared/types` path ==="
ls -la web/packages/agenta-shared/src/types/ 2>/dev/null || ls -la web/packages/agenta-shared/src/ 2>/dev/null || find web/packages/agenta-shared -name "types.ts" -o -name "types" -type d
echo ""
echo "=== Full content search for User type in agenta-shared ==="
rg "User" web/packages/agenta-shared --type=ts --type=tsx -A2 -B2 | head -50Repository: Agenta-AI/agenta
Length of output: 895
🏁 Script executed:
#!/bin/bash
echo "=== Check User type in agenta-shared/src/types/user.ts ==="
cat -n web/packages/agenta-shared/src/types/user.ts
echo ""
echo "=== Check index exports in agenta-shared/src/types/index.ts ==="
cat -n web/packages/agenta-shared/src/types/index.ts
echo ""
echo "=== Check what's exported from agenta-shared/src/state/user.ts ==="
cat -n web/packages/agenta-shared/src/state/user.ts
echo ""
echo "=== Check current Types.ts to see what User was exported ==="
grep -n "User" web/oss/src/lib/Types.ts | head -20Repository: Agenta-AI/agenta
Length of output: 3028
Migrate remaining User import from removed location in web/ee/src/state/billing/atoms.ts.
The User type has been removed from this file. One consumer was missed during migration: web/ee/src/state/billing/atoms.ts line 6 still imports from @/oss/lib/Types. Update it to import from @agenta/shared/types instead (the User type is properly exported there). The secret-domain types have been correctly migrated by consumers.
Comment on lines
+57
to
+76
| export const fetchActions = async ( | ||
| providerKey: string, | ||
| integrationKey: string, | ||
| params?: { | ||
| query?: string | ||
| categories?: string[] | ||
| limit?: number | ||
| cursor?: string | ||
| important?: boolean | ||
| }, | ||
| ): Promise<ToolCatalogActionsResponse> => { | ||
| return getToolsClient().listToolActions( | ||
| { | ||
| provider_key: providerKey, | ||
| integration_key: integrationKey, | ||
| query: params?.query, | ||
| categories: params?.categories, | ||
| limit: params?.limit, | ||
| cursor: params?.cursor, | ||
| }, |
There was a problem hiding this comment.
important filter is accepted but never sent to the backend.
fetchActions exposes important (Line 65) but the request payload omits it, so callers cannot actually filter by importance.
Proposed fix
export const fetchActions = async (
@@
return getToolsClient().listToolActions(
{
provider_key: providerKey,
integration_key: integrationKey,
query: params?.query,
categories: params?.categories,
+ important: params?.important,
limit: params?.limit,
cursor: params?.cursor,
},
projectScopedRequest(),
)
}📝 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 fetchActions = async ( | |
| providerKey: string, | |
| integrationKey: string, | |
| params?: { | |
| query?: string | |
| categories?: string[] | |
| limit?: number | |
| cursor?: string | |
| important?: boolean | |
| }, | |
| ): Promise<ToolCatalogActionsResponse> => { | |
| return getToolsClient().listToolActions( | |
| { | |
| provider_key: providerKey, | |
| integration_key: integrationKey, | |
| query: params?.query, | |
| categories: params?.categories, | |
| limit: params?.limit, | |
| cursor: params?.cursor, | |
| }, | |
| export const fetchActions = async ( | |
| providerKey: string, | |
| integrationKey: string, | |
| params?: { | |
| query?: string | |
| categories?: string[] | |
| limit?: number | |
| cursor?: string | |
| important?: boolean | |
| }, | |
| ): Promise<ToolCatalogActionsResponse> => { | |
| return getToolsClient().listToolActions( | |
| { | |
| provider_key: providerKey, | |
| integration_key: integrationKey, | |
| query: params?.query, | |
| categories: params?.categories, | |
| important: params?.important, | |
| limit: params?.limit, | |
| cursor: params?.cursor, | |
| }, | |
| projectScopedRequest(), | |
| ) | |
| } |
Comment on lines
+13
to
+36
| export const removeEmptyFromObjects = <T = unknown>(obj: T): T => { | ||
| if (Array.isArray(obj)) { | ||
| return obj | ||
| .map((item) => removeEmptyFromObjects(item)) | ||
| .filter( | ||
| (item) => | ||
| item != null && | ||
| (typeof item !== "object" || Object.keys(item as object).length > 0), | ||
| ) as unknown as T | ||
| } | ||
| if (obj && typeof obj === "object") { | ||
| return Object.entries(obj as Record<string, unknown>).reduce( | ||
| (acc, [key, value]) => { | ||
| const cleaned = removeEmptyFromObjects(value) | ||
| if (cleaned !== null && cleaned !== undefined && cleaned !== "") { | ||
| acc[key] = cleaned | ||
| } | ||
| return acc | ||
| }, | ||
| {} as Record<string, unknown>, | ||
| ) as unknown as T | ||
| } | ||
| return obj | ||
| } |
There was a problem hiding this comment.
Implementation doesn't match documentation for empty arrays and objects in object values.
The documentation claims to remove "empty objects ({})" and "empty arrays after recursive cleanup," but the implementation is inconsistent:
Array items (lines 17-21): Empty arrays and objects ARE filtered via Object.keys(item).length > 0.
Object values (line 27): Only checks !== null, !== undefined, and !== "", so empty arrays and objects are KEPT.
Example demonstrating the bug:
removeEmptyFromObjects({outer: [{inner: []}, {}]})
// Returns: {outer: []} — array items filtered correctly
removeEmptyFromObjects({a: [], b: {}})
// Returns: {a: [], b: {}} — object values NOT filtered🛡️ Proposed fix to align implementation with documentation
export const removeEmptyFromObjects = <T = unknown>(obj: T): T => {
if (Array.isArray(obj)) {
return obj
.map((item) => removeEmptyFromObjects(item))
.filter(
(item) =>
item != null &&
(typeof item !== "object" || Object.keys(item as object).length > 0),
) as unknown as T
}
if (obj && typeof obj === "object") {
return Object.entries(obj as Record<string, unknown>).reduce(
(acc, [key, value]) => {
const cleaned = removeEmptyFromObjects(value)
- if (cleaned !== null && cleaned !== undefined && cleaned !== "") {
+ if (
+ cleaned !== null &&
+ cleaned !== undefined &&
+ cleaned !== "" &&
+ !(Array.isArray(cleaned) && cleaned.length === 0) &&
+ !(typeof cleaned === "object" && Object.keys(cleaned as object).length === 0)
+ ) {
acc[key] = cleaned
}
return acc
},
{} as Record<string, unknown>,
) as unknown as T
}
return obj
}
Contributor
Railway Preview Environment
Updated at 2026-05-19T19:27:42.448Z |
…OSS _app Without this the @agenta/sdk singleton falls back to its built-in default host (https://cloud.agenta.ai) because AGENTA_HOST is only set server-side, so staging/preview deployments would issue tools/secrets requests against the production origin and trip CORS.
dosubot
Bot
added
size:L
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/oss/src/components/pages/_app/index.tsx (1)
27-32: 💤 Low valueConsider adding error handling for SDK initialization.
If
getAgentaApiUrl()orgetAgentaSdkClient()throws during module load, the entire app will fail to initialize. Consider wrapping this in a try-catch to handle potential initialization failures gracefully.🛡️ Proposed defensive initialization
-// Pin the workspace Fern-client singleton to the host this deployment -// actually talks to. Without this the SDK defaults to `https://cloud.agenta.ai` -// (its built-in fallback) because `AGENTA_HOST` is only set server-side — -// staging/preview deployments would otherwise issue tools/secrets requests -// against the production origin. -getAgentaSdkClient({host: getAgentaApiUrl()}) +// Pin the workspace Fern-client singleton to the host this deployment +// actually talks to. Without this the SDK defaults to `https://cloud.agenta.ai` +// (its built-in fallback) because `AGENTA_HOST` is only set server-side — +// staging/preview deployments would otherwise issue tools/secrets requests +// against the production origin. +try { + getAgentaSdkClient({host: getAgentaApiUrl()}) +} catch (error) { + console.error("Failed to initialize Agenta SDK:", error) + // SDK will fall back to default host +}
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 285261e9-638a-4b48-9c06-df4357735f88
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
web/oss/package.jsonweb/oss/src/components/pages/_app/index.tsx
bekossy
approved these changes
May 19, 2026
…tools-to-entities
…ation - Forward `important` filter through queryParams in fetchActions (Fern's ListToolActionsRequest doesn't model it, but the backend honours it). - Include port in origin fallback so getAgentaApiUrl/getAgentaWebUrl return the right URL on non-default ports (was dropping :3000 in local dev). - Switch EE billing atoms to import User from @agenta/shared/types (the type was removed from @/oss/lib/Types when secrets were lifted). - Prefer the matched server-side secret id when deciding update vs create so the update path is reached when callers seed LlmProvider without id. - Make removeEmptyFromObjects actually strip empty objects/arrays from object values (the docstring already advertised this, the object branch didn't enforce it). - Use a stable per-row key fallback for connections in GatewayToolsPanel (`integrationKey-index`) so missing id+slug doesn't collide on "". - Guard redirect_url with typeof === "string" in ConnectDrawer instead of casting unknown to string. - Skip opening the execution drawer when a connection lacks id or slug rather than passing empty strings.
…ttps://github.com/Agenta-AI/agenta into frontend-chore/move-secrets-and-tools-to-entities
Addresses remaining CodeRabbit threads on the secrets migration: - createVaultSecret/updateVaultSecret/deleteVaultSecret now take/return the typed Fern request/response shapes derived from getSecretsClient(). The fake <T> generics + `as unknown as` casts are gone; callers get real compile-time checks on payload shape. - transformSecret and getEnvNameMap now share a single STANDARD_PROVIDER_ENV_BY_KIND source of truth (kind → env) — getEnvNameMap derives the reverse via Object.entries. Unmapped standard provider kinds are now logged + dropped instead of surfacing as nameless rows. - Secret mutation atoms no longer reach for getDefaultStore() to read projectId. Callers (createStandardSecretAtom / createCustomSecretAtom / deleteSecretAtom) read it via the atom's own `get` and pass it explicitly via mutateAsync — the mutation atoms themselves are now pure forwarders. - README: added `text` language tag to the folder-shape fenced block, and refreshed the api.ts comment now that it's Fern-backed.
…tools-to-entities # Conflicts: # web/oss/src/lib/Types.ts
…tools-to-entities
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
3 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
tba...
Testing
Verified locally
secrets & tools are working as expected
QA follow-up
vault related features in FE and tools integrations in playground
Checklist
Contributor Resources