Repository navigation
Conversation
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
github-actions
Bot
added
the
needs-review:MEDIUM
rajnisht7
left a comment
rajnisht7
left a comment
There was a problem hiding this comment.
the changes and direction looks good, isolation is solid, fresh venv, helper copied in, python -I, PYTHONPATH/PYTHONHOME stripped, and __file__ checked under sys.prefix. That cleanly stops source-tree import leakage. The behavioral checks (valid/invalid fixtures, golden event construction, version) are a clear step up from the old minimal smoke
One thing: the description says this exercises all six valid and seven invalid fixtures, but the harness only requires each category to be non-empty. both check_controls and the unit tests accept any count > 0, so a deleted/renamed/moved fixture still lets the smoke pass with reduced coverage. test_empty_controls_are_caught only catches the zero case
kindly tighten that so the claimed inventory is actually enforced, either exact counts (valid == 6, invalid == 7) or, better, the expected filenames, and add a regression test that a partially missing set is rejected.
A few smaller notes (non-blocking):
- The version regex on pyproject.toml works today but is brittle to whitespace/quoting changes.
- Unit tests cover the helper well; the full smoke_wheel.py path is only exercised when the script runs (CI does this on 3.12). That’s fine, just worth being clear about the layering.
- Controls come from the source checkout by design, so this proves installed behavior matches current source controls, not that the artifact embeds the exact same fixture set. The PR already scopes this correctly.
Prepared with OpenAI Codex. Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
@rajnisht7 please re-review cb6c0d7. Exact fixture filenames now enforce all six valid/seven invalid controls, with missing, renamed and duplicate regressions. 164 tests and both installed wheel/sdist smoke paths pass; restoring the old helper fails all six new inventory subcases. This remains installed behavior against source controls, not embedded-fixture verification. Updated description records validation and limits. Prepared with OpenAI Codex. |
|
@rajnisht7 following up for your re-review and disposition of the fixture-inventory finding at cb6c0d7. The published correction enforces the exact six valid/seven invalid filenames and rejects partial, renamed or duplicate inventories. The description records 164 passing tests, both installed wheel/sdist smoke paths, and six inventory subcases that fail with the old helper restored. I have just rechecked this head: CI and CodeQL succeeded; the inapplicable Dependabot auto-merge workflow was skipped. Please confirm whether your requested change is satisfied or identify remaining corrections. Formal reviewer assignment returned 403, so this conversation comment is the re-review request. Prior local validation is cited, not a fresh suite run. Prepared with OpenAI Codex. |
Merge the installed Python distribution controls from agentrust-io#121 while preserving PR agentrust-io#123's TypeScript artifact checks unchanged. Conflict-free merge; no SDK runtime, schema, dependency, or release-permission changes. Validated combined tree on Linux/Python 3.12 and Node 24.22.1: 164 Python tests, 44 TypeScript tests, four installed npm controls, strict consumer compilation, three damaged-declaration controls, and isolated wheel/sdist smoke checks pass. Schema, version, OTel, conformance, workflow OIDC and whitespace checks pass. TypeScript tests used Node's --import tsx entry point because the standard tsx launcher hits sandbox IPC EPERM. Hosted CI and any required renewed approval remain pending. Prepared with OpenAI Codex. Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
…rtifact (#123) ## Problem and change The packed npm smoke test checks declaration existence but does not compile a TypeScript consumer. Replacing dist/index.d.ts with `export {};` still passes all four existing runtime checks. Compile an isolated strict NodeNext consumer against the installed tarball, including public value/type exports, factory construction, validator narrowing, and expected errors for invalid input/output types. The compiler comes from the existing npm-ci lockfile installation; SDK resolution stays in the temporary consumer, with skipLibCheck disabled and no ambient types from the checkout. Existing CI and release calls to smoke_npm.mjs both gain this check. Add three CI regression controls that preserve JavaScript and prove declaration compilation rejects empty exports, a missing transitive declaration, and runId widened to any. Each control first verifies all four runtime checks passed. Use explicit TAP output so those assertions work across supported Node versions. ## Validation - Current packed artifact: four runtime checks and strict consumer compilation passed on Node 24.22.1. - All three damaged-artifact regression controls passed. - TypeScript build passed; all 44 SDK tests passed with `node --import tsx --test test/*.test.ts`; npm pack --dry-run passed. The standard npm run check hit this sandbox's EPERM restriction on the tsx CLI IPC socket, so its test phase was rerun through Node directly. - Three release safeguard tests, workflow OIDC check, and git diff --check passed. No SDK runtime, schema, dependency lock, or release permission changes. Extends the npm smoke coverage from #79; independent of the Python artifact work in #121. No package published. ## DCO Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com> Prepared with OpenAI Codex. Independent maintainer review and hosted CI remain required. ## Contract impact - [x] No wire-contract change. ## Privacy and security - [x] Consumer controls use synthetic identifiers and values; no production data, prompts, source content, or credentials are added. - [x] Release permissions, dependency locks, SDK behavior, and schemas are unchanged. - Independent maintainer review remains pending; this self-review is not approval. ## Self-review evidence (2026-10-28) Reviewed head `fc2c765ea6510771b292627027b474cf9d87be68` against repository ownership/contribution rules and accumulated AgentTrust guidance on isolated validation, positive/refusal controls, guard-removal tests, DCO, scope and acceptance boundaries. A fresh detached worktree at this exact head was installed with npm ci, built, and packed. The valid tarball passed four runtime checks and strict consumer compilation; all three damaged-artifact controls passed. Temporarily removing only the compiler invocation made all three controls fail because the damaged packages exited successfully (0 instead of 1). Restored the original source and reran the positive and negative controls; the worktree remains unchanged. Three release safeguard tests, OIDC checks, JavaScript syntax checks and whitespace checks passed. Hosted checks for this head passed: TypeScript (Node 20.20.2, 44 SDK tests, four artifact runtime checks, consumer compilation and three regression controls), Python 3.10–3.13, CodeQL and actionlint. [TypeScript job evidence](https://github.com/agentrust-io/agentrust-telemetry/actions/runs/40704653328/job/122873379741). The single PR commit carries the contributor's DCO sign-off. No blocking code finding identified. ## Limits This is a sampled ESM/NodeNext consumer check with the lockfile-installed TypeScript compiler, not exhaustive public-API, compiler-version, CommonJS, browser/bundler, or operating-system compatibility certification. The regression harness uses tar on the existing Ubuntu CI runner. Runtime consumer dependencies resolve using their supported ranges, as in the existing smoke test. No release was dispatched, package published, acceptance tracker closed, or maintainer approval asserted. Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com> Co-authored-by: Rajnish Tiwari <121179385+rajnisht7@users.noreply.github.com>
Extend the wheel/sdist consumer smoke check to validate installed import origin/version, six valid and seven invalid source controls, and deterministic event construction against the shared Python/npm golden control. The helper runs with Python -I in a fresh environment outside the checkout.
The expected filenames are now enforced exactly. Missing, renamed, duplicate and empty controls fail; a nonempty subset can no longer silently reduce coverage.
Validation on Linux/Python 3.12: fresh hash-locked test/release dependencies; 164 tests pass with process-local OTEL_TRACES_SAMPLER=always_on. Built wheel and sdist each pass the full smoke path in separate fresh environments, including all 13 fixtures and golden construction. Six helper tests cover accept-all/reject-all, changed construction and inventory faults. Restoring the old helper makes all six partial/renamed/duplicate inventory subcases fail. Changed-file lint and whitespace pass.
Controls come from the source checkout: this proves installed behavior against source controls, not that artifacts embed those fixtures. Consumer installs resolve runtime dependencies afresh; this is not locked deployment certification. No SDK runtime/schema or release permission changes.
Prepared with OpenAI Codex. Signed-off-by: Srinivasa Dinakar Jainti dinakarjs@gmail.com.
Independent current-head review and hosted CI remain required; prior-head CI is historical.
Current head: cb6c0d7. Published tree d14d829251b65edf134ef918a4dbabd8668a64a2 matches the locally validated source tree.
Hosted validation update: all returned current-head PR workflows completed successfully (the inapplicable Dependabot auto-merge workflow was skipped). Relevant runs: workflow 41166473023, workflow 41166473025. This workflow query is not a complete branch-protection inventory. Independent re-review has been requested in the PR conversation and remains pending.