Repository navigation
Conversation
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>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
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>
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
@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. |
|
@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:
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
left a comment
There was a problem hiding this comment.
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>
|
@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
left a comment
There was a problem hiding this comment.
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_ackretries the same event identity after the receiver commits and drops its acknowledgement. One backend commit across multiple attempts demonstrates synthetic backend deduplication. inflight_restartkills 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.
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
3cf135ad1698d9157e99ff1396e8c1665bb0b4a3against 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.test (3.10),test (3.11),test (3.12)andtest (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.typescriptsucceeds; workflow builds/tests the SDK and exercises the packed npm artifact.analyzesucceeds. CodeQL workflow also completes successfully.collectorsucceeds in run 40204492721. The workflow runs baseline delivery cases, observer controls and stress cases, then uploads both evidence directories even on failure.actionlintsucceeds.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.typescriptsucceeds.analyzesucceeds.Additional evidence for this PR
collectorsucceeds for the documented twenty delivery cases and six observer controls.actionlintsucceeds.Recommendation for maintainers to decide
actionlintmandatory: 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.collectorjob 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.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.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.