diff --git a/src/agent_comms/acp_extension.py b/src/agent_comms/acp_extension.py index 8d953813ec..666fec1122 100644 --- a/src/agent_comms/acp_extension.py +++ b/src/agent_comms/acp_extension.py @@ -12,7 +12,7 @@ from abc import abstractmethod from dataclasses import dataclass, field, replace -from typing import TYPE_CHECKING, Annotated, Callable, ClassVar +from typing import TYPE_CHECKING, Annotated, Callable, ClassVar, Self from .acp_failure import ( @@ -52,8 +52,10 @@ from .reported_turn import ReportedTurn, ReportedTurnState from .routing import MessageRoute, NoMessageRoute from .thread_identity import ClaimScoped, ThreadIncarnation -from .tool_result import DeclaredReply -from .transcripts import NativeCommittedPrefix, TranscriptCursor, TranscriptPage, TranscriptReadIdentity +from .tool_result import ToolResult +from .transcripts import ( + NativeCommittedPrefix, TranscriptCursor, TranscriptPage, TranscriptRead, TranscriptReadIdentity, +) from .turn_context import ContextManifest from .turn_lease import TurnState from .working_memory_labels import ModelLabel @@ -138,7 +140,7 @@ def of(cls, *updates: AgentCommsUpdate) -> UpdateMetadata: @dataclass(frozen=True) -class GoalSnapshotReply(DeclaredReply, GoalSnapshot): +class GoalSnapshotReply(ToolResult, GoalSnapshot): """A goal request's reply: the owner's goal snapshot and its turn state.""" execution: Annotated[GoalExecution, WireName("goalExecution")] @@ -698,13 +700,13 @@ class TranscriptDraftUpdate(AgentCommsUpdate): @dataclass(frozen=True) -class TranscriptSnapshotUpdate(AgentCommsUpdate): +class TranscriptSnapshotUpdate(AgentCommsUpdate, ToolResult): page: TranscriptPage identity: TranscriptReadIdentity draft: DraftRow | None = None @classmethod - def from_read(cls, read): + def from_read(cls, read: TranscriptRead) -> Self: page, draft = read.read_snapshot() return cls(page, read.identity, draft.external_row()) diff --git a/src/agent_comms/bus_read.py b/src/agent_comms/bus_read.py new file mode 100644 index 0000000000..a1fc6ae2c3 --- /dev/null +++ b/src/agent_comms/bus_read.py @@ -0,0 +1,119 @@ +"""One declaration per bus read: the operation owns its arguments and its read once. + +A ``BusRead`` member is the operation. Each boundary that serves it keeps only what is +its own: the UI its root, request id and refusals (``ReadRequest``), MCP its caller, +exposure and reply (``ToolRequest``). Neither boundary restates the read, and the +reader's authority reaches the operation as a ``BusReader`` the boundary supplies, +never as an argument the caller may choose. +""" +from __future__ import annotations + +import asyncio +from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import TYPE_CHECKING, NoReturn + +from .launch_runtime import CallingThreadUnavailable +from .owner_connection import ( + OwnerConnection, OwnerReading, OwnerReplyConnection, ResultRuntimeRequest, RuntimeRequestContext, +) +from .presentation import AllOutcomes +from .tool_declaration import ToolDeclaration +from .tool_result import ToolResult + +if TYPE_CHECKING: + from .comms import Comms + from .presentation import OutcomeViewer + + +class BusReader(ABC): + """Whose authority a read runs under; each boundary supplies its own reader.""" + + @abstractmethod + def outcome_viewer(self, comms: Comms) -> OutcomeViewer: + """Whose recipient outcomes this reader may read.""" + + def subject(self, named: str) -> str: + return named or self.own_thread() + + def own_thread(self) -> str: + """The thread a read targets when its call names none; a reader with no thread has none.""" + return self.refuse_thread() + + def refuse_thread(self) -> NoReturn: + raise CallingThreadUnavailable("This tool requires a thread caller.") + + +class OperatorOutcomes: + """A reader speaking for the owner reads every recipient outcome, as the owner's UI does.""" + + __slots__ = () + + def outcome_viewer(self, comms: Comms) -> OutcomeViewer: + return AllOutcomes() + + +class OperatorReader(OperatorOutcomes, BusReader): + """The owner's UI: it observes the whole bus as the operator.""" + + __slots__ = () + + +class BusRead[Result](ToolDeclaration, ABC): + """A read of Core's stores, declared once with its arguments; boundaries derive it.""" + + def apply(self, comms: Comms, caller: BusReader) -> Result: + """The MCP boundary runs the read under its calling thread's authority.""" + return self.read_as(comms, caller) + + @abstractmethod + def read_as(self, comms: Comms, reader: BusReader) -> Result: + """The read under the reader's authority.""" + + +class UiRead[Result](BusRead[Result], ABC): + """The UI serves this read as the operator; its envelope (root, request id, refusal) + stays on ``ReadRequest``.""" + + def read(self, comms: Comms) -> Result: + return self.read_as(comms, OperatorReader()) + + +class OwnerLiveRead[Result: ToolResult = ToolResult](BusRead[Result], ABC): + """A read only the selected thread's live native owner can answer, from its own queue. + + MCP and the UI both reach the owner through ``OwnerReadRuntimeRequest``; neither answers + from its own process, and owner loss reaches the caller as the original refusal. The + owner runs ``owned`` against its own registration and session, never against the public + thread selector. + """ + + if TYPE_CHECKING: + # The composed ThreadReadTool declares the field; an annotation here would shadow it. + thread: str + + def read_as(self, comms: Comms, reader: BusReader) -> Result: + owner = comms.registry.require(reader.subject(self.thread)).incarnation + request = OwnerReadRuntimeRequest(thread=owner.name, operation=self) + return self.decode_reply(asyncio.run(OwnerReplyConnection.answer(comms, owner, request))) + + @abstractmethod + async def owned(self, ctx: RuntimeRequestContext) -> Result: + """The answer computed inside the owner, under its own context.""" + + @abstractmethod + def decode_reply(self, raw: object) -> Result: ... + + +@dataclass(frozen=True, kw_only=True) +class OwnerReadRuntimeRequest(OwnerReading[object], ResultRuntimeRequest): + """The one runtime envelope for every owner-live read: sent once to the held owner, and + answered there by the operation under the owner's context.""" + + operation: OwnerLiveRead + + async def read(self, owner: OwnerConnection) -> object: + return await owner.request(self.declared_name, operation=self.operation) + + async def result(self, ctx: RuntimeRequestContext) -> ToolResult: + return await self.operation.owned(ctx) diff --git a/src/agent_comms/commander_hud.py b/src/agent_comms/commander_hud.py index 2f9342db0d..235d926eec 100644 --- a/src/agent_comms/commander_hud.py +++ b/src/agent_comms/commander_hud.py @@ -147,7 +147,7 @@ class CommanderOrderDetail: @dataclass(frozen=True) -class CommanderUnitDetail: +class CommanderUnitDetail(ToolResult): """A lazy disclosure; activity uses the existing bounded turn wire projection.""" unit: Annotated[CommanderUnit, DisclosureIdentity()] diff --git a/src/agent_comms/connector.py b/src/agent_comms/connector.py index 0c19939c21..d7bef1ae34 100644 --- a/src/agent_comms/connector.py +++ b/src/agent_comms/connector.py @@ -54,6 +54,7 @@ from .declared_family import DeclaredFamily, UnknownFamilyMember from .errors import RelationViolationError, ToolRefusedError, UnregisteredThreadError from .coordination_errors import CoordinationError, Refused +from .work_branches import WorkBranchError from .field_codec import ( Argument, FieldCodec, GitCommitText, MalformedRecord, PathText, WireName, WireOmitDefault, RecordMember, boundary_decoder, projected, @@ -459,7 +460,10 @@ async def result(self, connector: Connector, caller: ConnectorCaller) -> ToolsCa try: reply = await connector.call(tool, self.params.arguments, caller) return ToolsCallResult((TextContent(json.dumps(reply)),), False, reply) - except (ToolRefusedError, RelationViolationError, CoordinationError) as error: + except ( + ToolRefusedError, RelationViolationError, CoordinationError, WorkBranchError, + MalformedRecord, Refused, ConnectionError, TimeoutError, + ) as error: logging.exception("Connector tool %s failed", self.params.name) return ToolsCallResult.failed(error) diff --git a/src/agent_comms/connector_caller.py b/src/agent_comms/connector_caller.py index 69b4caf582..43646991ba 100644 --- a/src/agent_comms/connector_caller.py +++ b/src/agent_comms/connector_caller.py @@ -13,7 +13,8 @@ from .head_review import HeadReviewOrder, PullRequestHead from .thread_identity import ThreadIncarnation from .todos import DirectiveText -from .tool_caller import CommanderForkCaller, NoThreadCaller, OperatorOutcomes, ThreadCaller, ToolCaller +from .bus_read import OperatorOutcomes +from .tool_caller import CommanderForkCaller, NoThreadCaller, ThreadCaller, ToolCaller from .tool_request import ToolArgumentsRefusedError, ToolRequest diff --git a/src/agent_comms/goal_history.py b/src/agent_comms/goal_history.py index 27e3590fde..48698d16bf 100644 --- a/src/agent_comms/goal_history.py +++ b/src/agent_comms/goal_history.py @@ -29,7 +29,7 @@ from .locked_store import DurableFile from .private_registry_guard import PrivateRegistryGuard from .store_files import AtomicFile -from .tool_result import DeclaredReply +from .tool_result import ToolResult from .reported_turn import ReportedTurnState from .typed_table import ( Column, @@ -170,7 +170,7 @@ def reconcile( @dataclass(frozen=True) -class RecordedGoalHistory(DeclaredReply): +class RecordedGoalHistory(ToolResult): """A thread's goal revisions beside the attempt ledger's failures, which only the ledger writes.""" history: tuple[RecordedGoalRevision, ...] diff --git a/src/agent_comms/goal_management.py b/src/agent_comms/goal_management.py index 554679d7c9..7fbf96d655 100644 --- a/src/agent_comms/goal_management.py +++ b/src/agent_comms/goal_management.py @@ -14,7 +14,7 @@ from dataclasses import replace from functools import cached_property from pathlib import Path -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING from agent_comms.field_codec import GoalId from .agent_activity import AgentActivity @@ -72,7 +72,6 @@ from .todos import SendBackVerdict, StrikeReason, Todo, JudgedCall, TodoBatch, TodoCommand, TodoStore _LOG = logging.getLogger(__name__) -HistoryReply = TypeVar("HistoryReply", bound=RecordedGoalHistory) class Goals(GoalWaitRecovery): @@ -294,12 +293,10 @@ def goal_history( """Read durable revisions for one thread incarnation within ``scope``.""" return self.registry.goal_history(name, scope) - def recorded_history( - self, reply: type[HistoryReply], name: str, scope: GoalScope - ) -> HistoryReply: + def recorded_history(self, name: str, scope: GoalScope) -> RecordedGoalHistory: """Revisions from goal history, execution failures from the attempt ledger.""" owner = self.registry.require(name).incarnation.wire_birth - return reply( + return RecordedGoalHistory( tuple(entry.recorded() for entry in self.goal_history(name, scope)), self.open_goal_store().failures(owner, scope), ) diff --git a/src/agent_comms/historical_views.py b/src/agent_comms/historical_views.py index 19e2e8a795..9e0842adca 100644 --- a/src/agent_comms/historical_views.py +++ b/src/agent_comms/historical_views.py @@ -7,7 +7,7 @@ import tempfile from abc import ABC, abstractmethod from collections import deque -from collections.abc import Callable, Iterator +from collections.abc import Callable, Iterator, Sequence from contextlib import contextmanager from dataclasses import dataclass, replace from functools import partial @@ -31,7 +31,9 @@ MessagePageRequest, PageBounds, ) +from .message_reference import MessageReference from .messages import CarriesEnvelope, Message +from .notification_evidence import NotificationWindow from .page_traversal import HistoryCursor, OlderTraversal from .presentation import MessageNotification, OutcomeViewer from .private_path import FileObservation, FileRevision, ObservedPath, PrivateDirectoryRole @@ -352,8 +354,9 @@ def require(cls, sources: tuple[HistorySource, ...], key: str) -> HistorySource: source.validate() return source - def notification_references(self, sources: tuple[HistorySource, ...], references, - viewer: OutcomeViewer): + def notification_references(self, sources: tuple[HistorySource, ...], + references: Sequence[MessageReference], + viewer: OutcomeViewer) -> NotificationWindow: """Read this namespace within its archive's current catalogue read, as this viewer may see it.""" diff --git a/src/agent_comms/notification_assignment.py b/src/agent_comms/notification_assignment.py index 69ee7d757c..20043faf2c 100644 --- a/src/agent_comms/notification_assignment.py +++ b/src/agent_comms/notification_assignment.py @@ -38,7 +38,7 @@ from .agent_activity import RecipientActivity from .bus_publication import Delivered from .coordination_database import InvalidStore, MissingStore - from .presentation import MessageNotification + from .presentation import MessageNotification, OutcomeViewer from .private_registry_guard import RegistryPrivacy from .wake_policy import RecipientDecision @@ -236,14 +236,20 @@ class NotificationAssignment(NotificationSource): @classmethod def changed(cls, reads: Iterable[NotificationAssignmentRead], root: Path) -> tuple[NotificationAssignmentRead, ...]: - """Acquire fresh bounded rows once; retained reads own their equality.""" - reads = tuple(reads) - sources = tuple({source.message.reference: source - for read in reads for source in read.sources}.values()) - fresh: dict[MessageReference, SourceEvidence] = {} - for sources_chunk in NotificationWindow.chunks(sources): - fresh.update(cls.read(root, sources_chunk).evidence) - return tuple(read for read in reads if read.differs_from(fresh)) + """Acquire fresh bounded rows once per captured viewer; retained reads own their + equality, each against evidence its own viewer may see.""" + by_viewer: dict[OutcomeViewer, list[NotificationAssignmentRead]] = {} + for read in reads: + by_viewer.setdefault(read.viewer, []).append(read) + changed: list[NotificationAssignmentRead] = [] + for viewer, viewed in by_viewer.items(): + sources = tuple({source.message.reference: source + for read in viewed for source in read.sources}.values()) + fresh: dict[MessageReference, SourceEvidence] = {} + for sources_chunk in NotificationWindow.chunks(sources): + fresh.update(cls.read(root, sources_chunk, viewer=viewer).evidence) + changed.extend(read for read in viewed if read.differs_from(fresh)) + return tuple(changed) @classmethod def resolve(cls, root: Path, read: NotificationAssignmentRead) -> tuple[NotificationAssignment, ...]: @@ -254,8 +260,10 @@ def resolve(cls, root: Path, read: NotificationAssignmentRead) -> tuple[Notifica @classmethod def for_deliveries( cls, originals: Iterable[Delivered], records: Iterable[NotificationAssignment], + viewer: OutcomeViewer, ) -> Iterator[tuple[Delivered, tuple[NotificationSource, ...]]]: - """Match notification assignments to messages and their recorded recipients.""" + """Match notification assignments to messages and the recorded recipients this + viewer may see; an invisible recipient allocates no placeholder.""" by_recipient = {} for record in records: key = (record.assignment.source, record.assignment.recipient_lookup) @@ -264,6 +272,8 @@ def for_deliveries( reference = original.message.reference sources = [] for recipient, decision in zip(original.recipients, original.decisions, strict=True): + if not viewer.includes(original.sender_lookup, recipient): + continue matching = by_recipient.get((reference, recipient.recipient_lookup), ()) if len(matching) > 1: raise ValueError("Original notification recipient has multiple handling rows") @@ -305,65 +315,77 @@ def _recorded_conversation_responses( @classmethod def capture( cls, connection: sqlite3.Connection, sources: Sequence[Delivered], - privacy: RegistryPrivacy, + privacy: RegistryPrivacy, viewer: OutcomeViewer, ) -> NotificationAssignmentRead: - """Capture detached assignment evidence for one bounded window of originals - from the caller's SQLite snapshot.""" + """Capture detached assignment evidence for one bounded window of originals from the + caller's SQLite snapshot: only the claims and attempts of recipients this viewer + may see are selected, by their exact original keys, before any row is decoded.""" sources = tuple(sources) if len(sources) > NotificationWindow.window_limit: raise ValueError("Notification reads require a bounded visible message window") if not sources: - return NotificationAssignmentRead((), ()) + return NotificationAssignmentRead((), (), viewer=viewer) privacy.coordinator_schema().require_schema(connection) + visible = tuple((source, recipient) for source in sources for recipient in source.recipients + if viewer.includes(source.sender_lookup, recipient)) columns = ",".join(f"w.{name}" for name in WakeAssignment.columns()) - # The window's assignments and their native-input membership are each - # selected once, membership by its assignment index, not once per row - # and per column. - rows = WakeAssignment.joined( - connection.execute( - f"WITH w AS MATERIALIZED (SELECT * FROM {WakeAssignment.declared_name} " - f"WHERE wire_seq IN ({','.join('?' for _ in sources)})), " - f"context AS MATERIALIZED ({privacy.notification_context_sql(connection)}) " - f"SELECT {columns},context.triage_inflight," - "pointer.execution_id IS NOT NULL AS queued,context.context_committed_at_ms," - "p.seq AS response_seq, p.message_id AS response_message_id, " - "p.received_at_ms AS response_received_at_ms " - "FROM w " - "JOIN context ON context.assignment_id=w.assignment_id " - f"LEFT JOIN {CurrentExecutions.declared_name} pointer " - "ON pointer.owner_lookup=w.recipient_lookup " - f"LEFT JOIN {PublicationEvidence.declared_name} p " - "ON p.execution_id=w.execution_id AND p.exact_target=w.exact_target " - "ORDER BY w.wire_seq DESC,w.recipient", - tuple(source.message.seq for source in sources), - ), - AssignmentActivityEvidence, - ) + context = privacy.notification_context_sql(connection) + rows = [] + # Each batch's requested keys select their assignments through the + # recipient/seq index; their native-input membership is selected once per + # batch by its assignment index, not once per row and per column. + for requested in WakeAssignment.requested_keys(connection, tuple( + {"recipient_lookup": recipient.recipient_lookup, + "wire_seq": source.message.seq, "message_id": source.message.message_id} + for source, recipient in visible)): + rows.extend(WakeAssignment.joined( + connection.execute( + f"WITH {requested.values}, " + f"w AS MATERIALIZED (SELECT * {requested.join}), " + f"context AS MATERIALIZED ({context}) " + f"SELECT {columns},context.triage_inflight," + "pointer.execution_id IS NOT NULL AS queued,context.context_committed_at_ms," + "p.seq AS response_seq, p.message_id AS response_message_id, " + "p.received_at_ms AS response_received_at_ms " + "FROM w " + "JOIN context ON context.assignment_id=w.assignment_id " + f"LEFT JOIN {CurrentExecutions.declared_name} pointer " + "ON pointer.owner_lookup=w.recipient_lookup " + f"LEFT JOIN {PublicationEvidence.declared_name} p " + "ON p.execution_id=w.execution_id AND p.exact_target=w.exact_target", + requested.bindings, + ), + AssignmentActivityEvidence, + )) attempts = HarnessDelivery.for_recipients(connection, tuple( - (recipient.recipient_lookup, source.message.reference) - for source in sources for recipient in source.recipients)) - return NotificationAssignmentRead(sources, tuple(rows), attempts) + (recipient.recipient_lookup, source.message.reference) for source, recipient in visible)) + return NotificationAssignmentRead( + sources, tuple(sorted(rows, key=lambda row: (-row[0].wire_seq, row[0].recipient))), + attempts, viewer=viewer) @classmethod def read( - cls, root: Path, sources: Sequence[Delivered] + cls, root: Path, sources: Sequence[Delivered], *, viewer: OutcomeViewer, ) -> NotificationAssignmentRead: - """Capture assignment evidence through the single observing connection owner.""" + """Capture this viewer's assignment evidence through the single observing + connection owner.""" privacy = RegistryStore.at(root).privacy() with CoordinationStore.observed(CoordinationStore.path_in(root)).observing( - cls, sources, privacy, + cls, sources, privacy, viewer, ) as result: return result @classmethod def missing_store( cls, store: MissingStore, sources: Sequence[Delivered], privacy: RegistryPrivacy, + viewer: OutcomeViewer, ) -> NotificationAssignmentRead: - return privacy.missing_notification_assignments(sources) + return privacy.missing_notification_assignments(sources, viewer) @classmethod def invalid_store( cls, store: InvalidStore, sources: Sequence[Delivered], privacy: RegistryPrivacy, + viewer: OutcomeViewer, ) -> NoReturn: raise ValueError(f"Channel notification status unavailable: {store.failure_reason}") diff --git a/src/agent_comms/notification_evidence.py b/src/agent_comms/notification_evidence.py index 9759d51f48..8caeb4051b 100644 --- a/src/agent_comms/notification_evidence.py +++ b/src/agent_comms/notification_evidence.py @@ -19,7 +19,7 @@ from .message_reference import MessageReference from .notification_assignment import AssignmentActivityEvidence from .notification_progress import DeliveryProgress - from .presentation import MessageNotification, WireRevision + from .presentation import MessageNotification, OutcomeViewer, WireRevision from .thread_identity import ThreadIncarnation @@ -41,7 +41,9 @@ def consumed_progress(self, recipient: ThreadLookup, @dataclass(frozen=True) class NotificationAssignmentRead: - """A window's claims and its harness attempts, both keyed by the exact original. + """A window's claims and its harness attempts, both keyed by the exact original, as the + viewer captured with them may see them: every later projection and change check of this + read answers for that same viewer. A harness attempt is its own consumption evidence: a no-wake or passive recipient has no claim for the attempt to move, and its message still entered the session.""" @@ -49,10 +51,7 @@ class NotificationAssignmentRead: sources: tuple[Delivered, ...] rows: tuple[tuple[WakeAssignment, AssignmentActivityEvidence], ...] attempts: tuple[HarnessDelivery, ...] = () - - @classmethod - def empty(cls) -> NotificationAssignmentRead: - return cls((), ()) + viewer: OutcomeViewer = field(kw_only=True) @cached_property def evidence(self) -> Mapping[MessageReference, SourceEvidence]: diff --git a/src/agent_comms/notification_read.py b/src/agent_comms/notification_read.py index 22467ba40f..9820c7b10a 100644 --- a/src/agent_comms/notification_read.py +++ b/src/agent_comms/notification_read.py @@ -1,17 +1,21 @@ -"""A captured notification window answers whether its own evidence changed.""" +"""A captured notification window, and where a notification read settled, answer whether +their own evidence changed.""" from __future__ import annotations -from dataclasses import dataclass +from collections.abc import Mapping +from dataclasses import dataclass, field from typing import TYPE_CHECKING from .notification_assignment import NotificationAssignment -from .notification_evidence import NotificationWindow -from .revision_views import NotificationEvidenceView +from .notification_evidence import NoNotificationWindow, NotificationWindow +from .revision_views import ActivityView, NotificationEvidenceView if TYPE_CHECKING: + from .activity import ObservedActivity from .comms import Comms from .presentation import WireRevision + from .registry_provenance import DeletedThread @dataclass(frozen=True) @@ -24,3 +28,38 @@ def changed(self, comms: Comms, revision: WireRevision, earlier: WireRevision) - self.read_marks, comms.registry.snapshot(), )) or NotificationAssignment.changed(self.reads, comms.root) ) + + +@dataclass(frozen=True) +class NotificationsRead: + """Where one notification read settled: its revision and the activity of the participants + its notifications name, captured before the read for participants already known and right + after it for new ones. The window names every watched participant; activity holds only + the ones the registry projects, so a human or archived participant is absent from it.""" + + revision: WireRevision + activity: Mapping[str, ObservedActivity | DeletedThread] = field(default_factory=dict) + notification_window: NotificationWindow = field(default_factory=NoNotificationWindow) + + @classmethod + def of(cls, revision: WireRevision, comms: Comms, + known: Mapping[str, ObservedActivity | DeletedThread], + window: NotificationWindow, + ) -> NotificationsRead: + names = window.activity_names() + known = {**comms.views.observed_activity(revision, names - known.keys()), **known} + return cls(revision, {name: known[name] for name in names if name in known}, window) + + def live_changed(self, comms: Comms, revision: WireRevision) -> bool: + """Read marks, participant activity or retained assignment evidence changed.""" + return ((revision.changed_since(ActivityView(), self.revision) + and comms.views.observed_activity( + revision, self.notification_window.activity_names()) != self.activity) + or self.notification_window.changed(comms, revision, self.revision)) + + +class UnreadableNotificationsRead(NotificationsRead): + """An unreadable read names no participants, so any later revision reads again.""" + + def live_changed(self, comms: Comms, revision: WireRevision) -> bool: + return revision != self.revision diff --git a/src/agent_comms/notification_reads.py b/src/agent_comms/notification_reads.py new file mode 100644 index 0000000000..a3b0986a31 --- /dev/null +++ b/src/agent_comms/notification_reads.py @@ -0,0 +1,62 @@ +"""Root-free notification window operations; UI transport and MCP paging stay at their boundaries. + +A notification read owns its references (and, when recorded, the archive it reads) and how +its settled evidence goes stale. The UI carries one in ``ReadNotifications`` with its own root +and request id; MCP's single-message page calls the live read directly under its caller. +""" +from __future__ import annotations + +from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from .bus_read import BusReader, UiRead +from .historical_views import HistorySource +from .message_reference import MessageReference +from .notification_evidence import NotificationWindow +from .notification_read import NotificationsRead + +if TYPE_CHECKING: + from .comms import Comms + from .presentation import WireRevision + + +class NotificationRead(UiRead[NotificationWindow], ABC): + """A batched notification window read; each member owns when its settled read is stale.""" + + @abstractmethod + def changed_since(self, comms: Comms, revision: WireRevision, + previous: NotificationsRead) -> bool: ... + + +@dataclass(frozen=True) +class LiveNotificationRead(NotificationRead): + """Recipient handling of the referenced original messages, as its reader may see them. + The UI reads as the operator; an MCP caller reads under its own viewer.""" + + references: tuple[MessageReference, ...] + + def read_as(self, comms: Comms, reader: BusReader) -> NotificationWindow: + return comms.views.message_notifications_for_references( + reader.outcome_viewer(comms), self.references) + + def changed_since(self, comms: Comms, revision: WireRevision, + previous: NotificationsRead) -> bool: + return previous.live_changed(comms, revision) + + +@dataclass(frozen=True) +class RecordedNotificationRead(LiveNotificationRead): + """The selected archive's recorded handling, independent of today's recipients.""" + + source: str + + def read_as(self, comms: Comms, reader: BusReader) -> NotificationWindow: + with comms.bus.history.read_sources() as sources: + source = HistorySource.require(sources, self.source) + return source.notification_references( + sources, self.references, reader.outcome_viewer(comms)) + + def changed_since(self, comms: Comms, revision: WireRevision, + previous: NotificationsRead) -> bool: + return revision.history != previous.revision.history diff --git a/src/agent_comms/order_tools.py b/src/agent_comms/order_tools.py index c30347268e..894acf23b5 100644 --- a/src/agent_comms/order_tools.py +++ b/src/agent_comms/order_tools.py @@ -5,7 +5,8 @@ from abc import abstractmethod from collections.abc import Mapping from dataclasses import dataclass, field, replace -from typing import Annotated, Literal +from functools import partial +from typing import TYPE_CHECKING, Annotated, Literal from pydantic import JsonValue @@ -33,9 +34,12 @@ JudgedCall, ToolCallVerdict, ) +from .acp_extension import GoalSnapshotReply, TranscriptSnapshotUpdate, TurnChangedUpdate, UpdateMetadata +from .bus_read import BusReader, OwnerLiveRead, UiRead from .commander.vibe_check import OverloadDegradation -from .commander_hud import CommanderHud +from .commander_hud import CommanderFrame, CommanderHud, CommanderUnitDetail from .comms import Comms +from .coordinator import Coordination from .errors import ToolRefusedError from .field_codec import FieldCodec, MessageId, OrderId, RecordProblems, WireName from .goal_actions import CreateProblemGoalAction, OrderGoalAction @@ -59,6 +63,7 @@ tool_field, ) from .tool_result import ToolResult +from .transcripts import TranscriptCursor, TranscriptRead from .transcript_events import SentTranscript from .turn_digests import ( ActivityTurnView, @@ -71,6 +76,9 @@ UnjudgedEdits, ) +if TYPE_CHECKING: + from .owner_connection import RuntimeRequestContext + @dataclass(frozen=True) class TodoChanged(ToolResult): @@ -508,13 +516,108 @@ def apply(self, comms: Comms, caller: ToolCaller) -> ThreadTurns: @dataclass(frozen=True, kw_only=True) -class CommsInputDeliveryTool(ThreadReadTool): +class InputDeliveryRead( + OwnerLiveRead[DeliveryOverview], ThreadReadTool, declared_name="comms_input_delivery", +): + """The one declaration of a unit's input delivery, answered by its live native owner.""" + label = "Input delivery" description = ("Read a unit's inputs that failed or are UNKNOWN, and its earlier delivery " "notices, each with its bus sequence and input id; none is ever replayed.") + include_history: Annotated[ + bool, *tool_field("Include earlier delivery notices; current inputs are always read") + ] = True + + async def owned(self, ctx: RuntimeRequestContext) -> DeliveryOverview: + scope = await ctx.server.agent.inputs.delivery_scope(ctx.session_id) + return await Coordination.run_worker(partial( + ctx.server.agent._comms.goals.input_delivery, + ctx.name, include_history=self.include_history, scope=scope, + )) + + def decode_reply(self, raw: object) -> DeliveryOverview: + return FieldCodec.decode(DeliveryOverview, raw) + + +@dataclass(frozen=True, kw_only=True) +class OwnerGoalRead(OwnerLiveRead[GoalSnapshotReply], ThreadReadTool, declared_name="comms_owner_goal"): + """A unit's goal snapshot and the turn it was read with, answered by its live native owner + under the owner's own registration and session; the thread selector only finds the owner.""" + + label = "Owner goal" + description = ("Read a unit's current goal snapshot and the turn state its live native " + "owner read with it.") + + async def owned(self, ctx: RuntimeRequestContext) -> GoalSnapshotReply: + snapshot = await Coordination.run_worker(partial( + ctx.server.agent._comms.goals.goal_snapshot, ctx.name, + )) + turn = await Coordination.run_worker(partial( + ctx.server.agent.turns.turn_state, ctx.session_id, + )) + return GoalSnapshotReply.from_snapshot(snapshot, UpdateMetadata.of(TurnChangedUpdate(turn))) - def apply(self, comms: Comms, caller: ToolCaller) -> DeliveryOverview: - return comms.goals.input_delivery(caller.subject(self.thread), include_history=True) + def decode_reply(self, raw: object) -> GoalSnapshotReply: + return FieldCodec.decode(GoalSnapshotReply, raw) + + +@dataclass(frozen=True, kw_only=True) +class TranscriptPageRead( + UiRead[TranscriptSnapshotUpdate], ThreadReadTool, declared_name="comms_transcript_page", +): + """One bounded transcript page with its current draft and the source witness that read it. + The cursors and historical source select the page; they never choose whose authority the + read runs under.""" + + label = "Transcript page" + description = ("Read one bounded page of a thread's transcript with its current draft. " + "Send a returned cursor as before or after for the next page; " + "historical_source reads an archived source.") + before: Annotated[TranscriptCursor | None, *tool_field("Read the page before this cursor")] = None + after: Annotated[TranscriptCursor | None, *tool_field("Read the page after this cursor")] = None + through: Annotated[TranscriptCursor | None, *tool_field("Read through this cursor")] = None + historical_source: Annotated[ + str | None, *tool_field("Archived source to read instead of current history") + ] = None + + def capture_as(self, comms: Comms, reader: BusReader) -> TranscriptRead: + return comms.transcripts.capture_page_read( + reader.subject(self.thread), + before=self.before, + after=self.after, + through=self.through, + historical_source=self.historical_source, + ) + + def read_as(self, comms: Comms, reader: BusReader) -> TranscriptSnapshotUpdate: + return TranscriptSnapshotUpdate.from_read(self.capture_as(comms, reader)) + + +@dataclass(frozen=True, kw_only=True) +class CommanderUnitRead( + UiRead[CommanderUnitDetail], ReadTool, ToolRequest, declared_name="comms_commander_unit", +): + """One unit's card on a commander's published frame, expanded with its orders' bounded + instructions, stories, commits and activity. The held incarnations and orders select the + exact card and witness it; they are never whose authority the read runs under.""" + + label = "Commander unit" + description = ("Expand one unit's card on a commander's published frame: its orders' " + "bounded instructions, stories and commits, and its recent activity. " + "Refuses when either incarnation or the unit's assignment changed.") + commander: Annotated[ThreadIncarnation, *tool_field("Commander incarnation whose frame shows the card")] + unit: Annotated[ThreadIncarnation, *tool_field("Unit incarnation on that card")] + orders: Annotated[tuple[OrderId, ...], *tool_field("The card's orders, exactly as published")] + + def read_as(self, comms: Comms, reader: BusReader) -> CommanderUnitDetail: + snapshot = comms.registry.snapshot() + if not self.commander.current(snapshot) or not self.unit.current(snapshot): + raise ToolRefusedError("The selected commander or unit incarnation is no longer current") + hud = CommanderFrame.current(comms, self.commander.name) + for card in hud.units: + if card.unit == self.unit: + return card.detail(comms, self.commander, self.orders) + raise ToolRefusedError("The selected unit is no longer in this commander's published frame") @dataclass(frozen=True, kw_only=True) diff --git a/src/agent_comms/owner_connection.py b/src/agent_comms/owner_connection.py index 205b51650f..54f2d386f7 100644 --- a/src/agent_comms/owner_connection.py +++ b/src/agent_comms/owner_connection.py @@ -8,8 +8,8 @@ import socket from functools import partial from pathlib import Path -from typing import TYPE_CHECKING, Annotated, ClassVar, cast -from abc import abstractmethod +from typing import TYPE_CHECKING, Annotated, ClassVar, Self, cast +from abc import ABC, abstractmethod from collections.abc import Awaitable from dataclasses import dataclass, replace from .command import Command @@ -28,7 +28,7 @@ from .comms import Comms from .child_process import ProcessIdentity from .registry_document import RegistryEntry - from .thread_identity import OwnerIdentity + from .thread_identity import OwnerIdentity, ThreadIncarnation from .runtime import RuntimeServer, SocketClient from .threads import Thread @@ -150,6 +150,11 @@ def __init__(self, comms: Comms, session_id: str, path: Path): self._identity = comms.registry.entry(session_id).entry.thread.incarnation self._root_identity = comms.root.stat() + def require_identity(self, owner: ThreadIncarnation) -> None: + """The incarnation this connection captured is the one the caller holds.""" + if self._identity != owner: + raise OwnerIdentityChangedError("The owner incarnation changed while preparing the request.") + def _owner_entry(self) -> RegistryEntry: if self._closed: raise ConnectionError("Owner attachment was closed before request dispatch") @@ -251,3 +256,51 @@ async def close(self) -> None: task.cancel() await asyncio.gather(*tasks, return_exceptions=True) + +class OwnerReading[Result](ABC): + """One exchange with exactly one live native owner incarnation over its socket.""" + + def require_native_owner(self, owner: Thread) -> None: + """Only a native owner serves a runtime socket; an external bridge with a live PID + has none, so it is refused here instead of waited on before dispatch.""" + owner.execution.require_native() + + @abstractmethod + async def read(self, owner: OwnerConnection) -> Result: ... + + +class OwnerReplyConnection(OwnerConnection): + """Bound only the live owner's response after a request has been dispatched.""" + + REPLY_TIMEOUT: ClassVar[float] = 3 + + @classmethod + def held(cls, comms: Comms, owner: ThreadIncarnation, reading: OwnerReading) -> Self: + """A connection to exactly this registered native owner incarnation. + + The selected registration must be the held incarnation before its process is + asked for; the connection's own capture is then checked again, since the + registry may change between the two reads. + """ + thread = comms.registry.require(owner.name) + if thread.incarnation != owner: + raise OwnerIdentityChangedError("The owner incarnation changed while preparing the request.") + reading.require_native_owner(thread) + connection = cls(comms, thread.name, socket_path(comms.root, thread.require_process().pid)) + connection.require_identity(owner) + return connection + + @classmethod + async def answer[Result]( + cls, comms: Comms, owner: ThreadIncarnation, reading: OwnerReading[Result], + ) -> Result: + """One reading from the held owner; the connection closes whatever the outcome.""" + connection = await asyncio.to_thread(cls.held, comms, owner, reading) + try: + return await reading.read(connection) + finally: + await connection.close() + + async def _read_response(self, reader: asyncio.StreamReader) -> bytes: + async with asyncio.timeout(self.REPLY_TIMEOUT): + return await super()._read_response(reader) diff --git a/src/agent_comms/presentation.py b/src/agent_comms/presentation.py index 3a21c7715a..c34358c34b 100644 --- a/src/agent_comms/presentation.py +++ b/src/agent_comms/presentation.py @@ -9,7 +9,7 @@ from dataclasses import dataclass, field, replace from datetime import datetime from pathlib import Path -from typing import TYPE_CHECKING, Annotated, ClassVar +from typing import TYPE_CHECKING, Annotated, ClassVar, Self from . import relationships from .activity import Activity, ActivityData, ActivityState, ObservedActivity @@ -251,8 +251,8 @@ def references( """Borrow mounted references in the original bounded delivery windows, as this viewer may see them.""" def project(sources: Sequence[Delivered]) -> NotificationWindow: - read = NotificationAssignment.read(root, sources) - return cls.delivery_window(root, registry, sources, read, viewer=viewer) + read = NotificationAssignment.read(root, sources, viewer=viewer) + return cls.delivery_window(root, registry, sources, read) return cls.read_references(log, references, project) @@ -276,20 +276,21 @@ def read_references( @classmethod def delivery_window( cls, root: Path, registry: Registration, sources: Sequence[Delivered], - assignment_read: NotificationAssignmentRead, *, viewer: OutcomeViewer, + assignment_read: NotificationAssignmentRead, ) -> NotificationWindow: - """Project the original frozen audience even before handling is recorded.""" + """Project the original frozen audience even before handling is recorded, as the + read's captured viewer may see it.""" if not sources: return CapturedNotificationWindow({}, (assignment_read,)) snapshot = registry.snapshot() + viewer = assignment_read.viewer observations = AgentActivity(root, registry).observe_recipients( ((source, recipient) for source in sources for recipient in source.recipients if viewer.includes(source.sender_lookup, recipient)), snapshot=snapshot, ) return cls.project_delivery_window( - root, snapshot, sources, observations, - viewer=viewer, assignment_read=assignment_read, + root, snapshot, sources, observations, assignment_read=assignment_read, ) @classmethod @@ -299,37 +300,37 @@ def recorded_delivery_window(cls, root: Path, namespace: RegistryProvenance, observations = { recipient.recipient_lookup: RecordedRecipientActivity() for source in sources for recipient in source.recipients + if viewer.includes(source.sender_lookup, recipient) } - read = NotificationAssignment.read(root, sources) + read = NotificationAssignment.read(root, sources, viewer=viewer) return cls.project_delivery_window( - root, namespace, sources, observations, - assignment_read=read, viewer=viewer, + root, namespace, sources, observations, assignment_read=read, ) @classmethod def project_delivery_window( cls, root: Path, snapshot: RegistryProvenance, sources: Sequence[Delivered], observations: Mapping[ThreadLookup, RecipientActivity], - *, assignment_read: NotificationAssignmentRead, viewer: OutcomeViewer, + *, assignment_read: NotificationAssignmentRead, ) -> NotificationWindow: - """Original assignment and read authorities own both display projections.""" + """Original assignment and read authorities own both display projections; the read's + captured viewer decides which recipients are projected at all.""" if len(sources) > NotificationWindow.window_limit: raise ValueError("Notification reads require a bounded visible message window") keys = {(source.message.seq, source.message.message_id) for source in sources} result: dict[tuple[int, str], list[MessageNotification]] = {key: [] for key in keys} if not keys: return CapturedNotificationWindow({}, (assignment_read,)) + viewer = assignment_read.viewer rows = NotificationAssignment.resolve(root, assignment_read) evidence = assignment_read.evidence read_marks = [] reads = ReadLedger.at(root) document = reads.read() - for source, outcomes in NotificationAssignment.for_deliveries(sources, rows): + for source, outcomes in NotificationAssignment.for_deliveries(sources, rows, viewer): key = (source.message.seq, source.message.message_id) for outcome in outcomes: recipient = outcome.recipient() - if not viewer.includes(source.sender_lookup, recipient): - continue observation = observations[recipient.recipient_lookup] notification = outcome.project(observation, source.sender_lookup) state = evidence[source.message.reference].consumed_progress( @@ -391,16 +392,14 @@ def sent_sources( @classmethod def for_sources( - cls, root: Path, registry: Registration, viewer: OutcomeViewer, - sources: Sequence[Delivered], *, + cls, root: Path, registry: Registration, sources: Sequence[Delivered], *, assignment_read: NotificationAssignmentRead, ) -> tuple[MessageNotification, ...]: - """Attach notification outcomes to the messages the caller already checked.""" - projected = cls.delivery_window( - root, registry, sources, assignment_read, viewer=viewer, - ) + """Attach notification outcomes, as the read's viewer sees them, to the public + envelopes of the messages the caller already checked.""" + projected = cls.delivery_window(root, registry, sources, assignment_read) return tuple( - replace(notification, message=source.message) + replace(notification, message=source.envelope) for source in sources for notification in projected.outcomes[source.message.seq, source.message.message_id] ) @@ -460,14 +459,13 @@ class SentMessageSummary: summary: str @classmethod - def recent(cls, presentation: ThreadPresentation) -> tuple[SentMessageSummary, ...]: - """One summary per recent sent source, from the presentation's captured window.""" - window = presentation.notification_window - return tuple( - cls(reference, MessageNotification.summary( - window.outcomes.get((reference.seq, reference.message_id), ()))) - for reference in presentation.sent_references - ) + def for_sources( + cls, + sources: Iterable[tuple[MessageReference, Iterable[MessageNotification]]], + ) -> tuple[Self, ...]: + """One summary per sent source, aggregated by MessageNotification.summary.""" + return tuple(cls(reference, MessageNotification.summary(outcomes)) + for reference, outcomes in sources) class ChannelReadMark(ABC): diff --git a/src/agent_comms/private_registry_guard.py b/src/agent_comms/private_registry_guard.py index 3f4baeafdf..fcb3d96110 100644 --- a/src/agent_comms/private_registry_guard.py +++ b/src/agent_comms/private_registry_guard.py @@ -53,6 +53,7 @@ from .acp_extension import CursorAdvancedUpdate, CursorScope from .bus_publication import Delivered from .cursor_publication import CursorPublication + from .presentation import OutcomeViewer from .private_nk_entrypoint import RootLaunch from .registry_document import RegistrySnapshot from .typed_table import TableSchema @@ -162,7 +163,7 @@ def context_entered_sql(self, assignment: str) -> str: @abstractmethod def missing_notification_assignments( - self, sources: Sequence[Delivered], + self, sources: Sequence[Delivered], viewer: OutcomeViewer, ) -> NotificationAssignmentRead: ... @abstractmethod @@ -202,9 +203,9 @@ def context_entered_sql(self, assignment: str) -> str: return "0" def missing_notification_assignments( - self, sources: Sequence[Delivered], + self, sources: Sequence[Delivered], viewer: OutcomeViewer, ) -> NotificationAssignmentRead: - return NotificationAssignmentRead(tuple(sources), ()) + return NotificationAssignmentRead(tuple(sources), (), viewer=viewer) def cursor_scope( self, snapshot: RegistrySnapshot, thread_name: str, session_id: str, @@ -283,7 +284,7 @@ def context_entered_sql(self, assignment: str) -> str: return f"EXISTS (SELECT 1 {NativeRuntimeInput.context_entered_from(members, assignment)})" def missing_notification_assignments( - self, sources: Sequence[Delivered], + self, sources: Sequence[Delivered], viewer: OutcomeViewer, ) -> NotificationAssignmentRead: raise PublicationActivationBlocked("Private notification coordinator is missing") diff --git a/src/agent_comms/runtime.py b/src/agent_comms/runtime.py index 89dfdde624..f42d7143e4 100644 --- a/src/agent_comms/runtime.py +++ b/src/agent_comms/runtime.py @@ -25,7 +25,7 @@ from contextvars import ContextVar from dataclasses import dataclass from pathlib import Path -from typing import TYPE_CHECKING, Any, ClassVar, Self +from typing import TYPE_CHECKING, Any, Self from pydantic import BaseModel @@ -423,16 +423,6 @@ def request_payload(self, action: str, parameters: dict[str, object]) -> dict[st ) -class OwnerReplyConnection(RuntimeConnection): - """Bound only the live owner's response after a request has been dispatched.""" - - REPLY_TIMEOUT: ClassVar[float] = 3 - - async def _read_response(self, reader: asyncio.StreamReader) -> bytes: - async with asyncio.timeout(self.REPLY_TIMEOUT): - return await super()._read_response(reader) - - @dataclass(frozen=True) class OwnerAttachment: """A completed subscription: its stream, the owner's ready line and presented metadata.""" diff --git a/src/agent_comms/runtime_requests.py b/src/agent_comms/runtime_requests.py index e27e56237e..20565f1eb2 100644 --- a/src/agent_comms/runtime_requests.py +++ b/src/agent_comms/runtime_requests.py @@ -23,8 +23,6 @@ from .acp_extension import ( GoalSnapshotReply, - TurnChangedUpdate, - UpdateMetadata, ) from .agent_event_updates import AcpEventConsumer @@ -41,6 +39,7 @@ from .goal_scope import AllGoals, OneGoal from .goals import Goal from .manual_compaction_bridge import compact_context +from .order_tools import OwnerGoalRead from .owner_connection import NoControllerToken, OwnerConnection, ResultRuntimeRequest, RuntimeRequest, RuntimeRequestContext from .owner_protocol import ClientLine, OwnerAnnouncement, OwnerLine, OwnerReady, OwnerResult from .thread_identity import ThreadIncarnation @@ -82,11 +81,6 @@ class GoalReply(DeclaredReply): goal: Goal -@dataclass(frozen=True) -class GoalHistoryReply(RecordedGoalHistory): - pass - - @dataclass(frozen=True) class AcpResponseReply(DeclaredReply, WireValue): """An ACP schema response, sent exactly as the schema writes it.""" @@ -271,19 +265,6 @@ async def result(self, ctx: RuntimeRequestContext) -> CompactionOutcome: return await compact_context(ctx.server.agent.turns, ctx.session_id, self.instructions) -@dataclass(frozen=True, kw_only=True) -class InputDispositionsRuntimeRequest(ResultRuntimeRequest): - include_history: bool = False - - async def result(self, ctx: RuntimeRequestContext) -> DeliveryOverview: - return await Coordination.run_worker(partial(ctx.server.agent._comms.goals.input_delivery, - ctx.name, - include_history=self.include_history, - scope=await ctx.server.agent.inputs.delivery_scope(ctx.session_id), - - )) - - @dataclass(frozen=True, kw_only=True) class DismissHistoricalInputsRuntimeRequest(ResultRuntimeRequest): async def result(self, ctx: RuntimeRequestContext) -> DeliveryOverview: @@ -298,10 +279,9 @@ async def result(self, ctx: RuntimeRequestContext) -> DeliveryOverview: class GoalHistoryRuntimeRequest(ResultRuntimeRequest): goal_id: OneGoal | AllGoals = AllGoals() - async def result(self, ctx: RuntimeRequestContext) -> GoalHistoryReply: + async def result(self, ctx: RuntimeRequestContext) -> RecordedGoalHistory: return await Coordination.run_worker(partial( - ctx.server.agent._comms.goals.recorded_history, - GoalHistoryReply, ctx.name, self.goal_id, + ctx.server.agent._comms.goals.recorded_history, ctx.name, self.goal_id, )) @@ -311,19 +291,7 @@ async def change(self, ctx: RuntimeRequestContext) -> None: ... async def result(self, ctx: RuntimeRequestContext) -> GoalSnapshotReply: await self.change(ctx) - snapshot = await Coordination.run_worker(partial( - ctx.server.agent._comms.goals.goal_snapshot, ctx.name, - )) - turn = await Coordination.run_worker(partial( - ctx.server.agent.turns.turn_state, ctx.session_id, - )) - return GoalSnapshotReply.from_snapshot(snapshot, UpdateMetadata.of(TurnChangedUpdate(turn))) - - -@dataclass(frozen=True, kw_only=True) -class GoalSnapshotRuntimeRequest(GoalSnapshotResultRuntimeRequest): - async def change(self, ctx: RuntimeRequestContext) -> None: - pass + return await OwnerGoalRead().owned(ctx) @dataclass(frozen=True, kw_only=True) diff --git a/src/agent_comms/selected_participant.py b/src/agent_comms/selected_participant.py index cdf17be4a9..f2985f37eb 100644 --- a/src/agent_comms/selected_participant.py +++ b/src/agent_comms/selected_participant.py @@ -41,7 +41,6 @@ from .presentation import MessageNotification, OwnerOutcomes, SentMessageOutcome from .publisher import ParticipantPublisher from .selected_source_batch import SelectedSource, SelectedSourceBatch -from .threads import Thread from .turn_phase import PreparingPhase, TurnPhase from .turn_goal_permission import OwnerGoalPermission, TurnGoalPermission @@ -203,13 +202,12 @@ async def select( @staticmethod def capture_sent_outcomes( - comms: Comms, owner: Thread, sources: Sequence[Delivered], + comms: Comms, sources: Sequence[Delivered], assignment_read: NotificationAssignmentRead, ) -> tuple[SentMessageOutcome, ...]: """Project optional prompt context from the drain's captured source and SQL cuts.""" notifications = MessageNotification.for_sources( - comms.root, comms.registry, OwnerOutcomes.of(owner), sources, - assignment_read=assignment_read, + comms.root, comms.registry, sources, assignment_read=assignment_read, ) return tuple( outcome @@ -253,13 +251,11 @@ def prepare( store, lookup, after_seq=after_seq ) sent_read = NotificationAssignment.capture( - store.session._connection, sent_sources, privacy, + store.session._connection, sent_sources, privacy, OwnerOutcomes.of(sent_owner), ) if pending: participant.pointer.require_idle() - sent_outcomes = SelectedParticipant.capture_sent_outcomes( - comms, sent_owner, sent_sources, sent_read - ) + sent_outcomes = SelectedParticipant.capture_sent_outcomes(comms, sent_sources, sent_read) return owner, participant, pending, sent_outcomes @staticmethod diff --git a/src/agent_comms/thread_presentation.py b/src/agent_comms/thread_presentation.py index 975703cae8..37772366ec 100644 --- a/src/agent_comms/thread_presentation.py +++ b/src/agent_comms/thread_presentation.py @@ -4,7 +4,7 @@ from collections.abc import Callable from dataclasses import dataclass, field, fields, replace -from typing import TYPE_CHECKING, ClassVar, Self +from typing import TYPE_CHECKING, ClassVar, NoReturn, Self from agent_comms.coordination_errors import StaleRevision from agent_comms.errors import RelationViolationError, RouteChangedError, UnregisteredThreadError @@ -12,7 +12,7 @@ from .declared_family import DeclaredFamily from .thread_identity import OwnerIdentity from .child_process import ProcessIdentity -from .notification_evidence import NoNotificationWindow, NotificationWindow +from .notification_evidence import NotificationWindow if TYPE_CHECKING: from .message_reference import MessageReference @@ -209,13 +209,20 @@ def same_status(self, other: ThreadStatusDisplay) -> bool: @dataclass(frozen=True, slots=True) class ThreadPresentation(ThreadStatusDisplay): - """A thread's acquired presentation, or its declared absence.""" + """What a viewer shows for a thread, or its declared absence: no acquired evidence.""" notifications: tuple[MessageNotification, ...] = () read_identity: TranscriptReadIdentity | None = None - incoming_references: tuple[MessageReference, ...] = () - sent_references: tuple[MessageReference, ...] = () - notification_window: NotificationWindow = field(default_factory=NoNotificationWindow) + + def published(self) -> ThreadPresentation: + """The value a viewer may receive: this presentation itself.""" + return self + + def sent_notifications( + self, + ) -> tuple[tuple[MessageReference, tuple[MessageNotification, ...]], ...]: + """A presentation without acquired sources holds no sent message.""" + return () def require(self, name: str) -> ThreadPresentation: """Require a presentation for a caller already bound to this thread.""" @@ -230,8 +237,9 @@ def with_paint(self, shown: ThreadPresentation) -> Self: return replace(self, title=shown.title, marker=shown.marker, summary=shown.summary, status=shown.status) - def collect_revision_input(self, name: str, readable: dict[str, ThreadPresentation]) -> None: - readable[name] = self + def collect_revision_input(self, name: str, + readable: dict[str, AcquiredThreadPresentation]) -> None: + """Only acquired evidence can say whether a later revision changed it.""" def read_sources(self, transcripts: Transcripts, snapshot: RegistrySnapshot, name: str, sent_source_limit: int) -> ThreadPresentation: @@ -257,36 +265,71 @@ def same_observation(self, other: ThreadPresentation) -> bool: return original.thread == current.thread and original.same_snapshot(current) def with_sources(self, window: TranscriptSourceWindow) -> ThreadPresentation: - """Incoming and sent outcomes are views of one captured notification window.""" + """Incoming and sent outcomes are views of one captured notification window, read + as the thread's owner may see them; the incoming outcomes carry public envelopes.""" from .notification_assignment import NotificationAssignment from .presentation import MessageNotification, OwnerOutcomes transcripts = window.read.owner sources = tuple({source.message.reference: source for source in (*window.sources, *window.sent_sources)}.values()) - notification_read = NotificationAssignment.read(transcripts.root, sources) + notification_read = NotificationAssignment.read( + transcripts.root, sources, viewer=OwnerOutcomes.of(window.read.identity.thread)) projected = MessageNotification.delivery_window( - transcripts.root, transcripts.registry, sources, notification_read, - viewer=OwnerOutcomes.of(window.read.identity.thread), - ) + transcripts.root, transcripts.registry, sources, notification_read) incoming = tuple( - replace(notification, message=source.message) + replace(notification, message=source.envelope) for source in window.sources for notification in projected.outcomes[source.message.seq, source.message.message_id] ) - return replace( - self, notifications=incoming, + shown = replace(self, notifications=incoming, read_identity=window.read.identity) + return AcquiredThreadPresentation( + **{declared.name: getattr(shown, declared.name) + for declared in fields(ThreadPresentation)}, notification_window=projected, - read_identity=window.read.identity, incoming_references=tuple(source.message.reference for source in window.sources), - sent_references=tuple(source.message.reference for source in window.sent_sources), - ) + sent_references=tuple(source.message.reference for source in window.sent_sources)) @property def label(self) -> str: return f"{self.marker} {self.title}".strip() +@dataclass(frozen=True, slots=True) +class AcquiredThreadPresentation(ThreadPresentation): + """A presentation together with the notification evidence it was acquired from. + + The evidence stays with the reader that acquired it, for its change checks and sent + summaries; a viewer receives only the published ThreadPresentation.""" + + incoming_references: tuple[MessageReference, ...] = field(kw_only=True) + sent_references: tuple[MessageReference, ...] = field(kw_only=True) + notification_window: NotificationWindow = field(kw_only=True) + + def published(self) -> ThreadPresentation: + return ThreadPresentation(**{declared.name: getattr(self, declared.name) + for declared in fields(ThreadPresentation)}) + + def sent_notifications( + self, + ) -> tuple[tuple[MessageReference, tuple[MessageNotification, ...]], ...]: + """Each recent sent source with its outcomes from this presentation's captured window.""" + return tuple( + (reference, + self.notification_window.outcomes.get((reference.seq, reference.message_id), ())) + for reference in self.sent_references + ) + + def collect_revision_input(self, name: str, + readable: dict[str, AcquiredThreadPresentation]) -> None: + readable[name] = self + + def __reduce_ex__(self, protocol: object) -> NoReturn: + raise TypeError( + "AcquiredThreadPresentation keeps private delivery evidence with its reader; " + "send its published() ThreadPresentation") + + @dataclass(frozen=True, slots=True) class NoThreadPresentation(ThreadPresentation): """No acquired thread: no sources, notifications or readable revision.""" @@ -301,9 +344,6 @@ def require(self, name: str) -> ThreadPresentation: def for_status(self, name: str) -> ThreadPresentation: return self.with_paint(ThreadRetired().presentation(name)) - def collect_revision_input(self, name: str, readable: dict[str, ThreadPresentation]) -> None: - pass - def read_sources(self, transcripts: Transcripts, snapshot: RegistrySnapshot, name: str, sent_source_limit: int) -> ThreadPresentation: return self diff --git a/src/agent_comms/tool_caller.py b/src/agent_comms/tool_caller.py index 451c08fa7d..b35af559de 100644 --- a/src/agent_comms/tool_caller.py +++ b/src/agent_comms/tool_caller.py @@ -1,17 +1,17 @@ """The caller bound at tool entry owns caller-relative tool behavior.""" from __future__ import annotations -from abc import ABC, abstractmethod +from abc import abstractmethod from dataclasses import dataclass -from typing import TYPE_CHECKING, Annotated, NoReturn +from typing import TYPE_CHECKING, Annotated from .bus_publication import AgentOrigin from .cli_result import ThreadToolCatalogResult, ToolCatalogResult from .errors import ToolRefusedError from .field_codec import WireForm from .launch_root import LaunchRoot -from .launch_runtime import CallingThreadUnavailable -from .presentation import AllOutcomes, OutcomeViewer, OwnerOutcomes, SentMessageSummary, ThreadRecord +from .bus_read import BusReader, OperatorOutcomes +from .presentation import OutcomeViewer, OwnerOutcomes, SentMessageSummary, ThreadRecord from .thread_execution import CommanderForkExecution from .thread_identity import ThreadIncarnation from .tool_result import ToolResult @@ -31,7 +31,7 @@ class ThreadListing(ToolResult): sent_messages: tuple[SentMessageSummary, ...] -class ToolCaller(ABC): +class ToolCaller(BusReader): def acting_caller(self, grant: ToolCaller) -> ToolCaller: return self @@ -49,10 +49,6 @@ def listing(self, comms: Comms, threads: tuple[ThreadRecord, ...]) -> ThreadList @abstractmethod def catalog(self, comms: Comms, tools: tuple[dict[str, object], ...]) -> ToolCatalogResult: ... - @abstractmethod - def outcome_viewer(self, comms: Comms) -> OutcomeViewer: - """Whose recipient outcomes this caller may read.""" - @abstractmethod def acting_name(self, default: str) -> str: ... @@ -60,13 +56,6 @@ def acting_name(self, default: str) -> str: ... def from_flag(self) -> tuple[str, ...]: """The command line's --from argument naming this caller, if it names a thread.""" - @abstractmethod - def own_thread(self) -> str: - """The thread a tool acts on when its call names none.""" - - def subject(self, named: str) -> str: - return named or self.own_thread() - def commanding(self, comms: Comms, commander: str) -> None: """An ordinary caller needs no commander-child registration.""" return None @@ -95,15 +84,6 @@ def origin(self) -> AgentOrigin: return AgentOrigin() -class OperatorOutcomes: - """A caller speaking for the owner reads every recipient outcome, as the owner's UI does.""" - - __slots__ = () - - def outcome_viewer(self, comms: Comms) -> OutcomeViewer: - return AllOutcomes() - - @dataclass(frozen=True) class NoThreadCaller(OperatorOutcomes, ToolCaller): def acting_caller(self, grant: ToolCaller) -> ToolCaller: @@ -121,18 +101,12 @@ def listing(self, comms: Comms, threads: tuple[ThreadRecord, ...]) -> ThreadList def catalog(self, comms: Comms, tools: tuple[dict[str, object], ...]) -> ToolCatalogResult: return ToolCatalogResult(tools) - def refuse_thread(self) -> NoReturn: - raise CallingThreadUnavailable("This tool requires a thread caller.") - def acting_name(self, default: str) -> str: return default def from_flag(self) -> tuple[str, ...]: return () - def own_thread(self) -> NoReturn: - return self.refuse_thread() - def require_directive(self, comms: Comms, body: str) -> None: """A caller with no thread gives no directive.""" @@ -161,7 +135,7 @@ def invoke(self, request: ToolRequest[ThreadCaller], comms: Comms) -> ToolResult def listing(self, comms: Comms, threads: tuple[ThreadRecord, ...]) -> ThreadListing: presentation = comms.views.thread_presentations((self.name,)).require(self.name) - return ThreadListing(threads, SentMessageSummary.recent(presentation)) + return ThreadListing(threads, SentMessageSummary.for_sources(presentation.sent_notifications())) def outcome_viewer(self, comms: Comms) -> OutcomeViewer: return OwnerOutcomes.of(comms.registry.require(self.name)) diff --git a/src/agent_comms/tools.py b/src/agent_comms/tools.py index 9e27aac205..6613b5e5df 100644 --- a/src/agent_comms/tools.py +++ b/src/agent_comms/tools.py @@ -87,8 +87,8 @@ from .thread_model import ThreadModel from .threads import NextTurnConfiguration from .tool_caller import ThreadCaller, ThreadListing, ToolCaller -from .ui_model.observation import ReadNotifications from .tool_result import ToolResult +from .notification_reads import LiveNotificationRead from .work_branches import WorkBranch, WorkBranches if TYPE_CHECKING: @@ -163,21 +163,6 @@ class GoalUpdated(OwnGoalPublication): goal: GoalDeclaration -@dataclass(frozen=True) -class GoalHistory(ToolResult, RecordedGoalHistory): - pass - - - - - - - - - - - - @dataclass(frozen=True) class WorkBranchList(ToolResult): branches: tuple[WorkBranch, ...] @@ -622,8 +607,8 @@ class CommsGoalHistoryTool(ThreadReadTool): OneGoal | AllGoals, *tool_field("Optional goal ID; omit for this thread's full history") ] = AllGoals() - def apply(self, comms: Comms, caller: ToolCaller) -> GoalHistory: - return comms.goals.recorded_history(GoalHistory, caller.subject(self.thread), self.goal_id) + def apply(self, comms: Comms, caller: ToolCaller) -> RecordedGoalHistory: + return comms.goals.recorded_history(caller.subject(self.thread), self.goal_id) @@ -716,10 +701,9 @@ def apply(self, comms: Comms, caller: ToolCaller) -> MessageOutcomes: if self.limit < 1: raise ToolRefusedError("limit must be positive.") source, = comms.bus.bus.deliveries_for_references((self.reference,)) - viewer = caller.outcome_viewer(comms) - if not viewer.sees(source): + if not caller.outcome_viewer(comms).sees(source): raise ToolRefusedError(f"Message {source.message.seq} is not readable by this caller.") - window = ReadNotifications(str(comms.root), (self.reference,), viewer=viewer).read(comms) + window = LiveNotificationRead((self.reference,)).read_as(comms, caller) following = self.after.following( window.outcomes.get((source.message.seq, source.message.message_id), ())) page = following[:self.limit] diff --git a/src/agent_comms/transcript_updates.py b/src/agent_comms/transcript_updates.py index de435ec6a1..cf65b6bf13 100644 --- a/src/agent_comms/transcript_updates.py +++ b/src/agent_comms/transcript_updates.py @@ -18,6 +18,7 @@ from .coordinator import Coordination from .declared_family import DeclaredFamily from .turn_lease import TurnState +from .order_tools import TranscriptPageRead from .ui_model.observation import ObservationProcess, ReadTranscript, SubscribeTranscript if TYPE_CHECKING: @@ -75,13 +76,13 @@ async def observing(self) -> AsyncIterator[ObservationProcess]: async def replay(self, session_id: str, client: SessionUpdateSink) -> None: async with self.observing() as reads: - snapshot = await reads.read(ReadTranscript(str(self.comms.root), session_id)) + snapshot = await reads.read(ReadTranscript(str(self.comms.root), TranscriptPageRead(thread=session_id))) await self._publish(session_id, snapshot, client) @asynccontextmanager async def subscribe(self, session_id: str, client: SessionUpdateSink) -> AsyncIterator[None]: async with self.observing() as reads: - snapshot = await reads.read(SubscribeTranscript(str(self.comms.root), session_id)) + snapshot = await reads.read(SubscribeTranscript(str(self.comms.root), TranscriptPageRead(thread=session_id))) await self._publish(session_id, snapshot, client) yield diff --git a/src/agent_comms/typed_table.py b/src/agent_comms/typed_table.py index db1a6f6bac..85da4013db 100644 --- a/src/agent_comms/typed_table.py +++ b/src/agent_comms/typed_table.py @@ -298,6 +298,16 @@ def reference_sql(self) -> str: ) +@dataclass(frozen=True, slots=True) +class RequestedKeys: + """One batch of distinct declared keys: the ``requested`` VALUES table they bind, and + the null-safe join that drives the table's index from it.""" + + values: str + join: str + bindings: tuple[object, ...] + + @dataclass(frozen=True) class Column(FieldDirective): """Constraints on a field; its name and type remain on the dataclass field.""" @@ -2299,9 +2309,20 @@ def select_keys(cls, db: sqlite3.Connection, """Rows equal to any of these named keys under the same null-safe IS equality select() uses: the requested keys drive an indexed join, one batch per the connection's own variable limit, each distinct key probed once.""" + return [row for requested in cls.requested_keys(db, keys) for row in cls.iterate(db.execute( + f"WITH {requested.values} " + f"SELECT {cls.qualified(*cls.column_projection())} {requested.join}", + requested.bindings))] + + @classmethod + def requested_keys(cls, db: sqlite3.Connection, + keys: Sequence[Mapping[str, object]]) -> Iterator[RequestedKeys]: + """The distinct named keys, encoded by their declared columns, in batches that fit the + connection's own variable limit; each batch joins this table as ``FROM requested + JOIN ON ...`` under select()'s null-safe IS equality.""" bound = tuple(dict.fromkeys(cls._bound(key) for key in keys)) if not bound: - return [] + return field_sets = {tuple(item.name for item in selected) for selected, _ in bound} if len(field_sets) != 1: raise ValueError(f"{cls.__name__} keys name different fields: {sorted(field_sets)}") @@ -2312,11 +2333,11 @@ def select_keys(cls, db: sqlite3.Connection, for item, alias in zip(selected, aliases, strict=True)) probe = "(" + ",".join("?" for _ in selected) + ")" batch = db.getlimit(sqlite3.SQLITE_LIMIT_VARIABLE_NUMBER) // len(selected) - return [row for chunk in batched(bound, batch) for row in cls.iterate(db.execute( - f"WITH requested({','.join(aliases)}) AS (VALUES {','.join(probe for _ in chunk)}) " - f"SELECT {cls.qualified(*cls.column_projection())} FROM requested " - f"JOIN {table} ON {joined}", - tuple(value for _, values in chunk for value in values)))] + for chunk in batched(bound, batch): + yield RequestedKeys( + f"requested({','.join(aliases)}) AS (VALUES {','.join(probe for _ in chunk)})", + f"FROM requested JOIN {table} ON {joined}", + tuple(value for _, values in chunk for value in values)) @classmethod def _query( diff --git a/src/agent_comms/ui_model/observation.py b/src/agent_comms/ui_model/observation.py index b517aac113..891fdb49b0 100644 --- a/src/agent_comms/ui_model/observation.py +++ b/src/agent_comms/ui_model/observation.py @@ -35,16 +35,17 @@ from metaclass_registry.mro_dispatch import MroDispatch, handles from agent_comms.acp_extension import ( - GoalSnapshotReply, TranscriptDraftUpdate, TranscriptSnapshotUpdate, ) from agent_comms.active_route import CommsRoute from agent_comms.activity import ObservedActivity +from agent_comms.bus_read import OperatorReader, OwnerLiveRead, OwnerReadRuntimeRequest, UiRead from agent_comms.child_process import join_retirement -from agent_comms.commander_hud import CommanderFrame, CommanderHud, CommanderUnitDetail +from agent_comms.commander_hud import CommanderFrame, CommanderHud from agent_comms.comms import Comms from agent_comms.coordinator import Coordination +from agent_comms.owner_connection import OwnerConnection, OwnerReading, OwnerReplyConnection from agent_comms.coordination_errors import ( CoordinationError, CoordinationReadUnavailable, @@ -54,19 +55,17 @@ StaleRevision, ) from agent_comms.errors import RelationViolationError, ToolRefusedError -from agent_comms.field_codec import FieldCodec, OrderId -from agent_comms.historical_views import HistorySource from agent_comms.history_views import RetiredViews -from agent_comms.message_reference import MessageReference -from agent_comms.notification_evidence import NoNotificationWindow, NotificationWindow +from agent_comms.notification_evidence import NotificationWindow +from agent_comms.notification_read import NotificationsRead, UnreadableNotificationsRead +from agent_comms.notification_reads import NotificationRead from agent_comms.presentation import ( - AllOutcomes, CoordinationSnapshot, MessageNotification, NoRevision, OutcomeViewer, ReadState, + CoordinationSnapshot, MessageNotification, NoRevision, ReadState, WireRevision, ) from agent_comms.registry_document import RosterVisibility from agent_comms.registry_provenance import DeletedThread from agent_comms.revision_views import ( - ActivityView, CommanderFramesView, NotificationsView, RegistrationsView, @@ -76,9 +75,9 @@ StoresView, ) from agent_comms.thread_identity import ThreadIncarnation -from agent_comms.thread_presentation import ThreadPresentation -from agent_comms.transcripts import TranscriptCursor, TranscriptReadIdentity -from agent_comms.ui_model.delivery import ReadDeliveryView +from agent_comms.thread_presentation import AcquiredThreadPresentation, ThreadPresentation +from agent_comms.order_tools import TranscriptPageRead +from agent_comms.transcripts import TranscriptReadIdentity from agent_comms.ui_model.sidebar import ( ChannelRowModel, SidebarModel, @@ -94,7 +93,7 @@ from concurrent.futures import Future from multiprocessing.connection import Connection - from agent_comms.runtime import RuntimeConnection + from agent_comms.tool_result import ToolResult from agent_comms.transcripts import TranscriptRead @@ -429,30 +428,21 @@ def answer(self, comms: Comms) -> ReadOutcome: @dataclass(frozen=True) -class ReadCommanderUnit(ReadRequest): - """Expand an exact published card, refusing a reused name or changed assignment.""" +class ReadOperation(ReadRequest): + """The UI's root and request witness around one root-free shared read.""" - commander: ThreadIncarnation - unit: ThreadIncarnation - orders: tuple[OrderId, ...] + operation: UiRead @property def refusal_errors(self) -> tuple[type[Exception], ...]: return (*super().refusal_errors, ToolRefusedError, WorkBranchError) - def read(self, comms: Comms) -> CommanderUnitDetail: - snapshot = comms.registry.snapshot() - if not self.commander.current(snapshot) or not self.unit.current(snapshot): - raise ToolRefusedError("The selected commander or unit incarnation is no longer current") - hud = CommanderFrame.current(comms, self.commander.name) - for card in hud.units: - if card.unit == self.unit: - return card.detail(comms, self.commander, self.orders) - raise ToolRefusedError("The selected unit is no longer in this commander's published frame") + def read(self, comms: Comms) -> object: + return self.operation.read(comms) @dataclass(frozen=True) -class OwnerRead(AwaitedRead): +class OwnerRead(OwnerReading[object], AwaitedRead): """A read the live owner of thread incarnation ``owner`` answers over its socket. Served as it arrives, beside store reads; the owner's reply is decoded @@ -486,25 +476,8 @@ async def answer(self, comms: Comms) -> ReadOutcome: except self.refusal_errors as error: return self.refused(error) - def connect(self, comms: Comms) -> RuntimeConnection: - from agent_comms.runtime import OwnerIdentityChangedError, OwnerReplyConnection, socket_path - - owner = comms.registry.require(self.owner.name) - if owner.incarnation != self.owner: - raise OwnerIdentityChangedError("The owner incarnation changed while preparing the request.") - return OwnerReplyConnection( - comms, owner.name, socket_path(comms.root, owner.require_process().pid), - ) - async def ask(self, comms: Comms) -> object: - connection = await asyncio.to_thread(self.connect, comms) - try: - return await self.read(connection) - finally: - await connection.close() - - @abstractmethod - async def read(self, owner: RuntimeConnection) -> object: ... + return await OwnerReplyConnection.answer(comms, self.owner, self) @dataclass(frozen=True) @@ -521,25 +494,16 @@ def receive(self, service: ObservationService) -> None: @dataclass(frozen=True) class TranscriptRequest[ResultT](ReadRequest): - """Capture one bounded read; its owner keeps the page and witness together.""" + """The UI's lifecycle around one shared page read: root, request id, how contention is + answered and which result it publishes. The page arguments belong to the operation.""" - thread: str - before: TranscriptCursor | None = None - after: TranscriptCursor | None = None - through: TranscriptCursor | None = None - historical_source: str | None = None + operation: TranscriptPageRead def read(self, comms: Comms) -> ResultT: return UnreadableHistory.outcome(lambda: self.result(self.page_read(comms))).raised() def page_read(self, comms: Comms) -> TranscriptRead: - return comms.transcripts.capture_page_read( - self.thread, - before=self.before, - after=self.after, - through=self.through, - historical_source=self.historical_source, - ) + return self.operation.capture_as(comms, OperatorReader()) @abstractmethod def result(self, read: TranscriptRead) -> ResultT: ... @@ -608,97 +572,32 @@ def read(self, comms: Comms) -> TranscriptSnapshotUpdate | None: ) if read.content_current(): return None - return ReadTranscript( - self.root, identity.requested_name, before=identity.before, after=identity.after, + operation = TranscriptPageRead( + thread=identity.requested_name, before=identity.before, after=identity.after, through=identity.through, historical_source=identity.historical_source, - ).read(comms) + ) + return UnreadableHistory.outcome(lambda: operation.read(comms)).raised() @dataclass(frozen=True) class ReadNotifications(ReadRequest): - """Recipient handling of the referenced original messages, as its viewer may see them. - The UI reads as the operator, who sees every outcome.""" + """The UI's root and request witness around one root-free notification operation.""" - references: tuple[MessageReference, ...] - viewer: OutcomeViewer = field(default_factory=AllOutcomes, kw_only=True) + operation: NotificationRead def read(self, comms: Comms) -> NotificationWindow: - return comms.views.message_notifications_for_references(self.viewer, self.references) - - def changed_since(self, comms: Comms, revision: WireRevision, - previous: NotificationsRead) -> bool: - return previous.live_changed(comms, revision) - - -@dataclass(frozen=True) -class ReadRecordedNotifications(ReadNotifications): - """The selected archive's recorded handling, independent of today's recipients.""" - - source: str - - def read(self, comms: Comms) -> NotificationWindow: - with comms.bus.history.read_sources() as sources: - source = HistorySource.require(sources, self.source) - return source.notification_references(sources, self.references, self.viewer) - - def changed_since(self, comms: Comms, revision: WireRevision, - previous: NotificationsRead) -> bool: - return revision.history != previous.revision.history - - -@dataclass(frozen=True) -class NotificationsRead: - """Where one notification read settled: its revision and the activity of every participant - its notifications name, captured before the read for participants already known and right - after it for new ones.""" - - revision: WireRevision - activity: Mapping[str, ObservedActivity | DeletedThread | None] = field(default_factory=dict) - notification_window: NotificationWindow = field(default_factory=NoNotificationWindow) - - @classmethod - def of(cls, revision: WireRevision, comms: Comms, - known: Mapping[str, ObservedActivity | DeletedThread], - window: NotificationWindow, - ) -> NotificationsRead: - names = window.activity_names() - known = {**comms.views.observed_activity(revision, names - known.keys()), **known} - return cls(revision, {name: known.get(name) for name in names}, window) - - def live_changed(self, comms: Comms, revision: WireRevision) -> bool: - """Read marks, participant activity or retained assignment evidence changed.""" - current = (comms.views.observed_activity(revision, self.activity) - if revision.changed_since(ActivityView(), self.revision) - else self.activity) - return (any(current.get(name) != seen for name, seen in self.activity.items()) - or self.notification_window.changed(comms, revision, self.revision)) - - -class UnreadableNotificationsRead(NotificationsRead): - """An unreadable read names no participants, so any later revision reads again.""" - - def live_changed(self, comms: Comms, revision: WireRevision) -> bool: - return revision != self.revision + return self.operation.read(comms) @dataclass(frozen=True) -class ReadOwnerGoal(OwnerRead): - """The owner's goal snapshot and the turn it read with it.""" - - async def read(self, owner: RuntimeConnection) -> GoalSnapshotReply: - return FieldCodec.decode(GoalSnapshotReply, await owner.request("goal_snapshot")) +class ReadOwnerOperation(OwnerRead): + """The UI's envelope for an owner-live read: the operation's own reply, from the held owner.""" + operation: OwnerLiveRead -@dataclass(frozen=True) -class ReadDeliveryOverview(OwnerRead): - """The owner's delivery notices, checked against its live queue.""" - - include_history: bool = False - - async def read(self, owner: RuntimeConnection) -> ReadDeliveryView: - return ReadDeliveryView.from_wire( - await owner.request("input_dispositions", include_history=self.include_history) - ) + async def read(self, owner: OwnerConnection) -> ToolResult: + request = OwnerReadRuntimeRequest(thread=owner.session_id, operation=self.operation) + return self.operation.decode_reply(await request.read(owner)) @dataclass(frozen=True) @@ -860,7 +759,7 @@ def continue_from(self, previous: Self) -> Self: def read(self, comms: Comms, revision: WireRevision) -> Generator[ObservationStep]: for request in self.request.reads: previous = self.read_states.get(request) - if previous is not None and not request.changed_since(comms, revision, previous): + if previous is not None and not request.operation.changed_since(comms, revision, previous): continue yield self.notifications(request, comms, revision, previous) @@ -920,8 +819,11 @@ def read(self, comms: Comms, revision: WireRevision) -> Generator[ObservationSte changed = observed.changed_since(previous) | self.initial self.initial = frozenset() if changed: + # The reader keeps its acquired results for the next change check; a viewer + # receives only each changed result's published projection. yield replace(observed, changed=changed, - results={name: value for name, value in results.items() if name in changed}) + results={name: value.published() for name, value in results.items() + if name in changed}) def located(self, comms: Comms, revision: WireRevision, previous: ThreadsObserved) -> LocatedThreads: # Keep each completed stage when a later stored read fails, exactly as @@ -1108,10 +1010,15 @@ class ThreadReadResult(PublishState, ABC): def same_as_presented(self, presented: ThreadPresented) -> bool: return False + def published(self) -> ThreadReadResult: + """What a viewer receives of this read: the read itself.""" + return self + @abstractmethod def status_row(self, name: str, owner: OpenThreadStatus) -> ThreadStatusModel: ... - def collect_revision_inputs(self, name: str, readable: dict[str, ThreadPresentation], retry: set[str]) -> None: + def collect_revision_inputs(self, name: str, readable: dict[str, AcquiredThreadPresentation], + retry: set[str]) -> None: """Supply readable values and reads that must be tried again.""" def same_status(self, presentation: ThreadPresentation) -> bool: @@ -1129,10 +1036,14 @@ def same_as_presented(self, presented: ThreadPresented) -> bool: return (presented.presentation is self.presentation or presented.presentation.same_observation(self.presentation)) + def published(self) -> ThreadPresented: + return ThreadPresented(self.presentation.published()) + def status_row(self, name: str, owner: OpenThreadStatus) -> ThreadStatusModel: return ThreadStatusModel.known(name, self.presentation) - def collect_revision_inputs(self, name: str, readable: dict[str, ThreadPresentation], retry: set[str]) -> None: + def collect_revision_inputs(self, name: str, readable: dict[str, AcquiredThreadPresentation], + retry: set[str]) -> None: self.presentation.collect_revision_input(name, readable) def same_status(self, presentation: ThreadPresentation) -> bool: @@ -1152,7 +1063,8 @@ class ThreadBusy(ThreadReadResult): def status_row(self, name: str, owner: OpenThreadStatus) -> ThreadStatusModel: return owner.open(name) - def collect_revision_inputs(self, name: str, readable: dict[str, ThreadPresentation], retry: set[str]) -> None: + def collect_revision_inputs(self, name: str, readable: dict[str, AcquiredThreadPresentation], + retry: set[str]) -> None: retry.add(name) @@ -1164,8 +1076,8 @@ class ThreadsObserved(Observed): changed: frozenset[str] activity: Mapping[str, ObservedActivity | DeletedThread] - def revision_inputs(self) -> tuple[dict[str, ThreadPresentation], set[str]]: - readable: dict[str, ThreadPresentation] = {} + def revision_inputs(self) -> tuple[dict[str, AcquiredThreadPresentation], set[str]]: + readable: dict[str, AcquiredThreadPresentation] = {} retry: set[str] = set() for name, result in self.results.items(): result.collect_revision_inputs(name, readable, retry) diff --git a/tests/conftest.py b/tests/conftest.py index 267bdec096..cf7d9fc0d9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -55,7 +55,7 @@ from agent_comms.registry_document import RegisteredCommanderProject from agent_comms_hooks.watch import WatchClaim, WatchRecord -pytest_plugins = (goal_owner_fixture.__name__, "github_fixture", "pytest_session") +pytest_plugins = (goal_owner_fixture.__name__, "github_fixture", "owner_socket_fixture", "pytest_session") class TranscriptRecords: diff --git a/tests/guards/test_debt_ratchet.py b/tests/guards/test_debt_ratchet.py index 738aecee97..98de8fc63f 100644 --- a/tests/guards/test_debt_ratchet.py +++ b/tests/guards/test_debt_ratchet.py @@ -46,10 +46,12 @@ AnyName, BooleanChainTerms, BuiltinHandlerTypeSwitch, + ClientTypeErrors, CodecSubclass, FamilyFlattened, ForeignAbsenceProbe, GodClassExcess, + HeadSide, LintFindings, LongBooleanChain, Measure, @@ -66,17 +68,36 @@ Unformatted, compare, ) +from module_layers import StackClient pytestmark = pytest.mark.refactor_guard def test_measure_owner_rejects_a_new_absence() -> None: name = NoneAbsence.at("tools/example.py") - result = Comparison("tools", "base", "head", (), {name: 0}, {name: 1}, {}, {}) + result = Comparison("tools", "base", "head", (), {name: 0}, {name: 1}, {name: [1]}, {}) assert result.delta[name] == 1 assert result.failed +GROWN_ABSENCE = NoneAbsence.at("tools/example.py") + + +@pytest.mark.parametrize( + ("introduced", "failed", "missing"), + [({}, True, [GROWN_ABSENCE]), ({GROWN_ABSENCE: []}, False, []), ({GROWN_ABSENCE: [1]}, True, [])], + ids=["unreported", "reported_none_new", "reported_new"], +) +def test_clean_lines_growth_fails_only_on_a_new_site_or_a_missing_report(introduced, failed, missing): + """Raw growth alone is a move; an explicit empty report says nothing is new, while an + absent report proves nothing.""" + result = Comparison("tools", "base", "head", (), {GROWN_ABSENCE: 0}, {GROWN_ABSENCE: 1}, introduced, {}) + assert result.delta[GROWN_ABSENCE] == 1 + assert result.failed is failed + assert result.missing == missing + assert FieldCodec.project(result, "report")["missing"] == [key.text for key in missing] + + @pytest.mark.parametrize( ("source", "expected"), [ @@ -834,6 +855,51 @@ def test_both_revisions_are_checked_under_the_head_configuration(repo: Repositor assert report["delta"][measure.at(f"{repo.root}/kept.py").text] == 0 +USES_EXTERNAL_VALUE = "import extdep\n\nvalue: int = extdep.VALUE\n" + + +def typed_dependency(directory: Path, annotation: str, value: str) -> Path: + """An installed PEP 561 package outside every committed root, found through sys.path.""" + package = directory / "extdep" + package.mkdir(parents=True) + (package / "py.typed").write_text("") + (package / "__init__.py").write_text(f"VALUE: {annotation} = {value}\n") + return directory + + +def source_type_errors(repo: Repository, client: StackClient) -> int: + revision = repo.commit({"uses_external.py": USES_EXTERNAL_VALUE}) + selected = [f"{repo.root}/uses_external.py"] + _, counted = TypeErrors.findings(repo.path, revision, revision, repo.root, selected, HeadSide) + return counted[selected[0]] + + +def client_type_errors(repo: Repository, client: StackClient) -> int: + revision = repo.commit({"__init__.py": ""}) + _, counted = ClientTypeErrors.findings(repo.path, revision, client, HeadSide) + return counted[f"{client.name}/src/uses_external.py"] + + +@pytest.mark.parametrize("type_errors", [source_type_errors, client_type_errors]) +def test_type_counts_follow_the_installed_dependency_not_a_stored_count( + repo: Repository, tmp_path: Path, monkeypatch, type_errors +) -> None: + """Mypy reads installed packages no content key names. With committed source, client and + configuration unchanged, switching the installed dependency must change the count, under + the same count-cache directory and mypy's own cache: hosted #3538 compared a base count + stored under a Core-only environment with a head count from the full stack.""" + accepting = typed_dependency(tmp_path / "accepting", "int", "1") + rejecting = typed_dependency(tmp_path / "rejecting", "str", "'one'") + (tmp_path / "client").mkdir() + checkout = Repository(tmp_path / "client", "src") + checkout.commit({"uses_external.py": "import agent_comms\n" + USES_EXTERNAL_VALUE}) + client = StackClient("probe-client", checkout.path) + monkeypatch.setenv("PYTHONPATH", search_path(accepting)) + assert type_errors(repo, client) == 0 + monkeypatch.setenv("PYTHONPATH", search_path(rejecting)) + assert type_errors(repo, client) == 1 + + def test_foreign_probes_and_codec_bases_use_ast_not_text(repo: Repository) -> None: base = repo.commit({}) head = repo.commit( @@ -1124,6 +1190,7 @@ def test_an_absence_moved_with_its_class_to_another_module_is_not_written(repo): base = repo.commit({"attempts.py": holder, "executions.py": "VALUE = 1\n"}) moved = repo.commit({"attempts.py": "VALUE = 2\n", "executions.py": holder}) status, report = repo.compare(base, moved, NoneAbsence) + assert status == 0 assert report["introduced"][NoneAbsence.at(f"{repo.root}/executions.py").text] == [] deleted = repo.commit({"attempts.py": holder, "executions.py": None}) moved_out = repo.commit({"attempts.py": None, "owners.py": holder + "\n\n" + other}) @@ -1132,6 +1199,106 @@ def test_an_absence_moved_with_its_class_to_another_module_is_not_written(repo): assert report["introduced"][NoneAbsence.at(f"{repo.root}/owners.py").text] == [8] +EXTRACTED_FROM = ( + "class Request:\n thread: str\n before: int | None = None\n archive: str | None = None\n" + "\n\nclass Other:\n name: str\n" +) +HOLDER = "from .pages import Page\n\n\nclass Request:\n thread: str\n operation: Page\n" +OTHER = "\n\nclass Other:\n name: str\n" +PAGE = ( + "from typing import Annotated\n\n\nclass Page:\n" + " before: Annotated[int | None, \"Read before this cursor\"] = None\n" + " archive: str | None = None\n" +) + + +@pytest.mark.parametrize( + ("holder", "pages", "written"), + [ + (HOLDER + OTHER, PAGE, 0), + (HOLDER.replace(" operation", " before: int | None = None\n operation") + OTHER, PAGE, 1), + (HOLDER.replace("operation: Page", "operation: object") + OTHER, PAGE, 2), + (HOLDER + OTHER + " page: Page\n", PAGE, 2), + (HOLDER + "\n\nPage = dict\n" + OTHER, PAGE, 2), + (HOLDER.replace(".pages", ".elsewhere") + OTHER, PAGE, 2), + (HOLDER + OTHER, PAGE.replace("before:", "earlier:"), 1), + (HOLDER + OTHER, PAGE.replace("int | None", "float | None"), 1), + (HOLDER + OTHER, PAGE.replace("archive: str | None = None", "archive: str | None = ''"), 1), + (HOLDER + OTHER, PAGE + " limit: int | None = None\n", 1), + (HOLDER + OTHER, PAGE + "\n def empty(self):\n return self.before is None\n", 1), + (HOLDER.replace("import Page", "import Copy, Page").replace( + "operation: Page", "operation: Page\n copy: Copy") + OTHER, + PAGE + "\n\nclass Copy:\n before: int | None = None\n", 3), + (HOLDER.replace("import Page", "import Page as Held").replace("operation: Page", "operation: Held") + + OTHER, PAGE.replace("import Annotated", "import Annotated as Meta").replace("Annotated[", "Meta["), 0), + (HOLDER.replace("from .pages import Page\n", "def load():\n from .pages import Page\n") + OTHER, + PAGE, 2), + (HOLDER + "\n\ndef make():\n Page = dict\n return Page\n" + OTHER, PAGE, 0), + (HOLDER + OTHER, PAGE + "\n\nAnnotated = list\n", 1), + (HOLDER + OTHER, PAGE.replace("from typing import Annotated\n", "def typed():\n from typing import Annotated\n"), 1), + (HOLDER + OTHER, PAGE + "\n\nPage = dict\n", 2), + (HOLDER + OTHER, PAGE + " archive = ''\n", 1), + (HOLDER.replace(" operation: Page", " operation: Page\n before, extra = 1, 2") + OTHER, PAGE, 1), + (HOLDER + OTHER, PAGE.replace("class Page:\n", "class Page:\n Annotated = dict\n"), 1), + (HOLDER.replace(" operation: Page", " Page = dict\n operation: Page") + OTHER, PAGE, 2), + ], + ids=["extracted", "retained", "unheld", "second_owner", "shadowed", "unresolved", "renamed", + "retyped", "redefaulted", "extra_field", "method", "copied_twice", "aliased_imports", + "nested_only_import", "unrelated_local_name", "annotated_rebound", "annotated_local_only", + "export_rebound", "field_rebound", "source_rebound_by_unpacking", + "annotated_shadowed_in_class", "held_class_shadowed_in_owner"], +) +def test_fields_extracted_into_the_one_new_held_class_are_moved_not_written(repo, holder, pages, written): + """An old class that now holds one new class in a typed field carries each field it lost + into that class unchanged; anything the proof does not cover is still written.""" + base = repo.commit({"holder.py": EXTRACTED_FROM, "pages.py": "VALUE = 1\n"}) + head = repo.commit({"holder.py": holder, "pages.py": pages}) + status, report = repo.compare(base, head, NoneAbsence) + source, destination = (NoneAbsence.at(f"{repo.root}/{name}").text for name in ("holder.py", "pages.py")) + assert status == int(written > 0) + assert len(report["introduced"][destination]) == written + assert report["introduced"][source] == [] + assert report["base"][source] == 2 and report["head"][destination] >= 2 + assert report["delta"][destination] == report["head"][destination] + + +TYPED_FROM = ( + "from .alpha import Cursor\n\n\nclass Request:\n thread: str\n" + " before: Cursor | None = None\n archive: str | None = None\n" +) +TYPED_PAGE = ( + "from .alpha import Cursor\n\n\nclass Page:\n" + " before: Cursor | None = None\n archive: str | None = None\n" +) + + +@pytest.mark.parametrize( + ("source", "pages", "written"), + [ + (TYPED_FROM, TYPED_PAGE, 0), + (TYPED_FROM, TYPED_PAGE.replace("import Cursor", "import Cursor as Mark").replace( + "before: Cursor", "before: Mark"), 0), + (TYPED_FROM, TYPED_PAGE.replace(".alpha", ".beta"), 1), + (TYPED_FROM, TYPED_PAGE + "\n\nstr = bytes\n", 1), + (TYPED_FROM, TYPED_PAGE.replace(" archive", " str = bytes\n archive"), 1), + (TYPED_FROM.replace("from .alpha import Cursor", "from typing import TypeVar\n\nCursor = TypeVar('Cursor')"), + TYPED_PAGE.replace("from .alpha import Cursor", "from typing import TypeVar\n\nCursor = TypeVar('Cursor')"), 1), + ], + ids=["same_owner", "same_owner_aliased", "changed_owner", "builtin_shadowed", + "builtin_shadowed_in_class", "type_variable"], +) +def test_extracted_field_types_are_compared_by_the_owner_they_import(repo, source, pages, written): + """A field type is the module and name its spelling is bound to, not the spelling: the same + Cursor imported from another module is another type, and an alias of the same is not.""" + base = repo.commit({"holder.py": source + OTHER, "pages.py": "VALUE = 1\n"}) + head = repo.commit({"holder.py": HOLDER + OTHER, "pages.py": pages}) + status, report = repo.compare(base, head, NoneAbsence) + source_count, destination = (NoneAbsence.at(f"{repo.root}/{name}").text for name in ("holder.py", "pages.py")) + assert status == int(written > 0) + assert len(report["introduced"][destination]) == written + assert report["introduced"][source_count] == [] + + def test_git_context_configuration_cannot_mark_unchanged_lines_as_new(repo): repo.git("config", "diff.interHunkContext", "3") base = repo.commit({"names.py": "first = 1\nold_receipt = 1\nlast = 1\n"}) diff --git a/tests/observed_queue.py b/tests/observed_queue.py new file mode 100644 index 0000000000..e65a850d6d --- /dev/null +++ b/tests/observed_queue.py @@ -0,0 +1,38 @@ +"""What the real observer receiver delivers, in receive order, for tests that watch it. + +``ObservationProcess.start_receiving`` owns reading the pipe; a test passes this queue's +callbacks to it once and waits here. A receiver failure is raised as its original exception +when the waiting test reaches it, never swallowed or replaced. +""" +from __future__ import annotations + +import asyncio +from collections.abc import Callable + +from agent_comms.ui_model.observation import Observed + + +class ObservedQueue: + def __init__(self) -> None: + self._delivered: asyncio.Queue[Callable[[], Observed]] = asyncio.Queue() + + def observed(self, result: Observed) -> None: + self._delivered.put_nowait(lambda: result) + + def failed(self, error: Exception) -> None: + def raise_original() -> Observed: + raise error + + self._delivered.put_nowait(raise_original) + + async def next(self) -> Observed: + async with asyncio.timeout(30): + delivered = await self._delivered.get() + return delivered() + + async def until(self, accepted: Callable[[Observed], bool]) -> Observed: + """One 30 s bound for the whole match, however many other results arrive first.""" + async with asyncio.timeout(30): + while not accepted(result := await self.next()): + pass + return result diff --git a/tests/owner_socket_fixture.py b/tests/owner_socket_fixture.py new file mode 100644 index 0000000000..c328086c64 --- /dev/null +++ b/tests/owner_socket_fixture.py @@ -0,0 +1,69 @@ +"""Registered owner threads, their runtime sockets and their child processes for owner-read tests. + +A native owner's socket here is a real Unix-socket peer that receives exactly one request +and closes without replying: it proves dispatch and then reply loss, never the domain +answer. An external owner has a live process and no socket. Child processes are real +spawned processes that a test may stop through their own owner. +""" +from __future__ import annotations + +import asyncio +import json +import sys +from collections.abc import AsyncIterator, Iterator +from contextlib import asynccontextmanager + +import pytest + +from agent_comms.child_process import ProcessIdentity, SpawnedProcess +from agent_comms.comms import Comms +from agent_comms.private_path import PrivateSocketRole +from agent_comms.repo import Checkout +from agent_comms.runtime import socket_path +from agent_comms.thread_execution import NativeThreadExecution, ThreadExecution +from agent_comms.threads import Thread + + +@pytest.fixture +def runtime_processes() -> Iterator[tuple[SpawnedProcess, SpawnedProcess]]: + children = [] + try: + for _ in range(2): + children.append( + SpawnedProcess.launch((sys.executable, "-c", "import time; time.sleep(60)")) + ) + yield tuple(children) + finally: + for child in children: + child.stop_sync() + + +def declare_owner( + comms: Comms, name: str, process: ProcessIdentity, + execution: type[ThreadExecution] = NativeThreadExecution, +) -> Thread: + return comms.registry.declare(Thread( + name, frozenset(), Checkout(comms.root.parent), process_identity=process, execution=execution, + )) + + +@asynccontextmanager +async def silent_socket(comms: Comms, process: ProcessIdentity) -> AsyncIterator[list[dict[str, object]]]: + """The owner's socket path served by a peer that reads one request and hangs up.""" + path = socket_path(comms.root, process.pid) + path.parent.mkdir(parents=True, exist_ok=True) + received: list[dict[str, object]] = [] + + async def disconnect_after_request(reader, writer): + received.append(json.loads(await reader.readline())) + writer.close() + await writer.wait_closed() + + with PrivateSocketRole.address(path) as address: + server = await asyncio.start_unix_server(disconnect_after_request, path=address) + try: + yield received + finally: + server.close() + await server.wait_closed() + path.unlink(missing_ok=True) diff --git a/tests/round2/test_typed_table.py b/tests/round2/test_typed_table.py index e8c6dc5093..ac23d0d4b9 100644 --- a/tests/round2/test_typed_table.py +++ b/tests/round2/test_typed_table.py @@ -179,12 +179,19 @@ def test_declared_table_family(tmp_path): for row in (parent, child): row.insert(db) assert type(row).select(db) == [row] + # Requested keys join under IS, so a null key selects the null row, once. + null_key = {"key": "c", "weight": None} + assert TableChildRow.select_keys(db, (null_key, null_key)) == [child] + assert TableChildRow.select_keys(db, ({"key": "c", "weight": 3.25},)) == [] with pytest.raises(sqlite3.IntegrityError): replace(child, key="bad", parent="missing").insert(db) with pytest.raises(TypeError): replace(parent, key="bool", count=True).insert(db) replace(child, weight=3.25).upsert(db) assert TableChildRow.one(db, key="c").weight == 3.25 + assert TableChildRow.select_keys(db, (null_key,)) == [] + assert TableChildRow.select_keys(db, ({"key": "c", "weight": 3.25},)) == [ + replace(child, weight=3.25)] child.upsert(db) assert TableChildRow.one(db, key="c") == child TableChildRow.update(db, where="key=?", parameters=("c",), enabled=False) diff --git a/tests/test_busy_history_loading.py b/tests/test_busy_history_loading.py index db4643f23f..abf8aa9ecd 100644 --- a/tests/test_busy_history_loading.py +++ b/tests/test_busy_history_loading.py @@ -11,6 +11,8 @@ from agent_comms.acp_extension import TranscriptSnapshotUpdate from agent_comms.comms import wire +from agent_comms.notification_reads import LiveNotificationRead +from agent_comms.order_tools import TranscriptPageRead from agent_comms.ui_model.observation import ( ObservationProcess, ReadBusy, ReadNotifications, ReadTranscript, ) @@ -57,15 +59,15 @@ def held_source(comms): async def test_busy_source_keeps_original_waiter_and_other_reads_move(tmp_path): comms, session = saved_history(tmp_path) original = session.read_bytes() - with ObservationProcess.awaited(comms.route) as process: + async with ObservationProcess.awaited(comms.route) as process: with held_source(comms): - answer = ReadTranscript(str(comms.root), "saved", request_id=19).answer(comms) + answer = ReadTranscript(str(comms.root), TranscriptPageRead(thread="saved"), request_id=19).answer(comms) assert isinstance(answer, ReadBusy) assert answer.title == "History loading" - waiting = asyncio.create_task(process.read(ReadTranscript(str(comms.root), "saved"))) - independent = asyncio.create_task(process.read(ReadNotifications(str(comms.root), ()))) + waiting = asyncio.create_task(process.read(ReadTranscript(str(comms.root), TranscriptPageRead(thread="saved")))) + independent = asyncio.create_task(process.read(ReadNotifications(str(comms.root), LiveNotificationRead(())))) async with asyncio.timeout(15): - assert await independent == {} + assert (await independent).outcomes == {} assert not waiting.done() assert len(process.reads) == 1 (request_id,) = process.reads @@ -84,16 +86,16 @@ async def test_busy_source_keeps_original_waiter_and_other_reads_move(tmp_path): async def test_cancelled_busy_read_is_removed_without_resubmission(tmp_path): comms, _session = saved_history(tmp_path) - with ObservationProcess.awaited(comms.route) as process: + async with ObservationProcess.awaited(comms.route) as process: with held_source(comms): - waiting = asyncio.create_task(process.read(ReadTranscript(str(comms.root), "saved"))) + waiting = asyncio.create_task(process.read(ReadTranscript(str(comms.root), TranscriptPageRead(thread="saved")))) await asyncio.sleep(0.15) waiting.cancel() with pytest.raises(asyncio.CancelledError): await waiting assert process.reads == {} async with asyncio.timeout(5): - assert await process.read(ReadNotifications(str(comms.root), ())) == {} + assert (await process.read(ReadNotifications(str(comms.root), LiveNotificationRead(())))).outcomes == {} assert process.reads == {} @@ -112,10 +114,10 @@ def closed(connection): monkeypatch.setattr(Connection, "close", closed) with pytest.raises(type(error)) as raised: - with process: + async with process: raise error assert raised.value is error - process.close() + await process.close() assert closes == [process.connection] assert process.connection.closed and process.process.exitcode == 0 @@ -187,7 +189,8 @@ async def test_peer_write_failure_retires_once_and_cancels_owner_reads(comms): async def test_owner_reply_uses_the_same_peer_retirement(comms): import multiprocessing - from agent_comms.ui_model.observation import ObservationService, ReadOwnerGoal + from agent_comms.order_tools import OwnerGoalRead + from agent_comms.ui_model.observation import ObservationService, ReadOwnerOperation connection, viewer = multiprocessing.get_context("spawn").Pipe() with connection, viewer: @@ -195,7 +198,8 @@ async def test_owner_reply_uses_the_same_peer_retirement(comms): # Owner reads live in the service's task group, which only its run enters. async with service.tasks: # A real missing-owner refusal still has to be sent through the shared endpoint. - request = ReadOwnerGoal(str(comms.root), UnrecordedThreadIncarnation("missing"), request_id=7) + request = ReadOwnerOperation(str(comms.root), UnrecordedThreadIncarnation("missing"), + request_id=7, operation=OwnerGoalRead()) request.receive(service) asking = service.owner_reads[7] viewer.close() @@ -206,8 +210,8 @@ async def test_owner_reply_uses_the_same_peer_retirement(comms): async def test_awaited_notifications_then_immediate_close_retires_successfully(comms): - with ObservationProcess.awaited(comms.route) as process: - assert await process.read(ReadNotifications(str(comms.root), ())) == {} + async with ObservationProcess.awaited(comms.route) as process: + assert (await process.read(ReadNotifications(str(comms.root), LiveNotificationRead(())))).outcomes == {} assert process.connection.closed and process.process.exitcode == 0 assert not process.reads diff --git a/tests/test_command_families.py b/tests/test_command_families.py index e241ada8bb..83634ab361 100644 --- a/tests/test_command_families.py +++ b/tests/test_command_families.py @@ -1,5 +1,6 @@ from agent_comms.repo import Checkout, Repo -from agent_comms.field_codec import FieldCodec, GoalId +from agent_comms.declared_family import UnknownFamilyMember +from agent_comms.field_codec import FieldCodec, GoalId, MalformedRecord """External contracts and adding commands without editing any consumer catalog.""" @@ -9,7 +10,6 @@ from acp.schema import PromptRequest, TextContentBlock from agent_comms.cli import build_parser, main -from agent_comms.cli_command import CliCommand from agent_comms.comms import wire from agent_comms.history_result import ( ChannelHistoryPageResult, @@ -87,10 +87,13 @@ def parse(args): {"action": "cancel"}, {"action": "set_config_option", "config_id": "model", "value": "test/model"}, {"action": "compact", "instructions": "focus"}, - {"action": "input_dispositions", "include_history": True}, + {"action": "owner_read", "operation": { + "tool": "comms_input_delivery", "thread": "", "include_history": True}}, + {"action": "owner_read", "operation": { + "tool": "comms_input_delivery", "thread": "", "include_history": False}}, {"action": "dismiss_historical_inputs"}, {"action": "goal_history", "goal_id": None}, - {"action": "goal_snapshot"}, + {"action": "owner_read", "operation": {"tool": "comms_owner_goal", "thread": ""}}, {"action": "edit_goal", "goal_id": None, "expected_revision": 3, "text": "new goal"}, {"action": "update_goal", "goal_id": None, "expected_revision": 3, "status": "paused"}, {"action": "retry_goal", "goal_id": None, "expected_revision": 3}, @@ -118,7 +121,9 @@ def test_runtime_wire_format_round_trip(parameters): @pytest.mark.parametrize( "parameters", [ - {"action": "input_dispositions", "include_history": "yes"}, + {"action": "owner_read", "operation": { + "tool": "comms_input_delivery", "include_history": "yes"}}, + {"action": "owner_read", "operation": {"tool": "comms_goal_history"}}, {"action": "goal_history", "goal_id": 2}, {"action": "edit_goal", "goal_id": None, "expected_revision": True, "text": "x"}, {"action": "retry_goal"}, @@ -139,6 +144,19 @@ def test_invalid_runtime_parameters_use_canonical_decode_and_domain_errors(param assert str(refused.value) == str(declared.value) +@pytest.mark.parametrize("parameters", [ + {"action": "goal_snapshot"}, + {"action": "goal_snapshot", "operation": {"tool": "comms_owner_goal", "thread": ""}}, +], ids=["bare", "with_operation"]) +def test_the_deleted_goal_snapshot_command_is_an_undeclared_member(parameters): + with pytest.raises(UnknownFamilyMember) as undeclared: + RuntimeRequest.member_named("goal_snapshot") + with pytest.raises(MalformedRecord) as refused: + FieldCodec.decode(RuntimeRequest, {"thread": "owner", **parameters}) + assert type(refused.value.__cause__) is UnknownFamilyMember + assert str(refused.value) == str(refused.value.__cause__) == str(undeclared.value) + + @pytest.mark.parametrize( "arguments,result_type,per_root", [ diff --git a/tests/test_commander_notifications.py b/tests/test_commander_notifications.py index 9df9d90041..e015403f5c 100644 --- a/tests/test_commander_notifications.py +++ b/tests/test_commander_notifications.py @@ -28,7 +28,12 @@ from agent_comms.todos import ProgressRead, UnitOccupied from agent_comms.ui_model import observation from agent_comms.ui_model.observation import CommanderHudObserved, ObservationService, ObserveCommanderHud -from agent_comms.ui_model.observation import ReadCommanderUnit, ReadRefused +from agent_comms.field_codec import FieldCodec +from agent_comms.order_tools import CommanderUnitRead +from agent_comms.tool_caller import ThreadCaller +from agent_comms.tools import invoke_tool +from agent_comms.work_branches import WorkBranchError +from agent_comms.ui_model.observation import ReadOperation, ReadRefused from agent_comms.cli_bridges.codex_connection import CodexConnection from agent_comms.standing_rules import ServicePassFailing, StandingRule, UndatedStandingRule from agent_comms.ui_model.unit_state import ( @@ -88,9 +93,16 @@ def test_expanded_unit_reads_original_orders_and_bounded_activity_then_refuses_s hud = beat(comms, states=state_cards(comms, UnitState)) ServicePassStore.at(comms.root).heartbeat_passed((hud.commander,), (hud.commander,)) card = next(card for card in hud.units if card.unit.name == 'worker') - request = ReadCommanderUnit(str(comms.root), hud.commander, card.unit, - tuple(dict.fromkeys(order.order for order in card.orders))) + operation = CommanderUnitRead(commander=hud.commander, unit=card.unit, + orders=tuple(dict.fromkeys(order.order for order in card.orders))) + request = ReadOperation(str(comms.root), operation) + direct = operation.read(comms) detail = pickle.loads(pickle.dumps(request.read(comms))) + # The shared operation, the UI's root-bound envelope and the MCP boundary answer the same + # selected card with the same bounded records. + assert detail == direct + assert invoke_tool(comms, CommanderUnitRead.declared_name, FieldCodec.encode_fields(operation), + caller=ThreadCaller("lead")) == FieldCodec.encode(direct) assert detail.unit == card assert {order.card.problem for order in detail.orders} == {'P1', 'P2'} for order in detail.orders: @@ -102,10 +114,10 @@ def test_expanded_unit_reads_original_orders_and_bounded_activity_then_refuses_s assert 'contents' not in json.dumps(detail.activity) assert (comms.root / 'state-source.jsonl').read_bytes() == original for stale in ( - replace(request, orders=()), + replace(request, operation=replace(operation, orders=())), replace(request, root=str(comms.root / 'other')), - replace(request, unit=replace(card.unit, created_at=card.unit.wire_birth + 1)), - replace(request, commander=card.unit), + replace(request, operation=replace(operation, unit=replace(card.unit, created_at=card.unit.wire_birth + 1))), + replace(request, operation=replace(operation, commander=card.unit)), ): assert isinstance(stale.answer(comms), ReadRefused) checkout = comms.goals.todos.get('P1').order.checked().check.directory @@ -115,6 +127,10 @@ def test_expanded_unit_reads_original_orders_and_bounded_activity_then_refuses_s try: metadata.write_text('gitdir: ./missing-original-git\n') assert isinstance(request.answer(comms), ReadRefused) + # MCP reaches the same nominal refusal, which the connector answers as a failed call. + with pytest.raises(WorkBranchError): + invoke_tool(comms, CommanderUnitRead.declared_name, FieldCodec.encode_fields(operation), + caller=ThreadCaller("lead")) finally: metadata.unlink() saved.rename(metadata) diff --git a/tests/test_connector.py b/tests/test_connector.py index 6e8e8ee011..e238f4afab 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -22,6 +22,7 @@ from aiohttp.test_utils import TestClient, TestServer from connector_readiness import ConnectorReadiness +from conftest import transcript_records from agent_comms.bus import Bus from agent_comms.comms import Comms, wire @@ -32,22 +33,27 @@ ConnectorVerb, ServeVerb, RevokeVerb, + ToolsCallResult, ) from agent_comms.connector_caller import CALLER, DirectorCaller from agent_comms.connector_store import ( CALLBACK, CODE_SECONDS, AuthorizationQuery, ConnectorGrants, ConnectorStore, IssuedCode, TrustedBrowser, digest, ) +from agent_comms.coordination_errors import TranscriptChanged from agent_comms.coordinator import Coordination from agent_comms.locked_store import DocumentChange from agent_comms.message_bus import WatchPass from agent_comms.service_passes import ServicePassStore from agent_comms.standing_rules import ServicePassFailing -from agent_comms.field_codec import FieldCodec +from agent_comms.field_codec import FieldCodec, MalformedRecord from agent_comms.host_services import ServiceNotServingError from agent_comms.host_services.connector import ConnectorService from agent_comms.installer.backup import StoreBackup from agent_comms.launch_root import LaunchRoot +from agent_comms.native_transcript import NativeTranscript +from agent_comms.order_tools import TranscriptPageRead +from agent_comms.session_file import RegistrySessionLocation from agent_comms.maintenance_barrier import ( InstallingPhase, MaintenanceBarrier, MaintenanceHeldError, MaintenanceOperator, PausedPhase, ReadyPhase, @@ -269,6 +275,65 @@ async def test_expected_tool_argument_refusal_keeps_its_mcp_reply(client, comms, assert json.loads(reply["result"]["content"][0]["text"])["error"] +async def tool_call(client: TestClient, token: str, call_id: int, name: str, arguments: dict): + response = await client.post("/mcp", headers={"Authorization": f"Bearer {token}"}, json={ + "jsonrpc": "2.0", "id": call_id, "method": "tools/call", + "params": {"name": name, "arguments": arguments}, + }) + return response.status, await response.json() + + +def bind_worker_session(comms: Comms, path: Path) -> None: + worker = comms.registry.require("worker") + comms.registry.declare(replace( + worker, session_file=RegistrySessionLocation.from_path(str(path), comms.root))) + + +def saved_session(path: Path, *entries: bytes) -> Path: + header = json.dumps(transcript_records.session(id="session").to_wire()).encode() + path.write_bytes(b"".join((header + b"\n", *entries))) + return path + + +async def test_an_unreadable_transcript_is_a_failed_call_and_the_connector_keeps_serving( + client, comms, tmp_path, +): + """A corrupt complete native entry escaped tools/call as an HTTP 500 with no id.""" + path = saved_session(tmp_path / "corrupt.jsonl", b'{"type":"message"\n') + bind_worker_session(comms, path) + with pytest.raises(MalformedRecord) as unreadable: + tuple(NativeTranscript(path).tail()) + token = await approved_token(client, comms) + status, reply = await tool_call( + client, token, 41, TranscriptPageRead.declared_name, {"thread": "worker"}) + assert (status, reply["id"]) == (200, 41) + # The domain's own MalformedRecord, with its JSONDecodeError cause, is the failed result. + assert reply["result"] == FieldCodec.encode(ToolsCallResult.failed(unreadable.value)) + assert reply["result"]["isError"] is True and "structuredContent" not in reply["result"] + assert "JSONDecodeError" in reply["result"]["content"][0]["text"] + status, served = await tool_call(client, token, 42, CommsChannelsTool.declared_name, {}) + assert (status, served["id"], served["result"]["isError"]) == (200, 42, False) + + +async def test_a_stale_transcript_cursor_is_a_failed_call_with_its_coordination_cause( + client, comms, tmp_path, +): + message = transcript_records.message({"role": "assistant", "content": [{"type": "text", "text": "held"}]}) + bind_worker_session(comms, saved_session( + tmp_path / "session.jsonl", json.dumps(message).encode() + b"\n")) + page = comms.transcripts.capture_page_read("worker").read() + bind_worker_session(comms, saved_session(tmp_path / "other.jsonl")) + stale = TranscriptPageRead(thread="worker", before=page.before) + with pytest.raises(TranscriptChanged) as changed: + stale.read(comms) + token = await approved_token(client, comms) + status, reply = await tool_call( + client, token, 43, stale.declared_name, FieldCodec.encode_fields(stale)) + assert (status, reply["id"]) == (200, 43) + assert reply["result"] == FieldCodec.encode(ToolsCallResult.failed(changed.value)) + assert "structuredContent" not in reply["result"] + + async def test_request_log_names_the_route_and_never_its_values(client, store, root, comms, caplog): """Serve actual GET/POST approval secrets, with aiohttp's raw access log off.""" login = await ClientLogin.register(client) diff --git a/tests/test_deleted_thread_observation.py b/tests/test_deleted_thread_observation.py index 490fa29e19..0abcc884ad 100644 --- a/tests/test_deleted_thread_observation.py +++ b/tests/test_deleted_thread_observation.py @@ -11,12 +11,13 @@ from agent_comms.registry_document import DEFAULT_ROSTER from agent_comms.registry_provenance import DeletedThread from agent_comms.thread_presentation import NoThreadPresentation, ThreadRetired +from agent_comms.notification_reads import LiveNotificationRead from agent_comms.ui_model.observation import ( ObservationProcess, ObserveThreads, ObserveViews, ReadNotifications, ThreadPresented, ThreadsObserved, ViewsRetired, ) from agent_comms.ui_model.status import OpenThreadStatus -from test_observer_death import readable +from observed_queue import ObservedQueue from agent_comms.thread_execution import ExternalThreadExecution from agent_comms.thread_text import GivenThreadText from agent_comms.threads import Thread @@ -80,47 +81,34 @@ async def test_observer_process_survives_deleted_interest_and_keeps_serving(comm declare(comms, 'surviving', 'deleted') original = comms.registry.require('deleted').incarnation names = frozenset({'surviving', 'deleted'}) - with ObservationProcess(comms.route) as process: + delivered = ObservedQueue() + async with ObservationProcess(comms.route) as process: + process.start_receiving(delivered.observed, delivered.failed) process.request(ObserveThreads(names)) process.request(ObserveViews(frozenset({original}), frozenset())) - - async def until(accepted): - async with asyncio.timeout(30): - while True: - await readable(process) - for result in process.results(): - if accepted(result): - return result - - await until(lambda result: isinstance(result, ThreadsObserved) and result.changed == names) + await delivered.until(lambda result: isinstance(result, ThreadsObserved) and result.changed == names) comms.registry.remove('deleted') missing = None retired = None async with asyncio.timeout(30): while missing is None or retired is None: - await readable(process) - for result in process.results(): - if isinstance(result, ThreadsObserved) and 'deleted' in result.changed: - missing = result - elif isinstance(result, ViewsRetired): - retired = result + result = await delivered.next() + if isinstance(result, ThreadsObserved) and 'deleted' in result.changed: + missing = result + elif isinstance(result, ViewsRetired): + retired = result assert missing.activity['deleted'] == DeletedThread('deleted') assert isinstance(missing.results['deleted'], ThreadPresented) assert missing.results['deleted'].presentation == NoThreadPresentation() assert retired.retired.threads == frozenset({original}) surviving = comms.registry.require('surviving') comms.registry.declare(replace(surviving, title=GivenThreadText('Still updating after deletion'))) - updated = await until(lambda result: isinstance(result, ThreadsObserved) - and 'surviving' in result.changed) + updated = await delivered.until(lambda result: isinstance(result, ThreadsObserved) + and 'surviving' in result.changed) assert isinstance(updated.activity['surviving'], ObservedActivity) assert process.process.is_alive() - loop = asyncio.get_running_loop() - loop.add_reader(process.fileno(), process.results) - try: - async with asyncio.timeout(30): - assert await process.read(ReadNotifications(str(comms.root), ())) == {} - finally: - loop.remove_reader(process.fileno()) + async with asyncio.timeout(30): + assert (await process.read(ReadNotifications(str(comms.root), LiveNotificationRead(())))).outcomes == {} def test_one_revision_captures_each_thread_activity_once(comms): diff --git a/tests/test_goal_execution.py b/tests/test_goal_execution.py index 75ce9cfb1a..ada9864a55 100644 --- a/tests/test_goal_execution.py +++ b/tests/test_goal_execution.py @@ -23,6 +23,7 @@ SetGoalAction, ) from agent_comms.goal_failure_observation import FailedTurnEvidence +from agent_comms.goal_history import RecordedGoalHistory from agent_comms.goal_presentation import ( AttemptFailedExecution, GoalExecution, @@ -36,7 +37,7 @@ from agent_comms.owned_turn import OwnedTurn from agent_comms.threads import Thread from agent_comms.tool_caller import ToolCaller -from agent_comms.tools import CommsGoalHistoryTool, GoalHistory +from agent_comms.tools import CommsGoalHistoryTool from agent_comms.turn_failure import NativeChildSignaled from agent_comms.turn_input_source import NoTurnAcceptance @@ -125,7 +126,7 @@ async def test_goal_history_shows_the_ledgers_failed_attempt_beside_active_revis assert recorded.failures[0].turn_id == turn.turn_id other = CommsGoalHistoryTool(thread="worker", goal_id=OneGoal(GoalId.mint())).apply( comms, ToolCaller.from_actor("worker")) - assert other == GoalHistory.empty() + assert other == RecordedGoalHistory.empty() @pytest.mark.asyncio diff --git a/tests/test_historical_notifications.py b/tests/test_historical_notifications.py index 6c18bb27d0..09252c0d2d 100644 --- a/tests/test_historical_notifications.py +++ b/tests/test_historical_notifications.py @@ -490,4 +490,15 @@ def test_live_cli_ack_does_not_become_archived_ack(archived_notification): messages[0].seq, messages[0].message_id ] assert archived.label == "Sent" and not archived.displayed_to + # The root-free recorded operation reads the same archive, not today's live evidence, + # and a key that is no longer attached refuses instead of falling back to live reads. + from agent_comms.coordination_errors import HistoricalSourceChanged + from agent_comms.notification_reads import RecordedNotificationRead + + recorded, = RecordedNotificationRead((messages[0].reference,), source.key).read(live).outcomes[ + messages[0].seq, messages[0].message_id + ] + assert recorded == archived + with pytest.raises(HistoricalSourceChanged): + RecordedNotificationRead((messages[0].reference,), source.key + "-detached").read(live) assert FieldCodec.decode(RecipientActivity, FieldCodec.encode(RecordedRecipientActivity())) == RecordedRecipientActivity() diff --git a/tests/test_historical_views.py b/tests/test_historical_views.py index 84f2c3bc21..9840d8e064 100644 --- a/tests/test_historical_views.py +++ b/tests/test_historical_views.py @@ -270,6 +270,12 @@ def test_recorded_transcript_witness_roundtrip_keeps_original_namespace(tmp_path assert FieldCodec.value_schema(type[TranscriptReadIdentity])["enum"] == list(TranscriptReadIdentity.names()) with pytest.raises(ValueError): FieldCodec.decode(TranscriptReadIdentity, {**FieldCodec.encode(read.identity), "kind": "unregistered"}) + from agent_comms.order_tools import TranscriptPageRead + from test_transcript_pages import shared_page + + recorded = shared_page(live, TranscriptPageRead(thread="alice", historical_source=source.key)) + assert recorded.identity == read.identity + assert any(event.text == "original recorded answer" for event in recorded.page.events) rebound = live.transcripts.bind_page_read("recorded-alice", decoded) assert rebound.current_identity() == decoded assert any(event.text == "original recorded answer" for event in rebound.read().events) diff --git a/tests/test_input_delivery_history.py b/tests/test_input_delivery_history.py index 19c94ada35..e5172282e1 100644 --- a/tests/test_input_delivery_history.py +++ b/tests/test_input_delivery_history.py @@ -1,23 +1,35 @@ """Current owner queues scope notices without altering durable input evidence.""" from agent_comms.repo import Checkout -import io -import json -from contextlib import redirect_stdout +import asyncio from dataclasses import replace -from agent_comms.cli import main +import pytest +from acp.schema import TextContentBlock + +from agent_comms.acp_extension import ( + GoalSnapshotReply, QueuePromptRequest, TurnChangedUpdate, UpdateMetadata, encode_request, +) +from agent_comms.child_process import NoProcess +from agent_comms.errors import RelationViolationError from agent_comms.input_key import AcpInputKey, BusInputKey from agent_comms.comms import wire -from agent_comms.field_codec import GoalId, InputId, TurnId +from agent_comms.field_codec import FieldCodec, GoalId, InputId, TurnId from agent_comms.input_attempt import ( OwnerQueueInputDeliveryScope, UnobservedInputDeliveryScope, UnresolvedInput, ) from agent_comms.input_disposition import InputDispositions +from agent_comms.goal_actions import SetGoalAction +from agent_comms.goals import NoGoal +from agent_comms.order_tools import InputDeliveryRead, OwnerGoalRead from agent_comms.reported_turn import ReportedTurn +from agent_comms.thread_status import RunningThreadStatus, StoppedThreadStatus from agent_comms.threads import Thread +from agent_comms.tool_caller import ToolCaller +from agent_comms.ui_model.observation import ReadOwnerOperation +from delivery_owner_fixture import canonical_delivery_owner from stored_inputs import stored_inputs @@ -103,18 +115,80 @@ def test_history_clear_is_notice_only_and_survives_rename_and_reopen(tmp_path): assert stored_inputs(ledger.root).rows[BusInputKey(7)].native_id == "b" * 32 -def test_commander_reads_a_units_failed_inputs_through_the_tool(tmp_path): - """2026-10-24: the commander hand-wrote SQL against the bus to list pi-mi's lost inputs.""" - comms = wire(tmp_path) - comms.registry.declare(Thread('owner', frozenset(), Checkout(tmp_path))) - seed(comms, 'owner') - output = io.StringIO() - with redirect_stdout(output): - code = main(['--root', str(comms.root), 'tool', '--tool', 'comms_input_delivery', - '--arguments', json.dumps({'thread': 'owner'})]) - assert code == 0, output.getvalue() - overview = json.loads(output.getvalue()) - assert {row['sequence'] for row in overview['inputs']} >= {2, 8} +async def test_mcp_and_the_ui_read_input_delivery_from_the_live_owner_queue(tmp_path): + """2026-10-24: the commander hand-wrote SQL to list pi-mi's lost inputs; the tool then + answered from the connector's own process, where no owner queue exists. Both boundaries + now carry one InputDeliveryRead to the owner and receive the owner's own answer.""" + async with canonical_delivery_owner(tmp_path) as (comms, agent, _, _): + comms.registry.register(comms.registry.require("alpha"), RunningThreadStatus()) + comms.agents.begin_turn("alpha", TurnId.mint(), "Held native fixture") + agent.inputs.state("alpha").open_inbox() + await agent.prompt("alpha", [TextContentBlock(type="text", text="queued")], + field_meta=encode_request(QueuePromptRequest("queued"))) + InputDispositions(comms.root).record( + BusInputKey(91), seq=91, owner="alpha", claim_scope=1, target="alpha", text="Off queue") + owner = comms.registry.require("alpha") + scope = await agent.inputs.delivery_scope("alpha") + assert type(scope) is OwnerQueueInputDeliveryScope + unobserved = comms.goals.input_delivery("alpha") + assert BusInputKey(91) in [row.input_id for row in unobserved.inputs] + answers = {} + for include_history in (False, True): + owned = comms.goals.input_delivery("alpha", include_history=include_history, scope=scope) + async with asyncio.timeout(30): + mcp = await asyncio.to_thread( + InputDeliveryRead.invoke, comms, + {"thread": "alpha", "include_history": include_history}, + caller=ToolCaller.from_actor("sender")) + ui = await ReadOwnerOperation( + str(comms.root), owner.incarnation, InputDeliveryRead(include_history=include_history), + ).answer(comms) + assert mcp == FieldCodec.encode(owned) + assert ui.value == owned + answers[include_history] = owned + assert BusInputKey(91) not in [row.input_id for row in answers[False].inputs] + assert answers[False].historical_inputs == () + assert BusInputKey(91) in [row.input_id for row in answers[True].historical_inputs] + assert replace(answers[True], historical_inputs=()) == answers[False] + comms.registry.register(replace(owner, process_identity=NoProcess()), StoppedThreadStatus()) + with pytest.raises(RelationViolationError): + async with asyncio.timeout(30): + await asyncio.to_thread(InputDeliveryRead.invoke, comms, {"thread": "alpha"}, + caller=ToolCaller.from_actor("sender")) + + +async def test_mcp_and_the_ui_read_the_owner_goal_from_the_live_owner(tmp_path): + """The owner answers comms_owner_goal under its own registration and session: MCP and the + UI's owner envelope receive the owner's own projection, its turn metadata included, for an + empty goal and for a set one.""" + async with canonical_delivery_owner(tmp_path) as (comms, agent, _, _): + comms.registry.register(comms.registry.require("alpha"), RunningThreadStatus()) + comms.agents.begin_turn("alpha", TurnId.mint(), "Held native fixture") + owner = comms.registry.require("alpha") + replies = [] + for change in (lambda: None, + lambda: comms.goals.update_goal("alpha", SetGoalAction(text="Ship the read"))): + change() + owned = GoalSnapshotReply.from_snapshot( + comms.goals.goal_snapshot("alpha"), + UpdateMetadata.of(TurnChangedUpdate(agent.turns.turn_state("alpha")))) + async with asyncio.timeout(30): + mcp = await asyncio.to_thread( + OwnerGoalRead.invoke, comms, {"thread": "alpha"}, + caller=ToolCaller.from_actor("sender")) + ui = await ReadOwnerOperation( + str(comms.root), owner.incarnation, OwnerGoalRead()).answer(comms) + assert mcp == FieldCodec.encode(owned) + assert ui.value == owned + assert ui.value.meta.batch.updates == (TurnChangedUpdate(agent.turns.turn_state("alpha")),) + replies.append(owned) + assert replies[0].goal == NoGoal() + assert replies[1].goal.text == "Ship the read" + comms.registry.register(replace(owner, process_identity=NoProcess()), StoppedThreadStatus()) + with pytest.raises(RelationViolationError): + async with asyncio.timeout(30): + await asyncio.to_thread(OwnerGoalRead.invoke, comms, {"thread": "alpha"}, + caller=ToolCaller.from_actor("sender")) def test_no_owner_queue_observation_never_dismisses_inputs(tmp_path): diff --git a/tests/test_message_notifications.py b/tests/test_message_notifications.py index 23d86badf8..2ea2ed8c64 100644 --- a/tests/test_message_notifications.py +++ b/tests/test_message_notifications.py @@ -70,3 +70,21 @@ def test_individual_view_keeps_alias_goal_and_active_owner_semantics(tmp_path): assert state.active.phase.summary in comms.views.thread_presentations(('beta',)).require('beta').summary comms.agents.finish_turn(lease) assert not comms.views.thread_presentations(('beta',)).require('beta').status.working + + +def test_a_live_batch_beyond_one_window_chunk_keeps_every_message(tmp_path): # noqa: F811 + """The live operation reads every 120-reference chunk into one window; no batch cap.""" + from agent_comms.notification_evidence import NotificationWindow + from agent_comms.notification_reads import LiveNotificationRead + + _path, _root_id, comms, initial, _people = _root(tmp_path) + messages = [initial.message, *( + comms.messaging.send('sender', '#team', f'Batch {number}').message + for number in range(NotificationWindow.window_limit + 5) + )] + window = LiveNotificationRead(tuple(message.reference for message in messages)).read(comms) + assert set(window.outcomes) == {(message.seq, message.message_id) for message in messages} + for message in (messages[0], messages[-1]): + alone = LiveNotificationRead((message.reference,)).read(comms) + key = (message.seq, message.message_id) + assert window.outcomes[key] == alone.outcomes[key] and window.outcomes[key] diff --git a/tests/test_notification_read_state.py b/tests/test_notification_read_state.py index f9f1aa477e..8e1bca0b2b 100644 --- a/tests/test_notification_read_state.py +++ b/tests/test_notification_read_state.py @@ -1,26 +1,53 @@ +import pickle import sys from dataclasses import replace from pathlib import Path +import pytest + +from agent_comms.bus_read import BusRead, UiRead from agent_comms.comms import Comms +from agent_comms.coordination_errors import StaleRevision +from agent_comms.field_codec import FieldCodec, MessageId from agent_comms.history_views import HistoryViews +from agent_comms.message_reference import MessageReference +from agent_comms.notification_read import NotificationsRead, UnreadableNotificationsRead +from agent_comms.notification_reads import ( + LiveNotificationRead, NotificationRead, RecordedNotificationRead, +) +from agent_comms.order_tools import ( + CommanderUnitRead, InputDeliveryRead, OwnerGoalRead, TranscriptPageRead, +) from agent_comms.private_path import FileRevision +from agent_comms.registry_provenance import DeletedThread +from agent_comms.repo import Checkout from agent_comms.presentation import NoRevision, WireRevision from agent_comms.revision_views import ( ActivityView, NotificationEvidenceView, NotificationsView, RegistrationsView, SidebarView, StatusView, StoresView, ) +from agent_comms.thread_identity import AgentThreadRole, UserThreadRole +from agent_comms.threads import Thread +from agent_comms.tool_declaration import NativeRequestTools +from agent_comms.tool_request import ToolRequest +from agent_comms.tools import CommsMessageOutcomesTool +from agent_comms.acp_extension import GoalSnapshotReply, TranscriptSnapshotUpdate +from agent_comms.bus_read import OwnerLiveRead +from agent_comms.commander_hud import CommanderUnitDetail +from agent_comms.field_codec import OrderId +from agent_comms.thread_identity import RecordedThreadIncarnation +from agent_comms.transcripts import NativeTranscriptCursor from agent_comms.ui_model.observation import ( - NotificationsRead, ObserveNotifications, ReadNotifications, ReadRecordedNotifications, - UnreadableNotificationsRead, + ObserveNotifications, ReadNotifications, ReadOperation, ReadOwnerOperation, + ReadTranscript, ReadTranscriptUpdate, RefreshTranscript, SubscribeTranscript, ) def test_notification_read_states_use_the_request_revision_contract(comms): original = WireRevision.empty() later = replace(original, expiry_tick=original.expiry_tick + 60) - live = ReadNotifications("root", ()) - recorded = ReadRecordedNotifications("root", (), "archive") + live = LiveNotificationRead(()) + recorded = RecordedNotificationRead((), "archive") settled = NotificationsRead(original, {}) assert not live.changed_since(comms, original, settled) assert not live.changed_since(comms, later, settled) @@ -29,13 +56,71 @@ def test_notification_read_states_use_the_request_revision_contract(comms): assert not live.changed_since(comms, original, UnreadableNotificationsRead(original)) +TEAM = (("sender", AgentThreadRole), ("worker", AgentThreadRole), ("human", UserThreadRole), + ("archived", AgentThreadRole), ("gone", AgentThreadRole)) + + +def declare_team(comms: Comms, tmp_path: Path) -> MessageReference: + """One real #team message from an agent sender, seen by a human and by three native + agent recipients; one of those is archived or deleted afterwards. The window names only + native recipients, so the sender and the human are never watched.""" + comms.channels.create_tag("team") + for name, role in TEAM: + comms.registry.declare(Thread(name, frozenset({"team"}), Checkout(tmp_path), role=role)) + return comms.messaging.send("sender", "#team", "Who is still here?").message.reference + + +def settled_read(comms: Comms, reference: MessageReference) -> tuple[WireRevision, NotificationsRead]: + revision = comms.views.revision() + window = LiveNotificationRead((reference,)).read(comms) + return revision, NotificationsRead.of(revision, comms, {}, window) + + +def test_the_window_names_native_recipients_and_activity_holds_only_projected_ones(comms, tmp_path): + reference = declare_team(comms, tmp_path) + comms.registry.archive("archived") + comms.registry.remove("gone") + revision, settled = settled_read(comms, reference) + + names = settled.notification_window.activity_names() + assert names == {"worker", "archived", "gone"} + assert not names & {"sender", "human"} + assert settled.activity.keys() == names - {"archived"} + assert settled.activity["gone"] == DeletedThread("gone") + assert settled.activity["worker"] == comms.views.observed_activity(revision, ("worker",))["worker"] + assert not settled.live_changed(comms, revision) + + +def test_an_archived_participant_becoming_visible_changes_the_read(comms, tmp_path): + reference = declare_team(comms, tmp_path) + comms.registry.archive("archived") + revision, settled = settled_read(comms, reference) + assert "archived" not in settled.activity + + comms.registry.declare(Thread("archived", frozenset({"team"}), Checkout(tmp_path))) + later = comms.views.revision() + assert later.changed_since(ActivityView(), revision) + assert settled.live_changed(comms, later) + + +def test_a_participant_deleted_after_the_read_changes_it(comms, tmp_path): + reference = declare_team(comms, tmp_path) + revision, settled = settled_read(comms, reference) + assert "gone" in settled.activity and settled.activity["gone"] != DeletedThread("gone") + + comms.registry.remove("gone") + assert settled.live_changed(comms, comms.views.revision()) + + def test_notification_read_only_acquires_activity_when_its_source_changed(comms, tmp_path): + reference = declare_team(comms, tmp_path) + comms.registry.archive("archived") + original, settled = settled_read(comms, reference) + assert settled.notification_window.activity_names() - settled.activity.keys() == {"archived"} marker = tmp_path / 'changed-source' marker.write_text('changed') source = FileRevision.capture(marker).revisions() - original = WireRevision.empty() assert replace(original, history=source).changed_since(NotificationsView(), original) - settled = NotificationsRead(original, {'absent-participant': None}) calls = 0 def count_reads(frame, event, argument): @@ -48,7 +133,7 @@ def count_reads(frame, event, argument): try: assert not settled.live_changed(comms, replace(original, reads=source)) assert calls == 0 - assert settled.live_changed(comms, replace(original, activity=source)) + assert not settled.live_changed(comms, replace(original, activity=source)) assert calls == 1 finally: sys.setprofile(previous) @@ -66,9 +151,9 @@ def test_unrelated_registry_change_keeps_the_notification_window(comms: Comms, t def test_changed_notification_interest_keeps_only_retained_read_states(): - kept = ReadNotifications('kept', ()) - removed = ReadNotifications('removed', ()) - added = ReadNotifications('added', ()) + kept = ReadNotifications('kept', LiveNotificationRead(())) + removed = ReadNotifications('removed', LiveNotificationRead(())) + added = ReadNotifications('added', LiveNotificationRead(())) previous = ObserveNotifications(frozenset((kept, removed))).observation() state = NotificationsRead(WireRevision.empty()) previous.read_states = {kept: state, removed: state} @@ -80,6 +165,78 @@ def test_changed_notification_interest_keeps_only_retained_read_states(): assert requested.read_states == {kept: state} +REFERENCES = (MessageReference(1, MessageId.mint("original")),) + + +@pytest.mark.parametrize("operation", [ + LiveNotificationRead(REFERENCES), RecordedNotificationRead(REFERENCES, "archive"), +], ids=["live", "recorded"]) +def test_notification_operations_are_root_free_family_records(operation): + encoded = FieldCodec.encode(operation) + assert "root" not in encoded and "request_id" not in encoded + assert FieldCodec.decode(NotificationRead, encoded) == operation + assert FieldCodec.decode(UiRead, encoded) == operation + request = ReadNotifications("root", operation, request_id=4) + assert FieldCodec.decode(ReadNotifications, FieldCodec.encode(request)) == request + transported = pickle.loads(pickle.dumps(request)) + assert transported == request and hash(transported) == hash(request) + assert {transported: "held"}[request] == "held" and len({request, transported}) == 1 + + +def test_root_request_references_and_archive_each_distinguish_a_notification_request(): + other_reference = (MessageReference(2, MessageId.mint("other")),) + request = ReadNotifications("root", RecordedNotificationRead(REFERENCES, "archive"), request_id=4) + distinct = ( + replace(request, root="other-root"), + replace(request, request_id=5), + replace(request, operation=RecordedNotificationRead(other_reference, "archive")), + replace(request, operation=RecordedNotificationRead(REFERENCES, "other-archive")), + replace(request, operation=LiveNotificationRead(REFERENCES)), + ) + assert len({request, *distinct}) == 1 + len(distinct) + + +def test_equal_requests_keep_their_read_state_and_a_new_archive_drops_it(): + kept = ReadNotifications("root", RecordedNotificationRead(REFERENCES, "archive")) + replaced = ReadNotifications("root", RecordedNotificationRead(REFERENCES, "detached")) + reattached = ReadNotifications("root", RecordedNotificationRead(REFERENCES, "attached")) + previous = ObserveNotifications(frozenset((kept, replaced))).observation() + state = NotificationsRead(WireRevision.empty()) + previous.read_states = {kept: state, replaced: state} + requested = ObserveNotifications(frozenset(( + ReadNotifications("root", RecordedNotificationRead(REFERENCES, "archive")), reattached, + ))).observation() + + assert previous.for_interest(requested) is requested + assert requested.read_states == {kept: state} + assert requested.read_states[kept] is state + + +def test_one_operation_in_two_root_envelopes_keeps_root_validation(comms): + operation = LiveNotificationRead(REFERENCES) + ours = ReadNotifications(str(comms.root), operation) + foreign = ReadNotifications(str(comms.root.parent / "another-wire"), operation) + assert ours.operation is foreign.operation and ours != foreign + ours.require_root(comms) + with pytest.raises(StaleRevision): + foreign.require_root(comms) + + +def test_notification_operations_are_ui_reads_and_not_tools(): + assert BusRead.members_with(NotificationRead) == (LiveNotificationRead, RecordedNotificationRead) + assert set(BusRead.members_with(NotificationRead)) <= set(BusRead.members_with(UiRead)) + tools = (*ToolRequest.members_with(ToolRequest), *NativeRequestTools.members_with(NativeRequestTools)) + assert not any(issubclass(tool, NotificationRead) for tool in tools) + assert CommsMessageOutcomesTool in ToolRequest.members_with(ToolRequest) + assert CommsMessageOutcomesTool in NativeRequestTools.members_with(NativeRequestTools) + + +def test_a_notification_request_refuses_an_operation_outside_the_notification_family(): + encoded = FieldCodec.encode(ReadNotifications("root", LiveNotificationRead(REFERENCES))) + with pytest.raises(ValueError, match="is not a member of NotificationRead"): + FieldCodec.decode(ReadNotifications, {**encoded, "operation": FieldCodec.encode(InputDeliveryRead())}) + + def test_activity_views_receive_expiry_without_expiring_store_or_evidence_views(): original = WireRevision.empty() later = replace(original, expiry_tick=60) @@ -113,3 +270,78 @@ def test_notification_rank_overrides_follow_the_mro_without_changing_parent_rank assert ShownToModelProgress(100).rank() == DeliveryRank.position_of(SettledDelivery) assert SendingProgress(100).rank() == DeliveryRank.position_of(ActiveDelivery) assert SelectedConsumedProgress(100).rank() == DeliveryRank.position_of(ActiveDelivery) + + +LEAD, WORKER = RecordedThreadIncarnation("lead", 1.0), RecordedThreadIncarnation("worker", 2.0) +ORDERS = (OrderId.mint(),) +UI_ONLY_FIELDS = {"root", "request_id", "previous", "owner", "identity"} +SHARED_UI_READS = [ + CommanderUnitRead(commander=LEAD, unit=WORKER, orders=ORDERS), + TranscriptPageRead(thread="worker", before=NativeTranscriptCursor("session.jsonl", 7), + historical_source="archive"), +] + + +@pytest.mark.parametrize("operation", SHARED_UI_READS, ids=["commander_unit", "transcript_page"]) +def test_shared_ui_reads_are_root_free_family_records_in_one_generic_envelope(operation): + encoded = FieldCodec.encode(operation) + assert not UI_ONLY_FIELDS & set(encoded) + assert FieldCodec.decode(UiRead, encoded) == operation + request = ReadOperation("root", operation, request_id=4) + assert FieldCodec.decode(ReadOperation, FieldCodec.encode(request)) == request + transported = pickle.loads(pickle.dumps(request)) + assert transported == request and hash(transported) == hash(request) + assert {transported: "held"}[request] == "held" and len({request, transported}) == 1 + + +def test_the_owner_goal_is_a_root_free_owner_live_record_in_its_envelope(): + operation = OwnerGoalRead(thread="worker") + encoded = FieldCodec.encode(operation) + assert not UI_ONLY_FIELDS & set(encoded) + assert FieldCodec.decode(OwnerLiveRead, encoded) == operation + request = ReadOwnerOperation("root", WORKER, operation=operation, request_id=4) + transported = pickle.loads(pickle.dumps(request)) + assert transported == request and hash(transported) == hash(request) + + +def test_witnesses_cursors_and_archive_each_distinguish_a_shared_read_request(): + commander = ReadOperation("root", SHARED_UI_READS[0], request_id=4) + page = ReadOperation("root", SHARED_UI_READS[1], request_id=4) + distinct = ( + replace(commander, root="other-root"), + replace(commander, request_id=5), + replace(commander, operation=replace(SHARED_UI_READS[0], unit=RecordedThreadIncarnation("worker", 3.0))), + replace(commander, operation=replace(SHARED_UI_READS[0], orders=(OrderId.mint(),))), + page, + replace(page, operation=replace(SHARED_UI_READS[1], before=NativeTranscriptCursor("session.jsonl", 8))), + replace(page, operation=replace(SHARED_UI_READS[1], historical_source="other-archive")), + ) + assert len({commander, *distinct}) == 1 + len(distinct) + + +def test_one_shared_read_in_two_root_envelopes_keeps_root_validation(comms): + operation = SHARED_UI_READS[1] + ours = ReadOperation(str(comms.root), operation) + foreign = ReadOperation(str(comms.root.parent / "another-wire"), operation) + assert ours.operation is foreign.operation and ours != foreign + ours.require_root(comms) + with pytest.raises(StaleRevision): + foreign.require_root(comms) + + +def test_the_three_public_reads_are_cataloged_once_with_their_original_results(): + sys.path.insert(0, str(Path(__file__).parent)) + from test_tool_replies import result_type + + tools = ToolRequest.members_with(ToolRequest) + native = NativeRequestTools.members_with(NativeRequestTools) + results = {CommanderUnitRead: CommanderUnitDetail, OwnerGoalRead: GoalSnapshotReply, + TranscriptPageRead: TranscriptSnapshotUpdate} + for operation, result in results.items(): + assert tools.count(operation) == 1 and native.count(operation) == 1 + assert result_type(operation) is result + assert set(ToolRequest.members_with(UiRead)) == {CommanderUnitRead, TranscriptPageRead} + assert set(ToolRequest.members_with(OwnerLiveRead)) == {InputDeliveryRead, OwnerGoalRead} + lifecycle = (ReadOperation, ReadOwnerOperation, ReadNotifications, ReadTranscript, + SubscribeTranscript, ReadTranscriptUpdate, RefreshTranscript) + assert not set(lifecycle) & {*tools, *native} diff --git a/tests/test_observer_death.py b/tests/test_observer_death.py index 78379f87c3..2d90709887 100644 --- a/tests/test_observer_death.py +++ b/tests/test_observer_death.py @@ -4,6 +4,7 @@ from agent_comms.repo import Checkout import asyncio import os +from functools import partial import signal import pytest @@ -15,62 +16,68 @@ from agent_comms.thread_identity import RecordedThreadIncarnation from agent_comms.owner_connection import OwnerIdentityChangedError from agent_comms.message_reference import MessageReference +from agent_comms.bus_read import BusRead, OwnerLiveRead +from agent_comms.notification_reads import LiveNotificationRead from agent_comms.ui_model.observation import ( - ObservationProcess, ObserverDied, OwnerRead, ReadNotifications, StopSidebar, + ObservationProcess, ObserverDied, ReadNotifications, ReadOwnerOperation, StopSidebar, ) +from agent_comms.thread_execution import ExternalThreadExecution from agent_comms.threads import Thread +from owner_socket_fixture import declare_owner -async def readable(process: ObservationProcess) -> None: - """Wait until the observer's pipe has something to read, as the UI's event loop does.""" - ready = asyncio.Event() - loop = asyncio.get_running_loop() - loop.add_reader(process.fileno(), ready.set) - try: - async with asyncio.timeout(30): - await ready.wait() - finally: - loop.remove_reader(process.fileno()) +def owner_reads(): + """Every owner read the UI can send: each owner-live operation in its one envelope.""" + for operation in BusRead.members_with(OwnerLiveRead): + yield pytest.param(partial(ReadOwnerOperation, operation=operation()), id=operation.declared_name) + + +def empty_window(root) -> ReadNotifications: + return ReadNotifications(str(root), LiveNotificationRead(())) async def test_a_killed_observer_is_reported_once_with_its_signal_and_fails_awaited_reads(comms, durable_home): - process = ObservationProcess(comms.route) - try: - await readable(process) - assert process.results() # the first observation: the service is running - os.kill(process.process.pid, signal.SIGSTOP) # the read below is still unanswered when it dies - waiting = asyncio.ensure_future(process.read(ReadNotifications(str(comms.root), ()))) - await asyncio.sleep(0) - os.kill(process.process.pid, signal.SIGKILL) - with pytest.raises(ObserverDied) as died: - while True: - await readable(process) - process.results() - assert died.value.pid == process.process.pid and died.value.exit_status == -signal.SIGKILL - assert f"pid {process.process.pid}" in str(died.value) and "signal 9" in str(died.value) - with pytest.raises(ObserverDied) as awaited: - await waiting - assert awaited.value is died.value + async with ObservationProcess.awaited(comms.route) as process: + async with asyncio.timeout(30): + assert (await process.read(empty_window(comms.root))).outcomes == {} # the service runs + child = process.process.pid + os.kill(child, signal.SIGSTOP) # the read below stays unanswered until the child dies + waiting = asyncio.ensure_future(process.read(empty_window(comms.root))) + try: + async with asyncio.timeout(30): + _, stopped = await asyncio.to_thread(os.waitpid, child, os.WUNTRACED) + assert os.WIFSTOPPED(stopped) + await asyncio.sleep(0) + assert len(process.reads) == 1 and not waiting.done() + os.kill(child, signal.SIGKILL) + with pytest.raises(ObserverDied) as died: + async with asyncio.timeout(30): + await waiting + finally: + if process.process.exitcode is None: + os.kill(child, signal.SIGKILL) + waiting.cancel() + await asyncio.gather(waiting, return_exceptions=True) + assert died.value.pid == child and died.value.exit_status == -signal.SIGKILL + assert f"pid {child}" in str(died.value) and "signal 9" in str(died.value) with pytest.raises(ObserverDied) as stopping: process.request(StopSidebar()) # what a closing view sends assert stopping.value is died.value with pytest.raises(ObserverDied) as later: - await process.read(ReadNotifications(str(comms.root), ())) + await process.read(empty_window(comms.root)) assert later.value is died.value - finally: - process.close() async def test_an_invalid_notification_reference_does_not_kill_the_observer(comms, durable_home): - with ObservationProcess.awaited(comms.route) as process: + async with ObservationProcess.awaited(comms.route) as process: with pytest.raises(RelationViolationError): - await process.read(ReadNotifications( - str(comms.root), (MessageReference(1, MessageId.mint("not-the-original")),) - )) - assert await process.read(ReadNotifications(str(comms.root), ())) == {} + await process.read(ReadNotifications(str(comms.root), LiveNotificationRead( + (MessageReference(1, MessageId.mint("not-the-original")),) + ))) + assert (await process.read(empty_window(comms.root))).outcomes == {} -@pytest.mark.parametrize("read_type", OwnerRead.__subclasses__()) +@pytest.mark.parametrize("read_type", owner_reads()) @pytest.mark.parametrize("declared, refusal", [ (True, RelationViolationError), (False, UnregisteredThreadError), ]) @@ -80,25 +87,41 @@ async def test_unavailable_owner_reads_preserve_the_observer( owner = Thread("read-subject", frozenset(), Checkout(comms.root.parent)) if declared: comms.registry.declare(owner) - with ObservationProcess.awaited(comms.route) as process: + async with ObservationProcess.awaited(comms.route) as process: with pytest.raises(refusal): await process.read(read_type(str(comms.root), owner.incarnation)) - assert await process.read(ReadNotifications(str(comms.root), ())) == {} + assert (await process.read(empty_window(comms.root))).outcomes == {} assert process.process.is_alive() process.status.raise_if_failed() -@pytest.mark.parametrize("read_type", OwnerRead.__subclasses__()) +@pytest.mark.parametrize("read_type", owner_reads()) @pytest.mark.parametrize("other_root", [False, True]) async def test_stale_owner_reads_preserve_the_observer(comms, durable_home, read_type, other_root): owner = Thread("read-subject", frozenset(), Checkout(comms.root.parent)) comms.registry.declare(owner) root = str(comms.root.parent / "another-wire") if other_root else str(comms.root) identity = owner.incarnation if other_root else RecordedThreadIncarnation(owner.name, 1.0) - with ObservationProcess.awaited(comms.route) as process: + async with ObservationProcess.awaited(comms.route) as process: with pytest.raises(StaleRevision if other_root else OwnerIdentityChangedError): await process.read(read_type(root, identity)) - assert await process.read(ReadNotifications(str(comms.root), ())) == {} + assert (await process.read(empty_window(comms.root))).outcomes == {} + assert process.process.is_alive() + process.status.raise_if_failed() + + +@pytest.mark.parametrize("read_type", owner_reads()) +async def test_external_owner_reads_refuse_before_waiting_for_a_socket( + comms, durable_home, runtime_processes, read_type, +): + """An external bridge has a live process and no runtime socket: the read refuses with the + original error instead of waiting for a socket that will never be served.""" + owner = declare_owner(comms, "external-owner", runtime_processes[0].identity, ExternalThreadExecution) + async with ObservationProcess.awaited(comms.route) as process: + with pytest.raises(RelationViolationError): + async with asyncio.timeout(30): + await process.read(read_type(str(comms.root), owner.incarnation)) + assert (await process.read(empty_window(comms.root))).outcomes == {} assert process.process.is_alive() process.status.raise_if_failed() @@ -106,14 +129,10 @@ async def test_stale_owner_reads_preserve_the_observer(comms, durable_home, read async def test_an_observer_that_failed_is_reported_with_its_own_traceback_and_exit_status(tmp_path, durable_home): not_a_wire = tmp_path / "not-a-wire" not_a_wire.write_text("") - process = ObservationProcess(LocalRoute(not_a_wire)) - try: + async with ObservationProcess.awaited(LocalRoute(not_a_wire)) as process: with pytest.raises(ObserverDied) as died: - while True: - await readable(process) - process.results() - assert died.value.exit_status == 1 - assert "Traceback" in died.value.error and "serve" in died.value.error - assert "exited with status 1" in str(died.value) and died.value.error in str(died.value) - finally: - process.close() + async with asyncio.timeout(30): + await process.read(empty_window(not_a_wire)) + assert died.value.exit_status == 1 + assert "Traceback" in died.value.error and "serve" in died.value.error + assert "exited with status 1" in str(died.value) and died.value.error in str(died.value) diff --git a/tests/test_outbound_transcript_source.py b/tests/test_outbound_transcript_source.py index 83dec2c377..4dba1a4890 100644 --- a/tests/test_outbound_transcript_source.py +++ b/tests/test_outbound_transcript_source.py @@ -1,10 +1,11 @@ """Whole conversations derive their original frozen wire sources, in both directions.""" -from agent_comms.presentation import AllOutcomes +from agent_comms.presentation import AllOutcomes, OwnerOutcomes from agent_comms.repo import Checkout from agent_comms.channel_targets import CommandTarget import sqlite3 +import sys from dataclasses import replace import pytest @@ -33,21 +34,21 @@ def test_original_read_contention_is_unavailable_then_same_receipts_return(tmp_p root, _root_id, comms, initial, _people = _root(tmp_path) database = root / "coordination.sqlite3" - captured = NotificationAssignment.read(root, (initial,)) + captured = NotificationAssignment.read(root, (initial,), viewer=AllOutcomes()) original = NotificationAssignment.resolve(root, captured) wire_before = comms.bus.bus.revision() assert original with Coordination(str(database)) as writer: with writer.session.writing(): with pytest.raises(CoordinationReadUnavailable) as unavailable: - NotificationAssignment.read(root, (initial,)) + NotificationAssignment.read(root, (initial,), viewer=AllOutcomes()) assert unavailable.value.__cause__.sqlite_errorcode & 0xFF in ( sqlite3.SQLITE_BUSY, sqlite3.SQLITE_LOCKED, ) with pytest.raises(CoordinationReadUnavailable): with TranscriptRead.publication_source(root): pytest.fail("Busy acquisition reached the native schema decoder") - reopened = NotificationAssignment.read(root, (initial,)) + reopened = NotificationAssignment.read(root, (initial,), viewer=AllOutcomes()) assert reopened == captured assert NotificationAssignment.resolve(root, reopened) == original with TranscriptRead.publication_source(root) as native_read: @@ -55,6 +56,85 @@ def test_original_read_contention_is_unavailable_then_same_receipts_return(tmp_p assert comms.bus.bus.revision() == wire_before +def test_a_read_selects_only_its_viewers_claims_and_attempts_before_decoding(tmp_path): # noqa: F811 + from agent_comms.notification_assignment import AssignmentActivityEvidence + + root, _root_id, comms, initial, people = _root(tmp_path) + lookups = {person.name: person.incarnation.lookup for person in people} + reference = initial.message.reference + everyone = NotificationAssignment.read(root, (initial,), viewer=AllOutcomes()) + recipients = {recipient.canonical_thread: recipient.recipient_lookup + for recipient in initial.recipients} + assert len(recipients) >= 2 and "sender" not in recipients + assert {claim.recipient for claim, _evidence in everyone.rows} == recipients.keys() + decoded = 0 + decode = AssignmentActivityEvidence._decode_row.__func__.__code__ + + def count_decodes(frame, event, argument): + nonlocal decoded + if event == "call" and frame.f_code is decode: + decoded += 1 + + for name, lookup in recipients.items(): + viewer = OwnerOutcomes(lookup) + decoded = 0 + profiling = sys.getprofile() + sys.setprofile(count_decodes) + try: + own = NotificationAssignment.read(root, (initial,), viewer=viewer) + finally: + sys.setprofile(profiling) + assert decoded == 1 + assert own.viewer == viewer and own.sources == (initial,) + assert own.rows == tuple(row for row in everyone.rows if row[0].recipient == name) + assert own.evidence[reference].claims == own.rows + assert own.evidence[reference].attempts == { + recipient: attempts for recipient, attempts in everyone.evidence[reference].attempts.items() + if recipient == lookup} + sender = NotificationAssignment.read(root, (initial,), viewer=OwnerOutcomes(lookups["sender"])) + assert sender.rows == everyone.rows + assert sender.evidence == everyone.evidence + + +def test_invisible_recipients_allocate_no_placeholder_source(tmp_path): # noqa: F811 + root, _root_id, _comms, initial, people = _root(tmp_path) + lookups = {person.name: person.incarnation.lookup for person in people} + recipient = initial.recipients[0] + (source, outcomes), = NotificationAssignment.for_deliveries( + (initial,), (), OwnerOutcomes(recipient.recipient_lookup)) + assert source is initial + assert [outcome.recipient() for outcome in outcomes] == [recipient] + (_source, all_outcomes), = NotificationAssignment.for_deliveries( + (initial,), (), OwnerOutcomes(lookups["sender"])) + assert [outcome.recipient() for outcome in all_outcomes] == list(initial.recipients) + + +def test_retained_reads_refresh_only_for_changes_their_viewer_sees(tmp_path): # noqa: F811 + from agent_comms.assignment_states import DeferredAssignment + from agent_comms.coordination_cohort import sealed_cohort_assignments + + root, _root_id, _comms, initial, people = _root(tmp_path) + lookups = {person.name: person.incarnation.lookup for person in people} + changed, unchanged = initial.recipients[:2] + reads = { + "changed": NotificationAssignment.read( + root, (initial,), viewer=OwnerOutcomes(changed.recipient_lookup)), + "foreign": NotificationAssignment.read( + root, (initial,), viewer=OwnerOutcomes(unchanged.recipient_lookup)), + "sender": NotificationAssignment.read( + root, (initial,), viewer=OwnerOutcomes(lookups["sender"])), + "operator": NotificationAssignment.read(root, (initial,), viewer=AllOutcomes()), + } + assert NotificationAssignment.changed(reads.values(), root) == () + with Coordination(str(root / "coordination.sqlite3")) as store: + assignment, = sealed_cohort_assignments(store, changed.recipient_lookup) + store.assignments.transition_preengagement( + assignment.assignment_id, DeferredAssignment, expected_revision=assignment.revision) + refreshed = NotificationAssignment.changed(reads.values(), root) + assert {name for name, read in reads.items() if read in refreshed} == { + "changed", "sender", "operator"} + + @pytest.mark.parametrize('direct', [False, True]) def test_saved_notification_does_not_mix_owner_drain_readiness(tmp_path, direct): # noqa: F811 from agent_comms.activity import StoppedDrainDiagnostic @@ -128,15 +208,15 @@ def test_capture_page_window_names_zero_and_nonzero_sent_windows(tmp_path): # n def test_notification_window_preserves_exact_receipts_and_rejects_duplicates(tmp_path): # noqa: F811 root, _root_id, comms, initial, _people = _root(tmp_path) - records = NotificationAssignment.resolve(root, NotificationAssignment.read(root, (initial,))) + records = NotificationAssignment.resolve(root, NotificationAssignment.read(root, (initial,), viewer=AllOutcomes())) assert records - projected = tuple(NotificationAssignment.for_deliveries((initial,), records)) + projected = tuple(NotificationAssignment.for_deliveries((initial,), records, AllOutcomes())) assert projected[0][0] is initial assert any(outcome is records[0] for outcome in projected[0][1]) with pytest.raises(ValueError): - tuple(NotificationAssignment.for_deliveries((initial,), (*records, records[0]))) + tuple(NotificationAssignment.for_deliveries((initial,), (*records, records[0]), AllOutcomes())) # Duplicate evidences outside the acquired deliveries do not change their result. - assert tuple(NotificationAssignment.for_deliveries((), (*records, records[0]))) == () + assert tuple(NotificationAssignment.for_deliveries((), (*records, records[0]), AllOutcomes())) == () def test_publication_intent_joins_original_sender_and_target_only(tmp_path): # noqa: F811 @@ -257,3 +337,77 @@ def test_missing_bus_cannot_be_an_empty_presentation(tmp_path): # noqa: F811 with pytest.raises(sqlite3.OperationalError) as missing: comms.views.thread_presentations(("beta",)).require("beta") assert missing.value.sqlite_errorcode == sqlite3.SQLITE_CANTOPEN + + +def test_a_forced_key_batch_captures_the_same_evidence_through_the_recipient_index(tmp_path): # noqa: F811 + from conftest import accept_initial, publish_initial + from agent_comms.coordination_tables.assignments import WakeAssignment + from agent_comms.native_runtime_input import NativeRuntimeTable + from agent_comms.notification_evidence import NotificationWindow + from agent_comms.registry_store import RegistryStore + + root, root_id, comms, initial, _people = _root(tmp_path) + database = root / "coordination.sqlite3" + privacy = RegistryStore.at(root).privacy() + # The lowered limit still admits capture's own schema validation: each catalog read + # binds every declared object name and every table name of its schema. + limit = max(6, *( + len({name for row in schema.schema_rows() for name in row.schema_names}) + + len(schema.schema_rows()) + for schema in (privacy.coordinator_schema(), NativeRuntimeTable))) + key_fields = 3 + originals = limit // (key_fields * len(initial.recipients)) + 1 + assert originals + 1 <= NotificationWindow.window_limit + sources = [initial] + for index in range(originals - 1): + later = publish_initial(comms, "sender", "#team", f"Broadcast {index} for the window") + with Coordination(str(database)) as store: + accept_initial(comms.bus, root_id, later.seq, store) + sources.append(comms.bus.bus.delivery(later.seq)) + sources = (*sources, initial) + keys = tuple({"recipient_lookup": recipient.recipient_lookup, "wire_seq": source.message.seq, + "message_id": source.message.message_id} + for source in sources for recipient in source.recipients) + distinct = tuple(dict.fromkeys(tuple(key.values()) for key in keys)) + assert key_fields * len(distinct) > limit and len(keys) > len(distinct) + ordinary = NotificationAssignment.read(root, sources, viewer=AllOutcomes()) + assert ordinary.rows + statements: list[str] = [] + with Coordination(str(database)) as store, store.session.read(): + connection = store.session._connection + unlimited, = WakeAssignment.requested_keys(connection, keys) + previous = connection.setlimit(sqlite3.SQLITE_LIMIT_VARIABLE_NUMBER, limit) + assert limit < previous + connection.set_trace_callback(statements.append) + try: + bound = connection.getlimit(sqlite3.SQLITE_LIMIT_VARIABLE_NUMBER) + batches = tuple(WakeAssignment.requested_keys(connection, keys)) + limited = NotificationAssignment.capture(connection, sources, privacy, AllOutcomes()) + finally: + connection.set_trace_callback(None) + connection.setlimit(sqlite3.SQLITE_LIMIT_VARIABLE_NUMBER, previous) + index, = (name for _seq, name, unique, *_rest in connection.execute( + f'PRAGMA index_list("{WakeAssignment.declared_name}")') + if unique and [column for _rank, _cid, column in connection.execute( + f'PRAGMA index_info("{name}")')] == ["recipient_lookup", "wire_seq"]) + claim_statements = [statement for statement in statements + if statement.startswith("WITH requested") and "w AS MATERIALIZED" in statement] + plans = [[row[3] for row in connection.execute(f"EXPLAIN QUERY PLAN {statement}")] + for statement in claim_statements] + + assert len(batches) > 1 + assert all(len(batch.bindings) <= bound for batch in batches) + assert sum(len(batch.bindings) for batch in batches) == key_fields * len(distinct) + assert tuple(value for batch in batches for value in batch.bindings) == unlimited.bindings + assert limited.viewer == AllOutcomes() and limited.sources == sources + assert limited.rows == ordinary.rows + assert limited.attempts == ordinary.attempts + assert limited.evidence == ordinary.evidence + assert len({claim.assignment_id for claim, _evidence in limited.rows}) == len(limited.rows) + order = [(-claim.wire_seq, claim.recipient) for claim, _evidence in limited.rows] + assert order == sorted(order) + assert len(claim_statements) >= 2 + for plan in plans: + assert any(detail.startswith(f"SEARCH {WakeAssignment.declared_name} USING INDEX {index}") + for detail in plan), plan + assert not any(detail.startswith(f"SCAN {WakeAssignment.declared_name}") for detail in plan), plan diff --git a/tests/test_recipient_activity_index.py b/tests/test_recipient_activity_index.py index 6d35c16edb..f187780c34 100644 --- a/tests/test_recipient_activity_index.py +++ b/tests/test_recipient_activity_index.py @@ -88,6 +88,59 @@ def test_viewer_selects_the_outcomes_of_a_delivery_window(tmp_path, viewer_of, s for name, thread in comms.registry.snapshot().threads.items()} window = MessageNotification.delivery_window( comms.root, comms.registry, (original,), - NotificationAssignment.read(comms.root, (original,)), viewer=viewer_of(lookups), + NotificationAssignment.read(comms.root, (original,), viewer=viewer_of(lookups)), ) assert {item.recipient for item in window.outcomes[(sent.seq, sent.message_id)]} == seen + + +def test_a_viewer_follows_frozen_lookups_through_rename_and_never_into_a_rebirth(tmp_path): + comms = canonical_goal_wire(tmp_path / "bus") + for name in ("sender", "peer-a", "peer-b"): + comms.registry.declare(Thread(name, frozenset({"ci"}), Checkout(tmp_path))) + sent = comms.messaging.send("sender", "#ci", "channel outcome").message + original = comms.bus.bus.delivery(sent.seq) + key = (sent.seq, sent.message_id) + + def seen_by(name: str) -> set[str]: + viewer = OwnerOutcomes.of(comms.registry.require(name)) + read = NotificationAssignment.read(comms.root, (original,), viewer=viewer) + assert read.viewer == viewer + window = MessageNotification.delivery_window(comms.root, comms.registry, (original,), read) + return {item.recipient for item in window.outcomes[key]} + + assert seen_by("peer-a") == {"peer-a"} + comms.registry.rename("peer-a", "renamed-a") + assert seen_by("renamed-a") == {"peer-a"} + comms.registry.remove("peer-b") + comms.registry.declare(Thread("peer-b", frozenset({"ci"}), Checkout(tmp_path))) + assert seen_by("peer-b") == set() + assert seen_by("sender") == {"peer-a", "peer-b"} + + +def test_a_public_root_without_a_coordinator_keeps_its_viewer_and_projects_visible_recipients(tmp_path): + from agent_comms.coordination_database import CoordinationStore + + comms = wire(tmp_path / "bus") + for name in ("sender", "peer-a", "peer-b"): + comms.registry.declare(Thread(name, frozenset({"ci"}), Checkout(tmp_path))) + sent = comms.messaging.send("sender", "#ci", "channel outcome").message + original = comms.bus.bus.delivery(sent.seq) + CoordinationStore.path_in(comms.root).unlink(missing_ok=True) + viewer = OwnerOutcomes.of(comms.registry.require("peer-a")) + read = NotificationAssignment.read(comms.root, (original,), viewer=viewer) + assert (read.sources, read.rows, read.attempts, read.viewer) == ((original,), (), (), viewer) + window = MessageNotification.delivery_window(comms.root, comms.registry, (original,), read) + assert {item.recipient for item in window.outcomes[(sent.seq, sent.message_id)]} == {"peer-a"} + + +def test_a_private_root_without_a_coordinator_still_refuses_notification_reads(private_comms): + from agent_comms.coordination_database import CoordinationStore + from agent_comms.coordination_errors import PublicationActivationBlocked + + for name in ("sender", "peer-a"): + private_comms.registry.declare(Thread(name, frozenset({"ci"}), Checkout(private_comms.root))) + sent = private_comms.messaging.send("sender", "#ci", "channel outcome").message + original = private_comms.bus.bus.delivery(sent.seq) + CoordinationStore.path_in(private_comms.root).unlink() + with pytest.raises(PublicationActivationBlocked): + NotificationAssignment.read(private_comms.root, (original,), viewer=AllOutcomes()) diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 0a609af0cb..ec43cb2299 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -20,7 +20,16 @@ decode_updates, encode_updates, ) -from agent_comms.child_process import ProcessIdentity, SpawnedProcess +from agent_comms.child_process import ProcessIdentity +from agent_comms.errors import RelationViolationError +from agent_comms.field_codec import FieldCodec +from agent_comms.input_disposition import InputDispositions, InputDocument +from agent_comms.input_key import BusInputKey +from agent_comms.bus_read import OwnerReadRuntimeRequest +from agent_comms.order_tools import InputDeliveryRead +from agent_comms.owner_connection import OwnerIdentityChangedError, OwnerReplyConnection, RuntimeRequest +from agent_comms.thread_execution import ExternalThreadExecution +from agent_comms.tool_caller import ToolCaller from agent_comms.comms import wire from agent_comms.field_codec import RootId from agent_comms.owner_protocol import OwnerAnnouncement, OwnerReady, OwnerResult, OwnerUpdate @@ -32,42 +41,16 @@ from agent_comms.session_file import RegistrySessionLocation from agent_comms.native_arguments import GivenBinary from delivery_owner_fixture import canonical_agent +from owner_socket_fixture import declare_owner, silent_socket +from stored_inputs import stored_inputs from agent_comms.threads import Thread -@pytest.fixture -def runtime_processes(): - import sys - - children = [] - try: - for _ in range(2): - children.append( - SpawnedProcess.launch((sys.executable, "-c", "import time; time.sleep(60)")) - ) - yield tuple(child.identity for child in children) - finally: - for child in children: - child.stop_sync() - - async def test_dispatched_request_without_a_reply_remains_unknown_and_is_not_replayed(comms): process = ProcessIdentity.capture(os.getpid()) - comms.registry.declare(Thread( - "reply-loss", frozenset(), Checkout(comms.root.parent), process_identity=process, - )) - path = socket_path(comms.root, process.pid) - path.parent.mkdir(parents=True, exist_ok=True) - received = [] - - async def disconnect_after_request(reader, writer): - received.append(json.loads(await reader.readline())) - writer.close() - await writer.wait_closed() - - with PrivateSocketRole.address(path) as address: - server = await asyncio.start_unix_server(disconnect_after_request, path=address) - connection = RuntimeConnection(comms, "reply-loss", path) + declare_owner(comms, "reply-loss", process) + async with silent_socket(comms, process) as received: + connection = RuntimeConnection(comms, "reply-loss", socket_path(comms.root, process.pid)) try: with pytest.raises(OwnerReplyLostError) as lost: await connection.request("cancel") @@ -76,9 +59,113 @@ async def disconnect_after_request(reader, writer): assert [request["action"] for request in received] == ["cancel"] finally: await connection.close() - server.close() - await server.wait_closed() - path.unlink(missing_ok=True) + + +def recorded_input(comms) -> InputDocument: + """One durable input of the read's subject; a refused or lost read leaves it unchanged.""" + ledger = InputDispositions(comms.root) + ledger.record(BusInputKey(5), seq=5, owner="held", claim_scope=1, target="held", text="Kept") + return stored_inputs(ledger.root) + + +async def mcp_input_delivery(comms, include_history: bool = True): + async with asyncio.timeout(30): + return await asyncio.to_thread( + InputDeliveryRead.invoke, comms, {"thread": "held", "include_history": include_history}, + caller=ToolCaller.from_actor(None), + ) + + +@pytest.mark.parametrize("include_history", [False, True]) +async def test_an_input_delivery_read_without_a_reply_is_lost_after_one_dispatch(comms, include_history): + """The owner received exactly the declared operation once, then hung up: the caller + gets the original reply loss, and no durable input moved. The peer runs no domain body.""" + process = ProcessIdentity.capture(os.getpid()) + declare_owner(comms, "held", process) + before = recorded_input(comms) + async with silent_socket(comms, process) as received: + with pytest.raises(OwnerReplyLostError): + await mcp_input_delivery(comms, include_history) + assert [FieldCodec.decode(RuntimeRequest, request) for request in received] == [ + OwnerReadRuntimeRequest( + thread="held", operation=InputDeliveryRead(thread="held", include_history=include_history), + ), + ] + assert stored_inputs(comms.root) == before + + +async def test_a_held_owner_read_refuses_a_replaced_root_before_dispatch(comms): + process = ProcessIdentity.capture(os.getpid()) + owner = declare_owner(comms, "held", process) + reading = OwnerReadRuntimeRequest(thread="held", operation=InputDeliveryRead(thread="held")) + async with silent_socket(comms, process) as received: + connection = OwnerReplyConnection.held(comms, owner.incarnation, reading) + moved = comms.root.with_name(comms.root.name + "-replaced") + comms.root.rename(moved) + comms.root.mkdir() + try: + with pytest.raises(OwnerIdentityChangedError): + async with asyncio.timeout(30): + await reading.read(connection) + finally: + comms.root.rmdir() + moved.rename(comms.root) + await connection.close() + assert received == [] + + +async def test_a_same_name_recreation_refuses_the_held_read_and_the_old_witness(comms): + process = ProcessIdentity.capture(os.getpid()) + old = declare_owner(comms, "held", process) + reading = OwnerReadRuntimeRequest(thread="held", operation=InputDeliveryRead(thread="held")) + async with silent_socket(comms, process) as received: + connection = OwnerReplyConnection.held(comms, old.incarnation, reading) + try: + comms.registry.unregister("held") + comms.registry.begin_delete("held") + with comms.registry.store.editing() as edit: + edit.document.remove_originals((old,)) + edit.commit() + new = declare_owner(comms, "held", process) + assert new.name == old.name and new.incarnation != old.incarnation + assert comms.registry.require("held").incarnation == new.incarnation + with pytest.raises(OwnerIdentityChangedError): + async with asyncio.timeout(30): + await reading.read(connection) + with pytest.raises(OwnerIdentityChangedError): + OwnerReplyConnection.held(comms, old.incarnation, reading) + finally: + await connection.close() + assert received == [] + + +async def test_a_held_owner_read_refuses_after_its_process_dies(comms, runtime_processes): + child = runtime_processes[0] + owner = declare_owner(comms, "held", child.identity) + reading = OwnerReadRuntimeRequest(thread="held", operation=InputDeliveryRead(thread="held")) + before = recorded_input(comms) + async with silent_socket(comms, child.identity) as received: + connection = OwnerReplyConnection.held(comms, owner.incarnation, reading) + try: + child.stop_sync() + assert not child.identity.alive() + with pytest.raises(ConnectionError): + async with asyncio.timeout(30): + await reading.read(connection) + finally: + await connection.close() + assert received == [] + assert stored_inputs(comms.root) == before + + +async def test_an_external_owner_refuses_the_input_delivery_read_before_any_socket_wait( + comms, runtime_processes, +): + declare_owner(comms, "held", runtime_processes[0].identity, ExternalThreadExecution) + before = recorded_input(comms) + with pytest.raises(RelationViolationError): + await mcp_input_delivery(comms) + assert stored_inputs(comms.root) == before def test_owner_cursor_scope_rebases_only_attachment_session_alias(): @@ -185,7 +272,7 @@ async def test_existing_proxy_follows_renamed_owner_restart_and_resubscribes( tmp_path, runtime_processes ): comms = wire(tmp_path / "wire") - old, new = runtime_processes + old, new = (child.identity for child in runtime_processes) old_pid, new_pid = old.pid, new.pid comms.registry.declare(Thread('worker', frozenset(), Checkout(tmp_path), process_identity=old)) client = canonical_agent(comms) diff --git a/tests/test_thread_presentation.py b/tests/test_thread_presentation.py index 5679155b19..402e1a9b9c 100644 --- a/tests/test_thread_presentation.py +++ b/tests/test_thread_presentation.py @@ -43,7 +43,8 @@ def test_missing_presentation_owns_source_absence_and_retired_status(comms): presentations = comms.views.thread_presentations(("missing",)) absent = presentations.read("missing") assert absent == presentations.view("missing") == NoThreadPresentation() - assert not absent.notification_window.activity_names() + assert absent.published() is absent + assert absent.sent_notifications() == () assert absent.label == "" painted = ThreadStatusModel.known("missing", absent) retired = ThreadStatusModel.retired("missing") diff --git a/tests/test_thread_read_results.py b/tests/test_thread_read_results.py index 14356cfcbd..04c44f17b4 100644 --- a/tests/test_thread_read_results.py +++ b/tests/test_thread_read_results.py @@ -77,7 +77,8 @@ def test_assignment_row_changes_attribute_only_their_view_without_projection( from agent_comms.coordinator import Coordination from agent_comms.presentation import MessageNotification from agent_comms.threads import Thread - from agent_comms.ui_model.observation import NotificationsRead, ReadNotifications + from agent_comms.notification_read import NotificationsRead + from agent_comms.notification_reads import LiveNotificationRead root, root_id, comms, initial, _people = _root(private_directory, direct=True) for name in ('gamma', 'outside'): @@ -100,7 +101,7 @@ def test_assignment_row_changes_attribute_only_their_view_without_projection( revision = comms.views.revision() activity = comms.views.observed_activity(revision, names) previous = ThreadsObserved(revision, results, names, activity) - notification_window = ReadNotifications(root_id, (notification_source.reference,)).read(comms) + notification_window = LiveNotificationRead((notification_source.reference,)).read(comms) notification_read = NotificationsRead.of(revision, comms, activity, notification_window) if changed_name in ('read', 'unrelated_read'): ledger = comms.bus.reads @@ -145,3 +146,94 @@ def count_projection(frame, event, argument): assert calls == 0 if changed_name == 'expiry': assert evidence_calls == 0 + + +def reachable_types(root: object) -> set[str]: + """Class names of every instance reachable from root, not following classes, + modules or functions.""" + import gc + import types + + seen, names, frontier = set(), set(), [root] + skipped = (type, types.ModuleType, types.FunctionType, types.BuiltinFunctionType, + types.MethodType) + while frontier: + item = frontier.pop() + if id(item) in seen or isinstance(item, skipped): + continue + seen.add(id(item)) + names.add(type(item).__name__) + frontier.extend(gc.get_referents(item)) + return names + + +def pickled_types(value: object) -> set[str]: + """Class names of every object the transport pickles for value.""" + import io + import pickle + + names: set[str] = set() + + class Recording(pickle.Pickler): + def reducer_override(self, obj): + names.add(type(obj).__name__) + return NotImplemented + + Recording(io.BytesIO(), protocol=pickle.HIGHEST_PROTOCOL).dump(value) + return names + + +ACQUIRED_EVIDENCE = { + "AcquiredThreadPresentation", "CapturedNotificationWindow", "NotificationAssignmentRead", + "SourceEvidence", "WakeAssignment", "AssignmentActivityEvidence", "HarnessDelivery", + "Delivered", "NotificationReadMark", +} + + +def test_thread_reading_keeps_acquired_evidence_and_publishes_its_projection(private_directory): + import pickle + + from conftest import publish_initial + from test_coordinated_runtime import _root + from agent_comms.thread_presentation import AcquiredThreadPresentation, ThreadPresentation + + _path, _root_id, comms, initial, _people = _root(private_directory, direct=True) + reading = ObserveThreads(frozenset({'beta'})).observation() + status = OpenThreadStatus(lambda flush: None) + status.open('beta') + + def observe() -> ThreadsObserved: + published = [step for step in reading.read(comms, comms.views.revision()) + if type(step) is ThreadsObserved] + assert len(published) == 1 + status.observed(published[0]) + status.rows.flush() + return published[0] + + first = observe() + retained = reading.observed.results['beta'].presentation + assert type(retained) is AcquiredThreadPresentation + assert retained.incoming_references + outgoing = first.results['beta'].presentation + assert type(outgoing) is ThreadPresentation + assert outgoing == retained.published() + assert outgoing.status_display() == retained.status_display() + assert outgoing.read_identity is retained.read_identity + assert outgoing.notifications == retained.notifications + incoming = [item for item in outgoing.notifications if item.message.seq == initial.message.seq] + assert incoming and all(item.message == initial.envelope for item in incoming) + assert not ACQUIRED_EVIDENCE & pickled_types(first) + with pytest.raises(TypeError, match="published"): + pickle.dumps(replace(first, results={'beta': ThreadPresented(retained)})) + assert (ThreadStatusModel.known('beta', outgoing).lines + == ThreadStatusModel.known('beta', retained).lines) + + publish_initial(comms, 'sender', 'beta', 'A later message for beta') + second = observe() + assert second.results['beta'].presentation != outgoing + assert reading.observed.results['beta'].presentation is not retained + # The UI's current and last-published rows hold projections only; the old acquired + # value is reachable from neither. + assert status.rows._published == status.rows.rows + assert not ACQUIRED_EVIDENCE & reachable_types(status.rows) + assert type(status.rows.rows['beta'].presentation) is ThreadPresentation diff --git a/tests/test_tool_replies.py b/tests/test_tool_replies.py index 7e8f779ab1..0a21f3cdf1 100644 --- a/tests/test_tool_replies.py +++ b/tests/test_tool_replies.py @@ -1,18 +1,62 @@ """Every declared tool's reply is the encoding of its declared result record.""" -from typing import get_type_hints +from types import get_original_bases +from typing import TypeVar, get_args, get_origin, get_type_hints +import pytest + +from agent_comms.bus_read import BusRead from agent_comms.comms import Comms from agent_comms.tool_result import ToolResult from agent_comms.tool_request import ToolRequest -def result_type(tool: type[ToolRequest]) -> type: +def bus_read_result(tool: type[BusRead]) -> type: + """The result a bus read declares once: the first concrete specialisation of a BusRead + capability along its MRO. A family header still carrying its type parameter declares + nothing; two different concrete answers contradict each other.""" + declared = tuple(dict.fromkeys( + argument + for declaration in tool.__mro__ + for base in get_original_bases(declaration) + if isinstance(get_origin(base), type) and issubclass(get_origin(base), BusRead) + for argument in get_args(base) + if not isinstance(argument, TypeVar) + )) + if len(declared) != 1: + raise TypeError(f"{tool.__qualname__} declares results {declared!r}, not exactly one") + return declared[0] + + +def apply_result(tool: type[ToolRequest]) -> type: return get_type_hints(tool.apply, localns={ "Comms": Comms, **{param.__name__: param for param in ToolRequest.__type_params__}, })["return"] +def result_type(tool: type[ToolRequest]) -> type: + if issubclass(tool, BusRead): + return bus_read_result(tool) + return apply_result(tool) + + +def test_bus_read_result_follows_an_inherited_specialisation_and_refuses_an_unspecialised_family(): + from agent_comms.bus_read import OwnerLiveRead, UiRead + from agent_comms.notification_evidence import NotificationWindow + from agent_comms.notification_reads import RecordedNotificationRead + + # RecordedNotificationRead restates no header: it inherits NotificationRead's UiRead[...]. + assert bus_read_result(RecordedNotificationRead) is NotificationWindow + for family in (BusRead, UiRead, OwnerLiveRead): + with pytest.raises(TypeError, match="not exactly one"): + bus_read_result(family) + + +def test_an_ordinary_tool_keeps_its_apply_annotation(): + ordinary = next(tool for tool in ToolRequest.members_with(ToolRequest) if not issubclass(tool, BusRead)) + assert result_type(ordinary) is apply_result(ordinary) + + def stores(comms: Comms) -> dict: """registry.json, the todo store and the goal store, byte for byte.""" paths = (comms.root / "registry.json", comms.goals.todos.path, @@ -22,6 +66,5 @@ def stores(comms: Comms) -> dict: def test_every_tool_declares_a_tool_result(): for tool in ToolRequest.members_with(ToolRequest): - from typing import get_args result = result_type(tool) assert all(issubclass(member, ToolResult) for member in (get_args(result) or (result,))), tool.declared_name diff --git a/tests/test_tools.py b/tests/test_tools.py index aed648ac23..d752df53d1 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -139,7 +139,8 @@ def test_threads_tool_summarizes_each_recent_sent_message_once(self, comms, monk presentation = comms.views.thread_presentations(("sender",)).require("sender") assert all(row.message.reference != sent.reference for row in presentation.notifications) outcomes = presentation.notification_window.outcomes[sent.seq, sent.message_id] - assert [summary.reference for summary in SentMessageSummary.recent(presentation)] == [sent.reference] + assert [summary.reference for summary in SentMessageSummary.for_sources( + presentation.sent_notifications())] == [sent.reference] result = invoke_tool(comms, "comms_threads", {}, caller=ThreadCaller("sender")) # One row per sent message, never one per recipient, and never the message body. diff --git a/tests/test_transcript_observation_lifetime.py b/tests/test_transcript_observation_lifetime.py index 3d04b673f7..e5a37f215c 100644 --- a/tests/test_transcript_observation_lifetime.py +++ b/tests/test_transcript_observation_lifetime.py @@ -12,6 +12,7 @@ from agent_comms.coordinator import Coordination from agent_comms.native_arguments import GivenBinary from agent_comms.runtime import RuntimeProxy, socket_path +from agent_comms.notification_reads import LiveNotificationRead from agent_comms.ui_model.observation import ( ObservationProcess, ObserverDied, @@ -90,7 +91,7 @@ async def test_subscriptions_share_process_and_last_close_joins_it(headless_owne async def test_broken_pipe_ends_subscription_with_observer_died(headless_owner): process = ObservationProcess(headless_owner._comms.route) pending = asyncio.create_task(process.read( - ReadNotifications(str(headless_owner._comms.root), ()))) + ReadNotifications(str(headless_owner._comms.root), LiveNotificationRead(())))) try: await asyncio.sleep(0) process.process.terminate() diff --git a/tests/test_transcript_pages.py b/tests/test_transcript_pages.py index f9e86fbf87..38fa13ed2d 100644 --- a/tests/test_transcript_pages.py +++ b/tests/test_transcript_pages.py @@ -12,6 +12,12 @@ from agent_comms.thread_identity import RecordedThreadIncarnation from agent_comms.transcripts import NativeTranscriptCursor, WireTranscriptCursor from agent_comms.thread_text import ForkedParent, GivenThreadText +from agent_comms.acp_extension import TranscriptSnapshotUpdate +from agent_comms.field_codec import FieldCodec +from agent_comms.order_tools import TranscriptPageRead +from agent_comms.tool_caller import ThreadCaller +from agent_comms.tools import invoke_tool +from agent_comms.ui_model.observation import ReadTranscript def transcript(path, count): @@ -57,6 +63,44 @@ def test_pages_reach_beginning_and_return_to_tail_without_duplicates(tmp_path): assert seen == [f"Record {i}" for i in range(105)] +def shared_page(comms, operation: TranscriptPageRead) -> TranscriptSnapshotUpdate: + """One page operation served directly, inside the UI's root-bound envelope and through MCP: + all three carry the same page, draft and witness as the domain capture.""" + expected = TranscriptSnapshotUpdate.from_read(comms.transcripts.capture_page_read( + operation.thread, before=operation.before, after=operation.after, + through=operation.through, historical_source=operation.historical_source)) + direct = operation.read(comms) + assert ReadTranscript(str(comms.root), operation).read(comms) == direct == expected + assert invoke_tool(comms, operation.declared_name, FieldCodec.encode_fields(operation), + caller=ThreadCaller(operation.thread)) == FieldCodec.encode(direct) + return direct + + +def test_one_page_operation_pages_alike_for_the_ui_and_mcp(tmp_path): + path = tmp_path / "session.jsonl" + transcript(path, 45) + comms = installed_comms(tmp_path / "wire") + comms.registry.declare(Thread('worker', frozenset(), Checkout(tmp_path), session_file=RegistrySessionLocation.from_path(str(path), comms.root))) + snapshot = shared_page(comms, TranscriptPageRead(thread="worker")) + through = snapshot.page.after + seen = [event.text for event in snapshot.page.events] + pages = 1 + while snapshot.page.has_older: + snapshot = shared_page(comms, TranscriptPageRead(thread="worker", before=snapshot.page.before)) + seen[:0] = [event.text for event in snapshot.page.events] + pages += 1 + assert pages > 1 and seen == [f"Record {i}" for i in range(45)] + seen = [event.text for event in snapshot.page.events] + while snapshot.page.has_newer: + snapshot = shared_page(comms, TranscriptPageRead( + thread="worker", after=snapshot.page.after, through=through)) + seen.extend(event.text for event in snapshot.page.events) + assert seen == [f"Record {i}" for i in range(45)] + comms.registry.declare(Thread('quiet', frozenset(), Checkout(tmp_path))) + empty = shared_page(comms, TranscriptPageRead(thread="quiet")) + assert not empty.page.events and not empty.page.has_older and not empty.page.has_newer + + def test_oversized_message_and_file_identity(tmp_path): path = tmp_path / "session.jsonl" text = "x" * 200000 diff --git a/tests/test_unreadable_history_observation.py b/tests/test_unreadable_history_observation.py index 3791deea7a..2320995c3d 100644 --- a/tests/test_unreadable_history_observation.py +++ b/tests/test_unreadable_history_observation.py @@ -26,6 +26,7 @@ from agent_comms.owner_connection import OwnerIdentityChangedError, OwnerReplyLostError from agent_comms.context_store import ContextRequest from agent_comms.presentation import WireRevision +from agent_comms.notification_reads import LiveNotificationRead from agent_comms.ui_model.observation import ( ObservationProcess, ObserveSidebar, @@ -45,7 +46,7 @@ ) from agent_comms.ui_model.status import OpenThreadStatus from agent_comms.turn_context import RecordedContextManifest -from test_observer_death import readable +from observed_queue import ObservedQueue from test_turn_context_wire import corrupt_context_kind, manifest from agent_comms.threads import Thread @@ -222,56 +223,23 @@ async def test_unknown_saved_member_does_not_kill_the_real_observation_process(c corrupt_context_kind(comms, "saved", "unavailable_future_segment") original = comms.contexts.path.read_bytes() original_bus = comms.bus.bus.path.read_bytes() - process = ObservationProcess(comms.route) - try: - await readable(process) - process.results() + delivered = ObservedQueue() + async with ObservationProcess(comms.route) as process: + process.start_receiving(delivered.observed, delivered.failed) process.request(ObserveSidebar(Checkout(comms.root.parent), True, True)) - async with asyncio.timeout(30): - while True: - await readable(process) - results = process.results() - sidebar = next( - (result for result in results if isinstance(result, SidebarObserved)), None - ) - if sidebar is not None: - break + await delivered.until(lambda result: isinstance(result, SidebarObserved)) assert process.process.is_alive() process.request(ObserveThreads(frozenset({"saved"}))) - async with asyncio.timeout(30): - while True: - await readable(process) - results = process.results() - threads = next( - (result for result in results if isinstance(result, ThreadsObserved)), None - ) - if threads is not None: - break + threads = await delivered.until(lambda result: isinstance(result, ThreadsObserved)) assert not isinstance(threads.results["saved"], ThreadUnavailable) # Only the request needing the damaged payload is refused, over the # same actual child/pipe that served the sidebar and thread reads. - damaged = asyncio.create_task(process.read(ReadRecordedContext(str(comms.root), "saved"))) - await asyncio.sleep(0) - async with asyncio.timeout(30): - while not damaged.done(): - await readable(process) - process.results() - await asyncio.sleep(0) with pytest.raises(StoredHistoryUnreadable): - await damaged + async with asyncio.timeout(30): + await process.read(ReadRecordedContext(str(comms.root), "saved")) # Serve a separate, readable request using the same actual pipe/child. - waiting = asyncio.create_task(process.read(ReadNotifications(str(comms.root), ()))) - await asyncio.sleep(0) async with asyncio.timeout(30): - while not waiting.done(): - await readable(process) - process.results() - # The pipe resolved the Future; let its awaiting Task resume. - # An unchanged error no longer emits an unrelated extra wake. - await asyncio.sleep(0) - assert await waiting == {} + assert (await process.read(ReadNotifications(str(comms.root), LiveNotificationRead(())))).outcomes == {} assert process.process.is_alive() assert comms.contexts.path.read_bytes() == original assert comms.bus.bus.path.read_bytes() == original_bus - finally: - process.close() diff --git a/tools/debt_ratchet.py b/tools/debt_ratchet.py index 301c03804a..68a3a3789a 100644 --- a/tools/debt_ratchet.py +++ b/tools/debt_ratchet.py @@ -5,6 +5,8 @@ import argparse import ast +import builtins +import copy import atexit import dataclasses import enum @@ -187,9 +189,20 @@ def at(cls, path: str) -> FileCount: return FileCount(cls, path) @classmethod - def blocks(cls, delta: Counts) -> bool: + def grown(cls, delta: Counts) -> bool: + """Whether any of this measure's counts grew.""" return any(value > 0 for key, value in delta.items() if cls.owns(key)) + @classmethod + def blocks(cls, delta: Counts) -> bool: + return cls.grown(delta) + + @classmethod + def missing(cls, head: Counts, introduced: dict[FileCount, list[int]]) -> list[MeasureCount]: + """This measure's counts the comparison has no required evidence for; a measure that + needs none beyond its counts misses nothing.""" + return [] + @classmethod def grew(cls, key: MeasureCount, grown: int) -> str: """The line reporting one count's growth; the count words it.""" @@ -794,9 +807,10 @@ def inputs(cls, repo: Path, ref: str, head: str, root: str, selected: list[str]) @classmethod def findings(cls, repo: Path, ref: str, head: str, root: str, selected: list[str], side: type[Side]) -> tuple[type[Side], Counter[str]]: - return side, ContentCache.memo( - partial(cls.checked, repo, ref, head, root, selected, side), - cls.__name__, root, cls.inputs(repo, ref, head, root, selected), *selected) + """Each comparison runs the checker. A checker that reads installed packages (mypy) + depends on more than the committed tree, so its count is never stored by content; + the checker's own cache tracks those inputs.""" + return side, cls.checked(repo, ref, head, root, selected, side) @classmethod def checked(cls, repo: Path, ref: str, head: str, root: str, selected: list[str], @@ -896,6 +910,15 @@ class LintFindings(Advisory, CheckedRevision): def command(tree: Path, selected: list[str], side: type[Side]) -> list[str]: return ["ruff", "check", "--no-cache", "--output-format", "concise", *selected] + @classmethod + def findings(cls, repo: Path, ref: str, head: str, root: str, selected: list[str], + side: type[Side]) -> tuple[type[Side], Counter[str]]: + """Ruff reads only the revision's tree and the head's configuration, so its count is + stored under them.""" + return side, ContentCache.memo( + partial(cls.checked, repo, ref, head, root, selected, side), + cls.__name__, root, cls.inputs(repo, ref, head, root, selected), *selected) + class TypeErrors(CheckedRevision): """Mypy errors per changed file. Mypy checks the changed files and every module they import, @@ -954,10 +977,9 @@ def selected(client: StackClient) -> list[str]: @classmethod def findings(cls, repo: Path, ref: str, client: StackClient, side: type[Side]) -> tuple[type[Side], Counter[str]]: - commit = git(client.checkout, "rev-parse", "HEAD").decode().strip() - tree = git(repo, "rev-parse", f"{ref}:src").decode().strip() - return side, ContentCache.memo(partial(cls.checked, repo, ref, client, side), - cls.__name__, client.name, commit, tree) + """Mypy resolves the client's other imports from the installed environment, which no + content key names, so each comparison checks afresh under mypy's own cache.""" + return side, cls.checked(repo, ref, client, side) @classmethod def checked(cls, repo: Path, ref: str, client: StackClient, side: type[Side]) -> Counter[str]: @@ -1047,11 +1069,30 @@ def departed(cls, repo: Path, base: str, head: str, path: str) -> ChangedSource: class ScopeNames(ast.NodeVisitor): - """Each node's enclosing class and function names, dotted, through the AST's own dispatch.""" + """Each node's enclosing class and function names, dotted, through the AST's own dispatch, + with the names each node binds where it stands and the nodes that declare them: a class or + function, an import alias, an annotated name. A star import binds ``*``.""" def __init__(self) -> None: self.found: dict[ast.AST, str] = {} self.scope = "" + self.binds: dict[ast.AST, tuple[str, ...]] = {} + self.declares: set[ast.AST] = set() + + @classmethod + def of(cls, tree: ast.AST) -> ScopeNames: + names = cls() + names.visit(tree) + return names + + def bindings(self, scope: str) -> dict[str, list[ast.AST]]: + """Every binding of each name in one lexical scope, in walk order.""" + found: dict[str, list[ast.AST]] = {} + for node, names in self.binds.items(): + if self.found[node] == scope: + for name in names: + found.setdefault(name, []).append(node) + return found def generic_visit(self, node: ast.AST) -> None: self.found[node] = self.scope @@ -1059,10 +1100,65 @@ def generic_visit(self, node: ast.AST) -> None: def visit_named(self, node: ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef) -> None: self.found[node] = outer = self.scope + self.binds[node] = (node.name,) + self.declares.add(node) self.scope = f"{outer}.{node.name}" super().generic_visit(node) self.scope = outer + def visit_alias(self, node: ast.alias) -> None: + self.binds[node] = (node.asname or node.name.split(".")[0],) + self.declares.add(node) + self.generic_visit(node) + + def visit_Name(self, node: ast.Name) -> None: + match node.ctx: + case ast.Store() | ast.Del(): + self.binds[node] = (node.id,) + self.generic_visit(node) + + def visit_AnnAssign(self, node: ast.AnnAssign) -> None: + match node.target: + case ast.Name(): + self.declares.add(node.target) + self.generic_visit(node) + + def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None: + self.binds[node] = (node.name,) if node.name else () + self.generic_visit(node) + + def visit_MatchAs(self, node: ast.MatchAs) -> None: + self.binds[node] = (node.name,) if node.name else () + self.generic_visit(node) + + def visit_MatchStar(self, node: ast.MatchStar) -> None: + self.binds[node] = (node.name,) if node.name else () + self.generic_visit(node) + + def visit_MatchMapping(self, node: ast.MatchMapping) -> None: + self.binds[node] = (node.rest,) if node.rest else () + self.generic_visit(node) + + def visit_TypeVar(self, node: ast.TypeVar) -> None: + self.binds[node] = (node.name,) + self.generic_visit(node) + + def visit_ParamSpec(self, node: ast.ParamSpec) -> None: + self.binds[node] = (node.name,) + self.generic_visit(node) + + def visit_TypeVarTuple(self, node: ast.TypeVarTuple) -> None: + self.binds[node] = (node.name,) + self.generic_visit(node) + + def visit_Global(self, node: ast.Global) -> None: + self.binds[node] = tuple(node.names) + self.generic_visit(node) + + def visit_Nonlocal(self, node: ast.Nonlocal) -> None: + self.binds[node] = tuple(node.names) + self.generic_visit(node) + def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self.visit_named(node) @@ -1073,6 +1169,154 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: self.visit_named(node) +class NominalProof: + """What an expression's resolved owners prove it is; only two proved identities with + the same owners and structure are the same.""" + + def same(self, other: NominalProof) -> bool: + return False + + def matches(self, key: str) -> bool: + return False + + +class UnprovedIdentity(NominalProof): + """A reference the binding facts could not resolve: equal to nothing, itself included.""" + + +@dataclass(frozen=True) +class ProvedIdentity(NominalProof): + key: str + + def same(self, other: NominalProof) -> bool: + return other.matches(self.key) + + def matches(self, key: str) -> bool: + return key == self.key + + +@dataclass(frozen=True) +class DeclaredClass: + """One top-level class of one changed module at one revision, read for its fields.""" + + path: str + tree: ast.Module + node: ast.ClassDef + + @classmethod + def top(cls, path: str, tree: ast.Module) -> dict[str, DeclaredClass]: + """The module's classes whose module-level name is still that one declaration.""" + return {name: cls(path, tree, statement) + for name, (_, statement) in DecoratorBinding.declarations(tree, "", tree.body).items() + if isinstance(statement, ast.ClassDef)} + + @property + def scope(self) -> str: + return f".{self.node.name}" + + @cached_property + def fields(self) -> dict[str, ast.AnnAssign]: + """The class body's annotated names bound by that annotation alone.""" + return {name: statement for name, (_, statement) in DecoratorBinding.declarations( + self.tree, self.scope, self.node.body).items() + if isinstance(statement, ast.AnnAssign)} + + def binds(self, name: str) -> bool: + """Whether the class scope still binds ``name`` in any form.""" + return name in DecoratorBinding.bindings(self.tree, self.scope) + + @cached_property + def annotated(self) -> DecoratorBinding: + return DecoratorBinding.named("typing", "Annotated", self.tree, self.path, self.scope) + + def compared(self, field: str) -> list[ast.expr]: + """The field's name, its type without typing.Annotated metadata, and its default.""" + statement = self.fields[field] + match statement.annotation: + case ast.Subscript(value=value, slice=ast.Tuple(elts=[declared, *_])) if self.annotated.marks(value): + annotation: ast.expr = declared + case annotation: + pass + return [statement.target, annotation, *([statement.value] if statement.value else [])] + + def declared(self, field: str) -> NominalProof: + """The field's type and default, each reference resolved to its owner in this class's + scope; the name is the field's own and needs no owner.""" + _, *written = self.compared(field) + return DecoratorBinding.identity(self.tree, self.path, self.scope, ast.Tuple(elts=written)) + + def holding(self, held: DeclaredClass, base: DeclaredClass) -> tuple[str, ...]: + """The fields this class newly declares whose type is the module-level binding of + ``held``.""" + binding = DecoratorBinding.named( + ".".join(DecoratorBinding.module_of(held.path)), held.node.name, self.tree, self.path, + self.scope) + return tuple(name for name, statement in self.fields.items() + if name not in base.fields and binding.marks(statement.annotation)) + + +class FieldExtraction: + """Fields a class gave up to the one new class it now holds in a typed field. + + The held field's annotation is the provenance edge: an old class present at both + revisions newly declares one field typed as one class absent from the base, and no other + old class holds that class. Each field the old class lost, declared once in the new class + under the same name with the same type (typing.Annotated metadata aside) and the same + default, carries its occurrences there. Only occurrences inside that one proved field + declaration are credited, each spending one occurrence that left the old class, so a + retained field, an extra field, a method or a second copy is still written. + """ + + @classmethod + def credit(cls, measure: type[CleanChangedLines], sources: list[ChangedSource], + left: list[ChangedSource], departed: Counter[tuple[str, str]]) -> Counter[ast.AST]: + """Each proved destination occurrence's credit; ``departed`` spends what it pays.""" + existing = {name for source in [*sources, *left] for name in DeclaredClass.top( + source.path, source.base_tree)} + new = [held for source in sources + for name, held in DeclaredClass.top(source.path, source.tree).items() + if name not in existing] + edges: list[tuple[DeclaredClass, DeclaredClass, DeclaredClass]] = [] + for source in sources: + before = DeclaredClass.top(source.path, source.base_tree) + for name, owner in DeclaredClass.top(source.path, source.tree).items(): + if name in before: + for held in new: + edges.extend((before[name], owner, held) for _ in owner.holding(held, before[name])) + owners = Counter((base.path, base.node.name) for base, _, _ in edges) + holders = Counter((held.path, held.node.name) for _, _, held in edges) + credited: Counter[ast.AST] = Counter() + for base, owner, held in edges: + if owners[base.path, base.node.name] == holders[held.path, held.node.name] == 1: + for field in sorted(base.fields): + if (not owner.binds(field) and field in held.fields + and base.declared(field).same(held.declared(field))): + cls.transfer(measure, base, held, field, departed, credited) + return credited + + @staticmethod + def transfer(measure: type[CleanChangedLines], base: DeclaredClass, held: DeclaredClass, + field: str, departed: Counter[tuple[str, str]], credited: Counter[ast.AST]) -> None: + """One field's occurrences, paid one for one by those that left the old declaration. + The declarations are proved the same, so their parts pair node for node in walk order; + parts whose raw shapes differ pair nothing.""" + scope = f".{base.node.name}" + counted = {declared: ({id(node) for node in measure.sites(declared.tree, declared.path)}, + measure.scopes(declared.tree)) for declared in (base, held)} + for gave, took in zip(base.compared(field), held.compared(field), strict=True): + left, arrived = list(ast.walk(gave)), list(ast.walk(took)) + if [type(node) for node in left] != [type(node) for node in arrived]: + continue + for old, new in zip(left, arrived, strict=True): + if ((carried := min(measure.occurrences(old), measure.occurrences(new))) + and id(old) in counted[base][0] and id(new) in counted[held][0]): + identity = counted[base][1][old], ast.unparse(old) + if identity[0] == scope: + paid = min(carried, departed[identity]) + departed[identity] -= paid + credited[new] += paid + + class CleanChangedLines(OccurrenceMeasure): """No occurrence may be written, even when another occurrence is deleted. @@ -1083,9 +1327,10 @@ class CleanChangedLines(OccurrenceMeasure): class moved between files, is the same occurrence moved, not a new one written. """ - @staticmethod + @classmethod def introduced( - repo: Path, base: str, head: str, paths: tuple[str, ...], departed_from: tuple[str, ...], + cls: type[typing.Self], repo: Path, base: str, head: str, paths: tuple[str, ...], + departed_from: tuple[str, ...], ) -> dict[FileCount, list[int]]: """Each strict measure's sites on each path's changed lines. Occurrences that left the changed modules in ``departed_from`` (those present at the base) pay for the same @@ -1093,14 +1338,15 @@ def introduced( sources = [ChangedSource.read(repo, base, head, path) for path in paths] left = [ChangedSource.departed(repo, base, head, path) for path in departed_from] found: dict[FileCount, list[int]] = {} - for measure in Measure.members_with(CleanChangedLines): + for measure in cls.members_with(cls): departed: Counter[tuple[str, str]] = Counter() for source in left: gone = measure.identities(source.base_tree, source.path) gone.subtract(measure.identities(source.tree, source.path)) departed.update(+gone) + extracted = FieldExtraction.credit(measure, sources, left, departed) for source in sources: - found[measure.at(source.path)] = measure.changed_sites(source, departed) + found[measure.at(source.path)] = measure.changed_sites(source, departed, extracted) return found @staticmethod @@ -1121,9 +1367,26 @@ def identities(cls, tree: ast.AST, filename: str) -> Counter[tuple[str, str]]: return found @classmethod - def changed_sites(cls, source: ChangedSource, departed: Counter[tuple[str, str]]) -> list[int]: + def blocks(cls, delta: Counts) -> bool: + """Never on raw growth: Comparison.failed fails every introduced occurrence on its own, + and a moved or extracted occurrence grows its destination's raw count while writing + nothing. A new occurrence still fails through ``introduced`` even when an unrelated + deletion offsets the totals.""" + return False + + @classmethod + def missing(cls, head: Counts, introduced: dict[FileCount, list[int]]) -> list[MeasureCount]: + """A file with occurrences at the head but no introduced-site report is unproved: only + an explicit report, even an empty one, says no occurrence there is new.""" + return [key for key, value in head.items() + if cls.owns(key) and value > 0 and key not in introduced] + + @classmethod + def changed_sites(cls, source: ChangedSource, departed: Counter[tuple[str, str]], + extracted: Counter[ast.AST]) -> list[int]: """The first changed line of each occurrence written there that the base lacks and no - changed module gave up; a moved occurrence spends the departure it came from.""" + changed module gave up; a moved occurrence spends the departure it came from, and an + occurrence in a proved field extraction spends its credit.""" scopes = cls.scopes(source.tree) unwritten = cls.identities(source.tree, source.path) unwritten.subtract(cls.identities(source.base_tree, source.path)) @@ -1136,7 +1399,9 @@ def changed_sites(cls, source: ChangedSource, departed: Counter[tuple[str, str]] unwritten[identity] -= arrived moved = min(arrived, departed[identity]) departed[identity] -= moved - sites.extend([min(touched)] * (arrived - moved)) + carried = min(arrived - moved, extracted[node]) + extracted[node] -= carried + sites.extend([min(touched)] * (arrived - moved - carried)) return sites @@ -1237,16 +1502,19 @@ def lines(cls, source: bytes, filename: str) -> list[int]: return sorted([*super().lines(source, filename), *cls.prose_lines(module)]) @classmethod - def changed_sites(cls, source: ChangedSource, departed: Counter[tuple[str, str]]) -> list[int]: + def changed_sites(cls, source: ChangedSource, departed: Counter[tuple[str, str]], + extracted: Counter[ast.AST]) -> list[int]: prose = cls.prose_lines(source) & source.lines - return [*super().changed_sites(source, departed), *prose] + return [*super().changed_sites(source, departed, extracted), *prose] @dataclass(frozen=True) class DecoratorBinding: - """The spellings one module binds to a declared decorator, read from its imports alone. + """The spellings one module binds to a declared name (a decorator, a field factory, a + class), read from its module-level imports and declarations. - A decorator of the same name declared or imported from anywhere else marks nothing. + A name of the same spelling declared or imported from anywhere else marks nothing, and + neither does a spelling the module binds twice, conditionally or by star import. """ names: frozenset[str] @@ -1254,25 +1522,101 @@ class DecoratorBinding: @classmethod def of(cls, decorator: typing.Callable[..., object], tree: ast.AST, filename: str) -> DecoratorBinding: - owner, name = decorator.__module__, decorator.__name__ + return cls.named(decorator.__module__, decorator.__name__, tree, filename) + + @staticmethod + def module_of(filename: str) -> tuple[str, ...]: + """The dotted module a source path imports as; ``src`` is the import root.""" parts = Path(filename).with_suffix("").parts - module = parts[1:] if parts[:1] == ("src",) else parts + return parts[1:] if parts[:1] == ("src",) else parts + + @classmethod + def named(cls, owner: str, name: str, tree: ast.AST, filename: str, + scope: str = "") -> DecoratorBinding: + """The spellings binding ``owner.name`` in the module at ``filename``, as read from + ``scope``: a spelling whose root that scope rebinds names something else there.""" + module = cls.module_of(filename) package = module[:-1] - names = {name} if ".".join(module) == owner else set() - for node in ast.walk(tree): - match node: - case ast.Import(names=aliases): - names.update(f"{alias.asname or alias.name}.{name}" - for alias in aliases if alias.name == owner) - case ast.ImportFrom(module=source, names=aliases, level=level): + names: set[str] = set() + body = typing.cast(ast.Module, tree).body + for bound, (declaration, statement) in cls.declarations(tree, "", body).items(): + match statement, declaration: + case ast.ClassDef() | ast.FunctionDef() | ast.AsyncFunctionDef(), _ if ( + ".".join(module) == owner and bound == name): + names.add(name) + case ast.Import(), ast.alias(name=imported, asname=alias) if imported == owner: + names.add(f"{alias or imported}.{name}") + case ast.ImportFrom(module=source, level=level), ast.alias(name=imported): base = package[:len(package) - level + 1] if level else () resolved = ".".join((*base, source) if source else base) - for alias in aliases: - if f"{resolved}.{alias.name}" == owner: - names.add(f"{alias.asname or alias.name}.{name}") - if resolved == owner and alias.name == name: - names.add(alias.asname or name) - return cls(frozenset(names)) + if f"{resolved}.{imported}" == owner: + names.add(f"{bound}.{name}") + if resolved == owner and imported == name: + names.add(bound) + hidden = cls.bindings(tree, scope).keys() if scope else frozenset() + return cls(frozenset(spelling for spelling in names if spelling.split(".")[0] not in hidden)) + + @staticmethod + def bindings(tree: ast.AST, scope: str) -> dict[str, list[ast.AST]]: + """Every binding of each name in one lexical module or class scope, as ScopeNames + decodes it; bindings inside nested functions and classes are theirs, not this scope's.""" + return ScopeNames.of(tree).bindings(scope) + + @staticmethod + def declarations(tree: ast.AST, scope: str, + body: list[ast.stmt]) -> dict[str, tuple[ast.AST, ast.stmt]]: + """Each name the scope binds exactly once, by a declaring node that is one of its body's + statements or such a statement's direct child, with that statement. A star import + leaves no name proved.""" + names = ScopeNames.of(tree) + bindings = names.bindings(scope) + if "*" in bindings: + return {} + owners = {child: statement for statement in body + for child in (statement, *ast.iter_child_nodes(statement))} + return {name: (node, owners[node]) for name, (node, *others) in bindings.items() + if not others and node in names.declares and node in owners} + + @classmethod + def identity(cls, tree: ast.Module, filename: str, scope: str, expression: ast.expr) -> NominalProof: + """The expression with every name and attribute reference replaced by the module and + name that own it, as evaluated in ``scope``: an imported name is its source module's + export, a declared class its own module's, a builtin one the module never binds. A name + the scope itself binds, or any reference these declarations cannot resolve, proves + nothing.""" + module = cls.module_of(filename) + package = module[:-1] + declared = cls.declarations(tree, "", tree.body) + bound = cls.bindings(tree, "") + enclosing = cls.bindings(tree, scope) + unresolved: list[str] = [] + + def owner(name: str) -> str: + match declared.get(name) if name not in enclosing else (): + case ast.alias(name=imported, asname=alias), ast.Import(): + return imported if alias else imported.split(".")[0] + case ast.alias(name=imported), ast.ImportFrom(module=source, level=level): + base = package[:len(package) - level + 1] if level else () + return ".".join((*base, *(source.split(".") if source else ()), imported)) + case _, ast.ClassDef(): + return ".".join((*module, name)) + if name not in bound and name not in enclosing and name in vars(builtins): + return f"builtins.{name}" + unresolved.append(name) + return name + + class Owned(ast.NodeTransformer): + def visit_Name(self, node: ast.Name) -> ast.Name: + return ast.Name(id=owner(node.id)) + + def visit_Attribute(self, node: ast.Attribute) -> ast.AST: + match ast.unparse(node).split("."): + case [root, *attributes] if root.isidentifier() and all(map(str.isidentifier, attributes)): + return ast.Name(id=".".join((owner(root), *attributes))) + return self.generic_visit(node) + + owned = Owned().visit(copy.deepcopy(expression)) + return UnprovedIdentity() if unresolved else ProvedIdentity(ast.dump(owned)) def marks(self, decorator: ast.expr) -> bool: return ast.unparse(decorator) in self.names @@ -2821,10 +3165,20 @@ def found_report(self) -> dict[str, list[Site]]: def delta_report(self) -> dict[str, int]: return self.keyed(self.delta) + @property + def missing(self) -> list[MeasureCount]: + """Counts whose measure's required evidence the comparison lacks.""" + return [key for measure in Measure.members_with(Measure) + for key in measure.missing(self.head, self.introduced)] + + @projected(view="report", name="missing") + def missing_report(self) -> list[str]: + return sorted(key.text for key in self.missing) + @property def failed(self) -> bool: return (any(measure.blocks(self.delta) for measure in Measure.members_with(Measure)) - or any(self.introduced.values())) + or any(self.introduced.values()) or bool(self.missing)) def compare(repo: Path, base: str, head: str, root: str, workers: int, @@ -2880,7 +3234,7 @@ def compare(repo: Path, base: str, head: str, root: str, workers: int, changed.paired(after), introduced, {}) # Only a grown measure is read for its sites, so a clean change pays for none. grown = [measure for measure in LocatedSource.members_with(LocatedSource) - if measure in measured and measure.blocks(comparison.delta)] + if measure in measured and measure.grown(comparison.delta)] if not grown: return comparison sources = GitMeasure.sources(repo, head, paths & python_paths(repo, head, root)) @@ -2912,6 +3266,8 @@ def main() -> int: for identity, lines in sorted(result.introduced_report.items()): if lines: print(f"introduced {identity} at lines {lines}", file=sys.stderr) + for identity in result.missing_report: + print(f"missing introduced-site report for {identity}", file=sys.stderr) return int(result.failed)