Skip to content

Remove residual bus-file locks and repair shared boundary behavior - #2622

Closed
trissim wants to merge 6 commits into
integration/bus-current-mainfrom
next/bus-deletion
Closed

trissim wants to merge 6 commits into
integration/bus-current-mainfrom
next/bus-deletion

Conversation

@trissim

@trissim trissim commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

34 lines removed; 221 added across source, controls and the updated plan. The five obsolete bus modules were already absent at base acb2ddd0e. This removes their three remaining hand-spelled lock users and fixes the deletion evidence that both matched its own declaration and missed .with_name("wire").

Before/after ownership:

  • NativeReplyPreparation.finished: remove the physical wire lock. The existing local state lock owns retirement; source revision checks and the SQLite write transaction own publication. A busy SQLite writer retains completed readers for the next observation.
  • SendRefusal.record: replace the wire lock plus an unlocked permission check with existing MaintenanceBarrier.allow_input, holding registry permission through the existing SQL transaction.
  • CollaborationLedger.merge: replace the wire lock with the registered-author snapshot through the existing document update. Registry-before-ledger follows the declared store lock order.
  • BusDeletionPhase: retain the absent-module evidence and use BusStorageAbsent over existing MainTree parsed committed source. Controls cover real symbols, filenames and three path spellings, while allowing comments/docstrings describing deletion. This is known-syntax screening; dynamic paths and aliases remain source-review obligations.

Two independently extractable boundary repairs are included:

  • 3a2b4c30c narrows ChangeSignal._on_ready to its actual native read/decode failures (OSError, ValueError). Failures already reached waiting readers; this does not claim the old handler swallowed them. 29d106eea exercises a real failed native descriptor and proves a later normal wake does not replace that failure.
  • 55edba7a4 fixes the actual ledger CLI failing in json.dump on nested LedgerValue objects. SingleValueEncoding owns the existing typed single-field encoding, inherited by SingleFieldValue and JsonShapeFamily; the raw return self.value implementation is deleted. Distinct shape decoding/schema and CLI argument behavior remain on their original owners. The existing triage fixture used uppercase SQL text despite the strict lowercase decoder already present at base; it now checks lowercase round-trip and uppercase refusal.

Lock deletion is 9215d556a; phase evidence and plan are 85643b97b. Documentation correction 194957ee5 labels the removed NativeSourceCursor incident as historical and names the current CoordinatedTurn → CursorDelivery → CursorOwner → coordinator transaction path with bounded StoreWait contention. This final commit changes prose only; the behavior checks were not repeated. All commits target integration/bus-current-main.

Source evidence: NRA original-ClassDef census and canonical inheritance projection preceded edits. Bus ownership/consumer corpus: 953 Core production/test modules, zero parse omissions. Codec corpus includes paired Toad production/tests: 1,505 modules, 6,766 original classes before and 6,768 after; 101 selected family classes become 102 with the shared encoding capability. Original unprojected declarations remain in the evidence. Lexical consumer leads and nominal projection do not prove dynamic receiver bindings or behavior. This addresses the concrete MEMB-3 repeated physical lock-path mechanism and the codec bypass of its existing typed encoding owner.

Validation, using the existing Python 3.14 environment and this checkout's source:

PYTHONPATH=$PWD/src TMPDIR=/home/ts/.cache/agent-scratch timeout 90 /home/ts/code/projects/agent-comms/.venv/bin/python -m pytest -n 0 tests/test_json_shape_field_codec.py tests/test_field_codec.py tests/test_acp_failure.py tests/test_cli.py::TestCliSuccess::test_ledger_read_and_merge -q --tb=short
# 107 passed, 1.18s

PYTHONPATH=$PWD/src TMPDIR=/home/ts/.cache/agent-scratch timeout 90 /home/ts/code/projects/agent-comms/.venv/bin/python -m pytest -n 0 tests/test_thread_unread.py tests/test_operations.py tests/test_held_sends.py tests/test_concurrency.py tests/test_runtime_collaboration_documents.py tests/test_phase_evidence.py -q --tb=short
# 94 passed, 40.84s

The new refusal control uses a real competing SQLite writer and verifies the registry permission remains held while SQL is blocked. Watcher checks: 15 existing cases passed; the new real-descriptor error control passed separately. git diff --check acb2ddd0e..HEAD passes.

