From 409704a63f86c78a718643f0be71bd0f1c3701ab Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:16:09 -0700 Subject: [PATCH 01/11] feat: final reviewers run focused checks in their own scratch folder --- run.workflow.js | 12 ++++++++++-- src/prompts-phase.js | 4 ++++ src/rules.js | 8 ++++++-- tests/engine.test.mjs | 11 ++++++++++- tests/phase-prompts.test.mjs | 35 +++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 5 deletions(-) diff --git a/run.workflow.js b/run.workflow.js index c85f7b3..26559ec 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -64,9 +64,13 @@ function agentRules() { 'and the Bash tool fails on a command of about 7,000 characters or more (a long heredoc is one). ' + 'To add to an existing file such as a report, Read it and then Edit it, or Write the whole file. ' + "Running the project's own tools that change files is fine: generators (for example scripts/build), " + - 'formatters, codemods, and other maintenance commands.', + 'formatters, codemods, and other maintenance commands. ' + + 'Edit and Write turn a backslash-u escape you type into the character itself: to keep such an escape in ' + + 'a file (for example where the project wants plain ASCII), generate that text with a script (for example ' + + "json.dumps(..., ensure_ascii=True)) and check the file's bytes before you commit.", 'Read a file right before you edit or overwrite it (and again after anything else may have changed it): ' + - 'an Edit or Write on a file not read first, or changed since, fails and costs a retry.', + 'an Edit or Write on a file not read first, or changed since, fails and costs a retry. ' + + 'Read it with the Read tool: output of grep, sed or cat through Bash does not count.', ].join('\n'); } @@ -3233,6 +3237,10 @@ function finalReviewFrame(m, intro, focus, minors = [], other = false) { 'Also scan every commit message in the range and the whole diff for anything the commit rules forbid;', 'report each as a finding (for a commit message use file "commit " and line 0).', ...fixMessagesLines(), + 'Run only the focused checks a finding needs; do not run the full project commands.' + + (finalChecks(m).length > 0 ? ' Verify runs every check at the delivered revision after the review.' : ''), + 'Put any scratch copy or file in your own folder made with mktemp -d, outside the checkout,' + + " the project and the session's scratchpad, and remove it when done.", phaseRules(m), '', 'Return findings = [{severity ("critical", "important", or "minor"), file, line (0 when no single line', diff --git a/src/prompts-phase.js b/src/prompts-phase.js index 71a9c56..55dd70a 100644 --- a/src/prompts-phase.js +++ b/src/prompts-phase.js @@ -962,6 +962,10 @@ function finalReviewFrame(m, intro, focus, minors = [], other = false) { 'Also scan every commit message in the range and the whole diff for anything the commit rules forbid;', 'report each as a finding (for a commit message use file "commit " and line 0).', ...fixMessagesLines(), + 'Run only the focused checks a finding needs; do not run the full project commands.' + + (finalChecks(m).length > 0 ? ' Verify runs every check at the delivered revision after the review.' : ''), + 'Put any scratch copy or file in your own folder made with mktemp -d, outside the checkout,' + + " the project and the session's scratchpad, and remove it when done.", phaseRules(m), '', 'Return findings = [{severity ("critical", "important", or "minor"), file, line (0 when no single line', diff --git a/src/rules.js b/src/rules.js index 2078af2..93f149b 100644 --- a/src/rules.js +++ b/src/rules.js @@ -49,9 +49,13 @@ function agentRules() { 'and the Bash tool fails on a command of about 7,000 characters or more (a long heredoc is one). ' + 'To add to an existing file such as a report, Read it and then Edit it, or Write the whole file. ' + "Running the project's own tools that change files is fine: generators (for example scripts/build), " + - 'formatters, codemods, and other maintenance commands.', + 'formatters, codemods, and other maintenance commands. ' + + 'Edit and Write turn a backslash-u escape you type into the character itself: to keep such an escape in ' + + 'a file (for example where the project wants plain ASCII), generate that text with a script (for example ' + + "json.dumps(..., ensure_ascii=True)) and check the file's bytes before you commit.", 'Read a file right before you edit or overwrite it (and again after anything else may have changed it): ' + - 'an Edit or Write on a file not read first, or changed since, fails and costs a retry.', + 'an Edit or Write on a file not read first, or changed since, fails and costs a retry. ' + + 'Read it with the Read tool: output of grep, sed or cat through Bash does not count.', ].join('\n'); } diff --git a/tests/engine.test.mjs b/tests/engine.test.mjs index 1547e81..bfae1d7 100644 --- a/tests/engine.test.mjs +++ b/tests/engine.test.mjs @@ -1305,9 +1305,15 @@ test('every agent gets the file-editing rule', () => { 'generators (for example scripts/build), formatters, codemods', // f1a 17, hx1 10, d1a 12 edit_retry slowdowns: an Edit or Write on a file not read first fails. 'Read a file right before you edit or overwrite it', + // 1.6.7 (spec 4.7): Edit and Write turn a typed escape into the character. + 'turn a backslash-u escape you type into the character itself', + 'ensure_ascii=True', + 'Read it with the Read tool: output of grep, sed or cat through Bash does not count.', ]) { assert.ok(r.includes(phrase), phrase); } + // The escape rule gives no literal escape as its example. + assert.ok(!r.includes('\\'), r); }); test('the agent rules are plain ASCII', () => { @@ -1511,9 +1517,12 @@ test('prompts carry only the project rules: no ASCII or trailer rule of their ow postIntegrate: postIntegratePrompt(m), preflight: preflightPrompt(m), }; + // The agent rules' escape sentence (1.6.7, spec 4.7) names plain ASCII only as an example of + // a project that wants it; it imposes no ASCII rule. + const escape = agentRules().match(/Edit and Write turn a backslash-u escape[^\n]*?before you commit\./)[0]; for (const [name, text] of Object.entries(prompts)) { assert.ok(text.includes(rules), `${name}: the project's rules`); - const own = text.split(rules).join(''); + const own = text.split(rules).join('').split(escape).join(''); assert.ok(!/ASCII/i.test(own), `${name}: an ASCII rule of its own`); assert.ok(!/trailer|co-authored|AI (name|attribution)|Claude/i.test(own), `${name}: a trailer or AI rule of its own`); } diff --git a/tests/phase-prompts.test.mjs b/tests/phase-prompts.test.mjs index 1422202..7bec0a1 100644 --- a/tests/phase-prompts.test.mjs +++ b/tests/phase-prompts.test.mjs @@ -386,3 +386,38 @@ test('task prompts tell agents to set PL_PHASE for commands they run themselves' const lane = flat(implementPrompt(m, m.lanes[0].tasks[0], laneWhere(m, m.lanes[0]), H('b'))); assert.ok(lane.includes('PL_PHASE=lane'), lane); }); + +// ---- 1.6.7: final reviewers run focused checks in their own scratch folder (spec 4.6) ---- + +const { finalReviewPrompt, combinedFinalReviewPrompt } = await loadHelpers(['finalReviewPrompt', 'combinedFinalReviewPrompt']); + +const FOCUSED = 'Run only the focused checks a finding needs; do not run the full project commands.'; +const VERIFY_RUNS = ' Verify runs every check at the delivered revision after the review.'; +const SCRATCH = 'Put any scratch copy or file in your own folder made with mktemp -d, outside the checkout,' + + " the project and the session's scratchpad, and remove it when done."; + +test('final reviewers run focused checks in their own scratch folder', () => { + const finalPrompts = (m) => [ + ...['sp', 'security', 'correctness'].map((lens) => [lens, finalReviewPrompt(m, lens, null)]), + ['lite', combinedFinalReviewPrompt(m, { e2e: null })], + ]; + const m = manifest(); + assert.ok(finalChecks(m).length > 0); + for (const [name, p] of finalPrompts(m)) { + const lines = p.split('\n'); + assert.ok(lines.includes(FOCUSED + VERIFY_RUNS), `${name}: ${p}`); + assert.ok(lines.includes(SCRATCH), `${name}: ${p}`); + // Both come before the phase rules. + const rulesAt = p.indexOf('Never amend, rebase, reset, or force-update a branch.'); + assert.ok(rulesAt > p.indexOf(SCRATCH) && p.indexOf(SCRATCH) > p.indexOf(FOCUSED), `${name}: order`); + } + // No project checks: nothing for verify to run, so no verify sentence. + const bare = manifest({ commands: { setup: [], test: [], lint: [], build: [] } }); + assert.equal(finalChecks(bare).length, 0); + for (const [name, p] of finalPrompts(bare)) { + const lines = p.split('\n'); + assert.ok(lines.includes(FOCUSED), `${name}: ${p}`); + assert.ok(!p.includes(VERIFY_RUNS.trim()), `${name}: ${p}`); + assert.ok(lines.includes(SCRATCH), `${name}: ${p}`); + } +}); From 4cbcf9733dc28da89d26825352f6d2c9f9b60fe2 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:26:48 -0700 Subject: [PATCH 02/11] feat: setup records the run's git identity from the commit rules --- scripts/_doctor.py | 61 +++++++++++++++---------------------- scripts/_identity.py | 48 +++++++++++++++++++++++++++++ scripts/setup | 11 +++++-- tests/doctor.test.mjs | 11 +++++++ tests/identity.test.mjs | 54 ++++++++++++++++++++++++++++++++ tests/setup-script.test.mjs | 15 +++++++-- 6 files changed, 159 insertions(+), 41 deletions(-) create mode 100644 scripts/_identity.py create mode 100644 tests/identity.test.mjs diff --git a/scripts/_doctor.py b/scripts/_doctor.py index dc210ac..56f859b 100644 --- a/scripts/_doctor.py +++ b/scripts/_doctor.py @@ -36,9 +36,11 @@ the upstream tip is read with git ls-remote. Hard failures (they block a launch): -- git_identity: git var GIT_COMMITTER_IDENT fails with user.useConfigOnly - set (so git does not invent user@host), after the -c user.name=... and - -c user.email=... pairs commit_rules passes. +- git_identity: git var GIT_COMMITTER_IDENT or git var GIT_AUTHOR_IDENT + fails with user.useConfigOnly set (so git does not invent user@host), + after the -c user.name=... and -c user.email=... pairs commit_rules passes + (_identity.identity_pairs). A commit needs both: a committer identity + alone (GIT_COMMITTER_NAME/EMAIL) still fails "Author identity unknown". - base_not_current: the local repo.base_ref does not contain its upstream's tip. The detail is behind (the base is an ancestor of the tip), diverged (neither contains the other) or unknown (the tip is not in the local repo, @@ -99,6 +101,7 @@ import tempfile import time +import _identity import _shell # The skill's folder: the scripts folder's parent, which holds VERSION. @@ -514,45 +517,29 @@ def _git_prefix(repo, probes): return ["-C", root] -# -c NAME=VALUE in commit_rules: the pair quoted, NAME="VALUE", or a bare word. -_CONFIG_PAIR = re.compile( - r"""(?:^|[\s`(])-c\s+(?:"([^"]*)"|'([^']*)'|([\w.]+=(?:"[^"]*"|'[^']*'))|([^\s`]+))""") -_IDENTITY_KEYS = ("user.name=", "user.email=") - - -def identity_pairs(commit_rules): - """The -c user.name=... and -c user.email=... pairs commit_rules passes.""" - pairs = [] - for m in _CONFIG_PAIR.finditer(commit_rules or ""): - if m.group(4) is not None: - # A bare word may end a sentence or a parenthesis. - pair = m.group(4).rstrip(".,;:)") - else: - pair = next(g for g in m.groups()[:3] if g is not None) - key, eq, value = pair.partition("=") - if eq and (key.lower() + "=") in _IDENTITY_KEYS and value.strip(" \"'"): - pairs.append(key + "=" + value.strip(" \"'")) - return pairs +# The commit rules' -c user.name/user.email pairs; kept here under its old +# name for any caller of _doctor.identity_pairs. +identity_pairs = _identity.identity_pairs def check_git_identity(manifest, probes, prefix, report): - rules = manifest.get("commit_rules") - pairs = identity_pairs(rules if isinstance(rules, str) else "") + """git commit needs both identities: the committer's, then the author's.""" args = list(prefix) if prefix else ["-C", tempfile.gettempdir()] - for pair in pairs: + for pair in identity_pairs(manifest.get("commit_rules")): args += ["-c", pair] - args += ["-c", "user.useConfigOnly=true", "var", "GIT_COMMITTER_IDENT"] - res = probes.git(args) - if res.code == 0 and res.out.strip(): - report.passed("git_identity") - return - report.fail( - "git_identity", - "git has no committer identity (git var GIT_COMMITTER_IDENT fails)", - "set one with git config --global user.name NAME and git config --global " - "user.email EMAIL, or name one that commit_rules passes as git -c " - "user.name=... -c user.email=... commit.", - ) + args += ["-c", "user.useConfigOnly=true", "var"] + for who, var in (("committer", "GIT_COMMITTER_IDENT"), ("author", "GIT_AUTHOR_IDENT")): + res = probes.git(args + [var]) + if res.code != 0 or not res.out.strip(): + report.fail( + "git_identity", + "git has no " + who + " identity (git var " + var + " fails)", + "set one with git config --global user.name NAME and git config --global " + "user.email EMAIL, or name one that commit_rules passes as git -c " + "user.name=... -c user.email=... commit.", + ) + return + report.passed("git_identity") def _ancestor(probes, prefix, a, b): diff --git a/scripts/_identity.py b/scripts/_identity.py new file mode 100644 index 0000000..572b429 --- /dev/null +++ b/scripts/_identity.py @@ -0,0 +1,48 @@ +"""The git identity a run's commit rules pass: -c user.name=... and -c user.email=... + +Import with: + sys.dont_write_bytecode = True + sys.path.insert(0, os.path.dirname(os.path.realpath(__file__))) + import _identity + +identity_pairs(commit_rules) returns the -c pairs for user.name and +user.email that the text passes, as ["user.name=VALUE", "user.email=VALUE"]: +keys in lower case, at most one pair per key (the last one given wins), +user.name first. A pair is written -c "NAME=VALUE", -c 'NAME=VALUE', +-c NAME="VALUE" (or 'VALUE') or -c NAME=VALUE; a bare word loses a trailing +. , ; : or ) (it may end a sentence or a parenthesis). A pair with an empty +value is not a pair. Text with no such pair, or a value that is not a +string, gives []. + +setup prints the result as setup_result.git_identity; doctor's git_identity +check passes the pairs to git var. +""" + +import re +import sys + +sys.dont_write_bytecode = True + +# -c NAME=VALUE: the pair quoted, NAME="VALUE", or a bare word. +_CONFIG_PAIR = re.compile( + r"""(?:^|[\s`(])-c\s+(?:"([^"]*)"|'([^']*)'|([\w.]+=(?:"[^"]*"|'[^']*'))|([^\s`]+))""") +_IDENTITY_KEYS = ("user.name", "user.email") + + +def identity_pairs(commit_rules): + """The -c user.name=... and -c user.email=... pairs commit_rules passes.""" + if not isinstance(commit_rules, str): + return [] + values = {} + for m in _CONFIG_PAIR.finditer(commit_rules): + if m.group(4) is not None: + # A bare word may end a sentence or a parenthesis. + pair = m.group(4).rstrip(".,;:)") + else: + pair = next(g for g in m.groups()[:3] if g is not None) + key, eq, value = pair.partition("=") + key = key.lower() + value = value.strip(" \"'") + if eq and key in _IDENTITY_KEYS and value: + values[key] = value + return [key + "=" + values[key] for key in _IDENTITY_KEYS if key in values] diff --git a/scripts/setup b/scripts/setup index 1b7bd30..401487b 100755 --- a/scripts/setup +++ b/scripts/setup @@ -94,8 +94,13 @@ On success setup appends {"task":"_run","event":"run_started","phase": to the ledger lane _run in repo.ledger_dir (ledger status --plan/--spec compares the hashes on a resume) and prints the manifest's setup_result: {"feature_head": , "worktrees": {: }, - "discarded": [...], "preserved": [...], "setup_skipped": []} + "discarded": [...], "preserved": [...], "setup_skipped": [], + "returned": [{"worktree", "from"}], "git_identity": ["user.name=...", "user.email=..."]} The worktree paths are exactly the paths the run uses for the lanes. +git_identity is _identity.identity_pairs(commit_rules): the -c user.name and +user.email pairs the manifest's commit rules pass (keys lower case, at most +one per key, user.name first), or [] when they pass none; the run passes +them to the merges and commits it spells out. On Windows MANIFEST and repo.root, repo.git_dir, repo.worktree_root and repo.ledger_dir may be in any form (C:\\x, /c/x, /tmp/x); they are used, and @@ -120,6 +125,7 @@ import sys sys.dont_write_bytecode = True sys.path.insert(0, os.path.dirname(os.path.realpath(__file__))) +import _identity # noqa: E402 import _locks # noqa: E402 import _owners # noqa: E402 import _shell # noqa: E402 @@ -479,7 +485,8 @@ def setup(m, owner=None): raise Failure(1, f"cannot write the owner map {owners_file!r}: {e}") worktrees = {lane["id"]: feature if lite else lane_dir(m, lane) for lane in m["lanes"]} return {"feature_head": head, "worktrees": worktrees, "discarded": discarded, "preserved": preserved, - "setup_skipped": skipped, "returned": returned} + "setup_skipped": skipped, "returned": returned, + "git_identity": _identity.identity_pairs(m.get("commit_rules"))} def main(argv): diff --git a/tests/doctor.test.mjs b/tests/doctor.test.mjs index c177e28..b03ac35 100644 --- a/tests/doctor.test.mjs +++ b/tests/doctor.test.mjs @@ -156,6 +156,17 @@ test('a missing git identity fails unless commit_rules passes one', () => { assert.equal(named(ok.out.failures, 'git_identity'), undefined); }); +test('a committer-only identity fails git_identity', () => { + // git commit needs an author too: a committer identity alone is not enough. + const f = fixture(); + const env = { ...NO_IDENTITY, GIT_COMMITTER_NAME: 'c', GIT_COMMITTER_EMAIL: 'c@x.invalid' }; + const r = doctor(f.manifest(), { env }); + assert.equal(r.code, 3, r.stderr); + assert.equal(r.out.status, 'blocked'); + assert.ok(named(r.out.failures, 'git_identity'), JSON.stringify(r.out)); + assert.equal(outcome(r.out, 'git_identity'), 'failed'); +}); + test('a base behind its upstream fails a new run', () => { const f = fixture(); f.advanceOrigin('b.txt'); diff --git a/tests/identity.test.mjs b/tests/identity.test.mjs new file mode 100644 index 0000000..e794d73 --- /dev/null +++ b/tests/identity.test.mjs @@ -0,0 +1,54 @@ +// scripts/_identity.py: the git identity a run's commit rules pass as +// -c user.name=... and -c user.email=... (setup's git_identity, doctor's +// git_identity check). +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { join } from 'node:path'; +import { SKILL_DIR } from './harness.mjs'; + +const SCRIPTS = join(SKILL_DIR, 'scripts'); + +// identity_pairs(text) through a Python child with scripts/ importable (no +// bytecode); text goes in on stdin as JSON, the pairs come back as JSON. +function identityPairs(text) { + const code = [ + 'import json, sys', + 'sys.dont_write_bytecode = True', + `sys.path.insert(0, ${JSON.stringify(SCRIPTS)})`, + 'import _identity', + 'print(json.dumps(_identity.identity_pairs(json.load(sys.stdin))))', + ].join('\n'); + const r = spawnSync('python3', ['-B', '-c', code], { encoding: 'utf8', input: JSON.stringify(text) }); + assert.equal(r.status, 0, r.stderr); + return JSON.parse(r.stdout); +} + +test('identity_pairs: bare pairs in the order name, email', () => { + assert.deepEqual(identityPairs('git -c user.name=a -c user.email=b commit'), ['user.name=a', 'user.email=b']); +}); + +test('identity_pairs: user.name comes first and a bare word loses a sentence end', () => { + assert.deepEqual(identityPairs('git -c user.email=b -c user.name=a commit.'), ['user.name=a', 'user.email=b']); + assert.deepEqual(identityPairs('(git -c user.name=a -c user.email=b)'), ['user.name=a', 'user.email=b']); +}); + +test('identity_pairs: quoted pairs keep their spaces', () => { + assert.deepEqual( + identityPairs('git -c "user.name=Riley Q" -c \'user.email=r@x.invalid\' commit'), + ['user.name=Riley Q', 'user.email=r@x.invalid'], + ); + assert.deepEqual(identityPairs('git -c user.name="Riley Q" commit'), ['user.name=Riley Q']); +}); + +test('identity_pairs: keys are lower case and the last pair of a key wins', () => { + assert.deepEqual(identityPairs('git -c User.Name=a -c user.name=c commit'), ['user.name=c']); +}); + +test('identity_pairs: no identity pair gives an empty list', () => { + assert.deepEqual(identityPairs('plain ASCII, no trailers'), []); + assert.deepEqual(identityPairs('git -c core.autocrlf=false commit'), []); + assert.deepEqual(identityPairs(''), []); + assert.deepEqual(identityPairs(null), []); + assert.deepEqual(identityPairs(['git -c user.name=a commit']), []); +}); diff --git a/tests/setup-script.test.mjs b/tests/setup-script.test.mjs index 59fa9ee..3044a12 100644 --- a/tests/setup-script.test.mjs +++ b/tests/setup-script.test.mjs @@ -170,7 +170,9 @@ test('setup: a fresh git run creates the branch and lane worktrees and prints se const base = git(c.project, 'rev-parse', 'main'); const { worktrees, ...result } = setupOk(c, manifest(c)); - assert.deepEqual(result, { feature_head: base, discarded: [], preserved: [], setup_skipped: [], returned: [] }); + assert.deepEqual(result, { + feature_head: base, discarded: [], preserved: [], setup_skipped: [], returned: [], git_identity: [], + }); assertSamePaths(worktrees, { a: laneDir(c, 'a'), b: laneDir(c, 'b') }); assert.equal(git(c.project, 'rev-parse', '--abbrev-ref', 'HEAD'), 'feature/x'); assert.equal(git(laneDir(c, 'a'), 'rev-parse', '--abbrev-ref', 'HEAD'), 'pl-r1-a'); @@ -187,6 +189,13 @@ test('setup: a fresh git run creates the branch and lane worktrees and prints se assert.equal(git(c.project, 'status', '--porcelain'), ''); }); +test("setup: git_identity holds the commit rules' pairs", () => { + const c = newCase(); + const rules = 'Commit with git -c user.name=n -c user.email=e@x.invalid commit.'; + const result = setupOk(c, manifest(c, { commit_rules: rules })); + assert.deepEqual(result.git_identity, ['user.name=n', 'user.email=e@x.invalid']); +}); + test('setup: a rerun reuses branches and worktrees and lists the discarded edits', () => { const c = newCase(); const m = manifest(c); @@ -417,7 +426,9 @@ test('setup: shadow mode creates the feature worktree and never touches the proj }); const feature = `${c.worktreeRoot}/feature`; const { worktrees, ...result } = setupOk(c, m); - assert.deepEqual(result, { feature_head: baseline, discarded: [], preserved: [], setup_skipped: [], returned: [] }); + assert.deepEqual(result, { + feature_head: baseline, discarded: [], preserved: [], setup_skipped: [], returned: [], git_identity: [], + }); assertSamePaths(worktrees, { a: laneDir(c, 'a'), b: laneDir(c, 'b') }); assert.equal(git(feature, 'rev-parse', '--abbrev-ref', 'HEAD'), 'pl-r1'); assert.equal(git(laneDir(c, 'a'), 'rev-parse', '--abbrev-ref', 'HEAD'), 'pl-r1-a'); From 67daa8ffd1e5573d542700abac0e21a3248c1963 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:30:42 -0700 Subject: [PATCH 03/11] feat: merge-wave commits with the run's identity and reports a refused merge --- scripts/ledger | 50 ++++++-- scripts/merge-wave | 234 ++++++++++++++++++++++++---------- tests/join-waves-e2e.test.mjs | 6 +- tests/ledger-waves.test.mjs | 61 +++++++++ tests/merge-wave.test.mjs | 199 ++++++++++++++++++++++++++--- 5 files changed, 455 insertions(+), 95 deletions(-) diff --git a/scripts/ledger b/scripts/ledger index 982ddd0..b0a4e9f 100755 --- a/scripts/ledger +++ b/scripts/ledger @@ -48,10 +48,15 @@ and an 'event' of: carried task _run only, with 'wf', 'pf' ({"PF-n": ruling id}) and 'bound' ([{file, line}]); carry-launch writes it last, once per launch - wave_merged task _run only, with 'units': non-empty list of unit ids, - 'head', and exactly one of 'wave' (integer >= 1) and - 'recovery': true; optional 'overlaps' ([{path, units}]) and - 'conflict_files' (paths); merge-wave writes it + wave_merged task _run only, with 'units': list of unit ids (in the + history after the merge; empty only with non-empty + 'conflict_files' or a 'failed'), 'head', and exactly one of + 'wave' (integer >= 1) and 'recovery': true; optional + 'overlaps' ([{path, units}]), 'conflict_files' (paths), and + (1.6.7) 'merged' (ids within 'units' this event merged), + 'conflict_unit' (id), 'failed' ({unit, error}, both + non-empty) and 'resolved' (true); merge-wave writes it, and + the wave resolver with 'resolved' wave_checked task _run only, with 'wave' (integer >= 1), 'head' and 'ok' (true or false); 'unfinished': true when the checks stopped without a result (ok false) @@ -121,7 +126,8 @@ join_started: join.jsonl holds a 'started' event (the join began in an earlier launch: it then runs serially, 1.6.5). unit_branches: {task: {branch, dir}} from each join task's latest 'started' event with a unit_branch, since its latest restart. wave_merges: the _run wave_merged events in order, each -{wave or recovery, units, head, overlaps, conflict_files}. start_bases: {task: +{wave or recovery, units, head, overlaps, conflict_files}, plus merged, +conflict_unit, failed and resolved when recorded (1.6.7). start_bases: {task: base} of each task not done, from its latest 'started' event. A 'started' event with restart true begins its task afresh: its earlier events (committed, reviewed, blocked, settled) no longer count. @@ -224,7 +230,8 @@ its unfinished tasks are skipped. It also prints "join_tip": among the heads of the wave_merged and wave_checked events and the backfill heads of join tasks that did not run on a unit branch, the one every other is an ancestor of (null when there is none); two heads neither of which descends from the -other are an error. +other are an error. A wave_merged event with an empty 'merged' (a conflict or +refusal before any merge) adds no head. Exit 0 on success; 2 on usage error or a malformed entry; 3 on an unsafe lane or task name, an unreadable ledger, plan or manifest, or a backfill error. @@ -414,8 +421,19 @@ def wave_error(entry): if not is_text(entry.get("head")): return "'head' must be a non-empty string" if entry["event"] == "wave_merged": - if not (isinstance(entry.get("units"), list) and entry["units"] and is_text_list(entry["units"])): - return "'units' must be a non-empty list of unit ids" + if not is_text_list(entry.get("units")): + return "'units' must be a list of unit ids" + if "merged" in entry and not (is_text_list(entry["merged"]) + and all(u in entry["units"] for u in entry["merged"])): + return "'merged' must be a list of ids within 'units'" + if "conflict_unit" in entry and not is_text(entry["conflict_unit"]): + return "'conflict_unit' must be a non-empty string" + failed = entry.get("failed") + if "failed" in entry and not (isinstance(failed, dict) and is_text(failed.get("unit")) + and is_text(failed.get("error"))): + return "'failed' must be {unit, error}, both non-empty strings" + if "resolved" in entry and entry["resolved"] is not True: + return "'resolved' must be true when present" has_wave, has_recovery = "wave" in entry, "recovery" in entry if has_wave == has_recovery: return "a wave_merged event has exactly one of 'wave' and 'recovery'" @@ -429,6 +447,9 @@ def wave_error(entry): return "'overlaps' must be a list of {path, units}" if not is_text_list(entry.get("conflict_files", [])): return "'conflict_files' must be a list of paths" + # Empty units (1.6.7): a conflict or refusal before any unit merged. + if not entry["units"] and not (entry.get("conflict_files") or "failed" in entry): + return "'units' may be empty only with non-empty 'conflict_files' or a 'failed'" else: if not is_count(entry.get("wave"), 1): return "'wave' must be an integer >= 1" @@ -799,7 +820,8 @@ def join_facts(events_at): unit_branches ({task: {branch, dir}} from each join task's latest started event with a unit_branch since its latest restart), and wave_merges (the wave_merged events of _run.jsonl in order, as {wave or - recovery, units, head, overlaps, conflict_files}).""" + recovery, units, head, overlaps, conflict_files}, plus merged, + conflict_unit, failed and resolved when the event has them).""" started, branches, merges = False, {}, [] for name, _, entry in events_at: event = entry["event"] @@ -813,6 +835,7 @@ def join_facts(events_at): item = {"wave": entry["wave"]} if "wave" in entry else {"recovery": True} item.update({"units": entry["units"], "head": entry["head"], "overlaps": entry.get("overlaps", []), "conflict_files": entry.get("conflict_files", [])}) + item.update({k: entry[k] for k in ("merged", "conflict_unit", "failed", "resolved") if k in entry}) merges.append(item) return {"join_started": started, "unit_branches": branches, "wave_merges": merges} @@ -820,11 +843,16 @@ def join_facts(events_at): def join_tip(events, ranges, unit_branches, git): """The join's tip: among the heads of wave_merged and wave_checked events and the recorded heads of join tasks that did not run on a unit - branch, the one every other is an ancestor of. Returns (tip or None, - error or None).""" + branch, the one every other is an ancestor of. A wave_merged event whose + merged is present and empty (a conflict or refusal before any merge, + 1.6.7) records the checkout's head, not a join merge, and is skipped; one + with no merged field (before 1.6.7) counts. Returns (tip or None, error + or None).""" heads = [] for entry in events: if entry["task"] == "_run" and entry["event"] in ("wave_merged", "wave_checked"): + if entry["event"] == "wave_merged" and entry.get("merged") == []: + continue heads.append(entry["head"]) for task, rng in ranges.items(): if task not in unit_branches: diff --git a/scripts/merge-wave b/scripts/merge-wave index 872d11f..3954627 100755 --- a/scripts/merge-wave +++ b/scripts/merge-wave @@ -3,17 +3,31 @@ usage: merge-wave DIR LEDGER_DIR (--wave N | --recovery) [--unit ID BASE HEAD ...] [--known ID HEAD ...] + [--git-config KEY=VALUE ...] -DIR is the feature checkout, on the feature branch, and must be clean -(git status --porcelain --untracked-files=all prints nothing), else exit 3 -with nothing changed. Each --unit, in the order given (the engine passes -plan order), is a unit of the wave: its id, and the recorded base and head -of its range on its own branch (by sha, never by branch name). A unit whose -HEAD is already in DIR's history is skipped; any other is merged with -git merge --no-ff -m "join wave N: merge ID" (with --recovery, "join -recovery: merge ID": a relaunch bringing in units an earlier launch finished -but did not merge). A conflict aborts that merge (git merge --abort, so DIR -is clean again) and ends the loop: the units after it are not tried. +Options come in any order. DIR is the feature checkout, on the feature +branch, and must be clean (git status --porcelain --untracked-files=all +prints nothing), else exit 3 with nothing changed. Each --unit, in the order +given (the engine passes plan order), is a unit of the wave: its id, and the +recorded base and head of its range on its own branch (by sha, never by +branch name). A unit whose HEAD is already in DIR's history is skipped; any +other is merged with git merge --no-ff -m "chore: join wave N: merge ID" +(with --recovery, "chore: join recovery: merge ID": a relaunch bringing in +units an earlier launch finished but did not merge). + +Each --git-config KEY=VALUE (1.6.7: the run's identity from +setup_result.git_identity) goes to every git merge as one -c KEY=VALUE argv +item. KEY is user.name or user.email in any case (passed lower case) and +VALUE is non-empty; anything else is a usage error. + +A conflict (git merge fails with unmerged paths) aborts that merge (git +merge --abort, so DIR is clean again) and ends the loop: the units after it +are not tried. A merge git refuses with no unmerged paths (no identity, a +failing hook, a held index.lock) is a failure, not a conflict: merge-wave +runs git merge --abort only when a merge is in progress (MERGE_HEAD exists), +checks DIR is clean, and ends the loop. When git's message names index.lock +and DIR is clean, it first waits 2 s and tries that unit's merge once more. +Units merged before the refused one stay merged. Each --known ID HEAD (a relaunch's recovery merge) is a unit that ran on its own branch and is not to be merged: in_history lists, after the merges, every @@ -24,22 +38,30 @@ overlaps: the paths that two or more units of this call changed (each git diff --name-only BASE HEAD), among those now in DIR's history, as [{path, units}] sorted by path, reported even when the merge was clean. -When some unit is in DIR's history and this call merged one or hit a -conflict, it appends to LEDGER_DIR/_run.jsonl {"task": "_run", "event": -"wave_merged", "wave": N (or "recovery": true), "units": [the units in DIR's -history, in the given order], "head", "overlaps", "conflict_files"}. +When this call merged a unit, hit a conflict or was refused, it appends to +LEDGER_DIR/_run.jsonl {"task": "_run", "event": "wave_merged", "wave": N (or +"recovery": true), "units": [the units in DIR's history, in the given +order], "merged": [the units this call merged], "head", "overlaps", +"conflict_files"}, plus "conflict_unit" on a conflict and "failed": {"unit", +"error"} on a refusal. A call that only skipped units records nothing. Prints one JSON object: - {"status": "merged"|"conflict", "head": , + {"status": "merged"|"conflict"|"failed", "head": , "merged": [ids merged now], "skipped": [ids already merged], "conflict_unit": id|null, "conflict_files": [paths], "overlaps": [...], - "in_history": [ids], "notes": [str]} + "in_history": [ids], "notes": [str], "failed_unit": id|null, + "error": str|null} +error is git's message for the refused merge (stderr, else stdout, +stripped, at most 2,000 characters; "git merge exited " when both are +empty). Exit 0 when every unit is merged (or there was none: --recovery may have nothing to merge); 1 on a conflict; 2 on a usage error (no --wave or --recovery, both, a --wave that is not an integer >= 1, --wave with no ---unit, an --unit without three values or with an unsafe id); 3 when DIR is -not a clean git checkout, a BASE or HEAD is not a commit, or a git step -fails. +--unit, an --unit without three values or with an unsafe id, a --git-config +of another key or with an empty value); 3 when DIR is not a clean git +checkout, a BASE or HEAD is not a commit, or a git step fails (a refused +merge whose abort fails, or that leaves DIR not clean, is recorded with +"failed" first); 4 when git refused a merge. """ import json @@ -47,6 +69,7 @@ import os import re import subprocess import sys +import time sys.dont_write_bytecode = True sys.path.insert(0, os.path.dirname(os.path.realpath(__file__))) @@ -57,7 +80,12 @@ GIT_REDIRECTS = ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_OBJECT_DIRECTORY", "GIT_COMMON_DIR") ENV = {k: v for k, v in os.environ.items() if k not in GIT_REDIRECTS} SAFE_ID = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9._-]*$") -USAGE = "usage: merge-wave DIR LEDGER_DIR (--wave N | --recovery) [--unit ID BASE HEAD ...] [--known ID HEAD ...]" +USAGE = ("usage: merge-wave DIR LEDGER_DIR (--wave N | --recovery) [--unit ID BASE HEAD ...] " + "[--known ID HEAD ...] [--git-config KEY=VALUE ...]") +GIT_CONFIG_USAGE = "--git-config takes user.name=VALUE or user.email=VALUE" +IDENTITY_KEYS = ("user.name", "user.email") +ERROR_LIMIT = 2000 +LOCK_RETRY_S = 2 def fail(code, message): @@ -70,11 +98,20 @@ def git(directory, *args): errors="surrogateescape", env=ENV) +def config_pair(text): + """KEY=VALUE as git -c takes it, KEY lower case; a usage error unless + KEY is user.name or user.email and VALUE is non-empty.""" + key, sep, value = text.partition("=") + if not sep or key.lower() not in IDENTITY_KEYS or not value: + fail(2, GIT_CONFIG_USAGE) + return f"{key.lower()}={value}" + + def parse(argv): if len(argv) < 2 or not argv[0] or not argv[1] or argv[0].startswith("-") or argv[1].startswith("-"): fail(2, USAGE) directory, ledger = _shell.native_path(argv[0]), _shell.native_path(argv[1]) - wave, recovery, units, known = None, False, [], [] + wave, recovery, units, known, config = None, False, [], [], [] rest = argv[2:] while rest: if rest[0] == "--wave" and len(rest) >= 2 and wave is None: @@ -93,13 +130,18 @@ def parse(argv): fail(2, f"unsafe unit id {rest[1]!r}") known.append((rest[1], rest[2])) rest = rest[3:] + elif rest[0] == "--git-config": + if len(rest) < 2: + fail(2, GIT_CONFIG_USAGE) + config.append(config_pair(rest[1])) + rest = rest[2:] else: fail(2, USAGE) if (wave is None) == (not recovery): fail(2, "give exactly one of --wave N and --recovery") if wave is not None and not units: fail(2, "--wave needs at least one --unit") - return directory, ledger, wave, units, known + return directory, ledger, wave, units, known, config def commit_of(directory, ref): @@ -123,9 +165,37 @@ def changed(directory, base, head): return [p for p in res.stdout.splitlines() if p] +def unmerged(directory): + files = git(directory, "-c", "core.quotePath=false", "diff", "--name-only", "--diff-filter=U") + return sorted(p for p in files.stdout.splitlines() if p) + + +def git_message(res, what="git merge"): + """git's message for a failed command: stderr, else stdout, stripped and + cut to ERROR_LIMIT characters.""" + text = (res.stderr or "").strip() or (res.stdout or "").strip() + return text[:ERROR_LIMIT] if text else f"{what} exited {res.returncode}" + + +def undo_refused(directory): + """After a merge git refused with no unmerged paths: abort it when a + merge is in progress, then check DIR is clean. Returns None when it is, + else why not.""" + if git(directory, "rev-parse", "-q", "--verify", "MERGE_HEAD").returncode == 0: + aborted = git(directory, "merge", "--abort") + if aborted.returncode != 0: + return f"git merge --abort failed in {directory!r}: {git_message(aborted, 'git merge --abort')}" + status = git(directory, "status", "--porcelain", "--untracked-files=all") + if status.returncode != 0: + return f"git status failed in {directory!r} after the refused merge: {git_message(status, 'git status')}" + if status.stdout.strip(): + return f"{directory!r} is not clean after the refused merge:\n{status.stdout.rstrip()}" + return None + + def main(argv): _shell.setup_io() - directory, ledger, wave, units, known = parse(argv) + directory, ledger, wave, units, known, config = parse(argv) top = git(directory, "rev-parse", "--show-toplevel") if top.returncode != 0: fail(3, f"{directory!r} is not a git checkout") @@ -135,55 +205,87 @@ def main(argv): # Every sha resolves before the first merge: a bad one changes nothing. units = [(uid, commit_of(directory, base), commit_of(directory, head)) for uid, base, head in units] known = [(uid, commit_of(directory, head)) for uid, head in known] + identity = [arg for pair in config for arg in ("-c", pair)] merged, skipped, notes = [], [], [] - conflict_unit, conflict_files = None, [] + outcome = {"conflict_unit": None, "conflict_files": [], "failed_unit": None, "error": None} + + def record(): + """The head, in_history and overlaps after the merges; appends the + wave_merged event when this call merged, conflicted or was refused.""" + head = head_of(directory) + inside = [u for u in units if u[0] in merged or u[0] in skipped] + in_history = [u[0] for u in inside] + [uid for uid, sha in known + if git(directory, "merge-base", "--is-ancestor", sha, + "HEAD").returncode == 0] + by_path = {} + for uid, base, unit_head in inside: + for path in changed(directory, base, unit_head): + by_path.setdefault(path, []).append(uid) + overlaps = [{"path": p, "units": ids} for p, ids in sorted(by_path.items()) if len(ids) > 1] + if merged or outcome["conflict_unit"] is not None or outcome["failed_unit"] is not None: + event = {"task": "_run", "event": "wave_merged"} + event.update({"wave": wave} if wave is not None else {"recovery": True}) + event.update({"units": [u[0] for u in inside], "merged": list(merged), "head": head, + "overlaps": overlaps, "conflict_files": outcome["conflict_files"]}) + if outcome["conflict_unit"] is not None: + event["conflict_unit"] = outcome["conflict_unit"] + if outcome["failed_unit"] is not None: + event["failed"] = {"unit": outcome["failed_unit"], "error": outcome["error"]} + res = subprocess.run([sys.executable, os.path.join(HERE, "ledger"), "append", ledger, "_run", + json.dumps(event)], capture_output=True, text=True, encoding="utf-8", + errors="replace", env=ENV) + if res.returncode != 0: + notes.append(f"recording wave_merged failed: {res.stderr.strip()}") + return head, in_history, overlaps + for uid, _, head in units: if git(directory, "merge-base", "--is-ancestor", head, "HEAD").returncode == 0: skipped.append(uid) continue - message = f"join recovery: merge {uid}" if wave is None else f"join wave {wave}: merge {uid}" - res = git(directory, "merge", "--no-ff", "--no-edit", "-m", message, "--end-of-options", head) - if res.returncode == 0: - merged.append(uid) - continue - files = git(directory, "-c", "core.quotePath=false", "diff", "--name-only", "--diff-filter=U") - conflict_files = sorted(p for p in files.stdout.splitlines() if p) - aborted = git(directory, "merge", "--abort") - if not conflict_files: - # Not a content conflict: git refused the merge for another reason. - notes.append(f"git merge of {uid} failed: {(res.stderr or res.stdout).strip()}") - if aborted.returncode != 0 and git(directory, "status", "--porcelain").stdout.strip(): - fail(3, f"git merge --abort failed in {directory!r}: {aborted.stderr.strip()}") - conflict_unit = uid - break - - head = head_of(directory) - inside = [u for u in units if u[0] in merged or u[0] in skipped] - in_history = [u[0] for u in inside] + [uid for uid, sha in known - if git(directory, "merge-base", "--is-ancestor", sha, "HEAD").returncode == 0] - by_path = {} - for uid, base, unit_head in inside: - for path in changed(directory, base, unit_head): - by_path.setdefault(path, []).append(uid) - overlaps = [{"path": p, "units": ids} for p, ids in sorted(by_path.items()) if len(ids) > 1] - - if inside and (merged or conflict_unit is not None): - event = {"task": "_run", "event": "wave_merged"} - event.update({"wave": wave} if wave is not None else {"recovery": True}) - event.update({"units": [u[0] for u in inside], "head": head, "overlaps": overlaps, - "conflict_files": conflict_files}) - res = subprocess.run([sys.executable, os.path.join(HERE, "ledger"), "append", ledger, "_run", - json.dumps(event)], capture_output=True, text=True, encoding="utf-8", - errors="replace", env=ENV) - if res.returncode != 0: - notes.append(f"recording wave_merged failed: {res.stderr.strip()}") - - print(json.dumps({"status": "conflict" if conflict_unit is not None else "merged", "head": head, - "merged": merged, "skipped": skipped, "conflict_unit": conflict_unit, - "conflict_files": conflict_files, "overlaps": overlaps, "in_history": in_history, - "notes": notes})) - sys.exit(1 if conflict_unit is not None else 0) + message = f"chore: join recovery: merge {uid}" if wave is None else f"chore: join wave {wave}: merge {uid}" + retried = False + while True: + res = git(directory, *identity, "merge", "--no-ff", "--no-edit", "-m", message, "--end-of-options", head) + if res.returncode == 0: + merged.append(uid) + break + files = unmerged(directory) + if files: + aborted = git(directory, "merge", "--abort") + if aborted.returncode != 0 and git(directory, "status", "--porcelain").stdout.strip(): + fail(3, f"git merge --abort failed in {directory!r}: {aborted.stderr.strip()}") + outcome.update(conflict_unit=uid, conflict_files=files) + break + # No unmerged paths: git refused the merge (no identity, a hook, + # a held index.lock), which is a failure, not a conflict. + refusal = git_message(res) + problem = undo_refused(directory) + if problem is not None: + outcome.update(failed_unit=uid, error=f"{refusal}\nthen {problem}"[:ERROR_LIMIT]) + record() + fail(3, problem) + if not retried and "index.lock" in refusal: + retried = True + time.sleep(LOCK_RETRY_S) + continue + outcome.update(failed_unit=uid, error=refusal) + break + if outcome["conflict_unit"] is not None or outcome["failed_unit"] is not None: + break + + head, in_history, overlaps = record() + if outcome["failed_unit"] is not None: + status, code = "failed", 4 + elif outcome["conflict_unit"] is not None: + status, code = "conflict", 1 + else: + status, code = "merged", 0 + print(json.dumps({"status": status, "head": head, + "merged": merged, "skipped": skipped, "conflict_unit": outcome["conflict_unit"], + "conflict_files": outcome["conflict_files"], "overlaps": overlaps, "in_history": in_history, + "notes": notes, "failed_unit": outcome["failed_unit"], "error": outcome["error"]})) + sys.exit(code) if __name__ == "__main__": diff --git a/tests/join-waves-e2e.test.mjs b/tests/join-waves-e2e.test.mjs index 9bcc991..15ef9f7 100644 --- a/tests/join-waves-e2e.test.mjs +++ b/tests/join-waves-e2e.test.mjs @@ -173,7 +173,7 @@ test('a join wave runs on unit branches in lane worktrees and is merged', async assert.ok(/cd '[^']*\/wt\/lane-[ab]'/.test(lineAfter(p, /^Run this first/)), `${id} runs in a lane worktree`); } const subs = subjects(c, 'main..feature/w'); - assert.ok(subs.includes('join wave 1: merge J1') && subs.includes('join wave 1: merge J2'), subs.join('\n')); + assert.ok(subs.includes('chore: join wave 1: merge J1') && subs.includes('chore: join wave 1: merge J2'), subs.join('\n')); assert.ok(existsSync(join(c.repo, 'J1.txt')) && existsSync(join(c.repo, 'J2.txt'))); const events = readFileSync(`${c.m.repo.ledger_dir}/_run.jsonl`, 'utf8').trim().split('\n').map((l) => JSON.parse(l)); assert.ok(events.some((e) => e.event === 'wave_merged' && e.wave === 1 && e.units.join() === 'J1,J2')); @@ -186,8 +186,8 @@ test('a stop mid-wave relaunches through setup, recovery and the serial continua assert.equal(first.status, 'stopped'); assert.equal(first.tasks.J1.status, 'done'); assert.equal(first.tasks.J2.status, 'blocked'); - assert.ok(subjects(c, 'main..feature/w').includes('join wave 1: merge J1'), 'the finished unit was merged'); - assert.ok(!subjects(c, 'main..feature/w').includes('join wave 1: merge J2'), 'the stopped unit was not'); + assert.ok(subjects(c, 'main..feature/w').includes('chore: join wave 1: merge J1'), 'the finished unit was merged'); + assert.ok(!subjects(c, 'main..feature/w').includes('chore: join wave 1: merge J2'), 'the stopped unit was not'); const j2Head = git(c.repo, 'rev-parse', 'pl-w1-u-J2'); // The relaunch, as SKILL.md's resume builds it: setup returns both lane diff --git a/tests/ledger-waves.test.mjs b/tests/ledger-waves.test.mjs index 53bdb34..1e6b2a4 100644 --- a/tests/ledger-waves.test.mjs +++ b/tests/ledger-waves.test.mjs @@ -184,6 +184,29 @@ test('wave_merged needs exactly one of wave and recovery', () => { } }); +test('wave_merged with no units needs a conflict or a failure', () => { + const dir = freshDir(); + appendOk(dir, '_run', { task: '_run', event: 'wave_merged', wave: 1, units: [], merged: [], head: 'abc', + conflict_unit: 'A', conflict_files: ['x'] }); + appendOk(dir, '_run', { task: '_run', event: 'wave_merged', recovery: true, units: [], merged: [], head: 'abc', + failed: { unit: 'A', error: 'e' } }); + appendOk(dir, '_run', { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], merged: ['A'], head: 'abd', + conflict_files: ['x'], resolved: true }); + for (const bad of [ + { task: '_run', event: 'wave_merged', wave: 1, units: [], head: 'abc' }, + { task: '_run', event: 'wave_merged', wave: 1, units: [], merged: [], head: 'abc', conflict_unit: 'A' }, + { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], merged: ['Z'], head: 'abc' }, + { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], merged: 'A', head: 'abc' }, + { task: '_run', event: 'wave_merged', wave: 1, units: [], merged: [], head: 'abc', failed: { unit: 'A', error: '' } }, + { task: '_run', event: 'wave_merged', wave: 1, units: [], merged: [], head: 'abc', failed: { unit: '', error: 'e' } }, + { task: '_run', event: 'wave_merged', wave: 1, units: [], merged: [], head: 'abc', failed: 'A' }, + { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], head: 'abc', conflict_unit: '' }, + { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], merged: ['A'], head: 'abc', resolved: false }, + ]) { + assert.equal(ledger('append', dir, '_run', JSON.stringify(bad)).code, 2, JSON.stringify(bad)); + } +}); + test('started accepts dir, unit_branch and restart', () => { const dir = freshDir(); appendOk(dir, 'join', { task: 'DJ2', event: 'started', base: 'abc', dir: 'C:/wt/lane-a', @@ -322,6 +345,18 @@ test('wave_merges lists merge events in order', () => { ]); }); +test('ledger status copies the new wave_merged fields', () => { + const dir = freshDir(); + appendOk(dir, '_run', { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], merged: ['A'], head: 'm1', + conflict_files: ['x'], conflict_unit: 'B', failed: { unit: 'B', error: 'e' }, resolved: true }); + appendOk(dir, '_run', { task: '_run', event: 'wave_merged', wave: 1, units: ['A'], head: 'm1' }); + assert.deepEqual(statusOf(dir).wave_merges, [ + { wave: 1, units: ['A'], head: 'm1', overlaps: [], conflict_files: ['x'], merged: ['A'], conflict_unit: 'B', + failed: { unit: 'B', error: 'e' }, resolved: true }, + { wave: 1, units: ['A'], head: 'm1', overlaps: [], conflict_files: [] }, + ]); +}); + test('backfill continues past an unfinished wave unit', () => { const c = joinCase(['A', 'B', 'C']); for (const [id, branch] of [['A', 'pl-r-u-A'], ['B', 'pl-r-u-B'], ['C', 'pl-r-u-C']]) { @@ -364,6 +399,32 @@ test('join_tip is the descendant of wave and serial heads', () => { assert.deepEqual(out.backfill.D, { base: m, head: d }); }); +test('join_tip ignores events that merged nothing', () => { + const c = joinCase(['A', 'B', 'D']); + for (const [id, branch] of [['A', 'pl-r-u-A'], ['B', 'pl-r-u-B']]) { + appendOk(c.ledger, 'join', { task: id, event: 'started', base: c.w, dir: 'C:/wt/x', unit_branch: branch }); + } + const a = unitCommit(c, 'pl-r-u-A', c.w, 'A', 'a.txt'); + const b = unitCommit(c, 'pl-r-u-B', c.w, 'B', 'b.txt'); + git(c.repo, 'checkout', '-q', 'feat'); + git(c.repo, 'merge', '-q', '--no-ff', '-m', 'chore: join wave 1: merge A', a); + git(c.repo, 'merge', '-q', '--no-ff', '-m', 'chore: join wave 1: merge B', b); + const m = git(c.repo, 'rev-parse', 'HEAD'); + // An event written before 1.6.7 (no merged field) still counts. + appendOk(c.ledger, '_run', { task: '_run', event: 'wave_merged', wave: 1, units: ['A', 'B'], head: m }); + appendOk(c.ledger, 'join', { task: 'D', event: 'started', base: m }); + const d = unitCommit(c, 'feat', m, 'D', 'd.txt'); + // A commit after D that no join task made: a refused wave merge records + // the checkout's head, and must not move the tip there. + git(c.repo, 'commit', '-q', '--allow-empty', '-m', 'not a join merge'); + const later = git(c.repo, 'rev-parse', 'HEAD'); + appendOk(c.ledger, '_run', { task: '_run', event: 'wave_merged', wave: 2, units: [], merged: [], head: later, + failed: { unit: 'E', error: 'e' } }); + const out = backfillOf(c); + assert.equal(out.code, 0, JSON.stringify(out.errors)); + assert.equal(out.join_tip, d); +}); + test('join_tip refuses unordered heads', () => { const c = joinCase(['A', 'D']); appendOk(c.ledger, 'join', { task: 'A', event: 'started', base: c.w, dir: 'C:/wt/x', unit_branch: 'pl-r-u-A' }); diff --git a/tests/merge-wave.test.mjs b/tests/merge-wave.test.mjs index 723d8b1..b5c723c 100644 --- a/tests/merge-wave.test.mjs +++ b/tests/merge-wave.test.mjs @@ -2,11 +2,14 @@ // feature checkout by recorded sha, in the order given, idempotently; a // conflict is aborted and reported; overlaps are the paths two units both // changed; a wave_merged event goes to the ledger. Real git, real ledger. +// 1.6.7: --git-config gives the merges the run's identity; a merge git +// refuses (no identity, a hook, a held index.lock) is status failed, exit 4; +// every call that merged, conflicted or was refused is recorded. import { test, after } from 'node:test'; import assert from 'node:assert/strict'; -import { spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { spawn, spawnSync } from 'node:child_process'; +import { chmodSync, existsSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { SKILL_DIR } from './harness.mjs'; import { tempDir, mergeEnv } from './platform.mjs'; @@ -26,16 +29,44 @@ function git(dir, ...args) { return res.stdout.replace(/\r\n/g, '\n').trim(); } -function mergeWave(c, ...args) { - const res = spawnSync('python3', ['-B', join(SCRIPTS, 'merge-wave'), c.repo, c.ledger, ...args], - { encoding: 'utf8', env: mergeEnv(process.env, GIT_ENV) }); - let out = null; +// No identity at all (1.6.7): no global or system config, no GIT_AUTHOR_* or +// GIT_COMMITTER_* variables (nor EMAIL, which git reads as an email), so a +// case repo with user.useConfigOnly set cannot commit without -c pairs. +const EMPTY_CONFIG = join(TMP, 'empty.gitconfig'); +writeFileSync(EMPTY_CONFIG, ''); +const NO_ID = { GIT_CONFIG_GLOBAL: EMPTY_CONFIG, GIT_CONFIG_NOSYSTEM: '1' }; +function noIdEnv() { + const env = {}; + for (const [k, v] of Object.entries(process.env)) { + if (!/^GIT_(AUTHOR|COMMITTER)_/i.test(k) && k.toUpperCase() !== 'EMAIL') env[k] = v; + } + return { ...env, ...NO_ID }; +} + +const parsed = (stdout) => { try { - out = JSON.parse(res.stdout); + return JSON.parse(stdout); } catch { - out = null; + return null; } - return { code: res.status, out, stderr: res.stderr }; +}; + +// mergeWave(c, ...args[, { env }]): runs merge-wave on the case; env, when +// given, is the whole environment (default: process.env with GIT_ENV). +function mergeWave(c, ...args) { + const opts = args.length && typeof args[args.length - 1] === 'object' ? args.pop() : {}; + const res = spawnSync('python3', ['-B', join(SCRIPTS, 'merge-wave'), c.repo, c.ledger, ...args], + { encoding: 'utf8', env: opts.env || mergeEnv(process.env, GIT_ENV) }); + return { code: res.status, out: parsed(res.stdout), stderr: res.stderr }; +} + +function mergeWaveAsync(c, args, env = mergeEnv(process.env, GIT_ENV)) { + const p = spawn('python3', ['-B', join(SCRIPTS, 'merge-wave'), c.repo, c.ledger, ...args], { env }); + let stdout = ''; + let stderr = ''; + p.stdout.on('data', (d) => { stdout += d; }); + p.stderr.on('data', (d) => { stderr += d; }); + return { child: p, done: new Promise((resolve) => p.on('close', (code) => resolve({ code, out: parsed(stdout), stderr }))) }; } const events = (c) => (existsSync(join(c.ledger, '_run.jsonl')) @@ -78,11 +109,11 @@ test('merges in the given order', () => { assert.equal(r.out.status, 'merged'); assert.deepEqual(r.out.merged, ['B', 'A']); assert.deepEqual(r.out.skipped, []); - assert.deepEqual(firstParents(c), ['join wave 2: merge A', 'join wave 2: merge B']); + assert.deepEqual(firstParents(c), ['chore: join wave 2: merge A', 'chore: join wave 2: merge B']); assert.equal(r.out.head, git(c.repo, 'rev-parse', 'HEAD')); const [ev] = events(c); - assert.deepEqual(ev, { task: '_run', event: 'wave_merged', wave: 2, units: ['B', 'A'], head: r.out.head, - overlaps: [], conflict_files: [] }); + assert.deepEqual(ev, { task: '_run', event: 'wave_merged', wave: 2, units: ['B', 'A'], merged: ['B', 'A'], + head: r.out.head, overlaps: [], conflict_files: [] }); }); test('a rerun skips merged units and changes nothing', () => { @@ -153,7 +184,7 @@ test('a batch unit is merged once', () => { const r = mergeWave(c, '--wave', '2', ...batch); assert.equal(r.code, 0, r.stderr); assert.deepEqual(r.out.merged, ['J2-J3']); - assert.deepEqual(firstParents(c), ['join wave 2: merge J2-J3']); + assert.deepEqual(firstParents(c), ['chore: join wave 2: merge J2-J3']); }); test('a dirty checkout is refused', () => { @@ -171,7 +202,7 @@ test('--recovery records recovery in the event', () => { const a = unit(c, 'A', { 'a.txt': 'a\n' }); const r = mergeWave(c, '--recovery', ...a); assert.equal(r.code, 0, r.stderr); - assert.deepEqual(firstParents(c), ['join recovery: merge A']); + assert.deepEqual(firstParents(c), ['chore: join recovery: merge A']); const [ev] = events(c); assert.equal(ev.recovery, true); assert.equal('wave' in ev, false); @@ -182,7 +213,7 @@ test('--recovery with no unit reports HEAD and records nothing', () => { const r = mergeWave(c, '--recovery'); assert.equal(r.code, 0, r.stderr); assert.deepEqual(r.out, { status: 'merged', head: c.w, merged: [], skipped: [], conflict_unit: null, - conflict_files: [], overlaps: [], in_history: [], notes: [] }); + conflict_files: [], overlaps: [], in_history: [], notes: [], failed_unit: null, error: null }); assert.deepEqual(events(c), []); }); @@ -217,3 +248,141 @@ test('--known units are reported in_history without being merged', () => { assert.deepEqual(both.out.in_history, ['B', 'A']); assert.equal(mergeWave(c, '--recovery', '--known', 'A').code, 2, 'a --known needs ID and HEAD'); }); + +// ---- 1.6.7: the run's identity, refused merges, every merge recorded ---- + +// A case whose repo commits only with an identity from config (the setup +// commits above used GIT_ENV's variables), with units A and B. +function noIdCase() { + const c = newCase(); + const a = unit(c, 'A', { 'a.txt': 'a\n' }); + const b = unit(c, 'B', { 'b.txt': 'b\n' }); + git(c.repo, 'config', 'user.useConfigOnly', 'true'); + return { c, a, b }; +} +const IDENTITY = ['--git-config', 'user.name=Riley Q', '--git-config', 'user.email=r@x.invalid']; +const mergeInProgress = (c) => spawnSync('git', ['-C', c.repo, 'rev-parse', '-q', '--verify', 'MERGE_HEAD'], + { env: mergeEnv(process.env, GIT_ENV) }).status === 0; + +test('a refused merge is reported as failed, not as a conflict', () => { + const { c, a, b } = noIdCase(); + const r = mergeWave(c, '--wave', '1', ...a, ...b, { env: noIdEnv() }); + assert.equal(r.code, 4, r.stderr); + assert.equal(r.out.status, 'failed'); + assert.equal(r.out.failed_unit, 'A'); + assert.match(r.out.error, /identity unknown/i); + assert.ok(r.out.error.length <= 2000); + assert.deepEqual(r.out.merged, []); + assert.equal(r.out.conflict_unit, null); + assert.equal(git(c.repo, 'status', '--porcelain'), ''); + assert.equal(mergeInProgress(c), false); + assert.equal(git(c.repo, 'rev-parse', 'HEAD'), c.w); + const evs = events(c); + assert.equal(evs.length, 1); + assert.deepEqual(evs[0].units, []); + assert.deepEqual(evs[0].merged, []); + assert.deepEqual(evs[0].failed, { unit: 'A', error: r.out.error }); + assert.equal('conflict_unit' in evs[0], false); + assert.equal(evs[0].head, c.w); +}); + +test("--git-config commits with the run's identity", () => { + const { c, a, b } = noIdCase(); + const r = mergeWave(c, '--wave', '1', ...a, ...b, ...IDENTITY, { env: noIdEnv() }); + assert.equal(r.code, 0, r.stderr); + assert.equal(r.out.status, 'merged'); + assert.deepEqual(r.out.merged, ['A', 'B']); + assert.equal(r.out.failed_unit, null); + assert.equal(r.out.error, null); + assert.equal(git(c.repo, 'log', '-2', '--first-parent', '--format=%an <%ae>|%cn <%ce>'), + 'Riley Q |Riley Q \nRiley Q |Riley Q '); + assert.deepEqual(firstParents(c), ['chore: join wave 1: merge B', 'chore: join wave 1: merge A']); + const [ev] = events(c); + assert.deepEqual(ev.merged, ['A', 'B']); + assert.deepEqual(ev.units, ['A', 'B']); +}); + +test('a refusal after a merge keeps the earlier unit', () => { + const { c, a, b } = noIdCase(); + const hook = join(c.repo, '.git', 'hooks', 'pre-merge-commit'); + mkdirSync(join(c.repo, '.git', 'hooks'), { recursive: true }); + writeFileSync(hook, '#!/bin/sh\ngit diff --cached --name-only | grep -qx b.txt && exit 1\nexit 0\n'); + chmodSync(hook, 0o755); + const r = mergeWave(c, '--wave', '1', ...IDENTITY, ...a, ...b, { env: noIdEnv() }); + assert.equal(r.code, 4, r.stderr); + assert.equal(r.out.status, 'failed'); + assert.deepEqual(r.out.merged, ['A']); + assert.equal(r.out.failed_unit, 'B'); + assert.ok(r.out.error); + assert.deepEqual(r.out.in_history, ['A']); + assert.equal(mergeInProgress(c), false); + assert.equal(git(c.repo, 'status', '--porcelain'), ''); + assert.equal(r.out.head, git(c.repo, 'rev-parse', 'HEAD')); + assert.deepEqual(firstParents(c), ['chore: join wave 1: merge A']); + const [ev] = events(c); + assert.deepEqual(ev.units, ['A']); + assert.deepEqual(ev.merged, ['A']); + assert.deepEqual(ev.failed, { unit: 'B', error: r.out.error }); + assert.equal(ev.head, r.out.head); +}); + +test('a held index.lock is retried once', async () => { + const c = newCase(); + const a = unit(c, 'A', { 'a.txt': 'a\n' }); + const lock = join(c.repo, '.git', 'index.lock'); + writeFileSync(lock, ''); + const run = mergeWaveAsync(c, ['--wave', '1', ...a]); + setTimeout(() => rmSync(lock, { force: true }), 500); + const r = await run.done; + assert.equal(r.code, 0, r.stderr); + assert.deepEqual(r.out.merged, ['A']); + + const held = newCase(); + const h = unit(held, 'A', { 'a.txt': 'a\n' }); + writeFileSync(join(held.repo, '.git', 'index.lock'), ''); + const r2 = await mergeWaveAsync(held, ['--wave', '1', ...h]).done; + rmSync(join(held.repo, '.git', 'index.lock'), { force: true }); + assert.equal(r2.code, 4, r2.stderr); + assert.equal(r2.out.status, 'failed'); + assert.equal(r2.out.failed_unit, 'A'); + assert.ok(r2.out.error.includes('index.lock'), r2.out.error); + assert.equal(git(held.repo, 'rev-parse', 'HEAD'), held.w); +}); + +test('a conflict on the first unit is recorded', () => { + const c = newCase(); + writeFileSync(join(c.repo, 'shared.txt'), 'one\nTWO-FEAT\nthree\nfour\nfive\n'); + git(c.repo, 'commit', '-q', '-am', 'feat change'); + const featHead = git(c.repo, 'rev-parse', 'HEAD'); + const a = unit(c, 'A', { 'shared.txt': 'one\nTWO-A\nthree\nfour\nfive\n' }); + const r = mergeWave(c, '--wave', '1', ...a); + assert.equal(r.code, 1, r.stderr); + assert.equal(r.out.status, 'conflict'); + assert.equal(r.out.conflict_unit, 'A'); + assert.equal(r.out.failed_unit, null); + assert.equal(git(c.repo, 'status', '--porcelain'), ''); + const evs = events(c); + assert.equal(evs.length, 1); + assert.deepEqual(evs[0], { task: '_run', event: 'wave_merged', wave: 1, units: [], merged: [], head: featHead, + overlaps: [], conflict_files: ['shared.txt'], conflict_unit: 'A' }); +}); + +test('--git-config refuses other keys', () => { + const c = newCase(); + const a = unit(c, 'A', { 'a.txt': 'a\n' }); + for (const pair of ['user.signingkey=x', 'user.name=', 'core.editor=x', 'user.name']) { + const r = mergeWave(c, '--wave', '1', ...a, '--git-config', pair); + assert.equal(r.code, 2, pair); + assert.match(r.stderr, /--git-config takes user\.name=VALUE or user\.email=VALUE/, pair); + } + assert.equal(mergeWave(c, '--wave', '1', ...a, '--git-config').code, 2, 'a --git-config needs a value'); + assert.equal(git(c.repo, 'rev-parse', 'HEAD'), c.w); +}); + +test('--git-config keys are matched in any case and passed lower case', () => { + const { c, a } = noIdCase(); + const r = mergeWave(c, '--git-config', 'User.Name=O Neil', '--wave', '1', '--git-config', 'USER.EMAIL=o@x.invalid', ...a, + { env: noIdEnv() }); + assert.equal(r.code, 0, r.stderr); + assert.equal(git(c.repo, 'log', '-1', '--format=%an <%ae>'), 'O Neil '); +}); From 0d5dcfe0ba00cd79160fd20a18102ce01d98b73a Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:42:46 -0700 Subject: [PATCH 04/11] docs: the run's git identity, refused merges and merge messages --- SKILL.md | 3 +- reference.md | 101 +++++++++++++++++++++++++++++++++++++------ tests/skill.test.mjs | 20 +++++++++ 3 files changed, 109 insertions(+), 15 deletions(-) diff --git a/SKILL.md b/SKILL.md index fe12e30..de22b08 100644 --- a/SKILL.md +++ b/SKILL.md @@ -257,7 +257,8 @@ Relaunch once with the relaunch notice: carry budgets over (reference.md "Transi `repairs_spent` to the repairs in the stopped run's `final.repairs` plus the earlier value. First recompute `done`, `reviewed`, `deferred`, the `carry` notes, `carried_minors`, `inputs_changed`, the join wave fields and `backfill` as in Resume -steps 2-3, then rerun `scripts/setup --owner ` and take `start_points`; if validation +steps 2-3, then rerun `scripts/setup --owner `, put its output in `setup_result` and take +`start_points`; if validation rejects its `setup_skipped`, rerun setup. Launch again. No second table. A second transient stop is a real stop, unless it is a usage-limit stop (reference.md "Usage limit"). diff --git a/reference.md b/reference.md index 7bbe0a0..74b5c49 100644 --- a/reference.md +++ b/reference.md @@ -93,7 +93,7 @@ PATH, else `C:/Program Files/Git`), never the WSL `bash.exe` in System32. | `limits` | `review_rounds: 5`, `max_parallel_lanes: min(5, CPU count + 2)` (the count from `os.cpu_count()`), `max_agents` (2 x the dry-run `agents` length), `max_rulings` (25); see Budgets. | | `autonomy` | `autonomous` (default: the adjudicator settles blocked tasks, questions, review caps, pre-flight conflicts) or `supervised` (they stop the run and wait for the user). The user may override it in the table. | | `profile` | `full` (default) or `lite` (see Profiles). | -| `setup_result` | The output of `scripts/setup --owner `: `{feature_head, worktrees, discarded, preserved, setup_skipped}` (`setup_skipped`: the lanes whose setup commands were skipped because every task is done and reviewed; validation rejects a listed lane with pending work, and then setup reruns). Added after the yes to the table; required for a launch (there is no setup agent; a dry run does without it). `preserved` lists the commits under `refs/parallel-lanes//abandoned/` that hold changes setup discarded (see Cleanup). | +| `setup_result` | The output of `scripts/setup --owner `: `{feature_head, worktrees, discarded, preserved, setup_skipped, returned, git_identity}` (`git_identity`, from 1.6.7: the commit rules' name and email pairs, see "Git identity"; optional, absent means `[]`; `setup_skipped`: the lanes whose setup commands were skipped because every task is done and reviewed; validation rejects a listed lane with pending work, and then setup reruns). Added after the yes to the table; required for a launch (there is no setup agent; a dry run does without it). `preserved` lists the commits under `refs/parallel-lanes//abandoned/` that hold changes setup discarded (see Cleanup). | | `carried_minors` | Optional (1.6.3), set on a resume: `{open, covered}` from `ledger status`'s `minors`, copied as printed: the ids of the task minors earlier launches left open, and the tasks the snapshot covers ("Carry-over"). | | `inputs_changed` | Optional (1.6.3), set on a resume: `{plan, spec}` from `ledger status --plan --spec`'s `inputs` (`plan` is `plan_changed`, `spec` is `spec_changed`; each `true`, `false` or `null`), read before setup, so a relaunch's pre-flight knows what changed since the last launch. | | `start_points` | The feature heads `ledger status` prints after setup, copied exactly as it prints them: `prelude` from setup, `join` only once a launch got past integration (a new run has no `join` key). | @@ -143,7 +143,24 @@ resume lines filled in (`notices`); the script logs the same line; you still pri a commit without an identity fails partway through the run. If it fails, ask the user to set one (`git config --global user.name` and `user.email`) or to name one that `commit_rules` passes with `-c` (`git -c user.name=... -c user.email=... commit`). `scripts/setup` does not -check this, since a `-c` identity is legitimate. +check this, since a `-c` identity is legitimate. Doctor's `git_identity` check runs both `git var +GIT_COMMITTER_IDENT` and `git var GIT_AUTHOR_IDENT` with the commit rules' pairs, and both must +pass (from 1.6.7). + +From 1.6.7 `scripts/setup` records the commit rules' `-c user.name=...` and `-c user.email=...` +pairs as `setup_result.git_identity`: a list of at most two strings, `user.name=` first, +then `user.email=`, keys lower case, the last pair given for a key winning (as with git's +own `-c`), `[]` when the commit rules name none. It is recorded whether or not a global identity +exists, so the run's merges commit as the agents' own commits do. The run passes the pairs to +every merge and every commit it spells out itself: the integrate prompt's lane merges, the +resolved-merge commits (`git -C -c ... commit --no-edit`) of the integrate prompt, the +lane resolver and the wave resolver, the wave resolver's merge, the supervised conflict stop's +by-hand commands, and the wave merge (merge-wave's `--git-config`, one per pair). Each pair is +shell-quoted in a prompt and one argv item in merge-wave. With none, these use the user's git +config, as before 1.6.7. Agents' own commits keep following `commit_rules`. Only name and email +are carried: other `-c` options in the commit rules (a signing key, an editor) do not reach the +scripted merges. A manifest whose setup ran under 1.6.6 has no `git_identity` (read as `[]`); +setup reruns on a resume or relaunch and adds it. A repository with SHA-256 object names (`git rev-parse --show-object-format` prints `sha256`) is not supported: the run names commits by 40-hex SHA-1 ids. `scripts/setup` refuses it (exit 3, @@ -184,8 +201,13 @@ Code. Change nothing yourself. SKILL.md gives the rule; this is how it reads the causes. Not transient, so never relaunched (stop and notify): `review_rounds`, a supervised blocked or question task, `adjudication_cap`, `adjudicator_stop`, `budget`, a failed integration or post-integrate on real failures, -`integration checks did not finish: ...` (see "Exit codes"), and status `invalid` or -`preflight_conflicts`. The causes are every `stopped_lanes[].reason` when `reason` is `prelude +`integration checks did not finish: ...` (see "Exit codes"), status `invalid` or +`preflight_conflicts`, and a refused merge in the join (from 1.6.7: `join : the merge of + failed: `, or `join : the merge failed: ` when the result names no +unit or error; `` is `wave ` or `recovery`) or a merge that reported done but left a +unit out (`join : the merge left out `). A refused merge needs the user (an identity, +a hook, a held lock), then Resume. A missing wave or recovery merge result reads exactly `no +result from merge`, so it is transient like any other missing result. The causes are every `stopped_lanes[].reason` when `reason` is `prelude stopped`, `lanes stopped`, `join stopped`, or `agents_unavailable`, else `reason` itself. A cause is transient only if it starts with `no result from`, `error:`, or `setup failed`, or reads `integration failed: no result from ...` or `post-integrate failed: no result from ...`, @@ -320,12 +342,61 @@ worktrees, when no wave has two units, and when the join started in an earlier l NAME: a retry), checks NAME out and runs the project's setup there. Its ledger lane, checks and phase stay `join`. Once a unit of a wave stops, no further unit of it starts. - Integration keeps the lane worktrees. After each multi-unit wave a wave merge agent (Sonnet) - runs `scripts/merge-wave DIR LEDGER_DIR --wave N --unit ID BASE HEAD ...`: it merges each - finished unit's recorded head with `join wave N: merge ID` (skipping one already merged), - aborts and reports a conflict, lists `overlaps` (paths two units changed), and records - `wave_merged`. A conflict goes to a wave resolver (Opus, one per conflict; on failure it aborts - the merge) in an autonomous run; a supervised run stops with the sha to merge by hand. A unit - that stopped is not merged, and the join stops after the merge. + runs `scripts/merge-wave DIR LEDGER_DIR --git-config PAIR ... --wave N --unit ID BASE HEAD + ...`: it merges each finished unit's recorded head with `chore: join wave : merge ` + (skipping one already merged), aborts and reports a conflict, lists `overlaps` (paths two units + changed), and records `wave_merged`. A conflict goes to a wave resolver (Opus, one per + conflict; on failure it aborts the merge) in an autonomous run; a supervised run stops with the + sha to merge by hand. A unit that stopped is not merged, and the join stops after the merge. +- merge-wave (from 1.6.7). `--git-config KEY=VALUE`, repeatable, in any position among the + options, is one pair of `setup_result.git_identity` (see "Git identity"): KEY `user.name` or + `user.email` in any case (passed lower case), VALUE non-empty; each goes to every `git merge` + as one `-c KEY=VALUE` argv item. Any other key or an empty value is a usage error (exit 2: + `--git-config takes user.name=VALUE or user.email=VALUE`). Its JSON `status` is `merged`, + `conflict` (a merge with unmerged paths, aborted; `conflict_unit`, `conflict_files`) or + `failed`: git refused a merge with no unmerged paths (no identity, a failing hook, a held + `index.lock`). On a refusal merge-wave runs `git merge --abort` only when a merge is in + progress (`MERGE_HEAD` exists), checks the checkout is clean, and stops the loop; units merged + earlier in the same call stay merged (`merged`). When git's message names `index.lock` and the + checkout is clean, it waits 2 s and tries that unit's merge once more (one retry only); a + second refusal is the failure. Every output carries `failed_unit` and `error` (null when + none): on a refusal the unit and git's message (stderr, else stdout, stripped, at most 2,000 + characters, or `git merge exited ` when both are empty). Exits: 0 every unit merged (or + nothing to merge), 1 a conflict, 2 a usage error, 3 the checkout is not clean, a BASE or HEAD + is not a commit, or a git step failed (a refusal whose abort fails or leaves the checkout not + clean is recorded with `failed` first), 4 git refused a merge. The wave merge agent maps exit 4 + to status `failed` with `failed_unit`, `error`, `merged`, `in_history` and `head`, runs no + checks, and does not change git config, set an identity or retry the merge. +- The engine stops on a refused merge in both autonomies, with no resolver: `join : the + merge of failed: ` (`` is `wave ` or `recovery`; `join : the merge + failed: ` when the result lacks the unit or error), with the report entry's `head` and + `in_history` filled from the result. A `conflict` with no `conflict_files` counts as refused. A + `done` result that has some unit it was asked for in neither `merged` nor `in_history` stops + with `join : the merge left out ` (ids joined by `, `). A missing result stops with + `no result from merge`, which is transient ("Transient stops"); the other two are not. +- Merge messages. From 1.6.7 the join's merges are `chore: join wave : merge ` and + `chore: join recovery: merge ` (merge-wave, the wave resolver, the supervised stop's + by-hand command); before 1.6.7 they were `join wave : merge ` and `join recovery: + merge `, and the integrate prompt's history check accepts both forms, since a relaunch + after an upgrade can meet merges an earlier launch made. Every reviewer and final lens is told + these and the wave checks fix `fix: make the wave checks pass` are the engine's own commits, + not commit-rule findings. +- `wave_merged` (from 1.6.7). merge-wave appends it on every call that merged a unit, ended in a + conflict or was refused; a call that only skipped units already in the history and ended clean + records nothing. Fields: `wave` (or `recovery: true`), `units` (the units in the history after + the call, merged or skipped; empty only with non-empty `conflict_files` or a `failed`), + `merged` (the units this event's merges added, possibly empty), `head`, `overlaps`, + `conflict_files`, and when they apply `conflict_unit`, `failed` (`{unit, error}`, both + non-empty) and `resolved: true`; absent fields are left out, not null. The wave resolver records + its own merge: after committing it appends `wave_merged` with the wave (or `recovery`), `units` + and `merged` both `[]`, `head` (its merge commit), `conflict_files` and `resolved: true`, + through the ledger command its prompt gives. `join_tip` follows only events that merged: an + event's head counts when its `merged` is non-empty, or when it has no `merged` field (an event + from before 1.6.7), so a conflict or refusal event never moves the tip. `ledger status` copies + `merged`, `conflict_unit`, `failed` and `resolved` into `wave_merges` when recorded. The final + lenses read the conflict files of a `resolved` event (or one from before 1.6.7) as settled by + the wave resolver, and those of a conflict no later `resolved` event of the same wave settled + as settled outside the run's resolver (by hand or in a later launch). - After the last multi-unit wave, its units' targeted checks (`task_checks`) run once on the merged tip, with `--phase join`, and `wave_checked` is recorded. A failure gets one Opus fix (`fix: make the wave checks pass`), its re-review and a recheck; still failing, an autonomous @@ -333,7 +404,7 @@ worktrees, when no wave has two units, and when the join started in an earlier l supervised one stops. Checks that did not finish get no fix. - A relaunch with `join_started` and `unit_branches`: setup puts each lane worktree left on one of the run's unit branches back on its lane branch (output `returned`), saving its changes; a - recovery merge (`merge-wave --recovery`, `join recovery: merge ID`) brings in every unit that + recovery merge (`merge-wave --recovery`, `chore: join recovery: merge `) brings in every unit that ran on its own branch and is done and reviewed or deferred, and reports (`--known`, `in_history`) which of the other such units an earlier launch already merged; the rest of the join runs serially on the feature checkout from that tip. A merged unit whose approval went @@ -364,8 +435,10 @@ worktrees, when no wave has two units, and when the join started in an earlier l run's unit branches and removes merged unit branches; an unmerged one stays. autopsy reads the wave labels (`wave N merge`, `wave N resolve ID`, `wave checks fix`, `wave checks re-review`, `wave checks`, `recovery merge`, `recovery resolve ID`) as Join phase agents. A supervised - conflict stop gives the exact `git merge --no-ff -m 'join wave N: merge ID' ` to run by - hand, so integration's history audit accepts it on the relaunch. + conflict stop gives the exact `git -C merge --no-ff -m 'chore: join wave N: + merge ID' ` and `git -C commit --no-edit` to run by hand (`` the + `-c` options of `git_identity`, empty with none), so integration's history audit accepts it on + the relaunch. - `worth-it --join-deps ID:DEPS` (one per join task) counts the join as the sum over its waves of ceil(size / lanes at once), declared files not checked; without it, one slot per join task. `tools/worth-it-backtest.py --waves` counts the archived joins in waves. @@ -1757,7 +1830,7 @@ Hard failures: | Check | Fails when | |---|---| -| `git_identity` | `git var GIT_COMMITTER_IDENT` fails, after any `-c user.name=... -c user.email=...` pair `commit_rules` passes | +| `git_identity` | `git var GIT_COMMITTER_IDENT` or (from 1.6.7) `git var GIT_AUTHOR_IDENT` fails, after any `-c user.name=... -c user.email=...` pair `commit_rules` passes | | `base_not_current` | the local `base_ref` does not contain its upstream tip (details below) | | `python` | the manifest's `python` does not run Python 3.8 or newer | | `dirty_checkout` | git mode: the checkout has uncommitted changes or untracked, non-ignored files; its fix says to commit any file a check needs and gitignore only known generated output, and never to ignore source a check needs | diff --git a/tests/skill.test.mjs b/tests/skill.test.mjs index 1dc2cbd..616c959 100644 --- a/tests/skill.test.mjs +++ b/tests/skill.test.mjs @@ -299,6 +299,26 @@ test('SKILL.md records the spend and releases the lock in one step, and prints t assert.ok(launch.includes('notices'), launch); }); +// 1.6.7: the run carries the commit rules' identity to its merges, a refused +// wave merge stops the run for good, and a missing merge result relaunches. +test("reference.md documents the run's git identity and refused merges", () => { + const raw = read(REFERENCE_MD).toString('utf8'); + const text = raw.replace(/\s+/g, ' '); + for (const word of ['git_identity', '--git-config', 'exit 4', 'chore: join wave : merge ', 'index.lock']) { + assert.ok(text.includes(word), `reference.md lacks ${word}`); + } + const start = raw.indexOf('\n### Transient stops\n'); + assert.ok(start >= 0, 'reference.md lacks a Transient stops heading'); + const end = raw.indexOf('\n#', start + 1); + const transient = raw.slice(start, end === -1 ? undefined : end).replace(/\s+/g, ' '); + for (const word of ['a refused merge', 'no result from merge']) { + assert.ok(transient.includes(word), `Transient stops lacks ${word}`); + } + const skill = read(SKILL_MD).toString('utf8').replace(/\s+/g, ' '); + const relaunch = skill.slice(skill.indexOf('Relaunch once with the relaunch notice'), skill.indexOf('Launch again.')); + assert.ok(relaunch.includes('setup_result'), relaunch); +}); + test('reference.md documents the Workflow concurrency cap', () => { const text = read(REFERENCE_MD).toString('utf8'); assert.ok(text.includes('CLAUDE_CODE_WORKFLOW_MAX_CONCURRENT_AGENTS'), 'reference.md lacks the concurrency cap'); From efd35f86068c6d0b93ce618592fcd3fa7bb782ad Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:43:32 -0700 Subject: [PATCH 05/11] feat: the engine's merges and commits carry the run's identity --- manifest.schema.json | 9 +++ run.workflow.js | 59 ++++++++++++--- src/phases.js | 5 +- src/prompts-phase.js | 39 +++++++--- src/validate.js | 15 ++++ tests/git-identity.test.mjs | 130 ++++++++++++++++++++++++++++++++++ tests/join-waves-run.test.mjs | 9 ++- tests/validate.test.mjs | 25 +++++++ 8 files changed, 267 insertions(+), 24 deletions(-) create mode 100644 tests/git-identity.test.mjs diff --git a/manifest.schema.json b/manifest.schema.json index 5ccf7ec..d522731 100644 --- a/manifest.schema.json +++ b/manifest.schema.json @@ -274,6 +274,15 @@ "discarded": { "$ref": "#/$defs/textList" }, + "git_identity": { + "description": "The run's git identity, from scripts/setup: the -c user.name=VALUE and user.email=VALUE pairs the commit rules name, user.name first, each key once; [] when the commit rules name none. The engine's merges and the commits it spells out pass them as -c options (merge-wave as --git-config).", + "type": "array", + "maxItems": 2, + "items": { + "type": "string", + "pattern": "^user\\.(name|email)=.+$" + } + }, "setup_skipped": { "description": "Lane ids whose setup commands setup skipped because every task of the lane is in done and reviewed. A lane with no setup commands is never listed. validateManifest rejects a listed lane that has pending work.", "type": "array", diff --git a/run.workflow.js b/run.workflow.js index 26559ec..d39f4a6 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -715,6 +715,21 @@ function validateManifest(m) { } } } + // git_identity (1.6.7): the -c user.name=/user.email= pairs the commit + // rules name, as setup resolved them; the engine's merges and the + // commits it spells out carry them. Absent before 1.6.7. + if ('git_identity' in r) { + const pairs = r.git_identity; + const keys = Array.isArray(pairs) + ? pairs.map((p) => (typeof p === 'string' ? /^(user\.(?:name|email))=.+$/.exec(p) : null)) : null; + const ok = keys !== null && keys.length <= 2 && keys.every((k) => k !== null) + && new Set(keys.map((k) => k[1])).size === keys.length + && (keys.length < 2 || keys[0][1] === 'user.name'); + if (!ok) { + err('setup_result.git_identity: must be a list of at most two strings, ' + + 'user.name=VALUE then user.email=VALUE, each key once'); + } + } if (!isObject(r.worktrees)) { err('setup_result.worktrees: must be an object mapping lane ids to absolute paths'); } else { @@ -2280,6 +2295,20 @@ function gitAdmin(m) { : `git -C ${shellQuote(m.repo.root)}`; } +// The run's git identity (1.6.7): the user.name=/user.email= pairs setup +// read from the commit rules, [] when it recorded none (a dry run, or a +// manifest from before 1.6.7). +function gitIdentity(m) { + const r = m.setup_result; + return r && Array.isArray(r.git_identity) ? r.git_identity : []; +} + +// The identity as git options, each shell-quoted and followed by a space so +// it goes between `git -C ` and the subcommand; '' for none. +function gitIdentityArgs(m) { + return gitIdentity(m).map((p) => `-c ${shellQuote(p)} `).join(''); +} + // Rules every phase agent gets, the general long-command rule included. // checkout (optional) replaces the checkout rules for the feature checkout // (setup works in several checkouts). @@ -2615,8 +2644,9 @@ function integratePrompt(m, preludeTip, opts = {}) { const q = shellQuote; const dir = q(featureDir(m)); const admin = gitAdmin(m); + const id = gitIdentityArgs(m); const merges = m.lanes.map((lane) => - ` git -C ${dir} merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); + ` git -C ${dir} ${id}merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); // Step 3 runs the delivery inventory once (spec 4.4): the project's setup // and the lanes' distinct ones, then every distinct check of the project // and the lane_commands overrides, with the long-check procedure (1.6.1) @@ -2677,8 +2707,9 @@ function integratePrompt(m, preludeTip, opts = {}) { ' file in conflict_files (a later agent resolves them).', ] : [ ' A branch that is already merged reports already up to date; that is fine. On a conflict, resolve it', - ' keeping the intent of both lanes (read the plan tasks that touched the file) and commit the merge; if', - ' you cannot resolve it with confidence, run git merge --abort and fail naming the files.', + ' keeping the intent of both lanes (read the plan tasks that touched the file) and commit the merge with', + ` \`git -C ${dir} ${id}commit --no-edit\`; if you cannot resolve it with confidence, run git merge --abort`, + ' and fail naming the files.', ]; const testFailureLine = testFailure === 'heal' ? ' Fix only small, obvious integration breakage (commit it per the commit rules). If a project command ' @@ -2767,8 +2798,9 @@ function integratePrompt(m, preludeTip, opts = {}) { function resolveConflictsPrompt(m, preludeTip, conflictFiles) { const q = shellQuote; const dir = q(featureDir(m)); + const id = gitIdentityArgs(m); const merges = m.lanes.map((lane) => - ` git -C ${dir} merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); + ` git -C ${dir} ${id}merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); const files = conflictFiles && conflictFiles.length > 0 ? conflictFiles.join(', ') : '(the files the merge reports)'; return [ `You are the conflict resolver for parallel-lanes run ${m.run_id}.`, @@ -2781,7 +2813,8 @@ function resolveConflictsPrompt(m, preludeTip, conflictFiles) { ...merges, ' A branch already merged reports already up to date; that is fine. On a conflict, resolve it keeping', ' the intent of both lanes: read the plan tasks that touched the conflicting files and keep what each', - ' lane meant to do, then commit the merge. Resolve the conflicts only; make no other change.', + ` lane meant to do, then commit the merge with \`git -C ${dir} ${id}commit --no-edit\`. Resolve the`, + ' conflicts only; make no other change.', ' Do not run the project commands, rewrite history, or remove any worktree or branch.', '', `Plan: ${m.plan}`, @@ -2807,7 +2840,8 @@ function waveMergePrompt(m, wave, units, checks = [], known = []) { const dir = featureDir(m); const recovery = wave === null; const merge = [`cd ${q(dir)} && ${pythonCommand(m)} ${q(`${m.skill_dir}/scripts/merge-wave`)} ${q(dir)}`, - q(m.repo.ledger_dir), ...(recovery ? ['--recovery'] : ['--wave', q(String(wave))]), + q(m.repo.ledger_dir), ...gitIdentity(m).flatMap((p) => ['--git-config', q(p)]), + ...(recovery ? ['--recovery'] : ['--wave', q(String(wave))]), ...units.flatMap((u) => ['--unit', q(u.id), q(u.base), q(u.head)]), ...known.flatMap((u) => ['--known', q(u.id), q(u.head)])].join(' '); const run = checks.length > 0 ? runChecksCommand(m, dir, checks, resultPath(), 'join') : null; @@ -2889,6 +2923,7 @@ function waveMergeSchema() { function waveResolvePrompt(m, wave, unit, conflictFiles) { const q = shellQuote; const dir = q(featureDir(m)); + const id = gitIdentityArgs(m); const files = conflictFiles && conflictFiles.length > 0 ? conflictFiles.join(', ') : '(the files the merge reports)'; const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; return [ @@ -2898,10 +2933,11 @@ function waveResolvePrompt(m, wave, unit, conflictFiles) { '', `1. git -C ${dir} status --porcelain --untracked-files=all must print nothing; otherwise return status`, ' failed (never make it so by deleting, cleaning, or stashing files).', - `2. git -C ${dir} merge --no-ff -m ${q(message)} ${q(unit.head)}`, + `2. git -C ${dir} ${id}merge --no-ff -m ${q(message)} ${q(unit.head)}`, " Resolve each conflict keeping the intent of both sides: read the plan tasks of the units that touched the", - ' file and keep what each meant to do, then commit the merge. Resolve the conflicts only; make no other', - ' change, run no project command, rewrite no history, and remove no worktree or branch.', + ` file and keep what each meant to do, then commit the merge with \`git -C ${dir} ${id}commit --no-edit\`.`, + ' Resolve the conflicts only; make no other change, run no project command, rewrite no history, and remove', + ' no worktree or branch.', `3. If you cannot resolve it, run git -C ${dir} merge --abort so the checkout is clean, and return status`, ' failed saying why.', '', @@ -5910,9 +5946,10 @@ async function runAll(manifest, io) { for (const f of files) if (!entry.conflict_files.includes(f)) entry.conflict_files.push(f); if (!autonomous) { const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; + const git = `git -C ${shellQuote(featureDir(m))} ${gitIdentityArgs(m)}`; return { stopped: `join ${name}: ${unit.id} conflicts in ${files.join(', ') || 'files git reported'}; ` - + `merge it by hand with git -C ${shellQuote(featureDir(m))} merge --no-ff -m ${shellQuote(message)} ` - + `${unit.head}, resolve and commit, then relaunch` }; + + `merge it by hand with ${git}merge --no-ff -m ${shellQuote(message)} ` + + `${unit.head}, resolve and commit (${git}commit --no-edit), then relaunch` }; } const res = await callM(`${name} resolve ${unit.id}`, 'Join', waveResolvePrompt(m, wave, unit, files), statusSchema(), standard); diff --git a/src/phases.js b/src/phases.js index 189ecc4..229699c 100644 --- a/src/phases.js +++ b/src/phases.js @@ -913,9 +913,10 @@ async function runAll(manifest, io) { for (const f of files) if (!entry.conflict_files.includes(f)) entry.conflict_files.push(f); if (!autonomous) { const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; + const git = `git -C ${shellQuote(featureDir(m))} ${gitIdentityArgs(m)}`; return { stopped: `join ${name}: ${unit.id} conflicts in ${files.join(', ') || 'files git reported'}; ` - + `merge it by hand with git -C ${shellQuote(featureDir(m))} merge --no-ff -m ${shellQuote(message)} ` - + `${unit.head}, resolve and commit, then relaunch` }; + + `merge it by hand with ${git}merge --no-ff -m ${shellQuote(message)} ` + + `${unit.head}, resolve and commit (${git}commit --no-edit), then relaunch` }; } const res = await callM(`${name} resolve ${unit.id}`, 'Join', waveResolvePrompt(m, wave, unit, files), statusSchema(), standard); diff --git a/src/prompts-phase.js b/src/prompts-phase.js index 55dd70a..9e1fba0 100644 --- a/src/prompts-phase.js +++ b/src/prompts-phase.js @@ -5,6 +5,20 @@ function gitAdmin(m) { : `git -C ${shellQuote(m.repo.root)}`; } +// The run's git identity (1.6.7): the user.name=/user.email= pairs setup +// read from the commit rules, [] when it recorded none (a dry run, or a +// manifest from before 1.6.7). +function gitIdentity(m) { + const r = m.setup_result; + return r && Array.isArray(r.git_identity) ? r.git_identity : []; +} + +// The identity as git options, each shell-quoted and followed by a space so +// it goes between `git -C ` and the subcommand; '' for none. +function gitIdentityArgs(m) { + return gitIdentity(m).map((p) => `-c ${shellQuote(p)} `).join(''); +} + // Rules every phase agent gets, the general long-command rule included. // checkout (optional) replaces the checkout rules for the feature checkout // (setup works in several checkouts). @@ -340,8 +354,9 @@ function integratePrompt(m, preludeTip, opts = {}) { const q = shellQuote; const dir = q(featureDir(m)); const admin = gitAdmin(m); + const id = gitIdentityArgs(m); const merges = m.lanes.map((lane) => - ` git -C ${dir} merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); + ` git -C ${dir} ${id}merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); // Step 3 runs the delivery inventory once (spec 4.4): the project's setup // and the lanes' distinct ones, then every distinct check of the project // and the lane_commands overrides, with the long-check procedure (1.6.1) @@ -402,8 +417,9 @@ function integratePrompt(m, preludeTip, opts = {}) { ' file in conflict_files (a later agent resolves them).', ] : [ ' A branch that is already merged reports already up to date; that is fine. On a conflict, resolve it', - ' keeping the intent of both lanes (read the plan tasks that touched the file) and commit the merge; if', - ' you cannot resolve it with confidence, run git merge --abort and fail naming the files.', + ' keeping the intent of both lanes (read the plan tasks that touched the file) and commit the merge with', + ` \`git -C ${dir} ${id}commit --no-edit\`; if you cannot resolve it with confidence, run git merge --abort`, + ' and fail naming the files.', ]; const testFailureLine = testFailure === 'heal' ? ' Fix only small, obvious integration breakage (commit it per the commit rules). If a project command ' @@ -492,8 +508,9 @@ function integratePrompt(m, preludeTip, opts = {}) { function resolveConflictsPrompt(m, preludeTip, conflictFiles) { const q = shellQuote; const dir = q(featureDir(m)); + const id = gitIdentityArgs(m); const merges = m.lanes.map((lane) => - ` git -C ${dir} merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); + ` git -C ${dir} ${id}merge --no-ff -m ${q(laneWhere(m, lane).branch)}`); const files = conflictFiles && conflictFiles.length > 0 ? conflictFiles.join(', ') : '(the files the merge reports)'; return [ `You are the conflict resolver for parallel-lanes run ${m.run_id}.`, @@ -506,7 +523,8 @@ function resolveConflictsPrompt(m, preludeTip, conflictFiles) { ...merges, ' A branch already merged reports already up to date; that is fine. On a conflict, resolve it keeping', ' the intent of both lanes: read the plan tasks that touched the conflicting files and keep what each', - ' lane meant to do, then commit the merge. Resolve the conflicts only; make no other change.', + ` lane meant to do, then commit the merge with \`git -C ${dir} ${id}commit --no-edit\`. Resolve the`, + ' conflicts only; make no other change.', ' Do not run the project commands, rewrite history, or remove any worktree or branch.', '', `Plan: ${m.plan}`, @@ -532,7 +550,8 @@ function waveMergePrompt(m, wave, units, checks = [], known = []) { const dir = featureDir(m); const recovery = wave === null; const merge = [`cd ${q(dir)} && ${pythonCommand(m)} ${q(`${m.skill_dir}/scripts/merge-wave`)} ${q(dir)}`, - q(m.repo.ledger_dir), ...(recovery ? ['--recovery'] : ['--wave', q(String(wave))]), + q(m.repo.ledger_dir), ...gitIdentity(m).flatMap((p) => ['--git-config', q(p)]), + ...(recovery ? ['--recovery'] : ['--wave', q(String(wave))]), ...units.flatMap((u) => ['--unit', q(u.id), q(u.base), q(u.head)]), ...known.flatMap((u) => ['--known', q(u.id), q(u.head)])].join(' '); const run = checks.length > 0 ? runChecksCommand(m, dir, checks, resultPath(), 'join') : null; @@ -614,6 +633,7 @@ function waveMergeSchema() { function waveResolvePrompt(m, wave, unit, conflictFiles) { const q = shellQuote; const dir = q(featureDir(m)); + const id = gitIdentityArgs(m); const files = conflictFiles && conflictFiles.length > 0 ? conflictFiles.join(', ') : '(the files the merge reports)'; const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; return [ @@ -623,10 +643,11 @@ function waveResolvePrompt(m, wave, unit, conflictFiles) { '', `1. git -C ${dir} status --porcelain --untracked-files=all must print nothing; otherwise return status`, ' failed (never make it so by deleting, cleaning, or stashing files).', - `2. git -C ${dir} merge --no-ff -m ${q(message)} ${q(unit.head)}`, + `2. git -C ${dir} ${id}merge --no-ff -m ${q(message)} ${q(unit.head)}`, " Resolve each conflict keeping the intent of both sides: read the plan tasks of the units that touched the", - ' file and keep what each meant to do, then commit the merge. Resolve the conflicts only; make no other', - ' change, run no project command, rewrite no history, and remove no worktree or branch.', + ` file and keep what each meant to do, then commit the merge with \`git -C ${dir} ${id}commit --no-edit\`.`, + ' Resolve the conflicts only; make no other change, run no project command, rewrite no history, and remove', + ' no worktree or branch.', `3. If you cannot resolve it, run git -C ${dir} merge --abort so the checkout is clean, and return status`, ' failed saying why.', '', diff --git a/src/validate.js b/src/validate.js index 0c924eb..b7998c7 100644 --- a/src/validate.js +++ b/src/validate.js @@ -616,6 +616,21 @@ function validateManifest(m) { } } } + // git_identity (1.6.7): the -c user.name=/user.email= pairs the commit + // rules name, as setup resolved them; the engine's merges and the + // commits it spells out carry them. Absent before 1.6.7. + if ('git_identity' in r) { + const pairs = r.git_identity; + const keys = Array.isArray(pairs) + ? pairs.map((p) => (typeof p === 'string' ? /^(user\.(?:name|email))=.+$/.exec(p) : null)) : null; + const ok = keys !== null && keys.length <= 2 && keys.every((k) => k !== null) + && new Set(keys.map((k) => k[1])).size === keys.length + && (keys.length < 2 || keys[0][1] === 'user.name'); + if (!ok) { + err('setup_result.git_identity: must be a list of at most two strings, ' + + 'user.name=VALUE then user.email=VALUE, each key once'); + } + } if (!isObject(r.worktrees)) { err('setup_result.worktrees: must be an object mapping lane ids to absolute paths'); } else { diff --git a/tests/git-identity.test.mjs b/tests/git-identity.test.mjs new file mode 100644 index 0000000..2087464 --- /dev/null +++ b/tests/git-identity.test.mjs @@ -0,0 +1,130 @@ +// The run's git identity (1.6.7 design 4.2): setup_result.git_identity goes +// as -c options on every merge and commit the engine spells out, and as +// --git-config to merge-wave. Without it the prompts carry no identity. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { loadHelpers, H } from './harness.mjs'; + +const { + integratePrompt, resolveConflictsPrompt, waveMergePrompt, waveResolvePrompt, gitIdentityArgs, +} = await loadHelpers([ + 'integratePrompt', 'resolveConflictsPrompt', 'waveMergePrompt', 'waveResolvePrompt', 'gitIdentityArgs', +]); + +function task(id) { + return { id, title: `Task ${id}`, files: [`src/${id}.js`], tier: 'standard', security: false }; +} + +function manifest(identity) { + const m = { + version: 1, + run_id: 'run-1', + plan: '/work/plan.md', + spec: '/work/spec.md', + commit_rules: 'plain ASCII', + repo: { + mode: 'git', root: '/work/repo', git_dir: null, base_ref: 'main', branch: 'pl/run-1', + worktree_root: '/work/wt', ledger_dir: '/work/ledger', + }, + commands: { setup: ['npm ci'], test: ['npm test'], lint: [], build: [] }, + prelude: [], + lanes: [ + { id: 'alpha', name: 'Lane alpha', tasks: [task('A1')] }, + { id: 'beta', name: 'Lane beta', tasks: [task('B1')] }, + ], + join: [task('J1'), task('J2')], + hooks: {}, + limits: { review_rounds: 5, max_parallel_lanes: 3 }, + dry_run: false, + done: [], + reviewed: [], + sp_dir: null, + skill_dir: '/skills/parallel-lanes', + setup_result: { + feature_head: H('F0'), discarded: [], worktrees: { alpha: '/work/wt/lane-alpha', beta: '/work/wt/lane-beta' }, + }, + }; + if (identity !== undefined) m.setup_result.git_identity = identity; + return m; +} + +const IDENTITY = ["user.name=Riley O'Q", 'user.email=r@x.invalid']; +const ARGS = "-c 'user.name=Riley O'\\''Q' -c 'user.email=r@x.invalid' "; +const DIR = "'/work/repo'"; +const UNITS = [{ id: 'J1', base: H('I1'), head: H('J1') }, { id: 'J2', base: H('I1'), head: H('J2') }]; + +// Every prompt the identity reaches, built from one manifest. +function prompts(m) { + return { + 'integrate resolve': integratePrompt(m, H('P0'), { conflictMode: 'resolve' }), + 'integrate abort': integratePrompt(m, H('P0'), { conflictMode: 'abort', testFailure: 'heal' }), + resolver: resolveConflictsPrompt(m, H('P0'), ['src/x.js']), + 'wave merge': waveMergePrompt(m, 1, UNITS), + 'recovery merge': waveMergePrompt(m, null, UNITS), + 'wave resolver': waveResolvePrompt(m, 1, UNITS[1], ['src/x.js']), + 'recovery resolver': waveResolvePrompt(m, null, UNITS[1], ['src/x.js']), + }; +} + +test('gitIdentityArgs shell-quotes each pair as a -c option with a trailing space', () => { + assert.equal(gitIdentityArgs(manifest(IDENTITY)), ARGS); + assert.equal(gitIdentityArgs(manifest(['user.email=r@x.invalid'])), "-c 'user.email=r@x.invalid' "); + assert.equal(gitIdentityArgs(manifest([])), ''); + assert.equal(gitIdentityArgs(manifest()), ''); + const dry = manifest(); + delete dry.setup_result; + assert.equal(gitIdentityArgs(dry), ''); +}); + +test('every lane merge of integrate and the lane resolver carries the identity', () => { + const p = prompts(manifest(IDENTITY)); + for (const name of ['integrate resolve', 'integrate abort', 'resolver']) { + const lines = p[name].split('\n').filter((l) => l.includes('merge --no-ff')); + assert.equal(lines.length, 2, `${name}: one merge per lane`); + for (const line of lines) { + assert.ok(line.includes(`git -C ${DIR} ${ARGS}merge --no-ff -m `), `${name}: ${line}`); + } + } +}); + +test('the resolving prompts spell out the commit with the identity', () => { + const p = prompts(manifest(IDENTITY)); + for (const name of ['integrate resolve', 'resolver', 'wave resolver', 'recovery resolver']) { + assert.ok(p[name].includes(`\`git -C ${DIR} ${ARGS}commit --no-edit\``), name); + } + assert.ok(!p['integrate abort'].includes('commit --no-edit'), 'abort mode commits no merge'); +}); + +test('the wave resolver merges with the identity', () => { + const p = prompts(manifest(IDENTITY)); + assert.ok(p['wave resolver'].includes( + `git -C ${DIR} ${ARGS}merge --no-ff -m 'join wave 1: merge J2' '${H('J2')}'`), p['wave resolver']); + assert.ok(p['recovery resolver'].includes( + `git -C ${DIR} ${ARGS}merge --no-ff -m 'join recovery: merge J2' '${H('J2')}'`), p['recovery resolver']); +}); + +test('the wave merge passes each pair to merge-wave as --git-config', () => { + const p = prompts(manifest(IDENTITY)); + const pairs = "--git-config 'user.name=Riley O'\\''Q' --git-config 'user.email=r@x.invalid'"; + assert.ok(p['wave merge'].includes(`'/work/ledger' ${pairs} --wave '1' --unit`), p['wave merge']); + assert.ok(p['recovery merge'].includes(`'/work/ledger' ${pairs} --recovery --unit`), p['recovery merge']); +}); + +test('with no identity the prompts carry none', () => { + const dry = manifest(); + delete dry.setup_result; + const none = prompts(manifest()); + for (const m of [manifest([]), dry]) { + const p = prompts(m); + for (const [name, text] of Object.entries(p)) assert.equal(text, none[name], name); + } + for (const [name, text] of Object.entries(none)) { + assert.ok(!text.includes("-c 'user."), `${name}: no -c option`); + assert.ok(!text.includes('--git-config'), `${name}: no --git-config`); + } + for (const name of ['integrate resolve', 'resolver', 'wave resolver', 'recovery resolver']) { + assert.ok(none[name].includes(`\`git -C ${DIR} commit --no-edit\``), name); + } + assert.ok(none['wave merge'].includes(`'/work/ledger' --wave '1' --unit`), none['wave merge']); + assert.ok(none['integrate resolve'].includes(`git -C ${DIR} merge --no-ff -m `)); +}); diff --git a/tests/join-waves-run.test.mjs b/tests/join-waves-run.test.mjs index 4ada85b..0b10fd3 100644 --- a/tests/join-waves-run.test.mjs +++ b/tests/join-waves-run.test.mjs @@ -141,12 +141,17 @@ test('a failed resolver stops the run', async () => { test('a supervised conflict stops with the sha to merge by hand', async () => { const conflict = merged(H('I1'), { status: 'conflict', merged: ['J1'], conflict_unit: 'J2', conflict_files: ['src/x.js'] }); - const { result, calls } = await run(manifest({ autonomy: 'supervised' }), script(H('J3'), { 'wave 1 merge': [conflict] })); + const m = manifest({ autonomy: 'supervised' }); + m.setup_result.git_identity = ["user.name=Riley O'Q", 'user.email=r@x.invalid']; + const { result, calls } = await run(m, script(H('J3'), { 'wave 1 merge': [conflict] })); assert.equal(result.status, 'stopped'); assert.ok(!labels(calls).some((l) => l.includes('resolve'))); const why = result.stopped_lanes.find((s) => s.lane === 'join').reason; assert.ok(why.includes(H('J2')) && /by hand/.test(why) && why.includes('src/x.js'), why); - assert.ok(why.includes(`merge --no-ff -m 'join wave 1: merge J2' ${H('J2')}`), `the exact command: ${why}`); + const args = "-c 'user.name=Riley O'\\''Q' -c 'user.email=r@x.invalid' "; + assert.ok(why.includes(`git -C '/work/repo' ${args}merge --no-ff -m 'join wave 1: merge J2' ${H('J2')}`), + `the exact command: ${why}`); + assert.ok(why.includes(`git -C '/work/repo' ${args}commit --no-edit`), `the commit: ${why}`); }); const withChecks = (extra = {}) => manifest({ diff --git a/tests/validate.test.mjs b/tests/validate.test.mjs index b201df5..6363bd0 100644 --- a/tests/validate.test.mjs +++ b/tests/validate.test.mjs @@ -564,6 +564,31 @@ test('setup_result shape errors are reported', () => { assertError(validateManifest(m), 'setup_result'); }); +test('setup_result.git_identity is optional and checked', () => { + const message = 'setup_result.git_identity: must be a list of at most two strings, ' + + 'user.name=VALUE then user.email=VALUE, each key once'; + const withIdentity = (identity) => { + const m = validManifest(); + m.setup_result = setupResult(m); + if (identity !== undefined) m.setup_result.git_identity = identity; + return validateManifest(m); + }; + for (const identity of [undefined, [], ['user.name=n', 'user.email=e'], ['user.email=e']]) { + assert.deepEqual(withIdentity(identity), [], JSON.stringify(identity)); + } + const bad = [ + 'x', + ['user.name=n', 'user.email=e', 'user.name=m'], + ['user.signingkey=k'], + ['user.name='], + ['user.name=a', 'user.name=b'], + ['User.Name=a'], + ]; + for (const identity of bad) { + assert.deepEqual(withIdentity(identity), [message], JSON.stringify(identity)); + } +}); + test('start_points: optional prelude and join shas', () => { const m = validManifest(); m.start_points = {}; From 696c08a513fffe4c2fe1f9f1db0a5d559b9c44f1 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:32:20 -0700 Subject: [PATCH 06/11] fix: a refused wave merge stops the run instead of going to a resolver --- run.workflow.js | 83 ++++++++++++++++++------ src/phases.js | 25 ++++++-- src/prompts-phase.js | 55 ++++++++++++---- src/prompts-task.js | 3 + tests/engine.test.mjs | 6 ++ tests/git-identity.test.mjs | 4 +- tests/join-waves-run.test.mjs | 115 +++++++++++++++++++++++++++++++--- 7 files changed, 246 insertions(+), 45 deletions(-) diff --git a/run.workflow.js b/run.workflow.js index d39f4a6..3ad05e7 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1665,6 +1665,9 @@ function fixMessagesLines() { 'with `chore: continue Task `.', "The plan's message applies to a task's first commit; a fix commit carries its fix message, and a", 'continuation commit its continuation message; none of them is a commit-rule finding.', + "The engine's own commits: join merges `chore: join wave : merge ` and", + '`chore: join recovery: merge ` (before 1.6.7 without `chore: `) and the wave checks fix', + `\`${waveChecksFixMessage()}\`; none of them is a commit-rule finding.`, ]; } @@ -2756,9 +2759,9 @@ function integratePrompt(m, preludeTip, opts = {}) { ' may list only merges (two parents) of the lane branches above, integration or post-integration fix', ' commits (yours, or from an earlier attempt in this run), and commits an earlier attempt at a join task', ` made (join tasks: ${joinIds}; their recorded commits are in ${m.repo.ledger_dir}/join.jsonl).`, - " A join that ran in waves also merged its units' heads ('join wave : merge ' and", - " 'join recovery: merge ', a resolver's included) and may have a wave checks fix", - ' (`fix: make the wave checks pass`).', + " A join that ran in waves also merged its units' heads ('chore: join wave : merge ' and", + " 'chore: join recovery: merge ', or the same without 'chore: ' from before 1.6.7, a resolver's", + ' included) and may have a wave checks fix (`fix: make the wave checks pass`).', ...finalFixes, " Any other commit landed on the feature branch outside the run's steps: fail listing it (do not", ' rewrite history).', @@ -2862,7 +2865,10 @@ function waveMergePrompt(m, wave, units, checks = [], known = []) { ' conflicts and stops there, records the wave in the ledger, and prints one JSON line. Do not merge', ' anything yourself, resolve a conflict, or change any file.', '2. If it printed status conflict, stop: return status conflict with its head, merged, conflict_unit,', - ' conflict_files and overlaps. If it exited 2 or 3, stop: return status failed with its message.', + ' conflict_files and overlaps. If it exited 4 (status failed: git refused a merge), stop: return status', + ' failed with its failed_unit, error, merged, in_history and head; run no checks;', + ' do not change git config, set an identity, or retry the merge.', + ' If it exited 2 or 3, stop: return status failed with its message.', ...(run === null ? [] : [ `3. It merged every unit: run the wave's checks on the merged tip (the last wave with more than one unit,`, " whose merge no later task's checks cover). One run-checks call:", @@ -2881,7 +2887,8 @@ function waveMergePrompt(m, wave, units, checks = [], known = []) { phaseRules(m), '', 'Return status done (every unit merged), conflict or failed; head = the head merge-wave printed; merged,', - 'conflict_unit (empty when none), conflict_files, overlaps and in_history copied from its JSON;', + 'conflict_unit (empty when none), conflict_files, overlaps, in_history, failed_unit and error (empty when', + 'none) copied from its JSON;', ...(run === null ? [] : ['checks = {ok, unfinished, notes} for the checks of step 3 (notes: the failing commands);']), 'notes.', ].join('\n'); @@ -2897,6 +2904,8 @@ function waveMergeSchema() { conflict_unit: { type: 'string' }, conflict_files: { type: 'array', items: { type: 'string' } }, in_history: { type: 'array', items: { type: 'string' } }, + failed_unit: { type: 'string' }, + error: { type: 'string' }, overlaps: { type: 'array', items: { @@ -2925,7 +2934,12 @@ function waveResolvePrompt(m, wave, unit, conflictFiles) { const dir = q(featureDir(m)); const id = gitIdentityArgs(m); const files = conflictFiles && conflictFiles.length > 0 ? conflictFiles.join(', ') : '(the files the merge reports)'; - const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; + const message = wave === null ? `chore: join recovery: merge ${unit.id}` : `chore: join wave ${wave}: merge ${unit.id}`; + // The resolver records its own merge (1.6.7 design 4.5). + const record = ledgerCommand(m, '_run', { + task: '_run', event: 'wave_merged', ...(wave === null ? { recovery: true } : { wave }), units: [unit.id], + merged: [unit.id], head: '', conflict_files: conflictFiles || [], resolved: true, + }, featureDir(m)); return [ `You are the wave resolver for parallel-lanes run ${m.run_id}, ${wave === null ? 'the recovery merge' : `join wave ${wave}`}.`, `Work in ${featureDir(m)} on the feature branch ${m.repo.branch}; do not switch branches.`, @@ -2940,6 +2954,8 @@ function waveResolvePrompt(m, wave, unit, conflictFiles) { ' no worktree or branch.', `3. If you cannot resolve it, run git -C ${dir} merge --abort so the checkout is clean, and return status`, ' failed saying why.', + `4. After committing the merge, record it, with the full sha of git -C ${dir} rev-parse HEAD:`, + ` ${record}`, '', `Plan: ${m.plan}`, keepFilesRule(), @@ -3031,17 +3047,33 @@ function waveChecksSchema() { // wave's paths two units changed, and the conflicts the resolver settled, // from earlier launches (m.wave_merges, ledger status) and this one // (m.join_waves_now, the run result's join_waves). +// 1.6.7 (design 4.5): lines appear once each. A ledger event's conflict files +// are the resolver's when the event is the resolver's own (resolved) or was +// written before 1.6.7 (no merged field); a conflict no later resolved event +// of the same wave (or recovery) merged the unit of was settled outside the +// run's resolver. This launch's report entries keep the resolver text. function waveLines(m) { const lines = []; - const all = [...(Array.isArray(m.wave_merges) ? m.wave_merges : []), - ...(Array.isArray(m.join_waves_now) ? m.join_waves_now : [])]; - for (const w of all) { + const add = (line) => { if (!lines.includes(line)) lines.push(line); }; + const events = Array.isArray(m.wave_merges) ? m.wave_merges : []; + const now = Array.isArray(m.join_waves_now) ? m.join_waves_now : []; + const sameMerge = (a, b) => (a.recovery ? b.recovery === true : !b.recovery && a.wave === b.wave); + const settledLater = (w, i) => events.slice(i + 1).some((e) => e.resolved === true && sameMerge(w, e) + && Array.isArray(e.merged) && e.merged.includes(w.conflict_unit)); + [...events, ...now].forEach((w, i) => { const name = w.recovery ? 'the recovery merge' : `wave ${w.wave}`; - for (const o of Array.isArray(w.overlaps) ? w.overlaps : []) lines.push(`- ${name}: ${o.path} (${o.units.join(', ')})`); - const resolved = Array.isArray(w.resolved_files) && w.resolved_files.length > 0 ? w.resolved_files + for (const o of Array.isArray(w.overlaps) ? w.overlaps : []) add(`- ${name}: ${o.path} (${o.units.join(', ')})`); + const fromLedger = i < events.length; + const files = !fromLedger && Array.isArray(w.resolved_files) && w.resolved_files.length > 0 ? w.resolved_files : (Array.isArray(w.conflict_files) ? w.conflict_files : []); - if (resolved.length > 0) lines.push(`- ${name}: merge conflicts the wave resolver settled in ${resolved.join(', ')}`); - } + if (files.length === 0) return; + if (!fromLedger || w.resolved === true || !('merged' in w)) { + add(`- ${name}: merge conflicts the wave resolver settled in ${files.join(', ')}`); + } else if (!settledLater(w, i)) { + add(`- ${name}: merge conflicts in ${files.join(', ')}, settled outside the run's resolver ` + + '(by hand or in a later launch)'); + } + }); if (lines.length === 0) return []; return [ 'Join waves: units of a wave ran side by side on their own branches and were merged. Review these paths', @@ -5931,6 +5963,8 @@ async function runAll(manifest, io) { // run with the sha to merge by hand (supervised). With checks (the last // multi-unit wave's, design 4.4) the merge agent runs them; a failure gets // one fix, its re-review and a recheck (autonomous) or stops (supervised). + // A merge git refused (status failed, or a conflict with no files: 1.6.7 + // design 4.3) stops the run in both autonomies with no resolver. // entry is the wave's report entry, filled in. Returns {head} or {stopped}. const mergeUnits = async (wave, units, checks, entry, known = []) => { const name = wave === null ? 'recovery' : `wave ${wave}`; @@ -5939,13 +5973,24 @@ async function runAll(manifest, io) { r = await callM(`${name} merge`, 'Join', waveMergePrompt(m, wave, units, checks, known), waveMergeSchema(), sonnetHigh); if (state.refused.length > 0) return { stopped: 'an agent was refused' }; - if (!r || r.status !== 'conflict') break; + // Worded so the transient rule relaunches it (reference.md). + if (!r) return { stopped: `no result from ${name} merge` }; + const noFiles = !Array.isArray(r.conflict_files) || r.conflict_files.length === 0; + if (r.status === 'failed' || (r.status === 'conflict' && noFiles)) { + if (isSha(r.head)) entry.head = r.head; + if (Array.isArray(r.in_history)) entry.in_history = r.in_history; + const what = r.failed_unit && r.error ? `the merge of ${r.failed_unit} failed: ${r.error}` + : `the merge failed: ${r.notes}`; + return { stopped: `join ${name}: ${what}` }; + } + if (r.status !== 'conflict') break; const unit = units.find((u) => u.id === r.conflict_unit) || units.find((u) => !(Array.isArray(r.merged) ? r.merged : []).includes(u.id)) || units[0]; const files = Array.isArray(r.conflict_files) ? r.conflict_files : []; for (const f of files) if (!entry.conflict_files.includes(f)) entry.conflict_files.push(f); if (!autonomous) { - const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; + const message = wave === null ? `chore: join recovery: merge ${unit.id}` + : `chore: join wave ${wave}: merge ${unit.id}`; const git = `git -C ${shellQuote(featureDir(m))} ${gitIdentityArgs(m)}`; return { stopped: `join ${name}: ${unit.id} conflicts in ${files.join(', ') || 'files git reported'}; ` + `merge it by hand with ${git}merge --no-ff -m ${shellQuote(message)} ` @@ -5960,9 +6005,11 @@ async function runAll(manifest, io) { } for (const f of files) if (!entry.resolved_files.includes(f)) entry.resolved_files.push(f); } - if (!r || r.status !== 'done' || !isSha(r.head)) { - return { stopped: `join ${name}: the merge failed: ${r ? r.notes : `no result from ${name} merge`}` }; - } + if (r.status !== 'done' || !isSha(r.head)) return { stopped: `join ${name}: the merge failed: ${r.notes}` }; + // Every unit asked for must now be in the history (1.6.7 design 4.3). + const landed = [...(Array.isArray(r.merged) ? r.merged : []), ...(Array.isArray(r.in_history) ? r.in_history : [])]; + const left = units.filter((u) => !landed.includes(u.id)).map((u) => u.id); + if (left.length > 0) return { stopped: `join ${name}: the merge left out ${left.join(', ')}` }; let head = r.head; entry.head = head; entry.overlaps = Array.isArray(r.overlaps) ? r.overlaps : []; diff --git a/src/phases.js b/src/phases.js index 229699c..486a7e2 100644 --- a/src/phases.js +++ b/src/phases.js @@ -898,6 +898,8 @@ async function runAll(manifest, io) { // run with the sha to merge by hand (supervised). With checks (the last // multi-unit wave's, design 4.4) the merge agent runs them; a failure gets // one fix, its re-review and a recheck (autonomous) or stops (supervised). + // A merge git refused (status failed, or a conflict with no files: 1.6.7 + // design 4.3) stops the run in both autonomies with no resolver. // entry is the wave's report entry, filled in. Returns {head} or {stopped}. const mergeUnits = async (wave, units, checks, entry, known = []) => { const name = wave === null ? 'recovery' : `wave ${wave}`; @@ -906,13 +908,24 @@ async function runAll(manifest, io) { r = await callM(`${name} merge`, 'Join', waveMergePrompt(m, wave, units, checks, known), waveMergeSchema(), sonnetHigh); if (state.refused.length > 0) return { stopped: 'an agent was refused' }; - if (!r || r.status !== 'conflict') break; + // Worded so the transient rule relaunches it (reference.md). + if (!r) return { stopped: `no result from ${name} merge` }; + const noFiles = !Array.isArray(r.conflict_files) || r.conflict_files.length === 0; + if (r.status === 'failed' || (r.status === 'conflict' && noFiles)) { + if (isSha(r.head)) entry.head = r.head; + if (Array.isArray(r.in_history)) entry.in_history = r.in_history; + const what = r.failed_unit && r.error ? `the merge of ${r.failed_unit} failed: ${r.error}` + : `the merge failed: ${r.notes}`; + return { stopped: `join ${name}: ${what}` }; + } + if (r.status !== 'conflict') break; const unit = units.find((u) => u.id === r.conflict_unit) || units.find((u) => !(Array.isArray(r.merged) ? r.merged : []).includes(u.id)) || units[0]; const files = Array.isArray(r.conflict_files) ? r.conflict_files : []; for (const f of files) if (!entry.conflict_files.includes(f)) entry.conflict_files.push(f); if (!autonomous) { - const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; + const message = wave === null ? `chore: join recovery: merge ${unit.id}` + : `chore: join wave ${wave}: merge ${unit.id}`; const git = `git -C ${shellQuote(featureDir(m))} ${gitIdentityArgs(m)}`; return { stopped: `join ${name}: ${unit.id} conflicts in ${files.join(', ') || 'files git reported'}; ` + `merge it by hand with ${git}merge --no-ff -m ${shellQuote(message)} ` @@ -927,9 +940,11 @@ async function runAll(manifest, io) { } for (const f of files) if (!entry.resolved_files.includes(f)) entry.resolved_files.push(f); } - if (!r || r.status !== 'done' || !isSha(r.head)) { - return { stopped: `join ${name}: the merge failed: ${r ? r.notes : `no result from ${name} merge`}` }; - } + if (r.status !== 'done' || !isSha(r.head)) return { stopped: `join ${name}: the merge failed: ${r.notes}` }; + // Every unit asked for must now be in the history (1.6.7 design 4.3). + const landed = [...(Array.isArray(r.merged) ? r.merged : []), ...(Array.isArray(r.in_history) ? r.in_history : [])]; + const left = units.filter((u) => !landed.includes(u.id)).map((u) => u.id); + if (left.length > 0) return { stopped: `join ${name}: the merge left out ${left.join(', ')}` }; let head = r.head; entry.head = head; entry.overlaps = Array.isArray(r.overlaps) ? r.overlaps : []; diff --git a/src/prompts-phase.js b/src/prompts-phase.js index 9e1fba0..68c1a4f 100644 --- a/src/prompts-phase.js +++ b/src/prompts-phase.js @@ -466,9 +466,9 @@ function integratePrompt(m, preludeTip, opts = {}) { ' may list only merges (two parents) of the lane branches above, integration or post-integration fix', ' commits (yours, or from an earlier attempt in this run), and commits an earlier attempt at a join task', ` made (join tasks: ${joinIds}; their recorded commits are in ${m.repo.ledger_dir}/join.jsonl).`, - " A join that ran in waves also merged its units' heads ('join wave : merge ' and", - " 'join recovery: merge ', a resolver's included) and may have a wave checks fix", - ' (`fix: make the wave checks pass`).', + " A join that ran in waves also merged its units' heads ('chore: join wave : merge ' and", + " 'chore: join recovery: merge ', or the same without 'chore: ' from before 1.6.7, a resolver's", + ' included) and may have a wave checks fix (`fix: make the wave checks pass`).', ...finalFixes, " Any other commit landed on the feature branch outside the run's steps: fail listing it (do not", ' rewrite history).', @@ -572,7 +572,10 @@ function waveMergePrompt(m, wave, units, checks = [], known = []) { ' conflicts and stops there, records the wave in the ledger, and prints one JSON line. Do not merge', ' anything yourself, resolve a conflict, or change any file.', '2. If it printed status conflict, stop: return status conflict with its head, merged, conflict_unit,', - ' conflict_files and overlaps. If it exited 2 or 3, stop: return status failed with its message.', + ' conflict_files and overlaps. If it exited 4 (status failed: git refused a merge), stop: return status', + ' failed with its failed_unit, error, merged, in_history and head; run no checks;', + ' do not change git config, set an identity, or retry the merge.', + ' If it exited 2 or 3, stop: return status failed with its message.', ...(run === null ? [] : [ `3. It merged every unit: run the wave's checks on the merged tip (the last wave with more than one unit,`, " whose merge no later task's checks cover). One run-checks call:", @@ -591,7 +594,8 @@ function waveMergePrompt(m, wave, units, checks = [], known = []) { phaseRules(m), '', 'Return status done (every unit merged), conflict or failed; head = the head merge-wave printed; merged,', - 'conflict_unit (empty when none), conflict_files, overlaps and in_history copied from its JSON;', + 'conflict_unit (empty when none), conflict_files, overlaps, in_history, failed_unit and error (empty when', + 'none) copied from its JSON;', ...(run === null ? [] : ['checks = {ok, unfinished, notes} for the checks of step 3 (notes: the failing commands);']), 'notes.', ].join('\n'); @@ -607,6 +611,8 @@ function waveMergeSchema() { conflict_unit: { type: 'string' }, conflict_files: { type: 'array', items: { type: 'string' } }, in_history: { type: 'array', items: { type: 'string' } }, + failed_unit: { type: 'string' }, + error: { type: 'string' }, overlaps: { type: 'array', items: { @@ -635,7 +641,12 @@ function waveResolvePrompt(m, wave, unit, conflictFiles) { const dir = q(featureDir(m)); const id = gitIdentityArgs(m); const files = conflictFiles && conflictFiles.length > 0 ? conflictFiles.join(', ') : '(the files the merge reports)'; - const message = wave === null ? `join recovery: merge ${unit.id}` : `join wave ${wave}: merge ${unit.id}`; + const message = wave === null ? `chore: join recovery: merge ${unit.id}` : `chore: join wave ${wave}: merge ${unit.id}`; + // The resolver records its own merge (1.6.7 design 4.5). + const record = ledgerCommand(m, '_run', { + task: '_run', event: 'wave_merged', ...(wave === null ? { recovery: true } : { wave }), units: [unit.id], + merged: [unit.id], head: '', conflict_files: conflictFiles || [], resolved: true, + }, featureDir(m)); return [ `You are the wave resolver for parallel-lanes run ${m.run_id}, ${wave === null ? 'the recovery merge' : `join wave ${wave}`}.`, `Work in ${featureDir(m)} on the feature branch ${m.repo.branch}; do not switch branches.`, @@ -650,6 +661,8 @@ function waveResolvePrompt(m, wave, unit, conflictFiles) { ' no worktree or branch.', `3. If you cannot resolve it, run git -C ${dir} merge --abort so the checkout is clean, and return status`, ' failed saying why.', + `4. After committing the merge, record it, with the full sha of git -C ${dir} rev-parse HEAD:`, + ` ${record}`, '', `Plan: ${m.plan}`, keepFilesRule(), @@ -741,17 +754,33 @@ function waveChecksSchema() { // wave's paths two units changed, and the conflicts the resolver settled, // from earlier launches (m.wave_merges, ledger status) and this one // (m.join_waves_now, the run result's join_waves). +// 1.6.7 (design 4.5): lines appear once each. A ledger event's conflict files +// are the resolver's when the event is the resolver's own (resolved) or was +// written before 1.6.7 (no merged field); a conflict no later resolved event +// of the same wave (or recovery) merged the unit of was settled outside the +// run's resolver. This launch's report entries keep the resolver text. function waveLines(m) { const lines = []; - const all = [...(Array.isArray(m.wave_merges) ? m.wave_merges : []), - ...(Array.isArray(m.join_waves_now) ? m.join_waves_now : [])]; - for (const w of all) { + const add = (line) => { if (!lines.includes(line)) lines.push(line); }; + const events = Array.isArray(m.wave_merges) ? m.wave_merges : []; + const now = Array.isArray(m.join_waves_now) ? m.join_waves_now : []; + const sameMerge = (a, b) => (a.recovery ? b.recovery === true : !b.recovery && a.wave === b.wave); + const settledLater = (w, i) => events.slice(i + 1).some((e) => e.resolved === true && sameMerge(w, e) + && Array.isArray(e.merged) && e.merged.includes(w.conflict_unit)); + [...events, ...now].forEach((w, i) => { const name = w.recovery ? 'the recovery merge' : `wave ${w.wave}`; - for (const o of Array.isArray(w.overlaps) ? w.overlaps : []) lines.push(`- ${name}: ${o.path} (${o.units.join(', ')})`); - const resolved = Array.isArray(w.resolved_files) && w.resolved_files.length > 0 ? w.resolved_files + for (const o of Array.isArray(w.overlaps) ? w.overlaps : []) add(`- ${name}: ${o.path} (${o.units.join(', ')})`); + const fromLedger = i < events.length; + const files = !fromLedger && Array.isArray(w.resolved_files) && w.resolved_files.length > 0 ? w.resolved_files : (Array.isArray(w.conflict_files) ? w.conflict_files : []); - if (resolved.length > 0) lines.push(`- ${name}: merge conflicts the wave resolver settled in ${resolved.join(', ')}`); - } + if (files.length === 0) return; + if (!fromLedger || w.resolved === true || !('merged' in w)) { + add(`- ${name}: merge conflicts the wave resolver settled in ${files.join(', ')}`); + } else if (!settledLater(w, i)) { + add(`- ${name}: merge conflicts in ${files.join(', ')}, settled outside the run's resolver ` + + '(by hand or in a later launch)'); + } + }); if (lines.length === 0) return []; return [ 'Join waves: units of a wave ran side by side on their own branches and were merged. Review these paths', diff --git a/src/prompts-task.js b/src/prompts-task.js index 67741ae..8dd24ec 100644 --- a/src/prompts-task.js +++ b/src/prompts-task.js @@ -471,6 +471,9 @@ function fixMessagesLines() { 'with `chore: continue Task `.', "The plan's message applies to a task's first commit; a fix commit carries its fix message, and a", 'continuation commit its continuation message; none of them is a commit-rule finding.', + "The engine's own commits: join merges `chore: join wave : merge ` and", + '`chore: join recovery: merge ` (before 1.6.7 without `chore: `) and the wave checks fix', + `\`${waveChecksFixMessage()}\`; none of them is a commit-rule finding.`, ]; } diff --git a/tests/engine.test.mjs b/tests/engine.test.mjs index bfae1d7..480b853 100644 --- a/tests/engine.test.mjs +++ b/tests/engine.test.mjs @@ -1593,6 +1593,12 @@ test('an implement retry uses the fix message only when review findings are behi assert.ok(later(reviewed).includes(`${TASK_FIX_MESSAGE} T2`), reviewed); assert.ok(reviewPrompt(m, task('T2'), WHERE, H('b0'), H('h1')).includes('chore: continue Task '), 'reviewers are told the continuation message'); + // The engine's own commits: the join merges (both forms) and the wave checks fix. + const review = reviewPrompt(m, task('T2'), WHERE, H('b0'), H('h1')); + for (const part of ['`chore: join wave : merge `', '`chore: join recovery: merge `', + '`fix: make the wave checks pass`']) { + assert.ok(review.includes(part), `${part}: ${review}`); + } }); test('the final fix and the post-integrate fix use their own messages', async () => { diff --git a/tests/git-identity.test.mjs b/tests/git-identity.test.mjs index 2087464..b2fcd55 100644 --- a/tests/git-identity.test.mjs +++ b/tests/git-identity.test.mjs @@ -98,9 +98,9 @@ test('the resolving prompts spell out the commit with the identity', () => { test('the wave resolver merges with the identity', () => { const p = prompts(manifest(IDENTITY)); assert.ok(p['wave resolver'].includes( - `git -C ${DIR} ${ARGS}merge --no-ff -m 'join wave 1: merge J2' '${H('J2')}'`), p['wave resolver']); + `git -C ${DIR} ${ARGS}merge --no-ff -m 'chore: join wave 1: merge J2' '${H('J2')}'`), p['wave resolver']); assert.ok(p['recovery resolver'].includes( - `git -C ${DIR} ${ARGS}merge --no-ff -m 'join recovery: merge J2' '${H('J2')}'`), p['recovery resolver']); + `git -C ${DIR} ${ARGS}merge --no-ff -m 'chore: join recovery: merge J2' '${H('J2')}'`), p['recovery resolver']); }); test('the wave merge passes each pair to merge-wave as --git-config', () => { diff --git a/tests/join-waves-run.test.mjs b/tests/join-waves-run.test.mjs index 0b10fd3..b076165 100644 --- a/tests/join-waves-run.test.mjs +++ b/tests/join-waves-run.test.mjs @@ -6,8 +6,8 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { loadHelpers, loadScript, H } from './harness.mjs'; -const { waveResolvePrompt, waveChecksFixPrompt, waveChecksPrompt } = await loadHelpers([ - 'waveResolvePrompt', 'waveChecksFixPrompt', 'waveChecksPrompt', +const { waveResolvePrompt, waveChecksFixPrompt, waveChecksPrompt, finalReviewPrompt } = await loadHelpers([ + 'waveResolvePrompt', 'waveChecksFixPrompt', 'waveChecksPrompt', 'finalReviewPrompt', ]); function task(id, deps, extra = {}) { @@ -149,11 +149,89 @@ test('a supervised conflict stops with the sha to merge by hand', async () => { const why = result.stopped_lanes.find((s) => s.lane === 'join').reason; assert.ok(why.includes(H('J2')) && /by hand/.test(why) && why.includes('src/x.js'), why); const args = "-c 'user.name=Riley O'\\''Q' -c 'user.email=r@x.invalid' "; - assert.ok(why.includes(`git -C '/work/repo' ${args}merge --no-ff -m 'join wave 1: merge J2' ${H('J2')}`), + assert.ok(why.includes(`git -C '/work/repo' ${args}merge --no-ff -m 'chore: join wave 1: merge J2' ${H('J2')}`), `the exact command: ${why}`); assert.ok(why.includes(`git -C '/work/repo' ${args}commit --no-edit`), `the commit: ${why}`); }); +// ---- 1.6.7: a refused merge, a merge that left a unit out, a missing result ---- + +const refused = () => merged(H('I1'), { status: 'failed', merged: ['J1'], in_history: ['J1'], failed_unit: 'J2', + error: 'Committer identity unknown' }); +const joinReason = (result) => result.stopped_lanes.find((s) => s.lane === 'join').reason; + +test('a refused wave merge stops without a resolver', async () => { + const { result, calls } = await run(manifest(), script(H('J3'), { 'wave 1 merge': [refused()] })); + assert.equal(result.status, 'stopped'); + assert.ok(!labels(calls).some((l) => l.includes('resolve')), JSON.stringify(labels(calls))); + assert.ok(!labels(calls).includes('J3 implement')); + assert.equal(joinReason(result), 'join wave 1: the merge of J2 failed: Committer identity unknown'); + assert.equal(result.join_waves[0].head, H('I1')); + assert.deepEqual(result.join_waves[0].in_history, ['J1']); +}); + +test('a supervised refused wave merge stops the same way', async () => { + const { result, calls } = await run(manifest({ autonomy: 'supervised' }), script(H('J3'), { 'wave 1 merge': [refused()] })); + assert.equal(result.status, 'stopped'); + assert.ok(!labels(calls).some((l) => l.includes('resolve'))); + const why = joinReason(result); + assert.equal(why, 'join wave 1: the merge of J2 failed: Committer identity unknown'); + assert.ok(!why.includes('by hand'), why); +}); + +test('a refused merge without failed_unit or error stops with its notes', async () => { + const bare = merged(H('I1'), { status: 'failed', merged: ['J1'], notes: 'merge-wave exited 4' }); + const { result } = await run(manifest(), script(H('J3'), { 'wave 1 merge': [bare] })); + assert.equal(joinReason(result), 'join wave 1: the merge failed: merge-wave exited 4'); +}); + +test('a conflict with no files is a refusal', async () => { + const conflict = merged(H('I1'), { status: 'conflict', merged: ['J1'], conflict_unit: 'J2', conflict_files: [], + notes: 'git said no' }); + const { result, calls } = await run(manifest(), script(H('J3'), { 'wave 1 merge': [conflict] })); + assert.equal(result.status, 'stopped'); + assert.ok(!labels(calls).some((l) => l.includes('resolve'))); + assert.match(joinReason(result), /the merge (of J2 )?failed/); + const missing = merged(H('I1'), { status: 'conflict', merged: ['J1'], conflict_unit: 'J2', conflict_files: undefined, + notes: 'git said no' }); + const again = await run(manifest(), script(H('J3'), { 'wave 1 merge': [missing] })); + assert.ok(!labels(again.calls).some((l) => l.includes('resolve'))); + assert.match(joinReason(again.result), /the merge (of J2 )?failed/); +}); + +test('a done merge that left a unit out stops', async () => { + const { result, calls } = await run(manifest(), script(H('J3'), { 'wave 1 merge': [merged(H('M1'), { merged: ['J1'] })] })); + assert.equal(result.status, 'stopped'); + assert.ok(!labels(calls).includes('J3 implement')); + assert.equal(joinReason(result), 'join wave 1: the merge left out J2'); +}); + +test('a missing merge result reads as transient', async () => { + const { result } = await run(manifest(), script(H('J3'), { 'wave 1 merge': [null], 'wave 1 merge retry': [null] })); + assert.equal(result.status, 'stopped'); + assert.equal(joinReason(result), 'no result from wave 1 merge'); +}); + +test('the wave merge prompt maps exit 4', async () => { + const { calls } = await run(manifest(), script(H('J3'))); + const p = callOf(calls, 'wave 1 merge').prompt; + assert.ok(/exit(ed)? 4/.test(p), p); + assert.ok(p.includes('failed_unit'), p); + assert.ok(p.includes('do not change git config'), p); +}); + +test('the wave resolver records its merge', () => { + const p = waveResolvePrompt(manifest(), 1, { id: 'J2', head: H('J2') }, ['src/x.js']); + assert.ok(p.includes("'chore: join wave 1: merge J2'"), p); + for (const part of ['"event":"wave_merged"', '"wave":1', '"units":["J2"]', '"merged":["J2"]', + '"conflict_files":["src/x.js"]', '"resolved":true']) { + assert.ok(p.includes(part), `${part}: ${p}`); + } + const rec = waveResolvePrompt(manifest(), null, { id: 'J2', head: H('J2') }, ['src/x.js']); + assert.ok(rec.includes("'chore: join recovery: merge J2'"), rec); + assert.ok(rec.includes('"recovery":true') && !rec.includes('"wave":'), rec); +}); + const withChecks = (extra = {}) => manifest({ task_checks: { J1: { test: ['npm test -- j1'] }, J2: { test: ['npm test -- j2'] } }, ...extra, }); @@ -260,10 +338,31 @@ test('lenses are told overlaps and resolved conflicts', async () => { } }); +test('lenses tell resolved from unresolved conflicts', () => { + const overlapped = { wave: 1, units: ['J1'], merged: ['J1'], head: 'h2', overlaps: [{ path: 'src/s.js', units: ['J1', 'J2'] }], + conflict_files: [] }; + const events = [{ wave: 1, units: [], merged: [], head: 'h', overlaps: [], conflict_unit: 'J2', conflict_files: ['src/c.js'] }, + overlapped, { ...overlapped }]; + const count = (text, part) => text.split(part).length - 1; + const open = finalReviewPrompt(manifest({ wave_merges: events }), 'correctness', null); + assert.equal(count(open, "src/c.js, settled outside the run's resolver"), 1, open); + assert.equal(count(open, 'src/s.js (J1, J2)'), 1, open); + assert.ok(!open.includes('the wave resolver settled'), open); + const resolved = { wave: 1, units: ['J1', 'J2'], merged: ['J2'], head: 'h3', overlaps: [], conflict_files: ['src/c.js'], + resolved: true }; + const settled = finalReviewPrompt(manifest({ wave_merges: [...events, resolved] }), 'correctness', null); + assert.equal(count(settled, 'merge conflicts the wave resolver settled in src/c.js'), 1, settled); + assert.ok(!settled.includes('settled outside'), settled); + // An event written before 1.6.7 (no merged field) keeps the resolver text. + const old = finalReviewPrompt(manifest({ wave_merges: [{ wave: 1, units: ['J1', 'J2'], head: 'h', overlaps: [], + conflict_files: ['src/c.js'] }] }), 'correctness', null); + assert.ok(old.includes('merge conflicts the wave resolver settled in src/c.js') && !old.includes('settled outside'), old); +}); + test('waveResolvePrompt aborts on failure', () => { const p = waveResolvePrompt(manifest(), 1, { id: 'J2', head: H('J2') }, ['src/x.js']); assert.ok(p.includes("git -C '/work/repo' merge --abort"), p); - assert.ok(p.includes("'join wave 1: merge J2'"), p); + assert.ok(p.includes("'chore: join wave 1: merge J2'"), p); assert.ok(p.includes(H('J2')) && p.includes('src/x.js')); }); @@ -378,12 +477,13 @@ test('an autonomous recovery conflict goes to a resolver', async () => { const { result, calls } = await run(relaunch(), relaunchScript(H('J3'), { 'recovery merge': [ { status: 'conflict', head: H('I1'), merged: [], conflict_unit: 'J2', conflict_files: ['src/x.js'], overlaps: [], notes: '' }, - { status: 'done', head: H('R1'), merged: [], conflict_unit: '', conflict_files: [], overlaps: [], notes: '' }, + { status: 'done', head: H('R1'), merged: [], in_history: ['J2'], conflict_unit: '', conflict_files: [], overlaps: [], + notes: '' }, ], 'recovery resolve J2': [{ status: 'done', head: H('R1'), notes: 'kept both' }], })); assert.equal(result.status, 'complete', JSON.stringify(result.reason)); - assert.ok(callOf(calls, 'recovery resolve J2').prompt.includes("'join recovery: merge J2'")); + assert.ok(callOf(calls, 'recovery resolve J2').prompt.includes("'chore: join recovery: merge J2'")); }); test('a relaunch of a serial join keeps the serial path', async () => { @@ -413,8 +513,9 @@ test('the audit allowance uses join_tip', () => { backfill: { ...relaunch().backfill, J3: { base: H('R1'), head: H('J3') } }, join_tip: H('W9') }); const p = integratePrompt(m, H('pre'), {}); assert.ok(p.includes(`may follow the last join commit ${H('W9')}`), p); - assert.ok(p.includes("'join wave : merge '") && p.includes("'join recovery: merge '"), p); + assert.ok(p.includes("'chore: join wave : merge '") && p.includes("'chore: join recovery: merge '"), p); assert.ok(p.includes('fix: make the wave checks pass'), p); + assert.ok(p.includes("or the same without 'chore: ' from before 1.6.7"), p); }); // ---- Task 11: pre-flight sees the waves ---- From bc62606b001908785e52fcd01518adf147bc50d2 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:36:54 -0700 Subject: [PATCH 07/11] test: join waves merge end to end with the identity only in the commit rules --- tests/join-waves-e2e.test.mjs | 47 ++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/tests/join-waves-e2e.test.mjs b/tests/join-waves-e2e.test.mjs index 15ef9f7..7ee93c2 100644 --- a/tests/join-waves-e2e.test.mjs +++ b/tests/join-waves-e2e.test.mjs @@ -101,9 +101,20 @@ function run1(command) { return r.stdout; } +// The environment of a shell with no identity of its own: no GIT_AUTHOR_* or +// GIT_COMMITTER_* keys, an empty global config and no system config. +function bareGitEnv(c) { + const empty = `${c.root}/empty-gitconfig`; + writeFileSync(empty, ''); + const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => !/^GIT_(AUTHOR|COMMITTER)_/i.test(k))); + return mergeEnv(env, { GIT_CONFIG_GLOBAL: empty, GIT_CONFIG_NOSYSTEM: '1' }); +} + // Scripted agents. stopAfterCommit: unit ids whose implementer commits, // records it, and then reports blocked (with an uncommitted change left). -function agents(c, { stopAfterCommit = [] } = {}) { +// mergeEnv: the environment the wave merge stub runs merge-wave in (default: +// sh's, with GIT_ENV's identity). +function agents(c, { stopAfterCommit = [], mergeEnv: waveEnv = null } = {}) { const calls = []; const featureHead = () => git(c.repo, 'rev-parse', 'HEAD'); const agent = async (prompt, opts) => { @@ -119,10 +130,15 @@ function agents(c, { stopAfterCommit = [] } = {}) { return { status: 'done', head: featureHead(), notes: 'merged' }; } if (/^(wave \d+|recovery) merge$/.test(label)) { - const r = sh(lineAfter(prompt, /^1\. Run, as one call:/)); - assert.ok(r.code === 0 || r.code === 1, r.stderr); + const command = lineAfter(prompt, /^1\. Run, as one call:/); + const r = waveEnv === null ? sh(command) + : (({ status, stdout, stderr }) => ({ code: status, stdout, stderr }))( + spawnSync(BASH, ['-c', command], { encoding: 'utf8', env: waveEnv })); + assert.ok(r.code === 0 || r.code === 1 || r.code === 4, `exit ${r.code}: ${r.stderr}`); const out = JSON.parse(r.stdout); - return { ...out, status: out.status === 'merged' ? 'done' : 'conflict', conflict_unit: out.conflict_unit || '' }; + const status = { merged: 'done', conflict: 'conflict', failed: 'failed' }[out.status]; + assert.ok(status, `merge-wave status ${out.status}`); + return { ...out, status, conflict_unit: out.conflict_unit || '', failed_unit: out.failed_unit || '', error: out.error || '' }; } if (label.startsWith('final review')) return { findings: [], cannot_verify: [], head: featureHead() }; if (role === 'implement') { @@ -180,6 +196,29 @@ test('a join wave runs on unit branches in lane worktrees and is merged', async assert.deepEqual(result.join_waves.map((w) => w.units), [['J1', 'J2']]); }); +test('a join wave merges with the identity only in the commit rules', async () => { + const c = newCase(); + c.m.commit_rules = 'plain ASCII. Commit with git -c "user.name=Lane Bot" -c user.email=lane@x.invalid commit.'; + // Git may not guess an identity; setup and the scripted implement, review + // and integrate agents keep GIT_ENV's, standing in for agents that commit + // per the commit rules. The wave merge runs with no identity of its own. + git(c.repo, 'config', 'user.useConfigOnly', 'true'); + const setupResult = setup(c, c.m); + assert.deepEqual(setupResult.git_identity, ['user.name=Lane Bot', 'user.email=lane@x.invalid']); + const s = agents(c, { mergeEnv: bareGitEnv(c) }); + const result = await runAll({ ...c.m, setup_result: setupResult }, s); + assert.equal(result.status, 'complete', JSON.stringify(result.reason || result.errors)); + const labels = s.calls.map((x) => x.label); + assert.equal(labels.filter((l) => l === 'wave 1 merge').length, 1, labels.join('\n')); + assert.ok(!labels.some((l) => / resolve /.test(l)), labels.join('\n')); + const who = git(c.repo, 'log', '--format=%an <%ae>|%cn <%ce>', '-1', '--grep', 'chore: join wave 1: merge J2'); + assert.equal(who, 'Lane Bot |Lane Bot '); + const events = readFileSync(`${c.m.repo.ledger_dir}/_run.jsonl`, 'utf8').trim().split('\n').map((l) => JSON.parse(l)); + const merges = events.filter((e) => e.event === 'wave_merged' && e.wave === 1); + assert.equal(merges.length, 1, JSON.stringify(merges)); + assert.deepEqual(merges[0].merged, ['J1', 'J2']); +}); + test('a stop mid-wave relaunches through setup, recovery and the serial continuation', async () => { const c = newCase(); const first = await runAll({ ...c.m, setup_result: setup(c, c.m) }, agents(c, { stopAfterCommit: ['J2'] })); From 784c38b6b3289361f0ace5aff418a2ab635d7325 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:53:10 -0700 Subject: [PATCH 08/11] fix: address the final review findings --- SKILL.md | 9 ++++----- reference.md | 30 +++++++++++++++--------------- tests/join-waves-e2e.test.mjs | 2 +- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/SKILL.md b/SKILL.md index de22b08..8b6911a 100644 --- a/SKILL.md +++ b/SKILL.md @@ -256,11 +256,10 @@ earlier snapshot belongs to the earlier launch. Relaunch once with the relaunch notice: carry budgets over (reference.md "Transient stops"). Set `repairs_spent` to the repairs in the stopped run's `final.repairs` plus the earlier value. First recompute `done`, `reviewed`, `deferred`, the `carry` notes, `carried_minors`, -`inputs_changed`, the join wave fields and `backfill` as in Resume -steps 2-3, then rerun `scripts/setup --owner `, put its output in `setup_result` and take -`start_points`; if validation -rejects its `setup_skipped`, rerun setup. Launch again. No second table. A second transient -stop is a real stop, unless it is a usage-limit stop (reference.md "Usage limit"). +`inputs_changed`, the join wave fields and `backfill` as in Resume steps 2-3, then rerun +`scripts/setup --owner `, put its output in `setup_result` and take `start_points`; if +validation rejects its `setup_skipped`, rerun setup. Launch again. No second table. A second +transient stop is a real stop, unless it is a usage-limit stop (reference.md "Usage limit"). ## Notify diff --git a/reference.md b/reference.md index 74b5c49..c9cf890 100644 --- a/reference.md +++ b/reference.md @@ -152,12 +152,12 @@ pairs as `setup_result.git_identity`: a list of at most two strings, `user.name= then `user.email=`, keys lower case, the last pair given for a key winning (as with git's own `-c`), `[]` when the commit rules name none. It is recorded whether or not a global identity exists, so the run's merges commit as the agents' own commits do. The run passes the pairs to -every merge and every commit it spells out itself: the integrate prompt's lane merges, the -resolved-merge commits (`git -C -c ... commit --no-edit`) of the integrate prompt, the -lane resolver and the wave resolver, the wave resolver's merge, the supervised conflict stop's -by-hand commands, and the wave merge (merge-wave's `--git-config`, one per pair). Each pair is -shell-quoted in a prompt and one argv item in merge-wave. With none, these use the user's git -config, as before 1.6.7. Agents' own commits keep following `commit_rules`. Only name and email +every merge and every commit it spells out itself: the integrate prompt's and the lane +resolver's lane merges, the resolved-merge commits (`git -C -c ... commit --no-edit`) of +the integrate prompt, the lane resolver and the wave resolver, the wave resolver's merge, the +supervised conflict stop's by-hand commands, and the wave merge (merge-wave's `--git-config`, +one per pair). Each pair is shell-quoted in a prompt and one argv item in merge-wave. With +none, these use the user's git config, as before 1.6.7. Agents' own commits keep following `commit_rules`. Only name and email are carried: other `-c` options in the commit rules (a signing key, an editor) do not reach the scripted merges. A manifest whose setup ran under 1.6.6 has no `git_identity` (read as `[]`); setup reruns on a resume or relaunch and adds it. @@ -207,9 +207,9 @@ SKILL.md gives the rule; this is how it reads the causes. Not transient, so neve unit or error; `` is `wave ` or `recovery`) or a merge that reported done but left a unit out (`join : the merge left out `). A refused merge needs the user (an identity, a hook, a held lock), then Resume. A missing wave or recovery merge result reads exactly `no -result from merge`, so it is transient like any other missing result. The causes are every `stopped_lanes[].reason` when `reason` is `prelude -stopped`, `lanes stopped`, `join stopped`, or `agents_unavailable`, else `reason` itself. A -cause is transient only if it starts with `no result from`, `error:`, or `setup failed`, or +result from merge`, so it is transient like any other missing result. The causes are +every `stopped_lanes[].reason` when `reason` is `prelude stopped`, `lanes stopped`, `join +stopped`, or `agents_unavailable`, else `reason` itself. A cause is transient only if it starts with `no result from`, `error:`, or `setup failed`, or reads `integration failed: no result from ...` or `post-integrate failed: no result from ...`, and, under `agents_unavailable` only, if it starts with `agents unavailable:` (a lane whose agent the latch refused). So `agents_unavailable` is transient only when every other cause is @@ -404,12 +404,12 @@ worktrees, when no wave has two units, and when the join started in an earlier l supervised one stops. Checks that did not finish get no fix. - A relaunch with `join_started` and `unit_branches`: setup puts each lane worktree left on one of the run's unit branches back on its lane branch (output `returned`), saving its changes; a - recovery merge (`merge-wave --recovery`, `chore: join recovery: merge `) brings in every unit that - ran on its own branch and is done and reviewed or deferred, and reports (`--known`, - `in_history`) which of the other such units an earlier launch already merged; the rest of the - join runs serially on the feature checkout from that tip. A merged unit whose approval went - stale is reviewed again over its recorded range without moving the tip. A unit that ran on its - own branch, is not done and reviewed, and is not in the history starts again from the tip + recovery merge (`merge-wave --recovery`, `chore: join recovery: merge `) brings in + every unit that ran on its own branch and is done and reviewed or deferred, and reports + (`--known`, `in_history`) which of the other such units an earlier launch already merged; the + rest of the join runs serially on the feature checkout from that tip. A merged unit whose + approval went stale is reviewed again over its recorded range without moving the tip. A unit + that ran on its own branch, is not done and reviewed, and is not in the history starts again from the tip (`start-task --restart`: the ledger drops its earlier attempt), told its branch and abandoned refs to reuse. Any other unit starts from its latest `started` base (`start_bases`), else the tip. A unit branch an earlier launch left that is not built on the wave's base is saved under diff --git a/tests/join-waves-e2e.test.mjs b/tests/join-waves-e2e.test.mjs index 7ee93c2..cef3efb 100644 --- a/tests/join-waves-e2e.test.mjs +++ b/tests/join-waves-e2e.test.mjs @@ -210,7 +210,7 @@ test('a join wave merges with the identity only in the commit rules', async () = assert.equal(result.status, 'complete', JSON.stringify(result.reason || result.errors)); const labels = s.calls.map((x) => x.label); assert.equal(labels.filter((l) => l === 'wave 1 merge').length, 1, labels.join('\n')); - assert.ok(!labels.some((l) => / resolve /.test(l)), labels.join('\n')); + assert.ok(!labels.some((l) => l.includes('resolve')), labels.join('\n')); const who = git(c.repo, 'log', '--format=%an <%ae>|%cn <%ce>', '-1', '--grep', 'chore: join wave 1: merge J2'); assert.equal(who, 'Lane Bot |Lane Bot '); const events = readFileSync(`${c.m.repo.ledger_dir}/_run.jsonl`, 'utf8').trim().split('\n').map((l) => JSON.parse(l)); From 7c1ee901345049aeb7c1a377cd8fadba4c1092e7 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:21:34 -0700 Subject: [PATCH 09/11] test: prove the index.lock retry waits and tries again --- tests/merge-wave.test.mjs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/merge-wave.test.mjs b/tests/merge-wave.test.mjs index b5c723c..5fff6e2 100644 --- a/tests/merge-wave.test.mjs +++ b/tests/merge-wave.test.mjs @@ -331,11 +331,18 @@ test('a held index.lock is retried once', async () => { const a = unit(c, 'A', { 'a.txt': 'a\n' }); const lock = join(c.repo, '.git', 'index.lock'); writeFileSync(lock, ''); + // Held for 1.5 s, less than the 2 s retry wait: only a merge retried after + // that wait succeeds, so a passing call took at least 2 s. Without the + // retry the call fails while the lock is held, or (with a slow start) + // merges at once in under 2 s; either fails this test. + const started = Date.now(); const run = mergeWaveAsync(c, ['--wave', '1', ...a]); - setTimeout(() => rmSync(lock, { force: true }), 500); + setTimeout(() => rmSync(lock, { force: true }), 1500); const r = await run.done; + const elapsed = Date.now() - started; assert.equal(r.code, 0, r.stderr); assert.deepEqual(r.out.merged, ['A']); + assert.ok(elapsed >= 2000, `the merge was retried after the 2 s wait (took ${elapsed} ms)`); const held = newCase(); const h = unit(held, 'A', { 'a.txt': 'a\n' }); From 59599fb773b88778fa820a00a23f1a295a806b30 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:23:03 -0700 Subject: [PATCH 10/11] docs: rewrap the lines the 1.6.7 docs left long --- reference.md | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/reference.md b/reference.md index c9cf890..6421d74 100644 --- a/reference.md +++ b/reference.md @@ -157,10 +157,10 @@ resolver's lane merges, the resolved-merge commits (`git -C -c ... commit the integrate prompt, the lane resolver and the wave resolver, the wave resolver's merge, the supervised conflict stop's by-hand commands, and the wave merge (merge-wave's `--git-config`, one per pair). Each pair is shell-quoted in a prompt and one argv item in merge-wave. With -none, these use the user's git config, as before 1.6.7. Agents' own commits keep following `commit_rules`. Only name and email -are carried: other `-c` options in the commit rules (a signing key, an editor) do not reach the -scripted merges. A manifest whose setup ran under 1.6.6 has no `git_identity` (read as `[]`); -setup reruns on a resume or relaunch and adds it. +none, these use the user's git config, as before 1.6.7. Agents' own commits keep following +`commit_rules`. Only name and email are carried: other `-c` options in the commit rules (a +signing key, an editor) do not reach the scripted merges. A manifest whose setup ran under +1.6.6 has no `git_identity` (read as `[]`); setup reruns on a resume or relaunch and adds it. A repository with SHA-256 object names (`git rev-parse --show-object-format` prints `sha256`) is not supported: the run names commits by 40-hex SHA-1 ids. `scripts/setup` refuses it (exit 3, @@ -209,9 +209,10 @@ unit out (`join : the merge left out `). A refused merge needs the us a hook, a held lock), then Resume. A missing wave or recovery merge result reads exactly `no result from merge`, so it is transient like any other missing result. The causes are every `stopped_lanes[].reason` when `reason` is `prelude stopped`, `lanes stopped`, `join -stopped`, or `agents_unavailable`, else `reason` itself. A cause is transient only if it starts with `no result from`, `error:`, or `setup failed`, or -reads `integration failed: no result from ...` or `post-integrate failed: no result from ...`, -and, under `agents_unavailable` only, if it starts with `agents unavailable:` (a lane whose +stopped`, or `agents_unavailable`, else `reason` itself. A cause is transient only if it +starts with `no result from`, `error:`, or `setup failed`, or reads `integration failed: no +result from ...` or `post-integrate failed: no result from ...`, and, under +`agents_unavailable` only, if it starts with `agents unavailable:` (a lane whose agent the latch refused). So `agents_unavailable` is transient only when every other cause is transient too: a lane already stopped at `review_rounds`, a supervised question and the like keep their own cause, the stop is real, and Resume step 4 applies. A stop with @@ -409,12 +410,13 @@ worktrees, when no wave has two units, and when the join started in an earlier l (`--known`, `in_history`) which of the other such units an earlier launch already merged; the rest of the join runs serially on the feature checkout from that tip. A merged unit whose approval went stale is reviewed again over its recorded range without moving the tip. A unit - that ran on its own branch, is not done and reviewed, and is not in the history starts again from the tip - (`start-task --restart`: the ledger drops its earlier attempt), told its branch and abandoned - refs to reuse. Any other unit starts from its latest `started` base (`start_bases`), else the - tip. A unit branch an earlier launch left that is not built on the wave's base is saved under - the abandoned refs and made again (`start-task` prints `recreated`). Integration's history audit allows the - merge, resolver and wave checks fix commits, and final-fix commits after `join_tip`. + that ran on its own branch, is not done and reviewed, and is not in the history starts again + from the tip (`start-task --restart`: the ledger drops its earlier attempt), told its branch + and abandoned refs to reuse. Any other unit starts from its latest `started` base + (`start_bases`), else the tip. A unit branch an earlier launch left that is not built on the + wave's base is saved under the abandoned refs and made again (`start-task` prints + `recreated`). Integration's history audit allows the merge, resolver and wave checks fix + commits, and final-fix commits after `join_tip`. - `run-checks --phase NAME` (and `task-checks --phase`) sets `PL_PHASE=NAME` (prelude, lane, join, integrate, post-integrate, final-fix, verify) for every check; without it `PL_PHASE` is removed. A project's check script may read it: a join unit runs in a lane worktree. Task prompts From 64330e612f6db924cb04441ea87cb8e405393208 Mon Sep 17 00:00:00 2001 From: noderaven <130021665+noderaven@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:23:06 -0700 Subject: [PATCH 11/11] chore: release 1.6.7 --- VERSION | 2 +- run.workflow.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index ec70f75..400084b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.6.6 +1.6.7 diff --git a/run.workflow.js b/run.workflow.js index 3ad05e7..6bbdbfe 100644 --- a/run.workflow.js +++ b/run.workflow.js @@ -1,6 +1,6 @@ export const meta = { name: 'parallel-lanes', - description: 'parallel-lanes v1.6.6: Execute an implementation plan as parallel lanes of tasks with per-task review, then integrate.', + description: 'parallel-lanes v1.6.7: Execute an implementation plan as parallel lanes of tasks with per-task review, then integrate.', phases: [ { title: 'Pre-flight', detail: 'plan and spec conflicts' }, { title: 'Prelude', detail: 'shared tasks on the feature branch' }, @@ -6277,7 +6277,7 @@ function noHead(r, label) { // The skill version this script was built from (scripts/build stamps VERSION // here and in the meta description): the version actually running. Logged // first and returned with every result, dry runs included. -const VERSION = '1.6.6'; +const VERSION = '1.6.7'; log(`parallel-lanes v${VERSION} loaded`); if (args !== null && typeof args === 'object' && args.dry_run === true) {