Skip to content

Use declared disk scratch for writer-store rehearsal - #3599

Merged
trissim merged 4 commits into
mainfrom
work/install-speed--writer-scratch
Oct 29, 2026
Merged

trissim merged 4 commits into
mainfrom
work/install-speed--writer-scratch

Conversation

@trissim

@trissim trissim commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

P938 recovery: 4 commits, 6 files, +36/-26. Two selected-store rehearsal loops → one. Current head: 4bd9bce04182c19cab6102ce6c23d5bee86128f2, rebased onto main 507cf78cf. Since rehearsed 5ee6d84: 86b415d deletes only the unused LiveFilesRequest import; 4bd9bce corrects the existing bridge fixture without weakening its assertions.

Both planned reconciliation and post-carry validation now use ExternalStore.practice_openings(files, *, scratch, stores). The copied-root carry gets scratch configuration from the original root through FilesRequest.source_root(), not from an incomplete practice copy or the worker's temporary HOME.

Source delivery only. No author tests, ratchet or live install. Root's rehearsal at 5ee6d84 completes the previously failing planned-reconcile in 8.291 s, but pending=[] means the post-carry probe was not exercised. Root's combined selection was 8 passed, 1 failed: the refusal fixture used a deliberately retired table. That fixture is now corrected to the commander's specified current-table refusal and retired-table acceptance cases, and root's exact-head rerun at 4bd9bce passes both cases in 6.00 s. Full current-head suite and installed acceptance are not claimed. P935 remains open and stopped.

Failure, including the first incomplete repair

The failed install copied:

/home/ts/.local/state/claude-comms-inbox/claude-code/delivery.sqlite3
→ /tmp/tmp00k9hrbz/writer-owned/bridge_delivery/.local/state/claude-comms-inbox/claude-code/delivery.sqlite3
[Errno 122] Disk quota exceeded

My first commit supplied configured scratch to ExternalStore.practice_openings, but missed the same operation in WriterOwnedOpeningProbe.run: another bare TemporaryDirectory and another selected-store practice loop. That would send the same database back into quota-limited tmpfs after reconciliation passed. The second commit deletes that duplicate rather than supplying it another independent directory policy.

A second ownership correction matters during rehearsal: PreparedMigration.root is the copied root. practice.select need not copy storage.json, so loading StorageRule there could silently select default scratch. The final producer uses StorageRule.load(self.files.source_root()).scratch_base. LiveFilesRequest and RetainedCopyRequest already own those original-root answers.

Before / after ownership

Before:

PlannedReconcile.answer(root)
  → ExternalStore.practice_openings
    → temporary-directory choice + selected-store practice loop

PreparedMigration.carry
  → WriterOwnedOpeningProbe.run
    → another temporary-directory choice + another selected-store practice loop

After:

PlannedReconcile.answer(root)
  → LiveFilesRequest(root), family minus pending-carry stores,
    StorageRule.load(root).scratch_base
                                      \
                           ExternalStore.practice_openings
                            → PrivateDirectoryRole.make_parents
                            → TemporaryDirectory(dir=scratch)
                            → one store.practice_opening loop
                                      /
PreparedMigration.carry
  → WriterOwnedOpeningProbe(files, stores,
      scratch=StorageRule.load(files.source_root()).scratch_base)

The probe retains only resource-path reporting after shared validation. Its TemporaryDirectory import and seven-line directory/validation/result-building block are removed. File selection belongs to callers; directory lifetime and validation iteration belong to ExternalStore. The successor commit also deletes typed_table's now-unused LiveFilesRequest import: whole-tree search finds all remaining uses importing its actual locked_store owner, not this incidental re-export.

Complete caller and producer closure

Production

  • cli_verbs/planned_reconcile.py: derives the same family-minus-carry exclusion set as before, supplies LiveFilesRequest(root), selected stores and the actual root's StorageRule scratch.
  • typed_table.py: requires files, scratch and stores; creates missing ancestors through existing PrivateDirectoryRole policy, owns the only TemporaryDirectory and selected-store practice loop. It does not import StorageRule.
  • installer/probes/carry.py: WriterOwnedOpeningProbe carries required scratch: Annotated[Path, PathText] and delegates validation to that owner, passing its unchanged live/copied FilesRequest and stores. Returned resource paths retain store/resource order.
  • installer/upgrade.py: the sole production probe constructor resolves configuration through self.files.source_root(), preserving original configuration even when the retained copy lacks storage.json. It passes the value across the existing target-worker transport.

Fixtures and wire

All five explicit probe constructors are migrated: one in upgrade.py, three in test_writer_owned_stores.py and one in test_reconcile_stores.py. Fixture constructors supply their owned scratch path. Existing live/copied request codec-roundtrip controls now include the required scratch field through their existing whole-record equality.

WriterOwnedOpeningProbe inherits TargetProbe (interpreter_operation.py:465): only the reviewed target runtime decodes it; outgoing_wire() returns empty. This new target-only PathText field needs no old-runtime reader, compatibility alias or fallback.

Unchanged: WriterOwnedStore.practice_opening's copying and schema install/readability checks; file resources; exclusion meaning; primary/committed WAL behavior; original durable files; exception propagation and context-manager cleanup. No process-wide tempfile setting, new environment variable, scratch constant, quota change or data deletion.

Existing bridge control corrected at its fixture

BridgeStateTable.retired_tables at shared.py:130 deliberately retires observed_part. DeliveredBoardContent at shared.py:134 declares board_delivery(content, since: float, delivered_at: float). The refusal branch now creates that current table pre-STRICT, with since TEXT holding not-a-time. The accepted branch installs actual schema and adds the retired observed_part table and row. All refusal, original/copied path and byte-equality assertions remain unchanged; no fallback table, production change or weakened check. It also removes the old setup's needless create-wrong-store/unlink/recreate detour.

Source evidence

The initial refactor-audit Package.load map at f701b1c parsed 1,168 tracked Python files under src/tests/tools, zero omissions. It retained 50 related modules. The expanded semantic-family map at the rejected 74d47e3 again parsed 1,168 files, zero omissions, retaining 39 related modules including both loops, the carry producer, FilesRequest and fixture consumers. Retained maps: p938-source-before.json, p938-family-before.json under /home/ts/.cache/agent-scratch/install-speed.

The rebase brings in #3598's review-gate change, not another scratch implementation. Targeted applied-source reproof covers all six changed modules and records every probe constructor and practice call in p938-family-after.json. Whole-tree search finds two production calls to the shared plural operation and one production call to the singular store operation, inside that owner's loop. No TemporaryDirectory remains in installer/probes/carry.py. This is static source evidence, not application acceptance; dynamic plugin behavior is not certified by AST.

  • IMPL-12: fact = directory lifetime and selected-store practice procedure; at = old carry.py loop beside ExternalStore's loop; derive = existing ExternalStore.practice_openings; by = deleting the probe's copy and routing both callers through the owner.
  • IDEN-5: fact = configured scratch root; at = former ambient tempfile choices; derive = selected root's StorageRule; by = required scratch argument and TemporaryDirectory.dir.
  • IDEN-6: fact = configuration's original root; at = a load through copied PreparedMigration.root; derive = FilesRequest.source_root; by = the existing live/retained request polymorphism, with no new root field or guess.

Root-run receipts, bounded to the head and branch actually exercised

At 5ee6d84, root ran the real native --rehearse path. I read /home/ts/..agent-comms.installer-e45ab41e9c7d43179d9746af00415b05/result.json: target matches, result.kind=rehearsed, rehearsal.kind=uncopied, pending=[], rewritten=0, inputs=[], copied bytes=0. The exact --root /home/ts/.agent-comms planned-reconcile --carrying has 8.291418 s caller elapsed and 7.066325 s nested body. They are nested, not additive. No copy/carry or WriterOwnedOpeningProbe ran. This proves the failed planning step now completes; it does not prove the unexercised carry branch or an installation.

Root's combined control log /home/ts/.cache/agent-scratch/codex-greentext/p938-carry-controls.log records 8 passed, 1 failed in 7.88 s. The former false refusal case created observed_part; BridgeStateTable.retired_tables intentionally retires it, so planned-reconcile correctly returned success. claude-code traced the declaration, codex-greentext ordered the replacement, and 4bd9bce implements it. Root reran only the corrected bridge test, both parameters, at committed 4bd9bce: 2 passed in 6.00 s, from /home/ts/.cache/agent-scratch/codex-greentext/p938-bridge-control.log, read in full. The malformed current table is refused; valid schema containing retired-table data is accepted, with existing path and byte assertions intact. No fallback was needed. The earlier eight passing results belong to 5ee; this focused pair qualifies the changed fixture. These counts overlap and are not summed.

Root-owned remaining acceptance — no author-run receipts

1. Independent real planned-reconcile directory/cleanup check

With reviewed source and configured dependencies, Python's existing audit event observes real allocation without patching our code or overriding tempfile. The failed command had an empty --carrying list, matching this command's defaults.

PYTHONPATH=src timeout -s INT 300 nice -n 19 \
  /home/ts/code/projects/agent-comms/.venv/bin/python - <<'PY'
import sys
from pathlib import Path
from agent_comms.cli_verbs.planned_reconcile import PlannedReconcileCliCommand
from agent_comms.storage_rules import StorageRule
root = Path('/home/ts/.agent-comms')
scratch = StorageRule.load(root).scratch_base
allocated = []
def observe(event, arguments):
    if event == 'tempfile.mkdtemp':
        allocated.append(Path(arguments[0]))
sys.addaudithook(observe)
answer = PlannedReconcileCliCommand().answer(root)
assert allocated
assert all(path.parent == scratch for path in allocated), (scratch, allocated)
assert all(not path.exists() for path in allocated), allocated
print('selected scratch:', scratch, 'removed practice directories:', allocated)
print(answer)
PY

2. Copied-store controls already run; actual live upgrade still owed

The original-root configuration selection in PreparedMigration.carry is source-reviewed, not claimed from the uncopied native rehearsal. It uses the existing FilesRequest.source_root for both request members; no copied storage.json or worker-HOME inference remains. The combined copied-store/WAL/worker checks and corrected bridge pair cover the real retained-file validation, returned paths, refusal and original-byte behavior on fixture-owned stores.

The already-run controls are listed for reproducibility, not a request for another reassurance run:

tests/test_reconcile_stores.py::test_planned_reconcile_refuses_a_bridge_delivery_store_its_declaration_cannot_open
tests/test_writer_owned_stores.py::test_writer_owned_carry_copies_migrates_and_restores_original_bytes
tests/test_writer_owned_stores.py::test_writer_owned_carry_preserves_its_declaration_across_cli_and_worker_boundaries
tests/test_writer_owned_stores.py::test_writer_owned_carry_opens_valid_targets_without_writing_the_source
tests/test_writer_owned_stores.py::test_writer_owned_carry_refuses_an_undecodable_target_with_its_rowid
tests/test_writer_owned_stores.py::test_writer_owned_backup_preserves_primary_and_committed_wal_bytes

Root ran under its test lease using the configured interpreter and fixture-owned stores. No author run was made. Legitimate concurrent changes to live databases must not be mistaken for rehearsal writes. Root's next acceptance step is the actual native upgrade after source verdict and merge, as ordered; no additional local gate is introduced here.

Preserved work and assumptions

The parser correction is committed/pushed on #3596 at 300e2aa, with no stash/discard. That PR explicitly limits its older measurement/worker qualification to 98d8. P935 checker-site implementation remains incomplete and stopped; this repair does not complete it.

The same checkout was reused; no new worktree, venv or build. No author tests/app runs, local ratchets, user-data deletion or UNKNOWN-input replay. Open overlapping source PRs remain #3576 (planned reconciliation) and #3580/#3566/#3538 (typed_table); keep this shared operation and source-root selection if larger moves land.

Assumed: root reviews this exact new head, runs both real routes and retries the failed native upgrade. A passed copy is not an installed build; neither state is claimed here.

fixed the tmpfs database copy
its sequel was hiding in carry.py
deleted the sequel, kept the one owner

@trissim
trissim marked this pull request as ready for review October 29, 2026 08:58
Copilot AI balanced review requested due to automatic review settings October 29, 2026 08:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim force-pushed the work/install-speed--writer-scratch branch from 74d47e3 to 5ee6d84 Compare October 29, 2026 09:09
@github-actions

Copy link
Copy Markdown

debt-src at 918681e, every finding:

grew ClientTypeErrors:batrachian-toad/src/toad/widgets/sidebar_tree.py +1: this change removes or changes an agent_comms name that client file uses; run mypy on it to see which

@trissim
trissim enabled auto-merge October 29, 2026 09:27
@trissim
trissim added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit c81fbcd Oct 29, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants