fix(compression): make durable adoption proof-preserving across repair and restore - #88551
fix(compression): make durable adoption proof-preserving across repair and restore#88551andrexibiza wants to merge 3 commits into
Conversation
aeb486c to
5ae1bb6
Compare
…watermark The preflight durable-snapshot adoption gate compared active-row COUNT (len(durable_parent) > len(messages)). Count is not monotonic: rotation ends a parent and publishes a child whose rows are fewer but strictly NEWER (fresh AUTOINCREMENT ids), so the one-directional length check was blind to the stale-but-larger-snapshot class: - crash/exception AFTER a committed rotation left the caller holding the pre-rotation transcript while agent.session_id pointed at the committed child; the gate refused to reconcile and re-summarized the same content into a second child (W1 class A); - a session ended by a NON-compression path (/new, gateway hygiene, timeout) between the rotated-parent gate and the durable read was not deflected; its full transcript plus orphan appends could be adopted (W1 class B); - with _persist_user_message_idx unset, the legacy path ASSUMED full durability and an un-persisted live tail was silently dropped from the compress input (W1 class C). Replace the length comparison with a monotonic max-row-id watermark: adopt iff the durable reload's max _row_id strictly exceeds the snapshot's max known _row_id (loader now passes include_row_ids=True), guarded by a same-lock-window liveness re-check of the parent session row and a fail-closed preflush when the anchor is unset. Skip-adoption only, never an abort: the NousResearch#14694 content-equality wedge invariant is preserved, and genuine concurrent-writer adoption (the block's original purpose) still fires because appended rows carry strictly higher ids. Regression tests (tests/agent/test_compression_adoption_watermark.py, all RED on the pinned parent): crash-after-commit retry re-anchors to the committed child; ended-by-other-path parent with orphan appends is never adopted; unset flush anchor with an un-persisted live tail fails closed. Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
Required by the check-attribution CI gate for commits authored under andrexibiza@gmail.com. Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
… liveness TOCTOU Pass B (ReviewB-067b283) required changes, minimal: 1. Watermark reachable in production (CRITICAL-1/P4): gateway SessionStore.load_transcript now passes include_row_ids=True, so the snapshot fed to compress_context carries _row_id on every gateway surface (run_conversation, /compress msgs, hygiene _hyg_msgs) instead of being silently watermark-dead. _build_replay_entry carries _row_id through the main-loop replay rebuild, and the lease-wait reload in run_conversation passes include_row_ids too (W1 class-A re-anchor). 2. Q2 TOCTOU: re-verify ended_at at the adopt decision, after the post-flush re-read (CHANGE 6). end_session()/append_message() take no compression lock, so a gateway timeout/hygiene end between the pre-flush re-check and the adopt now skips adoption. 3. P1 content-blindness pinned: the max-id watermark is content-blind by design; duplicate higher-id rows are adopted while the session stays live. Pinned and documented by regression. 4. MINOR-4: _parent_row is None (get_session unavailable/no row) fails closed instead of falling through to flush+adopt (CHANGE 7), and the adopt-decision re-check fails closed on None too. Regressions (tests/agent/test_compression_adoption_watermark.py, all RED on 067b2830745, GREEN here): gateway loader stamps rows and concurrent rows are adopted (P4); session ended during the pre-adoption flush is not adopted (P2); duplicate higher-id rows pinned when live (P1). Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
5ae1bb6 to
2345516
Compare
|
Rebased #88551 onto current main (
This is the compression-slice rebase only — the #88740 restore boundaries remain intentionally unstamped pending #88758. |
|
Current interlock receipt on head
Do not do a standalone mechanical rebase and merge this head by itself. Its own body is correct that #88740 and #88758 form the proof-preserving follow-on boundary: caller stamping without the projection-level greatest-consumed watermark can turn today's safe fail-closed restore into a false-positive re-adoption of rows that cleanup already consumed/removed. Maintenance order:
Until that combined candidate exists, the current green head proves #88551's own first slice only. It does not prove the newly eligible restore surfaces. |
Architecture interlock — this stack is the durable-projection instance of #90145Keep #88551 + #88740 + #88758 as one proof-preserving slice. The repository-level invariant is now explicit in #90145, but it should not replace the subsystem-specific row/watermark design here. The important distinction remains: A surviving row ID names one durable message generation. It does not reconstruct authority over rows that repair/cleanup already consumed. The greatest-raw-consumed watermark is therefore a separate generation/window proof, not redundant metadata. Pattern 1 formulation for this stack:
Keep the existing fail-closed treatment for missing/malformed/forged/unpersisted proof. In particular, do not allow legacy records lacking Required ABA shape remains: This composes with #90144: survivor identity proves the surviving object; the projection watermark proves the wider consumed mutation window. Neither may be widened into the other. |
andrexibiza
left a comment
There was a problem hiding this comment.
Coordination update: treat #88551 + #88740 + #88758 as one proof-preserving implementation model, not sequential independent fixes. _row_id is surviving durable object identity; _row_id_watermark is the greatest raw durable row consumed by the repaired projection. Cleanup may merge/drop the highest row, so neither fact substitutes for the other. Both live and durable candidates must pass the same repair/projection transform before freshness comparison; missing/malformed/unpersisted proof remains fail-closed; both fields remain internal and are stripped before provider/display serialization. This PR stays ready-for-review (draft: false) while the current branch is semantically compressed/rebased around that combined contract.
|
Source-of-truth correction on the publication blocker: ref mutation is currently available through the connected GitHub surface, so the older PR-body sentence attributing this rebase solely to missing GitHub App repository authority is no longer current. The real blocker is now the implementation state itself. Against upstream I am not force-moving or mechanically rebasing this branch: unlike the empty #84963 placeholder branch, it carries three unique implementation commits and a proof-preserving semantic restack is required. Completion still requires the composed current-main slice described in this PR: surviving |
One-lane proof contract published in #91913The authority-continuity manifest now makes this PR the single compression implementation owner and records #88740 / #88758 as interlocked specification nodes, not independent rebase lanes. The conformance gate requires both proof dimensions: This prevents a caller-only row-stamping rebase from becoming merge-authorized without projection-consumption proof. Durable and live candidates must use the same repair/projection transform, and provider/display serialization must strip both internal fields. Contract PR: #91913 |
Current disposition
Exact head:
2345516d852de091d5ac845287add95c2f1a322d.Do not rebase or merge this branch as a standalone caller-only row-stamping change. Maintain it as one proof-preserving compression slice with #88740 and #88758.
Current topology: 3 commits ahead, 291 behind current main; merge base
e02d1e41...; non-mergeable.Proof-preserving rebase contract
_row_ididentity through repair/cleanup./undo, and lease-reload witnesses on the composed SHA.contributors/emails/andrexibiza@gmail.commapping instead of the branch’s comment-only variant;A caller-only stamp can make a restore eligible while losing proof that cleanup consumed a higher durable row. That would re-adopt malformed or intentionally stripped content and reopen the amplification class under a more convincing watermark.
Repository ref/Contents writes currently fail because the connector has no installed GitHub App account/repository principal; no rebase is claimed.
Original class closed by this branch
Follow-up to #88197:
compress_contextadopted a durable parent wheneverlen(durable_parent) > len(messages). Active-row count is not monotonic. Rotation can end a parent and publish a newer child with fewer rows but strictly newer AUTOINCREMENT identities. The length gate could therefore re-adopt a stale-but-larger snapshot after an aborted rotation or concurrent flush, resummarize the same content, and amplify stored rows.This branch replaces the direction-blind length gate with a monotonic row-identity adoption check:
Existing implementation
agent/conversation_compression.py: max-row-id append gate and liveness recheck.SessionStore.load_transcript: production row-ID retrieval chokepoint.Why #88740 and #88758 are prerequisites, not follow-ups
Repair transforms can merge or delete the message that carried the greatest raw
_row_id:The repaired projection therefore needs two distinct facts:
Every survivor receives the greatest valid raw durable row consumed by that projection. Durable reload and live comparison use the same projected representation. Projection metadata is internal-only and is not written back as provider content.
Prepared evidence
The stacked B2 patch/harness is useful design evidence only:
compression-restore-row-ids-b2-robust.patch, SHA-256f6d6d4406e1ef69e4604bbd4271a7090bb4d4b10c2f3bc1060c82a2c9fbd3f6c;It was not rebased to current main, imported against a complete live repository, or run through Ruff/full CI. The newer packaged handoff is still a handoff, not a remote branch or commit.
Historical verification
git diff --check: clean;Attribution and interlocks