diff --git a/bin/gstack-artifacts-init b/bin/gstack-artifacts-init index 0b02526ea..bfe2a58b7 100755 --- a/bin/gstack-artifacts-init +++ b/bin/gstack-artifacts-init @@ -403,6 +403,9 @@ projects/*/designs/**/*.md merge=union projects/*/ceo-plans/**/*.md merge=union projects/*/*-design-*.md merge=union projects/*/*-test-plan-*.md merge=union +# Derived from decisions.jsonl, so never merged as text: gstack-brain-sync +# rebuilds a conflicted snapshot from the merged log (#4174). +projects/*/decisions.active.json merge=binary EOF # ---- register merge drivers in local git config ---- diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index 36bcfa2a3..4fe78e506 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -598,6 +598,46 @@ transcripts_consented() { # stays in the kept stash entry, which the status names with the command # that restores it. MERGE_ERR="" +# decisions.active.json is derived from decisions.jsonl (rebuildSnapshot in +# lib/gstack-decision.ts), so two machines' snapshots are never merged as +# bytes (#4174). When a stopped merge's only conflicts are snapshots whose +# logs merged cleanly and only grew on both sides, each is rebuilt from its +# merged log and the merge is concluded. The jsonl-append driver takes the +# union of base, ours and theirs, so a log that lost lines on one side +# (--compact expunging a redacted decision) would get them back; that merge +# is left to abort as before. Anything else returns 1 and the caller aborts +# the merge, which also undoes a partial rebuild. Works without the +# .gitattributes line, so repositories initialized before it recover too. +rebuild_conflicted_snapshots() { + local -a lit=(git --literal-pathspecs -C "$GSTACK_HOME") slugs=() + local p slug base side + "${lit[@]}" rev-parse -q --verify MERGE_HEAD >/dev/null 2>&1 || return 1 + base=$("${lit[@]}" merge-base HEAD MERGE_HEAD 2>/dev/null) || return 1 + command -v bun >/dev/null 2>&1 || return 1 + while IFS= read -r -d '' p; do + case "$p" in projects/*/decisions.active.json) ;; *) return 1 ;; esac + slug="${p#projects/}"; slug="${slug%/decisions.active.json}" + case "$slug" in ''|*/*|.|..) return 1 ;; esac + slugs+=("$slug") + done < <("${lit[@]}" diff -z --name-only --diff-filter=U 2>/dev/null) + [ "${#slugs[@]}" -gt 0 ] || return 1 + for slug in "${slugs[@]}"; do + p="projects/$slug/decisions.jsonl" + "${lit[@]}" cat-file -e ":0:$p" 2>/dev/null || return 1 + "${lit[@]}" diff --quiet -- "$p" 2>/dev/null || return 1 + if "${lit[@]}" cat-file -e "$base:$p" 2>/dev/null; then + for side in HEAD MERGE_HEAD; do + [ -z "$(LC_ALL=C comm -23 <("${lit[@]}" show "$base:$p" 2>/dev/null | LC_ALL=C sort -u) \ + <("${lit[@]}" show "$side:$p" 2>/dev/null | LC_ALL=C sort -u))" ] || return 1 + done + fi + GSTACK_STATE_ROOT="$GSTACK_HOME" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" \ + "$SCRIPT_DIR/gstack-decision-log" --rebuild-snapshot "$slug" >/dev/null 2>&1 || return 1 + "${lit[@]}" add -f -- "projects/$slug/decisions.active.json" >/dev/null 2>&1 || return 1 + done + "${lit[@]}" commit -q --no-edit >/dev/null 2>&1 +} + merge_origin_keeping_local_edits() { local branch="$1" out rc=0 p stash_before="" stash_sha="" local -a lit=(git --literal-pathspecs -C "$GSTACK_HOME") blocking=() @@ -614,6 +654,7 @@ merge_origin_keeping_local_edits() { [ "$stash_sha" != "$stash_before" ] || stash_sha="" fi out=$("${lit[@]}" merge --no-edit "origin/$branch" 2>&1) || rc=$? + [ "$rc" -ne 0 ] && rebuild_conflicted_snapshots && rc=0 if [ "$rc" -ne 0 ]; then MERGE_ERR=$(printf '%s\n' "$out" | grep -m1 -E '^(CONFLICT|error|fatal)' || printf '%s\n' "$out" | head -1) "${lit[@]}" merge --abort >/dev/null 2>&1 || true diff --git a/bin/gstack-decision-log b/bin/gstack-decision-log index 6ee872a26..11ca06596 100755 --- a/bin/gstack-decision-log +++ b/bin/gstack-decision-log @@ -8,6 +8,7 @@ * gstack-decision-log --supersede * gstack-decision-log --redact * gstack-decision-log --compact + * gstack-decision-log --rebuild-snapshot * * Event-sourced (lib/gstack-decision): every call appends an event and refreshes the * bounded active snapshot. NON-INTERACTIVE — never prompts (agents/skills call this; @@ -16,6 +17,7 @@ */ import { dirname } from "path"; +import { existsSync } from "fs"; import { mkdirpSync } from "../lib/fs-utils"; import { spawnSync } from "child_process"; import { @@ -39,6 +41,7 @@ const usage = `Usage: gstack-decision-log --supersede [''] gstack-decision-log --redact gstack-decision-log --compact + gstack-decision-log --rebuild-snapshot gstack-decision-log --help Decision JSON: @@ -62,6 +65,8 @@ JSON Successful writes print the decision id. Invalid payloads exit 1 without a record. Supersede retires the old decision and optionally records a validated replacement. Redact accepts no replacement; compact removes redacted history and archives superseded records. +Rebuild-snapshot rewrites one project's decisions.active.json from its decisions.jsonl +(gstack-brain-sync runs it when a merge of two machines' snapshots conflicts). Help prints this schema and exits without resolving the project or writing state. `; @@ -75,6 +80,17 @@ if (args.every((arg) => !arg.trim())) { process.exit(1); } +if (args.includes("--rebuild-snapshot")) { + const target = flagValue(args, "--rebuild-snapshot"); + const targetPaths = target && /^[^/\\]+$/.test(target) && target !== "." && target !== ".." ? decisionPaths(target) : undefined; + if (!targetPaths || !existsSync(targetPaths.log)) { + process.stderr.write("gstack-decision-log: --rebuild-snapshot needs the slug of a project that has a decisions.jsonl\n"); + process.exit(1); + } + rebuildSnapshot(targetPaths); + process.exit(0); +} + const slug = resolveSlug(`${HERE}/gstack-slug`); const paths = decisionPaths(slug); // mkdirpSync, not bare mkdirSync: bun on Windows throws EEXIST from a diff --git a/changelog.d/fix-decisions-snapshot-merge-conflict.md b/changelog.d/fix-decisions-snapshot-merge-conflict.md new file mode 100644 index 000000000..942cb3d30 --- /dev/null +++ b/changelog.d/fix-decisions-snapshot-merge-conflict.md @@ -0,0 +1 @@ +- **Two machines that each log a decision between pushes no longer stop one of them from syncing.** The second machine's push was rejected, its fetch-and-merge retry conflicted on `projects//decisions.active.json`, and `gstack-brain-sync` aborted the merge and wrote `push_failed` on every later drain until someone repaired the clone by hand (#4174). The snapshot is derived from `decisions.jsonl`, so when a merge's only conflicts are decision snapshots whose logs merged cleanly, brain-sync now rebuilds each snapshot from the merged log with the new `gstack-decision-log --rebuild-snapshot `, commits the merge and pushes; it never merges the JSON array itself. This needs no `.gitattributes` change, so a clone already stuck this way publishes on its next drain that has records to sync. A conflict in any other file still aborts the merge as before, and so does a log that lost lines on either side, as `--compact` does to a redacted decision, since rebuilding from the union of both logs would bring that decision back. New artifacts repositories also get `projects/*/decisions.active.json merge=binary` in `.gitattributes`, so a merge run by hand leaves no conflict markers in the snapshot. diff --git a/docs/gbrain-sync.md b/docs/gbrain-sync.md index 756e8186f..29d7e03bc 100644 --- a/docs/gbrain-sync.md +++ b/docs/gbrain-sync.md @@ -206,6 +206,21 @@ merge can leave conflict markers in them. The sync parses every staged `invalid-json`) until you repair it, and `gstack-doctor` warns when a synced profile or decision snapshot holds conflict markers. +A project's decision snapshot, `decisions.active.json`, is never merged: it +is rebuilt from that project's `decisions.jsonl`, which merges like any other +`.jsonl` file. When two machines each log a decision between pushes, their +snapshots conflict; if every conflict in brain-sync's merge is such a snapshot, +brain-sync rebuilds each one from the merged log, commits the merge and pushes, +whether or not your `.gitattributes` predates this. The one exception is a log +that lost lines on either side since the machines last agreed, as +`gstack-decision-log --compact` does to redacted decisions: the merged log is +the union of both sides, so a rebuild would bring the redacted decision back, +and brain-sync aborts that merge as before. Repositories set up by +`gstack-artifacts-init` also mark the snapshot `merge=binary`, so a merge you +run by hand keeps your side without conflict markers; finish it with +`gstack-decision-log --rebuild-snapshot ` and `git add -f` the +snapshot before committing. + ## Cross-machine pull cadence The preamble runs `git fetch` + `git merge --ff-only` once per 24 hours diff --git a/test/brain-sync.test.ts b/test/brain-sync.test.ts index f4da3bd56..b6f7882c0 100644 --- a/test/brain-sync.test.ts +++ b/test/brain-sync.test.ts @@ -1304,6 +1304,204 @@ describe('gstack-brain-sync per-file hold', () => { expect(remoteShow(LOG).stdout).toContain('"ccc"'); expect(remoteShow(SNAP).stdout).toContain('"ccc"'); }); + + describe('two machines that each log a decision between pushes (#4174)', () => { + const ALOG = 'projects/acme-app/decisions.jsonl'; + const ASNAP = 'projects/acme-app/decisions.active.json'; + const PRE_FIX_ATTRIBUTES = [ + '# gstack-artifacts: merge drivers for cross-machine sync conflicts.', + '*.jsonl merge=jsonl-append', + 'retros/*.md merge=union', + 'projects/*/designs/**/*.md merge=union', + 'projects/*/ceo-plans/**/*.md merge=union', + 'projects/*/*-design-*.md merge=union', + 'projects/*/*-test-plan-*.md merge=union', + '', + ].join('\n'); + let proj = ''; + let machineA = ''; + beforeEach(() => { + proj = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-sync-proj-')); + machineA = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-sync-machineA-')); + expect(git(['init', '-q', '-b', 'main'], proj).status).toBe(0); + expect(git(['remote', 'add', 'origin', 'https://example.invalid/acme/app.git'], proj).status).toBe(0); + }); + afterEach(() => { + fs.rmSync(proj, { recursive: true, force: true }); + fs.rmSync(machineA, { recursive: true, force: true }); + }); + function logDecision(home: string, decision: string): string { + const r = spawnSync(path.join(BIN, 'gstack-decision-log'), [JSON.stringify({ decision, rationale: 'r', scope: 'repo', source: 'user' })], { + cwd: proj, + env: { ...process.env, HOME: tmpHome, GSTACK_HOME: home }, + encoding: 'utf-8', + timeout: 30_000, + }); + expect(r.stderr).toBe(''); + expect(r.status).toBe(0); + return r.stdout.trim(); + } + function setupMachineB(attributes: 'current' | 'pre-fix') { + init(); + expect(git(['config', 'user.name', 'fixture']).status).toBe(0); + expect(git(['config', 'user.email', 'fixture@example.com']).status).toBe(0); + if (attributes === 'pre-fix') { + fs.writeFileSync(path.join(tmpHome, '.gitattributes'), PRE_FIX_ATTRIBUTES); + if (git(['diff', '--quiet', '--', '.gitattributes']).status !== 0) { + expect(git(['commit', '-qm', 'gitattributes as gstack-artifacts-init wrote it before #4174', '--', '.gitattributes']).status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD']).status).toBe(0); + } + } + const seed = logDecision(tmpHome, 'seed'); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + return seed; + } + function machineAPushesADecision(): string { + expect(git(['clone', '-q', bareRemote, machineA]).status).toBe(0); + const id = logDecision(machineA, 'decision on machine A'); + expect(git(['add', '-f', '--', ALOG, ASNAP], machineA).status).toBe(0); + expect(git(['-c', 'user.name=peer', '-c', 'user.email=peer@example.com', 'commit', '-qm', 'sync A'], machineA).status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD'], machineA).status).toBe(0); + return id; + } + async function expectPublishedSnapshotRebuiltFromMergedLog(ids: string[]) { + const { computeActive } = await import('../lib/gstack-decision'); + const log = remoteShow(ALOG).stdout; + const events = log.split('\n').filter(Boolean).map((l) => JSON.parse(l)); + expect(events.map((e) => e.id).sort()).toEqual([...ids].sort()); + const rebuilt = JSON.stringify(computeActive(events)); + expect(remoteShow(ASNAP).stdout).toBe(rebuilt); + expect(fs.readFileSync(path.join(tmpHome, ASNAP), 'utf-8')).toBe(rebuilt); + expect(git(['rev-list', '--left-right', '--count', 'HEAD...origin/main']).stdout.trim()).toBe('0\t0'); + expect(mergeHead()).toBe(false); + expect(unmerged()).toBe(''); + } + const unmerged = () => git(['ls-files', '--unmerged']).stdout.trim(); + const mergeHead = () => fs.existsSync(path.join(tmpHome, '.git', 'MERGE_HEAD')); + + for (const attributes of ['current', 'pre-fix'] as const) { + test(`the retry merge rebuilds the snapshot from the merged log and publishes (${attributes} .gitattributes)`, async () => { + const seed = setupMachineB(attributes); + const a = machineAPushesADecision(); + const b = logDecision(tmpHome, 'decision on machine B'); + expect(drain().status).toBe(0); + const s = statusJson(); + expect(s.message).not.toContain('Merge conflict'); + expect(s.status).toBe('ok'); + await expectPublishedSnapshotRebuiltFromMergedLog([seed, a, b]); + }); + } + + test('a machine left holding a conflicting commit by an earlier failed merge recovers on its next drain', async () => { + const seed = setupMachineB('pre-fix'); + const a = machineAPushesADecision(); + const b = logDecision(tmpHome, 'decision on machine B'); + const shim = gitShim('case " $* " in *" merge --no-edit "*) exit 1;; esac'); + expect(drain(shim.env).status).toBe(0); + expect(statusJson().status).toBe('push_failed'); + expect(git(['rev-list', '--left-right', '--count', 'HEAD...origin/main']).stdout.trim()).toBe('1\t1'); + const c = logDecision(tmpHome, 'another decision on machine B'); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + await expectPublishedSnapshotRebuiltFromMergedLog([seed, a, b, c]); + }); + + test('a merge that would bring back a decision the other machine redacted and compacted away still aborts', () => { + const seed = setupMachineB('current'); + const x = machineAPushesADecision(); + const b = logDecision(tmpHome, 'decision on machine B'); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + expect(git(['pull', '-q', '--ff-only'], machineA).status).toBe(0); + for (const args of [['--redact', x], ['--compact']]) { + const r = spawnSync(path.join(BIN, 'gstack-decision-log'), args, { + cwd: proj, + env: { ...process.env, HOME: tmpHome, GSTACK_HOME: machineA }, + encoding: 'utf-8', + timeout: 30_000, + }); + expect(r.status).toBe(0); + } + expect(fs.readFileSync(path.join(machineA, ALOG), 'utf-8')).not.toContain(x); + expect(git(['add', '-f', '--', ALOG, ASNAP], machineA).status).toBe(0); + expect(git(['-c', 'user.name=peer', '-c', 'user.email=peer@example.com', 'commit', '-qm', 'redact and compact on A'], machineA).status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD'], machineA).status).toBe(0); + const c = logDecision(tmpHome, 'another decision on machine B'); + expect(drain().status).toBe(0); + const s = statusJson(); + expect(s.status).toBe('push_failed'); + expect(s.message).toContain(`Merge conflict in ${ASNAP}`); + expect(mergeHead()).toBe(false); + expect(unmerged()).toBe(''); + expect(remoteShow(ALOG).stdout).not.toContain(x); + expect(remoteShow(ASNAP).stdout).not.toContain(x); + expect(remoteShow(ALOG).stdout).not.toContain(c); + expect([seed, b].every((id) => remoteShow(ALOG).stdout.includes(id))).toBe(true); + }); + + test('a decision log that stash pop brings back with an expunged decision stays held and never reaches the remote', () => { + setupMachineB('current'); + const x = machineAPushesADecision(); + logDecision(tmpHome, 'decision on machine B'); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + expect(git(['pull', '-q', '--ff-only'], machineA).status).toBe(0); + for (const args of [['--redact', x], ['--compact']]) { + const r = spawnSync(path.join(BIN, 'gstack-decision-log'), args, { + cwd: proj, + env: { ...process.env, HOME: tmpHome, GSTACK_HOME: machineA }, + encoding: 'utf-8', + timeout: 30_000, + }); + expect(r.status).toBe(0); + } + expect(git(['add', '-f', '--', ALOG, ASNAP], machineA).status).toBe(0); + expect(git(['-c', 'user.name=peer', '-c', 'user.email=peer@example.com', 'commit', '-qm', 'redact and compact on A'], machineA).status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD'], machineA).status).toBe(0); + const compactedSnapshot = remoteShow(ASNAP).stdout; + write(CLEAN, '# a plan from machine B\n'); + const marker = path.join(tmpHome, 'late-decision-logged'); + const late = JSON.stringify({ decision: 'decision logged during the merge', rationale: 'r', scope: 'repo', source: 'user' }); + const shim = gitShim( + `case " $* " in *" push "*) [ -e '${marker}' ] || { : > '${marker}'; (cd '${proj}' && HOME='${tmpHome}' GSTACK_HOME='${tmpHome}' '${path.join(BIN, 'gstack-decision-log')}' '${late}' >/dev/null); };; esac`, + ); + expect(drain(shim.env).status).toBe(0); + expect(fs.existsSync(marker)).toBe(true); + expect(fs.readFileSync(path.join(tmpHome, ALOG), 'utf-8')).toContain(x); + expect(drain().status).toBe(0); + const s = statusJson(); + expect(s.status).not.toBe('ok'); + expect(`${s.status} ${s.message ?? ''}`).toMatch(/held|consistent generation/); + expect(remoteShow(ALOG).stdout).not.toContain(x); + expect(remoteShow(ASNAP).stdout).toBe(compactedSnapshot); + expect(remoteShow(CLEAN).stdout).toContain('a plan from machine B'); + }); + + test('a conflict in another file still aborts the merge and leaves the snapshot alone', () => { + setupMachineB('current'); + machineAPushesADecision(); + expect(git(['clone', '-q', bareRemote, machineA + '-x']).status).toBe(0); + try { + fs.writeFileSync(path.join(machineA + '-x', 'developer-profile.json'), '{"note":"from the peer"}\n'); + expect(git(['add', '-f', '--', 'developer-profile.json'], machineA + '-x').status).toBe(0); + expect(git(['-c', 'user.name=peer', '-c', 'user.email=peer@example.com', 'commit', '-qm', 'peer profile'], machineA + '-x').status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD'], machineA + '-x').status).toBe(0); + } finally { + fs.rmSync(machineA + '-x', { recursive: true, force: true }); + } + logDecision(tmpHome, 'decision on machine B'); + write('developer-profile.json', '{"note":"local"}\n'); + const before = fs.readFileSync(path.join(tmpHome, ASNAP), 'utf-8'); + expect(drain().status).toBe(0); + const s = statusJson(); + expect(s.status).toBe('push_failed'); + expect(s.message).toContain('Merge conflict in developer-profile.json'); + expect(mergeHead()).toBe(false); + expect(unmerged()).toBe(''); + expect(fs.readFileSync(path.join(tmpHome, ASNAP), 'utf-8')).toBe(before); + }); + }); }); describe('a synced JSON file that does not parse is never committed (#3763)', () => { diff --git a/test/gstack-decision-bins.test.ts b/test/gstack-decision-bins.test.ts index c54a44040..c91224078 100644 --- a/test/gstack-decision-bins.test.ts +++ b/test/gstack-decision-bins.test.ts @@ -109,6 +109,24 @@ describe("gstack-decision-log", () => { .toHaveLength(2); }); + test("--rebuild-snapshot rewrites one project's snapshot from its log and refuses anything else", () => { + const stateDir = path.join(tmpDir, "rebuild-state"); + const kept = invoke(['{"decision":"Keep this one"}'], stateDir).stdout.trim(); + const retired = invoke(['{"decision":"Retire this one"}'], stateDir).stdout.trim(); + expect(invoke(["--redact", retired], stateDir).status).toBe(0); + const snapshot = path.join(stateDir, "projects", "help-contract", "decisions.active.json"); + fs.writeFileSync(snapshot, "<<<<<<< HEAD\n[]\n=======\n[]\n>>>>>>> origin/main\n"); + const r = invoke(["--rebuild-snapshot", "help-contract"], stateDir); + expect(r.status).toBe(0); + expect(r.stdout).toBe(""); + expect(JSON.parse(fs.readFileSync(snapshot, "utf-8")).map((d: { id: string }) => d.id)).toEqual([kept]); + for (const args of [["--rebuild-snapshot"], ["--rebuild-snapshot", ""], ["--rebuild-snapshot", ".."], ["--rebuild-snapshot", "help-contract/x"], ["--rebuild-snapshot", "no-such-project"]]) { + const bad = invoke(args, stateDir); + expect(bad.status).toBe(1); + expect(bad.stderr).toContain("--rebuild-snapshot needs"); + } + }); + test("help does not relax captured invalid scopes or payload validation", () => { const stateDir = path.join(tmpDir, "invalid-state"); for (const [payload, message] of [