Skip to content

fix: Windows CRLF crash + npm shim / native binary detection#2

Open
kurushimee wants to merge 3 commits into
BenIsLegit:mainfrom
kurushimee:fix/windows-compat
Open

fix: Windows CRLF crash + npm shim / native binary detection#2
kurushimee wants to merge 3 commits into
BenIsLegit:mainfrom
kurushimee:fix/windows-compat

Conversation

@kurushimee
Copy link
Copy Markdown

Summary

Two Windows-specific fixes that together resolve the TypeError: Expected CommonJS module to have a function wrapper crash after --apply on Windows with CC 2.1.118.

  • CRLF line ending handling in system prompt escaping — On Windows, .md files have \r\n line endings. The newline escaping for " and ' JS string contexts only replaced \n, leaving bare \r (a JS line terminator) in the patched output. All 273 prompt files are affected (4,875 stray \r chars). Changed /\n/g to /\r\n|\r|\n/g for quote contexts; added \r normalization in the backtick branch too.
  • npm shim resolution + native binary search paths — On Windows, which('claude') returns an npm shim (sh/cmd wrapper), not the actual binary. New resolveNpmShimTarget() follows the shim to the real executable. Also adds bin/claude.exe search paths for CC 2.1.116+ which ships as a native binary with no cli.js.

Verification

  • node --check on the patched JS after stripping \r from the pre-fix output: exit 0 (confirms \r is the sole syntax cause)
  • Unit test: old escaping turns CRLF into \r\n (invalid); new escaping turns it into \n (valid)
  • pnpm run lint clean; pnpm test: 228 passed, 4 skipped (baseline)
  • End-to-end: --restore--applyclaude launches without error on Windows 11, CC 2.1.118 (npm install)

Test plan

  • On Windows: node dist/index.mjs --apply then claude — should launch without TypeError
  • On Windows: node --check "%USERPROFILE%\.tweakcc\native-claudejs-patched.js" — should exit 0
  • On macOS/Linux: verify no regression (LF-only files should behave identically)

🤖 Generated with Claude Code

kurushimee and others added 2 commits May 3, 2026 20:39
… on Windows

On Windows, `which('claude')` returns an npm-generated shim script
(a `#!/bin/sh` wrapper or `.cmd` batch file) rather than the actual
binary. `resolvePathToInstallationType` sees an unrecognized text
file and falls through to the hardcoded search paths, which may not
find the installation either.

New `resolveNpmShimTarget()` reads the shim content and extracts the
real target path:
  - sh shim: `"$basedir/node_modules/.../claude.exe"`
  - .cmd shim: `"%dp0%\node_modules\...\claude.exe"`

The PATH detection now tries the shim resolver (including the `.cmd`
variant alongside the bare shim) before falling back to search paths.

Also adds native binary search paths (`bin/claude.exe`) that mirror
the existing CLIJS search locations for CC 2.1.116+, which ships as
a native binary inside the npm package with no `cli.js`.

Verified on Windows 11 with CC 2.1.118 installed via npm — detection
now finds `bin/claude.exe` through the npm shim chain without needing
hardcoded search path fallback.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On Windows, system prompt `.md` files have CRLF (`\r\n`) line endings.
The newline escaping for `"` and `'` delimited JS strings only replaced
`\n` via `/\n/g`, leaving `\r` (0x0D) as a bare character in the output.
A bare `\r` is a line terminator in JavaScript and is illegal inside
double-quoted and single-quoted string literals, producing a syntax error
that Bun reports as:

  TypeError: Expected CommonJS module to have a function wrapper

All 273 `.md` files are affected — 4,875 stray `\r` characters end up
in the patched JS, but `node --check` fails on the first one it hits
(the Computer tool action parameter description, which lives in a
double-quoted JSON schema `description` field).

Fix: change the newline replacement regex from `/\n/g` to
`/\r\n|\r|\n/g` for both `"` and `'` delimiters, correctly handling
all three line-ending variants (CRLF, lone CR, LF).

Also normalize `\r\n` → `\n` in the backtick branch before
`escapeDepthZeroBackticks` runs. While `\r` is syntactically valid
inside template literals, the original cli.js doesn't contain bare `\r`
and injecting them would alter the actual prompt text sent to the model.

Verified:
  - `node --check` on the patched JS after stripping `\r`: exit 0.
  - Unit test: old escaping turns `\r\n` into `\r\n` (invalid in JS
    strings); new escaping turns `\r\n` into `\n` (valid).
  - `npm run lint` clean; `npm test`: 228 passed, 4 skipped (baseline).
  - End-to-end: `--apply` then `claude` launches without error on
    Windows 11 with CC 2.1.118.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@BenIsLegit
Copy link
Copy Markdown
Owner

Read through this. Looks mostly good, but I have two notes

1. resolveNpmShimTarget regex grabs the first "$basedir/..." match

For a native-binary shim (CC 2.1.116+) that's the only such reference, so it works as you intend. But JS-style npm shims look more like:

