From d0872f549f4ea894cac677422385a2df6b6407fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 12:40:34 +0000 Subject: [PATCH] fix(devx): refuse an appended path filter that a baked positional already swallows (objectui#7814) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pnpm --filter test ` does not narrow the run. objectui#3240 bakes a positional into every package `test` script (`vitest run --root ../.. packages//`), and Vitest UNIONS positional filters, so the appended path sits beside the baked one and every file the baked filter admits still runs. It exits 0 and prints a green summary for the PACKAGE, which reads exactly like a narrowed run for the FILE — the count is real, the attribution is not. Measured on this base, argv captured from inside the guard itself during a real run: both filters arrive in ONE `positionals` array, in ONE process, and the guard returned `null` for every config load of that run. It could see the union and said nothing. `evaluateVitestInvocation` now returns a `subsumed-positional-filter` verdict when one positional contains another as a substring — which is exactly how Vitest matches them, so the longer one admits a subset of what the shorter already admits and changes the collected set by nothing. The message names which filter was swallowed by which, and hands back the form that does narrow (`pnpm exec vitest run --root ../.. `), spelled for the caller's cwd. The check runs LAST, after the three existing verdicts, so it only ever adds refusals: every invocation refused before is still refused with the same code. The bare `pnpm --filter test` — the baked filter alone — stays allowed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013VGeMu3p6qEFWR6K6GGLaW --- .../__tests__/vitest-invocation-guard.test.ts | 81 ++++++++++++++++++ scripts/vitest-invocation-guard.mjs | 85 ++++++++++++++++++- 2 files changed, 165 insertions(+), 1 deletion(-) diff --git a/scripts/__tests__/vitest-invocation-guard.test.ts b/scripts/__tests__/vitest-invocation-guard.test.ts index bd6e946773..d0a21a17c5 100644 --- a/scripts/__tests__/vitest-invocation-guard.test.ts +++ b/scripts/__tests__/vitest-invocation-guard.test.ts @@ -224,6 +224,87 @@ describe('evaluateVitestInvocation — objectui#3288, the filter that never land }); }); +describe('evaluateVitestInvocation — objectui#7814, the appended filter that WIDENS', () => { + // The argv below is not a reconstruction. It was captured out of the guard + // itself, on `origin/main`, during a real + // pnpm --filter @object-ui/cli test packages/cli/src/__tests__/app-generator.test.ts + // by appending a dump inside `assertCanonicalVitestInvocation`. Both filters + // arrive in ONE `positionals` array, in ONE process, and the guard returned + // `null` for all five config loads of that run — which is the defect: the run + // executed the whole package, exited 0, and printed a green summary that reads + // exactly like a narrowed run for the one file. + const CAPTURED = ['run', '--root', '../..', 'packages/cli/']; + const APPENDED = 'packages/cli/src/__tests__/app-generator.test.ts'; + const AT_PKG = { cwd: `${FAKE_ROOT}/packages/cli` }; + + it('refuses the exact invocation the card measured', () => { + const verdict = judge([...CAPTURED, APPENDED], AT_PKG); + + expect(verdict?.code).toBe('subsumed-positional-filter'); + expect(verdict?.message).toContain('objectui#7814'); + // It has to NAME the union — which filter was swallowed by which — or the + // reader is told "no" without being told what to drop. + expect(verdict?.message).toContain(APPENDED); + expect(verdict?.message).toContain('packages/cli/'); + // ...and hand back the form that actually narrows, spelled for the + // directory the caller is standing in. + expect(verdict?.message).toContain(`pnpm exec vitest run --root ../.. ${APPENDED}`); + }); + + it('is the control: without the appended path the SAME command stays allowed', () => { + // `pnpm --filter @object-ui/cli test` — the baked filter alone — is the one + // legitimate package-level run and must not become collateral. This is the + // negative half that proves the check above discriminates rather than + // refusing every package-level invocation. + expect(judge(CAPTURED, AT_PKG)).toBeNull(); + }); + + it('leaves disjoint filters alone — subsumption is the trigger, not arity', () => { + expect(judge(['run', 'packages/fields/', 'packages/core/'])).toBeNull(); + }); + + it('reads subsumption the way Vitest matches: substring of the path, not path prefix', () => { + // `vitest run cli` matches every file whose path CONTAINS `cli`, so naming a + // file underneath it adds nothing — the same union, spelled without a slash. + expect(judge(['run', 'cli', APPENDED])?.code).toBe('subsumed-positional-filter'); + // And the containment really is textual: `packages/core` matches + // `packages/core-extras/...` too, so that pair is genuinely redundant. + expect(judge(['run', 'packages/core', 'packages/core-extras/src/a.test.ts'])?.code).toBe( + 'subsumed-positional-filter' + ); + }); + + it('does not fire on an exact repeat, which asks for nothing narrower', () => { + // `vitest run packages/cli/ packages/cli/` collects what the caller asked + // for. Nothing is misattributed, so there is nothing to refuse — pinned so + // that stays a decision rather than an accident of the comparison. + expect(judge(['run', 'packages/cli/', 'packages/cli/'])).toBeNull(); + }); + + it('yields to the older verdicts, so this check only ever ADDS refusals', () => { + // A `--` run and a package-cwd run both also carry a subsumed pair here. + // They keep their original codes: the new check runs last, so no invocation + // that was refused before is refused differently now. + expect(judge(['run', 'packages/cli/', '--', APPENDED], AT_PKG)?.code).toBe('double-dash-args'); + // Same subsumed pair, but launched from a package directory with no --root: + // that is objectui#3378 and it keeps saying so. + expect(judge(['run', 'packages/cli/', APPENDED], AT_PKG)?.code).toBe('package-cwd'); + // A missing appended path is still objectui#3288's verdict, not this one. + expect( + judge(['run', 'packages/cli/', 'packages/cli/src/typo.test.ts'], { exists: () => false }) + ?.code + ).toBe('missing-path-filter'); + // With none of those in play, the new verdict is what is left. + expect(judge(['run', 'packages/cli/', APPENDED])?.code).toBe('subsumed-positional-filter'); + }); + + it('stands down for the escape hatch like every other verdict', () => { + expect( + judge([...CAPTURED, APPENDED], { ...AT_PKG, env: { OBJECTUI_VITEST_GUARD: 'off' } }) + ).toBeNull(); + }); +}); + describe('evaluateVitestInvocation — the escape hatch', () => { it('stands down for OBJECTUI_VITEST_GUARD=off', () => { expect(judge(['run'], { cwd: FAKE_PKG, env: { OBJECTUI_VITEST_GUARD: 'off' } })).toBeNull(); diff --git a/scripts/vitest-invocation-guard.mjs b/scripts/vitest-invocation-guard.mjs index 695d6f2b93..e5069f5263 100644 --- a/scripts/vitest-invocation-guard.mjs +++ b/scripts/vitest-invocation-guard.mjs @@ -1,6 +1,9 @@ #!/usr/bin/env node /** - * Rejects the two Vitest invocations that silently produce a FALSE GREEN. + * Rejects the Vitest invocations that silently produce a FALSE GREEN — the + * verdicts `evaluateVitestInvocation` returns below, each pinned in + * `scripts/__tests__/vitest-invocation-guard.test.ts`. Read that test for the + * set that is refused today; a count written here would go stale in silence. * * Called from the top of `vitest.config.mts` — the repo's ONE Vitest config * since objectui#3240 — and from every other config file Vitest can pick up @@ -102,6 +105,20 @@ * named files and zero matched" is an error, while "no filter, and one project * happens to hold no files" stays fine. * + * ## Trap 3 — an appended path filter that WIDENS the run (objectui#7814) + * + * pnpm --filter @object-ui/cli test packages/cli/src/__tests__/app-generator.test.ts + * => Test Files 17 passed (17) <- the whole package, not the one file + * Tests 266 passed (266) (the file alone is 1 file / 47 tests) + * + * objectui#3240 bakes a positional into every package `test` script + * (`vitest run --root ../.. packages//`), and Vitest UNIONS positional + * filters. The appended path therefore does not replace the baked one, it sits + * beside it, and every file the baked filter admits still runs. Exit 0, green + * summary — the package's count read as the file's. `subsumed-positional-filter` + * refuses it; the numbers above are from the measurement on that card and are a + * timestamp, not a live reading. + * * ## The canonical invocation * * pnpm exec vitest run packages//src/.test.ts # from the REPO ROOT @@ -479,6 +496,72 @@ export function evaluateVitestInvocation({ }; } + // ## Trap 3 — an appended filter another positional already swallows + // + // Vitest matches each positional as a SUBSTRING of the test file path and + // takes the UNION of them, never the intersection. So when one positional + // contains another as a substring, the longer one admits a subset of what the + // shorter already admits and changes the collected set by nothing at all. + // + // This is not hypothetical spelling: objectui#3240 gave every package a + // `test` script that bakes its own positional in — `vitest run --root ../.. + // packages//` — so `pnpm --filter test ` appends a + // SECOND filter beside that one and runs the whole package. Measured, exact + // argv as the guard receives it (objectui#7814): + // + // pnpm --filter @object-ui/cli test packages/cli/src/__tests__/app-generator.test.ts + // => positionals: ['packages/cli/', 'packages/cli/src/__tests__/app-generator.test.ts'] + // + // It exits 0 and prints a green summary for the PACKAGE, which reads exactly + // like a successful narrowed run for the FILE. The count is real; the + // attribution is not, and nothing on screen separates the two. That is the + // same false-green shape as traps 1 and 2 — a caller who asked for one file + // is handed somebody else's count — so it is refused on the same terms, + // rather than left to a sentence somewhere that nobody is reading at the + // moment it fires. + const swallowed = positionals + .map((filter) => ({ + filter, + broader: positionals.find((other) => other !== filter && filter.includes(other)), + })) + .filter((pair) => pair.broader !== undefined); + + if (swallowed.length > 0) { + const { filter, broader } = swallowed[0]; + const backToRoot = path.relative(realpath(cwd), root) || '.'; + const fromHere = pkgDir + ? [` pnpm exec vitest run --root ${backToRoot} ${filter} # 就在当前目录(${pkgDir}/)`] + : []; + return { + code: 'subsumed-positional-filter', + message: box( + 'vitest 调用被拒绝:追加的路径过滤没有缩小范围,反而被并进了更宽的那个 (objectui#7814)', + [ + `位置参数: ${positionals.join(' ')}`, + `其中 ${filter} 被 ${broader} 整个包含。`, + '', + 'vitest 把多个位置参数按【子串匹配】取【并集】,不取交集:凡是', + `${broader} 能匹配到的文件,${filter} 一个也拦不掉 ——`, + '追加的这个过滤器一个文件都没多跑,也一个都没少跑。', + '', + '包级 `test` 脚本自带一个 `packages//` 过滤(objectui#3240 定下的写法),', + '所以 `pnpm --filter test <路径>` 追加的路径是【第二个】过滤器,跑的仍然是', + '整个包。它退出码 0、摘要一片绿,屏幕上没有任何东西把「整包」和「一个文件」区分开 ——', + '把整包的测试数当成那个文件的测试数,数字是真的,归属是假的。', + '', + '要真正只跑那一个文件,用【不带】baked 过滤器的形式:', + '', + ...fromHere, + ` pnpm exec vitest run ${filter} # 或 cd 到仓库根再跑`, + '', + '确实要跑整个包,就把追加的路径去掉(`pnpm --filter test` 本身就是整包)。', + '', + '确需绕过(自担风险): OBJECTUI_VITEST_GUARD=off', + ] + ), + }; + } + return null; }