Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 48 additions & 17 deletions core/src/host/project-assistant.js
Original file line number Diff line number Diff line change
Expand Up @@ -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})`))),
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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?.()
Expand All @@ -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 }
Expand Down
105 changes: 101 additions & 4 deletions core/test/project-assistant.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading