Skip to content

Fix acceptance fixture cold-install after the minimatch override - #10

Merged
dennisdornon merged 2 commits into
mainfrom
fix/acceptance-consumer-overrides
Jul 27, 2026
Merged

Fix acceptance fixture cold-install after the minimatch override#10
dennisdornon merged 2 commits into
mainfrom
fix/acceptance-consumer-overrides

Conversation

@dennisdornon

@dennisdornon dennisdornon commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

The Phase 2 release gate caught test:acceptance:fixture failing on merged main: the cold-install step resolves the packed tarball against a stub registry built from the repo lock, and the filelist minimatch override (869bcba) moved the lock to minimatch 10.x while a bare consumer still resolves minimatch@^5.0.1. The stub has no 5.x to serve, so the install failed with ETARGET.

The fix mirrors the root overrides into the scratch consumer manifest before the install, keeping its resolution inside the stub's catalog. Harness-only change; no product code touched.

Context for review: real consumers resolve minimatch 5.x from the public registry and still receive the vulnerable brace-expansion under filelist. That is an upstream dead end (ejs pins jake 10.x; patched filelist 2.x ships only with jake 12) and affects every ejs-based CLI, so the harness should not pretend otherwise for the product tree; it only needs its hermetic install to work.

Verified: typecheck, lint, test:acceptance:fixture green, npm audit --omit=dev --audit-level=high clean.

Summary by CodeRabbit

  • Tests
    • Improved performance checks to more reliably detect slowdowns across different machines.
    • Updated acceptance-test setup to apply repository configuration consistently during installation.
    • No changes to public APIs or end-user functionality.

The fixture cold-install resolves the packed tarball against a stub
registry that serves only the versions in the repo lock. Root overrides
never apply to dependents, so after the filelist minimatch override the
consumer asked for minimatch@^5.0.1, which the lock no longer carries,
and the install died with ETARGET.

Copy the overrides from the repo manifest into the scratch consumer
manifest before the install so its resolution stays inside the stub's
catalog. Real consumers hit the public registry, resolve minimatch 5.x
normally, and get the vulnerable brace-expansion under filelist; ejs
pins jake to 10.x and the patched filelist 2.x only ships with jake 12,
so nothing in our manifest can change that.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74506994-fa12-48b5-bf67-3394b37aca95

📥 Commits

Reviewing files that changed from the base of the PR and between a86bb70 and cdfe01d.

📒 Files selected for processing (1)
  • src/utils/format.test.ts

Walkthrough

Acceptance packaging now propagates repository dependency overrides into consumer manifests. Formatting performance tests now validate linear scaling through workload comparisons instead of fixed timing thresholds.

Changes

Acceptance package override propagation

Layer / File(s) Summary
Synchronize package overrides before installation
tests/acceptance/lib/pack.ts
packAndInstall reads repository overrides and writes them into the consumer package.json before installation.

Performance scaling test updates

Layer / File(s) Summary
Use scaling-based performance assertions
src/utils/format.test.ts
Adds expectLinearScaling and applies it to targeted repeat and unclosed-bracket performance cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main harness fix for acceptance fixture cold-installs caused by the minimatch override.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/acceptance-consumer-overrides

Comment @coderabbitai help to get the list of available commands.

The two maskUrlUserinfoInText performance tests asserted absolute
wall-clock budgets, and a loaded Windows CI runner blew the 1s budget by
27ms. Time a tenth-size run of the same hostile input first, then
require the full run to finish within 40x that baseline (floored at
10ms). Linear scaling lands near 10x, quadratic near 100x, so the
regression signal survives without the machine-speed dependency.
@dennisdornon
dennisdornon merged commit 40fa44a into main Jul 27, 2026
9 checks passed
@dennisdornon
dennisdornon deleted the fix/acceptance-consumer-overrides branch July 27, 2026 14:03
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