Open
Conversation
Add pre-publish artifact verification to catch broken files/exports, wrong entry paths, or incorrect types resolution before 0.6.0 ships. Runs as a dedicated verify-package job on Node 22 LTS (artifact check, not runtime check) that needs the build matrix to succeed first. - npm pack --dry-run: confirm tarball manifest is producible - publint: lint package.json exports/main/module/types/files - @arethetypeswrong/cli: verify TS consumers (node16/bundler) resolve working types for both CJS and ESM entrypoints Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace Array(48) + pad()/parseInt()/substr() chain with Buffer.alloc(48) plus direct byte assignment and buf.writeBigUInt64BE for the origin and transmit timestamps. Drops the now-unused pad() helper. The new implementation splits the NTP 64-bit timestamp into whole seconds and fractional seconds before combining via BigInt so the fractional portion no longer loses low-order bits to the Number mantissa during the legacy (seconds * 2**32) float round-trip.
acceptResponse previously validated only originTimestamp, leaving transmitTimestamp, receiveTimestamp, and precision as non-null-asserted in collectSamples. Add explicit undefined checks for those fields in acceptResponse (throwing the same Format error pattern as existing checks) so malformed replies are rejected before they become samples, and drop the ! assertions in collectSamples. A guard in the forEach provides type narrowing that is now backed by the validator.
The previous ordering registered client.once("message") inside the
send() callback, so an NTP reply that arrived after the packet was on
the wire but before Node fired the send callback would hit no listener
and be silently dropped; the caller would then wait the full
replyTimeout before rejecting. Attach the message listener up front and
keep hasFinished/timeoutHandler guards so the timeout, send errors, and
socket errors still short-circuit the promise correctly.
The retry counter only advanced when ntpResults was still empty, so any partial success that produced fewer than numSamples usable packets (e.g. one server responding, the rest persistently timing out) could spin the do/while loop indefinitely because retry stayed at 0. Snapshot the result count before each round and bump retry when the post-round count is unchanged, keeping the existing retry < 3 cap.
Two related leaks: (1) dgram.send can throw synchronously when the socket is in a bad state, and the throw was not caught, so the timeout handler, UDP socket, and registered listeners all leaked and the surrounding promise was never rejected; wrap the send call so we clear the timeout, run NtpTimeSync.cleanup, and reject exactly once. (2) NtpTimeSync.cleanup only called client.close(), leaving "message" and "error" listeners attached on an abandoned socket where a late event could still fire resolve/reject or prevent the process from exiting; call client.removeAllListeners() before close().
….lock
The cache key used hashFiles('**/yarn.lock'), but yarn.lock is
gitignored and thus absent from CI checkouts. hashFiles returned an
empty hash, collapsing the key to a constant (e.g. Linux-modules--22)
that never invalidated across commits.
This caused verify-package to restore a stale node_modules from
before commit 1d20851 and skip install (cache-hit guard), so newly
added devDependencies publint and @arethetypeswrong/cli were missing:
/bin/sh: 1: publint: not found (exit 127).
Switch all three jobs (build, verify-package, deno) to key on
package.json, which is tracked and changes when deps change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-release hardening and test scaffolding for v0.6.0. Nine atomic commits grouped by concern:
Core library fixes
new NtpTimeSync(...)constructionscreatePacketwithBuffer.writeBigUInt64BE(6b2b18f) — removes deprecatedsubstrusage, ends per-packet string allocations, bit-accurate timestampsacceptResponse(4602083) — replaces!assertions ontransmitTimestamp/receiveTimestamp/precisionsend(f8377c3) — closes a race where fast replies were missedcreatePacket, usesremoveAllListenersbeforeclosePublic API
src/index.tsfromexport *to explicit named exports; deep-freezeNtpTimeSyncDefaultOptions(5c1032e). Surface area unchanged (5 names), runtime mutation now blocked.Tests
CI
verify-packagejob runsnpm pack --dry-run,publint, and@arethetypeswrong/clion Node 22 (1d20851). Catchesexports/files/types regressions before publish.repository.urlwithgit+prefix for publint compliance.Test plan
yarn buildexits 0yarn test:unit— 22/22 passingyarn verify:package— pack + publint + attw all greenyarn test:integration:cjsandyarn test:integration:esm— passingpackage.jsonto0.6.0before tagging