if [ -x "$basedir/node" ]; then
  exec "$basedir/node"  "$basedir/node_modules/.../cli.js" "$@"

The first match is "$basedir/node". doesFileExist returns true on that, resolvePathToInstallationType doesn't recognize node as a Claude install, and the function returns null. So the new code path silently no-ops on JS shims rather than resolving them. Doesn't break anything since the fallback path handles those, but it does mean resolveNpmShimTarget only ever fires for native-binary shims today.

Drop-in fix — switch to matchAll and require node_modules in the candidate path:

async function resolveNpmShimTarget(shimPath: string): Promise<string | null> {
  try {
    const content = await fs.readFile(shimPath, 'utf8');

    // npm sh shim: exec "$basedir/node_modules/.../claude.exe"  "$@"
    // Multiple "$basedir/..." references can appear (e.g. the "$basedir/node"
    // probe at the top of npm-cli-shim sh scripts); enumerate all and pick
    // the first node_modules path that exists on disk.
    for (const m of content.matchAll(/"\$basedir\/([^"]+)"/g)) {
      if (!m[1].includes('node_modules')) continue;
      const target = path.resolve(path.dirname(shimPath), m[1]);
      if (await doesFileExist(target)) {
        debug(`resolveNpmShimTarget: resolved sh shim -> ${target}`);
        return target;
      }
    }

    // npm .cmd shim: "%dp0%\node_modules\...\claude.exe"  %*
    for (const m of content.matchAll(/"%dp0%\\([^"]+)"/g)) {
      if (!m[1].includes('node_modules')) continue;
      const target = path.resolve(
        path.dirname(shimPath),
        m[1].replace(/\\/g, '/')
      );
      if (await doesFileExist(target)) {
        debug(`resolveNpmShimTarget: resolved cmd shim -> ${target}`);
        return target;
      }
    }

    return null;
  } catch {
    return null;
  }
}

2. No committed regression test for the CRLF fix

The PR body mentions a unit test confirming old/new behavior, but src/patches/systemPrompts.test.ts isn't touched in the diff. Bare \r is invisible in source, so a regression here only surfaces for Windows users running --apply — reverting /\r\n|\r|\n/g back to /\n/g six months from now wouldn't trip CI.

Drop-in tests that fit the existing fixture style — add these next to the 'should convert newlines to \\n for double-quoted string literals' test in src/patches/systemPrompts.test.ts:

it('should convert CRLF line endings to \\n for double-quoted string literals (Windows)', async () => {
  const mockPromptData = buildMockPromptData({
    prompt: { content: 'Hello\r\nWorld' },
    regex: 'Hello(?:\n|\\\\n)World',
    getInterpolatedContent: () => 'Hello\r\nWorld',
    pieces: ['Hello\r\nWorld'],
  });

  setupMocks(mockPromptData);

  const cliContent = 'description:"Hello\\nWorld"';

  const result = await applySystemPrompts(cliContent, '1.0.0', false);

  expect(result.newContent).toBe('description:"Hello\\nWorld"');
  expect(result.newContent).not.toMatch(/\r/);
});

it('should convert CRLF line endings to \\n for single-quoted string literals (Windows)', async () => {
  const mockPromptData = buildMockPromptData({
    prompt: { content: 'Hello\r\nWorld' },
    regex: 'Hello(?:\n|\\\\n)World',
    getInterpolatedContent: () => 'Hello\r\nWorld',
    pieces: ['Hello\r\nWorld'],
  });

  setupMocks(mockPromptData);

  const cliContent = "msg:'Hello\\nWorld'";

  const result = await applySystemPrompts(cliContent, '1.0.0', false);

  expect(result.newContent).toBe("msg:'Hello\\nWorld'");
  expect(result.newContent).not.toMatch(/\r/);
});

it('should normalize CRLF to LF for backtick template literals (Windows)', async () => {
  const mockPromptData = buildMockPromptData({
    prompt: { content: 'Hello\r\nWorld' },
    regex: 'Hello(?:\n|\\\\n)World',
    getInterpolatedContent: () => 'Hello\r\nWorld',
    pieces: ['Hello\r\nWorld'],
  });

  setupMocks(mockPromptData);

  const cliContent = 'description:`Hello\nWorld`';

  const result = await applySystemPrompts(cliContent, '1.0.0', false);

  expect(result.newContent).toBe('description:`Hello\nWorld`');
  expect(result.newContent).not.toMatch(/\r/);
});

Three tests, one per delimiter branch. The not.toMatch(/\r/) assertion is the key one — it's what would have caught the original bug

- resolveNpmShimTarget: switch to matchAll and require node_modules
  in candidate path. Single-match form grabbed the "$basedir/node"
  probe at the top of npm-cli-shim sh scripts and silently no-op'd
  for JS-style shims.
- systemPrompts.test.ts: commit three CRLF regression tests, one per
  delimiter branch (double-quote, single-quote, backtick). Bare \r
  is invisible in source, so not.toMatch(/\r/) is the assertion that
  catches a future revert to /\n/g.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants