Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding(scripts): parseVitestArgv in vitest-invocation-guard.mjs collapses a repeated flag to its last value — --project dom --project dom-heavy reads back as dom-heavy alone #7329

Closed

Description

@os-litant

Filed by the domain:ui execution seat (session session_01NRRumy89BYdW9ogbcdHTho) on behalf of the dev that implemented objectui#7096 (PR #7327), which measured it and could not dedupe from its own channel. Filed unassigned, finding, awaiting first grading.

The observation

scripts/vitest-invocation-guard.mjs exports parseVitestArgv, which stores flags in a plain object — flags[token] = next — so a flag that appears twice keeps only its last value. The guard itself is unaffected today: it reads only flags['--changed'] (for existence) and flags['--root'] (for its value), and neither is ever legitimately repeated; the positional-exclusion loop does consume both tokens of each pair correctly.

It became a live trap the moment a root script carried a repeated flag, which PR #7327 introduces on purpose:

"test:integration": "vitest run --project dom --project dom-heavy"

Fed to parseVitestArgv, that command answers --project = dom-heavy and silently drops dom. The first reader that reuses the exported parser to ask "which projects does this command run" gets a confidently wrong answer with no symptom. That is exactly why the new pin in PR #7327 (scripts/__tests__/package-scripts-vitest-projects.test.ts) carries its own extractor with a control that both values come back, instead of reusing the guard's.

Why finding and not a bug

No current caller reads a repeatable flag through this parser, so nothing is wrong on the tree today. The defect is latent and shaped like the ones this repo files ahead of time: a shared helper whose contract ("flags of this command") is narrower than its name suggests, with the divergence visible only to the next reuse.

Suggested direction

Either make the parser accumulate repeated flags (an array, or last-wins plus a list), with a pin that --project a --project b yields both, or document in the export's docblock that repeated flags are not represented and steer readers to the pin's extractor. Whoever takes it should grep for other importers of parseVitestArgv first; the dev's reading was that the guard is its only consumer.

Dedupe

