From 92b460238396e38789bc01f8677f06b20cad66fa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 07:53:18 +0000 Subject: [PATCH] fix(scripts): let parseVitestArgv keep every value of a repeated flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parseVitestArgv` stored flags in a plain object (`flags[token] = next`), so a flag written twice on one command line kept only its last value. The guard's own readers never noticed — `:303` asks `--changed` for existence and `:357` asks `--root` for its one effective value, and neither is legitimately repeated — but the root `test:integration` script is `vitest run --project dom --project dom-heavy`, which the parser answers as `dom-heavy` with `dom` dropped and no symptom. The first reader to reuse the exported parser to ask "which projects does this command run" gets a confidently wrong answer. `flags` is left exactly as it was, last-wins, so the two scalar readers are byte-for-byte unaffected. A sibling `flagValues` map carries every occurrence of every flag in argv order; a flag seen once is a one-element array there, so a reader never needs a scalar-or-array fallback. All three recording sites (the `--flag=value` form, the space-separated value form and the bare boolean) route through one `record()` helper, so the two views cannot drift. The pin asserts both values come back in order for `--project dom --project dom-heavy`, that the scalar still reads the last one, and that a single `--project a` reads back as `['a']` — a non-empty-only assertion would pass against the unfixed parser. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MM7kaS4dPpYHV5BsMyu4tQ --- .../__tests__/vitest-invocation-guard.test.ts | 25 ++++++++++++ scripts/vitest-invocation-guard.mjs | 39 ++++++++++++++++--- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/scripts/__tests__/vitest-invocation-guard.test.ts b/scripts/__tests__/vitest-invocation-guard.test.ts index 7a9c2f2700..bd6e946773 100644 --- a/scripts/__tests__/vitest-invocation-guard.test.ts +++ b/scripts/__tests__/vitest-invocation-guard.test.ts @@ -84,6 +84,31 @@ describe('parseVitestArgv', () => { expect(parsed.afterDoubleDash).toEqual(['--run', 'packages/fields/src/a.test.ts']); }); + it('keeps EVERY value of a repeated flag, in order, next to the last-wins scalar', () => { + // objectui#7329. The root `test:integration` script really is + // `vitest run --project dom --project dom-heavy`; `flags` is last-wins, so it + // answers `dom-heavy` and drops `dom` with no symptom. A reader asking "which + // projects does this command run" reads `flagValues` instead. + const parsed = parseVitestArgv(argvFor('run', '--project', 'dom', '--project', 'dom-heavy')); + + expect(parsed.flagValues['--project']).toEqual(['dom', 'dom-heavy']); + // Backward compatibility, the whole reason `flags` was not turned into arrays: + // the guard's own `--root` / `--changed` readers still see one scalar, the last. + expect(parsed.flags['--project']).toBe('dom-heavy'); + // Neither value leaked into the file filters. + expect(parsed.positionals).toEqual([]); + }); + + it('lists a flag that appears once as a one-element array, so readers need no fallback', () => { + const parsed = parseVitestArgv(argvFor('run', '--project', 'a', '--shard=1/4', '--watch')); + + expect(parsed.flagValues['--project']).toEqual(['a']); + // Both other spellings are recorded too: `--flag=value` and a bare boolean. + expect(parsed.flagValues['--shard']).toEqual(['1/4']); + expect(parsed.flagValues['--watch']).toEqual([true]); + expect(parsed.flagValues['--reporter']).toBeUndefined(); + }); + it('treats a leading subcommand as the subcommand, not a filter', () => { expect(parseVitestArgv(argvFor('run')).positionals).toEqual([]); expect(parseVitestArgv(argvFor('list')).subcommand).toBe('list'); diff --git a/scripts/vitest-invocation-guard.mjs b/scripts/vitest-invocation-guard.mjs index d102ba72be..27e24f8aa5 100644 --- a/scripts/vitest-invocation-guard.mjs +++ b/scripts/vitest-invocation-guard.mjs @@ -239,8 +239,24 @@ const CONCRETE_TEST_PATH = /[\\/].*\.(test|spec)\.(c|m)?[jt]sx?$/; /** * Split a `process.argv`-shaped array into the parts this guard reasons about. * + * Flags come back twice, because two readers want two different answers: + * + * - `flags` is LAST-WINS — one scalar per flag, the final occurrence. That is + * what this guard's own readers want (`--changed` for existence, `--root` + * for the one root that actually takes effect), and it is the shape they + * have always had. + * - `flagValues` is the lossless sibling — every occurrence of every flag, in + * argv order. A repeated flag is legal and meaningful in this repo: the root + * `test:integration` script is `vitest run --project dom --project + * dom-heavy`, which `flags` alone reports as `dom-heavy` with `dom` silently + * dropped (objectui#7329). A reader asking "which projects does this command + * run" reads `flagValues['--project']`. + * + * Every flag is in `flagValues`, including one that appears once (as a + * one-element array), so a reader never needs a scalar-or-array fallback. + * * @param {string[]} argv full `process.argv` (node binary + script + args) - * @returns {{ subcommand: string | null, positionals: string[], afterDoubleDash: string[], flags: Record }} + * @returns {{ subcommand: string | null, positionals: string[], afterDoubleDash: string[], flags: Record, flagValues: Record> }} */ export function parseVitestArgv(argv) { const args = argv.slice(2); @@ -250,8 +266,21 @@ export function parseVitestArgv(argv) { const afterDoubleDash = []; /** @type {Record} */ const flags = {}; + /** @type {Record>} */ + const flagValues = {}; let subcommand = null; + /** + * Record one occurrence of a flag into both views. + * + * @param {string} name the flag as written, e.g. `--project` + * @param {string | true} value its value, or `true` for a bare boolean flag + */ + const record = (name, value) => { + flags[name] = value; + (flagValues[name] ??= []).push(value); + }; + for (let i = 0; i < args.length; i += 1) { const token = args[i]; @@ -263,16 +292,16 @@ export function parseVitestArgv(argv) { if (token.startsWith('-')) { const eq = token.indexOf('='); if (eq !== -1) { - flags[token.slice(0, eq)] = token.slice(eq + 1); + record(token.slice(0, eq), token.slice(eq + 1)); continue; } const next = args[i + 1]; if (VALUE_FLAGS.has(token) && next !== undefined && !next.startsWith('-')) { - flags[token] = next; + record(token, next); i += 1; continue; } - flags[token] = true; + record(token, true); continue; } @@ -284,7 +313,7 @@ export function parseVitestArgv(argv) { positionals.push(token); } - return { subcommand, positionals, afterDoubleDash, flags }; + return { subcommand, positionals, afterDoubleDash, flags, flagValues }; } /**