test: wave coverage for sanitizer, link_or_copy, build script, doc drift

Four new test files (29 cases total):

browse/test/server-sanitize-surrogates.test.ts:
  - 11 unit cases for sanitizeLoneSurrogates (passthrough, valid pair,
    lone high/low mid-string, trailing/leading lone, adjacent doubles,
    pair-then-lone, lone-then-pair, empty)
  - 2 bug-repro tests pinning the regression intent (UTF-8 round-trip,
    JSON.parse round-trip with codepoint assertion)
  - 4 wiring invariants asserting the architectural choke points stay
    intact (handleCommandInternalImpl rename, central sanitization
    line, sanitizeReplacer function exists, SSE producers stringify
    with replacer)
  Function extracted from server.ts via regex + eval'd in test scope
  so no production-code export is needed.

test/setup-windows-fallback.test.ts:
  - Static invariant (D7): zero raw `ln` calls outside the
    _link_or_copy helper body and comments
  - Helper-existence assertions
  - 4-cell behavior matrix (file/dir × Windows/Unix) via awk-style
    helper extraction + bash -c sourcing
  - Windows-note printer registration check
  Mirrors test/setup-conductor-worktree.test.ts patterns.

test/build-script-shell-compat.test.ts:
  - Regex assertion that package.json scripts.* contain no bash brace
    groups (Bun-Windows-hostile)
  - Subshell-precedence check for `.version` redirects
  Strips single-quoted strings before regexing so embedded JS code
  inside echo '...' doesn't false-positive.

test/docs-config-keys.test.ts:
  - DEPRECATED_KEYS denylist scanned across docs/**/*.md
  - Round-trip test for `gstack-config get artifacts_sync_mode`
  Defends the v1.27.0.0 rename from doc drift.

Updates to two existing tests:
  - test/setup-conductor-worktree.test.ts: expect `_link_or_copy`
    instead of `ln -snf` at the Conductor-worktree guard call site
  - test/gen-skill-docs.test.ts: same swap at three assertion sites
    (Codex section, Claude link_claude_skill_dirs body, Codex
    link_codex_skill_dirs body)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

browse/test/server-sanitize-surrogates.test.ts

@@ -0,0 +1,129 @@
+import { describe, test, expect } from 'bun:test';
+import * as fs from 'fs';
+import * as path from 'path';
+
+// The sanitizer is module-private in server.ts. Rather than refactor it to a
+// separate module just for testing, we extract its source via a regex slice and
+// eval it in a fresh function scope. Keeps the production layout untouched.
+const SERVER_PATH = path.resolve(import.meta.dir, '..', 'src', 'server.ts');
+const SERVER_SRC = fs.readFileSync(SERVER_PATH, 'utf-8');
+
+const fnMatch = SERVER_SRC.match(
+  /function sanitizeLoneSurrogates\(str: string\): string \{[\s\S]*?\n\}/
+);
+if (!fnMatch) throw new Error('Could not locate sanitizeLoneSurrogates in server.ts');
+
+// Strip TS annotations so eval works under plain JS.
+const jsSrc = fnMatch[0].replace('(str: string): string', '(str)');
+const sanitizeLoneSurrogates = new Function(`${jsSrc}\nreturn sanitizeLoneSurrogates;`)() as (
+  s: string,
+) => string;
+
+describe('sanitizeLoneSurrogates — unit cases', () => {
+  test('passthrough ASCII', () => {
+    expect(sanitizeLoneSurrogates('hello')).toBe('hello');
+  });
+
+  test('passthrough empty string', () => {
+    expect(sanitizeLoneSurrogates('')).toBe('');
+  });
+
+  test('preserves valid surrogate pair (U+1F389 🎉)', () => {
+    expect(sanitizeLoneSurrogates('hi 🎉')).toBe('hi 🎉');
+  });
+
+  test('replaces lone high surrogate mid-string', () => {
+    expect(sanitizeLoneSurrogates('a\uD800b')).toBe('a�b');
+  });
+
+  test('replaces lone low surrogate mid-string', () => {
+    expect(sanitizeLoneSurrogates('a\uDC00b')).toBe('a�b');
+  });
+
+  test('replaces trailing lone high at end of string', () => {
+    expect(sanitizeLoneSurrogates('a\uD800')).toBe('a�');
+  });
+
+  test('replaces leading lone low at start of string', () => {
+    expect(sanitizeLoneSurrogates('\uDC00b')).toBe('�b');
+  });
+
+  test('replaces two adjacent lone highs', () => {
+    expect(sanitizeLoneSurrogates('\uD800\uD800')).toBe('��');
+  });
+
+  test('replaces two adjacent lone lows', () => {
+    expect(sanitizeLoneSurrogates('\uDC00\uDC00')).toBe('��');
+  });
+
+  test('preserves valid pair followed by lone low', () => {
+    // 𐀀 = U+10000 = 𐀀, then a separate lone low.
+    const input = '𐀀\uDC00';
+    const output = sanitizeLoneSurrogates(input);
+    // Valid pair intact, trailing lone low replaced.
+    expect(output).toBe('𐀀�');
+  });
+
+  test('preserves valid pair preceded by lone low', () => {
+    const input = '\uDC00𐀀';
+    const output = sanitizeLoneSurrogates(input);
+    expect(output).toBe('�𐀀');
+  });
+});
+
+describe('sanitizeLoneSurrogates — bug-repro (D5)', () => {
+  // Pin the regression intent: a future refactor that drops sanitization
+  // must fail this test even if happy-path tests still pass.
+  test('unsanitized lone surrogate causes UTF-8 encode to substitute, sanitized version is stable', () => {
+    const badPayload = 'page content\uD800more content';
+
+    // Buffer.from(str, 'utf-8') silently substitutes invalid sequences with
+    // EF BF BD (U+FFFD). Round-trip is therefore lossy for lone surrogates.
+    const roundTrippedRaw = Buffer.from(badPayload, 'utf-8').toString('utf-8');
+    expect(roundTrippedRaw).not.toBe(badPayload); // proves the bug exists pre-sanitize
+
+    // After sanitization the round-trip is stable.
+    const sanitized = sanitizeLoneSurrogates(badPayload);
+    const roundTrippedSanitized = Buffer.from(sanitized, 'utf-8').toString('utf-8');
+    expect(roundTrippedSanitized).toBe(sanitized);
+  });
+
+  test('JSON.parse(JSON.stringify(...)) round-trip is stable after sanitization', () => {
+    // Anthropic's API path wraps the response body in a tool_result JSON
+    // object. JSON.stringify CAN encode a lone surrogate (escapes it), but
+    // some downstream consumers reject the resulting body.
+    const badPayload = 'before\uD800after';
+    const sanitized = sanitizeLoneSurrogates(badPayload);
+    const wrapped = JSON.stringify({ content: sanitized });
+    const reparsed = JSON.parse(wrapped) as { content: string };
+    // .toBe(sanitized) already proves the surrogate was replaced; the
+    // additional explicit check below documents the specific code points.
+    expect(reparsed.content).toBe(sanitized);
+    expect(reparsed.content.charCodeAt(6)).toBe(0xfffd); // � not \uD800
+  });
+});
+
+describe('sanitizeLoneSurrogates — wiring invariants', () => {
+  test('server.ts wraps every command result through handleCommandInternal', () => {
+    // The architectural choice is to wrap once at handleCommandInternal so
+    // both single-command HTTP and the batch loop inherit. If a future
+    // refactor moves sanitization back to handleCommand only, this test
+    // fails by detecting the missing wrapper.
+    expect(SERVER_SRC).toContain('async function handleCommandInternalImpl(');
+    expect(SERVER_SRC).toContain('result: sanitizeLoneSurrogates(cr.result)');
+  });
+
+  test('SSE activity feed sanitizes outbound frames via sanitizeReplacer', () => {
+    // Replacer must run DURING stringify; post-stringify regex is ineffective
+    // because JSON.stringify converts \uD800 → "\\ud800" before our regex sees it.
+    expect(SERVER_SRC).toContain('JSON.stringify(entry, sanitizeReplacer)');
+  });
+
+  test('SSE inspector stream sanitizes outbound frames via sanitizeReplacer', () => {
+    expect(SERVER_SRC).toContain('JSON.stringify(event, sanitizeReplacer)');
+  });
+
+  test('sanitizeReplacer is a function defined in server.ts', () => {
+    expect(SERVER_SRC).toContain('function sanitizeReplacer(');
+  });
+});

test/build-script-shell-compat.test.ts

@@ -0,0 +1,40 @@
+import { describe, test, expect } from 'bun:test';
+import * as fs from 'fs';
+import * as path from 'path';
+
+const ROOT = path.resolve(import.meta.dir, '..');
+const PKG = JSON.parse(fs.readFileSync(path.join(ROOT, 'package.json'), 'utf-8')) as {
+  scripts: Record<string, string>;
+};
+
+// Strip single-quoted strings so JS code emitted as `echo '{ ... }'` doesn't
+// trip the shell-brace-group check. Conservative: only `'...'` segments.
+function stripSingleQuoted(s: string): string {
+  return s.replace(/'[^']*'/g, "''");
+}
+
+describe('package.json build scripts — POSIX shell compat (D-1460)', () => {
+  // Bun's Windows shell parser doesn't grok bash brace groups `{ cmd; }`.
+  // Subshells `( cmd )` are POSIX-universal. This test prevents regression.
+  test('no bash brace groups in any npm script', () => {
+    const offending: { script: string; pattern: string }[] = [];
+    for (const [name, body] of Object.entries(PKG.scripts)) {
+      const stripped = stripSingleQuoted(body);
+      const match = stripped.match(/\{\s+[^}]*;\s*\}/);
+      if (match) {
+        offending.push({ script: name, pattern: match[0] });
+      }
+    }
+    expect(offending).toEqual([]);
+  });
+
+  test('every `> path/.version` redirect is preceded by a subshell, not a brace group', () => {
+    // The original PR #1460 target: package.json line 12 had three of these.
+    const build = PKG.scripts.build ?? '';
+    const versionRedirects = [...build.matchAll(/(\([^)]*\)|\{[^}]*\})\s*>\s*\S+\/\.version/g)];
+    expect(versionRedirects.length).toBeGreaterThan(0);
+    for (const m of versionRedirects) {
+      expect(m[1].startsWith('(')).toBe(true);
+    }
+  });
+});

test/docs-config-keys.test.ts

@@ -0,0 +1,81 @@
+import { describe, test, expect } from 'bun:test';
+import { spawnSync } from 'child_process';
+import * as fs from 'fs';
+import * as path from 'path';
+
+const ROOT = path.resolve(import.meta.dir, '..');
+const CONFIG_BIN = path.join(ROOT, 'bin', 'gstack-config');
+
+// gstack-config accepts arbitrary keys (free-form YAML store), so we can't
+// build an authoritative set of "valid keys" from the script. Instead, defend
+// the specific invariant this wave introduces: deprecated keys must not
+// reappear in user-facing docs. Extend the denylist as future renames happen.
+const DEPRECATED_KEYS = new Set<string>([
+  // Renamed to artifacts_sync_mode in v1.27.0.0, doc references re-deprecated
+  // in v1.36.0.0 alongside the same rename of *_prompted.
+  'gbrain_sync_mode',
+  'gbrain_sync_mode_prompted',
+]);
+
+function scanDocsForConfigKeys(): { docPath: string; key: string; line: number }[] {
+  const hits: { docPath: string; key: string; line: number }[] = [];
+  const docsDir = path.join(ROOT, 'docs');
+  // Recurse docs/ but skip dotfiles. CHANGELOG.md/TODOS.md are excluded by virtue
+  // of being top-level; we only scan docs/**.
+  const stack = [docsDir];
+  while (stack.length) {
+    const cur = stack.pop()!;
+    for (const ent of fs.readdirSync(cur, { withFileTypes: true })) {
+      if (ent.name.startsWith('.')) continue;
+      const full = path.join(cur, ent.name);
+      if (ent.isDirectory()) {
+        stack.push(full);
+        continue;
+      }
+      if (!ent.name.endsWith('.md')) continue;
+      const text = fs.readFileSync(full, 'utf-8');
+      const lines = text.split('\n');
+      lines.forEach((line, idx) => {
+        // Match `gstack-config set <key>` or `gstack-config get <key>`.
+        for (const m of line.matchAll(/gstack-config\s+(?:set|get)\s+([a-z][a-z0-9_]*)/g)) {
+          hits.push({ docPath: full, key: m[1], line: idx + 1 });
+        }
+      });
+    }
+  }
+  return hits;
+}
+
+describe('docs ↔ gstack-config key drift guard', () => {
+  test('docs/ references at least one config key (smoke)', () => {
+    const hits = scanDocsForConfigKeys();
+    expect(hits.length).toBeGreaterThan(0);
+  });
+
+  test('no doc references a deprecated config key', () => {
+    const hits = scanDocsForConfigKeys();
+    const stale = hits.filter((h) => DEPRECATED_KEYS.has(h.key));
+    if (stale.length > 0) {
+      console.error('Deprecated config keys referenced in docs:', stale);
+    }
+    expect(stale).toEqual([]);
+  });
+
+  test('`gstack-config get artifacts_sync_mode` returns a value (the rename landed)', () => {
+    // Run from a clean HOME so the user's local config doesn't pollute.
+    const tmpHome = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gstack-cfg-'));
+    try {
+      const result = spawnSync(CONFIG_BIN, ['get', 'artifacts_sync_mode'], {
+        encoding: 'utf-8',
+        env: { ...process.env, HOME: tmpHome, GSTACK_HOME: tmpHome },
+        timeout: 5000,
+      });
+      expect(result.status).toBe(0);
+      // A known key returns its default value, not the "unknown key" error string.
+      expect(result.stderr).not.toContain('not recognized');
+      expect(result.stdout.trim().length).toBeGreaterThan(0);
+    } finally {
+      fs.rmSync(tmpHome, { recursive: true, force: true });
+    }
+  });
+});

test/gen-skill-docs.test.ts

@@ -2198,7 +2198,7 @@ describe('setup script validation', () => {
     expect(codexSection).toContain('create_codex_runtime_root');
     expect(codexSection).toContain('link_codex_skill_dirs');
     expect(codexSection).not.toContain('link_claude_skill_dirs');
-    expect(codexSection).not.toContain('ln -snf "$GSTACK_DIR" "$CODEX_GSTACK"');
+    expect(codexSection).not.toContain('_link_or_copy "$GSTACK_DIR" "$CODEX_GSTACK"');
   });
 
   test('Codex install prefers repo-local .agents/skills when setup runs from there', () => {
@@ -2238,7 +2238,8 @@ describe('setup script validation', () => {
     const fnEnd = setupContent.indexOf('}', setupContent.indexOf('linked[@]}', fnStart));
     const fnBody = setupContent.slice(fnStart, fnEnd);
     expect(fnBody).toContain('mkdir -p "$target"');
-    expect(fnBody).toContain('ln -snf "$gstack_dir/$dir_name/SKILL.md" "$target/SKILL.md"');
+    // v1.36.0.0: routes through _link_or_copy helper for Windows fallback (cp on MSYS2/Git Bash).
+    expect(fnBody).toContain('_link_or_copy "$gstack_dir/$dir_name/SKILL.md" "$target/SKILL.md"');
   });
 
   // REGRESSION: cleanup functions must handle both old symlinks AND new real-directory pattern
@@ -2345,7 +2346,7 @@ describe('setup script validation', () => {
     expect(fnBody).toContain('design-checklist.md');
     expect(fnBody).toContain('greptile-triage.md');
     expect(fnBody).toContain('TODOS-format.md');
-    expect(fnBody).not.toContain('ln -snf "$gstack_dir" "$codex_gstack"');
+    expect(fnBody).not.toContain('_link_or_copy "$gstack_dir" "$codex_gstack"');
   });
 
   test('direct Codex installs are migrated out of ~/.codex/skills/gstack', () => {

test/setup-conductor-worktree.test.ts

@@ -8,10 +8,11 @@ const ROOT = path.resolve(import.meta.dir, '..');
 const SETUP_SCRIPT = path.join(ROOT, 'setup');
 
 describe('setup: Conductor worktree guard', () => {
-  test('setup contains the real-dir guard before the ln -snf into ~/.claude/skills/', () => {
+  test('setup contains the real-dir guard before the symlink-or-copy into ~/.claude/skills/', () => {
     const content = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
     const guardIdx = content.indexOf('_SKIP_CLAUDE_REGISTER=0');
-    const lnIdx = content.indexOf('ln -snf "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK"');
+    // v1.36.0.0: symlink work routes through _link_or_copy helper for Windows fallback.
+    const lnIdx = content.indexOf('_link_or_copy "$SOURCE_GSTACK_DIR" "$CLAUDE_GSTACK_LINK"');
     expect(guardIdx).toBeGreaterThan(-1);
     expect(lnIdx).toBeGreaterThan(-1);
     expect(guardIdx).toBeLessThan(lnIdx);

test/setup-windows-fallback.test.ts

@@ -0,0 +1,122 @@
+import { describe, test, expect } from 'bun:test';
+import { spawnSync } from 'child_process';
+import * as path from 'path';
+import * as fs from 'fs';
+import * as os from 'os';
+
+const ROOT = path.resolve(import.meta.dir, '..');
+const SETUP_SCRIPT = path.join(ROOT, 'setup');
+const SETUP_SRC = fs.readFileSync(SETUP_SCRIPT, 'utf-8');
+
+// Slice out the _link_or_copy helper body via awk-style anchors so the test is
+// resilient to line-number drift.
+function extractHelper(): string {
+  const start = SETUP_SRC.indexOf('_link_or_copy() {');
+  const end = SETUP_SRC.indexOf('\n}\n', start);
+  if (start < 0 || end < 0) throw new Error('Could not locate _link_or_copy() in setup');
+  return SETUP_SRC.slice(start, end + 2);
+}
+
+describe('setup: _link_or_copy invariant (D7)', () => {
+  test('helper function is defined near the top of setup', () => {
+    expect(SETUP_SRC).toContain('_link_or_copy() {');
+    expect(SETUP_SRC).toContain('if [ "$IS_WINDOWS" -eq 1 ]; then');
+  });
+
+  test('zero raw `ln` calls outside the helper body and comments', () => {
+    // Pull the helper body out of the source first so its internal `ln -snf`
+    // (the Unix branch) is exempted from the invariant.
+    const helper = extractHelper();
+    const withoutHelper = SETUP_SRC.replace(helper, '');
+
+    // Strip shell comments to allow prose mentions of `ln -snf` in docstrings.
+    const lines = withoutHelper.split('\n');
+    const offending: { lineNo: number; line: string }[] = [];
+    lines.forEach((line, idx) => {
+      const trimmed = line.trim();
+      if (trimmed.startsWith('#')) return;
+      // Match standalone `ln ` invocations (allow `ln` as a substring in
+      // variable names like `linked`, `_LINK`).
+      if (/(^|[\s;&|`])ln\s+-/.test(line)) {
+        offending.push({ lineNo: idx + 1, line: line.trim() });
+      }
+    });
+    expect(offending).toEqual([]);
+  });
+
+  test('Windows-copy note message exists in setup', () => {
+    expect(SETUP_SRC).toContain('Windows install uses file copies');
+    expect(SETUP_SRC).toContain('_print_windows_copy_note_once');
+  });
+
+  test('link_claude_skill_dirs calls the Windows note printer', () => {
+    const fnStart = SETUP_SRC.indexOf('link_claude_skill_dirs() {');
+    const fnEnd = SETUP_SRC.indexOf('\n}\n', fnStart);
+    const fnBody = SETUP_SRC.slice(fnStart, fnEnd);
+    expect(fnBody).toContain('_print_windows_copy_note_once');
+  });
+});
+
+describe('setup: _link_or_copy helper — behavior matrix', () => {
+  // Source the helper into a temp shell with IS_WINDOWS set and exercise
+  // each cell of the file/dir × Windows/Unix matrix.
+  function runHelper(
+    isWindows: '0' | '1',
+    srcKind: 'file' | 'dir',
+  ): { ok: boolean; targetIsSymlink: boolean; targetExists: boolean; stderr: string } {
+    const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-helper-'));
+    try {
+      const src = path.join(tmp, 'source');
+      const dst = path.join(tmp, 'dest');
+      if (srcKind === 'file') {
+        fs.writeFileSync(src, 'hello\n');
+      } else {
+        fs.mkdirSync(src);
+        fs.writeFileSync(path.join(src, 'inner.txt'), 'hello\n');
+      }
+      const helper = extractHelper();
+      // IS_WINDOWS must exist as a shell-readable var before sourcing.
+      const script = `IS_WINDOWS=${isWindows}\n${helper}\n_link_or_copy "${src}" "${dst}"\n`;
+      const result = spawnSync('bash', ['-c', script], {
+        encoding: 'utf-8',
+        timeout: 5000,
+      });
+      const lst = fs.lstatSync(dst, { throwIfNoEntry: false });
+      return {
+        ok: result.status === 0,
+        targetIsSymlink: lst?.isSymbolicLink() ?? false,
+        targetExists: lst !== undefined,
+        stderr: result.stderr,
+      };
+    } finally {
+      fs.rmSync(tmp, { recursive: true, force: true });
+    }
+  }
+
+  test('IS_WINDOWS=0 + file → symlink (existing Unix behavior)', () => {
+    const r = runHelper('0', 'file');
+    expect(r.ok).toBe(true);
+    expect(r.targetExists).toBe(true);
+    expect(r.targetIsSymlink).toBe(true);
+  });
+
+  test('IS_WINDOWS=0 + dir → symlink', () => {
+    const r = runHelper('0', 'dir');
+    expect(r.ok).toBe(true);
+    expect(r.targetIsSymlink).toBe(true);
+  });
+
+  test('IS_WINDOWS=1 + file → regular file copy (no symlink)', () => {
+    const r = runHelper('1', 'file');
+    expect(r.ok).toBe(true);
+    expect(r.targetExists).toBe(true);
+    expect(r.targetIsSymlink).toBe(false);
+  });
+
+  test('IS_WINDOWS=1 + dir → real directory copy', () => {
+    const r = runHelper('1', 'dir');
+    expect(r.ok).toBe(true);
+    expect(r.targetExists).toBe(true);
+    expect(r.targetIsSymlink).toBe(false);
+  });
+});