Skip to content

fix(agent): restart codex agents after config changes #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wanghaojie124 merged 3 commits into from
Aug 3, 2026
+637 −127
Merged

fix(agent): restart codex agents after config changes #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3087e42
fix(agent): restart codex agents after config changes
Aug 3, 2026
f7003cc
fix(codex): preserve conversations across agent restarts
Aug 3, 2026
52def89
fix(codex): preserve conversations across restarts
Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions internal/agent/lifecycle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,92 @@ import (
"context"
"fmt"
"strings"
"time"

agentruntime "csgclaw/internal/runtime"
)

// restartCodexRuntime restarts the in-process Codex app-server without deleting
// its runtime directory. In particular, persisted Codex threads and conversation
// mappings survive profile changes such as reasoning effort updates.
func (s *Service) restartCodexRuntime(ctx context.Context, id string) (Agent, error) {
ctx, release, err := s.acquireAgentLifecycle(ctx, id)
if err != nil {
return Agent{}, err
}
defer release()
return s.restartCodexRuntimeLocked(ctx, id)
}

func (s *Service) restartCodexRuntimeLocked(ctx context.Context, id string) (Agent, error) {
got, ok := s.Agent(id)
if !ok {
return Agent{}, fmt.Errorf("agent %q not found", strings.TrimSpace(id))
}
if !strings.EqualFold(strings.TrimSpace(got.RuntimeKind), RuntimeKindCodex) {
return Agent{}, fmt.Errorf("agent %q is not a codex agent", got.ID)
}

runtimeImpl, err := s.runtimeForKind(RuntimeKindCodex)
if err != nil {
return Agent{}, err
}
handle := runtimeHandleForAgent(got)
s.stopLifecycleAgent(got.ID)
if _, err := runtimeImpl.Stop(ctx, handle); err != nil {
return Agent{}, fmt.Errorf("stop codex agent for restart: %w", err)
}
if _, err := s.updateRuntimeState(got.ID, agentruntime.Info{
HandleID: strings.TrimSpace(got.BoxID),
State: agentruntime.StateStopped,
}); err != nil {
return Agent{}, fmt.Errorf("save stopped codex agent state: %w", err)
}
if err := s.provisionRuntimeForAgent(ctx, runtimeImpl, got, ""); err != nil {
return Agent{}, fmt.Errorf("provision codex agent for restart: %w", err)
}
state, err := runtimeImpl.Start(ctx, handle)

@xxx7xxxx xxx7xxxx Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This restart enters Runtime.Start, but that path still creates SessionSpec without reading the new conversation_sessions field. ensureSession then persists the restarted session with an empty map, so every automatic profile/MCP restart erases all room-to-thread mappings. I reproduced this with Stop -> Start: the saved room-1 mapping was absent from the restart spec. Load the mapping in the normal Start path and add a stop/start regression test.

@wanghaojie124 wanghaojie124 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 52def89. Runtime.Start now reads the persisted session metadata and passes ConversationSessions into the new SessionSpec. I also added a Stop -> Start regression test that verifies the room-to-thread mapping reaches the restarted session.

if err != nil {
return Agent{}, fmt.Errorf("start codex agent after settings update: %w", err)
}
info, err := s.runtimeInfo(ctx, runtimeImpl, handle)
if err != nil {
return Agent{}, fmt.Errorf("read codex agent state after restart: %w", err)
}
if info.State == "" {
info.State = state
}

s.mu.Lock()
current, key, ok := s.agentByIDLocked(id)
if !ok {
s.mu.Unlock()
return Agent{}, fmt.Errorf("agent %q not found", strings.TrimSpace(id))
}
if handleID := strings.TrimSpace(info.HandleID); handleID != "" {
current.BoxID = handleID
}
current.Status = string(info.State)
current.AgentProfile.EnvRestartRequired = false
current.UpdatedAt = time.Now().UTC()
s.agents[key] = current
s.syncRuntimeRecordLocked(current)
err = s.saveLocked()
s.mu.Unlock()
if err != nil {
return Agent{}, err
}

restarted, ok := s.Agent(id)
if !ok {
return Agent{}, fmt.Errorf("agent %q not found", strings.TrimSpace(id))
}
if err := s.syncLifecycleForAgent(ctx, restarted); err != nil {
return Agent{}, err
}
return restarted, nil
}

