fix(deno): recognize the npm-referrer miss that names the owning package - #3563
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change parses Deno npm-resolution errors that include referrer details. Tests verify detection of missing first-party extensions and rejection of missing transitive dependencies. ChangesDeno extension resolution
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fc6a8eca4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`deno add npm:veryfront` and `deno install -g npm:veryfront` resolve the CLI out of the global Deno npm cache. From that referrer Deno appends the owning package identifier to its resolution failure: Could not find package 'X' from referrer 'Y' (veryfront@0.1.1228). #3560 taught `reportedMissingSpecifier` the bare referrer shape, but the trailing parenthetical left the message unparsed, so every optional first-party extension the root npm package deliberately does not depend on still threw a fatal unclassified error on the documented `deno add` install path and the published CLI could not start a project. Extend that one pattern with the optional ` (pkg@version)` suffix. The transitive-dependency guard is unchanged: a missing `jose` reported from an installed extension still fails the anchor check and surfaces. Found during a DX dogfood walk of the published installation docs, which advertise Deno as a first-class runtime. Confidence: high Scope-risk: narrow Reversibility: clean
2fc6a8e to
e755780
Compare
Found during a DX dogfood walk of https://veryfront.com/docs/code/getting-started/installation, which advertises Deno as a first-class runtime (
deno add npm:veryfront, "Deno 1.45 or later").Symptom
The published
veryfrontCLI was dead on arrival under Deno. Every command exited 1 before anything bound:Installing the named package only advanced the chain to the next one (
@veryfront/ext-db-sqlite, …). The same node_modules tree started fine under Node.Root cause
Not a missing dependency. The root npm package depends only on
STANDARD_ROOT_NPM_EXTENSION_DIRECTORIESby design (src/extensions/first-party-defaults.ts) — the other first-party extensions arebuiltin-deferred/builtin-directand are meant to degrade to "not installed" when absent.loadOptionalBuiltinExtensionalready implements that skip, gated onisMissingFirstPartyExtensionModule, which classifies a failed dynamic import by parsing the runtime's message.#3560 landed the bare Deno npm-resolver shape while this branch was open:
That covers
deno task devin a scaffolded project, where the referrer sits in a localnode_modules/.deno/…tree. It does not cover the documented global install path. When the referrer resolves out of the global Deno npm cache —deno add npm:veryfront,deno install -g npm:veryfront— Deno appends the owning package identifier:The trailing parenthetical leaves the message unparsed,
reportedMissingSpecifierreturnsundefined, the error is not recognised as "extension not installed", and it is rethrown as fatal. So on the documented install path the intentional optional-extension split still turned into a hard startup crash on the first optional extension.Fix
One optional group on the pattern #3560 added:
(pkg@version). Nothing else changes.Regression test
src/extensions/first-party-import.test.ts, beside the existing per-runtime message-shape cases where this classifier's runtime-message contract is already pinned. It covers the global-cache referrer with the trailing package identifier, plus a negative case proving the suffix does not let a genuinely broken transitive dependency inside an installed extension (Could not find package 'jose' from referrer '…/@veryfront/ext-auth-jwt/…' (@veryfront/ext-auth-jwt@0.1.1228).) be swallowed as "not installed".Confirmed failing before the fix (
falsewheretrueis expected) and passing after. Home directory in the fixture referrer is a redacted placeholder; only the message shape is under test.