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
27 changes: 27 additions & 0 deletions scripts/build_runtime_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,13 @@ def rewrite_imports(text: str, regex: re.Pattern) -> str:
`import X` → `import molecule_runtime.X as X` (preserve binding)
`from X import Y` → `from molecule_runtime.X import Y`
`from X.sub import Y` → `from molecule_runtime.X.sub import Y`

Rejects `import X as Y` because the rewrite would produce
`import molecule_runtime.X as X as Y`, a syntax error. The PR #2433
incident shipped this exact pattern past `Python Lint & Test` (which
runs against pre-rewrite source) but blew up the wheel-smoke gate.
Detecting it here turns the silent build failure into a build-time
error with a clear path: use `from X import …` or plain `import X`.
"""
def repl(m: re.Match) -> str:
indent, kw, mod, rest = m.group("indent"), m.group("kw"), m.group("mod"), m.group("rest")
Expand All @@ -163,6 +170,26 @@ def repl(m: re.Match) -> str:
# `import X.sub` — rewrite as `import molecule_runtime.X.sub` and
# leave the trailing dot pattern intact for the rest of the line.
return f"{indent}import molecule_runtime.{mod}{rest}"
# Detect `import X as Y` — the regex's `rest` group captures only
# the immediate following char (whitespace, comma, or EOL), so we
# have to peek at the surrounding line context. The match start is
# at the line's `import` keyword; everything after the matched
# name on the same line is what the source author wrote.
line_start = text.rfind("\n", 0, m.start()) + 1
line_end = text.find("\n", m.end())
if line_end == -1:
line_end = len(text)
line_after = text[m.end() - len(rest):line_end]
# Strip comments from consideration so `import X # noqa` doesn't trip.
line_after_no_comment = line_after.split("#", 1)[0]
if re.search(r"^\s*as\s+\w+", line_after_no_comment):
raise ValueError(
f"rewrite_imports: cannot rewrite 'import {mod} as <alias>' on a "
f"workspace module — the regex would produce "
f"'import molecule_runtime.{mod} as {mod} as <alias>', invalid syntax. "
f"Use 'from {mod} import …' or plain 'import {mod}' instead. "
f"Offending line: {text[line_start:line_end]!r}"
)
# Plain `import X` — alias preserves the local name.
return f"{indent}import molecule_runtime.{mod} as {mod}{rest}"
return regex.sub(repl, text)
Expand Down
2 changes: 2 additions & 0 deletions scripts/wheel_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ def smoke_imports_and_invariants() -> None:
InboxState,
activate as inbox_activate,
get_state as inbox_get_state,
set_notification_callback as inbox_set_notification_callback,
start_poller_thread as inbox_start_poller_thread,
)
assert callable(inbox_activate), "inbox.activate must be callable"
assert callable(inbox_get_state), "inbox.get_state must be callable"
assert callable(inbox_start_poller_thread), "inbox.start_poller_thread must be callable"
assert callable(inbox_set_notification_callback), "inbox.set_notification_callback must be callable"

assert a2a_client._A2A_ERROR_PREFIX, "a2a_client missing error sentinel"
assert callable(get_adapter), "adapters.get_adapter must be callable"
Expand Down
14 changes: 14 additions & 0 deletions workspace-server/internal/handlers/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,26 @@ var openTunnelCmd = func(o eicSSHOptions) *exec.Cmd {
// to 22; with CP provisioning today the workspace runs as a native
// process under the ubuntu user, so landing at ubuntu's shell IS the
// terminal experience.
//
// ConnectTimeout=10 is the user-experience guard — without it, ssh waits
// indefinitely for the remote sshd's banner. When the workspace EC2's
// sshd is unresponsive (mid-restart, SG drop, AMI without ec2-instance-
// connect installed) the canvas's xterm shows the user's typed bytes
// echoed back by the workspace-server's *local* PTY (cooked + echo mode
// before ssh finishes its handshake) and then closes silently when CF's
// idle WebSocket timer fires, with no "Connection refused" or "Permission
// denied" output ever reaching the user. Capping at 10s makes the failure
// surface as a real ssh error message in the terminal — caught 2026-04-30
// when hongmingwang's hermes shell hung after the heartbeat-fix redeploy
// and a probe at /workspaces/<id>/terminal sat for 60s with the only
// frame being the local-PTY echo of a single 'X' typed mid-handshake.
var sshCommandCmd = func(o eicSSHOptions) *exec.Cmd {
return exec.Command(
"ssh",
"-i", o.PrivateKeyPath,
"-o", "StrictHostKeyChecking=no",
"-o", "UserKnownHostsFile=/dev/null",
"-o", "ConnectTimeout=10",
"-o", "ServerAliveInterval=30",
"-o", "ServerAliveCountMax=3",
"-p", fmt.Sprintf("%d", o.LocalPort),
Expand Down
55 changes: 55 additions & 0 deletions workspace-server/internal/handlers/terminal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ func TestSSHCommandCmd_BuildsArgv(t *testing.T) {
"-i", "/tmp/k",
"-o", "StrictHostKeyChecking=no",
"-o", "UserKnownHostsFile=/dev/null",
"-o", "ConnectTimeout=10",
"-o", "ServerAliveInterval=30",
"-o", "ServerAliveCountMax=3",
"-p", "2222",
Expand Down Expand Up @@ -490,3 +491,57 @@ func TestKI005_OrgToken_SkipsValidateToken(t *testing.T) {
}
}

// TestSSHCommandCmd_ConnectTimeoutPresent pins the user-experience guard
// against ssh-handshake-hang. Without ConnectTimeout, ssh waits forever
// for the remote sshd's banner — which masquerades as a "silently dead"
// shell to the user, because the workspace-server's local PTY is in
// cooked + echo mode before ssh finishes its handshake, so the canvas
// echoes the user's keystrokes back without ever reaching remote bash,
// and Cloudflare eventually closes the WebSocket on idle (~100s) with
// no error frame to surface what went wrong.
//
// Repro 2026-04-30: a 60s probe at hongmingwang's hermes /terminal
// endpoint after the heartbeat-fix redeploy showed only the local-PTY
// echo of a single 'X' typed mid-handshake. Workspace EC2 was up and
// heartbeating but its sshd was unresponsive; ssh hung indefinitely.
//
// Behavior-based: matches the literal `-o ConnectTimeout=N` arg pair so
// this stays pinned even if the rest of the args reorder. Does not pin
// the exact value — operators may tune it — but does pin presence.
func TestSSHCommandCmd_ConnectTimeoutPresent(t *testing.T) {
t.Parallel()

cmd := sshCommandCmd(eicSSHOptions{
InstanceID: "i-test",
OSUser: "ubuntu",
Region: "us-east-2",
LocalPort: 2222,
PrivateKeyPath: "/tmp/test-key",
})

args := cmd.Args
found := false
for i, a := range args {
if a != "-o" {
continue
}
if i+1 >= len(args) {
continue
}
val := args[i+1]
if len(val) >= len("ConnectTimeout=") &&
val[:len("ConnectTimeout=")] == "ConnectTimeout=" {
found = true
break
}
}
if !found {
t.Errorf("sshCommandCmd is missing `-o ConnectTimeout=N` — without it, "+
"ssh hangs forever when the workspace EC2's sshd is unresponsive "+
"and the canvas terminal silently dies on Cloudflare's idle WS "+
"timeout with no error message reaching the user. See terminal.go "+
"sshCommandCmd comment (2026-04-30 hongmingwang hermes). args=%v",
args)
}
}

70 changes: 70 additions & 0 deletions workspace/a2a_mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
import logging
import sys

import inbox # noqa: F401 — bridge wiring lives in main(); the rewriter
# produces `import molecule_runtime.inbox as inbox`
# which preserves this binding for set_notification_callback.

from a2a_tools import (
tool_check_task_status,
tool_commit_memory,
Expand Down Expand Up @@ -130,6 +134,44 @@ async def handle_tool_call(name: str, arguments: dict) -> str:
return f"Unknown tool: {name}"


# --- MCP Notification bridge ---

# `notifications/claude/channel` matches the contract used by the
# molecule-mcp-claude-channel bun bridge (server.ts:509). Claude Code's
# MCP runtime treats this method as a conversation interrupt — `content`
# becomes the agent turn, `meta` is structured metadata. Notification-
# capable hosts (Claude Code today; any compliant client tomorrow)
# get push UX automatically; pollers (`wait_for_message` / `inbox_peek`)
# still work unchanged. See task #46 + the deprecation path documented
# in workspace/inbox.py:set_notification_callback.
_CHANNEL_NOTIFICATION_METHOD = "notifications/claude/channel"


def _build_channel_notification(msg: dict) -> dict:
"""Transform an ``InboxMessage.to_dict()`` into the MCP notification
envelope expected by Claude Code's channel-bridge contract.

Pure function so the wire shape is unit-testable without spinning
up an asyncio loop. The wire-up in ``main()`` just composes this
with ``asyncio.run_coroutine_threadsafe``.
"""
return {
"jsonrpc": "2.0",
"method": _CHANNEL_NOTIFICATION_METHOD,
"params": {
"content": msg.get("text", ""),
"meta": {
"source": "molecule",
"kind": msg.get("kind", ""),
"peer_id": msg.get("peer_id", ""),
"method": msg.get("method", ""),
"activity_id": msg.get("activity_id", ""),
"ts": msg.get("created_at", ""),
},
},
}


# --- MCP Server (JSON-RPC over stdio) ---

async def main(): # pragma: no cover
Expand All @@ -148,6 +190,34 @@ async def write_response(response: dict):
writer.write(data.encode())
await writer.drain()

# Wire the inbox → MCP notification bridge. Inbox poller (daemon
# thread) calls into here when a new activity row lands; we
# schedule the notification onto the asyncio loop and best-effort
# fire it on the same stdout the responses go to.
loop = asyncio.get_running_loop()

async def _emit_notification(payload: dict) -> None:
data = json.dumps(payload) + "\n"
writer.write(data.encode())
try:
await writer.drain()
except Exception: # noqa: BLE001
# Closed pipe (host disconnected) shouldn't crash the
# inbox poller; let it sit until the host reconnects.
pass

def _on_inbox_message(msg: dict) -> None:
try:
asyncio.run_coroutine_threadsafe(
_emit_notification(_build_channel_notification(msg)),
loop,
)
except RuntimeError:
# Loop closed during shutdown — best-effort, swallow.
pass

inbox.set_notification_callback(_on_inbox_message)

buffer = ""
while True:
try:
Expand Down
14 changes: 13 additions & 1 deletion workspace/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,19 @@ def load_config(config_path: Optional[str] = None) -> WorkspaceConfig:
args=runtime_raw.get("args", []),
required_env=runtime_raw.get("required_env", []),
timeout=runtime_raw.get("timeout", 0),
model=runtime_raw.get("model", ""),
# Fall back to top-level resolved `model` (which already honors
# MODEL_PROVIDER env override, line 277) when YAML doesn't carry
# runtime_config.model. Without this fallback, SaaS workspaces
# silently boot with the adapter's hard-coded default —
# claude-code-default reads `runtime_config.model or "sonnet"`,
# so a user who picks Opus in the canvas Config tab gets Sonnet
# on the next CP-driven restart. Root cause: the CP user-data
# script regenerates /configs/config.yaml at every boot with
# only `name`, `runtime`, `a2a` keys (intentionally minimal so
# it doesn't carry stale state), losing runtime_config.model.
# MODEL_PROVIDER is plumbed as an env var, so picking it up via
# the top-level resolved model keeps the selection sticky.
model=runtime_raw.get("model") or model,
# Deprecated fields — kept for backward compat
auth_token_env=runtime_raw.get("auth_token_env", ""),
auth_token_file=runtime_raw.get("auth_token_file", ""),
Expand Down
52 changes: 49 additions & 3 deletions workspace/inbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
from collections import deque
from dataclasses import dataclass, field
from pathlib import Path
from typing import Any
from typing import Any, Callable

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -173,17 +173,34 @@ def reset_cursor(self) -> None:
logger.warning("inbox: failed to delete cursor %s: %s", self.cursor_path, exc)

def record(self, message: InboxMessage) -> None:
"""Append a message and wake any waiter.
"""Append a message, wake any waiter, and fire the notification
callback (if registered) for push-UX-capable hosts.

Skips a row whose activity_id we've already queued — defensive
against the poller racing with the consumer + cursor save.
against the poller racing with the consumer + cursor save. The
dedupe short-circuits BEFORE the notification fires, so a
notification-capable host doesn't see duplicate push events on
backlog overlap.
"""
with self._lock:
for existing in self._queue:
if existing.activity_id == message.activity_id:
return
self._queue.append(message)
self._arrival.set()
# Fire notification AFTER releasing the lock so the callback
# is free to do anything (including calling back into inbox)
# without deadlock. Best-effort: a raising callback must not
# prevent the message from landing in the queue — observability
# is more important than push delivery.
cb = _NOTIFICATION_CALLBACK
if cb is not None:
try:
cb(message.to_dict())
except Exception:
logger.warning(
"inbox: notification callback raised", exc_info=True
)

def peek(self, limit: int = 10) -> list[InboxMessage]:
"""Return up to ``limit`` pending messages without removing them."""
Expand Down Expand Up @@ -240,6 +257,35 @@ def wait(self, timeout_secs: float) -> InboxMessage | None:
_STATE: InboxState | None = None


# Notification bridge — set by the universal MCP server (a2a_mcp_server.py)
# at startup so that new inbox arrivals can be pushed to notification-
# capable hosts (Claude Code) as MCP `notifications/claude/channel`
# events. Kept module-level (rather than a method on InboxState) so the
# inbox doesn't need to know about MCP — a thin pluggable seam.
#
# Defaults to None: in-container runtimes that don't activate the inbox
# also don't push notifications, and tests start clean. The wheel's
# wiring is exercised by tests/test_a2a_mcp_server.py + the bridge
# tests below.
_NOTIFICATION_CALLBACK: Callable[[dict], None] | None = None


def set_notification_callback(cb: Callable[[dict], None] | None) -> None:
"""Register (or clear) the per-message notification callback.

The callback receives ``InboxMessage.to_dict()`` for each new
arrival — same shape ``inbox_peek`` returns to the agent, so a
bridge can build its MCP notification payload without re-deriving
fields.

Best-effort: a raising callback does NOT prevent the message from
landing in the queue (see ``InboxState.record``). Pass ``None`` to
clear (used by tests + the wheel's shutdown path).
"""
global _NOTIFICATION_CALLBACK
_NOTIFICATION_CALLBACK = cb


def activate(state: InboxState) -> None:
"""Register an InboxState as the singleton this module exposes.

Expand Down
Loading
Loading