fix(trekexecute): the back-to-planning save line reads the whole diff from a subdirectory
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
afe56899a9
commit
9145d2bbf5
5 changed files with 64 additions and 3 deletions
|
|
@ -1886,7 +1886,7 @@ session's effort, Sub-step E).
|
||||||
| 4 | the plan | **After three failed fix rounds the question is the architecture, not a fourth fix.** The step goes back to planning (below). Sub-step B sends three answers here directly, with no fix round: a `"not-run"` caused by the `Verify:` command, a second `plan-unreadable` (the executor's own call still wrong after one correction), and a `not-red-at-base` with `tests_only: true` (a step of only tests). Stop class 4 |
|
| 4 | the plan | **After three failed fix rounds the question is the architecture, not a fourth fix.** The step goes back to planning (below). Sub-step B sends three answers here directly, with no fix round: a `"not-run"` caused by the `Verify:` command, a second `plan-unreadable` (the executor's own call still wrong after one correction), and a `not-red-at-base` with `tests_only: true` (a step of only tests). Stop class 4 |
|
||||||
| 5 | the operator | A step already sent to the plan (`fix_rounds` 4) that fails its control again after re-planning goes to the operator. The executor never builds this step again: on a later `--resume` it only runs the checks (B, D1, D2, D3, D4, E) on whatever the operator changed. If they fail again the step stays at 5 — there is no sixth round, `fix_rounds` never exceeds 5 — and the run stops again. Stop class 4 |
|
| 5 | the operator | A step already sent to the plan (`fix_rounds` 4) that fails its control again after re-planning goes to the operator. The executor never builds this step again: on a later `--resume` it only runs the checks (B, D1, D2, D3, D4, E) on whatever the operator changed. If they fail again the step stays at 5 — there is no sixth round, `fix_rounds` never exceeds 5 — and the run stops again. Stop class 4 |
|
||||||
|
|
||||||
**Back to planning** (`fix_rounds = 4`): save `git -c core.quotepath=off diff --no-ext-diff --no-color --no-renames --src-prefix=a/ --dst-prefix=b/ {start_sha}` (one form whatever the user's git config) to
|
**Back to planning** (`fix_rounds = 4`): save `git -c core.quotepath=off diff --no-ext-diff --no-color --no-renames --no-relative --src-prefix=a/ --dst-prefix=b/ {start_sha}` (one form whatever the user's git config) to
|
||||||
`{report dir}/step-{N}-upward.diff` and write `{report dir}/step-{N}-upward.md`:
|
`{report dir}/step-{N}-upward.diff` and write `{report dir}/step-{N}-upward.md`:
|
||||||
the step verbatim, every round's findings and verdicts, and the question for
|
the step verbatim, every round's findings and verdicts, and the question for
|
||||||
the planner — which assumption in the step (its Changes, its Interface, its
|
the planner — which assumption in the step (its Changes, its Interface, its
|
||||||
|
|
|
||||||
|
|
@ -70,7 +70,7 @@ PARKED="$P/.git/voyage/parked"
|
||||||
savediff() {
|
savediff() {
|
||||||
color=--no-color
|
color=--no-color
|
||||||
[ "${2:-}" = color ] && color=--color=always
|
[ "${2:-}" = color ] && color=--color=always
|
||||||
git -C "$P" -c core.quotepath=off diff --no-ext-diff "$color" --no-renames --src-prefix=a/ --dst-prefix=b/ "$START" > "$1" \
|
git -C "$P" -c core.quotepath=off diff --no-ext-diff "$color" --no-renames --no-relative --src-prefix=a/ --dst-prefix=b/ "$START" > "$1" \
|
||||||
|| fail "git diff for $1"
|
|| fail "git diff for $1"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1561,7 +1561,7 @@ test('[ORD] sibling: agents/review-coordinator.md — Pass 1 groups in two passe
|
||||||
// or core.quotepath on in the user's config, its headers were coloured or
|
// 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).
|
// 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}';
|
const PARK_WRITER = 'git -c core.quotepath=off diff --no-ext-diff --no-color --no-renames --no-relative --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', () => {
|
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 `'));
|
const lines = read(EXEC).split('\n').filter((l) => l.startsWith('**Back to planning** (`fix_rounds = 4`): save `'));
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,8 @@
|
||||||
// E22e The park tool parks every file new in the attempt's diff, untracked or
|
// E22e The park tool parks every file new in the attempt's diff, untracked or
|
||||||
// staged but not in HEAD: its header and /trekexecute's resume paragraph
|
// staged but not in HEAD: its header and /trekexecute's resume paragraph
|
||||||
// no longer say "still untracked".
|
// no longer say "still untracked".
|
||||||
|
// M3 The "Back to planning" save line runs its git diff with --no-relative,
|
||||||
|
// so a session in a subdirectory with diff.relative on saves every file.
|
||||||
// No static import of a new export: the file must load at be0b631.
|
// No static import of a new export: the file must load at be0b631.
|
||||||
// Fix the SOURCE a pin guards, not the pin.
|
// Fix the SOURCE a pin guards, not the pin.
|
||||||
|
|
||||||
|
|
@ -130,3 +132,14 @@ test('[E22e] the execute command says the same', () => {
|
||||||
assert.ok(hits.some((s) => /new in the attempt's diff/.test(s) && /untracked, or staged but not in HEAD/.test(s)),
|
assert.ok(hits.some((s) => /new in the attempt's diff/.test(s) && /untracked, or staged but not in HEAD/.test(s)),
|
||||||
`no /trekexecute sentence says every file new in the attempt's diff is parked, untracked, or staged but not in HEAD: ${hits.join(' | ')}`);
|
`no /trekexecute sentence says every file new in the attempt's diff is parked, untracked, or staged but not in HEAD: ${hits.join(' | ')}`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// --- M3 ----------------------------------------------------------------------------
|
||||||
|
|
||||||
|
test('[M3] the back-to-planning save line carries --no-relative', () => {
|
||||||
|
const lines = read('commands/trekexecute.md').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}`);
|
||||||
|
const m = lines[0].match(/save `([^`]+)`/);
|
||||||
|
assert.ok(m, `no backticked command on the save line: ${lines[0]}`);
|
||||||
|
assert.match(m[1], /^git (?:-c \S+ )*diff (?:--\S+ )*--no-relative (?:--\S+ )*\{start_sha\}$/,
|
||||||
|
`the save line's git diff does not carry --no-relative: ${m[1]}`);
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -512,3 +512,51 @@ test('[E22c] a CRLF diff parks its new files', async () => {
|
||||||
rmSync(dir, { recursive: true, force: true });
|
rmSync(dir, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// --- fix round 4-5b: M3 ----------------------------------------------------------
|
||||||
|
// [M3] The "Back to planning" save line ran `git diff` as the user's config had
|
||||||
|
// it: with diff.relative on and the session in a subdirectory, the saved diff
|
||||||
|
// held only that subdirectory's files, with paths relative to it, so the park
|
||||||
|
// tool missed every new file outside it.
|
||||||
|
|
||||||
|
/** The command of the one "Back to planning" save line in commands/trekexecute.md, as written. */
|
||||||
|
function saveLine() {
|
||||||
|
const md = readFileSync(join(ROOT, 'commands/trekexecute.md'), 'utf8');
|
||||||
|
const lines = md.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}`);
|
||||||
|
const m = lines[0].match(/save `([^`]+)`/);
|
||||||
|
assert.ok(m, `no backticked command on the save line: ${lines[0]}`);
|
||||||
|
return m[1];
|
||||||
|
}
|
||||||
|
|
||||||
|
test('[M3] the save line, read from the execute command as written, parks every file from a subdirectory with diff.relative on', () => {
|
||||||
|
const dir = repo();
|
||||||
|
try {
|
||||||
|
git(dir, 'config', 'diff.relative', 'true');
|
||||||
|
mkdirSync(join(dir, 'sub'));
|
||||||
|
writeFileSync(join(dir, 'sub/keep.txt'), 'kept\n');
|
||||||
|
git(dir, 'add', 'sub/keep.txt');
|
||||||
|
git(dir, 'commit', '-q', '-m', 'sub');
|
||||||
|
const start = git(dir, 'rev-parse', 'HEAD').trim();
|
||||||
|
attemptFiles(dir);
|
||||||
|
writeFileSync(join(dir, 'sub/new-in-sub.txt'), 'stale sub\n');
|
||||||
|
git(dir, 'add', '--intent-to-add', '--', 'tests/batch.sh', 'tests/cli-stdin.test.mjs', 'sub/new-in-sub.txt');
|
||||||
|
const values = { start_sha: start };
|
||||||
|
const cmd = saveLine().replace(/\{([a-z_ ]+)\}/g, (all, key) => (key in values ? values[key] : all));
|
||||||
|
assert.doesNotMatch(cmd, /\{[^}]*\}/, `the save line holds a placeholder this test does not fill: ${cmd}`);
|
||||||
|
const out = join(dir, '.git', 'upward.diff');
|
||||||
|
const sub = join(dir, 'sub');
|
||||||
|
const saved = spawnSync('bash', ['-c', `${cmd} > "$1"`, 'save', out], { cwd: sub, encoding: 'utf8', env: childEnv() });
|
||||||
|
assert.equal(saved.status, 0, `the save line failed from sub/: ${saved.stderr}`);
|
||||||
|
const r = spawnSync(process.execPath, [CLI, '--diff', out, '--step', '1', '--attempt', '1', '--cwd', sub], { encoding: 'utf8', env: childEnv() });
|
||||||
|
assert.equal(r.status, 0, r.stdout + r.stderr);
|
||||||
|
assert.deepEqual(JSON.parse(r.stdout).parked, ['sub/new-in-sub.txt', 'tests/batch.sh', 'tests/cli-stdin.test.mjs'],
|
||||||
|
`the diff saved from sub/ with diff.relative on did not park every new file: ${r.stdout}`);
|
||||||
|
assert.equal(existsSync(join(dir, 'tests/batch.sh')), false, 'a new file outside sub/ is still in the tree');
|
||||||
|
assert.equal(existsSync(join(dir, 'sub/new-in-sub.txt')), false, 'a new file inside sub/ is still in the tree');
|
||||||
|
assert.equal(readFileSync(parkedPath(dir, 'sub/new-in-sub.txt'), 'utf8'), 'stale sub\n', 'the file is not under the parked directory');
|
||||||
|
assert.equal(readFileSync(join(dir, 'sub/keep.txt'), 'utf8'), 'kept\n', 'a committed file is never touched');
|
||||||
|
} finally {
|
||||||
|
rmSync(dir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue