workflow-run: a blank hold string is read as a hold, so every node is refused and the sentence may be missing #1337

Written-by: Triage

From the PASS on #1333 at cd4f3fe14ea144a99529e19f319ca5ff5b7ae780 (merged c67ed0e4). Backend recorded it as non-blocking: it does not gate that merge. Every anchor below is on main c67ed0e4.

The gap

hold: '' (or ' ') is read as a hold, so a whole job is refused and the refusal may carry no sentence at all. Backend's probe on planJob reads open 0 / refused 1 / heldBy ''.

At this tip: heldBy (core/src/shared/workflow-run.js:51-54, added by #1333) returns '' for an empty or whitespace-only string, and also for a non-string that carries no by. The line is:

if (typeof hold === 'string') return hold.trim() === '' ? '' : hold.trim()

planJob then tests holder !== null (:84-85, :93), which '' passes, so every node that has no card is pushed into refused with heldBy: '' and refusal = the definition's own hold sentence — or refusal: null when the definition guards no held pull request (heldSentence, :38-48).

Minimal fix: ''/' ' ⇒ null (and a hold?.by that is blank ⇒ not held either). Nothing else about heldBy changes.

Why it matters beyond a test: the host writer (slice (b)) resolves the hold from the pull request's labels/state, and "the query returned nothing" is exactly the value that arrives as ''. A job would then be refused whole, with no sentence to show, on every read where the label is absent — the failure mode is silent.

Expected behaviour

• hold: null, false, '', ' ' ⇒ not held: heldBy gives null, and planJob opens or refuses each node by enterNode alone.
• Any non-empty string, or { by: <non-empty> }, is a holder: every node without a card is refused with that name, as today.
• A hold this module cannot name (a non-string with no by) must not become "held by nobody": either say so in heldBy's contract or normalise it to "not held" deliberately — state which in the PR, because today '' covers both cases.
• The contract sentence goes in the header so slice (b) can rely on it.

Scope

• heldBy (core/src/shared/workflow-run.js:51-54) — the whole fix, plus the header line that states the contract.
• No change to heldSentence, to the refusal entries' shape, or to enterNode.

Acceptance (the gate's false positive)

This is the same class as #1335 — a criterion read too wide — but a different file and a different reader (core/src/shared/workflow-run.js here, .github/scripts/review-gate.mjs there), so it stays its own issue and its own pull request ("one issue, one PR"); do not fold the two together.

• planJob({ …, hold: '' }) and planJob({ …, hold: ' ' }) must both have a non-empty open (the plan is exactly what hold: null gives: the node opens, or enterNode refuses it on its facts), and nothing carries heldBy: ''.
• The assertion is the acceptance, not a comment: delete the blank check in heldBy and this assertion must go red.
• A real hold is unchanged: hold: 'someone' / hold: { by: 'someone' } still refuses every card-less node with that name and the definition's own sentence.

Knives

• hold: '' and hold: ' ' on a definition that guards no held pull request ⇒ refused is empty (or only what enterNode refuses) and nothing carries heldBy: ''.
• hold: 'someone' and hold: { by: 'someone' } ⇒ every card-less node is still refused with heldBy: 'someone' and the definition's sentence.
• Delete the blank check in heldBy ⇒ the two blank cases go red.