feat(capabilities): name-based credential selection for multi-credential delegation#329
feat(capabilities): name-based credential selection for multi-credential delegation#329
Conversation
📝 WalkthroughWalkthroughThis PR enhances credential selection in the AI agent capability delegation system by enabling name-based credential references alongside numeric IDs. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Deploying bubblelab-documentation with
|
| Latest commit: |
8e83000
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac1e6d77.bubblelab-documentation.pages.dev |
| Branch Preview URL: | https://feat-multi-credential.bubblelab-documentation.pages.dev |
There was a problem hiding this comment.
Pull request overview
Adds multi-credential delegation support by exposing multiple credentials to the master agent (as a pool) and enabling use-capability to select credentials by account name (with numeric fallback). This fits into the capability delegation pipeline by improving routing reliability when multiple accounts exist for the same integration.
Changes:
- Extend credential plumbing to support multiple credential IDs per type and propagate credential
namemetadata through API → runtime injection. - Inject an
credentialPoolintoai-agentbubbles (when multiple creds exist) and allowuse-capabilityto override subagent credentials by matching on account name (case-insensitive + substring) with numeric ID fallback. - Update capability prompt generation and subagent default model reasoning effort.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/create-bubblelab-app/templates/reddit-scraper/package.json | Bump template deps to newer BubbleLab package versions. |
| packages/create-bubblelab-app/templates/basic/package.json | Bump template deps to newer BubbleLab package versions. |
| packages/create-bubblelab-app/package.json | Version bump for create-bubblelab-app. |
| packages/bubble-shared-schemas/src/bubbleflow-execution-schema.ts | Allow credential mappings to accept arrays of IDs in validation schema. |
| packages/bubble-shared-schemas/package.json | Version bump for shared schemas package. |
| packages/bubble-scope-manager/package.json | Version bump for ts-scope-manager package. |
| packages/bubble-runtime/src/injection/BubbleInjector.ts | Inject credential pool + first-wins default behavior; add credential name metadata. |
| packages/bubble-runtime/package.json | Version bump for bubble-runtime package. |
| packages/bubble-core/src/bubbles/service-bubble/capability-pipeline.ts | Include credential account names in capability summaries; tweak subagent reasoning effort; add capability Q&A rule. |
| packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts | Add credentialPool param schema and implement name-based credential overrides for use-capability. |
| packages/bubble-core/package.json | Version bump for bubble-core package. |
| apps/bubblelab-api/src/services/execution.ts | Propagate credential name into runtime credential mappings. |
| apps/bubblelab-api/src/services/credential-helper.ts | Support credential ID arrays and include credential name in mappings. |
| apps/bubblelab-api/src/services/bubble-flow-parser.ts | Update credential merging types/logic to tolerate arrays. |
Comments suppressed due to low confidence (1)
apps/bubblelab-api/src/services/credential-helper.ts:66
- When supporting arrays of credential IDs, the implementation should preserve the user-specified ordering (since downstream injection treats the first credential as the default and preserves pool order).
inArray(userCredentials.id, credentialIds)does not guarantee result order, so the resultingcredentialMappings(and thus pool/default ordering) can be non-deterministic. Consider sortingencryptedCredentialsto match thecredentialIdssequence (or add an explicitorderBy) before constructingcredentialMappings.
for (const [, credentialIdOrIds] of Object.entries(credentialsObj)) {
const ids = Array.isArray(credentialIdOrIds)
? credentialIdOrIds
: [credentialIdOrIds];
for (const credentialId of ids) {
if (!credentialIds.includes(credentialId)) {
credentialIds.push(credentialId);
}
// Support multiple variable names per credential ID
if (!credentialIdToVarNames.has(credentialId)) {
credentialIdToVarNames.set(credentialId, []);
}
credentialIdToVarNames.get(credentialId)!.push(varName);
}
}
| // For ai-agent bubbles, build and inject credential pool when | ||
| // multiple user credentials exist for the same credential type | ||
| if (bubble.bubbleName === 'ai-agent' && userCreds.length > 0) { | ||
| const credsByType = new Map< | ||
| CredentialType, | ||
| Array<{ id: number; name: string; value: string }> | ||
| >(); | ||
| for (const uc of userCreds) { | ||
| if ( | ||
| !allCredentialOptions.includes(uc.credentialType) || | ||
| uc.credentialId == null | ||
| ) | ||
| continue; | ||
| if (!credsByType.has(uc.credentialType)) { | ||
| credsByType.set(uc.credentialType, []); | ||
| } | ||
| credsByType.get(uc.credentialType)!.push({ | ||
| id: uc.credentialId, | ||
| name: uc.name ?? `${uc.credentialType} (${uc.credentialId})`, | ||
| value: this.escapeString(uc.secret), | ||
| }); | ||
| } | ||
| // Only inject pool if at least one type has multiple credentials | ||
| const hasMultiple = Array.from(credsByType.values()).some( | ||
| (entries) => entries.length > 1 | ||
| ); | ||
| if (hasMultiple) { | ||
| const pool: Record< | ||
| string, | ||
| Array<{ id: number; name: string; value: string }> | ||
| > = {}; | ||
| for (const [credType, entries] of credsByType) { | ||
| if (entries.length > 1) { | ||
| pool[credType] = entries; | ||
| } | ||
| } | ||
| this.injectCredentialPoolIntoBubble(bubble, pool); | ||
| } | ||
| } |
There was a problem hiding this comment.
Injecting an additional credentialPool parameter will break bubbles instantiated with a single variable first argument (e.g. new AIAgentBubble(params)), because the parameter formatter only has a special-case for spreading the first arg when only credentials is added. With credentialPool present too, it will emit something like { arg0: params, credentialPool: ..., credentials: ... }, which changes the runtime params shape and can make the agent fail schema validation. Consider updating the parameter formatting logic to spread the first-arg variable and then merge both credentials and credentialPool (and any other injected params), or alternatively inject credentialPool into the existing first-arg object rather than as a separate parameter entry.
| if (!seenUserCredTypes.has(userCredType)) { | ||
| seenUserCredTypes.add(userCredType); | ||
| credentialMapping[userCredType] = this.escapeString( | ||
| userCred.secret | ||
| ); | ||
| injectedCredentials[`${bubble.variableId}.${userCredType}`] = { | ||
| isUserCredential: true, | ||
| credentialType: userCredType, | ||
| credentialValue: this.maskCredential(userCred.secret), | ||
| }; |
There was a problem hiding this comment.
Credential secrets are being passed through escapeString() and then later serialized into an object literal via JSON.stringify/parameter formatting. That double-escaping will corrupt secrets that contain backslashes or newlines (e.g. multi-line keys, JSON creds), because \n will roundtrip as the two-character sequence instead of an actual newline. Prefer storing the raw secret in the injected parameter object and relying on JSON.stringify/formatter escaping once.
| credsByType.get(uc.credentialType)!.push({ | ||
| id: uc.credentialId, | ||
| name: uc.name ?? `${uc.credentialType} (${uc.credentialId})`, | ||
| value: this.escapeString(uc.secret), |
There was a problem hiding this comment.
The credential pool entries are also pre-escaped via escapeString(uc.secret) before being serialized into an object literal, which can double-escape and corrupt secrets with backslashes/newlines. Use the raw secret for value and let the object serializer handle escaping once.
| value: this.escapeString(uc.secret), | |
| value: uc.secret, |
| // Inject user credentials (first credential of each type wins as the "default") | ||
| // User credentials always override system credentials. | ||
| // Within a multi-credential pool, the first entry wins (pool order | ||
| // is preserved by CredentialHelper.getUserCredentials()). | ||
| const seenUserCredTypes = new Set<string>(); | ||
| for (const userCred of userCreds) { | ||
| const userCredType = userCred.credentialType; | ||
|
|
||
| if (allCredentialOptions.includes(userCredType)) { | ||
| credentialMapping[userCredType] = this.escapeString( | ||
| userCred.secret | ||
| ); | ||
| injectedCredentials[`${bubble.variableId}.${userCredType}`] = { | ||
| isUserCredential: true, | ||
| credentialType: userCredType, | ||
| credentialValue: this.maskCredential(userCred.secret), | ||
| }; | ||
| if (!seenUserCredTypes.has(userCredType)) { | ||
| seenUserCredTypes.add(userCredType); | ||
| credentialMapping[userCredType] = this.escapeString( | ||
| userCred.secret | ||
| ); | ||
| injectedCredentials[`${bubble.variableId}.${userCredType}`] = { | ||
| isUserCredential: true, | ||
| credentialType: userCredType, | ||
| credentialValue: this.maskCredential(userCred.secret), | ||
| }; | ||
| } |
There was a problem hiding this comment.
This code now depends on the order of userCreds ("first credential of each type wins" and pool order). However, CredentialHelper.getUserCredentials() does not enforce a deterministic ordering for decrypted credentials from the DB, so the selected default (and pool ordering) may vary between runs. Consider explicitly sorting userCreds (e.g., by credentialId or a stable user-defined order) before applying the "first wins" logic so defaults are consistent.
| // Resolve credential overrides from the credential pool | ||
| const subAgentCredentials = this.params.credentials | ||
| ? { ...this.params.credentials } | ||
| : undefined; | ||
|
|
||
| if ( | ||
| credentialOverrides && | ||
| this.params.credentialPool && | ||
| subAgentCredentials | ||
| ) { | ||
| for (const [credType, credSelector] of Object.entries( | ||
| credentialOverrides | ||
| )) { | ||
| const pool = | ||
| this.params.credentialPool[credType as CredentialType]; | ||
| if (!pool) continue; | ||
|
|
||
| // Match by name first (string), fall back to ID (number) | ||
| let match: (typeof pool)[number] | undefined; | ||
| if (typeof credSelector === 'string') { | ||
| const sel = credSelector.toLowerCase(); | ||
| // Exact match first, then substring (handles "email (label)" format) | ||
| match = pool.find((c) => c.name.toLowerCase() === sel); | ||
| if (!match) { | ||
| match = pool.find((c) => | ||
| c.name.toLowerCase().includes(sel) | ||
| ); | ||
| } | ||
| } | ||
| if (!match && typeof credSelector === 'number') { | ||
| match = pool.find((c) => c.id === credSelector); | ||
| } | ||
| // Also try parsing string as number for ID fallback | ||
| if (!match && typeof credSelector === 'string') { | ||
| const asNum = Number(credSelector); | ||
| if (!Number.isNaN(asNum)) { | ||
| match = pool.find((c) => c.id === asNum); | ||
| } | ||
| } | ||
|
|
||
| if (match) { | ||
| subAgentCredentials[credType as CredentialType] = match.value; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
New delegation credential selection logic isn’t covered by unit tests. Since AIAgentBubble already has a credential-system test suite, add tests that exercise use-capability credential overrides (exact name match, substring match, numeric ID fallback, and the behavior when a selector doesn’t match) to prevent regressions.
…gent defaults - Switch use-capability credential parameter from numeric IDs to account names - Match credentials by name (case-insensitive, substring), with numeric ID fallback - Update capability prompt to list credential names without IDs - Change subagent default model to Gemini 3 Flash with low thinking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zhubzy
force-pushed
the
feat/multi-credential
branch
from
ee182dc to
8e83000
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts`:
- Around line 1722-1732: The string selector logic allows an empty string to
match every credential because "".includes("") is true; update the credSelector
handling in the block around variables credSelector, sel, pool, and match so you
trim and lower-case the selector (const sel = credSelector.trim().toLowerCase())
and only perform exact/substr matching when sel.length > 0; if sel is empty skip
the name-based search (leaving match undefined so the code can fall back to
numeric ID or other logic).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d58f83e-0c62-49eb-a8ec-9e0fd4d6d2bb
📒 Files selected for processing (2)
packages/bubble-core/src/bubbles/service-bubble/ai-agent.tspackages/bubble-core/src/bubbles/service-bubble/capability-pipeline.ts
| // Match by name first (string), fall back to ID (number) | ||
| let match: (typeof pool)[number] | undefined; | ||
| if (typeof credSelector === 'string') { | ||
| const sel = credSelector.toLowerCase(); | ||
| // Exact match first, then substring (handles "email (label)" format) | ||
| match = pool.find((c) => c.name.toLowerCase() === sel); | ||
| if (!match) { | ||
| match = pool.find((c) => | ||
| c.name.toLowerCase().includes(sel) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Empty string selector matches first credential unintentionally.
If the AI passes an empty string as credSelector, "".toLowerCase().includes("") evaluates to true for any credential name, causing the first credential in the pool to be selected. This could lead to unexpected behavior if the LLM hallucinates an empty credential reference.
🛡️ Proposed guard against empty selectors
if (typeof credSelector === 'string') {
const sel = credSelector.toLowerCase();
+ if (!sel) continue; // Skip empty selectors
// Exact match first, then substring (handles "email (label)" format)
match = pool.find((c) => c.name.toLowerCase() === sel);
if (!match) {
match = pool.find((c) =>
c.name.toLowerCase().includes(sel)
);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts` around lines
1722 - 1732, The string selector logic allows an empty string to match every
credential because "".includes("") is true; update the credSelector handling in
the block around variables credSelector, sel, pool, and match so you trim and
lower-case the selector (const sel = credSelector.trim().toLowerCase()) and only
perform exact/substr matching when sel.length > 0; if sel is empty skip the
name-based search (leaving match undefined so the code can fall back to numeric
ID or other logic).
Summary
use-capabilitycredential parameter from numeric IDs to account names for more reliable AI selectionTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements