mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
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>
This commit is contained in:
1 parent
208d0c8fcc
commit
ecaca6fd34
6 files changed
+159
-6
No files matched your search
@@ -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"] }
|
||||
}
|
||||
@@ -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.
|
||||
+11
-2
@@ -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`.
|
||||
|
||||
|
||||
Executable
+75
@@ -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 <ref>]
|
||||
*
|
||||
* Reads every line the working tree adds against the merge base with <ref> (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: <path>:<line> <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<string, Set<number>>();
|
||||
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<number>();
|
||||
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)})`);
|
||||
@@ -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<string, string> = { '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<string, string>) => { 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<string, string>) => { 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}`)]);
|
||||
});
|
||||
});
|
||||
@@ -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([]);
|
||||
|
||||
Reference in new issue
Block a user