From 685cf65850b77ecd141ff9bb8c6b4d4b950deb09 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Thu, 30 Jul 2026 00:39:41 +0800 Subject: [PATCH] test(hub): TriggerAgentTask success + error path coverage (audit T-M1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add DB-backed unit test for DispatchService.TriggerAgentTask covering the success path and four key error paths — the highest remaining ROI coverage gap from the 2026-07-29 comprehensive audit (audit finding T-M1). Success path (TestTriggerAgentTask_SuccessAndTurnInProgressGate): - Empty targetID skips validateDispatchTarget, so the test exercises the real orchestration: GetMessageByID → GetSessionByID (#116 dissolved check) → ListAgentInstancesByInviter → SelectAgentInstance → IsMemberActive → NewQueuedPendingTask → CreatePendingTaskUnlessActive (#1430 per- agent_instance TurnInProgress gate). - Asserts returned task fields (AgentInstanceID, Status==queued, ids, TargetID/EdgeDeviceID empty) and that the pending_agent_task row is committed synchronously by CreatePendingTaskUnlessActive BEFORE the dispatch goroutine fires, so the DB read is race-free without syncing. - Second trigger for the same agent_instance is rejected with errcode.TurnInProgress (HTTP 409); anti-cheat asserts no duplicate row. Error paths (TestTriggerAgentTask_ErrorPaths, table-driven subtests): - dissolved session → errcode.SessionDissolved (#116) - no agent instances → errcode.AgentNotFound - inactive member (left_at set) → errcode.SessionNotMember - message not found → errcode.MsgNotFound Race handling: dispatchTask is nil-safe with all ports nil — cachePort() falls back to cache.NoOpCache, outbox wrappers short-circuit, mgr is gated by ManagerPortAvailable. A dead AGENTHUB_EDGE_URL routes the unbound task through the offline no-op path without a status transition, so the first task stays queued deterministically and the second trigger hits the TurnInProgress gate. No time.Sleep, no goroutine sync — assertions read the synchronously committed DB row. Schema: explicit DDL matching the newAgentTaskTargetContractDB pattern (agent_test.go:1700) — sqlite maps timestamptz to TEXT which the driver cannot scan back into time.Time, so AutoMigrate is avoided. SetMaxOpenConns(1) serializes sqlite writes so the #1430 transactional lock + status check serialize in-process. Refs: audit T-M1, #1430, #116, #100. No production code changes. --- .../service/agent_dispatch_trigger_test.go | 320 ++++++++++++++++++ 1 file changed, 320 insertions(+) create mode 100644 hub-server/internal/service/agent_dispatch_trigger_test.go diff --git a/hub-server/internal/service/agent_dispatch_trigger_test.go b/hub-server/internal/service/agent_dispatch_trigger_test.go new file mode 100644 index 00000000..05eb87d3 --- /dev/null +++ b/hub-server/internal/service/agent_dispatch_trigger_test.go @@ -0,0 +1,320 @@ +package service + +import ( + "context" + "testing" + + "github.com/agenthub/hub-server/internal/errcode" + "github.com/agenthub/hub-server/internal/model" + "github.com/glebarez/sqlite" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +// agent_dispatch_trigger_test.go covers the DB-backed success path and key +// error paths for DispatchService.TriggerAgentTask (audit T-M1, the highest +// remaining ROI coverage gap from the 2026-07-29 comprehensive audit). +// +// The success path asserts the real TriggerAgentTask orchestration against an +// in-memory SQLite DB: message lookup → session dissolved check → inviter +// agent list → agent selection → member-active check → (empty targetID skips +// validateDispatchTarget) → NewQueuedPendingTask → CreatePendingTaskUnlessActive +// (#1430 per-agent_instance TurnInProgress gate) → returned task. The +// dispatch goroutine is allowed to run against nil ports; see the race note +// below. +// +// Error paths covered (each a subtest, no goroutine concern — they return +// before dispatchTask fires): +// - dissolved session → errcode.SessionDissolved (#116) +// - no agent instances → errcode.AgentNotFound +// - inactive member → errcode.SessionNotMember +// - message not found → errcode.MsgNotFound +// +// Race note (#1430): TriggerAgentTask launches `go s.dispatchTask(...)` after +// CreatePendingTaskUnlessActive commits the pending_agent_task row +// synchronously. The success-path assertions read the committed DB row and +// the returned task — both are set before the goroutine starts, so the +// assertions are race-free without a sync primitive. The dispatch goroutine +// is nil-safe with the partial service constructed below: the nil outbox port +// returns ErrOutboxUnavailable (logged, continues with empty deliveryID), a +// dead AGENTHUB_EDGE_URL routes the unbound task to the mock cache offline +// queue (no status transition), and resolveDispatchTeamContext short-circuits +// to EmptyTeamContext because the agent has no CustomAgentID. The task +// therefore stays queued, which is also what makes the second-call +// TurnInProgress assertion deterministic. + +// newTriggerAgentTaskDB opens an in-memory SQLite DB with the tables +// TriggerAgentTask touches on the trigger path. Explicit DDL (matching the +// newAgentTaskTargetContractDB pattern in agent_test.go) keeps DATETIME +// columns scannable into time.Time — AutoMigrate maps `type:timestamptz` to +// TEXT on sqlite, which the driver cannot scan back into time.Time. +// SetMaxOpenConns(1) serializes sqlite writes so the #1430 transactional lock +// (a no-op UpdateColumn on sqlite) plus the active-status check serialize +// concurrent triggers in-process, matching the production postgres FOR UPDATE +// semantics closely enough for the gate to fire. +func newTriggerAgentTaskDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + require.NoError(t, err) + sqlDB, err := db.DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(1) + for _, ddl := range []string{ + `CREATE TABLE sessions ( + id TEXT PRIMARY KEY, + type TEXT NOT NULL, + name TEXT, + avatar_url TEXT, + announcement TEXT, + owner_user_id TEXT, + workspace_id TEXT, + next_seq INTEGER NOT NULL DEFAULT 0, + last_message_at DATETIME, + dissolved BOOLEAN NOT NULL DEFAULT FALSE, + created_at DATETIME + )`, + `CREATE TABLE session_members ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL, + member_type TEXT NOT NULL, + member_id TEXT NOT NULL, + role TEXT NOT NULL, + pinned BOOLEAN NOT NULL DEFAULT FALSE, + archived BOOLEAN NOT NULL DEFAULT FALSE, + muted BOOLEAN NOT NULL DEFAULT FALSE, + last_read_seq INTEGER NOT NULL DEFAULT 0, + joined_at DATETIME, + left_at DATETIME + )`, + `CREATE TABLE agent_instances ( + id TEXT PRIMARY KEY, + agent_type TEXT NOT NULL, + custom_agent_id TEXT, + session_id TEXT NOT NULL, + inviter_user_id TEXT NOT NULL, + workspace_id TEXT, + display_name TEXT NOT NULL, + created_at DATETIME + )`, + `CREATE TABLE messages ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL, + seq_id INTEGER NOT NULL, + client_msg_id TEXT NOT NULL, + sender_type TEXT NOT NULL, + sender_id TEXT NOT NULL, + content_type TEXT NOT NULL, + content TEXT NOT NULL, + reply_to_message_id TEXT, + recalled BOOLEAN NOT NULL DEFAULT FALSE, + edited BOOLEAN NOT NULL DEFAULT FALSE, + edited_at DATETIME, + created_at DATETIME + )`, + `CREATE TABLE pending_agent_tasks ( + id TEXT PRIMARY KEY, + agent_instance_id TEXT NOT NULL, + triggered_by_user_id TEXT NOT NULL, + trigger_message_id TEXT NOT NULL, + target_id TEXT, + status TEXT NOT NULL, + edge_run_id TEXT DEFAULT '', + edge_device_id TEXT DEFAULT '', + error_message TEXT DEFAULT '', + created_at DATETIME, + dispatched_at DATETIME, + finished_at DATETIME, + expire_at DATETIME NOT NULL + )`, + } { + require.NoError(t, db.Exec(ddl).Error) + } + return db +} + +// seedSessionUserMessage seeds a live (not dissolved) group session, an active +// user SessionMember (left_at nil), and a text trigger message in that +// session. It does NOT seed an agent instance so callers can exercise the +// "no agents available" error path. Returns the userID, sessionID, and +// triggerMessageID needed to call TriggerAgentTask. +func seedSessionUserMessage(t *testing.T, db *gorm.DB) (userID, sessionID, triggerMessageID string) { + t.Helper() + userID = "00000000-0000-0000-0000-00000000a101" + session := &model.Session{ + Type: model.SessionTypeGroup, + OwnerUserID: &userID, + } + require.NoError(t, db.Create(session).Error) + sessionID = session.ID + + require.NoError(t, db.Create(&model.SessionMember{ + SessionID: sessionID, + MemberType: model.MemberTypeUser, + MemberID: userID, + Role: model.MemberRoleOwner, + }).Error) + + msg := &model.Message{ + SessionID: sessionID, + SeqID: 1, + ClientMsgID: "client-trigger-1", + SenderType: model.SenderTypeUser, + SenderID: userID, + ContentType: model.ContentTypeText, + Content: `{"text":"run the agent"}`, + } + require.NoError(t, db.Create(msg).Error) + triggerMessageID = msg.ID + return userID, sessionID, triggerMessageID +} + +// seedTriggerFixture seeds a live session + active user member + a trigger +// message + one AgentInstance invited by that user. Returns the four IDs +// needed to call TriggerAgentTask on the success path. +func seedTriggerFixture(t *testing.T, db *gorm.DB) (userID, sessionID, agentInstanceID, triggerMessageID string) { + t.Helper() + userID, sessionID, triggerMessageID = seedSessionUserMessage(t, db) + agent := &model.AgentInstance{ + AgentType: "claude-code", + SessionID: sessionID, + InviterUserID: userID, + DisplayName: "Claude", + } + require.NoError(t, db.Create(agent).Error) + agentInstanceID = agent.ID + return userID, sessionID, agentInstanceID, triggerMessageID +} + +// TestTriggerAgentTask_SuccessAndTurnInProgressGate is the T-M1 success path: +// empty targetID (skips validateDispatchTarget) returns a queued task whose +// row is committed synchronously, and a second trigger for the same +// agent_instance is rejected with errcode.TurnInProgress (HTTP 409) by the +// #1430 per-agent_instance gate. +func TestTriggerAgentTask_SuccessAndTurnInProgressGate(t *testing.T) { + db := newTriggerAgentTaskDB(t) + userID, _, agentInstanceID, triggerMessageID := seedTriggerFixture(t, db) + + cache := &mockAgentCache{} + svc := &AgentService{db: db, cacheClient: cache} + // Dead Edge URL so the dispatch goroutine's HTTP attempt fails fast and + // falls through to the mock cache offline queue without a status + // transition — this keeps the first task queued so the second trigger + // deterministically hits the TurnInProgress gate. + t.Setenv("AGENTHUB_EDGE_URL", "http://127.0.0.1:1") + + task, err := svc.TriggerAgentTask(context.Background(), userID, triggerMessageID, agentInstanceID, "", "", "", "") + + require.NoError(t, err) + require.NotNil(t, task) + require.Equal(t, agentInstanceID, task.AgentInstanceID, "returned task must target the seeded agent_instance") + require.Equal(t, model.TaskStatusQueued, task.Status, "dispatch.NewQueuedPendingTask sets queued status") + require.Equal(t, userID, task.TriggeredByUserID) + require.Equal(t, triggerMessageID, task.TriggerMessageID) + require.Empty(t, task.TargetID, "empty targetID skips validateDispatchTarget (no execution target)") + require.Empty(t, task.EdgeDeviceID, "empty targetID yields no edge device binding") + require.NotEmpty(t, task.ID, "BeforeCreate hook assigns a uuidv7 id") + require.False(t, task.ExpireAt.IsZero(), "TTL injected by TriggerAgentTask") + + // The pending_agent_task row is committed synchronously by + // CreatePendingTaskUnlessActive BEFORE `go s.dispatchTask(...)` fires, so + // reading it here is race-free without syncing with the goroutine. + var stored model.PendingAgentTask + require.NoError(t, db.Where("id = ?", task.ID).First(&stored).Error) + require.Equal(t, model.TaskStatusQueued, stored.Status, "persisted row must be non-terminal (queued)") + require.Equal(t, agentInstanceID, stored.AgentInstanceID) + require.Equal(t, userID, stored.TriggeredByUserID) + require.Equal(t, triggerMessageID, stored.TriggerMessageID) + + // #1430: a second concurrent trigger for the same agent_instance must be + // rejected with errcode.TurnInProgress (HTTP 409) while the first task is + // non-terminal. The first task stays queued because the dead Edge URL + + // mock cache route the dispatch to the offline queue without a status + // transition, so FindActivePendingTaskByAgentInstance still observes it. + secondTask, err := svc.TriggerAgentTask(context.Background(), userID, triggerMessageID, agentInstanceID, "", "", "", "") + + require.ErrorIs(t, err, errcode.TurnInProgress) + require.Nil(t, secondTask, "rejected trigger must not return a task") + + // Anti-cheat: the gate must not create a duplicate row. The first task + // remains the only pending_agent_tasks row for this agent_instance. + var count int64 + require.NoError(t, db.Table("pending_agent_tasks"). + Where("agent_instance_id = ?", agentInstanceID).Count(&count).Error) + require.Equal(t, int64(1), count, "TurnInProgress gate must not create a duplicate task") +} + +// TestTriggerAgentTask_ErrorPaths covers the cheap error paths that return +// before dispatchTask fires (#116, #100). Each subtest builds its own +// in-memory DB so failures are isolated. +func TestTriggerAgentTask_ErrorPaths(t *testing.T) { + t.Run("DissolvedSession_returnsSessionDissolved", func(t *testing.T) { + // #116: reject new agent tasks for dissolved sessions. + db := newTriggerAgentTaskDB(t) + userID, sessionID, agentInstanceID, triggerMessageID := seedTriggerFixture(t, db) + require.NoError(t, db.Model(&model.Session{}). + Where("id = ?", sessionID).Update("dissolved", true).Error) + + svc := &AgentService{db: db, cacheClient: &mockAgentCache{}} + task, err := svc.TriggerAgentTask(context.Background(), userID, triggerMessageID, agentInstanceID, "", "", "", "") + + require.ErrorIs(t, err, errcode.SessionDissolved) + require.Nil(t, task) + var count int64 + require.NoError(t, db.Table("pending_agent_tasks").Count(&count).Error) + require.Equal(t, int64(0), count, "no task must be created for a dissolved session") + }) + + t.Run("NoAgentInstances_returnsAgentNotFound", func(t *testing.T) { + // Inviter has no agent instances in the session → TriggerAgentsAvailableError + // collapses the empty list into errcode.AgentNotFound. + db := newTriggerAgentTaskDB(t) + userID, _, triggerMessageID := seedSessionUserMessage(t, db) + + svc := &AgentService{db: db, cacheClient: &mockAgentCache{}} + task, err := svc.TriggerAgentTask(context.Background(), userID, triggerMessageID, "", "", "", "", "") + + require.ErrorIs(t, err, errcode.AgentNotFound) + require.Nil(t, task) + var count int64 + require.NoError(t, db.Table("pending_agent_tasks").Count(&count).Error) + require.Equal(t, int64(0), count, "no task must be created when no agent instances are available") + }) + + t.Run("InactiveMember_returnsSessionNotMember", func(t *testing.T) { + // left_at set on the user member → IsMemberActive returns false → + // TriggerMemberActiveError surfaces errcode.SessionNotMember. A lookup + // failure must NOT be misread as inactive (audit T-M1 honest-error + // guarantee, pinned by TestTriggerAgentTask_MemberActiveLookupErrorSurfaces). + db := newTriggerAgentTaskDB(t) + userID, sessionID, agentInstanceID, triggerMessageID := seedTriggerFixture(t, db) + require.NoError(t, db.Model(&model.SessionMember{}). + Where("session_id = ? AND member_type = ? AND member_id = ?", sessionID, model.MemberTypeUser, userID). + Update("left_at", "2030-01-01T00:00:00Z").Error) + + svc := &AgentService{db: db, cacheClient: &mockAgentCache{}} + task, err := svc.TriggerAgentTask(context.Background(), userID, triggerMessageID, agentInstanceID, "", "", "", "") + + require.ErrorIs(t, err, errcode.SessionNotMember) + require.Nil(t, task) + var count int64 + require.NoError(t, db.Table("pending_agent_tasks").Count(&count).Error) + require.Equal(t, int64(0), count, "no task must be created for an inactive member") + }) + + t.Run("MessageNotFound_returnsMsgNotFound", func(t *testing.T) { + // MapTriggerMessageLookupError collapses any GetMessageByID failure + // (including gorm.ErrRecordNotFound) into errcode.MsgNotFound. + db := newTriggerAgentTaskDB(t) + userID, _, agentInstanceID, _ := seedTriggerFixture(t, db) + + svc := &AgentService{db: db, cacheClient: &mockAgentCache{}} + task, err := svc.TriggerAgentTask(context.Background(), userID, "nonexistent-trigger-message-id", agentInstanceID, "", "", "", "") + + require.ErrorIs(t, err, errcode.MsgNotFound) + require.Nil(t, task) + var count int64 + require.NoError(t, db.Table("pending_agent_tasks").Count(&count).Error) + assert.Equal(t, int64(0), count, "no task must be created when the trigger message is not found") + }) +}