test(hub): TriggerAgentTask success + error path coverage (audit T-M1) - #1476
Conversation
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.
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Audit reference
Covers audit finding T-M1 (highest remaining ROI — critical path coverage gap) from the 2026-07-29 comprehensive AgentHub audit.
What's covered
New DB-backed unit test for
DispatchService.TriggerAgentTask(hub-server/internal/service/agent_dispatch.go:28) covering the success path + four key error paths.Success path —
TestTriggerAgentTask_SuccessAndTurnInProgressGateSeeds a live session (not dissolved), an active
SessionMember(left_at nil), anAgentInstanceinvited by that user, and a text triggerMessage. CallsTriggerAgentTaskwith emptytargetID(skipsvalidateDispatchTarget), exercising the real orchestration end-to-end:GetMessageByID→GetSessionByID(#116 dissolved check) →ListAgentInstancesByInviter→SelectAgentInstance→IsMemberActive→NewQueuedPendingTask→CreatePendingTaskUnlessActive(#1430 per-agent_instance TurnInProgress gate) →go s.dispatchTask(...).Asserts:
task.AgentInstanceID,task.Status == model.TaskStatusQueued,task.TriggeredByUserID,task.TriggerMessageIDmatchtask.TargetIDandtask.EdgeDeviceIDempty (empty targetID path)task.IDnon-empty (uuidv7 BeforeCreate hook),task.ExpireAtnon-zero (TTL injected)pending_agent_tasksrow exists in DB with non-terminal queued status#1430 TurnInProgress gate (anti-cheat)
A second
TriggerAgentTaskcall for the sameagent_instanceis rejected witherrcode.TurnInProgress(HTTP 409); asserts the gate does not create a duplicate row. The first task stays queued because the dead Edge URL + mock cache route the dispatch through the offline no-op path without a status transition, soFindActivePendingTaskByAgentInstancedeterministically observes it.Error paths —
TestTriggerAgentTask_ErrorPaths(table-driven subtests)Each returns before
dispatchTaskfires (no goroutine concern):errcode.SessionDissolvederrcode.AgentNotFounderrcode.SessionNotMembererrcode.MsgNotFoundRace-handling decision
TriggerAgentTasklaunchesgo s.dispatchTask(...)afterCreatePendingTaskUnlessActivecommits the row synchronously. VerifieddispatchTaskis fully nil-safe with all ports nil:cachePort()falls back tocache.NoOpCache{}viaresolveDispatchCache(cache_fallback.go:29) —GetRoute/PushPendingTaskare no-ops, never NPE.recordDelivery/markDeliverySentare nil-safe outbox wrappers (agent_dispatch_ports.go:156/164) — short-circuit when outbox nil.s.mgr.FindByConnID/PushToConnonly run whendispatch.ManagerPortAvailable(s.mgr != nil)is true; withmgr=nilthe route falls to the offline/cache branch.targetID=""→RouteHTTP→dispatchToEdgeHTTP(dead Edge URL → returns "") →cacheClient.GetRoute(NoOp → "") →RouteOffline→PushPendingTask(NoOp) → return. The goroutine completes cleanly with no NPE and no observable side effect on the task row.Therefore the success-path assertions read the committed DB row (written synchronously by
CreatePendingTaskUnlessActivebefore the goroutine fires) and the returned task. Notime.Sleep, no goroutine sync — the DB read is race-free.Schema choice
Explicit DDL matching the
newAgentTaskTargetContractDBpattern (agent_test.go:1700).AutoMigratemapstype:timestamptzto TEXT on sqlite, which the driver cannot scan back intotime.Time(the existing contract DB pattern avoids AutoMigrate for the same reason).SetMaxOpenConns(1)serializes sqlite writes so the #1430 transactional lock (a no-opUpdateColumnon sqlite) plus the active-status check serialize concurrent triggers in-process, matching production postgresFOR UPDATEsemantics closely enough for the gate to fire.Verification
Also
go vet ./internal/service/clean, fullgo test ./internal/service/clean, and 50x repeat of the success-path test clean.Refs: audit T-M1, #1430, #116, #100. No production code changes — test-only.