Node v22.18.0 nsolid v6.0.0 release#360
Node v22.18.0 nsolid v6.0.0 release#360santigimeno merged 141 commits intonode-v22.x-nsolid-v5.xfrom
Conversation
PR-URL: nodejs/node#58610 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Port 9229 may already be used by another process. Use a random available one. PR-URL: nodejs/node#58799 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Allow the test to be run in parallel. Refs: nodejs/node#58799 (comment) PR-URL: nodejs/node#58799 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
PR-URL: nodejs/node#58914 Reviewed-By: Richard Lau <richard.lau@ibm.com>
PR-URL: nodejs/node#56350 Backport-PR-URL: nodejs/node#57298 Fixes: nodejs/typescript#17 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
PR-URL: nodejs/node#58175 Backport-PR-URL: nodejs/node#57298 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
This commit refactors the CommonJS loader to remove TypeScript-specific extensions from the require.extensions object for compatibility with libraries that depended on it to initialize extenal TypeScript loaders. PR-URL: nodejs/node#58657 Backport-PR-URL: nodejs/node#57298 Refs: nodejs/typescript#37 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#58643 Backport-PR-URL: nodejs/node#57298 Refs: nodejs/typescript#24 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#57687 Backport-PR-URL: nodejs/node#57298 Fixes: nodejs/node#56830 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#57687 Backport-PR-URL: nodejs/node#57298 Fixes: nodejs/node#56830 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#57704 Backport-PR-URL: nodejs/node#57298 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#57871 Backport-PR-URL: nodejs/node#57298 Fixes: nodejs/node#57471 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#58174 Backport-PR-URL: nodejs/node#57298 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#58639 Backport-PR-URL: nodejs/node#57298 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
PR-URL: nodejs/node#58754 Backport-PR-URL: nodejs/node#57298 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> PR-URL: nodejs/node#56350 Fixes: nodejs/typescript#17
Reverts: nodejs/node#56160 Fixes: nodejs/node#56159 Fixes: nodejs/node#58913 Refs: nodejs/node#58121 PR-URL: nodejs/node#58942 Backport-PR-URL: nodejs/node#58960 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs/node#57525 Backport-PR-URL: nodejs/node#58745 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: nodejs/node#58604 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#58614 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs/node#58615 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#58625 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: nodejs/node#58628 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#58629 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: nodejs/node#58630 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs/node#58580 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs/node#58567 Fixes: nodejs/node#58562 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs/node#58490 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Split indirect eval import tests as they depends on the JS stack to resolve the referrer. PR-URL: nodejs/node#58637 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When --import is used, the ESM loader is used to handle even pure CJS entry points, and it can run into CJS module facades in the evaluating state when the parent CJS module is being evaluated. In this case it should be allowed, since the ESM <-> CJS cycles that are meant to be disallowed (for the time being) should already be detected before evaluation and wouldn't get here, and CJS <-> CJS cycles are fine. PR-URL: nodejs/node#58598 Fixes: nodejs/node#58515 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
PR-URL: nodejs/node#58951 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Ilyas Shabi <ilyasshabi94@gmail.com>
PR-URL: nodejs/node#58125 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs/node#58952 Refs: nodejs/reliability#1250 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: nodejs/node#58925 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
The permission model grew enough to be splited into multiple files, the link in the md file is broken. And as it is not part of a single file, the best way to fix this is just remove the link, many APIs in Node.js does not have the source code linked, this is not bad for our docs. Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com> PR-URL: nodejs/node#58972 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#58973 Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Added serialization and deserialization support for `DOMException`. Co-Authored-By: jazelly <xzha4350@gmail.com> PR-URL: nodejs/node#58649 Fixes: nodejs/node#49181 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jason Zhang <xzha4350@gmail.com>
PR-URL: nodejs/node#58901 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com>
Previously, only child_process.fork propagated the exec arguments (execvArgs) to the child process. This commit adds support for spawn and spawnSync to propagate permission model flags — except when they are already provided explicitly via arguments or through NODE_OPTIONS. Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs/node#58853 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
PR-URL: nodejs/node#58877 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Richard Lau <richard.lau@ibm.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Ruy Adorno <ruy@vlt.sh> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs/node#58847 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Boolean value to check if an ES Module is the entrypoint of the current process. Implements: #57226 Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: nodejs/node#57804 Backport-PR-URL: nodejs/node#58693 Fixes: nodejs/node#57226 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: nodejs/node#58661 Backport-PR-URL: nodejs/node#58693 Fixes: nodejs/node#58660 Reviewed-By: Xuguang Mei <meixuguang@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: nodejs/node#58054 Backport-PR-URL: nodejs/node#59065 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
There are several motivation for removing this: 1. The implementation does not align with InternalModuleStat, most noticably it does not namespace the path or convert it to UTF-16 before using it with std::filesystem::path on Windows which could crash on non-English locale. 2. It needs the Environment - if not for decoding the string, at least for env->exec_path() to resolve the path for namespacing - and therefore needs a handle to the Context which requires a handle scope which actually makes the fast API version slower than the normal binding. For simplicity this just removes the fast API to fix the bug and improve the performance. PR-URL: nodejs/node#58489 Backport-PR-URL: nodejs/node#59065 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#58896 Backport-PR-URL: nodejs/node#59065 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#58385 Backport-PR-URL: nodejs/node#58455 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: nodejs/node#58385 Backport-PR-URL: nodejs/node#58455 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: nodejs/node#58577 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Jacob Smith <jacob@frende.me>
Notable changes: deps: * (SEMVER-MINOR) update amaro to 1.1.0 (Node.js GitHub Bot) nodejs/node#56350 doc: * add islandryu to collaborators (Shima Ryuhei) nodejs/node#58714 esm: * (SEMVER-MINOR) implement `import.meta.main` (Joe) nodejs/node#57804 fs: * (SEMVER-MINOR) allow correct handling of burst in fs-events with AsyncIterator (Philipp Dunkel) nodejs/node#58490 module: * (SEMVER-MINOR) remove experimental warning from type stripping (Marco Ippolito) nodejs/node#56350 * (SEMVER-MINOR) unflag `--experimental-strip-types` (Marco Ippolito) nodejs/node#56350 permission: * (SEMVER-MINOR) propagate permission model flags on spawn (Rafael Gonzaga) nodejs/node#58853 sqlite: * (SEMVER-MINOR) add support for `readBigInts` option in db connection level (Miguel Marcondes Filho) nodejs/node#58697 src,permission: * (SEMVER-MINOR) add support to `permission.has(addon)` (Rafael Gonzaga) nodejs/node#58951 url: * (SEMVER-MINOR) add `fileURLToPathBuffer` API (James M Snell) nodejs/node#58700 watch: * (SEMVER-MINOR) add `--watch-kill-signal` flag (Dario Piotrowicz) nodejs/node#58719 worker: * (SEMVER-MINOR) make `Worker` async disposable (James M Snell) nodejs/node#58385 PR-URL: nodejs/node#59256 Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
2025-07-31 Node.js v22.18.0 Jod (LTS) Release Git-EVTag-v0-SHA512: 54697e2867b91f7258423437f182c3a27f45ba95f91e8446fa67397ab118011c5cf42f4e8b685bc6a73674ab5a82df8948c84b548a58d1b4e1ac81d2874276b8
WalkthroughThis PR updates repository governance/docs, workflows, and build config; adds new benchmarks; upgrades dependencies (Acorn, Amaro, npm, inspector protocol CRDTP, googletest); refactors inspector protocol optional handling (std::optional/unique_ptr); adjusts npm PowerShell command handling; and makes small maintenance tweaks (.mailmap, Makefile, LICENSE, configure help). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PS as PowerShell (-Command)
participant Script as npm.ps1/npx.ps1
participant Parser as PS AST Parser
participant Node as Node.exe
participant CLI as npm/npx CLI
PS->>Script: Invoke with -Command
Script->>Script: Get original command text
Script->>Script: Escape backticks in paths
Script->>Parser: ParseInput(original)
Parser-->>Script: Extract EndBlock text
Script->>Script: Compute CLI args
Script->>Node: Invoke-Expression "& Node CLI args"
Node->>CLI: Execute with args
CLI-->>PS: Exit code (propagated)
note over Script,PS: Pipeline and -File paths unchanged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
deps/inspector_protocol/crdtp/dispatch_test.cc (1)
5-5: Add missing<array>include in dispatch_test.ccThe file
deps/inspector_protocol/crdtp/dispatch_test.ccusesstd::array(e.g. at lines 172 and 189) but does not include the corresponding header. Without an explicit#include <array>, changes to transitive includes could break the build.Please insert the missing include, for example:
#include <vector> +#include <array> // for std::arrayThis will ensure IWYU hygiene and prevent future build breakage.
common.gypi (1)
452-457: Guard shared-cage define behind pointer-compression enable to avoid inconsistent macro set.Currently
V8_COMPRESS_POINTERS_IN_SHARED_CAGEmay be defined even ifV8_COMPRESS_POINTERSis not. Tighten the condition to require both.Apply:
- ['v8_enable_pointer_compression_shared_cage == 1', { + ['v8_enable_pointer_compression == 1 and v8_enable_pointer_compression_shared_cage == 1', { 'defines': ['V8_COMPRESS_POINTERS_IN_SHARED_CAGE'], }],This preserves the isolate-cage branch below:
['v8_enable_pointer_compression == 1 and v8_enable_pointer_compression_shared_cage != 1', { 'defines': ['V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE'], }],deps/npm/docs/output/commands/npm-repo.html (1)
160-164: Update broken--browserconfig` links across npm command docsThe
href="../using-npm/config#browser.html"links in several npm command docs are incorrect—they should point toconfig.html#browser, notconfig#browser.html.Impacted files:
- deps/npm/docs/output/commands/npm-repo.html (line 162)
- deps/npm/docs/output/commands/npm-fund.html (line 165)
- deps/npm/docs/output/commands/npm-docs.html (line 164)
- deps/npm/docs/output/commands/npm-bugs.html (line 164)
Please apply this change in each file:
- <a href="../using-npm/config#browser.html"><code>--browser</code> config</a> + <a href="../using-npm/config.html#browser"><code>--browser</code> config</a>deps/npm/docs/output/commands/npm-rebuild.html (1)
157-160: Fix unmatched closing bracket innpm rebuildsynopsisThe synopsis on line 157 of
deps/npm/docs/output/commands/npm-rebuild.htmlhas an extra closing bracket. It should wrap the<package-spec> ...ellipsis inside a single optional group:• File:
deps/npm/docs/output/commands/npm-rebuild.html
Line 157:-<pre><code class="language-bash">npm rebuild [<package-spec>] ...]</code></pre> +<pre><code class="language-bash">npm rebuild [<package-spec> ...]</code></pre>This is the only occurrence in the
npm-rebuilddocs and should be corrected to match the format used by other npm command synopses.deps/npm/man/man1/npm-repo.1 (1)
13-13: Update man page links to include “.html” extensionThe
la/using-npm/config#browserfragment appears in four man pages and should mirror the HTML docs by usingla/using-npm/config.html#browser. Please update all occurrences:• deps/npm/man/man1/npm-repo.1 (line 13)
• deps/npm/man/man1/npm-docs.1 (line 15)
• deps/npm/man/man1/npm-fund.1 (line 13)
• deps/npm/man/man1/npm-bugs.1 (line 15)Minimal diff (apply to each file at the noted line):
- \fI\(la/using-npm/config#browser\(ra\fR + \fI\(la/using-npm/config.html#browser\(ra\fRdeps/npm/docs/output/using-npm/logging.html (1)
195-197: Broken anchor: swap order to file.html#anchor.Link currently uses "../using-npm/config#timing.html" which likely 404s. Should be "../using-npm/config.html#timing".
Apply:
-<p>The <a href="../using-npm/config#timing.html"><code>--timing</code> config</a> can be set which does a few +<p>The <a href="../using-npm/config.html#timing"><code>--timing</code> config</a> can be set which does a fewdeps/npm/docs/output/commands/npm-bugs.html (1)
164-166: Broken anchor format in link to config page.Use config.html#browser instead of config#browser.html.
-open it using the <a href="../using-npm/config#browser.html"><code>--browser</code> config</a> param. If no +open it using the <a href="../using-npm/config.html#browser"><code>--browser</code> config</a> param. If noconfigure.py (1)
867-872: Fix remaining OpenSSL typos
- In configure.py (around line 380), update
help='link to a shared OpenSSl DLL instead of static linking'
to
help='link to a shared OpenSSL DLL instead of static linking'.- In test/parallel/test-crypto-ecb.js (line 37), update
'OpenSSl 3.x'
to
'OpenSSL 3.x'.No other typos remain:
- “tcp2 DLLs” is a valid reference.
- “Stoage” only appears in the changelog entry documenting a prior fix.
deps/npm/man/man1/npm-update.1 (1)
10-10: Fix alias typo: “udpate” → “update”.
User-facing man page; should not ship with a typo.Apply:
-aliases: up, upgrade, udpate +aliases: up, upgrade, updatedeps/npm/docs/output/configuring-npm/package-json.html (1)
1014-1014: Fix typo: “differening” → “differing”.
User-facing content.-uses a differening npm or node version +uses a differing npm or node versiondeps/npm/docs/output/commands/npm-audit.html (1)
311-313: Grammar: “to toplevel” → “to top-level”.
Minor polish in examples.-Have `audit fix` install SemVer-major updates to toplevel dependencies, not +Have `audit fix` install SemVer-major updates to top-level dependencies, notdeps/npm/man/man1/npm-doctor.1 (1)
20-21: Fix registry domain: .com vs .org inconsistency.
Earlier it says registry.npmjs.com, later and in defaults it’s registry.npmjs.org. Use .org consistently.- The primary npm registry, \fBregistry.npmjs.com\fR, or another service that uses the registry API, is available. + The primary npm registry, \fBregistry.npmjs.org\fR, or another service that uses the registry API, is available.deps/inspector_protocol/crdtp/protocol_core.h (1)
63-66: Header/impl signature mismatch for int32 specialization.The header declares int parameters, but the implementation uses int32_t. This will not link/compile.
Apply:
template <> struct CRDTP_EXPORT ProtocolTypeTraits<int32_t> { - static bool Deserialize(DeserializerState* state, int* value); - static void Serialize(int value, std::vector<uint8_t>* bytes); + static bool Deserialize(DeserializerState* state, int32_t* value); + static void Serialize(int32_t value, std::vector<uint8_t>* bytes); };
🧹 Nitpick comments (72)
deps/npm/man/man1/npm-help.1 (1)
10-10: Alias looks like a typo: “hlep”If unintentional, fix to “help”. If intentional (matching upstream), ignore.
Proposed change:
-alias: hlep +alias: helpdeps/npm/docs/output/commands/npm-restart.html (1)
197-205: Self-link in “See Also”.Minor: the list links to npm-restart.html itself. If this is generator-driven, ignore; otherwise consider removing the self-referential entry.
deps/npm/man/man1/npm-owner.1 (1)
12-13: Confirm CLI-only alias and optional man page
Thenpm-owner.1man page declares an “alias: author” (lines 12–13), but nonpm-author.1file exists underdeps/npm/man/man1. This indicates that “author” is a CLI-only alias in npm 10.9.3. If you’d like to improve discoverability, consider adding a minimalnpm-author.1man page; otherwise, this is acceptable.Pinpoint location needing attention:
- deps/npm/man/man1/npm-owner.1 – alias: author (lines 12–13)
deps/npm/docs/output/configuring-npm/npmrc.html (1)
144-144: Add a backward-compatible anchor for the previous h1 idThe search across
deps/npm/docs/outputdid not find any existing occurrences ofid="----.*----1092", so the old anchor isn’t preserved in the generated HTML. To avoid breaking external bookmarks pointing to----npmrc----1092, please add a fallback anchor immediately before the updated heading.Locations to update:
- File:
deps/npm/docs/output/configuring-npm/npmrc.html- Line: around the
<h1 id="----npmrc----1093">Proposed minimal change:
-<h1 id="----npmrc----1093"> +<a id="----npmrc----1092"></a> +<h1 id="----npmrc----1093">deps/npm/man/man1/npm-hook.1 (1)
1-1: Version update OK; confirm TH date is intentional.TH header now says “June 2025” with NPM@10.9.3. If this manpage is meant to reflect the packaging/release date of this PR (August 2025), consider updating the month; otherwise keep it matching upstream npm’s generation date.
Optional change if you want it to reflect the PR’s release month:
-.TH "NPM-HOOK" "1" "June 2025" "NPM@10.9.3" "" +.TH "NPM-HOOK" "1" "August 2025" "NPM@10.9.3" ""deps/npm/docs/output/commands/npm-init.html (1)
144-144: Consider stabilizing top-level heading anchors to avoid link rot.If feasible in the docs generator, prefer deterministic slugs (e.g., "npm-init") over numeric suffixes that increment each bump; it reduces external link breakage risk.
deps/npm/lib/commands/diff.js (1)
214-217: Directory-spec branch now uses raw file: specs — add coverage.Suggest adding tests for
npm diff <dir>where<dir>and cwd contain '#'.I can draft minimal tap tests exercising these code paths if helpful.
deps/npm/bin/npm.ps1 (2)
30-36: Harden fallback when ScriptPosition is unavailable (PS 5.1/CLM/future changes).Reflecting a non-public property can fail. Fall back to $MyInvocation.Line to avoid breaking -Command in restricted hosts.
Apply:
- } else { - $NPM_ORIGINAL_COMMAND = ( - [Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [Reflection.BindingFlags] 'Instance, NonPublic') - ).GetValue($MyInvocation).Text - } + } else { + try { + $NPM_ORIGINAL_COMMAND = ( + [Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [Reflection.BindingFlags] 'Instance, NonPublic') + ).GetValue($MyInvocation).Text + } catch { + # Fallback for hosts where ScriptPosition is inaccessible + $NPM_ORIGINAL_COMMAND = $MyInvocation.Line + } + }
45-45: Please verify quoting/redirection with -Command.Confirm these cases behave as intended (no leakage of pipeline tokens into args):
- pwsh -NoProfile -Command "& { ./deps/npm/bin/npm.ps1 view express version | Select-Object -First 1 }"
- pwsh -NoProfile -Command "& { ./deps/npm/bin/npm.ps1 view express version > $env:TEMP/npmt.txt }"
I can add Pester tests covering -Command with pipelines/redirections across Windows PowerShell 5.1 and PowerShell 7.x. Want me to open a follow-up PR?
deps/npm/bin/npx.ps1 (2)
30-36: Add resilient fallback when ScriptPosition reflection fails.Mirror the npm.ps1 try/catch fallback to $MyInvocation.Line.
Apply:
- } else { - $NPX_ORIGINAL_COMMAND = ( - [Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [Reflection.BindingFlags] 'Instance, NonPublic') - ).GetValue($MyInvocation).Text - } + } else { + try { + $NPX_ORIGINAL_COMMAND = ( + [Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [Reflection.BindingFlags] 'Instance, NonPublic') + ).GetValue($MyInvocation).Text + } catch { + $NPX_ORIGINAL_COMMAND = $MyInvocation.Line + } + }
45-45: Request validation for -Command pipelines/redirections.Example checks:
- pwsh -NoProfile -Command "& { ./deps/npm/bin/npx.ps1 cowsay hello | Select-Object -First 1 }"
- pwsh -NoProfile -Command "& { ./deps/npm/bin/npx.ps1 --version > $env:TEMP/npxv.txt }"
Happy to contribute Pester coverage mirroring npm.ps1.
deps/googletest/include/gtest/gtest-matchers.h (1)
776-804: Add a compile-time check for convertibility and tighten invariants.Enforce the “must be implicitly convertible” contract and produce clearer diagnostics.
Apply:
class ImplicitCastEqMatcher { public: explicit ImplicitCastEqMatcher(const StoredRhs& rhs) : stored_rhs_(rhs) {} using is_gtest_matcher = void; + static_assert(std::is_convertible<const StoredRhs&, Rhs>::value, + "StoredRhs must be implicitly convertible to Rhs"); template <typename Lhs> bool MatchAndExplain(const Lhs& lhs, std::ostream*) const { return lhs == rhs(); }Optional: consider documenting lifetime expectations when StoredRhs is a reference type.
deps/googletest/src/gtest.cc (1)
6781-6782: Flag parsing wired correctly.New flag is parsed alongside peers. Consider adding it to the help banner for discoverability.
Add to the help text (kColorEncodedHelpMessage) under “Test Execution”:
" @G--" GTEST_FLAG_PREFIX_ "fail_if_no_test_selected@D\n" " Fail the run if no test is selected to execute after filtering/disable rules.\n"deps/npm/docs/output/commands/npm-sbom.html (1)
179-183: Example metadata still shows npm/cli 10.1.0; consider updating or making generic.To avoid confusion with the page’s 10.9.3 header, either bump the example tool/version strings or replace them with placeholders.
Example tweaks:
- "version": "10.1.0" + "version": "10.9.3" @@ - "Tool: npm/cli-10.1.0" + "Tool: npm/cli-10.9.3"Also applies to: 281-283
deps/npm/man/man5/npm-global.5 (1)
145-151: Duplicate “npm help config” in See also.Appears twice; remove one entry.
- npm help config ... - npm help configMakefile (1)
223-223: Include config.status in distclean — good additionThis aligns with the config.gypi rule invoking ./config.status and avoids stale builds after distclean. Optionally consider also removing configure artifacts like config.log if present.
distclean: ## Remove all build and test artifacts. $(RM) -r out $(RM) config.gypi icu_config.gypi - $(RM) config.mk config.status + $(RM) config.mk config.status + $(RM) -r config.logdeps/npm/docs/output/commands/npm-team.html (1)
144-147: Add legacy anchor for backward compatibilityI ran a fixed-string ripgrep search for the previous anchor ID (
----npm-team----1092) across the repo (excludingnode_modules) and found no internal references to it. Even though there are no stale links in our codebase, inserting a no-op legacy anchor here will prevent any external links targeting the old ID from breaking.• File:
deps/npm/docs/output/commands/npm-team.html(around lines 144–147)
• Change:-<h1 id="----npm-team----1093"> +<a id="----npm-team----1092"></a> +<h1 id="----npm-team----1093"> <span>npm-team</span> <span class="version">@10.9.3</span> </h1>deps/npm/man/man5/npm-json.5 (1)
592-592: Consolidate duplicate troff bold escapes across man pages
The regex scan (rg -nP '\\fB\\fB|\\fR\\fR' deps/npm/man) uncovered numerous instances of redundant\fB\fB/\fR\fRmarkers that will render incorrectly in troff. We should collapse all of these to single delimiters.For example, in
deps/npm/man/man5/npm-json.5at line 592:- See \fB\fBpackage-spec\fR\fR \fI\(la/using-npm/package-spec#aliases\(ra\fR + See \fBpackage-spec\fR \fI\(la/using-npm/package-spec#aliases\(ra\fROther representative occurrences:
- deps/npm/man/man7/workspaces.7:10, 13, 67 (
\fB\fBpackage.json\fR\fR,\fB\fBworkspace\fR\fR)- deps/npm/man/man5/package-json.5:315, 592, 865, 987 (
\fB\fBnpm-shrinkwrap.json\fR\fR,\fB\fBpackage.json\fR\fR,\fB\fBengine-strict\fR\fR)- deps/npm/man/man7/scripts.7:230, 331, 385 (double escapes around
npm run <user defined>andpackage.json)- deps/npm/man/man1/npm.1:39, 61 (double escapes around
gitandpackage.json)- …and dozens more under
deps/npm/manTo auto-fix all of them, you can run:
rg -lP '\\fB\\fB|\\fR\\fR' deps/npm/man \ | xargs sed -i -E 's/\\fB\\fB/\\fB/g; s/\\fR\\fR/\\fR/g'deps/npm/docs/content/configuring-npm/package-json.md (3)
624-625: Add missing period in alias bullet.Grammar nit: add a period before “See …”.
-* `npm:@scope/pkg@version` Custom alias for a package See [`package-spec`](/using-npm/package-spec#aliases) +* `npm:@scope/pkg@version` Custom alias for a package. See [`package-spec`](/using-npm/package-spec#aliases)
1138-1141: Fix typos/grammar in devEngines note.“differening” ➜ “different”; “that” ➜ “than”. Tighten phrasing.
-> Note: `engines` and `devEngines` differ in object shape. They also function very differently. `engines` is designed to alert the user when a dependency uses a differening npm or node version that the project it's being used in, whereas `devEngines` is used to alert people interacting with the source code of a project. +> Note: `engines` and `devEngines` differ in object shape. They also function very differently. `engines` is designed to alert the user when a dependency uses a different npm or node version than the project it's used in, whereas `devEngines` alerts people interacting with the project's source code.
749-751: Correct “is ran” to “is run”.-*note*: Packages linked by local path will not have their own -dependencies installed when `npm install` is ran in this case. You must +*note*: Packages linked by local path will not have their own +dependencies installed when `npm install` is run in this case. You must run `npm install` from inside the local path itself.configure.py (2)
371-375: Nit: help text says "tcp2" instead of "ngtcp2"Minor typo in option help.
Apply:
- help='a directory to search for the shared tcp2 DLLs') + help='a directory to search for the shared ngtcp2 DLLs')
376-381: Nit: "OpenSSl" capitalizationShould be "OpenSSL".
Apply:
- help='link to a shared OpenSSl DLL instead of static linking') + help='link to a shared OpenSSL DLL instead of static linking')deps/npm/docs/output/commands/npm-version.html (1)
156-161: Typo: "alias: verison"Spelling mistake in displayed content.
If this file is auto-generated from npm upstream, consider reporting upstream rather than patching locally.
- alias: verison + alias: versiondeps/npm/man/man1/npm-ls.1 (1)
262-273: Duplicate “npm help explain” in See Also.Minor doc nit: “npm help explain” appears twice in this list.
Apply this diff to remove the duplicate:
@@ npm help npmrc npm help folders -npm help explain npm help install npm help link npm help prune npm help outdated npm help updatedeps/npm/docs/output/commands/npm-update.html (1)
159-159: Typo in aliases: “udpate” → “update”.Public-facing doc; fix the misspelling.
-aliases: up, upgrade, udpate +aliases: up, upgrade, updatedeps/npm/man/man1/npm-doctor.1 (1)
32-32: Grammar nit: “are ran” → “are run”.- You can limit what checks are ran by specifying them as extra arguments. + You can limit which checks are run by specifying them as extra arguments.deps/npm/man/man5/package-json.5 (1)
592-592: Typo Fix Needed: “pacakge” → “package” in SPDX Test SchemaThe typo “pacakge” was found in the SPDX JSON schema tests, not in the manpages. Please correct the following occurrences:
- deps/npm/test/schemas/spdx/spdx-2.3.schema.json, line 369
Change:- filesAnalyzed property for that same pacakge is true or omitted + filesAnalyzed property for that same package is true or omitted- deps/npm/test/schemas/spdx/spdx-2.3.schema.json, line 372
Change:- same pacakge is true or omitted + same package is true or omitteddeps/inspector_protocol/crdtp/README.md (1)
54-57: Prefer distro-agnostic install guidance for the C++ stdlib headers.Pinning to libstdc++-14-dev may go stale or differ across distros. Suggest using the generic package name and adding notes for alternatives.
Apply this diff:
-You'll probably also need to install libstdc++, since Clang uses this to find the +You'll probably also need to install the C++ standard library headers (libstdc++), since Clang uses this to find the standard C++ headers. E.g., - sudo apt-get install libstdc++-14-dev + # Debian/Ubuntu + sudo apt-get install libstdc++-dev + # Fedora + sudo dnf install libstdc++-devel + # Arch + sudo pacman -S gcc-libs + # macOS: install Xcode Command Line Tools (xcode-select --install).github/workflows/codeql.yml (1)
30-40: CodeQL action bump to v3.29.2 LGTM; ensure triggers cover PRs, not just the daily cron.Right now it runs only on schedule. If you intend to scan PRs/merges, add push/pull_request triggers.
Apply this diff if desired:
on: - schedule: - - cron: 0 0 * * * + schedule: + - cron: 0 0 * * * + pull_request: + branches: [ "**" ] + push: + branches: [ "**" ]deps/npm/man/man1/npm-ci.1 (1)
10-10: Typo in aliases: “isntall-clean” → “install-clean”.
Minor user-facing nit in the man page aliases list.-aliases: clean-install, ic, install-clean, isntall-clean +aliases: clean-install, ic, install-clean, install-cleandeps/inspector_protocol/crdtp/cbor.h (4)
258-260: Clarified precondition for GetString16WireRep() is good; consider a debug-time assert.
Nice to spell out TokenTag() == STRING16 and even-length guarantee. Recommend adding a DCHECK/ASSERT in the implementation to enforce the invariant at runtime in debug builds.// In GetString16WireRep() implementation (not this header): DCHECK_EQ(ret.size() % 2, 0u); // ensure even length for UTF-16 wire bytes
49-56: Nit: “leightweight” → “lightweight”.-// Performs a leightweight check of |msg|. +// Performs a lightweight check of |msg|.
115-121: Nit: fix “byte sisze” typo.- // byte sisze so that encoding can continue. + // byte size so that encoding can continue.
227-233: Nit: “in which ase” → “in which case”.- // or ERROR_VALUE (in which ase ::Status() has details) + // or ERROR_VALUE (in which case ::Status() has details).github/workflows/scorecard.yml (2)
38-38: Flip egress-policy to block after soakOnce you've validated needed endpoints, switch audit→block to actually enforce egress restrictions.
- egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + egress-policy: block
76-79: Label SARIF uploads to de-dupe in Code ScanningAdd a category so Scorecard results are grouped distinctly from CodeQL.
with: sarif_file: results.sarif + category: ossf-scorecarddeps/npm/lib/utils/verify-signatures.js (1)
26-31: Tiny hardening: cache or friendlier error on dynamic importIf run() can be called multiple times, cache the imported module or surface a clearer failure if p-map is missing.
Example:
let _pMap; async run () { if (!_pMap) ({ default: _pMap } = await import('p-map').catch((e) => { e.message = `Failed to load 'p-map' (ESM). Ensure deps are present. ${e.message}`; throw e; })); const pMap = _pMap; // ... }.github/CODEOWNERS (1)
220-225: Add Inspector test patterns to CODEOWNERS (optional)We verified that there are many Inspector-related tests under both
test/parallelandtest/sequential, but no standalonetest/inspector/directory. To ensure the Inspector team is notified on test-only changes, please consider adding the following entries to .github/CODEOWNERS under the Inspector section:# Inspector /src/inspector/* @nodejs/inspector /src/inspector_* @nodejs/inspector /lib/internal/inspector/* @nodejs/inspector /lib/internal/inspector_* @nodejs/inspector /lib/inspector.js @nodejs/inspector + /test/parallel/test-inspector-* @nodejs/inspector + /test/sequential/test-inspector-* @nodejs/inspectorSince there isn’t currently a top-level
test/inspector/folder, the more general catch-all isn’t necessary right now—but feel free to add it later if such a directory is introduced.benchmark/source_map/source-map.js (2)
51-59: Avoid potential flakiness in findEntry/findOrigin assertions.Using i, i as generated coordinates may exceed the actual mapping ranges of the provided payloads; depending on implementation details, findEntry/findOrigin could return undefined/null on the last iteration, failing the assert. Consider constraining the queried positions or using stable known-good coordinates from the fixture to ensure determinism.
Example change (use bounded coords while preserving variation):
- for (let i = 0; i < n; i++) { - sourceMapMethod = sourceMap.findEntry(i, i); - } + for (let i = 0; i < n; i++) { + // Keep coords within a small, likely-covered range + sourceMapMethod = sourceMap.findEntry((i & 0x3F) + 1, (i & 0x1F)); + }Apply the same pattern to findEntry-sectioned/findOrigin/findOrigin-sectioned, or switch to fixed known-good coords such as (1, 0) if acceptable for this benchmark.
Also applies to: 61-69, 71-79, 81-89
32-41: Minor: reduce duplicate setup to cut noise outside the timed region.You can hoist bench.start() and bench.end() usage into helpers and keep constructor calls inside the timed window for both parse variants; not critical, but it reduces visual duplication across cases.
Also applies to: 43-50
benchmark/source_map/source-map-cache.js (1)
46-48: Replace eval with vm.runInThisContext to avoid global eval risks.This is a benchmark, but using eval is unnecessary and flagged by linters. vm.runInThisContext preserves filename attribution and works with sourceURL/sourceMappingURL.
Apply:
- eval(sourceWithGeneratedSourceMap); + vm.runInThisContext(sourceWithGeneratedSourceMap, { filename: generatedFileName });And add the import:
- const Module = require('node:module'); + const Module = require('node:module'); + const vm = require('node:vm');deps/amaro/lib/wasm.d.ts (1)
4-7: Export public types directly to simplify consumption and IDE discoverability.Re-exporting local declarations via
export type { ... }works, but directly exporting the declarations (and Mode) is clearer and more ergonomic for consumers.Apply:
-export declare function transform(src: string | Uint8Array, opts?: Options): Promise<TransformOutput>; -export declare function transformSync(src: string | Uint8Array, opts?: Options): TransformOutput; -export type { Options, TransformOutput }; +export declare function transform(src: string | Uint8Array, opts?: Options): Promise<TransformOutput>; +export declare function transformSync(src: string | Uint8Array, opts?: Options): TransformOutput; @@ -interface Options { +export interface Options { @@ -interface TransformConfig { +export interface TransformConfig { @@ -type Mode = "strip-only" | "transform"; +export type Mode = "strip-only" | "transform"; @@ -interface TransformOutput { +export interface TransformOutput {Also applies to: 10-17, 19-46, 50-57
deps/amaro/package.json (1)
31-36: Build pipeline: consider emitting types without skipping checks.
tsc --noCheckspeeds builds but can mask declaration drift. If possible, runnpm run typecheckin CI to catch type regressions.GOVERNANCE.md (8)
104-110: Clarify public/private split and add privacy linkGood addition. Consider linking to a canonical privacy/data-handling policy and defining “sensitive topics” with examples to reduce ambiguity. Also state who decides when a topic moves to private.
113-115: Tone and intent are clear; minor grammar polishCurrent phrasing is fine; optional concision below.
- patches on GitHub. The preference is to minimize the need for TSC meetings to - make decisions that can otherwise be made by collaborators on GitHub. + patches on GitHub. Prefer minimizing TSC decisions that collaborators can make on GitHub.
124-126: Non-voting participants: define scope brieflyAdd a short note on whether non-voting invitees may speak, and how conflicts of interest are handled.
127-131: Set a publication SLA for public minutesRecommend adding a timeframe (e.g., “publish within 7 days”) and a fallback assignee if the chair is unavailable.
- public pull request with the minutes from the public portion of the meeting. + public pull request with the minutes from the public portion of the meeting within 7 days.
139-145: Private summaries: grammar and approval flowMinor grammar and clarity improvements.
- produces a summary of the discussions, gets it reviewed by the attendees, - and shares it to all the TSC members once approved by the attendees via a + produces a summary of the discussions, gets it reviewed by the attendees, + and shares it with all TSC members once approved by the attendees via a private discussion channel such as the TSC private mailing list. The summary - may be made public if there is consensus within the TSC and the non-TSC - attendees to make it public. + may be made public if there is consensus among the TSC and non-TSC attendees.
151-154: Decision semantics: avoid misinterpretationReplace informal “folks” and remove ambiguity about “provisional.”
- All discussions made during meetings are considered provisional, receiving no - objections from folks at the TSC meeting to take an action is not equivalent to - the TSC endorsing that action. + All discussions during meetings are considered provisional. The absence of objections + to taking an action does not constitute TSC endorsement of that action.
155-161: Voting: define notification channel and windowSpecify where “informed” happens and that 48 hours is measured in UTC; small grammar fixes.
- The decision is considered confirmed once the rest of the TSC voting members have - been informed and no objection for taking that vote has been raised in 48 hours. - To clarify, TSC voting members can object to the vote taking place during the - meeting, but not to the vote itself. + The decision is confirmed once the remaining TSC voting members have been informed + via <defined channel, e.g., private mailing list> and no objection to taking that vote + is raised within 48 hours (UTC). To clarify: TSC voting members may object to taking + the vote during the meeting, but not to the vote’s outcome.
162-173: Public-issue flow: punctuation and optionsMinor punctuation; explicitly title the two paths.
- The process for public issues in the issue tracker is: + The process for public issues in the issue tracker is:And consider introducing a short sublist naming the two options: “(A) Add to TSC agenda” and “(B) Motion for a vote.”
deps/amaro/README.md (9)
4-4: Add missing noun for clarity“as an internal” reads awkwardly.
-It's used as an internal in Node.js for [Type Stripping](https://nodejs.org/api/typescript.html#type-stripping) but can also be used as a standalone package. +It's used as an internal tool in Node.js for [Type Stripping](https://nodejs.org/api/typescript.html#type-stripping) but can also be used as a standalone package.
8-9: Preposition fix and article usageImprove flow and correctness.
-This package provides a stable API for the TypeScript parser and allows users to upgrade to the latest version of TypeScript transpiler independently from the one used internally in Node.js. +This package provides a stable API for the TypeScript parser and allows users to upgrade to the latest version of the TypeScript transpiler independently of the version used internally in Node.js.
33-36: Loader flags: unify product naming and call out consent requirementStandardize “Node.js v23+” and make the default explicit.
-In node v23 and later you can omit the `--experimental-strip-types` flag, as it is enabled by default. +In Node.js v23 and later, you can omit the `--experimental-strip-types` flag because type stripping is enabled by default.
38-45: Consistency between strip and transform loader examplesAdd symmetrical phrasing for both examples for easier scanning.
-node --experimental-strip-types --import="amaro/strip" file.ts +node --experimental-strip-types --import="amaro/strip" file.ts ... -node --experimental-transform-types --import="amaro/transform" file.ts +node --experimental-transform-types --import="amaro/transform" file.tsAdditionally, consider adding ESM example (node --import) vs CJS (node -r) if applicable.
50-53: Behavior difference vs core: note potential performance implicationsSince Amaro processes TS under node_modules when used as a loader, suggest a short caution about startup overhead and how to exclude paths if needed.
+Note: Processing TypeScript in dependencies can increase startup time. Consider excluding heavy paths with loader options or prebuilding frequently-used packages.
56-67: Monorepo exports: add caveat about dual CJS/ESM and toolingNice section. Consider noting that the “typescript” condition must be consumed by Node.js and that bundlers may ignore it.
+Note: The "typescript" condition is honored by Node.js. Some bundlers and tools may not resolve it unless explicitly configured.
71-73: Command example: add Windows PowerShell variantOptional developer experience improvement for cross-platform users.
+# PowerShell +node --watch --import="amaro/strip" --conditions=typescript .\src\index.ts
75-76: DX note is good; consider watch-mode caveatWhen using --watch, warn about loaders reinitializing on restart.
+When using --watch, Node.js may reinitialize loaders on restart. Ensure your loader has idempotent setup.
79-79: Version wording: specify range or policyIf the intent is “5.8.x,” say so; otherwise link to compatibility policy.
-The supported TypeScript version is 5.8. +Supported TypeScript version: 5.8.x.deps/amaro/lib/wasm_bg.wasm.d.ts (1)
1-12: Mark bindings as internal to avoid locking public APIThese wasm-bindgen symbols are low-level and likely unstable. Add TSDoc @internal and a file header to discourage external usage. Ensure this file isn’t exported via your package’s main “types” entry or exports map.
/* tslint:disable */ /* eslint-disable */ +/** + * Internal wasm-bindgen surface; not a stable API. + * @internal + */ export const memory: WebAssembly.Memory; export const transform: (a: number, b: number) => number; export const transformSync: (a: number, b: number, c: number) => void; export const __wbindgen_export_0: (a: number) => void; export const __wbindgen_export_1: WebAssembly.Table; export const __wbindgen_export_2: (a: number, b: number) => number; export const __wbindgen_export_3: (a: number, b: number, c: number, d: number) => number; export const __wbindgen_add_to_stack_pointer: (a: number) => number; export const __wbindgen_export_4: (a: number, b: number, c: number) => void; export const __wbindgen_export_5: (a: number, b: number, c: number, d: number) => void;Also consider documenting pointer/length semantics for the numeric parameters to aid maintainers.
deps/inspector_protocol/inspector_protocol.gni (1)
34-38: Reasonable defaulting and override for jinja_dir.The conditional assignment is fine. Note: changes to the Jinja installation won’t invalidate the GN action since jinja_dir isn’t listed in inputs, which is acceptable for a tool dep.
deps/inspector_protocol/crdtp/protocol_core_test.cc (1)
361-364: Const-correctness for presence/accessors.Make HasTestTypeBasicField() const to match other getters and allow calling on const instances.
Apply:
- bool HasTestTypeBasicField() { return !!test_type_basic_field_; } + bool HasTestTypeBasicField() const { return !!test_type_basic_field_; }deps/inspector_protocol/crdtp/protocol_core.h (1)
221-235: Explicit std::optional trait assumes presence during Serialize.This is fine since ContainerSerializer(AddField(optional)) guards has_value(). If used elsewhere, value() on empty will UB. Optional: add a debug check to make misuse fail fast.
Apply:
static void Serialize(const std::optional<T>& value, std::vector<uint8_t>* bytes) { - ProtocolTypeTraits<T>::Serialize(value.value(), bytes); + assert(value.has_value()); + ProtocolTypeTraits<T>::Serialize(*value, bytes); }deps/inspector_protocol/templates/TypeBuilder_cpp.template (4)
119-123: Avoid by-value for optional event params to reduce copies.Pass optional/unique_ptr parameters by const reference; serializer doesn’t consume them.
Apply:
- {{protocol.optional_type(parameter)}} + const {{protocol.optional_type(parameter)}}&
218-224: Avoid by-value for optional return params in async sendSuccess.Use const& to prevent copies of optional payloads.
Apply:
- {{protocol.optional_type(parameter)}} {{parameter.name}} + const {{protocol.optional_type(parameter)}}& {{parameter.name}}
253-256: Prefer const& for optional command input members in Params when feasible.If these are stored in the params struct (not just forwarded), keeping by-value is fine. If not mutated, const& reduces copies. Consider generator-level decision based on type size; otherwise, keep as-is for simplicity.
289-293: Avoid by-value for optional out_… variables if backend fills by reference.The backend writes via pointers, so out_… can remain by-value; however, if outputs are large (e.g., vectors/objects), consider std::optional with in-place emplace to avoid extra moves.
deps/inspector_protocol/BUILD.gn (2)
52-73: Mark the crdtp_test target as test-only.Explicitly scope the target.
Apply this diff:
test("crdtp_test") { + testonly = true
91-95: Consider gating the ‘transcode’ tool behind a flag.If this utility isn’t shipped in Node builds, hide it behind a GN arg similar to crdtp_enable_tests.
I can draft the guard if you confirm desired default.
deps/inspector_protocol/templates/TypeBuilder_h.template (1)
102-105: Optional-type accessor looks good; consider non-const overload.Returning a const reference is great for read-only use. If mutation of the wrapper is a common pattern, add a non-const overload.
Example:
const {{protocol.optional_type(property)}}& {{ "get" | to_method_case }}{{property_name}}() const { return {{property_field}}; } + {{protocol.optional_type(property)}}& {{ "get" | to_method_case }}{{property_name}}() { + return {{property_field}}; + }
| const sourceMapUrl = generatedFileName.replace(/\.js$/, '.map'); | ||
| const sourceWithGeneratedSourceMap = | ||
| `${generatedFileContent}\n//# sourceMappingURL=${sourceMapUrl}\n//# sourceURL=${generatedFileName}`; | ||
| const generatedExpectedUrl = `file://${generatedFileName}`; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use file URL utilities for cross-platform correctness (Windows paths).
String-building file:// URLs from paths can be wrong on Windows (needs file:///C:/...). Build URLs via pathToFileURL and also emit an absolute URL in sourceMappingURL.
Apply:
- const sourceMapUrl = generatedFileName.replace(/\.js$/, '.map');
+ const { pathToFileURL } = require('node:url');
+ const sourceMapUrl = pathToFileURL(generatedFileName.replace(/\.js$/, '.map')).href;
@@
- const generatedExpectedUrl = `file://${generatedFileName}`;
+ const generatedExpectedUrl = pathToFileURL(generatedFileName).href;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sourceMapUrl = generatedFileName.replace(/\.js$/, '.map'); | |
| const sourceWithGeneratedSourceMap = | |
| `${generatedFileContent}\n//# sourceMappingURL=${sourceMapUrl}\n//# sourceURL=${generatedFileName}`; | |
| const generatedExpectedUrl = `file://${generatedFileName}`; | |
| const { pathToFileURL } = require('node:url'); | |
| const sourceMapUrl = pathToFileURL(generatedFileName.replace(/\.js$/, '.map')).href; | |
| const sourceWithGeneratedSourceMap = | |
| `${generatedFileContent}\n//# sourceMappingURL=${sourceMapUrl}\n//# sourceURL=${generatedFileName}`; | |
| const generatedExpectedUrl = pathToFileURL(generatedFileName).href; |
🤖 Prompt for AI Agents
In benchmark/source_map/source-map-cache.js around lines 29 to 33, the code
builds file:// URLs by string concatenation which breaks on Windows; replace
that with Node's path-to-file-URL utilities: import path.resolve (or use
path.join) and url.pathToFileURL, compute an absolute filesystem path for
generatedFileName (e.g. path.resolve(generatedFileName)) and call
pathToFileURL(...) to produce a proper file URL, use that absolute file URL for
both the sourceMappingURL embedded into generatedFileContent and for
generatedExpectedUrl; update the string interpolation to insert the
pathToFileURL result instead of manual `file://` strings.
| // If there were no tests to run, bail if we were requested to be strict. | ||
| constexpr char kNoTestsSelectedMessage[] = | ||
| "No tests were selected to run. Please make sure at least one test " | ||
| "exists and is not disabled! If the test is sharded, you may have " | ||
| "defined more shards than test cases, which is wasteful."; | ||
| ColoredPrintf(GTestColor::kRed, "%s\n", kNoTestsSelectedMessage); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Early return skips listeners/teardown and suppresses XML/JSON output.
Returning here occurs after OnTestIterationStart but before OnTestIterationEnd/OnTestProgramEnd, leaking lifecycle symmetry and preventing reporters from flushing artifacts.
Apply:
- ColoredPrintf(GTestColor::kRed, "%s\n", kNoTestsSelectedMessage);
- return false;
+ ColoredPrintf(GTestColor::kRed, "%s\n", kNoTestsSelectedMessage);
+ // Mark run as failed but allow the normal end-of-iteration/program
+ // flow to execute so reporters and listeners complete correctly.
+ failed = true;
+ // Continue to the common epilogue (elapsed time, listeners, etc.).This preserves correct event ordering, elapsed time assignment, XML/JSON emission, and teardown. If you prefer to stop repeating immediately, add a break; after setting failed = true;.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If there were no tests to run, bail if we were requested to be strict. | |
| constexpr char kNoTestsSelectedMessage[] = | |
| "No tests were selected to run. Please make sure at least one test " | |
| "exists and is not disabled! If the test is sharded, you may have " | |
| "defined more shards than test cases, which is wasteful."; | |
| ColoredPrintf(GTestColor::kRed, "%s\n", kNoTestsSelectedMessage); | |
| return false; | |
| } | |
| // If there were no tests to run, bail if we were requested to be strict. | |
| constexpr char kNoTestsSelectedMessage[] = | |
| "No tests were selected to run. Please make sure at least one test " | |
| "exists and is not disabled! If the test is sharded, you may have " | |
| "defined more shards than test cases, which is wasteful."; | |
| ColoredPrintf(GTestColor::kRed, "%s\n", kNoTestsSelectedMessage); | |
| // Mark run as failed but allow the normal end-of-iteration/program | |
| // flow to execute so reporters and listeners complete correctly. | |
| failed = true; | |
| // Continue to the common epilogue (elapsed time, listeners, etc.). | |
| } |
| import("//testing/test.gni") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gate test infra behind a GN arg; move import inside the guard.
Unconditional import of //testing/test.gni will break in repos without Chromium’s testing infra. Wrap test targets and the import behind a toggle so downstream consumers (e.g., Node-only builds) aren’t impacted.
Apply this diff:
+# Toggle to enable building CRDTP tests in non-Chromium trees.
+declare_args() {
+ crdtp_enable_tests = false
+}
-
-import("//testing/test.gni")
+
+# Tests are only available when explicitly enabled.
+if (crdtp_enable_tests) {
+ import("//testing/test.gni")
+}
@@
-# In this (upstream) standalone package, we declare crdtp_test, and a
-# few minimal files in testing (accessed via test_platform.{h,cc}) to
-# make it look like Chromium's testing package. In Chromium,
-# we run these tests as part of the content_unittests, declared in
-# content/test/BUILD.gn, and in V8, we run them via unittests, declared
-# in test/unittests/BUILD.gn.
+# In this (upstream) standalone package, we declare crdtp_test, and a
+# few minimal files in testing (accessed via test_platform.{h,cc}) to
+# make it look like Chromium's testing package.
@@
-test("crdtp_test") {
+if (crdtp_enable_tests) {
+test("crdtp_test") {
@@
-}
+}
@@
-static_library("crdtp_test_platform") {
+static_library("crdtp_test_platform") {
@@
-}
+}
+} # crdtp_enable_testsAlso applies to: 45-50, 52-74, 75-89
🤖 Prompt for AI Agents
In deps/inspector_protocol/BUILD.gn around lines 8-9 (and similarly for 45-50,
52-74, 75-89), the file unconditionally imports //testing/test.gni and defines
test targets, which breaks builds that don't include Chromium testing infra; add
a GN boolean arg (e.g., enable_test_infra or enable_chromium_tests) and wrap the
import and all test target blocks inside an if (enable_test_infra) { ... } guard
so downstream consumers can disable the test infra import and targets by setting
the flag to false.
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Make crdtp_platform a public_dep of crdtp.
Downstream targets (e.g., transcode) depend on :crdtp only. To avoid missing symbols and ensure transitive linkage for this static lib, forward the dependency.
Apply this diff:
- deps = [ ":crdtp_platform" ]
+ public_deps = [ ":crdtp_platform" ]If your toolchain already pulls transitive static libs via deps, this change is harmless; otherwise it prevents link errors.
Forward crdtp_platform as a public dependency of crdtp
To ensure that any target linking against :crdtp also pulls in crdtp_platform (avoiding potential missing-symbol linker errors), update the crdtp static library’s dependency from a private deps to a public one.
• File: deps/inspector_protocol/BUILD.gn
• Location: around lines 34–35
• Change this block:
- deps = [ ":crdtp_platform" ]
+ public_deps = [ ":crdtp_platform" ]This makes :crdtp_platform transitively available to downstream targets that list :crdtp in their deps. Without this, those targets may fail to link if your toolchain doesn’t automatically propagate static‐library dependencies.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| deps = [ ":crdtp_platform" ] | |
| } | |
| public_deps = [ ":crdtp_platform" ] | |
| } |
🤖 Prompt for AI Agents
In deps/inspector_protocol/BUILD.gn around lines 34–35, the crdtp static library
only lists ":crdtp_platform" under private deps which prevents downstream
targets from inheriting that dependency; update the target so ":crdtp_platform"
is a public dependency (e.g., add it to public_deps or replace the private deps
entry with a public_deps entry) so that any target depending on ":crdtp" will
transitively link against crdtp_platform and avoid missing-symbol linker errors.
| void HandleBinary(span<uint8_t> bytes) override { | ||
| AddValueToParent( | ||
| BinaryValue::create(Binary::fromSpan(bytes.data(), bytes.size()))); | ||
| AddValueToParent(BinaryValue::create(Binary::fromSpan(bytes))); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
API migration: Binary::fromSpan now takes span<uint8_t> — OK; verify callsites.
Switching from (data, size) to (span) aligns with span-based APIs in crdtp. Please confirm no remaining Binary::fromSpan(ptr, size) usages elsewhere.
🏁 Script executed:
#!/bin/bash
# Find outdated Binary::fromSpan(ptr, size) style usages
rg -nP 'Binary::fromSpan\s*\(' -C2Length of output: 8349
Update remaining Binary::fromSpan(ptr, size) call sites to use the span overload
The migration in Values_cpp.template was updated correctly, but there are still several call sites across the codebase that invoke the old (const uint8_t* data, size_t size) overload. These must be switched to the new span-based API (Binary::fromSpan(span<uint8_t>)) and the legacy overload removed once all callers are updated.
Please update the following locations:
- src/inspector/node_json.cc:75
Binary::fromSpan(bytes.data(), bytes.size())→Binary::fromSpan(bytes) - deps/v8/third_party/inspector_protocol/lib/Values_cpp.template:91
Binary::fromSpan(bytes.data(), bytes.size())→Binary::fromSpan(bytes) - deps/v8/src/inspector/string-util.cc:62, 68, 77, 83, 89
Replace allBinary::fromSpan(nullptr, 0)andBinary::fromSpan(bin.data(), bin.size())withBinary::fromSpan(span<uint8_t>())(empty span) andBinary::fromSpan(bin), respectively - deps/v8/src/inspector/v8-debugger-agent-impl.cc:1225–1226, 1238, 1407
Change eachprotocol::Binary::fromSpan(ptr, size)toprotocol::Binary::fromSpan(span) - deps/v8/test/unittests/inspector/inspector-unittest.cc:147, 152, 157, 172
UpdateBinary::fromSpan(input, N)calls toBinary::fromSpan(input_span)
These updates will ensure the old overload can be cleanly removed and fully align with the span-based API.
🤖 Prompt for AI Agents
In deps/inspector_protocol/lib/Values_cpp.template around lines 89–91 and in the
other listed files (src/inspector/node_json.cc:75;
deps/v8/third_party/inspector_protocol/lib/Values_cpp.template:91;
deps/v8/src/inspector/string-util.cc:62,68,77,83,89;
deps/v8/src/inspector/v8-debugger-agent-impl.cc:1225–1226,1238,1407;
deps/v8/test/unittests/inspector/inspector-unittest.cc:147,152,157,172) update
all calls that use the old Binary::fromSpan(const uint8_t* data, size_t size)
overload to use the span-based API: replace Binary::fromSpan(bytes.data(),
bytes.size()) with Binary::fromSpan(bytes), replace Binary::fromSpan(bin.data(),
bin.size()) with Binary::fromSpan(bin), replace Binary::fromSpan(nullptr, 0)
with Binary::fromSpan(span<uint8_t>()) and replace
protocol::Binary::fromSpan(ptr, size) or Binary::fromSpan(input, N) with the
appropriate span variable (e.g., span<uint8_t> or existing container) so every
call uses Binary::fromSpan(span<uint8_t>) before removing the legacy overload.
| <li><code>tag</code> A specific version tagged and published as <code>tag</code> See <a href="../commands/npm-dist-tag.html"><code>npm dist-tag</code></a></li> | ||
| <li><code>path/path/path</code> See <a href="#local-paths">Local Paths</a> below</li> | ||
| <li><code>npm:@scope/pkg@version</code> Custom alias for a pacakge See <a href="../using-npm/package-spec#aliases.html"><code>package-spec</code></a></li> | ||
| <li><code>npm:@scope/pkg@version</code> Custom alias for a package See <a href="../using-npm/package-spec#aliases.html"><code>package-spec</code></a></li> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potentially broken link format for package-spec aliases.
Other internal links use “.../file.html#anchor”. This one is “package-spec#aliases.html”.
Consider:
-<a href="../using-npm/package-spec#aliases.html"><code>package-spec</code></a>
+<a href="../using-npm/package-spec.html#aliases"><code>package-spec</code></a>Run to verify target exists and update similar cases:
🏁 Script executed:
#!/bin/bash
# Verify file exists and find mismatched link patterns
fd -a 'package-spec.html' docs output || fd -a 'package-spec.html'
rg -nP '\.\./using-npm/package-spec#aliases\.html' -C2
rg -nP '\.\./using-npm/package-spec\.html#aliases' -C2Length of output: 1861
🏁 Script executed:
#!/bin/bash
ls -1 deps/npm/docs/output/configuring-npmLength of output: 177
🏁 Script executed:
#!/bin/bash
# Check for the existence of the "aliases" anchor in the target file
grep -nE '(id|name)="aliases"' -C2 deps/npm/docs/output/using-npm/package-spec.htmlLength of output: 232
Fix broken anchor links for package-spec aliases
The using-npm/package-spec.html file defines the “aliases” section with an id="aliases" anchor, so links must be in the form package-spec.html#aliases. The current links—package-spec#aliases.html—are invalid and 404’ing.
Please update both occurrences in the configuring-npm docs:
- File:
deps/npm/docs/output/configuring-npm/package-json.html(line 625) - File:
deps/npm/docs/output/configuring-npm/npm-json.html(line 625)
Apply this diff in each file:
- <a href="../using-npm/package-spec#aliases.html"><code>package-spec</code></a>
+ <a href="../using-npm/package-spec.html#aliases"><code>package-spec</code></a>Optional: run the following to catch any remaining instances and verify the correct format exists:
# Find all wrong patterns
rg -lP '\.\./using-npm/package-spec#aliases\.html'
# Confirm correct links
rg -nP '\.\./using-npm/package-spec\.html#aliases' -C2🤖 Prompt for AI Agents
In deps/npm/docs/output/configuring-npm/package-json.html around line 625, the
anchor link to the aliases section is malformed as
"../using-npm/package-spec#aliases.html"; update it to
"../using-npm/package-spec.html#aliases". Do the same in
deps/npm/docs/output/configuring-npm/npm-json.html around line 625 (replace
"../using-npm/package-spec#aliases.html" with
"../using-npm/package-spec.html#aliases"). After editing, run the suggested
ripgrep checks to ensure no remaining occurrences of the incorrect pattern and
that the corrected pattern exists.
| <h1 id="----config----1093"> | ||
| <span>config</span> | ||
| <span class="version">@10.9.2</span> | ||
| <span class="version">@10.9.3</span> | ||
| </h1> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Docs metadata bump looks good; please ensure cross-page consistency.
Anchor id and version updated (1093, @10.9.3). Suggest verifying no stale 1092 anchors or @10.9.2 badges remain and that cross-links target the new anchors.
🏁 Script executed:
#!/bin/bash
# 1) Find any leftover old anchor ids or version badges.
rg -nP '----\w+----1092|@10\.9\.2' deps/npm/docs/output
# 2) Sanity-check for misordered hash+extension anchors (e.g., config#timing.html).
rg -nP '\#[A-Za-z0-9_-]+\.html' deps/npm/docs/output
# 3) Confirm npm vendored version matches docs badge (expect 10.9.3).
fd package.json deps/npm -x jq -r '.version' {} 2>/dev/nullLength of output: 9017
I’ve updated the checks to avoid the ripgrep option parsing issue and to pinpoint the root package.json. Please re-run and share the outputs:
🏁 Script executed:
#!/bin/bash
set -eo pipefail
# 1) Find any leftover old anchor ids (----…----1092) or version badges (@10.9.2)
rg -nP -e '----\w+----1092|@10\.9\.2' deps/npm/docs/output
# 2) Sanity‐check for misordered hash+extension anchors in using-npm/config pages
rg -nP 'using-npm/config#[A-Za-z0-9_-]+\.html' -n deps/npm/docs/output
# 3) Confirm vendored npm version matches docs badge (expect 10.9.3)
jq -r '.version' deps/npm/package.jsonLength of output: 79
I’ve adjusted the script so each grep can fail without aborting, and it will explicitly print each step. Please rerun and share the full output:
🏁 Script executed:
#!/bin/bash
set -eo pipefail
echo "1) Searching for stale anchor ids or version badges…"
rg -nP -e '----\w+----1092|@10\.9\.2' deps/npm/docs/output || echo " ✔ no stale 1092 anchors or @10.9.2 badges"
echo "2) Checking for misordered hash+extension anchors in using-npm/config…"
rg -nP 'using-npm/config#[A-Za-z0-9_-]+\.html' -n deps/npm/docs/output || echo " ✔ no misordered .html anchors"
echo "3) Vendored npm version (should be 10.9.3):"
jq -r '.version' deps/npm/package.jsonLength of output: 6221
🏁 Script executed:
#!/bin/bash
set -eo pipefail
echo "Inspecting deps/npm/docs/output/using-npm/config.html for anchor IDs"
for anchor in workspace registry timing; do
echo "Checking id for anchor '$anchor'"
rg -nP -C1 "id=\"${anchor}(\\.html)?\"" -n deps/npm/docs/output/using-npm/config.html || echo " ✖ no exact id match for $anchor(.html)?"
doneLength of output: 1195
Fix misordered .html suffix in cross-page config anchors
The metadata bump to 10.9.3 is correct and there are no remaining 1092 anchors or @10.9.2 badges, and your vendored version matches (10.9.3). However, I found that several cross-page links still include a trailing .html in their fragment identifiers even though the target IDs do not:
• deps/npm/docs/output/using-npm/workspaces.html:213
<a href="../using-npm/config#workspace.html">…</a> → should be …#workspace
• deps/npm/docs/output/using-npm/registry.html:173
<a href="../using-npm/config#registry.html">…</a> → should be …#registry
• deps/npm/docs/output/using-npm/package-spec.html:172,192
…#registry.html → …#registry
• deps/npm/docs/output/using-npm/logging.html:195
…#timing.html → …#timing
• (and similar instances throughout the using-npm/* pages)
Action items:
- Globally search for
href="../using-npm/config#*\.html"indeps/npm/docs/outputand remove the.htmlsuffix from each fragment. - Rebuild and spot-check that each link correctly targets the new IDs (e.g.
#workspace,#registry,#timing).
🤖 Prompt for AI Agents
In deps/npm/docs/output/using-npm/config.html around lines 144-147 and across
deps/npm/docs/output/using-npm/*, several cross-page links include a trailing
".html" in fragment identifiers (e.g.
href="../using-npm/config#workspace.html"); search the repo for
href="../using-npm/config#*.html" and remove the ".html" suffix from each
fragment (so "#workspace.html" → "#workspace", "#registry.html" → "#registry",
"#timing.html" → "#timing", etc.), save the updated files, rebuild the docs, and
spot-check the updated links to confirm they target the correct IDs.
| @@ -1,4 +1,4 @@ | |||
| .TH "NPM-CI" "1" "December 2024" "NPM@10.9.2" "" | |||
| .TH "NPM-CI" "1" "June 2025" "NPM@10.9.3" "" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Doc metadata bump looks correct (June 2025 / NPM@10.9.3). Please verify repo-wide consistency.
Looks good. To avoid mixed versions/anchors lingering elsewhere, run the checks below.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Check for stale npm doc versions (@10.9.2):"
rg -n '@10\.9\.2' deps/npm || echo "OK: no stale @10.9.2 references"
echo
echo "2) Inspect manpage .TH lines (date/version summary):"
rg -n '^\.TH "NPM-[^"]+" "1" "[A-Za-z]+ [0-9]{4}" "NPM@[0-9.]+"' deps/npm/man/man1 \
| sed -E 's/.*"NPM-([^"]+)".*"([A-Za-z]+ [0-9]{4})".*"(NPM@[0-9.]+)".*/\1\t\2\t\3/' \
| sort
echo
echo "3) Check for old heading anchors (…1092) in generated HTML:"
rg -n '----[a-z0-9-]+----1092' deps/npm/docs/output || echo "OK: no old 1092 anchors"Length of output: 2981
Doc metadata bump confirmed for June 2025 / NPM@10.9.3
All manpage headings under deps/npm/man/man1 now uniformly show “June 2025” and “NPM@10.9.3,” and there are no lingering references to @10.9.2. The only issue is that the third grep command in the verification script misinterprets the pattern as an option (because it begins with hyphens). Please update that check as follows:
• No stale @10.9.2 references
• All .TH lines list June 2025 / NPM@10.9.3
• Fixed old-anchor check:
- rg -n '----[a-z0-9-]+----1092' deps/npm/docs/output || echo "OK: no old 1092 anchors"
+ rg -n -e '----[a-z0-9-]\+----1092' deps/npm/docs/output || echo "OK: no old 1092 anchors"
# –or–
+ rg -n -- '----[a-z0-9-]\+----1092' deps/npm/docs/output || echo "OK: no old 1092 anchors"With that update, all three consistency checks will run correctly.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In deps/npm/man/man1/npm-ci.1 around line 1, the verification script’s third
grep is misinterpreting its pattern as an option because it starts with a
hyphen; update that check to stop option parsing (for example use grep --
'<pattern>' or grep -e '<pattern>' and quote/escape the leading hyphen) so the
old-anchor check runs correctly, and ensure the script asserts no stale
'@10.9.2' references and that all .TH lines list "June 2025" and "NPM@10.9.3".
| The public portion of the TSC meeting is expected to be recorded and made | ||
| available for live streaming during the meeting or download by anyone after. | ||
| This expectation is to be announced to all participants at the start of the | ||
| each meeting before the recording is started. Continued participation in the | ||
| public portion of the meeting after this announcement is interpreted as consent to the | ||
| recording. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consent and wording fixes for recording/streaming
Tighten grammar and make consent explicit before recording starts.
- This expectation is to be announced to all participants at the start of the
- each meeting before the recording is started. Continued participation in the
- public portion of the meeting after this announcement is interpreted as consent to the
- recording.
+ Announce this at the start of each meeting before recording. Recording must not begin
+ until the announcement is made. Continued participation after the announcement
+ constitutes consent to recording.Also consider adding: “Side-channel chats (e.g., Zoom chat/DMs) are not recorded or published.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The public portion of the TSC meeting is expected to be recorded and made | |
| available for live streaming during the meeting or download by anyone after. | |
| This expectation is to be announced to all participants at the start of the | |
| each meeting before the recording is started. Continued participation in the | |
| public portion of the meeting after this announcement is interpreted as consent to the | |
| recording. | |
| The public portion of the TSC meeting is expected to be recorded and made | |
| available for live streaming during the meeting or download by anyone after. | |
| Announce this at the start of each meeting before recording. Recording must not begin | |
| until the announcement is made. Continued participation after the announcement | |
| constitutes consent to recording. |
🧰 Tools
🪛 LanguageTool
[grammar] ~135-~135: There might be a mistake here.
Context: ... started. Continued participation in the public portion of the meeting after this...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...ncement is interpreted as consent to the recording. For the private portion of t...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In GOVERNANCE.md around lines 132 to 138, tighten the grammar and make consent
explicit by changing the paragraph so it clearly states that recording/streaming
will begin only after an announced consent notice is read aloud at the start of
the public portion and that remaining in the meeting constitutes consent; reword
sentences for clarity and concision (remove redundant words like "the each
meeting"), and add a final sentence stating: "Side-channel chats (e.g., Zoom
chat/DMs) are not recorded or published." Ensure the updated wording explicitly
instructs the chair to announce the recording and consent requirement before
starting the recording.
| Recording the private portion of a meeting or maintaining or publishing a | ||
| detailed transcript is only permitted when all participants present during the | ||
| private portion of the meeting explicitly agree to the recording and/or | ||
| transcript, in order to comply to privacy regulations. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
“Comply to” → “comply with”; explicit consent language
Align wording with standard legal phrasing and emphasize explicit consent.
- Recording the private portion of a meeting or maintaining or publishing a
- detailed transcript is only permitted when all participants present during the
- private portion of the meeting explicitly agree to the recording and/or
- transcript, in order to comply to privacy regulations.
+ Recording the private portion of a meeting, or maintaining/publishing a detailed
+ transcript, is permitted only when all participants present explicitly agree,
+ to comply with applicable privacy regulations.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Recording the private portion of a meeting or maintaining or publishing a | |
| detailed transcript is only permitted when all participants present during the | |
| private portion of the meeting explicitly agree to the recording and/or | |
| transcript, in order to comply to privacy regulations. | |
| Recording the private portion of a meeting, or maintaining/publishing a detailed | |
| transcript, is permitted only when all participants present explicitly agree, | |
| to comply with applicable privacy regulations. | |
🧰 Tools
🪛 LanguageTool
[grammar] ~146-~146: There might be a mistake here.
Context: ...a meeting or maintaining or publishing a detailed transcript is only permitted wh...
(QB_NEW_EN)
[grammar] ~147-~147: There might be a mistake here.
Context: ...when all participants present during the private portion of the meeting explicitl...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ...explicitly agree to the recording and/or transcript, in order to comply to privac...
(QB_NEW_EN)
[style] ~149-~149: Consider a more concise word here.
Context: ...ree to the recording and/or transcript, in order to comply to privacy regulations. All dis...
(IN_ORDER_TO_PREMIUM)
🤖 Prompt for AI Agents
In GOVERNANCE.md around lines 146 to 150, replace the phrase “comply to privacy
regulations” with the correct legal phrasing “comply with privacy regulations”
and tighten the consent wording to emphasize explicit, informed agreement (e.g.,
change “explicitly agree to the recording and/or transcript” to “explicitly and
knowingly consent to the recording and/or transcript” or similar), so the
sentence reads clearly and uses standard legal terminology.
Summary by CodeRabbit