From f01a17c6c9a52c0e3e047447292960f22a12f9de Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:53:50 +0800 Subject: [PATCH] Compare the quiet window with the due instant and list only cards that wait Three items in the Project Assistant's digest, prepared together. A quiet window is wall clock in the owner's zone, while the digest is due at 09:00 Asia/Shanghai. Comparing the two strings is only right when both zones are the same, so a UTC owner whose window covers 01:00Z was dropped for a window that never touches their due time. The window is now tested against the due instant read back in the owner's zone, and the held label starts at the instant the digest was held at instead of at another zone's 09:00. The personal block now says how many decisions the five-card cap left out, and it lists only the cards that wait on the reader: blocked and result-ready, read from the card's own status. A row with no status keeps the older answer, so no card disappears silently. --- core/src/host/project-assistant.js | 65 ++++++++--- core/test/project-assistant.test.js | 105 +++++++++++++++++- docs/subsystems/core-host.md | 20 +++- .../files/src/host/project-assistant.js | 65 ++++++++--- .../files/test/project-assistant.test.js | 105 +++++++++++++++++- 5 files changed, 313 insertions(+), 47 deletions(-) diff --git a/core/src/host/project-assistant.js b/core/src/host/project-assistant.js index 158e22d39..a532b0326 100644 --- a/core/src/host/project-assistant.js +++ b/core/src/host/project-assistant.js @@ -70,8 +70,12 @@ export function buildDigest({ project, rows = [], since = null, files = [], at = const line = (head, entries) => [head, ...entries.map(entry => `- ${entry}`)] // The personal block comes last: the same rows answer the same four sections for every owner, // and only this one names the cards that wait on the person reading it. - const asks = owner === null ? [] : actionable.slice(0, MAX_ACTIONABLE).map(item => `${item.id} — ${item.ask}`) - const personal = asks.length === 0 ? [] : line(`Needs ${owner}'s decision:`, asks) + const shown = owner === null ? [] : actionable.slice(0, MAX_ACTIONABLE).map(item => `${item.id} — ${item.ask}`) + // The block stays five cards: a person with seven waiting is told how many are left over instead + // of being handed the whole board (#P-197). Five or fewer print exactly as they did before. + const rest = owner === null ? 0 : actionable.length - MAX_ACTIONABLE + if (rest > 0) shown.push(`… and ${rest} more waiting on you`) + const personal = shown.length === 0 ? [] : line(`Needs ${owner}'s decision:`, shown) const sections = [ ...(merged.length === 0 ? [] : line('Merged since the previous digest:', merged.map(row => `${row.id} (${row.source})`))), ...(closed.length === 0 ? [] : line('Closed:', closed.map(row => `${row.id} (${row.source})`))), @@ -87,26 +91,42 @@ export function buildDigest({ project, rows = [], since = null, files = [], at = return { changed: true, day: dayIn(at), text: [head, stateLine, ...sections].join('\n') } } -// The cards that need *this* owner's decision: still open, and owned by that owner. A card -// somebody else owns is their decision, and never appears in this owner's digest. The board's -// `assignee` is a Bot name or null, never a member id (#1733), so the owner comes from `ownerOf`: -// the host resolves it from the card's own Bot, and a hand-built row may carry `ownerId` itself. +// The cards that need *this* owner's decision: owned by that owner, and stopped where only the +// reader can move them. A card somebody else owns is their decision, and never appears in this +// owner's digest. The board's `assignee` is a Bot name or null, never a member id (#1733), so the +// owner comes from `ownerOf`: the host resolves it from the card's own Bot, and a hand-built row +// may carry `ownerId` itself. +const WAITING_STATUS = new Set(['blocked', 'result-ready']) export function actionableFor(rows, ownerId, ownerOf = row => text(row?.ownerId)) { const owner = text(ownerId) if (owner === '') return [] return (rows ?? []) .filter(row => row?.column !== 'done' && ownerOf(row) === owner) + // A row that carries no status keeps the older answer: absence is not evidence that the card + // is moving, and reading it as "not waiting" would silently delete a card from the digest. + .filter((row) => { + const status = text(row?.status) + return status === '' || WAITING_STATUS.has(status) + }) .sort((a, b) => (a.updatedAt ?? 0) - (b.updatedAt ?? 0) || String(a.id).localeCompare(String(b.id))) .map(row => ({ id: String(row.id), ask: askOf(row) })) } -// One line, because the block is a list of decisions and a card description may be long. +// One line, because the block is a list of decisions and a card description may be long. What the +// card is waiting for is its own status, not its column: a blocked card wants a call, a finished +// result wants acceptance, and anything else the caller chose to list is still a decision to make. +const waitOf = (row) => { + const status = text(row?.status) + if (status === 'blocked') return 'needs your call' + if (status === 'result-ready') return 'ready for your acceptance' + return 'decide what happens next' +} const askOf = (row) => { const title = text(row.title).replace(/\s+/g, ' ') const where = text(row.column) === '' ? 'open' : `still ${text(row.column)}` - if (title === '') return `decide what happens next — ${where}` + if (title === '') return `${waitOf(row)} — ${where}` const short = title.length > 80 ? `${title.slice(0, 77)}…` : title - return `decide what happens next — ${short} (${where})` + return `${waitOf(row)} — ${short} (${where})` } // A write is attributed by text, because one shared GitHub login makes it impossible to tell @@ -314,8 +334,13 @@ export function install(rt) { // A board row carries the card's identity and its column, not its title; the ask line needs a // subject, so the card's own record supplies it. const withTitles = rows => rows.map((row) => { - const title = text(rt.state?.tasks?.[String(row?.id ?? '')]?.title) - return title === '' ? row : { ...row, title } + const task = rt.state?.tasks?.[String(row?.id ?? '')] + const title = text(task?.title) + // The card's status comes from its own record here, not from a second read further down: a + // card that is still moving waits on nobody, and the digest decides that from this row (#P-195). + const status = text(task?.status) + const shaped = title === '' ? row : { ...row, title } + return status === '' ? shaped : { ...shaped, status } }) // A card's owner is the owner of the Bot that owns the card: the board names the Bot, and the @@ -356,23 +381,29 @@ export function install(rt) { const bodyFor = (project, built, who) => (who.ok === true ? [attribution(project, who.login), built.text].join('\n\n') : built.text) // One owner's digest. Generation may proceed while a quiet window holds delivery; a window that - // covers 09:00 delivers nothing that day at all, and that is the expected outcome, not a failure. + // covers the due instant delivers nothing that day at all, and that is the expected outcome, not a + // failure. const digestForOwner = async ({ project, owner, rows, cards, files, at, force, who }) => { const assistant = assistantOf(project) ?? project.assistant ?? null const record = assistant === null ? { } : ownerRecord(assistant, owner) const quiet = quietOf(assistant, owner) + const day = dayIn(at) + // 09:00 in the digest's own zone is an instant, and a window is wall clock in the owner's own + // zone: two zones' wall clocks are not comparable (#P-194). Asia/Shanghai has no DST, so the + // due instant is today's date at 09:00+08:00, read back as a clock in the owner's zone. + const due = clockIn(quiet.zone, Date.parse(`${day}T${DIGEST_TIME}:00+08:00`)) const clock = clockIn(quiet.zone, at) - const coversDue = inQuietHours(quiet, DIGEST_TIME) + const coversDue = inQuietHours(quiet, due) const memory = memoryOf(project, owner) - const day = dayIn(at) if (record.off === true) { return { owner, ok: true, skipped: true, optedOut: true, delivered: false, day, reason: 'this owner turned the digest off, so nothing is generated' } } - // A digest held earlier today goes out once, when the window ends, labelled as held. + // A digest held earlier today goes out once, when the window ends, labelled as held from the + // instant it was held at — not from 09:00, which is another zone's wall clock. const held = typeof record.held === 'object' && record.held !== null ? record.held : null if (held !== null && held.day === day && coversDue === false && inQuietHours(quiet, clock) === false) { delete record.held - const label = `held from ${DIGEST_TIME} to ${clock} by quiet hours ${quiet.from}–${quiet.to}` + const label = `held from ${clockIn(quiet.zone, held.at ?? at)} to ${clock} by quiet hours ${quiet.from}–${quiet.to}` const body = [bodyFor(project, { text: text(held.text) }, who), `(${label})`].join('\n') const sent = await deliverTo(project, owner, body) await rt.save?.() @@ -385,7 +416,7 @@ export function install(rt) { const note = { day: built.day, at, text: built.text, ...(who.ok === true ? {} : { error: `GitHub write refused: ${who.reason}` }) } rt.setProjectDigest?.(project.id, note, owner) if (coversDue) { - return { owner, ok: true, skipped: false, delivered: false, held: true, dropped: true, day: built.day, changed: built.changed, text: built.text, reason: `quiet hours ${quiet.from}–${quiet.to} cover ${DIGEST_TIME}, so nothing is delivered — the expected outcome` } + return { owner, ok: true, skipped: false, delivered: false, held: true, dropped: true, day: built.day, changed: built.changed, text: built.text, reason: `quiet hours ${quiet.from}–${quiet.to} cover ${due}, so nothing is delivered — the expected outcome` } } if (inQuietHours(quiet, clock)) { record.held = { day: built.day, at, text: built.text, from: quiet.from, to: quiet.to } diff --git a/core/test/project-assistant.test.js b/core/test/project-assistant.test.js index a55bd844e..5ba8cca6c 100644 --- a/core/test/project-assistant.test.js +++ b/core/test/project-assistant.test.js @@ -322,7 +322,7 @@ test('a quiet window holds the digest and delivers it once, labelled, when it en const once = await rt.projectAssistant.digest(project.id, { at: after }) assert.equal(once.delivered, 1) - assert.match(delivered[0].text, /held from 09:00 to 08:45 by quiet hours 07:00\u201308:30/) + assert.match(delivered[0].text, /held from 08:00 to 08:45 by quiet hours 07:00\u201308:30/) const twice = await rt.projectAssistant.digest(project.id, { at: after + 60_000 }) assert.equal(twice.delivered, 0, 'delivered once, not again') assert.equal(delivered.length, 1) @@ -349,6 +349,92 @@ test('a quiet window that covers 09:00 delivers nothing, and that is the expecte } }) +// P-194: the due instant is 09:00 Asia/Shanghai; a quiet window is wall clock in the owner's own +// zone. Two different zones' wall clocks are not comparable, so the window is tested against the +// due instant read in the owner's zone. Red first drives this through setOwner directly: the +// `project-assistant-owner` endpoint has no channel case, so no product-level path exists today +// and the trigger arrives with P-198. Until then this is a latent defect: the digest would be +// dropped silently once a UTC owner's window is set. +test('#P-194: a quiet window in the owner\'s zone is compared with the due instant, not with 09:00', async () => { + const { rt, stop } = await start() + try { + const project = await created(rt, { name: 'UtcWindow', ownerUserId: 'owner-1' }) + rt.projectBoard = () => [{ id: 't-utc', source: 'bot', column: 'in-progress', assignee: 'Owner One Bot', ownerId: 'owner-1', updatedAt: 1 }] + // 08:00–10:00 UTC does not contain the due instant (01:00Z = 09:00 Asia/Shanghai). + await rt.projectAssistant.setOwner(project.id, 'owner-1', { quiet: { on: true, from: '08:00', to: '10:00' }, zone: 'UTC' }) + const result = await rt.projectAssistant.digest(project.id, { at: AT }) // 01:00Z + const owner = result.owners[0] + assert.equal(result.delivered, 1, 'the window does not cover the due instant, so the digest goes out') + assert.notEqual(owner.dropped, true, 'nothing was dropped') + assert.doesNotMatch(String(owner.reason ?? ''), /cover/, 'no drop reason is recorded') + assert.notEqual(owner.held, true, 'nothing was held either') + } finally { + await stop() + } +}) + +test('#P-194: a window that really covers the due instant still drops the digest', async () => { + const { rt, delivered, stop } = await start() + try { + const project = await created(rt, { name: 'UtcCovered', ownerUserId: 'owner-1' }) + rt.projectBoard = () => [] + await rt.projectAssistant.setOwner(project.id, 'owner-1', { quiet: { on: true, from: '00:30', to: '01:30' }, zone: 'UTC' }) + const result = await rt.projectAssistant.digest(project.id, { at: AT }) // 01:00Z is inside it + const owner = result.owners[0] + assert.equal(result.delivered, 0, 'a window covering the due instant delivers nothing') + assert.equal(owner.dropped, true) + assert.match(owner.reason, /cover 01:00, so nothing is delivered — the expected outcome/) + assert.equal(delivered.length, 0) + } finally { + await stop() + } +}) + +// P-197: the personal block stays five cards, and says how many are left over; five or fewer +// print exactly as they did before. +test('#P-197: more than five decisions say how many are still waiting', () => { + const rows = Array.from({ length: MAX_ACTIONABLE + 2 }, (_, index) => ({ id: `t-${index}`, source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: index, title: `card ${index}`, status: 'blocked' })) + const items = actionableFor(rows, 'owner-1') + assert.equal(items.length, MAX_ACTIONABLE + 2, 'the helper keeps them all') + const built = buildDigest({ project: { id: 'p', name: 'More' }, rows, owner: 'owner-1', actionable: items, at: AT }) + const personal = built.text.slice(built.text.indexOf("Needs owner-1's decision:")) + const listed = personal.split('\n').filter(line => /^- t-\d/.test(line)) + assert.equal(listed.length, MAX_ACTIONABLE, 'the digest still lists at most MAX_ACTIONABLE cards') + assert.match(built.text, /^- … and 2 more waiting on you$/m) +}) + +test('#P-197: five or fewer decisions print exactly as before', () => { + const rows = Array.from({ length: MAX_ACTIONABLE }, (_, index) => ({ id: `t-${index}`, source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: index, title: `card ${index}`, status: 'blocked' })) + const built = buildDigest({ project: { id: 'p', name: 'Five' }, rows, owner: 'owner-1', actionable: actionableFor(rows, 'owner-1'), at: AT }) + assert.doesNotMatch(built.text, /more waiting on you/) + const personal = built.text.slice(built.text.indexOf("Needs owner-1's decision:")) + const listed = personal.split('\n').filter(line => /^- t-\d/.test(line)) + assert.equal(listed.length, MAX_ACTIONABLE) +}) + +// P-195: the personal block is what waits on the reader, not the whole board: a card still being +// worked on is somebody's move, a blocked card needs this reader's call, and a finished result +// needs their acceptance. +test('#P-195: only the cards that wait on the reader reach the personal block', () => { + const rows = [ + { id: 't-working', source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: 1, title: 'Still moving', status: 'working' }, + { id: 't-blocked', source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: 2, title: 'Stopped on a question', status: 'blocked' }, + { id: 't-ready', source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: 3, title: 'Back for acceptance', status: 'result-ready' }, + ] + const items = actionableFor(rows, 'owner-1') + assert.deepEqual(items.map(item => item.id), ['t-blocked', 't-ready'], 'a card that is still moving is nobody else\'s decision to make') + const built = buildDigest({ project: { id: 'p', name: 'Waiting' }, rows, owner: 'owner-1', actionable: items, at: AT }) + const personal = built.text.slice(built.text.indexOf("Needs owner-1's decision:")) + assert.match(personal, /Stopped on a question/) + assert.match(personal, /needs your call/) + assert.match(personal, /Back for acceptance/) + assert.match(personal, /ready for your acceptance/) + assert.doesNotMatch(personal, /Still moving/, 'a card still in progress is not a decision waiting on the reader') + const shared = built.text.slice(0, built.text.indexOf("Needs owner-1's decision:")) + assert.match(shared, /In progress, waiting on:/) + assert.match(shared, /t-working — Worker/, 'the shared section still lists it') +}) + test('opting out stops generation and pauses the 09:00 task, and re-enabling restores both', async () => { const { rt, service, delivered, stop } = await start() try { @@ -424,16 +510,27 @@ test('a card lists for the owner of the Bot that owns it, through the real task rt.state.bots.worker = { id: 'worker', name: 'Worker', role: 'Dev', sessionId: 'w', createdAt: 3, ownerId: 'owner-2' } await rt.projectEndpoint('project-bind', { kind: 'bot', id: 'a', projectId: project.id }) await rt.projectEndpoint('project-bind', { kind: 'bot', id: 'worker', projectId: project.id }) - await rt.taskEndpoint('task-create', { botId: 'worker', title: 'Pick the release date' }, null) - await rt.taskEndpoint('task-create', { botId: 'a', title: 'Rotate the deploy key' }, null) + const pick = (await rt.taskEndpoint('task-create', { botId: 'worker', title: 'Pick the release date' }, null)).id + const rotate = (await rt.taskEndpoint('task-create', { botId: 'a', title: 'Rotate the deploy key' }, null)).id + const moving = (await rt.taskEndpoint('task-create', { botId: 'a', title: 'Still moving' }, null)).id + // A card waits on its reader only once it has stopped: drive both to `blocked`, which the + // digest reads from the task store itself, and leave one of the owner's cards `working`. + for (const id of [pick, rotate]) { + await rt.taskEndpoint('task-accept', { id }, null) + await rt.taskEndpoint('task-start', { id }, null) + await rt.taskEndpoint('task-wait-input', { id, question: 'Which one?' }, null) + } + await rt.taskEndpoint('task-accept', { id: moving }, null) + await rt.taskEndpoint('task-start', { id: moving }, null) const rows = rt.projectBoard(project.id) - assert.equal(rows.length, 2) + assert.equal(rows.length, 3) assert.ok(rows.every(row => row.assignee === null), 'the board carries a Bot name or null, never a member id') assert.equal(actionableFor(rows, 'owner-1').length, 0, 'the raw helper sees no owner without a resolver') const result = await rt.projectAssistant.digest(project.id, { at: AT }) const [one, two] = result.owners assert.match(one.text, /Rotate the deploy key/, 'the card of the primary owner\'s Bot is theirs') assert.doesNotMatch(one.text, /Pick the release date/, 'another owner\'s card never appears in this digest') + assert.doesNotMatch(one.text, /Still moving/, 'a card the owner\'s Bot is still working on waits on nobody') assert.match(two.text, /Pick the release date/, 'the card of the second owner\'s Bot is theirs') assert.doesNotMatch(two.text, /Rotate the deploy key/) } finally { diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index e03f95e1a..ce840ca93 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -654,16 +654,26 @@ the host side only, so the tier stays T2 and `core/src/shared/` is untouched. `listProjectState(project.workDir)` once, then renders once per owner with that owner's memory and that owner's actionable block. Shared facts are identical in both texts; a card assigned to somebody else never appears in the "Needs 's decision:" block, and `MAX_ACTIONABLE` (5) - caps that block, each item carrying the task id and a one-line ask. A prompt-injected line cannot + caps that block, each item carrying the task id and a one-line ask. A sixth line, + `… and N more waiting on you`, counts what the cap left out; five or fewer print exactly the + lines they printed before (#P-197). The block holds the cards that wait on the reader and no + others: `blocked` ("needs your call") and `result-ready` ("ready for your acceptance"), read from + the card's own status through `withTitles`, so a card still `open`, `claimed` or `working` stays + only in the shared "In progress, waiting on:" section, and a row that carries no status keeps the + older answer rather than being dropped (#P-195). A prompt-injected line cannot cross: digest A carries no entry from B's memory because A's render is handed only A's entries. - **Quiet window per owner.** `rt.projectAssistant.setOwner(projectId, ownerId, { quiet, zone, off })` stores `{ quiet: { on, from, to }, zone, off }` on `project.assistant.owners[ownerId]`, never in `state.prefs` (the host-wide #1166 window keeps its own job, refusing a new or edited task). The - window is read in the owner's zone with `clockIn`, and: + window is wall clock in the owner's zone, so it is compared with the **due instant** — today's + 09:00 Asia/Shanghai, read back as a clock in the owner's zone with `clockIn` (#P-194) — never with + the string `09:00`, which is another zone's wall clock. Then: a digest due inside it is **held** — still generated and stored, then delivered once when the - window ends, labelled `held from 09:00 to by quiet hours –`; - a window that covers 09:00 delivers **nothing** that day, and that is the expected outcome, not a - failure (the note is still written, so the next day is still one a day). + window ends, labelled `held from to by quiet hours –`; + a window that covers the due instant delivers **nothing** that day, and that is the expected + outcome, not a failure (the note is still written, so the next day is still one a day). + `setOwner` has no channel case yet, so no product-level path reaches these controls today; the + window matters once that endpoint is wired (P-198). - **Opt-out.** `off: true` on the owner record stops generation for that owner (no note, no message) and the 09:00 task is **paused** through `rt.scheduleEndpoint('schedule-pause')` once every owner is off — the task is shared, so one owner turning back on **resumes** it diff --git a/patches/ds-bot/files/src/host/project-assistant.js b/patches/ds-bot/files/src/host/project-assistant.js index 158e22d39..a532b0326 100644 --- a/patches/ds-bot/files/src/host/project-assistant.js +++ b/patches/ds-bot/files/src/host/project-assistant.js @@ -70,8 +70,12 @@ export function buildDigest({ project, rows = [], since = null, files = [], at = const line = (head, entries) => [head, ...entries.map(entry => `- ${entry}`)] // The personal block comes last: the same rows answer the same four sections for every owner, // and only this one names the cards that wait on the person reading it. - const asks = owner === null ? [] : actionable.slice(0, MAX_ACTIONABLE).map(item => `${item.id} — ${item.ask}`) - const personal = asks.length === 0 ? [] : line(`Needs ${owner}'s decision:`, asks) + const shown = owner === null ? [] : actionable.slice(0, MAX_ACTIONABLE).map(item => `${item.id} — ${item.ask}`) + // The block stays five cards: a person with seven waiting is told how many are left over instead + // of being handed the whole board (#P-197). Five or fewer print exactly as they did before. + const rest = owner === null ? 0 : actionable.length - MAX_ACTIONABLE + if (rest > 0) shown.push(`… and ${rest} more waiting on you`) + const personal = shown.length === 0 ? [] : line(`Needs ${owner}'s decision:`, shown) const sections = [ ...(merged.length === 0 ? [] : line('Merged since the previous digest:', merged.map(row => `${row.id} (${row.source})`))), ...(closed.length === 0 ? [] : line('Closed:', closed.map(row => `${row.id} (${row.source})`))), @@ -87,26 +91,42 @@ export function buildDigest({ project, rows = [], since = null, files = [], at = return { changed: true, day: dayIn(at), text: [head, stateLine, ...sections].join('\n') } } -// The cards that need *this* owner's decision: still open, and owned by that owner. A card -// somebody else owns is their decision, and never appears in this owner's digest. The board's -// `assignee` is a Bot name or null, never a member id (#1733), so the owner comes from `ownerOf`: -// the host resolves it from the card's own Bot, and a hand-built row may carry `ownerId` itself. +// The cards that need *this* owner's decision: owned by that owner, and stopped where only the +// reader can move them. A card somebody else owns is their decision, and never appears in this +// owner's digest. The board's `assignee` is a Bot name or null, never a member id (#1733), so the +// owner comes from `ownerOf`: the host resolves it from the card's own Bot, and a hand-built row +// may carry `ownerId` itself. +const WAITING_STATUS = new Set(['blocked', 'result-ready']) export function actionableFor(rows, ownerId, ownerOf = row => text(row?.ownerId)) { const owner = text(ownerId) if (owner === '') return [] return (rows ?? []) .filter(row => row?.column !== 'done' && ownerOf(row) === owner) + // A row that carries no status keeps the older answer: absence is not evidence that the card + // is moving, and reading it as "not waiting" would silently delete a card from the digest. + .filter((row) => { + const status = text(row?.status) + return status === '' || WAITING_STATUS.has(status) + }) .sort((a, b) => (a.updatedAt ?? 0) - (b.updatedAt ?? 0) || String(a.id).localeCompare(String(b.id))) .map(row => ({ id: String(row.id), ask: askOf(row) })) } -// One line, because the block is a list of decisions and a card description may be long. +// One line, because the block is a list of decisions and a card description may be long. What the +// card is waiting for is its own status, not its column: a blocked card wants a call, a finished +// result wants acceptance, and anything else the caller chose to list is still a decision to make. +const waitOf = (row) => { + const status = text(row?.status) + if (status === 'blocked') return 'needs your call' + if (status === 'result-ready') return 'ready for your acceptance' + return 'decide what happens next' +} const askOf = (row) => { const title = text(row.title).replace(/\s+/g, ' ') const where = text(row.column) === '' ? 'open' : `still ${text(row.column)}` - if (title === '') return `decide what happens next — ${where}` + if (title === '') return `${waitOf(row)} — ${where}` const short = title.length > 80 ? `${title.slice(0, 77)}…` : title - return `decide what happens next — ${short} (${where})` + return `${waitOf(row)} — ${short} (${where})` } // A write is attributed by text, because one shared GitHub login makes it impossible to tell @@ -314,8 +334,13 @@ export function install(rt) { // A board row carries the card's identity and its column, not its title; the ask line needs a // subject, so the card's own record supplies it. const withTitles = rows => rows.map((row) => { - const title = text(rt.state?.tasks?.[String(row?.id ?? '')]?.title) - return title === '' ? row : { ...row, title } + const task = rt.state?.tasks?.[String(row?.id ?? '')] + const title = text(task?.title) + // The card's status comes from its own record here, not from a second read further down: a + // card that is still moving waits on nobody, and the digest decides that from this row (#P-195). + const status = text(task?.status) + const shaped = title === '' ? row : { ...row, title } + return status === '' ? shaped : { ...shaped, status } }) // A card's owner is the owner of the Bot that owns the card: the board names the Bot, and the @@ -356,23 +381,29 @@ export function install(rt) { const bodyFor = (project, built, who) => (who.ok === true ? [attribution(project, who.login), built.text].join('\n\n') : built.text) // One owner's digest. Generation may proceed while a quiet window holds delivery; a window that - // covers 09:00 delivers nothing that day at all, and that is the expected outcome, not a failure. + // covers the due instant delivers nothing that day at all, and that is the expected outcome, not a + // failure. const digestForOwner = async ({ project, owner, rows, cards, files, at, force, who }) => { const assistant = assistantOf(project) ?? project.assistant ?? null const record = assistant === null ? { } : ownerRecord(assistant, owner) const quiet = quietOf(assistant, owner) + const day = dayIn(at) + // 09:00 in the digest's own zone is an instant, and a window is wall clock in the owner's own + // zone: two zones' wall clocks are not comparable (#P-194). Asia/Shanghai has no DST, so the + // due instant is today's date at 09:00+08:00, read back as a clock in the owner's zone. + const due = clockIn(quiet.zone, Date.parse(`${day}T${DIGEST_TIME}:00+08:00`)) const clock = clockIn(quiet.zone, at) - const coversDue = inQuietHours(quiet, DIGEST_TIME) + const coversDue = inQuietHours(quiet, due) const memory = memoryOf(project, owner) - const day = dayIn(at) if (record.off === true) { return { owner, ok: true, skipped: true, optedOut: true, delivered: false, day, reason: 'this owner turned the digest off, so nothing is generated' } } - // A digest held earlier today goes out once, when the window ends, labelled as held. + // A digest held earlier today goes out once, when the window ends, labelled as held from the + // instant it was held at — not from 09:00, which is another zone's wall clock. const held = typeof record.held === 'object' && record.held !== null ? record.held : null if (held !== null && held.day === day && coversDue === false && inQuietHours(quiet, clock) === false) { delete record.held - const label = `held from ${DIGEST_TIME} to ${clock} by quiet hours ${quiet.from}–${quiet.to}` + const label = `held from ${clockIn(quiet.zone, held.at ?? at)} to ${clock} by quiet hours ${quiet.from}–${quiet.to}` const body = [bodyFor(project, { text: text(held.text) }, who), `(${label})`].join('\n') const sent = await deliverTo(project, owner, body) await rt.save?.() @@ -385,7 +416,7 @@ export function install(rt) { const note = { day: built.day, at, text: built.text, ...(who.ok === true ? {} : { error: `GitHub write refused: ${who.reason}` }) } rt.setProjectDigest?.(project.id, note, owner) if (coversDue) { - return { owner, ok: true, skipped: false, delivered: false, held: true, dropped: true, day: built.day, changed: built.changed, text: built.text, reason: `quiet hours ${quiet.from}–${quiet.to} cover ${DIGEST_TIME}, so nothing is delivered — the expected outcome` } + return { owner, ok: true, skipped: false, delivered: false, held: true, dropped: true, day: built.day, changed: built.changed, text: built.text, reason: `quiet hours ${quiet.from}–${quiet.to} cover ${due}, so nothing is delivered — the expected outcome` } } if (inQuietHours(quiet, clock)) { record.held = { day: built.day, at, text: built.text, from: quiet.from, to: quiet.to } diff --git a/patches/ds-bot/files/test/project-assistant.test.js b/patches/ds-bot/files/test/project-assistant.test.js index a55bd844e..5ba8cca6c 100644 --- a/patches/ds-bot/files/test/project-assistant.test.js +++ b/patches/ds-bot/files/test/project-assistant.test.js @@ -322,7 +322,7 @@ test('a quiet window holds the digest and delivers it once, labelled, when it en const once = await rt.projectAssistant.digest(project.id, { at: after }) assert.equal(once.delivered, 1) - assert.match(delivered[0].text, /held from 09:00 to 08:45 by quiet hours 07:00\u201308:30/) + assert.match(delivered[0].text, /held from 08:00 to 08:45 by quiet hours 07:00\u201308:30/) const twice = await rt.projectAssistant.digest(project.id, { at: after + 60_000 }) assert.equal(twice.delivered, 0, 'delivered once, not again') assert.equal(delivered.length, 1) @@ -349,6 +349,92 @@ test('a quiet window that covers 09:00 delivers nothing, and that is the expecte } }) +// P-194: the due instant is 09:00 Asia/Shanghai; a quiet window is wall clock in the owner's own +// zone. Two different zones' wall clocks are not comparable, so the window is tested against the +// due instant read in the owner's zone. Red first drives this through setOwner directly: the +// `project-assistant-owner` endpoint has no channel case, so no product-level path exists today +// and the trigger arrives with P-198. Until then this is a latent defect: the digest would be +// dropped silently once a UTC owner's window is set. +test('#P-194: a quiet window in the owner\'s zone is compared with the due instant, not with 09:00', async () => { + const { rt, stop } = await start() + try { + const project = await created(rt, { name: 'UtcWindow', ownerUserId: 'owner-1' }) + rt.projectBoard = () => [{ id: 't-utc', source: 'bot', column: 'in-progress', assignee: 'Owner One Bot', ownerId: 'owner-1', updatedAt: 1 }] + // 08:00–10:00 UTC does not contain the due instant (01:00Z = 09:00 Asia/Shanghai). + await rt.projectAssistant.setOwner(project.id, 'owner-1', { quiet: { on: true, from: '08:00', to: '10:00' }, zone: 'UTC' }) + const result = await rt.projectAssistant.digest(project.id, { at: AT }) // 01:00Z + const owner = result.owners[0] + assert.equal(result.delivered, 1, 'the window does not cover the due instant, so the digest goes out') + assert.notEqual(owner.dropped, true, 'nothing was dropped') + assert.doesNotMatch(String(owner.reason ?? ''), /cover/, 'no drop reason is recorded') + assert.notEqual(owner.held, true, 'nothing was held either') + } finally { + await stop() + } +}) + +test('#P-194: a window that really covers the due instant still drops the digest', async () => { + const { rt, delivered, stop } = await start() + try { + const project = await created(rt, { name: 'UtcCovered', ownerUserId: 'owner-1' }) + rt.projectBoard = () => [] + await rt.projectAssistant.setOwner(project.id, 'owner-1', { quiet: { on: true, from: '00:30', to: '01:30' }, zone: 'UTC' }) + const result = await rt.projectAssistant.digest(project.id, { at: AT }) // 01:00Z is inside it + const owner = result.owners[0] + assert.equal(result.delivered, 0, 'a window covering the due instant delivers nothing') + assert.equal(owner.dropped, true) + assert.match(owner.reason, /cover 01:00, so nothing is delivered — the expected outcome/) + assert.equal(delivered.length, 0) + } finally { + await stop() + } +}) + +// P-197: the personal block stays five cards, and says how many are left over; five or fewer +// print exactly as they did before. +test('#P-197: more than five decisions say how many are still waiting', () => { + const rows = Array.from({ length: MAX_ACTIONABLE + 2 }, (_, index) => ({ id: `t-${index}`, source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: index, title: `card ${index}`, status: 'blocked' })) + const items = actionableFor(rows, 'owner-1') + assert.equal(items.length, MAX_ACTIONABLE + 2, 'the helper keeps them all') + const built = buildDigest({ project: { id: 'p', name: 'More' }, rows, owner: 'owner-1', actionable: items, at: AT }) + const personal = built.text.slice(built.text.indexOf("Needs owner-1's decision:")) + const listed = personal.split('\n').filter(line => /^- t-\d/.test(line)) + assert.equal(listed.length, MAX_ACTIONABLE, 'the digest still lists at most MAX_ACTIONABLE cards') + assert.match(built.text, /^- … and 2 more waiting on you$/m) +}) + +test('#P-197: five or fewer decisions print exactly as before', () => { + const rows = Array.from({ length: MAX_ACTIONABLE }, (_, index) => ({ id: `t-${index}`, source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: index, title: `card ${index}`, status: 'blocked' })) + const built = buildDigest({ project: { id: 'p', name: 'Five' }, rows, owner: 'owner-1', actionable: actionableFor(rows, 'owner-1'), at: AT }) + assert.doesNotMatch(built.text, /more waiting on you/) + const personal = built.text.slice(built.text.indexOf("Needs owner-1's decision:")) + const listed = personal.split('\n').filter(line => /^- t-\d/.test(line)) + assert.equal(listed.length, MAX_ACTIONABLE) +}) + +// P-195: the personal block is what waits on the reader, not the whole board: a card still being +// worked on is somebody's move, a blocked card needs this reader's call, and a finished result +// needs their acceptance. +test('#P-195: only the cards that wait on the reader reach the personal block', () => { + const rows = [ + { id: 't-working', source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: 1, title: 'Still moving', status: 'working' }, + { id: 't-blocked', source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: 2, title: 'Stopped on a question', status: 'blocked' }, + { id: 't-ready', source: 'bot', column: 'in-progress', assignee: 'Worker', ownerId: 'owner-1', updatedAt: 3, title: 'Back for acceptance', status: 'result-ready' }, + ] + const items = actionableFor(rows, 'owner-1') + assert.deepEqual(items.map(item => item.id), ['t-blocked', 't-ready'], 'a card that is still moving is nobody else\'s decision to make') + const built = buildDigest({ project: { id: 'p', name: 'Waiting' }, rows, owner: 'owner-1', actionable: items, at: AT }) + const personal = built.text.slice(built.text.indexOf("Needs owner-1's decision:")) + assert.match(personal, /Stopped on a question/) + assert.match(personal, /needs your call/) + assert.match(personal, /Back for acceptance/) + assert.match(personal, /ready for your acceptance/) + assert.doesNotMatch(personal, /Still moving/, 'a card still in progress is not a decision waiting on the reader') + const shared = built.text.slice(0, built.text.indexOf("Needs owner-1's decision:")) + assert.match(shared, /In progress, waiting on:/) + assert.match(shared, /t-working — Worker/, 'the shared section still lists it') +}) + test('opting out stops generation and pauses the 09:00 task, and re-enabling restores both', async () => { const { rt, service, delivered, stop } = await start() try { @@ -424,16 +510,27 @@ test('a card lists for the owner of the Bot that owns it, through the real task rt.state.bots.worker = { id: 'worker', name: 'Worker', role: 'Dev', sessionId: 'w', createdAt: 3, ownerId: 'owner-2' } await rt.projectEndpoint('project-bind', { kind: 'bot', id: 'a', projectId: project.id }) await rt.projectEndpoint('project-bind', { kind: 'bot', id: 'worker', projectId: project.id }) - await rt.taskEndpoint('task-create', { botId: 'worker', title: 'Pick the release date' }, null) - await rt.taskEndpoint('task-create', { botId: 'a', title: 'Rotate the deploy key' }, null) + const pick = (await rt.taskEndpoint('task-create', { botId: 'worker', title: 'Pick the release date' }, null)).id + const rotate = (await rt.taskEndpoint('task-create', { botId: 'a', title: 'Rotate the deploy key' }, null)).id + const moving = (await rt.taskEndpoint('task-create', { botId: 'a', title: 'Still moving' }, null)).id + // A card waits on its reader only once it has stopped: drive both to `blocked`, which the + // digest reads from the task store itself, and leave one of the owner's cards `working`. + for (const id of [pick, rotate]) { + await rt.taskEndpoint('task-accept', { id }, null) + await rt.taskEndpoint('task-start', { id }, null) + await rt.taskEndpoint('task-wait-input', { id, question: 'Which one?' }, null) + } + await rt.taskEndpoint('task-accept', { id: moving }, null) + await rt.taskEndpoint('task-start', { id: moving }, null) const rows = rt.projectBoard(project.id) - assert.equal(rows.length, 2) + assert.equal(rows.length, 3) assert.ok(rows.every(row => row.assignee === null), 'the board carries a Bot name or null, never a member id') assert.equal(actionableFor(rows, 'owner-1').length, 0, 'the raw helper sees no owner without a resolver') const result = await rt.projectAssistant.digest(project.id, { at: AT }) const [one, two] = result.owners assert.match(one.text, /Rotate the deploy key/, 'the card of the primary owner\'s Bot is theirs') assert.doesNotMatch(one.text, /Pick the release date/, 'another owner\'s card never appears in this digest') + assert.doesNotMatch(one.text, /Still moving/, 'a card the owner\'s Bot is still working on waits on nobody') assert.match(two.text, /Pick the release date/, 'the card of the second owner\'s Bot is theirs') assert.doesNotMatch(two.text, /Rotate the deploy key/) } finally {