Skip to content

Commit 07ce845

Browse files
fix(tooling): merge 驱动不再绑定到「上一个装过依赖的 worktree」,并补上悬空判红的断言 (#4868) (#4908)
`setup-git-hooks.mjs` 把绝对路径 `${REPO_ROOT}/scripts/git-merge-regen.mjs` 写进 `.git/config`。linked worktree 共用同一份 config,于是每次 `pnpm install` 都把全容器 的驱动改指向刚装完的那个 worktree;而 AGENTS.md 要求收尾时 `git worktree remove` —— 遵守这条纪律恰恰就是触发缺陷的动作。该 worktree 一删,所有 agent 凡碰到 `merge=os-regen` 映射文件的 merge 全部 MODULE_NOT_FOUND。路径已漂过四个 worktree。 改为 `node "$(git rev-parse --show-toplevel)/scripts/git-merge-regen.mjs" %O %A %B %P`: git 把 merge 驱动交给 shell 执行,命令替换在每次调用时、在正在被合并的那个工作树里 求值 —— 既不绑定任何具体 worktree,又仍能解析到当前工作树根(绝对路径当初正是为了 后者)。既有 clone 下次 `pnpm install` 自愈。 自检此前照不出这个缺陷:每条既有 `--self-test` 检查都自建临时仓库、注册自己的驱动, 所以真实 config 悬空时它们全绿。新增 `registeredDriverResolves()` 读**实时** config, 在脚本不存在、指向本工作树以外(缺陷咬人前一步)、或与注册值漂移时判红。注册方与 校验方现在共读 `regen-artifacts.mjs` 的同一份 `GIT_SETTINGS` 声明。 Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 50c2f64 commit 07ce845

4 files changed

Lines changed: 221 additions & 15 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
---
3+
4+
Tooling-only: `merge.os-regen.driver` is registered as a worktree-independent
5+
command, and `check:merge-driver` now asserts the registered driver actually
6+
resolves (#4868). Releases nothing.
7+
8+
`setup-git-hooks.mjs` baked an absolute `${REPO_ROOT}/scripts/git-merge-regen.mjs`
9+
into `.git/config`. Linked worktrees SHARE one `.git/config`, so every
10+
`pnpm install` re-pointed the container-wide driver at whichever worktree had just
11+
installed — and the moment that worktree was removed, which AGENTS.md *requires*
12+
on task cleanup, every merge touching a `merge=os-regen` path in every other
13+
worktree died with `MODULE_NOT_FOUND`. Following the cleanup rule is what
14+
triggered the breakage, which is why it recurred across four worktrees.
15+
16+
The value is now `node "$(git rev-parse --show-toplevel)/scripts/git-merge-regen.mjs" %O %A %B %P`.
17+
Git hands a merge driver to a shell, so the substitution runs per invocation
18+
inside the worktree being merged: it binds to no worktree yet still resolves to
19+
the right root — the property the absolute path was there to guarantee. Existing
20+
clones self-heal on the next `pnpm install`.
21+
22+
The gate could not see any of this, because it never looked: every existing
23+
`--self-test` check builds its own temp repo and registers its own driver, so all
24+
of them stayed green while the live config dangled. A new `registeredDriverResolves()`
25+
check reads the *live* config and fails when the script does not exist, when it
26+
points outside the current worktree (the same bug one step before it bites), or
27+
when the value has drifted from what the registrar writes. The registrar and the
28+
gate now read one declaration, `GIT_SETTINGS` in `regen-artifacts.mjs`.

scripts/git-merge-regen.mjs

Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,19 @@
5555
*/
5656

5757
import { execFileSync } from 'node:child_process';
58-
import { appendFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
58+
import { appendFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
5959
import { tmpdir } from 'node:os';
60-
import { dirname, join, resolve } from 'node:path';
60+
import { dirname, join, relative, resolve } from 'node:path';
6161
import { fileURLToPath } from 'node:url';
6262

63-
import { NOT_DRIVER_MANAGED, PENDING_MARKER, REGEN_ARTIFACTS, entryForPath } from './regen-artifacts.mjs';
63+
import {
64+
DRIVER_NAME,
65+
GIT_SETTINGS,
66+
NOT_DRIVER_MANAGED,
67+
PENDING_MARKER,
68+
REGEN_ARTIFACTS,
69+
entryForPath,
70+
} from './regen-artifacts.mjs';
6471

6572
const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..');
6673

@@ -193,6 +200,117 @@ function hookIsExecutable() {
193200
}
194201
}
195202

203+
/**
204+
* The driver registered in THIS clone must resolve — here, now (#4868).
205+
*
206+
* Every other check in this self-test builds a throwaway repo and registers its own
207+
* driver into it, so all of them stayed green for weeks while the real
208+
* `merge.os-regen.driver` in the shared `.git/config` pointed at a DELETED worktree
209+
* and every real merge of a `merge=os-regen` path died with MODULE_NOT_FOUND. The
210+
* self-test and the live merge path were simply not the same path. This check reads
211+
* the live one, which is the only reason it can catch that class of failure.
212+
*
213+
* It fails in three distinguishable ways, all of which have happened or are one
214+
* `pnpm install` away:
215+
* - the script the value names does not exist (the dangling-worktree bug);
216+
* - it exists but lives outside this worktree (bound to someone else's worktree —
217+
* green for whoever installed last, broken for everyone else, so this is the
218+
* check that catches the bug *before* the other worktree is removed);
219+
* - the value has drifted from what `setup-git-hooks.mjs` registers.
220+
*/
221+
function registeredDriverResolves() {
222+
const { key, value: expected } = GIT_SETTINGS.find((s) => s.key === `merge.${DRIVER_NAME}.driver`);
223+
224+
let actual = '';
225+
try {
226+
actual = execFileSync('git', ['config', '--get', key], {
227+
cwd: REPO_ROOT,
228+
encoding: 'utf8',
229+
stdio: ['ignore', 'pipe', 'ignore'],
230+
}).trim();
231+
} catch {
232+
actual = ''; // unset — `git config --get` exits 1
233+
}
234+
235+
if (!actual) {
236+
// A supported state, not a failure: git falls back to a text merge, which is
237+
// exactly the pre-#4675 behaviour. `pnpm install` registers it.
238+
console.log(`✓ ${key} is unregistered — merges text-merge as they did before #4675`);
239+
return true;
240+
}
241+
242+
const expansion = expandDriverScript(actual);
243+
if (expansion.skip) {
244+
console.log(`✓ ${key} not checked for resolution (${expansion.skip})`);
245+
return true;
246+
}
247+
if (expansion.error) return fail(`could not expand ${key} ("${actual}"): ${expansion.error}`);
248+
249+
const script = expansion.path;
250+
if (!existsSync(script)) {
251+
return fail(`${key} names a script that does not exist:\n`
252+
+ ` ${script}\n`
253+
+ ` Registered value: ${actual}\n`
254+
+ ' Every merge touching a merge=os-regen path in this clone dies with MODULE_NOT_FOUND,\n'
255+
+ ' and git leaves the path CONFLICTED with ours in it and no conflict markers.\n'
256+
+ ' Fix: pnpm install (re-registers the driver for this worktree)');
257+
}
258+
259+
const root = realpath(REPO_ROOT);
260+
if (relative(root, realpath(script)).startsWith('..')) {
261+
return fail(`${key} points OUTSIDE this worktree:\n`
262+
+ ` ${script}\n`
263+
+ ` Linked worktrees share one .git/config, so this is bound to another worktree and\n`
264+
+ ' breaks for everyone the moment that one is removed.\n'
265+
+ ' Fix: pnpm install (re-registers the driver for this worktree)');
266+
}
267+
268+
if (actual !== expected) {
269+
return fail(`${key} has drifted from what setup-git-hooks.mjs registers.\n`
270+
+ ` registered: ${actual}\n`
271+
+ ` expected: ${expected}\n`
272+
+ ' Fix: pnpm install');
273+
}
274+
275+
console.log(`✓ merge.${DRIVER_NAME}.driver resolves in THIS worktree (${relative(root, realpath(script))})`);
276+
return true;
277+
}
278+
279+
/**
280+
* Expand the driver value's script path the way git will: git hands a merge driver
281+
* command to a shell, so `$(git rev-parse --show-toplevel)` is only meaningful once
282+
* a shell has run it, from inside the worktree being merged.
283+
*/
284+
function expandDriverScript(value) {
285+
// Drop the trailing %O %A %B %P placeholders; what remains is `node <script>`.
286+
const command = value.replace(/(\s+%[A-Za-z])+\s*$/, '');
287+
const expr = /^\s*node\s+(\S.*)$/.exec(command)?.[1];
288+
if (!expr) return { skip: `not a \`node <script>\` command: "${value}"` };
289+
try {
290+
return {
291+
path: execFileSync('sh', ['-c', `printf '%s' ${expr}`], {
292+
cwd: REPO_ROOT,
293+
encoding: 'utf8',
294+
stdio: ['ignore', 'pipe', 'pipe'],
295+
}).trim(),
296+
};
297+
} catch (err) {
298+
// No POSIX shell (some Windows setups). Git could not run the driver either,
299+
// so there is nothing this check could assert that would still be true.
300+
if (err?.code === 'ENOENT') return { skip: 'no POSIX shell available to expand it' };
301+
return { error: err?.stderr?.toString().trim() || err?.message || String(err) };
302+
}
303+
}
304+
305+
/** Best-effort realpath: symlinked checkouts otherwise read as "outside the worktree". */
306+
function realpath(p) {
307+
try {
308+
return realpathSync(p);
309+
} catch {
310+
return p;
311+
}
312+
}
313+
196314
/**
197315
* Prove the driver end to end against real git: a conflicting change on both
198316
* sides of a mapped path must come out resolved, marker-free, and recorded.
@@ -207,7 +325,12 @@ function endToEnd() {
207325
git('config', 'user.email', 'selftest@objectstack.ai');
208326
git('config', 'user.name', 'self-test');
209327
git('config', 'merge.os-regen.name', 'regenerate instead of text-merging');
210-
git('config', 'merge.os-regen.driver', `node ${join(REPO_ROOT, 'scripts/git-merge-regen.mjs')} %O %A %B %P`);
328+
// Absolute on purpose, and NOT the value we register in a real clone: this temp
329+
// repo is not the ObjectStack worktree, so the registered
330+
// `$(git rev-parse --show-toplevel)` would resolve to `dir` — which has no
331+
// scripts/. Here we want the driver under test, i.e. this clone's copy.
332+
// Checking the value real clones get is `registeredDriverResolves()`'s job (#4868).
333+
git('config', 'merge.os-regen.driver', `node "${join(REPO_ROOT, 'scripts/git-merge-regen.mjs')}" %O %A %B %P`);
211334

212335
const target = REGEN_ARTIFACTS[0].path;
213336
mkdirSync(join(dir, dirname(target)), { recursive: true });
@@ -245,7 +368,13 @@ function endToEnd() {
245368

246369
if (process.argv.includes('--self-test')) {
247370
console.log('git-merge-regen --self-test\n');
248-
const results = [reconcileAttributes(), reconcileScripts(), hookIsExecutable(), endToEnd()];
371+
const results = [
372+
reconcileAttributes(),
373+
reconcileScripts(),
374+
hookIsExecutable(),
375+
registeredDriverResolves(),
376+
endToEnd(),
377+
];
249378
console.log(
250379
results.every(Boolean)
251380
? `\n✓ merge driver wiring is consistent (${NOT_DRIVER_MANAGED.length} path(s) deliberately excluded).`

scripts/regen-artifacts.mjs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,33 @@ export const PENDING_MARKER = 'os-regen-pending';
8989
/** The git config key pair that registers the driver in a clone. */
9090
export const DRIVER_NAME = 'os-regen';
9191

92+
/**
93+
* The script git runs as the merge driver, as a shell word (#4868).
94+
*
95+
* Resolved at MERGE time against the worktree being merged, never at install time
96+
* against the worktree that happened to run `pnpm install`. Linked worktrees share
97+
* one `.git/config`, so an absolute path here is a container-wide setting written
98+
* by whoever installed last — and it dangles the moment that worktree is removed,
99+
* which AGENTS.md requires on task cleanup. See `setup-git-hooks.mjs` for the two
100+
* constraints this spelling satisfies and the two traps it avoids.
101+
*/
102+
export const DRIVER_SCRIPT_EXPR = '"$(git rev-parse --show-toplevel)/scripts/git-merge-regen.mjs"';
103+
104+
/**
105+
* Every git config setting that `pnpm install` registers, declared once.
106+
*
107+
* `setup-git-hooks.mjs` writes these; `git-merge-regen.mjs --self-test` asserts the
108+
* live config still matches them and that the driver script actually resolves. One
109+
* declaration, so the registrar and the gate cannot drift apart.
110+
*/
111+
export const GIT_SETTINGS = Object.freeze([
112+
{ key: `merge.${DRIVER_NAME}.name`, value: 'regenerate generator-owned artifacts instead of text-merging' },
113+
// %O %A %B %P — ancestor, ours (the output file), theirs, pathname. Unquoted on
114+
// purpose: git generates %O %A %B as temp names and shell-quotes %P itself.
115+
{ key: `merge.${DRIVER_NAME}.driver`, value: `node ${DRIVER_SCRIPT_EXPR} %O %A %B %P` },
116+
{ key: 'core.hooksPath', value: '.githooks' },
117+
]);
118+
92119
/** Resolve the entry that owns a path, or undefined. Handles the one `**` entry. */
93120
export function entryForPath(p) {
94121
return REGEN_ARTIFACTS.find((e) =>

scripts/setup-git-hooks.mjs

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,43 @@
2626
*/
2727

2828
import { execFileSync } from 'node:child_process';
29-
import { dirname, join, resolve } from 'node:path';
29+
import { dirname, resolve } from 'node:path';
3030
import { fileURLToPath } from 'node:url';
3131

32-
import { DRIVER_NAME } from './regen-artifacts.mjs';
32+
import { GIT_SETTINGS } from './regen-artifacts.mjs';
3333

3434
const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..');
3535

36-
const SETTINGS = [
37-
{ key: `merge.${DRIVER_NAME}.name`, value: 'regenerate generator-owned artifacts instead of text-merging' },
38-
// %O %A %B %P — ancestor, ours (the output file), theirs, pathname. `node` and
39-
// a repo-relative path keep this working on Windows and in linked worktrees,
40-
// where a bare `./scripts/...` would resolve against the wrong root.
41-
{ key: `merge.${DRIVER_NAME}.driver`, value: `node "${join(REPO_ROOT, 'scripts/git-merge-regen.mjs')}" %O %A %B %P` },
42-
{ key: 'core.hooksPath', value: '.githooks' },
43-
];
36+
// What gets registered lives in `regen-artifacts.mjs` — one declaration, read both
37+
// by this registrar and by the `--self-test` gate that verifies it (#4868).
38+
//
39+
// The driver value is deliberately NOT an absolute path any more. It must satisfy
40+
// two constraints at once, and the obvious spellings each satisfy only one:
41+
//
42+
// - It must not bind to one specific worktree. Baking an absolute
43+
// `${REPO_ROOT}/scripts/...` in here did exactly that: linked worktrees SHARE
44+
// one `.git/config`, so every `pnpm install` re-pointed the container-wide
45+
// driver at the installing worktree, and the moment that worktree was removed
46+
// — which AGENTS.md *requires* on task cleanup — every merge of a
47+
// `merge=os-regen` path in every other worktree died with MODULE_NOT_FOUND.
48+
// Observed drifting across four worktrees before anyone noticed.
49+
// - It must still resolve to the right root. A bare `./scripts/...` relies on
50+
// git's (undocumented) choice of cwd for merge drivers, which is what the
51+
// absolute path was originally there to avoid.
52+
//
53+
// `$(git rev-parse --show-toplevel)` satisfies both: git runs merge drivers
54+
// through a shell, so the substitution happens per invocation, inside the worktree
55+
// being merged. Verified in git 2.43 from a linked worktree, invoked from both the
56+
// worktree root and a subdirectory.
57+
//
58+
// Two traps, both verified empirically rather than assumed:
59+
// - NO leading `!`. That prefix is alias/credential-helper syntax; a merge driver
60+
// value is already handed to the shell verbatim, so `!node ...` runs a program
61+
// literally named `!node` — "not found", and git falls back to a text merge.
62+
// - The placeholders stay UNQUOTED. git substitutes %O %A %B as generated temp
63+
// names and already shell-quotes %P itself; wrapping them in quotes of our own
64+
// hands the driver a pathname with literal quote characters in it.
65+
const SETTINGS = GIT_SETTINGS;
4466

4567
function git(args, opts = {}) {
4668
return execFileSync('git', args, { cwd: REPO_ROOT, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], ...opts });

0 commit comments

Comments
 (0)