fix(office): stop agents from being woken by their own comments (#2840)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>

apps/backend/cmd/agentctl/kandev_comment.go

@@ -25,7 +25,8 @@ func runCommentCmd(args []string) int {
 	}
 }
 
-// commentAdd posts a comment on a task. If --body is "-", reads from stdin.
+// commentAdd posts a comment through the signed runtime scope. If --body is
+// "-", reads from stdin.
 func commentAdd(args []string) int {
 	fs := flag.NewFlagSet("comment add", flag.ContinueOnError)
 	taskFlag := fs.String("task", "", "Task ID (defaults to $KANDEV_TASK_ID)")
@@ -58,13 +59,11 @@ func commentAdd(args []string) int {
 	}
 
 	payload := map[string]string{
-		"body":        body,
-		"author_type": "agent",
-		"author_id":   client.agentID,
+		"task_id": taskID,
+		"body":    body,
 	}
 
-	respBody, status, doErr := client.do(http.MethodPost,
-		fmt.Sprintf("/api/v1/office/tasks/%s/comments", taskID), payload)
+	respBody, status, doErr := client.do(http.MethodPost, "/api/v1/office/runtime/comments", payload)
 	return handleResponse(respBody, status, doErr)
 }
 

apps/backend/cmd/agentctl/kandev_test.go

@@ -584,8 +584,12 @@ func TestOfficeSetup_ProjectThenTaskUsesReturnedProjectAndWorkspace(t *testing.T
 // --- Comment Tests ---
 
 func TestCommentAdd_PostsComment(t *testing.T) {
-	srv, captured := setupMockServer(t, 201, `{"ok":true}`)
-	setEnvVars(t, srv)
+	captured := setupMockTransport(t, http.StatusCreated, `{"ok":true}`)
+	t.Setenv("KANDEV_API_URL", "http://kandev.test")
+	t.Setenv("KANDEV_API_KEY", "signed-office-run-token")
+	t.Setenv("KANDEV_RUN_ID", "run-456")
+	t.Setenv("KANDEV_TASK_ID", "task-abc")
+	t.Setenv("KANDEV_AGENT_ID", "agent-789")
 
 	code := runKandevCLI([]string{
 		"comment", "add", "--body", "This is a test comment",
@@ -596,19 +600,25 @@ func TestCommentAdd_PostsComment(t *testing.T) {
 	if captured.Method != "POST" {
 		t.Errorf("expected POST, got %s", captured.Method)
 	}
-	if captured.Path != "/api/v1/office/tasks/task-abc/comments" {
+	if captured.Path != "/api/v1/office/runtime/comments" {
 		t.Errorf("unexpected path: %s", captured.Path)
 	}
+	assertRunIDHeader(t, captured, "run-456")
 
-	var body map[string]string
+	var body map[string]any
 	if err := json.Unmarshal([]byte(captured.Body), &body); err != nil {
 		t.Fatalf("unmarshal body: %v", err)
 	}
 	if body["body"] != "This is a test comment" {
-		t.Errorf("unexpected body: %s", body["body"])
+		t.Errorf("unexpected body: %v", body["body"])
 	}
-	if body["author_id"] != "agent-789" {
-		t.Errorf("expected author_id=agent-789, got %s", body["author_id"])
+	if body["task_id"] != "task-abc" {
+		t.Errorf("expected task_id=task-abc, got %v", body["task_id"])
+	}
+	for _, key := range []string{"author_type", "author_id", "source"} {
+		if _, ok := body[key]; ok {
+			t.Errorf("runtime comment payload must not contain %q: %#v", key, body)
+		}
 	}
 }
 

apps/backend/internal/office/configloader/skills/kandev-task-ops/references/comments.md

@@ -12,6 +12,6 @@ Comment on another task:
 $KANDEV_CLI kandev tasks message --id T-42 --prompt "Blocked: the worktree has a dirty submodule. Owner: please investigate."
 ```
 
-`tasks message` writes as the current agent. `comment add` writes as the user, so only use `comment add` when acting as a test fixture or intentionally simulating user input.
+Both `tasks message` and `comment add` write as the current agent.
 
 Avoid empty acknowledgements such as `Done.` unless they include useful progress. Link to file paths or commits instead of quoting long code blocks.

apps/backend/internal/office/dashboard/handler_comments.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/google/uuid"
+	"github.com/kandev/kandev/internal/office/agents"
 	"github.com/kandev/kandev/internal/office/models"
 	"github.com/kandev/kandev/internal/office/repository/sqlite"
 	"go.uber.org/zap"
@@ -72,14 +73,20 @@ func (h *Handler) createComment(c *gin.Context) {
 		c.JSON(http.StatusBadRequest, gin.H{"error": "body is required"})
 		return
 	}
-	authorType := req.AuthorType
-	if authorType == "" {
-		authorType = userSentinel
+	taskID := c.Param("id")
+	if agents.CallerFromContext(c) != nil ||
+		(req.AuthorType != "" && req.AuthorType != userSentinel) {
+		h.logger.Warn("reject agent comment on dashboard endpoint",
+			zap.String("task_id", taskID))
+		c.JSON(http.StatusForbidden, gin.H{
+			"error": "agent comments must use the runtime comments endpoint",
+		})
+		return
 	}
 	comment := &models.TaskComment{
 		ID:         uuid.New().String(),
-		TaskID:     c.Param("id"),
-		AuthorType: authorType,
+		TaskID:     taskID,
+		AuthorType: userSentinel,
 		AuthorID:   userSentinel,
 		Body:       req.Body,
 		Source:     userSentinel,

apps/backend/internal/office/dashboard/handler_comments_security_test.go

@@ -0,0 +1,191 @@
+package dashboard_test
+
+import (
+	"bytes"
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/jmoiron/sqlx"
+	_ "github.com/mattn/go-sqlite3"
+
+	settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
+	"github.com/kandev/kandev/internal/common/logger"
+	"github.com/kandev/kandev/internal/office/agents"
+	"github.com/kandev/kandev/internal/office/dashboard"
+	"github.com/kandev/kandev/internal/office/models"
+	"github.com/kandev/kandev/internal/office/repository/sqlite"
+	"github.com/kandev/kandev/internal/office/shared"
+	workflowrepo "github.com/kandev/kandev/internal/workflow/repository"
+)
+
+// commentSecurityFixture wires the authenticated dashboard stack. The route
+// accepts UI requests without a token, but agent tokens must use the runtime
+// endpoint so capability and task-scope checks cannot be bypassed.
+type commentSecurityFixture struct {
+	router    *gin.Engine
+	agentsSvc *agents.AgentService
+	repo      *sqlite.Repository
+}
+
+func newCommentSecurityFixture(t *testing.T) *commentSecurityFixture {
+	t.Helper()
+	gin.SetMode(gin.TestMode)
+
+	db, err := sqlx.Open("sqlite3", ":memory:")
+	if err != nil {
+		t.Fatalf("open sqlite: %v", err)
+	}
+	db.SetMaxOpenConns(1)
+	t.Cleanup(func() { _ = db.Close() })
+
+	if _, _, err := settingsstore.Provide(db, db, nil); err != nil {
+		t.Fatalf("settings store: %v", err)
+	}
+
+	repo, err := sqlite.NewWithDB(db, db, nil)
+	if err != nil {
+		t.Fatalf("new repo: %v", err)
+	}
+
+	_, err = db.Exec(`
+		CREATE TABLE IF NOT EXISTS tasks (
+			id TEXT PRIMARY KEY,
+			workspace_id TEXT NOT NULL DEFAULT '',
+			title TEXT NOT NULL DEFAULT '',
+			description TEXT DEFAULT '',
+			state TEXT DEFAULT 'todo',
+			priority TEXT NOT NULL DEFAULT 'medium' CHECK (priority IN ('critical','high','medium','low')),
+			parent_id TEXT DEFAULT '',
+			project_id TEXT DEFAULT '',
+			assignee_agent_profile_id TEXT DEFAULT '',
+			labels TEXT DEFAULT '[]',
+			metadata TEXT DEFAULT '{}',
+			identifier TEXT DEFAULT '',
+			is_ephemeral INTEGER DEFAULT 0,
+			origin TEXT DEFAULT 'manual',
+			execution_policy TEXT DEFAULT '',
+			execution_state TEXT DEFAULT '',
+			workflow_id TEXT NOT NULL DEFAULT '',
+			workflow_step_id TEXT DEFAULT '',
+			archived_at TIMESTAMP,
+			created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
+			updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
+		)
+	`)
+	if err != nil {
+		t.Fatalf("create tasks table: %v", err)
+	}
+	_, err = db.Exec(`CREATE TABLE IF NOT EXISTS workspaces (
+		id TEXT PRIMARY KEY,
+		office_workflow_id TEXT DEFAULT ''
+	)`)
+	if err != nil {
+		t.Fatalf("create workspaces table: %v", err)
+	}
+	_, err = db.Exec(`CREATE TABLE IF NOT EXISTS workflows (
+		id TEXT PRIMARY KEY, workspace_id TEXT NOT NULL DEFAULT '',
+		workflow_template_id TEXT DEFAULT '', name TEXT NOT NULL,
+		description TEXT DEFAULT '', created_at TIMESTAMP NOT NULL, updated_at TIMESTAMP NOT NULL
+	)`)
+	if err != nil {
+		t.Fatalf("create workflows: %v", err)
+	}
+	if _, err := workflowrepo.NewWithDB(db, db, nil); err != nil {
+		t.Fatalf("workflow repo: %v", err)
+	}
+
+	log := logger.Default()
+	activity := shared.NewActivityLogger(repo, log)
+	agentsSvc := agents.NewAgentService(repo, log, nil)
+	agentsSvc.SetAuth(agents.NewAgentAuth("test-key"))
+	svc := dashboard.NewDashboardService(repo, log, activity, agentsSvc, &stubCostChecker{})
+
+	r := gin.New()
+	r.Use(agents.AgentAuthMiddleware(agentsSvc))
+	group := r.Group("/api/v1/office")
+	dashboard.RegisterRoutes(group, svc, repo, nil, log)
+
+	return &commentSecurityFixture{router: r, agentsSvc: agentsSvc, repo: repo}
+}
+
+func seedCommentAgent(t *testing.T, svc *agents.AgentService, id, workspaceID string) *models.AgentInstance {
+	t.Helper()
+	a := &models.AgentInstance{
+		ID:          id,
+		WorkspaceID: workspaceID,
+		Name:        id,
+		Role:        models.AgentRoleWorker,
+		Status:      models.AgentStatusIdle,
+		Permissions: shared.DefaultPermissions(shared.AgentRoleWorker),
+	}
+	if err := svc.CreateAgentInstance(context.Background(), a); err != nil {
+		t.Fatalf("create agent %q: %v", id, err)
+	}
+	return a
+}
+
+func seedCommentTask(t *testing.T, repo *sqlite.Repository, id, workspaceID, assigneeAgentID string) {
+	t.Helper()
+	_, err := repo.ExecRaw(context.Background(),
+		`INSERT INTO tasks (id, workspace_id, title, assignee_agent_profile_id) VALUES (?, ?, ?, ?)`,
+		id, workspaceID, "task", assigneeAgentID,
+	)
+	if err != nil {
+		t.Fatalf("seed task %q: %v", id, err)
+	}
+}
+
+func postCommentReq(taskID, token, body string) *http.Request {
+	req := httptest.NewRequest(
+		http.MethodPost,
+		"/api/v1/office/tasks/"+taskID+"/comments",
+		bytes.NewBufferString(body),
+	)
+	req.Header.Set("Content-Type", "application/json")
+	if token != "" {
+		req.Header.Set("Authorization", "Bearer "+token)
+	}
+	return req
+}
+
+func TestCreateComment_AgentCallerMustUseRuntimeEndpoint(t *testing.T) {
+	f := newCommentSecurityFixture(t)
+	agent := seedCommentAgent(t, f.agentsSvc, "agent-a", "ws-1")
+	seedCommentTask(t, f.repo, "task-1", "ws-1", agent.ID)
+	seedCommentTask(t, f.repo, "task-2", "ws-1", "")
+
+	// The token has no post_comment capability and is scoped to task-1. A
+	// dashboard request for task-2 must still be rejected before persistence.
+	token, err := f.agentsSvc.MintRuntimeJWT(agent.ID, "task-1", agent.WorkspaceID, "run-1", "sess-1", "")
+	if err != nil {
+		t.Fatalf("mint jwt: %v", err)
+	}
+	rec := httptest.NewRecorder()
+	f.router.ServeHTTP(rec, postCommentReq("task-2", token, `{"body":"hello"}`))
+
+	if rec.Code != http.StatusForbidden {
+		t.Fatalf("status = %d, want 403; body=%s", rec.Code, rec.Body.String())
+	}
+	comments, err := f.repo.ListTaskComments(context.Background(), "task-2")
+	if err != nil {
+		t.Fatalf("list comments: %v", err)
+	}
+	if len(comments) != 0 {
+		t.Fatalf("comments = %+v, want none persisted", comments)
+	}
+}
+
+func TestCreateComment_RejectsAgentAuthorTypeWithoutJWT(t *testing.T) {
+	f := newCommentSecurityFixture(t)
+	seedCommentTask(t, f.repo, "task-1", "ws-1", "")
+
+	rec := httptest.NewRecorder()
+	f.router.ServeHTTP(rec, postCommentReq("task-1", "", `{"body":"hello","author_type":"agent"}`))
+
+	if rec.Code != http.StatusForbidden {
+		t.Fatalf("status = %d, want 403; body=%s", rec.Code, rec.Body.String())
+	}
+}

apps/backend/internal/office/dashboard/handler_test.go

@@ -651,6 +651,38 @@ func TestCreateComment_RequiresBody(t *testing.T) {
 	}
 }
 
+// TestCreateComment_UserAuthorUnchanged locks in that the UI comment path
+// (author_type unset or "user") still persists the user sentinel — the
+// fix must not touch this path.
+func TestCreateComment_UserAuthorUnchanged(t *testing.T) {
+	deps := newTestDeps(t)
+
+	body := `{"body":"hello from the UI","author_type":"user"}`
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/office/tasks/taskUser/comments",
+		strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+	deps.router.ServeHTTP(w, req)
+
+	if w.Code != http.StatusCreated {
+		t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
+	}
+
+	var resp dashboard.CommentResponse
+	if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	if resp.Comment == nil {
+		t.Fatal("expected comment in response")
+	}
+	if resp.Comment.AuthorID != "user" {
+		t.Errorf("expected authorId 'user', got %q", resp.Comment.AuthorID)
+	}
+	if resp.Comment.Source != "user" {
+		t.Errorf("expected source 'user', got %q", resp.Comment.Source)
+	}
+}
+
 func TestUpdateWorkspaceSettings_ReturnsOK(t *testing.T) {
 	deps := newTestDeps(t)
 

apps/backend/internal/office/runtime/handler_test.go

@@ -172,7 +172,7 @@ func TestRuntimeHandler_PostCommentUsesRuntimeToken(t *testing.T) {
 		t.Fatalf("comments = %d, want 1", len(h.comments.comments))
 	}
 	comment := h.comments.comments[0]
-	if comment.TaskID != "task-1" || comment.AuthorID != "agent-1" || comment.AuthorType != "agent" {
+	if comment.TaskID != "task-1" || comment.AuthorID != "agent-1" || comment.AuthorType != "agent" || comment.Source != "agent" {
 		t.Fatalf("comment identity = %#v", comment)
 	}
 	assertActionRunEvent(t, h.runEvents, "post_comment", "task", "task-1")

apps/backend/internal/office/scheduler/reactivity_test.go

@@ -0,0 +1,90 @@
+package scheduler
+
+import (
+	"context"
+	"testing"
+
+	"github.com/kandev/kandev/internal/common/logger"
+)
+
+// recordedQueueCall captures one queue(agentID, RunContext) invocation from
+// reactToComment for assertion.
+type recordedQueueCall struct {
+	agentID string
+	ctx     RunContext
+}
+
+// TestReactToComment_SelfCommentDoesNotWakeAssignee is the card's required
+// regression: a comment authored by the task's own assignee (author_type
+// "agent", author_id == the assignee's agent profile id) must NOT queue a
+// task_comment run for that assignee. This is the guard in reactToComment
+// (reactivity.go); it only works when the comment's author_id was
+// persisted as the acting agent's real profile id (dashboard's
+// createComment handler is the thing that used to break this by hardcoding
+// author_id="user").
+func TestReactToComment_SelfCommentDoesNotWakeAssignee(t *testing.T) {
+	ss := &SchedulerService{logger: logger.Default()}
+	task := &TaskSnapshot{
+		ID:                     "task-1",
+		WorkspaceID:            "ws-1",
+		State:                  "IN_PROGRESS",
+		AssigneeAgentProfileID: "agent-a",
+	}
+	comment := &MutationComment{
+		ID:         "comment-1",
+		Body:       "Landscape scan complete, no action needed.",
+		AuthorType: "agent",
+		AuthorID:   "agent-a",
+	}
+
+	var calls []recordedQueueCall
+	queue := func(agentID string, c RunContext) {
+		calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
+	}
+
+	ss.reactToComment(context.Background(), task, comment, queue)
+
+	for _, c := range calls {
+		if c.agentID == "agent-a" && c.ctx.Reason == RunReasonTaskComment {
+			t.Fatalf("agent-a was woken by its own comment: %+v", c)
+		}
+	}
+}
+
+// TestReactToComment_OtherAuthorWakesAssignee is the positive counterpart:
+// a comment from a different agent (or the user) on a task assigned to
+// agent-b DOES queue a task_comment run for agent-b. Preserving this is
+// explicitly required by the card — the fix must not suppress cross-agent
+// wakes while closing the self-wake hole.
+func TestReactToComment_OtherAuthorWakesAssignee(t *testing.T) {
+	ss := &SchedulerService{logger: logger.Default()}
+	task := &TaskSnapshot{
+		ID:                     "task-2",
+		WorkspaceID:            "ws-1",
+		State:                  "IN_PROGRESS",
+		AssigneeAgentProfileID: "agent-b",
+	}
+	comment := &MutationComment{
+		ID:         "comment-2",
+		Body:       "Please take a look at this.",
+		AuthorType: "agent",
+		AuthorID:   "agent-a",
+	}
+
+	var calls []recordedQueueCall
+	queue := func(agentID string, c RunContext) {
+		calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
+	}
+
+	ss.reactToComment(context.Background(), task, comment, queue)
+
+	found := false
+	for _, c := range calls {
+		if c.agentID == "agent-b" && c.ctx.Reason == RunReasonTaskComment {
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("expected agent-b to be queued a task_comment run, got calls: %+v", calls)
+	}
+}