type LifecycleObserver interface {
EnsureAgent(context.Context, Agent) error
StopAgent(string)
Expand Down
3 changes: 3 additions & 0 deletions internal/agent/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1678,6 +1678,9 @@ func (s *Service) Start(ctx context.Context, id string) (Agent, error) {
if !ok {
return Agent{}, fmt.Errorf("agent %q not found", id)
}
if got.AgentProfile.EnvRestartRequired && !got.AgentProfile.ImageUpgradeRequired && strings.EqualFold(strings.TrimSpace(got.RuntimeKind), RuntimeKindCodex) {
return s.restartCodexRuntimeLocked(ctx, id)
}
if got.AgentProfile.EnvRestartRequired || got.AgentProfile.ImageUpgradeRequired {
return s.Recreate(ctx, id)
}
Expand Down
14 changes: 11 additions & 3 deletions internal/agent/service_profiles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ func (s *Service) UpdateAgentProfile(id string, profile AgentProfile) (AgentProf
return AgentProfileView{}, err
}
s.mu.Unlock()
if restartRequired && runtimeRunning && !isGatewayRuntimeKind(runtimeKind) {
if restartRequired && runtimeRunning && strings.EqualFold(runtimeKind, RuntimeKindCodex) {
if _, err := s.restartCodexRuntime(context.Background(), id); err != nil {
return AgentProfileView{}, err
}
} else if restartRequired && runtimeRunning && !isGatewayRuntimeKind(runtimeKind) {
s.stopLifecycleAgent(id)
}
if err := s.syncGatewayAfterProfileChange(context.Background(), id, previous, normalized, restartRequired); err != nil {
Expand Down Expand Up @@ -503,15 +507,19 @@ func (s *Service) update(ctx context.Context, id string, req UpdateRequest) (Age
}
}
}
if restartRequired && runtimeRunning && !isGatewayRuntimeKind(runtimeKind) {
if restartRequired && runtimeRunning && strings.EqualFold(runtimeKind, RuntimeKindCodex) {
if _, err := s.restartCodexRuntime(ctx, id); err != nil {

@xxx7xxxx xxx7xxxx Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new restart path makes the existing Codex MCP API subtest terminate its own test process: apiFakeCodexManager stores os.Getpid() and its Stop is a no-op, so Runtime.Stop sends SIGINT to the package. go test ./internal/api -run '^TestHandleAgentsMCPServersClosedLoopForSupportedRuntimes$/^codex$' fails deterministically with signal: interrupt. Update the fixture to model a safely stoppable process and assert the restart behavior.

@wanghaojie124 wanghaojie124 Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 52def89. The API fixture now launches a dedicated test child process and tracks/stops it through the fake manager instead of using the package test PID. The exact Codex MCP subtest now passes, including with the race detector.

return Agent{}, err
}
} else if restartRequired && runtimeRunning && !isGatewayRuntimeKind(runtimeKind) {
s.stopLifecycleAgent(id)
}

updated, ok := s.Agent(id)
if !ok {
return Agent{}, fmt.Errorf("agent %q not found", id)
}
if mcpServersUpdated && restartRequired && runtimeRunning && isManagerAgent(updated) {
if mcpServersUpdated && restartRequired && runtimeRunning && isManagerAgent(updated) && updated.AgentProfile.EnvRestartRequired {
return s.Recreate(ctx, id)
}
if runtimeAffectingUpdate {
Expand Down
109 changes: 90 additions & 19 deletions internal/agent/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1239,13 +1239,20 @@ func TestUpdateCodexAgentProfilePatchRestartsActiveBridge(t *testing.T) {
if current.Profile.ModelID != "deepseek-v4-pro" {
t.Fatalf("responses probe modelID = %q, want deepseek-v4-pro", current.Profile.ModelID)
}
if current.Profile.ReasoningEffort != "high" {
t.Fatalf("responses probe reasoning effort = %q, want high", current.Profile.ReasoningEffort)
}
return nil
},
restart: func(change agentruntime.RuntimeConfigChange) (bool, error) {
return change.Previous.Profile.BaseURL != change.Current.Profile.BaseURL ||
change.Previous.Profile.APIKey != change.Current.Profile.APIKey ||
change.Previous.Profile.ModelID != change.Current.Profile.ModelID, nil
},
del: func(context.Context, agentruntime.Handle) error {
t.Fatal("automatic Codex restart deleted the runtime and its history")
return nil
},
}),
)
if err != nil {
Expand Down Expand Up @@ -1273,10 +1280,11 @@ func TestUpdateCodexAgentProfilePatchRestartsActiveBridge(t *testing.T) {
CreatedAt: time.Date(2026, 5, 18, 9, 0, 0, 0, time.UTC),
}
nextProfile := AgentProfile{
Provider: ProviderAPI,
BaseURL: "https://api.deepseek.com",
APIKey: "deepseek-key",
ModelID: "deepseek-v4-pro",
Provider: ProviderAPI,
BaseURL: "https://api.deepseek.com",
APIKey: "deepseek-key",
ModelID: "deepseek-v4-pro",
ReasoningEffort: "high",
}

updated, err := svc.Update(context.Background(), "u-dev", UpdateRequest{AgentProfile: &nextProfile})
Expand All @@ -1286,12 +1294,15 @@ func TestUpdateCodexAgentProfilePatchRestartsActiveBridge(t *testing.T) {
if probeCalls != 1 {
t.Fatalf("responses probe calls = %d, want 1", probeCalls)
}
if !updated.AgentProfile.EnvRestartRequired {
t.Fatal("Update().AgentProfile.EnvRestartRequired = false, want true so running Codex bridge is refreshed")
if updated.AgentProfile.EnvRestartRequired {
t.Fatal("Update().AgentProfile.EnvRestartRequired = true, want false after automatic Codex restart")
}
if len(observer.stopCalls) != 1 || observer.stopCalls[0] != "u-dev" {
t.Fatalf("StopAgent() calls = %+v, want [u-dev]", observer.stopCalls)
}
if len(observer.ensureCalls) != 1 || observer.ensureCalls[0].ID != "u-dev" {
t.Fatalf("EnsureAgent() calls = %+v, want one restart for u-dev", observer.ensureCalls)
}
}

func TestUpdateAgentProfileCodexRuntimeFallbackRestartsActiveBridge(t *testing.T) {
Expand Down Expand Up @@ -1366,19 +1377,76 @@ func TestUpdateAgentProfileCodexRuntimeFallbackRestartsActiveBridge(t *testing.T
if err != nil {
t.Fatalf("UpdateAgentProfile() error = %v", err)
}
if !view.EnvRestartRequired {
t.Fatal("UpdateAgentProfile().EnvRestartRequired = false, want true so running Codex bridge is refreshed")
if view.EnvRestartRequired {
t.Fatal("UpdateAgentProfile().EnvRestartRequired = true, want false after automatic Codex restart")
}
if len(observer.stopCalls) != 1 || observer.stopCalls[0] != "u-dev" {
t.Fatalf("StopAgent() calls = %+v, want [u-dev]", observer.stopCalls)
}

started, err := svc.Start(context.Background(), "u-dev")
if len(observer.ensureCalls) != 1 || observer.ensureCalls[0].ID != "u-dev" {
t.Fatalf("EnsureAgent() calls = %+v, want one automatic restart for u-dev", observer.ensureCalls)
}
}

func TestCodexAutomaticRestartFailurePersistsStoppedAndRetriesWithoutDelete(t *testing.T) {
t.Setenv("HOME", t.TempDir())
startErr := errors.New("start failed")
deleteCalls := 0
svc, err := NewService(
testModelConfig(),
config.ServerConfig{},
"manager-image:test",
filepath.Join(t.TempDir(), "agents.json"),
WithRuntime(fakeAgentRuntime{
kind: RuntimeKindCodex,
restart: func(agentruntime.RuntimeConfigChange) (bool, error) { return true, nil },
start: func(context.Context, agentruntime.Handle) (agentruntime.State, error) {
if startErr != nil {
return agentruntime.StateStopped, startErr
}
return agentruntime.StateRunning, nil
},
info: func(context.Context, agentruntime.Handle) (agentruntime.Info, error) {
return agentruntime.Info{State: agentruntime.StateRunning}, nil
},
del: func(context.Context, agentruntime.Handle) error {
deleteCalls++
return nil
},
}),
)
if err != nil {
t.Fatalf("NewService() error = %v", err)
}
profile := AgentProfile{Name: "dev", Provider: ProviderAPI, BaseURL: "https://old.example/v1", APIKey: "key", ModelID: "old", ProfileComplete: true}
svc.agents["u-dev"] = Agent{ID: "u-dev", Name: "dev", RuntimeID: "rt-u-dev", RuntimeKind: RuntimeKindCodex, Role: RoleWorker, Status: string(agentruntime.StateRunning), AgentProfile: profile, ProfileComplete: true}

_, err = svc.Update(context.Background(), "u-dev", UpdateRequest{AgentProfile: &AgentProfile{Provider: ProviderAPI, BaseURL: "https://new.example/v1", APIKey: "key", ModelID: "new"}})
if !errors.Is(err, startErr) {
t.Fatalf("Update() error = %v, want %v", err, startErr)
}
failed, ok := svc.agentSnapshot("u-dev")
if !ok {
t.Fatal("Agent() ok = false")
}
if got, want := failed.Status, string(agentruntime.StateStopped); got != want {
t.Fatalf("failed restart status = %q, want %q", got, want)
}
if !failed.AgentProfile.EnvRestartRequired {
t.Fatal("failed restart cleared EnvRestartRequired")
}

startErr = nil
restarted, err := svc.Start(context.Background(), "u-dev")
if err != nil {
t.Fatalf("Start() error = %v", err)
t.Fatalf("Start() retry error = %v", err)
}
if started.AgentProfile.EnvRestartRequired {
t.Fatal("Start().AgentProfile.EnvRestartRequired = true, want false after recreate")
if restarted.AgentProfile.EnvRestartRequired || restarted.Status != string(agentruntime.StateRunning) {
t.Fatalf("Start() retry agent = %+v, want running without restart flag", restarted)
}
if deleteCalls != 0 {
t.Fatalf("Delete() calls = %d, want 0 to preserve history", deleteCalls)
}
}

Expand Down Expand Up @@ -1742,12 +1810,15 @@ func TestUpdateCodexLocalWorkspaceDirMarksRunningRuntimeForRestart(t *testing.T)
if err != nil {
t.Fatalf("Update() error = %v", err)
}
if !updated.AgentProfile.EnvRestartRequired {
t.Fatal("Update().AgentProfile.EnvRestartRequired = false, want true after codex local_workspace_dir change")
if updated.AgentProfile.EnvRestartRequired {
t.Fatal("Update().AgentProfile.EnvRestartRequired = true, want false after automatic Codex restart")
}
if len(observer.stopCalls) != 1 || observer.stopCalls[0] != "u-dev" {
t.Fatalf("StopAgent() calls = %+v, want [u-dev]", observer.stopCalls)
}
if len(observer.ensureCalls) != 1 || observer.ensureCalls[0].ID != "u-dev" {
t.Fatalf("EnsureAgent() calls = %+v, want one automatic restart for u-dev", observer.ensureCalls)
}
}

func TestAddMCPServersFromHubImportsRuntimeServersOnce(t *testing.T) {
Expand Down Expand Up @@ -2139,7 +2210,7 @@ func TestAddMCPServersSerializesWithDirectMCPServersUpdate(t *testing.T) {
}
}

func TestAddMCPServersFromHubRecreatesRunningCodexManager(t *testing.T) {
func TestAddMCPServersFromHubRestartsRunningCodexManagerWithoutDeletingHistory(t *testing.T) {
t.Setenv("HOME", t.TempDir())
origLocateCodexCLI := locateCodexCLI
locateCodexCLI = func() (string, error) { return "/usr/local/bin/codex", nil }
Expand Down Expand Up @@ -2204,15 +2275,15 @@ func TestAddMCPServersFromHubRecreatesRunningCodexManager(t *testing.T) {
if err != nil {
t.Fatalf("AddMCPServersFromHub() error = %v", err)
}
if deleteCalls != 1 || newCalls != 1 {
t.Fatalf("manager recreate calls = delete %d/new %d, want 1/1", deleteCalls, newCalls)
if deleteCalls != 0 || newCalls != 0 {
t.Fatalf("manager restart calls = delete %d/new %d, want 0/0 to preserve history", deleteCalls, newCalls)
}
assertMCPServersHasServer(t, provisionedMCPServers, "context7")
assertMCPServersHasServer(t, updated.MCPServers, "context7")
if updated.AgentProfile.EnvRestartRequired {
t.Fatal("AddMCPServersFromHub().AgentProfile.EnvRestartRequired = true, want false after successful manager recreate")
t.Fatal("AddMCPServersFromHub().AgentProfile.EnvRestartRequired = true, want false after successful manager restart")
}
if got, want := updated.BoxID, "codex-manager-session-new"; got != want {
if got, want := updated.BoxID, "codex-manager-session-old"; got != want {
t.Fatalf("AddMCPServersFromHub().BoxID = %q, want %q", got, want)
}
}
Expand Down
Loading
Loading