From 1cebf334234aae773f6e2ebb2b3ed3689a8e9bcd Mon Sep 17 00:00:00 2001 From: prode Date: Mon, 8 Jun 2026 13:18:44 -0300 Subject: [PATCH] feat(agent-skill-model-effort): add model/effort to skill & agent create Implements spec agent-skill-model-effort; tasks 1-7. Adds optional --model/--effort to `csdd skill create` and --effort to `csdd agent create` (model already existed). Effort is validated against the closed set {low,medium,high,xhigh,max} at the create boundary before any filesystem write, so an invalid value exits 1 and leaves no artifact; unset keys are omitted from frontmatter so the session config is inherited (output is byte-identical when no flag is given). Mirrors the params in the MCP shim (csdd_skill_create gains model+effort, csdd_agent_create gains effort) and adds model/effort-by-complexity guidance to the shipped setup commands. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cli/agent.go | 7 ++ internal/cli/agent_effort_test.go | 69 ++++++++++++++++ internal/cli/effort.go | 28 +++++++ internal/cli/effort_test.go | 37 +++++++++ internal/cli/skill.go | 11 +++ internal/cli/skill_effort_test.go | 82 +++++++++++++++++++ internal/templater/setup_guidance_test.go | 25 ++++++ .../templater/templates/agent/agent.md.tmpl | 3 + .../commands/csdd-setup-init.md.tmpl | 4 + .../commands/csdd-setup-update.md.tmpl | 4 +- .../templater/templates/skill/SKILL.md.tmpl | 6 ++ mcp-server/src/tools/agent.ts | 2 + mcp-server/src/tools/skill.ts | 4 + mcp-server/test/tools.test.ts | 15 ++++ 14 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 internal/cli/agent_effort_test.go create mode 100644 internal/cli/effort.go create mode 100644 internal/cli/effort_test.go create mode 100644 internal/cli/skill_effort_test.go create mode 100644 internal/templater/setup_guidance_test.go diff --git a/internal/cli/agent.go b/internal/cli/agent.go index 89bf512..58bb31d 100644 --- a/internal/cli/agent.go +++ b/internal/cli/agent.go @@ -45,6 +45,7 @@ type AgentCreateOptions struct { Description string Tools []string Model string + Effort string // optional; empty = omit (inherit session config) Title string Force bool } @@ -57,6 +58,7 @@ func agentCreate(args []string, templates embed.FS) int { fs.StringVar(&opts.Description, "description", "", "When the orchestrator should pick this agent.") fs.Var(&tools, "tools", "Tool name (repeatable). Default: Read, Grep.") fs.StringVar(&opts.Model, "model", "", "Optional model override (e.g., sonnet, opus, haiku).") + fs.StringVar(&opts.Effort, "effort", "", "Optional effort: low|medium|high|xhigh|max.") fs.StringVar(&opts.Title, "title", "", "Document title (default: derived from name).") addForce(fs, &opts.Force) positionals, err := parseFlags(fs, args) @@ -84,6 +86,10 @@ func AgentCreate(templates embed.FS, opts AgentCreateOptions) error { if err := workspace.KebabCheck(opts.Name, "agent"); err != nil { return err } + // Validate effort before writing any file, so an invalid value creates nothing. + if err := validateEffort(opts.Effort); err != nil { + return err + } r, err := workspace.Resolve(opts.Root) if err != nil { return err @@ -107,6 +113,7 @@ func AgentCreate(templates embed.FS, opts AgentCreateOptions) error { "Description": opts.Description, "Tools": toolsStr, "Model": opts.Model, + "Effort": opts.Effort, "Title": title, }) if err != nil { diff --git a/internal/cli/agent_effort_test.go b/internal/cli/agent_effort_test.go new file mode 100644 index 0000000..9033b9a --- /dev/null +++ b/internal/cli/agent_effort_test.go @@ -0,0 +1,69 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func readAgentMD(t *testing.T, root, name string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, ".claude/agents", name+".md")) + if err != nil { + t.Fatalf("read agent md: %v", err) + } + return string(data) +} + +func TestAgentCreateEffort(t *testing.T) { + dir := freshWorkspace(t) + code, _, errOut := run(t, "agent", "create", "effort-agent", + "--description", "d", "--effort", "high", "--root", dir) + if code != 0 { + t.Fatalf("agent create with effort failed (code=%d): %s", code, errOut) + } + body := readAgentMD(t, dir, "effort-agent") + if !strings.Contains(body, "effort: high") { + t.Errorf("agent missing effort line:\n%s", body) + } +} + +func TestAgentCreateModelAndEffort(t *testing.T) { + dir := freshWorkspace(t) + code, _, _ := run(t, "agent", "create", "both-agent", + "--description", "d", "--model", "sonnet", "--effort", "max", "--root", dir) + if code != 0 { + t.Fatalf("agent create with model+effort should succeed, got exit %d", code) + } + body := readAgentMD(t, dir, "both-agent") + for _, want := range []string{"model: sonnet", "effort: max"} { + if !strings.Contains(body, want) { + t.Errorf("agent missing %q:\n%s", want, body) + } + } +} + +func TestAgentCreateNoEffortNoLine(t *testing.T) { + dir := freshWorkspace(t) + _, _, _ = run(t, "agent", "create", "plain-agent", "--description", "d", "--root", dir) + body := readAgentMD(t, dir, "plain-agent") + if strings.Contains(body, "effort:") { + t.Errorf("agent without --effort must not carry an effort line:\n%s", body) + } + if strings.Contains(body, "model:") { + t.Errorf("agent without --model must not carry a model line:\n%s", body) + } +} + +func TestAgentCreateInvalidEffortRejected(t *testing.T) { + dir := freshWorkspace(t) + code, _, _ := run(t, "agent", "create", "bad-agent", + "--description", "d", "--effort", "HIGH", "--root", dir) + if code != 1 { + t.Errorf("invalid --effort should exit 1, got %d", code) + } + if _, err := os.Stat(filepath.Join(dir, ".claude/agents/bad-agent.md")); !os.IsNotExist(err) { + t.Errorf("invalid --effort must not create the agent file (err=%v)", err) + } +} diff --git a/internal/cli/effort.go b/internal/cli/effort.go new file mode 100644 index 0000000..544421f --- /dev/null +++ b/internal/cli/effort.go @@ -0,0 +1,28 @@ +package cli + +import ( + "fmt" + "strings" +) + +// effortLevels is the closed, case-sensitive, ordered set of effort levels +// Claude Code honors in agent/skill frontmatter. It is the single source of +// truth shared by the agent and skill create ops so the rule cannot drift +// between them — both the membership check and the error message derive from it. +var effortLevels = []string{"low", "medium", "high", "xhigh", "max"} + +// validateEffort reports whether an effort value may be written to frontmatter. +// The empty string is accepted and means "omit the key" (inherit the session +// configuration); any other value must appear in effortLevels exactly. It never +// touches the filesystem, so callers run it before creating any artifact. +func validateEffort(effort string) error { + if effort == "" { + return nil + } + for _, lvl := range effortLevels { + if effort == lvl { + return nil + } + } + return fmt.Errorf("invalid --effort %q: must be one of %s", effort, strings.Join(effortLevels, ", ")) +} diff --git a/internal/cli/effort_test.go b/internal/cli/effort_test.go new file mode 100644 index 0000000..2e0a094 --- /dev/null +++ b/internal/cli/effort_test.go @@ -0,0 +1,37 @@ +package cli + +import "testing" + +// TestValidateEffort pins the closed, case-sensitive effort set shared by the +// agent and skill create ops. The empty string is accepted (it means "omit the +// key"); anything else must be one of low|medium|high|xhigh|max exactly. +func TestValidateEffort(t *testing.T) { + cases := []struct { + name string + effort string + wantErr bool + }{ + {"empty means omit", "", false}, + {"low", "low", false}, + {"medium", "medium", false}, + {"high", "high", false}, + {"xhigh", "xhigh", false}, + {"max", "max", false}, + {"out of set", "highish", true}, + {"wrong case High", "High", true}, + {"wrong case LOW", "LOW", true}, + {"numeric", "5", true}, + {"whitespace", " low", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateEffort(tc.effort) + if tc.wantErr && err == nil { + t.Errorf("validateEffort(%q): expected error, got nil", tc.effort) + } + if !tc.wantErr && err != nil { + t.Errorf("validateEffort(%q): expected nil, got %v", tc.effort, err) + } + }) + } +} diff --git a/internal/cli/skill.go b/internal/cli/skill.go index 2eb1b2d..7be6a96 100644 --- a/internal/cli/skill.go +++ b/internal/cli/skill.go @@ -53,6 +53,8 @@ type SkillCreateOptions struct { Name string Description string Title string + Model string // optional; empty = omit (inherit session config) + Effort string // optional; empty = omit (inherit session config) } func skillCreate(args []string, templates embed.FS) int { @@ -61,6 +63,8 @@ func skillCreate(args []string, templates embed.FS) int { addRoot(fs, &opts.Root) fs.StringVar(&opts.Description, "description", "", "One-sentence activation trigger.") fs.StringVar(&opts.Title, "title", "", "Document title (default: derived from name).") + fs.StringVar(&opts.Model, "model", "", "Optional model override (e.g., sonnet, opus, haiku).") + fs.StringVar(&opts.Effort, "effort", "", "Optional effort: low|medium|high|xhigh|max.") positionals, err := parseFlags(fs, args) if err != nil { return failOnFlagParse(err) @@ -85,6 +89,11 @@ func SkillCreate(templates embed.FS, opts SkillCreateOptions) error { if err := workspace.KebabCheck(opts.Name, "skill"); err != nil { return err } + // Validate effort before creating any directory or file, so an invalid value + // leaves no partial artifact behind. + if err := validateEffort(opts.Effort); err != nil { + return err + } r, err := workspace.Resolve(opts.Root) if err != nil { return err @@ -107,6 +116,8 @@ func SkillCreate(templates embed.FS, opts SkillCreateOptions) error { "Name": opts.Name, "Description": opts.Description, "Title": title, + "Model": opts.Model, + "Effort": opts.Effort, }) if err != nil { return err diff --git a/internal/cli/skill_effort_test.go b/internal/cli/skill_effort_test.go new file mode 100644 index 0000000..33d9d29 --- /dev/null +++ b/internal/cli/skill_effort_test.go @@ -0,0 +1,82 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// readSkillMD returns the SKILL.md contents for a created skill. +func readSkillMD(t *testing.T, root, name string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, ".claude/skills", name, "SKILL.md")) + if err != nil { + t.Fatalf("read SKILL.md: %v", err) + } + return string(data) +} + +func TestSkillCreateModelEffort(t *testing.T) { + dir := freshWorkspace(t) + code, _, errOut := run(t, "skill", "create", "with-both", + "--description", "Demo trigger.", + "--model", "opus", "--effort", "low", "--root", dir) + if code != 0 { + t.Fatalf("skill create with model+effort failed (code=%d): %s", code, errOut) + } + body := readSkillMD(t, dir, "with-both") + for _, want := range []string{"model: opus", "effort: low"} { + if !strings.Contains(body, want) { + t.Errorf("SKILL.md missing %q:\n%s", want, body) + } + } +} + +func TestSkillCreateModelOnly(t *testing.T) { + dir := freshWorkspace(t) + // A free-form model string (not a known alias) is accepted — model is not validated. + code, _, _ := run(t, "skill", "create", "model-only", + "--description", "x.", "--model", "claude-opus-4-8", "--root", dir) + if code != 0 { + t.Fatalf("free-form model should be accepted, got exit %d", code) + } + body := readSkillMD(t, dir, "model-only") + if !strings.Contains(body, "model: claude-opus-4-8") { + t.Errorf("SKILL.md missing free-form model line:\n%s", body) + } + if strings.Contains(body, "effort:") { + t.Errorf("SKILL.md must not carry effort when --effort absent:\n%s", body) + } +} + +func TestSkillCreateNeitherIsByteIdentical(t *testing.T) { + dir := freshWorkspace(t) + _, _, _ = run(t, "skill", "create", "plain", "--description", "Plain trigger.", "--root", dir) + // Normalize line endings so the check is checkout-agnostic (templates ship CRLF). + body := strings.ReplaceAll(readSkillMD(t, dir, "plain"), "\r\n", "\n") + // With neither flag the frontmatter block (between the first two `---`) is + // exactly name + description — byte-identical to the pre-feature shape, with + // no model/effort keys. Compare the whole block, not just a prefix, so any + // stray key or whitespace change is caught. + const wantFrontmatter = "name: plain\ndescription: Plain trigger.\n" + parts := strings.SplitN(body, "---\n", 3) + if len(parts) < 3 || parts[0] != "" { + t.Fatalf("SKILL.md does not start with a `---` frontmatter block:\n%q", body) + } + if parts[1] != wantFrontmatter { + t.Errorf("frontmatter not byte-identical to pre-feature shape.\ngot:\n%q\nwant:\n%q", parts[1], wantFrontmatter) + } +} + +func TestSkillCreateInvalidEffortRejected(t *testing.T) { + dir := freshWorkspace(t) + code, _, _ := run(t, "skill", "create", "bad-effort", + "--description", "x.", "--effort", "highish", "--root", dir) + if code != 1 { + t.Errorf("invalid --effort should exit 1, got %d", code) + } + if _, err := os.Stat(filepath.Join(dir, ".claude/skills/bad-effort")); !os.IsNotExist(err) { + t.Errorf("invalid --effort must not create the skill directory (err=%v)", err) + } +} diff --git a/internal/templater/setup_guidance_test.go b/internal/templater/setup_guidance_test.go new file mode 100644 index 0000000..0f461cd --- /dev/null +++ b/internal/templater/setup_guidance_test.go @@ -0,0 +1,25 @@ +package templater + +import ( + "strings" + "testing" +) + +// TestSetupCommandsModelEffortGuidance covers requirements 5.1, 5.2, 5.3: both +// shipped setup commands must instruct picking model/effort by task complexity +// and omitting them (to inherit the session config) when neither is warranted. +func TestSetupCommandsModelEffortGuidance(t *testing.T) { + cmds, err := CommandFiles(FS) + if err != nil { + t.Fatal(err) + } + for _, name := range []string{"csdd-setup-init.md", "csdd-setup-update.md"} { + body := cmds[name] + lower := strings.ToLower(body) + for _, want := range []string{"complexity", "--effort", "--model", "inherit"} { + if !strings.Contains(lower, strings.ToLower(want)) { + t.Errorf("%s is missing model/effort-by-complexity guidance: no %q", name, want) + } + } + } +} diff --git a/internal/templater/templates/agent/agent.md.tmpl b/internal/templater/templates/agent/agent.md.tmpl index b17ab5b..6bf49c2 100644 --- a/internal/templater/templates/agent/agent.md.tmpl +++ b/internal/templater/templates/agent/agent.md.tmpl @@ -5,6 +5,9 @@ tools: {{.Tools}} {{- if .Model}} model: {{.Model}} {{- end}} +{{- if .Effort}} +effort: {{.Effort}} +{{- end}} --- # {{.Title}} diff --git a/internal/templater/templates/commands/csdd-setup-init.md.tmpl b/internal/templater/templates/commands/csdd-setup-init.md.tmpl index 7b79000..cd079bd 100644 --- a/internal/templater/templates/commands/csdd-setup-init.md.tmpl +++ b/internal/templater/templates/commands/csdd-setup-init.md.tmpl @@ -31,6 +31,10 @@ Work in this order, and **drive every change through the `csdd` CLI**: --tools Grep --tools Glob --tools Edit --tools Write --tools Bash`, then fill its body from the generic `implementer` plus this stack's test command, idioms, and gate. Add the skills the stack needs (e.g. a framework or `` test skill) with `csdd skill create` when one is missing. + When you scaffold a sub-agent or skill, choose its `--model` and `--effort` by the task's + complexity: a lower/cheaper model and lower effort for mechanical, well-bounded work; a stronger + model and higher effort for genuinely complex reasoning. When neither a specific model nor a + specific effort is warranted, omit both flags so the agent/skill inherits the session configuration. 5. **Validate.** Run `csdd steering validate` and `csdd skill validate` (and `csdd spec validate` / `status` if a spec exists); confirm the new agent with `csdd agent show ` (there is no `csdd agent validate`). Fix anything the validators report; if `csdd` exits non-zero, stop and diff --git a/internal/templater/templates/commands/csdd-setup-update.md.tmpl b/internal/templater/templates/commands/csdd-setup-update.md.tmpl index 7cc9fdf..f37b743 100644 --- a/internal/templater/templates/commands/csdd-setup-update.md.tmpl +++ b/internal/templater/templates/commands/csdd-setup-update.md.tmpl @@ -26,7 +26,9 @@ not a rebuild: change only what drifted. As with `/csdd-setup-init`, drive every 3. **Apply targeted adjustments** through the CLI only: add a missing skill (`csdd skill create`), add or refresh an agent (`csdd agent create`), or update steering (`csdd steering create`) for the drift you found. Do **not** recreate artifacts that are already correct. Never add secrets to - steering or specs. + steering or specs. When you create or refresh a sub-agent or skill, choose its `--model` and + `--effort` by the task's complexity (lower/cheaper for mechanical work, stronger/higher for complex + reasoning), and omit both flags when neither is warranted so it inherits the session configuration. 4. **Validate** with `csdd steering validate` and `csdd skill validate` (confirm agents with `csdd agent show` — there is no `csdd agent validate`); if `csdd` exits non-zero, stop and report rather than forcing past it. diff --git a/internal/templater/templates/skill/SKILL.md.tmpl b/internal/templater/templates/skill/SKILL.md.tmpl index f1cdbb7..3f3a4f7 100644 --- a/internal/templater/templates/skill/SKILL.md.tmpl +++ b/internal/templater/templates/skill/SKILL.md.tmpl @@ -1,6 +1,12 @@ --- name: {{.Name}} description: {{.Description}} +{{- if .Model}} +model: {{.Model}} +{{- end}} +{{- if .Effort}} +effort: {{.Effort}} +{{- end}} --- # {{.Title}} diff --git a/mcp-server/src/tools/agent.ts b/mcp-server/src/tools/agent.ts index c870b94..58aafa8 100644 --- a/mcp-server/src/tools/agent.ts +++ b/mcp-server/src/tools/agent.ts @@ -25,6 +25,7 @@ export const agentTools: ToolDef[] = [ .optional() .describe("Tool names to grant (e.g. Read, Grep, Bash, Edit). Repeatable."), model: z.string().optional().describe("Model override (e.g. sonnet, opus, haiku)."), + effort: z.string().optional().describe("Effort level: low|medium|high|xhigh|max. Omit to inherit the session config."), title: z.string().optional().describe("Document title (defaults to Title Case of name)."), force: forceField, root: rootField, @@ -36,6 +37,7 @@ export const agentTools: ToolDef[] = [ ...flag("--description", p.description), ...multi("--tools", p.tools), ...flag("--model", p.model), + ...flag("--effort", p.effort), ...flag("--title", p.title), ...bool("--force", p.force), ...rootArg(p), diff --git a/mcp-server/src/tools/skill.ts b/mcp-server/src/tools/skill.ts index 1c731b4..765f781 100644 --- a/mcp-server/src/tools/skill.ts +++ b/mcp-server/src/tools/skill.ts @@ -27,6 +27,8 @@ export const skillTools: ToolDef[] = [ name: skillName, description: z.string().describe("One-sentence activation trigger for the skill."), title: z.string().optional().describe("Document title (defaults to Title Case of name)."), + model: z.string().optional().describe("Model override (e.g. sonnet, opus, haiku). Omit to inherit the session config."), + effort: z.string().optional().describe("Effort level: low|medium|high|xhigh|max. Omit to inherit the session config."), root: rootField, }, toArgs: (p) => [ @@ -34,6 +36,8 @@ export const skillTools: ToolDef[] = [ "create", p.name, ...flag("--description", p.description), + ...flag("--model", p.model), + ...flag("--effort", p.effort), ...flag("--title", p.title), ...rootArg(p), ], diff --git a/mcp-server/test/tools.test.ts b/mcp-server/test/tools.test.ts index cdb34cc..657d5fe 100644 --- a/mcp-server/test/tools.test.ts +++ b/mcp-server/test/tools.test.ts @@ -72,6 +72,11 @@ const CASES: Array<[name: string, params: Record, expected: str // skill ["csdd_skill_create", { name: "s", description: "d" }, ["skill", "create", "s", "--description", "d"]], ["csdd_skill_create", { name: "s", description: "d", title: "S", root: "/p" }, ["skill", "create", "s", "--description", "d", "--title", "S", "--root", "/p"]], + [ + "csdd_skill_create", + { name: "s", description: "d", model: "opus", effort: "low" }, + ["skill", "create", "s", "--description", "d", "--model", "opus", "--effort", "low"], + ], ["csdd_skill_list", {}, ["skill", "list"]], ["csdd_skill_show", { name: "s" }, ["skill", "show", "s"]], ["csdd_skill_add_reference", { skill: "s", file: "r.md" }, ["skill", "add-reference", "s", "r.md"]], @@ -87,6 +92,16 @@ const CASES: Array<[name: string, params: Record, expected: str { name: "rev", description: "d", tools: ["Read", "Grep"], model: "opus", title: "Rev", force: true, root: "/p" }, ["agent", "create", "rev", "--description", "d", "--tools", "Read", "--tools", "Grep", "--model", "opus", "--title", "Rev", "--force", "--root", "/p"], ], + [ + "csdd_agent_create", + { name: "rev", description: "d", effort: "high" }, + ["agent", "create", "rev", "--description", "d", "--effort", "high"], + ], + [ + "csdd_agent_create", + { name: "rev", description: "d", model: "opus", effort: "max" }, + ["agent", "create", "rev", "--description", "d", "--model", "opus", "--effort", "max"], + ], ["csdd_agent_list", {}, ["agent", "list"]], ["csdd_agent_show", { name: "rev" }, ["agent", "show", "rev"]], ["csdd_agent_delete", { name: "rev", force: true }, ["agent", "delete", "rev", "--force"]],