From 30474f72284bdfe13ed1fe754cabee094c5ed5c1 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Wed, 21 Oct 2026 19:38:02 -0400 Subject: [PATCH 1/3] P451: let order states own check bases and scopes build deliveries --- src/agent_comms/commander/order.py | 63 +++++------------ src/agent_comms/goals.py | 9 ++- src/agent_comms/todos.py | 109 +++++++++++++++++++++++++---- tests/test_order_check_writes.py | 3 +- tests/test_order_completion.py | 94 +++++++++++++++++++++++++ 5 files changed, 213 insertions(+), 65 deletions(-) create mode 100644 tests/test_order_completion.py diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 01a31afde2..7d5178f173 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -54,7 +54,6 @@ from agent_comms.native_tool_call import SelectedToolDenied from agent_comms.plan_phases import OrderPhase from agent_comms.problem_number import ProblemNumber -from agent_comms.progress_claims import MergedClaim, WorkingClaim from agent_comms.recorded_time import RecordedEventTime from agent_comms.repo import Repo from agent_comms.standing_rules import HuntDue, OwedAnswer @@ -63,14 +62,11 @@ from agent_comms.todos import ( BluntText, CleanHunt, - CommitDelivery, DirectiveText, - FileDeliveryEvidence, LocatedProblemFinding, MeasuredReview, - NoDeliveryEvidence, NoSuccessor, - OperationDelivery, + OrderDelivery, OrderedHunt, OrderMessage, OrderProgress, @@ -227,12 +223,6 @@ def problems_found(self, clean: bool) -> list[str]: ) # where a mistake was caught; later is worse -def revision(repo: str, ref: str) -> str: - """Resolve a supplied Git revision once at the command boundary.""" - return subprocess.check_output( - ['git', '-C', repo, 'rev-parse', '--verify', f'{ref}^{{commit}}'], text=True).strip() - - class CommanderContext(CommanderConfig): """The selected root's bus and canonical reads, shared by command families.""" @@ -470,12 +460,14 @@ def send(self, unit, document: OrderFile, clean=False, validate() except MalformedRecord as error: problems.append(f'{label}: {error}; fix: name an existing source location or directory') - # Send and correction ask the check's owner the same question on clean main. + # The new order owns the source against which its check must fail. try: with MalformedRecord.refusing('order.check'): order.require_writable() if Path(check.directory).is_dir(): - check.require_acceptance(self.worktrees) + Repo.git(Path(check.directory), 'fetch', '-q', 'origin', 'main') + base = order.check_base(Path(check.directory)) + check.require_acceptance(base, self.worktrees) except MalformedRecord as error: problems.append(f'order.check: {error}; fix: supply a focused measurable command') references = list(dict.fromkeys(( @@ -558,13 +550,14 @@ def send(self, unit, document: OrderFile, clean=False, evidence=(order.owner,)) def correct_check(self, pid: str, line: str, why: str) -> None: - """Replace a wrong check of a pending or claimed order by the rule send uses; Core refuses - any other order or a closed problem (CorrectCheckTodo.apply); done checks the SHA.""" + """Correct the current check against the source preceding this order's delivery.""" todo = self.comms.goals.todos.get(pid) check = self.read_check(line) if len(why.split()) < 8: sys.exit('Refused: --why says what was wrong with the recorded check (8+ words)') - check.require_acceptance(self.worktrees) + repo = self.repo_directory(check.directory) + Repo.git(repo, 'fetch', '-q', 'origin', 'main') + check.require_acceptance(todo.order.check_base(repo), self.worktrees) self.call(CommsOrderCheckTool(problem=pid, check=check, why=why)) print(f'{pid}: order {todo.order.id} now checks `{check.command}`') @@ -645,23 +638,13 @@ def posted(self, unit: str, todo: Todo, strike: StrikeTodo, self.on_record(line, 'Strike', VerdictAudience.of(unit, todo.goal), answers) print(f'{unit}: {n} strike(s) on {todo.id}' + (f'. Reassign {todo.id} now and freeze its branch.' if n >= 2 else '')) - def done(self, pid, sha, screenshot='', seen=''): - """Core runs the delivery's check at its checked commit and strikes a late delivered commit - in the same revision; this posts the strikes Core recorded.""" + def done(self, pid: str, delivery: OrderDelivery) -> None: + """Storage is checked here; the delivery owns its evidence and claims.""" todo = self.comms.goals.todos.get(pid) todo.state.require_open(todo) todo.order.checked().require_delivery(todo.order.id) unit = todo.held().owner - repo = str(self.repo_directory(todo.order.check.directory)) - operation = sha == 'op' # an operation on live state, not a commit: the check runs now and now is delivery - if not operation: - diff = subprocess.run(['git', '-C', repo, 'show', '--first-parent', - '-m', '--format=', sha], capture_output=True, text=True).stdout - changed = re.findall(r'^\+\+\+ b/(\S+\.py)$', diff, re.M) - after = [subprocess.run(['git', '-C', repo, 'show', f'{sha}:{f}'], - capture_output=True, text=True).stdout for f in changed] - if added := forwarders_added(diff, after): - sys.exit(f'Refused: {sha} adds forwarders {added[:5]}; slop never counts as delivery. Order the hierarchy fix.') + claims = delivery.done_claims(todo.order.checked()) thread = self.comms.registry.require(unit) storage = UnitStorage.read(self.comms.registry.storage_rule, thread) for checkout in storage.rule.problem_checkouts(unit, pid): @@ -672,19 +655,9 @@ def done(self, pid, sha, screenshot='', seen=''): storage.require_quota() except SelectedToolDenied as quota: sys.exit(f'Refused: {unit}: {quota}; storage rule: it cleans up before the order counts as done.') - todo.order.run.require_artifact(seen or screenshot) - evidence = FileDeliveryEvidence(seen or screenshot) if seen or screenshot else NoDeliveryEvidence() - if operation: - delivery = OperationDelivery(evidence=evidence) - claim = WorkingClaim(evidence.artifact()) - else: - delivered_commit = revision(repo, sha) - delivery = CommitDelivery(commit=revision(repo, 'HEAD'), - delivered_commit=delivered_commit, evidence=evidence) - claim = MergedClaim(delivery.delivered) self.call(CommsOrderDoneTool(problem=pid, - order_id=todo.order.id, delivery=delivery, claims=(claim,))) - print(f'{unit}: order {todo.order.id} delivered as {delivery.delivered if not operation else sha}') + order_id=todo.order.id, delivery=delivery, claims=claims)) + print(f'{unit}: order {todo.order.id} delivered as {delivery.work}') def hunt(self, where, why, sent): """Record the commander's source read, whether or not it became an order.""" @@ -966,9 +939,9 @@ class DoneVerb(OrderVerb): sha: Annotated[ str, Argument( - flags=("--sha",), options={"help": "commit in the recorded checkout, or op"} + flags=("--sha",), options={"help": "for a fix: commit in the recorded checkout, or op"} ), - ] + ] = "" screenshot: Annotated[ str, Argument( @@ -984,7 +957,9 @@ class DoneVerb(OrderVerb): def run(self, ctx: Commander) -> None: todo = ctx.comms.goals.todos.resolve_order(ctx.incarnation, self.unit, self.problem) - ctx.done(todo.id, self.sha, self.screenshot, self.seen) + delivery = todo.order.checked().scope.delivery( + ctx.repo_directory(todo.order.check.directory), self.sha, self.screenshot, self.seen) + ctx.done(todo.id, delivery) @dataclass(frozen=True) diff --git a/src/agent_comms/goals.py b/src/agent_comms/goals.py index cd843bdddb..53eef05601 100644 --- a/src/agent_comms/goals.py +++ b/src/agent_comms/goals.py @@ -362,14 +362,13 @@ def git_command(words: Sequence[str]) -> str: return word return "" - def require_acceptance(self, checkout_base: Path) -> None: - """A new or corrected check must distinguish origin/main from delivered work.""" + def require_acceptance(self, base: str, checkout_base: Path) -> None: + """The order state supplies the source preceding its delivered work.""" self.require_writable() - CheckedCommit.git(self.directory, "fetch", "-q", "origin", "main") try: - ran = self.run_at("origin/main", self.directory, checkout_base) + ran = self.run_at(base, self.directory, checkout_base) except subprocess.TimeoutExpired as error: - raise ValueError("The order check timed out on origin/main.") from error + raise ValueError(f"The order check timed out on {base}.") from error if refused := self.result.distance(ran).unproven(self.command, 0): raise ValueError("; ".join(refused)) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 64a8df8811..ea61be1d1b 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -81,7 +81,7 @@ from agent_comms.plan_phases import OrderPhase, UnplannedPhase from agent_comms.private_bus_checkpoint import PublicMessages from agent_comms.problem_number import ProblemNumber -from agent_comms.progress_claims import MergedClaim, ProgressClaim +from agent_comms.progress_claims import MergedClaim, ProgressClaim, WorkingClaim from agent_comms.recorded_time import ( EventTime, RecordedEventTime, @@ -118,6 +118,9 @@ class DeliveryEvidence(DeclaredFamily, affix="DeliveryEvidence"): def artifact(self) -> str: raise ValueError("No application evidence was supplied at delivery; pass --artifact FILE.") + def require_run(self, run: OrderRun) -> None: + run.require_artifact("") + @dataclass(frozen=True) class NoDeliveryEvidence(DeliveryEvidence): @@ -133,11 +136,20 @@ def artifact(self) -> str: raise ValueError("Delivery evidence must name an existing file.") return self.path + def require_run(self, run: OrderRun) -> None: + run.require_artifact(self.path) + @dataclass(frozen=True, kw_only=True) class OrderDelivery(DeclaredFamily, StoryEvent, affix="Delivery"): at: EventTime = field(default_factory=UnrecordedEventTime) + def check_base(self, repo: Path) -> str: + raise ValueError("This delivery has no source commit to establish a check base.") + + def done_claims(self, order: CheckedProblemOrder) -> tuple[ProgressClaim, ...]: + return () + @property def installation_commits(self) -> tuple[str, ...]: return () @@ -192,6 +204,10 @@ class CheckedDelivery(OrderDelivery, declared_name=None): def closing_artifact(self) -> str: return self.evidence.artifact() + def done_claims(self, order: CheckedProblemOrder) -> tuple[ProgressClaim, ...]: + self.evidence.require_run(order.run) + return super().done_claims(order) + @property def installation_build(self) -> BuildIdentity: return self.proof_build @@ -239,6 +255,22 @@ def __post_init__(self) -> None: GitCommitText.decode(self.commit) GitCommitText.decode(self.delivered) + def check_base(self, repo: Path) -> str: + return Repo.git(repo, "merge-base", self.delivered, WorkBranches.main).strip() + + def done_claims(self, order: CheckedProblemOrder) -> tuple[ProgressClaim, ...]: + from agent_comms.commander.order import forwarders_added + + super().done_claims(order) + repo = Path(order.check.directory) + diff = Repo.git(repo, "show", "--first-parent", "-m", "--format=", self.delivered) + changed = re.findall(r'^\+\+\+ b/(\S+\.py)$', diff, re.M) + sources = [Repo.git(repo, "show", f"{self.delivered}:{path}") for path in changed] + if added := forwarders_added(diff, sources): + raise ValueError(f"Refused: {self.delivered} adds forwarders {added[:5]}; " + "slop never counts as delivery. Order the hierarchy fix.") + return (MergedClaim(self.delivered),) + @property def delivered(self) -> str: return self.commit @@ -389,6 +421,13 @@ def line(self) -> str: @dataclass(frozen=True, kw_only=True) class OperationDelivery(CheckedDelivery): + def check_base(self, repo: Path) -> str: + return WorkBranches.main + + def done_claims(self, order: CheckedProblemOrder) -> tuple[ProgressClaim, ...]: + super().done_claims(order) + return (WorkingClaim(self.evidence.artifact()),) + @property def line(self) -> str: return "operation checked" @@ -1415,9 +1454,11 @@ class ProblemOrder(LifecycleCapability, DeclaredFamily, StorySource, affix="Prob def line(self) -> str: return "No order" + def check_base(self, repo: Path) -> str: + raise ValueError("This problem has no active checked order.") + def corrected(self, correction: CorrectCheckTodo) -> ProblemOrder: - """Only pending and claimed orders take a check correction (DeliverableProblemOrder).""" - raise ValueError("Only a pending or claimed order takes a check correction.") + raise ValueError("Only an active checked order takes a check correction.") finding: Annotated[ProblemFinding, WireOmitDefault()] = field( default_factory=UnlocatedProblemFinding @@ -1573,11 +1614,19 @@ def verified_claims( comms: Comms, checkout: Path, ) -> dict[ProgressClaim, tuple[str, ...]]: ... + @abstractmethod + def delivery(self, repo: Path, sha: str, screenshot: str, seen: str) -> Delivery: ... + @abstractmethod def completed(self, todo: Todo, delivery: Delivery) -> Todo: ... class TriageScope(OrderScope[ReportDelivery]): + def delivery(self, repo: Path, sha: str, screenshot: str, seen: str) -> ReportDelivery: + if sha or screenshot: + raise ValueError("Triage returns --seen REPORT, not a commit or screenshot.") + return ReportDelivery(report=Path(seen).read_text()) + def instructions(self, problem: str) -> tuple[str, ...]: return ("Scope: triage; classify and report only. Do not edit source or deliver commits.", "Return the report to your commander; do not open an implementation PR.") @@ -1599,6 +1648,19 @@ def completed(self, todo: Todo, delivery: ReportDelivery) -> Todo: class FixScope(OrderScope[CheckedDelivery]): shape: FixShape + def delivery(self, repo: Path, sha: str, screenshot: str, seen: str) -> CheckedDelivery: + path = seen or screenshot + evidence = FileDeliveryEvidence(path) if path else NoDeliveryEvidence() + if sha == 'op': + return OperationDelivery(evidence=evidence) + if not sha: + raise ValueError("A fix delivery needs --sha COMMIT or --sha op.") + return CommitDelivery( + commit=Repo.git(repo, 'rev-parse', '--verify', 'HEAD^{commit}').strip(), + delivered_commit=Repo.git(repo, 'rev-parse', '--verify', f'{sha}^{{commit}}').strip(), + evidence=evidence, + ) + def require_references(self, root: Path, revision: SourceRevision) -> None: self.shape.require_references(root, revision) @@ -1820,7 +1882,22 @@ def withdrawn(self, previous: Assignment, why: str, superseded_by: Successor) -> @dataclass(frozen=True, kw_only=True) -class AwaitingInstallationProblemOrder(CheckedProblemOrder, declared_name=None): +class OpenProblemOrder(CheckedProblemOrder, declared_name=None): + """An active order keeps corrections and accepts its checked delivery by identity.""" + + def corrected(self, correction: CorrectCheckTodo) -> Self: + if correction.previous != self.check: + raise ValueError("The order's check changed since it was read; read it again.") + return replace(self, check=correction.check, + check_corrections=(*self.check_corrections, correction)) + + def require_delivery(self, order_id: str) -> None: + if self.id != order_id: + raise ValueError("The order changed; refresh its state before delivering it.") + + +@dataclass(frozen=True, kw_only=True) +class AwaitingInstallationProblemOrder(OpenProblemOrder, declared_name=None): def waiting_on_install(self, todo: Todo, read: dict[str, OrderProgress]) -> tuple[Todo, ...]: return (todo,) @@ -1830,6 +1907,9 @@ class MergedProblemOrder(AwaitingInstallationProblemOrder): """The checked order installed by a merge, with its actual merge identity.""" sha: str + def check_base(self, repo: Path) -> str: + return f'{self.sha}^1' + @property def line(self) -> str: return self.run.merged_line @@ -1857,19 +1937,9 @@ def installed(self, works: str, artifact: str) -> ProblemOrder: @dataclass(frozen=True, kw_only=True) -class DeliverableProblemOrder(CheckedProblemOrder, declared_name=None): +class DeliverableProblemOrder(OpenProblemOrder, declared_name=None): """Pending work and a holder's claim both await the commander's delivery verdict.""" - def corrected(self, correction: CorrectCheckTodo) -> DeliverableProblemOrder: - if correction.previous != self.check: - raise ValueError("The order's check changed since it was read; read it again.") - return replace(self, check=correction.check, - check_corrections=(*self.check_corrections, correction)) - - def require_delivery(self, order_id: str) -> None: - if self.id != order_id: - raise ValueError("The order changed; refresh its state before delivering it.") - def sent_back(self, reason: StrikeReason) -> PendingProblemOrder: return self.fresh_order(time.time(), (*self.edits, reason.text)) @@ -1889,6 +1959,9 @@ class PendingProblemOrder(Pending, DeliverableProblemOrder): first_call_deadline: EventTime = field( default_factory=lambda: RecordedEventTime(time.time() + 120)) + def check_base(self, repo: Path) -> str: + return WorkBranches.main + def reissued(self, at: float) -> PendingProblemOrder: return self.fresh_order(at, self.edits) @@ -1943,6 +2016,9 @@ class ClaimedProblemOrder(DeliverableProblemOrder): claim: OrderClaim line: ClassVar[str] = "Claimed by the unit, waiting on the commander's verdict" + def check_base(self, repo: Path) -> str: + return WorkBranches.main + def require_reassignment(self, todo: Todo, proposed: Assignment) -> None: """A corrected order to the claimant replaces its claim; another owner is a transfer.""" if proposed.owner_identity != todo.holder_state.previous_assignment().owner_identity: @@ -1956,6 +2032,9 @@ def observed_merges(self, repo: Repo, source: ProgressSource) -> tuple[MergedPro class DeliveredProblemOrder(AwaitingInstallationProblemOrder): delivery: OrderDelivery + def check_base(self, repo: Path) -> str: + return self.delivery.check_base(repo) + @property def line(self) -> str: return f"Delivered: {self.delivery.line}, waiting on install" diff --git a/tests/test_order_check_writes.py b/tests/test_order_check_writes.py index 69393fa2a4..a39cbe27a6 100644 --- a/tests/test_order_check_writes.py +++ b/tests/test_order_check_writes.py @@ -4,6 +4,7 @@ from agent_comms.field_codec import FieldCodec from agent_comms.goals import CountCheckResult, OrderCheck +from agent_comms.work_branches import WorkBranches @pytest.mark.parametrize('command', ['git checkout main', 'git -C /repo switch work', @@ -32,7 +33,7 @@ def test_multiline_checks_keep_quoted_newlines_and_command_boundaries(): def test_acceptance_refuses_passing_and_unmeasurable_checks(source_repository, tmp_path, command, expected): check = OrderCheck(str(source_repository.checkout), command, CountCheckResult(expected)) with pytest.raises(ValueError): - check.require_acceptance(tmp_path / 'checks') + check.require_acceptance(WorkBranches.main, tmp_path / 'checks') assert not tuple((tmp_path / 'checks').iterdir()) diff --git a/tests/test_order_completion.py b/tests/test_order_completion.py new file mode 100644 index 0000000000..7670374ffd --- /dev/null +++ b/tests/test_order_completion.py @@ -0,0 +1,94 @@ +"""One copied-bus command family owns check correction, delivery and closure.""" +from dataclasses import replace + +from agent_comms.commander.order import main +from agent_comms.goals import CountCheckResult, OrderCheck +from agent_comms.order_runs import ApplicationOrderRun +from agent_comms.todos import ( + DeliveredProblemOrder, + DoneTodoState, + InstalledMergedProblemOrder, + MergedProblemOrder, + ReportedProblemOrder, + TriageScope, +) +from agent_comms.work_branches import WorkBranches + +pytest_plugins = ('test_todos', 'test_held_problem_orders') + + +class TestOrderCompletion: + @staticmethod + def record(ctx, order): + store = ctx.comms.goals.todos + current = store.get('P1') + with store.writing() as connection: + replace(current, order=order, revision=current.revision + 1).upsert(connection) + return store.get('P1') + + def test_merged_correction_and_done_use_the_current_check( + self, held_commander, proof_build_checkouts, + ): + ctx = held_commander + repo = ctx.repos[0] + graph = next(graph for graph in proof_build_checkouts if graph.repository == repo) + original = ctx.comms.goals.todos.get('P1').order + merged = replace(original, check=OrderCheck( + str(repo.checkout), 'printf 1', CountCheckResult(0))) + self.record(ctx, merged.merged(graph.installed)) + command = "git log -1 --format=%s | grep -c 'proof 0'" + main(['check', 'P1', '--check', f'{repo.checkout} :: {command} -> 0', + '--why', 'The old check included declarations outside the authorized family'], ctx) + corrected = ctx.comms.goals.todos.get('P1') + assert isinstance(corrected.order, MergedProblemOrder) + assert corrected.order.check_base(repo.checkout) == f'{graph.installed}^1' + assert corrected.order.check_corrections[-1].previous == merged.check + main(['done', 'worker', 'P1', '--sha', graph.installed], ctx) + delivered = ctx.comms.goals.todos.get('P1') + assert isinstance(delivered.order, DeliveredProblemOrder) + assert delivered.order.check.command == command + assert delivered.order.delivery.delivered == graph.installed + assert delivered.order.check_base(repo.checkout) == graph.installed + assert delivered.strike_count == corrected.strike_count + + def test_open_correction_uses_main_and_keeps_the_order(self, held_commander): + ctx = held_commander + original = ctx.comms.goals.todos.get('P1') + repo = ctx.repos[0] + main(['check', 'P1', '--check', f'{repo.checkout} :: printf 1 -> 0', + '--why', 'The original check counted source outside the declared fix family'], ctx) + corrected = ctx.comms.goals.todos.get('P1') + assert corrected.order.check_base(repo.checkout) == WorkBranches.main + assert corrected.order.check.command == 'printf 1' + assert (corrected.order.id, corrected.assignment, + corrected.review_at, corrected.strikes) == ( + original.order.id, original.assignment, original.review_at, original.strikes) + + def test_triage_done_reads_the_report_without_a_sha(self, held_commander): + ctx = held_commander + self.record(ctx, replace(ctx.comms.goals.todos.get('P1').order, scope=TriageScope())) + report = ctx.root / 'triage.txt' + report.write_text( + 'The source check belongs to its order state; no implementation was requested.') + main(['done', 'worker', 'P1', '--seen', str(report)], ctx) + closed = ctx.comms.goals.todos.get('P1') + assert closed.state is DoneTodoState + assert isinstance(closed.order, ReportedProblemOrder) + assert closed.order.delivery.report == report.read_text() + assert closed.assignment is None + + def test_merged_close_accepts_the_explicit_artifact( + self, held_commander, proof_build_checkouts, + ): + ctx = held_commander + graph = next(graph for graph in proof_build_checkouts if graph.repository == ctx.repos[0]) + works = 'Open retained workflow and confirm its report stays visible' + original = ctx.comms.goals.todos.get('P1').order + self.record(ctx, replace(original, run=ApplicationOrderRun(works)).merged(graph.installed)) + artifact = ctx.root / 'application.txt' + artifact.write_text(works) + main(['close', 'P1', '--works', works, '--artifact', str(artifact)], ctx) + closed = ctx.comms.goals.todos.get('P1') + assert closed.state is DoneTodoState + assert isinstance(closed.order, InstalledMergedProblemOrder) + assert closed.order.artifact == str(artifact) From af60fa0fa8120ded9bc87a6fd356505c39a0302c Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Wed, 21 Oct 2026 19:50:17 -0400 Subject: [PATCH 2/3] P451: decode delivery sources through their declared family --- src/agent_comms/commander/order.py | 89 ++++++++++++++++++++++++++---- src/agent_comms/todos.py | 43 +++++++-------- tests/test_order_completion.py | 20 +++++++ 3 files changed, 118 insertions(+), 34 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 7d5178f173..04a50b4f9c 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -61,11 +61,16 @@ from agent_comms.thread_identity import ThreadIncarnation from agent_comms.todos import ( BluntText, + CheckedDelivery, CleanHunt, + CommitDelivery, + DeliveryEvidence, + DeliverySource, DirectiveText, LocatedProblemFinding, MeasuredReview, NoSuccessor, + OperationDelivery, OrderDelivery, OrderedHunt, OrderMessage, @@ -174,6 +179,62 @@ 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 '')]) +class CommandDeliverySource(DeliverySource, declared_name=None): + @classmethod + def from_text(cls, text: str) -> CommandDeliverySource: + # Reserved arguments derive from declarations; every other token is a Git reference. + return ArgumentDeliverySource.member_or_default(text, SingleFieldValue).from_argument(text) + + +class ArgumentDeliverySource(CommandDeliverySource, declared_name=None): + @classmethod + @abstractmethod + def from_argument(cls, text: str) -> Self: ... + + +@dataclass(frozen=True) +class NoDeliverySource(NullMember, CommandDeliverySource): + def require_report(self) -> None: + pass + + def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: + raise ValueError("A fix delivery needs --sha COMMIT or --sha op.") + + +@dataclass(frozen=True) +class OpDeliverySource(ArgumentDeliverySource): + @classmethod + def from_argument(cls, text: str) -> Self: + return cls() + + def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: + return OperationDelivery(evidence=evidence) + + +@dataclass(frozen=True) +class CommitDeliverySource(SingleFieldValue, ArgumentDeliverySource): + reference: str + + @classmethod + def from_argument(cls, text: str) -> Self: + if not text.strip(): + raise ValueError("A commit delivery needs a nonempty Git reference.") + return cls(text) + + def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: + delivered = Repo.git(repo, 'rev-parse', '--verify', f'{self.reference}^{{commit}}').strip() + diff = Repo.git(repo, 'show', '--first-parent', '-m', '--format=', delivered) + changed = re.findall(r'^\+\+\+ b/(\S+\.py)$', diff, re.M) + sources = [Repo.git(repo, 'show', f'{delivered}:{path}') for path in changed] + if added := forwarders_added(diff, sources): + raise ValueError(f'Refused: {delivered} adds forwarders {added[:5]}; ' + 'slop never counts as delivery. Order the hierarchy fix.') + return CommitDelivery( + commit=Repo.git(repo, 'rev-parse', '--verify', 'HEAD^{commit}').strip(), + delivered_commit=delivered, evidence=evidence, + ) + + @dataclass(frozen=True) class OrderFile: """CLI context and prose around the same order the native tools accept.""" @@ -552,14 +613,15 @@ def send(self, unit, document: OrderFile, clean=False, def correct_check(self, pid: str, line: str, why: str) -> None: """Correct the current check against the source preceding this order's delivery.""" todo = self.comms.goals.todos.get(pid) + order = todo.order.checked() check = self.read_check(line) if len(why.split()) < 8: sys.exit('Refused: --why says what was wrong with the recorded check (8+ words)') repo = self.repo_directory(check.directory) Repo.git(repo, 'fetch', '-q', 'origin', 'main') - check.require_acceptance(todo.order.check_base(repo), self.worktrees) + check.require_acceptance(order.check_base(repo), self.worktrees) self.call(CommsOrderCheckTool(problem=pid, check=check, why=why)) - print(f'{pid}: order {todo.order.id} now checks `{check.command}`') + print(f'{pid}: order {order.id} now checks `{check.command}`') def say(self, units: str, body_file: str, answers: tuple[MessageReference, ...] = ()) -> None: @@ -642,9 +704,10 @@ def done(self, pid: str, delivery: OrderDelivery) -> None: """Storage is checked here; the delivery owns its evidence and claims.""" todo = self.comms.goals.todos.get(pid) todo.state.require_open(todo) - todo.order.checked().require_delivery(todo.order.id) + order = todo.order.checked() + order.require_delivery(order.id) unit = todo.held().owner - claims = delivery.done_claims(todo.order.checked()) + claims = delivery.done_claims(order) thread = self.comms.registry.require(unit) storage = UnitStorage.read(self.comms.registry.storage_rule, thread) for checkout in storage.rule.problem_checkouts(unit, pid): @@ -656,8 +719,8 @@ def done(self, pid: str, delivery: OrderDelivery) -> None: except SelectedToolDenied as quota: sys.exit(f'Refused: {unit}: {quota}; storage rule: it cleans up before the order counts as done.') self.call(CommsOrderDoneTool(problem=pid, - order_id=todo.order.id, delivery=delivery, claims=claims)) - print(f'{unit}: order {todo.order.id} delivered as {delivery.work}') + order_id=order.id, delivery=delivery, claims=claims)) + print(f'{unit}: order {order.id} delivered as {delivery.work}') def hunt(self, where, why, sent): """Record the commander's source read, whether or not it became an order.""" @@ -937,11 +1000,14 @@ class DoneVerb(OrderVerb): ), ] sha: Annotated[ - str, + CommandDeliverySource, + Normalize(FieldCodec.encode), Argument( - flags=("--sha",), options={"help": "for a fix: commit in the recorded checkout, or op"} + flags=("--sha",), + options={"type": CommandDeliverySource.from_text, "choices": None, + "help": "for a fix: commit in the recorded checkout, or op"}, ), - ] = "" + ] = dataclasses.field(default_factory=NoDeliverySource) screenshot: Annotated[ str, Argument( @@ -957,8 +1023,9 @@ class DoneVerb(OrderVerb): def run(self, ctx: Commander) -> None: todo = ctx.comms.goals.todos.resolve_order(ctx.incarnation, self.unit, self.problem) - delivery = todo.order.checked().scope.delivery( - ctx.repo_directory(todo.order.check.directory), self.sha, self.screenshot, self.seen) + order = todo.order.checked() + delivery = order.scope.delivery( + ctx.repo_directory(order.check.directory), self.sha, self.screenshot, self.seen) ctx.done(todo.id, delivery) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index ea61be1d1b..671b2047b9 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -259,16 +259,7 @@ def check_base(self, repo: Path) -> str: return Repo.git(repo, "merge-base", self.delivered, WorkBranches.main).strip() def done_claims(self, order: CheckedProblemOrder) -> tuple[ProgressClaim, ...]: - from agent_comms.commander.order import forwarders_added - super().done_claims(order) - repo = Path(order.check.directory) - diff = Repo.git(repo, "show", "--first-parent", "-m", "--format=", self.delivered) - changed = re.findall(r'^\+\+\+ b/(\S+\.py)$', diff, re.M) - sources = [Repo.git(repo, "show", f"{self.delivered}:{path}") for path in changed] - if added := forwarders_added(diff, sources): - raise ValueError(f"Refused: {self.delivered} adds forwarders {added[:5]}; " - "slop never counts as delivery. Order the hierarchy fix.") return (MergedClaim(self.delivered),) @property @@ -1587,6 +1578,16 @@ def require_references(self, root: Path, revision: SourceRevision) -> None: location.require_class(root, revision) +class DeliverySource(DeclaredFamily, affix="DeliverySource"): + """The command's decoded source, before a commit reference is resolved in its repository.""" + + @abstractmethod + def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: ... + + def require_report(self) -> None: + raise ValueError("Triage returns --seen REPORT, not a commit or operation.") + + @dataclass(frozen=True) class OrderScope[Delivery: OrderDelivery](DeclaredFamily, affix="Scope"): """What the commander authorizes, including the delivery type at its boundary.""" @@ -1615,16 +1616,19 @@ def verified_claims( ) -> dict[ProgressClaim, tuple[str, ...]]: ... @abstractmethod - def delivery(self, repo: Path, sha: str, screenshot: str, seen: str) -> Delivery: ... + def delivery(self, repo: Path, source: DeliverySource, + screenshot: str, seen: str) -> Delivery: ... @abstractmethod def completed(self, todo: Todo, delivery: Delivery) -> Todo: ... class TriageScope(OrderScope[ReportDelivery]): - def delivery(self, repo: Path, sha: str, screenshot: str, seen: str) -> ReportDelivery: - if sha or screenshot: - raise ValueError("Triage returns --seen REPORT, not a commit or screenshot.") + def delivery(self, repo: Path, source: DeliverySource, + screenshot: str, seen: str) -> ReportDelivery: + source.require_report() + if screenshot: + raise ValueError("Triage returns --seen REPORT, not a screenshot.") return ReportDelivery(report=Path(seen).read_text()) def instructions(self, problem: str) -> tuple[str, ...]: @@ -1648,18 +1652,11 @@ def completed(self, todo: Todo, delivery: ReportDelivery) -> Todo: class FixScope(OrderScope[CheckedDelivery]): shape: FixShape - def delivery(self, repo: Path, sha: str, screenshot: str, seen: str) -> CheckedDelivery: + def delivery(self, repo: Path, source: DeliverySource, + screenshot: str, seen: str) -> CheckedDelivery: path = seen or screenshot evidence = FileDeliveryEvidence(path) if path else NoDeliveryEvidence() - if sha == 'op': - return OperationDelivery(evidence=evidence) - if not sha: - raise ValueError("A fix delivery needs --sha COMMIT or --sha op.") - return CommitDelivery( - commit=Repo.git(repo, 'rev-parse', '--verify', 'HEAD^{commit}').strip(), - delivered_commit=Repo.git(repo, 'rev-parse', '--verify', f'{sha}^{{commit}}').strip(), - evidence=evidence, - ) + return source.delivery(repo, evidence) def require_references(self, root: Path, revision: SourceRevision) -> None: self.shape.require_references(root, revision) diff --git a/tests/test_order_completion.py b/tests/test_order_completion.py index 7670374ffd..7fa228a29c 100644 --- a/tests/test_order_completion.py +++ b/tests/test_order_completion.py @@ -1,6 +1,8 @@ """One copied-bus command family owns check correction, delivery and closure.""" from dataclasses import replace +import pytest + from agent_comms.commander.order import main from agent_comms.goals import CountCheckResult, OrderCheck from agent_comms.order_runs import ApplicationOrderRun @@ -9,6 +11,7 @@ DoneTodoState, InstalledMergedProblemOrder, MergedProblemOrder, + OperationDelivery, ReportedProblemOrder, TriageScope, ) @@ -77,6 +80,23 @@ def test_triage_done_reads_the_report_without_a_sha(self, held_commander): assert closed.order.delivery.report == report.read_text() assert closed.assignment is None + def test_operation_done_keeps_its_existing_evidence(self, held_commander): + ctx = held_commander + evidence = ctx.root / 'operation.txt' + evidence.write_text('The copied operation completed through its current check.') + main(['done', 'worker', 'P1', '--sha', 'op', '--seen', str(evidence)], ctx) + delivered = ctx.comms.goals.todos.get('P1') + assert isinstance(delivered.order.delivery, OperationDelivery) + assert delivered.order.delivery.evidence.artifact() == str(evidence) + + def test_triage_rejects_an_operation_without_recording(self, held_commander): + ctx = held_commander + original = self.record( + ctx, replace(ctx.comms.goals.todos.get('P1').order, scope=TriageScope())) + with pytest.raises(ValueError, match='Triage returns --seen REPORT'): + main(['done', 'worker', 'P1', '--sha', 'op'], ctx) + assert ctx.comms.goals.todos.get('P1') == original + def test_merged_close_accepts_the_explicit_artifact( self, held_commander, proof_build_checkouts, ): From 69e80447a7dad63ed02123061e07ea309affd886 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Wed, 21 Oct 2026 19:59:49 -0400 Subject: [PATCH 3/3] P451: retain decoded delivery requests across the argument boundary --- src/agent_comms/commander/order.py | 36 +++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 04a50b4f9c..b543c24658 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -39,6 +39,7 @@ from agent_comms.errors import RelationViolationError from agent_comms.field_codec import ( Argument, + DecodedValue, FieldCodec, MalformedRecord, Normalize, @@ -183,17 +184,28 @@ class CommandDeliverySource(DeliverySource, declared_name=None): @classmethod def from_text(cls, text: str) -> CommandDeliverySource: # Reserved arguments derive from declarations; every other token is a Git reference. - return ArgumentDeliverySource.member_or_default(text, SingleFieldValue).from_argument(text) + return ArgumentDeliverySource.member_or_default(text, ReferenceDeliverySource).from_argument(text) class ArgumentDeliverySource(CommandDeliverySource, declared_name=None): @classmethod - @abstractmethod - def from_argument(cls, text: str) -> Self: ... + def from_argument(cls, text: str) -> Self: + return cls() + + +@dataclass(frozen=True) +class ReferenceDeliverySource(ArgumentDeliverySource, declared_name=None): + reference: str + + @classmethod + def from_argument(cls, text: str) -> Self: + if not text.strip(): + raise ValueError("A commit delivery needs a nonempty Git reference.") + return cls(text) @dataclass(frozen=True) -class NoDeliverySource(NullMember, CommandDeliverySource): +class NoDeliverySource(CommandDeliverySource): def require_report(self) -> None: pass @@ -203,24 +215,12 @@ def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: @dataclass(frozen=True) class OpDeliverySource(ArgumentDeliverySource): - @classmethod - def from_argument(cls, text: str) -> Self: - return cls() - def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: return OperationDelivery(evidence=evidence) @dataclass(frozen=True) -class CommitDeliverySource(SingleFieldValue, ArgumentDeliverySource): - reference: str - - @classmethod - def from_argument(cls, text: str) -> Self: - if not text.strip(): - raise ValueError("A commit delivery needs a nonempty Git reference.") - return cls(text) - +class CommitDeliverySource(ReferenceDeliverySource): def delivery(self, repo: Path, evidence: DeliveryEvidence) -> CheckedDelivery: delivered = Repo.git(repo, 'rev-parse', '--verify', f'{self.reference}^{{commit}}').strip() diff = Repo.git(repo, 'show', '--first-parent', '-m', '--format=', delivered) @@ -1001,7 +1001,7 @@ class DoneVerb(OrderVerb): ] sha: Annotated[ CommandDeliverySource, - Normalize(FieldCodec.encode), + Normalize(DecodedValue), Argument( flags=("--sha",), options={"type": CommandDeliverySource.from_text, "choices": None,