Report one version and one Node floor, asserted against the tag - #42
Merged
Conversation
package.json declares engines >=22.12.0, so an install on Node 20 fails with an engine error while this line promised it would work. The README badge and quickstart already moved; this was the last place the old floor was stated as a support claim. README.md:47 keeps saying Node 20+ on purpose. It describes when fetch began bypassing https_proxy, which raising the floor does not make false.
docs/install.md still said Node.js 20+ while engines says >=22.12.0, so an
install on Node 20 failed with an engine error the install doc promised would
not happen. Raising that line exposed two more copies of the same class of bug
that a markdown sweep does not reach.
agentwall doctor compared only the major version, `Number(...split(".")[0])
>= 20`, so it printed a green check on Node 20.x and on 22.0.0. npm refuses to
install on both. A tool that tells an operator their runtime is fine, after the
install has already disagreed, is worse than one that says nothing.
GET /health and the telemetry scope each carried the literal "0.1.0". Both
would have reported a version that was never released for the whole of 0.2.0,
and /health is the first thing anyone curls at a running instance.
src/version.ts now reads package.json once and exports the version and the
engines floor, so a release bump reaches every surface at once instead of
needing four edits nobody remembers to make. cli.ts loses its private
getPackageVersion in favour of it: one reader, not two. The lookup walks up
from the module rather than assuming a depth, because the same code runs from
dist/, from dist/routes/, from src/ under ts-node, and from inside
node_modules once published.
Tests cover the floor comparison at the boundary that was broken (major equal,
minor below) and pin /health to the manifest version. Both fail if the old
behaviour is restored.
README.md:47 keeps saying Node 20+ on purpose: it describes when fetch began
bypassing https_proxy, which raising the floor does not make false.
Test Suites: 35 passed, 35 total
Tests: 311 passed, 311 total
…x them The build job already asks the Go binary what version it reports. The Node surfaces had no such check, which is how three string literals sat at 0.1.0 for months: GET /health, the telemetry scope, and a doctor check comparing the wrong number the wrong way. All of them derive from dist/version.js now, so asserting that module and the CLI covers every one. A release stops rather than publishing a build whose reported version is not the tag. This is the difference between fixing four surfaces and making it impossible for them to silently diverge again. Falsified both directions: passes at the real version, fails naming each disagreeing surface when handed a tag the tree does not carry.
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.
Report one version and one Node floor, and assert them against the tag.
Opened on behalf of a subagent that could not reach the API from this host (
ghis not installed and the credential sits in a locked keyring). It pushed over SSH; only the HTTPS API needed a token.Why this is not a docs sweep
docs/install.mdsaid Node.js 20+ whileenginessays>=22.12.0, so an install on Node 20 failed with an engine error the install doc promised would not happen. Raising that line exposed two more instances of the same class of bug that a markdown sweep does not reach.Three string literals had sat at
0.1.0for months:GET /health, the telemetry scope, and adoctorcheck. The doctor check was also comparing only the major version, viaNumber(...split(".")[0]), which is the wrong number compared the wrong way. All three now derive fromdist/version.js.Why it cannot regress
The build job already asks the Go binary what version it reports; the Node surfaces had no equivalent check, which is precisely how three literals drifted unnoticed. The release workflow now asserts every version surface against the tag rather than trusting that someone fixed them.
The Go side was already better covered than the Node side, by three things that predate this branch:
verifier/report.goholds the version as a literal,verifier/main_test.gofails if that literal disagrees withpackage.json, andrelease.ymlstamps it from the tag via ldflags and then asserts the built binary prints it. That unit test is exactly what the Node side lacked, andtests/version.test.tsis now its analogue.Verification
Blast-radius run rather than the full suite: cli, telemetry-otel, route-auth, version, manifest-inspection, route-error-shape, dashboard, 7 suites and 69 tests passing.
tsc --noEmitclean.check-workflow-pins.jsclean.One interaction risk was checked rather than assumed:
src/version.tsreadspackage.jsonat module load, so any suite mockingfsthat transitively imports health, otel, or cli would break. No suite mocksfs, so it is not realized. Worth knowing before someone adds one.Provenance
This originated on the stranded branch
agent/4.2a-install-node-floor, which had no PR and predates the MCP plane, enforcement modes, the perimeter, therepsecurerename, and thekill-switch/scanrenames. All three defects were confirmed still live on currentmainbefore rebasing, rather than assumed. A comment referencing the retired@reesebuiltscope was corrected to@repsecurein passing.Correction to an earlier draft of this description
An earlier version of this body claimed
README.md:47deliberately kept "Node 20+" to describe whenfetchbecame available. That was true when the original commit message was written and is not true now: the README was rebuilt from 651 lines to 354 in #40, which removed the line. The file currently contains no occurrence of "Node 20" or "fetch", and its only Node statements are "Node.js 22.12 or newer" and "Node.js 22.12+", both already correct. The claim was relayed from a commit message without being rechecked against the tree.