From 33b6680490009c6d051deeaa57542d34026b9a21 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:35:43 -0400 Subject: [PATCH 01/25] P251: let Git answer ancestry and growth in bulk, outside the connector Src-Budget: 86 --- src/agent_comms/commander/standing.py | 161 +++++++--- src/agent_comms/work_branches.py | 155 ++++++---- tests/verify_connector_tools.py | 430 ++++++++++++++++++++++++++ 3 files changed, 631 insertions(+), 115 deletions(-) create mode 100644 tests/verify_connector_tools.py diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index b8408a26b7..9906650423 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -11,9 +11,13 @@ from ..comms import Comms from ..goal_actions import OrderGoalAction -from ..standing_rules import (DatedStandingRule, ReasonedStandingRule, StandingRule, - UndatedStandingRule) from ..goals import Goal +from ..standing_rules import ( + DatedStandingRule, + ReasonedStandingRule, + StandingRule, + UndatedStandingRule, +) from ..threads import Thread from ..todos import FlatTrend, OrderProgress, PullRequests, StrikeTodo, Todo from ..turn_digests import TurnDigestRead, TurnsSince @@ -35,13 +39,17 @@ def violations(cls, comms: Comms, commander: str) -> tuple[Detached, ...]: for todo in cls.todos(comms, commander): owed.setdefault(todo.holder, []).append(todo.id) statuses = comms.registry.snapshot().statuses - away = [unit for unit in sorted(owed) if unit not in statuses or not statuses[unit].receiving] + away = [ + unit for unit in sorted(owed) if unit not in statuses or not statuses[unit].receiving + ] unread = comms.bus.pending_counts_all(tuple(unit for unit in away if unit in statuses)) return tuple(cls(unit, unread.get(unit, 0), tuple(owed[unit])) for unit in away) def line(self) -> str: - return (f"DETACHED {self.unit}: {self.unread} unread, orders {', '.join(self.orders)}" - " -> wake it or reassign") + return ( + f"DETACHED {self.unit}: {self.unread} unread, orders {', '.join(self.orders)}" + " -> wake it or reassign" + ) @dataclass(frozen=True) @@ -51,13 +59,18 @@ class RequestStalled(UndatedStandingRule): @classmethod def violations(cls, comms: Comms, commander: str) -> tuple[RequestStalled, ...]: thread = comms.registry.require(commander) - return tuple(cls(request) for goal in thread.goal.active() - for request, served in goal.order.open_requests(goal, comms.goals, thread.incarnation) - if not served) + return tuple( + cls(request) + for goal in thread.goal.active() + for request, served in goal.order.open_requests(goal, comms.goals, thread.incarnation) + if not served + ) def line(self) -> str: - return (f"REQUEST STALLED: {self.goal.id} {self.goal.text}" - " -> order its next step or mark it done") + return ( + f"REQUEST STALLED: {self.goal.id} {self.goal.text}" + " -> order its next step or mark it done" + ) @dataclass(frozen=True) @@ -75,19 +88,32 @@ def idle(cls, comms: Comms, commander: str) -> list[Thread]: held = {todo.holder for todo in cls.todos(comms, commander)} excluded = held | CONFIG.parked | {commander, ""} bound = TurnsSince(time.time() - cls.recent) - return sorted((thread for thread in comms.registry.all_threads().values() - if thread.name not in excluded - and TurnDigestRead(comms.root, thread, bound).native()), - key=lambda thread: thread.name) + return sorted( + ( + thread + for thread in comms.registry.all_threads().values() + if thread.name not in excluded + and TurnDigestRead(comms.root, thread, bound).native() + ), + key=lambda thread: thread.name, + ) def assign(self, comms: Comms, commander: str, unit: Thread) -> None: """The order sent again to ``unit`` through its Goal, with the order's own text.""" todo = comms.goals.todos.get(self.todo.id) order = todo.order.reissued(time.time()) - comms.goals.update_goal(unit.name, OrderGoalAction( - problem=todo.id, expected_revision=todo.revision, order=order, - request=todo.goal.request(), deadline=todo.deadline, commander=commander, - traced=todo.order.owner)) + comms.goals.update_goal( + unit.name, + OrderGoalAction( + problem=todo.id, + expected_revision=todo.revision, + order=order, + request=todo.goal.request(), + deadline=todo.deadline, + commander=commander, + traced=todo.order.owner, + ), + ) comms.messaging.send(commander, unit.name, f"{self.line()}\n{order.instructions(todo)}") @@ -102,29 +128,46 @@ def violations(cls, comms: Comms, commander: str) -> tuple[FlatWheelSpin, ...]: held = tuple(todo for todo in cls.todos(comms, commander) if todo.pending) pulls = PullRequests(held) storage = comms.registry.storage_rule - trends = ((todo, OrderProgress.read(todo, storage, pulls).distance_readings().trend) - for todo in held) + trends = ( + (todo, OrderProgress.read(todo, storage, pulls).distance_readings().trend) + for todo in held + ) flat = ((todo, trend.since) for todo, trend in trends if isinstance(trend, FlatTrend)) - return tuple(cls(todo, since) for todo, since in flat - if TurnDigestRead(comms.root, comms.registry.require(todo.held().owner), - TurnsSince(since)).read().turns) + return tuple( + cls(todo, since) + for todo, since in flat + if TurnDigestRead( + comms.root, comms.registry.require(todo.held().owner), TurnsSince(since) + ) + .read() + .turns + ) def began(self) -> float: return self.since def line(self) -> str: - return (f"WHEEL SPIN: {self.todo.holder} kept working on {self.todo.id} while its distance" - " to done did not fall across two readings -> Core strikes it and gives the order" - " to an idle unit") + return ( + f"WHEEL SPIN: {self.todo.holder} kept working on {self.todo.id} while its distance" + " to done did not fall across two readings -> Core strikes it and gives the order" + " to an idle unit" + ) def enforce(self, comms: Comms, commander: str) -> None: todo = comms.goals.todos.get(self.todo.id) held = todo.held() if not todo.struck_on_order(held.owner): - comms.goals.change_problem(todo.id, StrikeTodo( - todo.revision, comms.registry.require(commander).incarnation, held, - f"{held.owner} kept working on {todo.id} while the distance to done did not fall" - " across two readings"), commander) + comms.goals.change_problem( + todo.id, + StrikeTodo( + todo.revision, + comms.registry.require(commander).incarnation, + held, + f"{held.owner} kept working on {todo.id} while the distance to done did not fall" + " across two readings", + ), + commander, + ) for unit in self.idle(comms, commander)[:1]: self.assign(comms, commander, unit) @@ -137,14 +180,21 @@ class UnownedWheelSpin(WheelSpin, UndatedStandingRule): @classmethod def violations(cls, comms: Comms, commander: str) -> tuple[UnownedWheelSpin, ...]: - rows = (todo for todo in comms.goals.todos.for_commander( - comms.registry.require(commander).incarnation) if todo.pending and not todo.holder) + rows = ( + todo + for todo in comms.goals.todos.for_commander( + comms.registry.require(commander).incarnation + ) + if todo.pending and not todo.holder + ) idle = cls.idle(comms, commander) return tuple(cls(todo, unit.name) for unit, todo in zip(idle, rows, strict=False)) def line(self) -> str: - return (f"WHEEL SPIN: {self.unit} holds no order while {self.todo.id} is unowned" - f" -> Core gives it to {self.unit}") + return ( + f"WHEEL SPIN: {self.unit} holds no order while {self.todo.id} is unowned" + f" -> Core gives it to {self.unit}" + ) def enforce(self, comms: Comms, commander: str) -> None: self.assign(comms, commander, comms.registry.require(self.unit)) @@ -196,31 +246,42 @@ def git(checkout: Path, *args: str) -> str: ["git", "-C", str(checkout), *args], capture_output=True, text=True ).stdout - @classmethod - def numstat(cls, checkout: Path, sha: str, path: str) -> tuple[int, int]: - diff = cls.git(checkout, "diff", "--numstat", f"{sha}^1", sha, "--", path) - counted = [ - (int(a), int(d)) - for a, d, _ in (row.split("\t", 2) for row in diff.splitlines()) - if a.isdigit() - ] - return sum(a for a, _ in counted), sum(d for _, d in counted) - @classmethod def merges(cls, repo: Repo) -> Iterator[MergeGrowth]: - """Each first-parent commit on the repository's main inside the window, newest first.""" - cls.git(repo.checkout, "fetch", "-q", "origin", "main") + """Read first-parent growth from one captured, already-fetched Git history. + + Fetch belongs to the checkout's update owner, not a standing-rule read. + Git emits each commit's first-parent diff in this same process. + """ log = cls.git( repo.checkout, "log", "--first-parent", f"--since={cls.window}", - "--format=%H %ct %s", + "--format=%x1e%H%x00%ct%x00%s", + "--numstat", + "--no-renames", + "--diff-merges=first-parent", "origin/main", + "--", + repo.source, + "tests", ) - for sha, at, subject in (line.split(" ", 2) for line in log.splitlines()): - src, tests = (cls.numstat(repo.checkout, sha, path) for path in (repo.source, "tests")) - yield cls(repo.github, sha, subject, float(at), src, tests) + for record in log.split("\x1e")[1:]: + header, _, diff = record.partition("\n") + sha, at, subject = header.split("\x00", 2) + src, tests = [0, 0], [0, 0] + for row in diff.splitlines(): + if not row: + continue + added, deleted, path = row.split("\t", 2) + if not (added.isdigit() and deleted.isdigit()): + continue + for root, counts in ((repo.source, src), ("tests", tests)): + if path == root or path.startswith(root.rstrip("/") + "/"): + counts[0] += int(added) + counts[1] += int(deleted) + yield cls(repo.github, sha, subject, float(at), (src[0], src[1]), (tests[0], tests[1])) @classmethod def violations(cls, comms: Comms, commander: str) -> tuple[MergeGrowth, ...]: diff --git a/src/agent_comms/work_branches.py b/src/agent_comms/work_branches.py index 8d94a82ef5..ba9268e546 100644 --- a/src/agent_comms/work_branches.py +++ b/src/agent_comms/work_branches.py @@ -8,7 +8,6 @@ from __future__ import annotations -import json import subprocess from dataclasses import dataclass from pathlib import Path @@ -17,8 +16,8 @@ from pydantic import ConfigDict from pydantic.alias_generators import to_camel -from .declared_family import DeclaredFamily from .cli_protocol import WireModel +from .declared_family import DeclaredFamily from .field_codec import projected if TYPE_CHECKING: @@ -94,38 +93,11 @@ class Connection[Node](ForgeAnswer): page_info: PageInfo | None = None -class ForgeCommit(ForgeAnswer): - committed_date: str - - -class ForgeComparison(ForgeAnswer): - behind_by: int - - class ForgePullRequest(ForgeAnswer): number: int head_ref_name: str -class ForgeRef(ForgeAnswer): - name: str - target: ForgeCommit - compare: ForgeComparison | None - associated_pull_requests: Connection[ForgePullRequest] - - @property - def unmerged_commits(self) -> int: - # ref.compare(main).behindBy counts the branch's commits that main lacks. - return 0 if self.compare is None else self.compare.behind_by - - @property - def own_pull_requests(self) -> tuple[PullRequestBranchLink, ...]: - # A ref's associated PRs include those that only use it as their base. - return tuple(PullRequestBranchLink(pull.number) - for pull in self.associated_pull_requests.nodes - if pull.head_ref_name == self.name) - - class ForgeRefName(ForgeAnswer): name: str @@ -139,12 +111,12 @@ class ForgeIssue(ForgeAnswer): linked_branches: Connection[ForgeLinkedBranch] -class RefsRepository(ForgeAnswer): - refs: Connection[ForgeRef] +class PullsRepository(ForgeAnswer): + pull_requests: Connection[ForgePullRequest] @property - def connection(self) -> Connection[ForgeRef]: - return self.refs + def connection(self) -> Connection[ForgePullRequest]: + return self.pull_requests class IssuesRepository(ForgeAnswer): @@ -171,17 +143,12 @@ class WorkBranches: main: ClassVar[str] = "main" timeout: ClassVar[float] = 60.0 - _REFS: ClassVar[str] = """ -query($owner: String!, $name: String!, $main: String!, $after: String) { + _PULLS: ClassVar[str] = """ +query($owner: String!, $name: String!, $after: String) { repository(owner: $owner, name: $name) { - refs(refPrefix: "refs/heads/", first: 100, after: $after) { + pullRequests(states: OPEN, first: 100, after: $after) { pageInfo { hasNextPage endCursor } - nodes { - name - target { ... on Commit { committedDate } } - compare(headRef: $main) { behindBy } - associatedPullRequests(states: OPEN, first: 50) { nodes { number headRefName } } - } + nodes { number headRefName } } } }""" @@ -195,8 +162,14 @@ class WorkBranches: } }""" - def _run(self, *command: str) -> str: - result = subprocess.run(command, cwd=self.checkout, capture_output=True, timeout=self.timeout) + def _run(self, *command: str, input: str | None = None) -> str: + result = subprocess.run( + command, + cwd=self.checkout, + capture_output=True, + input=input.encode() if input is not None else None, + timeout=self.timeout, + ) if result.returncode != 0: detail = result.stderr.decode(errors="replace").strip() or f"{command[0]} failed" raise WorkBranchError(f"{' '.join(command[:3])} in {self.checkout}: {detail}") @@ -204,17 +177,20 @@ def _run(self, *command: str) -> str: @property def repository(self) -> tuple[str, str]: - owner, name = self._run("gh", "repo", "view", "--json", "nameWithOwner", "-q", - ".nameWithOwner").strip().split("/") + from .commander.config import Repo + + owner, name = Repo.of(self.checkout).github.split("/") return owner, name def _pages(self, query: str, answer: type[ForgeQuery], **variables: str): pages: list[tuple[str, ...]] = [()] for after in pages: arguments = ["gh", "api", "graphql", "-f", f"query={query}"] - for key, value in {**variables, **dict(zip(("after",), after))}.items(): + for key, value in {**variables, **dict(zip(("after",), after, strict=False))}.items(): arguments += ["-F", f"{key}={value}"] - connection = answer.model_validate_json(self._run(*arguments)).data.repository.connection + connection = answer.model_validate_json( + self._run(*arguments) + ).data.repository.connection yield from connection.nodes pages.extend(connection.page_info.following) @@ -232,8 +208,13 @@ def _worktrees(self) -> dict[str, tuple[str, ...]]: @staticmethod def _checkout(snapshot: RegistrySnapshot, worktree: str) -> BranchCheckout: resolved = str(Path(worktree).resolve()) - threads = tuple(sorted(thread.name for thread in snapshot.threads.values() - if str(Path(thread.worktree).expanduser().resolve()) == resolved)) + threads = tuple( + sorted( + thread.name + for thread in snapshot.threads.values() + if str(Path(thread.worktree).expanduser().resolve()) == resolved + ) + ) if not threads: return UnregisteredBranchCheckout(worktree) return ThreadBranchCheckout(threads, worktree) @@ -241,21 +222,65 @@ def _checkout(snapshot: RegistrySnapshot, worktree: str) -> BranchCheckout: def read(self, snapshot: RegistrySnapshot) -> tuple[WorkBranch, ...]: """Every branch with commits not on main, most recently committed first.""" owner, name = self.repository - issue_links: dict[str, list[BranchLink]] = {} - for issue in self._pages(self._ISSUES, ForgeQuery[IssuesRepository], owner=owner, name=name): - for linked in issue.linked_branches.nodes: - issue_links.setdefault(linked.ref.name, []).append(IssueBranchLink(issue.number)) - worktrees = self._worktrees() - branches = tuple( - WorkBranch( - name=ref.name, - unmerged_commits=ref.unmerged_commits, - committed_at=ref.target.committed_date, - links=(*ref.own_pull_requests, *issue_links.get(ref.name, ())), - checkouts=tuple(self._checkout(snapshot, path) for path in worktrees.get(ref.name, ())), + refs = { + ref.removeprefix("refs/heads/"): oid + for oid, ref in ( + line.split() + for line in self._run("git", "ls-remote", "--heads", "origin").splitlines() ) - for ref in self._pages(self._REFS, ForgeQuery[RefsRepository], - owner=owner, name=name, main=self.main) - if ref.name != self.main and ref.unmerged_commits + } + main = refs[self.main] + # Git already owns commit ancestry. Ask it once for all locally named + # commits, rather than asking GitHub to compare every branch separately. + objects = self._run( + "git", + "cat-file", + "--batch-check=%(objectname) %(objecttype)", + input="\n".join(set(refs.values())) + "\n", ) + missing = [line.split()[0] for line in objects.splitlines() if line.endswith(" missing")] + if missing: + self._run("git", "fetch", "--no-tags", "origin", *missing) + commits = { + oid: (int(ahead), date) + for oid, date, ahead, _behind in ( + line.split() + for line in self._run( + "git", + "for-each-ref", + f"--format=%(objectname) %(committerdate:iso-strict) %(ahead-behind:{main})", + "refs/heads", + "refs/remotes/origin", + ).splitlines() + ) + } + links: dict[str, list[BranchLink]] = {} + for pull in self._pages(self._PULLS, ForgeQuery[PullsRepository], owner=owner, name=name): + links.setdefault(pull.head_ref_name, []).append(PullRequestBranchLink(pull.number)) + for issue in self._pages( + self._ISSUES, ForgeQuery[IssuesRepository], owner=owner, name=name + ): + for linked in issue.linked_branches.nodes: + links.setdefault(linked.ref.name, []).append(IssueBranchLink(issue.number)) + worktrees = self._worktrees() + branches = [] + for branch, oid in refs.items(): + if oid not in commits: + # A branch published since the last local fetch has no local + # ref yet. Compare its captured object, never an older ref. + commits[oid] = ( + int(self._run("git", "rev-list", "--count", f"{main}..{oid}")), + self._run("git", "show", "-s", "--format=%cI", oid).strip(), + ) + ahead, date = commits[oid] + if branch != self.main and ahead: + branches.append( + WorkBranch( + branch, + ahead, + date, + tuple(links.get(branch, ())), + tuple(self._checkout(snapshot, path) for path in worktrees.get(branch, ())), + ) + ) return tuple(sorted(branches, key=lambda branch: branch.committed_at, reverse=True)) diff --git a/tests/verify_connector_tools.py b/tests/verify_connector_tools.py new file mode 100644 index 0000000000..03681927ab --- /dev/null +++ b/tests/verify_connector_tools.py @@ -0,0 +1,430 @@ +"""Time every advertised MCP tool over curl on independent copies of a scratch bus. + +Uses real Core, OAuth grants, connector processes and HTTP. Process-control calls +name a deliberately absent scratch owner: no agent is launched or signalled. +Retains request/response/timing/trace files under the supplied owned output path. +""" + +from __future__ import annotations + +import argparse +import json +import os +import shutil +import signal +import socket +import subprocess +import sys +import threading +import time +from dataclasses import replace +from pathlib import Path + +from agent_comms.change_report import FileLine +from agent_comms.channels import AnyOfMatch, SavedView, ViewKind, ViewPredicate +from agent_comms.child_process import ProcessIdentity +from agent_comms.comms import Comms +from agent_comms.connector import Connector +from agent_comms.connector_store import ConnectorStore +from agent_comms.field_codec import FieldCodec +from agent_comms.goal_actions import CreateProblemGoalAction, PausedGoalAction, SetGoalAction +from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.progress_claims import WorkingClaim +from agent_comms.recorded_time import RecordedEventTime +from agent_comms.thread_status import StoppedThreadStatus +from agent_comms.todos import MechanicalOrderRun, OperationDelivery, PendingProblemOrder +from agent_comms.tool_caller import ThreadCaller +from agent_comms.tools import CommsOrderDoneTool, CommsTodoCreateTool +from verify_connector_live import wait_started + + +def seed(root: Path, project: Path) -> None: + root.mkdir(mode=0o700) + comms = Comms(root) + for name in ("claude-ai", "worker", "archivable"): + comms.registry.declare( + comms.registry.new_thread(name, frozenset({"probe"}), str(project)), + status=StoppedThreadStatus(), + ) + comms.messaging.initialize_private_initial_protocol() + comms.channels.set_saved_view( + SavedView( + "probe-view", ViewKind.PARTICIPANTS, ViewPredicate(AnyOfMatch, frozenset({"probe"})) + ) + ) + comms.goals.update_goal( + "claude-ai", SetGoalAction(text="Measure the connector calls.", goal_id="probe-goal") + ) + original = comms.messaging.send("worker", "claude-ai", "Original scratch history.").message + assert original.sender == "worker" + for worker in threading.enumerate(): + if worker.name.startswith("agent-comms-"): + worker.join(20) + assert not worker.is_alive(), worker.name + + +def arguments(comms: Comms, name: str, project: Path, checkout: Path) -> dict: + actor = FieldCodec.encode(comms.registry.require("claude-ai").incarnation) + pending = PendingProblemOrder( + id="probe-order", + owner=FileLine(__file__, 1), + run=MechanicalOrderRun(), + check=OrderCheck(str(project), "printf 1", CountCheckResult(1)), + edits=("Read the original saved source before changing its owner",), + banned=("No second copy",), + report="Report what worked", + at=RecordedEventTime(time.time()), + first_call_deadline=RecordedEventTime(time.time() + 300), + ) + order = CreateProblemGoalAction( + problem="P1", + order=pending, + request="request", + deadline=RecordedEventTime(time.time() + 600), + commander="claude-ai", + repo="OpenHCSDev/agent-comms", + text="Measure the connector order.", + ) + order_tools = { + "comms_todo", + "comms_verdict", + "comms_fire", + "comms_correct_wording", + "comms_todo_list", + "comms_order", + "comms_todo_create", + "comms_order_done", + "comms_order_check", + "comms_order_serve", + "comms_problem_close", + "comms_order_work", + "comms_unjudged", + } + if name in order_tools: + from agent_comms.goal_actions import ClearGoalAction + + comms.goals.update_goal("claude-ai", ClearGoalAction()) + comms.goals.update_goal( + "claude-ai", + SetGoalAction( + text="Measure the connector calls.", goal_id="request", order=RequestGoalOrder() + ), + ) + if name != "comms_todo_create": + CommsTodoCreateTool(owner="worker", order=order).apply(comms, ThreadCaller("claude-ai")) + if name == "comms_resume_goal": + comms.goals.update_goal( + "claude-ai", PausedGoalAction(progress="Paused only on this scratch bus.") + ) + todo = comms.goals.todos.get("P1") if name in order_tools - {"comms_todo_create"} else None + previous = FieldCodec.encode(todo.held()) if todo else {} + verdict = { + "kind": "accepted_verdict", + "expected_revision": todo.revision if todo else 1, + "actor": actor, + "previous": previous, + "tool_call_id": "absent-original-call", + "quote": "Read the original saved source", + } + screenshot = project / "probe.png" + screenshot.write_bytes(b"scratch verification artifact") + if name == "comms_problem_close": + CommsOrderDoneTool( + problem="P1", + order_id="probe-order", + delivery=OperationDelivery(), + claims=(WorkingClaim(str(screenshot)),), + ).apply(comms, ThreadCaller("claude-ai")) + if name in {"comms_decision", "comms_constraint", "comms_subtask"}: + thread = comms.registry.require("claude-ai") + comms.registry.declare( + replace(thread, process_identity=ProcessIdentity.capture(os.getpid())) + ) + owner, claim = comms.registry.live_owner_with_claim("claude-ai") + comms.registry.lease_live_turn_with_claim(owner, "probe-turn", expected_generation=claim) + cases = { + "comms_pin_channel": {"name": "#probe", "pinned": True}, + "comms_pin_thread": {"channel": "#probe", "name": "worker", "pinned": True}, + "comms_model": {"thread": "worker", "model": ""}, + "comms_sort_channels": {"order": "name"}, + "comms_tags": {}, + "comms_thread_tags": {"add": "probe"}, + "comms_channels": {}, + "comms_set_channel_metadata": {"name": "#probe", "parent": "", "archived": False}, + "comms_set_view": { + "name": "new-view", + "kind": "participants", + "match": "any_of", + "tags": "probe", + }, + "comms_delete_view": {"name": "probe-view"}, + "comms_sort_channel": {"name": "#probe", "order": "last_activity"}, + "comms_set_project": {"path": str(project)}, + "comms_set_goal": {"text": "Measure this scratch connector."}, + "comms_goal": { + "goal_id": "probe-goal", + "status": "active", + "progress": "Reading scratch history.", + }, + "comms_resume_goal": { + "goal_id": "probe-goal", + "progress": "Continue the authorized scratch work.", + }, + "comms_edit_goal": {"goal_id": "probe-goal", "text": "Measure only the scratch connector."}, + "comms_goal_history": {}, + "comms_todo": { + "id": "P1", + "operation": { + "kind": "change_todo_state", + "expected_revision": todo.revision if todo else 1, + "state": "blocked", + "actor": actor, + "generation": todo.held().generation if todo else None, + }, + }, + "comms_verdict": {"id": "P1", "operation": verdict}, + "comms_unjudged": {}, + "comms_fire": { + "unit": "worker", + "reason": "The scratch unit changed the saved file without reading its original contents first.", + }, + "comms_correct_wording": { + "unit": "worker", + "message_id": "probe", + "text": "Repair the authority.", + }, + "comms_todo_list": {}, + "comms_standing_rules": {}, + "comms_rule_review": { + "entry": {"kind": "drawn", "drawn": ["Read the original source."], "cited": 1} + }, + "comms_order": { + "owner": "worker", + "order": { + **FieldCodec.encode(order), + "kind": "order", + "expected_revision": todo.revision if todo else 1, + }, + }, + "comms_todo_create": {"owner": "worker", "order": FieldCodec.encode(order)}, + "comms_order_done": { + "problem": "P1", + "order_id": "probe-order", + "delivery": {"kind": "operation"}, + "claims": [{"kind": "working", "screenshot": str(screenshot)}], + }, + "comms_order_check": { + "problem": "P1", + "check": FieldCodec.encode(pending.check), + "why": "The earlier check counted the wrong output.", + }, + "comms_order_serve": {"problem": "P1", "serves": "skill"}, + "comms_problem_close": { + "problem": "P1", + "works": "The scratch connector returned its original recorded answer.", + "artifact": str(screenshot), + }, + "comms_turns": {"thread": "worker"}, + "comms_order_work": {"problem": "P1"}, + "comms_threads": {}, + "comms_work_branches": {"checkout": str(checkout)}, + "comms_collaboration": { + "action": "add", + "peer": "worker", + "note": "Scratch connector verification.", + }, + "comms_collaborations": {}, + "comms_rename_self": {"new_name": "claude-ai"}, + "comms_send_refusals": {}, + "comms_send": { + "from": "claude-ai", + "to": "worker", + "body": "Read the saved source and report what worked.", + }, + "comms_decision": { + "to": "worker", + "chosen": "Read the saved file", + "rejected": ["Discard the saved file"], + }, + "comms_constraint": {"to": "worker", "text": "Preserve the original saved file."}, + "comms_subtask": {"to": "worker", "text": "Read the saved file.", "completed": False}, + "comms_history": {"peer": "worker"}, + "comms_inbox": {"thread": "claude-ai", "ack": False}, + "comms_fork": {"name": "never-launched", "parent": "absent-owner"}, + "comms_stop": {"name": "absent-owner"}, + "comms_start": {"name": "absent-owner"}, + "comms_queue_restart": {"name": "absent-owner"}, + "comms_restart_queue": {"name": "worker"}, + "comms_cancel_restart": {"name": "worker"}, + "comms_archive": {"name": "archivable"}, + } + assert set(cases) == set(Connector.exposed()), ( + "The timing cases must cover the live declaration family." + ) + selected = cases[name] + if name == "comms_order": + selected["order"].pop("repo") + selected["order"].pop("text") + Connector.exposed()[name].parse_arguments(selected, call_id="timing-probe") + return selected + + +def run(output: Path, checkout: Path, selected: list[str]) -> None: + output.mkdir(mode=0o700, parents=True, exist_ok=False) + project = output / "project" + project.mkdir() + original = output / "original-wire" + seed(original, project) + config = json.loads((Path.home() / ".config/agent-comms/commander.json").read_text()) + rows = [] + names = selected or list(Connector.exposed()) + for name in names: + case = output / name + case.mkdir() + root, home = case / "wire", case / "home" + shutil.copytree(original, root) + comms = Comms(root) + args = arguments(comms, name, project, checkout) + for worker in threading.enumerate(): + if worker.name.startswith("agent-comms-"): + worker.join(20) + assert not worker.is_alive(), worker.name + config_path = home / ".config/agent-comms/commander.json" + config_path.parent.mkdir(parents=True) + forge_auth = Path.home() / ".config/gh" + if forge_auth.is_dir(): + (home / ".config/gh").symlink_to(forge_auth, target_is_directory=True) + config_path.write_text( + json.dumps( + { + **config, + "commander": "claude-ai", + "root": str(root), + "repos": [ + { + "checkout": str(checkout), + "github": "OpenHCSDev/agent-comms", + "source": "src", + } + ], + "state": str(case / "commander"), + "board": str(case / "board.md"), + "narrative": str(case / "narrative.md"), + "scratch": str(case), + } + ) + ) + store_path = home / ".local/state/agent-comms/connector" + store_path.mkdir(mode=0o700, parents=True) + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + environment = {**os.environ, "HOME": str(home), "PYTHONUNBUFFERED": "1"} + command = ( + "import faulthandler,signal;faulthandler.register(signal.SIGUSR1,all_threads=True);" + "from agent_comms.connector import main;raise SystemExit(main())" + ) + with (case / "server.log").open("w") as errors: + service = subprocess.Popen( + [ + sys.executable, + "-X", + "faulthandler", + "-u", + "-c", + command, + "serve", + "--root", + str(root), + "--port", + str(port), + ], + env=environment, + stdout=subprocess.PIPE, + stderr=errors, + text=True, + ) + try: + wait_started(service, []) + with ConnectorStore.at(store_path).granted("scratch timing") as token: + + def call(payload): + request = case / (payload["method"].replace("/", "-") + "-request.json") + request.write_text(json.dumps(payload)) + curl = subprocess.Popen( + [ + "curl", + "--silent", + "--show-error", + "-w", + "\n%{http_code} %{time_total}\n", + "-H", + "Content-Type: application/json", + "-H", + f"Authorization: Bearer {token}", + "--data-binary", + f"@{request}", + f"http://127.0.0.1:{port}/mcp", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + out, err = curl.communicate(timeout=2) + except subprocess.TimeoutExpired: + service.send_signal(signal.SIGUSR1) + out, err = curl.communicate(timeout=180) + assert curl.returncode == 0, err + response, timing = out.strip().rsplit("\n", 1) + status, seconds = timing.split() + (case / (payload["method"].replace("/", "-") + "-response.txt")).write_text( + response + ) + return status, float(seconds), response + + _, _, listed = call({"jsonrpc": "2.0", "id": 1, "method": "tools/list"}) + assert name in {tool["name"] for tool in json.loads(listed)["result"]["tools"]} + status, seconds, response = call( + { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": name, "arguments": args}, + } + ) + row = { + "tool": name, + "seconds": seconds, + "http": status, + "isError": json.loads(response)["result"]["isError"] + if status == "200" + else None, + } + rows.append(row) + (output / "timings.json").write_text(json.dumps(rows, indent=2) + "\n") + print(json.dumps(row), flush=True) + if name == "comms_send" and status == "200" and not row["isError"]: + from agent_comms.rule_reviews import NoRuleReview, RuleReviews + from agent_comms.wire_record import WireRecord + + saved = [ + message + for line in comms.bus.log.path.read_text().splitlines() + for message in WireRecord.public_from_wire(json.loads(line)).messages() + ] + assert any(message.body == args["body"] for message in saved) + assert isinstance(RuleReviews(root).review("claude-ai"), NoRuleReview) + finally: + service.terminate() + service.communicate(timeout=15) + assert len(rows) == len(names) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("output", type=Path) + parser.add_argument("--checkout", type=Path, default=Path(__file__).resolve().parents[1]) + parser.add_argument("--tools", nargs="*", choices=tuple(Connector.exposed()), default=[]) + args = parser.parse_args() + run(args.output.resolve(), args.checkout.resolve(), args.tools) From ca6612c67006be82c6273629ed96732080801a60 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:36:15 -0400 Subject: [PATCH 02/25] P257: let execution select the idle-unit transcript --- src/agent_comms/commander/standing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index b8408a26b7..a3c64948fa 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -71,13 +71,13 @@ class WheelSpin(StandingRule, declared_name=None): @classmethod def idle(cls, comms: Comms, commander: str) -> list[Thread]: - """Units with a native turn in the last ten minutes and no order, parked threads aside.""" + """Units with a recent turn in their own transcript and no order, parked threads aside.""" held = {todo.holder for todo in cls.todos(comms, commander)} excluded = held | CONFIG.parked | {commander, ""} bound = TurnsSince(time.time() - cls.recent) return sorted((thread for thread in comms.registry.all_threads().values() if thread.name not in excluded - and TurnDigestRead(comms.root, thread, bound).native()), + and TurnDigestRead(comms.root, thread, bound).read().turns), key=lambda thread: thread.name) def assign(self, comms: Comms, commander: str, unit: Thread) -> None: From 98d03d5c08b50ddc17ef97cee6735f7c4a15ec7b Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:38:01 -0400 Subject: [PATCH 03/25] Check idle selection with working and empty external transcripts --- tests/test_standing_rules.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_standing_rules.py b/tests/test_standing_rules.py index e1e5534aa9..7147a3dd81 100644 --- a/tests/test_standing_rules.py +++ b/tests/test_standing_rules.py @@ -143,6 +143,19 @@ def working(comms, name, created): name, frozenset(), str(comms.root), session_file=str(path), created_at=created)) +def test_idle_reads_external_execution_and_requires_an_actual_recent_turn(standing, tmp_path): + from agent_comms.transcript_events import AssistantTranscript + comms, _, module = standing + for name in ("external-working", "external-empty"): + comms.registry.declare(comms.registry.new_thread( + name, frozenset(), str(tmp_path), execution=ExternalThreadExecution)) + comms.transcripts.external("external-working").finish( + turn_id="current", item_id="answer", events=(AssistantTranscript("working"),)) + names = [thread.name for thread in module.WheelSpin.idle(comms, "lead")] + assert "external-working" in names + assert "external-empty" not in names and "worker" not in names + + def test_wheel_spin_strikes_and_reassigns_an_order_flat_across_two_readings( commander, standing, tmp_path, monkeypatch): from agent_comms.storage_rules import StorageRule From 9b122be8287022c95fc88d7419030e7999a35222 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:39:11 -0400 Subject: [PATCH 04/25] Use the declared external transcript position in verification --- tests/test_standing_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_standing_rules.py b/tests/test_standing_rules.py index 7147a3dd81..bfdada19f1 100644 --- a/tests/test_standing_rules.py +++ b/tests/test_standing_rules.py @@ -150,7 +150,7 @@ def test_idle_reads_external_execution_and_requires_an_actual_recent_turn(standi comms.registry.declare(comms.registry.new_thread( name, frozenset(), str(tmp_path), execution=ExternalThreadExecution)) comms.transcripts.external("external-working").finish( - turn_id="current", item_id="answer", events=(AssistantTranscript("working"),)) + turn_id="current", item_id="answer", events=(AssistantTranscript("working", position="answer"),)) names = [thread.name for thread in module.WheelSpin.idle(comms, "lead")] assert "external-working" in names assert "external-empty" not in names and "worker" not in names From 0740e0694d6e45935975e2b9ed146ab19f1bf4c1 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:50:41 -0400 Subject: [PATCH 05/25] P251: verify live remote heads and keep surrounding rule code unchanged --- src/agent_comms/commander/standing.py | 110 ++++++++------------------ src/agent_comms/work_branches.py | 7 +- tests/test_work_branches.py | 107 ++++++++++++++++++------- tests/verify_connector_tools.py | 2 +- 4 files changed, 115 insertions(+), 111 deletions(-) diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index 9906650423..f1f859b430 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -39,17 +39,13 @@ def violations(cls, comms: Comms, commander: str) -> tuple[Detached, ...]: for todo in cls.todos(comms, commander): owed.setdefault(todo.holder, []).append(todo.id) statuses = comms.registry.snapshot().statuses - away = [ - unit for unit in sorted(owed) if unit not in statuses or not statuses[unit].receiving - ] + away = [unit for unit in sorted(owed) if unit not in statuses or not statuses[unit].receiving] unread = comms.bus.pending_counts_all(tuple(unit for unit in away if unit in statuses)) return tuple(cls(unit, unread.get(unit, 0), tuple(owed[unit])) for unit in away) def line(self) -> str: - return ( - f"DETACHED {self.unit}: {self.unread} unread, orders {', '.join(self.orders)}" - " -> wake it or reassign" - ) + return (f"DETACHED {self.unit}: {self.unread} unread, orders {', '.join(self.orders)}" + " -> wake it or reassign") @dataclass(frozen=True) @@ -59,18 +55,13 @@ class RequestStalled(UndatedStandingRule): @classmethod def violations(cls, comms: Comms, commander: str) -> tuple[RequestStalled, ...]: thread = comms.registry.require(commander) - return tuple( - cls(request) - for goal in thread.goal.active() - for request, served in goal.order.open_requests(goal, comms.goals, thread.incarnation) - if not served - ) + return tuple(cls(request) for goal in thread.goal.active() + for request, served in goal.order.open_requests(goal, comms.goals, thread.incarnation) + if not served) def line(self) -> str: - return ( - f"REQUEST STALLED: {self.goal.id} {self.goal.text}" - " -> order its next step or mark it done" - ) + return (f"REQUEST STALLED: {self.goal.id} {self.goal.text}" + " -> order its next step or mark it done") @dataclass(frozen=True) @@ -88,32 +79,19 @@ def idle(cls, comms: Comms, commander: str) -> list[Thread]: held = {todo.holder for todo in cls.todos(comms, commander)} excluded = held | CONFIG.parked | {commander, ""} bound = TurnsSince(time.time() - cls.recent) - return sorted( - ( - thread - for thread in comms.registry.all_threads().values() - if thread.name not in excluded - and TurnDigestRead(comms.root, thread, bound).native() - ), - key=lambda thread: thread.name, - ) + return sorted((thread for thread in comms.registry.all_threads().values() + if thread.name not in excluded + and TurnDigestRead(comms.root, thread, bound).native()), + key=lambda thread: thread.name) def assign(self, comms: Comms, commander: str, unit: Thread) -> None: """The order sent again to ``unit`` through its Goal, with the order's own text.""" todo = comms.goals.todos.get(self.todo.id) order = todo.order.reissued(time.time()) - comms.goals.update_goal( - unit.name, - OrderGoalAction( - problem=todo.id, - expected_revision=todo.revision, - order=order, - request=todo.goal.request(), - deadline=todo.deadline, - commander=commander, - traced=todo.order.owner, - ), - ) + comms.goals.update_goal(unit.name, OrderGoalAction( + problem=todo.id, expected_revision=todo.revision, order=order, + request=todo.goal.request(), deadline=todo.deadline, commander=commander, + traced=todo.order.owner)) comms.messaging.send(commander, unit.name, f"{self.line()}\n{order.instructions(todo)}") @@ -128,46 +106,29 @@ def violations(cls, comms: Comms, commander: str) -> tuple[FlatWheelSpin, ...]: held = tuple(todo for todo in cls.todos(comms, commander) if todo.pending) pulls = PullRequests(held) storage = comms.registry.storage_rule - trends = ( - (todo, OrderProgress.read(todo, storage, pulls).distance_readings().trend) - for todo in held - ) + trends = ((todo, OrderProgress.read(todo, storage, pulls).distance_readings().trend) + for todo in held) flat = ((todo, trend.since) for todo, trend in trends if isinstance(trend, FlatTrend)) - return tuple( - cls(todo, since) - for todo, since in flat - if TurnDigestRead( - comms.root, comms.registry.require(todo.held().owner), TurnsSince(since) - ) - .read() - .turns - ) + return tuple(cls(todo, since) for todo, since in flat + if TurnDigestRead(comms.root, comms.registry.require(todo.held().owner), + TurnsSince(since)).read().turns) def began(self) -> float: return self.since def line(self) -> str: - return ( - f"WHEEL SPIN: {self.todo.holder} kept working on {self.todo.id} while its distance" - " to done did not fall across two readings -> Core strikes it and gives the order" - " to an idle unit" - ) + return (f"WHEEL SPIN: {self.todo.holder} kept working on {self.todo.id} while its distance" + " to done did not fall across two readings -> Core strikes it and gives the order" + " to an idle unit") def enforce(self, comms: Comms, commander: str) -> None: todo = comms.goals.todos.get(self.todo.id) held = todo.held() if not todo.struck_on_order(held.owner): - comms.goals.change_problem( - todo.id, - StrikeTodo( - todo.revision, - comms.registry.require(commander).incarnation, - held, - f"{held.owner} kept working on {todo.id} while the distance to done did not fall" - " across two readings", - ), - commander, - ) + comms.goals.change_problem(todo.id, StrikeTodo( + todo.revision, comms.registry.require(commander).incarnation, held, + f"{held.owner} kept working on {todo.id} while the distance to done did not fall" + " across two readings"), commander) for unit in self.idle(comms, commander)[:1]: self.assign(comms, commander, unit) @@ -180,21 +141,14 @@ class UnownedWheelSpin(WheelSpin, UndatedStandingRule): @classmethod def violations(cls, comms: Comms, commander: str) -> tuple[UnownedWheelSpin, ...]: - rows = ( - todo - for todo in comms.goals.todos.for_commander( - comms.registry.require(commander).incarnation - ) - if todo.pending and not todo.holder - ) + rows = (todo for todo in comms.goals.todos.for_commander( + comms.registry.require(commander).incarnation) if todo.pending and not todo.holder) idle = cls.idle(comms, commander) return tuple(cls(todo, unit.name) for unit, todo in zip(idle, rows, strict=False)) def line(self) -> str: - return ( - f"WHEEL SPIN: {self.unit} holds no order while {self.todo.id} is unowned" - f" -> Core gives it to {self.unit}" - ) + return (f"WHEEL SPIN: {self.unit} holds no order while {self.todo.id} is unowned" + f" -> Core gives it to {self.unit}") def enforce(self, comms: Comms, commander: str) -> None: self.assign(comms, commander, comms.registry.require(self.unit)) diff --git a/src/agent_comms/work_branches.py b/src/agent_comms/work_branches.py index ba9268e546..d0a86e51c2 100644 --- a/src/agent_comms/work_branches.py +++ b/src/agent_comms/work_branches.py @@ -10,6 +10,7 @@ import subprocess from dataclasses import dataclass +from datetime import UTC, datetime from pathlib import Path from typing import TYPE_CHECKING, ClassVar @@ -162,12 +163,12 @@ class WorkBranches: } }""" - def _run(self, *command: str, input: str | None = None) -> str: + def _run(self, *command: str, input: str = "") -> str: result = subprocess.run( command, cwd=self.checkout, capture_output=True, - input=input.encode() if input is not None else None, + input=input.encode(), timeout=self.timeout, ) if result.returncode != 0: @@ -278,7 +279,7 @@ def read(self, snapshot: RegistrySnapshot) -> tuple[WorkBranch, ...]: WorkBranch( branch, ahead, - date, + datetime.fromisoformat(date).astimezone(UTC).isoformat().replace("+00:00", "Z"), tuple(links.get(branch, ())), tuple(self._checkout(snapshot, path) for path in worktrees.get(branch, ())), ) diff --git a/tests/test_work_branches.py b/tests/test_work_branches.py index 46cda4ff4e..009ff613b1 100644 --- a/tests/test_work_branches.py +++ b/tests/test_work_branches.py @@ -1,55 +1,56 @@ import json +import subprocess from agent_comms.field_codec import FieldCodec -from agent_comms.threads import Thread from agent_comms.work_branches import ( - IssueBranchLink, PullRequestBranchLink, ThreadBranchCheckout, UnregisteredBranchCheckout, + IssueBranchLink, + PullRequestBranchLink, + ThreadBranchCheckout, + UnregisteredBranchCheckout, WorkBranches, ) -def _ref(name, unmerged, date, *pulls): - return {"name": name, "target": {"committedDate": date}, "compare": {"behindBy": unmerged}, - "associatedPullRequests": {"nodes": [{"number": n, "headRefName": head} - for n, head in pulls]}} - - def _page(field, nodes, after=None): return json.dumps({"data": {"repository": {field: { "nodes": nodes, "pageInfo": {"hasNextPage": after is not None, "endCursor": after}}}}}) def test_branches_off_main_carry_only_their_own_pr_issue_and_checkout(comms, tmp_path, monkeypatch): - comms.registry.declare(comms.registry.new_thread(name='unit', tags=frozenset(), worktree=str(tmp_path / 'unit'))) + comms.registry.declare(comms.registry.new_thread( + name="unit", tags=frozenset(), worktree=str(tmp_path / "unit"))) (tmp_path / "unit").mkdir() - answers = { - "repo": "Org/repo\n", - "worktree": (f"worktree {tmp_path / 'unit'}\nHEAD 1\nbranch refs/heads/fix/a\n\n" - f"worktree {tmp_path / 'stray'}\nHEAD 2\nbranch refs/heads/base\n"), + commits = { + "main": (0, "2026-10-16T10:00:00Z"), + "fix/a": (3, "2026-10-16T12:00:00Z"), + "base": (2, "2026-10-16T13:00:00+02:00"), + "fix/pr": (1, "2026-10-16T09:00:00Z"), + "fix/issue": (4, "2026-10-16T08:00:00Z"), + "merged": (0, "2026-10-16T13:00:00Z"), + } + pages = { "issues": [_page("issues", [{"number": 7, "linkedBranches": { "nodes": [{"ref": {"name": "fix/issue"}}]}}])], - "refs": [ - _page("refs", [_ref("main", 0, "2026-10-16T10:00:00Z"), - _ref("fix/a", 3, "2026-10-16T12:00:00Z"), - # PR 9 only uses base as its base branch: base is unlinked. - _ref("base", 2, "2026-10-16T11:00:00Z", (9, "fix/pr"))], after="c1"), - _page("refs", [_ref("fix/pr", 1, "2026-10-16T09:00:00Z", (9, "fix/pr")), - _ref("fix/issue", 4, "2026-10-16T08:00:00Z"), - _ref("merged", 0, "2026-10-16T13:00:00Z")]), - ], + "pulls": [_page("pullRequests", [{"number": 9, "headRefName": "fix/pr"}], after="c1"), + _page("pullRequests", [])], } - def run(self, *command): - if command[:3] == ("gh", "repo", "view"): - return answers["repo"] + def run(self, *command, input=None): + if command[:2] == ("git", "ls-remote"): + return "\n".join(f"{name} refs/heads/{name}" for name in commits) + if command[:2] == ("git", "cat-file"): + return "\n".join(f"{name} commit" for name in commits) + if command[:2] == ("git", "for-each-ref"): + return "\n".join(f"{name} {date} {ahead} 0" for name, (ahead, date) in commits.items()) if command[:2] == ("git", "worktree"): - return answers["worktree"] - query = command[4] - return answers["refs" if "refs(" in query else "issues"].pop(0) + return (f"worktree {tmp_path / 'unit'}\nHEAD 1\nbranch refs/heads/fix/a\n\n" + f"worktree {tmp_path / 'stray'}\nHEAD 2\nbranch refs/heads/base\n") + assert command[:3] == ("gh", "api", "graphql") + return pages["pulls" if "pullRequests(" in command[4] else "issues"].pop(0) + monkeypatch.setattr(WorkBranches, "repository", property(lambda self: ("Org", "repo"))) monkeypatch.setattr(WorkBranches, "_run", run) branches = WorkBranches(tmp_path).read(comms.registry.snapshot()) - assert [(b.name, b.unmerged_commits, b.links, b.checkouts) for b in branches] == [ ("fix/a", 3, (), (ThreadBranchCheckout(("unit",), str(tmp_path / "unit")),)), ("base", 2, (), (UnregisteredBranchCheckout(str(tmp_path / "stray")),)), @@ -57,3 +58,51 @@ def run(self, *command): ("fix/issue", 4, (IssueBranchLink(7),), ()), ] assert [FieldCodec.encode(b)["linked"] for b in branches] == [False, False, True, True] + assert branches[1].committed_at == "2026-10-16T11:00:00Z" + + +def test_live_remote_heads_fetch_missing_objects_without_using_stale_tracking_refs( + comms, tmp_path, monkeypatch, +): + origin, publisher, reader = (tmp_path / name for name in ("origin.git", "publisher", "reader")) + for path in (publisher, reader): + path.mkdir() + + def git(path, *arguments): + return subprocess.run(["git", "-C", str(path), "-c", "user.name=probe", + "-c", "user.email=probe@example.invalid", *arguments], check=True, + capture_output=True, text=True).stdout.strip() + + subprocess.run(["git", "init", "--bare", "-q", str(origin)], check=True) + git(publisher, "init", "-q", "-b", "main") + git(publisher, "remote", "add", "origin", str(origin)) + git(publisher, "commit", "-q", "--allow-empty", "-m", "original main") + git(publisher, "push", "-q", "origin", "main") + git(reader, "init", "-q", "-b", "main") + git(reader, "remote", "add", "origin", str(origin)) + git(reader, "fetch", "-q", "origin", "main") + git(reader, "checkout", "-q", "-B", "main", "origin/main") + stale = git(reader, "rev-parse", "origin/main") + git(publisher, "checkout", "-q", "-b", "fresh") + git(publisher, "commit", "-q", "--allow-empty", "-m", "fresh branch") + fresh = git(publisher, "rev-parse", "HEAD") + git(publisher, "push", "-q", "origin", "fresh") + git(publisher, "checkout", "-q", "main") + git(publisher, "commit", "-q", "--allow-empty", "-m", "advanced main") + main = git(publisher, "rev-parse", "HEAD") + git(publisher, "push", "-q", "origin", "main") + run = WorkBranches._run + fetched = [] + + def observe(self, *command, **options): + if command[:2] == ("git", "fetch"): + fetched.append(command) + return run(self, *command, **options) + + # Forge links are independent of the real Git graph under examination. + monkeypatch.setattr(WorkBranches, "_pages", lambda *args, **kwargs: ()) + monkeypatch.setattr(WorkBranches, "_run", observe) + branches = WorkBranches(reader).read(comms.registry.snapshot()) + assert [(branch.name, branch.unmerged_commits) for branch in branches] == [("fresh", 1)] + assert len(fetched) == 1 and set(fetched[0][4:]) == {main, fresh} + assert git(reader, "rev-parse", "origin/main") == stale diff --git a/tests/verify_connector_tools.py b/tests/verify_connector_tools.py index 03681927ab..ccf089639d 100644 --- a/tests/verify_connector_tools.py +++ b/tests/verify_connector_tools.py @@ -348,7 +348,7 @@ def run(output: Path, checkout: Path, selected: list[str]) -> None: wait_started(service, []) with ConnectorStore.at(store_path).granted("scratch timing") as token: - def call(payload): + def call(payload, case=case, port=port, service=service): request = case / (payload["method"].replace("/", "-") + "-request.json") request.write_text(json.dumps(payload)) curl = subprocess.Popen( From edc5b7be138e78b37e09f2a97376ecfb2dd4cb7c Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:52:52 -0400 Subject: [PATCH 06/25] P251: keep fork pull requests separate and record complete timing runs --- src/agent_comms/work_branches.py | 15 +++++-- tests/test_work_branches.py | 68 +++++++++++++++++++++++++------- tests/verify_connector_tools.py | 8 +++- 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/agent_comms/work_branches.py b/src/agent_comms/work_branches.py index d0a86e51c2..d9d27e479f 100644 --- a/src/agent_comms/work_branches.py +++ b/src/agent_comms/work_branches.py @@ -97,6 +97,13 @@ class Connection[Node](ForgeAnswer): class ForgePullRequest(ForgeAnswer): number: int head_ref_name: str + is_cross_repository: bool + + @property + def branch_links(self) -> tuple[tuple[str, PullRequestBranchLink], ...]: + # A fork's identically spelled branch is not this origin's branch. + return () if self.is_cross_repository else ( + (self.head_ref_name, PullRequestBranchLink(self.number)),) class ForgeRefName(ForgeAnswer): @@ -149,7 +156,7 @@ class WorkBranches: repository(owner: $owner, name: $name) { pullRequests(states: OPEN, first: 100, after: $after) { pageInfo { hasNextPage endCursor } - nodes { number headRefName } + nodes { number headRefName isCrossRepository } } } }""" @@ -257,7 +264,8 @@ def read(self, snapshot: RegistrySnapshot) -> tuple[WorkBranch, ...]: } links: dict[str, list[BranchLink]] = {} for pull in self._pages(self._PULLS, ForgeQuery[PullsRepository], owner=owner, name=name): - links.setdefault(pull.head_ref_name, []).append(PullRequestBranchLink(pull.number)) + for branch, link in pull.branch_links: + links.setdefault(branch, []).append(link) for issue in self._pages( self._ISSUES, ForgeQuery[IssuesRepository], owner=owner, name=name ): @@ -279,7 +287,8 @@ def read(self, snapshot: RegistrySnapshot) -> tuple[WorkBranch, ...]: WorkBranch( branch, ahead, - datetime.fromisoformat(date).astimezone(UTC).isoformat().replace("+00:00", "Z"), + datetime.fromisoformat(date).astimezone(UTC).isoformat().replace( + "+00:00", "Z"), tuple(links.get(branch, ())), tuple(self._checkout(snapshot, path) for path in worktrees.get(branch, ())), ) diff --git a/tests/test_work_branches.py b/tests/test_work_branches.py index 009ff613b1..ab514e79d9 100644 --- a/tests/test_work_branches.py +++ b/tests/test_work_branches.py @@ -12,13 +12,24 @@ def _page(field, nodes, after=None): - return json.dumps({"data": {"repository": {field: { - "nodes": nodes, "pageInfo": {"hasNextPage": after is not None, "endCursor": after}}}}}) + return json.dumps( + { + "data": { + "repository": { + field: { + "nodes": nodes, + "pageInfo": {"hasNextPage": after is not None, "endCursor": after}, + } + } + } + } + ) def test_branches_off_main_carry_only_their_own_pr_issue_and_checkout(comms, tmp_path, monkeypatch): - comms.registry.declare(comms.registry.new_thread( - name="unit", tags=frozenset(), worktree=str(tmp_path / "unit"))) + comms.registry.declare( + comms.registry.new_thread(name="unit", tags=frozenset(), worktree=str(tmp_path / "unit")) + ) (tmp_path / "unit").mkdir() commits = { "main": (0, "2026-10-16T10:00:00Z"), @@ -29,10 +40,23 @@ def test_branches_off_main_carry_only_their_own_pr_issue_and_checkout(comms, tmp "merged": (0, "2026-10-16T13:00:00Z"), } pages = { - "issues": [_page("issues", [{"number": 7, "linkedBranches": { - "nodes": [{"ref": {"name": "fix/issue"}}]}}])], - "pulls": [_page("pullRequests", [{"number": 9, "headRefName": "fix/pr"}], after="c1"), - _page("pullRequests", [])], + "issues": [ + _page( + "issues", + [{"number": 7, "linkedBranches": {"nodes": [{"ref": {"name": "fix/issue"}}]}}], + ) + ], + "pulls": [ + _page( + "pullRequests", + [ + {"number": 9, "headRefName": "fix/pr", "isCrossRepository": False}, + {"number": 10, "headRefName": "base", "isCrossRepository": True}, + ], + after="c1", + ), + _page("pullRequests", []), + ], } def run(self, *command, input=None): @@ -43,8 +67,10 @@ def run(self, *command, input=None): if command[:2] == ("git", "for-each-ref"): return "\n".join(f"{name} {date} {ahead} 0" for name, (ahead, date) in commits.items()) if command[:2] == ("git", "worktree"): - return (f"worktree {tmp_path / 'unit'}\nHEAD 1\nbranch refs/heads/fix/a\n\n" - f"worktree {tmp_path / 'stray'}\nHEAD 2\nbranch refs/heads/base\n") + return ( + f"worktree {tmp_path / 'unit'}\nHEAD 1\nbranch refs/heads/fix/a\n\n" + f"worktree {tmp_path / 'stray'}\nHEAD 2\nbranch refs/heads/base\n" + ) assert command[:3] == ("gh", "api", "graphql") return pages["pulls" if "pullRequests(" in command[4] else "issues"].pop(0) @@ -62,16 +88,30 @@ def run(self, *command, input=None): def test_live_remote_heads_fetch_missing_objects_without_using_stale_tracking_refs( - comms, tmp_path, monkeypatch, + comms, + tmp_path, + monkeypatch, ): origin, publisher, reader = (tmp_path / name for name in ("origin.git", "publisher", "reader")) for path in (publisher, reader): path.mkdir() def git(path, *arguments): - return subprocess.run(["git", "-C", str(path), "-c", "user.name=probe", - "-c", "user.email=probe@example.invalid", *arguments], check=True, - capture_output=True, text=True).stdout.strip() + return subprocess.run( + [ + "git", + "-C", + str(path), + "-c", + "user.name=probe", + "-c", + "user.email=probe@example.invalid", + *arguments, + ], + check=True, + capture_output=True, + text=True, + ).stdout.strip() subprocess.run(["git", "init", "--bare", "-q", str(origin)], check=True) git(publisher, "init", "-q", "-b", "main") diff --git a/tests/verify_connector_tools.py b/tests/verify_connector_tools.py index ccf089639d..65cad711ec 100644 --- a/tests/verify_connector_tools.py +++ b/tests/verify_connector_tools.py @@ -186,7 +186,10 @@ def arguments(comms: Comms, name: str, project: Path, checkout: Path) -> dict: "comms_unjudged": {}, "comms_fire": { "unit": "worker", - "reason": "The scratch unit changed the saved file without reading its original contents first.", + "reason": ( + "The scratch unit changed the saved file without reading " + "its original contents first." + ), }, "comms_correct_wording": { "unit": "worker", @@ -419,6 +422,9 @@ def call(payload, case=case, port=port, service=service): service.terminate() service.communicate(timeout=15) assert len(rows) == len(names) + (output / "completed.json").write_text( + json.dumps({"tools": len(rows), "completed": True}) + "\n" + ) if __name__ == "__main__": From f437ff78db93d2cd56e87d685b5d721fad14a178 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:57:18 -0400 Subject: [PATCH 07/25] P251: declare the repository owner dependency at module scope --- src/agent_comms/work_branches.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/agent_comms/work_branches.py b/src/agent_comms/work_branches.py index d9d27e479f..c075cdd61f 100644 --- a/src/agent_comms/work_branches.py +++ b/src/agent_comms/work_branches.py @@ -18,6 +18,7 @@ from pydantic.alias_generators import to_camel from .cli_protocol import WireModel +from .commander.config import Repo from .declared_family import DeclaredFamily from .field_codec import projected @@ -185,8 +186,6 @@ def _run(self, *command: str, input: str = "") -> str: @property def repository(self) -> tuple[str, str]: - from .commander.config import Repo - owner, name = Repo.of(self.checkout).github.split("/") return owner, name From 3e2242409cc402cbb1dd77ee1a18435bf11ce409 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:40:56 -0400 Subject: [PATCH 08/25] P262 resolve configured check repositories once before recording --- src/agent_comms/commander/config.py | 5 +++++ src/agent_comms/commander/order.py | 23 +++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/agent_comms/commander/config.py b/src/agent_comms/commander/config.py index f6607f7d09..c4c11c4e7f 100644 --- a/src/agent_comms/commander/config.py +++ b/src/agent_comms/commander/config.py @@ -81,6 +81,11 @@ def packaged_skill() -> Path: def installed(cls) -> CommanderConfig: return cls.model_validate_json(cls.path().read_text()) + def repo_directory(self, name: str) -> Path: + """A configured GitHub repo name takes precedence over a directory spelling.""" + return next((repo.checkout for repo in self.repos + if repo.github.rsplit('/', 1)[-1] == name), Path(name)).resolve() + def write(self, home: Path) -> Path: """Record the config and start each state file empty, so every tool reads a real file.""" self.require_commander(Comms(self.root).registry.require(self.commander)) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index de6a972d2e..4b175f292e 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -19,7 +19,7 @@ """ import argparse, asyncio, json, os, re, subprocess, sys, time from abc import abstractmethod -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from uuid import uuid4 @@ -177,6 +177,17 @@ def forwarders_added(diff, sources=()): if m.group(2) not in kept and not state_call.search(diff[m.start():m.end() + 200].split('\n+', 2)[1] if '\n+' in diff[m.start():m.end()+200] else '')]) +def read_check(line: str) -> OrderCheck | None: + """Resolve the commander's check boundary once; stored checks only carry directories.""" + check = OrderCheck.parse(line) + if check: + directory = CONFIG.repo_directory(check.directory) + if not directory.is_dir(): + sys.exit(f'Refused: Check repo {check.directory} is not a directory') + check = replace(check, directory=str(directory)) + return check + + def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new=None, traced=None, replaces=None, disk=False, finding=UnlocatedProblemFinding()): """Validate an order body and record it in Core: a new problem (new=(PID, TEXT)) or the listed open problems. @@ -209,7 +220,7 @@ def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new fact = re.search(r'(?m)^Problems?:\s*(P\d+(?:\s*,\s*P\d+)*)\b', body) group = [new[0]] if new else re.findall(r'P\d+', fact[1]) if fact else [] owner = re.search(r'(?m)^Owner:\s*(\S+):(\d+)\s*$', body) - check = (line := re.search(r'(?m)^Check:(.*)$', body)) and OrderCheck.parse(line[1]) + check = (line := re.search(r'(?m)^Check:(.*)$', body)) and read_check(line[1]) listed = {t.id: t for t in todos()} if not group: problems.append('no Problem: P line; record it first with `order problem` (every order owns a listed problem)') @@ -221,12 +232,10 @@ def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new problems.append(f'Owner {owner[1]}:{owner[2]} does not exist; read the code and name the real owner') if not check: problems.append('no Check: :: -> N; done runs it, so make it the real acceptance count') - elif not os.path.isdir(check.directory): - problems.append(f'Check repo {check.directory} is not a directory') # A check that can never print its target, or a premise naming lines that do something else, wastes the order. # An order's check must fail now (or it proves nothing), and every absolute # path:line it cites must exist. - if check and os.path.isdir(check.directory) and not problems: + if check and not problems: try: ran = subprocess.run(['bash', '-c', check.command], cwd=check.directory, capture_output=True, timeout=90) printed = CheckPrinted.decoded(ran.stdout) @@ -328,10 +337,8 @@ def correct_check(pid: str, line: str, why: str) -> None: todo = COMMS.goals.todos.get(pid) if not todo.state.open or not todo.pending: sys.exit(f'Refused: {pid} has no pending order') - if not (check := OrderCheck.parse(line)): + if not (check := read_check(line)): sys.exit('Refused: --check reads " :: -> "') - if not os.path.isdir(check.directory): - sys.exit(f'Refused: check repo {check.directory} is not a directory') if len(why.split()) < 8: sys.exit('Refused: --why says what was wrong with the recorded check (8+ words)') def git(*args: str) -> str: From ac3df189b6da7b3592ffb8cbece1778ac016ed10 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:51:01 -0400 Subject: [PATCH 09/25] P262 keep decoded checks nonnullable and verify both command paths The name-to-configured-checkout boundary adds nine production lines after deleting the duplicated refusal and repeated directory probes. No debt-count suppressions. Src-Budget: 9 --- src/agent_comms/commander/order.py | 27 ++++++------- tests/test_commander_problem_orders.py | 55 ++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 4b175f292e..83e875478f 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -177,15 +177,14 @@ def forwarders_added(diff, sources=()): if m.group(2) not in kept and not state_call.search(diff[m.start():m.end() + 200].split('\n+', 2)[1] if '\n+' in diff[m.start():m.end()+200] else '')]) -def read_check(line: str) -> OrderCheck | None: +def read_check(line: str) -> OrderCheck: """Resolve the commander's check boundary once; stored checks only carry directories.""" - check = OrderCheck.parse(line) - if check: - directory = CONFIG.repo_directory(check.directory) - if not directory.is_dir(): - sys.exit(f'Refused: Check repo {check.directory} is not a directory') - check = replace(check, directory=str(directory)) - return check + if not (check := OrderCheck.parse(line)): + sys.exit('Refused: Check reads " :: -> "') + directory = CONFIG.repo_directory(check.directory) + if not directory.is_dir(): + sys.exit(f'Refused: Check repo {check.directory} is not a directory') + return replace(check, directory=str(directory)) def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new=None, @@ -220,7 +219,8 @@ def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new fact = re.search(r'(?m)^Problems?:\s*(P\d+(?:\s*,\s*P\d+)*)\b', body) group = [new[0]] if new else re.findall(r'P\d+', fact[1]) if fact else [] owner = re.search(r'(?m)^Owner:\s*(\S+):(\d+)\s*$', body) - check = (line := re.search(r'(?m)^Check:(.*)$', body)) and read_check(line[1]) + line = re.search(r'(?m)^Check:(.*)$', body) + check = read_check(line[1] if line else '') listed = {t.id: t for t in todos()} if not group: problems.append('no Problem: P line; record it first with `order problem` (every order owns a listed problem)') @@ -230,12 +230,10 @@ def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new problems.append('no Owner: path:line of the existing owner the fix belongs on') elif not os.path.exists(owner[1]) or int(owner[2]) > sum(1 for _ in open(owner[1])): problems.append(f'Owner {owner[1]}:{owner[2]} does not exist; read the code and name the real owner') - if not check: - problems.append('no Check: :: -> N; done runs it, so make it the real acceptance count') # A check that can never print its target, or a premise naming lines that do something else, wastes the order. # An order's check must fail now (or it proves nothing), and every absolute # path:line it cites must exist. - if check and not problems: + if not problems: try: ran = subprocess.run(['bash', '-c', check.command], cwd=check.directory, capture_output=True, timeout=90) printed = CheckPrinted.decoded(ran.stdout) @@ -249,7 +247,7 @@ def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new # P12 deleted acp_ingress.py, which Toad imports. deleted = {stem for edit in edits for stem in re.findall(r'(?i)\bdelete\s+`?(?:\S*/)?(\w+)\.py\b', edit)} - if check and deleted and (unread := importers(deleted, check)): + if deleted and (unread := importers(deleted, check)): problems.append(f'the order deletes {sorted(deleted)}, which {unread} import; ' 'the check must also count their `from agent_comms` imports') if any(re.search(r'(? None: todo = COMMS.goals.todos.get(pid) if not todo.state.open or not todo.pending: sys.exit(f'Refused: {pid} has no pending order') - if not (check := read_check(line)): - sys.exit('Refused: --check reads " :: -> "') + check = read_check(line) if len(why.split()) < 8: sys.exit('Refused: --why says what was wrong with the recorded check (8+ words)') def git(*args: str) -> str: diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 2d7c880cdc..6052ac9a2a 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -565,6 +565,61 @@ def test_installed_order_entrypoint_refuses_then_accepts_printed_replacement( print(f'Accepted --replaces {replacement}: {accepted.stdout.strip()}') +@pytest.mark.parametrize('spelling', ['agent-comms', '.', 'absolute']) +def test_check_repo_resolves_once_for_send_and_correction( + commander, commander_script, monkeypatch, tmp_path, spelling): + comms, _ = commander + script, repo = commander_script + monkeypatch.setattr(script, 'CONFIG', script.CONFIG.model_copy(update={ + 'repos': (script.Repo.of(repo),)})) + monkeypatch.chdir(repo) + # A same-named directory must not override the configured repository. + (repo / 'agent-comms').mkdir() + (repo / 'count.txt').write_text('0\n') + marker = tmp_path / 'check-cwd' + command = f'pwd > {shlex.quote(str(marker))}; cat count.txt' + git = lambda *args: subprocess.run( + ['git', '-C', str(repo), '-c', 'user.name=t', '-c', 'user.email=t@t', *args], + check=True, capture_output=True, text=True).stdout.strip() + git('add', 'count.txt') + git('commit', '-qm', 'base') + git('update-ref', 'refs/remotes/origin/main', 'HEAD') + directory = str(repo) if spelling == 'absolute' else spelling + body = order_body(repo, f'{command} -> 1') + Path(body).write_text(Path(body).read_text().replace(f'Check: {repo}', f'Check: {directory}')) + problem(script, body) + sent = comms.goals.todos.get('P1') + assert sent.order.check.directory == str(repo.resolve()) + assert marker.read_text().strip() == str(repo.resolve()) + script.correct_check('P1', f'{directory} :: {command} -> 0', + 'The recorded expected count was wrong; the unchanged file contains zero.') + corrected = comms.goals.todos.get('P1') + assert corrected.order.id == sent.order.id + assert corrected.order.check.directory == str(repo.resolve()) + assert marker.read_text().strip() == str(repo.resolve()) + + +@pytest.mark.parametrize('entrypoint', ['send', 'correct']) +def test_missing_check_repo_refuses_before_recording( + commander, commander_script, tmp_path, entrypoint): + comms, _ = commander + script, repo = commander_script + (repo / 'count.txt').write_text('0\n') + if entrypoint == 'correct': + problem(script, order_body(repo)) + before = comms.goals.todos.for_commander(comms.registry.require('lead').incarnation) + missing = tmp_path / 'missing-repo' + with pytest.raises(SystemExit, match='is not a directory'): + if entrypoint == 'send': + body = order_body(repo) + Path(body).write_text(Path(body).read_text().replace(f'Check: {repo}', f'Check: {missing}')) + problem(script, body) + else: + script.correct_check('P1', f'{missing} :: printf 0 -> 0', + 'The recorded check must run in the correct configured source checkout.') + assert comms.goals.todos.for_commander(comms.registry.require('lead').incarnation) == before + + def test_resend_after_strike_counts_one_miss(commander, commander_script, monkeypatch): """A miss struck with `order strike` and then sent back is one strike; the next send-back strikes again.""" comms, _ = commander From ccd17007a98654f2a6418880e42219076426bf34 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 14:55:14 -0400 Subject: [PATCH 10/25] P228: return typed tool connection outcomes and close each live writer once Src-Budget: 32 --- src/agent_comms/native_tool_transport.py | 99 ++++++++++++++++-------- 1 file changed, 67 insertions(+), 32 deletions(-) diff --git a/src/agent_comms/native_tool_transport.py b/src/agent_comms/native_tool_transport.py index e28aa3b9fe..0fef11af59 100644 --- a/src/agent_comms/native_tool_transport.py +++ b/src/agent_comms/native_tool_transport.py @@ -7,8 +7,9 @@ import os import socket import struct +from abc import ABC, abstractmethod from collections.abc import Callable -from contextlib import AsyncExitStack +from contextlib import AsyncExitStack, ExitStack from dataclasses import dataclass from functools import cached_property from pathlib import Path @@ -28,6 +29,42 @@ Call = TypeVar("Call", bound=NativeToolCall) +class ToolConnectionOutcome(ABC): + """One response attempt decides whether the owner still has a writer to close.""" + + @classmethod + async def send(cls, writer: asyncio.StreamWriter, response: dict[str, object], + answer: str) -> ToolConnectionOutcome: + try: + writer.write((json.dumps(response) + "\n").encode("ascii")) + await writer.drain() + except ConnectionError: + return ToolChildGone(answer) + return ToolAnswerSent() + + @abstractmethod + def close(self, ownership: ExitStack) -> None: ... + + +class ToolAnswerSent(ToolConnectionOutcome): + def close(self, ownership: ExitStack) -> None: + ownership.close() + + +@dataclass(frozen=True) +class ToolChildGone(ToolConnectionOutcome): + answer: str + + def close(self, ownership: ExitStack) -> None: + # asyncio has already closed the failed transport. Do not close it or + # await its failed close future a second time. + ownership.pop_all() + _LOG.warning( + "The Pi child stopped waiting for a tool check before its answer " + "(%s) could be sent", self.answer, + ) + + @dataclass(frozen=True, slots=True) class ToolCallIdentity: """A provider call id in the assistant tool round that declared it.""" @@ -281,37 +318,35 @@ async def _handle(self, reader: asyncio.StreamReader, writer: asyncio.StreamWrit assert task is not None self._handlers.add(task) try: + with ExitStack() as connection: + connection.callback(writer.close) + outcome = await self._reply(reader, writer) + outcome.close(connection) + finally: + self._handlers.discard(task) + + async def _reply(self, reader: asyncio.StreamReader, + writer: asyncio.StreamWriter) -> ToolConnectionOutcome: + try: + if not self._server.is_serving(): + raise SelectedToolDenied("Owner tool socket is closed") + sock = writer.get_extra_info("socket") + if sock is None or self.expected_pid is None: + raise SelectedToolDenied("Selected tool child is not bound") + pid, uid, _gid = struct.unpack( + "3i", sock.getsockopt(socket.SOL_SOCKET, socket.SO_PEERCRED, 12) + ) + if pid != self.expected_pid or uid != os.geteuid(): + raise SelectedToolDenied("Selected tool peer is not the launched Pi child") try: - if not self._server.is_serving(): - raise SelectedToolDenied("Owner tool socket is closed") - sock = writer.get_extra_info("socket") - if sock is None or self.expected_pid is None: - raise SelectedToolDenied("Selected tool child is not bound") - pid, uid, _gid = struct.unpack( - "3i", sock.getsockopt(socket.SOL_SOCKET, socket.SO_PEERCRED, 12) - ) - if pid != self.expected_pid or uid != os.geteuid(): - raise SelectedToolDenied("Selected tool peer is not the launched Pi child") raw = await asyncio.wait_for(reader.readline(), timeout=10) - response = await self.handle_request(raw) - answer = "allowed" - except (ValueError, TimeoutError) as error: - # SelectedToolDenied (a ValueError) is every typed denial and - # UNKNOWN outcome; ValueError also covers the untrusted child's - # undecodable request. Any other failure is a defect: it - # propagates and the child sees a closed socket, never a reply. - response = self.failure_response(error) - answer = "refused" - try: - writer.write((json.dumps(response) + "\n").encode("ascii")) - await writer.drain() except ConnectionError: - # Not an owner failure: the child gave up and told the model - # so. Whatever the owner did for this call stays recorded. - _LOG.warning( - "The Pi child stopped waiting for a tool check before its answer " - "(%s) could be sent", answer, - ) - finally: - writer.close() - self._handlers.discard(task) + return ToolChildGone("unanswered") + response = await self.handle_request(raw) + answer = "allowed" + except (ValueError, TimeoutError) as error: + # Only declared denials and malformed child input become a refusal. + # Owner defects propagate; they never become a successful answer. + response = self.failure_response(error) + answer = "refused" + return await ToolConnectionOutcome.send(writer, response, answer) From 917c5a1365d3dcc650e042aa80a48a7dafca6f75 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 15:12:12 -0400 Subject: [PATCH 11/25] P228: prove a real Pi child disconnect and cover both socket policies --- tests/test_channel_coding_tools.py | 16 +++-- tests/test_native_tool_disconnect.py | 95 ++++++++++++++++++++++++++++ tests/test_native_tool_lifecycle.py | 41 ++++++++++++ 3 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 tests/test_native_tool_disconnect.py diff --git a/tests/test_channel_coding_tools.py b/tests/test_channel_coding_tools.py index 914b4edde3..7795c8b4c0 100644 --- a/tests/test_channel_coding_tools.py +++ b/tests/test_channel_coding_tools.py @@ -243,7 +243,8 @@ async def allow(self, turn, round_number, call): @pytest.mark.asyncio -async def test_child_that_stopped_waiting_is_logged_not_raised(tmp_path, caplog): +@pytest.mark.parametrize("refused", [False, True]) +async def test_child_that_stopped_waiting_is_logged_not_raised(tmp_path, caplog, refused): """The live owner logged two tracebacks when Pi gave up on a slow tool check.""" import asyncio import os @@ -254,15 +255,17 @@ class Owner: input_id = secrets.token_hex(16) async def allow(self, turn, round_number, call): + from agent_comms.native_tool_call import SelectedToolDenied + answered.set() + if refused: + raise SelectedToolDenied("Original owner refusal") return NoRoomNeeded() tmp_path.chmod(0o700) socket = CodingToolSocket(tmp_path, secrets.token_hex(32), Owner()) socket.turn = object() socket.expected_pid = os.getpid() - unhandled = [] - asyncio.get_running_loop().set_exception_handler(lambda _loop, context: unhandled.append(context)) await socket.start() try: call = ("slow", "read", {"path": "a"}) @@ -281,8 +284,11 @@ async def allow(self, turn, round_number, call): await asyncio.sleep(0.01) finally: await socket.close() - assert not unhandled - assert "stopped waiting for a tool check before its answer (allowed)" in caplog.text + assert "Unhandled exception" not in caplog.text + answer = "refused" if refused else "allowed" + warnings = [record for record in caplog.records if record.name == "agent_comms.native_tool_transport"] + assert len(warnings) == 1 + assert f"stopped waiting for a tool check before its answer ({answer})" in caplog.text @pytest.mark.asyncio diff --git a/tests/test_native_tool_disconnect.py b/tests/test_native_tool_disconnect.py new file mode 100644 index 0000000000..53649a7d97 --- /dev/null +++ b/tests/test_native_tool_disconnect.py @@ -0,0 +1,95 @@ +"""A real saved Pi child abandons its authenticated tool check on a scratch bus.""" + +import asyncio +import json +import signal + +import pytest + +from agent_comms.channel_coding_tools import CodingToolOwner +from agent_comms.field_codec import FieldCodec +from agent_comms.native_pi import NativePiUnavailable +from agent_comms.native_tool_transport import ToolTransport +from agent_comms.pi_rpc import PiRpcChannel +from delivery_owner_fixture import reply_turn_finished + + +async def test_actual_pi_child_aborts_its_tool_socket(native_backend, monkeypatch, caplog): + native = native_backend + await native.author_history() + original = native.session.read_bytes() + (native.project / "readable.txt").write_text("No tool execution is needed for this check.\n") + handled = asyncio.Event() + connections, grants, children = [], [], [] + handle = ToolTransport._handle + allow = CodingToolOwner.allow + next_event = PiRpcChannel.next_event + + async def observe_connection(socket, reader, writer): + connections.append((socket, reader)) + try: + return await handle(socket, reader, writer) + finally: + handled.set() + + async def finish_check_after_disconnect(owner, turn, round_number, call): + reservation = await allow(owner, turn, round_number, call) + child = turn.native.proc + assert child.pid == connections[0][0].expected_pid + children.append(FieldCodec.encode(child.identity)) + # The attested child on this scratch bus dies mid-check; its kernel + # closes the real authenticated socket. No provider/input is replayed. + child.platform.send(child.identity, signal.SIGKILL) + assert await asyncio.wait_for(connections[0][1].read(1), 10) == b"" + grants.append(call.call_id) + return reservation + + async def eof_after_pending_reply(channel): + event = await next_event(channel) + if event is None and connections: + # Observe child failure after its pending socket callback. Otherwise + # turn retirement cancels that callback before its response attempt. + await asyncio.wait_for(handled.wait(), 10) + return event + + def chunks(): + assert native.provider.posts == 1, "No provider retry or tool replay" + yield {"tool_calls": [{"index": 0, "id": "disconnect-check", "type": "function", + "function": {"name": "read", "arguments": json.dumps({"path": "readable.txt"})}}]}, None + yield {}, "tool_calls" + + monkeypatch.setattr(native.provider, "response_chunks", chunks) + monkeypatch.setattr(ToolTransport, "_handle", observe_connection) + monkeypatch.setattr(CodingToolOwner, "allow", finish_check_after_disconnect) + monkeypatch.setattr(PiRpcChannel, "next_event", eof_after_pending_reply) + async with native.open_owner(runtime_enabled=True, auto_wake=True, native_options=()) as (owner, session): + comms = owner._comms + comms.threads.claim_thread("verifier", tags=frozenset(), worktree=str(native.project)) + comms.messaging.send("verifier", session, "Fresh isolated owner socket disconnect verification.").message + await owner.inputs.drain_inbox(session) + with pytest.raises(NativePiUnavailable, match="native exit=-9"): + await asyncio.wait_for(reply_turn_finished(owner, session), 40) + + assert len(connections) == 1 and grants == ["disconnect-check"] and handled.is_set() + assert native.provider.posts == 1 + assert native.session.read_bytes().startswith(original) + messages = [row["message"] for line in native.session.read_text().splitlines() + if (row := json.loads(line)).get("type") == "message"] + tools = [message for message in messages if message.get("role") == "toolResult"] + assert not tools + assert len([message for message in messages if message.get("inputId")]) == 1 + assert not connections[0][0]._handlers + + warnings = [record for record in caplog.records + if record.name == "agent_comms.native_tool_transport" and record.levelname == "WARNING"] + assert len(warnings) == 1 + assert "answer (allowed) could be sent" in warnings[0].getMessage() + assert "Unhandled exception" not in caplog.text and "BrokenPipeError" not in caplog.text + print("P228_OWNER_WARNING " + warnings[0].getMessage()) + print("P228_LIVE_RESULT " + json.dumps({ + "root": str(native.root), "child_identities": children, + "expected_native_failure": "native exit=-9", "input_replayed": False, + "owner_checks": grants, "warnings": len(warnings), "provider_requests": native.provider.posts, + "original_history_preserved": True, "new_inputs": 1, "tool_replays": 0, + "children_retired": all(child.retired for child in native.children), + })) diff --git a/tests/test_native_tool_lifecycle.py b/tests/test_native_tool_lifecycle.py index 9b47fd2319..0ac41099b2 100644 --- a/tests/test_native_tool_lifecycle.py +++ b/tests/test_native_tool_lifecycle.py @@ -340,6 +340,47 @@ async def test_socket_close_cancels_pending_claim_and_closes_client(policy): assert not p.effects +async def test_sent_answer_closes_its_connection_once(policy, monkeypatch): + p = policy + closed = [] + reply = p.socket._reply + + async def observe_reply(reader, writer): + close = writer.close + + def close_once(): + closed.append(writer) + close() + + monkeypatch.setattr(writer, "close", close_once) + return await reply(reader, writer) + + monkeypatch.setattr(p.socket, "_reply", observe_reply) + p.announce() + p.start() + p.socket.expected_pid = os.getpid() + await p.socket.start() + try: + reader, writer = await asyncio.open_unix_connection(str(p.socket.address)) + try: + writer.write(p.raw()) + await writer.drain() + assert json.loads(await reader.readline()) == {"ok": True} + assert await reader.read() == b"" + finally: + writer.close() + await writer.wait_closed() + async with asyncio.timeout(2): + while p.socket._handlers: + await asyncio.sleep(0) + assert len(closed) == 1 and p.effects == ["one"] + p.end() + p.socket.assert_complete() + finally: + await p.socket.close() + assert len(closed) == 1 + + async def test_interleaved_calls_wait_on_their_own_native_start(tmp_path): tmp_path.chmod(0o700) observed = [] From a6fbd6eb44a2dd43d1553ed6acb251f4c0942996 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 15:15:13 -0400 Subject: [PATCH 12/25] P228: observe the registered close callback before the reply begins --- tests/test_native_tool_lifecycle.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/test_native_tool_lifecycle.py b/tests/test_native_tool_lifecycle.py index 0ac41099b2..bb830fb2a0 100644 --- a/tests/test_native_tool_lifecycle.py +++ b/tests/test_native_tool_lifecycle.py @@ -342,19 +342,19 @@ async def test_socket_close_cancels_pending_claim_and_closes_client(policy): async def test_sent_answer_closes_its_connection_once(policy, monkeypatch): p = policy - closed = [] + closed, observed = [], [] reply = p.socket._reply + close = asyncio.StreamWriter.close - async def observe_reply(reader, writer): - close = writer.close - - def close_once(): - closed.append(writer) - close() + def close_once(writer): + closed.append(writer) + close(writer) - monkeypatch.setattr(writer, "close", close_once) + async def observe_reply(reader, writer): + observed.append(writer) return await reply(reader, writer) + monkeypatch.setattr(asyncio.StreamWriter, "close", close_once) monkeypatch.setattr(p.socket, "_reply", observe_reply) p.announce() p.start() @@ -373,12 +373,13 @@ def close_once(): async with asyncio.timeout(2): while p.socket._handlers: await asyncio.sleep(0) - assert len(closed) == 1 and p.effects == ["one"] + assert len(observed) == 1 and closed.count(observed[0]) == 1 + assert p.effects == ["one"] p.end() p.socket.assert_complete() finally: await p.socket.close() - assert len(closed) == 1 + assert closed.count(observed[0]) == 1 async def test_interleaved_calls_wait_on_their_own_native_start(tmp_path): From f3509dc598850636f20fca15dc3b174ee4792465 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:33:47 -0400 Subject: [PATCH 13/25] =?UTF-8?q?P228:=20follow=20main=E2=80=99s=20start-e?= =?UTF-8?q?vent=20lifetime=20in=20the=20unstarted-call=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Account for the three-line source delta introduced by preserving main’s existing listener lifecycle during rebase. Src-Budget: 3 --- tests/test_channel_coding_tools.py | 34 +++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/tests/test_channel_coding_tools.py b/tests/test_channel_coding_tools.py index 7795c8b4c0..1df100d9c2 100644 --- a/tests/test_channel_coding_tools.py +++ b/tests/test_channel_coding_tools.py @@ -292,7 +292,10 @@ async def allow(self, turn, round_number, call): @pytest.mark.asyncio -async def test_tool_call_pi_never_started_is_refused_with_its_reason(tmp_path): +async def test_tool_call_pi_never_started_is_cancelled_by_transport_retirement(tmp_path): + import asyncio + import os + class Owner: input_id = secrets.token_hex(16) @@ -304,12 +307,23 @@ async def allow(self, turn, round_number, call): socket.turn = object() call = ("never", "read", {"path": "a"}) socket.announce([ToolCallContent(id=call[0], name=call[1], arguments=call[2])]) - with pytest.raises(ValueError, match="did not report this tool call as started within 10 s") as refused: - await socket.handle_request(_request(socket, *call)) - # The child shows the model this text; it used to be empty. - assert socket.failure_response(refused.value)["error"].endswith( - "The tool was not started; call it again." - ) - # The call stays refused if Pi asks again; a new call is checked afresh. - with pytest.raises(ValueError, match="not executing"): - await socket.handle_request(_request(socket, *call)) + socket.bind_process(os.getpid()) + await socket.start() + reader, writer = await asyncio.open_unix_connection(str(socket.address)) + try: + writer.write(_request(socket, *call)) + await writer.drain() + async with asyncio.timeout(5): + while not socket._handlers or not socket.calls.require(0, call[0]).progress.started._waiters: + await asyncio.sleep(0) + await socket.close() + assert await reader.read() == b"" + assert not socket._handlers + # Retirement consumes the interrupted request; a late start is not + # permission to replay it. Main deliberately has no ten-second timer. + with pytest.raises(ValueError, match="not executing"): + await socket.handle_request(_request(socket, *call)) + finally: + writer.close() + await writer.wait_closed() + await socket.close() From 90a4d0fb5ef05daa452f230955f308901816b880 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:12:33 -0400 Subject: [PATCH 14/25] P228: construct child-bound policies and refuse only declared input errors Src-Budget: 50 --- src/agent_comms/backend.py | 17 +-- src/agent_comms/channel_coding_tools.py | 32 +++-- src/agent_comms/native_tool_call.py | 4 +- src/agent_comms/native_tool_transport.py | 154 ++++++++++++++--------- src/agent_comms/selected_tool_broker.py | 20 +-- src/agent_comms/turn_inputs.py | 2 +- tests/conftest.py | 13 ++ tests/test_channel_coding_tools.py | 31 +++-- tests/test_native_argument_refusal.py | 7 +- tests/test_native_tool_lifecycle.py | 94 ++++++++++++-- tests/test_selected_tool_broker.py | 67 +++++----- tests/test_tool_refusal_reasons.py | 4 +- 12 files changed, 284 insertions(+), 161 deletions(-) diff --git a/src/agent_comms/backend.py b/src/agent_comms/backend.py index 545f370ddf..24fca030ef 100644 --- a/src/agent_comms/backend.py +++ b/src/agent_comms/backend.py @@ -54,7 +54,7 @@ from .prompt_state import UnwrittenPrompt from .request_progress import NO_REQUEST_OBSERVER, RequestObserver, RequestProgress from .native_tool_call import NativeToolMode -from .native_tool_transport import ToolTransport +from .native_tool_transport import ToolTransport, UnboundToolTransport from .mro_dispatch import MroDispatch, handles from .selected_tool_broker import SelectedToolDenied from .send_outcome import SendBoundary @@ -382,8 +382,8 @@ def context_info(self) -> events.ContextInfo: context_size=self.usage.size, ) - async def open_transport(self, resources: AsyncExitStack) -> None: - self.tool_socket = await ToolTransport.open(self, resources) + async def open_transport(self, resources: AsyncExitStack) -> UnboundToolTransport: + return await ToolTransport.open(self, resources) @contextmanager def native_acquisition(self): @@ -398,9 +398,9 @@ async def acquire_native(self, resources: AsyncExitStack, *, reuse: bool) -> PiS with self.native_acquisition(): async with AsyncExitStack() as transport: with self.startup.measurements.operation("open_transport"): - await self.open_transport(transport) + unbound = await self.open_transport(transport) native = await self.open_native(reuse=reuse) - self.tool_socket.bind_process(native.proc.pid) + self.tool_socket = await unbound.bind_process(native.proc.pid) native.transport = transport.pop_all() self.owner = asyncio.current_task() if self.owner is not None: @@ -587,11 +587,12 @@ def __init__(self, launch, make_command: Callable[..., commands.Prompt], *, self.tool_mode = tool_mode self.make_command = make_command - async def open_transport(self, resources: AsyncExitStack) -> None: - self.tool_socket = await self.tool_mode.open_transport(self, resources) - self.command = self.make_command(owner_tools=self.tool_socket.connection) + async def open_transport(self, resources: AsyncExitStack) -> UnboundToolTransport: + unbound = await self.tool_mode.open_transport(self, resources) + self.command = self.make_command(owner_tools=unbound.listener.connection) del self.make_command self.output.sensitive = bool(self.command.images) + return unbound async def context_proof(self): if not self.started_input or not self.accepts_output: diff --git a/src/agent_comms/channel_coding_tools.py b/src/agent_comms/channel_coding_tools.py index d5794d54d5..2c37800151 100644 --- a/src/agent_comms/channel_coding_tools.py +++ b/src/agent_comms/channel_coding_tools.py @@ -9,32 +9,30 @@ import asyncio from abc import ABC, abstractmethod -from collections.abc import Awaitable, Callable from dataclasses import dataclass, field, replace from pathlib import Path from agent_comms.coordinator import Coordination -from .selected_claim import ( - publish_selected_resource_claim, - verify_selected_wake, -) from .comms import Comms -from .field_codec import FieldCodec from .envelope_claim_transitions import ( ClaimOwner, - FileClaimPath, WakeClaim, ) -from .native_tool_call import CodingCall, NativeToolMode, SelectedToolDenied +from .field_codec import FieldCodec from .native_arguments import NativeArguments -from .storage_room import NoRoomNeeded, RoomReservation +from .native_tool_call import CodingCall, NativeToolMode, SelectedToolDenied +from .native_tool_transport import ToolListener, ToolTransport, UnboundToolTransport from .native_tools import NativeToolDeclaration -from .native_tool_transport import ToolTransport +from .selected_claim import ( + publish_selected_resource_claim, + verify_selected_wake, +) from .selected_tool_broker import ( consume_selected_slot, verify_sent_full_input, ) +from .storage_room import NoRoomNeeded, RoomReservation @dataclass(frozen=True, kw_only=True) @@ -131,14 +129,14 @@ def launch_arguments(self, package: Path) -> tuple[str, ...]: thread = self.owner.comms.registry.require(self.owner.owner_name) return thread.native_launch_arguments(NativeArguments.parse(arguments), package) - async def open_transport(self, turn, ownership) -> ToolTransport: + async def open_transport(self, turn, ownership) -> UnboundToolTransport: return await CodingToolSocket.open(turn, ownership, self.owner) class CodingToolSocket(ToolTransport[CodingCall]): """Multi-call native policy on the same peer-authenticated owner transport.""" - def __init__(self, directory: Path, token: str, owner: CodingToolOwner) -> None: - super().__init__(directory, token) + def __init__(self, listener: ToolListener, owner: CodingToolOwner, *, pid: int) -> None: + super().__init__(listener, pid=pid) self.owner = owner self.completions = set() @@ -152,7 +150,7 @@ def decode_request(self, raw: bytes) -> CodingCall: if ( type(value) is not dict or set(value) != {"token", "call_id", "name", "arguments"} - or value["token"] != self.token + or value["token"] != self.listener.token ): raise SelectedToolDenied("Coding request is not authenticated") return CodingCall.decode(value["call_id"], value["name"], value["arguments"]) @@ -171,12 +169,12 @@ def tool_finished(self, event, input_id): task = asyncio.create_task(call.progress.finish_room()) self.completions.add(task) - async def _close(self): - await super()._close() + async def _drain_calls(self): if self.completions: await asyncio.gather(*tuple(self.completions)) for call in self.calls.values(): await asyncio.to_thread(call.progress.room.abandon) - def failure_response(self, error: Exception) -> dict[str, object]: + @staticmethod + def failure_response(error: Exception) -> dict[str, object]: return {"ok": False, "error": str(error)} diff --git a/src/agent_comms/native_tool_call.py b/src/agent_comms/native_tool_call.py index a4e2b61c90..d5867d908e 100644 --- a/src/agent_comms/native_tool_call.py +++ b/src/agent_comms/native_tool_call.py @@ -18,7 +18,7 @@ if TYPE_CHECKING: - from .native_tool_transport import ToolTransport + from .native_tool_transport import UnboundToolTransport from .envelope_claim_transitions import FileClaimPath from .storage_room import RoomReservation from .registration import Registration @@ -359,7 +359,7 @@ def launch_arguments(self, package: Path) -> tuple[str, ...]: """Return the native tool selection and reviewed extension.""" @abstractmethod - async def open_transport(self, turn, ownership) -> ToolTransport: + async def open_transport(self, turn, ownership) -> UnboundToolTransport: """Open the transport implementing this declared permission.""" def or_else(self, factory: Callable[[], NativeToolMode]) -> NativeToolMode: diff --git a/src/agent_comms/native_tool_transport.py b/src/agent_comms/native_tool_transport.py index 0fef11af59..f35177a961 100644 --- a/src/agent_comms/native_tool_transport.py +++ b/src/agent_comms/native_tool_transport.py @@ -13,16 +13,21 @@ from dataclasses import dataclass from functools import cached_property from pathlib import Path -from typing import Generic, TypeVar, TYPE_CHECKING +from typing import TYPE_CHECKING, Generic, TypeVar from .child_process import join_retirement from .field_codec import MalformedRecord, Token256Text -from .native_tool_call import NativeToolCall, OwnerToolConnection, SelectedToolDenied, DuplicateToolCallIdentity +from .native_tool_call import ( + DuplicateToolCallIdentity, + NativeToolCall, + OwnerToolConnection, + SelectedToolDenied, +) from .pi_payloads import PiContent, ToolCallContent from .private_path import PrivateDirectoryRole, PrivateSocketRole if TYPE_CHECKING: - from .pi_events import ToolExecutionStart, ToolExecutionEnd + from .pi_events import ToolExecutionEnd, ToolExecutionStart _LOG = logging.getLogger(__name__) @@ -182,39 +187,96 @@ def require(self, round_number: int, call_id: str) -> Call: -class ToolTransport(Generic[Call]): - """Authenticated native child transport; the policy decides which requests may run.""" +class ToolListener: + """One private endpoint and joined resource lifetime, independent of child binding.""" - max_request = 1024 * 1024 + def __init__(self, directory: Path, token: str) -> None: + try: + Token256Text.decode(token) + except MalformedRecord as error: + raise ValueError("Selected tool transport requires a random 256-bit token") from error + self.path = Path(directory).absolute() / "s" + self.token = token + self._resources = AsyncExitStack() + if not hasattr(socket, "SO_PEERCRED") or not hasattr(os, "O_NOFOLLOW"): + raise SelectedToolDenied("Selected tool requires a peer-credential Unix socket") + PrivateDirectoryRole.require(self.path.parent.lstat()) + if self.path.parent.resolve() != self.path.parent: + raise SelectedToolDenied("Selected tool socket parent is not private") + if self.path.exists() or self.path.is_symlink(): + raise SelectedToolDenied("Selected tool socket already exists") + with ExitStack() as prepared: + self.address = prepared.enter_context(PrivateSocketRole.address(self.path)) + self.socket = prepared.enter_context(socket.socket(socket.AF_UNIX)) + self.socket.bind(str(self.address)) + prepared.callback(self.path.unlink, missing_ok=True) + os.chmod(self.path, 0o600) + self._resources.enter_context(prepared.pop_all()) @property def connection(self) -> OwnerToolConnection: return OwnerToolConnection(str(self.address), self.token) + async def listen(self, transport: ToolTransport) -> None: + try: + self.server = await asyncio.start_unix_server( + transport._handle, sock=self.socket, limit=transport.max_request + 1, + start_serving=False, cleanup_socket=False, + ) + self._resources.push_async_callback(transport._drain_calls) + self._resources.push_async_callback(self.server.wait_closed) + self._resources.push_async_callback(transport._drain_handlers) + self._resources.callback(self.server.close) + await self.server.start_serving() + except BaseException: + await self.close() + raise + + @cached_property + def _retirement(self) -> asyncio.Task[None]: + return asyncio.create_task(self._resources.aclose()) + + async def close(self) -> None: + await join_retirement(self._retirement) + + +@dataclass(frozen=True) +class UnboundToolTransport(Generic[Call]): + """A prepared endpoint cannot accept requests until its actual child exists.""" + + listener: ToolListener + bind: Callable[[int], ToolTransport[Call]] + + async def bind_process(self, pid: int) -> ToolTransport[Call]: + transport = self.bind(pid) + await transport.start() + return transport + + +class ToolTransport(Generic[Call]): + """A child-bound request policy. An instance always has its authenticated PID.""" + + max_request = 1024 * 1024 + @property def retainable(self) -> bool: return True @classmethod - async def open(cls, turn, ownership, *policy): - transport = cls(turn.launch.session.directory, os.urandom(32).hex(), *policy) - transport.turn = turn - ownership.push_async_callback(transport.close) - await transport.start() - return transport + async def open(cls, turn, ownership, *policy) -> UnboundToolTransport: + listener = ToolListener(turn.launch.session.directory, os.urandom(32).hex()) + ownership.push_async_callback(listener.close) - def bind_process(self, pid: int) -> None: - self.expected_pid = pid + def bind(pid: int): + transport = cls(listener, *policy, pid=pid) + transport.turn = turn + return transport - def __init__(self, directory: Path, token: str) -> None: - try: - Token256Text.decode(token) - except MalformedRecord as error: - raise ValueError("Selected tool transport requires a random 256-bit token") from error - self.path = Path(directory).absolute() / "s" - self._resources = AsyncExitStack() - self.token = token - self.expected_pid: int | None = None + return UnboundToolTransport(listener, bind) + + def __init__(self, listener: ToolListener, *, pid: int) -> None: + self.listener = listener + self.expected_pid = pid self.calls: NativeToolCallTable[Call] = NativeToolCallTable() self._handlers: set[asyncio.Task] = set() @@ -248,7 +310,7 @@ def tool_finished(self, event: ToolExecutionEnd, input_id: str) -> None: event.tool_call_id, "Native tool terminal has no declared call" ) call.finish( - event.tool_name, event.is_error, self.path.parent, input_id, + event.tool_name, event.is_error, self.listener.path.parent, input_id, identity.round_number, ) @@ -267,44 +329,18 @@ async def handle_request(self, raw: bytes) -> dict[str, object]: await call.authorize(lambda: self.allow(self.calls.identity_for(call).round_number, call)) return {"ok": True} - def failure_response(self, error: Exception) -> dict[str, object]: + @staticmethod + def failure_response(error: Exception) -> dict[str, object]: return {"ok": False, "error": "Selected tool denied or outcome UNKNOWN; no retry"} async def start(self) -> None: - if not hasattr(socket, "SO_PEERCRED") or not hasattr(os, "O_NOFOLLOW"): - raise SelectedToolDenied("Selected tool requires a peer-credential Unix socket") - info = self.path.parent.lstat() - if PrivateDirectoryRole.violation(info) is not None or self.path.parent.resolve() != self.path.parent: - raise SelectedToolDenied("Selected tool socket parent is not private") - if self.path.exists() or self.path.is_symlink(): - raise SelectedToolDenied("Selected tool socket already exists") - try: - self.address = self._resources.enter_context(PrivateSocketRole.address(self.path)) - listener = self._resources.enter_context(socket.socket(socket.AF_UNIX)) - listener.bind(str(self.address)) - self._resources.callback(self.path.unlink, missing_ok=True) - os.chmod(self.path, 0o600) - self._server = await asyncio.start_unix_server( - self._handle, sock=listener, limit=self.max_request + 1, - start_serving=False, cleanup_socket=False, - ) - self._resources.push_async_callback(self._server.wait_closed) - self._resources.push_async_callback(self._drain_handlers) - self._resources.callback(self._server.close) - await self._server.start_serving() - except BaseException: - await self.close() - raise - - @cached_property - def _retirement(self) -> asyncio.Task[None]: - return asyncio.create_task(self._close()) + await self.listener.listen(self) async def close(self) -> None: - await join_retirement(self._retirement) + await self.listener.close() - async def _close(self) -> None: - await self._resources.aclose() + async def _drain_calls(self) -> None: + """Retire policy-owned call resources after handler and server retirement.""" async def _drain_handlers(self) -> None: handlers = tuple(self._handlers) @@ -328,11 +364,9 @@ async def _handle(self, reader: asyncio.StreamReader, writer: asyncio.StreamWrit async def _reply(self, reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> ToolConnectionOutcome: try: - if not self._server.is_serving(): + if not self.listener.server.is_serving(): raise SelectedToolDenied("Owner tool socket is closed") sock = writer.get_extra_info("socket") - if sock is None or self.expected_pid is None: - raise SelectedToolDenied("Selected tool child is not bound") pid, uid, _gid = struct.unpack( "3i", sock.getsockopt(socket.SOL_SOCKET, socket.SO_PEERCRED, 12) ) @@ -344,7 +378,7 @@ async def _reply(self, reader: asyncio.StreamReader, return ToolChildGone("unanswered") response = await self.handle_request(raw) answer = "allowed" - except (ValueError, TimeoutError) as error: + except (SelectedToolDenied, MalformedRecord) as error: # Only declared denials and malformed child input become a refusal. # Owner defects propagate; they never become a successful answer. response = self.failure_response(error) diff --git a/src/agent_comms/selected_tool_broker.py b/src/agent_comms/selected_tool_broker.py index da51391a1c..19eb7b534d 100644 --- a/src/agent_comms/selected_tool_broker.py +++ b/src/agent_comms/selected_tool_broker.py @@ -8,8 +8,6 @@ from __future__ import annotations -from typing import ClassVar - import logging import os import re @@ -18,10 +16,10 @@ from dataclasses import dataclass, field from functools import partial from pathlib import Path +from typing import ClassVar from agent_comms.coordinator import Coordination -from .selected_claim import publish_selected_resource_claim, write_selected_claimed_file from .comms import Comms from .coordinated_runtime_schema import assert_native_runtime_schema from .coordination_errors import CoordinationReadUnavailable @@ -36,9 +34,15 @@ NativeToolMode, SelectedToolDenied, ) +from .native_tool_transport import ( + NativeToolRound, + ToolListener, + ToolTransport, + UnboundToolTransport, +) from .private_path import PrivateDirectoryRole, PrivateFileRole, fsync_directory -from .native_tool_transport import ToolTransport, NativeToolRound from .selected_actions import SelectedAction +from .selected_claim import publish_selected_resource_claim, write_selected_claimed_file from .turn_context import InstructionFile # Stay well below the existing native RPC record cap (1 MiB, including JSON). @@ -92,7 +96,7 @@ def launch_arguments(self, package: Path) -> tuple[str, ...]: str(selected_extension(package)), ) - async def open_transport(self, turn, ownership) -> ToolTransport: + async def open_transport(self, turn, ownership) -> UnboundToolTransport: return await SelectedToolSocket.open(turn, ownership, self.action) @@ -339,9 +343,9 @@ class SelectedToolSocket(ToolTransport[SelectedToolRequest]): """One selected replacement, preserving the existing one-slot authority.""" def __init__( - self, directory: Path, token: str, action: Callable[[SelectedToolRequest], None] + self, listener: ToolListener, action: Callable[[SelectedToolRequest], None], *, pid: int ) -> None: - super().__init__(directory, token) + super().__init__(listener, pid=pid) self.action = action def validate_round(self, round: NativeToolRound[SelectedToolRequest]) -> None: @@ -368,7 +372,7 @@ def decode_request(self, raw: bytes) -> SelectedToolRequest: ) except MalformedRecord as error: raise SelectedToolDenied("Invalid selected tool request") from error - return envelope.authenticate(self.token) + return envelope.authenticate(self.listener.token) async def allow(self, round_number: int, call: SelectedToolRequest) -> None: await Coordination.run_worker(partial(self.action, call)) diff --git a/src/agent_comms/turn_inputs.py b/src/agent_comms/turn_inputs.py index ab1eeff8a7..f74e744398 100644 --- a/src/agent_comms/turn_inputs.py +++ b/src/agent_comms/turn_inputs.py @@ -144,7 +144,7 @@ async def place(self, session: TurnSession, item: InputRequest) -> bool: message=text, images=item.content.images or None, context_contributions=item.content.contributions, - owner_tools=session.tool_socket.connection, + owner_tools=session.tool_socket.listener.connection, streaming_behavior=_PI_STREAMING_BEHAVIOR[item.when_busy], ) def write(): diff --git a/tests/conftest.py b/tests/conftest.py index ddd086df86..395c560951 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,7 @@ from agent_comms.selected_session import SelectedSession import os import tempfile +from contextlib import AsyncExitStack from pathlib import Path import pytest @@ -12,6 +13,7 @@ from agent_comms.bus_publication import AgentOrigin from agent_comms.comms import Comms from agent_comms.message_bus import MessageBus +from agent_comms.native_tool_transport import ToolListener from agent_comms.publisher import Publisher, ResponseWrites from agent_comms.registration import Registration from agent_comms.messages import ConversationRole, InfoKind, Message @@ -60,6 +62,17 @@ def pytest_addoption(parser): help="Root of the native test environments stack/bin/pi-native-test-env builds") +@pytest.fixture +async def tool_listener(): + """Every private test endpoint has one joined retirement owner.""" + async with AsyncExitStack() as resources: + def create(directory, token): + listener = ToolListener(directory, token) + resources.push_async_callback(listener.close) + return listener + yield create + + @pytest.fixture def source_reads(): from agent_comms.native_entries import NativeEvidenceScope diff --git a/tests/test_channel_coding_tools.py b/tests/test_channel_coding_tools.py index 1df100d9c2..2c5e0bbb05 100644 --- a/tests/test_channel_coding_tools.py +++ b/tests/test_channel_coding_tools.py @@ -1,6 +1,7 @@ """Normal coding policies use existing claims; no provider or user work is touched.""" import json +import os import secrets from pathlib import Path from tempfile import TemporaryDirectory @@ -116,7 +117,7 @@ def test_family_selector_is_denied_but_record_presentation_stays_readable(): @pytest.mark.asyncio -async def test_malformed_edit_reports_error_and_allows_a_later_tool_round(tmp_path): +async def test_malformed_edit_reports_error_and_allows_a_later_tool_round(tmp_path, tool_listener): from agent_comms.native_tool_call import SelectedToolDenied from agent_comms.selected_tool_broker import consume_selected_slot @@ -131,7 +132,7 @@ async def allow(self, turn, round_number, call): return NoRoomNeeded() owner = Owner() - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), owner) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), owner, pid=os.getpid()) socket.turn = object() malformed = {"path": "x", "edits": [{"oldText": "old", "newText": "new", "newText_should_not_exist": "extra"}]} for id, name, args in (("bad-edit", "edit", malformed), ("later-read", "read", {"path": "x"})): @@ -166,7 +167,7 @@ def test_pi_argument_fields_own_roundtrip_and_display(name, arguments, title): @pytest.mark.asyncio -async def test_multicall_native_transport_matches_events_and_refuses_duplicate(tmp_path): +async def test_multicall_native_transport_matches_events_and_refuses_duplicate(tmp_path, tool_listener): class Owner: input_id = secrets.token_hex(16) calls = [] @@ -177,7 +178,7 @@ async def allow(self, turn, round_number, call): owner = Owner() tmp_path.chmod(0o700) - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), owner) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), owner, pid=os.getpid()) socket.turn = object() # Actual owner claim_scope/storage is covered separately. This exercises the # strict native-event/socket correlation and multiple normal tool calls. @@ -189,7 +190,7 @@ async def allow(self, turn, round_number, call): for id, name, args in calls: socket.tool_started(ToolExecutionStart(tool_call_id=id, tool_name=name, args=args)) raw = ( - json.dumps({"token": socket.token, "call_id": id, "name": name, "arguments": args}) + json.dumps({"token": socket.listener.token, "call_id": id, "name": name, "arguments": args}) + "\n" ).encode() assert await socket.handle_request(raw) == {"ok": True} @@ -210,12 +211,12 @@ async def allow(self, turn, round_number, call): def _request(socket, id, name, args): return ( - json.dumps({"token": socket.token, "call_id": id, "name": name, "arguments": args}) + "\n" + json.dumps({"token": socket.listener.token, "call_id": id, "name": name, "arguments": args}) + "\n" ).encode() @pytest.mark.asyncio -async def test_tool_error_while_check_waits_cannot_receive_late_permission(tmp_path): +async def test_tool_error_while_check_waits_cannot_receive_late_permission(tmp_path, tool_listener): import asyncio checking, release = asyncio.Event(), asyncio.Event() @@ -229,7 +230,7 @@ async def allow(self, turn, round_number, call): return NoRoomNeeded() owner = Owner() - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), owner) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), owner, pid=os.getpid()) socket.turn = object() socket.announce((ToolCallContent(id="late", name="read", arguments={"path": "a"}),)) socket.tool_started(ToolExecutionStart(tool_call_id="late", tool_name="read", args={"path": "a"})) @@ -244,7 +245,7 @@ async def allow(self, turn, round_number, call): @pytest.mark.asyncio @pytest.mark.parametrize("refused", [False, True]) -async def test_child_that_stopped_waiting_is_logged_not_raised(tmp_path, caplog, refused): +async def test_child_that_stopped_waiting_is_logged_not_raised(tmp_path, caplog, refused, tool_listener): """The live owner logged two tracebacks when Pi gave up on a slow tool check.""" import asyncio import os @@ -263,14 +264,13 @@ async def allow(self, turn, round_number, call): return NoRoomNeeded() tmp_path.chmod(0o700) - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), Owner()) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), Owner(), pid=os.getpid()) socket.turn = object() - socket.expected_pid = os.getpid() await socket.start() try: call = ("slow", "read", {"path": "a"}) socket.announce([ToolCallContent(id=call[0], name=call[1], arguments=call[2])]) - reader, writer = await asyncio.open_unix_connection(str(socket.address)) + reader, writer = await asyncio.open_unix_connection(str(socket.listener.address)) writer.write(_request(socket, *call)) await writer.drain() # Pi's side: stop waiting and reset the connection before the owner @@ -292,7 +292,7 @@ async def allow(self, turn, round_number, call): @pytest.mark.asyncio -async def test_tool_call_pi_never_started_is_cancelled_by_transport_retirement(tmp_path): +async def test_tool_call_pi_never_started_is_cancelled_by_transport_retirement(tmp_path, tool_listener): import asyncio import os @@ -303,13 +303,12 @@ async def allow(self, turn, round_number, call): raise AssertionError("an unstarted call was checked") tmp_path.chmod(0o700) - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), Owner()) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), Owner(), pid=os.getpid()) socket.turn = object() call = ("never", "read", {"path": "a"}) socket.announce([ToolCallContent(id=call[0], name=call[1], arguments=call[2])]) - socket.bind_process(os.getpid()) await socket.start() - reader, writer = await asyncio.open_unix_connection(str(socket.address)) + reader, writer = await asyncio.open_unix_connection(str(socket.listener.address)) try: writer.write(_request(socket, *call)) await writer.drain() diff --git a/tests/test_native_argument_refusal.py b/tests/test_native_argument_refusal.py index 8b49ccb056..0118ad7e0c 100644 --- a/tests/test_native_argument_refusal.py +++ b/tests/test_native_argument_refusal.py @@ -2,6 +2,7 @@ import asyncio import json +import os import pytest @@ -25,14 +26,14 @@ def test_pi_optional_nulls_are_omitted_by_the_existing_codec(name, arguments, en @pytest.mark.parametrize("request_first", [False, True]) -async def test_refused_arguments_complete_without_an_owner_callback(tmp_path, request_first): +async def test_refused_arguments_complete_without_an_owner_callback(tmp_path, request_first, tool_listener): class Owner: async def allow(self, round_number, call): raise AssertionError("invalid arguments must never acquire authority") - socket = CodingToolSocket(tmp_path, "a" * 64, Owner()) + socket = CodingToolSocket(tool_listener(tmp_path, "a" * 64), Owner(), pid=os.getpid()) args = {"command": False} - raw = (json.dumps({"token": socket.token, "call_id": "bad", "name": "bash", + raw = (json.dumps({"token": socket.listener.token, "call_id": "bad", "name": "bash", "arguments": {"command": "false"}}) + "\n").encode() pending = asyncio.create_task(socket.handle_request(raw)) if request_first else None if request_first: diff --git a/tests/test_native_tool_lifecycle.py b/tests/test_native_tool_lifecycle.py index bb830fb2a0..65df361acd 100644 --- a/tests/test_native_tool_lifecycle.py +++ b/tests/test_native_tool_lifecycle.py @@ -4,6 +4,8 @@ import json import os import secrets +import socket as sockets +from contextlib import AsyncExitStack from types import SimpleNamespace import pytest @@ -12,12 +14,13 @@ from agent_comms.channel_coding_tools import CodingToolSocket from agent_comms.storage_room import NoRoomNeeded from agent_comms.native_tool_call import SelectedToolDenied +from agent_comms.native_tool_transport import ToolTransport, UnboundToolTransport from agent_comms.pi_events import ToolExecutionEnd, ToolExecutionStart from agent_comms.pi_payloads import ToolCallContent @pytest.fixture(params=["coding", "selected"]) -def policy(request, tmp_path): +def policy(request, tmp_path, tool_listener): tmp_path.chmod(0o700) token, input_id = secrets.token_hex(32), secrets.token_hex(16) effects = [] @@ -38,11 +41,11 @@ async def allow_coding(turn, round_number, call): return NoRoomNeeded() socket = ( - broker.SelectedToolSocket(tmp_path, token, lambda call: commit(call, 0)) + broker.SelectedToolSocket(tool_listener(tmp_path, token), lambda call: commit(call, 0), pid=os.getpid()) if selected else CodingToolSocket( - tmp_path, token, - SimpleNamespace(allow=allow_coding), + tool_listener(tmp_path, token), + SimpleNamespace(allow=allow_coding), pid=os.getpid(), ) ) @@ -323,8 +326,7 @@ async def test_lost_terminal_receipt_cannot_be_replaced_by_error(policy, monkeyp async def test_socket_close_cancels_pending_claim_and_closes_client(policy): p = policy await p.socket.start() - p.socket.expected_pid = os.getpid() - reader, writer = await asyncio.open_unix_connection(str(p.socket.path)) + reader, writer = await asyncio.open_unix_connection(str(p.socket.listener.path)) writer.write(p.raw()) await writer.drain() async with asyncio.timeout(2): @@ -334,7 +336,7 @@ async def test_socket_close_cancels_pending_claim_and_closes_client(policy): assert await reader.read() == b"" writer.close() await writer.wait_closed() - assert not p.socket.path.exists() and not p.socket._handlers + assert not p.socket.listener.path.exists() and not p.socket._handlers with pytest.raises(SelectedToolDenied): p.socket.assert_complete() assert not p.effects @@ -358,10 +360,9 @@ async def observe_reply(reader, writer): monkeypatch.setattr(p.socket, "_reply", observe_reply) p.announce() p.start() - p.socket.expected_pid = os.getpid() await p.socket.start() try: - reader, writer = await asyncio.open_unix_connection(str(p.socket.address)) + reader, writer = await asyncio.open_unix_connection(str(p.socket.listener.address)) try: writer.write(p.raw()) await writer.drain() @@ -382,7 +383,7 @@ async def observe_reply(reader, writer): assert closed.count(observed[0]) == 1 -async def test_interleaved_calls_wait_on_their_own_native_start(tmp_path): +async def test_interleaved_calls_wait_on_their_own_native_start(tmp_path, tool_listener): tmp_path.chmod(0o700) observed = [] @@ -391,8 +392,8 @@ async def allow_coding(turn, round_number, call): return NoRoomNeeded() socket = CodingToolSocket( - tmp_path, secrets.token_hex(32), - SimpleNamespace(allow=allow_coding), + tool_listener(tmp_path, secrets.token_hex(32)), + SimpleNamespace(allow=allow_coding), pid=os.getpid(), ) socket.turn = SimpleNamespace(command=SimpleNamespace(input_id=secrets.token_hex(16))) arguments = {"path": "x"} @@ -403,7 +404,7 @@ async def allow_coding(turn, round_number, call): def raw(id): return ( json.dumps( - {"token": socket.token, "call_id": id, "name": "read", "arguments": arguments} + {"token": socket.listener.token, "call_id": id, "name": "read", "arguments": arguments} ) + "\n" ).encode() @@ -429,3 +430,70 @@ def test_selected_slot_rejects_second_call(policy): policy.announce() with pytest.raises(SelectedToolDenied): policy.announce("second") + + +@pytest.mark.parametrize("bind_child", [False, True]) +async def test_only_child_bound_transport_can_reply(tmp_path, bind_child): + turn = SimpleNamespace(launch=SimpleNamespace(session=SimpleNamespace(directory=tmp_path))) + async with AsyncExitStack() as resources: + unbound = await ToolTransport.open(turn, resources) + assert isinstance(unbound, UnboundToolTransport) + assert not hasattr(unbound, "_reply") and not hasattr(unbound, "_handle") + with pytest.raises(TypeError, match="pid"): + ToolTransport(unbound.listener) + with pytest.raises(ConnectionRefusedError): + await asyncio.open_unix_connection(str(unbound.listener.address)) + if bind_child: + bound = await unbound.bind_process(os.getpid()) + assert isinstance(bound, ToolTransport) + assert bound.expected_pid == os.getpid() + assert bound.listener.server.is_serving() + assert not (tmp_path / "s").exists() + + +@pytest.mark.parametrize("failure", [ValueError("owner defect"), TimeoutError("owner deadline")]) +async def test_owner_failures_are_not_converted_to_refusals(policy, monkeypatch, failure): + p = policy + + def fail_selected(_call): + raise failure + + async def fail_coding(_turn, _round, _call): + raise failure + + if p.selected: + monkeypatch.setattr(p.socket, "action", fail_selected) + else: + monkeypatch.setattr(p.socket.owner, "allow", fail_coding) + p.announce() + p.start() + await p.socket.start() + reader = asyncio.StreamReader() + reader.feed_data(p.raw()) + reader.feed_eof() + + def unexpected_reply(_raw): + pytest.fail("an owner defect became a tool reply") + + peer, client = sockets.socketpair() + with peer, client: + writer = SimpleNamespace(get_extra_info=lambda _key: peer, write=unexpected_reply) + with pytest.raises(type(failure), match=str(failure)) as caught: + await p.socket._reply(reader, writer) + assert caught.value is failure + assert not p.effects + + +async def test_malformed_child_json_receives_a_declared_refusal(policy): + p = policy + await p.socket.start() + reader, writer = await asyncio.open_unix_connection(str(p.socket.listener.address)) + try: + writer.write(b"{\n") + await writer.drain() + assert json.loads(await reader.readline())["ok"] is False + assert await reader.read() == b"" + assert not p.effects + finally: + writer.close() + await writer.wait_closed() diff --git a/tests/test_selected_tool_broker.py b/tests/test_selected_tool_broker.py index 0c08d53222..e69dfd1fdf 100644 --- a/tests/test_selected_tool_broker.py +++ b/tests/test_selected_tool_broker.py @@ -9,6 +9,8 @@ import secrets import sys from pathlib import Path +from contextlib import AsyncExitStack +from types import SimpleNamespace import pytest @@ -34,7 +36,7 @@ def _wire(token: str, **changes: object) -> bytes: @pytest.mark.asyncio -async def test_provider_call_id_is_scoped_to_its_tool_round(tmp_path: Path) -> None: +async def test_provider_call_id_is_scoped_to_its_tool_round(tmp_path: Path, tool_listener) -> None: class Owner: input_id = secrets.token_hex(16) @@ -44,7 +46,7 @@ async def allow(self, turn, round_number, call) -> RoomReservation: tmp_path.chmod(0o700) owner = Owner() - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), owner) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), owner, pid=os.getpid()) socket.turn = owner first_arguments = {"path": "one"} socket.announce((ToolCallContent(id="reused", name="read", arguments=first_arguments),)) @@ -52,7 +54,7 @@ async def allow(self, turn, round_number, call) -> RoomReservation: ToolExecutionStart(tool_call_id="reused", tool_name="read", args=first_arguments) ) request = json.dumps({ - "token": socket.token, + "token": socket.listener.token, "call_id": "reused", "name": "read", "arguments": first_arguments, @@ -68,7 +70,7 @@ async def allow(self, turn, round_number, call) -> RoomReservation: second_arguments = {"path": "two"} second_request = json.dumps({ - "token": socket.token, + "token": socket.listener.token, "call_id": "reused", "name": "read", "arguments": second_arguments, @@ -93,14 +95,14 @@ async def allow(self, turn, round_number, call) -> RoomReservation: assert first.slot(owner.input_id, 0) != second.slot(owner.input_id, 1) -def test_duplicate_call_id_in_one_tool_round_is_named_decode_failure(tmp_path: Path) -> None: +def test_duplicate_call_id_in_one_tool_round_is_named_decode_failure(tmp_path: Path, tool_listener) -> None: class Owner: input_id = secrets.token_hex(16) async def allow(self, round_number, call) -> None: raise AssertionError("invalid round must not allow a call") - socket = CodingToolSocket(tmp_path, secrets.token_hex(32), Owner()) + socket = CodingToolSocket(tool_listener(tmp_path, secrets.token_hex(32)), Owner(), pid=os.getpid()) repeated = ToolCallContent(id="duplicate", name="read", arguments={"path": "one"}) with pytest.raises(DuplicateToolCallIdentity, match="repeats call identity 'duplicate'"): socket.announce((repeated, repeated)) @@ -326,18 +328,23 @@ def write(*_args: object) -> None: @pytest.mark.skipif(sys.platform != "linux", reason="selected tool socket requires SO_PEERCRED") -def test_fake_socket_requires_pid_token_matching_emitted_call(tmp_path: Path) -> None: +def test_fake_socket_requires_pid_token_matching_emitted_call(tmp_path: Path, tool_listener) -> None: tmp_path.chmod(0o700) token = secrets.token_hex(32) observed: list[broker.SelectedToolRequest] = [] async def run() -> None: - server = broker.SelectedToolSocket(tmp_path, token, observed.append) + wrong = broker.SelectedToolSocket( + tool_listener(tmp_path, token), observed.append, pid=os.getpid() + 100000) + await wrong.start() + try: + assert not await query(wrong.listener.path, _wire(token)) + finally: + await wrong.close() + server = broker.SelectedToolSocket( + tool_listener(tmp_path, token), observed.append, pid=os.getpid()) await server.start() try: - server.expected_pid = os.getpid() + 100000 - assert not await query(server.path, _wire(token)) - server.expected_pid = os.getpid() for changes in ( {"resource": "../escape"}, {"resource": "/absolute"}, @@ -348,10 +355,10 @@ async def run() -> None: {"contents": "\ud800"}, {"root": "forged"}, ): - assert not await query(server.path, _wire(token, **changes)) + assert not await query(server.listener.path, _wire(token, **changes)) assert not observed and not server.calls arguments = {"resource": "notes.txt", "contents": "é"} - pending = asyncio.create_task(query(server.path, _wire(token))) + pending = asyncio.create_task(query(server.listener.path, _wire(token))) await asyncio.sleep(0) assert not pending.done() # Socket arrival is not a native start. server.announce( @@ -371,8 +378,8 @@ async def run() -> None: ) ) assert await pending - assert not await query(server.path, _wire("0" * 64)) - assert not await query(server.path, _wire(token)) # Consumed exactly once. + assert not await query(server.listener.path, _wire("0" * 64)) + assert not await query(server.listener.path, _wire(token)) # Consumed exactly once. assert observed == [ broker.SelectedToolRequest( "call_1", broker.SelectedWriteArguments("notes.txt", "é") @@ -380,7 +387,7 @@ async def run() -> None: ] finally: await server.close() - assert not server.path.exists() + assert not server.listener.path.exists() async def query(path: Path, payload: bytes) -> bool: reader, writer = await asyncio.open_unix_connection(str(path)) @@ -411,17 +418,11 @@ def commit(request): target.write_text(request.arguments.contents) broker.record_selected_terminal(tmp_path, input_id, request.call_id) - socket = broker.SelectedToolSocket(tmp_path, token, commit) + resources = AsyncExitStack() + unbound = await broker.SelectedToolSocket.open( + SimpleNamespace(launch=SimpleNamespace(session=SimpleNamespace(directory=tmp_path))), + resources, commit) arguments = {"resource": "notes.txt", "contents": "paired producer é"} - socket.announce( - (ToolCallContent(id="paired", name="selected_claimed_write", arguments=arguments),) - ) - socket.tool_started( - ToolExecutionStart( - tool_call_id="paired", tool_name="selected_claimed_write", args=arguments - ) - ) - await socket.start() process = None try: extension = Path(broker.__file__).with_name("selected_claimed_write.mjs") @@ -433,14 +434,20 @@ def commit(request): import {{OwnerToolConnection}} from {json.dumps(extension.with_name('owner_tool_socket.mjs').as_uri())}; let tool; register({{registerTool(value) {{tool = value}}}}); +await new Promise(resolve => process.stdin.once('data', resolve)); await tool.execute('paired', {json.dumps(arguments)}, new AbortController().signal, undefined, - {{ownerTools: OwnerToolConnection.capture({json.dumps({'path': str(socket.address), 'token': token})})}}); + {{ownerTools: OwnerToolConnection.capture({json.dumps({'path': str(unbound.listener.address), 'token': token})})}}); """, + stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) - socket.expected_pid = process.pid - stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=5) + socket = await unbound.bind_process(process.pid) + socket.announce((ToolCallContent( + id="paired", name="selected_claimed_write", arguments=arguments),)) + socket.tool_started(ToolExecutionStart( + tool_call_id="paired", tool_name="selected_claimed_write", args=arguments)) + stdout, stderr = await asyncio.wait_for(process.communicate(input=b"go"), timeout=5) assert process.returncode == 0, (stdout, stderr) socket.tool_finished( ToolExecutionEnd( @@ -454,4 +461,4 @@ def commit(request): if process is not None and process.returncode is None: process.kill() await process.wait() - await socket.close() + await resources.aclose() diff --git a/tests/test_tool_refusal_reasons.py b/tests/test_tool_refusal_reasons.py index 44ae3b8051..59caf5b09a 100644 --- a/tests/test_tool_refusal_reasons.py +++ b/tests/test_tool_refusal_reasons.py @@ -16,9 +16,7 @@ def reply(tmp_path: Path, error: Exception) -> str: """What the agent reads as the tool result when the owner refuses a call.""" - directory = tmp_path / "socket" - directory.mkdir(mode=0o700, exist_ok=True) - return CodingToolSocket(directory, secrets.token_hex(32), None).failure_response(error)["error"] + return CodingToolSocket.failure_response(error)["error"] def test_edit_of_a_missing_file_inside_the_project_says_it_does_not_exist(tmp_path): From 52bd8ef89f597556e9750192f0867270ce5547b6 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:13:16 -0400 Subject: [PATCH 15/25] P252: parallelize the captured owner cutover and prepare pinned submodules --- tools/upgrade_bus.py | 65 +++++++++++--------------------------------- 1 file changed, 16 insertions(+), 49 deletions(-) diff --git a/tools/upgrade_bus.py b/tools/upgrade_bus.py index 541c03ffab..e4e88f76c6 100644 --- a/tools/upgrade_bus.py +++ b/tools/upgrade_bus.py @@ -36,6 +36,7 @@ import tempfile import time from abc import ABC, abstractmethod +from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager, suppress from dataclasses import dataclass, replace from pathlib import Path @@ -48,7 +49,6 @@ from cutover.backup import Backup, UpgradeRefused, bus_files, transcripts_held import agent_comms.activity_checkpoint # noqa: F401 (declares the activity runtime files) -from agent_comms.active_route import acquire_exclusive_route_lock, route_lock_holders from agent_comms.external_transcript import ExternalTranscript from agent_comms.field_codec import FieldCodec from agent_comms.history_result import EverythingHistoryPageResult @@ -145,13 +145,10 @@ def cli(self, root: Path, *arguments: str) -> dict: f"{completed.stdout.strip() or completed.stderr.strip()}") return result - def require_idle(self, root: Path, names: list[str]) -> None: - """The installed lifecycle reads its own turn records before any stop.""" - run(self.python, "-c", - "import sys; from pathlib import Path; from agent_comms.comms import wire; " - "comms=wire(Path(sys.argv[1])); snapshot=comms.registry.snapshot(); " - "[comms.owners.require_restart_idle(snapshot.require(name)) for name in sys.argv[2:]]", - root, *names, cwd=self.checkout, env=self.environment()) + def owners(self, root: Path, command: str, names: list[str]) -> None: + """Admit the captured owner set together and join every request before proceeding.""" + with ThreadPoolExecutor(max_workers=max(1, len(names))) as pool: + list(pool.map(lambda name: self.cli(root, command, "--name", name), names)) def verify(self, root: Path) -> dict[str, int]: threads = self.cli(root, "threads")["threads"] @@ -171,12 +168,18 @@ class Revisions: def __post_init__(self) -> None: self.trees: dict[str, Path] = {} + @staticmethod + def prepare(path: Path) -> None: + run("git", "-C", path, "submodule", "update", "--init", "--recursive", "--checkout") + def tree(self, revision: str) -> Path: if revision not in self.trees: path = self.scratch / f"tree-{revision[:12]}" run("git", "-C", self.repo, "worktree", "add", "--detach", "-q", path, revision) self.trees[revision] = path - return self.trees[revision] + path = self.trees[revision] + self.prepare(path) + return path def close(self) -> None: for path in self.trees.values(): @@ -275,31 +278,6 @@ def carry_and_read(root: Path, before: dict[str, str], pending: tuple[Step, ...] return {"steps": steps, "files": carried(root, before, steps), "read": reader.verify(root)} -def route_readers(patience: int = 120) -> None: - """Bus watches close on their own once maintenance is declared. Toad's observation process - (ui_model/observation.py:1223) and an ACP agent (acp.py:512) hold the route for their whole - life and never pause, so the wait gives up after ``patience`` seconds and names them.""" - directory = Path.home() / ".local/state/agent-comms" - if not directory.exists(): - return - descriptor = os.open(directory, os.O_RDONLY | os.O_DIRECTORY) - - def gave_up(signum: int, frame: object) -> None: - raise TimeoutError - - previous = signal.signal(signal.SIGALRM, gave_up) - signal.alarm(patience) - try: - acquire_exclusive_route_lock(descriptor) - except TimeoutError: - holders = "; ".join(holder.describe() for holder in route_lock_holders(descriptor)) - raise UpgradeRefused(f"Clients still hold the route after {patience}s: {holders}") from None - finally: - signal.alarm(0) - signal.signal(signal.SIGALRM, previous) - os.close(descriptor) - - def upgrade(checkout: Path, root: Path, target: str, installer: str, rehearse_only: bool) -> dict: current = run("git", "-C", checkout, "rev-parse", "HEAD").strip() run("git", "-C", checkout, "fetch", "-q", "origin") @@ -392,28 +370,22 @@ def live(checkout: Path, root: Path, current: str, target: str, pending: tuple[S before_stop(root, pending, staged, revisions) maintenance = MaintenanceBarrier(RegistryStore.at(root).path) held = maintenance.begin(MaintenanceOperator("upgrade-bus")) - try: - installed.require_idle(root, running) - except BaseException: - maintenance.clear(held) - raise lock = installed.lock try: - for name in running: - installed.cli(root, "stop", "--name", name) - route_readers() + installed.owners(root, "stop", running) with transcripts_held(root): backup = Backup.take(root, ROUTE, (root,), root.parent / f"{root.name}.upgrade-{time.strftime('%Y%m%dT%H%M%S')}") except BaseException as error: maintenance.clear(held) - start(installed, root, running) + installed.owners(root, "start", running) print(f"upgrade-bus: aborted before the backup ({type(error).__name__}: {error}); " f"maintenance cleared and {running} restarted", file=sys.stderr) raise shutil.copy2(lock, backup.path / "uv.lock") if lock.exists() else None try: run("git", "-C", checkout, "merge", "--ff-only", "-q", target) + Revisions.prepare(checkout) installed.relock() installed.prepare_native() passed = carry_and_read(root, backup.files(), pending, staged, installed, revisions, []) @@ -427,16 +399,11 @@ def live(checkout: Path, root: Path, current: str, target: str, pending: tuple[S f"maintenance held, checkout and bus preserved; backup: {backup.path}", file=sys.stderr) raise - start(installed, root, running) + installed.owners(root, "start", running) return {"stopped_and_started": running, **passed, "services": services, "backup": str(backup.path)} -def start(stack: Stack, root: Path, names: list[str]) -> None: - for name in names: - stack.cli(root, "start", "--name", name) - - def stopped(signum: int, frame: object) -> None: """A stop signal runs the same abort as a failure; a second one cannot cut that abort short.""" for each in (signal.SIGTERM, signal.SIGINT): From f3257d188e42c24876fb30aa9844e6532b16da77 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:13:38 -0400 Subject: [PATCH 16/25] Refuse maintenance before connector store access Src-Budget: 8 --- src/agent_comms/comms.py | 4 ++++ src/agent_comms/connector.py | 5 +++-- src/agent_comms/maintenance_barrier.py | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/comms.py b/src/agent_comms/comms.py index 3af24a50ba..2195dcb1c7 100644 --- a/src/agent_comms/comms.py +++ b/src/agent_comms/comms.py @@ -14,6 +14,7 @@ from .collaboration_ledger import CollaborationLedger from .goal_management import Goals from .history_views import HistoryViews +from .maintenance_barrier import MaintenanceBarrier from .message_bus import MessageBus from .messaging import Messaging from .owner_lifecycle import OwnerLifecycle @@ -85,4 +86,7 @@ def client_wire(root: Path | None) -> Iterator[Comms]: """ route = resolve_comms_route(root) with route.hold_client_route(): + # Refuse before opening stores that maintenance may hold locked. This + # observation grants nothing: writes still check under their own locks. + MaintenanceBarrier(route.root / "registry.json").assert_open_unlocked() yield Comms(route, projection=CompletingRouteCounts) diff --git a/src/agent_comms/connector.py b/src/agent_comms/connector.py index 9ff7e81d2a..b765e7b0b1 100644 --- a/src/agent_comms/connector.py +++ b/src/agent_comms/connector.py @@ -48,6 +48,7 @@ CODE_SECONDS, AuthorizationRequest, ConnectorGrants, ConnectorStore, IssuedCode, RegisteredClient, digest, ) +from .maintenance_barrier import MaintenanceHeldError from .messages import Message from .todos import DirectiveText from .tool_caller import ThreadCaller @@ -291,7 +292,7 @@ async def result(self, connector: Connector) -> ToolsCallResult: ) try: reply = await asyncio.to_thread(connector.invoke, tool, self.params.arguments) - except ToolRefusedError as error: + except (ToolRefusedError, MaintenanceHeldError) as error: return ToolsCallResult.refused(error) return ToolsCallResult((TextContent(json.dumps(reply)),), False, reply) @@ -314,7 +315,7 @@ class ToolsCallResult: ) @classmethod - def refused(cls, error: ToolRefusedError) -> ToolsCallResult: + def refused(cls, error: ToolRefusedError | MaintenanceHeldError) -> ToolsCallResult: return cls((TextContent(CoreError.from_exception(error).model_dump_json()),), True) diff --git a/src/agent_comms/maintenance_barrier.py b/src/agent_comms/maintenance_barrier.py index 047deeb48c..36dc441b83 100644 --- a/src/agent_comms/maintenance_barrier.py +++ b/src/agent_comms/maintenance_barrier.py @@ -282,8 +282,10 @@ def _read(path: Path, declaration): return value def current_unlocked(self) -> MaintenanceStatus: - """Call while holding the registry lock OR the wire lock. + """Capture admission while holding the registry lock OR the wire lock. + A client may observe this without a lock only for early refusal, never + to grant admission. An incomplete transition fails closed in that read. For a send, the wire lock stays held through stdin.write; for a claim, the registry lock spans the turn's durable transition. A phase change takes both locks in wire -> registry order. From f7ff66c78082a486a32390f6dbbc8112bd830eac Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:13:57 -0400 Subject: [PATCH 17/25] P228: use the prepared endpoint credential in the real Node producer check --- tests/test_selected_tool_broker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_selected_tool_broker.py b/tests/test_selected_tool_broker.py index e69dfd1fdf..285be362ad 100644 --- a/tests/test_selected_tool_broker.py +++ b/tests/test_selected_tool_broker.py @@ -409,7 +409,7 @@ async def test_shipped_javascript_tool_uses_authenticated_owner_socket(tmp_path) tmp_path = tmp_path / ("persistent-session-" * 8) tmp_path.mkdir(mode=0o700) assert len(os.fsencode(tmp_path)) > 108 - input_id, token = secrets.token_hex(16), secrets.token_hex(32) + input_id = secrets.token_hex(16) target = tmp_path / "notes.txt" target.write_text("before") @@ -436,7 +436,7 @@ def commit(request): register({{registerTool(value) {{tool = value}}}}); await new Promise(resolve => process.stdin.once('data', resolve)); await tool.execute('paired', {json.dumps(arguments)}, new AbortController().signal, undefined, - {{ownerTools: OwnerToolConnection.capture({json.dumps({'path': str(unbound.listener.address), 'token': token})})}}); + {{ownerTools: OwnerToolConnection.capture({json.dumps({'path': str(unbound.listener.address), 'token': unbound.listener.token})})}}); """, stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, From 3e9dabd8296ee31631d8819f664d1e23f2864eab Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:14:39 -0400 Subject: [PATCH 18/25] Prove maintenance refusal through real connector on a copied bus --- tests/test_connector.py | 43 ++++++++++ tests/verify_connector_maintenance.py | 115 ++++++++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 tests/verify_connector_maintenance.py diff --git a/tests/test_connector.py b/tests/test_connector.py index 15c389300a..cc572ab036 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -5,6 +5,7 @@ import secrets import shutil import json +import time from pathlib import Path from urllib.parse import parse_qs, urlsplit @@ -21,6 +22,10 @@ RevokeVerb, ) from agent_comms.connector_store import ConnectorStore +from agent_comms.maintenance_barrier import ( + InstallingPhase, MaintenanceBarrier, MaintenanceOperator, PausedPhase, +) +from agent_comms.store_files import _store_lock from agent_comms.rule_reviews import AnsweredRuleReviewEntry, DrawnRuleReviewEntry, RuleReviews from agent_comms.tools import ReadTool, ToolRequest, invoke_tool @@ -280,6 +285,44 @@ async def test_a_call_returns_what_the_tool_returns_in_process(client, store, ro assert [thread["name"] for thread in in_process["threads"]] == ["claude-ai", "worker"] +@pytest.mark.parametrize("phases", [(), (PausedPhase,), (PausedPhase, InstallingPhase)]) +async def test_maintenance_refuses_before_locked_stores_on_a_copied_bus(comms, tmp_path, phases): + for name in ("worker", "claude-ai"): + comms.registry.declare(comms.registry.new_thread(name, frozenset(), str(comms.root))) + original = comms.messaging.send("worker", "claude-ai", "Ready for your message").message + copied_root = tmp_path / "maintenance-copy" + shutil.copytree(comms.root, copied_root) + store = ConnectorStore.at(tmp_path / "copy-auth") + connector = Connector(store, "http://127.0.0.1:18766", copied_root) + before = (copied_root / "bus.jsonl").read_bytes() + async with TestClient(TestServer(connector.application())) as reader: + headers = {"Authorization": f"Bearer {await approved_token(reader, store)}"} + barrier = MaintenanceBarrier(copied_root / "registry.json") + held = barrier.begin(MaintenanceOperator("connector-refusal-proof")) + for phase in phases: + held = barrier.advance(held, phase) + with _store_lock(barrier.wire_path), _store_lock(barrier.registry_path): + started = time.monotonic() + response = await reader.post("/mcp", headers=headers, timeout=2, json={ + "jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": "comms_send", "arguments": { + "from": "claude-ai", "to": "worker", "body": "Please read this message"}}, + }) + reply = await response.json() + elapsed = time.monotonic() - started + assert elapsed < 1, (elapsed, reply) + assert response.status == 200 + result = reply["result"] + assert result["isError"] is True + refusal = json.loads(result["content"][0]["text"]) + assert refusal == { + "type": "held", "error": "Maintenance holds ingress: the bus is under maintenance"} + assert barrier.current_unlocked() == held + assert (copied_root / "bus.jsonl").read_bytes() == before + assert comms.bus.log.latest_sequence() == original.seq + assert comms.owners.maintenance.read().turn_pending is False + + async def test_a_revoked_token_is_refused(client, store): token = await approved_token(client, store) assert RevokeVerb().apply(store) == 0 diff --git a/tests/verify_connector_maintenance.py b/tests/verify_connector_maintenance.py new file mode 100644 index 0000000000..56c01e3e81 --- /dev/null +++ b/tests/verify_connector_maintenance.py @@ -0,0 +1,115 @@ +"""Real connector HTTP refusal on a copied, maintenance-held bus; no test doubles. + +PYTHONPATH=src python -X faulthandler tests/verify_connector_maintenance.py --scratch DIR +Only private scratch and the loopback server are changed; no installed service is restarted. +""" +from __future__ import annotations + +import argparse +import json +import os +import shutil +import socket +import subprocess +import sys +import tempfile +import threading +import time +from pathlib import Path + +from agent_comms.comms import Comms +from agent_comms.connector_store import ConnectorStore +from agent_comms.maintenance_barrier import ( + InstallingPhase, MaintenanceBarrier, MaintenanceOperator, PausedPhase, +) +from agent_comms.store_files import _store_lock +from verify_connector_live import curl_call, wait_started + + +def prove(scratch: Path) -> None: + scratch.mkdir(mode=0o700, parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix="maintenance-proof-", dir=scratch) as directory: + base = Path(directory) + original = Comms(base / "original") + for name in ("claude-ai", "worker"): + original.registry.declare(original.registry.new_thread(name, frozenset(), str(base))) + original.messaging.initialize_private_initial_protocol() + original.messaging.send("worker", "claude-ai", "Ready for your message") + for worker in threading.enumerate(): + if worker.name == "agent-comms-candidate-maintenance": + worker.join(timeout=10) + assert not worker.is_alive() + before = original.bus.log.path.read_bytes() + copied = base / "copied" + shutil.copytree(original.root, copied) + home = base / "home" + auth = home / ".local/state/agent-comms/connector" + auth.mkdir(mode=0o700, parents=True) + store = ConnectorStore.at(auth) + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + project = Path(__file__).resolve().parents[1] + environment = { + **os.environ, "HOME": str(home), "PYTHONUNBUFFERED": "1", + "PYTHONPATH": str(project / "src"), + } + entrypoint = shutil.which("agent-comms-connector") + assert entrypoint, "the installed connector entrypoint is required" + error_path = base / "server.log" + with error_path.open("w") as errors: + service = subprocess.Popen( + [entrypoint, "serve", "--root", str(copied), "--port", str(port)], + env=environment, stdout=subprocess.PIPE, stderr=errors, text=True, + ) + server_log = [] + try: + wait_started(service, server_log) + barrier = MaintenanceBarrier(copied / "registry.json") + held = barrier.begin(MaintenanceOperator("connector-maintenance-proof")) + held = barrier.advance(held, PausedPhase) + held = barrier.advance(held, InstallingPhase) + with store.granted("private maintenance proof") as token: + with _store_lock(barrier.wire_path), _store_lock(barrier.registry_path): + started = time.monotonic() + output = curl_call(port, token, "Please read this message") + wall = time.monotonic() - started + response, timing = output.strip().rsplit("\n", 1) + status, curl_seconds = timing.split() + reply = json.loads(response) + result = reply["result"] + refusal = json.loads(result["content"][0]["text"]) + assert status == "200" and result["isError"] is True, output + assert refusal == { + "type": "held", + "error": "Maintenance holds ingress: the bus is under maintenance", + }, output + assert wall < 1 and float(curl_seconds) < 1, output + assert barrier.current_unlocked() == held + assert (copied / "bus.jsonl").read_bytes() == before + print(json.dumps({ + "entrypoint": entrypoint, "core_source": str(project / "src"), + "interpreter": reply["interpreter"], "phase": held.phase.declared_name, + "wire_and_registry_locks": "held exclusively throughout HTTP call", + "tool": "comms_send", "status": int(status), + "curl_seconds": float(curl_seconds), "wall_seconds": wall, + "refusal": refusal, "copied_bus_unchanged": True, + "original_bus_unchanged": original.bus.log.path.read_bytes() == before, + }, indent=2)) + assert original.bus.log.path.read_bytes() == before + assert not original.owners.maintenance.read().turn_pending + finally: + service.terminate() + tail, _ = service.communicate(timeout=15) + server_log.append(tail) + assert "Error handling request" not in error_path.read_text(), error_path.read_text() + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--scratch", required=True, type=Path) + prove(parser.parse_args().scratch) + + +if __name__ == "__main__": + main() From c270107433004ce5b9937d44996ace90f020ad37 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:18:12 -0400 Subject: [PATCH 19/25] Refuse held maintenance before route publication locks as well --- src/agent_comms/comms.py | 6 +++--- tests/test_connector.py | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/agent_comms/comms.py b/src/agent_comms/comms.py index 2195dcb1c7..3e7f76d516 100644 --- a/src/agent_comms/comms.py +++ b/src/agent_comms/comms.py @@ -85,8 +85,8 @@ def client_wire(root: Path | None) -> Iterator[Comms]: Only the UI yields between index units. """ route = resolve_comms_route(root) + # Refuse before borrowing a route or opening stores held by maintenance. + # This grants nothing: writes still check under their determining locks. + MaintenanceBarrier(route.root / "registry.json").assert_open_unlocked() with route.hold_client_route(): - # Refuse before opening stores that maintenance may hold locked. This - # observation grants nothing: writes still check under their own locks. - MaintenanceBarrier(route.root / "registry.json").assert_open_unlocked() yield Comms(route, projection=CompletingRouteCounts) diff --git a/tests/test_connector.py b/tests/test_connector.py index cc572ab036..5c472841f5 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -1,17 +1,21 @@ """The claude.ai connector: OAuth in front of an MCP transport over the ToolRequest family.""" import base64 +import fcntl import hashlib +import os import secrets import shutil import json import time +from contextlib import ExitStack from pathlib import Path from urllib.parse import parse_qs, urlsplit import pytest from aiohttp.test_utils import TestClient, TestServer +from agent_comms.active_route import active_route_path from agent_comms.comms import Comms from agent_comms.connector import ( CALLER, @@ -286,12 +290,15 @@ async def test_a_call_returns_what_the_tool_returns_in_process(client, store, ro @pytest.mark.parametrize("phases", [(), (PausedPhase,), (PausedPhase, InstallingPhase)]) -async def test_maintenance_refuses_before_locked_stores_on_a_copied_bus(comms, tmp_path, phases): +async def test_maintenance_refuses_before_locked_stores_on_a_copied_bus( + comms, tmp_path, phases, monkeypatch, +): for name in ("worker", "claude-ai"): comms.registry.declare(comms.registry.new_thread(name, frozenset(), str(comms.root))) original = comms.messaging.send("worker", "claude-ai", "Ready for your message").message - copied_root = tmp_path / "maintenance-copy" + copied_root = tmp_path / ".agent-comms" shutil.copytree(comms.root, copied_root) + monkeypatch.setenv("HOME", str(tmp_path)) store = ConnectorStore.at(tmp_path / "copy-auth") connector = Connector(store, "http://127.0.0.1:18766", copied_root) before = (copied_root / "bus.jsonl").read_bytes() @@ -301,7 +308,10 @@ async def test_maintenance_refuses_before_locked_stores_on_a_copied_bus(comms, t held = barrier.begin(MaintenanceOperator("connector-refusal-proof")) for phase in phases: held = barrier.advance(held, phase) - with _store_lock(barrier.wire_path), _store_lock(barrier.registry_path): + with _store_lock(barrier.wire_path), _store_lock(barrier.registry_path), ExitStack() as locks: + directory = os.open(active_route_path().parent, os.O_RDONLY | os.O_DIRECTORY) + locks.callback(os.close, directory) + fcntl.flock(directory, fcntl.LOCK_EX | fcntl.LOCK_NB) started = time.monotonic() response = await reader.post("/mcp", headers=headers, timeout=2, json={ "jsonrpc": "2.0", "id": 1, "method": "tools/call", From 5b0426465823ac05a2c4f384e3c32ce38be3cc46 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:18:32 -0400 Subject: [PATCH 20/25] P264: fold contiguous reads into the conversation prefix Insert new sparse reads with bisect, then consume the contiguous prefix once. Preserve through/union contracts and cover 10,000 sequential additions, duplicate reads and closed gaps. Allocate the nine implementation lines through the existing source-budget mechanism. Src-Budget: 9 --- src/agent_comms/read_basis.py | 15 ++++++++++++--- tests/test_conversation_read_cursor.py | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/read_basis.py b/src/agent_comms/read_basis.py index 9a3743eee1..e171f217a5 100644 --- a/src/agent_comms/read_basis.py +++ b/src/agent_comms/read_basis.py @@ -323,9 +323,18 @@ def union(cls, cursors: Iterable[ConversationCursor]) -> ConversationCursor: for sequence in cursor.exceptions if sequence > through}))) def add(self, sequences: tuple[int, ...]) -> ConversationCursor: - return type(self)(self.high_water, tuple(sorted( - n for n in set(self.exceptions).union(sequences) if n > self.high_water - ))) + exceptions = list(self.exceptions) + for sequence in sequences: + if sequence > self.high_water: + index = bisect_left(exceptions, sequence) + if index == len(exceptions) or exceptions[index] != sequence: + exceptions.insert(index, sequence) + high_water = self.high_water + prefix = 0 + while prefix < len(exceptions) and exceptions[prefix] == high_water + 1: + high_water += 1 + prefix += 1 + return type(self)(high_water, tuple(exceptions[prefix:])) def through(self, sequence: int) -> ConversationCursor: sequence = max(sequence, self.high_water) diff --git a/tests/test_conversation_read_cursor.py b/tests/test_conversation_read_cursor.py index 5bba6614b9..09885def29 100644 --- a/tests/test_conversation_read_cursor.py +++ b/tests/test_conversation_read_cursor.py @@ -25,6 +25,26 @@ def test_sparse_prefix_drops_only_its_covered_exceptions(): assert cursor.add((30, 20, 5)) == ConversationCursor(3, (5, 20, 30, 40, 60)) +def test_in_order_additions_become_one_prefix(): + cursor = ConversationCursor() + for sequence in range(1, 10_001): + cursor = cursor.add((sequence,)) + assert cursor.high_water == 10_000 + assert cursor.exceptions == () + + +def test_add_closes_gaps_without_losing_sparse_reads_or_mutating_the_source(): + original = ConversationCursor(3, (7, 9, 12)) + cursor = original.add((6, 4, 11, 5, 11, 2)) + assert cursor == ConversationCursor(7, (9, 11, 12)) + assert cursor.add((8, 10)) == ConversationCursor(12) + assert original == ConversationCursor(3, (7, 9, 12)) + assert cursor.through(8) == ConversationCursor(8, (9, 11, 12)) + assert ConversationCursor.union((ConversationCursor(3, (5, 7)), + ConversationCursor(4, (6, 9)))) == ConversationCursor( + 4, (5, 6, 7, 9)) + + def setup_conversations(root): comms = wire(root) for name in ("sender", "reader"): From e04e60f12ea4b4ace4509a0a71724d3e852e05f3 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:18:55 -0400 Subject: [PATCH 21/25] P252: verify captured-set overlap, preparation failure and carry ordering --- tests/test_carry.py | 160 ++++++++++++++++++++++++------------------- tools/upgrade_bus.py | 2 +- 2 files changed, 89 insertions(+), 73 deletions(-) diff --git a/tests/test_carry.py b/tests/test_carry.py index f4b86ddf3a..0d47f268c9 100644 --- a/tests/test_carry.py +++ b/tests/test_carry.py @@ -478,88 +478,112 @@ def test_turn_records_drop_an_idle_stored_turn_and_refuse_a_held_one(tmp_path, m assert not thread.executing -def test_upgrade_busy_owner_refuses_before_any_stop_or_start(tmp_path): +def test_upgrade_stops_and_starts_exactly_the_captured_set_concurrently(comms, tmp_path, monkeypatch): + from threading import Barrier + + import tools.upgrade_bus as upgrade_bus from agent_comms.maintenance_barrier import ReadyPhase - from agent_comms.restart_refusals import OwnerBusyRefusal - from tools.upgrade_bus import live - root = tmp_path / "bus" - Comms(root) + names = [f"worker-{number}" for number in range(19)] + stop = Barrier(len(names), timeout=10) + start = Barrier(len(names), timeout=10) calls = [] + commands = [] + carried_bus = [] - class BusyStack(Stack): + class Installed(Stack): def cli(self, root, *arguments): - calls.append(arguments) - return {"threads": [{"name": "worker", "role": "agent", "status": "running"}]} + if arguments == ("threads",): + assert not calls # Never rediscover a possibly changed owner set. + return {"threads": [ + *({"name": name, "role": "agent", "status": "running"} for name in names), + {"name": "idle", "role": "agent", "status": "stopped"}, + {"name": "human", "role": "human", "status": "running"}, + ]} + command, flag, name = arguments + assert flag == "--name" and name in names + if command == "stop": + stop.wait() # A serial stop loop cannot reach this barrier. + else: + assert command == "start" and carried_bus + start.wait() # Nor can a serial start loop. + calls.append((command, name)) + return {} - def require_idle(self, root, names): - assert names == ["worker"] - assert MaintenanceBarrier(root / "registry.json").read().phase is not ReadyPhase - raise OwnerBusyRefusal() + def relock(self): + assert commands[-1][-5:] == ( + "submodule", "update", "--init", "--recursive", "--checkout") - stack = BusyStack(REPO) - with pytest.raises(OwnerBusyRefusal): - live(REPO, root, "", "", (), stack, stack, None, "worker") - assert calls == [("threads",)] - assert MaintenanceBarrier(root / "registry.json").read().phase is ReadyPhase + def prepare_native(self): + pass + + def verify(self, root): + return {"threads": len(names)} + def carry(root, pending, staged, revisions): + assert sorted(calls) == sorted(("stop", name) for name in names) + carried_bus.append(root) + return [] -LIVE_DRIVER = """ -import signal, sys -from pathlib import Path -from upgrade_bus import Stack, live, stopped + monkeypatch.setattr(upgrade_bus, "carry_all", carry) + monkeypatch.setattr(upgrade_bus, "run", lambda *command, **options: commands.append(command)) + monkeypatch.setattr(upgrade_bus, "ROUTE", tmp_path / "absent-route.json") + monkeypatch.setattr(upgrade_bus.HostService, "restart_running", staticmethod(lambda python: [])) + monkeypatch.setattr(upgrade_bus.PlannedInstallDeadline, "plan", lambda self, root: None) + stack = Installed(tmp_path / "checkout") + result = upgrade_bus.live(stack.checkout, comms.root, "old", "new", (), stack, stack, None, "worker") + assert result["stopped_and_started"] == names + assert sorted(calls) == sorted((command, name) for command in ("stop", "start") for name in names) + assert MaintenanceBarrier(comms.root / "registry.json").read().phase is ReadyPhase + assert commands[0] == ("git", "-C", stack.checkout, "merge", "--ff-only", "-q", "new") -class Owners(Stack): - def require_idle(self, root, names): - pass +def test_owner_request_failure_joins_the_entire_set_before_propagating(tmp_path): + from threading import Barrier - def cli(self, root, *arguments): - print(' '.join(arguments), flush=True) - return {"threads": [{"name": "worker", "role": "agent", "status": "running"}]} + names = ["failed", "other", "last"] + admitted = Barrier(len(names), timeout=10) + finished = [] + class Installed(Stack): + def cli(self, root, *arguments): + admitted.wait() + name = arguments[-1] + finished.append(name) + if name == "failed": + raise UpgradeRefused("stop failed") + return {} -for each in (signal.SIGTERM, signal.SIGINT): - signal.signal(each, stopped) -root = Path(sys.argv[1]) -live(root, root, "", "", (), Owners(root), Owners(root), None, "worker") -""" + with pytest.raises(UpgradeRefused, match="stop failed"): + Installed(REPO).owners(tmp_path, "stop", names) + assert sorted(finished) == sorted(names) -def test_upgrade_killed_while_waiting_clears_maintenance(tmp_path): - """A reader holds the route; SIGTERM while the upgrade waits for it runs the abort: the bus - reads READY and the stopped thread is started again.""" - import fcntl - import signal +def test_selected_revision_prepares_pins_and_cannot_return_a_failed_cached_tree(tmp_path, monkeypatch): + import tools.upgrade_bus as upgrade_bus - from agent_comms.maintenance_barrier import MaintenanceBarrier, ReadyPhase + commands = [] - root = tmp_path / "bus" - Comms(root) - routes = tmp_path / "home/.local/state/agent-comms" - routes.mkdir(parents=True) - reader = os.open(routes, os.O_RDONLY | os.O_DIRECTORY) - fcntl.flock(reader, fcntl.LOCK_SH) - try: - tool = subprocess.Popen( - [sys.executable, "-c", LIVE_DRIVER, str(root)], text=True, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, - env={**os.environ, "HOME": str(tmp_path / "home"), - "PYTHONPATH": os.pathsep.join(str(REPO / part) for part in ("src", "tools"))}) - waited = "" - while "Waiting for default-route readers" not in waited: - line = tool.stderr.readline() - assert line, waited + tool.stdout.read() - waited += line - assert MaintenanceBarrier(root / "registry.json").read().phase is not ReadyPhase - tool.send_signal(signal.SIGTERM) - out, err = tool.communicate(timeout=60) - finally: - os.close(reader) - assert tool.returncode != 0 - assert "stopped by SIGTERM" in err and "aborted before the backup" in err - assert out.splitlines() == ["threads", "stop --name worker", "start --name worker"] - assert MaintenanceBarrier(root / "registry.json").read().phase is ReadyPhase + def run(*command): + commands.append(command) + return "" + + monkeypatch.setattr(upgrade_bus, "run", run) + revisions = Revisions(tmp_path / "repo", tmp_path / "trees") + selected = revisions.tree("abcdef123456") + assert commands == [ + ("git", "-C", revisions.repo, "worktree", "add", "--detach", "-q", selected, "abcdef123456"), + ("git", "-C", selected, "submodule", "update", "--init", "--recursive", "--checkout"), + ] + + def missing_dependency(*command): + assert command[3:] == ("submodule", "update", "--init", "--recursive", "--checkout") + raise UpgradeRefused("missing pinned dependency") + + monkeypatch.setattr(upgrade_bus, "run", missing_dependency) + with pytest.raises(UpgradeRefused, match="missing pinned dependency"): + revisions.tree("abcdef123456") + assert revisions.trees == {"abcdef123456": selected} # Failed preparation still owns cleanup. def test_failed_carry_preserves_changed_bus_and_main_under_maintenance(comms, tmp_path, monkeypatch): @@ -576,9 +600,6 @@ def cli(self, root, *arguments): assert arguments == ("threads",) return {"threads": []} - def require_idle(self, root, names): - assert not names - def relock(self): pass @@ -591,7 +612,6 @@ def failed_carry(*args): monkeypatch.setattr(upgrade_bus, "run", lambda *command, **options: commands.append(command)) monkeypatch.setattr(upgrade_bus, "carry_all", failed_carry) - monkeypatch.setattr(upgrade_bus, "route_readers", lambda: None) monkeypatch.setattr(upgrade_bus, "ROUTE", tmp_path / "route.json") stack = Installed(tmp_path / "checkout") with pytest.raises(RuntimeError, match="carry failed after writing"): @@ -680,9 +700,6 @@ class Installed(Stack): def cli(self, root, *arguments): return {"threads": []} - def require_idle(self, root, names): - pass - def relock(self): pass @@ -694,7 +711,6 @@ def verify(self, root): monkeypatch.setattr(upgrade_bus, "carry_all", tool_call_index) monkeypatch.setattr(upgrade_bus, "run", lambda *command, **options: "") - monkeypatch.setattr(upgrade_bus, "route_readers", lambda: None) # A rehearsal places the route under its own home, so the route sits under this one. monkeypatch.setattr(upgrade_bus, "ROUTE", upgrade_bus.ROUTE.with_name(f"absent-{tmp_path.name}.json")) monkeypatch.setattr(upgrade_bus.HostService, "restart_running", staticmethod(lambda python: [])) diff --git a/tools/upgrade_bus.py b/tools/upgrade_bus.py index e4e88f76c6..8bb34727e6 100644 --- a/tools/upgrade_bus.py +++ b/tools/upgrade_bus.py @@ -146,7 +146,7 @@ def cli(self, root: Path, *arguments: str) -> dict: return result def owners(self, root: Path, command: str, names: list[str]) -> None: - """Admit the captured owner set together and join every request before proceeding.""" + """Run the captured owner set together and join every request before proceeding.""" with ThreadPoolExecutor(max_workers=max(1, len(names))) as pool: list(pool.map(lambda name: self.cli(root, command, "--name", name), names)) From b9965646fa89f8a76944eddf4772a0c50b44bf5e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:22:39 -0400 Subject: [PATCH 22/25] Describe maintenance refusal in plain language --- src/agent_comms/maintenance_barrier.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/maintenance_barrier.py b/src/agent_comms/maintenance_barrier.py index 36dc441b83..df2e2bb705 100644 --- a/src/agent_comms/maintenance_barrier.py +++ b/src/agent_comms/maintenance_barrier.py @@ -282,10 +282,10 @@ def _read(path: Path, declaration): return value def current_unlocked(self) -> MaintenanceStatus: - """Capture admission while holding the registry lock OR the wire lock. + """Check the phase under the registry lock OR the wire lock. - A client may observe this without a lock only for early refusal, never - to grant admission. An incomplete transition fails closed in that read. + Clients may read it before taking locks only to refuse work, never to + permit a write. An incomplete phase change refuses that read. For a send, the wire lock stays held through stdin.write; for a claim, the registry lock spans the turn's durable transition. A phase change takes both locks in wire -> registry order. From 30fea4b9987df253314de0254e2c903512b814a8 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:29:13 -0400 Subject: [PATCH 23/25] P264: rebind checked bus copies without rebuilding their unchanged index --- src/agent_comms/checkpoint_seals.py | 14 ++++++- src/agent_comms/private_bus_checkpoint.py | 49 ++++++++++++++--------- tests/test_declared_family.py | 2 +- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/agent_comms/checkpoint_seals.py b/src/agent_comms/checkpoint_seals.py index 30327b35a3..6616a76bec 100644 --- a/src/agent_comms/checkpoint_seals.py +++ b/src/agent_comms/checkpoint_seals.py @@ -10,7 +10,7 @@ from .declared_family import DeclaredFamily from .errors import CheckpointNeedsRepairError, RelationViolationError from .field_codec import Id128Text, Sha256Text -from .private_path import FileRevision +from .private_path import FileIdentity, FileRevision if TYPE_CHECKING: import os @@ -100,7 +100,15 @@ def recover( path: Path, saved: PrefixWitness, info: os.stat_result, - ) -> None: + ) -> PrefixWitness | None: + if saved.revision.identity != FileIdentity.from_stat(info): + self.require_rebuilt(saved, saved) + copied = FileRevision.from_stat(path.stat()) + if (copied.size, copied.mtime_ns) != (self.db_revision.size, self.db_revision.mtime_ns): + raise RelationViolationError("Private bus copied index content revision changed.") + if info.st_size == saved.offset: + return saved.rebind_source(bus, marker, db, path, info) + return None self.check_final(saved, path) def require_rebuilt(self, saved: PrefixWitness, rebuilt: PrefixWitness) -> None: @@ -137,6 +145,8 @@ def recover( or self.prior.root_id != marker.root_id ): raise RelationViolationError("Private bus checkpoint pending intent is inconsistent.") + if replace(self.prior, revision=self.expected.revision) == self.expected: + return saved.rebind_source(bus, marker, db, path, info) return _rebuild_checkpoint_unlocked(bus, marker, db, path, info, self) def require_rebuilt(self, saved: PrefixWitness, rebuilt: PrefixWitness) -> None: diff --git a/src/agent_comms/private_bus_checkpoint.py b/src/agent_comms/private_bus_checkpoint.py index 7687dd4816..d0fb511ccb 100644 --- a/src/agent_comms/private_bus_checkpoint.py +++ b/src/agent_comms/private_bus_checkpoint.py @@ -92,15 +92,28 @@ def require_marker(self, marker: WireMetadata) -> None: raise RelationViolationError("Checked bus prefix differs from its stored marker.") def require_source(self, marker: WireMetadata, info: os.stat_result, stream: BinaryIO) -> None: - """Reject changed bytes; a copied file needs rebuilding under the write lock.""" + """Validate the logical prefix independently of its physical file identity.""" if self.root_id != marker.root_id or info.st_size < self.offset: raise RelationViolationError("Private bus checkpoint root/size changed.") if _tail(stream, self.offset) != self.tail: raise RelationViolationError("Private bus checkpoint prefix tail changed.") if self.through_seq > marker.last_seq: raise RelationViolationError("Private bus checkpoint exceeds the durable sequence marker.") - if self.revision.identity != FileIdentity.from_stat(info): - raise CheckpointNeedsRepairError("Private bus source was relocated.") + + def rebind_source(self, bus: WireLog, marker: WireMetadata, db: sqlite3.Connection, + path: Path, info: os.stat_result) -> PrefixWitness: + """Publish the unchanged prefix's new revision under the exclusive bus lock.""" + rebound = replace(self, revision=replace(FileRevision.from_stat(info), size=self.offset)) + marker.seal.require_rebuilt(self, rebound) + marker.seal_with(PendingSeal.capture(self, rebound, path)) + bus.write_metadata_unlocked(marker) + with db: + CheckedPrefix.capture(rebound.root_id, rebound.revision, rebound.through_seq, + bytes.fromhex(rebound.digest), rebound.tail).upsert(db) + _directory_sync(path) + marker.seal_with(FinalSeal.capture(rebound, path)) + bus.write_metadata_unlocked(marker) + return rebound def require_append(self, marker: WireMetadata, info: os.stat_result, offset: int, sequence: int) -> None: @@ -1086,7 +1099,7 @@ def collect(offset, raw, record): != FileRevision.from_stat(info) ): raise RelationViolationError("Private bus changed during checkpoint installation.") - cls.capture(marker.root_id, info, through_seq, digest, + cls.capture(marker.root_id, FileRevision.from_stat(info), through_seq, digest, _tail(stream, info.st_size)).insert(db) _directory_sync(staged) with closing(_connect(staged, readonly=True)) as db: @@ -1135,20 +1148,20 @@ def witness(self) -> PrefixWitness: @classmethod def capture( - cls, root_id: str, info: os.stat_result, seq: int, digest: bytes, tail: str + cls, root_id: str, revision: FileRevision, seq: int, digest: bytes, tail: str ) -> CheckedPrefix: return cls( 1, 2, root_id, - info.st_dev, - info.st_ino, - info.st_size, + revision.identity.device, + revision.identity.inode, + revision.size, seq, digest.hex(), tail, - info.st_mtime_ns, - info.st_ctime_ns, + revision.mtime_ns, + revision.ctime_ns, ) @@ -1309,7 +1322,7 @@ def collect(offset, raw, record): seal.require_rebuilt(saved, expected) marker.seal_with(PendingSeal.capture(saved, expected, db_path)) bus.write_metadata_unlocked(marker) - CheckedPrefix.capture(expected.root_id, info, last_seq, digest, expected.tail).upsert( + CheckedPrefix.capture(expected.root_id, expected.revision, last_seq, digest, expected.tail).upsert( db ) _directory_sync(db_path) @@ -1323,9 +1336,9 @@ def _check_final_checkpoint_read_only(bus, marker, db, stream, path) -> PrefixWi saved = _saved(db) info = os.fstat(stream.fileno()) saved.require_source(marker, info, stream) - marker.seal.require_final(saved, path) if FileRevision.from_stat(info) != saved.revision: - raise CheckpointNeedsRepairError("Bus has rows the checkpoint does not cover yet.") + raise CheckpointNeedsRepairError("Bus revision requires exclusive checkpoint validation.") + marker.seal.require_final(saved, path) return saved @@ -1358,11 +1371,7 @@ def _verify_open_checkpoint_unlocked(bus, marker, db, stream, path) -> PrefixWit _directory_sync(bus.path) saved = _saved(db) info = os.fstat(stream.fileno()) - try: - saved.require_source(marker, info, stream) - except CheckpointNeedsRepairError: - # The copied index rows point at the old file; rebuild them. - return _rebuild_checkpoint_unlocked(bus, marker, db, path, info, marker.seal) + saved.require_source(marker, info, stream) recovered = marker.seal.recover(bus, marker, db, path, saved, info) if recovered is not None: return recovered @@ -1430,7 +1439,7 @@ def collect( for offset, raw, record in additions: indexed_sequence = _index_row(db, offset, raw, record, indexed_sequence) CheckedPrefix.capture( - saved.root_id, info, last_seq, digest, expected.tail + saved.root_id, expected.revision, last_seq, digest, expected.tail ).upsert(db) _directory_sync(path) marker.seal_with(FinalSeal.capture(expected, path)) @@ -1502,7 +1511,7 @@ def append_private_bus_checkpoint_unlocked( with db: _index_row(db, offset, raw, record, saved.through_seq) CheckedPrefix.capture( - saved.root_id, info, sequence, digest, expected.tail + saved.root_id, expected.revision, sequence, digest, expected.tail ).upsert(db) _directory_sync(path) marker.seal_with(FinalSeal.capture(expected, path)) diff --git a/tests/test_declared_family.py b/tests/test_declared_family.py index 373a1a3e88..110b206679 100644 --- a/tests/test_declared_family.py +++ b/tests/test_declared_family.py @@ -151,7 +151,7 @@ def test_checkpoint_schema_query_stays_local_but_external_names_still_discover() for table in tables: table.create(db) checked_prefix = checkpoint.CheckedPrefix.capture( - '0' * 32, os.fstat(source.fileno()), 0, bytes(32), '0' * 64) + '0' * 32, checkpoint.FileRevision.from_file(source), 0, bytes(32), '0' * 64) checked_prefix.insert(db) assert checkpoint._saved(db) == checked_prefix.witness() assert 'agent_comms.acp' not in sys.modules From 354b3aeec7e3375d27325a23de10b053aa19e80a Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:30:01 -0400 Subject: [PATCH 24/25] Declare maintenance holds as tool refusals MaintenanceHeldError composes relation and tool refusal behavior; connector consumers catch the one owning family. Remove the explicit exception tuple and union. Tighten real copied-bus proof to 0.1 seconds. --- src/agent_comms/connector.py | 5 ++--- src/agent_comms/maintenance_barrier.py | 4 ++-- tests/verify_connector_maintenance.py | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/agent_comms/connector.py b/src/agent_comms/connector.py index b765e7b0b1..9ff7e81d2a 100644 --- a/src/agent_comms/connector.py +++ b/src/agent_comms/connector.py @@ -48,7 +48,6 @@ CODE_SECONDS, AuthorizationRequest, ConnectorGrants, ConnectorStore, IssuedCode, RegisteredClient, digest, ) -from .maintenance_barrier import MaintenanceHeldError from .messages import Message from .todos import DirectiveText from .tool_caller import ThreadCaller @@ -292,7 +291,7 @@ async def result(self, connector: Connector) -> ToolsCallResult: ) try: reply = await asyncio.to_thread(connector.invoke, tool, self.params.arguments) - except (ToolRefusedError, MaintenanceHeldError) as error: + except ToolRefusedError as error: return ToolsCallResult.refused(error) return ToolsCallResult((TextContent(json.dumps(reply)),), False, reply) @@ -315,7 +314,7 @@ class ToolsCallResult: ) @classmethod - def refused(cls, error: ToolRefusedError | MaintenanceHeldError) -> ToolsCallResult: + def refused(cls, error: ToolRefusedError) -> ToolsCallResult: return cls((TextContent(CoreError.from_exception(error).model_dump_json()),), True) diff --git a/src/agent_comms/maintenance_barrier.py b/src/agent_comms/maintenance_barrier.py index df2e2bb705..3901015c0b 100644 --- a/src/agent_comms/maintenance_barrier.py +++ b/src/agent_comms/maintenance_barrier.py @@ -20,7 +20,7 @@ from typing import Annotated, Literal from .declared_family import DeclaredFamily -from .errors import RelationViolationError +from .errors import RelationViolationError, ToolRefusedError from .field_codec import FieldCodec, Id128Text, MalformedRecord, TextRepresentation from .lifecycle import LifecycleState from .send_outcome import GrantedSend, HeldSend @@ -33,7 +33,7 @@ ) -class MaintenanceHeldError(RelationViolationError): +class MaintenanceHeldError(RelationViolationError, ToolRefusedError): """The bus is in maintenance: its readers close and wait for READY.""" diff --git a/tests/verify_connector_maintenance.py b/tests/verify_connector_maintenance.py index 56c01e3e81..71a36975a0 100644 --- a/tests/verify_connector_maintenance.py +++ b/tests/verify_connector_maintenance.py @@ -84,7 +84,7 @@ def prove(scratch: Path) -> None: "type": "held", "error": "Maintenance holds ingress: the bus is under maintenance", }, output - assert wall < 1 and float(curl_seconds) < 1, output + assert wall < 0.1 and float(curl_seconds) < 0.1, output assert barrier.current_unlocked() == held assert (copied / "bus.jsonl").read_bytes() == before print(json.dumps({ From 787a99bfb20e2c0b87edafb5eb7cab989126f972 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 17:43:30 -0400 Subject: [PATCH 25/25] P264: recover concrete prefixes through chunked tail commits Incorporates chunked commit_tail and removal of the cold recovery bound from desktop PR #1976 (core-read-locks-2), head 1c7cd836697d79c2f0bd9b790455d2bd05636773. PrefixWitness owns tail continuation; the seal family owns each recovery cut. Delete nullable recovery and both full-prefix rescans. --- src/agent_comms/checkpoint_seals.py | 48 ++-- src/agent_comms/private_bus_checkpoint.py | 260 +++++++--------------- src/agent_comms/wire_log.py | 11 +- 3 files changed, 126 insertions(+), 193 deletions(-) diff --git a/src/agent_comms/checkpoint_seals.py b/src/agent_comms/checkpoint_seals.py index 6616a76bec..b20046bd45 100644 --- a/src/agent_comms/checkpoint_seals.py +++ b/src/agent_comms/checkpoint_seals.py @@ -66,7 +66,14 @@ def recover( path: Path, saved: PrefixWitness, info: os.stat_result, - ) -> PrefixWitness | None: ... + ) -> PrefixWitness: + return saved.index_tail(bus, marker, db, path, info) + + @abstractmethod + def recovery_end(self, info: os.stat_result) -> int: ... + + @abstractmethod + def chunk_complete(self, offset: int, size: int, rows: int) -> bool: ... def check_final(self, saved: PrefixWitness, db_path: Path) -> None: raise RelationViolationError("Private bus checkpoint index seal changed or is pending.") @@ -100,16 +107,24 @@ def recover( path: Path, saved: PrefixWitness, info: os.stat_result, - ) -> PrefixWitness | None: + ) -> PrefixWitness: if saved.revision.identity != FileIdentity.from_stat(info): self.require_rebuilt(saved, saved) copied = FileRevision.from_stat(path.stat()) if (copied.size, copied.mtime_ns) != (self.db_revision.size, self.db_revision.mtime_ns): raise RelationViolationError("Private bus copied index content revision changed.") - if info.st_size == saved.offset: - return saved.rebind_source(bus, marker, db, path, info) - return None - self.check_final(saved, path) + saved = saved.rebind_source(bus, marker, db, info) + else: + self.check_final(saved, path) + if saved.revision == FileRevision.from_stat(info): + return saved + return super().recover(bus, marker, db, path, saved, info) + + def recovery_end(self, info: os.stat_result) -> int: + return info.st_size + + def chunk_complete(self, offset: int, size: int, rows: int) -> bool: + return size >= 16 * 1024 * 1024 or rows >= 10_000 def require_rebuilt(self, saved: PrefixWitness, rebuilt: PrefixWitness) -> None: if self.witness != saved.seal(): @@ -137,17 +152,18 @@ def recover( saved: PrefixWitness, info: os.stat_result, ) -> PrefixWitness: - from .private_bus_checkpoint import _rebuild_checkpoint_unlocked - - if ( - saved.seal() not in (self.prior, self.expected) - or self.expected.revision != FileRevision.from_stat(info) - or self.prior.root_id != marker.root_id - ): + if (saved.seal() not in (self.prior, self.expected) + or not self.expected.revision.holds_cut( + FileRevision.from_stat(info), self.expected.revision.size) + or self.prior.root_id != marker.root_id): raise RelationViolationError("Private bus checkpoint pending intent is inconsistent.") - if replace(self.prior, revision=self.expected.revision) == self.expected: - return saved.rebind_source(bus, marker, db, path, info) - return _rebuild_checkpoint_unlocked(bus, marker, db, path, info, self) + return super().recover(bus, marker, db, path, saved, info) + + def recovery_end(self, info: os.stat_result) -> int: + return self.expected.revision.size + + def chunk_complete(self, offset: int, size: int, rows: int) -> bool: + return offset == self.expected.revision.size def require_rebuilt(self, saved: PrefixWitness, rebuilt: PrefixWitness) -> None: if ( diff --git a/src/agent_comms/private_bus_checkpoint.py b/src/agent_comms/private_bus_checkpoint.py index d0fb511ccb..5d43cc2048 100644 --- a/src/agent_comms/private_bus_checkpoint.py +++ b/src/agent_comms/private_bus_checkpoint.py @@ -26,7 +26,7 @@ from typing import TYPE_CHECKING, BinaryIO, ClassVar, Generic, Literal, TypeVar from .bus_source_page import AddressedPage -from .checkpoint_seals import CheckpointSeal, FinalSeal, PendingSeal, PrefixSeal, PrefixSource +from .checkpoint_seals import FinalSeal, PendingSeal, PrefixSeal, PrefixSource from .coordination_database import CoordinationStore from .coordination_errors import IdentityConflict from .envelope_claim_transitions import ( @@ -101,19 +101,59 @@ def require_source(self, marker: WireMetadata, info: os.stat_result, stream: Bin raise RelationViolationError("Private bus checkpoint exceeds the durable sequence marker.") def rebind_source(self, bus: WireLog, marker: WireMetadata, db: sqlite3.Connection, - path: Path, info: os.stat_result) -> PrefixWitness: + info: os.stat_result) -> PrefixWitness: """Publish the unchanged prefix's new revision under the exclusive bus lock.""" rebound = replace(self, revision=replace(FileRevision.from_stat(info), size=self.offset)) - marker.seal.require_rebuilt(self, rebound) - marker.seal_with(PendingSeal.capture(self, rebound, path)) - bus.write_metadata_unlocked(marker) - with db: - CheckedPrefix.capture(rebound.root_id, rebound.revision, rebound.through_seq, - bytes.fromhex(rebound.digest), rebound.tail).upsert(db) - _directory_sync(path) - marker.seal_with(FinalSeal.capture(rebound, path)) - bus.write_metadata_unlocked(marker) - return rebound + return CheckedPrefix.commit_tail(bus, marker, db, self, rebound, []) + + def index_tail(self, bus: WireLog, marker: WireMetadata, db: sqlite3.Connection, + path: Path, info: os.stat_result) -> PrefixWitness: + """Continue the proved cut, committing bounded chunks without a prefix rescan.""" + saved = self + if saved.offset == 0 and saved.digest != _SEED.hex(): + raise RelationViolationError("Private bus has no proved empty prefix.") + digest = bytes.fromhex(saved.digest) + last_seq = saved.through_seq + additions: list[tuple[int, bytes, WireRecord]] = [] + size = 0 + end = marker.seal.recovery_end(info) + revision = FileRevision.from_stat(info) + with bus.path.open("rb") as stream: + def commit(cut: int) -> None: + nonlocal saved, additions, size + if (FileRevision.from_file(stream) != revision + or FileRevision.from_stat(bus.path.stat()) != revision): + raise RelationViolationError("Private bus changed during tail recovery.") + expected = replace( + saved, revision=replace(revision, size=cut), through_seq=last_seq, + digest=digest.hex(), tail=_tail(stream, cut)) + saved = CheckedPrefix.commit_tail(bus, marker, db, saved, expected, additions) + additions, size = [], 0 + + def collect(offset: int, raw: bytes, record: WireRecord) -> None: + nonlocal digest, last_seq, size + digest = _chain(digest, raw) + last_seq = record.sequence_after(last_seq) + if last_seq > marker.last_seq: + raise RelationViolationError("Private bus tail exceeds its reserved sequence.") + additions.append((offset, raw, record)) + size += len(raw) + cut = offset + len(raw) + if marker.seal.chunk_complete(cut, size, len(additions)): + commit(cut) + + for _ in bus.verified_records_unlocked( + marker, on_row=collect, offset=saved.offset, + previous_sequence=saved.through_seq, end=end, + ): + pass + if additions or saved.revision != replace(revision, size=end): + commit(end) + else: + marker.seal.require_rebuilt(saved, saved) + marker.seal_with(FinalSeal.capture(saved, path)) + bus.write_metadata_unlocked(marker) + return marker.seal.recover(bus, marker, db, path, saved, info) def require_append(self, marker: WireMetadata, info: os.stat_result, offset: int, sequence: int) -> None: @@ -1099,8 +1139,9 @@ def collect(offset, raw, record): != FileRevision.from_stat(info) ): raise RelationViolationError("Private bus changed during checkpoint installation.") - cls.capture(marker.root_id, FileRevision.from_stat(info), through_seq, digest, - _tail(stream, info.st_size)).insert(db) + cls.capture( + marker.root_id, FileRevision.from_stat(info), through_seq, + digest, _tail(stream, info.st_size)).insert(db) _directory_sync(staged) with closing(_connect(staged, readonly=True)) as db: witness = _saved(db) @@ -1150,19 +1191,35 @@ def witness(self) -> PrefixWitness: def capture( cls, root_id: str, revision: FileRevision, seq: int, digest: bytes, tail: str ) -> CheckedPrefix: - return cls( - 1, - 2, - root_id, - revision.identity.device, - revision.identity.inode, - revision.size, - seq, - digest.hex(), - tail, - revision.mtime_ns, - revision.ctime_ns, - ) + return cls.from_witness(PrefixWitness(root_id, revision, seq, digest.hex(), tail)) + + @classmethod + def from_witness(cls, witness: PrefixWitness) -> CheckedPrefix: + revision = witness.revision + return cls(1, 2, witness.root_id, revision.identity.device, revision.identity.inode, + witness.offset, witness.through_seq, witness.digest, witness.tail, + revision.mtime_ns, revision.ctime_ns) + + @classmethod + def commit_tail(cls, bus: WireLog, marker: WireMetadata, db: sqlite3.Connection, + prior: PrefixWitness, expected: PrefixWitness, + additions: list[tuple[int, bytes, WireRecord]]) -> PrefixWitness: + """Commit one chunk through the same intent as an ordinary append.""" + path = _path(bus.path) + marker.seal.require_rebuilt(prior, expected) + marker.seal_with(PendingSeal.capture(prior, expected, path)) + bus.write_metadata_unlocked(marker) + with db: + sequence = prior.through_seq + for offset, raw, record in additions: + sequence = _index_row(db, offset, raw, record, sequence) + if sequence != expected.through_seq: + raise RelationViolationError("Indexed tail differs from its sealed sequence.") + cls.from_witness(expected).upsert(db) + _directory_sync(path) + marker.seal_with(FinalSeal.capture(expected, path)) + bus.write_metadata_unlocked(marker) + return expected def _path(bus_path: Path) -> Path: @@ -1268,69 +1325,6 @@ def checked_prefix_exists(bus_path: Path) -> bool: return path.exists() or path.is_symlink() -def _rebuild_checkpoint_unlocked( - bus: WireLog, - marker: WireMetadata, - db: sqlite3.Connection, - db_path: Path, - info: os.stat_result, - seal: CheckpointSeal, -) -> PrefixWitness: - """Read the whole bus and rebuild its index while holding the write lock.""" - saved = _saved(db) - digest = _SEED - prior_seen = saved.offset == 0 - last_seq = 0 - count = 0 - size = 0 - - with db: - for owner in TypedTable.members_with(CheckpointTable): - owner.delete(db) - - def collect(offset, raw, record): - nonlocal digest, prior_seen, last_seq, count, size - count += 1 - size += len(raw) - if count > 100_000 or size > 128 * 1024 * 1024: - raise RelationViolationError("Private bus pending recovery exceeds cold bound.") - digest = _chain(digest, raw) - last_seq = _index_row(db, offset, raw, record, last_seq) - if offset + len(raw) == saved.offset: - prior_seen = True - if digest.hex() != saved.digest or last_seq != saved.through_seq: - raise RelationViolationError( - "Private bus checkpoint old prefix changed during recovery." - ) - - for _ in bus.verified_records_unlocked(marker, on_row=collect): - pass - if not prior_seen or (saved.offset == 0 and saved.digest != _SEED.hex()): - raise RelationViolationError("Private bus checkpoint old prefix is unavailable.") - if FileRevision.from_stat(bus.path.stat()) != FileRevision.from_stat(info): - raise RelationViolationError("Private bus changed during pending recovery.") - with bus.path.open("rb") as stream: - expected = PrefixWitness( - marker.root_id, - FileRevision.from_stat(info), - last_seq, - digest.hex(), - _tail(stream, info.st_size), - ) - if last_seq > marker.last_seq: - raise RelationViolationError("Private bus checkpoint exceeds the durable sequence marker.") - seal.require_rebuilt(saved, expected) - marker.seal_with(PendingSeal.capture(saved, expected, db_path)) - bus.write_metadata_unlocked(marker) - CheckedPrefix.capture(expected.root_id, expected.revision, last_seq, digest, expected.tail).upsert( - db - ) - _directory_sync(db_path) - marker.seal_with(FinalSeal.capture(expected, db_path)) - bus.write_metadata_unlocked(marker) - return expected - - def _check_final_checkpoint_read_only(bus, marker, db, stream, path) -> PrefixWitness: """Check a final, up-to-date checkpoint under the shared bus lock; never writes.""" saved = _saved(db) @@ -1372,79 +1366,7 @@ def _verify_open_checkpoint_unlocked(bus, marker, db, stream, path) -> PrefixWit saved = _saved(db) info = os.fstat(stream.fileno()) saved.require_source(marker, info, stream) - recovered = marker.seal.recover(bus, marker, db, path, saved, info) - if recovered is not None: - return recovered - if FileRevision.from_stat(info) == saved.revision: - return saved - # Changed revision: a suffix alone cannot rule out an earlier in-place edit. - # Validate complete canonical history, compare digest at the saved offset, - # and collect only newly appended rows for one atomic index transaction. - digest = _SEED - observed_prefix = saved.offset == 0 - prefix_seq = 0 - additions: list[tuple[int, bytes, WireRecord]] = [] - suffix_bytes = 0 - last_seq = 0 - - def collect( - offset: int, - raw: bytes, - record: WireRecord, - ) -> None: - nonlocal digest, observed_prefix, prefix_seq, suffix_bytes, last_seq - digest = _chain(digest, raw) - end = offset + len(raw) - last_seq = record.sequence_after(last_seq) - if end == saved.offset: - observed_prefix = True - prefix_seq = last_seq - if digest.hex() != saved.digest or prefix_seq != saved.through_seq: - raise RelationViolationError( - "Private bus checkpoint checked prefix changed." - ) - elif end > saved.offset: - if not observed_prefix or offset < saved.offset: - raise RelationViolationError( - "Private bus checkpoint offset is not a row boundary." - ) - suffix_bytes += len(raw) - if suffix_bytes > 16 * 1024 * 1024 or len(additions) >= 10_000: - raise RelationViolationError( - "Private bus checkpoint crash suffix exceeds recovery bound." - ) - additions.append((offset, raw, record)) - - for _ in bus.verified_records_unlocked(marker, on_row=collect): - pass - if not observed_prefix or (saved.offset == 0 and saved.digest != _SEED.hex()): - raise RelationViolationError("Private bus checkpoint prefix is unavailable.") - if ( - FileRevision.from_stat(os.fstat(stream.fileno())) != FileRevision.from_stat(info) - or FileRevision.from_stat(bus.path.stat()) != FileRevision.from_stat(info) - ): - raise RelationViolationError("Private bus changed during checkpoint validation.") - # Full parser above checked all cross-prefix response key duplicates. - expected = PrefixWitness( - saved.root_id, - FileRevision.from_stat(info), - last_seq, - digest.hex(), - _tail(stream, info.st_size), - ) - marker.seal_with(PendingSeal.capture(saved, expected, path)) - bus.write_metadata_unlocked(marker) - with db: - indexed_sequence = saved.through_seq - for offset, raw, record in additions: - indexed_sequence = _index_row(db, offset, raw, record, indexed_sequence) - CheckedPrefix.capture( - saved.root_id, expected.revision, last_seq, digest, expected.tail - ).upsert(db) - _directory_sync(path) - marker.seal_with(FinalSeal.capture(expected, path)) - bus.write_metadata_unlocked(marker) - return _saved(db) + return marker.seal.recover(bus, marker, db, path, saved, info) @contextmanager @@ -1506,17 +1428,7 @@ def append_private_bus_checkpoint_unlocked( digest.hex(), _tail(stream, info.st_size), ) - marker.seal_with(PendingSeal.capture(saved, expected, path)) - bus.write_metadata_unlocked(marker) - with db: - _index_row(db, offset, raw, record, saved.through_seq) - CheckedPrefix.capture( - saved.root_id, expected.revision, sequence, digest, expected.tail - ).upsert(db) - _directory_sync(path) - marker.seal_with(FinalSeal.capture(expected, path)) - bus.write_metadata_unlocked(marker) - return _saved(db) + return CheckedPrefix.commit_tail(bus, marker, db, saved, expected, [(offset, raw, record)]) diff --git a/src/agent_comms/wire_log.py b/src/agent_comms/wire_log.py index 4aaa135198..4129ba0cc5 100644 --- a/src/agent_comms/wire_log.py +++ b/src/agent_comms/wire_log.py @@ -8,6 +8,7 @@ import os import sqlite3 import stat +import sys import time from abc import ABC, abstractmethod from collections.abc import Awaitable, Callable, Iterable, Iterator, Mapping @@ -772,13 +773,17 @@ def verified_records_unlocked( metadata: WireMetadata, *, on_row: Callable[[int, bytes, WireRecord], None] | None = None, + offset: int = 0, + previous_sequence: int = 0, + end: int = sys.maxsize, ) -> Iterator[WireRecord]: - """Read the whole file strictly; reject corruption even after the requested append.""" - scan = WireScan(metadata) + """Decode the selected range, continuing its already proved sequence.""" + scan = WireScan(metadata, previous_sequence=previous_sequence) if not self.path.exists(): return with self.path.open("rb") as stream: - for offset, line in WireRecord.rows(stream): + stream.seek(offset) + for offset, line in WireRecord.rows(stream, end, end): try: record = scan.read(line) except StoredHistoryUnreadable as error: