Skip to content

test: validate Python and TypeScript collector fault cases - #100

Open
dinakarjs wants to merge 15 commits into
agentrust-io:mainfrom
dinakarjs:test/collector-fault-harness
Open

dinakarjs wants to merge 15 commits into
agentrust-io:mainfrom
dinakarjs:test/collector-fault-harness

Conversation

@dinakarjs

@dinakarjs dinakarjs commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Connect the real Python and TypeScript telemetry SDKs through pinned official Collector 0.163.2 to a synthetic SQLite-backed loopback receiver. Local evidence, observed requests and backend commits are recorded separately.

Twenty real-pipeline cases now pass: twelve baseline fault cases plus eight queue/fidelity cases (healthy queue, blocked-consumer saturation, collector kill between commit and ACK followed by a fresh event, and reversed source-timestamp input, for both languages). The stress receiver compares complete normalized bodies, event names and exact nanosecond timestamps. Six observer unit tests catch corrupted projections and vacuous delivery assertions.

Validation: Ubuntu run https://github.com/agentrust-io/agentrust-telemetry/actions/runs/40204045774 at code head 02d5c64 passed all twenty cases and six observer tests. Both SDKs produced evidence/attempts/commits of 8/8/8 healthy, 8/1/1 saturation, 2/2/2 in-flight restart and 3/3/3 reversed timestamp input. Collector overflow is also independently required in its log. Full configs/inputs/logs/SQLite/summaries are retained as workflow artifacts. Two earlier stress attempts exposed harness scheduling/capacity assumptions; those were corrected before the successful run. The subsequent documentation confirmation run 40204226853 delivered all reversed-input TypeScript events intact but exposed an unsupported arrival-order assertion. This head fixes that observer contract: it verifies descending source timestamps, local input event-ID order, all backend commits and full per-event fidelity, while recording and permitting concurrent HTTP reordering. Six observer controls and all twenty delivery cases now pass on final code head 3cf135a in Ubuntu run https://github.com/agentrust-io/agentrust-telemetry/actions/runs/40204492721 . CI, CodeQL and workflow lint also pass on this exact head. The local and published source trees are identical.

Python compilation, JavaScript syntax and TypeScript build pass locally. Local Collector delivery is blocked before receiver startup by process-metrics registration; actual delivery evidence above comes from supported Ubuntu CI.

Limits: synthetic loopback only. New cases use an in-memory queue; restart does not establish persistent queue replay. Saturation deliberately observes loss; SQLite uniqueness is backend deduplication, not an exactly-once Collector guarantee. No production endpoint, runtime SDK behavior, release permission or separately provisioned cloud resource changes. Independent maintainer review remains pending.

Signed-off-by: Srinivasa Dinakar Jainti dinakarjs@gmail.com. Prepared with OpenAI Codex. See experiments/collector/README.md.

Self-review: hosted checks and acceptance boundaries — 31 October 2026

Reviewed current head 3cf135ad1698d9157e99ff1396e8c1665bb0b4a3 against the workflow definitions, CONTRIBUTING.md, GOVERNANCE.md, CODEOWNERS and the active default-branch ruleset. This is an author self-review of checker coverage and reported evidence, not an independent approval or a fresh execution/source audit.

Check Observation
Required Python matrix test (3.10), test (3.11), test (3.12) and test (3.13) all succeed on this head. Workflow covers version/schema/OTel compatibility, conformance, unit tests and compilation; 3.12 also builds and smoke-tests wheel/sdist.
Required TypeScript check typescript succeeds; workflow builds/tests the SDK and exercises the packed npm artifact.
Required analysis check analyze succeeds. CodeQL workflow also completes successfully.
Collector experiment collector succeeds in run 40204492721. The workflow runs baseline delivery cases, observer controls and stress cases, then uploads both evidence directories even on failure.
Workflow validity actionlint succeeds.
Applicability Dependabot auto-merge is skipped for this contribution. Collector and actionlint pass but are not listed among the six mandatory status contexts in the inspected ruleset. The experiment README explicitly says it is not a release gate yet.

