Skip to content

feat(devin): add and harden CLI provider support - #9335

Open
soramikan wants to merge 12 commits into
pingdotgg:mainfrom
soramikan:feat/devin-provider
Open

feat(devin): add and harden CLI provider support#9335
soramikan wants to merge 12 commits into
pingdotgg:mainfrom
soramikan:feat/devin-provider

Conversation

@soramikan

@soramikan soramikan commented Sep 3, 2026

Copy link
Copy Markdown

Summary

This PR adds Devin as a first-class T3 Code provider using the existing ACP runtime and provider-instance infrastructure.

  • Implements DevinAdapter, DevinProvider, DevinTextGeneration, and DevinProfile to bridge devin acp (JSON-RPC over stdio).
  • Hardens AcpSessionRuntime with on-demand authentication and retention of startup state-bearing updates.
  • Discovers models through devin models list --format json when ACP initialize does not advertise them, so the provider reaches ready instead of warning.
  • Wires Devin into the web and mobile provider pickers, settings, and chat icon mapping.
  • Adds tests for the Devin driver, adapter, ACP runtime, and provider status.

This is a fresh implementation against current main, informed by the closed #7567 but not cherry-picked from it.

Test plan

  • npx vp run --filter t3 typecheck
  • npx vp run --filter @t3tools/web typecheck
  • npx vp run --filter @t3tools/mobile typecheck
  • npx vp run --filter @t3tools/contracts typecheck
  • npx vp test run apps/server/src/provider/Drivers/DevinDriver.test.ts
  • npx vp test run apps/server/src/provider/Layers/DevinAdapter.test.ts
  • npx vp test run apps/server/src/provider/acp/AcpSessionRuntime.test.ts
  • npx vp test run apps/server/src/provider/Layers/DevinProvider.test.ts
  • npx vp test run apps/server/src/provider/Layers/ProviderRegistry.test.ts
  • npx vp check --fix

Model/harness: SWE-1.7 Max via the T3 Code dev server with Devin CLI 3000.6.12.


Note

Medium Risk
Large new provider path that spawns external CLI processes, handles permission/elicitation flows, and changes shared ACP runtime auth behavior used by Devin (and potentially other ACP providers).

Overview
Adds Devin as an opt-in built-in provider: contracts gain DevinSettings and defaults, DevinDriver registers in BUILT_IN_DRIVERS, and server settings/history treat Devin like other optional drivers (Cursor/Grok/OpenCode).

The server stack spawns devin acp through a new DevinAdapter (sessions, turns, permissions, elicitation, attachments, runtime events) with DevinProfile for DEVIN_HOME/DEVIN_CONFIG and continuation identity. DevinProvider health-checks the CLI, discovers models from ACP initialize or devin models list --format json, and exposes an Early Access snapshot. DevinTextGeneration reuses the same ACP path for commit/PR/branch/title helpers.

AcpSessionRuntime is extended for Devin: optional on-demand authentication (authenticate after a failed prompt), buffering of state-bearing startup updates, tracking of config options and available commands, and safer session-id filtering on updates. DevinAcpSupport centralizes spawn args, permission modes, and in-session model/reasoning selection.

Web and mobile pick up Devin icons, settings metadata, and a “new” provider picker entry. Tests cover the driver, adapter construction, provider status/model parsing, registry listing, and on-demand auth behavior.

Note: rollbackThread is explicitly unsupported for Devin ACP sessions.

Reviewed by Cursor Bugbot for commit afdcc23. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add Devin CLI provider with adapter, settings, and ACP runtime support

  • Adds a full Devin provider stack: DevinDriver, DevinAdapter, DevinProvider status checks, DevinProfile runtime resolution, DevinTextGeneration, and DevinSettings/DevinSettingsPatch schemas in settings.ts and model constants in model.ts
  • DevinAdapter implements ACP-backed session lifecycle, per-thread serialization, permission/elicitation callbacks, attachment handling, runtime event streaming, and turn interrupt/resume
  • DevinProvider probes CLI version, discovers models via ACP initialization then CLI models-list fallback, and merges built-in/custom/discovered models into the provider snapshot
  • AcpSessionRuntime gains on-demand authentication (retry prompt after auth failure), getAvailableCommands, and state-aware update routing that ignores notifications for other session IDs
  • Web and mobile UI add DevinIcon, provider picker entry with a "new" badge, Early Access provider definition, and settings form metadata
  • Risk: DevinAdapter.rollbackThread always returns a validation error because provider-side Devin ACP rollback is unsupported; clients expecting rollback support will fail. discoverDevinModelsViaAcpInitialize has an 8-second timeout and discoverDevinModelsViaModelsList has a 10-second timeout — slow CLI responses yield empty model lists rather than errors.

Macroscope summarized afdcc23.

- AcpSessionRuntime now supports on-demand authentication and retains
  state-bearing session/update notifications (available_commands_update,
  config_option_update, current_mode_update) during session startup,
  replaying them once the session is established.
- AcpRuntimeModel parses the new session update types into typed events.
- contracts/settings adds DevinSettings and DevinSettingsPatch for the
  new driver.
Add coverage for the Devin driver registration, adapter construction,
and ACP on-demand authentication behavior used by the Devin runtime.
Add Devin icon and badge to the web and mobile provider pickers,
settings driver meta, and chat provider icon mapping.
…nitialize omits them

Devin ACP `initialize` does not advertise `modelState`, so the provider
probe fell back to a single "Devin Default" model and a warning.

Add a secondary probe that runs `devin models list --format json`,
parses the family/variant JSON into `ServerProviderModel` entries, and
uses them when ACP does not provide models. Keep the ACP path as the
preferred source so future Devin versions that advertise models in
`initialize` continue to work.

