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 {