Merge pull request #306 from multica-ai/agent/lambda/832eb090
fix(server): improve comment trigger logic for agent execution
This commit is contained in:
Jiayuan Zhang
GitHub
commit
8f1526d2bb
5 changed files with 738 additions and 27 deletions
332
server/cmd/server/comment_trigger_integration_test.go
Normal file
332
server/cmd/server/comment_trigger_integration_test.go
Normal file
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue