feat(add-agent): opt-in --custom flag for non-catalog role names

cmd/add_agent.go

@@ -20,6 +20,7 @@ import (
 var newAddAgentRunner = func() iexec.Runner { return &iexec.DefaultRunner{} }
 
 var addAgentList bool
+var addAgentCustom bool
 
 var addAgentCmd = &cobra.Command{
 	Use:     "add-agent <name>",
@@ -38,8 +39,14 @@ The agent name must be a known role or a member of a numbered family
 Known roles: %s.
 Numbered families: eng1..N, qa1..N (e.g. eng7, qa10).
 
+For roles outside the catalog (e.g. "marketing", "designer", "dba"), pass
+--custom to opt out of the strict catalog gate. Custom names must match
+^[a-z][a-z0-9-]{0,31}$. Custom roles receive sensible defaults: Autonomous
+permission, no src submodule, no playbooks.
+
 Restart initech (or run 'initech' in a new session) to activate the new agent.`, knownRoleNames())
 	addAgentCmd.Flags().BoolVarP(&addAgentList, "list", "l", false, "List all agents and their install status")
+	addAgentCmd.Flags().BoolVar(&addAgentCustom, "custom", false, "Allow a custom role name outside the catalog (must match ^[a-z][a-z0-9-]{0,31}$)")
 	addAgentCmd.ValidArgsFunction = completeAddAgent
 	rootCmd.AddCommand(addAgentCmd)
 }
@@ -85,7 +92,12 @@ func runAddAgent(cmd *cobra.Command, args []string) error {
 	runner := newAddAgentRunner()
 
 	if !roles.IsValidRoleName(roleName) {
-		return fmt.Errorf("unknown agent %q. Known agents: %s. Numbered families also accepted: eng1..N, qa1..N", roleName, knownRoleNames())
+		if !addAgentCustom {
+			return fmt.Errorf("unknown agent %q. Known agents: %s. Numbered families also accepted: eng1..N, qa1..N. Pass --custom to add a role outside the catalog", roleName, knownRoleNames())
+		}
+		if !roles.IsValidCustomRoleName(roleName) {
+			return fmt.Errorf("invalid custom role name %q: must match ^[a-z][a-z0-9-]{0,31}$ (lowercase letter start, then up to 31 lowercase letters, digits, or hyphens)", roleName)
+		}
 	}
 
 	wd, err := os.Getwd()

cmd/add_agent_test.go

@@ -23,6 +23,117 @@ func TestRunAddAgent_UnknownRole(t *testing.T) {
 	if !strings.Contains(err.Error(), "unknown agent") {
 		t.Errorf("error = %q, want to contain 'unknown agent'", err.Error())
 	}
+	if !strings.Contains(err.Error(), "--custom") {
+		t.Errorf("error = %q, want hint mentioning --custom", err.Error())
+	}
+}
+
+// TestRunAddAgent_CustomFlag_AllowsArbitraryName proves the opt-in path:
+// names outside the catalog and numbered families (e.g. "marketing") are
+// accepted when --custom is passed. Regression for the gap where the package
+// claimed an open set ("unknown role names are valid and receive sensible
+// defaults") but the CLI gate rejected every non-catalog name.
+func TestRunAddAgent_CustomFlag_AllowsArbitraryName(t *testing.T) {
+	root := t.TempDir()
+	writeTestConfig(t, root, []string{"pm"})
+	if err := os.MkdirAll(filepath.Join(root, "pm"), 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	chdirTemp(t, root)
+
+	origRunner := newAddAgentRunner
+	newAddAgentRunner = func() iexec.Runner { return &iexec.FakeRunner{} }
+	t.Cleanup(func() { newAddAgentRunner = origRunner })
+
+	addAgentCustom = true
+	t.Cleanup(func() { addAgentCustom = false })
+
+	var buf bytes.Buffer
+	addAgentCmd.SetOut(&buf)
+	t.Cleanup(func() { addAgentCmd.SetOut(nil) })
+
+	if err := runAddAgent(addAgentCmd, []string{"marketing"}); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if _, err := os.Stat(filepath.Join(root, "marketing")); err != nil {
+		t.Error("marketing/ directory not created")
+	}
+
+	updated, err := config.Load(filepath.Join(root, "initech.yaml"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	found := false
+	for _, r := range updated.Roles {
+		if r == "marketing" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("marketing not added to config roles: %v", updated.Roles)
+	}
+}
+
+// TestRunAddAgent_CustomFlag_RejectsBadIdentifier verifies the opt-in still
+// enforces the safe identifier pattern. Uppercase, leading digits, and other
+// shell- or filesystem-hostile inputs must reject with a clear pattern error.
+func TestRunAddAgent_CustomFlag_RejectsBadIdentifier(t *testing.T) {
+	root := t.TempDir()
+	writeTestConfig(t, root, []string{"pm"})
+	chdirTemp(t, root)
+
+	addAgentCustom = true
+	t.Cleanup(func() { addAgentCustom = false })
+
+	cases := []string{
+		"Marketing",   // uppercase
+		"1marketing",  // leads with digit
+		"market.ing",  // dot
+		"market ing",  // space
+		"data_eng",    // underscore
+		"",            // empty
+	}
+
+	for _, name := range cases {
+		t.Run(name, func(t *testing.T) {
+			// Empty triggers cobra's "agent name required" branch instead
+			// of the validator. Skip the empty case here so this test
+			// stays focused on the pattern gate.
+			if name == "" {
+				return
+			}
+			err := runAddAgent(addAgentCmd, []string{name})
+			if err == nil {
+				t.Fatalf("expected error for invalid custom name %q", name)
+			}
+			if !strings.Contains(err.Error(), "invalid custom role name") {
+				t.Errorf("error = %q, want to mention 'invalid custom role name'", err.Error())
+			}
+		})
+	}
+}
+
+// TestRunAddAgent_WithoutCustomFlag_StillRejectsUnknown locks in the
+// existing behavior: --custom is strictly opt-in. The default closed-set
+// gate remains for typo protection.
+func TestRunAddAgent_WithoutCustomFlag_StillRejectsUnknown(t *testing.T) {
+	root := t.TempDir()
+	writeTestConfig(t, root, []string{"pm"})
+	chdirTemp(t, root)
+
+	// Ensure custom is off (other tests set it; defend against ordering).
+	addAgentCustom = false
+
+	err := runAddAgent(addAgentCmd, []string{"marketing"})
+	if err == nil {
+		t.Fatal("expected error for unknown role without --custom")
+	}
+	if !strings.Contains(err.Error(), "unknown agent") {
+		t.Errorf("error = %q, want 'unknown agent'", err.Error())
+	}
 }
 
 func TestRunAddAgent_AlreadyExists(t *testing.T) {

internal/roles/catalog.go

@@ -20,6 +20,13 @@ import (
 // (CLI gate) and LookupRole (default selection for unlisted family members).
 var numberedRoleRe = regexp.MustCompile(`^(qa|eng)\d+$`)
 
+// customRoleNameRe matches the safe identifier pattern for opt-in custom role
+// names. Lowercase letter start, then up to 31 more lowercase letters, digits,
+// or hyphens. Hyphens (not underscores) to keep names URL- and directory-shape
+// friendly and consistent with the catalog's own naming. Used by
+// IsValidCustomRoleName; not used by IsValidRoleName.
+var customRoleNameRe = regexp.MustCompile(`^[a-z][a-z0-9-]{0,31}$`)
+
 // PermissionTier controls whether an agent runs with --dangerously-skip-permissions.
 type PermissionTier int
 
@@ -98,21 +105,38 @@ func LookupRole(name string) RoleDef {
 }
 
 // IsValidRoleName reports whether name is acceptable as a role name for CLI
-// commands like 'initech hire' / 'initech add-agent'. Two paths to acceptance:
+// commands like 'initech hire' / 'initech add-agent' without any opt-in flag.
+// Two paths to acceptance:
 //  - exact match against the Catalog (covers the curated role set), or
 //  - match against the numbered family pattern qa\d+ / eng\d+ (covers
 //    arbitrary scaling like qa10, eng7, qa007).
 //
 // Custom non-numbered names (e.g. "designer", "dba") are deliberately rejected
-// at the CLI to preserve typo protection. Operators wanting truly custom roles
-// must add them to the Catalog or design a separate opt-in (out of scope here).
+// here to preserve typo protection. The opt-in path is IsValidCustomRoleName,
+// gated at the CLI by an explicit --custom flag.
 func IsValidRoleName(name string) bool {
 	if _, ok := Catalog[name]; ok {
 		return true
 	}
 	return numberedRoleRe.MatchString(name)
 }
 
+// IsValidCustomRoleName reports whether name is acceptable as an opt-in custom
+// role name. The check is strictly a pattern check: lowercase identifier,
+// 1-32 characters, starting with a letter, with alphanumerics and hyphens
+// allowed thereafter (^[a-z][a-z0-9-]{0,31}$).
+//
+// Callers should reach for this only after IsValidRoleName has returned false
+// and the operator has explicitly opted in (e.g. via --custom). Catalog and
+// numbered-family names also satisfy this pattern; callers can safely check
+// IsValidRoleName first and treat IsValidCustomRoleName as the fallback gate.
+//
+// The pattern keeps custom names URL- and directory-friendly and consistent
+// with the catalog's own naming. Underscores are intentionally excluded.
+func IsValidCustomRoleName(name string) bool {
+	return customRoleNameRe.MatchString(name)
+}
+
 // RoleFamily groups roles that share notification and lifecycle semantics.
 // Used by initech deliver to pick the right announce template per caller, and
 // by status-transition logic to decide which lifecycle move (if any) to make.

internal/roles/catalog_test.go

@@ -161,6 +161,79 @@ func TestIsValidRoleName(t *testing.T) {
 	}
 }
 
+// TestIsValidCustomRoleName_AcceptsAndRejects covers the opt-in custom-role
+// gate. The function is a pure pattern check — lowercase letter start, then up
+// to 31 lowercase letters, digits, or hyphens. Catalog and numbered-family
+// names also satisfy the pattern; the catalog clash is handled by callers
+// that consult IsValidRoleName first.
+//
+// Regression for the missing opt-in scoped out by IsValidRoleName's prior doc
+// comment: operators legitimately want custom names like "marketing" and
+// "designer" but the CLI rejected every non-catalog name.
+func TestIsValidCustomRoleName_AcceptsAndRejects(t *testing.T) {
+	tests := []struct {
+		name string
+		want bool
+	}{
+		// Legitimate custom names operators want to use.
+		{"marketing", true},
+		{"designer", true},
+		{"dba", true},
+		{"content", true},
+		{"demandgen", true},
+		{"leadops", true},
+		{"events", true},
+		{"community", true},
+		{"data-eng", true},
+		{"lead-gen", true},
+
+		// Catalog and numbered-family names also satisfy the pattern.
+		// Callers check IsValidRoleName first; this is fine.
+		{"super", true},
+		{"pm", true},
+		{"eng1", true},
+		{"qa10", true},
+
+		// Empty rejects.
+		{"", false},
+
+		// Must start with a lowercase letter.
+		{"1marketing", false},
+		{"-marketing", false},
+		{"9designer", false},
+
+		// Uppercase rejects (consistency with IsValidRoleName).
+		{"Marketing", false},
+		{"DESIGNER", false},
+		{"Data-Eng", false},
+
+		// Underscores rejected by design (hyphens only).
+		{"data_eng", false},
+		{"lead_gen", false},
+
+		// Other punctuation and whitespace rejected.
+		{"marketing!", false},
+		{"market.ing", false},
+		{"market ing", false},
+		{"marketing/", false},
+		{"marketing\n", false},
+		{" marketing", false},
+		{"marketing ", false},
+
+		// Max length 32 characters: the boundary cases.
+		{"abcdefghijklmnopqrstuvwxyz012345", true},  // 32 chars
+		{"abcdefghijklmnopqrstuvwxyz0123456", false}, // 33 chars
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := IsValidCustomRoleName(tt.name)
+			if got != tt.want {
+				t.Errorf("IsValidCustomRoleName(%q) = %v, want %v", tt.name, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestLookupRole_NumberedFamily(t *testing.T) {
 	t.Run("qa10 inherits qa1/qa2 defaults", func(t *testing.T) {
 		def := LookupRole("qa10")