Keep an unknown timeZone from throwing out of the whole Project board

changelog.d/1849-timezone-fallback.fixed.md

@@ -0,0 +1 @@
+An unknown `timeZone` in a Project board read no longer takes the whole board down: the zone is resolved once in `project-metrics.js`, an unknown name falls back to the named `UTC`, and the answer says which zone it used (`timeZone`), while rows, queues and decisions keep answering and a valid zone reads exactly as before ([#1849](https://github.com/ThinkFlowLab/FlyBot/issues/1849)).

core/src/host/project-metrics.js

@@ -16,8 +16,23 @@ const timeOf = value => {
   return Number.isFinite(ms) ? ms : null
 }
 
+// One zone, one decision (#1849): `timeZone` reaches this module from a request body, and an
+// unknown name used to throw out of the whole board read — rows, queues and decisions with it.
+// It is validated here once, and an unknown name answers the named `UTC` rather than guessing:
+// callers pass the resolved value to `dayKey`, to the answer and to the cache key.
+export function resolveTimeZone(timeZone = 'UTC') {
+  const wanted = text(timeZone) || 'UTC'
+  try {
+    new Intl.DateTimeFormat('en-CA', { timeZone: wanted })
+    return wanted
+  } catch {
+    return 'UTC'
+  }
+}
+
 // The natural day a timestamp falls in, in the caller's time zone. `en-CA` prints ISO order, which
-// is what the comparison below needs; an unknown zone throws and the caller degrades.
+// is what the comparison below needs; an unknown zone throws here, so a caller that takes one from
+// outside (the board read) resolves it with `resolveTimeZone` first.
 export function dayKey(ms, timeZone = 'UTC') {
   if (!Number.isFinite(ms)) return ''
   const parts = new Intl.DateTimeFormat('en-CA', { timeZone, year: 'numeric', month: '2-digit', day: '2-digit' })
@@ -99,16 +114,19 @@ export function summarizeMetrics({ now = Date.now(), timeZone = 'UTC', source, t
   // #1848: the running rows are the board's own visible cards in every branch, so a source that is
   // missing or failed no longer empties a table that needs no token to read.
   const running = () => taskList(tasks).map(task => runningOf(task, now)).filter(row => row !== null)
-  const unavailable = (reason) => ({ ok: false, reason, timeZone, generatedAt: now, metrics: emptyMetrics(), running: running() })
+  // #1849: one zone, one decision — an unknown name is answered as the named `UTC` here, not thrown
+  // out of the whole board read.
+  const zone = resolveTimeZone(timeZone)
+  const unavailable = (reason) => ({ ok: false, reason, timeZone: zone, generatedAt: now, metrics: emptyMetrics(), running: running() })
   if (source === undefined || source === null) return unavailable('no-source')
   if (source.ok !== true) return unavailable(text(source.reason) || 'source-unavailable')
   const pullRequests = Array.isArray(source.pullRequests) ? source.pullRequests : []
   const issues = Array.isArray(source.issues) ? source.issues : []
-  const key = dayKey(now, timeZone)
-  const day = { key, timeZone }
+  const key = dayKey(now, zone)
+  const day = { key, timeZone: zone }
   return {
     ok: true,
-    timeZone,
+    timeZone: zone,
     generatedAt: now,
     metrics: {
       prsOpenedToday: countToday(pullRequests, item => item.created_at, day),
@@ -133,10 +151,13 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no
   // shared entry must not answer one board with the numbers of the board that filled it.
   const liveRunning = () => taskList(tasks).map(task => runningOf(task, now)).filter(row => row !== null)
   const withLive = value => ({ ...value, metrics: { ...value.metrics, taskDoneMedianMs: median(doneTaskDurations(tasks, now)) }, running: liveRunning() })
+  // #1849: resolved once, before any early return, so the cache key, every `summarizeMetrics` call
+  // and the answer all carry the same zone.
+  const zone = resolveTimeZone(timeZone)
   const bearer = text(token)
-  if (bearer === '') return { ...summarizeMetrics({ now, timeZone, tasks, source: { ok: false, reason: 'no-credentials' } }), cache: 'miss' }
-  if (typeof fetchImpl !== 'function') return { ...summarizeMetrics({ now, timeZone, tasks, source: { ok: false, reason: 'no-fetch' } }), cache: 'miss' }
-  const key = `${repo}|${timeZone}|${Math.floor(now / ttlMs)}`
+  if (bearer === '') return { ...summarizeMetrics({ now, timeZone: zone, tasks, source: { ok: false, reason: 'no-credentials' } }), cache: 'miss' }
+  if (typeof fetchImpl !== 'function') return { ...summarizeMetrics({ now, timeZone: zone, tasks, source: { ok: false, reason: 'no-fetch' } }), cache: 'miss' }
+  const key = `${repo}|${zone}|${Math.floor(now / ttlMs)}`
   const hit = cacheStore.get(key)
   if (hit !== undefined && hit.expiresAt > now) return { ...withLive(hit.value), cache: 'hit' }
   const since = new Date(now - RECENT_MS).toISOString()
@@ -154,9 +175,9 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no
     ])
     source = { ok: true, pullRequests, issues: issues.filter(item => item.pull_request === undefined) }
   } catch (error) {
-    return { ...summarizeMetrics({ now, timeZone, tasks, source: { ok: false, reason: `read-failed: ${error.message}` } }), cache: 'miss' }
+    return { ...summarizeMetrics({ now, timeZone: zone, tasks, source: { ok: false, reason: `read-failed: ${error.message}` } }), cache: 'miss' }
   }
-  const value = summarizeMetrics({ now, timeZone, source, tasks })
+  const value = summarizeMetrics({ now, timeZone: zone, source, tasks })
   // Written field by field rather than by spreading, so a caller-scoped number added later cannot
   // ride into a shared entry by accident: this list is the whole shape the cache may hold.
   const { prsOpenedToday, prsMergedToday, issuesOpenedToday, issuesClosedToday, prMergeMedianMs, issueCloseMedianMs } = value.metrics

core/test/project-board.test.js

@@ -137,6 +137,35 @@ test('the board takes its token from the host secret store and its zone from the
   }
 })
 
+// #1849: the request body's `timeZone` reaches `Intl` unvalidated, so one unknown name used to
+// throw out of the whole board read — rows, queues and decisions with it — instead of costing the
+// seven numbers. The board still returns its cards, and the metrics answer says which zone it used.
+test('#1849: an unknown zone does not take the board down with the metrics', async () => {
+  const dir = mkdtempSync(join(tmpdir(), 'ds-bot-board-tz-'))
+  const { rt, stop } = await start(dir)
+  const realSecret = rt.secretValue
+  const realFetch = globalThis.fetch
+  rt.secretValue = name => (name === 'GITHUB_TOKEN' ? 'tok' : undefined)
+  globalThis.fetch = async () => ({ ok: true, json: async () => [] })
+  try {
+    const before = readFileSync(join(dir, 'state.json'))
+    const board = await rt.projectBoard('project-p1', null, { timeZone: 'Not/AZone' })
+    assert.deepEqual(board.rows.map(row => row.id), ['task-group'], 'the cards are still there')
+    assert.equal(board.counts.waiting, 0)
+    assert.equal(board.metrics.ok, true)
+    // The answer says which zone it used instead of inheriting the bad name.
+    assert.equal(board.metrics.timeZone, 'UTC')
+    // The fallback is decided in this read only: nothing about the request's zone is written back.
+    assert.deepEqual(readFileSync(join(dir, 'state.json')), before, 'the fallback is not saved to state')
+    assert.equal(before.includes('Not/AZone'), false, 'the bad name is not written to state')
+  } finally {
+    globalThis.fetch = realFetch
+    rt.secretValue = realSecret
+    await stop()
+    rmSync(dir, { recursive: true, force: true })
+  }
+})
+
 test('an empty Project answers [] and a missing one is not an error', async () => {
   const { rt, stop } = await start()
   try {

core/test/project-metrics.test.js

@@ -124,3 +124,63 @@ test('the task median reads the store stamps only', () => {
   const out = summarizeMetrics({ now: NOW, timeZone: 'UTC', source: { ok: true, pullRequests: [], issues: [] }, tasks })
   assert.equal(out.metrics.taskDoneMedianMs, 2 * HOUR)
 })
+
+// #1849: a request body's `timeZone` reaches `Intl` unvalidated, so one unknown name used to throw
+// out of the whole board read instead of costing the seven numbers. The zone is resolved once, and
+// an unknown name answers the named `UTC` — on every path that answers, and in the cache key too.
+const BAD_ZONE = 'Not/AZone'
+
+test('#1849: an unknown zone answers the named UTC fallback on every path', async () => {
+  const source = { ok: true, pullRequests: [pr('2026-10-30T10:00:00Z', null)], issues: [] }
+  const direct = summarizeMetrics({ now: NOW, timeZone: BAD_ZONE, source })
+  assert.equal(direct.timeZone, 'UTC')
+  assert.equal(direct.metrics.prsOpenedToday, 1)
+  assert.equal(summarizeMetrics({ now: NOW, timeZone: BAD_ZONE }).timeZone, 'UTC')
+  assert.equal(summarizeMetrics({ now: NOW, timeZone: BAD_ZONE, source: { ok: false, reason: 'x' } }).timeZone, 'UTC')
+
+  const noCredentials = await readMetrics({ repo: 'o/r', token: '', timeZone: BAD_ZONE, now: NOW })
+  assert.equal(noCredentials.timeZone, 'UTC')
+  assert.equal(noCredentials.ok, false)
+
+  const noFetch = await readMetrics({ repo: 'o/r', token: 'tok', timeZone: BAD_ZONE, now: NOW })
+  assert.equal(noFetch.timeZone, 'UTC')
+
+  const readFailure = await readMetrics({
+    repo: 'o/r', token: 'tok', timeZone: BAD_ZONE, now: NOW,
+    fetchImpl: async () => ({ ok: false, status: 500, json: async () => ({}) }),
+  })
+  assert.equal(readFailure.timeZone, 'UTC')
+  assert.equal(readFailure.ok, false)
+
+  // The cache key carries the resolved zone: a bad name shares the UTC entry instead of making a
+  // second one, and the hit still answers `UTC`.
+  const cacheStore = new Map()
+  const fetchImpl = async () => ({ ok: true, json: async () => [] })
+  const miss = await readMetrics({ repo: 'o/r', token: 'tok', timeZone: 'UTC', now: NOW, fetchImpl, cacheStore })
+  assert.equal(miss.cache, 'miss')
+  const hit = await readMetrics({ repo: 'o/r', token: 'tok', timeZone: BAD_ZONE, now: NOW, fetchImpl, cacheStore })
+  assert.equal(hit.cache, 'hit')
+  assert.equal(hit.timeZone, 'UTC')
+  assert.equal(cacheStore.size, 1)
+
+  const success = await readMetrics({ repo: 'o/r', token: 'tok', timeZone: BAD_ZONE, now: NOW, fetchImpl, cacheStore: new Map() })
+  assert.equal(success.timeZone, 'UTC')
+  assert.equal(success.ok, true)
+})
+
+test('#1849: a valid zone keeps the day boundary and the answer it had', () => {
+  const source = {
+    ok: true,
+    pullRequests: [pr('2026-10-30T10:00:00Z', '2026-10-30T11:00:00Z')],
+    issues: [issue('2026-10-30T09:00:00Z', null)],
+  }
+  const shanghai = summarizeMetrics({ now: NOW, timeZone: 'Asia/Shanghai', source })
+  assert.equal(shanghai.timeZone, 'Asia/Shanghai')
+  assert.equal(dayKey(NOW, 'Asia/Shanghai'), '2026-10-30')
+  assert.deepEqual(shanghai.metrics, {
+    prsOpenedToday: 1, prsMergedToday: 1, issuesOpenedToday: 1, issuesClosedToday: 0,
+    prMergeMedianMs: HOUR, issueCloseMedianMs: null, taskDoneMedianMs: null,
+  })
+  // A zone `Intl` accepts but the host would never guess stays as sent, and UTC still reads one day.
+  assert.equal(summarizeMetrics({ now: NOW, timeZone: 'UTC', source }).metrics.prsOpenedToday, 1)
+})

docs/subsystems/core-host.md

@@ -790,7 +790,7 @@ the host side only, so the tier stays T2 and `core/src/shared/` is untouched.
 
 ## Project metrics (#1829)
 
-`core/src/host/project-metrics.js` answers the Project board's seven numbers: PRs opened and merged today, issues opened and closed today, and the seven-day medians of PR merge, issue close and task completion durations. The four day counts compare `created_at` / `merged_at` / `closed_at` against the reader's own time zone (`Intl.DateTimeFormat`), not the host clock, and the local task half reads `createdAt` / `completedAt` from `state.tasks`. The running table and the task-done median read one board's own cards, not the deployment's task store: `project-board.js` hands `readMetrics` the same `mine` list its rows were built from (#1848), so a card of another Project, or one this reader is refused, appears in neither, and every branch (no credential, no fetch, a failed read, a cache hit, a success) reproduces both from that same list. The five-minute cache is shared by every Project on the repository, so an entry holds only the repository-level numbers (`ok`, the time zone, the stamp and the six GitHub counts and medians); `running` and `taskDoneMedianMs` are re-projected from the caller's cards on a hit exactly as on a miss (#1880). The numbers ride on the existing `project-board` endpoint (`store.js` carries the `metrics` section beside the rows), which passes the host secret store's `GITHUB_TOKEN` and the reader's time zone from the request, requires the Project, and answers `ok: false` with `null` values when the credential is missing or a read fails — the client paints `—`, never `0`. One five-minute in-process cache keys on repository, time zone and window, so a hit spends no source call; a failed read is not cached. `runningOf` projects a claimed card into the running table's row (who, claim age, source, last activity).
+`core/src/host/project-metrics.js` answers the Project board's seven numbers: PRs opened and merged today, issues opened and closed today, and the seven-day medians of PR merge, issue close and task completion durations. The four day counts compare `created_at` / `merged_at` / `closed_at` against the reader's own time zone (`Intl.DateTimeFormat`), not the host clock — the zone arrives in the request body, so `resolveTimeZone` validates it once and an unknown name answers the named `UTC` instead of throwing out of the whole board read, rows, queues and decisions with it (#1849) — and the local task half reads `createdAt` / `completedAt` from `state.tasks`. The running table and the task-done median read one board's own cards, not the deployment's task store: `project-board.js` hands `readMetrics` the same `mine` list its rows were built from (#1848), so a card of another Project, or one this reader is refused, appears in neither, and every branch (no credential, no fetch, a failed read, a cache hit, a success) reproduces both from that same list. The five-minute cache is shared by every Project on the repository, so an entry holds only the repository-level numbers (`ok`, the time zone, the stamp and the six GitHub counts and medians); `running` and `taskDoneMedianMs` are re-projected from the caller's cards on a hit exactly as on a miss (#1880). The numbers ride on the existing `project-board` endpoint (`store.js` carries the `metrics` section beside the rows), which passes the host secret store's `GITHUB_TOKEN` and the reader's time zone from the request, requires the Project, and answers `ok: false` with `null` values when the credential is missing or a read fails — the client paints `—`, never `0`. One five-minute in-process cache keys on repository, time zone and window, so a hit spends no source call; a failed read is not cached. `runningOf` projects a claimed card into the running table's row (who, claim age, source, last activity).
 
 ## The Project run seam (#1856)
 

patches/ds-bot/files/src/host/project-metrics.js

@@ -16,8 +16,23 @@ const timeOf = value => {
   return Number.isFinite(ms) ? ms : null
 }
 
+// One zone, one decision (#1849): `timeZone` reaches this module from a request body, and an
+// unknown name used to throw out of the whole board read — rows, queues and decisions with it.
+// It is validated here once, and an unknown name answers the named `UTC` rather than guessing:
+// callers pass the resolved value to `dayKey`, to the answer and to the cache key.
+export function resolveTimeZone(timeZone = 'UTC') {
+  const wanted = text(timeZone) || 'UTC'
+  try {
+    new Intl.DateTimeFormat('en-CA', { timeZone: wanted })
+    return wanted
+  } catch {
+    return 'UTC'
+  }
+}
+
 // The natural day a timestamp falls in, in the caller's time zone. `en-CA` prints ISO order, which
-// is what the comparison below needs; an unknown zone throws and the caller degrades.
+// is what the comparison below needs; an unknown zone throws here, so a caller that takes one from
+// outside (the board read) resolves it with `resolveTimeZone` first.
 export function dayKey(ms, timeZone = 'UTC') {
   if (!Number.isFinite(ms)) return ''
   const parts = new Intl.DateTimeFormat('en-CA', { timeZone, year: 'numeric', month: '2-digit', day: '2-digit' })
@@ -99,16 +114,19 @@ export function summarizeMetrics({ now = Date.now(), timeZone = 'UTC', source, t
   // #1848: the running rows are the board's own visible cards in every branch, so a source that is
   // missing or failed no longer empties a table that needs no token to read.
   const running = () => taskList(tasks).map(task => runningOf(task, now)).filter(row => row !== null)
-  const unavailable = (reason) => ({ ok: false, reason, timeZone, generatedAt: now, metrics: emptyMetrics(), running: running() })
+  // #1849: one zone, one decision — an unknown name is answered as the named `UTC` here, not thrown
+  // out of the whole board read.
+  const zone = resolveTimeZone(timeZone)
+  const unavailable = (reason) => ({ ok: false, reason, timeZone: zone, generatedAt: now, metrics: emptyMetrics(), running: running() })
   if (source === undefined || source === null) return unavailable('no-source')
   if (source.ok !== true) return unavailable(text(source.reason) || 'source-unavailable')
   const pullRequests = Array.isArray(source.pullRequests) ? source.pullRequests : []
   const issues = Array.isArray(source.issues) ? source.issues : []
-  const key = dayKey(now, timeZone)
-  const day = { key, timeZone }
+  const key = dayKey(now, zone)
+  const day = { key, timeZone: zone }
   return {
     ok: true,
-    timeZone,
+    timeZone: zone,
     generatedAt: now,
     metrics: {
       prsOpenedToday: countToday(pullRequests, item => item.created_at, day),
@@ -133,10 +151,13 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no
   // shared entry must not answer one board with the numbers of the board that filled it.
   const liveRunning = () => taskList(tasks).map(task => runningOf(task, now)).filter(row => row !== null)
   const withLive = value => ({ ...value, metrics: { ...value.metrics, taskDoneMedianMs: median(doneTaskDurations(tasks, now)) }, running: liveRunning() })
+  // #1849: resolved once, before any early return, so the cache key, every `summarizeMetrics` call
+  // and the answer all carry the same zone.
+  const zone = resolveTimeZone(timeZone)
   const bearer = text(token)
-  if (bearer === '') return { ...summarizeMetrics({ now, timeZone, tasks, source: { ok: false, reason: 'no-credentials' } }), cache: 'miss' }
-  if (typeof fetchImpl !== 'function') return { ...summarizeMetrics({ now, timeZone, tasks, source: { ok: false, reason: 'no-fetch' } }), cache: 'miss' }
-  const key = `${repo}|${timeZone}|${Math.floor(now / ttlMs)}`
+  if (bearer === '') return { ...summarizeMetrics({ now, timeZone: zone, tasks, source: { ok: false, reason: 'no-credentials' } }), cache: 'miss' }
+  if (typeof fetchImpl !== 'function') return { ...summarizeMetrics({ now, timeZone: zone, tasks, source: { ok: false, reason: 'no-fetch' } }), cache: 'miss' }
+  const key = `${repo}|${zone}|${Math.floor(now / ttlMs)}`
   const hit = cacheStore.get(key)
   if (hit !== undefined && hit.expiresAt > now) return { ...withLive(hit.value), cache: 'hit' }
   const since = new Date(now - RECENT_MS).toISOString()
@@ -154,9 +175,9 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no
     ])
     source = { ok: true, pullRequests, issues: issues.filter(item => item.pull_request === undefined) }
   } catch (error) {
-    return { ...summarizeMetrics({ now, timeZone, tasks, source: { ok: false, reason: `read-failed: ${error.message}` } }), cache: 'miss' }
+    return { ...summarizeMetrics({ now, timeZone: zone, tasks, source: { ok: false, reason: `read-failed: ${error.message}` } }), cache: 'miss' }
   }
-  const value = summarizeMetrics({ now, timeZone, source, tasks })
+  const value = summarizeMetrics({ now, timeZone: zone, source, tasks })
   // Written field by field rather than by spreading, so a caller-scoped number added later cannot
   // ride into a shared entry by accident: this list is the whole shape the cache may hold.
   const { prsOpenedToday, prsMergedToday, issuesOpenedToday, issuesClosedToday, prMergeMedianMs, issueCloseMedianMs } = value.metrics