From a580b4f1fa6eb2558e2448f6f1e8c3a924fc440c Mon Sep 17 00:00:00 2001 From: Citation fixture Date: Fri, 23 Oct 2026 10:09:48 -0400 Subject: [PATCH 1/2] Lock a copied file only when its writers lock it The rehearsal copy took a shared store lock on every root file it copied, through the base RootFile.copy_group. Taking the lock creates the lock inode (store_files.py _store_lock_file), so a root holding 118,271 plain selected-tool ledger files came out of PracticeBus.select with 118,271 `.NAME.lock` files beside them, and the ledger carry counted each one as a slot (236,542). Whether a file has a store lock is a fact of its owner. The base copy_group now copies without one; the new LockedFile capability carries the shared lock for the owners whose writers take it: the LockedStore family and HistoryArchive, which locks history_sources.json itself. SQLiteFile and RegistryFile keep their own copies. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/historical_views.py | 4 ++-- src/agent_comms/locked_store.py | 19 +++++++++++++++---- tests/test_thread_catalog_files.py | 26 ++++++++++++++++++++++++-- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/agent_comms/historical_views.py b/src/agent_comms/historical_views.py index cf07533b34..5e6e3a796a 100644 --- a/src/agent_comms/historical_views.py +++ b/src/agent_comms/historical_views.py @@ -18,7 +18,7 @@ from .coordination_errors import HistoricalSourceChanged from .errors import UnregisteredThreadError from .field_codec import FieldCodec -from .locked_store import DurableFile, RegistryFile, RootFile +from .locked_store import DurableFile, LockedFile, RegistryFile, RootFile from .message_page import ( NEWEST_PAGE, ConversationMessagePageRequest, @@ -398,7 +398,7 @@ class HistoricalThread: thread: ThreadProvenance -class HistoryArchive(DurableFile): +class HistoryArchive(DurableFile, LockedFile): """Ordered immutable source snapshots; never a delivery/launch authority.""" filename = "history_sources.json" diff --git a/src/agent_comms/locked_store.py b/src/agent_comms/locked_store.py index d6076ac4af..49cd19f371 100644 --- a/src/agent_comms/locked_store.py +++ b/src/agent_comms/locked_store.py @@ -116,9 +116,9 @@ def copy_file(source: str | Path, target: str | Path) -> Path: @classmethod def copy_group(cls, source: Path, target: Path) -> frozenset[Path]: - """Copy this owner's consistent file cut and name the source files included.""" - with _store_lock(source, mode=SharedLock()): - cls.copy_file(source, target) + """Copy this owner's file cut and name the source files included. A file no writer + locks is copied as it is: locking it would create a lock inode beside it.""" + cls.copy_file(source, target) return frozenset((source,)) @staticmethod @@ -319,6 +319,17 @@ class UndeclaredFile(RootFile): path: Path +class LockedFile(RootFile): + """A declared file whose writers hold its store lock, so its copy shares that lock.""" + + __slots__ = () + + @classmethod + def copy_group(cls, source: Path, target: Path) -> frozenset[Path]: + with _store_lock(source, mode=SharedLock()): + return super().copy_group(source, target) + + class FileStore(RootFile, ABC): """Construction belongs only to owners whose initializer takes a file.""" @@ -672,7 +683,7 @@ def publish_change(cls, path: Path) -> None: @dataclass(frozen=True, slots=True) -class LockedStore(FileStore, StoreLockOwner, JsonDocument[T], declared_name=None): +class LockedStore(FileStore, LockedFile, StoreLockOwner, JsonDocument[T], declared_name=None): """Ordered document transactions; callbacks never mutate or reenter.""" path: Path diff --git a/tests/test_thread_catalog_files.py b/tests/test_thread_catalog_files.py index 2630f12594..34c1b71403 100644 --- a/tests/test_thread_catalog_files.py +++ b/tests/test_thread_catalog_files.py @@ -2,7 +2,8 @@ import pytest -from agent_comms.locked_store import RootFile +from agent_comms.locked_store import LockedFile, RootFile +from agent_comms.read_ledger import ReadLedger from agent_comms.store_files import store_lock_path from agent_comms.threads import ThreadCatalog from cutover.carry_lock_chains import LockChains @@ -20,11 +21,32 @@ def test_session_locks_are_undeclared_and_never_locked(tmp_path): lock = store_lock_path(catalog) lock.touch() assert ThreadCatalog.matching_in(tmp_path) == frozenset((catalog,)) + RootFile.copy_group(lock, tmp_path / "copy") with pytest.raises(ValueError, match="never itself a lockable store"): - RootFile.copy_group(lock, tmp_path / "copy") + LockedFile.copy_group(lock, tmp_path / "locked-copy") assert not store_lock_path(lock).exists() +def test_copy_locks_only_files_their_writers_lock(tmp_path): + """A copy of a root creates a store lock only beside a store whose writers take it.""" + root, copy = tmp_path / "root", tmp_path / "copy" + ledger = root / "imported_sessions" / "selected-tool-ledger" + ledger.mkdir(parents=True) + plain = tuple(ledger / f"slot-{index}" for index in range(3)) + for path in plain: + path.write_text("{}") + store = ReadLedger.path_in(root) + store.write_text("{}") + for resource, owner in RootFile.owned_files(root, (*plain, store)).items(): + target = copy / resource.path + target.parent.mkdir(parents=True, exist_ok=True) + owner.copy_group(root / resource.path, target) + assert not any(ledger.glob(".*.lock")) + assert store_lock_path(store).exists() + assert sorted(path.name for path in (copy / ledger.relative_to(root)).iterdir()) == [ + path.name for path in plain] + + def test_lock_chains_are_deleted_and_store_locks_kept(tmp_path): lock = store_lock_path(_catalog(tmp_path)) chain = [lock] From e1d5b7bcff20b1377970cd6c5d291c68929dd0b3 Mon Sep 17 00:00:00 2001 From: Citation fixture Date: Fri, 23 Oct 2026 10:14:01 -0400 Subject: [PATCH 2/2] Make the store lock owner and its locked copy one capability Review of a580b4f1f: "this file's writers hold its store lock" was stated twice. LockedStore listed both StoreLockOwner and LockedFile, and HistoryArchive took a raw _store_lock(self.path) beside its LockedFile parent, so a new StoreLockOwner could forget LockedFile and copy unlocked. LockedFile now is the store lock owner on the file side: LockedFile(RootFile, StoreLockOwner) is StoreLockOwner's only direct child, LockedStore lists only LockedFile, and HistoryArchive writes under its inherited locked(), with its place first in STORE_LOCK_ORDER (attach reads the source registry inside it). RootFile precedes StoreLockOwner because FileStore(RootFile, ABC) and DeclaredFamily's ABC otherwise have no consistent MRO; the only name both sides define is __init_subclass__, and RootFile's calls super. Co-Authored-By: Claude Opus 5.5 --- src/agent_comms/historical_views.py | 4 ++-- src/agent_comms/locked_store.py | 4 ++-- src/agent_comms/store_order.py | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/agent_comms/historical_views.py b/src/agent_comms/historical_views.py index 5e6e3a796a..17c0e7fde2 100644 --- a/src/agent_comms/historical_views.py +++ b/src/agent_comms/historical_views.py @@ -47,7 +47,7 @@ from .registration import Registration from .registry_provenance import RegistryProvenance from .revision_views import BusSourceView -from .store_files import AtomicFile, _store_lock +from .store_files import AtomicFile from .thread_identity import UnrecordedThreadIncarnation from .threads import ThreadProvenance @@ -469,7 +469,7 @@ def attach(self, source_root: Path, *, source_read: HistorySource | None = None) source_root = source_root.resolve() if source_root == self.bus_path.parent.resolve(): raise ValueError("The live bus cannot be its own history source") - with _store_lock(self.path): + with self.locked(): sources = self.sources() existing = next((s for s in sources if s.original_root == str(source_root)), None) if existing is not None: diff --git a/src/agent_comms/locked_store.py b/src/agent_comms/locked_store.py index 49cd19f371..77f8c65660 100644 --- a/src/agent_comms/locked_store.py +++ b/src/agent_comms/locked_store.py @@ -319,7 +319,7 @@ class UndeclaredFile(RootFile): path: Path -class LockedFile(RootFile): +class LockedFile(RootFile, StoreLockOwner, declared_name=None): """A declared file whose writers hold its store lock, so its copy shares that lock.""" __slots__ = () @@ -683,7 +683,7 @@ def publish_change(cls, path: Path) -> None: @dataclass(frozen=True, slots=True) -class LockedStore(FileStore, LockedFile, StoreLockOwner, JsonDocument[T], declared_name=None): +class LockedStore(FileStore, LockedFile, JsonDocument[T], declared_name=None): """Ordered document transactions; callbacks never mutate or reenter.""" path: Path diff --git a/src/agent_comms/store_order.py b/src/agent_comms/store_order.py index 02c5e4d500..b1e3ba8918 100644 --- a/src/agent_comms/store_order.py +++ b/src/agent_comms/store_order.py @@ -1,4 +1,5 @@ """One complete acquisition order, loaded before store instances open.""" +from .historical_views import HistoryArchive from .registry_store import RegistryStore from .catalog_store import ChannelCatalog from .read_ledger import ReadLedger @@ -13,7 +14,7 @@ from .store_files import StoreLockOwner STORE_LOCK_ORDER = ( - RegistryStore, ChannelCatalog, ReadLedger, + HistoryArchive, RegistryStore, ChannelCatalog, ReadLedger, RelationshipStore, RuntimeInfoStore, SharedLedger, CollaborationLedger, ConnectorStore, RuntimeChildrenStore, RootRuntimeStore, ServicePassStore, )