Skip to content

fix(build): require dist/tls-options.mjs in pack artifact guard (#5452) - #5504

Closed
diegosouzapw wants to merge 1 commit into
release/v3.8.42from
fix/5452-dist-tls-options-pack-guard
Closed

diegosouzapw wants to merge 1 commit into
release/v3.8.42from
fix/5452-dist-tls-options-pack-guard

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Closes #5452

Problem

A fresh npm i -g omniroute@3.8.41 crashes on omniroute serve with ERR_MODULE_NOT_FOUND: dist/tls-options.mjs imported from dist/server-ws.mjs (confirmed on macOS by @DKotsyuba and Windows by @containmethod). Same class as #5227.

Root cause

dist/server-ws.mjs (and dist/open-sse/mcp-server/server.js) import ./tls-options.mjs — the opt-in HTTPS/TLS resolver added in #5242. The assembler (assembleStandalone → syncExtraModulesToDir) copies it into dist/, but it was missing from PACK_ARTIFACT_REQUIRED_PATHS in scripts/build/pack-artifact-policy.ts. Every other server-ws sidecar (server-ws.mjs, responses-ws-proxy.mjs, peer-stamp.mjs, webdav-handler.mjs, http-method-guard.cjs) is required — tls-options.mjs was the gap. So when the 3.8.41 artifact was assembled without it (the release ran through a 34-commit parallel-session race), check:pack-artifact shipped green and users crashed at boot.

Fix

Add "dist/tls-options.mjs" to PACK_ARTIFACT_REQUIRED_PATHS, grouped with its sibling server-ws sidecars. check:pack-artifact (run in prepublishOnly + CI) now asserts the file is packed and fails fast if it is ever dropped again.

Validation (Hard Rule #18 — TDD)

tests/unit/pack-artifact-policy.test.ts:

  • new membership assert (PACK_ARTIFACT_REQUIRED_PATHS includes dist/tls-options.mjs) — failed before, passes after;
  • new behavioral test simulating the broken 3.8.41 tarball (findMissingArtifactPaths flags exactly dist/tls-options.mjs) — proves the guard would have blocked the publish;
  • existing findMissingArtifactPaths expectation updated.
ℹ tests 8  ℹ pass 8  ℹ fail 0

CI's check:pack-artifact provides the integration proof that the current build actually emits dist/tls-options.mjs.

Scope

Build/pack policy + test only. No runtime code change.

Follow-up (separate issue, not this PR): the reporter's suggested generalized guard — assert every relative import inside shipped dist/** resolves to a packed file — would catch this whole class (#5227 + #5452 + future).

dist/server-ws.mjs and dist/open-sse/mcp-server/server.js import
./tls-options.mjs (opt-in HTTPS/TLS resolver, #5242), but the sidecar was
absent from PACK_ARTIFACT_REQUIRED_PATHS. When the 3.8.41 tarball was
assembled without it, check:pack-artifact stayed green and a fresh
`omniroute serve` crashed with ERR_MODULE_NOT_FOUND. Mark it required so
the publish gate fails fast if it is ever dropped again.

Regression guard: tests/unit/pack-artifact-policy.test.ts.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@diegosouzapw

Copy link
Copy Markdown
Owner Author

Closing as redundant — superseded by #5503, which already merged the complete fix into release/v3.8.42 (commit 8d04875). A parallel maintainer session landed it first.

#5503's fix is more complete than this PR: it adds both the staging-allowlist entry for tls-options.mjs (so prepublish no longer prunes the sidecar — the actual cause) and the dist/tls-options.mjs required-path guard (what this PR added). This PR only added the guard, which would have caught the omission but not prevented the prune. Same class as the webdav-handler fix (#5230).

No further action needed here — #5452 is resolved. 🙏

@diegosouzapw
diegosouzapw deleted the fix/5452-dist-tls-options-pack-guard branch June 30, 2026 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant