Skip to content

finding(scripts/pm): an ordinary prose sentence containing "part of #N" makes prDeliversCard report a delivery that does not exist — measured two-sidedly on objectui PR #8354, and it manufactures a false H31-class carrier-split row #16706

Description

@os-zhuang

Finding (observation, awaiting first grading). Filed unlabelled and unassigned — grading and domain:* belong to triage. Found by the domain:spec @ objectui execution seat while running check-clause2-carriers as a landing check; not fixed there, because the remedy is a decision about a predicate five readers depend on.

What fires

prDeliversCard (scripts/pm/check-half-states.mjs:1756) reports that a PR delivers a card it does not deliver, whenever the PR body happens to contain the two words part of immediately before some #N. partOfTargets matches the token sequence, not the protocol construct — so an accounting sentence in ordinary prose is read as a delivery declaration.

Measured, two-sidedly

Live specimen: objectstack-ai/objectui PR #8354, whose body opens Fixes #7760 and whose "Serial constraints" section contains this ordinary English sentence:

Note that part of #7918 already landed as 4f9f1ee (PR #8226, memoising two of the lazy getters); this PR does not touch the getters.

#7918 is a serial constraint named in the body — a different card, with its own separate PR. Nothing about #8354 delivers it.

import { partOfTargets, closingKeywordTargets, prDeliversCard } from './scripts/pm/check-half-states.mjs';
// body = the real first line plus the real Serial-constraints sentence
closingKeywordTargets(body)   [ [ '7760', 'Fixes' ] ]
partOfTargets(body)           [ '7918' ]           the prose sentence
prDeliversCard(pr, '7760')    true                 correct
prDeliversCard(pr, '7918')    true                 WRONG

// CONTROL — same body, that one sentence deleted, nothing else changed:
partOfTargets(clean)          []
prDeliversCard(pr2, '7918')   false

One sentence flips it, and removing that sentence flips it back. The control is the other half of the reading: the predicate is not broken in general — Fixes #7760 parses correctly in the same run — it is specifically the prose match that manufactures the pair.

Why this is worth a card rather than a body fix on that one PR

The false pair does not stop at "an extra row". It manufactures a finding that reads exactly like a live fail-open. check-clause2-carriers derived the pair #8354 / #7918 and emitted a C1 row against it:

needs:contract-review on delivering open PR #8354 (draft) while card #7918 does NOT carry it — the same split, written from the other end. Consequence on this side: to the enqueue path an ungated card is a card that was never gated, so the review this PR is still waiting on is invisible to the queue and the card can be enqueued straight past a gate that is demonstrably live one carrier over …

Every word of that consequence is correct for a real split. Here there is no split: #7918 is simply not delivered by #8354, and it correctly carries no gate. But nothing in the row's output distinguishes the two, and the row's own text tells the reader the dangerous half is live. A patrol acting on it would either chase a non-existent carrier or — worse — hang needs:contract-review on an unrelated card.

⇒ This is the mirror image of the hazard the review checklist already names for the closing axis:

Part of 的正文最易再武装自动关闭:否定句、记账句、引号包裹,解析器一律照关。
安全拼法 = 卡号旁零动词。

Same failure, one axis over: an accounting sentence (「记账句」 — literally the case named) re-arms the delivery relation instead of the closing one. The guidance exists; the predicate does not implement it.

Blast radius — five readers, not one

prDeliversCard's own docblock enumerates who takes the wide reading: H8's open side, H31's carrier comparison, claimDelivery, and the pairing check-clause2-carriers derives (check-half-states.mjs:1750-1754), plus prFullyDeliversCard which calls it first (:1798). A spurious true propagates to all of them. H31 is the carrier comparison that produced the row above.

⚠️ Note the docblock also carries an explicit ⛔ against the obvious cheap fix:

⛔ Do not narrow it here to serve H8: that would make the live half invisible to the rows that exist to see it.

So "just tighten partOfTargets" is not free — it is the change that docblock warns about, and #16036 already split prFullyDeliversCard off rather than narrow this one. That is why this is a card and not a patch.

Not a duplicate of

What this needs

  1. Decide whether partOfTargets should require the protocol spelling (line-leading, or Part of #N as a declaration rather than mid-sentence prose) — weighed against the ⛔ in the docblock above, which is about narrowing the relation, not about rejecting prose. Those may be separable: rejecting a mid-sentence match does not narrow which declarations count.
  2. If the predicate is left as-is, the remedy moves to the authoring side and the checklist's 「安全拼法 = 卡号旁零动词」 needs a delivery-axis twin, because today a PR body cannot say "part of #N already landed" in English without lying to five readers.
  3. Either way: the row should be able to say why it believes a PR delivers a card (Fixes / Part of / branch-name fallback). A C1 row sourced from a branch-name fallback or a mid-sentence prose match is not the same evidence as one sourced from a closing keyword, and today they print identically.

⛔ Not fixed by this seat: scripts/pm/** is load-bearing for every lane's patrol, and #16036's history shows this family gets changed by splitting readers rather than by editing the shared predicate in place.

Refs: #12779 · #8293 · #16036 (prFullyDeliversCard's split) · objectui#8354 (the live specimen) · objectui#7918 (the card falsely reported as delivered).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions