Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ uv run python -m mcp_acp.server
### Three-Layer Design

**1. MCP Server Layer (`server.py`)**
- Exposes 26 MCP tools via stdio protocol
- Exposes 41 MCP tools via stdio protocol
- Inline JSON Schema definitions per tool
- if/elif dispatch in `call_tool()` maps tool names to handlers
- Server-layer confirmation enforcement for destructive bulk operations
Expand Down
212 changes: 212 additions & 0 deletions src/mcp_acp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,163 @@ async def update_session(
except ValueError as e:
return {"updated": False, "message": f"Failed to update session: {str(e)}"}

# ── Scheduled Sessions ──────────────────────────────────────────────

async def list_scheduled_sessions(self, project: str) -> dict[str, Any]:
"""List all scheduled sessions."""
self._validate_input(project, "project")
response = await self._request("GET", "/v1/scheduled-sessions", project)
items = response.get("items", [])
return {"scheduled_sessions": items, "total": len(items)}

async def get_scheduled_session(self, project: str, name: str) -> dict[str, Any]:
"""Get a specific scheduled session by name."""
self._validate_input(project, "project")
self._validate_input(name, "name")
return await self._request("GET", f"/v1/scheduled-sessions/{name}", project)

async def create_scheduled_session(
self,
project: str,
schedule: str,
session_template: dict[str, Any],
display_name: str | None = None,
suspend: bool = False,
dry_run: bool = False,
) -> dict[str, Any]:
"""Create a scheduled session backed by a Kubernetes CronJob."""
self._validate_input(project, "project")

payload: dict[str, Any] = {
"schedule": schedule,
"sessionTemplate": session_template,
"suspend": suspend,
}
if display_name:
payload["displayName"] = display_name

if dry_run:
return {
"dry_run": True,
"success": True,
"message": f"Would create scheduled session with schedule '{schedule}'",
"manifest": payload,
"project": project,
}

try:
result = await self._request("POST", "/v1/scheduled-sessions", project, json_data=payload)
name = result.get("name", "unknown")
return {
"created": True,
"name": name,
"project": project,
"message": f"Scheduled session '{name}' created with schedule '{schedule}'",
}
except (ValueError, TimeoutError) as e:
return {"created": False, "message": str(e)}

async def update_scheduled_session(
self,
project: str,
name: str,
schedule: str | None = None,
display_name: str | None = None,
session_template: dict[str, Any] | None = None,
suspend: bool | None = None,
dry_run: bool = False,
) -> dict[str, Any]:
"""Update a scheduled session (partial update)."""
self._validate_input(project, "project")
self._validate_input(name, "name")

payload: dict[str, Any] = {}
if schedule is not None:
payload["schedule"] = schedule
if display_name is not None:
payload["displayName"] = display_name
if session_template is not None:
payload["sessionTemplate"] = session_template
if suspend is not None:
payload["suspend"] = suspend

if not payload:
raise ValueError("No fields to update. Provide schedule, display_name, session_template, or suspend.")

if dry_run:
return {
"dry_run": True,
"success": True,
"message": f"Would update scheduled session '{name}'",
"patch": payload,
}

try:
await self._request("PUT", f"/v1/scheduled-sessions/{name}", project, json_data=payload)
return {"updated": True, "message": f"Successfully updated scheduled session '{name}'"}
except ValueError as e:
return {"updated": False, "message": f"Failed to update: {str(e)}"}
Comment on lines +681 to +720
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, read the exact code at the specified lines to confirm the HTTP method
sed -n '681,720p' src/mcp_acp/client.py

Repository: ambient-code/mcp

Length of output: 1599


🏁 Script executed:

# Search for any API documentation or contract definitions for scheduled-sessions
rg "scheduled-session|scheduled_session" -A 2 -B 2 --type py | head -100

Repository: ambient-code/mcp

Length of output: 6199


🏁 Script executed:

# Look for tests related to scheduled-session updates
fd test -type f -name "*.py" | xargs rg "update_scheduled_session|scheduled.session" 2>/dev/null | head -50

Repository: ambient-code/mcp

Length of output: 4342


🏁 Script executed:

# Search for any backend API specification or documentation files
fd -e md -e txt -e json | xargs rg "scheduled.session.*PUT|scheduled.session.*PATCH" 2>/dev/null

Repository: ambient-code/mcp

Length of output: 44


🏁 Script executed:

# Check how _request method is defined and what verbs it supports
rg "def _request\(" -A 10 src/mcp_acp/client.py

Repository: ambient-code/mcp

Length of output: 462


🏁 Script executed:

# Find and read the test_update_scheduled_session test
rg "test_update_scheduled_session" -A 20 tests/test_client.py

Repository: ambient-code/mcp

Length of output: 953


🏁 Script executed:

# Search for PATCH usage in the client
rg "PATCH" src/mcp_acp/client.py

Repository: ambient-code/mcp

Length of output: 596


🏁 Script executed:

# Check the full _request method implementation
sed -n '100,200p' src/mcp_acp/client.py | head -100

Repository: ambient-code/mcp

Length of output: 3718


🏁 Script executed:

# Search for any API documentation or backend contract info
find . -name "*.md" -o -name "*.txt" -o -name "*.rst" | xargs rg "scheduled.session.*PUT\|scheduled.session.*PATCH\|scheduled-sessions.*verb" 2>/dev/null || echo "No docs found with those patterns"

Repository: ambient-code/mcp

Length of output: 93


🏁 Script executed:

# Check if there are other partial update methods in the client that use PUT or PATCH
rg "partial update\|sparse.*payload" -B 5 src/mcp_acp/client.py

Repository: ambient-code/mcp

Length of output: 42


Use PATCH instead of PUT for this partial update.

The method docstring states "partial update" and builds a sparse payload, but line 717 sends it with PUT. This creates a semantic mismatch: PUT typically means full replacement (omitted fields could be cleared), while PATCH means partial update. The codebase already uses PATCH for other partial updates on sessions. Either switch to PATCH or fetch and resend the full resource document. Additionally, the existing test does not validate the HTTP method, so add an assertion to prevent regression.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/mcp_acp/client.py` around lines 681 - 720, The update_scheduled_session
method builds a sparse payload for a partial update but currently uses PUT;
change the HTTP verb in the self._request call inside update_scheduled_session
from "PUT" to "PATCH" (keeping the same path and payload) so semantics match the
docstring and other partial-update endpoints, and update related tests to assert
that the client issues a PATCH request (add an assertion checking the HTTP
method) to prevent regressions.


async def delete_scheduled_session(self, project: str, name: str, dry_run: bool = False) -> dict[str, Any]:
"""Delete a scheduled session."""
self._validate_input(project, "project")
self._validate_input(name, "name")

if dry_run:
try:
data = await self._request("GET", f"/v1/scheduled-sessions/{name}", project)
return {
"dry_run": True,
"success": True,
"message": f"Would delete scheduled session '{name}'",
"session_info": {
"name": data.get("name"),
"schedule": data.get("schedule"),
"suspend": data.get("suspend"),
},
}
except ValueError:
return {"dry_run": True, "success": False, "message": f"Scheduled session '{name}' not found"}

try:
await self._request("DELETE", f"/v1/scheduled-sessions/{name}", project)
return {"deleted": True, "message": f"Successfully deleted scheduled session '{name}'"}
except ValueError as e:
return {"deleted": False, "message": f"Failed to delete: {str(e)}"}

async def suspend_scheduled_session(self, project: str, name: str) -> dict[str, Any]:
"""Suspend (pause) a scheduled session."""
self._validate_input(project, "project")
self._validate_input(name, "name")

await self._request("POST", f"/v1/scheduled-sessions/{name}/suspend", project)
return {"suspended": True, "message": f"Scheduled session '{name}' suspended"}

async def resume_scheduled_session(self, project: str, name: str) -> dict[str, Any]:
"""Resume a suspended scheduled session."""
self._validate_input(project, "project")
self._validate_input(name, "name")

await self._request("POST", f"/v1/scheduled-sessions/{name}/resume", project)
return {"resumed": True, "message": f"Scheduled session '{name}' resumed"}

async def trigger_scheduled_session(self, project: str, name: str) -> dict[str, Any]:
"""Manually trigger a scheduled session to run immediately."""
self._validate_input(project, "project")
self._validate_input(name, "name")

await self._request("POST", f"/v1/scheduled-sessions/{name}/trigger", project)
return {"triggered": True, "message": f"Scheduled session '{name}' triggered"}

async def list_scheduled_session_runs(self, project: str, name: str) -> dict[str, Any]:
"""List past runs (AgenticSessions) created by a scheduled session."""
self._validate_input(project, "project")
self._validate_input(name, "name")

response = await self._request("GET", f"/v1/scheduled-sessions/{name}/runs", project)
items = response.get("items", [])
return {"runs": items, "total": len(items), "scheduled_session": name}

# ── Observability ────────────────────────────────────────────────────

async def get_session_logs(
Expand Down Expand Up @@ -676,6 +833,14 @@ async def get_session_metrics(self, project: str, session: str) -> dict[str, Any
result["session"] = session
return result

async def export_session(self, project: str, session: str) -> dict[str, Any]:
"""Export session chat as markdown."""
self._validate_input(project, "project")
self._validate_input(session, "session")

text = await self._request_text("GET", f"/v1/sessions/{session}/export", project)
return {"export": text, "session": session}
Comment on lines +836 to +842
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the _request_text() method implementation
cd src/mcp_acp && grep -n "_request_text" client.py | head -20

Repository: ambient-code/mcp

Length of output: 297


🏁 Script executed:

# Search for _request_text method definition
rg -n "def _request_text" src/mcp_acp/client.py -A 15

Repository: ambient-code/mcp

Length of output: 661


🏁 Script executed:

# Check for any Accept header specifications in the codebase
rg -n "Accept" src/mcp_acp/ -B 2 -A 2

Repository: ambient-code/mcp

Length of output: 602


🏁 Script executed:

# Look for export endpoint tests or documentation
fd -type f \( -name "*.py" -o -name "*.md" \) | xargs rg -l "export.*session\|export_session" 2>/dev/null

Repository: ambient-code/mcp

Length of output: 232


🏁 Script executed:

# Search for API specification files
fd -type f \( -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) | head -20

Repository: ambient-code/mcp

Length of output: 230


🏁 Script executed:

# Get the complete _request_text() method
sed -n '205,240p' src/mcp_acp/client.py

Repository: ambient-code/mcp

Length of output: 1418


🏁 Script executed:

# Look for test files related to export
fd -name "*test*.py" -o -name "test_*.py" | xargs rg -l "export_session\|export" 2>/dev/null

Repository: ambient-code/mcp

Length of output: 292


🏁 Script executed:

# Search for API specs or documentation
find . -name "*.md" -o -name "*.txt" -o -name "README*" | xargs rg -l "export\|markdown" 2>/dev/null | head -10

Repository: ambient-code/mcp

Length of output: 42


🏁 Script executed:

# Check if there are any comments about markdown in the export context
rg -n "markdown\|export" src/mcp_acp/client.py -i -B 2 -A 2

Repository: ambient-code/mcp

Length of output: 42


🏁 Script executed:

# Check if there are other methods that override Accept header
rg -n "Accept.*:" src/mcp_acp/ -B 3 -A 1

Repository: ambient-code/mcp

Length of output: 668


🏁 Script executed:

# Look for backend or spec files
find . -type f -name "*.py" | xargs rg -l "export.*markdown\|markdown.*export" 2>/dev/null

Repository: ambient-code/mcp

Length of output: 44


🏁 Script executed:

# Check all usages of Accept header in the entire codebase
rg -r "Accept" . --type py 2>/dev/null | head -30

Repository: ambient-code/mcp

Length of output: 4475


🏁 Script executed:

# Look for the export endpoint handler or documentation in comments
rg -n "export" src/mcp_acp/client.py -B 3 -A 3

Repository: ambient-code/mcp

Length of output: 618


🏁 Script executed:

# Check if there's a different method that accepts custom headers
rg -n "def _request" src/mcp_acp/client.py -A 8

Repository: ambient-code/mcp

Length of output: 674


🏁 Script executed:

# Look for any markdown-related Accept headers or content types
rg -i "markdown\|text/markdown\|application/vnd" src/mcp_acp/

Repository: ambient-code/mcp

Length of output: 42


🏁 Script executed:

# Check if there are integration tests or docs that specify the expected format
find . -type f \( -name "*.md" -o -name "*.txt" \) -exec grep -l "export\|session" {} \;

Repository: ambient-code/mcp

Length of output: 175


🏁 Script executed:

# Check API_REFERENCE.md for export endpoint specification
grep -n -i "export\|/v1/sessions" ./API_REFERENCE.md -A 5 -B 2

Repository: ambient-code/mcp

Length of output: 3533


🏁 Script executed:

# Check README for export session information
grep -n -i "export\|markdown" ./README.md -A 3 -B 2

Repository: ambient-code/mcp

Length of output: 1178


🏁 Script executed:

# Check CLAUDE.md for any context about export format
grep -n -i "export\|markdown" ./CLAUDE.md -A 3 -B 2

Repository: ambient-code/mcp

Length of output: 155


🏁 Script executed:

# Look at the complete _request method to understand the pattern
sed -n '142,203p' src/mcp_acp/client.py

Repository: ambient-code/mcp

Length of output: 2454


Ensure export endpoint explicitly requests markdown format.

The export_session() method is documented to export as markdown, but _request_text() hardcodes Accept: text/plain. A strict backend requiring explicit markdown content negotiation could return 406 Not Acceptable. Either:

  1. Add an optional accept_header parameter to _request_text() and pass text/markdown from export_session(), or
  2. Follow the pattern used by get_session_transcript() (which accepts a format query parameter per API_REFERENCE.md) and add format negotiation to the export call.

Verify against the backend export endpoint spec to confirm expected content type or query parameter convention.


# ── Labels ───────────────────────────────────────────────────────────

async def label_session(self, project: str, session: str, labels: dict[str, str]) -> dict[str, Any]:
Expand Down Expand Up @@ -874,6 +1039,53 @@ async def bulk_restart_sessions_by_label(
"""Restart sessions matching label selectors (max 3 matches)."""
return await self._run_bulk_by_label(project, labels, self.restart_session, "restart", "restarted", dry_run)

# ── Workflow Management ─────────────────────────────────────────────

async def set_workflow(self, project: str, session: str, workflow: str) -> dict[str, Any]:
"""Set the active workflow on a session."""
self._validate_input(project, "project")
self._validate_input(session, "session")

return await self._request(
"POST", f"/v1/sessions/{session}/workflow", project, json_data={"workflow": workflow}
)

async def get_workflow_metadata(self, project: str, session: str) -> dict[str, Any]:
"""Get workflow metadata for a session."""
self._validate_input(project, "project")
self._validate_input(session, "session")

result = await self._request("GET", f"/v1/sessions/{session}/workflow/metadata", project)
result["session"] = session
return result

# ── Repo Management ─────────────────────────────────────────────────

async def add_repo(self, project: str, session: str, repo_url: str) -> dict[str, Any]:
"""Add a repository to a running session."""
self._validate_input(project, "project")
self._validate_input(session, "session")

return await self._request("POST", f"/v1/sessions/{session}/repos", project, json_data={"url": repo_url})

async def remove_repo(self, project: str, session: str, repo_name: str) -> dict[str, Any]:
"""Remove a repository from a session."""
self._validate_input(project, "project")
self._validate_input(session, "session")
self._validate_input(repo_name, "repo_name")

await self._request("DELETE", f"/v1/sessions/{session}/repos/{repo_name}", project)
return {"removed": True, "message": f"Repo '{repo_name}' removed from session '{session}'"}
Comment on lines +1064 to +1078
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

repo_name is validated with the wrong contract.

Line 1075 reuses the DNS-1123 validator, but repository names derived from URLs are not guaranteed to fit that shape. add_repo() accepts arbitrary repo URLs, so a repo with a common name like foo_bar or foo.bar can be added and then become impossible to remove through this tool. Validate only that the value is non-empty and URL-encode the path segment instead of treating it like a session name.

Proposed fix
+from urllib.parse import quote
+
 async def remove_repo(self, project: str, session: str, repo_name: str) -> dict[str, Any]:
     """Remove a repository from a session."""
     self._validate_input(project, "project")
     self._validate_input(session, "session")
-    self._validate_input(repo_name, "repo_name")
+    if not repo_name:
+        raise ValueError("repo_name must not be empty")
 
-    await self._request("DELETE", f"/v1/sessions/{session}/repos/{repo_name}", project)
+    encoded_repo_name = quote(repo_name, safe="")
+    await self._request("DELETE", f"/v1/sessions/{session}/repos/{encoded_repo_name}", project)
     return {"removed": True, "message": f"Repo '{repo_name}' removed from session '{session}'"}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/mcp_acp/client.py` around lines 1064 - 1078, The remove_repo() method is
incorrectly validating repo_name with the DNS-1123 validator used for session
names; update remove_repo to only check repo_name is non-empty (use a simple
non-empty string check instead of self._validate_input(..., "repo_name")), and
when building the DELETE request path call self._request use a URL-encoded path
segment for repo_name (e.g., encode repo_name before interpolating into
f"/v1/sessions/{session}/repos/{repo_name}") so names like "foo_bar" or
"foo.bar" can be removed; keep existing validations for project and session and
continue to return the same dict response after the await self._request call.


async def get_repos_status(self, project: str, session: str) -> dict[str, Any]:
"""Get repository clone status for a session."""
self._validate_input(project, "project")
self._validate_input(session, "session")

result = await self._request("GET", f"/v1/sessions/{session}/repos/status", project)
result["session"] = session
return result

# ── Cluster & auth ───────────────────────────────────────────────────

def list_clusters(self) -> dict[str, Any]:
Expand Down
56 changes: 56 additions & 0 deletions src/mcp_acp/formatters.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,59 @@ def format_login(result: dict[str, Any]) -> str:
output += f"Error: {result.get('message', 'unknown error')}\n"

return output


def format_scheduled_sessions_list(result: dict[str, Any]) -> str:
"""Format scheduled sessions list."""
output = f"Found {result['total']} scheduled session(s)\n\n"

for ss in result["scheduled_sessions"]:
name = ss.get("name", "unknown")
schedule = ss.get("schedule", "unknown")
suspended = ss.get("suspend", False)
active = ss.get("activeCount", 0)
display = ss.get("displayName", "")

output += f"- {name}"
if display:
output += f" ({display})"
output += f"\n Schedule: {schedule}"
output += f"\n Suspended: {suspended}"
output += f"\n Active runs: {active}\n"

return output


def format_scheduled_session_created(result: dict[str, Any]) -> str:
"""Format scheduled session creation result."""
if result.get("dry_run"):
output = "DRY RUN MODE - No changes made\n\n"
output += result.get("message", "")
if "manifest" in result:
output += f"\n\nManifest:\n{json.dumps(result['manifest'], indent=2)}"
return output

if not result.get("created"):
return f"Failed to create scheduled session: {result.get('message', 'unknown error')}"

name = result.get("name", "unknown")
output = f"Scheduled session created: {name}\n"
output += result.get("message", "")
return output


def format_scheduled_session_runs(result: dict[str, Any]) -> str:
"""Format list of past runs for a scheduled session."""
ss_name = result.get("scheduled_session", "unknown")
output = f"Runs for scheduled session '{ss_name}': {result['total']} total\n\n"

for run in result["runs"]:
run_id = run.get("id", run.get("name", "unknown"))
status = run.get("status", "unknown")
created = run.get("createdAt", run.get("creationTimestamp", "unknown"))

output += f"- {run_id}\n"
output += f" Status: {status}\n"
output += f" Created: {created}\n"

return output
Loading
Loading