From 43f77886d01eb1d1d20e36b9c52d672e446edd18 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:33:48 -0400 Subject: [PATCH 01/10] P271: let orders close merges through nominal repository identity --- src/agent_comms/commander/config.py | 2 +- src/agent_comms/commander/standing.py | 3 +- src/agent_comms/goal_actions.py | 3 +- src/agent_comms/repo.py | 19 +++++++-- src/agent_comms/repository_name.py | 54 ++++++++++++++++++++++++ src/agent_comms/todos.py | 60 +++++++++++++-------------- src/agent_comms/work_branches.py | 4 +- 7 files changed, 105 insertions(+), 40 deletions(-) create mode 100644 src/agent_comms/repository_name.py diff --git a/src/agent_comms/commander/config.py b/src/agent_comms/commander/config.py index c837f375d7..6700c80308 100644 --- a/src/agent_comms/commander/config.py +++ b/src/agent_comms/commander/config.py @@ -85,7 +85,7 @@ def packaged_rules_files(cls) -> tuple[Path, ...]: def repo_directory(self, name: str) -> Path: """A configured repository 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() + if repo.github.named(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.""" diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index 8b6b85eb00..e8b602127f 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -13,6 +13,7 @@ from ..goal_actions import OrderGoalAction from ..goals import Goal from ..repo import Repo +from ..repository_name import RepositoryName from ..standing_rules import ( DatedStandingRule, ReasonedStandingRule, @@ -211,7 +212,7 @@ class MergeGrowth(DatedStandingRule): limit: ClassVar[int] = 40 window: ClassVar[str] = "12 hours ago" - repo: str + repo: RepositoryName sha: str subject: str merged: float diff --git a/src/agent_comms/goal_actions.py b/src/agent_comms/goal_actions.py index 59b73b4ae7..2088e9735e 100644 --- a/src/agent_comms/goal_actions.py +++ b/src/agent_comms/goal_actions.py @@ -37,6 +37,7 @@ UnorderedGoalOrder, ) from .progress_claims import ProgressClaim +from .repository_name import RepositoryName from .recorded_time import EventTime from .thread_identity import ThreadIncarnation from .threads import Thread @@ -563,7 +564,7 @@ def reserve(self, ctx: GoalActionContext, generation: str, goal: GoalRef) -> Tod @dataclass(frozen=True, kw_only=True) class CreateProblemGoalAction(ProblemOrderGoalAction): - repo: str + repo: RepositoryName text: str def bound_to(self, actor: ThreadIncarnation) -> CreateProblemGoalAction: diff --git a/src/agent_comms/repo.py b/src/agent_comms/repo.py index b6cfe20dbb..f080cfaf21 100644 --- a/src/agent_comms/repo.py +++ b/src/agent_comms/repo.py @@ -4,8 +4,14 @@ import subprocess from pathlib import Path +from typing import TYPE_CHECKING, Annotated -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, InstanceOf, PlainSerializer, PlainValidator + +from .repository_name import RepositoryName + +if TYPE_CHECKING: + from .todos import Todo class Repo(BaseModel): @@ -14,9 +20,16 @@ class Repo(BaseModel): model_config = ConfigDict(frozen=True) checkout: Path - github: str + github: InstanceOf[Annotated[ + RepositoryName, + PlainValidator(RepositoryName.from_wire, json_schema_input_type=str), + PlainSerializer(RepositoryName.to_wire), + ]] source: str = "src" + def holds(self, todo: Todo) -> bool: + return self.github == todo.repo + def contains(self, sha: str) -> bool: """Only a successful ancestry proof in this installed checkout permits closure.""" result = subprocess.run( @@ -43,4 +56,4 @@ def of(cls, checkout: Path) -> Repo: """Its GitHub name comes from the checkout's own origin remote.""" url = cls.git(checkout, "remote", "get-url", "origin").strip() github = url.removesuffix(".git").replace(":", "/").split("/")[-2:] - return cls(checkout=checkout.resolve(), github="/".join(github)) + return cls(checkout=checkout.resolve(), github=RepositoryName.from_wire("/".join(github))) diff --git a/src/agent_comms/repository_name.py b/src/agent_comms/repository_name.py new file mode 100644 index 0000000000..ee52f0fabb --- /dev/null +++ b/src/agent_comms/repository_name.py @@ -0,0 +1,54 @@ +"""A GitHub repository identity, with its existing owner/name wire form.""" + +from __future__ import annotations + +from dataclasses import dataclass + +from .field_codec import MalformedRecord, WireValue + + +@dataclass(frozen=True, slots=True, eq=False) +class RepositoryName(WireValue): + owner: str + name: str + + def __post_init__(self) -> None: + if len(f"{self.owner}/{self.name}") > 200 or any( + part in ("", ".", "..") or not all(char.isalnum() or char in "-_." for char in part) + for part in (self.owner, self.name) + ): + raise ValueError("Repo ID must have the exact owner/repo form.") + + @property + def key(self) -> tuple[str, str]: + return self.owner.casefold(), self.name.casefold() + + def __eq__(self, other: object) -> bool: + if not isinstance(other, RepositoryName): + return NotImplemented + return self.key == other.key + + def __hash__(self) -> int: + return hash(self.key) + + def named(self, name: str) -> bool: + return self.name.casefold() == name.casefold() + + def to_wire(self) -> str: + return "/".join(self.key) + + def __str__(self) -> str: + return self.to_wire() + + @classmethod + def from_wire(cls, data: object) -> RepositoryName: + if not isinstance(data, str): + raise MalformedRecord("Repo ID must be an owner/repo string.") + owner, separator, name = data.partition("/") + if not separator: + raise ValueError("Repo ID must have the exact owner/repo form.") + return cls(owner, name) + + @classmethod + def schema(cls) -> dict[str, object]: + return {"type": "string", "description": "GitHub repository owner/name"} diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 896e0be57f..5d0d9ac830 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -39,7 +39,8 @@ from .recorded_time import EventTime, RecordedEventTime, UnrecordedEventTime from .thread_identity import RecordedThreadIncarnation, ThreadIncarnation from .threads import Thread -from .typed_table import Column, Index, JsonStorage, TypedStore, TypedTable +from .typed_table import Column, Index, JsonStorage, TextStorage, TypedStore, TypedTable +from .repository_name import RepositoryName from .standing_rules import UndatedStandingRule from .storage_rules import StorageRule from .plan_phases import OrderPhase, UnplannedPhase @@ -371,21 +372,21 @@ class PullRequests: """Pull requests queried by immutable order identity, cached within one board read.""" def __init__(self, todos: Sequence[Todo] = ()) -> None: - self._read: dict[tuple[str, str, str], tuple[PullRequest, ...]] = {} - self._orders: dict[str, dict[str, CheckedProblemOrder]] = {} + self._read: dict[tuple[RepositoryName, str, str], tuple[PullRequest, ...]] = {} + self._orders: dict[RepositoryName, dict[str, CheckedProblemOrder]] = {} for todo in todos: if todo.holder: order = todo.order.checked() self._orders.setdefault(todo.repo, {})[order.id] = order - def listed(self, repo: str, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: + def listed(self, repo: RepositoryName, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: checked = order.checked() key = (repo, state, checked.id) if key not in self._read: self._read[key] = self.read(repo, state, checked) return tuple(request for request in self._read[key] if checked.id in request.orders) - def read(self, repo: str, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: + def read(self, repo: RepositoryName, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: checked = order.checked() orders = self._orders.setdefault(repo, {}) orders[checked.id] = checked @@ -1341,6 +1342,10 @@ def for_phase(self, phase: type[OrderPhase]) -> ProblemOrder: def accepted(self) -> bool: return False + def closed_by_merge(self, todo: Todo, repos: Sequence[Repo], storage: StorageRule, + pulls: PullRequests) -> tuple[Todo, ...]: + return () + def require_transfer(self, todo: Todo) -> None: raise ValueError("This problem cannot receive another order.") @@ -1474,6 +1479,13 @@ def copy_as[Kind: ProblemOrder](self, kind: type[Kind], **evidence) -> Kind: return kind(**{item.name: getattr(self, item.name) for item in fields(kind) if item.name not in evidence}, **evidence) + def closed_by_merge(self, todo: Todo, repos: Sequence[Repo], storage: StorageRule, + pulls: PullRequests) -> tuple[Todo, ...]: + return tuple(DoneTodoState.closed(todo, self.merged(merged.sha)) + for repo in repos + for merged in MergedProgress.found(ProgressSource.of(todo, storage, pulls)) + if repo.contains(merged.sha)) + def merged(self, sha: str) -> ProblemOrder: return self.copy_as(MergedProblemOrder, sha=sha) @@ -1492,7 +1504,7 @@ def pull_request_body(self, body: str) -> str: def open_pull_request(self, todo: Todo, checkout: Path, title: str, body: str) -> str: """The order creates its PR with the immutable identity every progress reader uses.""" created = subprocess.run( - ("gh", "pr", "create", "-R", todo.repo, "--title", title, "--body-file", "-"), + ("gh", "pr", "create", "-R", todo.repo.to_wire(), "--title", title, "--body-file", "-"), input=self.pull_request_body(body), cwd=checkout, capture_output=True, text=True, timeout=60) if created.returncode: @@ -2347,7 +2359,7 @@ class Todo(TypedTable): indexes = (Index(("creator", "creator_created", "state")), Index(("state",))) id: str = field(metadata={"sql": Column(primary_key=True)}) - repo: str + repo: RepositoryName = field(metadata={"sql": Column(storage=TextStorage)}) text: str creator: str creator_created: float @@ -2523,17 +2535,6 @@ def _participant(thread: Thread) -> ThreadIncarnation: return thread.incarnation -def _repo(value: str) -> str: - _text(value, "Repo ID", 200) - pieces = value.split("/") - if len(pieces) != 2 or any( - piece in {"", ".", ".."} or not all(char.isalnum() or char in "-_." for char in piece) - for piece in pieces - ): - raise TodoError("Repo ID must have the exact owner/repo form.") - return value.lower() - - @dataclass(frozen=True, slots=True) class TodoStore(TypedStore): """One SQLite row owns both todo state and its exclusive assignment. @@ -2642,8 +2643,8 @@ def get(self, todo_id: str) -> Todo: _text(todo_id, "Todo ID", 128) return self._one(self._select("id=?", (todo_id,)), todo_id) - def list(self, repo: str) -> tuple[Todo, ...]: - return self._select("repo=?", (_repo(repo),)) + def list(self, repo: RepositoryName) -> tuple[Todo, ...]: + return self._select("repo=?", (repo.to_wire(),)) def for_goal(self, owner: ThreadIncarnation, goal_id: str) -> tuple[Todo, ...]: return self._select( @@ -2662,15 +2663,11 @@ def stalled(self, storage: StorageRule) -> tuple[Todo, ...]: def reconcile_merges(self, commander: ThreadIncarnation, repos: Sequence[Repo], storage: StorageRule) -> None: """Read before locking; close only orders whose observed revision still holds.""" - checked = tuple((todo, order) for todo in self.held_for_commander(commander) - if isinstance(order := todo.order, CheckedProblemOrder)) - pulls = PullRequests(tuple(todo for todo, _ in checked)) - completed = tuple(DoneTodoState.closed(todo, order.merged(merged.sha)) - for todo, order in checked - for repo in repos if repo.github.lower() == todo.repo - for merged in MergedProgress.found( - ProgressSource.of(todo, storage, pulls)) - if repo.contains(merged.sha)) + held = self.held_for_commander(commander) + pulls = PullRequests(held) + completed = tuple(closed for todo in held + for closed in todo.order.closed_by_merge( + todo, tuple(repo for repo in repos if repo.holds(todo)), storage, pulls)) if not completed: return with self.writing() as db: @@ -2735,7 +2732,7 @@ def _open(self, where: str, parameters: tuple, def create( self, todo_id: str, - repo: str, + repo: RepositoryName, text: str, *, creator: Thread, @@ -2747,14 +2744,13 @@ def create( ) -> Todo: _text(todo_id, "Todo ID", 128) _text(text, "Todo text", 2000) - repo_id = _repo(repo) identity, assignee = _participant(creator), _thread(owner) deadline.require_deadline() goal.require_reference() order.require_writable() proposed = Todo( id=todo_id, - repo=repo_id, + repo=repo, text=text, creator=identity.name, creator_created=identity.created_at, diff --git a/src/agent_comms/work_branches.py b/src/agent_comms/work_branches.py index 7c997d1459..886d5b1d3a 100644 --- a/src/agent_comms/work_branches.py +++ b/src/agent_comms/work_branches.py @@ -186,8 +186,8 @@ def _run(self, *command: str, input: str = "") -> str: @property def repository(self) -> tuple[str, str]: - owner, name = Repo.of(self.checkout).github.split("/") - return owner, name + repository = Repo.of(self.checkout).github + return repository.owner, repository.name def _pages(self, query: str, answer: type[ForgeQuery], **variables: str): pages: list[tuple[str, ...]] = [()] From 9299ea44779d0415cbfb59fb3852d9f0b5f505c0 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:37:23 -0400 Subject: [PATCH 02/10] P271: retain batched PR queries through each order's capability An unordered held finding has no PR id; let the same order family expose its actual PR queries instead of making PullRequests probe all kinds. Cover mixed-case install reconciliation, config/wire/SQL identity and findings. Src-Budget: 84 --- src/agent_comms/todos.py | 10 +++++-- tests/test_commander_problem_orders.py | 3 ++- tests/test_repository_name.py | 37 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 tests/test_repository_name.py diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 5d0d9ac830..acb35e6625 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -376,8 +376,8 @@ def __init__(self, todos: Sequence[Todo] = ()) -> None: self._orders: dict[RepositoryName, dict[str, CheckedProblemOrder]] = {} for todo in todos: if todo.holder: - order = todo.order.checked() - self._orders.setdefault(todo.repo, {})[order.id] = order + for order in todo.order.pull_queries(): + self._orders.setdefault(todo.repo, {})[order.id] = order def listed(self, repo: RepositoryName, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: checked = order.checked() @@ -1342,6 +1342,9 @@ def for_phase(self, phase: type[OrderPhase]) -> ProblemOrder: def accepted(self) -> bool: return False + def pull_queries(self) -> tuple[CheckedProblemOrder, ...]: + return () + def closed_by_merge(self, todo: Todo, repos: Sequence[Repo], storage: StorageRule, pulls: PullRequests) -> tuple[Todo, ...]: return () @@ -1479,6 +1482,9 @@ def copy_as[Kind: ProblemOrder](self, kind: type[Kind], **evidence) -> Kind: return kind(**{item.name: getattr(self, item.name) for item in fields(kind) if item.name not in evidence}, **evidence) + def pull_queries(self) -> tuple[CheckedProblemOrder, ...]: + return (self,) + def closed_by_merge(self, todo: Todo, repos: Sequence[Repo], storage: StorageRule, pulls: PullRequests) -> tuple[Todo, ...]: return tuple(DoneTodoState.closed(todo, self.merged(merged.sha)) diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 2bc690a82d..00d6b872ef 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -31,6 +31,7 @@ from agent_comms.goals import CountCheckResult, OrderCheck, ProblemGoalOrder from agent_comms.progress_claims import WorkingClaim from agent_comms.recorded_time import RecordedEventTime, UnrecordedEventTime +from agent_comms.repository_name import RepositoryName from agent_comms.threads import Thread from agent_comms.tool_caller import ThreadCaller from agent_comms.tools import (CommsCorrectWordingTool, CommsSendTool, CommsFireTool, CommsOrderCheckTool, CommsOrderDoneTool, CommsOrderTool, @@ -56,7 +57,7 @@ def create(commander, problem, command='printf 1', expected=1, deadline=1000, ow CommsTodoCreateTool(owner=owner, order=CreateProblemGoalAction( problem=problem, order=order(comms.root, 'order-' + problem, command, expected), request='request', deadline=RecordedEventTime(deadline), commander='lead', - repo='OpenHCSDev/agent-comms', text='Fix ' + problem, + repo=RepositoryName.from_wire('OpenHCSDev/agent-comms'), text='Fix ' + problem, )).apply(comms, caller) return comms.goals.todos.get(problem) diff --git a/tests/test_repository_name.py b/tests/test_repository_name.py new file mode 100644 index 0000000000..59caa96955 --- /dev/null +++ b/tests/test_repository_name.py @@ -0,0 +1,37 @@ +"""Repository identities cross the wire and SQL once, without case-sensitive aliases.""" +from dataclasses import replace + +from agent_comms.commander.config import Repo +from agent_comms.field_codec import FieldCodec +from agent_comms.repository_name import RepositoryName +from agent_comms.todos import PullRequests, UnorderedProblemOrder +from test_commander_problem_orders import create + + +def test_repository_identity_hash_and_existing_wire_form(): + first = RepositoryName("OpenHCSDev", "Agent-Comms") + second = RepositoryName.from_wire("OPENHCSDEV/agent-comms") + assert first == second + assert len({first, second}) == 1 + assert FieldCodec.encode(first) == "openhcsdev/agent-comms" + assert FieldCodec.decode(RepositoryName, FieldCodec.encode(first)) == first + + +def test_repository_config_json_and_saved_todo_have_the_same_identity(commander): + comms, _ = commander + todo = create(commander, "P1") + repo = Repo.model_validate_json('{"checkout":"/unused", "github":"OPENHCSDEV/AGENT-COMMS"}') + assert repo.holds(todo) + assert Repo.model_validate_json(repo.model_dump_json()) == repo + assert comms.goals.todos.list(repo.github) == (todo,) + restored = comms.goals.todos.get(todo.id) + assert restored.repo == repo.github + assert FieldCodec.decode(type(todo), FieldCodec.encode(todo)) == restored + assert not Repo(checkout=repo.checkout, github=RepositoryName("another", "repo")).holds(todo) + + +def test_order_family_owns_which_kinds_a_merge_closes(commander): + comms, _ = commander + todo = create(commander, "P1") + finding = replace(todo, order=UnorderedProblemOrder()) + assert finding.order.closed_by_merge(finding, (), comms.registry.storage_rule, PullRequests((finding,))) == () From 29e0e9b80b33c4389a59855ef2cbb3b3044d4737 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:39:19 -0400 Subject: [PATCH 03/10] P271: own repository syntax in its value declaration Validate owner/name components through the declared grammar rather than redispatching strings. Keep typed API signatures within the existing lint limits. Src-Budget: 6 --- src/agent_comms/repository_name.py | 8 +++++--- src/agent_comms/todos.py | 24 ++++++++++++++---------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/agent_comms/repository_name.py b/src/agent_comms/repository_name.py index ee52f0fabb..10428a3785 100644 --- a/src/agent_comms/repository_name.py +++ b/src/agent_comms/repository_name.py @@ -2,7 +2,9 @@ from __future__ import annotations +import re from dataclasses import dataclass +from typing import ClassVar from .field_codec import MalformedRecord, WireValue @@ -11,11 +13,11 @@ class RepositoryName(WireValue): owner: str name: str + _part: ClassVar[re.Pattern[str]] = re.compile(r"(?!\.{1,2}$)[\w.-]+\Z") def __post_init__(self) -> None: - if len(f"{self.owner}/{self.name}") > 200 or any( - part in ("", ".", "..") or not all(char.isalnum() or char in "-_." for char in part) - for part in (self.owner, self.name) + if len(f"{self.owner}/{self.name}") > 200 or not all( + self._part.fullmatch(part) for part in (self.owner, self.name) ): raise ValueError("Repo ID must have the exact owner/repo form.") diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index acb35e6625..65793e80b2 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -7,14 +7,14 @@ from __future__ import annotations +import asyncio import calendar import json import re -import subprocess import sqlite3 +import subprocess import sys import time -import asyncio from abc import abstractmethod from collections.abc import Iterator, Sequence from contextlib import contextmanager @@ -32,18 +32,18 @@ from .declared_family import DeclaredFamily from .errors import ToolRefusedError from .field_codec import ( - FieldCodec, GitCommitText, MalformedRecord, NullMember, RecordMember, SubclassUnion, WireValue, + FieldCodec, GitCommitText, MalformedRecord, NullMember, RecordMember, SubclassUnion, + WireValue, boundary_decoder, ) -from .channel_targets import CommandTarget from .goals import CheckedCommit, CountCheckResult, OrderCheck, ProblemGoalOrder +from .plan_phases import OrderPhase, UnplannedPhase from .recorded_time import EventTime, RecordedEventTime, UnrecordedEventTime -from .thread_identity import RecordedThreadIncarnation, ThreadIncarnation -from .threads import Thread -from .typed_table import Column, Index, JsonStorage, TextStorage, TypedStore, TypedTable from .repository_name import RepositoryName from .standing_rules import UndatedStandingRule from .storage_rules import StorageRule -from .plan_phases import OrderPhase, UnplannedPhase +from .thread_identity import RecordedThreadIncarnation, ThreadIncarnation +from .threads import Thread +from .typed_table import Column, Index, JsonStorage, TextStorage, TypedStore, TypedTable from .work_branches import WorkBranchError, WorkBranches if TYPE_CHECKING: @@ -379,14 +379,18 @@ def __init__(self, todos: Sequence[Todo] = ()) -> None: for order in todo.order.pull_queries(): self._orders.setdefault(todo.repo, {})[order.id] = order - def listed(self, repo: RepositoryName, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: + def listed( + self, repo: RepositoryName, state: str, order: ProblemOrder, + ) -> tuple[PullRequest, ...]: checked = order.checked() key = (repo, state, checked.id) if key not in self._read: self._read[key] = self.read(repo, state, checked) return tuple(request for request in self._read[key] if checked.id in request.orders) - def read(self, repo: RepositoryName, state: str, order: ProblemOrder) -> tuple[PullRequest, ...]: + def read( + self, repo: RepositoryName, state: str, order: ProblemOrder, + ) -> tuple[PullRequest, ...]: checked = order.checked() orders = self._orders.setdefault(repo, {}) orders[checked.id] = checked From cc7f0a609776458d30263a710fab8be9513dba8e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:41:17 -0400 Subject: [PATCH 04/10] P271: use the repository wire value at GitHub and display boundaries Delete the redundant key projection and implicit string conversion. Equality, hashing and transport now consume the value's one case-normalized wire form. --- src/agent_comms/commander/standing.py | 3 ++- src/agent_comms/repository_name.py | 13 +++---------- src/agent_comms/todos.py | 5 +++-- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index e8b602127f..7359efa6c4 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -263,6 +263,7 @@ def began(self) -> float: def line(self) -> str: (a, d), (ta, td) = self.src, self.tests return ( - f"GROWTH {self.repo} {self.sha[:9]} src +{a}/-{d} (net {a - d:+}) tests +{ta}/-{td}: " + f"GROWTH {self.repo.to_wire()} {self.sha[:9]} src +{a}/-{d} (net {a - d:+}) " + f"tests +{ta}/-{td}: " f"{self.subject[:90]} -> justify or send back the owner with a deletion order" ) diff --git a/src/agent_comms/repository_name.py b/src/agent_comms/repository_name.py index 10428a3785..2d2afc6b7c 100644 --- a/src/agent_comms/repository_name.py +++ b/src/agent_comms/repository_name.py @@ -21,26 +21,19 @@ def __post_init__(self) -> None: ): raise ValueError("Repo ID must have the exact owner/repo form.") - @property - def key(self) -> tuple[str, str]: - return self.owner.casefold(), self.name.casefold() - def __eq__(self, other: object) -> bool: if not isinstance(other, RepositoryName): return NotImplemented - return self.key == other.key + return self.to_wire() == other.to_wire() def __hash__(self) -> int: - return hash(self.key) + return hash(self.to_wire()) def named(self, name: str) -> bool: return self.name.casefold() == name.casefold() def to_wire(self) -> str: - return "/".join(self.key) - - def __str__(self) -> str: - return self.to_wire() + return f"{self.owner.casefold()}/{self.name.casefold()}" @classmethod def from_wire(cls, data: object) -> RepositoryName: diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 65793e80b2..d83297627d 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -397,7 +397,7 @@ def read( aliases = {f"o{index}": candidate for index, candidate in enumerate(orders.values())} selections = [] for alias, candidate in aliases.items(): - search = json.dumps(f'repo:{repo} is:pr is:{state} in:body ' + search = json.dumps(f'repo:{repo.to_wire()} is:pr is:{state} in:body ' f'"{candidate.pull_request_reference}" sort:updated-desc') selections.append( f'{alias}: search(query: {search}, type: ISSUE, first: 100) {{ nodes {{ ' @@ -411,7 +411,8 @@ def read( f'. + {{kind: "{MergedListedPullRequest.declared_name}"}} end))'), capture_output=True, text=True, timeout=60) if listed.returncode: - raise WorkBranchError(listed.stderr.strip() or f"GitHub order lookup in {repo} failed.") + raise WorkBranchError( + listed.stderr.strip() or f"GitHub order lookup in {repo.to_wire()} failed.") rows = FieldCodec.decode(dict[str, tuple[ListedPullRequest, ...]], FieldCodec.parse(listed.stdout)) for alias, candidate in aliases.items(): From 9652c583b8824038701a1c4f1626707129d7b2fc Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:42:54 -0400 Subject: [PATCH 05/10] P271: let repository construction own all name validation Delete the duplicate separator refusal: missing, empty and extra separators already fail the value's component grammar. Keep GitHub error text at its existing boundary. --- src/agent_comms/repository_name.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/agent_comms/repository_name.py b/src/agent_comms/repository_name.py index 2d2afc6b7c..a74eccf570 100644 --- a/src/agent_comms/repository_name.py +++ b/src/agent_comms/repository_name.py @@ -39,9 +39,7 @@ def to_wire(self) -> str: def from_wire(cls, data: object) -> RepositoryName: if not isinstance(data, str): raise MalformedRecord("Repo ID must be an owner/repo string.") - owner, separator, name = data.partition("/") - if not separator: - raise ValueError("Repo ID must have the exact owner/repo form.") + owner, _, name = data.partition("/") return cls(owner, name) @classmethod From ddc5067970051988ce2051a966a263b8029f6eff Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:50:51 -0400 Subject: [PATCH 06/10] P271: migrate direct Python repository callers to the typed value --- tests/test_commander_problem_orders.py | 8 ++++---- tests/test_todos.py | 23 ++++++++++++----------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 00d6b872ef..cba4207c3d 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -695,7 +695,7 @@ def test_a_deletion_order_must_check_the_imports_of_every_repo_that_imports_the_ (toad / 'src').mkdir(parents=True) (toad / 'src' / 'app.py').write_text('from agent_comms.acp_ingress import Ingress\n') script = script.model_copy(update={'repos': ( - Repo(checkout=toad, github='OpenHCSDev/toad'),)}) + Repo(checkout=toad, github=RepositoryName.from_wire('OpenHCSDev/toad')),)}) (repo / 'acp_ingress.py').write_text('class Ingress: ...\n') edit = 'Delete src/agent_comms/acp_ingress.py.' with pytest.raises(SystemExit, match=rf"\['{toad}'\] import"): @@ -1232,7 +1232,7 @@ def test_dropped_problem_links_through_the_unordered_lifecycle(commander): problem=pid, order=replace(order(comms.root, 'order-' + pid), at=RecordedEventTime(at), first_call_deadline=RecordedEventTime(at + 120), finding=finding), request='request', deadline=RecordedEventTime(1000), commander='lead', - repo='OpenHCSDev/agent-comms', text='Fix ' + pid)).apply(comms, caller) + repo=RepositoryName.from_wire('OpenHCSDev/agent-comms'), text='Fix ' + pid)).apply(comms, caller) old_message = order_message(commander, 'P1', 'The earlier counter order') new_message = order_message(commander, 'P2', 'The newer counter order') lead = comms.registry.require('lead').incarnation @@ -1487,7 +1487,7 @@ def test_close_quotes_the_works_line(commander): run = ApplicationOrderRun('The board shows the problem closed after the commander reopens the session') CommsTodoCreateTool(owner='worker', order=CreateProblemGoalAction( problem='P1', order=replace(order(comms.root, 'order-P1'), run=run), request='request', - deadline=RecordedEventTime(1000), commander='lead', repo='OpenHCSDev/agent-comms', text='Fix P1', + deadline=RecordedEventTime(1000), commander='lead', repo=RepositoryName.from_wire('OpenHCSDev/agent-comms'), text='Fix P1', )).apply(comms, caller) CommsOrderDoneTool(problem='P1', order_id=comms.goals.todos.get('P1').order.id, delivery=OperationDelivery(), claims=working(comms)).apply(comms, caller) artifact = comms.root / 'installed-evidence' @@ -1518,7 +1518,7 @@ def test_delivery_that_renames_the_decision_is_refused(commander, tmp_path): CommsTodoCreateTool(owner='worker', order=CreateProblemGoalAction( problem='P520', order=replace(order(comms.root, 'order-P520'), deletes=deletes), request='request', deadline=RecordedEventTime(1000), commander='lead', - repo='OpenHCSDev/agent-comms', text='Delete the is_set decision', + repo=RepositoryName.from_wire('OpenHCSDev/agent-comms'), text='Delete the is_set decision', )).apply(comms, caller) pending = comms.goals.todos.get('P520') assert 'Deletes: is_set absence decision at' in pending.order.instructions(pending) diff --git a/tests/test_todos.py b/tests/test_todos.py index 13c9d61363..f6a5278c73 100644 --- a/tests/test_todos.py +++ b/tests/test_todos.py @@ -15,6 +15,7 @@ from agent_comms.field_codec import FieldCodec from agent_comms.goals import OrderCheck from agent_comms.recorded_time import RecordedEventTime +from agent_comms.repository_name import RepositoryName from agent_comms.registration import Registration from agent_comms.threads import Thread from agent_comms.todos import ( @@ -57,25 +58,25 @@ def test_create_is_idempotent_and_goal_reference_is_historical(tmp_path: Path) - lead = thread("lead", tmp_path, 1.0) goal = GoalRef("lead", 1.0, "goal-1") original = store.create( - "todo-1", "OpenHCSDev/agent-comms", "Fix the sidebar", creator=lead, goal=goal, + "todo-1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "Fix the sidebar", creator=lead, goal=goal, owner=lead, generation="initial", deadline=RecordedEventTime(1000.0), order=order(tmp_path), ) - assert original.repo == "openhcsdev/agent-comms" + assert original.repo == RepositoryName.from_wire("OPENHCSDEV/agent-comms") assert original.revision == 1 assert original.assignment == Assignment("lead", 1.0, "lead", 1.0, "initial") assert ( store.create( - "todo-1", "openhcsdev/agent-comms", "Fix the sidebar", creator=lead, goal=goal, + "todo-1", RepositoryName.from_wire("openhcsdev/agent-comms"), "Fix the sidebar", creator=lead, goal=goal, owner=lead, generation="initial", deadline=RecordedEventTime(1000.0), order=order(tmp_path), ) == original ) with pytest.raises(TodoConflict): store.create( - "todo-1", "openhcsdev/agent-comms", "A different task", creator=lead, goal=goal, + "todo-1", RepositoryName.from_wire("openhcsdev/agent-comms"), "A different task", creator=lead, goal=goal, owner=lead, generation="initial", deadline=RecordedEventTime(1000.0), order=order(tmp_path), ) - assert TodoStore(tmp_path).list("openhcsdev/agent-comms") == (original,) + assert TodoStore(tmp_path).list(RepositoryName.from_wire("openhcsdev/agent-comms")) == (original,) newer = thread("lead", tmp_path, 2.0) assert store.get(original.id).goal == goal with pytest.raises(TodoConflict): @@ -86,7 +87,7 @@ def test_create_is_idempotent_and_goal_reference_is_historical(tmp_path: Path) - def test_commander_reads_decode_only_open_todos(tmp_path, monkeypatch): store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) - first = store.create("P1", "org/repo", "Open order", creator=lead, owner=lead, + first = store.create("P1", RepositoryName.from_wire("org/repo"), "Open order", creator=lead, owner=lead, generation="one", goal=GoalRef("lead", 1.0, "request"), deadline=RecordedEventTime(1000), order=order(tmp_path)) second = replace(first, id="P2", state=BlockedTodoState) @@ -143,7 +144,7 @@ def test_two_processes_assign_one_todo_before_any_worker_is_started(tmp_path: Pa store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) store.create( - "todo-1", "OpenHCSDev/agent-comms", "One implementation", creator=lead, goal=GoalRef("lead", 1.0, "goal-1"), + "todo-1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "One implementation", creator=lead, goal=GoalRef("lead", 1.0, "goal-1"), owner=lead, generation="initial", deadline=RecordedEventTime(1000.0), order=order(tmp_path), ) with Manager() as manager, ProcessPoolExecutor(max_workers=2) as pool: @@ -159,22 +160,22 @@ def test_two_processes_assign_one_todo_before_any_worker_is_started(tmp_path: Pa persisted = TodoStore(tmp_path).get("todo-1") assert persisted.assignment.owner == observed[0][1] assert persisted.revision == 2 - assert len(store.list("OpenHCSDev/agent-comms")) == 1 + assert len(store.list(RepositoryName.from_wire("OpenHCSDev/agent-comms"))) == 1 def test_validation_rejects_ambiguous_identity_and_bad_state(tmp_path: Path) -> None: store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) for index, repo in enumerate(("org/repo/branch", "../repo", "org/", "org/repo name")): - with pytest.raises(TodoError): + with pytest.raises(ValueError): store.create( - f"invalid-{index}", repo, "task", creator=lead, goal=GoalRef("lead", 1.0, "goal-1"), + f"invalid-{index}", RepositoryName.from_wire(repo), "task", creator=lead, goal=GoalRef("lead", 1.0, "goal-1"), owner=lead, generation="initial", deadline=RecordedEventTime(1000.0), order=order(tmp_path), ) with pytest.raises(TodoError): GoalRef("lead", float("nan"), "goal-1") store.create( - "todo-1", "org/repo", "task", creator=lead, goal=GoalRef("lead", 1.0, "goal-1"), + "todo-1", RepositoryName.from_wire("org/repo"), "task", creator=lead, goal=GoalRef("lead", 1.0, "goal-1"), owner=lead, generation="initial", deadline=RecordedEventTime(1000.0), order=order(tmp_path), ) with pytest.raises(ValueError): From 77843e3e5b00222b583f50555efdde9111ded9d3 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 19:52:56 -0400 Subject: [PATCH 07/10] P271: keep repository identity independent of its wire rendering Equality and hashing derive the case-normalized component pair, not rendered text. Keep the query-index fixture from deleting the unrelated hunt index. Src-Budget: 3 --- src/agent_comms/repository_name.py | 10 +++++++--- tests/test_repository_name.py | 8 ++++++++ tests/test_todos.py | 3 ++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/agent_comms/repository_name.py b/src/agent_comms/repository_name.py index a74eccf570..875e2dbdb0 100644 --- a/src/agent_comms/repository_name.py +++ b/src/agent_comms/repository_name.py @@ -21,19 +21,23 @@ def __post_init__(self) -> None: ): raise ValueError("Repo ID must have the exact owner/repo form.") + @property + def key(self) -> tuple[str, str]: + return self.owner.casefold(), self.name.casefold() + def __eq__(self, other: object) -> bool: if not isinstance(other, RepositoryName): return NotImplemented - return self.to_wire() == other.to_wire() + return self.key == other.key def __hash__(self) -> int: - return hash(self.to_wire()) + return hash(self.key) def named(self, name: str) -> bool: return self.name.casefold() == name.casefold() def to_wire(self) -> str: - return f"{self.owner.casefold()}/{self.name.casefold()}" + return "/".join(self.key) @classmethod def from_wire(cls, data: object) -> RepositoryName: diff --git a/tests/test_repository_name.py b/tests/test_repository_name.py index 59caa96955..bba6529ae8 100644 --- a/tests/test_repository_name.py +++ b/tests/test_repository_name.py @@ -17,6 +17,14 @@ def test_repository_identity_hash_and_existing_wire_form(): assert FieldCodec.decode(RepositoryName, FieldCodec.encode(first)) == first +def test_identity_uses_owned_components_not_rendered_text(monkeypatch): + first = RepositoryName("org", "first") + second = RepositoryName("org", "second") + monkeypatch.setattr(RepositoryName, "to_wire", lambda self: "display") + assert first != second + assert first == RepositoryName("ORG", "FIRST") + + def test_repository_config_json_and_saved_todo_have_the_same_identity(commander): comms, _ = commander todo = create(commander, "P1") diff --git a/tests/test_todos.py b/tests/test_todos.py index f6a5278c73..8c3adba9b5 100644 --- a/tests/test_todos.py +++ b/tests/test_todos.py @@ -100,7 +100,8 @@ def test_commander_reads_decode_only_open_todos(tmp_path, monkeypatch): assignment=None, order=delivered).insert(db) with closing(CoordinationStore.writer(store.path)) as db: indexes = tuple(name for name, in db.execute( - "SELECT name FROM sqlite_master WHERE type='index' AND name NOT LIKE 'sqlite_%'")) + "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='todo' " + "AND name NOT LIKE 'sqlite_%'")) for name in indexes: db.execute(f'DROP INDEX "{name}"') db.commit() From 9f168c20bff0aafea5a45defeca4b5ae479daac9 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 20:02:14 -0400 Subject: [PATCH 08/10] P271: validate the canonical repository value before persistence Case folding must not produce an overlength or undecodable stored name. Also repair the goal-action import finding exposed by current main. --- src/agent_comms/goal_actions.py | 2 +- src/agent_comms/repo.py | 12 +++++++----- src/agent_comms/repository_name.py | 4 +--- src/agent_comms/todos.py | 11 ++++++++--- tests/test_repository_name.py | 8 ++++++++ 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/agent_comms/goal_actions.py b/src/agent_comms/goal_actions.py index 2088e9735e..1f9b289cdb 100644 --- a/src/agent_comms/goal_actions.py +++ b/src/agent_comms/goal_actions.py @@ -37,8 +37,8 @@ UnorderedGoalOrder, ) from .progress_claims import ProgressClaim -from .repository_name import RepositoryName from .recorded_time import EventTime +from .repository_name import RepositoryName from .thread_identity import ThreadIncarnation from .threads import Thread from .todos import GoalRef, PendingProblemOrder, SendBackVerdict, Todo, TodoOperation diff --git a/src/agent_comms/repo.py b/src/agent_comms/repo.py index f080cfaf21..1ac3e6205a 100644 --- a/src/agent_comms/repo.py +++ b/src/agent_comms/repo.py @@ -20,11 +20,13 @@ class Repo(BaseModel): model_config = ConfigDict(frozen=True) checkout: Path - github: InstanceOf[Annotated[ - RepositoryName, - PlainValidator(RepositoryName.from_wire, json_schema_input_type=str), - PlainSerializer(RepositoryName.to_wire), - ]] + github: InstanceOf[ + Annotated[ + RepositoryName, + PlainValidator(RepositoryName.from_wire, json_schema_input_type=str), + PlainSerializer(RepositoryName.to_wire), + ] + ] source: str = "src" def holds(self, todo: Todo) -> bool: diff --git a/src/agent_comms/repository_name.py b/src/agent_comms/repository_name.py index 875e2dbdb0..327e7da59a 100644 --- a/src/agent_comms/repository_name.py +++ b/src/agent_comms/repository_name.py @@ -16,9 +16,7 @@ class RepositoryName(WireValue): _part: ClassVar[re.Pattern[str]] = re.compile(r"(?!\.{1,2}$)[\w.-]+\Z") def __post_init__(self) -> None: - if len(f"{self.owner}/{self.name}") > 200 or not all( - self._part.fullmatch(part) for part in (self.owner, self.name) - ): + if len(self.to_wire()) > 200 or not all(self._part.fullmatch(part) for part in self.key): raise ValueError("Repo ID must have the exact owner/repo form.") @property diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index d83297627d..2156009158 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -22,7 +22,7 @@ from functools import cache, cached_property from importlib.resources import files from pathlib import Path -from typing import TYPE_CHECKING, ClassVar, Final, Self +from typing import TYPE_CHECKING, ClassVar, Self from uuid import uuid4 from . import skills @@ -32,8 +32,13 @@ from .declared_family import DeclaredFamily from .errors import ToolRefusedError from .field_codec import ( - FieldCodec, GitCommitText, MalformedRecord, NullMember, RecordMember, SubclassUnion, - WireValue, boundary_decoder, + FieldCodec, + GitCommitText, + MalformedRecord, + NullMember, + RecordMember, + SubclassUnion, + WireValue, ) from .goals import CheckedCommit, CountCheckResult, OrderCheck, ProblemGoalOrder from .plan_phases import OrderPhase, UnplannedPhase diff --git a/tests/test_repository_name.py b/tests/test_repository_name.py index bba6529ae8..7485b4805d 100644 --- a/tests/test_repository_name.py +++ b/tests/test_repository_name.py @@ -1,6 +1,8 @@ """Repository identities cross the wire and SQL once, without case-sensitive aliases.""" from dataclasses import replace +import pytest + from agent_comms.commander.config import Repo from agent_comms.field_codec import FieldCodec from agent_comms.repository_name import RepositoryName @@ -17,6 +19,12 @@ def test_repository_identity_hash_and_existing_wire_form(): assert FieldCodec.decode(RepositoryName, FieldCodec.encode(first)) == first +@pytest.mark.parametrize("owner", ["ß" * 100, "İ"]) +def test_repository_rejects_names_whose_canonical_wire_cannot_be_decoded(owner): + with pytest.raises(ValueError): + RepositoryName(owner, "repo") + + def test_identity_uses_owned_components_not_rendered_text(monkeypatch): first = RepositoryName("org", "first") second = RepositoryName("org", "second") From b522b96ccd7eb0d3888076e33dbdf5c2e6b6102f Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 20:46:01 -0400 Subject: [PATCH 09/10] Let rotation read the commander reconciled order view The board already used Commander.orders, but rotate bypassed it through held_orders and could never close a newly installed merge on its next tick. Both now consume the existing Commander owner before selecting outstanding work. --- src/agent_comms/commander/order.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 33e6ee33de..468e4163d7 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -261,7 +261,7 @@ def read_check(self, line: str) -> OrderCheck: return replace(check, directory=str(directory)) def held_orders(self) -> tuple[Todo, ...]: - return self.comms.goals.todos.held_for_commander(self.incarnation) + return tuple(todo for todo in self.todos() if todo.holder) def held_by(self, unit: str) -> tuple[Todo, ...]: return self.comms.goals.todos.held_by(self.incarnation, unit) From 2fd1c6bac0529cee4578901934e7883c148a9242 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 23:22:52 -0400 Subject: [PATCH 10/10] Keep the current verdict audience target owner after rebasing --- src/agent_comms/todos.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 2156009158..9129555fb9 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -27,6 +27,7 @@ from . import skills from .change_report import FileLine +from .channel_targets import CommandTarget from .child_process import BoundedRun, CheckPrinted, ChildResult, ExitedOutcome, NoIntegerPrinted from .command import Command from .declared_family import DeclaredFamily