Repository navigation
Conversation
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
@imran-siddique Please independently review the two bounded delivery-test contributions, each on its own scope:
All returned technical workflows for both heads succeed. Existing formal review requests remain; this single comment avoids separate duplicate reminders. Please identify remaining changes or disposition each bounded PR. Prior test results are not a fresh local rerun, independent approval or production-delivery certification. Prepared with OpenAI Codex. |
drk-ram
left a comment
There was a problem hiding this comment.
Reviewed head 57e75a9aeacbdb68328b4a9297afbb9141aaf0d0.
Approving. These tests hold the boundary that matters: operational projection and evidence acceptance are not the same event, and a commit that loses its acknowledgement is not local acceptance.
EvidenceAccumulator.append builds the entry, calls durable_append on a copy, and only then appends to _entries and _event_ids. A callback that returns anything other than True, or raises, becomes EvidencePersistenceError with the local chain unchanged. The new Python and TypeScript cases both use that contract: the idempotent test store records the commit, the first acknowledgement is false or an exception, the snapshot and the projection stay empty, the retry is byte-identical, and a later duplicate is EvidenceError before a second store write or a second projection. That is the fail-closed claim-to-evidence rule in draft-durga-agenttrust-trace-verification-00. A projection, a flush, or an exporter return is not the acceptance receipt. TR-2026-06 (10.5281/zenodo.23776574) is why the shared matrix also pins the evidence-chain digests: drop, duplicate, and reorder of the projection must not rewrite the hash chain or let the caller seal anything other than unknown.
I ran python -m unittest discover -s tests -p test_delivery_faults.py -v on this head. All three methods passed, including the real SDK case with ALWAYS_OFF / ALWAYS_ON and an exporter that returns FAILURE. Evidence still persists, completeness stays unknown, and span_event_emitted staying true is correctly treated as "add_event returned", not as collector acknowledgement. The caller-owned provider is shut down in finally, so the test does not mutate the global provider. Hosted CI is green for Python 3.10 through 3.13, the TypeScript job, and CodeQL. I did not re-run the TypeScript suite locally; the hosted typescript job on this head succeeded, and the Python twin covers the same matrix and lost-ack contract.
The duplicate, reverse, and drop cases live in the test emitter, not in a second SDK implementation. docs/delivery-fault-testing.md says that, and it is the right scope. The remaining collector work (pinned receiver, retry, queue exhaustion, restart, partial failure, and separate counts for accepted evidence, attempted projection, collector receipt, and durable backend acknowledgement) is stated as not established. I agree. This does not prove exactly-once delivery, crash recovery, or Collector interoperability, and it should not be read as closing that experiment.
No runtime, schema, or dependency change. I have no change request on this head.
Ready for independent review — 31 October 2026
Readiness assessed at
57e75a9aeacbdb68328b4a9297afbb9141aaf0d0: current metadata is mergeable; all returned PR-triggered technical workflows succeeded, and the public discussion contains no explicit unresolved draft hold or code-change request. Changed-file scope and current CONTRIBUTING requirements were checked. This is a readiness assessment, not a new full source audit, fresh suite run, independent approval or complete branch-protection inventory.Synthetic delivery/lost-ack regression coverage only; no durable recovery, exactly-once delivery or Collector interoperability claim. The real Collector experiment remains separate in #100.
Independent maintainer review and normal merge gates remain required; no merge or release is authorized by this transition. Earlier test counts apply to their stated revisions; obsolete baseline CI failures below are superseded by the successful current-head runs. Prepared with OpenAI Codex.
Problem
Operational telemetry delivery and accepted evidence have different guarantees. Regression tests should preserve that boundary when projections disappear, duplicate, reorder, or fail, and when a persistence callback commits but loses its acknowledgement.
Changes
Validation
node --import tsx --test test/*.test.ts; npm pack --dry-run passed.Limits
Synthetic emitters and an in-memory store do not prove durable storage, crash recovery, OTLP wire behavior or collector interoperability. Python's real SDK exporter test is not a TypeScript exporter parity claim. The original local test environment did not supply Docker or a Collector; this PR's evidence remains synthetic. The documentation records the remaining version-pinned receiver/backend acceptance criteria.
Ready for independent maintainer review; this PR is not a draft.