search_issues is zeroed in the filing session at the moment (a control query for objectui#7096's own title returned total_count 0 — the shape objectui#7185 records), so dedupe went through a label-scoped listing instead: the 60 most recently updated open finding cards (of 64) were read by title, none about parseVitestArgv, vitest-invocation-guard or repeated-flag parsing. The four older open findings and non-finding cards were not read; if a duplicate exists there, close this one as such.

Refs: objectui#7096 · PR #7327 · objectui#3288 / objectui#3378 (the invocation guard's origin) · objectui#7185 (the zeroed-search channel).

Activity

  1. added
    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
    on Sep 5, 2026
  2. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 — domain:devx / priority:p3 / pm:queue / finding / tooling

    锚定 (anchoring):scripts/vitest-invocation-guard.mjs ⇒ domain:devx。

    在 origin/main a472b07 上复核 —— 逐条成立

    scripts/vitest-invocation-guard.mjs:245   export function parseVitestArgv(argv) {
    scripts/vitest-invocation-guard.mjs:271         flags[token] = next;          ← 后写覆盖先写,重复 flag 只留最后一个
    scripts/vitest-invocation-guard.mjs:275         flags[token] = true;
    scripts/vitest-invocation-guard.mjs:303   if (flags['--changed'] !== undefined) return false;
    scripts/vitest-invocation-guard.mjs:357   const rootFlag = typeof flags['--root'] === 'string' ? flags['--root'] : flags['-r'];
    

    :271 就是那个覆盖点。守卫自己的两个读者也如卡片所说::303 只判 --changed 的存在性,:357 取 --root 的值,两者都不会合法地重复出现 —— 所以卡片"守卫今天不受影响"的判断成立。

    触发条件也已经在树上:

    package.json:25   "test:integration": "vitest run --project dom --project dom-heavy",
    

    一字不差。喂给 parseVitestArgv,--project 会答 dom-heavy 并静默丢掉 dom。

    ⚠️ 对卡片 dedupe 段的一处修正

    卡片写 "the dev's reading was that the guard is its only consumer"。实测的导入者是三个:

    scripts/vitest-invocation-guard.mjs                              ← 自身
    scripts/__tests__/vitest-invocation-guard.test.ts                 ← 它自己的测试
    scripts/__tests__/runner-package-test-entry-3746.test.ts          ← ⚠️ 卡片没点名的第三方
    

    "生产代码只有守卫自己在用"成立;但 runner-package-test-entry-3746.test.ts 也在用这个解析器。⛔ 接手者改签名/返回形状前必须先看那个测试在问什么 —— 如果它也在问"这条命令跑哪些 project",那这个隐患已经不是纯潜在的了。卡片让人"先 grep 其他 importer",这里已经 grep 好了。

    定级理由

    priority:p3 —— 采纳卡片自评并复核为真:树上今天没有任何东西是错的,没有调用者通过这个解析器读可重复 flag。

    值得记账而不是关掉的理由,卡片说得准,我复述:一个名字比契约宽的共享 helper(叫"parse vitest argv",实际只表示得了不重复的 flag),分歧只在下一个复用者身上才可见。而 PR #7327 已经用行动证明了这个判断 —— 它的新钉子 scripts/__tests__/package-scripts-vitest-projects.test.ts 自带提取器并带一个"两个值都回来"的控制,而不是复用守卫的解析器。⭐ 有人已经绕开了这个 helper,这是这类隐患最可靠的早期信号。

    定型:tidy,不踩 manual floor

    不改门禁强度、不改授权面、不改任何命令的行为。

    ⛔ 给执行席的边界

    1. 卡片给的两条路都可接受,但第一条(累积重复 flag)优于第二条(只写文档劝退):fix(test): point test:integration at projects that exist, and pin the script/config agreement #7327 已经出现了一个绕开者,再来一个绕开者就是第三份提取逻辑。修契约比记录契约窄更能止血。
    2. 若走第一条,保持向后兼容:--root / --changed 两个现有读者拿的是标量,⛔ 别把返回值统一改成数组把它们弄坏。卡片提的 "last-wins plus a list" 正是为此,照做。
    3. 钉子照卡片指定的形状写:--project a --project b 两个值都要回来。⛔ 一个只断言"不为空"的测试抓不住这个 bug(今天它也返回非空)。
    4. ⛔ 不要顺手把 package-scripts-vitest-projects.test.ts 里那个自带提取器删掉改成复用 —— 那是另一个 PR 的验收物,且它的独立性正是当前唯一的保护。等这个解析器修好并有钉子之后,再由那张卡的所有者决定要不要合并。

    ⛔ 分诊席不认领、不派单、不写码。


    Generated by Claude Code

  3. self-assigned this
    on Sep 6, 2026
  4. baozhoutao commented on Sep 6, 2026

    @baozhoutao
    Contributor

    Claim: PM loop round R45
    Session: session_01MM7kaS4dPpYHV5BsMyu4tQ
    Branch: claude/issue-7329-parse-vitest-argv-repeated-flags
    Worktree: objectui-issue-7329
    Domain: domain:devx
    File surface: scripts/vitest-invocation-guard.mjs (parseVitestArgv only — last-wins scalar kept for the existing --root / --changed readers PLUS a repeated-flag list, per triage's boundary 2) and scripts/__tests__/vitest-invocation-guard.test.ts (the pin: --project a --project b yields BOTH values; a non-empty-only assertion is not the pin); ⛔ scripts/__tests__/package-scripts-vitest-projects.test.ts's own extractor stays (another PR's acceptance artefact, triage boundary 4); ⛔ scripts/__tests__/runner-package-test-entry-3746.test.ts is a READER to keep green, not a surface — read what it asks the parser before changing any return shape (stop on breach; explain in the report)
    Container & model: S, not mechanical (backward-compatible return shape is a small design), mode:subagent, model: opus — no path-derived tier mandate on scripts/** (floor sonnet · default opus · ceiling fable); default judgment tier taken
    Clause-②: no
    Serial constraints cleared: premise on origin/main 15789d5 at 07:49Z — parseVitestArgv at :245, the overwrite at :271 flags[token] = next, root package.json:25 "test:integration": "vitest run --project dom --project dom-heavy"; importers exactly the three triage named (the guard, its test, runner-package-test-entry-3746.test.ts); no open PR touches the guard or its test (file lists read 06:27Z; #7685's 67 paths over git; PRs opened since by this seat are on other surfaces); this seat's in-flight batch (#5174 b17 in packages/plugin-detail, #7855 in apps/console + a new scripts/__tests__/ pin file) is disjoint. pm:queue → pm:dispatched + assignee written in one stroke and read back.


    Generated by Claude Code

  5. baozhoutao commented on Sep 6, 2026

    @baozhoutao
    Contributor

    os-dev-report

    {
      "issue": 7329,
      "status": "done",
      "branch": "claude/issue-7329-parse-vitest-argv-repeated-flags",
      "pr": "https://github.com/objectstack-ai/objectui/pull/8002",
      "premise_still_valid": true,
      "summary": "Route 1 taken per triage boundary 1. `parseVitestArgv` keeps `flags` byte-for-byte as it was (last-wins scalar per flag) and gains a sibling `flagValues` map holding every occurrence of every flag in argv order. Shape chosen over an array-valued `flags` because backward compatibility becomes structural rather than careful: no existing reader's expression changes and none CAN be broken, since the type of `flags` never moved; an array-or-scalar union would have pushed an `Array.isArray()` fallback into every call site, the lenient-fallback shape AGENTS.md #0.1 argues against. Named `flagValues` rather than `repeated` because it holds singly-occurring flags too, as one-element arrays, so a reader never needs `flagValues[x] ?? [flags[x]]`. All three recording sites (the `--flag=value` form, the space-separated value form, the bare boolean) now route through one `record(name, value)` helper that writes both views, so they cannot drift; previously they were three independent assignments. Zone 2 A1 and A3 verified verbatim on 15789d5. A2 SHARPENED, and the sharpening resolves the other way than the dispatch's live branch: the third importer `runner-package-test-entry-3746.test.ts` asks the parser for `flags['--root'] ?? flags['-r']` (:133) and `positionals` (:193) — neither is a repeatable flag — so the defect stays LATENT exactly as the card graded it, not live. That reader was left untouched and is green.",
      "tests": "Reverse verification in two legs, because the cheap leg is weak on its own. LEG 1, the pin run against the unmodified parser before the fix: `Tests  2 failed | 35 passed (37)`, exit 1 — but the failure was `TypeError: Cannot read properties of undefined (reading '--project')`, which proves the field is ABSENT, not that the pin detects the DEFECT. LEG 2, value-level ablation on the committed implementation (commit 92b4602 first, so restore has a target): `flagValues` kept, only the accumulation broken — `(flagValues[name] ??= []).push(value)` replaced by `flagValues[name] = [value]`, i.e. last-wins in both views. On-disk proof before reading any result: injected spelling grep -c = 1, deleted spelling grep -c = 0, blob 27e24f8aa5fb165b7af42344e0ddc61b692f6d55 to 57d4435145d4a6617f2502630e4e8395c53f3cad (differs and non-empty, asserted). Result: `AssertionError: expected [ 'dom-heavy' ] to deeply equal [ 'dom', 'dom-heavy' ]`, `Tests  1 failed | 36 passed (37)` — the card's defect verbatim. Restored under an EXIT/INT/TERM trap with `git checkout HEAD -- ABSPATH`; restored blob back to 27e24f8aa5fb165b7af42344e0ddc61b692f6d55, identical to `git rev-parse HEAD:scripts/vitest-invocation-guard.mjs`, `git diff HEAD` zero bytes, `git status --porcelain` empty. GREEN after fix: the three importer files `Test Files  3 passed (3)` / `Tests  52 passed (52)` (exit 0); whole scripts/__tests__ suite `Test Files  107 passed (107)` / `Tests  3247 passed (3247)` (exit 0). All exit codes captured by redirect-then-capture, never across a pipe.",
      "gates": {
        "vitest scripts/__tests__/ (whole suite)": "PASS — exit 0; `Test Files  107 passed (107)` / `Tests  3247 passed (3247)`",
        "vitest (the 3 importer files)": "PASS — exit 0; `Test Files  3 passed (3)` / `Tests  52 passed (52)`",
        "pnpm type-check:scripts": "PASS — exit 0, tsc -p tsconfig.scripts.json emitted no diagnostics",
        "pnpm lint:root": "PASS — exit 0; `✖ 32 problems (0 errors, 32 warnings)`. 0 errors as required. Per-file `--format json` over the same 254 linted files: scripts/vitest-invocation-guard.mjs errors=0 warnings=0; scripts/__tests__/vitest-invocation-guard.test.ts errors=0 warnings=1, and that one warning is PRE-EXISTING — `VITE_CONFIG_NAME` unused, at origin/main line 282, shifted to line 307 by exactly this diff's 25 added lines.",
        "pnpm check:control-bytes": "PASS — exit 0; `✅  check-control-bytes: OK (scanned 6439 tracked text file(s); skipped 85 binary).` Plus a manual sweep of the two changed files for control bytes: no hits.",
        "node scripts/check-changeset-presence.mjs": "PASS — exit 0; `Compared the working tree with 15789d565 (merge-base with origin/main): 2 file(s) changed, 0 of them published source of a package the release covers, 0 of them a manifest whose published contract moved, 0 under a package changesets ignores, 0 changeset(s) added.` then `✅  No source or published contract of a released package changed in this range, so no changeset is owed.` NONE OWED, as the dispatch expected. No `skip-changeset` label applied — per the os-dev standard clause that label is never applied in objectui, and this gate's verdict line is the authority.",
        "node scripts/check-governed-queue-guard.mjs --test (2 paths)": "PASS — exit 0; `✅ NOT GOVERNED — 2 path(s) checked against 5 governed surface(s); none matched.` then `An ordinary pull request: the normal review and merge-queue route applies.` So no maintainer-speed-read section is owed on the PR body.",
        "node scripts/check-node-esm-load.mjs": "NOT RUN — OUT OF THIS DIFF'S SCOPE, with evidence rather than assumption. Its main() grades `discoverPackages(root)` filtered to non-private manifests, against `MIN_PACKAGES = 30` published workspace packages and their dist artifacts; its docblock opens 'Every published ESM package must be loadable by Node's own resolver.' `scripts/` is not a workspace package and this diff publishes nothing, so the gate reads none of it.",
        "CI": "in_progress — reported at draft-PR time per the dispatch contract; CI convergence is the PM's to read. `Live E2E (informational)` is red on every branch today for an upstream reason (#7990 / objectstack#16186) and is not this diff's."
      },
      "line_budget": "n/a — no `skills/**` path touched, so no published-skill line ratchet applies. For the record the diff is +59 / -5 over 2 files: scripts/vitest-invocation-guard.mjs +34/-5 (23 of the added lines are the docblock that states the two-view contract), scripts/__tests__/vitest-invocation-guard.test.ts +25/-0.",
      "deviations": [
        "Reverse verification was done in TWO legs instead of one. The prescribed single leg (pin RED against the unmodified parser) passed but is weak: it failed with `TypeError: Cannot read properties of undefined`, which only proves the new field is absent. A second value-level ablation was added — field kept, accumulation broken — which fails with `expected [ 'dom-heavy' ] to deeply equal [ 'dom', 'dom-heavy' ]`, the actual defect. Both legs are reported; the second is the load-bearing one.",
        "`node scripts/check-node-esm-load.mjs` was not run. The dispatch said 'if it covers scripts/ (say)' — it does not; the scope evidence is in `gates`. Nothing else in the dispatch's gate list was skipped or narrowed.",
        "Zone 2 A2 resolved opposite to the dispatch's live branch. The dispatch said 'if it reads a repeatable flag, this defect is live, not latent — report that as a sharpened premise'. Measured: the third importer reads only `--root` / `-r` (scalar) and `positionals`, so the defect is still latent and the card's own `finding` grading stands. `premise_still_valid` is therefore true — the premise held in full, including the part the dispatch flagged as possibly stale."
      ],
      "files_changed": [
        "scripts/vitest-invocation-guard.mjs (+34/-5) — `parseVitestArgv` only: new `flagValues` map, new `record()` helper writing both views, three assignment sites routed through it, return shape extended, docblock and @returns updated. No other export touched; `cliHasTestFilters` and `evaluateVitestInvocation` unchanged.",
        "scripts/__tests__/vitest-invocation-guard.test.ts (+25/-0) — two new cases inside the existing `parseVitestArgv` describe. Nothing existing edited or deleted; the 35 pre-existing cases in this file are untouched and green."
      ],
      "open_questions": [],
      "readers": [
        "scripts/vitest-invocation-guard.mjs — itself, twice. `:332` reads `flags['--changed']` for EXISTENCE inside `cliHasTestFilters`; `:386` reads `flags['--root']` / `flags['-r']` for its VALUE inside `evaluateVitestInvocation`. Both scalar, neither legitimately repeated, both byte-for-byte unaffected.",
        "scripts/__tests__/vitest-invocation-guard.test.ts — the parser's own test; the new pin lands here. Reads `flags`, `positionals`, `subcommand`, `afterDoubleDash` and now `flagValues`.",
        "scripts/__tests__/runner-package-test-entry-3746.test.ts — the third importer triage named and the card missed. `:133` `flags['--root'] ?? flags['-r']`, `:193` `positionals`. NOT a repeatable-flag reader, so untouched and green; this is what keeps the defect latent rather than live.",
        "scripts/__tests__/package-scripts-vitest-projects.test.ts is NOT an importer — it carries its own extractor and is left exactly as it was per triage boundary 4. `git grep -l parseVitestArgv` re-run on the branch still returns exactly the three files above."
      ],
      "out_of_scope_findings": [
        "filed as #8001 (labels: finding, tooling, domain:devx, unassigned): scripts/__tests__/vitest-invocation-guard.test.ts declares `VITE_CONFIG_NAME` with a comment naming the 'no vitest.config.*, so Vitest falls back to vite.config.*' case, then never sweeps for it — grep -c = 1, its own declaration. The un-swept case is the exact mechanism of objectui#3746, which this repo already paid for. Not fixed in this PR: different defect class, so it fails the bounded-in-place-fix test. Dedup: search_issues reads in this session (control query returned #7329's own title, total_count 12), two targeted searches, no duplicate."
      ],
      "mcp_calls": "7"
    }

    Generated by Claude Code

  6. baozhoutao commented on Sep 6, 2026

    @baozhoutao
    Contributor

    ACCEPT — PR #8002, head 92b46023; flip and arming on CI convergence

    domain:devx @ objectui execution seat, PM session session_01MM7kaS4dPpYHV5BsMyu4tQ, R45, 2026-09-06T08:03Z. Report 5557893326 reviewed against GitHub and the tree, ⛔ not against its narrative.

    item reading (mine)
    PR shape draft ✅ · base main ✅ · first line Fixes #7329 ✅ · 2 files ✅
    The fix, read (+34/−5 in the guard, non-comment lines) parseVitestArgv gains flagValues and a record(name, value) helper that writes both views; the three assignment sites (--flag=value, space-separated value, bare boolean) route through it; the return adds flagValues. flags stays byte-for-byte last-wins, so --root / -r / --changed readers are structurally unaffected — the shape triage's boundary 2 asked for, chosen over an array-or-scalar union that would have pushed Array.isArray fallbacks into call sites
    The pin, read (+25) --project dom --project dom-heavy → flagValues['--project'] equals ['dom', 'dom-heavy'] (order) while flags['--project'] is 'dom-heavy'; a once-seen flag is a one-element array (['a'], ['1/4'], [true]), an absent flag undefined — the "both values" assertion, not a non-empty one
    Reverse verification leg 1 (pin vs the unmodified parser) red only with a TypeError — correctly judged weak; leg 2 (field kept, accumulation broken) red with expected ['dom-heavy'] to deeply equal ['dom', 'dom-heavy'] — the card's defect verbatim; mutation and restore proven by blob hash
    A2 sharpened the third importer (runner-package-test-entry-3746.test.ts) reads --root/-r and positionals only — the defect stays latent, the finding grading stands; that reader is untouched and green
    Gates three importer files 52/52 · whole scripts/__tests__/ 107 files / 3247 tests · type-check:scripts · lint:root 0 errors (the one warning on the test file is pre-existing, shifted by the diff) · control-bytes · changeset-presence none owed · governed guard NOT GOVERNED ×2 · node-esm-load correctly out of scope (published packages only)
    Boundary 4 package-scripts-vitest-projects.test.ts's own extractor untouched ✅
    Out of scope #8001 filed (VITE_CONFIG_NAME declared and never swept — the #3746 mechanism)
    CI on 92b46023 at 08:02Z 18 success, 3 skipped, 8 in progress — an honest reading

    On all-green: standing-down note if the informational lane reports red, flip ready, wait for the post-flip guard, arm, confirm added_to_merge_queue. On MERGED: content probe on re-fetched origin/main (flagValues and record( in the guard; the "keeps EVERY value" case in its test; control — export function parseVitestArgv still present), card closes via Fixes, strip pm:dispatched, clear assignee.


    Generated by Claude Code

  7. baozhoutao commented on Sep 6, 2026

    @baozhoutao
    Contributor

    Armed. PR #8002 flipped ready 08:18:03Z on all-green-but-the-base-red lane (standing-down note 5557930971, anchor #7990); post-flip guard success (30 runs); auto-merge enabled; added_to_merge_queue 08:18:50Z read from the REST timeline. Landing stroke on MERGED per the ACCEPT above.


    Generated by Claude Code

  8. removed their assignment
    on Sep 6, 2026
  9. baozhoutao commented on Sep 6, 2026

    @baozhoutao
    Contributor

    LANDED — PR #8002 merged 2026-09-06T08:34:15Z (queued 08:18:50Z). Verified on origin/main 023f8ce by CONTENT, ⛔ not by the merge event.

    domain:devx @ objectui execution seat, PM session session_01MM7kaS4dPpYHV5BsMyu4tQ, R45, 2026-09-06T08:40Z.

    reading expected got
    flagValues in scripts/vitest-invocation-guard.mjs ≥ 1 7
    const record = (the two-view helper) 1 1
    the keeps EVERY value of a repeated flag case in the guard's test 1 1
    control — export function parseVitestArgv still present 1 1 ✅ probe fires

    Card closed via Fixes on merge; this stroke strips pm:dispatched and clears the assignee in one write. Follow-on with triage: #8001 (VITE_CONFIG_NAME declared and never swept in the same test file).


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repofindingpriority:p3tooling

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions