Add Devin provider integration - #47
Merged
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 993a0d6f81055c8545dc01c4122b9a433af9c8ad and 335e69b. 📒 Files selected for processing (41)
📝 WalkthroughWalkthroughThis pull request adds comprehensive Devin AI provider integration to the platform, including service definitions, an Effect-based adapter layer with session/message management and polling, an HTTP API client, provider configuration resolution, test coverage, and frontend UI components for provider selection and model discovery. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Frontend
participant Adapter as DevinAdapter<br/>(Session Manager)
participant LocalCtx as Local Session<br/>Context (Map)
participant DevinAPI as Devin API<br/>Client
participant RemoteAPI as Devin Remote<br/>API Endpoint
Client->>Adapter: sendTurn(text, attachments)
activate Adapter
alt First Turn (No Session)
Adapter->>LocalCtx: Initialize session context
Adapter->>DevinAPI: createAttachment(file)
DevinAPI->>RemoteAPI: POST /v3/.../attachments
RemoteAPI-->>DevinAPI: attachment ID
Adapter->>DevinAPI: createSession(attachments, text)
DevinAPI->>RemoteAPI: POST /v3/.../sessions
RemoteAPI-->>DevinAPI: session summary
Adapter->>LocalCtx: Store remote binding
Adapter->>Client: emit thread.started
else Subsequent Turns
Adapter->>DevinAPI: sendSessionMessage(text)
DevinAPI->>RemoteAPI: POST /v3/.../messages
RemoteAPI-->>DevinAPI: void
end
Adapter->>Adapter: Start polling fiber
Adapter-->>Client: emit turn.started
deactivate Adapter
par Polling Loop
loop Every poll interval
Adapter->>DevinAPI: getSession()
DevinAPI->>RemoteAPI: GET /v3/.../sessions/{id}
RemoteAPI-->>DevinAPI: session state
Adapter->>DevinAPI: listSessionMessages(pagination)
DevinAPI->>RemoteAPI: GET /v3/.../messages
RemoteAPI-->>DevinAPI: messages page
Adapter->>Adapter: Deduplicate by event_id
Adapter->>Client: emit item.started,<br/>content.delta,<br/>item.completed
Adapter->>Adapter: Update local state
Adapter->>Client: emit session.state.changed
alt Session Complete
Adapter->>Client: emit turn.completed
Adapter->>Adapter: Stop polling
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
github-actions
Bot
added
size:XXL
vouch:trusted
ranvier2d2
force-pushed
the
feat/devin-provider-integration
branch
from
March 30, 2026 23:45
993a0d6 to
661aa98
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: 6
🧹 Nitpick comments (3)
apps/server/src/provider/providerSnapshot.ts (1)
87-95: Hoist normalized-provider set to module scope.
knownProvidersis recreated on each call. Hoisting it reduces repeated allocations and keeps intent centralized.♻️ Suggested refactor
+const NORMALIZED_MODEL_PROVIDERS = new Set<ProviderKind>([ + "codex", + "claudeAgent", + "cursor", + "opencode", +]); + export function providerModelsFromSettings( builtInModels: ReadonlyArray<ServerProviderModel>, provider: ServerProvider["provider"], customModels: ReadonlyArray<string>, ): ReadonlyArray<ServerProviderModel> { - const knownProviders = new Set<ProviderKind>(["codex", "claudeAgent", "cursor", "opencode"]); const resolvedBuiltInModels = [...builtInModels]; const seen = new Set(resolvedBuiltInModels.map((model) => model.slug)); const customEntries: ServerProviderModel[] = []; for (const candidate of customModels) { - const normalized = knownProviders.has(provider as ProviderKind) + const normalized = NORMALIZED_MODEL_PROVIDERS.has(provider as ProviderKind) ? normalizeModelSlug(candidate, provider as ProviderKind) : (nonEmptyTrimmed(candidate) ?? null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/provider/providerSnapshot.ts` around lines 87 - 95, The Set named knownProviders is being recreated on every call; hoist it to module scope by declaring a single constant Set<ProviderKind> (e.g., knownProviders) at the top-level of the module so it is allocated once and reused; update any references in providerSnapshot.ts that currently use the local knownProviders (the loop using normalizeModelSlug(candidate, provider as ProviderKind) and the conditional knownProviders.has(provider as ProviderKind)) to use the module-scoped constant and remove the local declaration to avoid redundant allocations and centralize intent.apps/web/src/routes/_chat.settings.tsx (1)
81-136: Centralize the provider custom-model metadata.
supportsCustomModels, placeholders, and the fallback copy now live partly here and partly inapps/web/src/modelSelection.ts, and this branch still hard-codes Devin-specific text. Pull the shared bits into one exported config so the picker and settings page can’t drift the next time a provider changes.Also applies to: 1286-1326
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_chat.settings.tsx` around lines 81 - 136, The provider custom-model metadata is duplicated between PROVIDER_SETTINGS (type InstallProviderSettings and the array) and modelSelection.ts; centralize supportsCustomModels, customModelPlaceholder and any fallback/harnessDescription text into one exported shared config so both the settings page and the model picker import the same source. Create a single exported ProviderModelConfig (or similar) that contains provider keys (e.g., "codex","claudeAgent","cursor","opencode","devin") with supportsCustomModels, customModelPlaceholder and fallback copy, replace the duplicated fields in PROVIDER_SETTINGS with references/imports to that shared export, and update modelSelection.ts to import the same config instead of hard-coding Devin-specific text. Ensure unique symbols to change are InstallProviderSettings, PROVIDER_SETTINGS, and the usages in modelSelection.ts.apps/server/src/provider/Layers/DevinAdapter.ts (1)
51-63: UnboundedseenMessageEventIdsSet may cause memory growth in long-running sessions.The
seenMessageEventIdsSet grows indefinitely as messages are received. For long-running Devin sessions with many messages, this could lead to significant memory consumption.Consider implementing a bounded cache (e.g., LRU with a size limit) or periodically pruning old entries based on
lastMessageCursor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/provider/Layers/DevinAdapter.ts` around lines 51 - 63, The seenMessageEventIds Set in DevinSessionContext is unbounded and can grow without limit; replace it with a bounded cache (e.g., an LRU or size-limited set) and evict old IDs as new ones are added to prevent memory growth. Concretely, change DevinSessionContext.seenMessageEventIds from Set<string> to a bounded structure (e.g., BoundedEventIdCache or LRUCache with a fixed capacity) and update any code that calls add/has/delete on seenMessageEventIds to use the new API; additionally, prune or compact the cache when lastMessageCursor is advanced (or periodically) to remove IDs older than the cursor. Ensure the new cache preserves O(1) membership checks used by message-processing code and choose a sensible default capacity (or make it configurable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/provider/devinApi.ts`:
- Around line 233-236: The call to options.apiKey.trim() in createDevinApiClient
can throw if apiKey is undefined/null; update createDevinApiClient to
defensively validate options.apiKey (from DevinApiClientOptions) before
trimming: either throw a clear Error when missing/invalid or coerce safely
(e.g., ensure it's a string via String(options.apiKey) or use a helper like
trimString) and then call .trim(); adjust the local variable apiKey assignment
accordingly so the function never calls .trim() on null/undefined.
In `@apps/server/src/provider/Layers/DevinAdapter.ts`:
- Around line 250-281: The getApiClient Effect currently assumes
settings.providers.devin and settings.orgId exist and calls .trim() directly;
guard these accesses by first checking that settings.providers and
settings.providers.devin are present and that settings.providers.devin.orgId is
a non-empty string before calling .trim(), and return Effect.fail(...) with
toValidationError("DevinAdapter.config", "...") when missing; also ensure the
apiKey extraction uses safe optional chaining/trim (e.g., check process.env keys
exist before trimming) so you only call createDevinApiClient when both the
provider settings and a valid apiKey/orgId are confirmed.
- Around line 542-577: The polling loop is being started outside the Effect
runtime with Effect.runFork(loop), so the forked fiber won't inherit services
like ServerSettingsService used by pollOnce; change the fork to run inside the
Effect.gen runtime and assign the returned Fiber to context.pollFiber by
yielding a runtime fork (e.g., replace context.pollFiber = Effect.runFork(loop)
with context.pollFiber = yield* Effect.forkDaemon(loop) or context.pollFiber =
yield* Effect.forkScoped(loop) inside the generator), ensuring the loop (defined
in ensurePolling) inherits the parent runtime and can access
getApiClient()/ServerSettingsService; also ensure the chosen fork function is
imported.
In `@apps/web/src/hooks/useSettings.ts`:
- Around line 155-159: The migration currently checks Schema.is(ModelSelection)
for legacySettings.textGenerationModelSelection but then filters providers to a
hardcoded allowlist ("codex" | "claudeAgent" | "devin"), which drops valid
providers; update the logic in useSettings by removing the restrictive provider
check and instead assign legacySettings.textGenerationModelSelection to
patch.textGenerationModelSelection when Schema.is(ModelSelection) passes (or
validate against the ModelSelection schema/enumeration dynamically), ensuring
references to Schema.is(ModelSelection),
legacySettings.textGenerationModelSelection, and
patch.textGenerationModelSelection are used to locate and change the code.
In `@packages/contracts/src/orchestration.ts`:
- Around line 79-91: DevinModelSelection currently allows any non-empty string
which is too permissive; change the schema for DevinModelSelection to use
Schema.Literal("devin-default") instead of TrimmedNonEmptyString and update the
corresponding ModelSelection union to reflect that literal; also mirror the same
tightening in ModelSelectionPatch (replace any Devin-related optional/patch
schema field allowing arbitrary strings with the literal "devin-default") so
only the supported devin-default slug is accepted across contracts.
In `@packages/contracts/src/settings.ts`:
- Around line 80-85: DevinSettings currently exposes customModels
(Schema.Array(Schema.String)) which creates a persisted, unusable field
providers.devin.customModels; remove that field from the public contract by
deleting the customModels entry from the DevinSettings Schema (and any duplicate
definitions around lines referenced as also applying) so the schema no longer
accepts or serializes customModels, and update any related patch/validation code
that references DevinSettings.customModels to avoid accepting or emitting that
property; ensure Schema.Struct only contains enabled, orgId and baseUrl to
prevent the wire field from being created.
---
Nitpick comments:
In `@apps/server/src/provider/Layers/DevinAdapter.ts`:
- Around line 51-63: The seenMessageEventIds Set in DevinSessionContext is
unbounded and can grow without limit; replace it with a bounded cache (e.g., an
LRU or size-limited set) and evict old IDs as new ones are added to prevent
memory growth. Concretely, change DevinSessionContext.seenMessageEventIds from
Set<string> to a bounded structure (e.g., BoundedEventIdCache or LRUCache with a
fixed capacity) and update any code that calls add/has/delete on
seenMessageEventIds to use the new API; additionally, prune or compact the cache
when lastMessageCursor is advanced (or periodically) to remove IDs older than
the cursor. Ensure the new cache preserves O(1) membership checks used by
message-processing code and choose a sensible default capacity (or make it
configurable).
In `@apps/server/src/provider/providerSnapshot.ts`:
- Around line 87-95: The Set named knownProviders is being recreated on every
call; hoist it to module scope by declaring a single constant Set<ProviderKind>
(e.g., knownProviders) at the top-level of the module so it is allocated once
and reused; update any references in providerSnapshot.ts that currently use the
local knownProviders (the loop using normalizeModelSlug(candidate, provider as
ProviderKind) and the conditional knownProviders.has(provider as ProviderKind))
to use the module-scoped constant and remove the local declaration to avoid
redundant allocations and centralize intent.
In `@apps/web/src/routes/_chat.settings.tsx`:
- Around line 81-136: The provider custom-model metadata is duplicated between
PROVIDER_SETTINGS (type InstallProviderSettings and the array) and
modelSelection.ts; centralize supportsCustomModels, customModelPlaceholder and
any fallback/harnessDescription text into one exported shared config so both the
settings page and the model picker import the same source. Create a single
exported ProviderModelConfig (or similar) that contains provider keys (e.g.,
"codex","claudeAgent","cursor","opencode","devin") with supportsCustomModels,
customModelPlaceholder and fallback copy, replace the duplicated fields in
PROVIDER_SETTINGS with references/imports to that shared export, and update
modelSelection.ts to import the same config instead of hard-coding
Devin-specific text. Ensure unique symbols to change are
InstallProviderSettings, PROVIDER_SETTINGS, and the usages in modelSelection.ts.
🪄 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: c30fa8b4-9c1f-46d3-96a6-b385413f1636
📥 Commits
Reviewing files that changed from the base of the PR and between 50939b8 and 993a0d6f81055c8545dc01c4122b9a433af9c8ad.
📒 Files selected for processing (38)
ai_docs/tasks/007_devin_provider_integration.mdapps/server/integration/TestProviderAdapter.integration.tsapps/server/src/provider/Layers/DevinAdapter.test.tsapps/server/src/provider/Layers/DevinAdapter.tsapps/server/src/provider/Layers/DevinProvider.test.tsapps/server/src/provider/Layers/DevinProvider.tsapps/server/src/provider/Layers/ProviderAdapterRegistry.test.tsapps/server/src/provider/Layers/ProviderAdapterRegistry.tsapps/server/src/provider/Layers/ProviderRegistry.tsapps/server/src/provider/Layers/ProviderService.tsapps/server/src/provider/Services/DevinAdapter.tsapps/server/src/provider/Services/DevinProvider.tsapps/server/src/provider/devinApi.test.tsapps/server/src/provider/devinApi.tsapps/server/src/provider/providerCapabilities.tsapps/server/src/provider/providerSnapshot.tsapps/server/src/serverLayers.tsapps/server/src/serverSettings.tsapps/web/src/components/ChatView.tsxapps/web/src/components/Icons.tsxapps/web/src/components/KeybindingsToast.browser.tsxapps/web/src/components/chat/ProviderModelPicker.tsxapps/web/src/components/chat/composerProviderRegistry.test.tsxapps/web/src/components/chat/composerProviderRegistry.tsxapps/web/src/hooks/useSettings.tsapps/web/src/modelSelection.tsapps/web/src/providerModels.test.tsapps/web/src/providerModels.tsapps/web/src/routes/_chat.settings.tsxapps/web/src/session-logic.test.tsapps/web/src/session-logic.tsapps/web/src/store.tspackages/contracts/src/model.tspackages/contracts/src/orchestration.tspackages/contracts/src/provider.test.tspackages/contracts/src/settings.test.tspackages/contracts/src/settings.tspackages/shared/src/model.test.ts
Comment on lines
+542
to
+577
| const ensurePolling = (threadId: ThreadId, context: DevinSessionContext) => | ||
| Effect.sync(() => { | ||
| if (context.pollFiber || !context.remote) { | ||
| return; | ||
| } | ||
| const loop = Effect.gen(function* () { | ||
| let failureDelayMs = 5_000; | ||
| while (!context.stopped && context.remote) { | ||
| const pollResult = yield* Effect.exit(pollOnce(threadId, context)); | ||
| if (pollResult._tag === "Success") { | ||
| failureDelayMs = 5_000; | ||
| if (!pollResult.value.continuePolling || pollResult.value.delayMs === null) { | ||
| break; | ||
| } | ||
| yield* Effect.sleep(pollResult.value.delayMs); | ||
| continue; | ||
| } | ||
|
|
||
| const message = Cause.pretty(pollResult.cause); | ||
| yield* emit( | ||
| makeEvent( | ||
| "runtime.warning", | ||
| { | ||
| message: "Devin polling failed; backing off before retrying.", | ||
| detail: { error: message }, | ||
| }, | ||
| { threadId, turnId: context.activeTurnId }, | ||
| ), | ||
| ); | ||
| yield* Effect.sleep(failureDelayMs); | ||
| failureDelayMs = Math.min(failureDelayMs + 5_000, 30_000); | ||
| } | ||
| context.pollFiber = null; | ||
| }); | ||
| context.pollFiber = Effect.runFork(loop); | ||
| }); |
There was a problem hiding this comment.
Polling fiber runs outside Effect runtime context.
Effect.runFork(loop) on Line 576 executes the polling loop outside the current Effect runtime, which means:
- It won't have access to services from the parent scope unless explicitly provided.
- Errors may not propagate correctly to the parent runtime.
The getApiClient() call inside pollOnce requires ServerSettingsService, which may not be available in the forked fiber.
🔧 Proposed fix to fork within the Effect runtime
Consider using Effect.forkDaemon or Effect.forkScoped within the generator context instead:
const ensurePolling = (threadId: ThreadId, context: DevinSessionContext) =>
- Effect.sync(() => {
+ Effect.gen(function* () {
if (context.pollFiber || !context.remote) {
return;
}
const loop = Effect.gen(function* () {
// ... loop body unchanged
});
- context.pollFiber = Effect.runFork(loop);
+ context.pollFiber = yield* Effect.forkDaemon(loop);
});This ensures the fiber inherits the runtime context including access to ServerSettingsService.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/provider/Layers/DevinAdapter.ts` around lines 542 - 577, The
polling loop is being started outside the Effect runtime with
Effect.runFork(loop), so the forked fiber won't inherit services like
ServerSettingsService used by pollOnce; change the fork to run inside the
Effect.gen runtime and assign the returned Fiber to context.pollFiber by
yielding a runtime fork (e.g., replace context.pollFiber = Effect.runFork(loop)
with context.pollFiber = yield* Effect.forkDaemon(loop) or context.pollFiber =
yield* Effect.forkScoped(loop) inside the generator), ensuring the loop (defined
in ensurePolling) inherits the parent runtime and can access
getApiClient()/ServerSettingsService; also ensure the chosen fork function is
imported.
Run oxfmt on the two files flagged by CI and replace unnecessary Effect.fail wrapping of yieldable TaggedError instances. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Combine chained Effect.provide calls into a single provide with Layer.provideMerge so the Effect language-service plugin does not flag it as a lifecycle issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Validation
Summary by CodeRabbit
New Features
Documentation