feat: proactive plugin#25
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new @apeira/plugin-proactive package, which enables proactive agent behavior using a Default Mode Network (DMN) state machine, a task scheduler, and a todo list. It also updates the core package to support sending unsolicited messages via a send callback in session options. The review feedback highlights critical issues in the tick scheduling logic, specifically the premature pruning of one-time tasks before they are sent to the agent, and the lack of await on the asynchronous state.send call, which can cause race conditions and unhandled rejections. Additionally, the sleep tool's duration parameter is currently ignored in favor of a hardcoded resting interval, which should be resolved.
| state.timer = setTimeout(() => { | ||
| state.scheduler.prune() | ||
|
|
||
| if (state.dmn.state === 'paused') | ||
| return scheduleTick(state) | ||
|
|
||
| if (shouldSkipTick(state.dmn)) | ||
| return scheduleTick(state) | ||
|
|
||
| state.dmn.lastTickAt = Date.now() | ||
|
|
||
| const content = buildTickContent(state) | ||
|
|
||
| // Acknowledge interval tasks so they don't repeat on every tick | ||
| for (const task of state.scheduler.due()) { | ||
| if (task.type === 'interval') | ||
| task.lastTriggeredAt = Date.now() | ||
| } | ||
|
|
||
| try { | ||
| state.send?.({ content, role: 'user', type: 'message' }) | ||
| } | ||
| catch { | ||
| if (state.timer != null) | ||
| clearTimeout(state.timer) | ||
| return | ||
| } | ||
|
|
||
| scheduleTick(state) | ||
| }, interval) |
There was a problem hiding this comment.
This block contains two critical issues:
- Premature Pruning of
onceTasks: Callingstate.scheduler.prune()at the start of the tick deletes all dueoncetasks beforebuildTickContentis called. As a result,state.scheduler.due()returns an empty list for these tasks, and they are never included in the tick message sent to the agent. - Unawaited Asynchronous
state.send:state.send(which wrapsruntime.send) is asynchronous. Calling it withoutawaitmeans the next tick is scheduled immediately, leading to concurrent ticks/turns being sent to the LLM (especially in the 3-secondworkingstate), which causes race conditions and API errors. It also prevents thetry/catchblock from catching asynchronous rejections, causing unhandled promise rejections and failing to clean up the timer/session.
Additionally, we should persist the updated scheduler and DMN state to storage immediately after updating them to prevent duplicate task execution on server restarts.
state.timer = setTimeout(async () => {
if (state.dmn.state === 'paused')
return scheduleTick(state)
if (shouldSkipTick(state.dmn))
return scheduleTick(state)
state.dmn.lastTickAt = Date.now()
const content = buildTickContent(state)
// Acknowledge interval tasks and remove completed once tasks
for (const task of state.scheduler.due()) {
if (task.type === 'interval') {
task.lastTriggeredAt = Date.now()
} else if (task.type === 'once') {
state.scheduler.remove(task.id)
}
}
// Persist the updated scheduler and DMN state
await saveState(state)
try {
await state.send?.({ content, role: 'user', type: 'message' })
}
catch {
if (state.timer != null)
clearTimeout(state.timer)
sessions.delete(state.sessionId)
return
}
scheduleTick(state)
}, interval)| const sleepTool = tool({ | ||
| description: 'Enter a resting state. Call this when there is nothing useful to do after a tick. This conserves tokens and respects the user\'s attention.', | ||
| execute: (input: unknown) => { | ||
| const args = z.parse(sleepSchema, input) | ||
| callbacks.onSleep() | ||
| return `Sleeping for ${args.duration}. Next tick will arrive after that. Reason: ${args.reason ?? 'idle'}` | ||
| }, | ||
| name: 'sleep', | ||
| parameters: sleepSchema, | ||
| }) |
There was a problem hiding this comment.
The sleep tool defines a duration parameter in its schema (e.g., "5m", "30s", "1h"), but the implementation completely ignores this value and always transitions the agent to the fixed 'resting' state (which has a hardcoded 5-minute interval). This is misleading to the model.
Consider either:
- Parsing the
durationstring (e.g., using a simple regex or utility to convert"5m"to milliseconds) and dynamically scheduling the next tick interval. - Removing the
durationparameter from the schema and updating the tool description to state that the sleep duration is fixed.
No description provided.