diff --git a/src/agent_comms/bus_publication.py b/src/agent_comms/bus_publication.py index d12b531386..7315a6c7a2 100644 --- a/src/agent_comms/bus_publication.py +++ b/src/agent_comms/bus_publication.py @@ -185,15 +185,18 @@ def require_source(self, source: BusReader, sender: str, message_id: str) -> Non with source.observing() as db: if MessageIdentitySource.exists(db, sender=sender, message_id=message_id): - raise HumanSendRefusedError("Human message identity was already published.") + raise HumanSendRefusedError(RelationViolationError( + "Human message identity was already published.")) def append(self, bus: Bus, build: RecordBuilder) -> BusRecord: try: return bus.append(build) - except CommittedMessageError: + except (CommittedMessageError, HumanSendRefusedError): raise - except Exception as error: - raise HumanSendRefusedError(f"Message was not sent: {error}") from error + except (OSError, sqlite3.Error, ValueError) as error: + # The transaction owns rollback for storage and declared-data refusals. + # Programming errors propagate as defects, not ordinary send refusals. + raise HumanSendRefusedError(error) from error def require_registered(self, snapshot: RegistrySnapshot) -> None: """Check the determining USER declaration, never a transport or prose role.""" diff --git a/src/agent_comms/channel_targets.py b/src/agent_comms/channel_targets.py index 16ec4dac34..c585eead36 100644 --- a/src/agent_comms/channel_targets.py +++ b/src/agent_comms/channel_targets.py @@ -146,7 +146,11 @@ class NamedTarget(CommandTarget): name: str sigil: ClassVar[str] batchable = False - direct_names = () + + @property + def direct_names(self) -> tuple[str, ...]: + return () + guidance_scope = "" default_audience = MessageAudience.DIRECT @@ -241,7 +245,7 @@ def require_frozen_recipients(self, message, audience): message.role.require_recipients(self, audience) @property - def direct_names(self): + def direct_names(self) -> tuple[str, ...]: return self.names def canonical_name(self, canonical): diff --git a/src/agent_comms/commander/judge.py b/src/agent_comms/commander/judge.py index 43ed256939..1e9ab57342 100644 --- a/src/agent_comms/commander/judge.py +++ b/src/agent_comms/commander/judge.py @@ -374,7 +374,7 @@ def check_edits(self, edits: list[Edit]) -> None: def report(self, ctx: order.Commander, todos: tuple[Todo, ...]) -> None: for todo in todos: - audience = todo.verdicts[-1].audience.of(todo.held().owner, todo.goal) + audience = todo.verdicts[-1].audience.of(todo.held().owner, todo) roast(ctx, audience, todo.holder, self.lines, self.roast, self.fix, self.slop()) line = (f'{time.strftime("%Y-%m-%d %H:%M")} {todo.holder} ' f'(strike {todo.strike_count} on {todo.id}; caught by {ctx.actor_name} at review): ' diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index d312a49625..8f545458b9 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -54,6 +54,7 @@ from agent_comms.message_reference import MessageReference from agent_comms.messages import Message from agent_comms.native_tool_call import SelectedToolDenied +from agent_comms.owner_requests import NewestOpenRequest, OwnerRequestId, RequestSelection from agent_comms.plan_phases import OrderPhase from agent_comms.problem_number import ProblemNumber from agent_comms.recorded_time import RecordedEventTime @@ -435,9 +436,8 @@ def message(self, context: CommanderContext, unit: str, group: Sequence[str], def record(self, context: CommanderContext, unit: str, group: Sequence[str], target: OrderedProblems, traced: FileLine | NoSourceReference, - review_at: RecordedEventTime) -> None: + review_at: RecordedEventTime, request: OwnerRequestId) -> None: """The native tools record the typed order; the document supplies no second store.""" - request = self.order.request(context.comms.registry.require(context.commander), tuple(group)) target.record(context, self.order, unit, group, traced, request, review_at) def send_back(self, context: Commander, unit: str, group: Sequence[str], @@ -496,10 +496,9 @@ def actor_identity(self) -> ThreadIncarnation: def post(self, audience: VerdictAudience, body: str, answers: tuple[MessageReference, ...] = ()) -> str: - target = (audience.threads[0] if len(audience.threads) == 1 - else CommsTag.channel()) - published = self.comms.messaging.send(self.actor_name, target, - audience.addressed(target, body), answers=answers) + target = audience.target + published = self.comms.messaging.send(self.actor_name, target.name, + target.addressed(audience.threads, body), answers=answers) return published.message.message_id def call(self, tool): @@ -579,7 +578,7 @@ def replacing(self, unit: str, group: Sequence[str], listed: Mapping[str, Todo]) @abstractmethod def record(self, context: CommanderContext, order: PendingProblemOrder, unit: str, - group: Sequence[str], traced: FileLine | NoSourceReference, request: str, + group: Sequence[str], traced: FileLine | NoSourceReference, request: OwnerRequestId, review_at: RecordedEventTime) -> None: ... @@ -601,7 +600,7 @@ def replacing(self, unit: str, group: Sequence[str], listed: Mapping[str, Todo]) return False def record(self, context: CommanderContext, order: PendingProblemOrder, unit: str, - group: Sequence[str], traced: FileLine | NoSourceReference, request: str, + group: Sequence[str], traced: FileLine | NoSourceReference, request: OwnerRequestId, review_at: RecordedEventTime) -> None: context.call(CommsTodoCreateTool(owner=unit, order=CreateProblemGoalAction( problem=self.problem, repo=Repo.of(Path(order.check.directory)).github, @@ -625,7 +624,7 @@ def replacing(self, unit: str, group: Sequence[str], listed: Mapping[str, Todo]) return bool(group) and all(pid in listed and listed[pid].holder == unit for pid in group) def record(self, context: CommanderContext, order: PendingProblemOrder, unit: str, - group: Sequence[str], traced: FileLine | NoSourceReference, request: str, + group: Sequence[str], traced: FileLine | NoSourceReference, request: OwnerRequestId, review_at: RecordedEventTime) -> None: for pid in group: context.call(CommsOrderTool(owner=unit, order=OrderGoalAction( @@ -736,7 +735,7 @@ def send(self, unit: str, document: OrderFile, clean: bool = False, reassign: Reassignment = NoReassignment(), target: OrderedProblems = ListedProblems(), traced: FileLine | NoSourceReference = NoSourceReference(), disk: bool = False, finding: ProblemFinding = UnlocatedProblemFinding(), - answers: tuple[MessageReference, ...] = ()) -> None: + answers: tuple[MessageReference, ...] = (), *, request: RequestSelection) -> None: """Record the decoded order, or report every failed prerequisite before any write.""" document = document.resolved(self, finding) group = target.group(document) @@ -748,7 +747,8 @@ def send(self, unit: str, document: OrderFile, clean: bool = False, measured = MeasuredReview.read(self.comms.goals.todos) review_at = measured.review_at(document.order) message = document.message(self, unit, group, replacing, reassign) - document.record(self, unit, group, target, traced, review_at) + request_id = request.for_order(self.comms.goals.todos, self.incarnation) + document.record(self, unit, group, target, traced, review_at, request_id) sent = self.post(VerdictAudience((unit,)), message, answers) for pid in group: self.call(CommsChangeProblemTool(problem=pid, operation=OrderMessage( @@ -781,20 +781,6 @@ def say(self, units: str, body_file: str, refuse(body, *OUTGOING) print(self.post(VerdictAudience(tuple(units.split(','))), body.rstrip(), answers)) - def problem(self, order_file: str, unit: str, where: str, disk: bool = False, - answers: tuple[MessageReference, ...] = ()) -> None: - """A problem is born owned: Core records it with its order in one step, or not at all.""" - document = OrderFile.read(Path(order_file).read_text()) - location = FileLine.from_wire(where) - checkout = self.repo_directory(document.order.check.directory) - if checkout.is_dir(): - self.comms.goals.todos.require_recurrence_declaration( - document.order, location, checkout, document.slop, time.time()) - pid = self.comms.goals.todos.allocate_problem() - self.send(unit, document, target=NewProblem(pid, document.defect), - disk=disk, finding=LocatedProblemFinding(location), answers=answers) - print(pid) - def close(self, pid, works, artifact=''): """Core owns closing: the installed evidence, the artifact and the no-open-order rule live on the Todo.""" evidence = artifact or self.comms.goals.todos.get(pid).order.closing_artifact() @@ -847,7 +833,7 @@ def posted(self, unit: str, todo: Todo, strike: StrikeTodo, n = struck(todo, unit) line = (f'{time.strftime("%Y-%m-%d")} {strike.at.clock()} {unit} (strike {n} on {todo.id}' f'{f", {todo.id} taken away" if n >= 2 else ""}): {strike.reason}') - self.on_record(line, 'Strike', VerdictAudience.of(unit, todo.goal), answers) + self.on_record(line, 'Strike', VerdictAudience.of(unit, todo), 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: str, delivery: OrderDelivery) -> None: @@ -899,7 +885,7 @@ def unstrike(self, task: str, why: str, at: StrikeSelector = NewestStrike()) -> n = struck(self.comms.goals.todos.get(task), unit) line = (f'{time.strftime("%Y-%m-%d %H:%M")} {unit} (strike voided on {task}, ' f'recorded {voided.at.clock()}; {n} strike(s) remain): {why}') - self.on_record(line, 'Void', VerdictAudience.of(unit, todo.goal)) + self.on_record(line, 'Void', VerdictAudience.of(unit, todo)) print(f'{unit}: {n} strike(s) on {task}') def withdraw(self, pid: str, why: str, superseded_by: Successor = NoSuccessor()) -> None: @@ -916,7 +902,7 @@ def withdraw(self, pid: str, why: str, superseded_by: Successor = NoSuccessor()) unit = previous.owner withdrawal, = self.comms.goals.todos.get(pid).order.withdrawals(todo) self.on_record(f'{time.strftime("%Y-%m-%d %H:%M")} {withdrawal}', 'Withdrawal', - VerdictAudience.of(unit, todo.goal)) + VerdictAudience.of(unit, todo)) self.post(VerdictAudience((unit,)), f'Withdrawn: {withdrawal}. Stop that work; it counts against no one.') print(withdrawal) @@ -1004,7 +990,15 @@ class OrderVerb(CommanderVerb): @dataclass(frozen=True, kw_only=True) -class SendVerb(OrderVerb, DirectiveVerb): +class RequestedOrderVerb(OrderVerb, DirectiveVerb, declared_name=None): + request: Annotated[ + RequestSelection, Argument(flags=("--request",), + options={"help": "owner request ID; defaults to newest open"}), + ] = NewestOpenRequest() + + +@dataclass(frozen=True, kw_only=True) +class SendVerb(RequestedOrderVerb): help = "record the order in Core and send it" unit: Annotated[str, Argument(flags=("unit",))] body_file: Annotated[ @@ -1043,7 +1037,7 @@ class SendVerb(OrderVerb, DirectiveVerb): def respond(self, ctx: Commander, references: tuple[MessageReference, ...]) -> None: ctx.send(self.unit, OrderFile.read(Path(self.body_file).read_text()), clean=self.clean, reassign=self.reassign, traced=self.traced, - disk=self.disk, answers=references) + disk=self.disk, answers=references, request=self.request) @dataclass(frozen=True, kw_only=True) @@ -1284,7 +1278,7 @@ def run(self, ctx: Commander) -> None: @dataclass(frozen=True, kw_only=True) -class ProblemVerb(OrderVerb, DirectiveVerb): +class ProblemVerb(RequestedOrderVerb): help = "Core records the problem with its order, or nothing" order: Annotated[ str, Argument(flags=("order",), options={"metavar": "ORDER.json"}) @@ -1299,8 +1293,17 @@ class ProblemVerb(OrderVerb, DirectiveVerb): disk: Annotated[bool, Argument(flags=("--disk",))] = False def respond(self, ctx: Commander, references: tuple[MessageReference, ...]) -> None: - ctx.problem(self.order, self.to, self.where, self.disk, - references) + document = OrderFile.read(Path(self.order).read_text()) + location = FileLine.from_wire(self.where) + checkout = ctx.repo_directory(document.order.check.directory) + if checkout.is_dir(): + ctx.comms.goals.todos.require_recurrence_declaration( + document.order, location, checkout, document.slop, time.time()) + pid = ctx.comms.goals.todos.allocate_problem() + ctx.send(self.to, document, target=NewProblem(pid, document.defect), disk=self.disk, + finding=LocatedProblemFinding(location), answers=references, + request=self.request) + print(pid) @dataclass(frozen=True, kw_only=True) diff --git a/src/agent_comms/commander/request.py b/src/agent_comms/commander/request.py index f383061e0b..7b446ec5e8 100644 --- a/src/agent_comms/commander/request.py +++ b/src/agent_comms/commander/request.py @@ -1,71 +1,67 @@ -"""What the owner asked for, recorded as the commander's own Goal so no request is forgotten. +"""Record, list and complete independent owner requests. -`request add "TEXT"` records one and prints its id; `request complete ID --why "..."` closes -it, refused while an order serving it is open; `request list` shows the open request with its -open orders. A thread holds one Goal, so one request is open at a time. Every order names the -request it serves (`order send|problem --request ID`); rotation prints REQUEST STALLED for an -open request that no open order serves. +Every request keeps its own id and serving orders. Completing one request never +replaces the commander's current goal or another request. """ from __future__ import annotations -from typing import Annotated from dataclasses import dataclass +from functools import partial +from typing import Annotated from agent_comms import field_codec +from agent_comms.commander.order import Commander, CommanderVerb +from agent_comms.message_bus import HeldBusWrite +from agent_comms.owner_requests import OwnerRequestId -from ..goal_actions import CompletedGoalAction, GoalPrecondition, SetGoalAction -from ..goal_states import ActiveGoal -from ..goals import Goal, RequestGoalOrder -from ..todos import Todo -from .order import Commander, CommanderVerb +class RequestVerb(CommanderVerb): + """A verb of `request`.""" -def open_requests(ctx: Commander) -> tuple[tuple[Goal, tuple[Todo, ...]], ...]: - """The commander's open owner requests and their undelivered orders.""" - thread = ctx.comms.registry.require(ctx.commander) - return tuple(request for goal in thread.goal.active() - for request in goal.order.open_requests(goal, ctx.comms.goals, thread.incarnation)) +class RequestMutation(HeldBusWrite, RequestVerb, declared_name=None): + """Request mutations use the same bus writer as orders and directives.""" -class RequestVerb(CommanderVerb): - """A verb of `request`.""" + def apply(self, ctx: Commander) -> None: + self.write_when_open(ctx.comms, partial(super().apply, ctx)) @dataclass(frozen=True, kw_only=True) -class RequestAddVerb(RequestVerb, declared_name="add"): - help = "record an owner request as the commander's Goal; prints its id" +class RequestAddVerb(RequestMutation, declared_name="add"): + help = "record an owner request; prints its id" text: Annotated[str, field_codec.Argument(flags=("text",))] def run(self, ctx: Commander) -> None: - goal = ctx.comms.goals.update_goal(ctx.commander, - SetGoalAction(text=self.text, order=RequestGoalOrder())) - print(goal.require().id) + commander = ctx.comms.registry.require(self.who(ctx)).incarnation + request = ctx.comms.goals.todos.add_request(commander, self.text) + print(request.id.value) @dataclass(frozen=True, kw_only=True) -class RequestCompleteVerb(RequestVerb, declared_name="complete"): - help = "close a request; refused while an order serving it is open" - id: Annotated[str, field_codec.Argument(flags=("id",))] +class RequestCompleteVerb(RequestMutation, declared_name="complete"): + help = "close a request; refused while an order serving it is pending" + id: Annotated[OwnerRequestId, field_codec.Argument(flags=("id",))] why: Annotated[str, field_codec.Argument(flags=("--why",))] def run(self, ctx: Commander) -> None: - expect = GoalPrecondition(goal_id=self.id, expected_state=ActiveGoal()) - ctx.comms.goals.update_goal(ctx.commander, - CompletedGoalAction(progress=self.why, expect=expect)) - print(f"{self.id} done") + commander = ctx.comms.registry.require(self.who(ctx)).incarnation + ctx.comms.goals.todos.complete_request(commander, self.id, self.why) + print(f"{self.id.value} done") class RequestListVerb(RequestVerb, declared_name="list"): - help = "the open request and the open orders serving it" + help = "every open request and its pending orders" def run(self, ctx: Commander) -> None: - requests = open_requests(ctx) + commander = ctx.comms.registry.require(self.who(ctx)).incarnation + store = ctx.comms.goals.todos + requests = store.open_requests(commander) if not requests: print("No open request.") - for goal, served in requests: - print(f"{goal.id} {goal.text}") - for todo in served: + for request in requests: + print(f"{request.id.value} {request.text}") + for todo in store.serving_orders(request.id): print(f" {todo.id} review {todo.review_at.clock()}: {todo.text}") diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index c4dd3f71c9..bb631e6b25 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -15,7 +15,6 @@ from agent_comms.commander_heartbeat import CommanderHeartbeat from agent_comms.comms import Comms from agent_comms.goal_actions import ReissuedOrderGoalAction -from agent_comms.goals import Goal from agent_comms.message_bus import WatchPass from agent_comms.message_page import MessageReadRequest, PageBounds from agent_comms.messages import Message @@ -35,7 +34,7 @@ from agent_comms.storage_rules import OrderCheckoutWork from agent_comms.thread_identity import ThreadIncarnation from agent_comms.threads import Thread -from agent_comms.todos import Todo +from agent_comms.todos import OwnerRequest, Todo from agent_comms.turn_digests import OrderEditCountRead, TurnDigestRead, TurnsSince from agent_comms.ui_model.unit_state import AwaitingCommander, UnitState from agent_comms.work_dispatch import ReusedWorkDispatch, WorkDispatch @@ -81,18 +80,17 @@ def line(self) -> str: @dataclass(frozen=True) class RequestStalled(IndependentStandingRule, UndatedStandingRule): - goal: Goal + request: OwnerRequest @classmethod def violations(cls, comms: Comms, commander: str) -> tuple[RequestStalled, ...]: thread = comms.registry.require(commander) - return tuple(cls(request) for goal in thread.goal.active() - for request, served in goal.order.open_requests( - goal, comms.goals, thread.incarnation) - if not served) + store = comms.goals.todos + return tuple(cls(request) for request in store.open_requests(thread.incarnation) + if not store.serving_orders(request.id)) def line(self) -> str: - return (f"REQUEST STALLED: {self.goal.id} {self.goal.text}" + return (f"REQUEST STALLED: {self.request.id.value} {self.request.text}" " -> order its next step or mark it done") @@ -191,7 +189,8 @@ def scan_observed(cls, comms: Comms, commander: str, @classmethod def read_open(cls, source: BusReader, snapshot: RegistrySnapshot, - held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str) -> tuple[Self, ...]: + held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str, + requests: Sequence[OwnerRequest]) -> tuple[Self, ...]: return tuple(cls(message, owner.name) for owner, questions in UnansweredUnitAsks.for_orders( source, snapshot, commander, held, time.time(), diff --git a/src/agent_comms/commander/vibe_check.py b/src/agent_comms/commander/vibe_check.py index ea4f0b6d40..5a415f8d77 100644 --- a/src/agent_comms/commander/vibe_check.py +++ b/src/agent_comms/commander/vibe_check.py @@ -30,7 +30,7 @@ from agent_comms.read_basis import RepliesDisplayScope from agent_comms.registry_document import RegistrySnapshot from agent_comms.thread_identity import ThreadIncarnation -from agent_comms.todos import Todo +from agent_comms.todos import OwnerRequest, Todo from agent_comms.service_sender import ServiceSender from agent_comms.standing_rules import ( IndependentStandingRule, @@ -239,7 +239,8 @@ class VibeCheckUnanswered(Escalation, OwedAnswer, UndatedStandingRule): @classmethod def read_open(cls, source: BusReader, snapshot: RegistrySnapshot, - held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str) -> tuple[Self, ...]: + held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str, + requests: Sequence[OwnerRequest]) -> tuple[Self, ...]: return tuple(cls(message) for message in VibeCheckSender.unanswered(source, snapshot, commander, time.time())) @@ -327,7 +328,8 @@ class ChokeholdUnanswered(Escalation, OwedAnswer, UndatedStandingRule): @classmethod def read_open(cls, source: BusReader, snapshot: RegistrySnapshot, - held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str) -> tuple[Self, ...]: + held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str, + requests: Sequence[OwnerRequest]) -> tuple[Self, ...]: return tuple(cls(message) for message in ChokeholdSender.unanswered(source, snapshot, commander, time.time())) diff --git a/src/agent_comms/commander_heartbeat.py b/src/agent_comms/commander_heartbeat.py index a52dc1466f..a82676fd2e 100644 --- a/src/agent_comms/commander_heartbeat.py +++ b/src/agent_comms/commander_heartbeat.py @@ -88,9 +88,7 @@ def commanders(cls, registry: Registration, goals: Goals) -> tuple[Thread, ...]: thread for thread in registry.all_threads().values() if thread.process_alive - and any( - goal.order.open_requests(goal, goals, thread.incarnation) for goal in thread.goal - ) + and goals.todos.open_requests(thread.incarnation, limit=1) ) def observe(self, comms: Comms) -> tuple[CommanderObservation, ...]: diff --git a/src/agent_comms/commander_parity/deletion.py b/src/agent_comms/commander_parity/deletion.py index 194bfeaa6c..05efd7bb45 100644 --- a/src/agent_comms/commander_parity/deletion.py +++ b/src/agent_comms/commander_parity/deletion.py @@ -20,6 +20,7 @@ from ..field_codec import FieldCodec from ..goal_actions import CreateProblemGoalAction from ..goals import CountCheckResult, OrderCheck +from ..owner_requests import NewestOpenRequest from ..progress_claims import MergedClaim from ..recorded_time import RecordedEventTime from ..repo import Repo @@ -251,8 +252,9 @@ def send(cls, run: LifecycleRun, document: OrderFile) -> Todo: repo=Repo.of(Path(document.order.check.directory)).github, text=document.defect, order=document.order, - request=document.order.request( - run.commander.comms.registry.require(run.commander.commander), ("P1",) + request=NewestOpenRequest().for_order( + run.commander.comms.goals.todos, + run.commander.incarnation, ), review_at=RecordedEventTime(time.time() + 3600), commander=run.commander.commander, diff --git a/src/agent_comms/commander_parity/delivery.py b/src/agent_comms/commander_parity/delivery.py index 19aaf01223..5ef9506afa 100644 --- a/src/agent_comms/commander_parity/delivery.py +++ b/src/agent_comms/commander_parity/delivery.py @@ -13,7 +13,7 @@ from agent_comms.change_report import FileLine from agent_comms.child_process import ChildOutcome, ChildResult, ExitedOutcome -from agent_comms.commander.order import OrderFile +from agent_comms.commander.order import OrderFile, ProblemVerb from agent_comms.commander_parity import ParityComparison from agent_comms.commander_parity.lifecycle import LifecycleRun from agent_comms.field_codec import FieldCodec @@ -153,9 +153,8 @@ def create_order(cls, run: LifecycleRun, repository: DeliveryRepository) -> Todo todo.id for todo in run.commander.comms.goals.todos.for_commander(run.commander.incarnation) } - run.commander.problem( - str(run.document), run.worker, repository.owner.to_wire() - ) + ProblemVerb(order=str(run.document), to=run.worker, + where=repository.owner.to_wire()).run(run.commander) (created,) = ( todo for todo in run.commander.comms.goals.todos.for_commander(run.commander.incarnation) diff --git a/src/agent_comms/commander_parity/handoff.py b/src/agent_comms/commander_parity/handoff.py index 76b6d431c1..7886378e09 100644 --- a/src/agent_comms/commander_parity/handoff.py +++ b/src/agent_comms/commander_parity/handoff.py @@ -66,14 +66,16 @@ def current(run: LifecycleRun, receiver: str, document: OrderFile, pid: str) -> return run.invoke( "import sys; from agent_comms.change_report import FileLine; " "from agent_comms.commander.order import Commander, OrderFile, ReassignReason; " + "from agent_comms.owner_requests import OwnerRequestId; " "Commander.model_validate_json(sys.argv[1]).send(sys.argv[2], " "OrderFile.read(sys.argv[3]), reassign=ReassignReason(sys.argv[4]), " - "traced=FileLine.from_wire(sys.argv[5]))", + "traced=FileLine.from_wire(sys.argv[5]), request=OwnerRequestId(sys.argv[6]))", run.commander.model_dump_json(), receiver, json.dumps(FieldCodec.encode(document)), "Compare the actual refusal to hand completed checked work to another real unit.", f"{Path(__file__).resolve()}:1", + run.commander.comms.goals.todos.get(pid).request.require().value, ) @staticmethod @@ -89,9 +91,7 @@ def native(run: LifecycleRun, receiver: str, document: OrderFile, pid: str) -> C order=document.order, review_at=RecordedEventTime(time.time() + 3600), commander=context.commander, - request=document.order.request( - context.comms.registry.require(context.commander), (pid,) - ), + request=todo.request.require(), traced=FileLine.from_wire(f"{Path(__file__).resolve()}:1"), ), ) diff --git a/src/agent_comms/commander_parity/lifecycle.py b/src/agent_comms/commander_parity/lifecycle.py index f773cc5428..cfd7c3603e 100644 --- a/src/agent_comms/commander_parity/lifecycle.py +++ b/src/agent_comms/commander_parity/lifecycle.py @@ -24,7 +24,7 @@ from agent_comms.check_root import CheckRoot from agent_comms.child_process import BoundedRun, ChildResult from agent_comms.commander.config import CommanderConfig -from agent_comms.commander.order import Commander, OrderFile +from agent_comms.commander.order import Commander, OrderFile, ProblemVerb from agent_comms.commander.request import RequestAddVerb from agent_comms.commander_parity import ParityComparison, UnverifiedParity from agent_comms.field_codec import FieldCodec @@ -208,7 +208,8 @@ def invoke_command(command: tuple[str, ...], **options) -> ChildResult: def create(self) -> Todo: owner = OrderFile.read(self.document.read_text()).order.owner - self.commander.problem(str(self.document), self.worker, owner.to_wire()) + ProblemVerb(order=str(self.document), to=self.worker, + where=owner.to_wire()).run(self.commander) rows = tuple(self.commander.comms.goals.todos.for_commander(self.commander.incarnation)) if len(rows) != 1: raise ValueError(f"A fresh copied workflow must create one problem, got {len(rows)}") @@ -297,7 +298,7 @@ def exercise(cls, run: LifecycleRun) -> tuple[Todo, ...]: run.worker, "The copied workflow is released before its next native assignment." ) document = replace(OrderFile.read(run.document.read_text()), problems=(before.id,)) - run.commander.send(run.worker, document, clean=True) + run.commander.send(run.worker, document, clean=True, request=before.request.require()) return before, run.commander.comms.goals.todos.get(before.id) @classmethod diff --git a/src/agent_comms/commander_parity/story.py b/src/agent_comms/commander_parity/story.py index b20249ad50..765ea55292 100644 --- a/src/agent_comms/commander_parity/story.py +++ b/src/agent_comms/commander_parity/story.py @@ -43,6 +43,7 @@ def exercise(cls, run: LifecycleRun) -> tuple[Todo, ...]: run.worker, document, clean=True, + request=created.request.require(), ) run.commander.strike(run.worker, created.id, cls.strike_reason, evidence=(created.order.checked().owner,)) @@ -53,9 +54,7 @@ def exercise(cls, run: LifecycleRun) -> tuple[Todo, ...]: order=CreateProblemGoalAction( problem="P2", order=replace(document.order, at=saved.order.checked().at), - request=run.commander.comms.registry.require(run.commander.commander) - .goal.require() - .id, + request=saved.request.require(), review_at=saved.review_at, commander=run.commander.commander, assigner=run.commander.actor_identity, diff --git a/src/agent_comms/commander_parity/wording.py b/src/agent_comms/commander_parity/wording.py index 3deb496ad6..dec85ab7ad 100644 --- a/src/agent_comms/commander_parity/wording.py +++ b/src/agent_comms/commander_parity/wording.py @@ -131,22 +131,22 @@ def agrees(current: WordingOutcome, native: WordingOutcome) -> bool: def describe(self) -> str: return super().describe() + ( f"; current={OrderFile.problems_found.__qualname__}; " - f"native_entry={OrderGoalAction.request_ref.__qualname__}" + f"native_entry={OrderGoalAction.require_ready.__qualname__}" ) @staticmethod - def native_request(context: CommanderContext, todo: Todo, text: str) -> object: + def native_request(context: CommanderContext, todo: Todo, text: str) -> None: order = replace(todo.order.checked().copy_as(PendingProblemOrder), edits=(text,)) action = OrderGoalAction( problem=todo.id, expected_revision=todo.revision, order=order, - request=context.comms.registry.require(context.commander).goal.require().id, + request=todo.request.require(), review_at=todo.review_at, commander=context.commander, assigner=context.incarnation, ) - return action.request_ref( + action.require_ready( GoalActionContext( context.comms.goals, context.comms.registry.require(todo.held().owner), diff --git a/src/agent_comms/connector.py b/src/agent_comms/connector.py index 7454bac358..36039f635b 100644 --- a/src/agent_comms/connector.py +++ b/src/agent_comms/connector.py @@ -52,6 +52,8 @@ Argument, FieldCodec, GitCommitText, MalformedRecord, PathText, WireName, WireOmitDefault, boundary_decoder, ) +from agent_comms.channel_targets import NamedTarget +from agent_comms.owner_requests import NewestOpenRequest, RequestSelection from .host_services import HostService, SystemdUserService from .connector_store import ( CODE_SECONDS, ApprovalQueueFull, ApprovalRefused, AuthorizationQuery, @@ -67,9 +69,11 @@ from .repository_name import RepositoryName from .todos import DirectiveText, ReviewedHeadVerdict from .tool_result import ToolResult -from .tool_caller import ThreadCaller +from .task_sources import OwnerRequestAttachment +from .tool_caller import ThreadCaller, ToolCaller from .tools import ( - CommsVerdictTool, ReadTool, TodoChanged, ToolRequest, tool_field, + CommsSendTool, CommsVerdictTool, MessagePublished, ReadTool, SendTool, + TodoChanged, ToolRequest, tool_field, ) LOOPBACK = "127.0.0.1" @@ -94,6 +98,18 @@ class ConnectorTool(ToolRequest[DirectorCaller], affix="ConnectorTool"): """Connector-only declarations are absent from the native/CLI tool registry.""" +@dataclass(frozen=True, kw_only=True) +class ConnectorSendTool(SendTool, ConnectorTool, declared_name=CommsSendTool.declared_name): + request: Annotated[ + RequestSelection, + *tool_field("Owner request ID; omitted selects the recipient's newest open request"), + ] = NewestOpenRequest() + + def attachment(self, comms: Comms, target: NamedTarget) -> OwnerRequestAttachment: + requests = self.request.for_message(comms.goals.todos, target, comms.registry) + return OwnerRequestAttachment(tuple(request.id for request in requests)) + + @dataclass(frozen=True, kw_only=True) class ConnectorVerdictTool(ConnectorTool, declared_name=CommsVerdictTool.declared_name): label = "Review PR head" diff --git a/src/agent_comms/errors.py b/src/agent_comms/errors.py index eca99d2acc..489c47cd71 100644 --- a/src/agent_comms/errors.py +++ b/src/agent_comms/errors.py @@ -64,6 +64,10 @@ class HumanSendError(RelationViolationError): class HumanSendRefusedError(ToolRefusedError, HumanSendError): """The send transaction rolled back; the human's message was not sent.""" + def __init__(self, cause: Exception) -> None: + self.cause = cause + super().__init__(f"Message was not sent: {cause}") + class CommittedMessageError(RuntimeError): """The message was sent; signalling or connection retirement then failed.""" diff --git a/src/agent_comms/goal_actions.py b/src/agent_comms/goal_actions.py index 0741d5229a..6ae4a7902e 100644 --- a/src/agent_comms/goal_actions.py +++ b/src/agent_comms/goal_actions.py @@ -19,12 +19,11 @@ NoGoal, ProblemGoalOrder, ReportedTurn, - RequestDirective, - RequestGoalOrder, UnobservedGoalCheckpoint, UnorderedGoalOrder, ) from agent_comms.input_disposition import InputDispositions +from agent_comms.todos import PendingProblemOrder, SendBackVerdict, Todo, TodoOperation from .change_report import FileLine, NoSourceReference from .child_process import ProcessIdentity @@ -44,12 +43,12 @@ ) from .goal_waits import GoalWaits, RecordedGoalWait from .lifecycle_capability import Settled +from .owner_requests import OwnerRequestId from .progress_claims import ProgressClaim 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 if TYPE_CHECKING: from .goal_management import Goals @@ -489,34 +488,11 @@ def create(self, ctx: GoalActionContext) -> Goal: return goal -@dataclass(frozen=True, kw_only=True) -class DirectiveGoalAction(GoalAction[GoalDeclaration], RuntimeInvocable): - """An owner-voice message to a thread that holds owner requests becomes one of its requests.""" - - text: str - seq: int - at: float - - def change(self, goal: Goal, ctx: GoalActionContext) -> Goal: - return goal.order.directed(goal, self, ctx) - - def create(self, ctx: GoalActionContext) -> GoalDeclaration: - return ctx.thread.goal - - def directive(self) -> RequestDirective: - return RequestDirective(self.seq, self.at + RequestDirective.window) - - def request(self, goal: Goal, ctx: GoalActionContext) -> Goal: - ReplacementGoalAction.retire(goal, ctx) - order = RequestGoalOrder((self.directive(),)) - return SetGoalAction(text=self.text, order=order).change(goal, ctx) - - @dataclass(frozen=True, kw_only=True) class ProblemOrderGoalAction(GoalAction[Goal], RuntimeInvocable): problem: str order: PendingProblemOrder - request: str + request: OwnerRequestId review_at: EventTime commander: str assigner: ThreadIncarnation @@ -527,14 +503,14 @@ def bound_to( return replace(self, commander=commander.name, assigner=actor) @abstractmethod - def reserve(self, ctx: GoalActionContext, generation: str, goal: GoalRef) -> Todo: ... + def reserve(self, ctx: GoalActionContext, generation: str) -> Todo: ... def change(self, goal: Goal, ctx: GoalActionContext) -> Goal: return goal.state.accept_order(self, goal, ctx) def continue_goal(self, goal: Goal, ctx: GoalActionContext) -> Goal: - request = self.request_ref(ctx) - self.reserve(ctx, goal.id, request) + self.require_ready(ctx) + self.reserve(ctx, goal.id) store = ctx.goals.open_goal_store() generation = store.require_generation(goal.id) generation.lifecycle.continue_work(store, generation, self.order.id) @@ -544,14 +520,12 @@ def continue_goal(self, goal: Goal, ctx: GoalActionContext) -> Goal: ProblemGoalOrder.text, goal.id, revision, ctx.thread, ctx.goals.registry.snapshot())) - def request_ref(self, ctx: GoalActionContext) -> GoalRef: - """Refuse invalid orders before reserving their Todo.""" + def require_ready(self, ctx: GoalActionContext) -> None: + """Refuse an invalid request before goal reservation or allocating a child.""" self.require_sendable() commander = ctx.goals.registry.require(self.commander) - commander.goal.require().require_id(self.request).require_active().order.require_request() + ctx.goals.todos.owner_request(self.request).require_open(commander.incarnation) self.review_at.require_recorded("A review time") - request = GoalRef(commander.name, commander.created_at, self.request) - return request def require_sendable(self) -> None: """A new order's wording and source references hold at its declared revision.""" @@ -560,9 +534,9 @@ def require_sendable(self) -> None: self.order.check.require_directory() def create(self, ctx: GoalActionContext) -> Goal: - request = self.request_ref(ctx) + self.require_ready(ctx) fresh = SetGoalAction(text=ProblemGoalOrder.text, order=ProblemGoalOrder()) - self.reserve(ctx, fresh.goal_id, request) + self.reserve(ctx, fresh.goal_id) return fresh.create(ctx) @@ -574,10 +548,10 @@ class OrderGoalAction(ProblemOrderGoalAction): traced: Annotated[FileLine | NoSourceReference, field_codec.WireOmitDefault()] = ( NoSourceReference() ) - def reserve(self, ctx: GoalActionContext, generation: str, goal: GoalRef) -> Todo: + def reserve(self, ctx: GoalActionContext, generation: str) -> Todo: return ctx.goals.todos.assign(self.problem, expected_revision=self.expected_revision, owner=ctx.thread, assigner=self.assigner, - generation=generation, goal=goal, review_at=self.review_at, order=self.order, + generation=generation, request=self.request, review_at=self.review_at, order=self.order, traced=self.traced) @@ -597,7 +571,7 @@ def for_todo(cls, todo: Todo, commander: str) -> Self: order = todo.order.reissued(now) return cls( problem=todo.id, expected_revision=todo.revision, - order=order, request=todo.goal.request(), + order=order, request=todo.request.require(), review_at=todo.reissued_review(now), commander=commander, assigner=todo.acting_assigner, traced=order.owner) @@ -611,10 +585,10 @@ class CreateProblemGoalAction(ProblemOrderGoalAction): repo: RepositoryName text: str - def reserve(self, ctx: GoalActionContext, generation: str, goal: GoalRef) -> Todo: + def reserve(self, ctx: GoalActionContext, generation: str) -> Todo: return ctx.goals.todos.create(self.problem, self.repo, self.text, creator=ctx.goals.registry.require(self.commander), owner=ctx.thread, - assigner=self.assigner, generation=generation, goal=goal, + assigner=self.assigner, generation=generation, request=self.request, review_at=self.review_at, order=self.order) diff --git a/src/agent_comms/goal_history.py b/src/agent_comms/goal_history.py index 4d2f177bcc..ddffceb111 100644 --- a/src/agent_comms/goal_history.py +++ b/src/agent_comms/goal_history.py @@ -8,16 +8,19 @@ from __future__ import annotations +import json import os import sqlite3 import stat import time from collections.abc import Iterator from contextlib import closing, contextmanager -from dataclasses import dataclass, field +from dataclasses import dataclass, field, fields from pathlib import Path from typing import Annotated, Literal +from agent_comms.field_codec import WireOmitDefault +from agent_comms.owner_requests import NoRequestRevision, RequestAtRevision, RequestRevision from .field_codec import FieldCodec, MalformedRecord, HistoryExclude from .goals import GoalDeclaration, NoGoal @@ -31,26 +34,42 @@ class GoalHistoryError(RuntimeError): """Goal history could not be durably written or reconciled.""" +@dataclass(frozen=True, kw_only=True) +class RequestHistory: + """Moved request data stays linked to its original history observation.""" + + request_before: Annotated[ + RequestRevision, Column(storage=JsonStorage, nullable=True), WireOmitDefault(), + ] = NoRequestRevision() + request_after: Annotated[ + RequestRevision, Column(storage=JsonStorage, nullable=True), WireOmitDefault(), + ] = NoRequestRevision() + + @classmethod + def endpoint_columns(cls) -> dict[str, str]: + """Request-prefix fields declare the goal endpoint each preserves.""" + return {entry.name.removeprefix("request_"): entry.name for entry in fields(RequestHistory)} + + @property + def request_revisions(self) -> tuple[RequestRevision, ...]: + return tuple(getattr(self, entry.name) for entry in fields(RequestHistory)) + + @dataclass(frozen=True) -class RecordedGoalRevision: +class RecordedGoalRevision(RequestHistory): """A history entry's public fields; its owner key and journal state stay in the store.""" - sequence: int + sequence: Annotated[int, Column(primary_key=True)] kind: Literal["transition", "baseline", "observed_gap"] observed_at: float - before: GoalDeclaration - after: GoalDeclaration + before: Annotated[GoalDeclaration, Column(storage=JsonStorage, nullable=True)] + after: Annotated[GoalDeclaration, Column(storage=JsonStorage, nullable=True)] @dataclass(frozen=True) -class GoalHistoryEntry(TypedTable, DurableTables): +class GoalHistoryEntry(RecordedGoalRevision, TypedTable, DurableTables): # The durable kind field is a journal fact, not the TypedTable family tag. schema_error = GoalHistoryError - sequence: Annotated[int, Column(primary_key=True)] - kind: Literal["transition", "baseline", "observed_gap"] - observed_at: float - before: Annotated[GoalDeclaration, Column(storage=JsonStorage, nullable=True)] - after: Annotated[GoalDeclaration, Column(storage=JsonStorage, nullable=True)] owner_created_at: Annotated[float, HistoryExclude()] = field( kw_only=True ) @@ -59,13 +78,43 @@ class GoalHistoryEntry(TypedTable, DurableTables): ] = field(kw_only=True) indexes = (Index(("owner_created_at", "sequence")),) + @classmethod + def orders_with_replacement( + cls, db: sqlite3.Connection, column: str, replacement: str, *, + order_kind: str, where: str = "1", + ) -> sqlite3.Cursor: + """Read matching endpoint orders through this table's column declaration.""" + selected = cls._column_list((column,)) + return db.execute( + f'SELECT {cls._column_list(("sequence", "observed_at", "owner_created_at"))}, ' + f"json_set({selected}, '$.order', json(?)) AS goal, " + f"coalesce(json_extract({selected}, '$.order.directives'), '[]') AS directives " + f'FROM "{cls.declared_name}" ' + f"WHERE json_extract({selected}, '$.order.kind')=? AND ({where}) " + f'ORDER BY {cls._column_list(("sequence",))}', (replacement, order_kind)) + + @classmethod + def move_request_order( + cls, db: sqlite3.Connection, column: str, sequence: int, + request: RequestAtRevision, replacement: str, + ) -> None: + """Move an endpoint's request to its prefix without dropping the goal observation.""" + goal_column = cls._column_list((column,)) + prefix_column = cls._column_list((cls.endpoint_columns()[column],)) + db.execute( + f'UPDATE "{cls.declared_name}" ' + f"SET {goal_column}=json_set({goal_column}, '$.order', json(?)), {prefix_column}=? " + f'WHERE {cls._column_list(("sequence",))}=?', + (replacement, json.dumps(FieldCodec.encode(request)), sequence)) + def reports_turn(self, turn_id: str) -> bool: """A committed goal revision records its original model reporting turn.""" return self.after.reports_turn(turn_id) def recorded(self) -> RecordedGoalRevision: - return RecordedGoalRevision( - self.sequence, self.kind, self.observed_at, self.before, self.after) + return FieldCodec.construct(RecordedGoalRevision, { + entry.name: getattr(self, entry.name) for entry in fields(RecordedGoalRevision) + }) def to_wire(self) -> dict[str, object]: return FieldCodec.encode_record(self.recorded()) diff --git a/src/agent_comms/goal_management.py b/src/agent_comms/goal_management.py index 834339e400..3f503c998f 100644 --- a/src/agent_comms/goal_management.py +++ b/src/agent_comms/goal_management.py @@ -16,7 +16,6 @@ from .coordination_errors import StaleFence from .errors import RelationViolationError from .goal_actions import ( - DirectiveGoalAction, GoalAction, GoalActionContext, GoalPrecondition, @@ -33,6 +32,7 @@ StandbyReview, ) from .native_input_owner import GoalLaunchOwner +from .owner_requests import NewestOpenRequest, RequestDirective from .registration import Registration if TYPE_CHECKING: @@ -416,9 +416,11 @@ def _consume_reply_wait(self, goal: Goal, current: Thread, reference: MessageRef return wait.consume_reply(self.waits, goal.id) def direct(self, name: str, message: Message) -> None: - """The owner's plain message to ``name`` becomes a directive on its goal.""" - self.update_goal(name, DirectiveGoalAction( - text=message.body, seq=message.seq, at=message.timestamp)) + """A plain owner message directs the recipient's newest open request.""" + owner = self.registry.require(name).incarnation + directive = RequestDirective.from_message(message) + for request in NewestOpenRequest().select(self.todos, owner): + self.todos.record_directive(request.id, directive) def update_goal[Result: GoalDeclaration]( self, diff --git a/src/agent_comms/goals.py b/src/agent_comms/goals.py index 3905510f00..83266488b0 100644 --- a/src/agent_comms/goals.py +++ b/src/agent_comms/goals.py @@ -2,7 +2,7 @@ from __future__ import annotations -from agent_comms.lifecycle_capability import Active, Settled +from agent_comms.lifecycle_capability import Active import json import os @@ -11,10 +11,9 @@ import signal import subprocess import tempfile -import time from abc import abstractmethod from collections.abc import Iterator, Sequence -from dataclasses import dataclass, field, replace +from dataclasses import dataclass, field from pathlib import Path from typing import TYPE_CHECKING, ClassVar, Literal @@ -37,8 +36,7 @@ from .ui_model.goal import GoalDisplay if TYPE_CHECKING: - from .bus import BusReader - from .goal_actions import DirectiveGoalAction, GoalAction, GoalActionContext + from .goal_actions import GoalAction, GoalActionContext from .goal_presentation import GoalExecution from .goal_waits import GoalWaits from .task_sources import TaskScope @@ -51,7 +49,6 @@ from .turn_goal_account import VerifiedGoalSettlement from .thread_identity import TurnId from .todos import Todo - from .messages import Message @dataclass(frozen=True, kw_only=True) @@ -481,14 +478,6 @@ class GoalOrder(DeclaredFamily, affix="GoalOrder"): def require_completion(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None: pass - def require_request(self) -> None: - raise ValueError("An order serves an open owner request: request add records one.") - - def open_requests( - self, goal: Goal, goals: Goals, owner: ThreadIncarnation - ) -> tuple[tuple[Goal, tuple[Todo, ...]], ...]: - return () - def instructions(self, todos: Sequence[Todo]) -> str: return "" @@ -504,15 +493,6 @@ def progress(self, goal: Goal, todos: Sequence[Todo]) -> str: def require_model_block(self) -> None: pass - def directed(self, goal: Goal, directive: DirectiveGoalAction, ctx: GoalActionContext) -> Goal: - """Only a thread holding owner requests records an owner directive.""" - return goal - - def directive_messages( - self, source: BusReader, responders: frozenset[str], - ) -> tuple[tuple[RequestDirective, Message], ...]: - return () - @abstractmethod def require_order(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None: ... @@ -533,69 +513,6 @@ def require_replacement(self, goal: Goal, goals: Goals, owner: ThreadIncarnation pass -@dataclass(frozen=True) -class RequestDirective: - """One original directive; only a reply linked to that message answers it.""" - - window: ClassVar[int] = 10 * 60 # seconds the commander has to act on a directive - seq: int - deadline: float - - @property - def due_time(self) -> str: - return time.strftime("%H:%M", time.localtime(self.deadline)) - - def original(self, source: BusReader) -> Message: - return source.lookup(self.seq) - - -@dataclass(frozen=True) -class RequestGoalOrder(GoalOrder): - """An owner request on the commander's own Goal: each order names the one it serves, and the - request stays open until no order serving it is open. Each directive the commander receives - from the owner or claude-ai is recorded on it when it arrives.""" - - directives: tuple[RequestDirective, ...] = () - - def require_order(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None: - pass - - def require_replacement(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None: - if not goal.state.items_with(Settled, goal): - raise ValueError(f"Request {goal.id} is still open; mark it done first.") - - def require_completion(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None: - if self.serving(goal, goals, owner): - raise ValueError(f"Request {goal.id} still has open orders.") - - def require_request(self) -> None: - pass - - def serving(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> tuple[Todo, ...]: - return tuple(todo for todo in goals.todos.for_request(owner, goal.id) if todo.pending) - - def directed(self, goal: Goal, directive: DirectiveGoalAction, ctx: GoalActionContext) -> Goal: - """A closed request is replaced by one the directive opens; an open one records it.""" - if not goal.state.items_with(Settled, goal): - return replace(goal, revision=goal.revision + 1, order=replace( - self, directives=(*self.directives, directive.directive()))) - return directive.request(goal, ctx) - - def directive_messages( - self, source: BusReader, responders: frozenset[str], - ) -> tuple[tuple[RequestDirective, Message], ...]: - """Only still-unanswered directives load originals; replies stay in the index.""" - unanswered = {reference.seq for reference in source.unanswered_references( - tuple(directive.seq for directive in self.directives), responders)} - return tuple((directive, directive.original(source)) for directive in self.directives - if directive.seq in unanswered) - - def open_requests( - self, goal: Goal, goals: Goals, owner: ThreadIncarnation - ) -> tuple[tuple[Goal, tuple[Todo, ...]], ...]: - return ((goal, self.serving(goal, goals, owner)),) - - @dataclass(frozen=True) class ProblemGoalOrder(GoalOrder): """A unit's one Goal groups its canonical Todo assignments, not copied orders.""" diff --git a/src/agent_comms/installer/carry.py b/src/agent_comms/installer/carry.py index 3c7e29a324..700ccee28d 100644 --- a/src/agent_comms/installer/carry.py +++ b/src/agent_comms/installer/carry.py @@ -4,8 +4,9 @@ subclass. Only ``agent-comms install`` runs it, with the target revision's ``src`` and ``tools``, so every step runs against the source it is installed with. A step is idempotent: data it already carried reports no change, so a step's own dry run is the -one owner of whether a bus still needs it. Steps run in the order the main line added -them, from the first whose dry run reports a change. An edited step is judged by what +one owner of whether a bus still needs it. Declared dependencies constrain the order +the main line added steps, starting at the first whose dry run reports a change. +An edited step is judged by what it does now, never by its name. A finished step is retired by deleting its file once every bus has run it. """ @@ -17,6 +18,7 @@ from collections.abc import Callable from dataclasses import dataclass, field from importlib import import_module +from graphlib import TopologicalSorter from pathlib import Path from typing import Annotated, ClassVar @@ -47,6 +49,8 @@ class Carry(ABC): spill_dir: Annotated[Path, PathText] declared: ClassVar[dict[str, type[Carry]]] = {} + precedes: ClassVar[tuple[type[Carry], ...]] = () + """Steps whose decoders require this step's output, regardless of addition date.""" """Every step imported so far, by its module name.""" def __init_subclass__(cls, **kwargs: object) -> None: @@ -110,7 +114,7 @@ def run(self, step: Carry) -> Carried: @dataclass(frozen=True) class CarryChain: - """Every step a tree declares, in the order the main line added them.""" + """Every declared step, dependency order first and addition order for independent steps.""" steps: tuple[type[Carry], ...] @@ -130,7 +134,13 @@ def at(cls, tree: Path) -> CarryChain: unordered = sorted(step.name() for step in steps if step.name() not in added) if unordered: raise ValueError(f"{unordered} are not on the main line, so nothing orders them") - return cls(tuple(sorted(steps, key=lambda step: added[step.name()]))) + ordered = sorted(steps, key=lambda step: added[step.name()]) + dependencies = {step: set() for step in ordered} + for step in ordered: + for later in step.precedes: + if later in dependencies: + dependencies[later].add(step) + return cls(tuple(TopologicalSorter(dependencies).static_order())) def pending(self, dry_run: Callable[[type[Carry]], Carried]) -> tuple[type[Carry], ...]: """Every step from the first whose dry run reports a change. A later step's dry run diff --git a/src/agent_comms/owner_requests.py b/src/agent_comms/owner_requests.py new file mode 100644 index 0000000000..63e67267ca --- /dev/null +++ b/src/agent_comms/owner_requests.py @@ -0,0 +1,187 @@ +"""Owner request identities, lifecycle, and original directive references.""" + +from __future__ import annotations + +import time +from abc import abstractmethod +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from typing import TYPE_CHECKING, Annotated, ClassVar, Self +from uuid import uuid4 + +from agent_comms.declared_family import DeclaredFamily +from agent_comms.errors import RelationViolationError +from agent_comms.field_codec import ( + BoundedText, + NullMember, + RecordMember, + SingleFieldValue, + SubclassUnion, +) + +if TYPE_CHECKING: + from agent_comms.channel_targets import NamedTarget + from agent_comms.messages import Message + from agent_comms.registration import Registration + from agent_comms.thread_identity import ThreadIncarnation + from agent_comms.todos import OwnerRequest, Todo, TodoStore + from agent_comms.bus import BusReader + + +class RequestValue(SubclassUnion, affix=""): + """One id can be a stored link and an explicitly selected request.""" + + +class RequestLink(RequestValue, declared_name=None): + @abstractmethod + def require(self) -> OwnerRequestId: ... + + +class RequestSelection(RequestValue, declared_name=None): + def for_message( + self, + store: TodoStore, + target: NamedTarget, + registry: Registration, + ) -> tuple[OwnerRequest, ...]: + return tuple( + request + for name in target.direct_names + for request in self.select(store, registry.require(name).incarnation) + ) + + def for_order(self, store: TodoStore, commander: ThreadIncarnation) -> OwnerRequestId: + selected = self.select(store, commander) + if len(selected) != 1: + raise RelationViolationError("An order needs exactly one open owner request.") + return selected[0].id + + @abstractmethod + def select( + self, + store: TodoStore, + commander: ThreadIncarnation, + ) -> tuple[OwnerRequest, ...]: ... + + +@dataclass(frozen=True, slots=True) +class OwnerRequestId(SingleFieldValue, RequestLink, RequestSelection): + value: Annotated[str, BoundedText] + + @classmethod + def new(cls) -> Self: + return cls(uuid4().hex) + + def require(self) -> Self: + return self + + def for_message( + self, + store: TodoStore, + target: NamedTarget, + registry: Registration, + ) -> tuple[OwnerRequest, ...]: + return self.select(store, registry.require(target.require_dm_peer()).incarnation) + + def select( + self, + store: TodoStore, + commander: ThreadIncarnation, + ) -> tuple[OwnerRequest, ...]: + request = store.owner_request(self) + request.require_open(commander) + return (request,) + + +@dataclass(frozen=True) +class NoRequestLink(NullMember, RequestLink): + def require(self) -> OwnerRequestId: + raise RelationViolationError("An unordered problem has no owner request.") + + +@dataclass(frozen=True) +class NewestOpenRequest(NullMember, RequestSelection): + def select( + self, + store: TodoStore, + commander: ThreadIncarnation, + ) -> tuple[OwnerRequest, ...]: + return store.open_requests(commander, limit=1) + + +class RequestState(DeclaredFamily, affix="RequestState"): + def require_open(self, request: OwnerRequestId) -> OpenRequestState: + raise ValueError(f"Request {request.value} is already completed.") + + +@dataclass(frozen=True) +class OpenRequestState(RequestState): + def require_open(self, request: OwnerRequestId) -> Self: + return self + + def complete( + self, + request: OwnerRequestId, + orders: Sequence[Todo], + reason: str, + ) -> CompletedRequestState: + if orders: + raise ValueError(f"Request {request.value} still has pending orders.") + return CompletedRequestState(reason) + + +@dataclass(frozen=True) +class CompletedRequestState(RequestState): + reason: str + + def __post_init__(self) -> None: + if not self.reason.strip(): + raise ValueError("Completing an owner request requires a reason.") + + +class RequestRevision(SubclassUnion, affix="RequestRevision"): + @abstractmethod + def validate(self, requests: Mapping[OwnerRequestId, OwnerRequest]) -> None: ... + + +@dataclass(frozen=True) +class NoRequestRevision(NullMember, RequestRevision): + def validate(self, requests: Mapping[OwnerRequestId, OwnerRequest]) -> None: + pass + + +@dataclass(frozen=True) +class RequestAtRevision(RecordMember, RequestRevision): + """A historical directive prefix, not another writable request state.""" + + id: OwnerRequestId + directive_count: int + + def __post_init__(self) -> None: + if self.directive_count < 0: + raise ValueError("A directive prefix cannot have negative length.") + + def validate(self, requests: Mapping[OwnerRequestId, OwnerRequest]) -> None: + request = requests[self.id] + if self.directive_count > len(request.directives): + raise RelationViolationError("Request history exceeds its recorded directives.") + + +@dataclass(frozen=True) +class RequestDirective: + """One original directive; only a reply linked to that message answers it.""" + + window: ClassVar[int] = 10 * 60 + seq: int + deadline: float + + @property + def due_time(self) -> str: + return time.strftime("%H:%M", time.localtime(self.deadline)) + + @classmethod + def from_message(cls, message: Message) -> Self: + return cls(message.seq, message.timestamp + cls.window) + + def original(self, source: BusReader) -> Message: + return source.lookup(self.seq) diff --git a/src/agent_comms/pi_payloads.py b/src/agent_comms/pi_payloads.py index 30e036524e..e31d120fdd 100644 --- a/src/agent_comms/pi_payloads.py +++ b/src/agent_comms/pi_payloads.py @@ -203,6 +203,8 @@ def decode_fields( # Native null means omitted where the declaration names its not-reported member. elif key in value and not (value[key] is None and isinstance(f.default, PiPayload)): decoded[f.name] = FieldCodec.decode_external(hints[f.name], value[key], value) + elif (plan := FieldCodec.field_plan(cls, f.name)).constructor_required or plan.required: + raise MalformedRecord(f"Missing required field {key}") return decoded @classmethod diff --git a/src/agent_comms/registration.py b/src/agent_comms/registration.py index 061b5d0a3c..4034bef36e 100644 --- a/src/agent_comms/registration.py +++ b/src/agent_comms/registration.py @@ -13,7 +13,6 @@ from .errors import RelationViolationError from .goal_actions import GoalActionContext, SetGoalAction from .goal_history import GoalHistoryEntry, GoalHistoryStore -from .goals import RequestGoalOrder from .maintenance_barrier import MaintenanceBarrier from .native_input_owner import RegistryOwner from .private_registry_guard import PrivateRegistryGuard @@ -94,7 +93,7 @@ def prepare_native_root(self) -> PrivateRegistryGuard: return guard def register_commander_child(self, name: str, parent_name: str, goals: Goals) -> Thread: - """Declare or resume one child and its own request under the registry's lock.""" + """Declare or resume one child and its own goal under the registry's lock.""" with self.store.editing() as edit: parent = edit.document.require(parent_name) if edit.document.canonical_name(name) != name: @@ -109,10 +108,10 @@ def register_commander_child(self, name: str, parent_name: str, goals: Goals) -> parent=ForkedParent(parent.name), model=parent.model, thinking_level=parent.thinking_level, service_tier=parent.service_tier, execution=CommanderForkExecution) - request = ('\n'.join(goal.text for goal in parent.goal.active()) - or f'Coordinate assigned work for {parent.name}.') + text = ('\n'.join(goal.text for goal in parent.goal.active()) + or f'Coordinate assigned work for {parent.name}.') with self._preparing_declaration(edit, child, _RUNNING_STATUS) as change: - goal = SetGoalAction(text=request, order=RequestGoalOrder()).create( + goal = SetGoalAction(text=text).create( GoalActionContext(goals, change.thread)) return self._commit_registration(edit, replace( change, thread=replace(change.thread, goal=goal))).thread diff --git a/src/agent_comms/standing_rules.py b/src/agent_comms/standing_rules.py index 63754a05c4..ea7d78b04d 100644 --- a/src/agent_comms/standing_rules.py +++ b/src/agent_comms/standing_rules.py @@ -24,12 +24,12 @@ if TYPE_CHECKING: from agent_comms.bus import BusReader from agent_comms.comms import Comms - from agent_comms.goals import RequestDirective + from agent_comms.owner_requests import OwnerRequestId, RequestDirective from agent_comms.messages import Message from agent_comms.registry_document import RegistrySnapshot from agent_comms.storage_rules import MemoryUse, StorageRule from agent_comms.thread_identity import ThreadIncarnation - from agent_comms.todos import Todo + from agent_comms.todos import OwnerRequest, Todo from agent_comms.ui_model.unit_state import UnitState @@ -172,7 +172,8 @@ def cited(self) -> str: @classmethod @abstractmethod def read_open(cls, source: BusReader, snapshot: RegistrySnapshot, - held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str) -> tuple[Self, ...]: + held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str, + requests: Sequence[OwnerRequest]) -> tuple[Self, ...]: """This member's open debts, inside their window or past it, on one bus cut.""" def due(self, now: float) -> bool: @@ -185,13 +186,14 @@ def violations(cls, comms: Comms, commander: str) -> tuple[Self, ...]: @classmethod def opened(cls, comms: Comms, commander: str) -> tuple[Self, ...]: - # Orders and goal directives name already-committed messages. Capture them + # Orders and request directives name already-committed messages. Capture them # before the bus cut so every recorded reference exists in that cut. held = cls.holders(comms, commander) snapshot = comms.registry.snapshot() + requests = comms.goals.todos.open_requests(snapshot.require(commander).incarnation) with comms.bus.bus.reading() as source: return tuple(rule for member in cls.members_with(cls) - for rule in member.read_open(source, snapshot, held, commander)) + for rule in member.read_open(source, snapshot, held, commander, requests)) def answered_by(self, named: tuple[Message, ...]) -> bool: """Whether an action naming these open messages answers this one.""" @@ -328,17 +330,17 @@ def line(self) -> str: class DirectiveUnacted(OwedAnswer, DatedStandingRule): """An owner directive recorded on an open request, past its deadline with no order since.""" - request: str + request: OwnerRequestId directive: RequestDirective @classmethod def read_open(cls, source: BusReader, snapshot: RegistrySnapshot, - held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str) -> tuple[Self, ...]: - thread = snapshot.require(commander) + held: Mapping[ThreadIncarnation, Sequence[Todo]], commander: str, + requests: Sequence[OwnerRequest]) -> tuple[Self, ...]: return tuple( - cls(original, goal.id, directive) - for goal in thread.goal.active() - for directive, original in goal.order.directive_messages(source, snapshot.names_for(commander)) + cls(original, request.id, directive) + for request in requests + for directive, original in request.directive_messages(source, snapshot.names_for(commander)) ) def due(self, now: float) -> bool: @@ -350,7 +352,7 @@ def began(self) -> float: def line(self) -> str: return ( f"DIRECTIVE UNACTED: owner directive {self.cited}" - f" on request {self.request}," + f" on request {self.request.value}," f" due {self.directive.due_time} -> order its next step or mark the request done" ) @@ -361,7 +363,7 @@ def heartbeat_line(self, current: Sequence[Self]) -> str: oldest = min(current, key=lambda rule: rule.directive.seq).directive newest = max(current, key=lambda rule: rule.directive.seq).directive return ( - f"DIRECTIVE UNACTED: request {self.request}: {len(current)} owner directives; " + f"DIRECTIVE UNACTED: request {self.request.value}: {len(current)} owner directives; " f"oldest {oldest.seq} due {oldest.due_time}; " f"newest {newest.seq} due {newest.due_time} " "-> order the next step or mark the request done" diff --git a/src/agent_comms/task_sources.py b/src/agent_comms/task_sources.py index f0b3d3aa71..6681d940d3 100644 --- a/src/agent_comms/task_sources.py +++ b/src/agent_comms/task_sources.py @@ -12,6 +12,7 @@ from .field_codec import FieldCodec, WireOmitDefault from .goals import GoalDeclaration, GoalRevision from .message_reference import MessageReference +from .owner_requests import OwnerRequestId, RequestDirective from .thread_identity import ( RecordedThreadIncarnation, ThreadIncarnation, @@ -272,6 +273,18 @@ def direct(self, goals: Goals, message: Message) -> None: CommandTarget.named(message.target).direct(goals, message) +@dataclass(frozen=True) +class OwnerRequestAttachment(TaskAttachment): + """The selected requests stay bound to their original committed directive.""" + + requests: tuple[OwnerRequestId, ...] + + def direct(self, goals: Goals, message: Message) -> None: + directive = RequestDirective.from_message(message) + for request in self.requests: + goals.todos.record_directive(request, directive) + + @dataclass(frozen=True) class UserTaskSupersession(TaskAttachment, declared_name="user_supersession"): """The human's exact correction stays on its original enclosing Message. diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 5aa681d9a8..aa9e5660ac 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -44,7 +44,7 @@ SourceReference, SourceRevision, ) -from agent_comms.channel_targets import CommandTarget, CommsTag +from agent_comms.channel_targets import CommandTarget, CommsTag, NamedTarget from agent_comms.child_process import ( BoundedRun, CheckPrinted, @@ -61,7 +61,6 @@ GitCommitText, MalformedRecord, NullMember, - RecordMember, SingleFieldValue, SubclassUnion, WireOmitDefault, @@ -76,6 +75,14 @@ from agent_comms.messages import InfoKind, Message from agent_comms.order_runs import OrderRun from agent_comms.order_story import ProblemStory, StoryEvent, StoryLine, StorySource +from agent_comms.owner_requests import ( + OpenRequestState, + OwnerRequestId, + RequestAtRevision, + RequestDirective, + RequestLink, + RequestState, +) from agent_comms.plan_phases import OrderPhase, UnplannedPhase from agent_comms.problem_number import ProblemNumber from agent_comms.progress_claims import MergedClaim, ProgressClaim, WorkingClaim @@ -96,9 +103,11 @@ from agent_comms.typed_table import ( Column, DurableStore, + DurableTables, ExpressionIndex, + Generated, Index, - JsonStorage, + SharedFileTables, TextStorage, TypedTable, ) @@ -1604,15 +1613,6 @@ def folded(self, into: str, why: str) -> ProblemOrder: raise ValueError("The problem has open orders; link or drop them first.") return FoldedProblemOrder(original=self, into=into, why=why) - @staticmethod - def request(sender: Thread, problems: tuple[str, ...]) -> str: - if not problems: - raise ValueError('An order needs its Problems line.') - request = sender.goal.require().require_active() - request.order.require_request() - return request.id - - @dataclass(frozen=True, kw_only=True) class UnorderedProblemOrder(ProblemOrder): @@ -2097,11 +2097,12 @@ def originals[K]( @classmethod def for_order( cls, todo: Todo, actor: ThreadIncarnation, number: int, head: str, - report: str, answer: MessageReference, + report: str, answer: MessageReference, *, store: TodoStore, ) -> OrderClaim: + request = store.owner_request(todo.request.require()) return cls( PullRequestLink(todo.repo, number), head, - Message(actor.name, todo.creator, report, InfoKind, answers=(answer,)), + Message(actor.name, request.commander.name, report, InfoKind, answers=(answer,)), ) @@ -2336,53 +2337,6 @@ def __init__(self, current: Todo): super().__init__(f"Todo {current.id} changed (revision {current.revision}, owner {owner}).") -class GoalReference(SubclassUnion): - @abstractmethod - def request(self) -> str: - """The commander's request goal this problem serves.""" - - def require_reference(self) -> None: - pass - - def threads(self) -> tuple[str, ...]: - return () - - -@dataclass(frozen=True) -class NoGoalReference(NullMember, GoalReference): - def request(self) -> str: - raise TodoError("A problem with no request goal cannot be ordered again.") - - -@dataclass(frozen=True) -class GoalRef(RecordMember, GoalReference): - - owner: str - owner_created: float - goal_id: str - - def __post_init__(self) -> None: - self.require_reference() - - def require_reference(self) -> None: - _text(self.owner, "Thread name", 128) - try: - self.incarnation.require_recorded() - except ValueError as error: - raise TodoError("Goal reference requires its recorded owner incarnation") from error - _text(self.goal_id, "Goal ID", 128) - - @property - def incarnation(self) -> ThreadIncarnation: - return RecordedThreadIncarnation(self.owner, self.owner_created) - - def threads(self) -> tuple[str, ...]: - return (self.owner,) - - def request(self) -> str: - return self.goal_id - - class TodoHolder(ABC): """Current, released and absent holders own their assignment behaviour.""" @@ -2578,8 +2532,8 @@ def record(self, goals: Goals, problem: str) -> Todo: def after_record(self, goals: Goals, changed: Todo) -> None: """Most operations have no automatic public effect.""" - def require_unit(self, db: sqlite3.Connection, current: Todo) -> None: - """Only assignment operations compete for a unit.""" + def validate(self, db: sqlite3.Connection, current: Todo) -> None: + """Validate related records inside the operation's write transaction.""" def apply(self, current: Todo) -> Todo: self.require_current(current) @@ -2607,29 +2561,30 @@ class AssignTodo(TodoOperation): proposed: Assignment review_at: EventTime order: PendingProblemOrder | MergedProblemOrder - goal: GoalReference + request: OwnerRequestId traced: Annotated[FileLine | NoSourceReference, WireOmitDefault()] = NoSourceReference() @classmethod def by(cls, actor: ThreadIncarnation, expected_revision: int, owner: ThreadIncarnation, generation: str, review_at: EventTime, order: PendingProblemOrder | MergedProblemOrder, - goal: GoalReference, traced: FileLine | NoSourceReference) -> AssignTodo: + request: OwnerRequestId, traced: FileLine | NoSourceReference) -> AssignTodo: """The one place the typed actor and the assignment's recorded assigner are made.""" return cls(expected_revision, actor, Assignment.of(owner, actor, generation), - review_at, order, goal, traced) + review_at, order, request, traced) def authored_by(self, actor: ThreadIncarnation) -> Self: return replace(self, actor=actor, proposed=replace(self.proposed, parent=actor.name, parent_created=actor.wire_birth)) - def require_unit(self, db: sqlite3.Connection, current: Todo) -> None: + def validate(self, db: sqlite3.Connection, current: Todo) -> None: TodoStore.require_unit_in(db, self.proposed.owner_identity, self.actor) + OwnerRequest.require(db, id=self.request).require_open(current.creator_identity) def apply(self, current: Todo) -> Todo: self.review_at.require_recorded("A review time") proposed = current.proposed_assignment(self.proposed) - if (current.assignment, current.review_at, current.order, current.goal) == ( - proposed, self.review_at, self.order, self.goal): + if (current.assignment, current.review_at, current.order, current.request) == ( + proposed, self.review_at, self.order, self.request): return current return super(AssignTodo, replace(self, proposed=proposed)).apply(current) @@ -2642,7 +2597,7 @@ def changed(self, current: Todo) -> Todo: else current.state.assign(current, self.proposed, self.traced)) return replace(assigned, review_at=self.review_at, order=self.order, prior_orders=current.orders_before(self.order), - goal=self.goal, + request=self.request, revision=current.revision + 1) @@ -2740,12 +2695,14 @@ class VerdictAudience: threads: tuple[str, ...] @classmethod - def of(cls, unit: str, goal: GoalReference) -> VerdictAudience: - """An order's unit and its goal's thread; no verdict kind today concerns every unit.""" - return cls(tuple(dict.fromkeys((unit, *goal.threads())))) + def of(cls, unit: str, todo: Todo) -> VerdictAudience: + """The order records its commander; a request lookup adds no authority.""" + return cls(tuple(dict.fromkeys((unit, todo.creator)))) - def addressed(self, target: str, body: str) -> str: - return CommandTarget.named(target).addressed(self.threads, body) + @property + def target(self) -> NamedTarget: + name = self.threads[0] if len(self.threads) == 1 else CommsTag.channel() + return CommandTarget.named(name) @dataclass(frozen=True) @@ -3214,14 +3171,66 @@ def changed(self, current): return current.state.change(current, self) +@dataclass(frozen=True, slots=True) +class OwnerRequest(TypedTable, SharedFileTables, DurableTables): + """One owner's request, independent of any current turn goal.""" + + indexes = (Index(("commander_created_at", "state", "created_at")),) + + id: Annotated[OwnerRequestId, Column(primary_key=True, storage=TextStorage)] + commander: RecordedThreadIncarnation + commander_created_at: ClassVar[Generated] = Generated( + float, Column(generated="json_extract({commander}, '$.created_at')"), + ) + text: str + state: RequestState = OpenRequestState() + directives: tuple[RequestDirective, ...] = () + created_at: float = field(default_factory=time.time) + + def __post_init__(self) -> None: + if not self.text.strip(): + raise TodoError("An owner request needs its text.") + + @classmethod + def new(cls, commander: ThreadIncarnation, text: str) -> Self: + return cls(OwnerRequestId.new(), commander.require_recorded(), text) + + def require_commander(self, commander: ThreadIncarnation) -> None: + if self.commander != commander.require_recorded(): + raise TodoError(f"Request {self.id.value} belongs to another commander.") + + def require_open(self, commander: ThreadIncarnation) -> OpenRequestState: + self.require_commander(commander) + return self.state.require_open(self.id) + + def at_revision(self, directives: tuple[RequestDirective, ...]) -> RequestAtRevision: + if self.directives[:len(directives)] != directives: + raise TodoError(f"Request {self.id.value} history is not a recorded directive prefix.") + return RequestAtRevision(self.id, len(directives)) + + def directive_messages( + self, source: BusReader, responders: frozenset[str], + ) -> tuple[tuple[RequestDirective, Message], ...]: + """Select exact unanswered references before loading their originals.""" + unanswered = {reference.seq for reference in source.unanswered_references( + tuple(directive.seq for directive in self.directives), responders)} + return tuple((directive, directive.original(source)) for directive in self.directives + if directive.seq in unanswered) + + @dataclass(frozen=True, slots=True) class Todo(TypedTable): unit_index: ClassVar[ExpressionIndex] = ExpressionIndex(( Assignment.sql_field('owner'), Assignment.sql_field('owner_created'), '{state}', )) problem_index: ClassVar[ExpressionIndex] = ExpressionIndex((ProblemNumber.sql_number('{id}'),)) + serving_request: ClassVar[str] = ( + f'json_extract("order",\'$.{ProblemOrder.family_discriminator}\') ' + f'IN {DeliverableProblemOrder.sql_choices()}' + ) indexes = (Index(("creator", "creator_created", "state")), Index(("state",)), - unit_index, problem_index) + unit_index, problem_index, Index(("request",)), + Index(("request",), where=serving_request)) id: Annotated[str, Column(primary_key=True)] repo: Annotated[RepositoryName, Column(storage=TextStorage)] @@ -3230,7 +3239,9 @@ class Todo(TypedTable): creator_created: float state: type[TodoState] revision: Annotated[int, Column(check="revision>0")] - goal: Annotated[GoalReference, Column(storage=JsonStorage, nullable=True)] + request: Annotated[ + RequestLink, Column(storage=TextStorage, nullable=True, references=(OwnerRequest, "id")), + ] assignment: Assignment | None order: ProblemOrder review_at: EventTime = field(default_factory=UnrecordedEventTime) @@ -3294,19 +3305,16 @@ def held_order_ids(self) -> tuple[str, ...]: def reference_pattern(self) -> str: return rf"(^|[^A-Za-z0-9_]){re.escape(self.id)}([^A-Za-z0-9_]|$)" - unrequested_goal: ClassVar[GoalReference] = NoGoalReference() - @classmethod def ordered(cls, todo_id: str, repo: RepositoryName, text: str, creator: ThreadIncarnation, assignment: Assignment, review_at: EventTime, order: PendingProblemOrder | MergedProblemOrder, - goal: GoalReference = unrequested_goal) -> Todo: + request: OwnerRequestId) -> Todo: _text(todo_id, "Todo ID", 128) _text(text, "Todo text", 2000) review_at.require_recorded("A review time") - goal.require_reference() return cls(todo_id, repo, text, creator.name, creator.wire_birth, OpenTodoState, - 1, goal, assignment, order, review_at) + 1, request, assignment, order, review_at) @property def pending(self): @@ -3460,7 +3468,7 @@ def require_same_creation(self, proposed: Todo) -> None: text=proposed.text, creator=proposed.creator, creator_created=proposed.creator_created, - goal=proposed.goal, + request=proposed.request, review_at=proposed.review_at, order=proposed.order, ) @@ -3522,8 +3530,64 @@ def allocate(cls, db: sqlite3.Connection) -> ProblemNumber: return cls.issue(db, ProblemNumber(number)) +class OwnerRequestStore(DurableStore): + """Request lifetime and serving-work queries share the store's transaction.""" + + def add_request(self, commander: ThreadIncarnation, text: str) -> OwnerRequest: + request = OwnerRequest.new(commander, text) + with self.writing() as db: + request.insert(db) + return request + + def owner_request(self, request: OwnerRequestId) -> OwnerRequest: + with self.snapshot() as db: + return OwnerRequest.require(db, id=request) + + def open_requests( + self, commander: ThreadIncarnation, *, limit: int = -1, + ) -> tuple[OwnerRequest, ...]: + with self.snapshot() as db: + return tuple(OwnerRequest.select( + db, where="commander_created_at=?", + parameters=(commander.require_recorded().created_at,), state=OpenRequestState(), + order_by=("created_at", "id"), descending=True, limit=limit, + )) + + def record_directive(self, request: OwnerRequestId, directive: RequestDirective) -> None: + with self.writing() as db: + current = OwnerRequest.require(db, id=request) + current.state.require_open(current.id) + replace(current, directives=(*current.directives, directive)).upsert(db) + + def complete_request( + self, commander: ThreadIncarnation, request: OwnerRequestId, reason: str, + ) -> OwnerRequest: + with self.writing() as db: + current = OwnerRequest.require(db, id=request) + state = current.require_open(commander) + completed = replace(current, state=state.complete( + request, self._serving(db, request), reason, + )) + completed.upsert(db) + return completed + + @staticmethod + def _serving(db: sqlite3.Connection, request: OwnerRequestId) -> tuple[Todo, ...]: + return tuple(Todo.select(db, request=request, where=Todo.serving_request)) + + def serving_orders(self, request: OwnerRequestId) -> tuple[Todo, ...]: + """Pending work and unaccepted claims, through the request's partial index.""" + with self.snapshot() as db: + return self._serving(db, request) + + def for_request(self, request: OwnerRequestId) -> tuple[Todo, ...]: + """Every order that served this independent request, open or done.""" + with self.snapshot() as db: + return tuple(Todo.select(db, request=request)) + + @dataclass(frozen=True, slots=True) -class TodoStore(DurableStore): +class TodoStore(OwnerRequestStore): """One SQLite row owns both todo state and its exclusive assignment. Assignment identity and revision deduplicate known operations. An uncertain @@ -3533,7 +3597,7 @@ class TodoStore(DurableStore): root: Path filename: ClassVar[str] = "todos.sqlite3" - rows: ClassVar[tuple[type[TypedTable], ...]] = (Todo, HuntRecord, ProblemAllocation) + rows: ClassVar[tuple[type[TypedTable], ...]] = (OwnerRequest, Todo, HuntRecord, ProblemAllocation) @contextmanager def writing(self) -> Iterator[sqlite3.Connection]: @@ -3638,7 +3702,7 @@ def link( linked = target.order.linked(source.order) replace(target, order=linked, prior_orders=target.orders_before(linked, source.prior_orders), - assignment=source.held(), goal=source.goal, review_at=source.review_at, + assignment=source.held(), request=source.request, review_at=source.review_at, messages=source.messages, revision=target.revision + 1).upsert(db) @staticmethod @@ -3751,12 +3815,6 @@ def holds_pending(self, owner: ThreadIncarnation) -> bool: return any(todo.pending for todo in self._select( '1', (), reader=partial(self.held_in, unit=owner))) - def for_request(self, commander: ThreadIncarnation, goal_id: str) -> tuple[Todo, ...]: - """Every order that served one request, a Goal on the commander's thread, open or done.""" - return self._select( - "json_extract(goal,'$.owner')=? AND json_extract(goal,'$.owner_created')=? " - "AND json_extract(goal,'$.goal_id')=?", (commander.name, commander.wire_birth, goal_id)) - def for_commander(self, commander: ThreadIncarnation) -> tuple[Todo, ...]: return self._select("creator=? AND creator_created=?", (commander.name, commander.wire_birth), reader=self._open) @@ -3814,7 +3872,7 @@ def create( generation: str, review_at: EventTime, order: PendingProblemOrder, - goal: GoalRef, + request: OwnerRequestId, ) -> Todo: number = ProblemNumber.parse(todo_id) identity, assignee = _participant(creator), _thread(owner) @@ -3823,9 +3881,10 @@ def create( order.require_writable() proposed = Todo.ordered( todo_id, repo, text, identity, Assignment.of(assignee, assigner, generation), - review_at, order, goal) + review_at, order, request) with self.writing() as db: self.require_unit_in(db, assignee, assigner) + OwnerRequest.require(db, id=request).require_open(identity) proposed.insert_if_absent(db) current = self._current(db, todo_id) current.require_same_creation(proposed) @@ -3836,10 +3895,10 @@ def create( def apply(self, todo_id: str, command: TodoCommand) -> Todo: with self.writing() as db: current = self._current(db, todo_id) - command.require_unit(db, current) result = command.apply(current) if result == current: return current + command.validate(db, current) result.upsert(db) return self._current(db, todo_id) @@ -3881,10 +3940,10 @@ def assign( owner: Thread, assigner: ThreadIncarnation, generation: str, - goal: GoalRef, + request: OwnerRequestId, review_at: EventTime, order: PendingProblemOrder, traced: FileLine | NoSourceReference = NoSourceReference(), ) -> Todo: return self.apply(todo_id, AssignTodo.by(assigner, expected_revision, _thread(owner), - generation, review_at, order, goal, traced)) + generation, review_at, order, request, traced)) diff --git a/src/agent_comms/tools.py b/src/agent_comms/tools.py index 9737ef0083..85ea877ab9 100644 --- a/src/agent_comms/tools.py +++ b/src/agent_comms/tools.py @@ -38,7 +38,7 @@ from . import cli as comms_cli from .change_report import NoChangeReport, ReportEvidence from .channel_management import KeepThreadsTagDisposition, TagAction, TagDisposition -from .channel_targets import ChannelTarget, CommandTarget +from agent_comms.channel_targets import ChannelTarget, CommandTarget, NamedTarget from .channels import AnyChannel, Channel, SavedView, ViewKind, ViewMatch, ViewPredicate from .cli_commands import ( ArchiveCliCommand, @@ -135,6 +135,7 @@ from .task_sources import ( Constraint, CurrentTaskScopeSelection, + NoTaskAttachment, Decision, OriginalTaskChange, Subtask, @@ -483,9 +484,7 @@ def apply_without_thread( return self.apply(comms, caller) -class PublishingToolRequest[Caller: ToolCaller = ToolCaller]( - HeldBusWrite, ToolRequest[Caller], declared_name=None -): +class PublishingTool[Caller: ToolCaller = ToolCaller](HeldBusWrite): """A publication waits at the bus owner before any caller or message mutation.""" def apply(self, comms: Comms, caller: Caller) -> ToolResult: @@ -495,6 +494,12 @@ def apply(self, comms: Comms, caller: Caller) -> ToolResult: def publish(self, comms: Comms, caller: Caller) -> ToolResult: ... +class PublishingToolRequest[Caller: ToolCaller = ToolCaller]( + PublishingTool[Caller], ToolRequest[Caller], declared_name=None, +): + """Native declarations using the shared publication capability.""" + + class LiveOwnerTool: """A tool whose reply needs a launched native owner or a running restart watcher.""" @@ -1101,7 +1106,7 @@ def publish(self, comms: Comms, caller: ThreadCaller) -> TodoChanged: original = comms.bus.bus.lookup_message(todo.latest_message.message_id) claim = OrderClaim.for_order( todo, actor, self.pull_request, self.head, self.report, - original.reference, + original.reference, store=comms.goals.todos, ) changed = ClaimOrder( todo.revision, actor, generation, claim @@ -1353,7 +1358,7 @@ def apply(self, comms: Comms, caller: ToolCaller) -> SendRefusalsRead: @dataclass(frozen=True, kw_only=True) -class CommsSendTool(PublishingToolRequest): +class SendTool(PublishingTool): label = "Comms Send" description = ( "Send a message to another thread or channel. Address specific threads with " @@ -1375,11 +1380,13 @@ class CommsSendTool(PublishingToolRequest): ] = NoChangeReport() def publish(self, comms: Comms, caller: ToolCaller) -> MessagePublished: + target = CommandTarget.named(self.target) try: caller.require_directive(comms, self.body) - body = VerdictAudience(self.audience).addressed(self.target, self.body) - result = comms.messaging.send(self.sender, self.target, body, self.type, - report=self.report, origin=caller.origin()) + body = target.addressed(self.audience, self.body) + result = comms.messaging.send( + self.sender, target.name, body, self.type, report=self.report, + task=self.attachment(comms, target), origin=caller.origin()) except ToolRefusedError as error: # The caller hears the refusal now; the record waits for the wire lock on its own. threading.Thread(target=SendRefusal.record, @@ -1387,6 +1394,14 @@ def publish(self, comms: Comms, caller: ToolCaller) -> MessagePublished: raise return MessagePublished.from_send(result) + def attachment(self, comms: Comms, target: NamedTarget) -> TaskAttachment: + """An ordinary send carries no authored task or selected request.""" + return NoTaskAttachment() + + +class CommsSendTool(SendTool, ToolRequest): + """The native send declaration; its fields and publication belong to SendTool.""" + @dataclass(frozen=True, kw_only=True) class CommsAuthoredTaskTool(ThreadToolRequest, PublishingToolRequest[ThreadCaller]): diff --git a/src/agent_comms/typed_table.py b/src/agent_comms/typed_table.py index a60fb51395..99b27d4bfa 100644 --- a/src/agent_comms/typed_table.py +++ b/src/agent_comms/typed_table.py @@ -475,6 +475,12 @@ def for_field(cls, plan: FieldPlan) -> _Field: value=plan.value, ) + def rebuild_expression(self, stored: tuple[str, ...]) -> str: + """A carried value remains stored; table constraints own whether NULL is valid.""" + if self.wire_name in stored: + return _identifier(self.wire_name) + return "NULL" + def sql_literal(value: str | None | type[DeclaredFamily]) -> str: """Quote a declaration-owned SQL constant, never a query parameter.""" @@ -804,11 +810,12 @@ def create_schema(cls, db: sqlite3.Connection) -> None: @classmethod def derive_schema(cls, db: sqlite3.Connection) -> None: """Inside the caller's write transaction, the stored schema onto the declaration - wherever the declaration says how: a row's declared earlier table name, an index, - trigger or view rebuilt from its SQL, a declared object the file lacks. What it cannot - say, a table whose columns differ, is refused with the rows untouched.""" + wherever the declaration says how: a row's declared earlier table name, missing + nullable fields, an index, trigger or view rebuilt from its SQL, or a missing object. + Other table differences are refused with the rows untouched.""" for row in cls.schema_rows(): row.take_previous_name(db) + row.derive_nullable_fields(db) SQLiteCatalogObject.carry(db, cls.declared_schema(), frozenset(item.name for item in cls.stored_schema(db))) cls.require_schema(db) @@ -1448,14 +1455,15 @@ def _sql(cls, expression: str) -> str: return expression @classmethod - def schema_objects(cls, *, if_not_exists: bool = False) -> dict[str, str]: + def _table_sql(cls, *, if_not_exists: bool = False, columns: tuple[str, ...] = ()) -> str: existence = "IF NOT EXISTS " if if_not_exists else "" definitions = [] - keys = tuple(item.name for item in cls._columns() if item.column.primary_key) - auto_keys = tuple(item.name for item in cls._columns() if item.column.auto_increment) + fields = cls._fields_named(columns) if columns else cls._columns() + keys = tuple(item.name for item in fields if item.column.primary_key) + auto_keys = tuple(item.name for item in fields if item.column.auto_increment) if auto_keys and (auto_keys != keys or len(keys) != 1 or cls.without_rowid): raise TypeError("AUTOINCREMENT requires one rowid primary key") - for item in cls._columns(): + for item in fields: column = item.column sql = f"{_identifier(item.wire_name)} {item.storage.sql_type}" if column.auto_increment: @@ -1481,10 +1489,12 @@ def schema_objects(cls, *, if_not_exists: bool = False) -> dict[str, str]: definitions.extend(f"CHECK ({cls._sql(check)})" for check in cls.checks) definitions.extend(reference.sql(cls) for reference in cls.references()) table = _identifier(cls.declared_name) - statements = { - cls.declared_name: f"CREATE TABLE {existence}{table} ({', '.join(definitions)}) STRICT" - + (", WITHOUT ROWID" if cls.without_rowid else "") - } + return (f"CREATE TABLE {existence}{table} ({', '.join(definitions)}) STRICT" + + (", WITHOUT ROWID" if cls.without_rowid else "")) + + @classmethod + def schema_objects(cls, *, if_not_exists: bool = False) -> dict[str, str]: + statements = {cls.declared_name: cls._table_sql(if_not_exists=if_not_exists)} indexes = cls.indexes + tuple( Index((item.name,)) for item in cls._columns() if item.column.index ) @@ -1534,6 +1544,53 @@ def drop(cls, db: sqlite3.Connection) -> None: """Drop the declared table; SQLite drops its indexes with it.""" db.execute(f"DROP TABLE IF EXISTS {_identifier(cls.declared_name)}") + @classmethod + def rebuild(cls, db: sqlite3.Connection, + changed: Mapping[str, str] = types.MappingProxyType({})) -> None: + """Copy writable fields into the declared schema, in the caller's transaction. + + Data conversions supply expressions only for the fields they change. + The table owns every other column, its indexes, constraints and triggers. + """ + cls._writable_fields(tuple(changed)) + fields = cls._writable_fields() + stored = cls.stored_columns(db) + expressions = ( + changed[item.name] if item.name in changed else item.rebuild_expression(stored) + for item in fields + ) + selected = ", ".join(f'{expression} AS {_identifier(item.wire_name)}' + for item, expression in zip(fields, expressions, strict=True)) + table = _identifier(cls.declared_name) + temporary = _identifier(f"rebuild_{cls.declared_name}") + db.execute(f"CREATE TEMP TABLE {temporary} AS SELECT {selected} FROM {table}") + cls.drop(db) + cls.create(db) + columns = cls._column_list(tuple(item.name for item in fields)) + db.execute(f"INSERT INTO {table} ({columns}) SELECT {columns} FROM {temporary}") + db.execute(f"DROP TABLE {temporary}") + + @classmethod + def derive_nullable_fields(cls, db: sqlite3.Connection) -> None: + """Only an exact earlier declaration missing nullable fields can grow rows. + + Extra columns or any other table difference still require a data conversion. + Compare through the same DDL owner; do not maintain a second column schema. + """ + stored = cls.stored_columns(db) + fields = cls._columns() + missing = tuple(item for item in fields if item.wire_name not in stored) + if not stored or not missing: + return + if not all(item.nullable and item.column.writable for item in missing): + return + previous = cls._table_sql( + columns=tuple(item.name for item in fields if item.wire_name in stored), + ) + actual = {item.name: item.sql for item in cls.stored_objects(db)} + if actual.get(cls.declared_name) == previous: + cls.rebuild(db) + @classmethod def take_previous_name(cls, db: sqlite3.Connection) -> None: """Rows stored under a declared earlier name move to the declared one; rows under both diff --git a/src/agent_comms/work_dispatch.py b/src/agent_comms/work_dispatch.py index 6980497f8d..a0886dad88 100644 --- a/src/agent_comms/work_dispatch.py +++ b/src/agent_comms/work_dispatch.py @@ -20,6 +20,7 @@ OrderGoalAction, ProblemOrderGoalAction, ) +from agent_comms.owner_requests import NewestOpenRequest from .native_settings import NativeThreadDefaults from .phase_tasks import PhaseTask @@ -67,7 +68,7 @@ def start(self, comms: Comms) -> Thread: model=defaults.model, thinking_level=defaults.thinking_level, ) - self.action.request_ref(GoalActionContext(comms.goals, candidate)) + self.action.require_ready(GoalActionContext(comms.goals, candidate)) comms.registry.require_declaration_available(candidate, StoppedThreadStatus()) CheckoutStorageEffect(checkout, Path(self.action.order.check.directory)).check( StorageProjection(storage, candidate) @@ -147,7 +148,7 @@ def start( repo=self.repo, text=self.text, order=order, - request=order.request(commander, (self.problem,)), + request=NewestOpenRequest().for_order(comms.goals.todos, commander.incarnation), commander=commander.name, assigner=commander.incarnation, review_at=RecordedEventTime(now + self.review_seconds), diff --git a/tests/round2/test_typed_table.py b/tests/round2/test_typed_table.py index b5f80f5fb8..0ccec59c00 100644 --- a/tests/round2/test_typed_table.py +++ b/tests/round2/test_typed_table.py @@ -17,6 +17,7 @@ IntegerStorage, SqlStorage, SqlValue, + SQLiteSchemaObject, TypedRow, TypedTable, RowNotFoundError, @@ -108,9 +109,55 @@ def test_declared_table_family(tmp_path): assert TableChildRow.select(db)[0].weight is None +def test_rebuild_converts_a_field_and_preserves_the_declared_rows_and_constraints(): + with sqlite3.connect(':memory:') as db: + db.execute('PRAGMA foreign_keys=ON') + TableParentRow.create(db) + TableChildRow.create(db) + TableParentRow('p', 3).insert(db) + rows = (TableChildRow('first', 'p', True, ('one',), 125.0), + TableChildRow('second', 'p', False, ('two',), 250.0)) + for row in rows: + row.insert(db) + db.execute(f'ALTER TABLE "{TableChildRow.declared_name}" RENAME COLUMN weight TO retired_weight') + TableChildRow.rebuild(db, {'weight': 'retired_weight / 100.0'}) + assert TableChildRow.select(db, order_by=('key',)) == [ + replace(row, weight=row.weight / 100) for row in rows] + assert not SQLiteSchemaObject.differences( + TableChildRow.stored_objects(db), TableChildRow.schema_objects()) + assert db.execute('PRAGMA foreign_key_check').fetchall() == [] + with pytest.raises(sqlite3.IntegrityError): + replace(rows[0], key='invalid', parent='missing').insert(db) + db.execute(f'ALTER TABLE "{TableChildRow.declared_name}" DROP COLUMN weight') + TableChildRow.rebuild(db) + assert TableChildRow.select(db, order_by=('key',)) == [replace(row, weight=None) for row in rows] + + +def test_writer_derives_missing_nullable_fields_but_refuses_unclaimed_data(): + from agent_comms.goal_history import GoalHistoryEntry, GoalHistoryError + from agent_comms.goals import NoGoal + from agent_comms.typed_table import WriterOwnedFile + + entry = GoalHistoryEntry(1, 'baseline', 1.0, NoGoal(), NoGoal(), + owner_created_at=1.0, state='committed') + with sqlite3.connect(':memory:') as db: + GoalHistoryEntry.create(db) + entry.insert(db) + for column in ('request_before', 'request_after'): + db.execute(f'ALTER TABLE "{GoalHistoryEntry.declared_name}" DROP COLUMN {column}') + db.commit() + GoalHistoryEntry.reconcile_schema(db, WriterOwnedFile()) + assert GoalHistoryEntry.require(db, sequence=1) == entry + db.execute(f'ALTER TABLE "{GoalHistoryEntry.declared_name}" ADD COLUMN unclaimed TEXT') + db.execute(f'UPDATE "{GoalHistoryEntry.declared_name}" SET unclaimed=?', ('preserve this',)) + db.commit() + with pytest.raises(GoalHistoryError): + GoalHistoryEntry.reconcile_schema(db, WriterOwnedFile()) + assert db.execute(f'SELECT unclaimed FROM "{GoalHistoryEntry.declared_name}"').fetchall() == [('preserve this',)] + + def test_atomic_creation_preserves_declared_indexes_triggers_and_strict_creation(): from agent_comms.native_pi import NativeContextJournal - from agent_comms.typed_table import SQLiteSchemaObject with sqlite3.connect(':memory:') as db: NativeContextJournal.create(db, if_not_exists=True) diff --git a/tests/test_bus.py b/tests/test_bus.py index ec8d19d239..9fd075cb55 100644 --- a/tests/test_bus.py +++ b/tests/test_bus.py @@ -376,13 +376,15 @@ def test_human_refusal_rolls_back_the_header(tmp_path, bus): "BEGIN SELECT RAISE(ABORT, 'refused'); END" ) origin = HumanOrigin("author", 1, str(tmp_path)) - with pytest.raises(HumanSendRefusedError, match="was not sent"): + with pytest.raises(HumanSendRefusedError, match="was not sent") as failed: origin.append( bus, lambda seq, db: record_from( PrivateRecord, Message("author", "worker", "refused", InfoKind, seq=seq) ), ) + assert isinstance(failed.value.cause, sqlite3.IntegrityError) + assert failed.value.__cause__ is failed.value.cause with closing(sqlite3.connect(bus.path)) as db: assert all(table.count(db) == 0 for table in TypedTable.members_with(BusTables)) @@ -405,6 +407,22 @@ def test_notification_failure_carries_the_committed_message(tmp_path, bus): ) +def test_human_programming_failure_propagates_after_rollback(tmp_path, bus): + failure = RuntimeError("Broken publication implementation") + + def build(seq, db): + record_from( + PrivateRecord, Message("author", "worker", "not committed", InfoKind, seq=seq) + ).append_to(db, seq) + raise failure + + with pytest.raises(RuntimeError) as failed: + HumanOrigin("author", 1, str(tmp_path)).append(bus, build) + assert failed.value is failure + with closing(sqlite3.connect(bus.path)) as db: + assert all(table.count(db) == 0 for table in TypedTable.members_with(BusTables)) + + def test_readonly_archive_and_backup_use_the_same_bus(tmp_path): bus = Bus(Bus.path_in(tmp_path / "source")) assert not bus.path.parent.exists() diff --git a/tests/test_bus_files.py b/tests/test_bus_files.py index 0a7b12d6bf..2e927ded69 100644 --- a/tests/test_bus_files.py +++ b/tests/test_bus_files.py @@ -16,7 +16,7 @@ from agent_comms.private_registry_guard import PrivateOwnerRenameIntent from agent_comms.recorded_time import RecordedEventTime from agent_comms.repository_name import RepositoryName -from agent_comms.todos import GoalRef, TodoStore +from agent_comms.todos import TodoStore from test_todos import order @@ -47,7 +47,8 @@ def test_every_bus_file_is_a_declared_root_file(tmp_path: Path, source_repositor todo = TodoStore(comms.root).create( "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "Keep declared bus files", creator=owner, assigner=owner.incarnation, owner=owner, - generation="declared-files", goal=GoalRef(owner.name, owner.created_at, "goal-1"), + generation="declared-files", + request=comms.goals.todos.add_request(owner.incarnation, "Keep declared bus files").id, review_at=RecordedEventTime(1000.0), order=order(source_repository), ) assert todo.id == "P1" diff --git a/tests/test_carry_bus_history.py b/tests/test_carry_bus_history.py index a7c2a72b1d..fee19d55d3 100644 --- a/tests/test_carry_bus_history.py +++ b/tests/test_carry_bus_history.py @@ -25,6 +25,8 @@ def publish_predecessor_history(directory: Path) -> None: from agent_comms.coordination_tables.publications import PublicationIntents from agent_comms.coordination_tables.responses import ResponseObligation from agent_comms.historical_views import HistoryArchive + from agent_comms.goal_actions import SetGoalAction + from agent_comms.goals import RequestGoalOrder from agent_comms.installer.backup import StoreBackup from agent_comms.messages import InfoKind, Message from agent_comms.obligation_states import PublishingResponse @@ -73,6 +75,10 @@ def publish_predecessor_history(directory: Path) -> None: intent, conversation=conversation, registry_snapshot=comms.registry.snapshot(), activity=activity, ) + comms.goals.update_goal("owner", SetGoalAction( + text="Preserve this independent owner request", goal_id="retained-request", + order=RequestGoalOrder(), + )) owner = comms.registry.snapshot().require("owner").incarnation.require_recorded() document = ReadDocument( messages={owner: ReadCursor({AllConversation(): ConversationCursor(1, (3,))})}, @@ -136,10 +142,18 @@ def test_predecessor_publisher_history_round_trips_into_bus(tmp_path): from agent_comms.historical_views import HistoryArchive from agent_comms.installer.backup import StoreBackup, bus_files from agent_comms.installer.carry import Carried, MigrateOperation + from agent_comms.installer.carry import CarryChain from agent_comms.installer.upgrade import RehearsalFiles, StepRun, carried from agent_comms.messages import BusRecord from agent_comms.read_ledger import ReadLedger from agent_comms.recorded_bus import RecordedBusIdentity + from agent_comms.maintenance_barrier import MaintenanceBarrier, MaintenanceOperator + from agent_comms.registry_store import RegistryStore + from agent_comms.todos import TodoStore + from cutover.carry_owner_requests import OwnerRequests + + chain = CarryChain.at(REPOSITORY) + assert chain.steps.index(OwnerRequests) < chain.steps.index(BusHistory) generated = predecessor_history(tmp_path) original = generated / "destination" @@ -163,20 +177,31 @@ def test_predecessor_publisher_history_round_trips_into_bus(tmp_path): assert all(not Bus.path_in(root / relative).exists() for relative in relatives) assert not tuple(spill.iterdir()) operation = MigrateOperation(True) - saved = bus_files(root) + requests = OwnerRequests(root, original, spill) + request_dry_run = requests.carry(apply=False) + assert request_dry_run.detail["requests"] == 2 + maintenance = MaintenanceBarrier(RegistryStore.at(root).path) + hold = maintenance.begin(MaintenanceOperator.current()) + with hold.holding(): + saved = bus_files(root) + request_result = requests.carry(apply=True) + assert request_result.detail == request_dry_run.detail + assert requests.carry(apply=False).changed == 0 result_path = tmp_path / "carried.json" - completed = subprocess.run( - [sys.executable, "-m", "agent_comms.installer.step_worker", request.name(), - json.dumps(FieldCodec.encode(request)), json.dumps(FieldCodec.encode(operation)), - str(result_path)], cwd=REPOSITORY, - env={**os.environ, "PYTHONPATH": os.pathsep.join( - map(str, (REPOSITORY / "src", REPOSITORY / "tools")))}, - capture_output=True, text=True, timeout=60, - ) + with hold.holding(): + completed = subprocess.run( + [sys.executable, "-m", "agent_comms.installer.step_worker", request.name(), + json.dumps(FieldCodec.encode(request)), json.dumps(FieldCodec.encode(operation)), + str(result_path)], cwd=REPOSITORY, + env={**os.environ, "PYTHONPATH": os.pathsep.join( + map(str, (REPOSITORY / "src", REPOSITORY / "tools")))}, + capture_output=True, text=True, timeout=60, + ) assert completed.returncode == 0, completed.stdout + completed.stderr result = operation.decode_answer(result_path.read_text()) - steps = (StepRun(request.name(), result),) + steps = (StepRun(requests.name(), request_result), StepRun(request.name(), result)) carried(root, saved, steps, RehearsalFiles.declared(steps)) + maintenance.clear(hold) assert result.changed == 8 assert {key: result.detail[key] for key in ("rows", "continuous", "round_tripped")} == { "rows": 8, "continuous": 8, "round_tripped": 8, @@ -188,6 +213,11 @@ def test_predecessor_publisher_history_round_trips_into_bus(tmp_path): assert not tuple(spill.iterdir()) for relative in relatives: directory = root / relative + registry = RegistryStore.at(directory).read() + owner_requests = TodoStore(directory).open_requests(registry.threads["owner"].incarnation) + assert len(owner_requests) == 1 + assert owner_requests[0].id.value == "retained-request" + assert owner_requests[0].text == "Preserve this independent owner request" source = directory / BusHistory.filename metadata = HistoryMetadata.read(source.parent) assert RecordedBusIdentity.capture(source) == identities[relative] diff --git a/tests/test_commander_forks.py b/tests/test_commander_forks.py index a78382879f..11f28c77f3 100644 --- a/tests/test_commander_forks.py +++ b/tests/test_commander_forks.py @@ -27,7 +27,8 @@ from agent_comms.transcript_events import ToolStartTranscript from agent_comms.goal_actions import SetGoalAction from agent_comms.installer.backup import StoreBackup -from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.maintenance_barrier import MaintenanceBarrier, MaintenanceNotHeldError, MaintenanceOperator +from agent_comms.goals import CountCheckResult, OrderCheck from agent_comms.order_runs import MechanicalOrderRun from agent_comms.recorded_time import RecordedEventTime from agent_comms.repo import Repo @@ -37,7 +38,7 @@ from agent_comms.tools import CommsTodoListTool from agent_comms.thread_model import DeclaredModel from agent_comms.todos import ( - DeclarationDerivation, GoalRef, PendingProblemOrder, ProblemAllocation, ReleaseTodo, + DeclarationDerivation, PendingProblemOrder, ProblemAllocation, ReleaseTodo, Slop, StrikeReason, SupersededVerdict, Todo, TodoError, ) @@ -57,7 +58,9 @@ def fork_bus(tmp_path): name, frozenset({'comms'}), str(source.checkout), model=DeclaredModel('anthropic/claude-opus-4-6'), execution=ExternalThreadExecution)) comms.goals.update_goal('lead', SetGoalAction( - text='Repair the commander workflow', goal_id='request', order=RequestGoalOrder())) + text='Repair the commander workflow', goal_id='request')) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair the commander workflow') core = Repo.of(Path(__file__).parents[1]) config = Commander.on_machine('lead', (source, core), comms.root, tmp_path) config.path_in(comms.root).write_text(config.model_dump_json()) @@ -72,7 +75,7 @@ def fork_bus(tmp_path): comms.goals.todos.create('P40', source.github, 'Existing parent problem', creator=lead, owner=comms.registry.require('retained'), assigner=lead.incarnation, generation='retained', review_at=RecordedEventTime(now + 600), order=replace(order, id='retained-order'), - goal=GoalRef('lead', lead.created_at, 'request')) + request=request.id) copied = tmp_path / 'copied-bus' StoreBackup.copy_tree(comms.root, copied) return Comms(copied), source, order @@ -129,7 +132,7 @@ def test_registered_fork_sends_parent_problem_with_fork_actor(fork_bus, tmp_path fork = comms.registry.require('fork-one') saved = store.get('P40') assert saved.creator_identity == parent.incarnation - assert saved.goal.owner == parent.name + assert store.owner_request(saved.request.require()).commander == parent.incarnation assert saved.held().parent_identity == fork.incarnation assert saved.messages[-1].actor == fork.incarnation assert fork.parent.named(parent.name) @@ -243,9 +246,9 @@ def assign(name): print(refused.stderr.strip()) -def test_a_retained_store_takes_its_declared_schema_on_the_next_write(fork_bus): - """The writer's open derives what the declaration says: a missing table and indexes are - created, and rows stored under ProblemAllocation's declared earlier name move to it.""" +def test_retained_schema_is_carried_under_maintenance_before_runtime_writes(fork_bus): + """Only the installer carries missing objects and declared earlier names; + runtime writes refuse a changed schema and resume after that carry.""" comms, source, order = fork_bus store = comms.goals.todos retained = store.get('P40') @@ -253,11 +256,24 @@ def test_a_retained_store_takes_its_declared_schema_on_the_next_write(fork_bus): db.execute(f'DROP TABLE "{ProblemAllocation.declared_name}"') db.execute(f'DROP INDEX "{Todo.unit_index.name(Todo)}"') db.execute(f'DROP INDEX "{Todo.problem_index.name(Todo)}"') + with pytest.raises(MaintenanceNotHeldError): + store.allocate_problem() + barrier = MaintenanceBarrier(comms.registry.store.path) + hold = barrier.begin(MaintenanceOperator.current()) + with hold.holding(): + assert store.reconcile_at(store.path) + barrier.clear(hold) assert store.allocate_problem() == 'P41' assert store.get('P40') == retained with store.writing() as db: db.execute(f'ALTER TABLE "{ProblemAllocation.declared_name}" RENAME TO problem_number') db.execute(f'DROP INDEX "{Todo.problem_index.name(Todo)}"') + with pytest.raises(MaintenanceNotHeldError): + store.allocate_problem() + hold = barrier.begin(MaintenanceOperator.current()) + with hold.holding(): + assert store.reconcile_at(store.path) + barrier.clear(hold) assert store.allocate_problem() == 'P42' with store.snapshot() as db: assert [row.number for row in ProblemAllocation.select(db)] == [41, 42] @@ -272,13 +288,13 @@ def test_created_ids_live_only_on_todos_and_allocation_uses_numeric_index(fork_b issued = store.allocate_problem() assert issued == 'P41' created = store.create(issued, source.github, 'Create a real problem', creator=lead, - assigner=lead.incarnation, owner=comms.registry.require('other'), generation='created', goal=retained.goal, + assigner=lead.incarnation, owner=comms.registry.require('other'), generation='created', request=retained.request.require(), review_at=retained.review_at, order=order) with store.snapshot() as db: assert ProblemAllocation.select(db) == [] # Direct creation must affect allocation without copying its key into the allocator. store.create('P120', source.github, 'Exercise numeric ordering', creator=lead, - assigner=lead.incarnation, owner=comms.registry.require('worker'), generation='direct', goal=retained.goal, + assigner=lead.incarnation, owner=comms.registry.require('worker'), generation='direct', request=retained.request.require(), review_at=retained.review_at, order=replace(order, id='direct-order')) assert store.allocate_problem() == 'P121' with store.snapshot() as db: @@ -310,7 +326,7 @@ def test_fork_superseded_verdict_reads_its_commanders_orders(fork_bus): store.create('P41', source.github, 'Replacing parent problem', creator=lead, owner=comms.registry.require('worker'), assigner=lead.incarnation, generation='worker', review_at=RecordedEventTime(time.time() + 600), order=replace(order, id='replacing-order'), - goal=GoalRef('lead', lead.created_at, 'request')) + request=store.get('P40').request.require()) comms.registry.register_commander_child('fork-one', 'lead', comms.goals) actor = CommanderForkCaller('fork-one').identity(comms) current = store.get('P40') diff --git a/tests/test_commander_run_rules.py b/tests/test_commander_run_rules.py index 6d231ebbc7..0a6846d4a4 100644 --- a/tests/test_commander_run_rules.py +++ b/tests/test_commander_run_rules.py @@ -8,7 +8,7 @@ from agent_comms.field_codec import FieldCodec from agent_comms.recorded_time import RecordedEventTime from agent_comms.repository_name import RepositoryName -from agent_comms.todos import GoalRef, Todo, TodoStore +from agent_comms.todos import Todo, TodoStore from agent_comms.order_runs import ApplicationOrderRun from test_todos import order, thread @@ -18,10 +18,11 @@ def test_declared_run_round_trips_and_short_runs_are_refused(tmp_path, source_re checked = replace(order(source_repository), run=ApplicationOrderRun(planned)) store = TodoStore(tmp_path) lead = thread('lead', tmp_path, 1) + request = store.add_request(lead.incarnation, 'Repair the thread list').id todo = store.create('P1', RepositoryName.from_wire('OpenHCSDev/agent-comms'), 'Repair the thread list', - creator=lead, assigner=lead.incarnation, owner=lead, generation='one', goal=GoalRef('lead', 1, 'one'), review_at=RecordedEventTime(1000), - order=checked) + creator=lead, assigner=lead.incarnation, owner=lead, generation='one', + request=request, review_at=RecordedEventTime(1000), order=checked) assert FieldCodec.decode(Todo, FieldCodec.encode(todo)) == todo assert planned in todo.order.instructions(todo) with pytest.raises(ToolRefusedError): @@ -36,16 +37,17 @@ def test_declared_run_round_trips_and_short_runs_are_refused(tmp_path, source_re def test_clean_transfers_keep_history_and_third_owner_requires_a_trace(tmp_path, source_repository): store = TodoStore(tmp_path) lead = thread('lead', tmp_path, 1) + request = store.add_request(lead.incarnation, 'Repair the thread list').id first = store.create('P1', RepositoryName.from_wire('OpenHCSDev/agent-comms'), 'Repair the thread list', - creator=lead, assigner=lead.incarnation, owner=lead, generation='one', goal=GoalRef('lead', 1, 'one'), review_at=RecordedEventTime(1000), - order=order(source_repository)) + creator=lead, assigner=lead.incarnation, owner=lead, generation='one', + request=request, review_at=RecordedEventTime(1000), order=order(source_repository)) second = store.assign('P1', expected_revision=first.revision, - owner=thread('second', tmp_path, 2), assigner=lead.incarnation, generation='two', goal=GoalRef('second', 2, 'two'), + owner=thread('second', tmp_path, 2), assigner=lead.incarnation, generation='two', request=request, review_at=RecordedEventTime(1000), order=order(source_repository)) assert not second.strikes kwargs = dict(expected_revision=second.revision, owner=thread('third', tmp_path, 3), - assigner=lead.incarnation, generation='three', goal=GoalRef('third', 3, 'three'), review_at=RecordedEventTime(1000), order=order(source_repository)) + assigner=lead.incarnation, generation='three', request=request, review_at=RecordedEventTime(1000), order=order(source_repository)) with pytest.raises(ValueError): store.assign('P1', **kwargs) # Source existence belongs to the commander boundary, not a Core record. diff --git a/tests/test_delivery_failure.py b/tests/test_delivery_failure.py index 7bdced1fec..81b15c6975 100644 --- a/tests/test_delivery_failure.py +++ b/tests/test_delivery_failure.py @@ -20,7 +20,7 @@ "error, member", ( (OSError("refused"), BackendDeliveryFailure), - (HumanSendRefusedError("Message was not sent"), BackendDeliveryFailure), + (HumanSendRefusedError(ValueError("refused")), BackendDeliveryFailure), (HumanClaimBlockedError("reserved gap"), ClaimBlockedFailure), ), ) @@ -34,7 +34,7 @@ def test_delivery_failure_decodes_once(error, member): @pytest.mark.parametrize( "error", - (HumanSendRefusedError("rolled back"), HumanClaimBlockedError("reserved gap")), + (HumanSendRefusedError(ValueError("rolled back")), HumanClaimBlockedError("reserved gap")), ) def test_human_send_failures_reach_the_ui_catch(error): with pytest.raises(HumanSendError): @@ -43,4 +43,4 @@ def test_human_send_failures_reach_the_ui_catch(error): def test_rolled_back_human_send_remains_a_tool_refusal(): with pytest.raises(ToolRefusedError): - raise HumanSendRefusedError("rolled back") + raise HumanSendRefusedError(ValueError("rolled back")) diff --git a/tests/test_goal_standby.py b/tests/test_goal_standby.py index 59e124ad5f..34c10bb3eb 100644 --- a/tests/test_goal_standby.py +++ b/tests/test_goal_standby.py @@ -291,16 +291,3 @@ def test_standby_refuses_canonical_reply_that_arrived_before_wait(tmp_path, ackn assert isinstance(comms.goals.goal_wait("parent"), NoGoalWait) assert comms.registry.require("parent").goal == goal assert not (tmp_path / "input_dispositions.sqlite3").exists() - - -def test_an_open_owner_request_takes_a_new_wording_and_keeps_its_identity(tmp_path): - from agent_comms.goals import RequestGoalOrder - - comms = canonical_goal_wire(tmp_path) - comms.registry.declare(comms.registry.new_thread('lead', frozenset(), str(tmp_path))) - opened = comms.goals.update_goal('lead', SetGoalAction( - text='Commander native in Core', goal_id='request', order=RequestGoalOrder())) - edited = comms.goals.update_goal('lead', EditGoalAction(text="Tristan's plans, in order")) - assert (edited.id, edited.order, edited.text) == (opened.id, opened.order, "Tristan's plans, in order") - with pytest.raises(ValueError, match="still open"): - comms.goals.update_goal('lead', SetGoalAction(text='Another request', order=RequestGoalOrder())) diff --git a/tests/test_held_problem_orders.py b/tests/test_held_problem_orders.py index f071025c39..1a75f9c3d8 100644 --- a/tests/test_held_problem_orders.py +++ b/tests/test_held_problem_orders.py @@ -17,7 +17,8 @@ from agent_comms.goals import OrderCheck from agent_comms.recorded_time import RecordedEventTime from agent_comms.repo import Repo -from agent_comms.todos import DeliveredProblemOrder, GoalRef, OrderMessage +from agent_comms.todos import DeliveredProblemOrder, OrderMessage +from agent_comms.owner_requests import NewestOpenRequest from agent_comms.tool_caller import ThreadCaller from agent_comms.tools import CommsChannelsTool @@ -34,7 +35,8 @@ def held_commander(pr_commander): lead = source.comms.registry.require(source.commander) todo = source.comms.goals.todos.create( "P1", source.repos[0].github, "Repair the held workflow", creator=lead, assigner=lead.incarnation, - owner=worker, generation="initial-order", goal=GoalRef(lead.name, lead.created_at, lead.goal.require().id), + owner=worker, generation="initial-order", + request=NewestOpenRequest().for_order(source.comms.goals.todos, lead.incarnation), review_at=RecordedEventTime(time.time() + 1000), order=order(source.repos[0]), ) published = source.comms.messaging.send(source.commander, worker.name, "Repair the held workflow through its existing declaration.") @@ -94,7 +96,7 @@ def test_second_unplanned_order_is_refused_without_recording(held_commander): lead = ctx.comms.registry.require(ctx.commander) ctx.comms.goals.todos.create( "P2", ctx.repos[0].github, "Repair the second workflow", creator=lead, assigner=lead.incarnation, owner=lead, - generation="second-workflow", goal=GoalRef(lead.name, lead.created_at, lead.goal.require().id), + generation="second-workflow", request=original.request.require(), review_at=RecordedEventTime(time.time() + 1000), order=document.order, ) path = ctx.root / "second-order.json" diff --git a/tests/test_holder_standing_rules.py b/tests/test_holder_standing_rules.py index eb0f699716..a7b7491e5a 100644 --- a/tests/test_holder_standing_rules.py +++ b/tests/test_holder_standing_rules.py @@ -30,13 +30,13 @@ from agent_comms.field_codec import FieldCodec from agent_comms.goal_actions import ( CreateProblemGoalAction, - DirectiveGoalAction, ReissuedOrderGoalAction, SetGoalAction, ) from agent_comms.goal_presentation import RunnableExecution from agent_comms.goal_states import ActiveGoal -from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.goals import CountCheckResult, OrderCheck +from agent_comms.owner_requests import NewestOpenRequest, RequestDirective from agent_comms.input_disposition import InputDispositions from agent_comms.installer.backup import StoreBackup from agent_comms.launch_root import LaunchRoot @@ -76,13 +76,12 @@ def held_bus(comms, tmp_path, source_repository, configured_repository): comms.registry.new_thread(name=name, tags=frozenset({'comms'}), worktree=str(source.checkout)) for name in ('lead', 'worker', 'outsider') )) - comms.goals.update_goal("lead", SetGoalAction( - text="Repair the held work", goal_id="request", order=RequestGoalOrder(), - )) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair the held work').id now = time.time() for problem in ("P1", "P2"): comms.goals.update_goal("worker", CreateProblemGoalAction( - problem=problem, commander="lead", request="request", + problem=problem, commander="lead", request=request, assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire("OpenHCSDev/agent-comms"), text=f"Repair {problem}", review_at=RecordedEventTime(now + 3600), @@ -126,7 +125,8 @@ def claim(problem): todo = held_bus.goals.todos.apply( todo.id, OrderMessage(todo.revision, lead, original.message_id)) claimed = OrderClaim.for_order(todo, worker, 1, "a" * 40, - "Review the published repair", original.reference) + "Review the published repair", original.reference, + store=held_bus.goals.todos) claimed = replace(claimed, message=replace( claimed.message, timestamp=time.time() - AwaitingCommander.seconds - 1)) ClaimOrder(todo.revision, worker, todo.held().generation, claimed).record( @@ -248,7 +248,7 @@ def test_goal_generation_cannot_transfer_to_a_different_owner(held_bus): held_bus.goals.todos.assign( todo.id, expected_revision=todo.revision, owner=held_bus.registry.require("outsider"), assigner=CoreWatcherIncarnation.watcher(), - generation=todo.held().generation, goal=todo.goal, review_at=todo.review_at, + generation=todo.held().generation, request=todo.request.require(), review_at=todo.review_at, order=todo.order.checked().reissued(time.time()), ) assert held_bus.goals.todos.get(todo.id) == todo @@ -424,8 +424,10 @@ def test_an_overdue_owner_directive_gates_until_answered(held_bus, tmp_path): Message(sender="outsider", target="lead", type=InfoKind, body="Ship the install now", timestamp=time.time() - 3600), origin=AgentOrigin(), ) - held_bus.goals.update_goal("lead", DirectiveGoalAction( - text=directive.body, seq=directive.seq, at=directive.timestamp)) + store = held_bus.goals.todos + selected = NewestOpenRequest().select(store, held_bus.registry.require('lead').incarnation) + store.record_directive(selected[0].id, RequestDirective( + directive.seq, directive.timestamp + RequestDirective.window)) ctx = Commander.read_root(held_bus.root) reply = tmp_path / "reply.txt" reply.write_text("Installing now") diff --git a/tests/test_order_progress.py b/tests/test_order_progress.py index 22f874ad70..b7f3937558 100644 --- a/tests/test_order_progress.py +++ b/tests/test_order_progress.py @@ -13,8 +13,7 @@ from source_revision_fixture import SourceRepository from agent_comms.comms import Comms -from agent_comms.goal_actions import CreateProblemGoalAction, SetGoalAction -from agent_comms.goals import RequestGoalOrder +from agent_comms.goal_actions import CreateProblemGoalAction from agent_comms.native_tools import NativeRequestTools from agent_comms.recorded_time import RecordedEventTime from agent_comms.repository_name import RepositoryName @@ -122,10 +121,10 @@ def held(tmp_path): Bus.install(Bus.path_in(comms.root)) for name in ('lead', 'worker'): comms.registry.declare(comms.registry.new_thread(name, frozenset(), str(tmp_path))) - comms.goals.update_goal('lead', SetGoalAction( - text='Repair the worker', goal_id='request', order=RequestGoalOrder())) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair the worker').id ThreadCaller('lead').invoke(CommsTodoCreateTool(owner='worker', order=CreateProblemGoalAction( - problem='P1', order=order(source), request='request', review_at=RecordedEventTime(1000), + problem='P1', order=order(source), request=request, review_at=RecordedEventTime(1000), commander='lead', assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire('org/repo'), text='Repair the worker')), comms) return comms, unit_checkout(tmp_path / 'wt' / 'worker--P1') diff --git a/tests/test_order_scopes.py b/tests/test_order_scopes.py index 8f83ca7d20..5e515234a4 100644 --- a/tests/test_order_scopes.py +++ b/tests/test_order_scopes.py @@ -15,8 +15,7 @@ from agent_comms.check_root import CheckRoot from agent_comms.commander.order import OrderFile from agent_comms.field_codec import FieldCodec, MalformedRecord -from agent_comms.goal_actions import CreateProblemGoalAction, SetGoalAction -from agent_comms.goals import RequestGoalOrder +from agent_comms.goal_actions import CreateProblemGoalAction from agent_comms.recorded_time import RecordedEventTime from agent_comms.repository_name import RepositoryName from agent_comms.tool_caller import ThreadCaller @@ -62,12 +61,11 @@ def assigned(comms, scope, source, problem='P1'): def requested(comms, scope, source, problem='P1'): - comms.goals.update_goal('lead', SetGoalAction( - text='Classify the failing source or fix its declared parent', goal_id='request', - order=RequestGoalOrder())) + request = comms.goals.todos.add_request(comms.registry.require('lead').incarnation, + 'Classify the failing source or fix its declared parent').id caller = ThreadCaller('lead') CommsTodoCreateTool(owner='worker', order=CreateProblemGoalAction( - problem=problem, order=replace(order(source), scope=scope), request='request', + problem=problem, order=replace(order(source), scope=scope), request=request, review_at=RecordedEventTime(1000), commander='lead', assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire('OpenHCSDev/agent-comms'), @@ -121,4 +119,3 @@ def test_report_returns_through_the_real_cli_on_a_selected_history_copy(comms, t assert report in result.stdout assert copied.goals.todos.get(current.id).state is DoneTodoState assert comms.goals.todos.get('P1').pending - diff --git a/tests/test_order_story.py b/tests/test_order_story.py index e6d1f186ef..cc487b7f8b 100644 --- a/tests/test_order_story.py +++ b/tests/test_order_story.py @@ -9,8 +9,7 @@ from agent_comms.comms import Comms from agent_comms.field_codec import FieldCodec -from agent_comms.goal_actions import CreateProblemGoalAction, OrderGoalAction, SetGoalAction -from agent_comms.goals import RequestGoalOrder +from agent_comms.goal_actions import CreateProblemGoalAction, OrderGoalAction from agent_comms.order_story import FleetStory, OrderStory from agent_comms.recorded_time import RecordedEventTime from agent_comms.todos import ( @@ -30,12 +29,12 @@ def test_order_story_copied_bus_strike_and_send_back_once(comms, tmp_path): Bus.install(Bus.path_in(comms.root)) for name in ("lead", "worker"): comms.registry.declare(comms.registry.new_thread(name, frozenset(), str(tmp_path))) - comms.goals.update_goal("lead", SetGoalAction( - text="Repair the worker", goal_id="request", order=RequestGoalOrder())) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair the worker').id caller = ThreadCaller("lead") caller.invoke(CommsTodoCreateTool(owner="worker", order=CreateProblemGoalAction( problem="P1", order=replace(order(source), edits=("Repair the existing record owner",)), - request="request", review_at=RecordedEventTime(1000), + request=request, review_at=RecordedEventTime(1000), commander="lead", assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire("org/repo"), text="Repair the worker")), comms) todo = comms.goals.todos.get("P1") @@ -54,7 +53,7 @@ def test_order_story_copied_bus_strike_and_send_back_once(comms, tmp_path): todo = comms.goals.todos.get("P1") caller.invoke(CommsOrderTool(owner="worker", order=OrderGoalAction( problem="P1", expected_revision=todo.revision, assigner=actor, commander="lead", - request="request", review_at=RecordedEventTime(2000), + request=request, review_at=RecordedEventTime(2000), order=replace(order(source), id="resent-order"))), comms) current = comms.goals.todos.get("P1") with pytest.raises(ValueError): diff --git a/tests/test_owned_turn_goal_replacement.py b/tests/test_owned_turn_goal_replacement.py index cac111a83f..96c6c48448 100644 --- a/tests/test_owned_turn_goal_replacement.py +++ b/tests/test_owned_turn_goal_replacement.py @@ -15,10 +15,9 @@ from conftest import installed_comms from agent_comms.field_codec import Id128Text from agent_comms.goal_actions import ( - BlockedGoalAction, CreateProblemGoalAction, OrderGoalAction, SetGoalAction, + BlockedGoalAction, CreateProblemGoalAction, OrderGoalAction, ) from agent_comms.goal_states import ActiveGoal -from agent_comms.goals import RequestGoalOrder from agent_comms.owned_send import OwnedSend from agent_comms.owned_turn import OwnedTurn from agent_comms.queued_input import InitialInput, QueuedInput @@ -55,11 +54,10 @@ async def test_turn_after_order_replacement_is_allowed(tmp_path, native_runtime_ 'lead', frozenset(), str(saved.project), process_identity=ProcessIdentity.capture(os.getpid()), )) - comms.goals.update_goal('lead', SetGoalAction( - text='Repair following input', goal_id='request', order=RequestGoalOrder(), - )) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair following input').id comms.goals.update_goal(worker, CreateProblemGoalAction( - problem='P1', order=order(source), request='request', + problem='P1', order=order(source), request=request, review_at=RecordedEventTime(1000), commander='lead', assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire('org/repo'), text='Repair following input', @@ -79,7 +77,7 @@ async def test_turn_after_order_replacement_is_allowed(tmp_path, native_runtime_ replaced = comms.goals.update_goal(worker, OrderGoalAction( problem='P1', expected_revision=todo.revision, assigner=comms.registry.require('lead').incarnation, - commander='lead', request='request', review_at=RecordedEventTime(2000), + commander='lead', request=request, review_at=RecordedEventTime(2000), order=replace(order(source), id='replacement'), )) # A replaced order keeps the running turn's authority; only following input waits. @@ -122,11 +120,10 @@ async def test_goal_published_during_preparation_is_current_at_reservation(tmp_p name, frozenset(), str(tmp_path), process_identity=ProcessIdentity.capture(os.getpid()), )) - comms.goals.update_goal('lead', SetGoalAction( - text='Repair the turn owner', goal_id='request', order=RequestGoalOrder(), - )) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair the turn owner').id initial = comms.goals.update_goal('worker', CreateProblemGoalAction( - problem='P1', order=order(source), request='request', + problem='P1', order=order(source), request=request, review_at=RecordedEventTime(1000), commander='lead', assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire('org/repo'), text='Repair the turn owner', @@ -141,7 +138,7 @@ async def test_goal_published_during_preparation_is_current_at_reservation(tmp_p todo = comms.goals.todos.get('P1') replaced = comms.goals.update_goal('worker', OrderGoalAction( problem='P1', expected_revision=todo.revision, assigner=actor, - commander='lead', request='request', review_at=RecordedEventTime(2000), + commander='lead', request=request, review_at=RecordedEventTime(2000), order=replace(order(source), id='replacement'), )) old.original.goal_permission.require_current(replaced) @@ -162,7 +159,7 @@ async def test_goal_published_during_preparation_is_current_at_reservation(tmp_p todo = comms.goals.todos.get('P1') latest = comms.goals.update_goal('worker', OrderGoalAction( problem='P1', expected_revision=todo.revision, assigner=actor, - commander='lead', request='request', review_at=RecordedEventTime(3000), + commander='lead', request=request, review_at=RecordedEventTime(3000), order=replace(order(source), id='replacement-during-preparation'), )) assert current.thread.goal == replaced != latest @@ -187,11 +184,10 @@ async def test_order_reactivating_a_blocked_goal_keeps_the_running_turn(tmp_path name, frozenset(), str(tmp_path), process_identity=ProcessIdentity.capture(os.getpid()), )) - comms.goals.update_goal('lead', SetGoalAction( - text='Repair the blocked unit', goal_id='request', order=RequestGoalOrder(), - )) + request = comms.goals.todos.add_request( + comms.registry.require('lead').incarnation, 'Repair the blocked unit').id comms.goals.update_goal('worker', CreateProblemGoalAction( - problem='P1', order=order(source), request='request', + problem='P1', order=order(source), request=request, review_at=RecordedEventTime(1000), commander='lead', assigner=comms.registry.require('lead').incarnation, repo=RepositoryName.from_wire('org/repo'), text='Repair the blocked unit', @@ -210,7 +206,7 @@ async def test_order_reactivating_a_blocked_goal_keeps_the_running_turn(tmp_path todo = comms.goals.todos.get('P1') retried = comms.goals.update_goal('worker', OrderGoalAction( problem='P1', expected_revision=todo.revision, assigner=actor, - commander='lead', request='request', review_at=RecordedEventTime(2000), + commander='lead', request=request, review_at=RecordedEventTime(2000), order=replace(order(source), id='retry'), )) assert retried.state == ActiveGoal() diff --git a/tests/test_owner_directives.py b/tests/test_owner_directives.py index 61fc99f750..ae04c8a959 100644 --- a/tests/test_owner_directives.py +++ b/tests/test_owner_directives.py @@ -3,9 +3,9 @@ import pytest -from agent_comms.connector import DirectorCaller -from agent_comms.goal_actions import DirectiveGoalAction, SetGoalAction -from agent_comms.goals import RequestDirective, RequestGoalOrder +from agent_comms.connector import ConnectorSendTool, DirectorCaller +from agent_comms.errors import CommittedMessageError +from agent_comms.owner_requests import RequestDirective from agent_comms.tools import invoke_tool from agent_comms.typed_table import RowNotFoundError from goal_owner_fixture import canonical_goal_wire @@ -15,15 +15,15 @@ def requesting_lead(tmp_path): comms = canonical_goal_wire(tmp_path) for name in ("lead", "claude-ai"): comms.registry.declare(comms.registry.new_thread(name, frozenset({"base"}), str(tmp_path))) - comms.goals.update_goal("lead", SetGoalAction( - text="Tristan's plans, in order", goal_id="request", order=RequestGoalOrder())) + comms.goals.todos.add_request( + comms.registry.require("lead").incarnation, "Tristan's plans, in order") return comms def overdue(comms, directive: RequestDirective): - goal = comms.registry.require("lead").goal + request, = comms.goals.todos.open_requests(comms.registry.require("lead").incarnation) with comms.bus.bus.reading() as source: - return tuple(due for due, _original in goal.order.directive_messages( + return tuple(due for due, _original in request.directive_messages( source, comms.registry.snapshot().names_for("lead")) if due.deadline < directive.deadline + 1) @@ -31,7 +31,8 @@ def overdue(comms, directive: RequestDirective): def test_the_owners_message_is_a_directive_the_overdue_rule_reads(tmp_path): comms = requesting_lead(tmp_path) message = comms.messaging.send_user_message("lead", "Send the order now.", worktree=str(tmp_path)) - directives = comms.registry.require("lead").goal.order.directives + request, = comms.goals.todos.open_requests(comms.registry.require("lead").incarnation) + directives = request.directives assert [directive.seq for directive in directives] == [message.seq] assert overdue(comms, directives[0]) == directives @@ -40,7 +41,8 @@ def test_an_owner_message_to_a_channel_records_no_directive(tmp_path): comms = requesting_lead(tmp_path) comms.channels.create_tag("base") comms.messaging.send_user_message("#base", "Everyone, stop now.", worktree=str(tmp_path)) - assert comms.registry.require("lead").goal.order.directives == () + request, = comms.goals.todos.open_requests(comms.registry.require("lead").incarnation) + assert request.directives == () def test_claude_ai_send_records_exactly_one_directive(tmp_path): @@ -48,18 +50,38 @@ def test_claude_ai_send_records_exactly_one_directive(tmp_path): invoke_tool(comms, "comms_send", {"from": "claude-ai", "to": "lead", "body": "Send the order now."}, caller=DirectorCaller("claude-ai")) - directives = comms.registry.require("lead").goal.order.directives + request, = comms.goals.todos.open_requests(comms.registry.require("lead").incarnation) + directives = request.directives assert len(directives) == 1 assert overdue(comms, directives[0]) == directives +def test_connector_keeps_the_selected_request_when_notification_fails(tmp_path): + comms = requesting_lead(tmp_path) + store = comms.goals.todos + original, = store.open_requests(comms.registry.require("lead").incarnation) + newer = store.add_request(original.commander, "An independent newer request") + comms.bus.bus.path.with_suffix(".changed").mkdir() + with pytest.raises(CommittedMessageError) as failed: + ConnectorSendTool( + sender="claude-ai", target="lead", request=original.id, + body="Read the request owner and report the result.", + ).apply(comms, DirectorCaller("claude-ai")) + committed = failed.value.record.message + assert comms.bus.bus.lookup(committed.seq) == committed + assert committed.task.requests == (original.id,) + assert store.owner_request(original.id).directives == (RequestDirective.from_message(committed),) + assert store.owner_request(newer.id).directives == () + + def test_recorded_directives_keep_repeated_sequences_and_refuse_an_older_bus_cut(tmp_path): comms = requesting_lead(tmp_path) with comms.bus.bus.reading() as older: message = comms.messaging.send_user_message("lead", "Record the requested work", worktree=str(tmp_path)) - comms.goals.update_goal("lead", DirectiveGoalAction( - text=message.body, seq=message.seq, at=message.timestamp + 1)) - order = comms.registry.require("lead").goal.order + request, = comms.goals.todos.open_requests(comms.registry.require("lead").incarnation) + comms.goals.todos.record_directive(request.id, RequestDirective( + message.seq, message.timestamp + RequestDirective.window + 1)) + order = comms.goals.todos.owner_request(request.id) responders = comms.registry.snapshot().names_for("lead") assert len(order.directives) == 2 with pytest.raises(RowNotFoundError): diff --git a/tests/test_owner_requests.py b/tests/test_owner_requests.py new file mode 100644 index 0000000000..436e18dccd --- /dev/null +++ b/tests/test_owner_requests.py @@ -0,0 +1,217 @@ +"""One request family journey through the real store and publication tools.""" + +from dataclasses import replace +from pathlib import Path + +import pytest + +from agent_comms.change_report import FileLine, MainRevision +from agent_comms.commander.order import Commander +from agent_comms.commander.request import main as request_main +from agent_comms.comms import Comms +from agent_comms.connector import CALLER, ConnectorSendTool +from agent_comms.errors import RelationViolationError, ToolRefusedError +from agent_comms.goals import OrderCheck +from agent_comms.order_runs import MechanicalOrderRun +from agent_comms.owner_requests import NewestOpenRequest +from agent_comms.recorded_time import RecordedEventTime +from agent_comms.repo import Repo +from agent_comms.repository_name import RepositoryName +from agent_comms.thread_status import StoppedThreadStatus +from agent_comms.todos import ( + ClaimOrder, + OrderClaim, + PendingProblemOrder, + TodoStore, + TriageScope, + WithdrawTodo, +) +from agent_comms.tool_caller import ThreadCaller +from agent_comms.tools import CommsSendTool + + +def test_independent_requests_orders_and_directives(comms: Comms, durable_home, capsys) -> None: + repository = Repo(checkout=Path.cwd(), github=RepositoryName("OpenHCSDev", "agent-comms")) + for name in ("claude-ai", "commander", "worker"): + thread = comms.registry.new_thread(name, frozenset(), str(repository.checkout)) + comms.registry.declare(thread, StoppedThreadStatus()) + commander = comms.registry.require("commander") + worker = comms.registry.require("worker") + store = comms.goals.todos + ctx = Commander.on_machine("commander", (repository,), comms.root, durable_home) + request_main(["add", "commander native in Core"], ctx) + first_id = capsys.readouterr().out.strip() + request_main(["add", "virtual bus"], ctx) + second_id = capsys.readouterr().out.strip() + second, first = store.open_requests(commander.incarnation) + assert (first.id.value, second.id.value) == (first_id, second_id) + assert NewestOpenRequest().for_order(store, commander.incarnation) == second.id + assert first.id.for_order(store, commander.incarnation) == first.id + assert [ + request.id + for request in store.open_requests( + commander.incarnation.renamed("commander-renamed"), + ) + ] == [second.id, first.id] + + order = PendingProblemOrder( + owner=FileLine.class_declaration(TodoStore, repository, MainRevision()), + scope=TriageScope(), + check=OrderCheck(directory=str(repository.checkout), command="true"), + edits=("Read the request owner and its consumers.",), + banned=("Leave the saved bus and its sessions unchanged.",), + report="Report the result of the source investigation.", + run=MechanicalOrderRun(), + ) + todo = store.create( + "P1", + repository.github, + "Inspect the request owner", + creator=commander, + assigner=commander.incarnation, + owner=worker, + generation="request-family", + review_at=RecordedEventTime(order.at.recorded_at("Order") + 60), + order=order, + request=first.id, + ) + assert store.serving_orders(first.id) == (todo,) + assert not store.serving_orders(second.id) + request_main(["list"], ctx) + listed = capsys.readouterr().out + assert all( + text in listed + for text in (first_id, second_id, "P1", "commander native in Core", "virtual bus") + ) + with pytest.raises(ValueError): + request_main(["complete", first_id, "--why", "The work is finished."], ctx) + assert len(store.open_requests(commander.incarnation)) == 2 + assert store.serving_orders(first.id) == (todo,) + + ConnectorSendTool( + sender="claude-ai", + target="commander", + request=first.id, + body="Read the request records and report the result.", + ).apply(comms, CALLER) + named = comms.bus.bus.revision() + ConnectorSendTool( + sender="claude-ai", + target="commander", + body="Read the stored orders and report the result.", + ).apply(comms, CALLER) + newest = comms.bus.bus.revision() + first, second = store.owner_request(first.id), store.owner_request(second.id) + assert len(first.directives) == len(second.directives) == 1 + with comms.bus.bus.reading() as source: + original = first.directives[0].original(source) + assert original.seq == named + assert second.directives[0].original(source).seq == newest + prefix = first.at_revision(first.directives) + prefix.validate({first.id: first}) + with pytest.raises(RelationViolationError): + replace(prefix, directive_count=len(first.directives) + 1).validate({first.id: first}) + claim = OrderClaim.for_order( + todo, + worker.incarnation, + 1, + "a" * 40, + "The request owner was inspected.", + original.reference, + store=store, + ) + assert claim.message.target == first.commander.name + assert claim.message.answers == (original.reference,) + comms.messaging.send( + "commander", + "claude-ai", + "The request was read.", + answers=(original.reference,), + ) + renamed = replace(first, commander=commander.incarnation.renamed("commander-renamed")) + with comms.bus.bus.reading() as source: + assert not renamed.directive_messages(source, comms.registry.snapshot().names_for(commander.name)) + + CommsSendTool( + sender="worker", + target="commander", + body="The source records are available.", + ).apply(comms, ThreadCaller("worker")) + assert store.owner_request(first.id).directives == first.directives + assert store.owner_request(second.id).directives == second.directives + before = comms.bus.bus.revision() + with pytest.raises(ValueError): + ConnectorSendTool( + sender="claude-ai", + target="#any", + request=first.id, + body="Read the request records and report the result.", + ).apply(comms, CALLER) + with pytest.raises(ToolRefusedError): + ConnectorSendTool( + sender="claude-ai", + target="worker", + request=first.id, + body="Read the request records and report the result.", + ).apply(comms, CALLER) + assert comms.bus.bus.revision() == before + + request_main(["complete", second_id, "--why", "The independent request is finished."], ctx) + capsys.readouterr() + request_main(["list"], ctx) + remaining = capsys.readouterr().out + assert first_id in remaining and "P1" in remaining + assert second_id not in remaining + assert [request.id for request in store.open_requests(commander.incarnation)] == [first.id] + store.apply( + todo.id, + WithdrawTodo( + todo.revision, + commander.incarnation, + todo.held(), + "The isolated request check is complete.", + ), + ) + store.complete_request(commander.incarnation, first.id, "No pending order remains.") + assert not store.open_requests(commander.incarnation) + + awaiting = store.add_request( + commander.incarnation, "Wait for the commander's delivery verdict." + ) + awaiting_order = order.reissued(order.at.recorded_at("Order") + 1) + awaiting_todo = store.create( + "P2", + repository.github, + "Review the holder's delivery claim", + creator=commander, + assigner=commander.incarnation, + owner=worker, + generation="claim-review", + request=awaiting.id, + review_at=RecordedEventTime(awaiting_order.at.recorded_at("Order") + 60), + order=awaiting_order, + ) + awaiting_claim = OrderClaim.for_order( + awaiting_todo, + worker.incarnation, + 2, + "b" * 40, + "The request's work is ready for review.", + original.reference, + store=store, + ) + claimed = store.apply( + awaiting_todo.id, + ClaimOrder( + awaiting_todo.revision, + worker.incarnation, + awaiting_todo.held().generation, + awaiting_claim, + ), + ) + assert store.serving_orders(awaiting.id) == (claimed,) + with pytest.raises(ValueError): + store.complete_request(commander.incarnation, awaiting.id, "The claim is not yet accepted.") + assert store.for_request(first.id)[0].request == first.id + assert comms.registry.require("commander").goal == commander.goal + assert not any(thread.process_alive for thread in comms.registry.all_threads().values()) diff --git a/tests/test_pi_payloads.py b/tests/test_pi_payloads.py index 3d3464883b..92a8e11b76 100644 --- a/tests/test_pi_payloads.py +++ b/tests/test_pi_payloads.py @@ -5,7 +5,7 @@ import pytest -from agent_comms.field_codec import FieldCodec +from agent_comms.field_codec import FieldCodec, MalformedRecord from agent_comms.tool_results import NoToolDiff from agent_comms.pi_commands import GetState, PiCommand, Prompt from agent_comms.pi_counts import PiCount, ReportedCount, UnreportedCount @@ -173,7 +173,7 @@ def test_tracked_commit_observations_preserve_original_receipt_and_refuse_repeti ], ) def test_malformed_known_authority_fields_fail_at_boundary(record): - with pytest.raises((ValueError, TypeError)): + with pytest.raises(MalformedRecord): decode(record) diff --git a/tests/test_proof_build.py b/tests/test_proof_build.py index 4637807e58..f51bffb5d3 100644 --- a/tests/test_proof_build.py +++ b/tests/test_proof_build.py @@ -22,7 +22,7 @@ from agent_comms.comms import Comms from agent_comms.field_codec import FieldCodec from agent_comms.goal_actions import CreateProblemGoalAction, SetGoalAction -from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.goals import CountCheckResult, OrderCheck from agent_comms.installer.installed_packages import ( ArchiveRecord, CommitRecord, @@ -40,7 +40,6 @@ from agent_comms.todos import ( CommitDelivery, DoneTodoState, - GoalRef, OpenTodoState, FileDeliveryEvidence, OperationDelivery, @@ -147,7 +146,7 @@ def test_merge_in_head_cannot_close_until_the_recorded_build_contains_it( todo = comms.goals.todos.create( "P1", graph.repository.github, "Prove installation instead of inferring it from merge", creator=lead, assigner=lead.incarnation, owner=lead, generation="fixture-goal", - goal=GoalRef(lead.name, lead.created_at, "fixture-goal"), + request=comms.goals.todos.add_request(lead.incarnation, "Prove the installed build").id, review_at=RecordedEventTime(1000), order=merged, ) with pytest.raises(DeliveredCommitNotInstalledError): @@ -255,14 +254,9 @@ def test_installed_proof_closes_and_older_build_is_refused_on_copied_bus( artifact.write_text(works) source.registry.storage_rule = replace(source.registry.storage_rule, scratch_base=tmp_path) with CheckRoot(source, ("lead", "worker"), tmp_path / "proof" / "copied").open() as copied: - copied.goals.update_goal( - "lead", - SetGoalAction( - text="Record the copied application proof", - goal_id="request", - order=RequestGoalOrder(), - ), - ) + request = copied.goals.todos.add_request( + copied.registry.require("lead").incarnation, "Record the copied application proof", + ).id cases = ( ("P1", OperationDelivery(proof_build=installed), None), ("P2", OperationDelivery(proof_build=older), ProofBuildMismatchError), @@ -294,7 +288,7 @@ def test_installed_proof_closes_and_older_build_is_refused_on_copied_bus( order=CreateProblemGoalAction( problem=problem, order=order, - request="request", + request=request, review_at=RecordedEventTime(4102444800), commander="lead", assigner=caller.identity(copied), repo=graph.repository.github, diff --git a/tests/test_source_revision.py b/tests/test_source_revision.py index 075fac6359..0575f08740 100644 --- a/tests/test_source_revision.py +++ b/tests/test_source_revision.py @@ -10,6 +10,7 @@ from agent_comms.field_codec import MalformedRecord from agent_comms.goal_actions import OrderGoalAction from agent_comms.goals import GitCommandFailed +from agent_comms.owner_requests import OwnerRequestId from agent_comms.recorded_time import RecordedEventTime from agent_comms.thread_identity import RecordedThreadIncarnation from agent_comms.todos import FixScope, FixShape, LocatedProblemFinding @@ -60,7 +61,7 @@ def test_order_derives_its_only_source_revision_from_its_pull_request(tmp_path): location = FileLine('branch.py', 2) declaration = replace(declaration, owner=location, finding=LocatedProblemFinding(location), scope=FixScope(FixShape(parents=(location,), members=())), pull_requests=(source.link,)) - action = OrderGoalAction(problem='P1', order=declaration, request='request', + action = OrderGoalAction(problem='P1', order=declaration, request=OwnerRequestId('request'), review_at=RecordedEventTime(1000), commander='lead', expected_revision=1, assigner=RecordedThreadIncarnation('lead', 1.0)) action.require_sendable() diff --git a/tests/test_storage_violations.py b/tests/test_storage_violations.py index 27a69abc18..189b66a585 100644 --- a/tests/test_storage_violations.py +++ b/tests/test_storage_violations.py @@ -26,9 +26,10 @@ StorageViolation, UnitStorage, ) -from agent_comms.todos import GoalRef, PendingProblemOrder, PresentCheckout, ProgressRead +from agent_comms.todos import PendingProblemOrder, PresentCheckout, ProgressRead from agent_comms.ui_model.unit_state import UnitState from order_scope_fixture import FIX_SCOPE +from agent_comms.order_runs import MechanicalOrderRun @pytest.mark.parametrize( @@ -103,7 +104,7 @@ def test_storage_family_reads_once_and_renders_each_broken_fact( owner=worker, generation="storage-case", review_at=RecordedEventTime(1000), - goal=GoalRef("lead", lead.created_at, "storage-case"), + request=comms.goals.todos.add_request(lead.incarnation, "Fix the filesystem paths").id, order=PendingProblemOrder( id="storage-case-order", scope=FIX_SCOPE, owner=FileLine(__file__, 1), diff --git a/tests/test_todos.py b/tests/test_todos.py index 3a3f589a10..4374a3d6d6 100644 --- a/tests/test_todos.py +++ b/tests/test_todos.py @@ -1,4 +1,4 @@ -"""Todo creation and assignment retain exact owners, revisions and goal references.""" +"""Todo creation and assignment retain exact owners, revisions and request links.""" from __future__ import annotations @@ -28,9 +28,11 @@ from agent_comms.errors import ToolRefusedError from agent_comms.field_codec import FieldCodec from agent_comms.goal_actions import SetGoalAction -from agent_comms.goals import OrderCheck, RequestGoalOrder +from agent_comms.goals import OrderCheck from agent_comms.installer.practice import practice_copy from agent_comms.installer.runtime_resolution import CorePackage +from agent_comms.owner_requests import NewestOpenRequest +from agent_comms.plan_phases import PlanPhase from agent_comms.launch_root import LaunchRoot from agent_comms.order_runs import ApplicationOrderRun, MechanicalOrderRun, OrderRun from agent_comms.recorded_time import RecordedEventTime @@ -46,7 +48,6 @@ ClosedListedPullRequest, DeliverTodo, DoneTodoState, - GoalRef, MergedListedPullRequest, OperationDelivery, PendingProblemOrder, @@ -59,7 +60,6 @@ Todo, TodoCommand, TodoConflict, - TodoError, TodoState, TodoStore, UnmergedListedPullRequest, @@ -98,7 +98,7 @@ def _assign_process(root: str, checkout: str, name: str, created: float, gate) - row = store.assign( "P1", expected_revision=1, owner=thread(name, Path(root), created), assigner=thread("lead", Path(root), 1.0).incarnation, - generation=f"generation-{name}", goal=GoalRef("lead", 1.0, "goal-1"), + generation=f"generation-{name}", request=store.get("P1").request.require(), review_at=RecordedEventTime(1000.0), order=order(SourceRepository.of(Path(checkout))), ) return "won", row.assignment.owner @@ -106,12 +106,12 @@ def _assign_process(root: str, checkout: str, name: str, created: float, gate) - return "conflict", error.current.assignment.owner -def test_create_is_idempotent_and_goal_reference_is_historical(tmp_path: Path, source_repository) -> None: +def test_create_is_idempotent_and_request_link_is_historical(tmp_path: Path, source_repository) -> None: store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) - goal = GoalRef("lead", 1.0, "goal-1") + request = store.add_request(lead.incarnation, "Repair the sidebar").id original = store.create( - "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "Fix the sidebar", creator=lead, assigner=lead.incarnation, goal=goal, + "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "Fix the sidebar", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository), ) assert original.repo == RepositoryName.from_wire("OPENHCSDEV/agent-comms") @@ -119,19 +119,19 @@ def test_create_is_idempotent_and_goal_reference_is_historical(tmp_path: Path, s assert original.assignment == Assignment("lead", 1.0, "lead", 1.0, "initial") assert ( store.create( - "P1", RepositoryName.from_wire("openhcsdev/agent-comms"), "Fix the sidebar", creator=lead, assigner=lead.incarnation, goal=goal, + "P1", RepositoryName.from_wire("openhcsdev/agent-comms"), "Fix the sidebar", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository), ) == original ) with pytest.raises(TodoConflict): store.create( - "P1", RepositoryName.from_wire("openhcsdev/agent-comms"), "A different task", creator=lead, assigner=lead.incarnation, goal=goal, + "P1", RepositoryName.from_wire("openhcsdev/agent-comms"), "A different task", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository), ) 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 + assert store.get(original.id).request == request with pytest.raises(TodoConflict): store.apply("P1", ChangeTodoState(1, DoneTodoState, newer.incarnation, None)) assert store.get("P1") == original @@ -149,10 +149,11 @@ def changed(self, current: Todo) -> Todo: store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) + request = store.add_request(lead.incarnation, "Repair the inherited revision admission") original = store.create( "P1", RepositoryName.from_wire("org/repo"), "Original title", creator=lead, assigner=lead.incarnation, owner=lead, generation="first", - goal=GoalRef("lead", 1.0, "request"), review_at=RecordedEventTime(1000), + request=request.id, review_at=RecordedEventTime(1000), order=order(source_repository), ) @@ -168,8 +169,9 @@ def test_withdraw_retires_a_released_problem(tmp_path, source_repository): store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) worker = thread("worker", tmp_path, 2.0) + request = store.add_request(lead.incarnation, "Withdraw obsolete work").id todo = store.create("P1", RepositoryName.from_wire("org/repo"), "Obsolete problem", - creator=lead, assigner=lead.incarnation, owner=worker, generation="first", goal=GoalRef("lead", 1.0, "request"), + creator=lead, assigner=lead.incarnation, owner=worker, generation="first", request=request, review_at=RecordedEventTime(1000), order=order(source_repository)) released = store.apply(todo.id, ReleaseTodo(todo.revision, lead.incarnation, todo.held(), StrikeReason("The previous worker stopped before completing this assigned repair on main."))) @@ -193,8 +195,9 @@ def test_withdraw_retires_a_released_problem(tmp_path, source_repository): def test_withdraw_refuses_a_delivered_problem(tmp_path, released, source_repository): store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) + request = store.add_request(lead.incarnation, "Retain delivered work").id todo = store.create("P1", RepositoryName.from_wire("org/repo"), "Delivered problem", - creator=lead, assigner=lead.incarnation, owner=lead, generation="first", goal=GoalRef("lead", 1.0, "request"), + creator=lead, assigner=lead.incarnation, owner=lead, generation="first", request=request, review_at=RecordedEventTime(1000), order=order(source_repository)) delivered = store.apply(todo.id, DeliverTodo(todo.revision, lead.incarnation, OperationDelivery(at=RecordedEventTime(20)), todo.order.id)) @@ -210,8 +213,9 @@ def test_withdraw_refuses_a_delivered_problem(tmp_path, released, source_reposit def test_two_processes_assign_one_todo_before_any_worker_is_started(tmp_path: Path, source_repository) -> None: store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) + request = store.add_request(lead.incarnation, "Assign one implementation").id store.create( - "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "One implementation", creator=lead, assigner=lead.incarnation, goal=GoalRef("lead", 1.0, "goal-1"), + "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "One implementation", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository), ) with Manager() as manager, ProcessPoolExecutor(max_workers=2) as pool: @@ -233,16 +237,15 @@ def test_two_processes_assign_one_todo_before_any_worker_is_started(tmp_path: Pa def test_validation_rejects_ambiguous_identity_and_bad_state(tmp_path: Path, source_repository) -> None: store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) + request = store.add_request(lead.incarnation, "Validate the repository and state").id for index, repo in enumerate(("org/repo/branch", "../repo", "org/", "org/repo name")): with pytest.raises(ValueError): store.create( - f"invalid-{index}", RepositoryName.from_wire(repo), "task", creator=lead, assigner=lead.incarnation, goal=GoalRef("lead", 1.0, "goal-1"), + f"invalid-{index}", RepositoryName.from_wire(repo), "task", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository), ) - with pytest.raises(TodoError): - GoalRef("lead", float("nan"), "goal-1") store.create( - "P1", RepositoryName.from_wire("org/repo"), "task", creator=lead, assigner=lead.incarnation, goal=GoalRef("lead", 1.0, "goal-1"), + "P1", RepositoryName.from_wire("org/repo"), "task", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository), ) with pytest.raises(ValueError): @@ -252,8 +255,9 @@ def test_validation_rejects_ambiguous_identity_and_bad_state(tmp_path: Path, sou def test_merged_order_keeps_its_checked_fields(tmp_path: Path, source_repository) -> None: store = TodoStore(tmp_path) lead = thread("lead", tmp_path, 1.0) + request = store.add_request(lead.incarnation, "Repair the sidebar").id todo = store.create( - "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "Fix the sidebar", creator=lead, assigner=lead.incarnation, goal=GoalRef("lead", 1.0, "goal-1"), + "P1", RepositoryName.from_wire("OpenHCSDev/agent-comms"), "Fix the sidebar", creator=lead, assigner=lead.incarnation, request=request, owner=lead, generation="initial", review_at=RecordedEventTime(1000.0), order=order(source_repository)) merged = replace(todo, order=todo.order.checked().merged("2b9c1f1ab")) cells = merged.order.board_cells(merged) @@ -270,6 +274,15 @@ def pr_commander(tmp_path, monkeypatch, proof_build_checkouts, configured_reposi graph = next(graph for graph in proof_build_checkouts if graph.repository.github == CorePackage.repository) source = graph.repository + # Borrow only the plan inputs; the proof fixture owns the commit graph. + current = Path(__file__).resolve().parents[1] + shutil.copytree(current / 'plans', source.checkout / 'plans') + module = source.module_file(PlanPhase.__module__) + (source.checkout / module).parent.mkdir(parents=True) + shutil.copyfile(current / module, source.checkout / module) + source.git(source.checkout, 'add', 'plans', 'src') + source.git(source.checkout, 'commit', '-qm', 'Declare current commander plans') + source.git(source.checkout, 'push', '-q', 'origin', 'HEAD:main') merged = graph.installed for number in (1, 2): source.git(source.checkout, 'push', 'origin', f'{merged}:refs/pull/{number}/head') @@ -297,17 +310,9 @@ def pr_commander(tmp_path, monkeypatch, proof_build_checkouts, configured_reposi rule = StorageRule(checkout_base=source.checkout.parent) (tmp_path / "storage.json").write_text(json.dumps(FieldCodec.encode(rule))) Registration(tmp_path / "registry.json").register(lead) - from agent_comms.plan_phases import PlanPhase - - config = configured_repository(Comms(tmp_path), source) - declared = source.module_file(PlanPhase.__module__) - plans = config.module_repo(PlanPhase.__module__) - target = source.checkout / declared - target.parent.mkdir(parents=True, exist_ok=True) - target.write_bytes((plans.checkout / plans.module_file(PlanPhase.__module__)).read_bytes()) - source.git(source.checkout, 'add', str(declared)) - source.git(source.checkout, 'commit', '-qm', 'Declare the fixture plan owners') - source.git(source.checkout, 'push', 'origin', 'main') + configured_repository(Comms(tmp_path), source) + bus = Comms(tmp_path) + bus.goals.todos.add_request(lead.incarnation, "Repair the copied order workflow") return Commander(commander=lead.name, root=tmp_path, state=tmp_path / "state", repos=(source,), worktrees=source.checkout.parent, scratch=tmp_path, disk=tmp_path) @@ -318,7 +323,7 @@ def pr_order(ctx, pid, run: OrderRun = MechanicalOrderRun()): lead = ctx.comms.registry.require(ctx.commander) return ctx.comms.goals.todos.create( pid, source.github, "Fix the sidebar", creator=lead, assigner=lead.incarnation, owner=lead, - generation="pr-work", goal=GoalRef(lead.name, 1.0, "pr-work"), + generation="pr-work", request=NewestOpenRequest().for_order(ctx.comms.goals.todos, lead.incarnation), review_at=RecordedEventTime(1000), order=replace(order(source), id=f"order-{pid}", run=run)) @@ -404,7 +409,7 @@ def test_watcher_git_and_github_reads_leave_wire_free(pr_commander, monkeypatch) ctx.write(ctx.root / "config-home") c.goals.update_goal(ctx.commander, SetGoalAction( text="Keep the commander request active for both watcher passes", - order=RequestGoalOrder(), goal_id="pr-work")) + goal_id="pr-work")) lead = c.registry.require(ctx.commander) c.registry.register(replace(lead, process_identity=ProcessIdentity.capture(os.getpid()))) original = adopt_pr(ctx, pr_order(ctx, "P1", ApplicationOrderRun( @@ -504,7 +509,7 @@ def test_installed_merge_closes_only_the_run_that_has_been_proven(pr_commander): ctx.comms.goals.todos.create( f"P{index}", source.github, "Keep application work held until its run is recorded", creator=lead, assigner=lead.incarnation, owner=worker, generation="merge-run", - goal=GoalRef(lead.name, lead.created_at, "merge-run"), + request=NewestOpenRequest().for_order(ctx.comms.goals.todos, lead.incarnation), review_at=RecordedEventTime(1000), order=replace(order(source), id=f"merge-order-{index}", run=run, pull_requests=(link,))) diff --git a/tests/test_unit_state.py b/tests/test_unit_state.py index 282d6c501e..0890a43f8e 100644 --- a/tests/test_unit_state.py +++ b/tests/test_unit_state.py @@ -238,15 +238,13 @@ def test_heartbeat_keeps_every_unit_when_one_checkout_is_unreadable( assert isinstance(cards[0], Unreadable) assert {card.unit.name for card in cards} >= {"worker", "outsider"} heartbeat.pass_body(held_bus, observed) - page = held_bus.bus.log.read_page( - MessagePageRequest( - DMDisplayScope.capture( - heartbeat.sender, - "lead", - held_bus.registry.snapshot(), - ) + page = MessagePageRequest( + DMDisplayScope.capture( + heartbeat.sender, + "lead", + held_bus.registry.snapshot(), ) - ) + ).read_opened(held_bus.bus.bus) message = next(row for row in page.messages if row.sender == heartbeat.sender) assert all(card.line() in message.body for card in cards) assert message.body.index(cards[0].line()) < message.body.index(cards[-1].line()) diff --git a/tests/test_unowned_checkout_work.py b/tests/test_unowned_checkout_work.py index 614fc1b62d..14c151116a 100644 --- a/tests/test_unowned_checkout_work.py +++ b/tests/test_unowned_checkout_work.py @@ -29,8 +29,7 @@ ) from agent_comms.comms import Comms from agent_comms.field_codec import FieldCodec -from agent_comms.goal_actions import SetGoalAction -from agent_comms.goals import OrderCheck, RequestGoalOrder +from agent_comms.goals import OrderCheck from agent_comms.installer.practice import practice_copy from agent_comms.launch_root import LaunchRoot from agent_comms.order_runs import MechanicalOrderRun @@ -39,7 +38,7 @@ from agent_comms.storage_rules import StorageDenied, StorageRule, shell_storage_effects from agent_comms.thread_execution import ExternalThreadExecution from agent_comms.thread_identity import CoreWatcherIncarnation -from agent_comms.todos import GoalRef, PendingProblemOrder, ProgressRead, ReleaseTodo, StrikeReason +from agent_comms.todos import PendingProblemOrder, ProgressRead, ReleaseTodo, StrikeReason from agent_comms.transcript_events import AssistantTranscript from agent_comms.ui_model.unit_state import UnitState, Unreadable from agent_comms.work_branches import UnreadableCheckoutRead, WorkBranches @@ -133,8 +132,8 @@ def checkout_bus(tmp_path: Path, configured_repository, installed_runtime) -> Co for name in ('lead', 'worker', 'idle'): c.registry.declare(c.registry.new_thread( name, frozenset(), str(home), execution=ExternalThreadExecution)) - c.goals.update_goal('lead', SetGoalAction( - text='Keep changes with their author', order=RequestGoalOrder(), goal_id='request')) + request = c.goals.todos.add_request( + c.registry.require('lead').incarnation, 'Keep changes with their author').id lead, worker = c.registry.require('lead'), c.registry.require('worker') now = time.time() order = PendingProblemOrder( @@ -148,7 +147,7 @@ def checkout_bus(tmp_path: Path, configured_repository, installed_runtime) -> Co problem, RepositoryName.from_wire('OpenHCSDev/agent-comms'), 'Keep changes with their author', creator=lead, assigner=lead.incarnation, owner=worker, generation='shared-work', review_at=RecordedEventTime(now + 3600), - order=order, goal=GoalRef('lead', lead.created_at, 'request')) + order=order, request=request) # Both current IDs change, as they do when Core hands work out again. for problem in ('P1', 'P2'): row = c.goals.todos.get(problem) @@ -156,12 +155,12 @@ def checkout_bus(tmp_path: Path, configured_repository, installed_runtime) -> Co problem, expected_revision=row.revision, owner=worker, assigner=lead.incarnation, generation='reissued-work', review_at=RecordedEventTime(now + 3600), order=order.reissued(now + 1), - goal=GoalRef('lead', lead.created_at, 'request')) + request=request) c.goals.todos.create( 'P3', RepositoryName.from_wire('OpenHCSDev/agent-comms'), 'Keep unrelated work separate', creator=lead, assigner=lead.incarnation, owner=worker, generation='other-work', review_at=RecordedEventTime(now + 3600), - order=order.reissued(now + 2), goal=GoalRef('lead', lead.created_at, 'request')) + order=order.reissued(now + 2), request=request) row = c.goals.todos.get('P2') assert {related.id for related in c.goals.todos.related_problems(row)} == {'P1', 'P2'} c.goals.todos.apply('P2', ReleaseTodo( diff --git a/tests/verify_connector_tools.py b/tests/verify_connector_tools.py index e1b61ace14..749282c364 100644 --- a/tests/verify_connector_tools.py +++ b/tests/verify_connector_tools.py @@ -31,7 +31,8 @@ from agent_comms.connector_store import ConnectorStore from agent_comms.field_codec import FieldCodec from agent_comms.goal_actions import CreateProblemGoalAction, PausedGoalAction, SetGoalAction -from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.goals import CountCheckResult, OrderCheck +from agent_comms.owner_requests import NewestOpenRequest from agent_comms.progress_claims import WorkingClaim from agent_comms.recorded_time import RecordedEventTime from agent_comms.thread_status import StoppedThreadStatus @@ -60,6 +61,8 @@ def seed(root: Path, project: Path) -> None: comms.goals.update_goal( "claude-ai", SetGoalAction(text="Measure the connector calls.", goal_id="probe-goal") ) + comms.goals.todos.add_request(comms.registry.require('claude-ai').incarnation, + 'Measure the connector calls.') original = comms.messaging.send("worker", "claude-ai", "Original scratch history.").message assert original.sender == "worker" for worker in threading.enumerate(): @@ -84,7 +87,8 @@ def arguments(comms: Comms, name: str, project: Path, checkout: Path) -> dict: order = CreateProblemGoalAction( problem="P1", order=pending, - request="request", + request=NewestOpenRequest().for_order(comms.goals.todos, + comms.registry.require('claude-ai').incarnation), review_at=RecordedEventTime(time.time() + 600), commander="claude-ai", assigner=comms.registry.require('claude-ai').incarnation, repo=RepositoryName.from_wire("OpenHCSDev/agent-comms"), @@ -105,18 +109,8 @@ def arguments(comms: Comms, name: str, project: Path, checkout: Path) -> dict: "comms_order_work", "comms_unjudged", } - if name in order_tools: - from agent_comms.goal_actions import ClearGoalAction - - comms.goals.update_goal("claude-ai", ClearGoalAction()) - comms.goals.update_goal( - "claude-ai", - SetGoalAction( - text="Measure the connector calls.", goal_id="request", order=RequestGoalOrder() - ), - ) - if name != "comms_todo_create": - CommsTodoCreateTool(owner="worker", order=order).apply(comms, ThreadCaller("claude-ai")) + if name in order_tools - {'comms_todo_create'}: + CommsTodoCreateTool(owner="worker", order=order).apply(comms, ThreadCaller("claude-ai")) if name == "comms_resume_goal": comms.goals.update_goal( "claude-ai", PausedGoalAction(progress="Paused only on this scratch bus.") diff --git a/tools/carry_rts_orders.py b/tools/carry_rts_orders.py index 093a383c61..7d6917f2d6 100644 --- a/tools/carry_rts_orders.py +++ b/tools/carry_rts_orders.py @@ -25,6 +25,7 @@ boundary_decoder, ) from agent_comms.goals import OrderCheck +from agent_comms.owner_requests import NewestOpenRequest from agent_comms.recorded_time import RecordedEventTime from agent_comms.thread_identity import ThreadIncarnation @@ -191,6 +192,7 @@ def carry(rts: Path, root: Path, apply: bool, retire: bool) -> None: if apply: comms = Comms(root) actor = comms.registry.require("claude-code").incarnation + request = NewestOpenRequest().for_order(comms.goals.todos, actor) with todos.TodoStore(root).writing() as db: existing = {todo.id: todo for todo in todos.Todo.stream(db)} for pid, (unit, row, order) in selected.items(): @@ -206,7 +208,9 @@ def carry(rts: Path, root: Path, apply: bool, retire: bool) -> None: repo = Repo.of(Path(order.owner.file).parent) text = problems[pid].text proposed = todos.Assignment.of(owner, actor, generation) - todo = todos.Todo.ordered(pid, repo.github, text, actor, proposed, due, order) + todo = todos.Todo.ordered( + pid, repo.github, text, actor, proposed, due, order, request + ) todo = todos.AssignTodo.by( actor, todo.revision, @@ -214,7 +218,7 @@ def carry(rts: Path, root: Path, apply: bool, retire: bool) -> None: generation, due, row.merged.wrap(order), - todo.goal, + todo.request.require(), todos.NoSourceReference(), ).apply(todo) history = tuple( diff --git a/tools/cutover/carry_bus_history.py b/tools/cutover/carry_bus_history.py index fe7796f735..56986a75cb 100644 --- a/tools/cutover/carry_bus_history.py +++ b/tools/cutover/carry_bus_history.py @@ -104,6 +104,10 @@ def pending(self) -> int: ... @abstractmethod def require_snapshot(self) -> None: ... + @abstractmethod + def carried_registry(self, mapped: Path) -> CarryCatalogSource: + """Reseal a registry carried before the bus format changes.""" + @abstractmethod def converted( self, @@ -185,6 +189,13 @@ def require_snapshot(self) -> None: ): raise RelationViolationError(f"Retired archive changed before carrying: {original}") + def carried_registry(self, mapped: Path) -> CarryCatalogSource: + return replace( + self, root=str(mapped), + snapshot_bus_revision=FileRevision.capture(mapped / BusHistory.filename), + snapshot_registry_revision=FileRevision.capture(RegistryFile.path_in(mapped)), + ) + def converted( self, root: Path, @@ -221,23 +232,25 @@ def pending(self) -> int: def require_snapshot(self) -> None: self.validate() - def converted( - self, - root: Path, - original_root: Path, - bindings: Mapping[RecordedBusIdentity, RecordedBusIdentity], - ) -> tuple[HistorySource, Carried]: - mapped = self.mapped_root(root, original_root) + def carried_registry(self, mapped: Path) -> CarryCatalogSource: bus = Bus(Bus.path_in(mapped)) source = replace( - self, - root=str(mapped), + self, root=str(mapped), snapshot_bus_identity=RecordedBusIdentity.capture(bus.path), snapshot_bus_revision=bus.revision(), snapshot_registry_revision=FileRevision.capture(RegistryFile.path_in(mapped)), ) source.validate() - return source, Carried(0) + return source + + def converted( + self, + root: Path, + original_root: Path, + bindings: Mapping[RecordedBusIdentity, RecordedBusIdentity], + ) -> tuple[HistorySource, Carried]: + mapped = self.mapped_root(root, original_root) + return self.carried_registry(mapped), Carried(0) class HistoryDecision(SubclassUnion, ABC, affix="HistoryDecision"): diff --git a/tools/cutover/carry_owner_requests.py b/tools/cutover/carry_owner_requests.py new file mode 100644 index 0000000000..1342ed6c83 --- /dev/null +++ b/tools/cutover/carry_owner_requests.py @@ -0,0 +1,368 @@ +"""Move retained request goals into Core rows after the old owners stop. + +Delete after every bus has run this step. The retired wire tag is understood +only here; no request-goal member or compatibility decoder is registered. +""" + +from __future__ import annotations + +import json +import sqlite3 +import time +from contextlib import ExitStack, closing +from dataclasses import dataclass +from pathlib import Path +from typing import Annotated + +from agent_comms.errors import RelationViolationError +from agent_comms.field_codec import FieldCodec +from agent_comms.goal_history import GoalHistoryEntry, GoalHistoryStore +from agent_comms.goals import Goal, UnorderedGoalOrder +from agent_comms.installer.carry import Carried, Carry +from agent_comms.historical_views import HistoryArchive +from agent_comms.lifecycle_capability import Complete +from agent_comms.locked_store import SQLiteFile +from agent_comms.owner_requests import ( + CompletedRequestState, + OpenRequestState, + OwnerRequestId, + RequestDirective, +) +from agent_comms.registry_store import RegistryStore +from agent_comms.store_files import _atomic_write_text, _store_lock +from agent_comms.thread_identity import RecordedThreadIncarnation +from agent_comms.todos import OwnerRequest, Todo, TodoStore +from agent_comms.typed_table import ( + Column, + JsonStorage, + SQLiteSchemaObject, + TextStorage, + TypedRow, + TypedTable, + WriterOwnedFile, +) +from .carry_bus_history import BusHistory, CarryCatalogSource + + +@dataclass(frozen=True) +class StoredRequestGoal(TypedRow): + """Decode the old directive payload separately from the normalized Goal.""" + + goal: Annotated[Goal, Column(storage=JsonStorage)] + directives: Annotated[tuple[RequestDirective, ...], Column(storage=JsonStorage)] + owner_created_at: float + + def request(self, commander: RecordedThreadIncarnation, created_at: float) -> OwnerRequest: + if commander.created_at != self.owner_created_at: + raise RelationViolationError( + "Retained request belongs to another commander incarnation." + ) + state = ( + CompletedRequestState(self.goal.progress) + if self.goal.state.items_with(Complete, self.goal) + else OpenRequestState() + ) + return OwnerRequest( + OwnerRequestId(self.goal.id), + commander, + self.goal.text, + state, + self.directives, + created_at, + ) + + +@dataclass(frozen=True) +class CurrentRequestGoal(StoredRequestGoal): + name: str + path: str + + +@dataclass(frozen=True) +class HistoricalRequestGoal(StoredRequestGoal): + sequence: int + observed_at: float + + +@dataclass(frozen=True) +class StoredRequestLink(TypedRow): + request: Annotated[OwnerRequestId, Column(storage=TextStorage)] + owner: str + owner_created: float + + +# Both the query and the replacement use the new declaration's codec. The old +# optional directives field had the empty-tuple default. +UNORDERED = json.dumps(FieldCodec.encode(UnorderedGoalOrder())) + + +def memory_copy(lifetime: ExitStack, path: Path) -> sqlite3.Connection: + db = lifetime.enter_context(closing(SQLiteFile.writer(":memory:"))) + if path.exists(): + with closing(SQLiteFile.read_snapshot(path)) as original: + original.backup(db) + return db + + +def historical( + db: sqlite3.Connection, column: str, *, where: str = "1" +) -> list[HistoricalRequestGoal]: + return HistoricalRequestGoal.read( + GoalHistoryEntry.orders_with_replacement( + db, column, UNORDERED, order_kind="request", where=where + ) + ) + + +class OwnerRequests(Carry): + precedes = (BusHistory,) + + def backup_files(self) -> tuple[Path, ...]: + sources = CarryCatalogSource.read(self.root, self.original_root) + roots = (self.root, *(source.mapped_root(self.root, self.original_root) for source in sources)) + paths = (HistoryArchive.path_in(self.root), + *(path for root in roots for path in self.root_files(root))) + for path in paths: + self.require_confined(self.root, path) + return tuple(path for path in paths if path.exists()) + + @staticmethod + def root_files(root: Path) -> tuple[Path, ...]: + registry = RegistryStore.at(root) + return ( + *registry.written, + TodoStore.path_in(root), + *TodoStore.source_paths(root), + GoalHistoryStore.path_in(root), + ) + + def carry(self, *, apply: bool) -> Carried: + with _store_lock(HistoryArchive.path_in(self.root)): + self.backup_files() + sources = CarryCatalogSource.read(self.root, self.original_root) + results = [(self.root, self.carry_root(self.root, apply=apply))] + rebound = [] + resealed = False + for source in sources: + mapped = source.mapped_root(self.root, self.original_root) + result = self.carry_root(mapped, apply=apply) + results.append((mapped, result)) + if apply and RegistryStore.at(mapped).path.name in result.rewritten: + source = source.carried_registry(mapped) + resealed = True + rebound.append(source) + rewritten = [str(root.relative_to(self.root) / name) + for root, result in results for name in result.rewritten] + if resealed: + _atomic_write_text(HistoryArchive.path_in(self.root), json.dumps(FieldCodec.encode(tuple(rebound)))) + rewritten.append(HistoryArchive.filename) + detail: dict[str, int] = {} + for _, result in results: + for key, value in result.detail.items(): + detail[key] = detail.get(key, 0) + value + return Carried(sum(result.changed for _, result in results), detail, tuple(rewritten)) + + def carry_root(self, root: Path, *, apply: bool) -> Carried: + registry = RegistryStore.at(root) + todo_path, history_path = TodoStore.path_in(root), GoalHistoryStore.path_in(root) + if not registry.path.exists(): + if todo_path.exists() or history_path.exists(): + raise RelationViolationError("Request carry needs the retained registry.") + return Carried(0) + # The installer holds the wire lock and stops old owners. Use the existing + # registry lock/guard for its publication, not a second migration lock. + with registry.locked(), ExitStack() as lifetime: + registry.private_guard_unlocked() + raw = registry.path.read_text() + sql = lifetime.enter_context(closing(SQLiteFile.writer(":memory:"))) + current = CurrentRequestGoal.read( + sql.execute( + "SELECT key AS name, fullkey || '.goal.order' AS path, " + "json_extract(value, '$.created_at') AS owner_created_at, " + "json_set(json_extract(value, '$.goal'), '$.order', json(?)) AS goal, " + "coalesce(json_extract(value, '$.goal.order.directives'), '[]') AS directives " + "FROM json_each(?, '$.threads') " + "WHERE json_extract(value, '$.goal.order.kind')='request'", + (UNORDERED, raw), + ) + ) + for current_source in current: + (raw,) = sql.execute( + "SELECT json_set(?, ?, json(?))", (raw, current_source.path, UNORDERED) + ).fetchone() + document = registry._decode(FieldCodec.parse(raw)) + owners = {} + for thread in document.threads.values(): + recorded_commander = thread.incarnation.require_recorded() + owners[recorded_commander.created_at] = recorded_commander + todos = memory_copy(lifetime, todo_path) + history = memory_copy(lifetime, history_path) + todo_columns = Todo.stored_columns(todos) + old_links = "goal" in todo_columns + if old_links: + for link in StoredRequestLink.read( + todos.execute( + "SELECT json_extract(goal,'$.goal_id') AS request, " + "json_extract(goal,'$.owner') AS owner, " + "json_extract(goal,'$.owner_created') AS owner_created " + f'FROM "{Todo.declared_name}" WHERE goal IS NOT NULL GROUP BY goal' + ) + ): + # Deleted commanders still have their original incarnation + # on the orders they issued. Do not manufacture a new actor. + owners.setdefault( + link.owner_created, + RecordedThreadIncarnation(link.owner, link.owner_created), + ) + if OwnerRequest.stored_columns(todos): + differences = SQLiteSchemaObject.differences( + TypedTable.objects_of(todos, (OwnerRequest,)), + OwnerRequest.schema_objects(), + ) + if differences: + raise RelationViolationError( + f"Request table differs from its declaration: {differences}" + ) + requests = {request.id: request for request in OwnerRequest.stream(todos)} + else: + requests = {} + existing = dict(requests) + sources: dict[str, list[HistoricalRequestGoal]] = {} + first_seen: dict[OwnerRequestId, float] = {} + if GoalHistoryEntry.stored_columns(history): + for column in GoalHistoryEntry.endpoint_columns(): + sources[column] = historical(history, column) + committed = sorted( + ( + source + for column in sources + for source in historical(history, column, where="state='committed'") + ), + key=lambda source: source.sequence, + ) + for source in committed: + identity = OwnerRequestId(source.goal.id) + first_seen.setdefault(identity, source.observed_at) + try: + commander = owners[source.owner_created_at] + except KeyError: + raise RelationViolationError( + f"Request {identity.value} has no retained commander incarnation." + ) from None + requests[identity] = source.request(commander, first_seen[identity]) + for current_source in current: + identity = OwnerRequestId(current_source.goal.id) + first_seen.setdefault(identity, time.time()) + requests[identity] = current_source.request( + document.threads[current_source.name].incarnation.require_recorded(), + first_seen[identity], + ) + for identity, previous in existing.items(): + if identity in requests: + proposed = requests[identity] + if (previous.commander, previous.text, previous.state, previous.directives) != ( + proposed.commander, + proposed.text, + proposed.state, + proposed.directives, + ): + raise RelationViolationError( + f"Request {identity.value} differs while finishing its carry." + ) + requests[identity] = previous + # Validate every historical endpoint, including pending/aborted/ + # uncertain intents. They cannot become new accepted directives. + prefixes = {} + for column, snapshots in sources.items(): + for source in snapshots: + identity = OwnerRequestId(source.goal.id) + if identity not in requests: + raise RelationViolationError( + f"Request {identity.value} at history {source.sequence} " + "exists only in an unresolved intent." + ) + prefixes[column, source.sequence] = requests[identity].at_revision( + source.directives + ) + new_rows = len(requests.keys() - existing.keys()) + todo_changed = old_links or new_rows > 0 + if todo_columns: + if not old_links and "request" not in todo_columns: + raise RelationViolationError( + "Todo has neither the retired goal nor a request link." + ) + if not OwnerRequest.stored_columns(todos): + OwnerRequest.reconcile_schema(todos, WriterOwnedFile()) + elif requests: + TodoStore.reconcile_schema(todos, WriterOwnedFile()) + todo_changed = True + for request in requests.values(): + request.upsert(todos) + linked = 0 + if old_links: + (linked,) = todos.execute( + f'SELECT count(*) FROM "{Todo.declared_name}" WHERE goal IS NOT NULL' + ).fetchone() + Todo.rebuild(todos, {"request": "json_extract(goal,'$.goal_id')"}) + if todo_columns or requests: + TodoStore.require_schema(todos) + for _ in Todo.stream(todos): + pass + if list(todos.execute("PRAGMA foreign_key_check")): + raise RelationViolationError( + "Carried Todo request links violate their foreign keys." + ) + history_columns = GoalHistoryEntry.stored_columns(history) + history_changed = bool( + history_columns and (prefixes or GoalHistoryEntry.schema_differs(history)) + ) + if history_changed: + GoalHistoryEntry.reconcile_schema(history, WriterOwnedFile()) + for column, snapshots in sources.items(): + for source in snapshots: + GoalHistoryEntry.move_request_order( + history, + column, + source.sequence, + prefixes[column, source.sequence], + UNORDERED, + ) + if history_columns: + GoalHistoryEntry.require_schema(history) + for entry in GoalHistoryEntry.stream(history): + for prefix in entry.request_revisions: + prefix.validate(requests) + rewritten: list[str] = [] + if apply: + # Prepared in-memory files are private to this carry. Replacing a + # retained file requires the installer's actual maintenance hold. + if todo_changed: + TodoStore.schema_change(TodoStore.path_in(self.root)).require() + if history_changed: + GoalHistoryStore.schema_change(GoalHistoryStore.path_in(self.root)).require() + for changed, path, prepared in ( + (todo_changed, todo_path, todos), + (history_changed, history_path, history), + ): + if changed: + prepared.commit() + with closing(SQLiteFile.writer(path)) as destination: + prepared.backup(destination) + path.chmod(0o600) + rewritten.append(str(path.relative_to(root))) + if todo_changed: + TodoStore.publish_change(todo_path) + rewritten.extend(str(path.relative_to(root)) for path in TodoStore.source_paths(root)) + if current: + registry._write_unlocked(raw) + rewritten.extend(str(path.relative_to(root)) for path in registry.written) + return Carried( + new_rows + int(old_links) + int(history_changed) + len(current), + detail={ + "requests": new_rows, + "linked_todos": linked, + "directives": sum(len(request.directives) for request in requests.values()), + "history_endpoints": len(prefixes), + }, + rewritten=tuple(rewritten), + ) diff --git a/tools/proofs/cn_phase3_record.py b/tools/proofs/cn_phase3_record.py index 40a4a93c99..2e6a0c2a61 100644 --- a/tools/proofs/cn_phase3_record.py +++ b/tools/proofs/cn_phase3_record.py @@ -20,7 +20,8 @@ from agent_comms.repo import Repo from agent_comms.comms import Comms from agent_comms.goal_actions import CreateProblemGoalAction, SetGoalAction -from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.goals import CountCheckResult, OrderCheck +from agent_comms.owner_requests import NewestOpenRequest from agent_comms.progress_claims import MergedClaim from agent_comms.recorded_time import RecordedEventTime from agent_comms.todos import ( @@ -76,14 +77,8 @@ def setup() -> None: comms.registry.declare( comms.registry.new_thread(name, frozenset(), str(CHECKOUT), created_at=time.time()) ) - comms.goals.update_goal( - "record-lead", - SetGoalAction( - text="Confirm the order record through the actual Core tools on this isolated bus", - goal_id="record-request", - order=RequestGoalOrder(), - ), - ) + comms.goals.todos.add_request(comms.registry.require('record-lead').incarnation, + 'Confirm the order record through the actual Core tools on this isolated bus') comms.goals.update_goal( "record-worker", SetGoalAction( @@ -97,6 +92,9 @@ def setup() -> None: def create_order(problem: str = "P1") -> None: now = time.time() repo = Repo.of(CHECKOUT) + comms = Comms(ROOT) + request = NewestOpenRequest().for_order( + comms.goals.todos, comms.registry.require('record-lead').incarnation) cli( "record-lead", CommsTodoCreateTool( @@ -105,7 +103,7 @@ def create_order(problem: str = "P1") -> None: problem=problem, repo=repo.github, text="Show the real verdict and strike records without a second source", - request="record-request", + request=request, commander="record-lead", assigner=Comms(ROOT).registry.require("record-lead").incarnation, review_at=RecordedEventTime(now + 3600), diff --git a/tools/prove_commander_dispatch.py b/tools/prove_commander_dispatch.py index 219e626c1d..59588a09b5 100644 --- a/tools/prove_commander_dispatch.py +++ b/tools/prove_commander_dispatch.py @@ -28,8 +28,7 @@ from agent_comms.comms import Comms from agent_comms.declared_family import DeclaredFamily from agent_comms.field_codec import Argument, FieldCodec, PathText -from agent_comms.goal_actions import SetGoalAction -from agent_comms.goals import CountCheckResult, OrderCheck, RequestGoalOrder +from agent_comms.goals import CountCheckResult, OrderCheck from agent_comms.messages import Message from agent_comms.order_runs import MechanicalOrderRun from agent_comms.plan_phases import RecordPlanePhase @@ -150,14 +149,8 @@ def apply(self, project: Path) -> int: ), StoppedThreadStatus(), ) - comms.goals.update_goal( - commander.name, - SetGoalAction( - text="Verify one rotation-started native unit on this scratch bus only.", - goal_id=uuid4().hex, - order=RequestGoalOrder(), - ), - ) + comms.goals.todos.add_request(commander.incarnation, + 'Verify one rotation-started native unit on this scratch bus only.') CommanderConfig( commander=commander.name, root=root,