fix(capabilities): enable dynamic capability discovery and mid-conversation delegation#330
fix(capabilities): enable dynamic capability discovery and mid-conversation delegation#330
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughBumped several package versions to 0.1.246 and relaxed/adjusted capability delegation logic: Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
Deploying bubblelab-documentation with
|
| Latest commit: |
55445eb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe47b87e.bubblelab-documentation.pages.dev |
| Branch Preview URL: | https://feat-multi-credential.bubblelab-documentation.pages.dev |
|
Companion PR: bubblelabai/BubbleLab-Pro#179 |
…sation delegation
zhubzy
force-pushed
the
feat/multi-credential
branch
from
55445eb to
738d479
Compare
There was a problem hiding this comment.
Pull request overview
Enables more flexible capability delegation by allowing dynamically-added capabilities (mid-conversation) to be targeted by use-capability, and updates capability prompting to encourage discovery via get_capabilities before claiming a task is unsupported.
Changes:
- Relaxed
use-capabilityinput validation (capabilityIdnow accepts any string) and added a fallbackcapConfigfor dynamically-added capabilities. - Updated credential selection to support choosing accounts by name (in addition to ID), and adjusted capability summaries accordingly.
- Adjusted sub-agent default reasoning effort and broadened delegation rules in the multi-capability system prompt.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bubble-core/src/bubbles/service-bubble/ai-agent.ts | Updates use-capability schema/behavior (dynamic IDs, credential selectors by name/ID, output shape). |
| packages/bubble-core/src/bubbles/service-bubble/capability-pipeline.ts | Updates delegation/system prompt guidance and sub-agent default reasoning effort. |
| packages/create-bubblelab-app/package.json | Bumps create-bubblelab-app version to 0.1.246. |
| packages/bubble-core/package.json | Bumps @bubblelab/bubble-core version to 0.1.246. |
| packages/bubble-runtime/package.json | Bumps @bubblelab/bubble-runtime version to 0.1.246. |
| packages/bubble-shared-schemas/package.json | Bumps @bubblelab/shared-schemas version to 0.1.246. |
| packages/bubble-scope-manager/package.json | Bumps @bubblelab/ts-scope-manager version to 0.1.246. |
| packages/create-bubblelab-app/templates/basic/package.json | Updates template dependencies to ^0.1.246. |
| packages/create-bubblelab-app/templates/reddit-scraper/package.json | Updates template dependencies to ^0.1.246. |
| // Single-cap: sub-agents (multi-cap delegation) default to Gemini 3 Flash + low thinking | ||
| const isSubAgent = params.name?.startsWith('Capability Agent: '); | ||
| if (isSubAgent) { | ||
| params.model.model = | ||
| RECOMMENDED_MODELS.GOOGLE_FLAGSHIP as typeof params.model.model; | ||
| params.model.reasoningEffort = 'low'; |
There was a problem hiding this comment.
Changing the sub-agent default from reasoningEffort = undefined to 'low' will enable provider “thinking”/reasoning mode by default (see model client construction in ai-agent.ts), which can materially increase latency and cost for every delegated capability call. If the goal is still “no thinking” for sub-agents, keep this undefined (or gate it behind an explicit config). If the goal is to improve reliability, consider documenting/measuring the cost impact or limiting it to specific providers/models.
| @@ -1684,4 +1684,4 @@ | |||
| credentials: z | |||
| .record( | |||
| z.nativeEnum(CredentialType), | |||
| z.union([z.string(), z.number()]) | |||
| @@ -1697,10 +1697,14 @@ | |||
| const credentialOverrides = input.credentials as | |||
| | Record<string, string | number> | |||
| | undefined; | |||
| const capConfig = caps.find((c) => c.id === capabilityId); | |||
| const capConfig = caps.find((c) => c.id === capabilityId) ?? { | |||
| id: capabilityId, | |||
| }; | |||
| const capDef = getCapability(capabilityId); | |||
| if (!capConfig || !capDef) | |||
| return { error: `Capability "${capabilityId}" not found` }; | |||
| if (!capDef) | |||
| return { | |||
| error: `Capability "${capabilityId}" not found. Available: ${capIds.join(', ')}`, | |||
| }; | |||
There was a problem hiding this comment.
The use-capability tool now accepts any string capabilityId and has additional behavior (fallback capConfig, name-based credential selection, revised error message). There doesn’t appear to be automated test coverage exercising these new paths (dynamic capability IDs, unknown IDs, credential selection by name/ID). Adding a focused test would help prevent regressions, especially around delegation correctness and error messages.
| credentials: z | ||
| .record( | ||
| z.nativeEnum(CredentialType), | ||
| z.union([z.string(), z.number()]) |
There was a problem hiding this comment.
credentials schema now allows any number, including non-integers. Since credential IDs are integers and matching later uses strict equality against c.id, non-integer inputs will never match and will silently fall back to the default credential. Consider constraining the number branch to an int (and/or rejecting other number types) to keep validation aligned with runtime behavior.
| z.union([z.string(), z.number().int()]) |
| // 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); |
There was a problem hiding this comment.
The credential selection logic falls back to substring matching (includes(sel)) when a string selector isn’t an exact match. This can select the wrong account when multiple credential names share a common substring (e.g., multiple emails/domains), which is risky because it may run actions under an unintended account. Prefer exact (case-insensitive) match only; if there’s no exact match (or there are multiple candidates), return an error listing available account names for that credential type so the caller can disambiguate.
| // Match by name first (string), or by ID (number/stringified number). | |
| let match: (typeof pool)[number] | undefined; | |
| if (typeof credSelector === 'string') { | |
| const sel = credSelector.toLowerCase(); | |
| const exactNameMatches = pool.filter( | |
| (c) => c.name.toLowerCase() === sel | |
| ); | |
| if (exactNameMatches.length === 1) { | |
| match = exactNameMatches[0]; | |
| } else if (exactNameMatches.length > 1) { | |
| const matchingNames = exactNameMatches | |
| .map((c) => c.name) | |
| .join(', '); | |
| throw new Error( | |
| `Multiple ${credType} credentials match override "${credSelector}". Matching names: ${matchingNames}` | |
| ); | |
| } else { | |
| // No exact name match; try interpreting the selector as an ID. | |
| const asNum = Number(credSelector); | |
| if (!Number.isNaN(asNum)) { | |
| const idMatches = pool.filter((c) => c.id === asNum); | |
| if (idMatches.length === 1) { | |
| match = idMatches[0]; | |
| } else if (idMatches.length > 1) { | |
| const matchingNames = idMatches.map((c) => c.name).join(', '); | |
| throw new Error( | |
| `Multiple ${credType} credentials have ID ${asNum}. Matching names: ${matchingNames}` | |
| ); | |
| } else { | |
| const availableNames = | |
| pool.map((c) => c.name).join(', ') || '<none>'; | |
| throw new Error( | |
| `No ${credType} credential matches override "${credSelector}". Available names: ${availableNames}` | |
| ); | |
| } | |
| } else { | |
| const availableNames = | |
| pool.map((c) => c.name).join(', ') || '<none>'; | |
| throw new Error( | |
| `No ${credType} credential matches override "${credSelector}". Available names: ${availableNames}` | |
| ); | |
| } | |
| } | |
| } else if (typeof credSelector === 'number') { | |
| const idMatches = pool.filter((c) => c.id === credSelector); | |
| if (idMatches.length === 1) { | |
| match = idMatches[0]; | |
| } else if (idMatches.length > 1) { | |
| const matchingNames = idMatches.map((c) => c.name).join(', '); | |
| throw new Error( | |
| `Multiple ${credType} credentials have ID ${credSelector}. Matching names: ${matchingNames}` | |
| ); | |
| } else { | |
| const availableNames = | |
| pool.map((c) => c.name).join(', ') || '<none>'; | |
| throw new Error( | |
| `No ${credType} credential found with ID ${credSelector}. Available names: ${availableNames}` | |
| ); |
Summary
use-capabilitytool schema fromz.enum(capIds)toz.string()so dynamically-added capabilities can be delegated to mid-conversationcapConfigconstruction for capabilities added viamanage_capabilitythat aren't in the startup capability listget_capabilitiescheck before telling users a task can't be doneContext
Pearl accuracy testing (0/10 → 10/10) revealed three cascading issues when a user asks for a task that requires an unattached capability:
get_capabilitiesto discover available capabilities (fixed in capability-pipeline.ts)use-capability'sz.enumrejected the dynamically-added capability ID (fixed here)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Improvements