From b41536467d7eee30b9fe3179ca1c8a0e15aeab51 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Wed, 1 Apr 2026 22:11:46 +0800 Subject: [PATCH] test(server): add integration tests for comment trigger logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-to-end tests through the full HTTP router + real database: TestCommentTriggerOnComment (6 subtests): - Top-level comment without mentions → triggers agent - Top-level comment mentioning only others → suppresses trigger - Top-level comment mentioning assignee → triggers agent - Reply to agent thread without mentions → triggers agent - Reply to member thread without mentions → suppresses trigger (Bohan's bug) - Reply to member thread mentioning assignee → triggers agent TestCommentTriggerOnAssignNoStatusGate: - Assigning agent to in_progress issue → triggers (no todo restriction) TestCommentTriggerOnMentionNoStatusGate: - @mentioning agent on done issue → triggers (no status gate) TestCommentTriggerCoalescing: - Rapid-fire comments → only 1 task created (dedup) --- .../comment_trigger_integration_test.go | 332 ++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 server/cmd/server/comment_trigger_integration_test.go diff --git a/server/cmd/server/comment_trigger_integration_test.go b/server/cmd/server/comment_trigger_integration_test.go new file mode 100644 index 00000000..a353ea70 --- /dev/null +++ b/server/cmd/server/comment_trigger_integration_test.go @@ -0,0 +1,332 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" +) + +// authRequestWithAgent makes an authenticated request with X-Agent-ID header, +// causing the server to resolve the actor as an agent instead of a member. +func authRequestWithAgent(t *testing.T, method, path string, body any, agentID string) *http.Response { + t.Helper() + resp := authRequest(t, method, path, body) + // We can't add headers after authRequest, so we build it manually: + resp.Body.Close() + + var bodyReader io.Reader + if body != nil { + b, _ := json.Marshal(body) + bodyReader = &readCloserWrapper{data: b} + } + req, err := http.NewRequest(method, testServer.URL+path, bodyReader) + if err != nil { + t.Fatalf("failed to create request: %v", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+testToken) + req.Header.Set("X-Workspace-ID", testWorkspaceID) + req.Header.Set("X-Agent-ID", agentID) + + r, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + return r +} + +type readCloserWrapper struct { + data []byte + pos int +} + +func (r *readCloserWrapper) Read(p []byte) (int, error) { + if r.pos >= len(r.data) { + return 0, io.EOF + } + n := copy(p, r.data[r.pos:]) + r.pos += n + return n, nil +} + +// countPendingTasks returns the number of queued/dispatched tasks for an issue. +func countPendingTasks(t *testing.T, issueID string) int { + t.Helper() + var count int + err := testPool.QueryRow(context.Background(), + `SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND status IN ('queued', 'dispatched')`, + issueID).Scan(&count) + if err != nil { + t.Fatalf("failed to count pending tasks: %v", err) + } + return count +} + +// clearTasks deletes all tasks for an issue (cleanup between subtests). +func clearTasks(t *testing.T, issueID string) { + t.Helper() + _, err := testPool.Exec(context.Background(), + `DELETE FROM agent_task_queue WHERE issue_id = $1`, issueID) + if err != nil { + t.Fatalf("failed to clear tasks: %v", err) + } +} + +// getAgentID returns the ID of the first agent in the test workspace. +func getAgentID(t *testing.T) string { + t.Helper() + resp := authRequest(t, "GET", "/api/agents?workspace_id="+testWorkspaceID, nil) + var agents []map[string]any + readJSON(t, resp, &agents) + if len(agents) == 0 { + t.Fatal("no agents in test workspace") + } + return agents[0]["id"].(string) +} + +// createIssueAssignedToAgent creates a todo issue assigned to the given agent. +func createIssueAssignedToAgent(t *testing.T, title, agentID string) string { + t.Helper() + resp := authRequest(t, "PUT", fmt.Sprintf("/api/issues/%s", createIssue(t, title)), map[string]any{ + "assignee_type": "agent", + "assignee_id": agentID, + }) + var issue map[string]any + readJSON(t, resp, &issue) + return issue["id"].(string) +} + +// createIssue creates a basic todo issue and returns its ID. +func createIssue(t *testing.T, title string) string { + t.Helper() + resp := authRequest(t, "POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{ + "title": title, + "status": "todo", + }) + if resp.StatusCode != 201 { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("CreateIssue: expected 201, got %d: %s", resp.StatusCode, body) + } + var issue map[string]any + readJSON(t, resp, &issue) + return issue["id"].(string) +} + +// postComment posts a comment as the test member. +func postComment(t *testing.T, issueID, content string, parentID *string) string { + t.Helper() + body := map[string]any{ + "content": content, + "type": "comment", + } + if parentID != nil { + body["parent_id"] = *parentID + } + resp := authRequest(t, "POST", "/api/issues/"+issueID+"/comments", body) + if resp.StatusCode != 201 { + b, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("postComment: expected 201, got %d: %s", resp.StatusCode, b) + } + var comment map[string]any + readJSON(t, resp, &comment) + return comment["id"].(string) +} + +// postCommentAsAgent posts a comment with the X-Agent-ID header. +func postCommentAsAgent(t *testing.T, issueID, content, agentID string, parentID *string) string { + t.Helper() + body := map[string]any{ + "content": content, + "type": "comment", + } + if parentID != nil { + body["parent_id"] = *parentID + } + resp := authRequestWithAgent(t, "POST", "/api/issues/"+issueID+"/comments", body, agentID) + if resp.StatusCode != 201 { + b, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("postCommentAsAgent: expected 201, got %d: %s", resp.StatusCode, b) + } + var comment map[string]any + readJSON(t, resp, &comment) + return comment["id"].(string) +} + +// strPtr returns a pointer to a string. +func strPtr(s string) *string { return &s } + +// TestCommentTriggerOnComment tests on_comment trigger scenarios end-to-end. +// Verifies that the agent task queue is populated correctly based on: +// - top-level vs threaded comments +// - member vs agent thread starters +// - presence/absence of @mentions +func TestCommentTriggerOnComment(t *testing.T) { + agentID := getAgentID(t) + issueID := createIssueAssignedToAgent(t, "Comment trigger integration test", agentID) + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + t.Run("top-level comment without mentions triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + postComment(t, issueID, "Please fix this bug", nil) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task, got %d", n) + } + }) + + t.Run("top-level comment mentioning only others suppresses trigger", func(t *testing.T) { + clearTasks(t, issueID) + // Mention a fake agent UUID that is not the assignee. + content := "[@SomeoneElse](mention://agent/00000000-0000-0000-0000-000000000001) what do you think?" + postComment(t, issueID, content, nil) + if n := countPendingTasks(t, issueID); n != 0 { + t.Errorf("expected 0 pending tasks, got %d", n) + } + }) + + t.Run("top-level comment mentioning assignee triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + content := fmt.Sprintf("[@Agent](mention://agent/%s) fix this", agentID) + postComment(t, issueID, content, nil) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task, got %d", n) + } + }) + + t.Run("reply to agent thread without mentions triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + // Agent starts a thread. + threadID := postCommentAsAgent(t, issueID, "I analyzed the issue.", agentID, nil) + // Member replies in the agent's thread. + postComment(t, issueID, "Looks good, please proceed", strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task, got %d", n) + } + }) + + t.Run("reply to member thread without mentions suppresses trigger", func(t *testing.T) { + clearTasks(t, issueID) + // Member starts a thread. + threadID := postComment(t, issueID, "Hey team, what do you think?", nil) + // Clear the task that was created by the top-level comment. + clearTasks(t, issueID) + // Another member reply (same user in this test, but the key is parent is by member). + postComment(t, issueID, "I agree with you", strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 0 { + t.Errorf("expected 0 pending tasks (member-to-member reply), got %d", n) + } + }) + + t.Run("reply to member thread mentioning assignee triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + // Member starts a thread. + threadID := postComment(t, issueID, "Question about this", nil) + clearTasks(t, issueID) + // Reply mentioning the assignee agent. + content := fmt.Sprintf("[@Agent](mention://agent/%s) can you help with this?", agentID) + postComment(t, issueID, content, strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 0 { + // The mention of the assignee agent unblocks on_comment but + // the assignee-mention path in on_mention skips the assignee. + // Either 0 or 1 is acceptable depending on the on_comment logic. + // With our implementation: isReplyToMemberThread returns false + // (assignee mentioned), and commentMentionsOthersButNotAssignee + // returns false (assignee is mentioned). So on_comment triggers. + // Let's re-check. + } + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task (assignee mentioned in member thread), got %d", n) + } + }) +} + +// TestCommentTriggerOnAssignNoStatusGate verifies that assigning an agent to +// a non-todo issue still triggers the agent (status gate was removed). +func TestCommentTriggerOnAssignNoStatusGate(t *testing.T) { + agentID := getAgentID(t) + + // Create an in_progress issue. + issueID := createIssue(t, "On-assign status gate test") + resp := authRequest(t, "PUT", "/api/issues/"+issueID, map[string]any{ + "status": "in_progress", + }) + resp.Body.Close() + + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + // Assign the agent — should trigger despite non-todo status. + resp = authRequest(t, "PUT", "/api/issues/"+issueID, map[string]any{ + "assignee_type": "agent", + "assignee_id": agentID, + }) + if resp.StatusCode != 200 { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("assign agent: expected 200, got %d: %s", resp.StatusCode, body) + } + resp.Body.Close() + + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task after assigning to in_progress issue, got %d", n) + } +} + +// TestCommentTriggerOnMentionNoStatusGate verifies that @mentioning an agent +// on a done issue still triggers the agent (no status gate on on_mention). +func TestCommentTriggerOnMentionNoStatusGate(t *testing.T) { + agentID := getAgentID(t) + + // Create a done issue (not assigned to agent). + issueID := createIssue(t, "On-mention done issue test") + resp := authRequest(t, "PUT", "/api/issues/"+issueID, map[string]any{ + "status": "done", + }) + resp.Body.Close() + + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + // @mention the agent on a done issue — should still trigger. + content := fmt.Sprintf("[@Agent](mention://agent/%s) found a problem here", agentID) + postComment(t, issueID, content, nil) + + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task after @mention on done issue, got %d", n) + } +} + +// TestCommentTriggerCoalescing verifies that rapid-fire comments don't create +// duplicate tasks (coalescing dedup). +func TestCommentTriggerCoalescing(t *testing.T) { + agentID := getAgentID(t) + issueID := createIssueAssignedToAgent(t, "Coalescing test", agentID) + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + // Post two comments rapidly — only 1 task should be created (coalescing). + postComment(t, issueID, "First comment", nil) + postComment(t, issueID, "Second comment", nil) + + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task (coalescing), got %d", n) + } +}