diff --git a/tests/acceptance/hand_started.py b/tests/acceptance/hand_started.py index 5237e60..43cdf8a 100644 --- a/tests/acceptance/hand_started.py +++ b/tests/acceptance/hand_started.py @@ -489,6 +489,103 @@ def codex_rollout(workspace: Path, since: float) -> Path | None: return None +@dataclass(frozen=True) +class Policy: + """The ground a permission was measured on, and whether it is the right ground. + + Two fields rather than one string, because the `approval` step has to do two + different things with them. `named` is what its evidence line says. `unsound` + is empty when the ground is the one the lane meant, and otherwise says what is + wrong with it — and a step that cannot stand on its own ground has not proved + what it claims, however green the round trip looked. + """ + + named: str + unsound: str = "" + + +#: What the Codex lane's `approval` step must be standing on for its green to +#: mean what it says. The sandbox is the harness's own pin — without it the write +#: it relays is not refused and no permission is raised at all. The policy and +#: the reviewer are the **product's** pin, asserted on every turn it starts +#: (`agent/codex/threads.py:36-40`), and both are checked for *equality* rather +#: than against a list of known-bad values: refusing only `never` would pass +#: `None`, `on-failure` and `untrusted` alike, and none of those is the pin #77 +#: built — a step that cannot tell them apart cannot claim the pin was applied. +#: Any reviewer but `user` means prompts are answered by a subagent and never +#: reach the person, which is the product's own `MISROUTED`. +WANTED_SANDBOX = "workspace-write" +WANTED_APPROVAL_POLICY = "on-request" +WANTED_REVIEWER = "user" + + +def codex_turn_policy(rollout: Path | None) -> Policy: + """What Codex says the last turn ran under, in Codex's own words. + + Every turn appends a `turn_context` record carrying `approval_policy`, + `approvals_reviewer` and `sandbox_policy` (measured 2026-08-27 on codex-cli + 0.149.1 and 0.150.0, including on the run that opened #105). The `approval` + step names this rather than the flag the harness passed, because two of the + three are not the harness's to claim: the **product** pins the policy and the + reviewer on every turn it starts, so reading them back off the far side is + the difference between a run that says the pin was applied and a run that + assumes it. + + **A policy that cannot be read is a failure, not a footnote.** Returning the + words "no policy" and letting the step pass would leave exactly the hole + #105 was opened on: a green `approval` that never says which ground it stood + on reads the same as one standing on ground where no permission could have + been raised. So every unreadable case, and every case that contradicts the + ground this lane pinned, comes back `unsound`. + + The last record wins: it is the turn whose permission the step just graded. + """ + if rollout is None: + return Policy("no policy", "codex has written no record of this Session to read one from") + latest: dict | None = None + try: + with rollout.open() as lines: + for line in lines: + try: + record = json.loads(line) + except json.JSONDecodeError: + continue + if record.get("type") == "turn_context" and isinstance(record.get("payload"), dict): + latest = record["payload"] + except OSError as unreadable: + return Policy("no policy", f"{rollout.name} could not be read ({unreadable})") + if latest is None: + return Policy("no policy", f"{rollout.name} carries no turn_context record") + + sandbox = latest.get("sandbox_policy") + sandbox_kind = sandbox.get("type") if isinstance(sandbox, dict) else sandbox + approval_policy = latest.get("approval_policy") + reviewer = latest.get("approvals_reviewer") + named = ( + f"sandbox {sandbox_kind!r}, approval_policy {approval_policy!r}, " + f"approvals_reviewer {reviewer!r} " + f"(codex's own `turn_context`, {rollout.name})" + ) + wrong = [] + if sandbox_kind != WANTED_SANDBOX: + wrong.append( + f"the sandbox is {sandbox_kind!r}, not the {WANTED_SANDBOX!r} this lane pins — " + f"the write it relays is only refused, and a permission only raised, inside that one" + ) + if approval_policy != WANTED_APPROVAL_POLICY: + wrong.append( + f"approval_policy is {approval_policy!r}, not the {WANTED_APPROVAL_POLICY!r} the " + f"product pins on every turn it starts — so this turn does not show that pin being " + f"applied, and `never` among those values would mean no prompt could exist at all" + ) + if reviewer != WANTED_REVIEWER: + wrong.append( + f"approvals_reviewer is {reviewer!r}, not {WANTED_REVIEWER!r} — prompts from this " + f"thread are answered by a subagent and never reach the person" + ) + return Policy(named, "; ".join(wrong)) + + def _first_session_meta(rollout: Path) -> dict | None: try: with rollout.open() as lines: diff --git a/tests/acceptance/journey.py b/tests/acceptance/journey.py index ac6fede..f72e64c 100644 --- a/tests/acceptance/journey.py +++ b/tests/acceptance/journey.py @@ -97,13 +97,23 @@ class Instruction: """ words: str - filename: str | None = None + #: Where the effect lands: relative to the workspace, or absolute. Both are + #: real cases — the Codex lane asks for a file *outside* the workspace, + #: because that is what its sandbox will not let it write (`writing_at`) — + #: and `path_in` resolves the one against the other. + target: Path | None = None content: str | None = None + def path_in(self, workspace: Path) -> Path | None: + if self.target is None: + return None + return self.target if self.target.is_absolute() else workspace / self.target + def effect_in(self, workspace: Path) -> str | None: - if self.filename is None: + target = self.path_in(workspace) + if target is None: return None - text = support.read_if_exists(workspace / self.filename) + text = support.read_if_exists(target) return text.strip() if text is not None else None def performed_in(self, workspace: Path) -> bool: @@ -118,7 +128,25 @@ def writing(filename: str, content: str) -> Instruction: f"contents are the single word {content}. Do nothing else, and do not " f"ask any questions." ), - filename=filename, + target=Path(filename), + content=content, + ) + + +def writing_at(path: Path, content: str) -> Instruction: + """The same action, named by absolute path, so the sandbox is what decides. + + Identical to `writing` in everything the steps read back — one file, one word, + read off the filesystem. The only difference is *where*, and on the Codex lane + that is the whole point: the path is outside the Session's writable roots, so + the action cannot be taken without asking. + """ + return Instruction( + words=( + f"Create a file at the absolute path {path} whose entire contents are the " + f"single word {content}. Do nothing else, and do not ask any questions." + ), + target=path, content=content, ) @@ -131,11 +159,21 @@ def writing(filename: str, content: str) -> Instruction: words="Reply with the single word READY. Do not use any tools, and do not ask anything." ) -#: Turn 2 — arrives by Relay, raises a permission on the way. Measured 2026-08-26 -#: on `claude` 2.1.246 at `--permission-mode default`: a Write of a new file -#: raises `Do you want to create ?` and the roster's `status` goes to -#: `waiting`. That is the permission `approval` answers. -RELAYED = writing("relay.txt", "BRAVO") +#: Turn 2 — arrives by Relay and raises a permission on the way. The file and the +#: word are one shape for both lanes; **where** it is written is the lane's, and +#: that is what `Lane.relayed` holds. One instruction for both lanes is what left +#: the codex `approval` step silent (#105): at its own sandbox a Codex writes +#: inside its workspace without asking anybody, so there was nothing to +#: round-trip. See `CLAUDE` and `CODEX` for each lane's measurement. +RELAY_FILE = "relay.txt" +RELAY_WORD = "BRAVO" + +#: Where the Codex lane's relayed instruction writes: beside the workspace, under +#: the same run directory, and outside the Session's writable roots. Kept inside +#: the run directory so the design's rule still holds — nothing outside it is +#: written by the agents — and kept out of the workspace because being outside is +#: the entire reason Codex has to ask before writing there. +OUTSIDE_THE_SANDBOX = "outside-the-sandbox" #: Turn 3 — arrives from Telegram. INBOUND = writing("inbound.txt", "CHARLIE") @@ -182,6 +220,21 @@ class Lane: #: the instruction work. Everywhere the harness speaks about the concept, it #: is a Child Process (`CONTEXT.md`). child_words: str + #: The instruction `relay` carries and `approval` grades, given the lane's + #: workspace. It is the lane's because *what a permission is* is the lane's: + #: the two agents' policies refuse different actions, and an instruction that + #: asks one of them for permission asks the other for nothing (#105). + relayed: Callable[[Path], Instruction] + #: The ground the permission was measured on, given the agent's own record of + #: the Session. `approval` says its `named` half in the evidence line, so a + #: green step states the ground it stood on rather than implying some + #: default — and fails on the `unsound` half, because a step that cannot + #: stand on its own ground has not proved what it claims. + policy_at: Callable[[Path | None], hand_started.Policy] + #: What that policy is measured to ask about. Said by the step that finds no + #: permission at all, so a silent lane reports the measurement it contradicts + #: instead of the other lane's. + asks_about: str #: What the agent itself says about a Session the harness started, or None #: when it says nothing yet. Takes the pid, the workspace, the environment to #: read a roster with, and the moment the harness started looking. @@ -214,6 +267,16 @@ class Lane: agent="claude", binary="claude", arguments=("--permission-mode", "default"), + relayed=lambda workspace: writing(RELAY_FILE, RELAY_WORD), + # The flag the harness passes *is* the whole policy on this lane, and Claude + # publishes no per-turn readback of it, so there is nothing to read back and + # nothing that can disagree. Sound by construction, and said out loud here so + # the asymmetry with the Codex lane is a measurement rather than an oversight. + policy_at=lambda record: hand_started.Policy("`--permission-mode default`"), + asks_about=( + "a Write of a new file asks `Do you want to create ?` and the roster's " + "`status` goes to `waiting` (measured 2026-08-26 on claude 2.1.246)" + ), ground_truth=lambda pid, workspace, environment, since: hand_started.claude_ground_truth( pid, environment ), @@ -225,14 +288,42 @@ class Lane: ), ) -#: Codex takes no flag here. Measured 2026-08-26: `~/.codex/config.toml` sets no -#: `approval_policy`, so the lane runs on the product's own default — which is -#: what #60 asked for and what this lane still gets. +#: `--sandbox workspace-write` pins the **sandbox**, and nothing else. It is the +#: Codex config surface #105 asks this lane to name, and it is chosen because it +#: is the one thing here the product never asserts: `turn/start` pins +#: `approvalPolicy` and `approvalsReviewer` on every relayed turn +#: (`agent/codex/threads.py:36-40`), so pinning those at the keyboard too would +#: pre-arrange the very assertion #77's approval route has to make for itself. +#: What the sandbox *allows* is nobody's assertion, and until this flag it came +#: from `~/.codex/config.toml` — a file the user owns, where one +#: `sandbox_mode = "danger-full-access"` would silence this step exactly as +#: `permissions.defaultMode = "auto"` silenced the Claude one. The value is what +#: a trusted workspace already gives (measured 2026-08-26 on the failing run's +#: own rollout, `turn_context.sandbox_policy = workspace-write`), so the flag +#: fixes the ground rather than moving it. CODEX = Lane( name="codex", agent="codex", binary="codex", - arguments=(), + arguments=("--sandbox", "workspace-write"), + # Measured 2026-08-27 through the shared daemon with the product's own pin + # and no sandbox override, on codex-cli 0.149.1 and again on 0.150.0 over a + # 0.149.1 app-server: a write to a path outside the workspace raises + # `item/fileChange/requestApproval`, the thread goes to `waitingOnApproval`, + # and **the file does not appear until the approval is answered**. The same + # instruction aimed *inside* the workspace raised nothing, both times, which + # is what `approval.none` was reporting on run `20260826T122216Z`. The + # directory is made by the harness, so the one refused action is the write. + relayed=lambda workspace: writing_at( + workspace.parent / OUTSIDE_THE_SANDBOX / RELAY_FILE, RELAY_WORD + ), + policy_at=lambda record: hand_started.codex_turn_policy(record), + asks_about=( + "a write to a path outside the Session's writable roots raises " + "`item/fileChange/requestApproval` and the thread goes to `waitingOnApproval` " + "(measured 2026-08-27 on codex-cli 0.149.1 and 0.150.0); a write *inside* the " + "workspace raises nothing, which is the silence #105 was opened for" + ), ground_truth=lambda pid, workspace, environment, since: hand_started.codex_ground_truth( pid, workspace, since ), @@ -566,24 +657,36 @@ def relay(self) -> str: **DELIVERED is never inferred from a write** (#71, carried into #77), so this step wants two things the engine cannot fake: a reply that says - `delivered` rather than retained or unknown, and the file in the - workspace. The permission this turn raises on the Claude lane is answered - here — through `bridgectl approve`, so the *bridge* answers it — and the - evidence is handed to `approval`, which is the step that grades it. + `delivered` rather than retained or unknown, and the file the words asked + for. The permission this turn raises is answered here — through + `bridgectl approve`, so the *bridge* answers it — and the evidence is + handed to `approval`, which is the step that grades it. + + **The words are the lane's** (`Lane.relayed`). Both lanes ask for one + file containing one word; only the path differs, because only the path + decides whether the agent has to ask permission first (#105). """ if self.address is None: raise LaneBlocked("no Session to relay to") + relayed = self.lane.relayed(self.config.workspace) + # The directory the effect lands in is the harness's to make. On the + # Codex lane it is outside the sandbox, and an agent that had to create + # it as well would be asking permission twice for one instruction — two + # permissions where the step grades one. + target = relayed.path_in(self.config.workspace) + if target is not None: + target.parent.mkdir(parents=True, exist_ok=True) mark = self.person.latest_message_id() started = time.monotonic() answer = self.bridgectl( - "relay", self.address, RELAYED.words, timeout=support.RELAY_DEADLINE_SECONDS + "relay", self.address, relayed.words, timeout=support.RELAY_DEADLINE_SECONDS ) if not answer.ok: raise StepFailed(f"relay refused: {answer.text}") self.approval_evidence = self._answer_pending_approval(mark) performed = support.wait_for( - lambda: RELAYED.performed_in(self.config.workspace), + lambda: relayed.performed_in(self.config.workspace), deadline_seconds=self.far_side.workspace_effect_seconds, ) self.turns.append(Turn("relay", time.monotonic() - started, performed)) @@ -603,15 +706,14 @@ def relay(self) -> str: else "; AND no reason travelled with it — #68 requires a delivery failure " "to carry one, and `seams/delivery.py:47-49` cannot construct one without" ) - + f". {RELAYED.filename} is {RELAYED.effect_in(self.config.workspace)!r}" + + f". {target} is {relayed.effect_in(self.config.workspace)!r}" ) if not performed: raise StepFailed( - f"relay answered {answer.text!r} but {RELAYED.filename} never appeared in " - f"{self.config.workspace} within " + f"relay answered {answer.text!r} but {target} never appeared within " f"{self.far_side.workspace_effect_seconds:.0f}s — a receipt without an effect" ) - return f"{answer.text}; {RELAYED.filename} contains {RELAYED.content}" + return f"{answer.text}; {target} contains {relayed.content}" # --- approval --------------------------------------------------------- @@ -622,15 +724,33 @@ def approval(self) -> str: Splitting them into two turns would prove less: the shape the product has to survive is a *relayed* instruction that needs a permission, and that is one turn by definition. + + The evidence names the policy the permission was measured at, read from + the agent's own record where the agent records one. A green step that did + not say which ground it stood on is a step that would read the same on + ground where the permission could not have been raised at all — which is + the run #105 was opened on. """ + policy = self.lane.policy_at(self._record_now()) if self.approval_evidence is None: + # Each lane reports the measurement its own silence contradicts. One + # shared sentence here is how the codex step spent a run explaining + # what a Claude at `--permission-mode default` would have done + # (#105): true, and about the other lane. + raise StepFailed( + f"no permission was raised by the relayed instruction, so nothing " + f"round-tripped. Measured at {policy.named}: {self.lane.asks_about}" + ) + if policy.unsound: + # A round trip that happened is not the whole claim. #105 asks this + # step to *name* the policy it was measured at, and a green line + # reading `no policy` — or naming ground on which no permission could + # have been raised — is the same silent pass in a new costume. raise StepFailed( - "no permission was raised by the relayed instruction, so nothing round-tripped. " - "On the Claude lane that is a red: measured 2026-08-26 at " - "`--permission-mode default`, a Write of a new file asks " - "`Do you want to create ?` and the roster goes to `waiting`." + f"a permission did round-trip ({self.approval_evidence}), but the ground it was " + f"measured on is not the ground this lane stands on: {policy.unsound}" ) - return self.approval_evidence + return f"{self.approval_evidence}; measured at {policy.named}" # --- companion inbound ------------------------------------------------ @@ -650,16 +770,16 @@ def companion_inbound(self) -> str: if not performed: raise StepFailed( f"message {sent.id} addressed to @{name} was sent to the bot but " - f"{INBOUND.filename} never appeared in {self.config.workspace}; engine.log " + f"{INBOUND.target} never appeared in {self.config.workspace}; engine.log " f"inbound lines: {inbound_lines[-3:] or 'none'}" ) if not inbound_lines: raise StepFailed( - f"{INBOUND.filename} was written, so the words arrived, but engine.log carries " + f"{INBOUND.target} was written, so the words arrived, but engine.log carries " f"no inbound line — #48's requirement" ) return ( - f"message {sent.id} → @{name} → {INBOUND.filename} contains {INBOUND.content}; " + f"message {sent.id} → @{name} → {INBOUND.target} contains {INBOUND.content}; " f"engine.log: {inbound_lines[-1]!r}" ) @@ -869,11 +989,15 @@ def _name_now(self) -> str | None: name = row.get("name") return str(name) if name else None + def _record_now(self) -> Path | None: + """Where the agent's own record of this Session is, at this moment.""" + if self.truth is None: + return None + return self.lane.record_now(self.truth, self.started_at) + def _record_size(self) -> int: """How big the agent's own record is — the far side's own measure of work.""" - if self.truth is None: - return 0 - record = self.lane.record_now(self.truth, self.started_at) + record = self._record_now() return record.stat().st_size if record and record.exists() else 0 def _drive_turn(