feat(platform): external-agent BYO credentials + user env & secrets#1892
Merged
Conversation
…(backend)
Add a per-agent authMode ('managed' | 'byo') for external agents and a
user-level env/secrets store that auto-attaches to a user's sandboxes.
- agent schema + SerializableAgentConfig + AgentJsonConfig carry authMode;
superRefine restricts it to external-agent.
- New org governance policy external_agent_byo (default off, fail-closed)
gates byo at the runner; managed path is unchanged.
- New sandboxUserEnv table + CRUD: V8 mutations/queries (write-only secrets,
masked reads) and a Node action that encrypts (JWE) on write and
decrypts+returns env for injection. Reuses lib/crypto + the credential
audit trail.
- Runner: for byo, skip provider provisioning / gateway hardening / VK mint /
token insert; pass the raw model + no gateway; inject the user's env/secrets
every turn (managed and byo). Continuation re-attaches, so it's
mode-agnostic.
- Adapter: byo injects no gateway/key env, raw model passthrough, no alias
pinning, and lifts the governance-motivated WebSearch/WebFetch denial
(native tools work on the user's own credential). gateway made optional;
OpenCode stays managed-only.
… env validators - build-exec.test.ts: byo injects no gateway/key env, raw model passthrough, no alias pinning, lifts WebSearch/WebFetch denial, omits the integration bridge; managed unaffected; OpenCode rejects byo. - agents.test.ts: authMode accepted on external-agent, rejected on chat. - governance.test.ts: external_agent_byo defaults allowed=false (fail-closed). - user_env_constants.test.ts: env key/value validators.
…ttings UI - New user-level Environment & Secrets settings page (route + desktop rail + mobile menu), wired to listMyEnv / upsertMyEnvVar / deleteMyEnvVar; secrets are write-only (masked), inline ConvexError validation, per-row delete. - Agent editor (instructions tab): authMode RadioGroup shown only for external-agent (default Managed); selecting BYO swaps the platform model catalog for a free-text raw-model-id editor, clears stale platform slugs on managed->byo, and shows the BYO note. authMode persists via saveAgent. - i18n keys (en/de/fr); routeTree regenerated. - Backend: rewrite two user_env list maps without object-spread (no-map-spread lint).
Driven by live E2E of a BYO Claude Code agent: - Drop the org-level external_agent_byo enable switch entirely (governance policy + runner gate + editor). The per-agent authMode is the sole control — configuring an agent is already privileged. - BYO model is optional: supportedModels defaults to [] and the >=1 rule moves into the superRefine (skipped for BYO); config conversion no longer throws on empty; the runner treats the 'default' sentinel as 'no model' (omit -> CLI default); the BYO model editor allows zero entries. - Bypass platform model-access RBAC + default-model resolution for BYO in chat_turn_generate (raw provider ids aren't catalog entries) — this was blocking the turn with 'no model permitted'. - On a BYO turn, unset stale platform LLM env (ANTHROPIC_*, alias/subagent pins) from the reused sandbox's session env so a prior managed turn's revoked VK can't shadow CLAUDE_CODE_OAUTH_TOKEN. - Guard supportedModels against undefined in the agent editor (memoized). E2E confirms the wiring: the user's CLAUDE_CODE_OAUTH_TOKEN is injected cleanly with no conflicting gateway/VK env and a raw model passthrough; the turn reaches the live LLM call.
A pasted credential commonly carries a trailing newline / leading indent (e.g. the wrapped output of 'claude setup-token'), which silently corrupts a bearer token and 401s. Trim leading/trailing whitespace on store; interior whitespace is preserved so multi-line secrets (PEM keys) survive.
Pasting a token that wrapped across terminal lines embeds an interior space/ newline that silently corrupts the credential (→ 401), and it's brutal to debug after the fact. Add hasInteriorWhitespace() + a loud inline warning on the env/secret add form (en/de/fr). Non-blocking — legitimately multi-line secrets (PEM keys) contain interior newlines, so the user can still proceed.
…odels warning A BYO external agent has no platform-catalog model, so the model picker showed a red 'No models available' / 'no usable model' warning — misleading, since BYO uses the user's own credentials and a raw model id. Thread authMode through the listAgents projection + ChatAgent shape, and render a non-interactive indicator (the raw model id, or 'Default model') with a BYO tooltip. en/de/fr.
The authMode docstrings in agents.ts, file_utils.ts, run_external_agent.ts, and the agent_chat config type claimed BYO is "gated by the org `external_agent_byo` policy at run time." No such policy exists, and the run handler comment already states the per-agent authMode is the sole control (configuring an agent is itself a privileged action). Align the docstrings with the implemented behavior so they stop describing a gate that was never built.
Document the bring-your-own credentials mode and the per-user Environment & secrets page added on this branch. - Rework platform/agents/external-agent.md: scope the "model reached through the gateway, never a raw provider key" claim to managed mode, and add a "Managed and bring-your-own credentials" section plus BYO notes in the engines/models and cost sections. BYO bypasses the gateway, injects the user's own provider key into the sandbox, and moves billing and limits to the user's provider account. - Add platform/member/environment.md documenting the per-(org,user) env vars and write-only secrets, their validation rules, and how they are injected into the sandbox; register it in nav. - Cross-link from member/preferences.md and reconcile member/overview.md's "Settings hidden for Members" claim with the personal settings group. - Mirror every change across en/de/fr and regenerate the frontmatter manifest.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -0,0 +1,127 @@ | |||
| 'use node'; | |||
Contributor
📝 WalkthroughWalkthroughThis PR introduces two interconnected features. First, external agents gain an Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent_adapters/src/claude_code/adapter.ts (1)
106-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
authMode, not gateway presence, as the adapter mode switch. Both adapter paths currently let a leftovergatewayoverride the requested BYO semantics.
packages/agent_adapters/src/claude_code/adapter.ts#L106-L119: require!byobefore adding the integration MCP bridge.packages/agent_adapters/src/opencode/adapter.ts#L27-L33: throw whenspec.authMode === 'byo'before checkingspec.gateway.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent_adapters/src/claude_code/adapter.ts` around lines 106 - 119, The adapter is using gateway presence as the primary check for whether to add the integration MCP bridge, which can incorrectly allow a leftover gateway configuration to override BYO (bring-your-own) semantics. In packages/agent_adapters/src/claude_code/adapter.ts (lines 106-119), add a check for !byo or ensure authMode is not 'byo' before adding the integrations MCP server configuration block. In packages/agent_adapters/src/opencode/adapter.ts (lines 27-33), throw an error when spec.authMode === 'byo' before any gateway-related checks to explicitly prevent gateway from being used in BYO mode. Use authMode as the authoritative mode switch to ensure BYO semantics are never overridden by gateway presence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/agent_adapters/src/types.ts`:
- Around line 32-40: The current type definition in types.ts allows authMode to
be omitted (defaulting to 'managed') while also allowing gateway to be omitted,
which violates the documented coupling requirement that gateway must be present
for managed runs and absent for byo runs. Refactor the type definition using
TypeScript discriminated unions to create two distinct type variants: one for
authMode 'managed' that requires gateway to be present, and one for authMode
'byo' that prohibits gateway. Use a union type to combine these variants,
ensuring that invalid combinations like omitting both authMode (defaulting to
managed) and gateway cannot compile and must be caught at the type level rather
than at runtime.
In
`@services/platform/app/features/settings/user-env/components/user-env-settings.tsx`:
- Around line 231-232: The <ul> elements in user-env-settings.tsx are missing
the explicit role="list" attribute required for proper accessibility with
VoiceOver semantics in Safari. Add role="list" to the <ul> element that has
className="divide-border divide-y" and aria-hidden="true" at line 231, and also
add the same role attribute to the corresponding <ul> element at line 250, to
ensure consistent accessibility compliance across all list elements in this
component.
- Around line 123-128: Remove unnecessary memoization hooks from four functions
in the user-env-settings component. Unwrap the reset function from useCallback
(lines 123–128), unwrap the handleSubmit function from useCallback (lines
130–156), unwrap the displayValue variable from useMemo (lines 275–281), and
unwrap the handleDelete function from useCallback (lines 283–293). These hooks
provide no performance benefit because the functions are either simple with no
dependencies, called inline without being passed to memoized children, or passed
to non-memoized components. Replace each memoized definition with the direct
function or variable assignment.
In `@services/platform/convex/agents/external_agent/run_external_agent.ts`:
- Around line 453-456: The sessionEnvPatch call only sets the current user
environment values but does not unset entries that have been removed or renamed,
allowing revoked credentials to persist in the session. Modify the
sessionEnvPatch invocation to include an unset operation alongside the set
operation. Identify which environment variable keys existed in the previous
session state but are no longer present in the current userEnv.env, and add
those keys to an unset array in the patch object passed to sessionEnvPatch. This
ensures that deleted or renamed secrets are properly removed from the sandbox
session environment along with the current values being set.
In `@services/platform/convex/lib/agent_chat/start_agent_chat.ts`:
- Around line 656-658: The current approach of only forwarding authMode is
insufficient because budget enforcement gates still execute before scheduling in
the start_agent_chat function. Refactor the code to separate budget enforcement
logic from feature-flag resolution: keep file-upload and feature-flag policy
checks applied to all turns, but conditionally skip the platform user/agent
budget cap enforcement when enforcedConfig.authMode indicates a BYO (Bring Your
Own) external-agent turn, since BYO turns mint no VK and should bill directly to
the user's provider account rather than consuming platform spend capacity.
Restructure the budget gate block to check authMode and bypass spending cap
validation specifically for BYO cases while preserving all other policy
enforcement.
In `@services/platform/convex/node_only/sandbox/run_agent.ts`:
- Around line 252-259: The code currently omits the spec.gateway property when
gatewayBaseUrl or gatewayToken are missing, which allows managed runs to bypass
gateway governance. Add a fail-closed validation check that requires both
args.gatewayBaseUrl and args.gatewayToken to be present when args.authMode is
undefined or set to managed. If either gateway credential is missing in a
managed run, throw an error to prevent the run from proceeding without proper
gateway configuration instead of silently omitting it.
In `@services/platform/convex/sandbox/user_env.ts`:
- Around line 83-90: Replace the `.collect()` materializations with `for await`
iteration patterns in the Convex query handlers throughout this file. At lines
83-90, instead of calling `.collect()` and then accessing `.length` on the
result, use a `for await` loop to iterate over the query results and count items
as you iterate. Apply the same pattern at lines 123-128 and 169-176 where
`.collect()` is being used to materialize Convex query results. The goal is to
avoid loading full result sets into memory and instead process results
iteratively as Convex best practices recommend.
- Around line 56-105: The mutation handler in upsertUserEnvInternal (the async
function starting at line 56) needs to re-validate organization membership
before performing any database operations. Currently, membership is only checked
in the caller upsertMyEnvVar, which creates a race condition if membership
changes between the validation and this mutation execution. Add an organization
membership validation check at the beginning of the handler, before the
ctx.db.query call for the existing environment variable lookup, to ensure the
user still has access to the organization within the same transaction as the
write operations.
In `@services/platform/lib/shared/schemas/agents.test.ts`:
- Around line 115-134: Add two new test cases to provide direct regression
coverage for the empty supportedModels array behavior across different authMode
paths. First, add a positive test case that verifies an external-agent with
authMode set to 'byo' and an empty supportedModels array passes validation (this
should succeed). Second, add a negative test case that verifies an
external-agent with authMode set to 'managed' and an empty supportedModels array
fails validation (this should have success: false). Insert these test cases
after the existing authMode validation tests (after the 'rejects authMode on a
non-external-agent' test) to ensure the conditional model rule allowing empty
supportedModels only for BYO external-agents is properly covered and
regression-protected.
In `@services/platform/messages/de.json`:
- Around line 896-897: The byoDefault label value "Standardmodell" does not
clearly communicate that this is for a user-supplied provider model and reads
like a standard catalog default instead. In the de.json file, replace the
byoDefault value with a label that explicitly indicates this is a user-supplied
or user-provided model, making it clear to users that they are using their own
credentials and model configuration rather than a platform-provided default.
- Line 4102: The whitespaceWarning message in de.json incorrectly warns about
all whitespace when the backend only flags interior whitespace (leading and
trailing whitespace is trimmed). Update the whitespaceWarning string to clarify
that only interior whitespace is problematic, while explicitly confirming that
leading/trailing whitespace is removed and multi-line secrets like PEM keys are
permitted. This will align the user-facing text with actual backend behavior.
In `@services/platform/messages/fr.json`:
- Line 898: The French UI text is using formal address pronouns (vous/votre)
instead of the informal pronouns (tu/ton/tes) required by the repository's
coding guidelines. At services/platform/messages/fr.json#L898-L898 for the BYO
tooltip, replace "vos propres identifiants" with "tes propres identifiants" and
"les vôtres" with "les tiens". At services/platform/messages/fr.json#L4103-L4103
for the whitespace warning, similarly replace all instances of vous and votre
with their informal equivalents tu and ton/tes to maintain consistent informal
French across the entire codebase.
---
Outside diff comments:
In `@packages/agent_adapters/src/claude_code/adapter.ts`:
- Around line 106-119: The adapter is using gateway presence as the primary
check for whether to add the integration MCP bridge, which can incorrectly allow
a leftover gateway configuration to override BYO (bring-your-own) semantics. In
packages/agent_adapters/src/claude_code/adapter.ts (lines 106-119), add a check
for !byo or ensure authMode is not 'byo' before adding the integrations MCP
server configuration block. In packages/agent_adapters/src/opencode/adapter.ts
(lines 27-33), throw an error when spec.authMode === 'byo' before any
gateway-related checks to explicitly prevent gateway from being used in BYO
mode. Use authMode as the authoritative mode switch to ensure BYO semantics are
never overridden by gateway presence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bbe353a1-617c-46c1-a080-54e752d66af9
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (49)
docs/de/platform/agents/external-agent.mddocs/de/platform/member/environment.mddocs/de/platform/member/overview.mddocs/de/platform/member/preferences.mddocs/en/platform/agents/external-agent.mddocs/en/platform/member/environment.mddocs/en/platform/member/overview.mddocs/en/platform/member/preferences.mddocs/fr/platform/agents/external-agent.mddocs/fr/platform/member/environment.mddocs/fr/platform/member/overview.mddocs/fr/platform/member/preferences.mddocs/nav.jsonpackages/agent_adapters/src/build-exec.test.tspackages/agent_adapters/src/claude_code/adapter.tspackages/agent_adapters/src/opencode/adapter.tspackages/agent_adapters/src/types.tsservices/docs/app/content/frontmatter.jsonservices/platform/app/features/agents/components/agent-navigation.tsxservices/platform/app/features/agents/components/byo-model-editor.tsxservices/platform/app/features/chat/components/model-selector.tsxservices/platform/app/features/chat/hooks/queries.tsservices/platform/app/features/settings/components/settings-rail.tsxservices/platform/app/features/settings/components/use-settings-menu-groups.tsservices/platform/app/features/settings/user-env/components/user-env-settings.tsxservices/platform/app/features/settings/user-env/hooks/mutations.tsservices/platform/app/features/settings/user-env/hooks/queries.tsservices/platform/app/routeTree.gen.tsservices/platform/app/routes/dashboard/$id/agents/$agentId/instructions.tsxservices/platform/app/routes/dashboard/$id/settings/environment.tsxservices/platform/convex/agents/chat_turn_generate.tsservices/platform/convex/agents/config.tsservices/platform/convex/agents/external_agent/run_external_agent.tsservices/platform/convex/agents/file_actions.tsservices/platform/convex/agents/file_utils.tsservices/platform/convex/lib/agent_chat/start_agent_chat.tsservices/platform/convex/lib/agent_chat/types.tsservices/platform/convex/node_only/sandbox/run_agent.tsservices/platform/convex/sandbox/sessions_schema.tsservices/platform/convex/sandbox/user_env.tsservices/platform/convex/sandbox/user_env_actions.tsservices/platform/convex/sandbox/user_env_constants.test.tsservices/platform/convex/sandbox/user_env_constants.tsservices/platform/convex/schema.tsservices/platform/lib/shared/schemas/agents.test.tsservices/platform/lib/shared/schemas/agents.tsservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.json
Comment on lines
+32
to
+40
| /** | ||
| * Credential mode. 'managed' (default / absent): route through the platform | ||
| * gateway with the minted virtual key. 'byo': no gateway — the agent uses the | ||
| * credentials the user injected into the session env, with a raw model | ||
| * passthrough and native web tools enabled. | ||
| */ | ||
| authMode?: 'managed' | 'byo'; | ||
| /** Platform LLM gateway. Present for managed runs; ABSENT for byo. */ | ||
| gateway?: GatewayTarget; |
Contributor
There was a problem hiding this comment.
Enforce authMode↔gateway coupling at the type level.
authMode and gateway are documented as dependent, but the current shape allows authMode to be omitted (managed default) while also omitting gateway. That invalid state now compiles and shifts failure to runtime.
Suggested contract hardening
-export interface AgentRunSpec {
+interface AgentRunSpecBase {
prompt: string;
model?: string;
agentSessionId?: string;
maxTurns?: number;
permissionMode?: 'plan' | 'execute';
systemPromptAppend?: string;
- authMode?: 'managed' | 'byo';
- gateway?: GatewayTarget;
integrationsBaseUrl?: string;
workdir: string;
browserMcp?: boolean;
browserCdp?: boolean;
execId?: string;
-}
+}
+
+type ManagedAuth = { authMode?: 'managed'; gateway: GatewayTarget };
+type ByoAuth = { authMode: 'byo'; gateway?: never };
+
+export type AgentRunSpec = AgentRunSpecBase & (ManagedAuth | ByoAuth);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/agent_adapters/src/types.ts` around lines 32 - 40, The current type
definition in types.ts allows authMode to be omitted (defaulting to 'managed')
while also allowing gateway to be omitted, which violates the documented
coupling requirement that gateway must be present for managed runs and absent
for byo runs. Refactor the type definition using TypeScript discriminated unions
to create two distinct type variants: one for authMode 'managed' that requires
gateway to be present, and one for authMode 'byo' that prohibits gateway. Use a
union type to combine these variants, ensuring that invalid combinations like
omitting both authMode (defaulting to managed) and gateway cannot compile and
must be caught at the type level rather than at runtime.
Comment on lines
+123
to
+128
| const reset = useCallback(() => { | ||
| setKey(''); | ||
| setValue(''); | ||
| setIsSecret(false); | ||
| setError(null); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
cat -n services/platform/app/features/settings/user-env/components/user-env-settings.tsx | head -300Repository: tale-project/tale
Length of output: 11003
🏁 Script executed:
tail -n +300 services/platform/app/features/settings/user-env/components/user-env-settings.tsxRepository: tale-project/tale
Length of output: 1105
🏁 Script executed:
# Check if DeleteDialog or any child components used are memoized
rg "memo\(|React\.memo" services/platform/app/features/settings/user-env/components/
rg "memo\(|React\.memo" services/platform/app/components/ui/dialog/Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check if Input, Switch, Button, or DeleteDialog components are wrapped with memo
rg "export.*memo|React\.memo" services/platform/app/components/ui/forms/input services/platform/app/components/ui/forms/switch services/platform/app/components/ui/button services/platform/app/components/ui/dialog/ 2>/dev/null | head -20Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check the DeleteDialog component definition
fd -e tsx "delete-dialog" services/platform/app/components/ui/dialog/ -x cat -n {}Repository: tale-project/tale
Length of output: 13514
🏁 Script executed:
# Check if EnvVarRow is wrapped in memo
rg "memo.*EnvVarRow|React\.memo" services/platform/app/features/settings/user-env/components/user-env-settings.tsxRepository: tale-project/tale
Length of output: 43
Remove unprofiled useMemo/useCallback usage.
These memoization hooks wrap local logic with no demonstrated performance benefit. Per the project guidelines, use them only when profiling justifies the additional complexity.
- Lines 123–128 (
reset): Simple state reset function with no external dependencies. No memoization benefit. - Lines 130–156 (
handleSubmit): Called via inline arrow function in form submission handler, not passed to memoized children. Memoization provides no performance gain. - Lines 275–281 (
displayValue): Trivial ternary operation; memoization overhead exceeds any benefit. - Lines 283–293 (
handleDelete): Passed toDeleteDialog, which is not memoized. Stable callback reference provides no optimization since the parent component rerenders regardless.
Remove all four memoizations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/settings/user-env/components/user-env-settings.tsx`
around lines 123 - 128, Remove unnecessary memoization hooks from four functions
in the user-env-settings component. Unwrap the reset function from useCallback
(lines 123–128), unwrap the handleSubmit function from useCallback (lines
130–156), unwrap the displayValue variable from useMemo (lines 275–281), and
unwrap the handleDelete function from useCallback (lines 283–293). These hooks
provide no performance benefit because the functions are either simple with no
dependencies, called inline without being passed to memoized children, or passed
to non-memoized components. Replace each memoized definition with the direct
function or variable assignment.
Source: Coding guidelines
Comment on lines
+231
to
+232
| <ul className="divide-border divide-y" aria-hidden="true"> | ||
| {Array.from({ length: 3 }).map((_, i) => ( |
Contributor
There was a problem hiding this comment.
Add explicit role="list" on the <ul> elements.
For this codebase’s accessibility baseline (including Safari/VoiceOver behavior), these lists should carry role="list".
Suggested fix
- <ul className="divide-border divide-y" aria-hidden="true">
+ <ul role="list" className="divide-border divide-y" aria-hidden="true">
@@
- <ul className="divide-border divide-y">
+ <ul role="list" className="divide-border divide-y">Based on learnings: keep role="list" on ul elements in TSX components for VoiceOver semantics.
Also applies to: 250-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/settings/user-env/components/user-env-settings.tsx`
around lines 231 - 232, The <ul> elements in user-env-settings.tsx are missing
the explicit role="list" attribute required for proper accessibility with
VoiceOver semantics in Safari. Add role="list" to the <ul> element that has
className="divide-border divide-y" and aria-hidden="true" at line 231, and also
add the same role attribute to the corresponding <ul> element at line 250, to
ensure consistent accessibility compliance across all list elements in this
component.
Source: Learnings
Comment on lines
+453
to
+456
| if (Object.keys(userEnv.env).length > 0) { | ||
| const denied = await sessionEnvPatch(sessionId, { | ||
| set: userEnv.env, | ||
| }); |
Contributor
There was a problem hiding this comment.
Unset removed user env entries before applying the current set.
This sandbox is reused per user/org, but this patch only writes the currently resolved env values. If a member deletes or renames a secret, the previous session env value remains available to later turns, so revoked BYO credentials can keep authenticating.
🛡️ Suggested direction
- if (Object.keys(userEnv.env).length > 0) {
+ // Have resolveUserEnv return the names previously injected for this
+ // user/session, or store a session-scoped injected-name marker, then
+ // unset names that are no longer present before setting current values.
+ const unset = userEnv.unset ?? [];
+ if (unset.length > 0 || Object.keys(userEnv.env).length > 0) {
const denied = await sessionEnvPatch(sessionId, {
+ ...(unset.length > 0 && { unset }),
set: userEnv.env,
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/agents/external_agent/run_external_agent.ts` around
lines 453 - 456, The sessionEnvPatch call only sets the current user environment
values but does not unset entries that have been removed or renamed, allowing
revoked credentials to persist in the session. Modify the sessionEnvPatch
invocation to include an unset operation alongside the set operation. Identify
which environment variable keys existed in the previous session state but are no
longer present in the current userEnv.env, and add those keys to an unset array
in the patch object passed to sessionEnvPatch. This ensures that deleted or
renamed secrets are properly removed from the sandbox session environment along
with the current values being set.
Comment on lines
+656
to
+658
| ...(enforcedConfig.authMode !== undefined && { | ||
| authMode: enforcedConfig.authMode, | ||
| }), |
Contributor
There was a problem hiding this comment.
Bypass platform spend caps for BYO external-agent turns.
Forwarding authMode here is not enough: this function still runs the platform user/agent budget gates before scheduling. A BYO turn mints no VK and bills the user's provider account, so an exhausted platform cap can incorrectly block BYO usage.
🛠️ Suggested direction
+ const isByoExternal =
+ enforcedConfig.primaryBehavior === 'external-agent' &&
+ enforcedConfig.authMode === 'byo';
+
let enforcedConfig = agentConfig;
let governanceMaxContextTokens: number | undefined;
- if (userId) {
+ if (userId && !isByoExternal) {
const { userTeamIds, userRole } = await resolveBudgetContext(If file-upload and feature-flag policy should still apply to BYO, split budget enforcement from feature-flag resolution instead of skipping the whole block.
📝 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
| ...(enforcedConfig.authMode !== undefined && { | |
| authMode: enforcedConfig.authMode, | |
| }), | |
| let enforcedConfig = agentConfig; | |
| let governanceMaxContextTokens: number | undefined; | |
| const isByoExternal = | |
| agentConfig.primaryBehavior === 'external-agent' && | |
| agentConfig.authMode === 'byo'; | |
| if (userId && !isByoExternal) { | |
| const { userTeamIds, userRole } = await resolveBudgetContext( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/lib/agent_chat/start_agent_chat.ts` around lines 656
- 658, The current approach of only forwarding authMode is insufficient because
budget enforcement gates still execute before scheduling in the start_agent_chat
function. Refactor the code to separate budget enforcement logic from
feature-flag resolution: keep file-upload and feature-flag policy checks applied
to all turns, but conditionally skip the platform user/agent budget cap
enforcement when enforcedConfig.authMode indicates a BYO (Bring Your Own)
external-agent turn, since BYO turns mint no VK and should bill directly to the
user's provider account rather than consuming platform spend capacity.
Restructure the budget gate block to check authMode and bypass spending cap
validation specifically for BYO cases while preserving all other policy
enforcement.
Comment on lines
+83
to
+90
| const count = ( | ||
| await ctx.db | ||
| .query('sandboxUserEnv') | ||
| .withIndex('by_org_user', (q) => | ||
| q.eq('organizationId', args.organizationId).eq('userId', args.userId), | ||
| ) | ||
| .collect() | ||
| ).length; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace .collect() with for await iteration in Convex query handlers.
This file materializes full result sets in multiple handlers. Repo guidance for Convex queries is to iterate with for await instead of .collect().
Suggested direction
- const count = (
- await ctx.db
- .query('sandboxUserEnv')
- .withIndex('by_org_user', (q) =>
- q.eq('organizationId', args.organizationId).eq('userId', args.userId),
- )
- .collect()
- ).length;
+ let count = 0;
+ for await (const _row of ctx.db
+ .query('sandboxUserEnv')
+ .withIndex('by_org_user', (q) =>
+ q.eq('organizationId', args.organizationId).eq('userId', args.userId),
+ )) {
+ count += 1;
+ if (count >= MAX_ENV_VARS_PER_USER) break;
+ }Based on learnings: “Always use for await loops to iterate over Convex query results and avoid collecting with .collect().”
Also applies to: 123-128, 169-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/sandbox/user_env.ts` around lines 83 - 90, Replace
the `.collect()` materializations with `for await` iteration patterns in the
Convex query handlers throughout this file. At lines 83-90, instead of calling
`.collect()` and then accessing `.length` on the result, use a `for await` loop
to iterate over the query results and count items as you iterate. Apply the same
pattern at lines 123-128 and 169-176 where `.collect()` is being used to
materialize Convex query results. The goal is to avoid loading full result sets
into memory and instead process results iteratively as Convex best practices
recommend.
Source: Learnings
Comment on lines
+115
to
+134
| it('accepts authMode managed/byo on an external-agent', () => { | ||
| expect( | ||
| agentJsonSchema.safeParse({ ...externalBase, authMode: 'managed' }) | ||
| .success, | ||
| ).toBe(true); | ||
| expect( | ||
| agentJsonSchema.safeParse({ ...externalBase, authMode: 'byo' }).success, | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it('rejects authMode on a non-external-agent (chat)', () => { | ||
| expect( | ||
| agentJsonSchema.safeParse({ | ||
| displayName: 'Chat Agent', | ||
| supportedModels: ['openrouter:anthropic/claude-sonnet-4.6'], | ||
| systemInstructions: 'hi', | ||
| authMode: 'byo', | ||
| }).success, | ||
| ).toBe(false); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add direct regression coverage for empty supportedModels in BYO vs managed paths.
This suite validates authMode placement, but it does not pin the newly introduced conditional model rule ([] allowed only for BYO external-agent). Add one positive (BYO+empty) and one negative (managed/implicit managed+empty) case.
Suggested test additions
it('accepts authMode managed/byo on an external-agent', () => {
@@
});
+ it('accepts BYO external-agent with no supportedModels', () => {
+ expect(
+ agentJsonSchema.safeParse({
+ displayName: 'BYO Agent',
+ primaryBehavior: 'external-agent' as const,
+ authMode: 'byo',
+ }).success,
+ ).toBe(true);
+ });
+
+ it('rejects managed external-agent with no supportedModels', () => {
+ expect(
+ agentJsonSchema.safeParse({
+ displayName: 'Managed Agent',
+ primaryBehavior: 'external-agent' as const,
+ authMode: 'managed',
+ }).success,
+ ).toBe(false);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('accepts authMode managed/byo on an external-agent', () => { | |
| expect( | |
| agentJsonSchema.safeParse({ ...externalBase, authMode: 'managed' }) | |
| .success, | |
| ).toBe(true); | |
| expect( | |
| agentJsonSchema.safeParse({ ...externalBase, authMode: 'byo' }).success, | |
| ).toBe(true); | |
| }); | |
| it('rejects authMode on a non-external-agent (chat)', () => { | |
| expect( | |
| agentJsonSchema.safeParse({ | |
| displayName: 'Chat Agent', | |
| supportedModels: ['openrouter:anthropic/claude-sonnet-4.6'], | |
| systemInstructions: 'hi', | |
| authMode: 'byo', | |
| }).success, | |
| ).toBe(false); | |
| }); | |
| it('accepts authMode managed/byo on an external-agent', () => { | |
| expect( | |
| agentJsonSchema.safeParse({ ...externalBase, authMode: 'managed' }) | |
| .success, | |
| ).toBe(true); | |
| expect( | |
| agentJsonSchema.safeParse({ ...externalBase, authMode: 'byo' }).success, | |
| ).toBe(true); | |
| }); | |
| it('accepts BYO external-agent with no supportedModels', () => { | |
| expect( | |
| agentJsonSchema.safeParse({ | |
| displayName: 'BYO Agent', | |
| primaryBehavior: 'external-agent', | |
| authMode: 'byo', | |
| }).success, | |
| ).toBe(true); | |
| }); | |
| it('rejects managed external-agent with no supportedModels', () => { | |
| expect( | |
| agentJsonSchema.safeParse({ | |
| displayName: 'Managed Agent', | |
| primaryBehavior: 'external-agent', | |
| authMode: 'managed', | |
| }).success, | |
| ).toBe(false); | |
| }); | |
| it('rejects authMode on a non-external-agent (chat)', () => { | |
| expect( | |
| agentJsonSchema.safeParse({ | |
| displayName: 'Chat Agent', | |
| supportedModels: ['openrouter:anthropic/claude-sonnet-4.6'], | |
| systemInstructions: 'hi', | |
| authMode: 'byo', | |
| }).success, | |
| ).toBe(false); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/lib/shared/schemas/agents.test.ts` around lines 115 - 134,
Add two new test cases to provide direct regression coverage for the empty
supportedModels array behavior across different authMode paths. First, add a
positive test case that verifies an external-agent with authMode set to 'byo'
and an empty supportedModels array passes validation (this should succeed).
Second, add a negative test case that verifies an external-agent with authMode
set to 'managed' and an empty supportedModels array fails validation (this
should have success: false). Insert these test cases after the existing authMode
validation tests (after the 'rejects authMode on a non-external-agent' test) to
ensure the conditional model rule allowing empty supportedModels only for BYO
external-agents is properly covered and regression-protected.
Comment on lines
+896
to
+897
| "byoDefault": "Standardmodell", | ||
| "byoTooltip": "Dieser Agent verwendet deine eigenen Anmeldedaten (BYO) — Modell, Abrechnung und Limits liegen bei dir.", |
Contributor
There was a problem hiding this comment.
Rename the BYO badge.
"Standardmodell" reads like a normal catalog default, but this label is rendered for BYO agents and should clearly indicate a user-supplied provider model.
💡 Suggested copy
- "byoDefault": "Standardmodell",
+ "byoDefault": "Eigenes Modell",📝 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
| "byoDefault": "Standardmodell", | |
| "byoTooltip": "Dieser Agent verwendet deine eigenen Anmeldedaten (BYO) — Modell, Abrechnung und Limits liegen bei dir.", | |
| "byoDefault": "Eigenes Modell", | |
| "byoTooltip": "Dieser Agent verwendet deine eigenen Anmeldedaten (BYO) — Modell, Abrechnung und Limits liegen bei dir.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/messages/de.json` around lines 896 - 897, The byoDefault
label value "Standardmodell" does not clearly communicate that this is for a
user-supplied provider model and reads like a standard catalog default instead.
In the de.json file, replace the byoDefault value with a label that explicitly
indicates this is a user-supplied or user-provided model, making it clear to
users that they are using their own credentials and model configuration rather
than a platform-provided default.
| "valuePlaceholder": "Wert", | ||
| "secretLabel": "Geheimnis", | ||
| "secretHelp": "Wenn aktiviert, wird der Wert verschlüsselt und ist nur beschreibbar — nach dem Speichern wird er nie wieder angezeigt.", | ||
| "whitespaceWarning": "Dieser Wert enthält Leerzeichen oder Zeilenumbrüche. Anmeldedaten sollten das nicht — falls du ein Token eingefügt hast, das im Terminal über mehrere Zeilen umgebrochen wurde, entferne sie. (Mehrzeilige Geheimnisse wie PEM-Schlüssel dürfen sie behalten.)", |
Contributor
There was a problem hiding this comment.
Tighten the whitespace warning.
The current copy warns about any whitespace, but the backend only flags interior whitespace; leading/trailing whitespace is trimmed, and multi-line secrets are allowed. This text should mirror that behavior.
💡 Suggested copy
- "whitespaceWarning": "Dieser Wert enthält Leerzeichen oder Zeilenumbrüche. Anmeldedaten sollten das nicht — falls du ein Token eingefügt hast, das im Terminal über mehrere Zeilen umgebrochen wurde, entferne sie. (Mehrzeilige Geheimnisse wie PEM-Schlüssel dürfen sie behalten.)",
+ "whitespaceWarning": "Dieser Wert enthält innere Leerzeichen oder Zeilenumbrüche. Falls du ein Token eingefügt hast, das im Terminal über mehrere Zeilen umgebrochen wurde, entferne diese Zeichen. Mehrzeilige Geheimnisse wie PEM-Schlüssel sind davon ausgenommen.",📝 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
| "whitespaceWarning": "Dieser Wert enthält Leerzeichen oder Zeilenumbrüche. Anmeldedaten sollten das nicht — falls du ein Token eingefügt hast, das im Terminal über mehrere Zeilen umgebrochen wurde, entferne sie. (Mehrzeilige Geheimnisse wie PEM-Schlüssel dürfen sie behalten.)", | |
| "whitespaceWarning": "Dieser Wert enthält innere Leerzeichen oder Zeilenumbrüche. Falls du ein Token eingefügt hast, das im Terminal über mehrere Zeilen umgebrochen wurde, entferne diese Zeichen. Mehrzeilige Geheimnisse wie PEM-Schlüssel sind davon ausgenommen.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/messages/de.json` at line 4102, The whitespaceWarning
message in de.json incorrectly warns about all whitespace when the backend only
flags interior whitespace (leading and trailing whitespace is trimmed). Update
the whitespaceWarning string to clarify that only interior whitespace is
problematic, while explicitly confirming that leading/trailing whitespace is
removed and multi-line secrets like PEM keys are permitted. This will align the
user-facing text with actual backend behavior.
| "auto": "Auto", | ||
| "autoDescription": "Tale choisit le meilleur modèle pour chaque message — équilibre qualité, vitesse et coût.", | ||
| "byoDefault": "Modèle par défaut", | ||
| "byoTooltip": "Cet agent utilise vos propres identifiants (BYO) — le modèle, la facturation et les limites sont les vôtres.", |
Contributor
There was a problem hiding this comment.
Keep the French copy consistently informal.
Both strings slip into vous/votre, which breaks the repository rule to use tu across French UI text.
services/platform/messages/fr.json#L898-L898: rewrite the BYO tooltip withtu/ton/tes.services/platform/messages/fr.json#L4103-L4103: rewrite the whitespace warning withtu/ton/tes.
As per coding guidelines, use informal form across all languages (tu in French).
📍 Affects 1 file
services/platform/messages/fr.json#L898-L898(this comment)services/platform/messages/fr.json#L4103-L4103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/messages/fr.json` at line 898, The French UI text is using
formal address pronouns (vous/votre) instead of the informal pronouns
(tu/ton/tes) required by the repository's coding guidelines. At
services/platform/messages/fr.json#L898-L898 for the BYO tooltip, replace "vos
propres identifiants" with "tes propres identifiants" and "les vôtres" with "les
tiens". At services/platform/messages/fr.json#L4103-L4103 for the whitespace
warning, similarly replace all instances of vous and votre with their informal
equivalents tu and ton/tes to maintain consistent informal French across the
entire codebase.
Source: Coding guidelines
yannickmonney
pushed a commit
that referenced
this pull request
Jun 16, 2026
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
1 participant
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
Adds a bring-your-own (BYO) credentials auth mode for external agents, plus a per-user Environment & secrets store that feeds it.
By default an external agent runs in managed mode: the platform mints a per-turn Bifrost virtual key, routes the agent through its gateway, and enforces allowed models, usage metering, and the org's spend caps. The new per-agent BYO mode takes the platform out of the request path — no virtual key is minted, the agent authenticates with the user's own provider credentials, the model is a raw provider-id passthrough (no catalogue), native web tools stay enabled, and billing/limits move to the user's provider account. Credentials come from the new Environment & secrets page (scoped per
(org, user), secrets encrypted at rest and write-only) and are injected into the user's sandbox at turn start — for both modes.What's in the branch
authMode: 'managed' | 'byo'on external-agent configs. The BYO run path skips VK minting, gateway hardening, model-access RBAC, and the WebSearch/WebFetch denial; the schema relaxes the ≥1-model rule for BYO agents (empty = use the credential's default).run_external_agent.ts,config.ts,chat_turn_generate.ts,schemas/agents.ts.sandboxUserEnvtable +upsertMyEnvVar/listMyEnv/deleteMyEnvVarCRUD and the internalresolveUserEnvinjector. Secrets stored as JWE, write-only (masked on read), validation isolated in pureuser_env_constants.ts(unit-tested).AgentRunSpec.authMode+ optionalgateway; the Claude Code adapter branches managed/BYO; OpenCode is managed-only (throws on BYO)./settings/environmentpage.\uXXXXescapes).Docstring correction
The branch's
authModedocstrings claimed BYO is "gated by the orgexternal_agent_byopolicy at run time." No such policy was ever implemented, and the run handler already documents the per-agent toggle as the sole control. The four docstrings are corrected to match the shipped behaviour — there is no org-level gate; configuring an agent is itself the privileged action. If an operator allowlist is wanted, that's a follow-up, not a silent contradiction.Reviewer notes — worth scrutinising
external-agent.mdandmember/environment.md.PATH/LD_PRELOAD. The BYO path explicitly unsets staleANTHROPIC_*/ model vars before injection, andsessionEnvPatchreturns a runnerd-side denied list — so the reserved-name boundary lives in runnerd. Worth confirming it covers dangerous names.Pre-merge checklist
bun run check. Format, lint, and typecheck are green across all 48 workspaces. The platform test task reported 5convexTesttimeouts (auto_route_cache,update_user_name,slack/http_actions,integration_processing_records) under full-suite turbo load — all 34 pass in isolation in ~3s and none touch this branch's code. The feature's ownuser_env_constants.test.tspasses; the docs suite (142 tests) is green; platformtscis green after the docstring edit.services/platform/messages/{en,de,fr}.json— 38 keys, full en/de/fr parity.docs/,docs/de/, anddocs/fr/— reworkedplatform/agents/external-agent.md, addedplatform/member/environment.md, registered it in nav, and cross-linked frommember/preferences.md+member/overview.md, in all three locales.bun run --filter @tale/docs lintandbun run --filter @tale/docs test— both green (regenerated thefrontmatter.jsonmanifest).README.md/README.de.md/README.fr.md— README lists Claude Code as a connector but documents no credential/auth detail; BYO is a platform feature surfaced indocs/, not the high-level README.<Skeletonize>+ skeleton-aware leaves; the env list's placeholder rows useSkeletonText(the sanctioned primitive for variable-count rows), no magich-[…].Does this change need docs and translations? (decision tree)
Yes on every count — new message keys, new UI (the
/settings/environmentpage, the Credentials toggle, the BYO model editor, the chat picker indicator), a new agent config field (authMode), and new validation rules. Docs + translations ship in this PR.Test plan
••••••••) and is never returned in plaintext; delete it and confirm it leaves the list.Summary by CodeRabbit
Release Notes
New Features
Documentation