Skip to content

Commit ef5dae4

Browse files
authored
feat(workflow): nodes, artifacts and the flowchart as data (#1272 step 1) (#1291)
Written-by: Dev3 Fixes #1290 Refs #1272 Owner: Dev3 Reviewer: Reviewer Tip SHA: 8f1864f Step 1 of `#1272`, the pure core, so the parent issue stays open: step 2 adds the `repo-dev` preset and the fact providers that mirror `enqueue-guard.mjs` / `merge-gate.mjs`, step 3 adds the Workflows page. `Fixes #1290` names the step-1 child issue for the same reason `#1275` closed `#1279`: one issue, one pull request, and `#1272` is closed by the last step. ## Reproduce On the base of this PR, main `63f81f03`, there is no such file and no such answer: ``` $ ls core/src/shared/workflow.js ls: core/src/shared/workflow.js: No such file or directory ``` So nothing can say whether a node may start, and today's board has no way to refuse a node whose input moved. The three confusions the issue records (#1239's `Verdict: Incorrect` parsed as nothing, a verdict naming an old tip, a `Tip SHA` line naming the previous tip) were each a node started on a stale input that only a repo-side script noticed afterwards. This PR is the function that would have answered: `enterNode(def, 'gate', { present, current })` returns `ok: false` with `Gate refuses: its input verdict@abc1234 is stale, the tip is now def5678.` ## What changed Three new pure modules, no host, no UI, no GitHub: - `core/src/shared/artifact.js` — the artifact vocabulary (`check:tests-ok`, `verdict`, `branch`, `pr`, `mergeable`, `merge-commit`, `release`, `risk:tier`, `doc|file|link|text`), what the revision of each kind is, and `freshness()`: present and current are two questions, and a fact nobody could read is `unreadable` instead of a match. - `core/src/shared/workflow.js` — `validateWorkflow()` (an input nothing produces, an output nothing consumes that is not a deliverable, two producers of one fact, an undeclared cycle, a node with no Bot, a Bot not on the team, an unknown artifact kind, an edge that does not line up — each with its own sentence), `workflowColumns()` (the node ids are the columns; no built-in list), `layoutWorkflow()` (one box per node with its Bot and rails, one edge per `edges[]` row, ranks left to right, computed not stored), `enterNode()` / `inputStates()` / `renderRefusal()` (missing or stale, both revisions named, the node's own `refusal` wording when it has one), `holderOf()` (the node's Bot is the card's assignee). - `core/src/shared/workflow-blank.js` — the `blank` preset as plain data: three nodes, one Bot each, no guards, no repository. Nothing in the engine names its ids, and a test renames them all to prove it. `docs/subsystems/core-host.md` gains the three rows in the shared-module table and one invariant under `## Invariants`. The four `patches/ds-bot/files/` mirrors are the generated whole-file copies, byte-identical to `core/`. No changelog fragment: the engine is not reachable from any surface yet, so a user cannot see it; the Workflows page in step 3 carries the entry (changelog.d/README: "A change that users cannot see needs no fragment"). ## What I ran On the pushed tip `8f1864ff0c6f22843251c2c98137c873f6017bf9` (base `63f81f03`, which is main when this was written): ``` $ node --import ./test/harness-resolve.mjs --test test/workflow.test.js # tests 19 # pass 19 # fail 0 $ node --import ./test/harness-resolve.mjs --test test/workflow.test.js test/board-card.test.js test/kanban.test.js test/delivery-view.test.js # tests 48 # pass 48 # fail 0 $ 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 1119 text file(s) ``` The 19 tests cover: the artifact vocabulary and revisions, present vs current, the `blank` preset against its team, a five-node content workflow through the same calls (no repository anywhere), the flowchart asserted against the definition (one box per node with its Bot and rails, one edge per row, ranks in dependency order, the same layout twice), each validation refusal with its own code and sentence, a declared loop versus an undeclared cycle, the node's own refusal wording, a guard over a verdict value, renamed ids still laying out the same, and the three answers the INC at `38ef59de` asked for: an unread source refuses whether the key is missing or `null`, the sentence names the fact and never fills an empty slot, and the card reads `stale: true` on the same input the gate refuses. ## The INC at 38ef59d `Verdict: INC at 38ef59d` (Reviewer2, comment `6414982598`) found one defect in three places: `current` without a key was treated as "nothing to compare" and the node started. Fixed here, one site each: 1. **`enterNode` no longer skips a missing `current` key** (`core/src/shared/workflow.js:383`): every input goes through `freshness()`, so `current: {}` and `current: { verdict: null }` both answer `unreadable` and the node refuses, the same branch the `is: 'fresh'` guard already took. Test: *a fact whose source revision is unreadable refuses, whether the key is missing or null*. 2. **`renderRefusal` says what it read instead of leaving a hole** (`:302`): the unreadable case has its own sentence — the fact with its revision, that the source revision could not be read, and that the card must be read before the node starts — and `null` never reaches `{now}` or `{have}` (the old path rendered `stale, the tip is now .`). Test: *the unreadable sentence names the fact it read and never fills the source revision with an empty slot*. 3. **`inputStates` no longer reports `stale: false` for an input the gate refuses** (`:324`): it now returns `stale: !fresh` plus `reason` and `unreadable`, so the card and `enterNode` say the same thing. Test: *the card and the gate say the same thing about an unreadable input*. ## Red without the source change Two knives, each cut on the pushed tree `8f1864ff0c6f22843251c2c98137c873f6017bf9` and restored with `cp`: ``` $ # A: enterNode no longer compares the input's revision with the source's not ok 14 - a stale input refuses with both revisions, so a card reads without asking anyone not ok 17 - a fact whose source revision is unreadable refuses, whether the key is missing or null (#1291 INC) not ok 18 - the unreadable sentence names the fact it read and never fills the source revision with an empty slot (#1291 INC) not ok 19 - the card and the gate say the same thing about an unreadable input (#1291 INC) # tests 19 / # pass 15 / # fail 4 $ # B: the cycle check is removed from validateWorkflow not ok 11 - a cycle that is not declared is refused; a declared loop is how work goes back # tests 19 / # pass 18 / # fail 1 ``` Knife A now reds four rather than one because the three INC tests sit on the same three lines; the narrow version of the same cut is the INC defect itself (put the `hasOwnProperty … continue` back), which reds exactly two: *not ok 17*, *not ok 19* (`# pass 17 / # fail 2`). The other two sites have their own knives on the same tree: putting the old `renderRefusal` back reds *not ok 17* and *not ok 18* (`# pass 17 / # fail 2`), and restoring the old `inputStates` `stale` computation reds *not ok 14* and *not ok 19* (`# pass 17 / # fail 2`). With either cut, a stale input would start a node (#1211's sin), and an undeclared loop would validate. Removing the modules themselves is not a useful knife: the test file cannot import them at all, which says nothing about which rule is load-bearing. ## Not run - `npm run lint` (oxlint + `arch-docs`): this machine has no `dev/harness` (no `git`, so `npm run harness` cannot run either), and `arch-docs` needs `git log`. CI's `package` job runs both on the tip. - The full `npm test`: the box refuses it with `pthread_create: Resource temporarily unavailable`; the four suites above are the ones this change can reach, and CI runs the whole suite. - `node scripts/ds-bot-patches.mjs generate`: it needs `git` for the diff of `ds-bot/` against `core/`, so the `files/` mirrors were written whole and compared byte for byte; `check` passes. - **Four review points left as they are, deliberately, this round** (they are not this INC and changing them would widen the diff): `bot` and `role` are still two words for the same thing in the docs; a `guard` with an unknown `is` still defaults to `present` instead of refusing; `producersOf` still keys on the raw spec text rather than the parsed one; and `resolvedEdges` still reports `artifact` and `carries` for the same value. They are listed here so step 2 does not read this half as settled on them. ## Risk The engine is not wired to anything yet, so a defect here cannot block a real card; the risk is the opposite — step 2 may find a shape this half got wrong (a fact that needs two revisions, a node that needs an input it does not declare). `blank` names Bots that no team has, so it validates only against its own three names; the Workflows page must let an owner map them. And a custom workflow may be looser than GitHub's rules: the engine's `ok` is not permission, and step 2/3 must never report a merge or a queue entry the underlying system refused — in GitHub's words, not the engine's. The four review points under `## Not run` are the same kind of residue: an unknown guard `is` reads as `present`, so a typo in a definition is a no-op guard rather than a refusal, and `producersOf` keyed on raw text would let two spellings of one fact both produce it. Both are worth fixing before the `repo-dev` preset starts writing facts. <!-- flybot:checkpoint --> ```json { "pr": 1291, "claim": "Dev3", "issue": 1290, "summary": "A workflow node now has a typed input check: artifact.js keeps an artifact's revision, so present and current are separate questions, and an unreadable fact refuses instead of passing. workflow.js validates a definition with its own sentence per defect, draws the columns from the node ids, lays the flowchart out as a pure function, and refuses a node whose input is missing or stale with both revisions named. The blank preset is plain data, and a five-node content workflow with no repository goes through the same calls. Pure half only: no host, no UI, no GitHub.", "files": ["core/src/shared/artifact.js", "core/src/shared/workflow.js", "core/src/shared/workflow-blank.js", "core/test/workflow.test.js", "docs/subsystems/core-host.md", "patches/ds-bot/files/src/shared/artifact.js", "patches/ds-bot/files/src/shared/workflow.js", "patches/ds-bot/files/src/shared/workflow-blank.js", "patches/ds-bot/files/test/workflow.test.js"], "checks": { "ran": "node --import ./test/harness-resolve.mjs --test test/workflow.test.js test/board-card.test.js test/kanban.test.js test/delivery-view.test.js && node scripts/ds-bot-patches.mjs check", "result": "green", "at": "8f1864ff0c6f22843251c2c98137c873f6017bf9" }, "next": "step 2 — the repo-dev preset and the fact providers" } ``` <!-- /flybot:checkpoint -->
1 parent 63f81f0 commit ef5dae4

9 files changed

Lines changed: 2450 additions & 0 deletions

File tree

‎core/src/shared/artifact.js‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// A workflow node takes in and hands out typed artifacts (#1272). An artifact is a fact with a
2+
// revision, so "is it there" and "is it current" are two questions instead of one: a verdict carries
3+
// the SHA it judged, a check the SHA it ran on, a branch its head commit. The engine compares
4+
// revisions and refuses a stale input; it never invents a revision, and the provider named for a
5+
// kind is what reads the current one. Pure: no I/O, no clock, no GitHub.
6+
7+
export const ARTIFACT_KINDS = [
8+
'issue',
9+
'issue.ready',
10+
'branch',
11+
'pr',
12+
'verdict',
13+
'check',
14+
'risk',
15+
'mergeable',
16+
'merge-commit',
17+
'release',
18+
'doc',
19+
'file',
20+
'link',
21+
'text',
22+
]
23+
24+
// A kind that carries a name (`check:tests-ok`, `text:topic`): two of them on one card are two
25+
// different facts, so an unnamed one is not a spec.
26+
const NAMED_KINDS = new Set(['check', 'risk', 'doc', 'file', 'link', 'text'])
27+
28+
// What the revision of each kind is. The engine compares revisions for equality; this table is
29+
// what a provider reads, and what a node's refusal names when it says the input moved.
30+
export const REVISION_RULES = {
31+
issue: 'the issue number, as read',
32+
'issue.ready': 'the labels and body the triage node saw',
33+
branch: 'the branch head commit SHA',
34+
pr: 'the pull request number and its headRefOid',
35+
verdict: 'the SHA the verdict names',
36+
check: 'the SHA the check ran on',
37+
risk: 'the SHA the tier was computed on',
38+
mergeable: 'the tip SHA it was computed against',
39+
'merge-commit': 'the SHA the queue wrote to main',
40+
release: 'the tag it published',
41+
doc: 'the content hash of the document',
42+
file: 'the content hash of the file',
43+
link: 'the target the node recorded',
44+
text: 'the text the node recorded',
45+
}
46+
47+
// `check:tests-ok` -> `{ kind: 'check', name: 'tests-ok' }`, `issue.ready` -> `{ kind: 'issue.ready',
48+
// name: '' }`. Anything outside the vocabulary, a named kind without a name, or an unnamed part of
49+
// a named kind is not a spec: a definition that spells an artifact this way is invalid, not empty.
50+
export function parseArtifactSpec(spec) {
51+
const raw = typeof spec === 'string' ? spec.trim() : ''
52+
if (raw === '') return null
53+
const cut = raw.indexOf(':')
54+
const kind = cut === -1 ? raw : raw.slice(0, cut).trim()
55+
const name = cut === -1 ? '' : raw.slice(cut + 1).trim()
56+
if (!ARTIFACT_KINDS.includes(kind)) return null
57+
if (NAMED_KINDS.has(kind)) return name === '' ? null : { kind, name }
58+
return name === '' ? { kind, name: '' } : null
59+
}
60+
61+
export function formatArtifactSpec(parsed) {
62+
if (!parsed || !ARTIFACT_KINDS.includes(parsed.kind)) return null
63+
const name = typeof parsed.name === 'string' ? parsed.name.trim() : ''
64+
return name === '' ? parsed.kind : `${parsed.kind}:${name}`
65+
}
66+
67+
export function isArtifactSpec(spec) {
68+
return parseArtifactSpec(spec) !== null
69+
}
70+
71+
// Revisions compare as text, with a SHA case-folded so `ABC1234` and `abc1234` are one revision.
72+
// Everything else (`1274@1519968b`, a tag, a content hash) is compared exactly.
73+
export function normaliseRevision(revision) {
74+
if (revision === null || revision === undefined) return null
75+
const text = String(revision).trim()
76+
if (text === '') return null
77+
return /^[0-9a-f]{6,40}$/i.test(text) ? text.toLowerCase() : text
78+
}
79+
80+
export function sameRevision(a, b) {
81+
const left = normaliseRevision(a)
82+
return left !== null && left === normaliseRevision(b)
83+
}
84+
85+
export function revisionRule(kind) {
86+
return REVISION_RULES[kind] ?? null
87+
}
88+
89+
// One instance of an artifact: what it is, which revision of it a node saw, and the value that
90+
// revision carried (a verdict's PASS / INC, a text's body). No revision, no artifact.
91+
export function artifact(spec, revision, extra = {}) {
92+
const parsed = typeof spec === 'string' ? parseArtifactSpec(spec) : parseArtifactSpec(formatArtifactSpec(spec))
93+
if (!parsed) return null
94+
const at = normaliseRevision(revision)
95+
if (at === null) return null
96+
return {
97+
spec: formatArtifactSpec(parsed),
98+
kind: parsed.kind,
99+
name: parsed.name,
100+
revision: at,
101+
value: extra.value === undefined ? null : extra.value,
102+
by: extra.by === undefined ? null : extra.by,
103+
at: extra.at === undefined ? null : extra.at,
104+
}
105+
}
106+
107+
// `check:tests-ok@abc1234` — the spelling a refusal and a card use, so a reader sees which
108+
// revision was present without opening the record.
109+
export function formatArtifact(input) {
110+
if (!input || !input.spec) return null
111+
return input.revision ? `${input.spec}@${input.revision}` : input.spec
112+
}
113+
114+
export function sameArtifact(a, b) {
115+
return Boolean(a && b && a.spec === b.spec) && sameRevision(a.revision, b.revision)
116+
}
117+
118+
// Present is not current (#1272). The caller re-reads what produces the artifact and hands in the
119+
// revision that source holds now; a fact nobody could read is not a match either, so it refuses
120+
// with `unreadable` rather than pass on a guess (#1211).
121+
export function freshness(input, currentRevision) {
122+
const now = normaliseRevision(currentRevision)
123+
if (!input) return { fresh: false, reason: 'missing', have: null, now }
124+
if (now === null) return { fresh: false, reason: 'unreadable', have: input.revision ?? null, now: null }
125+
const fresh = sameRevision(input.revision, now)
126+
return { fresh, reason: fresh ? 'current' : 'stale', have: input.revision ?? null, now }
127+
}
128+
129+
export function artifactsBySpec(list) {
130+
const map = {}
131+
for (const item of Array.isArray(list) ? list : []) {
132+
if (item && item.spec) map[item.spec] = item
133+
}
134+
return map
135+
}

‎core/src/shared/workflow-blank.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// The `blank` preset (#1272): three nodes, one Bot each, no guards, no repository. It is here as
2+
// proof that a workflow is data — nothing in src/shared/workflow.js names these ids, and the same
3+
// JSON dropped in the workspace as `workflows/blank.json` is the whole definition.
4+
5+
export const blankWorkflow = Object.freeze({
6+
id: 'blank',
7+
name: 'Blank',
8+
version: 1,
9+
inputs: ['text:topic'],
10+
deliverables: ['link:published'],
11+
nodes: [
12+
{
13+
id: 'draft',
14+
name: 'Draft',
15+
bot: 'Writer',
16+
inputs: ['text:topic'],
17+
outputs: ['text:draft'],
18+
guards: [],
19+
refusal: '{node} cannot start: it needs {spec} and has none.',
20+
},
21+
{
22+
id: 'review',
23+
name: 'Review',
24+
bot: 'Editor',
25+
inputs: ['text:draft'],
26+
outputs: ['text:edited'],
27+
guards: [],
28+
refusal: '{node} cannot start: its input {input} is missing or stale.',
29+
},
30+
{
31+
id: 'publish',
32+
name: 'Publish',
33+
bot: 'Publisher',
34+
inputs: ['text:edited'],
35+
outputs: ['link:published'],
36+
guards: [],
37+
refusal: '{node} cannot start: it needs {spec} and has none.',
38+
},
39+
],
40+
edges: [
41+
{ from: 'draft.text:draft', to: 'review.text:draft' },
42+
{ from: 'review.text:edited', to: 'publish.text:edited' },
43+
],
44+
loops: [],
45+
})
46+
47+
// The Bots the preset names, so a caller can pass them as the team without inventing a roster.
48+
export const BLANK_TEAM = Object.freeze(['Writer', 'Editor', 'Publisher'])

0 commit comments

Comments
 (0)