From d4256a29a91187b39c0e2124eb4f536d4d3919b1 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 13:32:12 -0400 Subject: [PATCH 01/26] Keep imported-thread acknowledgement in its acquired wire lifetime --- docs/toad-list-investigation.md | 86 +++++++++++++++++++++++++ src/agent_comms/message_bus.py | 36 ++++++++--- src/agent_comms/thread_management.py | 2 +- tools/reproduce_thread_list.py | 93 ++++++++++++++++++++++++++++ tools/reproduce_toad_import_blank.py | 85 +++++++++++++++++++++++++ 5 files changed, 293 insertions(+), 9 deletions(-) create mode 100644 docs/toad-list-investigation.md create mode 100644 tools/reproduce_thread_list.py create mode 100644 tools/reproduce_toad_import_blank.py diff --git a/docs/toad-list-investigation.md b/docs/toad-list-investigation.md new file mode 100644 index 0000000000..1efafd490f --- /dev/null +++ b/docs/toad-list-investigation.md @@ -0,0 +1,86 @@ +# Toad blank thread list during import + +Owner: pi-toad-list. Scope: importer acknowledgement and the MessageBus snapshot lifetime. + +## Actual source + +Core `ui_model.observation.SidebarReading` reads an initial roster and then +`HistoryViews.viewer_snapshot`. Toad `comms_root.CoordinationAccess.sidebar_observed` +applies that result to Core's `SidebarModel`; Toad's `SidebarModelView` and +`CommsSidebar` render its keyed rows. Native unread preparation is downstream of +that same captured registry, not an independent thread roster. Delivery cursors +filter messages, not registry membership. + +`RegistryStore.read_revision_unlocked` reads under its registry shared lock; +`PublicRegistry.write` and `PrivateRegistryGuard.write` publish with +`store_files._atomic_write_text`, using `os.replace` without unlinking the old +registry first. A genuinely missing registry creates the empty initial document; +malformed content raises. `RegistrySnapshot.capture` copies the current maps. +Visibility can intentionally omit stopped/archived/non-executor threads. + +A stored read failure becomes `SidebarUnreadable`, not `SidebarObserved` with an +empty roster. Toad clears the failed model and calls `show_error` with the original +cause. Deferred reads publish nothing. Thus a failed sidebar can look blank after +its notification; there is no evidence that this path caused the reported event. +No failed read is changed to a successful empty result by this patch. + +## Reproduction on main + +Installed Core main at 8b27e02aa and assigned branch base 0280f852e have the same +relevant implementation. Independent registration/sidebar loops on main: + +- public bus: 20.016 s, 1,072 observations, 3,808 writes, zero empty/error results; +- private bus: 20.021 s, 1,113 observations, 3,419 writes, zero empty/error results. + +Actual import, unlike registration alone, deadlocks. `ThreadManagement.import_thread` +holds root/wire exclusively at line 187 and calls `mark_delivered_through` at line +202. `MessageBus.snapshot` at line 102 opens another descriptor and asks for the +same wire lock shared. The importer blocks itself. The observer waits for wire in +`Messaging.user_identity`, called by `HistoryViews.viewer_snapshot`. Both original +process stacks are retained in the owner's small `import-stack.log`. + +`tools/reproduce_toad_import_blank.py` starts real private Core, 85 stopped threads, +a real CLI import, Xvfb and plain st running the installed Toad ACP entrypoint. +It sends no model input. At 15.000 s the importer was still running, the registry +contained 87 entries (85 agents, user, newly imported agent), and Toad's actual +screen showed Channels / New Session with no channel or thread rows. Terminating +only that scratch importer released the wire; at 25.209 s the same Toad showed +`#any 0/86`, `imported`, and sentinel rows without a restart. Original physical +screens are in the owner's scratch `ui-main-acp` directory. This demonstrates an +unanswered observation, not an observation that returned an empty registry. + +## Ownership change + +MessageBus retains one implementation of coherent registry/catalog/read/source +capture and one delivery-acknowledgement implementation. Normal readers acquire +wire for capture and release it before consuming the opened cut, as before. The +importing writer uses that same capture under the wire lock it already owns. +The import's declaration, acknowledgement and rollback remain within its original +exclusive lifetime; no newly published thread is exposed to a start between +registration and acknowledgement. No retry, retained nonempty roster or sleep is +added to production. + +AST evidence used the existing refactor-audit `Package.load` on all 379 Core +production modules, zero parse omissions. Original declarations, wire acquisition +lifetimes and every mark_delivered_through caller are retained in the owner's +`source-before.txt`. Toad's installed observer/model/widget consumers were read +semantically. Generic dynamically bound snapshot calls are not claimed resolved. +NRA's latest ModuleSyntaxIndex could not load with the stack's newer +metaclass-registry; its existing 3.12 environment cannot parse default type +parameters. The working refactor-audit parser uses the project's Python 3.14. + +## Evidence custody + +Small disposable scratch: `/home/ts/.cache/agent-scratch/pi-toad-list`, owned only +by pi-toad-list, for scratch buses, physical captures and short command results. +No saved user sessions, wire or uncertain inputs were moved, replayed or removed. +Physical captures for failed setup attempts are disposable; retain only the +accepted main/branch comparison and concise logs until review. + +## Assumed + +- The historical 13:02 event used the installed importer/snapshot code whose + deadlock is reproduced here. No historical observer stack was recorded. +- Legitimate stopped/archived filters and a truly empty new bus may show no rows. +- The Xvfb comparison verifies the installed Toad and real Core source path; + the production installation is not changed by this work. diff --git a/src/agent_comms/message_bus.py b/src/agent_comms/message_bus.py index c95885f0c6..c20c9a4113 100644 --- a/src/agent_comms/message_bus.py +++ b/src/agent_comms/message_bus.py @@ -6,7 +6,7 @@ import inspect from collections.abc import AsyncIterator, Awaitable, Callable, Iterator, Mapping, Sequence -from contextlib import ExitStack, aclosing, contextmanager +from contextlib import AbstractContextManager, ExitStack, aclosing, contextmanager from pathlib import Path from typing import TYPE_CHECKING @@ -100,12 +100,20 @@ def snapshot( """ with ExitStack() as stack: with self.locked(mode=SharedLock()): - source = stack.enter_context(self.log.opened_wire_snapshot()) - with CapturedSelection.reading( - self._registry, self._channels, self.reads, viewer, - self.activity_index.snapshot(source), - ) as basis: - pass + captured = stack.enter_context(self._snapshot_unlocked(viewer)) + yield captured + + @contextmanager + def _snapshot_unlocked( + self, viewer: DisplayViewer, + ) -> Iterator[tuple[CapturedSelection, WireSnapshot]]: + """Capture under the caller's wire lock; retain the opened source after capture.""" + with self.log.opened_wire_snapshot() as source: + with CapturedSelection.reading( + self._registry, self._channels, self.reads, viewer, + self.activity_index.snapshot(source), + ) as basis: + pass yield basis, source def inbox( @@ -287,9 +295,21 @@ def mark_delivered_through( self, name: str, sequence: int, *, target: CommandTarget = ANY_CHANNEL, ) -> None: """Mark the selected inbox through one captured cut, without loading its messages.""" + self._mark_delivered_snapshot(self.snapshot(NamedViewer(name)), sequence, target) + + def mark_delivered_through_unlocked( + self, name: str, sequence: int, *, target: CommandTarget = ANY_CHANNEL, + ) -> None: + """The importing writer already holds wire; never acquire it on another descriptor.""" + self._mark_delivered_snapshot(self._snapshot_unlocked(NamedViewer(name)), sequence, target) + + def _mark_delivered_snapshot( + self, captured: AbstractContextManager[tuple[CapturedSelection, WireSnapshot]], + sequence: int, target: CommandTarget, + ) -> None: if sequence < 0: raise ValueError("Delivery sequence cannot be negative.") - with self.snapshot(NamedViewer(name)) as (basis, source): + with captured as (basis, source): canonical = basis.require_viewer() delivery = DeliveryScope.capture(canonical, basis.registry, basis.catalog) scope = delivery.selection(target, basis.registry, diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index 4cd678afa5..ee4ca0b13e 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -199,7 +199,7 @@ def import_thread( _atomic_write_text(session_path, snapshot.pi_session(project)) try: self.registry._declare_unlocked(thread, StoppedThreadStatus()) - self.bus.mark_delivered_through(name, self.bus.log.latest_sequence()) + self.bus.mark_delivered_through_unlocked(name, self.bus.log.latest_sequence()) except (OSError, ValueError, CoordinationError): if name in self.registry: self.registry.remove(name) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py new file mode 100644 index 0000000000..a255727944 --- /dev/null +++ b/tools/reproduce_thread_list.py @@ -0,0 +1,93 @@ +"""Real scratch-bus sidebar observation concurrent with registry publication.""" +import argparse +import json +import faulthandler +import multiprocessing as mp +from dataclasses import replace +import time +from pathlib import Path + +from agent_comms.comms import Comms +from agent_comms.storage_rules import StorageRoots +from agent_comms.threads import Thread +from agent_comms.thread_status import StoppedThreadStatus +from agent_comms.ui_model.observation import ObserveSidebar, SidebarObserved, SidebarUnreadable + + +def writer(root, stop, ready, result, imports, dump_after): + if dump_after: + faulthandler.dump_traceback_later(dump_after) + comms = Comms(root) + ready.set() + count = 0 + while not stop.is_set(): + name = f'writer-{count % 85}' + if imports and count < 85: + from agent_comms.cli_bridges.claude import ClaudeTranscript + comms.threads.import_thread(ClaudeTranscript(root / 'source.jsonl'), name=name, + worktree=str(root), tags=frozenset({'proof'})) + elif imports: + comms.registry.register(replace(comms.registry.require(name), title=f'write {count}'), + StoppedThreadStatus()) + else: + comms.registry.register(Thread(name, frozenset({'proof'}), str(root), + created_at=float(count % 85 + 1), title=f'write {count}', + storage_roots=StorageRoots(root / 'scratch' / name)), StoppedThreadStatus()) + count += 1 + result.put(count) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('root', type=Path) + parser.add_argument('--seconds', type=float, default=120) + parser.add_argument('--imports', action='store_true') + parser.add_argument('--dump-after', type=float, default=0) + args = parser.parse_args() + if args.dump_after: + faulthandler.dump_traceback_later(args.dump_after) + root = args.root.resolve() + root.mkdir(mode=0o700, parents=True, exist_ok=True) + comms = Comms(root) + comms.messaging.initialize_private_initial_protocol() + source = root / 'source.jsonl' + source.write_text('\n'.join(json.dumps({ + 'type': kind, 'uuid': str(index), 'timestamp': '2026-10-19T00:00:00Z', + 'cwd': str(root), 'sessionId': 'proof', 'promptId': 'p-1', 'origin': {'kind': 'human'}, + 'message': {'role': kind, 'content': text if kind == 'user' else [{'type': 'text', 'text': text}]}, + }) for index, (kind, text) in enumerate((('user', 'Scratch source input'), + ('assistant', 'Scratch source reply')))) + '\n') + comms.registry.register(Thread('sentinel', frozenset({'proof'}), str(root), + storage_roots=StorageRoots(root / 'scratch' / 'sentinel')), StoppedThreadStatus()) + request = ObserveSidebar(str(root), True, False) + reading = request.observation() + stop, ready, counts = mp.Event(), mp.Event(), mp.Queue() + process = mp.Process(target=writer, args=(root, stop, ready, counts, args.imports, args.dump_after)) + process.start() + ready.wait(10) + start = time.monotonic() + reads = empty = errors = 0 + try: + while time.monotonic() - start < args.seconds: + for result in reading.read(comms, comms.views.revision()): + if isinstance(result, SidebarObserved): + reads += 1 + if not result.derived[1]: + empty += 1 + print(f'EMPTY after {reads} reads: registry={len(comms.registry.snapshot().threads)}', flush=True) + elif isinstance(result, SidebarUnreadable): + errors += 1 + print(f'UNREADABLE after {reads} reads: {result.error}', flush=True) + if not process.is_alive(): + raise RuntimeError(f'writer exited: {process.exitcode}') + finally: + stop.set() + process.join(15) + comms.views.transcript_reads.close() + print(f'elapsed={time.monotonic()-start:.3f}s reads={reads} empty={empty} errors={errors} writes={counts.get(timeout=2)} writer_exit={process.exitcode} imports={args.imports}', flush=True) + if not reads or empty or errors or process.exitcode != 0: + raise RuntimeError('Sidebar observation failed; see the original results above') + + +if __name__ == '__main__': + main() diff --git a/tools/reproduce_toad_import_blank.py b/tools/reproduce_toad_import_blank.py new file mode 100644 index 0000000000..e446ffdb64 --- /dev/null +++ b/tools/reproduce_toad_import_blank.py @@ -0,0 +1,85 @@ +"""Physical Toad on an isolated X display while a real import holds the wire.""" +import argparse +import json +import os +from pathlib import Path +import signal +import subprocess +import sys +import time +from threading import Event + +from agent_comms.comms import Comms +from agent_comms.thread_status import StoppedThreadStatus + + +def capture(display, destination): + subprocess.run(['import', '-display', display, '-window', 'root', str(destination)], check=True) + print(f'physical screen: {destination}', flush=True) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('directory', type=Path) + args = parser.parse_args() + directory = args.directory.resolve() + directory.mkdir(parents=True, exist_ok=True) + root = directory / 'wire' + comms = Comms(root) + comms.messaging.initialize_private_initial_protocol() + for index in range(85): + comms.registry.register(comms.registry.new_thread(f'sentinel-{index:02}', frozenset({'proof'}), str(directory)), + StoppedThreadStatus()) + comms.messaging.user_identity(str(directory)) + source = directory / 'source.jsonl' + source.write_text('\n'.join(json.dumps({ + 'type': kind, 'uuid': str(index), 'timestamp': '2026-10-19T00:00:00Z', + 'cwd': str(directory), 'sessionId': 'proof', 'promptId': 'p-1', 'origin': {'kind': 'human'}, + 'message': {'content': text if kind == 'user' else [{'type': 'text', 'text': text}]}, + }) for index, (kind, text) in enumerate((('user', 'Scratch input'), ('assistant', 'Scratch reply')))) + '\n') + environment = {key: value for key, value in os.environ.items() if not key.startswith('PI_') + and key not in ('AGENT_COMMS_MANAGED', 'AGENT_COMMS_THREAD')} + environment.update(AGENT_COMMS_ROOT=str(root), XDG_STATE_HOME=str(directory / 'state'), + XDG_CONFIG_HOME=str(directory / 'config')) + processes = [] + try: + x = subprocess.Popen(['Xvfb', '-displayfd', '1', '-screen', '0', '1500x1000x24', '-nolisten', 'tcp'], + stdout=subprocess.PIPE, stderr=directory.joinpath('xvfb.log').open('w'), text=True) + processes.append(x) + display = ':' + x.stdout.readline().strip() + environment['DISPLAY'] = display + importer = subprocess.Popen([sys.executable, str(Path(sys.executable).with_name('agent-comms')), '--root', str(root), + 'import-thread', '--harness', 'claude_code', '--source', str(source), + '--name', 'imported', '--worktree', str(directory)], env=environment, + stdout=directory.joinpath('import.stdout').open('w'), + stderr=directory.joinpath('import.stderr').open('w'), start_new_session=True) + processes.append(importer) + ui = subprocess.Popen(['st', '-g', '160x50', '-e', sys.executable, '-m', 'toad', 'acp', + str(Path(sys.executable).with_name('agent-comms-acp')), str(directory)], + env=environment, stdout=directory.joinpath('toad.stdout').open('w'), + stderr=directory.joinpath('toad.stderr').open('w'), start_new_session=True) + processes.append(ui) + started = time.monotonic() + Event().wait(15) + print(f't={time.monotonic()-started:.3f}s importer_exit={importer.poll()} registry={len(comms.registry.snapshot().threads)} ui_exit={ui.poll()}', flush=True) + capture(display, directory / 'during-import.png') + if importer.poll() is None: + os.killpg(importer.pid, signal.SIGTERM) + importer.wait(timeout=5) + Event().wait(10) + print(f't={time.monotonic()-started:.3f}s importer_exit={importer.poll()} registry={len(comms.registry.snapshot().threads)} ui_exit={ui.poll()}', flush=True) + capture(display, directory / 'after-import.png') + finally: + for process in reversed(processes): + if process.poll() is None: + process.terminate() + try: + process.wait(timeout=10) + except subprocess.TimeoutExpired: + process.kill() + process.wait() + comms.views.transcript_reads.close() + + +if __name__ == '__main__': + main() From a797e429ca4dfc785ed963c176c85ed0edb0c394 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 13:41:48 -0400 Subject: [PATCH 02/26] Record real Toad and concurrent-import acceptance Src-Budget: 20 --- docs/toad-list-investigation.md | 52 ++++++++++++++++++++++++++++ tests/test_import_observation.py | 24 +++++++++++++ tools/reproduce_thread_list.py | 2 +- tools/reproduce_toad_import_blank.py | 6 ++-- 4 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 tests/test_import_observation.py diff --git a/docs/toad-list-investigation.md b/docs/toad-list-investigation.md index 1efafd490f..8d2164baf1 100644 --- a/docs/toad-list-investigation.md +++ b/docs/toad-list-investigation.md @@ -69,6 +69,58 @@ NRA's latest ModuleSyntaxIndex could not load with the stack's newer metaclass-registry; its existing 3.12 environment cannot parse default type parameters. The working refactor-audit parser uses the project's Python 3.14. +## Branch verification + +```text +PYTHONPATH=src timeout 180 python -X faulthandler tools/reproduce_thread_list.py \ + /home/ts/.cache/agent-scratch/pi-toad-list/import-branch --seconds 120 --imports +elapsed=120.136s reads=904 empty=0 errors=0 writes=4157 writer_exit=0 imports=True +``` + +The writer imports 85 actual two-message Claude transcripts, then republishes +registry rows while Toad's actual SidebarReading runs. The standalone process +raises for no observations, any empty/error result or a failed writer. Main +cannot complete this loop: it blocks during the first import. Both original +faulthandler stacks were captured at 15 s; timeout bounded the run. There was no +successful empty registry read to paste. The physical screen is the blank-list +witness, not a fabricated empty read. + +Physical st/Xvfb comparison, same installed Toad, changing Core source selection: + +```text +main t=15.000s importer_exit=None registry=87 + Channels: + New Session; no channel or thread rows +main t=25.209s importer_exit=-15 registry=87 [scratch importer terminated] + #any 0/86; imported; sentinel-84; sentinel-83; ... +branch t=15.000s importer_exit=0 registry=87 + #any 0/86; imported; sentinel-84; sentinel-83; ... +branch t=25.331s importer_exit=0 registry=87 + same populated sidebar +``` + +The branch CLI reports two imported messages, zero omitted or truncated. Original +physical screens are retained under `ui-main-acp` / `ui-branch`. This demonstrates +an import-induced startup/reopening blank, not previously painted rows erased. +No provider/model input was sent and the production installation was not changed. + +After merging current main normally, focused checks under `timeout 300`, Python +`-X faulthandler`, and one pytest worker passed: 12 tests, including the real +independent importer/sidebar process, registry revisions and snapshot lifetime. +The broader five-file check completed with 53 passed / 5 failed. Failures remain: +malformed Claude-line skip expectation, removed Thread.pid, a model-change fixture +missing timestamp, changed checkpoint error wording, and NoReadNotice compared to +an empty string. These are not acceptance evidence; the import owner received its +three relevant failures. No test expectations were weakened. The stack Python +has no pytest; tests used the shared Core Python. The broad command inherited +eight workers from project defaults; the final check explicitly used one worker +and owned scratch. + +The source ratchet runs as its own command. Its first run identified the actual +20-line source growth and literal environment/kind comparisons in diagnostic +tools. Those now use the existing ThreadLaunch environment owner and the external +format's uniform content-block form, without spelling switches. The source budget +accounts for shared capture/acknowledgement lifetimes, not structural exceptions. + ## Evidence custody Small disposable scratch: `/home/ts/.cache/agent-scratch/pi-toad-list`, owned only diff --git a/tests/test_import_observation.py b/tests/test_import_observation.py new file mode 100644 index 0000000000..1118a82f9e --- /dev/null +++ b/tests/test_import_observation.py @@ -0,0 +1,24 @@ +"""An actual importer and sidebar reader must finish, not lock each other out.""" +import os +from pathlib import Path +import signal +import subprocess +import sys + + +def test_import_and_sidebar_observation_complete_on_a_real_private_bus(tmp_path): + repository = Path(__file__).resolve().parents[1] + process = subprocess.Popen( + [sys.executable, '-X', 'faulthandler', str(repository / 'tools/reproduce_thread_list.py'), + str(tmp_path / 'wire'), '--seconds', '3', '--imports'], + cwd=repository, env={**os.environ, 'PYTHONPATH': str(repository / 'src')}, + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, start_new_session=True, + ) + try: + output, _ = process.communicate(timeout=25) + print(output) + assert process.returncode == 0, output + finally: + if process.poll() is None: + os.killpg(process.pid, signal.SIGKILL) + process.wait() diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index a255727944..8fceede75a 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -54,7 +54,7 @@ def main(): source.write_text('\n'.join(json.dumps({ 'type': kind, 'uuid': str(index), 'timestamp': '2026-10-19T00:00:00Z', 'cwd': str(root), 'sessionId': 'proof', 'promptId': 'p-1', 'origin': {'kind': 'human'}, - 'message': {'role': kind, 'content': text if kind == 'user' else [{'type': 'text', 'text': text}]}, + 'message': {'role': kind, 'content': [{'type': 'text', 'text': text}]}, }) for index, (kind, text) in enumerate((('user', 'Scratch source input'), ('assistant', 'Scratch source reply')))) + '\n') comms.registry.register(Thread('sentinel', frozenset({'proof'}), str(root), diff --git a/tools/reproduce_toad_import_blank.py b/tools/reproduce_toad_import_blank.py index e446ffdb64..3790f8f75a 100644 --- a/tools/reproduce_toad_import_blank.py +++ b/tools/reproduce_toad_import_blank.py @@ -10,6 +10,7 @@ from threading import Event from agent_comms.comms import Comms +from agent_comms.launch_runtime import ThreadLaunch from agent_comms.thread_status import StoppedThreadStatus @@ -35,10 +36,9 @@ def main(): source.write_text('\n'.join(json.dumps({ 'type': kind, 'uuid': str(index), 'timestamp': '2026-10-19T00:00:00Z', 'cwd': str(directory), 'sessionId': 'proof', 'promptId': 'p-1', 'origin': {'kind': 'human'}, - 'message': {'content': text if kind == 'user' else [{'type': 'text', 'text': text}]}, + 'message': {'content': [{'type': 'text', 'text': text}]}, }) for index, (kind, text) in enumerate((('user', 'Scratch input'), ('assistant', 'Scratch reply')))) + '\n') - environment = {key: value for key, value in os.environ.items() if not key.startswith('PI_') - and key not in ('AGENT_COMMS_MANAGED', 'AGENT_COMMS_THREAD')} + environment = ThreadLaunch.strip(dict(os.environ)) environment.update(AGENT_COMMS_ROOT=str(root), XDG_STATE_HOME=str(directory / 'state'), XDG_CONFIG_HOME=str(directory / 'config')) processes = [] From 804b24aa0f23d199d73e2bfbf6b138a1849f4cb9 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 14:14:58 -0400 Subject: [PATCH 03/26] Own nested physical store acquisitions and delete caller-selected locking twins Reuse the held inode and mode inside an execution, preserving independent task/thread exclusion and joined checked-read worker lifetimes. Keep source checks on every borrow. Retire ownership before physical release; reject read-to-write nesting rather than deadlocking. Remove the investigation report and physical UI helper from the tree; evidence belongs to PR #1923. Src-Budget: 110 --- docs/toad-list-investigation.md | 138 --------------------------- src/agent_comms/message_bus.py | 36 ++----- src/agent_comms/messaging.py | 3 +- src/agent_comms/store_files.py | 118 +++++++++++++++++++---- src/agent_comms/thread_management.py | 2 +- src/agent_comms/wire_log.py | 26 +++-- tests/test_private_bus_checkpoint.py | 12 ++- tests/test_store_lock_reentry.py | 123 ++++++++++++++++++++++++ tools/reproduce_toad_import_blank.py | 85 ----------------- 9 files changed, 258 insertions(+), 285 deletions(-) delete mode 100644 docs/toad-list-investigation.md create mode 100644 tests/test_store_lock_reentry.py delete mode 100644 tools/reproduce_toad_import_blank.py diff --git a/docs/toad-list-investigation.md b/docs/toad-list-investigation.md deleted file mode 100644 index 8d2164baf1..0000000000 --- a/docs/toad-list-investigation.md +++ /dev/null @@ -1,138 +0,0 @@ -# Toad blank thread list during import - -Owner: pi-toad-list. Scope: importer acknowledgement and the MessageBus snapshot lifetime. - -## Actual source - -Core `ui_model.observation.SidebarReading` reads an initial roster and then -`HistoryViews.viewer_snapshot`. Toad `comms_root.CoordinationAccess.sidebar_observed` -applies that result to Core's `SidebarModel`; Toad's `SidebarModelView` and -`CommsSidebar` render its keyed rows. Native unread preparation is downstream of -that same captured registry, not an independent thread roster. Delivery cursors -filter messages, not registry membership. - -`RegistryStore.read_revision_unlocked` reads under its registry shared lock; -`PublicRegistry.write` and `PrivateRegistryGuard.write` publish with -`store_files._atomic_write_text`, using `os.replace` without unlinking the old -registry first. A genuinely missing registry creates the empty initial document; -malformed content raises. `RegistrySnapshot.capture` copies the current maps. -Visibility can intentionally omit stopped/archived/non-executor threads. - -A stored read failure becomes `SidebarUnreadable`, not `SidebarObserved` with an -empty roster. Toad clears the failed model and calls `show_error` with the original -cause. Deferred reads publish nothing. Thus a failed sidebar can look blank after -its notification; there is no evidence that this path caused the reported event. -No failed read is changed to a successful empty result by this patch. - -## Reproduction on main - -Installed Core main at 8b27e02aa and assigned branch base 0280f852e have the same -relevant implementation. Independent registration/sidebar loops on main: - -- public bus: 20.016 s, 1,072 observations, 3,808 writes, zero empty/error results; -- private bus: 20.021 s, 1,113 observations, 3,419 writes, zero empty/error results. - -Actual import, unlike registration alone, deadlocks. `ThreadManagement.import_thread` -holds root/wire exclusively at line 187 and calls `mark_delivered_through` at line -202. `MessageBus.snapshot` at line 102 opens another descriptor and asks for the -same wire lock shared. The importer blocks itself. The observer waits for wire in -`Messaging.user_identity`, called by `HistoryViews.viewer_snapshot`. Both original -process stacks are retained in the owner's small `import-stack.log`. - -`tools/reproduce_toad_import_blank.py` starts real private Core, 85 stopped threads, -a real CLI import, Xvfb and plain st running the installed Toad ACP entrypoint. -It sends no model input. At 15.000 s the importer was still running, the registry -contained 87 entries (85 agents, user, newly imported agent), and Toad's actual -screen showed Channels / New Session with no channel or thread rows. Terminating -only that scratch importer released the wire; at 25.209 s the same Toad showed -`#any 0/86`, `imported`, and sentinel rows without a restart. Original physical -screens are in the owner's scratch `ui-main-acp` directory. This demonstrates an -unanswered observation, not an observation that returned an empty registry. - -## Ownership change - -MessageBus retains one implementation of coherent registry/catalog/read/source -capture and one delivery-acknowledgement implementation. Normal readers acquire -wire for capture and release it before consuming the opened cut, as before. The -importing writer uses that same capture under the wire lock it already owns. -The import's declaration, acknowledgement and rollback remain within its original -exclusive lifetime; no newly published thread is exposed to a start between -registration and acknowledgement. No retry, retained nonempty roster or sleep is -added to production. - -AST evidence used the existing refactor-audit `Package.load` on all 379 Core -production modules, zero parse omissions. Original declarations, wire acquisition -lifetimes and every mark_delivered_through caller are retained in the owner's -`source-before.txt`. Toad's installed observer/model/widget consumers were read -semantically. Generic dynamically bound snapshot calls are not claimed resolved. -NRA's latest ModuleSyntaxIndex could not load with the stack's newer -metaclass-registry; its existing 3.12 environment cannot parse default type -parameters. The working refactor-audit parser uses the project's Python 3.14. - -## Branch verification - -```text -PYTHONPATH=src timeout 180 python -X faulthandler tools/reproduce_thread_list.py \ - /home/ts/.cache/agent-scratch/pi-toad-list/import-branch --seconds 120 --imports -elapsed=120.136s reads=904 empty=0 errors=0 writes=4157 writer_exit=0 imports=True -``` - -The writer imports 85 actual two-message Claude transcripts, then republishes -registry rows while Toad's actual SidebarReading runs. The standalone process -raises for no observations, any empty/error result or a failed writer. Main -cannot complete this loop: it blocks during the first import. Both original -faulthandler stacks were captured at 15 s; timeout bounded the run. There was no -successful empty registry read to paste. The physical screen is the blank-list -witness, not a fabricated empty read. - -Physical st/Xvfb comparison, same installed Toad, changing Core source selection: - -```text -main t=15.000s importer_exit=None registry=87 - Channels: + New Session; no channel or thread rows -main t=25.209s importer_exit=-15 registry=87 [scratch importer terminated] - #any 0/86; imported; sentinel-84; sentinel-83; ... -branch t=15.000s importer_exit=0 registry=87 - #any 0/86; imported; sentinel-84; sentinel-83; ... -branch t=25.331s importer_exit=0 registry=87 - same populated sidebar -``` - -The branch CLI reports two imported messages, zero omitted or truncated. Original -physical screens are retained under `ui-main-acp` / `ui-branch`. This demonstrates -an import-induced startup/reopening blank, not previously painted rows erased. -No provider/model input was sent and the production installation was not changed. - -After merging current main normally, focused checks under `timeout 300`, Python -`-X faulthandler`, and one pytest worker passed: 12 tests, including the real -independent importer/sidebar process, registry revisions and snapshot lifetime. -The broader five-file check completed with 53 passed / 5 failed. Failures remain: -malformed Claude-line skip expectation, removed Thread.pid, a model-change fixture -missing timestamp, changed checkpoint error wording, and NoReadNotice compared to -an empty string. These are not acceptance evidence; the import owner received its -three relevant failures. No test expectations were weakened. The stack Python -has no pytest; tests used the shared Core Python. The broad command inherited -eight workers from project defaults; the final check explicitly used one worker -and owned scratch. - -The source ratchet runs as its own command. Its first run identified the actual -20-line source growth and literal environment/kind comparisons in diagnostic -tools. Those now use the existing ThreadLaunch environment owner and the external -format's uniform content-block form, without spelling switches. The source budget -accounts for shared capture/acknowledgement lifetimes, not structural exceptions. - -## Evidence custody - -Small disposable scratch: `/home/ts/.cache/agent-scratch/pi-toad-list`, owned only -by pi-toad-list, for scratch buses, physical captures and short command results. -No saved user sessions, wire or uncertain inputs were moved, replayed or removed. -Physical captures for failed setup attempts are disposable; retain only the -accepted main/branch comparison and concise logs until review. - -## Assumed - -- The historical 13:02 event used the installed importer/snapshot code whose - deadlock is reproduced here. No historical observer stack was recorded. -- Legitimate stopped/archived filters and a truly empty new bus may show no rows. -- The Xvfb comparison verifies the installed Toad and real Core source path; - the production installation is not changed by this work. diff --git a/src/agent_comms/message_bus.py b/src/agent_comms/message_bus.py index c20c9a4113..c95885f0c6 100644 --- a/src/agent_comms/message_bus.py +++ b/src/agent_comms/message_bus.py @@ -6,7 +6,7 @@ import inspect from collections.abc import AsyncIterator, Awaitable, Callable, Iterator, Mapping, Sequence -from contextlib import AbstractContextManager, ExitStack, aclosing, contextmanager +from contextlib import ExitStack, aclosing, contextmanager from pathlib import Path from typing import TYPE_CHECKING @@ -100,20 +100,12 @@ def snapshot( """ with ExitStack() as stack: with self.locked(mode=SharedLock()): - captured = stack.enter_context(self._snapshot_unlocked(viewer)) - yield captured - - @contextmanager - def _snapshot_unlocked( - self, viewer: DisplayViewer, - ) -> Iterator[tuple[CapturedSelection, WireSnapshot]]: - """Capture under the caller's wire lock; retain the opened source after capture.""" - with self.log.opened_wire_snapshot() as source: - with CapturedSelection.reading( - self._registry, self._channels, self.reads, viewer, - self.activity_index.snapshot(source), - ) as basis: - pass + source = stack.enter_context(self.log.opened_wire_snapshot()) + with CapturedSelection.reading( + self._registry, self._channels, self.reads, viewer, + self.activity_index.snapshot(source), + ) as basis: + pass yield basis, source def inbox( @@ -295,21 +287,9 @@ def mark_delivered_through( self, name: str, sequence: int, *, target: CommandTarget = ANY_CHANNEL, ) -> None: """Mark the selected inbox through one captured cut, without loading its messages.""" - self._mark_delivered_snapshot(self.snapshot(NamedViewer(name)), sequence, target) - - def mark_delivered_through_unlocked( - self, name: str, sequence: int, *, target: CommandTarget = ANY_CHANNEL, - ) -> None: - """The importing writer already holds wire; never acquire it on another descriptor.""" - self._mark_delivered_snapshot(self._snapshot_unlocked(NamedViewer(name)), sequence, target) - - def _mark_delivered_snapshot( - self, captured: AbstractContextManager[tuple[CapturedSelection, WireSnapshot]], - sequence: int, target: CommandTarget, - ) -> None: if sequence < 0: raise ValueError("Delivery sequence cannot be negative.") - with captured as (basis, source): + with self.snapshot(NamedViewer(name)) as (basis, source): canonical = basis.require_viewer() delivery = DeliveryScope.capture(canonical, basis.registry, basis.catalog) scope = delivery.selection(target, basis.registry, diff --git a/src/agent_comms/messaging.py b/src/agent_comms/messaging.py index 5822de74a3..045edbaeab 100644 --- a/src/agent_comms/messaging.py +++ b/src/agent_comms/messaging.py @@ -197,6 +197,5 @@ def acknowledge( return basis.message_count def acknowledge_through(self, name: str, sequence: int) -> None: - # The bus owns capture under its read lock and the ledger validates that - # source. Nesting an exclusive wire hold deadlocks its snapshot reader. + # The bus owns capture under its read lock and the ledger validates that source. self.bus.mark_delivered_through(self.registry.require(name).name, sequence) diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index 512f4f9abb..11abff8790 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -8,6 +8,7 @@ import os import stat import tempfile +import threading import time from abc import ABC, abstractmethod from collections.abc import AsyncIterator, Iterator, Mapping @@ -25,7 +26,7 @@ from .declared_family import DeclaredFamily from .errors import RelationViolationError -from .private_path import PrivateFileRole +from .private_path import FileIdentity, PrivateFileRole with suppress(ModuleNotFoundError): # Windows; Win32Platform never asks a mode for its flock. import fcntl @@ -64,6 +65,10 @@ def verify_checkpoint(self, checkpoint: WritableCheckpoint, bus: WireLog, def require_writer(self) -> None: """Require the exclusive holder before rebuilding a replaced source.""" + @abstractmethod + def require_held(self, held: LockMode) -> None: + """Require an enclosing acquisition that already grants this mode.""" + @abstractmethod def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: """The mode that may repair what this holder could only name.""" @@ -87,6 +92,9 @@ def require_writer(self) -> None: from .private_bus_checkpoint import CheckpointNeedsRepair raise CheckpointNeedsRepair("Replaced bus file needs its page index rebuilt.") + def require_held(self, held: LockMode) -> None: + pass + def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: return ExclusiveLock() @@ -108,6 +116,13 @@ def verify_checkpoint(self, checkpoint: WritableCheckpoint, bus: WireLog, def require_writer(self) -> None: pass + def require_held(self, held: LockMode) -> None: + from .private_bus_checkpoint import CheckpointNeedsRepair + try: + held.require_writer() + except CheckpointNeedsRepair as error: + raise RelationViolationError("Cannot upgrade an enclosing shared store lock.") from error + def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: raise error @@ -287,21 +302,78 @@ def _store_lock_file(store_path: Path): yield lock_file -@contextmanager -def _held_store_source(store_path, lock_file, platform, max_bus_bytes, *, mode, lock_type): - """Check durability after locking; release the lock if the check refuses access.""" - try: - if max_bus_bytes is not None: - if type(max_bus_bytes) is not int or max_bus_bytes < 0: - raise ValueError("bus read cap must be a nonnegative integer") - if store_path.exists(): - bus_info = store_path.lstat() - if not stat.S_ISREG(bus_info.st_mode) or bus_info.st_size > max_bus_bytes: - raise RelationViolationError("Bus exceeds bounded read budget.") - with lock_type.hold(store_path, lock_file.fileno(), mode) as lock: - yield lock - finally: - platform.release_store_lock(lock_file.fileno()) +@dataclass +class StoreLockAcquisition: + """One physical acquisition, borrowed only by its own nested execution. + + Context copies cannot lend an exclusive transaction to another thread or + asyncio task. The inode identifies aliases; an ended scope is never reused. + """ + + file: BinaryIO + mode: LockMode + identity: FileIdentity + active: bool = True + _held: ClassVar[ + ContextVar[tuple[tuple[tuple[int, int, asyncio.Task | None], StoreLockAcquisition], ...]] + ] = ContextVar("store_lock_acquisitions", default=()) + + @staticmethod + def execution() -> tuple[int, int, asyncio.Task | None]: + try: + task = asyncio.current_task() + except RuntimeError: # Synchronous execution has no running event loop. + task = None + return os.getpid(), threading.get_ident(), task + + @classmethod + def current(cls, store_path: Path, mode: LockMode) -> StoreLockAcquisition | None: + execution = cls.execution() + held = cls._held.get() + try: + identity = FileIdentity.from_stat(store_lock_path(store_path).stat()) + except FileNotFoundError: + return None + for borrower, acquisition in reversed(held): + if acquisition.active and borrower == execution and acquisition.identity == identity: + mode.require_held(acquisition.mode) + return acquisition + return None + + @classmethod + @contextmanager + def holding(cls, store_path, file, platform, max_bus_bytes, *, mode, lock_type): + acquisition = cls(file, mode, FileIdentity.from_stat(os.fstat(file.fileno()))) + try: + with acquisition.source(store_path, max_bus_bytes, lock_type) as lock: + yield lock + finally: + if acquisition.active: + acquisition.active = False + platform.release_store_lock(file.fileno()) + + @contextmanager + def source(self, store_path: Path, max_bus_bytes: int | None, lock_type: type[StoreLock]): + """Recheck each borrow's source without reopening or downgrading its lock. + + An explicit joined-worker borrow installs the same owner in that worker; + merely inheriting a context never grants another execution access. + """ + if not self.active: + raise RelationViolationError("Store lock acquisition has ended.") + token = self._held.set((*self._held.get(), (self.execution(), self))) + try: + if max_bus_bytes is not None: + if type(max_bus_bytes) is not int or max_bus_bytes < 0: + raise ValueError("bus read cap must be a nonnegative integer") + if store_path.exists(): + bus_info = store_path.lstat() + if not stat.S_ISREG(bus_info.st_mode) or bus_info.st_size > max_bus_bytes: + raise RelationViolationError("Bus exceeds bounded read budget.") + with lock_type.hold(store_path, self.file.fileno(), self.mode) as lock: + yield lock + finally: + self._held.reset(token) @contextmanager @@ -318,6 +390,11 @@ def _store_lock( from .child_process import Platform from .private_bus_checkpoint import CheckpointNeedsRepair + held = StoreLockAcquisition.current(store_path, mode) + if held is not None: + with held.source(store_path, max_bus_bytes, lock_type) as lock: + yield lock + return platform = Platform.current() with _store_lock_file(store_path) as lock_file, ExitStack() as resources: def acquire(mode): @@ -325,7 +402,7 @@ def acquire(mode): platform.acquire_store_lock(lock_file.fileno(), mode=mode, blocking=blocking) else: contention.acquire(lock_file.fileno(), platform, mode=mode) - return resources.enter_context(_held_store_source( + return resources.enter_context(StoreLockAcquisition.holding( store_path, lock_file, platform, max_bus_bytes, mode=mode, lock_type=lock_type)) try: @@ -353,11 +430,16 @@ async def _async_store_lock( """ from .child_process import Platform + held = StoreLockAcquisition.current(store_path, mode) + if held is not None: + with held.source(store_path, max_bus_bytes, lock_type) as lock: + yield lock + return platform = Platform.current() wait = contention if contention is not None else StoreLockContention(math.inf if blocking else 0) with _store_lock_file(store_path) as lock_file: await wait.acquire_async(lock_file.fileno(), platform, mode=mode) - with _held_store_source(store_path, lock_file, platform, max_bus_bytes, + with StoreLockAcquisition.holding(store_path, lock_file, platform, max_bus_bytes, mode=mode, lock_type=lock_type) as lock: yield lock diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index ee4ca0b13e..4cd678afa5 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -199,7 +199,7 @@ def import_thread( _atomic_write_text(session_path, snapshot.pi_session(project)) try: self.registry._declare_unlocked(thread, StoppedThreadStatus()) - self.bus.mark_delivered_through_unlocked(name, self.bus.log.latest_sequence()) + self.bus.mark_delivered_through(name, self.bus.log.latest_sequence()) except (OSError, ValueError, CoordinationError): if name in self.registry: self.registry.remove(name) diff --git a/src/agent_comms/wire_log.py b/src/agent_comms/wire_log.py index fba44b60c0..6df1a67b37 100644 --- a/src/agent_comms/wire_log.py +++ b/src/agent_comms/wire_log.py @@ -51,11 +51,11 @@ StoreLock, StoreLockOwner, StoreLockContention, + StoreLockAcquisition, _atomic_write_text, _iter_jsonl_stream, _store_lock, _store_lock_file, - _held_store_source, ) from .wire_metadata import WireMetadata from .wire_record import WireRecord, WireScan @@ -603,20 +603,28 @@ async def _read_checked_under_lock(self, read: Callable, mode: LockMode): from .child_process import Platform from .coordinator import Coordination + def consume(acquisition): + with acquisition as lock, self._checked_source(lock) as source: + return read(source) + + held = StoreLockAcquisition.current(self.path, mode) + if held is not None: + return await Coordination.run_worker(partial( + consume, held.source(self.path, None, self.lock_type), + )) platform = Platform.current() with _store_lock_file(self.path) as lock_file: await StoreLockContention(math.inf).acquire_async( lock_file.fileno(), platform, mode=mode, ) - return await Coordination.run_worker( - partial(self._read_checked, lock_file, platform, read, mode), - ) - def _read_checked(self, lock_file, platform, read, mode): - with lock_file, _held_store_source(self.path, lock_file, platform, None, - mode=mode, lock_type=self.lock_type) as lock: - with self._checked_source(lock) as source: - return read(source) + def read_acquired(): + with lock_file: + return consume(StoreLockAcquisition.holding( + self.path, lock_file, platform, None, mode=mode, lock_type=self.lock_type, + )) + + return await Coordination.run_worker(read_acquired) @contextmanager def _checked_source(self, lock: CheckedWireStoreLock): diff --git a/tests/test_private_bus_checkpoint.py b/tests/test_private_bus_checkpoint.py index 7f92296aab..e4f94ff4ac 100644 --- a/tests/test_private_bus_checkpoint.py +++ b/tests/test_private_bus_checkpoint.py @@ -524,7 +524,7 @@ def test_reopened_root_preserves_checked_source_and_supports_append( def test_copied_root_rebuilds_its_index(tmp_path: Path, copy_root: bool) -> None: from agent_comms.child_process import Platform from agent_comms.private_bus_checkpoint import CheckpointNeedsRepair - from agent_comms.store_files import _store_lock_file + from agent_comms.store_files import StoreLockAcquisition, _store_lock_file original, root_id = _root(tmp_path) first = publish_initial(original, "sender", "Alice", "copied original") @@ -544,7 +544,9 @@ def test_copied_root_rebuilds_its_index(tmp_path: Path, copy_root: bool) -> None with _store_lock_file(log.path) as lock_file: platform.acquire_store_lock(lock_file.fileno(), mode=SharedLock(), blocking=True) with pytest.raises(CheckpointNeedsRepair, match="relocated"): - log._read_checked(lock_file, platform, lambda source: None, SharedLock()) + with StoreLockAcquisition.holding(log.path, lock_file, platform, None, + mode=SharedLock(), lock_type=log.lock_type): + pass # The real reader upgrades its lock and rebuilds the copied SQLite index. request = AddressedPage.capture(lookup=RecordedThreadIncarnation("fixture", 17002.0).lookup) witness, messages, _ = asyncio.run(log.read_checked_async( @@ -651,7 +653,7 @@ def test_shared_reader_hands_lagged_suffix_to_exclusive_repair(tmp_path: Path, m import agent_comms.private_bus_checkpoint as checkpoint from agent_comms.child_process import Platform - from agent_comms.store_files import _store_lock_file + from agent_comms.store_files import StoreLockAcquisition, _store_lock_file comms, _ = _root(tmp_path) publish_initial(comms, "sender", "Alice", "first") @@ -669,7 +671,9 @@ def test_shared_reader_hands_lagged_suffix_to_exclusive_repair(tmp_path: Path, m with _store_lock_file(log.path) as lock_file: platform.acquire_store_lock(lock_file.fileno(), mode=SharedLock(), blocking=True) with pytest.raises(checkpoint.CheckpointNeedsRepair): - log._read_checked(lock_file, platform, lambda source: None, SharedLock()) + with StoreLockAcquisition.holding(log.path, lock_file, platform, None, + mode=SharedLock(), lock_type=log.lock_type): + pass # The async reader then takes the exclusive lock, which repairs and reads. through = asyncio.run(log.read_checked_async(lambda source: source.witness.through_seq)) assert through == 2 diff --git a/tests/test_store_lock_reentry.py b/tests/test_store_lock_reentry.py new file mode 100644 index 0000000000..03b11635e2 --- /dev/null +++ b/tests/test_store_lock_reentry.py @@ -0,0 +1,123 @@ +"""Physical locking, not fake store objects: nesting must keep outsiders out.""" +import asyncio +from contextvars import copy_context +import fcntl +from pathlib import Path +import subprocess +import sys + +import pytest + +from agent_comms.comms import wire +from agent_comms.errors import RelationViolationError +from agent_comms.store_files import ( + SharedLock, StoreLockContention, _async_store_lock, _store_lock, store_lock_path, +) + + +def probe(path, *, shared=True): + """A fresh interpreter cannot inherit our process-local acquisition context.""" + result = subprocess.run([ + sys.executable, '-c', + 'import fcntl,sys\n' + 'with open(sys.argv[1], "rb") as stream:\n' + ' try: fcntl.flock(stream, int(sys.argv[2]) | fcntl.LOCK_NB)\n' + ' except BlockingIOError: sys.exit(73)\n', + str(store_lock_path(path)), str(fcntl.LOCK_SH if shared else fcntl.LOCK_EX), + ], capture_output=True, text=True, timeout=5) + assert result.returncode in (0, 73), result.stderr + return result.returncode == 0 + + +def test_nested_reader_reuses_descriptor_without_downgrading_or_releasing(tmp_path): + path = tmp_path / 'wire' + with _store_lock(path) as outer: + with pytest.raises(ValueError, match='consumer failed'): + with _store_lock(path, mode=SharedLock(), contention=StoreLockContention(0)) as inner: + assert inner.descriptor == outer.descriptor + links = [] + for descriptor in Path('/proc/self/fd').iterdir(): + try: + links.append(descriptor.readlink()) + except FileNotFoundError: + pass + assert links.count(store_lock_path(path)) == 1 + assert not probe(path) + raise ValueError('consumer failed') + assert not probe(path) + assert probe(path, shared=False) + + +def test_shared_nesting_never_silently_upgrades(tmp_path): + path = tmp_path / 'wire' + with _store_lock(path, mode=SharedLock()) as outer: + with _store_lock(path, mode=SharedLock()) as inner: + assert inner.descriptor == outer.descriptor + assert probe(path) + assert not probe(path, shared=False) + with pytest.raises(RelationViolationError, match='Cannot upgrade'): + with _store_lock(path): + pytest.fail('upgraded the enclosing read transaction') + assert probe(path, shared=False) + + +def test_ended_context_does_not_reuse_its_closed_descriptor(tmp_path): + path = tmp_path / 'wire' + with _store_lock(path): + retained = copy_context() + def capture_again(): + with _store_lock(path): + assert not probe(path) + retained.run(capture_again) + assert probe(path, shared=False) + + +def test_nested_source_still_checks_each_borrows_budget(tmp_path): + path = tmp_path / 'wire' + path.write_bytes(b'not within the nested budget') + with _store_lock(path): + with pytest.raises(RelationViolationError, match='bounded read budget'): + with _store_lock(path, mode=SharedLock(), max_bus_bytes=1): + pytest.fail('nested source skipped its read budget') + assert not probe(path) + assert probe(path, shared=False) + + +def test_async_and_sync_nest_but_independent_tasks_and_threads_do_not(tmp_path): + path = tmp_path / 'wire' + async def independent(): + with pytest.raises(BlockingIOError): + async with _async_store_lock(path, mode=SharedLock(), blocking=False): + pytest.fail('another task borrowed an exclusive transaction') + def thread_reader(): + with pytest.raises(BlockingIOError): + with _store_lock(path, mode=SharedLock(), blocking=False): + pytest.fail('another thread borrowed an exclusive transaction') + async def run(): + async with _async_store_lock(path) as outer: + async with _async_store_lock(path, mode=SharedLock()) as inner: + assert inner.descriptor == outer.descriptor + with _store_lock(path, mode=SharedLock()) as synchronous: + assert synchronous.descriptor == outer.descriptor + assert not probe(path) + await asyncio.create_task(independent()) + await asyncio.to_thread(thread_reader) + assert not probe(path) + assert probe(path, shared=False) + asyncio.run(run()) + + +def test_checked_async_worker_explicitly_borrows_the_original_acquisition(tmp_path): + comms = wire(tmp_path) + comms.messaging.initialize_private_initial_protocol() + async def run(): + with comms.bus.log.locked() as outer: + def read(source): + # The joined worker may itself nest a normal source acquisition. + with comms.bus.log.locked(mode=SharedLock()) as inner: + assert inner.descriptor == outer.descriptor + return source.witness.through_seq + assert await comms.bus.log.read_checked_async(read) == 0 + assert not probe(comms.bus.log.path) + assert probe(comms.bus.log.path, shared=False) + asyncio.run(run()) diff --git a/tools/reproduce_toad_import_blank.py b/tools/reproduce_toad_import_blank.py deleted file mode 100644 index 3790f8f75a..0000000000 --- a/tools/reproduce_toad_import_blank.py +++ /dev/null @@ -1,85 +0,0 @@ -"""Physical Toad on an isolated X display while a real import holds the wire.""" -import argparse -import json -import os -from pathlib import Path -import signal -import subprocess -import sys -import time -from threading import Event - -from agent_comms.comms import Comms -from agent_comms.launch_runtime import ThreadLaunch -from agent_comms.thread_status import StoppedThreadStatus - - -def capture(display, destination): - subprocess.run(['import', '-display', display, '-window', 'root', str(destination)], check=True) - print(f'physical screen: {destination}', flush=True) - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument('directory', type=Path) - args = parser.parse_args() - directory = args.directory.resolve() - directory.mkdir(parents=True, exist_ok=True) - root = directory / 'wire' - comms = Comms(root) - comms.messaging.initialize_private_initial_protocol() - for index in range(85): - comms.registry.register(comms.registry.new_thread(f'sentinel-{index:02}', frozenset({'proof'}), str(directory)), - StoppedThreadStatus()) - comms.messaging.user_identity(str(directory)) - source = directory / 'source.jsonl' - source.write_text('\n'.join(json.dumps({ - 'type': kind, 'uuid': str(index), 'timestamp': '2026-10-19T00:00:00Z', - 'cwd': str(directory), 'sessionId': 'proof', 'promptId': 'p-1', 'origin': {'kind': 'human'}, - 'message': {'content': [{'type': 'text', 'text': text}]}, - }) for index, (kind, text) in enumerate((('user', 'Scratch input'), ('assistant', 'Scratch reply')))) + '\n') - environment = ThreadLaunch.strip(dict(os.environ)) - environment.update(AGENT_COMMS_ROOT=str(root), XDG_STATE_HOME=str(directory / 'state'), - XDG_CONFIG_HOME=str(directory / 'config')) - processes = [] - try: - x = subprocess.Popen(['Xvfb', '-displayfd', '1', '-screen', '0', '1500x1000x24', '-nolisten', 'tcp'], - stdout=subprocess.PIPE, stderr=directory.joinpath('xvfb.log').open('w'), text=True) - processes.append(x) - display = ':' + x.stdout.readline().strip() - environment['DISPLAY'] = display - importer = subprocess.Popen([sys.executable, str(Path(sys.executable).with_name('agent-comms')), '--root', str(root), - 'import-thread', '--harness', 'claude_code', '--source', str(source), - '--name', 'imported', '--worktree', str(directory)], env=environment, - stdout=directory.joinpath('import.stdout').open('w'), - stderr=directory.joinpath('import.stderr').open('w'), start_new_session=True) - processes.append(importer) - ui = subprocess.Popen(['st', '-g', '160x50', '-e', sys.executable, '-m', 'toad', 'acp', - str(Path(sys.executable).with_name('agent-comms-acp')), str(directory)], - env=environment, stdout=directory.joinpath('toad.stdout').open('w'), - stderr=directory.joinpath('toad.stderr').open('w'), start_new_session=True) - processes.append(ui) - started = time.monotonic() - Event().wait(15) - print(f't={time.monotonic()-started:.3f}s importer_exit={importer.poll()} registry={len(comms.registry.snapshot().threads)} ui_exit={ui.poll()}', flush=True) - capture(display, directory / 'during-import.png') - if importer.poll() is None: - os.killpg(importer.pid, signal.SIGTERM) - importer.wait(timeout=5) - Event().wait(10) - print(f't={time.monotonic()-started:.3f}s importer_exit={importer.poll()} registry={len(comms.registry.snapshot().threads)} ui_exit={ui.poll()}', flush=True) - capture(display, directory / 'after-import.png') - finally: - for process in reversed(processes): - if process.poll() is None: - process.terminate() - try: - process.wait(timeout=10) - except subprocess.TimeoutExpired: - process.kill() - process.wait() - comms.views.transcript_reads.close() - - -if __name__ == '__main__': - main() From e10cbf64b3a2b2ed09fdeca0869d629b7a74cf3b Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 14:21:56 -0400 Subject: [PATCH 04/26] Type the acquisition and joined-reader resource boundaries --- src/agent_comms/store_files.py | 13 ++++++++++--- src/agent_comms/wire_log.py | 10 ++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index 11abff8790..a07d50973b 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -121,7 +121,9 @@ def require_held(self, held: LockMode) -> None: try: held.require_writer() except CheckpointNeedsRepair as error: - raise RelationViolationError("Cannot upgrade an enclosing shared store lock.") from error + raise RelationViolationError( + "Cannot upgrade an enclosing shared store lock." + ) from error def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: raise error @@ -342,7 +344,10 @@ def current(cls, store_path: Path, mode: LockMode) -> StoreLockAcquisition | Non @classmethod @contextmanager - def holding(cls, store_path, file, platform, max_bus_bytes, *, mode, lock_type): + def holding( + cls, store_path: Path, file: BinaryIO, platform: Platform, max_bus_bytes: int | None, + *, mode: LockMode, lock_type: type[StoreLock], + ) -> Iterator[StoreLock]: acquisition = cls(file, mode, FileIdentity.from_stat(os.fstat(file.fileno()))) try: with acquisition.source(store_path, max_bus_bytes, lock_type) as lock: @@ -353,7 +358,9 @@ def holding(cls, store_path, file, platform, max_bus_bytes, *, mode, lock_type): platform.release_store_lock(file.fileno()) @contextmanager - def source(self, store_path: Path, max_bus_bytes: int | None, lock_type: type[StoreLock]): + def source( + self, store_path: Path, max_bus_bytes: int | None, lock_type: type[StoreLock], + ) -> Iterator[StoreLock]: """Recheck each borrow's source without reopening or downgrading its lock. An explicit joined-worker borrow installs the same owner in that worker; diff --git a/src/agent_comms/wire_log.py b/src/agent_comms/wire_log.py index 6df1a67b37..3b198d72c1 100644 --- a/src/agent_comms/wire_log.py +++ b/src/agent_comms/wire_log.py @@ -11,7 +11,7 @@ import time from abc import ABC, abstractmethod from collections.abc import Callable, Iterable, Iterator, Mapping -from contextlib import ExitStack, contextmanager +from contextlib import AbstractContextManager, ExitStack, contextmanager from dataclasses import dataclass, field, replace from functools import partial from io import DEFAULT_BUFFER_SIZE @@ -599,11 +599,13 @@ async def read_checked_async(self, read: Callable): except CheckpointNeedsRepair as error: return await self._read_checked_under_lock(read, mode.after_repair_needed(error)) - async def _read_checked_under_lock(self, read: Callable, mode: LockMode): + async def _read_checked_under_lock[T]( + self, read: Callable[[CheckedRead], T], mode: LockMode, + ) -> T: from .child_process import Platform from .coordinator import Coordination - def consume(acquisition): + def consume(acquisition: AbstractContextManager) -> T: with acquisition as lock, self._checked_source(lock) as source: return read(source) @@ -618,7 +620,7 @@ def consume(acquisition): lock_file.fileno(), platform, mode=mode, ) - def read_acquired(): + def read_acquired() -> T: with lock_file: return consume(StoreLockAcquisition.holding( self.path, lock_file, platform, None, mode=mode, lock_type=self.lock_type, From f3df2d740b299b65028eab77aa205ea2622c0586 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 14:54:36 -0400 Subject: [PATCH 05/26] P223: own missing acquisition, execution and read-budget states Decode absent task/acquisition states once; keep budget checking on declared read policies. File closure owns physical liveness instead of an active mirror. Move the shared repair error to errors and import its consumers at module scope. Mount the pinned audit package at its canonical source import name for fresh checkouts, without a second implementation or an environment install. No budget, baseline or suppression increase. --- refactor_audit | 1 + src/agent_comms/checkpoint_seals.py | 6 +- src/agent_comms/errors.py | 4 + src/agent_comms/private_bus_checkpoint.py | 19 +-- src/agent_comms/store_files.py | 174 +++++++++++++--------- src/agent_comms/wire_log.py | 27 ++-- tests/test_private_bus_checkpoint.py | 23 ++- tests/test_store_lock_reentry.py | 5 +- 8 files changed, 136 insertions(+), 123 deletions(-) create mode 120000 refactor_audit diff --git a/refactor_audit b/refactor_audit new file mode 120000 index 0000000000..2f54d52fc8 --- /dev/null +++ b/refactor_audit @@ -0,0 +1 @@ +src/agent_comms/skills/refactor-audit/scripts/audit \ No newline at end of file diff --git a/src/agent_comms/checkpoint_seals.py b/src/agent_comms/checkpoint_seals.py index 531f8b2f08..30327b35a3 100644 --- a/src/agent_comms/checkpoint_seals.py +++ b/src/agent_comms/checkpoint_seals.py @@ -8,7 +8,7 @@ from typing import TYPE_CHECKING, Literal from .declared_family import DeclaredFamily -from .errors import RelationViolationError +from .errors import CheckpointNeedsRepairError, RelationViolationError from .field_codec import Id128Text, Sha256Text from .private_path import FileRevision @@ -148,6 +148,4 @@ def require_rebuilt(self, saved: PrefixWitness, rebuilt: PrefixWitness) -> None: raise RelationViolationError("Private bus checkpoint pending suffix differs from intent.") def require_final(self, saved: PrefixWitness, db_path: Path) -> None: - from .private_bus_checkpoint import CheckpointNeedsRepair - - raise CheckpointNeedsRepair("Private bus checkpoint has an unfinished write.") + raise CheckpointNeedsRepairError("Private bus checkpoint has an unfinished write.") diff --git a/src/agent_comms/errors.py b/src/agent_comms/errors.py index fa7e60689a..73eeb92824 100644 --- a/src/agent_comms/errors.py +++ b/src/agent_comms/errors.py @@ -42,3 +42,7 @@ def __init__(self, wire_root_id: str, wire_seq: int, message_id: str) -> None: class HumanClaimBlockedError(RelationViolationError): """A prior private reservation cannot be safely retried.""" + + +class CheckpointNeedsRepairError(Exception): + """A shared source requires exclusive checkpoint repair before it can open.""" diff --git a/src/agent_comms/private_bus_checkpoint.py b/src/agent_comms/private_bus_checkpoint.py index 80f874cdb7..7687dd4816 100644 --- a/src/agent_comms/private_bus_checkpoint.py +++ b/src/agent_comms/private_bus_checkpoint.py @@ -33,7 +33,7 @@ ClaimOwner, ClaimProjection, ClaimTransition, ClaimTransitionError, ) from .coordination_tables.publications import PublicationIntents -from .errors import HumanClaimBlockedError, RelationViolationError +from .errors import CheckpointNeedsRepairError, HumanClaimBlockedError, RelationViolationError from .private_path import FileIdentity, FileRevision from .store_files import ExclusiveLock, LockMode, SharedLock from .field_codec import FieldCodec, MalformedRecord @@ -100,7 +100,7 @@ def require_source(self, marker: WireMetadata, info: os.stat_result, stream: Bin if self.through_seq > marker.last_seq: raise RelationViolationError("Private bus checkpoint exceeds the durable sequence marker.") if self.revision.identity != FileIdentity.from_stat(info): - raise CheckpointNeedsRepair("Private bus source was relocated.") + raise CheckpointNeedsRepairError("Private bus source was relocated.") def require_append(self, marker: WireMetadata, info: os.stat_result, offset: int, sequence: int) -> None: @@ -1318,15 +1318,6 @@ def collect(offset, raw, record): return expected -class CheckpointNeedsRepair(Exception): # noqa: N818 - names a state - """A reader holding the shared bus lock found rows the checkpoint does not - cover yet, or an unfinished checkpoint write. - - Only a holder of the exclusive bus lock may repair the checkpoint, so the - reader takes the exclusive lock and checks again. - """ - - def _check_final_checkpoint_read_only(bus, marker, db, stream, path) -> PrefixWitness: """Check a final, up-to-date checkpoint under the shared bus lock; never writes.""" saved = _saved(db) @@ -1334,7 +1325,7 @@ def _check_final_checkpoint_read_only(bus, marker, db, stream, path) -> PrefixWi saved.require_source(marker, info, stream) marker.seal.require_final(saved, path) if FileRevision.from_stat(info) != saved.revision: - raise CheckpointNeedsRepair("Bus has rows the checkpoint does not cover yet.") + raise CheckpointNeedsRepairError("Bus has rows the checkpoint does not cover yet.") return saved @@ -1369,7 +1360,7 @@ def _verify_open_checkpoint_unlocked(bus, marker, db, stream, path) -> PrefixWit info = os.fstat(stream.fileno()) try: saved.require_source(marker, info, stream) - except CheckpointNeedsRepair: + except CheckpointNeedsRepairError: # The copied index rows point at the old file; rebuild them. return _rebuild_checkpoint_unlocked(bus, marker, db, path, info, marker.seal) recovered = marker.seal.recover(bus, marker, db, path, saved, info) @@ -1453,7 +1444,7 @@ def opened_private_checkpoint_unlocked(bus: WireLog, marker: WireMetadata, *, """Keep only this canonical lock's verified source/index resources open. Under the shared bus lock other readers run concurrently, so the check is - read-only and raises CheckpointNeedsRepair where an exclusive holder would + read-only and raises CheckpointNeedsRepairError where an exclusive holder would repair the checkpoint. """ path = _path(bus.path) diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index a07d50973b..fa49752b18 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -14,6 +14,7 @@ from collections.abc import AsyncIterator, Iterator, Mapping from contextlib import ( AbstractAsyncContextManager, + AbstractContextManager, ExitStack, asynccontextmanager, contextmanager, @@ -24,8 +25,9 @@ from pathlib import Path from typing import TYPE_CHECKING, BinaryIO, ClassVar +from .child_process import Platform from .declared_family import DeclaredFamily -from .errors import RelationViolationError +from .errors import CheckpointNeedsRepairError, RelationViolationError from .private_path import FileIdentity, PrivateFileRole with suppress(ModuleNotFoundError): # Windows; Win32Platform never asks a mode for its flock. @@ -34,9 +36,7 @@ if TYPE_CHECKING: import sqlite3 - from .child_process import Platform from .private_bus_checkpoint import ( - CheckpointNeedsRepair, PrefixWitness, WritableCheckpoint, ) @@ -70,7 +70,7 @@ def require_held(self, held: LockMode) -> None: """Require an enclosing acquisition that already grants this mode.""" @abstractmethod - def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: + def after_repair_needed(self, error: CheckpointNeedsRepairError) -> LockMode: """The mode that may repair what this holder could only name.""" @@ -89,13 +89,12 @@ def verify_checkpoint(self, checkpoint: WritableCheckpoint, bus: WireLog, return checkpoint.check_final(bus, marker, db, stream) def require_writer(self) -> None: - from .private_bus_checkpoint import CheckpointNeedsRepair - raise CheckpointNeedsRepair("Replaced bus file needs its page index rebuilt.") + raise CheckpointNeedsRepairError("Replaced bus file needs its page index rebuilt.") def require_held(self, held: LockMode) -> None: pass - def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: + def after_repair_needed(self, error: CheckpointNeedsRepairError) -> LockMode: return ExclusiveLock() @@ -117,15 +116,14 @@ def require_writer(self) -> None: pass def require_held(self, held: LockMode) -> None: - from .private_bus_checkpoint import CheckpointNeedsRepair try: held.require_writer() - except CheckpointNeedsRepair as error: + except CheckpointNeedsRepairError as error: raise RelationViolationError( "Cannot upgrade an enclosing shared store lock." ) from error - def after_repair_needed(self, error: CheckpointNeedsRepair) -> LockMode: + def after_repair_needed(self, error: CheckpointNeedsRepairError) -> LockMode: raise error @@ -249,6 +247,31 @@ def releasing(cls): scope.close() +class StoreReadBudget: + """The unbounded source-read state.""" + + def check(self, path: Path) -> None: + pass + + +@dataclass(frozen=True) +class BoundedStoreReadBudget(StoreReadBudget): + maximum: int + + def __post_init__(self) -> None: + if type(self.maximum) is not int or self.maximum < 0: + raise ValueError("bus read cap must be a nonnegative integer") + + def check(self, path: Path) -> None: + if path.exists(): + info = path.lstat() + if not stat.S_ISREG(info.st_mode) or info.st_size > self.maximum: + raise RelationViolationError("Bus exceeds bounded read budget.") + + +UNBOUNDED_STORE_READ = StoreReadBudget() + + class StoreOwner(DeclaredFamily): """Shared declaration membership for composable store capabilities.""" @@ -274,9 +297,9 @@ def declare_lock_order(cls, owners: tuple[type[StoreLockOwner], ...]) -> None: for rank, owner in enumerate(owners): owner.lock_rank = rank - def locked(self, *, blocking: bool = True, max_bus_bytes: int | None = None, + def locked(self, *, blocking: bool = True, read_budget: StoreReadBudget = UNBOUNDED_STORE_READ, mode: LockMode = ExclusiveLock(), contention: StoreLockContention | None = None): - return _store_lock(self.path, blocking=blocking, max_bus_bytes=max_bus_bytes, + return _store_lock(self.path, blocking=blocking, read_budget=read_budget, mode=mode, contention=contention, lock_type=self.lock_type) def locked_async(self) -> AbstractAsyncContextManager[StoreLock]: @@ -304,79 +327,90 @@ def _store_lock_file(store_path: Path): yield lock_file +@dataclass(frozen=True) +class SynchronousStoreExecution: + """This thread is not executing an asyncio task.""" + + +type StoreLockExecution = tuple[int, int, asyncio.Task | SynchronousStoreExecution] + + +class UnacquiredStoreLock: + """No physical acquisition can be borrowed by this execution.""" + + def sources(self, path: Path, budget: StoreReadBudget, lock_type: type[StoreLock] + ) -> tuple[AbstractContextManager[StoreLock], ...]: + return () + + @dataclass class StoreLockAcquisition: - """One physical acquisition, borrowed only by its own nested execution. - - Context copies cannot lend an exclusive transaction to another thread or - asyncio task. The inode identifies aliases; an ended scope is never reused. - """ + """One physical file lock, borrowed only within its owning execution.""" file: BinaryIO mode: LockMode identity: FileIdentity - active: bool = True - _held: ClassVar[ - ContextVar[tuple[tuple[tuple[int, int, asyncio.Task | None], StoreLockAcquisition], ...]] - ] = ContextVar("store_lock_acquisitions", default=()) + _held: ClassVar[ContextVar[tuple[tuple[StoreLockExecution, StoreLockAcquisition], ...]]] = ( + ContextVar("store_lock_acquisitions", default=()) + ) @staticmethod - def execution() -> tuple[int, int, asyncio.Task | None]: + def execution() -> StoreLockExecution: + owner = os.getpid(), threading.get_ident() try: task = asyncio.current_task() - except RuntimeError: # Synchronous execution has no running event loop. - task = None - return os.getpid(), threading.get_ident(), task + except RuntimeError as error: + if error.args != ("no running event loop",): + raise + return *owner, SynchronousStoreExecution() + match task: + case None: + return *owner, SynchronousStoreExecution() + case _: + return *owner, task @classmethod - def current(cls, store_path: Path, mode: LockMode) -> StoreLockAcquisition | None: + def current( + cls, store_path: Path, mode: LockMode, + ) -> StoreLockAcquisition | UnacquiredStoreLock: execution = cls.execution() held = cls._held.get() try: identity = FileIdentity.from_stat(store_lock_path(store_path).stat()) except FileNotFoundError: - return None + return UnacquiredStoreLock() for borrower, acquisition in reversed(held): - if acquisition.active and borrower == execution and acquisition.identity == identity: + if (borrower == execution and acquisition.identity == identity + and not acquisition.file.closed): mode.require_held(acquisition.mode) return acquisition - return None + return UnacquiredStoreLock() + + def sources(self, path: Path, budget: StoreReadBudget, lock_type: type[StoreLock] + ) -> tuple[AbstractContextManager[StoreLock], ...]: + return (self.source(path, budget, lock_type),) @classmethod @contextmanager def holding( - cls, store_path: Path, file: BinaryIO, platform: Platform, max_bus_bytes: int | None, + cls, store_path: Path, file: BinaryIO, platform: Platform, read_budget: StoreReadBudget, *, mode: LockMode, lock_type: type[StoreLock], ) -> Iterator[StoreLock]: - acquisition = cls(file, mode, FileIdentity.from_stat(os.fstat(file.fileno()))) - try: - with acquisition.source(store_path, max_bus_bytes, lock_type) as lock: - yield lock - finally: - if acquisition.active: - acquisition.active = False - platform.release_store_lock(file.fileno()) + with ExitStack() as resources: + resources.callback(platform.release_store_lock, file.fileno()) + acquisition = cls(file, mode, FileIdentity.from_stat(os.fstat(file.fileno()))) + yield resources.enter_context(acquisition.source(store_path, read_budget, lock_type)) @contextmanager def source( - self, store_path: Path, max_bus_bytes: int | None, lock_type: type[StoreLock], + self, store_path: Path, read_budget: StoreReadBudget, lock_type: type[StoreLock], ) -> Iterator[StoreLock]: - """Recheck each borrow's source without reopening or downgrading its lock. - - An explicit joined-worker borrow installs the same owner in that worker; - merely inheriting a context never grants another execution access. - """ - if not self.active: + """Verify and lend the source, including to an explicitly joined worker.""" + if self.file.closed: raise RelationViolationError("Store lock acquisition has ended.") token = self._held.set((*self._held.get(), (self.execution(), self))) try: - if max_bus_bytes is not None: - if type(max_bus_bytes) is not int or max_bus_bytes < 0: - raise ValueError("bus read cap must be a nonnegative integer") - if store_path.exists(): - bus_info = store_path.lstat() - if not stat.S_ISREG(bus_info.st_mode) or bus_info.st_size > max_bus_bytes: - raise RelationViolationError("Bus exceeds bounded read budget.") + read_budget.check(store_path) with lock_type.hold(store_path, self.file.fileno(), self.mode) as lock: yield lock finally: @@ -388,18 +422,16 @@ def _store_lock( store_path: Path, *, blocking: bool = True, - max_bus_bytes: int | None = None, + read_budget: StoreReadBudget = UNBOUNDED_STORE_READ, mode: LockMode = ExclusiveLock(), contention: StoreLockContention | None = None, lock_type: type[StoreLock] = StoreLock, ) -> Iterator[StoreLock]: """Acquire a store lock synchronously; inherited POSIX locks end at last close.""" - from .child_process import Platform - from .private_bus_checkpoint import CheckpointNeedsRepair - - held = StoreLockAcquisition.current(store_path, mode) - if held is not None: - with held.source(store_path, max_bus_bytes, lock_type) as lock: + for source in StoreLockAcquisition.current(store_path, mode).sources( + store_path, read_budget, lock_type + ): + with source as lock: yield lock return platform = Platform.current() @@ -410,11 +442,11 @@ def acquire(mode): else: contention.acquire(lock_file.fileno(), platform, mode=mode) return resources.enter_context(StoreLockAcquisition.holding( - store_path, lock_file, platform, max_bus_bytes, mode=mode, lock_type=lock_type)) + store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type)) try: lock = acquire(mode) - except CheckpointNeedsRepair as error: + except CheckpointNeedsRepairError as error: lock = acquire(mode.after_repair_needed(error)) yield lock @@ -424,29 +456,23 @@ async def _async_store_lock( store_path: Path, *, blocking: bool = True, - max_bus_bytes: int | None = None, + read_budget: StoreReadBudget = UNBOUNDED_STORE_READ, mode: LockMode = ExclusiveLock(), contention: StoreLockContention | None = None, lock_type: type[StoreLock] = StoreLock, ) -> AsyncIterator[StoreLock]: - """Borrow the same physical resource without blocking its owner event loop. - - Cancellation during acquisition closes only the unacquired descriptor. The - durability check runs only after the lock is acquired; no SQLite - connection or consumer is transferred to another thread. - """ - from .child_process import Platform - - held = StoreLockAcquisition.current(store_path, mode) - if held is not None: - with held.source(store_path, max_bus_bytes, lock_type) as lock: + """Borrow the same physical resource cancellably on this event loop.""" + for source in StoreLockAcquisition.current(store_path, mode).sources( + store_path, read_budget, lock_type + ): + with source as lock: yield lock return platform = Platform.current() wait = contention if contention is not None else StoreLockContention(math.inf if blocking else 0) with _store_lock_file(store_path) as lock_file: await wait.acquire_async(lock_file.fileno(), platform, mode=mode) - with StoreLockAcquisition.holding(store_path, lock_file, platform, max_bus_bytes, + with StoreLockAcquisition.holding(store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type) as lock: yield lock diff --git a/src/agent_comms/wire_log.py b/src/agent_comms/wire_log.py index 3b198d72c1..d3e9d254fb 100644 --- a/src/agent_comms/wire_log.py +++ b/src/agent_comms/wire_log.py @@ -35,6 +35,7 @@ ClaimProjection, ) from .errors import ( + CheckpointNeedsRepairError, RelationViolationError, StoredHistoryUnreadable, ) @@ -52,6 +53,7 @@ StoreLockOwner, StoreLockContention, StoreLockAcquisition, + UNBOUNDED_STORE_READ, _atomic_write_text, _iter_jsonl_stream, _store_lock, @@ -581,22 +583,13 @@ def checked_read(self, *, blocking: bool = True, contention: StoreLockContention yield source async def read_checked_async(self, read: Callable): - """Acquire cancellably, then consume the whole source in its worker. - - Return detached observations only. Verification and the callback share - one resource thread; no SQLite connection crosses to the event loop. - The worker releases its locks before returning its result; result - delivery must not keep a POSIX lock until the event loop resumes. - Cancellation joins acquired work before the original descriptor closes. - """ - from .private_bus_checkpoint import CheckpointNeedsRepair - + """Consume in the joined source worker; cancellation waits for acquired work.""" # Readers share the bus lock so idle workers don't queue behind each # other; only a checkpoint repair needs the exclusive lock. mode = SharedLock() try: return await self._read_checked_under_lock(read, mode) - except CheckpointNeedsRepair as error: + except CheckpointNeedsRepairError as error: return await self._read_checked_under_lock(read, mode.after_repair_needed(error)) async def _read_checked_under_lock[T]( @@ -609,11 +602,10 @@ def consume(acquisition: AbstractContextManager) -> T: with acquisition as lock, self._checked_source(lock) as source: return read(source) - held = StoreLockAcquisition.current(self.path, mode) - if held is not None: - return await Coordination.run_worker(partial( - consume, held.source(self.path, None, self.lock_type), - )) + for source in StoreLockAcquisition.current(self.path, mode).sources( + self.path, UNBOUNDED_STORE_READ, self.lock_type + ): + return await Coordination.run_worker(partial(consume, source)) platform = Platform.current() with _store_lock_file(self.path) as lock_file: await StoreLockContention(math.inf).acquire_async( @@ -623,7 +615,8 @@ def consume(acquisition: AbstractContextManager) -> T: def read_acquired() -> T: with lock_file: return consume(StoreLockAcquisition.holding( - self.path, lock_file, platform, None, mode=mode, lock_type=self.lock_type, + self.path, lock_file, platform, UNBOUNDED_STORE_READ, + mode=mode, lock_type=self.lock_type, )) return await Coordination.run_worker(read_acquired) diff --git a/tests/test_private_bus_checkpoint.py b/tests/test_private_bus_checkpoint.py index e4f94ff4ac..280f55dd27 100644 --- a/tests/test_private_bus_checkpoint.py +++ b/tests/test_private_bus_checkpoint.py @@ -18,13 +18,15 @@ from agent_comms.child_process import ProcessIdentity from agent_comms.comms import Comms from agent_comms.envelope_claim_transitions import ExistingFileClaim -from agent_comms.errors import RelationViolationError +from agent_comms.errors import CheckpointNeedsRepairError, RelationViolationError from agent_comms.messages import Message, HandoffKind, InfoKind from agent_comms.bus_source_page import AddressedPage from agent_comms.private_bus_checkpoint import ( install_private_bus_checkpoint, ) -from agent_comms.store_files import ExclusiveLock, SharedLock, _store_lock +from agent_comms.store_files import ( + BoundedStoreReadBudget, ExclusiveLock, SharedLock, UNBOUNDED_STORE_READ, _store_lock, +) from agent_comms.threads import Thread @@ -470,7 +472,7 @@ def test_over_1000_initials_and_8mib_complete_lookup(tmp_path: Path, monkeypatch # source API supplies a complete-prefix+bounded-page alternative. with ( pytest.raises(RelationViolationError), - comms.bus.log.locked(max_bus_bytes=8 * 1024 * 1024), + comms.bus.log.locked(read_budget=BoundedStoreReadBudget(8 * 1024 * 1024)), ): pass with monkeypatch.context() as patch: @@ -523,7 +525,6 @@ def test_reopened_root_preserves_checked_source_and_supports_append( @pytest.mark.parametrize("copy_root", [False, True]) def test_copied_root_rebuilds_its_index(tmp_path: Path, copy_root: bool) -> None: from agent_comms.child_process import Platform - from agent_comms.private_bus_checkpoint import CheckpointNeedsRepair from agent_comms.store_files import StoreLockAcquisition, _store_lock_file original, root_id = _root(tmp_path) @@ -543,8 +544,8 @@ def test_copied_root_rebuilds_its_index(tmp_path: Path, copy_root: bool) -> None platform = Platform.current() with _store_lock_file(log.path) as lock_file: platform.acquire_store_lock(lock_file.fileno(), mode=SharedLock(), blocking=True) - with pytest.raises(CheckpointNeedsRepair, match="relocated"): - with StoreLockAcquisition.holding(log.path, lock_file, platform, None, + with pytest.raises(CheckpointNeedsRepairError, match="relocated"): + with StoreLockAcquisition.holding(log.path, lock_file, platform, UNBOUNDED_STORE_READ, mode=SharedLock(), lock_type=log.lock_type): pass # The real reader upgrades its lock and rebuilds the copied SQLite index. @@ -670,8 +671,8 @@ def test_shared_reader_hands_lagged_suffix_to_exclusive_repair(tmp_path: Path, m # A shared-lock reader never writes the checkpoint; it names the repair. with _store_lock_file(log.path) as lock_file: platform.acquire_store_lock(lock_file.fileno(), mode=SharedLock(), blocking=True) - with pytest.raises(checkpoint.CheckpointNeedsRepair): - with StoreLockAcquisition.holding(log.path, lock_file, platform, None, + with pytest.raises(CheckpointNeedsRepairError): + with StoreLockAcquisition.holding(log.path, lock_file, platform, UNBOUNDED_STORE_READ, mode=SharedLock(), lock_type=log.lock_type): pass # The async reader then takes the exclusive lock, which repairs and reads. @@ -683,11 +684,9 @@ def test_shared_reader_hands_lagged_suffix_to_exclusive_repair(tmp_path: Path, m @pytest.mark.parametrize(("mode", "escalated"), [(SharedLock(), ExclusiveLock), (ExclusiveLock(), None)]) def test_only_a_shared_reader_retries_a_needed_repair_exclusively(mode, escalated) -> None: - from agent_comms.private_bus_checkpoint import CheckpointNeedsRepair - - error = CheckpointNeedsRepair("lagged suffix") + error = CheckpointNeedsRepairError("lagged suffix") if escalated is None: - with pytest.raises(CheckpointNeedsRepair) as raised: + with pytest.raises(CheckpointNeedsRepairError) as raised: mode.after_repair_needed(error) assert raised.value is error else: diff --git a/tests/test_store_lock_reentry.py b/tests/test_store_lock_reentry.py index 03b11635e2..57a1f40431 100644 --- a/tests/test_store_lock_reentry.py +++ b/tests/test_store_lock_reentry.py @@ -11,7 +11,8 @@ from agent_comms.comms import wire from agent_comms.errors import RelationViolationError from agent_comms.store_files import ( - SharedLock, StoreLockContention, _async_store_lock, _store_lock, store_lock_path, + BoundedStoreReadBudget, SharedLock, StoreLockContention, + _async_store_lock, _store_lock, store_lock_path, ) @@ -77,7 +78,7 @@ def test_nested_source_still_checks_each_borrows_budget(tmp_path): path.write_bytes(b'not within the nested budget') with _store_lock(path): with pytest.raises(RelationViolationError, match='bounded read budget'): - with _store_lock(path, mode=SharedLock(), max_bus_bytes=1): + with _store_lock(path, mode=SharedLock(), read_budget=BoundedStoreReadBudget(1)): pytest.fail('nested source skipped its read budget') assert not probe(path) assert probe(path, shared=False) From ba5403c08593e68f37cc87598e0ba15091137136 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 15:13:34 -0400 Subject: [PATCH 06/26] P223: dispatch acquisition through its state and bootstrap the declared audit source --- refactor_audit | 1 - src/agent_comms/store_files.py | 134 ++++++++++++++++++++------------- src/agent_comms/wire_log.py | 36 ++++++--- tools/debt_ratchet.py | 19 ++++- tools/reproduce_thread_list.py | 6 +- 5 files changed, 126 insertions(+), 70 deletions(-) delete mode 120000 refactor_audit diff --git a/refactor_audit b/refactor_audit deleted file mode 120000 index 2f54d52fc8..0000000000 --- a/refactor_audit +++ /dev/null @@ -1 +0,0 @@ -src/agent_comms/skills/refactor-audit/scripts/audit \ No newline at end of file diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index fa49752b18..b10eb36788 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -11,7 +11,7 @@ import threading import time from abc import ABC, abstractmethod -from collections.abc import AsyncIterator, Iterator, Mapping +from collections.abc import AsyncIterator, Awaitable, Callable, Iterator, Mapping from contextlib import ( AbstractAsyncContextManager, AbstractContextManager, @@ -37,6 +37,7 @@ import sqlite3 from .private_bus_checkpoint import ( + CheckedRead, PrefixWitness, WritableCheckpoint, ) @@ -332,15 +333,57 @@ class SynchronousStoreExecution: """This thread is not executing an asyncio task.""" -type StoreLockExecution = tuple[int, int, asyncio.Task | SynchronousStoreExecution] +@dataclass(frozen=True) +class StoreLockExecution: + pid: int + thread: int + task: asyncio.Task | SynchronousStoreExecution class UnacquiredStoreLock: """No physical acquisition can be borrowed by this execution.""" - def sources(self, path: Path, budget: StoreReadBudget, lock_type: type[StoreLock] - ) -> tuple[AbstractContextManager[StoreLock], ...]: - return () + @contextmanager + def locked( + self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, + contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + ) -> Iterator[StoreLock]: + platform = Platform.current() + with _store_lock_file(store_path) as lock_file, ExitStack() as resources: + def acquire(mode): + if contention is None: + platform.acquire_store_lock(lock_file.fileno(), mode=mode, blocking=blocking) + else: + contention.acquire(lock_file.fileno(), platform, mode=mode) + return resources.enter_context(StoreLockAcquisition.holding( + store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type)) + try: + lock = acquire(mode) + except CheckpointNeedsRepairError as error: + lock = acquire(mode.after_repair_needed(error)) + yield lock + + @asynccontextmanager + async def locked_async( + self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, + contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + ) -> AsyncIterator[StoreLock]: + platform = Platform.current() + wait = contention if contention is not None else StoreLockContention( + math.inf if blocking else 0 + ) + with _store_lock_file(store_path) as lock_file: + await wait.acquire_async(lock_file.fileno(), platform, mode=mode) + with StoreLockAcquisition.holding( + store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type, + ) as lock: + yield lock + + async def read_checked[T]( + self, log: WireLog, read: Callable[[CheckedRead], T], mode: LockMode, + worker: Callable[[Callable[[], T]], Awaitable[T]], + ) -> T: + return await log._read_checked_fresh(read, mode, worker) @dataclass @@ -356,18 +399,15 @@ class StoreLockAcquisition: @staticmethod def execution() -> StoreLockExecution: - owner = os.getpid(), threading.get_ident() - try: - task = asyncio.current_task() - except RuntimeError as error: - if error.args != ("no running event loop",): - raise - return *owner, SynchronousStoreExecution() - match task: + task: asyncio.Task | SynchronousStoreExecution = SynchronousStoreExecution() + match asyncio._get_running_loop(): case None: - return *owner, SynchronousStoreExecution() - case _: - return *owner, task + pass + case loop: + match asyncio.current_task(loop): + case asyncio.Task() as running: + task = running + return StoreLockExecution(os.getpid(), threading.get_ident(), task) @classmethod def current( @@ -386,9 +426,25 @@ def current( return acquisition return UnacquiredStoreLock() - def sources(self, path: Path, budget: StoreReadBudget, lock_type: type[StoreLock] - ) -> tuple[AbstractContextManager[StoreLock], ...]: - return (self.source(path, budget, lock_type),) + def locked( + self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, + contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + ) -> AbstractContextManager[StoreLock]: + return self.source(store_path, read_budget, lock_type) + + @asynccontextmanager + async def locked_async( + self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, + contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + ) -> AsyncIterator[StoreLock]: + with self.source(store_path, read_budget, lock_type) as lock: + yield lock + + async def read_checked[T]( + self, log: WireLog, read: Callable[[CheckedRead], T], mode: LockMode, + worker: Callable[[Callable[[], T]], Awaitable[T]], + ) -> T: + return await log._read_checked_borrowed(read, self, worker) @classmethod @contextmanager @@ -428,26 +484,9 @@ def _store_lock( lock_type: type[StoreLock] = StoreLock, ) -> Iterator[StoreLock]: """Acquire a store lock synchronously; inherited POSIX locks end at last close.""" - for source in StoreLockAcquisition.current(store_path, mode).sources( - store_path, read_budget, lock_type - ): - with source as lock: - yield lock - return - platform = Platform.current() - with _store_lock_file(store_path) as lock_file, ExitStack() as resources: - def acquire(mode): - if contention is None: - platform.acquire_store_lock(lock_file.fileno(), mode=mode, blocking=blocking) - else: - contention.acquire(lock_file.fileno(), platform, mode=mode) - return resources.enter_context(StoreLockAcquisition.holding( - store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type)) - - try: - lock = acquire(mode) - except CheckpointNeedsRepairError as error: - lock = acquire(mode.after_repair_needed(error)) + with StoreLockAcquisition.current(store_path, mode).locked( + store_path, read_budget, mode, contention, lock_type, blocking, + ) as lock: yield lock @@ -462,19 +501,10 @@ async def _async_store_lock( lock_type: type[StoreLock] = StoreLock, ) -> AsyncIterator[StoreLock]: """Borrow the same physical resource cancellably on this event loop.""" - for source in StoreLockAcquisition.current(store_path, mode).sources( - store_path, read_budget, lock_type - ): - with source as lock: - yield lock - return - platform = Platform.current() - wait = contention if contention is not None else StoreLockContention(math.inf if blocking else 0) - with _store_lock_file(store_path) as lock_file: - await wait.acquire_async(lock_file.fileno(), platform, mode=mode) - with StoreLockAcquisition.holding(store_path, lock_file, platform, read_budget, - mode=mode, lock_type=lock_type) as lock: - yield lock + async with StoreLockAcquisition.current(store_path, mode).locked_async( + store_path, read_budget, mode, contention, lock_type, blocking, + ) as lock: + yield lock def _atomic_write_text( diff --git a/src/agent_comms/wire_log.py b/src/agent_comms/wire_log.py index d3e9d254fb..4aaa135198 100644 --- a/src/agent_comms/wire_log.py +++ b/src/agent_comms/wire_log.py @@ -10,7 +10,7 @@ import stat import time from abc import ABC, abstractmethod -from collections.abc import Callable, Iterable, Iterator, Mapping +from collections.abc import Awaitable, Callable, Iterable, Iterator, Mapping from contextlib import AbstractContextManager, ExitStack, contextmanager from dataclasses import dataclass, field, replace from functools import partial @@ -23,6 +23,7 @@ ) from .audience_manifest import MAX_WIRE_SEQ +from .child_process import Platform from .bus_publication import ( CommittedDelivery, has_private_wire_fields, @@ -595,17 +596,30 @@ async def read_checked_async(self, read: Callable): async def _read_checked_under_lock[T]( self, read: Callable[[CheckedRead], T], mode: LockMode, ) -> T: - from .child_process import Platform from .coordinator import Coordination - def consume(acquisition: AbstractContextManager) -> T: - with acquisition as lock, self._checked_source(lock) as source: - return read(source) + return await StoreLockAcquisition.current(self.path, mode).read_checked( + self, read, mode, Coordination.run_worker, + ) - for source in StoreLockAcquisition.current(self.path, mode).sources( - self.path, UNBOUNDED_STORE_READ, self.lock_type - ): - return await Coordination.run_worker(partial(consume, source)) + def _consume_checked[T]( + self, read: Callable[[CheckedRead], T], acquisition: AbstractContextManager, + ) -> T: + with acquisition as lock, self._checked_source(lock) as source: + return read(source) + + async def _read_checked_borrowed[T]( + self, read: Callable[[CheckedRead], T], held: StoreLockAcquisition, + worker: Callable[[Callable[[], T]], Awaitable[T]], + ) -> T: + return await worker(partial( + self._consume_checked, read, held.source(self.path, UNBOUNDED_STORE_READ, self.lock_type) + )) + + async def _read_checked_fresh[T]( + self, read: Callable[[CheckedRead], T], mode: LockMode, + worker: Callable[[Callable[[], T]], Awaitable[T]], + ) -> T: platform = Platform.current() with _store_lock_file(self.path) as lock_file: await StoreLockContention(math.inf).acquire_async( @@ -614,12 +628,12 @@ def consume(acquisition: AbstractContextManager) -> T: def read_acquired() -> T: with lock_file: - return consume(StoreLockAcquisition.holding( + return self._consume_checked(read, StoreLockAcquisition.holding( self.path, lock_file, platform, UNBOUNDED_STORE_READ, mode=mode, lock_type=self.lock_type, )) - return await Coordination.run_worker(read_acquired) + return await worker(read_acquired) @contextmanager def _checked_source(self, lock: CheckedWireStoreLock): diff --git a/tools/debt_ratchet.py b/tools/debt_ratchet.py index 88d166ce97..b4819cd18b 100644 --- a/tools/debt_ratchet.py +++ b/tools/debt_ratchet.py @@ -23,9 +23,18 @@ from dataclasses import dataclass from functools import cached_property from pathlib import Path -from refactor_audit.handler_declarations import BuiltinHandlerDeclarations -from refactor_audit.measures import FamilyFlattened as AuditFamilyFlattened -from refactor_audit.measures import ForeignAbsenceProbe as AuditForeignAbsenceProbe + +sys.path.insert(0, str( + Path(__file__).resolve().parents[1] / subprocess.check_output( + ["git", "config", "--file", str(Path(__file__).resolve().parents[1] / ".gitmodules"), + "--get", "submodule.src/agent_comms/skills/refactor-audit.path"], text=True, + ).strip() / "scripts" +)) + +import audit +from audit.handler_declarations import BuiltinHandlerDeclarations +from audit.measures import FamilyFlattened as AuditFamilyFlattened +from audit.measures import ForeignAbsenceProbe as AuditForeignAbsenceProbe from agent_comms.core_stamp import WheelStamp from agent_comms.declared_family import DeclaredFamily @@ -34,6 +43,8 @@ from agent_comms.field_codec import FieldCodec, boundary_decoder, projected from agent_comms.mro_dispatch import MroDispatch, handles +AUDIT_SCRIPTS = Path(audit.__path__[0]).parent + class Measure(DeclaredFamily, affix="Measure"): @staticmethod @@ -228,7 +239,7 @@ def environment(root: str) -> dict[str, str]: Tools import the same revision's ``src`` (``upgrade_bus.py`` puts it first), never the installed one.""" paths = dict.fromkeys((Path(root).parts[0], "src")) - return {**os.environ, "MYPYPATH": os.pathsep.join(paths)} + return {**os.environ, "MYPYPATH": os.pathsep.join((*paths, str(AUDIT_SCRIPTS)))} class SourceLines(Measure): diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 8fceede75a..41b9cf5e10 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -7,6 +7,7 @@ import time from pathlib import Path +from agent_comms.cli_bridges.claude import ClaudeTranscript from agent_comms.comms import Comms from agent_comms.storage_rules import StorageRoots from agent_comms.threads import Thread @@ -23,7 +24,6 @@ def writer(root, stop, ready, result, imports, dump_after): while not stop.is_set(): name = f'writer-{count % 85}' if imports and count < 85: - from agent_comms.cli_bridges.claude import ClaudeTranscript comms.threads.import_thread(ClaudeTranscript(root / 'source.jsonl'), name=name, worktree=str(root), tags=frozenset({'proof'})) elif imports: @@ -85,7 +85,9 @@ def main(): process.join(15) comms.views.transcript_reads.close() print(f'elapsed={time.monotonic()-start:.3f}s reads={reads} empty={empty} errors={errors} writes={counts.get(timeout=2)} writer_exit={process.exitcode} imports={args.imports}', flush=True) - if not reads or empty or errors or process.exitcode != 0: + if not reads: + raise RuntimeError('Sidebar observation produced no reads.') + if (empty, errors, process.exitcode) != (0, 0, 0): raise RuntimeError('Sidebar observation failed; see the original results above') From 79fadbde8c588bf73c615535cc9fa4b2d8453e69 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 15:19:00 -0400 Subject: [PATCH 07/26] P223: pass concrete contention resources and derive package roots for src-wide checks --- src/agent_comms/store_files.py | 87 +++++++++------------------------- tools/debt_ratchet.py | 9 +++- 2 files changed, 29 insertions(+), 67 deletions(-) diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index b10eb36788..528005f374 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -66,9 +66,8 @@ def verify_checkpoint(self, checkpoint: WritableCheckpoint, bus: WireLog, def require_writer(self) -> None: """Require the exclusive holder before rebuilding a replaced source.""" - @abstractmethod def require_held(self, held: LockMode) -> None: - """Require an enclosing acquisition that already grants this mode.""" + pass @abstractmethod def after_repair_needed(self, error: CheckpointNeedsRepairError) -> LockMode: @@ -92,9 +91,6 @@ def verify_checkpoint(self, checkpoint: WritableCheckpoint, bus: WireLog, def require_writer(self) -> None: raise CheckpointNeedsRepairError("Replaced bus file needs its page index rebuilt.") - def require_held(self, held: LockMode) -> None: - pass - def after_repair_needed(self, error: CheckpointNeedsRepairError) -> LockMode: return ExclusiveLock() @@ -130,11 +126,7 @@ def after_repair_needed(self, error: CheckpointNeedsRepairError) -> LockMode: @dataclass(slots=True) class StoreLockContention: - """One operation's borrowed wait resource, spent only at physical locks. - - Source reading, decoding and consumer work cannot spend this resource. - It grants no owner, input, proof or replay permission. - """ + """One operation's contention budget, spent only while acquiring physical locks.""" remaining: float @@ -167,8 +159,6 @@ async def acquire_async(self, descriptor: int, platform: Platform, *, mode: Lock @dataclass class StoreLockRelease: - """Actions owned by one outer physical-lock lifetime.""" - actions: ExitStack = field(default_factory=ExitStack) active: bool = True @@ -182,16 +172,12 @@ def callback(self, operation, /, *args, **kwargs) -> None: def close(self) -> None: if self.active: - # Callbacks may take fresh locks; every inherited context sees this - # scope as ended before the first callback runs. self.active = False self.actions.close() @dataclass(frozen=True) class StoreLock: - """A held store lock's file descriptor.""" - descriptor: int _releases: ClassVar[ContextVar[StoreLockRelease]] = ContextVar( "store_lock_release", default=StoreLockRelease(active=False) @@ -200,7 +186,6 @@ class StoreLock: @classmethod @contextmanager def hold(cls, path: Path, descriptor: int, mode: LockMode): - """Keep a plain store lock; it does not hold a checked bus read.""" yield cls(descriptor) @classmethod @@ -230,12 +215,7 @@ def worker_context(cls) -> Context: @classmethod @contextmanager def releasing(cls): - """Nested locks share the outer lock's post-release work. - - The scope object, not a context-specific token, owns retirement. Marking - it inactive retires every inherited context before callbacks take fresh - locks. A stale inherited value therefore starts an independent scope. - """ + """Share outer retirement; inherited contexts observe that same scope ending.""" current = cls._releases.get() if current.active: yield current @@ -274,14 +254,10 @@ def check(self, path: Path) -> None: class StoreOwner(DeclaredFamily): - """Shared declaration membership for composable store capabilities.""" - __slots__ = () class StoreLockOwner(StoreOwner, declared_name=None): - """Store transactions use ranks assigned by the complete ordered declaration.""" - __slots__ = () path: Path lock_rank: ClassVar[int] @@ -314,12 +290,7 @@ def store_lock_path(store_path: Path) -> Path: @contextmanager def _store_lock_file(store_path: Path): - """Own the original lock inode's descriptor before any acquisition. - - The lock file is never written, so it is opened read-only: closing a - descriptor opened for writing is a close-write event that wakes every - owner watching the store directory, once per lock taken. - """ + """Open read-only: lock retirement must not generate a close-write watch event.""" store_path.parent.mkdir(parents=True, exist_ok=True) lock_path = store_lock_path(store_path) with StoreLock.releasing(), os.fdopen( @@ -330,7 +301,7 @@ def _store_lock_file(store_path: Path): @dataclass(frozen=True) class SynchronousStoreExecution: - """This thread is not executing an asyncio task.""" + pass @dataclass(frozen=True) @@ -341,20 +312,15 @@ class StoreLockExecution: class UnacquiredStoreLock: - """No physical acquisition can be borrowed by this execution.""" - @contextmanager def locked( self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, - contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + contention: StoreLockContention, lock_type: type[StoreLock], blocking: bool, ) -> Iterator[StoreLock]: platform = Platform.current() with _store_lock_file(store_path) as lock_file, ExitStack() as resources: def acquire(mode): - if contention is None: - platform.acquire_store_lock(lock_file.fileno(), mode=mode, blocking=blocking) - else: - contention.acquire(lock_file.fileno(), platform, mode=mode) + contention.acquire(lock_file.fileno(), platform, mode=mode) return resources.enter_context(StoreLockAcquisition.holding( store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type)) try: @@ -366,14 +332,11 @@ def acquire(mode): @asynccontextmanager async def locked_async( self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, - contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + contention: StoreLockContention, lock_type: type[StoreLock], blocking: bool, ) -> AsyncIterator[StoreLock]: platform = Platform.current() - wait = contention if contention is not None else StoreLockContention( - math.inf if blocking else 0 - ) with _store_lock_file(store_path) as lock_file: - await wait.acquire_async(lock_file.fileno(), platform, mode=mode) + await contention.acquire_async(lock_file.fileno(), platform, mode=mode) with StoreLockAcquisition.holding( store_path, lock_file, platform, read_budget, mode=mode, lock_type=lock_type, ) as lock: @@ -388,8 +351,6 @@ async def read_checked[T]( @dataclass class StoreLockAcquisition: - """One physical file lock, borrowed only within its owning execution.""" - file: BinaryIO mode: LockMode identity: FileIdentity @@ -428,14 +389,14 @@ def current( def locked( self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, - contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + contention: StoreLockContention, lock_type: type[StoreLock], blocking: bool, ) -> AbstractContextManager[StoreLock]: return self.source(store_path, read_budget, lock_type) @asynccontextmanager async def locked_async( self, store_path: Path, read_budget: StoreReadBudget, mode: LockMode, - contention: StoreLockContention | None, lock_type: type[StoreLock], blocking: bool, + contention: StoreLockContention, lock_type: type[StoreLock], blocking: bool, ) -> AsyncIterator[StoreLock]: with self.source(store_path, read_budget, lock_type) as lock: yield lock @@ -473,7 +434,6 @@ def source( self._held.reset(token) -@contextmanager def _store_lock( store_path: Path, *, @@ -482,16 +442,14 @@ def _store_lock( mode: LockMode = ExclusiveLock(), contention: StoreLockContention | None = None, lock_type: type[StoreLock] = StoreLock, -) -> Iterator[StoreLock]: - """Acquire a store lock synchronously; inherited POSIX locks end at last close.""" - with StoreLockAcquisition.current(store_path, mode).locked( - store_path, read_budget, mode, contention, lock_type, blocking, - ) as lock: - yield lock +) -> AbstractContextManager[StoreLock]: + wait = contention or StoreLockContention(math.inf if blocking else 0) + return StoreLockAcquisition.current(store_path, mode).locked( + store_path, read_budget, mode, wait, lock_type, blocking, + ) -@asynccontextmanager -async def _async_store_lock( +def _async_store_lock( store_path: Path, *, blocking: bool = True, @@ -499,12 +457,11 @@ async def _async_store_lock( mode: LockMode = ExclusiveLock(), contention: StoreLockContention | None = None, lock_type: type[StoreLock] = StoreLock, -) -> AsyncIterator[StoreLock]: - """Borrow the same physical resource cancellably on this event loop.""" - async with StoreLockAcquisition.current(store_path, mode).locked_async( - store_path, read_budget, mode, contention, lock_type, blocking, - ) as lock: - yield lock +) -> AbstractAsyncContextManager[StoreLock]: + wait = contention or StoreLockContention(math.inf if blocking else 0) + return StoreLockAcquisition.current(store_path, mode).locked_async( + store_path, read_budget, mode, wait, lock_type, blocking, + ) def _atomic_write_text( diff --git a/tools/debt_ratchet.py b/tools/debt_ratchet.py index b4819cd18b..e863477dd2 100644 --- a/tools/debt_ratchet.py +++ b/tools/debt_ratchet.py @@ -1458,7 +1458,9 @@ def declares_version(source: bytes) -> bool: @classmethod def formats(cls, repo: Path, ref: str, root: str, changed: set[str]) -> dict[str, StoredForm]: """Each versioned record in the changed modules, measured by that revision's own code.""" - package = Path(root).parent + package = Path(root) + while git(repo, "ls-tree", "--name-only", ref, "--", str(package / "__init__.py")): + package = package.parent modules = [".".join(Path(path).relative_to(package).with_suffix("").parts) for path in sorted(changed & python_paths(repo, ref, root)) if cls.declares_version(git(repo, "show", f"{ref}:{path}"))] @@ -1468,7 +1470,10 @@ def formats(cls, repo: Path, ref: str, root: str, changed: set[str]) -> dict[str source = git(repo, "archive", ref, "--", str(package)) subprocess.run(["tar", "-x", "-C", scratch], input=source, check=True, timeout=60) # Extracted source is stamped with its commit the way a build stamps it. - WheelStamp(revision(repo, ref)).write(Path(scratch) / root / "package_stamp.py") + for module in {module.split(".")[0] for module in modules}: + WheelStamp(revision(repo, ref)).write( + Path(scratch) / package / module / "package_stamp.py" + ) result = subprocess.run([sys.executable, "-c", cls.probe, str(Path(scratch) / package), str(Path(__file__).resolve().parent), *modules], cwd=scratch, env=cls.environment(root), From 2c6c21899eda005ba666cca3a8b53dc42c11987b Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 15:20:01 -0400 Subject: [PATCH 08/26] Use the acquisition context directly during lookup --- src/agent_comms/store_files.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/agent_comms/store_files.py b/src/agent_comms/store_files.py index 528005f374..d6a415e143 100644 --- a/src/agent_comms/store_files.py +++ b/src/agent_comms/store_files.py @@ -375,12 +375,11 @@ def current( cls, store_path: Path, mode: LockMode, ) -> StoreLockAcquisition | UnacquiredStoreLock: execution = cls.execution() - held = cls._held.get() try: identity = FileIdentity.from_stat(store_lock_path(store_path).stat()) except FileNotFoundError: return UnacquiredStoreLock() - for borrower, acquisition in reversed(held): + for borrower, acquisition in reversed(cls._held.get()): if (borrower == execution and acquisition.identity == identity and not acquisition.file.closed): mode.require_held(acquisition.mode) From 64e2d47d0863c7bc87fb1fb5ca66b1b19220630f Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 15:24:26 -0400 Subject: [PATCH 09/26] Check the physical contention boundary and remove a redundant proof local --- tests/test_store_lock_order.py | 5 +++-- tools/reproduce_thread_list.py | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_store_lock_order.py b/tests/test_store_lock_order.py index 77b60c319e..32ee92869d 100644 --- a/tests/test_store_lock_order.py +++ b/tests/test_store_lock_order.py @@ -12,7 +12,8 @@ from agent_comms.historical_views import DMDisplayHistory from agent_comms.message_bus import MessageBus from agent_comms.store_files import ( - ExclusiveLock, SharedLock, StoreLock, StoreLockOwner, StoreOwner, store_lock_path, + ExclusiveLock, SharedLock, StoreLock, StoreLockContention, StoreLockOwner, + StoreOwner, store_lock_path, ) from agent_comms.threads import Thread from agent_comms.wire_log import WireLog @@ -67,7 +68,7 @@ def profile(frame, event, arg): ranks[str(store_lock_path(owner.path))] = owner.lock_rank return operation = frame.f_code.co_name - if operation not in {"acquire_store_lock", "_store_lock_file"}: + if operation != "_store_lock_file" and frame.f_code is not StoreLockContention.acquire.__code__: return actor = threading.current_thread().name active = held.setdefault(actor, {}) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 41b9cf5e10..19375aa926 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -59,8 +59,7 @@ def main(): ('assistant', 'Scratch source reply')))) + '\n') comms.registry.register(Thread('sentinel', frozenset({'proof'}), str(root), storage_roots=StorageRoots(root / 'scratch' / 'sentinel')), StoppedThreadStatus()) - request = ObserveSidebar(str(root), True, False) - reading = request.observation() + reading = ObserveSidebar(str(root), True, False).observation() stop, ready, counts = mp.Event(), mp.Event(), mp.Queue() process = mp.Process(target=writer, args=(root, stop, ready, counts, args.imports, args.dump_after)) process.start() From 496f161bab69a4cc88a80d57e2fc8dbe1e3972dd Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:28:38 -0400 Subject: [PATCH 10/26] P223: type and format the real-process thread-list reproduction --- tools/reproduce_thread_list.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 19375aa926..406a18b503 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -1,21 +1,27 @@ """Real scratch-bus sidebar observation concurrent with registry publication.""" +from __future__ import annotations + import argparse -import json import faulthandler +import json import multiprocessing as mp -from dataclasses import replace import time +from dataclasses import replace +from multiprocessing.queues import Queue +from multiprocessing.synchronize import Event from pathlib import Path from agent_comms.cli_bridges.claude import ClaudeTranscript from agent_comms.comms import Comms from agent_comms.storage_rules import StorageRoots -from agent_comms.threads import Thread from agent_comms.thread_status import StoppedThreadStatus +from agent_comms.threads import Thread from agent_comms.ui_model.observation import ObserveSidebar, SidebarObserved, SidebarUnreadable -def writer(root, stop, ready, result, imports, dump_after): +def writer( + root: Path, stop: Event, ready: Event, result: Queue[int], imports: bool, dump_after: float, +) -> None: if dump_after: faulthandler.dump_traceback_later(dump_after) comms = Comms(root) @@ -37,7 +43,7 @@ def writer(root, stop, ready, result, imports, dump_after): result.put(count) -def main(): +def main() -> None: parser = argparse.ArgumentParser() parser.add_argument('root', type=Path) parser.add_argument('--seconds', type=float, default=120) From a47539c31584f9455ce511fda38a7e0211c4f4b7 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:28:59 -0400 Subject: [PATCH 11/26] P727 rehold claims across scopes through their original owner and envelope --- src/agent_comms/envelope_claim_transitions.py | 6 +++++- src/agent_comms/selected_claim.py | 19 +++---------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/agent_comms/envelope_claim_transitions.py b/src/agent_comms/envelope_claim_transitions.py index 610b0749c6..e64fb29ccc 100644 --- a/src/agent_comms/envelope_claim_transitions.py +++ b/src/agent_comms/envelope_claim_transitions.py @@ -257,7 +257,7 @@ def apply(self, previous: ClaimProjection) -> ClaimProjection: raise ClaimTransitionError("Only the exact owner incarnation/generation may release.") for resource in self.claims: if existing := next_state.get(resource): - raise ClaimConflict(existing) + existing.require_holder(self.owner, self.incarnation) for release in self.releases: del next_state[release.resource] for resource in self.claims: @@ -278,6 +278,10 @@ class ClaimOwner: def __post_init__(self) -> None: FieldCodec.require_fields(self) + def require_holder(self, owner: str, incarnation: str) -> None: + if (self.owner, self.incarnation) != (owner, incarnation): + raise ClaimConflict(self) + @dataclass(frozen=True, slots=True) class ClaimProjection(Mapping[str, ClaimOwner]): diff --git a/src/agent_comms/selected_claim.py b/src/agent_comms/selected_claim.py index ec0b4ab623..fe4cce202c 100644 --- a/src/agent_comms/selected_claim.py +++ b/src/agent_comms/selected_claim.py @@ -22,7 +22,6 @@ from .coordination_errors import StaleFence from .coordination_tables.participants import OwnerGenerations from .envelope_claim_transitions import ( - ClaimConflict, ClaimOwner, ExistingFileClaim, FileClaimPath, @@ -179,13 +178,9 @@ def publish_selected_resource_claim( with bus_lock.current_source(bus.log, metadata) as source: existing = source.claim(resource) if existing is not None: - if ( - existing.claim_scope == claim_scope - and existing.incarnation == str(owner.created_at) - and existing.owner == owner.name - ): + existing.require_holder(owner.name, str(owner.created_at)) + if existing.claim_scope == claim_scope: return existing - raise ClaimConflict(existing) if source.claim_operation_used(claim_scope.operation_id): raise IdentityConflict("Wake claim operation was already consumed") committed = bus.publisher._publish_claim_envelope_unlocked( @@ -198,15 +193,7 @@ def publish_selected_resource_claim( ) transition = committed.claim_transition assert transition is not None and transition.generation is not None - return ClaimOwner( - resource, - transition.owner, - transition.incarnation, - transition.generation, - committed.seq, - committed.message_id, - claim_scope, - ) + return transition.claimed(resource) @contextmanager From b79a779cfe657a267e7c9d9d0197a7bcf13e0c69 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:29:30 -0400 Subject: [PATCH 12/26] Complete reproduction formatting and type its process result queue --- tools/reproduce_thread_list.py | 113 ++++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 406a18b503..66d0f1524a 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -1,4 +1,5 @@ """Real scratch-bus sidebar observation concurrent with registry publication.""" + from __future__ import annotations import argparse @@ -20,7 +21,12 @@ def writer( - root: Path, stop: Event, ready: Event, result: Queue[int], imports: bool, dump_after: float, + root: Path, + stop: Event, + ready: Event, + result: Queue[int], + imports: bool, + dump_after: float, ) -> None: if dump_after: faulthandler.dump_traceback_later(dump_after) @@ -28,27 +34,40 @@ def writer( ready.set() count = 0 while not stop.is_set(): - name = f'writer-{count % 85}' + name = f"writer-{count % 85}" if imports and count < 85: - comms.threads.import_thread(ClaudeTranscript(root / 'source.jsonl'), name=name, - worktree=str(root), tags=frozenset({'proof'})) + comms.threads.import_thread( + ClaudeTranscript(root / "source.jsonl"), + name=name, + worktree=str(root), + tags=frozenset({"proof"}), + ) elif imports: - comms.registry.register(replace(comms.registry.require(name), title=f'write {count}'), - StoppedThreadStatus()) + comms.registry.register( + replace(comms.registry.require(name), title=f"write {count}"), StoppedThreadStatus() + ) else: - comms.registry.register(Thread(name, frozenset({'proof'}), str(root), - created_at=float(count % 85 + 1), title=f'write {count}', - storage_roots=StorageRoots(root / 'scratch' / name)), StoppedThreadStatus()) + comms.registry.register( + Thread( + name, + frozenset({"proof"}), + str(root), + created_at=float(count % 85 + 1), + title=f"write {count}", + storage_roots=StorageRoots(root / "scratch" / name), + ), + StoppedThreadStatus(), + ) count += 1 result.put(count) def main() -> None: parser = argparse.ArgumentParser() - parser.add_argument('root', type=Path) - parser.add_argument('--seconds', type=float, default=120) - parser.add_argument('--imports', action='store_true') - parser.add_argument('--dump-after', type=float, default=0) + parser.add_argument("root", type=Path) + parser.add_argument("--seconds", type=float, default=120) + parser.add_argument("--imports", action="store_true") + parser.add_argument("--dump-after", type=float, default=0) args = parser.parse_args() if args.dump_after: faulthandler.dump_traceback_later(args.dump_after) @@ -56,18 +75,42 @@ def main() -> None: root.mkdir(mode=0o700, parents=True, exist_ok=True) comms = Comms(root) comms.messaging.initialize_private_initial_protocol() - source = root / 'source.jsonl' - source.write_text('\n'.join(json.dumps({ - 'type': kind, 'uuid': str(index), 'timestamp': '2026-10-19T00:00:00Z', - 'cwd': str(root), 'sessionId': 'proof', 'promptId': 'p-1', 'origin': {'kind': 'human'}, - 'message': {'role': kind, 'content': [{'type': 'text', 'text': text}]}, - }) for index, (kind, text) in enumerate((('user', 'Scratch source input'), - ('assistant', 'Scratch source reply')))) + '\n') - comms.registry.register(Thread('sentinel', frozenset({'proof'}), str(root), - storage_roots=StorageRoots(root / 'scratch' / 'sentinel')), StoppedThreadStatus()) + source = root / "source.jsonl" + source.write_text( + "\n".join( + json.dumps( + { + "type": kind, + "uuid": str(index), + "timestamp": "2026-10-19T00:00:00Z", + "cwd": str(root), + "sessionId": "proof", + "promptId": "p-1", + "origin": {"kind": "human"}, + "message": {"role": kind, "content": [{"type": "text", "text": text}]}, + } + ) + for index, (kind, text) in enumerate( + (("user", "Scratch source input"), ("assistant", "Scratch source reply")) + ) + ) + + "\n" + ) + comms.registry.register( + Thread( + "sentinel", + frozenset({"proof"}), + str(root), + storage_roots=StorageRoots(root / "scratch" / "sentinel"), + ), + StoppedThreadStatus(), + ) reading = ObserveSidebar(str(root), True, False).observation() - stop, ready, counts = mp.Event(), mp.Event(), mp.Queue() - process = mp.Process(target=writer, args=(root, stop, ready, counts, args.imports, args.dump_after)) + stop, ready = mp.Event(), mp.Event() + counts: Queue[int] = mp.Queue() + process = mp.Process( + target=writer, args=(root, stop, ready, counts, args.imports, args.dump_after) + ) process.start() ready.wait(10) start = time.monotonic() @@ -79,22 +122,30 @@ def main() -> None: reads += 1 if not result.derived[1]: empty += 1 - print(f'EMPTY after {reads} reads: registry={len(comms.registry.snapshot().threads)}', flush=True) + print( + f"EMPTY after {reads} reads: " + f"registry={len(comms.registry.snapshot().threads)}", + flush=True, + ) elif isinstance(result, SidebarUnreadable): errors += 1 - print(f'UNREADABLE after {reads} reads: {result.error}', flush=True) + print(f"UNREADABLE after {reads} reads: {result.error}", flush=True) if not process.is_alive(): - raise RuntimeError(f'writer exited: {process.exitcode}') + raise RuntimeError(f"writer exited: {process.exitcode}") finally: stop.set() process.join(15) comms.views.transcript_reads.close() - print(f'elapsed={time.monotonic()-start:.3f}s reads={reads} empty={empty} errors={errors} writes={counts.get(timeout=2)} writer_exit={process.exitcode} imports={args.imports}', flush=True) + print( + f"elapsed={time.monotonic()-start:.3f}s reads={reads} empty={empty} errors={errors} " + f"writes={counts.get(timeout=2)} writer_exit={process.exitcode} imports={args.imports}", + flush=True, + ) if not reads: - raise RuntimeError('Sidebar observation produced no reads.') + raise RuntimeError("Sidebar observation produced no reads.") if (empty, errors, process.exitcode) != (0, 0, 0): - raise RuntimeError('Sidebar observation failed; see the original results above') + raise RuntimeError("Sidebar observation failed; see the original results above") -if __name__ == '__main__': +if __name__ == "__main__": main() From ff298af02edd2871214d241567256132c32fd653 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:31:42 -0400 Subject: [PATCH 13/26] Share scratch declarations and simplify the actual import fixture --- tools/reproduce_thread_list.py | 92 +++++++++++----------------------- 1 file changed, 30 insertions(+), 62 deletions(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 66d0f1524a..1b3ab2f66c 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -20,18 +20,20 @@ from agent_comms.ui_model.observation import ObserveSidebar, SidebarObserved, SidebarUnreadable -def writer( - root: Path, - stop: Event, - ready: Event, - result: Queue[int], - imports: bool, - dump_after: float, -) -> None: +def declaration(root: Path, name: str, created_at: float = 0) -> Thread: + return Thread( + name, + frozenset({"proof"}), + str(root), + created_at=created_at, + storage_roots=StorageRoots(root / "scratch" / name), + ) + + +def writer(root: Path, stop: Event, result: Queue[int], imports: bool, dump_after: float) -> None: if dump_after: faulthandler.dump_traceback_later(dump_after) comms = Comms(root) - ready.set() count = 0 while not stop.is_set(): name = f"writer-{count % 85}" @@ -42,22 +44,13 @@ def writer( worktree=str(root), tags=frozenset({"proof"}), ) - elif imports: - comms.registry.register( - replace(comms.registry.require(name), title=f"write {count}"), StoppedThreadStatus() - ) else: - comms.registry.register( - Thread( - name, - frozenset({"proof"}), - str(root), - created_at=float(count % 85 + 1), - title=f"write {count}", - storage_roots=StorageRoots(root / "scratch" / name), - ), - StoppedThreadStatus(), + thread = ( + comms.registry.require(name) + if imports + else declaration(root, name, float(count % 85 + 1)) ) + comms.registry.register(replace(thread, title=f"write {count}"), StoppedThreadStatus()) count += 1 result.put(count) @@ -75,44 +68,23 @@ def main() -> None: root.mkdir(mode=0o700, parents=True, exist_ok=True) comms = Comms(root) comms.messaging.initialize_private_initial_protocol() - source = root / "source.jsonl" - source.write_text( - "\n".join( - json.dumps( - { - "type": kind, - "uuid": str(index), - "timestamp": "2026-10-19T00:00:00Z", - "cwd": str(root), - "sessionId": "proof", - "promptId": "p-1", - "origin": {"kind": "human"}, - "message": {"role": kind, "content": [{"type": "text", "text": text}]}, - } - ) - for index, (kind, text) in enumerate( - (("user", "Scratch source input"), ("assistant", "Scratch source reply")) - ) - ) - + "\n" - ) - comms.registry.register( - Thread( - "sentinel", - frozenset({"proof"}), - str(root), - storage_roots=StorageRoots(root / "scratch" / "sentinel"), - ), - StoppedThreadStatus(), + event = dict( + timestamp="2026-10-19T00:00:00Z", + cwd=str(root), + sessionId="proof", + promptId="p-1", + origin=dict(kind="human"), ) + with (root / "source.jsonl").open("w") as stream: + for kind in ("user", "assistant"): + event.update(type=kind, uuid=kind, message=dict(role=kind, content=f"Scratch {kind}")) + print(json.dumps(event), file=stream) + comms.registry.register(declaration(root, "sentinel"), StoppedThreadStatus()) reading = ObserveSidebar(str(root), True, False).observation() - stop, ready = mp.Event(), mp.Event() + stop = mp.Event() counts: Queue[int] = mp.Queue() - process = mp.Process( - target=writer, args=(root, stop, ready, counts, args.imports, args.dump_after) - ) + process = mp.Process(target=writer, args=(root, stop, counts, args.imports, args.dump_after)) process.start() - ready.wait(10) start = time.monotonic() reads = empty = errors = 0 try: @@ -122,11 +94,7 @@ def main() -> None: reads += 1 if not result.derived[1]: empty += 1 - print( - f"EMPTY after {reads} reads: " - f"registry={len(comms.registry.snapshot().threads)}", - flush=True, - ) + print(f"EMPTY after {reads} reads", flush=True) elif isinstance(result, SidebarUnreadable): errors += 1 print(f"UNREADABLE after {reads} reads: {result.error}", flush=True) From 867978461717b85426bee8f9b86fa332978aff11 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:32:26 -0400 Subject: [PATCH 14/26] Keep reproduction failure reporting unified and parent-owned --- tools/reproduce_thread_list.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 1b3ab2f66c..6a3df55c7e 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -1,5 +1,3 @@ -"""Real scratch-bus sidebar observation concurrent with registry publication.""" - from __future__ import annotations import argparse @@ -30,9 +28,7 @@ def declaration(root: Path, name: str, created_at: float = 0) -> Thread: ) -def writer(root: Path, stop: Event, result: Queue[int], imports: bool, dump_after: float) -> None: - if dump_after: - faulthandler.dump_traceback_later(dump_after) +def writer(root: Path, stop: Event, result: Queue[int], imports: bool) -> None: comms = Comms(root) count = 0 while not stop.is_set(): @@ -83,7 +79,7 @@ def main() -> None: reading = ObserveSidebar(str(root), True, False).observation() stop = mp.Event() counts: Queue[int] = mp.Queue() - process = mp.Process(target=writer, args=(root, stop, counts, args.imports, args.dump_after)) + process = mp.Process(target=writer, args=(root, stop, counts, args.imports)) process.start() start = time.monotonic() reads = empty = errors = 0 @@ -109,9 +105,7 @@ def main() -> None: f"writes={counts.get(timeout=2)} writer_exit={process.exitcode} imports={args.imports}", flush=True, ) - if not reads: - raise RuntimeError("Sidebar observation produced no reads.") - if (empty, errors, process.exitcode) != (0, 0, 0): + if (reads > 0, empty, errors, process.exitcode) != (True, 0, 0, 0): raise RuntimeError("Sidebar observation failed; see the original results above") From d2f8cb5b49489df86c435f8caea70ebb62b9500b Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:32:59 -0400 Subject: [PATCH 15/26] Give every scratch declaration a recorded positive birth --- tools/reproduce_thread_list.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index 6a3df55c7e..ea1eb6bad2 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -18,7 +18,7 @@ from agent_comms.ui_model.observation import ObserveSidebar, SidebarObserved, SidebarUnreadable -def declaration(root: Path, name: str, created_at: float = 0) -> Thread: +def declaration(root: Path, name: str, created_at: float = 1) -> Thread: return Thread( name, frozenset({"proof"}), From ee5515ac4f2108a56ceeeb65e37bab0f8687fc53 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:33:24 -0400 Subject: [PATCH 16/26] Retain the actual Claude content-list schema in the compact fixture --- tools/reproduce_thread_list.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index ea1eb6bad2..c463ea1f67 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -73,7 +73,8 @@ def main() -> None: ) with (root / "source.jsonl").open("w") as stream: for kind in ("user", "assistant"): - event.update(type=kind, uuid=kind, message=dict(role=kind, content=f"Scratch {kind}")) + content = [dict(type="text", text=f"Scratch {kind}")] + event.update(type=kind, uuid=kind, message=dict(role=kind, content=content)) print(json.dumps(event), file=stream) comms.registry.register(declaration(root, "sentinel"), StoppedThreadStatus()) reading = ObserveSidebar(str(root), True, False).observation() From 88435ce658027424428252d59791874cf8384f77 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:36:33 -0400 Subject: [PATCH 17/26] Declare each Claude fixture frame together with its source text --- tools/reproduce_thread_list.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/reproduce_thread_list.py b/tools/reproduce_thread_list.py index c463ea1f67..eb013925d3 100644 --- a/tools/reproduce_thread_list.py +++ b/tools/reproduce_thread_list.py @@ -72,8 +72,8 @@ def main() -> None: origin=dict(kind="human"), ) with (root / "source.jsonl").open("w") as stream: - for kind in ("user", "assistant"): - content = [dict(type="text", text=f"Scratch {kind}")] + for kind, text in (("user", "Scratch input"), ("assistant", "Scratch reply")): + content = [dict(type="text", text=text)] event.update(type=kind, uuid=kind, message=dict(role=kind, content=content)) print(json.dumps(event), file=stream) comms.registry.register(declaration(root, "sentinel"), StoppedThreadStatus()) From 7226acdf73866286afc4d436326a330459b08983 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:36:45 -0400 Subject: [PATCH 18/26] P727 confirm holder conflict and stale-generation release fences --- tests/test_envelope_claim_transitions.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_envelope_claim_transitions.py b/tests/test_envelope_claim_transitions.py index 435fbf179c..6927c57214 100644 --- a/tests/test_envelope_claim_transitions.py +++ b/tests/test_envelope_claim_transitions.py @@ -95,6 +95,20 @@ def test_two_serialized_conflicting_entrants_have_one_winner_and_one_synchronous first[a] = None +def test_same_holder_moves_scope_but_another_holder_cannot(tmp_path): + _, resource, _ = resources(tmp_path) + held = transition(1, claims=(resource,), generation=G1).apply(ClaimProjection()) + moved = transition(2, claims=(resource,), generation=G2).apply(held) + assert held[resource].generation == G1 + assert moved[resource].generation == G2 + for changed in ({"owner": "other"}, {"incarnation": "new-birth"}): + with pytest.raises(ClaimConflict): + transition(3, claims=(resource,), generation=G1, **changed).apply(moved) + with pytest.raises(ClaimTransitionError, match="exact owner"): + transition(3, releases=(ClaimRelease(resource, G1),)).apply(moved) + assert dict(transition(3, releases=(ClaimRelease(resource, G2),)).apply(moved)) == {} + + def test_full_set_claim_and_release_are_all_or_nothing(tmp_path): _, a, b = resources(tmp_path) first = transition(1, claims=(b,), generation=G1).apply(ClaimProjection()) From c5e04b55a8d845833a7a2629cd817d0033be17fa Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:38:29 -0400 Subject: [PATCH 19/26] P261 name the current order message in replacement refusals --- src/agent_comms/todos.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 21893e014e..24e7ac3ee7 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -2053,8 +2053,13 @@ def sent_in(self, message_id: str) -> bool: return bool(self.messages) and self.messages[-1].message_id == message_id def require_replacement(self, message_id: str) -> None: + if not self.messages: + raise ValueError(f"{self.holder} holds {self.id}, but no order message was recorded.") if not self.sent_in(message_id): - raise ValueError(f"{self.holder} already holds {self.id}; pass --replaces with its current message.") + raise ValueError( + f"{self.holder} already holds {self.id}; " + f"pass --replaces {self.messages[-1].message_id}" + ) @classmethod def require_order_body(cls, body: str) -> None: From ef6def65a7cd296970c90d6c36fed65e8f4e7b8c Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:39:18 -0400 Subject: [PATCH 20/26] P727 type the claim boundary and distinguish operation lookup from holder identity --- src/agent_comms/selected_claim.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/selected_claim.py b/src/agent_comms/selected_claim.py index fe4cce202c..a34cc5772c 100644 --- a/src/agent_comms/selected_claim.py +++ b/src/agent_comms/selected_claim.py @@ -29,6 +29,10 @@ ) from .errors import RelationViolationError from .messages import ClaimNoticeKind +from .message_bus import MessageBus +from .publisher import Publisher +from .registry_document import RegistrySnapshot +from .wire_metadata import WireMetadata from .native_input_owner import RegistryOwner from .private_registry_guard import _require_no_private_owner_rename from .store_files import SharedLock, _store_lock @@ -124,7 +128,7 @@ def _committed_source( @contextmanager def _selected_claim_boundary( comms: Comms, store: Coordination, claim_scope: WakeClaim, owner_name: str -): +) -> Iterator[tuple[MessageBus[Publisher], CheckedWireStoreLock, WireMetadata, RegistrySnapshot, Thread]]: """One wire/bus/registry boundary for selected claim acquisition.""" if store.session.path.resolve() != (comms.root / "coordination.sqlite3").resolve(): raise IdentityConflict("Wake coordinator does not belong to this wire root") @@ -179,7 +183,8 @@ def publish_selected_resource_claim( existing = source.claim(resource) if existing is not None: existing.require_holder(owner.name, str(owner.created_at)) - if existing.claim_scope == claim_scope: + # Same-operation lookup; holder identity was already checked above. + if claim_scope == existing.claim_scope: return existing if source.claim_operation_used(claim_scope.operation_id): raise IdentityConflict("Wake claim operation was already consumed") From de1a682a00092fa41a91514e63893fe3ea688fdd Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:39:43 -0400 Subject: [PATCH 21/26] Preserve missing check counts through progress and overdue reporting Keep check validation strict while distance observations retain the original count state. A missing latest count names its check instead of aborting the fleet report. Src-Budget: 64 --- src/agent_comms/child_process.py | 38 +++++++++++++++++++++++---- src/agent_comms/goals.py | 18 ++++++------- src/agent_comms/todos.py | 45 +++++++++++++++++++++++--------- 3 files changed, 74 insertions(+), 27 deletions(-) diff --git a/src/agent_comms/child_process.py b/src/agent_comms/child_process.py index c327025401..12370fc36b 100644 --- a/src/agent_comms/child_process.py +++ b/src/agent_comms/child_process.py @@ -1024,15 +1024,34 @@ def decoded(stdout: bytes) -> CheckPrinted: def unproven(self, command: str, expected: int) -> tuple[str, ...]: ... @abstractmethod - def distance(self, expected: int) -> int: ... + def distance(self, expected: int) -> CheckPrinted: ... + + @abstractmethod + def plus(self, amount: int) -> CheckPrinted: ... + + @abstractmethod + def require(self, expected: object) -> None: ... + + @abstractmethod + def project[T](self, counted: Callable[[int], T], uncounted: Callable[[], T]) -> T: ... @dataclass(frozen=True) class CountPrinted(CheckPrinted): count: int - def distance(self, expected: int) -> int: - return abs(self.count - expected) + def distance(self, expected: int) -> CheckPrinted: + return CountPrinted(abs(self.count - expected)) + + def plus(self, amount: int) -> CheckPrinted: + return CountPrinted(self.count + amount) + + def require(self, expected: object) -> None: + if self.count: + raise ValueError(f"The order check is {self.count} away from {expected}.") + + def project[T](self, counted: Callable[[int], T], uncounted: Callable[[], T]) -> T: + return counted(self.count) def unproven(self, command: str, expected: int) -> tuple[str, ...]: return ((f"Check already prints {self.count} before any work: `{command}` proves nothing; " @@ -1041,8 +1060,17 @@ def unproven(self, command: str, expected: int) -> tuple[str, ...]: @dataclass(frozen=True) class NoIntegerPrinted(CheckPrinted): - def distance(self, expected: int) -> int: - raise ValueError(f"The order check printed no integer, so no count of {expected}.") + def distance(self, expected: int) -> CheckPrinted: + return self + + def plus(self, amount: int) -> CheckPrinted: + return self + + def require(self, expected: object) -> None: + raise ValueError(f"The order check printed no integer; cannot prove {expected}.") + + def project[T](self, counted: Callable[[int], T], uncounted: Callable[[], T]) -> T: + return uncounted() def unproven(self, command: str, expected: int) -> tuple[str, ...]: return ("the check printed no integer; use grep -c PATTERN FILE or ... | wc -l",) diff --git a/src/agent_comms/goals.py b/src/agent_comms/goals.py index 05055b3114..85653a9dcc 100644 --- a/src/agent_comms/goals.py +++ b/src/agent_comms/goals.py @@ -15,8 +15,8 @@ from pathlib import Path from typing import TYPE_CHECKING, ClassVar, Literal -from .child_process import (BoundedRun, ChildOutcome, ChildResult, ExitedOutcome, SignaledOutcome, - TimedOutOutcome) +from .child_process import (BoundedRun, CheckPrinted, ChildOutcome, ChildResult, CountPrinted, + ExitedOutcome, SignaledOutcome, TimedOutOutcome) from .declared_family import DeclaredFamily from .errors import RelationViolationError from .field_codec import ( @@ -230,19 +230,17 @@ def require_identity(self, goal_id: str) -> None: class CheckResult(DeclaredFamily, affix="CheckResult"): def require(self, result: ChildResult) -> None: - distance = self.distance(result) - if distance: - raise ValueError(f"The order check is {distance} away from {self}.") + self.distance(result).require(self) @abstractmethod - def distance(self, result: ChildResult) -> int: - """How far this run is from the result the order requires; zero when it is met.""" + def distance(self, result: ChildResult) -> CheckPrinted: + """The measured distance, or the run's declared missing-count state.""" @dataclass(frozen=True) class SilentCheckResult(CheckResult): - def distance(self, result: ChildResult) -> int: - return int(result.outcome != ExitedOutcome(0) or bool(result.stdout)) + def distance(self, result: ChildResult) -> CheckPrinted: + return CountPrinted(int(result.outcome != ExitedOutcome(0) or bool(result.stdout))) @dataclass(frozen=True) @@ -253,7 +251,7 @@ def __post_init__(self): if self.expected < 0: raise ValueError("An expected count must be nonnegative.") - def distance(self, result: ChildResult) -> int: + def distance(self, result: ChildResult) -> CheckPrinted: return result.outcome.printed_count(result.stdout).distance(self.expected) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 21893e014e..93fb16f8a8 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -22,7 +22,7 @@ from uuid import uuid4 from .change_report import FileLine -from .child_process import BoundedRun, ChildResult, ExitedOutcome +from .child_process import BoundedRun, CheckPrinted, ChildResult, ExitedOutcome from .command import Command from .declared_family import DeclaredFamily from .errors import ToolRefusedError @@ -258,7 +258,7 @@ def _clock(at: float) -> str: @dataclass(frozen=True) class DistanceReading: at: float - distance: int + distance: CheckPrinted @dataclass(frozen=True) @@ -266,18 +266,29 @@ class DistanceReadings: """The order's last two distance readings, oldest first: its base at the order, then its newest commit. Fewer than two read, or a last reading at done, project nothing.""" readings: tuple[DistanceReading, ...] = () + command: str = "" @property def trend(self) -> DistanceTrend: - """Velocity across the two readings: falling projects the finish, else it is flat.""" + """A missing count diagnoses the check; only measured counts project a finish.""" + if not self.readings: + return UnreadTrend() + return self.readings[-1].distance.project( + self.counted_trend, lambda: UncountedTrend(self.command)) + + def counted_trend(self, final: int) -> DistanceTrend: last = self.readings[-2:] - if len(last) < 2 or not last[1].distance: + if len(last) < 2 or not final: return UnreadTrend() - first, final = last - if final.distance >= first.distance: - return FlatTrend(final.distance, final.at) - rate = (first.distance - final.distance) / max(final.at - first.at, 1.0) - return MovingTrend(final.distance, final.at + final.distance / rate) + first, latest = last + + def compared(previous: int) -> DistanceTrend: + if final >= previous: + return FlatTrend(final, latest.at) + rate = (previous - final) / max(latest.at - first.at, 1.0) + return MovingTrend(final, latest.at + final / rate) + + return first.distance.project(compared, UnreadTrend) class DistanceTrend(DeclaredFamily, affix="Trend"): @@ -310,6 +321,15 @@ def projection(self) -> str: return f"; distance {self.distance}, not falling" +@dataclass(frozen=True) +class UncountedTrend(DistanceTrend): + command: str + + @property + def projection(self) -> str: + return f"; check prints no integer: {self.command}" + + @dataclass(frozen=True) class UnreadTrend(DistanceTrend): """Fewer than two readings, or the last one at done: nothing to project from.""" @@ -362,15 +382,16 @@ def readings(self) -> DistanceReadings: "--not", base).splitlines()))] points = [(base, self.since), *commits][-2:] return DistanceReadings( - tuple(DistanceReading(at, self.distance(base, sha)) for sha, at in points)) + tuple(DistanceReading(at, self.distance(base, sha)) for sha, at in points), + self.todo.order.check.command) - def distance(self, base: str, sha: str) -> int: + def distance(self, base: str, sha: str) -> CheckPrinted: """The order's Check run on ``sha`` against its N, plus its numbered edits the diff since ``base`` has not touched.""" check = self.todo.order.check touched = self.git("diff", "--name-only", base, sha).split() run = check.at(sha, str(self.checkout)) - return check.result.distance(run) + self.todo.order.untouched(touched) + return check.result.distance(run).plus(self.todo.order.untouched(touched)) @dataclass(frozen=True) From a5bf2c103361afc490c3d57fb69055801d9d8850 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:40:11 -0400 Subject: [PATCH 22/26] Verify P261 through the installed commander entrypoint on a private bus The five additional source lines distinguish an unrecorded send and print the retained current message without a lookup. Src-Budget: 5 --- tests/test_commander_problem_orders.py | 49 +++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 4db2cc46e9..02e4767b88 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -438,10 +438,23 @@ def test_commander_script_records_every_order_step_in_core(commander, commander_ 'worker', ('Set count.txt to 1.',), ('no second store', 'no cache')) numbered = body.read_text().replace('Problem: NEW', f'Problem: {created.id}') + # The refusal itself supplies the exact replacement argument; no DM lookup. + with pytest.raises(ValueError) as refusal: + script.send('worker', due, numbered) + expected = f"worker already holds P1; pass --replaces {created.messages[-1].message_id}" + assert str(refusal.value) == expected + replaces = str(refusal.value).split('--replaces ', 1)[1] + print(f'Refused order: {refusal.value}') # Without --clean this replaces the prior order and counts a send-back. - script.send('worker', due, numbered, replaces=created.messages[-1].message_id) + script.send('worker', due, numbered, replaces=replaces) sent_back = comms.goals.todos.get('P1') assert sent_back.order.id != created.order.id and script.struck(sent_back, 'worker') == 1 + print(f'Accepted replacement: {sent_back.order.id}') + # A historical send cannot replace the latest order. + with pytest.raises(ValueError, match=sent_back.messages[-1].message_id): + script.send('worker', due, numbered, replaces=replaces) + with pytest.raises(ValueError, match='no order message was recorded'): + replace(sent_back, messages=()).require_replacement(replaces) (link,) = sent_back.messages[-1:] sent = next(m.body for m in comms.views.history_page(CommandTarget.named('worker'), viewer='lead').messages if m.message_id == link.message_id) assert 'Set count.txt to 1.' in sent and 'Start now and push as you go.' in sent @@ -479,6 +492,40 @@ def problem(script, body): script.problem('The counter file holds zero where the fleet needs one', 'worker', '23:59', body, f'{__file__}:1') +def test_installed_order_entrypoint_refuses_then_accepts_printed_replacement( + commander, commander_script): + from agent_comms.rule_reviews import ( + AnsweredRuleReviewEntry, DrawnRuleReviewEntry, RuleReviews, + ) + + comms, _ = commander + script, repo = commander_script + (repo / 'count.txt').write_text('0\n') + body = Path(order_body(repo)) + problem(script, str(body)) + body.write_text(body.read_text().replace('Problem: NEW', 'Problem: P1')) + reviews = RuleReviews(comms.root) + reviews.write('lead', DrawnRuleReviewEntry(('Keep the current order message with its Todo.',), 0)) + reviews.write('lead', AnsweredRuleReviewEntry( + 1, 'P1 keeps the current message in its Todo and prints it when refusing replacement.')) + command = [str(Path(sys.executable).with_name('agent-comms-commander')), + 'order', 'send', 'worker', '23:59', str(body)] + env = {**os.environ, 'PYTHONPATH': str(Path(__file__).parents[1] / 'src'), + 'AGENT_COMMS_ROOT': str(comms.root)} + refused = subprocess.run(command, env=env, capture_output=True, text=True, timeout=30) + assert refused.returncode != 0 + current = comms.goals.todos.get('P1') + expected = f'worker already holds P1; pass --replaces {current.messages[-1].message_id}' + assert expected in refused.stderr + replacement = re.search(r'pass --replaces (\w+)', refused.stderr).group(1) + print(expected) + accepted = subprocess.run([*command, '--replaces', replacement], env=env, + capture_output=True, text=True, timeout=30) + assert accepted.returncode == 0, accepted.stderr + assert comms.goals.todos.get('P1').order.id != current.order.id + print(f'Accepted --replaces {replacement}: {accepted.stdout.strip()}') + + def test_resend_after_strike_counts_one_miss(commander, commander_script, monkeypatch): """A miss struck with `order strike` and then sent back is one strike; the next send-back strikes again.""" comms, _ = commander From 7d564932d03164a264693312d39a673fbaab2838 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:40:35 -0400 Subject: [PATCH 23/26] P727 wrap the typed boundary without adding a lint finding --- src/agent_comms/selected_claim.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/agent_comms/selected_claim.py b/src/agent_comms/selected_claim.py index a34cc5772c..ae7bde4946 100644 --- a/src/agent_comms/selected_claim.py +++ b/src/agent_comms/selected_claim.py @@ -128,7 +128,9 @@ def _committed_source( @contextmanager def _selected_claim_boundary( comms: Comms, store: Coordination, claim_scope: WakeClaim, owner_name: str -) -> Iterator[tuple[MessageBus[Publisher], CheckedWireStoreLock, WireMetadata, RegistrySnapshot, Thread]]: +) -> Iterator[ + tuple[MessageBus[Publisher], CheckedWireStoreLock, WireMetadata, RegistrySnapshot, Thread] +]: """One wire/bus/registry boundary for selected claim acquisition.""" if store.session.path.resolve() != (comms.root / "coordination.sqlite3").resolve(): raise IdentityConflict("Wake coordinator does not belong to this wire root") From 5f12ed9e96f402920795e206832f4064027474f5 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:42:47 -0400 Subject: [PATCH 24/26] Cover missing-count progress alongside unaffected overdue orders --- tests/test_commander_problem_orders.py | 2 +- tests/test_order_progress.py | 28 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 4db2cc46e9..bc6b80fbfa 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -620,7 +620,7 @@ def test_commander_refuses_a_check_that_prints_no_count(commander_script): @pytest.mark.parametrize('printed, refused', [ - ("printf 'no count here'", 'The order check printed no integer, so no count of 1.'), + ("printf 'no count here'", 'The order check printed no integer; cannot prove CountCheckResult(expected=1).'), ('echo 2', 'The order check is 1 away from CountCheckResult(expected=1).'), ('echo found 1', None), ]) diff --git a/tests/test_order_progress.py b/tests/test_order_progress.py index 9b42b53046..db9b899947 100644 --- a/tests/test_order_progress.py +++ b/tests/test_order_progress.py @@ -65,6 +65,34 @@ def test_no_commit_since_the_order_is_explicit(checkout): assert progress.line == 'no commit since the order' +def test_uncounted_check_keeps_other_overdue_orders_visible(commander, commander_script, checkout, monkeypatch): + from agent_comms.child_process import CountPrinted, NoIntegerPrinted + from agent_comms.todos import DistanceReading, UncountedTrend + + script, _ = commander_script + todo, unit = checkout + comms, _ = commander + monkeypatch.setattr(script.COMMS.registry, 'storage_rule', StorageRule(checkout_base=unit.parent)) + monkeypatch.setattr(PullRequests, 'read', lambda *args: ()) + broken = replace(todo.order, check=replace(todo.order.check, command="printf 'no count here'")) + with comms.goals.todos.writing() as db: + replace(todo, order=broken).upsert(db) + create(commander, 'P2', deadline=1) + commit(unit) + + progress = read((comms.goals.todos.get('P1'), unit)) + assert progress.line.endswith("; check prints no integer: printf 'no count here'") + lines = deadline_lines(script) + alarm = next(line for line in lines if line.startswith('OVERDUE:')) + assert "order order-P1" in alarm and "order order-P2" in alarm + assert "check prints no integer: printf 'no count here'" in alarm + assert 'distance 0' not in alarm and 'projected done' not in alarm + missing = NoIntegerPrinted() + assert missing.distance(1) is missing and missing.plus(3) is missing + assert DistanceReadings((DistanceReading(1, missing),), 'printf missing').trend == UncountedTrend('printf missing') + assert DistanceReadings((DistanceReading(1, missing), DistanceReading(2, CountPrinted(2)))).trend.projection == '' + + def test_an_unpushed_commit_names_its_sha_and_time(checkout): sha = commit(checkout[1]) progress = read(checkout) From 28960d3f49f9c1ff23955bee9c419f719b3e5233 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:52:50 -0400 Subject: [PATCH 25/26] Retain the original check command on each distance observation --- src/agent_comms/todos.py | 14 +++++++------- tests/test_order_progress.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index ac85650989..b31cf455b1 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -262,6 +262,7 @@ def _clock(at: float) -> str: class DistanceReading: at: float distance: CheckPrinted + command: str = "" @dataclass(frozen=True) @@ -269,7 +270,6 @@ class DistanceReadings: """The order's last two distance readings, oldest first: its base at the order, then its newest commit. Fewer than two read, or a last reading at done, project nothing.""" readings: tuple[DistanceReading, ...] = () - command: str = "" @property def trend(self) -> DistanceTrend: @@ -277,7 +277,7 @@ def trend(self) -> DistanceTrend: if not self.readings: return UnreadTrend() return self.readings[-1].distance.project( - self.counted_trend, lambda: UncountedTrend(self.command)) + self.counted_trend, lambda: UncountedTrend(self.readings[-1].command)) def counted_trend(self, final: int) -> DistanceTrend: last = self.readings[-2:] @@ -389,8 +389,7 @@ def readings(self, source: ProgressSource) -> DistanceReadings: "--not", base).splitlines()))] points = [(base, source.since), *commits][-2:] return DistanceReadings( - tuple(DistanceReading(at, source.distance(self, base, sha)) for sha, at in points), - source.todo.order.check.command) + tuple(source.reading(self, base, sha, at) for sha, at in points)) @dataclass(frozen=True) @@ -421,12 +420,13 @@ def commits(self, *excluded: str) -> Iterator[tuple[str, float]]: def readings(self) -> DistanceReadings: return self.checkout.readings(self) - def distance(self, checkout: PresentCheckout, base: str, sha: str) -> CheckPrinted: - """Run the order's check and preserve its count state beside untouched edits.""" + def reading(self, checkout: PresentCheckout, base: str, sha: str, at: float) -> DistanceReading: + """Keep the check command with its measured or missing distance.""" check = self.todo.order.check touched = checkout.git("diff", "--name-only", base, sha).split() run = check.at(sha, str(checkout.path)) - return check.result.distance(run).plus(self.todo.order.untouched(touched)) + return DistanceReading(at, check.result.distance(run).plus( + self.todo.order.untouched(touched)), check.command) @dataclass(frozen=True) diff --git a/tests/test_order_progress.py b/tests/test_order_progress.py index ee1eddce87..b676797076 100644 --- a/tests/test_order_progress.py +++ b/tests/test_order_progress.py @@ -90,7 +90,7 @@ def test_uncounted_check_keeps_other_overdue_orders_visible(commander, commander assert 'distance 0' not in alarm and 'projected done' not in alarm missing = NoIntegerPrinted() assert missing.distance(1) is missing and missing.plus(3) is missing - assert DistanceReadings((DistanceReading(1, missing),), 'printf missing').trend == UncountedTrend('printf missing') + assert DistanceReadings((DistanceReading(1, missing, 'printf missing'),)).trend == UncountedTrend('printf missing') assert DistanceReadings((DistanceReading(1, missing), DistanceReading(2, CountPrinted(2)))).trend.projection == '' From 1a044998771dfb82869d1d7a3a1b0fe093b43375 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Mon, 19 Oct 2026 16:59:22 -0400 Subject: [PATCH 26/26] Keep proof refusal distinct from non-throwing distance observations --- src/agent_comms/child_process.py | 2 +- tests/test_commander_problem_orders.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/child_process.py b/src/agent_comms/child_process.py index 12370fc36b..f12253f903 100644 --- a/src/agent_comms/child_process.py +++ b/src/agent_comms/child_process.py @@ -1067,7 +1067,7 @@ def plus(self, amount: int) -> CheckPrinted: return self def require(self, expected: object) -> None: - raise ValueError(f"The order check printed no integer; cannot prove {expected}.") + raise ValueError(f"The order check cannot prove {expected}: it printed no integer.") def project[T](self, counted: Callable[[int], T], uncounted: Callable[[], T]) -> T: return uncounted() diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index bb6a80f282..f98a2d39f4 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -659,7 +659,7 @@ def test_commander_refuses_a_check_that_prints_no_count(commander_script): @pytest.mark.parametrize('printed, refused', [ - ("printf 'no count here'", 'The order check printed no integer; cannot prove CountCheckResult(expected=1).'), + ("printf 'no count here'", 'The order check cannot prove CountCheckResult(expected=1): it printed no integer.'), ('echo 2', 'The order check is 1 away from CountCheckResult(expected=1).'), ('echo found 1', None), ])