Diagnose recursive OpenCode MCP config timeouts#410
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if error is OpenCodeACPLaunchResolutionError || error is ExecutableFileIdentityError { | ||
| return AIProviderError.invalidConfiguration(detail: error.localizedDescription) | ||
| } | ||
| if let guidance = Self.openCodeACPStartupGuidance(for: error) { | ||
| if let guidance = Self.openCodeACPStartupGuidance( |
There was a problem hiding this comment.
Bug: The recursiveMCPConfigGuidance() closure is eagerly evaluated for all errors in normalizeError, causing unnecessary filesystem I/O even when the result is discarded for non-startup errors.
Severity: MEDIUM
Suggested Fix
Modify openCodeACPStartupGuidance to accept recursiveMCPConfigGuidance as an escaping closure (@escaping @Sendable () -> String?). Call the closure inside the function, only after the guard check for the error type has passed. In normalizeError, pass the closure itself, not its result.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
Sources/RepoPrompt/Infrastructure/AI/Providers/OpenCode/OpenCodeACPAgentProvider.swift#L133-L136
Potential issue: The `normalizeError` function calls `recursiveMCPConfigGuidance()` as
an argument to `openCodeACPStartupGuidance`. This causes the closure to be evaluated
immediately, triggering synchronous filesystem I/O (checking for config files, reading
them, and parsing JSON) for every error processed. However, `openCodeACPStartupGuidance`
only uses the result for specific startup timeout errors, discarding it for all other
error types. This leads to unnecessary, potentially expensive I/O operations in a hot
path, such as during `session/prompt` timeouts, which degrades performance. This
contradicts the PR's intent to perform this check only for startup timeouts.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Good catch — this was valid. Fixed in c99f229 by passing the diagnostic closure through lazily and invoking it only after the startup-timeout guard passes. I also strengthened the non-startup timeout test so session/prompt normalization asserts the recursive MCP config diagnostic is not called.
1 participant
Summary
Follow-up to #407 for #117. Thanks to @jpmurray for isolating the root cause: OpenCode can load RepoPrompt/RepoPrompt CE MCP entries from its global config during
opencode acpstartup, recursively launch RepoPrompt CE, and blocksession/newuntil RepoPrompt CE reports a startup timeout.This PR keeps the existing upgrade guidance, but adds a targeted best-effort diagnostic when the startup timeout may be caused by that recursive MCP config:
$XDG_CONFIG_HOME/opencode/opencode.jsonor$HOME/.config/opencode/opencode.json) only after an ACP startup timeoutmcpentries whose key or nested values reference RepoPrompt/RepoPrompt CE"mcp": {}The app still does not edit OpenCode config automatically.
Validation
make dev-formatmake dev-lintmake dev-test FILTER=OpenCodeACPLaunchResolverTestsmake dev-test-listpython3 Scripts/test_suite_optimizer.py verify-ledger --ledger Scripts/Fixtures/test-suite-contract-ledger.tsvmake dev-swift-build PRODUCT=RepoPrompt.agents/skills/rpce-contribution-check/scripts/preflight.sh commit.agents/skills/rpce-contribution-check/scripts/preflight.sh pushStack
Stacked on #407 /
wt/opencode-acp-timeout.