fix(engines): drop the <26 upper bound so Node 26 hits the guard, not a silent downgrade - #87
Merged
Merged
Conversation
… a silent downgrade The engines cap added in c23120e to protect Node 26+ users is self-defeating. The plugin (and the README/extension examples) launch the server with a bare `npx -y socraticode`, which npm resolves as a version *range*. npm engine-filters a range, so on Node 26 the `<26.0.0` bound excludes every guard-bearing version (1.8.11-1.9.0) and silently resolves to 1.8.10 -- the last version that predates BOTH the cap and the runtime guard. 1.8.10 then boots on Node 26, passes the plain-fetch health probe, and dies on the first qdrant client call (UND_ERR_INVALID_ARG: invalid onError method), storing nothing while codebase_index reports an optimistic background ack. The loud 'use Node 22' guard lives only in the versions npm refuses to serve. Removing the upper bound makes a bare install resolve to the guarded version on Node 26 (verified against the live packument: '*' at node 26.5.0 -> the guarded release), converting silent data loss into the loud, actionable refusal. The runtime guard in src/index.ts remains the sole gate; its comment is corrected (the causality was inverted: qdrant's bundled undici 6 rejects the handler that Node 26's undici 8 builds, not Node rejecting the v6 Agent) and now warns against ever re-adding an engines upper bound. Does not make Node 26 work (upstream qdrant-js #134/#123/#128 still open); it makes the failure honest. Refs #82
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Node.js engine requirement now permits versions 18 and newer. Runtime guard documentation explains the Node 26+ undici incompatibility and import timing; the guard logic itself is unchanged. ChangesNode 26 compatibility
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): Comment |
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.
The problem
A user pilot on macOS (Homebrew default Node 26) found SocratiCode indexes nothing and eventually errors with
fetch failed / UND_ERR_INVALID_ARG: invalid onError method. Root cause is a two-part trap, both verified on a real Node 26.5.0 runtime:npx -y socraticode. A bare package name is a version range (*), not thelatesttag, and npm engine-filters a range. Theengines.node: >=18.0.0 <26.0.0cap (added in c23120e to "protect" Node 26 users) therefore excludes every guard-bearing version (v1.8.11–v1.9.0) on Node 26, and npm silently resolves to v1.8.10 — the last version that predates both the cap and the runtime guard.pickManifest("*", node=26.5.0)→ 1.8.10;pickManifest("latest", node=26.5.0)→ 1.9.0. And on real Node 26.5.0,npm install socraticode --dry-run→ "add socraticode 1.8.10".fetch/healthzprobe (the undici bug is dispatcher-specific), then dies on the first qdrant client call.codebase_indexis fire-and-forget, so it returns an optimistic "indexing started" ack and stores nothing; the failure surfaces only on the nextcodebase_status/codebase_searchas a raw undici error that reads like an env/npx-cache quirk.So the cap meant to protect Node 26 users is exactly what hides the loud "use Node 22" guard from them and serves the guardless build instead.
The fix (Tier 1: make the failure honest)
<26.0.0upper bound (engines.nodeback to>=18.0.0). With no upper bound, a bare install resolves to the guarded version on Node 26. Verified against the live packument with a simulated post-fix release:*at node 26.5.0 → the guarded version → the runtime guard firesexit(1)with "Use Node 22.x" instead of silently downgrading.onErrorrenamed toonResponseError), not Node rejecting the v6 Agent. The comment now also warns future maintainers never to re-add an engines upper bound (it is the misrouting mechanism).This does not make Node 26 work (upstream qdrant-js #134 / #123 / #128 are still open); it converts silent data loss into a loud, actionable refusal for every consumer of the bare name.
Why not pin the manifests to
@latestRemoving the cap fixes bare-name resolution systemically, for all ~15
npx -y socraticodereferences (README, extension, gemini config, both mcp.json). Pinning only the two manifests would be redundant once this ships and would leave the docs inconsistent, and@latestforces a per-launch registry check on every user. So the one-line engines change is the smaller, complete fix.Type of change
Testing
npx tsc --noEmitclean,npm run lint(biome) clean,npm run buildsucceeds (guard still compiled intodist/index.js)*@node26 → 1.8.10; post-fix*@node26 → guarded version@latest→ 1.9.0 (guard fires)Refs #82
Summary by CodeRabbit
Compatibility
Documentation