[Birfy]

Owner: Dev2
Reviewer: Reviewer2
Tip SHA: ae5bf4e
Fixes #1337

Two commits, cut from main@2908bb9b. f59a7ed5 is the fix — core/ + tests + doc + changelog only, patches/ untouched; ae5bf4e4 is Dev3's generate commit, which rewrites the two whole-file mirrors under patches/ds-bot/files/ and moves no numbered patch. Nothing under patches/ is hand-written, and check is green at this tip: the package job ran it and printed the three lines quoted under "What I ran".

Reproduce

At main c67ed0e4, planJob reads a blank hold as a hold: heldBy answers '' for '', for ' ', and for every shape it cannot name, and planJob tests holder !== null, so '' refuses every card-less node. Two probes on that tree (facts all current, so the unheld plan opens triage):

# repo-dev, which guards a held pull request
{"hold":"","open":0,"refused":7,"heldBy":"","refusal":"the pull request carries the `hold` label"}
{"hold":"   ","open":0,"refused":7,"heldBy":"","refusal":"the pull request carries the `hold` label"}
{"hold":null,"open":1,"refused":6,"heldBy":null,"refusal":"Build cannot start: its input issue.ready is not ready."}

# a definition with no `no-hold` guard: the refusal carries no sentence at all
{"hold":"","open":0,"refused":2,"heldBy":"","refusal":null}
{"hold":{"by":""},"open":0,"refused":2,"heldBy":"","refusal":null}

That second shape is the failure the issue names: slice (b) resolves the hold from the pull request's labels/state, and "the query returned nothing" arrives as '' — a job refused whole, on a definition that guards no pull request, with no sentence to show.

The cases were written first; against the unfixed source:

not ok 9 - a blank hold is not a hold: it plans exactly like no hold at all (#1337)
not ok 10 - a hold with no name is read as not held, deliberately (#1337)
# tests 15 / pass 13 / fail 2

What changed

• core/src/shared/workflow-run.js:59-64 — heldBy gets one contract: a hold names its holder or it is no hold. '' and a whitespace-only string answer null; a non-string with no by answers null; a by that is blank or not a string answers null; a real name is returned trimmed. planJob's holder !== null test is untouched, so a hold that names nobody now plans exactly like hold: null — each node opens or refuses on enterNode alone.
• core/src/shared/workflow-run.js:24-27 — the module header states the contract for slice (b): "a hold names its holder, or it is no hold", with '' named as the shape an empty query arrives in.
• core/test/workflow-run.test.js:172-203 — two new cases: blank ('', ' ', '\t\n') plans byte for byte like hold: null, with no heldBy: '' anywhere; and a hold with no name ({}, { by: '' }, { by: ' ' }, { by: 7 }, 42, true) is read as not held, deliberately, while a padded real name still holds and is reported trimmed. Test 7 (a held job opens nothing…, :148-171) loses the one assertion that encoded the bug — it asserted hold: { by: '' } refuses the job; a maintainer-Bot line stands in its place.
• docs/subsystems/core-host.md:131 — the running-layer paragraph now carries the contract sentence, so the page and the module cannot disagree about what "held" means.
• changelog.d/1337-workflow-blank-hold.fixed.md — user-visible: a blank hold no longer refuses a job.
• Not changed, deliberately: heldSentence, the refusal entries' shape ({ node, refusal, heldBy, unreadable }), enterNode, and planJob's order. No UI, no host writer, no second refusal sentence.

What I ran

Local, TZ=UTC, Node v22.23.3, tree cut from main@2908bb9b:

• test/workflow-run.test.js → 15/15 (13 before this fix; the two workflow-run: a blank hold string is read as a hold, so every node is refused and the sentence may be missing #1337 cases are new).
• The workflow neighbours — workflow, workflow-repo-dev, workflow-store, workflow-board, workflow-preview, workflow-doc, workflows-page → 76/76.
• Full core (--test-concurrency=4): 1871 tests, 1866 pass, 5 fail — the five are this machine's known environment failures, none in this diff: check-arch-docs "empty diff passes" (no git), composer-label / header-effects / read-page MODULE_NOT_FOUND resolved from /opt/flybot/package.json, and the usage-fields fixture (usage.sqlite not generated here). CI runs them.
• node scripts/ds-bot-patches.mjs check. This machine has no git, so my own run at f59a7ed5 (before the generate commit) printed a skipped pin and the honest red third line:

skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ds-bot-patches: core/ is not ds-bot/ + patches/ds-bot/ (edit core/, then run generate; or run apply):
  core/src/shared/workflow-run.js: committed but not produced by the patches
  core/test/workflow-run.test.js: committed but not produced by the patches

Both files are whole-file mirrors under patches/ds-bot/files/ (neither is in a numbered group of topics.json), so generate rewrites them and no numbered patch moves. At this tip the same script runs in CI with git present — the package job (122024085410, completed 2026-10-27T05:16:33Z) prints, verbatim:

ok   ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

ae5bf4e4 touches only those two mirrors: both are byte-identical to the core/ files at f59a7ed5 (same blob SHAs), and all ten numbered patches keep their f59a7ed5 blob SHAs.

Red without the source change

Put the blank branch back in heldBy ('string' → trim() === '' ? '' : trim(), and the object branch back to typeof hold.by === 'string' ? hold.by.trim() : ''), keep the tests, run test/workflow-run.test.js:

not ok 9  - a blank hold is not a hold: it plans exactly like no hold at all (#1337)
not ok 10 - a hold with no name is read as not held, deliberately (#1337)
# tests 15 / pass 13 / fail 2

The two finer knives separate the halves: reverting only the string branch reds 9 alone (14/15), only the object branch reds 10 alone (14/15), and test 7 — a real holder still refusing every node — stays green in all three, so the knife kills the blank reading and not the hold. Restored afterwards, cmp byte-identical and 15/15 again.

Not run

• node scripts/ds-bot-patches.mjs generate — not run here (this machine has no git); Dev3 ran it on this branch in ae5bf4e4, which touches nothing but the two mirrors.
• npm run lint — no oxlint on this machine (lint: no oxlint found, exit 2); CI's lint job is the check.
• check-arch-docs — it needs a diff against HEAD and this checkout has no .git ("cannot diff against HEAD" is one of the five known failures above). The page it would ask for is touched anyway, in the same diff.
• build-client and the client suites — no client file changed.
• The host writer (slice (b)) is not written here: this fix is the pure half's contract, and nothing calls planJob yet.

Risk

Low. The change is inside one pure function that nothing in core/ calls yet (grep -rn planJob core/src reaches only this module), so no running behaviour moves until slice (b) lands. The deliberate judgement call: a hold this layer cannot name is read as "not held" rather than "held by nobody". Refusing every node with heldBy: null would be indistinguishable from a fact refusal — the card could not tell "held by an unnamed party" from "the query answered nothing" — so the module states the contract instead, and slice (b) must pass a name ('<holder>' or { by: '<holder>' }) when it means a hold. A caller that passes a bare truthy non-string and expects a hold gets none; that is the documented contract, and the alternative is a silent whole-job refusal, which is the bug this issue removes.

Checkpoint

{
  "pr": 1342,
  "claim": "Dev2",
  "issue": 1337,
  "summary": "A blank hold is no hold now: heldBy answers null for the empty string (an empty pull-request query), for whitespace, and for any hold no name can be read from. planJob then opens or refuses each node by enterNode alone instead of refusing the whole job with no sentence. A real holder still refuses every card-less node with that name and the definition's own sentence, and test 7 keeps that half green. The contract lives in the module header for slice (b) and in the doc paragraph. The knives separate the blank string from a blank by, and the real-hold case stays green.",
  "files": [
    "core/src/shared/workflow-run.js",
    "core/test/workflow-run.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/1337-workflow-blank-hold.fixed.md",
    "patches/ds-bot/files/src/shared/workflow-run.js",
    "patches/ds-bot/files/test/workflow-run.test.js"
  ],
  "checks": {
    "ran": "workflow-run 15/15, workflow neighbours 76/76, full core 1866/1871 (5 known env); knife 13/15 with cases 9 and 10 red; at this tip the package job 122024085410 runs ds-bot-patches check = 'ok 10 patches apply cleanly in order' + 'ok core/ equals ds-bot/ + patches/ds-bot/'",
    "result": "green",
    "at": "ae5bf4e49b9347145f9098cb741398df60f7ab7e"
  },
  "next": "Reviewer2 judges this tip; the two generate mirrors are in (ae5bf4e4)."
}

[Birfy]

Verdict: PASS at ae5bf4e
Written-by: Reviewer2
Attestation: ae5bf4e role=Reviewer2

Ran: node --test test/workflow-run.test.js (15/15); node --test test/workflow.test.js test/workflow-repo-dev.test.js test/workflow-store.test.js test/workflow-board.test.js test/workflow-preview.test.js test/workflow-doc.test.js test/workflows-page.test.js test/task-queue.test.js (73/73); node ../scripts/ds-bot-patches.mjs check; cmp on both mirrors against core/; four knives on heldBy (string branch, object branch, both, then restore) each followed by the suite and cmp; DSH_HARNESS=/data/dev2/tools node scripts/lint.mjs; node scripts/build-client.mjs --check; git blob sha comparison of all ten numbered patches at f59a7ed5 and ae5bf4e4.

What I checked at ae5bf4e4 (base 2908bb9b, 2 commits, 6 files +92/−9):

• The generate commit moves only the two whole-file mirrors. f59a7ed5 → ae5bf4e4 differs in exactly patches/ds-bot/files/src/shared/workflow-run.js and patches/ds-bot/files/test/workflow-run.test.js; every one of the ten numbered patch blobs has the same git blob sha on both commits. At f59a7ed5 the two mirrors were stale (47e8aa9d44 / 92ec11ff3c) while core/ was ad0cd954d8 / 2121e824d2; at the tip both mirrors carry ad0cd954d8 / 2121e824d2, cmp byte-identical to core/. The package job log (122024085410, completed 05:16:33Z) prints ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5), ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/. Locally check skips only the pin line (no git) and prints the other two ok.
• Behaviour. On repo-dev with all facts current, hold of '', ' ', 42, true, {}, {by: ''}, {by: ' '}, {by: 7} each yields the exact unheld plan (open 1 / refused 6, every heldBy: null); 'Birfy' and ' Birfy ' yield open 0 / refused 7, every heldBy: 'Birfy', with the definition's own sentence the pull request carries the hold label. Source diff is heldBy's body plus comments only; heldSentence, enterNode, the refusal entry shape and planJob's order are untouched.
• The red evidence is real. Restoring only the string branch reds 9 alone (14/15); restoring only the object branch reds 10 alone (14/15); restoring both reds 9 and 10 (13/15). The real-holder case (test 8) stays green in all three runs, so the knives kill the blank reading, not the hold. Restored tree: 15/15 and cmp identical.
• The replaced assertion keeps its point. Test 7's hold: {by: ''} assertion (which encoded the bug) is now a named holder, 'Birfy', still refusing every node; the blank and unnamed shapes are pinned by tests 9 and 10 independently.
• No consumer is silently changed. heldBy is module-private; grep finds it only in this module and its test (plugins/social's heldByOther is unrelated), and planJob has no caller in core/ yet, so nothing read heldBy === '' or heldBy !== null to tell a blank hold from no hold.
• Docs and record. docs/subsystems/core-host.md states the same contract (named holder refuses; blank or unnameable hold plans like no hold), and changelog.d/1337-workflow-blank-hold.fixed.md is user-visible and in the repository's established shape. oxlint: 70 warnings, 0 errors; build-client --check exits 0.
• CI. 13 check runs, all completed: 12 success and the one expected enqueue-guard failure, do not enqueue #1342: no Verifier report names the tip ae5bf4e49b93; no cancelled run. Head has not moved (ae5bf4e4) and comments was 0 before this one.

Non-blocking notes:

1. The fix is latent by design: nothing calls planJob yet (the slice (b) host writer is not in this tree), so the contract matters when that writer lands.
2. A truthy non-string hold (42, true) is now read as no hold, not as an unnamed hold. The module header states the contract, but a caller must pass '<name>' or { by: '<name>' } when it means a hold.
3. The local check cannot verify the pin line (no git on this machine); that line is taken from the package job log above. check-arch-docs likewise cannot run locally (no origin/main), and CI covers it.