feat(agent): add Devin (Cognition AI) as the 12th provider, plus 2 ACP shared-transport fixes#1977
feat(agent): add Devin (Cognition AI) as the 12th provider, plus 2 ACP shared-transport fixes#1977mskutlu wants to merge 5 commits intomultica-ai:mainfrom
Conversation
Wires Devin for Terminal into Multica's local-daemon runtime by
reusing the existing ACP transport (the same protocol that drives
hermes / kimi / kiro). A probe of `devin acp` confirmed the wire
shape — agentCapabilities.loadSession=true, standard ACP
session/update notifications, JSON-RPC over stdio — so the
integration is a thin shim around the shared hermesClient.
Backend
- New `server/pkg/agent/devin.go` (~340 LOC) modeled on kiro.go.
Spawns `devin acp` and lets hermesClient handle initialize /
session/{new,load} / session/prompt and auto-approval of
session/request_permission.
- Devin's ACP server does NOT implement session/set_model. Rather
than calling and failing, the backend logs a warning when
opts.Model is set and continues with Devin's own default model.
- ModelSelectionSupported("devin") returns false so the runtime UI
can hide the model dropdown for Devin runtimes; ListModels still
returns a single placeholder entry so list consumers don't break.
- agent.New / launchHeaders updated; daemon config.go auto-detects
`devin` on PATH (configurable via MULTICA_DEVIN_PATH /
MULTICA_DEVIN_MODEL).
Skill / context surfaces
- Project-level skills auto-discovered from `.devin/skills/`
(matches Devin for Terminal's native convention).
- User-level skills resolved at `~/.config/devin/skills/`.
- Devin shares the AGENTS.md context-injection branch with the
other agents-spec providers.
Frontend
- DevinLogo component + `case "devin"` in the provider-logo switch.
- "devin" added to the ProviderName union in step-welcome.tsx.
Tests
- `devin_test.go` covers: New() returns *devinBackend, tool-name
normalization, `acp` spawn, custom-args filtering (`acp` blocked,
`--agent-type` allowed through), prompt -> output round-trip,
Model option logs a warning without failing, and resume goes
through `session/load` (never `session/new`).
- `agent_test` and `models_test` extended to lock in LaunchHeader
coverage and ModelSelectionSupported("devin")=false.
Docs
- README.md / README.zh-CN.md, providers.mdx (+ .zh), index.mdx
(+ .zh), daemon-runtimes.mdx (+ .zh), how-multica-works.mdx
(+ .zh), cloud-quickstart.mdx (+ .zh), CLI_INSTALL.md,
CLI_AND_DAEMON.md, SELF_HOSTING.md updated to advertise the new
twelfth provider and document MULTICA_DEVIN_PATH /
MULTICA_DEVIN_MODEL.
Verified: `go build ./...`, `go vet ./...`, `gofmt -d` on changed
files, `go test ./...` (full server suite), `pnpm typecheck` (6
tasks), `pnpm test` (33 view test files / 241 tests) — all green.
Anonymous page loads probe `/api/me` and `/api/workspaces` to bootstrap the session. Both return 401 by design — they're answering "no active session, please log in," not signalling a real failure. The dev console was logging those at `error` level, on top of an `auth` logger.error "cookie auth init failed" for the same expected outcome, which trains developers to ignore real auth breakage. Two surgical changes: - `packages/core/api/client.ts`: `logLevel` already special-cases 404 to `warn`. Add 401 to that list. The thrown ApiError still propagates to the caller, so callers that consider a particular 401 fatal can react however they want; only the log-level on the response itself changes. - `packages/core/platform/auth-initializer.tsx`: introduce `logAuthInitFailure` that demotes 401 to `info` with a "no active session" reason and keeps `error` for everything else (network, 5xx, malformed response). Both the cookie-mode and token-mode catch blocks route through it. `error` level is now reserved for unexpected auth breakage, which is the only thing developers should be glancing at red text for.
`hermesClient.handleAgentRequest` hardcoded `optionId: "approve_for_session"` in its `session/request_permission` reply. That works for kimi / kiro because both happen to use that exact string — but it's a kimi-flavoured optionId, not the canonical ACP one. Devin's ACP server presents options whose IDs follow the spec's `kind` names (`allow_once` / `allow_always` / `reject_once`) and treats unknown IDs as a rejection, logging "Tool execution was rejected by the user: Unknown permission option" and aborting the turn without emitting any final assistant text — surfaced to the daemon as a misleading "devin returned empty output" / blocked task. Fix: parse the request's `options` array, pick the most permissive accept option by canonical `kind` (`allow_always` > `allow_once`), and echo back whichever `optionId` the agent itself advertised. The selection is provider-agnostic: kimi / kiro still resolve to `approve_for_session` (their `kind=allow_always` entry), devin resolves to whatever it labels its allow-always option. A defensive fallback to the legacy `approve_for_session` literal preserves behaviour if a future agent emits an empty options array. Verified end-to-end: - New unit tests cover Devin-style optionIds, kimi/kiro-style optionIds, allow_always > allow_once priority, reject-only options, unknown kinds, and empty options. - Existing kimi/kiro/hermes tests pass unchanged (regression guard kept the literal `approve_for_session` assertion — fix still resolves to that for kimi-flavoured option arrays). - Live test against a real `devin acp` session: the daemon now logs `option_id=allow_session` (Devin's `kind=allow_always` optionId), the terminal tool executes successfully, and Devin emits a final `agent_message_chunk` with the result. Task status flips from `blocked: devin returned empty output` to `completed`.
|
@mskutlu is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Adds Devin (Cognition AI) as a new ACP-based provider and hardens shared ACP transport behavior, while reducing noisy auth logs on expected 401s during cold-start session probing.
Changes:
- Add a new
devinagent backend that spawnsdevin acp, reuses the shared ACP transport, and wires skills + runtime config support. - Fix ACP auto-approval to select the agent-advertised permission
optionIdbased on canonicalkind(preferallow_always), improving provider portability. - Demote expected cold-start 401s from
errorlogging towarn/infoin the core API client and auth initializer.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/pkg/agent/devin.go | New Devin backend (spawns devin acp, ACP lifecycle, tool-name normalization, model-selection warning behavior). |
| server/pkg/agent/devin_test.go | Unit tests for Devin backend behaviors (spawn args, prompt round-trip, resume via session/load, no session/set_model). |
| server/pkg/agent/hermes.go | Shared ACP fix: choose permission approval optionId by advertised kind rather than hardcoding. |
| server/pkg/agent/hermes_test.go | Regression + unit tests for permission option selection logic (Devin-style and kimi/kiro-style). |
| server/pkg/agent/models.go | Add Devin static placeholder model list + ModelSelectionSupported("devin") = false. |
| server/pkg/agent/models_test.go | Tests covering Devin model-selection unsupported and placeholder model defaults. |
| server/pkg/agent/agent.go | Register devin in backend factory and launch header mappings + docstrings. |
| server/pkg/agent/agent_test.go | Ensure launch header coverage includes devin. |
| server/internal/daemon/config.go | Auto-detect devin on PATH; support MULTICA_DEVIN_PATH / MULTICA_DEVIN_MODEL. |
| server/internal/daemon/execenv/context.go | Add project-level Devin skills path .devin/skills. |
| server/internal/daemon/local_skills.go | Add user-level Devin skills path ~/.config/devin/skills. |
| server/internal/daemon/execenv/runtime_config.go | Include devin in AGENTS.md context injection path. |
| packages/views/runtimes/components/provider-logo.tsx | Add Devin provider logo and routing. |
| packages/views/onboarding/steps/step-welcome.tsx | Extend provider union type to include devin. |
| packages/core/api/client.ts | Demote 401 responses to warn logging (while still throwing ApiError). |
| packages/core/platform/auth-initializer.tsx | Log expected cold-start 401 auth init failures at info instead of error. |
| apps/docs/content/docs/providers.mdx | Docs matrix updated to include Devin and “12 tools” count. |
| apps/docs/content/docs/providers.zh.mdx | Chinese docs matrix updated to include Devin and “12 tools” count. |
| apps/docs/content/docs/index.mdx | Docs homepage updated to list Devin as supported runtime tool (12 total). |
| apps/docs/content/docs/index.zh.mdx | Chinese docs homepage updated to list Devin as supported runtime tool (12 total). |
| apps/docs/content/docs/how-multica-works.mdx | Update architecture explanation to include Devin among supported tools. |
| apps/docs/content/docs/how-multica-works.zh.mdx | Chinese architecture explanation updated to include Devin. |
| apps/docs/content/docs/daemon-runtimes.mdx | Update daemon startup docs to reference 12 tools incl. Devin. |
| apps/docs/content/docs/daemon-runtimes.zh.mdx | Chinese daemon startup docs updated to reference 12 tools incl. Devin. |
| apps/docs/content/docs/cloud-quickstart.mdx | Update quickstart prerequisite tool list to include Devin. |
| apps/docs/content/docs/cloud-quickstart.zh.mdx | Chinese quickstart prerequisite tool list updated to include Devin. |
| README.md | Top-level README updated to include Devin in supported tools list and runtime diagram. |
| README.zh-CN.md | Chinese README updated to include Devin in supported tools list and runtime diagram. |
| CLI_INSTALL.md | CLI install guide updated to mention Devin in detected agents list. |
| CLI_AND_DAEMON.md | CLI/daemon guide updated for Devin detection + env var overrides. |
| SELF_HOSTING.md | Self-hosting prerequisites updated to include Devin CLI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| defer cancel() | ||
| defer close(msgCh) | ||
| defer close(resCh) | ||
| defer func() { | ||
| stdin.Close() | ||
| _ = cmd.Wait() |
There was a problem hiding this comment.
In the ACP lifecycle goroutine, msgCh/resCh are deferred to close, but on early returns (e.g. initialize/session/load/session/new failures) there’s no synchronization with the stdout reader goroutine. cmd.Wait() does not guarantee the reader has finished draining stdout, so the reader can still call trySend(msgCh, ...) after the channels are closed, causing a send-on-closed-channel panic. Consider explicitly waiting for readerDone before closing the channels in all exit paths (including early failures), e.g. by moving channel closes to a shared cleanup block that joins the reader goroutine first.
| defer cancel() | |
| defer close(msgCh) | |
| defer close(resCh) | |
| defer func() { | |
| stdin.Close() | |
| _ = cmd.Wait() | |
| defer func() { | |
| cancel() | |
| stdin.Close() | |
| _ = cmd.Wait() | |
| <-readerDone | |
| close(msgCh) | |
| close(resCh) |
| // entirely by hermesClient.handleAgentRequest replying | ||
| // "approve_for_session" to every session/request_permission request, | ||
| // identical to how kimi / kiro are driven. |
There was a problem hiding this comment.
The Devin ACP dialect notes still say auto-approval works by replying with a hardcoded "approve_for_session" optionId. hermesClient.handleAgentRequest now selects the agent-advertised optionId by kind, so this comment is no longer accurate (and is the root cause this PR is fixing). Please update the note to describe the new portable behavior (select allow_always/allow_once by kind and echo back the corresponding optionId).
| // entirely by hermesClient.handleAgentRequest replying | |
| // "approve_for_session" to every session/request_permission request, | |
| // identical to how kimi / kiro are driven. | |
| // entirely by hermesClient.handleAgentRequest selecting the | |
| // agent-advertised allow_always / allow_once option by kind for each | |
| // session/request_permission request and echoing back the matching | |
| // optionId. |
| // ACP `session/set_model` RPC before each prompt, which means the | ||
| // UI's dropdown choice is carried all the way down to the LLM call. |
There was a problem hiding this comment.
The comment says Hermes/Kimi/Kiro route model selection through session/set_model “before each prompt”, but the current implementations set the model once after session creation (and before the first prompt). Consider adjusting the wording to match the actual behavior so future readers don’t assume model switching is performed per-turn.
| // ACP `session/set_model` RPC before each prompt, which means the | |
| // UI's dropdown choice is carried all the way down to the LLM call. | |
| // ACP `session/set_model` RPC after session creation and before the | |
| // first prompt, which means the UI's dropdown choice is carried all | |
| // the way down to the LLM call. |
…mments
Three review-feedback fixes:
1. Channel-close race in the ACP lifecycle goroutine (devin.go, plus the
identical pattern in kiro.go / kimi.go / hermes.go). The previous
layout relied on three deferred `close(...)` statements that fired
purely on goroutine exit, with no synchronization against the
stdout-reader goroutine. `cmd.Wait()` returns when the process exits,
but does not guarantee the reader has drained the stdout buffer or
finished its in-flight `c.handleLine(line)` call — which eventually
reaches `trySend(msgCh, ...)`. On any early return (initialize /
session/load / session/new / session/set_model failures) this opens
a window where the reader can send to a closed `msgCh` and panic
with "send on closed channel". The race is gated by
`streamingCurrentTurn` in current code so it's mostly latent, but
the pattern is fragile and any future early-return path (or a
notification handler that doesn't gate on streamingCurrentTurn)
would make it live.
Fix: replace the four separate defers with a single deferred
cleanup block that does the operations in an explicit, correct
order:
cancel() // unblock anything bound to runCtx
stdin.Close() // ask the agent to exit gracefully
_ = cmd.Wait() // wait for the process; stdout closes here
<-readerDone // wait for the reader goroutine to fully drain
close(msgCh) // safe: reader was the only sender, now finished
close(resCh) // safe: only this goroutine sends to resCh
Applied identically to all four ACP backends so the next ACP shim
can copy the pattern without inheriting the bug. Existing tests
still pass (including with -race) because the happy paths already
invoked `<-readerDone` explicitly before reading shared state — the
deferred calls are now idempotent no-ops on those paths.
2. Devin docstring claimed auto-approval works by replying with a
hardcoded "approve_for_session" optionId. After commit d23e9c3 that
is no longer accurate — `hermesClient.handleAgentRequest` selects
the agent-advertised optionId by canonical ACP `kind`. Updated the
note in `devin.go` to describe the new portable behaviour and call
out that for Devin specifically the resolved optionId is
"allow_session" while kimi / kiro resolve to "approve_for_session".
3. `ModelSelectionSupported` doc said Hermes / Kimi / Kiro route
`opts.Model` through `session/set_model` "before each prompt". The
actual implementations call it once per session (immediately after
creation, before the session's only prompt). Reworded to match the
real behaviour and added a note that switching mid-task is not
wired today.
98c849d to
9c71672
Compare
|
All three review points addressed in 1. Channel-close race in the ACP lifecycle goroutineYou're right, and the same fragile pattern exists in defer func() {
cancel() // unblock anything bound to runCtx
stdin.Close() // ask the agent to exit gracefully
_ = cmd.Wait() // wait for the process; stdout closes here
<-readerDone // wait for the reader goroutine to fully drain
close(msgCh) // safe: the reader was the only msgCh sender, now finished
close(resCh) // safe: only this goroutine sends to resCh
}()The happy paths still call In current code the race is largely gated by 2. Stale Devin docstring on auto-approvalUpdated the comment block in 3.
|
Summary
Adds Devin (Cognition AI's terminal coding agent) as Multica's 12th provider, plus two related fixes that surfaced while building and exercising the integration.
Devin's ACP server (
devin acp) speaks the same protocol Hermes / Kimi / Kiro already use, so the new backend is a thin shim around the existinghermesClienttransport. The two fixes harden the shared ACP path for any future provider — they're not Devin-specific.Commits
1.
feat(agent): add Devin (Cognition AI) as the 12th providerserver/pkg/agent/devin.go(~340 LOC) modeled onkiro.go. Spawnsdevin acpand reuses the sharedhermesClientfor initialize /session/{new,load}/session/prompt/ auto-approval ofsession/request_permission.session/set_model(model selection lives in Devin's own config). Rather than calling and failing, the backend logs a warning whenopts.Modelis set and continues with Devin's default.ModelSelectionSupported("devin")returnsfalseso the runtime UI hides the model dropdown for Devin runtimes.agent.Newswitch +launchHeadersupdated; daemonconfig.goauto-detectsdevinon PATH (configurable viaMULTICA_DEVIN_PATH/MULTICA_DEVIN_MODEL)..devin/skills/(matches Devin for Terminal's native convention)~/.config/devin/skills/AGENTS.mdcontext-injection branch with the other agents-spec providers.DevinLogoSVG +case "devin"inprovider-logo.tsx;"devin"added to theProviderNameunion instep-welcome.tsx.devin_test.gocovers New() returns*devinBackend, tool-name normalization,acpspawn, custom-args filtering, prompt → output round-trip, theModeloption warning path, andsession/loadresume.agent_testandmodels_testextended to lockLaunchHeadercoverage andModelSelectionSupported("devin") = false.README,providers.mdx,index.mdx,daemon-runtimes.mdx,how-multica-works.mdx,cloud-quickstart.mdx,CLI_INSTALL.md,CLI_AND_DAEMON.md,SELF_HOSTING.md— all bumped 11 → 12 with a Devin section + theMULTICA_DEVIN_PATH/MULTICA_DEVIN_MODELenv vars.2.
fix(api): demote cold-start 401 noise to warn / infopackages/core/api/client.tsalready special-cases 404 →warn. Add 401 to that list. The thrownApiErrorstill propagates to the caller, so callers that consider a particular 401 fatal can still react; only the log level on the response itself changes.packages/core/platform/auth-initializer.tsx: introducelogAuthInitFailurethat demotes 401 toinfo("no active session") and keepserrorfor everything else (network, 5xx, malformed response). Both cookie-mode and token-mode catch blocks route through it.Anonymous page loads probe
/api/meand/api/workspacesto bootstrap the session; both return 401 by design — they're answering "no active session, please log in," not signalling a real failure. Logging that aterrorlevel on every cold start trains developers to ignore real auth breakage.3.
fix(agent): pick ACP permission optionId from agent's actual optionshermesClient.handleAgentRequesthardcodedoptionId: "approve_for_session"in itssession/request_permissionreply. That works for kimi / kiro because both happen to use that exact string — but it's a kimi-flavoured optionId, not the canonical ACP one. Devin's ACP server presents options whose IDs follow the spec'skindnames (allow_once/allow_always/reject_once) and treats unknown IDs as a rejection, logging "Tool execution was rejected by the user: Unknown permission option" and aborting the turn without emitting any final assistant text — surfaced to the daemon as a misleading "devin returned empty output" / blocked task.Fix: parse the request's
optionsarray, pick the most permissive accept option by canonicalkind(allow_always>allow_once), and echo back whicheveroptionIdthe agent itself advertised. Provider-agnostic: kimi / kiro still resolve toapprove_for_session(theirkind=allow_alwaysentry), Devin resolves toallow_session(its label for the same kind). A defensive fallback to the legacyapprove_for_sessionliteral preserves behaviour if a future agent ever emits an empty options array.New unit tests cover Devin-style optionIds, kimi/kiro-style optionIds,
allow_always>allow_oncepriority, reject-only options, unknown kinds, and empty options. Existing kimi/kiro/hermes tests pass unchanged (regression guard kept the literalapprove_for_sessionassertion — fix still resolves to that for kimi-flavoured option arrays).Verification
go build ./...,go vet ./...,gofmt -don every changed file — all clean.go test ./...(full server suite, ~25 packages) — green.pnpm typecheck(6 tasks) +pnpm test(33 view test files / 241 tests) — green.devin acpsession on macOS:devinon PATH and registered the runtime:Agents: claude, codex, opencode, gemini, devin.DevinLogoicon.pwdandwhoamiand report the output as a single line" to a Devin agent.auto-approved agent permission request method=session/request_permission option_id=allow_session(Devin's actualkind=allow_alwaysoptionId, not the kimi-flavoured literal).agent_message_chunk:/Users/.../workdir | msk.blocked: devin returned empty output(pre-fix) tocompleted.Caveats
forest autosave failed: database disk image is malformedwarnings on stderr during sessions. Those are Devin-side issues with their internal session DB and aren't actionable from Multica's side; just noise in the daemon log.agentInfo.namereportsaffogato(their internal codename) — so any user-facing strings derived fromagentInfo(none today, but worth a flag) won't say "Devin".session/set_modelsupport later,ModelSelectionSupported("devin")should flip totrueanddevinStaticModelsshould be replaced with a CLI-discovery function (TODO comment inmodels.go).Happy to split into three separate PRs (the two fixes apply standalone) if reviewers prefer.