From 6861733279e3bea6439540f59c915e43af0e9fcf Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:41:56 -0400 Subject: [PATCH 01/18] Delete commander capacity and rule-review scripts at their existing owners --- src/agent_comms/commander/capacity.py | 42 ------ src/agent_comms/commander/rule_file.py | 12 +- src/agent_comms/commander/rules.py | 57 -------- .../commander/rules/coordination.md | 2 +- src/agent_comms/commander/vibe_check.py | 36 ++++- src/agent_comms/commander_parity/load.py | 87 ------------- .../commander_parity/measurements.py | 23 ++-- .../commander_parity/observations.py | 123 ------------------ src/agent_comms/commander_parity/rows.py | 15 +-- .../skills/comms-commander/SKILL.md | 4 +- src/agent_comms/tools.py | 8 +- tests/test_commander_capacity.py | 4 +- tests/test_commander_forks.py | 4 +- tests/test_commander_parity_load.py | 57 -------- tests/test_commander_parity_measurements.py | 19 ++- 15 files changed, 81 insertions(+), 412 deletions(-) delete mode 100644 src/agent_comms/commander/capacity.py delete mode 100644 src/agent_comms/commander/rules.py delete mode 100644 src/agent_comms/commander_parity/load.py delete mode 100644 src/agent_comms/commander_parity/observations.py delete mode 100644 tests/test_commander_parity_load.py diff --git a/src/agent_comms/commander/capacity.py b/src/agent_comms/commander/capacity.py deleted file mode 100644 index 3694843f56..0000000000 --- a/src/agent_comms/commander/capacity.py +++ /dev/null @@ -1,42 +0,0 @@ -"""One commander load report for native tools and the commander's CLI.""" -from __future__ import annotations - -import time -from dataclasses import dataclass - -from ..comms import Comms -from ..field_codec import projected -from ..tool_result import ToolResult -from ..turn_digests import OrderEditCountRead - - -@dataclass(frozen=True) -class Load(ToolResult): - """Order and edit debt. Forking relieves it only by what forks clear: each verdict, order - or answer a fork records lowers the debt itself, never a division by how many forks exist.""" - owed: int - timers_up: int - unjudged: int - - @classmethod - def of(cls, comms: Comms, commander: str) -> Load: - snapshot, todos, now = comms.registry.snapshot(), comms.goals.todos, time.time() - owner = snapshot.require(commander) - held = todos.held_for_commander(owner.incarnation) - owed = tuple(t for t in held if t.pending) - unjudged = sum( - read.read() for read in OrderEditCountRead.for_orders(comms.root, snapshot, held) - ) - return cls(len(owed), sum(bool(t.timer_up(now)) for t in owed), unjudged) - - @projected(view="wire") - def total(self) -> float: - return self.owed + 2 * self.timers_up + self.unjudged / 10 - - def __str__(self) -> str: - return (f'load {self.total:.0f} ({self.owed} open orders, {self.timers_up} review timers up, ' - f'{self.unjudged} unjudged edits)') - - -def main(a, ctx): - print(Load.of(ctx.comms, ctx.commander)) diff --git a/src/agent_comms/commander/rule_file.py b/src/agent_comms/commander/rule_file.py index 4ef5b6e33f..dea3efd0d5 100644 --- a/src/agent_comms/commander/rule_file.py +++ b/src/agent_comms/commander/rule_file.py @@ -115,14 +115,8 @@ def drift_tells(cls) -> tuple[str, ...]: """The reminder hook's one-line tells, one declared by each packaged rule.""" return tuple(rule.drift for rule in cls.packaged()) - @abstractmethod - def review_rules(self) -> tuple[str, ...]: ... - - class FeedbackRuleFile(RuleFile): - - def review_rules(self) -> tuple[str, ...]: - return (self.summary,) + """A rule that supplies feedback in the commander's instructions.""" class HardRuleFile(FeedbackRuleFile, declared_name="hard-rule"): @@ -130,6 +124,4 @@ class HardRuleFile(FeedbackRuleFile, declared_name="hard-rule"): class ReferenceRuleFile(RuleFile): - - def review_rules(self) -> tuple[str, ...]: - return () + """Supporting instructions read alongside the feedback rules.""" diff --git a/src/agent_comms/commander/rules.py b/src/agent_comms/commander/rules.py deleted file mode 100644 index fdd45c4aa4..0000000000 --- a/src/agent_comms/commander/rules.py +++ /dev/null @@ -1,57 +0,0 @@ -"""Rule review for the commander: print every rule, nothing drawn, nothing stored. - -The rules have one source each: the commander skill's bold-lead lines, every packaged rule file -in `rules/`, and the OOPSLA paper's correct-maintenance results (the packaged correct-maintenance rule). - - agent-comms-commander rules review print every rule with its file:line -""" -import re -from pathlib import Path - -from agent_comms import field_codec - -from .config import CommanderConfig -from .order import Commander, CommanderVerb -from .rule_file import RuleFile - -PAPER = f'{CommanderConfig.packaged_rules()}/correct-maintenance.md' - - -def principles() -> list[str]: - """The OOPSLA paper's correct-maintenance results, the definitions every judgment is measured against.""" - text = RuleFile.read(Path(PAPER)).body - return [l[2:].strip() for l in text.splitlines() if l.startswith('- ') and len(l) > 40] - - -def rules() -> list[tuple[str, str]]: - """Every rule as (file:line, text).""" - skill = CommanderConfig.packaged_skill() - text = skill.read_text() - found = [(f'{skill.name}:{text.count(chr(10), 0, m.start()) + 1}', (m.group(1) + m.group(2)).strip()) - for m in re.finditer(r'^\s*- \*\*(.+?)\*\*(.*)$', text, re.M)] - for rule in RuleFile.packaged(): - found.extend((f'rules/{rule.name}.md:1', line) for line in rule.review_rules()) - return found - - -def review() -> None: - for i, (where, r) in enumerate(rules(), 1): - print(f'{i:3}. {where} {r}') - print('\nCorrect maintenance (the OOPSLA paper; every judgment is measured against these):') - for r in principles(): - print(f' - {r}') - - -class RulesVerb(CommanderVerb): - """Rules use the shared commander verb declaration.""" - - -class ReviewVerb(RulesVerb): - help = "print every standing rule" - - def run(self, ctx: Commander) -> None: - review() - - -def main(a, ctx): - RulesVerb.parse(a).apply(ctx) diff --git a/src/agent_comms/commander/rules/coordination.md b/src/agent_comms/commander/rules/coordination.md index ff5bae02fa..aba77b2ce6 100644 --- a/src/agent_comms/commander/rules/coordination.md +++ b/src/agent_comms/commander/rules/coordination.md @@ -10,7 +10,7 @@ type: hard-rule - Group problems by surface into one order per owner; no problem is ever without an owner. - **Units gather evidence; the commander diagnoses and shapes.** A unit reproduces, traces, measures and reports with file:line. The commander decides what the defect is and the exact shape of the fix (owner, parents, members, what gets deleted). A triage order classifies and reports only; any src edit goes out as a new order whose shape the commander wrote. -- Short fix: a Claude fork writes it, commits and pushes. Long fix: a bus unit with exact numbered edits. Running units scale with the work and with the commander's free load (`agent-comms-commander capacity`: owed orders, timers up, unjudged edits): start a unit only when a plan step is unblocked with no holder and the load leaves room to review it; stop one whose step waits, or when the load is past what the commander can judge. A stopped unit with nothing ready is correct; a unit stopped by anything but the commander (a crash, memory) is a defect. Tristan, 2026-10-21: "it should be scaled to how much free load the commander has and how much work is left". +- Short fix: a Claude fork writes it, commits and pushes. Long fix: a bus unit with exact numbered edits. Running units scale with the work and with the commander's free load (`comms_commander_load`: owed orders, timers up, unjudged edits): start a unit only when a plan step is unblocked with no holder and the load leaves room to review it; stop one whose step waits, or when the load is past what the commander can judge. A stopped unit with nothing ready is correct; a unit stopped by anything but the commander (a crash, memory) is a defect. Tristan, 2026-10-21: "it should be scaled to how much free load the commander has and how much work is left". - **The main loop dispatches; Claude forks diagnose and shape.** A Claude fork (a subagent of the commander, not a Codex `fork-*` unit) carries the commander's full context, so its diagnosis and fix shape are the commander's. Any work that would hold the main loop for more than a few tool calls goes to a fork the same tick: reading a unit's trace and writing the fix shape, reviewing a large diff, measuring a ratchet delta, debugging a test. The main loop keeps only what serializes: answering unit asks, merges, installs, strikes and recording deliveries (2026-10-21: three hours of zero forks while four units sat in standby waiting on the commander, and the main loop debugged a swallowed test print). - **Finished work never waits on the commander.** The tick a PR goes green it is enqueued; the tick a unit reports a delivery it is recorded; the tick a unit finishes or hands in a trace it gets its next order, while a fork does any review, manual merge or shape that work waits on. Installs follow ship-and-install.md. Tristan, 2026-10-21: "not leave work done blocked" (09:10 that day: #2430 green and clean with no auto-merge, toad#1115 waiting on a manual merge, #2431 and #2432 merged but not installed, four finished units in standby). - **Every unit ask is answered before the next rotation step.** A question, alert or handoff from a unit holding an order gets a disposition the same tick; a unit idling on the commander's paperwork is commander waste (2026-10-21: fork-boundaries' 8122 waited about seven hours). diff --git a/src/agent_comms/commander/vibe_check.py b/src/agent_comms/commander/vibe_check.py index 5a415f8d77..9c45c4ff0d 100644 --- a/src/agent_comms/commander/vibe_check.py +++ b/src/agent_comms/commander/vibe_check.py @@ -14,13 +14,13 @@ from dataclasses import dataclass, field from typing import ClassVar, Self -from agent_comms.commander.capacity import Load from agent_comms.bus import BusReader from agent_comms.commander.config import CommanderConfig from agent_comms.commander.questions import SenderMessages, UnansweredBurst from agent_comms.commander.standing import AnswersTo, PlanStalled from agent_comms.comms import Comms from agent_comms.declared_family import DeclaredFamily +from agent_comms.field_codec import projected from agent_comms.goal_actions import ( ChokeholdPausedGoalAction, CompliedGoalAction, @@ -29,15 +29,18 @@ from agent_comms.messages import Message from agent_comms.read_basis import RepliesDisplayScope from agent_comms.registry_document import RegistrySnapshot -from agent_comms.thread_identity import ThreadIncarnation -from agent_comms.todos import OwnerRequest, Todo from agent_comms.service_sender import ServiceSender from agent_comms.standing_rules import ( IndependentStandingRule, OwedAnswer, + ReviewTimerUp, StandingRule, UndatedStandingRule, ) +from agent_comms.thread_identity import ThreadIncarnation +from agent_comms.todos import OwnerRequest, Todo +from agent_comms.tool_result import ToolResult +from agent_comms.turn_digests import OrderEditCountRead class EscalationSender(ServiceSender): @@ -171,7 +174,8 @@ def doublings(held: float) -> int: return int(math.log2(max(held, PlanStalled.window) / PlanStalled.window)) -class OverloadDegradation(Degradation, BlocksRelease): +@dataclass(frozen=True) +class OverloadDegradation(Degradation, BlocksRelease, ToolResult): """More owed orders, expired timers and unjudged edits than one commander keeps up with. Forks relieve it by judging and ordering, which lowers the debt; load moves every beat, so it stops the commander once per round while it stays over.""" @@ -179,9 +183,31 @@ class OverloadDegradation(Degradation, BlocksRelease): ceiling: ClassVar[float] = 100.0 round_seconds: ClassVar[int] = 30 * 60 + owed: int + timers_up: int + unjudged: int + + @classmethod + def of(cls, comms: Comms, commander: str) -> Self: + return cls( + len(StandingRule.pending_todos(comms, commander)), + len(ReviewTimerUp.violations(comms, commander)), + OrderEditCountRead.count(comms, commander), + ) + + @projected(view="wire") + def total(self) -> float: + return self.owed + 2 * self.timers_up + self.unjudged / 10 + + def __str__(self) -> str: + return ( + f"load {self.total:.0f} ({self.owed} open orders, " + f"{self.timers_up} review timers up, {self.unjudged} unjudged edits)" + ) + @classmethod def found(cls, comms: Comms, commander: str) -> tuple[Sign, ...]: - load = Load.of(comms, commander) + load = cls.of(comms, commander) if load.total < cls.ceiling: return () return (Sign(cls, (int(time.time() // cls.round_seconds),), diff --git a/src/agent_comms/commander_parity/load.py b/src/agent_comms/commander_parity/load.py deleted file mode 100644 index 7f131371ae..0000000000 --- a/src/agent_comms/commander_parity/load.py +++ /dev/null @@ -1,87 +0,0 @@ -"""Observe the native load measurement used by the installed capacity command.""" - -from __future__ import annotations - -import asyncio -from dataclasses import dataclass -from pathlib import Path -from typing import ClassVar - -from ..child_process import BoundedRun -from ..commander.config import CommanderConfig -from ..field_codec import FieldCodec -from ..launch_runtime import LaunchRuntime -from ..private_path import FileRevision -from . import ParityComparison, UnverifiedParity -from .observations import CapacityObservation, MeasuredOutput - - -def state_files(directory: Path) -> tuple[tuple[str, FileRevision], ...]: - return tuple( - (str(path.relative_to(directory)), FileRevision.from_stat(path.stat())) - for path in sorted(directory.rglob("*")) - if path.is_file() - ) - - -@dataclass(frozen=True) -class CommandComparison(ParityComparison): - arguments: ClassVar[tuple[str, ...]] - timeout: ClassVar[int] = 30 - observation: MeasuredOutput - state_before: tuple[tuple[str, FileRevision], ...] - state_after: tuple[tuple[str, FileRevision], ...] - - @property - def passed(self) -> bool: - return ( - self.observation.current.strip() == self.observation.native - and self.state_before == self.state_after - ) - - def describe(self) -> str: - return ( - f"current={self.observation.current.strip()!r}; " - f"native={self.observation.native!r}; owner={self.observation.source}; " - f"commander={self.observation.commander}; root={self.observation.root}; " - f"private_state_unchanged={self.state_before == self.state_after}" - ) - - @classmethod - def read(cls) -> ParityComparison: - return cls.invoke(CommanderConfig.read(CommanderConfig.path()), cls.arguments) - - @classmethod - def invoke(cls, config: CommanderConfig, arguments: tuple[str, ...]) -> ParityComparison: - """Bind one native invocation and all its observations to the selected configuration.""" - before = state_files(config.state) - # -I uses installed imports even when this candidate runs with PYTHONPATH=src. - # HOME and the environment stay unchanged; the actual CLI main runs. - runtime = LaunchRuntime.running() - current = asyncio.run( - BoundedRun.run( - (str(runtime.interpreter), "-I", *arguments), - timeout=cls.timeout, - runtime=runtime, - ) - ) - if not current.outcome.successful: - return UnverifiedParity( - f"{cls.__name__}: configured native owner {current.outcome.summary}; " - f"bound={cls.timeout}s; {current.stderr.decode(errors='replace')}" - ) - observed = FieldCodec.decode(MeasuredOutput, FieldCodec.parse(current.stdout)) - source = Path(observed.source) - if ( - observed.root != str(config.root) - or observed.commander != config.commander - or not any(source.is_relative_to(repo.checkout) for repo in config.repos) - ): - return UnverifiedParity( - "The installed command used a different configuration or source" - ) - return cls(observed, before, state_files(config.state)) - - -class LoadComparison(CommandComparison): - arguments = CapacityObservation.arguments() diff --git a/src/agent_comms/commander_parity/measurements.py b/src/agent_comms/commander_parity/measurements.py index 40c4a6723b..dc2910238b 100644 --- a/src/agent_comms/commander_parity/measurements.py +++ b/src/agent_comms/commander_parity/measurements.py @@ -3,13 +3,14 @@ from __future__ import annotations from dataclasses import dataclass +from pathlib import Path from sys import monitoring from types import CodeType, MethodType from agent_comms.commander.config import CommanderConfig from agent_comms.commander_parity import ParityComparison, UnverifiedParity -from agent_comms.commander_parity.load import LoadComparison, state_files from agent_comms.comms import Comms +from agent_comms.private_path import FileRevision from agent_comms.standing_rules import DiskLow, StandingRule from agent_comms.storage_rules import StorageViolation from agent_comms.todos import ProgressRead @@ -24,6 +25,14 @@ class RuleComparison(ParityComparison): calls: int private_state_unchanged: bool + @staticmethod + def state_files(directory: Path) -> tuple[tuple[str, FileRevision], ...]: + return tuple( + (str(path.relative_to(directory)), FileRevision.from_stat(path.stat())) + for path in sorted(directory.rglob("*")) + if path.is_file() + ) + @property def passed(self) -> bool: return self.calls == 1 and self.current == self.native and self.private_state_unchanged @@ -47,7 +56,7 @@ def read(cls, owner: type[StandingRule]) -> ParityComparison: config = CommanderConfig.read(CommanderConfig.path()) comms = Comms(config.root) states = UnitState.current(comms, config.commander, ProgressRead()) - before = state_files(config.state) + before = cls.state_files(config.state) observed: list[tuple[StandingRule, ...]] = [] def observe(_code: CodeType, _offset: int, value: tuple[StandingRule, ...]) -> None: @@ -73,7 +82,7 @@ def observe(_code: CodeType, _offset: int, value: tuple[StandingRule, ...]) -> N tuple(rule.line() for rule in current), tuple(rule.line() for rule in native), len(observed), - before == state_files(config.state), + before == cls.state_files(config.state), ) @@ -92,11 +101,3 @@ def describe(self) -> str: @classmethod def read(cls) -> StorageComparison: return cls(RuleComparison.read(StorageViolation.scan_owner()), RuleComparison.read(DiskLow)) - - -def measured_load() -> ParityComparison: - observed = LoadComparison.read() - return UnverifiedParity( - f"Live Load comparison passed={observed.passed}: {observed.describe()}; " - "missing native self-report comparison owner" - ) diff --git a/src/agent_comms/commander_parity/observations.py b/src/agent_comms/commander_parity/observations.py deleted file mode 100644 index 11067be95c..0000000000 --- a/src/agent_comms/commander_parity/observations.py +++ /dev/null @@ -1,123 +0,0 @@ -"""Observe real native command returns and their actual output.""" - -from __future__ import annotations - -import argparse -import io -import json -import sys -from abc import abstractmethod -from collections.abc import Callable, Iterator -from contextlib import contextmanager, redirect_stdout -from dataclasses import dataclass -from pathlib import Path -from types import CodeType, MethodType - -from agent_comms.commander import capacity -from agent_comms.commander.cli import main as commander_main -from agent_comms.commander.cli import tool_name -from agent_comms.commander.config import CommanderConfig -from agent_comms.commander.order import Commander -from agent_comms.declared_family import DeclaredFamily -from agent_comms.field_codec import FieldCodec - - -@dataclass(frozen=True) -class MeasuredOutput: - current: str - native: str - source: str - root: str - commander: str - - -class NativeObservation(DeclaredFamily, affix="Observation"): - @classmethod - def arguments(cls) -> tuple[str, ...]: - return (str(Path(__file__).resolve()), cls.declared_name) - - @abstractmethod - def read(self, root: Path) -> MeasuredOutput: ... - - -def method_code(method: object) -> CodeType: - if not isinstance(method, MethodType): - raise TypeError("An observation requires a bound Python method") - return method.__func__.__code__ - - -@contextmanager -def returned_values(callbacks: dict[CodeType, Callable[[object], None]]) -> Iterator[None]: - """Observe only declared native returns, not every Python call in the command.""" - monitor = sys.monitoring - tool = monitor.PROFILER_ID - - def returned(code: CodeType, offset: int, value: object) -> None: - callbacks[code](value) - - monitor.use_tool_id(tool, "commander-parity-observation") - try: - monitor.register_callback(tool, monitor.events.PY_RETURN, returned) - for code in callbacks: - monitor.set_local_events(tool, code, monitor.events.PY_RETURN) - yield - finally: - for code in callbacks: - monitor.set_local_events(tool, code, 0) - monitor.register_callback(tool, monitor.events.PY_RETURN, None) - monitor.free_tool_id(tool) - - -class CapacityObservation(NativeObservation): - def read(self, root: Path) -> MeasuredOutput: - measured: list[capacity.Load] = [] - contexts: list[Commander] = [] - - def measurement(value: object) -> None: - if not isinstance(value, capacity.Load): - raise TypeError("Load.of returned outside its declared contract") - measured.append(value) - - def context(value: object) -> None: - if not isinstance(value, Commander): - raise TypeError("The commander CLI returned an undeclared context") - contexts.append(value) - - output = io.StringIO() - # This process owns its command line. The actual CLI selects its root and command. - sys.argv[1:] = ["--root", str(root), tool_name(capacity.__name__)] - with ( - returned_values( - { - method_code(capacity.Load.of): measurement, - method_code(Commander.read_root): context, - } - ), - redirect_stdout(output), - ): - commander_main() - if len(measured) != 1 or len(contexts) != 1: - raise ValueError("Capacity must use one native measurement and selected context") - selected = contexts[0] - return MeasuredOutput( - output.getvalue(), - str(measured[0]), - str(capacity.__file__), - str(selected.comms.root), - selected.commander, - ) - - -def main() -> None: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("observation", type=NativeObservation.member_named) - parser.add_argument( - "--root", type=Path, default=CommanderConfig.read(CommanderConfig.path()).root - ) - arguments = parser.parse_args() - observed = arguments.observation().read(arguments.root) - print(json.dumps(FieldCodec.encode(observed))) - - -if __name__ == "__main__": - main() diff --git a/src/agent_comms/commander_parity/rows.py b/src/agent_comms/commander_parity/rows.py index 1290b52ea1..bb7d9b6448 100644 --- a/src/agent_comms/commander_parity/rows.py +++ b/src/agent_comms/commander_parity/rows.py @@ -11,8 +11,7 @@ StrikeLimitCase, verify_order_progress, ) -from agent_comms.commander_parity.load import LoadComparison -from agent_comms.commander_parity.measurements import StorageComparison, measured_load +from agent_comms.commander_parity.measurements import StorageComparison from agent_comms.commander_parity.story import StoryCase from agent_comms.commander_parity.verdicts import VerdictComparison from agent_comms.commander_parity.wording import ( @@ -182,15 +181,15 @@ class RuleReviewParityRow(ParityRow): class LoadForkingParityRow(ParityRow): number = 17 capability = "Load and forking" - current = "`agent-comms-commander capacity`: load and live fork reports; ten-minute beat age" + current = "`comms_commander_load`: open orders, review timers and unjudged edits" native = ( - "Reporting-only `commander.capacity.Load.of`; no capacity refusal or automatic fork policy" + "`OverloadDegradation` reports load and raises overload; automatic forking is unverified" ) phases = (ControlPlanePhase,) @classmethod def verify(cls) -> ParityComparison: - return LoadComparison.read() + return UnverifiedParity("Automatic forking has no native comparison owner") class StorageHygieneParityRow(ParityRow): @@ -290,16 +289,16 @@ class MomentumParityRow(ParityRow): class MeasuredLoadParityRow(ParityRow): number = 28 capability = "Measured load and self-report" - current = "`capacity.py` unit count" + current = "`comms_commander_load` order and edit counts" native = ( - "`commander.capacity.Load.of` reports orders, review timers up and edits; " + "`OverloadDegradation.of` reports orders, review timers up and edits; " "self-report comparison unverified" ) phases = (ControlPlanePhase,) @classmethod def verify(cls) -> ParityComparison: - return measured_load() + return UnverifiedParity("Missing native self-report comparison owner") class FinishedWorkTransferParityRow(ParityRow): diff --git a/src/agent_comms/skills/comms-commander/SKILL.md b/src/agent_comms/skills/comms-commander/SKILL.md index 7eb86ba012..9ba0a225c7 100644 --- a/src/agent_comms/skills/comms-commander/SKILL.md +++ b/src/agent_comms/skills/comms-commander/SKILL.md @@ -24,7 +24,7 @@ All tools are `agent-comms-commander ...`, configured by `~/.config/agent ## Start of session, and after every compaction -- **The commander cannot trust itself to remember these rules.** Read in full, as files, before acting: `src/agent_comms/commander/indictment.md`, every file in `src/agent_comms/commander/rules/` (each fact has one owner there; this skill does not restate them), and the index of the pinned refactor-audit catalog (`src/agent_comms/skills/refactor-audit/references/patterns/README.md`); read a rung file when an order or roast cites one of its pattern IDs. The session hook rereads them at every start and compaction; `rules review` prints every rule with its file:line. A summary or memory note is a paraphrase that drifts and never counts. +- **The commander cannot trust itself to remember these rules.** Read in full, as files, before acting: `src/agent_comms/commander/indictment.md`, every file in `src/agent_comms/commander/rules/` (each fact has one owner there; this skill does not restate them), and the index of the pinned refactor-audit catalog (`src/agent_comms/skills/refactor-audit/references/patterns/README.md`); read a rung file when an order or roast cites one of its pattern IDs. The session hook supplies them at every start and compaction from their declared sources. A summary or memory note is a paraphrase that drifts and never counts. - **Memory files point at this skill; they never restate its rules.** Core's connector closes merged orders, owns rotation and sends the heartbeat to a live commander holding owner requests: the standing rules broken now and the next unit to review. Messages to the commander arrive through `agent-comms-claude-bridge watch` (the `comms` skill covers the first binding); `agent-comms-commander tool-watch` prints every native unit's tool calls per turn, its final words, and storage refusals. Neither replaces the heartbeat, and no other poll is kept beside it. @@ -94,7 +94,7 @@ rules/correct-maintenance.md owns the definition; rules/slop-and-polymorphism.md ## Forks, strikes and the record -- **"Fork" means a Claude subagent of the commander.** Codex units named `fork-*` are units, never forks. Fork reviews to parallelize them: each fork takes a slice of the unjudged edits or open PRs and records every judgment in Core with `judge ok`, `judge back` or `judge superseded` before it ends. A review that exists only as prose clears nothing, and the load keeps rising. Its #comms post (`order say '#comms' FILE --fork NAME`) announces what it recorded and its open items, so closing it loses nothing. `capacity` does not yet count forks (commander-native plan, phase 7). +- **"Fork" means a Claude subagent of the commander.** Codex units named `fork-*` are units, never forks. Fork reviews to parallelize them: each fork takes a slice of the unjudged edits or open PRs and records every judgment in Core with `judge ok`, `judge back` or `judge superseded` before it ends. A review that exists only as prose clears nothing, and the load keeps rising. Its #comms post (`order say '#comms' FILE --fork NAME`) announces what it recorded and its open items, so closing it loses nothing. `comms_commander_load` measures the work those forks clear, never divides the debt by their count. - **Compliance is not evidence, and neither is the unit's proof.** Judge the diff, then rerun the proof or watch the behaviour: a real message answered, a real turn, a practice install on a copy of the bus. A numeric check (`-> N`) is a target hit by the cheapest route; pair it with a behaviour check and read how the number moved. - **Undisclosed assumptions are false claims.** An assumption found in the diff that the report's `Assumed:` list left out is a strike. - **Strikes** (rules/send-backs-and-strikes.md owns when): `order strike UNIT PID FILE:LINE --why "..."` (10+ plain words) records it in Core and posts to #comms; `order fire UNIT --why` releases every problem the unit holds; `order unstrike PID --why` voids a strike recorded in error. `order done UNIT PROBLEM --sha` only after reading the diff that meets the order. diff --git a/src/agent_comms/tools.py b/src/agent_comms/tools.py index 7f055133eb..dc48c865a3 100644 --- a/src/agent_comms/tools.py +++ b/src/agent_comms/tools.py @@ -56,7 +56,7 @@ ThreadStoppedResult, ) from .cli_result import ToolCatalogResult -from .commander.capacity import Load +from .commander.vibe_check import OverloadDegradation from .coordination_tables.refusals import SendRefusal from .declared_family import DeclaredFamily from .display_order import ChannelSort, ThreadSort @@ -976,10 +976,10 @@ def apply(self, comms: Comms, caller: ToolCaller) -> UnjudgedEdits: @dataclass(frozen=True, kw_only=True) class CommsCommanderLoadTool(ThreadReadTool): label = "Measured commander load" - description = "Read canonical order/edit debt and receiving child commanders; never self-reported." + description = "Read open orders, review timers and unjudged edits used by the overload rule." - def apply(self, comms: Comms, caller: ToolCaller) -> Load: - return Load.of(comms, caller.subject(self.thread)) + def apply(self, comms: Comms, caller: ToolCaller) -> OverloadDegradation: + return OverloadDegradation.of(comms, caller.subject(self.thread)) @dataclass(frozen=True, kw_only=True) diff --git a/tests/test_commander_capacity.py b/tests/test_commander_capacity.py index e303057b78..7981046367 100644 --- a/tests/test_commander_capacity.py +++ b/tests/test_commander_capacity.py @@ -2,13 +2,13 @@ import pytest -from agent_comms.commander.capacity import Load +from agent_comms.commander.vibe_check import OverloadDegradation from agent_comms.field_codec import FieldCodec @pytest.mark.parametrize("unjudged", [0, 1, 395, 1_000_000]) def test_capacity_reports_load(unjudged): - load = Load(24, 14, unjudged) + load = OverloadDegradation(24, 14, unjudged) assert FieldCodec.encode(load)["total"] == load.total assert load.total == 24 + 2 * 14 + unjudged / 10 assert str(load) == ( diff --git a/tests/test_commander_forks.py b/tests/test_commander_forks.py index 11f28c77f3..c8345f45ab 100644 --- a/tests/test_commander_forks.py +++ b/tests/test_commander_forks.py @@ -35,7 +35,7 @@ from agent_comms.problem_number import ProblemNumber from agent_comms.thread_execution import ExternalThreadExecution from agent_comms.thread_identity import RecordedThreadIncarnation -from agent_comms.tools import CommsTodoListTool +from agent_comms.tools import CommsCommanderLoadTool, CommsTodoListTool from agent_comms.thread_model import DeclaredModel from agent_comms.todos import ( DeclarationDerivation, PendingProblemOrder, ProblemAllocation, ReleaseTodo, @@ -149,7 +149,7 @@ def test_registered_fork_sends_parent_problem_with_fork_actor(fork_bus, tmp_path assert set(ctx.model_copy(deep=True).progress()) == {'P40'} assert ctx.held_orders() == (saved,) assert ctx.call(CommsTodoListTool(thread=ctx.commander)).todos == (saved,) - assert "unjudged edits" in command(comms, 'capacity').stdout + assert "unjudged edits" in str(ctx.call(CommsCommanderLoadTool(thread=ctx.commander))) refused = command(comms, 'order', 'send', 'worker', body, '--fork', 'fork-two') assert refused.returncode != 0 and 'worker is assigned by fork-one' in refused.stderr assert store.get('P40') == saved diff --git a/tests/test_commander_parity_load.py b/tests/test_commander_parity_load.py deleted file mode 100644 index 1a5cfe9250..0000000000 --- a/tests/test_commander_parity_load.py +++ /dev/null @@ -1,57 +0,0 @@ -from dataclasses import replace - -import pytest - -from agent_comms.commander.capacity import Load -from agent_comms.commander.config import CommanderConfig -from agent_comms.commander_parity.load import CommandComparison, LoadComparison, state_files -from agent_comms.commander_parity.observations import MeasuredOutput - - -def test_load_comparison_requires_every_reported_field_and_no_private_write(tmp_path): - load = Load(37, 8, 1692, ("merger",)) - observed = MeasuredOutput(str(load) + "\n", str(load), "capacity.py", "/bus", "lead") - comparison = LoadComparison(observed, (), ()) - assert comparison.passed - for different in ( - replace(load, owed=38), - replace(load, timers_up=9), - replace(load, unjudged=1693), - replace(load, forks=()), - ): - assert not replace(comparison, observation=replace(observed, native=str(different))).passed - path = tmp_path / "old-state" - path.write_text("before") - before = state_files(tmp_path) - path.write_text("after") - after = state_files(tmp_path) - assert not LoadComparison(observed, before, after).passed - - -@pytest.mark.parametrize( - ("program", "expected"), - ( - ("raise SystemExit(7)", "exited with code 7"), - ("import time; time.sleep(10)", "timed out"), - ), -) -def test_failed_real_children_remain_named_unverified(tmp_path, program, expected): - """Negative process checks produce no fabricated native measurement or parity pass.""" - - class BriefComparison(CommandComparison): - timeout = 3 - - config = CommanderConfig( - commander="reader", - root=tmp_path / "bus", - state=tmp_path / "state", - repos=(), - worktrees=tmp_path / "projects", - scratch=tmp_path / "scratch", - disk=tmp_path, - ) - comparison = BriefComparison.invoke(config, ("-c", program)) - assert not comparison.passed - assert expected in comparison.describe() - assert "bound=3s" in comparison.describe() - assert LoadComparison.timeout == 30 diff --git a/tests/test_commander_parity_measurements.py b/tests/test_commander_parity_measurements.py index 73856410af..ecc8fa48ad 100644 --- a/tests/test_commander_parity_measurements.py +++ b/tests/test_commander_parity_measurements.py @@ -3,7 +3,11 @@ from dataclasses import replace from agent_comms.commander_parity.measurements import RuleComparison, StorageComparison -from agent_comms.commander_parity.rows import UnitTimeParityRow +from agent_comms.commander_parity.rows import ( + LoadForkingParityRow, + MeasuredLoadParityRow, + UnitTimeParityRow, +) def test_measurement_family_requires_native_calls_exact_delivery_and_no_private_write(): @@ -23,3 +27,16 @@ def test_measurement_family_requires_native_calls_exact_delivery_and_no_private_ missing = UnitTimeParityRow.verify() assert not missing.passed assert UnitTimeParityRow.native in missing.describe() + + +def test_a_load_report_does_not_prove_automatic_forking_or_self_report_comparison(): + for row in (LoadForkingParityRow, MeasuredLoadParityRow): + assert not row.verify().passed + + +def test_storage_comparison_notices_private_state_changes(tmp_path): + path = tmp_path / "private-state" + path.write_text("before") + before = RuleComparison.state_files(tmp_path) + path.write_text("after") + assert RuleComparison.state_files(tmp_path) != before From 427e1642e93c6d38a78a7c78316fa18e39b066dd Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:45:26 -0400 Subject: [PATCH 02/18] Keep hook qualification on valid commander configuration --- src/agent_comms/commander/rule_file.py | 1 + tests/test_commander_forks.py | 4 +++- tests/test_commander_skill_hook.py | 12 ++++++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/agent_comms/commander/rule_file.py b/src/agent_comms/commander/rule_file.py index dea3efd0d5..32967a635c 100644 --- a/src/agent_comms/commander/rule_file.py +++ b/src/agent_comms/commander/rule_file.py @@ -115,6 +115,7 @@ def drift_tells(cls) -> tuple[str, ...]: """The reminder hook's one-line tells, one declared by each packaged rule.""" return tuple(rule.drift for rule in cls.packaged()) + class FeedbackRuleFile(RuleFile): """A rule that supplies feedback in the commander's instructions.""" diff --git a/tests/test_commander_forks.py b/tests/test_commander_forks.py index c8345f45ab..4bd444d1a1 100644 --- a/tests/test_commander_forks.py +++ b/tests/test_commander_forks.py @@ -149,7 +149,9 @@ def test_registered_fork_sends_parent_problem_with_fork_actor(fork_bus, tmp_path assert set(ctx.model_copy(deep=True).progress()) == {'P40'} assert ctx.held_orders() == (saved,) assert ctx.call(CommsTodoListTool(thread=ctx.commander)).todos == (saved,) - assert "unjudged edits" in str(ctx.call(CommsCommanderLoadTool(thread=ctx.commander))) + load = ctx.call(CommsCommanderLoadTool(thread=ctx.commander)) + assert load.owed == 1 and load.timers_up == 0 and load.unjudged == 0 + assert "unjudged edits" in str(load) refused = command(comms, 'order', 'send', 'worker', body, '--fork', 'fork-two') assert refused.returncode != 0 and 'worker is assigned by fork-one' in refused.stderr assert store.get('P40') == saved diff --git a/tests/test_commander_skill_hook.py b/tests/test_commander_skill_hook.py index 39dcf7800a..961bbe39ff 100644 --- a/tests/test_commander_skill_hook.py +++ b/tests/test_commander_skill_hook.py @@ -162,8 +162,10 @@ def test_a_session_bound_to_another_bus_thread_gets_no_commander_sources(tmp_pat monkeypatch.setenv("HOME", str(tmp_path / "home")) config = CommanderConfig.path() config.parent.mkdir(parents=True) - config.write_text(json.dumps( - {"commander": "claude-code", "root": str(tmp_path), "state": str(tmp_path), "repos": []})) + config.write_text(CommanderConfig( + commander="claude-code", root=tmp_path, state=tmp_path, repos=(), + worktrees=tmp_path / "worktrees", scratch=tmp_path / "scratch", disk=tmp_path, + ).model_dump_json()) transcript = tmp_path / "session.jsonl" transcript.write_text("") record = {"hook_event_name": "SessionStart", "session_id": "s", "transcript_path": str(transcript)} @@ -178,8 +180,10 @@ def test_the_commander_session_gets_its_sources(tmp_path, monkeypatch): monkeypatch.setenv("HOME", str(tmp_path / "home")) config = CommanderConfig.path() config.parent.mkdir(parents=True) - config.write_text(json.dumps( - {"commander": "claude-code", "root": str(tmp_path), "state": str(tmp_path), "repos": []})) + config.write_text(CommanderConfig( + commander="claude-code", root=tmp_path, state=tmp_path, repos=(), + worktrees=tmp_path / "worktrees", scratch=tmp_path / "scratch", disk=tmp_path, + ).model_dump_json()) transcript = tmp_path / "session.jsonl" transcript.write_text("") WatchClaim("s").enrol(WatchRecord("claude-code", str(tmp_path), ())) From 51cd3580f9d6811bc49ba3ef195d31d09063cda8 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:46:28 -0400 Subject: [PATCH 03/18] Own the finished turn on the turn record (TurnStatePhase) A thread kept its turn twice: turn_record said whether a turn was live, and last_finished_turn_id beside it said which one finished, with a validator and four writers keeping the two in step (IDEN-5). The finished turn is now a FinishedTurn member of the TurnRecord family; the field, its validator and every writer that cleared it are gone. - release_turn records FinishedTurn(turn_id); lease_turn's StoredTurnRecord replaces it, so nothing clears it by hand. - TurnRecord.state() owns the TurnState capture; released() keeps a finished turn when the live reference is dropped; require_generation() refuses a finished turn without a prior generation. - A stale metadata writer now keeps the previous turn record whole instead of nulling the finished id. Carry: tools/cutover/carry_finished_turns.py moves each stored last_finished_turn_id into turn_record. On a copy of the live bus: 194 threads, 49 finished IDs carried, second dry run reports 0. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/check_root.py | 5 +- src/agent_comms/registration_change.py | 1 - src/agent_comms/registry_document.py | 6 +- src/agent_comms/registry_store.py | 1 - src/agent_comms/threads.py | 25 ++---- src/agent_comms/turn_lease.py | 50 +++++++++++- tests/goal_stop_journey.py | 12 +-- tests/test_goal_failure_observation.py | 4 +- tools/cutover/carry_finished_turns.py | 101 +++++++++++++++++++++++++ 9 files changed, 172 insertions(+), 33 deletions(-) create mode 100644 tools/cutover/carry_finished_turns.py diff --git a/src/agent_comms/check_root.py b/src/agent_comms/check_root.py index 6dd7f100c8..701e180564 100644 --- a/src/agent_comms/check_root.py +++ b/src/agent_comms/check_root.py @@ -29,6 +29,7 @@ from .session_file import SavedSessionFile from .threads import Thread from .transcript_routes import TranscriptRoutes +from .turn_lease import NoTurnRecord @dataclass(frozen=True) @@ -105,7 +106,8 @@ def _populate(self, copied: Comms) -> None: project.mkdir(parents=True, mode=0o700) session = self._session(original, copied.root, slot, project) declarations[name] = replace( - original.without_turn(), + original, + turn_record=NoTurnRecord(), worktree=str(project), parent=original.parent.kept_in(selected), task=NoThreadText(), @@ -116,7 +118,6 @@ def _populate(self, copied: Comms) -> None: project_resume_due=False, storage_roots=StorageRoots(copied.root / "scratch" / slot), turn_generation=0, - last_finished_turn_id=None, ) with copied.registry.store.editing() as edit: edit.document.restore_stopped(snapshot, names) diff --git a/src/agent_comms/registration_change.py b/src/agent_comms/registration_change.py index 3a8ac97d53..a0a797022f 100644 --- a/src/agent_comms/registration_change.py +++ b/src/agent_comms/registration_change.py @@ -88,7 +88,6 @@ def declared(self, requested: Thread) -> RegistrationChange: process_identity=self.previous.process_identity, turn_record=self.previous.turn_record, turn_generation=self.previous.turn_generation, - last_finished_turn_id=self.previous.last_finished_turn_id, ), ) return requested.process_identity.registration_change( diff --git a/src/agent_comms/registry_document.py b/src/agent_comms/registry_document.py index 2b3fb606db..532c40cdd4 100644 --- a/src/agent_comms/registry_document.py +++ b/src/agent_comms/registry_document.py @@ -56,7 +56,7 @@ ) from .thread_text import ForkedParent, NoParent from .threads import Thread -from .turn_lease import FinishedTurnFence, NoFinishedTurn, TurnCompletion, TurnLeaseFence +from .turn_lease import FinishedTurn, FinishedTurnFence, NoFinishedTurn, TurnCompletion, TurnLeaseFence if TYPE_CHECKING: from .activity import ObservedActivity @@ -700,8 +700,8 @@ def release_turn(self, lease: TurnLeaseFence) -> tuple[bool, TurnCompletion]: keeps_resume = turn.phase.keeps_project_resume record.without_claim() self.threads[name] = replace( - current.without_turn(), - last_finished_turn_id=(lease.turn_id if current.turn_generation else None), + current, + turn_record=FinishedTurn(lease.turn_id), project_resume_due=current.project_resume_due and keeps_resume, ) self.last_seen[name] = time.time() diff --git a/src/agent_comms/registry_store.py b/src/agent_comms/registry_store.py index 800a333472..2b50c5e26f 100644 --- a/src/agent_comms/registry_store.py +++ b/src/agent_comms/registry_store.py @@ -170,7 +170,6 @@ def lease_turn( leased = replace( current, turn_generation=identity.generation, - last_finished_turn_id=None, project_resume_due=False, turn_record=StoredTurnRecord.begin(self.store.path, identity, turn), ) diff --git a/src/agent_comms/threads.py b/src/agent_comms/threads.py index e01e1f3499..ac4c3e5a1e 100644 --- a/src/agent_comms/threads.py +++ b/src/agent_comms/threads.py @@ -55,7 +55,7 @@ TurnId, TurnIdentity, ) -from .turn_lease import NoTurnRecord, TurnLease, TurnLeaseFence, TurnRecord, TurnState +from .turn_lease import FinishedTurn, NoTurnRecord, TurnLease, TurnLeaseFence, TurnRecord, TurnState if TYPE_CHECKING: from .goal_management import Goals @@ -207,7 +207,6 @@ class Thread(DurableFile, PatternFile, ThreadProvenance): turn_record: TurnRecord = field(default_factory=NoTurnRecord) channel_scope_generation: int = 0 turn_generation: int = 0 - last_finished_turn_id: str | None = None def inherited_history(self, registry, visited): if self.name in visited: @@ -251,7 +250,7 @@ def owes_project_resume(self) -> bool: @property def turn_state(self) -> TurnState: - return TurnState(self.turn_record.read(), self.last_finished_turn_id) + return self.turn_record.state() def restart_candidates(self, status): return self.execution.restart_candidates(self, status) @@ -311,12 +310,7 @@ def __post_init__(self) -> None: raise ValueError("Channel scope generation must be a nonnegative 63-bit integer.") if type(self.turn_generation) is not int or not 0 <= self.turn_generation < 1 << 63: raise ValueError("Turn generation must be an exact nonnegative 63-bit integer.") - if self.last_finished_turn_id is not None and ( - type(self.last_finished_turn_id) is not str - or not self.last_finished_turn_id - or self.turn_generation == 0 - ): - raise ValueError("Finished turn requires a prior turn generation and ID.") + self.turn_record.require_generation(self.turn_generation) if self.parent.named(self.name): raise RelationViolationError(f"Thread {self.name!r} cannot be its own parent.") @@ -412,7 +406,7 @@ def same_turn(self, other: Thread) -> bool: def without_turn(self) -> Thread: """Drop the reference; the registry transaction owns revoking the record.""" - return replace(self, turn_record=NoTurnRecord()) + return replace(self, turn_record=self.turn_record.released()) def preserve_registration_history(self, previous: Thread) -> Thread: """Metadata writers cannot forge channel scope or reset a finished turn.""" @@ -424,11 +418,7 @@ def preserve_registration_history(self, previous: Thread) -> Thread: result = replace(self, created_at=previous.created_at, channel_scope_generation=scope) if self.turn_generation != previous.turn_generation: result = replace( - result, turn_generation=previous.turn_generation, - last_finished_turn_id=( - previous.last_finished_turn_id - if self.turn_record == previous.turn_record else None - ), + result, turn_generation=previous.turn_generation, turn_record=previous.turn_record, ) return result @@ -478,11 +468,12 @@ def has_observed_task_turn(self, identity: TurnIdentity, turn: TurnId) -> bool: return False return self.turn_lease.observes_task_turn( identity, turn, - idle=identity.generation == self.turn_generation and self.last_finished_turn_id == turn.value) + idle=identity.generation == self.turn_generation + and self.turn_record == FinishedTurn(turn.value)) def observed_turn(self, claim_scope: int) -> TurnFence | None: """Passive current/last-completed witness; never a begin-turn grant.""" - turn_id = self.turn_record.read().observed_id(self.last_finished_turn_id) + turn_id = self.turn_state.observed_id() if turn_id is None: return None return TurnFence(TurnIdentity(self.incarnation, self.turn_generation), turn_id, claim_scope) diff --git a/src/agent_comms/turn_lease.py b/src/agent_comms/turn_lease.py index bd9b8cdb4d..17c50c205f 100644 --- a/src/agent_comms/turn_lease.py +++ b/src/agent_comms/turn_lease.py @@ -12,6 +12,7 @@ from .activity import ObservedActivity from .errors import RelationViolationError from .field_codec import ( + MalformedRecord, NullMember, RecordMember, SingleFieldValue, @@ -313,8 +314,11 @@ def phase_effects(self, phase: TurnPhase) -> Iterator[TurnState]: def native_phase_effects(self, phase: TurnPhase) -> Iterator[TurnState]: yield from self.phase_effects(self.active.phase.observed(phase)) + def observed_id(self) -> str | None: + return self.active.observed_id(self.finished_turn_id) + def matches(self, turn_id: str | None) -> bool: - return turn_id is None or self.active.observed_id(self.finished_turn_id) == turn_id + return turn_id is None or self.observed_id() == turn_id class TurnLease(ABC): @@ -440,6 +444,16 @@ def without_claim(self) -> None: ... def lease(self, incarnation: ThreadIncarnation, process: ProcessState, generation: int) -> TurnLease: ... + def state(self) -> TurnState: + return TurnState(self.read()) + + def released(self) -> TurnRecord: + """What stays once the live reference is dropped.""" + return self + + def require_generation(self, generation: int) -> None: + pass + @dataclass(frozen=True) class NoTurnRecord(NullMember, TurnRecord): @@ -458,6 +472,37 @@ def lease(self, incarnation: ThreadIncarnation, process: ProcessState, return NoTurnLease() +@dataclass(frozen=True) +class FinishedTurn(RecordMember, TurnRecord): + """No live turn; the last one finished with this ID.""" + + turn_id: str + + def __post_init__(self) -> None: + if type(self.turn_id) is not str or not self.turn_id: + raise MalformedRecord("Finished turn requires its ID.") + + def bound_to(self, registry_path: Path, identity: TurnIdentity) -> FinishedTurn: + return self + + def read(self) -> NoTurn: + return NoTurn() + + def state(self) -> TurnState: + return TurnState(finished_turn_id=self.turn_id) + + def without_claim(self) -> None: + pass + + def lease(self, incarnation: ThreadIncarnation, process: ProcessState, + generation: int) -> NoTurnLease: + return NoTurnLease() + + def require_generation(self, generation: int) -> None: + if generation == 0: + raise ValueError("Finished turn requires a prior turn generation.") + + @dataclass(frozen=True) class StoredTurnReference(RecordMember, TurnRecord): """Wire presence only; the registry's thread supplies the turn identity.""" @@ -471,6 +516,9 @@ def read(self) -> TurnDeclaration: def without_claim(self) -> None: self.read() + def released(self) -> NoTurnRecord: + return NoTurnRecord() + def lease(self, incarnation: ThreadIncarnation, process: ProcessState, generation: int) -> TurnLease: raise RelationViolationError("Turn reference must be bound by its registry") diff --git a/tests/goal_stop_journey.py b/tests/goal_stop_journey.py index aee00b2905..9a05f7f0f4 100644 --- a/tests/goal_stop_journey.py +++ b/tests/goal_stop_journey.py @@ -64,11 +64,11 @@ async def stopped_goal_journey( assert resumed is not None and resumed.number == ready.number fixture.provider.response_gate.set() await wait_for(lambda: ( - bool(comms.registry.require("stop-copy").last_finished_turn_id) - and comms.registry.require("stop-copy").last_finished_turn_id - != stopped.last_finished_turn_id)) + bool(comms.registry.require("stop-copy").turn_state.finished_turn_id) + and comms.registry.require("stop-copy").turn_state.finished_turn_id + != stopped.turn_state.finished_turn_id)) continued = comms.registry.require("stop-copy") - assert continued.last_finished_turn_id + assert continued.turn_state.finished_turn_id with closing(store._connect()) as db: finished = AttemptRecord.one(db, attempt_id=resumed.attempt_id) assert finished is not None and isinstance(finished.phase, SucceededAttempt) @@ -82,8 +82,8 @@ async def stopped_goal_journey( "before": before, "after": after, "continued": FieldCodec.encode(continued.goal), "provider_posts": fixture.provider.posts, - "stopped_turn": stopped.last_finished_turn_id, - "continued_turn": continued.last_finished_turn_id, + "stopped_turn": stopped.turn_state.finished_turn_id, + "continued_turn": continued.turn_state.finished_turn_id, "failed_turn_observations": failures, "stopped_attempt_phase": FieldCodec.encode(attempt.phase), "continued_attempt_phase": FieldCodec.encode(finished.phase), diff --git a/tests/test_goal_failure_observation.py b/tests/test_goal_failure_observation.py index b5c90ec931..5b9e2c271d 100644 --- a/tests/test_goal_failure_observation.py +++ b/tests/test_goal_failure_observation.py @@ -19,7 +19,7 @@ from agent_comms.goal_generation import BlockedGeneration from agent_comms.goals import Goal from agent_comms.thread_identity import RecordedThreadIncarnation, TurnIdentity -from agent_comms.turn_lease import StoredTurnRecord, TurnLeaseFence +from agent_comms.turn_lease import FinishedTurn, StoredTurnRecord, TurnLeaseFence @pytest.fixture @@ -31,7 +31,7 @@ def bound(tmp_path, *, thread_registration): owner = thread_registration.new_thread( 'owner', frozenset(), '/private-worktree', process_identity=ProcessIdentity.capture(os.getpid()), created_at=10.0, - goal=goal, turn_generation=7, last_finished_turn_id='a' * 32, + goal=goal, turn_generation=7, turn_record=FinishedTurn('a' * 32), ) store.create_goal(goal.id) permit = store.claim_launch(store.reserve(goal.id, 1)) diff --git a/tools/cutover/carry_finished_turns.py b/tools/cutover/carry_finished_turns.py new file mode 100644 index 0000000000..fead5a2d7b --- /dev/null +++ b/tools/cutover/carry_finished_turns.py @@ -0,0 +1,101 @@ +"""Move each thread's finished turn ID into its turn record. + +Delete after every bus has run this step. The retired ``last_finished_turn_id`` +field is understood only here; the thread decoder refuses it. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from agent_comms.errors import RelationViolationError +from agent_comms.field_codec import FieldCodec +from agent_comms.historical_views import HistoryArchive +from agent_comms.installer.carry import Carried, Carry +from agent_comms.registry_store import RegistryStore +from agent_comms.store_files import _atomic_write_text, _store_lock + +from .carry_bus_history import CarryCatalogSource + +RETIRED = "last_finished_turn_id" + + +class FinishedTurns(Carry): + def backup_files(self) -> tuple[Path, ...]: + sources = CarryCatalogSource.read(self.root, self.original_root) + roots = ( + self.root, + *(source.mapped_root(self.root, self.original_root) for source in sources), + ) + paths = ( + HistoryArchive.path_in(self.root), + *(path for root in roots for path in RegistryStore.at(root).written), + ) + for path in paths: + self.require_confined(self.root, path) + return tuple(path for path in paths if path.exists()) + + def carry(self, *, apply: bool) -> Carried: + with _store_lock(HistoryArchive.path_in(self.root)): + self.backup_files() + sources = CarryCatalogSource.read(self.root, self.original_root) + results = [(self.root, self.carry_root(self.root, apply=apply))] + rebound = [] + resealed = False + for source in sources: + mapped = source.mapped_root(self.root, self.original_root) + result = self.carry_root(mapped, apply=apply) + results.append((mapped, result)) + if apply and result.rewritten: + source = source.carried_registry(mapped) + resealed = True + rebound.append(source) + rewritten = [ + str(root.relative_to(self.root) / name) + for root, result in results + for name in result.rewritten + ] + if resealed: + _atomic_write_text( + HistoryArchive.path_in(self.root), json.dumps(FieldCodec.encode(tuple(rebound))) + ) + rewritten.append(HistoryArchive.filename) + finished = sum(int(result.detail.get("finished", 0)) for _, result in results) + return Carried( + sum(result.changed for _, result in results), + {"finished": finished}, + tuple(rewritten), + ) + + def carry_root(self, root: Path, *, apply: bool) -> Carried: + registry = RegistryStore.at(root) + if not registry.path.exists(): + return Carried(0) + with registry.locked(): + registry.private_guard_unlocked() + document = FieldCodec.parse(registry.path.read_text()) + threads = document["threads"] + carried = finished = 0 + for name, thread in threads.items(): + if RETIRED not in thread: + continue + turn_id = thread.pop(RETIRED) + carried += 1 + if turn_id is None: + continue + if thread.get("turn_record") is not None: + raise RelationViolationError( + f"Thread {name!r} holds a live turn beside a finished one." + ) + thread["turn_record"] = {"turn_id": turn_id} + finished += 1 + if not carried: + return Carried(0) + raw = json.dumps(document) + registry._decode(FieldCodec.parse(raw)) + rewritten: tuple[str, ...] = () + if apply: + registry._write_unlocked(raw) + rewritten = tuple(str(path.relative_to(root)) for path in registry.written) + return Carried(carried, {"finished": finished}, rewritten) From d9791e599aa5d71c73d421f48adc9610d821000c Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:49:08 -0400 Subject: [PATCH 04/18] Join every owner request before an owner failure propagates Stack.owners used Executor.map, which cancels the requests not yet started the moment the first result raises, and drops every sibling failure but the first. Submit every request, let the pool exit join them all, then refuse naming each failed owner. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/installer/upgrade.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/installer/upgrade.py b/src/agent_comms/installer/upgrade.py index aef22907cd..0fc42e202c 100644 --- a/src/agent_comms/installer/upgrade.py +++ b/src/agent_comms/installer/upgrade.py @@ -584,9 +584,16 @@ def running_owners(self, root: Path) -> list[str]: def owners(self, root: Path, command: type[CliCommand[CliResult]], names: list[str]) -> None: - """Run the captured owner set together and join every request before proceeding.""" + """Run the captured owner set together and join every request before proceeding. + + Every request is submitted and the pool's exit joins them all; only then does a + failure propagate, naming each failed owner. ``Executor.map`` is not used: it + cancels the requests not yet started the moment the first result raises.""" with ThreadPoolExecutor(max_workers=max(1, len(names))) as pool: - list(pool.map(lambda name: self.cli(root, command, "--name", name), names)) + requests = [pool.submit(self.cli, root, command, "--name", name) for name in names] + if failures := [request.exception() for request in requests if request.exception()]: + raise UpgradeRefused("; ".join(str(failure.args[0]) for failure in failures) + ) from failures[0] def start(self, root: Path, names: list[str]) -> tuple[OwnerStart, ...]: """Start the captured owner set together; each owner's outcome is its own.""" From da8363979ebecca50b80dd9cb779ae1d8cb643b6 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:49:45 -0400 Subject: [PATCH 05/18] Centralize durable file publication in AtomicFile --- src/agent_comms/diagnostics.py | 12 ++-- src/agent_comms/exporting.py | 62 +++++------------- src/agent_comms/goal_attempts.py | 27 ++------ src/agent_comms/goal_history.py | 36 +++-------- src/agent_comms/harness_install.py | 7 +- src/agent_comms/historical_views.py | 7 +- src/agent_comms/installer/backup.py | 33 +++------- src/agent_comms/locked_store.py | 75 ++++++++-------------- src/agent_comms/maintenance_barrier.py | 6 +- src/agent_comms/native_package.py | 15 ++--- src/agent_comms/native_session_files.py | 15 ++--- src/agent_comms/private_registry_guard.py | 43 +++++-------- src/agent_comms/restart_queue.py | 13 ++-- src/agent_comms/selected_write_plan.py | 20 ++---- src/agent_comms/store_files.py | 78 ++++++++++++++++------- src/agent_comms/thread_management.py | 10 +-- src/agent_comms/threads.py | 26 +++++--- tests/test_atomic_file.py | 61 ++++++++++++++++++ tests/test_goal_history.py | 18 +++--- tests/test_registry_revision.py | 12 ++-- tools/cutover/carry_bus_history.py | 4 +- tools/cutover/carry_owner_requests.py | 6 +- 22 files changed, 274 insertions(+), 312 deletions(-) create mode 100644 tests/test_atomic_file.py diff --git a/src/agent_comms/diagnostics.py b/src/agent_comms/diagnostics.py index ef91cf1817..8aa0b78f3a 100644 --- a/src/agent_comms/diagnostics.py +++ b/src/agent_comms/diagnostics.py @@ -14,9 +14,11 @@ from typing import TYPE_CHECKING, Literal from uuid import uuid4 +from agent_comms.private_path import fsync_directory + from .field_codec import FieldCodec, Id128Text, MalformedRecord from .locked_store import DurableFile, PatternFile -from .store_files import _atomic_write_text +from .store_files import AtomicFile if TYPE_CHECKING: from .activity import DrainDiagnostic @@ -276,10 +278,6 @@ def write(cls, root: Path, name: str, document: dict) -> Path: path = cls.path_in(root, name=name) path.parent.mkdir(mode=0o700, exist_ok=True) if os.name == "posix": - fd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(fd) - finally: - os.close(fd) - _atomic_write_text(path, json.dumps(document, indent=2) + "\n", fsync_parent=True) + fsync_directory(root) + AtomicFile(path).write_text(json.dumps(document, indent=2) + "\n") return path.resolve() diff --git a/src/agent_comms/exporting.py b/src/agent_comms/exporting.py index 95296734ee..a4960abea6 100644 --- a/src/agent_comms/exporting.py +++ b/src/agent_comms/exporting.py @@ -6,7 +6,6 @@ import json import math import os -import tempfile from abc import abstractmethod from collections import deque from collections.abc import Callable, Iterable, Mapping @@ -19,9 +18,10 @@ from .cli_result import CliResult from .declared_family import DeclaredFamily from .field_codec import FieldCodec, NullMember, SingleFieldValue, SubclassUnion -from .messages import Message from .message_reference import MessageReference +from .messages import Message from .registration import Registration +from .store_files import AtomicFile, ExclusiveAtomicFile class WireExportFormat(DeclaredFamily, affix="Format"): @@ -444,35 +444,25 @@ def export( if not overwrite and os.path.lexists(destination): raise FileExistsError(f"Wire-export destination {str(destination)!r} already exists.") - descriptor, temporary = tempfile.mkstemp( - prefix=f".{destination.name}.", suffix=".tmp", dir=destination.parent - ) - temporary_path = Path(temporary) + artifact = AtomicFile(destination) if overwrite else ExclusiveAtomicFile(destination) stats = ExportSelectionStats() exported = 0 first_selected: Message | None = None last_selected: Message | None = None - try: - with os.fdopen(descriptor, "wb") as output: - writer = _ArtifactWriter(output) - writer.write(header) - rows = self._rows(messages, stats) - for row in self.limit.select( - rows, len(header), self.boundary.export_started_at, stats - ): - writer.write(row.data) - exported += 1 - if first_selected is None: - first_selected = row.message - last_selected = row.message - output.flush() - os.fsync(output.fileno()) - bytes_written = writer.size - output_sha256 = writer.digest.hexdigest() - - self._publish(temporary_path, destination, overwrite=overwrite) - finally: - temporary_path.unlink(missing_ok=True) + with artifact.staged() as output: + writer = _ArtifactWriter(output) + writer.write(header) + rows = self._rows(messages, stats) + for row in self.limit.select( + rows, len(header), self.boundary.export_started_at, stats + ): + writer.write(row.data) + exported += 1 + if first_selected is None: + first_selected = row.message + last_selected = row.message + bytes_written = writer.size + output_sha256 = writer.digest.hexdigest() omitted = stats.source_messages - exported return WireExportProvenance( @@ -529,21 +519,3 @@ def _validate_sequence(message: Message, previous: int) -> int: if message.seq <= previous: raise ValueError("Wire-export envelopes must have unique ascending sequences.") return message.seq - - @staticmethod - def _publish(temporary: Path, destination: Path, *, overwrite: bool) -> None: - if overwrite: - os.replace(temporary, destination) - else: - try: - os.link(temporary, destination) - except FileExistsError: - raise FileExistsError( - f"Wire-export destination {str(destination)!r} already exists." - ) from None - if os.name != "nt": - directory = os.open(destination.parent, os.O_RDONLY) - try: - os.fsync(directory) - finally: - os.close(directory) diff --git a/src/agent_comms/goal_attempts.py b/src/agent_comms/goal_attempts.py index 7a93d00ab2..728c880246 100644 --- a/src/agent_comms/goal_attempts.py +++ b/src/agent_comms/goal_attempts.py @@ -29,6 +29,7 @@ from uuid import uuid4 from agent_comms.goal_turn_permit import GoalTurnPermit +from agent_comms.private_path import fsync_directory from .declared_family import DeclaredFamily from .field_codec import MalformedRecord @@ -62,6 +63,7 @@ from .locked_store import DurableFile, PublishedSQLiteFile, WireWakeFile from .pi_counts import PiCount, UnreportedCount from .pi_payloads import PiUsage +from .store_files import AtomicFile from .typed_table import Column, Generated, SQLiteJournalMode, TypedTable if TYPE_CHECKING: @@ -710,12 +712,12 @@ def initialize(cls, root: str | Path) -> GoalAttemptStore: conn.execute("PRAGMA foreign_keys=ON") with cls.write_transaction(conn): GoalLedgerTable.create_schema(conn) - cls._sync_paths(staging, directory) + AtomicFile(staging).sync() # Publish only the complete schema; a concurrent initializer may win. # The staging descriptor is closed before its inode becomes shared. with suppress(FileExistsError): os.link(staging, path, follow_symlinks=False) - cls._sync_directory(directory) + fsync_directory(directory) except (sqlite3.Error, OSError) as error: raise StorageUncertainError("Goal attempt initialization is uncertain.") from error finally: @@ -753,27 +755,8 @@ def database(cls, path: Path) -> sqlite3.Connection: except sqlite3.Error as error: raise StorageUncertainError("Goal attempt database unavailable.") from error - @classmethod - def _sync_paths(cls, path: Path, directory: Path) -> None: - # Windows' CRT _commit (used by os.fsync) needs a writable handle. - file_fd = os.open(path, os.O_RDWR if os.name == "nt" else os.O_RDONLY) - try: - os.fsync(file_fd) - finally: - os.close(file_fd) - cls._sync_directory(directory) - - @staticmethod - def _sync_directory(directory: Path) -> None: - if os.name == "posix": - dir_fd = os.open(directory, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(dir_fd) - finally: - os.close(dir_fd) - def _sync(self) -> None: - self._sync_paths(self.path, self.root) + AtomicFile(self.path).sync() def _change( self, operation: Callable[[sqlite3.Connection], _T], verify: Callable[[_T], bool] diff --git a/src/agent_comms/goal_history.py b/src/agent_comms/goal_history.py index ddffceb111..ba46507c79 100644 --- a/src/agent_comms/goal_history.py +++ b/src/agent_comms/goal_history.py @@ -22,11 +22,17 @@ from agent_comms.field_codec import WireOmitDefault from agent_comms.owner_requests import NoRequestRevision, RequestAtRevision, RequestRevision -from .field_codec import FieldCodec, MalformedRecord, HistoryExclude +from .field_codec import FieldCodec, HistoryExclude, MalformedRecord from .goals import GoalDeclaration, NoGoal from .locked_store import DurableFile +from .store_files import AtomicFile from .typed_table import ( - Column, DurableTables, Index, JsonStorage, ReconciledFile, TypedTable, + Column, + DurableTables, + Index, + JsonStorage, + ReconciledFile, + TypedTable, ) @@ -169,27 +175,7 @@ def _connect(self) -> sqlite3.Connection: raise GoalHistoryError("Goal history database is unavailable.") from error def _sync(self) -> None: - self._sync_file(self.path) - if os.name == "posix": - self._sync_directory(self.path.parent) - - @staticmethod - def _sync_file(path: Path) -> None: - # Windows' CRT _commit (used by os.fsync) rejects a read-only fd. - # Retain the existing read-only POSIX reopen semantics. - descriptor = os.open(path, os.O_RDWR if os.name == "nt" else os.O_RDONLY) - try: - os.fsync(descriptor) - finally: - os.close(descriptor) - - @staticmethod - def _sync_directory(path: Path) -> None: - descriptor = os.open(path, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(descriptor) - finally: - os.close(descriptor) + AtomicFile(self.path).sync() def _initialize(self) -> None: try: @@ -262,9 +248,7 @@ def _sync_visible_registry(self) -> None: targets.append(guard) try: for target in targets: - self._sync_file(target) - if os.name == "posix": - self._sync_directory(self.registry_path.parent) + AtomicFile(target).sync() except OSError as error: raise GoalHistoryError("Cannot sync visible goal authority.") from error diff --git a/src/agent_comms/harness_install.py b/src/agent_comms/harness_install.py index 420cbe2353..fedb4a1bd8 100644 --- a/src/agent_comms/harness_install.py +++ b/src/agent_comms/harness_install.py @@ -16,6 +16,7 @@ from pathlib import Path from typing import TYPE_CHECKING, ClassVar +from agent_comms.private_path import fsync_directory from agent_comms_hooks import ClaudeHookProgram from .declared_family import DeclaredFamily @@ -34,11 +35,7 @@ def replace_link(link: Path, target: Path) -> None: try: temporary.symlink_to(target) os.replace(temporary, link) - directory = os.open(link.parent, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW) - try: - os.fsync(directory) - finally: - os.close(directory) + fsync_directory(link.parent) finally: temporary.unlink(missing_ok=True) diff --git a/src/agent_comms/historical_views.py b/src/agent_comms/historical_views.py index 8c175b58cd..26261f9da9 100644 --- a/src/agent_comms/historical_views.py +++ b/src/agent_comms/historical_views.py @@ -47,7 +47,7 @@ from .registration import Registration from .registry_provenance import RegistryProvenance from .revision_views import BusSourceView -from .store_files import _atomic_write_text, _store_lock +from .store_files import AtomicFile, _store_lock from .thread_identity import UnrecordedThreadIncarnation from .thread_provenance import ThreadProvenance @@ -505,10 +505,7 @@ def attach(self, source_root: Path, *, source_read: HistorySource | None = None) ) if not source.provenance.threads: raise ValueError("Historical source has no identity declarations") - _atomic_write_text( - self.path, - json.dumps([FieldCodec.encode(item) for item in (*sources, source)]), - ) + AtomicFile(self.path).write_text(json.dumps([FieldCodec.encode(item) for item in (*sources, source)])) return source except BaseException: shutil.rmtree(stage) diff --git a/src/agent_comms/installer/backup.py b/src/agent_comms/installer/backup.py index 05ead906f3..e4d8be0822 100644 --- a/src/agent_comms/installer/backup.py +++ b/src/agent_comms/installer/backup.py @@ -26,8 +26,8 @@ from agent_comms.field_codec import FieldCodec from agent_comms.installer.interpreter_operation import HoldTranscriptsProbe, RouteSelection from agent_comms.locked_store import LockedStore, RootFile, RuntimeFile, SQLiteFile -from agent_comms.private_path import FileObservation, FileRevision, MissingFile -from agent_comms.store_files import _atomic_write_text +from agent_comms.private_path import FileObservation, FileRevision, MissingFile, fsync_directory +from agent_comms.store_files import AtomicFile class UpgradeRefused(CoordinationError): @@ -205,11 +205,7 @@ def files(self) -> dict[str, str]: def retire(self) -> Path: archived = self.path.with_name(f'{self.path.name}.restored-{time.time_ns()}') self.path.rename(archived) - directory = os.open(archived.parent, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(directory) - finally: - os.close(directory) + fsync_directory(archived.parent) print(f'Restored data backup kept at {archived}', flush=True) return archived @@ -222,7 +218,7 @@ def restore_route(self) -> None: return saved = self.path / 'active-route.json' if saved.exists(): - _atomic_write_text(self.route, saved.read_text(), fsync_parent=True) + AtomicFile(self.route).write_text(saved.read_text()) else: os.unlink(self.route.name, dir_fd=directory) os.fsync(directory) @@ -269,14 +265,13 @@ def take(cls, root: Path, route: Path, files: tuple[Path, ...], spill_dir: Path) backup.route_revision = FileRevision.capture(route) return backup path.mkdir(mode=0o700, parents=True) - _atomic_write_text(path / 'selected.json', json.dumps(selected), fsync_parent=True) + AtomicFile(path / 'selected.json').write_text(json.dumps(selected)) contents = backup.capture(files) with route_directory(route): if route.exists(): shutil.copy2(route, path / 'active-route.json') backup.route_revision = FileRevision.capture(route) - _atomic_write_text(path / 'files.json', json.dumps(FieldCodec.encode(contents), indent=1), - fsync_parent=True) + AtomicFile(path / 'files.json').write_text(json.dumps(FieldCodec.encode(contents), indent=1)) return backup def retain(self, spill_dir: Path) -> tuple[Path, ...]: @@ -348,11 +343,7 @@ def capture(self, files: tuple[Path, ...]) -> dict[str, str]: with target.open('rb') as ready: os.fsync(ready.fileno()) contents[name] = self.digest(target) - directory = os.open(target.parent, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(directory) - finally: - os.close(directory) + fsync_directory(target.parent) return contents def file_original(self, name: str, saved: dict[str, str]) -> bool: @@ -384,20 +375,14 @@ def restore(self) -> None: stage = Path(temporary) try: shutil.copy2(self.data_root / name, stage) - with stage.open('rb') as ready: - os.fsync(ready.fileno()) - os.replace(stage, target) + AtomicFile(target).replace(stage) finally: stage.unlink(missing_ok=True) else: target.unlink(missing_ok=True) for companion in SQLiteFile.files(target)[1:]: companion.unlink(missing_ok=True) - directory = os.open(self.root, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(directory) - finally: - os.close(directory) + fsync_directory(self.root) self.restore_route() if not self.stores_original(): raise UpgradeRefused('Selected stores differ after backup restoration') diff --git a/src/agent_comms/locked_store.py b/src/agent_comms/locked_store.py index a99805b2bf..2107edead5 100644 --- a/src/agent_comms/locked_store.py +++ b/src/agent_comms/locked_store.py @@ -21,8 +21,15 @@ from .coordination_errors import CoordinationReadUnavailable from .field_codec import FieldCodec, MalformedRecord +from .private_path import PrivateFileRole, fsync_directory from .store_files import ( - SharedLock, StoreLock, StoreLockOwner, StoreOwner, _store_lock, store_lock_path, + AtomicFile, + SharedLock, + StoreLock, + StoreLockOwner, + StoreOwner, + _store_lock, + store_lock_path, ) T = TypeVar("T") @@ -85,58 +92,32 @@ def _write_unlocked(self, text: str) -> None: old-or-new semantics, not a multi-file transaction or recovery journal. """ fd, staged = StoreOwner.stage_file(self.path) + os.close(fd) + staged.unlink() backup = StoreOwner.rollback_file(staged) - directory_fd = None - published = False - had_original = False - keep_backup = False try: - with os.fdopen(fd, "w", encoding="utf-8") as output: - try: - mode = stat.S_IMODE(self.path.stat().st_mode) - except FileNotFoundError: - mode = 0o600 - output.write(text) - output.flush() - os.chmod(staged, mode) - os.fsync(output.fileno()) - if os.name == "posix": - directory_fd = os.open( - self.path.parent, os.O_RDONLY | os.O_DIRECTORY - ) + mode = stat.S_IMODE(self.path.stat().st_mode) + os.link(self.path, backup) + except FileNotFoundError: + mode = PrivateFileRole.permissions + try: try: - os.link(self.path, backup) - had_original = True - except FileNotFoundError: - pass - os.replace(staged, self.path) - published = True - if directory_fd is not None: - os.fsync(directory_fd) - # A failed backup cleanup is still a failed update: retain the old - # inode until unlink succeeds so the exception path can restore it. - backup.unlink(missing_ok=True) - except BaseException: - if published: - if had_original: - try: + AtomicFile(self.path, mode).write_text(text) + backup.unlink(missing_ok=True) + except BaseException: + if backup.exists(): + if not self.path.exists() or not self.path.samefile(backup): os.replace(backup, self.path) - except BaseException: - # Keep the recovery bytes if the filesystem also - # refuses rollback; never erase the remaining copy. - keep_backup = True - raise + fsync_directory(self.path.parent) else: - self.path.unlink() - if directory_fd is not None: - os.fsync(directory_fd) - raise + self.path.unlink(missing_ok=True) + fsync_directory(self.path.parent) + raise finally: - if directory_fd is not None: - os.close(directory_fd) - staged.unlink(missing_ok=True) - if not keep_backup: - backup.unlink(missing_ok=True) + # A failed restore leaves the last original inode for recovery. + if backup.exists() and self.path.exists() and self.path.samefile(backup): + backup.unlink() + class RootFile: diff --git a/src/agent_comms/maintenance_barrier.py b/src/agent_comms/maintenance_barrier.py index 767c8b4663..12d0ac506c 100644 --- a/src/agent_comms/maintenance_barrier.py +++ b/src/agent_comms/maintenance_barrier.py @@ -26,9 +26,9 @@ from .locked_store import DurableFile, LockedStore, RegistryFile from .send_outcome import GrantedSend, HeldSend from .store_files import ( + AtomicFile, SharedLock, _async_store_lock, - _atomic_write_text, _store_lock, ) @@ -407,8 +407,8 @@ def _write_unlocked(self, hold: MaintenanceState) -> None: marker_text = json.dumps(FieldCodec.encode(marker), sort_keys=True) state_text = json.dumps(FieldCodec.encode(hold), sort_keys=True) # The marker leads so an interrupted replacement still holds new turns. - _atomic_write_text(self.marker_path, marker_text, fsync_parent=True) - _atomic_write_text(self.state_path, state_text, fsync_parent=True) + AtomicFile(self.marker_path).write_text(marker_text) + AtomicFile(self.state_path).write_text(state_text) def begin(self, operator: MaintenanceOperator) -> MaintenanceState: """Hold new turns before fleet drain; an uncertain write stays closed.""" diff --git a/src/agent_comms/native_package.py b/src/agent_comms/native_package.py index 79938518ab..591b61ebcb 100644 --- a/src/agent_comms/native_package.py +++ b/src/agent_comms/native_package.py @@ -22,7 +22,9 @@ from agent_comms.field_codec import FieldCodec, MalformedRecord, PathText, Sha256Text, WireName from agent_comms.native_build_inputs import INPUTS_PREFIX, NativeBuildInputs -from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole +from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole, fsync_directory + +from .store_files import AtomicFile TREE_PREFIX = "# agent-comms-native-tree-v1 " MAX_ENTRIES = 30_000 @@ -310,8 +312,7 @@ def stage(self) -> Path: try: with os.fdopen(descriptor, "wb") as stream: stream.write(self.content) - stream.flush() - os.fsync(stream.fileno()) + os.chmod(temporary, 0o600) if temporary.stat().st_nlink != 1: raise NativePackageError("Native extension source staging file is shared") @@ -356,15 +357,11 @@ def install_declared_sources(root: Path, snapshot_root: Path) -> None: if not source.is_installed(): replacements.append((source.stage(), source)) for temporary, source in replacements: - os.replace(temporary, source.destination) + AtomicFile(source.destination).replace(temporary) for source in sources: source.verify_installed() for parent in {source.destination.parent for source in sources}: - descriptor = os.open(parent, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(descriptor) - finally: - os.close(descriptor) + fsync_directory(parent) except OSError as error: raise NativePackageError("Native extension source could not be installed") from error finally: diff --git a/src/agent_comms/native_session_files.py b/src/agent_comms/native_session_files.py index 940cad4730..fac9a7ff84 100644 --- a/src/agent_comms/native_session_files.py +++ b/src/agent_comms/native_session_files.py @@ -8,14 +8,15 @@ from pathlib import Path from typing import Self -from .errors import RelationViolationError -from .native_failures import NativePiUnavailable -from agent_comms.locked_store import DurableFile, PatternFile, SQLiteFile from agent_comms.field_codec import Id128Text -from agent_comms.participant_identity import ParticipantIdentity +from agent_comms.locked_store import DurableFile, PatternFile, SQLiteFile from agent_comms.native_session_reopen import NativeSessionIdentity +from agent_comms.participant_identity import ParticipantIdentity from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole, fsync_directory +from .errors import RelationViolationError +from .native_failures import NativePiUnavailable + class SessionFiles: """Declared session members share reference checks and deletion lifecycle.""" @@ -94,11 +95,7 @@ def delete(self) -> int: freed += path.lstat().st_size path.unlink() if self.session.parent.is_dir(): - directory = os.open(self.session.parent, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(directory) - finally: - os.close(directory) + fsync_directory(self.session.parent) return freed diff --git a/src/agent_comms/private_registry_guard.py b/src/agent_comms/private_registry_guard.py index ff5bf8733c..7db93224ca 100644 --- a/src/agent_comms/private_registry_guard.py +++ b/src/agent_comms/private_registry_guard.py @@ -210,7 +210,7 @@ def namespace(self) -> str: _reject("native namespace requires the original registry guard") def write(self, registry_path: Path, text: str) -> None: - store_files._atomic_write_text(registry_path, text, fsync_parent=True) + store_files.AtomicFile(registry_path).write_text(text) def register_restored(self, threads: Iterable[Thread]) -> None: """A public bus has no coordinator identities for restored threads.""" @@ -446,7 +446,7 @@ def write(self, registry_path: Path, text: str) -> None: # A pending guard after any failed write MUST remain fail-closed. digest = hashlib.sha256(b"present\0" + text.encode("utf-8")).digest() sequence, slot = self.prepare(digest) - store_files._atomic_write_text(registry_path, text, fsync_parent=True) + store_files.AtomicFile(registry_path).write_text(text) self.commit(sequence, slot, digest) def register_restored(self, threads: Iterable[Thread]) -> None: @@ -470,33 +470,21 @@ def create_pending(self) -> None: os.fsync(registry_fd) finally: os.close(registry_fd) - directory_fd = os.open( - self.registry_path.parent, os.O_RDONLY | os.O_DIRECTORY + fsync_directory(self.registry_path.parent) + fd = os.open( + self.path, + os.O_RDWR | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, + 0o600, ) try: - os.fsync(directory_fd) # baseline registry name and existing root - fd = os.open( - self.path, - os.O_RDWR | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, - 0o600, - ) - try: - if ( - os.pwrite( - fd, - _record(self.root_id, 0, _COMMITTED, digest) * 2, - 0, - ) - != 2 * _SIZE - ): - _reject("initial slots were incomplete") - os.fsync(fd) - os.fsync(directory_fd) # guard name is durable before native preparation returns - self._write(fd, 1, _record(self.root_id, 1, _PENDING, digest)) - finally: - os.close(fd) + if os.pwrite(fd, _record(self.root_id, 0, _COMMITTED, digest) * 2, 0) != 2 * _SIZE: + _reject("initial slots were incomplete") + os.fsync(fd) + fsync_directory(self.registry_path.parent) + self._write(fd, 1, _record(self.root_id, 1, _PENDING, digest)) finally: - os.close(directory_fd) + os.close(fd) + def commit_initial(self) -> None: fd = self._open() @@ -529,8 +517,7 @@ class PrivateOwnerRenameIntent(DurableFile): completed: bool = False def write(self, root: Path) -> None: - store_files._atomic_write_text(self.path_in(root), - json.dumps(FieldCodec.encode(self), sort_keys=True), fsync_parent=True) + store_files.AtomicFile(self.path_in(root)).write_text(json.dumps(FieldCodec.encode(self), sort_keys=True)) @classmethod def complete(cls, root: Path) -> None: diff --git a/src/agent_comms/restart_queue.py b/src/agent_comms/restart_queue.py index 8a61fef9f5..9d0c9e3d6e 100644 --- a/src/agent_comms/restart_queue.py +++ b/src/agent_comms/restart_queue.py @@ -19,16 +19,17 @@ from pathlib import Path from typing import TYPE_CHECKING, Annotated +from agent_comms.private_path import fsync_directory from .child_process import OutlivesLauncherLifetime, ProcessIdentity, SpawnedProcess from .comms import Comms, wire from .coordination_errors import CoordinationError from .coordinator import Coordination from .declared_family import DeclaredFamily -from .lifecycle_capability import Active, LifecycleCapability, Pending from .errors import RelationViolationError from .field_codec import FieldCodec, WireOmitDefault from .launch_runtime import LaunchRuntime, RestartEnvironment +from .lifecycle_capability import Active, LifecycleCapability, Pending from .native_arguments import GivenBinary, NativeArguments from .owner_cutover import PreserveOwnerRuntime from .owner_launch import RetainedOwnerLaunch @@ -37,7 +38,7 @@ from .private_path import PrivateDirectoryRole from .restart_refusals import RestartRefusal from .restart_scope import OwnersNamed -from .store_files import _atomic_write_text, _store_lock +from .store_files import AtomicFile, _store_lock from .thread_identity import RecordedThreadIncarnation from .watched import ChangeSignal, DirectorySource, FileSource @@ -241,15 +242,11 @@ def move(self, path: Path, record: QueuedRestart) -> None: raise FileExistsError(destination) os.rename(path, destination) for parent in (path.parent, destination.parent): - descriptor = os.open(parent, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(descriptor) - finally: - os.close(descriptor) + fsync_directory(parent) def save(self, record: QueuedRestart) -> None: path = self.active_path(record.selection.identity.incarnation.require_recorded()) - _atomic_write_text(path, json.dumps(FieldCodec.encode(record), sort_keys=True), fsync_parent=True) + AtomicFile(path).write_text(json.dumps(FieldCodec.encode(record), sort_keys=True)) if not record.state.items_with(Active, record): self.move(path, record) diff --git a/src/agent_comms/selected_write_plan.py b/src/agent_comms/selected_write_plan.py index 5e6d2ee2b3..ea4eaf62b6 100644 --- a/src/agent_comms/selected_write_plan.py +++ b/src/agent_comms/selected_write_plan.py @@ -12,14 +12,14 @@ import hashlib import json import os -import secrets from contextlib import suppress from dataclasses import dataclass, fields from pathlib import Path -from typing import TYPE_CHECKING, Literal, Annotated +from typing import TYPE_CHECKING, Annotated, Literal from .coordination_database import CoordinationStore from .locked_store import DurableFile, PatternFile +from .store_files import AtomicFile if TYPE_CHECKING: from .acp_extension import SelectedWriteAcceptedUpdate @@ -268,16 +268,6 @@ def applied(self, assignment: WakeAssignment, owner: Thread, operation_id: str) if accepted.operation_id != operation_id: raise IdentityConflict("Selected write intent is no longer unique") completed = accepted.applied() - temporary = path.with_name(f".{path.name}.{secrets.token_hex(8)}.tmp") - fd = os.open(temporary, os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, 0o600) - try: - with os.fdopen(fd, "wb") as stream: - stream.write(json.dumps(FieldCodec.encode(completed), sort_keys=True, separators=(",", ":")).encode()) - stream.flush() - os.fsync(stream.fileno()) - os.replace(temporary, path) - fsync_directory(self.directory) - except BaseException: - # A post-write failure is UNKNOWN. The bus claim and attempt - # remain visible; no path automatically reissues the write. - raise + AtomicFile(path).write_bytes(json.dumps( + FieldCodec.encode(completed), sort_keys=True, separators=(",", ":") + ).encode()) diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index 042eb4cb50..909e267a6b 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -29,7 +29,7 @@ from .child_process import Platform from .declared_family import DeclaredFamily from .errors import RelationViolationError -from .private_path import FileIdentity, PrivateFileRole +from .private_path import FileIdentity, PrivateFileRole, fsync_directory with suppress(ModuleNotFoundError): # Windows; Win32Platform never asks a mode for its flock. import fcntl @@ -432,28 +432,60 @@ def _async_store_lock( ) -def _atomic_write_text( - path: Path, text: str, *, fsync_parent: bool = False, - mode: int = PrivateFileRole.permissions, -) -> None: - """Replace a snapshot; private guarded writes also durably sync its name.""" - path.parent.mkdir(parents=True, exist_ok=True) - fd, temporary_path = StoreOwner.stage_file(path) - try: - with os.fdopen(fd, "w", encoding="utf-8") as output: - os.chmod(temporary_path, mode) - output.write(text) - output.flush() - os.fsync(output.fileno()) - os.replace(temporary_path, path) - if fsync_parent: - directory_fd = os.open(path.parent, os.O_RDONLY | os.O_DIRECTORY) - try: - os.fsync(directory_fd) - finally: - os.close(directory_fd) - finally: - temporary_path.unlink(missing_ok=True) +@dataclass(frozen=True) +class AtomicFile: + """A staged replacement always syncs its contents and directory entry.""" + + path: Path + mode: int = PrivateFileRole.permissions + + @contextmanager + def staged(self) -> Iterator[BinaryIO]: + self.path.parent.mkdir(parents=True, exist_ok=True) + descriptor, temporary = StoreOwner.stage_file(self.path) + try: + with os.fdopen(descriptor, "wb") as output: + os.chmod(temporary, self.mode) + yield output + output.flush() + os.fsync(output.fileno()) + self._publish(temporary) + finally: + temporary.unlink(missing_ok=True) + + def write_text(self, text: str) -> None: + self.write_bytes(text.encode("utf-8")) + + def write_bytes(self, data: bytes) -> None: + with self.staged() as output: + output.write(data) + + def replace(self, staged: Path) -> None: + """Publish an independently prepared file after syncing its data.""" + with staged.open("rb") as source: + os.fsync(source.fileno()) + self._publish(staged) + + def _publish(self, staged: Path) -> None: + self._install(staged) + fsync_directory(self.path.parent) + + def _install(self, staged: Path) -> None: + os.replace(staged, self.path) + + def sync(self) -> None: + """Sync an existing file without replacing an open database's inode.""" + # Windows' CRT needs a writable handle; these files are owned writes. + with self.path.open("rb+") as source: + os.fsync(source.fileno()) + fsync_directory(self.path.parent) + + +class ExclusiveAtomicFile(AtomicFile): + """Create a new name atomically, refusing an existing destination.""" + + def _install(self, staged: Path) -> None: + os.link(staged, self.path) def _repair_trailing_jsonl(path: Path) -> None: diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index 7e1d4aec59..ad643618ac 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -30,8 +30,8 @@ from .catalog_store import ChannelCatalog from .collaboration_ledger import CollaborationLedger from .errors import RelationViolationError -from .importing import ImportLimits, ImportSnapshot, ThreadImported from .field_codec import FieldCodec, NullMember, RecordMember, SingleFieldValue, SubclassUnion +from .importing import ImportLimits, ImportSnapshot, ThreadImported from .launch_runtime import LaunchRuntime, TagListText from .message_bus import MessageBus from .native_arguments import ConfiguredBinary @@ -43,13 +43,13 @@ ) from .registry_document import RegistrySnapshot from .registry_provenance import RegistryNames -from .store_files import _atomic_write_text -from .thread_identity import AgentThreadRole, ThreadRole -from .thread_text import ForkedParent, GivenThreadText, NoThreadText, ThreadText from .session_file import SavedSessionFile +from .store_files import AtomicFile +from .thread_identity import AgentThreadRole, ThreadRole from .thread_model import DeclaredModel, ThreadModel, UndeclaredModel from .thread_owned_state import ThreadOwnedState from .thread_rename import RenameThreadResult, ThreadRename +from .thread_text import ForkedParent, GivenThreadText, NoThreadText, ThreadText from .threads import KeptModel, ModelChoice, NextTurnConfiguration, Thread, current_thread @@ -258,7 +258,7 @@ def import_thread( or stat.S_IMODE(info.st_mode) != 0o700 ): raise ValueError("Imported session directory is not owner-controlled.") - _atomic_write_text(session_path, snapshot.pi_session(project)) + AtomicFile(session_path).write_text(snapshot.pi_session(project)) try: self.registry._declare_unlocked(thread, StoppedThreadStatus()) self.bus.mark_delivered_through(name, self.bus.bus.revision()) diff --git a/src/agent_comms/threads.py b/src/agent_comms/threads.py index e01e1f3499..7ed9c60095 100644 --- a/src/agent_comms/threads.py +++ b/src/agent_comms/threads.py @@ -8,8 +8,7 @@ from abc import ABC, abstractmethod from dataclasses import dataclass, field, replace from pathlib import Path -from typing import TYPE_CHECKING, ClassVar, Annotated - +from typing import TYPE_CHECKING, Annotated, ClassVar from .agent_backend import AgentBackend, NewThreadBackend from .child_process import NoProcess, ProcessIdentity, ProcessState @@ -17,8 +16,15 @@ from .context_segments.wake_values import WakeWorkContext from .errors import RelationViolationError from .field_codec import ( - Argument, Description, FieldCodec, NullMember, RegistrationPolicy, - SingleFieldValue, SubclassUnion, WireOmitDefault, WireRequired, + Argument, + Description, + FieldCodec, + NullMember, + RegistrationPolicy, + SingleFieldValue, + SubclassUnion, + WireOmitDefault, + WireRequired, ) from .goals import ( GoalDeclaration, @@ -40,14 +46,11 @@ InheritEmpty, InheritPrevious, ) +from .session_file import NoSessionFile, SessionFile from .storage_rules import StoragePath, StorageRoots, StorageRule -from .store_files import _atomic_write_text +from .store_files import AtomicFile from .task_sources import TaskScope from .thread_execution import NativeThreadExecution, ThreadExecution -from .thread_model import DeclaredModel, ThreadModel, UndeclaredModel -from .thread_text import NoParent, NoThreadText, ThreadParent, ThreadText -from .session_file import NoSessionFile, SessionFile -from .thread_provenance import ThreadProvenance from .thread_identity import ( AgentThreadRole, ThreadRole, @@ -55,6 +58,9 @@ TurnId, TurnIdentity, ) +from .thread_model import DeclaredModel, ThreadModel, UndeclaredModel +from .thread_provenance import ThreadProvenance +from .thread_text import NoParent, NoThreadText, ThreadParent, ThreadText from .turn_lease import NoTurnRecord, TurnLease, TurnLeaseFence, TurnRecord, TurnState if TYPE_CHECKING: @@ -284,7 +290,7 @@ def native_launch_arguments( catalog = ThreadToolCatalogResult( tuple(NativeRequestTools.catalog()), self.service_tier, launch.arguments(), ) - _atomic_write_text(path, json.dumps(catalog.to_wire(), separators=(",", ":"))) + AtomicFile(path).write_text(json.dumps(catalog.to_wire(), separators=(",", ":"))) return (*arguments.argv, "--comms-catalog", str(path)) def launch(self) -> ThreadLaunch: diff --git a/tests/test_atomic_file.py b/tests/test_atomic_file.py new file mode 100644 index 0000000000..1e21825326 --- /dev/null +++ b/tests/test_atomic_file.py @@ -0,0 +1,61 @@ +"""File contents and directory names become durable through the same writer.""" + +import os +import stat + +import pytest + +from agent_comms.store_files import AtomicFile, ExclusiveAtomicFile + + +def test_text_and_binary_writes_sync_contents_then_the_directory(tmp_path, monkeypatch): + target = tmp_path / "value" + synced = [] + fsync = os.fsync + + def observed(descriptor): + synced.append(stat.S_ISDIR(os.fstat(descriptor).st_mode)) + fsync(descriptor) + + monkeypatch.setattr(os, "fsync", observed) + AtomicFile(target, mode=0o640).write_text("first α") + assert target.read_text() == "first α" + assert stat.S_IMODE(target.stat().st_mode) == 0o640 + AtomicFile(target).write_bytes(b"second") + assert target.read_bytes() == b"second" + assert synced == [False, True, False, True] + + +def test_failed_stream_preparation_keeps_the_old_file(tmp_path): + target = tmp_path / "value" + target.write_bytes(b"original") + with pytest.raises(ValueError, match="stream failed"): + with AtomicFile(target).staged() as output: + output.write(b"partial") + raise ValueError("stream failed") + assert target.read_bytes() == b"original" + assert set(tmp_path.iterdir()) == {target} + + +def test_parent_sync_failure_is_reported_after_the_visible_replace(tmp_path, monkeypatch): + target = tmp_path / "value" + fsync = os.fsync + + def failed_directory(descriptor): + if stat.S_ISDIR(os.fstat(descriptor).st_mode): + raise OSError("directory sync failed") + fsync(descriptor) + + monkeypatch.setattr(os, "fsync", failed_directory) + with pytest.raises(OSError, match="directory sync failed"): + AtomicFile(target).write_bytes(b"visible but not attested durable") + assert target.read_bytes() == b"visible but not attested durable" + + +def test_exclusive_publication_never_replaces_an_existing_destination(tmp_path): + target = tmp_path / "value" + ExclusiveAtomicFile(target).write_text("original") + with pytest.raises(FileExistsError): + ExclusiveAtomicFile(target).write_text("replacement") + assert target.read_text() == "original" + assert set(tmp_path.iterdir()) == {target} diff --git a/tests/test_goal_history.py b/tests/test_goal_history.py index 0b74baa703..3326c09830 100644 --- a/tests/test_goal_history.py +++ b/tests/test_goal_history.py @@ -9,7 +9,6 @@ import agent_comms.store_files as store_files from agent_comms.comms import Comms -from conftest import installed_comms from agent_comms.goal_actions import ( ClearGoalAction, CompletedGoalAction, @@ -18,10 +17,11 @@ SetGoalAction, ) from agent_comms.goal_history import GoalHistoryError, GoalHistoryStore -from agent_comms.goals import NoGoal from agent_comms.goal_states import ActiveGoal -from agent_comms.tools import ToolRequest +from agent_comms.goals import NoGoal from agent_comms.tool_caller import ThreadCaller +from agent_comms.tools import ToolRequest +from conftest import installed_comms def _wire(tmp_path) -> Comms: @@ -34,7 +34,7 @@ def test_history_file_fsync_reopens_with_supported_platform_flags(tmp_path): path = tmp_path / "history.sqlite3" path.write_bytes(b"journal bytes") # Windows' os.fsync delegates to the CRT, which rejects O_RDONLY fds. - GoalHistoryStore._sync_file(path) + store_files.AtomicFile(path).sync() assert path.read_bytes() == b"journal bytes" @@ -105,15 +105,15 @@ def test_existing_goal_seeds_only_current_observed_baseline(tmp_path): def test_crash_before_registry_write_does_not_expose_history_intent(tmp_path, monkeypatch): comms = _wire(tmp_path) current = comms.goals.update_goal("worker", SetGoalAction(text="Keep current")) - original_write = store_files._atomic_write_text + original_replace = store_files.os.replace - def failed_registry_write(path, text, *, fsync_parent=False): - if path == comms.registry.store.path: + def failed_registry_write(source, target, *args, **kwargs): + if target == comms.registry.store.path: raise OSError("simulated registry write failure") - return original_write(path, text, fsync_parent=fsync_parent) + return original_replace(source, target, *args, **kwargs) with monkeypatch.context() as patch: - patch.setattr(store_files, "_atomic_write_text", failed_registry_write) + patch.setattr(store_files.os, "replace", failed_registry_write) with pytest.raises(OSError): comms.goals.update_goal( "worker", PausedGoalAction(expect=GoalPrecondition(goal_id=current.id)) diff --git a/tests/test_registry_revision.py b/tests/test_registry_revision.py index 58959eaddd..d90726c31d 100644 --- a/tests/test_registry_revision.py +++ b/tests/test_registry_revision.py @@ -6,7 +6,7 @@ from agent_comms.field_codec import MalformedRecord from agent_comms.registration import Registration -from agent_comms.store_files import _atomic_write_text +from agent_comms.store_files import AtomicFile from agent_comms.thread_status import StoppedThreadStatus @@ -16,11 +16,11 @@ def test_malformed_replacement_never_serves_stale_success(tmp_path): registry.register(registry.new_thread(name='worker', tags=frozenset(), worktree=str(tmp_path))) saved = path.read_text() registry.require("worker") - _atomic_write_text(path, "{") + AtomicFile(path).write_text("{") for _ in range(2): with pytest.raises(MalformedRecord): registry.require("worker") - _atomic_write_text(path, saved) + AtomicFile(path).write_text(saved) assert registry.require("worker").name == "worker" @@ -41,7 +41,7 @@ def test_a_new_revision_decodes_only_the_threads_that_changed(tmp_path): stopped.register(stopped.new_thread(name='second', tags=frozenset(), worktree=str(tmp_path))) stopped.unregister("second") raw["statuses"]["second"] = json.loads(stopped.store.path.read_text())["statuses"]["second"] - _atomic_write_text(path, json.dumps(raw, indent=2)) + AtomicFile(path).write_text(json.dumps(raw, indent=2)) after = reader.all_threads() # Unchanged threads keep their decoded declarations and statuses. @@ -65,12 +65,12 @@ def test_a_changed_revision_is_checked_as_a_whole(tmp_path): # A declaration without its status: the unchanged rows do not excuse it. raw = json.loads(path.read_text()) del raw["statuses"]["second"] - _atomic_write_text(path, json.dumps(raw, indent=2)) + AtomicFile(path).write_text(json.dumps(raw, indent=2)) with pytest.raises(RelationViolationError): registry.require("first") # A changed row that does not decode is rejected too. raw = json.loads(path.read_text()) raw["statuses"]["second"] = {"kind": "no-such-status"} - _atomic_write_text(path, json.dumps(raw, indent=2)) + AtomicFile(path).write_text(json.dumps(raw, indent=2)) with pytest.raises(RelationViolationError): registry.require("first") diff --git a/tools/cutover/carry_bus_history.py b/tools/cutover/carry_bus_history.py index 19ec5a9327..e22e44881d 100644 --- a/tools/cutover/carry_bus_history.py +++ b/tools/cutover/carry_bus_history.py @@ -55,7 +55,7 @@ from agent_comms.registry_provenance import RegistryProvenance from agent_comms.registry_store import RegistryStore from agent_comms.response_acknowledgement import KeyedResponseAcknowledgement -from agent_comms.store_files import SharedLock, _atomic_write_text, _store_lock +from agent_comms.store_files import AtomicFile, SharedLock, _store_lock from agent_comms.typed_table import Column, DerivedTables, SharedFileTables, TypedTable from agent_comms.wake_policy import ( MessageAudience, @@ -149,7 +149,7 @@ def publish( payload = json.dumps(FieldCodec.encode(converted)) if path.read_bytes() == payload.encode(): return Carried(changed, rewritten=rewritten) - _atomic_write_text(path, payload) + AtomicFile(path).write_text(payload) return Carried(changed + len(converted), rewritten=(*rewritten, HistoryArchive.filename)) def mapped_root(self, root: Path, original_root: Path) -> Path: diff --git a/tools/cutover/carry_owner_requests.py b/tools/cutover/carry_owner_requests.py index 386d24c179..4f583b76e1 100644 --- a/tools/cutover/carry_owner_requests.py +++ b/tools/cutover/carry_owner_requests.py @@ -29,7 +29,7 @@ RequestDirective, ) from agent_comms.registry_store import RegistryStore -from agent_comms.store_files import _atomic_write_text, _store_lock +from agent_comms.store_files import AtomicFile, _store_lock from agent_comms.thread_identity import RecordedThreadIncarnation from agent_comms.todos import OwnerRequest, Todo, TodoStore from agent_comms.typed_table import ( @@ -163,9 +163,7 @@ def carry(self, *, apply: bool) -> Carried: for name in result.rewritten ] if resealed: - _atomic_write_text( - HistoryArchive.path_in(self.root), json.dumps(FieldCodec.encode(tuple(rebound))) - ) + AtomicFile(HistoryArchive.path_in(self.root)).write_text(json.dumps(FieldCodec.encode(tuple(rebound)))) rewritten.append(HistoryArchive.filename) detail: dict[str, int] = {} for _, result in results: From 89b7c6274eb564de521296d19e5f83db050cb045 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:51:50 -0400 Subject: [PATCH 06/18] Order the finished-turn carry before every registry decoder BusHistory, OwnerRequests and TurnOwner each decode the registry with the target decoder, which refuses the retired last_finished_turn_id. Co-Authored-By: Claude Opus 5.5 --- tools/cutover/carry_finished_turns.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/cutover/carry_finished_turns.py b/tools/cutover/carry_finished_turns.py index fead5a2d7b..097cbfbec6 100644 --- a/tools/cutover/carry_finished_turns.py +++ b/tools/cutover/carry_finished_turns.py @@ -16,12 +16,17 @@ from agent_comms.registry_store import RegistryStore from agent_comms.store_files import _atomic_write_text, _store_lock -from .carry_bus_history import CarryCatalogSource +from .carry_bus_history import BusHistory, CarryCatalogSource +from .carry_owner_requests import OwnerRequests +from .carry_turn_owner import TurnOwner RETIRED = "last_finished_turn_id" class FinishedTurns(Carry): + precedes = (BusHistory, OwnerRequests, TurnOwner) + """Each decodes the registry, which refuses the retired field.""" + def backup_files(self) -> tuple[Path, ...]: sources = CarryCatalogSource.read(self.root, self.original_root) roots = ( From 0a0684ca029cf00210439eda0cd109e3c2358348 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:52:27 -0400 Subject: [PATCH 07/18] Derive failure and native proof ordering from declarations --- src/agent_comms/declared_family.py | 15 +++++++++ src/agent_comms/historical_native_inputs.py | 2 +- src/agent_comms/native_input_record.py | 2 -- src/agent_comms/turn_failure.py | 37 ++++++++------------- src/agent_comms/turn_output.py | 6 +--- tests/test_turn_failure_privacy.py | 11 +++++- 6 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/agent_comms/declared_family.py b/src/agent_comms/declared_family.py index 7bee054fc2..81b89284d0 100644 --- a/src/agent_comms/declared_family.py +++ b/src/agent_comms/declared_family.py @@ -88,6 +88,16 @@ class UnknownFamilyMember(ValueError): """The selected name is not a declared member of the requested family.""" +class DeclaredOrder(ABC): + """Plain rank capabilities ordered by their direct declarations.""" + + @classmethod + def position_of(cls, member: type) -> int: + declarations = cls.__subclasses__() + rank, = (base for base in declarations if issubclass(member, base)) + return declarations.index(rank) + + class DeclaredFamily(ABC, metaclass=_FamilyMeta): """A family root owns one registry; names and capability views derive from members. @@ -174,3 +184,8 @@ def members_with(cls, capability: type) -> tuple[type[Self], ...]: for member in cls._member_declarations() if issubclass(member, cls) and issubclass(member, capability) ) + + @classmethod + def declared_position(cls) -> int: + root = cast(type[DeclaredFamily], cls._family_root) + return root.members_with(root).index(cls) diff --git a/src/agent_comms/historical_native_inputs.py b/src/agent_comms/historical_native_inputs.py index 454a5c3c0b..4db953591d 100644 --- a/src/agent_comms/historical_native_inputs.py +++ b/src/agent_comms/historical_native_inputs.py @@ -160,7 +160,7 @@ def read_historical_native_inputs( raise IdentityConflict("historical source has ambiguous native input evidence") source_reads.observe_inputs(tuple(row.require_context_proof() for _source, row in rows)) with expected_prompt_bindings(store, (row.input_id for _source, row in rows)) as bindings: - rows.sort(key=lambda item: item[1].execution.proof_order) + rows.sort(key=lambda item: type(item[1].execution).declared_position()) evidence: list[HistoricalNativeInput] = [] for source, row in rows: original_session = row.require_session_identity() diff --git a/src/agent_comms/native_input_record.py b/src/agent_comms/native_input_record.py index 3bbb4bb633..d730f6aa39 100644 --- a/src/agent_comms/native_input_record.py +++ b/src/agent_comms/native_input_record.py @@ -228,7 +228,6 @@ def binding_fields(self): @dataclass(frozen=True) class TriageNativeExecution(NativeInputExecution): - proof_order: ClassVar[int] = 0 def transcript_events(self, entry, projection, user): return [event.as_context() for event in @@ -261,7 +260,6 @@ def historical_proof(self, record, *, lifecycle, **source): @dataclass(frozen=True) class FullNativeExecution(NativeInputExecution): - proof_order: ClassVar[int] = 1 execution_id: str attempt_ordinal: int diff --git a/src/agent_comms/turn_failure.py b/src/agent_comms/turn_failure.py index 7086ebf53f..d25ecf679e 100644 --- a/src/agent_comms/turn_failure.py +++ b/src/agent_comms/turn_failure.py @@ -19,7 +19,6 @@ class TurnFailure(DeclaredFamily): text: str code: ClassVar[str | None] = None - precedence: ClassVar[int] = 0 # A failure of the model request itself; Pi re-sending that request ends it. ends_with_request_retry: ClassVar[bool] = False @@ -30,7 +29,7 @@ def with_startup_diagnostics(self, stderr: str) -> TurnFailure: return self def supersedes(self, previous: TurnFailure | None) -> bool: - return previous is None or self.precedence >= previous.precedence + return previous is None or self.declared_position() < previous.declared_position() def prepare_execution_retry(self, db, execution_id: str) -> None: """Consume any earlier provider proof for this nonretryable failure.""" @@ -56,22 +55,18 @@ def with_startup_diagnostics(self, stderr: str) -> TurnFailure: return self.with_text(self.text + "\nBackend startup reported:\n" + stderr) code = FailureReason.INPUT_ID_UNAVAILABLE - precedence = 100 class IdentityUncertain(TurnFailure): code = FailureReason.IDENTITY_UNCERTAIN - precedence = 80 class AuthorityChanged(TurnFailure): code = FailureReason.AUTHORITY_CHANGED - precedence = 70 class FollowupUnrecognized(TurnFailure): code = FailureReason.FOLLOWUP_UNRECOGNIZED - precedence = 60 class TerminalFailure(TurnFailure): @@ -90,7 +85,6 @@ def explanation(cls, output: TurnOutput) -> str: class InputMissing(TerminalFailure): code = FailureReason.INPUT_MISSING - precedence = 50 default_text = "Pi RPC run ended without this prompt's user message start." @classmethod @@ -100,20 +94,6 @@ def detected(cls, session: TurnSession, transport_ok: bool) -> bool: ) -class FinalStopMissing(TerminalFailure): - code = FailureReason.FINAL_STOP_MISSING - precedence = 40 - default_text = "Pi RPC run ended without an authoritative final assistant stop." - - @classmethod - def detected(cls, session: TurnSession, transport_ok: bool) -> bool: - return transport_ok and not session.output.final_assistant_stop - - @classmethod - def explanation(cls, output: TurnOutput) -> str: - return output.failure_text or output.error_message or cls.default_text - - class ModelRequestFailed(TurnFailure): """An original native error terminal is evidence, not a missing stop.""" @@ -133,7 +113,6 @@ def raise_native(self, proof, model: SelectedModel) -> None: raise NativePiTerminalFailure(self.text, proof, model, self) code = FailureReason.MODEL_REQUEST_FAILED - precedence = 45 ends_with_request_retry = True @@ -161,9 +140,21 @@ class ProviderTransportFailed(RetryableProviderFailure, ModelRequestFailed): """The native diagnostic declared a provider transport failure.""" +class FinalStopMissing(TerminalFailure): + code = FailureReason.FINAL_STOP_MISSING + default_text = "Pi RPC run ended without an authoritative final assistant stop." + + @classmethod + def detected(cls, session: TurnSession, transport_ok: bool) -> bool: + return transport_ok and not session.output.final_assistant_stop + + @classmethod + def explanation(cls, output: TurnOutput) -> str: + return output.failure_text or output.error_message or cls.default_text + + class QueuedInputMissing(TerminalFailure): code = FailureReason.QUEUED_INPUT_MISSING - precedence = 30 default_text = "Pi RPC run ended with an unstarted queued input; delivery is uncertain." @classmethod diff --git a/src/agent_comms/turn_output.py b/src/agent_comms/turn_output.py index cc0d6b07a1..24cbd3ba06 100644 --- a/src/agent_comms/turn_output.py +++ b/src/agent_comms/turn_output.py @@ -143,11 +143,7 @@ def done(self, session: TurnSession, stderr: str) -> Iterator[events.AgentEvent] yield terminal def settle(self, session: TurnSession, transport_ok: bool) -> None: - for cause in sorted( - failures.TurnFailure.members_with(failures.TerminalFailure), - key=lambda member: member.precedence, - reverse=True, - ): + for cause in failures.TurnFailure.members_with(failures.TerminalFailure): if cause.detected(session, transport_ok): self.record_failure(cause(cause.explanation(self))) diff --git a/tests/test_turn_failure_privacy.py b/tests/test_turn_failure_privacy.py index 076ea84dac..cd2c95b4bc 100644 --- a/tests/test_turn_failure_privacy.py +++ b/tests/test_turn_failure_privacy.py @@ -23,7 +23,7 @@ def test_recorded_failure_has_one_safe_text_and_keeps_its_behavior(kind, sensiti assert stored.text == output.failure_text == expected assert output.terminal_text(False, 'private stderr', 1) == expected assert stored.code == original.code - assert stored.precedence == original.precedence + assert stored.declared_position() == original.declared_position() assert stored.ends_with_request_retry == original.ends_with_request_retry if sensitive: assert original.text not in repr(output) @@ -40,3 +40,12 @@ def test_redacted_copy_keeps_failure_precedence_and_inherited_copy_operation(): changed = selected.with_startup_diagnostics('ordinary startup evidence') assert type(changed) is InputIdUnavailable assert changed.text == selected.text + '\nBackend startup reported:\nordinary startup evidence' + + +def test_later_failure_of_the_same_kind_keeps_the_first_text(): + output = TurnOutput() + output.record_failure(ModelRequestFailed('first provider failure')) + first = output.declared_failure + output.record_failure(ModelRequestFailed('later provider failure')) + assert output.declared_failure is first + assert output.failure_text == 'first provider failure' From 2b824acfeb974d0707531d0f1d619b6f513eeada Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:52:56 -0400 Subject: [PATCH 08/18] Run pytest serially unless the full-suite command asks for workers pyproject addopts forced -n auto on every invocation, so running one touched test spawned eight xdist workers; two such loops held the box at load 36. The worker pool now lives on the two CI full-suite commands that want it. The installer's -n 0 and the acceptance doc's -o addopts='' only existed to undo the forced default, so both are deleted. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 4 ++-- README.md | 6 +++--- pyproject.toml | 1 - src/agent_comms/installer/upgrade.py | 2 +- tests/mcp-acceptance.md | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38e53fe931..5a5884fc4c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,14 +104,14 @@ jobs: - name: Run tests with coverage (85% enforced on Linux) if: matrix.os == 'ubuntu-latest' run: | - pytest --cov=agent_comms --cov-report=xml --cov-report=term-missing -v + pytest -n auto --maxprocesses=8 --dist=worksteal --cov=agent_comms --cov-report=xml --cov-report=term-missing -v # Sealed /var/tmp, /proc and SO_PEERCRED fixtures run only on Linux. # Keep macOS test execution, but do not enforce Linux coverage there. - name: Run tests (coverage reported, Linux-only fixtures skipped) if: matrix.os != 'ubuntu-latest' run: | - pytest --cov=agent_comms --cov-fail-under=0 --cov-report=xml --cov-report=term-missing -v + pytest -n auto --maxprocesses=8 --dist=worksteal --cov=agent_comms --cov-fail-under=0 --cov-report=xml --cov-report=term-missing -v build: runs-on: ubuntu-latest diff --git a/README.md b/README.md index c32687d6f1..9a42e4df8b 100644 --- a/README.md +++ b/README.md @@ -112,9 +112,9 @@ It does not publish the runtime or change the live installation. Pass `--home DI to retain a private test build for inspection. The pytest basetemp is disposable, named for the checkout's HEAD, and the test run is limited to 300 seconds. -Pytest uses separate `pytest-xdist` worker processes (automatic CPU detection, -capped at eight) with work stealing and combined coverage. Use `pytest -n 0` -for serial debugging or `pytest -n 4` to choose a worker count. +Pytest runs serially by default, so a run of a few touched tests starts no +worker pool. The full suite in CI passes `-n auto --maxprocesses=8 +--dist=worksteal` itself; do the same locally for a full run. ## Releasing diff --git a/pyproject.toml b/pyproject.toml index 8c295a2dc9..c3a7259273 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,7 +101,6 @@ pythonpath = [".", "src", "tools"] python_files = ["test_*.py"] python_classes = ["Test*"] python_functions = ["test_*"] -addopts = "-n auto --maxprocesses=8 --dist=worksteal" asyncio_mode = "auto" # A hung test fails by name instead of holding the whole job. timeout = 60 diff --git a/src/agent_comms/installer/upgrade.py b/src/agent_comms/installer/upgrade.py index e308a2a7db..9529976c83 100644 --- a/src/agent_comms/installer/upgrade.py +++ b/src/agent_comms/installer/upgrade.py @@ -489,7 +489,7 @@ def test(self, *pytest_arguments: str) -> None: scratch.mkdir(mode=0o700, parents=True, exist_ok=True) with tempfile.TemporaryDirectory(dir=scratch, prefix=f"tests-{core[:12]}-") as temporary: print(self.commands.run( - "timeout", "300", self.python, "-m", "pytest", "-n", "0", "-q", "-s", + "timeout", "300", self.python, "-m", "pytest", "-q", "-s", "-o", "pythonpath=. tools", "-o", "faulthandler_timeout=60", "--basetemp", Path(temporary) / "pytest", "-p", "no:cacheprovider", *pytest_arguments, cwd=self.checkout, env=self.environment()), end="") diff --git a/tests/mcp-acceptance.md b/tests/mcp-acceptance.md index 0b51c6f90c..437ddb8ba8 100644 --- a/tests/mcp-acceptance.md +++ b/tests/mcp-acceptance.md @@ -10,7 +10,7 @@ env -i HOME="$HOME" PATH="$PATH" \ TMPDIR=/absolute/to/home-test-tmp \ AC_MCP_NATIVE_BIN=/absolute/to/prepared/pi-native \ AC_MCP_TOAD_ADAPTER=/absolute/to/toad/tests/mcp_linked_adapter.py \ - /absolute/to/compatible/python -m pytest -q -o addopts='' \ + /absolute/to/compatible/python -m pytest -q \ tests/test_mcp_acceptance.py --basetemp=/absolute/to/fresh-evidence-directory ``` From bb522b65cd54879ddec581e852189f45bd4869f1 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:52:58 -0400 Subject: [PATCH 09/18] Retire each superseded turn generation when its successor is leased lease_turn wrote turns/{lookup}-{N+1}.json and never deleted generation N, so the live bus held 8,739 turn files for 51 current turns. The registry edit now deletes the superseded file after it saves the registry naming N+1, inside the same registry lock. A lease from a superseded generation asks the registry snapshot whether its thread still names that generation before it reads its file, so a late transition returns nothing instead of reading a deleted file. Thread.turn_identity owns the (incarnation, generation) pair its three callers spelled by hand. carry_dead_turns deletes the 8,688 dead files already on disk and refuses if any thread's current file is missing. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/registry_document.py | 7 ++++- src/agent_comms/registry_store.py | 9 ++++++- src/agent_comms/threads.py | 7 ++++- src/agent_comms/turn_lease.py | 4 +++ tests/test_registration_authority.py | 14 ++++++++++ tools/cutover/carry_dead_turns.py | 39 ++++++++++++++++++++++++++++ 6 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 tools/cutover/carry_dead_turns.py diff --git a/src/agent_comms/registry_document.py b/src/agent_comms/registry_document.py index 2b3fb606db..56a61f55a2 100644 --- a/src/agent_comms/registry_document.py +++ b/src/agent_comms/registry_document.py @@ -688,7 +688,7 @@ def release_turn(self, lease: TurnLeaseFence) -> tuple[bool, TurnCompletion]: record = current.turn_record if record != canonical_lease.record: return False, NoFinishedTurn() - if TurnIdentity(current.incarnation, current.turn_generation) != canonical_lease.identity: + if current.turn_identity != canonical_lease.identity: return False, NoFinishedTurn() turn = record.read().require_active() if not turn.may_release(canonical_lease): @@ -794,6 +794,11 @@ def retired_names(self) -> tuple[str, ...]: def threads_for(self, incarnation: RecordedThreadIncarnation) -> tuple[Thread, ...]: return tuple(self.by_lookup[key] for key in (incarnation.lookup,) if key in self.by_lookup) + def holds_turn(self, identity: TurnIdentity) -> bool: + """Whether its thread still names this turn generation; a superseded lease does not.""" + return any(thread.turn_identity == identity + for thread in self.threads_for(identity.incarnation.require_recorded())) + def idle_after(self, fence: FinishedTurnFence) -> tuple[Thread, ...]: return tuple( thread diff --git a/src/agent_comms/registry_store.py b/src/agent_comms/registry_store.py index cd6a4e158a..ce0c36774e 100644 --- a/src/agent_comms/registry_store.py +++ b/src/agent_comms/registry_store.py @@ -141,6 +141,8 @@ class RegistryEdit: store: RegistryStore document: RegistryDocument original: RegistryRevision + superseded: list[StoredTurnRecord] = field(default_factory=list) + """Turn files a lease in this edit replaced; deleted once the registry stops naming them.""" def prepare_native_source(self, owner: RegistryOwner, session_file: str) -> NativeSourcePublication: current = owner.require_source_snapshot(self.original.snapshot) @@ -174,6 +176,8 @@ def lease_turn( project_resume_due=False, turn_record=StoredTurnRecord.begin(self.store.path, identity, turn), ) + if current.turn_generation: + self.superseded.append(StoredTurnRecord.for_identity(self.store.path, current.turn_identity)) self.document.threads[current.name] = leased self.document.last_seen[current.name] = time.time() return leased, self.document.owners.generations[current.name] @@ -182,6 +186,9 @@ def commit(self) -> None: if self.document != self.original.document: self.store.save_unlocked(self.document) self.original = self.store.read_revision_unlocked() + for record in self.superseded: + record.path.unlink() + self.superseded.clear() @dataclass(frozen=True, slots=True) @@ -279,7 +286,7 @@ def _encode(self, value: RegistryDocument) -> dict: def _bind_thread(self, thread: Thread) -> Thread: record = thread.turn_record - bound = record.bound_to(self.path, TurnIdentity(thread.incarnation, thread.turn_generation)) + bound = record.bound_to(self.path, thread.turn_identity) return thread if bound is record else replace(thread, turn_record=bound) def _bind_document(self, document: RegistryDocument) -> RegistryDocument: diff --git a/src/agent_comms/threads.py b/src/agent_comms/threads.py index e01e1f3499..619ebf7f70 100644 --- a/src/agent_comms/threads.py +++ b/src/agent_comms/threads.py @@ -480,12 +480,17 @@ def has_observed_task_turn(self, identity: TurnIdentity, turn: TurnId) -> bool: identity, turn, idle=identity.generation == self.turn_generation and self.last_finished_turn_id == turn.value) + @property + def turn_identity(self) -> TurnIdentity: + """The turn generation this thread's registry entry currently names.""" + return TurnIdentity(self.incarnation, self.turn_generation) + def observed_turn(self, claim_scope: int) -> TurnFence | None: """Passive current/last-completed witness; never a begin-turn grant.""" turn_id = self.turn_record.read().observed_id(self.last_finished_turn_id) if turn_id is None: return None - return TurnFence(TurnIdentity(self.incarnation, self.turn_generation), turn_id, claim_scope) + return TurnFence(self.turn_identity, turn_id, claim_scope) def latest_idle_turn(self, fence: TurnFence, claim_scope: int) -> bool: """Only the exact latest completed turn can release a dependency.""" diff --git a/src/agent_comms/turn_lease.py b/src/agent_comms/turn_lease.py index ad212ffe0c..dbdfba8cc3 100644 --- a/src/agent_comms/turn_lease.py +++ b/src/agent_comms/turn_lease.py @@ -529,7 +529,11 @@ def without_claim(self) -> None: def _turn_effects( self, lease: TurnLeaseFence, observe: Callable[[TurnState], Iterable[TurnState]], ) -> tuple[TurnState, ...]: + from .registry_store import RegistryStore + with _store_lock(self.registry_path): + if not RegistryStore(self.registry_path).read_snapshot_unlocked().holds_turn(self.identity): + return () active = self.read() if not active.held_by(lease): return () diff --git a/tests/test_registration_authority.py b/tests/test_registration_authority.py index 47e56c6db3..456472ed3c 100644 --- a/tests/test_registration_authority.py +++ b/tests/test_registration_authority.py @@ -12,6 +12,7 @@ from agent_comms.thread_identity import UserThreadRole from agent_comms.thread_text import GivenThreadText from agent_comms.turn_lease import RecordedGeneration +from agent_comms.turn_phase import PreparingPhase def registered(root): @@ -77,3 +78,16 @@ def test_live_owner_read_rejects_raw_stale_turn_witness(tmp_path): edit.commit() with pytest.raises(RelationViolationError): registry.live_owner_with_claim('owner') + + +def test_a_superseded_generation_is_deleted_and_its_late_transition_changes_nothing(tmp_path): + registry = registered(tmp_path) + registry.live_owner_with_claim('owner') + first, _ = registry.lease_local_turn('owner', 'first') + stale = first.turn_lease + assert registry.release_turn(stale)[0] + second, _ = registry.lease_local_turn('owner', 'second') + assert not stale.record.path.exists() and second.turn_record.path.exists() + assert stale.record.transition(stale, PreparingPhase()) == () + assert stale.record.observe_native_phase(stale, PreparingPhase()) == () + assert second.turn_record.read().id == 'second' diff --git a/tools/cutover/carry_dead_turns.py b/tools/cutover/carry_dead_turns.py new file mode 100644 index 0000000000..df26ca133f --- /dev/null +++ b/tools/cutover/carry_dead_turns.py @@ -0,0 +1,39 @@ +"""Delete the turn files of generations the registry no longer names. + +``RegistryEdit.lease_turn`` now deletes the generation it supersedes, but every +bus before that kept each thread's old generations forever (8,688 dead files on +the live bus). A thread's current generation is kept, whether or not a turn still +references it, because a released lease's late transition may still read it. A +thread whose current file is missing has no recorded turn to keep, and the carry +refuses it instead of guessing. + +Delete after every bus has run this step. +""" + +from __future__ import annotations + +from agent_comms.errors import RelationViolationError +from agent_comms.installer.carry import Carried, Carry +from agent_comms.registry_store import RegistryStore +from agent_comms.turn_lease import StoredTurnRecord + + +class DeadTurns(Carry): + def carry(self, *, apply: bool) -> Carried: + registry = RegistryStore.at(self.root) + if not registry.path.exists(): + return Carried(0) + current = frozenset( + StoredTurnRecord.for_identity(registry.path, thread.turn_identity).path + for thread in registry.read_snapshot_unlocked().threads.values() + if thread.turn_generation + ) + missing = sorted(str(path) for path in current if not path.exists()) + if missing: + raise RelationViolationError(f"Current turn files are missing: {missing}") + dead = sorted(StoredTurnRecord.matching_in(registry.path.parent) - current) + for path in dead: + self.require_confined(self.root, path) + if apply: + path.unlink() + return Carried(len(dead), detail={"kept": len(current), "deleted": len(dead)}) From 5f7d21cc3fffebe406ff4c7626a28c435665e5d0 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:53:21 -0400 Subject: [PATCH 10/18] Own file and directory sync in the Platform family --- src/agent_comms/child_process.py | 31 ++++++++++++++++++++++- src/agent_comms/diagnostics.py | 4 +-- src/agent_comms/fresh_private_session.py | 5 ++-- src/agent_comms/goal_attempts.py | 4 +-- src/agent_comms/harness_install.py | 4 +-- src/agent_comms/installer/backup.py | 9 ++++--- src/agent_comms/locked_store.py | 7 ++--- src/agent_comms/native_package.py | 5 ++-- src/agent_comms/native_session_files.py | 9 ++++--- src/agent_comms/private_path.py | 9 ------- src/agent_comms/private_registry_guard.py | 8 +++--- src/agent_comms/restart_queue.py | 5 ++-- src/agent_comms/selected_tool_broker.py | 9 ++++--- src/agent_comms/selected_write_plan.py | 7 ++--- src/agent_comms/store_files.py | 14 +++++----- tests/test_atomic_file.py | 22 ++++++++++++++++ 16 files changed, 99 insertions(+), 53 deletions(-) diff --git a/src/agent_comms/child_process.py b/src/agent_comms/child_process.py index 0ce62d27d0..93368dea8e 100644 --- a/src/agent_comms/child_process.py +++ b/src/agent_comms/child_process.py @@ -36,7 +36,7 @@ from dataclasses import dataclass from functools import partial from pathlib import Path -from typing import TYPE_CHECKING, Any, ClassVar, NoReturn +from typing import TYPE_CHECKING, Any, BinaryIO, ClassVar, NoReturn from .declared_family import DeclaredFamily from .errors import RelationViolationError @@ -413,6 +413,18 @@ class Platform(DeclaredFamily, affix="Platform"): def current(cls) -> Platform: return cls.member_named(sys.platform)() + @abstractmethod + def fsync_directory(self, path: Path) -> None: + """Finish publication using this platform's directory durability contract.""" + + @abstractmethod + def open_for_sync(self, path: Path) -> BinaryIO: + """Open an existing file with the access required by the native flush.""" + + def fsync_file(self, path: Path) -> None: + with self.open_for_sync(path) as source: + os.fsync(source.fileno()) + @abstractmethod def try_store_lock(self, descriptor: int, *, mode: LockMode) -> None: """One nonblocking native attempt; busy is BlockingIOError.""" @@ -481,6 +493,16 @@ def signal_group(self, leader: ProcessIdentity, signum: int) -> None: class PosixPlatform(Platform): + def fsync_directory(self, path: Path) -> None: + descriptor = os.open(path, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW) + try: + os.fsync(descriptor) + finally: + os.close(descriptor) + + def open_for_sync(self, path: Path) -> BinaryIO: + return path.open("rb") + def acquire_store_lock(self, descriptor: int, *, mode: LockMode, blocking: bool) -> None: import fcntl @@ -685,6 +707,13 @@ class Win32Platform(Platform): store_lock_interval = 0.01 + def fsync_directory(self, path: Path) -> None: + """NTFS journals the rename; there is no POSIX directory flush.""" + + def open_for_sync(self, path: Path) -> BinaryIO: + # The Windows CRT's fsync requires a writable descriptor. + return path.open("rb+") + def try_store_lock(self, descriptor: int, *, mode: LockMode) -> None: import errno import msvcrt diff --git a/src/agent_comms/diagnostics.py b/src/agent_comms/diagnostics.py index 8aa0b78f3a..66d4903121 100644 --- a/src/agent_comms/diagnostics.py +++ b/src/agent_comms/diagnostics.py @@ -14,8 +14,8 @@ from typing import TYPE_CHECKING, Literal from uuid import uuid4 -from agent_comms.private_path import fsync_directory +from .child_process import Platform from .field_codec import FieldCodec, Id128Text, MalformedRecord from .locked_store import DurableFile, PatternFile from .store_files import AtomicFile @@ -278,6 +278,6 @@ def write(cls, root: Path, name: str, document: dict) -> Path: path = cls.path_in(root, name=name) path.parent.mkdir(mode=0o700, exist_ok=True) if os.name == "posix": - fsync_directory(root) + Platform.current().fsync_directory(root) AtomicFile(path).write_text(json.dumps(document, indent=2) + "\n") return path.resolve() diff --git a/src/agent_comms/fresh_private_session.py b/src/agent_comms/fresh_private_session.py index 13fe73e5f0..f636402ef9 100644 --- a/src/agent_comms/fresh_private_session.py +++ b/src/agent_comms/fresh_private_session.py @@ -8,13 +8,14 @@ from pathlib import Path from uuid import uuid4 +from agent_comms.child_process import Platform from .field_codec import RecordedTimestamp from .native_entries import SessionEntry from .native_failures import NativePiUnavailable from agent_comms.native_session_files import NativeSessionFiles from .native_session_files import PrivateSession, SavedSessionFile from .native_session_reopen import NativeSessionIdentity -from .private_path import NativeSessionFileRole, fsync_directory +from .private_path import NativeSessionFileRole def create_fresh_private_session( @@ -47,7 +48,7 @@ def create_fresh_private_session( os.fsync(descriptor) finally: os.close(descriptor) - fsync_directory(session_dir) + Platform.current().fsync_directory(session_dir) return NativeSessionIdentity(session_id, str(path)) except OSError as error: # A failed sync can leave a committed file. Preserve it, never infer success. diff --git a/src/agent_comms/goal_attempts.py b/src/agent_comms/goal_attempts.py index 728c880246..fd4c53b000 100644 --- a/src/agent_comms/goal_attempts.py +++ b/src/agent_comms/goal_attempts.py @@ -28,8 +28,8 @@ from typing import TYPE_CHECKING, Annotated, ClassVar, TypeVar from uuid import uuid4 +from agent_comms.child_process import Platform from agent_comms.goal_turn_permit import GoalTurnPermit -from agent_comms.private_path import fsync_directory from .declared_family import DeclaredFamily from .field_codec import MalformedRecord @@ -717,7 +717,7 @@ def initialize(cls, root: str | Path) -> GoalAttemptStore: # The staging descriptor is closed before its inode becomes shared. with suppress(FileExistsError): os.link(staging, path, follow_symlinks=False) - fsync_directory(directory) + Platform.current().fsync_directory(directory) except (sqlite3.Error, OSError) as error: raise StorageUncertainError("Goal attempt initialization is uncertain.") from error finally: diff --git a/src/agent_comms/harness_install.py b/src/agent_comms/harness_install.py index fedb4a1bd8..28e88178a3 100644 --- a/src/agent_comms/harness_install.py +++ b/src/agent_comms/harness_install.py @@ -16,9 +16,9 @@ from pathlib import Path from typing import TYPE_CHECKING, ClassVar -from agent_comms.private_path import fsync_directory from agent_comms_hooks import ClaudeHookProgram +from .child_process import Platform from .declared_family import DeclaredFamily from .thread_execution import ExternalThreadExecution @@ -35,7 +35,7 @@ def replace_link(link: Path, target: Path) -> None: try: temporary.symlink_to(target) os.replace(temporary, link) - fsync_directory(link.parent) + Platform.current().fsync_directory(link.parent) finally: temporary.unlink(missing_ok=True) diff --git a/src/agent_comms/installer/backup.py b/src/agent_comms/installer/backup.py index e4d8be0822..0e744535b4 100644 --- a/src/agent_comms/installer/backup.py +++ b/src/agent_comms/installer/backup.py @@ -21,12 +21,13 @@ from typing import ClassVar, Iterator, Self from agent_comms.active_route import ActiveRoute, _route_directory, route_directory +from agent_comms.child_process import Platform from agent_comms.coordination_errors import CoordinationError from agent_comms.external_transcript import ExternalTranscript from agent_comms.field_codec import FieldCodec from agent_comms.installer.interpreter_operation import HoldTranscriptsProbe, RouteSelection from agent_comms.locked_store import LockedStore, RootFile, RuntimeFile, SQLiteFile -from agent_comms.private_path import FileObservation, FileRevision, MissingFile, fsync_directory +from agent_comms.private_path import FileObservation, FileRevision, MissingFile from agent_comms.store_files import AtomicFile @@ -205,7 +206,7 @@ def files(self) -> dict[str, str]: def retire(self) -> Path: archived = self.path.with_name(f'{self.path.name}.restored-{time.time_ns()}') self.path.rename(archived) - fsync_directory(archived.parent) + Platform.current().fsync_directory(archived.parent) print(f'Restored data backup kept at {archived}', flush=True) return archived @@ -343,7 +344,7 @@ def capture(self, files: tuple[Path, ...]) -> dict[str, str]: with target.open('rb') as ready: os.fsync(ready.fileno()) contents[name] = self.digest(target) - fsync_directory(target.parent) + Platform.current().fsync_directory(target.parent) return contents def file_original(self, name: str, saved: dict[str, str]) -> bool: @@ -382,7 +383,7 @@ def restore(self) -> None: target.unlink(missing_ok=True) for companion in SQLiteFile.files(target)[1:]: companion.unlink(missing_ok=True) - fsync_directory(self.root) + Platform.current().fsync_directory(self.root) self.restore_route() if not self.stores_original(): raise UpgradeRefused('Selected stores differ after backup restoration') diff --git a/src/agent_comms/locked_store.py b/src/agent_comms/locked_store.py index 2107edead5..2f89169006 100644 --- a/src/agent_comms/locked_store.py +++ b/src/agent_comms/locked_store.py @@ -19,9 +19,10 @@ from pkgutil import walk_packages from typing import Any, ClassVar, Generic, Self, TypeVar +from .child_process import Platform from .coordination_errors import CoordinationReadUnavailable from .field_codec import FieldCodec, MalformedRecord -from .private_path import PrivateFileRole, fsync_directory +from .private_path import PrivateFileRole from .store_files import ( AtomicFile, SharedLock, @@ -108,10 +109,10 @@ def _write_unlocked(self, text: str) -> None: if backup.exists(): if not self.path.exists() or not self.path.samefile(backup): os.replace(backup, self.path) - fsync_directory(self.path.parent) + Platform.current().fsync_directory(self.path.parent) else: self.path.unlink(missing_ok=True) - fsync_directory(self.path.parent) + Platform.current().fsync_directory(self.path.parent) raise finally: # A failed restore leaves the last original inode for recovery. diff --git a/src/agent_comms/native_package.py b/src/agent_comms/native_package.py index 591b61ebcb..1c3c5663ba 100644 --- a/src/agent_comms/native_package.py +++ b/src/agent_comms/native_package.py @@ -20,9 +20,10 @@ from pathlib import Path from typing import Annotated, Literal +from agent_comms.child_process import Platform from agent_comms.field_codec import FieldCodec, MalformedRecord, PathText, Sha256Text, WireName from agent_comms.native_build_inputs import INPUTS_PREFIX, NativeBuildInputs -from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole, fsync_directory +from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole from .store_files import AtomicFile @@ -361,7 +362,7 @@ def install_declared_sources(root: Path, snapshot_root: Path) -> None: for source in sources: source.verify_installed() for parent in {source.destination.parent for source in sources}: - fsync_directory(parent) + Platform.current().fsync_directory(parent) except OSError as error: raise NativePackageError("Native extension source could not be installed") from error finally: diff --git a/src/agent_comms/native_session_files.py b/src/agent_comms/native_session_files.py index fac9a7ff84..f056b44f9f 100644 --- a/src/agent_comms/native_session_files.py +++ b/src/agent_comms/native_session_files.py @@ -8,11 +8,12 @@ from pathlib import Path from typing import Self +from agent_comms.child_process import Platform from agent_comms.field_codec import Id128Text from agent_comms.locked_store import DurableFile, PatternFile, SQLiteFile from agent_comms.native_session_reopen import NativeSessionIdentity from agent_comms.participant_identity import ParticipantIdentity -from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole, fsync_directory +from agent_comms.private_path import PrivateDirectoryRole, TrustedAncestorRole from .errors import RelationViolationError from .native_failures import NativePiUnavailable @@ -95,7 +96,7 @@ def delete(self) -> int: freed += path.lstat().st_size path.unlink() if self.session.parent.is_dir(): - fsync_directory(self.session.parent) + Platform.current().fsync_directory(self.session.parent) return freed @@ -172,11 +173,11 @@ def prepare_private_directory(cls, directory: Path) -> None: for path in reversed(missing): path.mkdir(mode=0o700, exist_ok=True) cls.require_private_directory(path) - fsync_directory(path.parent) + Platform.current().fsync_directory(path.parent) cls.require_private_directory(directory) parent = directory.parent while True: - fsync_directory(parent) + Platform.current().fsync_directory(parent) if parent == parent.parent: break try: diff --git a/src/agent_comms/private_path.py b/src/agent_comms/private_path.py index 3381337db4..e14127e3f0 100644 --- a/src/agent_comms/private_path.py +++ b/src/agent_comms/private_path.py @@ -22,15 +22,6 @@ from .field_codec import NullMember, RecordMember, SubclassUnion -def fsync_directory(path: Path) -> None: - """Make a directory's entries durable without following a final symlink.""" - descriptor = os.open(path, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW) - try: - os.fsync(descriptor) - finally: - os.close(descriptor) - - @dataclass(frozen=True, slots=True) class FileIdentity: device: int diff --git a/src/agent_comms/private_registry_guard.py b/src/agent_comms/private_registry_guard.py index 7db93224ca..8fada2fd64 100644 --- a/src/agent_comms/private_registry_guard.py +++ b/src/agent_comms/private_registry_guard.py @@ -28,6 +28,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Annotated, NoReturn +from .child_process import Platform from . import store_files from .coordination_database import CoordinationStore from .coordinator import Coordination @@ -35,7 +36,6 @@ from .field_codec import FieldCodec, Id128Text, MalformedRecord, WireName from .locked_store import DurableFile, WireWakeFile from .native_runtime_input import NativeRuntimeInput, NativeRuntimeTable -from .private_path import fsync_directory if TYPE_CHECKING: from .acp_extension import CursorAdvancedUpdate, CursorScope @@ -470,7 +470,7 @@ def create_pending(self) -> None: os.fsync(registry_fd) finally: os.close(registry_fd) - fsync_directory(self.registry_path.parent) + Platform.current().fsync_directory(self.registry_path.parent) fd = os.open( self.path, os.O_RDWR | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, @@ -480,7 +480,7 @@ def create_pending(self) -> None: if os.pwrite(fd, _record(self.root_id, 0, _COMMITTED, digest) * 2, 0) != 2 * _SIZE: _reject("initial slots were incomplete") os.fsync(fd) - fsync_directory(self.registry_path.parent) + Platform.current().fsync_directory(self.registry_path.parent) self._write(fd, 1, _record(self.root_id, 1, _PENDING, digest)) finally: os.close(fd) @@ -525,7 +525,7 @@ def complete(cls, root: Path) -> None: intent = cls.path_in(root) replace(FieldCodec.decode(cls, json.loads(intent.read_text())), completed=True).write(root) intent.unlink() - fsync_directory(root) + Platform.current().fsync_directory(root) def _require_no_private_owner_rename(root: Path) -> None: diff --git a/src/agent_comms/restart_queue.py b/src/agent_comms/restart_queue.py index 9d0c9e3d6e..fff9f63f07 100644 --- a/src/agent_comms/restart_queue.py +++ b/src/agent_comms/restart_queue.py @@ -19,9 +19,8 @@ from pathlib import Path from typing import TYPE_CHECKING, Annotated -from agent_comms.private_path import fsync_directory -from .child_process import OutlivesLauncherLifetime, ProcessIdentity, SpawnedProcess +from .child_process import OutlivesLauncherLifetime, Platform, ProcessIdentity, SpawnedProcess from .comms import Comms, wire from .coordination_errors import CoordinationError from .coordinator import Coordination @@ -242,7 +241,7 @@ def move(self, path: Path, record: QueuedRestart) -> None: raise FileExistsError(destination) os.rename(path, destination) for parent in (path.parent, destination.parent): - fsync_directory(parent) + Platform.current().fsync_directory(parent) def save(self, record: QueuedRestart) -> None: path = self.active_path(record.selection.identity.incarnation.require_recorded()) diff --git a/src/agent_comms/selected_tool_broker.py b/src/agent_comms/selected_tool_broker.py index 7b0b4cf3d8..1e6e02acde 100644 --- a/src/agent_comms/selected_tool_broker.py +++ b/src/agent_comms/selected_tool_broker.py @@ -18,6 +18,7 @@ from pathlib import Path from typing import ClassVar +from agent_comms.child_process import Platform from agent_comms.coordinator import Coordination from .comms import Comms @@ -40,7 +41,7 @@ ToolTransport, UnboundToolTransport, ) -from .private_path import PrivateDirectoryRole, PrivateFileRole, fsync_directory +from .private_path import PrivateDirectoryRole, PrivateFileRole from .selected_actions import SelectedAction from .selected_claim import publish_selected_resource_claim, write_selected_claimed_file from .turn_context import InstructionFile @@ -194,7 +195,7 @@ def consume_selected_slot(directory: Path, input_id: str, call_id: str) -> None: info = ledger.lstat() if PrivateDirectoryRole.violation(info) is not None: raise SelectedToolDenied("Selected tool ledger is not private") - fsync_directory(directory) + Platform.current().fsync_directory(directory) fd = os.open(ledger / input_id, os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, 0o600) except FileExistsError as error: raise SelectedToolDenied( @@ -207,7 +208,7 @@ def consume_selected_slot(directory: Path, input_id: str, call_id: str) -> None: stream.write((call_id + "\n").encode("ascii")) stream.flush() os.fsync(stream.fileno()) - fsync_directory(ledger) + Platform.current().fsync_directory(ledger) except OSError as error: # Do NOT remove the visible slot; a failed fsync or a crash may have # committed it. No automatic re-append, retry, or write follows. @@ -223,7 +224,7 @@ def record_selected_terminal(directory: Path, input_id: str, call_id: str) -> No output.write((call_id + "\n").encode("ascii")) output.flush() os.fsync(output.fileno()) - fsync_directory(result.parent) + Platform.current().fsync_directory(result.parent) except OSError as error: raise SelectedToolDenied("Selected tool terminal acknowledgement UNKNOWN; no retry") from error diff --git a/src/agent_comms/selected_write_plan.py b/src/agent_comms/selected_write_plan.py index ea4eaf62b6..3929a25499 100644 --- a/src/agent_comms/selected_write_plan.py +++ b/src/agent_comms/selected_write_plan.py @@ -17,6 +17,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Annotated, Literal +from agent_comms.child_process import Platform from .coordination_database import CoordinationStore from .locked_store import DurableFile, PatternFile from .store_files import AtomicFile @@ -34,7 +35,7 @@ from .envelope_claim_transitions import ExistingFileClaim from .field_codec import FieldCodec, Id128Text, MalformedRecord, WireName from .message_reference import MessageReference -from .private_path import PrivateDirectoryRole, PrivateFileRole, fsync_directory +from .private_path import PrivateDirectoryRole, PrivateFileRole from .thread_identity import ClaimedIdentity, RecordedThreadIncarnation from .threads import Thread @@ -128,7 +129,7 @@ def _prepare_dir(self) -> None: # Also on an existing directory: a previous mkdir may have survived # while its parent fsync failed. Do not acknowledge a later intent # until the directory entry itself is known durable. - fsync_directory(self.comms.root) + Platform.current().fsync_directory(self.comms.root) def _validate_dir(self) -> bool: try: @@ -232,7 +233,7 @@ def submit( stream.write(data) stream.flush() os.fsync(stream.fileno()) - fsync_directory(self.directory) + Platform.current().fsync_directory(self.directory) except BaseException: # The visible file may have committed despite the error. Keep # it as an UNKNOWN barrier; never unlink/retry the operation. diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index 909e267a6b..6f42bdc15f 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -29,7 +29,7 @@ from .child_process import Platform from .declared_family import DeclaredFamily from .errors import RelationViolationError -from .private_path import FileIdentity, PrivateFileRole, fsync_directory +from .private_path import FileIdentity, PrivateFileRole with suppress(ModuleNotFoundError): # Windows; Win32Platform never asks a mode for its flock. import fcntl @@ -462,23 +462,21 @@ def write_bytes(self, data: bytes) -> None: def replace(self, staged: Path) -> None: """Publish an independently prepared file after syncing its data.""" - with staged.open("rb") as source: - os.fsync(source.fileno()) + Platform.current().fsync_file(staged) self._publish(staged) def _publish(self, staged: Path) -> None: self._install(staged) - fsync_directory(self.path.parent) + Platform.current().fsync_directory(self.path.parent) def _install(self, staged: Path) -> None: os.replace(staged, self.path) def sync(self) -> None: """Sync an existing file without replacing an open database's inode.""" - # Windows' CRT needs a writable handle; these files are owned writes. - with self.path.open("rb+") as source: - os.fsync(source.fileno()) - fsync_directory(self.path.parent) + platform = Platform.current() + platform.fsync_file(self.path) + platform.fsync_directory(self.path.parent) class ExclusiveAtomicFile(AtomicFile): diff --git a/tests/test_atomic_file.py b/tests/test_atomic_file.py index 1e21825326..fc6b452480 100644 --- a/tests/test_atomic_file.py +++ b/tests/test_atomic_file.py @@ -5,9 +5,11 @@ import pytest +from agent_comms.child_process import Platform, Win32Platform from agent_comms.store_files import AtomicFile, ExclusiveAtomicFile +@pytest.mark.skipif(os.name != "posix", reason="POSIX directory flush") def test_text_and_binary_writes_sync_contents_then_the_directory(tmp_path, monkeypatch): target = tmp_path / "value" synced = [] @@ -37,6 +39,7 @@ def test_failed_stream_preparation_keeps_the_old_file(tmp_path): assert set(tmp_path.iterdir()) == {target} +@pytest.mark.skipif(os.name != "posix", reason="POSIX directory flush") def test_parent_sync_failure_is_reported_after_the_visible_replace(tmp_path, monkeypatch): target = tmp_path / "value" fsync = os.fsync @@ -59,3 +62,22 @@ def test_exclusive_publication_never_replaces_an_existing_destination(tmp_path): ExclusiveAtomicFile(target).write_text("replacement") assert target.read_text() == "original" assert set(tmp_path.iterdir()) == {target} + + +def test_windows_publication_and_existing_file_sync_use_writable_handles(tmp_path, monkeypatch): + # These file operations need no kernel32 process handles, so exercise the + # actual Windows implementation on either host without initializing them. + platform = object.__new__(Win32Platform) + monkeypatch.setattr(Platform, "current", classmethod(lambda cls: platform)) + target = tmp_path / "value" + AtomicFile(target).write_bytes(b"first") + staged = tmp_path / "prepared" + staged.write_bytes(b"second") + AtomicFile(target).replace(staged) + AtomicFile(target).sync() + with platform.open_for_sync(target) as source: + assert source.writable() + assert source.read() == b"second" + # A Windows directory flush does not open the directory, including a path + # that does not exist; it must not ask os for POSIX-only flags. + platform.fsync_directory(tmp_path / "absent") From 52e97e6b59c0940bbb2c513e78a715c9011bd2e6 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:53:43 -0400 Subject: [PATCH 11/18] Order progress, ACP classification and tool actions by declaration --- src/agent_comms/acp_failure.py | 62 +++++++++++-------------- src/agent_comms/todos.py | 84 ++++++++++++++++------------------ src/agent_comms/tools.py | 55 ++++++++++------------ tests/test_acp_failure.py | 14 ++---- tests/test_pi_rpc_nominal.py | 9 +--- 5 files changed, 94 insertions(+), 130 deletions(-) diff --git a/src/agent_comms/acp_failure.py b/src/agent_comms/acp_failure.py index d6d141453b..e6caf6edf5 100644 --- a/src/agent_comms/acp_failure.py +++ b/src/agent_comms/acp_failure.py @@ -173,7 +173,6 @@ def summary(self) -> str: class ACPFailure(DeliveryFailure): code: int | None detail: str - classification_priority: ClassVar[int] = 0 input_state: type[InputAttempt] | None = None diagnostics: tuple[PiDiagnostic, ...] = () # Public failed-outcome projection of the turn-owned immutable result. @@ -224,11 +223,7 @@ def from_detail(cls, code, detail, *, diagnostics=()): detail = detail or "ACP request failed" owner = next( member - for member in sorted( - cls.members_with(ACPFailure), - key=lambda member: member.classification_priority, - reverse=True, - ) + for member in cls.members_with(ACPFailure) if member.matches(code, detail, diagnostics) ) return owner(code, detail, diagnostics=diagnostics) @@ -346,20 +341,33 @@ def raise_if_failed(self) -> NoReturn: raise NativeTurnError(self.request) -class RequestACPFailure(ACPFailure): +class ProviderConnectionFailure(ACPFailure): @classmethod - def matches( - cls, code: int | None, detail: str, diagnostics: tuple[PiDiagnostic, ...] = () - ) -> bool: - return True + def matches(cls, code, detail, diagnostics=()) -> bool: + return any(diagnostic.provider_connection_failure for diagnostic in diagnostics) @property def title(self) -> str: - return "Request failed" + return "Provider connection failed" + + @property + def description(self) -> str: + known = "\n".join( + diagnostic.description + for diagnostic in self.diagnostics + if diagnostic.provider_connection_failure + ) + return "\n".join(part for part in (self.detail, known) if part) + + @property + def next_step(self) -> str: + return ( + "The provider connection failed. Inspect the response and input delivery " + "before choosing whether to send a new message." + ) class ProviderQuotaFailure(ACPFailure): - classification_priority = 100 display_pattern = re.compile( r"usage limit|quota|insufficient credits|credit balance|rate.limit|too many requests", re.I, @@ -383,32 +391,16 @@ def next_step(self) -> str: ) -class ProviderConnectionFailure(ACPFailure): - classification_priority = 150 - +class RequestACPFailure(ACPFailure): @classmethod - def matches(cls, code, detail, diagnostics=()) -> bool: - return any(diagnostic.provider_connection_failure for diagnostic in diagnostics) + def matches( + cls, code: int | None, detail: str, diagnostics: tuple[PiDiagnostic, ...] = () + ) -> bool: + return True @property def title(self) -> str: - return "Provider connection failed" - - @property - def description(self) -> str: - known = "\n".join( - diagnostic.description - for diagnostic in self.diagnostics - if diagnostic.provider_connection_failure - ) - return "\n".join(part for part in (self.detail, known) if part) - - @property - def next_step(self) -> str: - return ( - "The provider connection failed. Inspect the response and input delivery " - "before choosing whether to send a new message." - ) + return "Request failed" class ErrorValue(DeclaredFamily, JsonShapeFamily, affix="ErrorValue"): diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index fafd3a6b34..6a4d8cb7a9 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -1038,7 +1038,7 @@ def read(cls, todo: Todo, registry: Registration, pulls: ProgressRead) -> OrderP @classmethod def from_source(cls, source: ProgressSource) -> OrderProgress: - members = sorted(cls.members_with(FoundProgress), key=lambda member: member.precedence) + members = cls.members_with(FoundProgress) try: return next((progress for member in members for progress in member.observations(source)), NoCommitProgress()) @@ -1068,8 +1068,7 @@ def missing_merges(self, source: ProgressSource) -> tuple[MergedProgress, ...]: @dataclass(frozen=True) class FoundProgress(OrderProgress): - """A state read finds in the checkout or on GitHub; the lowest ``precedence`` found wins.""" - precedence: ClassVar[int] + """A state read finds in the checkout or on GitHub; the first declaration found wins.""" @classmethod @abstractmethod @@ -1101,20 +1100,6 @@ def state(self) -> str: return "no commit since the order" -@dataclass(frozen=True) -class EditingProgress(Moving, NoCommitProgress, FoundProgress): - """Uncommitted changes in the holder's own checkout: work is moving, nothing is delivered.""" - precedence: ClassVar[int] = 6 - - @classmethod - def found(cls, source: ProgressSource) -> Iterator[Self]: - return (cls(at=edited) for edited in source.edited_at) - - @property - def state(self) -> str: - return f"uncommitted edits at {RecordedEventTime(self.at).clock()}, no commit" - - @dataclass(frozen=True) class UnreadProgress(OrderProgress): reason: str @@ -1133,9 +1118,41 @@ def observations(cls, source: ProgressSource) -> Iterator[Self]: for progress in cls.found(source)) +@dataclass(frozen=True) +class MergedProgress(FoundProgress): + """The forge proves a linked merge; the order's run owns what still needs doing.""" + sha: str + + @classmethod + def found(cls, source: ProgressSource) -> Iterator[Self]: + return (cls(sha) for pr in source.requests(MergedListedPullRequest) + for sha in pr.merged_commits(source.since)) + + @property + def state(self) -> str: + return f"merged as {self.sha[:9]}" + + def waiting_on_install(self, todo: Todo) -> tuple[Todo, ...]: + return (todo,) + + +@dataclass(frozen=True) +class OpenPrProgress(GitProgress): + number: int + head: str + + @classmethod + def found(cls, source: ProgressSource) -> Iterator[Self]: + return (cls(pr.number, pr.head[:9], at=pr.updated_at, + readings=source.readings()) for pr in source.requests(UnmergedListedPullRequest)) + + @property + def state(self) -> str: + return f"PR #{self.number} open at {self.head}" + + @dataclass(frozen=True) class CommittedProgress(GitProgress): - precedence: ClassVar[int] = 4 sha: str @classmethod @@ -1151,7 +1168,6 @@ def state(self) -> str: @dataclass(frozen=True) class PushedProgress(CommittedProgress): """Pushed past the default branch, with no PR naming the order yet.""" - precedence: ClassVar[int] = 5 @classmethod def found(cls, source: ProgressSource) -> Iterator[Self]: @@ -1166,38 +1182,16 @@ def state(self) -> str: @dataclass(frozen=True) -class OpenPrProgress(GitProgress): - precedence: ClassVar[int] = 3 - number: int - head: str - - @classmethod - def found(cls, source: ProgressSource) -> Iterator[Self]: - return (cls(pr.number, pr.head[:9], at=pr.updated_at, - readings=source.readings()) for pr in source.requests(UnmergedListedPullRequest)) - - @property - def state(self) -> str: - return f"PR #{self.number} open at {self.head}" - - -@dataclass(frozen=True) -class MergedProgress(FoundProgress): - """The forge proves a linked merge; the order's run owns what still needs doing.""" - precedence: ClassVar[int] = 1 - sha: str +class EditingProgress(Moving, NoCommitProgress, FoundProgress): + """Uncommitted changes in the holder's own checkout: work is moving, nothing is delivered.""" @classmethod def found(cls, source: ProgressSource) -> Iterator[Self]: - return (cls(sha) for pr in source.requests(MergedListedPullRequest) - for sha in pr.merged_commits(source.since)) + return (cls(at=edited) for edited in source.edited_at) @property def state(self) -> str: - return f"merged as {self.sha[:9]}" - - def waiting_on_install(self, todo: Todo) -> tuple[Todo, ...]: - return (todo,) + return f"uncommitted edits at {RecordedEventTime(self.at).clock()}, no commit" @dataclass(frozen=True) diff --git a/src/agent_comms/tools.py b/src/agent_comms/tools.py index aaa4df6cd4..c4a2f03fb5 100644 --- a/src/agent_comms/tools.py +++ b/src/agent_comms/tools.py @@ -421,7 +421,6 @@ class ToolRequest[Caller: ToolCaller = ToolCaller]( ) context: ClassVar[str | None] = None action_label: ClassVar[str | None] = None - action_order: ClassVar[int] = 0 @classmethod def context_bindings(cls) -> dict[str, type[ContextBinding]]: @@ -443,7 +442,7 @@ def schema(cls) -> dict[str, object]: result.update( context=cls.context, action_label=cls.action_label or cls.label, - action_order=cls.action_order, + action_order=cls.declared_position(), context_bindings={ key: binding.declared_name for key, binding in cls.context_bindings().items() }, @@ -1587,13 +1586,32 @@ def apply(self, comms: Comms, caller: ToolCaller) -> InboxPageRead: ) +@dataclass(frozen=True, kw_only=True) +class CommsRestartQueueTool(ReadTool, ToolRequest): + label = "Restart Queue Status" + description = "Inspect queued, restarted, stale, or uncertain attempts for a thread." + name: Annotated[str, *tool_field("Agent thread")] + + def apply(self, comms: Comms, caller: ToolCaller) -> RestartAttempts: + return RestartAttempts(tuple(restart_status(comms, self.name))) + + +@dataclass(frozen=True, kw_only=True) +class CommsCancelRestartTool(ToolRequest): + label = "Cancel Queued Restart" + description = "Cancel pending restarts for a thread; cannot undo an attempted restart." + name: Annotated[str, *tool_field("Agent thread")] + + def apply(self, comms: Comms, caller: ToolCaller) -> RestartsCancelled: + return RestartsCancelled(tuple(cancel_restart(comms, self.name))) + + @dataclass(frozen=True, kw_only=True) class CommsForkTool(LiveOwnerTool, NativeOwnerCommand, ToolRequest): label = "Comms Fork" description = "Fork a child agent thread from a registered parent's persistent session." context = "thread" action_label = "Fork from this thread" - action_order = 10 name: Annotated[str, *tool_field("Child thread name")] parent: Annotated[str, *tool_field("Parent thread name", binding=SubjectBinding)] task: Annotated[str, *tool_field("Optional task for the child")] = "" @@ -1615,7 +1633,6 @@ class CommsStopTool(NativeOwnerCommand, OwnerLifecycleControl, ToolRequest): description = "Stop a thread after verifying that its process owns the registered identity." context = "thread" action_label = "Stop process" - action_order = 20 name: Annotated[str, *tool_field("Thread name", binding=SubjectBinding)] def apply(self, comms: Comms, caller: ToolCaller) -> ThreadStoppedResult: @@ -1632,7 +1649,6 @@ class CommsStartTool(LiveOwnerTool, NativeOwnerCommand, StartOwnerControl, ToolR ) context = "thread" action_label = "Start thread" - action_order = 21 name: Annotated[str, *tool_field("Thread to start", binding=SubjectBinding)] def apply(self, comms: Comms, caller: ToolCaller) -> OwnerStartResult: @@ -1649,7 +1665,6 @@ class CommsQueueRestartTool(LiveOwnerTool, NativeOwnerCommand, OwnerLifecycleCon ) context = "thread" action_label = "Queue idle restart" - action_order = 22 name: Annotated[str, *tool_field("Live agent thread", binding=SubjectBinding)] def apply(self, comms: Comms, caller: ToolCaller) -> RestartQueued: @@ -1658,33 +1673,12 @@ def apply(self, comms: Comms, caller: ToolCaller) -> RestartQueued: launch_runtime=LaunchRuntime.running())) -@dataclass(frozen=True, kw_only=True) -class CommsRestartQueueTool(ReadTool, ToolRequest): - label = "Restart Queue Status" - description = "Inspect queued, restarted, stale, or uncertain attempts for a thread." - name: Annotated[str, *tool_field("Agent thread")] - - def apply(self, comms: Comms, caller: ToolCaller) -> RestartAttempts: - return RestartAttempts(tuple(restart_status(comms, self.name))) - - -@dataclass(frozen=True, kw_only=True) -class CommsCancelRestartTool(ToolRequest): - label = "Cancel Queued Restart" - description = "Cancel pending restarts for a thread; cannot undo an attempted restart." - name: Annotated[str, *tool_field("Agent thread")] - - def apply(self, comms: Comms, caller: ToolCaller) -> RestartsCancelled: - return RestartsCancelled(tuple(cancel_restart(comms, self.name))) - - @dataclass(frozen=True, kw_only=True) class CommsArchiveTool(ArchiveOwnerControl, ToolRequest): label = "Archive Comms Thread" description = "Archive a stopped thread while retaining its messages." context = "thread" action_label = "Archive stopped thread" - action_order = 30 name: Annotated[str, *tool_field("Stopped thread name", binding=SubjectBinding)] def apply(self, comms: Comms, caller: ToolCaller) -> ThreadArchivedResult: @@ -1741,11 +1735,8 @@ def run(self, ctx: Comms, output: TextIO) -> int: def context_tool_catalog(context: str) -> list[dict[str, object]]: - tools = sorted( - (tool for tool in ToolRequest.members_with(ToolRequest) if tool.context == context), - key=lambda tool: tool.action_order, - ) - return [tool.schema() for tool in tools] + return [tool.schema() for tool in ToolRequest.members_with(ToolRequest) + if tool.context == context] def invoke_tool( diff --git a/tests/test_acp_failure.py b/tests/test_acp_failure.py index ec5cbe162e..f9f29b425b 100644 --- a/tests/test_acp_failure.py +++ b/tests/test_acp_failure.py @@ -48,17 +48,9 @@ def test_public_not_sent_and_ambiguous_unknown_do_not_invent_binding(): assert unknown.input_state is None and unknown.input_disposition == "Unconfirmed" -def test_new_display_case_needs_only_its_declaration(): - class FixtureDisplayFailure(ACPFailure): - classification_priority = 200 - title = "Fixture feedback" - - @classmethod - def matches(cls, code, detail, diagnostics=()): - return detail == "T2 display extension fixture" - - failure = ACPFailure.from_error(RequestError(-32603, "T2 display extension fixture")) - assert isinstance(failure, FixtureDisplayFailure) +def test_unmatched_display_detail_reaches_the_last_declared_fallback(): + failure = ACPFailure.from_error(RequestError(-32603, "T2 unmatched display detail")) + assert isinstance(failure, RequestACPFailure) assert failure.input_disposition == "Unconfirmed" assert decode_updates(encode_updates(RequestFailedUpdate(failure)))[0].failure == failure diff --git a/tests/test_pi_rpc_nominal.py b/tests/test_pi_rpc_nominal.py index 001ffee0f8..a94dcd3c93 100644 --- a/tests/test_pi_rpc_nominal.py +++ b/tests/test_pi_rpc_nominal.py @@ -46,11 +46,6 @@ class AuditPauseEnd(pi.PiEvent, ends=AuditPausePhase): pass -class AuditFailure(failures.TurnFailure): - code = "audit_failure" - precedence = 110 - - def test_new_excursion_declares_both_transitions_without_dispatch_edits(): phase = PiRpcChannel.decode_record(b'{"type":"audit_pause_start"}\n').on(ModelWaitPhase()) assert isinstance(phase, AuditPausePhase) @@ -108,7 +103,7 @@ def test_phase_watchdogs_and_overlapping_tools_preserve_protocol_semantics(): failures.InputMissing, failures.FinalStopMissing, failures.QueuedInputMissing, - AuditFailure, + failures.ModelRequestFailed, ], 2, ) @@ -121,7 +116,7 @@ def test_failure_precedence_and_text_have_one_owner(left, right): ) for cls in (left, right, left): session.output.record_failure(cls(cls.__name__)) - expected = max((left, right), key=lambda cls: cls.precedence) + expected = min((left, right), key=lambda cls: cls.declared_position()) assert type(session.output.failure) is expected assert session.output.failure_text == expected.__name__ assert session.output.failure.code == expected.code From 04e0194c68ab368b66efec9af4e628fc6e95e9a0 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:54:14 -0400 Subject: [PATCH 12/18] Fold backend relocations into their behavior owners --- src/agent_comms/historical_views.py | 2 +- src/agent_comms/pi_events.py | 25 +++- src/agent_comms/pi_turn_phase.py | 33 ----- src/agent_comms/plan_phases.py | 10 +- src/agent_comms/presentation.py | 2 +- src/agent_comms/private_registry_guard.py | 73 ++++++++++ src/agent_comms/registry_provenance.py | 2 +- src/agent_comms/thread_management.py | 25 +++- src/agent_comms/thread_provenance.py | 68 --------- src/agent_comms/thread_rename.py | 154 -------------------- src/agent_comms/threads.py | 82 ++++++++--- src/agent_comms/tools.py | 3 +- src/agent_comms/transcripts.py | 3 +- tests/guards/test_source_proof_ownership.py | 2 +- tests/test_historical_views.py | 2 +- tests/test_importing.py | 2 +- tests/test_phase_evidence.py | 6 +- 17 files changed, 198 insertions(+), 296 deletions(-) delete mode 100644 src/agent_comms/pi_turn_phase.py delete mode 100644 src/agent_comms/thread_provenance.py delete mode 100644 src/agent_comms/thread_rename.py diff --git a/src/agent_comms/historical_views.py b/src/agent_comms/historical_views.py index 8c175b58cd..aa9ba46a46 100644 --- a/src/agent_comms/historical_views.py +++ b/src/agent_comms/historical_views.py @@ -49,7 +49,7 @@ from .revision_views import BusSourceView from .store_files import _atomic_write_text, _store_lock from .thread_identity import UnrecordedThreadIncarnation -from .thread_provenance import ThreadProvenance +from .threads import ThreadProvenance if TYPE_CHECKING: from .catalog_document import CatalogDocument diff --git a/src/agent_comms/pi_events.py b/src/agent_comms/pi_events.py index fbe3953b28..c1f08e013a 100644 --- a/src/agent_comms/pi_events.py +++ b/src/agent_comms/pi_events.py @@ -39,7 +39,6 @@ UnknownPayload, UnreportedUsage, ) -from .pi_turn_phase import PiTurnPhase from .pi_vocabulary import CompactionReason, UnknownCompactionReason from .request_progress import RequestProgress from .tool_declaration import ToolDeclaration @@ -57,9 +56,31 @@ def retry_progress(self) -> bool: @dataclass(frozen=True) -class PiEvent(PiTurnPhase, OpenPayload, affix=""): +class PiEvent(OpenPayload, affix=""): """One declaration-owned record received by the Pi RPC channel.""" + _start_phases: ClassVar[dict[type[object], tuple[type[phases.TurnPhase], ...]]] = {} + _end_events: ClassVar[dict[type[object], frozenset[type[object]]]] = {} + + def on(self, current: phases.TurnPhase) -> phases.TurnPhase: + for event_type in type(self).__mro__: + for member in self._start_phases.get(event_type, ()): + return self.enter(member, current) + return self.leave(current) if self.matches_end(current) else current + + def enter(self, member: type[phases.TurnPhase], current: phases.TurnPhase) -> phases.TurnPhase: + return member() + + def matches_end(self, current: phases.TurnPhase) -> bool: + return any( + event_type in self._end_events.get(member, frozenset()) + for member in type(current).__mro__ + for event_type in type(self).__mro__ + ) + + def leave(self, current: phases.TurnPhase) -> phases.TurnPhase: + return phases.ModelWaitPhase() + def __init_subclass__( cls, *, starts: type[phases.TurnPhase] | None = None, ends: type[phases.TurnPhase] | None = None, **kwargs, diff --git a/src/agent_comms/pi_turn_phase.py b/src/agent_comms/pi_turn_phase.py deleted file mode 100644 index 91984b4cf7..0000000000 --- a/src/agent_comms/pi_turn_phase.py +++ /dev/null @@ -1,33 +0,0 @@ -"""Pi event declarations own their transitions into shared turn phases.""" - -from __future__ import annotations - -from typing import ClassVar - -from . import turn_phase as phases - - -class PiTurnPhase: - """Pi declarations own event entry/end matching; shared phases own stage behavior.""" - - _start_phases: ClassVar[dict[type[object], tuple[type[phases.TurnPhase], ...]]] = {} - _end_events: ClassVar[dict[type[object], frozenset[type[object]]]] = {} - - def on(self, current: phases.TurnPhase) -> phases.TurnPhase: - for event_type in type(self).__mro__: - for member in self._start_phases.get(event_type, ()): - return self.enter(member, current) - return self.leave(current) if self.matches_end(current) else current - - def enter(self, member: type[phases.TurnPhase], current: phases.TurnPhase) -> phases.TurnPhase: - return member() - - def matches_end(self, current: phases.TurnPhase) -> bool: - return any( - event_type in self._end_events.get(member, frozenset()) - for member in type(current).__mro__ - for event_type in type(self).__mro__ - ) - - def leave(self, current: phases.TurnPhase) -> phases.TurnPhase: - return phases.ModelWaitPhase() diff --git a/src/agent_comms/plan_phases.py b/src/agent_comms/plan_phases.py index 24fa581c95..980e33401e 100644 --- a/src/agent_comms/plan_phases.py +++ b/src/agent_comms/plan_phases.py @@ -13,9 +13,9 @@ from agent_comms.declared_family import DeclaredFamily from agent_comms.installer.carry import Carry -from agent_comms.pi_turn_phase import PiTurnPhase +from agent_comms.pi_events import PiEvent from agent_comms.thread_identity import BirthlessIncarnation -from agent_comms.thread_rename import ThreadRename +from agent_comms.private_registry_guard import RegistryPrivacy from agent_comms.tool_calls import CliToolArguments from agent_comms.phase_evidence import ( AllEvidence, @@ -667,11 +667,11 @@ class RostersPhase(RemainingSlopPhase): ClassFlagsAbsent(r"^\s*" + attribute + r"\s*[:=]", owners=(owner,)) for owner, attribute in ( (Carry, "declared"), - (ThreadRename, "kinds"), + (RegistryPrivacy, "kinds"), (BirthlessIncarnation, "members"), (CliToolArguments, "types"), - (PiTurnPhase, "_start_phases"), - (PiTurnPhase, "_end_events"), + (PiEvent, "_start_phases"), + (PiEvent, "_end_events"), ) ), ) diff --git a/src/agent_comms/presentation.py b/src/agent_comms/presentation.py index 259db557c1..dc41e9ba31 100644 --- a/src/agent_comms/presentation.py +++ b/src/agent_comms/presentation.py @@ -75,7 +75,7 @@ from .runtime_info import RuntimeInfo from .storage_room import RoomLedger from .thread_identity import ThreadIncarnation -from .thread_provenance import ThreadProvenance +from .threads import ThreadProvenance from .thread_presentation import ( ThreadOwnerBinding, ThreadPresentation, diff --git a/src/agent_comms/private_registry_guard.py b/src/agent_comms/private_registry_guard.py index 4e8025d103..58941c1130 100644 --- a/src/agent_comms/private_registry_guard.py +++ b/src/agent_comms/private_registry_guard.py @@ -29,7 +29,9 @@ from typing import TYPE_CHECKING, Annotated, NoReturn from . import store_files +from .coordination_contracts import MAX_IDENTIFIER_CHARS from .coordination_database import CoordinationStore +from .coordination_errors import ParticipantNotRegistered from .coordinator import Coordination from .errors import RelationViolationError from .field_codec import FieldCodec, Id128Text, MalformedRecord, WireName @@ -44,6 +46,7 @@ from .notification_assignment import NotificationAssignmentRead from .private_nk_entrypoint import RootLaunch from .registry_document import RegistrySnapshot + from .thread_management import ThreadManagement from .threads import Thread from .typed_table import TableSchema @@ -118,6 +121,15 @@ def _decode(data: bytes, root_id: bytes) -> tuple[int, int, bytes]: class RegistryPrivacy(ABC): """What a registry root's protocol marker decides, selected by the registry store.""" + @abstractmethod + def rename( + self, threads: ThreadManagement, before: Thread, new_name: str + ) -> tuple[str, str]: + """Rename under this root's original registry and coordinator authority.""" + + def rename_applied(self, root: Path) -> None: + """Public roots have no private owner intent to complete.""" + @staticmethod def capture(root: Path) -> RegistryPrivacy: """Read original privacy authority under its registry's existing lock.""" @@ -174,6 +186,11 @@ def coordinator_schema(self) -> type[TableSchema]: class PublicRegistry(RegistryPrivacy): + def rename( + self, threads: ThreadManagement, before: Thread, new_name: str + ) -> tuple[str, str]: + return threads.registry.rename(before.name, new_name) + def coordinator_schema(self) -> type[TableSchema]: from .coordination_schema import CoordinatorTable @@ -231,6 +248,62 @@ def require_root(self, root_id: str, error: Exception) -> None: class PrivateRegistryGuard(DurableFile, WireWakeFile, RegistryPrivacy): + def rename( + self, threads: ThreadManagement, before: Thread, new_name: str + ) -> tuple[str, str]: + root, registry = threads.root, threads.registry + _require_no_private_owner_rename(root) + coordinator = CoordinationStore.path_in(root) + if not coordinator.exists() or before.name == new_name: + return registry.rename(before.name, new_name) + # Validate registry/SQL name refusals before renaming the private owner. + # Registry readers cannot select a different owner while both stores change. + registry.status(before.name).require_self_rename() + if registry.snapshot().taken(new_name, owner=before.name): + raise RelationViolationError(f"Thread name {new_name!r} is already in use.") + before.require_name(new_name) + if len(new_name) > MAX_IDENTIFIER_CHARS: + raise ValueError("Private coordinator owner name exceeds its bound.") + with Coordination(str(coordinator)) as store: + try: + person = store.participants.get(before.incarnation.require_recorded().lookup) + except ParticipantNotRegistered: + return registry.rename(before.name, new_name) + if not person.committed or person.owner_identity.owner_thread != before.name: + raise RelationViolationError( + "Private coordinator owner differs from the registered owner." + ) + generation = person.owner_identity.generation + PrivateOwnerRenameIntent( + lookup=person.lookup, + old=before.name, + new=new_name, + generation=generation, + wire_root_id=self.root_id.hex(), + ).write(root) + store.participants.rename_owner(person.lookup, new_name, expected_generation=generation) + try: + return registry.rename(before.name, new_name) + except BaseException as error: + # Registry persistence can fail after the SQL CAS. If its atomic snapshot still + # names the old owner, restore that name without revoking its active attempt. + uncertain = RelationViolationError( + "Private owner rename is uncertain; inspect both authorities." + ) + try: + if registry.require(before.name).name == before.name: + store.participants.rename_owner( + person.lookup, before.name, expected_generation=generation + ) + except BaseException as compensation: + # Ambiguous dual-store failure: the inspection needs both failures. + raise uncertain from compensation + raise uncertain from error + + def rename_applied(self, root: Path) -> None: + if PrivateOwnerRenameIntent.path_in(root).exists(): + PrivateOwnerRenameIntent.complete(root) + filename = ".registry-owner-guard" def coordinator_schema(self) -> type[TableSchema]: diff --git a/src/agent_comms/registry_provenance.py b/src/agent_comms/registry_provenance.py index 86a6373275..c0c6333630 100644 --- a/src/agent_comms/registry_provenance.py +++ b/src/agent_comms/registry_provenance.py @@ -15,7 +15,7 @@ from .errors import RelationViolationError, UnregisteredThreadError from .routing import DeliveryScope from .thread_identity import ThreadIncarnation -from .thread_provenance import ThreadProvenance +from .threads import ThreadProvenance if TYPE_CHECKING: from .audience_manifest import FrozenRecipient diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index d1d18b589d..27f6b32125 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -49,10 +49,17 @@ from .session_file import SavedSessionFile from .thread_model import DeclaredModel, ThreadModel, UndeclaredModel from .thread_owned_state import ThreadOwnedState -from .thread_rename import RenameThreadResult, ThreadRename +from .tool_result import ToolResult from .threads import KeptModel, ModelChoice, NextTurnConfiguration, Thread, current_thread +@dataclass(frozen=True, slots=True) +class RenameThreadResult(ToolResult): + previous: str + current: str + changed: bool + + class TagChoice(SubclassUnion, affix="Tags"): """Select explicit, inherited or changed tags from an existing set.""" @@ -380,7 +387,21 @@ def _rename_thread( def _rename_thread_unlocked( self, name: str, new_name: str, *, title: ThreadText = NoThreadText() ) -> RenameThreadResult: - return ThreadRename.chosen(self, self.registry.require(name), new_name).applied(title) + privacy = self._privacy() + previous, current = privacy.rename(self, self.registry.require(name), new_name) + thread = self.registry.require(current) + titled = replace(thread, auto_title_pending=False, title=title.applied(thread.title)) + if titled != thread: + self.registry.register(titled, self.registry.status(current)) + changed = previous != current + if changed: + self.agents.activity.rename_thread(previous, current) + self.agents.runtime_info.rename_thread(previous, current) + self.ledger.rename_thread(previous, current) + with self.catalog.editing() as document: + document.rename_thread(previous, current) + privacy.rename_applied(self.root) + return RenameThreadResult(previous, current, changed) def heartbeat(self, name: str) -> None: self.registry.heartbeat(name) diff --git a/src/agent_comms/thread_provenance.py b/src/agent_comms/thread_provenance.py deleted file mode 100644 index a2c21d6f4e..0000000000 --- a/src/agent_comms/thread_provenance.py +++ /dev/null @@ -1,68 +0,0 @@ -"""A recorded thread declaration, independent of live execution authority.""" - -from __future__ import annotations - -from dataclasses import dataclass, field, fields -from functools import cached_property -from typing import Annotated - -from .agent_backend import AgentBackend, NewThreadBackend, RuntimeLocation -from .channel_targets import Tag -from .field_codec import WireOmitDefault, WireRequired -from .thread_identity import ThreadIncarnation, ThreadIncarnationRecord -from .thread_text import NoThreadText, ThreadText -from .session_file import NoSessionFile, SessionFile - - -@dataclass(frozen=True, kw_only=True) -class ThreadProvenance: - """Recorded thread identity, without permission to own a process or start a turn.""" - - name: str - tags: frozenset[str] - worktree: str - created_at: Annotated[float, WireRequired()] - session_file: SessionFile = field(default_factory=NoSessionFile) - title: ThreadText = field(default_factory=NoThreadText) - backend: Annotated[type[AgentBackend], WireOmitDefault()] = field( - default_factory=NewThreadBackend.selected - ) - - @cached_property - def runtime(self) -> RuntimeLocation: - """The codec decoded both stored fields; their record owns this binding.""" - return self.session_file.location(self.backend) - - @property - def display_title(self) -> str: - return self.title.otherwise(self.name) - - @staticmethod - def require_name(name: str) -> None: - allowed = set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_") - if not name or not set(name) <= allowed: - raise ValueError(f"Thread name {name!r} must be alphanumeric with hyphens/underscores.") - - def __post_init__(self) -> None: - self.incarnation - self.require_name(self.name) - if not self.worktree: - raise ValueError("Thread worktree cannot be empty.") - for tag in self.tags: - Tag.validate(tag) - - @staticmethod - def capture(thread: ThreadProvenance) -> ThreadProvenance: - """Freeze only determining provenance from the original typed declaration.""" - return ThreadProvenance( - **{ - item.name: getattr(thread, item.name) - for item in fields(ThreadProvenance) - if item.init - } - ) - - @cached_property - def incarnation(self) -> ThreadIncarnation: - """The declaration's birth member, created once and shared by its readers.""" - return ThreadIncarnationRecord(self.name, self.created_at).incarnation() diff --git a/src/agent_comms/thread_rename.py b/src/agent_comms/thread_rename.py deleted file mode 100644 index b89cbadd96..0000000000 --- a/src/agent_comms/thread_rename.py +++ /dev/null @@ -1,154 +0,0 @@ -"""The rename operation and its privacy-specific ownership transaction.""" - -from __future__ import annotations - -from abc import ABC, abstractmethod -from dataclasses import dataclass, replace -from typing import TYPE_CHECKING, ClassVar - -from .coordination_contracts import MAX_IDENTIFIER_CHARS -from .coordination_database import CoordinationStore -from .coordination_errors import ParticipantNotRegistered -from .coordinator import Coordination -from .errors import RelationViolationError -from .private_registry_guard import ( - PrivateOwnerRenameIntent, - PrivateRegistryGuard, - PublicRegistry, - RegistryPrivacy, - _require_no_private_owner_rename, -) -from .thread_provenance import ThreadProvenance -from .thread_text import ThreadText -from .tool_result import ToolResult - -if TYPE_CHECKING: - from .thread_management import ThreadManagement - from .threads import Thread - - -@dataclass(frozen=True, slots=True) -class RenameThreadResult(ToolResult): - previous: str - current: str - changed: bool - - -@dataclass(frozen=True, slots=True) -class ThreadRename(ABC): - """One rename, its kind derived from the registry's privacy.""" - - privacy: ClassVar[type[RegistryPrivacy]] - kinds: ClassVar[dict[type[RegistryPrivacy], type[ThreadRename]]] = {} - threads: ThreadManagement - before: Thread - new_name: str - registry_privacy: RegistryPrivacy - - def __init_subclass__(cls, **kwargs): - # slots=True rebuilds the class; its final declaration replaces the first. - super(ThreadRename, cls).__init_subclass__(**kwargs) - cls.kinds[cls.privacy] = cls - - @classmethod - def chosen(cls, threads: ThreadManagement, before: Thread, new_name: str) -> ThreadRename: - privacy = threads._privacy() - return cls.kinds[type(privacy)](threads, before, new_name, privacy) - - @abstractmethod - def renamed(self) -> tuple[str, str]: - """Move the registry name, after whatever this kind's owner stores need first.""" - - def registry_renamed(self) -> tuple[str, str]: - return self.threads.registry.rename(self.before.name, self.new_name) - - def applied(self, title: ThreadText) -> RenameThreadResult: - previous, current = self.renamed() - threads = self.threads - thread = threads.registry.require(current) - titled = replace(thread, auto_title_pending=False, title=title.applied(thread.title)) - if titled != thread: - threads.registry.register(titled, threads.registry.status(current)) - if previous == current: - return RenameThreadResult(previous, current, False) - threads.agents.activity.rename_thread(previous, current) - threads.agents.runtime_info.rename_thread(previous, current) - threads.ledger.rename_thread(previous, current) - with threads.catalog.editing() as document: - document.rename_thread(previous, current) - return RenameThreadResult(previous, current, True) - - -class PlainRename(ThreadRename): - privacy = PublicRegistry - - def renamed(self) -> tuple[str, str]: - return self.registry_renamed() - - -@dataclass(frozen=True, slots=True) -class PrivateOwnerRename(ThreadRename): - """Moves a committed coordinator owner under a durable intent, so a crash between the two - stores fails closed instead of turning an old attempt into a new send.""" - - privacy = PrivateRegistryGuard - registry_privacy: PrivateRegistryGuard - - def renamed(self) -> tuple[str, str]: - root, registry = self.threads.root, self.threads.registry - before, new_name = self.before, self.new_name - _require_no_private_owner_rename(root) - coordinator = CoordinationStore.path_in(root) - if not coordinator.exists() or before.name == new_name: - return self.registry_renamed() - # Validate registry/SQL name refusals before renaming the private owner. - # Registry readers cannot select a different owner while both stores change. - registry.status(before.name).require_self_rename() - if registry.snapshot().taken(new_name, owner=before.name): - raise RelationViolationError(f"Thread name {new_name!r} is already in use.") - ThreadProvenance.require_name(new_name) - if len(new_name) > MAX_IDENTIFIER_CHARS: - raise ValueError("Private coordinator owner name exceeds its bound.") - with Coordination(str(coordinator)) as store: - try: - person = store.participants.get(before.incarnation.require_recorded().lookup) - except ParticipantNotRegistered: - return self.registry_renamed() - if not person.committed or person.owner_identity.owner_thread != before.name: - raise RelationViolationError( - "Private coordinator owner differs from the registered owner." - ) - generation = person.owner_identity.generation - PrivateOwnerRenameIntent( - lookup=person.lookup, - old=before.name, - new=new_name, - generation=generation, - wire_root_id=self.registry_privacy.root_id.hex(), - ).write(root) - store.participants.rename_owner(person.lookup, new_name, expected_generation=generation) - try: - return self.registry_renamed() - except BaseException as error: - # Registry persistence can fail after the SQL CAS. If its atomic snapshot still - # names the old owner, restore that name without revoking its active attempt. - uncertain = RelationViolationError( - "Private owner rename is uncertain; inspect both authorities." - ) - try: - if registry.require(before.name).name == before.name: - store.participants.rename_owner( - person.lookup, before.name, expected_generation=generation - ) - except BaseException as compensation: - # Ambiguous dual-store failure: the inspection needs both failures. - raise uncertain from compensation - raise uncertain from error - - def applied(self, title: ThreadText) -> RenameThreadResult: - result = ThreadRename.applied(self, title) - # The registry lock and renamed() bind a present intent to this rename; - # it completes only after both authorities and the ancillary stores agree. - if PrivateOwnerRenameIntent.path_in(self.threads.root).exists(): - PrivateOwnerRenameIntent.complete(self.threads.root) - return result diff --git a/src/agent_comms/threads.py b/src/agent_comms/threads.py index e01e1f3499..672a679c43 100644 --- a/src/agent_comms/threads.py +++ b/src/agent_comms/threads.py @@ -6,12 +6,14 @@ import os import time from abc import ABC, abstractmethod -from dataclasses import dataclass, field, replace +from dataclasses import dataclass, field, fields, replace +from functools import cached_property from pathlib import Path from typing import TYPE_CHECKING, ClassVar, Annotated -from .agent_backend import AgentBackend, NewThreadBackend +from .agent_backend import AgentBackend, NewThreadBackend, RuntimeLocation +from .channel_targets import Tag from .child_process import NoProcess, ProcessIdentity, ProcessState from .cli_result import ThreadToolCatalogResult from .context_segments.wake_values import WakeWorkContext @@ -47,9 +49,10 @@ from .thread_model import DeclaredModel, ThreadModel, UndeclaredModel from .thread_text import NoParent, NoThreadText, ThreadParent, ThreadText from .session_file import NoSessionFile, SessionFile -from .thread_provenance import ThreadProvenance from .thread_identity import ( AgentThreadRole, + ThreadIncarnation, + ThreadIncarnationRecord, ThreadRole, TurnFence, TurnId, @@ -139,23 +142,72 @@ def configured(self, thread: Thread) -> Thread: return self.service_tier.configured(self.model.configured(thought)) +@dataclass(frozen=True, kw_only=True) +class ThreadProvenance: + """Recorded thread identity, without permission to own a process or start a turn.""" + + name: str = field(kw_only=False) + tags: Annotated[frozenset[str], RegistrationPolicy(InheritEmpty)] = field(kw_only=False) + worktree: str = field(kw_only=False) + created_at: Annotated[float, WireRequired()] + session_file: Annotated[SessionFile, RegistrationPolicy(InheritApplied)] = field( + default_factory=NoSessionFile + ) + title: Annotated[ThreadText, RegistrationPolicy(InheritApplied)] = field( + default_factory=NoThreadText + ) + backend: Annotated[ + type[AgentBackend], WireOmitDefault(), RegistrationPolicy(InheritPrevious) + ] = field(default_factory=NewThreadBackend.selected) + + @cached_property + def runtime(self) -> RuntimeLocation: + """The codec decoded both stored fields; their record owns this binding.""" + return self.session_file.location(self.backend) + + @property + def display_title(self) -> str: + return self.title.otherwise(self.name) + + @staticmethod + def require_name(name: str) -> None: + allowed = set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_") + if not name or not set(name) <= allowed: + raise ValueError(f"Thread name {name!r} must be alphanumeric with hyphens/underscores.") + + def __post_init__(self) -> None: + self.require_name(self.incarnation.name) + if not self.worktree: + raise ValueError("Thread worktree cannot be empty.") + for tag in self.tags: + Tag.validate(tag) + + @staticmethod + def capture(thread: ThreadProvenance) -> ThreadProvenance: + """Freeze only determining provenance from the original typed declaration.""" + return ThreadProvenance( + **{ + item.name: getattr(thread, item.name) + for item in fields(ThreadProvenance) + if item.init + } + ) + + @cached_property + def incarnation(self) -> ThreadIncarnation: + """The declaration's birth member, created once and shared by its readers.""" + return ThreadIncarnationRecord(self.name, self.created_at).incarnation() + + @dataclass(frozen=True, slots=True) class Thread(DurableFile, PatternFile, ThreadProvenance): filename: ClassVar[str] = "native-sessions/{lookup}/{name}" catalog_filename: ClassVar[str] = "comms-catalog.json" """Declares one agent thread's identity and provenance.""" - name: str = field(kw_only=False) - tags: Annotated[frozenset[str], RegistrationPolicy(InheritEmpty)] = field( - kw_only=False - ) - worktree: str = field(kw_only=False) parent: ThreadParent = field(default_factory=NoParent) task: ThreadText = field(default_factory=NoThreadText) process_identity: ProcessState = field(default_factory=NoProcess) - session_file: Annotated[SessionFile, RegistrationPolicy(InheritApplied)] = ( - field(default_factory=NoSessionFile, kw_only=True) - ) model: Annotated[ThreadModel, RegistrationPolicy(InheritApplied)] = field( default_factory=UndeclaredModel ) @@ -190,20 +242,12 @@ class Thread(DurableFile, PatternFile, ThreadProvenance): project_resume_due: Annotated[ bool, RegistrationPolicy(InheritPrevious), WireOmitDefault() ] = False - title: Annotated[ThreadText, RegistrationPolicy(InheritApplied)] = field( - default_factory=NoThreadText, kw_only=True - ) role: Annotated[type[ThreadRole], RegistrationPolicy(InheritPrevious)] = ( AgentThreadRole ) execution: Annotated[type[ThreadExecution], WireOmitDefault()] = field( default=NativeThreadExecution, kw_only=True ) - backend: Annotated[ - type[AgentBackend], - WireOmitDefault(), - RegistrationPolicy(InheritPrevious), - ] = field(default_factory=NewThreadBackend.selected, kw_only=True) turn_record: TurnRecord = field(default_factory=NoTurnRecord) channel_scope_generation: int = 0 turn_generation: int = 0 diff --git a/src/agent_comms/tools.py b/src/agent_comms/tools.py index 7f055133eb..85ea877ab9 100644 --- a/src/agent_comms/tools.py +++ b/src/agent_comms/tools.py @@ -156,8 +156,7 @@ ThreadCommandAvailability, ) from .thread_identity import ThreadIncarnation -from .thread_management import InheritedTags, TagChoice -from .thread_rename import RenameThreadResult +from .thread_management import InheritedTags, RenameThreadResult, TagChoice from .thread_model import ThreadModel from .threads import NextTurnConfiguration from .tool_caller import NoThreadCaller, ThreadCaller, ThreadListing, ToolCaller diff --git a/src/agent_comms/transcripts.py b/src/agent_comms/transcripts.py index 37757e260e..5ca82a8a05 100644 --- a/src/agent_comms/transcripts.py +++ b/src/agent_comms/transcripts.py @@ -33,8 +33,7 @@ from .recorded_bus import RecordedBusIdentity from .registration import Registration from .routing import TurnRouting -from .thread_provenance import ThreadProvenance -from .threads import Thread +from .threads import Thread, ThreadProvenance from .transcript_events import ForkOriginTranscript, ForkTaskTranscript, TranscriptEvent from .transcript_routes import TranscriptRoutes from .transcript_sources import ( diff --git a/tests/guards/test_source_proof_ownership.py b/tests/guards/test_source_proof_ownership.py index 4b899cbdf0..281e18c8e2 100644 --- a/tests/guards/test_source_proof_ownership.py +++ b/tests/guards/test_source_proof_ownership.py @@ -79,7 +79,7 @@ def test_archived_identity_cannot_reacquire_a_live_registry_or_thread(): from dataclasses import MISSING, fields from typing import get_type_hints from agent_comms.historical_views import HistorySource, HistoricalThread - from agent_comms.thread_provenance import ThreadProvenance + from agent_comms.threads import ThreadProvenance provenance = next(item for item in fields(HistorySource) if item.name == "provenance") assert provenance.default is MISSING and provenance.default_factory is MISSING diff --git a/tests/test_historical_views.py b/tests/test_historical_views.py index b7156d66ed..b074cc2498 100644 --- a/tests/test_historical_views.py +++ b/tests/test_historical_views.py @@ -358,7 +358,7 @@ def test_historical_namespace_has_recorded_provenance_and_no_live_authority(migr from agent_comms.field_codec import FieldCodec from agent_comms.registry_provenance import RegistryProvenance - from agent_comms.thread_provenance import ThreadProvenance + from agent_comms.threads import ThreadProvenance old, _, live, source = migrated recorded = source.provenance.require("alice") diff --git a/tests/test_importing.py b/tests/test_importing.py index bd8a135dcd..90f6fe301a 100644 --- a/tests/test_importing.py +++ b/tests/test_importing.py @@ -26,7 +26,7 @@ from agent_comms.harness_install import ClaudeCodeHarness, CodexHarness, OpenCodeHarness from agent_comms.importing import ImportBuffer, ThreadImported, ImportSnapshot, ImportedSessionMetadata from agent_comms.native_entries import NativeEntry, ImportedMetadataEntry -from agent_comms.thread_provenance import ThreadProvenance +from agent_comms.threads import ThreadProvenance from agent_comms.thread_status import StoppedThreadStatus from agent_comms.runtime_updates import FinishedItem from agent_comms.transcript_events import ImportedReply, ImportedRequest, TextTranscript diff --git a/tests/test_phase_evidence.py b/tests/test_phase_evidence.py index 3b498c2c5c..9c8ac0194b 100644 --- a/tests/test_phase_evidence.py +++ b/tests/test_phase_evidence.py @@ -113,11 +113,11 @@ def test_roster_evidence_keeps_external_abi_and_each_remaining_internal_registry "class Backend:\n serves_threads = True\n", "class Backend:\n serves_new_threads: ClassVar[bool] = True\n", "class Carry:\n declared: ClassVar[dict] = {}\n", - "class ThreadRename:\n kinds = {}\n", + "class RegistryPrivacy:\n kinds = {}\n", "class BirthlessIncarnation:\n members = {}\n", "class CliToolArguments:\n types = {}\n", - "class PiTurnPhase:\n _start_phases = {}\n", - "class PiTurnPhase:\n _end_events = {}\n", + "class PiEvent:\n _start_phases = {}\n", + "class PiEvent:\n _end_events = {}\n", ): (tmp_path / "src/remaining.py").write_text(remaining) git(tmp_path, "add", ".") From 6a7e0f228f0ebe8be60366d1a616ec4cd045b8a6 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:54:52 -0400 Subject: [PATCH 13/18] Guard declaration locality and pin ordered audit term family --- src/agent_comms/skills/refactor-audit | 2 +- tests/test_declared_family.py | 36 ++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/skills/refactor-audit b/src/agent_comms/skills/refactor-audit index 8185dc8b86..4712221d08 160000 --- a/src/agent_comms/skills/refactor-audit +++ b/src/agent_comms/skills/refactor-audit @@ -1 +1 @@ -Subproject commit 8185dc8b86b2d58f83fe20d9ec25a00071459fb0 +Subproject commit 4712221d08d54f8b44ae3d4e9bf9647f76420c37 diff --git a/tests/test_declared_family.py b/tests/test_declared_family.py index 70eb304bef..50cb9b3308 100644 --- a/tests/test_declared_family.py +++ b/tests/test_declared_family.py @@ -4,7 +4,7 @@ import pytest from metaclass_registry import DuplicateRegistryKeyError -from agent_comms.declared_family import DeclaredFamily +from agent_comms.declared_family import DeclaredFamily, DeclaredOrder class SampleFamily(DeclaredFamily, affix="Case"): @@ -38,6 +38,7 @@ def test_golden_names_decode_capability_and_slots_identity(): assert SampleFamily.member_named("http_ready") is HTTPReadyCase assert SampleFamily.members_with(Capability) == (HTTPReadyCase,) assert HTTPReadyCase(3).execute() == 3 + assert HTTPReadyCase.declared_position() < CustomCase.declared_position() with pytest.raises(TypeError): AbstractCase() with pytest.raises(ValueError): @@ -131,3 +132,36 @@ class Child(HTTPReady): class Cross(HTTPReady, CustomCase): pass + + +def test_ordered_family_views_have_one_declaration_module(): + from agent_comms.acp_failure import ACPFailure + from agent_comms.native_input_record import NativeInputExecution + from agent_comms.todos import FoundProgress + from agent_comms.tools import ToolRequest + from agent_comms.turn_failure import TurnFailure + + for family in (TurnFailure, FoundProgress, ACPFailure, NativeInputExecution, ToolRequest): + assert {member.__module__ for member in family.members_with(family)} == {family.__module__} + + +def test_plain_order_requires_exactly_one_rank_and_derives_its_position(): + class Rank(DeclaredOrder): + pass + + class First(Rank): + pass + + class Second(Rank): + pass + + class Selected(Second): + pass + + class Ambiguous(First, Second): + pass + + assert Rank.position_of(First) < Rank.position_of(Selected) + for member in (object, Ambiguous): + with pytest.raises(ValueError): + Rank.position_of(member) From 4fb3582e4c2632560b5c7bb718287207946f84a9 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:57:07 -0400 Subject: [PATCH 14/18] Run the dead-turn sweep after the finished-turn carry DeadTurns decodes the registry, and the registry decoder refuses the retired last_finished_turn_id field that FinishedTurns removes. Ordered by addition date, whichever PR merged first decided whether the sweep crashed on the old field. FinishedTurns rewrites only registry.json, so declaring DeadTurns among the steps it precedes costs nothing. Co-Authored-By: Claude Opus 5.5 --- tools/cutover/carry_finished_turns.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/cutover/carry_finished_turns.py b/tools/cutover/carry_finished_turns.py index 097cbfbec6..c6ff817133 100644 --- a/tools/cutover/carry_finished_turns.py +++ b/tools/cutover/carry_finished_turns.py @@ -17,6 +17,7 @@ from agent_comms.store_files import _atomic_write_text, _store_lock from .carry_bus_history import BusHistory, CarryCatalogSource +from .carry_dead_turns import DeadTurns from .carry_owner_requests import OwnerRequests from .carry_turn_owner import TurnOwner @@ -24,7 +25,7 @@ class FinishedTurns(Carry): - precedes = (BusHistory, OwnerRequests, TurnOwner) + precedes = (BusHistory, DeadTurns, OwnerRequests, TurnOwner) """Each decodes the registry, which refuses the retired field.""" def backup_files(self) -> tuple[Path, ...]: From b21a18223557f139659758d34919f3f3ae872baf Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:57:43 -0400 Subject: [PATCH 15/18] Let notification rank mixins override through the MRO --- src/agent_comms/declared_family.py | 6 ++- src/agent_comms/notification_progress.py | 58 +++++++++++++++--------- src/agent_comms/presentation.py | 2 +- tests/test_declared_family.py | 10 ++-- tests/test_notification_read_state.py | 13 ++++++ 5 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/agent_comms/declared_family.py b/src/agent_comms/declared_family.py index 81b89284d0..afac50b5f8 100644 --- a/src/agent_comms/declared_family.py +++ b/src/agent_comms/declared_family.py @@ -94,8 +94,10 @@ class DeclaredOrder(ABC): @classmethod def position_of(cls, member: type) -> int: declarations = cls.__subclasses__() - rank, = (base for base in declarations if issubclass(member, base)) - return declarations.index(rank) + for base in member.__mro__: + if base in declarations: + return declarations.index(base) + raise ValueError(f"{member.__name__} has no {cls.__name__} rank base") class DeclaredFamily(ABC, metaclass=_FamilyMeta): diff --git a/src/agent_comms/notification_progress.py b/src/agent_comms/notification_progress.py index 52f7d9a9e4..5bfeb400a8 100644 --- a/src/agent_comms/notification_progress.py +++ b/src/agent_comms/notification_progress.py @@ -5,6 +5,7 @@ from dataclasses import dataclass, replace from typing import TYPE_CHECKING, ClassVar +from .declared_family import DeclaredOrder from .message_reference import MessageReference from .thread_presentation import ThreadPresentationStatus, WorkingThreadPresentationStatus @@ -12,13 +13,31 @@ from .presentation import MessageNotification +class DeliveryRank(DeclaredOrder): + """Notification rank is selected by the first rank base in the progress MRO.""" + + +class ActiveDelivery(DeliveryRank): + pass + + +class SettledDelivery(DeliveryRank): + pass + + +class WaitingDelivery(DeliveryRank): + pass + + class DeliveryProgress(ThreadPresentationStatus): label: ClassVar[str] = "Sent" - priority: ClassVar[int] = 3 detail: ClassVar[str] = "" delivered: ClassVar[bool] read: ClassVar[bool] + def rank(self) -> int: + return DeliveryRank.position_of(type(self)) + def notification_label(self, notification: MessageNotification) -> str: return self.label @@ -28,22 +47,20 @@ class PendingDelivery: read: ClassVar[bool] = False -class ReadDelivery: +class ReadDelivery(SettledDelivery): label: ClassVar[str] = "Read" - priority: ClassVar[int] = 2 delivered: ClassVar[bool] = True read: ClassVar[bool] = True @dataclass(frozen=True) -class UnrecordedProgress(PendingDelivery, DeliveryProgress): +class UnrecordedProgress(WaitingDelivery, PendingDelivery, DeliveryProgress): """No recipient delivery has been recorded.""" @dataclass(frozen=True) -class AcknowledgedProgress(DeliveryProgress): +class AcknowledgedProgress(SettledDelivery, DeliveryProgress): label = "Acknowledged" - priority = 2 delivered: ClassVar[bool] = True read: ClassVar[bool] = False @@ -54,7 +71,7 @@ class HumanReadProgress(ReadDelivery, DeliveryProgress): @dataclass(frozen=True) -class ContextualProgress(DeliveryProgress): +class ContextualProgress(WaitingDelivery, DeliveryProgress): """A lifecycle label surrounds the original proof without copying its facts.""" proof: DeliveryProgress @@ -72,9 +89,8 @@ class PassiveProgress(ContextualProgress): detail = "Available as context; no automatic response requested." -class IgnoredProgress(ContextualProgress): +class IgnoredProgress(SettledDelivery, ContextualProgress): label = ReadDelivery.label - priority = ReadDelivery.priority def notification_label(self, notification): return self.proof.notification_label(notification) @@ -88,13 +104,12 @@ class FailedProgress(ContextualProgress): label = "Failed" -class EngagedProgress(ContextualProgress, WorkingThreadPresentationStatus): - priority = 1 +class EngagedProgress(ActiveDelivery, ContextualProgress, WorkingThreadPresentationStatus): + pass -class AnsweredProgress(ContextualProgress): +class AnsweredProgress(SettledDelivery, ContextualProgress): label = "Answered" - priority = 2 def notification_label(self, notification): return notification.timed_state(self.label, notification.answered_at_ms) @@ -130,7 +145,7 @@ def answered_notification(self, notification: MessageNotification, @dataclass(frozen=True) -class SentProgress(PendingDelivery, AssignmentProgress): +class SentProgress(WaitingDelivery, PendingDelivery, AssignmentProgress): sent_at_ms: int @property @@ -147,14 +162,13 @@ def read_notification(self, notification): raise ValueError("Read notification has no model-context proof") -class SendingProgress(SentProgress, WorkingThreadPresentationStatus): - priority = 1 +class SendingProgress(ActiveDelivery, SentProgress, WorkingThreadPresentationStatus): + pass @dataclass(frozen=True) -class QueuedProgress(SentProgress): +class QueuedProgress(ActiveDelivery, SentProgress): label = "Queued" - priority = 1 @dataclass(frozen=True) @@ -176,13 +190,13 @@ def read_notification(self, notification): return replace(notification, state=self) -class SelectedReadProgress(ShownToModelProgress): - priority = 1 +class SelectedReadProgress(ActiveDelivery, ShownToModelProgress): + pass class ReadingProgress(SelectedReadProgress, WorkingThreadPresentationStatus): pass -class InterruptedReadProgress(ShownToModelProgress): - priority = 3 +class InterruptedReadProgress(WaitingDelivery, ShownToModelProgress): + pass diff --git a/src/agent_comms/presentation.py b/src/agent_comms/presentation.py index 259db557c1..c270dea720 100644 --- a/src/agent_comms/presentation.py +++ b/src/agent_comms/presentation.py @@ -177,7 +177,7 @@ def sent_outcome(self) -> SentMessageOutcome | None: @classmethod def ordered(cls, notifications: Iterable[MessageNotification]) -> tuple[MessageNotification, ...]: - return tuple(sorted(notifications, key=lambda item: (item.state.priority, item.recipient.casefold()))) + return tuple(sorted(notifications, key=lambda item: (item.state.rank(), item.recipient.casefold()))) @classmethod def summary(cls, notifications: Iterable[MessageNotification]) -> str: diff --git a/tests/test_declared_family.py b/tests/test_declared_family.py index 50cb9b3308..5053a2121e 100644 --- a/tests/test_declared_family.py +++ b/tests/test_declared_family.py @@ -145,7 +145,7 @@ def test_ordered_family_views_have_one_declaration_module(): assert {member.__module__ for member in family.members_with(family)} == {family.__module__} -def test_plain_order_requires_exactly_one_rank_and_derives_its_position(): +def test_plain_order_uses_the_first_rank_in_the_mro_and_requires_a_rank(): class Rank(DeclaredOrder): pass @@ -158,10 +158,10 @@ class Second(Rank): class Selected(Second): pass - class Ambiguous(First, Second): + class Override(First, Selected): pass assert Rank.position_of(First) < Rank.position_of(Selected) - for member in (object, Ambiguous): - with pytest.raises(ValueError): - Rank.position_of(member) + assert Rank.position_of(Override) == Rank.position_of(First) + with pytest.raises(ValueError): + Rank.position_of(object) diff --git a/tests/test_notification_read_state.py b/tests/test_notification_read_state.py index 08565f5e9e..22ae99a7c7 100644 --- a/tests/test_notification_read_state.py +++ b/tests/test_notification_read_state.py @@ -31,3 +31,16 @@ def test_first_revision_requires_reads_even_for_unchanged_empty_stores(): assert not known.changed_since(StoresView(), known) assert not known.changed_since(RegistrationsView(), known) assert not known.changed_since(SidebarView(), known) + + +def test_notification_rank_overrides_follow_the_mro_without_changing_parent_ranks(): + from agent_comms.notification_progress import ( + ActiveDelivery, DeliveryRank, SelectedReadProgress, SendingProgress, + SentProgress, SettledDelivery, ShownToModelProgress, WaitingDelivery, + ) + + assert {rank.__module__ for rank in DeliveryRank.__subclasses__()} == {DeliveryRank.__module__} + assert SentProgress(100).rank() == DeliveryRank.position_of(WaitingDelivery) + assert ShownToModelProgress(100).rank() == DeliveryRank.position_of(SettledDelivery) + assert SendingProgress(100).rank() == DeliveryRank.position_of(ActiveDelivery) + assert SelectedReadProgress(100).rank() == DeliveryRank.position_of(ActiveDelivery) From 0dcf1f3df02f746a81cead82347cc965d3006179 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:57:01 -0400 Subject: [PATCH 16/18] Rebuild bus projections from one joined history cursor BusProjectionTables.create_schema replayed history with one header stream plus one point SELECT per capability table per record (read_from), each through the strict decode boundary. On a copy of the live bus (19,448 records) that replay took 46.5 s; the joined cursor takes 37.7 s with identical projection rows. The reads were about 9 s of it; the rest is project_into writing about 162k projection rows, which this change does not touch. BusCapability.history reads every record in sequence order from one cursor: the history header LEFT JOINed to each capability table (members derived from TypedTable.members_with(CapabilityRow)), each stored row decoded once at its column offset. read_from and history share one assembly step (BusCapability.assembled). The per-row replay loop in create_schema is deleted; the projection stamp is unchanged. TypedTable gains qualified() and decode_at() so a select joining tables that share column names decodes each table's slice by declaration order. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/bus_tables.py | 42 ++++++++++++++++++++++++++-------- src/agent_comms/typed_table.py | 12 ++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/agent_comms/bus_tables.py b/src/agent_comms/bus_tables.py index e71f1caac1..431ee53885 100644 --- a/src/agent_comms/bus_tables.py +++ b/src/agent_comms/bus_tables.py @@ -3,11 +3,12 @@ from __future__ import annotations import sqlite3 -from collections.abc import Iterator +from collections.abc import Iterable, Iterator from contextlib import closing from dataclasses import Field, dataclass, fields, make_dataclass from functools import cache from inspect import get_annotations +from itertools import accumulate from typing import TYPE_CHECKING, Annotated, ClassVar, Self, get_origin, get_type_hints from agent_comms import messages @@ -54,9 +55,8 @@ class BusProjectionTables(ProducedRow, SharedFileTables, DerivedTables): def create_schema(cls, db: sqlite3.Connection) -> None: """Create/rebuild rows from history using the append projection owner.""" super().create_schema(db) - with closing(BusRow.stream(db, order_by=("seq",))) as headers: - for header in headers: - header.kind.read_from(db, header.seq).project_into(db) + for record in BusCapability.history(db): + record.project_into(db) class BusSchema(ComposedSchemas, BusTables, ProjectionProducerTables, BusProjectionTables): @@ -438,13 +438,37 @@ def rows(self) -> tuple[CapabilityRow, ...]: @classmethod def read_from(cls, db: sqlite3.Connection, seq: int) -> Self: """Reconstruct this declared value from its own capability tables.""" - values = {} + rows = [] for table in cls.tables(): - rows = table.select(db, seq=seq) - if not rows: + stored = table.select(db, seq=seq) + if not stored: raise RelationViolationError(f"Bus row {seq} lacks {table.declared_name}") - values.update({item.name: getattr(rows[0], item.name) for item in fields(table)}) - return FieldCodec.construct(cls, values) + rows.append(stored[0]) + return cls.assembled(rows) + + @classmethod + def assembled(cls, rows: Iterable[CapabilityRow]) -> Self: + """This declared value from one stored row of each of its capability tables.""" + return FieldCodec.construct(cls, { + item.name: getattr(row, item.name) for row in rows for item in fields(row)}) + + @staticmethod + def history(db: sqlite3.Connection) -> Iterator[BusCapability]: + """Every record in sequence order from one joined cursor, each stored row decoded once.""" + tables = TypedTable.members_with(CapabilityRow) + offsets = dict(zip(tables, accumulate( + (len(BusRow.columns()), *(len(table.columns()) for table in tables))))) + header = BusRow.qualified("seq") + with closing(db.execute( + f"SELECT {', '.join(row.qualified(*row.column_projection()) for row in (BusRow, *tables))} " + f'FROM "{BusRow.declared_name}" ' + + " ".join(f'LEFT JOIN "{table.declared_name}" ON {table.qualified("seq")} = {header}' + for table in tables) + + f" ORDER BY {header}" + )) as cursor: + for stored in cursor: + kind = BusRow.decode_at(stored, 0).kind + yield kind.assembled(table.decode_at(stored, offsets[table]) for table in kind.tables()) def projections(self) -> Iterator[TypedTable]: return iter(()) diff --git a/src/agent_comms/typed_table.py b/src/agent_comms/typed_table.py index b08232fbf5..10454db5f1 100644 --- a/src/agent_comms/typed_table.py +++ b/src/agent_comms/typed_table.py @@ -1753,6 +1753,18 @@ def stream( """Rows decoded as the caller consumes them; close it to release the query.""" return cls._query(db, where, parameters, order_by, descending, key, row_type, limit) + @classmethod + def qualified(cls, *names: str) -> str: + """Columns named through this table, for one select joining tables that share names.""" + table = _identifier(cls.declared_name) + return ", ".join(f"{table}.{_identifier(item.wire_name)}" for item in cls._fields_named(names)) + + @classmethod + def decode_at(cls, row: tuple[object, ...], offset: int) -> Self: + """Decode this table's columns, selected by ``qualified`` in declaration order at ``offset``.""" + return cls._decode_row(row, {name: offset + index + for index, name in enumerate(cls.column_projection())}) + @classmethod def one(cls, db: sqlite3.Connection, **key: object) -> Self | None: rows = cls._keyed(db, key) From 39df5ad83503326a0188e867a569da0e35a18275 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 03:57:01 -0400 Subject: [PATCH 17/18] Rebuild bus projections from one joined history cursor BusProjectionTables.create_schema replayed history with one header stream plus one point SELECT per capability table per record (read_from), each through the strict decode boundary. On a copy of the live bus (19,448 records) that replay took 46.5 s; the joined cursor takes 37.7 s with identical projection rows. Writes are not the remaining cost: a profile puts all projection inserts at about 6 s. The rest is decoding records and replaying claim transitions. BusCapability.history reads every record in sequence order from one cursor: the history header LEFT JOINed to each capability table (members derived from TypedTable.members_with(CapabilityRow)), each stored row decoded once at its column offset. read_from and history share one assembly step (BusCapability.assembled). The per-row replay loop in create_schema is deleted; the projection stamp is unchanged. TypedTable gains qualified() and decode_at() so a select joining tables that share column names decodes each table's slice by declaration order. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/bus_tables.py | 42 ++++++++++++++++++++++++++-------- src/agent_comms/typed_table.py | 12 ++++++++++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/agent_comms/bus_tables.py b/src/agent_comms/bus_tables.py index e71f1caac1..431ee53885 100644 --- a/src/agent_comms/bus_tables.py +++ b/src/agent_comms/bus_tables.py @@ -3,11 +3,12 @@ from __future__ import annotations import sqlite3 -from collections.abc import Iterator +from collections.abc import Iterable, Iterator from contextlib import closing from dataclasses import Field, dataclass, fields, make_dataclass from functools import cache from inspect import get_annotations +from itertools import accumulate from typing import TYPE_CHECKING, Annotated, ClassVar, Self, get_origin, get_type_hints from agent_comms import messages @@ -54,9 +55,8 @@ class BusProjectionTables(ProducedRow, SharedFileTables, DerivedTables): def create_schema(cls, db: sqlite3.Connection) -> None: """Create/rebuild rows from history using the append projection owner.""" super().create_schema(db) - with closing(BusRow.stream(db, order_by=("seq",))) as headers: - for header in headers: - header.kind.read_from(db, header.seq).project_into(db) + for record in BusCapability.history(db): + record.project_into(db) class BusSchema(ComposedSchemas, BusTables, ProjectionProducerTables, BusProjectionTables): @@ -438,13 +438,37 @@ def rows(self) -> tuple[CapabilityRow, ...]: @classmethod def read_from(cls, db: sqlite3.Connection, seq: int) -> Self: """Reconstruct this declared value from its own capability tables.""" - values = {} + rows = [] for table in cls.tables(): - rows = table.select(db, seq=seq) - if not rows: + stored = table.select(db, seq=seq) + if not stored: raise RelationViolationError(f"Bus row {seq} lacks {table.declared_name}") - values.update({item.name: getattr(rows[0], item.name) for item in fields(table)}) - return FieldCodec.construct(cls, values) + rows.append(stored[0]) + return cls.assembled(rows) + + @classmethod + def assembled(cls, rows: Iterable[CapabilityRow]) -> Self: + """This declared value from one stored row of each of its capability tables.""" + return FieldCodec.construct(cls, { + item.name: getattr(row, item.name) for row in rows for item in fields(row)}) + + @staticmethod + def history(db: sqlite3.Connection) -> Iterator[BusCapability]: + """Every record in sequence order from one joined cursor, each stored row decoded once.""" + tables = TypedTable.members_with(CapabilityRow) + offsets = dict(zip(tables, accumulate( + (len(BusRow.columns()), *(len(table.columns()) for table in tables))))) + header = BusRow.qualified("seq") + with closing(db.execute( + f"SELECT {', '.join(row.qualified(*row.column_projection()) for row in (BusRow, *tables))} " + f'FROM "{BusRow.declared_name}" ' + + " ".join(f'LEFT JOIN "{table.declared_name}" ON {table.qualified("seq")} = {header}' + for table in tables) + + f" ORDER BY {header}" + )) as cursor: + for stored in cursor: + kind = BusRow.decode_at(stored, 0).kind + yield kind.assembled(table.decode_at(stored, offsets[table]) for table in kind.tables()) def projections(self) -> Iterator[TypedTable]: return iter(()) diff --git a/src/agent_comms/typed_table.py b/src/agent_comms/typed_table.py index b08232fbf5..10454db5f1 100644 --- a/src/agent_comms/typed_table.py +++ b/src/agent_comms/typed_table.py @@ -1753,6 +1753,18 @@ def stream( """Rows decoded as the caller consumes them; close it to release the query.""" return cls._query(db, where, parameters, order_by, descending, key, row_type, limit) + @classmethod + def qualified(cls, *names: str) -> str: + """Columns named through this table, for one select joining tables that share names.""" + table = _identifier(cls.declared_name) + return ", ".join(f"{table}.{_identifier(item.wire_name)}" for item in cls._fields_named(names)) + + @classmethod + def decode_at(cls, row: tuple[object, ...], offset: int) -> Self: + """Decode this table's columns, selected by ``qualified`` in declaration order at ``offset``.""" + return cls._decode_row(row, {name: offset + index + for index, name in enumerate(cls.column_projection())}) + @classmethod def one(cls, db: sqlite3.Connection, **key: object) -> Self | None: rows = cls._keyed(db, key) From ddda8419311652c6f61d192b8d7e3bc0f53a9396 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 23 Oct 2026 04:13:10 -0400 Subject: [PATCH 18/18] Carry claim holders through the projection replay Every replayed claim called HeldResources.claims_for, which reselected and fully decoded the current holders' Claimed rows that the same in-order pass had already decoded. HeldResources.project and apply now take a ClaimHolders member: StoredHolders reads the holders of one live append from their table (the read a separate append transaction needs); ReplayedHolders, owned by create_schema's pass, carries the owners it already projected and never rereads them. apply stays the one owner of which resources leave and hands that set to the holders. The Claimed.replay_claim split is folded into project_into. On a copy of the live bus (19,457 records), the same joined-cursor pass with stored holders took 41.5 s and with carried holders 34.5 s; every projection table's contents are identical. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/bus_tables.py | 57 ++++++++++++++++++++++++++++--- src/agent_comms/messages.py | 22 +++++++----- tests/test_bus_resource_claims.py | 12 +++++-- 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/src/agent_comms/bus_tables.py b/src/agent_comms/bus_tables.py index 431ee53885..29dce0497c 100644 --- a/src/agent_comms/bus_tables.py +++ b/src/agent_comms/bus_tables.py @@ -4,8 +4,9 @@ import sqlite3 from collections.abc import Iterable, Iterator +from abc import ABC, abstractmethod from contextlib import closing -from dataclasses import Field, dataclass, fields, make_dataclass +from dataclasses import Field, dataclass, field, fields, make_dataclass from functools import cache from inspect import get_annotations from itertools import accumulate @@ -14,6 +15,7 @@ from agent_comms import messages from agent_comms.coordination_errors import IdentityConflict from agent_comms.envelope_claim_transitions import ( + ClaimOwner, ClaimProjection, ClaimTransition, ClaimTransitionError, @@ -55,8 +57,9 @@ class BusProjectionTables(ProducedRow, SharedFileTables, DerivedTables): def create_schema(cls, db: sqlite3.Connection) -> None: """Create/rebuild rows from history using the append projection owner.""" super().create_schema(db) + holders = ReplayedHolders() for record in BusCapability.history(db): - record.project_into(db) + record.project_into(db, holders) class BusSchema(ComposedSchemas, BusTables, ProjectionProducerTables, BusProjectionTables): @@ -357,17 +360,20 @@ def _project( }) @classmethod - def project(cls, db: sqlite3.Connection, transition: ClaimTransition) -> ClaimProjection: + def project( + cls, db: sqlite3.Connection, transition: ClaimTransition, holders: ClaimHolders, + ) -> ClaimProjection: """Validate before the original is inserted; the caller retains the writer.""" try: transition.require_new_operation(lambda operation: ClaimOperations.used(db, operation)) - return transition.project(cls.claims_for(db, transition.resources, transition.seq - 1)) + return transition.project(holders.before(db, transition)) except (ClaimTransitionError, IdentityConflict) as error: raise RelationViolationError("Committed claim transition is inconsistent.") from error @classmethod def apply( cls, db: sqlite3.Connection, transition: ClaimTransition, projected: ClaimProjection, + holders: ClaimHolders, ) -> None: removed = transition.resources + transition.expired marks = ",".join("?" for _ in removed) @@ -376,6 +382,47 @@ def apply( cls(owner.resource, owner.seq).insert(db) for operation_id in transition.operation_ids: ClaimOperations(operation_id, transition.seq).insert(db) + holders.advance(removed, projected) + + +class ClaimHolders(ABC): + """The current holders a claim transition projects from, before it is applied.""" + + @abstractmethod + def before(self, db: sqlite3.Connection, transition: ClaimTransition) -> ClaimProjection: ... + + def advance(self, removed: tuple[str, ...], projected: ClaimProjection) -> None: + """Held rows already record the applied transition.""" + + +class StoredHolders(ClaimHolders): + """One append reads the holders of its resources from their table.""" + + def before(self, db: sqlite3.Connection, transition: ClaimTransition) -> ClaimProjection: + return HeldResources.claims_for(db, transition.resources, transition.seq - 1) + + +@dataclass +class ReplayedHolders(ClaimHolders): + """An in-order rebuild carries the holders it already projected; it never rereads them. + + Each owner was validated in the projection that produced it; only the subset a + transition projects from is validated again, as one ``ClaimProjection``. + """ + + last_seq: int = 0 + owners: dict[str, ClaimOwner] = field(default_factory=dict) + + def before(self, db: sqlite3.Connection, transition: ClaimTransition) -> ClaimProjection: + return ClaimProjection(self.last_seq, { + resource: self.owners[resource] + for resource in transition.resources if resource in self.owners}) + + def advance(self, removed: tuple[str, ...], projected: ClaimProjection) -> None: + for resource in removed: + self.owners.pop(resource, None) + self.owners.update(projected) + self.last_seq = projected.last_seq @dataclass(frozen=True) @@ -473,7 +520,7 @@ def history(db: sqlite3.Connection) -> Iterator[BusCapability]: def projections(self) -> Iterator[TypedTable]: return iter(()) - def project_into(self, db: sqlite3.Connection) -> None: + def project_into(self, db: sqlite3.Connection, holders: ClaimHolders) -> None: for row in self.projections(): row.insert(db) diff --git a/src/agent_comms/messages.py b/src/agent_comms/messages.py index b7722c205d..9aff478d3a 100644 --- a/src/agent_comms/messages.py +++ b/src/agent_comms/messages.py @@ -14,7 +14,14 @@ from pathlib import Path from typing import TYPE_CHECKING, Annotated, ClassVar, Self, Type, cast, dataclass_transform -from agent_comms.bus_tables import Addressed, BusCapability, BusRow, HeldResources +from agent_comms.bus_tables import ( + Addressed, + BusCapability, + BusRow, + ClaimHolders, + HeldResources, + StoredHolders, +) from agent_comms.response_policy import InformationalPolicy, ResponseEligibility, ResponsePolicy from agent_comms.typed_table import Column @@ -576,7 +583,7 @@ def append_to(self, db: sqlite3.Connection, seq: int) -> int: stored = replace(self, seq=seq) for row in stored.rows(): row.insert(db) - stored.project_into(db) + stored.project_into(db, StoredHolders()) return stored.seq @property @@ -614,14 +621,11 @@ class Claimed(BusCapability): def require_claim_transition(self) -> ClaimTransition: return self.claim_transition - def replay_claim(self, db: sqlite3.Connection) -> None: + def project_into(self, db: sqlite3.Connection, holders: ClaimHolders) -> None: """Append and the shared derived rebuild replay this same original transition.""" - projected = HeldResources.project(db, self.claim_transition) - HeldResources.apply(db, self.claim_transition, projected) - - def project_into(self, db: sqlite3.Connection) -> None: - super().project_into(db) - self.replay_claim(db) + super().project_into(db, holders) + projected = HeldResources.project(db, self.claim_transition, holders) + HeldResources.apply(db, self.claim_transition, projected, holders) @dataclass(frozen=True, kw_only=True) diff --git a/tests/test_bus_resource_claims.py b/tests/test_bus_resource_claims.py index 0d4c13bf10..17589592ee 100644 --- a/tests/test_bus_resource_claims.py +++ b/tests/test_bus_resource_claims.py @@ -11,7 +11,14 @@ from agent_comms.bus import Bus, ClaimRecord, KeyedPrivateRecord, PublicRecord, RecordBuilder from agent_comms.bus_publication import HumanOrigin -from agent_comms.bus_tables import BusProjectionTables, BusRow, BusTables, ClaimOperations, HeldResources +from agent_comms.bus_tables import ( + BusProjectionTables, + BusRow, + BusTables, + ClaimOperations, + HeldResources, + ReplayedHolders, +) from agent_comms.child_process import NoProcess, ProcessIdentity from agent_comms.coordination_errors import IdentityConflict from agent_comms.envelope_claim_transitions import ( @@ -184,8 +191,9 @@ def test_expiry_and_rebuild_use_claimed_replay_not_a_checkpoint(claims: ClaimBus with claims.bus.writing() as db: HeldResources.delete(db) ClaimOperations.delete(db) + holders = ReplayedHolders() for original in Claimed.table.select(db, order_by=("seq",)): - Claimed(claim_transition=original.claim_transition).replay_claim(db) + Claimed(claim_transition=original.claim_transition).project_into(db, holders) assert claims.bus.claim_projection() == before assert claims.counts() == counts