chore(issue-ref): drop the dead delivery-board nesting helper (#1376)

changelog.d/1376.removed.md

@@ -0,0 +1 @@
+The leftover delivery-board helper is gone ([#1376](https://github.com/ThinkFlowLab/FlyBot/issues/1376)). `issue-ref.js` exports only the matcher the dispatch and the task store call (`issueRefs`, `namesIssue`, `cardNamesIssue`): `issueCardForTask` and the private half it alone used (`cardMentions`, `mentionOf`, `MENTION`, `asNumber`) were deleted with the board that read them, and `core-host.md` no longer lists them.

core/src/shared/issue-ref.js

@@ -38,63 +38,3 @@ export function cardNamesIssue(task, wanted) {
 }
 
 export const issueRefPattern = () => new RegExp(ISSUE_REF.source, 'gi')
-
-// The same mentions as ISSUE_REF, with the repository when the text writes one beside the number
-// (`owner/repo#12`, or a github.com issues/pull URL). A bare `#12` carries no repository. The
-// number is the same one `issueRefs` returns; the repository is not a second decision.
-const MENTION = /(?:github\.com\/)?([\w.-]+)\/([\w.-]+)#(\d{1,7})(?![\w])|github\.com\/([\w.-]+)\/([\w.-]+)\/(?:issues|pull)\/(\d{1,7})(?![\w])|(?<![\w./-])#(\d{1,7})(?![\w])/gi
-
-const mentionOf = (match) => {
-  if (match[3]) return { repo: `${match[1]}/${match[2]}`, number: match[3] }
-  if (match[6]) return { repo: `${match[4]}/${match[5]}`, number: match[6] }
-  if (match[7]) return { repo: null, number: match[7] }
-  return null
-}
-
-// Mentions in the text `cardNamesIssue` consults: title and `target.href`, and the note only
-// when those name no issue. A note cannot attach a repository to a number the title already named.
-export function cardMentions(task) {
-  const own = `${task?.title ?? ''} ${task?.target?.href ?? ''}`
-  const text = issueRefs(own).length > 0 ? own : `${own} ${task?.note ?? ''}`
-  const mentions = []
-  const seen = new Set()
-  for (const match of text.matchAll(new RegExp(MENTION.source, 'gi'))) {
-    const mention = mentionOf(match)
-    if (!mention) continue
-    const key = `${mention.repo ?? ''}#${mention.number}`
-    if (seen.has(key)) continue
-    seen.add(key)
-    mentions.push(mention)
-  }
-  return mentions
-}
-
-const asNumber = (value) => (value == null || value === '' ? null : String(value))
-
-// The one issue card this task hangs under, or null when `cardNamesIssue` names none or names
-// more than one. `issues` are `{ repo, number, linked }` or `{ repo, card: { number, linked } }`.
-// A repository written next to the number selects that card; a bare number selects the only
-// card with that number (or the only card whose linked pull request has it).
-export function issueCardForTask(task, issues) {
-  const mentions = cardMentions(task)
-  if (mentions.length === 0) return null
-  const list = (issues ?? []).filter((issue) => asNumber(issue?.card?.number ?? issue?.number) != null)
-  const numberOf = (issue) => asNumber(issue?.card?.number ?? issue?.number)
-  const linkedOf = (issue) => asNumber(issue?.card?.linked ?? issue?.linked)
-  const repoOf = (issue) => (typeof issue?.repo === 'string' ? issue.repo : '')
-  const qualified = list.filter((issue) => mentions.some((mention) =>
-    mention.repo != null && mention.repo === repoOf(issue)
-    && (mention.number === numberOf(issue) || mention.number === linkedOf(issue))))
-  if (qualified.length === 1) return qualified[0]
-  if (qualified.length > 1) {
-    const byNumber = qualified.filter((issue) => mentions.some((mention) =>
-      mention.repo === repoOf(issue) && mention.number === numberOf(issue)))
-    return byNumber.length === 1 ? byNumber[0] : null
-  }
-  const bare = new Set(mentions.filter((mention) => mention.repo == null).map((mention) => mention.number))
-  if (bare.size === 0) return null
-  const bareHits = list.filter((issue) => bare.has(numberOf(issue)) || (linkedOf(issue) != null && bare.has(linkedOf(issue))))
-  if (bareHits.length === 1) return bareHits[0]
-  const byNumber = bareHits.filter((issue) => bare.has(numberOf(issue)))
-  return byNumber.length === 1 ? byNumber[0] : null
-}

core/test/issue-ref.test.js

@@ -4,7 +4,7 @@
 // the shapes that do name an issue have to keep working beside them.
 import assert from 'node:assert/strict'
 import { test } from 'node:test'
-import { cardMentions, issueRefs, namesIssue } from '../src/shared/issue-ref.js'
+import { issueRefs, namesIssue } from '../src/shared/issue-ref.js'
 
 const rejected = [
   ['a hex colour', '@Dev-2 set the accent colour to #3366ff'],
@@ -47,11 +47,3 @@ test('namesIssue answers the same question, so a colour cannot name an issue', (
   assert.equal(namesIssue('owner/repo#1360', 1360), true)
   assert.equal(namesIssue('@Dev-2 rebase #1360', 1360), true)
 })
-
-test('a card whose title only carries a colour names no issue', () => {
-  assert.deepEqual(cardMentions({ title: 'accent colour #3366ff', target: { href: '' } }), [])
-  assert.deepEqual(
-    cardMentions({ title: 'owner/repo#1360', target: { href: '' } }),
-    [{ repo: 'owner/repo', number: '1360' }],
-  )
-})

core/test/no-delivery-board.test.js

@@ -2,7 +2,7 @@
 // probe script. These assertions are red while that surface is still in the tree, and red
 // again if a later change puts the panel or either endpoint back.
 import assert from 'node:assert/strict'
-import { existsSync, readFileSync } from 'node:fs'
+import { existsSync, readdirSync, readFileSync } from 'node:fs'
 import { test } from 'node:test'
 
 const url = (path) => new URL(`../${path}`, import.meta.url)
@@ -39,3 +39,19 @@ test('#1363: the task board does not mount the delivery view', () => {
   assert.doesNotMatch(client, /delivery-board|delivery-probe|deliveryBoard|deliveryProbe/)
   assert.match(kanban, /h\(BoardView/)
 })
+
+// #1376: the delivery board's nesting helper went with it. The matcher keeps `cardNamesIssue`,
+// which the dispatch and the task store call; `issueCardForTask` and its private half have no
+// reader, so no file may name them again. This file is the only place the two names may appear.
+test('#1376: the dead nesting helper is gone and nothing names it', () => {
+  assert.doesNotMatch(read('src/shared/issue-ref.js'), /issueCardForTask|cardMentions/)
+  const self = 'test/no-delivery-board.test.js'
+  for (const root of ['src', 'test']) {
+    for (const entry of readdirSync(url(root), { recursive: true })) {
+      const path = `${root}/${String(entry)}`
+      if (path === self) continue
+      if (!/\.(?:js|mjs|cjs|json)$/.test(path)) continue
+      assert.doesNotMatch(read(path), /issueCardForTask|cardMentions/, path)
+    }
+  }
+})

docs/subsystems/core-host.md

@@ -53,7 +53,7 @@ The server side of the patched `ds-bot` core (`core/`): the part of [ds-bot-core
 | `core/src/shared/preview.js` | What a stored line may show a person (#130, #1347): `visibleText(text)` unwraps a complete `<untrusted-message>` wrap, then removes leftover marker fragments until neither changes the text, then folds to one line. `stripFence` is the marker half alone. The sidebar rows read it (`previewOf` in `src/client/text.js`), and so does the `preview` this channel puts on a room in the `state` payload, which used to be a plain `unwrapUntrusted` and so leaked `from="User"> … >` on a line whose opener had already been cut. Tests: `core/test/ui-clickthrough.test.js`. |
 | `core/src/shared/failure-copy.js` | A failed turn's sentence a person can act on (#1347): `failureSentence(raw, translate)` keys on the provider's own `code` field in the stored text — not on the message, which is not stable — and returns the mapped sentence (naming the feature in an `UnsupportedModel` refusal, and where to change the model), or the text unchanged when the code is not mapped, because a failure nobody has mapped reads better as it came than as a guess. `src/shared/turn-failures.js` and `src/client/inbox.js` both read it, so the chat's dead-turn lines and the Inbox card say the same thing. Tests: `core/test/ui-clickthrough.test.js`. |
 | `core/src/shared/board-card.js` | Task-card identity for the per-Bot board (#1363): `taskCardKey(id)` is `task:<id>` and `taskCard(task)` builds the model the client's `BoardCard` draws (`source: 'task'`, `data-key`). Tests: `core/test/board-card.test.js`. |
-| `core/src/shared/issue-ref.js` | The one issue/PR matcher (#1254, #1331, #1360): `issueRefs`, `namesIssue`, `cardNamesIssue` and `issueCardForTask`. Own fields (`title`, `target.href`) name the card; a note counts only when those name no issue, and a repository written in that note does not count either. `work-dispatch.js` re-exports the first three and `tasks.js` `assignFromDispatch` uses `cardNamesIssue` to pick the newest open record. `issueCardForTask` answers which GitHub issue card a task would hang under; that hang was the delivery board and is not drawn (#1363). Tests: `core/test/work-dispatch.test.js`. |
+| `core/src/shared/issue-ref.js` | The one issue/PR matcher (#1254, #1331, #1360): `issueRefs`, `namesIssue` and `cardNamesIssue`. Own fields (`title`, `target.href`) name the card; a note counts only when those name no issue, and a repository written in that note does not count either. `work-dispatch.js` re-exports the first three and `tasks.js` `assignFromDispatch` uses `cardNamesIssue` to pick the newest open record. Tests: `core/test/work-dispatch.test.js`. |
 | `core/src/shared/workflow.js` | The workflow engine's pure half (#1272): a workflow is data — `nodes` (each with a required `role` and typed `inputs` / `outputs`), `edges` (`<node>.<artifact> -> <node>.<artifact>`) and declared `loops` — and every function below is a function of that file. `validateWorkflow` refuses each way a definition cannot run, with its own sentence (an input nothing produces, an output nothing consumes and that is not a `deliverable`, two producers of one fact, an undeclared cycle, a node with no `role`, a role not on the team, an unknown artifact kind, a misaligned edge, a guard whose `is` is not one of `present` / `fresh` / `equals`, a `column` the board does not have); `workflowColumns` is the board's columns in dependency order, so the node ids are the state and no built-in list exists, and a node carrying `column: false` draws no column at all while a definition that states none keeps the node id as its column (#1265); `layoutWorkflow` is the flowchart (one box per node carrying its role and rails, one edge per `edges[]` row, ranks left to right); `enterNode` / `inputStates` / `renderRefusal` answer whether a card may start and, when it may not, say which input is missing or stale with both revisions, in the node's own `refusal` wording when it has one and in the guard's own wording when that guard carries one; both read a fact by the same parsed key the rest of the engine uses, so the spelling on the card and the spelling on the producer cannot disagree (#1298); `holderOf` is the node's role, which is the card's assignee. #1291's four debts are closed here: one word for one thing (`role`, never `bot`), an unknown guard `is` refuses instead of being read as `present`, `producersOf` / `consumersOf` key on the parsed spec so `text: draft` and `text:draft` are one fact, and `resolvedEdges` reports the artifact it carries once. Tests: `core/test/workflow.test.js`. |
 | `core/src/shared/workflow-blank.js` | The `blank` preset (#1272): three nodes (`draft` / `review` / `publish`, roles `Writer` / `Editor` / `Publisher`), no guards, artifacts `text:topic -> text:draft -> text:edited -> link:published`. Data, not logic — the same JSON is what a workspace `workflows/blank.json` holds — and the proof that the engine special-cases no node id. Tests: `core/test/workflow.test.js`. |
 | `core/src/shared/workflow-repo-dev.js` | The `repo-dev` preset (#1272 step 2): one repository change as data — `triage` → `build` → `verify` → `review` → `gate` → `merge` → `release`, roles `Triage` / `Dev` / `SelfTest` / `Reviewer` / `system` / `Release`, and the artifacts `issue` → `issue.ready` → `pr` → `check:tests-ok` → `verdict` → `risk:tier` → `merge-commit` → `release`. Two roles say what they are rather than who: `verify` is `SelfTest` **and** CI, the run being the `check:tests-ok` fact it hands out, and `gate` is held by the Reviewer that judged the pull request, read from the verdict's own `Written-by` through a guard. The gate's guards refuse in the merge gate's own words against `.github/scripts/fixtures/workflow-cases.json`, which no file imports and both sides read. `column` per node: `not-started` / `in-progress` / `done`, with `review` on `false` — a review is the pull request's state, not a third lane (#1265). Tests: `core/test/workflow-repo-dev.test.js`. |

patches/ds-bot/files/src/shared/issue-ref.js

@@ -38,63 +38,3 @@ export function cardNamesIssue(task, wanted) {
 }
 
 export const issueRefPattern = () => new RegExp(ISSUE_REF.source, 'gi')
-
-// The same mentions as ISSUE_REF, with the repository when the text writes one beside the number
-// (`owner/repo#12`, or a github.com issues/pull URL). A bare `#12` carries no repository. The
-// number is the same one `issueRefs` returns; the repository is not a second decision.
-const MENTION = /(?:github\.com\/)?([\w.-]+)\/([\w.-]+)#(\d{1,7})(?![\w])|github\.com\/([\w.-]+)\/([\w.-]+)\/(?:issues|pull)\/(\d{1,7})(?![\w])|(?<![\w./-])#(\d{1,7})(?![\w])/gi
-
-const mentionOf = (match) => {
-  if (match[3]) return { repo: `${match[1]}/${match[2]}`, number: match[3] }
-  if (match[6]) return { repo: `${match[4]}/${match[5]}`, number: match[6] }
-  if (match[7]) return { repo: null, number: match[7] }
-  return null
-}
-
-// Mentions in the text `cardNamesIssue` consults: title and `target.href`, and the note only
-// when those name no issue. A note cannot attach a repository to a number the title already named.
-export function cardMentions(task) {
-  const own = `${task?.title ?? ''} ${task?.target?.href ?? ''}`
-  const text = issueRefs(own).length > 0 ? own : `${own} ${task?.note ?? ''}`
-  const mentions = []
-  const seen = new Set()
-  for (const match of text.matchAll(new RegExp(MENTION.source, 'gi'))) {
-    const mention = mentionOf(match)
-    if (!mention) continue
-    const key = `${mention.repo ?? ''}#${mention.number}`
-    if (seen.has(key)) continue
-    seen.add(key)
-    mentions.push(mention)
-  }
-  return mentions
-}
-
-const asNumber = (value) => (value == null || value === '' ? null : String(value))
-
-// The one issue card this task hangs under, or null when `cardNamesIssue` names none or names
-// more than one. `issues` are `{ repo, number, linked }` or `{ repo, card: { number, linked } }`.
-// A repository written next to the number selects that card; a bare number selects the only
-// card with that number (or the only card whose linked pull request has it).
-export function issueCardForTask(task, issues) {
-  const mentions = cardMentions(task)
-  if (mentions.length === 0) return null
-  const list = (issues ?? []).filter((issue) => asNumber(issue?.card?.number ?? issue?.number) != null)
-  const numberOf = (issue) => asNumber(issue?.card?.number ?? issue?.number)
-  const linkedOf = (issue) => asNumber(issue?.card?.linked ?? issue?.linked)
-  const repoOf = (issue) => (typeof issue?.repo === 'string' ? issue.repo : '')
-  const qualified = list.filter((issue) => mentions.some((mention) =>
-    mention.repo != null && mention.repo === repoOf(issue)
-    && (mention.number === numberOf(issue) || mention.number === linkedOf(issue))))
-  if (qualified.length === 1) return qualified[0]
-  if (qualified.length > 1) {
-    const byNumber = qualified.filter((issue) => mentions.some((mention) =>
-      mention.repo === repoOf(issue) && mention.number === numberOf(issue)))
-    return byNumber.length === 1 ? byNumber[0] : null
-  }
-  const bare = new Set(mentions.filter((mention) => mention.repo == null).map((mention) => mention.number))
-  if (bare.size === 0) return null
-  const bareHits = list.filter((issue) => bare.has(numberOf(issue)) || (linkedOf(issue) != null && bare.has(linkedOf(issue))))
-  if (bareHits.length === 1) return bareHits[0]
-  const byNumber = bareHits.filter((issue) => bare.has(numberOf(issue)))
-  return byNumber.length === 1 ? byNumber[0] : null
-}

patches/ds-bot/files/test/issue-ref.test.js

@@ -4,7 +4,7 @@
 // the shapes that do name an issue have to keep working beside them.
 import assert from 'node:assert/strict'
 import { test } from 'node:test'
-import { cardMentions, issueRefs, namesIssue } from '../src/shared/issue-ref.js'
+import { issueRefs, namesIssue } from '../src/shared/issue-ref.js'
 
 const rejected = [
   ['a hex colour', '@Dev-2 set the accent colour to #3366ff'],
@@ -47,11 +47,3 @@ test('namesIssue answers the same question, so a colour cannot name an issue', (
   assert.equal(namesIssue('owner/repo#1360', 1360), true)
   assert.equal(namesIssue('@Dev-2 rebase #1360', 1360), true)
 })
-
-test('a card whose title only carries a colour names no issue', () => {
-  assert.deepEqual(cardMentions({ title: 'accent colour #3366ff', target: { href: '' } }), [])
-  assert.deepEqual(
-    cardMentions({ title: 'owner/repo#1360', target: { href: '' } }),
-    [{ repo: 'owner/repo', number: '1360' }],
-  )
-})

patches/ds-bot/files/test/no-delivery-board.test.js

@@ -2,7 +2,7 @@
 // probe script. These assertions are red while that surface is still in the tree, and red
 // again if a later change puts the panel or either endpoint back.
 import assert from 'node:assert/strict'
-import { existsSync, readFileSync } from 'node:fs'
+import { existsSync, readdirSync, readFileSync } from 'node:fs'
 import { test } from 'node:test'
 
 const url = (path) => new URL(`../${path}`, import.meta.url)
@@ -39,3 +39,19 @@ test('#1363: the task board does not mount the delivery view', () => {
   assert.doesNotMatch(client, /delivery-board|delivery-probe|deliveryBoard|deliveryProbe/)
   assert.match(kanban, /h\(BoardView/)
 })
+
+// #1376: the delivery board's nesting helper went with it. The matcher keeps `cardNamesIssue`,
+// which the dispatch and the task store call; `issueCardForTask` and its private half have no
+// reader, so no file may name them again. This file is the only place the two names may appear.
+test('#1376: the dead nesting helper is gone and nothing names it', () => {
+  assert.doesNotMatch(read('src/shared/issue-ref.js'), /issueCardForTask|cardMentions/)
+  const self = 'test/no-delivery-board.test.js'
+  for (const root of ['src', 'test']) {
+    for (const entry of readdirSync(url(root), { recursive: true })) {
+      const path = `${root}/${String(entry)}`
+      if (path === self) continue
+      if (!/\.(?:js|mjs|cjs|json)$/.test(path)) continue
+      assert.doesNotMatch(read(path), /issueCardForTask|cardMentions/, path)
+    }
+  }
+})