fix(execute): the park step parks intent-to-add and coloured-diff files from any directory and exits 2 on a git failure
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
773aa44dc9
commit
7869c4cbae
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
5 changed files with 486 additions and 15 deletions
|
|
@ -1390,3 +1390,50 @@ test('[ORD] sibling: agents/review-coordinator.md — Pass 1 groups in two passe
|
|||
assert.match(pass1, /Group in two passes, so the order the findings arrive in never changes a row: first every finding joins the row of its title/,
|
||||
'Pass 1 does not name the two passes, title first');
|
||||
});
|
||||
|
||||
// --- F4b sibling: the writer of step-{N}-upward.diff ------------------------
|
||||
// The park tool reads the diff "Back to planning" saves; with color.diff=always
|
||||
// or core.quotepath on in the user's config, its headers were coloured or
|
||||
// quoted. The save line pins both off, on the same line (no line_ref move).
|
||||
|
||||
const PARK_WRITER = 'git -c core.quotepath=off diff --no-ext-diff --no-color --no-renames --src-prefix=a/ --dst-prefix=b/ {start_sha}';
|
||||
|
||||
test('[F4b] sibling: commands/trekexecute.md "Back to planning" saves the diff with core.quotepath=off and --no-color', () => {
|
||||
const lines = read(EXEC).split('\n').filter((l) => l.startsWith('**Back to planning** (`fix_rounds = 4`): save `'));
|
||||
assert.equal(lines.length, 1, `expected one "Back to planning" save line; found ${lines.length}`);
|
||||
assert.ok(lines[0].includes(`save \`${PARK_WRITER}\` (one form whatever the user's git config) to`),
|
||||
`the upward diff is not saved as ${PARK_WRITER}: ${lines[0]}`);
|
||||
});
|
||||
|
||||
// --- e2e script: tests/e2e-fix-round-4-5a.sh --------------------------------
|
||||
// The round run end to end (the plan's End-to-end:). Static pins only: the
|
||||
// script itself is the End-to-end: command.
|
||||
|
||||
const E2E_ROUND = 'tests/e2e-fix-round-4-5a.sh';
|
||||
|
||||
test('e2e script: tests/e2e-fix-round-4-5a.sh exists, is ASCII only and passes /bin/bash -n', () => {
|
||||
assert.ok(existsSync(join(ROOT, E2E_ROUND)), `missing file ${E2E_ROUND}`);
|
||||
const buf = readFileSync(join(ROOT, E2E_ROUND));
|
||||
assert.equal(buf.findIndex((b) => b > 127), -1, 'the e2e script is not ASCII only');
|
||||
const r = spawnSync('/bin/bash', ['-n', join(ROOT, E2E_ROUND)], { encoding: 'utf8', env: childEnv() });
|
||||
assert.equal(r.status, 0, r.stderr);
|
||||
const text = buf.toString('utf8');
|
||||
assert.match(text, /e2e: ok/, 'the script does not print e2e: ok');
|
||||
assert.match(text, /^set -u$/m, 'the script does not set -u');
|
||||
for (const bash4 of [/\bdeclare -A\b/, /\bmapfile\b/, /\breadarray\b/, /\|&/]) {
|
||||
assert.doesNotMatch(text, bash4, `the script uses ${bash4}, which bash 3.2 lacks`);
|
||||
}
|
||||
});
|
||||
|
||||
test('e2e script: it makes its temp directory with mktemp -d and an EXIT trap removes it', () => {
|
||||
const text = read(E2E_ROUND);
|
||||
assert.match(text, /^TMP="\$\(mktemp -d [^\n]*\)"/m, 'the script does not make $TMP with mktemp -d');
|
||||
assert.match(text, /^trap 'rm -rf "\$TMP"' EXIT$/m, 'no EXIT trap removes $TMP');
|
||||
});
|
||||
|
||||
test('e2e script: it names each of the four CLIs it runs', () => {
|
||||
const text = read(E2E_ROUND);
|
||||
for (const cli of ['lib/util/park-attempt-files.mjs', 'lib/review/verdict-gate.mjs', 'lib/parsers/arg-parser.mjs', 'lib/util/autonomy-gate.mjs']) {
|
||||
assert.ok(text.includes(cli), `the e2e script does not run ${cli}`);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -100,6 +100,9 @@ test('park: a new file that is tracked now, or already gone, is kept and named,
|
|||
assert.match(out.kept[0].reason, /tracked/);
|
||||
assert.match(out.kept[1].reason, /not on disk/);
|
||||
assert.equal(existsSync(join(dir, 'tests/batch.sh')), true);
|
||||
const inIndex = spawnSync('git', ['ls-files', '--error-unmatch', '--', 'tests/batch.sh'], { cwd: dir, encoding: 'utf8' });
|
||||
assert.equal(inIndex.status, 0, `the committed file left the index: ${inIndex.stderr}`);
|
||||
assert.equal(git(dir, 'status', '--porcelain', '--', 'tests/batch.sh'), '', 'the committed file is no longer clean in the index');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
|
|
@ -194,3 +197,170 @@ test('trekexecute: a re-planned step parks the earlier attempt\'s new files befo
|
|||
assert.match(para, /never deleted/);
|
||||
assert.match(para, /Exit 1 means a file the attempt made is still in the tree/);
|
||||
});
|
||||
|
||||
// --- fix round 4-5a: F4a, F4b, F4c ---------------------------------------------
|
||||
// [F4a] `git diff {start_sha}` names the attempt's new files because the helper
|
||||
// registered them --intent-to-add, so they are in the index: the tool called
|
||||
// them "tracked since the attempt" and left them in the tree with exit 0.
|
||||
// [F4b] A coloured diff (color.diff=always) hid every header; a diff with no
|
||||
// readable header at all parked nothing with exit 0.
|
||||
// [F4c] The root was the --cwd as given, so a subdirectory read every
|
||||
// root-relative path as absent; a git call that failed read as "nothing tracked".
|
||||
|
||||
/** The CLI run with `--cwd <cwd>` on `diff` written into the repo's git dir. */
|
||||
function runFrom(dir, cwd, diff = DIFF) {
|
||||
writeFileSync(join(dir, '.git', 'upward.diff'), diff);
|
||||
return spawnSync(process.execPath, [CLI, '--diff', join(dir, '.git', 'upward.diff'), '--step', '1', '--attempt', '1', '--cwd', cwd], { encoding: 'utf8' });
|
||||
}
|
||||
|
||||
/** The attempt's two new files, written on disk. */
|
||||
function attemptFiles(dir) {
|
||||
mkdirSync(join(dir, 'tests'));
|
||||
writeFileSync(join(dir, 'tests/batch.sh'), 'stale | cat\n');
|
||||
writeFileSync(join(dir, 'tests/cli-stdin.test.mjs'), 'stale test\n');
|
||||
}
|
||||
|
||||
test('[F4a] a new file still registered --intent-to-add is parked: gone from the tree and the index, kept under the parked directory', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
attemptFiles(dir);
|
||||
git(dir, 'add', '--intent-to-add', '--', 'tests/batch.sh', 'tests/cli-stdin.test.mjs');
|
||||
assert.match(git(dir, 'ls-files'), /tests\/batch\.sh/, 'control: the file is in the index before the park');
|
||||
const r = run(dir);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.equal(existsSync(join(dir, 'tests/batch.sh')), false, 'the intent-to-add file is still in the work tree');
|
||||
assert.equal(existsSync(join(dir, 'tests/cli-stdin.test.mjs')), false, 'the intent-to-add test file is still in the work tree');
|
||||
assert.doesNotMatch(git(dir, 'ls-files'), /tests\//, 'the parked files are still in the index');
|
||||
assert.equal(readFileSync(parkedPath(dir, 'tests/batch.sh'), 'utf8'), 'stale | cat\n', 'the file is not under the parked directory');
|
||||
assert.equal(readFileSync(parkedPath(dir, 'tests/cli-stdin.test.mjs'), 'utf8'), 'stale test\n');
|
||||
assert.deepEqual(JSON.parse(r.stdout).parked, ['tests/batch.sh', 'tests/cli-stdin.test.mjs']);
|
||||
assert.equal(readFileSync(join(dir, 'bin/slug.mjs'), 'utf8'), 'old\n', 'a committed file is never touched');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('[F4b] a diff with \\x1b[1m...\\x1b[m around its headers parks the file', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
attemptFiles(dir);
|
||||
const coloured = DIFF.split('\n')
|
||||
.map((l) => (/^(diff --git |new file mode |index |--- |\+\+\+ )/.test(l) ? `\x1b[1m${l}\x1b[m` : l))
|
||||
.join('\n');
|
||||
assert.ok(coloured.includes('\x1b[1mdiff --git a/tests/batch.sh b/tests/batch.sh\x1b[m'), 'control: the headers are coloured');
|
||||
const r = run(dir, coloured);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.equal(existsSync(join(dir, 'tests/batch.sh')), false, 'a coloured header left the file in the tree');
|
||||
assert.equal(readFileSync(parkedPath(dir, 'tests/batch.sh'), 'utf8'), 'stale | cat\n', 'the file is not under the parked directory');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('[F4b] a non-empty diff with no readable diff --git header exits 1 with a blocking entry', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
attemptFiles(dir);
|
||||
const headerless = ['--- /dev/null', '+++ b/tests/batch.sh', '@@ -0,0 +1 @@', '+cat', ''].join('\n');
|
||||
const r = run(dir, headerless);
|
||||
assert.equal(r.status, 1, `a diff it could not read must stop the run: ${r.stdout}${r.stderr}`);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.equal(out.kept.filter((k) => k.blocking).length, 1, `expected one blocking entry: ${r.stdout}`);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('[F4c] --cwd <repo>/bin parks the root-relative path', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
attemptFiles(dir);
|
||||
const r = runFrom(dir, join(dir, 'bin'));
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
assert.ok(JSON.parse(r.stdout).parked.includes('tests/batch.sh'), `tests/batch.sh was not parked from a subdirectory: ${r.stdout}`);
|
||||
assert.equal(existsSync(join(dir, 'tests/batch.sh')), false, 'the file is still in the tree');
|
||||
assert.equal(readFileSync(parkedPath(dir, 'tests/batch.sh'), 'utf8'), 'stale | cat\n');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('[F4c] a corrupt .git/index (ls-files exits 128) exits 2 and moves nothing', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
attemptFiles(dir);
|
||||
writeFileSync(join(dir, '.git', 'index'), 'not an index\n');
|
||||
const ls = spawnSync('git', ['ls-files', '-z'], { cwd: dir, encoding: 'utf8' });
|
||||
assert.equal(ls.status, 128, `control: ls-files must fail on the corrupt index: ${ls.stderr}`);
|
||||
const r = run(dir);
|
||||
assert.equal(r.status, 2, `a failed git call must be exit 2, never a park: ${r.stdout}${r.stderr}`);
|
||||
assert.equal(existsSync(join(dir, 'tests/batch.sh')), true, 'a file was moved although git failed');
|
||||
assert.equal(existsSync(parkedPath(dir, 'tests/batch.sh')), false);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('park: an empty diff exits 0 and parks nothing', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
const r = run(dir, '');
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
const out = JSON.parse(r.stdout);
|
||||
assert.deepEqual(out.parked, []);
|
||||
assert.deepEqual(out.kept, []);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('[F4a] an intent-to-add file named like a glob is parked, and the committed file it would match stays in the index', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
mkdirSync(join(dir, 'tests'));
|
||||
mkdirSync(join(dir, 'app/s'), { recursive: true });
|
||||
writeFileSync(join(dir, 'tests/real.sh'), 'committed\n');
|
||||
writeFileSync(join(dir, 'app/s/page.tsx'), 'committed\n');
|
||||
git(dir, 'add', '--', 'tests/real.sh', 'app/s/page.tsx');
|
||||
git(dir, 'commit', '-q', '-m', 'operator');
|
||||
writeFileSync(join(dir, 'tests/*.sh'), 'stale glob\n');
|
||||
mkdirSync(join(dir, 'app/[s]'));
|
||||
writeFileSync(join(dir, 'app/[s]/page.tsx'), 'stale bracket\n');
|
||||
git(dir, '--literal-pathspecs', 'add', '--intent-to-add', '--', 'tests/*.sh', 'app/[s]/page.tsx');
|
||||
const diff = DIFF.replaceAll('tests/batch.sh', 'tests/*.sh').replaceAll('tests/cli-stdin.test.mjs', 'app/[s]/page.tsx');
|
||||
const r = run(dir, diff);
|
||||
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||
for (const committed of ['tests/real.sh', 'app/s/page.tsx']) {
|
||||
const inIndex = spawnSync('git', ['ls-files', '--error-unmatch', '--', committed], { cwd: dir, encoding: 'utf8' });
|
||||
assert.equal(inIndex.status, 0, `${committed} left the index: ${inIndex.stderr}`);
|
||||
assert.equal(git(dir, 'status', '--porcelain', '--', committed), '', `${committed} is no longer clean in the index`);
|
||||
}
|
||||
assert.equal(existsSync(join(dir, 'tests/*.sh')), false);
|
||||
assert.equal(existsSync(parkedPath(dir, 'tests/*.sh')), true, 'the glob-named file is not under the parked directory');
|
||||
assert.equal(existsSync(parkedPath(dir, 'app/[s]/page.tsx')), true, 'the bracket-named file is not under the parked directory');
|
||||
assert.equal(readFileSync(parkedPath(dir, 'tests/*.sh'), 'utf8'), 'stale glob\n', 'the glob-named file is not parked');
|
||||
assert.equal(readFileSync(parkedPath(dir, 'app/[s]/page.tsx'), 'utf8'), 'stale bracket\n', 'the bracket-named file is not parked');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('[F4a] one index entry that cannot be removed exits 2 with every entry still in the index and nothing parked', () => {
|
||||
const dir = repo();
|
||||
try {
|
||||
attemptFiles(dir);
|
||||
git(dir, 'add', '--intent-to-add', '--', 'tests/batch.sh');
|
||||
git(dir, 'add', '--', 'tests/cli-stdin.test.mjs');
|
||||
writeFileSync(join(dir, 'tests/cli-stdin.test.mjs'), 'edited after staging\n');
|
||||
const r = run(dir);
|
||||
assert.equal(r.status, 2, `a removal git refuses must be exit 2: ${r.stdout}${r.stderr}`);
|
||||
const listed = git(dir, 'ls-files').split('\n');
|
||||
assert.ok(listed.includes('tests/batch.sh'), 'the intent-to-add entry was taken out of the index although the run could not finish');
|
||||
assert.ok(listed.includes('tests/cli-stdin.test.mjs'), 'the staged entry was taken out of the index');
|
||||
assert.equal(existsSync(parkedPath(dir, 'tests/batch.sh')), false, 'a file was parked although the run could not finish');
|
||||
assert.equal(existsSync(parkedPath(dir, 'tests/cli-stdin.test.mjs')), false);
|
||||
assert.equal(existsSync(join(dir, 'tests/batch.sh')), true);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue