From 6890bafa65a309b540cbe1562c39df137f202369 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 05:01:05 +0000 Subject: refactor(api): remove the natural-language elaborate REST endpoint (Phase 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chatbots now elaborate tasks in-conversation and submit them via the chatbot MCP server, so the POST /api/tasks/elaborate shell-out path is no longer needed. Removes the route, handleElaborateTask, and the task-elaborate-only helpers (buildElaboratePrompt, sanitizeElaboratedTask, readProjectContext, appendRawNarrative, elaborateWith{Claude,Local,Gemini}, and the elaboratedTask/ elaboratedAgent types) plus their dedicated tests. Shared helpers (extractJSON, claude/geminiBinaryPath, claude/geminiJSONResult, elaborateTimeout, the per-IP limiter) are retained — the story-elaborate endpoint and task-validate endpoint still use them. Stories themselves are untouched here; their removal stays in the Phase 8 cleanup pass. https://claude.ai/code/session_01SESwn7kQ7oP62trWw6pc39 --- internal/api/elaborate_test.go | 527 ----------------------------------------- 1 file changed, 527 deletions(-) delete mode 100644 internal/api/elaborate_test.go (limited to 'internal/api/elaborate_test.go') diff --git a/internal/api/elaborate_test.go b/internal/api/elaborate_test.go deleted file mode 100644 index 32cec3c..0000000 --- a/internal/api/elaborate_test.go +++ /dev/null @@ -1,527 +0,0 @@ -package api - -import ( - "bytes" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "strings" - "testing" - "time" -) - -// createFakeClaude writes a shell script to a temp dir that prints output and exits with the -// given code. Returns the script path. Used to mock the claude binary in elaborate tests. -func createFakeClaude(t *testing.T, output string, exitCode int) string { - t.Helper() - dir := t.TempDir() - outputFile := filepath.Join(dir, "output.json") - if err := os.WriteFile(outputFile, []byte(output), 0600); err != nil { - t.Fatal(err) - } - script := filepath.Join(dir, "claude") - content := fmt.Sprintf("#!/bin/sh\ncat %q\nexit %d\n", outputFile, exitCode) - if err := os.WriteFile(script, []byte(content), 0755); err != nil { - t.Fatal(err) - } - return script -} - -// hasTool is a test helper that reports whether name is in the tools slice. -func hasTool(tools []string, name string) bool { - for _, t := range tools { - if t == name { - return true - } - } - return false -} - -// --- sanitizeElaboratedTask unit tests --- - -func TestSanitize_AddsWriteWhenInstructionsMentionFileCreation(t *testing.T) { - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: "Create a new file called output.txt with the results.", - AllowedTools: []string{"Bash"}, - }, - } - sanitizeElaboratedTask(task) - if !hasTool(task.Agent.AllowedTools, "Write") { - t.Errorf("expected Write in allowed_tools, got %v", task.Agent.AllowedTools) - } -} - -func TestSanitize_AddsReadWhenEditIsPresent(t *testing.T) { - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: "Modify the configuration file.", - AllowedTools: []string{"Edit"}, - }, - } - sanitizeElaboratedTask(task) - if !hasTool(task.Agent.AllowedTools, "Read") { - t.Errorf("expected Read added alongside Edit, got %v", task.Agent.AllowedTools) - } -} - -func TestSanitize_NoDuplicateTools(t *testing.T) { - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: "Run go test ./...", - AllowedTools: []string{"Bash"}, - }, - } - sanitizeElaboratedTask(task) - count := 0 - for _, tool := range task.Agent.AllowedTools { - if tool == "Bash" { - count++ - } - } - if count != 1 { - t.Errorf("Bash duplicated in allowed_tools: %v", task.Agent.AllowedTools) - } -} - -func TestSanitize_AddsAcceptanceCriteriaWhenMissing(t *testing.T) { - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: "Do something useful with the codebase.", - AllowedTools: []string{"Bash"}, - }, - } - sanitizeElaboratedTask(task) - lower := strings.ToLower(task.Agent.Instructions) - if !strings.Contains(lower, "acceptance") && !strings.Contains(lower, "done when") { - t.Error("expected acceptance criteria section appended to instructions") - } -} - -func TestSanitize_NoopWhenAcceptanceCriteriaAlreadyPresent(t *testing.T) { - original := "Do something.\n\n## Acceptance Criteria\n- All tests pass." - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: original, - AllowedTools: []string{"Bash"}, - }, - } - sanitizeElaboratedTask(task) - if task.Agent.Instructions != original { - t.Errorf("instructions were modified when acceptance criteria were already present") - } -} - -func TestSanitize_AddsTDDReminderForCodingTaskWithoutTestMention(t *testing.T) { - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: "## Acceptance Criteria\nFix the bug.\n\nModify the handler to return 404 instead of 500.", - AllowedTools: []string{"Edit", "Read"}, - }, - } - sanitizeElaboratedTask(task) - lower := strings.ToLower(task.Agent.Instructions) - if !strings.Contains(lower, "tdd") && !strings.Contains(lower, "test") { - t.Error("expected TDD reminder for coding task without test mention") - } -} - -func TestSanitize_NoTDDReminderWhenTestsAlreadyMentioned(t *testing.T) { - original := "## Acceptance Criteria\nAll tests pass.\n\nEdit the file and run go test ./... to verify." - task := &elaboratedTask{ - Agent: elaboratedAgent{ - Instructions: original, - AllowedTools: []string{"Edit", "Read", "Bash"}, - }, - } - before := task.Agent.Instructions - sanitizeElaboratedTask(task) - // Should NOT add a second TDD block since tests are already mentioned. - // Count occurrences of "tdd" / "test" — just verify no double-append. - if strings.Count(strings.ToLower(task.Agent.Instructions), "tdd") > 1 { - t.Errorf("TDD block added twice; instructions:\n%s", task.Agent.Instructions) - } - _ = before -} - -func TestElaboratePrompt_RequiresAcceptanceCriteria(t *testing.T) { - prompt := buildElaboratePrompt("") - lower := strings.ToLower(prompt) - if !strings.Contains(lower, "acceptance criteria") { - t.Error("elaborate prompt should instruct the model to include acceptance criteria") - } -} - -func TestElaboratePrompt_RequiresAllRelevantTools(t *testing.T) { - prompt := buildElaboratePrompt("") - // Prompt must remind the model to include file-creating tools when needed. - if !strings.Contains(prompt, "Write") { - t.Error("elaborate prompt should mention the Write tool so models know to include it") - } -} - -func TestElaborateTask_SanitizationAppliedToResponse(t *testing.T) { - srv, _ := testServer(t) - - // Elaborator returns a task that needs Write (instructions say "create file") - // but does NOT include it in allowed_tools. - task := elaboratedTask{ - Name: "Generate report", - Description: "Creates a report file.", - Agent: elaboratedAgent{ - Type: "claude", - Model: "sonnet", - Instructions: "Create a new file called report.md with the analysis results.\n\n## Acceptance Criteria\n- report.md exists.", - MaxBudgetUSD: 0.5, - AllowedTools: []string{"Bash"}, // Write intentionally missing - }, - Timeout: "15m", - Priority: "normal", - Tags: []string{"report"}, - } - taskJSON, _ := json.Marshal(task) - wrapper := map[string]string{"result": string(taskJSON)} - wrapperJSON, _ := json.Marshal(wrapper) - - srv.elaborateCmdPath = createFakeClaude(t, string(wrapperJSON), 0) - - body := `{"prompt":"generate a report"}` - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status: want 200, got %d; body: %s", w.Code, w.Body.String()) - } - - var result elaboratedTask - if err := json.NewDecoder(w.Body).Decode(&result); err != nil { - t.Fatalf("failed to decode response: %v", err) - } - if !hasTool(result.Agent.AllowedTools, "Write") { - t.Errorf("expected Write in sanitized allowed_tools, got %v", result.Agent.AllowedTools) - } -} - -func TestElaboratePrompt_ContainsWorkDir(t *testing.T) { - prompt := buildElaboratePrompt("/some/custom/path") - if !strings.Contains(prompt, "/some/custom/path") { - t.Error("prompt should contain the provided workDir") - } - if strings.Contains(prompt, "/root/workspace/claudomator") { - t.Error("prompt should not hardcode /root/workspace/claudomator") - } -} - -func TestElaboratePrompt_EmptyWorkDir(t *testing.T) { - prompt := buildElaboratePrompt("") - if strings.Contains(prompt, "/root") { - t.Error("prompt should not reference /root when workDir is empty") - } -} - -func TestElaborateTask_Success(t *testing.T) { - srv, _ := testServer(t) - - // Build fake Claude output: {"result": ""} - task := elaboratedTask{ - Name: "Run Go tests with race detector", - Description: "Runs the Go test suite with -race flag and checks coverage.", - Agent: elaboratedAgent{ - Type: "claude", - Model: "sonnet", - Instructions: "Run go test -race ./... and report results.", - ProjectDir: "", - MaxBudgetUSD: 0.5, - AllowedTools: []string{"Bash"}, - }, - Timeout: "15m", - Priority: "normal", - Tags: []string{"testing", "ci"}, - } - taskJSON, err := json.Marshal(task) - if err != nil { - t.Fatal(err) - } - wrapper := map[string]string{"result": string(taskJSON)} - wrapperJSON, err := json.Marshal(wrapper) - if err != nil { - t.Fatal(err) - } - - srv.elaborateCmdPath = createFakeClaude(t, string(wrapperJSON), 0) - - body := `{"prompt":"run the Go test suite with race detector and fail if coverage < 80%"}` - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status: want 200, got %d; body: %s", w.Code, w.Body.String()) - } - - var result elaboratedTask - if err := json.NewDecoder(w.Body).Decode(&result); err != nil { - t.Fatalf("failed to decode response: %v", err) - } - if result.Name == "" { - t.Error("expected non-empty name") - } - if result.Agent.Instructions == "" { - t.Error("expected non-empty instructions") - } -} - -func TestElaborateTask_EmptyPrompt(t *testing.T) { - srv, _ := testServer(t) - - body := `{"prompt":""}` - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusBadRequest { - t.Fatalf("status: want 400, got %d; body: %s", w.Code, w.Body.String()) - } - - var resp map[string]string - json.NewDecoder(w.Body).Decode(&resp) - if resp["error"] == "" { - t.Error("expected error message in response") - } -} - -func TestElaborateTask_MarkdownFencedJSON(t *testing.T) { - srv, _ := testServer(t) - - // Build a valid task JSON but wrap it in markdown fences as haiku sometimes does. - task := elaboratedTask{ - Name: "Test task", - Description: "Does something.", - Agent: elaboratedAgent{ - Type: "claude", - Model: "sonnet", - Instructions: "Do the thing.", - MaxBudgetUSD: 0.5, - AllowedTools: []string{"Bash"}, - }, - Timeout: "15m", - Priority: "normal", - Tags: []string{"test"}, - } - taskJSON, _ := json.Marshal(task) - fenced := "```json\n" + string(taskJSON) + "\n```" - wrapper := map[string]string{"result": fenced} - wrapperJSON, _ := json.Marshal(wrapper) - - srv.elaborateCmdPath = createFakeClaude(t, string(wrapperJSON), 0) - - body := `{"prompt":"do something"}` - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status: want 200, got %d; body: %s", w.Code, w.Body.String()) - } - - var result elaboratedTask - if err := json.NewDecoder(w.Body).Decode(&result); err != nil { - t.Fatalf("failed to decode response: %v", err) - } - if result.Name != task.Name { - t.Errorf("name: want %q, got %q", task.Name, result.Name) - } -} - -func TestElaborateTask_InvalidJSONFromClaude(t *testing.T) { - srv, _ := testServer(t) - - // Fake Claude returns something that is not valid JSON. - srv.elaborateCmdPath = createFakeClaude(t, "not valid json at all", 0) - // Ensure Gemini fallback also fails so we get the expected 502. - srv.geminiBinPath = "/nonexistent/gemini" - - body := `{"prompt":"do something"}` - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusBadGateway { - t.Fatalf("status: want 502, got %d; body: %s", w.Code, w.Body.String()) - } - - var resp map[string]string - json.NewDecoder(w.Body).Decode(&resp) - if resp["error"] == "" { - t.Error("expected error message in response") - } -} - -func createFakeClaudeCapturingArgs(t *testing.T, output string, exitCode int, argsFile string) string { - t.Helper() - dir := t.TempDir() - outputFile := filepath.Join(dir, "output.json") - if err := os.WriteFile(outputFile, []byte(output), 0600); err != nil { - t.Fatal(err) - } - script := filepath.Join(dir, "claude") - // Use printf to handle arguments safely - content := fmt.Sprintf("#!/bin/sh\nprintf \"%%s\\n\" \"$@\" > %q\ncat %q\nexit %d\n", argsFile, outputFile, exitCode) - if err := os.WriteFile(script, []byte(content), 0755); err != nil { - t.Fatal(err) - } - return script -} - -func TestElaborateTask_WithProjectContext(t *testing.T) { - srv, _ := testServer(t) - - // Create a temporary workspace with CLAUDE.md and .agent/worklog.md - workDir := t.TempDir() - claudeContent := "Claude context info" - sessionContent := "Session state info" - if err := os.WriteFile(filepath.Join(workDir, "CLAUDE.md"), []byte(claudeContent), 0600); err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(filepath.Join(workDir, ".agent"), 0700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(workDir, ".agent", "worklog.md"), []byte(sessionContent), 0600); err != nil { - t.Fatal(err) - } - - // Capture arguments passed to claude - argsFile := filepath.Join(t.TempDir(), "args.txt") - - task := elaboratedTask{ - Name: "Task with context", - Agent: elaboratedAgent{ - Instructions: "Instructions", - }, - } - taskJSON, _ := json.Marshal(task) - wrapper := map[string]string{"result": string(taskJSON)} - wrapperJSON, _ := json.Marshal(wrapper) - - // Modified createFakeClaude to capture arguments - srv.elaborateCmdPath = createFakeClaudeCapturingArgs(t, string(wrapperJSON), 0, argsFile) - - body := fmt.Sprintf(`{"prompt":"do something", "project_dir":"%s"}`, workDir) - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status: want 200, got %d; body: %s", w.Code, w.Body.String()) - } - - // Check if captured arguments contain the context - capturedArgs, err := os.ReadFile(argsFile) - if err != nil { - t.Fatal(err) - } - argsStr := string(capturedArgs) - if !strings.Contains(argsStr, claudeContent) { - t.Errorf("expected arguments to contain CLAUDE.md content, got %s", argsStr) - } - if !strings.Contains(argsStr, sessionContent) { - t.Errorf("expected arguments to contain .agent/worklog.md content, got %s", argsStr) - } -} - -func TestElaborateTask_NoRawNarrativeWithoutExplicitProjectDir(t *testing.T) { - srv, _ := testServer(t) - // Point workDir at a temp dir so any accidental write is detectable. - srv.workDir = t.TempDir() - - task := elaboratedTask{ - Name: "Task", - Agent: elaboratedAgent{Instructions: "Instructions"}, - } - taskJSON, _ := json.Marshal(task) - wrapper := map[string]string{"result": string(taskJSON)} - wrapperJSON, _ := json.Marshal(wrapper) - srv.elaborateCmdPath = createFakeClaude(t, string(wrapperJSON), 0) - - // No project_dir in request body. - body := `{"prompt":"do something"}` - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status: want 200, got %d", w.Code) - } - - time.Sleep(30 * time.Millisecond) // let goroutine run if it was incorrectly triggered - narrativePath := filepath.Join(srv.workDir, "docs", "RAW_NARRATIVE.md") - if _, err := os.Stat(narrativePath); err == nil { - t.Errorf("RAW_NARRATIVE.md should NOT be written when project_dir is not provided by the user") - } -} - -func TestElaborateTask_AppendsRawNarrative(t *testing.T) { - srv, _ := testServer(t) - - workDir := t.TempDir() - prompt := "this is my raw request" - - task := elaboratedTask{ - Name: "Task", - Agent: elaboratedAgent{ - Instructions: "Instructions", - }, - } - taskJSON, _ := json.Marshal(task) - wrapper := map[string]string{"result": string(taskJSON)} - wrapperJSON, _ := json.Marshal(wrapper) - - srv.elaborateCmdPath = createFakeClaude(t, string(wrapperJSON), 0) - - body := fmt.Sprintf(`{"prompt":"%s", "project_dir":"%s"}`, prompt, workDir) - req := httptest.NewRequest("POST", "/api/tasks/elaborate", bytes.NewBufferString(body)) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status: want 200, got %d; body: %s", w.Code, w.Body.String()) - } - - // It runs in a goroutine, so wait a bit - path := filepath.Join(workDir, "docs", "RAW_NARRATIVE.md") - var data []byte - var err error - for i := 0; i < 10; i++ { - data, err = os.ReadFile(path) - if err == nil { - break - } - time.Sleep(10 * time.Millisecond) - } - - if err != nil { - t.Fatalf("failed to read RAW_NARRATIVE.md: %v", err) - } - if !strings.Contains(string(data), prompt) { - t.Errorf("expected RAW_NARRATIVE.md to contain prompt, got %s", string(data)) - } -} -- cgit v1.2.3