feat(pi-task): separate WATCHED roots from WRITABLE ones, and catch a real violation
Closes the gap the reliability testing left open: no real child had ever tripped the boundary diff. T2 could not do it, and the reason is structural rather than bad luck — with read_only: true a write is DEFIANCE, and a well-behaved child refuses, so the detector never runs against a real delta. Fix: `roots` is now the WATCHED set and `write_allowed` the CHANGEABLE subset. A violation is then producible by a child that is OBEYING, which is also the realistic hazard: nobody's agent defiantly rewrites a repo, but plenty of commands leave artefacts behind. T4, run to prove it: write task in root A, plus an instruction to verify a module in root B (watched, NOT writable) with `python3 -m py_compile`. The child obeyed perfectly — status=ok, typo fixed, module compiled — and still tripped the diff, because py_compile dropped __pycache__/ into B. Exit 1, violation named, and A's authorised edit correctly NOT flagged. It also served as the in-anger test of this morning's --ignored fix: __pycache__/ is gitignored in B, so `git status --porcelain` reported B as CLEAN on the very same event that `--porcelain --ignored` caught. Pre-fix, T4 would have PASSED. The fixture test said the same thing; this says it about a real child.
This commit is contained in:
+40
-4
@@ -106,8 +106,25 @@ def boundary_delta(before: dict, after: dict) -> list:
|
||||
|
||||
|
||||
# -------------------------------------------------------------------- the brief
|
||||
def writable_roots(spec: dict) -> list:
|
||||
"""Which roots the child is allowed to change.
|
||||
|
||||
`roots` is the WATCHED set; `write_allowed` is the CHANGEABLE subset. Keeping
|
||||
them separate is what makes a violation detectable at all: if the two are the
|
||||
same list, a write task can never trip the diff, and the only way to provoke
|
||||
one is to order the child to defy its own authority line — which a
|
||||
well-behaved child simply refuses (measured 2026-09-07, test T2). The real
|
||||
hazard is not defiance anyway; it is an INCIDENTAL write, e.g. a verification
|
||||
command that drops __pycache__ into a repo the child was only meant to read.
|
||||
"""
|
||||
if spec.get("read_only", True):
|
||||
return []
|
||||
aw = spec.get("write_allowed")
|
||||
return list(aw) if aw else list(spec.get("roots") or [])
|
||||
def build_prompt(spec: dict) -> str:
|
||||
ctx = spec.get("context") or {}
|
||||
roots = spec.get("roots") or []
|
||||
allowed = writable_roots(spec)
|
||||
lines = [
|
||||
"You are a TASK WORKER invoked by another agent. You are NOT continuing a",
|
||||
"conversation and you have NO shared history: there is no 'earlier', no",
|
||||
@@ -122,7 +139,15 @@ def build_prompt(spec: dict) -> str:
|
||||
"command that mutates state (no git commit/push, no installs). A boundary",
|
||||
"diff runs after you exit and a violation fails the whole task."]
|
||||
else:
|
||||
lines += ["\n## AUTHORITY\nYou may modify files under: " + ", ".join(spec.get("roots", []))]
|
||||
lines += ["\n## AUTHORITY\nYou MAY create and modify files under:"]
|
||||
lines += [f" - {a}" for a in allowed]
|
||||
watched_only = [r for r in roots if r not in allowed]
|
||||
if watched_only:
|
||||
lines += ["You may READ these, but must NOT change anything under them:"]
|
||||
lines += [f" - {r}" for r in watched_only]
|
||||
lines += ["A boundary diff runs after you exit over every path above. ANY change",
|
||||
"outside the writable set fails the task — including one made incidentally",
|
||||
"by a command you ran rather than by an edit you intended."]
|
||||
if ctx.get("facts"):
|
||||
lines += ["\n## VERIFIED CONTEXT (asserted by the caller; you need not re-derive it)"]
|
||||
lines += [f"- {f}" for f in ctx["facts"]]
|
||||
@@ -339,8 +364,15 @@ def cmd_run(args):
|
||||
if not any(e.get("type") == "agent_end" for e in events):
|
||||
problems.append("no agent_end event — child did not complete a turn")
|
||||
delta = boundary_delta(before, after)
|
||||
if spec.get("read_only", True) and delta:
|
||||
problems.append("BOUNDARY VIOLATION: read_only task mutated its roots")
|
||||
allowed = writable_roots(spec)
|
||||
violations = [d for d in delta
|
||||
if not any(d.startswith(f"{a}:") for a in allowed)]
|
||||
if violations:
|
||||
if spec.get("read_only", True):
|
||||
problems.append("BOUNDARY VIOLATION: read_only task mutated its roots")
|
||||
else:
|
||||
problems.append("BOUNDARY VIOLATION: task changed a watched root it was "
|
||||
f"not authorised to write (authorised: {allowed or 'none'})")
|
||||
over = budget.get("usd")
|
||||
if over and cost > float(over):
|
||||
problems.append(f"cost ${cost:.4f} over budget ${float(over):.4f}")
|
||||
@@ -350,7 +382,8 @@ def cmd_run(args):
|
||||
"metrics": {"cost_usd": round(cost, 6), "elapsed_s": round(elapsed, 1),
|
||||
"tool_results": tools, "rc": rc, "model": prof["id"],
|
||||
"effort": spec["effort"]},
|
||||
"boundary_delta": delta, "audit_dir": str(audit),
|
||||
"boundary_delta": delta, "boundary_violations": violations,
|
||||
"audit_dir": str(audit),
|
||||
"raw_text_if_envelope_missing": None if env else text[-2000:]}
|
||||
(audit / "result.json").write_text(json.dumps(result, indent=2))
|
||||
|
||||
@@ -380,6 +413,9 @@ def cmd_schema(_args):
|
||||
"effort": "fast | balanced | deep (resolved via ~/.pi/agent/settings.json pi-fork.effortProfiles)",
|
||||
"read_only": True,
|
||||
"roots": ["/workspace/repo-the-task-touches"],
|
||||
"write_allowed": ["subset of roots the child may CHANGE; only meaningful when "
|
||||
"read_only is false. Defaults to all of roots, which makes "
|
||||
"violations undetectable — set it explicitly for write tasks."],
|
||||
"context": {"facts": ["verified fact the caller asserts"],
|
||||
"files": ["/abs/path the child should read itself"],
|
||||
"commands": ["exact command the child may run"]},
|
||||
|
||||
Reference in New Issue
Block a user