diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index 9407659e..edf3cd43 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -569,7 +569,7 @@ export function install(rt) { if (task.status === 'accepted') { if (countsAsDelivery(task)) row.deliveries += 1 const lease = leaseOf(task) - if (lease !== undefined && Number.isFinite(lease.takenAt)) row.workerSeconds += Math.max(0, Math.round((task.updatedAt - lease.takenAt) / 1000)) + if (lease !== undefined && Number.isFinite(lease.takenAt) && Number.isFinite(task.updatedAt)) row.workerSeconds += Math.max(0, Math.round((task.updatedAt - lease.takenAt) / 1000)) } if (run !== '') { const runRow = bucket(runs, run) diff --git a/core/test/task-counters-updated-at.test.js b/core/test/task-counters-updated-at.test.js new file mode 100644 index 00000000..2817f5b5 --- /dev/null +++ b/core/test/task-counters-updated-at.test.js @@ -0,0 +1,90 @@ +// `rt.taskCounters` adds a card's own `updatedAt` into `workerSeconds`. A card may be stored by +// an older version or edited by hand, so the field can be missing or non-numeric while the lease +// is valid. The arithmetic must guard that operand too: `NaN` would become `null` through JSON +// for every reader of the counter. The numeric case still measures the lease window. +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createRuntime } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +const TAKEN_AT = 1_700_000_000_000 + +const acceptedCard = (updatedAt) => ({ + id: 'task-1', + title: 'Count the delivery', + status: 'accepted', + taskClass: 'work', + ownerBotId: 'd1', + createdAt: TAKEN_AT, + ...(updatedAt === undefined ? {} : { updatedAt }), + lease: { holder: 'd1', takenAt: TAKEN_AT, expiresAt: TAKEN_AT + 1_800_000 }, +}) + +const seed = (task) => ({ + version: 2, + revision: 1, + mainBotIds: ['a'], + members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }, + bots: { + a: { id: 'a', name: 'Chief', role: 'Lead', sessionId: 'a', createdAt: 1, ownerId: 'owner-1' }, + d1: { id: 'd1', name: 'Dev1', role: 'Dev', sessionId: 'd1', createdAt: 2, ownerId: 'owner-1', createdBy: 'a' }, + }, + rooms: {}, + grants: {}, + agents: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, + prefs: {}, + tasks: { [task.id]: task }, +}) + +async function boot(t, task) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-counters-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(seed(task))) + const ctx = fakeContext() + const agents = new Map() + const agentOf = (sessionId) => { + if (!agents.has(sessionId)) { + const inbox = { nextStep: [], nextTurn: [], remove() { return true } } + agents.set(sessionId, { + session: { id: sessionId }, id: sessionId, calls: [], status: 'idle', inbox, + followup() {}, steer() {}, inject() {}, cancel() {}, async whenIdle() {}, + }) + } + return agents.get(sessionId) + } + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { create: async () => ({ sessionId: 'made-1' }), resolveAgent: async id => ({ agent: agentOf(id) }), selectModel: async () => {}, page: async () => ({ records: [], hasMore: false }) }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = createRuntime(ctx, { home }) + await rt.load() + t.after(async () => { + try { await ctx.dispose() } catch {} + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + return rt +} + +test('a non-numeric or missing updatedAt keeps workerSeconds finite (#2588)', async (t) => { + for (const updatedAt of ['not-a-number', undefined, null]) { + const rt = await boot(t, acceptedCard(updatedAt)) + const counts = rt.taskCounters() + const workerSeconds = counts.roles.Dev.workerSeconds + assert.equal(Number.isFinite(workerSeconds), true, `updatedAt=${String(updatedAt)} gave workerSeconds=${workerSeconds}`) + const throughJson = JSON.parse(JSON.stringify(counts)).roles.Dev.workerSeconds + assert.notEqual(throughJson, null, `updatedAt=${String(updatedAt)} serialised to null`) + } +}) + +test('a numeric updatedAt still measures the lease window (#2588)', async (t) => { + const rt = await boot(t, acceptedCard(TAKEN_AT + 90_000)) + assert.equal(rt.taskCounters().roles.Dev.workerSeconds, 90) +}) diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index 9407659e..edf3cd43 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -569,7 +569,7 @@ export function install(rt) { if (task.status === 'accepted') { if (countsAsDelivery(task)) row.deliveries += 1 const lease = leaseOf(task) - if (lease !== undefined && Number.isFinite(lease.takenAt)) row.workerSeconds += Math.max(0, Math.round((task.updatedAt - lease.takenAt) / 1000)) + if (lease !== undefined && Number.isFinite(lease.takenAt) && Number.isFinite(task.updatedAt)) row.workerSeconds += Math.max(0, Math.round((task.updatedAt - lease.takenAt) / 1000)) } if (run !== '') { const runRow = bucket(runs, run) diff --git a/patches/ds-bot/files/test/task-counters-updated-at.test.js b/patches/ds-bot/files/test/task-counters-updated-at.test.js new file mode 100644 index 00000000..2817f5b5 --- /dev/null +++ b/patches/ds-bot/files/test/task-counters-updated-at.test.js @@ -0,0 +1,90 @@ +// `rt.taskCounters` adds a card's own `updatedAt` into `workerSeconds`. A card may be stored by +// an older version or edited by hand, so the field can be missing or non-numeric while the lease +// is valid. The arithmetic must guard that operand too: `NaN` would become `null` through JSON +// for every reader of the counter. The numeric case still measures the lease window. +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createRuntime } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +const TAKEN_AT = 1_700_000_000_000 + +const acceptedCard = (updatedAt) => ({ + id: 'task-1', + title: 'Count the delivery', + status: 'accepted', + taskClass: 'work', + ownerBotId: 'd1', + createdAt: TAKEN_AT, + ...(updatedAt === undefined ? {} : { updatedAt }), + lease: { holder: 'd1', takenAt: TAKEN_AT, expiresAt: TAKEN_AT + 1_800_000 }, +}) + +const seed = (task) => ({ + version: 2, + revision: 1, + mainBotIds: ['a'], + members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }, + bots: { + a: { id: 'a', name: 'Chief', role: 'Lead', sessionId: 'a', createdAt: 1, ownerId: 'owner-1' }, + d1: { id: 'd1', name: 'Dev1', role: 'Dev', sessionId: 'd1', createdAt: 2, ownerId: 'owner-1', createdBy: 'a' }, + }, + rooms: {}, + grants: {}, + agents: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, + prefs: {}, + tasks: { [task.id]: task }, +}) + +async function boot(t, task) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-counters-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(seed(task))) + const ctx = fakeContext() + const agents = new Map() + const agentOf = (sessionId) => { + if (!agents.has(sessionId)) { + const inbox = { nextStep: [], nextTurn: [], remove() { return true } } + agents.set(sessionId, { + session: { id: sessionId }, id: sessionId, calls: [], status: 'idle', inbox, + followup() {}, steer() {}, inject() {}, cancel() {}, async whenIdle() {}, + }) + } + return agents.get(sessionId) + } + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { create: async () => ({ sessionId: 'made-1' }), resolveAgent: async id => ({ agent: agentOf(id) }), selectModel: async () => {}, page: async () => ({ records: [], hasMore: false }) }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = createRuntime(ctx, { home }) + await rt.load() + t.after(async () => { + try { await ctx.dispose() } catch {} + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + return rt +} + +test('a non-numeric or missing updatedAt keeps workerSeconds finite (#2588)', async (t) => { + for (const updatedAt of ['not-a-number', undefined, null]) { + const rt = await boot(t, acceptedCard(updatedAt)) + const counts = rt.taskCounters() + const workerSeconds = counts.roles.Dev.workerSeconds + assert.equal(Number.isFinite(workerSeconds), true, `updatedAt=${String(updatedAt)} gave workerSeconds=${workerSeconds}`) + const throughJson = JSON.parse(JSON.stringify(counts)).roles.Dev.workerSeconds + assert.notEqual(throughJson, null, `updatedAt=${String(updatedAt)} serialised to null`) + } +}) + +test('a numeric updatedAt still measures the lease window (#2588)', async (t) => { + const rt = await boot(t, acceptedCard(TAKEN_AT + 90_000)) + assert.equal(rt.taskCounters().roles.Dev.workerSeconds, 90) +})