Skip to content

fix(pack): register bin/cli/utils/volatileEnvPath.mjs as a required artifact path (#11437) - #11588

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/11437-pack-artifact-volatile-env-path
Aug 26, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/11437-pack-artifact-volatile-env-path

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

Part of the "every gate is red on every branch" cleanup — one of six independent unit-test failures on release/v3.8.51.

The red test

✖ every bin/omniroute.mjs local import is enforced by check:pack-artifact
  AssertionError: add bin/<file> to PACK_ARTIFACT_REQUIRED_PATHS: cli/utils/volatileEnvPath.mjs
  actual:   [ 'cli/utils/volatileEnvPath.mjs' ]
  expected: []

Cause

#11437 (943b9aaa8 — the commit
issue #11449 names as the offending push range) added to bin/omniroute.mjs:

import { describeVolatileEnvWarning } from "./cli/utils/volatileEnvPath.mjs";

without registering the file in PACK_ARTIFACT_REQUIRED_PATHS.

Why the list matters

This is not bookkeeping. The list's own comment says it:

Direct imports of bin/omniroute.mjs — bin/cli/ is only an allowlist PREFIX, so a
file vanishing from the tarball never fails the unexpected-paths check; only these
required entries make its absence loud.

assembleStandalone copies wrappers to the dist root and the prepublish prune deletes
anything not allowlisted. A bin/cli/ file is inside a prefix, so its disappearance is
silent — and describeVolatileEnvWarning is called on every CLI boot, so the first
symptom would be a crash on a user's machine after npm i -g omniroute. Exactly the
#7065 class the tls-options and head-response-guard entries directly above it exist
to prevent.

Verification

# base branch
✖ every bin/omniroute.mjs local import is enforced by check:pack-artifact
ℹ tests 6  ℹ pass 5  ℹ fail 1

# with this change
✔ sanity: EXTRA_MODULE_ENTRIES parsing finds the known dist-root wrappers
✔ every local import of every npm-shipped wrapper survives the prune (allowlist)
✔ every local import of every npm-shipped wrapper is enforced by check:pack-artifact
✔ dynamic import() closure is covered (server-ws boots dist/server.js)
✔ every bin/omniroute.mjs local import is enforced by check:pack-artifact
✔ no npm-shipped wrapper uses a parent-relative (../) import
ℹ tests 6  ℹ pass 6  ℹ fail 0

Two added lines, no code change.

…rtifact path (diegosouzapw#11437)

`Unit Tests fast-path` is red on every branch:

    ✖ every bin/omniroute.mjs local import is enforced by check:pack-artifact
      add bin/<file> to PACK_ARTIFACT_REQUIRED_PATHS: cli/utils/volatileEnvPath.mjs

diegosouzapw#11437 (`943b9aaa8`) added

    import { describeVolatileEnvWarning } from "./cli/utils/volatileEnvPath.mjs";

to `bin/omniroute.mjs` without registering the file. That list is not decoration: as its
own comment says, `bin/cli/` is only an allowlist PREFIX, so a file vanishing from the
tarball never trips the unexpected-paths check — only a required entry makes its absence
loud. Without it, the prune could drop a module the published CLI imports on every boot
and the failure would first appear as a crash on a user's machine.

Same diegosouzapw#7065 class as the `tls-options` / `head-response-guard` entries above it.
…aths list

The previous commit added bin/cli/utils/volatileEnvPath.mjs to
PACK_ARTIFACT_REQUIRED_PATHS but left the hardcoded expectation in
"findMissingArtifactPaths flags missing root runtime files in the tarball"
untouched, so that test failed on the new entry.

The golden list is the point of that test — it is what makes an accidental
removal from the policy visible — so the fix is to extend it, not to derive it
from the constant under test.
@ntdatt812

Copy link
Copy Markdown
Contributor Author

Follow-up in 2625a03: adding bin/cli/utils/volatileEnvPath.mjs to PACK_ARTIFACT_REQUIRED_PATHS also broke a sibling test — findMissingArtifactPaths flags missing root runtime files in the tarball (pack-artifact-policy.test.ts:243) pins the full expected list literally, so the new entry showed up as an unexpected diff.

I extended the golden list rather than deriving it from the constant under test: the literal list is what makes an accidental removal from the policy visible, and deriving it would make the assertion vacuous. tests/unit/pack-artifact-policy.test.ts is 17/17 locally.

Worth knowing for future entries: every addition to PACK_ARTIFACT_REQUIRED_PATHS needs the same one-line update here.

@diegosouzapw
diegosouzapw merged commit f40f6e1 into diegosouzapw:release/v3.8.51 Aug 26, 2026
10 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 26, 2026
Merged via /merge-batch (lote 2026-08-26 batch 2, v3.8.51). 7 conflitos, todos triviais/duplicados (mesmos base-reds já corrigidos por PRs paralelas mergeadas neste lote — #11580/#11582/#11583/#11585/#11588/#11589/#11590/#11591/#11609): mantida a versão já validada nesses casos. Validado: 68/68 testes passando. Obrigado por resolver os base-reds.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…rtifact path (diegosouzapw#11437) (diegosouzapw#11588)

Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…osouzapw#11608)

Merged via /merge-batch (lote 2026-08-26 batch 2, v3.8.51). 7 conflitos, todos triviais/duplicados (mesmos base-reds já corrigidos por PRs paralelas mergeadas neste lote — diegosouzapw#11580/diegosouzapw#11582/diegosouzapw#11583/diegosouzapw#11585/diegosouzapw#11588/diegosouzapw#11589/diegosouzapw#11590/diegosouzapw#11591/diegosouzapw#11609): mantida a versão já validada nesses casos. Validado: 68/68 testes passando. Obrigado por resolver os base-reds.
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.

2 participants