Fix resume creating new agents instead of restarting stopped ones#271
Conversation
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where resuming a stopped agent created a new agent instead of resuming the existing one in-place. It introduces a Resume flag to the CreateAgentRequest and updates the agent-handling logic to preserve the agent record and restart the container when this flag is set. Comprehensive unit tests have been added to verify this behavior. The review feedback suggests defensively initializing existingAgent.AppliedConfig if it is nil to prevent potential issues when applying task and attach parameters.
| if req.Task != "" && existingAgent.AppliedConfig != nil { | ||
| existingAgent.AppliedConfig.Task = req.Task | ||
| existingAgent.AppliedConfig.Attach = req.Attach | ||
| } |
There was a problem hiding this comment.
If existingAgent.AppliedConfig is nil, the task and attach parameters will not be updated, and subsequent calls (such as dispatcher.DispatchAgentStart or database updates) may encounter nil pointer dereferences.
To prevent panics and ensure robust handling, defensively initialize existingAgent.AppliedConfig if it is nil before applying the request parameters.
if existingAgent.AppliedConfig == nil {
existingAgent.AppliedConfig = &store.AgentAppliedConfig{}
}
if req.Task != "" {
existingAgent.AppliedConfig.Task = req.Task
existingAgent.AppliedConfig.Attach = req.Attach
}The Hub server's CreateAgentRequest struct was missing the Resume field that the CLI client sends, causing all stopped agents to be treated as stale and deleted+recreated rather than restarted in-place. Add the Resume field to the Hub's CreateAgentRequest and handle the stopped+resume case in handleExistingAgent: when Resume=true and the agent is in PhaseStopped, restart it in-place (preserving agent ID, metadata, and template association) rather than deleting and recreating. When Resume is not set (i.e. scion start), the existing delete+recreate behavior is preserved unchanged.
- Defensively initialize existingAgent.AppliedConfig when nil before applying task/attach params during resume, preventing potential nil pointer dereference (review feedback). - Fix profile-telegram.ts TS6133 error: _linkedTelegramId was declared but never read, causing CI Build & Test failure.
621a818 to
e21c723
Compare
Summary
Fixes ptone#61 —
scion resumewas creating new agents instead of resuming stopped ones.CreateAgentRequeststruct was missing theResumefield that the CLI client sends. All stopped agents were treated as "stale" and deleted+recreated, losing the agent ID, template association, and metadata.Resumefield to the Hub'sCreateAgentRequestand added a new handler path inhandleExistingAgentthat restarts stopped agents in-place whenResume=true, mirroring the existing suspended-agent path.scion starton a stopped agent (withoutResume) still deletes and recreates — onlyscion resumetriggers the in-place restart.Test plan
TestCreateAgent_ResumeFromStoppedStatus— verifies resume returns 200, preserves agent ID, callsDispatchAgentStart(not delete+create)TestCreateAgent_StartFromStoppedStatus_NoResume— verifies start (no resume flag) still recreates with 201TestCreateProjectAgent_ResumeFromStoppedStatus— same resume test through project-scoped endpointTestCreateAgent_RecreateFromStoppedStatusstill passes (no regression)go test ./pkg/hub/...passesgo vet ./...andgo build ./...pass