Skip to content

[finding] check-half-states self-test: 111 bare .includes() on nullable predicate returns turn a mutation into a TypeError that ABORTS the suite instead of a named failing case #12273

Description

@os-zhuang

Finding — recording only, unassigned. Hit live on 2026-08-25 while reverse-verifying #12090 (PR #12272); filed rather than fixed there because it is a different defect class (self-test harness robustness vs. claim-separator visibility) and the right shape is a judgement call, not pinned by existing evidence.

Measured

scripts/pm/check-half-states.mjs's self-test asserts a row's TEXT with the shape

t('H33: …and it names both stamps',h33ClaimPredatesRuling(dispatched33(),[claimRow33,rulingRow33]).includes(RULING_AT),true);

Every one of these predicates is three-valued by designnull when clean, a string when it fires. So .includes(...) on the return value throws TypeError: Cannot read properties of null (reading 'includes') the moment a change makes that predicate go clean.

Counted over the self-test body only (selfTest() through the assertion loop), grouped by predicate:

predicatebare .includes() call sites
h9OnHoldNoRestartWhen14
h14BlockingCacheIncoherent14
h16StuckMergeConflict12
h22ClosedCardPmResidue8
h4BlockedNoBlockedBy8
h8MergedPrStillDispatched8
h18RetriageAged7
h11ImportantParked · h19BlockOutlivedBlocker · h26BlockOnIndefiniteTarget6 each
h13DomainWithoutPmState · h24QueuedWithAssignee5 each
h25AwaitingMaintainerExclusivity · h32SeatIdleOverQueue3 each
h12OrphanLanding · h7PartOfWithClosingKeyword2 each
h10StaleUnclaimedP0 · h33ClaimPredatesRuling1 each
total111 across 18 predicates

What it costs — measured, not hypothesized

Reverse-verifying a new row in this file means mutating a predicate and reading which cases go red. Observed on the first ablation leg (widening CLAIM_COMMENT_MARKER, a mutation that legitimately makes several rows go clean):

TypeError: Cannot read properties of null (reading 'includes')
at selfTest (…/check-half-states.mjs:11117:121)
Node.js v22.22.2

Three separate costs, all in the direction that hides information:

  1. The suite aborts at the first such line. Every case after it never runs, so the run reports nothing about the rest of the file — including cases that would have gone red for the real reason.
  2. The output names a TypeError, not a row.failing cases: 0 is what a grep '✗' count reads, next to a non-zero exit — a shape that reads like a harness problem rather than a predicate verdict, and sends the reader to debug the test harness instead of the change.
  3. It is silently order-dependent. Which cases you learn about depends on which fragile line the mutation happens to reach first.

⚠️ The direction matters: this never makes a red run look green, so nothing has shipped wrong because of it. What it degrades is the ablation discipline this file's own headers lean on repeatedly ("that a check reports nothing today is worth distinguishing from a check that CANNOT report anything") — the cheapest way to prove a new row is failable is exactly the operation this shape breaks.

Shape of a fix — ⛔ not decided here

The one used in PR #12272 for the new row's own cases, as a local precedent rather than a proposal for the other 111:

consth34row=(...args)=>String(h34ClaimShapedNonCanonicalSeparator(...args)??'');

…so a nulled row reports (got false, want true) under the row's own case name and the suite runs to completion. Alternatives triage should weigh: a shared rowText(fn, ...args) helper used by all 18 (one import, but it re-spells 111 lines); or a t() overload that stringifies nullable actuals centrally (touches the harness, not the cases, but makes every case's failure text less literal). Not obvious which is right, and the third option — leave it, on the ground that a crash is loud and the shape only bites during deliberate mutation — is defensible enough that it should be ruled on rather than assumed.

Not measured here: whether sibling patrol scripts under scripts/pm/ carry the same shape in their own self-tests. Worth a companion sweep if this lands.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions