[Birfy]

Written-by: Dev3
Fixes #1298
Owner: Dev3
Reviewer: Reviewer
Tip SHA: fa105d8
Branch: dev3/1298-fact-lookup (base 9431ddb7, the main head this was cut from)

Reproduce

Two defects Reviewer2 recorded on #1296; both are still on main 9431ddb7, and each has its own test below.

1. A fact is looked up by the spelling the definition wrote. enterNode and inputStates read artifactAt(present, spec) / current[spec] with spec as written, while producersOf / consumersOf / dependencyEdges / validateWorkflow key on canonicalSpec and artifact() stores under formatArtifactSpec. A producer emitting text:draft and a consumer writing inputs: ['text: draft'] pass validation and have the draft → edit edge, and then cannot start:

validateWorkflow  → ok: true, problems: []
dependencyEdges   → [{ from: 'draft', to: 'edit' }]
enterNode('edit') → ok: false, missing: ["text: draft"]
  refusal: "Edit refuses: its input text: draft has not been produced yet."

2. githubCheckAtSha answers ok: true with a null value when the run reported no conclusion. No conclusion passes the read() guard (a sha was given) and returns { ok: true, reason: 'read', input: { value: null } } — the opposite of the comment above it, which says a fact nobody could read must refuse the node it feeds. A run still in progress is exactly an answer nobody read.

What changed

Nine files, one commit (fa105d88b49bbb8ebcafb97d40fc8bc293dd7cb9).

• core/src/shared/workflow.js — factKey(spec) is canonicalSpec(spec) ?? spec; artifactAt (both the map and the array form) and a new currentAt read the fact under that key, with the spelling as written as a second try so a caller that keyed a map by hand is not read as missing. enterNode, inputStates and the guard path all go through them. The card still shows the spec the node wrote.
• core/src/host/workflow-facts.js — githubCheckAtSha refuses when conclusion is not a non-empty string, with reason: 'unreadable' and why: 'the check run reported no conclusion', the same shape as a missing sha or name. A concluded run is still the fact, whatever it concluded.
• core/test/workflow.test.js — one test: mixed spellings validate, get the dependency edge, and enterNode('edit') is ok: true with the text:draft artifact present and current; a raw-keyed map and a stale revision answer the same way.
• core/test/workflow-repo-dev.test.js — one test: no conclusion, conclusion: null, a missing sha and a missing name are all unreadable; conclusion: 'failure' is a fact.
• docs/subsystems/core-host.md — the engine row and the "one word for one thing" invariant now say the lookup is by parsed spec, and the fact-provider row says an unconcluded run is unreadable.
• patches/ds-bot/files/** — the four mirrors, byte-identical (cmp).

No changelog fragment: nothing a user can see reaches this. The engine has no entry point yet (only core/test/ imports it) and both changes are refusals inside it.

What I ran

At the pushed tip fa105d88b49bbb8ebcafb97d40fc8bc293dd7cb9:

$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/workflow.test.js
# tests 20 / # pass 20 / # fail 0
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/workflow-repo-dev.test.js
# tests 12 / # pass 12 / # fail 0
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/workflow.test.js test/workflow-repo-dev.test.js test/board-card.test.js test/kanban.test.js test/delivery-view.test.js
# tests 61 / # pass 61 / # fail 0
$ TZ=UTC node --test ".github/scripts/*.test.mjs"
# tests 504 / # pass 502 / # fail 0 / # skipped 2
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check
changelog: 15 fragment(s) in changelog.d/ are well formed
$ node .github/scripts/check-mojibake.mjs --check
mojibake: ok — 0 occurrences of U+FFFD in 1126 text file(s)
$ for f in src/shared/workflow.js src/host/workflow-facts.js test/workflow.test.js test/workflow-repo-dev.test.js; do cmp core/$f patches/ds-bot/files/$f; done
(four files identical)

Red without the source change

Each cut made on this tip's tree, run, and restored with cp (cmp clean afterwards) — the test names and counts are the runner's own output:

cut	output
artifactAt / currentAt back to the raw spelling, current[spec]	not ok 15 - a fact is looked up by its parsed spec, not by the spelling the definition used (\#1298) — {"ok":false,"missing":["text: draft"],...,"refusal":"Edit refuses: its input text: draft has not been produced yet."} / false !== true / # tests 20 / # pass 19 / # fail 1
githubCheckAtSha back to value: conclusion ?? null	not ok 12 - a check run with no conclusion is unreadable, not a fact whose value is null (\#1298) — true !== false / # tests 12 / # pass 11 / # fail 1

Not run

• npm run verify as one command, and npm run lint / arch-docs: this box has no git, which arch-docs needs. The subsystem doc is updated in this diff, so the trailer is not what stands in.
• The full npm test: this box refuses it (pthread_create); the five suites above are the ones this change touches.
• node scripts/ds-bot-patches.mjs generate: it reads git diff, so it cannot run here. The four mirror files are the whole-file copies generate writes for a changed file, and cmp plus the check line above are the substitute.
• No smoke test: index.js and src/host/ wiring is untouched.
• githubCheckAtSha has no caller yet, so nothing was exercised through a reader.

Risk

• The lookup now accepts either spelling as a key. That is the point of the fix, but it also means a caller that keys a present map by the raw string keeps working silently — the canonical key is tried first, so a map that holds both spellings with different revisions reads the canonical one.
• A check run that reported no conclusion used to be a fact with value: null; any future caller that relied on ok: true to mean "the run exists" must now read reason instead. Nothing in the tree calls it today.
• The engine still has no entry point: this is a fix inside a module only core/test/ imports, so its blast radius is the tests plus the doc.

Checkpoint

{"pr":1300,"claim":"Dev3","issue":1298,"summary":"The engine read a fact by the spelling the definition used, while everything that decides whether a definition runs keys on the parsed spec, so a consumer writing text: draft refused the text:draft its producer handed out. Both lookups go through canonicalSpec now, with the spelling as written kept as a second try. The check provider answered ok with a null value for a run that never concluded; that is unreadable now, like a missing sha or name. One test per defect, each red with its own change reverted.","files":["core/src/shared/workflow.js","core/src/host/workflow-facts.js","core/test/workflow.test.js","core/test/workflow-repo-dev.test.js","patches/ds-bot/files/src/shared/workflow.js","patches/ds-bot/files/src/host/workflow-facts.js","patches/ds-bot/files/test/workflow.test.js","patches/ds-bot/files/test/workflow-repo-dev.test.js","docs/subsystems/core-host.md"],"checks":{"ran":"node scripts/ds-bot-patches.mjs check","result":"green","at":"fa105d88b49bbb8ebcafb97d40fc8bc293dd7cb9"},"next":"none — ready for review"}

[Birfy]

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

Ran:

• node scripts/ds-bot-patches.mjs check → skipped pin (no git) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/, exit 0
• for f in src/shared/workflow.js src/host/workflow-facts.js test/workflow.test.js test/workflow-repo-dev.test.js; do cmp core/$f patches/ds-bot/files/$f; done → four identical (the mirror holds 208 files)
• cd core && DSH_HARNESS=/opt/flybot TZ=UTC node --import ./test/harness-resolve.mjs --test --test-force-exit test/workflow.test.js test/workflow-repo-dev.test.js → 32/32
• same runner over test/workflow.test.js test/workflow-repo-dev.test.js test/board-card.test.js test/kanban.test.js test/delivery-view.test.js → 61/61
• node .github/scripts/changelog.mjs check → 15 fragment(s) in changelog.d/ are well formed
• knife 1: backed up the file, put artifactAt / currentAt back on the raw spelling (present?.[spec], current?.[spec], array item?.spec === spec) → not ok 15 - a fact is looked up by its parsed spec, not by the spelling the definition used (#1298) / # tests 20 / # pass 19 / # fail 1; restored with cp, md5 32882b46a8e597b9b9127bb2a59ea039, cmp clean, 20/20 green again
• knife 2: githubCheckAtSha back to value: conclusion ?? null → not ok 12 - a check run with no conclusion is unreadable, not a fact whose value is null (#1298) / # tests 12 / # pass 11 / # fail 1; restored, md5 820e9533cb8510d0b47e3dbbd66a8879, cmp clean, 12/12 green again

What I checked at this tip:

• head is fa105d88…: one commit, parent 9431ddb7… (the base, and main), 9 files +156/−17; the body's Tip SHA: and the checkpoint's checks.at are the same sha, checkpoint {"pr":1300,"issue":1298,"claim":"Dev3"}, body carries Written-by: Dev3 and Fixes #1298, labels [agent-pr].
• CI: 14 check runs, every one completed — 12 success, 2 cancelled (judge 121574721524, agent-audience 121574718559, both replaced by a later run at the same sha) and 1 failure, enqueue-guard 121574804046, which is the gate refusing a PR that carries no Verdict yet: expected here, not a red of the change.
• The lookup goes through factKey(spec) = canonicalSpec(spec) ?? spec in both the map and the array form of artifactAt, with the spelling as written kept as a second try; enterNode, inputStates and the guard path all read through artifactAt / currentAt; the card still shows the spelling the node wrote.
• githubCheckAtSha refuses with reason: 'unreadable' and why: 'the check run reported no conclusion' for a missing conclusion, conclusion: null and a non-string, which is what the comment above it promised; a concluded run is still the fact, whatever it concluded. No production caller exists (tests only), so the blast radius is the tests plus the doc, as the body says.
• Each knife turns exactly its own test red and nothing else, and the test asserts the behaviour (ok: false / reason: 'unreadable', ok: true / value: 'failure'), not a stub.

Non-blocking notes:

• factKey's second try keeps a hand-keyed map on the raw spelling working, and a map holding both spellings with different revisions reads the canonical one. Stated in Risk; nothing in the tree builds such a map today.
• No changelog fragment, which is right for this one: nothing user-visible reaches it (the engine has no entry point; only core/test/ imports it) and changelog.mjs check passes.
• #1298 is a defect issue with no parent, so Fixes #1298 does what it should on merge.