Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 35 additions & 9 deletions backend/internal/adapters/workspace/gitworktree/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (w *Workspace) Create(ctx context.Context, cfg ports.WorkspaceConfig) (port
if err := w.validateBranch(ctx, repo, cfg.Branch); err != nil {
return ports.WorkspaceInfo{}, err
}
path, err := w.managedPath(cfg.ProjectID, cfg.SessionID)
path, err := w.managedPath(cfg)
if err != nil {
return ports.WorkspaceInfo{}, err
}
Expand Down Expand Up @@ -189,7 +189,7 @@ func (w *Workspace) Restore(ctx context.Context, cfg ports.WorkspaceConfig) (por
if err != nil {
return ports.WorkspaceInfo{}, err
}
path, err := w.managedPath(cfg.ProjectID, cfg.SessionID)
path, err := w.managedPath(cfg)
if err != nil {
return ports.WorkspaceInfo{}, err
}
Expand Down Expand Up @@ -383,11 +383,18 @@ func validateConfig(cfg ports.WorkspaceConfig) error {
if err := validatePathComponent("project id", string(cfg.ProjectID)); err != nil {
return err
}
if cfg.SessionID == "" {
return errors.New("gitworktree: session id is required")
}
if err := validatePathComponent("session id", string(cfg.SessionID)); err != nil {
return err
if cfg.Kind == domain.KindOrchestrator {
prefix := resolvedSessionPrefix(cfg)
if err := validatePathComponent("session prefix", prefix); err != nil {
return err
}
} else {
if cfg.SessionID == "" {
return errors.New("gitworktree: session id is required")
}
if err := validatePathComponent("session id", string(cfg.SessionID)); err != nil {
return err
}
}
if cfg.Branch == "" {
return errors.New("gitworktree: branch is required")
Expand All @@ -410,11 +417,30 @@ func validatePathComponent(name, value string) error {
return nil
}

func (w *Workspace) managedPath(project domain.ProjectID, session domain.SessionID) (string, error) {
path := filepath.Join(w.managedRoot, string(project), string(session))
func (w *Workspace) managedPath(cfg ports.WorkspaceConfig) (string, error) {
var path string
if cfg.Kind == domain.KindOrchestrator {
prefix := resolvedSessionPrefix(cfg)
path = filepath.Join(w.managedRoot, string(cfg.ProjectID), "orchestrator", prefix+"-orchestrator")
} else {
path = filepath.Join(w.managedRoot, string(cfg.ProjectID), string(cfg.SessionID))
}
return w.validateManagedPath(path)
}

// resolvedSessionPrefix returns cfg.SessionPrefix when set, otherwise the first
// 12 characters of the project ID (matching the display-prefix convention).
func resolvedSessionPrefix(cfg ports.WorkspaceConfig) string {
if p := strings.TrimSpace(cfg.SessionPrefix); p != "" {
return p
}
id := string(cfg.ProjectID)
if len(id) <= 12 {
return id
}
return id[:12]
}

func (w *Workspace) validateManagedPath(path string) (string, error) {
if path == "" {
return "", fmt.Errorf("%w: empty path", ErrUnsafePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"
"testing"

"github.com/aoagents/agent-orchestrator/backend/internal/domain"
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
)

Expand Down Expand Up @@ -101,7 +102,7 @@ func TestManagedPathSafety(t *testing.T) {
if err != nil {
t.Fatalf("new: %v", err)
}
path, err := ws.managedPath("proj", "sess")
path, err := ws.managedPath(ports.WorkspaceConfig{ProjectID: "proj", SessionID: "sess"})
if err != nil {
t.Fatalf("managed path: %v", err)
}
Expand All @@ -116,6 +117,63 @@ func TestManagedPathSafety(t *testing.T) {
}
}

func TestOrchestratorManagedPath(t *testing.T) {
root := t.TempDir()
ws, err := New(Options{ManagedRoot: root, RepoResolver: StaticRepoResolver{"proj": root}})
if err != nil {
t.Fatalf("new: %v", err)
}

t.Run("explicit prefix", func(t *testing.T) {
cfg := ports.WorkspaceConfig{
ProjectID: "proj",
SessionID: "proj-1",
Kind: domain.KindOrchestrator,
SessionPrefix: "ao-agents",
}
path, err := ws.managedPath(cfg)
if err != nil {
t.Fatalf("managed path: %v", err)
}
want := filepath.Join(ws.managedRoot, "proj", "orchestrator", "ao-agents-orchestrator")
if path != want {
t.Fatalf("path = %q, want %q", path, want)
}
})

t.Run("prefix derived from project id", func(t *testing.T) {
cfg := ports.WorkspaceConfig{
ProjectID: "longprojectid123",
SessionID: "longprojectid123-1",
Kind: domain.KindOrchestrator,
}
path, err := ws.managedPath(cfg)
if err != nil {
t.Fatalf("managed path: %v", err)
}
want := filepath.Join(ws.managedRoot, "longprojectid123", "orchestrator", "longprojecti-orchestrator")
if path != want {
t.Fatalf("path = %q, want %q", path, want)
}
})

t.Run("short project id used as prefix", func(t *testing.T) {
cfg := ports.WorkspaceConfig{
ProjectID: "proj",
SessionID: "proj-1",
Kind: domain.KindOrchestrator,
}
path, err := ws.managedPath(cfg)
if err != nil {
t.Fatalf("managed path: %v", err)
}
want := filepath.Join(ws.managedRoot, "proj", "orchestrator", "proj-orchestrator")
if path != want {
t.Fatalf("path = %q, want %q", path, want)
}
})
}

// TestValidateConfigRejectsPathEscapingIDs covers review item RB: filepath.Join
// in managedPath cleans `..` segments before validateManagedPath sees them, so a
// session id of "../other" would stay inside managedRoot while jumping projects.
Expand Down
6 changes: 5 additions & 1 deletion backend/internal/ports/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ var (
type WorkspaceConfig struct {
ProjectID domain.ProjectID
SessionID domain.SessionID
Branch string
Kind domain.SessionKind
// SessionPrefix is the human-readable project prefix used to name the
// orchestrator worktree. Defaults to a truncation of ProjectID when empty.
SessionPrefix string
Branch string
// BaseBranch is the per-project default branch new session branches are
// created from. Empty falls back to the workspace adapter's own default.
BaseBranch string
Expand Down
38 changes: 29 additions & 9 deletions backend/internal/session_manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,12 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess
branch = "ao/" + string(id)
}
ws, err := m.workspace.Create(ctx, ports.WorkspaceConfig{
ProjectID: cfg.ProjectID,
SessionID: id,
Branch: branch,
BaseBranch: project.Config.WithDefaults().DefaultBranch,
ProjectID: cfg.ProjectID,
SessionID: id,
Kind: cfg.Kind,
SessionPrefix: sessionPrefix(project),
Branch: branch,
BaseBranch: project.Config.WithDefaults().DefaultBranch,
})
if err != nil {
// Nothing observable exists yet — no worktree, no runtime — so the seed
Expand Down Expand Up @@ -307,6 +309,18 @@ func roleOverride(kind domain.SessionKind, cfg domain.ProjectConfig) domain.Role
return cfg.Worker
}

// sessionPrefix returns the display prefix for a project: the explicit
// SessionPrefix when set, otherwise the first 12 characters of the project ID.
func sessionPrefix(project domain.ProjectRecord) string {
if p := strings.TrimSpace(project.Config.SessionPrefix); p != "" {
return p
}
if len(project.ID) <= 12 {
return project.ID
}
return project.ID[:12]
}

// markSpawnFailedTerminated best-effort parks an orphaned spawn as terminated.
// A phantom half-spawned row is worse than a terminal one; we only delete the
// row when nothing observable has landed yet (seed state) via rollbackSpawn or
Expand Down Expand Up @@ -418,7 +432,17 @@ func (m *Manager) Restore(ctx context.Context, id domain.SessionID) (domain.Sess
return domain.SessionRecord{}, fmt.Errorf("restore %s: nothing to resume from", id)
}

ws, err := m.workspace.Restore(ctx, ports.WorkspaceConfig{ProjectID: rec.ProjectID, SessionID: id, Branch: meta.Branch})
project, err := m.loadProject(ctx, rec.ProjectID)
if err != nil {
return domain.SessionRecord{}, fmt.Errorf("restore %s: %w", id, err)
}
ws, err := m.workspace.Restore(ctx, ports.WorkspaceConfig{
ProjectID: rec.ProjectID,
SessionID: id,
Kind: rec.Kind,
SessionPrefix: sessionPrefix(project),
Branch: meta.Branch,
})
if err != nil {
return domain.SessionRecord{}, fmt.Errorf("restore %s: workspace: %w", id, err)
}
Expand All @@ -429,10 +453,6 @@ func (m *Manager) Restore(ctx context.Context, id domain.SessionID) (domain.Sess
if err := m.prepareWorkspace(ctx, agent, id, ws.Path); err != nil {
return domain.SessionRecord{}, fmt.Errorf("restore %s: %w", id, err)
}
project, err := m.loadProject(ctx, rec.ProjectID)
if err != nil {
return domain.SessionRecord{}, fmt.Errorf("restore %s: %w", id, err)
}
// Restore re-applies the project's resolved agent config so a configured
// model/permissions carry across a restore, matching fresh spawn.
argv, err := restoreArgv(ctx, agent, id, ws.Path, meta, effectiveAgentConfig(rec.Kind, project.Config))
Expand Down
Loading