Skip to content

Commit 0f1aaad

Browse files
committed
fix(cli): refuse migration 0004 when a file occupies an engine directory
`movePreservingContent` documented a bail-out for a file at the *destination* but stat'd the source, so a directory source with a file destination recursed and then failed part-way through the migration. Found in review by Copilot on #79. The guard now requires both sides to be directories, and the migration checks every engine directory up front and refuses with SCAFFOLD_CONFLICT naming the path. Bailing out of a single move was not enough on its own — scaffolding still ran `mkdir` over the same file and died with a bare EEXIST, after earlier moves had already landed. Validating first keeps `.taskless/` in a state the user can still reason about.
1 parent ba24d48 commit 0f1aaad

3 files changed

Lines changed: 77 additions & 3 deletions

File tree

‎packages/cli/src/filesystem/migrations/0004-vale-engine.ts‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import { dirname, join } from "node:path";
1313

1414
import type { Migration } from "../types";
15+
import { CLIError } from "../../util/cli-error";
1516

1617
/**
1718
* Default `sgconfig.yml` written when a project has none to move. `ruleDirs`
@@ -94,9 +95,15 @@ async function movePreservingContent(
9495
return;
9596
}
9697

97-
const sourceStats = await stat(source);
98-
if (!sourceStats.isDirectory()) {
99-
// A file already exists at the destination — leave both alone.
98+
const [sourceStats, destinationStats] = await Promise.all([
99+
stat(source),
100+
stat(destination),
101+
]);
102+
if (!sourceStats.isDirectory() || !destinationStats.isDirectory()) {
103+
// Something already occupies the destination and at least one side is a
104+
// file, so there is no merge to perform. Checking only the source would
105+
// let a directory recurse into a file destination and fail with ENOTDIR
106+
// part-way through the migration, having already moved some entries.
100107
return;
101108
}
102109

@@ -163,6 +170,36 @@ async function anchorSgConfigIgnore(directory: string): Promise<void> {
163170
await writeFile(gitignorePath, rewritten.join("\n"), "utf8");
164171
}
165172

173+
/**
174+
* Refuse to start when a **file** sits where an engine directory belongs.
175+
*
176+
* Checked up front, before anything moves. Every one of these paths must end up
177+
* a directory, and none of the steps below can merge a file into one: the move
178+
* declines, then `mkdir` fails with a bare `EEXIST` — after earlier moves have
179+
* already happened. Failing first keeps `.taskless/` in the state the user can
180+
* still reason about, and says which path to deal with.
181+
*/
182+
async function assertNoDirectoryConflicts(directory: string): Promise<void> {
183+
const conflicts: string[] = [];
184+
for (const segments of SCAFFOLD_DIRECTORIES) {
185+
const path = join(directory, ...segments);
186+
if (!(await pathExists(path))) continue;
187+
const stats = await stat(path);
188+
if (!stats.isDirectory()) conflicts.push(segments.join("/"));
189+
}
190+
if (conflicts.length === 0) return;
191+
192+
throw new CLIError(
193+
`Cannot partition .taskless/ by engine: ${conflicts
194+
.map((path) => `.taskless/${path}`)
195+
.join(
196+
", "
197+
)} ${conflicts.length === 1 ? "is a file" : "are files"}, but ` +
198+
`must be a directory. Move or delete it, then run the command again.`,
199+
"SCAFFOLD_CONFLICT"
200+
);
201+
}
202+
166203
/** Create `path` and drop a `.gitkeep` in it when it would otherwise be empty. */
167204
async function ensureTrackedDirectory(path: string): Promise<void> {
168205
await mkdir(path, { recursive: true });
@@ -187,6 +224,8 @@ async function ensureTrackedDirectory(path: string): Promise<void> {
187224
* beyond re-asserting the scaffold.
188225
*/
189226
const migration: Migration = async (directory) => {
227+
await assertNoDirectoryConflicts(directory);
228+
190229
for (const [from, to] of MOVES) {
191230
await movePreservingContent(
192231
join(directory, ...from),

‎packages/cli/src/types/errors.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export type CLIErrorCode =
1717
| "SCAN_FAILED"
1818
| "RECONCILE_FAILED"
1919
| "SCAFFOLD_VERSION_MISMATCH"
20+
| "SCAFFOLD_CONFLICT"
2021
| "INTERNAL_ERROR";
2122

2223
/**

‎packages/cli/test/migrate-engine-layout.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,40 @@ describe("migration 0004 — engine-partitioned layout", () => {
333333
await readFile(join(tasklessDirectory, "sg", "sgconfig.yml"), "utf8")
334334
).toBe(custom);
335335
});
336+
337+
it("refuses to start when a file occupies an engine directory", async () => {
338+
await writeFile(
339+
join(tasklessDirectory, "taskless.json"),
340+
JSON.stringify({ version: 3 }),
341+
"utf8"
342+
);
343+
// A *file* sits where `rules/` would land. Nothing can merge the two, so
344+
// the migration must fail before it moves anything — a partial move would
345+
// leave `.taskless/` split across both layouts.
346+
await writeTree(tasklessDirectory, {
347+
"rules/no-eval.yml": CAPTURE_YML,
348+
"runtime-rules/no-eval-capture/rule.yml": CAPTURE_YML,
349+
"sg/rules": "not a directory\n",
350+
});
351+
352+
await expect(ensureTasklessDirectory(temporaryDirectory)).rejects.toThrow(
353+
/\.taskless\/sg\/rules is a file/
354+
);
355+
356+
// Everything is exactly where it was: no half-migration.
357+
expect(await sha256(join(tasklessDirectory, "rules", "no-eval.yml"))).toBe(
358+
createHash("sha256").update(CAPTURE_YML).digest("hex")
359+
);
360+
expect(
361+
await exists(join(tasklessDirectory, "runtime-rules", "no-eval-capture"))
362+
).toBe(true);
363+
expect(await exists(join(tasklessDirectory, "runtime", "rules"))).toBe(
364+
false
365+
);
366+
expect(await readFile(join(tasklessDirectory, "sg", "rules"), "utf8")).toBe(
367+
"not a directory\n"
368+
);
369+
});
336370
});
337371

338372
describe("scaffold version gating", () => {

0 commit comments

Comments
 (0)