From dca613546c28176b73cf4c887fc8183308d24daf Mon Sep 17 00:00:00 2001 From: yyovil Date: Tue, 2 Jun 2026 08:19:36 +0530 Subject: [PATCH 1/6] Add `ao spawn` and `ao project add`; resolve project repos for worktrees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make a registered project spawnable end-to-end from the CLI: - DB-backed RepoResolver: the daemon resolves a project's on-disk repo path from the projects table (replacing the empty StaticRepoResolver that failed every lookup), so a session's worktree is cut from the right repo. - session_manager defaults an empty spawn branch to ao/ — a fresh, unique branch per session, since gitworktree can't reuse a branch already checked out elsewhere (e.g. main). - `ao project add --path `: register a local git repo (POST /api/v1/projects). - `ao spawn --project [--harness] [--branch] [--prompt] [--issue]`: spawn a worker session (POST /api/v1/sessions); harness defaults to the daemon's AO_AGENT. - Shared postJSON daemon client (reads the run-file for the port, surfaces the API error envelope). Stacked on #65, which lands the agent-adapter + session-manager wiring this depends on. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/internal/cli/client.go | 80 +++++++++++++++++++ backend/internal/cli/project.go | 71 ++++++++++++++++ backend/internal/cli/root.go | 2 + backend/internal/cli/spawn.go | 76 ++++++++++++++++++ backend/internal/cli/spawn_test.go | 37 +++++++++ backend/internal/daemon/lifecycle_wiring.go | 33 +++++++- backend/internal/daemon/wiring_test.go | 28 +++++++ backend/internal/session_manager/manager.go | 9 ++- .../internal/session_manager/manager_test.go | 23 ++++++ 9 files changed, 354 insertions(+), 5 deletions(-) create mode 100644 backend/internal/cli/client.go create mode 100644 backend/internal/cli/project.go create mode 100644 backend/internal/cli/spawn.go create mode 100644 backend/internal/cli/spawn_test.go diff --git a/backend/internal/cli/client.go b/backend/internal/cli/client.go new file mode 100644 index 000000000..acf4147d3 --- /dev/null +++ b/backend/internal/cli/client.go @@ -0,0 +1,80 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/config" + "github.com/aoagents/agent-orchestrator/backend/internal/runfile" +) + +// commandTimeout bounds a mutating daemon call. Spawns do real work (git +// worktree add, zellij launch, hook install), so it is generous compared to the +// status probe timeout. +const commandTimeout = 2 * time.Minute + +// apiError is the subset of the daemon's JSON error envelope the CLI surfaces. +type apiError struct { + Message string `json:"message"` + Code string `json:"code"` +} + +// postJSON sends body as JSON to POST /api/v1/ on the running daemon and +// decodes a 2xx response into out (out may be nil). A non-2xx response becomes +// an error built from the API error envelope, and a missing daemon yields a +// clear "not running" message rather than a connection-refused dump. +func (c *commandContext) postJSON(ctx context.Context, path string, body, out any) error { + cfg, err := config.Load() + if err != nil { + return err + } + info, err := runfile.Read(cfg.RunFilePath) + if err != nil { + return err + } + if info == nil { + return fmt.Errorf("AO daemon is not running — start it with `ao start`") + } + + payload, err := json.Marshal(body) + if err != nil { + return err + } + url := fmt.Sprintf("http://%s:%d/api/v1/%s", config.LoopbackHost, info.Port, path) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(payload)) + if err != nil { + return err + } + req.Header.Set("Content-Type", "application/json") + + // A dedicated client: mutating calls need far more headroom than the 2s + // status-probe client in Deps. + resp, err := (&http.Client{Timeout: commandTimeout}).Do(req) + if err != nil { + return fmt.Errorf("call daemon: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + var e apiError + _ = json.NewDecoder(resp.Body).Decode(&e) + switch { + case e.Message != "" && e.Code != "": + return fmt.Errorf("%s (%s)", e.Message, e.Code) + case e.Message != "": + return fmt.Errorf("%s", e.Message) + default: + return fmt.Errorf("daemon returned HTTP %d", resp.StatusCode) + } + } + if out != nil { + if err := json.NewDecoder(resp.Body).Decode(out); err != nil { + return fmt.Errorf("decode response: %w", err) + } + } + return nil +} diff --git a/backend/internal/cli/project.go b/backend/internal/cli/project.go new file mode 100644 index 000000000..e25d29066 --- /dev/null +++ b/backend/internal/cli/project.go @@ -0,0 +1,71 @@ +package cli + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +type projectAddOptions struct { + path string + id string + name string +} + +// addProjectRequest mirrors the daemon's project AddInput body for +// POST /api/v1/projects. projectId and name are optional (pointers omit them). +type addProjectRequest struct { + Path string `json:"path"` + ProjectID *string `json:"projectId,omitempty"` + Name *string `json:"name,omitempty"` +} + +type projectResult struct { + Project struct { + ID string `json:"id"` + Path string `json:"path"` + } `json:"project"` +} + +func newProjectCommand(ctx *commandContext) *cobra.Command { + cmd := &cobra.Command{ + Use: "project", + Short: "Manage projects", + } + cmd.AddCommand(newProjectAddCommand(ctx)) + return cmd +} + +func newProjectAddCommand(ctx *commandContext) *cobra.Command { + var opts projectAddOptions + cmd := &cobra.Command{ + Use: "add", + Short: "Register a local git repo as a project", + Long: "Register a local git repo as a project so sessions can be spawned in it.\n\n" + + "The path must be an existing git repository on disk.", + Args: noArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if opts.path == "" { + return usageError{fmt.Errorf("--path is required")} + } + req := addProjectRequest{Path: opts.path} + if opts.id != "" { + req.ProjectID = &opts.id + } + if opts.name != "" { + req.Name = &opts.name + } + var res projectResult + if err := ctx.postJSON(cmd.Context(), "projects", req, &res); err != nil { + return err + } + _, err := fmt.Fprintf(cmd.OutOrStdout(), "registered project %s at %s\n", res.Project.ID, res.Project.Path) + return err + }, + } + f := cmd.Flags() + f.StringVar(&opts.path, "path", "", "Absolute path to the local git repo (required)") + f.StringVar(&opts.id, "id", "", "Project id (default: derived by the daemon from the path)") + f.StringVar(&opts.name, "name", "", "Display name") + return cmd +} diff --git a/backend/internal/cli/root.go b/backend/internal/cli/root.go index ce0157389..293d431fb 100644 --- a/backend/internal/cli/root.go +++ b/backend/internal/cli/root.go @@ -147,6 +147,8 @@ func NewRootCommand(deps Deps) *cobra.Command { root.AddCommand(newStopCommand(ctx)) root.AddCommand(newStatusCommand(ctx)) root.AddCommand(newDoctorCommand(ctx)) + root.AddCommand(newSpawnCommand(ctx)) + root.AddCommand(newProjectCommand(ctx)) root.AddCommand(newCompletionCommand()) root.AddCommand(newVersionCommand()) diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go new file mode 100644 index 000000000..4a054054f --- /dev/null +++ b/backend/internal/cli/spawn.go @@ -0,0 +1,76 @@ +package cli + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +type spawnOptions struct { + project string + harness string + branch string + prompt string + issue string + rules string +} + +// spawnRequest mirrors the daemon's SpawnSessionRequest body for +// POST /api/v1/sessions. The CLI keeps its own copy so it need not import httpd. +type spawnRequest struct { + ProjectID string `json:"projectId"` + IssueID string `json:"issueId,omitempty"` + Harness string `json:"harness,omitempty"` + Branch string `json:"branch,omitempty"` + Prompt string `json:"prompt,omitempty"` + AgentRules string `json:"agentRules,omitempty"` +} + +type spawnResult struct { + Session struct { + ID string `json:"id"` + Status string `json:"status"` + Branch string `json:"branch"` + } `json:"session"` +} + +func newSpawnCommand(ctx *commandContext) *cobra.Command { + var opts spawnOptions + cmd := &cobra.Command{ + Use: "spawn", + Short: "Spawn a worker agent session in a registered project", + Long: "Spawn a worker agent session in a registered project.\n\n" + + "The session runs the chosen agent (default: the daemon's AO_AGENT) in a\n" + + "fresh git worktree. Register the project first with `ao project add`.", + Args: noArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if opts.project == "" { + return usageError{fmt.Errorf("--project is required")} + } + req := spawnRequest{ + ProjectID: opts.project, + IssueID: opts.issue, + Harness: opts.harness, + Branch: opts.branch, + Prompt: opts.prompt, + AgentRules: opts.rules, + } + var res spawnResult + if err := ctx.postJSON(cmd.Context(), "sessions", req, &res); err != nil { + return err + } + _, err := fmt.Fprintf(cmd.OutOrStdout(), + "spawned session %s (%s) on branch %s\nattach with: zellij attach %s (or `zellij list-sessions`)\n", + res.Session.ID, res.Session.Status, res.Session.Branch, res.Session.ID) + return err + }, + } + f := cmd.Flags() + f.StringVar(&opts.project, "project", "", "Project id to spawn the session in (required)") + f.StringVar(&opts.harness, "harness", "", "Agent harness: claude-code, codex, … (default: the daemon's AO_AGENT)") + f.StringVar(&opts.branch, "branch", "", "Branch for the session worktree (default: ao/)") + f.StringVar(&opts.prompt, "prompt", "", "Initial prompt for the agent") + f.StringVar(&opts.issue, "issue", "", "Issue id to associate with the session") + f.StringVar(&opts.rules, "rules", "", "Agent rules appended to the prompt") + return cmd +} diff --git a/backend/internal/cli/spawn_test.go b/backend/internal/cli/spawn_test.go new file mode 100644 index 000000000..baca6e3ad --- /dev/null +++ b/backend/internal/cli/spawn_test.go @@ -0,0 +1,37 @@ +package cli + +import ( + "bytes" + "strings" + "testing" +) + +// TestSpawnCommand_RequiresProject asserts `ao spawn` rejects a missing +// --project before touching the network, so it fails fast without a daemon. +func TestSpawnCommand_RequiresProject(t *testing.T) { + var out, errb bytes.Buffer + root := NewRootCommand(Deps{Out: &out, Err: &errb}) + root.SetArgs([]string{"spawn"}) + err := root.Execute() + if err == nil { + t.Fatal("expected an error when --project is missing") + } + if !strings.Contains(err.Error(), "--project is required") { + t.Fatalf("error = %v, want it to mention --project is required", err) + } +} + +// TestProjectAddCommand_RequiresPath asserts `ao project add` rejects a missing +// --path before touching the network. +func TestProjectAddCommand_RequiresPath(t *testing.T) { + var out, errb bytes.Buffer + root := NewRootCommand(Deps{Out: &out, Err: &errb}) + root.SetArgs([]string{"project", "add"}) + err := root.Execute() + if err == nil { + t.Fatal("expected an error when --path is missing") + } + if !strings.Contains(err.Error(), "--path is required") { + t.Fatalf("error = %v, want it to mention --path is required", err) + } +} diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index 34685670f..3f56e01c0 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -63,10 +63,10 @@ func startSession(cfg config.Config, runtime ports.Runtime, store *sqlite.Store, // Per-session worktrees live under the data dir, so a single AO_DATA_DIR // override moves all durable per-user state together. ManagedRoot: filepath.Join(cfg.DataDir, "worktrees"), - // An empty resolver fails every project lookup with a clear - // "no repo configured for project" error until the projects table feeds - // repo paths in — better than silently misrouting spawns. - RepoResolver: gitworktree.StaticRepoResolver{}, + // Resolve each project's source repo from the projects table, so a + // session spawned for a registered project materialises its worktree off + // that repo. Unregistered projects fail loudly. + RepoResolver: projectRepoResolver{store: store}, }) if err != nil { return nil, fmt.Errorf("session workspace: %w", err) @@ -145,3 +145,28 @@ func buildAgentResolver(defaultAgent string, log *slog.Logger) (ports.AgentResol log.Info("built per-session agent resolver", "default", defaultAgent, "registered", ids) return resolver, nil } + +// projectRepoResolver resolves a project's on-disk repo path from the projects +// table so gitworktree can materialise per-session worktrees off it. It replaces +// the empty StaticRepoResolver the daemon used before (which failed every +// lookup), turning a registered project into a spawnable one. +type projectRepoResolver struct{ store *sqlite.Store } + +var _ gitworktree.RepoResolver = projectRepoResolver{} + +func (r projectRepoResolver) RepoPath(projectID domain.ProjectID) (string, error) { + rec, ok, err := r.store.GetProject(context.Background(), string(projectID)) + if err != nil { + return "", fmt.Errorf("look up project %q: %w", projectID, err) + } + if !ok { + return "", fmt.Errorf("no project registered with id %q — add one with `ao project add`", projectID) + } + if !rec.ArchivedAt.IsZero() { + return "", fmt.Errorf("project %q is archived", projectID) + } + if rec.Path == "" { + return "", fmt.Errorf("project %q has no repo path on record", projectID) + } + return rec.Path, nil +} diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index 33ba2839f..b7b242c74 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -132,3 +132,31 @@ func TestWiring_StartSessionBuildsSessionService(t *testing.T) { t.Fatal("startSession returned nil session service") } } + +// TestProjectRepoResolver_ResolvesRegisteredProject asserts the DB-backed repo +// resolver turns a registered project into its on-disk repo path (so spawns +// materialise a worktree), and fails loudly for an unregistered project. +func TestProjectRepoResolver_ResolvesRegisteredProject(t *testing.T) { + store, err := sqlite.Open(t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + + ctx := context.Background() + if err := store.UpsertProject(ctx, domain.ProjectRecord{ID: "mer", Path: "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/repo/mer", RegisteredAt: time.Now()}); err != nil { + t.Fatal(err) + } + + r := projectRepoResolver{store: store} + got, err := r.RepoPath("mer") + if err != nil { + t.Fatalf("RepoPath(mer): %v", err) + } + if got != "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/repo/mer" { + t.Fatalf("RepoPath(mer) = %q, want /repo/mer", got) + } + if _, err := r.RepoPath("nope"); err == nil { + t.Fatal("expected an error for an unregistered project") + } +} diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index d1cc1007a..577e5134f 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -101,7 +101,14 @@ func (m *Manager) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Sess } id := rec.ID - ws, err := m.workspace.Create(ctx, ports.WorkspaceConfig{ProjectID: cfg.ProjectID, SessionID: id, Branch: cfg.Branch}) + branch := cfg.Branch + if branch == "" { + // A fresh, unique branch per session: gitworktree can't add a worktree on + // a branch already checked out elsewhere (e.g. main), so default to one + // derived from the assigned session id. + branch = "ao/" + string(id) + } + ws, err := m.workspace.Create(ctx, ports.WorkspaceConfig{ProjectID: cfg.ProjectID, SessionID: id, Branch: branch}) if err != nil { m.markSpawnFailedTerminated(ctx, id) return domain.SessionRecord{}, fmt.Errorf("spawn %s: workspace: %w", id, err) diff --git a/backend/internal/session_manager/manager_test.go b/backend/internal/session_manager/manager_test.go index 74bf7bbcf..78900d473 100644 --- a/backend/internal/session_manager/manager_test.go +++ b/backend/internal/session_manager/manager_test.go @@ -245,3 +245,26 @@ func TestCleanup_ReclaimsTerminalWorkspaces(t *testing.T) { t.Fatal("live workspace must not be destroyed") } } + +func TestSpawn_DefaultsBranchFromSessionID(t *testing.T) { + m, st, _, _ := newManager() + s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker}) + if err != nil { + t.Fatal(err) + } + // An empty SpawnConfig.Branch defaults to a unique per-session branch. + if got := st.sessions[s.ID].Metadata.Branch; got != "ao/mer-1" { + t.Fatalf("default branch = %q, want ao/mer-1", got) + } +} + +func TestSpawn_KeepsExplicitBranch(t *testing.T) { + m, st, _, _ := newManager() + s, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker, Branch: "feature/x"}) + if err != nil { + t.Fatal(err) + } + if got := st.sessions[s.ID].Metadata.Branch; got != "feature/x" { + t.Fatalf("explicit branch = %q, want feature/x", got) + } +} From 5b242b4930a90a3c99cbe9991e6337f4a9cf1f0d Mon Sep 17 00:00:00 2001 From: yyovil Date: Tue, 2 Jun 2026 08:56:20 +0530 Subject: [PATCH 2/6] Address Copilot review on #77 - `ao spawn` no longer prints a branch the sessions API doesn't return (session metadata is json:"-"), so the output is no longer misleading. - Unregistered/archived/no-path projects now surface a 400 PROJECT_NOT_RESOLVABLE with an actionable message instead of a generic 500: a new sessionmanager.ErrProjectNotResolvable sentinel the resolver wraps and writeSessionError maps. - postJSON reuses the injected Deps.HTTPClient (cloned, with a longer timeout) instead of a fresh client, keeping HTTP behaviour stubbable. - postJSON treats a stale run-file (dead PID) as "not running" via ProcessAlive, matching its docstring. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/internal/cli/client.go | 16 +++++++++++----- backend/internal/cli/spawn.go | 5 ++--- backend/internal/daemon/lifecycle_wiring.go | 6 +++--- backend/internal/httpd/controllers/sessions.go | 2 ++ backend/internal/session_manager/manager.go | 3 +++ 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/backend/internal/cli/client.go b/backend/internal/cli/client.go index acf4147d3..628428044 100644 --- a/backend/internal/cli/client.go +++ b/backend/internal/cli/client.go @@ -25,8 +25,9 @@ type apiError struct { // postJSON sends body as JSON to POST /api/v1/ on the running daemon and // decodes a 2xx response into out (out may be nil). A non-2xx response becomes -// an error built from the API error envelope, and a missing daemon yields a -// clear "not running" message rather than a connection-refused dump. +// an error built from the API error envelope. A missing run-file or a stale one +// (dead PID) yields a clear "not running" message rather than a +// connection-refused dump. func (c *commandContext) postJSON(ctx context.Context, path string, body, out any) error { cfg, err := config.Load() if err != nil { @@ -39,6 +40,9 @@ func (c *commandContext) postJSON(ctx context.Context, path string, body, out an if info == nil { return fmt.Errorf("AO daemon is not running — start it with `ao start`") } + if !c.deps.ProcessAlive(info.PID) { + return fmt.Errorf("AO daemon is not running (stale run-file at %s) — start it with `ao start`", cfg.RunFilePath) + } payload, err := json.Marshal(body) if err != nil { @@ -51,9 +55,11 @@ func (c *commandContext) postJSON(ctx context.Context, path string, body, out an } req.Header.Set("Content-Type", "application/json") - // A dedicated client: mutating calls need far more headroom than the 2s - // status-probe client in Deps. - resp, err := (&http.Client{Timeout: commandTimeout}).Do(req) + // Reuse the injected client's transport (keeps it stubbable in tests) but + // give mutating calls far more headroom than the 2s status-probe timeout. + client := *c.deps.HTTPClient + client.Timeout = commandTimeout + resp, err := client.Do(req) if err != nil { return fmt.Errorf("call daemon: %w", err) } diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index 4a054054f..ba0fef621 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -30,7 +30,6 @@ type spawnResult struct { Session struct { ID string `json:"id"` Status string `json:"status"` - Branch string `json:"branch"` } `json:"session"` } @@ -60,8 +59,8 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { return err } _, err := fmt.Fprintf(cmd.OutOrStdout(), - "spawned session %s (%s) on branch %s\nattach with: zellij attach %s (or `zellij list-sessions`)\n", - res.Session.ID, res.Session.Status, res.Session.Branch, res.Session.ID) + "spawned session %s (%s)\nattach with: zellij attach %s (or `zellij list-sessions`)\n", + res.Session.ID, res.Session.Status, res.Session.ID) return err }, } diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index 3f56e01c0..51232299d 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -160,13 +160,13 @@ func (r projectRepoResolver) RepoPath(projectID domain.ProjectID) (string, error return "", fmt.Errorf("look up project %q: %w", projectID, err) } if !ok { - return "", fmt.Errorf("no project registered with id %q — add one with `ao project add`", projectID) + return "", fmt.Errorf("no project registered with id %q — add one with `ao project add`: %w", projectID, sessionmanager.ErrProjectNotResolvable) } if !rec.ArchivedAt.IsZero() { - return "", fmt.Errorf("project %q is archived", projectID) + return "", fmt.Errorf("project %q is archived: %w", projectID, sessionmanager.ErrProjectNotResolvable) } if rec.Path == "" { - return "", fmt.Errorf("project %q has no repo path on record", projectID) + return "", fmt.Errorf("project %q has no repo path on record: %w", projectID, sessionmanager.ErrProjectNotResolvable) } return rec.Path, nil } diff --git a/backend/internal/httpd/controllers/sessions.go b/backend/internal/httpd/controllers/sessions.go index c97f388ad..bc88fd721 100644 --- a/backend/internal/httpd/controllers/sessions.go +++ b/backend/internal/httpd/controllers/sessions.go @@ -253,6 +253,8 @@ func writeSessionError(w http.ResponseWriter, r *http.Request, err error) { envelope.WriteAPIError(w, r, http.StatusConflict, "conflict", "SESSION_NOT_RESTORABLE", "Session is not restorable", nil) case errors.Is(err, sessionmanager.ErrIncompleteHandle): envelope.WriteAPIError(w, r, http.StatusConflict, "conflict", "SESSION_INCOMPLETE_HANDLE", "Session is missing runtime or workspace handles", nil) + case errors.Is(err, sessionmanager.ErrProjectNotResolvable): + envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "PROJECT_NOT_RESOLVABLE", "Project is not registered or has no repo — register it with `ao project add`", nil) default: envelope.WriteAPIError(w, r, http.StatusInternalServerError, "internal", "SESSION_OPERATION_FAILED", "Session operation failed", nil) } diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index 577e5134f..fcf4277f7 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -17,6 +17,9 @@ var ( ErrNotFound = errors.New("session: not found") ErrNotRestorable = errors.New("session: not restorable (not terminal)") ErrIncompleteHandle = errors.New("session: incomplete teardown handle") + // ErrProjectNotResolvable means the spawn's project has no usable repo + // (unregistered, archived, or missing a path). The API maps it to a 400. + ErrProjectNotResolvable = errors.New("session: project repo not resolvable") ) // Env vars a spawned process reads to learn who it is. From 798a4e544cba1da0fc7abc83dd1a7291ed39b335 Mon Sep 17 00:00:00 2001 From: yyovil Date: Tue, 2 Jun 2026 09:06:41 +0530 Subject: [PATCH 3/6] Assert the project-not-resolvable sentinel in the resolver test Greptile review: harden TestProjectRepoResolver to verify the unregistered -project error wraps ErrProjectNotResolvable, so a future regression in the sentinel wrapping (which the HTTP 400 mapping relies on) is caught. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/internal/daemon/wiring_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index b7b242c74..bf2083b41 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -2,6 +2,7 @@ package daemon import ( "context" + "errors" "io" "log/slog" "sync" @@ -15,6 +16,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/lifecycle" "github.com/aoagents/agent-orchestrator/backend/internal/ports" + sessionmanager "github.com/aoagents/agent-orchestrator/backend/internal/session_manager" "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" ) @@ -156,7 +158,12 @@ func TestProjectRepoResolver_ResolvesRegisteredProject(t *testing.T) { if got != "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/repo/mer" { t.Fatalf("RepoPath(mer) = %q, want /repo/mer", got) } - if _, err := r.RepoPath("nope"); err == nil { + _, err = r.RepoPath("nope") + if err == nil { t.Fatal("expected an error for an unregistered project") } + // Guard the sentinel wrapping so the HTTP 400 mapping can't silently regress. + if !errors.Is(err, sessionmanager.ErrProjectNotResolvable) { + t.Fatalf("unregistered-project error should wrap ErrProjectNotResolvable, got %v", err) + } } From cb77df55f2fcecac0d4af939311df1ef6a0a8001 Mon Sep 17 00:00:00 2001 From: yyovil Date: Tue, 2 Jun 2026 10:15:47 +0530 Subject: [PATCH 4/6] Fix ao spawn 500 on long session ids (zellij socket-path overflow) Root cause: the daemon built the zellij runtime with an empty SocketDir, so zellij fell back to its $TMPDIR-based default (long on macOS). That left almost none of the ~103-byte unix-socket-path budget for the session name, so a long session id (e.g. "aoagents-agent-orchestrator-1", derived from a long project id) was rejected by zellij with "session name must be less than 0 characters". runtime.Create failed, the spawn 500'd, and the worktree was rolled back (leaving an orphan ao/ branch). - New zellij.DefaultSocketDir(): a short, stable per-user socket dir (/tmp/ao-zellij-); the daemon uses it (and MkdirAll's it). - ao spawn's attach hint now prefixes ZELLIJ_SOCKET_DIR so it stays copy-pasteable against the daemon's socket dir. - Regression test guards that the socket dir leaves >= 48 bytes for the session name within the 103-byte limit. Verified: ao spawn against a long-id project now succeeds (session live, worktree created) where it previously 500'd. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../adapters/runtime/zellij/zellij.go | 13 ++++++++++++ backend/internal/cli/spawn.go | 17 +++++++++++++--- backend/internal/daemon/daemon.go | 8 +++++++- backend/internal/daemon/wiring_test.go | 20 +++++++++++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/backend/internal/adapters/runtime/zellij/zellij.go b/backend/internal/adapters/runtime/zellij/zellij.go index f30a21ecf..391a72200 100644 --- a/backend/internal/adapters/runtime/zellij/zellij.go +++ b/backend/internal/adapters/runtime/zellij/zellij.go @@ -58,6 +58,19 @@ type Runtime struct { var _ ports.Runtime = (*Runtime)(nil) +// DefaultSocketDir returns a short, stable ZELLIJ_SOCKET_DIR for AO's daemon. +// zellij's own default lives under $TMPDIR (long on macOS), which leaves almost +// none of the ~103-byte unix-socket-path budget for the session name — a long +// session id then fails with "session name must be less than 0 characters". A +// short dir restores ample budget. Empty on Windows, where zellij is not used. +// Pure: callers that run zellij should MkdirAll the result. +func DefaultSocketDir() string { + if runtime.GOOS == "windows" { + return "" + } + return "/tmp/ao-zellij-" + strconv.Itoa(os.Getuid()) +} + type runner interface { Run(ctx context.Context, env []string, name string, args ...string) ([]byte, error) } diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index ba0fef621..ca1ba38d4 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -4,6 +4,8 @@ import ( "fmt" "github.com/spf13/cobra" + + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/runtime/zellij" ) type spawnOptions struct { @@ -58,9 +60,18 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { if err := ctx.postJSON(cmd.Context(), "sessions", req, &res); err != nil { return err } - _, err := fmt.Fprintf(cmd.OutOrStdout(), - "spawned session %s (%s)\nattach with: zellij attach %s (or `zellij list-sessions`)\n", - res.Session.ID, res.Session.Status, res.Session.ID) + out := cmd.OutOrStdout() + if _, err := fmt.Fprintf(out, "spawned session %s (%s)\n", res.Session.ID, res.Session.Status); err != nil { + return err + } + // The daemon runs zellij under a short, non-default socket dir (see + // zellij.DefaultSocketDir), so a plain `zellij attach` wouldn't find + // the session — prefix the env so the hint is copy-pasteable. + attach := fmt.Sprintf("zellij attach %s", res.Session.ID) + if dir := zellij.DefaultSocketDir(); dir != "" { + attach = fmt.Sprintf("ZELLIJ_SOCKET_DIR=%s %s", dir, attach) + } + _, err := fmt.Fprintf(out, "attach with: %s\n", attach) return err }, } diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 3b75e4fb0..597f26349 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -62,7 +62,13 @@ func Run() error { // liveness; the CDC broadcaster feeds the session-state channel. The manager // is handed to httpd, which mounts it at /mux. Raw PTY bytes never flow // through the CDC change_log — only session-state events do. - runtimeAdapter := zellij.New(zellij.Options{}) + // zellij's default socket dir is too long on macOS for long session ids + // (see zellij.DefaultSocketDir); use a short, stable one and ensure it exists. + zellijSocketDir := zellij.DefaultSocketDir() + if zellijSocketDir != "" { + _ = os.MkdirAll(zellijSocketDir, 0o700) + } + runtimeAdapter := zellij.New(zellij.Options{SocketDir: zellijSocketDir}) termMgr := terminal.NewManager(runtimeAdapter, cdcPipe.Broadcaster, log) defer termMgr.Close() diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index bf2083b41..d15f9deef 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -167,3 +167,23 @@ func TestProjectRepoResolver_ResolvesRegisteredProject(t *testing.T) { t.Fatalf("unregistered-project error should wrap ErrProjectNotResolvable, got %v", err) } } + +// TestDaemonZellijSocketDir_LeavesBudgetForSessionNames guards the fix for the +// zellij "session name must be less than 0 characters" spawn failure: the +// daemon's socket dir must be short enough that a max-length (48-char) session +// name still fits the ~103-byte unix-domain-socket-path budget. zellij's long +// $TMPDIR default (the bug) would fail this. +func TestDaemonZellijSocketDir_LeavesBudgetForSessionNames(t *testing.T) { + dir := zellij.DefaultSocketDir() + if dir == "" { + t.Skip("zellij not used on this platform") + } + const ( + unixSocketPathMax = 103 // sun_path budget zellij enforces on macOS + zellijOverhead = 24 // zellij's version subdir + separators (generous) + maxSessionName = 48 // zellijSessionName's cap + ) + if budget := unixSocketPathMax - len(dir) - zellijOverhead; budget < maxSessionName { + t.Fatalf("zellij socket dir %q too long: %d bytes left for the session name, need >= %d", dir, budget, maxSessionName) + } +} From 9a5e9405f30d0f927194507398286655e9db0871 Mon Sep 17 00:00:00 2001 From: harshitsinghbhandari <24b4506@iitb.ac.in> Date: Tue, 2 Jun 2026 17:27:37 +0530 Subject: [PATCH 5/6] test(cli): guard CLI/daemon DTO drift with an e2e round-trip The CLI keeps its own request structs (spawnRequest, addProjectRequest) separate from the daemon's canonical DTOs (controllers.SpawnSessionRequest, project.AddInput). Nothing verified the JSON field names agreed, so a renamed tag on either side would compile but break at runtime. Drive `ao spawn` and `ao project add` through the real httpd router and controllers (fakes only at the service layer) over a real loopback round trip via postJSON, asserting each field decodes into the right SpawnConfig/AddInput field. Runs in the normal test lane (no extra ports/processes). Co-Authored-By: Claude Opus 4.7 --- backend/internal/cli/dto_drift_e2e_test.go | 215 +++++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 backend/internal/cli/dto_drift_e2e_test.go diff --git a/backend/internal/cli/dto_drift_e2e_test.go b/backend/internal/cli/dto_drift_e2e_test.go new file mode 100644 index 000000000..07c10a656 --- /dev/null +++ b/backend/internal/cli/dto_drift_e2e_test.go @@ -0,0 +1,215 @@ +package cli + +// dto_drift_e2e_test.go is the DTO-drift guard for the `ao spawn` and +// `ao project add` commands. The CLI defines its OWN request structs +// (spawnRequest in spawn.go, addProjectRequest in project.go) that are separate +// copies of the daemon's canonical request DTOs (controllers.SpawnSessionRequest +// and project.AddInput). Nothing else verifies the two sides agree on JSON field +// names — a renamed `json:"..."` tag on either side compiles fine but silently +// breaks at runtime. +// +// This test stands up the REAL daemon HTTP router + REAL controllers (with fakes +// only BELOW the controller, at the service layer) and drives the actual CLI +// commands through the actual postJSON client over a real loopback HTTP round +// trip. If the CLI's JSON field names diverge from what the controllers decode, +// the captured values are wrong/empty and the subtests fail. +// +// (This lives in a separate file from the build-tagged e2e_test.go so it runs in +// the normal `go test ./...` lane — it binds no extra ports beyond httptest and +// spawns no processes.) + +import ( + "bytes" + "context" + "io" + "log/slog" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/config" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/controllers" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" + "github.com/aoagents/agent-orchestrator/backend/internal/runfile" + projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" + sessionsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/session" +) + +// fakeSessionService captures the ports.SpawnConfig the controller decodes from +// the CLI's request body. Every other method is a no-op so it satisfies the +// controllers.SessionService interface. +type fakeSessionService struct { + spawned ports.SpawnConfig +} + +var _ controllers.SessionService = (*fakeSessionService)(nil) + +func (f *fakeSessionService) List(context.Context, sessionsvc.ListFilter) ([]domain.Session, error) { + return nil, nil +} + +func (f *fakeSessionService) Spawn(_ context.Context, cfg ports.SpawnConfig) (domain.Session, error) { + f.spawned = cfg + return domain.Session{ + SessionRecord: domain.SessionRecord{ID: domain.SessionID(string(cfg.ProjectID) + "-1")}, + Status: domain.StatusIdle, + }, nil +} + +func (f *fakeSessionService) Get(context.Context, domain.SessionID) (domain.Session, error) { + return domain.Session{}, nil +} + +func (f *fakeSessionService) Restore(context.Context, domain.SessionID) (domain.Session, error) { + return domain.Session{}, nil +} + +func (f *fakeSessionService) Kill(context.Context, domain.SessionID) (bool, error) { + return false, nil +} + +func (f *fakeSessionService) Send(context.Context, domain.SessionID, string) error { + return nil +} + +// fakeProjectManager captures the project.AddInput the controller decodes from +// the CLI's request body. Every other method is a no-op so it satisfies the +// projectsvc.Manager interface. +type fakeProjectManager struct { + added projectsvc.AddInput +} + +var _ projectsvc.Manager = (*fakeProjectManager)(nil) + +func (f *fakeProjectManager) List(context.Context) ([]projectsvc.Summary, error) { + return nil, nil +} + +func (f *fakeProjectManager) Get(context.Context, domain.ProjectID) (projectsvc.GetResult, error) { + return projectsvc.GetResult{}, nil +} + +func (f *fakeProjectManager) Add(_ context.Context, in projectsvc.AddInput) (projectsvc.Project, error) { + f.added = in + id := domain.ProjectID("demo") + if in.ProjectID != nil { + id = domain.ProjectID(*in.ProjectID) + } + return projectsvc.Project{ID: id, Path: in.Path}, nil +} + +func (f *fakeProjectManager) Remove(context.Context, domain.ProjectID) (projectsvc.RemoveResult, error) { + return projectsvc.RemoveResult{}, nil +} + +// startDriftTestDaemon stands up the real router+controllers backed by the +// supplied fakes and points the CLI's run-file at it. The CLI discovers the +// server purely via AO_RUN_FILE + the run-file port, so this is a genuine +// loopback round trip through postJSON. +func startDriftTestDaemon(t *testing.T, sessions controllers.SessionService, projects projectsvc.Manager) { + t.Helper() + + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + router := httpd.NewRouterWithAPI(config.Config{}, log, nil, httpd.APIDeps{ + Sessions: sessions, + Projects: projects, + }) + srv := httptest.NewServer(router) + t.Cleanup(srv.Close) + + port := srv.Listener.Addr().(*net.TCPAddr).Port + + rfPath := filepath.Join(t.TempDir(), "running.json") + t.Setenv("AO_RUN_FILE", rfPath) + if err := runfile.Write(rfPath, runfile.Info{PID: os.Getpid(), Port: port, StartedAt: time.Now()}); err != nil { + t.Fatalf("write run-file: %v", err) + } +} + +func TestE2E_SpawnAndProjectAddDTORoundTrip(t *testing.T) { + t.Run("spawn", func(t *testing.T) { + sessions := &fakeSessionService{} + startDriftTestDaemon(t, sessions, &fakeProjectManager{}) + + var out bytes.Buffer + root := NewRootCommand(Deps{ + Out: &out, + Err: &out, + HTTPClient: &http.Client{}, + ProcessAlive: func(int) bool { return true }, + }) + root.SetArgs([]string{ + "spawn", + "--project", "mer", + "--harness", "codex", + "--branch", "feat/x", + "--prompt", "hi", + "--issue", "ISS-1", + }) + if err := root.Execute(); err != nil { + t.Fatalf("spawn execute: %v\noutput: %s", err, out.String()) + } + + got := sessions.spawned + if got.ProjectID != "mer" { + t.Errorf("ProjectID = %q, want %q (CLI json:\"projectId\" vs SpawnSessionRequest)", got.ProjectID, "mer") + } + if got.Harness != "codex" { + t.Errorf("Harness = %q, want %q", got.Harness, "codex") + } + if got.Branch != "feat/x" { + t.Errorf("Branch = %q, want %q", got.Branch, "feat/x") + } + if got.Prompt != "hi" { + t.Errorf("Prompt = %q, want %q", got.Prompt, "hi") + } + if got.IssueID != "ISS-1" { + t.Errorf("IssueID = %q, want %q", got.IssueID, "ISS-1") + } + if !bytes.Contains(out.Bytes(), []byte("spawned session")) { + t.Errorf("output missing %q; got: %s", "spawned session", out.String()) + } + }) + + t.Run("project add", func(t *testing.T) { + projects := &fakeProjectManager{} + startDriftTestDaemon(t, &fakeSessionService{}, projects) + + var out bytes.Buffer + root := NewRootCommand(Deps{ + Out: &out, + Err: &out, + HTTPClient: &http.Client{}, + ProcessAlive: func(int) bool { return true }, + }) + root.SetArgs([]string{ + "project", "add", + "--path", "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/repo/mer", + "--id", "demo", + "--name", "Demo", + }) + if err := root.Execute(); err != nil { + t.Fatalf("project add execute: %v\noutput: %s", err, out.String()) + } + + got := projects.added + if got.Path != "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/repo/mer" { + t.Errorf("Path = %q, want %q", got.Path, "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/repo/mer") + } + if got.ProjectID == nil || *got.ProjectID != "demo" { + t.Errorf("ProjectID = %v, want %q (CLI json:\"projectId\" vs AddInput)", got.ProjectID, "demo") + } + if got.Name == nil || *got.Name != "Demo" { + t.Errorf("Name = %v, want %q", got.Name, "Demo") + } + if !bytes.Contains(out.Bytes(), []byte("registered project")) { + t.Errorf("output missing %q; got: %s", "registered project", out.String()) + } + }) +} From 5deafde8e3378c91ad73d702cf27a72007635b6a Mon Sep 17 00:00:00 2001 From: harshitsinghbhandari <24b4506@iitb.ac.in> Date: Tue, 2 Jun 2026 17:48:59 +0530 Subject: [PATCH 6/6] fix(cli,daemon): address review findings on ao spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - spawn: print the sanitised zellij session name (zellij.SessionName) in the attach hint; a long/non-conforming session id is registered under a different name, so the raw id sent users to a missing session. - client: surface the daemon error envelope's requestId so a failed command can be correlated with daemon logs. - daemon: don't swallow the zellij socket-dir MkdirAll error — log it, since a failure otherwise surfaces later as an opaque socket-bind error on every spawn. - project: reject an embedded ".." in a project id up front; it passed the id pattern but yielded an invalid branch (ao/a..b-1) and an opaque 500 at spawn. Co-Authored-By: Claude Opus 4.7 --- .../adapters/runtime/zellij/zellij.go | 14 +++++++--- .../adapters/runtime/zellij/zellij_test.go | 25 +++++++++++++++++ backend/internal/cli/client.go | 27 +++++++++++++------ backend/internal/cli/client_test.go | 25 +++++++++++++++++ backend/internal/cli/spawn.go | 6 +++-- backend/internal/daemon/daemon.go | 6 ++++- backend/internal/service/project/service.go | 5 +++- .../internal/service/project/service_test.go | 5 ++++ 8 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 backend/internal/cli/client_test.go diff --git a/backend/internal/adapters/runtime/zellij/zellij.go b/backend/internal/adapters/runtime/zellij/zellij.go index 391a72200..536ad28a7 100644 --- a/backend/internal/adapters/runtime/zellij/zellij.go +++ b/backend/internal/adapters/runtime/zellij/zellij.go @@ -343,10 +343,18 @@ func zellijSessionName(id domain.SessionID) (string, error) { if raw == "" { return "", errors.New("zellij runtime: session id is required") } - if sessionIDPattern.MatchString(raw) && len(raw) <= 48 { - return raw, nil + return SessionName(raw), nil +} + +// SessionName returns the zellij session name the runtime registers for a given +// session id — applying the same sanitisation Create does. Callers that print an +// attach hint (e.g. `ao spawn`) must use this rather than the raw id, since a +// long or non-conforming id maps to a different, sanitised session name. +func SessionName(id string) string { + if sessionIDPattern.MatchString(id) && len(id) <= 48 { + return id } - return sanitizedSessionName(raw), nil + return sanitizedSessionName(id) } func sanitizedSessionName(raw string) string { diff --git a/backend/internal/adapters/runtime/zellij/zellij_test.go b/backend/internal/adapters/runtime/zellij/zellij_test.go index c5ef9b1c1..22e2e5092 100644 --- a/backend/internal/adapters/runtime/zellij/zellij_test.go +++ b/backend/internal/adapters/runtime/zellij/zellij_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/ports" ) @@ -59,6 +60,30 @@ func TestZellijSessionNameSanitizesIssueRefs(t *testing.T) { } } +// SessionName must return the exact name Create registers a session under, so +// callers that print an attach hint (e.g. `ao spawn`) reference the real +// session. A short, conforming id passes through; a long one is sanitised to a +// different name — printing the raw id there would send users to a missing +// session. +func TestSessionNameMatchesCreateNaming(t *testing.T) { + short := "myproj-1" + if got := SessionName(short); got != short { + t.Fatalf("SessionName(%q) = %q, want it unchanged", short, got) + } + + long := domain.SessionID(strings.Repeat("x", 60) + "-1") + viaCreate, err := zellijSessionName(long) + if err != nil { + t.Fatalf("zellijSessionName: %v", err) + } + if got := SessionName(string(long)); got != viaCreate { + t.Fatalf("SessionName = %q, but Create uses %q", got, viaCreate) + } + if SessionName(string(long)) == string(long) { + t.Fatal("expected a long id to be sanitised to a different name") + } +} + func TestValidateSessionAndPaneID(t *testing.T) { for _, id := range []string{"sess-1", "S_2", "abc123"} { if err := validateSessionID(id); err != nil { diff --git a/backend/internal/cli/client.go b/backend/internal/cli/client.go index 628428044..e6de41fe6 100644 --- a/backend/internal/cli/client.go +++ b/backend/internal/cli/client.go @@ -18,9 +18,24 @@ import ( const commandTimeout = 2 * time.Minute // apiError is the subset of the daemon's JSON error envelope the CLI surfaces. +// RequestID is surfaced so a failed command can be correlated with daemon logs. type apiError struct { - Message string `json:"message"` - Code string `json:"code"` + Message string `json:"message"` + Code string `json:"code"` + RequestID string `json:"requestId"` +} + +// String renders the envelope for the user: " () [request ]", +// omitting whichever parts the daemon left empty. +func (e apiError) String() string { + msg := e.Message + if e.Code != "" { + msg = fmt.Sprintf("%s (%s)", msg, e.Code) + } + if e.RequestID != "" { + msg = fmt.Sprintf("%s [request %s]", msg, e.RequestID) + } + return msg } // postJSON sends body as JSON to POST /api/v1/ on the running daemon and @@ -68,14 +83,10 @@ func (c *commandContext) postJSON(ctx context.Context, path string, body, out an if resp.StatusCode < 200 || resp.StatusCode >= 300 { var e apiError _ = json.NewDecoder(resp.Body).Decode(&e) - switch { - case e.Message != "" && e.Code != "": - return fmt.Errorf("%s (%s)", e.Message, e.Code) - case e.Message != "": - return fmt.Errorf("%s", e.Message) - default: + if e.Message == "" { return fmt.Errorf("daemon returned HTTP %d", resp.StatusCode) } + return fmt.Errorf("%s", e.String()) } if out != nil { if err := json.NewDecoder(resp.Body).Decode(out); err != nil { diff --git a/backend/internal/cli/client_test.go b/backend/internal/cli/client_test.go new file mode 100644 index 000000000..eb07dee34 --- /dev/null +++ b/backend/internal/cli/client_test.go @@ -0,0 +1,25 @@ +package cli + +import "testing" + +// TestAPIErrorString covers how the CLI renders the daemon's error envelope, +// including the requestId it now surfaces for log correlation. +func TestAPIErrorString(t *testing.T) { + cases := []struct { + name string + in apiError + want string + }{ + {"message only", apiError{Message: "boom"}, "boom"}, + {"message and code", apiError{Message: "boom", Code: "X"}, "boom (X)"}, + {"with request id", apiError{Message: "boom", Code: "X", RequestID: "req-1"}, "boom (X) [request req-1]"}, + {"message and request id", apiError{Message: "boom", RequestID: "req-1"}, "boom [request req-1]"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.in.String(); got != tc.want { + t.Fatalf("String() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index ca1ba38d4..534fff78f 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -66,8 +66,10 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { } // The daemon runs zellij under a short, non-default socket dir (see // zellij.DefaultSocketDir), so a plain `zellij attach` wouldn't find - // the session — prefix the env so the hint is copy-pasteable. - attach := fmt.Sprintf("zellij attach %s", res.Session.ID) + // the session — prefix the env so the hint is copy-pasteable. Use the + // sanitised name zellij actually registers (zellij.SessionName): a long + // session id maps to a different name than the raw id. + attach := fmt.Sprintf("zellij attach %s", zellij.SessionName(res.Session.ID)) if dir := zellij.DefaultSocketDir(); dir != "" { attach = fmt.Sprintf("ZELLIJ_SOCKET_DIR=%s %s", dir, attach) } diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 597f26349..d38fa4ea1 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -66,7 +66,11 @@ func Run() error { // (see zellij.DefaultSocketDir); use a short, stable one and ensure it exists. zellijSocketDir := zellij.DefaultSocketDir() if zellijSocketDir != "" { - _ = os.MkdirAll(zellijSocketDir, 0o700) + if err := os.MkdirAll(zellijSocketDir, 0o700); err != nil { + // Don't abort startup, but surface it: every spawn's zellij session + // would otherwise fail later with an opaque socket-bind error. + log.Warn("could not create zellij socket dir; spawns may fail", "dir", zellijSocketDir, "error", err) + } } runtimeAdapter := zellij.New(zellij.Options{SocketDir: zellijSocketDir}) termMgr := terminal.NewManager(runtimeAdapter, cdcPipe.Broadcaster, log) diff --git a/backend/internal/service/project/service.go b/backend/internal/service/project/service.go index 0eb213abe..89cf5a10b 100644 --- a/backend/internal/service/project/service.go +++ b/backend/internal/service/project/service.go @@ -228,7 +228,10 @@ var projectIDPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) func validateProjectID(id domain.ProjectID) error { raw := string(id) - if raw == "" || raw == "." || raw == ".." || strings.ContainsAny(raw, `/\`) || !projectIDPattern.MatchString(raw) { + // Reject any "." run: a "." prefix fails the pattern, but an embedded ".." + // (e.g. "a..b") passes it yet yields a branch like "ao/a..b-1" that git's + // check-ref-format rejects — surfacing as an opaque 500 at spawn time. + if raw == "" || raw == "." || strings.Contains(raw, "..") || strings.ContainsAny(raw, `/\`) || !projectIDPattern.MatchString(raw) { return badRequest("INVALID_PROJECT_ID", "Project id failed storage-path validation", nil) } return nil diff --git a/backend/internal/service/project/service_test.go b/backend/internal/service/project/service_test.go index d8ae22ec5..03dedfad8 100644 --- a/backend/internal/service/project/service_test.go +++ b/backend/internal/service/project/service_test.go @@ -125,6 +125,11 @@ func TestManager_AddValidationAndConflicts(t *testing.T) { _, err = m.Add(ctx, project.AddInput{Path: t.TempDir()}) // exists but not a git repo wantCode(t, err, "NOT_A_GIT_REPO") + // An embedded ".." passes the id pattern but would yield an invalid git + // branch (ao/a..b-1) at spawn time; reject it up front as a clear 400. + _, err = m.Add(ctx, project.AddInput{Path: gitRepo(t), ProjectID: ptr("a..b")}) + wantCode(t, err, "INVALID_PROJECT_ID") + repoA, repoB := gitRepo(t), gitRepo(t) if _, err := m.Add(ctx, project.AddInput{Path: repoA, ProjectID: ptr("shared")}); err != nil { t.Fatalf("seed add: %v", err)