Skip to content

Commit 629fb2e

Browse files
committed
fix(parts): keep a task's anchor when a handoff moves it (#1245)
A part handoff moved each task by creating it again on the fresh part: DSH's create anchors an `every` task to now + interval and gives it a fresh id and an empty delivery history, so the run it was already due to make slipped by up to a full interval and the reset history hid it. Rewrite the stored row in place inside `service.serialize` instead: id, record and history stay, only `sessionId` changes, and a failed write leaves the task on the earlier part. The row move reaches DSH's storage domain directly — `getDomain()`, `serialize()` and the `tasks` row shape are private in @deepseek-ai/dsh-schedule@0.2.0-rc.2 — so the call site records that and the failure shape for the next upgrade. The driver needs no re-arm: `drive()` re-scans storage on every tick and the timer is the global `scheduledAt` minimum, which the move leaves unchanged; if DSH ever turns it event-driven, a moved task needs an explicit `emitChanged()`. Tests: a handoff keeps the anchor, id and delivery history, a failed write changes nothing, and a disabled or exhausted row stays on the earlier part with its status untouched. part-archive's schedule fake models `getDomain()`/`serialize()`/`put`, so #896's archive-waits and failed-move-stops-activity properties still run.
1 parent de82852 commit 629fb2e

7 files changed

Lines changed: 420 additions & 85 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
A part handoff keeps a repeating task exactly where it was: the same id, the same delivery history and the same instant it was already due, instead of re-anchoring the next run to the handoff and hiding the move in a reset history. ([#1245](https://github.com/ThinkFlowLab/FlyBot/issues/1245))

‎core/src/host/schedules.js‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -519,26 +519,43 @@ export function install(rt) {
519519
},
520520
}
521521

522-
// A fresh part takes over the earlier part's active tasks: each is created again on
523-
// the fresh Session before the old one goes, so a failure leaves it where it was.
522+
// A fresh part takes over the earlier part's active tasks. A handoff is not an edit, so
523+
// each row is rewritten in place with the fresh Session: the id, the delivery history and
524+
// the instant it was already due all stay put, and a failed write leaves it where it was.
525+
// Creating the task again would re-anchor it (DSH's create sets now + interval, #1245).
526+
//
527+
// This reaches DSH's storage domain directly: `getDomain()` and `serialize()`, and the
528+
// `tasks` row shape, are PRIVATE API of `@deepseek-ai/dsh-schedule@0.2.0-rc.2` (declared
529+
// in `lib/types/index.d.ts:119` and `:132`), and this is the first call into them. Re-check
530+
// them on every DSH upgrade. If a bump renames or reshapes either, the call throws,
531+
// `moveSchedules` catches it and warns `ds-bot: tasks of %s not moved`, the tasks stay on
532+
// the earlier part, and the archive then ends in the schedule-only `stopActivity` (#896's
533+
// path) — so they silently stop firing rather than crashing. The tests fake the same
534+
// private shape, so they will not catch such a rename either.
535+
//
536+
// No re-arm is needed after a move at this DSH version: the driver's `drive()` re-scans
537+
// storage on every tick and the timer is the global minimum over `scheduledAt`, so the
538+
// moved row's due instant is already covered. If DSH ever turns the driver event-driven,
539+
// a moved task needs an explicit `emitChanged()`.
524540
async function moveSchedules(fromId, toId) {
525541
if (service === null) return
526-
let records
527542
try {
528-
records = await service.list({ sessionId: fromId })
543+
await service.serialize(async () => {
544+
const tasks = (await service.getDomain()).table('tasks')
545+
for (const [id, task] of tasks.entries()) {
546+
if (task.sessionId !== fromId || task.status !== 'active') continue
547+
// A one-shot whose time has passed can never run on the fresh part.
548+
const oneShot = task.record.kind === 'at' || task.record.kind === 'after'
549+
if (oneShot && Date.parse(task.record.scheduledAt) <= Date.now()) continue
550+
try {
551+
await tasks.put(id, { ...task, sessionId: toId })
552+
} catch (error) {
553+
rt.warn('ds-bot: task %s stayed with the earlier part: %s', task.record.title, error?.message ?? error)
554+
}
555+
}
556+
})
529557
} catch (error) {
530558
rt.warn('ds-bot: tasks of %s not moved: %s', fromId, error?.message ?? error)
531-
return
532-
}
533-
for (const record of records) {
534-
const request = requestOf(record)
535-
if (request === null || (request.at !== undefined && Date.parse(request.at) <= Date.now())) continue
536-
try {
537-
await service.create(toId, request)
538-
await service.delete({ sessionId: fromId, id: record.id })
539-
} catch (error) {
540-
rt.warn('ds-bot: task %s stayed with the earlier part: %s', record.title, error?.message ?? error)
541-
}
542559
}
543560
}
544561

‎core/test/part-archive.test.js‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,31 +26,53 @@ const TEAM = {
2626

2727
// DSH's schedule service, as the host sees it: tasks are stored per Session, and a
2828
// Session with an active task refuses to be archived unless its activity is stopped.
29-
function scheduleService({ listDelay = 0, failCreate = false } = {}) {
29+
function scheduleService({ delay = 0, failPut = false } = {}) {
3030
const tasks = new Map()
3131
let next = 1
32-
const armedOn = (sessionId) => [...tasks.entries()].filter(([, task]) => task.sessionId === sessionId).map(([id]) => id)
32+
let chain = Promise.resolve()
33+
const armedOn = (sessionId) => [...tasks.entries()].filter(([, task]) => task.sessionId === sessionId && task.status === 'active').map(([id]) => id)
3334
return {
3435
tasks,
35-
listDelay,
36-
failCreate,
36+
delay,
37+
failPut,
3738
armedOn,
3839
stop(sessionId) {
3940
for (const id of armedOn(sessionId)) tasks.delete(id)
4041
},
4142
arm(sessionId, title = 'nightly check') {
4243
const id = `schedule-${next++}`
43-
tasks.set(id, { sessionId, record: { id, title, prompt: 'Wake up.', kind: 'at', scheduledAt: '2030-01-01T01:00:00.000Z' } })
44+
tasks.set(id, { sessionId, status: 'active', record: { id, title, prompt: 'Wake up.', kind: 'at', scheduledAt: '2030-01-01T01:00:00.000Z' }, deliveryHistory: { records: [] } })
4445
return id
4546
},
4647
async list({ sessionId }) {
47-
if (listDelay > 0) await new Promise(resolve => setTimeout(resolve, listDelay))
48+
if (delay > 0) await new Promise(resolve => setTimeout(resolve, delay))
4849
return [...tasks.values()].filter(task => task.sessionId === sessionId).map(task => task.record)
4950
},
51+
// A part move rewrites the stored row (DSH's `getDomain`, inside `serialize`).
52+
async getDomain() {
53+
return {
54+
table(name) {
55+
assert.equal(name, 'tasks')
56+
return {
57+
entries: () => tasks.entries(),
58+
get: id => tasks.get(id),
59+
put(id, task) {
60+
if (failPut) throw new Error('write failed')
61+
tasks.set(id, task)
62+
},
63+
}
64+
},
65+
}
66+
},
67+
serialize(work) {
68+
// Queued behind whatever DSH is already running, like the real service.
69+
const pending = chain.then(() => (delay > 0 ? new Promise(resolve => setTimeout(resolve, delay)) : undefined)).then(work)
70+
chain = pending.catch(() => undefined)
71+
return pending
72+
},
5073
async create(sessionId, request) {
51-
if (failCreate) throw new Error('create failed')
5274
const id = `schedule-${next++}`
53-
tasks.set(id, { sessionId, record: { id, title: request.title, prompt: request.prompt, kind: 'at', scheduledAt: request.at } })
75+
tasks.set(id, { sessionId, status: 'active', record: { id, title: request.title, prompt: request.prompt, kind: 'at', scheduledAt: request.at }, deliveryHistory: { records: [] } })
5476
return tasks.get(id).record
5577
},
5678
async delete({ sessionId, id }) {
@@ -179,7 +201,7 @@ const drained = async () => { for (let i = 0; i < 25; i += 1) await settle() }
179201
const archivedEntry = (archived, sessionId) => archived.find(entry => (Array.isArray(entry) ? entry[0] : entry) === sessionId)
180202

181203
test('#896: a part whose task moved is archived once the move finished', async (t) => {
182-
const schedule = scheduleService({ listDelay: 40 })
204+
const schedule = scheduleService({ delay: 40 })
183205
const h = await start({ schedule })
184206
t.after(h.stop)
185207
schedule.arm('b')
@@ -195,7 +217,7 @@ test('#896: a part whose task moved is archived once the move finished', async (
195217
})
196218

197219
test('#896: a failed task move does not leave an armed earlier part active', async (t) => {
198-
const schedule = scheduleService({ failCreate: true })
220+
const schedule = scheduleService({ failPut: true })
199221
const h = await start({ schedule })
200222
t.after(h.stop)
201223
schedule.arm('b')

‎core/test/schedules.test.js‎

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
33
import { tmpdir } from 'node:os'
44
import { join } from 'node:path'
55
import { test } from 'node:test'
6-
import { apply } from '../index.js'
6+
import { createRuntime, provideFlyBot } from '../index.js'
77
import { fakeContext } from './fake-ctx.js'
88
import { reminderText } from '../src/host/schedules.js'
99

@@ -22,9 +22,11 @@ const TEAM = {
2222
}
2323

2424
// The record fields DSH stores for each create selector (see dsh-schedule's README).
25+
// `every_seconds` is the one DSH anchors to the create instant (now + interval), so a
26+
// move that went through create would re-anchor it; the others keep a fixed instant.
2527
const KINDS = {
2628
at: at => ({ kind: 'at', scheduledAt: at }),
27-
every_seconds: seconds => ({ kind: 'every', everySeconds: seconds, scheduledAt: '2030-01-01T01:00:00.000Z' }),
29+
every_seconds: seconds => ({ kind: 'every', everySeconds: seconds, scheduledAt: new Date(Date.now() + seconds * 1000).toISOString() }),
2830
daily: ({ time, time_zone }) => ({ kind: 'daily', time, timeZone: time_zone, scheduledAt: '2030-01-01T01:00:00.000Z' }),
2931
weekly: ({ time, time_zone, weekdays }) => ({ kind: 'weekly', time, timeZone: time_zone, weekdays, scheduledAt: '2030-01-01T01:00:00.000Z' }),
3032
cron: ({ expression, time_zone }) => ({ kind: 'cron', expression, timeZone: time_zone, scheduledAt: '2030-01-01T01:00:00.000Z' }),
@@ -36,21 +38,45 @@ function scheduleService() {
3638
const tasks = new Map()
3739
const updates = []
3840
let next = 1
39-
return {
41+
// DSH runs management and delivery through one queue (`serialize`); the fake keeps one too.
42+
let chain = Promise.resolve()
43+
const service = {
4044
tasks,
4145
updates,
4246
failCreate: false,
4347
failDelete: false,
48+
failPut: false,
4449
async create(sessionId, request) {
4550
if (this.failCreate) throw new Error('create failed')
4651
const id = `schedule-${next++}`
4752
const kind = Object.keys(request).find(key => !['title', 'prompt'].includes(key))
4853
const record = { id, title: request.title, prompt: request.prompt, timing: request[kind], ...KINDS[kind](request[kind]) }
49-
tasks.set(id, { sessionId, record, status: 'active' })
54+
tasks.set(id, { sessionId, record, status: 'active', deliveryHistory: { records: [], earlierRecordsUnavailable: false } })
5055
return record
5156
},
5257
async list({ sessionId }) { return [...tasks.values()].filter(task => task.sessionId === sessionId && task.status === 'active').map(task => task.record) },
5358
async catalog() { return [...tasks.values()].map(task => ({ ...task.record, sessionId: task.sessionId, status: task.status })) },
59+
// DSH's storage domain, as `getDomain()` exposes it: the row itself is what a move rewrites.
60+
async getDomain() {
61+
return {
62+
table(name) {
63+
assert.equal(name, 'tasks')
64+
return {
65+
entries: () => tasks.entries(),
66+
get: id => tasks.get(id),
67+
put(id, task) {
68+
if (service.failPut) throw new Error('write failed')
69+
tasks.set(id, task)
70+
},
71+
}
72+
},
73+
}
74+
},
75+
serialize(work) {
76+
const pending = chain.then(work)
77+
chain = pending.catch(() => undefined)
78+
return pending
79+
},
5480
async update(request) {
5581
updates.push(request)
5682
const task = tasks.get(request.id)
@@ -68,6 +94,7 @@ function scheduleService() {
6894
return { id, deleted: true }
6995
},
7096
}
97+
return service
7198
}
7299

73100
function setup(services = {}, team = TEAM) {
@@ -76,9 +103,10 @@ function setup(services = {}, team = TEAM) {
76103
const ctx = fakeContext({ services })
77104
const register = ctx.connection.fetch.register
78105
ctx.connection.fetch.register = (route) => { ctx.route = route; return register(route) }
79-
apply(ctx, { home })
106+
const rt = createRuntime(ctx, { home })
107+
provideFlyBot(ctx, rt.flybot)
80108
const call = async (endpoint, payload) => (await ctx.route.fetch(new Request('http://localhost/api/bot', { method: 'POST', body: JSON.stringify({ endpoint, payload }) }))).json()
81-
return { ctx, home, call }
109+
return { ctx, home, call, rt }
82110
}
83111

84112
test('without the schedule service the page says so and creating is refused', async (t) => {
@@ -212,6 +240,79 @@ test('editing a task onto another chat creates the new one before deleting the o
212240
assert.deepEqual(again.value.tasks.map(task => [task.prompt, task.status]), [['Again', 'active']])
213241
})
214242

243+
// #1245: a handoff is not an edit. The earlier part's task moves to the fresh part with its
244+
// id, its delivery history and the instant it was already due — not re-anchored to the
245+
// handoff, which would slip its next run by up to a full interval and hide it.
246+
test('#1245: a handoff moves an “every” task without re-anchoring it', async (t) => {
247+
const schedule = scheduleService()
248+
const { ctx, home, rt } = setup({ schedule })
249+
t.after(async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) })
250+
// A handoff happens well after the task was created, so a create-based move would visibly
251+
// re-anchor it: the fake's create samples this clock (`now + every_seconds`).
252+
let clock = Date.parse('2030-01-01T00:00:00.000Z')
253+
t.mock.method(Date, 'now', () => clock)
254+
255+
const record = await schedule.create('m1', { title: 'Watch', prompt: 'Check the board', every_seconds: 7200 })
256+
const row = schedule.tasks.get(record.id)
257+
row.deliveryHistory = { records: [{ scheduledAt: '2030-01-01T01:00:00.000Z', deliveredAt: '2030-01-01T01:00:01.000Z', messageId: 'msg-old' }], earlierRecordsUnavailable: false }
258+
const anchor = row.record.scheduledAt
259+
const history = structuredClone(row.deliveryHistory)
260+
261+
clock += 45 * 60_000
262+
await rt.moveSchedules('m1', 'm1-fresh')
263+
264+
const moved = [...schedule.tasks.values()].filter(task => task.sessionId === 'm1-fresh')
265+
assert.equal(moved.length, 1, 'the task moved to the fresh part')
266+
assert.equal(moved[0].record.scheduledAt, anchor, 'the instant it was already due is kept')
267+
assert.equal(moved[0].record.id, record.id, 'the task keeps its id')
268+
assert.deepEqual(moved[0].deliveryHistory, history, 'the delivery history is kept')
269+
assert.equal(moved[0].record.everySeconds, 7200)
270+
assert.equal(schedule.tasks.size, 1, 'the row was rewritten, not copied')
271+
272+
// A failed write leaves the task where it was, with nothing half-deleted (#901).
273+
schedule.failPut = true
274+
await rt.moveSchedules('m1-fresh', 'm1-newer')
275+
schedule.failPut = false
276+
const [still] = [...schedule.tasks.values()]
277+
assert.equal(still.sessionId, 'm1-fresh', 'a failed move changes nothing')
278+
assert.equal(still.record.id, record.id)
279+
assert.equal(still.record.scheduledAt, anchor)
280+
281+
// A one-shot whose time has already passed cannot run on the fresh part, so it stays.
282+
const once = await schedule.create('m1-fresh', { title: 'Once', prompt: 'Ping', at: '2030-01-01T00:00:00.000Z' })
283+
schedule.tasks.get(once.id).record = { ...schedule.tasks.get(once.id).record, scheduledAt: new Date(Date.now() - 1000).toISOString() }
284+
await rt.moveSchedules('m1-fresh', 'm1-newer')
285+
const byId = id => [...schedule.tasks.values()].find(task => task.record.id === id)
286+
assert.equal(byId(once.id).sessionId, 'm1-fresh', 'a passed one-shot stays where it is')
287+
assert.equal(byId(record.id).sessionId, 'm1-newer', 'the recurring task still moved')
288+
})
289+
290+
// The move only takes active rows: a disabled task and an exhausted one-shot have no next run
291+
// to follow the handoff, so they stay on the earlier part with their status untouched.
292+
test('#1245: a handoff leaves a disabled and an exhausted row on the earlier part', async (t) => {
293+
const schedule = scheduleService()
294+
const { ctx, home, rt } = setup({ schedule })
295+
t.after(async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) })
296+
t.mock.method(Date, 'now', () => Date.parse('2030-01-01T00:00:00.000Z'))
297+
298+
const disabled = await schedule.create('m1', { title: 'Disabled watch', prompt: 'Switched off', every_seconds: 7200 })
299+
const exhausted = await schedule.create('m1', { title: 'Exhausted ping', prompt: 'Ran once', at: '2030-01-01T00:10:00.000Z' })
300+
// DSH's only ended state is `inactive`: a disabled task and a finished one-shot both keep it.
301+
schedule.tasks.get(disabled.id).status = 'inactive'
302+
schedule.tasks.get(exhausted.id).status = 'inactive'
303+
const active = await schedule.create('m1', { title: 'Live watch', prompt: 'Moves', every_seconds: 3600 })
304+
305+
await rt.moveSchedules('m1', 'm1-fresh')
306+
307+
const row = id => [...schedule.tasks.values()].find(task => task.record.id === id)
308+
assert.equal(row(disabled.id).sessionId, 'm1', 'a disabled task stays on the earlier part')
309+
assert.equal(row(disabled.id).status, 'inactive', 'its status is untouched')
310+
assert.equal(row(exhausted.id).sessionId, 'm1', 'an exhausted task stays on the earlier part')
311+
assert.equal(row(exhausted.id).status, 'inactive', 'its status is untouched')
312+
assert.equal(row(active.id).sessionId, 'm1-fresh', 'the active task still moves')
313+
assert.deepEqual([...schedule.tasks.values()].filter(task => task.sessionId === 'm1-fresh').map(task => task.record.id), [active.id], 'the fresh part gets neither inactive row')
314+
})
315+
215316
test('pause keeps the task in FlyBot and removes it from DSH; resume brings it back', async (t) => {
216317
const schedule = scheduleService()
217318
const { ctx, home, call } = setup({ schedule })

0 commit comments

Comments
 (0)