fix(trekexecute): assign PLAN_PATH inside the Check 2 block, keep both blocks ASCII
Follow-up to63e78c5. Adversarial review found that the fix had reintroduced, in a smaller form, the exact defect the order was filed about. 1. The rewritten Check 2 block opened with `git ls-files --error-unmatch "$PLAN_PATH"` while the assignment lived in the prose above it ("Set PLAN_PATH to the plan path, then run:"). Nothing upstream sets it — Check 2 is Phase 2.55, and REPO_ROOT/WORKTREE_DIR are only created in Phase 2.6 Step 1, which runs after. An agent copying the block verbatim would have run it with PLAN_PATH empty. The note was prose, the code was what ran. The block now carries `PLAN_PATH="{plan-path}"` as its first line, the same placeholder idiom the rest of the file uses (`BRANCH_NAME="trek/{slug}/session-{N}"`). The three Check 2 tests could not have caught this: they injected PLAN_PATH through the environment, supplying what the doc has to supply itself. They now substitute `{plan-path}` the way an agent does, assert the placeholder is present, and pass PLAN_PATH="" in the environment so a block that fails to assign it goes red. Control run with the assignment line stripped: `fatal: empty string is not a valid pathspec` -> exit 1. 2. Both copied blocks contained an em-dash (one in an `echo` string, one in a comment). Shell that bash 3.2 executes stays ASCII — a multibyte char under `set -u` has crashed it before. Replaced with plain `-`; the prose outside the fences keeps its em-dashes. New test asserts both blocks are ASCII-clean, with a known-positive proving the detector fires on an em-dash. Suite 1022 (1020/0/2) -> 1023 (1021/0/2). No version bump, no release.63e78c5stays valid in history: the order archive and two coord messages point at it. Co-Authored-By: Claude <claude-opus-5>
This commit is contained in:
parent
63e78c5ec0
commit
bcd2600918
2 changed files with 33 additions and 11 deletions
|
|
@ -430,9 +430,8 @@ publishes to. When the plan file is ignored, Phase 2.6 Step 2a' (which copies
|
|||
brief/plan/research into each worktree) is the delivery path, and this check
|
||||
must step aside instead of failing.
|
||||
|
||||
Set `PLAN_PATH` to the plan path, then run:
|
||||
|
||||
```bash
|
||||
PLAN_PATH="{plan-path}"
|
||||
if git ls-files --error-unmatch "$PLAN_PATH" >/dev/null 2>&1; then
|
||||
PLAN_TRACKING="tracked"
|
||||
else
|
||||
|
|
@ -440,7 +439,7 @@ else
|
|||
case "$?" in
|
||||
0) PLAN_TRACKING="ignored" ;;
|
||||
1) PLAN_TRACKING="untracked" ;;
|
||||
*) echo "Error: git check-ignore failed on $PLAN_PATH — a fatal probe is not an answer about ignore status." >&2
|
||||
*) echo "Error: git check-ignore failed on $PLAN_PATH - a fatal probe is not an answer about ignore status." >&2
|
||||
exit 1 ;;
|
||||
esac
|
||||
fi
|
||||
|
|
@ -636,7 +635,7 @@ REPO_ROOT_REAL="$(realpath "${REPO_ROOT}")"
|
|||
# Compute destination relpath: PROJECT_DIR relative to REPO_ROOT.
|
||||
# This makes $wt/$PROJECT_REL valid regardless of whether the operator
|
||||
# passed --project as relative (.claude/projects/...) or absolute.
|
||||
# python3 + os.path.relpath is stdlib and portable — see the note below the
|
||||
# python3 + os.path.relpath is stdlib and portable - see the note below the
|
||||
# block for why no realpath flag may be used here.
|
||||
PROJECT_REL="$(python3 -c 'import os.path,sys; print(os.path.relpath(sys.argv[1], sys.argv[2]))' "$PROJECT_SOURCE" "$REPO_ROOT_REAL")"
|
||||
case "$PROJECT_REL" in
|
||||
|
|
|
|||
|
|
@ -259,10 +259,33 @@ test("2a' — the GNU-only form is gone from the block agents copy", () => {
|
|||
assert.ok(block.includes('os.path.relpath'), 'the copied block must derive the relpath portably');
|
||||
});
|
||||
|
||||
test("both copied blocks are ASCII-clean (bash 3.2 dies on a multibyte char under set -u)", () => {
|
||||
const nonAscii = (s) => s.split('\n')
|
||||
.map((line, i) => [i + 1, line])
|
||||
.filter(([, line]) => /[^\x00-\x7F]/.test(line));
|
||||
// Known-positive: the detector must actually fire on a multibyte char.
|
||||
assert.equal(nonAscii('echo "a — b"').length, 1, 'detector must find an em-dash');
|
||||
for (const anchor of [COPY_BLOCK_ANCHOR, CHECK2_ANCHOR]) {
|
||||
assert.deepEqual(nonAscii(extractBashBlock(anchor)), [],
|
||||
`non-ASCII inside the shell block after ${anchor} (prose outside the fence is fine)`);
|
||||
}
|
||||
});
|
||||
|
||||
// --- Defect 2: gitignored project directory ------------------------------
|
||||
|
||||
function check2Env(repo, planPath, pathPrefixDir) {
|
||||
const env = { REPO_ROOT: repo.root, PLAN_PATH: planPath };
|
||||
// The block carries the `{plan-path}` placeholder the way every other block in
|
||||
// trekexecute.md does. Substitute it exactly as an agent would — never inject
|
||||
// PLAN_PATH through the environment: that would supply what the doc must supply
|
||||
// itself, and a block that never assigns the variable would still pass.
|
||||
function check2Script(planPath) {
|
||||
const block = extractBashBlock(CHECK2_ANCHOR);
|
||||
assert.ok(block.includes('{plan-path}'),
|
||||
'Check 2 block must carry the {plan-path} placeholder for the agent to substitute');
|
||||
return block.replace('{plan-path}', planPath);
|
||||
}
|
||||
|
||||
function check2Env(repo, pathPrefixDir) {
|
||||
const env = { REPO_ROOT: repo.root, PLAN_PATH: '' };
|
||||
if (pathPrefixDir) env.PATH = `${pathPrefixDir}:${process.env.PATH}`;
|
||||
return env;
|
||||
}
|
||||
|
|
@ -277,8 +300,8 @@ test('Check 2 — gitignored project dir: no commit, no failure, and 2a\' still
|
|||
assert.equal(ci.status, 0, 'fixture premise: the plan file must actually be gitignored');
|
||||
|
||||
const head = git(repo.root, 'rev-parse', 'HEAD').trim();
|
||||
const r = runBlock(extractBashBlock(CHECK2_ANCHOR),
|
||||
{ cwd: repo.root, env: check2Env(repo, join(repo.projectRel, 'plan.md')) });
|
||||
const r = runBlock(check2Script(join(repo.projectRel, 'plan.md')),
|
||||
{ cwd: repo.root, env: check2Env(repo) });
|
||||
assert.equal(r.status, 0, `Check 2 must tolerate a gitignored plan file. stderr: ${r.stderr}`);
|
||||
assert.equal(git(repo.root, 'rev-parse', 'HEAD').trim(), head,
|
||||
'an ignored plan file must NOT be forced into history (origin is a public mirror)');
|
||||
|
|
@ -298,7 +321,7 @@ test('Check 2 — KNOWN-POSITIVE: an untracked, NOT-ignored plan file is still a
|
|||
try {
|
||||
const planPath = join(repo.projectRel, 'plan.md');
|
||||
const head = git(repo.root, 'rev-parse', 'HEAD').trim();
|
||||
const r = runBlock(extractBashBlock(CHECK2_ANCHOR), { cwd: repo.root, env: check2Env(repo, planPath) });
|
||||
const r = runBlock(check2Script(planPath), { cwd: repo.root, env: check2Env(repo) });
|
||||
assert.equal(r.status, 0, `Check 2 must succeed on a normal untracked plan. stderr: ${r.stderr}`);
|
||||
assert.notEqual(git(repo.root, 'rev-parse', 'HEAD').trim(), head,
|
||||
'a trackable plan file must still be committed for worktree visibility');
|
||||
|
|
@ -314,8 +337,8 @@ test('Check 2 — a FATAL git check-ignore (128) is not read as "not ignored"',
|
|||
const stubDir = fatalCheckIgnoreGitStubDir();
|
||||
try {
|
||||
const head = git(repo.root, 'rev-parse', 'HEAD').trim();
|
||||
const r = runBlock(extractBashBlock(CHECK2_ANCHOR),
|
||||
{ cwd: repo.root, env: check2Env(repo, join(repo.projectRel, 'plan.md'), stubDir) });
|
||||
const r = runBlock(check2Script(join(repo.projectRel, 'plan.md')),
|
||||
{ cwd: repo.root, env: check2Env(repo, stubDir) });
|
||||
assert.notEqual(r.status, 0, 'a fatal check-ignore must stop, not fall through to git add');
|
||||
assert.match(r.stderr, /check-ignore/, 'the stop must name the failing probe');
|
||||
assert.equal(git(repo.root, 'rev-parse', 'HEAD').trim(), head, 'no commit may be made on a fatal probe');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue