Skip to content

Commit 110b813

Browse files
A sandboxed child that dies without sending names the tool it was running
`poll()` returns true when the pipe is readable, and a closed pipe is readable. So a child that died without sending — `os._exit`, a segfault, an OOM-kill, anything that kills the process rather than raising inside `spec.fn` — fell through the timeout guard into `recv()` and raised a bare `EOFError('')`. `run` has two failure shapes and that was neither: `SandboxViolation` for a confinement breach, `RuntimeError(f"tool {name!r} failed: ...")` for the tool's own exception. An EOFError with an empty message is attributable to nothing. Downstream is where it bit. `AgentNode`'s loop catches it under its blanket `except Exception` and renders `f"TOOL_ERROR: {exc}"` — and `str(EOFError(''))` is `''`, so the model was handed `TOOL_ERROR:` with nothing after the colon. It was told its call failed and given no way to tell why, which tool, or whether a retry could help; the same text went into the trace, so the audit trail could not explain the failure either. The `_CALL_SHAPE_ERROR` hint below that clause cannot fire on it, since it matches on message text and there is none. This is the shape of a defect already closed here once: a curtailed phase writing `[budget_exhausted] ` with nothing after it. Same empty message, one layer down. Now a `RuntimeError` naming the tool and the exit code, with a negative one rendered as the signal that killed it — which is what tells an OOM-kill apart from a deliberate `_exit`. Deliberately not a `SandboxViolation`: a child dying is not evidence it tried to escape confinement, and a violation is a specific accusation that lands in the trace as one. The three new tests go red without the fix, with the EOFError raising out of `multiprocessing/connection.py` exactly as reported. The normal return, the tool-raises and the timeout paths are untouched. Closes #111 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 00e0c7d commit 110b813

3 files changed

Lines changed: 94 additions & 2 deletions

File tree

docs/deep-dive.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ A stable system is not one that claims to have no edges — it is one whose edge
254254
- **`.env` and `grapharc.toml` follow the same discovery rule: the working directory, and nowhere else.** Neither searches parent directories — a run must not be governed by a file you did not know about, and must not be *billed* to one either. **This is a behaviour change:** the credential loader used to walk up to `/`, so a `.env` in an ancestor directory (a `$HOME` one on a shared box, a client project one above a demo checkout) was picked up silently. If you relied on that, move the file into the directory you run from, `export` the variable, or pass `env_file=` to name it explicitly. A real environment variable still beats any file.
255255
- **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.
256256

257-
**Verified this pass:** `pytest` → green, 2,145 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.6` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
257+
**Verified this pass:** `pytest` → green, 2,148 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.6` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
258258

259259
[ROADMAP.md](../ROADMAP.md) tracks what is built and what is not, item by item.
260260

grapharc/harness/executor.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,32 @@ def run(self, spec: ToolSpec, args: dict[str, Any]) -> Any:
653653
raise SandboxViolation(
654654
f"tool {spec.name!r} exceeded its {spec.timeout_seconds}s timeout"
655655
)
656-
kind, payload = parent_conn.recv()
656+
try:
657+
kind, payload = parent_conn.recv()
658+
except EOFError:
659+
# `poll()` is true at EOF as well as on a readable message, so a
660+
# child that died without sending — `os._exit`, a segfault, an
661+
# OOM-kill, a guard that killed the process rather than raising —
662+
# lands here. It used to escape as a bare `EOFError('')`, which
663+
# names nothing: not the tool, not the cause. The agent loop
664+
# catches it under its blanket `except Exception` and reports
665+
# `TOOL_ERROR: ` with nothing after the colon, so a model is told
666+
# its call failed and given no way to tell why or self-correct.
667+
#
668+
# Reported as a tool failure rather than a violation: the child
669+
# dying is not evidence it tried to escape confinement, and a
670+
# `SandboxViolation` is a specific accusation. The exit code is
671+
# what distinguishes the cases, so it is in the message; a
672+
# negative one is the signal that killed it.
673+
proc.join(5)
674+
code = proc.exitcode
675+
signal_note = (
676+
f" (killed by signal {-code})" if code is not None and code < 0 else ""
677+
)
678+
raise RuntimeError(
679+
f"tool {spec.name!r} failed: the sandboxed child exited without "
680+
f"sending a result, exit code {code}{signal_note}"
681+
) from None
657682
proc.join(5)
658683
if kind == "violation":
659684
raise SandboxViolation(payload)

