fix(schedule): close concurrent durability gaps
This commit is contained in:
@@ -22,7 +22,7 @@ Replay rejects unknown versions, extra fields, reused ids, and delete or dispatc
|
||||
|
||||
The generated [tool catalog](../../../docs/tool-catalog.md) owns the argument and output schemas for `schedule_create`, `schedule_list`, and `schedule_delete`. Their canonical values use camelCase record fields even though model input uses `after_seconds`.
|
||||
|
||||
`schedule_create` validates shape-only failures before persistence, then checkpoints, allocates a never-reused id, appends the create, and checkpoints again. `schedule_list` returns every active record in create order with `state: "scheduled" | "overdue"` and `deliveryMode: "session-local"`. `schedule_delete` rejects an empty or whitespace-padded id before persistence and appends only for an active id; an unknown or terminal id returns `{ id, deleted: false, code: "schedule_not_found" }` after its preflight.
|
||||
One Agent-scoped queue serializes each accepted management transaction and the live owner's due transaction from preflight through any post-append barrier. Direct callers therefore cannot interleave a fold with another Schedule mutation or observe a dispatch before its own barrier. `schedule_create` validates shape-only failures before entering that queue, then checkpoints, allocates a never-reused id, appends the create, and checkpoints again. `schedule_list` returns every active record in create order with `state: "scheduled" | "overdue"` and `deliveryMode: "session-local"`. `schedule_delete` rejects an empty or whitespace-padded id before entering the queue and appends only for an active id; an unknown or terminal id returns `{ id, deleted: false, code: "schedule_not_found" }` after its preflight.
|
||||
|
||||
Every successful management preflight also asks the live owner to recompute. This matters after a create or delete barrier returned `persistence_uncertain`: a later list or mutation can confirm the retained batch and immediately arm or retire the now-durable record without a private persistence-retry timer.
|
||||
|
||||
@@ -79,7 +79,7 @@ The reminder appends after existing history and preserves its reusable prefix. I
|
||||
## Known Limitations and Deferred Work
|
||||
|
||||
- **Session-local delivery only** — a reminder runs on time only while its original session is live; a cold session receives no external notification and processes an overdue record only after resume.
|
||||
- **Activity-driven persistence retry** — a rejected due preflight leaves the overdue record active but starts no private retry timer; the owner retries after later Agent activity reaches idle or a successful Schedule management preflight asks it to recompute.
|
||||
- **Activity-driven retry** — a rejected due preflight or contained framing/enqueue failure leaves the overdue record active but starts no private retry timer; the owner retries after later Agent activity reaches idle or a successful Schedule management preflight asks it to recompute.
|
||||
- **After-only protocol** — version 1 rejects `at`, `every_seconds`, `cron`, and `time_zone`; those rules require later protocol variants rather than hidden compatibility fields.
|
||||
- **Narrow crash duplicate window** — a crash after synchronous followup admission but before the dispatch checkpoint can repeat the reminder after recovery; the package does not claim model completion, user acknowledgement, or exactly-once external effects.
|
||||
- **Load-order boundary** — the plugin does not scan or adopt agents that were already live when it loaded.
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
|
||||
生成的[工具目录](../../../docs/tool-catalog.md)负责 `schedule_create`、`schedule_list` 和 `schedule_delete` 的参数与输出 schema。虽然模型输入使用 `after_seconds`,但其规范值中的记录字段使用 camelCase。
|
||||
|
||||
`schedule_create` 会在持久化前验证只依赖输入形状的失败,随后执行检查点、分配永不复用的 id、追加 create,再次执行检查点。`schedule_list` 按创建顺序返回所有活动记录,其中包含 `state: "scheduled" | "overdue"` 与 `deliveryMode: "session-local"`。`schedule_delete` 会在持久化前拒绝空 id 或前后带空白的 id,并只为活动 id 追加事件;未知或已终结的 id 会在 preflight(预检)后返回 `{ id, deleted: false, code: "schedule_not_found" }`。
|
||||
一条 Agent-scoped 队列会将每项已接纳的管理事务与 live owner 的到期事务从 preflight 到任何 post-append barrier 全程串行化。因此,直接调用方无法让一次 fold 与另一项 Schedule 变更交错,也无法在自身的 barrier 前观察到 dispatch。`schedule_create` 会在进入该队列前验证只依赖输入形状的失败,随后执行检查点、分配永不复用的 id、追加 create,再次执行检查点。`schedule_list` 按创建顺序返回所有活动记录,其中包含 `state: "scheduled" | "overdue"` 与 `deliveryMode: "session-local"`。`schedule_delete` 会在进入该队列前拒绝空 id 或前后带空白的 id,并只为活动 id 追加事件;未知或已终结的 id 会在 preflight(预检)后返回 `{ id, deleted: false, code: "schedule_not_found" }`。
|
||||
|
||||
每次成功的管理 preflight 还会要求 live owner 重新计算。这对 create 或 delete barrier 返回 `persistence_uncertain` 的情况很重要:后续 list 或 mutation 可以确认保留的 batch,并立即 arm 或退役此时已持久化的 record,而无需私有 persistence retry timer。
|
||||
|
||||
@@ -79,7 +79,7 @@ reminder_prompt_json: <JSON.stringify(prompt)>
|
||||
## 已知限制与暂缓事项
|
||||
|
||||
- **仅限会话本地交付**:提醒只有在原会话 live 时才能准时运行;cold 会话不会收到外部通知,只有恢复后才会处理 overdue 记录。
|
||||
- **活动驱动的持久化重试**:到期 preflight 被拒绝后,overdue 记录仍保持活动,但不会启动私有重试 timer;后续 agent 活动进入 idle,或成功的 Schedule 管理 preflight 要求 owner 重新计算后,owner 会重试。
|
||||
- **活动驱动的重试**:到期 preflight 被拒绝或 framing/入队失败被收容后,overdue 记录仍保持活动,但不会启动私有重试 timer;后续 agent 活动进入 idle,或成功的 Schedule 管理 preflight 要求 owner 重新计算后,owner 会重试。
|
||||
- **仅支持 after 协议**:版本 1 拒绝 `at`、`every_seconds`、`cron` 和 `time_zone`;这些规则需要后续协议变体,而不是隐藏的兼容字段。
|
||||
- **存在狭窄的崩溃重复窗口**:同步 `followup` 获得准入后、dispatch 检查点完成前发生崩溃,可能使提醒在恢复后重复;此包不承诺模型完成、用户确认或外部副作用恰好一次。
|
||||
- **加载顺序边界**:插件不会扫描或接管加载时已经 live 的 agent。
|
||||
|
||||
@@ -9,6 +9,7 @@ import { createUserMessage } from '@deepseek-ai/dsh-llm'
|
||||
import type { AfterScheduleRecord } from './types.ts'
|
||||
import { foldScheduleEvents, renderReminderFraming, ScheduleLogError } from './domain.ts'
|
||||
import { flushSchedulePersistence } from './persistence.ts'
|
||||
import { runScheduleTransaction } from './transaction.ts'
|
||||
|
||||
/** Largest delay that Node timers represent without clamping. */
|
||||
export const MAX_TIMER_DELAY_MS = 2_147_483_647
|
||||
@@ -102,7 +103,7 @@ export class ScheduleOwner {
|
||||
private async runRequested(): Promise<void> {
|
||||
while (this.requested && !this.stopping && !this.faulted) {
|
||||
this.requested = false
|
||||
await this.driveOnce()
|
||||
await runScheduleTransaction(this.agent, () => this.driveOnce())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
scheduleView,
|
||||
} from './domain.ts'
|
||||
import { flushSchedulePersistence } from './persistence.ts'
|
||||
import { runScheduleTransaction } from './transaction.ts'
|
||||
import type {
|
||||
AfterScheduleRecord,
|
||||
PersistenceUncertainError,
|
||||
@@ -255,31 +256,33 @@ export function registerScheduleTools(
|
||||
if (exec.agent !== agent) return internalError()
|
||||
const invalid = validateCreateArgs(args)
|
||||
if (invalid !== undefined) return invalid
|
||||
const uncertain = await preflight(rootCtx, agent, 'create')
|
||||
if (uncertain !== undefined) return uncertain
|
||||
notifyDurableChange()
|
||||
const folded = foldForTool(agent)
|
||||
if (isToolError(folded)) return folded
|
||||
const id = allocateScheduleId(folded)
|
||||
let record: AfterScheduleRecord
|
||||
try {
|
||||
record = createAfterScheduleRecord(id, args.prompt, args.after_seconds, Date.now())
|
||||
} catch (error: unknown) {
|
||||
return error instanceof ScheduleInputError ? inputError(error) : internalError()
|
||||
}
|
||||
try {
|
||||
agent.session.append('schedule/change', {
|
||||
version: 1,
|
||||
operation: 'create',
|
||||
schedule: record,
|
||||
})
|
||||
} catch {
|
||||
return internalError()
|
||||
}
|
||||
const barrier = await preflight(rootCtx, agent, 'create', id)
|
||||
if (barrier !== undefined) return barrier
|
||||
notifyDurableChange()
|
||||
return scheduleView(record, Date.now())
|
||||
return runScheduleTransaction(agent, async () => {
|
||||
const uncertain = await preflight(rootCtx, agent, 'create')
|
||||
if (uncertain !== undefined) return uncertain
|
||||
notifyDurableChange()
|
||||
const folded = foldForTool(agent)
|
||||
if (isToolError(folded)) return folded
|
||||
const id = allocateScheduleId(folded)
|
||||
let record: AfterScheduleRecord
|
||||
try {
|
||||
record = createAfterScheduleRecord(id, args.prompt, args.after_seconds, Date.now())
|
||||
} catch (error: unknown) {
|
||||
return error instanceof ScheduleInputError ? inputError(error) : internalError()
|
||||
}
|
||||
try {
|
||||
agent.session.append('schedule/change', {
|
||||
version: 1,
|
||||
operation: 'create',
|
||||
schedule: record,
|
||||
})
|
||||
} catch {
|
||||
return internalError()
|
||||
}
|
||||
const barrier = await preflight(rootCtx, agent, 'create', id)
|
||||
if (barrier !== undefined) return barrier
|
||||
notifyDurableChange()
|
||||
return scheduleView(record, Date.now())
|
||||
})
|
||||
},
|
||||
presentCall: args => present('Create reminder', 'other', args.prompt),
|
||||
})))
|
||||
@@ -291,13 +294,15 @@ export function registerScheduleTools(
|
||||
output: { schema: LIST_OUTPUT_SCHEMA, render: renderValue },
|
||||
async execute(_args, exec): Promise<ScheduleListValue> {
|
||||
if (exec.agent !== agent) return internalError()
|
||||
const uncertain = await preflight(rootCtx, agent, 'list')
|
||||
if (uncertain !== undefined) return uncertain
|
||||
notifyDurableChange()
|
||||
const folded = foldForTool(agent)
|
||||
if (isToolError(folded)) return folded
|
||||
const now = Date.now()
|
||||
return folded.active.map(record => scheduleView(record, now))
|
||||
return runScheduleTransaction(agent, async () => {
|
||||
const uncertain = await preflight(rootCtx, agent, 'list')
|
||||
if (uncertain !== undefined) return uncertain
|
||||
notifyDurableChange()
|
||||
const folded = foldForTool(agent)
|
||||
if (isToolError(folded)) return folded
|
||||
const now = Date.now()
|
||||
return folded.active.map(record => scheduleView(record, now))
|
||||
})
|
||||
},
|
||||
presentCall: () => present('List reminders', 'read'),
|
||||
})))
|
||||
@@ -315,23 +320,25 @@ export function registerScheduleTools(
|
||||
}
|
||||
const id = ScheduleId(args.id)
|
||||
if (exec.agent !== agent) return internalError()
|
||||
const uncertain = await preflight(rootCtx, agent, 'delete', id)
|
||||
if (uncertain !== undefined) return uncertain
|
||||
notifyDurableChange()
|
||||
const folded = foldForTool(agent)
|
||||
if (isToolError(folded)) return folded
|
||||
if (!folded.active.some(record => record.id === id)) {
|
||||
return { id, deleted: false, code: 'schedule_not_found' }
|
||||
}
|
||||
try {
|
||||
agent.session.append('schedule/change', { version: 1, operation: 'delete', id })
|
||||
} catch {
|
||||
return internalError()
|
||||
}
|
||||
const barrier = await preflight(rootCtx, agent, 'delete', id)
|
||||
if (barrier !== undefined) return barrier
|
||||
notifyDurableChange()
|
||||
return { id, deleted: true }
|
||||
return runScheduleTransaction(agent, async () => {
|
||||
const uncertain = await preflight(rootCtx, agent, 'delete', id)
|
||||
if (uncertain !== undefined) return uncertain
|
||||
notifyDurableChange()
|
||||
const folded = foldForTool(agent)
|
||||
if (isToolError(folded)) return folded
|
||||
if (!folded.active.some(record => record.id === id)) {
|
||||
return { id, deleted: false, code: 'schedule_not_found' }
|
||||
}
|
||||
try {
|
||||
agent.session.append('schedule/change', { version: 1, operation: 'delete', id })
|
||||
} catch {
|
||||
return internalError()
|
||||
}
|
||||
const barrier = await preflight(rootCtx, agent, 'delete', id)
|
||||
if (barrier !== undefined) return barrier
|
||||
notifyDurableChange()
|
||||
return { id, deleted: true }
|
||||
})
|
||||
},
|
||||
presentCall: args => present('Delete reminder', 'other', args.id),
|
||||
})))
|
||||
|
||||
23
packages/schedule/tool-schedule/src/transaction.ts
Normal file
23
packages/schedule/tool-schedule/src/transaction.ts
Normal file
@@ -0,0 +1,23 @@
|
||||
/** Agent-scoped serialization for Schedule reads and durable mutations. */
|
||||
|
||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
|
||||
const tails = new WeakMap<Agent, Promise<void>>()
|
||||
|
||||
/**
|
||||
* Run one complete Schedule transaction after its exact Agent's prior transaction.
|
||||
* @param agent - Exact Schedule owner and serialization key.
|
||||
* @param operation - Complete preflight, fold, mutation, and postflight operation.
|
||||
* @returns The operation result after exclusive execution.
|
||||
*/
|
||||
export async function runScheduleTransaction<T>(agent: Agent, operation: () => Promise<T>): Promise<T> {
|
||||
const prior = tails.get(agent) ?? Promise.resolve()
|
||||
const run = prior.then(operation)
|
||||
const tail = run.then(() => undefined, () => undefined)
|
||||
tails.set(agent, tail)
|
||||
try {
|
||||
return await run
|
||||
} finally {
|
||||
if (tails.get(agent) === tail) tails.delete(agent)
|
||||
}
|
||||
}
|
||||
@@ -16,7 +16,7 @@ const contexts: Context[] = []
|
||||
interface ToolHarness {
|
||||
readonly ctx: Context
|
||||
readonly agent: Agent
|
||||
readonly flushes: { count: number; outcomes: Array<'resolve' | 'reject'> }
|
||||
readonly flushes: { count: number; outcomes: Array<'resolve' | 'reject' | Promise<'resolve' | 'reject'>> }
|
||||
readonly changes: { count: number }
|
||||
readonly disposeTools: () => void
|
||||
}
|
||||
@@ -50,11 +50,12 @@ async function harness(withPersistence = true): Promise<ToolHarness> {
|
||||
await ctx.plugin(ToolRegistry)
|
||||
const agent = stubAgent(ctx, `schedule-tools-${Math.random()}`)
|
||||
ctx.agents.register(agent)
|
||||
const flushes = { count: 0, outcomes: [] as Array<'resolve' | 'reject'> }
|
||||
const flushes = { count: 0, outcomes: [] as Array<'resolve' | 'reject' | Promise<'resolve' | 'reject'>> }
|
||||
if (withPersistence) {
|
||||
ctx.on('session/flush', async () => {
|
||||
flushes.count += 1
|
||||
if (flushes.outcomes.shift() === 'reject') return Promise.reject(new Error('disk unavailable'))
|
||||
const outcome = await (flushes.outcomes.shift() ?? 'resolve')
|
||||
if (outcome === 'reject') return Promise.reject(new Error('disk unavailable'))
|
||||
return true as const
|
||||
})
|
||||
}
|
||||
@@ -278,6 +279,31 @@ describe('Schedule persistence failure boundaries', () => {
|
||||
expect(test.changes.count).toBe(2)
|
||||
})
|
||||
|
||||
it('serializes concurrent management transactions across both persistence barriers', async () => {
|
||||
const test = await harness()
|
||||
let releaseCreatePreflight: (() => void) | undefined
|
||||
const createPreflight = new Promise<'resolve'>((resolve) => {
|
||||
releaseCreatePreflight = () => { resolve('resolve') }
|
||||
})
|
||||
test.flushes.outcomes.push(createPreflight, 'reject', 'resolve')
|
||||
|
||||
const creating = execute(test, 'schedule_create', { prompt: 'persist me', after_seconds: 10 })
|
||||
await vi.waitFor(() => { expect(test.flushes.count).toBe(1) })
|
||||
const listing = execute(test, 'schedule_list', {})
|
||||
await Promise.resolve()
|
||||
expect(test.flushes.count).toBe(1)
|
||||
|
||||
if (releaseCreatePreflight === undefined) throw new Error('missing create preflight release')
|
||||
releaseCreatePreflight()
|
||||
expect(value(await creating)).toMatchObject({
|
||||
code: 'persistence_uncertain', operation: 'create', id: 'schedule-1',
|
||||
})
|
||||
expect(value(await listing)).toEqual([
|
||||
expect.objectContaining({ id: 'schedule-1', prompt: 'persist me' }),
|
||||
])
|
||||
expect(test.flushes.count).toBe(3)
|
||||
})
|
||||
|
||||
it('returns uncertainty before create or delete reads when their preflight rejects', async () => {
|
||||
const createTest = await harness()
|
||||
createTest.flushes.outcomes.push('reject')
|
||||
|
||||
Reference in New Issue
Block a user