fix(dm): report observed durable-ACK ingress provenance separately from send strategy (#461) - #563
Merged
Merged
Conversation
…om send strategy (#461) The durable-ack receipt path hardcoded path: gossip_inbox — the send strategy — so an ACK hedged home over the direct typed/raw-QUIC path still reported gossip_inbox. Additive provenance, no public breaks, no wire change: - dm.rs: new pub DmAckIngress { DirectTyped, Subscription }; InFlightAck carries a provenance cell stamped immediately before reply.send (send/recv happens-before); resolve_for_protocol delegates to resolve_inner(None) — signature unchanged; additive resolve_for_ingress stamps only inside the matched first-valid-winner branch (failed attempts stamp nothing); additive register_for_protocol_with_provenance returns (Receiver, ProvenanceCell). - dm_inbox.rs: handle_incoming threads the ingress label — subscription loop stamps Subscription, the direct-hedge synthetic path stamps DirectTyped; the authenticated resolve stamps the winner's ingress. - dm_send.rs: durable waiter registration uses the provenance variant; ack_outcome_to_receipt threads the cell so every outcome exit reports observed ingress or None. DmReceipt gains observed_ack_ingress: Option<DmAckIngress> (additive read; in-crate literals updated; publish-only/relay-lane/fallback paths are None — unknown never fabricated). - direct.rs HTTP: optional observed_ack_ingress key (direct_typed/subscription) only when Some; path remains the strategy. Tests (focused exit 0): waiter first-valid-winner in both arrival orders + losing attempt stamps nothing; wrong-binding/invalid-signature leaves None then valid wins (waiter- and handler-level); subscription-labelled handler path stamps Subscription; direct-hedge ingest test now asserts DirectTyped; unstamped legacy path stays None. Broad dm subset 100/100 exit 0. The literal spawn_subscription_loop wrapper is a six-line pass-through to the exercised handler call site; driving it needs transport-signed gossip frames (no hermetic seam) — noted, not silently skipped. Gates on this exact tree (CARGO_BUILD_JOBS=2, real exits): fmt 0; all-target clippy -D warnings 0; production lib/bins panic/unwrap/expect clippy 0; workspace all-target check 0. Committed immediately after gates; clean status post-commit proves byte identity.
…-selected dispatch seam (#461 r2) Independent review of a9cfdcc found two P2s; both closed: - Public struct preserved: DmReceipt restored to its original four public fields; observed ingress rides an internal Result<(DmReceipt, Option<DmAckIngress>)> — pub(crate) send_via_gossip_with_provenance (public send_via_gossip is a thin wrapper) and pub(crate) Agent::send_direct_with_config_with_provenance (public send_direct_with_config unchanged, history wiring shared via the renamed inner). Raw-QUIC/loopback/relay/publish-only paths carry None. The direct.rs route reads the tuple. Downstream compatibility is proven by a NEW integration-crate fixture (tests/dm_receipt_compat.rs) constructing the ORIGINAL four-field literal — a required field would break that compilation. - Production-selected dispatch: the subscription loop body is extracted into dispatch_subscription_message, which itself selects DmAckIngress::Subscription; the loop and BOTH #461 controls (subscription-label stamping; invalid-signature-first-leaves-None-then-valid-wins) drive that same seam — a test can no longer choose the label it asserts. Verification on this exact tree (CARGO_BUILD_JOBS=2): focused 8/8 exit 0 incl. the seam-driven controls; compat fixture 1/1 exit 0; ordered gates with raw commands and real exits — fmt 0, all-target clippy -D warnings 0, production panic/unwrap/expect clippy 0, workspace all-target check 0. PRE/POST sha256 custody in i461-final-gates.log (fmt is gate 1; gates 2-4 and the committed bytes are the post-fmt hashes: dm_inbox.rs dec742be…, dm_send.rs 4a694091…).
…ce path + strategy-wording docs (#461 r3) Senior review of b8a9e2a found the provenance entry bypassed the ADR-0023 outbound history wiring (classification + record_dm_outbound), so the REST /direct/send route lost sender LocalSend history on success. Fix: the history block lives in send_direct_with_config_with_provenance — the single common path — and the public send_direct_with_config only maps away the tuple; exact once-history semantics preserved for every caller. Also corrects the DmReceipt/DmPath docs that claimed path reports the transport that actually delivered: it is the send strategy (observed ACK ingress is the separate #461 surface). Discriminating regression: provenance_entry_records_outbound_history_on_success — a fully offline agent (no sockets, no daemon) with a real history DB self-DMs through the exact route entry and polls the agent's own store handle for the Outbound row (a second SQLite connection cannot take the live writer's locks). Structural discrimination: the b8a9e2a entry had no record call at all. Integrated origin/main 694f03d (normal merge) before final gates. Verification on this exact tree (CARGO_BUILD_JOBS=2): focused 8/8 + compat 1/1 exit 0 (i461-final2-tests.log); four ordered gates with logged commands and EXIT=0 each (i461-hold-gates.log); post-fmt sha256 of all changed files recorded in the same log (dm.rs c645aa98…, dm_inbox.rs dec742be…, dm_send.rs 4a694091…, lib.rs cb46d8b0…, direct.rs 164ecac8…, dm_receipt_compat.rs e8845a68…).
dirvine
marked this pull request as ready for review
September 7, 2026 21:54
dirvine
marked this pull request as draft
September 7, 2026 22:02
…CK provenance Senior + Claude confirmed: against a v1-only peer the durable send registers a V1 waiter, and an authenticated v1 gossip ACK could populate observed_ack_ingress, which the HTTP mapper describes as the durable-ACK transport. Fix at the CENTRAL stamping point per Root/Claude: resolve_inner stamps the ingress cell only when the resolved waiter negotiated protocol_version >= DM_PROTOCOL_DURABLE_ACK. v1 waiters still complete normally (no v1 failure, no negotiation change, internal ACK behavior preserved); v2 stamping is unchanged; unknown stays None. Regression: v1_ack_resolves_waiter_but_stamps_no_durable_ingress — a fully signed/attested V1 envelope through the REAL subscription dispatch seam resolves its v1 waiter (Accepted) while the provenance cell stays None (the HTTP mapper omits the field on None by construction). The v2 stamping positives (subscription-label, direct-hedge, invalid-first-then-valid) and all prior focused controls remain green: 10/10 exit 0. Gates on this exact tree (CARGO_BUILD_JOBS=2, commands + EXIT=0 each in i563-p2-gates.log): fmt; all-target clippy -D warnings; production lib/bins panic/unwrap/expect clippy; workspace all-target check. Post-fmt sha256 of all changed files recorded in the same log (dm.rs 66e281c9…, dm_inbox.rs 76c5087b…).
Contributor
Author
|
Greptile: the durable-type invariant is enforced at the central internal guard ( |
dirvine
marked this pull request as ready for review
September 8, 2026 00:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes (#461)
The durable-ack receipt path hardcoded
path: gossip_inbox— the send strategy the sender chose, not the transport that actually carried the ACK. When the gossip-inbox publish was slow or lost and the durable ACK was hedged home over the direct typed/raw-QUIC path, the response still saidgossip_inbox.Design
DmAckIngress { DirectTyped, Subscription }is stamped at the single authenticated waiter completion (first-valid-matching-ACK wins; failed attempts stamp nothing; unstamped staysNone— unknown is never fabricated). The existingDmReceipt.pathkeeps its meaning (send strategy — docs corrected to say so); the observed ingress rides an internalResult<(DmReceipt, Option<DmAckIngress>)>exposed to the/direct/sendroute as an optionalobserved_ack_ingressJSON key (direct_typed/subscription), present only when known.DmReceiptkeeps its original four public fields; every pre-existing public signature is unchanged (send_via_gossipandsend_direct_with_configare thin wrappers). A downstream-crate fixture (tests/dm_receipt_compat.rs) constructs the ORIGINAL struct literal — any required-field addition breaks that compilation.record_dm_outbound) lives in the single common provenance path; the public method only maps away the tuple. Regressionprovenance_entry_records_outbound_history_on_successdrives a fully offline agent (no sockets, no daemon) through the route's exact entry and asserts the Outbound row in the agent's real history store.Subscription; both dm: /direct/send responsepathreports gossip_inbox even when the receipt was hedged home over the direct/raw-QUIC path #461 controls (label stamping; invalid-signature-first-leaves-None-then-valid-wins) drive that seam, so a test cannot choose the label it asserts.Verification (exact tree
5c13dd5, CARGO_BUILD_JOBS=2)-D warnings; production lib/bins panic/unwrap/expect clippy; workspace all-target check); post-fmt sha256 of all six changed files recorded in the same log.Not claimed: network/mesh acceptance runs (no runtime executed by reviewers); wire-format changes (none — the ACK wire is untouched).
Fixes #461
P2 correction (follow-up commits)
Senior + Claude found an authenticated v1 gossip ACK could populate
observed_ack_ingress, which the field describes as the durable-ACK transport. Fixed at the central stamping point:resolve_innerstamps the ingress cell only when the resolved waiter negotiatedprotocol_version >= DM_PROTOCOL_DURABLE_ACK. A v1 waiter still completes normally (no v1 failure, no negotiation change, internal ACK behavior preserved); v2 stamping is unchanged; unknown staysNoneand the HTTP mapper omits the field by construction. Regression:v1_ack_resolves_waiter_but_stamps_no_durable_ingress— a fully signed/attested V1 envelope through the real subscription dispatch seam resolves its v1 waiter while the cell staysNone, paired with v2 stamping positives through the same seam.Honest test-scope note (no overclaim): no HTTP-level response regression was added or run — the existing receipt tests are internal, and the REST-side guard was source-verified only. This focused code-fix proof is accepted for the PR; full live HTTP/E2E coverage remains a separate, unmet requirement.
Verified on the admitted head
a4748815(integrated with mainfe0a775): four ordered gates with commands/EXIT=0, focused 10/10, post-fmt source hashes in the gate log.Greptile Summary
This PR separates the selected DM send strategy from the observed transport of the winning authenticated durable ACK.
direct_typedandsubscriptionACK-ingress provenance without changing the publicDmReceiptshape.observed_ack_ingressfield on successful/direct/sendresponses.Confidence Score: 5/5
The PR appears safe to merge; the previously reported v1 provenance issue is fully fixed and no new actionable defects remain.
ACK resolution still requires exact protocol, recipient, and machine binding, and ingress is stamped only for waiters using the durable protocol or newer. The v1 path continues to resolve normally while leaving provenance absent, so the earlier misleading durable-ingress response is no longer present.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR C[Direct-send client] --> R[POST /direct/send] R --> S[Select payload send strategy] S --> W[Register protocol-bound ACK waiter] S --> P[Publish DM] P --> A{Authenticated matching ACK} A -->|Direct typed/raw QUIC| D[Stamp DirectTyped] A -->|Gossip subscription| G[Stamp Subscription] A -->|v1 or unknown| N[Leave provenance unset] D --> J[Return receipt plus observed ingress] G --> J N --> J J --> H[HTTP response] H --> X[path: send strategy] H --> Y[observed_ack_ingress: optional ACK transport]Reviews (2): Last reviewed commit: "docs: explain observed durable ACK ingre..." | Re-trigger Greptile
Context used: