From 25de390478f769ee7e6e7925f30295977ec584a3 Mon Sep 17 00:00:00 2001 From: Harshit Singh Bhandari Date: Mon, 20 Jul 2026 00:57:04 +0530 Subject: [PATCH 1/2] fix(cli): make ao spawn --name required again Reverts only the --name relaxation from #2411, keeping that PR's agent catalog and auth preflight intact. Deliberate, readable sidebar names are load-bearing for dashboard UX (#2302); the prompt-derived fallback produced low-quality names (first prompt words truncated at 20 runes). - Restore client-side required check: reject missing --name (and keep the existing >20-rune cap), before any daemon call. - Remove resolveSpawnDisplayName/deriveDisplayNameFromPrompt. - Restore the --name help text and the non-omitempty displayName tag. - Tests: add spawn-without-name-fails coverage; pass --name in the resolution/preflight tests that exercised the derived path. Daemon stays as is: displayName remains optional at POST /sessions with the 20-char cap (the desktop new-task dialog depends on omitting it). Closes #2849 Co-Authored-By: Claude Opus 4.8 --- backend/internal/cli/spawn.go | 40 +++++------------------------ backend/internal/cli/spawn_test.go | 41 ++++++++++++++++++------------ 2 files changed, 32 insertions(+), 49 deletions(-) diff --git a/backend/internal/cli/spawn.go b/backend/internal/cli/spawn.go index 4445dde59a..9854d56b73 100644 --- a/backend/internal/cli/spawn.go +++ b/backend/internal/cli/spawn.go @@ -43,7 +43,7 @@ type spawnRequest struct { Harness string `json:"harness,omitempty"` Branch string `json:"branch,omitempty"` Prompt string `json:"prompt,omitempty"` - DisplayName string `json:"displayName,omitempty"` + DisplayName string `json:"displayName"` } type spawnResult struct { @@ -72,7 +72,11 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { if opts.noTakeover && opts.claimPR == "" { return usageError{fmt.Errorf("--no-takeover requires --claim-pr")} } - if explicitName := strings.TrimSpace(opts.name); utf8.RuneCountInString(explicitName) > maxDisplayNameLen { + name := strings.TrimSpace(opts.name) + if name == "" { + return usageError{fmt.Errorf("--name is required")} + } + if utf8.RuneCountInString(name) > maxDisplayNameLen { return usageError{fmt.Errorf("--name must be %d characters or fewer", maxDisplayNameLen)} } @@ -88,7 +92,6 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { } opts.harness = harness - name := resolveSpawnDisplayName(opts.name, opts.prompt) if !opts.skipAgentCheck { if err := ctx.preflightSpawnAgentAuth(cmd.Context(), cmd, opts.harness); err != nil { return err @@ -161,7 +164,7 @@ func newSpawnCommand(ctx *commandContext) *cobra.Command { f.StringVar(&opts.branch, "branch", "", "Branch for the session worktree (default: ao//root)") 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.name, "name", "", "Display name shown in the sidebar (default: derived from --prompt, max 20 characters)") + f.StringVar(&opts.name, "name", "", "Display name shown in the sidebar (required, max 20 characters)") f.StringVar(&opts.claimPR, "claim-pr", "", "Immediately claim an existing PR for the spawned session") f.BoolVar(&opts.noTakeover, "no-takeover", false, "Refuse if another active session owns the claimed PR (requires --claim-pr)") f.BoolVar(&opts.skipAgentCheck, "skip-agent-check", false, "Skip advisory agent catalog install/auth preflight before spawning") @@ -306,35 +309,6 @@ func resolveSpawnHarness(explicit string, project projectDetails) (string, error return "", usageError{fmt.Errorf("agent could not be resolved; pass --agent or configure `ao project set-config %s --worker-agent `", project.ID)} } -func resolveSpawnDisplayName(explicit, prompt string) string { - if name := strings.TrimSpace(explicit); name != "" { - return name - } - return deriveDisplayNameFromPrompt(prompt) -} - -func deriveDisplayNameFromPrompt(prompt string) string { - fields := strings.Fields(strings.TrimSpace(prompt)) - if len(fields) == 0 { - return "" - } - var b strings.Builder - for _, field := range fields { - next := strings.Trim(field, " \t\r\n.,;:!?()[]{}\"'") - if next == "" { - continue - } - if b.Len() > 0 { - next = " " + next - } - if utf8.RuneCountInString(b.String()+next) > maxDisplayNameLen { - break - } - b.WriteString(next) - } - return b.String() -} - func (c *commandContext) preflightSpawnAgentAuth(ctx context.Context, cmd *cobra.Command, agentID string) error { inv, err := c.fetchAgentInventory(ctx, true) if err != nil { diff --git a/backend/internal/cli/spawn_test.go b/backend/internal/cli/spawn_test.go index 486077f86e..f48c5a99b6 100644 --- a/backend/internal/cli/spawn_test.go +++ b/backend/internal/cli/spawn_test.go @@ -35,7 +35,7 @@ func TestSpawnCommand_MissingProjectContext(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--agent", "codex") + _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--agent", "codex", "--name", "worker") if err == nil { t.Fatal("expected an error when project context is missing") } @@ -157,6 +157,15 @@ func TestSpawnNoTakeoverRequiresClaimPR(t *testing.T) { } } +// TestSpawnCommand_RequiresName asserts `ao spawn` rejects a missing --name +// without contacting the daemon. +func TestSpawnCommand_RequiresName(t *testing.T) { + _, _, err := executeCLI(t, Deps{}, "spawn", "--project", "demo", "--agent", "codex") + if err == nil || ExitCode(err) != 2 || !strings.Contains(err.Error(), "--name is required") { + t.Fatalf("err=%v exit=%d, want --name is required", err, ExitCode(err)) + } +} + // TestSpawnCommand_RejectsOverlongName asserts `ao spawn` rejects a --name // longer than 20 characters without contacting the daemon. func TestSpawnCommand_RejectsOverlongName(t *testing.T) { @@ -191,14 +200,14 @@ func TestSpawnResolvesProjectFromEnvAndDefaultAgent(t *testing.T) { writeRunFileFor(t, cfg, srv) t.Setenv("AO_PROJECT_ID", "demo") - out, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--prompt", "Fix failing tests in auth") + out, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--prompt", "Fix failing tests in auth", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } if !strings.Contains(out, "spawned session demo-11") { t.Fatalf("output missing spawn: %s", out) } - if req.ProjectID != "demo" || req.Harness != "codex" || req.DisplayName != "Fix failing tests in" { + if req.ProjectID != "demo" || req.Harness != "codex" || req.DisplayName != "worker" { t.Fatalf("spawn request = %#v", req) } want := []string{"GET /api/v1/projects/demo", "POST /api/v1/agents/refresh", "POST /api/v1/sessions"} @@ -234,7 +243,7 @@ func TestSpawnResolvesProjectFromAOSessionID(t *testing.T) { writeRunFileFor(t, cfg, srv) t.Setenv("AO_SESSION_ID", "demo-1") - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--prompt", "Fix tests") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--prompt", "Fix tests", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -265,7 +274,7 @@ func TestSpawnAOSessionIDFailureRequiresProject(t *testing.T) { writeRunFileFor(t, cfg, srv) t.Setenv("AO_SESSION_ID", "missing") - _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--agent", "codex") + _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--agent", "codex", "--name", "worker") if err == nil || !strings.Contains(err.Error(), `project could not be resolved from AO_SESSION_ID "missing"; pass --project`) { t.Fatalf("err=%v, want AO_SESSION_ID project error", err) } @@ -313,7 +322,7 @@ func TestSpawnResolvesProjectFromCWD(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--prompt", "Fix tests") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--prompt", "Fix tests", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -348,7 +357,7 @@ func TestSpawnStaleUnauthorizedAgentRefreshesProbesThenAllows(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -390,7 +399,7 @@ func TestSpawnFreshUnauthorizedWarnsAndAllows(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -433,7 +442,7 @@ func TestSpawnUnavailableFreshProbeWarnsAndAllows(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -467,7 +476,7 @@ func TestSpawnUnsupportedAgentRefreshesThenBlocks(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "unknown") + _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "unknown", "--name", "worker") if err == nil || !strings.Contains(err.Error(), "agent \"unknown\" is not supported") { t.Fatalf("err=%v, want unsupported", err) } @@ -497,7 +506,7 @@ func TestSpawnNotInstalledAgentRefreshesThenBlocks(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err == nil || !strings.Contains(err.Error(), "agent \"codex\" needs install") { t.Fatalf("err=%v, want needs install", err) } @@ -533,7 +542,7 @@ func TestSpawnStaleNotInstalledFreshInstalledWarnsAndAllows(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -576,7 +585,7 @@ func TestSpawnUnavailableFreshProbeForNotInstalledWarnsAndAllows(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -613,7 +622,7 @@ func TestSpawnFreshProbeServerErrorBlocks(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, _, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err == nil || !strings.Contains(err.Error(), "probe failed (PROBE_FAILED) [request req-1]") { t.Fatalf("err=%v, want probe server error", err) } @@ -645,7 +654,7 @@ func TestSpawnSkipAgentCheckBypassesOnlyPreflight(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "unsupported", "--skip-agent-check") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "unsupported", "--skip-agent-check", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } @@ -680,7 +689,7 @@ func TestSpawnUnknownAuthRefreshesWarnsAndAllows(t *testing.T) { t.Cleanup(srv.Close) writeRunFileFor(t, cfg, srv) - _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex") + _, errOut, err := executeCLI(t, Deps{ProcessAlive: func(int) bool { return true }}, "spawn", "--project", "demo", "--agent", "codex", "--name", "worker") if err != nil { t.Fatalf("spawn failed: %v stderr=%s", err, errOut) } From 065058f1ab9cb3198ea6d6896b143f2bcc36336c Mon Sep 17 00:00:00 2001 From: Harshit Singh Bhandari Date: Mon, 20 Jul 2026 21:15:04 +0530 Subject: [PATCH 2/2] fix(orchestrator): include --name in spawn prompt examples Now that `ao spawn --name` is required again, the built-in orchestrator prompt must not instruct workers to run spawn without it. Add `--name "