Skip to content

test(evidence): exercise delivery faults and lost acknowledgements - #99

Open
dinakarjs wants to merge 1 commit into
agentrust-io:mainfrom
dinakarjs:test/delivery-fault-acceptance
Open

dinakarjs wants to merge 1 commit into
agentrust-io:mainfrom
dinakarjs:test/delivery-fault-acceptance

Conversation

@dinakarjs

@dinakarjs dinakarjs commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Add a shared five-case delivery matrix consumed by Python and TypeScript, checked against existing pinned evidence-chain digests.
  • Exercise false/throwing lost acknowledgements with an idempotent test store: no local acceptance or projection before acknowledgement; identical retry; no second projection on a later duplicate.
  • Exercise Python OpenTelemetry ALWAYS_OFF/ALWAYS_ON sampling and a failing exporter without changing global providers.
  • Document the test boundaries and remaining collector acceptance work. No runtime, schema, dependency or completeness-contract changes.

Validation

  • Python: 161 tests passed with OTEL_TRACES_SAMPLER=always_on.
  • TypeScript: build passed; 51 tests passed using node --import tsx --test test/*.test.ts; npm pack --dry-run passed.
  • The normal npm test launcher was blocked by an environment EPERM on its IPC socket, so the same test files were run through Node's tsx loader.
  • Mutation check: bypassing Python's acknowledgement guard causes the new lost-ack test to fail; production source restored.
  • git diff --check 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.

Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>

Copy link
Copy Markdown
Contributor Author

@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 drk-ram 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.

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.

This branch has not been deployed

No deployments
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