Skip to content

Commit d3a04cd

Browse files
committed
fix: address PR review findings
- .lintstagedrc.mjs: match ignore patterns against a POSIX-normalized relative path so a Windows-style staged path can't bypass an oxlint-ignored directory; add -- before staged paths in both oxlint invocations so a path shaped like an oxlint flag (e.g. --config=...) can't be parsed as one; use string replacement and unicode-mode regexes for the escape step. - preflight.ts: forward the selected update version to the Windows installer via a PYTHINKER_VERSION env override (install.ps1 already reads it) instead of always installing whatever the CDN currently reports as latest. - install.ps1: only remove a previous update's stale backup once the current target is confirmed present, so an interrupted prior update can't lose its only runnable executable; initialize $extractDir before entering the try block so a failure before its first assignment can't resolve a same-named variable left over in the caller's iex-hosted scope during cleanup. - README.md: restore the Node.js badge to 26+ to match package.json's actual >=26.4.0 engine requirement.
1 parent 6224c7e commit d3a04cd

5 files changed

Lines changed: 63 additions & 17 deletions

File tree

.lintstagedrc.mjs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,29 @@ const OXLINT_IGNORE_PATTERNS = [
1616
const ignoreRegexes = OXLINT_IGNORE_PATTERNS.map((pattern) => {
1717
const isDir = pattern.endsWith('/');
1818
const body = isDir ? pattern.slice(0, -1) : pattern;
19-
const escaped = body.replaceAll(/[.+^${}()|[\]\\]/g, '\\$&').replaceAll(/\*/g, '[^/]*');
20-
return isDir ? new RegExp(`(^|/)${escaped}(/|$)`) : new RegExp(`(^|/)${escaped}$`);
19+
const escaped = body.replaceAll(/[.+^${}()|[\]\\]/gu, '\\$&').replaceAll('*', '[^/]*');
20+
return isDir
21+
? new RegExp(`(^|/)${escaped}(/|$)`, 'u')
22+
: new RegExp(`(^|/)${escaped}$`, 'u');
2123
});
2224

25+
// files are absolute, native-separator paths; match on the POSIX-style
26+
// relative path but keep the native path for the command line below.
2327
function lintableFiles(files) {
24-
return files
25-
.map((file) => relative(process.cwd(), file))
26-
.filter((file) => !ignoreRegexes.some((re) => re.test(file)));
28+
return files.filter((file) => {
29+
const posixRelative = relative(process.cwd(), file).replaceAll('\\', '/');
30+
return !ignoreRegexes.some((re) => re.test(posixRelative));
31+
});
2732
}
2833

2934
export default {
3035
'*.{js,jsx,ts,tsx,mjs,cjs,mts,cts}': (files) => {
3136
const targets = lintableFiles(files);
3237
if (targets.length === 0) return [];
3338
const quoted = targets.map((f) => JSON.stringify(f)).join(' ');
34-
return [`oxlint --fix --quiet ${quoted}`, `oxlint --type-aware --quiet ${quoted}`];
39+
return [
40+
`oxlint --fix --quiet -- ${quoted}`,
41+
`oxlint --type-aware --quiet -- ${quoted}`,
42+
];
3543
},
3644
};

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
[![npm version](https://img.shields.io/npm/v/@pythoughts/pythinker-code?style=for-the-badge&logo=npm&logoColor=white&color=CB3837&label=pythinker-code)](https://www.npmjs.com/package/@pythoughts/pythinker-code)
1313
[![Downloads](https://img.shields.io/npm/dm/@pythoughts/pythinker-code?style=for-the-badge&logo=npm&logoColor=white&color=16a34a&label=downloads)](https://www.npmjs.com/package/@pythoughts/pythinker-code)
14-
[![Node.js](https://img.shields.io/badge/Node.js-24%2B-339933?style=for-the-badge&logo=nodedotjs&logoColor=white)](https://github.com/Pythoughts-labs/pythinker-code/blob/main/package.json)
14+
[![Node.js](https://img.shields.io/badge/Node.js-26%2B-339933?style=for-the-badge&logo=nodedotjs&logoColor=white)](https://github.com/Pythoughts-labs/pythinker-code/blob/main/package.json)
1515
[![License: MIT](https://img.shields.io/badge/License-MIT-16a34a.svg?style=for-the-badge)](https://github.com/Pythoughts-labs/pythinker-code/blob/main/LICENSE)
1616
[![CI](https://img.shields.io/github/actions/workflow/status/Pythoughts-labs/pythinker-code/ci.yml?branch=main&label=CI&style=for-the-badge&logo=githubactions&logoColor=white)](https://github.com/Pythoughts-labs/pythinker-code/actions/workflows/ci.yml?query=branch%3Amain)
1717

apps/pythinker-code/src/cli/update/preflight.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export function automaticUpdateModeFor(
128128
interface SpawnCommand {
129129
readonly cmd: string;
130130
readonly args: readonly string[];
131+
readonly env?: Readonly<Record<string, string>>;
131132
}
132133

133134
export function spawnForSource(
@@ -148,9 +149,13 @@ export function spawnForSource(
148149
return { cmd: 'brew', args: ['upgrade', 'pythinker-code'] };
149150
case 'native':
150151
if (platform === 'win32') {
152+
// install.ps1 reads $env:PYTHINKER_VERSION when set instead of
153+
// fetching the CDN's current latest, so the version this preflight
154+
// decided on is the one actually installed.
151155
return {
152156
cmd: 'powershell.exe',
153157
args: ['-NoProfile', '-ExecutionPolicy', 'Bypass', '-Command', NATIVE_INSTALL_COMMAND_WIN],
158+
env: { PYTHINKER_VERSION: version },
154159
};
155160
}
156161
// `curl … | bash` reports only the trailing bash's exit status, so a
@@ -558,9 +563,12 @@ export async function installUpdate(
558563
version: string,
559564
platform: NodeJS.Platform,
560565
): Promise<void> {
561-
const { cmd, args } = spawnForSource(source, version, platform);
566+
const { cmd, args, env } = spawnForSource(source, version, platform);
562567
await new Promise<void>((resolve, reject) => {
563-
const child = spawn(cmd, [...args], { stdio: 'inherit' });
568+
const child = spawn(cmd, [...args], {
569+
stdio: 'inherit',
570+
env: env === undefined ? undefined : { ...process.env, ...env },
571+
});
564572
child.once('error', reject);
565573
child.once('exit', (code, signal) => {
566574
if (code === 0) {
@@ -737,7 +745,7 @@ async function startBackgroundInstall(
737745
source,
738746
});
739747

740-
const { cmd, args } = spawnForSource(source, target.version, platform);
748+
const { cmd, args, env } = spawnForSource(source, target.version, platform);
741749
// The child can exit before the pid-persist below finishes, so buffer
742750
// the terminal outcome until the handler is "ready".
743751
let ready = false;
@@ -802,7 +810,11 @@ async function startBackgroundInstall(
802810
}
803811
};
804812

805-
const child = spawn(cmd, [...args], { detached: true, stdio: 'ignore' });
813+
const child = spawn(cmd, [...args], {
814+
detached: true,
815+
stdio: 'ignore',
816+
env: env === undefined ? undefined : { ...process.env, ...env },
817+
});
806818
child.once('error', () => { void finish(false); });
807819
child.once('exit', (code) => { void finish(code === 0); });
808820
if (child.pid !== undefined && child.pid > 0) {

apps/pythinker-code/test/cli/update/preflight.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,11 @@ describe('runUpdatePreflight', () => {
574574
'-Command',
575575
'irm https://code.pythinker.com/pythinker-code/install.ps1 | iex',
576576
],
577-
{ detached: true, stdio: 'ignore' },
577+
{
578+
detached: true,
579+
stdio: 'ignore',
580+
env: expect.objectContaining({ PYTHINKER_VERSION: '0.5.0' }),
581+
},
578582
);
579583
} finally {
580584
Object.defineProperty(process, 'platform', { value: originalPlatform });
@@ -1506,7 +1510,7 @@ describe('spawnForSource native', () => {
15061510
});
15071511

15081512
it('win32: powershell.exe with -ExecutionPolicy Bypass and the irm|iex install command', () => {
1509-
const { cmd, args } = spawnForSource('native', '0.5.0', 'win32');
1513+
const { cmd, args, env } = spawnForSource('native', '0.5.0', 'win32');
15101514
expect(cmd).toBe('powershell.exe');
15111515
expect(args).toEqual([
15121516
'-NoProfile',
@@ -1515,6 +1519,14 @@ describe('spawnForSource native', () => {
15151519
'-Command',
15161520
'irm https://code.pythinker.com/pythinker-code/install.ps1 | iex',
15171521
]);
1522+
// install.ps1 reads $env:PYTHINKER_VERSION instead of fetching the CDN's
1523+
// current latest, so the selected update version is the one installed.
1524+
expect(env).toEqual({ PYTHINKER_VERSION: '0.5.0' });
1525+
});
1526+
1527+
it('darwin/linux: no version env override (install.sh has no such hook)', () => {
1528+
const { env } = spawnForSource('native', '0.5.0', 'darwin');
1529+
expect(env).toBeUndefined();
15181530
});
15191531
});
15201532

apps/pythinker-web/public/install.ps1

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,15 @@ New-Item -ItemType Directory -Path $tempDir | Out-Null
494494
$installerPath = Join-Path $tempDir $asset
495495
$shaPath = "$installerPath.sha256"
496496

497+
# This script runs via `irm ... | iex`, which dot-sources it into the
498+
# caller's scope rather than a fresh function scope. If $extractDir were
499+
# only ever assigned inside the try block, a failure before that
500+
# assignment (e.g. a SHA mismatch) could leave the `finally` block
501+
# resolving a same-named variable left over in the caller's session and
502+
# recursively deleting it. Initialize it here so `finally` always sees
503+
# this script's own value.
504+
$extractDir = $null
505+
497506
try {
498507
Download-WithProgress $installerUrl $installerPath
499508
Invoke-WebRequest -UseBasicParsing -Uri $shaUrl -OutFile $shaPath
@@ -524,9 +533,14 @@ try {
524533
# the running parent process or an AV scan.
525534
$target = Join-Path $installDir "pythinker.exe"
526535
$stale = "$target.old"
527-
# Opportunistic cleanup of a previous update's leftover (may be locked; ignore).
528-
if (Test-Path $stale) { Remove-Item -LiteralPath $stale -Force -ErrorAction SilentlyContinue }
529536
if (Test-Path $target) {
537+
# A prior backup is obsolete only while the current target remains
538+
# available. If $target is missing (a previous update was interrupted
539+
# after the rename but before Move-Item), $stale is the only runnable
540+
# executable — keep it until a replacement actually succeeds below.
541+
if (Test-Path $stale) {
542+
Remove-Item -LiteralPath $stale -Force -ErrorAction SilentlyContinue
543+
}
530544
try {
531545
Rename-Item -LiteralPath $target -NewName "pythinker.exe.old" -Force -ErrorAction Stop
532546
} catch {
@@ -564,7 +578,7 @@ try {
564578
} finally {
565579
Write-Host -NoNewline $SHOW
566580
Remove-Item -Recurse -Force $tempDir -ErrorAction SilentlyContinue
567-
if ($extractDir) {
568-
Remove-Item -Recurse -Force $extractDir -ErrorAction SilentlyContinue
581+
if ($null -ne $extractDir) {
582+
Remove-Item -LiteralPath $extractDir -Recurse -Force -ErrorAction SilentlyContinue
569583
}
570584
}

0 commit comments

Comments
 (0)