Reviewer acceptance should assess the twelve baseline and eight stress cases for both SDKs: healthy delivery, first-response 503, lost acknowledgement, partial rejection, duplicates, idle restart, healthy queue, blocked-consumer saturation, kill between commit and acknowledgement, and reversed source timestamps. Check that local evidence, observed attempts and backend commits remain distinct; saturation requires explicit Collector overflow and positive commits plus observed loss; full normalized bodies, event names and nanosecond timestamps are checked; and all six observer controls reject the documented corrupt or vacuous results. The corrected order contract requires the local input sequence and descending source timestamps while permitting concurrent HTTP arrival reordering.

The successful code-head run is recorded in this PR description and, as of documentation commit d70cdd9, in the README. The README correction removes the stale outstanding-confirmation wording and distinguishes baseline lost-ACK identity retries from the stress restart case, which emits a different event after restart and does not prove persistent replay. Run 40204492721 remains evidence for code head 3cf135a; workflow evidence for the documentation commit is tracked separately. No archived artifact was re-downloaded or re-audited in this follow-up.

Remaining acceptance: the inspected ruleset requires one approving review, code-owner review, approval of the latest reviewable push, resolved review threads and strict up-to-date required checks; it dismisses stale reviews on push and enables additional approval for unattributed changes. No submitted independent review is currently returned, and Imran's review request is pending. Classic branch-protection settings could not be inspected with this connection, so this is not a complete certification of every administrative merge rule.

Claims remain limited to the documented synthetic loopback pipeline. In-memory restart is not persistent-queue replay, backend uniqueness is not an exactly-once Collector guarantee, and no production-delivery assurance follows from these checks. Local Collector startup remains blocked before delivery; supported-host evidence comes from Ubuntu CI. No new run or additional coverage is claimed by this self-review.

Prepared with OpenAI Codex.

Required-checks checklist and proposed policy decision — 31 October 2026

Applies to head 3cf135ad1698d9157e99ff1396e8c1665bb0b4a3. Checked items below record the inspected head's existing evidence; proposed policy items remain pending maintainer decision.

Existing merge requirements

  • test (3.10) succeeds.
  • test (3.11) succeeds.
  • test (3.12) succeeds, including wheel/sdist build and smoke checks.
  • test (3.13) succeeds.
  • typescript succeeds.
  • analyze succeeds.
  • Independent maintainer/code-owner review and approval covering the latest reviewable push.
  • Maintainer confirms review threads are resolved and strict branch/ruleset requirements are satisfied at merge time.

Additional evidence for this PR

  • collector succeeds for the documented twenty delivery cases and six observer controls.
  • actionlint succeeds.
  • Current-head CI, Collector experiment, Workflow lint and CodeQL workflows complete successfully.
  • Experiment limits and evidence-source distinctions are recorded in the self-review above.

Recommendation for maintainers to decide

  • Make actionlint mandatory: run it on every PR before adding its status context to the required-checks policy. It is a small deterministic check that catches invalid workflows/expressions and shell issues ordinary YAML parsing misses.
  • Require Collector validation for relevant changes after accepting this experiment: use an always-reported result that requires the real collector job to succeed when experiment code, SDK/emitter/exporter behavior, relevant schemas/dependencies, Collector configuration/version or the workflow itself changes. Changes outside the accepted scope may report an explicit not-applicable success.
  • Approve the trigger/scope and implementation before enforcing Collector policy: today's workflow-level path filter covers only experiments/collector/** and its workflow; it neither reports on every PR nor covers all SDK/dependency changes. Do not add this filtered context directly as an unconditional required check.
  • Ensure any applicability/result check fails on failed, cancelled, missing or unexpectedly skipped required execution; unknown applicability must not silently pass. Verify both an unrelated-change PR and a relevant-change PR, plus a failing Collector control.
  • Name the Collector maintenance owner and agree reliability/runtime expectations and evidence retention before rollout.

Recommended order: accept the bounded experiment on its evidence and normal review, then roll out workflow scheduling and required-context policy in a separately reviewed change. Collector validation adds synthetic integration coverage; it does not become production-delivery or persistent-recovery certification. This policy recommendation is not approval to change repository settings.

Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Oct 21, 2026
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Consultants <dinakarjs@gmail.com>
…idelity

Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
@dinakarjs
dinakarjs marked this pull request as ready for review October 25, 2026 14:52
@dinakarjs
dinakarjs requested review from a team and imran-siddique as code owners October 25, 2026 14:52
@dinakarjs
dinakarjs marked this pull request as draft October 25, 2026 14:53
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>

Copy link
Copy Markdown
Contributor Author

@imran-siddique requesting your independent review and maintainer disposition at current head 3cf135a. Your formal review request is already pending; no submitted review is currently returned.

I have just rechecked the head: Collector fault experiment, CI, Workflow lint and CodeQL all succeeded. The experiment uses the real Python/TypeScript SDKs with pinned Collector 0.163.2 and a synthetic SQLite-backed loopback receiver. The published run records all 20 delivery cases and six observer controls passing; the description links the retained evidence and documents the corrected arrival-order observer contract.

Please review this bounded experiment and confirm whether it can proceed through normal merge gates, or identify remaining changes. In-memory queue restart does not establish persistent replay; saturation includes observed loss and backend uniqueness is deduplication, not an exactly-once guarantee. Prior experimental evidence is cited, not a fresh run. Prepared with OpenAI Codex.

Copy link
Copy Markdown
Contributor Author

@imran-siddique requesting a maintainer decision on whether actionlint and Collector validation should become mandatory merge checks. I updated this PR's required-checks checklist, distinguishing the six existing mandatory contexts (all passing at 3cf135a), the passing collector/actionlint evidence, and the outstanding independent approval.

Our recommendation:

  1. Make actionlint mandatory after changing its scheduling to run on every PR. It is low-cost and detects workflow/expression/shell failures beyond YAML validity.
  2. After accepting this bounded experiment, require Collector validation for relevant experiment, SDK/emitter/exporter, schema/dependency, Collector config/version and workflow changes through an always-reported applicability/result check. Unrelated changes can explicitly report not applicable; relevant execution must actually pass. Failed/cancelled/missing/unexpectedly skipped execution or unknown applicability must not pass.
  3. Do not directly require the current filtered collector/actionlint contexts: both workflows use pull_request.paths, so unaffected PRs can wait indefinitely for missing required checks. The current Collector trigger also misses relevant SDK/dependency paths.
  4. Agree the applicability scope, maintenance owner, reliability/runtime expectations and evidence retention, then implement and verify scheduling plus repository enforcement in a separately reviewed change. Test unrelated and relevant PRs plus a failing Collector control before enabling the rule.

Please approve, adjust or decline this recommendation and identify the owner/rollout conditions. Repository settings and workflow source have not been changed by this checklist update. This proposal does not strengthen the experiment's synthetic-loopback assurance claims. 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 3cf135ad1698d9157e99ff1396e8c1665bb0b4a3 and the Ubuntu run on that exact head. Run 40204492721 succeeded. The harness is mostly the right experiment. One statement in the tree is false, and it is the statement a later reader will trust.

experiments/collector/README.md still ends the transport-order section with "final-head confirmation is required." That sentence was true of run 40204045774 on 02d5c64572ee8341f3a4d54c10e8a400a51d76f8. It is not true of this head. I confirmed 40204492721 completed successfully on 3cf135ad. The PR description already says so. The file that will merge does not. For a harness whose product is a recorded evidence boundary, a README that says the current head is unconfirmed is a defect in the evidence record, not historical color. Please cite 40204492721 and 3cf135ad in that section and delete or qualify the outstanding-confirmation sentence. The PR body is not a substitute.

Keep the idempotency boundary accurate when you edit that paragraph. Collector fault-delivery idempotency, as specified in draft-durga-agenttrust-trace-verification-00 and measured in TR-2026-04 (DOI: 10.5281/zenodo.23775532), is a delivery identity that survives a fault between commit and acknowledgement: a retry of that same identity must not create a second effect. The baseline lost_ack case in experiments/collector/run.py tests that shape. The receiver inserts into committed with INSERT OR IGNORE on PRIMARY KEY(case_id, event_id) and only then shuts the connection down, and the assertion is committed == 1 with attempts >= 2. That is backend deduplication of a retried identity. The README is right that it is not an exactly-once Collector guarantee. Leave that distinction in the corrected paragraph.

stress.py's inflight_restart does not close the same loop. It commits, blocks before the ACK, kills the collector, and then emits a different event id. That observes the commit-before-ACK boundary. It does not replay the unacknowledged identity, and with an in-memory sending queue it cannot. The current README already refuses a persistent-replay claim. The corrected confirmation paragraph should say the same thing in one sentence, so a green stress row is not later cited as the idempotency proof the baseline lost_ack case is doing.

Two narrower notes, not additional blockers once the README is accurate.

experiments/collector/fidelity.py compares bodies with json.dumps(..., sort_keys=True). That is a reasonable structural check of a protobuf projection, and the 7 versus 7.0 control is the right refusal. It is not RFC 8785 JCS. environment.txt pins rfc8785==0.1.4 and does not use it. If this harness is later cited as trace-evidence receipt equivalence under the same draft, the comparison has to be JCS, particularly around the float in the usage fixture (0.0124), where Python's default number encoding and JCS diverge. Do not describe the current check as receipt canonicalization. I am not asking for that change in this experiment.

check_delivery for queue_saturation requires some loss and some commit, plus the collector log line sending queue is full. It does not require that the committed id is the seeded request. The README's account of Collector 0.163.2 counting the in-flight request against the one-slot queue is an observation of one run. The assertion would still pass if a pressure event committed and the seed did not. Tighten that when you next touch the observer. It is not what I am blocking on.

contents: read, the archive SHA-256, and the binary pin d4fa834fae73cb37e6058955a3833894d1ec815a1fc3529b0c2aaa99d8b22a2e are in good shape. The path-filtered workflow is the right call until someone separately decides this is a required check. I am not approving that policy here.

Requesting changes for the stale confirmation sentence. Once README.md records 40204492721 on this head and does not claim a replay inflight_restart does not perform, I will re-review.

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

dinakarjs commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

@drk-ram The blocking README correction is published at d70cdd9. The transport-order section now cites successful run 40204492721 at code head 3cf135a and removes the stale outstanding-confirmation sentence. It explicitly distinguishes the baseline lost_ack retry of the same identity (one backend commit across multiple attempts) from inflight_restart, which emits a different event after restart and proves neither replay of the unacknowledged identity nor persistent recovery. Backend deduplication remains distinct from an exactly-once Collector guarantee. The PR description is aligned. Runtime, observer and workflow source are unchanged; your nonblocking fidelity/seed-identity notes remain outside this documentation repair. All current-head technical workflows now pass: CI 41497392042, Workflow lint 41497392069, CodeQL 41497392217 and Collector fault experiment 41497391960. Please re-review this head. 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.

Follow-up verification of exact head d70cdd9d86eccf4bb9c5226d4301781035e80718, against my review of 3cf135ad1698d9157e99ff1396e8c1665bb0b4a3.

The blocking evidence-record correction is resolved. This follow-up commit changes only experiments/collector/README.md. The transport-order section now identifies successful run 40204492721 and its exact code head, removes the outstanding-confirmation claim, and correctly separates two experiments:

  • Baseline lost_ack retries the same event identity after the receiver commits and drops its acknowledgement. One backend commit across multiple attempts demonstrates synthetic backend deduplication.
  • inflight_restart kills the Collector after commit but before acknowledgement, then emits a different identity. It does not replay the unacknowledged identity or establish persistent queue recovery.

Neither result is represented as an exactly-once Collector guarantee.

Verification on a detached checkout of the reviewed head, Windows/Python 3.11:

  • python -m unittest discover -s experiments\collector -p 'test_*.py' -v: 6 passed.
  • python -m py_compile experiments\collector\run.py experiments\collector\stress.py experiments\collector\fidelity.py: passed.
  • node --check experiments\collector\emit.mjs: passed.
  • git diff --check 3cf135ad1698d9157e99ff1396e8c1665bb0b4a3..HEAD: passed.

I checked both run metadata and logs: 40204492721 succeeded on 3cf135ad1698d9157e99ff1396e8c1665bb0b4a3; 41497391960 also succeeded on this exact documentation head. Each records twelve baseline cases, six observer tests and eight stress cases. I did not rerun the Linux-only Collector delivery experiment locally on Windows.

My earlier structural-fidelity/JCS and saturation seed-identity observations remain nonblocking limitations, not fixes claimed by this documentation change. Approving this head: the requested README correction and its replay boundary are now accurate.

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

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants