fix(windows): preserve a file's existing line endings on rewrite

.changeset/preserve-crlf-line-endings.md

@@ -0,0 +1,9 @@
+---
+"@fission-ai/openspec": patch
+---
+
+Preserve a file's existing line endings when rewriting it, so Windows users no longer get whole-file diffs. Applying a delta to a CRLF spec (the default on a Windows checkout with `core.autocrlf=true`) rewrote the file to LF, turning a one-requirement change into a diff that touched every line. `openspec archive` now writes the spec back with the convention it already used; a spec that does not exist yet is still written with LF.
+
+The same fix covers marker-managed files: installing or updating shell completions in a CRLF `.bashrc` or `.zshrc` no longer leaves the file with mixed endings, which `bash` reports as `$'\r': command not found`.
+
+`scripts/pack-version-check.mjs` now spawns `npm` through `cross-spawn`, so the release guard can run on Windows, where `npm` is `npm.cmd` and cannot be resolved by `execFile`.

scripts/pack-version-check.mjs

@@ -10,18 +10,34 @@
 //   `changeset publish` triggers `prepublishOnly` (also builds here). This
 //   means an explicit build is not strictly necessary for the guard.
 
-import { execFileSync } from 'child_process';
 import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs';
 import { tmpdir } from 'os';
 import path from 'path';
+import spawn from 'cross-spawn';
 
 function log(msg) {
   if (process.env.CI) return; // keep CI logs quiet by default
   console.log(msg);
 }
 
+// cross-spawn, not execFileSync: on Windows `npm` is npm.cmd, which execFile
+// cannot resolve without a shell. Keeps the argv form, so no shell is involved.
 function run(cmd, args, opts = {}) {
-  return execFileSync(cmd, args, { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], ...opts });
+  const result = spawn.sync(cmd, args, {
+    encoding: 'utf-8',
+    stdio: ['ignore', 'pipe', 'pipe'],
+    ...opts,
+  });
+
+  if (result.error) throw result.error;
+  if (result.status !== 0) {
+    const stderr = (result.stderr || '').trim();
+    throw new Error(
+      `${cmd} ${args.join(' ')} exited with ${result.status}${stderr ? `: ${stderr}` : ''}`
+    );
+  }
+
+  return result.stdout;
 }
 
 function npmPack() {

src/core/specs-apply.ts

@@ -28,6 +28,7 @@ import {
 } from './validation/constants.js';
 import { discoverSpecFiles } from '../utils/spec-discovery.js';
 import { FileSystemUtils } from '../utils/file-system.js';
+import { matchLineEnding } from '../utils/line-endings.js';
 
 // -----------------------------------------------------------------------------
 // Types
@@ -1243,11 +1244,19 @@ export async function writeUpdatedSpec(
   // Create target directory if needed
   const targetDir = path.dirname(update.target);
   await fs.mkdir(targetDir, { recursive: true });
+
+  // The parsers normalize CRLF to LF on read, so `rebuilt` is always LF. Write
+  // it back with the convention the file already used, or a Windows checkout
+  // (core.autocrlf=true) sees every line of the spec change when one
+  // requirement moved. A spec that does not exist yet stays LF.
+  const previous = await fs.readFile(update.target, 'utf-8').catch(() => undefined);
+  const toWrite = previous === undefined ? rebuilt : matchLineEnding(rebuilt, previous);
+
   await options.beforeMutate?.();
   // Preserve the established in-place write semantics: symlink referents,
   // hard-linked specs, ACLs, extended attributes, and filesystems without hard
   // links must continue to behave as they did before capability retirement.
-  await fs.writeFile(update.target, rebuilt);
+  await fs.writeFile(update.target, toWrite);
   if (options.silent) return;
 
   const specName = update.id;

src/utils/file-system.ts

@@ -1,5 +1,6 @@
 import * as nodeFs from 'fs';
 import path from 'path';
+import { matchLineEnding } from './line-endings.js';
 
 const fs = nodeFs.promises;
 const { constants: fsConstants } = nodeFs;
@@ -325,10 +326,16 @@ export class FileSystemUtils {
     endMarker: string
   ): Promise<void> {
     let existingContent = '';
-
+    // The managed block is composed with '\n', so splicing it into a CRLF file
+    // would leave mixed endings behind. bash reports a stray '\r' in .bashrc as
+    // "$'\r': command not found", so settle the whole file on the convention it
+    // already used. A file that does not exist yet stays LF.
+    let originalContent: string | undefined;
+
     if (await this.fileExists(filePath)) {
       existingContent = await this.readFile(filePath);
-
+      originalContent = existingContent;
+
       const startIndex = findMarkerIndex(existingContent, startMarker);
       const endIndex = startIndex !== -1
         ? findMarkerIndex(existingContent, endMarker, startIndex + startMarker.length)
@@ -352,8 +359,13 @@ export class FileSystemUtils {
     } else {
       existingContent = startMarker + '\n' + content + '\n' + endMarker;
     }
-
-    await this.writeFile(filePath, existingContent);
+
+    await this.writeFile(
+      filePath,
+      originalContent === undefined
+        ? existingContent
+        : matchLineEnding(existingContent, originalContent)
+    );
   }
 
   static async ensureWritePermissions(dirPath: string): Promise<boolean> {

src/utils/line-endings.ts

@@ -0,0 +1,50 @@
+/**
+ * Line-ending helpers.
+ *
+ * Every parser in this codebase normalizes CRLF to LF on the way in, so all
+ * serialization logic can assume '\n'. That leaves the write side responsible
+ * for restoring whatever convention the file already used: without it, editing
+ * one requirement in a CRLF spec rewrites every line of the file and buries the
+ * real change in the diff.
+ */
+
+export type LineEnding = '\n' | '\r\n';
+
+/**
+ * The dominant line ending in `content`, or undefined when it holds no line
+ * break to judge from.
+ *
+ * Mixed files resolve to whichever ending is more common, with CRLF winning a
+ * tie: a file that is mostly CRLF is a CRLF file that picked up a stray LF, and
+ * settling the whole file on one ending is what keeps later diffs small.
+ */
+export function detectLineEnding(content: string): LineEnding | undefined {
+  const crlf = content.match(/\r\n/g)?.length ?? 0;
+  // Count LFs not preceded by CR, so CRLF is never also counted as LF.
+  const lf = content.match(/(?<!\r)\n/g)?.length ?? 0;
+
+  if (crlf === 0 && lf === 0) return undefined;
+  return crlf >= lf ? '\r\n' : '\n';
+}
+
+/**
+ * Re-apply `ending` to LF-normalized `content`.
+ *
+ * Normalizes to LF first, so the result is uniform even if the caller passed
+ * content that already contained CRLF.
+ */
+export function applyLineEnding(content: string, ending: LineEnding): string {
+  const normalized = content.replace(/\r\n/g, '\n');
+  return ending === '\n' ? normalized : normalized.replace(/\n/g, '\r\n');
+}
+
+/**
+ * Rewrite `content` to match the convention of `original`.
+ *
+ * When `original` has no line break to learn from, `content` is left as LF —
+ * the portable default this project writes new files with.
+ */
+export function matchLineEnding(content: string, original: string): string {
+  const ending = detectLineEnding(original);
+  return ending === undefined ? content : applyLineEnding(content, ending);
+}

test/core/specs-apply.line-endings.test.ts

@@ -0,0 +1,129 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { promises as fs } from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import {
+  buildUpdatedSpec,
+  findSpecUpdates,
+  writeUpdatedSpec,
+} from '../../src/core/specs-apply.js';
+
+/**
+ * A spec written by a Windows editor, or checked out with core.autocrlf=true,
+ * arrives with CRLF endings. Applying a delta must not silently convert the
+ * whole file to LF: that turns a one-requirement change into a diff touching
+ * every line, which is unreviewable.
+ */
+
+const ORIGINAL_REQUIREMENT = [
+  '### Requirement: Existing behavior',
+  'The project SHALL expose the original behavior.',
+  '',
+  '#### Scenario: Existing path',
+  '- **WHEN** the behavior is exercised',
+  '- **THEN** it SHALL remain available',
+].join('\n');
+
+const UPDATED_REQUIREMENT = [
+  '### Requirement: Existing behavior',
+  'The project SHALL expose the updated behavior.',
+  '',
+  '#### Scenario: Existing path',
+  '- **WHEN** the behavior is exercised',
+  '- **THEN** it SHALL remain available',
+].join('\n');
+
+const BASE_SPEC = [
+  '# demo Specification',
+  '',
+  '## Purpose',
+  'Demonstrates line-ending preservation.',
+  '',
+  '## Requirements',
+  ORIGINAL_REQUIREMENT,
+  '',
+].join('\n');
+
+const DELTA_SPEC = ['## MODIFIED Requirements', '', UPDATED_REQUIREMENT, ''].join('\n');
+
+function countEndings(content: string): { crlf: number; loneLf: number } {
+  const crlf = content.match(/\r\n/g)?.length ?? 0;
+  const loneLf = content.match(/(?<!\r)\n/g)?.length ?? 0;
+  return { crlf, loneLf };
+}
+
+describe('spec line-ending preservation', () => {
+  let tempDir: string;
+  let changeDir: string;
+  let mainSpecsDir: string;
+  let source: string;
+  let target: string;
+
+  beforeEach(async () => {
+    tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-spec-eol-'));
+    changeDir = path.join(tempDir, 'openspec', 'changes', 'eol-test');
+    mainSpecsDir = path.join(tempDir, 'openspec', 'specs');
+    source = path.join(changeDir, 'specs', 'demo', 'spec.md');
+    target = path.join(mainSpecsDir, 'demo', 'spec.md');
+    await fs.mkdir(path.dirname(source), { recursive: true });
+    await fs.mkdir(path.dirname(target), { recursive: true });
+    await fs.writeFile(source, DELTA_SPEC);
+  });
+
+  afterEach(async () => {
+    await fs.rm(tempDir, { recursive: true, force: true });
+  });
+
+  async function applyTo(targetContent: string): Promise<string> {
+    await fs.writeFile(target, targetContent);
+    const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
+    const built = await buildUpdatedSpec(update, 'eol-test', { silent: true });
+    await writeUpdatedSpec(update, built.rebuilt, built.counts, { silent: true });
+    return fs.readFile(target, 'utf-8');
+  }
+
+  it('keeps a CRLF spec on CRLF', async () => {
+    const written = await applyTo(BASE_SPEC.replaceAll('\n', '\r\n'));
+
+    expect(written).toContain('updated behavior.');
+    const { crlf, loneLf } = countEndings(written);
+    expect(loneLf).toBe(0);
+    expect(crlf).toBeGreaterThan(0);
+  });
+
+  it('keeps an LF spec on LF', async () => {
+    const written = await applyTo(BASE_SPEC);
+
+    expect(written).toContain('updated behavior.');
+    const { crlf } = countEndings(written);
+    expect(crlf).toBe(0);
+  });
+
+  it('writes a brand-new spec with LF', async () => {
+    // No existing target to take a convention from; LF is the portable default.
+    // A new spec only accepts ADDED requirements.
+    await fs.writeFile(
+      source,
+      ['## ADDED Requirements', '', ORIGINAL_REQUIREMENT, ''].join('\n')
+    );
+    await fs.rm(target, { force: true });
+
+    const [update] = await findSpecUpdates(changeDir, mainSpecsDir);
+    const built = await buildUpdatedSpec(update, 'eol-test', { silent: true });
+    await writeUpdatedSpec(update, built.rebuilt, built.counts, { silent: true });
+
+    const written = await fs.readFile(target, 'utf-8');
+    expect(countEndings(written).crlf).toBe(0);
+  });
+
+  it('normalizes a mixed-ending spec to its dominant ending', async () => {
+    const mixed = BASE_SPEC.replaceAll('\n', '\r\n').replace('## Purpose\r\n', '## Purpose\n');
+    const written = await applyTo(mixed);
+
+    // CRLF dominates the input, so the output should settle on CRLF throughout
+    // rather than preserving the stray LF.
+    const { crlf, loneLf } = countEndings(written);
+    expect(crlf).toBeGreaterThan(0);
+    expect(loneLf).toBe(0);
+  });
+});

test/utils/line-endings.test.ts

@@ -0,0 +1,75 @@
+import { describe, expect, it } from 'vitest';
+import {
+  applyLineEnding,
+  detectLineEnding,
+  matchLineEnding,
+} from '../../src/utils/line-endings.js';
+
+describe('detectLineEnding', () => {
+  it('reports LF for an LF file', () => {
+    expect(detectLineEnding('a\nb\nc')).toBe('\n');
+  });
+
+  it('reports CRLF for a CRLF file', () => {
+    expect(detectLineEnding('a\r\nb\r\nc')).toBe('\r\n');
+  });
+
+  it('reports undefined when there is no line break', () => {
+    expect(detectLineEnding('single line')).toBeUndefined();
+    expect(detectLineEnding('')).toBeUndefined();
+  });
+
+  it('does not count a CRLF as an LF', () => {
+    // Two CRLF and no lone LF: a naive /\n/ count would see 2 of each and tie.
+    expect(detectLineEnding('a\r\nb\r\nc')).toBe('\r\n');
+  });
+
+  it('picks the dominant ending in a mixed file', () => {
+    expect(detectLineEnding('a\r\nb\r\nc\r\nd\ne')).toBe('\r\n');
+    expect(detectLineEnding('a\nb\nc\nd\r\ne')).toBe('\n');
+  });
+
+  it('breaks a tie toward CRLF', () => {
+    expect(detectLineEnding('a\r\nb\nc')).toBe('\r\n');
+  });
+
+  it('handles a lone CR without treating it as a line ending', () => {
+    // A bare CR is not a line break this project emits; it must not be
+    // mistaken for CRLF.
+    expect(detectLineEnding('a\rb')).toBeUndefined();
+  });
+});
+
+describe('applyLineEnding', () => {
+  it('converts LF to CRLF', () => {
+    expect(applyLineEnding('a\nb\n', '\r\n')).toBe('a\r\nb\r\n');
+  });
+
+  it('leaves LF alone when LF is requested', () => {
+    expect(applyLineEnding('a\nb\n', '\n')).toBe('a\nb\n');
+  });
+
+  it('is idempotent on already-CRLF content', () => {
+    expect(applyLineEnding('a\r\nb\r\n', '\r\n')).toBe('a\r\nb\r\n');
+  });
+
+  it('collapses mixed content to the requested ending', () => {
+    expect(applyLineEnding('a\r\nb\nc', '\r\n')).toBe('a\r\nb\r\nc');
+    expect(applyLineEnding('a\r\nb\nc', '\n')).toBe('a\nb\nc');
+  });
+});
+
+describe('matchLineEnding', () => {
+  it('restores CRLF from a CRLF original', () => {
+    expect(matchLineEnding('x\ny\n', 'a\r\nb\r\n')).toBe('x\r\ny\r\n');
+  });
+
+  it('keeps LF from an LF original', () => {
+    expect(matchLineEnding('x\ny\n', 'a\nb\n')).toBe('x\ny\n');
+  });
+
+  it('defaults to LF when the original has no line break', () => {
+    expect(matchLineEnding('x\ny\n', 'single line')).toBe('x\ny\n');
+    expect(matchLineEnding('x\ny\n', '')).toBe('x\ny\n');
+  });
+});