Link only a closing keyword, and read every comment page, on the delivery board (#1189) (#1190)

Owner: Dev2
Reviewer: Reviewer
Tip SHA: 36ba643
Base: fc19e6d (current main)

Fixes #1189

## Reproduce

On `fc19e6d2`, `core/src/host/delivery-board.js:242` builds the issue →
pull request link with `text.matchAll(/#(\d+)/g)` over the pull
request's title and body, so a body that says `also see #12` makes issue
#12 show a pull request that closes nothing. `:128-130` reads
`comments?per_page=100&page=1` once, and `verdictOf` (`:41`) scans only
those, so on a pull request with more than 100 comments — ordinary here
(#1170, #1183) — the newest `Verdict:` is never read and the badge falls
back to `no verdict at this tip`.

Both are visible through the existing fake-fetch tests: give a pull
request the body `also see #12` and the card for #12 claims it; put the
newest verdict on page two and the card says `no-verdict`.

## What changed

- `core/src/host/delivery-board.js` — `closingLinks()`: only GitHub's
own closing keywords (`close[sd]?`, `fix(?:es|ed)?`, `resolve[sd]?`,
case-insensitive, word boundary, optional colon) followed by `#n` link a
pull request to an issue (#1189). `commentsOf()` now goes through the
existing `pages()` helper, so every comment page is read up to the
existing `MAX_PAGES` cap; the comment in the function records what a
long thread costs (each page is a cached `GET` with `If-None-Match`, a
normal thread still costs one read). Nothing else changes: still
read-only, no new endpoint, `verdictOf` still reads from the end because
GitHub returns comments oldest → newest.
- `core/test/delivery-board.test.js` — two cases: a pull request whose
body only mentions `#10`/`#12` leaves `#10` unlinked and `#12` on the
pull request that closes it, while `Closes: #13` links; and a
120-comment thread whose newest `Verdict: INC at <tip>` sits at index
101 decides the badge (`inc`) with page 2 actually requested.
- `docs/subsystems/core-host.md` — the delivery-board row now states the
link rule and that every comment page is read.
- `patches/ds-bot/files/src/host/delivery-board.js`,
`patches/ds-bot/files/test/delivery-board.test.js` — the mirrors,
byte-identical.
- `changelog.d/1189-delivery-board-links.fixed.md` — user-visible (the
board is in the UI).

## What I ran

- `cd core && node --import ./test/harness-resolve.mjs --test
--test-concurrency=1 test/delivery-board.test.js
test/delivery-view.test.js test/delivery-write-failure.test.js
test/kanban.test.js test/bot-tasks.test.js` → **30/30**, 0 failed.
`delivery-board` alone is **14/14** (base 12 + 2).
- oxlint (`-c core/.oxlintrc.json` on the two changed files) → **0
warnings, 0 errors**.
- `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/`.
- `node .github/scripts/changelog.mjs check` → `57 fragment(s) … well
formed`.
- `node core/scripts/build-client.mjs --check` → exit 0 (no client
change).

## Red without the source change

- `closingLinks` back to `/#(\d+)/g` → **13/14**, red only on "only a
closing keyword links a pull request to an issue (#1189)".
- `commentsOf` back to `?per_page=100&page=1` → **13/14**, red only on
"a verdict on a later page of comments still decides the badge (#1189)".
Both restored by `cmp` to the tip file, suite back to 14/14.

## Not run

- `npm run verify` end to end: its `arch-docs` step needs `git` and
`origin/main`, and this box has no `git`; the whole core suite is not
runnable here either (a pristine `fc19e6d2` reports the same eight
failures on this box). CI's `tests-ok` is the terminal answer.
- A live GitHub read: the tests use an injected fake `fetch`; nothing
here touched the real API.

## Risk

- Low. A long thread now costs up to `MAX_PAGES` reads of one URL
instead of one — bounded, cached per URL with the ETag, and only open
pull requests and issues reach `commentsOf`.
- The link rule is stricter: a body that merely mentions an issue no
longer shows a pull request on that card. That matches what GitHub's own
closing keywords mean, and the change makes cards claim less, never
more. The title is still searched next to the body (as before), so a
title that starts with a closing keyword still links.

Author: Birfy

changelog.d/1189-delivery-board-links.fixed.md

@@ -0,0 +1 @@
+The delivery board links an issue to a pull request only when the pull request closes it, and a verdict past the first hundred comments still decides the badge ([#1189](https://github.com/ThinkFlowLab/FlyBot/issues/1189)).

core/src/host/delivery-board.js

@@ -38,6 +38,11 @@ const orderLabels = (names) => [...names].sort((a, b) => {
 // The issues endpoint returns pull requests too; the pulls endpoint never does, so the two
 // lists are told apart by that field rather than by sniffing a PR's shape.
 const isPull = (row) => row.pull_request !== undefined || row.merge_commit_sha !== undefined
+// GitHub's closing keywords are the only mention that links a pull request to an issue (#1189).
+// `also see #12`, `same as #12` or a changelog line quoting a number is prose, and a card that
+// showed those as a link would claim a pull request that closes nothing.
+const CLOSING_KEYWORD = /\b(?:close[sd]?|fix(?:es|ed)?|resolve[sd]?)[ \t]*:?[ \t]*#(\d+)/gi
+const closingLinks = (text) => [...String(text).matchAll(CLOSING_KEYWORD)].map(match => Number(match[1]))
 const verdictOf = (comments) => {
   for (let index = (comments ?? []).length - 1; index >= 0; index -= 1) {
     const body = String(comments[index]?.body ?? '')
@@ -125,8 +130,13 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n
     return rows
   }
 
+  // Every page, not only the first (#1189). Past 100 comments — ordinary on this repository
+  // (#1170, #1183) — the newest verdict sat on a page nothing read, and the badge fell back to
+  // "no verdict at this tip" while one existed. The cap is the same MAX_PAGES as every other list
+  // and each page is a cached GET (ETag), so a long thread costs up to MAX_PAGES reads of one URL
+  // and a normal one still costs one.
   async function commentsOf(number) {
-    return get(`${base}/issues/${number}/comments?per_page=100&page=1`)
+    return pages(`/issues/${number}/comments`)
   }
 
   async function runsOfDay(from, to) {
@@ -238,9 +248,7 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n
     const pullRows = openPulls
     const linkedTo = new Map()
     for (const pr of pullRows) {
-      const text = `${pr.title ?? ''}\n${pr.body ?? ''}`
-      for (const match of text.matchAll(/#(\d+)/g)) {
-        const number = Number(match[1])
+      for (const number of closingLinks(`${pr.title ?? ''}\n${pr.body ?? ''}`)) {
         if (!linkedTo.has(number)) linkedTo.set(number, pr.number)
       }
     }

core/test/delivery-board.test.js

@@ -8,7 +8,7 @@ import { createDeliveryBoard, DELIVERY_COLUMNS, install, median, startOfDay } fr
 const NOW = Date.parse('2026-10-25T04:00:00Z') // 12:00 in Asia/Shanghai
 const AT = (iso) => Date.parse(iso)
 
-function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [] } = {}) {
+function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [], openPullsExtra = [] } = {}) {
   const calls = []
   const write = (body, status = 200) => ({ ok: status < 400, status, headers: { get: () => 'etag-1' }, json: async () => body })
   const request = async (url, init = {}) => {
@@ -29,6 +29,7 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra
       return write(page === 1 ? [
         { number: 40, title: 'Pending verdict', html_url: 'https://x/40', body: 'Owner: Dev', head: { sha: 'aaaaaaa1' }, mergeable_state: 'unstable', created_at: '2026-10-24T00:00:00Z', updated_at: '2026-10-24T10:00:00Z', labels: [] },
         { number: 41, title: 'Fix the thing', html_url: 'https://x/41', body: 'Fixes #12', head: { sha: 'bbbbbbb2' }, mergeable_state: 'clean', created_at: '2026-10-23T00:00:00Z', updated_at: '2026-10-24T09:00:00Z', labels: [{ name: 'agent-pr' }] },
+        ...openPullsExtra,
       ] : [])
     }
     if (path === '/pulls' && parsed.searchParams.get('state') === 'all') {
@@ -72,7 +73,10 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra
     }
     if (/^\/issues\/\d+\/comments$/.test(path)) {
       const number = Number(path.split('/')[2])
-      return write(comments[number] ?? [])
+      const value = comments[number] ?? []
+      // A thread longer than one page is passed as pages; a flat array is page one (#1189).
+      const pagesOf = Array.isArray(value[0]) ? value : [value]
+      return write(pagesOf[page - 1] ?? [])
     }
     if (/^\/commits\/.+\/check-runs$/.test(path)) {
       const sha = path.split('/')[2]
@@ -115,6 +119,23 @@ test('five columns come from one snapshot, an issue and its PR linked (#1173)',
   assert.deepEqual(snapshot.counts, { raw: 2, ready: 1, working: 1, review: 2, 'done-today': 2 })
 })
 
+test('only a closing keyword links a pull request to an issue (#1189)', async () => {
+  const { board } = build({
+    openPullsExtra: [
+      { number: 43, title: 'Related work', html_url: 'https://x/43', body: 'also see #10, same as #12', head: { sha: 'ddddddd4' }, mergeable_state: 'clean', created_at: '2026-10-22T00:00:00Z', updated_at: '2026-10-24T08:00:00Z', labels: [] },
+      { number: 44, title: 'Close it', html_url: 'https://x/44', body: 'Closes: #13', head: { sha: 'eeeeeee5' }, mergeable_state: 'clean', created_at: '2026-10-22T00:00:00Z', updated_at: '2026-10-24T08:00:00Z', labels: [] },
+    ],
+  })
+  const snapshot = await board.read()
+  const card = (column, number) => snapshot.columns[column].find(item => item.number === number)
+  // "also see #10, same as #12" is prose: #10 links to nothing, and #12 keeps the pull request
+  // that closes it (41), not the one that only mentions it.
+  assert.equal(card('raw', 10).linked, null)
+  assert.equal(card('working', 12).linked, 41)
+  // A closing keyword with a colon counts; nothing else does.
+  assert.equal(card('raw', 13).linked, 44)
+})
+
 test('a pull request closed today without a merge is done today, not nowhere (#1173)', async () => {
   const { board } = build({
     closedExtra: [
@@ -180,6 +201,25 @@ test('a verdict line wins over an Attestation that names an older sha (#1173)',
   assert.equal(card.badge, 'pass-enqueue')
 })
 
+test('a verdict on a later page of comments still decides the badge (#1189)', async () => {
+  // 120 comments with the newest verdict at index 101: on a board that reads one page it never
+  // appears, and the card says "no verdict at this tip" while the tip has one.
+  const firstPage = Array.from({ length: 100 }, (_, index) => ({ body: `note ${index}` }))
+  const secondPage = [
+    { body: 'note 100' },
+    { body: 'Verdict: INC at bbbbbbb2' },
+    ...Array.from({ length: 18 }, (_, index) => ({ body: `note ${102 + index}` })),
+  ]
+  const { board, github } = build({
+    comments: { 41: [firstPage, secondPage] },
+    checks: { bbbbbbb2: [{ status: 'completed', conclusion: 'success' }] },
+  })
+  const snapshot = await board.read()
+  const card = snapshot.columns.review.find(item => item.number === 41)
+  assert.equal(card.badge, 'inc')
+  assert.ok(github.calls.some(call => call.path === '/issues/41/comments' && /[?&]page=2(&|$)/.test(call.query)))
+})
+
 test('the token is read per request, and install loads the secrets first (#1173)', async () => {
   // A token captured at install time is empty until the host loads secrets; every request must
   // resolve it again, and the first read must ask the host to load them (#1173 INC).

docs/subsystems/core-host.md

@@ -38,7 +38,7 @@ The server side of the patched `ds-bot` core (`core/`): the part of [ds-bot-core
 | --- | --- |
 | `core/index.js` | Server entry. |
 | `core/src/host/` | Host-side helpers (non-hooks). |
-| `core/src/host/delivery-board.js` | The delivery board (#1173): one read-only snapshot of the repository's GitHub issues, pull requests and CI runs, laid out by delivery stage. Every request is a `GET`; the token is read host-side (`rt.secretValue`, falling back to none) and never leaves the payload, which carries no credential. `read({ force })` caches per URL with the ETag and revalidates with `If-None-Match`, serving the old snapshot on a 304 and while a refresh runs; there is no timer — the client asks. Issues are placed by label (`agent-working` → working, `agent-ready` → ready, else original), pull requests in review, and pull requests merged, and issues or pull requests closed, since the day start in done today — a pull request closed without merging counts, so finished work never sits in no column. A PR card carries the verdict badge of a recent `Verdict:` on its own tip (an old-tip verdict does not count; CI decides), and the header reports the Asia/Shanghai day, the 7-day window and CI as `counted` of `total`. Endpoint `delivery-board`, allowed to any signed-in member because it reads one repository rather than an owner's Bots. Tests: `core/test/delivery-board.test.js`. |
+| `core/src/host/delivery-board.js` | The delivery board (#1173): one read-only snapshot of the repository's GitHub issues, pull requests and CI runs, laid out by delivery stage. Every request is a `GET`; the token is read host-side (`rt.secretValue`, falling back to none) and never leaves the payload, which carries no credential. `read({ force })` caches per URL with the ETag and revalidates with `If-None-Match`, serving the old snapshot on a 304 and while a refresh runs; there is no timer — the client asks. Issues are placed by label (`agent-working` → working, `agent-ready` → ready, else original), pull requests in review, and pull requests merged, and issues or pull requests closed, since the day start in done today — a pull request closed without merging counts, so finished work never sits in no column. A PR card carries the verdict badge of a recent `Verdict:` on its own tip (an old-tip verdict does not count; CI decides) — every comment page is read, so a verdict past the first hundred comments still decides — and an issue is linked to a pull request only when a closing keyword (`close`/`fix`/`resolve` with `#n`) names it, never because a body merely mentions the number (#1189), and the header reports the Asia/Shanghai day, the 7-day window and CI as `counted` of `total`. Endpoint `delivery-board`, allowed to any signed-in member because it reads one repository rather than an owner's Bots. Tests: `core/test/delivery-board.test.js`. |
 | `core/src/host/prompt.js` | The Bot system prompt; a Main Bot's section is headed `You are the Main Bot` and uses that one term, the one the UI shows (#276). The team rules state the idle default (#699): a Bot does nothing until the user addresses it, it is @-mentioned in a group, its parent gives it a task, or unfinished tasks it was already given are still on its board. It does not invent work. A coordinator acts on a user task rather than on its own initiative — an addressed Bot answers as before. |
 | `core/src/host/room-membership.js` | Typed room participants (human / local bot / granted remote), authorship, membership-window history (#50); rooms with friends' Bots or other owners take granted local Bots only and evict a Bot that stops qualifying (#90). Face-to-face rooms (#83) reuse this model (`origin: 'face-to-face'`, room-only remotes without a person-grant, `upsertFaceRoom` / `syncFaceParticipants` / `applyFaceRoomLine`); a face-room local Bot is `viaCode` (brought by number), `evictUnshared` skips face rooms (membership is the server's), and a Bot that returns to the member list drops the empty local `left` line (#236). Refusing an ungranted local Bot next to a friend's Bot throws `ungrantedRefusal`: a message that names the Bot, the friend and the two ways out, plus `error.details` `{ reason: 'ungranted-local', bots, remotes }`; `channel.js` `fail` forwards a plain-object `details` (empty otherwise) and `client/api.js` puts it on the thrown error, where `client/new-chat.js` draws a card (**Copy invite link for {Bot}** via `__flybot__.social.createInvite`, **Leave {Bot} out**) read through `ungrantedLocalFix` in `shared/remote-label.js`; the same file's `copiedToClipboard` reports whether the invite link really reached the clipboard (none, or a refused write, is `false`), and the card then shows the link to copy by hand instead of saying it was copied. |
 | `core/src/host/face-mentions.js` | Who "@" calls in a face-to-face room (#287): every alias of a participant (`faceAliases`, kept in `shared/remote-label.js` so the bubble chips the same names, #305; bare name, the picker label `<owner>'s Chief` / `<name> · <owner>'s Bot`, `<name> · <owner>`, handle), local and friend's Bots told apart. `faceMentions().handles` is the handle of every Bot a line names (the explicit mention list a sender attaches, #293) and `mentionedByHandle` resolves a received list to my Bots (`known` counts handles the room has). |

patches/ds-bot/files/src/host/delivery-board.js

@@ -38,6 +38,11 @@ const orderLabels = (names) => [...names].sort((a, b) => {
 // The issues endpoint returns pull requests too; the pulls endpoint never does, so the two
 // lists are told apart by that field rather than by sniffing a PR's shape.
 const isPull = (row) => row.pull_request !== undefined || row.merge_commit_sha !== undefined
+// GitHub's closing keywords are the only mention that links a pull request to an issue (#1189).
+// `also see #12`, `same as #12` or a changelog line quoting a number is prose, and a card that
+// showed those as a link would claim a pull request that closes nothing.
+const CLOSING_KEYWORD = /\b(?:close[sd]?|fix(?:es|ed)?|resolve[sd]?)[ \t]*:?[ \t]*#(\d+)/gi
+const closingLinks = (text) => [...String(text).matchAll(CLOSING_KEYWORD)].map(match => Number(match[1]))
 const verdictOf = (comments) => {
   for (let index = (comments ?? []).length - 1; index >= 0; index -= 1) {
     const body = String(comments[index]?.body ?? '')
@@ -125,8 +130,13 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n
     return rows
   }
 
+  // Every page, not only the first (#1189). Past 100 comments — ordinary on this repository
+  // (#1170, #1183) — the newest verdict sat on a page nothing read, and the badge fell back to
+  // "no verdict at this tip" while one existed. The cap is the same MAX_PAGES as every other list
+  // and each page is a cached GET (ETag), so a long thread costs up to MAX_PAGES reads of one URL
+  // and a normal one still costs one.
   async function commentsOf(number) {
-    return get(`${base}/issues/${number}/comments?per_page=100&page=1`)
+    return pages(`/issues/${number}/comments`)
   }
 
   async function runsOfDay(from, to) {
@@ -238,9 +248,7 @@ export function createDeliveryBoard({ repo, token, request = fetch, now = Date.n
     const pullRows = openPulls
     const linkedTo = new Map()
     for (const pr of pullRows) {
-      const text = `${pr.title ?? ''}\n${pr.body ?? ''}`
-      for (const match of text.matchAll(/#(\d+)/g)) {
-        const number = Number(match[1])
+      for (const number of closingLinks(`${pr.title ?? ''}\n${pr.body ?? ''}`)) {
         if (!linkedTo.has(number)) linkedTo.set(number, pr.number)
       }
     }

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

@@ -8,7 +8,7 @@ import { createDeliveryBoard, DELIVERY_COLUMNS, install, median, startOfDay } fr
 const NOW = Date.parse('2026-10-25T04:00:00Z') // 12:00 in Asia/Shanghai
 const AT = (iso) => Date.parse(iso)
 
-function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [] } = {}) {
+function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra = [], openPullsExtra = [] } = {}) {
   const calls = []
   const write = (body, status = 200) => ({ ok: status < 400, status, headers: { get: () => 'etag-1' }, json: async () => body })
   const request = async (url, init = {}) => {
@@ -29,6 +29,7 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra
       return write(page === 1 ? [
         { number: 40, title: 'Pending verdict', html_url: 'https://x/40', body: 'Owner: Dev', head: { sha: 'aaaaaaa1' }, mergeable_state: 'unstable', created_at: '2026-10-24T00:00:00Z', updated_at: '2026-10-24T10:00:00Z', labels: [] },
         { number: 41, title: 'Fix the thing', html_url: 'https://x/41', body: 'Fixes #12', head: { sha: 'bbbbbbb2' }, mergeable_state: 'clean', created_at: '2026-10-23T00:00:00Z', updated_at: '2026-10-24T09:00:00Z', labels: [{ name: 'agent-pr' }] },
+        ...openPullsExtra,
       ] : [])
     }
     if (path === '/pulls' && parsed.searchParams.get('state') === 'all') {
@@ -72,7 +73,10 @@ function fakeGitHub({ comments = {}, checks = {}, closedExtra = [], mergedExtra
     }
     if (/^\/issues\/\d+\/comments$/.test(path)) {
       const number = Number(path.split('/')[2])
-      return write(comments[number] ?? [])
+      const value = comments[number] ?? []
+      // A thread longer than one page is passed as pages; a flat array is page one (#1189).
+      const pagesOf = Array.isArray(value[0]) ? value : [value]
+      return write(pagesOf[page - 1] ?? [])
     }
     if (/^\/commits\/.+\/check-runs$/.test(path)) {
       const sha = path.split('/')[2]
@@ -115,6 +119,23 @@ test('five columns come from one snapshot, an issue and its PR linked (#1173)',
   assert.deepEqual(snapshot.counts, { raw: 2, ready: 1, working: 1, review: 2, 'done-today': 2 })
 })
 
+test('only a closing keyword links a pull request to an issue (#1189)', async () => {
+  const { board } = build({
+    openPullsExtra: [
+      { number: 43, title: 'Related work', html_url: 'https://x/43', body: 'also see #10, same as #12', head: { sha: 'ddddddd4' }, mergeable_state: 'clean', created_at: '2026-10-22T00:00:00Z', updated_at: '2026-10-24T08:00:00Z', labels: [] },
+      { number: 44, title: 'Close it', html_url: 'https://x/44', body: 'Closes: #13', head: { sha: 'eeeeeee5' }, mergeable_state: 'clean', created_at: '2026-10-22T00:00:00Z', updated_at: '2026-10-24T08:00:00Z', labels: [] },
+    ],
+  })
+  const snapshot = await board.read()
+  const card = (column, number) => snapshot.columns[column].find(item => item.number === number)
+  // "also see #10, same as #12" is prose: #10 links to nothing, and #12 keeps the pull request
+  // that closes it (41), not the one that only mentions it.
+  assert.equal(card('raw', 10).linked, null)
+  assert.equal(card('working', 12).linked, 41)
+  // A closing keyword with a colon counts; nothing else does.
+  assert.equal(card('raw', 13).linked, 44)
+})
+
 test('a pull request closed today without a merge is done today, not nowhere (#1173)', async () => {
   const { board } = build({
     closedExtra: [
@@ -180,6 +201,25 @@ test('a verdict line wins over an Attestation that names an older sha (#1173)',
   assert.equal(card.badge, 'pass-enqueue')
 })
 
+test('a verdict on a later page of comments still decides the badge (#1189)', async () => {
+  // 120 comments with the newest verdict at index 101: on a board that reads one page it never
+  // appears, and the card says "no verdict at this tip" while the tip has one.
+  const firstPage = Array.from({ length: 100 }, (_, index) => ({ body: `note ${index}` }))
+  const secondPage = [
+    { body: 'note 100' },
+    { body: 'Verdict: INC at bbbbbbb2' },
+    ...Array.from({ length: 18 }, (_, index) => ({ body: `note ${102 + index}` })),
+  ]
+  const { board, github } = build({
+    comments: { 41: [firstPage, secondPage] },
+    checks: { bbbbbbb2: [{ status: 'completed', conclusion: 'success' }] },
+  })
+  const snapshot = await board.read()
+  const card = snapshot.columns.review.find(item => item.number === 41)
+  assert.equal(card.badge, 'inc')
+  assert.ok(github.calls.some(call => call.path === '/issues/41/comments' && /[?&]page=2(&|$)/.test(call.query)))
+})
+
 test('the token is read per request, and install loads the secrets first (#1173)', async () => {
   // A token captured at install time is empty until the host loads secrets; every request must
   // resolve it again, and the first read must ask the host to load them (#1173 INC).