refactor: move toolsrv spawn/process management to toolclient
Move spawn.go from toolsrv/ to cmd/olliesrv/internal/toolclient/: - Process, ProcessKeeper, Spawn, SpawnRemote now in toolclient package - toolsrv/ now only contains client code (Dial, Conn, etc.) This clarifies the architecture: - toolsrv/ = client SDK for connecting to toolsrv - cmd/toolsrv/ = the toolsrv server - cmd/olliesrv/internal/toolclient/ = olliesrv's toolsrv process management Removed ProcessKeeper tests from toolsrv integration tests since they now belong to toolclient.
This commit is contained in:
parent
d292f2aa59
commit
a0315a558a
|
|
@ -10,6 +10,7 @@ import (
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"ollie/cmd/olliesrv/internal/agent"
|
"ollie/cmd/olliesrv/internal/agent"
|
||||||
|
"ollie/cmd/olliesrv/internal/toolclient"
|
||||||
olog "ollie/log"
|
olog "ollie/log"
|
||||||
"ollie/paths"
|
"ollie/paths"
|
||||||
"ollie/toolsrv"
|
"ollie/toolsrv"
|
||||||
|
|
@ -26,8 +27,8 @@ type Session struct {
|
||||||
Cancel context.CancelFunc
|
Cancel context.CancelFunc
|
||||||
|
|
||||||
// Tool server lifecycle (set during setup/teardown, not raced)
|
// Tool server lifecycle (set during setup/teardown, not raced)
|
||||||
Proc *toolsrv.Process
|
Proc *toolclient.Process
|
||||||
Keeper *toolsrv.ProcessKeeper
|
Keeper *toolclient.ProcessKeeper
|
||||||
Remote string
|
Remote string
|
||||||
|
|
||||||
// Agent state
|
// Agent state
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
|
||||||
"ollie/cmd/olliesrv/internal/agent"
|
"ollie/cmd/olliesrv/internal/agent"
|
||||||
|
"ollie/cmd/olliesrv/internal/toolclient"
|
||||||
"ollie/paths"
|
"ollie/paths"
|
||||||
"ollie/cmd/olliesrv/internal/prompts"
|
"ollie/cmd/olliesrv/internal/prompts"
|
||||||
"ollie/toolsrv"
|
"ollie/toolsrv"
|
||||||
|
|
@ -13,8 +14,8 @@ import (
|
||||||
// SessionInfra holds the tool server infrastructure for a session.
|
// SessionInfra holds the tool server infrastructure for a session.
|
||||||
// Created by SetupToolServer and used by both CreateAgent and restoreSession.
|
// Created by SetupToolServer and used by both CreateAgent and restoreSession.
|
||||||
type SessionInfra struct {
|
type SessionInfra struct {
|
||||||
Proc *toolsrv.Process
|
Proc *toolclient.Process
|
||||||
Keeper *toolsrv.ProcessKeeper
|
Keeper *toolclient.ProcessKeeper
|
||||||
ToolsConn *toolsrv.Conn
|
ToolsConn *toolsrv.Conn
|
||||||
NewToolServer func() *toolsrv.Conn
|
NewToolServer func() *toolsrv.Conn
|
||||||
Platform string
|
Platform string
|
||||||
|
|
@ -36,8 +37,8 @@ type ToolServerConfig struct {
|
||||||
|
|
||||||
// InfraConfig describes existing infrastructure to reuse.
|
// InfraConfig describes existing infrastructure to reuse.
|
||||||
type InfraConfig struct {
|
type InfraConfig struct {
|
||||||
Proc *toolsrv.Process
|
Proc *toolclient.Process
|
||||||
Keeper *toolsrv.ProcessKeeper
|
Keeper *toolclient.ProcessKeeper
|
||||||
ToolsConn *toolsrv.Conn
|
ToolsConn *toolsrv.Conn
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -76,15 +77,15 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) {
|
||||||
IsGitRepo: paths.IsGitRepo(cfg.CWD),
|
IsGitRepo: paths.IsGitRepo(cfg.CWD),
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
var proc *toolsrv.Process
|
var proc *toolclient.Process
|
||||||
var keeper *toolsrv.ProcessKeeper
|
var keeper *toolclient.ProcessKeeper
|
||||||
var newToolServer func() *toolsrv.Conn
|
var newToolServer func() *toolsrv.Conn
|
||||||
var err error
|
var err error
|
||||||
var platform string
|
var platform string
|
||||||
var isGitRepo bool
|
var isGitRepo bool
|
||||||
|
|
||||||
if cfg.RemoteTarget != "" {
|
if cfg.RemoteTarget != "" {
|
||||||
proc, err = toolsrv.SpawnRemote(cfg.Ctx, toolsrv.RemoteConfig{
|
proc, err = toolclient.SpawnRemote(cfg.Ctx, toolclient.RemoteConfig{
|
||||||
SSHTarget: cfg.RemoteTarget,
|
SSHTarget: cfg.RemoteTarget,
|
||||||
CWD: cfg.CWD,
|
CWD: cfg.CWD,
|
||||||
SessionID: cfg.SessionID,
|
SessionID: cfg.SessionID,
|
||||||
|
|
@ -93,8 +94,8 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("remote spawn: %w", err)
|
return nil, fmt.Errorf("remote spawn: %w", err)
|
||||||
}
|
}
|
||||||
keeper = toolsrv.NewProcessKeeper(cfg.Ctx, proc, func(ctx context.Context) (*toolsrv.Process, error) {
|
keeper = toolclient.NewProcessKeeper(cfg.Ctx, proc, func(ctx context.Context) (*toolclient.Process, error) {
|
||||||
return toolsrv.SpawnRemote(ctx, toolsrv.RemoteConfig{
|
return toolclient.SpawnRemote(ctx, toolclient.RemoteConfig{
|
||||||
SSHTarget: cfg.RemoteTarget,
|
SSHTarget: cfg.RemoteTarget,
|
||||||
CWD: cfg.CWD,
|
CWD: cfg.CWD,
|
||||||
SessionID: cfg.SessionID,
|
SessionID: cfg.SessionID,
|
||||||
|
|
@ -111,19 +112,19 @@ func SetupToolServer(cfg ToolServerConfig) (*SessionInfra, error) {
|
||||||
platform = proc.Info.Platform
|
platform = proc.Info.Platform
|
||||||
isGitRepo = proc.Info.IsGitRepo
|
isGitRepo = proc.Info.IsGitRepo
|
||||||
} else {
|
} else {
|
||||||
var dialOpts []toolsrv.Option
|
var dialOpts []toolclient.Option
|
||||||
if cfg.Yolo {
|
if cfg.Yolo {
|
||||||
dialOpts = append(dialOpts, toolsrv.WithYolo())
|
dialOpts = append(dialOpts, toolclient.WithYolo())
|
||||||
}
|
}
|
||||||
if cfg.SessionID != "" {
|
if cfg.SessionID != "" {
|
||||||
dialOpts = append(dialOpts, toolsrv.WithSessionID(cfg.SessionID))
|
dialOpts = append(dialOpts, toolclient.WithSessionID(cfg.SessionID))
|
||||||
}
|
}
|
||||||
proc, err = toolsrv.Spawn(cfg.Ctx, cfg.CWD, dialOpts...)
|
proc, err = toolclient.Spawn(cfg.Ctx, cfg.CWD, dialOpts...)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("local spawn: %w", err)
|
return nil, fmt.Errorf("local spawn: %w", err)
|
||||||
}
|
}
|
||||||
keeper = toolsrv.NewProcessKeeper(cfg.Ctx, proc, func(ctx context.Context) (*toolsrv.Process, error) {
|
keeper = toolclient.NewProcessKeeper(cfg.Ctx, proc, func(ctx context.Context) (*toolclient.Process, error) {
|
||||||
return toolsrv.Spawn(ctx, cfg.CWD, dialOpts...)
|
return toolclient.Spawn(ctx, cfg.CWD, dialOpts...)
|
||||||
})
|
})
|
||||||
newToolServer = func() *toolsrv.Conn {
|
newToolServer = func() *toolsrv.Conn {
|
||||||
conn, err := keeper.Dial()
|
conn, err := keeper.Dial()
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
// spawn.go - Process spawning and lifecycle management for toolsrv.
|
// spawn.go - Process spawning and lifecycle management for toolsrv.
|
||||||
package toolsrv
|
package toolclient
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"bufio"
|
"bufio"
|
||||||
|
|
@ -18,6 +18,7 @@ import (
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"ollie/paths"
|
"ollie/paths"
|
||||||
|
toolsrvclient "ollie/toolsrv"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Process represents a running toolsrv process.
|
// Process represents a running toolsrv process.
|
||||||
|
|
@ -314,13 +315,13 @@ func (pk *ProcessKeeper) SetContext(ctx context.Context) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Dial connects to the managed process, respawning if necessary.
|
// Dial connects to the managed process, respawning if necessary.
|
||||||
func (pk *ProcessKeeper) Dial() (*Conn, error) {
|
func (pk *ProcessKeeper) Dial() (*toolsrvclient.Conn, error) {
|
||||||
pk.mu.Lock()
|
pk.mu.Lock()
|
||||||
defer pk.mu.Unlock()
|
defer pk.mu.Unlock()
|
||||||
|
|
||||||
// Try to connect to existing process
|
// Try to connect to existing process
|
||||||
if pk.proc != nil {
|
if pk.proc != nil {
|
||||||
conn, err := Dial(pk.proc.SocketPath, pk.proc.Secret)
|
conn, err := toolsrvclient.Dial(pk.proc.SocketPath, pk.proc.Secret)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
// Save the secret if this was first connection
|
// Save the secret if this was first connection
|
||||||
if pk.proc.Secret == "" {
|
if pk.proc.Secret == "" {
|
||||||
|
|
@ -337,7 +338,7 @@ func (pk *ProcessKeeper) Dial() (*Conn, error) {
|
||||||
return nil, fmt.Errorf("respawn: %w", err)
|
return nil, fmt.Errorf("respawn: %w", err)
|
||||||
}
|
}
|
||||||
pk.proc = proc
|
pk.proc = proc
|
||||||
conn, err := Dial(proc.SocketPath, proc.Secret)
|
conn, err := toolsrvclient.Dial(proc.SocketPath, proc.Secret)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
@ -379,41 +379,6 @@ func TestIntegration_ToolExecutionError(t *testing.T) {
|
||||||
t.Logf("Error tool result: %+v", toolResult)
|
t.Logf("Error tool result: %+v", toolResult)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestIntegration_ProcessKeeperReconnect(t *testing.T) {
|
|
||||||
socketPath, cleanup := startTestServer(t)
|
|
||||||
defer cleanup()
|
|
||||||
|
|
||||||
secret := "test-secret-keeper"
|
|
||||||
|
|
||||||
// Create a process manually (simulating what Spawn returns)
|
|
||||||
proc := &toolsrv.Process{
|
|
||||||
SocketPath: socketPath,
|
|
||||||
Secret: secret,
|
|
||||||
}
|
|
||||||
|
|
||||||
// Create keeper without respawn (we're testing reconnect, not respawn)
|
|
||||||
keeper := toolsrv.NewProcessKeeper(context.Background(), proc, nil)
|
|
||||||
|
|
||||||
// First dial
|
|
||||||
conn1, err := keeper.Dial()
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("first Dial failed: %v", err)
|
|
||||||
}
|
|
||||||
token := conn1.Token()
|
|
||||||
conn1.Close()
|
|
||||||
|
|
||||||
// Second dial should reconnect with same secret
|
|
||||||
conn2, err := keeper.Dial()
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("second Dial failed: %v", err)
|
|
||||||
}
|
|
||||||
defer conn2.Close()
|
|
||||||
|
|
||||||
if conn2.Token() != token {
|
|
||||||
t.Errorf("token changed after reconnect: %q -> %q", token, conn2.Token())
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestIntegration_ConcurrentConnections(t *testing.T) {
|
func TestIntegration_ConcurrentConnections(t *testing.T) {
|
||||||
socketPath, cleanup := startTestServer(t)
|
socketPath, cleanup := startTestServer(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
@ -499,22 +464,6 @@ func TestIntegration_ProcessKeeperRespawn(t *testing.T) {
|
||||||
t.Error("old secret should not work on new server")
|
t.Error("old secret should not work on new server")
|
||||||
}
|
}
|
||||||
|
|
||||||
// Verify ProcessKeeper handles this correctly
|
|
||||||
proc := &toolsrv.Process{
|
|
||||||
SocketPath: socketPath2,
|
|
||||||
Secret: secret2,
|
|
||||||
}
|
|
||||||
keeper := toolsrv.NewProcessKeeper(context.Background(), proc, nil)
|
|
||||||
|
|
||||||
conn4, err := keeper.Dial()
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("keeper Dial failed: %v", err)
|
|
||||||
}
|
|
||||||
if conn4.Token() != token2 {
|
|
||||||
t.Error("keeper should reconnect with same token")
|
|
||||||
}
|
|
||||||
conn4.Close()
|
|
||||||
|
|
||||||
t.Logf("Successfully verified: old_token=%s new_token=%s", token1[:8], token2[:8])
|
t.Logf("Successfully verified: old_token=%s new_token=%s", token1[:8], token2[:8])
|
||||||
_ = cwd // silence unused warning
|
_ = cwd // silence unused warning
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue