feat(provider): add Droid SDK provider#82
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a first-party Droid provider: contracts and settings, server driver and adapter (session/turn lifecycle, SDK mappings, runtime-event translation, attachment resolver), model discovery/status checks, comprehensive tests, web UI integration (icons, provider-aware runtime-mode), and orchestrator-harness receipt/rollback updates. ChangesDroid Provider Implementation and Integration
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
🚥 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 There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
github-actions
Bot
added
vouch:trusted
aaditagrawal
force-pushed
the
sync/upstream-main-2026-05-16
branch
from
7bd8081 to
e441260
Compare
aaditagrawal
force-pushed
the
integrate/droid-sdk-provider-2026-05-16
branch
from
276ed03 to
f4d27b1
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: 5
🧹 Nitpick comments (1)
apps/web/src/components/settings/providerDriverMeta.ts (1)
78-84: ⚡ Quick winConsider a more user-friendly badge label.
The
badgeLabel: "WIP"is developer-focused terminology. For consistency with the Cursor provider and a more polished user experience, consider using"Early Access"or"Preview"instead of"WIP".Suggested refinement
{ value: ProviderDriverKind.make("droid"), label: "Droid", icon: DroidIcon, - badgeLabel: "WIP", + badgeLabel: "Early Access", settingsSchema: DroidSettings, },🤖 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 `@apps/web/src/components/settings/providerDriverMeta.ts` around lines 78 - 84, The badgeLabel for the Droid provider is developer-focused ("WIP"); update the entry where ProviderDriverKind.make("droid") is configured (the object with label "Droid", icon DroidIcon, settingsSchema DroidSettings) to use a user-facing badgeLabel such as "Early Access" or "Preview" instead of "WIP" to match the Cursor provider and improve UX.
🤖 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 `@apps/server/integration/OrchestrationEngineHarness.integration.ts`:
- Around line 570-578: The current shutdown pipeline uses Effect.timeout on
shutdown and then catchCause which treats any failure (including real
Scope.close/runtime.dispose errors) as a timeout; modify the pipeline so timeout
handling is explicit: replace Effect.timeout("5 seconds") + catchCause with
either Effect.timeoutOption("5 seconds") and log a timeout only when it returns
a None, or keep Effect.timeout("5 seconds") but in the Effect.catchCause handler
inspect the cause (using Cause.isFailType(cause) && cause.error instanceof
TimeoutException) and only log the "disposal timed out" message for
TimeoutException while rethrowing or logging other causes appropriately; target
the shutdown pipeline where shutdown, Effect.timeout, Effect.catchCause, and
TimeoutException are used.
- Around line 505-537: The durable fallback in
readMatchingReceipt/waitForReceipt cannot reconstruct
checkpoint.baseline.captured and may miss it if emitted before receiptHistory is
attached; fix by subscribing/attaching receiptHistory before starting the
reactor (i.e., ensure receiptHistory is initialized and its Ref is populated
prior to calling reactor.start or any reactor startup helper), or alternatively
persist/emmit checkpoint.baseline.captured into the durable event stream (so
engine.readEvents can replay it) and extend the durableReceipts mapping in
readMatchingReceipt to reconstruct that receipt type.
In `@apps/server/src/provider/droid/DroidRuntimeEvents.ts`:
- Around line 121-126: The CreateMessage completion uses firstTextBlock.id which
can differ from the `${message.messageId}-${index}` keys used earlier, causing
duplicate completions; change the completedItemId calculation in
DroidRuntimeEvents (replace the logic around
firstTextIndex/firstTextBlock/completedItemId) to always use the adapter's
index-based key when a text block exists — i.e., if firstTextIndex >= 0 set
completedItemId = `${message.messageId}-${firstTextIndex}`, otherwise fall back
to message.messageId — and keep activeCompletedAssistantItems checks using that
same completedItemId.
In `@apps/server/src/provider/Layers/DroidAdapter.ts`:
- Around line 264-299: The sendTurn implementation may start a second turn for
the same thread and then overwrite shared context fields (context.activeAbort,
context.activeAssistantItems, context.activeThinkingItems,
context.activeCompletedAssistantItems, activeTokenUsageBaseline), so before
mutating the context in sendTurn (function sendTurn) check the session's current
status/activeTurnId (from sessions.get(input.threadId) / context.status) and, if
a turn is already running, immediately return a ProviderAdapterValidationError
(same shape used elsewhere) rejecting concurrent sendTurn calls for that thread;
perform this guard before creating the AbortController, generating turnId,
pushing to context.turns, or calling updateDroidContextSession to ensure shared
state is not clobbered.
In `@apps/web/src/components/chat/ChatComposer.tsx`:
- Around line 163-166: The code dereferences runtimeModeOption (from
getRuntimeModeConfig(props.provider)) without checking it, which can crash if
props.runtimeMode is stale; update ChatComposer to first check that
runtimeModeOption is defined before accessing RuntimeModeIcon or other
properties (e.g., wrap const RuntimeModeIcon = runtimeModeOption?.icon or
early-return/render a safe fallback when runtimeModeOption is undefined), and
apply the same guard around the later usages (lines ~197-209) where
runtimeModeOption or its properties are read so provider switches cannot cause
undefined dereference.
---
Nitpick comments:
In `@apps/web/src/components/settings/providerDriverMeta.ts`:
- Around line 78-84: The badgeLabel for the Droid provider is developer-focused
("WIP"); update the entry where ProviderDriverKind.make("droid") is configured
(the object with label "Droid", icon DroidIcon, settingsSchema DroidSettings) to
use a user-facing badgeLabel such as "Early Access" or "Preview" instead of
"WIP" to match the Cursor provider and improve UX.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea79df35-aa1b-438d-9a15-0e7f5b7db441
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
apps/server/integration/OrchestrationEngineHarness.integration.tsapps/server/package.jsonapps/server/src/provider/Drivers/DroidDriver.tsapps/server/src/provider/Layers/CodexSessionRuntime.tsapps/server/src/provider/Layers/DroidAdapter.test.tsapps/server/src/provider/Layers/DroidAdapter.tsapps/server/src/provider/Layers/DroidProvider.test.tsapps/server/src/provider/Layers/DroidProvider.tsapps/server/src/provider/Layers/ProviderRegistry.test.tsapps/server/src/provider/builtInDrivers.tsapps/server/src/provider/droid/DroidAdapterTypes.tsapps/server/src/provider/droid/DroidAttachmentResolver.tsapps/server/src/provider/droid/DroidRuntimeEvents.tsapps/server/src/provider/droid/DroidSdkMappings.tsapps/web/src/components/ChatView.tsxapps/web/src/components/Icons.tsxapps/web/src/components/KeybindingsToast.browser.tsxapps/web/src/components/chat/ChatComposer.tsxapps/web/src/components/chat/CompactComposerControlsMenu.browser.tsxapps/web/src/components/chat/CompactComposerControlsMenu.tsxapps/web/src/components/chat/MessagesTimeline.test.tsxapps/web/src/components/chat/providerIconUtils.tsapps/web/src/components/chat/runtimeModePresentation.test.tsapps/web/src/components/chat/runtimeModePresentation.tsapps/web/src/components/settings/providerDriverMeta.tsapps/web/src/session-logic.test.tsapps/web/src/session-logic.tspackages/contracts/src/model.tspackages/contracts/src/orchestration.tspackages/contracts/src/providerRuntime.tspackages/contracts/src/settings.ts
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/server/src/provider/Layers/DroidAdapter.test.ts`:
- Line 162: The test is calling Array.from(...) on the result of
Effect.timeout(...) which yields an Option wrapping the chunk; update each
occurrence (e.g., where events is created from yield*
Fiber.join(eventsFiber).pipe(Effect.timeout("2 seconds"))) to unwrap the Option
first by checking that the returned value has _tag === "Some" (or using the
Option match), then call Array.from(...) on the inner chunk (e.g., the
Some.value) to get the actual events; apply the same change to all similar uses
of Fiber.join(...).pipe(Effect.timeout(...)) in this file so tests inspect the
collected events rather than a single Some wrapper.
In `@apps/web/src/components/ChatView.tsx`:
- Around line 1261-1286: providerConfigLoaded is too broad and can be true for a
fallback config, causing resolveSelectableProvider to pick a wrong provider and
persist a downgraded runtimeMode; change the gating so normalization only
happens when the active provider set can actually resolve the target: compute
whether resolveSelectableProvider(providerStatuses, requestedProvider ??
ProviderDriverKind.make("codex")) equals the intended provider (or, if
lockedProvider is set, equals lockedProvider) and only then derive
selectedProvider/runtimeMode and call
setComposerDraftRuntimeMode/setDraftThreadContext; otherwise keep rawRuntimeMode
and avoid writing until the true environment config can resolve the provider
(use the existing symbols requestedProvider, lockedProvider,
resolveSelectableProvider, providerStatuses, selectedProvider, runtimeMode,
rawRuntimeMode, setComposerDraftRuntimeMode, setDraftThreadContext,
isLocalDraftThread).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 909cee02-b621-4ef6-acf4-2734c5c2c2e9
📒 Files selected for processing (9)
apps/server/src/orchestration/Layers/CheckpointReactor.test.tsapps/server/src/orchestration/Layers/CheckpointReactor.tsapps/server/src/provider/Layers/DroidAdapter.test.tsapps/server/src/provider/Layers/DroidAdapter.tsapps/server/src/provider/Layers/DroidProvider.test.tsapps/server/src/provider/Layers/DroidProvider.tsapps/server/src/provider/droid/DroidAdapterTypes.tsapps/server/src/provider/droid/DroidRuntimeEvents.tsapps/web/src/components/ChatView.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/server/src/provider/droid/DroidAdapterTypes.ts
- apps/server/src/provider/droid/DroidRuntimeEvents.ts
- apps/server/src/provider/Layers/DroidProvider.ts
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
What Changed
Why
Droid needs to behave like a first-class provider without destabilizing existing providers or the local fork UX. The review fixes prevent shared Droid session state from being clobbered by overlapping turns, keep composer controls safe during provider switches, and make integration harness cleanup/debuggability more reliable.
Validation
Summary by CodeRabbit
New Features
Behavior Changes / Bug Fixes