Skip to content

Safety-critical UI logic keeps landing in the one tier with no executable coverage — and the stated reason is false #2918

Description

@vybe

The pattern

Two PRs ejected from the 2026-09-20 merge train failed the same way, and three from the previous train failed a near-identical variant. This is one habit, not five bugs, and it is worth fixing once.

The shape: the pure/rules layer gets genuinely good executing tests. The wiring/gate layer gets regex assertions over source text. And the wiring/gate layer is where the safety-critical logic lives.

PR Executing coverage Regex-only coverage What the untested tier held
#2778 focusTrap.js rules (25 tests, mutation-proven) BaseModal.vuenothing at all Esc-to-close, focus trap, focus return, scroll lock
#2756 membership primitive, MCP tool, store (teeth verified) SystemTeardownPanel.vue canRemove, TeardownPreview.vue toggle() the four-gate confirmation on the only verb that deletes a fleet

Both are invisible to CI. Concretely:

The part that makes it fixable

The justification is false. #2778 states it in three separate docblocks: "this repo's vitest runs environment: 'node' with no jsdom/happy-dom and no @vue/test-utils — I checked rather than assumed."

On dev today:

So the harness exists, is configured deliberately, and has 20 precedents. The belief that it doesn't is what routes delete-confirmations and keyboard contracts into regex-only coverage — and once there, the coverage cannot distinguish "written" from "runs".

Worth noting the belief is reasonable to arrive at: environment: 'node' is the default and the opt-in is a comment on line 1 of a spec file. Nothing announces the capability at the point someone is deciding how to test a component.

What closing looks like

  1. Make the capability discoverable at the decision point. A short section in docs/memory/design-system-contract.md (read before any src/frontend/ change) naming the // @vitest-environment jsdom opt-in and pointing at portalThemeSwitch.spec.js as the copyable precedent.
  2. Name the bar in the methodology. The tdd skill and /validate-pr already ask whether a test executes the changed path. Add the specific tell: a spec whose assertions are readFileSync/inspect.getsource/indexOf slices over the implementation proves the code was written. That is acceptable for spelling pins and AST guards, and not acceptable as the only coverage for a predicate that gates a destructive action or a keyboard contract.
  3. Consider a lint. A source-text assertion in a spec whose subject is a .vue file under components/ could warrant a warning, given the harness is available. Needs care — the legitimate uses (bug: a 503 from an exhausted credit balance is laundered into an "acceptable skip" across 32 tests #2889's AST call-site guard, the baseButtonVariantBorder structure pin) are real and should stay.

Why P1

Neither ejected PR was careless work — #2756's author built the store-lift pattern to make logic executable and applied it to teardownDefaultSelection one function away from canRemove. The habit survives good engineers paying attention, which is what makes it a process defect rather than a review miss. The near-miss was a fleet-delete confirmation that would have been enabled with nothing selected.

Found by /merge-train on 2026-09-20. Related: #2778, #2756, and the previous train's #2648 / #2645 / #2619 (backend half works, client half inert, suite asserts source text).

Activity

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

Metadata

Metadata

Assignees

Labels

complexity-highComplexity: high (board points 13)priority-p2Importantstatus-in-devMerged to dev, awaiting release cut to mainstatus-readyGreenlit and ready for development (vetted; counterpart to status-incubating)theme-devexTheme: DevExtype-refactorCode improvement

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions