Skip to content

Commit 4143dfb

Browse files
rohoswaggerclaude
andcommitted
refactor(craft): simplify ACP session management — single resume_or_create_session entry point
Remove redundant two-layer session management that was left over from the long-lived client pattern. With ephemeral clients, the in-memory session cache and public wrapper methods were dead code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5ec8aff commit 4143dfb

File tree

2 files changed

+13
-114
lines changed

2 files changed

+13
-114
lines changed

backend/onyx/server/features/build/sandbox/kubernetes/internal/acp_exec_client.py

Lines changed: 10 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,18 @@
44
using stdin/stdout for JSON-RPC communication. This bypasses the HTTP server
55
and uses the native ACP subprocess protocol.
66
7-
When multiple API server replicas share the same sandbox pod, this client
8-
uses ACP session resumption (session/list + session/resume) to maintain
9-
conversation context across replicas.
7+
Each message creates an ephemeral client (start → resume_or_create_session →
8+
send_message → stop) to prevent concurrent processes from corrupting
9+
opencode's flat file session storage.
1010
1111
Usage:
1212
client = ACPExecClient(
1313
pod_name="sandbox-abc123",
1414
namespace="onyx-sandboxes",
1515
)
1616
client.start(cwd="/workspace")
17-
for event in client.send_message("What files are here?"):
17+
session_id = client.resume_or_create_session(cwd="/workspace/sessions/abc")
18+
for event in client.send_message("What files are here?", session_id=session_id):
1819
print(event)
1920
client.stop()
2021
"""
@@ -160,7 +161,7 @@ def start(self, cwd: str = "/workspace", timeout: float = 30.0) -> None:
160161
"""Start the agent process via exec and initialize the ACP connection.
161162
162163
Only performs the ACP `initialize` handshake. Sessions are created
163-
separately via `create_session()` or `resume_session()`.
164+
separately via `resume_or_create_session()`.
164165
165166
Args:
166167
cwd: Working directory for the `opencode acp` process
@@ -502,42 +503,11 @@ def _try_resume_existing_session(self, cwd: str, timeout: float) -> str | None:
502503
)
503504
return None
504505

505-
def create_session(self, cwd: str, timeout: float = 30.0) -> str:
506-
"""Create a new ACP session on this connection.
506+
def resume_or_create_session(self, cwd: str, timeout: float = 30.0) -> str:
507+
"""Resume a session from opencode's on-disk storage, or create a new one.
507508
508-
Args:
509-
cwd: Working directory for the session
510-
timeout: Timeout for the request
511-
512-
Returns:
513-
The ACP session ID
514-
"""
515-
if not self._state.initialized:
516-
raise RuntimeError("Client not initialized. Call start() first.")
517-
return self._create_session(cwd=cwd, timeout=timeout)
518-
519-
def resume_session(self, session_id: str, cwd: str, timeout: float = 30.0) -> str:
520-
"""Resume an existing ACP session on this connection.
521-
522-
Args:
523-
session_id: The ACP session ID to resume
524-
cwd: Working directory for the session
525-
timeout: Timeout for the request
526-
527-
Returns:
528-
The ACP session ID
529-
"""
530-
if not self._state.initialized:
531-
raise RuntimeError("Client not initialized. Call start() first.")
532-
return self._resume_session(session_id=session_id, cwd=cwd, timeout=timeout)
533-
534-
def get_or_create_session(self, cwd: str, timeout: float = 30.0) -> str:
535-
"""Get an existing session for this cwd, or create/resume one.
536-
537-
Tries in order:
538-
1. Return an already-tracked session for this cwd
539-
2. Resume an existing session from opencode's storage (multi-replica)
540-
3. Create a new session
509+
With ephemeral clients (one process per message), this always hits disk.
510+
Tries resume first to preserve conversation context, falls back to new.
541511
542512
Args:
543513
cwd: Working directory for the session
@@ -549,14 +519,6 @@ def get_or_create_session(self, cwd: str, timeout: float = 30.0) -> str:
549519
if not self._state.initialized:
550520
raise RuntimeError("Client not initialized. Call start() first.")
551521

552-
# Check if we already have a session for this cwd
553-
for sid, session in self._state.sessions.items():
554-
if session.cwd == cwd:
555-
logger.info(
556-
f"[ACP] Reusing existing session: " f"acp_session={sid} cwd={cwd}"
557-
)
558-
return sid
559-
560522
# Try to resume from opencode's persisted storage
561523
resumed_id = self._try_resume_existing_session(cwd, timeout)
562524
if resumed_id:
@@ -844,11 +806,6 @@ def is_running(self) -> bool:
844806
"""Check if the exec session is running."""
845807
return self._ws_client is not None and self._ws_client.is_open()
846808

847-
@property
848-
def session_ids(self) -> list[str]:
849-
"""Get all tracked session IDs."""
850-
return list(self._state.sessions.keys())
851-
852809
def __enter__(self) -> "ACPExecClient":
853810
"""Context manager entry."""
854811
return self

backend/onyx/server/features/build/sandbox/kubernetes/kubernetes_sandbox_manager.py

Lines changed: 3 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -353,12 +353,6 @@ def _initialize(self) -> None:
353353
self._service_account = SANDBOX_SERVICE_ACCOUNT_NAME
354354
self._file_sync_service_account = SANDBOX_FILE_SYNC_SERVICE_ACCOUNT
355355

356-
# Maps (sandbox_id, craft_session_id) → ACP session ID.
357-
# Cached across messages so we can resume the same opencode session.
358-
# The actual opencode session is persisted on disk in the sandbox pod,
359-
# so this cache is just an optimization to skip session/list + resume.
360-
self._acp_session_ids: dict[tuple[UUID, UUID], str] = {}
361-
362356
# Load AGENTS.md template path
363357
build_dir = Path(__file__).parent.parent.parent # /onyx/server/features/build/
364358
self._agent_instructions_template_path = build_dir / "AGENTS.template.md"
@@ -1174,11 +1168,6 @@ def terminate(self, sandbox_id: UUID) -> None:
11741168
Args:
11751169
sandbox_id: The sandbox ID to terminate
11761170
"""
1177-
# Remove all session mappings for this sandbox
1178-
keys_to_remove = [key for key in self._acp_session_ids if key[0] == sandbox_id]
1179-
for key in keys_to_remove:
1180-
del self._acp_session_ids[key]
1181-
11821171
# Clean up Kubernetes resources (needs string for pod/service names)
11831172
self._cleanup_kubernetes_resources(str(sandbox_id))
11841173

@@ -1422,15 +1411,6 @@ def cleanup_session_workspace(
14221411
nextjs_port: Optional port where Next.js server is running (unused in K8s,
14231412
we use PID file instead)
14241413
"""
1425-
# Remove the ACP session mapping (shared client persists)
1426-
session_key = (sandbox_id, session_id)
1427-
acp_session_id = self._acp_session_ids.pop(session_key, None)
1428-
if acp_session_id:
1429-
logger.info(
1430-
f"[SANDBOX-ACP] Removed ACP session mapping: "
1431-
f"session={session_id} acp_session={acp_session_id}"
1432-
)
1433-
14341414
pod_name = self._get_pod_name(str(sandbox_id))
14351415
session_path = f"/workspace/sessions/{session_id}"
14361416

@@ -1865,43 +1845,6 @@ def _create_ephemeral_acp_client(self, sandbox_id: UUID) -> ACPExecClient:
18651845
)
18661846
return acp_client
18671847

1868-
def _get_or_create_acp_session(
1869-
self,
1870-
sandbox_id: UUID,
1871-
session_id: UUID,
1872-
acp_client: ACPExecClient,
1873-
) -> str:
1874-
"""Get the ACP session ID for a craft session, creating one if needed.
1875-
1876-
Uses the session mapping cache first, then falls back to
1877-
`get_or_create_session()` which handles resume from opencode's
1878-
persisted storage on disk.
1879-
1880-
Args:
1881-
sandbox_id: The sandbox ID
1882-
session_id: The craft session ID
1883-
acp_client: The ACP client for this message
1884-
1885-
Returns:
1886-
The ACP session ID
1887-
"""
1888-
session_key = (sandbox_id, session_id)
1889-
acp_session_id = self._acp_session_ids.get(session_key)
1890-
1891-
if acp_session_id and acp_session_id in acp_client.session_ids:
1892-
return acp_session_id
1893-
1894-
# Session not tracked or new process — get or create from disk
1895-
session_path = f"/workspace/sessions/{session_id}"
1896-
acp_session_id = acp_client.get_or_create_session(cwd=session_path)
1897-
self._acp_session_ids[session_key] = acp_session_id
1898-
1899-
logger.info(
1900-
f"[SANDBOX-ACP] Session mapped: "
1901-
f"craft_session={session_id} acp_session={acp_session_id}"
1902-
)
1903-
return acp_session_id
1904-
19051848
def send_message(
19061849
self,
19071850
sandbox_id: UUID,
@@ -1931,10 +1874,9 @@ def send_message(
19311874
acp_client = self._create_ephemeral_acp_client(sandbox_id)
19321875

19331876
try:
1934-
# Get or create the ACP session for this craft session
1935-
acp_session_id = self._get_or_create_acp_session(
1936-
sandbox_id, session_id, acp_client
1937-
)
1877+
# Resume (or create) the ACP session from opencode's on-disk storage
1878+
session_path = f"/workspace/sessions/{session_id}"
1879+
acp_session_id = acp_client.resume_or_create_session(cwd=session_path)
19381880

19391881
logger.info(
19401882
f"[SANDBOX-ACP] Sending message: "

0 commit comments

Comments
 (0)