feat(workspace): name orchestrator worktree orchestrator/{prefix}-orchestrator (#191)
* feat(workspace): name orchestrator worktree orchestrator/{prefix}-orchestrator
Orchestrator sessions now get a dedicated worktree path under
orchestrator/{prefix}-orchestrator within the project directory,
matching the pattern described in issue #184.
Workers retain the existing {sessionID} naming. The session prefix
falls back to the first 12 chars of the project ID when no explicit
SessionPrefix is configured.
Closes #184
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(lint): remove unnecessary string conversion in sessionPrefix
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(lint): drop redundant string() casts — project.ID is already string
This commit is contained in:
parent
a5d034ae1e
commit
53ca81286a
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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")
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import (
|
|||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/aoagents/agent-orchestrator/backend/internal/domain"
|
||||
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
|
||||
)
|
||||
|
||||
|
|
@ -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)
|
||||
}
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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)
|
||||
}
|
||||
|
|
@ -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))
|
||||
|
|
|
|||
Loading…
Reference in New Issue