Skip to content

Commit d1487be

Browse files
thecodedriftclaude
andcommitted
fix(cli): reject nested Vale fixture directories instead of skipping them
The two halves of verification disagreed about recursion. `fixtureFiles` reads one directory deep, but Vale is invoked over the whole `rule-tests/<rule>` tree and lints recursively, so a nested fixture was linted while never being collected. That fails in the dangerous direction. A nested `pass/` fixture that wrongly fires produces a finding the expected-set never knew about, so `unexpectedFindings` discards it; a nested `fail/` fixture is never required to fire. Either way the rule reports `passed: true` with half its fixtures unchecked -- the same "ships looking verified" failure `ValeFixtureCoverage` was added to prevent, one directory deeper. Flat and loud rather than recursive: one legal layout instead of two, and the error names the offending path the moment someone creates it. Nothing nests today, so this closes the gap before it can be hit rather than fixing a live break. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
1 parent 99fca71 commit d1487be

2 files changed

Lines changed: 51 additions & 0 deletions

File tree

packages/cli/src/rules/vale/verify.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,20 @@ async function directoryEntries(directory: string): Promise<Dirent[]> {
8989
* one-sided, and could report `passed: true` having never checked the pass side
9090
* at all. A permissions problem must not read as "no pass fixtures were
9191
* written".
92+
*
93+
* A bucket is one directory deep, and a nested directory is rejected rather
94+
* than ignored. The two halves of verification disagree about recursion: this
95+
* read is flat, but Vale is invoked over the whole `rule-tests/<rule>` tree and
96+
* lints recursively. Silently skipping a nested entry therefore fails in the
97+
* dangerous direction — a nested `pass/` fixture that wrongly fires produces a
98+
* finding this function never collected, so `unexpectedFindings` discards it,
99+
* and a nested `fail/` fixture is never required to fire. Either way the rule
100+
* reports `passed: true` while half its fixtures went unchecked, which is the
101+
* exact failure `ValeFixtureCoverage` exists to prevent.
102+
*
103+
* Flat-and-loud is chosen over recursing because it keeps one layout legal
104+
* instead of two, and because the error names the offending path at the moment
105+
* someone creates it.
92106
*/
93107
async function fixtureFiles(
94108
cwd: string,
@@ -97,6 +111,17 @@ async function fixtureFiles(
97111
): Promise<string[]> {
98112
const directory = join(valeRuleTestsDirectory(cwd, ruleId), bucket);
99113
const entries = await directoryEntries(directory);
114+
115+
const nested = entries.find((entry) => entry.isDirectory());
116+
if (nested !== undefined) {
117+
throw new Error(
118+
`Vale fixture buckets are flat: ${join(directory, nested.name)} is a ` +
119+
`directory. Move its documents directly into ${bucket}/ — Vale lints ` +
120+
`the rule's whole directory, so a nested fixture is linted but never ` +
121+
`checked.`
122+
);
123+
}
124+
100125
return entries
101126
.filter((entry) => entry.isFile())
102127
.map((entry) => join(directory, entry.name));

packages/cli/test/vale-verify.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,32 @@ asUser("verifyValeRule with an unreadable bucket", () => {
162162
});
163163
});
164164

165+
describe("fixture buckets are flat", () => {
166+
it("rejects a nested directory instead of silently skipping it", async () => {
167+
// The dangerous case: Vale lints `rule-tests/<rule>` recursively, so a
168+
// nested fixture IS linted, but a flat read never collects it. Skipping it
169+
// quietly would let a nested `pass/` fixture fire with its finding
170+
// discarded, and a nested `fail/` fixture never be required to fire —
171+
// `passed: true` over fixtures that were never checked.
172+
const cwd = makeProject(
173+
{ "no-simply": existence("simply") },
174+
{
175+
"no-simply": {
176+
pass: { "clean.md": "Nothing objectionable.\n" },
177+
fail: { "a.md": "Just simply do it.\n" },
178+
},
179+
}
180+
);
181+
mkdirSync(join(cwd, ".taskless", "vale", "rule-tests", "no-simply", "pass", "nested"), {
182+
recursive: true,
183+
});
184+
185+
await expect(verifyValeRule(cwd, "no-simply")).rejects.toThrow(
186+
/fixture buckets are flat/i
187+
);
188+
});
189+
});
190+
165191
withVale("verifyValeRule", () => {
166192
it("passes when every fail fixture fires and every pass fixture is clean", async () => {
167193
const cwd = makeProject(

0 commit comments

Comments
 (0)