- Add `parseDevinModelsListJson` and `discoverDevinModelsViaModelsList`
- Mark provider `ready` and `auth: authenticated` when CLI list succeeds
- Include the built-in "default" model alongside discovered variants
- Add focused tests covering parser behavior and CLI fallback
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Sep 3, 2026
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
break;
}
case "config_option_update": {
if (Array.isArray(upd.configOptions) && upd.configOptions.length > 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High acp/AcpRuntimeModel.ts:862

An empty config_option_update produces no ConfigOptionsChanged event, so removing the last option leaves stale options in the runtime. Nonempty updates are also later treated by mergeSessionConfigOptions as patches, so omitted option IDs remain stale; emit empty snapshots and replace the stored set rather than merging IDs.

Also found in 1 other location(s)

apps/server/src/provider/acp/AcpSessionRuntime.ts:976

mergeSessionConfigOptions treats config_option_update as a patch, but ACP defines configOptions as the full current set. If an agent removes option B and sends the remaining nonempty list containing only A, this function retains stale B; later reads and validation can present or accept a configuration option that no longer exists.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/AcpRuntimeModel.ts around line 862:

An empty `config_option_update` produces no `ConfigOptionsChanged` event, so removing the last option leaves stale options in the runtime. Nonempty updates are also later treated by `mergeSessionConfigOptions` as patches, so omitted option IDs remain stale; emit empty snapshots and replace the stored set rather than merging IDs.

Also found in 1 other location(s):
- apps/server/src/provider/acp/AcpSessionRuntime.ts:976 -- `mergeSessionConfigOptions` treats `config_option_update` as a patch, but ACP defines `configOptions` as the full current set. If an agent removes option `B` and sends the remaining nonempty list containing only `A`, this function retains stale `B`; later reads and validation can present or accept a configuration option that no longer exists.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c247b15d: empty config_option_update now emits ConfigOptionsChanged, and applySessionUpdate replaces the stored config option set instead of merging it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
Comment thread apps/server/src/provider/Drivers/DevinProfile.ts Outdated
Comment thread packages/contracts/src/settings.ts
Comment thread apps/server/src/serverSettings.ts
Comment thread apps/server/src/provider/acp/AcpSessionRuntime.ts
Comment thread apps/server/src/provider/Drivers/DevinProfile.ts Outdated
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a substantial Devin provider integration spanning ACP sessions, subprocess execution, permissions, model discovery, text generation, settings, and multiple clients. It also changes product defaults and adds static-analysis suppression directives, while the supplied ACP correctness finding remains a material review concern.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

- Pass resumeCursor as resumeSessionId and propagate started session id as resumeCursor.
- Read image and file attachments and include them in session/prompt.
- Reject overlapping sendTurn calls and only update activeTurnId after setup succeeds.
- Guard stopSession/stopAll against deleting replaced sessions and leaving ACP processes.
- Implement permission and user-input response handlers; only auto-approve in full-access mode.
- Make rollback a provider-side no-op with a clear error.
- Always emit ConfigOptionsChanged for config_option_update, including empty updates.
- Harden ACP startup: serialize pending startup update replay, filter by session id, and set startStateRef only after replay.
- Fix Devin profile identity to include resolved home path and env values.
- Fix configPath whitespace expansion and settings/server-settings devin defaults.
@soramikan

Copy link
Copy Markdown
Author

Pushed c247b15d to address the review findings:

  • resumeCursor is now passed to makeDevinAcpRuntime as resumeSessionId, and started.sessionId is returned as resumeCursor in both startSession and sendTurn.
  • sendTurn now reads image and file attachments from attachmentsDir and includes them as ACP content blocks; overlapping sendTurn calls are rejected and activeTurnId is only set after setup succeeds.
  • Permission requests in non-full-access modes are surfaced via request.opened and can be resolved through respondToRequest/respondToUserInput; full-access still auto-approves.
  • rollbackThread now reports that ACP-backed sessions don't support provider-side rollback instead of only slicing local state.
  • stopAll/stopSession guard against deleting a replaced session while leaving its ACP process running.
  • AcpSessionRuntime now replaces (not merges) configOptions on config_option_update, always emits ConfigOptionsChanged even for empty updates, filters startup replay by session id, and sets startStateRef to Started only after applyPendingStartupUpdates completes.
  • DevinProfile identity now includes the resolved home path and DEVIN_/XDG_ env values.
  • Settings/server settings default devin.enabled and include Devin in provider history.
  • DevinTextGeneration trims configPath before expanding ~.

Typecheck and the Devin/Acp/ProviderRegistry tests pass.

Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts Outdated
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
… event stamps

- Remove withThreadLock from respondToRequest/respondToUserInput so startup
  permission and elicitation requests can be answered while startSession holds
  the per-thread semaphore.
- Track pending approvals and user-inputs by request id at the adapter scope so
  responses can resolve the corresponding Deferreds before the session context is
  published.
- Generate a fresh event stamp for user-input.resolved and session.state.changed
  so lifecycle and resolution events have distinct event ids.

@soramikan soramikan left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed 54a7b3a16 to address the latest round of review findings.

Changes in this update:

  • respondToRequest and respondToUserInput no longer acquire the per-thread withThreadLock, so permission and elicitation requests emitted during acp.start() can be answered while startSession still holds that semaphore.
  • Pending approvals and user-input responses are tracked by request id at the adapter scope, allowing them to be resolved before the DevinSessionContext is published to sessions.
  • session.started and session.state.changed now mint separate event stamps.
  • user-input.resolved now mints its own event stamp instead of reusing user-input.requested's stamp.

All prior macroscope and Cursor findings (resume cursor, attachments, rollback, config option replace/emit, overlapping sendTurn, stopAll concurrency, identity, settings, provider history) remain addressed in c247b15d9608c91712fe044e58551e972a5ee7f4.

Typecheck and the Devin/Acp/ProviderService test suites pass.

Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
…in session

stopSessionInternal now interrupts all pending permission and elicitation
Deferreds for the stopped thread and removes their request IDs from both the
per-context and adapter-wide pending maps, preventing stale entries and late
responses from settling requests belonging to a closed session.
@soramikan

Copy link
Copy Markdown
Author

Pushed 4293323cf to address the latest Macroscope finding:

  • stopSessionInternal now interrupts every pending permission and elicitation Deferred for the stopped thread and removes the request IDs from both the per-context and adapter-wide pending maps, so a later respondToRequest / respondToUserInput cannot settle a request belonging to a closed session.

The earlier startup-permission deadlock and shared event-id findings are still fixed in 54a7b3a16.

Typecheck and the Devin/Acp/ProviderService test suites pass.

Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
…ession stop

- `respondToRequest` and `respondToUserInput` now reject responses for stopped
  sessions by deleting stale entries from the adapter-wide maps.
- Pending approval and user-input contexts store the metadata needed to emit
  resolved events, so `stopSessionInternal` can emit `request.resolved` and
  `user-input.resolved` with a cancel/empty payload when it interrupts a pending
  request, instead of leaving the UI waiting.
… interrupting

`stopSessionInternal` now resolves pending permission and user-input
Deferreds with cancel/empty payloads, letting the ACP callbacks emit
`request.resolved` and `user-input.resolved`, return a cancelled outcome,
and clean up the adapter-wide maps. `respondToRequest` / `respondToUserInput`
still guard against resolving stopped sessions.
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
Comment thread apps/server/src/provider/Layers/DevinAdapter.ts
… turn snapshots

- `respondToRequest` and `respondToUserInput` now verify the request belongs to
  the supplied `threadId` before resolving the deferred.
- `sendTurn` stores the user prompt and final prompt result on the active turn,
  and `handleParsedEvent` appends ACP content/tool/plan events as they arrive,
  so `readThread` no longer returns empty turn items.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit afdcc23. Configure here.

authenticatePayload,
acp.agent.authenticate(authenticatePayload),
);
return yield* promptOnce;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auth retry not gated on mode

High Severity

The new prompt-path authenticate-and-retry runs for every ACP provider, not only authenticationMode: "on-demand". Any AcpRequestError with code -32000 (a generic JSON-RPC server error) now triggers a second authenticate plus a second session/prompt on Cursor and Grok. That can re-run a failed turn and produce duplicate agent work.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit afdcc23. Configure here.

if (modelChanged && targetProtocolValue !== undefined) {
yield* input.runtime
.setModel(targetProtocolValue)
.pipe(Effect.mapError((cause) => input.mapError({ cause, step: "set-model" })));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Model switch compares mismatched IDs

Medium Severity

applyDevinAcpModelSelection treats a model change as requestedModelId !== currentModelId, but the requested value is a T3 slug and currentModelId is the ACP protocol id. After the first apply the stored id is the protocol value, so every later sendTurn with a non-default model looks like a switch and calls setModel again.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit afdcc23. Configure here.


const promptResult = yield* Effect.gen(function* () {
yield* runtime.start();
yield* Effect.ignore(runtime.setMode("ask"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text generation starts an unconstrained agent

High Severity

Commit-message, PR, branch, and title generation spawn devin acp with no runtimeMode and then ignore setMode("ask"). Devin's known modes are normal, accept-edits, smart, plan, and bypass — not ask — so the process stays in the CLI default, which can edit the workspace while producing structured text.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit afdcc23. Configure here.

@t3dotgg

t3dotgg commented Sep 4, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6 Astra (preview) responding on behalf of Theo

This note is part of an automated cleanup pass.

Carryover from #9483 at eb63014: its Devin CLI 3000.6.7 probe found the model catalog in session/new.configOptions under model, not a top-level model list. Preserve the DevinAcpCliProbe.test.ts and DevinAcpSupport.test.ts inputs for that catalog, mode selection, and the adaptive default. Review DevinSkills.ts for user/project .devin, .cognition, and .agents skill discovery plus $name to /name invocation. The reported CLI check covered handshake and discovery on Windows only, with no live prompt. Use those findings as follow-up tests, not as proof that live turns work or that this code was ported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants