Add stale agent cleanup helper - #8
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a non-exported method Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as Plugin
participant KV as kvstore
participant Posts as PostsService
participant Reactions as ReactionsService
participant Pub as Publisher
Plugin->>KV: ListActiveAgents()
KV-->>Plugin: []*AgentRecord
loop each agent
Plugin->>Plugin: compute age (now - CreatedAt)
alt age > maxAge and status RUNNING
Plugin->>KV: SaveAgent(Status=STOPPED, UpdatedAt)
KV-->>Plugin: save result
Plugin->>Posts: CreatePost(thread: agent trigger, message: stopped)
Posts-->>Plugin: post result
Plugin->>Reactions: RemoveReaction(hourglass), AddReaction(no_entry_sign)
Reactions-->>Plugin: reactions updated
Plugin->>Pub: PublishAgentStatusChange(agent)
Pub-->>Plugin: published
else status is terminal
Plugin-->>Plugin: skip polling this agent
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: unknown linters: 'modernize', run 'golangci-lint help linters' to see the list of supported linters Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@server/poller.go`:
- Around line 349-364: The method cleanupStaleAgents on type Plugin is unused —
either call it from your poller loop (e.g., inside the Plugin poll/run function
that performs periodic work) or delete it; if you choose to call it, invoke
p.cleanupStaleAgents(maxAge) on the periodic interval where other maintenance
runs, pass a configured maxAge (duration constant or config value), and
handle/log the returned cleaned count (e.g., processLogger.Infof or p.logger) so
the call is used; otherwise remove the cleanupStaleAgents function and any
related unused kvstore calls to satisfy lint.
- Around line 349-358: In Plugin.cleanupStaleAgents, stop ignoring errors from
p.kvstore.ListActiveAgents, p.kvstore.SaveAgent, and p.kvstore.DeleteAgent:
check the error returned by ListActiveAgents and return/log and short-circuit if
it fails; for each agent that needs cleanup, call SaveAgent and if it returns an
error log and skip deleting/incrementing cleaned (or return depending on
policy), then call DeleteAgent and log/short-circuit on its error as well;
ensure the cleaned counter is only incremented after both SaveAgent and
DeleteAgent succeed and use p.logger or appropriate logging helper to record
errors with contextual info (agent.CursorAgentID, createdAt) so cleanup failures
are visible.
- Around line 347-364: The cleanupStaleAgents function currently sets
agent.Status = "STOPPED" and deletes the record directly; instead call the
existing STOPPED handling flow so reactions, thread messages, post props, KV
save, and websocket events run. Locate cleanupStaleAgents and replace the direct
status set/save/delete with a call into the module that handles status
transitions (e.g., the existing onAgentStopped/handleAgentStatusChange function
or the same routine used when an agent normally goes STOPPED) passing the agent
(and new status "STOPPED"); ensure that routine performs the reaction swap
(hourglass -> no_entry_sign), posts the thread message, updates post props,
saves the agent to kvstore, publishes websocket events, and only after that
remove the KV record if the handler deems deletion appropriate.
| func (p *Plugin) cleanupStaleAgents(maxAge time.Duration) int { | ||
| agents, _ := p.kvstore.ListActiveAgents() | ||
|
|
||
| cleaned := 0 | ||
| for _, agent := range agents { | ||
| createdAt := time.Unix(0, agent.CreatedAt*int64(time.Millisecond)) | ||
| if time.Since(createdAt) > maxAge { | ||
| agent.Status = "STOPPED" | ||
| p.kvstore.SaveAgent(agent) | ||
| p.kvstore.DeleteAgent(agent.CursorAgentID) |
There was a problem hiding this comment.
Handle kvstore errors to avoid silent cleanup failures.
ListActiveAgents, SaveAgent, and DeleteAgent errors are ignored, which can silently fail cleanup and over-count cleaned. Please log and short-circuit on errors (see diff in previous comment).
🧰 Tools
🪛 GitHub Check: plugin-ci / lint
[failure] 349-349:
func (*Plugin).cleanupStaleAgents is unused (unused)
[failure] 358-358:
Error return value of p.kvstore.DeleteAgent is not checked (errcheck)
[failure] 357-357:
Error return value of p.kvstore.SaveAgent is not checked (errcheck)
🤖 Prompt for AI Agents
In `@server/poller.go` around lines 349 - 358, In Plugin.cleanupStaleAgents, stop
ignoring errors from p.kvstore.ListActiveAgents, p.kvstore.SaveAgent, and
p.kvstore.DeleteAgent: check the error returned by ListActiveAgents and
return/log and short-circuit if it fails; for each agent that needs cleanup,
call SaveAgent and if it returns an error log and skip deleting/incrementing
cleaned (or return depending on policy), then call DeleteAgent and
log/short-circuit on its error as well; ensure the cleaned counter is only
incremented after both SaveAgent and DeleteAgent succeed and use p.logger or
appropriate logging helper to record errors with contextual info
(agent.CursorAgentID, createdAt) so cleanup failures are visible.
…fter-save bug Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@cursor This function cleanupStaleAgents is still unused. Please wire it into the poller's pollAgentStatuses function to run on every poll cycle with a max age of 24 hours. |
|
Implemented and pushed in You can reply with:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/poller.go`:
- Around line 349-385: The cleanupStaleAgents loop currently marks stale agents
STOPPED but does not update the bot reply card or post props; call
handleAgentStopped(agent) (to run updateBotReplyWithAttachment and
reaction/attachment logic) before persisting the STOPPED state with
p.kvstore.SaveAgent, then after SaveAgent returns successfully call
updateBotPostProps(agent) to ensure post props reflect the STOPPED status and
keep webapp dropdown filtering correct; ensure you still log/post/create
reactions as now and only increment cleaned after both save and post-prop update
succeed.
🧹 Nitpick comments (1)
server/poller.go (1)
359-359: Use thecursor.AgentStatusStoppedconstant instead of hardcoded string.The rest of the codebase uses
cursor.AgentStatusStopped(see line 111). Using the constant ensures type safety and consistency.♻️ Proposed fix
- agent.Status = "STOPPED" + agent.Status = string(cursor.AgentStatusStopped)
Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/poller.go (2)
363-371: Consider adding a defensive status check before cleanup.The function processes any stale agent without verifying it's still in a non-terminal state. While
ListActiveAgentsshould only return CREATING/RUNNING agents per the index design, adding a guard would protect against data inconsistencies and make the docstring accurate.🛡️ Proposed defensive check
for _, agent := range agents { if agent == nil || agent.CreatedAt <= 0 { continue } + + status := cursor.AgentStatus(agent.Status) + if status.IsTerminal() { + continue + } createdAt := time.UnixMilli(agent.CreatedAt) if now.Sub(createdAt) <= maxAge { continue }
375-380: Consider posting a timeout-specific message for better UX.Currently
handleAgentStoppedposts "Agent was stopped." which doesn't indicate the reason was a timeout. Users might be confused why the agent stopped. Adding a timeout-specific notification would improve clarity.💬 Proposed timeout notification
p.handleAgentStopped(agent) + + // Post timeout-specific notification (supplements the generic stopped message). + if _, appErr := p.API.CreatePost(&model.Post{ + UserId: p.getBotUserID(), + ChannelId: agent.ChannelID, + RootId: agent.PostID, + Message: fmt.Sprintf("⏱️ Agent timed out after %s without completing.", maxAge), + }); appErr != nil { + p.API.LogError("Failed to post timeout notification", "agent_id", agent.CursorAgentID, "error", appErr.Error()) + } + if err := p.kvstore.SaveAgent(agent); err != nil {Alternatively, modify
handleAgentStoppedto accept an optional reason parameter, or create a separate handler for timeout-stopped agents.
2 participants
Summary
Test plan
Summary by CodeRabbit
New Features
Tests