Explicit committed-source comparison through PlanReading((), MainTree(repo, sha)).holds(BusDeletionPhase.evidence): false at acb2ddd0e9692699cff6551067d561e0afbda346, true at candidate 85643b97be3e972f55f0989caf0eab2e47459bcd. No main ref was rewritten.

Evidence is retained under /home/ts/.cache/agent-scratch/bus-deletion-* (source before/after, codec before/after, committed-phase comparison, test results). Initial failed checks remain recorded: the descriptor fixture first misused ExitStack.pop_all; the CLI exposed the codec bug; the family batch exposed the stale uppercase triage fixture. The final checks above include their repairs.

Not installed. The merger still owes the copied installed native/Toad journey: a fresh DM while the copied agent tab is open, its actual reply, Toad survival and tab return. Candidate source evidence does not mark the phase complete on main.

@trissim trissim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Accepted the separable watcher checkpoint after reading _on_ready and the full _drain/_refresh path. Native descriptor reads and inotify watch operations raise OSError; the checked header/name decoder raises ValueError before struct unpack can receive a short buffer. SourceFailed still carries the original error to the awaiting reader and keeps failure precedence. This is a native boundary contract, not an error swallowed into a default.

The existing watcher family reports 15 passed in 1.76s. This review covers this exact one-line catch correction, which the current integration may extract independently. The remaining three physical wire-lock sites and phase evidence are still unfinished; this draft does not certify bus-deletion closure or installation.

@trissim trissim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bounded acceptance of independent codec commit 55edba7. Source review confirms SingleValueEncoding moves the existing typed one-field serialization algorithm into one shared capability used by SingleFieldValue and JsonShapeFamily; the latter's raw self.value return is deleted. Distinct decode/schema/CLI argument contracts remain with their existing owners. The direct nested to_wire/json.dumps regression exposes the bug that outer FieldCodec.encode had hidden. Independently replayed tests/test_json_shape_field_codec.py plus tests/test_cli.py::TestCliSuccess::test_ledger_read_and_merge against this source: 13 passed in 9.35s. Pytest emitted unrelated historical temporary-directory cleanup warnings, not behavioral failures. This accepts the separable codec repair, not the unfinished physical-bus deletion or installed usability.

@trissim trissim changed the title Delete residual physical bus locks and own watcher failures Remove residual bus-file locks and repair shared boundary behavior Oct 23, 2026

@trissim trissim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Source review accepts the coherent deletion in 9215d55 and phase evidence in 85643b9: NativeReplyPreparation retains its existing local state lock and SQLite publication transaction; SendRefusal holds MaintenanceBarrier registry permission through its SQL write; CollaborationLedger retains the registered-author snapshot through the existing document update. These delete the three physical-bus locks without creating another exclusion owner. BusStorageAbsent consumes the existing committed MainTree ASTs and explicitly screens known symbol/file/path syntax, including the missed with_name path; aliases/dynamic paths remain outside its claim. Read the retained 94-case result and committed old-head=false/candidate=true controls. Independently replayed the real blocked-SQLite refusal permission test and source-use/prose phase controls: 2 passed in 3.33s. One documentation correction is with the implementation owner: the Done-means section describes deleted NativeSourceCursor as a current site and must label that incident historical. Installed copied fresh-DM/reply/Toad-tab-return qualification remains with the merger.

@trissim trissim left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Final checkpoint inspection: 85643b9..194957e changes only plans/VIRTUAL-BUS-PLAN.md (9 insertions, 1 deletion). The requested correction now labels NativeSourceCursor as historical and separates the current cursor/coordinator contention obligation from that removed implementation. Production and tests are unchanged from the source-accepted 85643b9 checkpoint and its recorded focused controls. This closes the documentation correction and retains that bounded source acceptance for integration; it adds no runtime, installed-path or whole-phase acceptance claim. Backend rosters owns combining this head with accepted #2623/#2626/#2627 in one side integration branch; route repair #2633 and installation do not wait on this batch.

@trissim

trissim commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Closed by the commander: no commit ahead of main; its work landed through #2635/#2636.

@trissim trissim closed this Oct 23, 2026
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.

1 participant