feat(agents): add Devin integration - #794
Conversation
|
HashemKhalifa
changed the title
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4bb218670
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let command_is_present = server | ||
| .get("command") | ||
| .and_then(serde_json::Value::as_str) | ||
| .is_some_and(|command| !command.is_empty()); |
There was a problem hiding this comment.
When an existing mcpServers.tracedecay entry contains any non-empty command and a serve argument, this returns Current without comparing the command to InstallContext.tracedecay_bin. The canonical Install preflight skips activation when every component is current, so reinstalling after the binary moves—or over an entry such as other-program serve—can report success while Devin continues launching the wrong executable. Add lifecycle-aware validation against the resolved TraceDecay path.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| _ctx: &InstallContext, | ||
| project_path: &Path, | ||
| ) -> Result<()> { | ||
| uninstall_mcp_if_selected(components, &devin_project_mcp_config_path(project_path)) |
There was a problem hiding this comment.
For a repository whose .devin directory is a symlink to a directory outside the project, the install path rejects the escape, but this uninstall path omits the same containment check. The shared config transaction follows a symlinked parent after canonicalizing it, so uninstall --local --agent devin can remove mcpServers.tracedecay from an external configuration; apply ensure_project_local_safe_paths before deactivation as the activation path does.
Useful? React with 👍 / 👎.
| doctor_check_mcp_registration( | ||
| dc, | ||
| &devin_mcp_config_path(&ctx.home), | ||
| "mcpServers", | ||
| load_json_file, |
There was a problem hiding this comment.
doctor_check_mcp_registration only verifies that mcpServers.tracedecay is an object, so configurations with disabled: true, no usable command, or arguments that omit serve receive a green “MCP server registered” result even though Devin cannot start TraceDecay. Use the Devin-specific registration-state validation for both user and project checks rather than reporting object presence as health.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| fn has_tracedecay(&self, home: &Path) -> bool { | ||
| super::mcp_config_has_tracedecay(&devin_mcp_config_path(home), "mcpServers", load_json_file) | ||
| } |
There was a problem hiding this comment.
When a user installs only .devin/mcp_config.json, this user-scoped has_tracedecay returns false. Doctor gates calls to healthcheck on that method (or reports_absence_to_doctor), and Devin overrides neither the latter nor detected_host_surface, so the project-health branch above is never executed and the local integration is omitted entirely from diagnostics. The doctor admission probe must account for the current project's registry.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| "cursor_desktop", | ||
| "cursor_cloud", | ||
| "codex", | ||
| "devin", |
There was a problem hiding this comment.
HostKindV1::ALL appends Devin after Copilot to preserve established row positions, but this assertion inserts "devin" immediately after Codex. Consequently stock_host_kinds_project_only_fixture_backed_observation_integrations fails unconditionally at index 4 whenever the integration catalog test suite runs; move this expected value to the end.
Useful? React with 👍 / 👎.
1 participant
Why
TraceDecay's redesigned host architecture supports independent integrations for Codex, Cursor, Claude, and other agent hosts, but it did not yet model Devin. Devin has its own local MCP registry and CLI contract, so treating it as an alias of another host would make lifecycle behavior, diagnostics, and future host capabilities ambiguous.
This change adds Devin as its own host while following Devin's existing stdio MCP configuration contract. It targets the PR #707 redesign branch so the integration lands on the new host architecture rather than the legacy implementation.
What Changed
Independent Devin host
HostKindV1::Devin, thedevinstable identifier, and theDevindisplay name.User and project MCP lifecycle
tracedecay install --agent devinmanages the user registry at~/.config/devin/mcp_config.json.tracedecay install --agent devin --localmanages the shared project registry at.devin/mcp_config.json.args: ["serve"], and an empty environment map.disabledfield as enabled, matching Devin's documented entry shape..devin/mcp_config.local.jsonand Devin permission modes unchanged.Documentation
docs/DEVIN-INTEGRATION.mdwith the supported scopes, native Devin CLI equivalents, ownership boundaries, and permission behavior.Devin Contract Checks
The adapter was checked against Devin's local MCP behavior:
~/.config/devin/mcp_config.json.devin/mcp_config.json.devin/mcp_config.local.json(intentionally unmanaged)command,args,env, and stdio transportuserandprojectThe native CLI round trip was also exercised with:
The returned registration preserved the executable,
serveargument, environment entry, andtransport: "stdio"shape expected by the adapter.Test Coverage Added
disabledis recognized as current.--localwithout touching sibling configuration.Validation
Completed:
cargo test -p tracedecay-agent-hosts devin --lib— 4 passed, 0 failed.cargo test -p tracedecay-domain --test integration_catalog_contract stock_host_order_preserves_existing_rows_and_appends_devin -- --exact— 1 passed, 0 failed. This assertion was completed through an intentional red/green TDD cycle.cargo test -p tracedecay-domain --test host_descriptor_contract— 5 passed, 0 failed.cargo test -p tracedecay-cli --features test-transport --test host_lifecycle_cli_acceptance production_cli_installs_devin_project_mcp_without_touching_siblings -- --exact— 1 passed, 0 failed.git diff --check.disabled, project lifecycle reachability, project doctor behavior, and configuration preservation were addressed.Reviewer Notes
mcpServers.tracedecay; it does not replace the registry or mutate Devin's permission policy.codex/tracedecay-total-redesign-plan-reopened; it should land with the PR feat: consolidate TraceDecay V2 PR8-PR13 delivery #707 architecture and is not a standalone release or runtime activation.