feat(devin): add Devin CLI + API worker (MOT-3880) - #425
Conversation
Rust binary worker exposing Devin on the iii bus across two surfaces. The local devin CLI drives one headless turn via devin::run and streams onto devin::events and agent::events. The Devin v3 REST API drives the cloud agent: devin::session::{create,get,list,message} for the org-scoped session lifecycle, devin::pr-review::{trigger,status}, devin::code-scan::{findings,metrics,remediate}, and a devin::api passthrough for the rest of the v3 surface.
Modeled on the grok worker: credentials, base URL, streams, and the devin CLI path come from the configuration worker and hot-reload; DEVIN_API_KEY and DEVIN_ORG_ID are env-expanded so no secret lives in the repo. Mutating functions stay at the needs_approval default; read-only introspection is allow-listed. Scheduling and fan-out are left to the cron and harness workers rather than re-implemented.
|
@rohitg00 is attempting to deploy a commit to the motia Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds a new ChangesDevin worker crate
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FunctionsMod as devin::run
participant CliModule as cli::run
participant Process as devin CLI process
participant StateModule as state.rs
participant EventsModule as events::emit
Client->>FunctionsMod: devin::run(RunRequest)
FunctionsMod->>CliModule: run(iii, cfg, req)
CliModule->>StateModule: load_session
CliModule->>Process: spawn devin with argv
Process-->>CliModule: stdout lines
CliModule->>EventsModule: emit raw_events_stream
CliModule->>StateModule: save_session(status)
CliModule->>EventsModule: emit turn_end/agent_end
CliModule-->>FunctionsMod: result JSON
FunctionsMod-->>Client: session result
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Live-verified against api.devin.ai: personal tokens (apk_/clerk) use the flat v1 API (GET /v1/sessions, GET /v1/session/{id}, POST /v1/session/{id}/message), while v3 is org-scoped and rejects personal tokens. The session wrappers now pick the path shape from whether org_id is set (v1 flat by default, v3 organizations/{org_id}/... when set), base_url defaults to v1, and the create body carries the union of v1 and v3 fields (each omitted when absent).
Config::load now env-expands ${NAME} like the peer workers, so DEVIN_API_KEY and DEVIN_ORG_ID resolve at seed load and an unset org var becomes empty (v1 mode). pr-review and code-scan remain v3/enterprise-only.
Match the grok and codex workers by defaulting iii_context on, so a devin::run turn is prepended with the iii runtime context. Extract the prompt composition into compose_prompt and cover it with tests (prepend on first turn, absent when disabled, not repeated on resume) plus a test that the CLI argv uses the documented devin -- <prompt> form. Verified end to end against a live engine: the worker registers 15 functions; devin::session::list/get and devin::api return real Devin data; devin::run creates a real session and streams onto agent::events and devin::events; and with iii_context on, the created session's prompt carries the iii runtime context (confirmed via session::get).
Match the grok/codex/claude-code/opencode base surface: add devin::start (fire-and-forget), rename devin::runs::list to devin::sessions::list, and drop the cloud devin::session::list (list all cloud sessions via devin::api {GET sessions} instead, removing the sessions::list vs session::list clash).
Fix the local record so it links to the Devin session the CLI opened: parse the {id,url} the devin CLI prints and populate devin_session_id (was always null) plus return url. devin::status and devin::sessions::list now point at the real Devin session; verified end to end (a run linked to devin-6d10fe20...).
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
devin/src/api.rs (1)
98-227: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPercent-encode path segments before building Devin URLs
The path helpers splice
org_id,session_id,scan_id, andfinding_iddirectly into the URL, so reserved characters can change the target endpoint. Percent-encode each segment before formatting it into the path.🤖 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 `@devin/src/api.rs` around lines 98 - 227, The URL path helpers in api.rs insert org_id, session_id, scan_id, and finding_id directly into Devin endpoints, which can break routing when those values contain reserved characters. Update org_scoped, sessions_collection, session_item, session_message_path, and code_scan_remediate to percent-encode each dynamic path segment before formatting the path. Keep the existing request call sites unchanged and apply the encoding at the helper level so all consumers get safe paths.
🧹 Nitpick comments (1)
devin/src/manifest.rs (1)
13-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
supported_targetswill always be a single-element list.The field name suggests multi-target support, but it's populated from a single compile-time
TARGETvalue, so it can only ever report the build's own triple. Harmless today, but worth a comment noting the intended semantics if this manifest is later consumed by tooling expecting multiple targets per artifact.Also applies to: 22-22
🤖 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 `@devin/src/manifest.rs` around lines 13 - 14, The supported_targets field in Manifest is only ever populated from the compile-time TARGET value, so document that it is intentionally a single-element list. Add a clear comment near Manifest/supported_targets or its population site explaining that it represents the build’s own triple, so future tooling does not assume multi-target support.
🤖 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 `@devin/src/main.rs`:
- Around line 62-69: Update the stale comment around Config::load in main to
match the actual fallback behavior: a seed config parse error does not fail
fast, it logs a warning via tracing::warn! and continues with seed = None so
later code can fall back to Config::default(). Keep the code path unchanged and
revise the comment near the seed loading logic to reflect that the configuration
worker is used as the fallback instead of treating a bad config as fatal.
In `@devin/src/state.rs`:
- Around line 62-70: The list_sessions path currently swallows corrupt
SessionRecord entries by using filter_map(...).ok(), which conflicts with the
fail-fast behavior used by load_session. Update the list_sessions parsing logic
to explicitly handle serde_json::from_value failures, and log each
skipped/corrupt item with enough context before continuing. Use the existing
SessionRecord and list_sessions symbols to locate the code, and preserve the
current behavior of returning only valid records while making
version-drift/corruption visible in logs.
---
Outside diff comments:
In `@devin/src/api.rs`:
- Around line 98-227: The URL path helpers in api.rs insert org_id, session_id,
scan_id, and finding_id directly into Devin endpoints, which can break routing
when those values contain reserved characters. Update org_scoped,
sessions_collection, session_item, session_message_path, and code_scan_remediate
to percent-encode each dynamic path segment before formatting the path. Keep the
existing request call sites unchanged and apply the encoding at the helper level
so all consumers get safe paths.
---
Nitpick comments:
In `@devin/src/manifest.rs`:
- Around line 13-14: The supported_targets field in Manifest is only ever
populated from the compile-time TARGET value, so document that it is
intentionally a single-element list. Add a clear comment near
Manifest/supported_targets or its population site explaining that it represents
the build’s own triple, so future tooling does not assume multi-target support.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b1db4f1-348a-42f7-9c64-59061a734c49
⛔ Files ignored due to path filters (1)
devin/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
README.mddevin/.gitignoredevin/Cargo.tomldevin/README.mddevin/build.rsdevin/config.yamldevin/iii-permissions.yamldevin/iii.worker.yamldevin/skills/SKILL.mddevin/src/api.rsdevin/src/cli.rsdevin/src/config.rsdevin/src/configuration.rsdevin/src/events.rsdevin/src/functions/mod.rsdevin/src/functions/types.rsdevin/src/iii_prompt.rsdevin/src/lib.rsdevin/src/main.rsdevin/src/manifest.rsdevin/src/state.rsdevin/src/wire.rsdevin/tests/config.rs
| // Seed from config.yaml when present; a parse error fails fast. | ||
| let seed = match Config::load(&cli.config) { | ||
| Ok(cfg) => Some(cfg), | ||
| Err(e) => { | ||
| tracing::warn!(path = %cli.config, error = %e, "failed to load seed config; relying on the configuration worker"); | ||
| None | ||
| } | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale comment: contradicts actual fallback behavior.
The comment says a parse error "fails fast," but the code does the opposite — it logs a warning and continues with seed = None, later falling back to Config::default(). This matches the repo's config-fallback convention correctly; only the comment is wrong and could mislead future maintainers into thinking a bad seed file is fatal.
📝 Suggested comment fix
- // Seed from config.yaml when present; a parse error fails fast.
+ // Seed from config.yaml when present; a parse error is logged and we fall
+ // back to the configuration worker / built-in defaults (never fails fast).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Seed from config.yaml when present; a parse error fails fast. | |
| let seed = match Config::load(&cli.config) { | |
| Ok(cfg) => Some(cfg), | |
| Err(e) => { | |
| tracing::warn!(path = %cli.config, error = %e, "failed to load seed config; relying on the configuration worker"); | |
| None | |
| } | |
| }; | |
| // Seed from config.yaml when present; a parse error is logged and we fall | |
| // back to the configuration worker / built-in defaults (never fails fast). | |
| let seed = match Config::load(&cli.config) { | |
| Ok(cfg) => Some(cfg), | |
| Err(e) => { | |
| tracing::warn!(path = %cli.config, error = %e, "failed to load seed config; relying on the configuration worker"); | |
| None | |
| } | |
| }; |
🤖 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 `@devin/src/main.rs` around lines 62 - 69, Update the stale comment around
Config::load in main to match the actual fallback behavior: a seed config parse
error does not fail fast, it logs a warning via tracing::warn! and continues
with seed = None so later code can fall back to Config::default(). Keep the code
path unchanged and revise the comment near the seed loading logic to reflect
that the configuration worker is used as the fallback instead of treating a bad
config as fatal.
Source: Learnings
| let arr = match v.as_array() { | ||
| Some(a) => a, | ||
| None => return Ok(vec![]), | ||
| }; | ||
| Ok(arr | ||
| .iter() | ||
| .filter_map(|item| serde_json::from_value::<SessionRecord>(item.clone()).ok()) | ||
| .collect()) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Silent record drop in list_sessions contradicts the fail-fast policy for corrupt records.
load_session treats a corrupt/version-drifted record as a real error to surface (per its doc comment), but list_sessions silently discards the same failure via .ok(), so a corrupted session simply disappears from devin::runs::list with no log trace.
🛠️ Suggested fix: log skipped/corrupt entries
Ok(arr
.iter()
- .filter_map(|item| serde_json::from_value::<SessionRecord>(item.clone()).ok())
+ .filter_map(|item| match serde_json::from_value::<SessionRecord>(item.clone()) {
+ Ok(rec) => Some(rec),
+ Err(e) => {
+ tracing::warn!(error = %e, "skipping corrupt session record in list");
+ None
+ }
+ })
.collect())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let arr = match v.as_array() { | |
| Some(a) => a, | |
| None => return Ok(vec![]), | |
| }; | |
| Ok(arr | |
| .iter() | |
| .filter_map(|item| serde_json::from_value::<SessionRecord>(item.clone()).ok()) | |
| .collect()) | |
| } | |
| let arr = match v.as_array() { | |
| Some(a) => a, | |
| None => return Ok(vec![]), | |
| }; | |
| Ok(arr | |
| .iter() | |
| .filter_map(|item| match serde_json::from_value::<SessionRecord>(item.clone()) { | |
| Ok(rec) => Some(rec), | |
| Err(e) => { | |
| tracing::warn!(error = %e, "skipping corrupt session record in list"); | |
| None | |
| } | |
| }) | |
| .collect()) | |
| } |
🤖 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 `@devin/src/state.rs` around lines 62 - 70, The list_sessions path currently
swallows corrupt SessionRecord entries by using filter_map(...).ok(), which
conflicts with the fail-fast behavior used by load_session. Update the
list_sessions parsing logic to explicitly handle serde_json::from_value
failures, and log each skipped/corrupt item with enough context before
continuing. Use the existing SessionRecord and list_sessions symbols to locate
the code, and preserve the current behavior of returning only valid records
while making version-drift/corruption visible in logs.
The official Devin CLI (the local SWE-1.6 coding agent, brew install --cask devin-cli) runs a bare 'devin -- <prompt>' as an interactive TUI that needs a TTY. Its non-interactive mode is -p/--print ('print response and exit'). build_args now emits '--permission-mode auto --print -- <prompt>' so devin::run runs the local agent headless, works with the files under cwd, and returns the agent's reply in result. Verified the invocation reaches the agent (blocked only by daily quota at test time).
Because the CLI agent runs locally, an iii_context turn can reach the engine at localhost with no exposure, which is the point of the CLI surface. Requires the official CLI plus 'devin auth login'; the earlier @usacognition-beta/devin npm build was a different, cloud-creating binary.
…hrough)
Audit trim: removed devin::code-scan::{findings,metrics,remediate}. Code scanning needs the enterprise UseAccountCodeScans permission, so the vast majority of iii users get 403 and cannot use it; the rare enterprise user reaches it through devin::api. Removes 3 functions, 3 request types, and 3 api wrappers (~150 LOC) that no typical iii user can call.
Worker is now 11 functions: the agent-worker family base (run/start/stop/status/sessions::list), the Devin cloud session lifecycle (session::create/get/message), pr-review::{trigger,status} (org-scoped, composable with a PR-opened trigger), and the devin::api passthrough.
Add four assets embedded via relative paths: iii-discovery (Devin discovering the live worker mesh from a plain question), capabilities (Devin grouping the engine's backend capabilities), session-reply (a real Devin cloud session in the app), and traces (every devin::run traced in the console). These show the bidirectional value: iii delegates to Devin and Devin operates the iii mesh on its own. Correct the permission-mode docs: the devin CLI --help lists a nonexistent 'smart' mode; the real modes are auto (read-only), accept-edits, and dangerous (all tools). Only dangerous auto-approves command execution, so it is what a headless iii-context run needs to run iii trigger; the committed default stays auto.
…he local CLI Default devin::run to --permission-mode dangerous so a headless iii-context run can run iii trigger against the engine out of the box (the point of the CLI surface, and consistent with grok/codex headless auto-approve). Drop to accept-edits or auto via cli_extra_args to restrict the local agent. Refresh SKILL.md, README, and cli.rs module docs for the current reality: the Devin CLI is the local SWE-1.6 coding agent (official cask + devin auth login), driven headless via --print, with iii_context letting it discover and operate the mesh at localhost with no exposure. Removes stale framing (thin cloud client, bare devin -- prompt, dropped session::list, v3-only wording).
Show the devin::session::create call returning a real cloud session (id, url, tags) over the bus, then the same session replying in the Devin app.
The console API_KEY field was ambiguous (service key vs legacy). Document that api_key takes a personal token (apk_, v1) or an organization service key (cog_, v3 with org_id + base_url), and that it is separate from the devin CLI's own devin auth login. Also drop a stale code-scan mention from the org_id comment.
Linear: MOT-3880 (subtickets MOT-3883/3884/3885/3886)
What
A
deploy: binaryRust worker that puts Devin on the iii bus, built on the grok scaffold and consistent with the grok / codex / claude-code / opencode agent-worker family. Devin has two distinct products and the worker exposes both:devin::run/start/stop/status/sessions::listdrive it, matching the family surface, and stream ontodevin::events+agent::events.devin::session::create/get/message,devin::pr-review::trigger/status, and adevin::apipassthrough for the rest.Functions (11)
devin::run/start/stop/status/sessions::listdevin::session::create/get/messagedevin::pr-review::trigger/statusdevin::api{method, path, query?, body?}The CLI surface (
devin::run)The value for an iii user is bidirectional: iii delegates a coding task to the local Devin agent, and with
iii_contexton, that agent discovers and calls any registered iii function (email, db, storage, other agents) mid-task. Because the CLI agent runs locally, it reaches the engine atlocalhostwith no exposure of the engine.Getting there took two corrections worth recording:
brew install --cask devin-cli(orcurl -fsSL https://cli.devin.ai/install.sh | bash), authenticated withdevin auth login. A different@usacognition-beta/devinnpm build creates cloud sessions and muddied early testing; the worker targets the official local CLI.devin -- "<prompt>"starts an interactive TUI that needs a TTY. The non-interactive mode is-p/--print("print response and exit").devin::runnow spawnsdevin --permission-mode auto --print -- <prompt>, so it runs headless, works with the files undercwd, and returns the agent's reply. The local record links to the Devin session id parsed from the CLI output.Design
code-scan(enterprise-gated,403for typical users) since it is reachable throughdevin::apiwhen a token has enterprise access.crontrigger), no sub-agent fan-out (harness::spawn);devin::apireaches the long tail.organizations/{org_id}. The session wrappers pick the shape from whetherorg_idis set.api_key/org_idare env-expanded on load, so no secret lives in the repo.needs_approvaldefault; read-only introspection andstopare allow-listed.Verified end to end (both API modes, real data)
GET /v1/sessions,GET /v1/session/{id}return session data; message + create validate their required fields.org_id): through the worker on a live engine,devin::session::createcreated a real session,devin::session::getreturned the full object, and thedevin::apipassthrough listed the org's sessions.--printinvocation reaches the local agent (verified; a full turn was limited only by daily quota at test time).devin::events+agent::events, and the local record linking to the Devin session all confirmed through the engine. fmt / clippy-D warnings/ tests clean.Draft note
Mutating functions (
session::create/message,pr-review::trigger,devin::run) spend ACUs and spawn real agents; they are confirmed at the path/validation layer and, for the cloud session lifecycle, run to a real session. Marked for maintainer review of the surface and scoping.