Skip to content

fix(runner): restore acp backend MCP tools as fallback when sidecar is absent#1607

Merged
mergify[bot] merged 2 commits into
mainfrom
fix/restore-acp-backend-tools
May 27, 2026
Merged

fix(runner): restore acp backend MCP tools as fallback when sidecar is absent#1607
mergify[bot] merged 2 commits into
mainfrom
fix/restore-acp-backend-tools

Conversation

@markturansky
Copy link
Copy Markdown
Contributor

@markturansky markturansky commented May 27, 2026

Summary

  • PR feat(runner): dynamic credential-aware MCP server configuration #1593 removed the acp backend MCP server (9 session management tools: acp_list_sessions, acp_get_session, acp_create_session, acp_stop_session, acp_send_message, acp_get_session_status, acp_restart_session, acp_list_workflows, acp_get_api_reference) when adding dynamic credential-aware MCP servers
  • The ambient MCP sidecar only provides equivalents for 5 of those 9 tools — stop_session, restart_session, list_workflows, and get_api_reference have no sidecar equivalent
  • Sessions without the sidecar (e.g. older CPs, missing MCP_IMAGE config, or CP_TOKEN_URL/CPTokenPublicKey not configured) lost all session management tooling
  • This re-registers the acp backend tools as a fallback when AMBIENT_MCP_URL is not set. When the sidecar IS present, its tools take precedence

Test plan

  • 549/549 runner tests pass (1 pre-existing mlflow skip)
  • 66/66 MCP + backend_tools tests pass
  • Deploy to staging and verify acp_* tools appear when sidecar is absent
  • Verify tools are NOT registered when sidecar IS present (no duplicates)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Backend API tools are now automatically available in more deployment configurations, expanding functionality and flexibility.
    • Improved logging now surfaces discovered backend tools and their availability.
  • Tests

    • End-to-end scheduled-session test now uses a stubbed workflow API to stabilize editing assertions and ensure OOTB workflow display and custom-field hiding.

…s absent

PR #1593 removed the acp backend MCP server (9 session management tools)
when adding dynamic credential-aware MCP servers, but the ambient MCP
sidecar only covers 5 of those 9 tools. Sessions without the sidecar
(e.g. older CPs or missing MCP_IMAGE config) lost all session management
tooling.

Re-register the acp backend tools as a fallback when AMBIENT_MCP_URL is
not set. When the sidecar IS present, its tools take precedence.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented May 27, 2026

Deploy Preview for cheerful-kitten-f556a0 canceled.

Name Link
🔨 Latest commit 4b3c76a
🔍 Latest deploy log https://app.netlify.com/projects/cheerful-kitten-f556a0/deploys/6a175ba0216f130008de3690

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 674eacab-f263-4d77-aaef-df8cb7a34d22

📥 Commits

Reviewing files that changed from the base of the PR and between 271a220 and 4b3c76a.

📒 Files selected for processing (1)
  • e2e/cypress/e2e/scheduled-sessions.cy.ts

📝 Walkthrough

Walkthrough

build_mcp_servers() now conditionally registers backend API tools as an "acp" MCP server when the ambient MCP sidecar URL (AMBIENT_MCP_URL) is not set, and logs discovered tool names and count. The E2E scheduled-sessions test now stubs the OOTB workflows API and waits for it before asserting UI state.

Changes

Backend MCP tool fallback

Layer / File(s) Summary
Conditional backend MCP tool registration
components/runners/ambient-runner/ambient_runner/bridges/claude/mcp.py
Import create_backend_mcp_tools and, when AMBIENT_MCP_URL is empty, build and register backend API tools under the acp MCP server name; log the number and names of discovered tools.
E2E: stub OOTB workflows in edit test
e2e/cypress/e2e/scheduled-sessions.cy.ts
Add cy.intercept for /api/workflows/ootb*, alias @ootbWorkflows, visit the edit page and wait for @ootbWorkflows before asserting the workflow select and custom fields visibility.

Possibly related PRs

  • ambient-code/platform#1006: Touches the same build_mcp_servers() path and updates ACP tool names/logging.
  • ambient-code/platform#1593: Modified build_mcp_servers() ACP wiring previously; this PR reintroduces conditional ACP registration when sidecar is absent.
🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title follows Conventional Commits format (fix(runner): ...) and clearly describes the main change: restoring acp backend MCP tools as a fallback mechanism when the sidecar is absent.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed Changes add conditional backend tool initialization (O(1) check, single bounded list of 9 tools) and test stubbing—no N+1, loops, unbounded growth, or expensive operations detected.
Security And Secret Handling ✅ Passed No blocking security violations: no plaintext secrets, proper Bearer token auth, mock test data appropriate, no injection vulnerabilities or data leakage found.
Kubernetes Resource Safety ✅ Passed PR contains only Python application logic and Cypress test changes; no Kubernetes manifests, RBAC policies, or resource definitions are modified.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/restore-acp-backend-tools
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/restore-acp-backend-tools

Comment @coderabbitai help to get the list of available commands and usage tips.

The scheduled-sessions E2E test for OOTB workflow name resolution
depends on the backend fetching workflows from the GitHub API. In CI
(Kind cluster), there's no GitHub token so the unauthenticated rate
limit (60 req/hr) causes intermittent failures where the OOTB list
comes back empty and the workflow dropdown falls back to "Custom
workflow..." instead of showing "Fix a bug".

Add cy.intercept stub for /api/workflows/ootb* with the expected
bugfix workflow fixture data.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@mergify mergify Bot added the queued label May 27, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 27, 2026

Merge Queue Status

  • Entered queue2026-05-27 21:11 UTC · Rule: default
  • Checks skipped · PR is already up-to-date
  • Merged2026-05-27 21:12 UTC · at 4b3c76a185c66d6a79c95f02e7451bcf0a15768a · squash

This pull request spent 1 minute 4 seconds in the queue, including 10 seconds running CI.

Required conditions to merge

@mergify mergify Bot merged commit 5b1e702 into main May 27, 2026
61 checks passed
@mergify mergify Bot deleted the fix/restore-acp-backend-tools branch May 27, 2026 21:12
@mergify mergify Bot removed the queued label May 27, 2026
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.

1 participant