From 3f9f4753ef390121cb46410c5962938eb5302384 Mon Sep 17 00:00:00 2001 From: Ollie Agent Date: Tue, 29 Sep 2026 23:01:44 +0200 Subject: [PATCH] session: make session/new get-or-create CreateEmpty now returns (session, created, error): if a session with the given name already exists it is returned untouched instead of erroring, and only a freshly created session receives the provided cwd/workflow/ variant. The o script no longer suppresses errors from session/new, so real failures surface while re-creating an existing session stays a no-op success. --- cmd/olliesrv/internal/fs/spec.go | 19 ++++++++++------ cmd/olliesrv/internal/session/registry.go | 27 ++++++++++++++--------- data/scripts/o | 5 +++-- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/cmd/olliesrv/internal/fs/spec.go b/cmd/olliesrv/internal/fs/spec.go index 56cb4d5..8661604 100644 --- a/cmd/olliesrv/internal/fs/spec.go +++ b/cmd/olliesrv/internal/fs/spec.go @@ -332,16 +332,21 @@ func buildTreeSpec(cfg *Config) virtfs.FsNodeDecl { if cwd == "" { return nil, fmt.Errorf("cwd is required") } - sess, err := session.CreateEmpty(name, remote, yolo) + sess, created, err := session.CreateEmpty(name, remote, yolo) if err != nil { return nil, err } - sess.SetCwd(cwd) - if workflow != "" { - sess.SetWorkflow(workflow) - } - if variant != "" { - sess.SetVariant(variant) + // Get-or-create: only a freshly created session takes the + // provided configuration. An existing session is returned + // untouched. + if created { + sess.SetCwd(cwd) + if workflow != "" { + sess.SetWorkflow(workflow) + } + if variant != "" { + sess.SetVariant(variant) + } } return []byte(sess.Name() + "\n"), nil }), diff --git a/cmd/olliesrv/internal/session/registry.go b/cmd/olliesrv/internal/session/registry.go index b7ae071..b9c51fd 100644 --- a/cmd/olliesrv/internal/session/registry.go +++ b/cmd/olliesrv/internal/session/registry.go @@ -284,7 +284,11 @@ func BuildAgentIndex(sess *Session) []byte { } // CreateEmpty creates an empty session (no agent) and registers it. -func CreateEmpty(name, remote string, yolo ...bool) (*Session, error) { +// CreateEmpty returns the session named `name`, creating it if it does not yet +// exist (get-or-create). created reports whether a new session was made; when +// false the returned session already existed and its configuration is left +// untouched — creation never mutates an existing session. +func CreateEmpty(name, remote string, yolo ...bool) (sess *Session, created bool, err error) { sessID := util.NewUUID() if name == "" { name = sessID @@ -293,16 +297,16 @@ func CreateEmpty(name, remote string, yolo ...bool) (*Session, error) { } } - // Early check (non-atomic, avoids expensive setup in the common case). + // Early path: return the existing session without building a tool server. mu.RLock() - _, exists := sessions[name] + existing := sessions[name] mu.RUnlock() - if exists { - return nil, fmt.Errorf("session already exists: %s", name) + if existing != nil { + return existing, false, nil } ctx, cancel := context.WithCancel(serverCtx) - sess := NewEmpty(sessID, ctx, cancel) + sess = NewEmpty(sessID, ctx, cancel) sess.SetName(name) sess.Remote = remote sess.Yolo = len(yolo) > 0 && yolo[0] @@ -317,7 +321,7 @@ func CreateEmpty(name, remote string, yolo ...bool) (*Session, error) { }) if err != nil { cancel() - return nil, fmt.Errorf("setup tool server: %w", err) + return nil, false, fmt.Errorf("setup tool server: %w", err) } sess.Proc = infra.Proc sess.Keeper = infra.Keeper @@ -328,19 +332,20 @@ func CreateEmpty(name, remote string, yolo ...bool) (*Session, error) { sess.StartBypassLoop(pkgBypassNotify) } - // Atomic check-and-insert to prevent races. + // Atomic check-and-insert. If another caller won the race, discard our + // freshly-built session and return theirs — still get-or-create. mu.Lock() - if _, exists := sessions[name]; exists { + if won := sessions[name]; won != nil { mu.Unlock() cancel() - return nil, fmt.Errorf("session already exists: %s", name) + return won, false, nil } sessions[name] = sess mu.Unlock() pkgLog.Info("created empty session %s (id=%s)", name, sessID) PublishEvent("session."+sessID+".new", name) - return sess, nil + return sess, true, nil } // NextUname generates a unique agent name (12 hex chars). diff --git a/data/scripts/o b/data/scripts/o index 46ccd08..a7ea4cf 100755 --- a/data/scripts/o +++ b/data/scripts/o @@ -281,8 +281,9 @@ cmd_new() { exit 1 fi local agent_cwd="${1:-$PWD}" - # Create session if it doesn't exist - printf 'name=%s\ncwd=%s\n' "$CTX_SESSION" "$agent_cwd" | ollie-9p write "session/new" 2>/dev/null || true + # session/new is get-or-create: creating an existing session is a no-op + # success, so real failures now surface instead of being suppressed. + printf 'name=%s\ncwd=%s\n' "$CTX_SESSION" "$agent_cwd" | ollie-9p write "session/new" # Create the agent printf 'name=%s\ncwd=%s\n' "$CTX_AGENT" "$agent_cwd" | ollie-9p write "session/$CTX_SESSION/agent/new" echo "created: $CTX_SESSION/$CTX_AGENT"