From 054275a1005f0ec423634afe3fccadd4e6bd4e69 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 20:43:14 -0400 Subject: [PATCH 1/7] P287: gate new commander work on stale orders and plan ownership --- src/agent_comms/commander/order.py | 42 +++++++++++++++++++++++++----- src/agent_comms/plan_phases.py | 13 ++++++++- src/agent_comms/repo.py | 6 ++--- src/agent_comms/todos.py | 23 +++++++++++++--- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 33e6ee33de..f0f8027849 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -41,7 +41,7 @@ from ..goals import OrderCheck, ProblemGoalOrder from ..message_reference import MessageReference from ..native_tool_call import SelectedToolDenied -from ..plan_phases import OrderPhase +from ..plan_phases import OrderPhase, PlanPhase from ..progress_claims import MergedClaim, WorkingClaim from ..recorded_time import RecordedEventTime from ..repo import Repo @@ -179,6 +179,7 @@ class OrderFile: order: PendingProblemOrder defect: str why_stupid: str + unplanned_reason: str = "" problems: tuple[str, ...] = () slop: tuple[Slop, ...] = () @@ -191,7 +192,8 @@ def read(cls, text: str): @property def prose(self) -> str: - return f'{self.defect}\n{self.why_stupid}' + reason = f'\nUnplanned reason: {self.unplanned_reason}' if self.unplanned_reason else '' + return f'{self.defect}\n{self.why_stupid}{reason}' def problems_found(self, clean: bool) -> list[str]: problems = [f'{name}: {error}; fix: state the defect plainly without wording rules on code' @@ -323,6 +325,19 @@ def send(self, unit, due, document: OrderFile, clean=False, reassign=None, new=N problems.append('due: fix: supply HH:MM local time (00:00 through 23:59)') group = [new[0]] if new else list(document.problems) listed = {t.id: t for t in self.todos()} + history = self.comms.goals.todos.history_for_commander(self.incarnation) + problems.extend(order.serves.order_refusals(history, document.unplanned_reason)) + held = tuple(todo for todo in listed.values() if todo.holder) + busy = tuple(todo for plan in PlanPhase.members_with(PlanPhase) + for todo in plan.serving(held) if todo.holder == unit and todo.id not in group) + if busy: + snapshot = self.comms.registry.snapshot() + occupied = {todo.holder for todo in held} | {self.commander} | self.parked + idle = sorted(thread.name for thread in snapshot.threads.values() + if thread.name not in occupied and thread.role.executor(thread) + and snapshot.statuses[thread.name].active) + problems.append(f'{unit} already holds plan orders {", ".join(todo.id for todo in busy)}; ' + f'running units holding no order: {", ".join(idle) or "none"}') if not group or any(not re.fullmatch(r'P[1-9]\d*', pid) for pid in group): problems.append('problems: fix: supply an array of P IDs, or use order problem') if new and document.problems: @@ -804,6 +819,21 @@ def parse(cls, argv: list[str], description: str) -> CommanderVerb: return cls.member_named(args.command).from_arguments(args) +class StaleOrderGate(CommanderVerb): + """All directive verbs read the configured commander's same retained board.""" + + def gate(self, ctx: Commander) -> None: + super().gate(ctx) + stale = tuple(todo for todo in ctx.held_orders() if todo.overdue(time.time() - 30 * 60)) + if stale: + lines = (f'{todo.id}: holder {todo.holder}, deadline ' + f'{time.strftime("%Y-%m-%d %H:%M", time.localtime(todo.deadline.recorded_at("An order deadline")))}' + for todo in stale) + sys.exit('REFUSED, nothing recorded: orders more than 30 minutes overdue:\n ' + + '\n '.join(lines) + + '\nMove their deadlines or withdraw them before sending more work.') + + class RuledVerb(CommanderVerb): """HARD RULE: no commander action, mine or a fork's, on rules not re-read in 20 min.""" @@ -857,7 +887,7 @@ class OrderVerb(CommanderVerb): @dataclass(frozen=True, kw_only=True) -class SendVerb(OrderVerb, DirectiveVerb): +class SendVerb(OrderVerb, DirectiveVerb, StaleOrderGate): help = "record the order in Core and send it" unit: str = option("unit") due: str = option("due", help="HH:MM local time") @@ -876,7 +906,7 @@ def run(self, ctx: Commander) -> None: @dataclass(frozen=True, kw_only=True) -class StrikeVerb(OrderVerb, DirectiveVerb): +class StrikeVerb(OrderVerb, DirectiveVerb, StaleOrderGate): help = "a send-back, a missed deadline or a false claim" unit: str = option("unit") why: str = option("--why") @@ -1000,7 +1030,7 @@ def run(self, ctx: Commander) -> None: @dataclass(frozen=True, kw_only=True) -class SayVerb(OrderVerb, DirectiveVerb): +class SayVerb(OrderVerb, DirectiveVerb, StaleOrderGate): help = "a non-order message; refused if it orders work" units: str = option("units", metavar="UNIT[,UNIT]") body_file: str = option("body_file", metavar="BODYFILE") @@ -1010,7 +1040,7 @@ def run(self, ctx: Commander) -> None: @dataclass(frozen=True, kw_only=True) -class ProblemVerb(OrderVerb, DirectiveVerb): +class ProblemVerb(OrderVerb, DirectiveVerb, StaleOrderGate): help = "Core records the problem with its order, or nothing" order: str = option("order", metavar="ORDER.json") to: str = option("--to") diff --git a/src/agent_comms/plan_phases.py b/src/agent_comms/plan_phases.py index 04865c669b..08a448ef7a 100644 --- a/src/agent_comms/plan_phases.py +++ b/src/agent_comms/plan_phases.py @@ -23,9 +23,20 @@ class OrderPhase(DeclaredFamily, affix="Phase"): """What an order serves: one plan phase, or nothing in the plan.""" + @classmethod + def order_refusals(cls, todos: Iterable[Todo], reason: str) -> tuple[str, ...]: + return () + class UnplannedPhase(OrderPhase): - pass + @classmethod + def order_refusals(cls, todos: Iterable[Todo], reason: str) -> tuple[str, ...]: + rows = tuple(todos) + ready = tuple(phase for phase in PlanPhase.members_with(PlanPhase) if phase.ready(rows)) + if ready and len(reason.split()) < 8: + return ('Plan phases are ready: ' + ', '.join(phase.declared_name for phase in ready) + + '. Name a phase, or declare Serves: unplanned with a reason of 8+ words.',) + return () class PlanPhase(OrderPhase, declared_name=None): diff --git a/src/agent_comms/repo.py b/src/agent_comms/repo.py index b6cfe20dbb..79df05b074 100644 --- a/src/agent_comms/repo.py +++ b/src/agent_comms/repo.py @@ -17,10 +17,10 @@ class Repo(BaseModel): github: str source: str = "src" - def contains(self, sha: str) -> bool: - """Only a successful ancestry proof in this installed checkout permits closure.""" + def contains(self, sha: str, revision: str = "HEAD") -> bool: + """Prove ancestry in the requested fetched revision without moving the checkout.""" result = subprocess.run( - ("git", "-C", str(self.checkout), "merge-base", "--is-ancestor", sha, "HEAD"), + ("git", "-C", str(self.checkout), "merge-base", "--is-ancestor", sha, revision), capture_output=True, timeout=60, ) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 896e0be57f..8d2a20ebb3 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -110,6 +110,10 @@ async def checked(self, check: OrderCheck) -> ChildResult: ... @abstractmethod async def delivery_time(self, check): ... + def merged_commits(self, repo: Repo) -> tuple[str, ...]: + """Operations have no commit whose ancestry could prove a merge.""" + return () + def strikes( self, todo: Todo, order_id: str, actor: ThreadIncarnation ) -> tuple[StrikeTodo, ...]: @@ -137,6 +141,9 @@ def delivered(self) -> str: def work(self) -> str: return f"delivered commit {self.delivered}" + def merged_commits(self, repo: Repo) -> tuple[str, ...]: + return (self.delivered,) if repo.contains(self.delivered, "origin/main") else () + async def require_source(self, check: OrderCheck) -> None: raise ValueError("A commit delivery names the unit's delivered commit and the checked one.") @@ -1477,6 +1484,10 @@ def copy_as[Kind: ProblemOrder](self, kind: type[Kind], **evidence) -> Kind: def merged(self, sha: str) -> ProblemOrder: return self.copy_as(MergedProblemOrder, sha=sha) + def observed_merges(self, repo: Repo, source: ProgressSource) -> tuple[ProblemOrder, ...]: + return tuple(self.merged(merged.sha) for merged in MergedProgress.found(source) + if repo.contains(merged.sha)) + @property def pull_request_reference(self) -> str: return f"{self.pull_request_label} {self.id}" @@ -1620,6 +1631,11 @@ def delivered(self, delivery): raise ValueError("The order already records a different delivery.") return self + def observed_merges(self, repo: Repo, source: ProgressSource) -> tuple[ProblemOrder, ...]: + commits = self.delivery.merged_commits(repo) + return (tuple(self.merged(commit) for commit in commits) + or super().observed_merges(repo, source)) + def closing_artifact(self) -> str: return self.delivery.evidence.artifact() @@ -2665,12 +2681,11 @@ def reconcile_merges(self, commander: ThreadIncarnation, repos: Sequence[Repo], 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)) + completed = tuple(DoneTodoState.closed(todo, merged) 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)) + for merged in order.observed_merges( + repo, ProgressSource.of(todo, storage, pulls))) if not completed: return with self.writing() as db: From b12f599ba9237f691962e799755397be370ed7bd Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 20:46:05 -0400 Subject: [PATCH 2/7] P287: checkpoint real TodoStore gate-family scenarios, not yet run --- tests/test_stale_order_gates.py | 97 +++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/test_stale_order_gates.py diff --git a/tests/test_stale_order_gates.py b/tests/test_stale_order_gates.py new file mode 100644 index 0000000000..f4e1bed6c6 --- /dev/null +++ b/tests/test_stale_order_gates.py @@ -0,0 +1,97 @@ +"""Order policy reads actual TodoStore rows through the declared commander verbs.""" +import os +import time +from dataclasses import replace +from pathlib import Path + +import pytest + +from agent_comms.plan_phases import PlanPhase, RecordPlanePhase, UnplannedPhase +from agent_comms.recorded_time import RecordedEventTime +from agent_comms.rule_reviews import AnsweredRuleReviewEntry, DrawnRuleReviewEntry, RuleReviews +from agent_comms.todos import AssignTodo, WithdrawTodo +from test_commander_problem_orders import create, order_body + + +def fresh_rules(comms): + reviews = RuleReviews(comms.root) + reviews.write('lead', DrawnRuleReviewEntry(('Keep orders in their Core owner.',), 0)) + reviews.write('lead', AnsweredRuleReviewEntry( + 1, 'P244 keeps its deadline in the Todo and every gate reads that owner.')) + + +def test_stale_gate_covers_its_verb_family_but_allows_board_repairs( + commander, commander_script, tmp_path): + comms, _ = commander + script, _ = commander_script + fresh_rules(comms) + todo = create(commander, 'P244', deadline=time.time() - 1900) + body = str(tmp_path / 'must-not-be-read') + blocked = ( + ['send', 'worker', '23:59', body], ['say', 'worker', body], + ['problem', 'repair the owner', '--to', 'worker', '--due', '23:59', + '--order', body, '--where', f'{__file__}:1'], + ['strike', 'worker', '--why', 'P244 missed its deadline'], + ) + for args in blocked: + with pytest.raises(SystemExit) as refused: + script.OrderVerb.parse(args, '').apply() + text = str(refused.value) + assert 'P244: holder worker, deadline ' in text + assert 'more than 30 minutes overdue' in text + why = 'The current source needs one more bounded check before delivery.' + repairs = ( + ['due', 'P244', '23:59', '--why', why], ['withdraw', 'P244', '--why', why], + ['done', 'worker', 'order-P244', '--sha', 'a' * 40], + ['close', 'P244', '--works', why], + ['check', 'P244', '--check', f'{tmp_path} :: printf 0 -> 0', '--why', why], + ['serve', 'P244', 'record_plane'], ['drop', 'worker', 'old', '--by', 'new'], + ) + for args in repairs: + script.OrderVerb.parse(args, '').gate() + actor = comms.registry.require('lead').incarnation + comms.goals.todos.apply(todo.id, AssignTodo( + todo.revision, todo.held(), RecordedEventTime(time.time() + 3600), todo.order, todo.goal)) + script.OrderVerb.parse(blocked[0], '').gate() + moved = comms.goals.todos.get(todo.id) + comms.goals.todos.apply(todo.id, WithdrawTodo( + moved.revision, actor, moved.held(), why, None)) + script.OrderVerb.parse(blocked[0], '').gate() + + +@pytest.mark.parametrize('phase', (UnplannedPhase, *PlanPhase.members_with(PlanPhase))) +def test_phase_gate_uses_the_phase_family_and_requires_an_explicit_unplanned_reason( + commander, commander_script, phase): + comms, _ = commander + script, repo = commander_script + (repo / 'count.txt').write_text('0\n') + body = Path(order_body(repo)).read_text() + if phase is UnplannedPhase: + with pytest.raises(SystemExit, match='Plan phases are ready'): + script.send('worker', '23:59', body, clean=True, new=('P1', 'Repair the owner')) + assert comms.goals.todos.for_commander(comms.registry.require('lead').incarnation) == () + reason = 'This urgent retained incident is outside the declared plan dependency chain.' + else: + reason = '' + body += f'\nServes: {phase.declared_name} {reason}\n' + script.send('worker', '23:59', body, clean=True, new=('P1', 'Repair the owner')) + assert comms.goals.todos.get('P1').order.serves is phase + + +def test_second_plan_order_gate_names_the_busy_order_and_running_unowned_units( + commander, commander_script): + comms, _ = commander + script, repo = commander_script + todo = create(commander, 'P1') + with comms.goals.todos.writing() as db: + replace(todo, order=replace(todo.order, serves=RecordPlanePhase)).upsert(db) + comms.registry.declare(comms.registry.new_thread('free-unit', frozenset(), str(repo))) + comms.owners.acquire_thread('free-unit', owner_pid=os.getpid()) + (repo / 'count.txt').write_text('0\n') + body = Path(order_body(repo)).read_text() + '\nServes: record_plane\n' + with pytest.raises(SystemExit) as refused: + script.send('worker', '23:59', body, clean=True, new=('P2', 'Repair another owner')) + assert 'worker already holds plan orders P1' in str(refused.value) + assert 'running units holding no order: free-unit' in str(refused.value) + assert [todo.id for todo in comms.goals.todos.for_commander( + comms.registry.require('lead').incarnation)] == ['P1'] From 77a8cadffa11965602e0237e3bd2363295c6ef63 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 21:46:39 -0400 Subject: [PATCH 3/7] P287: migrate gates onto typed order files and commander context --- src/agent_comms/commander/order.py | 8 ++- tests/test_commander_problem_orders.py | 1 + tests/test_stale_order_gates.py | 99 ++++++++++++++++++-------- 3 files changed, 74 insertions(+), 34 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index f0f8027849..cfbc51dd7e 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -826,9 +826,11 @@ def gate(self, ctx: Commander) -> None: super().gate(ctx) stale = tuple(todo for todo in ctx.held_orders() if todo.overdue(time.time() - 30 * 60)) if stale: - lines = (f'{todo.id}: holder {todo.holder}, deadline ' - f'{time.strftime("%Y-%m-%d %H:%M", time.localtime(todo.deadline.recorded_at("An order deadline")))}' - for todo in stale) + lines = [] + for todo in stale: + due = time.localtime(todo.deadline.recorded_at("An order deadline")) + lines.append(f'{todo.id}: holder {todo.holder}, deadline ' + f'{time.strftime("%Y-%m-%d %H:%M", due)}') sys.exit('REFUSED, nothing recorded: orders more than 30 minutes overdue:\n ' + '\n '.join(lines) + '\nMove their deadlines or withdraw them before sending more work.') diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 2bc690a82d..d89775727e 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -544,6 +544,7 @@ def order_body(repo, check=None, edit='Set count.txt to 1.', extra=''): body.write_text(json.dumps({ 'defect': 'The counter file holds zero where the fleet needs one', 'why_stupid': 'The counter is wrong and that is stupid.', + 'unplanned_reason': 'This isolated order checks command behavior rather than advancing the plan dependency chain.', 'order': {'kind': 'pending', 'owner': f'{__file__}:1', 'check': FieldCodec.encode(checked), 'edits': [edit], 'run': {'kind': 'application', 'text': 'the installed app shows the counter at one after restart'}, diff --git a/tests/test_stale_order_gates.py b/tests/test_stale_order_gates.py index f4e1bed6c6..27deec6c55 100644 --- a/tests/test_stale_order_gates.py +++ b/tests/test_stale_order_gates.py @@ -1,41 +1,38 @@ -"""Order policy reads actual TodoStore rows through the declared commander verbs.""" +"""Real TodoStore rows exercise the gate family and merged-delivery observer.""" +import json import os +import subprocess import time from dataclasses import replace from pathlib import Path import pytest +from agent_comms.commander.order import OrderFile, OrderVerb, StaleOrderGate from agent_comms.plan_phases import PlanPhase, RecordPlanePhase, UnplannedPhase from agent_comms.recorded_time import RecordedEventTime -from agent_comms.rule_reviews import AnsweredRuleReviewEntry, DrawnRuleReviewEntry, RuleReviews -from agent_comms.todos import AssignTodo, WithdrawTodo -from test_commander_problem_orders import create, order_body - - -def fresh_rules(comms): - reviews = RuleReviews(comms.root) - reviews.write('lead', DrawnRuleReviewEntry(('Keep orders in their Core owner.',), 0)) - reviews.write('lead', AnsweredRuleReviewEntry( - 1, 'P244 keeps its deadline in the Todo and every gate reads that owner.')) +from agent_comms.repo import Repo +from agent_comms.todos import AssignTodo, CommitDelivery, OperationDelivery, WithdrawTodo +from agent_comms.tools import CommsOrderDoneTool +from test_commander_problem_orders import create, order_body, working def test_stale_gate_covers_its_verb_family_but_allows_board_repairs( commander, commander_script, tmp_path): comms, _ = commander - script, _ = commander_script - fresh_rules(comms) + context, _ = commander_script todo = create(commander, 'P244', deadline=time.time() - 1900) body = str(tmp_path / 'must-not-be-read') blocked = ( ['send', 'worker', '23:59', body], ['say', 'worker', body], - ['problem', 'repair the owner', '--to', 'worker', '--due', '23:59', - '--order', body, '--where', f'{__file__}:1'], + ['problem', body, '--to', 'worker', '--due', '23:59', '--where', f'{__file__}:1'], ['strike', 'worker', '--why', 'P244 missed its deadline'], ) - for args in blocked: + verbs = tuple(OrderVerb.parse(args, '') for args in blocked) + assert {type(verb) for verb in verbs} == set(OrderVerb.members_with(StaleOrderGate)) + for verb in verbs: with pytest.raises(SystemExit) as refused: - script.OrderVerb.parse(args, '').apply() + verb.apply(context) text = str(refused.value) assert 'P244: holder worker, deadline ' in text assert 'more than 30 minutes overdue' in text @@ -48,50 +45,90 @@ def test_stale_gate_covers_its_verb_family_but_allows_board_repairs( ['serve', 'P244', 'record_plane'], ['drop', 'worker', 'old', '--by', 'new'], ) for args in repairs: - script.OrderVerb.parse(args, '').gate() + OrderVerb.parse(args, '').gate(context) actor = comms.registry.require('lead').incarnation comms.goals.todos.apply(todo.id, AssignTodo( todo.revision, todo.held(), RecordedEventTime(time.time() + 3600), todo.order, todo.goal)) - script.OrderVerb.parse(blocked[0], '').gate() + verbs[0].gate(context) moved = comms.goals.todos.get(todo.id) comms.goals.todos.apply(todo.id, WithdrawTodo( moved.revision, actor, moved.held(), why, None)) - script.OrderVerb.parse(blocked[0], '').gate() + verbs[0].gate(context) + assert not comms.goals.todos.get(todo.id).state.open @pytest.mark.parametrize('phase', (UnplannedPhase, *PlanPhase.members_with(PlanPhase))) def test_phase_gate_uses_the_phase_family_and_requires_an_explicit_unplanned_reason( commander, commander_script, phase): comms, _ = commander - script, repo = commander_script + context, repo = commander_script (repo / 'count.txt').write_text('0\n') - body = Path(order_body(repo)).read_text() + payload = json.loads(Path(order_body(repo)).read_text()) + payload['order']['serves'] = phase.declared_name if phase is UnplannedPhase: + del payload['unplanned_reason'] with pytest.raises(SystemExit, match='Plan phases are ready'): - script.send('worker', '23:59', body, clean=True, new=('P1', 'Repair the owner')) + context.send('worker', '23:59', OrderFile.read(json.dumps(payload)), + clean=True, new=('P1', 'Repair the owner')) assert comms.goals.todos.for_commander(comms.registry.require('lead').incarnation) == () - reason = 'This urgent retained incident is outside the declared plan dependency chain.' - else: - reason = '' - body += f'\nServes: {phase.declared_name} {reason}\n' - script.send('worker', '23:59', body, clean=True, new=('P1', 'Repair the owner')) + payload['unplanned_reason'] = ( + 'This urgent retained incident is outside the declared plan dependency chain.') + context.send('worker', '23:59', OrderFile.read(json.dumps(payload)), + clean=True, new=('P1', 'Repair the owner')) assert comms.goals.todos.get('P1').order.serves is phase def test_second_plan_order_gate_names_the_busy_order_and_running_unowned_units( commander, commander_script): comms, _ = commander - script, repo = commander_script + context, repo = commander_script todo = create(commander, 'P1') with comms.goals.todos.writing() as db: replace(todo, order=replace(todo.order, serves=RecordPlanePhase)).upsert(db) comms.registry.declare(comms.registry.new_thread('free-unit', frozenset(), str(repo))) comms.owners.acquire_thread('free-unit', owner_pid=os.getpid()) (repo / 'count.txt').write_text('0\n') - body = Path(order_body(repo)).read_text() + '\nServes: record_plane\n' + payload = json.loads(Path(order_body(repo)).read_text()) + payload['order']['serves'] = RecordPlanePhase.declared_name with pytest.raises(SystemExit) as refused: - script.send('worker', '23:59', body, clean=True, new=('P2', 'Repair another owner')) + context.send('worker', '23:59', OrderFile.read(json.dumps(payload)), + clean=True, new=('P2', 'Repair another owner')) assert 'worker already holds plan orders P1' in str(refused.value) assert 'running units holding no order: free-unit' in str(refused.value) assert [todo.id for todo in comms.goals.todos.for_commander( comms.registry.require('lead').incarnation)] == ['P1'] + + +@pytest.mark.parametrize('commit_delivery', (True, False)) +def test_observer_closes_only_delivered_commits_present_in_fetched_main( + commander, tmp_path, commit_delivery): + comms, caller = commander + root = comms.root + origin = tmp_path / 'origin.git' + subprocess.run(['git', 'init', '-q', '--bare', str(origin)], check=True) + def git(*args): + return subprocess.check_output(['git', '-C', str(root), '-c', 'user.name=t', + '-c', 'user.email=t@t', *args], text=True).strip() + git('init', '-q', '-b', 'main') + git('remote', 'add', 'origin', str(origin)) + git('commit', '-q', '--allow-empty', '-m', 'delivered source') + sha = git('rev-parse', 'HEAD') + git('push', '-q', 'origin', 'HEAD:main') + todo = create(commander, 'P1', deadline=time.time() + 3600) + with comms.goals.todos.writing() as db: + replace(todo, order=replace(todo.order, serves=RecordPlanePhase)).upsert(db) + delivery = (CommitDelivery(commit=sha, delivered_commit=sha) if commit_delivery + else OperationDelivery()) + CommsOrderDoneTool(problem='P1', order_id=todo.order.id, delivery=delivery, + claims=working(comms)).apply(comms, caller) + assert comms.goals.todos.get('P1').state.open + repo = Repo(checkout=root, github=todo.repo) + actor = comms.registry.require('lead').incarnation + comms.goals.todos.reconcile_merges(actor, (repo,), + comms.registry.storage_rule) + observed = comms.goals.todos.get('P1') + assert observed.state.open is not commit_delivery + assert observed.order.serves is RecordPlanePhase + comms.goals.todos.reconcile_merges(actor, (repo,), + comms.registry.storage_rule) + assert comms.goals.todos.get('P1') == observed From 71fdda70a9dcdbe8ea8c11d9c0e8c1155fad3a38 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 21:49:01 -0400 Subject: [PATCH 4/7] P287: open order-linked PRs from the actual validated checkout --- src/agent_comms/commander/order.py | 6 ++++-- tests/test_stale_order_gates.py | 21 ++++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index cfbc51dd7e..343f82554a 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -1004,9 +1004,11 @@ class PullRequestVerb(OrderVerb, declared_name="pr"): def run(self, ctx: Commander) -> None: todo = ctx.comms.goals.todos.get(self.pid) - checkout = ctx.comms.registry.storage_rule.order_checkout(todo.holder, todo.id) + checkout = Repo.of(Path.cwd()) + if checkout.github.lower() != todo.repo.lower(): + sys.exit(f'Refused: current checkout belongs to {checkout.github}, not {todo.repo}.') print(todo.order.open_pull_request( - todo, checkout, self.title, Path(self.body_file).read_text())) + todo, checkout.checkout, self.title, Path(self.body_file).read_text())) @dataclass(frozen=True, kw_only=True) diff --git a/tests/test_stale_order_gates.py b/tests/test_stale_order_gates.py index 27deec6c55..4d996d7b83 100644 --- a/tests/test_stale_order_gates.py +++ b/tests/test_stale_order_gates.py @@ -12,7 +12,9 @@ from agent_comms.plan_phases import PlanPhase, RecordPlanePhase, UnplannedPhase from agent_comms.recorded_time import RecordedEventTime from agent_comms.repo import Repo -from agent_comms.todos import AssignTodo, CommitDelivery, OperationDelivery, WithdrawTodo +from agent_comms.todos import ( + AssignTodo, CheckedProblemOrder, CommitDelivery, OperationDelivery, WithdrawTodo, +) from agent_comms.tools import CommsOrderDoneTool from test_commander_problem_orders import create, order_body, working @@ -99,6 +101,23 @@ def test_second_plan_order_gate_names_the_busy_order_and_running_unowned_units( comms.registry.require('lead').incarnation)] == ['P1'] +def test_pr_uses_actual_owned_checkout_instead_of_an_invented_problem_directory( + commander, commander_script, monkeypatch, tmp_path): + comms, _ = commander + context, repo = commander_script + todo = create(commander, 'P287') + body = tmp_path / 'pull-request.md' + body.write_text('The retained order owns its immutable pull request identity.') + seen = [] + def publish(order, current, checkout, title, text): + seen.append((current.id, checkout, order.pull_request_body(text))) + return 'https://github.com/OpenHCSDev/agent-comms/pull/1' + monkeypatch.setattr(CheckedProblemOrder, 'open_pull_request', publish) + monkeypatch.chdir(repo) + OrderVerb.parse(['pr', 'P287', '--title', 'P287', '--body-file', str(body)], '').apply(context) + assert seen == [('P287', repo, todo.order.pull_request_body(body.read_text()))] + + @pytest.mark.parametrize('commit_delivery', (True, False)) def test_observer_closes_only_delivered_commits_present_in_fetched_main( commander, tmp_path, commit_delivery): From 9c963ca1e4f524a83cab9f762154a742ac7f78c1 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 21:52:59 -0400 Subject: [PATCH 5/7] P287: report typed unplanned guidance and reject wrong-repository publication --- src/agent_comms/commander/order.py | 3 ++- src/agent_comms/plan_phases.py | 7 +++++-- tests/test_stale_order_gates.py | 5 +++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 343f82554a..dcba35f90c 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -336,7 +336,8 @@ def send(self, unit, due, document: OrderFile, clean=False, reassign=None, new=N idle = sorted(thread.name for thread in snapshot.threads.values() if thread.name not in occupied and thread.role.executor(thread) and snapshot.statuses[thread.name].active) - problems.append(f'{unit} already holds plan orders {", ".join(todo.id for todo in busy)}; ' + problems.append(f'{unit} already holds plan orders ' + f'{", ".join(todo.id for todo in busy)}; ' f'running units holding no order: {", ".join(idle) or "none"}') if not group or any(not re.fullmatch(r'P[1-9]\d*', pid) for pid in group): problems.append('problems: fix: supply an array of P IDs, or use order problem') diff --git a/src/agent_comms/plan_phases.py b/src/agent_comms/plan_phases.py index 08a448ef7a..50676f12ab 100644 --- a/src/agent_comms/plan_phases.py +++ b/src/agent_comms/plan_phases.py @@ -34,8 +34,11 @@ def order_refusals(cls, todos: Iterable[Todo], reason: str) -> tuple[str, ...]: rows = tuple(todos) ready = tuple(phase for phase in PlanPhase.members_with(PlanPhase) if phase.ready(rows)) if ready and len(reason.split()) < 8: - return ('Plan phases are ready: ' + ', '.join(phase.declared_name for phase in ready) - + '. Name a phase, or declare Serves: unplanned with a reason of 8+ words.',) + return ( + "Plan phases are ready: " + + ", ".join(phase.declared_name for phase in ready) + + ". Name a phase, or provide an explicit unplanned_reason of 8+ words.", + ) return () diff --git a/tests/test_stale_order_gates.py b/tests/test_stale_order_gates.py index 4d996d7b83..7dfcd19b15 100644 --- a/tests/test_stale_order_gates.py +++ b/tests/test_stale_order_gates.py @@ -116,6 +116,11 @@ def publish(order, current, checkout, title, text): monkeypatch.chdir(repo) OrderVerb.parse(['pr', 'P287', '--title', 'P287', '--body-file', str(body)], '').apply(context) assert seen == [('P287', repo, todo.order.pull_request_body(body.read_text()))] + subprocess.run(['git', '-C', str(repo), 'remote', 'set-url', 'origin', + 'https://github.com/another/project.git'], check=True) + with pytest.raises(SystemExit, match='not OpenHCSDev/agent-comms'): + OrderVerb.parse(['pr', 'P287', '--title', 'P287', '--body-file', str(body)], '').apply(context) + assert len(seen) == 1 # A different repository must never reach publication. @pytest.mark.parametrize('commit_delivery', (True, False)) From 92871c95ac85b8c37f2716bcd6b826503f50a153 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 21:56:51 -0400 Subject: [PATCH 6/7] P287: derive refusal expectations from the canonical repository name --- tests/test_stale_order_gates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_stale_order_gates.py b/tests/test_stale_order_gates.py index 7dfcd19b15..0e053ea2a1 100644 --- a/tests/test_stale_order_gates.py +++ b/tests/test_stale_order_gates.py @@ -118,7 +118,7 @@ def publish(order, current, checkout, title, text): assert seen == [('P287', repo, todo.order.pull_request_body(body.read_text()))] subprocess.run(['git', '-C', str(repo), 'remote', 'set-url', 'origin', 'https://github.com/another/project.git'], check=True) - with pytest.raises(SystemExit, match='not OpenHCSDev/agent-comms'): + with pytest.raises(SystemExit, match=f'not {todo.repo}'): OrderVerb.parse(['pr', 'P287', '--title', 'P287', '--body-file', str(body)], '').apply(context) assert len(seen) == 1 # A different repository must never reach publication. From 79fd1e24082be2dd58e1388c275eacb959ec739e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 23:13:56 -0400 Subject: [PATCH 7/7] P287: preserve withdrawal closure and consume original-owned phase --- src/agent_comms/commander/order.py | 2 +- src/agent_comms/todos.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index dcba35f90c..3ed66ffbe0 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -326,7 +326,7 @@ def send(self, unit, due, document: OrderFile, clean=False, reassign=None, new=N group = [new[0]] if new else list(document.problems) listed = {t.id: t for t in self.todos()} history = self.comms.goals.todos.history_for_commander(self.incarnation) - problems.extend(order.serves.order_refusals(history, document.unplanned_reason)) + problems.extend(order.phase.order_refusals(history, document.unplanned_reason)) held = tuple(todo for todo in listed.values() if todo.holder) busy = tuple(todo for plan in PlanPhase.members_with(PlanPhase) for todo in plan.serving(held) if todo.holder == unit and todo.id not in group) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 8d2a20ebb3..13da3ca8d3 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -1698,6 +1698,9 @@ class WithdrawnProblemOrder(CheckedProblemOrder): pending: ClassVar[bool] = False line: ClassVar[str] = "Withdrawn" + def require_close(self) -> None: + """Retiring an erroneous order does not claim installed work.""" + def withdrawals(self, todo: Todo) -> tuple[str, ...]: superseded = f" (superseded by {self.superseded_by})" if self.superseded_by else "" return (f"{todo.id}: order {self.id} withdrawn from {self.previous.owner}, "