tests/test_harness_gate.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import ctypes # noqa: F401
1515
import os
1616
import shutil
17+
import signal
1718
import sqlite3 # noqa: F401
1819
import sys
1920
import sysconfig
@@ -40,6 +41,15 @@ def _echo(**kwargs):
4041
return kwargs
4142

4243

44+
def _exits_without_sending(**kwargs):
45+
"""A child that dies mid-tool, standing in for a crash or an OOM-kill."""
46+
os._exit(3)
47+
48+
49+
def _killed_by_signal(**kwargs):
50+
os.kill(os.getpid(), signal.SIGKILL)
51+
52+
4353
def _policy(rules):
4454
return PermissionPolicy(rules=[PermissionRule(**r) for r in rules])
4555

@@ -206,6 +216,63 @@ def read_secret(path: str) -> str:
206216
harness.call("read_secret", {"path": str(secret)})
207217

208218

219+
@pytest.mark.skipif(
220+
not hasattr(sys, "addaudithook") or sys.platform == "win32",
221+
reason="requires POSIX fork + audit hooks",
222+
)
223+
@pytest.mark.parametrize(
224+
("label", "body"),
225+
[
226+
pytest.param("exit", _exits_without_sending, id="clean-exit"),
227+
pytest.param("kill", _killed_by_signal, id="sigkill"),
228+
],
229+
)
230+
def test_a_sandboxed_child_that_dies_without_sending_names_the_tool(
231+
tmp_path, label, body
232+
):
233+
"""`poll()` is true at EOF as well as on a readable message.
234+
235+
So a child that died without sending — `os._exit`, a segfault, an OOM-kill
236+
— reached `recv()` and raised a bare `EOFError('')`. That names nothing:
237+
not the tool, not the cause. The agent loop catches it under its blanket
238+
`except Exception` and reports `TOOL_ERROR: ` with *nothing after the
239+
colon*, so a model is told its call failed and given no way to tell why.
240+
"""
241+
workspace = tmp_path / "ws"
242+
workspace.mkdir()
243+
reg = ToolRegistry()
244+
reg.register(ToolSpec(name="dies", description="", fn=body))
245+
policy = _policy([{"action": "allow", "pattern": "dies"}])
246+
harness = Harness(reg, policy, workspace=str(workspace))
247+
248+
with pytest.raises(RuntimeError) as caught:
249+
harness.call("dies", {})
250+
251+
message = str(caught.value)
252+
assert "dies" in message, message
253+
assert "exited without sending a result" in message, message
254+
# Reported as a tool failure, not an accusation: a child dying is not
255+
# evidence it tried to escape confinement.
256+
assert not isinstance(caught.value, SandboxViolation)
257+
258+
259+
@pytest.mark.skipif(
260+
not hasattr(sys, "addaudithook") or sys.platform == "win32",
261+
reason="requires POSIX fork + audit hooks",
262+
)
263+
def test_a_signal_death_is_distinguishable_from_a_clean_exit(tmp_path):
264+
"""The exit code is what tells the two apart, so it has to be in the text."""
265+
workspace = tmp_path / "ws"
266+
workspace.mkdir()
267+
reg = ToolRegistry()
268+
reg.register(ToolSpec(name="killed", description="", fn=_killed_by_signal))
269+
policy = _policy([{"action": "allow", "pattern": "killed"}])
270+
harness = Harness(reg, policy, workspace=str(workspace))
271+
272+
with pytest.raises(RuntimeError, match="killed by signal 9"):
273+
harness.call("killed", {})
274+
275+
209276
@pytest.mark.skipif(
210277
not hasattr(sys, "addaudithook") or sys.platform == "win32",
211278
reason="requires POSIX fork + audit hooks",

0 commit comments

Comments
 (0)