From 3c3ea269a946a18adc14f3553e2cd57c242130bd Mon Sep 17 00:00:00 2001 From: quanticsoul4772 <71096528+quanticsoul4772@users.noreply.github.com> Date: Fri, 16 Oct 2026 15:28:39 -0700 Subject: [PATCH 1/2] fix(setup): stop Node's DEP0190 warning when npm runs on Windows (#1057) Windows runs npm.cmd through a shell, and passing an argument array with `shell: true` is deprecated in Node 24 (DEP0190). Every `npm run doctor` and every Pinokio install on Windows printed the warning. npmProcessSpec now takes the npm arguments and, on Windows, joins them into the command string; the arguments are fixed subcommands (`--version`, `ci`). Unix behavior is unchanged. --- changelog.d/1057-windows-npm-dep0190.md | 4 ++++ scripts/pinokio-install.mjs | 4 ++-- scripts/setup-doctor.mjs | 17 +++++++++++------ src/setupDoctor.test.mjs | 18 ++++++++++++++---- 4 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 changelog.d/1057-windows-npm-dep0190.md diff --git a/changelog.d/1057-windows-npm-dep0190.md b/changelog.d/1057-windows-npm-dep0190.md new file mode 100644 index 000000000..1e8e1da55 --- /dev/null +++ b/changelog.d/1057-windows-npm-dep0190.md @@ -0,0 +1,4 @@ +- On Windows, `npm run doctor` and the Pinokio install no longer print Node's + DEP0190 warning ("Passing args to a child process with shell option true can + lead to security vulnerabilities"): npm.cmd's fixed subcommand is now part of + the shell command string instead of an argument array (quanticsoul4772, #1057). diff --git a/scripts/pinokio-install.mjs b/scripts/pinokio-install.mjs index 01a26ccce..4e52bf3af 100644 --- a/scripts/pinokio-install.mjs +++ b/scripts/pinokio-install.mjs @@ -24,8 +24,8 @@ export function runChecked(command, args, { shell = false } = {}) { export function installPinokioDependencies() { applyPinokioEnvironment(); rmSync(READY_FILE, { force: true }); - const npm = npmProcessSpec(); - runChecked(npm.command, ['ci'], { shell: npm.shell }); + const npm = npmProcessSpec(['ci']); + runChecked(npm.command, npm.args, { shell: npm.shell }); // Pinokio starts Vite directly and loads only its ENVIRONMENT file plus the // normal dotenv ladder. Unlike dev-fresh.sh, it does not import macOS diff --git a/scripts/setup-doctor.mjs b/scripts/setup-doctor.mjs index 49c6722c2..01e048091 100644 --- a/scripts/setup-doctor.mjs +++ b/scripts/setup-doctor.mjs @@ -76,10 +76,15 @@ export function hasRequiredDependencies(rootDir = ROOT) { } } -/** Return the npm command and spawn mode required by the target platform. */ -export function npmProcessSpec(platform = process.platform) { - const windows = platform === 'win32'; - return { command: windows ? 'npm.cmd' : 'npm', shell: windows }; +/** + * Return the npm command, arguments and spawn mode for the target platform. + * Windows must run npm.cmd through a shell, and Node 24 deprecates passing an + * argument array with `shell: true` (DEP0190), so there the arguments, which + * are fixed npm subcommands, join the command string. + */ +export function npmProcessSpec(args, platform = process.platform) { + if (platform !== 'win32') return { command: 'npm', args, shell: false }; + return { command: ['npm.cmd', ...args].join(' '), args: [], shell: true }; } /** Read one key from Vite's dotenv file ladder without depending on Vite. */ @@ -190,8 +195,8 @@ export function buildCapabilitySummary( export function inspectSetup({ includeKeychain = true, authoritativeEnvironment = false, rootDir = ROOT } = {}) { const node = classifyNodeVersion(); - const npm = npmProcessSpec(); - const npmResult = spawnSync(npm.command, ['--version'], { + const npm = npmProcessSpec(['--version']); + const npmResult = spawnSync(npm.command, npm.args, { encoding: 'utf8', shell: npm.shell, }); diff --git a/src/setupDoctor.test.mjs b/src/setupDoctor.test.mjs index 71c6e44af..26d695ba1 100644 --- a/src/setupDoctor.test.mjs +++ b/src/setupDoctor.test.mjs @@ -69,12 +69,22 @@ test('placeholder values are never counted as configured credentials', () => { }); test('doctor selects a Windows-safe npm process without changing Unix behavior', () => { - assert.deepEqual(npmProcessSpec('win32'), { - command: 'npm.cmd', + // Windows passes no argument array with shell: true (Node DEP0190). + assert.deepEqual(npmProcessSpec(['ci'], 'win32'), { + command: 'npm.cmd ci', + args: [], shell: true, }); - assert.deepEqual(npmProcessSpec('darwin'), { command: 'npm', shell: false }); - assert.deepEqual(npmProcessSpec('linux'), { command: 'npm', shell: false }); + assert.deepEqual(npmProcessSpec(['--version'], 'darwin'), { + command: 'npm', + args: ['--version'], + shell: false, + }); + assert.deepEqual(npmProcessSpec(['ci'], 'linux'), { + command: 'npm', + args: ['ci'], + shell: false, + }); }); test('doctor recognizes every OpenSky OAuth keychain alias used by dev-fresh', () => { From fb1088984178fe29ea4d3b53035d2393cb37d695 Mon Sep 17 00:00:00 2001 From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:34:26 -0400 Subject: [PATCH 2/2] fix(street-level): release superseded and unmounted Google viewer work (#1117) A superseded open wait cleared the shared cancelShown slot that belonged to the newer wait, leaving its auth listener and timer alive after close; open() now settles the previous wait first. A lookup whose viewer unmounted while the Street View service loaded still sent a billed getPanorama; describe() now rechecks the lookup after acquisition. --- changelog.d/1117-google-viewer-lifecycle.md | 3 + docs/CURRENT-STATE.md | 5 +- .../streetLevel/providers/google/viewer.js | 3 + .../providers/google/viewer.test.mjs | 66 +++++++++++++++++++ 4 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 changelog.d/1117-google-viewer-lifecycle.md diff --git a/changelog.d/1117-google-viewer-lifecycle.md b/changelog.d/1117-google-viewer-lifecycle.md new file mode 100644 index 000000000..66d73d4a2 --- /dev/null +++ b/changelog.d/1117-google-viewer-lifecycle.md @@ -0,0 +1,3 @@ +- Release superseded Google Street View open waits and suppress metadata + requests whose viewer was unmounted while the service loaded (rudycelekli, + #1117). diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index f1c04711e..f425d6ebe 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -3465,7 +3465,10 @@ controls (the address goes in the caption); a pose goes out at once and again when the panorama's month, photographer and address arrive (one successful lookup per panorama per session, seeded by the nearest lookup; a details lookup that fails or takes over ten seconds waits five seconds -before a view event can retry, and an unmount drops lookups in flight). A key +before a view event can retry, and an unmount drops lookups in flight). Opening +another panorama settles the previous open before registering its wait; closing +then releases the current wait immediately. A details lookup released while its +service loads makes no panorama request after that service becomes available. A key Google refuses (`gm_authFailure`: the API not enabled, a referrer it does not allow, billing off) reads KEY REJECTED and fails a pending open at once; a lookup or open Google never answers fails after 10 or 20 s. In CI diff --git a/src/layers/streetLevel/providers/google/viewer.js b/src/layers/streetLevel/providers/google/viewer.js index 422f52c0c..25e0b08fe 100644 --- a/src/layers/streetLevel/providers/google/viewer.js +++ b/src/layers/streetLevel/providers/google/viewer.js @@ -97,6 +97,8 @@ export function createGoogleViewer({ loader, getService, render } = {}) { service ||= getService ? await getService() : new (await ensureLibrary()).StreetViewService(); + if (!current()) + throw new Error('Street View details lookup was released'); return service.getPanorama({ pano: panoId }); })().then(resolve, reject); }); @@ -236,6 +238,7 @@ export function createGoogleViewer({ loader, getService, render } = {}) { const panoId = String(imageId); if (!host) throw new Error('Street View is not mounted'); if (loader.authFailed()) throw new Error(KEY_REJECTED_MESSAGE); + cancelShown?.(); pendingOpen = panoId; const instance = await ensurePanorama(host); if (pendingOpen !== panoId) return; diff --git a/src/layers/streetLevel/providers/google/viewer.test.mjs b/src/layers/streetLevel/providers/google/viewer.test.mjs index a3d746a34..e1d4a187c 100644 --- a/src/layers/streetLevel/providers/google/viewer.test.mjs +++ b/src/layers/streetLevel/providers/google/viewer.test.mjs @@ -403,3 +403,69 @@ test('a remount looks up again a panorama whose lookup hung', async () => { await settle(); assert.equal(poses.length, count); }); + +test('superseded panorama waits cannot steal the current close cleanup', async (t) => { + t.mock.timers.enable({ apis: ['setTimeout'] }); + const { OPEN_TIMEOUT_MS } = await import('./policy.js'); + const flush = async () => { + for (let i = 0; i < 10; i++) await Promise.resolve(); + }; + const { fake, loader, viewer, host } = setup(); + await viewer.mount(host); + const [panorama] = fake.built; + panorama.setPano = (id) => { + panorama.pano = id; + }; + const first = viewer.open('pano-a'); + await flush(); + const second = viewer.open('pano-b'); + await flush(); + panorama.fire('position_changed'); // new panorama has not reached OK + await flush(); + viewer.close(); + const remaining = loader.listenerCount(); + t.mock.timers.tick(OPEN_TIMEOUT_MS); + await Promise.allSettled([first, second]); + assert.equal( + remaining, + 0, + 'close disposes every pending open without waiting 20 seconds', + ); + assert.equal(panorama.visible, false); +}); + +test('unmount during details-service acquisition makes no later billed lookup', async () => { + const fake = fakeStreetViewLibrary({ panoramas: PANORAMAS }); + const loader = fakeMapsLoader(fake.library); + const { createGoogleProvider } = await import('./index.js'); + let release; + const acquiring = new Promise((resolve) => { + release = resolve; + }); + const importLibrary = loader.importLibrary.bind(loader); + let imports = 0; + loader.importLibrary = async (name) => { + if (++imports > 1) await acquiring; + return importLibrary(name); + }; + const provider = createGoogleProvider({ + getApiKey: () => 'fixture-key', + loader, + }); + const instance = provider.create({ + notify() {}, + actions: { reportError() {} }, + }); + const viewer = instance.viewer; + await viewer.mount(fakeHost()); + await viewer.open('pano-a'); + viewer.unmount(); + release(); + for (let i = 0; i < 10; i++) await Promise.resolve(); + assert.equal( + fake.lookups.length, + 0, + 'teardown invalidates the request before calling getPanorama', + ); + instance.destroy(); +});