Skip to content

fix(hooks): respect package manager formatter fallback #365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tsubasakong wants to merge 2 commits into
base: main
Choose a base branch
from
+107 −5
Open

fix(hooks): respect package manager formatter fallback #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f06ffd
fix(hooks): respect package manager in formatter fallback
tsubasakong Mar 8, 2026
5c8a782
Merge remote-tracking branch 'upstream/main' into HEAD
tsubasakong Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 35 additions & 5 deletions scripts/hooks/post-edit-format.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
const { execFileSync } = require('child_process');
const fs = require('fs');
const path = require('path');
const { getPackageManager } = require('../lib/package-manager');

const MAX_STDIN = 1024 * 1024; // 1MB limit
let data = '';
Expand Down Expand Up @@ -60,13 +61,42 @@ function detectFormatter(projectRoot) {
return null;
}

function getFormatterCommand(formatter, filePath) {
const npxBin = process.platform === 'win32' ? 'npx.cmd' : 'npx';
function getRunnerBin(bin) {
if (process.platform !== 'win32') return bin;

if (bin === 'npx') return 'npx.cmd';
if (bin === 'pnpm') return 'pnpm.cmd';
if (bin === 'yarn') return 'yarn.cmd';
if (bin === 'bunx') return 'bunx.cmd';

return bin;
}

function getFormatterRunner(projectRoot) {
const pm = getPackageManager({ projectDir: projectRoot });
const execCmd = pm?.config?.execCmd || 'npx';
const [bin = 'npx', ...prefix] = execCmd.split(/\s+/).filter(Boolean);

return {
bin: getRunnerBin(bin),
prefix
};
}

function getFormatterCommand(formatter, filePath, projectRoot) {
const runner = getFormatterRunner(projectRoot);

if (formatter === 'biome') {
return { bin: npxBin, args: ['@biomejs/biome', 'format', '--write', filePath] };
return {
bin: runner.bin,
args: [...runner.prefix, '@biomejs/biome', 'format', '--write', filePath]
};
}
if (formatter === 'prettier') {
return { bin: npxBin, args: ['prettier', '--write', filePath] };
return {
bin: runner.bin,
args: [...runner.prefix, 'prettier', '--write', filePath]
};
}
return null;
}
Expand All @@ -80,7 +110,7 @@ process.stdin.on('end', () => {
try {
const projectRoot = findProjectRoot(path.dirname(path.resolve(filePath)));
const formatter = detectFormatter(projectRoot);
const cmd = getFormatterCommand(formatter, filePath);
const cmd = getFormatterCommand(formatter, filePath, projectRoot);

if (cmd) {
execFileSync(cmd.bin, cmd.args, {
Expand Down
72 changes: 72 additions & 0 deletions tests/hooks/hooks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,20 @@ function cleanupTestDir(testDir) {
fs.rmSync(testDir, { recursive: true, force: true });
}

function createCommandShim(binDir, name, logFile) {
const shimPath = path.join(binDir, name);
const script = `#!/bin/sh
{
printf '%s\n' "$(basename "$0")"
for arg in "$@"; do
printf '%s\n' "$arg"
done
} > ${JSON.stringify(logFile)}
`;
fs.writeFileSync(shimPath, script, { mode: 0o755 });
return shimPath;
}
Comment on lines +78 to +90
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Cross-platform issue: POSIX-only shims cause Windows CI failures.

This createCommandShim function creates POSIX shell scripts, but on Windows:

  1. getRunnerBin returns .cmd suffixed binaries (e.g., pnpm.cmd)
  2. The shim is created without the .cmd extension
  3. The shim content uses POSIX shell syntax (#!/bin/sh, printf, for arg in "$@")

This is the root cause of the pipeline failures:

ENOENT: no such file or directory, open '...\pnpm.log'
ENOENT: no such file or directory, open '...\bunx.log'
🔧 Proposed fix: Platform-aware shim creation 
 function createCommandShim(binDir, name, logFile) {
-  const shimPath = path.join(binDir, name);
-  const script = `#!/bin/sh
+  const isWindows = process.platform === 'win32';
+  const shimPath = path.join(binDir, isWindows ? `${name}.cmd` : name);
+  
+  const script = isWindows
+    ? `@echo off
+echo %~n0> "${logFile}"
+:loop
+if "%~1"=="" goto done
+echo %~1>> "${logFile}"
+shift
+goto loop
+:done
+`
+    : `#!/bin/sh
 {
   printf '%s\\n' "$(basename "$0")"
   for arg in "$@"; do
     printf '%s\\n' "$arg"
   done
 } > ${JSON.stringify(logFile)}
 `;
   fs.writeFileSync(shimPath, script, { mode: 0o755 });
   return shimPath;
 }

As per coding guidelines: "Ensure cross-platform compatibility for Windows, macOS, and Linux via Node.js scripts".

📝 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.

Suggested change
function createCommandShim(binDir, name, logFile) {
const shimPath = path.join(binDir, name);
const script = `#!/bin/sh
{
printf '%s\n' "$(basename "$0")"
for arg in "$@"; do
printf '%s\n' "$arg"
done
} > ${JSON.stringify(logFile)}
`;
fs.writeFileSync(shimPath, script, { mode: 0o755 });
return shimPath;
}
function createCommandShim(binDir, name, logFile) {
const isWindows = process.platform === 'win32';
const shimPath = path.join(binDir, isWindows ? `${name}.cmd` : name);
const script = isWindows
? `@echo off
echo %~n0> "${logFile}"
:loop
if "%~1"=="" goto done
echo %~1>> "${logFile}"
shift
goto loop
:done
`
: `#!/bin/sh
{
printf '%s\n' "$(basename "$0")"
for arg in "$@"; do
printf '%s\n' "$arg"
done
} > ${JSON.stringify(logFile)}
`;
fs.writeFileSync(shimPath, script, { mode: 0o755 });
return shimPath;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/hooks/hooks.test.js` around lines 78 - 90, The current
createCommandShim produces a POSIX shell script which breaks Windows CI because
getRunnerBin on Windows expects .cmd executables; update createCommandShim to
detect process.platform === 'win32' and when true create a .cmd shim (append
.cmd to shimPath if needed) whose content uses Windows batch syntax to echo the
command name and each %* arg into the JSON-stringified logFile, otherwise keep
the existing POSIX script for non-Windows; ensure the created shim has the
correct filename (with .cmd on Windows) and is written with appropriate
permissions (fs.writeFileSync) so tests invoking getRunnerBin find and execute
the shim on all platforms.


// Test suite
async function runTests() {
console.log('\n=== Testing Hook Scripts ===\n');
Expand Down Expand Up @@ -701,6 +715,64 @@ async function runTests() {
assert.ok(result.stdout.includes('tool_input'), 'Should pass through original data');
})) passed++; else failed++;

if (await asyncTest('uses CLAUDE_PACKAGE_MANAGER runner for formatter fallback', async () => {
const testDir = createTestDir();
const binDir = path.join(testDir, 'bin');
const logFile = path.join(testDir, 'pnpm.log');
fs.mkdirSync(binDir, { recursive: true });
createCommandShim(binDir, 'pnpm', logFile);

const testFile = path.join(testDir, 'src', 'example.ts');
fs.mkdirSync(path.dirname(testFile), { recursive: true });
fs.writeFileSync(path.join(testDir, 'package.json'), JSON.stringify({ name: 'pm-env-test' }));
fs.writeFileSync(path.join(testDir, '.prettierrc'), '{}');
fs.writeFileSync(testFile, 'const answer=42;\n');

try {
const stdinJson = JSON.stringify({ tool_input: { file_path: testFile } });
const result = await runScript(path.join(scriptsDir, 'post-edit-format.js'), stdinJson, {
CLAUDE_PACKAGE_MANAGER: 'pnpm',
PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}`
});

assert.strictEqual(result.code, 0, 'Should exit 0 with pnpm fallback');
const logLines = fs.readFileSync(logFile, 'utf8').trim().split('\n');
assert.deepStrictEqual(logLines, ['pnpm', 'dlx', 'prettier', '--write', testFile]);
} finally {
cleanupTestDir(testDir);
}
})) passed++; else failed++;
Comment on lines +718 to +744
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Test will fail on Windows due to platform-specific shim issue.

The test creates a pnpm shim, but on Windows getRunnerBin('pnpm') returns pnpm.cmd. The execFileSync call in post-edit-format.js will search for pnpm.cmd but won't find it since the shim is created as pnpm (no .cmd extension).

Additionally, the assertion at Line 740 expects ['pnpm', 'dlx', 'prettier', '--write', testFile], but on Windows the first element would be pnpm.cmd if the shim were created correctly.

🔧 Proposed fix: Handle Windows in test assertion 
       assert.strictEqual(result.code, 0, 'Should exit 0 with pnpm fallback');
       const logLines = fs.readFileSync(logFile, 'utf8').trim().split('\n');
-      assert.deepStrictEqual(logLines, ['pnpm', 'dlx', 'prettier', '--write', testFile]);
+      const expectedBin = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm';
+      assert.deepStrictEqual(logLines, [expectedBin, 'dlx', 'prettier', '--write', testFile]);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/hooks/hooks.test.js` around lines 718 - 744, The test fails on Windows
because the pnpm shim is created as 'pnpm' but the process looks for 'pnpm.cmd';
update the test to use the platform-aware runner name instead of a hardcoded
'pnpm' by creating the shim with getRunnerBin('pnpm') (i.e., call
createCommandShim(binDir, getRunnerBin('pnpm'), logFile)) and change the
assertion that checks the logged command to compare against getRunnerBin('pnpm')
(or accept both 'pnpm' and 'pnpm.cmd') so post-edit-format.js and the test both
use the same platform-aware executable name; references: createCommandShim,
getRunnerBin, post-edit-format.js, logFile, and the assertion that currently
expects ['pnpm', 'dlx', 'prettier', '--write', testFile].


if (await asyncTest('uses project package manager config for formatter fallback', async () => {
const testDir = createTestDir();
const binDir = path.join(testDir, 'bin');
const logFile = path.join(testDir, 'bunx.log');
fs.mkdirSync(binDir, { recursive: true });
fs.mkdirSync(path.join(testDir, '.claude'), { recursive: true });
createCommandShim(binDir, 'bunx', logFile);

const testFile = path.join(testDir, 'src', 'example.ts');
fs.mkdirSync(path.dirname(testFile), { recursive: true });
fs.writeFileSync(path.join(testDir, 'package.json'), JSON.stringify({ name: 'pm-project-test' }));
fs.writeFileSync(path.join(testDir, 'biome.json'), '{}');
fs.writeFileSync(path.join(testDir, '.claude', 'package-manager.json'), JSON.stringify({ packageManager: 'bun' }));
fs.writeFileSync(testFile, 'const answer=42;\n');

try {
const stdinJson = JSON.stringify({ tool_input: { file_path: testFile } });
const result = await runScript(path.join(scriptsDir, 'post-edit-format.js'), stdinJson, {
CLAUDE_PACKAGE_MANAGER: '',
PATH: `${binDir}${path.delimiter}${process.env.PATH || ''}`
});

assert.strictEqual(result.code, 0, 'Should exit 0 with project-config fallback');
const logLines = fs.readFileSync(logFile, 'utf8').trim().split('\n');
assert.deepStrictEqual(logLines, ['bunx', '@biomejs/biome', 'format', '--write', testFile]);
} finally {
cleanupTestDir(testDir);
}
})) passed++; else failed++;
Comment on lines +746 to +774
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same cross-platform issue affects the bunx test.

The bunx shim has the same problem—created without .cmd extension and with POSIX shell syntax. The assertion at Line 770 also needs to account for bunx.cmd on Windows.

🔧 Proposed fix: Handle Windows in test assertion 
       assert.strictEqual(result.code, 0, 'Should exit 0 with project-config fallback');
       const logLines = fs.readFileSync(logFile, 'utf8').trim().split('\n');
-      assert.deepStrictEqual(logLines, ['bunx', '@biomejs/biome', 'format', '--write', testFile]);
+      const expectedBin = process.platform === 'win32' ? 'bunx.cmd' : 'bunx';
+      assert.deepStrictEqual(logLines, [expectedBin, '@biomejs/biome', 'format', '--write', testFile]);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/hooks/hooks.test.js` around lines 746 - 774, The test currently asserts
the shim invocation is exactly 'bunx' but on Windows the shim will be
'bunx.cmd'; update the assertion in the asyncTest 'uses project package manager
config for formatter fallback' to accept the Windows variant by checking
process.platform === 'win32' and expecting 'bunx.cmd' (or compute expectedBin =
process.platform === 'win32' ? 'bunx.cmd' : 'bunx') then
assert.deepStrictEqual(logLines, [expectedBin, '@biomejs/biome', 'format',
'--write', testFile]) so the test passes cross-platform; modify the assertion
near the assert.deepStrictEqual call that references logLines accordingly.


// post-edit-typecheck.js tests
console.log('\npost-edit-typecheck.js:');

Expand Down
Loading