Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions .github/scripts/fixtures/workflow-cases.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
{
"why": "The sentences the merge gate refuses with, written once. `.github/scripts/merge-gate.mjs`'s `judge` and the `repo-dev` preset's gate guards are judged against the same entries, and neither file imports the other: the gate test reads `gate` and the preset test reads `preset`, both comparing against `sentence` byte for byte (#1272 acceptance 9). Change a sentence in one place and the other side's test goes red.",
"head": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f",
"oldHead": "175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90",
"cases": [
{
"id": "hold",
"where": "merge-gate.mjs `judge`: the `hold` label refuses whatever the review says",
"sentence": "the pull request carries the `hold` label",
"gate": {
"labels": ["hold"],
"head": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f",
"expectedHead": null,
"reports": [{ "verdict": "PASS", "tip": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f", "writtenBy": "Reviewer" }]
},
"preset": {
"node": "gate",
"guard": "no-hold",
"present": [
{ "spec": "pr", "revision": "1274@abaf938b0c1d", "value": "hold" },
{ "spec": "verdict", "revision": "abaf938b0c1d", "value": "PASS", "by": "Reviewer" },
{ "spec": "check:tests-ok", "revision": "abaf938b0c1d" }
],
"current": { "pr": "1274@abaf938b0c1d", "verdict": "abaf938b0c1d", "check:tests-ok": "abaf938b0c1d" }
}
},
{
"id": "head-moved",
"where": "merge-gate.mjs `judge`: the head is not the one the caller asked about",
"sentence": "the pull request head is abaf938b0c1d, not the expected 175328d0c1b2",
"gate": {
"labels": [],
"head": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f",
"expectedHead": "175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90",
"reports": [{ "verdict": "PASS", "tip": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f", "writtenBy": "Reviewer" }]
},
"preset": {
"node": "gate",
"guard": "verdict-at-tip",
"present": [
{ "spec": "pr", "revision": "1274@abaf938b0c1d", "value": "none" },
{ "spec": "verdict", "revision": "175328d0c1b2", "value": "PASS", "by": "Reviewer" },
{ "spec": "check:tests-ok", "revision": "abaf938b0c1d" }
],
"current": { "pr": "1274@abaf938b0c1d", "verdict": "abaf938b0c1d", "check:tests-ok": "abaf938b0c1d" }
}
},
{
"id": "verdict-not-pass",
"where": "merge-gate.mjs `judge`: the newest conclusion at the tip is not a PASS",
"sentence": "reviewer-agent says INC at this tip",
"gate": {
"labels": [],
"head": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f",
"expectedHead": null,
"reports": [{ "verdict": "INC", "tip": "abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f", "writtenBy": "Reviewer" }]
},
"preset": {
"node": "gate",
"guard": "verdict-says-pass",
"present": [
{ "spec": "pr", "revision": "1274@abaf938b0c1d", "value": "none" },
{ "spec": "verdict", "revision": "abaf938b0c1d", "value": "INC", "by": "reviewer-agent" },
{ "spec": "check:tests-ok", "revision": "abaf938b0c1d" }
],
"current": { "pr": "1274@abaf938b0c1d", "verdict": "abaf938b0c1d", "check:tests-ok": "abaf938b0c1d" }
}
}
]
}
48 changes: 48 additions & 0 deletions .github/scripts/merge-gate.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { test } from 'node:test'
import assert from 'node:assert/strict'
import { readFileSync } from 'node:fs'
import { judge, prFromHeadRef } from './merge-gate.mjs'
import { guard } from './enqueue-rules.mjs'
import { fileURLToPath } from 'node:url'

const HEAD = 'abaf938b0c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f'
Expand Down Expand Up @@ -594,3 +595,50 @@ test('#960: the pull_request report is unchanged and still never fails on the bo
assert.equal(result.ok, true)
assert.equal(result.mode, 'report')
})

// #1272 step 2, acceptance 9. The fixture is the one place three of these sentences are written down:
// `core/test/workflow-repo-dev.test.js` asserts the `repo-dev` preset's gate guards refuse with the
// same words, and neither side imports the other. The readers pinned here are `judge` — the function
// merge-gate's required check runs — and `guard`, whose joined reasons are exactly what
// `enqueue-guard` prints after "do not enqueue #N:". Change a sentence on either side and this is red.
const workflowCases = JSON.parse(readFileSync(fileURLToPath(new URL('./fixtures/workflow-cases.json', import.meta.url)), 'utf8'))

test('#1272 step 2: the workflow fixture sentences are the gate s own, byte for byte', () => {
assert.ok(workflowCases.cases.length >= 3)
for (const entry of workflowCases.cases) {
const reports = entry.gate.reports.map((r) => say([
`Tip SHA: ${r.tip}`,
`Written-by: ${r.writtenBy}`,
'Ran:',
' - npm run verify → pass',
`Verdict: ${r.verdict}`,
].join('\n')))
const result = judge({
labels: entry.gate.labels,
head: entry.gate.head,
author: 'opener',
reports,
policy,
expectedHead: entry.gate.expectedHead ?? undefined,
})
assert.ok(result.reasons.includes(entry.sentence), `${entry.id}: judge said ${JSON.stringify(result.reasons)}`)
// enqueue-guard's REFUSE line is `do not enqueue #N: <reasons joined>`, so the same sentence has to
// appear there too, for the same input.
const refused = guard({
pr: 1272,
head: entry.gate.head,
labels: [...entry.gate.labels, 'agent-pr'],
body: 'Fixes #1272\nOwner: Dev3\nReviewer: Reviewer',
files: ['core/src/shared/workflow.js'],
mechanicalRelease: false,
fork: false,
draft: false,
merged: false,
result,
reports,
policy,
})
assert.equal(refused.ok, false)
assert.ok(refused.message.includes(`${entry.sentence}`), `${entry.id}: enqueue-guard said ${refused.message}`)
}
})
116 changes: 116 additions & 0 deletions core/src/host/workflow-facts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// What reads a fact (#1272 step 2, acceptance 10): one function per fact a guard asks about. A guard
// is a predicate over facts somebody else read, so every function here takes the answer a reader
// already got and turns it into one artifact — the fact's spec, the revision it was read at, and the
// value the node asked for. Nothing here fetches: the reader is the caller's, which is what keeps this
// module pure and what lets a test drive it with a recorded answer.
//
// The rule that matters: **no token, no answer, or an answer missing the field means `unreadable`**,
// never a guess. A fact nobody could read must refuse the node it feeds (`artifact.js` `freshness`,
// `workflow.js` `enterNode`), because "I did not manage to look" and "it is current" are not the same
// answer and only one of them may move work forward.

import { artifact } from '../shared/artifact.js'

// Every fact this module can answer, as `grep`-able names: a provider that is not here is one nobody
// wrote yet, and `workflowFact` says so rather than returning an empty artifact.
export const FACT_NAMES = Object.freeze([
'github.labels',
'github.checkAtSha',
'github.verdictAtTip',
'github.mergeable',
'github.prBodyTip',
'task.progress',
'manual.tick',
])

// The answer a caller gets when the fact could not be read: `ok: false` and the reason, with no
// `input`, so a guard cannot mistake it for a fact that happened to be empty.
export function unreadable(fact, why) {
return { fact, ok: false, reason: 'unreadable', why, input: null }
}

// A fact that was read: one artifact and the same `ok` the caller checks.
const read = (fact, input) => (input ? { fact, ok: true, reason: 'read', why: null, input } : unreadable(fact, 'the answer carried no revision'))

// GitHub reads need the token the host holds. Without it nothing was read, and a fact filled in from
// "the pull request probably has no hold label" is the one thing that would let a held card pass.
const hasToken = (token) => typeof token === 'string' && token.trim() !== ''

const labelName = (label) => (typeof label === 'string' ? label : label?.name ?? null)

// `pr` at the head the labels were read at, valued `hold` when the pull request carries that label
// and `none` otherwise. The gate's first guard is `equals: 'none'`.
export function githubLabels({ token, head, labels } = {}) {
if (!hasToken(token)) return unreadable('github.labels', 'no GITHUB_TOKEN')
if (!head || !Array.isArray(labels)) return unreadable('github.labels', 'the pull request labels were not read')
const names = labels.map(labelName).filter(Boolean).map((name) => String(name).toLowerCase())
return read('github.labels', artifact('pr', head, { value: names.includes('hold') ? 'hold' : 'none' }))
}

// `check:<name>` at the sha the check ran on, valued with its conclusion. A check that never reported
// has no sha to be at, so it is unreadable rather than a check at the tip that failed.
export function githubCheckAtSha({ token, name, sha, conclusion } = {}) {
if (!hasToken(token)) return unreadable('github.checkAtSha', 'no GITHUB_TOKEN')
if (!name || !sha) return unreadable('github.checkAtSha', 'the check run was not read')
return read('github.checkAtSha', artifact(`check:${name}`, sha, { value: conclusion ?? null }))
}

// `verdict` at the tip the conclusion names, valued PASS / INC / FAIL / REQUEST_CHANGES and carrying
// the Bot that wrote it, which is what says who holds the gate.
export function githubVerdictAtTip({ token, tip, verdict, by } = {}) {
if (!hasToken(token)) return unreadable('github.verdictAtTip', 'no GITHUB_TOKEN')
if (!tip || !verdict) return unreadable('github.verdictAtTip', 'the newest conclusion at this tip was not read')
return read('github.verdictAtTip', artifact('verdict', tip, { value: verdict, by: by ?? null }))
}

// `mergeable` against the tip it was computed on: `mergeable` / `conflict` / `unknown`. GitHub's own
// answer for a tip it has not computed yet is `unknown`, and `unknown` is not `mergeable`.
export function githubMergeable({ token, head, mergeable } = {}) {
if (!hasToken(token)) return unreadable('github.mergeable', 'no GITHUB_TOKEN')
if (!head || !mergeable) return unreadable('github.mergeable', 'the mergeable answer was not read')
return read('github.mergeable', artifact('mergeable', head, { value: mergeable }))
}

// The `Tip SHA:` line of the pull request body, as the tip it names: this is the fact a checkpoint is
// read through, and a body with no such line is unreadable, not an empty tip.
export function githubPrBodyTip({ token, body } = {}) {
if (!hasToken(token)) return unreadable('github.prBodyTip', 'no GITHUB_TOKEN')
if (typeof body !== 'string') return unreadable('github.prBodyTip', 'the pull request body was not read')
const found = /^\s*Tip SHA:\s*([0-9a-f]{7,40})\s*$/im.exec(body)
if (!found) return unreadable('github.prBodyTip', 'the body carries no Tip SHA line')
return read('github.prBodyTip', artifact('text:pr-body-tip', found[1], { value: found[1].toLowerCase() }))
}

// A card's own progress, 0–100, as a text fact: the revision is the number itself, so a card that has
// not moved is the same revision and a card that moved is a different one.
export function taskProgress({ card } = {}) {
if (!card || typeof card !== 'object') return unreadable('task.progress', 'the card was not read')
const progress = Number.isInteger(card.progress) ? card.progress : null
if (progress === null || progress < 0 || progress > 100) return unreadable('task.progress', 'the card carries no progress between 0 and 100')
return read('task.progress', artifact('text:card-progress', String(progress), { value: progress }))
}

// A tick a node recorded itself — the one fact nothing outside read. It still needs a revision: the
// instant the node recorded, so a tick from before the work is not the tick that says it is done.
export function manualTick({ at, by } = {}) {
if (!at) return unreadable('manual.tick', 'the node recorded no instant')
return read('manual.tick', artifact('text:tick', at, { value: by ?? null }))
}

export const FACT_PROVIDERS = Object.freeze({
'github.labels': githubLabels,
'github.checkAtSha': githubCheckAtSha,
'github.verdictAtTip': githubVerdictAtTip,
'github.mergeable': githubMergeable,
'github.prBodyTip': githubPrBodyTip,
'task.progress': taskProgress,
'manual.tick': manualTick,
})

// One spelling for a caller that has a fact's name as data (a guard's `spec`, a provider list): a name
// nobody wrote a function for is unreadable, with the name in the reason.
export function workflowFact(name, args = {}) {
const provider = FACT_PROVIDERS[name]
if (!provider) return unreadable(String(name ?? ''), 'no provider answers this fact')
return provider(args)
}
10 changes: 5 additions & 5 deletions core/src/shared/workflow-blank.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// The `blank` preset (#1272): three nodes, one Bot each, no guards, no repository. It is here as
// The `blank` preset (#1272): three nodes, one role each, no guards, no repository. It is here as
// proof that a workflow is data — nothing in src/shared/workflow.js names these ids, and the same
// JSON dropped in the workspace as `workflows/blank.json` is the whole definition.

Expand All @@ -12,7 +12,7 @@ export const blankWorkflow = Object.freeze({
{
id: 'draft',
name: 'Draft',
bot: 'Writer',
role: 'Writer',
inputs: ['text:topic'],
outputs: ['text:draft'],
guards: [],
Expand All @@ -21,7 +21,7 @@ export const blankWorkflow = Object.freeze({
{
id: 'review',
name: 'Review',
bot: 'Editor',
role: 'Editor',
inputs: ['text:draft'],
outputs: ['text:edited'],
guards: [],
Expand All @@ -30,7 +30,7 @@ export const blankWorkflow = Object.freeze({
{
id: 'publish',
name: 'Publish',
bot: 'Publisher',
role: 'Publisher',
inputs: ['text:edited'],
outputs: ['link:published'],
guards: [],
Expand All @@ -44,5 +44,5 @@ export const blankWorkflow = Object.freeze({
loops: [],
})

// The Bots the preset names, so a caller can pass them as the team without inventing a roster.
// The roles the preset names, so a caller can pass them as the team without inventing a roster.
export const BLANK_TEAM = Object.freeze(['Writer', 'Editor', 'Publisher'])
Loading
Loading