Run every command on the build VM over SSH (M1.3)
CI / checks (push) Skipped
CI / checks (pull_request) Failing after 55s
CI / checks (push) Skipped
CI / checks (pull_request) Failing after 55s
execution.SSHHost.execute is now the only way the agent runs anything (L13). Commands travel as a script on SSH's stdin, so the secrets handed to a call are never on a command line, and every value handed out is redacted from the output. Timeouts kill the remote process group, and an unreachable host is reported as such, not as a failed command. Workspace and GitRepo run on the build VM. The clone uses the repository's SSH URL, so no credential is stored in it (#89); fetches and pushes get the push key over stdin, in a short-lived ssh-agent, and PRs are still opened from the agent's host with its API token. The bench runs on the build host too. Tests reach "the build VM" through a throwaway local sshd (nisse_dev .testing); CI installs openssh-server. The live push test now pushes with the sandbox's deploy key (NISSE_PUSH_KEY). Fixes #89. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE
This commit is contained in:
1 parent
b7f1f3371c
commit
761ebdb857
32 files changed
+1098
-280
No files matched your search
@@ -1,26 +1,27 @@
|
||||
"""Executor tools (architecture L9: reduced schema, flat arguments).
|
||||
"""Executor tools (architecture L9: reduced schema, flat arguments; L13).
|
||||
|
||||
Deliberately unsandboxed. The VM is the trust boundary (D7) and tool use is
|
||||
auto-approved inside an approved unit of work (L3), so per-command
|
||||
confinement would add failure modes without adding safety. The workspace is
|
||||
where work happens — the default for relative paths and the command cwd — not
|
||||
a wall. Protecting agent state from the agent is a mount-layout concern
|
||||
(L8/D7), not this module's job.
|
||||
Every tool runs on the work item's build VM over SSH (execution.py); the
|
||||
agent's own host never runs a command a model chose. Within the VM nothing is
|
||||
sandboxed: the VM is the trust boundary (D7), so per-command confinement would
|
||||
add failure modes without adding safety. The workspace is where work happens,
|
||||
the default for relative paths and the command's directory, not a wall.
|
||||
|
||||
What *is* enforced here is operational: commands cannot hang the loop, cannot
|
||||
consume the context window with megabytes of build log, and do not inherit
|
||||
the agent's own credentials.
|
||||
What *is* enforced here is operational: commands cannot hang the loop and
|
||||
cannot flood the context window with megabytes of build log. The agent's own
|
||||
credentials are never on the build VM to inherit.
|
||||
"""
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import signal
|
||||
import subprocess
|
||||
import base64
|
||||
import io
|
||||
import shlex
|
||||
import tarfile
|
||||
from collections.abc import Mapping
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from nisse_dev.context import head_and_tail
|
||||
from nisse_dev.execution import HostUnavailable, Result, SSHHost
|
||||
|
||||
# Tools that change or verify something. Reading a file or listing a
|
||||
# directory is a model looking around, not doing the work, so neither counts
|
||||
@@ -88,12 +89,6 @@ EXECUTOR_TOOLS: list[dict[str, Any]] = [
|
||||
]
|
||||
|
||||
|
||||
def _child_env() -> dict[str, str]:
|
||||
"""The project's build and test code is arbitrary; it has no business
|
||||
reading the agent's LiteLLM, Matrix, or Gitea credentials."""
|
||||
return {k: v for k, v in os.environ.items() if not k.startswith("NISSE_")}
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ToolOutcome:
|
||||
result: str
|
||||
@@ -101,30 +96,53 @@ class ToolOutcome:
|
||||
|
||||
|
||||
class Workspace:
|
||||
"""The work item's workspace: a directory on its tool VM (L13, D3).
|
||||
|
||||
Every operation runs there over SSH; nothing touches the agent's own
|
||||
host. ``root`` is the path on that host.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
root: Path,
|
||||
host: SSHHost,
|
||||
root: Path | str,
|
||||
*,
|
||||
timeout_s: int = DEFAULT_TIMEOUT_S,
|
||||
max_output_chars: int = MAX_OUTPUT_CHARS,
|
||||
secrets: Mapping[str, str] | None = None,
|
||||
) -> None:
|
||||
self.host = host
|
||||
self.root = Path(root)
|
||||
self.root.mkdir(parents=True, exist_ok=True)
|
||||
self._timeout_s = timeout_s
|
||||
self._max_output_chars = max_output_chars
|
||||
# The project secrets the model's commands may see (D7); redacted from
|
||||
# whatever comes back.
|
||||
self._secrets = dict(secrets or {})
|
||||
self._run(f"mkdir -p -- {shlex.quote(str(self.root))}")
|
||||
|
||||
def _run(
|
||||
self, command: str, *, stdin: str | None = None, cwd: str | None = None
|
||||
) -> Result:
|
||||
return self.host.execute(
|
||||
command, cwd=cwd, stdin=stdin, timeout_s=self._timeout_s
|
||||
)
|
||||
|
||||
def _truncate(self, text: str) -> str:
|
||||
return head_and_tail(text, self._max_output_chars)
|
||||
|
||||
def _resolve(self, path: str) -> Path:
|
||||
def _resolve(self, path: str) -> str:
|
||||
candidate = Path(path)
|
||||
return candidate if candidate.is_absolute() else self.root / candidate
|
||||
return str(candidate if candidate.is_absolute() else self.root / candidate)
|
||||
|
||||
def _inside(self, target: Path) -> bool:
|
||||
try:
|
||||
return target.resolve().is_relative_to(self.root.resolve())
|
||||
except OSError:
|
||||
def _inside(self, target: str) -> bool:
|
||||
result = self._run(
|
||||
f"realpath -m -- {shlex.quote(target)} {shlex.quote(str(self.root))}"
|
||||
)
|
||||
lines = result.stdout.splitlines()
|
||||
if not result.ok or len(lines) != 2:
|
||||
return False
|
||||
resolved, root = (Path(line) for line in lines)
|
||||
return resolved == root or root in resolved.parents
|
||||
|
||||
def dispatch(self, tool: str, arguments: dict[str, Any]) -> ToolOutcome:
|
||||
try:
|
||||
@@ -143,13 +161,24 @@ class Workspace:
|
||||
return ToolOutcome(f"no such tool: {tool}", success=False)
|
||||
except KeyError as exc:
|
||||
return ToolOutcome(f"missing argument {exc} for {tool}", success=False)
|
||||
except HostUnavailable as exc:
|
||||
# Not the work failing: the build VM is out of reach.
|
||||
return ToolOutcome(f"the build VM is unreachable: {exc}", success=False)
|
||||
|
||||
def read_file(self, path: str) -> ToolOutcome:
|
||||
target = self._resolve(path)
|
||||
# base64 on the wire, so the bytes arrive intact and decoding is
|
||||
# strict here, as it was for a local file.
|
||||
result = self._run(f"base64 -w0 -- {shlex.quote(target)}")
|
||||
if not result.ok:
|
||||
return ToolOutcome(
|
||||
f"could not read {path}: {result.stderr.strip()}", success=False
|
||||
)
|
||||
try:
|
||||
return ToolOutcome(self._truncate(target.read_text()), success=True)
|
||||
except (OSError, UnicodeDecodeError) as exc:
|
||||
text = base64.b64decode(result.stdout).decode()
|
||||
except (ValueError, UnicodeDecodeError) as exc:
|
||||
return ToolOutcome(f"could not read {path}: {exc}", success=False)
|
||||
return ToolOutcome(self._truncate(text), success=True)
|
||||
|
||||
def write_file(self, path: str, content: str) -> ToolOutcome:
|
||||
target = self._resolve(path)
|
||||
@@ -165,21 +194,25 @@ class Workspace:
|
||||
"relative paths.",
|
||||
success=False,
|
||||
)
|
||||
try:
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
target.write_text(content)
|
||||
except OSError as exc:
|
||||
return ToolOutcome(f"could not write {path}: {exc}", success=False)
|
||||
quoted = shlex.quote(target)
|
||||
result = self._run(
|
||||
f'mkdir -p -- "$(dirname -- {quoted})" && base64 -d > {quoted}',
|
||||
stdin=base64.b64encode(content.encode()).decode(),
|
||||
)
|
||||
if not result.ok:
|
||||
return ToolOutcome(
|
||||
f"could not write {path}: {result.stderr.strip()}", success=False
|
||||
)
|
||||
return ToolOutcome(f"wrote {len(content)} characters to {path}", success=True)
|
||||
|
||||
def list_dir(self, path: str) -> ToolOutcome:
|
||||
target = self._resolve(path)
|
||||
try:
|
||||
entries = sorted(
|
||||
f"{p.name}/" if p.is_dir() else p.name for p in target.iterdir()
|
||||
result = self._run(f"ls -1Ap -- {shlex.quote(target)}")
|
||||
if not result.ok:
|
||||
return ToolOutcome(
|
||||
f"could not list {path}: {result.stderr.strip()}", success=False
|
||||
)
|
||||
except OSError as exc:
|
||||
return ToolOutcome(f"could not list {path}: {exc}", success=False)
|
||||
entries = sorted(line for line in result.stdout.splitlines() if line)
|
||||
return ToolOutcome(
|
||||
self._truncate("\n".join(entries) or "(empty)"), success=True
|
||||
)
|
||||
@@ -187,56 +220,48 @@ class Workspace:
|
||||
def describe(self) -> str:
|
||||
"""Told to the model, so it does not have to guess where it is."""
|
||||
return (
|
||||
f"Your workspace is {self.root}. It is a git repository and the "
|
||||
"only place your work is recorded; commands run there by default. "
|
||||
"Use relative paths."
|
||||
f"Your workspace is {self.root} on the build VM. It is a git "
|
||||
"repository and the only place your work is recorded; commands run "
|
||||
"there by default. Use relative paths."
|
||||
)
|
||||
|
||||
def run_command(self, command: str) -> ToolOutcome:
|
||||
try:
|
||||
# Popen rather than run(): on timeout, run() kills only the direct
|
||||
# child, and TimeoutExpired carries no pid, so a backgrounded
|
||||
# grandchild (a dev server, say) would survive. Holding the pid
|
||||
# lets us kill the whole session.
|
||||
process = subprocess.Popen(
|
||||
result = self.host.execute(
|
||||
command,
|
||||
shell=True,
|
||||
cwd=self.root,
|
||||
env=_child_env(),
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
stdin=subprocess.DEVNULL,
|
||||
text=True,
|
||||
start_new_session=True,
|
||||
cwd=str(self.root),
|
||||
secrets=self._secrets,
|
||||
timeout_s=self._timeout_s,
|
||||
)
|
||||
except OSError as exc:
|
||||
return ToolOutcome(f"could not run command: {exc}", success=False)
|
||||
try:
|
||||
stdout, stderr = process.communicate(timeout=self._timeout_s)
|
||||
except subprocess.TimeoutExpired:
|
||||
self._kill_group(process)
|
||||
except HostUnavailable as exc:
|
||||
return ToolOutcome(f"the build VM is unreachable: {exc}", success=False)
|
||||
if result.timed_out:
|
||||
return ToolOutcome(
|
||||
f"command timed out after {self._timeout_s}s: {command}",
|
||||
success=False,
|
||||
)
|
||||
body = self._truncate(
|
||||
f"exit code: {process.returncode}\nstdout:\n{stdout}\nstderr:\n{stderr}"
|
||||
f"exit code: {result.exit_code}\nstdout:\n{result.stdout}"
|
||||
f"\nstderr:\n{result.stderr}"
|
||||
)
|
||||
return ToolOutcome(body, success=process.returncode == 0)
|
||||
return ToolOutcome(body, success=result.exit_code == 0)
|
||||
|
||||
@staticmethod
|
||||
def _kill_group(process: "subprocess.Popen[str]") -> None:
|
||||
try:
|
||||
os.killpg(os.getpgid(process.pid), signal.SIGKILL)
|
||||
except (ProcessLookupError, PermissionError):
|
||||
process.kill()
|
||||
# Drain the pipes of the now-dead group so no zombie is left behind.
|
||||
try:
|
||||
process.communicate(timeout=5)
|
||||
except subprocess.TimeoutExpired:
|
||||
pass
|
||||
def upload_tree(self, local: Path, dest: str = ".") -> None:
|
||||
"""Copy a local directory's contents into the workspace (the bench
|
||||
seeding a task); raises RuntimeError if it fails."""
|
||||
buffer = io.BytesIO()
|
||||
with tarfile.open(fileobj=buffer, mode="w:gz") as archive:
|
||||
for entry in sorted(Path(local).iterdir()):
|
||||
archive.add(entry, arcname=entry.name)
|
||||
target = shlex.quote(self._resolve(dest))
|
||||
result = self._run(
|
||||
f"mkdir -p -- {target} && base64 -d | tar -xzf - -C {target}",
|
||||
stdin=base64.b64encode(buffer.getvalue()).decode(),
|
||||
)
|
||||
if not result.ok:
|
||||
raise RuntimeError(f"upload to {dest} failed: {result.stderr.strip()}")
|
||||
|
||||
def reset(self) -> None:
|
||||
"""Discard workspace contents; used by tests, not by the agent."""
|
||||
shutil.rmtree(self.root, ignore_errors=True)
|
||||
self.root.mkdir(parents=True, exist_ok=True)
|
||||
root = shlex.quote(str(self.root))
|
||||
self._run(f"rm -rf -- {root} && mkdir -p -- {root}")
|
||||
Reference in new issue
Block a user