Skip to content

fix(opencode): prevent ACP session/new hang from inherited MCP servers - #789

Draft
dsebban wants to merge 6 commits into
repoprompt:mainfrom
dsebban:fix/opencode-acp-session-new-mcp-timeout
Draft

fix(opencode): prevent ACP session/new hang from inherited MCP servers#789
dsebban wants to merge 6 commits into
repoprompt:mainfrom
dsebban:fix/opencode-acp-session-new-mcp-timeout

Conversation

@dsebban

@dsebban dsebban commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • OpenCode ACP session/new was timing out after 30s with initialize succeeding (no matching ACP response; last stdout is the initialize result).
  • Root cause: OpenCode merges global/project mcp entries into ACP startup and waits for them to connect. RepoPrompt only overrode the same-name RepoPromptCE entry in OPENCODE_CONFIG_CONTENT, so other hanging MCP servers (and recursive RepoPrompt CE CLI configs) could still block bootstrap.
  • Dual Oracle PR Support configurable Oracle groups of up to five #782 does not address this path.

Fix

  • Discover MCP server names from global (~/.config/opencode/opencode.json / XDG) and project (opencode.json, .opencode/opencode.json) configs.
  • In the process-ephemeral overlay, disable every inherited non-RepoPrompt MCP name and keep the managed RepoPromptCE entry active or disabled as before.
  • Surface clearer OpenCode startup-timeout guidance when bootstrap still fails.

Validation

  • make dev-test FILTER=OpenCodeIntegrationConfigurationTests ✅ (3 tests)
  • make dev-test FILTER=OpenCodeACPLaunchResolverTests ✅ (11 tests)
  • Local OpenCode 1.18.16 repro: hanging inherited MCP blocked session/new; neutralizing discovered names returned a session in ~0.3s.

Note

Not part of #782. Separate PR off main.

OpenCode merges global/project MCP entries into ACP session/new and waits
for them to connect. A hanging non-RepoPrompt server (or recursive
RepoPrompt CE CLI entry) can exceed the 30s bootstrap timeout after
initialize succeeds. Disable discovered inherited MCP names in the
process-ephemeral OPENCODE_CONFIG_CONTENT overlay and improve startup
timeout guidance.

@baron baron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking on exact head 716d2e10940614422ff2603ea51e5325bec9779c.

The mitigation only discovers the global/current-directory opencode.json paths plus .opencode/opencode.json, and parses strict JSON (OpenCodeIntegrationConfiguration.swift:224-261). OpenCode's configuration authority also supports JSONC, OPENCODE_CONFIG, ancestor project configs, .opencode configs, and inline content; those sources are merged rather than replaced. An inherited MCP server from any omitted source can therefore remain enabled and reproduce the session/new hang. The current tests cover only the narrow JSON path (OpenCodeIntegrationConfigurationTests.swift:6-90).

Please make the overlay match the actual loader sources (or disable project discovery through an authoritative supported mechanism), and add JSONC/custom-path/ancestor-source regressions. Official config authority: https://opencode.ai/docs/config

@baron baron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking issue: the neutralization pass does not discover the same config sources that OpenCode actually inherits.

discoverInheritedMCPServerNames currently checks only global opencode.json, <cwd>/opencode.json, and <cwd>/.opencode/opencode.json, all via strict JSONSerialization. OpenCode v1.18.16 also loads global config.json and opencode.jsonc, walks ancestor directories to the worktree boundary for opencode.json[c] and .opencode/opencode.json[c], and accepts OPENCODE_CONFIG / OPENCODE_CONFIG_DIR sources. A hanging MCP defined in any of those supported locations is absent from the overlay, so session/new can still take the same timeout path this PR is intended to prevent.

Please either mirror the authoritative OpenCode source-resolution boundary (including JSONC and env-directed sources) or isolate the launched process from inherited MCP startup through an upstream-supported mechanism. Add coverage for at least global JSONC, an ancestor project config, and env-directed config. The new tests currently cover only the three paths implemented here.

Upstream v1.18.16 evidence:

@dsebban dsebban left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Follow-up notes for the two blocking config-authority reviews.

// content is replaced rather than inherited and cannot contribute an additional MCP name.
var seen = Set<String>()
return urls
.map(\.standardizedFileURL.path)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed the loader-authority gap here: inherited MCP discovery now covers global config.json / opencode.json / opencode.jsonc, OPENCODE_CONFIG, upward project opencode.json[c] and .opencode/opencode.json[c] to the worktree boundary, OPENCODE_CONFIG_DIR, and JSONC parsing. Parent OPENCODE_CONFIG_CONTENT is intentionally not scanned because the child launch replaces that variable with RepoPrompt's overlay.

XCTAssertEqual((mcp["Other"] as? [String: Any])?["enabled"] as? Bool, false)
XCTAssertEqual((mcp["RepoPromptCE"] as? [String: Any])?["enabled"] as? Bool, false)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Regression coverage now exercises the review-requested authorities independently: global JSONC, ancestor project + .opencode, env-directed OPENCODE_CONFIG / OPENCODE_CONFIG_DIR, the project-disable flag, and the inline-content replacement contract.

@dsebban
dsebban marked this pull request as draft August 11, 2026 09:19

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Audit disposition — remaining routing blocker (2026-08-14)

The inherited-MCP diagnosis is credible, but the overlay skips the managed RepoPrompt server name using a case-insensitive comparison while JSON object keys are case-sensitive. An inherited differently cased recursive entry can therefore be omitted from the disabling overlay and remain active.

Disable each exact inherited key except the exact key that the overlay replaces, and add case-variant/duplicate-name tests. Malformed, oversized, unreadable, or concurrently changed configuration should also produce conservative startup guidance rather than silently disappearing from discovery. Please fix those cases before merge.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Deep-review assessment — 2026-08-14

Disposition: block pending exact-key neutralization and conservative discovery failure handling. The reproduced root cause is convincing, and neutralizing inherited MCP entries in the process-ephemeral overlay is the right boundary. However, inherited names equal to RepoPromptCE are skipped case-insensitively. If OpenCode’s merge keys are case-sensitive, a differently cased recursive RepoPrompt entry can remain enabled while the canonical key is installed.

Disable every exact discovered key first, then install the canonical managed RepoPromptCE entry. Add case-variant and duplicate-key tests. Also do not silently fail open when a known config path is malformed, oversized, unreadable, or changes during discovery; surface a warning/block or otherwise prove OpenCode will not inherit an undiscovered hanging entry.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants