From ecaca6fd3486692a581a18127220302836ccf7ab Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 16:50:06 +0000 Subject: [PATCH 1/3] pregate: a conflict-markers guard refuses merge markers on the lines a branch adds (GST-1190) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .gstack/pregate.json | 3 +- changelog.d/gsta258-l6-conflict-markers.md | 4 ++ docs/pregate.md | 13 +++- scripts/check-conflict-markers.ts | 75 ++++++++++++++++++++++ test/conflict-markers.test.ts | 64 ++++++++++++++++++ test/pregate-guards.test.ts | 6 +- 6 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 changelog.d/gsta258-l6-conflict-markers.md create mode 100755 scripts/check-conflict-markers.ts create mode 100644 test/conflict-markers.test.ts diff --git a/.gstack/pregate.json b/.gstack/pregate.json index 8b61aafb9..608163b2f 100644 --- a/.gstack/pregate.json +++ b/.gstack/pregate.json @@ -38,7 +38,8 @@ { "id": "context-budget", "when": ["SKILL.md", "*/SKILL.md", "*/sections/*.md", "SKILL.md.tmpl", "*/SKILL.md.tmpl", "*/sections/*.tmpl", "*/sections/manifest.json", "*/templates/*.tmpl", "hosts/**", "model-overlays/**", "scripts/gen-skill-docs.ts", "scripts/resolvers/**", "test/context-budget-ratchet.test.ts", "test/fixtures/context-budget*.json"], "command": "bun test test/context-budget-ratchet.test.ts", "fix": "shorten the skill under its token ceiling, or lower the ceiling when the skill shrank" }, { "id": "module-size", "when": ["lib/**/*.ts", "bin/**", "scripts/**/*.ts", "browse/src/**/*.ts", "test/helpers/**/*.ts", "test/module-size-ratchet.test.ts", "test/fixtures/module-size-ratchet.json"], "command": "bun test test/module-size-ratchet.test.ts", "fix": "split the module or function the failure names; it may not grow past its recorded size" }, { "id": "goldens", "when": ["SKILL.md", "*/SKILL.md", "*/sections/*.md", "SKILL.md.tmpl", "*/SKILL.md.tmpl", "*/sections/*.tmpl", "*/sections/manifest.json", "*/templates/*.tmpl", "hosts/**", "model-overlays/**", "scripts/gen-skill-docs.ts", "scripts/resolvers/**", "ship/**", "scripts/gen-ship-goldens.ts", "test/host-config.test.ts", "test/fixtures/golden/**"], "command": "bun test test/host-config.test.ts", "fix": "when the render change is intended, run bun scripts/gen-ship-goldens.ts and commit the updated goldens" }, - { "id": "doc-caps", "when": ["CLAUDE.md", "test/claude-md-size.test.ts"], "command": "bun test test/claude-md-size.test.ts", "fix": "move contributor-only detail from CLAUDE.md into docs/ and leave a one-line pointer" } + { "id": "doc-caps", "when": ["CLAUDE.md", "test/claude-md-size.test.ts"], "command": "bun test test/claude-md-size.test.ts", "fix": "move contributor-only detail from CLAUDE.md into docs/ and leave a one-line pointer" }, + { "id": "conflict-markers", "when": ["**/*.ts", "**/*.tsx", "**/*.js", "**/*.mjs", "**/*.cjs", "**/*.sh", "**/*.json", "**/*.yml", "**/*.yaml", "**/*.tmpl", "bin/**"], "command": "bun --no-env-file --config=/dev/null scripts/check-conflict-markers.ts", "fix": "finish the merge in each file it names and delete the marker lines; a fixture that shows markers on purpose declares gstack-conflict-markers: fixture" } ], "strays": { "allow": ["modules/loadouts.json", "modules/loadouts.schema.json"] } } diff --git a/changelog.d/gsta258-l6-conflict-markers.md b/changelog.d/gsta258-l6-conflict-markers.md new file mode 100644 index 000000000..85ca4f682 --- /dev/null +++ b/changelog.d/gsta258-l6-conflict-markers.md @@ -0,0 +1,4 @@ +- Pushes from /ship now stop on leftover merge conflict markers. A new `conflict-markers` guard reads every line the branch adds and refuses one that starts with `<<<<<<<` or `>>>>>>>`, or is exactly `=======`, even inside a template literal that still parses and passes its tests (GST-1190, seen on #3927). Markdown code blocks that show a conflict are skipped, and a fixture that shows markers on purpose declares `gstack-conflict-markers: fixture`. + +### Tests +- `test/conflict-markers.test.ts`: a marker inside a template literal is refused (it failed before the guard existed); a Markdown code block that shows a marker passes while the same lines outside a fence are refused; unchanged lines, declared fixtures and uncommitted files are handled. diff --git a/docs/pregate.md b/docs/pregate.md index aff7f6376..b69bde8df 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -1361,8 +1361,17 @@ being pushed skips the guards, as `GUARDS: not run`. Outside /ship, run the command above yourself before `git push`. gstack declares its own list in `.gstack/pregate.json`: `typecheck`, -`skill-size`, `parity-suite`, `context-budget`, `module-size`, `goldens` and -`doc-caps`. Generated SKILL.md freshness is the `regen` check's job. Other +`skill-size`, `parity-suite`, `context-budget`, `module-size`, `goldens`, +`doc-caps` and `conflict-markers`. Generated SKILL.md freshness is the `regen` +check's job. `conflict-markers` runs `scripts/check-conflict-markers.ts` on a +change to code, templates or config, and reads every line the branch adds +against its merge base with `origin/main`. A line starting with `<<<<<<<` or +`>>>>>>>` (then a space or the line end), or a line that is exactly `=======`, +stops the push, even inside a template literal the parser accepts. In Markdown +it skips fenced code blocks and counts `=======` only between an unfenced +`<<<<<<<` and its close, so a code block that shows a conflict and a heading +underline both pass. A fixture that shows markers on purpose carries a line +with `gstack-conflict-markers: fixture`. Other repos add their own `guards` the same way; the key needs no newer gstack than the one that already reads `guards`. diff --git a/scripts/check-conflict-markers.ts b/scripts/check-conflict-markers.ts new file mode 100755 index 000000000..6b7c016ff --- /dev/null +++ b/scripts/check-conflict-markers.ts @@ -0,0 +1,75 @@ +#!/usr/bin/env -S bun --no-env-file --config=/dev/null +/** + * check-conflict-markers — refuse merge conflict markers on the lines a branch changed (GST-1190). + * + * bun scripts/check-conflict-markers.ts [--base ] + * + * Reads every line the working tree adds against the merge base with (default origin/main): committed, + * staged, unstaged and untracked, deleted files skipped. A line starting with `<<<<<<<` or `>>>>>>>` followed + * by a space or the line end, or a line that is exactly `=======`, is a marker. This catches a marker the + * parser accepts, such as one inside a template literal (#3927). Markdown files skip lines inside a ``` or ~~~ + * fence, and count `=======` only between an unfenced `<<<<<<<` and its close, so a setext heading underline + * and a code block that shows a conflict both pass. A file with a line containing + * `gstack-conflict-markers: fixture` is skipped. Prints one CONFLICT_MARKER: : per hit + * and exits 1, or CONFLICT_MARKERS: none and exits 0; a git failure exits 2. + */ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +const OPEN = /^<{7}( |$)/, CLOSE = /^>{7}( |$)/, MID = /^={7}$/; +const FENCE = /^ {0,3}(`{3,}|~{3,})/; +const MARKDOWN = /\.(md|mdx|markdown)$/i; +const FIXTURE = 'gstack-conflict-markers: fixture'; + +function git(args: string[]): string { + const r = spawnSync('git', args, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, timeout: 120_000 }); + if (r.status !== 0) { console.error(`check-conflict-markers: git ${args.join(' ')} failed: ${(r.stderr || r.error?.message || '').trim()}`); process.exit(2); } + return r.stdout; +} + +const i = process.argv.indexOf('--base'); +const base = i > 0 ? process.argv[i + 1]! : 'origin/main'; +const root = git(['rev-parse', '--show-toplevel']).trim(); +process.chdir(root); +const fork = git(['merge-base', base, 'HEAD']).trim(); + +const added = new Map>(); +let file = ''; +for (const line of git(['diff', '-U0', '--no-color', '--no-ext-diff', '--no-renames', '--diff-filter=AM', fork, '--']).split('\n')) { + if (line.startsWith('+++ ')) { file = line.startsWith('+++ b/') ? line.slice(6) : ''; continue; } + const hunk = /^@@ -\S+ \+(\d+)(?:,(\d+))? @@/.exec(line); + if (!hunk || !file) continue; + const start = Number(hunk[1]), count = hunk[2] === undefined ? 1 : Number(hunk[2]); + const set = added.get(file) ?? new Set(); + for (let n = start; n < start + count; n++) set.add(n); + added.set(file, set); +} +for (const f of git(['ls-files', '--others', '--exclude-standard', '-z']).split('\0').filter(Boolean)) added.set(f, new Set([Infinity])); + +const hits: string[] = []; +for (const [f, lines] of [...added].sort(([a], [b]) => a.localeCompare(b))) { + let text: string; + try { text = fs.readFileSync(path.join(root, f), 'utf8'); } catch { continue; } + if (text.includes('\0') || text.includes(FIXTURE)) continue; + const md = MARKDOWN.test(f); + let fence = '', open = false; + text.split('\n').forEach((raw, idx) => { + const l = raw.replace(/\r$/, ''); + if (md) { + const m = FENCE.exec(l); + if (m && (!fence || (m[1]![0] === fence[0] && m[1]!.length >= fence.length))) { fence = fence ? '' : m[1]!; return; } + if (fence) return; + } + const marker = OPEN.test(l) ? '<'.repeat(7) : CLOSE.test(l) ? '>'.repeat(7) : MID.test(l) && (!md || open) ? '='.repeat(7) : ''; + if (OPEN.test(l)) open = true; + if (CLOSE.test(l)) open = false; + if (marker && (lines.has(Infinity) || lines.has(idx + 1))) hits.push(`CONFLICT_MARKER: ${f}:${idx + 1} ${marker}`); + }); +} +if (hits.length) { + console.log(hits.join('\n')); + console.log(`fix: finish the merge in each file above (keep the right side, delete the marker lines), or add a line with "${FIXTURE}" to a fixture that shows markers on purpose`); + process.exit(1); +} +console.log(`CONFLICT_MARKERS: none (${added.size} changed file(s) against ${fork.slice(0, 12)})`); diff --git a/test/conflict-markers.test.ts b/test/conflict-markers.test.ts new file mode 100644 index 000000000..32b6808a7 --- /dev/null +++ b/test/conflict-markers.test.ts @@ -0,0 +1,64 @@ +import { afterAll, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +const SCRIPT = path.resolve(import.meta.dir, '..', 'scripts', 'check-conflict-markers.ts'); +const OPEN = '<'.repeat(7), MID = '='.repeat(7), CLOSE = '>'.repeat(7); +const dirs: string[] = []; +afterAll(() => { for (const d of dirs) fs.rmSync(d, { recursive: true, force: true }); }); + +function repo(base: Record = { 'README.md': 'base\n' }) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-markers-')); + dirs.push(root); + const git = (...args: string[]) => spawnSync('git', args, { cwd: root, encoding: 'utf8', timeout: 30_000 }); + git('init', '-q', '-b', 'main'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'test'); + const write = (files: Record) => { for (const [p, c] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, p)), { recursive: true }); fs.writeFileSync(path.join(root, p), c); } }; + write(base); + git('add', '-A'); + git('commit', '-q', '-m', 'base'); + git('checkout', '-q', '-b', 'feat'); + const change = (files: Record) => { write(files); git('add', '-A'); git('commit', '-q', '-m', 'change'); }; + const run = () => spawnSync('bun', ['--no-env-file', '--config=/dev/null', SCRIPT, '--base', 'main'], { cwd: root, encoding: 'utf8', timeout: 60_000 }); + return { root, change, write, run }; +} + +describe('check-conflict-markers (GST-1190: #3927 kept markers inside a template literal and its tests passed)', () => { + // Value: protects=a merge that leaves conflict markers in a changed file is refused before the push, even where the + // markers sit inside a string the parser accepts; fails_when=a marker line in a template literal passes, or a markdown + // code block that shows a marker is refused; why_new=GSTA-234 #3927; seam=a real git repository and the script + test('a marker inside a template literal is refused', () => { + const r = repo(); + r.change({ 'lib/prompt.ts': `export const PROMPT = \`\n${OPEN} HEAD\nkeep this\n${MID}\nor this\n${CLOSE} origin/main\n\`;\n` }); + const out = r.run(); + expect(out.status).toBe(1); + expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:2 ${OPEN}`); + expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:4 ${MID}`); + expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:6 ${CLOSE}`); + }); + + test('a markdown code block that shows a marker is not refused; the same lines outside a fence are', () => { + const r = repo(); + const block = `${OPEN} HEAD\nours\n${MID}\ntheirs\n${CLOSE} main\n`; + r.change({ 'docs/merging.md': `# Merging\n\nGit writes this:\n\n\`\`\`\n${block}\`\`\`\n\nA heading\n${MID}\n` }); + const fenced = r.run(); + expect([fenced.status, fenced.stdout]).toEqual([0, expect.stringContaining('CONFLICT_MARKERS: none')]); + r.change({ 'docs/merging.md': `# Merging\n\n${block}` }); + const bare = r.run(); + expect(bare.status).toBe(1); + expect(bare.stdout).toContain(`CONFLICT_MARKER: docs/merging.md:3 ${OPEN}`); + expect(bare.stdout).toContain(`CONFLICT_MARKER: docs/merging.md:5 ${MID}`); + }); + + test('only changed lines count, and a fixture that declares its markers is skipped', () => { + const r = repo({ 'old.txt': `${OPEN} kept on main\n` }); + r.change({ 'old.txt': `${OPEN} kept on main\nnew line\n`, 'test/fixtures/merge.txt': `gstack-conflict-markers: fixture\n${OPEN} HEAD\n${CLOSE} main\n` }); + expect(r.run()).toMatchObject({ status: 0 }); + r.write({ 'src/wip.js': `${CLOSE} uncommitted\n` }); + const dirty = r.run(); + expect([dirty.status, dirty.stdout]).toEqual([1, expect.stringContaining(`CONFLICT_MARKER: src/wip.js:1 ${CLOSE}`)]); + }); +}); diff --git a/test/pregate-guards.test.ts b/test/pregate-guards.test.ts index 6bf2153b9..fe966df2c 100644 --- a/test/pregate-guards.test.ts +++ b/test/pregate-guards.test.ts @@ -280,7 +280,7 @@ describe("gstack's own pre-push guards (GST-1138)", () => { test('the list validates, every guard has a fix, and every test file it names exists', () => { expect(codes(policy)).toEqual([]); - expect(guards.map(g => g.id).sort()).toEqual(['context-budget', 'doc-caps', 'goldens', 'module-size', 'parity-suite', 'skill-size', 'typecheck']); + expect(guards.map(g => g.id).sort()).toEqual(['conflict-markers', 'context-budget', 'doc-caps', 'goldens', 'module-size', 'parity-suite', 'skill-size', 'typecheck']); for (const g of guards) { expect(g.fix, g.id).toBeTruthy(); for (const f of g.command.match(/test\/[\w./-]+\.test\.ts/g) ?? []) expect(fs.existsSync(path.join(GSTACK, f)), `${g.id}: ${f}`).toBe(true); @@ -288,8 +288,8 @@ describe("gstack's own pre-push guards (GST-1138)", () => { }); test('each guard fires on its own inputs and none fires on a docs-only change', () => { - expect(selected('ship/SKILL.md.tmpl')).toEqual(['context-budget', 'goldens', 'parity-suite', 'skill-size']); - expect(selected('lib/pregate/index.ts')).toEqual(['module-size', 'typecheck']); + expect(selected('ship/SKILL.md.tmpl')).toEqual(['conflict-markers', 'context-budget', 'goldens', 'parity-suite', 'skill-size']); + expect(selected('lib/pregate/index.ts')).toEqual(['conflict-markers', 'module-size', 'typecheck']); expect(selected('test/fixtures/golden/claude-ship-SKILL.md')).toEqual(['goldens']); expect(selected('CLAUDE.md')).toEqual(['doc-caps']); expect(selected('docs/pregate.md')).toEqual([]); From c9c2f7f762d8aac0dbce809bd416c7e7864b4322 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 17:02:25 +0000 Subject: [PATCH 2/3] fix(pregate): conflict-markers reads the pushed commit, fixed git prefixes and real declarations Review fixes for GST-1190: read HEAD and the working tree, force a/ b/ prefixes and unquoted paths, skip untracked files, require the fixture declaration on its own line near the top, refuse conflicts inside code fences in Markdown, run the test spawn through bunIsolationArgs for Windows. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .gstack/pregate.json | 2 +- changelog.d/gsta258-l6-conflict-markers.md | 4 +- docs/pregate.md | 19 ++-- scripts/check-conflict-markers.ts | 122 +++++++++++++-------- test/conflict-markers.test.ts | 58 +++++++--- 5 files changed, 131 insertions(+), 74 deletions(-) diff --git a/.gstack/pregate.json b/.gstack/pregate.json index 608163b2f..1246f74c7 100644 --- a/.gstack/pregate.json +++ b/.gstack/pregate.json @@ -39,7 +39,7 @@ { "id": "module-size", "when": ["lib/**/*.ts", "bin/**", "scripts/**/*.ts", "browse/src/**/*.ts", "test/helpers/**/*.ts", "test/module-size-ratchet.test.ts", "test/fixtures/module-size-ratchet.json"], "command": "bun test test/module-size-ratchet.test.ts", "fix": "split the module or function the failure names; it may not grow past its recorded size" }, { "id": "goldens", "when": ["SKILL.md", "*/SKILL.md", "*/sections/*.md", "SKILL.md.tmpl", "*/SKILL.md.tmpl", "*/sections/*.tmpl", "*/sections/manifest.json", "*/templates/*.tmpl", "hosts/**", "model-overlays/**", "scripts/gen-skill-docs.ts", "scripts/resolvers/**", "ship/**", "scripts/gen-ship-goldens.ts", "test/host-config.test.ts", "test/fixtures/golden/**"], "command": "bun test test/host-config.test.ts", "fix": "when the render change is intended, run bun scripts/gen-ship-goldens.ts and commit the updated goldens" }, { "id": "doc-caps", "when": ["CLAUDE.md", "test/claude-md-size.test.ts"], "command": "bun test test/claude-md-size.test.ts", "fix": "move contributor-only detail from CLAUDE.md into docs/ and leave a one-line pointer" }, - { "id": "conflict-markers", "when": ["**/*.ts", "**/*.tsx", "**/*.js", "**/*.mjs", "**/*.cjs", "**/*.sh", "**/*.json", "**/*.yml", "**/*.yaml", "**/*.tmpl", "bin/**"], "command": "bun --no-env-file --config=/dev/null scripts/check-conflict-markers.ts", "fix": "finish the merge in each file it names and delete the marker lines; a fixture that shows markers on purpose declares gstack-conflict-markers: fixture" } + { "id": "conflict-markers", "when": ["**/*.ts", "**/*.tsx", "**/*.js", "**/*.mjs", "**/*.cjs", "**/*.sh", "**/*.json", "**/*.yml", "**/*.yaml", "**/*.tmpl", "bin/**"], "command": "bun scripts/check-conflict-markers.ts", "fix": "finish the merge in each file it names, delete the marker lines and commit; a fixture that shows markers on purpose declares itself as docs/pregate.md describes" } ], "strays": { "allow": ["modules/loadouts.json", "modules/loadouts.schema.json"] } } diff --git a/changelog.d/gsta258-l6-conflict-markers.md b/changelog.d/gsta258-l6-conflict-markers.md index 85ca4f682..4b87b93ad 100644 --- a/changelog.d/gsta258-l6-conflict-markers.md +++ b/changelog.d/gsta258-l6-conflict-markers.md @@ -1,4 +1,4 @@ -- Pushes from /ship now stop on leftover merge conflict markers. A new `conflict-markers` guard reads every line the branch adds and refuses one that starts with `<<<<<<<` or `>>>>>>>`, or is exactly `=======`, even inside a template literal that still parses and passes its tests (GST-1190, seen on #3927). Markdown code blocks that show a conflict are skipped, and a fixture that shows markers on purpose declares `gstack-conflict-markers: fixture`. +- Pushes from /ship now stop on leftover merge conflict markers. A new `conflict-markers` guard reads every line the branch adds, in the commit being pushed and in the working tree, and refuses one that starts with `<<<<<<<` or `>>>>>>>`, or is exactly `=======`, even inside a template literal that still parses and passes its tests (GST-1190, seen on #3927). Markdown code blocks that show git output (an unlabeled, `text` or `diff` fence) are skipped, while a conflict inside a `bash` or `ts` fence is still refused. A fixture that shows markers on purpose declares `gstack-conflict-markers: fixture` on a line of its own near the top. ### Tests -- `test/conflict-markers.test.ts`: a marker inside a template literal is refused (it failed before the guard existed); a Markdown code block that shows a marker passes while the same lines outside a fence are refused; unchanged lines, declared fixtures and uncommitted files are handled. +- `test/conflict-markers.test.ts`: a marker inside a template literal is refused (it failed before the guard existed); a Markdown code block that shows a marker passes while the same lines outside a fence or in a bash fence are refused; unchanged lines, declared fixtures, untracked files, quoted file names, git prefix settings and a committed conflict hidden by an unstaged repair are handled. diff --git a/docs/pregate.md b/docs/pregate.md index b69bde8df..0192eb467 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -1365,13 +1365,18 @@ gstack declares its own list in `.gstack/pregate.json`: `typecheck`, `doc-caps` and `conflict-markers`. Generated SKILL.md freshness is the `regen` check's job. `conflict-markers` runs `scripts/check-conflict-markers.ts` on a change to code, templates or config, and reads every line the branch adds -against its merge base with `origin/main`. A line starting with `<<<<<<<` or -`>>>>>>>` (then a space or the line end), or a line that is exactly `=======`, -stops the push, even inside a template literal the parser accepts. In Markdown -it skips fenced code blocks and counts `=======` only between an unfenced -`<<<<<<<` and its close, so a code block that shows a conflict and a heading -underline both pass. A fixture that shows markers on purpose carries a line -with `gstack-conflict-markers: fixture`. Other +against its merge base with `origin/main`, both in the committed HEAD (what the +push publishes) and in the working tree's tracked files. A line starting with +`<<<<<<<` or `>>>>>>>` (then a space or the line end), or a line that is exactly +`=======`, stops the push, even inside a template literal the parser accepts. +In Markdown (`.md`, `.mdx`, `.markdown`, `.md.tmpl`) it skips fences whose info +string is empty, `text`, `txt`, `plaintext`, `diff` or `console`, the fences a +doc uses to show git output, and counts `=======` only between a `<<<<<<<` and +its close, so such a code block and a heading underline both pass; a conflict +inside a `bash` or `ts` fence is still refused. A fixture that shows markers on +purpose puts a line that is only `gstack-conflict-markers: fixture` (a comment +leader such as `#` or `//` is fine) in its first five lines; the phrase +anywhere else exempts nothing. Other repos add their own `guards` the same way; the key needs no newer gstack than the one that already reads `guards`. diff --git a/scripts/check-conflict-markers.ts b/scripts/check-conflict-markers.ts index 6b7c016ff..4389ef0e6 100755 --- a/scripts/check-conflict-markers.ts +++ b/scripts/check-conflict-markers.ts @@ -4,72 +4,102 @@ * * bun scripts/check-conflict-markers.ts [--base ] * - * Reads every line the working tree adds against the merge base with (default origin/main): committed, - * staged, unstaged and untracked, deleted files skipped. A line starting with `<<<<<<<` or `>>>>>>>` followed - * by a space or the line end, or a line that is exactly `=======`, is a marker. This catches a marker the - * parser accepts, such as one inside a template literal (#3927). Markdown files skip lines inside a ``` or ~~~ - * fence, and count `=======` only between an unfenced `<<<<<<<` and its close, so a setext heading underline - * and a code block that shows a conflict both pass. A file with a line containing - * `gstack-conflict-markers: fixture` is skipped. Prints one CONFLICT_MARKER: : per hit - * and exits 1, or CONFLICT_MARKERS: none and exits 0; a git failure exits 2. + * Reads every line the branch adds against its merge base with (default origin/main) twice: in the + * committed HEAD, which is what a push publishes, and in the working tree's tracked files (deleted files are + * skipped; untracked files are never pushed, so they are not read). A line starting with `<<<<<<<` or + * `>>>>>>>` followed by a space or the line end, or a line that is exactly `=======`, is a marker. This + * catches a marker the parser accepts, such as one inside a template literal (#3927). Markdown files + * (`.md`, `.mdx`, `.markdown`, `.md.tmpl`) skip lines inside a ``` or ~~~ fence whose info string is empty, + * text, txt, plaintext, diff or console, the fences a doc uses to show git output; a conflict inside a bash, + * ts or other code fence is still refused. They count `=======` only between a `<<<<<<<` and its close, so a + * setext heading underline passes. A file whose first five lines include one that is only the declaration + * `gstack-conflict-markers: fixture` (after an optional comment leader such as `#`, `//` or `)?\s*$/; +const GIT = ['-c', 'core.quotePath=false', '-c', 'diff.noprefix=false', '-c', 'diff.mnemonicPrefix=false']; -function git(args: string[]): string { - const r = spawnSync('git', args, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, timeout: 120_000 }); - if (r.status !== 0) { console.error(`check-conflict-markers: git ${args.join(' ')} failed: ${(r.stderr || r.error?.message || '').trim()}`); process.exit(2); } - return r.stdout; +function git(args: string[], allowFail = false): string | null { + const r = spawnSync('git', [...GIT, ...args], { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, timeout: 120_000 }); + if (r.status === 0) return r.stdout; + if (allowFail) return null; + console.error(`check-conflict-markers: git ${args.join(' ')} failed: ${(r.stderr || r.error?.message || '').trim()}`); + process.exit(2); } -const i = process.argv.indexOf('--base'); -const base = i > 0 ? process.argv[i + 1]! : 'origin/main'; -const root = git(['rev-parse', '--show-toplevel']).trim(); -process.chdir(root); -const fork = git(['merge-base', base, 'HEAD']).trim(); - -const added = new Map>(); -let file = ''; -for (const line of git(['diff', '-U0', '--no-color', '--no-ext-diff', '--no-renames', '--diff-filter=AM', fork, '--']).split('\n')) { - if (line.startsWith('+++ ')) { file = line.startsWith('+++ b/') ? line.slice(6) : ''; continue; } - const hunk = /^@@ -\S+ \+(\d+)(?:,(\d+))? @@/.exec(line); - if (!hunk || !file) continue; - const start = Number(hunk[1]), count = hunk[2] === undefined ? 1 : Number(hunk[2]); - const set = added.get(file) ?? new Set(); - for (let n = start; n < start + count; n++) set.add(n); - added.set(file, set); +/** The added line numbers per path, from a zero-context diff whose headers name paths with fixed a/ and b/ prefixes. */ +function addedLines(diff: string): Map> { + const added = new Map>(); + let file = '', header = false; + for (const line of diff.split('\n')) { + if (line.startsWith('diff --git ')) { header = true; file = ''; continue; } + if (header && line.startsWith('+++ ')) { file = line.startsWith('+++ b/') ? line.slice(6).replace(/\t$/, '') : ''; header = false; continue; } + if (header) continue; + const hunk = /^@@ -\S+ \+(\d+)(?:,(\d+))? @@/.exec(line); + if (!hunk || !file) continue; + const start = Number(hunk[1]), count = hunk[2] === undefined ? 1 : Number(hunk[2]); + const set = added.get(file) ?? new Set(); + for (let n = start; n < start + count; n++) set.add(n); + added.set(file, set); + } + return added; } -for (const f of git(['ls-files', '--others', '--exclude-standard', '-z']).split('\0').filter(Boolean)) added.set(f, new Set([Infinity])); -const hits: string[] = []; -for (const [f, lines] of [...added].sort(([a], [b]) => a.localeCompare(b))) { - let text: string; - try { text = fs.readFileSync(path.join(root, f), 'utf8'); } catch { continue; } - if (text.includes('\0') || text.includes(FIXTURE)) continue; +const seen = new Set(); +function scan(f: string, text: string, lines: Set, where: string, hits: Set): void { + const all = text.split('\n'); + if (text.includes('\0') || all.slice(0, 5).some(l => DECLARATION.test(l))) return; const md = MARKDOWN.test(f); - let fence = '', open = false; - text.split('\n').forEach((raw, idx) => { + let fence = '', shown = false, open = false; + all.forEach((raw, idx) => { const l = raw.replace(/\r$/, ''); if (md) { const m = FENCE.exec(l); - if (m && (!fence || (m[1]![0] === fence[0] && m[1]!.length >= fence.length))) { fence = fence ? '' : m[1]!; return; } - if (fence) return; + if (m && !fence) { fence = m[1]!; shown = SHOWN.test(m[2]!); return; } + if (m && !m[2] && m[1]![0] === fence[0] && m[1]!.length >= fence.length) { fence = ''; shown = false; return; } + if (shown) return; } const marker = OPEN.test(l) ? '<'.repeat(7) : CLOSE.test(l) ? '>'.repeat(7) : MID.test(l) && (!md || open) ? '='.repeat(7) : ''; if (OPEN.test(l)) open = true; if (CLOSE.test(l)) open = false; - if (marker && (lines.has(Infinity) || lines.has(idx + 1))) hits.push(`CONFLICT_MARKER: ${f}:${idx + 1} ${marker}`); + if (!marker || !lines.has(idx + 1) || seen.has(`${f}:${idx + 1}`)) return; + seen.add(`${f}:${idx + 1}`); + hits.add(`CONFLICT_MARKER: ${f}:${idx + 1} ${marker}${where}`); }); } -if (hits.length) { - console.log(hits.join('\n')); - console.log(`fix: finish the merge in each file above (keep the right side, delete the marker lines), or add a line with "${FIXTURE}" to a fixture that shows markers on purpose`); + +const i = process.argv.indexOf('--base'); +const base = i > 0 ? process.argv[i + 1]! : 'origin/main'; +const root = git(['rev-parse', '--show-toplevel'])!.trim(); +process.chdir(root); +const fork = git(['merge-base', base, 'HEAD'])!.trim(); +const DIFF = ['diff', '-U0', '--no-color', '--no-ext-diff', '--no-renames', '--src-prefix=a/', '--dst-prefix=b/', '--diff-filter=AM']; + +const hits = new Set(); +const committed = addedLines(git([...DIFF, fork, 'HEAD', '--'])!); +for (const [f, lines] of committed) { + const text = git(['show', `HEAD:${f}`], true); + if (text !== null) scan(f, text, lines, '', hits); +} +const working = addedLines(git([...DIFF, fork, '--'])!); +for (const [f, lines] of working) { + let text: string; + try { text = fs.readFileSync(path.join(root, f), 'utf8'); } catch { continue; } + scan(f, text, lines, ' (working tree)', hits); +} +const files = new Set([...committed.keys(), ...working.keys()]).size; +if (hits.size) { + console.log([...hits].sort().join('\n')); + console.log('fix: finish the merge in each file above (keep the right side, delete the marker lines) and commit; a fixture that shows markers on purpose declares the fixture line documented in docs/pregate.md#prepush in its first five lines'); process.exit(1); } -console.log(`CONFLICT_MARKERS: none (${added.size} changed file(s) against ${fork.slice(0, 12)})`); +console.log(`CONFLICT_MARKERS: none (${files} changed file(s) against ${fork.slice(0, 12)})`); diff --git a/test/conflict-markers.test.ts b/test/conflict-markers.test.ts index 32b6808a7..ee05a48f1 100644 --- a/test/conflict-markers.test.ts +++ b/test/conflict-markers.test.ts @@ -3,8 +3,9 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { bunIsolationArgs } from '../lib/bun-isolation'; -const SCRIPT = path.resolve(import.meta.dir, '..', 'scripts', 'check-conflict-markers.ts'); +const SCRIPT = path.resolve(import.meta.dir, '..', 'scripts/check-conflict-markers.ts'); const OPEN = '<'.repeat(7), MID = '='.repeat(7), CLOSE = '>'.repeat(7); const dirs: string[] = []; afterAll(() => { for (const d of dirs) fs.rmSync(d, { recursive: true, force: true }); }); @@ -12,35 +13,39 @@ afterAll(() => { for (const d of dirs) fs.rmSync(d, { recursive: true, force: tr function repo(base: Record = { 'README.md': 'base\n' }) { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-markers-')); dirs.push(root); - const git = (...args: string[]) => spawnSync('git', args, { cwd: root, encoding: 'utf8', timeout: 30_000 }); - git('init', '-q', '-b', 'main'); - git('config', 'user.email', 'test@example.com'); - git('config', 'user.name', 'test'); + const git = (...args: string[]) => { + const r = spawnSync('git', ['-c', 'commit.gpgsign=false', '-c', 'core.hooksPath=/dev/null', '-c', 'user.email=test@example.com', '-c', 'user.name=test', ...args], { cwd: root, encoding: 'utf8', timeout: 30_000 }); + if (r.status !== 0) throw new Error(`git ${args.join(' ')}: ${r.stderr}`); + return r.stdout; + }; + git('init', '-q'); + git('symbolic-ref', 'HEAD', 'refs/heads/main'); const write = (files: Record) => { for (const [p, c] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, p)), { recursive: true }); fs.writeFileSync(path.join(root, p), c); } }; write(base); git('add', '-A'); git('commit', '-q', '-m', 'base'); git('checkout', '-q', '-b', 'feat'); const change = (files: Record) => { write(files); git('add', '-A'); git('commit', '-q', '-m', 'change'); }; - const run = () => spawnSync('bun', ['--no-env-file', '--config=/dev/null', SCRIPT, '--base', 'main'], { cwd: root, encoding: 'utf8', timeout: 60_000 }); - return { root, change, write, run }; + const run = (env: Record = {}) => spawnSync('bun', [...bunIsolationArgs(), SCRIPT, '--base', 'main'], { cwd: root, encoding: 'utf8', timeout: 60_000, env: { ...process.env, ...env } }); + return { root, git, change, write, run }; } describe('check-conflict-markers (GST-1190: #3927 kept markers inside a template literal and its tests passed)', () => { // Value: protects=a merge that leaves conflict markers in a changed file is refused before the push, even where the - // markers sit inside a string the parser accepts; fails_when=a marker line in a template literal passes, or a markdown - // code block that shows a marker is refused; why_new=GSTA-234 #3927; seam=a real git repository and the script + // markers sit inside a string the parser accepts; fails_when=a marker line in a template literal passes, a markdown + // code block that shows a marker is refused, or a git display setting hides a file; why_new=GSTA-234 #3927; + // seam=a real git repository and the script test('a marker inside a template literal is refused', () => { const r = repo(); r.change({ 'lib/prompt.ts': `export const PROMPT = \`\n${OPEN} HEAD\nkeep this\n${MID}\nor this\n${CLOSE} origin/main\n\`;\n` }); const out = r.run(); expect(out.status).toBe(1); - expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:2 ${OPEN}`); - expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:4 ${MID}`); - expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:6 ${CLOSE}`); + expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:2 ${OPEN}\n`); + expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:4 ${MID}\n`); + expect(out.stdout).toContain(`CONFLICT_MARKER: lib/prompt.ts:6 ${CLOSE}\n`); }); - test('a markdown code block that shows a marker is not refused; the same lines outside a fence are', () => { + test('a markdown code block that shows a marker is not refused; the same lines outside a fence, or in a bash fence, are', () => { const r = repo(); const block = `${OPEN} HEAD\nours\n${MID}\ntheirs\n${CLOSE} main\n`; r.change({ 'docs/merging.md': `# Merging\n\nGit writes this:\n\n\`\`\`\n${block}\`\`\`\n\nA heading\n${MID}\n` }); @@ -51,14 +56,31 @@ describe('check-conflict-markers (GST-1190: #3927 kept markers inside a template expect(bare.status).toBe(1); expect(bare.stdout).toContain(`CONFLICT_MARKER: docs/merging.md:3 ${OPEN}`); expect(bare.stdout).toContain(`CONFLICT_MARKER: docs/merging.md:5 ${MID}`); + r.change({ 'skill/sections/x.md': `# X\n\n\`\`\`bash\n${OPEN} HEAD\necho a\n${MID}\necho b\n${CLOSE} feat\n\`\`\`\n` }); + expect(r.run().stdout).toContain(`CONFLICT_MARKER: skill/sections/x.md:4 ${OPEN}`); }); - test('only changed lines count, and a fixture that declares its markers is skipped', () => { + test('only changed lines count; a fixture declares itself on a line of its own, a mention elsewhere does not exempt a file', () => { const r = repo({ 'old.txt': `${OPEN} kept on main\n` }); - r.change({ 'old.txt': `${OPEN} kept on main\nnew line\n`, 'test/fixtures/merge.txt': `gstack-conflict-markers: fixture\n${OPEN} HEAD\n${CLOSE} main\n` }); + r.change({ 'old.txt': `${OPEN} kept on main\nnew line\n`, 'test/fixtures/merge.txt': `# gstack-conflict-markers: fixture\n${OPEN} HEAD\n${CLOSE} main\n` }); expect(r.run()).toMatchObject({ status: 0 }); - r.write({ 'src/wip.js': `${CLOSE} uncommitted\n` }); - const dirty = r.run(); - expect([dirty.status, dirty.stdout]).toEqual([1, expect.stringContaining(`CONFLICT_MARKER: src/wip.js:1 ${CLOSE}`)]); + r.change({ 'cfg.json': `{"fix": "declare gstack-conflict-markers: fixture"}\n${OPEN} HEAD\n` }); + expect(r.run().stdout).toContain(`CONFLICT_MARKER: cfg.json:2 ${OPEN}`); + }); + + test('the pushed commit is read, not only the working tree; untracked files are not; git display settings change nothing', () => { + const r = repo(); + r.change({ 'src/a b/ünï.ts': `const s = \`\n${CLOSE} theirs\n\`;\n` }); + for (const env of [{}, { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.noprefix', GIT_CONFIG_VALUE_0: 'true' }, { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.mnemonicPrefix', GIT_CONFIG_VALUE_0: 'true' }]) { + expect(r.run(env).stdout, JSON.stringify(env)).toContain(`CONFLICT_MARKER: src/a b/ünï.ts:2 ${CLOSE}`); + } + r.write({ 'src/a b/ünï.ts': 'const s = 1;\n', 'merge.ts.orig': `${OPEN} HEAD\n` }); + const repaired = r.run(); + expect([repaired.status, repaired.stdout]).toEqual([1, expect.stringContaining(`CONFLICT_MARKER: src/a b/ünï.ts:2 ${CLOSE}`)]); + expect(repaired.stdout).not.toContain('merge.ts.orig'); + r.write({ 'README.md': `base\n${MID}\n` }); + r.git('add', '-A'); r.git('commit', '-q', '-m', 'repair'); + r.write({ 'lib/wip.ts': `${CLOSE} uncommitted\n` }); r.git('add', 'lib/wip.ts'); r.git('commit', '-q', '-m', 'wip'); r.write({ 'lib/wip.ts': `ok\n${OPEN} x\n` }); + expect(r.run().stdout).toContain(`CONFLICT_MARKER: lib/wip.ts:2 ${OPEN} (working tree)`); }); }); From 7cc5daad5cfb5b60c1db8f2e99d8a8d817c42d62 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 17:04:55 +0000 Subject: [PATCH 3/3] test(conflict-markers): type the git config env rows Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/conflict-markers.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/conflict-markers.test.ts b/test/conflict-markers.test.ts index ee05a48f1..14e285e91 100644 --- a/test/conflict-markers.test.ts +++ b/test/conflict-markers.test.ts @@ -71,7 +71,7 @@ describe('check-conflict-markers (GST-1190: #3927 kept markers inside a template test('the pushed commit is read, not only the working tree; untracked files are not; git display settings change nothing', () => { const r = repo(); r.change({ 'src/a b/ünï.ts': `const s = \`\n${CLOSE} theirs\n\`;\n` }); - for (const env of [{}, { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.noprefix', GIT_CONFIG_VALUE_0: 'true' }, { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.mnemonicPrefix', GIT_CONFIG_VALUE_0: 'true' }]) { + for (const env of []>[{}, { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.noprefix', GIT_CONFIG_VALUE_0: 'true' }, { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'diff.mnemonicPrefix', GIT_CONFIG_VALUE_0: 'true' }]) { expect(r.run(env).stdout, JSON.stringify(env)).toContain(`CONFLICT_MARKER: src/a b/ünï.ts:2 ${CLOSE}`); } r.write({ 'src/a b/ünï.ts': 'const s = 1;\n', 'merge.ts.orig': `${OPEN} HEAD\n` });