Repository navigation
Conversation
Compile an isolated strict NodeNext consumer using the lockfile-installed compiler and add CI controls for empty, missing, and widened declarations. Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
|
Closure review request to @rajnisht7 / @carloshvp: #123 (fc2c765) and the related Python installed-distribution #121 (a3ce038) retain successful hosted evidence on their published heads and still need independent maintainer review. Their descriptions record the positive/negative artifact controls and limits; I am not claiming independent approval of my own tests. The separate Collector delivery experiment #100 and delivery-fault draft #99 retain their original evidence boundaries and need their own disposition. No package release or merge is requested as an automatic consequence of these test results. Prepared with OpenAI Codex. |
|
@imran-siddique requesting your review and maintainer disposition on current head fc2c765. @rajnisht7 approved this head on 31 October. The current-head CI, Workflow lint and CodeQL workflows have all succeeded; the inapplicable Dependabot auto-merge workflow was skipped. GitHub currently reports the PR conflict-free, and your formal review request is already pending. This bounded change adds strict TypeScript consumer compilation against the packed npm artifact and three damaged-declaration controls; SDK runtime, schemas, dependencies and release permissions are unchanged. The description records the positive/negative validation and limits. Please review and confirm whether this can proceed through the normal merge gates, or identify any remaining changes or hold. No package publication is requested. Prepared with OpenAI Codex. |
drk-ram
left a comment
There was a problem hiding this comment.
Reviewed head fc2c765ea6510771b292627027b474cf9d87be68 against the packed-artifact contract, not the checkout sources.
The positive path is the right shape. tools/smoke_npm.mjs installs the tarball in a temporary consumer, runs the four runtime checks, then compiles tools/npm-consumer.mts with the lockfile tsc. Resolution stays in that consumer: cwd is the temp directory, types is empty, skipLibCheck is false, and the module mode is NodeNext. That is what catches export {} standing in for dist/index.d.ts, which an existence check cannot. root comes from import.meta.url, so the compiler path is the same whether the caller is the CI job (working-directory: packages/typescript, ../../tools/smoke_npm.mjs) or the release job (node tools/smoke_npm.mjs after npm ci in packages/typescript). I checked release.yml. The release step gains the positive compile and should not also grow the mutation harness. Mutating the tarball belongs in CI, not on the publish path.
The three controls in tools/test_smoke_npm.mjs are load-bearing. Each one requires the TAP summary # pass 4 before it accepts a non-zero compiler status, so a broken install or a runtime failure cannot be reported as a declaration failure. TS2305, TS7016, and TS2578 match the three mutations: empty exports, deleted dist/types.d.ts, and runId: string widened to any. The any case is the one that matters. An unused @ts-expect-error on factory.build(..., {runId: 42}, {}) is the only signal that the public signature stopped rejecting a numeric run id. The control asserts runId: string; is present before the replace, so a second occurrence fails closed rather than silently editing the wrong token. Keep that assert if factory.d.ts grows another runId field.
Residual limits, not merge blockers: one lockfile compiler, one NodeNext consumer, tar on the Ubuntu runner, and a TAP line that must be edited if a fifth runtime check is added. That last coupling fails closed, which is what I want. I am not asking for a compiler matrix in this change.
Approving. No wire-contract change. The negative controls are what make the positive compile meaningful.
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
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.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
Privacy and security
Self-review evidence (2026-10-28)
Reviewed head
fc2c765ea6510771b292627027b474cf9d87be68against 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. 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.