[feat] Starter credits: minimal web changes (hidden managed connection, replace-only secrets, add-your-key on exhaustion) - #6174
Conversation
|
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. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) 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:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change propagates run-error codes, adds write-only and managed secret handling, merges stored provider credentials for probing, wires a shared runtime key, and updates starter-credits seeding, generated clients, UI, deployment config, and tests. ChangesSecret visibility and runtime grants
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟠 High · up to This PR changes secret handling, provider probing, credential resolution, and deployment defaults, but the current head still permits stored credentials to be sent to a different provider, exposes credential material through caching or representations, and includes credential-resolution and startup-failure paths. These are high-impact security, correctness, and deployment risks, so the PR is not merge-ready until fixed. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
|
|
e207a29 to
4496dcc
Compare
dosubot
Bot
added
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-entities/src/secret/core/connections.ts (1)
681-698: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve custom-provider credentials during updates.
VaultService.update_secretcarries over onlymodelsandharnesses, while the DAO replacessecrets_dbe.datawith the submitted data. A write-only edit therefore sends emptyprovider.extrasand erases stored credentials. Merge omitted secret fields server-side or omitprovider.extrasfrom partial updates, and add regression tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e34ceaa0-41be-4d85-8b54-c08670bae659
📒 Files selected for processing (47)
web/oss/src/components/AgentChatSlice/components/AgentMessage.runError.test.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsxweb/oss/src/components/AgentChatSlice/components/ConnectModelBanner.tsxweb/oss/src/components/AgentChatSlice/hooks/useChatSlashCommands.tsxweb/oss/src/components/AgentChatSlice/hooks/useOnboardingProviderSetup.tsweb/oss/src/components/pages/settings/Vault/ConfigureSecretModal/index.tsxweb/packages/agenta-chat/src/assets/trace.tsweb/packages/agenta-chat/src/assets/transcriptToMessages.tsweb/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.tsweb/packages/agenta-chat/src/model/turnStatus.tsweb/packages/agenta-chat/src/model/turnViewModel.tsweb/packages/agenta-chat/tests/unit/assets/trace.test.tsweb/packages/agenta-chat/tests/unit/assets/transcriptToMessages.test.tsweb/packages/agenta-chat/tests/unit/model/turnStatus.test.tsweb/packages/agenta-entities/src/secret/api/probe.tsweb/packages/agenta-entities/src/secret/core/connections.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/connections.tsweb/packages/agenta-entities/src/secret/state/persistence.tsweb/packages/agenta-entities/src/workflow/state/agentCreationPrefs.tsweb/packages/agenta-entities/src/workflow/state/appUtils.tsweb/packages/agenta-entities/tests/unit/agent-creation-prefs.test.tsweb/packages/agenta-entities/tests/unit/provider-connections.test.tsweb/packages/agenta-entities/tests/unit/secret-transforms.test.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ProviderCredentialsSectionView.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ProviderKeyField.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/connectionPicker.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/connectionUtils.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/index.tsweb/packages/agenta-entity-ui/src/DrillInView/index.tsweb/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsxweb/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsxweb/packages/agenta-entity-ui/tests/unit/connectionPicker.test.tsweb/packages/agenta-entity-ui/tests/unit/connectionUtils.test.tsweb/packages/agenta-settings-ui/src/providers/AIProvidersPage.tsxweb/packages/agenta-shared/src/state/index.tsweb/packages/agenta-shared/src/state/openProviderDrawer.tsweb/packages/agenta-shared/src/types/llmProvider.tsweb/packages/agenta-ui/src/LLMIcons/assets/Agenta.tsxweb/packages/agenta-ui/src/LLMIcons/index.tsweb/packages/agenta-ui/src/SelectLLMProvider/utils.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
| /** | ||
| * The failed-run callout's one conditional affordance: the "Add your key" button. | ||
| * | ||
| * It appears only for the starter-credit failure classes the user can clear themselves. Every | ||
| * other failure (including a run that carried no code at all) shows the message and nothing more, | ||
| * so a plain crash never nags the user to go buy a provider key. | ||
| * | ||
| * Rendered with `renderToStaticMarkup` rather than a testing library: the repo has no | ||
| * `@testing-library/react`, and these are static presentational assertions that do not need one. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reduce the multi-line test comment.
The test names and assertions already describe this behavior. Delete this block or replace it with one short line.
As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
| /** The content to send: `undefined` keeps the stored value, `null` means the form is invalid. */ | ||
| const buildContent = (): CustomSecretContent | null | undefined => { | ||
| if (format === CustomSecretFormat.Text) { | ||
| if (valueHidden && !textValue) return undefined | ||
| return textValue | ||
| } | ||
| if (valueHidden && jsonView === "grid" && !kvRows.some((row) => row.key.trim())) { | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Track replacement state separately from the JSON view.
For a write-only JSON secret, opening the Editor view serializes the empty grid as {}. If the user saves without entering a replacement, jsonView === "json" bypasses the preserve branch and sends {}. This replaces the stored secret.
Track whether the user supplied replacement content. Send undefined while the content is untouched, regardless of the selected JSON view. If the user changes format, require replacement content before sending the new format.
| async function vaultConnectionsForNewAgent( | ||
| projectId: string, | ||
| userId?: string, | ||
| ): Promise<ProviderConnection[]> { | ||
| try { | ||
| const rows = await getHostQueryClient().ensureQueryData<LlmProvider[]>({ | ||
| queryKey: ["vault", "secrets", userId, projectId], | ||
| queryFn: () => fetchVaultSecret({projectId}), | ||
| staleTime: 5 * 60_000, | ||
| }) | ||
| return toProviderConnections(rows ?? []) | ||
| } catch { | ||
| return [] | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare the vault query key used here with the one used by vaultSecretsQueryAtom.
set -euo pipefail
fd -t f 'atoms.ts' web/packages/agenta-entities/src/secret | xargs rg -n -C 10 'vaultSecretsQueryAtom'
rg -n -C 4 --glob '!**/node_modules/**' '"vault",\s*"secrets"'Repository: Agenta-AI/agenta
Length of output: 3107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- appUtils.ts ---'
sed -n '100,175p' web/packages/agenta-entities/src/workflow/state/appUtils.ts
printf '%s\n' '--- vault query atom ---'
sed -n '85,130p' web/packages/agenta-entities/src/secret/atoms.ts
printf '%s\n' '--- create path and query-client configuration ---'
rg -n -C 8 'createEphemeralAppFromTemplate|vaultConnectionsForNewAgent|ensureQueryData|defaultOptions|retry' \
web/packages/agenta-entities/src web/packages/agenta-sdk/src \
--glob '!**/node_modules/**'Repository: Agenta-AI/agenta
Length of output: 3418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- vault atom locations ---'
fd -t f . web/packages/agenta-entities | rg '(^|/)(atoms|.*secret.*)\.(ts|tsx)$'
rg -n -C 8 'vaultSecretsQueryAtom|userAtom|vaultConnectionsForNewAgent|createEphemeralAppFromTemplate' \
web/packages/agenta-entities --glob '!**/node_modules/**'
printf '%s\n' '--- query-client configuration ---'
rg -n -C 10 'getHostQueryClient|QueryClient|defaultOptions|retry' \
web/packages/agenta-entities web --glob '!**/node_modules/**' \
--glob '*.{ts,tsx,js,jsx}'Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact vault atom matches ---'
rg -l 'vaultSecretsQueryAtom' web/packages/agenta-entities web/oss --glob '*.{ts,tsx}'
rg -n -C 6 'vaultSecretsQueryAtom' web/packages/agenta-entities web/oss --glob '*.{ts,tsx}'
printf '%s\n' '--- app create call site ---'
rg -n -C 12 'vaultConnectionsForNewAgent|userId:|userAtom' \
web/packages/agenta-entities/src/workflow/state/appUtils.ts --glob '*.ts'
printf '%s\n' '--- host query client references ---'
rg -l 'getHostQueryClient' web/packages/agenta-entities web/oss --glob '*.{ts,tsx}'
rg -n -C 6 'getHostQueryClient|new QueryClient|defaultOptions' \
web/packages/agenta-entities web/oss --glob '*.{ts,tsx}' \
--glob '!**/*.stories.tsx'Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- vault query definition ---'
sed -n '105,130p' web/packages/agenta-entities/src/secret/state/atoms.ts
printf '%s\n' '--- user atom definition and hydration ---'
rg -n -C 10 '(export )?(const|function) userAtom|userAtom\s*=' \
web/packages web/oss/src --glob '*.{ts,tsx}' | head -n 160
printf '%s\n' '--- host query-client definition ---'
rg -n -C 12 'getHostQueryClient' web/packages web/oss/src \
--glob '*.{ts,tsx}' --glob '!**/tests/**' --glob '!**/*.stories.tsx' \
| rg -B 12 -A 12 'getHostQueryClient|QueryClientProvider|queryClientAtom'Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- host query-client files ---'
fd -i 'hostQueryClient' web/packages web/oss
rg -l 'getHostQueryClient' web/packages/agenta-shared web/packages/agenta-entities --glob '*.{ts,tsx}'
printf '%s\n' '--- host query-client implementation ---'
for file in $(rg -l 'getHostQueryClient' web/packages/agenta-shared --glob '*.{ts,tsx}'); do
echo "--- $file"
sed -n '1,240p' "$file"
done
printf '%s\n' '--- app bootstrap writes to shared userAtom ---'
rg -n -C 8 'setUserAtom|set\(userAtom|userAtom' web/oss/src web/packages/agenta-shared \
--glob '*.{ts,tsx}' | head -n 240Repository: Agenta-AI/agenta
Length of output: 26316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- query-client defaults ---'
sed -n '1,240p' web/packages/agenta-shared/src/api/queryClient.ts
printf '%s\n' '--- shared user synchronization ---'
rg -n -C 10 'setUserAtom|setSharedUser|shared.*user|user.*shared' \
web/oss/src web/packages --glob '*.{ts,tsx}' \
| head -n 240
printf '%s\n' '--- project/user bootstrap ordering ---'
rg -n -C 12 'setUserAtom|setProjectIdAtom|projectIdAtom|profileQueryAtom' \
web/oss/src --glob '*.{ts,tsx}' \
| rg -B 12 -A 12 'setUserAtom|setProjectIdAtom|projectIdAtom|profileQueryAtom' \
| head -n 300Repository: Agenta-AI/agenta
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
app = Path("web/packages/agenta-entities/src/workflow/state/appUtils.ts").read_text()
vault = Path("web/packages/agenta-entities/src/secret/state/atoms.ts").read_text()
client = Path("web/packages/agenta-shared/src/api/queryClient.ts").read_text()
listener = Path("web/oss/src/state/profile/UserListener.tsx").read_text()
app_key = re.search(r'queryKey:\s*(\["vault",\s*"secrets",\s*userId,\s*projectId\])', app)
atom_key = re.search(r'queryKey:\s*(\["vault",\s*"secrets",\s*user\?\.id,\s*projectId\])', vault)
assert app_key and atom_key
print("key-shape-match:", app_key.group(1).replace("userId", "user?.id") == atom_key.group(1))
print("atom-enabled:", bool(re.search(r'enabled:\s*!!user\s*&&\s*!!projectId', vault)))
print("direct-call-retry-override:", bool(re.search(
r'ensureQueryData[\s\S]{0,500}?retry\s*:', app
)))
print("host-client-uses-default-query-retry:", bool(
re.search(r'new QueryClient\(\)', client)
and not re.search(r'setDefaultOptions[\s\S]*?retry\s*:', client)
))
print("shared-user-default-null:", bool(re.search(
r'export const userAtom\s*=\s*atom<User \| null>\(null\)',
Path("web/packages/agenta-shared/src/state/user.ts").read_text()
)))
print("shared-user-populated-in-effect:", bool(re.search(
r'useEffect\s*\([\s\S]*?setSharedUser\(user\)',
listener
)))
PYRepository: Agenta-AI/agenta
Length of output: 342
Gate the vault lookup on user hydration and bound its retries.
The cache-key shape matches vaultSecretsQueryAtom. However, the direct call can run before userAtom is populated and create an undefined-user cache entry. Defer the lookup until the user is available. Set retry: false or a small bounded retry because the host client uses TanStack Query defaults, and the create path waits before the catch runs.
| it("speaks the server's own words for a 4xx that carried a message", () => { | ||
| expect(probeFailureMessage(httpError(422, "Stored key is for another provider."))).toBe( | ||
| "Stored key is for another provider.", | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass the required title argument to probeFailureMessage.
probeFailureMessage declares (error: unknown, title: string). This call supplies only the error. TypeScript reports TS2554, so the type check fails.
The runtime assertion still passes, because a 4xx with a message returns before the template string is built. The type error is the blocker.
🐛 Proposed fix
- expect(probeFailureMessage(httpError(422, "Stored key is for another provider."))).toBe(
- "Stored key is for another provider.",
- )
+ expect(
+ probeFailureMessage(
+ httpError(422, "Stored key is for another provider."),
+ "OpenAI",
+ ),
+ ).toBe("Stored key is for another provider.")📝 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.
| it("speaks the server's own words for a 4xx that carried a message", () => { | |
| expect(probeFailureMessage(httpError(422, "Stored key is for another provider."))).toBe( | |
| "Stored key is for another provider.", | |
| ) | |
| }) | |
| it("speaks the server's own words for a 4xx that carried a message", () => { | |
| expect( | |
| probeFailureMessage( | |
| httpError(422, "Stored key is for another provider."), | |
| "OpenAI", | |
| ), | |
| ).toBe("Stored key is for another provider.") | |
| }) |
| {replaceOnly ? ( | ||
| <span className="text-[11px] text-colorTextTertiary"> | ||
| {/* TODO(copy: owner) */} | ||
| {connection?.keyPreview | ||
| ? `Key configured (${connection.keyPreview}). Leave blank to keep it.` | ||
| : "Key configured. Leave blank to keep it."} | ||
| </span> | ||
| ) : null} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show the key preview only next to the field it describes.
connection.keyPreview is one masked value for the whole record. replaceOnly is true for every stored secret field. For a provider with several secret fields (bedrock sends accessKeyId, accessKey, and sessionToken; vertex_ai sends vertexCredentials), the same preview appears under each field, and the text says "Key configured" for fields that are not the key.
Restrict the preview to the field the preview belongs to, and use the field label for the rest.
🐛 Proposed fix
{replaceOnly ? (
<span className="text-[11px] text-colorTextTertiary">
{/* TODO(copy: owner) */}
- {connection?.keyPreview
+ {field.key === "apiKey" && connection?.keyPreview
? `Key configured (${connection.keyPreview}). Leave blank to keep it.`
- : "Key configured. Leave blank to keep it."}
+ : "Saved. Leave blank to keep it."}
</span>
) : null}| <path | ||
| className="fill-[#1E1C1D] dark:fill-[#F2F25C]" | ||
| d="M115.504 95.9335C115.221 98.4384 116.607 99.1695 118.671 98.2233C124.787 95.4184 149.253 82.6572 162.347 82.6572C166.663 82.6572 184.04 84.7181 149.838 117.918C121.062 145.85 113.265 139.835 111.236 137.807C105.889 132.459 108.817 117.798 109.715 110.453C110.039 107.807 109.134 106.985 106.571 108.131C83.5096 118.441 40.4169 140 16.5021 140C-29.3433 140 33.8427 64.9164 43.6743 52.9651C76.3083 13.2951 97.3726 0 109.234 0C130.713 0 121.893 39.2078 115.504 95.9335Z" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace raw hex fill colors with supported theme variables.
Line 22 uses #1E1C1D and #F2F25C. Use supported semantic color variables so the icon follows the shared theme contract.
As per coding guidelines, consume theme colors through Ant Design semantic tokens, Tailwind color utilities, or supported var(--ag-color*) variables; do not use raw hex colors or --ag-c-* literals.
Source: Coding guidelines
Railway Preview Environment
|
4496dcc to
27b0abd
Compare
dosubot
Bot
added
size:XXL
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hosting/docker-compose/oss/docker-compose.gh.local.yml (1)
3-18: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winThis filename matches the operator-local override pattern, but the file is committed.
The coding guidelines state that
hosting/docker-compose/**/docker-compose.*.local.ymlfiles are operator-local and gitignored. This file matches that pattern and contains full service definitions rather than an override. Committing it meansrun.shwill auto-include it for every operator and will also silently merge with any operator's own*.local.yml. Confirm the intended filename, or add the file to the ignore list and move the shared content into a tracked, non-.localcompose file.As per coding guidelines: "Local compose override files are operator-local, gitignored, automatically included by
run.sh, sorted lexicographically, and should be preserved during normal and targeted rebuilds."Source: Coding guidelines
🧹 Nitpick comments (9)
api/oss/src/utils/env.py (1)
578-586: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
"replace-me"placeholder rule is declared in three files._services_internal_key_from_environmentalready maps a blank or"replace-me"value toNone, so both consumers re-check a condition that the sharedenvobject cannot produce. The literal is duplicated, and a change to the placeholder inenv.pywould leave the two copies stale.
api/oss/src/utils/env.py#L578-L586: export the placeholder rule, for example a module-levelUNCONFIGURED_KEY_PLACEHOLDERconstant, so consumers reuse one definition.api/oss/src/apis/fastapi/access/router.py#L101-L127: drop_UNCONFIGURED_KEYand theexpected == _UNCONFIGURED_KEYbranch, and keep only thenot expectedguard.api/ee/src/core/starter_credits_bridge/service.py#L440-L448: drop_UNCONFIGURED_RUNTIME_KEYand reduce_platform_runtime_key_configuredto a truthiness check onenv.agenta.services_internal_key.clients/python/agenta_client/__init__.py (1)
422-423: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueRename the remaining local TypeScript alias to
PublicSecretResponseDto.types.ts,transforms.ts, and the tests still useSecretResponseDto, although the Python generated client and services use the replacement.api/oss/src/dbs/postgres/secrets/dao.py (1)
137-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale comment.
The comment describes "the one-way write_only check below". No
write_onlycheck exists in this method. The lock now protects theresolve_updatecallback, which the comment at Line 154 already explains.♻️ Proposed comment fix
- # FOR UPDATE serializes concurrent updates so the one-way write_only check - # below always sees the latest committed flag — two racing updates cannot - # both observe False and let a stale explicit False win. + # FOR UPDATE serializes concurrent updates so `resolve_update` below always + # reads the latest committed row.api/oss/src/dbs/postgres/secrets/mappings.py (1)
90-101: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAlign the update dump with the create dump.
map_secrets_dto_to_dbeserializes data withmodel_dump(exclude_none=True)at Line 57. The update path callsupdate_secret_dto.secret.data.model_dump()withoutexclude_none. An update therefore persists explicit nulls, for example{"provider": {"key": null}}or"harnesses": null, where a create of the same secret stores no key at all.Two rows with identical logical content then differ on disk depending on the write path. Downstream helpers such as
_value_statusand_carry_over_saved_extrasread these fields withnot in (None, "")checks, so the divergence is easy to reintroduce as a behavior difference later.♻️ Proposed fix
- secrets_dbe.data = _data_payload( - update_secret_dto.secret.data.model_dump(), + secrets_dbe.data = _data_payload( + update_secret_dto.secret.data.model_dump(exclude_none=True), write_only=write_only, management=management, ).gitleaks.toml (1)
45-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo conflicting exemption strategies land in the same change.
This block exempts fixture strings by value and states that fingerprints are deliberately avoided because "a fingerprint names the commit it was seen in, so it goes stale every time a lane below is rebased."
.gitleaksignorelines 295-303 then adds four commit-anchored fingerprints for the same class of fixture and asks maintainers to regenerate them after any history rewrite.Both files describe the same rebase hazard, and one of them accepts it. Pick one strategy for these fixtures. Value-based exemption survives the rebases this stack expects.
web/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsx (1)
167-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce the new explanatory comments to one short line.
web/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsx#L167-L172: replace the multi-line state explanation with one short comment.web/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsx#L134-L139: remove or reduce the multi-line filtering explanation.web/packages/agenta-settings-ui/src/providers/AIProvidersPage.tsx#L76-L79: remove or reduce the multi-line filtering explanation.As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Source: Coding guidelines
api/ee/tests/pytest/unit/test_starter_credits_bridge_seeding.py (1)
103-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClass-level mutable state on
FakeProxyClientcouples tests to fixture ordering.
records,generate_failures, andinstancesare class attributes. Only theseeding_envfixture resets them. Any future test in this module that instantiatesFakeProxyClientwithoutseeding_envinherits state from a previous test. Consider moving the reset into an autouse fixture, or making the registry instance-scoped and passing it through the factory.Also applies to: 184-199
api/oss/tests/pytest/unit/vault/test_write_only_routes.py (1)
36-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the fake DAO store
datain one shape.
createstoresdataas a dict (model_dump(exclude_none=True)), andupdatestores it as the typed DTO object (update_secret_dto.secret.data). The router path then reads two different representations depending on whether a record was created or updated. Aligningupdatewithcreatekeeps the fake faithful to the postgres mapping and prevents a future assertion from passing for the wrong reason.♻️ Proposed alignment
if update_secret_dto.secret is not None: updated.kind = update_secret_dto.secret.kind - updated.data = update_secret_dto.secret.data + updated.data = update_secret_dto.secret.data.model_dump(exclude_none=True)Also applies to: 78-88
hosting/kubernetes/helm/tests/test_runner_secret_absence.py (1)
166-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse one rendered document set.
Line 166 renders the chart with
DEFAULT_TOKEN_ARGS, and line 169 renders the same arguments again. Eachrendercall spawns ahelm templatesubprocess. Render once and derive both results from the same documents.♻️ Single render
- names = runner_container_env_names(render(DEFAULT_TOKEN_ARGS)) + docs = render(DEFAULT_TOKEN_ARGS) + names = runner_container_env_names(docs) failures += check(names) - - docs = render(DEFAULT_TOKEN_ARGS)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 2502a529-0a02-4c7d-bf7a-80d7b6db3294
⛔ Files ignored due to path filters (24)
web/packages/agenta-api-client/src/generated/api/resources/secrets/client/Client.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/secrets/client/requests/CreateSecretDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/secrets/client/requests/ProbeProviderRequest.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/secrets/client/requests/UpdateSecretDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/resources/secrets/client/requests/index.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/CredentialResult.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/CredentialStatus.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/CustomProviderDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/CustomSecretSettingsDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/DiscoveryResult.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/DiscoveryStatus.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/ProbeProviderResponse.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/ProviderCredentials.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/PublicSecretManagementDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/PublicSecretResponseDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SecretDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SecretManagementPolicy.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SecretValueStatus.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/SsoProviderSettingsDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/StandardProviderDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/StandardProviderSettingsDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/UpdateSecretPayloadDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/WebhookProviderSettingsDto.tsis excluded by!**/generated/**web/packages/agenta-api-client/src/generated/api/types/index.tsis excluded by!**/generated/**
📒 Files selected for processing (127)
.gitleaks.toml.gitleaksignoreapi/ee/src/core/organizations/service.pyapi/ee/src/core/starter_credits_bridge/__init__.pyapi/ee/src/core/starter_credits_bridge/client.pyapi/ee/src/core/starter_credits_bridge/service.pyapi/ee/src/core/starter_credits_bridge/types.pyapi/ee/src/main.pyapi/ee/tests/pytest/unit/test_starter_credits_bridge_client.pyapi/ee/tests/pytest/unit/test_starter_credits_bridge_seeding.pyapi/ee/tests/pytest/unit/test_write_only_provider_settings.pyapi/entrypoints/routers.pyapi/oss/src/apis/fastapi/access/router.pyapi/oss/src/apis/fastapi/providers/models.pyapi/oss/src/apis/fastapi/providers/router.pyapi/oss/src/apis/fastapi/vault/router.pyapi/oss/src/core/secrets/dtos.pyapi/oss/src/core/secrets/interfaces.pyapi/oss/src/core/secrets/managed.pyapi/oss/src/core/secrets/redaction.pyapi/oss/src/core/secrets/services.pyapi/oss/src/core/webhooks/service.pyapi/oss/src/core/workflows/service.pyapi/oss/src/dbs/postgres/secrets/dao.pyapi/oss/src/dbs/postgres/secrets/mappings.pyapi/oss/src/middlewares/auth.pyapi/oss/src/utils/env.pyapi/oss/src/utils/helpers.pyapi/oss/tests/pytest/unit/access/test_grant_exchange.pyapi/oss/tests/pytest/unit/middlewares/test_auth_grants.pyapi/oss/tests/pytest/unit/providers/test_provider_probe.pyapi/oss/tests/pytest/unit/secrets/test_dtos.pyapi/oss/tests/pytest/unit/secrets/test_managed_secrets.pyapi/oss/tests/pytest/unit/secrets/test_services.pyapi/oss/tests/pytest/unit/secrets/test_write_only.pyapi/oss/tests/pytest/unit/utils/test_env_helpers.pyapi/oss/tests/pytest/unit/vault/test_managed_routes.pyapi/oss/tests/pytest/unit/vault/test_write_only_routes.pyapi/oss/tests/pytest/unit/webhooks/test_write_only_outward.pyclients/python/agenta_client/__init__.pyclients/python/agenta_client/secrets/client.pyclients/python/agenta_client/secrets/raw_client.pyclients/python/agenta_client/types/__init__.pyclients/python/agenta_client/types/credential_result.pyclients/python/agenta_client/types/credential_status.pyclients/python/agenta_client/types/custom_provider_dto.pyclients/python/agenta_client/types/custom_secret_settings_dto.pyclients/python/agenta_client/types/discovery_result.pyclients/python/agenta_client/types/discovery_status.pyclients/python/agenta_client/types/probe_provider_response.pyclients/python/agenta_client/types/provider_credentials.pyclients/python/agenta_client/types/public_secret_management_dto.pyclients/python/agenta_client/types/public_secret_response_dto.pyclients/python/agenta_client/types/public_secret_response_dto_data.pyclients/python/agenta_client/types/secret_dto.pyclients/python/agenta_client/types/secret_management_policy.pyclients/python/agenta_client/types/secret_value_status.pyclients/python/agenta_client/types/sso_provider_settings_dto.pyclients/python/agenta_client/types/standard_provider_dto.pyclients/python/agenta_client/types/standard_provider_settings_dto.pyclients/python/agenta_client/types/update_secret_payload_dto.pyclients/python/agenta_client/types/update_secret_payload_dto_data.pyclients/python/agenta_client/types/webhook_provider_settings_dto.pydocs/design/managed-secrets/README.mddocs/design/provider-connections-models/provider-discovery.mddocs/design/starter-credits-seeding/README.mddocs/design/write-only-secrets/README.mddocs/design/write-only-secrets/context.mddocs/design/write-only-secrets/implementation-report.mddocs/design/write-only-secrets/plan.mddocs/design/write-only-secrets/qa.mddocs/design/write-only-secrets/research.mddocs/design/write-only-secrets/review.mddocs/design/write-only-secrets/status.mdhosting/docker-compose/ee/docker-compose.dev.ymlhosting/docker-compose/ee/docker-compose.gh.local.ymlhosting/docker-compose/ee/docker-compose.gh.ymlhosting/docker-compose/ee/env.ee.dev.examplehosting/docker-compose/ee/env.ee.gh.examplehosting/docker-compose/oss/docker-compose.dev.ymlhosting/docker-compose/oss/docker-compose.gh.local.ymlhosting/docker-compose/oss/docker-compose.gh.ssl.ymlhosting/docker-compose/oss/docker-compose.gh.ymlhosting/docker-compose/oss/env.oss.dev.examplehosting/docker-compose/oss/env.oss.gh.examplehosting/kubernetes/ee/values.ee.example.yamlhosting/kubernetes/helm/templates/NOTES.txthosting/kubernetes/helm/templates/_helpers.tplhosting/kubernetes/helm/templates/_validations.tplhosting/kubernetes/helm/templates/api-deployment.yamlhosting/kubernetes/helm/templates/secrets.yamlhosting/kubernetes/helm/templates/services-deployment.yamlhosting/kubernetes/helm/tests/test_runner_secret_absence.pyhosting/kubernetes/helm/values.schema.jsonhosting/kubernetes/helm/values.yamlhosting/kubernetes/oss/values.oss.example.yamlhosting/railway/oss/README.mdhosting/railway/oss/scripts/configure.shhosting/railway/oss/template/template.jsonsdks/python/agenta/sdk/agents/connections/__init__.pysdks/python/agenta/sdk/agents/connections/credentials.pysdks/python/agenta/sdk/agents/connections/errors.pysdks/python/agenta/sdk/agents/platform/connections.pysdks/python/agenta/sdk/agents/platform/secrets.pysdks/python/agenta/sdk/middlewares/routing/auth.pysdks/python/agenta/sdk/middlewares/running/vault.pysdks/python/oss/tests/pytest/unit/agents/connections/test_credentials_parity.pysdks/python/oss/tests/pytest/unit/agents/platform/test_write_only_secrets.pysdks/python/oss/tests/pytest/unit/test_auth_middleware_credentials.pyservices/oss/tests/pytest/unit/agent/test_credential_exchange.pyweb/packages/agenta-entities/src/secret/api/probe.tsweb/packages/agenta-entities/src/secret/core/connections.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/workflow/state/agentCreationPrefs.tsweb/packages/agenta-entities/tests/unit/agent-creation-prefs.test.tsweb/packages/agenta-entities/tests/unit/provider-connections.test.tsweb/packages/agenta-entities/tests/unit/secret-transforms.test.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/connectionPicker.tsweb/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsxweb/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsxweb/packages/agenta-entity-ui/tests/unit/connectionPicker.test.tsweb/packages/agenta-settings-ui/src/providers/AIProvidersPage.tsxweb/packages/agenta-shared/src/types/llmProvider.ts
💤 Files with no reviewable changes (1)
- api/ee/src/main.py
🚧 Files skipped from review as they are similar to previous changes (1)
- web/packages/agenta-entity-ui/tests/unit/connectionPicker.test.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| presented = request.headers.get(_RUNTIME_KEY_HEADER) | ||
| if not presented: | ||
| return False | ||
|
|
||
| expected = env.agenta.services_internal_key | ||
| # A deployment that configured nothing keeps the well-known placeholder, which anyone | ||
| # could send. Treat it as "no runtime configured" rather than as a secret: such a | ||
| # deployment issues no grant at all, which costs it only the ability to run against | ||
| # write-only secrets — off by default — and never hands the ability to a stranger. | ||
| if not expected or expected == _UNCONFIGURED_KEY: | ||
| return False | ||
|
|
||
| return compare_digest(presented, expected) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm compare_digest rejects non-ASCII str operands on the project's target Python.
set -euo pipefail
# Resolve the declared runtime for the API package.
fd -H -t f 'pyproject.toml|\.python-version|\.tool-versions' api | head -20 | while IFS= read -r f; do
echo "=== $f ==="
rg -n 'requires-python|python|target-version' "$f" || true
done
fd -H -t f 'Dockerfile*' api --exec rg -n 'FROM .*python' {} \;
python - <<'PY'
import sys, hmac
print("sandbox python:", sys.version)
try:
hmac.compare_digest("\u00ff", "abc")
print("no TypeError")
except TypeError as exc:
print("TypeError:", exc)
PYRepository: Agenta-AI/agenta
Length of output: 842
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== access/router.py:90-230 ==="
sed -n '90,230p' api/oss/src/apis/fastapi/access/router.py
echo "=== relevant references ==="
rg -n -C 4 '_is_platform_runtime|check_permissions|RUNTIME_KEY_HEADER|compare_digest' api/oss/src/apis/fastapi/access/router.py
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("api/oss/src/apis/fastapi/access/router.py")
tree = ast.parse(path.read_text())
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name in {
"_is_platform_runtime",
"check_permissions",
}:
print(f"{node.name}: lines {node.lineno}-{node.end_lineno}")
for child in ast.walk(node):
if isinstance(child, ast.Try):
print(
f" try block: lines {child.lineno}-{child.end_lineno}; "
f"handlers={len(child.handlers)}"
)
if isinstance(child, ast.Call):
func = child.func
name = (
func.id
if isinstance(func, ast.Name)
else func.attr
if isinstance(func, ast.Attribute)
else None
)
if name in {"_is_platform_runtime", "compare_digest"}:
print(f" call {name}: line {child.lineno}")
PYRepository: Agenta-AI/agenta
Length of output: 11456
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== access/router.py:1-90 and 176-225 ==="
sed -n '1,90p' api/oss/src/apis/fastapi/access/router.py
sed -n '176,225p' api/oss/src/apis/fastapi/access/router.py
echo "=== route and exception wiring ==="
rg -n -C 3 'AccessRouter|check_permissions|intercept_exceptions|exception_handler|HTTPException' api/oss/src/apis/fastapi
echo "=== declared Starlette/FastAPI versions ==="
rg -n -C 2 'starlette|fastapi' api/pyproject.toml api/poetry.lock api/uv.lock api/requirements*.txt 2>/dev/null || true
python3 - <<'PY'
import ast
import hmac
from pathlib import Path
source = Path("api/oss/src/apis/fastapi/access/router.py").read_text()
tree = ast.parse(source)
functions = {
node.name: node
for node in ast.walk(tree)
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
}
runtime = functions["_is_platform_runtime"]
check = functions["check_permissions"]
runtime_compare_line = next(
node.lineno
for node in ast.walk(runtime)
if isinstance(node, ast.Call)
and isinstance(node.func, ast.Name)
and node.func.id == "compare_digest"
)
runtime_call_line = next(
node.lineno
for node in ast.walk(check)
if isinstance(node, ast.Call)
and isinstance(node.func, ast.Name)
and node.func.id == "_run_credential_grants"
)
try_lines = [
(node.lineno, node.end_lineno)
for node in ast.walk(check)
if isinstance(node, ast.Try)
]
print("compare_digest line:", runtime_compare_line)
print("_run_credential_grants call line:", runtime_call_line)
print("try blocks:", try_lines)
for value in ("\u00ff", "\u0080", "runtime-key"):
try:
result = hmac.compare_digest(value, "runtime-key")
print(repr(value), "->", result)
except TypeError as exc:
print(repr(value), "-> TypeError:", str(exc))
PYRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("api/oss/src/apis/fastapi/access/router.py")
tree = ast.parse(path.read_text())
for node in tree.body:
if isinstance(node, ast.ClassDef) and node.name == "AccessRouter":
for method in node.body:
if isinstance(method, (ast.FunctionDef, ast.AsyncFunctionDef)) and method.name in {
"_is_platform_runtime",
"_run_credential_grants",
"check_permissions",
}:
decorators = [
ast.unparse(decorator) for decorator in method.decorator_list
]
print(
f"{method.name}: lines {method.lineno}-{method.end_lineno}; "
f"decorators={decorators}"
)
for child in ast.walk(method):
if isinstance(child, ast.Call):
if isinstance(child.func, ast.Name) and child.func.id in {
"_run_credential_grants",
"compare_digest",
}:
print(
f" {child.func.id} call at line {child.lineno}"
)
if isinstance(child, ast.Try):
print(
f" try block at lines {child.lineno}-{child.end_lineno}; "
f"handlers={len(child.handlers)}"
)
PY
echo "=== focused exception decorator definitions ==="
rg -n -A 25 -B 5 '^def intercept_exceptions|^async def intercept_exceptions|^def suppress_exceptions|^async def suppress_exceptions' \
api/oss/src/utils api/oss/src/apis/fastapi/accessRepository: Agenta-AI/agenta
Length of output: 4432
Handle non-ASCII runtime keys before compare_digest.
A non-ASCII header value causes compare_digest to raise TypeError. _run_credential_grants calls this before check_permissions enters its try block, so the request returns 500 instead of being denied. Compare UTF-8 byte strings.
| if kind is not None and kind != stored_kind and typed.key is None: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, | ||
| detail=( | ||
| "Testing this connection as a different provider requires its " | ||
| "credential; the stored one belongs to the saved provider." | ||
| ), | ||
| ) | ||
|
|
||
| merged = ProviderCredentials( | ||
| key=_typed_or_stored(typed.key, stored_key), | ||
| url=_typed_or_stored(typed.url, getattr(settings, "url", None)), | ||
| version=_typed_or_stored(typed.version, getattr(settings, "version", None)), | ||
| extras=_merged_extras(typed.extras, stored_extras), | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm the declared Pydantic version and inspect probe request parsing plus regression coverage.
fd -a -t f 'pyproject.toml' . -x rg -n 'pydantic' {}
ast-grep outline api/oss/src/apis/fastapi/providers/models.py --items all
rg -n -C 5 'secret_id|SecretStr|ProviderCredentials|kind' \
api/oss/src/apis/fastapi/providers/models.py \
api/oss/tests/pytest/unit/providers/test_provider_probe.pyRepository: Agenta-AI/agenta
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
fd -a -t f . api/oss/src/apis/fastapi/providers api/oss/tests/pytest/unit/providers 2>/dev/null || true
printf '%s\n' '--- router helpers and probe flow ---'
rg -n -C 12 '_typed_or_stored|_stored_credential|stored_kind|ProviderCredentials|EDIT_SECRET|HTTPException|def .*probe|add_api_route' \
api/oss/src/apis/fastapi/providers/router.py
printf '%s\n' '--- credential model definitions ---'
rg -n -C 8 'SecretStr|class ProviderCredentials|key:' api/oss/src/apis/fastapi api/oss/tests 2>/dev/null || true
printf '%s\n' '--- provider probe tests ---'
rg -n -C 10 'probe|different|kind|key|SecretStr|EDIT_SECRET' \
api/oss/tests api/oss/src 2>/dev/null | head -n 500Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- provider API models ---'
cat -n api/oss/src/apis/fastapi/providers/models.py
printf '%s\n' '--- ProviderCredentials definition ---'
rg -n -l 'class ProviderCredentials|ProviderCredentials\s*=' api/oss/src api/oss/tests | head -n 20
rg -n -C 12 'class ProviderCredentials|ProviderCredentials\s*=' api/oss/src/core api/oss/src/apis/fastapi/providers api/oss/tests/pytest/unit/providers 2>/dev/null | head -n 250
printf '%s\n' '--- probe test structure and relevant cases ---'
rg -n '^(async )?def test_|^class Test|_merge_stored_secret|stored_provider|secret_id|different|empty|blank|SecretStr|key=' \
api/oss/tests/pytest/unit/providers/test_provider_probe.py | head -n 300
printf '%s\n' '--- focused test sections ---'
sed -n '850,1080p' api/oss/tests/pytest/unit/providers/test_provider_probe.pyRepository: Agenta-AI/agenta
Length of output: 24274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
router = Path("api/oss/src/apis/fastapi/providers/router.py").read_text()
assert "if kind is not None and kind != stored_kind and typed.key is None:" in router
assert "key=_typed_or_stored(typed.key, stored_key)" in router
try:
from pydantic import SecretStr
except Exception as exc:
print(f"pydantic probe unavailable: {exc}")
else:
def typed_or_stored(typed, stored):
if typed is None:
return stored
value = typed.get_secret_value() if isinstance(typed, SecretStr) else typed
if value in ("", {}, []):
return stored
return typed
stored = SecretStr("stored-write-only-key")
blank = SecretStr("")
supplied = SecretStr("caller-key")
print("SecretStr('') value:", repr(blank.get_secret_value()))
print("blank fallback result:", typed_or_stored(blank, stored))
print("blank mismatch guard (current):", blank is None)
print("blank mismatch guard (proposed):", typed_or_stored(blank, None) is None)
print("supplied mismatch guard (proposed):", typed_or_stored(supplied, None) is None)
assert typed_or_stored(blank, stored) is stored
assert blank is not None
assert typed_or_stored(blank, None) is None
print("No existing provider-probe test combines a different kind with an empty key:")
tests = Path("api/oss/tests/pytest/unit/providers/test_provider_probe.py").read_text()
for line_no, line in enumerate(tests.splitlines(), 1):
if '"kind": "anthropic"' in line or '"key": ""' in line:
print(f"{line_no}: {line}")
PY
printf '%s\n' '--- exact mismatch and blank-field tests ---'
sed -n '120,145p' api/oss/src/apis/fastapi/providers/router.py
sed -n '895,970p' api/oss/tests/pytest/unit/providers/test_provider_probe.pyRepository: Agenta-AI/agenta
Length of output: 4293
Reject blank override keys before using a stored credential.
When kind differs from stored_kind, SecretStr("") bypasses the typed.key is None guard. _typed_or_stored() then substitutes stored_key, allowing an EDIT_SECRET caller to send the stored credential to another provider.
Normalize typed.key before the mismatch check and add a regression test for a different kind with provider.key="".
| if project_id is not None: | ||
| secrets_dtos = await get_cache( | ||
| namespace="list_secrets", | ||
| project_id=str(project_id), | ||
| key={}, | ||
| model=SecretResponseDTO, | ||
| is_list=True, | ||
| ) | ||
| if secrets_dtos is not None: | ||
| return secrets_dtos | ||
|
|
||
| with set_data_encryption_key(data_encryption_key=self._data_encryption_key): | ||
| secrets_dtos = await self.secrets_dao.list( | ||
| project_id=project_id, | ||
| organization_id=organization_id, | ||
| project_id=project_id, organization_id=organization_id | ||
| ) | ||
|
|
||
| if project_id is not None: | ||
| await set_cache( | ||
| namespace="list_secrets", | ||
| project_id=str(project_id), | ||
| key={}, | ||
| value=secrets_dtos, | ||
| ) | ||
| return secrets_dtos | ||
| return secrets_dtos |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the cache implementation: backend, serialization, and TTL.
fd -t f 'caching.py' api --exec cat -n {}Repository: Agenta-AI/agenta
Length of output: 19597
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- services.py ---'
sed -n '360,470p' api/oss/src/core/secrets/services.py
printf '%s\n' '--- secret DTOs and models ---'
rg -n -C 5 'class Secret(Response)?DTO|SecretResponseDTO|provider_key|client_secret|custom_secret|decrypt|set_data_encryption_key' api/oss/src/core/secrets api/oss/src -g '*.py'
printf '%s\n' '--- cache engine/configuration ---'
rg -n -C 4 'def get_cache_engine|get_cache_engine|Redis|redis|caching.enabled|CACHE_URL|REDIS' api/oss/src -g '*.py'Repository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- secret files ---'
fd -t f . api/oss/src/core/secrets | sort
printf '%s\n' '--- DTO definitions ---'
rg -n -C 12 '^class (Secret|SecretResponse)DTO|SecretResponseDTO' api/oss/src/core/secrets -g '*.py'
printf '%s\n' '--- DAO list implementation ---'
rg -n -C 20 'async def list\(|def list\(' api/oss/src/core/secrets -g '*.py'
printf '%s\n' '--- cache engine implementation ---'
fd -t f . api/oss/src/dbs/redis | sort
rg -n -C 12 'def get_cache_engine|Redis\(|redis_url|REDIS|CACHE' api/oss/src/dbs/redis api/oss/src/utils/env* -g '*.py'Repository: Agenta-AI/agenta
Length of output: 44154
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- secret DTO definitions ---'
rg -n -C 12 '^class (Secret|SecretResponse)DTO|SecretResponseDTO' api/oss/src/core/secrets -g '*.py'
printf '%s\n' '--- DAO list implementation ---'
rg -n -C 20 'async def list\(|def list\(' api/oss/src/core/secrets -g '*.py'
printf '%s\n' '--- cache engine configuration ---'
rg -n -C 12 'def get_cache_engine|Redis\(|redis_url|REDIS|CACHE' api/oss/src/dbs/redis api/oss/src/utils/env* -g '*.py'Repository: Agenta-AI/agenta
Length of output: 43610
Do not cache decrypted secret values.
SecretResponseDTO explicitly retains credential material, and the cache serializes it to Redis with a five-minute TTL. Cache a credential-free projection or encrypt the cached payload before writing it.
| def test_every_secret_kind_the_classifier_knows_has_a_credential_location(): | ||
| # The probe asks the classifier where a kind keeps its credential. If a kind is ever | ||
| # added to the vault without an entry there, this probe would silently send nothing. | ||
| from oss.src.core.secrets.redaction import PRIMARY_CREDENTIAL_FIELDS | ||
|
|
||
| assert PRIMARY_CREDENTIAL_FIELDS["provider_key"] == ("provider", "key") | ||
| assert PRIMARY_CREDENTIAL_FIELDS["custom_provider"] == ("provider", "key") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The test does not check what its name and comment claim.
The name and the comment state that every secret kind the classifier knows has a credential location. The body asserts two hardcoded keys. If a new kind is added to the vault without an entry in PRIMARY_CREDENTIAL_FIELDS, this test still passes, and the probe silently sends no credential. Iterate over the kind enum instead.
💚 Proposed exhaustive check
def test_every_secret_kind_the_classifier_knows_has_a_credential_location():
# The probe asks the classifier where a kind keeps its credential. If a kind is ever
# added to the vault without an entry there, this probe would silently send nothing.
from oss.src.core.secrets.redaction import PRIMARY_CREDENTIAL_FIELDS
+ from oss.src.core.secrets.enums import SecretKind
assert PRIMARY_CREDENTIAL_FIELDS["provider_key"] == ("provider", "key")
assert PRIMARY_CREDENTIAL_FIELDS["custom_provider"] == ("provider", "key")
+ missing = [kind.value for kind in SecretKind if kind.value not in PRIMARY_CREDENTIAL_FIELDS]
+ assert missing == [], f"kinds without a credential location: {missing}"Confirm the enum name and that every member is expected to carry a credential location.
#!/bin/bash
fd -t f 'enums.py' -p 'core/secrets' | xargs -r cat -n
rg -n -C5 'PRIMARY_CREDENTIAL_FIELDS' --type=py| `key` is a `SecretStr` and `extras` is kept out of `repr`, so an accidental log line | ||
| or traceback that carries this object cannot print the credential. Unwrap the key with | ||
| `.get_secret_value()` at the point it is put on the wire, never earlier. | ||
| """ | ||
|
|
||
| key: typing.Optional[str] = None | ||
| url: typing.Optional[str] = None | ||
| version: typing.Optional[str] = None | ||
| extras: typing.Optional[typing.Dict[str, typing.Any]] = None |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n clients/python/agenta_client/types/provider_credentials.py
printf '%s\n' '--- nearby Python project files ---'
fd -t f -e py -e toml -e cfg -e ini . clients/python | head -80
printf '%s\n' '--- ProviderCredentials references ---'
rg -n -C 3 'ProviderCredentials|provider_credentials|SecretStr|model_repr|repr' clients/pythonRepository: Agenta-AI/agenta
Length of output: 24140
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- pydantic utilities ---'
cat -n clients/python/agenta_client/core/pydantic_utilities.py
printf '%s\n' '--- serialization helpers ---'
rg -n -C 6 'def convert_and_respect_annotation_metadata|class UniversalBaseModel|def dict|def model_dump|__repr__|repr=False|SecretStr' clients/python/agenta_client/core clients/python/agenta_client/types
printf '%s\n' '--- dependency declarations ---'
rg -n -C 3 'pydantic' clients/python pyproject.toml setup.py setup.cfg requirements*.txt 2>/dev/null || true
printf '%s\n' '--- standalone Pydantic probe ---'
python3 - <<'PY'
try:
import pydantic
from typing import Any, Dict, Optional
from pydantic import SecretStr
print("pydantic_version=", pydantic.__version__)
class Base(pydantic.BaseModel):
key: Optional[str] = None
extras: Optional[Dict[str, Any]] = None
if hasattr(pydantic, "ConfigDict"):
model_config = pydantic.ConfigDict(extra="allow", frozen=True)
else:
class Config:
extra = "allow"
frozen = True
plain = Base(key="API-KEY", extras={"token": "TOKEN"})
secret = Base(key=SecretStr("API-KEY"), extras={"token": SecretStr("TOKEN")})
extra = Base(key=None, extras=None, hidden="HIDDEN")
print("plain_repr=", repr(plain))
print("plain_str=", str(plain))
print("secret_repr=", repr(secret))
print("secret_str=", str(secret))
print("extra_repr=", repr(extra))
if hasattr(plain, "model_dump"):
print("plain_dump=", plain.model_dump())
else:
print("plain_dict=", plain.dict())
except Exception as exc:
print(type(exc).__name__ + ":", exc)
PYRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- UniversalBaseModel and version detection ---'
rg -n -A 45 -B 8 'IS_PYDANTIC_V2|UniversalBaseModel' clients/python/agenta_client/core/pydantic_utilities.py
printf '%s\n' '--- conversion helper ---'
rg -n -A 80 -B 10 'def convert_and_respect_annotation_metadata' clients/python/agenta_client/core
printf '%s\n' '--- dependency declarations ---'
rg -n -A 5 -B 3 'pydantic' clients/python/pyproject.toml clients/python/setup.py clients/python/setup.cfg clients/python/requirements.txt pyproject.toml setup.py setup.cfg 2>/dev/null || true
printf '%s\n' '--- standalone Pydantic probe ---'
python3 - <<'PY'
from typing import Any, Dict, Optional
import pydantic
from pydantic import SecretStr
print("pydantic_version=", pydantic.__version__)
class Base(pydantic.BaseModel):
key: Optional[str] = None
extras: Optional[Dict[str, Any]] = None
if hasattr(pydantic, "ConfigDict"):
model_config = pydantic.ConfigDict(extra="allow", frozen=True)
else:
class Config:
extra = "allow"
frozen = True
for label, value in {
"plain": Base(key="API-KEY", extras={"token": "TOKEN"}),
"secret": Base(key=SecretStr("API-KEY"), extras={"token": SecretStr("TOKEN")}),
"extra": Base(hidden="HIDDEN"),
}.items():
text = repr(value)
print(label + "_repr=", text)
print(label + "_contains_key=", "API-KEY" in text)
print(label + "_contains_token=", "TOKEN" in text)
print(label + "_contains_hidden=", "HIDDEN" in text)
PYRepository: Agenta-AI/agenta
Length of output: 22496
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- remaining model serialization code ---'
sed -n '120,285p' clients/python/agenta_client/core/pydantic_utilities.py
sed -n '110,230p' clients/python/agenta_client/core/serialization.py
printf '%s\n' '--- request encoding path ---'
rg -n -A 60 -B 15 'jsonable_encoder|convert_and_respect_annotation_metadata|httpx.Request|json=' clients/python/agenta_client/core clients/python/agenta_client/secrets/raw_client.py | head -500
printf '%s\n' '--- local Pydantic sources, if present ---'
find /usr /opt -type f -path '*/pydantic/*' 2>/dev/null | head -20 || trueRepository: Agenta-AI/agenta
Length of output: 50372
🌐 Web query:
Pydantic v2 BaseModel repr extra allow SecretStr serialization model_dump_json official documentation
💡 Result:
In Pydantic v2, SecretStr (and SecretBytes) types are designed to prevent accidental leakage of sensitive information in logs, tracebacks, and standard serialization methods [1][2]. By default, when you use model_dump_json, these fields are serialized as the string '' [1][2]. There is no native flag or configuration in Pydantic v2 to simply "allow" or "toggle" the serialization of SecretStr to plain text within model_dump_json [3][4]. The Pydantic team maintains this as a safety feature and does not plan to change the default behavior [4]. To serialize a SecretStr as plain text in the JSON output, you must explicitly define a custom serializer using the @field_serializer decorator [1][2]. Example of custom serialization: from pydantic import BaseModel, SecretStr, field_serializer class MyModel(BaseModel): password: SecretStr @field_serializer('password', when_used='json') def dump_password(self, v: SecretStr) -> str: return v.get_secret_value model = MyModel(password='super-secret') print(model.model_dump_json) # Output: {"password":"super-secret"} Regarding the other aspects mentioned: - model_dump_json and model_dump support standard arguments like include, exclude, exclude_unset, and serialize_as_any to control overall model serialization [5][6][7]. - Model extra fields are managed via the model configuration (e.g., model_config = ConfigDict(extra='allow')), where extra data is stored in the pydantic_extra attribute [8][5]. These settings are independent of the SecretStr serialization behavior.
Citations:
- 1: https://pydantic.dev/docs/validation/2.3/usage/types/secrets/
- 2: https://pydantic.dev/docs/validation/2.6/examples/secrets/
- 3: Round-tripping of secret types pydantic/pydantic-core#851
- 4: fix(secrets): keep the same type of secret field on json serialization pydantic/pydantic#9819
- 5: https://pydantic.dev/docs/validation/2.9/api/pydantic/base_model/
- 6: https://pydantic.dev/docs/validation/latest/concepts/serialization/
- 7: https://pydantic.dev/docs/validation/2.0/api/pydantic/main/
- 8: https://pydantic.dev/docs/validation/2.10/concepts/models/
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- JSON encoder ---'
cat -n clients/python/agenta_client/core/jsonable_encoder.py
printf '%s\n' '--- HTTP client request encoding ---'
rg -n -A 80 -B 20 'def request|jsonable_encoder|json=|request_options' clients/python/agenta_client/core/http_client.py clients/python/agenta_client/core/client_wrapper.pyRepository: Agenta-AI/agenta
Length of output: 50372
Redact credentials from ProviderCredentials representations.
ProviderCredentials currently exposes key, extras, and allowed top-level extras through Pydantic's default repr, which can leak credentials in logs or tracebacks. Use SecretStr and explicit repr=False handling, and add a wire serializer that calls .get_secret_value() so requests still contain the real credential.
| # Proves to the API that a caller IS the platform runtime (the workflow service), so the | ||
| # credential it receives may read write-only secret values. The API and the services | ||
| # container must hold the SAME dedicated value, and a browser, runner, worker, cron, or | ||
| # sandbox must never receive it. There is no AGENTA_AUTH_KEY fallback. Missing and | ||
| # placeholder values prevent write-only secret grants from being issued. | ||
| AGENTA_SERVICES_INTERNAL_KEY=replace-me |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Every env example ships the exact placeholder that stops API startup. validate_platform_runtime_key in api/oss/src/utils/helpers.py raises RuntimeError when AGENTA_SERVICES_INTERNAL_KEY is empty or equals replace-me. All three examples set replace-me, and all three comments say only that write-only grants are withheld, which understates the failure.
hosting/docker-compose/oss/env.oss.gh.example#L32-L37: set a non-placeholder default and state in the comment that the API fails startup on a missing or placeholder value.hosting/docker-compose/ee/env.ee.gh.example#L32-L37: apply the same default and comment correction.hosting/docker-compose/ee/env.ee.dev.example#L28-L33: apply the same default and comment correction, and add the key-generation command next to the variable.
📍 Affects 3 files
hosting/docker-compose/oss/env.oss.gh.example#L32-L37(this comment)hosting/docker-compose/ee/env.ee.gh.example#L32-L37hosting/docker-compose/ee/env.ee.dev.example#L28-L33
| for channel in _credential_channels(provider, candidate): | ||
| values = {name: (os.environ.get(name) or "").strip() for name in channel} | ||
| if all(values.values()): | ||
| return values |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="sdks/python/agenta/sdk/agents/platform/connections.py"
printf '%s\n' '--- target file outline ---'
ast-grep outline "$file" --lang python || true
printf '%s\n' '--- target implementation ---'
sed -n '1,280p' "$file"
printf '%s\n' '--- related tests and symbol uses ---'
rg -n -C 4 \
'AWS_SESSION_TOKEN|AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY|_credential_channels|ResolvedConnection|write.?only|redacted' \
sdks/pythonRepository: Agenta-AI/agenta
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("sdks/python/agenta/sdk/agents/platform/connections.py")
text = path.read_text()
for needle in (
"AWS_ACCESS_KEY_ID",
"AWS_SECRET_ACCESS_KEY",
"AWS_SESSION_TOKEN",
"_environment_credential",
"_credential_channels",
):
print(f"\n--- occurrences of {needle} ---")
for number, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f"{number}: {line}")
PY
printf '%s\n' '--- focused tests ---'
rg -l \
'AWS_SESSION_TOKEN|AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY|_environment_credential|_credential_channels|ResolvedConnection' \
sdks/python --glob '*test*.py' --glob '*tests*.py' | sort | while read -r file; do
echo "### $file"
rg -n -C 6 \
'AWS_SESSION_TOKEN|AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY|_environment_credential|_credential_channels|ResolvedConnection' \
"$file"
doneRepository: Agenta-AI/agenta
Length of output: 21712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- fallback call site ---'
sed -n '610,690p' sdks/python/agenta/sdk/agents/platform/connections.py
printf '%s\n' '--- credential construction ---'
sed -n '1,180p' sdks/python/agenta/sdk/agents/connections/endpoints.py
sed -n '1,90p' sdks/python/agenta/sdk/agents/connections/credentials.py
printf '%s\n' '--- write-only tests ---'
sed -n '1,270p' sdks/python/oss/tests/pytest/unit/agents/platform/test_write_only_secrets.py
printf '%s\n' '--- HTTP AWS test ---'
sed -n '350,455p' sdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.pyRepository: Agenta-AI/agenta
Length of output: 25937
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("sdks/python/agenta/sdk/agents/platform/connections.py")
tree = ast.parse(path.read_text())
function = next(
node for node in tree.body
if isinstance(node, ast.FunctionDef) and node.name == "_environment_credential"
)
source = ast.get_source_segment(path.read_text(), function)
print("--- _environment_credential source ---")
print(source)
print("--- direct return path after fallback ---")
lines = path.read_text().splitlines()
for number in range(662, 704):
print(f"{number}: {lines[number - 1]}")
# Standalone simulation of the exact loop semantics for the AWS channel.
def current_fallback(environ):
channel = ("AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY")
values = {name: (environ.get(name) or "").strip() for name in channel}
return values if all(values.values()) else None
cases = [
{
"AWS_ACCESS_KEY_ID": "AKIAEXAMPLE",
"AWS_SECRET_ACCESS_KEY": "secret",
"AWS_SESSION_TOKEN": "token",
},
{
"AWS_ACCESS_KEY_ID": "AKIAEXAMPLE",
"AWS_SECRET_ACCESS_KEY": "secret",
},
]
for environ in cases:
result = current_fallback(environ)
print("input:", sorted(environ))
print("returned:", sorted(result or {}))
assert result is not None
assert "AWS_SESSION_TOKEN" not in result
print("The current fallback returns only the two channel variables, even when AWS_SESSION_TOKEN is set.")
PY
printf '%s\n' '--- all AWS_SESSION_TOKEN cleanup references in write-only tests ---'
rg -n -C 2 'AWS_SESSION_TOKEN' sdks/python/oss/tests/pytest/unit/agents/platform/test_write_only_secrets.py || trueRepository: Agenta-AI/agenta
Length of output: 3383
Preserve AWS_SESSION_TOKEN with AWS fallback credentials.
When temporary AWS credentials provide all three variables, include AWS_SESSION_TOKEN in ResolvedConnection.credentials. Add a test that asserts all three values reach resolved.credentials.
| if ( | ||
| isinstance(secret, dict) | ||
| and secret.get("write_only") | ||
| and secret.get("has_key") | ||
| ): | ||
| data = secret.get("data") or {} | ||
| kind = secret.get("kind") | ||
| value = None | ||
| if kind in ("provider_key", "custom_provider"): | ||
| provider = data.get("provider") or {} | ||
| value = provider.get("key") or credential_extras( | ||
| provider.get("extras") or {} | ||
| ) | ||
| elif kind == "custom_secret": | ||
| value = (data.get("secret") or {}).get("content") | ||
|
|
||
| if not value: | ||
| header = secret.get("header") or {} | ||
| redacted_names.append(header.get("name") or secret.get("slug") or kind) | ||
| continue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Drop unconfigured write-only provider secrets.
Line 440 only processes entries when has_key is true. A write-only provider_key with has_key=False remains in vault_secrets. Lines 390-408 then classify it as a stored provider and remove a valid environment credential for the same provider family. The run receives no usable credential.
Process supported write-only provider entries regardless of has_key. Drop the entry when key extraction finds no credential. Add a regression test with write_only=True, has_key=False, and a matching local provider key.
Proposed fix
- if (
- isinstance(secret, dict)
- and secret.get("write_only")
- and secret.get("has_key")
- ):
+ if isinstance(secret, dict) and secret.get("write_only"):
data = secret.get("data") or {}
kind = secret.get("kind")
value = None
if kind in ("provider_key", "custom_provider"):
provider = data.get("provider") or {}
value = provider.get("key") or credential_extras(
provider.get("extras") or {}
)
+ if not value:
+ header = secret.get("header") or {}
+ redacted_names.append(
+ header.get("name") or secret.get("slug") or kind
+ )
+ continue
elif kind == "custom_secret":
value = (data.get("secret") or {}).get("content")
-
- if not value:
+ if not value:
header = secret.get("header") or {}
redacted_names.append(header.get("name") or secret.get("slug") or kind)
continue📝 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.
| if ( | |
| isinstance(secret, dict) | |
| and secret.get("write_only") | |
| and secret.get("has_key") | |
| ): | |
| data = secret.get("data") or {} | |
| kind = secret.get("kind") | |
| value = None | |
| if kind in ("provider_key", "custom_provider"): | |
| provider = data.get("provider") or {} | |
| value = provider.get("key") or credential_extras( | |
| provider.get("extras") or {} | |
| ) | |
| elif kind == "custom_secret": | |
| value = (data.get("secret") or {}).get("content") | |
| if not value: | |
| header = secret.get("header") or {} | |
| redacted_names.append(header.get("name") or secret.get("slug") or kind) | |
| continue | |
| if isinstance(secret, dict) and secret.get("write_only"): | |
| data = secret.get("data") or {} | |
| kind = secret.get("kind") | |
| value = None | |
| if kind in ("provider_key", "custom_provider"): | |
| provider = data.get("provider") or {} | |
| value = provider.get("key") or credential_extras( | |
| provider.get("extras") or {} | |
| ) | |
| if not value: | |
| header = secret.get("header") or {} | |
| redacted_names.append( | |
| header.get("name") or secret.get("slug") or kind | |
| ) | |
| continue | |
| elif kind == "custom_secret": | |
| value = (data.get("secret") or {}).get("content") | |
| if not value: | |
| header = secret.get("header") or {} | |
| redacted_names.append(header.get("name") or secret.get("slug") or kind) | |
| continue |
| /** The HTTP status of a failed request, when it carried one. */ | ||
| const statusOf = (error: unknown): number | null => { | ||
| const response = (error as {response?: {status?: unknown}})?.response | ||
| return typeof response?.status === "number" ? response.status : null | ||
| } | ||
|
|
||
| /** | ||
| * Why a Test never produced a verdict. | ||
| * | ||
| * A probe OUTCOME is an HTTP 200 with a status inside, so anything that throws here is the request | ||
| * itself failing. The default reads as "we could not reach the provider", which is true of a | ||
| * transport failure and false of everything the API rejects on its own — a 404 means the stored | ||
| * connection is gone, not that the provider is down. So a 4xx speaks with the server's own words | ||
| * where it gave any, and only a 5xx or a dead connection falls back to the reach-the-provider line. | ||
| */ | ||
| // TODO(copy: owner) | ||
| export const probeFailureMessage = (error: unknown, title: string): string => { | ||
| const status = statusOf(error) | ||
| if (status === 404) return "This connection no longer exists. Reload and try again." | ||
| if (status && status >= 400 && status < 500) { | ||
| const message = extractApiErrorMessage(error) | ||
| if (message && message !== String(error)) return message | ||
| } | ||
| return `Agenta could not reach ${title} to test this credential.` | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the generated API error class and its status field.
fd -i -t f 'errors' web/packages -x rg -n -C4 'statusCode|class .*Error' {} \; | head -80
# Find the shared error-message extractor.
rg -nP -C10 'export const extractApiErrorMessage|export function extractApiErrorMessage' web/packages
# Find the secrets client accessor used by the probe.
rg -nP -C6 'getSecretsClient|projectScopedRequest' web/packages/agenta-entities/src/secret/apiRepository: Agenta-AI/agenta
Length of output: 12565
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant error and extractor definitions ---'
rg -n -C8 'class .*ApiError|statusCode|body\??:|extractApiErrorMessage' web/packages web 2>/dev/null | head -240
printf '%s\n' '--- package manifests and SDK/generated-client references ---'
rg -n -C4 '"`@agentaai/api-client`"|api-client|Fern|ApiError' web/packages/agenta-entities web/packages/agenta-sdk web/package.json web/pnpm-lock.yaml 2>/dev/null | head -240
printf '%s\n' '--- target implementation and tests ---'
cat -n web/packages/agenta-entities/src/secret/core/connections.ts | sed -n '210,270p'
fd -i -t f 'probe' web/packages/agenta-entities | xargs -r -n1 sh -c 'echo "--- $0"; cat -n "$0" | sed -n "1,220p"'
printf '%s\n' '--- extractor implementation ---'
cat -n web/packages/agenta-shared/src/utils/extractApiErrorMessage.ts | sed -n '1,130p'Repository: Agenta-AI/agenta
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- generated client files ---'
git ls-files 'web/packages/agenta-api-client/**' | head -160
printf '%s\n' '--- generated error definitions and exports ---'
rg -n -C10 'AgentaApiError|ApiError|statusCode|body' web/packages/agenta-api-client --glob '*.ts' | head -260
printf '%s\n' '--- extractor implementation ---'
cat -n web/packages/agenta-shared/src/utils/extractApiErrorMessage.ts | sed -n '1,130p'
printf '%s\n' '--- probe tests and failure-message references ---'
rg -n -C8 'probeFailureMessage|response: \{status|statusCode|extractApiErrorMessage' web/packages/agenta-entities --glob '*.{ts,tsx}'Repository: Agenta-AI/agenta
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- generated error class ---'
cat -n web/packages/agenta-api-client/src/generated/errors/AgentaApiError.ts
printf '%s\n' '--- generated response/error path ---'
rg -n -C12 'handleNonStatusCodeError|statusCode: response.status|new errors\.AgentaApiError|response\.ok|statusCode' \
web/packages/agenta-api-client/src/generated/core \
web/packages/agenta-api-client/src/generated/errors \
web/packages/agenta-api-client/src/generated/api/resources/secrets
printf '%s\n' '--- connection tests ---'
rg -l 'probeFailureMessage|probeRequestFor|connections' web/packages/agenta-entities/tests web/packages/agenta-entities/src --glob '*.{ts,tsx}' |
while read -r f; do
echo "--- $f"
rg -n -C10 'probeFailureMessage|statusCode|response: \{status|extractApiErrorMessage' "$f" || true
doneRepository: Agenta-AI/agenta
Length of output: 50372
Update error handling for Fern errors.
AgentaApiError exposes statusCode and body, not response.status. The current code treats Fern 404 and 4xx failures as unreachable-provider errors. extractApiErrorMessage also does not inspect body, so it cannot return the API payload message.
27b0abd to
1145d25
Compare
6533af8 to
9e3da5e
Compare
1145d25 to
8dd9a93
Compare
9e3da5e to
e2a625c
Compare
8dd9a93 to
7d78428
Compare
2 participants
Context
The frontend previously copied backend fields by hand and called the provider probe through raw Axios. It also made managed behavior depend on the internal
managed_bycomponent string. That duplicated the contract and made the UI responsible for backend implementation details.Changes
Fern is regenerated from the final EE OpenAPI contract for Python and TypeScript. The secret entity now consumes
PublicSecretResponseDto,SecretValueStatus,SecretManagementPolicy, and the generated provider-probe request and response.Before:
After:
The UI maps
value_status.configuredandvalue_status.previewinto its provider-connection view model. It checks the exactmanagement.policy === "manager_only"policy rather than a manager-name string.Manager-only rows are hidden from Settings and edit drawers. They remain in the shared connection atom, agent creation defaults, model picker, and key-status checks, so seeded credentials still power agents.
The probe uses the Fern secrets client and project-scoped request options. Independent Zod validation remains at the frontend boundary for drift detection.
Tests / notes
@agenta/entitiestests passed.@agenta/entity-uitests passed.@agenta/entities,@agenta/entity-ui, and@agenta/settings-ui.pnpm lint-fixpassed. It reported four existing mobile hook warnings and no errors.docs/design/write-only-secrets/implementation-report.md.docs/design/write-only-secrets/qa.md.What to QA
secret_idthrough Fern and refresh models without exposing the key.This PR is stacked on #6195 (
fix/provider-probe-stored-key), whose chain is rooted inrelease/v0.114.0. Merge the stack in order: #6164, #6165, #6138, #6195, then #6174.